Skip to content

Commit 3d08e35

Browse files
committed
fix(request): block private account headers by prefix and stop fabricating a pinned index
Two review findings on the extracted modules, both pre-existing behavior moved verbatim by the phase-1 carve, fixed here to keep that PR zero-behavior-change: - responseHeadersForClient filtered account metadata by an exact-name allowlist, so a future x-codex-multi-auth-account-* header would leak to clients by default. The filter now blocks the whole prefix. - buildPinnedUnavailableErrorBody rendered a null pinned index (the desync path) as "Pinned account 1" while the machine-readable pinnedAccountIndex stayed null. The message now says "The pinned account" when the index is unknown. Tests updated/added for both, including the pre-existing null-index pin in test/issue-474-pin-honored.test.ts. https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB
1 parent 645042d commit 3d08e35

5 files changed

Lines changed: 35 additions & 12 deletions

File tree

lib/request/rate-limit-decision.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,14 @@ export function buildPinnedUnavailableErrorBody(
195195
? accountSkipReasons.get(normalizedPinnedIndex) ?? null
196196
: null;
197197
const reasonSuffix = skipReason ? ` (${skipReason})` : "";
198-
const displayIndex = (normalizedPinnedIndex ?? 0) + 1;
198+
// On the desync path the pin index is unknown (null); claiming "account 1"
199+
// there would contradict the machine-readable pinnedAccountIndex: null.
200+
const accountPhrase =
201+
normalizedPinnedIndex === null
202+
? "The pinned account"
203+
: `Pinned account ${normalizedPinnedIndex + 1}`;
199204
return {
200-
message: `Pinned account ${displayIndex} is currently unavailable${reasonSuffix}; run \`codex-multi-auth status\` for details, or \`codex-multi-auth unpin\` to allow rotation.`,
205+
message: `${accountPhrase} is currently unavailable${reasonSuffix}; run \`codex-multi-auth status\` for details, or \`codex-multi-auth unpin\` to allow rotation.`,
201206
code: "codex_pinned_account_unavailable",
202207
pinnedAccountIndex: normalizedPinnedIndex,
203208
reason: skipReason,

lib/request/stream-failover-runtime.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,11 @@ export const HOP_BY_HOP_HEADERS = new Set([
1313
"transfer-encoding",
1414
"upgrade",
1515
]);
16-
const PRIVATE_CLIENT_RESPONSE_HEADERS = new Set([
17-
"x-codex-multi-auth-account-index",
18-
"x-codex-multi-auth-account-label",
19-
"x-codex-multi-auth-account-email",
20-
"x-codex-multi-auth-account-id",
21-
]);
16+
// Any header under this prefix carries account-identifying rotation metadata
17+
// (index/label/email/id today) and must never reach the client; matching by
18+
// prefix means a future header added under it is blocked by default instead
19+
// of leaking until someone remembers to extend an allowlist.
20+
const PRIVATE_CLIENT_RESPONSE_HEADER_PREFIX = "x-codex-multi-auth-account-";
2221
const DECODED_UPSTREAM_RESPONSE_HEADERS = new Set([
2322
// Node fetch returns decoded bytes while preserving the upstream encoding header.
2423
"content-encoding",
@@ -29,7 +28,7 @@ export function responseHeadersForClient(upstreamHeaders: Headers): Record<strin
2928
for (const [key, value] of upstreamHeaders.entries()) {
3029
const normalizedKey = key.toLowerCase();
3130
if (HOP_BY_HOP_HEADERS.has(normalizedKey)) continue;
32-
if (PRIVATE_CLIENT_RESPONSE_HEADERS.has(normalizedKey)) continue;
31+
if (normalizedKey.startsWith(PRIVATE_CLIENT_RESPONSE_HEADER_PREFIX)) continue;
3332
if (DECODED_UPSTREAM_RESPONSE_HEADERS.has(normalizedKey)) continue;
3433
headers[key] = value;
3534
}

test/issue-474-pin-honored.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,10 @@ describe("issue #474 — manual pin honored by runtime proxy", () => {
719719
);
720720
expect(body.pinnedAccountIndex).toBeNull();
721721
expect(body.reason).toBeNull();
722-
expect(body.message).toContain("Pinned account 1");
722+
// The desync path must not fabricate an index that contradicts the
723+
// machine-readable pinnedAccountIndex: null.
724+
expect(body.message).toContain("The pinned account is currently unavailable;");
725+
expect(body.message).not.toContain("Pinned account 1");
723726
});
724727

725728
it("mirrors the full accountSkipReasons map even when the pinned entry is unknown", () => {

test/rate-limit-decision.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,11 +275,14 @@ describe("buildPinnedUnavailableErrorBody", () => {
275275
expect(body.account_skip_reasons).toEqual({ "0": "rate-limited", "1": "disabled" });
276276
});
277277

278-
it("omits the parenthetical and reason on the null-index desync path", () => {
278+
it("omits the index, parenthetical, and reason on the null-index desync path", () => {
279+
// Regression: the message used to render the null index as "Pinned
280+
// account 1", contradicting the machine-readable pinnedAccountIndex.
279281
const body = buildPinnedUnavailableErrorBody(null, new Map());
280282
expect(body.pinnedAccountIndex).toBeNull();
281283
expect(body.reason).toBeNull();
282-
expect(body.message).toContain("Pinned account 1 is currently unavailable;");
284+
expect(body.message).toContain("The pinned account is currently unavailable;");
285+
expect(body.message).not.toContain("Pinned account 1");
283286
expect(body.message).not.toContain("(");
284287
expect(body.account_skip_reasons).toEqual({});
285288
});

test/stream-failover-runtime.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,19 @@ describe("responseHeadersForClient", () => {
9696
});
9797
});
9898

99+
it("blocks any header under the private account prefix, not just known names", () => {
100+
// Regression: the filter used to be an exact-name allowlist, so a future
101+
// x-codex-multi-auth-account-* header would have leaked by default.
102+
const upstream = new Headers({
103+
"content-type": "application/json",
104+
"x-codex-multi-auth-account-plan": "pro",
105+
"X-Codex-Multi-Auth-Account-Future-Field": "secret",
106+
});
107+
expect(responseHeadersForClient(upstream)).toEqual({
108+
"content-type": "application/json",
109+
});
110+
});
111+
99112
it("covers every hop-by-hop header in the exported set", () => {
100113
const upstream = new Headers();
101114
for (const name of HOP_BY_HOP_HEADERS) {

0 commit comments

Comments
 (0)