Skip to content

fix: validate query params in feed-routes instead of unsafe casts#568

Merged
realfishsam merged 1 commit into
mainfrom
fix/558-feed-routes-casts
May 24, 2026
Merged

fix: validate query params in feed-routes instead of unsafe casts#568
realfishsam merged 1 commit into
mainfrom
fix/558-feed-routes-casts

Conversation

@realfishsam

Copy link
Copy Markdown
Contributor

Fixes #558

@realfishsam

Copy link
Copy Markdown
Contributor Author

PR Review: VERIFIED

What This Does

Replaces unsafe as string casts on Express req.query values with proper typeof runtime checks in all feed-routes handlers. Previously, array query params (e.g., ?symbol=a&symbol=b) would silently pass as string[], potentially causing downstream bugs. Now they return 400 with a clear error message.

Blast Radius

  • Core server only (core/src/server/feed-routes.ts)
  • Affects feed endpoints: fetchTicker, fetchTickers, fetchOHLCV, fetchOrderBook, fetchOracleRound, fetchOracleHistory, fetchHistoricalPrices
  • No exchange/SDK/OpenAPI impact

Findings

  1. The validation pattern is correct: typeof x !== 'string' catches both missing (undefined) and array (string[]) cases. Required params reject all non-string values; optional params check !== undefined first.
  2. The fetchHistoricalPrices handler still has a residual as 'asc' | 'desc' | undefined cast on line ~163 of the diff. This is acceptable -- at that point order has already been validated as string | undefined by the guard above, so the cast is narrowing a verified string to the union type. Not unsafe.
  3. Line 17 still has req.params.feed as string -- this is fine since Express route params are always strings (unlike query params).
  4. Error messages are consistently updated to say "query parameter" which is more precise than the previous "parameter".

PMXT Pipeline Check

  • Field propagation: N/A
  • OpenAPI sync: N/A
  • Type safety: OK -- removes unsafe casts, adds runtime validation

Semver Impact

patch -- tightens input validation, no new functionality. Could technically break clients sending malformed array query params, but those were already producing undefined behavior.

Risk

Low. The Number() coercions on since, limit, fromTimestamp, untilTimestamp, maxSize remain unvalidated (e.g., Number("abc") produces NaN). This is pre-existing and out of scope for this fix but worth noting for a follow-up.

@realfishsam realfishsam merged commit f5f684d into main May 24, 2026
10 of 11 checks passed
@realfishsam realfishsam deleted the fix/558-feed-routes-casts branch May 24, 2026 17:04
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.

[non-null] server/feed-routes.ts: 8 unsafe query-param casts at HTTP boundary

1 participant