Conversation
zoekt.repoid was parsed with strconv.ParseUint(v, 10, 32) and the error discarded, both at index time and when reading shard metadata. Go returns the maximum value on overflow, so every ID above 4294967295 became 4294967295 and collided in ReposMap, compound shard lookups, tombstones and RepoIDs filters. Malformed values became 0. Add zoekt.ParseRepoID and use it at every parse site so an invalid ID fails the index build, fails shard metadata loading (logged and skipped), and is rejected by zoekt-git-clone -repoid and the indexserver's HTTP and debug handlers, which previously wrapped via uint32(Atoi(v)). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
indexGitRepo logs and tolerates setTemplatesFromRepo errors because template and URL problems only degrade result links. A local smoke test showed that an out-of-range zoekt.repoid therefore still produced a shard, with ID 0 and without its raw config, instead of failing. Wrap ParseRepoID errors in a new zoekt.ErrInvalidRepoID sentinel and return from indexGitRepo when errors.Is matches it, so zoekt-git-index exits non-zero and writes no shard. Other config errors are still logged and tolerated. Add an end-to-end test through IndexGitRepo. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1170
Problem
zoekt.repoidis parsed withstrconv.ParseUint(v, 10, 32)and the error is discarded, both at index time (gitindex) and at metadata read time (Repository.UnmarshalJSON). Go returns the maximum value on overflow, so any ID above 4294967295 is silently stored as 4294967295 and collides with every other oversized repository inReposMap, compound shard lookups, tombstones andRepoIDsfilters. A malformed value is silently stored as 0.Changes
api.go: addParseRepoID, which rejects malformed and out-of-range values with a descriptive error.Repository.UnmarshalJSONnow uses it and returns the error. An emptyrepoidis treated as absent, as before.gitindex/index.go:setTemplatesFromRepoConfigusesParseRepoIDand returns the error, sozoekt-git-indexfails the build instead of writing a shard with a colliding ID.cmd/zoekt-git-clone/main.go:-repoidvalues above the uint32 maximum are rejected up front instead of being written to git config and clamped later.cmd/zoekt-sourcegraph-indexserver: theidandrepoHTTP parameters and thedebug indexargument useParseRepoIDinstead ofstrconv.Atoiplus auint32conversion, which wrapped modulo 2^32. An emptyrepoparameter still returns 400.Behaviour change
A shard whose stored
repoidcannot be parsed now failsReadMetadata. The shard loader already logs and skips shards that fail to load, so such a shard is dropped from search with an[ERROR] reloading:log line instead of being served under a colliding ID. These shards were already corrupt, since their ID collided with every other oversized repository, but reviewers should be aware this makes the corruption visible.Test plan
TestParseRepoID: valid, zero, max uint32, empty, out of range, negative, malformed.TestRepositoryUnmarshalJSON_RepoID: out-of-range and malformedrepoidreturn an error; emptyrepoidleaves the JSONIDuntouched.TestSetTemplates_RepoID: end-to-end throughgit config zoekt.repoidfor valid, max, out-of-range and malformed values.go test ./... -shortpasses locally.🤖 Generated with Claude Code