feat(capture): pre-pipeline request capture + source/gate provenance on /health

[Gunther-Schulz]

Stacked on #274 (header forwarding fix — this branch includes its commit). Review the tip commit.

Why

This is the attribution primitive behind #272/#273: a JSONL capture of what Claude Code actually sent, recorded before any mutating extension runs (order 60). A divergence present in the raw capture is CC's; one absent is ours. Without this line there is no way to distinguish a mitigated upstream bug from a self-inflicted one — we spent a full day learning that the expensive way (five of six suspected CC bugs turned out to be our own, found only by replaying raw captures).

It's also what makes independent verification possible for anyone running this proxy: with captures on disk, the replay/census tooling (next PR) can re-run the whole pipeline offline against real traffic and prove — not assume — that the mitigations forward what production forwarded.

What

Three record types share one capture file per session:

• request — pre-pipeline body + the headers that matter, joined by id;
• outcome — what the API charged (usage split by cache tier, output tokens, wall time) plus outSha, the hash of the bytes the proxy actually forwarded, computed at the single point those bytes exist (post-pipeline, in preForward). This is what lets an offline replay validate itself against production byte-for-byte;
• boot — restart boundaries and the gate set in force, so cross-restart analysis doesn't misattribute.

/health grows two provenance fields on the same principle (answer content questions, not label questions):

• proxy_tree — content fingerprint of the source this process loaded (mtimes false-fire on byte-identical restores; disk diverges from the process the moment someone edits without restarting);
• gates — the CACHE_FIX_* environment this process is actually serving with. The unit file answers "declared", not "serving" — that difference cost us a day of green gate runs that were verifying a pipeline nobody ran.

Size-capped (CACHE_FIX_CAPTURE_MAX_MB, oldest-file rotation). Off by default: CACHE_FIX_REQUEST_CAPTURE=1. Captures record header names relevant to caching only — no authorization material is written.

Evidence

15 unit tests (record shapes, join ids, rotation, fingerprint stability across byte-identical restores). In production on our machines for two days at ~2.5 GB/day of captures; the outcome-record outSha is what proved the replay faithful (109/109 reconstructions matched the wire on a fresh session) and what proved fable-5's tool_addition support in #273.

🤖 Generated with Claude Code

[vsits-codex-review-agent]

Review: PR #275 feat(capture): pre-pipeline request capture + source/gate provenance on /health

Date: 2026-07-31
Reviewed: merged worktree at origin/main 81f47a5 + PR head 7f9734ba
Round: 1
Label applied: changes-requested, schema-change

What Is Correct

• request-capture is actually placed pre-mutation. proxy/extensions.json:2-5 puts it at order 60, after bootstrap-defense (45), before cc-version-normalize (90), and ahead of the rest of the mutating body pipeline.
• outSha is still computed at the single post-pipeline/pre-forward byte boundary after #274's preForward() reshaping. proxy/server.mjs:117-137 hashes the serialized outbound body after runOnRequest() mutations and before forwardRequest() at proxy/server.mjs:166-175.
• The merged tree against current origin/main is smaller than the PR headline because the header-forwarding work from #274 is already on main. The actual merged delta I reviewed is 483 production LOC and 301 test LOC (0.62x), not the stale stacked diff.
• The full merged suite passed in the worktree: 1,455 tests, 0 failures.

Blockers

1. proxy/extensions/request-capture.mjs:85-101 and proxy/extensions/request-capture.mjs:249-280 persist the full request body verbatim, and the tests explicitly assert that behavior at test/request-capture.test.mjs:49-79. That is the feature, but the implementation leaves a security-sensitive plaintext corpus under ~/.claude/cache-fix-captures/ with no explicit owner-only permissions: both mkdir(..., { recursive: true }) calls and every appendFile(...) call rely on ambient umask. On a normal 0022 shell that means a 0755 directory and 0644 files for prompts, pasted secrets, file contents, and tool outputs. The same code also enforces CACHE_FIX_CAPTURE_MAX_MB only after append and only every 50 writes (proxy/extensions/request-capture.mjs:278-280), so the cap is post-write rather than pre-write. For a new full-body capture surface, explicit 0700/0600 creation and a bounded-write story are required.
2. /health and the boot records both dump the entire CACHE_FIX_* environment without an allowlist. proxy/server.mjs:378-402 returns gates: _gates, and proxy/server.mjs:600-604 populates _gates from every CACHE_FIX_* key in process.env. proxy/extensions/request-capture.mjs:224-235 repeats the same unfiltered dump into each capture file's boot record. Existing config keys include path/topology-bearing values such as CACHE_FIX_PROXY_CA_FILE, CACHE_FIX_CA_DIR, CACHE_FIX_EXTENSIONS_DIR, CACHE_FIX_EXTENSIONS_CONFIG, and CACHE_FIX_PROXY_UPSTREAM (proxy/config.mjs:29-37, proxy/config.mjs:55-59). /health is unauthenticated on the proxy port, and the current tests intentionally only assert status, forward_proxy, and https_proxy rather than any redaction contract (test/proxy-server.test.mjs:45-54, test/proxy-integration.test.mjs:102-118). This needs an allowlisted/public subset, not a raw env snapshot.

What Needs Attention

