From f0226b4bde34847c00996d12b3debe7f8c78b4d1 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Sun, 20 Sep 2026 12:25:44 +0900 Subject: [PATCH 01/32] fix(kiro): avoid mixed-script estimator allocations (cherry picked from commit 24c3cd3d2ca0e62228cbe34a225d1590a3ae15f5) --- src/adapters/kiro/usage.ts | 7 +++-- src/lib/token-estimate.ts | 16 ++++++++-- tests/lib/token-estimate.test.ts | 14 ++++++++- .../providers/kiro/kiro-wire-estimate.test.ts | 30 +++++++++++++++++++ 4 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 tests/providers/kiro/kiro-wire-estimate.test.ts diff --git a/src/adapters/kiro/usage.ts b/src/adapters/kiro/usage.ts index 3bb8f2b0ffe..372739e4f33 100644 --- a/src/adapters/kiro/usage.ts +++ b/src/adapters/kiro/usage.ts @@ -1,4 +1,4 @@ -import { estimateTokens } from "../../lib/token-estimate"; +import { estimateTokens, estimateTokensFromCharacterCounts } from "../../lib/token-estimate"; import { KIRO_MODEL_CONTEXT_WINDOWS, normalizeKiroModelId } from "../../providers/kiro-models"; import { modelRecordValue } from "../../reasoning-effort"; import { sniffImageDimensions } from "../anthropic-image-guard"; @@ -98,8 +98,9 @@ export function estimateKiroWireTokens(text: string, modelId: string): number { if (!text) return 0; const cjk = kiroCjkCount(text); if (cjk === 0) return Math.ceil(estimateKiroTokens(text, modelId) * KIRO_LATIN_WIRE_EXPANSION); - const latinTokens = estimateKiroTokens("x".repeat(text.length - cjk), modelId); - const cjkTokens = estimateKiroTokens("\uac00".repeat(cjk), modelId); + const prefixedModelId = `kiro/${modelId}`; + const latinTokens = estimateTokensFromCharacterCounts(text.length - cjk, 0, prefixedModelId); + const cjkTokens = estimateTokensFromCharacterCounts(0, cjk, prefixedModelId); return Math.ceil(latinTokens * KIRO_LATIN_WIRE_EXPANSION + cjkTokens); } diff --git a/src/lib/token-estimate.ts b/src/lib/token-estimate.ts index bf507e1cf27..0a9e4a74401 100644 --- a/src/lib/token-estimate.ts +++ b/src/lib/token-estimate.ts @@ -142,12 +142,24 @@ export function estimateTokens(text: string, modelId?: string, contextWindow?: n if (!text) return 0; const len = text.length; if (len === 0) return 0; - const latinRatio = charsPerToken(modelId); const cjk = countCjk(text); + return estimateTokensFromCharacterCounts(len - cjk, cjk, modelId, contextWindow); +} + +/** Estimate tokens from already-counted script buckets without materializing replacement text. */ +export function estimateTokensFromCharacterCounts( + latin: number, + cjk: number, + modelId?: string, + contextWindow?: number, +): number { + const len = latin + cjk; + if (len === 0) return 0; + const latinRatio = charsPerToken(modelId); // Continuous in the CJK share: no threshold, so one added Korean character moves the estimate // by a fraction of a token instead of switching the whole blob to a different divisor. const estimate = cjk === 0 ? Math.ceil(len / latinRatio) - : Math.ceil((len - cjk) / latinRatio + cjk / CJK_CHARS_PER_TOKEN); + : Math.ceil(latin / latinRatio + cjk / CJK_CHARS_PER_TOKEN); return capEstimateAtContextWindow(Math.max(1, estimate), contextWindow); } diff --git a/tests/lib/token-estimate.test.ts b/tests/lib/token-estimate.test.ts index a7ba989da4d..fa058fb0b90 100644 --- a/tests/lib/token-estimate.test.ts +++ b/tests/lib/token-estimate.test.ts @@ -1,5 +1,10 @@ import { describe, expect, test } from "bun:test"; -import { capEstimateAtContextWindow, charsPerToken, estimateTokens } from "../../src/lib/token-estimate"; +import { + capEstimateAtContextWindow, + charsPerToken, + estimateTokens, + estimateTokensFromCharacterCounts, +} from "../../src/lib/token-estimate"; describe("script-segmented ratio", () => { const korean = "한국어 텍스트는 토큰 밀도가 높아서 영어 기준 추정이 과소계산됩니다 ".repeat(10); @@ -10,6 +15,13 @@ describe("script-segmented ratio", () => { return Math.ceil((s.length - cjk) / latin + cjk / 1.5); }; + test("pre-counted script buckets preserve estimates without replacement strings", () => { + expect(estimateTokensFromCharacterCounts(10, 3, "kiro/kiro-auto")) + .toBe(estimateTokens("xxxxxxxxxx한한한", "kiro/kiro-auto")); + expect(estimateTokensFromCharacterCounts(250_000_000, 1, "kiro/kiro-auto")) + .toBe(Math.ceil(250_000_000 / 2.8 + 1 / 1.5)); + }); + test("CJK characters are counted at their own denser ratio, not the model ratio", () => { expect(estimateTokens(korean, "gpt-5.6-sol")).toBe(expected(korean, 4)); expect(estimateTokens(korean, "kiro/claude-opus-5")).toBe(expected(korean, 2.8)); diff --git a/tests/providers/kiro/kiro-wire-estimate.test.ts b/tests/providers/kiro/kiro-wire-estimate.test.ts new file mode 100644 index 00000000000..5e905d728b4 --- /dev/null +++ b/tests/providers/kiro/kiro-wire-estimate.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, test } from "bun:test"; +import { estimateKiroWireTokens, KIRO_LATIN_WIRE_EXPANSION, kiroCjkCount } from "../../../src/adapters/kiro/usage"; +import { estimateTokens } from "../../../src/lib/token-estimate"; + +describe("kiro wire token estimate", () => { + const model = "claude-opus-5"; + + // The mixed-script path must produce the same estimate as materializing per-script + // replacement strings did, without allocating them. + test("mixed-script estimate matches the replacement-string formula", () => { + const text = "const x = fetch(url); // 요청을 보내고 응답을 파싱한다".repeat(50); + const cjk = kiroCjkCount(text); + const prefixed = `kiro/${model}`; + const expected = Math.ceil( + estimateTokens("x".repeat(text.length - cjk), prefixed) * KIRO_LATIN_WIRE_EXPANSION + + estimateTokens("\uac00".repeat(cjk), prefixed), + ); + expect(estimateKiroWireTokens(text, model)).toBe(expected); + }); + + test("pure-Latin text keeps the wire expansion on the whole estimate", () => { + const english = "console.log(\"hello world\");".repeat(20); + expect(estimateKiroWireTokens(english, model)) + .toBe(Math.ceil(estimateTokens(english, `kiro/${model}`) * KIRO_LATIN_WIRE_EXPANSION)); + }); + + test("empty text estimates to zero", () => { + expect(estimateKiroWireTokens("", model)).toBe(0); + }); +}); From d9eb5d76d98bfe3ae68f9b0c8c022ce78e3eb1ef Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Tue, 22 Sep 2026 10:05:09 +0900 Subject: [PATCH 02/32] test(kiro): register kiro-wire-estimate in test layout (cherry picked from commit e796b5c07a5c91a3bb0a1707160eb6809d28de8c) --- scripts/test-layout/layout.json | 1 + tests/fixtures/test-layout-expected.json | 1 + 2 files changed, 2 insertions(+) diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index 2e31c8b2431..17f1fb4059e 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -895,6 +895,7 @@ "kiro-usage-quota.test.ts": "providers/kiro", "kiro-windows-cli-db-path.test.ts": "providers/kiro", "kiro-windows-cli-executable-path.test.ts": "providers/kiro", + "kiro-wire-estimate.test.ts": "providers/kiro", "lab-activation.test.ts": "lab", "lab-automation-coderabbit-regressions.test.ts": "lab", "lab-automation-final-coderabbit-regressions.test.ts": "lab", diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 25bf5372234..3b94eaae067 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -721,6 +721,7 @@ "kiro-usage-quota.test.ts": "providers/kiro", "kiro-windows-cli-db-path.test.ts": "providers/kiro", "kiro-windows-cli-executable-path.test.ts": "providers/kiro", + "kiro-wire-estimate.test.ts": "providers/kiro", "lab-activation.test.ts": "lab", "lab-automation-coderabbit-regressions.test.ts": "lab", "lab-automation-final-coderabbit-regressions.test.ts": "lab", From 9f128c1b12751f6754340576c4bb6a2a01f4bae4 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 05:14:27 +0900 Subject: [PATCH 03/32] fix(crash-guard): count unparenthesized JS frames as real throw sites isBenignAbortTeardown only recognised parenthesized "(file:line:col)" frames, so a genuine TypeError whose stack carries "at /abs/x.ts:1:2", "at file:///x.ts:1:2" or a Windows drive path was folded into the rate-limited benign-teardown summary. Scan every "at ..." frame and treat any non-native line:col location as a JS source frame. Hidden JSC fields (sourceURL/line/column) deliberately do not veto the benign class: Bun can attach them to errors raised from builtin frames, and the benign summary already records them through diagnose(). Reimplements #5286. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- src/lib/crash-guard.ts | 21 ++++++++++++++++++--- tests/service/crash-guard.test.ts | 22 ++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/src/lib/crash-guard.ts b/src/lib/crash-guard.ts index e7661f5bcbc..62bca7e0d0c 100644 --- a/src/lib/crash-guard.ts +++ b/src/lib/crash-guard.ts @@ -176,9 +176,24 @@ export function isBenignAbortTeardown(err: unknown): boolean { const lockedStreamTeardown = err.message === "Invalid state: ReadableStream is locked" && (err as { code?: unknown }).code === "ERR_INVALID_STATE"; if (!bareNullTeardown && !lockedStreamTeardown) return false; - const stack = err.stack ?? ""; - // Native-only: no JS source frame. A real app TypeError would carry a `(file:line:col)` frame. - return !/\((?!native:)[^)]*:\d+:\d+\)/.test(stack); + // Native-only: no JS source frame, parenthesized or not. Hidden JSC source fields + // (sourceURL/line/column) do not decide this: Bun can attach them to errors raised from + // its own builtin frames, and the benign summary still records them through diagnose(). + return !hasJsSourceFrame(err.stack ?? ""); +} + +/** + * True when a stack line is an `at …` frame ending in `line:col` (optionally inside + * parentheses) whose location is not a Bun builtin (`native:`). Covers `at fn (/abs/x.ts:1:2)`, + * `at /abs/x.ts:1:2`, `at async fn (file:///x.ts:1:2)` and Windows drive paths. + */ +function hasJsSourceFrame(stack: string): boolean { + return stack.split(/\r?\n/).some(raw => { + const frame = raw.trim(); + return frame.startsWith("at ") + && /:\d+:\d+\)?$/.test(frame) + && !/[(\s]native:\d+:\d+\)?$/.test(frame); + }); } function record(kind: string, err: unknown, promise?: unknown): void { diff --git a/tests/service/crash-guard.test.ts b/tests/service/crash-guard.test.ts index b09f40029a7..90074751d7b 100644 --- a/tests/service/crash-guard.test.ts +++ b/tests/service/crash-guard.test.ts @@ -118,6 +118,28 @@ describe("benign abort-teardown classification", () => { expect(isBenignAbortTeardown(err)).toBe(false); }); + test("does NOT flag unparenthesized, async, file-URL, or Windows JS source frames", () => { + for (const frame of [ + "at /abs/src/server.ts:120:13", + "at async handler (file:///abs/src/server.ts:120:13)", + "at file:///abs/src/server.ts:120:13", + "at C:\\app\\src\\server.ts:120:13", + ]) { + const err = new TypeError("null is not an object"); + err.stack = `TypeError: null is not an object\r\n at (native:1:11)\r\n ${frame}`; + expect(isBenignAbortTeardown(err), frame).toBe(false); + } + }); + + test("hidden JSC source fields alone do not veto a native-only stack", () => { + // Bun can attach sourceURL/line/column to errors raised from builtin frames, and the + // benign summary still records them through diagnose(); only a real JS frame vetoes. + const err = new TypeError("null is not an object"); + err.stack = "TypeError: null is not an object\n at (native:1:11)\n at native:7:39"; + Object.assign(err, { sourceURL: "/abs/src/server.ts", line: 1216, column: 24 }); + expect(isBenignAbortTeardown(err)).toBe(true); + }); + test("does NOT flag a different message or the (evaluating …) form", () => { const a = new TypeError("null is not an object (evaluating 'x.y')"); a.stack = "TypeError: ...\n at (native:1:11)"; From 5446f754520a121f7edfaabbd805c74c1901e480 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Mon, 21 Sep 2026 17:26:39 +0900 Subject: [PATCH 04/32] fix(proxy): keep loopback traffic outside inherited SOCKS (cherry picked from commit 37469917f1eb45a26aa6d611599db0316076d177) --- src/config/proxy-env.ts | 44 +++++++++++++++++++++++++--------- structure/config.md | 4 +++- tests/server/proxy-env.test.ts | 25 ++++++++++++++++--- 3 files changed, 58 insertions(+), 15 deletions(-) diff --git a/src/config/proxy-env.ts b/src/config/proxy-env.ts index adbfe5f6176..e9285e93266 100644 --- a/src/config/proxy-env.ts +++ b/src/config/proxy-env.ts @@ -98,6 +98,37 @@ function warnProxyConfigDiscardOnce(kind: "proxy" | "noProxy" | "noProxyElements } } +// Loopback only has a proxy to bypass when the environment already carries proxy state. +// Writing NO_PROXY into a proxy-free process is itself a proxy-env mutation that callers +// observe (the lab sandbox rejects any of these keys as a forbidden leak), so the +// early-return merge runs only when one is already present. +const PROXY_STATE_ENV_KEYS = [ + "HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY", + "http_proxy", "https_proxy", "all_proxy", "no_proxy", +] as const; + +function ambientProxyStateExists(): boolean { + for (const key of PROXY_STATE_ENV_KEYS) { + const value = process.env[key]; + if (value !== undefined && value !== "") return true; + } + return false; +} + +function mergeNoProxyEntries(configured: string[] = []): void { + const existing = process.env.NO_PROXY ?? process.env.no_proxy ?? ""; + const entries = existing.split(",").map(s => s.trim()).filter(Boolean); + const seen = new Set(entries.map(entry => entry.toLowerCase())); + for (const host of [...configured, "localhost", "127.0.0.1", "::1", "[::1]"]) { + const key = host.toLowerCase(); + if (!seen.has(key)) { + entries.push(host); + seen.add(key); + } + } + process.env.NO_PROXY = entries.join(","); +} + /** * Mirror `config.proxy` into HTTP(S)_PROXY env vars. Bun fetch consumes them natively; transports * such as the ChatGPT upstream WebSocket select the same environment explicitly. User-set HTTP(S)_PROXY @@ -130,6 +161,7 @@ export function applyProxyEnvWith( let proxy = typeof rawProxy === "string" ? resolveEnvValue(rawProxy) : undefined; if (!proxy) { if (rawProxy !== undefined) warnProxyConfigDiscardOnce("proxy"); + if (ambientProxyStateExists()) mergeNoProxyEntries(); configureSocks5Fetch(); return; } @@ -178,9 +210,6 @@ export function applyProxyEnvWith( if (!process.env.HTTPS_PROXY?.trim() && !process.env.https_proxy?.trim()) process.env.HTTPS_PROXY = proxy; } } - const existing = process.env.NO_PROXY ?? process.env.no_proxy ?? ""; - const entries = existing.split(",").map(s => s.trim()).filter(Boolean); - const seen = new Set(entries.map(e => e.toLowerCase())); // Configured entries first, then loopback: loopback is unconditional, so appending it last // keeps it present even when the operator lists a loopback host themselves. const raw = config.noProxy; @@ -200,13 +229,6 @@ export function applyProxyEnvWith( const configured = configuredEntries .map(entry => entry.trim()) .filter(Boolean); - for (const host of [...configured, "localhost", "127.0.0.1", "::1", "[::1]"]) { - const key = host.toLowerCase(); - if (!seen.has(key)) { - entries.push(host); - seen.add(key); - } - } - process.env.NO_PROXY = entries.join(","); + mergeNoProxyEntries(configured); configureSocks5Fetch(); } diff --git a/structure/config.md b/structure/config.md index 049166a52ae..ce35ef0a3fc 100644 --- a/structure/config.md +++ b/structure/config.md @@ -542,7 +542,9 @@ Stored Direct substitution follows the [credential identity contract](providers/ configuration. An explicit SOCKS5 or SOCKS5h URL selects ALL_PROXY and removes stale scheme-proxy variables; HTTP(S) settings retain their existing environment precedence. Activation keeps the existing Windows auto-discovery path and loopback -NO_PROXY entries. When the environment no longer selects SOCKS, activation +NO_PROXY entries; the no-configured-proxy return merges them only when the environment +already carries proxy state, leaving a proxy-free process untouched. When the +environment no longer selects SOCKS, activation restores the native fetch; removing a saved field alone does not erase inherited process environment variables. SOCKS4 is rejected instead of being advertised as a working transport. diff --git a/tests/server/proxy-env.test.ts b/tests/server/proxy-env.test.ts index 340454dcfb6..30ff4c9dcce 100644 --- a/tests/server/proxy-env.test.ts +++ b/tests/server/proxy-env.test.ts @@ -1,7 +1,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { createServer } from "node:http"; import { applyProxyEnv } from "../../src/config"; -import { resolveProxyRoute, configureSocks5Fetch } from "../../src/lib/proxy-env"; +import { configuredOutboundFetch, resolveProxyRoute, configureSocks5Fetch } from "../../src/lib/proxy-env"; import type { OcxConfig } from "../../src/types"; const PROXY_ENV_KEYS = ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "all_proxy", "no_proxy", "OCX_TEST_PROXY_REF", "OCX_TEST_NO_PROXY_REF"] as const; @@ -215,12 +215,31 @@ describe("applyProxyEnv with values the schema does not constrain", () => { }); describe("applyProxyEnv", () => { - test("no-op when config.proxy is unset", () => { + test("writes no proxy state into an environment that has none", () => { + applyProxyEnv(configWithProxy()); + for (const key of PROXY_ENV_KEYS) { + expect(process.env[key]).toBeUndefined(); + } + }); + + test("keeps mandatory loopback exclusions when config.proxy is unset", () => { process.env.NO_PROXY = "operator-owned.example"; applyProxyEnv(configWithProxy(undefined, "internal.example")); expect(process.env.HTTP_PROXY).toBeUndefined(); expect(process.env.HTTPS_PROXY).toBeUndefined(); - expect(process.env.NO_PROXY).toBe("operator-owned.example"); + expect(process.env.NO_PROXY).toBe("operator-owned.example,localhost,127.0.0.1,::1,[::1]"); + }); + + test.each(["ALL_PROXY", "all_proxy"])("inherited SOCKS %s cannot intercept loopback fetches", async key => { + process.env[key] = "socks5://untrusted-proxy.invalid:1080"; + applyProxyEnv(configWithProxy()); + let directCalls = 0; + const response = await configuredOutboundFetch("http://127.0.0.1:11434/v1/chat/completions", undefined, async () => { + directCalls += 1; + return new Response("direct"); + }); + expect(await response.text()).toBe("direct"); + expect(directCalls).toBe(1); }); test("merges configured comma-separated noProxy entries", () => { From 688f60d7a133d3fd48437a7d55296b7ecc8acd00 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Tue, 22 Sep 2026 07:03:44 +0900 Subject: [PATCH 05/32] fix(proxy): pin non-loopback SOCKS ownership and document the ambient noProxy scope (cherry picked from commit 7dd72d1d61adc9bf2af65a1e5951a6f697e605f9) --- src/config/proxy-env.ts | 4 ++++ tests/server/proxy-env.test.ts | 14 ++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/src/config/proxy-env.ts b/src/config/proxy-env.ts index e9285e93266..f096cd3885a 100644 --- a/src/config/proxy-env.ts +++ b/src/config/proxy-env.ts @@ -161,6 +161,10 @@ export function applyProxyEnvWith( let proxy = typeof rawProxy === "string" ? resolveEnvValue(rawProxy) : undefined; if (!proxy) { if (rawProxy !== undefined) warnProxyConfigDiscardOnce("proxy"); + // Ambient-proxy path: only loopback bypasses are appended. A configured noProxy is + // deliberately NOT merged here — with no config.proxy the operator's bypass list has + // no declared proxy to apply against, and merging it would silently widen direct + // egress beyond the loopback fix this branch exists for. if (ambientProxyStateExists()) mergeNoProxyEntries(); configureSocks5Fetch(); return; diff --git a/tests/server/proxy-env.test.ts b/tests/server/proxy-env.test.ts index 30ff4c9dcce..ca1016b16cf 100644 --- a/tests/server/proxy-env.test.ts +++ b/tests/server/proxy-env.test.ts @@ -242,6 +242,20 @@ describe("applyProxyEnv", () => { expect(directCalls).toBe(1); }); + test.each(["ALL_PROXY", "all_proxy"])("inherited SOCKS %s still owns non-loopback fetches", async key => { + process.env[key] = "socks5://untrusted-proxy.invalid:1080"; + applyProxyEnv(configWithProxy()); + // The bypass must be scoped to loopback only: a non-loopback URL still routes through + // the inherited SOCKS proxy, which fails here because the proxy is unreachable. The + // direct fallback must NOT be consulted — if it were, the bypass leaked. + let directCalls = 0; + await expect(configuredOutboundFetch("http://api.example.com/v1/chat/completions", undefined, async () => { + directCalls += 1; + return new Response("direct"); + })).rejects.toThrow(); + expect(directCalls).toBe(0); + }); + test("merges configured comma-separated noProxy entries", () => { applyProxyEnv(configWithProxy("http://proxy.corp:8080", "internal.example,10.0.0.0/8")); expect(process.env.NO_PROXY).toBe("internal.example,10.0.0.0/8,localhost,127.0.0.1,::1,[::1]"); From 20a65e8382da47640587acbc8e2dec791154b2f7 Mon Sep 17 00:00:00 2001 From: Epinephrine Date: Mon, 21 Sep 2026 02:22:01 +0000 Subject: [PATCH 06/32] fix(oauth): preserve hashes in command code callback JSON (cherry picked from commit 225eb81f1a61d645ac8c2e6160b2997d689382d7) --- src/oauth/login-flow-state.ts | 8 ++++---- structure/providers-and-adapters.md | 2 +- tests/oauth/oauth-manual-code.test.ts | 21 +++++++++++++++++++++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/oauth/login-flow-state.ts b/src/oauth/login-flow-state.ts index a3a4951c3f4..6a1592fbc3f 100644 --- a/src/oauth/login-flow-state.ts +++ b/src/oauth/login-flow-state.ts @@ -97,13 +97,13 @@ export function submitManualLoginCode(provider: string, input: string): { ok: tr // stashed and re-validated by the callback loop. const parsed = parseCallbackInput(trimmed); // Command Code's manual fallback accepts a pasted JSON callback payload - // (`{ apiKey, state, ... }`) which has no `code` param. Let it through the - // shared gate so the provider-specific parser can validate it. - const isCommandCodeJson = provider === "command-code" && trimmed.startsWith("{") && !parsed.code; + // (`{ apiKey, state, ... }`). Keep that opaque to the generic raw parser so + // hashes in JSON strings do not become a fake state suffix; its provider parser validates state. + const isCommandCodeJson = provider === "command-code" && trimmed.startsWith("{"); if (!parsed.code && !isCommandCodeJson) return { ok: false, error: "no authorization code found in input" }; // A raw paste carrying an explicit code#state suffix is state-bearing too: it // must match the expected state rather than bypass validation. - const stateBearing = parsed.kind !== "raw" || parsed.state !== undefined; + const stateBearing = !isCommandCodeJson && (parsed.kind !== "raw" || parsed.state !== undefined); if (stateBearing && slot.expectedState !== undefined) { if (parsed.state === undefined) return { ok: false, error: "redirect URL is missing the state parameter" }; if (parsed.state !== slot.expectedState) { diff --git a/structure/providers-and-adapters.md b/structure/providers-and-adapters.md index bf4271d54fe..fd601bc5868 100644 --- a/structure/providers-and-adapters.md +++ b/structure/providers-and-adapters.md @@ -15,7 +15,7 @@ only canonical Fable, Opus, or Sonnet labels after removing terminal controls; u | `src/providers/model-rename-fields.ts`, `src/providers/model-rename-migration.ts` | Classifies every provider config field for a declared model rename. Exact-model records, lists and nested request-pacing keys follow the replacement; an already saved replacement entry wins. Provider-wide settings and credential fields are not model identities. | | `src/providers/resolved-model-policy.ts`, `src/providers/resolved-model-policy-merge.ts` | Static provider/model policy resolution for the final upstream wire model, plus its pure clone/merge/URL/family helpers. The resolver detaches and freezes registry defaults, operator overrides, exact explicit input-modality declarations, hard wire pins, aliases, and explicit false/empty values with field-level provenance. Provider derivation, routing, catalog hints, gather admission, and adapter selection consume its detached frozen result. Callers supply transport match, the exact capability row, and a credential-free effective auth decision; credential bytes, usability evidence, account/quota/health state, and observed limits remain outside the result. | -| `src/oauth/` | OAuth providers, token storage, refresh, and auth-token resolution. The login callback listener binds a per-provider FIXED loopback port, so consecutive logins reuse the same number; every response it sends ends its connection (`Connection: close`, including non-callback paths such as a stray `/favicon.ico` 404). Stopping the listener does not close an established socket, so without that a pooled client would deliver the next login's callback to the retired flow, which rejects the unknown state as a CSRF mismatch while the live flow waits. Kiro add-account identity prefers same-session `whoami` over a leftover SQLite state profile, and never persists the Builder ID service profile ARN as `accountId`. | +| `src/oauth/` | OAuth providers, token storage, refresh, and auth-token resolution. The login callback listener binds a per-provider FIXED loopback port, so consecutive logins reuse the same number; every response it sends ends its connection (`Connection: close`, including non-callback paths such as a stray `/favicon.ico` 404). Stopping the listener does not close an established socket, so without that a pooled client would deliver the next login's callback to the retired flow, which rejects the unknown state as a CSRF mismatch while the live flow waits. Command Code manual callback JSON remains opaque to the shared `code#state` parser and is state-validated by its provider parser. Kiro add-account identity prefers same-session `whoami` over a leftover SQLite state profile, and never persists the Builder ID service profile ARN as `accountId`. | | `src/combos/request.ts` | Clones each selected combo target request and applies the existing target capability ladder: adaptive unknown targets and explicit empty ladders receive no unsupported reasoning/thinking controls, while known ladders retain per-target resolution. | | `src/adapters/openai-responses.ts` | Native OpenAI/ChatGPT Responses passthrough. | | `src/responses/muse-tool-name-alias.ts` | Host-gated Meta Muse 64-char tool-name alias/restore used by the Responses passthrough. | diff --git a/tests/oauth/oauth-manual-code.test.ts b/tests/oauth/oauth-manual-code.test.ts index 25aa20ed3e5..d1a52164cb8 100644 --- a/tests/oauth/oauth-manual-code.test.ts +++ b/tests/oauth/oauth-manual-code.test.ts @@ -13,6 +13,7 @@ import { submitManualLoginCode, } from "../../src/oauth"; import { parseCallbackInput } from "../../src/oauth/callback-server"; +import { loginState, waitForManualLoginCode } from "../../src/oauth/login-flow-state"; import { saveConfig } from "../../src/config"; import { startServer } from "../../src/server"; import { findAvailablePort } from "../../src/server/ports"; @@ -190,6 +191,26 @@ describe("OAuth manual login code fallback", () => { expect(submitManualLoginCode("xai", " ")).toEqual({ ok: false, error: "empty code" }); }); + test("Command Code callback JSON keeps hashes inside provider fields opaque", async () => { + loginState.set("command-code", { done: false }); + const controller = new AbortController(); + const pending = waitForManualLoginCode("command-code", controller.signal, "expected-state"); + const callback = JSON.stringify({ + apiKey: "key#segment", + state: "expected-state", + userId: "user-1", + userName: "alice#1", + keyName: "cli", + }); + try { + expect(submitManualLoginCode("command-code", callback)).toEqual({ ok: true }); + expect(await pending).toBe(callback); + } finally { + controller.abort("test complete"); + clearLoginState("command-code"); + } + }); + test("OAuth pending code rejects 4097 UTF-8 bytes in the owner", async () => { const originalFetch = globalThis.fetch; globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { From 60cbb55177c89ad958dabf6447383a9ac40658dc Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 05:14:54 +0900 Subject: [PATCH 07/32] test(oauth): drive Command Code callback JSON through the shared submit path The provider parser, not the shared code#state gate, owns the state check for pasted Command Code callback JSON. Pin both halves end to end: a wrong-state paste is accepted by submitManualLoginCode, rejected by the provider loop and re-prompted without a whoami call, and the right-state paste keeps "#" inside its fields intact through to the stored key. Follow-up to the carried #5413 fix. --- tests/providers/command-code-provider.test.ts | 58 ++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/tests/providers/command-code-provider.test.ts b/tests/providers/command-code-provider.test.ts index 692bbf45586..2c962f95cb5 100644 --- a/tests/providers/command-code-provider.test.ts +++ b/tests/providers/command-code-provider.test.ts @@ -11,7 +11,8 @@ import { budgetOwner } from "../helpers/send-budget-owner"; import type { OcxConfig } from "../../src/types"; import { commandCodeSessionId, createCommandCodeAdapter } from "../../src/adapters/command-code"; import { loginCommandCode, parseCommandCodeCallback, shouldImportLocalCommandCodeAuth } from "../../src/oauth/command-code"; -import { buildModelsRequest, OAUTH_PROVIDERS } from "../../src/oauth"; +import { buildModelsRequest, OAUTH_PROVIDERS, submitManualLoginCode } from "../../src/oauth"; +import { clearManualCodeSlot, loginState, waitForManualLoginCode } from "../../src/oauth/login-flow-state"; import { commandCodeReasoningEfforts, refreshCommandCodeReasoningEfforts, @@ -300,6 +301,61 @@ describe("Command Code provider", () => { } }); + test("callback JSON pasted through the shared submit path: wrong state re-prompts, hashes survive", async () => { + const controller = new AbortController(); + const originalFetch = globalThis.fetch; + const whoamiKeys: string[] = []; + globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => { + const href = String(input); + if (href.includes("whoami")) { + whoamiKeys.push(new Headers(init?.headers).get("authorization") ?? ""); + return new Response(JSON.stringify({ ok: true, user: { id: "u-1", userName: "alice#1" } }), { status: 200 }); + } + throw new Error(`unexpected fetch: ${href}`); + }) as typeof globalThis.fetch; + loginState.set("command-code", { done: false }); + const prompts: string[] = []; + const promptCount = async (count: number) => { + for (let i = 0; prompts.length < count && i < 400; i++) await Bun.sleep(5); + expect(prompts.length).toBeGreaterThanOrEqual(count); + }; + const callback = (state: string) => JSON.stringify({ + apiKey: "sk-key#segment", + state, + userId: "u-1", + userName: "alice#1", + keyName: "cli", + }); + try { + const login = loginCommandCode({ + onAuth: () => {}, + onProgress: () => {}, + onManualCodeInput: state => { + prompts.push(state); + return waitForManualLoginCode("command-code", controller.signal, state); + }, + signal: controller.signal, + }, { importLocal: "off" }); + await promptCount(1); + const state = prompts[0]!; + + // The shared gate lets Command Code JSON through; the provider parser owns the state check. + expect(submitManualLoginCode("command-code", callback(`${state}-other`))).toEqual({ ok: true }); + await promptCount(2); + expect(whoamiKeys).toHaveLength(0); + + // A "#" inside a JSON field must not be read as a code#state suffix. + expect(submitManualLoginCode("command-code", callback(state))).toEqual({ ok: true }); + expect(await login).toMatchObject({ access: "sk-key#segment", accountId: "u-1", source: "oauth" }); + expect(whoamiKeys).toEqual(["Bearer sk-key#segment"]); + } finally { + controller.abort(new Error("test complete")); + globalThis.fetch = originalFetch; + loginState.delete("command-code"); + clearManualCodeSlot("command-code"); + } + }); + test("uses live account discovery and only imports local CLI auth for the first account", () => { const request = buildModelsRequest(provider, "secret-command-key", "command-code"); expect(request).toEqual({ From 9567ead4c0aad2deff3d917eff7eb711cb1ec8fe Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Mon, 21 Sep 2026 18:44:01 +0900 Subject: [PATCH 08/32] docs(remote-hub): warn about shared-host unauthenticated loopback companion (cherry picked from commit a0233ca5d7ec2b838f039dd265ec30bbc09c5203) --- docs-site/src/content/docs/guides/remote-hub.md | 7 +++++++ docs-site/src/content/docs/ko/guides/remote-hub.md | 4 ++++ tests/ci-workflows/docs-remote-hub-claims.test.ts | 12 ++++++++++++ 3 files changed, 23 insertions(+) diff --git a/docs-site/src/content/docs/guides/remote-hub.md b/docs-site/src/content/docs/guides/remote-hub.md index d0399c213cf..07d1dbb55bc 100644 --- a/docs-site/src/content/docs/guides/remote-hub.md +++ b/docs-site/src/content/docs/guides/remote-hub.md @@ -111,6 +111,13 @@ Bind the data listener to the hub's Tailscale address, enable the loopback compa own processes reach that same port without a credential, and publish management separately. The values below are examples: +:::danger[Use a dedicated single-tenant host] +The loopback companion is unauthenticated: every process and OS user on this machine can use the +hub's provider credentials and account quota, and can starve authenticated remote clients. Do not +enable it on a shared or multi-tenant host. If the host is shared, omit the +`unauthenticatedLoopbackListener` command and do not run the hub's local integrations. +::: + ```bash ocx config set runtimeRole hub ocx config set hostname 100.64.0.10 diff --git a/docs-site/src/content/docs/ko/guides/remote-hub.md b/docs-site/src/content/docs/ko/guides/remote-hub.md index 0b1cb95941c..edd5824e28f 100644 --- a/docs-site/src/content/docs/ko/guides/remote-hub.md +++ b/docs-site/src/content/docs/ko/guides/remote-hub.md @@ -51,6 +51,10 @@ ocx sync 데이터 리스너는 허브의 Tailscale 주소에 바인드하고, 허브 자신의 프로세스가 같은 포트를 자격 증명 없이 쓸 수 있도록 루프백 companion을 켜고, 관리 평면은 따로 공개합니다. 아래 값은 예시입니다. +:::danger[전용 단일 테넌트 호스트를 사용하세요] +루프백 companion은 인증이 없습니다. 이 머신의 모든 프로세스와 OS 사용자가 허브의 프로바이더 자격 증명과 계정 쿼터를 사용할 수 있고, 인증된 원격 클라이언트를 굶길 수 있습니다. 공유 또는 다중 테넌트 호스트에서는 활성화하지 마세요. 호스트가 공유라면 `unauthenticatedLoopbackListener` 명령을 생략하고 허브의 로컬 통합을 실행하지 마세요. +::: + ```bash ocx config set runtimeRole hub ocx config set hostname 100.64.0.10 diff --git a/tests/ci-workflows/docs-remote-hub-claims.test.ts b/tests/ci-workflows/docs-remote-hub-claims.test.ts index e6812f31fb5..a9e32d732c2 100644 --- a/tests/ci-workflows/docs-remote-hub-claims.test.ts +++ b/tests/ci-workflows/docs-remote-hub-claims.test.ts @@ -123,6 +123,18 @@ describe("the one-port hub recipe", () => { } }); + test("both locales warn that the companion requires a dedicated host", async () => { + const warnings = [ + ["en", GUIDE, "every process and OS user", "shared or multi-tenant host"], + ["ko", KO_GUIDE, "모든 프로세스와 OS 사용자", "공유 또는 다중 테넌트 호스트에서는 활성화하지 마세요"], + ] as const; + for (const [locale, file, localAccess, sharedHost] of warnings) { + const source = await Bun.file(file).text(); + expect(source, locale).toContain(localAccess); + expect(source, locale).toContain(sharedHost); + } + }); + test("no locale tells the operator to export a data-plane token by hand", async () => { for (const [locale, file] of LOCALES) { const source = await Bun.file(file).text(); From 47a8128a8591a33d510f3681118adce862931cfa Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Tue, 22 Sep 2026 06:45:53 +0900 Subject: [PATCH 09/32] docs(remote-hub): align the loopback warning with the reference and cover the ported form (cherry picked from commit b00ed1ebf2c0d7d8029dd1172ef156a56ad1ab61) --- docs-site/src/content/docs/guides/remote-hub.md | 14 +++++++++++--- .../src/content/docs/ko/guides/remote-hub.md | 6 +++++- skills/ocx/references/05_remote_hub.md | 5 +++++ .../ci-workflows/docs-remote-hub-claims.test.ts | 16 +++++++++++++--- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/docs-site/src/content/docs/guides/remote-hub.md b/docs-site/src/content/docs/guides/remote-hub.md index 07d1dbb55bc..882344538e2 100644 --- a/docs-site/src/content/docs/guides/remote-hub.md +++ b/docs-site/src/content/docs/guides/remote-hub.md @@ -113,9 +113,14 @@ values below are examples: :::danger[Use a dedicated single-tenant host] The loopback companion is unauthenticated: every process and OS user on this machine can use the -hub's provider credentials and account quota, and can starve authenticated remote clients. Do not -enable it on a shared or multi-tenant host. If the host is shared, omit the -`unauthenticatedLoopbackListener` command and do not run the hub's local integrations. +hub's provider credentials and account quota, and can exhaust the shared turn capacity that +authenticated remote clients depend on. Do not enable it on a shared or multi-tenant host. If the +host is shared, omit the `unauthenticatedLoopbackListener` command and do not run the hub's local +integrations. + +Binding to `127.0.0.1` means the kernel refuses remote connections, but it does not stop a browser: +a page you visit can make your browser connect to `127.0.0.1`. The listener therefore applies the +same `Host` and `Origin` checks as an ordinary loopback bind. ::: ```bash @@ -223,6 +228,9 @@ separate ports: ocx config set unauthenticatedLoopbackListener '{"enabled":true,"port":10104}' ``` +The ported form is the same unauthenticated surface: the dedicated-host warning above applies to +this command too. + With a `port` set, the local integrations follow the listener and write `http://127.0.0.1:10104` instead. The port must differ from the proxy port and is never OS-assigned: an ephemeral port would change across restarts while already-running app-servers kept the previous `base_url`. diff --git a/docs-site/src/content/docs/ko/guides/remote-hub.md b/docs-site/src/content/docs/ko/guides/remote-hub.md index edd5824e28f..5034af5d0ce 100644 --- a/docs-site/src/content/docs/ko/guides/remote-hub.md +++ b/docs-site/src/content/docs/ko/guides/remote-hub.md @@ -52,7 +52,9 @@ ocx sync 데이터 리스너는 허브의 Tailscale 주소에 바인드하고, 허브 자신의 프로세스가 같은 포트를 자격 증명 없이 쓸 수 있도록 루프백 companion을 켜고, 관리 평면은 따로 공개합니다. 아래 값은 예시입니다. :::danger[전용 단일 테넌트 호스트를 사용하세요] -루프백 companion은 인증이 없습니다. 이 머신의 모든 프로세스와 OS 사용자가 허브의 프로바이더 자격 증명과 계정 쿼터를 사용할 수 있고, 인증된 원격 클라이언트를 굶길 수 있습니다. 공유 또는 다중 테넌트 호스트에서는 활성화하지 마세요. 호스트가 공유라면 `unauthenticatedLoopbackListener` 명령을 생략하고 허브의 로컬 통합을 실행하지 마세요. +루프백 companion은 인증이 없습니다. 이 머신의 모든 프로세스와 OS 사용자가 허브의 프로바이더 자격 증명과 계정 쿼터를 사용할 수 있고, 인증된 원격 클라이언트가 의존하는 공유 턴 용량을 고갈시킬 수 있습니다. 공유 또는 다중 테넌트 호스트에서는 활성화하지 마세요. 호스트가 공유라면 `unauthenticatedLoopbackListener` 명령을 생략하고 허브의 로컬 통합을 실행하지 마세요. + +`127.0.0.1`에 바인드하면 커널이 원격 접속을 거부하지만 브라우저까지 막지는 못합니다. 방문한 페이지가 브라우저를 통해 `127.0.0.1`에 접속하게 할 수 있습니다. 그래서 리스너는 일반 루프백 바인드와 같은 `Host`/`Origin` 검사를 적용합니다. ::: ```bash @@ -119,6 +121,8 @@ companion 형태는 `hostname`이 루프백도 와일드카드도 아닌 구체 ocx config set unauthenticatedLoopbackListener '{"enabled":true,"port":10104}' ``` +포트 지정 형태도 같은 인증 없는 표면입니다. 위의 전용 호스트 경고가 이 명령에도 적용됩니다. + `port`를 지정하면 로컬 통합이 리스너를 따라 `http://127.0.0.1:10104`를 기록합니다. 이 포트는 프록시 포트와 달라야 하고 OS가 자동 할당하지 않습니다. 임시 포트는 재시작 때마다 바뀌는데 이미 실행 중인 app-server는 예전 `base_url`을 들고 있기 때문입니다. **이 필드를 바꾸면 프록시를 재시작하세요.** 소켓은 시작할 때 한 번 바인드되고 로컬 클라이언트 파일도 그때 결정된 값으로 기록되므로, 실행 중인 허브는 예전 답을 유지합니다. 포트 지정 허브에서는 이것이 `ocx claude`가 리스너에 닿는지 `404`를 받는지의 차이입니다. 백그라운드 서비스라면 명령은 항상 재시작하는 `ocx service restart`입니다. [macOS 서비스 운영](#macos-서비스-운영)을 보세요. `ocx restart`는 다른 명령입니다. 직접 띄운 프록시 프로세스를 재시작하며, 서비스 관리자가 감독하는 서비스를 다루지 않습니다. diff --git a/skills/ocx/references/05_remote_hub.md b/skills/ocx/references/05_remote_hub.md index 0d8042fe615..aaaf36a1e3d 100644 --- a/skills/ocx/references/05_remote_hub.md +++ b/skills/ocx/references/05_remote_hub.md @@ -26,6 +26,11 @@ has to learn a new port. Setting a `port` (`{ "enabled": true, "port": 10104 }`) works and puts the two surfaces on separate ports; local integrations then follow the listener's port. +This listener is unauthenticated: every process and OS user on the hub machine can spend +its provider credentials and quota, and can exhaust the shared turn capacity remote +clients depend on. Enable it only on a dedicated single-tenant host — on a shared or +multi-tenant host, omit `unauthenticatedLoopbackListener` entirely. + The port-less form is refused on a loopback or wildcard `hostname` — `127.0.0.1`, `localhost`, `0.0.0.0`, `::` — because the public listener already holds that loopback address. The refusal happens at write time and again at startup, naming the collision. On diff --git a/tests/ci-workflows/docs-remote-hub-claims.test.ts b/tests/ci-workflows/docs-remote-hub-claims.test.ts index a9e32d732c2..2f116e86b09 100644 --- a/tests/ci-workflows/docs-remote-hub-claims.test.ts +++ b/tests/ci-workflows/docs-remote-hub-claims.test.ts @@ -125,13 +125,23 @@ describe("the one-port hub recipe", () => { test("both locales warn that the companion requires a dedicated host", async () => { const warnings = [ - ["en", GUIDE, "every process and OS user", "shared or multi-tenant host"], - ["ko", KO_GUIDE, "모든 프로세스와 OS 사용자", "공유 또는 다중 테넌트 호스트에서는 활성화하지 마세요"], + ["en", GUIDE, "every process and OS user", "shared or multi-tenant host", "dedicated single-tenant host", "Do not enable"], + ["ko", KO_GUIDE, "모든 프로세스와 OS 사용자", "공유 또는 다중 테넌트 호스트에서는 활성화하지 마세요", "전용 단일 테넌트 호스트", "활성화하지 마세요"], ] as const; - for (const [locale, file, localAccess, sharedHost] of warnings) { + for (const [locale, file, localAccess, sharedHost, dedicated, doNotEnable] of warnings) { const source = await Bun.file(file).text(); expect(source, locale).toContain(localAccess); expect(source, locale).toContain(sharedHost); + expect(source, locale).toContain(dedicated); + expect(source, locale).toContain(doNotEnable); + // The warning must render as a danger box, not flow past as ordinary prose. + expect(source, locale).toContain(":::danger"); + // The same unauthenticated surface is offered again by the ported form; the warning + // must reach that command too, or a reader following only that section misses it. + const ported = source.indexOf('"port":10104'); + expect(ported, locale).toBeGreaterThan(-1); + const after = source.slice(ported, ported + 600); + expect(after, locale).toMatch(/unauthenticated|인증/); } }); From 390f12c0b8cd9d5f71585a9506a5056fdaafbdf8 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Fri, 18 Sep 2026 06:50:14 +0900 Subject: [PATCH 10/32] fix(oauth): drop legacy credential backup on destructive mutations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit auth.json.pre-multiauth copies the whole legacy store for downgrade recovery, but removeCredential/removeAccount left it behind — and a legacy-shaped store re-created it — so logout and account deletion kept a file holding the very refresh tokens the user destroyed. mutateStore gains a removeLegacyBackup option: it skips the one-time create and unlinks the backup after persist. Removal is best-effort (ENOENT ignored, other failures warn) because it runs after the store is persisted — a failed unlink must not report a failed logout for an account that is already gone. A stale uninstall-manifest entry is harmless: removeOwnedConfigState skips missing paths. Covers the destructive-migration, pre-existing-backup, and stale-backup-on-migrated-store cases in oauth-store-multi tests. (cherry picked from commit f456a4f2ee9683501d1c32308f6386c148e367dc) --- src/oauth/store.ts | 33 +++++++++++++++++---- tests/oauth/oauth-store-multi.test.ts | 41 +++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/oauth/store.ts b/src/oauth/store.ts index a7ea6eaa2df..c53b396fd9d 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -4,8 +4,10 @@ * Multiauth shape (260706): each provider value is a ProviderAccountSet * `{ activeAccountId, accounts: [{ id, credential, needsReauth?, addedAt? }] }`. * Legacy single-credential values (`{ access, refresh, expires, ... }`) normalize on load, - * and the first new-shape persist writes a one-time `auth.json.pre-multiauth` backup so a - * downgraded loader (which silently drops unknown shapes) cannot destroy refresh tokens. + * and the first non-destructive new-shape persist writes a one-time + * `auth.json.pre-multiauth` backup so a downgraded loader (which silently drops unknown + * shapes) cannot destroy refresh tokens. Destructive mutations remove that backup so + * logout and account deletion do not retain the deleted credentials. * * Exceptions: * - `chatgpt` stays single-slot (always replaced): codex-auth-api uses it as a scratch slot @@ -448,6 +450,24 @@ function backupLegacyOnce(): void { } catch { /* best-effort */ } } +/** + * Destructive mutations (logout, account deletion) also drop the downgrade backup: it + * holds a copy of the very credentials the user removed, so keeping it would retain + * tokens the user asked to destroy. Best-effort like the create path — the removal runs + * after persist, so a failed unlink must not report a failed logout for an account that + * is already gone. A stale uninstall-manifest entry is harmless: removeOwnedConfigState + * skips paths that no longer exist. + */ +function removeLegacyBackup(): void { + try { + unlinkSync(`${getAuthStorePath()}.pre-multiauth`); + } catch (error) { + if (errorCode(error) !== "ENOENT") { + console.warn(`[oauth] could not remove legacy credential backup: ${error instanceof Error ? error.message : String(error)}`); + } + } +} + function isCredentialSource(value: unknown): value is OAuthCredentialSource { return value === "oauth" || value === "local-cli" || value === "credential-file" || value === "environment" || value === "manual"; } @@ -712,9 +732,9 @@ function serializeMutation(work: () => Promise, retainedValues: readonly u drainOAuthMutations(); return result; } -export function mutateStore(fn:(store:AuthStore)=>T|Promise, retainedValues: readonly unknown[] = [], options?: { waitMs?: number; assertBeforePersist?: () => void }):Promise{return serializeMutation(async()=>{const guard=await createOAuthFileLock({path:getAuthStoreLockPath(),staleAfterMs:30000}).acquire();try{ +export function mutateStore(fn:(store:AuthStore)=>T|Promise, retainedValues: readonly unknown[] = [], options?: { waitMs?: number; assertBeforePersist?: () => void; removeLegacyBackup?: boolean }):Promise{return serializeMutation(async()=>{const guard=await createOAuthFileLock({path:getAuthStoreLockPath(),staleAfterMs:30000}).acquire();try{ const { store, hadLegacy } = loadAuthStoreInternal(); - if (hadLegacy) backupLegacyOnce(); + if (hadLegacy && !options?.removeLegacyBackup) backupLegacyOnce(); const selections = new Map(Object.entries(store).map(([provider, set]) => [provider, { set, accountId: set.activeAccountId, @@ -743,6 +763,7 @@ export function mutateStore(fn:(store:AuthStore)=>T|Promise, retainedValue } } persist(store); + if (options?.removeLegacyBackup) removeLegacyBackup(); for (const provider of changedProviders) publishAccountSelection(provider, "oauth"); return result; }finally{guard.release();}}, retainedValues, options?.waitMs); @@ -886,7 +907,7 @@ export async function removeCredential(provider: string): Promise<"removed" | "n } set.activeAccountId = set.accounts[0]!.id; return "removed" as const; - }, [provider]); + }, [provider], { removeLegacyBackup: true }); } // --------------------------------------------------------------------------- @@ -1029,7 +1050,7 @@ export async function removeAccount(provider: string, accountId: string): Promis } if (set.activeAccountId === accountId) set.activeAccountId = set.accounts[0]!.id; return true; - }, [provider, accountId]); + }, [provider, accountId], { removeLegacyBackup: true }); return removed; } diff --git a/tests/oauth/oauth-store-multi.test.ts b/tests/oauth/oauth-store-multi.test.ts index 32fcf293f84..6be31c1e323 100644 --- a/tests/oauth/oauth-store-multi.test.ts +++ b/tests/oauth/oauth-store-multi.test.ts @@ -213,6 +213,47 @@ describe("multi-account auth store", () => { } }); + test("logout migrates a legacy store without retaining its credential backup", async () => { + const authPath = join(TEST_DIR, "auth.json"); + mkdirSync(TEST_DIR, { recursive: true, mode: 0o700 }); + writeFileSync(authPath, JSON.stringify({ + xai: { access: "legacy-access", refresh: "legacy-refresh", expires: Date.now() + 1000 }, + })); + + expect(await removeCredential("xai")).toBe("removed"); + + expect(JSON.parse(readFileSync(authPath, "utf-8"))).toEqual({}); + expect(existsSync(`${authPath}.pre-multiauth`)).toBe(false); + }); + + test("account deletion removes an existing legacy credential backup", async () => { + const authPath = join(TEST_DIR, "auth.json"); + mkdirSync(TEST_DIR, { recursive: true, mode: 0o700 }); + const legacy = { + xai: { access: "legacy-access", refresh: "legacy-refresh", expires: Date.now() + 1000 }, + }; + writeFileSync(authPath, JSON.stringify(legacy)); + writeFileSync(`${authPath}.pre-multiauth`, JSON.stringify(legacy)); + const accountId = getAccountSet("xai")!.activeAccountId; + + expect(await removeAccount("xai", accountId)).toBe(true); + + expect(JSON.parse(readFileSync(authPath, "utf-8"))).toEqual({}); + expect(existsSync(`${authPath}.pre-multiauth`)).toBe(false); + }); + + test("logout on a migrated store still removes a stale credential backup", async () => { + const authPath = join(TEST_DIR, "auth.json"); + mkdirSync(TEST_DIR, { recursive: true, mode: 0o700 }); + await saveCredential("xai", cred({ email: "a@example.test" })); + // A backup left over from an earlier migration holds copies of removed credentials. + writeFileSync(`${authPath}.pre-multiauth`, JSON.stringify({ xai: { access: "stale", refresh: "stale", expires: 1 } })); + + expect(await removeCredential("xai")).toBe("removed"); + + expect(existsSync(`${authPath}.pre-multiauth`)).toBe(false); + }); + test("legacy credential WITHOUT identity gets a deterministic account id across loads", async () => { // Legacy stores are re-normalized on EVERY load without being persisted, so the // derived id must be stable: a time-salted id would make getAccountSet and From b05a544761d96e53fd73346cf4f09e43be74b515 Mon Sep 17 00:00:00 2001 From: luvs01 <27862058+luvs01@users.noreply.github.com> Date: Fri, 18 Sep 2026 06:52:04 +0900 Subject: [PATCH 11/32] docs(structure): note legacy backup removal on destructive auth mutations (cherry picked from commit b00133c7bd3c357b10e67461b1ed9cd67fafccae) --- structure/overview.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/structure/overview.md b/structure/overview.md index e75cf6430ee..a79ef27060a 100644 --- a/structure/overview.md +++ b/structure/overview.md @@ -74,7 +74,7 @@ opencodex state root does not undo those writes. Putting native Codex back is th | Path | Owner | Notes | | --- | --- | --- | | `~/.opencodex/config.json` | opencodex | Init creates via private temp plus no-replace hard link; dashboard and explicit updates use atomic replacement. | -| `~/.opencodex/auth.json` | opencodex | OAuth tokens; not committed. Multiauth shape: `provider -> { activeAccountId, accounts[] }` (legacy single-credential values normalize on load; a one-time `auth.json.pre-multiauth` backup guards downgrades). ChatGPT scratch OAuth stays separate from the Codex account store. For multi-slot providers, credentials without `accountId`/email replace the active slot on a normal login; an explicit add-account login preserves the prior slot and appends a distinct one. Single-slot providers such as ChatGPT remain replacement-only. | +| `~/.opencodex/auth.json` | opencodex | OAuth tokens; not committed. Multiauth shape: `provider -> { activeAccountId, accounts[] }` (legacy single-credential values normalize on load; a one-time `auth.json.pre-multiauth` backup guards downgrades and is removed by destructive mutations such as logout or account deletion). ChatGPT scratch OAuth stays separate from the Codex account store. For multi-slot providers, credentials without `accountId`/email replace the active slot on a normal login; an explicit add-account login preserves the prior slot and appends a distinct one. Single-slot providers such as ChatGPT remain replacement-only. | | `~/.opencodex/codex-accounts.json` | opencodex | Hardened main-plus-added credential store used by `openai` in Pool mode. | | `~/.opencodex/catalog-backup.json` | opencodex | One-time pristine Codex catalog backup for restore; per-catalog copies are hashed variants (see [`catalog.md`](catalog.md)). | | `~/.opencodex/usage.jsonl` | opencodex | Append-only request usage log (0o600); request metadata + token counts only, never prompts or auth. | From b8242f8463df3b941092418791fb8f0251c3ab67 Mon Sep 17 00:00:00 2001 From: lidge-jun Date: Fri, 18 Sep 2026 09:47:50 +0900 Subject: [PATCH 12/32] fix(oauth): only drop the downgrade backup when a credential was actually removed removeCredential and removeAccount passed removeLegacyBackup unconditionally, so a stale or concurrent request that returned "not-found" or false still unlinked auth.json.pre-multiauth and still skipped creating it for a legacy store. That backup is a whole-store copy, so a removal that deleted nothing destroyed downgrade recovery for every provider in it. Decide from the mutation result instead. The decision moves to just after the mutation body, which only edits the in-memory store, so nothing has touched disk when it is taken, and the create and remove paths stay mutually exclusive as before. Cover both no-op results. (cherry picked from commit 160d3ab0991ecd94ce09359af573b09664bb5b7a) --- src/oauth/store.ts | 18 +++++++++++++----- tests/oauth/oauth-store-multi.test.ts | 24 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/oauth/store.ts b/src/oauth/store.ts index c53b396fd9d..fb60041424e 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -732,9 +732,8 @@ function serializeMutation(work: () => Promise, retainedValues: readonly u drainOAuthMutations(); return result; } -export function mutateStore(fn:(store:AuthStore)=>T|Promise, retainedValues: readonly unknown[] = [], options?: { waitMs?: number; assertBeforePersist?: () => void; removeLegacyBackup?: boolean }):Promise{return serializeMutation(async()=>{const guard=await createOAuthFileLock({path:getAuthStoreLockPath(),staleAfterMs:30000}).acquire();try{ +export function mutateStore(fn:(store:AuthStore)=>T|Promise, retainedValues: readonly unknown[] = [], options?: { waitMs?: number; assertBeforePersist?: () => void; removeLegacyBackup?: boolean | ((result: T) => boolean) }):Promise{return serializeMutation(async()=>{const guard=await createOAuthFileLock({path:getAuthStoreLockPath(),staleAfterMs:30000}).acquire();try{ const { store, hadLegacy } = loadAuthStoreInternal(); - if (hadLegacy && !options?.removeLegacyBackup) backupLegacyOnce(); const selections = new Map(Object.entries(store).map(([provider, set]) => [provider, { set, accountId: set.activeAccountId, @@ -742,6 +741,15 @@ export function mutateStore(fn:(store:AuthStore)=>T|Promise, retainedValue accountIds: set.accounts.map(account => account.id), }])); const result = await fn(store); + // A destructive mutation that removed nothing must not drop the downgrade + // backup: the request was a no-op, so there is no deleted credential to + // stop retaining, and the backup is a whole-store copy for every provider. + // The decision needs the mutation's result, so it is taken here; `fn` only + // edits the in-memory store, and nothing has touched disk yet. + const dropLegacyBackup = typeof options?.removeLegacyBackup === "function" + ? options.removeLegacyBackup(result) + : options?.removeLegacyBackup === true; + if (hadLegacy && !dropLegacyBackup) backupLegacyOnce(); options?.assertBeforePersist?.(); const changedProviders: string[] = []; for (const provider of new Set([...selections.keys(), ...Object.keys(store)])) { @@ -763,7 +771,7 @@ export function mutateStore(fn:(store:AuthStore)=>T|Promise, retainedValue } } persist(store); - if (options?.removeLegacyBackup) removeLegacyBackup(); + if (dropLegacyBackup) removeLegacyBackup(); for (const provider of changedProviders) publishAccountSelection(provider, "oauth"); return result; }finally{guard.release();}}, retainedValues, options?.waitMs); @@ -907,7 +915,7 @@ export async function removeCredential(provider: string): Promise<"removed" | "n } set.activeAccountId = set.accounts[0]!.id; return "removed" as const; - }, [provider], { removeLegacyBackup: true }); + }, [provider], { removeLegacyBackup: result => result === "removed" }); } // --------------------------------------------------------------------------- @@ -1050,7 +1058,7 @@ export async function removeAccount(provider: string, accountId: string): Promis } if (set.activeAccountId === accountId) set.activeAccountId = set.accounts[0]!.id; return true; - }, [provider, accountId], { removeLegacyBackup: true }); + }, [provider, accountId], { removeLegacyBackup: removed => removed }); return removed; } diff --git a/tests/oauth/oauth-store-multi.test.ts b/tests/oauth/oauth-store-multi.test.ts index 6be31c1e323..a084d2320bc 100644 --- a/tests/oauth/oauth-store-multi.test.ts +++ b/tests/oauth/oauth-store-multi.test.ts @@ -254,6 +254,30 @@ describe("multi-account auth store", () => { expect(existsSync(`${authPath}.pre-multiauth`)).toBe(false); }); + test("a logout that removed nothing keeps the downgrade backup", async () => { + const authPath = join(TEST_DIR, "auth.json"); + mkdirSync(TEST_DIR, { recursive: true, mode: 0o700 }); + await saveCredential("xai", cred({ email: "a@example.test" })); + // The backup is a whole-store copy, so a no-op removal for one provider must + // not destroy downgrade recovery for every other provider in it. + writeFileSync(`${authPath}.pre-multiauth`, JSON.stringify({ xai: { access: "stale", refresh: "stale", expires: 1 } })); + + expect(await removeCredential("anthropic")).toBe("not-found"); + + expect(existsSync(`${authPath}.pre-multiauth`)).toBe(true); + }); + + test("an account deletion that matched nothing keeps the downgrade backup", async () => { + const authPath = join(TEST_DIR, "auth.json"); + mkdirSync(TEST_DIR, { recursive: true, mode: 0o700 }); + await saveCredential("xai", cred({ email: "a@example.test" })); + writeFileSync(`${authPath}.pre-multiauth`, JSON.stringify({ xai: { access: "stale", refresh: "stale", expires: 1 } })); + + expect(await removeAccount("xai", "no-such-account")).toBe(false); + + expect(existsSync(`${authPath}.pre-multiauth`)).toBe(true); + }); + test("legacy credential WITHOUT identity gets a deterministic account id across loads", async () => { // Legacy stores are re-normalized on EVERY load without being persisted, so the // derived id must be stable: a time-salted id would make getAccountSet and From 2853a8a9506afbb296e0527e4edcfda590f81c5d Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 05:15:35 +0900 Subject: [PATCH 13/32] fix(oauth): drop the downgrade backup when a provider is deleted Deleting a provider from the dashboard clears its credentials through replaceProviderAccountSet(name, null), which bypassed the carried logout/account-deletion rule and kept (or first created) the auth.json.pre-multiauth copy of the deleted tokens. Treat clearing a provider that had credentials as destructive; a no-op clear and a non-empty replacement keep the backup. Pin the warning path when the backup cannot be removed after the credential is already gone. Completes the reimplementation of #4949. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- src/oauth/store.ts | 13 +++++++--- structure/overview.md | 2 +- tests/oauth/oauth-store-multi.test.ts | 34 +++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/oauth/store.ts b/src/oauth/store.ts index fb60041424e..ded9b42db7c 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -1062,15 +1062,21 @@ export async function removeAccount(provider: string, accountId: string): Promis return removed; } -/** Replace or clear a provider account set (used for transactional Kiro add-account rollback). */ +/** + * Replace or clear a provider account set (provider deletion, transactional Kiro add-account + * rollback). Clearing a provider that had credentials is a destructive mutation, so it also drops + * the legacy downgrade backup, like logout and account deletion. A non-empty replacement keeps it: + * a future caller that removes accounts through a replacement needs its own decision here. + */ export async function replaceProviderAccountSet( provider: string, set: ProviderAccountSet | null, ): Promise { await mutateStore(store => { if (!set || set.accounts.length === 0) { + const cleared = store[provider] !== undefined; delete store[provider]; - return; + return cleared; } store[provider] = { activeAccountId: set.activeAccountId, @@ -1082,7 +1088,8 @@ export async function replaceProviderAccountSet( ...(account.addedAt !== undefined ? { addedAt: account.addedAt } : {}), })), }; - }, [provider, set]); + return false; + }, [provider, set], { removeLegacyBackup: cleared => cleared }); } export type ProviderCredentialRekeyOutcome = "moved" | "absent" | "conflict"; diff --git a/structure/overview.md b/structure/overview.md index a79ef27060a..10713c2e272 100644 --- a/structure/overview.md +++ b/structure/overview.md @@ -74,7 +74,7 @@ opencodex state root does not undo those writes. Putting native Codex back is th | Path | Owner | Notes | | --- | --- | --- | | `~/.opencodex/config.json` | opencodex | Init creates via private temp plus no-replace hard link; dashboard and explicit updates use atomic replacement. | -| `~/.opencodex/auth.json` | opencodex | OAuth tokens; not committed. Multiauth shape: `provider -> { activeAccountId, accounts[] }` (legacy single-credential values normalize on load; a one-time `auth.json.pre-multiauth` backup guards downgrades and is removed by destructive mutations such as logout or account deletion). ChatGPT scratch OAuth stays separate from the Codex account store. For multi-slot providers, credentials without `accountId`/email replace the active slot on a normal login; an explicit add-account login preserves the prior slot and appends a distinct one. Single-slot providers such as ChatGPT remain replacement-only. | +| `~/.opencodex/auth.json` | opencodex | OAuth tokens; not committed. Multiauth shape: `provider -> { activeAccountId, accounts[] }` (legacy single-credential values normalize on load; a one-time `auth.json.pre-multiauth` backup guards downgrades and is removed by destructive mutations such as logout, account deletion, or provider deletion; a failed removal is warned, not fatal). ChatGPT scratch OAuth stays separate from the Codex account store. For multi-slot providers, credentials without `accountId`/email replace the active slot on a normal login; an explicit add-account login preserves the prior slot and appends a distinct one. Single-slot providers such as ChatGPT remain replacement-only. | | `~/.opencodex/codex-accounts.json` | opencodex | Hardened main-plus-added credential store used by `openai` in Pool mode. | | `~/.opencodex/catalog-backup.json` | opencodex | One-time pristine Codex catalog backup for restore; per-catalog copies are hashed variants (see [`catalog.md`](catalog.md)). | | `~/.opencodex/usage.jsonl` | opencodex | Append-only request usage log (0o600); request metadata + token counts only, never prompts or auth. | diff --git a/tests/oauth/oauth-store-multi.test.ts b/tests/oauth/oauth-store-multi.test.ts index a084d2320bc..ad856c50579 100644 --- a/tests/oauth/oauth-store-multi.test.ts +++ b/tests/oauth/oauth-store-multi.test.ts @@ -278,6 +278,40 @@ describe("multi-account auth store", () => { expect(existsSync(`${authPath}.pre-multiauth`)).toBe(true); }); + test("provider deletion drops the downgrade backup; a no-op clear or a replacement keeps it", async () => { + const authPath = join(TEST_DIR, "auth.json"); + const backup = `${authPath}.pre-multiauth`; + mkdirSync(TEST_DIR, { recursive: true, mode: 0o700 }); + await saveCredential("xai", cred({ email: "a@example.test" })); + writeFileSync(backup, JSON.stringify({ xai: { access: "stale", refresh: "stale", expires: 1 } })); + + await replaceProviderAccountSet("anthropic", null); + expect(existsSync(backup)).toBe(true); + await replaceProviderAccountSet("xai", getAccountSet("xai")!); + expect(existsSync(backup)).toBe(true); + + // The management provider-delete route clears credentials this way. + await replaceProviderAccountSet("xai", null); + expect(getAccountSet("xai")).toBeUndefined(); + expect(existsSync(backup)).toBe(false); + }); + + test("a backup that cannot be removed warns without failing the completed logout", async () => { + const authPath = join(TEST_DIR, "auth.json"); + mkdirSync(TEST_DIR, { recursive: true, mode: 0o700 }); + await saveCredential("xai", cred({ email: "a@example.test" })); + // A directory in the backup's place makes unlink fail with a non-ENOENT code on every OS. + mkdirSync(`${authPath}.pre-multiauth`); + const warning = spyOn(console, "warn").mockImplementation(() => {}); + try { + expect(await removeCredential("xai")).toBe("removed"); + expect(getAccountSet("xai")).toBeUndefined(); + expect(warning.mock.calls.some(call => String(call[0]).includes("could not remove legacy credential backup"))).toBe(true); + } finally { + warning.mockRestore(); + } + }); + test("legacy credential WITHOUT identity gets a deterministic account id across loads", async () => { // Legacy stores are re-normalized on EVERY load without being persisted, so the // derived id must be stable: a time-salted id would make getAccountSet and From aba89d090f3f633ba328215c58ed98bc79fc0a6a Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 05:26:47 +0900 Subject: [PATCH 14/32] fix(crash-guard): log a new hidden throw site inside the benign fold window Review of the carried #5286 frame fix found the remaining gap: a benign teardown whose JSC hidden sourceURL/line/column names a different throw site was still folded silently for five minutes, so a distinct fault could vanish between summaries. Log the summary when that hidden throw site differs from the last one logged; repeats of the same site, and errors without one, still fold. Classification is unchanged. --- src/lib/crash-guard.ts | 28 +++++++++++++++++++++++++- tests/service/crash-guard.test.ts | 33 ++++++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/lib/crash-guard.ts b/src/lib/crash-guard.ts index 62bca7e0d0c..aece7ae9789 100644 --- a/src/lib/crash-guard.ts +++ b/src/lib/crash-guard.ts @@ -150,7 +150,9 @@ function safeStringify(value: unknown): string { let benignSuppressed = 0; let benignLastLoggedAt = 0; +let benignLastOrigin: string | undefined; const BENIGN_LOG_INTERVAL_MS = 5 * 60_000; +const MAX_BENIGN_ORIGIN_BYTES = 1024; /** * Bun raises an off-path `unhandledRejection: TypeError: null is not an object` (native-only stack) @@ -196,12 +198,26 @@ function hasJsSourceFrame(stack: string): boolean { }); } +/** The JSC hidden throw site (`sourceURL:line:col`), when the error carries one. */ +function hiddenThrowSite(err: unknown): string | undefined { + if (!err || typeof err !== "object") return undefined; + const e = err as Record; + if (typeof e.sourceURL !== "string" || !e.sourceURL) return undefined; + const site = `${e.sourceURL}:${String(e.line ?? e.originalLine ?? "")}:${String(e.column ?? e.originalColumn ?? "")}`; + return truncateRetainedUtf8(site, MAX_BENIGN_ORIGIN_BYTES); +} + function record(kind: string, err: unknown, promise?: unknown): void { if (kind === "unhandledRejection" && isBenignAbortTeardown(err)) { benignSuppressed++; const now = Date.now(); - if (now - benignLastLoggedAt < BENIGN_LOG_INTERVAL_MS) return; // fold repeats silently + // A throw site JSC recorded on hidden fields is new information when it differs from + // the last one logged, so it is written even inside the fold window; repeats still fold. + const origin = hiddenThrowSite(err); + const novelOrigin = origin !== undefined && origin !== benignLastOrigin; + if (!novelOrigin && now - benignLastLoggedAt < BENIGN_LOG_INTERVAL_MS) return; // fold repeats silently benignLastLoggedAt = now; + if (origin !== undefined) benignLastOrigin = origin; const summary = `\n[${new Date(now).toISOString()}] benign-abort-teardown x${benignSuppressed}` + ` (Bun fetch-body abort; proxy unaffected)${diagnose(err)}${diagnosePromise(promise)}${breadcrumb()}\n`; benignSuppressed = 0; @@ -321,6 +337,16 @@ export function resetCrashRingForTests(): void { fetchRingBytes = 0; } +export function recordCrashForTests(kind: string, err: unknown): void { + record(kind, err); +} + +export function resetBenignFoldForTests(): void { + benignSuppressed = 0; + benignLastLoggedAt = 0; + benignLastOrigin = undefined; +} + /** Render the recent fetch ring (pending first) for the crash breadcrumb. */ function recentFetches(): string { try { diff --git a/tests/service/crash-guard.test.ts b/tests/service/crash-guard.test.ts index 90074751d7b..a6a64ef30e6 100644 --- a/tests/service/crash-guard.test.ts +++ b/tests/service/crash-guard.test.ts @@ -1,7 +1,11 @@ import { describe, expect, test } from "bun:test"; -import { appendCrashTraceForTests, crashRingEntriesForTests, formatCrashEntry, installCrashGuards, isBenignAbortTeardown, resetCrashRingForTests } from "../../src/lib/crash-guard"; +import { mkdtempSync, readFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { appendCrashTraceForTests, crashRingEntriesForTests, formatCrashEntry, installCrashGuards, isBenignAbortTeardown, recordCrashForTests, resetBenignFoldForTests, resetCrashRingForTests } from "../../src/lib/crash-guard"; import { RETAINED_TRUNCATION_MARKER, retainedUtf8Bytes } from "../../src/lib/admission"; import { sidecarEnter } from "../../src/lib/sidecar-tracker"; +import { removeTreeWithRetry } from "../helpers/remove-tree"; describe("crash-guard diagnostics", () => { test("the 13th fetch trace evicts the oldest and 8 KiB values truncate on UTF-8 boundaries", () => { @@ -140,6 +144,33 @@ describe("benign abort-teardown classification", () => { expect(isBenignAbortTeardown(err)).toBe(true); }); + test("a new hidden throw site is logged inside the fold window; repeats still fold", () => { + const home = mkdtempSync(join(tmpdir(), "ocx-crash-guard-")); + const previousHome = process.env.OPENCODEX_HOME; + process.env.OPENCODEX_HOME = home; + resetBenignFoldForTests(); + const teardown = (site: { sourceURL: string; line?: number; column?: number }) => { + const err = new TypeError("null is not an object"); + err.stack = "TypeError: null is not an object\n at (native:1:11)"; + return Object.assign(err, site); + }; + try { + recordCrashForTests("unhandledRejection", teardown({ sourceURL: "/abs/src/a.ts", line: 1, column: 2 })); + recordCrashForTests("unhandledRejection", teardown({ sourceURL: "/abs/src/a.ts", line: 1, column: 2 })); + recordCrashForTests("unhandledRejection", teardown({ sourceURL: "/abs/src/b.ts", line: 3, column: 4 })); + recordCrashForTests("unhandledRejection", teardown({ sourceURL: "" })); + const log = readFileSync(join(home, "crash.log"), "utf8"); + expect(log.match(/benign-abort-teardown/g)).toHaveLength(2); + expect(log).toContain("origin: /abs/src/a.ts:1:2"); + expect(log).toContain("origin: /abs/src/b.ts:3:4"); + } finally { + resetBenignFoldForTests(); + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeTreeWithRetry(home); + } + }); + test("does NOT flag a different message or the (evaluating …) form", () => { const a = new TypeError("null is not an object (evaluating 'x.y')"); a.stack = "TypeError: ...\n at (native:1:11)"; From fa49beaf68f8593e41664f2561b560902f892566 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 05:27:26 +0900 Subject: [PATCH 15/32] fix(proxy): merge loopback into an inherited lowercase no_proxy Review of the carried #5430 fix: Bun's native fetch reads a non-empty lowercase no_proxy before NO_PROXY, so with HTTP_PROXY and an inherited no_proxy the loopback entries written to NO_PROXY never applied and local calls could still go through the proxy. Merge the same configured and loopback entries into an inherited no_proxy, keeping its own entries. The non-loopback SOCKS test now uses a local proxy that drops every connection and an IP-literal target, so it no longer waits on DNS. --- src/config/proxy-env.ts | 15 +++++++++--- structure/config.md | 3 ++- tests/server/proxy-env.test.ts | 42 ++++++++++++++++++++++++++-------- 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/src/config/proxy-env.ts b/src/config/proxy-env.ts index f096cd3885a..a603b502d6a 100644 --- a/src/config/proxy-env.ts +++ b/src/config/proxy-env.ts @@ -115,8 +115,7 @@ function ambientProxyStateExists(): boolean { return false; } -function mergeNoProxyEntries(configured: string[] = []): void { - const existing = process.env.NO_PROXY ?? process.env.no_proxy ?? ""; +function withNoProxyEntries(existing: string, configured: readonly string[]): string { const entries = existing.split(",").map(s => s.trim()).filter(Boolean); const seen = new Set(entries.map(entry => entry.toLowerCase())); for (const host of [...configured, "localhost", "127.0.0.1", "::1", "[::1]"]) { @@ -126,7 +125,17 @@ function mergeNoProxyEntries(configured: string[] = []): void { seen.add(key); } } - process.env.NO_PROXY = entries.join(","); + return entries.join(","); +} + +function mergeNoProxyEntries(configured: string[] = []): void { + process.env.NO_PROXY = withNoProxyEntries(process.env.NO_PROXY ?? process.env.no_proxy ?? "", configured); + // Bun's native fetch reads a non-empty lowercase no_proxy before NO_PROXY + // (src/codex/catalog/remote.ts), so an inherited one would shadow the entries above. + const inherited = process.env.no_proxy; + if (inherited !== undefined && inherited.trim() !== "") { + process.env.no_proxy = withNoProxyEntries(inherited, configured); + } } /** diff --git a/structure/config.md b/structure/config.md index ce35ef0a3fc..13c571a147b 100644 --- a/structure/config.md +++ b/structure/config.md @@ -543,7 +543,8 @@ configuration. An explicit SOCKS5 or SOCKS5h URL selects ALL_PROXY and removes stale scheme-proxy variables; HTTP(S) settings retain their existing environment precedence. Activation keeps the existing Windows auto-discovery path and loopback NO_PROXY entries; the no-configured-proxy return merges them only when the environment -already carries proxy state, leaving a proxy-free process untouched. When the +already carries proxy state, leaving a proxy-free process untouched. An inherited non-empty +lowercase `no_proxy`, which Bun fetch reads first, receives the same entries. When the environment no longer selects SOCKS, activation restores the native fetch; removing a saved field alone does not erase inherited process environment variables. diff --git a/tests/server/proxy-env.test.ts b/tests/server/proxy-env.test.ts index ca1016b16cf..91789394be4 100644 --- a/tests/server/proxy-env.test.ts +++ b/tests/server/proxy-env.test.ts @@ -1,7 +1,8 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { createServer } from "node:http"; +import { createServer as createTcpServer } from "node:net"; import { applyProxyEnv } from "../../src/config"; -import { configuredOutboundFetch, resolveProxyRoute, configureSocks5Fetch } from "../../src/lib/proxy-env"; +import { configuredOutboundFetch, noProxyMatches, resolveProxyRoute, configureSocks5Fetch } from "../../src/lib/proxy-env"; import type { OcxConfig } from "../../src/types"; const PROXY_ENV_KEYS = ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "all_proxy", "no_proxy", "OCX_TEST_PROXY_REF", "OCX_TEST_NO_PROXY_REF"] as const; @@ -243,17 +244,40 @@ describe("applyProxyEnv", () => { }); test.each(["ALL_PROXY", "all_proxy"])("inherited SOCKS %s still owns non-loopback fetches", async key => { - process.env[key] = "socks5://untrusted-proxy.invalid:1080"; + // A local proxy that drops every connection: the SOCKS handshake fails at once, with no + // DNS lookup for the proxy or the IP-literal target, on every runner. + const refusing = createTcpServer(socket => socket.destroy()); + await new Promise((resolve, reject) => { + refusing.once("error", reject); + refusing.listen(0, "127.0.0.1", resolve); + }); + const address = refusing.address(); + if (!address || typeof address === "string") throw new Error("proxy fixture did not bind a TCP port"); + process.env[key] = `socks5://127.0.0.1:${address.port}`; applyProxyEnv(configWithProxy()); // The bypass must be scoped to loopback only: a non-loopback URL still routes through - // the inherited SOCKS proxy, which fails here because the proxy is unreachable. The - // direct fallback must NOT be consulted — if it were, the bypass leaked. + // the inherited SOCKS proxy, which fails here. The direct fallback must NOT be + // consulted — if it were, the bypass leaked. let directCalls = 0; - await expect(configuredOutboundFetch("http://api.example.com/v1/chat/completions", undefined, async () => { - directCalls += 1; - return new Response("direct"); - })).rejects.toThrow(); - expect(directCalls).toBe(0); + try { + await expect(configuredOutboundFetch("http://203.0.113.10/v1/chat/completions", undefined, async () => { + directCalls += 1; + return new Response("direct"); + })).rejects.toThrow(); + expect(directCalls).toBe(0); + } finally { + await new Promise(resolve => refusing.close(() => resolve())); + } + }); + + test("an inherited lowercase no_proxy also receives the loopback entries", () => { + // Bun's native fetch consults a non-empty lowercase no_proxy before NO_PROXY. + process.env.HTTP_PROXY = "http://proxy.invalid:3128"; + process.env.no_proxy = "internal.example"; + applyProxyEnv(configWithProxy()); + expect(process.env.no_proxy).toBe("internal.example,localhost,127.0.0.1,::1,[::1]"); + const loopback = new URL("http://127.0.0.1:11434/v1/models"); + expect(noProxyMatches(loopback, { no_proxy: process.env.no_proxy })).toBe(true); }); test("merges configured comma-separated noProxy entries", () => { From c0319d5af3e7840c30601f998c42498d268cf0fa Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 05:28:57 +0900 Subject: [PATCH 16/32] fix(oauth): keep other providers in the downgrade backup on deletion Review of the carried #4949 change: deleting one provider removed the whole auth.json.pre-multiauth file, so a legacy store with several providers lost downgrade recovery for every provider the user kept (the migrated auth.json is unreadable to an older loader). Destructive mutations now remove only the affected provider's entry and delete the file once it is empty. A backup entry that is not a regular file is removed rather than followed, and the rewrite replaces the entry itself without resolving a symlink. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- src/oauth/store.ts | 79 ++++++++++++++++----------- structure/overview.md | 2 +- tests/oauth/oauth-store-multi.test.ts | 39 ++++++++++++- 3 files changed, 86 insertions(+), 34 deletions(-) diff --git a/src/oauth/store.ts b/src/oauth/store.ts index ded9b42db7c..1abfaa2e9f6 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -4,10 +4,11 @@ * Multiauth shape (260706): each provider value is a ProviderAccountSet * `{ activeAccountId, accounts: [{ id, credential, needsReauth?, addedAt? }] }`. * Legacy single-credential values (`{ access, refresh, expires, ... }`) normalize on load, - * and the first non-destructive new-shape persist writes a one-time - * `auth.json.pre-multiauth` backup so a downgraded loader (which silently drops unknown - * shapes) cannot destroy refresh tokens. Destructive mutations remove that backup so - * logout and account deletion do not retain the deleted credentials. + * and the first new-shape persist writes a one-time `auth.json.pre-multiauth` backup so a + * downgraded loader (which silently drops unknown shapes) cannot destroy refresh tokens. + * Destructive mutations (logout, account deletion, provider deletion) remove the affected + * provider's entry from that backup, and the file once nothing is left, so deleted + * credentials are not retained while other providers keep their downgrade recovery. * * Exceptions: * - `chatgpt` stays single-slot (always replaced): codex-auth-api uses it as a scratch slot @@ -19,9 +20,10 @@ * both append distinct identified accounts under multiauth. */ import { createHash, randomUUID } from "node:crypto"; -import { chmodSync, closeSync, copyFileSync, existsSync, fstatSync, mkdirSync, openSync, readFileSync, statSync, unlinkSync, writeFileSync } from "node:fs"; +import { chmodSync, closeSync, copyFileSync, existsSync, fstatSync, lstatSync, mkdirSync, openSync, readFileSync, statSync, unlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { getConfigDir, atomicWriteFile, backupInvalidConfig, hardenConfigDir, hardenExistingSecret, withConfigMutationLockSync } from "../config"; +import { atomicWriteFileNoFollow } from "../config/atomic-write"; import { assertNotRealHomeUnderTest } from "../lib/test-home-guard"; import { recordOwnedConfigPath } from "../lib/config-ownership"; import { MAX_PENDING_OAUTH_MUTATIONS } from "../lib/translator-budget"; @@ -451,23 +453,42 @@ function backupLegacyOnce(): void { } /** - * Destructive mutations (logout, account deletion) also drop the downgrade backup: it - * holds a copy of the very credentials the user removed, so keeping it would retain - * tokens the user asked to destroy. Best-effort like the create path — the removal runs - * after persist, so a failed unlink must not report a failed logout for an account that - * is already gone. A stale uninstall-manifest entry is harmless: removeOwnedConfigState - * skips paths that no longer exist. + * Destructive mutations (logout, account deletion, provider deletion) remove the affected + * providers from the downgrade backup: it holds a copy of the very credentials the user + * removed. Entries for other providers stay, so their downgrade recovery survives; the file + * goes once nothing is left. Account deletion drops the provider's whole legacy entry, since + * a refreshed token cannot be matched to the account it came from. A backup entry that is + * not a regular file (a symlink, a directory) is removed, never followed or rewritten, and + * the rewrite replaces the entry itself. Best-effort like the create path: this runs after + * persist, so a failure must not report a failed logout for an account that is already + * gone; it warns instead. A stale uninstall-manifest entry is harmless: + * removeOwnedConfigState skips paths that no longer exist. */ -function removeLegacyBackup(): void { +function scrubLegacyBackup(providers: readonly string[]): void { + const backup = `${getAuthStorePath()}.pre-multiauth`; try { - unlinkSync(`${getAuthStorePath()}.pre-multiauth`); + const remaining = readLegacyBackupEntries(backup); + for (const provider of providers) delete remaining[provider]; + if (Object.keys(remaining).length > 0) atomicWriteFileNoFollow(backup, `${JSON.stringify(remaining, null, 2)}\n`); + else unlinkSync(backup); } catch (error) { if (errorCode(error) !== "ENOENT") { - console.warn(`[oauth] could not remove legacy credential backup: ${error instanceof Error ? error.message : String(error)}`); + console.warn(`[oauth] could not remove deleted credentials from the legacy credential backup: ${error instanceof Error ? error.message : String(error)}`); } } } +/** Provider entries of the backup; empty (so the file is removed) when unreadable or not a regular file. */ +function readLegacyBackupEntries(backup: string): Record { + if (!lstatSync(backup).isFile()) return {}; + try { + const parsed: unknown = JSON.parse(readFileSync(backup, "utf-8")); + return parsed && typeof parsed === "object" && !Array.isArray(parsed) ? { ...(parsed as Record) } : {}; + } catch { + return {}; + } +} + function isCredentialSource(value: unknown): value is OAuthCredentialSource { return value === "oauth" || value === "local-cli" || value === "credential-file" || value === "environment" || value === "manual"; } @@ -732,8 +753,9 @@ function serializeMutation(work: () => Promise, retainedValues: readonly u drainOAuthMutations(); return result; } -export function mutateStore(fn:(store:AuthStore)=>T|Promise, retainedValues: readonly unknown[] = [], options?: { waitMs?: number; assertBeforePersist?: () => void; removeLegacyBackup?: boolean | ((result: T) => boolean) }):Promise{return serializeMutation(async()=>{const guard=await createOAuthFileLock({path:getAuthStoreLockPath(),staleAfterMs:30000}).acquire();try{ +export function mutateStore(fn:(store:AuthStore)=>T|Promise, retainedValues: readonly unknown[] = [], options?: { waitMs?: number; assertBeforePersist?: () => void; scrubLegacyBackup?: (result: T) => readonly string[] }):Promise{return serializeMutation(async()=>{const guard=await createOAuthFileLock({path:getAuthStoreLockPath(),staleAfterMs:30000}).acquire();try{ const { store, hadLegacy } = loadAuthStoreInternal(); + if (hadLegacy) backupLegacyOnce(); const selections = new Map(Object.entries(store).map(([provider, set]) => [provider, { set, accountId: set.activeAccountId, @@ -741,15 +763,9 @@ export function mutateStore(fn:(store:AuthStore)=>T|Promise, retainedValue accountIds: set.accounts.map(account => account.id), }])); const result = await fn(store); - // A destructive mutation that removed nothing must not drop the downgrade - // backup: the request was a no-op, so there is no deleted credential to - // stop retaining, and the backup is a whole-store copy for every provider. - // The decision needs the mutation's result, so it is taken here; `fn` only - // edits the in-memory store, and nothing has touched disk yet. - const dropLegacyBackup = typeof options?.removeLegacyBackup === "function" - ? options.removeLegacyBackup(result) - : options?.removeLegacyBackup === true; - if (hadLegacy && !dropLegacyBackup) backupLegacyOnce(); + // Only providers whose credentials this mutation actually removed leave the downgrade + // backup; a no-op removal names none. The result decides, so it is read here. + const scrubbedProviders = options?.scrubLegacyBackup?.(result) ?? []; options?.assertBeforePersist?.(); const changedProviders: string[] = []; for (const provider of new Set([...selections.keys(), ...Object.keys(store)])) { @@ -771,7 +787,7 @@ export function mutateStore(fn:(store:AuthStore)=>T|Promise, retainedValue } } persist(store); - if (dropLegacyBackup) removeLegacyBackup(); + if (scrubbedProviders.length > 0) scrubLegacyBackup(scrubbedProviders); for (const provider of changedProviders) publishAccountSelection(provider, "oauth"); return result; }finally{guard.release();}}, retainedValues, options?.waitMs); @@ -915,7 +931,7 @@ export async function removeCredential(provider: string): Promise<"removed" | "n } set.activeAccountId = set.accounts[0]!.id; return "removed" as const; - }, [provider], { removeLegacyBackup: result => result === "removed" }); + }, [provider], { scrubLegacyBackup: result => result === "removed" ? [provider] : [] }); } // --------------------------------------------------------------------------- @@ -1058,15 +1074,16 @@ export async function removeAccount(provider: string, accountId: string): Promis } if (set.activeAccountId === accountId) set.activeAccountId = set.accounts[0]!.id; return true; - }, [provider, accountId], { removeLegacyBackup: removed => removed }); + }, [provider, accountId], { scrubLegacyBackup: removed => removed ? [provider] : [] }); return removed; } /** * Replace or clear a provider account set (provider deletion, transactional Kiro add-account - * rollback). Clearing a provider that had credentials is a destructive mutation, so it also drops - * the legacy downgrade backup, like logout and account deletion. A non-empty replacement keeps it: - * a future caller that removes accounts through a replacement needs its own decision here. + * rollback). Clearing a provider that had credentials is a destructive mutation, so it also removes + * the provider from the legacy downgrade backup, like logout and account deletion. A non-empty + * replacement leaves the backup alone: a future caller that removes accounts through a + * replacement needs its own decision here. */ export async function replaceProviderAccountSet( provider: string, @@ -1089,7 +1106,7 @@ export async function replaceProviderAccountSet( })), }; return false; - }, [provider, set], { removeLegacyBackup: cleared => cleared }); + }, [provider, set], { scrubLegacyBackup: cleared => cleared ? [provider] : [] }); } export type ProviderCredentialRekeyOutcome = "moved" | "absent" | "conflict"; diff --git a/structure/overview.md b/structure/overview.md index 10713c2e272..29781557d3e 100644 --- a/structure/overview.md +++ b/structure/overview.md @@ -74,7 +74,7 @@ opencodex state root does not undo those writes. Putting native Codex back is th | Path | Owner | Notes | | --- | --- | --- | | `~/.opencodex/config.json` | opencodex | Init creates via private temp plus no-replace hard link; dashboard and explicit updates use atomic replacement. | -| `~/.opencodex/auth.json` | opencodex | OAuth tokens; not committed. Multiauth shape: `provider -> { activeAccountId, accounts[] }` (legacy single-credential values normalize on load; a one-time `auth.json.pre-multiauth` backup guards downgrades and is removed by destructive mutations such as logout, account deletion, or provider deletion; a failed removal is warned, not fatal). ChatGPT scratch OAuth stays separate from the Codex account store. For multi-slot providers, credentials without `accountId`/email replace the active slot on a normal login; an explicit add-account login preserves the prior slot and appends a distinct one. Single-slot providers such as ChatGPT remain replacement-only. | +| `~/.opencodex/auth.json` | opencodex | OAuth tokens; not committed. Multiauth shape: `provider -> { activeAccountId, accounts[] }` (legacy single-credential values normalize on load; a one-time `auth.json.pre-multiauth` backup guards downgrades; logout, account deletion, and provider deletion remove that provider from it and delete it once empty, and a failed update is warned, not fatal). ChatGPT scratch OAuth stays separate from the Codex account store. For multi-slot providers, credentials without `accountId`/email replace the active slot on a normal login; an explicit add-account login preserves the prior slot and appends a distinct one. Single-slot providers such as ChatGPT remain replacement-only. | | `~/.opencodex/codex-accounts.json` | opencodex | Hardened main-plus-added credential store used by `openai` in Pool mode. | | `~/.opencodex/catalog-backup.json` | opencodex | One-time pristine Codex catalog backup for restore; per-catalog copies are hashed variants (see [`catalog.md`](catalog.md)). | | `~/.opencodex/usage.jsonl` | opencodex | Append-only request usage log (0o600); request metadata + token counts only, never prompts or auth. | diff --git a/tests/oauth/oauth-store-multi.test.ts b/tests/oauth/oauth-store-multi.test.ts index ad856c50579..ef720faa233 100644 --- a/tests/oauth/oauth-store-multi.test.ts +++ b/tests/oauth/oauth-store-multi.test.ts @@ -1,6 +1,6 @@ import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { INTERNAL_DEADLINE_MS, STORE_BUDGET_MS } from "../helpers/test-budget"; -import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { existsSync, lstatSync, mkdirSync, readFileSync, symlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import * as atomicWrite from "../../src/config/atomic-write"; import * as oauthStore from "../../src/oauth/store"; @@ -306,12 +306,47 @@ describe("multi-account auth store", () => { try { expect(await removeCredential("xai")).toBe("removed"); expect(getAccountSet("xai")).toBeUndefined(); - expect(warning.mock.calls.some(call => String(call[0]).includes("could not remove legacy credential backup"))).toBe(true); + expect(warning.mock.calls.some(call => String(call[0]).includes("could not remove deleted credentials from the legacy credential backup"))).toBe(true); } finally { warning.mockRestore(); } }); + test("logout keeps other providers' downgrade recovery in a legacy backup", async () => { + const authPath = join(TEST_DIR, "auth.json"); + const backup = `${authPath}.pre-multiauth`; + mkdirSync(TEST_DIR, { recursive: true, mode: 0o700 }); + writeFileSync(authPath, JSON.stringify({ + xai: { access: "xai-access", refresh: "xai-refresh", expires: Date.now() + 1000 }, + anthropic: { access: "anthropic-access", refresh: "anthropic-refresh", expires: Date.now() + 1000 }, + })); + + expect(await removeCredential("xai")).toBe("removed"); + + // An older loader cannot read the migrated auth.json, so the backup must still hold the + // provider the user kept, and must no longer hold the one the user removed. + const kept = JSON.parse(readFileSync(backup, "utf-8")) as Record; + expect(Object.keys(kept)).toEqual(["anthropic"]); + expect(kept.anthropic?.refresh).toBe("anthropic-refresh"); + expect(readFileSync(backup, "utf-8")).not.toContain("xai-refresh"); + }); + + test.skipIf(process.platform === "win32")("a symlinked backup is removed without touching its target", async () => { + const authPath = join(TEST_DIR, "auth.json"); + const backup = `${authPath}.pre-multiauth`; + const target = join(TEST_DIR, "elsewhere.json"); + mkdirSync(TEST_DIR, { recursive: true, mode: 0o700 }); + await saveCredential("xai", cred({ email: "a@example.test" })); + const outside = JSON.stringify({ xai: { access: "x", refresh: "x", expires: 1 }, other: { access: "o", refresh: "o", expires: 1 } }); + writeFileSync(target, outside); + symlinkSync(target, backup); + + expect(await removeCredential("xai")).toBe("removed"); + + expect(() => lstatSync(backup)).toThrow(); + expect(readFileSync(target, "utf-8")).toBe(outside); + }); + test("legacy credential WITHOUT identity gets a deterministic account id across loads", async () => { // Legacy stores are re-normalized on EVERY load without being persisted, so the // derived id must be stable: a time-salted id would make getAccountSet and From c623e4477bc9d283567fa21f6d5ded7834ae8876 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 05:29:35 +0900 Subject: [PATCH 17/32] fix(oauth): check an explicit #state on a raw Command Code paste Review of the carried #5413 change found the direct CLI prompt path, which does not go through submitManualLoginCode, accepting a raw "key#state" paste whose state did not match: only URL- and query-shaped input was compared. Treat a raw paste with an explicit #state suffix as state-bearing, as the shared submit gate already does; a bare key still needs no state. The end-to-end submit test now observes the login promise from the start so an early failure cannot leave it unhandled. --- src/oauth/command-code.ts | 5 ++- structure/providers-and-adapters.md | 2 +- tests/providers/command-code-provider.test.ts | 38 +++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/oauth/command-code.ts b/src/oauth/command-code.ts index 4f321770256..c55f520db94 100644 --- a/src/oauth/command-code.ts +++ b/src/oauth/command-code.ts @@ -172,8 +172,9 @@ function parsePastedCommandCodeInput(input: string, expectedState: string): Comm if (!apiKey) return undefined; // A URL/query-shaped paste is an authorization response and must carry a matching state, // mirroring the shared OAuth callback flow; a stale or attacker-supplied URL from another - // session must not be accepted. Raw in-session keys are exempt (no state to compare). - if (parsed.kind !== "raw" && parsed.state !== expectedState) return undefined; + // session must not be accepted. A raw paste with an explicit #state suffix is state-bearing + // too, as in the shared submit gate; only a bare in-session key has no state to compare. + if ((parsed.kind !== "raw" || parsed.state !== undefined) && parsed.state !== expectedState) return undefined; return { apiKey, state: expectedState, userId: "", userName: "", keyName: "manual" }; } diff --git a/structure/providers-and-adapters.md b/structure/providers-and-adapters.md index fd601bc5868..8d8994d017c 100644 --- a/structure/providers-and-adapters.md +++ b/structure/providers-and-adapters.md @@ -15,7 +15,7 @@ only canonical Fable, Opus, or Sonnet labels after removing terminal controls; u | `src/providers/model-rename-fields.ts`, `src/providers/model-rename-migration.ts` | Classifies every provider config field for a declared model rename. Exact-model records, lists and nested request-pacing keys follow the replacement; an already saved replacement entry wins. Provider-wide settings and credential fields are not model identities. | | `src/providers/resolved-model-policy.ts`, `src/providers/resolved-model-policy-merge.ts` | Static provider/model policy resolution for the final upstream wire model, plus its pure clone/merge/URL/family helpers. The resolver detaches and freezes registry defaults, operator overrides, exact explicit input-modality declarations, hard wire pins, aliases, and explicit false/empty values with field-level provenance. Provider derivation, routing, catalog hints, gather admission, and adapter selection consume its detached frozen result. Callers supply transport match, the exact capability row, and a credential-free effective auth decision; credential bytes, usability evidence, account/quota/health state, and observed limits remain outside the result. | -| `src/oauth/` | OAuth providers, token storage, refresh, and auth-token resolution. The login callback listener binds a per-provider FIXED loopback port, so consecutive logins reuse the same number; every response it sends ends its connection (`Connection: close`, including non-callback paths such as a stray `/favicon.ico` 404). Stopping the listener does not close an established socket, so without that a pooled client would deliver the next login's callback to the retired flow, which rejects the unknown state as a CSRF mismatch while the live flow waits. Command Code manual callback JSON remains opaque to the shared `code#state` parser and is state-validated by its provider parser. Kiro add-account identity prefers same-session `whoami` over a leftover SQLite state profile, and never persists the Builder ID service profile ARN as `accountId`. | +| `src/oauth/` | OAuth providers, token storage, refresh, and auth-token resolution. The login callback listener binds a per-provider FIXED loopback port, so consecutive logins reuse the same number; every response it sends ends its connection (`Connection: close`, including non-callback paths such as a stray `/favicon.ico` 404). Stopping the listener does not close an established socket, so without that a pooled client would deliver the next login's callback to the retired flow, which rejects the unknown state as a CSRF mismatch while the live flow waits. Command Code manual callback JSON remains opaque to the shared `code#state` parser and is state-validated by its provider parser. A raw Command Code paste with an explicit `#state` suffix must match the flow state on the direct prompt as well. Kiro add-account identity prefers same-session `whoami` over a leftover SQLite state profile, and never persists the Builder ID service profile ARN as `accountId`. | | `src/combos/request.ts` | Clones each selected combo target request and applies the existing target capability ladder: adaptive unknown targets and explicit empty ladders receive no unsupported reasoning/thinking controls, while known ladders retain per-target resolution. | | `src/adapters/openai-responses.ts` | Native OpenAI/ChatGPT Responses passthrough. | | `src/responses/muse-tool-name-alias.ts` | Host-gated Meta Muse 64-char tool-name alias/restore used by the Responses passthrough. | diff --git a/tests/providers/command-code-provider.test.ts b/tests/providers/command-code-provider.test.ts index 2c962f95cb5..337bd557742 100644 --- a/tests/providers/command-code-provider.test.ts +++ b/tests/providers/command-code-provider.test.ts @@ -315,6 +315,7 @@ describe("Command Code provider", () => { }) as typeof globalThis.fetch; loginState.set("command-code", { done: false }); const prompts: string[] = []; + let settled: Promise = Promise.resolve(); const promptCount = async (count: number) => { for (let i = 0; prompts.length < count && i < 400; i++) await Bun.sleep(5); expect(prompts.length).toBeGreaterThanOrEqual(count); @@ -336,6 +337,9 @@ describe("Command Code provider", () => { }, signal: controller.signal, }, { importLocal: "off" }); + // Observe the login from the start so an early assertion failure cannot leave its + // rejection unhandled once finally aborts it. + settled = login.then(() => undefined, () => undefined); await promptCount(1); const state = prompts[0]!; @@ -350,12 +354,46 @@ describe("Command Code provider", () => { expect(whoamiKeys).toEqual(["Bearer sk-key#segment"]); } finally { controller.abort(new Error("test complete")); + await settled; globalThis.fetch = originalFetch; loginState.delete("command-code"); clearManualCodeSlot("command-code"); } }); + test("the direct prompt rejects a raw key whose #state suffix does not match", async () => { + const controller = new AbortController(); + const originalFetch = globalThis.fetch; + let whoamiCalls = 0; + globalThis.fetch = (async (input: string | URL | Request) => { + const href = String(input); + if (href.includes("whoami")) { + whoamiCalls += 1; + return new Response(JSON.stringify({ ok: true, user: { id: "u-1", userName: "tester" } }), { status: 200 }); + } + throw new Error(`unexpected fetch: ${href}`); + }) as typeof globalThis.fetch; + let prompts = 0; + try { + const login = loginCommandCode({ + onAuth: () => {}, + onProgress: () => {}, + onManualCodeInput: async state => { + prompts += 1; + if (prompts === 1) return `sk-direct#${state}-other`; + controller.abort(new Error("cancelled after re-prompt")); + return undefined; + }, + signal: controller.signal, + }, { importLocal: "off" }); + await expect(login).rejects.toThrow("cancelled after re-prompt"); + expect(prompts).toBe(2); + expect(whoamiCalls).toBe(0); + } finally { + globalThis.fetch = originalFetch; + } + }); + test("uses live account discovery and only imports local CLI auth for the first account", () => { const request = buildModelsRequest(provider, "secret-command-key", "command-code"); expect(request).toEqual({ From 373d67b23fe104efc49085feb381140b780946d5 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 05:30:10 +0900 Subject: [PATCH 18/32] test: tighten the carried Kiro estimate and remote-hub warning checks Review nits on the carried #5248 and #5444 units: document that the count-based estimator takes non-negative integer counts, pin the pure-CJK and empty-model-id cases against the replacement-string formula, and require the dedicated-host warning to sit inside the danger callout in both locales rather than anywhere in the page. --- src/lib/token-estimate.ts | 5 ++++- tests/ci-workflows/docs-remote-hub-claims.test.ts | 10 ++++++++-- tests/providers/kiro/kiro-wire-estimate.test.ts | 13 +++++++++++++ 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/lib/token-estimate.ts b/src/lib/token-estimate.ts index 0a9e4a74401..cd3872936cd 100644 --- a/src/lib/token-estimate.ts +++ b/src/lib/token-estimate.ts @@ -146,7 +146,10 @@ export function estimateTokens(text: string, modelId?: string, contextWindow?: n return estimateTokensFromCharacterCounts(len - cjk, cjk, modelId, contextWindow); } -/** Estimate tokens from already-counted script buckets without materializing replacement text. */ +/** + * Estimate tokens from already-counted script buckets without materializing replacement text. + * `latin` and `cjk` are non-negative integer character counts, as produced from a string. + */ export function estimateTokensFromCharacterCounts( latin: number, cjk: number, diff --git a/tests/ci-workflows/docs-remote-hub-claims.test.ts b/tests/ci-workflows/docs-remote-hub-claims.test.ts index 2f116e86b09..65636d59e1e 100644 --- a/tests/ci-workflows/docs-remote-hub-claims.test.ts +++ b/tests/ci-workflows/docs-remote-hub-claims.test.ts @@ -134,8 +134,14 @@ describe("the one-port hub recipe", () => { expect(source, locale).toContain(sharedHost); expect(source, locale).toContain(dedicated); expect(source, locale).toContain(doNotEnable); - // The warning must render as a danger box, not flow past as ordinary prose. - expect(source, locale).toContain(":::danger"); + // The warning must render inside the danger box, not flow past as ordinary prose. + const opened = source.indexOf(":::danger"); + expect(opened, locale).toBeGreaterThan(-1); + const closing = /\r?\n:::\r?\n/.exec(source.slice(opened)); + expect(closing, locale).not.toBeNull(); + const callout = source.slice(opened, opened + (closing?.index ?? 0)); + expect(callout, locale).toContain(dedicated); + expect(callout, locale).toContain(sharedHost); // The same unauthenticated surface is offered again by the ported form; the warning // must reach that command too, or a reader following only that section misses it. const ported = source.indexOf('"port":10104'); diff --git a/tests/providers/kiro/kiro-wire-estimate.test.ts b/tests/providers/kiro/kiro-wire-estimate.test.ts index 5e905d728b4..cb660bab189 100644 --- a/tests/providers/kiro/kiro-wire-estimate.test.ts +++ b/tests/providers/kiro/kiro-wire-estimate.test.ts @@ -27,4 +27,17 @@ describe("kiro wire token estimate", () => { test("empty text estimates to zero", () => { expect(estimateKiroWireTokens("", model)).toBe(0); }); + + test("pure-CJK text and an empty model id keep the replacement-string results", () => { + const korean = "요청을 보내고 응답을 파싱한다".repeat(30); + const cjk = kiroCjkCount(korean); + const latin = korean.length - cjk; + const expectedFor = (prefixed: string) => Math.ceil( + estimateTokens("x".repeat(latin), prefixed) * KIRO_LATIN_WIRE_EXPANSION + + estimateTokens("\uac00".repeat(cjk), prefixed), + ); + expect(estimateKiroWireTokens(korean, model)).toBe(expectedFor(`kiro/${model}`)); + // The old path fell back to the bare "kiro" id for an empty model; both select the Kiro ratio. + expect(estimateKiroWireTokens(korean, "")).toBe(expectedFor("kiro")); + }); }); From c86c1a331d4f4fc9027c1f27c9dd3baf24a7e32d Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 05:34:33 +0900 Subject: [PATCH 19/32] fix(crash-guard): keep the hidden throw-site read inside the handler Re-review found that reading the hidden JSC fields for the benign fold could throw from an unusual accessor before the rejection was logged, escaping the process unhandledRejection listener. Make the read best-effort, as diagnose() already is, and pin it with a Proxy whose sourceURL getter throws. --- src/lib/crash-guard.ts | 19 ++++++++++----- tests/service/crash-guard.test.ts | 40 ++++++++++++++++++++++++------- 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/src/lib/crash-guard.ts b/src/lib/crash-guard.ts index aece7ae9789..ad9188857b3 100644 --- a/src/lib/crash-guard.ts +++ b/src/lib/crash-guard.ts @@ -198,13 +198,20 @@ function hasJsSourceFrame(stack: string): boolean { }); } -/** The JSC hidden throw site (`sourceURL:line:col`), when the error carries one. */ +/** + * The JSC hidden throw site (`sourceURL:line:col`), when the error carries one. Best-effort: + * this runs inside the process crash handler, so an accessor that throws yields no site. + */ function hiddenThrowSite(err: unknown): string | undefined { - if (!err || typeof err !== "object") return undefined; - const e = err as Record; - if (typeof e.sourceURL !== "string" || !e.sourceURL) return undefined; - const site = `${e.sourceURL}:${String(e.line ?? e.originalLine ?? "")}:${String(e.column ?? e.originalColumn ?? "")}`; - return truncateRetainedUtf8(site, MAX_BENIGN_ORIGIN_BYTES); + try { + if (!err || typeof err !== "object") return undefined; + const e = err as Record; + if (typeof e.sourceURL !== "string" || !e.sourceURL) return undefined; + const site = `${e.sourceURL}:${String(e.line ?? e.originalLine ?? "")}:${String(e.column ?? e.originalColumn ?? "")}`; + return truncateRetainedUtf8(site, MAX_BENIGN_ORIGIN_BYTES); + } catch { + return undefined; + } } function record(kind: string, err: unknown, promise?: unknown): void { diff --git a/tests/service/crash-guard.test.ts b/tests/service/crash-guard.test.ts index a6a64ef30e6..2ed66d1fd65 100644 --- a/tests/service/crash-guard.test.ts +++ b/tests/service/crash-guard.test.ts @@ -144,31 +144,53 @@ describe("benign abort-teardown classification", () => { expect(isBenignAbortTeardown(err)).toBe(true); }); - test("a new hidden throw site is logged inside the fold window; repeats still fold", () => { + const withCrashHome = (run: (crashLog: () => string) => void) => { const home = mkdtempSync(join(tmpdir(), "ocx-crash-guard-")); const previousHome = process.env.OPENCODEX_HOME; process.env.OPENCODEX_HOME = home; resetBenignFoldForTests(); + try { + run(() => readFileSync(join(home, "crash.log"), "utf8")); + } finally { + resetBenignFoldForTests(); + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + removeTreeWithRetry(home); + } + }; + + test("a new hidden throw site is logged inside the fold window; repeats still fold", () => { const teardown = (site: { sourceURL: string; line?: number; column?: number }) => { const err = new TypeError("null is not an object"); err.stack = "TypeError: null is not an object\n at (native:1:11)"; return Object.assign(err, site); }; - try { + withCrashHome(crashLog => { recordCrashForTests("unhandledRejection", teardown({ sourceURL: "/abs/src/a.ts", line: 1, column: 2 })); recordCrashForTests("unhandledRejection", teardown({ sourceURL: "/abs/src/a.ts", line: 1, column: 2 })); recordCrashForTests("unhandledRejection", teardown({ sourceURL: "/abs/src/b.ts", line: 3, column: 4 })); recordCrashForTests("unhandledRejection", teardown({ sourceURL: "" })); - const log = readFileSync(join(home, "crash.log"), "utf8"); + const log = crashLog(); expect(log.match(/benign-abort-teardown/g)).toHaveLength(2); expect(log).toContain("origin: /abs/src/a.ts:1:2"); expect(log).toContain("origin: /abs/src/b.ts:3:4"); - } finally { - resetBenignFoldForTests(); - if (previousHome === undefined) delete process.env.OPENCODEX_HOME; - else process.env.OPENCODEX_HOME = previousHome; - removeTreeWithRetry(home); - } + }); + }); + + test("a throwing hidden-field accessor cannot escape the crash handler", () => { + const err = new TypeError("null is not an object"); + err.stack = "TypeError: null is not an object\n at (native:1:11)"; + // A Proxy keeps instanceof TypeError while making the hidden field read throw. + const hostile = new Proxy(err, { + get(target, key) { + if (key === "sourceURL") throw new Error("accessor failure fixture"); + return Reflect.get(target, key, target); + }, + }); + withCrashHome(crashLog => { + expect(() => recordCrashForTests("unhandledRejection", hostile)).not.toThrow(); + expect(crashLog()).toContain("benign-abort-teardown"); + }); }); test("does NOT flag a different message or the (evaluating …) form", () => { From 8c5f4b1b16dc8a8abc360d47bf1bff12a317ea4b Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 05:35:04 +0900 Subject: [PATCH 20/32] fix(oauth): never write the downgrade backup through a link or claim it Re-review of the backup scrub found two gaps. backupLegacyOnce used existsSync, which reports a dangling symlink as absent, and then copied through it, so credentials could land outside the config directory; it now treats any existing entry as occupied and copies exclusively. The scrub rewrite went through the shared atomic writer, which records the path for uninstall, so a backup this install never registered was claimed and later deleted; it now replaces the entry through an exclusive private temp and a rename, leaving ownership unchanged. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- src/oauth/store.ts | 41 +++++++++++++++++++++++---- tests/oauth/oauth-store-multi.test.ts | 40 ++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/src/oauth/store.ts b/src/oauth/store.ts index 1abfaa2e9f6..3903f58a3d0 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -20,10 +20,9 @@ * both append distinct identified accounts under multiauth. */ import { createHash, randomUUID } from "node:crypto"; -import { chmodSync, closeSync, copyFileSync, existsSync, fstatSync, lstatSync, mkdirSync, openSync, readFileSync, statSync, unlinkSync, writeFileSync } from "node:fs"; +import { chmodSync, closeSync, constants as fsConstants, copyFileSync, existsSync, fstatSync, lstatSync, mkdirSync, openSync, readFileSync, renameSync, statSync, unlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { getConfigDir, atomicWriteFile, backupInvalidConfig, hardenConfigDir, hardenExistingSecret, withConfigMutationLockSync } from "../config"; -import { atomicWriteFileNoFollow } from "../config/atomic-write"; import { assertNotRealHomeUnderTest } from "../lib/test-home-guard"; import { recordOwnedConfigPath } from "../lib/config-ownership"; import { MAX_PENDING_OAUTH_MUTATIONS } from "../lib/translator-budget"; @@ -437,9 +436,11 @@ export function createOAuthRefreshIntentLock(provider:string,accountId:string,ov function backupLegacyOnce(): void { const path = getAuthStorePath(); const backup = `${path}.pre-multiauth`; - if (!existsSync(path) || existsSync(backup)) return; + // Any existing entry, including a dangling symlink existsSync would call absent, is left + // alone, and the copy is exclusive, so credentials are never written through a link. + if (!existsSync(path) || directoryEntryExists(backup)) return; try { - copyFileSync(path, backup); + copyFileSync(path, backup, fsConstants.COPYFILE_EXCL); try { chmodSync(backup, 0o600); } catch { /* best-effort */ } try { // Register only the copy we just created. An unowned home still needs downgrade recovery. @@ -469,7 +470,7 @@ function scrubLegacyBackup(providers: readonly string[]): void { try { const remaining = readLegacyBackupEntries(backup); for (const provider of providers) delete remaining[provider]; - if (Object.keys(remaining).length > 0) atomicWriteFileNoFollow(backup, `${JSON.stringify(remaining, null, 2)}\n`); + if (Object.keys(remaining).length > 0) replaceBackupEntry(backup, `${JSON.stringify(remaining, null, 2)}\n`); else unlinkSync(backup); } catch (error) { if (errorCode(error) !== "ENOENT") { @@ -489,6 +490,36 @@ function readLegacyBackupEntries(backup: string): Record { } } +/** + * Replace the backup entry itself through an exclusive private temp and a rename: an entry + * at the path is replaced, never followed, and uninstall ownership is left exactly as it was + * (the shared atomic writer would claim a backup this install never registered). + */ +function replaceBackupEntry(backup: string, content: string): void { + const temp = `${backup}.${process.pid}.${randomUUID()}.tmp`; + const fd = openSync(temp, "wx", 0o600); + try { + writeFileSync(fd, content); + } finally { + closeSync(fd); + } + try { + renameSync(temp, backup); + } catch (error) { + try { unlinkSync(temp); } catch { /* best-effort */ } + throw error; + } +} + +function directoryEntryExists(path: string): boolean { + try { + lstatSync(path); + return true; + } catch (error) { + return errorCode(error) !== "ENOENT"; + } +} + function isCredentialSource(value: unknown): value is OAuthCredentialSource { return value === "oauth" || value === "local-cli" || value === "credential-file" || value === "environment" || value === "manual"; } diff --git a/tests/oauth/oauth-store-multi.test.ts b/tests/oauth/oauth-store-multi.test.ts index ef720faa233..31b0baef1e2 100644 --- a/tests/oauth/oauth-store-multi.test.ts +++ b/tests/oauth/oauth-store-multi.test.ts @@ -347,6 +347,46 @@ describe("multi-account auth store", () => { expect(readFileSync(target, "utf-8")).toBe(outside); }); + test.skipIf(process.platform === "win32")("a dangling backup link is never written through", async () => { + const authPath = join(TEST_DIR, "auth.json"); + const backup = `${authPath}.pre-multiauth`; + const missing = join(TEST_DIR, "not-yet-created.json"); + mkdirSync(TEST_DIR, { recursive: true, mode: 0o700 }); + writeFileSync(authPath, JSON.stringify({ + xai: { access: "legacy-access", refresh: "legacy-refresh", expires: Date.now() + 1000 }, + })); + symlinkSync(missing, backup); + + expect(await removeCredential("xai")).toBe("removed"); + + expect(existsSync(missing)).toBe(false); + expect(() => lstatSync(backup)).toThrow(); + }); + + test("scrubbing a backup this install never registered leaves it unclaimed", async () => { + const dir = join(TEST_DIR, "unclaimed-backup"); + const path = join(dir, "auth.json"); + const backup = `${path}.pre-multiauth`; + process.env.OPENCODEX_HOME = dir; + try { + expect(recordOwnedConfigPath(dir, path)).toBe(true); + await saveCredential("xai", cred({ email: "a@example.test" })); + writeFileSync(backup, JSON.stringify({ + xai: { access: "stale", refresh: "stale", expires: 1 }, + anthropic: { access: "kept", refresh: "kept", expires: 1 }, + })); + + expect(await removeCredential("xai")).toBe("removed"); + expect(Object.keys(JSON.parse(readFileSync(backup, "utf8")))).toEqual(["anthropic"]); + + await flushConfigDirHardeningForTests(); + removeOwnedConfigState(dir); + expect(readFileSync(backup, "utf8")).toContain("kept"); + } finally { + process.env.OPENCODEX_HOME = TEST_DIR; + } + }); + test("legacy credential WITHOUT identity gets a deterministic account id across loads", async () => { // Legacy stores are re-normalized on EVERY load without being persisted, so the // derived id must be stable: a time-salted id would make getAccountSet and From d3c3cb9b388cd795ec384cfc8c57de0f44a6824a Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 05:35:04 +0900 Subject: [PATCH 21/32] test(kiro): make the pure-CJK estimate case actually zero-Latin --- tests/providers/kiro/kiro-wire-estimate.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/providers/kiro/kiro-wire-estimate.test.ts b/tests/providers/kiro/kiro-wire-estimate.test.ts index cb660bab189..4d0c8b4d596 100644 --- a/tests/providers/kiro/kiro-wire-estimate.test.ts +++ b/tests/providers/kiro/kiro-wire-estimate.test.ts @@ -29,9 +29,10 @@ describe("kiro wire token estimate", () => { }); test("pure-CJK text and an empty model id keep the replacement-string results", () => { - const korean = "요청을 보내고 응답을 파싱한다".repeat(30); + const korean = "요청을보내고응답을파싱한다".repeat(30); const cjk = kiroCjkCount(korean); const latin = korean.length - cjk; + expect(latin).toBe(0); const expectedFor = (prefixed: string) => Math.ceil( estimateTokens("x".repeat(latin), prefixed) * KIRO_LATIN_WIRE_EXPANSION + estimateTokens("\uac00".repeat(cjk), prefixed), From 748e7a846302970dd37fff49672871bc2734cc2c Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 05:38:16 +0900 Subject: [PATCH 22/32] fix(oauth): rewrite the downgrade backup through the shared writer Re-review: the local temp-and-rename used for the backup scrub skipped what the shared atomic writer guarantees (Windows ACL hardening before the temp holds a byte, an explicit 0600 on POSIX regardless of umask, scrub-and-unlink of a failed temp, the Windows rename retry), which structure/config.md forbids replacing. Add a no-follow variant of that writer that leaves the owner manifest untouched, and use it, so a backup an earlier install left unregistered still stays unclaimed. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- src/config/atomic-write.ts | 13 ++++++++++++- src/oauth/store.ts | 26 +++----------------------- structure/config.md | 4 +++- 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/src/config/atomic-write.ts b/src/config/atomic-write.ts index 2bdb9508e2c..b4b5d089cd2 100644 --- a/src/config/atomic-write.ts +++ b/src/config/atomic-write.ts @@ -277,8 +277,9 @@ function atomicWriteFileToTarget( target: string, io?: AtomicWriteIO, hooks: AtomicWriteHooks = {}, + recordOwnership = true, ): void { - recordOwnedConfigPath(getConfigDir(), path); + if (recordOwnership) recordOwnedConfigPath(getConfigDir(), path); assertResolvedTargetAllowed(path, target); const tmp = `${target}.ocx.${process.pid}.${nextAtomicTempSequence()}.tmp`; let hardened = false; @@ -399,6 +400,16 @@ export function atomicWriteFileNoFollow( atomicWriteFileToTarget(path, content, join(resolveWriteTarget(dirname(path)), basename(path)), io, hooks); } +/** + * The no-follow replacement above, for rewriting a file whose uninstall ownership must stay as + * it was: it does not record the path in the owner manifest. Used to rewrite the OAuth downgrade + * backup, which a pre-registration install may have left deliberately unclaimed; claiming it + * here would let a later uninstall delete recovery data it never owned. + */ +export function atomicWriteFileNoFollowUnclaimed(path: string, content: string): void { + atomicWriteFileToTarget(path, content, join(resolveWriteTarget(dirname(path)), basename(path)), undefined, {}, false); +} + export interface AtomicWriteAsyncIO { write: (path: string, content: string) => void | Promise; harden: (path: string) => void | Promise; diff --git a/src/oauth/store.ts b/src/oauth/store.ts index 3903f58a3d0..2d2507055aa 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -20,9 +20,10 @@ * both append distinct identified accounts under multiauth. */ import { createHash, randomUUID } from "node:crypto"; -import { chmodSync, closeSync, constants as fsConstants, copyFileSync, existsSync, fstatSync, lstatSync, mkdirSync, openSync, readFileSync, renameSync, statSync, unlinkSync, writeFileSync } from "node:fs"; +import { chmodSync, closeSync, constants as fsConstants, copyFileSync, existsSync, fstatSync, lstatSync, mkdirSync, openSync, readFileSync, statSync, unlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { getConfigDir, atomicWriteFile, backupInvalidConfig, hardenConfigDir, hardenExistingSecret, withConfigMutationLockSync } from "../config"; +import { atomicWriteFileNoFollowUnclaimed } from "../config/atomic-write"; import { assertNotRealHomeUnderTest } from "../lib/test-home-guard"; import { recordOwnedConfigPath } from "../lib/config-ownership"; import { MAX_PENDING_OAUTH_MUTATIONS } from "../lib/translator-budget"; @@ -470,7 +471,7 @@ function scrubLegacyBackup(providers: readonly string[]): void { try { const remaining = readLegacyBackupEntries(backup); for (const provider of providers) delete remaining[provider]; - if (Object.keys(remaining).length > 0) replaceBackupEntry(backup, `${JSON.stringify(remaining, null, 2)}\n`); + if (Object.keys(remaining).length > 0) atomicWriteFileNoFollowUnclaimed(backup, `${JSON.stringify(remaining, null, 2)}\n`); else unlinkSync(backup); } catch (error) { if (errorCode(error) !== "ENOENT") { @@ -490,27 +491,6 @@ function readLegacyBackupEntries(backup: string): Record { } } -/** - * Replace the backup entry itself through an exclusive private temp and a rename: an entry - * at the path is replaced, never followed, and uninstall ownership is left exactly as it was - * (the shared atomic writer would claim a backup this install never registered). - */ -function replaceBackupEntry(backup: string, content: string): void { - const temp = `${backup}.${process.pid}.${randomUUID()}.tmp`; - const fd = openSync(temp, "wx", 0o600); - try { - writeFileSync(fd, content); - } finally { - closeSync(fd); - } - try { - renameSync(temp, backup); - } catch (error) { - try { unlinkSync(temp); } catch { /* best-effort */ } - throw error; - } -} - function directoryEntryExists(path: string): boolean { try { lstatSync(path); diff --git a/structure/config.md b/structure/config.md index 13c571a147b..2f6b0d5e86e 100644 --- a/structure/config.md +++ b/structure/config.md @@ -414,7 +414,9 @@ traversing their targets. Unknown files remain in place and make the command rep uninstall with their exact paths. The newly created OAuth downgrade copy is registered after copying, so owned uninstall -includes it. Invalid-config recovery copies are deliberately NOT registered: their names carry +includes it. Destructive OAuth mutations rewrite that copy without the removed provider through the +no-follow writer variant that leaves the owner manifest untouched, so a copy an earlier install +left unregistered stays unclaimed. Invalid-config recovery copies are deliberately NOT registered: their names carry a timestamp, so one entry per invalid load would grow the uninstall manifest without bound, and the manifest stops validating past its path ceiling. A manifest that stops validating makes uninstall refuse outright, which would leave credentials on disk. Sweeping those copies by name From 48215a1e660fb3714d731153530449dba0b3da70 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 05:39:54 +0900 Subject: [PATCH 23/32] docs(structure): state when an existing OAuth downgrade copy is rewritten --- structure/config.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/structure/config.md b/structure/config.md index 2f6b0d5e86e..582f37e6039 100644 --- a/structure/config.md +++ b/structure/config.md @@ -421,8 +421,9 @@ a timestamp, so one entry per invalid load would grow the uninstall manifest wit the manifest stops validating past its path ceiling. A manifest that stops validating makes uninstall refuse outright, which would leave credentials on disk. Sweeping those copies by name pattern at removal time is the shape that fits; it is not in this change. Registration is best-effort: an intentionally -unowned legacy home or a metadata-write failure must not suppress the recovery copy. Existing -OAuth downgrade copies are neither rewritten nor retroactively claimed. Both a `false` registration +unowned legacy home or a metadata-write failure must not suppress the recovery copy. Migration +leaves an existing OAuth downgrade copy unchanged and never retroactively claims it; only a +destructive mutation rewrites it, to drop the removed provider. Both a `false` registration result and a thrown registration error emit the same fixed warning without error details. Unregistered copies remain subject to the existing partial/refused uninstall result. From f6e65bfe2e90ba753ea328fafdcce57d18790d02 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 06:02:36 +0900 Subject: [PATCH 24/32] fix(proxy): scope the inherited-proxy loopback bypass to exact hosts Security review of the carried #5430 change: on the no-config.proxy path loopback names were added to NO_PROXY for any inherited proxy, and both matchers treat entries as domain suffixes, so "localhost" also sent any *.localhost name direct, which need not resolve to loopback. Add the loopback entries only when an inherited SOCKS proxy owns HTTP(S) traffic; that proxy is applied by the in-process matcher, which now treats a bare localhost or IP-literal entry as one host (a leading dot still means subdomains). An inherited HTTP(S) proxy is Bun's own and is left as it was. Tests cover *.localhost staying on the SOCKS proxy, the no-widening cases, and the exact-match rules. --- src/config/proxy-env.ts | 30 +++++++---------- src/lib/proxy-env.ts | 12 ++++++- structure/config.md | 6 ++-- tests/server/proxy-env.test.ts | 59 +++++++++++++++++++++++++++++++--- 4 files changed, 82 insertions(+), 25 deletions(-) diff --git a/src/config/proxy-env.ts b/src/config/proxy-env.ts index a603b502d6a..d801e996a3c 100644 --- a/src/config/proxy-env.ts +++ b/src/config/proxy-env.ts @@ -1,4 +1,4 @@ -import { configureSocks5Fetch } from "../lib/proxy-env"; +import { configureSocks5Fetch, socks5ProxyFromEnv } from "../lib/proxy-env"; import { redactUrlForLog } from "../lib/redact"; import { join } from "node:path"; import { DEFAULT_SUBAGENT_MODELS, SUBAGENT_MODELS_VERSION } from "./subagent-models"; @@ -98,21 +98,15 @@ function warnProxyConfigDiscardOnce(kind: "proxy" | "noProxy" | "noProxyElements } } -// Loopback only has a proxy to bypass when the environment already carries proxy state. -// Writing NO_PROXY into a proxy-free process is itself a proxy-env mutation that callers -// observe (the lab sandbox rejects any of these keys as a forbidden leak), so the -// early-return merge runs only when one is already present. -const PROXY_STATE_ENV_KEYS = [ - "HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY", - "http_proxy", "https_proxy", "all_proxy", "no_proxy", -] as const; - -function ambientProxyStateExists(): boolean { - for (const key of PROXY_STATE_ENV_KEYS) { - const value = process.env[key]; - if (value !== undefined && value !== "") return true; - } - return false; +// With no config.proxy, loopback bypasses are written only for an inherited SOCKS proxy that +// owns HTTP(S) traffic: that proxy is applied by the in-process matcher, where the loopback +// entries match exactly. An inherited HTTP(S) proxy is Bun's own, and Bun matches NO_PROXY +// entries as domain suffixes, so adding "localhost" there would also send any *.localhost name +// direct. A proxy-free process is left untouched: writing NO_PROXY into it is itself a +// proxy-env mutation callers observe (the lab sandbox rejects these keys as a forbidden leak). +function inheritedSocksProxyOwnsTraffic(): boolean { + if (socks5ProxyFromEnv() === undefined) return false; + return !["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"].some(key => process.env[key]?.trim()); } function withNoProxyEntries(existing: string, configured: readonly string[]): string { @@ -170,11 +164,11 @@ export function applyProxyEnvWith( let proxy = typeof rawProxy === "string" ? resolveEnvValue(rawProxy) : undefined; if (!proxy) { if (rawProxy !== undefined) warnProxyConfigDiscardOnce("proxy"); - // Ambient-proxy path: only loopback bypasses are appended. A configured noProxy is + // Inherited-SOCKS path: only loopback bypasses are appended. A configured noProxy is // deliberately NOT merged here — with no config.proxy the operator's bypass list has // no declared proxy to apply against, and merging it would silently widen direct // egress beyond the loopback fix this branch exists for. - if (ambientProxyStateExists()) mergeNoProxyEntries(); + if (inheritedSocksProxyOwnsTraffic()) mergeNoProxyEntries(); configureSocks5Fetch(); return; } diff --git a/src/lib/proxy-env.ts b/src/lib/proxy-env.ts index bf2e70b9175..2ff8f89a94f 100644 --- a/src/lib/proxy-env.ts +++ b/src/lib/proxy-env.ts @@ -29,6 +29,7 @@ export function noProxyMatches( if (!entry) continue; if (entry === "*") return true; entry = entry.replace(/^(?:https?|wss?):\/\//, "").split("/", 1)[0]!; + const domainForm = /^\*?\./.test(entry); let entryHost = entry; let entryPort = ""; @@ -46,11 +47,20 @@ export function noProxyMatches( } if (entryPort && entryPort !== port) continue; entryHost = normalizeProxyHostname(entryHost.replace(/^\*?\./, "")); - if (entryHost && (hostname === entryHost || hostname.endsWith(`.${entryHost}`))) return true; + if (!entryHost) continue; + if (hostname === entryHost) return true; + // A bare loopback name or an IP literal names one host: "localhost" must not send + // "anything.localhost" direct, which need not resolve to loopback. ".localhost" still does. + if (!domainForm && isExactOnlyNoProxyHost(entryHost)) continue; + if (hostname.endsWith(`.${entryHost}`)) return true; } return false; } +function isExactOnlyNoProxyHost(host: string): boolean { + return host === "localhost" || host.includes(":") || /^\d{1,3}(?:\.\d{1,3}){3}$/.test(host); +} + export function resolveProxyRoute( url: URL, env: ProxyEnvMap = process.env, diff --git a/structure/config.md b/structure/config.md index 582f37e6039..a0067be28f9 100644 --- a/structure/config.md +++ b/structure/config.md @@ -545,8 +545,10 @@ Stored Direct substitution follows the [credential identity contract](providers/ configuration. An explicit SOCKS5 or SOCKS5h URL selects ALL_PROXY and removes stale scheme-proxy variables; HTTP(S) settings retain their existing environment precedence. Activation keeps the existing Windows auto-discovery path and loopback -NO_PROXY entries; the no-configured-proxy return merges them only when the environment -already carries proxy state, leaving a proxy-free process untouched. An inherited non-empty +NO_PROXY entries; the no-configured-proxy return merges them only for an inherited SOCKS +proxy that owns HTTP(S) traffic (no inherited HTTP(S) proxy), leaving a proxy-free process and +an inherited HTTP(S) proxy, which Bun matches by domain suffix, untouched. The in-process +matcher treats a bare `localhost` or IP-literal entry as one host, never a suffix. An inherited non-empty lowercase `no_proxy`, which Bun fetch reads first, receives the same entries. When the environment no longer selects SOCKS, activation restores the native fetch; removing a saved field alone does not erase inherited diff --git a/tests/server/proxy-env.test.ts b/tests/server/proxy-env.test.ts index 91789394be4..090dc5fbc13 100644 --- a/tests/server/proxy-env.test.ts +++ b/tests/server/proxy-env.test.ts @@ -223,7 +223,8 @@ describe("applyProxyEnv", () => { } }); - test("keeps mandatory loopback exclusions when config.proxy is unset", () => { + test("keeps mandatory loopback exclusions for an inherited SOCKS proxy when config.proxy is unset", () => { + process.env.ALL_PROXY = "socks5://untrusted-proxy.invalid:1080"; process.env.NO_PROXY = "operator-owned.example"; applyProxyEnv(configWithProxy(undefined, "internal.example")); expect(process.env.HTTP_PROXY).toBeUndefined(); @@ -231,6 +232,19 @@ describe("applyProxyEnv", () => { expect(process.env.NO_PROXY).toBe("operator-owned.example,localhost,127.0.0.1,::1,[::1]"); }); + test.each([ + ["an inherited HTTP proxy", { HTTP_PROXY: "http://proxy.invalid:3128" }], + ["SOCKS beside an inherited HTTPS proxy", { ALL_PROXY: "socks5://proxy.invalid:1080", https_proxy: "http://proxy.invalid:3128" }], + ["only NO_PROXY", {}], + ] as const)("leaves NO_PROXY untouched with %s and no config.proxy", (_label, inherited) => { + // Bun applies an inherited HTTP(S) proxy itself and matches NO_PROXY entries as domain + // suffixes, so adding "localhost" there would also send any *.localhost name direct. + Object.assign(process.env, inherited); + process.env.NO_PROXY = "operator-owned.example"; + applyProxyEnv(configWithProxy()); + expect(process.env.NO_PROXY).toBe("operator-owned.example"); + }); + test.each(["ALL_PROXY", "all_proxy"])("inherited SOCKS %s cannot intercept loopback fetches", async key => { process.env[key] = "socks5://untrusted-proxy.invalid:1080"; applyProxyEnv(configWithProxy()); @@ -270,16 +284,53 @@ describe("applyProxyEnv", () => { } }); - test("an inherited lowercase no_proxy also receives the loopback entries", () => { + test("an inherited SOCKS proxy keeps *.localhost names on the proxy", async () => { + const refusing = createTcpServer(socket => socket.destroy()); + await new Promise((resolve, reject) => { + refusing.once("error", reject); + refusing.listen(0, "127.0.0.1", resolve); + }); + const address = refusing.address(); + if (!address || typeof address === "string") throw new Error("proxy fixture did not bind a TCP port"); + // socks5h: the proxy resolves the name, so the fixture fails the request without local DNS. + process.env.ALL_PROXY = `socks5h://127.0.0.1:${address.port}`; + applyProxyEnv(configWithProxy()); + let directCalls = 0; + const direct = async () => { + directCalls += 1; + return new Response("direct"); + }; + try { + expect(await (await configuredOutboundFetch("http://localhost:11434/v1/models", undefined, direct)).text()).toBe("direct"); + expect(directCalls).toBe(1); + await expect(configuredOutboundFetch("http://app.localhost:11434/v1/models", undefined, direct)).rejects.toThrow(); + expect(directCalls).toBe(1); + } finally { + await new Promise(resolve => refusing.close(() => resolve())); + } + }); + + test("a configured proxy also merges loopback into an inherited lowercase no_proxy", () => { // Bun's native fetch consults a non-empty lowercase no_proxy before NO_PROXY. - process.env.HTTP_PROXY = "http://proxy.invalid:3128"; process.env.no_proxy = "internal.example"; - applyProxyEnv(configWithProxy()); + applyProxyEnv(configWithProxy("http://proxy.invalid:3128")); expect(process.env.no_proxy).toBe("internal.example,localhost,127.0.0.1,::1,[::1]"); const loopback = new URL("http://127.0.0.1:11434/v1/models"); expect(noProxyMatches(loopback, { no_proxy: process.env.no_proxy })).toBe(true); }); + test("loopback names and IP literals match exactly; a leading dot still means subdomains", () => { + const env = { NO_PROXY: "localhost,127.0.0.1,::1,[::1],example.com,.localtest" }; + const matches = (url: string) => noProxyMatches(new URL(url), env); + expect(matches("http://localhost:11434/")).toBe(true); + expect(matches("http://127.0.0.1:11434/")).toBe(true); + expect(matches("http://[::1]:11434/")).toBe(true); + expect(matches("http://app.localhost/")).toBe(false); + expect(matches("https://api.example.com/")).toBe(true); + expect(matches("http://app.localtest/")).toBe(true); + expect(noProxyMatches(new URL("http://app.localhost/"), { NO_PROXY: ".localhost" })).toBe(true); + }); + test("merges configured comma-separated noProxy entries", () => { applyProxyEnv(configWithProxy("http://proxy.corp:8080", "internal.example,10.0.0.0/8")); expect(process.env.NO_PROXY).toBe("internal.example,10.0.0.0/8,localhost,127.0.0.1,::1,[::1]"); From 51585db1b178a18b7e049b8bcc578503b41e96f0 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 06:05:18 +0900 Subject: [PATCH 25/32] fix(proxy): key the loopback bypass on SOCKS precedence Security re-review: with an inherited SOCKS proxy beside an inherited HTTP(S) proxy the fetch wrapper still applies SOCKS first, so skipping the loopback entries in that case let local requests pass through SOCKS. Add them whenever an inherited SOCKS proxy exists; its exact matcher decides first, so only a request already judged loopback reaches Bun's suffix-matching proxy. Pin the mixed case. --- src/config/proxy-env.ts | 17 +++++++++-------- structure/config.md | 4 ++-- tests/server/proxy-env.test.ts | 9 +++++++-- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/config/proxy-env.ts b/src/config/proxy-env.ts index d801e996a3c..6c0493be2ec 100644 --- a/src/config/proxy-env.ts +++ b/src/config/proxy-env.ts @@ -98,15 +98,16 @@ function warnProxyConfigDiscardOnce(kind: "proxy" | "noProxy" | "noProxyElements } } -// With no config.proxy, loopback bypasses are written only for an inherited SOCKS proxy that -// owns HTTP(S) traffic: that proxy is applied by the in-process matcher, where the loopback -// entries match exactly. An inherited HTTP(S) proxy is Bun's own, and Bun matches NO_PROXY -// entries as domain suffixes, so adding "localhost" there would also send any *.localhost name -// direct. A proxy-free process is left untouched: writing NO_PROXY into it is itself a -// proxy-env mutation callers observe (the lab sandbox rejects these keys as a forbidden leak). +// With no config.proxy, loopback bypasses are written only for an inherited SOCKS proxy. The +// installed fetch wrapper applies SOCKS before anything else (src/lib/proxy-env.ts +// configuredOutboundFetch), and its matcher treats these loopback entries as exact hosts, so +// only a request already judged loopback reaches Bun's own proxying, even beside an inherited +// HTTP(S) proxy. An inherited HTTP(S) proxy alone is Bun's, and Bun matches NO_PROXY entries as +// domain suffixes, so adding "localhost" there would also send any *.localhost name direct. +// A proxy-free process is left untouched: writing NO_PROXY into it is itself a proxy-env +// mutation callers observe (the lab sandbox rejects these keys as a forbidden leak). function inheritedSocksProxyOwnsTraffic(): boolean { - if (socks5ProxyFromEnv() === undefined) return false; - return !["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"].some(key => process.env[key]?.trim()); + return socks5ProxyFromEnv() !== undefined; } function withNoProxyEntries(existing: string, configured: readonly string[]): string { diff --git a/structure/config.md b/structure/config.md index a0067be28f9..21af4e195b3 100644 --- a/structure/config.md +++ b/structure/config.md @@ -546,8 +546,8 @@ configuration. An explicit SOCKS5 or SOCKS5h URL selects ALL_PROXY and removes stale scheme-proxy variables; HTTP(S) settings retain their existing environment precedence. Activation keeps the existing Windows auto-discovery path and loopback NO_PROXY entries; the no-configured-proxy return merges them only for an inherited SOCKS -proxy that owns HTTP(S) traffic (no inherited HTTP(S) proxy), leaving a proxy-free process and -an inherited HTTP(S) proxy, which Bun matches by domain suffix, untouched. The in-process +proxy, which the installed fetch wrapper applies first, leaving a proxy-free process and an +inherited HTTP(S) proxy alone, which Bun matches by domain suffix, untouched. The in-process matcher treats a bare `localhost` or IP-literal entry as one host, never a suffix. An inherited non-empty lowercase `no_proxy`, which Bun fetch reads first, receives the same entries. When the environment no longer selects SOCKS, activation diff --git a/tests/server/proxy-env.test.ts b/tests/server/proxy-env.test.ts index 090dc5fbc13..a929da88481 100644 --- a/tests/server/proxy-env.test.ts +++ b/tests/server/proxy-env.test.ts @@ -234,7 +234,7 @@ describe("applyProxyEnv", () => { test.each([ ["an inherited HTTP proxy", { HTTP_PROXY: "http://proxy.invalid:3128" }], - ["SOCKS beside an inherited HTTPS proxy", { ALL_PROXY: "socks5://proxy.invalid:1080", https_proxy: "http://proxy.invalid:3128" }], + ["an inherited HTTPS proxy", { https_proxy: "http://proxy.invalid:3128" }], ["only NO_PROXY", {}], ] as const)("leaves NO_PROXY untouched with %s and no config.proxy", (_label, inherited) => { // Bun applies an inherited HTTP(S) proxy itself and matches NO_PROXY entries as domain @@ -284,7 +284,10 @@ describe("applyProxyEnv", () => { } }); - test("an inherited SOCKS proxy keeps *.localhost names on the proxy", async () => { + test.each([ + ["alone", {}], + ["beside an inherited HTTP proxy", { HTTP_PROXY: "http://proxy.invalid:3128" }], + ] as const)("an inherited SOCKS proxy %s: loopback goes direct, *.localhost stays on SOCKS", async (_label, inherited) => { const refusing = createTcpServer(socket => socket.destroy()); await new Promise((resolve, reject) => { refusing.once("error", reject); @@ -294,7 +297,9 @@ describe("applyProxyEnv", () => { if (!address || typeof address === "string") throw new Error("proxy fixture did not bind a TCP port"); // socks5h: the proxy resolves the name, so the fixture fails the request without local DNS. process.env.ALL_PROXY = `socks5h://127.0.0.1:${address.port}`; + Object.assign(process.env, inherited); applyProxyEnv(configWithProxy()); + expect(process.env.NO_PROXY).toBe("localhost,127.0.0.1,::1,[::1]"); let directCalls = 0; const direct = async () => { directCalls += 1; From 70384c7bd9e990f69809ec1734dc53b8ed81cf71 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 06:06:20 +0900 Subject: [PATCH 26/32] fix(proxy): keep 127.0.0.1 off an inherited HTTP proxy without suffix risk Delta review: with no config.proxy and only an inherited HTTP(S) proxy, loopback calls such as the CLI's own management and health requests to 127.0.0.1 could go to that proxy. Add the loopback addresses there (a URL host ending in a numeric label parses as IPv4, so Bun's suffix matching cannot widen them) but not "localhost", which Bun would match as *.localhost. An inherited SOCKS proxy keeps the full list, and a process with no inherited proxy is still left untouched. --- src/config/proxy-env.ts | 40 ++++++++++++++++++++-------------- structure/config.md | 7 +++--- tests/server/proxy-env.test.ts | 10 +++++++-- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/src/config/proxy-env.ts b/src/config/proxy-env.ts index 6c0493be2ec..36e4e178e01 100644 --- a/src/config/proxy-env.ts +++ b/src/config/proxy-env.ts @@ -98,22 +98,29 @@ function warnProxyConfigDiscardOnce(kind: "proxy" | "noProxy" | "noProxyElements } } -// With no config.proxy, loopback bypasses are written only for an inherited SOCKS proxy. The -// installed fetch wrapper applies SOCKS before anything else (src/lib/proxy-env.ts -// configuredOutboundFetch), and its matcher treats these loopback entries as exact hosts, so -// only a request already judged loopback reaches Bun's own proxying, even beside an inherited -// HTTP(S) proxy. An inherited HTTP(S) proxy alone is Bun's, and Bun matches NO_PROXY entries as -// domain suffixes, so adding "localhost" there would also send any *.localhost name direct. -// A proxy-free process is left untouched: writing NO_PROXY into it is itself a proxy-env -// mutation callers observe (the lab sandbox rejects these keys as a forbidden leak). -function inheritedSocksProxyOwnsTraffic(): boolean { - return socks5ProxyFromEnv() !== undefined; +const LOOPBACK_NO_PROXY = ["localhost", "127.0.0.1", "::1", "[::1]"] as const; +const LOOPBACK_ADDRESS_NO_PROXY = ["127.0.0.1", "::1", "[::1]"] as const; + +// With no config.proxy, which loopback bypasses are written depends on who applies the +// inherited proxy. An inherited SOCKS proxy is applied first by the installed fetch wrapper +// (src/lib/proxy-env.ts configuredOutboundFetch), whose matcher treats these entries as exact +// hosts, so the full list is safe even beside an inherited HTTP(S) proxy: only a request already +// judged loopback reaches Bun's own proxying. An inherited HTTP(S) proxy alone is Bun's, and Bun +// matches NO_PROXY entries as domain suffixes, so "localhost" there would also send any +// *.localhost name direct; the loopback addresses cannot widen that way (a URL host ending in a +// numeric label parses as IPv4), so only they are added, keeping local health and management +// calls to 127.0.0.1 off that proxy. A proxy-free process is left untouched: writing NO_PROXY +// into it is itself a proxy-env mutation callers observe (the lab sandbox rejects these keys). +function inheritedLoopbackBypass(): readonly string[] | undefined { + if (socks5ProxyFromEnv() !== undefined) return LOOPBACK_NO_PROXY; + const schemeProxy = ["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"].some(key => process.env[key]?.trim()); + return schemeProxy ? LOOPBACK_ADDRESS_NO_PROXY : undefined; } -function withNoProxyEntries(existing: string, configured: readonly string[]): string { +function withNoProxyEntries(existing: string, configured: readonly string[], loopback: readonly string[]): string { const entries = existing.split(",").map(s => s.trim()).filter(Boolean); const seen = new Set(entries.map(entry => entry.toLowerCase())); - for (const host of [...configured, "localhost", "127.0.0.1", "::1", "[::1]"]) { + for (const host of [...configured, ...loopback]) { const key = host.toLowerCase(); if (!seen.has(key)) { entries.push(host); @@ -123,13 +130,13 @@ function withNoProxyEntries(existing: string, configured: readonly string[]): st return entries.join(","); } -function mergeNoProxyEntries(configured: string[] = []): void { - process.env.NO_PROXY = withNoProxyEntries(process.env.NO_PROXY ?? process.env.no_proxy ?? "", configured); +function mergeNoProxyEntries(configured: readonly string[] = [], loopback: readonly string[] = LOOPBACK_NO_PROXY): void { + process.env.NO_PROXY = withNoProxyEntries(process.env.NO_PROXY ?? process.env.no_proxy ?? "", configured, loopback); // Bun's native fetch reads a non-empty lowercase no_proxy before NO_PROXY // (src/codex/catalog/remote.ts), so an inherited one would shadow the entries above. const inherited = process.env.no_proxy; if (inherited !== undefined && inherited.trim() !== "") { - process.env.no_proxy = withNoProxyEntries(inherited, configured); + process.env.no_proxy = withNoProxyEntries(inherited, configured, loopback); } } @@ -169,7 +176,8 @@ export function applyProxyEnvWith( // deliberately NOT merged here — with no config.proxy the operator's bypass list has // no declared proxy to apply against, and merging it would silently widen direct // egress beyond the loopback fix this branch exists for. - if (inheritedSocksProxyOwnsTraffic()) mergeNoProxyEntries(); + const loopback = inheritedLoopbackBypass(); + if (loopback) mergeNoProxyEntries([], loopback); configureSocks5Fetch(); return; } diff --git a/structure/config.md b/structure/config.md index 21af4e195b3..8e791e809bd 100644 --- a/structure/config.md +++ b/structure/config.md @@ -545,9 +545,10 @@ Stored Direct substitution follows the [credential identity contract](providers/ configuration. An explicit SOCKS5 or SOCKS5h URL selects ALL_PROXY and removes stale scheme-proxy variables; HTTP(S) settings retain their existing environment precedence. Activation keeps the existing Windows auto-discovery path and loopback -NO_PROXY entries; the no-configured-proxy return merges them only for an inherited SOCKS -proxy, which the installed fetch wrapper applies first, leaving a proxy-free process and an -inherited HTTP(S) proxy alone, which Bun matches by domain suffix, untouched. The in-process +NO_PROXY entries; the no-configured-proxy return merges them for an inherited SOCKS proxy, +which the installed fetch wrapper applies first; for an inherited HTTP(S) proxy alone, which Bun +matches by domain suffix, it adds only the loopback addresses (never `localhost`); a proxy-free +process is left untouched. The in-process matcher treats a bare `localhost` or IP-literal entry as one host, never a suffix. An inherited non-empty lowercase `no_proxy`, which Bun fetch reads first, receives the same entries. When the environment no longer selects SOCKS, activation diff --git a/tests/server/proxy-env.test.ts b/tests/server/proxy-env.test.ts index a929da88481..a2f4ecdc46f 100644 --- a/tests/server/proxy-env.test.ts +++ b/tests/server/proxy-env.test.ts @@ -235,11 +235,17 @@ describe("applyProxyEnv", () => { test.each([ ["an inherited HTTP proxy", { HTTP_PROXY: "http://proxy.invalid:3128" }], ["an inherited HTTPS proxy", { https_proxy: "http://proxy.invalid:3128" }], - ["only NO_PROXY", {}], - ] as const)("leaves NO_PROXY untouched with %s and no config.proxy", (_label, inherited) => { + ] as const)("adds only loopback addresses for %s and no config.proxy", (_label, inherited) => { // Bun applies an inherited HTTP(S) proxy itself and matches NO_PROXY entries as domain // suffixes, so adding "localhost" there would also send any *.localhost name direct. + // Loopback addresses cannot widen that way and keep local 127.0.0.1 calls off the proxy. Object.assign(process.env, inherited); + process.env.NO_PROXY = "operator-owned.example"; + applyProxyEnv(configWithProxy()); + expect(process.env.NO_PROXY).toBe("operator-owned.example,127.0.0.1,::1,[::1]"); + }); + + test("leaves an inherited NO_PROXY untouched when no proxy is inherited", () => { process.env.NO_PROXY = "operator-owned.example"; applyProxyEnv(configWithProxy()); expect(process.env.NO_PROXY).toBe("operator-owned.example"); From e62eebd5ff8792fa849221ee4cdbf94113451488 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 06:10:04 +0900 Subject: [PATCH 27/32] fix(proxy): add bare localhost only when SOCKS is the sole inherited proxy Security re-review: beside an inherited HTTP(S) proxy, a bare localhost in process-wide NO_PROXY is also read by Bun, whose suffix matching can send an unwrapped fetch for any *.localhost name past that proxy. Write the full loopback list only when an inherited SOCKS proxy is the only inherited proxy; whenever Bun applies an inherited HTTP(S) proxy, add only the loopback addresses. The mixed case keeps 127.0.0.1 direct through the SOCKS wrapper and sends *.localhost to SOCKS. --- src/config/proxy-env.ts | 24 ++++++++++++------------ structure/config.md | 4 ++-- tests/server/proxy-env.test.ts | 11 ++++++----- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/config/proxy-env.ts b/src/config/proxy-env.ts index 36e4e178e01..b38ac4a902c 100644 --- a/src/config/proxy-env.ts +++ b/src/config/proxy-env.ts @@ -101,20 +101,20 @@ function warnProxyConfigDiscardOnce(kind: "proxy" | "noProxy" | "noProxyElements const LOOPBACK_NO_PROXY = ["localhost", "127.0.0.1", "::1", "[::1]"] as const; const LOOPBACK_ADDRESS_NO_PROXY = ["127.0.0.1", "::1", "[::1]"] as const; -// With no config.proxy, which loopback bypasses are written depends on who applies the -// inherited proxy. An inherited SOCKS proxy is applied first by the installed fetch wrapper -// (src/lib/proxy-env.ts configuredOutboundFetch), whose matcher treats these entries as exact -// hosts, so the full list is safe even beside an inherited HTTP(S) proxy: only a request already -// judged loopback reaches Bun's own proxying. An inherited HTTP(S) proxy alone is Bun's, and Bun -// matches NO_PROXY entries as domain suffixes, so "localhost" there would also send any -// *.localhost name direct; the loopback addresses cannot widen that way (a URL host ending in a -// numeric label parses as IPv4), so only they are added, keeping local health and management -// calls to 127.0.0.1 off that proxy. A proxy-free process is left untouched: writing NO_PROXY -// into it is itself a proxy-env mutation callers observe (the lab sandbox rejects these keys). +// With no config.proxy, which loopback bypasses are written depends on who reads them. The +// installed SOCKS fetch wrapper (src/lib/proxy-env.ts configuredOutboundFetch) matches these +// entries as exact hosts. Bun applies an inherited HTTP(S) proxy itself and matches NO_PROXY +// entries as domain suffixes, so a bare "localhost" there would also send any *.localhost name +// direct, including from a fetch that never passes the wrapper. The full list is therefore +// written only when an inherited SOCKS proxy is the only one. Whenever Bun applies an inherited +// HTTP(S) proxy, only the loopback addresses are added: they cannot widen that way (a URL host +// ending in a numeric label parses as IPv4) and keep local health and management calls to +// 127.0.0.1 off the proxy. A proxy-free process is left untouched: writing NO_PROXY into it is +// itself a proxy-env mutation callers observe (the lab sandbox rejects these keys). function inheritedLoopbackBypass(): readonly string[] | undefined { - if (socks5ProxyFromEnv() !== undefined) return LOOPBACK_NO_PROXY; const schemeProxy = ["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"].some(key => process.env[key]?.trim()); - return schemeProxy ? LOOPBACK_ADDRESS_NO_PROXY : undefined; + if (schemeProxy) return LOOPBACK_ADDRESS_NO_PROXY; + return socks5ProxyFromEnv() !== undefined ? LOOPBACK_NO_PROXY : undefined; } function withNoProxyEntries(existing: string, configured: readonly string[], loopback: readonly string[]): string { diff --git a/structure/config.md b/structure/config.md index 8e791e809bd..d369fe21bde 100644 --- a/structure/config.md +++ b/structure/config.md @@ -545,8 +545,8 @@ Stored Direct substitution follows the [credential identity contract](providers/ configuration. An explicit SOCKS5 or SOCKS5h URL selects ALL_PROXY and removes stale scheme-proxy variables; HTTP(S) settings retain their existing environment precedence. Activation keeps the existing Windows auto-discovery path and loopback -NO_PROXY entries; the no-configured-proxy return merges them for an inherited SOCKS proxy, -which the installed fetch wrapper applies first; for an inherited HTTP(S) proxy alone, which Bun +NO_PROXY entries; the no-configured-proxy return merges all of them only when an inherited +SOCKS proxy is the only inherited proxy; whenever Bun applies an inherited HTTP(S) proxy, which it matches by domain suffix, it adds only the loopback addresses (never `localhost`); a proxy-free process is left untouched. The in-process matcher treats a bare `localhost` or IP-literal entry as one host, never a suffix. An inherited non-empty diff --git a/tests/server/proxy-env.test.ts b/tests/server/proxy-env.test.ts index a2f4ecdc46f..d68cb5398f7 100644 --- a/tests/server/proxy-env.test.ts +++ b/tests/server/proxy-env.test.ts @@ -291,9 +291,9 @@ describe("applyProxyEnv", () => { }); test.each([ - ["alone", {}], - ["beside an inherited HTTP proxy", { HTTP_PROXY: "http://proxy.invalid:3128" }], - ] as const)("an inherited SOCKS proxy %s: loopback goes direct, *.localhost stays on SOCKS", async (_label, inherited) => { + ["alone", {}, "http://localhost:11434/v1/models", "localhost,127.0.0.1,::1,[::1]"], + ["beside an inherited HTTP proxy", { HTTP_PROXY: "http://proxy.invalid:3128" }, "http://127.0.0.1:11434/v1/models", "127.0.0.1,::1,[::1]"], + ] as const)("an inherited SOCKS proxy %s: loopback goes direct, *.localhost stays on SOCKS", async (_label, inherited, loopbackUrl, expectedNoProxy) => { const refusing = createTcpServer(socket => socket.destroy()); await new Promise((resolve, reject) => { refusing.once("error", reject); @@ -305,14 +305,15 @@ describe("applyProxyEnv", () => { process.env.ALL_PROXY = `socks5h://127.0.0.1:${address.port}`; Object.assign(process.env, inherited); applyProxyEnv(configWithProxy()); - expect(process.env.NO_PROXY).toBe("localhost,127.0.0.1,::1,[::1]"); + // Beside an HTTP(S) proxy Bun reads NO_PROXY too, with suffix matching, so no bare localhost. + expect(process.env.NO_PROXY).toBe(expectedNoProxy); let directCalls = 0; const direct = async () => { directCalls += 1; return new Response("direct"); }; try { - expect(await (await configuredOutboundFetch("http://localhost:11434/v1/models", undefined, direct)).text()).toBe("direct"); + expect(await (await configuredOutboundFetch(loopbackUrl, undefined, direct)).text()).toBe("direct"); expect(directCalls).toBe(1); await expect(configuredOutboundFetch("http://app.localhost:11434/v1/models", undefined, direct)).rejects.toThrow(); expect(directCalls).toBe(1); From 5607c7d68aadd0b4800fb2d00856175467c2a717 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 06:12:41 +0900 Subject: [PATCH 28/32] fix(proxy): never add a bare localhost to an inherited lowercase no_proxy Review: with a configured HTTP(S) proxy and an inherited lowercase no_proxy, which Bun reads first and matches by domain suffix, adding a bare localhost newly let *.localhost names bypass the proxy. The lowercase value now gains the configured entries and the loopback addresses only. --- src/config/proxy-env.ts | 7 +++++-- structure/config.md | 3 ++- tests/server/proxy-env.test.ts | 8 ++++++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/config/proxy-env.ts b/src/config/proxy-env.ts index b38ac4a902c..18e79ee6c12 100644 --- a/src/config/proxy-env.ts +++ b/src/config/proxy-env.ts @@ -133,10 +133,13 @@ function withNoProxyEntries(existing: string, configured: readonly string[], loo function mergeNoProxyEntries(configured: readonly string[] = [], loopback: readonly string[] = LOOPBACK_NO_PROXY): void { process.env.NO_PROXY = withNoProxyEntries(process.env.NO_PROXY ?? process.env.no_proxy ?? "", configured, loopback); // Bun's native fetch reads a non-empty lowercase no_proxy before NO_PROXY - // (src/codex/catalog/remote.ts), so an inherited one would shadow the entries above. + // (src/codex/catalog/remote.ts), so an inherited one would shadow the entries above. Only + // the loopback addresses join it: Bun matches entries as domain suffixes, and a bare + // "localhost" there would send any *.localhost name past a proxy the inherited value + // previously kept it on. const inherited = process.env.no_proxy; if (inherited !== undefined && inherited.trim() !== "") { - process.env.no_proxy = withNoProxyEntries(inherited, configured, loopback); + process.env.no_proxy = withNoProxyEntries(inherited, configured, loopback.filter(host => host !== "localhost")); } } diff --git a/structure/config.md b/structure/config.md index d369fe21bde..38dab6961c3 100644 --- a/structure/config.md +++ b/structure/config.md @@ -550,7 +550,8 @@ SOCKS proxy is the only inherited proxy; whenever Bun applies an inherited HTTP( matches by domain suffix, it adds only the loopback addresses (never `localhost`); a proxy-free process is left untouched. The in-process matcher treats a bare `localhost` or IP-literal entry as one host, never a suffix. An inherited non-empty -lowercase `no_proxy`, which Bun fetch reads first, receives the same entries. When the +lowercase `no_proxy`, which Bun fetch reads first with suffix matching, receives the same entries +except a bare `localhost`. When the environment no longer selects SOCKS, activation restores the native fetch; removing a saved field alone does not erase inherited process environment variables. diff --git a/tests/server/proxy-env.test.ts b/tests/server/proxy-env.test.ts index d68cb5398f7..70dfa3d88b7 100644 --- a/tests/server/proxy-env.test.ts +++ b/tests/server/proxy-env.test.ts @@ -323,10 +323,14 @@ describe("applyProxyEnv", () => { }); test("a configured proxy also merges loopback into an inherited lowercase no_proxy", () => { - // Bun's native fetch consults a non-empty lowercase no_proxy before NO_PROXY. + // Bun's native fetch consults a non-empty lowercase no_proxy before NO_PROXY, with suffix + // matching, so it gains the loopback addresses but never a bare localhost. process.env.no_proxy = "internal.example"; applyProxyEnv(configWithProxy("http://proxy.invalid:3128")); - expect(process.env.no_proxy).toBe("internal.example,localhost,127.0.0.1,::1,[::1]"); + // Windows environment names are case-insensitive: there no_proxy IS NO_PROXY. + expect(process.env.no_proxy).toBe(process.platform === "win32" + ? "internal.example,localhost,127.0.0.1,::1,[::1]" + : "internal.example,127.0.0.1,::1,[::1]"); const loopback = new URL("http://127.0.0.1:11434/v1/models"); expect(noProxyMatches(loopback, { no_proxy: process.env.no_proxy })).toBe(true); }); From 00e5111fce0fcf609634a697f623ad420eca4bca Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 06:14:27 +0900 Subject: [PATCH 29/32] fix(proxy): add only loopback addresses to an inherited lowercase no_proxy Delta review: configured noProxy entries were also copied into an inherited lowercase no_proxy, which Bun reads first and matches by suffix; on dev that inherited value always shadowed them, so a configured "localhost" newly let *.localhost bypass a configured proxy. The lowercase value now gains the loopback addresses only. --- src/config/proxy-env.ts | 10 +++++----- structure/config.md | 4 ++-- tests/server/proxy-env.test.ts | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/config/proxy-env.ts b/src/config/proxy-env.ts index 18e79ee6c12..0ef7c2b09ec 100644 --- a/src/config/proxy-env.ts +++ b/src/config/proxy-env.ts @@ -133,13 +133,13 @@ function withNoProxyEntries(existing: string, configured: readonly string[], loo function mergeNoProxyEntries(configured: readonly string[] = [], loopback: readonly string[] = LOOPBACK_NO_PROXY): void { process.env.NO_PROXY = withNoProxyEntries(process.env.NO_PROXY ?? process.env.no_proxy ?? "", configured, loopback); // Bun's native fetch reads a non-empty lowercase no_proxy before NO_PROXY - // (src/codex/catalog/remote.ts), so an inherited one would shadow the entries above. Only - // the loopback addresses join it: Bun matches entries as domain suffixes, and a bare - // "localhost" there would send any *.localhost name past a proxy the inherited value - // previously kept it on. + // (src/codex/catalog/remote.ts), so an inherited one would shadow the loopback entries above. + // Only the loopback addresses join it: Bun matches entries as domain suffixes, and any name + // (a bare "localhost", or a configured noProxy entry the inherited value always shadowed) + // would send its subdomains past a proxy the inherited value kept them on. const inherited = process.env.no_proxy; if (inherited !== undefined && inherited.trim() !== "") { - process.env.no_proxy = withNoProxyEntries(inherited, configured, loopback.filter(host => host !== "localhost")); + process.env.no_proxy = withNoProxyEntries(inherited, [], loopback.filter(host => host !== "localhost")); } } diff --git a/structure/config.md b/structure/config.md index 38dab6961c3..688649a7312 100644 --- a/structure/config.md +++ b/structure/config.md @@ -550,8 +550,8 @@ SOCKS proxy is the only inherited proxy; whenever Bun applies an inherited HTTP( matches by domain suffix, it adds only the loopback addresses (never `localhost`); a proxy-free process is left untouched. The in-process matcher treats a bare `localhost` or IP-literal entry as one host, never a suffix. An inherited non-empty -lowercase `no_proxy`, which Bun fetch reads first with suffix matching, receives the same entries -except a bare `localhost`. When the +lowercase `no_proxy`, which Bun fetch reads first with suffix matching, receives only the loopback +addresses, never a name it would match as a suffix. When the environment no longer selects SOCKS, activation restores the native fetch; removing a saved field alone does not erase inherited process environment variables. diff --git a/tests/server/proxy-env.test.ts b/tests/server/proxy-env.test.ts index 70dfa3d88b7..3785c75f376 100644 --- a/tests/server/proxy-env.test.ts +++ b/tests/server/proxy-env.test.ts @@ -326,10 +326,10 @@ describe("applyProxyEnv", () => { // Bun's native fetch consults a non-empty lowercase no_proxy before NO_PROXY, with suffix // matching, so it gains the loopback addresses but never a bare localhost. process.env.no_proxy = "internal.example"; - applyProxyEnv(configWithProxy("http://proxy.invalid:3128")); + applyProxyEnv(configWithProxy("http://proxy.invalid:3128", "localhost,internal.corp")); // Windows environment names are case-insensitive: there no_proxy IS NO_PROXY. expect(process.env.no_proxy).toBe(process.platform === "win32" - ? "internal.example,localhost,127.0.0.1,::1,[::1]" + ? "internal.example,localhost,internal.corp,127.0.0.1,::1,[::1]" : "internal.example,127.0.0.1,::1,[::1]"); const loopback = new URL("http://127.0.0.1:11434/v1/models"); expect(noProxyMatches(loopback, { no_proxy: process.env.no_proxy })).toBe(true); From 1d44ed502d6b4f271759bbad9d1f82dcd84d4ddb Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 07:11:55 +0900 Subject: [PATCH 30/32] test(oauth): assert a cleared provider set as null, as getAccountSet returns Hosted CI shard 1/4 at 00e5111fce: getAccountSet returns null for a missing provider, so the two new backup tests' toBeUndefined() failed. --- tests/oauth/oauth-store-multi.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/oauth/oauth-store-multi.test.ts b/tests/oauth/oauth-store-multi.test.ts index 31b0baef1e2..9451569b003 100644 --- a/tests/oauth/oauth-store-multi.test.ts +++ b/tests/oauth/oauth-store-multi.test.ts @@ -292,7 +292,7 @@ describe("multi-account auth store", () => { // The management provider-delete route clears credentials this way. await replaceProviderAccountSet("xai", null); - expect(getAccountSet("xai")).toBeUndefined(); + expect(getAccountSet("xai")).toBeNull(); expect(existsSync(backup)).toBe(false); }); @@ -305,7 +305,7 @@ describe("multi-account auth store", () => { const warning = spyOn(console, "warn").mockImplementation(() => {}); try { expect(await removeCredential("xai")).toBe("removed"); - expect(getAccountSet("xai")).toBeUndefined(); + expect(getAccountSet("xai")).toBeNull(); expect(warning.mock.calls.some(call => String(call[0]).includes("could not remove deleted credentials from the legacy credential backup"))).toBe(true); } finally { warning.mockRestore(); From 7cabef8efcefb7a714748080e6aa6f630f544aa7 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 10:18:59 +0900 Subject: [PATCH 31/32] fix: bypass inherited HTTP ALL_PROXY for loopback Cover both ALL_PROXY casings and repair review notes for hub docs, Kiro fallback coverage, and OAuth backup comments. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- docs-site/src/content/docs/guides/remote-hub.md | 2 +- docs-site/src/content/docs/ko/guides/remote-hub.md | 2 +- src/config/proxy-env.ts | 11 ++++++++++- src/oauth/store.ts | 5 +++-- structure/config.md | 5 +++-- tests/ci-workflows/docs-remote-hub-claims.test.ts | 1 + tests/providers/kiro/kiro-wire-estimate.test.ts | 6 ++++++ tests/server/proxy-env.test.ts | 2 ++ 8 files changed, 27 insertions(+), 7 deletions(-) diff --git a/docs-site/src/content/docs/guides/remote-hub.md b/docs-site/src/content/docs/guides/remote-hub.md index 882344538e2..2529cc6a76b 100644 --- a/docs-site/src/content/docs/guides/remote-hub.md +++ b/docs-site/src/content/docs/guides/remote-hub.md @@ -115,7 +115,7 @@ values below are examples: The loopback companion is unauthenticated: every process and OS user on this machine can use the hub's provider credentials and account quota, and can exhaust the shared turn capacity that authenticated remote clients depend on. Do not enable it on a shared or multi-tenant host. If the -host is shared, omit the `unauthenticatedLoopbackListener` command and do not run the hub's local +host is shared, leave the `unauthenticatedLoopbackListener` setting disabled and do not run the hub's local integrations. Binding to `127.0.0.1` means the kernel refuses remote connections, but it does not stop a browser: diff --git a/docs-site/src/content/docs/ko/guides/remote-hub.md b/docs-site/src/content/docs/ko/guides/remote-hub.md index 5034af5d0ce..f4ec3fea808 100644 --- a/docs-site/src/content/docs/ko/guides/remote-hub.md +++ b/docs-site/src/content/docs/ko/guides/remote-hub.md @@ -52,7 +52,7 @@ ocx sync 데이터 리스너는 허브의 Tailscale 주소에 바인드하고, 허브 자신의 프로세스가 같은 포트를 자격 증명 없이 쓸 수 있도록 루프백 companion을 켜고, 관리 평면은 따로 공개합니다. 아래 값은 예시입니다. :::danger[전용 단일 테넌트 호스트를 사용하세요] -루프백 companion은 인증이 없습니다. 이 머신의 모든 프로세스와 OS 사용자가 허브의 프로바이더 자격 증명과 계정 쿼터를 사용할 수 있고, 인증된 원격 클라이언트가 의존하는 공유 턴 용량을 고갈시킬 수 있습니다. 공유 또는 다중 테넌트 호스트에서는 활성화하지 마세요. 호스트가 공유라면 `unauthenticatedLoopbackListener` 명령을 생략하고 허브의 로컬 통합을 실행하지 마세요. +루프백 companion은 인증이 없습니다. 이 머신의 모든 프로세스와 OS 사용자가 허브의 프로바이더 자격 증명과 계정 쿼터를 사용할 수 있고, 인증된 원격 클라이언트가 의존하는 공유 턴 용량을 고갈시킬 수 있습니다. 공유 또는 다중 테넌트 호스트에서는 활성화하지 마세요. 호스트가 공유라면 `unauthenticatedLoopbackListener` 설정을 비활성화하고 허브의 로컬 통합을 실행하지 마세요. `127.0.0.1`에 바인드하면 커널이 원격 접속을 거부하지만 브라우저까지 막지는 못합니다. 방문한 페이지가 브라우저를 통해 `127.0.0.1`에 접속하게 할 수 있습니다. 그래서 리스너는 일반 루프백 바인드와 같은 `Host`/`Origin` 검사를 적용합니다. ::: diff --git a/src/config/proxy-env.ts b/src/config/proxy-env.ts index 0ef7c2b09ec..09123fd81ad 100644 --- a/src/config/proxy-env.ts +++ b/src/config/proxy-env.ts @@ -113,7 +113,16 @@ const LOOPBACK_ADDRESS_NO_PROXY = ["127.0.0.1", "::1", "[::1]"] as const; // itself a proxy-env mutation callers observe (the lab sandbox rejects these keys). function inheritedLoopbackBypass(): readonly string[] | undefined { const schemeProxy = ["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"].some(key => process.env[key]?.trim()); - if (schemeProxy) return LOOPBACK_ADDRESS_NO_PROXY; + const httpAllProxy = ["ALL_PROXY", "all_proxy"].some(key => { + const value = process.env[key]?.trim(); + if (!value) return false; + try { + return ["http:", "https:"].includes(new URL(value).protocol); + } catch { + return false; + } + }); + if (schemeProxy || httpAllProxy) return LOOPBACK_ADDRESS_NO_PROXY; return socks5ProxyFromEnv() !== undefined ? LOOPBACK_NO_PROXY : undefined; } diff --git a/src/oauth/store.ts b/src/oauth/store.ts index 2d2507055aa..a5b57ebcadf 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -460,8 +460,9 @@ function backupLegacyOnce(): void { * removed. Entries for other providers stay, so their downgrade recovery survives; the file * goes once nothing is left. Account deletion drops the provider's whole legacy entry, since * a refreshed token cannot be matched to the account it came from. A backup entry that is - * not a regular file (a symlink, a directory) is removed, never followed or rewritten, and - * the rewrite replaces the entry itself. Best-effort like the create path: this runs after + * not a regular file is never followed or rewritten; a symlink is removed, while a directory + * is left in place with a warning. A regular backup is replaced atomically when providers + * remain and removed when none do. Best-effort like the create path: this runs after * persist, so a failure must not report a failed logout for an account that is already * gone; it warns instead. A stale uninstall-manifest entry is harmless: * removeOwnedConfigState skips paths that no longer exist. diff --git a/structure/config.md b/structure/config.md index 688649a7312..8cbd61ee3ac 100644 --- a/structure/config.md +++ b/structure/config.md @@ -546,8 +546,9 @@ configuration. An explicit SOCKS5 or SOCKS5h URL selects ALL_PROXY and removes stale scheme-proxy variables; HTTP(S) settings retain their existing environment precedence. Activation keeps the existing Windows auto-discovery path and loopback NO_PROXY entries; the no-configured-proxy return merges all of them only when an inherited -SOCKS proxy is the only inherited proxy; whenever Bun applies an inherited HTTP(S) proxy, which it -matches by domain suffix, it adds only the loopback addresses (never `localhost`); a proxy-free +SOCKS proxy is the only inherited proxy; whenever Bun applies an inherited HTTP(S) scheme proxy +or HTTP(S) `ALL_PROXY`/`all_proxy`, it matches by domain suffix, so activation adds only the +loopback addresses (never `localhost`); a proxy-free process is left untouched. The in-process matcher treats a bare `localhost` or IP-literal entry as one host, never a suffix. An inherited non-empty lowercase `no_proxy`, which Bun fetch reads first with suffix matching, receives only the loopback diff --git a/tests/ci-workflows/docs-remote-hub-claims.test.ts b/tests/ci-workflows/docs-remote-hub-claims.test.ts index 65636d59e1e..b4ca0e4f6ed 100644 --- a/tests/ci-workflows/docs-remote-hub-claims.test.ts +++ b/tests/ci-workflows/docs-remote-hub-claims.test.ts @@ -142,6 +142,7 @@ describe("the one-port hub recipe", () => { const callout = source.slice(opened, opened + (closing?.index ?? 0)); expect(callout, locale).toContain(dedicated); expect(callout, locale).toContain(sharedHost); + expect(callout, locale).not.toMatch(/`unauthenticatedLoopbackListener` (?:command|명령)/); // The same unauthenticated surface is offered again by the ported form; the warning // must reach that command too, or a reader following only that section misses it. const ported = source.indexOf('"port":10104'); diff --git a/tests/providers/kiro/kiro-wire-estimate.test.ts b/tests/providers/kiro/kiro-wire-estimate.test.ts index 4d0c8b4d596..facd9d2c1c9 100644 --- a/tests/providers/kiro/kiro-wire-estimate.test.ts +++ b/tests/providers/kiro/kiro-wire-estimate.test.ts @@ -41,4 +41,10 @@ describe("kiro wire token estimate", () => { // The old path fell back to the bare "kiro" id for an empty model; both select the Kiro ratio. expect(estimateKiroWireTokens(korean, "")).toBe(expectedFor("kiro")); }); + + test("empty model id uses the Kiro ratio for Latin text", () => { + const latin = "command code".repeat(30); + expect(estimateKiroWireTokens(latin, "")) + .toBe(Math.ceil(estimateTokens(latin, "kiro") * KIRO_LATIN_WIRE_EXPANSION)); + }); }); diff --git a/tests/server/proxy-env.test.ts b/tests/server/proxy-env.test.ts index 3785c75f376..6a84287f9f7 100644 --- a/tests/server/proxy-env.test.ts +++ b/tests/server/proxy-env.test.ts @@ -235,6 +235,8 @@ describe("applyProxyEnv", () => { test.each([ ["an inherited HTTP proxy", { HTTP_PROXY: "http://proxy.invalid:3128" }], ["an inherited HTTPS proxy", { https_proxy: "http://proxy.invalid:3128" }], + ["an inherited HTTP ALL_PROXY", { ALL_PROXY: "http://proxy.invalid:3128" }], + ["an inherited lowercase HTTPS all_proxy", { all_proxy: "https://proxy.invalid:3128" }], ] as const)("adds only loopback addresses for %s and no config.proxy", (_label, inherited) => { // Bun applies an inherited HTTP(S) proxy itself and matches NO_PROXY entries as domain // suffixes, so adding "localhost" there would also send any *.localhost name direct. From 4e13ee5d7841b7ac1e9f64323e881dea41bf8f55 Mon Sep 17 00:00:00 2001 From: JUN Date: Wed, 23 Sep 2026 10:33:27 +0900 Subject: [PATCH 32/32] fix: keep localhost direct with mixed inherited proxies Force native direct fetch for exact localhost when the inherited SOCKS wrapper is active, and cover both ALL_PROXY casing combinations. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> --- src/lib/proxy-env.ts | 7 +++++++ structure/config.md | 4 +++- tests/server/proxy-env.test.ts | 17 +++++++++++------ 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/lib/proxy-env.ts b/src/lib/proxy-env.ts index 2ff8f89a94f..7af38f56535 100644 --- a/src/lib/proxy-env.ts +++ b/src/lib/proxy-env.ts @@ -221,6 +221,13 @@ export function configuredOutboundFetch( } catch { return base!(input, init); } + // A mixed inherited SOCKS/HTTP ALL_PROXY environment cannot put bare "localhost" in + // NO_PROXY: Bun would also bypass its HTTP proxy for app.localhost. Keep the name exact + // here and force native fetch direct so the opposite-case HTTP proxy cannot take over. + if (proxy && explicitProxy === undefined && (url.protocol === "http:" || url.protocol === "https:") + && normalizeProxyHostname(url.hostname) === "localhost") { + return base!(input, { ...init, proxy: false } as ProxyCapableRequestInit); + } if (proxy && (url.protocol === "http:" || url.protocol === "https:") && (explicitProxy !== undefined || !noProxyMatches(url))) { return socks5Fetch(input, init, proxy); } diff --git a/structure/config.md b/structure/config.md index 8cbd61ee3ac..20c03893201 100644 --- a/structure/config.md +++ b/structure/config.md @@ -550,7 +550,9 @@ SOCKS proxy is the only inherited proxy; whenever Bun applies an inherited HTTP( or HTTP(S) `ALL_PROXY`/`all_proxy`, it matches by domain suffix, so activation adds only the loopback addresses (never `localhost`); a proxy-free process is left untouched. The in-process -matcher treats a bare `localhost` or IP-literal entry as one host, never a suffix. An inherited non-empty +matcher treats a bare `localhost` or IP-literal entry as one host, never a suffix. When opposite-case +`ALL_PROXY` and `all_proxy` provide SOCKS and HTTP(S) together, the SOCKS wrapper forces an exact +`localhost` request direct while keeping the address-only environment bypass. An inherited non-empty lowercase `no_proxy`, which Bun fetch reads first with suffix matching, receives only the loopback addresses, never a name it would match as a suffix. When the environment no longer selects SOCKS, activation diff --git a/tests/server/proxy-env.test.ts b/tests/server/proxy-env.test.ts index 6a84287f9f7..e853a7a6014 100644 --- a/tests/server/proxy-env.test.ts +++ b/tests/server/proxy-env.test.ts @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { createServer } from "node:http"; import { createServer as createTcpServer } from "node:net"; import { applyProxyEnv } from "../../src/config"; -import { configuredOutboundFetch, noProxyMatches, resolveProxyRoute, configureSocks5Fetch } from "../../src/lib/proxy-env"; +import { configuredOutboundFetch, noProxyMatches, resolveProxyRoute, configureSocks5Fetch, type ProxyCapableRequestInit } from "../../src/lib/proxy-env"; import type { OcxConfig } from "../../src/types"; const PROXY_ENV_KEYS = ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY", "http_proxy", "https_proxy", "all_proxy", "no_proxy", "OCX_TEST_PROXY_REF", "OCX_TEST_NO_PROXY_REF"] as const; @@ -293,9 +293,11 @@ describe("applyProxyEnv", () => { }); test.each([ - ["alone", {}, "http://localhost:11434/v1/models", "localhost,127.0.0.1,::1,[::1]"], - ["beside an inherited HTTP proxy", { HTTP_PROXY: "http://proxy.invalid:3128" }, "http://127.0.0.1:11434/v1/models", "127.0.0.1,::1,[::1]"], - ] as const)("an inherited SOCKS proxy %s: loopback goes direct, *.localhost stays on SOCKS", async (_label, inherited, loopbackUrl, expectedNoProxy) => { + ["alone", "ALL_PROXY", {}, "http://localhost:11434/v1/models", "localhost,127.0.0.1,::1,[::1]", false], + ["beside an inherited HTTP proxy", "ALL_PROXY", { HTTP_PROXY: "http://proxy.invalid:3128" }, "http://127.0.0.1:11434/v1/models", "127.0.0.1,::1,[::1]", false], + ["with lowercase HTTP all_proxy", "ALL_PROXY", { all_proxy: "http://proxy.invalid:3128" }, "http://localhost:11434/v1/models", "127.0.0.1,::1,[::1]", true], + ["with uppercase HTTP ALL_PROXY", "all_proxy", { ALL_PROXY: "http://proxy.invalid:3128" }, "http://localhost:11434/v1/models", "127.0.0.1,::1,[::1]", true], + ] as const)("an inherited SOCKS proxy %s: loopback goes direct, *.localhost stays on SOCKS", async (_label, socksKey, inherited, loopbackUrl, expectedNoProxy, forcedDirect) => { const refusing = createTcpServer(socket => socket.destroy()); await new Promise((resolve, reject) => { refusing.once("error", reject); @@ -304,19 +306,22 @@ describe("applyProxyEnv", () => { const address = refusing.address(); if (!address || typeof address === "string") throw new Error("proxy fixture did not bind a TCP port"); // socks5h: the proxy resolves the name, so the fixture fails the request without local DNS. - process.env.ALL_PROXY = `socks5h://127.0.0.1:${address.port}`; + process.env[socksKey] = `socks5h://127.0.0.1:${address.port}`; Object.assign(process.env, inherited); applyProxyEnv(configWithProxy()); // Beside an HTTP(S) proxy Bun reads NO_PROXY too, with suffix matching, so no bare localhost. expect(process.env.NO_PROXY).toBe(expectedNoProxy); let directCalls = 0; - const direct = async () => { + let directProxy: string | false | undefined; + const direct = async (_input: RequestInfo | URL, init?: RequestInit) => { directCalls += 1; + directProxy = (init as ProxyCapableRequestInit | undefined)?.proxy; return new Response("direct"); }; try { expect(await (await configuredOutboundFetch(loopbackUrl, undefined, direct)).text()).toBe("direct"); expect(directCalls).toBe(1); + if (forcedDirect) expect(directProxy).toBe(false); await expect(configuredOutboundFetch("http://app.localhost:11434/v1/models", undefined, direct)).rejects.toThrow(); expect(directCalls).toBe(1); } finally {