Skip to content

Commit b012691

Browse files
codeslakeclaude
andcommitted
refactor(guard): pass the body to isBase64Body; cover the quanta rule
Three cuts, no behaviour change, plus one coverage hole they exposed. isBase64Body took (block, endMarker) and re-derived the body by slicing between the first newline and the last END marker — arithmetic the caller had already done to build the block. It now takes the body itself, which the caller has in hand as text.slice(m.index + m[0].length, end). Verified equivalent under both LF and CRLF before applying: the BEGIN match excludes the \r, so the two slices normalize to the same bytes. The two `if (remoteControl)` lines merged into one block, and a comment restating the line below it dropped. The hole: mutating away the `length % 4` check left the suite GREEN. No fixture had an alphabet-valid body of the wrong length, so a clause my previous commit message claimed was covered was not. Two rows added — a one-character body and `A===` — and both base64 clauses now fail exactly one test when removed. 175 measured shapes across six sweeps, 0 false accepts, identical to before the cuts. Full suite 1504 pass / 2 fail (EMFILE, same at the merge base). net: -8 lines. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent e28abd0 commit b012691

3 files changed

Lines changed: 18 additions & 13 deletions

File tree

bin/ca-trust.mjs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,9 @@ import { X509Certificate } from "node:crypto";
1515
// A body containing `-` reads as damaged here even though node accepts it
1616
// (openssl stops at the dash), which is the conservative direction and the only
1717
// measured disagreement.
18-
function isBase64Body(block, endMarker) {
19-
const bodyStart = block.indexOf("\n") + 1;
20-
const bodyEnd = block.lastIndexOf(endMarker);
21-
if (bodyStart === 0 || bodyEnd < bodyStart) return false;
22-
const body = block.slice(bodyStart, bodyEnd).replace(/\s+/g, "");
23-
if (body.length === 0 || body.length % 4 !== 0) return false;
24-
return /^[A-Za-z0-9+/]+={0,2}$/.test(body);
18+
function isBase64Body(body) {
19+
const b = body.replace(/\s+/g, "");
20+
return b.length > 0 && b.length % 4 === 0 && /^[A-Za-z0-9+/]+={0,2}$/.test(b);
2521
}
2622

2723
// Is this merged CA bundle safe to hand claude as NODE_EXTRA_CA_CERTS?
@@ -123,7 +119,7 @@ export function bundleCarriesOurCA(text, ourCaPem) {
123119
try { der = new X509Certificate(block).raw; }
124120
catch { return { ok: false, reason: "undecodable CERTIFICATE block" }; }
125121
if (der.equals(ourDer)) carriesUs = true;
126-
} else if (!isBase64Body(block, endMarker)) {
122+
} else if (!isBase64Body(text.slice(m.index + m[0].length, end))) {
127123
return { ok: false, reason: `undecodable ${label} block` };
128124
}
129125
}

bin/claude-via-proxy.mjs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,13 @@ if (proxyUpstream) proxyEnv.CACHE_FIX_PROXY_UPSTREAM = proxyUpstream;
133133
// Forward-proxy mode: the spawned proxy must attach the CONNECT/MITM handler,
134134
// or the HTTPS_PROXY wiring below would tunnel to a proxy that only speaks
135135
// reverse-proxy and never terminates TLS for the upstream host.
136-
if (remoteControl) proxyEnv.CACHE_FIX_FORWARD_PROXY = "on";
137-
// Tell the proxy we will wire claude ourselves, so it does not print a recipe
138-
// that would undo that. Internal handshake between these two files, deliberately
139-
// not documented as an operator knob — see the banner in proxy/server.mjs.
140-
if (remoteControl) proxyEnv.CACHE_FIX_WIRED_BY_LAUNCHER = "1";
136+
if (remoteControl) {
137+
proxyEnv.CACHE_FIX_FORWARD_PROXY = "on";
138+
// ...and tell it we will wire claude ourselves, so it does not print a recipe
139+
// that would undo that. Internal handshake between these two files,
140+
// deliberately not an operator knob — see the banner in proxy/server.mjs.
141+
proxyEnv.CACHE_FIX_WIRED_BY_LAUNCHER = "1";
142+
}
141143

142144
const proxyProc = fork(SERVER_PATH, [], {
143145
stdio: ["ignore", "pipe", "pipe", "ipc"],

test/proxy-forward-ca.test.mjs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,13 @@ test("ca-trust: the bundle guard never accepts a bundle NODE_EXTRA_CA_CERTS cann
391391
// every non-CERTIFICATE block outright waved these through: measured,
392392
// guard=accept while the handshake failed UNABLE_TO_VERIFY_LEAF_SIGNATURE.
393393
["corrupt PUBLIC KEY ahead", `-----BEGIN PUBLIC KEY-----\n!!!not base64!!!\n-----END PUBLIC KEY-----\n${ours}`, false],
394+
// Alphabet-valid but not whole 4-char quanta. Every character is legal
395+
// base64, so an alphabet-only test called this decodable — measured, node
396+
// reported `bad base64 decode` and loaded zero extra CAs. Without this row
397+
// the length check can be deleted with the suite still green.
398+
["PUBLIC KEY body of one char", `-----BEGIN PUBLIC KEY-----\nA\n-----END PUBLIC KEY-----\n${ours}`, false],
399+
// Padding is positional, not merely present: `AAA=` loads, `A===` does not.
400+
["PUBLIC KEY body with misplaced padding", `-----BEGIN PUBLIC KEY-----\nA===\n-----END PUBLIC KEY-----\n${ours}`, false],
394401
["corrupt X509 CRL ahead", `-----BEGIN X509 CRL-----\n!!!not base64!!!\n-----END X509 CRL-----\n${ours}`, false],
395402
// ...but a WELL-FORMED one must still be accepted, or the fix above turns
396403
// into the over-strict guard this PR set out to remove.

0 commit comments

Comments
 (0)