Skip to content

fix(http): reject negative limit/offset on list endpoints with 422 instead of raw Postgres 500#2357

Open
r266-tech wants to merge 1 commit into
vectorize-io:mainfrom
r266-tech:fix/list-endpoint-negative-pagination-422
Open

fix(http): reject negative limit/offset on list endpoints with 422 instead of raw Postgres 500#2357
r266-tech wants to merge 1 commit into
vectorize-io:mainfrom
r266-tech:fix/list-endpoint-negative-pagination-422

Conversation

@r266-tech

Copy link
Copy Markdown
Contributor

Several user-facing GET list endpoints declared limit/offset without FastAPI ge constraints, so a negative value flows straight into Postgres LIMIT/OFFSET (the engine emits LIMIT $n OFFSET $n with no max(0, ...) clamp), which raises LIMIT/OFFSET must not be negative. The generic except Exception -> HTTPException(500, str(e)) then turns a client input error into a 500 that also leaks the raw Postgres error string.

Affected endpoints (all verified against memory_engine.py SQL): GET …/graph, …/memories/list, …/documents, …/tags, …/entities, …/entities/graph.

The same router already enforces this on sibling list endpoints — document-chunks, directives, async-ops, audit all use Query(..., ge=…). This makes the missing ones consistent.

Fix: add Query(ge=0) to the limit/offset params. ge=0 rejects only negatives, so limit=0 (a valid empty page, LIMIT 0) and offset=0 still work — no behavior change for any previously-valid input, only the negative inputs that were always broken now get a clean 422 instead of a 500.

Test: test_list_endpoint_pagination_validation.py asserts negative limit → 422 across all six endpoints, negative offset → 422 where applicable, and a positive control that limit ∈ {0, 1, 100} is still accepted.

…stead of 500

Several user-facing GET list endpoints declared limit/offset without ge
constraints, so a negative value flowed straight into Postgres LIMIT/OFFSET
(emitted with no max(0, ...) clamp), which raises 'LIMIT/OFFSET must not be
negative'. The generic `except Exception -> HTTPException(500, str(e))` then
turned a client input error into a 500 that also leaked the raw Postgres error
string.

Add Query(ge=...) constraints (limit ge=0, offset ge=0) on the affected
endpoints (graph, memories/list, documents, tags, entities, entities/graph),
matching the ge constraints already enforced on the sibling list endpoints
(document-chunks, directives, async-ops, audit) so FastAPI returns a clean 422
at the boundary. ge=0 rejects only negatives and preserves limit=0 (a valid
empty page), so there is no behavior change for any previously-valid request.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant