fix(vector): reject non-positive similar_to neighbor count#9767
Open
eharris128 wants to merge 1 commit into
Open
fix(vector): reject non-positive similar_to neighbor count#9767eharris128 wants to merge 1 commit into
eharris128 wants to merge 1 commit into
Conversation
The number-of-neighbors argument of a similar_to() vector query is parsed from the request with no lower-bound check (worker/task.go). A zero or negative value reaches the HNSW bottom-layer search on the default (no ef / distance_threshold) path, where addPathNode computes effectiveMaxLen = maxResults + filtered and then evaluates slr.neighbors[:effectiveMaxLen]. A negative value panics with a negative slice bound, and zero empties the slice and then dereferences slr.neighbors[0], so a single query with a neighbor count <= 0 panics the query goroutine and takes down the Alpha. Reject a non-positive neighbor count at the request boundary with a clear error, and harden the HNSW library as defense in depth: mirror the maxResults < 0 clamp that SearchWithOptions already applies into SearchWithPath, and clamp effectiveMaxLen plus guard the neighbors[0] dereference in addPathNode. Add a regression test for addPathNode with non-positive maxResults.
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.
Description
A
similar_to()vector query takes a client-supplied number of neighbors. That argument is parsed inworker/task.gowithout a lower-bound check, so a zero or negative value flows into the HNSW search. On the default search path (noef/distance_thresholdoption),SearchWithPathpasses it through unclamped, andaddPathNode(tok/hnsw/search_layer.go) computeseffectiveMaxLen = maxResults + filteredand then evaluatesslr.neighbors[:effectiveMaxLen]. A negative count panics with a negative slice bound ([:-1]); a count of zero empties the slice and then dereferencesslr.neighbors[0]. Either way a single query with a neighbor count<= 0panics the query goroutine and crashes the Alpha. TheSearchWithOptionspath already guards against this with anif maxResults < 0 { maxResults = 0 }clamp; only the default path was left unprotected.This change:
worker/task.go) with a clear error, so an invalidsimilar_tocount returns a normal query error instead of crashing the server.SearchWithOptionsclamp intoSearchWithPathas defense in depth.effectiveMaxLenand guards theslr.neighbors[0]dereference inaddPathNode, so the bottom-layer search cannot panic on a non-positive limit regardless of caller.TestAddPathNodeNonPositiveMaxResults) that reproduces the panic on the old code and passes with the fix.Valid queries are unaffected: a normal positive neighbor count truncates exactly as before.
Checklist
fix:,feat:,chore:,ci:, etc.