[#145] Env sanitizer: strip ANTHROPIC_CUSTOM_HEADERS + detect/surface HTTP(S)_PROXY#154
Conversation
… proxy - Strip ANTHROPIC_CUSTOM_HEADERS with the credential set (can smuggle Authorization/x-api-key -> silent per-token billing, the #25 class), under the same intentional-custom-endpoint (ANTHROPIC_BASE_URL) exception. - Add detectProxy(): find an active HTTP(S)/ALL_PROXY (+ lowercase), return a SAFE host[:port] label only (never the full value, which may embed credentials). Proxies are detected, never stripped. - session.ts emits ONE content-free proxy notice per session on the CLI path (the transcript egresses only there; the on-device local tier never routes externally). Emitted once, not per engine. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Epic Alignment: PASS
#145 closes the #134 security/privacy env-sanitizer gap while preserving the sanitizeChildEnv child-process boundary and the ANTHROPIC_BASE_URL custom-endpoint exception from the ticket's EPIC Context.
Checked (evidence)
- Structural gate:
gh pr view 154atd10217385155cb4750086ab2d61f436b4781fa67→ filled## EPIC Alignment,## Self-Verification, and non-UI## Design FidelityN/A. - Context:
gh issue view 145→ contracts require stripping named auth-bearing Anthropic vars when no base URL, detecting proxy env without logging full values, and one content-free host-session notice;gh api repos/realproject7/livecap/issues/134checked security/privacy goal and #145 routing. - Sanitizer:
packages/engine/src/env.ts:8,:34-37stripANTHROPIC_CUSTOM_HEADERSwith existing credentials under the base-URL exception;packages/engine/test/env.test.ts:89-110covers default strip, case-insensitive strip, and base-URL preservation. - Proxy detection:
packages/engine/src/env.ts:45-65detects HTTPS/ALL/HTTP proxy vars plus lowercase;:69-75reduces to host[:port] or(set), not the raw value; tests atpackages/engine/test/env.test.ts:113-158cover precedence, lowercase, creds, schemeless, empty, and unparseable values. - Host notice:
src/host/session.ts:231-233makes start once-per-session;src/host/session.ts:315-325emits the content-free proxy notice in the CLI path outside per-engine wiring, so #142's two CLI engines do not duplicate it. - Riskiest part of this diff: the notice is not directly host-e2e tested; acceptable here because existing host tests document no headless start harness, and the pure detector plus guarded session-level call site make the once-per-session behavior directly inspectable.
- Kill-list: scanned all items — clean; no mock/stub runtime path, TODO/FIXME/HACK, console/debugger, new deps, caption logging, or scope creep beyond
env.ts, export, tests, andsession.ts. - CI:
gh pr checks 154→ app-macos, packages-linux, no-stub-gate, color-guard all pass.
Findings
None.
Decision
Approve. The implementation matches #145's amendment and #134's privacy contract: credentials that can redirect billing are stripped at the child-env boundary, proxy routing is surfaced without leaking values, and the two-lane CLI setup only emits one session-level notice.
|
RE2 — APPROVE (PR #154, Fixes #145 — env sanitizer: strip Reviewed vs main Checked (evidence)Layer 1 — EPIC #134 alignment (matches @Head's Batch 26 directive exactly):
Layer 2 — kill-list clean:
Correctness (unit-verified): precedence Ran live at Non-blocking nits (no change required):
No findings. RE2 APPROVE. (Attribution: formal GitHub APPROVE blocked — gh is the PR-author identity — so this verdict is a PR comment with the body-marker + |
Fixes #145
EPIC Alignment
Parent EPIC: #134 — LiveCap v1.3 latency, reliability & STT-core performance (Fable audit). This ticket's role: close the env-sanitizer security gap so Anthropic custom-header credentials can't leak and proxy routing is never silent — without breaking the "your audio only goes to the model you chose" promise. Depends on: none. Enables: #148 (archive hardening — not touched here). Out of scope (per ticket): #148 archive hardening, #146 release-build debug env gating — not implemented.
Ran the EPIC Alignment Check against #134 and the merged Cluster A siblings (135/137/139/142/136): this batch is disjoint. The one contract to respect —
sanitizeChildEnvstays the child-env boundary withANTHROPIC_BASE_URLas the explicit intentional-custom-endpoint exception — is preserved; #142's second CLI engine is honored (the sanitizer runs per spawn, and the proxy notice is emitted once per session, not per engine).Self-Verification
Adversarial diff re-read + kill-list scan (clean): no
console.*; the proxy notice is content-free and carries only a safehost[:port]label —proxyHost()strips any embeddeduser:pass@credentials and path, and an unparseable value degrades to(set)rather than echoing the raw string; no full proxy/secret value is ever logged or returned. No new runtime dependencies (URLis a Node global). No stub/TODO. Never logs caption content.Levers
ANTHROPIC_CUSTOM_HEADERS— added to the credential regex so it strips alongsideANTHROPIC_API_KEY/ANTHROPIC_AUTH_TOKEN(+ the existing Bedrock/Vertex set) when noANTHROPIC_BASE_URL, keeping the intentional-custom-endpoint exception. Custom headers can smuggleAuthorization/x-api-key→ silent per-token API billing (the [follow-up] sanitizeChildEnv misses CLAUDE_CODE_USE_BEDROCK/VERTEX → silent per-token billing #25 class).detectProxy()finds an activeHTTPS_PROXY/ALL_PROXY/HTTP_PROXY(+ lowercase), most-relevant-first (the HTTPS var that carries the transcript wins), and returns a safe host label only. Proxies are detected, never stripped (a user may legitimately need one), butsession.tsemits one content-free notice per session so the routing to Anthropic is a conscious choice.Acceptance criteria (1:1)
ANTHROPIC_CUSTOM_HEADERSabsent from child env — teststrips ANTHROPIC_CUSTOM_HEADERS by default.HTTPS_PROXYproduces a user-visible content-free notice; transcript never silently proxied —detectProxyreturns the host (tested) andsession.tsemits the single notice on the CLI (egress) path. Proxy is not stripped.keeps ANTHROPIC_CUSTOM_HEADERS when ANTHROPIC_BASE_URL is set+ all prior cases pass.detects the lowercase https_proxy variant.start()(guarded bythis.started), a session-level call site, outside the per-engine wiring; per the established host convention ([bug] Retranslate: failure erases archived translation + no dedup/priority (N6/N7) #139/Translation latency: dedicated CLI session (unblock live from summary) + idle-fast dispatch + defer first summary (audit H2/H1/N3) #142:start()spawns real children, so no headless e2e harness), the pure testable core isdetectProxy, and once-per-session holds by construction.Evidence @ latest commit
pnpm --filter @livecap/engine test→ 261 passed (env.test.ts 22, +12: 3 custom-headers + 9 detectProxy).pnpm test:app→ 134 passed.pnpm --filter @livecap/archive test→ 76 passed.pnpm typecheck(app+host) ✓,@livecap/engine typecheck✓,pnpm lint✓,pnpm no-stub✓,./scripts/color-guard.sh✓.Design Fidelity
N/A — not a UI PR (engine sanitizer + host status emit; no visual surface).
Deviations
ANTHROPIC_CUSTOM_HEADERS"unconditionally," but the later (2026-07-06) Review-pass amendment lists it among the vars stripped "when noANTHROPIC_BASE_URL" and directs "keep theANTHROPIC_BASE_URLexception." I implemented per the amendment (exception-gated): with a custom base URL the user's endpoint/headers are intentional (the existing credential model), and the ticket's own threat framing — "with no base-URL set the existing exception never engages" — confirms the danger is exactly the no-base-URL case, which is closed. So custom headers strip when no base URL, and are kept when one is set (tested both ways).if (cli)branch because only the CLI child egresses the transcript to Anthropic (through any inherited proxy); the on-device local tier never routes externally, so emitting there would be a false alarm. Detection logic itself (detectProxy) is engine-level and fully unit-tested regardless of path.detectProxyas a sibling detector rather than changingsanitizeChildEnv's signature (the ticket offered either) — smaller blast radius, and detection is orthogonal to stripping.