fix(launcher): coexist with another MITM via ca-trust.d instead of clobbering NODE_EXTRA_CA_CERTS - #1
Closed
codeslake wants to merge 7 commits into
Closed
Conversation
… instead of clobbering NODE_EXTRA_CA_CERTS
NODE_EXTRA_CA_CERTS takes ONE file, so `claudeEnv.NODE_EXTRA_CA_CERTS = caPem`
was not "set our CA" but "untrust whatever else needed trusting". Two real
losses from it: any OTHER component that also MITMs api.anthropic.com and also
sets the var (last writer wins, the loser's CA silently dropped), and an ambient
bundle the environment had already set.
Measured 2026-07-30: cswap's account-pin proxy also MITMs api.anthropic.com and
also wrote this var. Last writer won, the other CA went untrusted, and Remote
Control inbound broke on the work Mac — CC's debug log showed the extra certs
appended from the pin bundle at T+0.0, then 13x "unable to verify the first
certificate" on the SSE transport.
The irony is local: ten lines below that assignment, NO_PROXY is deliberately
MERGED, with a comment saying a corporate env may already have set one. Same
hazard, the rule had just never reached the CA.
So, per a contract agreed with the two other components that touch this:
write <config>/ca-trust.d/ccf.pem our CA only, byte-compare skip, EVERY
launch (the proxy regenerates its CA
when caDir is wiped, and a stale pem
would advertise a key nothing signs
with)
read <config>/ca-trust.pem when non-empty -> NODE_EXTRA_CA_CERTS
else our own caPem byte for byte what this did before
We deliberately do NOT build the merged bundle. Merging has to include the
ambient/corporate roots, and finding those is environment-specific: on one
measured Linux host four corporate CAs live in /usr/local/share/ca-certificates
and are absent from the 126-cert system bundle a shell points at, while a Mac
keeps them in the keychain. That knowledge does not belong in this repo. It also
keeps the writer count at one — two launchers both rebuilding the bundle would
race the same output. We are write-own + read-merged.
Ordering: publish happens before the client is exec'd (same process, and the
caPem existence gate above already guarantees the proxy generated it), so a
bundle builder that reads the dir on a cold start sees us.
Failure paths are deliberate. A publish failure warns on stderr and continues —
publishing is how OTHERS trust us, and must not kill this session, which only
needs its own CA. An absent, zero-byte, or unreadable bundle all resolve to the
same answer (our own CA), so one catch covers the three.
No override for the read path: the builder writes that exact path and resolves
the config dir the same way, so a knob could only ever point the two sides at
different files. CACHE_FIX_CA_TRUST_DIR overrides the publish dir, mirroring the
existing CACHE_FIX_CA_DIR.
Tests (4, each red before green): publish lands before exec and is our CA
verbatim; a present bundle wins; we never create the merged file and never touch
a sibling component's pem; with no bundle the env is our own CA exactly as
before.
Co-Authored-By: Claude <noreply@anthropic.com>
…m atomically
Two hardening gaps in the ca-trust contract, both hit in practice by the
sibling component that shares the directory, both reproduced here before
being fixed.
Read path: require balanced BEGIN/END markers as well as containment.
Containment alone cannot see a tear. A bundle whose EARLIER entry lost its
END line still literally contains our CA further down, so the contains-ours
gate accepts it — and that is the fatal ordering, not the benign one.
Measured on node v24.11.1 / openssl 3.5.4 against a leaf signed by the CCF
CA, with a real TLS handshake rather than a file-content check:
torn ahead of ours -> UNABLE_TO_VERIFY_LEAF_SIGNATURE
ours alone -> authorized
torn after ours -> authorized (warns "bad end line")
The builder concatenates sort(ca-trust.d/*.pem) and "ccf.pem" sorts first,
so a torn OURS lands in exactly the position that voids the whole bundle,
taking every other component CA and corporate root with it. The two checks
are complementary: containment catches a STALE bundle, the marker counts
catch a TORN one, and neither sees the other's case.
Write path: publish ca-trust.d/ccf.pem via temp + rename instead of
writeFileSync. A plain write opens the target with O_TRUNC and leaves a
torn pem visible to any builder reading the directory during the write —
producing exactly the bundle the read path now has to reject. The temp
lives in the same directory so the rename cannot cross a filesystem
boundary, and carries the pid so two concurrent launches do not collide.
Tests. Each guard is pinned by one case, and each was mutation-tested in
both directions so none is dead weight: dropping the marker check fails
only the torn-bundle case, dropping containment fails only the stale case,
and reverting the atomic publish fails only the atomicity case.
Also adds two handshake tests. Every other check in this area — ours and
the sibling components' — inspects file CONTENT, which never proves Node
verifies a leaf with the file we hand it. These stand up a TLS server using
the ensureCA() leaf and connect trusting only the selected bundle, with the
no-extra-CA control required to FAIL so a green result cannot pass for an
unrelated reason.
Test isolation, unrelated to the above but surfaced by it: cleanEnv() built
the child env by merging overrides into a copy of process.env, so ambient
variables leaked into the wrapper tests. Two cases read the developer's real
~/.claude/ca-trust.pem instead of a fixture, and the lowercase-no_proxy
merge case read the shell's NO_PROXY. Now stripped from the base before
overrides apply. The wrapper suite goes 15 pass / 2 fail -> 18 / 0 on a host
that exports NO_PROXY and has a merged bundle deployed.
README: document that CACHE_FIX_DOWNLOAD_REWRITE=on disables `claude update`
outright. It reads like a performance knob and was previously mentioned only
in a CHANGELOG parenthetical as "download-acceleration"; that misreading cost
several days of a broken updater. Rewriting a URL requires MITM-ing
downloads.claude.ai, whose release client pins public roots only. It cannot
be narrowed to the binary download (MITM is decided per host at CONNECT and
the version check shares the host), and no client-side override reaches that
client, so no CA injection can make it work.
Co-Authored-By: Claude <noreply@anthropic.com>
…ted comment ponytail-review on 226452e: runOnce was defined twice with byte-identical bodies, and the torn-PEM measurement was spelled out in full at both the write and the read path. Hoist runWrapper() next to cleanEnv and cross-reference the measurement instead of repeating it. No behaviour change. wrapper + forward-ca 28/28. Co-Authored-By: Claude <noreply@anthropic.com>
…ly check The README explained that --remote-control sets NODE_EXTRA_CA_CERTS but not how that coexists with another MITM on the same host, which is the case the publish/read paths exist for. Adds the contract: publish our own pem, read a bundle we never build, fall back to our own CA when it is absent or unusable. Includes the boundary the builder asked be stated explicitly — a consumer can verify the bundle is intact and carries its own CA, but not that it is complete. Completeness needs previous state to compare against, so it is the builder's guarantee; a legitimately small bundle and a narrowed one look identical to a reader. Every claim checked against the code, not written from the design notes. Co-Authored-By: Claude <noreply@anthropic.com>
…olate tests
Code review found the substring guards accept bundles that break the session.
Reproduced each against a real handshake before changing anything:
bundle shape old guard | handshake
corrupt base64 ahead, markers intact | accept | FAIL
torn BEGIN TRUSTED CERTIFICATE ahead | accept | FAIL
whole bundle CRLF-normalized | reject | authorized
Counting BEGIN/END says nothing about whether a body decodes, and hard-coding
the CERTIFICATE label makes any other label a corporate bundle carries invisible
to the count -- so a torn one lands in the fatal leading position unseen. Now
every PEM block must construct an X509Certificate and one must equal ours by
DER, which also fixes the CRLF false reject and closes a vacuous case: an empty
ca.pem made `merged.includes("")` true, accepting a bundle that did not carry us
at all. Verified against the live 132-cert bundle on this host: still accepted.
A rejected bundle that EXISTS now warns on stderr. Silence stays for absent,
which is the normal state on a host with no builder. In a three-component
contract a broken builder has to be visible: the session still works, while
every other component's CA is silently gone.
Tests were corrupting the developer's machine. Six --remote-control cases set no
CLAUDE_CONFIG_DIR, so the launcher published a throwaway temp CA over the real
~/.claude/ca-trust.d/ccf.pem -- measured going 5dc414fc -> 3773c611 from one
test, leaving the host's merged bundle advertising a CA nothing signs with, the
exact failure this feature prevents. It also poisoned a previous mutation
result. cleanEnv() now allocates a config dir by default rather than per-test.
The atomicity test was tautological: it passed against `unlink; write`, which is
maximally non-atomic. Rebuilt around sampling the path by name across the whole
launch, and renamed to what it actually proves -- no TRUNCATED file is ever
visible. Mutation-tested: O_TRUNC now fails it, unlink+write still passes (its
window is under the 1 ms sampler floor), rename passes. Named honestly rather
than claiming atomicity it cannot demonstrate.
Also: uuid in the temp name (pid collides across PID namespaces sharing a
bind-mounted config dir), reap orphaned temps, CHANGELOG entry, and the missing
CACHE_FIX_CA_TRUST_DIR row in the README env table.
wrapper + forward-ca 30/30, host pem unchanged across the run.
Co-Authored-By: Claude <noreply@anthropic.com>
…ns its place ponytail-review on ecd599c. Two of three findings applied: - the measurement table appeared in the commit message, the PR body, and the read-path comment. Comment now states the conclusion and points at the test that asserts it, which is the copy that cannot go stale. - noted at the impl site that test/proxy-forward-ca.test.mjs mirrors the same decision (the launcher is a top-level script, so a test cannot import it). The third finding was WRONG and is recorded here so it is not retried. It read the fd/inode assertions in the publish test as duplicated by the 1 ms sampler. Removing them was mutation-tested and silently dropped O_TRUNC detection: the sampler cannot see the truncation window for a ~1.2 KB write at all (100 samples across a truncate+write observed only the complete file, never length 0). The descriptor is the precise observer, the sampler is the cheap wide net, and they catch different things. Both kept, with the measurement in the comment. wrapper + forward-ca 30/30, host pem unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
codeslake
marked this pull request as draft
July 30, 2026 21:17
Owner
Author
|
Superseded by the upstream PR: cnighswonger#283 This one was opened against cnighswonger#283 branches from current upstream The branch here ( |
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
1 participant
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.
Goal
NODE_EXTRA_CA_CERTSaccepts exactly one file. On a host where something else also MITMsapi.anthropic.com— a corporate agent, an account-pinning proxy — whoever assigns that variable last silently untrusts every other CA. Measured 2026-07-30: two such components on one machine took turns breaking each other's TLS, with no error attributable to either. Remote Control inbound stopped working on a work Mac and neither component logged anything.This implements the consumer + publisher half of a cross-component contract, negotiated with the two other components' owners and implemented independently in three languages:
<config>/ca-trust.d/<component>.pem— own filename, never a sibling's, rewritten every launch, content-compare to skip a no-op<config>/ca-trust.pem= ambient/corporate roots +sort(ca-trust.d/*.pem); components never write it<config>=CLAUDE_CONFIG_DIR || ~/.claude, resolved identically by all threeA host with no other MITM and no bundle builder sees no change at all — the launcher falls back to its own CA, which is byte-for-byte what it did before.
Non-Functional Requirements
Size/complexity budget. ~130 lines in
bin/claude-via-proxy.mjs(two blocks in one existing function, no new module), ~500 in tests, ~80 in docs. No new dependency: the guard usesnode:crypto'sX509Certificaterather than forkingopenssl.Threat model. This decides what the client trusts for the host its API keys travel to, so both failure directions matter and are treated asymmetrically:
So the guard is allowed to be conservative, never permissive. Verified:
ca.pemis cert-only, no private key is read or published, and nothing is written outsideca-trust.d/. The publish path never follows a symlink into a victim file —renamereplaces the link itself (verified).Maintainability. No new abstraction in the launcher. One test helper was hoisted because two tests need the same two-launch sequence; the older inline forks were left alone.
Performance/reliability. One extra file read and up to N
X509Certificateconstructions per launch. Measured against the live 132-cert bundle on the dev host: not perceptible next to spawning the proxy.Load-bearing? YES. TLS trust path, shared on-disk contract with two external components.
What the guards actually check
Both conditions are checked by parsing, not by matching substrings. Node's PEM reader aborts the whole extras load on one block it cannot decode, so a damaged entry does not merely lose itself — it can void every other component CA and corporate root in the file, ours included.
A first implementation used substring checks (balanced
BEGIN/ENDmarkers +includes(ourCA)). Code review found it accepts bundles that break the session. Reproduced with real TLS handshakes against a leaf signed by the CCF CA, node v24.11.1 / openssl 3.5.4:BEGIN TRUSTED CERTIFICATEaheadCounting
BEGIN/ENDsays nothing about whether a body decodes, and hard-coding theCERTIFICATElabel makes any other label a corporate bundle carries invisible to the count — so a torn one lands in the fatal leading position unseen. There was also a vacuous case: an emptyca.pemmakesmerged.includes("")true, accepting a bundle that does not carry us at all.Now: every PEM block must construct an
X509Certificate, and one must equal ours by DER (which also fixes the CRLF false reject). Verified against the live 132-cert bundle on the dev host — still accepted, so real-world behavior is unchanged.Position matters, and it is why this is not belt-and-braces:
The builder concatenates
sort(ca-trust.d/*.pem), so a tornccf.pemsorts early and lands in exactly the fatal position.Atomic publish
ca-trust.d/ccf.pemis published via temp +renamerather thanwriteFileSynconto the target. A plain write opens withO_TRUNCand leaves a torn pem visible to a builder reading the directory — producing precisely the bundle the read path now has to reject. Temp is in the same directory (a cross-filesystem rename is not atomic and wouldEXDEV) and carries pid + uuid: pid alone collides across PID namespaces sharing a bind-mounted config dir. Temps orphaned by a kill between write and rename are reaped on the next launch.Observability
A bundle that exists and is refused now warns on stderr. Silence is kept for absent, which is the normal state on a host with no builder. In a three-component contract a broken builder has to be visible: the session still works (we fall back), while every other component's CA is silently gone — the failure nobody would otherwise notice.
What a consumer cannot check
The guard establishes that the file parses and carries us. It cannot establish that the bundle is complete — a reader has no previous state to compare against, and a legitimately small bundle is indistinguishable from a narrowed one. Measured across three healthy hosts: 2, 132, and 168 certs. Any cert-count floor tuned to one breaks another. Completeness is therefore the builder's guarantee, stated as such in the README so nobody later adds a floor.
Nor do the guards prove usability: only a handshake does that, and the launcher performs none. They are pre-flight checks that keep a known-bad bundle away from the client.
Tests
test/proxy-wrapper.test.mjs(18) andtest/proxy-forward-ca.test.mjs(12), including:ca.pemdoes not make the check vacuousensureCA()leaf, with a no-extra-CA control required to FAIL so a green result cannot pass for an unrelated reasonEvery guard was mutation-tested, so none is decoration:
writeFileSync(dst)(O_TRUNC)One test is named for less than it might appear to prove. The publish test is called "never leaves a truncated
ccf.pemvisible" rather than "publishes atomically", because mutation testing showedunlink; write— maximally non-atomic — still passes it: that window is shorter than the sampler's 1 ms floor. Catching it would need an inotify watch forIN_DELETE-before-IN_CREATE, which is not worth a dependency for a shape nothing here writes. Named for what it demonstrates.Test isolation fix (please read even if skimming)
Six
--remote-controltests set noCLAUDE_CONFIG_DIR, so the launcher published a throwaway temp CA over the developer's real~/.claude/ca-trust.d/ccf.pem. Measured: one test run took the host's pem from5dc414fcto3773c611, leaving that machine's merged bundle advertising a CA nothing signs with — the exact failure this feature exists to prevent. It also silently poisoned an earlier mutation-test result.cleanEnv()now allocates a config dir by default rather than per-test opt-in, and each run is verified to leave the host pem untouched.Separately, ambient
NO_PROXYleaked into the child and made the lowercase-no_proxymerge test read the shell's value instead of the fixture — pre-existing, unrelated to ca-trust, fixed by the same change.Docs
ca-trust.dcontract, the fallback ladder, and the consumer/builder boundary.CACHE_FIX_DOWNLOAD_REWRITE=ondisablesclaude updateentirely. It reads like a performance knob and was previously mentioned only in a CHANGELOG parenthetical as "download-acceleration"; that misreading cost several days of a broken updater on a fleet. Rewriting a URL requires MITM-ingdownloads.claude.ai, whose release client pins public roots only. It cannot be narrowed to the binary download (MITM is decided per host atCONNECT, and the version check shares the host), and no client-side override reaches that client —HTTPS_PROXY,/etc/hosts,/etc/resolv.conf,NODE_EXTRA_CA_CERTSwere each disproved against a control.CACHE_FIX_CA_TRUST_DIRrow added to the env table; CHANGELOG entry added.Test results
Full suite on this branch: 1390 tests, 1389 pass, 1 fail.
The one failure is
proxy-server.test.mjs→ "POST /v1/messages routes to upstream", which needs real upstream reachability. Proven pre-existing rather than asserted: the entire change was stashed to a tree byte-identical to the base (git statusempty), and the same test fails the same way there.Worth knowing for anyone running the suite behind a proxy: that test hangs indefinitely rather than failing when a proxy env var is set, and
--test-timeoutdoes not cut it. Run withenv -u HTTPS_PROXY -u https_proxy -u HTTP_PROXY -u http_proxy -u ALL_PROXY -u all_proxyor it never terminates.Base
Stacked on
fix/forward-absolute-form, notmain— that branch is not merged yet, and a main-based diff would drag in unrelated work.Note on the commit list: the range shows a
46bf13emerge commit that pulls the base branch into this one. Only the five1de7b89..7a6a565commits are this PR's work; the diff against the base is the four files above.🤖 Generated with Claude Code