test(web): pin the HTTP invariants nothing was watching - #3132
Merged
Conversation
These all pass today. That is the point: an audit of the runtime against the HTTP surface turned up a lot of behaviour that is deliberate, correct and completely unguarded, so the next change to any of it would be silent. - The origin gate's reading of the headers: `Origin: null`, an explicit `:443`, byte-for-byte comparison, punycode, a Referer that parses but names no origin, an unrecognised `Sec-Fetch-Site` falling through to Origin, and duplicated `Sec-Fetch-Site`/`Origin` (comma-joined in transit) still refusing. - Methods: exact matching, `Allow` on OPTIONS and an unknown verb, and that no method-override header or `?_method=` is honoured. - Addressing: an encoded path or control character does not fold onto the plain id, the `data` segment and the mount are read literally, and an oversized id is refused before the body is read. - CORS: nothing is emitted on any path and a browser preflight fails closed — which is the whole cross-origin story, and was untested. - Header hygiene: CR/LF stripped from the error label, `no-store` on the refusals built before dispatch, and the `Vary` merge preserving an author's value, deduping case-insensitively and short-circuiting on `*`. - HEAD over a streamed result: the GET's headers, no body, and the source cancelled rather than pumped. - Conditional requests and Range on a declared read: answered in full, never a bogus 304 or 206. Each test was checked by mutating the built runtime — 24 mutations, every new test killed by at least one. Two cases resisted: an explicit `:443` in the request url and the matching half of the punycode pair are both normalized by the URL parser before the runtime sees them, so no mutation of the gate can turn them red. The first was dropped and refolded into the Origin-header direction, where it is a real decision; the second is kept as half of a pair whose other half does discriminate.
|
Merging this PR will degrade performance by 28.51%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
…ant one Review found a second assertion that could not fail. `refuses Origin: null` passed on a gate that had stopped reading Origin at all, because the no-proof branch refuses by default — so the test never depended on the literal value it names. It now runs with `allowRequestsWithoutOriginCheck: true`, where a request carrying nothing is accepted, so only the `null` itself can produce the refusal. Verified: mutating the gate to treat `Origin: "null"` as absent now fails it. Also dropped `reads only the origin out of a Referer, path and all` — the pre-existing `falls back to Referer when Origin is absent` already uses a path-carrying Referer, and no mutation kills the new one alone. And renamed the oversized-id test to what it actually pins: there is no id size check in the runtime, only an unknown id answered before the body is buffered.
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
2 participants
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.
An audit of the server-function runtime against the HTTP surface — methods, addressing, headers, bodies, caching, CORS, cookies — turned up four defects (#3128, #3129, #3130, #3131) and something larger: a great deal of behaviour that is deliberate, correct, and guarded by nothing at all.
This PR pins that behaviour. Every test here passes on
nexttoday; none of it would have been noticed breaking. Tests only, no runtime change, no changeset.What is now pinned
The origin gate's reading of the headers —
Origin: null(sandboxed iframe,data:), an explicit:443, byte-for-byte comparison rather than normalised, punycode, aRefererthat parses but names no origin, an unrecognisedSec-Fetch-Sitevalue falling through to theOrigincheck, and duplicatedSec-Fetch-Site/Origin— comma-joined in transit — still refusing. #3111 pinned the gate's POST branches; these are the header-reading cases it did not reach.Methods — exact matching (
post,Post,POSTare notPOST),Allowcorrect onOPTIONSand on an unknown verb, and that no method-override header or?_method=is honoured, so a front proxy that translates one cannot smuggle a mutation past the gate.Addressing — an encoded path segment or control character does not fold onto the plain id, the
datasegment and the mount are read literally, and an oversized id is refused before the body is read.CORS — nothing is emitted on any path and a browser-shaped preflight fails closed. That is the entire cross-origin story of this runtime, and it rested on no test.
Header hygiene — CR/LF stripped from the error label so it cannot become a header of its own;
no-storeon the refusals built before dispatch; and theVarymerge preserving an author's value, deduping case-insensitively, and short-circuiting onVary: *.HEAD over a streamed result — the GET's headers, no body, and the source cancelled rather than pumped to completion.
Conditional requests and
Rangeon a declared read — answered in full, never a bogus304or206. Spec-legal, deliberate, and the kind of thing a well-meaning change would "fix".How each test earned its place
By mutating the built runtime and confirming the test fails — 24 mutations, every new test killed by at least one. A sample:
matchesOrigincompares both sides lowercasedSec-Fetch-SiteSec-Fetch-SiteorOriginX-HTTP-Method-OverrideAccess-Control-Allow-Origin: *Access-Control-*on any path; preflight fails closeddata/segment case-insensitivelydatasegment and the mount literallyno-storeonly on 2xxno-storeon the refusals built before dispatchTwo cases that resisted, stated rather than hidden
An explicit
:443in the request url, and the matching half of the punycode pair, are both normalised by the URL parser before the runtime ever sees them — so no mutation of the gate can turn either red. The first was dropped and refolded into theOrigin-header direction, where it is a real decision (Origin: https://app.example:443→ 403). The second is kept as half of a pair whose other half does discriminate, because the pair is the statement; trim it if you would rather have only killable assertions.Notes
No new files — every invariant had a home in an existing spec. Suite: 45 files, 474 passed, 1 expected fail, 2 skipped (up from 452 passed).
tscgains one error,bodySizeLimitmissing fromHandleServerFunctionOptionsin the generatedtypes/— the same stale-types problem that already produces five identical errors inserver-functions-request-bounds.spec.tsxonnext; the option is real in the source. I followed the existing convention rather than working around it.Follow-up from review
A later pass found a third assertion that could not fail —
refuses Origin: nullpassed on a gate that had stopped readingOriginentirely, since the no-proof branch refuses by default. It now runs with the no-proof escape hatch open, so only the literal value can produce the refusal, and a mutation treatingOrigin: "null"as absent fails it. A redundantReferercase was dropped (the pre-existing fallback test already carries a path), and the oversized-id test was renamed to what it pins: the runtime has no id size check, it answers an unknown id before buffering the body.