Skip to content

Reject non-numeric window before it reaches Postgres - #184

Merged
Miracle656 merged 1 commit into
Miracle656:mainfrom
001marvelqueen-blip:fix/issue-171
Sep 30, 2026
Merged

Miracle656 merged 1 commit into
Miracle656:mainfrom
001marvelqueen-blip:fix/issue-171

Conversation

@001marvelqueen-blip

Copy link
Copy Markdown

Overview

This PR rejects non-numeric window queries on TWAP and VWAP endpoints before they hit Postgres.

Related Issue

Closes #171

Changes

📈 API Schema Validation

  • [MODIFY] src/routes/price.ts

    • Adds Zod schema parsing to reject non-numeric properties for window, sampleInterval, method, and source early.
  • [MODIFY] README.md

    • Add /price/twap/:assetA/:assetB and /price/vwap/:assetA/:assetB to endpoints table.
  • [MODIFY] openapi.yaml

    • Add TWAP and VWAP paths to OpenAPI Spec.
  • [MODIFY] src/__tests__/price.test.ts

    • Coverage for input validation edge cases correctly returning 400.

Verification Results

npm run test
✅ All validations short-circuit appropriately and return expected HTTP 400
npx tsc --noEmit
✅ Passed
Acceptance Criteria Status
Non-numeric input is rejected with 400 on all three parameters ✅ Full Zod schema coercion enforced
method is validated against its allowed values ✅ Enums strictly pass only supported strings
Tests for edge cases implemented ✅ Rejects ?window=abc, ?window=0, ?window=99999, etc.
Routes added to README ✅ Done

@drips-wave

drips-wave Bot commented Sep 26, 2026

Copy link
Copy Markdown

@001marvelqueen-blip Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Miracle656
Miracle656 merged commit a5c7bd2 into Miracle656:main Sep 30, 2026
1 check passed
Miracle656 added a commit that referenced this pull request Sep 30, 2026
The pg driver puts the connection string into its error messages, so
`TWAP computation failed: ${err.message}` published the Postgres
credentials to any caller who could make the query fail. Log the message
and return a generic one instead, matching what /screener already does.

Also regenerates openapi.json from openapi.yaml, which #184 left stale —
scripts/generate-openapi.ts derives the JSON from the YAML.

Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB
@Miracle656

Copy link
Copy Markdown
Owner

Merged — thanks. The validation is exactly right: I checked that window is rejected, not coerced, and that nothing reaches Postgres.

?window=abc            -> 400 {"error":"Invalid input: expected number, received NaN"}
?window=5;DROP TABLE x -> 400 {"error":"Invalid input: expected number, received NaN"}
?window=1.5            -> 400 {"error":"Invalid input: expected int, received number"}
?source=bogus          -> 400 {"error":"Invalid option: expected one of \"SDEX\"|\"AMM\""}
pgPool.query called times: 0

Moving parseInt to z.coerce.number().int() is what closes it — the old code let NaN through both < 1 and > 1440. I also confirmed ?network=mainnet still works alongside the new schema (the z.object strips unknown keys rather than rejecting them), which would have been an easy thing to break.

Two small things I fixed on main in 1c7e200 rather than sending back to you, since neither was your doing:

  1. The 500 path was echoing the raw database error. While probing the validation I got this out of /price/twap:

    500 {"error":"TWAP computation failed: invalid input syntax for type integer: \"abc\" at postgres://u:secret@h/db"}
    

    The pg driver puts the connection string in its messages, so that publishes the Postgres credentials to anyone who can make the query fail. src/routes/price.ts:64,110 now log the message and return a bare TWAP computation failed / VWAP computation failed, matching what /screener already does. Added a regression test and checked it actually fails against the old line, not just passes against the new one.

  2. openapi.json was stale. It's generated from openapi.yaml by scripts/generate-openapi.ts (npm run openapi:gen), so editing only the YAML leaves the JSON behind. Regenerated. Worth running that script whenever you touch openapi.yaml.

Both are pre-existing on main — flagging them so they're on your radar, not as anything you needed to change.

Miracle656 added a commit that referenced this pull request Sep 30, 2026
Every function in src/aggregator/vwap.ts now takes a required network and
filters price_points and pool_snapshots on it, getAMMPrice filters both
legs of its pool lookup, /price/:assetA/:assetB passes req.network through
to the aggregator, and the aggregate refresh worker runs once per enabled
network instead of pinning itself to whichever network the instance
happens to be indexing.

Merged locally: src/__tests__/price.test.ts conflicted only because #184
and 1c7e200 appended test blocks to the same tail; resolved as a union.
The README paragraph was corrected on merge — #195, #202 and #205 landed
network scoping for /screener, /pools, /depth and /prices/history after
this branch was written, so the list of still-unscoped endpoints was
stale.

Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB
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.

Reject non-numeric window before it reaches Postgres

2 participants