Repository navigation
fix(config): reject protocol-relative rewrite/fallback destinations in schema - #225
Merged
Merged
Conversation
…n schema
The editor JSON schema validated routing.rewrites/fallbacks destinations
with ^/[^?]*$, which only requires a leading slash — so it accepted
protocol-relative destinations like "//evil.com/x". The runtime rejects
these (assertNotProtocolRelative in config.ts) because new URL("//host/x",
base) resolves to a cross-origin URL, silently turning an internal rewrite
into a cross-origin proxy. Editors using $schema therefore green-lit a
destination the build hard-rejects.
Forbid the second leading slash via ^/(?!/)[^?]*$ in both the rewrites and
fallbacks dest patterns, matching the already-tested runtime contract.
❌ Deploy Preview for staging-void-cloud failed.
|
hyfdev
marked this pull request as ready for review
June 18, 2026 14:36
Brooooooklyn
approved these changes
Jun 30, 2026
cpojer
pushed a commit
that referenced
this pull request
Sep 11, 2026
* fix(dispatch): serve static assets with percent-encoded filenames Static-asset serving-manifest keys are the decoded on-disk path (a file named `oxc masthead.riv` is keyed with a literal space), but dispatch looked them up with `url.pathname`, which stays percent-encoded (`%20`). Any filename containing a space / `#` / non-ASCII char returned 404 — e.g. the oxc.rs hero `.riv`. Confirmed against the live prod manifest: the key held a literal space and the bytes were present. Decode the pathname before the manifest lookup at both sites (`resolveStaticPath` and the hashed-asset fast path), trying the raw key first then the decoded key. Serve-side, so it fixes already-deployed sites with no redeploy. Decoding is segment-safe: it is rejected when it would change the `/`-segment count, so an encoded slash (`%2F`) can never alias `/assets%2Fx` onto `/assets/x` and slip past the `/__void/*` block, the hashed-prefix gate, or the header/redirect rules (all keyed on the raw pathname). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(dispatch): reject over-encoded aliases in the decoded asset lookup Addresses the P1 review comment. The prior segment-count guard only blocked encoded slashes (`%2F`); it still let over-encoded ordinary path text through — `/%61ssets/app.js` decoded to `/assets/app.js` and `/%5F%5Fvoid/secret.js` to `/__void/secret.js`, serving the manifest entry while the raw-`url.pathname` policy gates (the `/__void/*` reserved-route block, the hashed-asset prefix gate, and the header/redirect rules) never fired. Replace the segment-count check with a canonical round-trip: accept the decoded key only when re-encoding it via the WHATWG URL pathname setter reproduces the exact request path. That admits only the encodings a conformant URL must use (space, `#`, non-ASCII) and rejects every over-encoded spelling of ordinary text (`%2F`, `%61`, `%5F`, `%2e%2e`) a browser would never send. This subsumes the old encoded-slash guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(dispatch): accept lower-case escapes and re-encode decoded redirects Addresses two P2 review comments on the canonical decode guard: - Exact string equality was too strict: it rejected valid lower-case percent escapes (the URL encoder emits upper-case, so `%c3%a9` != `%C3%A9`). Compare after normalizing escape letter case, so a lower-case-encoded request for a non-ASCII filename still resolves. (A literal `%` in a filename remains unsupported — the round-trip can't reproduce the double-escaped request; it 404'd before this change too, so it is not a regression.) - A redirect Location produced from a decoded match was emitted decoded, e.g. `force-trailing-slash` on `/docs/a%23b` (backed by `/docs/a#b/index.html`) returned `Location: /docs/a#b/`, which a browser reads as fragment `b/`. Serialize the Location back to a safe URL pathname (`/docs/a%23b/`). Factor the WHATWG path re-encoder (`URL.pathname` setter) into a shared helper used by both the decode guard and the redirect fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Editors now flag a protocol-relative
routing.rewrites/routing.fallbacksdestination (e.g.//evil.com/x) instead of silently accepting a config the build hard-rejects — closing a cross-origin-proxy footgun at author time.The bug
The editor JSON schema validated rewrite/fallback destination values with
^/[^?]*$— "starts with/, no?". That permits a second leading slash, so"//evil.com/x"matched. The runtime rejects it (assertNotProtocolRelativeinconfig.ts, called at the Rewrite and Fallback sites) becausenew URL("//host/x", base)resolves tohttps://host/x, silently turning an internal rewrite into a cross-origin proxy. So an editor using$schemashowed no warning for a destination the build rejects.Expected-correct behavior
The schema should match the runtime: a rewrite/fallback destination must start with a single
/(internal path only), not//.The fix
Forbid the second leading slash in both dest patterns:
^/[^?]*$→^/(?!/)[^?]*$. Verified byte-for-byte equivalent to the runtime predicate across the destination space (including correctly rejecting externalhttps://dests, which rewrites/fallbacks don't allow).Scope note
This is the only place the
^/[^?]*$pattern appeared (rewrites + fallbacks share it).routing.redirectsis intentionally not changed: its destination legitimately allows externalhttps://URLs, so it uses a different (looser) schema shape and never carried this pattern; its//protection stays in the runtime (already tested).Tests
Added a parity test in
config.test.ts(same style as the existingsourceDir/redirect-status-enumparity tests): asserts the dest pattern accepts/en/docs,/,/index.htmland rejects//evil.com/x,//,///x,/x?a=1,en/docs— failing onorigin/mainand passing after. The runtime//rejection is itself already covered (config.test.ts). Local gate:config.test.ts159/159; full--project unitzero-collateral; schema.json valid JSON; typecheck + lint + fmt clean. No existingvoid.json/ doc example uses a now-rejected destination.Prepared with AI assistance.