fix(router-core): harden string encoding/decoding boundaries - #8141
fix(router-core): harden string encoding/decoding boundaries#8141Sheraff wants to merge 1 commit 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 b1fcee2
☁️ 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 10 benchmarks
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Memory | mem client mount-unmount (solid) |
481.8 KB | 540.9 KB | -10.93% |
| ❌ | Memory | mem server aborted-requests (react) |
865.1 KB | 948.9 KB | -8.83% |
| ❌ | Simulation | client-nested-params navigation loop (react) |
210.7 ms | 227.4 ms | -7.31% |
| ❌ | Memory | mem client interrupted-navigations (vue) |
352.7 KB | 377.4 KB | -6.54% |
| ❌ | Memory | mem server error-paths redirect (react) |
296.3 KB | 313 KB | -5.36% |
| ❌ | Memory | mem server request-churn (vue) |
822.9 KB | 866.4 KB | -5.03% |
| ❌ | Simulation | client-async-pipeline navigation loop (react) |
102.8 ms | 107.4 ms | -4.25% |
| ❌ | Memory | mem client navigation-churn (solid) |
589.5 KB | 613.6 KB | -3.92% |
| ❌ | Memory | mem server streaming-peak chunked (vue) |
11.2 MB | 11.6 MB | -3.55% |
| ❌ | Memory | mem client navigation-churn (vue) |
1.6 MB | 1.6 MB | -3.35% |
| ⚡ | Memory | mem server error-paths unmatched (react) |
1,830.3 KB | 416.5 KB | ×4.4 |
| ⚡ | Memory | mem server error-paths not-found (vue) |
2,071.7 KB | 485.5 KB | ×4.3 |
| ⚡ | Memory | mem server request-churn (react) |
712.7 KB | 656.6 KB | +8.55% |
| ⚡ | Memory | mem server server-fn-churn (react) |
399.9 KB | 370.9 KB | +7.82% |
| ⚡ | Memory | mem server error-paths not-found (react) |
435.6 KB | 407.4 KB | +6.91% |
| ⚡ | Memory | mem client unique-location-churn (vue) |
463.2 KB | 433.9 KB | +6.75% |
| ⚡ | Memory | mem server peak-large-page (solid) |
1.1 MB | 1.1 MB | +6.04% |
| ⚡ | Memory | mem server request-churn (solid) |
746.4 KB | 723.3 KB | +3.19% |
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 (b1fcee2) with main (cb281d7)
![nx-cloud[bot]](https://avatars.githubusercontent.com/in/80458?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nx Cloud is proposing a fix for your failed CI:
We corrected TypeScript type errors and ESLint import ordering issues introduced by the new test files added in this PR. The fixes address invalid fast-check API usage (maxKeys on fc.record), missing type imports (AsyncLocalStorage, proper ViolationInfo casting), untyped search-param return access, and import ordering/style violations across path.ts, router.ts, and frame-decoder.test.ts. These changes bring all six failing tasks (test:unit, test:types ×3, test:eslint ×2) back to green without altering any production logic.
Warning
- ❌ We could not verify this fix.
- 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 lS7L-msA5
Apply fix locally with your editor ↗ View interactive diff ↗
🎓 Learn more about Self-Healing CI on nx.dev
1 participant
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)