test(api): behavioural cover for the raw-route session gate - #246
Merged
Conversation
`requireSessionMiddleware` landed in #236 with only one test: that it carries the `CORE_GATE_MIDDLEWARE` tag. Nothing asserted what it does — neither the 401 that is its entire reason for existing, nor that it reads the session through #237's per-request memo. The second one is not hypothetical. #236 was written before the memo landed and rebased over it textually clean, so the gate resolved the session itself; only a resolution count says so, and there was none. Two defects the tests found, both fixed here: - The gate had lost the defensive read of `env.SESSION_SECRET` it shipped with. Some deployments expose `env` as a proxy whose getter throws for an unbound key, and an escaping TypeError turns a route whose documented answer is 401 into a 500. Removing the duplicate session resolution was right; removing this read with it was not. - The comment claimed resolving directly "would hand the handler an unfrozen copy". It would not — the gate used its session for the null check and discarded it. The real second cost is that the gate and the handler behind it answer from two independent resolutions, so a revocation landing between them decides the two differently. Corrected, and covered by a test asserting they share one frozen object. Verified red in both directions rather than only green here: against #236's original gate body the three memo assertions fail, and against current main the throwing-getter case fails — so that one is a live defect this closes, not a guard against a future one. Co-Authored-By: Claude Fable 5.1 <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.
Why
requireSessionMiddleware— the single supported gate for the rawext.routesescape hatch — merged in #236 with exactly one test: that itcarries the
CORE_GATE_MIDDLEWAREtag. Nothing asserted the 401 that is itsentire reason for existing, and nothing asserted that it resolves the session
through #237's per-request memo.
That second gap is why #236 shipped a semantic regression through a textually
clean rebase. The gate was written before the memo landed, so it resolved the
session itself; the only thing that says so is a resolution count, and there
was no test counting.
What the tests found
Writing them turned up two real defects, both fixed here:
1. A live 401→500 on current main. The gate had lost the defensive read of
env.SESSION_SECRETit originally shipped with. Some deployments exposeenvas a proxy whose getter throws for an unbound key; the escaping
TypeErrorturns a route whose documented answer is 401 into a 500. Removing the
duplicate session resolution was right — removing this read along with it
was not. Restored.
2. A comment that claimed something untrue. It said resolving directly
"would hand the handler an unfrozen copy". It would not: the gate used its
session for the null check and discarded it. The actual second cost is that
the gate and the handler behind it would answer from two independent
resolutions, so a revocation landing between them decides the two
differently. Corrected, and now covered by a test asserting they share one
frozen object.
The tests
middleware.test.ts— five behavioural cases: 401 with the handler neverinvoked; handler runs and its response passes through for a session; a GET is
gated too (unlike csrf's safe-method skip); fails closed on a missing
SESSION_SECRET; 401 rather than 500 on a throwing getter.request-identity.test.ts— the gate joins the T2 chain guard, plus threememo cases: it reuses a resolution the request already paid for, it and the
handler share one frozen session, and the no-context fallback still refuses
an anonymous request.
Verification
Red in both directions, not just green:
maintsc --build(the repo'slint) clean. No public API surface change, so nosnapshot regeneration.
🤖 Generated with Claude Code