fix(validation): reject non-positive page/limit in pagination query schemas#1271
Open
abhay-codes07 wants to merge 1 commit into
Open
fix(validation): reject non-positive page/limit in pagination query schemas#1271abhay-codes07 wants to merge 1 commit into
abhay-codes07 wants to merge 1 commit into
Conversation
…chemas
Both pagination query schemas validated only the upper bound, so
malformed pagination flowed straight through the API's source-of-truth
validation into the query layer instead of failing with a clean 400:
- ListMemoriesQuerySchema guards its string branch with /^\d+$/ (which
still admits "0"), but the z.number() branch accepted any number:
{ page: -1, limit: -5 } and fractional values like 2.5 parsed
successfully; only limit > 1100 was rejected
- DocumentsWithMemoriesQuerySchema (the input schema for
POST /documents/documents) had bare z.number() for both page and
limit with no bounds at all
Require page and limit to be positive integers in both schemas. The
1100 limit cap and all defaults are unchanged, and no new upper bound
is introduced for the documents schema since existing web flows
legitimately request large pages.
Covered with bun tests: defaults, numeric and numeric-string
acceptance, the 1100 cap, and rejection of zero/negative/fractional
values on both schemas.
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
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.
What
Both pagination query schemas in
@repo/validationvalidate only the upper bound, so malformed pagination passes the API's source-of-truth validation and flows into the query layer instead of failing with a clean 400:ListMemoriesQuerySchemaguards its string branch with/^\d+$/(which still admits"0"), but thez.number()branch accepts anything:{ page: -1, limit: -5 }and fractional values like2.5all parse successfully. Onlylimit > 1100is rejected.DocumentsWithMemoriesQuerySchema— the input schema forPOST /documents/documents, which the web app and MCP both use — has barez.number()forpageandlimitwith no bounds at all.Fix
Require
pageandlimitto be positive integers in both schemas:ListMemoriesQuerySchema: aNumber.isInteger(value) && value >= 1refine on both fields, ahead of the existing 1100 capDocumentsWithMemoriesQuerySchema:.int().min(1)on bothDefaults (
page: 1,limit: 10), the numeric-string branch, and the 1100 cap are unchanged. No new upper bound is added to the documents schema, since existing web flows legitimately request large pages.Testing
bun testcases inpackages/validation/api.test.ts: defaults preserved, numeric and numeric-string acceptance, the 1100 cap, and rejection of zero/negative/fractionalpage/limiton both schemas — 27 passtsc --noEmit -p packages/validation— cleanbiome checkon both files — clean