• The PR is over the repo's anti-bloat threshold for community work and still has no ## Non-Functional Requirements / Load-bearing? section. Independently: this is load-bearing. It adds a new plaintext request-capture corpus on disk, a new /health response shape, new env-vars, and new replay provenance fields; those are security-relevant and schema-relevant surfaces.
• Anti-bloat numbers on the actual merged delta: 483 production LOC = 258 code + 199 comment + 26 blank; 301 test LOC (0.62x); 2 new production files; 3 new env-vars/surfaces (CACHE_FIX_REQUEST_CAPTURE, CACHE_FIX_CAPTURE_MAX_MB, CACHE_FIX_PROXY_TREE); 1 new on-disk path family (~/.claude/cache-fix-captures/*.jsonl); comment:code ratio 0.77x. That is larger than the recent merged calibrators, but I am not calling size alone a blocker here because the current blocking issues are security-contract issues, not a proven behavior-preserving simplification opportunity.
• proxy/source-fingerprint.mjs is only called in-repo from proxy/server.mjs today, but its file header gives a concrete second consumer (doctor) via CLI shell-out (proxy/source-fingerprint.mjs:28-30, proxy/source-fingerprint.mjs:70-79). I therefore did not count it as dead abstraction under the anti-bloat lens.

Bloat / Non-Functional

• No separate safe-to-simplify blocker beyond the two security findings above.
• Coverage is strong on the happy-path feature and the merged suite is green, but there is still no test for owner-only capture-file permissions, no test that CACHE_FIX_CAPTURE_MAX_MB is enforced before write, and no test that /health or boot records redact/allowlist gate values.

Recommendations

• Make the capture directory and files explicitly owner-only (0700 dir, 0600 files) and add a regression test that fails under a permissive umask.
• Decide which capture fields are actually required for replay, then document and enforce that security contract explicitly. If full-body capture remains the design, the permission model and retention story need to be correspondingly strict.
• Replace the raw CACHE_FIX_* snapshot with an allowlisted gates view on /health and in boot records. If an operator needs full env diagnostics, that belongs behind a stronger access path than the public health endpoint and plaintext capture files.
• Add the missing NFR/load-bearing section to the PR description or companion directive text so the security posture, disk-retention contract, and schema surface are reviewable as first-class requirements.

Bottom Line

The feature shape is understandable, the merged test run is clean, and the pre-pipeline placement / outSha wiring are correct. I am requesting changes because the new security surface is not constrained tightly enough yet: full request bodies are written to disk without explicit owner-only permissions, and both /health and the boot records expose the entire CACHE_FIX_* environment instead of a reviewed public subset. — Codex review

[vsits-proxy-builder]

Review result: changes requested. Both blockers are security-surface rather than correctness — the feature shape and the wiring are right.

Note the two of us reviewed this merged onto current main, which matters: #274 landed today, so the real delta here is 483 production LOC (258 code + 199 comment), 301 test, not the stacked headline. Thanks for splitting #274 out — that made this reviewable on its own.

1. Full request bodies are written at ambient umask (request-capture.mjs:267-274)

Neither mkdir nor appendFile passes a mode. On a normal 0022 host that's a 0755 directory and 0644 files containing prompts, pasted credentials, file contents, and tool output. Verified against the diff — there's no mode: or post-write chmod anywhere in the module.

0700 on the directory and 0600 on the files, plus a regression test that fails under a permissive umask. The test matters more than the fix here: a permissions bug reintroduced silently is worse than one that was never fixed.

Related: CACHE_FIX_CAPTURE_MAX_MB is enforced after append and only every 50 writes (:278-280), so the cap can be overshot by up to 50 records. Worth making pre-write given what's in them.

2. /health returns the entire CACHE_FIX_* environment (server.mjs:579)

It's a bare startsWith("CACHE_FIX_") filter with no allowlist, and /health is unauthenticated on the proxy port. That currently includes path-bearing values — CACHE_FIX_CA_DIR, CACHE_FIX_EXTENSIONS_DIR, CACHE_FIX_PROXY_UPSTREAM — and will include whatever anyone adds later, which is the part that worries me: an allowlist fails safe as the config surface grows, a prefix filter fails open. The same unfiltered dump goes into each capture file's boot record.

An explicit allowlist of gate names, with values reduced to on/off where the value isn't itself the point.

On the design: the motivation is genuinely good, and "a divergence present in the raw capture is CC's, one absent is ours" is the right instinct — that distinction is exactly what we lacked when diagnosing the double-proxy 404 earlier this week. Order-60 placement checks out against extensions.json, and outSha is still at the correct single-point-of-truth after #274 reshaped preForward. source-fingerprint.mjs isn't dead abstraction either — the header names a concrete second consumer.

Two process notes:

• This is load-bearing — new plaintext corpus on disk, new /health response shape, new env vars. That means human review from Chris before merge, not just ours. A ## Non-Functional Requirements section in the PR body would help; for a PR this size we now ask for one (see CONTRIBUTING.md, added today).
• Fork PRs don't get CI here — GitHub gates workflow approval and our bot can't clear it. We run the suite locally at the merge commit instead. Yours: 1455/0 merged onto main.

Happy to re-review once the permissions and allowlist are in.

— Proxy Builder