fix(router-core): harden string encoding/decoding boundaries - #8141
fix(router-core): harden string encoding/decoding boundaries#8141Sheraff wants to merge 2 commits into
Conversation
Centralize all URL-path encoding/decoding primitives in a single string-encoding module with trust-boundary documentation, branded string kinds, and ESLint enforcement banning raw encodeURIComponent/ decodeURI*/btoa/atob elsewhere. Fixes a real crash: malformed percent-encoding (e.g. /post/%E4%BD) threw URIError out of extractParams through findFlatMatch (route masks) and findSingleMatch (router.matchRoute), which had no guard. findMatch is now the single choke point converting URIError into a null match (404), byte-neutral vs the previous per-caller guard. Adds fast-check property-based and concrete security tests for every encoding surface identified in docs/string-handling.md: - matcher totality, path param round-trips, open-redirect defense - search-param parse safety, null prototypes, JSON coercion contract - SSR inline-script XSS resistance (adversarial scroll-restoration keys) and seroval factory interpolation canary - frame protocol round-trips and client frame-decoder DoS limits - server-fn payload handling (size cap, method check, context merge, malformed input) - virtual-module base64url ID round-trips and hostile payload fallbacks - early-hints Link-header injection resistance and prerender SSRF Documents known quirks (legacy '*' splat collision, search-param JSON coercion, early-hints href interpolation) instead of silently changing behavior.
|
View your CI Pipeline Execution ↗ for commit f7d2b85
☁️ Nx Cloud last updated this comment at |
🚀 Changeset Version Preview7 package(s) bumped directly, 16 bumped as dependents. 🟩 Patch bumps
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Bundle Size Benchmarks
The following scenarios have bundle-size changes compared with the baseline:
Current gzip tracks all emitted client JS chunks. Initial gzip tracks only the entry/import graph. Trend sparkline is historical current gzip ending with this PR measurement; lower is better. |
Merging this PR will regress 8 benchmarks
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem client mount-unmount (solid) |
481.8 KB | 624.8 KB | -22.89% |
| ❌ | Memory | mem server error-paths error (solid) |
978.9 KB | 1,146.6 KB | -14.62% |
| ❌ | Memory | mem server request-churn (solid) |
712.7 KB | 809.8 KB | -11.98% |
| ❌ | Memory | mem server server-fn-churn (vue) |
345.7 KB | 373.7 KB | -7.49% |
| ❌ | Simulation | client-nested-params navigation loop (react) |
211.1 ms | 227.3 ms | -7.14% |
| ❌ | Memory | mem client navigation-churn (solid) |
592.5 KB | 627.7 KB | -5.61% |
| ❌ | Memory | mem server aborted-requests (react) |
855.7 KB | 899 KB | -4.82% |
| ❌ | Memory | mem server error-paths redirect (vue) |
427.7 KB | 448.2 KB | -4.56% |
| ⚡ | Memory | mem server error-paths not-found (vue) |
2,304.3 KB | 486.4 KB | ×4.7 |
| ⚡ | Memory | mem server error-paths unmatched (react) |
544.1 KB | 456.8 KB | +19.1% |
| ⚡ | Memory | mem server aborted-requests (vue) |
1,110.1 KB | 1,002.5 KB | +10.74% |
| ⚡ | Memory | mem client unique-location-churn (vue) |
467.7 KB | 427.2 KB | +9.46% |
| ⚡ | Memory | mem server error-paths redirect (solid) |
382.2 KB | 349.7 KB | +9.29% |
| ⚡ | Memory | mem server error-paths not-found (react) |
423.7 KB | 390.3 KB | +8.56% |
| ⚡ | Memory | mem server error-paths redirect (react) |
318.9 KB | 304.6 KB | +4.69% |
| ⚡ | Memory | mem server peak-large-page (react) |
1.2 MB | 1.2 MB | +4.49% |
| ⚡ | Memory | mem server error-paths not-found (solid) |
557.9 KB | 536.3 KB | +4.03% |
| ⚡ | Simulation | client-async-pipeline navigation loop (react) |
107.1 ms | 103 ms | +3.92% |
| ⚡ | Memory | mem client interrupted-navigations (vue) |
368.7 KB | 357.5 KB | +3.15% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing feat/string-encoding-hardening (f7d2b85) with main (d83a896)
There was a problem hiding this comment.
Nx Cloud is proposing a fix for your failed CI:
We updated seven files introduced by this PR to resolve ESLint and TypeScript errors that prevented CI from passing. The fixes address import ordering, top-level type-only imports, a removed { maxKeys } option unsupported by fc.record(), AnySchema property access casts, a while (true) condition flagged by no-unnecessary-condition, a missing ViolationInfo required fields in makeViolation, a tuple-index error in prerender-ssrf.test.ts, and a missing AsyncLocalStorage import for TS 5.6 compatibility.
Tip
✅ We verified this fix by re-running @tanstack/start-client-core:test:eslint, @tanstack/router-core:test:eslint, @tanstack/start-server-core:test:types and 3 more.
Warning
The suggested diff is too large to display here, but you can view it on Nx Cloud ↗
Or Apply changes locally with:
npx nx-cloud apply-locally zkun-q8vg
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
Summary
Centralizes and hardens every string encoding/decoding surface in the router, guided by a full inventory (now maintained as
packages/router-core/docs/string-handling.md).Fix: URIError crash via route masks and
matchRouteMalformed percent-encoding in a URL segment (e.g.
/post/%E4%BD,/post/%zz) madeextractParamsthrowURIError. The main matcher (findRouteMatch) caught it, butfindFlatMatch(route masks) andfindSingleMatch(router.matchRoute) did not — such URLs crashed matching outright.findMatchis now the single choke point convertingURIErrorinto "no match" (404), which is byte-neutral vs. the previous per-caller guard and structurally prevents a future fourth entry point from re-introducing the bug. Behavior is otherwise unchanged: malformed sequences still 404; valid ones decode exactly as before.Centralization + enforcement
router-core/src/string-encoding.tsowns all URL-path primitives (decodePath,encodePathParam,compileDecodeCharMap,escapeHtml, ...) with trust-boundary docsencodeURIComponent/decodeURI*/btoa/atoboutside that module (documented exceptions for the matcher's no-match contract and ssr/serializer base64)EncodedPathParam,DecodedPathParam,EncodedPath,DecodedPath) wired through real signatures — e.g. custom decoders are type-contracted to receive encoded valuesTests added (fast-check property-based + concrete regressions)
__proto__safety<script>context and round-trip semantically; injected code provably never executes; seroval factory interpolation canaryDocumented quirks (deliberately not changed)
*splat syntax collision: splat value'*'interpolates to/files/*and resolves to the legacy route with empty params (reproduced; fix sketched in docs, needs e2e coverage)'?q=%2042'→ number)hrefis interpolated verbatim (manifest-controlled today)Test plan
tests/string-encoding.bench.ts)