diff --git a/docs-site/src/content/docs/guides/remote-hub.md b/docs-site/src/content/docs/guides/remote-hub.md index d0399c213cf..2529cc6a76b 100644 --- a/docs-site/src/content/docs/guides/remote-hub.md +++ b/docs-site/src/content/docs/guides/remote-hub.md @@ -111,6 +111,18 @@ 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 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, 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: +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 ocx config set runtimeRole hub ocx config set hostname 100.64.0.10 @@ -216,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 0b1cb95941c..f4ec3fea808 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,12 @@ ocx sync 데이터 리스너는 허브의 Tailscale 주소에 바인드하고, 허브 자신의 프로세스가 같은 포트를 자격 증명 없이 쓸 수 있도록 루프백 companion을 켜고, 관리 평면은 따로 공개합니다. 아래 값은 예시입니다. +:::danger[전용 단일 테넌트 호스트를 사용하세요] +루프백 companion은 인증이 없습니다. 이 머신의 모든 프로세스와 OS 사용자가 허브의 프로바이더 자격 증명과 계정 쿼터를 사용할 수 있고, 인증된 원격 클라이언트가 의존하는 공유 턴 용량을 고갈시킬 수 있습니다. 공유 또는 다중 테넌트 호스트에서는 활성화하지 마세요. 호스트가 공유라면 `unauthenticatedLoopbackListener` 설정을 비활성화하고 허브의 로컬 통합을 실행하지 마세요. + +`127.0.0.1`에 바인드하면 커널이 원격 접속을 거부하지만 브라우저까지 막지는 못합니다. 방문한 페이지가 브라우저를 통해 `127.0.0.1`에 접속하게 할 수 있습니다. 그래서 리스너는 일반 루프백 바인드와 같은 `Host`/`Origin` 검사를 적용합니다. +::: + ```bash ocx config set runtimeRole hub ocx config set hostname 100.64.0.10 @@ -115,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/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/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/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/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/config/proxy-env.ts b/src/config/proxy-env.ts index adbfe5f6176..09123fd81ad 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,6 +98,60 @@ 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 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 { + const schemeProxy = ["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"].some(key => process.env[key]?.trim()); + 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; +} + +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, ...loopback]) { + const key = host.toLowerCase(); + if (!seen.has(key)) { + entries.push(host); + seen.add(key); + } + } + return entries.join(","); +} + +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 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, [], loopback.filter(host => host !== "localhost")); + } +} + /** * 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 +184,12 @@ export function applyProxyEnvWith( let proxy = typeof rawProxy === "string" ? resolveEnvValue(rawProxy) : undefined; if (!proxy) { if (rawProxy !== undefined) warnProxyConfigDiscardOnce("proxy"); + // 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. + const loopback = inheritedLoopbackBypass(); + if (loopback) mergeNoProxyEntries([], loopback); configureSocks5Fetch(); return; } @@ -178,9 +238,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 +257,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/src/lib/crash-guard.ts b/src/lib/crash-guard.ts index e7661f5bcbc..ad9188857b3 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) @@ -176,17 +178,53 @@ 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); + }); +} + +/** + * 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 { + 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 { 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; @@ -306,6 +344,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/src/lib/proxy-env.ts b/src/lib/proxy-env.ts index bf2e70b9175..7af38f56535 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, @@ -211,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/src/lib/token-estimate.ts b/src/lib/token-estimate.ts index bf507e1cf27..cd3872936cd 100644 --- a/src/lib/token-estimate.ts +++ b/src/lib/token-estimate.ts @@ -142,12 +142,27 @@ 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. + * `latin` and `cjk` are non-negative integer character counts, as produced from a string. + */ +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/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/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/src/oauth/store.ts b/src/oauth/store.ts index a7ea6eaa2df..a5b57ebcadf 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -6,6 +6,9 @@ * 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. + * 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 @@ -17,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, 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"; @@ -433,9 +437,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. @@ -448,6 +454,53 @@ function backupLegacyOnce(): void { } catch { /* best-effort */ } } +/** + * 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 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. + */ +function scrubLegacyBackup(providers: readonly string[]): void { + const backup = `${getAuthStorePath()}.pre-multiauth`; + try { + const remaining = readLegacyBackupEntries(backup); + for (const provider of providers) delete remaining[provider]; + if (Object.keys(remaining).length > 0) atomicWriteFileNoFollowUnclaimed(backup, `${JSON.stringify(remaining, null, 2)}\n`); + else unlinkSync(backup); + } catch (error) { + if (errorCode(error) !== "ENOENT") { + 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 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"; } @@ -712,7 +765,7 @@ 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; 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, { @@ -722,6 +775,9 @@ export function mutateStore(fn:(store:AuthStore)=>T|Promise, retainedValue accountIds: set.accounts.map(account => account.id), }])); const result = await fn(store); + // 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)])) { @@ -743,6 +799,7 @@ export function mutateStore(fn:(store:AuthStore)=>T|Promise, retainedValue } } persist(store); + if (scrubbedProviders.length > 0) scrubLegacyBackup(scrubbedProviders); for (const provider of changedProviders) publishAccountSelection(provider, "oauth"); return result; }finally{guard.release();}}, retainedValues, options?.waitMs); @@ -886,7 +943,7 @@ export async function removeCredential(provider: string): Promise<"removed" | "n } set.activeAccountId = set.accounts[0]!.id; return "removed" as const; - }, [provider]); + }, [provider], { scrubLegacyBackup: result => result === "removed" ? [provider] : [] }); } // --------------------------------------------------------------------------- @@ -1029,19 +1086,26 @@ 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], { scrubLegacyBackup: removed => removed ? [provider] : [] }); 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 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, 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, @@ -1053,7 +1117,8 @@ export async function replaceProviderAccountSet( ...(account.addedAt !== undefined ? { addedAt: account.addedAt } : {}), })), }; - }, [provider, set]); + return false; + }, [provider, set], { scrubLegacyBackup: cleared => cleared ? [provider] : [] }); } export type ProviderCredentialRekeyOutcome = "moved" | "absent" | "conflict"; diff --git a/structure/config.md b/structure/config.md index 049166a52ae..20c03893201 100644 --- a/structure/config.md +++ b/structure/config.md @@ -414,13 +414,16 @@ 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 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. @@ -542,7 +545,17 @@ 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 all of them only when an inherited +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. 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 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/structure/overview.md b/structure/overview.md index e75cf6430ee..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). 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/structure/providers-and-adapters.md b/structure/providers-and-adapters.md index bf4271d54fe..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. 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/ci-workflows/docs-remote-hub-claims.test.ts b/tests/ci-workflows/docs-remote-hub-claims.test.ts index e6812f31fb5..b4ca0e4f6ed 100644 --- a/tests/ci-workflows/docs-remote-hub-claims.test.ts +++ b/tests/ci-workflows/docs-remote-hub-claims.test.ts @@ -123,6 +123,35 @@ 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", "dedicated single-tenant host", "Do not enable"], + ["ko", KO_GUIDE, "모든 프로세스와 OS 사용자", "공유 또는 다중 테넌트 호스트에서는 활성화하지 마세요", "전용 단일 테넌트 호스트", "활성화하지 마세요"], + ] as const; + 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 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); + 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'); + expect(ported, locale).toBeGreaterThan(-1); + const after = source.slice(ported, ported + 600); + expect(after, locale).toMatch(/unauthenticated|인증/); + } + }); + 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(); 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", 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/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) => { diff --git a/tests/oauth/oauth-store-multi.test.ts b/tests/oauth/oauth-store-multi.test.ts index 32fcf293f84..9451569b003 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"; @@ -213,6 +213,180 @@ 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("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("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")).toBeNull(); + 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")).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(); + } + }); + + 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.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 diff --git a/tests/providers/command-code-provider.test.ts b/tests/providers/command-code-provider.test.ts index 692bbf45586..337bd557742 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,99 @@ 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[] = []; + 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); + }; + 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" }); + // 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]!; + + // 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")); + 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({ 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..facd9d2c1c9 --- /dev/null +++ b/tests/providers/kiro/kiro-wire-estimate.test.ts @@ -0,0 +1,50 @@ +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); + }); + + 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; + expect(latin).toBe(0); + 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")); + }); + + 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 340454dcfb6..e853a7a6014 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 { 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; @@ -215,14 +216,144 @@ 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 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(); expect(process.env.HTTPS_PROXY).toBeUndefined(); + 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" }], + ["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. + // 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"); }); + 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.each(["ALL_PROXY", "all_proxy"])("inherited SOCKS %s still owns non-loopback fetches", async key => { + // 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. The direct fallback must NOT be + // consulted — if it were, the bypass leaked. + let directCalls = 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.each([ + ["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); + 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[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; + 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 { + 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, 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", "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,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); + }); + + 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]"); diff --git a/tests/service/crash-guard.test.ts b/tests/service/crash-guard.test.ts index b09f40029a7..2ed66d1fd65 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", () => { @@ -118,6 +122,77 @@ 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); + }); + + 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); + }; + 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 = 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"); + }); + }); + + 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", () => { const a = new TypeError("null is not an object (evaluating 'x.y')"); a.stack = "TypeError: ...\n at (native:1:11)";