fix(security): close S-01 and S-02, found by falsifying the fixes (Session 7)

[ojassug]

Session 7 (§12 of docs/security-review-2026-08-30.md) was the first falsification pass over the remediation run by an agent that did not write it. It confirmed eleven of the fourteen fixes and found four defects. This closes the two that were demonstrably broken rather than merely incomplete.

They are the same mistake twice: each earlier fix was written against the route where its finding reproduced, and each left a sibling route untouched. Neither is subtle logic — the logic is correct everywhere it runs.

S-01 — the gateway creates a session only on a route that uses one

The credential check hoisted in the previous round is gated on isApiRoute; getOrCreateSession ran above the route dispatch and was not. POST /v1/anything and GET /nope answered 404 and minted a session on the way there, with no credential of any kind.

20 unauth POST /v1/anything (404)   -> sessionCount = 20
120 unauth GET /nope + x-session-id -> sessionCount = 100   (the whole maxSessions cap)

Reachable from a web page, which is the attacker the origin gate exists for. A no-cors GET carries no Origin header at all — the Fetch spec appends one only for CORS-tainted requests or non-GET/HEAD methods — so V-02's refusal never sees it, and the Host check passes because the browser really is talking to 127.0.0.1.

Driven from a page on another port against a live gateway, before:

{ "sessions": 100, "requests": 400, "distinctSockets": 400, "requestsCarryingOrigin": 0 }
victimSessionAfter: { exists: false, content: null }

After — same page, same 400 requests: sessionCount 1, and it is the victim's, content intact.

The session is now opened inside the two API branches and the 404 reports none. ProxyRequestResult.session has been optional since the hoist and nothing reads it. Unchanged: a local process still passes hasAuthHeaders with any string and may still name any session id — that is the exec trust boundary (audit C3), not something this closes.

S-02 — the CLI's fallback renderer escapes its label too

F-06 escaped \r and \n in core/render's itemLabel. renderFallbackBytes in cli/main.ts emits the same ==> … <== header over each file's original bytes on the fail-open path, and built it by interpolating file.path raw. Its own comment said "under the header the renderer emits", which stopped being true when the renderer started escaping.

Measured on ext4, one directory, two runs of the shipped binary:

success path : 3 headers, forged payload line absent
fallback path: 4 headers, the extra one `==> security_policy.py <==`
               followed by ALLOW_INSECURE_TLS = True

The attacker controls the trigger as well as the payload — one file of their own containing invalid UTF-8 forces the fallback through inputNotRepresentable.

The escaping is now an exported escapeDelimiterLabel in core/render, next to the delimiters it protects, called by both renderers. Only the header is escaped; file.bytes passes through untouched, because emitting the caller's original bytes is why that path exists (DECISIONS §35). Re-run on ext4 after: one header per file, all absolute, the crafted name escaped onto a single line.

The README needed no edit, which is the point. Its claim that "a crafted name cannot introduce a header line" was false on the fallback route when §12 was written. It is true again, on both.

Written test-first

Both defects were reproduced as failing unit cases before either fix existed:

expected 20 to be +0
expected 100 to be +0
expected [ Array(4) ] to have a length of 3 but got 4
expected [ Array(9) ] to not include '==> security_policy.py <=='

Nine cases now stand in test/unit/security-review-findings.test.ts, two of which fail if a fix over-reaches: a credentialed request on an API route must still get a session, and the fallback stream must still carry each file's original bytes.

Verification

• 99 test files / 927 passed, 2 skipped (the F-04 mode assertions, POSIX-only).
• Typecheck, lint and build clean.
• R-13 re-run in a real browser and R-14 re-run on ext4 under WSL2, both against the rebuilt artifact.
• No corpus run, deliberately. S-01 is off the optimize route, and S-02 moves stdout only for a filename containing a newline — no corpus file has one, so byte-identical would have measured nothing. OX-L7's shape.

Still open

S-03 (V8's JSON.parse message quoting ~15 bytes of payload into trace.fallbackReason, the field F-05 cleaned) and S-04 (the §6.3 upstream guard validating a base URL that fetch is then free to redirect away from, carrying x-api-key) remain open and are recorded as such. Neither is a fix that fails to do what it claims, which is what separated them from these two.

Docs: DECISIONS §73, CHANGELOG [Unreleased], report §12.8.

🤖 Generated with Claude Code