From 6dfe3e4c24e48336b6dd2161ffb161d0a21f3457 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Wed, 14 Oct 2026 00:30:16 +0900 Subject: [PATCH 1/5] fix(kiro): accept authorized social device grants Carries #7053. Preserve token, profile, principal, and HTTP validation. Co-authored-by: Veritas-7 <234569343+Veritas-7@users.noreply.github.com> --- .../src/content/docs/guides/providers.md | 2 +- src/oauth/kiro-device-login.ts | 3 ++- structure/providers/kiro.md | 2 +- .../providers/kiro/kiro-device-social.test.ts | 21 +++++++++++++++++++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/docs-site/src/content/docs/guides/providers.md b/docs-site/src/content/docs/guides/providers.md index eaf93b6b538..28e219f9fb5 100644 --- a/docs-site/src/content/docs/guides/providers.md +++ b/docs-site/src/content/docs/guides/providers.md @@ -512,7 +512,7 @@ an ambiguous token selection), when `KIROCLI_DB_PATH` / `KIRO_CLI_DB_FILE` redir from the live CLI store, or when an existing primary CLI database has no recognized token row. Repair or remove the unreadable database under the normal `kiro-cli` data path, unset those import selectors, then retry. Signing in from a machine with no existing `kiro-cli` session is unaffected. -The native dashboard choices are add-only and do not sign out `kiro-cli`. A device dialog shows the code and verification destination. Only recognized Kiro or Builder ID hosts are opened as links; an unexpected destination is shown as copyable text for review. Closing the dialog sends cancellation and leaves a bounded background status check to reconcile a commit already in progress. A stalled status response is retried; exhausting the flow deadline produces the neutral ended outcome rather than claiming success. +The native dashboard choices are add-only and do not sign out `kiro-cli`. A device dialog shows the code and verification destination. Only recognized Kiro or Builder ID hosts are opened as links; an unexpected destination is shown as copyable text for review. Closing the dialog sends cancellation and leaves a bounded background status check to reconcile a commit already in progress. A stalled status response is retried; exhausting the flow deadline produces the neutral ended outcome rather than claiming success. Google and GitHub approval completes when Kiro reports `authorized` (or the legacy `approved` status) with valid tokens and a profile ARN. The account list marks Kiro accounts excluded from automatic selection with a reason, when available. ## 3. API-key catalog diff --git a/src/oauth/kiro-device-login.ts b/src/oauth/kiro-device-login.ts index d57fc1dfbbf..9db5f28c296 100644 --- a/src/oauth/kiro-device-login.ts +++ b/src/oauth/kiro-device-login.ts @@ -250,9 +250,10 @@ async function poll(flow: Flow, principal: KiroDevicePrincipal): Promise { expect(JSON.stringify(done)).not.toMatch(/private-access|private-refresh|private-device/); }); + test("accepts the authorized status the social poll returns after a pending reply", async () => { + fixture = kiroDeviceFixture(); + const polls = [ + { status: "authorization_pending", accessToken: null, refreshToken: null, profileArn: null }, + { status: "authorized", identityProvider: "Google", accessToken: "private-access", refreshToken: "private-refresh", profileArn }, + ]; + fixture.setPost(async url => url.endsWith("/authorization") ? response(socialAuthorization) : response(polls.shift()!)); + const start = await startKiroDeviceLogin("google", "admin-token"); + fixture.advance(5_000); + expect((await statusKiroDeviceLogin(start.flowId, "admin-token"))?.state).toBe("pending"); + expect(getAccountSet("kiro")).toBeNull(); + fixture.advance(5_000); + const done = await statusKiroDeviceLogin(start.flowId, "admin-token"); + expect(done?.state).toBe("done"); + expect(polls).toHaveLength(0); + const account = getAccountSet("kiro")?.accounts[0]; + expect(account?.credential.refresh).toBe("private-refresh"); + expect(account?.credential.kiro?.profileArn).toBe(profileArn); + expect(JSON.stringify(done)).not.toMatch(/private-access|private-refresh|private-device/); + }); + test("unknown approval status or malformed profile persists nothing", async () => { for (const approval of [ { status: "strange", accessToken: "a", refreshToken: "r", profileArn }, From a61d5a025b3a6b98ad9e80a9cd0d4dbb4218862b Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Wed, 14 Oct 2026 00:30:16 +0900 Subject: [PATCH 2/5] fix(kiro): quarantine exact desktop bad-credentials refusals Carries #7054. Keep AWS SSO and other status/message pairs retryable. Co-authored-by: Veritas-7 <234569343+Veritas-7@users.noreply.github.com> --- .../src/content/docs/guides/providers.md | 4 ++ src/oauth/kiro.ts | 8 ++- structure/providers-and-adapters.md | 2 +- .../oauth-kiro-refresh-attention.test.ts | 51 ++++++++++++++++++- 4 files changed, 62 insertions(+), 3 deletions(-) diff --git a/docs-site/src/content/docs/guides/providers.md b/docs-site/src/content/docs/guides/providers.md index 28e219f9fb5..87f7a28b3e6 100644 --- a/docs-site/src/content/docs/guides/providers.md +++ b/docs-site/src/content/docs/guides/providers.md @@ -473,6 +473,10 @@ a later successful automatic refresh also clears the attention state. This statu classify the refresh grant as revoked or disable future refresh attempts. Network and server errors do not trigger this status. +For desktop-auth accounts, an HTTP 401 with exactly `Bad credentials` requires a new login +and immediately marks the rejected credential generation as needing re-authentication. +Other messages, statuses, and AWS SSO responses do not trigger this desktop-specific rule. + ### Kiro credential import The dashboard offers native Builder ID, Google, and GitHub device login without Kiro CLI. diff --git a/src/oauth/kiro.ts b/src/oauth/kiro.ts index 42ecff1340f..9d1ea73237a 100644 --- a/src/oauth/kiro.ts +++ b/src/oauth/kiro.ts @@ -45,6 +45,10 @@ const KIRO_TERMINAL_REFRESH_ERRORS = new Set([ "access_denied", "expired_token", ]); +// The desktop refresh endpoint rejects a dead refresh token with HTTP 401 and a bare +// `{"message":"Bad credentials"}` body, without an OAuth `error` code. Only that exact +// pair is a terminal verdict; it is surfaced under this fixed code, never the raw text. +const KIRO_DESKTOP_BAD_CREDENTIALS = "bad_credentials"; interface ImportedKiroToken { access: string; @@ -545,12 +549,14 @@ async function kiroTokenRefreshError(response: Response, awsSsoRefresh = false): let oauthError: string | undefined; let refreshAttention = false; try { - const payload = await response.json() as { error?: unknown }; + const payload = await response.json() as { error?: unknown; message?: unknown }; // This input-error code is attention evidence only for our AWS SSO refresh request, // never a terminal grant verdict. Do not surface arbitrary upstream descriptions. refreshAttention = awsSsoRefresh && response.status === 400 && payload.error === "invalid_request"; if (typeof payload.error === "string" && KIRO_TERMINAL_REFRESH_ERRORS.has(payload.error)) { oauthError = payload.error; + } else if (!awsSsoRefresh && response.status === 401 && payload.message === "Bad credentials") { + oauthError = KIRO_DESKTOP_BAD_CREDENTIALS; } } catch { // Error bodies are untrusted and intentionally excluded from the surfaced message. diff --git a/structure/providers-and-adapters.md b/structure/providers-and-adapters.md index 7d3aa123b26..c7c9665547a 100644 --- a/structure/providers-and-adapters.md +++ b/structure/providers-and-adapters.md @@ -140,7 +140,7 @@ are tried before the failure reaches `src/oauth/index.ts`. `src/oauth/store.ts` and operator pause. Once that generation's access token expires, account summaries project `needsReauth: true` and the active login projects `loggedIn: false`. `src/oauth/health.ts` uses the same generation/expiry predicate with its supplied observation time, so account-list health and CLI diagnostics project `reauth_required` with `refresh_failed`; `ocx doctor` warns with a login action. The internal terminal flag stays clear so a later refresh remains eligible; successful refresh or credential replacement -clears the evidence. Desktop input errors, network failures, and 5xx responses create no evidence. No upstream description or credential metadata enters the status response. +clears the evidence. Desktop input errors, network failures, and 5xx responses create no evidence. No upstream description or credential metadata enters the status response. The desktop refresh endpoint answers a dead refresh token with HTTP 401 and a bare `{"message":"Bad credentials"}` body; only that exact pair on the desktop endpoint is a terminal grant refusal, surfaced as the fixed `bad_credentials` code and quarantined like an allowlisted OAuth error. Kiro's account quota cache persists quota and an optional exhaustion verdict under one opaque account key and a non-secret login identity. Hydration admits only matching live accounts and bounds quota and verdict independently by reset and ten-minute TTL; a failed probe keeps the same-login last-good display bar. The protected OAuth store rotates diff --git a/tests/oauth/oauth-kiro-refresh-attention.test.ts b/tests/oauth/oauth-kiro-refresh-attention.test.ts index fa0e17d42d7..c76dfb9cfb0 100644 --- a/tests/oauth/oauth-kiro-refresh-attention.test.ts +++ b/tests/oauth/oauth-kiro-refresh-attention.test.ts @@ -3,7 +3,7 @@ import { Database } from "bun:sqlite"; import { mkdtempSync, readFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { getLoginStatus, getValidAccessTokenForAccount, OAUTH_PROVIDERS, refreshGenericAccountWithLock } from "../../src/oauth"; +import { getLoginStatus, getValidAccessTokenForAccount, OAuthLoginRequiredError, OAUTH_PROVIDERS, refreshGenericAccountWithLock } from "../../src/oauth"; import { credentialGeneration, getAccountSet, saveAccountCredential, saveCredential, setAccountPaused } from "../../src/oauth/store"; import type { OAuthCredentials } from "../../src/oauth/types"; import { handleManagementAPI } from "../../src/server/management-api"; @@ -114,6 +114,55 @@ describe("Kiro AWS SSO refresh attention", () => { expect(getAccountSet("kiro")!.accounts[0]!.needsReauth).toBeUndefined(); }); + test("desktop 401 Bad credentials quarantines the rejected generation", async () => { + stored = { ...stored, kiro: { profileArn, authType: "kiro_desktop" } }; + await saveAccountCredential("kiro", accountId, stored); + const urls: string[] = []; + globalThis.fetch = (async input => { + urls.push(String(input)); + return Response.json({ message: "Bad credentials" }, { status: 401 }); + }) as typeof fetch; + const error = await refresh().then(() => undefined, reason => reason); + expect(error).toBeInstanceOf(OAuthLoginRequiredError); + expect(urls).toEqual(["https://prod.us-east-1.auth.desktop.kiro.dev/refreshToken"]); + expect(getAccountSet("kiro")!.accounts[0]!.needsReauth).toBe(true); + expect(getLoginStatus("kiro").loggedIn).toBe(false); + }); + + test("desktop Bad credentials during matching CLI recovery quarantines the account", async () => { + seedCli(profileArn, false); + const urls: string[] = []; + globalThis.fetch = (async input => { + const url = String(input); + urls.push(url); + return url.endsWith("/refreshToken") + ? Response.json({ message: "Bad credentials" }, { status: 401 }) + : Response.json(rejection, { status: 400 }); + }) as typeof fetch; + const error = await refresh().then(() => undefined, reason => reason); + expect(urls).toEqual(["https://oidc.us-east-1.amazonaws.com/token", "https://prod.us-east-1.auth.desktop.kiro.dev/refreshToken"]); + expect(error).toBeInstanceOf(OAuthLoginRequiredError); + expect(getAccountSet("kiro")!.accounts[0]!.needsReauth).toBe(true); + }); + + for (const [label, status, payload, authType] of [ + ["desktop 401 with another message", 401, { message: "Unauthorized" }, "kiro_desktop"], + ["desktop 400 Bad credentials", 400, { message: "Bad credentials" }, "kiro_desktop"], + ["AWS SSO 401 Bad credentials", 401, { message: "Bad credentials" }, "aws_sso_oidc"], + ] as const) { + test(`${label} does not quarantine`, async () => { + if (authType === "kiro_desktop") { + stored = { ...stored, kiro: { profileArn, authType } }; + await saveAccountCredential("kiro", accountId, stored); + } + rejectRefresh(status, payload); + const error = await refresh().then(() => undefined, reason => reason); + expect(error).toBeDefined(); + expect(error).not.toBeInstanceOf(OAuthLoginRequiredError); + expect(getAccountSet("kiro")!.accounts[0]!.needsReauth).toBeUndefined(); + }); + } + for (const [label, status, payload] of [ ["5xx", 503, rejection], ["different input error", 400, { error: "invalid_client" }], ["unallowlisted error text", 400, { error: "invalid_request arbitrary-secret-text" }], From 383a9208751650a6a18aa4ae00b981a66c0c3f1c Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Wed, 14 Oct 2026 00:30:59 +0900 Subject: [PATCH 3/5] fix(chatgpt): preserve launcher when app inspection fails Carries #7055. Report unknown inspection state and refuse an unconfirmed restart. Co-authored-by: lcxhh --- .../content/docs/guides/chatgpt-desktop.md | 7 +++ src/cli/chatgpt-command.ts | 49 +++++++++++++------ structure/clients/chatgpt-desktop.md | 7 ++- .../desktop-app-server-shim-launcher.test.ts | 33 ++++++++++++- .../desktop-app-server-shim-command-child.ts | 19 +++++-- 5 files changed, 95 insertions(+), 20 deletions(-) diff --git a/docs-site/src/content/docs/guides/chatgpt-desktop.md b/docs-site/src/content/docs/guides/chatgpt-desktop.md index 761862d2ea3..887493f1935 100644 --- a/docs-site/src/content/docs/guides/chatgpt-desktop.md +++ b/docs-site/src/content/docs/guides/chatgpt-desktop.md @@ -95,6 +95,13 @@ This integration installs no certificate, network listener, PAC, or background watcher. It does not log the app's messages or environment. Status reports whether the running ChatGPT bundle process carries the expected launcher override. +If app discovery or process inspection fails, `status` reports `unknown` rather +than `not running`. `launch` and `restore` refuse to reopen an app whose exit +cannot be confirmed; restore keeps the launcher. Resolve the inspection error +and retry. Launch checks before writing its launcher, but a later inspection can +still fail after that atomic update; the commands do not guarantee an all-or-nothing +restart transaction. + ## Failure behavior and known limits When the platform is not macOS, the OpenCodex runtime is missing, or the filter diff --git a/src/cli/chatgpt-command.ts b/src/cli/chatgpt-command.ts index f870e66ff72..3006f44638c 100644 --- a/src/cli/chatgpt-command.ts +++ b/src/cli/chatgpt-command.ts @@ -20,7 +20,7 @@ const USAGE = `Usage (experimental, macOS only): function run(command: string, args: string[]) { const result = spawnSync(command, args, { encoding: "utf8", timeout: 5000 }); - return { ok: result.status === 0, output: `${result.stdout ?? ""}${result.stderr ?? ""}`.trim() }; + return { ok: result.status === 0, status: result.status, output: `${result.stdout ?? ""}${result.stderr ?? ""}`.trim() }; } /** @@ -42,11 +42,12 @@ function chatgptDesktopIssueInFile(): string | null { * desktop restart adapter does. "ChatGPT" is a display name another app can share, so quitting, * relaunching and the binary path all key on this verified bundle rather than on the name. */ -function discoverApp(): DesktopAppInstall | null { +function discoverApp(): DesktopAppInstall | null | undefined { try { return darwinDesktopAppAdapter.discover(darwinDefaultExec); } catch { - return null; + // An inspection error is not evidence that the app is uninstalled. + return undefined; } } @@ -55,14 +56,20 @@ function discoverApp(): DesktopAppInstall | null { * `-a` keeps ancestors in the match: when ocx runs inside a ChatGPT/Codex session the app is * one of this process's ancestors, and plain `pgrep -x` would report it as not running. */ -function appState(install: DesktopAppInstall, launcher: string): { running: boolean; shim: boolean } { +function appState(install: DesktopAppInstall, launcher: string): { running: boolean | null; shim: boolean | null } { const shell = join(install.root, "Contents", "MacOS", "ChatGPT"); // Only this user's processes: another account's ChatGPT can neither be quit nor relaunched here. const uid = typeof process.getuid === "function" ? process.getuid() : undefined; const pids = run("pgrep", [...(uid === undefined ? [] : ["-U", String(uid)]), "-a", "-x", "ChatGPT"]); - if (!pids.ok) return { running: false, shim: false }; - for (const pid of pids.output.split(/\s+/).filter(value => /^\d+$/.test(value))) { + // pgrep exit 1 means no match; timeout, signal, or command errors are unknown. + if (!pids.ok) return pids.status === 1 + ? { running: false, shim: false } : { running: null, shim: null }; + const candidates = pids.output.split(/\s+/).filter(value => /^\d+$/.test(value)); + if (candidates.length === 0) return { running: null, shim: null }; + let unreadable = false; + for (const pid of candidates) { const command = run("ps", ["eww", "-o", "command=", "-p", pid]); + if (!command.ok || !command.output) unreadable = true; if (command.ok && (command.output === shell || command.output.startsWith(`${shell} `))) { const marker = `CODEX_CLI_PATH=${launcher}`; const start = command.output.indexOf(marker); @@ -70,20 +77,24 @@ function appState(install: DesktopAppInstall, launcher: string): { running: bool && (start + marker.length === command.output.length || command.output[start + marker.length] === " ") }; } } - return { running: false, shim: false }; + return unreadable ? { running: null, shim: null } : { running: false, shim: false }; } /** Adapted from #5947: open ignores new launch settings until the previous app exits. */ async function quitApp(install: DesktopAppInstall, launcher: string): Promise { - if (!appState(install, launcher).running) return true; + const initial = appState(install, launcher).running; + if (initial === false) return true; + if (initial === null) return false; for (let attempt = 0; attempt < 3; attempt++) { run("/usr/bin/osascript", ["-e", `quit app id "${install.id}"`]); for (let poll = 0; poll < 20; poll++) { - if (!appState(install, launcher).running) return true; + const running = appState(install, launcher).running; + if (running === false) return true; + if (running === null) return false; await Bun.sleep(250); } } - return !appState(install, launcher).running; + return appState(install, launcher).running === false; } export async function handleChatgptCommand(args: string[], platform: NodeJS.Platform = process.platform): Promise { @@ -105,16 +116,21 @@ export async function handleChatgptCommand(args: string[], platform: NodeJS.Plat const launcher = chatgptShimLauncherPath(); const install = discoverApp(); if (sub === "status") { - const app = install ? appState(install, launcher) : { running: false, shim: false }; + const app = install ? appState(install, launcher) + : install === undefined ? { running: null, shim: null } : { running: false, shim: false }; const configIssue = config.chatgptDesktop?.appServerShim === true ? null : chatgptDesktopIssueInFile(); const flag = config.chatgptDesktop?.appServerShim === true ? "on" : configIssue ? `off (config.json ${configIssue}; the whole chatgptDesktop block is ignored)` : "off"; console.log(`app-server shim (experimental): ${flag} launcher: ${existsSync(launcher) ? "present" : "absent"} -app: ${install ? (app.running ? "running" : "not running") : "not installed"} -CODEX_CLI_PATH launcher: ${app.shim ? "yes" : "no"}`); +app: ${install === undefined ? "unknown (app discovery failed)" : install ? (app.running === null ? "unknown (process inspection failed)" : app.running ? "running" : "not running") : "not installed"} +CODEX_CLI_PATH launcher: ${app.shim === null ? "unknown" : app.shim ? "yes" : "no"}`); return 0; } + if (install === undefined) { + console.error("ChatGPT app discovery failed; no launcher was changed and no app was opened. Retry after resolving discovery."); + return 1; + } if (!install) { if (sub === "restore") rmSync(launcher, { force: true }); console.error("ChatGPT (com.openai.codex) was not found; install or open it once, then retry."); @@ -143,9 +159,14 @@ CODEX_CLI_PATH launcher: ${app.shim ? "yes" : "no"}`); console.error(`Refusing to ${sub === "launch" ? "launch the shim" : "restore ChatGPT"}: ${untrusted}.`); return 1; } + // Preflight before changing the launcher. quitApp rechecks at the action boundary. + if (appState(install, launcher).running === null) { + console.error("ChatGPT process inspection failed; no launcher was changed and no app was opened. Retry after resolving inspection."); + return 1; + } if (binary) writeChatgptShimLauncher(undefined, binary); if (!(await quitApp(install, launcher))) { - console.error("ChatGPT did not quit; quit it manually and retry."); + console.error("ChatGPT exit could not be confirmed; no new app was opened. Check the running app and retry."); return 1; } // Remove an inherited override too: restore must launch without CODEX_CLI_PATH. diff --git a/structure/clients/chatgpt-desktop.md b/structure/clients/chatgpt-desktop.md index daabf923d97..dca30b1d079 100644 --- a/structure/clients/chatgpt-desktop.md +++ b/structure/clients/chatgpt-desktop.md @@ -61,7 +61,12 @@ app-server binary. It relaunches without that override and removes the launcher `com.openai.codex` bundle is found it removes the launcher, relaunches nothing and exits 1. Status reports the experimental flag, launcher presence, and the verified bundle process's override -without printing its environment. Other platforms reject all three operations. +without printing its environment. Discovery exceptions and failed process inspection +are unknown rather than evidence of an absent app. Launch preflights process inspection +before writing the launcher; both operations refuse to open when exit cannot be confirmed. +Restore keeps the launcher on those failures. A later probe may fail after launch has +atomically updated it; this is not a transaction spanning the launcher and app restart. +Other platforms reject all three operations. The launcher and its executable paths are local code-execution inputs. This integration installs no network listener, CA, PAC or background watcher, and diff --git a/tests/clients/desktop-app-server-shim-launcher.test.ts b/tests/clients/desktop-app-server-shim-launcher.test.ts index 0f89aeea61d..db53fc7fd0b 100644 --- a/tests/clients/desktop-app-server-shim-launcher.test.ts +++ b/tests/clients/desktop-app-server-shim-launcher.test.ts @@ -186,7 +186,7 @@ describe("restore validates the discovered app before any quit or open", () => { expect(result.status).toBe(0); return { ...JSON.parse(result.stdout.trim().split("\n").at(-1)!), stdout: result.stdout, stderr: result.stderr } as { code: number; calls: Array<{ command: string; args: string[]; override?: string }>; - launcherExists: boolean; root: string; shell: string; binary: string; stdout: string; stderr: string; + launcherExists: boolean; launcherContent: string | null; root: string; shell: string; binary: string; stdout: string; stderr: string; }; } finally { removeTreeWithRetry(dir); } }; @@ -199,6 +199,37 @@ describe("restore validates the discovered app before any quit or open", () => { expect(result.calls.filter(call => ["pgrep", "ps", "/usr/bin/osascript", "/usr/bin/open"].includes(call.command))).toEqual([]); }); } + for (const sub of ["launch", "restore"]) { + for (const scenario of [ + { pgrepFails: true }, { pgrepFails: true, pgrepTimeout: true }, + { psFails: true, running: true }, { invalidPgrep: true }, { discoveryFails: true }, + ]) { + test(`${sub} preserves the launcher and refuses unknown app state ${JSON.stringify(scenario)}`, () => { + const result = run({ sub, flag: true, ...scenario }); + expect(result.code).toBe(1); + expect(result.launcherContent).toBe("preserve until restore succeeds"); + expect(result.calls.some(call => ["/usr/bin/osascript", "/usr/bin/open"].includes(call.command))).toBe(false); + expect(result.stderr).not.toContain("private"); + }); + } + } + test("an inspection failure after quit does not authorize open or launcher removal", () => { + const result = run({ running: true, probeFailsAfterQuit: true }); + expect(result.code).toBe(1); + expect(result.launcherContent).toBe("preserve until restore succeeds"); + expect(result.calls.some(call => call.command === "/usr/bin/osascript")).toBe(true); + expect(result.calls.some(call => call.command === "/usr/bin/open")).toBe(false); + }); + for (const scenario of [{ pgrepFails: true }, { discoveryFails: true }, { psFails: true, running: true }]) { + test(`status reports unknown without mutation ${JSON.stringify(scenario)}`, () => { + const result = run({ sub: "status", ...scenario }); + expect(result.code).toBe(0); + expect(result.stdout).toContain("app: unknown"); + expect(result.stdout).toContain("CODEX_CLI_PATH launcher: unknown"); + expect(result.launcherContent).toBe("preserve until restore succeeds"); + expect(result.stdout).not.toContain("private"); + }); + } test("valid restore works with the flag off and no bundled app-server", () => { const result = run({ flag: false, missingBinary: true, running: true }); expect(result.code).toBe(0); diff --git a/tests/helpers/desktop-app-server-shim-command-child.ts b/tests/helpers/desktop-app-server-shim-command-child.ts index 1143cb5dded..72362b026b3 100644 --- a/tests/helpers/desktop-app-server-shim-command-child.ts +++ b/tests/helpers/desktop-app-server-shim-command-child.ts @@ -19,6 +19,7 @@ for (let parent = dirname(root); ; parent = dirname(parent)) { } const calls: Array<{ command: string; args: string[]; override?: string }> = []; let running = scenario.running === true; +let pgrepCalls = 0; const uid = typeof process.getuid === "function" ? process.getuid() : -1; mock.module("node:fs", () => ({ ...fs, lstatSync(path: string, ...args: unknown[]) { @@ -48,8 +49,15 @@ mock.module("node:child_process", () => ({ ...childProcess, spawnSync(command: s status: scenario.trust === "unsigned" ? 1 : 0, stdout: args[0] === "--verify" ? "" : `TeamIdentifier=${scenario.trust === "foreign-team" ? "OTHERTEAM" : "2DC432GLL2"}\n`, stderr: "", }; - if (command === "pgrep") return { status: running ? 0 : 1, stdout: running ? "123" : "", stderr: "" }; - if (command === "ps") return { status: 0, stdout: shell, stderr: "" }; + if (command === "pgrep") { + pgrepCalls++; + if (scenario.pgrepFails || (scenario.probeFailsAfterQuit && pgrepCalls > 2)) return { + status: scenario.pgrepTimeout ? null : 2, stdout: "", stderr: "private fixture diagnostic", + }; + if (scenario.invalidPgrep) return { status: 0, stdout: "not-a-pid", stderr: "" }; + return { status: running ? 0 : 1, stdout: running ? "123" : "", stderr: "" }; + } + if (command === "ps") return { status: scenario.psFails ? 1 : 0, stdout: scenario.psFails ? "" : shell, stderr: "" }; if (command === "/usr/bin/osascript") running = false; if (command === "/usr/bin/open" && scenario.openFails) return { status: 1, stdout: "", stderr: "fixture open failed" }; return { status: 0, stdout: "", stderr: "" }; @@ -60,7 +68,10 @@ mock.module("../../src/config", () => ({ loadConfig: () => ({ chatgptDesktop: { mock.module("../../src/config/diagnostics", () => ({ readConfigFileSnapshot: () => ({ raw: scenario.configRaw }) })); mock.module("../../src/codex/desktop-app/darwin", () => ({ darwinDefaultExec: () => "", - darwinDesktopAppAdapter: { discover: () => scenario.noInstall ? null : { id: "com.openai.codex", root, relaunch: "com.openai.codex" } }, + darwinDesktopAppAdapter: { discover: () => { + if (scenario.discoveryFails) throw new Error("private discovery diagnostic"); + return scenario.noInstall ? null : { id: "com.openai.codex", root, relaunch: "com.openai.codex" }; + } }, })); mock.module("../../src/chatgpt/app-server-shim/launcher", () => ({ chatgptShimLauncherPath: () => launcher, @@ -69,4 +80,4 @@ mock.module("../../src/chatgpt/app-server-shim/launcher", () => ({ })); const { handleChatgptCommand } = await import("../../src/cli/chatgpt-command"); const code = await handleChatgptCommand([scenario.sub ?? "restore"], "darwin"); -console.log(JSON.stringify({ code, calls, launcherExists: fs.existsSync(launcher), root, shell, binary })); +console.log(JSON.stringify({ code, calls, launcherExists: fs.existsSync(launcher), launcherContent: fs.existsSync(launcher) ? fs.readFileSync(launcher, "utf8") : null, root, shell, binary })); From 146bfa888be0b4245a74fb078868df33b39a0222 Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Wed, 14 Oct 2026 00:31:25 +0900 Subject: [PATCH 4/5] fix(proxy): bound SOCKS5 TLS handshake lifetime Carries #7056. Clear deadline and listeners on every TLS handshake exit while keeping certificate verification. Co-authored-by: lcxhh --- .../docs/reference/configuration/server.md | 4 + scripts/test-layout/layout.json | 2 +- src/lib/socks5-fetch.ts | 39 +++++- structure/transports/inventory.md | 6 + tests/fixtures/test-layout-expected.json | 3 +- tests/lib/socks5-tls-lifecycle.test.ts | 123 ++++++++++++++++++ 6 files changed, 168 insertions(+), 9 deletions(-) create mode 100644 tests/lib/socks5-tls-lifecycle.test.ts diff --git a/docs-site/src/content/docs/reference/configuration/server.md b/docs-site/src/content/docs/reference/configuration/server.md index d71c2fc920e..ac9be7cc157 100644 --- a/docs-site/src/content/docs/reference/configuration/server.md +++ b/docs-site/src/content/docs/reference/configuration/server.md @@ -126,6 +126,10 @@ SOCKS5 (Clash mixed-port listeners included) belongs on `ALL_PROXY`, not `HTTP_P `ocx start --socks5` writes that value; `ocx start --socks5-off` clears it. +HTTPS requests through SOCKS5 have a separate 30-second TLS handshake deadline. +A stalled handshake fails without disabling certificate verification; caller +cancellation can stop it sooner. + If an older development build changed resume-history metadata before backup support existed, run `ocx recover-history --legacy-openai --yes` to force native-provider recovery. It force-relabels every user-message `opencodex` row, including legitimate dedicated-provider diff --git a/scripts/test-layout/layout.json b/scripts/test-layout/layout.json index bccf1d25f41..2d6f396164d 100644 --- a/scripts/test-layout/layout.json +++ b/scripts/test-layout/layout.json @@ -1728,7 +1728,7 @@ "sidecar-tracker.test.ts": "vision", "skill-ocx.test.ts": "ci-workflows", "slug-codec.test.ts": "codex-integration", - "socks5-fetch.test.ts": "lib", + "socks5-fetch.test.ts": "lib", "socks5-tls-lifecycle.test.ts": "lib", "socks5-upload-lifecycle.test.ts": "lib", "spend-ceiling-enforcement.test.ts": "lib", "spend-instrumentation-log.test.ts": "server", diff --git a/src/lib/socks5-fetch.ts b/src/lib/socks5-fetch.ts index 52bdc3d3b50..ed2145c9e99 100644 --- a/src/lib/socks5-fetch.ts +++ b/src/lib/socks5-fetch.ts @@ -341,25 +341,50 @@ function requestHeaders(request: Request, target: URL): { text: string; chunked: async function secureSocket(socket: Socket, target: URL, signal: AbortSignal): Promise { return new Promise((resolve, reject) => { + if (signal.aborted) { + socket.destroy(); + reject(signal.reason instanceof Error ? signal.reason : new Error("The operation was aborted")); + return; + } const tlsSocket = tls.connect({ socket, servername: target.hostname, rejectUnauthorized: true, }); - const onAbort = () => { + let settled = false; + // Absolute deadline: a peer sending occasional TLS bytes must not extend the handshake. + const timer = setTimeout(() => { + fail(new Socks5FetchError("SOCKS5 TLS handshake timed out")); + }, SOCKS5_CONNECT_TIMEOUT_MS); + const cleanup = () => { + clearTimeout(timer); + signal.removeEventListener("abort", onAbort); + tlsSocket.removeListener("secureConnect", onSecureConnect); + tlsSocket.removeListener("error", onError); + tlsSocket.removeListener("close", onClose); + }; + const fail = (error: Error) => { + if (settled) return; + settled = true; + cleanup(); + // No error argument: pending event observers must not receive a second error. tlsSocket.destroy(); - reject(signal.reason instanceof Error ? signal.reason : new Error("The operation was aborted")); + reject(error); }; + const onAbort = () => fail(signal.reason instanceof Error ? signal.reason : new Error("The operation was aborted")); + const onError = (error: Error) => fail(error); + const onClose = () => fail(new Socks5FetchError("SOCKS5 upstream closed during TLS handshake")); const onSecureConnect = () => { - signal.removeEventListener("abort", onAbort); + if (settled) return; + settled = true; + cleanup(); resolve(tlsSocket); }; tlsSocket.once("secureConnect", onSecureConnect); - tlsSocket.once("error", error => { - signal.removeEventListener("abort", onAbort); - reject(error); - }); + tlsSocket.once("error", onError); + tlsSocket.once("close", onClose); signal.addEventListener("abort", onAbort, { once: true }); + if (signal.aborted) onAbort(); }); } diff --git a/structure/transports/inventory.md b/structure/transports/inventory.md index 4956c0e7165..38dd6813dca 100644 --- a/structure/transports/inventory.md +++ b/structure/transports/inventory.md @@ -307,6 +307,12 @@ admission or account-snapshot pairing. The forwarding contract is covered in ## SOCKS5 dispatch boundary +HTTPS tunnels in `src/lib/socks5-fetch.ts` give the TLS handshake its own absolute +30-second deadline after SOCKS CONNECT. Successful TLS clears that deadline before +the response timer takes over; timeout, caller cancellation, error and premature +close release the owned socket and listeners. Certificate verification stays enabled. +`tests/lib/socks5-tls-lifecycle.test.ts` exercises stalled and interrupted handshakes. + `src/config/proxy-env.ts` activates configured SOCKS5 through `src/lib/proxy-env.ts`; the compatibility config facade does not own a second activation path. The library captures `startupOutboundProxyConfigured` at module evaluation, before config diff --git a/tests/fixtures/test-layout-expected.json b/tests/fixtures/test-layout-expected.json index 24ba62a1e66..d7d13cfacb5 100644 --- a/tests/fixtures/test-layout-expected.json +++ b/tests/fixtures/test-layout-expected.json @@ -1067,7 +1067,8 @@ "sidecar-settings-web-search-gate.test.ts": "vision", "sidecar-settings-web-search-off.test.ts": "vision", "sidecar-settings-web-search-stream.test.ts": "vision", "sidecar-tracker.test.ts": "vision", "skill-ocx.test.ts": "ci-workflows", "slug-codec.test.ts": "codex-integration", - "socks5-fetch.test.ts": "lib", "socks5-upload-lifecycle.test.ts": "lib", + "socks5-fetch.test.ts": "lib", "socks5-tls-lifecycle.test.ts": "lib", + "socks5-upload-lifecycle.test.ts": "lib", "spend-ceiling-enforcement.test.ts": "lib", "spend-instrumentation-log.test.ts": "server", "spend-ledger-file-journal.test.ts": "lib", "spend-ledger-lifecycle.test.ts": "server", "spend-ledger-owner-startup.test.ts": "server", "spend-ledger-owner.test.ts": "lib", diff --git a/tests/lib/socks5-tls-lifecycle.test.ts b/tests/lib/socks5-tls-lifecycle.test.ts new file mode 100644 index 00000000000..08e6233ff73 --- /dev/null +++ b/tests/lib/socks5-tls-lifecycle.test.ts @@ -0,0 +1,123 @@ +import { describe, test } from "bun:test"; +import assert from "node:assert/strict"; +import { createServer, type AddressInfo, type Socket } from "node:net"; +import { socks5Fetch } from "../../src/lib/socks5-fetch"; + +const CONNECT_TIMEOUT_MS = 30_000; +type Failure = "deadline" | "abort" | "null-abort" | "close" | "invalid-tls"; + +/** SOCKS accepts CONNECT, then observes real TLS bytes without contacting an upstream. */ +async function checkHandshakeFailure(failure: Failure): Promise { + const sockets = new Set(); + let markTlsSeen: () => void = () => {}; + let markPeerClosed: () => void = () => {}; + const tlsSeen = new Promise(resolve => { markTlsSeen = resolve; }); + const peerClosed = new Promise(resolve => { markPeerClosed = resolve; }); + let peer: Socket | undefined; + const server = createServer(socket => { + peer = socket; + sockets.add(socket); + socket.once("error", () => { /* fixture teardown may reset the connection */ }); + socket.once("close", () => { sockets.delete(socket); markPeerClosed(); }); + let stage: "greeting" | "connect" | "tls" = "greeting"; + let buffer = Buffer.alloc(0); + socket.on("data", chunk => { + buffer = Buffer.concat([buffer, typeof chunk === "string" ? Buffer.from(chunk) : chunk]); + if (stage === "greeting") { + if (buffer.length < 2 || buffer.length < 2 + buffer[1]!) return; + buffer = buffer.subarray(2 + buffer[1]!); + socket.write(Buffer.from([5, 0])); + stage = "connect"; + } + if (stage === "connect") { + if (buffer.length < 5 || buffer.length < 7 + buffer[4]!) return; + buffer = buffer.subarray(7 + buffer[4]!); + socket.write(Buffer.from([5, 0, 0, 1, 127, 0, 0, 1, 0, 80])); + stage = "tls"; + } + if (stage === "tls" && buffer.length > 0) { + markTlsSeen(); + buffer = Buffer.alloc(0); + } + }); + }); + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", resolve); + }); + + const originalSetTimeout = globalThis.setTimeout; + const originalClearTimeout = globalThis.clearTimeout; + const deadlines = new Map, () => void>(); + let deadlinesArmed = 0; + // Capture, but do not shorten, the transport deadline. Fire it only after the + // fixture actually receives ClientHello, so machine load cannot race TCP setup. + globalThis.setTimeout = ((callback: (...args: unknown[]) => void, ms?: number, ...args: unknown[]) => { + const handle = originalSetTimeout(callback, ms, ...args); + if (ms === CONNECT_TIMEOUT_MS) { + deadlinesArmed++; + deadlines.set(handle, () => callback(...args)); + } + return handle; + }) as typeof setTimeout; + globalThis.clearTimeout = ((handle: ReturnType | undefined) => { + if (handle !== undefined) deadlines.delete(handle); + return originalClearTimeout(handle); + }) as typeof clearTimeout; + + const controller = new AbortController(); + const reason = failure === "null-abort" ? null : new Error("fixture caller cancellation"); + const watchdog = originalSetTimeout(() => controller.abort(new Error("fixture watchdog")), 2_000); + let outcome: Promise | undefined; + try { + const port = (server.address() as AddressInfo).port; + outcome = socks5Fetch("https://destination.invalid/", { signal: controller.signal }, `socks5://127.0.0.1:${port}`) + .then(() => new Error("unexpected TLS success"), (error: unknown) => error); + // Receipt is bounded by the same request outcome: a failure before ClientHello + // must fail this assertion instead of leaving the test waiting indefinitely. + const ready = await Promise.race([tlsSeen.then(() => true), outcome.then(() => false)]); + assert.equal(ready, true, "the peer must receive ClientHello before exercising TLS failure"); + // Bun also implements a socket inactivity timer through global setTimeout. + // Its already-cleared internal timer must not make the transport test Node-only. + assert.ok(deadlinesArmed >= 2, "TCP and TLS deadlines must both be observed"); + assert.equal(deadlines.size, 1, "only the TLS deadline remains active"); + + if (failure === "deadline") deadlines.values().next().value!(); + else if (failure === "abort" || failure === "null-abort") controller.abort(reason); + else if (failure === "close") peer!.destroy(); + else peer!.write("fixture invalid TLS"); + + const result = await outcome; + if (failure === "abort" || failure === "null-abort") assert.equal(result, reason); + else { + assert.ok(result instanceof Error); + if (failure === "deadline") assert.match(result.message, /SOCKS5 TLS handshake timed out/); + else if (failure === "close") assert.match(result.message, /TLS handshake|disconnected before secure TLS/); + else assert.match(result.message, /ssl|wrong version|packet|tls/i); + } + assert.equal(deadlines.size, 0, "terminal failure clears the owned deadline"); + let closeWatchdog: ReturnType | undefined; + try { + await Promise.race([peerClosed, new Promise((_, reject) => { + closeWatchdog = originalSetTimeout(() => reject(new Error("TLS peer remained open")), 2_000); + })]); + } finally { originalClearTimeout(closeWatchdog); } + } finally { + controller.abort(); + await outcome; + originalClearTimeout(watchdog); + for (const handle of deadlines.keys()) originalClearTimeout(handle); + globalThis.setTimeout = originalSetTimeout; + globalThis.clearTimeout = originalClearTimeout; + for (const socket of sockets) socket.destroy(); + await new Promise(resolve => server.close(() => resolve())); + } +} + +describe("SOCKS5 TLS handshake lifecycle", () => { + test("a stalled TLS handshake reaches an absolute deadline and releases its socket", () => checkHandshakeFailure("deadline")); + test("caller cancellation preserves its reason and clears the TLS deadline", () => checkHandshakeFailure("abort")); + test("a null cancellation reason is preserved", () => checkHandshakeFailure("null-abort")); + test("premature peer close releases the TLS deadline", () => checkHandshakeFailure("close")); + test("TLS protocol failure releases the TLS deadline", () => checkHandshakeFailure("invalid-tls")); +}); From 12f8ef22ee6a3855b3067f8cefde2c1ee896b5fb Mon Sep 17 00:00:00 2001 From: JUN <243035832+lidge-jun@users.noreply.github.com> Date: Wed, 14 Oct 2026 00:31:52 +0900 Subject: [PATCH 5/5] docs: triage exceeded-retry-limit 429 errors Carries #6925. Align refresh release conditions and the resolved credit rediscovery reference with current dev. Co-authored-by: Howard-Jerry <197214646+Howard-Jerry@users.noreply.github.com> --- .../docs/reference/codex-retry-diagnostics.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs-site/src/content/docs/reference/codex-retry-diagnostics.md b/docs-site/src/content/docs/reference/codex-retry-diagnostics.md index 532ada6ddfd..ba4d7d81ab7 100644 --- a/docs-site/src/content/docs/reference/codex-retry-diagnostics.md +++ b/docs-site/src/content/docs/reference/codex-retry-diagnostics.md @@ -23,6 +23,24 @@ waiting visible: those items can contaminate history or imply work happened. Do not shorten the provider delay or provoke an extra request to advance the client's retry counter. +## Triage: `exceeded retry limit, last status: 429 Too Many Requests` + +The message is the client's retry budget summary, not a diagnosis. One refusal +that the client does not retransmit can end a turn in the same words as a +fully exhausted budget, so treat the text as a question. Three cases look +identical at this layer and are checked differently. + +| Check | Distinguishing evidence | If the check is positive | +| --- | --- | --- | +| Main-account hard lock (local admission policy) | The dashboard Settings or `GET /api/settings` reports the policy state, such as `mainAccountHardLock.state` being `blocked`, `ready`, or `off`, with `thresholds` and an optional `resetAt`. A non-streaming request may also carry an explicit main-policy body instead of a bare upstream 429. | Refresh usage with `ocx account refresh openai`; fresh usage below every blocking threshold or validated window absence releases the lock. If it remains blocked, wait for fresh lower usage or explicitly turn `codexMainAccountHardLock` off in Settings. Builds before [#6287](https://github.com/lidge-jun/opencodex/pull/6287) could keep an obsolete short-window block across restarts when no fresh observation ever replaced it; the release semantics are described in the [closing note on #6244](https://github.com/lidge-jun/opencodex/issues/6244). | +| Upstream ChatGPT plan window | The account quota section shows a 5-hour or weekly window at or near its limit while the lock reports `ready` or `off`. | Wait for the window to reset, inspect any reset option ChatGPT offers ([banked reset rules](https://help.openai.com/en/articles/20001498-how-banked-codex-resets-work), [plan windows](https://help.openai.com/en/articles/11369540-using-codex-with-your-chatgpt-plan)), or serve the request on another account with `ocx account use openai `. For explicitly authorized paid-credit use after the limit, see [account credit policy](/reference/cli/providers-accounts/#pool-policy-account-thresholds-and-paid-credit-intent). The credit-balance rediscovery defect in [#6845](https://github.com/lidge-jun/opencodex/issues/6845) was fixed by [#6886](https://github.com/lidge-jun/opencodex/pull/6886). | +| Provider incident or transport failure | Quota stays low, the lock stays released, and `ocx doctor` reports transport or ChatGPT reachability problems. | Retry after the incident clears or reroute through an existing combo. Changing lock or threshold policy is not a cure for a transport failure. | + +A locked main credential closes request admission; it does not reserve the +account's remaining quota, and a predicted reset never releases the block on +its own. The thresholds (by default `short` 90, `long` 98) are visible in the +status payload and configurable through `codexMainAccountHardLockThresholds`. + ## Why the first reconnect row can be absent The public Codex `rust-v0.158.0-alpha.2.1` source gates the ordinary stream-retry