From a2d032ca530316c8ae6a25304a4e2e200091115f Mon Sep 17 00:00:00 2001 From: Matthew Bright Date: Thu, 30 Jul 2026 19:54:31 +1000 Subject: [PATCH 01/31] Redact persisted scan failure credentials --- sdk/typescript/src/api.ts | 8 +-- sdk/typescript/src/cli.ts | 68 ++++++++------------ sdk/typescript/src/errors.ts | 22 +++++++ sdk/typescript/src/multiscan.ts | 16 +---- sdk/typescript/tests-ts/api.test.ts | 75 +++++++++++++++++++++++ sdk/typescript/tests-ts/multiscan.test.ts | 18 +++++- 6 files changed, 144 insertions(+), 63 deletions(-) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index d2381d3d..cd9dd33a 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -32,6 +32,7 @@ import { OutputDirectoryError, OutputInsideProtectedRootError, type ProtectedScanPathKind, + redactedErrorMessage, ScanCostLimitExceededError, ScanInterruptedError, } from "./errors.js"; @@ -843,11 +844,10 @@ export class CodexSecurity { "fail-scan", "--scan-id", activeScan.id, + // Redact before truncating: the stored message is read back by + // `scans show` and travels inside the results directory. "--message", - (failure instanceof Error - ? failure.message - : String(failure) - ).slice(0, 2400), + redactedErrorMessage(failure).slice(0, 2400), ...(snapshot?.cost ? ["--cost-json", JSON.stringify(snapshot.cost)] : []), diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index 1abe8302..1d7b2688 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -57,6 +57,7 @@ import { formatUsd } from "./cost.js"; import { CodexSecurityError, OutputInsideProtectedRootError, + redactedErrorMessage, ScanInterruptedError, } from "./errors.js"; import type { SeverityLevel } from "./models.js"; @@ -606,7 +607,7 @@ export async function main( try { return await select(await dependencies.runWorkbench(args)); } catch (error) { - errorOutput.write(`codex-security: ${cliErrorMessage(error)}\n`); + errorOutput.write(`codex-security: ${redactedErrorMessage(error)}\n`); exitCode = 2; return undefined; } @@ -805,7 +806,7 @@ export async function main( ]); scanArguments = scanArgumentsFromRecipe(recipe, args.scanId); } catch (error) { - const message = cliErrorMessage(error); + const message = redactedErrorMessage(error); errorOutput.write(`codex-security: ${message}\n`); exitCode = 2; return incurError({ @@ -868,7 +869,7 @@ export async function main( format, ); } catch (error) { - errorOutput.write(`codex-security: ${cliErrorMessage(error)}\n`); + errorOutput.write(`codex-security: ${redactedErrorMessage(error)}\n`); exitCode = 2; return undefined; } @@ -1146,7 +1147,7 @@ export async function main( failOnSeverity: options.failOnSeverity, }; } catch (error) { - errorOutput.write(`codex-security: ${cliErrorMessage(error)}\n`); + errorOutput.write(`codex-security: ${redactedErrorMessage(error)}\n`); exitCode = 2; return undefined; } @@ -1307,7 +1308,7 @@ export async function main( signal: controller.signal, onProgress: ({ repository, status, attempt, error }) => { errorOutput.write( - `codex-security: ${repository} ${status} (attempt ${attempt})${error === undefined ? "" : `: ${cliErrorMessage(error)}`}\n`, + `codex-security: ${repository} ${status} (attempt ${attempt})${error === undefined ? "" : `: ${redactedErrorMessage(error)}`}\n`, ); }, }); @@ -1319,7 +1320,7 @@ export async function main( (error instanceof Error && error.name === "ExitPromptError" ? 130 : 2); - errorOutput.write(`codex-security: ${cliErrorMessage(error)}\n`); + errorOutput.write(`codex-security: ${redactedErrorMessage(error)}\n`); } finally { dependencies.removeSignalListener("SIGINT", onInterrupt); dependencies.removeSignalListener("SIGTERM", onTerminate); @@ -1423,7 +1424,7 @@ export async function main( ); } catch (error) { exitCode = 2; - errorOutput.write(`codex-security: ${cliErrorMessage(error)}\n`); + errorOutput.write(`codex-security: ${redactedErrorMessage(error)}\n`); } }, }) @@ -1459,7 +1460,7 @@ export async function main( ); } catch (error) { exitCode = 2; - errorOutput.write(`codex-security: ${cliErrorMessage(error)}\n`); + errorOutput.write(`codex-security: ${redactedErrorMessage(error)}\n`); } }, }) @@ -1644,7 +1645,7 @@ export async function main( if (frameworkExit !== undefined) { if (exitCode !== 0) return exitCode; errorOutput.write( - `codex-security: ${cliErrorMessage(incurErrorMessage(frameworkOutput))}\n`, + `codex-security: ${redactedErrorMessage(incurErrorMessage(frameworkOutput))}\n`, ); return 2; } @@ -1653,7 +1654,7 @@ export async function main( await writeCliOutput(output, renderedHistory ?? frameworkOutput); return exitCode; } catch (error) { - errorOutput.write(`codex-security: ${cliErrorMessage(error)}\n`); + errorOutput.write(`codex-security: ${redactedErrorMessage(error)}\n`); return 2; } } @@ -2369,7 +2370,7 @@ async function runExport( } return 0; } catch (error) { - errorOutput.write(`codex-security: ${cliErrorMessage(error)}\n`); + errorOutput.write(`codex-security: ${redactedErrorMessage(error)}\n`); return 2; } } @@ -2517,7 +2518,7 @@ async function runScan( onOutputArchived: (archiveDir) => { progress?.stopTimer(); errorOutput.write( - `Moved existing results to: ${cliErrorMessage(archiveDir)}\n`, + `Moved existing results to: ${redactedErrorMessage(archiveDir)}\n`, ); }, signal: preparationAbortController.signal, @@ -2578,12 +2579,12 @@ async function runScan( }, onWarning: (warning) => { errorOutput.write( - `codex-security: warning: ${cliErrorMessage(warning)}\n`, + `codex-security: warning: ${redactedErrorMessage(warning)}\n`, ); }, onObserverError: (observer, error) => { errorOutput.write( - `codex-security: warning: ${observer} observer failed: ${cliErrorMessage(error)}\n`, + `codex-security: warning: ${observer} observer failed: ${redactedErrorMessage(error)}\n`, ); }, }; @@ -2620,7 +2621,7 @@ async function runScan( if (failed) { const message = failure instanceof OutputInsideProtectedRootError - ? cliErrorMessage(protectedRootErrorMessage(failure)) + ? redactedErrorMessage(protectedRootErrorMessage(failure)) : scanFailureMessage(failure, selectedAuthentication); errorOutput.write(`${message}\n`); if (failure instanceof ScanInterruptedError) { @@ -2628,7 +2629,7 @@ async function runScan( } if (scanDir !== null) { errorOutput.write( - `Partial output was kept at ${cliErrorMessage(scanDir)}.\n`, + `Partial output was kept at ${redactedErrorMessage(scanDir)}.\n`, ); } return { exitCode: 2, error: message }; @@ -2697,7 +2698,7 @@ function scanFailureMessage( case "network_error": case "timeout": case "unknown": - return cliErrorMessage(error); + return redactedErrorMessage(error); } } @@ -2711,7 +2712,9 @@ function scanScope(arguments_: ScanArguments): string | null { portable.startsWith("//") ? portable.split("/").at(-1) ?? portable : portable; - return cliErrorMessage(scoped.replaceAll(/[\u0000-\u001F\u007F]/gu, " ")); + return redactedErrorMessage( + scoped.replaceAll(/[\u0000-\u001F\u007F]/gu, " "), + ); }); return `${displayed.join(", ")}${arguments_.paths.length > displayed.length ? `, +${arguments_.paths.length - displayed.length} more` : ""}`; } @@ -2773,7 +2776,7 @@ function printScanSummary( ? 33 : 36; errorOutput.write( - `\n ${paint("REPORT", "1;36")} ${paint(cliErrorMessage(result.reportPath), 4)}\n\n` + + `\n ${paint("REPORT", "1;36")} ${paint(redactedErrorMessage(result.reportPath), 4)}\n\n` + ` ${paint("FINDINGS", 1)} ${paint(`${findingCount}${severitySummary === "" ? "" : ` (${severitySummary})`}`, findingColor)}\n` + ` ${paint("COVERAGE", 1)} ${result.coverage.completeness}\n` + ` ${paint("ELAPSED", 1)} ${duration}\n`, @@ -2789,7 +2792,7 @@ function printScanSummary( ); } errorOutput.write( - ` ${paint("RESULTS", 1)} ${cliErrorMessage(result.scanDir)}\n`, + ` ${paint("RESULTS", 1)} ${redactedErrorMessage(result.scanDir)}\n`, ); } @@ -3089,7 +3092,7 @@ function interruptedExit( errorOutput.write( scanDir === null ? "codex-security: No partial output was kept.\n" - : `codex-security: Partial output was kept at ${cliErrorMessage(scanDir)}.\n`, + : `codex-security: Partial output was kept at ${redactedErrorMessage(scanDir)}.\n`, ); return ctrlC ? 130 : 143; } @@ -3109,34 +3112,13 @@ function invokedAsMain(): boolean { } } -function cliErrorMessage(error: unknown): string { - const message = error instanceof Error ? error.message : String(error); - return message - .replaceAll( - /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|token|secret|credential|signature|sig|password|passwd)\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)[^\s"',;}&\\\]]+/giu, - "$1[redacted]", - ) - .replaceAll(/sk-(?:proj-)?[A-Za-z0-9_*=-]{8,}/gu, "[redacted]") - .replaceAll(/(?:github_pat_|gh[pousr]_)[A-Za-z0-9_-]{8,}/giu, "[redacted]") - .replaceAll(/npm_[A-Za-z0-9_-]{8,}/giu, "[redacted]") - .replaceAll( - /(^|%20|[^A-Za-z0-9_])(Bearer|Basic|Token)((?:\s|%20|\+)+)[A-Za-z0-9.%_~+/*=-]{8,}/giu, - "$1$2$3[redacted]", - ) - .replaceAll(/((?:https?|ssh|git\+ssh):\/\/)[^\s/@]+@/giu, "$1[redacted]@") - .replaceAll( - /((?:[?&]|%3F|%26)(?:(?!%3F|%26|%3D)(?:[A-Za-z0-9_.%-]|\[|\])){0,64}(?:api[_-]?key|access(?:[_-]|%5F|%2D)?key(?:(?:[_-]|%5F|%2D)?id)?|token|secret|credential|signature|sig|password|passwd)(?:\]|%5D)?(?:=|%3D))(?:(?!%26)[^&\s])+/giu, - "$1[redacted]", - ); -} - if (invokedAsMain()) { void main().then( (exitCode) => { process.exitCode = exitCode; }, (error: unknown) => { - process.stderr.write(`codex-security: ${cliErrorMessage(error)}\n`); + process.stderr.write(`codex-security: ${redactedErrorMessage(error)}\n`); process.exitCode = 2; }, ); diff --git a/sdk/typescript/src/errors.ts b/sdk/typescript/src/errors.ts index 10be04ca..3add0d1c 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -1,5 +1,27 @@ import { formatUsd, type ScanCost } from "./cost.js"; +/** Returns an error message with credential-shaped substrings redacted. */ +export function redactedErrorMessage(error: unknown): string { + const message = error instanceof Error ? error.message : String(error); + return message + .replaceAll( + /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|token|secret|credential|signature|sig|password|passwd)\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)[^\s"',;}&\\\]]+/giu, + "$1[redacted]", + ) + .replaceAll(/sk-(?:proj-)?[A-Za-z0-9_*=-]{8,}/gu, "[redacted]") + .replaceAll(/(?:github_pat_|gh[pousr]_)[A-Za-z0-9_-]{8,}/giu, "[redacted]") + .replaceAll(/npm_[A-Za-z0-9_-]{8,}/giu, "[redacted]") + .replaceAll( + /(^|%20|[^A-Za-z0-9_])(Bearer|Basic|Token)((?:\s|%20|\+)+)[A-Za-z0-9.%_~+/*=-]+/giu, + "$1$2$3[redacted]", + ) + .replaceAll(/((?:https?|ssh|git\+ssh):\/\/)[^\s/@]+@/giu, "$1[redacted]@") + .replaceAll( + /((?:[?&]|%3F|%26)(?:(?!%3F|%26|%3D)(?:[A-Za-z0-9_.%-]|\[|\])){0,64}(?:api[_-]?key|access(?:[_-]|%5F|%2D)?key(?:(?:[_-]|%5F|%2D)?id)?|token|secret|credential|signature|sig|password|passwd)(?:\]|%5D)?(?:=|%3D))(?:(?!%26)[^&\s])+/giu, + "$1[redacted]", + ); +} + /** Base error for Codex Security SDK failures. */ export class CodexSecurityError extends Error { public constructor(message: string, options?: ErrorOptions) { diff --git a/sdk/typescript/src/multiscan.ts b/sdk/typescript/src/multiscan.ts index b4a0fa7a..534715a6 100644 --- a/sdk/typescript/src/multiscan.ts +++ b/sdk/typescript/src/multiscan.ts @@ -17,6 +17,7 @@ import Papa from "papaparse"; import type { CodexSecurity } from "./api.js"; import type { CodexSecurityConfig } from "./config.js"; import type { ScanCost } from "./cost.js"; +import { redactedErrorMessage } from "./errors.js"; import type { ScanMode } from "./targets.js"; import { resolveTrustedExecutable } from "./trusted-executable.js"; @@ -190,7 +191,7 @@ async function runCampaign( } } catch (error) { if (options.signal?.aborted === true) options.signal.throwIfAborted(); - failure = redactError(error); + failure = redactedErrorMessage(error); } finally { await rm(checkout, { recursive: true, force: true }); } @@ -523,16 +524,3 @@ export function buildGitHubCredentialArgs(host: string | undefined): string[] { const key = `credential.${url.origin}.helper`; return ["-c", `${key}=`, "-c", `${key}=!gh auth git-credential`]; } - -function redactError(error: unknown): string { - return (error instanceof Error ? error.message : String(error)) - .replaceAll( - /((?:api[_-]?key|token|secret|credential|password)[A-Za-z0-9_-]*\s*[:=]\s*)[^\s,;]+/giu, - "$1[redacted]", - ) - .replaceAll( - /\b(?:sk-(?:proj-)?|gh[pousr]_|github_pat_|npm_)[A-Za-z0-9_*=-]{8,}/gu, - "[redacted]", - ) - .replaceAll(/\b(Bearer|Basic|Token)\s+[^\s,;]+/giu, "$1 [redacted]"); -} diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 13e73152..1a8183e2 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -45,6 +45,7 @@ import { setCodexSecurityCredentialLogout, } from "../src/runtime.js"; import { normalizeTarget } from "../src/targets.js"; +import { REDACTED_CREDENTIALS, SYNTHETIC_CREDENTIALS } from "./cli-fixtures.js"; import { INTEGRATION_TARGET, PLUGIN_ROOT } from "./plugin-root.js"; type ScanObserverName = Parameters< @@ -2164,6 +2165,80 @@ describe("CodexSecurity orchestration", () => { await client.close(); }); + test("redacts credentials from the stored scan failure message", async () => { + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const codexHome = join(root, "codex-home"); + const stateDirectory = join(root, "state"); + const scanDir = join(root, "scan"); + await mkdir(repository); + await mkdir(codexHome); + await mkdir(scanDir, { mode: 0o700 }); + const python = Bun.which("python3") ?? Bun.which("python"); + expect(python).not.toBeNull(); + const environment = { + PATH: process.env["PATH"], + CODEX_SECURITY_STATE_DIR: stateDirectory, + }; + const commands: Array = []; + const client = new TestClient( + {}, + { + environment, + prepareRuntime: async () => ({ + ...preparedRuntime(codexHome), + environment, + }), + resolvePluginPython: async () => python!, + prepareOutputDir: async () => scanDir, + repositoryRevision: async () => "deadbeef", + runWorkbench: async ( + options: Parameters[0], + args: readonly string[], + ) => { + commands.push(args); + return await runWorkbench(options, args); + }, + createCodex: () => ({ + startThread: () => ({ + id: null, + async runStreamed() { + async function* failingEvents(): AsyncGenerator { + yield { type: "error", message: SYNTHETIC_CREDENTIALS }; + } + return { events: failingEvents() }; + }, + }), + }), + }, + ); + + // The in-memory error keeps its original text; only what leaves the process + // is redacted, so the CLI can still classify the upstream failure. + await expect(client.run(repository)).rejects.toThrow(SYNTHETIC_CREDENTIALS); + const failure = commands.find((args) => args[0] === "fail-scan"); + const scanId = failure?.[2] ?? ""; + expect(scanId).toMatch(/^[0-9a-f-]{36}$/); + expect(failure?.[3]).toBe("--message"); + expect(failure?.[4]).toBe(REDACTED_CREDENTIALS); + + // `scans show` reads the stored message back through get-scan. + const context = await runWorkbench( + { python: python!, pluginRoot: PLUGIN_ROOT, environment }, + ["get-scan", "--scan-id", scanId], + ); + expect(context["scan"]).toMatchObject({ + progress: { status: "failed" }, + failureMessage: REDACTED_CREDENTIALS, + }); + + // Every synthetic credential is tagged SYNTHETIC, so the database file + // itself proves nothing was persisted anywhere on the failure path. + const database = await readFile(join(stateDirectory, "workbench.sqlite3")); + expect(database.toString("latin1")).not.toContain("SYNTHETIC"); + await client.close(); + }); + test("retains default scan output under persistent plugin state", async () => { const root = await temporaryDirectory(); const repository = join(root, "repository"); diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index e739ed74..5ed08ef5 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -444,6 +444,11 @@ describe("multiscan", () => { const paths = await fixture(); const source = await repository(paths.root, "retry"); const secret = "sk-proj-SYNTHETIC_MULTISCAN_SECRET_123"; + const proxyUrl = + "https://SYNTHETIC_USER:SYNTHETIC_MULTISCAN_PASSWORD@proxy.test/v1/responses"; + const queryUrl = + "https://proxy.test/v1/responses?api_key=SYNTHETIC_MULTISCAN_QUERY_123&safe=1"; + const shortAuthorization = "Bearer abc123"; await writeFile( paths.input, `id,repository,revision\nretry,${source.path},${source.revision}\n`, @@ -455,7 +460,11 @@ describe("multiscan", () => { paths, client(async (_repository, scanOptions = {}) => { attempts += 1; - if (attempts === 1) throw new Error(`temporary failure ${secret}`); + if (attempts === 1) { + throw new Error( + `temporary failure ${secret} ${shortAuthorization} sending request for url (${proxyUrl}) and ${queryUrl}`, + ); + } return await completedScan(scanOptions.outputDir!); }), ), @@ -467,7 +476,12 @@ describe("multiscan", () => { { id: "retry", status: "failed", attempt: 1 }, { id: "retry", status: "completed", attempt: 2 }, ]); - expect(await readFile(summary.resultsPath, "utf8")).not.toContain(secret); + const ledger = await readFile(summary.resultsPath, "utf8"); + expect(ledger).not.toContain(secret); + expect(ledger).not.toContain("SYNTHETIC_MULTISCAN_PASSWORD"); + expect(ledger).not.toContain("SYNTHETIC_MULTISCAN_QUERY_123"); + expect(ledger).not.toContain(shortAuthorization); + expect(ledger).toContain("https://[redacted]@proxy.test/v1/responses"); }); test("resumes complete bundles, repairs missing output, and rejects manifest drift", async () => { From 512531fe0a2d26bef30ceda9b92a6fe47aba4df5 Mon Sep 17 00:00:00 2001 From: GautamSharma99 Date: Thu, 30 Jul 2026 18:30:17 +0530 Subject: [PATCH 02/31] fix: bound login and skill child shutdown --- sdk/typescript/src/auth.ts | 72 +++++++++++++---- sdk/typescript/src/cli.ts | 72 ++++++++++++----- sdk/typescript/tests-ts/api.test.ts | 18 ++++- sdk/typescript/tests-ts/auth.test.ts | 41 +++++++++- sdk/typescript/tests-ts/cli-skills.test.ts | 90 ++++++++++++++++++++++ 5 files changed, 253 insertions(+), 40 deletions(-) diff --git a/sdk/typescript/src/auth.ts b/sdk/typescript/src/auth.ts index 83cd69fe..55b72e5f 100644 --- a/sdk/typescript/src/auth.ts +++ b/sdk/typescript/src/auth.ts @@ -3,6 +3,8 @@ import { isIP } from "node:net"; import { PluginBootstrapError } from "./errors.js"; import type { CodexCommand, ProcessEnvironment } from "./runtime.js"; +const LOGIN_CHILD_TERMINATION_GRACE_MS = 1_000; + export interface LoginResult { success: boolean; exitCode: number | null; @@ -27,6 +29,8 @@ export class CodexLoginHandle { #urlReadySettled = false; #deviceReadySettled = false; #canceled = false; + #forcedTermination: ReturnType | undefined; + #forceCompletion: (() => void) | null = null; #stdout = ""; #stderr = ""; @@ -63,21 +67,15 @@ export class CodexLoginHandle { this.#notifyInstructions(); }); this.#completion = new Promise((resolve, reject) => { - this.#child.once("error", (error) => { - this.#settleInstructionWaiters({ - success: false, - exitCode: null, - stdout: this.#stdout, - stderr: error.message, - }); - reject(error); - }); let fallback: ReturnType | undefined; let completed = false; const complete = (exitCode: number | null): void => { if (completed) return; completed = true; if (fallback !== undefined) clearTimeout(fallback); + this.#clearForcedTermination(); + this.#forceCompletion = null; + this.#destroyPipes(); const result = { success: exitCode === 0 && !this.#canceled, exitCode, @@ -88,14 +86,28 @@ export class CodexLoginHandle { if (result.success) onSuccess(); resolve(result); }; + this.#forceCompletion = () => complete(this.#child.exitCode); + this.#child.once("error", (error) => { + if (completed) return; + completed = true; + if (fallback !== undefined) clearTimeout(fallback); + this.#clearForcedTermination(); + this.#forceCompletion = null; + this.#destroyPipes(); + this.#settleInstructionWaiters({ + success: false, + exitCode: null, + stdout: this.#stdout, + stderr: error.message, + }); + reject(error); + }); this.#child.once("close", complete); this.#child.once("exit", (exitCode) => { - if (process.platform !== "win32") return; fallback = setTimeout(() => { - this.#child.stdout.destroy(); - this.#child.stderr.destroy(); + this.#destroyPipes(); complete(exitCode); - }, 1_000); + }, LOGIN_CHILD_TERMINATION_GRACE_MS); }); }); } @@ -132,10 +144,38 @@ export class CodexLoginHandle { } public cancel(): void { - if (this.#child.exitCode === null) { - this.#canceled = true; - this.#child.kill("SIGTERM"); + this.#canceled = true; + if ( + this.#child.exitCode !== null || + this.#child.signalCode !== null || + this.#forceCompletion === null + ) { + this.#destroyPipes(); + this.#forceCompletion?.(); + return; } + this.#child.kill("SIGTERM"); + if (this.#forcedTermination !== undefined) return; + this.#forcedTermination = setTimeout(() => { + this.#forcedTermination = undefined; + if (this.#child.exitCode === null && this.#child.signalCode === null) { + this.#child.kill("SIGKILL"); + } + this.#destroyPipes(); + this.#forceCompletion?.(); + }, LOGIN_CHILD_TERMINATION_GRACE_MS); + } + + #clearForcedTermination(): void { + if (this.#forcedTermination === undefined) return; + clearTimeout(this.#forcedTermination); + this.#forcedTermination = undefined; + } + + #destroyPipes(): void { + this.#child.stdin.destroy(); + this.#child.stdout.destroy(); + this.#child.stderr.destroy(); } #notifyInstructions(): void { diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index 1d7b2688..e63ee1a1 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -109,6 +109,7 @@ const SCAN_HISTORY_OUTPUT_OPTION = /^--(?:format|filter-output|full-output|token-count|token-limit|token-offset)(?:=|$)/u; const HIDE_CURSOR = "\u001B[?25l"; const SHOW_CURSOR = "\u001B[?25h"; +const CHILD_TERMINATION_GRACE_MS = 1_000; type Writable = Pick & { readonly isTTY?: boolean; @@ -434,13 +435,29 @@ export async function runCodexSkillCommand( windowsHide: true, }); let requestedSignal: SignalName | null = null; + let forcedTermination: ReturnType | undefined; + let forceStatusCompletion: (() => void) | null = null; + let forceCaptureCompletion: (() => void) | null = null; + const requestTermination = (signal: SignalName): void => { + requestedSignal = signal; + invocation.kill(signal); + if (forcedTermination !== undefined) return; + forcedTermination = setTimeout(() => { + forcedTermination = undefined; + if (invocation.exitCode === null && invocation.signalCode === null) { + invocation.kill("SIGKILL"); + } + forceCaptureCompletion?.(); + invocation.stdout?.destroy(); + invocation.stderr?.destroy(); + forceStatusCompletion?.(); + }, CHILD_TERMINATION_GRACE_MS); + }; const onInterrupt = (): void => { - requestedSignal = "SIGINT"; - invocation.kill("SIGINT"); + requestTermination("SIGINT"); }; const onTerminate = (): void => { - requestedSignal = "SIGTERM"; - invocation.kill("SIGTERM"); + requestTermination("SIGTERM"); }; process.on("SIGINT", onInterrupt); process.on("SIGTERM", onTerminate); @@ -452,22 +469,38 @@ export async function runCodexSkillCommand( const captured = output === undefined || invocation.stdout === null ? Promise.resolve(undefined) - : readSkillCommandOutput(invocation.stdout); + : Promise.race([ + readSkillCommandOutput(invocation.stdout), + new Promise((resolve) => { + forceCaptureCompletion = () => resolve(undefined); + }), + ]); const [status, events] = await Promise.all([ new Promise((resolve, reject) => { - invocation.once("error", reject); - invocation.once( - output === undefined ? "exit" : "close", - (code, signal) => { - resolve( - requestedSignal === "SIGINT" || signal === "SIGINT" - ? 130 - : requestedSignal === "SIGTERM" || signal === "SIGTERM" - ? 143 - : code ?? 1, - ); - }, - ); + let completed = false; + const complete = ( + code: number | null, + signal: NodeJS.Signals | null, + ): void => { + if (completed) return; + completed = true; + forceStatusCompletion = null; + resolve( + requestedSignal === "SIGINT" || signal === "SIGINT" + ? 130 + : requestedSignal === "SIGTERM" || signal === "SIGTERM" + ? 143 + : code ?? 1, + ); + }; + forceStatusCompletion = () => complete(null, null); + invocation.once("error", (error) => { + if (completed) return; + completed = true; + forceStatusCompletion = null; + reject(error); + }); + invocation.once(output === undefined ? "exit" : "close", complete); }), captured, ]); @@ -494,6 +527,9 @@ export async function runCodexSkillCommand( invocation.kill(); throw error; } finally { + if (forcedTermination !== undefined) clearTimeout(forcedTermination); + forceStatusCompletion = null; + forceCaptureCompletion = null; process.off("SIGINT", onInterrupt); process.off("SIGTERM", onTerminate); } diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 1a8183e2..84174d0a 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -3921,14 +3921,14 @@ if (process.argv.slice(2).join(" ") !== "login status") { } }); - test("cancels interactive login children during close", async () => { + test("forces interactive login children to settle during close", async () => { const root = await temporaryDirectory(); const codexHome = join(root, "codex-home"); const fakeCodex = join(root, "codex.mjs"); await mkdir(codexHome); await writeFile( fakeCodex, - 'console.error("Open https://auth.example.test/device");\nconsole.error("User code: ABCD-EFGH");\nsetInterval(() => {}, 1000);\n', + 'console.error("Open https://auth.example.test/device");\nconsole.error("User code: ABCD-EFGH");\nprocess.on("SIGTERM", () => {});\nsetInterval(() => {}, 1000);\n', ); const client = new TestClient( {}, @@ -3959,7 +3959,19 @@ if (process.argv.slice(2).join(" ") !== "login status") { const login = await client.loginChatGPTDeviceCode(); expect(login.verificationUrl).toBe("https://auth.example.test/device"); expect(login.userCode).toBe("ABCD-EFGH"); - await client.close(); + const timeout = AbortSignal.timeout(5_000); + await expect( + Promise.race([ + client.close(), + new Promise((_, reject) => { + timeout.addEventListener( + "abort", + () => reject(new Error("SDK close did not settle login cleanup.")), + { once: true }, + ); + }), + ]), + ).resolves.toBeUndefined(); await expect(login.wait()).resolves.toMatchObject({ success: false }); }); diff --git a/sdk/typescript/tests-ts/auth.test.ts b/sdk/typescript/tests-ts/auth.test.ts index 5e95da2f..b67848c1 100644 --- a/sdk/typescript/tests-ts/auth.test.ts +++ b/sdk/typescript/tests-ts/auth.test.ts @@ -237,7 +237,7 @@ grandchild.once("error", (error) => { ); test.skipIf(process.platform !== "win32")( - "releases native login pipes when the Windows fallback fires", + "releases native login pipes when the cross-platform fallback fires", async () => { const root = await mkdtemp(join(tmpdir(), "codex-security-auth-pipes-")); temporaryDirectories.push(root); @@ -350,7 +350,7 @@ grandchild.once("error", (error) => { new Promise((_, reject) => { timeout.addEventListener( "abort", - () => reject(new Error("The Windows login fallback timed out.")), + () => reject(new Error("The login fallback timed out.")), { once: true }, ); }), @@ -380,7 +380,7 @@ grandchild.once("error", (error) => { } if (Date.now() >= deadline) { throw new Error( - "The Windows login grandchild did not exit after pipe cleanup.", + "The login grandchild did not exit after pipe cleanup.", ); } await delay(25); @@ -390,6 +390,41 @@ grandchild.once("error", (error) => { }, ); + test("escalates cancellation when a login child ignores SIGTERM", async () => { + const root = await mkdtemp(join(tmpdir(), "codex-security-auth-sigkill-")); + temporaryDirectories.push(root); + const script = join(root, "codex.mjs"); + await writeFile( + script, + ` +console.error("Open https://auth.example.test/device"); +console.error("User code: ABCD-EFGH"); +process.on("SIGTERM", () => {}); +setInterval(() => {}, 1000); +`, + ); + let succeeded = false; + const handle = new CodexLoginHandle( + { command: process.execPath, prefixArgs: [script] }, + ["login", "--device-auth"], + process.env, + () => { + succeeded = true; + }, + ); + await handle.waitForInstructions({ deviceCode: true }); + handle.cancel(); + await expect( + Promise.race([ + handle.wait(), + delay(5_000).then(() => { + throw new Error("Login cancellation did not settle."); + }), + ]), + ).resolves.toMatchObject({ success: false }); + expect(succeeded).toBe(false); + }); + test("does not report a canceled interactive login as successful", async () => { const root = await mkdtemp(join(tmpdir(), "codex-security-auth-cancel-")); temporaryDirectories.push(root); diff --git a/sdk/typescript/tests-ts/cli-skills.test.ts b/sdk/typescript/tests-ts/cli-skills.test.ts index bb7f5c27..82dba4b5 100644 --- a/sdk/typescript/tests-ts/cli-skills.test.ts +++ b/sdk/typescript/tests-ts/cli-skills.test.ts @@ -1,6 +1,8 @@ +import { spawn } from "node:child_process"; import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, resolve } from "node:path"; +import { setTimeout as delay } from "node:timers/promises"; import { describe, expect, test } from "bun:test"; import { main, @@ -527,4 +529,92 @@ describe("CLI skill commands", () => { expect(stderr.text()).not.toContain("/private"); } }); + + test("forces a skill child to settle when it ignores SIGTERM", async () => { + const directory = await mkdtemp( + join(tmpdir(), "codex-security-skill-signal-"), + ); + const ready = join(directory, "ready"); + const child = join(directory, "child.mjs"); + const wrapper = join(directory, "wrapper.mjs"); + await writeFile( + child, + ` +import { spawn } from "node:child_process"; +import { writeFileSync } from "node:fs"; +const descendant = spawn( + process.execPath, + ["-e", "setInterval(() => {}, 1000)"], + { stdio: ["ignore", "inherit", "inherit"], windowsHide: true }, +); +writeFileSync(${JSON.stringify(ready)}, JSON.stringify({ + child: process.pid, + descendant: descendant.pid, +})); +process.on("SIGTERM", () => {}); +setInterval(() => {}, 1000); +`, + ); + await writeFile( + wrapper, + ` +import { runCodexSkillCommand } from ${JSON.stringify(new URL("../src/cli.ts", import.meta.url).href)}; +const status = await runCodexSkillCommand( + [], + { command: "validate", stdout: process.stdout, stderr: process.stderr }, + { command: process.execPath, prefixArgs: [${JSON.stringify(child)}] }, +); +process.exit(status); +`, + ); + + const invocation = spawn(process.execPath, [wrapper], { + stdio: "ignore", + windowsHide: true, + }); + let childPids: number[] = []; + try { + const deadline = Date.now() + 5_000; + while (true) { + try { + const marker = JSON.parse(await Bun.file(ready).text()) as { + child: number; + descendant: number; + }; + childPids = [marker.child, marker.descendant]; + break; + } catch (error) { + if (Date.now() >= deadline) throw error; + await delay(25); + } + } + invocation.kill("SIGTERM"); + const status = await Promise.race([ + new Promise((resolve, reject) => { + invocation.once("error", reject); + invocation.once("close", resolve); + }), + delay(5_000).then(() => { + throw new Error("CLI skill cancellation did not settle."); + }), + ]); + expect(status).toBe(143); + } finally { + invocation.kill("SIGKILL"); + for (const childPid of childPids) { + try { + process.kill(childPid, "SIGKILL"); + } catch (error) { + if ( + !(error instanceof Error) || + !("code" in error) || + error.code !== "ESRCH" + ) { + throw error; + } + } + } + await rm(directory, { recursive: true, force: true }); + } + }); }); From 8c90df9c30d7951aaa2b6c7bbd0581a856711579 Mon Sep 17 00:00:00 2001 From: GautamSharma99 Date: Thu, 30 Jul 2026 18:14:34 +0530 Subject: [PATCH 03/31] fix: bound authentication and skill output --- sdk/typescript/src/auth.ts | 191 ++++++++++++++++----- sdk/typescript/src/cli.ts | 77 ++++++++- sdk/typescript/tests-ts/auth.test.ts | 62 +++++++ sdk/typescript/tests-ts/cli-skills.test.ts | 52 ++++++ 4 files changed, 331 insertions(+), 51 deletions(-) diff --git a/sdk/typescript/src/auth.ts b/sdk/typescript/src/auth.ts index 55b72e5f..62c5c8dd 100644 --- a/sdk/typescript/src/auth.ts +++ b/sdk/typescript/src/auth.ts @@ -4,6 +4,12 @@ import { PluginBootstrapError } from "./errors.js"; import type { CodexCommand, ProcessEnvironment } from "./runtime.js"; const LOGIN_CHILD_TERMINATION_GRACE_MS = 1_000; +const MAX_CODEX_AUTH_OUTPUT_BYTES = 64 * 1024; +const LOGIN_OUTPUT_LIMIT_MESSAGE = + "Codex login output exceeded the 64 KiB safety limit."; +const COMMAND_OUTPUT_LIMIT_MESSAGE = + "Codex authentication command output exceeded the 64 KiB safety limit."; +const INSTRUCTION_TAIL_BYTES = 4 * 1024; export interface LoginResult { success: boolean; @@ -33,6 +39,13 @@ export class CodexLoginHandle { #forceCompletion: (() => void) | null = null; #stdout = ""; #stderr = ""; + #stdoutInstructionTail = ""; + #stderrInstructionTail = ""; + #stdoutAuthUrl: string | null = null; + #stderrAuthUrl: string | null = null; + #stdoutUserCode: string | null = null; + #stderrUserCode: string | null = null; + #outputLimitExceeded = false; public constructor( command: CodexCommand, @@ -59,12 +72,10 @@ export class CodexLoginHandle { this.#child.stdout.setEncoding("utf8"); this.#child.stderr.setEncoding("utf8"); this.#child.stdout.on("data", (chunk: string) => { - this.#stdout += chunk; - this.#notifyInstructions(); + this.#recordOutput("stdout", chunk); }); this.#child.stderr.on("data", (chunk: string) => { - this.#stderr += chunk; - this.#notifyInstructions(); + this.#recordOutput("stderr", chunk); }); this.#completion = new Promise((resolve, reject) => { let fallback: ReturnType | undefined; @@ -76,12 +87,19 @@ export class CodexLoginHandle { this.#clearForcedTermination(); this.#forceCompletion = null; this.#destroyPipes(); - const result = { - success: exitCode === 0 && !this.#canceled, - exitCode, - stdout: this.#stdout, - stderr: this.#stderr, - }; + const result = this.#outputLimitExceeded + ? { + success: false, + exitCode, + stdout: "", + stderr: LOGIN_OUTPUT_LIMIT_MESSAGE, + } + : { + success: exitCode === 0 && !this.#canceled, + exitCode, + stdout: this.#stdout, + stderr: this.#stderr, + }; this.#settleInstructionWaiters(result); if (result.success) onSuccess(); resolve(result); @@ -117,7 +135,7 @@ export class CodexLoginHandle { } public get authUrl(): string | null { - return preferredAuthUrl(`${this.#stdout}\n${this.#stderr}`); + return this.#stdoutAuthUrl ?? this.#stderrAuthUrl; } public get verificationUrl(): string | null { @@ -125,12 +143,7 @@ export class CodexLoginHandle { } public get userCode(): string | null { - const output = plainTerminalText(`${this.#stdout}\n${this.#stderr}`); - return ( - output.match(/(?:code|user code)\s*[:=]\s*([A-Z0-9-]{4,})/i)?.[1] ?? - output.match(/\b[A-Z0-9]{4,}(?:-[A-Z0-9]{4,})+\b/)?.[0] ?? - null - ); + return this.#stdoutUserCode ?? this.#stderrUserCode; } public async wait(): Promise { @@ -178,6 +191,51 @@ export class CodexLoginHandle { this.#child.stderr.destroy(); } + #recordOutput(stream: "stdout" | "stderr", chunk: string): void { + if (this.#outputLimitExceeded) return; + const current = stream === "stdout" ? this.#stdout : this.#stderr; + const appended = appendUtf8Tail( + current, + chunk, + MAX_CODEX_AUTH_OUTPUT_BYTES, + ); + if (stream === "stdout") { + this.#stdout = appended.value; + } else { + this.#stderr = appended.value; + } + if (appended.exceeded) { + this.#outputLimitExceeded = true; + this.#stdout = ""; + this.#stderr = LOGIN_OUTPUT_LIMIT_MESSAGE; + this.#child.kill("SIGTERM"); + return; + } + + const tail = + stream === "stdout" + ? this.#stdoutInstructionTail + : this.#stderrInstructionTail; + const candidate = `${tail}${chunk}`; + const url = preferredAuthUrl(candidate); + const userCode = userCodeFromOutput(candidate); + const nextTail = appendUtf8Tail( + "", + candidate, + INSTRUCTION_TAIL_BYTES, + ).value; + if (stream === "stdout") { + this.#stdoutAuthUrl ??= url; + this.#stdoutUserCode ??= userCode; + this.#stdoutInstructionTail = nextTail; + } else { + this.#stderrAuthUrl ??= url; + this.#stderrUserCode ??= userCode; + this.#stderrInstructionTail = nextTail; + } + this.#notifyInstructions(); + } + #notifyInstructions(): void { if (!this.#urlReadySettled && this.authUrl !== null) { this.#urlReadySettled = true; @@ -296,18 +354,33 @@ export async function runCodex( }); let stdout = ""; let stderr = ""; + let processError: Error | null = null; + const recordOutput = (stream: "stdout" | "stderr", chunk: string): void => { + if (processError !== null) return; + const appended = appendUtf8Tail( + stream === "stdout" ? stdout : stderr, + chunk, + MAX_CODEX_AUTH_OUTPUT_BYTES, + ); + if (stream === "stdout") stdout = appended.value; + else stderr = appended.value; + if (!appended.exceeded) return; + stdout = ""; + stderr = ""; + processError = new PluginBootstrapError(COMMAND_OUTPUT_LIMIT_MESSAGE); + child.kill("SIGTERM"); + }; child.stdout.setEncoding("utf8"); child.stderr.setEncoding("utf8"); child.stdout.on("data", (chunk: string) => { - stdout += chunk; + recordOutput("stdout", chunk); }); child.stderr.on("data", (chunk: string) => { - stderr += chunk; + recordOutput("stderr", chunk); }); const completion = new Promise((resolve, reject) => { - let processError: Error | null = null; child.once("error", (error) => { - processError = error; + processError ??= error; }); child.stdin.on("error", (error: NodeJS.ErrnoException) => { // A short-lived command can close stdin before Node flushes the input. @@ -335,31 +408,65 @@ export async function runCodex( } function preferredAuthUrl(value: string): string | null { - const urls = [ - ...plainTerminalText(value).matchAll(/https?:\/\/[^\s<>]+/g), - ].map((match) => match[0].replace(/[.,;:!?)\]}]+$/, "")); - return ( - urls.find((url) => { - try { - const hostname = new URL(url).hostname.toLowerCase().replace(/\.$/, ""); - return ( - hostname !== "localhost" && - !hostname.endsWith(".localhost") && - !(isIP(hostname) === 4 && hostname.startsWith("127.")) && - hostname !== "0.0.0.0" && - hostname !== "[::1]" && - hostname !== "[::]" && - hostname !== "[::ffff:0:0]" && - !hostname.startsWith("[::ffff:7f") && - !hostname.startsWith("[::7f") - ); - } catch { - return false; + for (const match of plainTerminalText(value).matchAll( + /https?:\/\/[^\s<>]+/g, + )) { + const url = match[0].replace(/[.,;:!?)\]}]+$/, ""); + try { + const hostname = new URL(url).hostname.toLowerCase().replace(/\.$/, ""); + if ( + hostname !== "localhost" && + !hostname.endsWith(".localhost") && + !(isIP(hostname) === 4 && hostname.startsWith("127.")) && + hostname !== "0.0.0.0" && + hostname !== "[::1]" && + hostname !== "[::]" && + hostname !== "[::ffff:0:0]" && + !hostname.startsWith("[::ffff:7f") && + !hostname.startsWith("[::7f") + ) { + return url; } - }) ?? null + } catch { + continue; + } + } + return null; +} + +function userCodeFromOutput(value: string): string | null { + const output = plainTerminalText(value); + return ( + output.match(/(?:code|user code)\s*[:=]\s*([A-Z0-9-]{4,})/i)?.[1] ?? + output.match(/\b[A-Z0-9]{4,}(?:-[A-Z0-9]{4,})+\b/)?.[0] ?? + null ); } +function appendUtf8Tail( + current: string, + chunk: string, + maximumBytes: number, +): { value: string; exceeded: boolean } { + const currentBytes = Buffer.from(current); + const chunkBytes = Buffer.from(chunk); + const exceeded = currentBytes.length + chunkBytes.length > maximumBytes; + if (!exceeded) return { value: `${current}${chunk}`, exceeded: false }; + if (chunkBytes.length >= maximumBytes) { + return { + value: chunkBytes.subarray(chunkBytes.length - maximumBytes).toString(), + exceeded: true, + }; + } + const retained = currentBytes.subarray( + Math.max(0, currentBytes.length - (maximumBytes - chunkBytes.length)), + ); + return { + value: Buffer.concat([retained, chunkBytes]).toString(), + exceeded: true, + }; +} + function plainTerminalText(value: string): string { return value .replace(/\u001b\][^\u0007]*(?:\u0007|\u001b\\)/g, "") diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index e63ee1a1..f778f745 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -22,8 +22,7 @@ import { win32, } from "node:path"; import { cwd } from "node:process"; -import { createInterface } from "node:readline"; -import { Readable, Writable as NodeWritable } from "node:stream"; +import { Writable as NodeWritable } from "node:stream"; import { pipeline } from "node:stream/promises"; import { fileURLToPath, pathToFileURL } from "node:url"; import type { ModelReasoningEffort } from "@openai/codex-sdk"; @@ -102,6 +101,10 @@ const MAX_CODEX_OVERRIDE_VALUE_LENGTH = 64 * 1_024; const MAX_CODEX_OVERRIDE_DEPTH = 64; const MAX_SKILL_INPUT_BYTES = 1_024 * 1_024; const MAX_SKILL_INPUT_COUNT = 64; +const MAX_SKILL_EVENT_BYTES = 1_024 * 1_024; +const MAX_SKILL_RESPONSE_BYTES = 256 * 1_024; +const SKILL_OUTPUT_LIMIT_MESSAGE = + "Codex skill output exceeded the 1 MiB event or 256 KiB response safety limit."; const WINDOWS_NETWORK_PATH = /^[\\/]{2}/u; const WINDOWS_LOCAL_DEVICE_ROOT = /^[\\/]{2}[?.][\\/](?:[A-Za-z]:|Volume\{[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12}\}|GLOBALROOT[\\/]Device[\\/]HarddiskVolume[0-9]+)(?=[\\/]|$)/iu; @@ -2232,19 +2235,23 @@ export async function readSkillCommandOutput( let message: string | undefined; let error: string | undefined; let malformed = false; + let exceeded = false; - for await (const line of createInterface({ input: Readable.from(stream) })) { - if (line.trim().length === 0) continue; + const readLine = (bytes: Buffer): void => { + const content = + bytes.at(-1) === 0x0d ? bytes.subarray(0, bytes.length - 1) : bytes; + const line = content.toString("utf8"); + if (line.trim().length === 0) return; let event: unknown; try { event = JSON.parse(line); } catch { malformed = true; - continue; + return; } if (typeof event !== "object" || event === null) { malformed = true; - continue; + return; } const value = event as Record; if (value["type"] === "item.completed") { @@ -2257,7 +2264,11 @@ export async function readSkillCommandOutput( "text" in item && typeof item.text === "string" ) { - message = item.text; + if (Buffer.byteLength(item.text, "utf8") > MAX_SKILL_RESPONSE_BYTES) { + exceeded = true; + } else { + message = item.text; + } } } else if (value["type"] === "turn.failed") { const detail = value["error"]; @@ -2267,15 +2278,63 @@ export async function readSkillCommandOutput( "message" in detail && typeof detail.message === "string" ) { - error = detail.message; + if ( + Buffer.byteLength(detail.message, "utf8") > MAX_SKILL_RESPONSE_BYTES + ) { + exceeded = true; + } else { + error = detail.message; + } } } else if ( value["type"] === "error" && typeof value["message"] === "string" ) { - error = value["message"]; + if ( + Buffer.byteLength(value["message"], "utf8") > MAX_SKILL_RESPONSE_BYTES + ) { + exceeded = true; + } else { + error = value["message"]; + } + } + }; + + const pending = Buffer.alloc(MAX_SKILL_EVENT_BYTES); + let pendingBytes = 0; + let discardingOversizedLine = false; + for await (const rawChunk of stream) { + const chunk = Buffer.isBuffer(rawChunk) + ? rawChunk + : Buffer.from(rawChunk, "utf8"); + let start = 0; + while (start < chunk.length) { + const newline = chunk.indexOf(0x0a, start); + const end = newline === -1 ? chunk.length : newline; + const segment = chunk.subarray(start, end); + if (!discardingOversizedLine) { + if (pendingBytes + segment.length > MAX_SKILL_EVENT_BYTES) { + exceeded = true; + discardingOversizedLine = true; + pendingBytes = 0; + } else if (segment.length > 0) { + segment.copy(pending, pendingBytes); + pendingBytes += segment.length; + } + } + if (newline === -1) break; + if (!discardingOversizedLine) { + readLine(pending.subarray(0, pendingBytes)); + } + pendingBytes = 0; + discardingOversizedLine = false; + start = newline + 1; } } + if (!discardingOversizedLine && pendingBytes > 0) { + readLine(pending.subarray(0, pendingBytes)); + } + if (exceeded) throw new CodexSecurityError(SKILL_OUTPUT_LIMIT_MESSAGE); return { ...(message === undefined ? {} : { message }), ...(error === undefined ? {} : { error }), diff --git a/sdk/typescript/tests-ts/auth.test.ts b/sdk/typescript/tests-ts/auth.test.ts index b67848c1..76017649 100644 --- a/sdk/typescript/tests-ts/auth.test.ts +++ b/sdk/typescript/tests-ts/auth.test.ts @@ -100,6 +100,30 @@ describe("Codex authentication process boundary", () => { ).resolves.toMatchObject({ success: false, exitCode: 1 }); }); + test("rejects oversized noninteractive authentication output", async () => { + const root = await mkdtemp(join(tmpdir(), "codex-security-auth-output-")); + temporaryDirectories.push(root); + const secret = "sk-proj-SYNTHETIC_OVERSIZED_OUTPUT_SECRET"; + for (const stream of ["stdout", "stderr"] as const) { + const script = join(root, `${stream}.mjs`); + await writeFile( + script, + `process.${stream}.write(${JSON.stringify(secret)}.repeat(3_000), () => process.exit(0));\n`, + ); + const failure = await runCodex( + { command: process.execPath, prefixArgs: [script] }, + [], + process.env, + ).then( + () => null, + (error: unknown) => error, + ); + expect(failure).toBeInstanceOf(PluginBootstrapError); + expect(String(failure)).toContain("64 KiB safety limit"); + expect(String(failure)).not.toContain(secret); + } + }); + test("reports account state and performs logout", async () => { const command = await fakeCodex(); await expect(accountStatus(command, process.env)).resolves.toMatchObject({ @@ -127,6 +151,44 @@ describe("Codex authentication process boundary", () => { expect(succeeded).toBe(true); }); + test("bounds interactive output while retaining discovered instructions", async () => { + const root = await mkdtemp(join(tmpdir(), "codex-security-auth-output-")); + temporaryDirectories.push(root); + const script = join(root, "login.mjs"); + const secret = "sk-proj-SYNTHETIC_INTERACTIVE_OUTPUT_SECRET"; + await writeFile( + script, + ` +console.error("Open https://auth.example.test/device"); +console.error("User code: ABCD-EFGH"); +setTimeout(() => { + process.stderr.write(${JSON.stringify(secret)}.repeat(3_000), () => process.exit(0)); +}, 10); +`, + ); + let succeeded = false; + const handle = new CodexLoginHandle( + { command: process.execPath, prefixArgs: [script] }, + ["login", "--device-auth"], + process.env, + () => { + succeeded = true; + }, + ); + + await handle.waitForInstructions({ deviceCode: true }); + expect(handle.verificationUrl).toBe("https://auth.example.test/device"); + expect(handle.userCode).toBe("ABCD-EFGH"); + const result = await handle.wait(); + expect(result).toMatchObject({ + success: false, + stdout: "", + stderr: "Codex login output exceeded the 64 KiB safety limit.", + }); + expect(JSON.stringify(result)).not.toContain(secret); + expect(succeeded).toBe(false); + }); + test("drains native login stderr before resolving authentication", async () => { const root = await mkdtemp(join(tmpdir(), "codex-security-auth-stderr-")); temporaryDirectories.push(root); diff --git a/sdk/typescript/tests-ts/cli-skills.test.ts b/sdk/typescript/tests-ts/cli-skills.test.ts index 82dba4b5..b30aadd6 100644 --- a/sdk/typescript/tests-ts/cli-skills.test.ts +++ b/sdk/typescript/tests-ts/cli-skills.test.ts @@ -457,6 +457,58 @@ describe("CLI skill commands", () => { }); }); + test("drains and rejects oversized skill events and responses", async () => { + let drained = false; + async function* oversizedLine(): AsyncGenerator { + for (let remaining = 1_024 * 1_024 + 1; remaining > 0; ) { + const length = Math.min(64 * 1_024, remaining); + yield Buffer.alloc(length, 0x78); + remaining -= length; + } + yield Buffer.from( + '\n{"type":"item.completed","item":{"type":"agent_message","text":"must still drain"}}\n', + ); + drained = true; + } + await expect(readSkillCommandOutput(oversizedLine())).rejects.toThrow( + "Codex skill output exceeded the 1 MiB event", + ); + expect(drained).toBe(true); + + async function* oversizedResponse(): AsyncGenerator { + yield Buffer.from( + `${JSON.stringify({ + type: "item.completed", + item: { + type: "agent_message", + text: "x".repeat(256 * 1_024 + 1), + }, + })}\n`, + ); + } + await expect(readSkillCommandOutput(oversizedResponse())).rejects.toThrow( + "Codex skill output exceeded the 1 MiB event", + ); + + const stdout = capture(); + const stderr = capture(); + await expect( + runCodexSkillCommand( + [], + { command: "validate", stdout: stdout.stream, stderr: stderr.stream }, + { + command: process.execPath, + prefixArgs: [ + "-e", + 'process.stdout.write(JSON.stringify({type:"item.completed",item:{type:"agent_message",text:"x".repeat(256*1024+1)}})+"\\n")', + ], + }, + ), + ).rejects.toThrow("Codex skill output exceeded the 1 MiB event"); + expect(stdout.text()).toBe(""); + expect(stderr.text()).toBe(""); + }); + test("summarizes skill failures without echoing credentials or private paths", () => { const cases = [ ["401 sk-proj-SYNTHETIC_SECRET", "Authentication failed"], From c9d49d9c16310eaaa63610d6e44b1afd45926f7f Mon Sep 17 00:00:00 2001 From: GautamSharma99 Date: Thu, 30 Jul 2026 18:53:49 +0530 Subject: [PATCH 04/31] fix: bound configured plugin traversal --- sdk/typescript/src/runtime.ts | 66 +++++++++++++------ sdk/typescript/tests-ts/runtime.test.ts | 86 +++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 21 deletions(-) diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index 93b98c5e..a069789e 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -8,6 +8,7 @@ import { mkdir, mkdtemp, open, + opendir, readFile, readdir, realpath, @@ -1108,24 +1109,25 @@ async function pluginProjectionFingerprint( } else { paths = []; const pending = [canonical]; - let entries = 0; + let entries = 1; while (pending.length > 0) { throwIfSignalAborted(signal); const path = pending.pop()!; const metadata = await lstat(path); - if (++entries > MAX_PLUGIN_COPY_ENTRIES) { - throw new PluginBootstrapError( - `Plugin source exceeds the copy entry limit: ${path}`, - ); - } if (metadata.isSymbolicLink()) { throw new PluginBootstrapError( `Plugin contains an unsafe source path: ${path}`, ); } if (metadata.isDirectory()) { - for (const entry of await readdir(path)) { - pending.push(join(path, entry)); + for await (const entry of pluginDirectoryEntries(path, signal)) { + const child = join(path, entry); + if (++entries > MAX_PLUGIN_COPY_ENTRIES) { + throw new PluginBootstrapError( + `Plugin source exceeds the copy entry limit: ${child}`, + ); + } + pending.push(child); } } else if (metadata.isFile()) { paths.push(relative(canonical, path).split(sep).join("/")); @@ -1661,7 +1663,7 @@ async function copyPluginTree( { source, destination }, ]; const directories = new Map(); - let entries = 0; + let entries = 1; let size = 0; await mkdir(dirname(destination), { recursive: true, mode: 0o700 }); try { @@ -1670,18 +1672,27 @@ async function copyPluginTree( const current = pending.pop()!; await requirePluginAncestors(source, current.source, directories, signal); const metadata = await lstat(current.source); - if (++entries > MAX_PLUGIN_COPY_ENTRIES) { - throw new PluginBootstrapError( - `Plugin source exceeds the copy entry limit: ${current.source}`, - ); - } if (metadata.isSymbolicLink()) { throw new PluginBootstrapError( `Plugin contains an unsafe source path: ${current.source}`, ); } if (metadata.isDirectory()) { - const children = await readdir(current.source); + for await (const entry of pluginDirectoryEntries( + current.source, + signal, + )) { + const childSource = join(current.source, entry); + if (++entries > MAX_PLUGIN_COPY_ENTRIES) { + throw new PluginBootstrapError( + `Plugin source exceeds the copy entry limit: ${childSource}`, + ); + } + pending.push({ + source: childSource, + destination: join(current.destination, entry), + }); + } const afterRead = await lstat(current.source); if (!samePluginFile(metadata, afterRead)) { throw new PluginBootstrapError( @@ -1690,12 +1701,6 @@ async function copyPluginTree( } directories.set(current.source, afterRead); await mkdir(current.destination, { mode: 0o700 }); - for (const child of children) { - pending.push({ - source: join(current.source, child), - destination: join(current.destination, child), - }); - } continue; } if (!metadata.isFile()) { @@ -1767,6 +1772,25 @@ async function copyPluginTree( } } +async function* pluginDirectoryEntries( + path: string, + signal?: AbortSignal, +): AsyncGenerator { + throwIfSignalAborted(signal); + const directory = await opendir(path); + try { + for (;;) { + throwIfSignalAborted(signal); + const entry = await directory.read(); + throwIfSignalAborted(signal); + if (entry === null) return; + yield entry.name; + } + } finally { + await directory.close(); + } +} + async function requirePluginAncestors( root: string, path: string, diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index aa334915..3734d378 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -385,6 +385,92 @@ describe("plugin runtime preparation", () => { ).toBeDefined(); }); + test("bounds configured plugin directory discovery", async () => { + const overflowRoot = await temporaryDirectory(); + const overflowSource = await plugin(overflowRoot); + const overflowDirectory = join(overflowSource, "many-files"); + await mkdir(overflowDirectory); + for (let offset = 0; offset < 4_096; offset += 128) { + await Promise.all( + Array.from({ length: 128 }, (_value, index) => + writeFile(join(overflowDirectory, String(offset + index)), ""), + ), + ); + } + const overflowDestination = join(overflowRoot, "overflow-home"); + await expect( + createMarketplace(overflowDestination, overflowSource), + ).rejects.toThrow("copy entry limit"); + expect( + existsSync( + join( + overflowDestination, + "sdk-marketplace", + "plugins", + "codex-security", + ), + ), + ).toBe(false); + }); + + test("cancels configured plugin directory discovery", async () => { + const cancellationRoot = await temporaryDirectory(); + const cancellationSource = await plugin(cancellationRoot); + const cancellationDirectory = join(cancellationSource, "many-files"); + await mkdir(cancellationDirectory); + await Promise.all( + Array.from({ length: 32 }, (_value, index) => + writeFile(join(cancellationDirectory, String(index)), ""), + ), + ); + const cancellationDestination = join(cancellationRoot, "canceled-home"); + const controller = new AbortController(); + const originalOpendir = fsPromises.opendir; + let discovered = 0; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + opendir: async (...args: Parameters) => { + const directory = await originalOpendir(...args); + if (String(args[0]) !== cancellationDirectory) return directory; + const originalRead = directory.read.bind(directory); + directory.read = async () => { + const entry = await originalRead(); + discovered += 1; + if (discovered === 2) { + controller.abort(new DOMException("canceled", "AbortError")); + } + return entry; + }; + return directory; + }, + })); + try { + await expect( + createMarketplace( + cancellationDestination, + cancellationSource, + controller.signal, + ), + ).rejects.toMatchObject({ name: "AbortError" }); + expect(discovered).toBe(2); + expect( + existsSync( + join( + cancellationDestination, + "sdk-marketplace", + "plugins", + "codex-security", + ), + ), + ).toBe(false); + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + opendir: originalOpendir, + })); + } + }); + testPosix( "rejects plugin symlinks and removes the partial marketplace", async () => { From 6b7e4f31a28dabbf8cfcb2c6c6c644b724e1cb90 Mon Sep 17 00:00:00 2001 From: GautamSharma99 Date: Thu, 30 Jul 2026 19:40:32 +0530 Subject: [PATCH 05/31] fix: bound cost session events --- sdk/typescript/README.md | 4 +- sdk/typescript/src/cost.ts | 65 +++++++++++++++++++++------- sdk/typescript/tests-ts/cost.test.ts | 56 ++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 17 deletions(-) diff --git a/sdk/typescript/README.md b/sdk/typescript/README.md index 9e1f4a51..3ae5bb25 100644 --- a/sdk/typescript/README.md +++ b/sdk/typescript/README.md @@ -380,7 +380,9 @@ including cached input and cache writes; fees and surcharges are not included. Use `--max-cost USD` to stop a scan, including its delegated workers, when its running cost exceeds the limit. Partial results are preserved. Requests -already in progress can finish above the limit. +already in progress can finish above the limit. Cost tracking accepts Codex +session events up to 1 MiB; an oversized event stops the scan because its +running cost can no longer be verified safely. Run `npx @openai/codex-security scan --help` or `npx @openai/codex-security bulk-scan --help` for the complete CLI references. diff --git a/sdk/typescript/src/cost.ts b/sdk/typescript/src/cost.ts index 912ccd27..1481e9dc 100644 --- a/sdk/typescript/src/cost.ts +++ b/sdk/typescript/src/cost.ts @@ -28,7 +28,9 @@ interface ScanTokenUsage { interface SessionUsage { offset: number; - remainder: Buffer; + pendingLine: Buffer[]; + pendingLineBytes: number; + unreadable: boolean; threadId: string | null; parentThreadId: string | null; usage: ScanTokenUsage | null; @@ -56,6 +58,7 @@ const MODEL_PRICING_NANODOLLARS: Readonly> = { const COST_POLL_INTERVAL_MS = 100; const SESSION_READ_SIZE = 64 * 1_024; +const MAX_SESSION_EVENT_BYTES = 1 * 1_024 * 1_024; export class ScanCostTracker { readonly #options: ScanCostTrackerOptions; @@ -115,7 +118,9 @@ export class ScanCostTracker { if (session === undefined) { session = { offset: 0, - remainder: Buffer.alloc(0), + pendingLine: [], + pendingLineBytes: 0, + unreadable: false, threadId: null, parentThreadId: null, usage: null, @@ -187,6 +192,7 @@ async function readSessionUsage( path: string, session: SessionUsage, ): Promise { + if (session.unreadable) return; let file; try { file = await open(path, "r"); @@ -205,27 +211,54 @@ async function readSessionUsage( ); if (bytesRead === 0) return; session.offset += bytesRead; - const contents = Buffer.concat([ - session.remainder, - buffer.subarray(0, bytesRead), - ]); - let lineStart = 0; - while (true) { - const lineEnd = contents.indexOf(0x0a, lineStart); - if (lineEnd === -1) break; - readSessionEvent( - contents.subarray(lineStart, lineEnd).toString("utf8"), - session, - ); - lineStart = lineEnd + 1; + try { + readSessionChunk(buffer.subarray(0, bytesRead), session); + } catch (error) { + session.unreadable = true; + session.pendingLine = []; + session.pendingLineBytes = 0; + throw error; } - session.remainder = Buffer.from(contents.subarray(lineStart)); } } finally { await file.close(); } } +function readSessionChunk(contents: Buffer, session: SessionUsage): void { + let lineStart = 0; + while (lineStart < contents.length) { + const newline = contents.indexOf(0x0a, lineStart); + const lineEnd = newline === -1 ? contents.length : newline; + const fragment = contents.subarray(lineStart, lineEnd); + const lineBytes = session.pendingLineBytes + fragment.length; + if (lineBytes > MAX_SESSION_EVENT_BYTES) { + throw new Error("Codex session event exceeds the 1 MiB safety limit."); + } + + if (newline === -1) { + if (fragment.length > 0) { + session.pendingLine.push(Buffer.from(fragment)); + session.pendingLineBytes = lineBytes; + } + return; + } + + if (session.pendingLineBytes === 0) { + readSessionEvent(fragment.toString("utf8"), session); + } else { + if (fragment.length > 0) session.pendingLine.push(Buffer.from(fragment)); + readSessionEvent( + Buffer.concat(session.pendingLine, lineBytes).toString("utf8"), + session, + ); + session.pendingLine = []; + session.pendingLineBytes = 0; + } + lineStart = newline + 1; + } +} + function readSessionEvent(line: string, session: SessionUsage): void { if (line.length === 0) return; let event: unknown; diff --git a/sdk/typescript/tests-ts/cost.test.ts b/sdk/typescript/tests-ts/cost.test.ts index 5e7d588b..9647d950 100644 --- a/sdk/typescript/tests-ts/cost.test.ts +++ b/sdk/typescript/tests-ts/cost.test.ts @@ -226,6 +226,62 @@ describe("live scan cost tracking", () => { }); }); + test("retains a bounded partial event across incremental reads", async () => { + const home = await codexHome(); + const path = await writeSession(home, "scan-thread", { + input_tokens: 100, + output_tokens: 10, + }); + const tracker = new ScanCostTracker({ + codexHome: home, + model: "gpt-5.6-terra", + }); + tracker.start("scan-thread"); + await tracker.refresh(); + + const event = JSON.stringify({ + type: "event_msg", + payload: { + type: "token_count", + info: { + total_token_usage: { input_tokens: 250, output_tokens: 20 }, + }, + }, + }); + const padding = " ".repeat(128 * 1_024); + await appendFile(path, `${padding}${event.slice(0, 40)}`); + expect((await tracker.refresh()).cost?.inputTokens).toBe(100); + + await appendFile(path, `${event.slice(40)}\n`); + expect((await tracker.stop()).cost?.inputTokens).toBe(250); + }); + + test("rejects and quarantines a session event larger than 1 MiB", async () => { + const home = await codexHome(); + const path = await writeSession(home, "scan-thread", { + input_tokens: 100, + output_tokens: 10, + }); + await appendFile(path, "x".repeat(1 * 1_024 * 1_024 + 1)); + const tracker = new ScanCostTracker({ + codexHome: home, + model: "gpt-5.6-terra", + }); + tracker.start("scan-thread"); + + await expect(tracker.refresh()).rejects.toThrow( + "Codex session event exceeds the 1 MiB safety limit.", + ); + expect((await tracker.refresh()).cost).toEqual({ + model: "gpt-5.6-terra", + inputTokens: 100, + cachedInputTokens: 0, + cacheWriteInputTokens: 0, + outputTokens: 10, + estimatedUsd: 0.0004, + }); + }); + test("reports a changed running cost only once", async () => { const home = await codexHome(); await writeSession(home, "scan-thread", { From 897ef5702c5a668f0464befdc6e6ac62cdf0f2f7 Mon Sep 17 00:00:00 2001 From: Hai Duong Date: Thu, 30 Jul 2026 14:04:49 +0700 Subject: [PATCH 06/31] fix: fail the scan on every turn.failed payload A failed turn was only honored when its error payload was an object with a string message. Every other shape fell through the event loop, and because a turn.completed had already been observed the scan was returned to the caller as successful and recorded with complete-scan. Treat any turn.failed as terminal. Only error.message is reused, because that is the single shape the previous code already surfaced; no other shape is forwarded or stringified, since this message reaches fail-scan --message and is stored in scans.failure_message without redaction. --- sdk/typescript/src/api.ts | 23 +++++-- sdk/typescript/tests-ts/api-events.test.ts | 79 ++++++++++++++++++++++ 2 files changed, 96 insertions(+), 6 deletions(-) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index cd9dd33a..091bd1e6 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -1395,12 +1395,8 @@ export async function runScanEvents( } else if (event.type === "turn.completed") { status = "completed"; usage = event["usage"]; - } else if ( - event.type === "turn.failed" && - isRecord(event["error"]) && - typeof event["error"]["message"] === "string" - ) { - throw new CodexSecurityError(event["error"]["message"]); + } else if (event.type === "turn.failed") { + throw new CodexSecurityError(turnFailureMessage(event["error"])); } else if ( event.type === "error" && typeof event["message"] === "string" @@ -1758,6 +1754,21 @@ function reconnectDetails(message: string): ScanReconnectDetails | undefined { }; } +// A failed turn must fail the scan whatever its error payload looks like. +// +// Only `error.message` is reused, because that is the single shape the previous +// code already surfaced. No other shape is forwarded or stringified: this message +// reaches `fail-scan --message` and is stored in `scans.failure_message` without +// redaction, so widening what is copied out of the payload would add a new +// credential-disclosure path to persistent scan history. +function turnFailureMessage(error: unknown): string { + if (isRecord(error) && typeof error["message"] === "string") { + const message = error["message"].trim(); + if (message.length > 0) return error["message"]; + } + return "The Codex Security scan turn failed without a readable error message."; +} + export function classifyConnectionFailure( error: unknown, ): diff --git a/sdk/typescript/tests-ts/api-events.test.ts b/sdk/typescript/tests-ts/api-events.test.ts index dca44fda..2858a469 100644 --- a/sdk/typescript/tests-ts/api-events.test.ts +++ b/sdk/typescript/tests-ts/api-events.test.ts @@ -355,6 +355,85 @@ describe("one-shot scan events", () => { }); }); + test("fails the scan for every turn.failed error payload shape", async () => { + const usage = { + input_tokens: 10, + cached_input_tokens: 2, + output_tokens: 3, + reasoning_output_tokens: 1, + }; + const payloads: Array<[string, unknown]> = [ + ["object message", { message: "model refused the turn" }], + ["null", null], + ["undefined", undefined], + ["string", "model refused the turn"], + ["non-string message", { message: { text: "model refused the turn" } }], + ["array", ["model refused the turn"]], + ["blank message", { message: " " }], + ]; + for (const [label, error] of payloads) { + // A complete, valid artifact bundle so the failed turn is the only variable. + const scanDir = await copyCompletedScan(await temporaryDirectory()); + async function* failedEvents(): AsyncGenerator { + yield { type: "thread.started", thread_id: "thread-1" }; + yield { type: "turn.completed", usage }; + yield { type: "turn.failed", error } as unknown as ThreadEvent; + } + await expect( + runEvents(scanDir, failedEvents()), + `turn.failed carrying ${label} must fail the scan`, + ).rejects.toMatchObject({ name: CodexSecurityError.name }); + } + }); + + test("reuses only a nested turn.failed message and falls back otherwise", async () => { + const usage = { + input_tokens: 10, + cached_input_tokens: 2, + output_tokens: 3, + reasoning_output_tokens: 1, + }; + async function* failedWith(error: unknown): AsyncGenerator { + yield { type: "thread.started", thread_id: "thread-1" }; + yield { type: "turn.completed", usage }; + yield { type: "turn.failed", error } as unknown as ThreadEvent; + } + + await expect( + runEvents( + await copyCompletedScan(await temporaryDirectory()), + failedWith({ message: "retry budget exhausted" }), + ), + ).rejects.toMatchObject({ + name: CodexSecurityError.name, + message: "retry budget exhausted", + }); + + // A bare string is deliberately NOT reused: it was never surfaced before, and + // this message is stored by fail-scan without redaction. + await expect( + runEvents( + await copyCompletedScan(await temporaryDirectory()), + failedWith("token sk-proj-EXAMPLE1234567890 rejected"), + ), + ).rejects.toMatchObject({ + name: CodexSecurityError.name, + message: + "The Codex Security scan turn failed without a readable error message.", + }); + + await expect( + runEvents( + await copyCompletedScan(await temporaryDirectory()), + failedWith({ code: 500 }), + ), + ).rejects.toMatchObject({ + name: CodexSecurityError.name, + message: + "The Codex Security scan turn failed without a readable error message.", + }); + }); + test("extracts bounded rate-limit context from reconnect notifications", async () => { const scanDir = await copyCompletedScan(await temporaryDirectory()); const reconnects: Array<{ From 1d261cf6e8fcab07e7c39f63ed594be9f5e3b692 Mon Sep 17 00:00:00 2001 From: Hai Duong Date: Thu, 30 Jul 2026 17:32:54 +0700 Subject: [PATCH 07/31] fix: keep scan cleanup failures from replacing the scan result The scan finally block awaited knowledge-base cleanup, target-paths removal, and the credential home release without guarding any of them. A rejection in a finally block replaces the pending exception, so a cleanup failure discarded the outcome the try and catch blocks had already produced and the caller was handed an unrelated filesystem error instead of the reason the scan failed. The catch block had already recorded the real message through fail-scan, so the persisted scan history and the reported error disagreed. Removing the temporary inputs is best effort and is now reported through onWarning instead of deciding the result. Releasing the credential home lock is not: it only marks itself done once the lock directory is gone, so a failed release leaves an owner.json naming a live process that stale-lock recovery will not reclaim. That failure still propagates, and is downgraded to a warning only when the scan already failed and its error is the one worth keeping. The failed-scan marker is recorded on entry to the catch block because requireOpen and throwIfAborted raise a different error for the same failed scan, the release keeps its own finally so reporting the earlier failures cannot skip it, and the warning formatter is total so no hostile reason can throw in place of the result. --- sdk/typescript/src/api.ts | 56 +++++++++++++++++++++++++++-- sdk/typescript/tests-ts/api.test.ts | 53 +++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 3 deletions(-) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 091bd1e6..f68ef9d9 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -312,6 +312,7 @@ export class CodexSecurity { let knowledgeBase: PreparedKnowledgeBase | null = null; let costTracker: ScanCostTracker | null = null; let releaseCredentialHome: (() => Promise) | null = null; + let scanFailure = false; let completionCost: ScanCost | null = null; let activeScan: { id: string; @@ -833,6 +834,9 @@ export class CodexSecurity { } return result; } catch (error) { + // Recorded first: everything below can throw a different error for this same failed + // scan, and cleanup must treat all of those as a failure it is not allowed to mask. + scanFailure = true; const snapshot = await costTracker?.stop().catch(() => null); const failure = signal.reason instanceof ScanCostLimitExceededError @@ -860,13 +864,37 @@ export class CodexSecurity { } throw failure; } finally { + // Removing the temporary scan inputs is best effort. A throw here would replace the + // outcome the try and catch blocks already produced, so these failures are reported + // as warnings: a scan that failed has to say why it failed, not why its temporary + // files outlived it. The whole step is guarded so that a cleanup which rejects, or + // throws synchronously, still cannot skip the credential lock release below. try { - await Promise.all([ + for (const cleanup of await Promise.allSettled([ knowledgeBase?.cleanup(), removeTargetPathsFile(targetPathsFile), - ]); + ])) { + if (cleanup.status === "rejected") { + warnCleanupFailed(options, cleanup.reason); + } + } + } catch (error) { + warnCleanupFailed(options, error); } finally { - await releaseCredentialHome?.(); + // Releasing the credential home lock is not best effort, so it keeps its own + // finally and runs even if reporting the failures above went wrong. The release + // only marks itself done once the lock directory is gone, so a failure leaves an + // owner.json naming this still-running process; recoverStaleCredentialHomeLock + // then refuses to reclaim it because that pid is alive, and later scans in this + // process wait on a lock nothing frees. Reporting success while leaving the client + // in that state is worse than failing, so the failure is only downgraded to a + // warning when the scan already failed and that error is the one worth keeping. + try { + await releaseCredentialHome?.(); + } catch (error) { + if (!scanFailure) throw error; + warnCleanupFailed(options, error); + } } } } @@ -1319,6 +1347,28 @@ export async function initialCredentialsAvailable( return await importer(ambientHome, isolatedHome); } +// Reports a cleanup failure without letting it decide the result of the scan. Only the +// message is forwarded, and it reaches the onWarning observer alone: unlike the fail-scan +// path it is never written to the workbench, so it adds no persisted, unredacted text. +function warnCleanupFailed( + options: Pick, + reason: unknown, +): void { + // This runs where a throw would replace the scan result, so every step is inside the + // guard: reading the reason, coercing it, and reading the observers off the options can + // each throw for a sufficiently hostile value, and none of them may become the outcome + // of the scan. Losing a warning is the correct trade against losing the result. + try { + const message = String(reason instanceof Error ? reason.message : reason); + notifyObserver( + "onWarning", + options.onWarning, + options.onObserverError, + `Could not clean up after the Codex Security scan: ${message}`, + ); + } catch {} +} + async function removeTargetPathsFile(path: string | null): Promise { if (path === null) return; try { diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 84174d0a..0eb36159 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -1031,6 +1031,59 @@ describe("CodexSecurity orchestration", () => { await client.close(); }); + test("reports the real scan failure when scan cleanup also fails", async () => { + const root = await temporaryDirectory(); + const repository = join(root, "repository"); + const codexHome = join(root, "codex-home"); + await mkdir(join(repository, "src"), { recursive: true }); + await mkdir(codexHome); + let failedCleanupPath: string | undefined; + const warnings: string[] = []; + const client = new TestClient( + {}, + { + environment: { OPENAI_API_KEY: "test-key" }, + prepareRuntime: async () => preparedRuntime(codexHome), + resolvePluginPython: async () => "/managed/python", + repositoryRevision: async () => "deadbeef", + createCodex: (options: CodexOptions) => ({ + startThread: () => ({ + id: null, + async runStreamed() { + // Cleanup removes the target paths file with a non-recursive rm, so + // replacing that file with a directory makes cleanup reject on every + // platform. + failedCleanupPath = + options.env?.["CODEX_SECURITY_TARGET_PATHS_FILE"]; + await rm(failedCleanupPath!, { force: true }); + await mkdir(failedCleanupPath!); + throw new Error("the model refused the scan"); + }, + }), + }), + }, + ); + + await expect( + client.run(repository, { + target: ["src"], + outputDir: join(root, "scan"), + onWarning: (warning) => { + warnings.push(warning); + }, + }), + ).rejects.toThrow("the model refused the scan"); + expect(failedCleanupPath).toBeDefined(); + // The cleanup failure is reported rather than discarded. + await new Promise((resolve) => setTimeout(resolve, 10)); + expect( + warnings.filter((warning) => + warning.startsWith("Could not clean up after the Codex Security scan:"), + ), + ).toHaveLength(1); + await client.close(); + }); + test("rejects scan output inside the repository before runtime initialization", async () => { const root = await temporaryDirectory(); const repository = join(root, "repository"); From 58b2adde70740b4250219aea3a50db250d92c64e Mon Sep 17 00:00:00 2001 From: GautamSharma99 Date: Thu, 30 Jul 2026 19:35:44 +0530 Subject: [PATCH 08/31] fix: prioritize active preflight config --- sdk/typescript/src/api.ts | 56 ++++++++++++--- sdk/typescript/tests-ts/api.test.ts | 102 ++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 9 deletions(-) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index f68ef9d9..6b9be442 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -399,6 +399,14 @@ export class CodexSecurity { ) { await this.#refreshPersistentRuntime(runtime, scanEnvironment, signal); } + const effectiveConfig = + runtime.effectiveConfig ?? (await mergedCodexConfig(this.config)); + if (runtime.configPath !== undefined) { + await writeCodexConfig( + runtime.configPath, + scanPreflightCodexConfig(effectiveConfig, repo), + ); + } const runtimeHome = await realpath(runtime.codexHome); requireOutputOutsideRepository(protectedRoot, runtimeHome, "runtime"); if ( @@ -535,8 +543,6 @@ export class CodexSecurity { mode, pluginVersion: runtime.plugin.version, }; - const effectiveConfig = - runtime.effectiveConfig ?? (await mergedCodexConfig(this.config)); const { model } = scanModelConfiguration(effectiveConfig); validateScanCostLimit(options.maxCostUsd, model); const tracker = new ScanCostTracker({ @@ -1624,7 +1630,7 @@ function scanRecipe( mode, ...(repositoryRevision === null ? {} : { repositoryRevision }), pluginVersion, - config: scanPreflightCodexConfig(effectiveConfig), + config: scanPreflightCodexConfig(effectiveConfig, repository), ...(failOnSeverity === undefined ? {} : { failOnSeverity }), ...(knowledgeBasePaths === undefined ? {} : { knowledgeBasePaths }), ...(maxCostUsd === undefined ? {} : { maxCostUsd }), @@ -1894,7 +1900,10 @@ export function scanRuntimeCodexConfig( }; } -export function scanPreflightCodexConfig(config: JsonObject): JsonObject { +export function scanPreflightCodexConfig( + config: JsonObject, + activeProjectPath?: string, +): JsonObject { const safeString = (value: unknown, maxLength: number): value is string => typeof value === "string" && value.length > 0 && @@ -1964,18 +1973,41 @@ export function scanPreflightCodexConfig(config: JsonObject): JsonObject { } return result; }; + const prioritizedEntries = ( + value: Record, + priority: string | undefined, + ): [string, unknown][] => { + const entries = Object.entries(value); + if (priority === undefined || !Object.hasOwn(value, priority)) { + return entries; + } + return [ + [priority, value[priority]], + ...entries.filter(([key]) => key !== priority), + ]; + }; const result = executionConfig(config); - if (safeProfileName(config["profile"])) { - result["profile"] = config["profile"]; + const selectedProfile = safeProfileName(config["profile"]) + ? config["profile"] + : undefined; + if (selectedProfile !== undefined) { + result["profile"] = selectedProfile; } const profiles = config["profiles"]; if (isRecord(profiles)) { const sanitized: JsonObject = {}; - for (const [name, profile] of Object.entries(profiles).slice(0, 256)) { + let accepted = 0; + for (const [name, profile] of prioritizedEntries( + profiles, + selectedProfile, + )) { if (!safeProfileName(name) || !isRecord(profile)) continue; const projected = executionConfig(profile as JsonObject); - if (Object.keys(projected).length > 0) sanitized[name] = projected; + if (Object.keys(projected).length === 0) continue; + sanitized[name] = projected; + accepted += 1; + if (accepted === 256) break; } if (Object.keys(sanitized).length > 0) result["profiles"] = sanitized; } @@ -1988,13 +2020,19 @@ export function scanPreflightCodexConfig(config: JsonObject): JsonObject { const projects = config["projects"]; if (isRecord(projects)) { const sanitized: JsonObject = {}; - for (const [path, project] of Object.entries(projects).slice(0, 256)) { + let accepted = 0; + for (const [path, project] of prioritizedEntries( + projects, + activeProjectPath, + )) { if (!safeString(path, 4096) || !isAbsolute(path) || !isRecord(project)) { continue; } const trust = project["trust_level"]; if (trust !== "trusted" && trust !== "untrusted") continue; sanitized[path] = { trust_level: trust }; + accepted += 1; + if (accepted === 256) break; } if (Object.keys(sanitized).length > 0) result["projects"] = sanitized; } diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 0eb36159..5da18c97 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -496,6 +496,93 @@ describe("CodexSecurity orchestration", () => { ).resolves.toBeUndefined(); }); + test("prioritizes the selected profile and active project before projection limits", () => { + const activeProject = "/workspace/active"; + const profiles = Object.fromEntries([ + ...Array.from({ length: 256 }, (_, index) => [ + `profile_${index}`, + { features: { goals: index % 2 === 0 } }, + ]), + ["selected", { agents: { max_threads: 17 } }], + ]); + const projects = Object.fromEntries([ + ...Array.from({ length: 256 }, (_, index) => [ + `/workspace/project-${index}`, + { trust_level: "untrusted" }, + ]), + [activeProject, { trust_level: "trusted" }], + ]); + + const prioritized = scanPreflightCodexConfig( + { + profile: "selected", + profiles, + projects, + }, + activeProject, + ); + + expect(prioritized["profile"]).toBe("selected"); + expect(Object.keys(prioritized["profiles"] as JsonObject)).toHaveLength( + 256, + ); + expect(prioritized["profiles"]).toMatchObject({ + selected: { agents: { max_threads: 17 } }, + }); + expect(Object.keys(prioritized["projects"] as JsonObject)).toHaveLength( + 256, + ); + expect(prioritized["projects"]).toMatchObject({ + [activeProject]: { trust_level: "trusted" }, + }); + + const validProfiles = Object.fromEntries( + Array.from({ length: 256 }, (_, index) => [ + `valid_${index}`, + { features: { goals: true } }, + ]), + ); + const validProjects = Object.fromEntries( + Array.from({ length: 256 }, (_, index) => [ + `/valid/project-${index}`, + { trust_level: "trusted" }, + ]), + ); + const afterInvalid = scanPreflightCodexConfig({ + profiles: { + ...Object.fromEntries( + Array.from({ length: 256 }, (_, index) => [ + `invalid.profile.${index}`, + { features: { goals: false } }, + ]), + ), + ...validProfiles, + }, + projects: { + ...Object.fromEntries( + Array.from({ length: 256 }, (_, index) => [ + `relative-${index}`, + { trust_level: "trusted" }, + ]), + ), + ...validProjects, + }, + }); + + expect(Object.keys(afterInvalid["profiles"] as JsonObject)).toHaveLength( + 256, + ); + expect(afterInvalid["profiles"]).toMatchObject({ + valid_255: { features: { goals: true } }, + }); + expect(Object.keys(afterInvalid["projects"] as JsonObject)).toHaveLength( + 256, + ); + expect(afterInvalid["projects"]).toMatchObject({ + "/valid/project-255": { trust_level: "trusted" }, + }); + }); + test("selects a real-scan target in the active repository layout", async () => { await expect( stat(join(REPOSITORY_ROOT, INTEGRATION_TARGET)), @@ -2396,11 +2483,21 @@ describe("CodexSecurity orchestration", () => { expect(interpreter).not.toBeNull(); let capturedConfigPath: string | undefined; let capturedCodexHome: string | undefined; + const unrelatedProjects = Object.fromEntries( + Array.from({ length: 256 }, (_, index) => [ + join(root, `unrelated-project-${index}`), + { trust_level: "untrusted" }, + ]), + ); const client = new TestClient( { pluginPath: PLUGIN_ROOT, codexOverrides: { features: { goals: true }, + projects: { + ...unrelatedProjects, + [repository]: { trust_level: "trusted" }, + }, mcp_servers: { private: { command: "echo", @@ -2459,6 +2556,11 @@ describe("CodexSecurity orchestration", () => { expect(serialized).not.toContain("RUNTIME_SHELL_SECRET"); expect(serialized).not.toContain("mcp_servers"); expect(serialized).not.toContain("shell_environment_policy"); + expect(parseToml(serialized)).toMatchObject({ + projects: { + [repository]: { trust_level: "trusted" }, + }, + }); expect(input).toContain('--config "$CODEX_SECURITY_CONFIG_PATH"'); expect(input).toContain("--effective-config"); const shellEnvironment = options.env as Record; From 0409a6e7d6e50488bf6765cf5e6dcfb043822d37 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 09:59:29 -0700 Subject: [PATCH 09/31] fix: make bounded authentication shutdown deterministic Co-authored-by: mangeshraut712 --- sdk/typescript/src/auth.ts | 25 ++- sdk/typescript/tests-ts/api.test.ts | 1 + sdk/typescript/tests-ts/auth.test.ts | 265 +++++++++++++++++---------- 3 files changed, 196 insertions(+), 95 deletions(-) diff --git a/sdk/typescript/src/auth.ts b/sdk/typescript/src/auth.ts index 62c5c8dd..e12e5f16 100644 --- a/sdk/typescript/src/auth.ts +++ b/sdk/typescript/src/auth.ts @@ -158,6 +158,10 @@ export class CodexLoginHandle { public cancel(): void { this.#canceled = true; + this.#requestTermination(); + } + + #requestTermination(): void { if ( this.#child.exitCode !== null || this.#child.signalCode !== null || @@ -208,7 +212,7 @@ export class CodexLoginHandle { this.#outputLimitExceeded = true; this.#stdout = ""; this.#stderr = LOGIN_OUTPUT_LIMIT_MESSAGE; - this.#child.kill("SIGTERM"); + this.#requestTermination(); return; } @@ -355,6 +359,21 @@ export async function runCodex( let stdout = ""; let stderr = ""; let processError: Error | null = null; + let forcedTermination: ReturnType | undefined; + const terminate = (): void => { + if (child.exitCode !== null || child.signalCode !== null) return; + child.kill("SIGTERM"); + if (forcedTermination !== undefined) return; + forcedTermination = setTimeout(() => { + forcedTermination = undefined; + if (child.exitCode === null && child.signalCode === null) { + child.kill("SIGKILL"); + } + child.stdin.destroy(); + child.stdout.destroy(); + child.stderr.destroy(); + }, LOGIN_CHILD_TERMINATION_GRACE_MS); + }; const recordOutput = (stream: "stdout" | "stderr", chunk: string): void => { if (processError !== null) return; const appended = appendUtf8Tail( @@ -368,7 +387,7 @@ export async function runCodex( stdout = ""; stderr = ""; processError = new PluginBootstrapError(COMMAND_OUTPUT_LIMIT_MESSAGE); - child.kill("SIGTERM"); + terminate(); }; child.stdout.setEncoding("utf8"); child.stderr.setEncoding("utf8"); @@ -381,6 +400,7 @@ export async function runCodex( const completion = new Promise((resolve, reject) => { child.once("error", (error) => { processError ??= error; + terminate(); }); child.stdin.on("error", (error: NodeJS.ErrnoException) => { // A short-lived command can close stdin before Node flushes the input. @@ -396,6 +416,7 @@ export async function runCodex( } }); child.once("close", (exitCode) => { + if (forcedTermination !== undefined) clearTimeout(forcedTermination); if (processError !== null) { reject(processError); } else { diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 5da18c97..1f33a679 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -4156,6 +4156,7 @@ if (args === "login --with-api-key") { } } else if (args === "login") { console.error("Open https://auth.example.test/login"); + process.exit(0); } else { process.exitCode = 2; } diff --git a/sdk/typescript/tests-ts/auth.test.ts b/sdk/typescript/tests-ts/auth.test.ts index 76017649..9a424ded 100644 --- a/sdk/typescript/tests-ts/auth.test.ts +++ b/sdk/typescript/tests-ts/auth.test.ts @@ -124,6 +124,40 @@ describe("Codex authentication process boundary", () => { } }); + test("forces oversized noninteractive authentication to terminate", async () => { + const root = await mkdtemp( + join(tmpdir(), "codex-security-auth-command-kill-"), + ); + temporaryDirectories.push(root); + const script = join(root, "command.mjs"); + await writeFile( + script, + ` +process.on("SIGTERM", () => {}); +process.stderr.write("x".repeat(128 * 1024)); +setInterval(() => {}, 1000); +`, + ); + const timeout = AbortSignal.timeout(2_500); + + await expect( + Promise.race([ + runCodex( + { command: process.execPath, prefixArgs: [script] }, + [], + process.env, + ), + new Promise((_, reject) => { + timeout.addEventListener( + "abort", + () => reject(new Error("Oversized authentication did not settle.")), + { once: true }, + ); + }), + ]), + ).rejects.toThrow("64 KiB safety limit"); + }); + test("reports account state and performs logout", async () => { const command = await fakeCodex(); await expect(accountStatus(command, process.env)).resolves.toMatchObject({ @@ -189,6 +223,54 @@ setTimeout(() => { expect(succeeded).toBe(false); }); + test("forces an oversized login to settle when it ignores termination", async () => { + const root = await mkdtemp( + join(tmpdir(), "codex-security-auth-output-kill-"), + ); + temporaryDirectories.push(root); + const script = join(root, "login.mjs"); + await writeFile( + script, + ` +console.error("Open https://auth.example.test/device"); +console.error("User code: ABCD-EFGH"); +process.on("SIGTERM", () => {}); +setTimeout(() => { + process.stderr.write("x".repeat(128 * 1024)); + setInterval(() => {}, 1000); +}, 10); +`, + ); + const handle = new CodexLoginHandle( + { command: process.execPath, prefixArgs: [script] }, + ["login", "--device-auth"], + process.env, + () => {}, + ); + + try { + await handle.waitForInstructions({ deviceCode: true }); + const timeout = AbortSignal.timeout(2_500); + const result = await Promise.race([ + handle.wait(), + new Promise((_, reject) => { + timeout.addEventListener( + "abort", + () => reject(new Error("Oversized login did not settle.")), + { once: true }, + ); + }), + ]); + expect(result).toMatchObject({ + success: false, + stderr: "Codex login output exceeded the 64 KiB safety limit.", + }); + } finally { + handle.cancel(); + await handle.wait(); + } + }); + test("drains native login stderr before resolving authentication", async () => { const root = await mkdtemp(join(tmpdir(), "codex-security-auth-stderr-")); temporaryDirectories.push(root); @@ -298,15 +380,13 @@ grandchild.once("error", (error) => { }, ); - test.skipIf(process.platform !== "win32")( - "releases native login pipes when the cross-platform fallback fires", - async () => { - const root = await mkdtemp(join(tmpdir(), "codex-security-auth-pipes-")); - temporaryDirectories.push(root); - const ready = join(root, "grandchild-ready"); - const release = join(root, "release-grandchild"); - const script = join(root, "login-pipes.mjs"); - const grandchildScript = ` + test("releases native login pipes when the cross-platform fallback fires", async () => { + const root = await mkdtemp(join(tmpdir(), "codex-security-auth-pipes-")); + temporaryDirectories.push(root); + const ready = join(root, "grandchild-ready"); + const release = join(root, "release-grandchild"); + const script = join(root, "login-pipes.mjs"); + const grandchildScript = ` import { existsSync, writeFileSync } from "node:fs"; const ready = process.argv[1]; @@ -320,9 +400,9 @@ const watcher = setInterval(() => { }, 25); writeFileSync(ready, String(process.pid)); `; - await writeFile( - script, - ` + await writeFile( + script, + ` import { spawn } from "node:child_process"; import { existsSync } from "node:fs"; @@ -354,103 +434,102 @@ grandchild.once("error", (error) => { process.exit(1); }); `, - ); + ); - const originalOnce = ChildProcess.prototype.once; - let loginChild: ChildProcess | undefined; - const processObserver = spyOn(ChildProcess.prototype, "once"); - processObserver.mockImplementation(function ( - this: ChildProcess, - event: string, - listener: (...eventArguments: never[]) => void, - ) { - if (event === "exit") loginChild = this; - return Reflect.apply(originalOnce, this, [event, listener]); - }); + const originalOnce = ChildProcess.prototype.once; + let loginChild: ChildProcess | undefined; + const processObserver = spyOn(ChildProcess.prototype, "once"); + processObserver.mockImplementation(function ( + this: ChildProcess, + event: string, + listener: (...eventArguments: never[]) => void, + ) { + if (event === "exit") loginChild = this; + return Reflect.apply(originalOnce, this, [event, listener]); + }); - let handle: CodexLoginHandle; - try { - handle = new CodexLoginHandle( - { command: process.execPath, prefixArgs: [script] }, - ["login"], - process.env, - () => {}, - ); - } finally { - processObserver.mockRestore(); + let handle: CodexLoginHandle; + try { + handle = new CodexLoginHandle( + { command: process.execPath, prefixArgs: [script] }, + ["login"], + process.env, + () => {}, + ); + } finally { + processObserver.mockRestore(); + } + + const readMarker = async (path: string): Promise => { + const deadline = Date.now() + 5_000; + while (true) { + try { + return await readFile(path, "utf8"); + } catch (error) { + if ( + !(error instanceof Error) || + !("code" in error) || + error.code !== "ENOENT" || + Date.now() >= deadline + ) { + throw error; + } + await delay(25); + } } + }; - const readMarker = async (path: string): Promise => { + let grandchildPid: number | undefined; + try { + const readyMarker = await readMarker(ready); + expect(readyMarker).toMatch(/^\d+$/u); + grandchildPid = Number(readyMarker); + expect(Number.isSafeInteger(grandchildPid)).toBe(true); + expect(grandchildPid).toBeGreaterThan(0); + const timeout = AbortSignal.timeout(5_000); + const completion = Promise.race([ + handle.wait(), + new Promise((_, reject) => { + timeout.addEventListener( + "abort", + () => reject(new Error("The login fallback timed out.")), + { once: true }, + ); + }), + ]); + await expect(completion).resolves.toMatchObject({ + success: true, + exitCode: 0, + }); + expect(loginChild?.stdout?.destroyed).toBe(true); + expect(loginChild?.stderr?.destroyed).toBe(true); + } finally { + await writeFile(release, "released"); + if (grandchildPid !== undefined) { const deadline = Date.now() + 5_000; while (true) { try { - return await readFile(path, "utf8"); + process.kill(grandchildPid, 0); } catch (error) { if ( - !(error instanceof Error) || - !("code" in error) || - error.code !== "ENOENT" || - Date.now() >= deadline + error instanceof Error && + "code" in error && + error.code === "ESRCH" ) { - throw error; + break; } - await delay(25); + throw error; } - } - }; - - let grandchildPid: number | undefined; - try { - const readyMarker = await readMarker(ready); - expect(readyMarker).toMatch(/^\d+$/u); - grandchildPid = Number(readyMarker); - expect(Number.isSafeInteger(grandchildPid)).toBe(true); - expect(grandchildPid).toBeGreaterThan(0); - const timeout = AbortSignal.timeout(5_000); - const completion = Promise.race([ - handle.wait(), - new Promise((_, reject) => { - timeout.addEventListener( - "abort", - () => reject(new Error("The login fallback timed out.")), - { once: true }, + if (Date.now() >= deadline) { + throw new Error( + "The login grandchild did not exit after pipe cleanup.", ); - }), - ]); - await expect(completion).resolves.toMatchObject({ - success: true, - exitCode: 0, - }); - expect(loginChild?.stdout?.destroyed).toBe(true); - expect(loginChild?.stderr?.destroyed).toBe(true); - } finally { - await writeFile(release, "released"); - if (grandchildPid !== undefined) { - const deadline = Date.now() + 5_000; - while (true) { - try { - process.kill(grandchildPid, 0); - } catch (error) { - if ( - error instanceof Error && - "code" in error && - error.code === "ESRCH" - ) { - break; - } - throw error; - } - if (Date.now() >= deadline) { - throw new Error( - "The login grandchild did not exit after pipe cleanup.", - ); - } - await delay(25); } + await delay(25); } } - }, - ); + } + }); test("escalates cancellation when a login child ignores SIGTERM", async () => { const root = await mkdtemp(join(tmpdir(), "codex-security-auth-sigkill-")); From 27924f5f5a02662c01e9b56b54872449b360fd37 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 10:13:52 -0700 Subject: [PATCH 10/31] test: restrict POSIX process signal propagation to supported platforms --- sdk/typescript/tests-ts/cli-skills.test.ts | 123 +++++++++++---------- 1 file changed, 63 insertions(+), 60 deletions(-) diff --git a/sdk/typescript/tests-ts/cli-skills.test.ts b/sdk/typescript/tests-ts/cli-skills.test.ts index b30aadd6..ca9a7807 100644 --- a/sdk/typescript/tests-ts/cli-skills.test.ts +++ b/sdk/typescript/tests-ts/cli-skills.test.ts @@ -582,16 +582,18 @@ describe("CLI skill commands", () => { } }); - test("forces a skill child to settle when it ignores SIGTERM", async () => { - const directory = await mkdtemp( - join(tmpdir(), "codex-security-skill-signal-"), - ); - const ready = join(directory, "ready"); - const child = join(directory, "child.mjs"); - const wrapper = join(directory, "wrapper.mjs"); - await writeFile( - child, - ` + test.skipIf(process.platform === "win32")( + "forces a skill child to settle when it ignores SIGTERM", + async () => { + const directory = await mkdtemp( + join(tmpdir(), "codex-security-skill-signal-"), + ); + const ready = join(directory, "ready"); + const child = join(directory, "child.mjs"); + const wrapper = join(directory, "wrapper.mjs"); + await writeFile( + child, + ` import { spawn } from "node:child_process"; import { writeFileSync } from "node:fs"; const descendant = spawn( @@ -606,10 +608,10 @@ writeFileSync(${JSON.stringify(ready)}, JSON.stringify({ process.on("SIGTERM", () => {}); setInterval(() => {}, 1000); `, - ); - await writeFile( - wrapper, - ` + ); + await writeFile( + wrapper, + ` import { runCodexSkillCommand } from ${JSON.stringify(new URL("../src/cli.ts", import.meta.url).href)}; const status = await runCodexSkillCommand( [], @@ -618,55 +620,56 @@ const status = await runCodexSkillCommand( ); process.exit(status); `, - ); + ); - const invocation = spawn(process.execPath, [wrapper], { - stdio: "ignore", - windowsHide: true, - }); - let childPids: number[] = []; - try { - const deadline = Date.now() + 5_000; - while (true) { - try { - const marker = JSON.parse(await Bun.file(ready).text()) as { - child: number; - descendant: number; - }; - childPids = [marker.child, marker.descendant]; - break; - } catch (error) { - if (Date.now() >= deadline) throw error; - await delay(25); + const invocation = spawn(process.execPath, [wrapper], { + stdio: "ignore", + windowsHide: true, + }); + let childPids: number[] = []; + try { + const deadline = Date.now() + 5_000; + while (true) { + try { + const marker = JSON.parse(await Bun.file(ready).text()) as { + child: number; + descendant: number; + }; + childPids = [marker.child, marker.descendant]; + break; + } catch (error) { + if (Date.now() >= deadline) throw error; + await delay(25); + } } - } - invocation.kill("SIGTERM"); - const status = await Promise.race([ - new Promise((resolve, reject) => { - invocation.once("error", reject); - invocation.once("close", resolve); - }), - delay(5_000).then(() => { - throw new Error("CLI skill cancellation did not settle."); - }), - ]); - expect(status).toBe(143); - } finally { - invocation.kill("SIGKILL"); - for (const childPid of childPids) { - try { - process.kill(childPid, "SIGKILL"); - } catch (error) { - if ( - !(error instanceof Error) || - !("code" in error) || - error.code !== "ESRCH" - ) { - throw error; + invocation.kill("SIGTERM"); + const status = await Promise.race([ + new Promise((resolve, reject) => { + invocation.once("error", reject); + invocation.once("close", resolve); + }), + delay(5_000).then(() => { + throw new Error("CLI skill cancellation did not settle."); + }), + ]); + expect(status).toBe(143); + } finally { + invocation.kill("SIGKILL"); + for (const childPid of childPids) { + try { + process.kill(childPid, "SIGKILL"); + } catch (error) { + if ( + !(error instanceof Error) || + !("code" in error) || + error.code !== "ESRCH" + ) { + throw error; + } } } + await rm(directory, { recursive: true, force: true }); } - await rm(directory, { recursive: true, force: true }); - } - }); + }, + ); }); From 418fd91072623a03a19a31aaa7577eba3d207a70 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 10:27:51 -0700 Subject: [PATCH 11/31] fix: close runtime credential and subprocess review gaps --- sdk/typescript/src/api.ts | 23 +++++++-- sdk/typescript/src/auth.ts | 54 +++++++++++++++++++++- sdk/typescript/src/cli.ts | 23 +++++++-- sdk/typescript/src/errors.ts | 4 +- sdk/typescript/tests-ts/api.test.ts | 2 +- sdk/typescript/tests-ts/auth.test.ts | 28 +++++++++++ sdk/typescript/tests-ts/cli-skills.test.ts | 32 +++++++++++++ sdk/typescript/tests-ts/multiscan.test.ts | 8 +++- 8 files changed, 161 insertions(+), 13 deletions(-) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index 6b9be442..da9dee98 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -404,7 +404,7 @@ export class CodexSecurity { if (runtime.configPath !== undefined) { await writeCodexConfig( runtime.configPath, - scanPreflightCodexConfig(effectiveConfig, repo), + scanPreflightCodexConfig(effectiveConfig, protectedRoot), ); } const runtimeHome = await realpath(runtime.codexHome); @@ -570,6 +570,7 @@ export class CodexSecurity { costTracker = tracker; const recipe = scanRecipe( repo, + protectedRoot, normalized, mode, expectation.repositoryRevision, @@ -1608,6 +1609,7 @@ function targetInstruction(target: NormalizedTarget): string { function scanRecipe( repository: string, + activeProjectPath: string, target: NormalizedTarget, mode: ScanMode, repositoryRevision: string | null, @@ -1630,7 +1632,7 @@ function scanRecipe( mode, ...(repositoryRevision === null ? {} : { repositoryRevision }), pluginVersion, - config: scanPreflightCodexConfig(effectiveConfig, repository), + config: scanPreflightCodexConfig(effectiveConfig, activeProjectPath), ...(failOnSeverity === undefined ? {} : { failOnSeverity }), ...(knowledgeBasePaths === undefined ? {} : { knowledgeBasePaths }), ...(maxCostUsd === undefined ? {} : { maxCostUsd }), @@ -2021,9 +2023,24 @@ export function scanPreflightCodexConfig( if (isRecord(projects)) { const sanitized: JsonObject = {}; let accepted = 0; + const activeProjectRoot = + activeProjectPath === undefined + ? undefined + : Object.keys(projects) + .filter((path) => { + if (!safeString(path, 4096) || !isAbsolute(path)) return false; + const remaining = relative(path, activeProjectPath); + return ( + remaining === "" || + (remaining !== ".." && + !remaining.startsWith(`..${sep}`) && + !isAbsolute(remaining)) + ); + }) + .sort((left, right) => right.length - left.length)[0]; for (const [path, project] of prioritizedEntries( projects, - activeProjectPath, + activeProjectRoot ?? activeProjectPath, )) { if (!safeString(path, 4096) || !isAbsolute(path) || !isRecord(project)) { continue; diff --git a/sdk/typescript/src/auth.ts b/sdk/typescript/src/auth.ts index e12e5f16..2f12c9c7 100644 --- a/sdk/typescript/src/auth.ts +++ b/sdk/typescript/src/auth.ts @@ -10,6 +10,7 @@ const LOGIN_OUTPUT_LIMIT_MESSAGE = const COMMAND_OUTPUT_LIMIT_MESSAGE = "Codex authentication command output exceeded the 64 KiB safety limit."; const INSTRUCTION_TAIL_BYTES = 4 * 1024; +type TerminalEscapeState = "text" | "escape" | "csi" | "osc" | "osc-escape"; export interface LoginResult { success: boolean; @@ -41,6 +42,8 @@ export class CodexLoginHandle { #stderr = ""; #stdoutInstructionTail = ""; #stderrInstructionTail = ""; + #stdoutTerminalState: TerminalEscapeState = "text"; + #stderrTerminalState: TerminalEscapeState = "text"; #stdoutAuthUrl: string | null = null; #stderrAuthUrl: string | null = null; #stdoutUserCode: string | null = null; @@ -220,7 +223,13 @@ export class CodexLoginHandle { stream === "stdout" ? this.#stdoutInstructionTail : this.#stderrInstructionTail; - const candidate = `${tail}${chunk}`; + const visible = visibleTerminalChunk( + chunk, + stream === "stdout" + ? this.#stdoutTerminalState + : this.#stderrTerminalState, + ); + const candidate = `${tail}${visible.text}`; const url = preferredAuthUrl(candidate); const userCode = userCodeFromOutput(candidate); const nextTail = appendUtf8Tail( @@ -232,10 +241,12 @@ export class CodexLoginHandle { this.#stdoutAuthUrl ??= url; this.#stdoutUserCode ??= userCode; this.#stdoutInstructionTail = nextTail; + this.#stdoutTerminalState = visible.state; } else { this.#stderrAuthUrl ??= url; this.#stderrUserCode ??= userCode; this.#stderrInstructionTail = nextTail; + this.#stderrTerminalState = visible.state; } this.#notifyInstructions(); } @@ -494,3 +505,44 @@ function plainTerminalText(value: string): string { .replace(/\u001b\[[0-?]*[ -/]*[@-~]/g, "") .replace(/\r/g, ""); } + +function visibleTerminalChunk( + value: string, + initialState: TerminalEscapeState, +): { text: string; state: TerminalEscapeState } { + let state = initialState; + let text = ""; + for (const character of value) { + if (state === "text") { + if (character === "\u001b") { + state = "escape"; + } else if (character !== "\r") { + text += character; + } + continue; + } + if (state === "escape") { + if (character === "[") { + state = "csi"; + } else if (character === "]") { + state = "osc"; + } else { + text += `\u001b${character}`; + state = "text"; + } + continue; + } + if (state === "csi") { + if (character >= "@" && character <= "~") state = "text"; + continue; + } + if (state === "osc") { + if (character === "\u0007") state = "text"; + else if (character === "\u001b") state = "osc-escape"; + continue; + } + if (character === "\\" || character === "\u0007") state = "text"; + else if (character !== "\u001b") state = "osc"; + } + return { text, state }; +} diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index f778f745..8312e1f9 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -438,6 +438,7 @@ export async function runCodexSkillCommand( windowsHide: true, }); let requestedSignal: SignalName | null = null; + let skillOutputLimitExceeded = false; let forcedTermination: ReturnType | undefined; let forceStatusCompletion: (() => void) | null = null; let forceCaptureCompletion: (() => void) | null = null; @@ -473,7 +474,10 @@ export async function runCodexSkillCommand( output === undefined || invocation.stdout === null ? Promise.resolve(undefined) : Promise.race([ - readSkillCommandOutput(invocation.stdout), + readSkillCommandOutput(invocation.stdout, () => { + skillOutputLimitExceeded = true; + requestTermination("SIGTERM"); + }), new Promise((resolve) => { forceCaptureCompletion = () => resolve(undefined); }), @@ -507,6 +511,9 @@ export async function runCodexSkillCommand( }), captured, ]); + if (skillOutputLimitExceeded) { + throw new CodexSecurityError(SKILL_OUTPUT_LIMIT_MESSAGE); + } if (output === undefined || status === 130 || status === 143) return status; if (status !== 0) { await writeCliOutput( @@ -2231,11 +2238,17 @@ async function runSkill( export async function readSkillCommandOutput( stream: AsyncIterable, + onLimitExceeded?: () => void, ): Promise<{ message?: string; error?: string; malformed: boolean }> { let message: string | undefined; let error: string | undefined; let malformed = false; let exceeded = false; + const markExceeded = (): void => { + if (exceeded) return; + exceeded = true; + onLimitExceeded?.(); + }; const readLine = (bytes: Buffer): void => { const content = @@ -2265,7 +2278,7 @@ export async function readSkillCommandOutput( typeof item.text === "string" ) { if (Buffer.byteLength(item.text, "utf8") > MAX_SKILL_RESPONSE_BYTES) { - exceeded = true; + markExceeded(); } else { message = item.text; } @@ -2281,7 +2294,7 @@ export async function readSkillCommandOutput( if ( Buffer.byteLength(detail.message, "utf8") > MAX_SKILL_RESPONSE_BYTES ) { - exceeded = true; + markExceeded(); } else { error = detail.message; } @@ -2293,7 +2306,7 @@ export async function readSkillCommandOutput( if ( Buffer.byteLength(value["message"], "utf8") > MAX_SKILL_RESPONSE_BYTES ) { - exceeded = true; + markExceeded(); } else { error = value["message"]; } @@ -2314,7 +2327,7 @@ export async function readSkillCommandOutput( const segment = chunk.subarray(start, end); if (!discardingOversizedLine) { if (pendingBytes + segment.length > MAX_SKILL_EVENT_BYTES) { - exceeded = true; + markExceeded(); discardingOversizedLine = true; pendingBytes = 0; } else if (segment.length > 0) { diff --git a/sdk/typescript/src/errors.ts b/sdk/typescript/src/errors.ts index 3add0d1c..d2590cd8 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -5,7 +5,7 @@ export function redactedErrorMessage(error: unknown): string { const message = error instanceof Error ? error.message : String(error); return message .replaceAll( - /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|token|secret|credential|signature|sig|password|passwd)\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)[^\s"',;}&\\\]]+/giu, + /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)[^\s"',;}&\\\]]+/giu, "$1[redacted]", ) .replaceAll(/sk-(?:proj-)?[A-Za-z0-9_*=-]{8,}/gu, "[redacted]") @@ -17,7 +17,7 @@ export function redactedErrorMessage(error: unknown): string { ) .replaceAll(/((?:https?|ssh|git\+ssh):\/\/)[^\s/@]+@/giu, "$1[redacted]@") .replaceAll( - /((?:[?&]|%3F|%26)(?:(?!%3F|%26|%3D)(?:[A-Za-z0-9_.%-]|\[|\])){0,64}(?:api[_-]?key|access(?:[_-]|%5F|%2D)?key(?:(?:[_-]|%5F|%2D)?id)?|token|secret|credential|signature|sig|password|passwd)(?:\]|%5D)?(?:=|%3D))(?:(?!%26)[^&\s])+/giu, + /((?:[?&]|%3F|%26)(?:(?!%3F|%26|%3D)(?:[A-Za-z0-9_.%-]|\[|\])){0,64}(?:api[_-]?key|access(?:[_-]|%5F|%2D)?key(?:(?:[_-]|%5F|%2D)?id)?|token|secret|credential|signature|sig|password|passwd)(?:(?:[_-]|%5F|%2D)[A-Za-z0-9_.%-]{1,64})?(?:\]|%5D)?(?:=|%3D))(?:(?!%26)[^&\s])+/giu, "$1[redacted]", ); } diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 1f33a679..32937f30 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -519,7 +519,7 @@ describe("CodexSecurity orchestration", () => { profiles, projects, }, - activeProject, + join(activeProject, "packages", "service"), ); expect(prioritized["profile"]).toBe("selected"); diff --git a/sdk/typescript/tests-ts/auth.test.ts b/sdk/typescript/tests-ts/auth.test.ts index 9a424ded..fe719f27 100644 --- a/sdk/typescript/tests-ts/auth.test.ts +++ b/sdk/typescript/tests-ts/auth.test.ts @@ -223,6 +223,34 @@ setTimeout(() => { expect(succeeded).toBe(false); }); + test("ignores authentication instructions hidden in a split terminal escape", async () => { + const root = await mkdtemp(join(tmpdir(), "codex-security-auth-escape-")); + temporaryDirectories.push(root); + const script = join(root, "login.mjs"); + await writeFile( + script, + ` +process.stderr.write("\\u001b]0;" + "x".repeat(5 * 1024)); +setTimeout(() => process.stderr.write("https://hidden.example.test/device"), 10); +setTimeout(() => { + process.stderr.write("\\u0007\\nOpen https://auth.example.test/device\\nUser code: SAFE-1234\\n"); + setTimeout(() => process.exit(0), 10); +}, 20); +`, + ); + const handle = new CodexLoginHandle( + { command: process.execPath, prefixArgs: [script] }, + ["login", "--device-auth"], + process.env, + () => {}, + ); + + await handle.waitForInstructions({ deviceCode: true }); + expect(handle.verificationUrl).toBe("https://auth.example.test/device"); + expect(handle.userCode).toBe("SAFE-1234"); + await expect(handle.wait()).resolves.toMatchObject({ success: true }); + }); + test("forces an oversized login to settle when it ignores termination", async () => { const root = await mkdtemp( join(tmpdir(), "codex-security-auth-output-kill-"), diff --git a/sdk/typescript/tests-ts/cli-skills.test.ts b/sdk/typescript/tests-ts/cli-skills.test.ts index ca9a7807..4703e8be 100644 --- a/sdk/typescript/tests-ts/cli-skills.test.ts +++ b/sdk/typescript/tests-ts/cli-skills.test.ts @@ -509,6 +509,38 @@ describe("CLI skill commands", () => { expect(stderr.text()).toBe(""); }); + test("terminates an oversized skill child that keeps its output open", async () => { + const stdout = capture(); + const stderr = capture(); + const timeout = AbortSignal.timeout(2_500); + const invocation = runCodexSkillCommand( + [], + { command: "validate", stdout: stdout.stream, stderr: stderr.stream }, + { + command: process.execPath, + prefixArgs: [ + "-e", + 'process.on("SIGTERM",()=>{});process.stdout.write(JSON.stringify({type:"item.completed",item:{type:"agent_message",text:"x".repeat(256*1024+1)}})+"\\n");setInterval(()=>{},1000)', + ], + }, + ); + + await expect( + Promise.race([ + invocation, + new Promise((_, reject) => { + timeout.addEventListener( + "abort", + () => reject(new Error("Oversized skill process did not settle.")), + { once: true }, + ); + }), + ]), + ).rejects.toThrow("Codex skill output exceeded the 1 MiB event"); + expect(stdout.text()).toBe(""); + expect(stderr.text()).toBe(""); + }); + test("summarizes skill failures without echoing credentials or private paths", () => { const cases = [ ["401 sk-proj-SYNTHETIC_SECRET", "Authentication failed"], diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index 5ed08ef5..61033a3b 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -449,6 +449,9 @@ describe("multiscan", () => { const queryUrl = "https://proxy.test/v1/responses?api_key=SYNTHETIC_MULTISCAN_QUERY_123&safe=1"; const shortAuthorization = "Bearer abc123"; + const suffixedSecret = "SYNTHETIC_SUFFIXED_CLIENT_SECRET_123"; + const suffixedToken = "SYNTHETIC_SUFFIXED_ACCESS_TOKEN_123"; + const suffixedQuery = "SYNTHETIC_SUFFIXED_QUERY_SECRET_123"; await writeFile( paths.input, `id,repository,revision\nretry,${source.path},${source.revision}\n`, @@ -462,7 +465,7 @@ describe("multiscan", () => { attempts += 1; if (attempts === 1) { throw new Error( - `temporary failure ${secret} ${shortAuthorization} sending request for url (${proxyUrl}) and ${queryUrl}`, + `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, ); } return await completedScan(scanOptions.outputDir!); @@ -480,6 +483,9 @@ describe("multiscan", () => { expect(ledger).not.toContain(secret); expect(ledger).not.toContain("SYNTHETIC_MULTISCAN_PASSWORD"); expect(ledger).not.toContain("SYNTHETIC_MULTISCAN_QUERY_123"); + expect(ledger).not.toContain(suffixedSecret); + expect(ledger).not.toContain(suffixedToken); + expect(ledger).not.toContain(suffixedQuery); expect(ledger).not.toContain(shortAuthorization); expect(ledger).toContain("https://[redacted]@proxy.test/v1/responses"); }); From f77b33d8928d6d29adbef808757a05576f858bb2 Mon Sep 17 00:00:00 2001 From: mldangelo Date: Thu, 30 Jul 2026 10:34:21 -0700 Subject: [PATCH 12/31] fix: report local scan failures without connectivity advice classifyConnectionFailure matches bare words anywhere in a message, and the unauthorized, forbidden and rate-limited branches of scanFailureMessage replace the message entirely with fixed advice. A local failure whose text merely contained one of those words was therefore reported as a credential problem, and its own text reached neither stderr nor the JSON error field. The reported trigger is "permission denied", the standard strerror for EACCES: a read-only TMPDIR surfaced as "The stored ChatGPT credentials cannot access the configured model." Skip classification for failures that cannot originate from the model transport -- InvalidTargetError, OutputDirectoryError, ConfigurationError, PluginPythonUnavailableError, and filesystem syscall errno codes -- so they keep their own message. Network errno codes are deliberately excluded from that set because they are genuine transport failures. Advice still replaces the underlying text for real transport failures. Upstream authentication and authorization errors can name the organization or project, and cli-authentication.test.ts pins that they must not reach stderr. Fixes #36 --- sdk/typescript/src/cli.ts | 55 ++++++++++++++++ sdk/typescript/tests-ts/cli.test.ts | 97 +++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+) diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index 8312e1f9..4a27f8ab 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -55,7 +55,11 @@ import { import { formatUsd } from "./cost.js"; import { CodexSecurityError, + ConfigurationError, + InvalidTargetError, + OutputDirectoryError, OutputInsideProtectedRootError, + PluginPythonUnavailableError, redactedErrorMessage, ScanInterruptedError, } from "./errors.js"; @@ -2783,10 +2787,61 @@ async function runScan( return { exitCode: blockingCount > 0 ? 1 : 0, data: result.toJSON() }; } +// Filesystem and OS syscall failures cannot originate from the model transport, +// so they must never be rewritten as connectivity or credential advice. Network +// errno codes are deliberately absent: they are genuine transport failures. +const LOCAL_SYSCALL_CODES = new Set([ + "EACCES", + "EBUSY", + "EEXIST", + "EFBIG", + "EIO", + "EISDIR", + "ELOOP", + "EMFILE", + "ENAMETOOLONG", + "ENFILE", + "ENOENT", + "ENOMEM", + "ENOSPC", + "ENOTDIR", + "ENOTEMPTY", + "EPERM", + "EROFS", + "EXDEV", +]); + +function isLocalScanFailure(error: unknown): boolean { + if ( + error instanceof InvalidTargetError || + error instanceof OutputDirectoryError || + error instanceof ConfigurationError || + error instanceof PluginPythonUnavailableError + ) { + return true; + } + return ( + typeof error === "object" && + error !== null && + "code" in error && + typeof (error as { code: unknown }).code === "string" && + LOCAL_SYSCALL_CODES.has((error as { code: string }).code) + ); +} + function scanFailureMessage( error: unknown, authentication: ScanAuthentication | null, ): string { + // A local failure keeps its own message. Classification matches bare words + // such as "permission denied" anywhere in the text, so an EACCES from a + // read-only TMPDIR would otherwise be reported as a credential problem. + // + // The advice branches below still replace the underlying text rather than + // appending it. That is deliberate: upstream authentication and authorization + // errors can name the organization or project, which must not reach stderr or + // the JSON error field. + if (isLocalScanFailure(error)) return cliErrorMessage(error); switch (classifyConnectionFailure(error)) { case "unauthorized": return authentication?.method === "api_key" diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index 84b7d7c2..6fc37032 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -25,7 +25,10 @@ import { BUNDLED_PLUGIN_VERSION, CodexSecurityError, DiffTarget, + InvalidTargetError, + OutputDirectoryError, OutputInsideProtectedRootError, + PluginPythonUnavailableError, ScanCostLimitExceededError, ScanInterruptedError, VERSION, @@ -2035,6 +2038,100 @@ describe("CLI", () => { } }); + test("reports local input and filesystem failures without connectivity advice", async () => { + // https://github.com/openai/codex-security/issues/36 -- classification + // matches bare words such as "permission denied" anywhere in the message, + // so local failures were reported as credential or connectivity problems + // and their own text was discarded. + const failures: Array<[string, unknown]> = [ + [ + "EACCES from a read-only TMPDIR", + Object.assign( + new Error( + "EACCES: permission denied, mkdtemp '/tmp/openai-codex-security-home-XXXXXX'", + ), + { code: "EACCES" }, + ), + ], + [ + "EPERM writing the scan directory", + Object.assign( + new Error("EPERM: operation not permitted, mkdir '/out/scan'"), + { code: "EPERM" }, + ), + ], + [ + "output directory rejected", + new OutputDirectoryError( + "Scan output directory must not be accessible to other users (chmod 700): /out", + ), + ], + [ + "path target naming a 403 directory", + new InvalidTargetError("Path target does not exist: src/403/client.ts"), + ], + [ + "git ref naming a forbidden branch", + new InvalidTargetError("unknown Git ref: origin/forbidden-paths"), + ], + [ + "python interpreter unavailable", + new PluginPythonUnavailableError( + "The configured plugin Python interpreter is unavailable or unusable: /usr/bin/python3", + ), + ], + ]; + + for (const [, failure] of failures) { + const stdout = capture(); + const stderr = capture(); + const deps = dependencies(); + deps.createSecurity = () => ({ + run: async () => { + throw failure; + }, + preflight: async () => fakePreflight(), + close: async () => {}, + }); + + expect( + await main(["scan", "."], stdout.stream, stderr.stream, deps), + ).toBe(2); + expect(stderr.text()).toContain((failure as Error).message); + expect(stderr.text()).not.toContain("cannot access the configured model"); + expect(stderr.text()).not.toContain("Authentication failed"); + expect(stderr.text()).not.toContain("reached its rate limit"); + } + }); + + test("keeps model authorization advice for genuine transport failures", async () => { + // The bypass must not swallow real 401/403 handling, and the advice must + // still replace upstream text that can name the organization or project. + for (const [detail, expected] of [ + ["401 invalid API key for org-private", "Authentication failed"], + [ + "403 model access denied for org-private", + "cannot access the configured model", + ], + ] as const) { + const stderr = capture(); + const deps = dependencies(); + deps.createSecurity = () => ({ + run: async () => { + throw new CodexSecurityError(detail); + }, + preflight: async () => fakePreflight(), + close: async () => {}, + }); + + expect( + await main(["scan", "."], capture().stream, stderr.stream, deps), + ).toBe(2); + expect(stderr.text()).toContain(expected); + expect(stderr.text()).not.toContain("org-private"); + } + }); + test("redacts credentials in underlying network errors", async () => { const stdout = capture(); const stderr = capture(); From 9794115933f373236dabe9f1f8cdbcfcdaa63128 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 11:08:24 -0700 Subject: [PATCH 13/31] fix: retain credential redaction for local scan failures --- sdk/typescript/src/cli.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index 4a27f8ab..cc4ddceb 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -2841,7 +2841,7 @@ function scanFailureMessage( // appending it. That is deliberate: upstream authentication and authorization // errors can name the organization or project, which must not reach stderr or // the JSON error field. - if (isLocalScanFailure(error)) return cliErrorMessage(error); + if (isLocalScanFailure(error)) return redactedErrorMessage(error); switch (classifyConnectionFailure(error)) { case "unauthorized": return authentication?.method === "api_key" From befeb3762e6f27366c31131b0a1772323e5393c0 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 11:22:05 -0700 Subject: [PATCH 14/31] fix: harden credential redaction and authentication boundaries --- sdk/typescript/src/api.ts | 2 +- sdk/typescript/src/auth.ts | 26 ++++++++++++++++++++++---- sdk/typescript/src/cost.ts | 13 ++++++++++++- sdk/typescript/src/errors.ts | 8 ++++++-- sdk/typescript/tests-ts/auth.test.ts | 28 ++++++++++++++++++++++++++++ sdk/typescript/tests-ts/cli.test.ts | 9 +++++++++ sdk/typescript/tests-ts/cost.test.ts | 23 +++++++++++++++++++++++ 7 files changed, 101 insertions(+), 8 deletions(-) diff --git a/sdk/typescript/src/api.ts b/sdk/typescript/src/api.ts index da9dee98..32269c06 100644 --- a/sdk/typescript/src/api.ts +++ b/sdk/typescript/src/api.ts @@ -404,7 +404,7 @@ export class CodexSecurity { if (runtime.configPath !== undefined) { await writeCodexConfig( runtime.configPath, - scanPreflightCodexConfig(effectiveConfig, protectedRoot), + scanPreflightCodexConfig(effectiveConfig, repo), ); } const runtimeHome = await realpath(runtime.codexHome); diff --git a/sdk/typescript/src/auth.ts b/sdk/typescript/src/auth.ts index 2f12c9c7..e432b97e 100644 --- a/sdk/typescript/src/auth.ts +++ b/sdk/typescript/src/auth.ts @@ -90,6 +90,7 @@ export class CodexLoginHandle { this.#clearForcedTermination(); this.#forceCompletion = null; this.#destroyPipes(); + this.#flushInstructionTails(); const result = this.#outputLimitExceeded ? { success: false, @@ -230,11 +231,16 @@ export class CodexLoginHandle { : this.#stderrTerminalState, ); const candidate = `${tail}${visible.text}`; - const url = preferredAuthUrl(candidate); - const userCode = userCodeFromOutput(candidate); + const lastDelimiter = Math.max( + candidate.lastIndexOf("\n"), + candidate.lastIndexOf("\r"), + ); + const completed = candidate.slice(0, lastDelimiter + 1); + const url = preferredAuthUrl(completed); + const userCode = userCodeFromOutput(completed); const nextTail = appendUtf8Tail( "", - candidate, + candidate.slice(lastDelimiter + 1), INSTRUCTION_TAIL_BYTES, ).value; if (stream === "stdout") { @@ -251,6 +257,13 @@ export class CodexLoginHandle { this.#notifyInstructions(); } + #flushInstructionTails(): void { + this.#stdoutAuthUrl ??= preferredAuthUrl(this.#stdoutInstructionTail); + this.#stdoutUserCode ??= userCodeFromOutput(this.#stdoutInstructionTail); + this.#stderrAuthUrl ??= preferredAuthUrl(this.#stderrInstructionTail); + this.#stderrUserCode ??= userCodeFromOutput(this.#stderrInstructionTail); + } + #notifyInstructions(): void { if (!this.#urlReadySettled && this.authUrl !== null) { this.#urlReadySettled = true; @@ -372,7 +385,12 @@ export async function runCodex( let processError: Error | null = null; let forcedTermination: ReturnType | undefined; const terminate = (): void => { - if (child.exitCode !== null || child.signalCode !== null) return; + if (child.exitCode !== null || child.signalCode !== null) { + child.stdin.destroy(); + child.stdout.destroy(); + child.stderr.destroy(); + return; + } child.kill("SIGTERM"); if (forcedTermination !== undefined) return; forcedTermination = setTimeout(() => { diff --git a/sdk/typescript/src/cost.ts b/sdk/typescript/src/cost.ts index 1481e9dc..ccb9c7f1 100644 --- a/sdk/typescript/src/cost.ts +++ b/sdk/typescript/src/cost.ts @@ -127,7 +127,18 @@ export class ScanCostTracker { }; this.#sessions.set(path, session); } - await readSessionUsage(path, session); + try { + await readSessionUsage(path, session); + } catch (error) { + if ( + session.threadId !== null && + session.threadId !== this.#threadId && + session.parentThreadId === null + ) { + continue; + } + throw error; + } } const included = new Set([this.#threadId]); diff --git a/sdk/typescript/src/errors.ts b/sdk/typescript/src/errors.ts index d2590cd8..a6bd9ee3 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -5,7 +5,11 @@ export function redactedErrorMessage(error: unknown): string { const message = error instanceof Error ? error.message : String(error); return message .replaceAll( - /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)[^\s"',;}&\\\]]+/giu, + /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b\s*[:=]\s*)(["'])(?:\\.|(?!\2)[^\\])*\2/giu, + "$1$2[redacted]$2", + ) + .replaceAll( + /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)(?!\[redacted\])[^\s"',;}&\\\]]+/giu, "$1[redacted]", ) .replaceAll(/sk-(?:proj-)?[A-Za-z0-9_*=-]{8,}/gu, "[redacted]") @@ -17,7 +21,7 @@ export function redactedErrorMessage(error: unknown): string { ) .replaceAll(/((?:https?|ssh|git\+ssh):\/\/)[^\s/@]+@/giu, "$1[redacted]@") .replaceAll( - /((?:[?&]|%3F|%26)(?:(?!%3F|%26|%3D)(?:[A-Za-z0-9_.%-]|\[|\])){0,64}(?:api[_-]?key|access(?:[_-]|%5F|%2D)?key(?:(?:[_-]|%5F|%2D)?id)?|token|secret|credential|signature|sig|password|passwd)(?:(?:[_-]|%5F|%2D)[A-Za-z0-9_.%-]{1,64})?(?:\]|%5D)?(?:=|%3D))(?:(?!%26)[^&\s])+/giu, + /((?:[?&]|%3F|%26)(?:(?!%3F|%26|%3D)(?:[A-Za-z0-9_.%-]|\[|\])){0,64}(?:api[_-]?key|access(?:[_-]|%5F|%2D)?key(?:(?:[_-]|%5F|%2D)?id)?|private(?:[_-]|%5F|%2D)?key|token|secret|credential|signature|sig|password|passwd)(?:(?:[_-]|%5F|%2D)[A-Za-z0-9_.%-]{1,64})?(?:\]|%5D)?(?:=|%3D))(?:(?!%26)[^&\s])+/giu, "$1[redacted]", ); } diff --git a/sdk/typescript/tests-ts/auth.test.ts b/sdk/typescript/tests-ts/auth.test.ts index fe719f27..fdb812b0 100644 --- a/sdk/typescript/tests-ts/auth.test.ts +++ b/sdk/typescript/tests-ts/auth.test.ts @@ -251,6 +251,34 @@ setTimeout(() => { await expect(handle.wait()).resolves.toMatchObject({ success: true }); }); + test("waits for complete login instructions split across output chunks", async () => { + const root = await mkdtemp(join(tmpdir(), "codex-security-auth-fragment-")); + temporaryDirectories.push(root); + const script = join(root, "login.mjs"); + await writeFile( + script, + ` +process.stderr.write("Open https://auth.example"); +setTimeout(() => process.stderr.write(".test/device\\nUser code: ABCD"), 10); +setTimeout(() => { + process.stderr.write("-EFGH\\n"); + setTimeout(() => process.exit(0), 10); +}, 20); +`, + ); + const handle = new CodexLoginHandle( + { command: process.execPath, prefixArgs: [script] }, + ["login", "--device-auth"], + process.env, + () => {}, + ); + + await handle.waitForInstructions({ deviceCode: true }); + expect(handle.verificationUrl).toBe("https://auth.example.test/device"); + expect(handle.userCode).toBe("ABCD-EFGH"); + await expect(handle.wait()).resolves.toMatchObject({ success: true }); + }); + test("forces an oversized login to settle when it ignores termination", async () => { const root = await mkdtemp( join(tmpdir(), "codex-security-auth-output-kill-"), diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index 6fc37032..b985853b 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -21,6 +21,7 @@ import type { JsonObject, ScanPreflight, } from "../src/index.js"; +import { redactedErrorMessage } from "../src/errors.js"; import { BUNDLED_PLUGIN_VERSION, CodexSecurityError, @@ -2157,6 +2158,14 @@ describe("CLI", () => { expect(stderr.text()).not.toContain("model service could not be reached"); }); + test("redacts quoted multiword credentials and private-key assignments", () => { + expect( + redactedErrorMessage( + 'password="correct horse battery staple" private_key=SYNTHETIC_PRIVATE_KEY_123', + ), + ).toBe('password="[redacted]" private_key=[redacted]'); + }); + test("reports database connection failures without claiming the model network failed", async () => { const stdout = capture(); const stderr = capture(); diff --git a/sdk/typescript/tests-ts/cost.test.ts b/sdk/typescript/tests-ts/cost.test.ts index 9647d950..602af45d 100644 --- a/sdk/typescript/tests-ts/cost.test.ts +++ b/sdk/typescript/tests-ts/cost.test.ts @@ -282,6 +282,29 @@ describe("live scan cost tracking", () => { }); }); + test("ignores oversized events from unrelated prior credential sessions", async () => { + const home = await codexHome(); + const unrelated = await writeSession(home, "unrelated-thread", { + input_tokens: 99, + output_tokens: 1, + }); + await appendFile(unrelated, "x".repeat(1 * 1_024 * 1_024 + 1)); + await writeSession(home, "scan-thread", { + input_tokens: 100, + output_tokens: 10, + }); + const tracker = new ScanCostTracker({ + codexHome: home, + model: "gpt-5.6-terra", + }); + tracker.start("scan-thread"); + + expect((await tracker.stop()).cost).toMatchObject({ + inputTokens: 100, + outputTokens: 10, + }); + }); + test("reports a changed running cost only once", async () => { const home = await codexHome(); await writeSession(home, "scan-thread", { From 70bda028c4b007a29a71f7389a6753ffc628b768 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 11:32:01 -0700 Subject: [PATCH 15/31] fix: exclude unrelated delegated cost sessions safely --- sdk/typescript/src/cost.ts | 14 ++++++-------- sdk/typescript/tests-ts/cost.test.ts | 29 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/sdk/typescript/src/cost.ts b/sdk/typescript/src/cost.ts index ccb9c7f1..321b9751 100644 --- a/sdk/typescript/src/cost.ts +++ b/sdk/typescript/src/cost.ts @@ -111,6 +111,7 @@ export class ScanCostTracker { async #readSessions(): Promise { if (this.#threadId === null) return; + const unreadable: Array<{ session: SessionUsage; error: unknown }> = []; for await (const path of sessionFiles( join(this.#options.codexHome, "sessions"), )) { @@ -130,14 +131,8 @@ export class ScanCostTracker { try { await readSessionUsage(path, session); } catch (error) { - if ( - session.threadId !== null && - session.threadId !== this.#threadId && - session.parentThreadId === null - ) { - continue; - } - throw error; + if (session.threadId === null) throw error; + unreadable.push({ session, error }); } } @@ -157,6 +152,9 @@ export class ScanCostTracker { } } } + for (const { session, error } of unreadable) { + if (included.has(session.threadId!)) throw error; + } let usage: ScanTokenUsage | null = null; for (const session of this.#sessions.values()) { diff --git a/sdk/typescript/tests-ts/cost.test.ts b/sdk/typescript/tests-ts/cost.test.ts index 602af45d..16d6fc5b 100644 --- a/sdk/typescript/tests-ts/cost.test.ts +++ b/sdk/typescript/tests-ts/cost.test.ts @@ -305,6 +305,35 @@ describe("live scan cost tracking", () => { }); }); + test("ignores oversized delegated sessions belonging to an unrelated scan", async () => { + const home = await codexHome(); + await writeSession(home, "unrelated-parent", { + input_tokens: 1, + output_tokens: 1, + }); + const unrelated = await writeSession( + home, + "unrelated-child", + { input_tokens: 1, output_tokens: 1 }, + "unrelated-parent", + ); + await appendFile(unrelated, "x".repeat(1 * 1_024 * 1_024 + 1)); + await writeSession(home, "scan-thread", { + input_tokens: 100, + output_tokens: 10, + }); + const tracker = new ScanCostTracker({ + codexHome: home, + model: "gpt-5.6-terra", + }); + tracker.start("scan-thread"); + + expect((await tracker.stop()).cost).toMatchObject({ + inputTokens: 100, + outputTokens: 10, + }); + }); + test("reports a changed running cost only once", async () => { const home = await codexHome(); await writeSession(home, "scan-thread", { From 495d6b3c098147a8b573070116708fd1c6879eab Mon Sep 17 00:00:00 2001 From: batmanknows <122247678+batmnnn@users.noreply.github.com> Date: Thu, 30 Jul 2026 23:48:46 +0530 Subject: [PATCH 16/31] fix: keep credential homes private under trusted ancestry Reject non-sticky or untrusted parents for durable auth state, re-check on every credential-home use path, harden auth.json, and pin identity for the lock session so shared-volume rename races cannot steal auth. Co-authored-by: Cursor --- sdk/typescript/src/cli.ts | 8 +- sdk/typescript/src/runtime.ts | 183 +++++++++++++++++- .../tests-ts/cli-authentication.test.ts | 7 +- sdk/typescript/tests-ts/runtime.test.ts | 156 ++++++++++++++- 4 files changed, 348 insertions(+), 6 deletions(-) diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index cc4ddceb..aa144d66 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -1541,7 +1541,9 @@ export async function main( ? await dependencies.prepareAuthenticationHome( dependencies.environment, ) - : codexSecurityCredentialHome(dependencies.environment); + : await prepareCodexSecurityCredentialHome( + dependencies.environment, + ); const authenticationEnvironment = { ...dependencies.environment, CODEX_HOME: credentialHome, @@ -1617,7 +1619,9 @@ export async function main( ? await dependencies.prepareAuthenticationHome( dependencies.environment, ) - : codexSecurityCredentialHome(dependencies.environment); + : await prepareCodexSecurityCredentialHome( + dependencies.environment, + ); const authenticationEnvironment = { ...dependencies.environment, CODEX_HOME: credentialHome, diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index a069789e..da39f1fa 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -154,7 +154,10 @@ export async function prepareCodexSecurityCredentialHome( const canonical = await realpath(path); requireModelSafeOutputDir(canonical); validateLocation?.(canonical); - await requirePrivateCredentialHome(metadata, canonical); + await requireSecureCredentialHome(canonical, { + applyWindowsAcl: true, + metadata, + }); return canonical; } catch (error) { if (error instanceof OutputDirectoryError) throw error; @@ -165,6 +168,76 @@ export async function prepareCodexSecurityCredentialHome( } } +/** + * Re-verify the credential home before every use. + * + * Durable cross-process `(st_dev, st_ino)` pins are deferred: there is no + * workbench row for credential homes, and path+mode+trusted-ancestry checks on + * each use already close the cross-user rename/replace class. Lock acquisition + * still pins identity for the duration of a single lock session. + */ +export async function requireSecureCredentialHome( + path: string, + options: { + platform?: NodeJS.Platform; + secureWindowsHome?: (path: string) => Promise; + applyWindowsAcl?: boolean; + metadata?: Stats; + expectedDevice?: number; + expectedInode?: number; + } = {}, +): Promise { + const platform = options.platform ?? process.platform; + let metadata = options.metadata; + if (metadata === undefined) { + try { + metadata = await lstat(path); + } catch (error) { + throw new OutputDirectoryError( + `Unable to inspect the Codex Security credential home: ${path}`, + { cause: error }, + ); + } + } + if (!metadata.isDirectory() || metadata.isSymbolicLink()) { + throw new OutputDirectoryError( + `Codex Security credential home is not a directory: ${path}`, + ); + } + const canonical = await realpath(path); + requireModelSafeOutputDir(canonical); + if (canonical !== path) { + metadata = await lstat(canonical); + if (!metadata.isDirectory() || metadata.isSymbolicLink()) { + throw new OutputDirectoryError( + `Codex Security credential home is not a directory: ${path}`, + ); + } + } + if ( + options.expectedDevice !== undefined && + options.expectedInode !== undefined && + (metadata.dev !== options.expectedDevice || + metadata.ino !== options.expectedInode) + ) { + throw new OutputDirectoryError( + `Codex Security credential home was replaced: ${canonical}`, + ); + } + if (platform === "win32") { + if (options.applyWindowsAcl) { + await requirePrivateCredentialHome(metadata, canonical, { + platform, + secureWindowsHome: options.secureWindowsHome, + }); + } + return metadata; + } + await requirePrivateCredentialHome(metadata, canonical, { platform }); + await requireSecureOutputAncestry(canonical); + return metadata; +} + export async function requirePrivateCredentialHome( metadata: Pick, path: string, @@ -233,12 +306,19 @@ export async function acquireCodexSecurityCredentialHomeLock( codexHome: string, signal?: AbortSignal, ): Promise<() => Promise> { + const homeMetadata = await requireSecureCredentialHome(codexHome); + const expectedDevice = homeMetadata.dev; + const expectedInode = homeMetadata.ino; const lock = join(codexHome, CREDENTIAL_LOCK_NAME); const ownerPath = join(lock, "owner.json"); const token = randomUUID(); while (true) { throwIfSignalAborted(signal); + await requireSecureCredentialHome(codexHome, { + expectedDevice, + expectedInode, + }); try { await mkdir(lock, { mode: 0o700 }); } catch (error) { @@ -262,6 +342,10 @@ export async function acquireCodexSecurityCredentialHomeLock( let released = false; return async () => { if (released) return; + await requireSecureCredentialHome(codexHome, { + expectedDevice, + expectedInode, + }); const owner = JSON.parse(await readFile(ownerPath, "utf8")) as { token?: unknown; }; @@ -335,6 +419,7 @@ export async function setCodexSecurityCredentialLogout( codexHome: string, loggedOut: boolean, ): Promise { + await requireSecureCredentialHome(codexHome); const marker = join(codexHome, CREDENTIAL_LOGOUT_MARKER); if (!loggedOut) { await rm(marker, { force: true }); @@ -363,6 +448,7 @@ export async function setCodexSecurityCredentialLogout( export async function codexSecurityCredentialAllowsAmbientImport( codexHome: string, ): Promise { + await requireSecureCredentialHome(codexHome); try { const marker = await lstat(join(codexHome, CREDENTIAL_LOGOUT_MARKER)); if (!marker.isFile() || marker.isSymbolicLink()) { @@ -380,6 +466,7 @@ export async function codexSecurityCredentialAllowsAmbientImport( export async function codexSecurityHasStoredFileCredentials( codexHome: string, ): Promise { + await requireSecureCredentialHome(codexHome); const path = join(codexHome, "auth.json"); let metadata: Stats; try { @@ -393,9 +480,28 @@ export async function codexSecurityHasStoredFileCredentials( `Codex Security stored authentication is not a regular file: ${path}`, ); } + requirePrivateCredentialFile(metadata, path); return true; } +export function requirePrivateCredentialFile( + metadata: Pick, + path: string, + effectiveUid = process.geteuid?.(), +): void { + if (process.platform === "win32") return; + if ((metadata.mode & 0o077) !== 0) { + throw new OutputDirectoryError( + `Codex Security stored authentication must not be accessible to other users: ${path}`, + ); + } + if (effectiveUid !== undefined && metadata.uid !== effectiveUid) { + throw new OutputDirectoryError( + `Codex Security stored authentication must be owned by the current user: ${path}`, + ); + } +} + export async function preserveCodexSecurityPluginRegistration( codexHome: string, config: JsonObject, @@ -726,6 +832,78 @@ export function requirePrivateOutputDirectory( } } +/** Reject shared parents whose owner can rename or replace a private directory. */ +export async function requireSecureOutputAncestry( + path: string, + effectiveUid = process.geteuid?.(), +): Promise { + if (process.platform === "win32") return; + let current = dirname(resolve(path)); + while (true) { + try { + current = await realpath(current); + break; + } catch (error) { + if (nodeErrorCode(error) !== "ENOENT") { + throw new OutputDirectoryError( + `Unable to inspect parent directory: ${current}`, + { cause: error }, + ); + } + const parent = dirname(current); + if (parent === current) { + throw new OutputDirectoryError( + `Unable to inspect parent directory: ${current}`, + { cause: error }, + ); + } + current = parent; + } + } + while (true) { + let metadata: Stats; + try { + metadata = await lstat(current); + } catch (error) { + throw new OutputDirectoryError( + `Unable to inspect parent directory: ${current}`, + { cause: error }, + ); + } + if (!metadata.isDirectory() || metadata.isSymbolicLink()) { + throw new OutputDirectoryError( + `Parent must be a non-symlink directory: ${current}`, + ); + } + requireTrustedOutputAncestor(metadata, current, effectiveUid); + const parent = dirname(current); + if (parent === current) return; + current = parent; + } +} + +export function requireTrustedOutputAncestor( + metadata: Pick, + path: string, + effectiveUid = process.geteuid?.(), +): void { + if ( + effectiveUid !== undefined && + metadata.uid !== 0 && + metadata.uid !== effectiveUid + ) { + throw new OutputDirectoryError( + `Parent must have a trusted owner: ${path}`, + ); + } + if ((metadata.mode & 0o022) === 0) return; + if ((metadata.mode & 0o1000) === 0) { + throw new OutputDirectoryError( + `Parent must not be group- or world-writable without the sticky bit: ${path}`, + ); + } +} + async function removeEmptyDirectories( path: string, root: string, @@ -779,6 +957,9 @@ export async function importAmbientAuth( return false; } await mkdir(isolatedHome, { recursive: true, mode: 0o700 }); + if (process.platform !== "win32" && (process.umask() & 0o700) !== 0) { + await chmod(isolatedHome, 0o700); + } if (await codexSecurityHasStoredFileCredentials(isolatedHome)) return true; const destination = join(isolatedHome, "auth.json"); const temporary = join(isolatedHome, `.auth-${randomUUID()}.tmp`); diff --git a/sdk/typescript/tests-ts/cli-authentication.test.ts b/sdk/typescript/tests-ts/cli-authentication.test.ts index 40f12a27..e51f4363 100644 --- a/sdk/typescript/tests-ts/cli-authentication.test.ts +++ b/sdk/typescript/tests-ts/cli-authentication.test.ts @@ -55,7 +55,12 @@ describe("CLI authentication", () => { test("uses the same stable credential home for login, status, and logout", async () => { const stateDirectory = join(tmpdir(), "codex-security-managed-auth-state"); - const expectedHome = join(stateDirectory, "codex-home"); + await mkdir(stateDirectory, { recursive: true, mode: 0o700 }); + const expectedHome = await realpath( + await prepareCodexSecurityCredentialHome({ + CODEX_SECURITY_STATE_DIR: stateDirectory, + }), + ); for (const argv of [["login"], ["login", "status"], ["logout"]] as const) { const stdout = capture(); diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index 3734d378..17a6b5f0 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -9,6 +9,7 @@ import { readFile, realpath, readdir, + rename, rm, stat, symlink, @@ -49,6 +50,7 @@ import { bundledPluginCandidates, codexSecurityCredentialAllowsAmbientImport, codexSecurityCredentialHome, + codexSecurityHasStoredFileCredentials, codexSecurityStateDirectory, codexPlatformPackage, isPythonPathCandidate, @@ -56,7 +58,11 @@ import { prepareCodexSecurityCredentialHome, preparePersistentScanRoot, requirePrivateCredentialHome, + requirePrivateCredentialFile, requirePrivateOutputDirectory, + requireSecureCredentialHome, + requireSecureOutputAncestry, + requireTrustedOutputAncestor, runWorkbench, setCodexSecurityCredentialLogout, } from "../src/runtime.js"; @@ -829,9 +835,15 @@ describe("plugin runtime preparation", () => { const ambient = join(root, "ambient"); const isolated = join(root, "isolated"); await mkdir(ambient); - await mkdir(isolated); + await mkdir(isolated, { mode: 0o700 }); + if (process.platform !== "win32") await chmod(isolated, 0o700); await writeFile(join(ambient, "auth.json"), '{"token":"ambient"}\n'); - await writeFile(join(isolated, "auth.json"), '{"token":"explicit"}\n'); + await writeFile(join(isolated, "auth.json"), '{"token":"explicit"}\n', { + mode: 0o600, + }); + if (process.platform !== "win32") { + await chmod(join(isolated, "auth.json"), 0o600); + } expect(await importAmbientAuth(ambient, isolated)).toBe(true); expect(await readFile(join(isolated, "auth.json"), "utf8")).toBe( @@ -1312,6 +1324,146 @@ describe("runtime directories and plugin Python boundary", () => { ).rejects.toThrow("credential home is not a directory"); }); + testPosix( + "rejects credential homes under a non-sticky shared parent directory", + async () => { + const root = await temporaryDirectory(); + const shared = join(root, "shared"); + await mkdir(shared, { mode: 0o777 }); + await chmod(shared, 0o777); + expect((await lstat(shared)).mode & 0o1000).toBe(0); + const environment = { CODEX_SECURITY_STATE_DIR: join(shared, "state") }; + + await expect( + prepareCodexSecurityCredentialHome(environment), + ).rejects.toThrow("sticky bit"); + await expect(requireSecureOutputAncestry(join(shared, "state"))).rejects.toThrow( + "sticky bit", + ); + }, + ); + + testPosix( + "accepts credential homes under a sticky shared parent directory", + async () => { + const root = await temporaryDirectory(); + // Some filesystems (notably user dirs on macOS APFS) ignore sticky on + // chmod; fall back to the process temp root when it is already sticky. + let stickyParent = join(root, "shared"); + await mkdir(stickyParent, { mode: 0o1777 }); + await chmod(stickyParent, 0o1777); + if (((await lstat(stickyParent)).mode & 0o1000) === 0) { + stickyParent = await realpath(tmpdir()); + if (((await lstat(stickyParent)).mode & 0o1000) === 0) { + return; + } + } + const stateDirectory = join( + stickyParent, + `codex-security-sticky-${process.pid}-${Date.now()}`, + ); + temporaryDirectories.push(stateDirectory); + await mkdir(stateDirectory, { recursive: true, mode: 0o700 }); + const home = await prepareCodexSecurityCredentialHome({ + CODEX_SECURITY_STATE_DIR: stateDirectory, + }); + await expect(requireSecureCredentialHome(home)).resolves.toBeDefined(); + await expect( + requireSecureOutputAncestry(home), + ).resolves.toBeUndefined(); + }, + ); + + testPosix("rejects sticky shared parents controlled by another user", () => { + expect(() => + requireTrustedOutputAncestor( + { mode: 0o41777, uid: 1001 }, + "/shared", + 1000, + ), + ).toThrow("trusted owner"); + expect(() => + requireTrustedOutputAncestor( + { mode: 0o40755, uid: 1001 }, + "/shared", + 1000, + ), + ).toThrow("trusted owner"); + expect(() => + requireTrustedOutputAncestor( + { mode: 0o41777, uid: 1000 }, + "/shared", + 1000, + ), + ).not.toThrow(); + expect(() => + requireTrustedOutputAncestor({ mode: 0o41777, uid: 0 }, "/tmp", 1000), + ).not.toThrow(); + }); + + testPosix( + "rejects a credential home that is no longer private to the current user", + async () => { + const root = await temporaryDirectory(); + const home = await prepareCodexSecurityCredentialHome({ + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }); + await chmod(home, 0o755); + await expect(requireSecureCredentialHome(home)).rejects.toThrow( + "must not be accessible to other users", + ); + await expect( + acquireCodexSecurityCredentialHomeLock(home), + ).rejects.toThrow("must not be accessible to other users"); + }, + ); + + testPosix( + "pins credential-home identity for the duration of a lock session", + async () => { + const root = await temporaryDirectory(); + const home = await prepareCodexSecurityCredentialHome({ + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }); + const release = await acquireCodexSecurityCredentialHomeLock(home); + const stolen = join(root, "stolen-home"); + await rename(home, stolen); + await mkdir(home, { recursive: true, mode: 0o700 }); + await chmod(home, 0o700); + await expect(release()).rejects.toThrow("credential home was replaced"); + }, + ); + + testPosix( + "rejects world-writable or symlink stored authentication files", + async () => { + const root = await temporaryDirectory(); + const home = await prepareCodexSecurityCredentialHome({ + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }); + const authPath = join(home, "auth.json"); + await writeFile(authPath, '{"token":"test"}\n', { mode: 0o600 }); + expect(await codexSecurityHasStoredFileCredentials(home)).toBe(true); + + await chmod(authPath, 0o644); + await expect(codexSecurityHasStoredFileCredentials(home)).rejects.toThrow( + "must not be accessible to other users", + ); + await rm(authPath); + + const target = join(home, "auth-target.json"); + await writeFile(target, '{"token":"test"}\n', { mode: 0o600 }); + await symlink(target, authPath); + await expect(codexSecurityHasStoredFileCredentials(home)).rejects.toThrow( + "not a regular file", + ); + + expect(() => + requirePrivateCredentialFile({ mode: 0o100644, uid: 1000 }, authPath, 1000), + ).toThrow("must not be accessible to other users"); + }, + ); + test("identifies a credential home that already exists as a regular file", async () => { const root = await temporaryDirectory(); const stateDirectory = join(root, "state"); From c996c4d61a0d350aef6140fc43c1714f76d6270e Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 11:34:09 -0700 Subject: [PATCH 17/31] fix: preserve safe credential login and shared ancestry contracts --- sdk/typescript/src/runtime.ts | 14 +++++++------- .../tests-ts/cli-authentication.test.ts | 17 ++++++++++++++++- sdk/typescript/tests-ts/runtime.test.ts | 16 +++++++++------- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index da39f1fa..2a761536 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -832,7 +832,7 @@ export function requirePrivateOutputDirectory( } } -/** Reject shared parents whose owner can rename or replace a private directory. */ +/** Reject shared parents whose owner can rename or replace private scan output. */ export async function requireSecureOutputAncestry( path: string, effectiveUid = process.geteuid?.(), @@ -846,14 +846,14 @@ export async function requireSecureOutputAncestry( } catch (error) { if (nodeErrorCode(error) !== "ENOENT") { throw new OutputDirectoryError( - `Unable to inspect parent directory: ${current}`, + `Unable to inspect scan output parent directory: ${current}`, { cause: error }, ); } const parent = dirname(current); if (parent === current) { throw new OutputDirectoryError( - `Unable to inspect parent directory: ${current}`, + `Unable to inspect scan output parent directory: ${current}`, { cause: error }, ); } @@ -866,13 +866,13 @@ export async function requireSecureOutputAncestry( metadata = await lstat(current); } catch (error) { throw new OutputDirectoryError( - `Unable to inspect parent directory: ${current}`, + `Unable to inspect scan output parent directory: ${current}`, { cause: error }, ); } if (!metadata.isDirectory() || metadata.isSymbolicLink()) { throw new OutputDirectoryError( - `Parent must be a non-symlink directory: ${current}`, + `Scan output parent must be a non-symlink directory: ${current}`, ); } requireTrustedOutputAncestor(metadata, current, effectiveUid); @@ -893,13 +893,13 @@ export function requireTrustedOutputAncestor( metadata.uid !== effectiveUid ) { throw new OutputDirectoryError( - `Parent must have a trusted owner: ${path}`, + `Scan output parent must have a trusted owner: ${path}`, ); } if ((metadata.mode & 0o022) === 0) return; if ((metadata.mode & 0o1000) === 0) { throw new OutputDirectoryError( - `Parent must not be group- or world-writable without the sticky bit: ${path}`, + `Scan output parent must not be group- or world-writable without the sticky bit: ${path}`, ); } } diff --git a/sdk/typescript/tests-ts/cli-authentication.test.ts b/sdk/typescript/tests-ts/cli-authentication.test.ts index e51f4363..f2e4c00b 100644 --- a/sdk/typescript/tests-ts/cli-authentication.test.ts +++ b/sdk/typescript/tests-ts/cli-authentication.test.ts @@ -19,11 +19,26 @@ import { } from "../src/runtime.js"; import { capture, - dependencies, + dependencies as cliDependencies, fakePreflight, fakeResult, } from "./support/cli.js"; +function dependencies( + options: Parameters[0] = {}, +): ReturnType { + return cliDependencies({ + ...options, + environment: { + CODEX_SECURITY_STATE_DIR: join( + tmpdir(), + `codex-security-cli-authentication-${process.pid}`, + ), + ...options.environment, + }, + }); +} + describe("CLI authentication", () => { test("delegates login and logout without overriding managed credential storage", async () => { const cases = [ diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index 17a6b5f0..3ba9ea11 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -1337,9 +1337,9 @@ describe("runtime directories and plugin Python boundary", () => { await expect( prepareCodexSecurityCredentialHome(environment), ).rejects.toThrow("sticky bit"); - await expect(requireSecureOutputAncestry(join(shared, "state"))).rejects.toThrow( - "sticky bit", - ); + await expect( + requireSecureOutputAncestry(join(shared, "state")), + ).rejects.toThrow("sticky bit"); }, ); @@ -1368,9 +1368,7 @@ describe("runtime directories and plugin Python boundary", () => { CODEX_SECURITY_STATE_DIR: stateDirectory, }); await expect(requireSecureCredentialHome(home)).resolves.toBeDefined(); - await expect( - requireSecureOutputAncestry(home), - ).resolves.toBeUndefined(); + await expect(requireSecureOutputAncestry(home)).resolves.toBeUndefined(); }, ); @@ -1459,7 +1457,11 @@ describe("runtime directories and plugin Python boundary", () => { ); expect(() => - requirePrivateCredentialFile({ mode: 0o100644, uid: 1000 }, authPath, 1000), + requirePrivateCredentialFile( + { mode: 0o100644, uid: 1000 }, + authPath, + 1000, + ), ).toThrow("must not be accessible to other users"); }, ); From 89e53213a54faebbff78da5b6b5bc668842087ab Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 11:37:57 -0700 Subject: [PATCH 18/31] test: use private credential homes for authentication fixtures --- sdk/typescript/tests-ts/api.test.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 32937f30..b116690c 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -3487,11 +3487,13 @@ if (process.argv.slice(2).join(" ") !== "login status") { ).resolves.toBe(false); expect(imported).toBe(false); + const isolatedHome = join(await temporaryDirectory(), "isolated-home"); + await mkdir(isolatedHome, { mode: 0o700 }); await expect( initialCredentialsAvailable( { OPENAI_API_KEY: " " }, "/ambient-home", - "/isolated-home", + isolatedHome, async () => true, ), ).resolves.toBe(true); @@ -3502,11 +3504,12 @@ if (process.argv.slice(2).join(" ") !== "login status") { const ambientHome = join(root, "ambient-home"); const credentialHome = join(root, "credential-home"); await mkdir(ambientHome); - await mkdir(credentialHome); + await mkdir(credentialHome, { mode: 0o700 }); await writeFile(join(ambientHome, "auth.json"), '{"token":"ambient"}\n'); await writeFile( join(credentialHome, "auth.json"), '{"token":"explicit"}\n', + { mode: 0o600 }, ); let imported = false; @@ -3528,7 +3531,7 @@ if (process.argv.slice(2).join(" ") !== "login status") { const ambientHome = join(root, "ambient-home"); const credentialHome = join(root, "credential-home"); await mkdir(ambientHome); - await mkdir(credentialHome); + await mkdir(credentialHome, { mode: 0o700 }); await writeFile(join(ambientHome, "auth.json"), '{"token":"ambient"}\n'); await setCodexSecurityCredentialLogout(credentialHome, true); let imported = false; @@ -3561,7 +3564,7 @@ if (process.argv.slice(2).join(" ") !== "login status") { const ambientAuthentication = '{"auth_mode":"chatgpt"}\n'; await mkdir(repository); await mkdir(ambientHome); - await mkdir(codexHome); + await mkdir(codexHome, { mode: 0o700 }); await mkdir(scanDir, { mode: 0o700 }); await writeFile(join(ambientHome, "auth.json"), ambientAuthentication); const authentications: ScanAuthentication[] = []; From 41781a9b52b7ef28dde19c12f93d7d0e266e4bea Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 12:04:14 -0700 Subject: [PATCH 19/31] fix: redact PEM credentials and revalidate Windows home ACLs --- sdk/typescript/src/errors.ts | 4 ++++ sdk/typescript/src/runtime.ts | 12 ++++-------- sdk/typescript/tests-ts/cli.test.ts | 7 +++++++ sdk/typescript/tests-ts/runtime.test.ts | 24 ++++++++++++++++++++++++ 4 files changed, 39 insertions(+), 8 deletions(-) diff --git a/sdk/typescript/src/errors.ts b/sdk/typescript/src/errors.ts index a6bd9ee3..b6044e06 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -4,6 +4,10 @@ import { formatUsd, type ScanCost } from "./cost.js"; export function redactedErrorMessage(error: unknown): string { const message = error instanceof Error ? error.message : String(error); return message + .replaceAll( + /(\b[A-Za-z0-9_-]{0,64}private[_-]?key(?:[_-][A-Za-z0-9_-]{1,64})?\b\s*[:=]\s*)(?:\\?["'])?-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z0-9 ]*PRIVATE KEY-----(?:\\?["'])?/giu, + "$1[redacted]", + ) .replaceAll( /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b\s*[:=]\s*)(["'])(?:\\.|(?!\2)[^\\])*\2/giu, "$1$2[redacted]$2", diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index 2a761536..b519c74c 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -155,7 +155,6 @@ export async function prepareCodexSecurityCredentialHome( requireModelSafeOutputDir(canonical); validateLocation?.(canonical); await requireSecureCredentialHome(canonical, { - applyWindowsAcl: true, metadata, }); return canonical; @@ -181,7 +180,6 @@ export async function requireSecureCredentialHome( options: { platform?: NodeJS.Platform; secureWindowsHome?: (path: string) => Promise; - applyWindowsAcl?: boolean; metadata?: Stats; expectedDevice?: number; expectedInode?: number; @@ -225,12 +223,10 @@ export async function requireSecureCredentialHome( ); } if (platform === "win32") { - if (options.applyWindowsAcl) { - await requirePrivateCredentialHome(metadata, canonical, { - platform, - secureWindowsHome: options.secureWindowsHome, - }); - } + await requirePrivateCredentialHome(metadata, canonical, { + platform, + secureWindowsHome: options.secureWindowsHome, + }); return metadata; } await requirePrivateCredentialHome(metadata, canonical, { platform }); diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index b985853b..f286b355 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -2164,6 +2164,13 @@ describe("CLI", () => { 'password="correct horse battery staple" private_key=SYNTHETIC_PRIVATE_KEY_123', ), ).toBe('password="[redacted]" private_key=[redacted]'); + for (const separator of ["\n", "\\n"]) { + expect( + redactedErrorMessage( + `private_key=-----BEGIN PRIVATE KEY-----${separator}MII_SYNTHETIC_PRIVATE_KEY${separator}-----END PRIVATE KEY----- safe=value`, + ), + ).toBe("private_key=[redacted] safe=value"); + } }); test("reports database connection failures without claiming the model network failed", async () => { diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index 3ba9ea11..a128c584 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -1590,6 +1590,30 @@ describe("runtime directories and plugin Python boundary", () => { ).rejects.toThrow("private Windows credential home"); }); + test("revalidates the Windows credential ACL every time the home is used", async () => { + const root = await temporaryDirectory(); + const home = join(root, "home"); + await mkdir(home); + const validations: string[] = []; + + await requireSecureCredentialHome(home, { + platform: "win32", + secureWindowsHome: async (path) => { + validations.push(path); + }, + }); + + expect(validations).toEqual([home]); + await expect( + requireSecureCredentialHome(home, { + platform: "win32", + secureWindowsHome: async () => { + throw new Error("ACL changed after preparation"); + }, + }), + ).rejects.toThrow("private Windows credential home"); + }); + test.skipIf(process.platform !== "win32")( "creates credential homes with a verified current-user-only Windows ACL", async () => { From ab15ca9ba01ecd04fb4863fd505a0b7af6300c57 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 12:11:51 -0700 Subject: [PATCH 20/31] test: synchronously flush synthetic ChatGPT login prompts --- sdk/typescript/tests-ts/api.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index b116690c..c28059ba 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -4146,7 +4146,7 @@ if (process.argv.slice(2).join(" ") !== "login status") { await writeFile( fakeCodex, ` -import { appendFileSync } from "node:fs"; +import { appendFileSync, writeSync } from "node:fs"; const args = process.argv.slice(2).join(" "); if (args === "login --with-api-key") { @@ -4158,7 +4158,7 @@ if (args === "login --with-api-key") { appendFileSync(${JSON.stringify(keyLog)}, apiKey); } } else if (args === "login") { - console.error("Open https://auth.example.test/login"); + writeSync(2, "Open https://auth.example.test/login\\n"); process.exit(0); } else { process.exitCode = 2; From e82905dd814beb26d22cde28b0cf4d6e68cb2ba4 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 12:43:02 -0700 Subject: [PATCH 21/31] fix: atomically publish ambient auth credentials --- sdk/typescript/src/runtime.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index b519c74c..b5484a50 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -4,6 +4,7 @@ import { constants, existsSync, type Stats } from "node:fs"; import { chmod, copyFile, + link, lstat, mkdir, mkdtemp, @@ -963,7 +964,7 @@ export async function importAmbientAuth( await copyFile(source, temporary, constants.COPYFILE_EXCL); await chmod(temporary, 0o600); try { - await copyFile(temporary, destination, constants.COPYFILE_EXCL); + await link(temporary, destination); } catch (error) { if ( nodeErrorCode(error) === "EEXIST" && From ac3189b1418bd2bfd217c98efc0c436bd82ecce5 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 13:01:07 -0700 Subject: [PATCH 22/31] fix: redact JSON-quoted credentials before persistent failures --- sdk/typescript/src/errors.ts | 4 ++-- sdk/typescript/tests-ts/api.test.ts | 13 ++++++++++--- sdk/typescript/tests-ts/cli.test.ts | 10 ++++++++++ sdk/typescript/tests-ts/multiscan.test.ts | 4 +++- 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/sdk/typescript/src/errors.ts b/sdk/typescript/src/errors.ts index b6044e06..69e992d2 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -5,11 +5,11 @@ export function redactedErrorMessage(error: unknown): string { const message = error instanceof Error ? error.message : String(error); return message .replaceAll( - /(\b[A-Za-z0-9_-]{0,64}private[_-]?key(?:[_-][A-Za-z0-9_-]{1,64})?\b\s*[:=]\s*)(?:\\?["'])?-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z0-9 ]*PRIVATE KEY-----(?:\\?["'])?/giu, + /(\b[A-Za-z0-9_-]{0,64}private[_-]?key(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*)(?:\\?["'])?-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z0-9 ]*PRIVATE KEY-----(?:\\?["'])?/giu, "$1[redacted]", ) .replaceAll( - /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b\s*[:=]\s*)(["'])(?:\\.|(?!\2)[^\\])*\2/giu, + /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*)(\\?["'])(?:(?!\2)(?:\\.|[^\\]))*\2/giu, "$1$2[redacted]$2", ) .replaceAll( diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index c28059ba..0ce7f5be 100644 --- a/sdk/typescript/tests-ts/api.test.ts +++ b/sdk/typescript/tests-ts/api.test.ts @@ -2321,6 +2321,10 @@ describe("CodexSecurity orchestration", () => { CODEX_SECURITY_STATE_DIR: stateDirectory, }; const commands: Array = []; + const quotedCredential = JSON.stringify({ + client_secret_value: "SYNTHETIC correct horse battery staple", + }); + const redactedFailure = `${REDACTED_CREDENTIALS} {"client_secret_value":"[redacted]"}`; const client = new TestClient( {}, { @@ -2344,7 +2348,10 @@ describe("CodexSecurity orchestration", () => { id: null, async runStreamed() { async function* failingEvents(): AsyncGenerator { - yield { type: "error", message: SYNTHETIC_CREDENTIALS }; + yield { + type: "error", + message: `${SYNTHETIC_CREDENTIALS} ${quotedCredential}`, + }; } return { events: failingEvents() }; }, @@ -2360,7 +2367,7 @@ describe("CodexSecurity orchestration", () => { const scanId = failure?.[2] ?? ""; expect(scanId).toMatch(/^[0-9a-f-]{36}$/); expect(failure?.[3]).toBe("--message"); - expect(failure?.[4]).toBe(REDACTED_CREDENTIALS); + expect(failure?.[4]).toBe(redactedFailure); // `scans show` reads the stored message back through get-scan. const context = await runWorkbench( @@ -2369,7 +2376,7 @@ describe("CodexSecurity orchestration", () => { ); expect(context["scan"]).toMatchObject({ progress: { status: "failed" }, - failureMessage: REDACTED_CREDENTIALS, + failureMessage: redactedFailure, }); // Every synthetic credential is tagged SYNTHETIC, so the database file diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index f286b355..8cbc5e52 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -2164,6 +2164,16 @@ describe("CLI", () => { 'password="correct horse battery staple" private_key=SYNTHETIC_PRIVATE_KEY_123', ), ).toBe('password="[redacted]" private_key=[redacted]'); + expect( + redactedErrorMessage( + '{"client_secret_value":"correct horse battery staple","safe":"visible"}', + ), + ).toBe('{"client_secret_value":"[redacted]","safe":"visible"}'); + expect( + redactedErrorMessage( + '{\\"access_token_value\\":\\"another horse battery staple\\"}', + ), + ).toBe('{\\"access_token_value\\":\\"[redacted]\\"}'); for (const separator of ["\n", "\\n"]) { expect( redactedErrorMessage( diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index 61033a3b..fb4dae34 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -452,6 +452,7 @@ describe("multiscan", () => { const suffixedSecret = "SYNTHETIC_SUFFIXED_CLIENT_SECRET_123"; const suffixedToken = "SYNTHETIC_SUFFIXED_ACCESS_TOKEN_123"; const suffixedQuery = "SYNTHETIC_SUFFIXED_QUERY_SECRET_123"; + const quotedSecret = "SYNTHETIC correct horse battery staple"; await writeFile( paths.input, `id,repository,revision\nretry,${source.path},${source.revision}\n`, @@ -465,7 +466,7 @@ describe("multiscan", () => { attempts += 1; if (attempts === 1) { throw new Error( - `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, + `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} ${JSON.stringify({ client_secret_value: quotedSecret })} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, ); } return await completedScan(scanOptions.outputDir!); @@ -486,6 +487,7 @@ describe("multiscan", () => { expect(ledger).not.toContain(suffixedSecret); expect(ledger).not.toContain(suffixedToken); expect(ledger).not.toContain(suffixedQuery); + expect(ledger).not.toContain(quotedSecret); expect(ledger).not.toContain(shortAuthorization); expect(ledger).toContain("https://[redacted]@proxy.test/v1/responses"); }); From 858ce979007f28fd18c8fa863b1bb4f23a66cd7a Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 13:11:08 -0700 Subject: [PATCH 23/31] fix: redact recursively escaped credential strings --- sdk/typescript/src/errors.ts | 46 +++++++++++++++++++++++------ sdk/typescript/tests-ts/cli.test.ts | 15 ++++++++++ 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/sdk/typescript/src/errors.ts b/sdk/typescript/src/errors.ts index 69e992d2..2d70e52b 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -3,15 +3,11 @@ import { formatUsd, type ScanCost } from "./cost.js"; /** Returns an error message with credential-shaped substrings redacted. */ export function redactedErrorMessage(error: unknown): string { const message = error instanceof Error ? error.message : String(error); - return message - .replaceAll( - /(\b[A-Za-z0-9_-]{0,64}private[_-]?key(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*)(?:\\?["'])?-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z0-9 ]*PRIVATE KEY-----(?:\\?["'])?/giu, - "$1[redacted]", - ) - .replaceAll( - /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*)(\\?["'])(?:(?!\2)(?:\\.|[^\\]))*\2/giu, - "$1$2[redacted]$2", - ) + const withoutPrivateKeys = message.replaceAll( + /(\b[A-Za-z0-9_-]{0,64}private[_-]?key(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*)(?:\\?["'])?-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z0-9 ]*PRIVATE KEY-----(?:\\?["'])?/giu, + "$1[redacted]", + ); + return redactQuotedCredentialValues(withoutPrivateKeys) .replaceAll( /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)(?!\[redacted\])[^\s"',;}&\\\]]+/giu, "$1[redacted]", @@ -30,6 +26,38 @@ export function redactedErrorMessage(error: unknown): string { ); } +function redactQuotedCredentialValues(message: string): string { + const assignment = + /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\*["'])?\s*[:=]\s*)(\\*)(["'])/giu; + let output = ""; + let consumed = 0; + for ( + let match = assignment.exec(message); + match !== null; + match = assignment.exec(message) + ) { + const openingSlashes = match[2]!.length; + const quote = match[3]!; + let position = assignment.lastIndex; + while (position < message.length) { + const delimiter = message.indexOf(quote, position); + if (delimiter < 0) break; + let preceding = delimiter; + while (preceding > position && message[preceding - 1] === "\\") { + preceding -= 1; + } + if (delimiter - preceding === openingSlashes) { + output += `${message.slice(consumed, assignment.lastIndex)}[redacted]${message.slice(preceding, delimiter + 1)}`; + consumed = delimiter + 1; + assignment.lastIndex = consumed; + break; + } + position = delimiter + 1; + } + } + return output + message.slice(consumed); +} + /** Base error for Codex Security SDK failures. */ export class CodexSecurityError extends Error { public constructor(message: string, options?: ErrorOptions) { diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index 8cbc5e52..0035cfee 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -2174,6 +2174,21 @@ describe("CLI", () => { '{\\"access_token_value\\":\\"another horse battery staple\\"}', ), ).toBe('{\\"access_token_value\\":\\"[redacted]\\"}'); + let encoded: string | { password: string } = { + password: 'foo "bar" baz', + }; + for (let depth = 1; depth <= 3; depth += 1) { + encoded = JSON.stringify(encoded); + const redacted = redactedErrorMessage(encoded); + expect(redacted).not.toContain("foo"); + expect(redacted).not.toContain("bar"); + expect(redacted).not.toContain("baz"); + let decoded: unknown = redacted; + for (let layer = 0; layer < depth; layer += 1) { + decoded = JSON.parse(decoded as string); + } + expect(decoded).toEqual({ password: "[redacted]" }); + } for (const separator of ["\n", "\\n"]) { expect( redactedErrorMessage( From 10fcd51cc109f711900b252a67d219e8d8d923db Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 13:27:50 -0700 Subject: [PATCH 24/31] fix: close credential redaction and home identity races --- sdk/typescript/src/errors.ts | 6 +-- sdk/typescript/src/runtime.ts | 49 ++++++++++++++++------ sdk/typescript/tests-ts/cli.test.ts | 7 ++++ sdk/typescript/tests-ts/multiscan.test.ts | 6 ++- sdk/typescript/tests-ts/runtime.test.ts | 50 +++++++++++++++++++++++ 5 files changed, 102 insertions(+), 16 deletions(-) diff --git a/sdk/typescript/src/errors.ts b/sdk/typescript/src/errors.ts index 2d70e52b..a8a293cb 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -9,7 +9,7 @@ export function redactedErrorMessage(error: unknown): string { ); return redactQuotedCredentialValues(withoutPrivateKeys) .replaceAll( - /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)(?!\[redacted\])[^\s"',;}&\\\]]+/giu, + /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|authorization|auth|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)(?!\[redacted\]|(?:Bearer|Basic|Token)(?:\s|%20|\+))[^\s"',;}&\\\]]+/giu, "$1[redacted]", ) .replaceAll(/sk-(?:proj-)?[A-Za-z0-9_*=-]{8,}/gu, "[redacted]") @@ -21,14 +21,14 @@ export function redactedErrorMessage(error: unknown): string { ) .replaceAll(/((?:https?|ssh|git\+ssh):\/\/)[^\s/@]+@/giu, "$1[redacted]@") .replaceAll( - /((?:[?&]|%3F|%26)(?:(?!%3F|%26|%3D)(?:[A-Za-z0-9_.%-]|\[|\])){0,64}(?:api[_-]?key|access(?:[_-]|%5F|%2D)?key(?:(?:[_-]|%5F|%2D)?id)?|private(?:[_-]|%5F|%2D)?key|token|secret|credential|signature|sig|password|passwd)(?:(?:[_-]|%5F|%2D)[A-Za-z0-9_.%-]{1,64})?(?:\]|%5D)?(?:=|%3D))(?:(?!%26)[^&\s])+/giu, + /((?:[?&]|%3F|%26)(?:(?!%3F|%26|%3D)(?:[A-Za-z0-9_.%-]|\[|\])){0,64}(?:api[_-]?key|access(?:[_-]|%5F|%2D)?key(?:(?:[_-]|%5F|%2D)?id)?|private(?:[_-]|%5F|%2D)?key|authorization|auth|token|secret|credential|signature|sig|password|passwd)(?:(?:[_-]|%5F|%2D)[A-Za-z0-9_.%-]{1,64})?(?:\]|%5D)?(?:=|%3D))(?:(?!%26)[^&\s])+/giu, "$1[redacted]", ); } function redactQuotedCredentialValues(message: string): string { const assignment = - /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\*["'])?\s*[:=]\s*)(\\*)(["'])/giu; + /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|authorization|auth|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\*["'])?\s*[:=]\s*)(\\*)(["'])/giu; let output = ""; let consumed = 0; for ( diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index b5484a50..d556667c 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -184,6 +184,7 @@ export async function requireSecureCredentialHome( metadata?: Stats; expectedDevice?: number; expectedInode?: number; + validateWindowsAcl?: boolean; } = {}, ): Promise { const platform = options.platform ?? process.platform; @@ -205,13 +206,20 @@ export async function requireSecureCredentialHome( } const canonical = await realpath(path); requireModelSafeOutputDir(canonical); - if (canonical !== path) { - metadata = await lstat(canonical); - if (!metadata.isDirectory() || metadata.isSymbolicLink()) { - throw new OutputDirectoryError( - `Codex Security credential home is not a directory: ${path}`, - ); - } + const canonicalMetadata = await lstat(canonical); + if ( + canonicalMetadata.dev !== metadata.dev || + canonicalMetadata.ino !== metadata.ino + ) { + throw new OutputDirectoryError( + `Codex Security credential home was replaced: ${canonical}`, + ); + } + metadata = canonicalMetadata; + if (!metadata.isDirectory() || metadata.isSymbolicLink()) { + throw new OutputDirectoryError( + `Codex Security credential home is not a directory: ${path}`, + ); } if ( options.expectedDevice !== undefined && @@ -224,10 +232,12 @@ export async function requireSecureCredentialHome( ); } if (platform === "win32") { - await requirePrivateCredentialHome(metadata, canonical, { - platform, - secureWindowsHome: options.secureWindowsHome, - }); + if (options.validateWindowsAcl !== false) { + await requirePrivateCredentialHome(metadata, canonical, { + platform, + secureWindowsHome: options.secureWindowsHome, + }); + } return metadata; } await requirePrivateCredentialHome(metadata, canonical, { platform }); @@ -302,8 +312,15 @@ async function secureWindowsCredentialHome(path: string): Promise { export async function acquireCodexSecurityCredentialHomeLock( codexHome: string, signal?: AbortSignal, + securityOptions: { + platform?: NodeJS.Platform; + secureWindowsHome?: (path: string) => Promise; + } = {}, ): Promise<() => Promise> { - const homeMetadata = await requireSecureCredentialHome(codexHome); + const homeMetadata = await requireSecureCredentialHome( + codexHome, + securityOptions, + ); const expectedDevice = homeMetadata.dev; const expectedInode = homeMetadata.ino; const lock = join(codexHome, CREDENTIAL_LOCK_NAME); @@ -313,8 +330,10 @@ export async function acquireCodexSecurityCredentialHomeLock( while (true) { throwIfSignalAborted(signal); await requireSecureCredentialHome(codexHome, { + ...securityOptions, expectedDevice, expectedInode, + validateWindowsAcl: false, }); try { await mkdir(lock, { mode: 0o700 }); @@ -326,6 +345,11 @@ export async function acquireCodexSecurityCredentialHomeLock( } try { + await requireSecureCredentialHome(codexHome, { + ...securityOptions, + expectedDevice, + expectedInode, + }); await writeFile( ownerPath, `${JSON.stringify({ pid: process.pid, token })}\n`, @@ -340,6 +364,7 @@ export async function acquireCodexSecurityCredentialHomeLock( return async () => { if (released) return; await requireSecureCredentialHome(codexHome, { + ...securityOptions, expectedDevice, expectedInode, }); diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index 0035cfee..f0419ab0 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -2174,6 +2174,13 @@ describe("CLI", () => { '{\\"access_token_value\\":\\"another horse battery staple\\"}', ), ).toBe('{\\"access_token_value\\":\\"[redacted]\\"}'); + expect( + redactedErrorMessage( + 'authorization="opaque secret value" _auth=Zm9vOmJhcg== https://example.test/?authorization=opaque%20query%20secret', + ), + ).toBe( + 'authorization="[redacted]" _auth=[redacted] https://example.test/?authorization=[redacted]', + ); let encoded: string | { password: string } = { password: 'foo "bar" baz', }; diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index fb4dae34..657c2c53 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -453,6 +453,8 @@ describe("multiscan", () => { const suffixedToken = "SYNTHETIC_SUFFIXED_ACCESS_TOKEN_123"; const suffixedQuery = "SYNTHETIC_SUFFIXED_QUERY_SECRET_123"; const quotedSecret = "SYNTHETIC correct horse battery staple"; + const opaqueAuthorization = "SYNTHETIC opaque authorization secret"; + const npmAuthorization = "SYNTHETIC_NPM_AUTH_VALUE_123"; await writeFile( paths.input, `id,repository,revision\nretry,${source.path},${source.revision}\n`, @@ -466,7 +468,7 @@ describe("multiscan", () => { attempts += 1; if (attempts === 1) { throw new Error( - `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} ${JSON.stringify({ client_secret_value: quotedSecret })} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, + `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} ${JSON.stringify({ client_secret_value: quotedSecret })} authorization="${opaqueAuthorization}" _auth=${npmAuthorization} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, ); } return await completedScan(scanOptions.outputDir!); @@ -488,6 +490,8 @@ describe("multiscan", () => { expect(ledger).not.toContain(suffixedToken); expect(ledger).not.toContain(suffixedQuery); expect(ledger).not.toContain(quotedSecret); + expect(ledger).not.toContain(opaqueAuthorization); + expect(ledger).not.toContain(npmAuthorization); expect(ledger).not.toContain(shortAuthorization); expect(ledger).toContain("https://[redacted]@proxy.test/v1/responses"); }); diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index a128c584..5d1658a1 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -1432,6 +1432,23 @@ describe("runtime directories and plugin Python boundary", () => { }, ); + testPosix( + "rejects stale credential-home metadata after canonical target replacement", + async () => { + const root = await temporaryDirectory(); + const home = await prepareCodexSecurityCredentialHome({ + CODEX_SECURITY_STATE_DIR: join(root, "state"), + }); + const stale = await lstat(home); + await rename(home, join(root, "original-home")); + await mkdir(home, { mode: 0o700 }); + + await expect( + requireSecureCredentialHome(home, { metadata: stale }), + ).rejects.toThrow("credential home was replaced"); + }, + ); + testPosix( "rejects world-writable or symlink stored authentication files", async () => { @@ -1522,6 +1539,39 @@ describe("runtime directories and plugin Python boundary", () => { } }); + test("does not rewrite Windows credential ACLs while polling a held lock", async () => { + const root = await temporaryDirectory(); + const home = join(root, "credential-home"); + await mkdir(home, { mode: 0o700 }); + const validations: string[] = []; + const securityOptions = { + platform: "win32" as const, + secureWindowsHome: async (path: string) => { + validations.push(path); + }, + }; + const release = await acquireCodexSecurityCredentialHomeLock( + home, + undefined, + securityOptions, + ); + const controller = new AbortController(); + const waiting = acquireCodexSecurityCredentialHomeLock( + home, + controller.signal, + securityOptions, + ); + + try { + await new Promise((resolve) => setTimeout(resolve, 100)); + expect(validations).toHaveLength(3); + controller.abort(new DOMException("canceled", "AbortError")); + await expect(waiting).rejects.toMatchObject({ name: "AbortError" }); + } finally { + await release(); + } + }); + test("recovers credential-home locks left by exited processes", async () => { const root = await temporaryDirectory(); const home = await prepareCodexSecurityCredentialHome({ From 73f1b51a61779c041bcc134a32e566cd36d104e4 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 13:50:30 -0700 Subject: [PATCH 25/31] fix: seal credential locking and redact custom authorization schemes --- sdk/typescript/src/errors.ts | 6 +++++- sdk/typescript/src/runtime.ts | 19 ++++++++++++++----- sdk/typescript/tests-ts/cli.test.ts | 13 +++++++++++++ sdk/typescript/tests-ts/multiscan.test.ts | 4 +++- sdk/typescript/tests-ts/runtime.test.ts | 4 ++++ 5 files changed, 39 insertions(+), 7 deletions(-) diff --git a/sdk/typescript/src/errors.ts b/sdk/typescript/src/errors.ts index a8a293cb..7200ffd5 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -9,7 +9,11 @@ export function redactedErrorMessage(error: unknown): string { ); return redactQuotedCredentialValues(withoutPrivateKeys) .replaceAll( - /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|authorization|auth|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)(?!\[redacted\]|(?:Bearer|Basic|Token)(?:\s|%20|\+))[^\s"',;}&\\\]]+/giu, + /(\b(?:authorization|auth)\b(?:\\?["'])?\s*[:=]\s*)([A-Za-z][A-Za-z0-9._~-]{0,63})((?:\s|%20|\+)+)(?!\[redacted\])[^\s"',;}&\\\]]+/giu, + "$1$2$3[redacted]", + ) + .replaceAll( + /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|authorization|auth|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)(?!\[redacted\]|[A-Za-z][A-Za-z0-9._~-]{0,63}(?:\s|%20|\+)+\[redacted\])[^\s"',;}&\\\]]+/giu, "$1[redacted]", ) .replaceAll(/sk-(?:proj-)?[A-Za-z0-9_*=-]{8,}/gu, "[redacted]") diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index d556667c..6855a22e 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -335,6 +335,20 @@ export async function acquireCodexSecurityCredentialHomeLock( expectedInode, validateWindowsAcl: false, }); + const existingLock = await lstat(lock).catch((error: unknown) => { + if (nodeErrorCode(error) === "ENOENT") return null; + throw error; + }); + if (existingLock !== null) { + if (await recoverStaleCredentialHomeLock(lock)) continue; + await delay(CREDENTIAL_LOCK_POLL_MILLISECONDS, undefined, { signal }); + continue; + } + await requireSecureCredentialHome(codexHome, { + ...securityOptions, + expectedDevice, + expectedInode, + }); try { await mkdir(lock, { mode: 0o700 }); } catch (error) { @@ -345,11 +359,6 @@ export async function acquireCodexSecurityCredentialHomeLock( } try { - await requireSecureCredentialHome(codexHome, { - ...securityOptions, - expectedDevice, - expectedInode, - }); await writeFile( ownerPath, `${JSON.stringify({ pid: process.pid, token })}\n`, diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index f0419ab0..39df343e 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -2181,6 +2181,19 @@ describe("CLI", () => { ).toBe( 'authorization="[redacted]" _auth=[redacted] https://example.test/?authorization=[redacted]', ); + for (const [authorization, redacted] of [ + [ + "Authorization: ApiKey SYNTHETIC_APIKEY_SECRET", + "Authorization: ApiKey [redacted]", + ], + ["auth=Custom%20SYNTHETIC_CUSTOM_SECRET", "auth=Custom%20[redacted]"], + [ + "Authorization: Digest+SYNTHETIC_DIGEST_SECRET", + "Authorization: Digest+[redacted]", + ], + ] as const) { + expect(redactedErrorMessage(authorization)).toBe(redacted); + } let encoded: string | { password: string } = { password: 'foo "bar" baz', }; diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index 657c2c53..626b1b0e 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -455,6 +455,7 @@ describe("multiscan", () => { const quotedSecret = "SYNTHETIC correct horse battery staple"; const opaqueAuthorization = "SYNTHETIC opaque authorization secret"; const npmAuthorization = "SYNTHETIC_NPM_AUTH_VALUE_123"; + const customAuthorization = "SYNTHETIC_CUSTOM_AUTHORIZATION_123"; await writeFile( paths.input, `id,repository,revision\nretry,${source.path},${source.revision}\n`, @@ -468,7 +469,7 @@ describe("multiscan", () => { attempts += 1; if (attempts === 1) { throw new Error( - `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} ${JSON.stringify({ client_secret_value: quotedSecret })} authorization="${opaqueAuthorization}" _auth=${npmAuthorization} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, + `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} ${JSON.stringify({ client_secret_value: quotedSecret })} authorization="${opaqueAuthorization}" _auth=${npmAuthorization} Authorization: ApiKey ${customAuthorization} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, ); } return await completedScan(scanOptions.outputDir!); @@ -492,6 +493,7 @@ describe("multiscan", () => { expect(ledger).not.toContain(quotedSecret); expect(ledger).not.toContain(opaqueAuthorization); expect(ledger).not.toContain(npmAuthorization); + expect(ledger).not.toContain(customAuthorization); expect(ledger).not.toContain(shortAuthorization); expect(ledger).toContain("https://[redacted]@proxy.test/v1/responses"); }); diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index 5d1658a1..198280a8 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -1547,6 +1547,10 @@ describe("runtime directories and plugin Python boundary", () => { const securityOptions = { platform: "win32" as const, secureWindowsHome: async (path: string) => { + const lock = join(path, ".codex-security-scan.lock"); + expect(existsSync(lock) && !existsSync(join(lock, "owner.json"))).toBe( + false, + ); validations.push(path); }, }; From 5b6d8ba84535e3eb17f34de083091e2e0de0f796 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 14:11:55 -0700 Subject: [PATCH 26/31] fix: bound login instructions and redact incomplete credentials --- sdk/typescript/src/auth.ts | 8 +++-- sdk/typescript/src/errors.ts | 9 ++++- sdk/typescript/tests-ts/auth.test.ts | 42 +++++++++++++++++++++++ sdk/typescript/tests-ts/cli.test.ts | 7 ++++ sdk/typescript/tests-ts/multiscan.test.ts | 4 ++- 5 files changed, 65 insertions(+), 5 deletions(-) diff --git a/sdk/typescript/src/auth.ts b/sdk/typescript/src/auth.ts index e432b97e..c8e75547 100644 --- a/sdk/typescript/src/auth.ts +++ b/sdk/typescript/src/auth.ts @@ -90,7 +90,9 @@ export class CodexLoginHandle { this.#clearForcedTermination(); this.#forceCompletion = null; this.#destroyPipes(); - this.#flushInstructionTails(); + if (!this.#outputLimitExceeded && !this.#canceled) { + this.#flushInstructionTails(); + } const result = this.#outputLimitExceeded ? { success: false, @@ -534,8 +536,8 @@ function visibleTerminalChunk( if (state === "text") { if (character === "\u001b") { state = "escape"; - } else if (character !== "\r") { - text += character; + } else { + text += character === "\r" ? "\n" : character; } continue; } diff --git a/sdk/typescript/src/errors.ts b/sdk/typescript/src/errors.ts index 7200ffd5..e224c361 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -9,7 +9,7 @@ export function redactedErrorMessage(error: unknown): string { ); return redactQuotedCredentialValues(withoutPrivateKeys) .replaceAll( - /(\b(?:authorization|auth)\b(?:\\?["'])?\s*[:=]\s*)([A-Za-z][A-Za-z0-9._~-]{0,63})((?:\s|%20|\+)+)(?!\[redacted\])[^\s"',;}&\\\]]+/giu, + /(\b[A-Za-z0-9_-]{0,64}(?:authorization|auth)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*)([A-Za-z][A-Za-z0-9._~-]{0,63})((?:\s|%20|\+)+)(?!\[redacted\]|[A-Za-z_][A-Za-z0-9_-]{0,64}\s*[:=])[^\s"',;}&\\\]]+/giu, "$1$2$3[redacted]", ) .replaceAll( @@ -43,6 +43,7 @@ function redactQuotedCredentialValues(message: string): string { const openingSlashes = match[2]!.length; const quote = match[3]!; let position = assignment.lastIndex; + let closed = false; while (position < message.length) { const delimiter = message.indexOf(quote, position); if (delimiter < 0) break; @@ -54,10 +55,16 @@ function redactQuotedCredentialValues(message: string): string { output += `${message.slice(consumed, assignment.lastIndex)}[redacted]${message.slice(preceding, delimiter + 1)}`; consumed = delimiter + 1; assignment.lastIndex = consumed; + closed = true; break; } position = delimiter + 1; } + if (!closed) { + output += `${message.slice(consumed, assignment.lastIndex)}[redacted]`; + consumed = message.length; + break; + } } return output + message.slice(consumed); } diff --git a/sdk/typescript/tests-ts/auth.test.ts b/sdk/typescript/tests-ts/auth.test.ts index fdb812b0..08a2e434 100644 --- a/sdk/typescript/tests-ts/auth.test.ts +++ b/sdk/typescript/tests-ts/auth.test.ts @@ -279,6 +279,48 @@ setTimeout(() => { await expect(handle.wait()).resolves.toMatchObject({ success: true }); }); + test("recognizes carriage-return-separated interactive login instructions", async () => { + const root = await mkdtemp(join(tmpdir(), "codex-security-auth-carriage-")); + temporaryDirectories.push(root); + const script = join(root, "login.mjs"); + await writeFile( + script, + 'process.stderr.write("Open https://auth.example.test/device\\rUser code: ABCD-EFGH\\r"); setTimeout(() => process.exit(0), 25);\n', + ); + const handle = new CodexLoginHandle( + { command: process.execPath, prefixArgs: [script] }, + ["login", "--device-auth"], + process.env, + () => {}, + ); + + await handle.waitForInstructions({ deviceCode: true }); + expect(handle.verificationUrl).toBe("https://auth.example.test/device"); + expect(handle.userCode).toBe("ABCD-EFGH"); + await expect(handle.wait()).resolves.toMatchObject({ success: true }); + }); + + test("rejects instruction waiters when an unfinished login exceeds its output bound", async () => { + const root = await mkdtemp(join(tmpdir(), "codex-security-auth-tail-")); + temporaryDirectories.push(root); + const script = join(root, "login.mjs"); + await writeFile( + script, + 'process.stderr.write("Open https://auth.example.test/device"); setTimeout(() => process.stderr.write("x".repeat(128 * 1024)), 20);\n', + ); + const handle = new CodexLoginHandle( + { command: process.execPath, prefixArgs: [script] }, + ["login"], + process.env, + () => {}, + ); + + await expect(handle.waitForInstructions()).rejects.toThrow( + "64 KiB safety limit", + ); + await expect(handle.wait()).resolves.toMatchObject({ success: false }); + }); + test("forces an oversized login to settle when it ignores termination", async () => { const root = await mkdtemp( join(tmpdir(), "codex-security-auth-output-kill-"), diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index 39df343e..f0f9346b 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -2191,9 +2191,16 @@ describe("CLI", () => { "Authorization: Digest+SYNTHETIC_DIGEST_SECRET", "Authorization: Digest+[redacted]", ], + [ + "client_authorization_value=ApiKey SYNTHETIC_SUFFIXED_SECRET", + "client_authorization_value=ApiKey [redacted]", + ], ] as const) { expect(redactedErrorMessage(authorization)).toBe(redacted); } + expect(redactedErrorMessage('password="correct horse battery staple')).toBe( + 'password="[redacted]', + ); let encoded: string | { password: string } = { password: 'foo "bar" baz', }; diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index 626b1b0e..99bc8639 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -456,6 +456,7 @@ describe("multiscan", () => { const opaqueAuthorization = "SYNTHETIC opaque authorization secret"; const npmAuthorization = "SYNTHETIC_NPM_AUTH_VALUE_123"; const customAuthorization = "SYNTHETIC_CUSTOM_AUTHORIZATION_123"; + const suffixedAuthorization = "SYNTHETIC_SUFFIXED_AUTHORIZATION_123"; await writeFile( paths.input, `id,repository,revision\nretry,${source.path},${source.revision}\n`, @@ -469,7 +470,7 @@ describe("multiscan", () => { attempts += 1; if (attempts === 1) { throw new Error( - `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} ${JSON.stringify({ client_secret_value: quotedSecret })} authorization="${opaqueAuthorization}" _auth=${npmAuthorization} Authorization: ApiKey ${customAuthorization} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, + `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} ${JSON.stringify({ client_secret_value: quotedSecret })} authorization="${opaqueAuthorization}" _auth=${npmAuthorization} Authorization: ApiKey ${customAuthorization} client_authorization_value=ApiKey ${suffixedAuthorization} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, ); } return await completedScan(scanOptions.outputDir!); @@ -494,6 +495,7 @@ describe("multiscan", () => { expect(ledger).not.toContain(opaqueAuthorization); expect(ledger).not.toContain(npmAuthorization); expect(ledger).not.toContain(customAuthorization); + expect(ledger).not.toContain(suffixedAuthorization); expect(ledger).not.toContain(shortAuthorization); expect(ledger).toContain("https://[redacted]@proxy.test/v1/responses"); }); From a020832eaa6224c464dcf66b65fa9e0e8974963f Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 14:28:22 -0700 Subject: [PATCH 27/31] fix: preserve padded authorization credentials during redaction --- sdk/typescript/src/errors.ts | 2 +- sdk/typescript/tests-ts/cli.test.ts | 6 ++++++ sdk/typescript/tests-ts/multiscan.test.ts | 4 +++- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/sdk/typescript/src/errors.ts b/sdk/typescript/src/errors.ts index e224c361..0198c495 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -9,7 +9,7 @@ export function redactedErrorMessage(error: unknown): string { ); return redactQuotedCredentialValues(withoutPrivateKeys) .replaceAll( - /(\b[A-Za-z0-9_-]{0,64}(?:authorization|auth)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*)([A-Za-z][A-Za-z0-9._~-]{0,63})((?:\s|%20|\+)+)(?!\[redacted\]|[A-Za-z_][A-Za-z0-9_-]{0,64}\s*[:=])[^\s"',;}&\\\]]+/giu, + /(\b[A-Za-z0-9_-]{0,64}(?:authorization|auth)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*)([A-Za-z][A-Za-z0-9._~-]{0,63})((?:\s|%20|\+)+)(?!\[redacted\]|[A-Za-z_][A-Za-z0-9_-]{0,64}\s*[:=]\s*(?=[^=\s"',;}&\\\]]))[^\s"',;}&\\\]]+/giu, "$1$2$3[redacted]", ) .replaceAll( diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index f0f9346b..89414421 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -2195,6 +2195,12 @@ describe("CLI", () => { "client_authorization_value=ApiKey SYNTHETIC_SUFFIXED_SECRET", "client_authorization_value=ApiKey [redacted]", ], + ["Authorization: ApiKey dGVzdA==", "Authorization: ApiKey [redacted]"], + ["Authorization: ApiKey dGVzdA=", "Authorization: ApiKey [redacted]"], + [ + "client_auth_token=Custom dGVzdA==", + "client_auth_token=Custom [redacted]", + ], ] as const) { expect(redactedErrorMessage(authorization)).toBe(redacted); } diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index 99bc8639..7afda307 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -457,6 +457,7 @@ describe("multiscan", () => { const npmAuthorization = "SYNTHETIC_NPM_AUTH_VALUE_123"; const customAuthorization = "SYNTHETIC_CUSTOM_AUTHORIZATION_123"; const suffixedAuthorization = "SYNTHETIC_SUFFIXED_AUTHORIZATION_123"; + const paddedAuthorization = "SYNTHETIC_PADDED_AUTHORIZATION_TOKEN=="; await writeFile( paths.input, `id,repository,revision\nretry,${source.path},${source.revision}\n`, @@ -470,7 +471,7 @@ describe("multiscan", () => { attempts += 1; if (attempts === 1) { throw new Error( - `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} ${JSON.stringify({ client_secret_value: quotedSecret })} authorization="${opaqueAuthorization}" _auth=${npmAuthorization} Authorization: ApiKey ${customAuthorization} client_authorization_value=ApiKey ${suffixedAuthorization} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, + `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} ${JSON.stringify({ client_secret_value: quotedSecret })} authorization="${opaqueAuthorization}" _auth=${npmAuthorization} Authorization: ApiKey ${customAuthorization} client_authorization_value=ApiKey ${suffixedAuthorization} auth=ApiKey ${paddedAuthorization} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, ); } return await completedScan(scanOptions.outputDir!); @@ -496,6 +497,7 @@ describe("multiscan", () => { expect(ledger).not.toContain(npmAuthorization); expect(ledger).not.toContain(customAuthorization); expect(ledger).not.toContain(suffixedAuthorization); + expect(ledger).not.toContain(paddedAuthorization); expect(ledger).not.toContain(shortAuthorization); expect(ledger).toContain("https://[redacted]@proxy.test/v1/responses"); }); From 5537f2357a91f0face1b455ae0de7c495c7d2d02 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 14:37:13 -0700 Subject: [PATCH 28/31] fix: redact camel-case credential fields before persistence --- sdk/typescript/src/errors.ts | 10 +++++----- sdk/typescript/tests-ts/cli.test.ts | 14 ++++++++++++++ sdk/typescript/tests-ts/multiscan.test.ts | 4 +++- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/sdk/typescript/src/errors.ts b/sdk/typescript/src/errors.ts index 0198c495..724d66df 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -4,16 +4,16 @@ import { formatUsd, type ScanCost } from "./cost.js"; export function redactedErrorMessage(error: unknown): string { const message = error instanceof Error ? error.message : String(error); const withoutPrivateKeys = message.replaceAll( - /(\b[A-Za-z0-9_-]{0,64}private[_-]?key(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*)(?:\\?["'])?-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z0-9 ]*PRIVATE KEY-----(?:\\?["'])?/giu, + /(\b[A-Za-z0-9_-]{0,64}private[_-]?key(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\?["'])?\s*[:=]\s*)(?:\\?["'])?-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z0-9 ]*PRIVATE KEY-----(?:\\?["'])?/giu, "$1[redacted]", ); return redactQuotedCredentialValues(withoutPrivateKeys) .replaceAll( - /(\b[A-Za-z0-9_-]{0,64}(?:authorization|auth)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*)([A-Za-z][A-Za-z0-9._~-]{0,63})((?:\s|%20|\+)+)(?!\[redacted\]|[A-Za-z_][A-Za-z0-9_-]{0,64}\s*[:=]\s*(?=[^=\s"',;}&\\\]]))[^\s"',;}&\\\]]+/giu, + /(\b[A-Za-z0-9_-]{0,64}(?:authorization|auth)(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\?["'])?\s*[:=]\s*)([A-Za-z][A-Za-z0-9._~-]{0,63})((?:\s|%20|\+)+)(?!\[redacted\]|[A-Za-z_][A-Za-z0-9_-]{0,64}\s*[:=]\s*(?=[^=\s"',;}&\\\]]))[^\s"',;}&\\\]]+/giu, "$1$2$3[redacted]", ) .replaceAll( - /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|authorization|auth|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)(?!\[redacted\]|[A-Za-z][A-Za-z0-9._~-]{0,63}(?:\s|%20|\+)+\[redacted\])[^\s"',;}&\\\]]+/giu, + /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|authorization|auth|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\?["'])?\s*[:=]\s*(?:\\?["'])?)(?!\[redacted\]|[A-Za-z][A-Za-z0-9._~-]{0,63}(?:\s|%20|\+)+\[redacted\])[^\s"',;}&\\\]]+/giu, "$1[redacted]", ) .replaceAll(/sk-(?:proj-)?[A-Za-z0-9_*=-]{8,}/gu, "[redacted]") @@ -25,14 +25,14 @@ export function redactedErrorMessage(error: unknown): string { ) .replaceAll(/((?:https?|ssh|git\+ssh):\/\/)[^\s/@]+@/giu, "$1[redacted]@") .replaceAll( - /((?:[?&]|%3F|%26)(?:(?!%3F|%26|%3D)(?:[A-Za-z0-9_.%-]|\[|\])){0,64}(?:api[_-]?key|access(?:[_-]|%5F|%2D)?key(?:(?:[_-]|%5F|%2D)?id)?|private(?:[_-]|%5F|%2D)?key|authorization|auth|token|secret|credential|signature|sig|password|passwd)(?:(?:[_-]|%5F|%2D)[A-Za-z0-9_.%-]{1,64})?(?:\]|%5D)?(?:=|%3D))(?:(?!%26)[^&\s])+/giu, + /((?:[?&]|%3F|%26)(?:(?!%3F|%26|%3D)(?:[A-Za-z0-9_.%-]|\[|\])){0,64}(?:api[_-]?key|access(?:[_-]|%5F|%2D)?key(?:(?:[_-]|%5F|%2D)?id)?|private(?:[_-]|%5F|%2D)?key|authorization|auth|token|secret|credential|signature|sig|password|passwd)(?:(?:[_-]|%5F|%2D)[A-Za-z0-9_.%-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_.%-]{0,48})?(?:\]|%5D)?(?:=|%3D))(?:(?!%26)[^&\s])+/giu, "$1[redacted]", ); } function redactQuotedCredentialValues(message: string): string { const assignment = - /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|authorization|auth|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64})?\b(?:\\*["'])?\s*[:=]\s*)(\\*)(["'])/giu; + /(\b[A-Za-z0-9_-]{0,64}(?:api[_-]?key|access[_-]?key(?:[_-]?id)?|private[_-]?key|authorization|auth|token|secret|credential|signature|sig|password|passwd)(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\*["'])?\s*[:=]\s*)(\\*)(["'])/giu; let output = ""; let consumed = 0; for ( diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index 89414421..0dbb6eee 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -2169,6 +2169,20 @@ describe("CLI", () => { '{"client_secret_value":"correct horse battery staple","safe":"visible"}', ), ).toBe('{"client_secret_value":"[redacted]","safe":"visible"}'); + expect( + redactedErrorMessage( + '{"clientSecretValue":"camel case secret","accessTokenValue":"camel case token"}', + ), + ).toBe( + '{"clientSecretValue":"[redacted]","accessTokenValue":"[redacted]"}', + ); + expect( + redactedErrorMessage( + "clientSecretValue=SYNTHETIC_CAMEL_SECRET accessTokenValue=SYNTHETIC_CAMEL_TOKEN https://example.test/?clientSecretValue=SYNTHETIC_CAMEL_QUERY", + ), + ).toBe( + "clientSecretValue=[redacted] accessTokenValue=[redacted] https://example.test/?clientSecretValue=[redacted]", + ); expect( redactedErrorMessage( '{\\"access_token_value\\":\\"another horse battery staple\\"}', diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index 7afda307..cfb87b0c 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -458,6 +458,7 @@ describe("multiscan", () => { const customAuthorization = "SYNTHETIC_CUSTOM_AUTHORIZATION_123"; const suffixedAuthorization = "SYNTHETIC_SUFFIXED_AUTHORIZATION_123"; const paddedAuthorization = "SYNTHETIC_PADDED_AUTHORIZATION_TOKEN=="; + const camelCaseSecret = "SYNTHETIC_CAMEL_CASE_CLIENT_SECRET_123"; await writeFile( paths.input, `id,repository,revision\nretry,${source.path},${source.revision}\n`, @@ -471,7 +472,7 @@ describe("multiscan", () => { attempts += 1; if (attempts === 1) { throw new Error( - `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} ${JSON.stringify({ client_secret_value: quotedSecret })} authorization="${opaqueAuthorization}" _auth=${npmAuthorization} Authorization: ApiKey ${customAuthorization} client_authorization_value=ApiKey ${suffixedAuthorization} auth=ApiKey ${paddedAuthorization} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, + `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} ${JSON.stringify({ client_secret_value: quotedSecret })} authorization="${opaqueAuthorization}" _auth=${npmAuthorization} Authorization: ApiKey ${customAuthorization} client_authorization_value=ApiKey ${suffixedAuthorization} auth=ApiKey ${paddedAuthorization} clientSecretValue=${camelCaseSecret} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, ); } return await completedScan(scanOptions.outputDir!); @@ -498,6 +499,7 @@ describe("multiscan", () => { expect(ledger).not.toContain(customAuthorization); expect(ledger).not.toContain(suffixedAuthorization); expect(ledger).not.toContain(paddedAuthorization); + expect(ledger).not.toContain(camelCaseSecret); expect(ledger).not.toContain(shortAuthorization); expect(ledger).toContain("https://[redacted]@proxy.test/v1/responses"); }); From ce61cfc407d58013ee272967ff54acb48f746bab Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 14:55:09 -0700 Subject: [PATCH 29/31] fix: settle terminated skill processes and portable credential imports --- sdk/typescript/src/cli.ts | 60 +++++++++++----------- sdk/typescript/src/runtime.ts | 13 ++++- sdk/typescript/tests-ts/cli-skills.test.ts | 32 ++++++++++++ sdk/typescript/tests-ts/runtime.test.ts | 30 +++++++++++ 4 files changed, 104 insertions(+), 31 deletions(-) diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index aa144d66..30dc6f4f 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -446,6 +446,7 @@ export async function runCodexSkillCommand( let forcedTermination: ReturnType | undefined; let forceStatusCompletion: (() => void) | null = null; let forceCaptureCompletion: (() => void) | null = null; + let invocationStatus: Promise | undefined; const requestTermination = (signal: SignalName): void => { requestedSignal = signal; invocation.kill(signal); @@ -486,35 +487,33 @@ export async function runCodexSkillCommand( forceCaptureCompletion = () => resolve(undefined); }), ]); - const [status, events] = await Promise.all([ - new Promise((resolve, reject) => { - let completed = false; - const complete = ( - code: number | null, - signal: NodeJS.Signals | null, - ): void => { - if (completed) return; - completed = true; - forceStatusCompletion = null; - resolve( - requestedSignal === "SIGINT" || signal === "SIGINT" - ? 130 - : requestedSignal === "SIGTERM" || signal === "SIGTERM" - ? 143 - : code ?? 1, - ); - }; - forceStatusCompletion = () => complete(null, null); - invocation.once("error", (error) => { - if (completed) return; - completed = true; - forceStatusCompletion = null; - reject(error); - }); - invocation.once(output === undefined ? "exit" : "close", complete); - }), - captured, - ]); + invocationStatus = new Promise((resolve, reject) => { + let completed = false; + const complete = ( + code: number | null, + signal: NodeJS.Signals | null, + ): void => { + if (completed) return; + completed = true; + forceStatusCompletion = null; + resolve( + requestedSignal === "SIGINT" || signal === "SIGINT" + ? 130 + : requestedSignal === "SIGTERM" || signal === "SIGTERM" + ? 143 + : code ?? 1, + ); + }; + forceStatusCompletion = () => complete(null, null); + invocation.once("error", (error) => { + if (completed) return; + completed = true; + forceStatusCompletion = null; + reject(error); + }); + invocation.once(output === undefined ? "exit" : "close", complete); + }); + const [status, events] = await Promise.all([invocationStatus, captured]); if (skillOutputLimitExceeded) { throw new CodexSecurityError(SKILL_OUTPUT_LIMIT_MESSAGE); } @@ -538,7 +537,8 @@ export async function runCodexSkillCommand( } catch (error) { invocation.stdout?.destroy(); invocation.stderr?.destroy(); - invocation.kill(); + requestTermination("SIGTERM"); + await invocationStatus?.catch(() => undefined); throw error; } finally { if (forcedTermination !== undefined) clearTimeout(forcedTermination); diff --git a/sdk/typescript/src/runtime.ts b/sdk/typescript/src/runtime.ts index 6855a22e..1111038a 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -998,7 +998,18 @@ export async function importAmbientAuth( await copyFile(source, temporary, constants.COPYFILE_EXCL); await chmod(temporary, 0o600); try { - await link(temporary, destination); + try { + await link(temporary, destination); + } catch (error) { + if ( + !["EPERM", "ENOTSUP", "EOPNOTSUPP", "EXDEV", "EMLINK"].includes( + nodeErrorCode(error) ?? "", + ) + ) { + throw error; + } + await copyFile(temporary, destination, constants.COPYFILE_EXCL); + } } catch (error) { if ( nodeErrorCode(error) === "EEXIST" && diff --git a/sdk/typescript/tests-ts/cli-skills.test.ts b/sdk/typescript/tests-ts/cli-skills.test.ts index 4703e8be..afae66f5 100644 --- a/sdk/typescript/tests-ts/cli-skills.test.ts +++ b/sdk/typescript/tests-ts/cli-skills.test.ts @@ -541,6 +541,38 @@ describe("CLI skill commands", () => { expect(stderr.text()).toBe(""); }); + test("terminates an oversized skill child after it closes its stdout", async () => { + const stdout = capture(); + const stderr = capture(); + const timeout = AbortSignal.timeout(2_500); + const invocation = runCodexSkillCommand( + [], + { command: "validate", stdout: stdout.stream, stderr: stderr.stream }, + { + command: process.execPath, + prefixArgs: [ + "-e", + 'process.on("SIGTERM",()=>{});process.stdout.write(JSON.stringify({type:"item.completed",item:{type:"agent_message",text:"x".repeat(256*1024+1)}})+"\\n");process.stdout.end();setInterval(()=>{},1000)', + ], + }, + ); + + await expect( + Promise.race([ + invocation, + new Promise((_, reject) => { + timeout.addEventListener( + "abort", + () => reject(new Error("Oversized skill process did not settle.")), + { once: true }, + ); + }), + ]), + ).rejects.toThrow("Codex skill output exceeded the 1 MiB event"); + expect(stdout.text()).toBe(""); + expect(stderr.text()).toBe(""); + }); + test("summarizes skill failures without echoing credentials or private paths", () => { const cases = [ ["401 sk-proj-SYNTHETIC_SECRET", "Authentication failed"], diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index 198280a8..362e42bf 100644 --- a/sdk/typescript/tests-ts/runtime.test.ts +++ b/sdk/typescript/tests-ts/runtime.test.ts @@ -830,6 +830,36 @@ describe("plugin runtime preparation", () => { } }); + test("imports ambient auth when credential files do not support hard links", async () => { + const root = await temporaryDirectory(); + const ambient = join(root, "ambient"); + const isolated = join(root, "isolated"); + await mkdir(ambient); + await writeFile(join(ambient, "auth.json"), '{"token":"portable"}\n'); + const originalLink = fsPromises.link; + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: async () => { + const error = new Error( + "hard links are unsupported", + ) as NodeJS.ErrnoException; + error.code = "ENOTSUP"; + throw error; + }, + })); + try { + expect(await importAmbientAuth(ambient, isolated)).toBe(true); + expect(await readFile(join(isolated, "auth.json"), "utf8")).toBe( + '{"token":"portable"}\n', + ); + } finally { + mock.module("node:fs/promises", () => ({ + ...fsPromises, + link: originalLink, + })); + } + }); + test("never replaces an explicitly stored sign-in with ambient credentials", async () => { const root = await temporaryDirectory(); const ambient = join(root, "ambient"); From af895a01162e4f56390f69165b914540c8d82500 Mon Sep 17 00:00:00 2001 From: Michael D'Angelo Date: Thu, 30 Jul 2026 15:08:48 -0700 Subject: [PATCH 30/31] fix: redact truncated private-key diagnostics completely --- sdk/typescript/src/errors.ts | 2 +- sdk/typescript/tests-ts/cli.test.ts | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/sdk/typescript/src/errors.ts b/sdk/typescript/src/errors.ts index 724d66df..3234cb5b 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -4,7 +4,7 @@ import { formatUsd, type ScanCost } from "./cost.js"; export function redactedErrorMessage(error: unknown): string { const message = error instanceof Error ? error.message : String(error); const withoutPrivateKeys = message.replaceAll( - /(\b[A-Za-z0-9_-]{0,64}private[_-]?key(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\?["'])?\s*[:=]\s*)(?:\\?["'])?-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]*?-----END [A-Z0-9 ]*PRIVATE KEY-----(?:\\?["'])?/giu, + /(\b[A-Za-z0-9_-]{0,64}private[_-]?key(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\?["'])?\s*[:=]\s*)(?:\\?["'])?-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]*?(?:-----END [A-Z0-9 ]*PRIVATE KEY-----(?:\\?["'])?|$)/giu, "$1[redacted]", ); return redactQuotedCredentialValues(withoutPrivateKeys) diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index 0dbb6eee..dd38af44 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -2242,6 +2242,11 @@ describe("CLI", () => { `private_key=-----BEGIN PRIVATE KEY-----${separator}MII_SYNTHETIC_PRIVATE_KEY${separator}-----END PRIVATE KEY----- safe=value`, ), ).toBe("private_key=[redacted] safe=value"); + expect( + redactedErrorMessage( + `private_key=-----BEGIN PRIVATE KEY-----${separator}MII_SYNTHETIC_TRUNCATED_PRIVATE_KEY`, + ), + ).toBe("private_key=[redacted]"); } }); From f26507de809416fc2dfcd3dd0e70d79e7088db82 Mon Sep 17 00:00:00 2001 From: Ian Webster Date: Thu, 30 Jul 2026 18:37:05 -0700 Subject: [PATCH 31/31] fix: redact key-value custom authorization credentials --- sdk/typescript/src/errors.ts | 2 +- sdk/typescript/tests-ts/cli.test.ts | 2 ++ sdk/typescript/tests-ts/multiscan.test.ts | 4 +++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/sdk/typescript/src/errors.ts b/sdk/typescript/src/errors.ts index 3234cb5b..a8face06 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -9,7 +9,7 @@ export function redactedErrorMessage(error: unknown): string { ); return redactQuotedCredentialValues(withoutPrivateKeys) .replaceAll( - /(\b[A-Za-z0-9_-]{0,64}(?:authorization|auth)(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\?["'])?\s*[:=]\s*)([A-Za-z][A-Za-z0-9._~-]{0,63})((?:\s|%20|\+)+)(?!\[redacted\]|[A-Za-z_][A-Za-z0-9_-]{0,64}\s*[:=]\s*(?=[^=\s"',;}&\\\]]))[^\s"',;}&\\\]]+/giu, + /(\b[A-Za-z0-9_-]{0,64}(?:authorization|auth)(?:[_-][A-Za-z0-9_-]{1,64}|(?:value|data|token|secret|credential|password|header|field|id|key)[A-Za-z0-9_-]{0,48})?\b(?:\\?["'])?\s*[:=]\s*)([A-Za-z][A-Za-z0-9._~-]{0,63})((?:\s|%20|\+)+)(?!\[redacted\]|(?!key\s*=)[A-Za-z_][A-Za-z0-9_-]{0,64}\s*[:=]\s*(?=[^=\s"',;}&\\\]]))[^\s"',;}&\\\]]+/giu, "$1$2$3[redacted]", ) .replaceAll( diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index dd38af44..d16b6dba 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -2211,6 +2211,8 @@ describe("CLI", () => { ], ["Authorization: ApiKey dGVzdA==", "Authorization: ApiKey [redacted]"], ["Authorization: ApiKey dGVzdA=", "Authorization: ApiKey [redacted]"], + ["Authorization: ApiKey key=SECRET", "Authorization: ApiKey [redacted]"], + ["auth=Custom key=SECRET", "auth=Custom [redacted]"], [ "client_auth_token=Custom dGVzdA==", "client_auth_token=Custom [redacted]", diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index cfb87b0c..f357fac9 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -458,6 +458,7 @@ describe("multiscan", () => { const customAuthorization = "SYNTHETIC_CUSTOM_AUTHORIZATION_123"; const suffixedAuthorization = "SYNTHETIC_SUFFIXED_AUTHORIZATION_123"; const paddedAuthorization = "SYNTHETIC_PADDED_AUTHORIZATION_TOKEN=="; + const keyedAuthorization = "SYNTHETIC_KEYED_AUTHORIZATION_SECRET_123"; const camelCaseSecret = "SYNTHETIC_CAMEL_CASE_CLIENT_SECRET_123"; await writeFile( paths.input, @@ -472,7 +473,7 @@ describe("multiscan", () => { attempts += 1; if (attempts === 1) { throw new Error( - `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} ${JSON.stringify({ client_secret_value: quotedSecret })} authorization="${opaqueAuthorization}" _auth=${npmAuthorization} Authorization: ApiKey ${customAuthorization} client_authorization_value=ApiKey ${suffixedAuthorization} auth=ApiKey ${paddedAuthorization} clientSecretValue=${camelCaseSecret} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, + `temporary failure ${secret} ${shortAuthorization} client_secret_value=${suffixedSecret} access_token_value=${suffixedToken} ${JSON.stringify({ client_secret_value: quotedSecret })} authorization="${opaqueAuthorization}" _auth=${npmAuthorization} Authorization: ApiKey ${customAuthorization} client_authorization_value=ApiKey ${suffixedAuthorization} auth=ApiKey ${paddedAuthorization} Authorization: Custom key=${keyedAuthorization} clientSecretValue=${camelCaseSecret} sending request for url (${proxyUrl}) and ${queryUrl}&client_secret_value=${suffixedQuery}`, ); } return await completedScan(scanOptions.outputDir!); @@ -499,6 +500,7 @@ describe("multiscan", () => { expect(ledger).not.toContain(customAuthorization); expect(ledger).not.toContain(suffixedAuthorization); expect(ledger).not.toContain(paddedAuthorization); + expect(ledger).not.toContain(keyedAuthorization); expect(ledger).not.toContain(camelCaseSecret); expect(ledger).not.toContain(shortAuthorization); expect(ledger).toContain("https://[redacted]@proxy.test/v1/responses");