diff --git a/sdk/typescript/README.md b/sdk/typescript/README.md index 96132508..21f02cec 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/api.ts b/sdk/typescript/src/api.ts index d2381d3d..32269c06 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"; @@ -311,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; @@ -397,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 ( @@ -533,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({ @@ -562,6 +570,7 @@ export class CodexSecurity { costTracker = tracker; const recipe = scanRecipe( repo, + protectedRoot, normalized, mode, expectation.repositoryRevision, @@ -832,6 +841,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 @@ -843,11 +855,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)] : []), @@ -860,13 +871,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 +1354,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 { @@ -1395,12 +1452,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" @@ -1556,6 +1609,7 @@ function targetInstruction(target: NormalizedTarget): string { function scanRecipe( repository: string, + activeProjectPath: string, target: NormalizedTarget, mode: ScanMode, repositoryRevision: string | null, @@ -1578,7 +1632,7 @@ function scanRecipe( mode, ...(repositoryRevision === null ? {} : { repositoryRevision }), pluginVersion, - config: scanPreflightCodexConfig(effectiveConfig), + config: scanPreflightCodexConfig(effectiveConfig, activeProjectPath), ...(failOnSeverity === undefined ? {} : { failOnSeverity }), ...(knowledgeBasePaths === undefined ? {} : { knowledgeBasePaths }), ...(maxCostUsd === undefined ? {} : { maxCostUsd }), @@ -1758,6 +1812,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, ): @@ -1833,7 +1902,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 && @@ -1903,18 +1975,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; } @@ -1927,13 +2022,34 @@ 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; + 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, + activeProjectRoot ?? 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/src/auth.ts b/sdk/typescript/src/auth.ts index 83cd69fe..c8e75547 100644 --- a/sdk/typescript/src/auth.ts +++ b/sdk/typescript/src/auth.ts @@ -3,6 +3,15 @@ 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; +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; +type TerminalEscapeState = "text" | "escape" | "csi" | "osc" | "osc-escape"; + export interface LoginResult { success: boolean; exitCode: number | null; @@ -27,8 +36,19 @@ export class CodexLoginHandle { #urlReadySettled = false; #deviceReadySettled = false; #canceled = false; + #forcedTermination: ReturnType | undefined; + #forceCompletion: (() => void) | null = null; #stdout = ""; #stderr = ""; + #stdoutInstructionTail = ""; + #stderrInstructionTail = ""; + #stdoutTerminalState: TerminalEscapeState = "text"; + #stderrTerminalState: TerminalEscapeState = "text"; + #stdoutAuthUrl: string | null = null; + #stderrAuthUrl: string | null = null; + #stdoutUserCode: string | null = null; + #stderrUserCode: string | null = null; + #outputLimitExceeded = false; public constructor( command: CodexCommand, @@ -55,47 +75,63 @@ 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) => { - 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); - const result = { - success: exitCode === 0 && !this.#canceled, - exitCode, - stdout: this.#stdout, - stderr: this.#stderr, - }; + this.#clearForcedTermination(); + this.#forceCompletion = null; + this.#destroyPipes(); + if (!this.#outputLimitExceeded && !this.#canceled) { + this.#flushInstructionTails(); + } + 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); }; + 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); }); }); } @@ -105,7 +141,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 { @@ -113,12 +149,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 { @@ -132,10 +163,107 @@ export class CodexLoginHandle { } public cancel(): void { - if (this.#child.exitCode === null) { - this.#canceled = true; - this.#child.kill("SIGTERM"); + this.#canceled = true; + this.#requestTermination(); + } + + #requestTermination(): void { + 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(); + } + + #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.#requestTermination(); + return; + } + + const tail = + stream === "stdout" + ? this.#stdoutInstructionTail + : this.#stderrInstructionTail; + const visible = visibleTerminalChunk( + chunk, + stream === "stdout" + ? this.#stdoutTerminalState + : this.#stderrTerminalState, + ); + const candidate = `${tail}${visible.text}`; + 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.slice(lastDelimiter + 1), + INSTRUCTION_TAIL_BYTES, + ).value; + if (stream === "stdout") { + 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(); + } + + #flushInstructionTails(): void { + this.#stdoutAuthUrl ??= preferredAuthUrl(this.#stdoutInstructionTail); + this.#stdoutUserCode ??= userCodeFromOutput(this.#stdoutInstructionTail); + this.#stderrAuthUrl ??= preferredAuthUrl(this.#stderrInstructionTail); + this.#stderrUserCode ??= userCodeFromOutput(this.#stderrInstructionTail); } #notifyInstructions(): void { @@ -256,18 +384,54 @@ 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) { + child.stdin.destroy(); + child.stdout.destroy(); + child.stderr.destroy(); + 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( + 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); + terminate(); + }; 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; + terminate(); }); child.stdin.on("error", (error: NodeJS.ErrnoException) => { // A short-lived command can close stdin before Node flushes the input. @@ -283,6 +447,7 @@ export async function runCodex( } }); child.once("close", (exitCode) => { + if (forcedTermination !== undefined) clearTimeout(forcedTermination); if (processError !== null) { reject(processError); } else { @@ -295,29 +460,63 @@ 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 { @@ -326,3 +525,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 { + text += character === "\r" ? "\n" : 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 1abe8302..30dc6f4f 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"; @@ -56,7 +55,12 @@ import { import { formatUsd } from "./cost.js"; import { CodexSecurityError, + ConfigurationError, + InvalidTargetError, + OutputDirectoryError, OutputInsideProtectedRootError, + PluginPythonUnavailableError, + redactedErrorMessage, ScanInterruptedError, } from "./errors.js"; import type { SeverityLevel } from "./models.js"; @@ -101,6 +105,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; @@ -108,6 +116,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; @@ -433,13 +442,31 @@ 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; + let invocationStatus: Promise | undefined; + 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); @@ -451,25 +478,45 @@ export async function runCodexSkillCommand( const captured = output === undefined || invocation.stdout === null ? Promise.resolve(undefined) - : readSkillCommandOutput(invocation.stdout); - 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, - ); - }, + : Promise.race([ + readSkillCommandOutput(invocation.stdout, () => { + skillOutputLimitExceeded = true; + requestTermination("SIGTERM"); + }), + new Promise((resolve) => { + forceCaptureCompletion = () => resolve(undefined); + }), + ]); + 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, ); - }), - captured, - ]); + }; + 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); + } if (output === undefined || status === 130 || status === 143) return status; if (status !== 0) { await writeCliOutput( @@ -490,9 +537,13 @@ 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); + forceStatusCompletion = null; + forceCaptureCompletion = null; process.off("SIGINT", onInterrupt); process.off("SIGTERM", onTerminate); } @@ -606,7 +657,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 +856,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 +919,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 +1197,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 +1358,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 +1370,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 +1474,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 +1510,7 @@ export async function main( ); } catch (error) { exitCode = 2; - errorOutput.write(`codex-security: ${cliErrorMessage(error)}\n`); + errorOutput.write(`codex-security: ${redactedErrorMessage(error)}\n`); } }, }) @@ -1490,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, @@ -1566,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, @@ -1644,7 +1699,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 +1708,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; } } @@ -2191,23 +2246,33 @@ 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?.(); + }; - 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") { @@ -2220,7 +2285,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) { + markExceeded(); + } else { + message = item.text; + } } } else if (value["type"] === "turn.failed") { const detail = value["error"]; @@ -2230,15 +2299,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 + ) { + markExceeded(); + } 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 + ) { + markExceeded(); + } 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) { + markExceeded(); + 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 }), @@ -2369,7 +2486,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 +2634,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 +2695,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 +2737,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 +2745,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 }; @@ -2674,10 +2791,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 redactedErrorMessage(error); switch (classifyConnectionFailure(error)) { case "unauthorized": return authentication?.method === "api_key" @@ -2697,7 +2865,7 @@ function scanFailureMessage( case "network_error": case "timeout": case "unknown": - return cliErrorMessage(error); + return redactedErrorMessage(error); } } @@ -2711,7 +2879,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 +2943,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 +2959,7 @@ function printScanSummary( ); } errorOutput.write( - ` ${paint("RESULTS", 1)} ${cliErrorMessage(result.scanDir)}\n`, + ` ${paint("RESULTS", 1)} ${redactedErrorMessage(result.scanDir)}\n`, ); } @@ -3089,7 +3259,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 +3279,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/cost.ts b/sdk/typescript/src/cost.ts index 912ccd27..321b9751 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; @@ -108,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"), )) { @@ -115,14 +119,21 @@ export class ScanCostTracker { if (session === undefined) { session = { offset: 0, - remainder: Buffer.alloc(0), + pendingLine: [], + pendingLineBytes: 0, + unreadable: false, threadId: null, parentThreadId: null, usage: null, }; this.#sessions.set(path, session); } - await readSessionUsage(path, session); + try { + await readSessionUsage(path, session); + } catch (error) { + if (session.threadId === null) throw error; + unreadable.push({ session, error }); + } } const included = new Set([this.#threadId]); @@ -141,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()) { @@ -187,6 +201,7 @@ async function readSessionUsage( path: string, session: SessionUsage, ): Promise { + if (session.unreadable) return; let file; try { file = await open(path, "r"); @@ -205,27 +220,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/src/errors.ts b/sdk/typescript/src/errors.ts index 10be04ca..a8face06 100644 --- a/sdk/typescript/src/errors.ts +++ b/sdk/typescript/src/errors.ts @@ -1,5 +1,74 @@ 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); + 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, + "$1[redacted]", + ); + 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\]|(?!key\s*=)[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}|(?: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]") + .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)?|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}|(?: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 ( + let match = assignment.exec(message); + match !== null; + match = assignment.exec(message) + ) { + 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; + 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; + closed = true; + break; + } + position = delimiter + 1; + } + if (!closed) { + output += `${message.slice(consumed, assignment.lastIndex)}[redacted]`; + consumed = message.length; + break; + } + } + 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/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/src/runtime.ts b/sdk/typescript/src/runtime.ts index a608ddbf..4a09a894 100644 --- a/sdk/typescript/src/runtime.ts +++ b/sdk/typescript/src/runtime.ts @@ -4,10 +4,12 @@ import { constants, existsSync, type Stats } from "node:fs"; import { chmod, copyFile, + link, lstat, mkdir, mkdtemp, open, + opendir, readFile, readdir, realpath, @@ -153,7 +155,9 @@ export async function prepareCodexSecurityCredentialHome( const canonical = await realpath(path); requireModelSafeOutputDir(canonical); validateLocation?.(canonical); - await requirePrivateCredentialHome(metadata, canonical); + await requireSecureCredentialHome(canonical, { + metadata, + }); return canonical; } catch (error) { if (error instanceof OutputDirectoryError) throw error; @@ -164,6 +168,83 @@ 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; + metadata?: Stats; + expectedDevice?: number; + expectedInode?: number; + validateWindowsAcl?: boolean; + } = {}, +): 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); + 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 && + 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.validateWindowsAcl !== false) { + 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, @@ -231,13 +312,43 @@ 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, + securityOptions, + ); + 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, { + ...securityOptions, + expectedDevice, + 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) { @@ -261,6 +372,11 @@ export async function acquireCodexSecurityCredentialHomeLock( let released = false; return async () => { if (released) return; + await requireSecureCredentialHome(codexHome, { + ...securityOptions, + expectedDevice, + expectedInode, + }); const owner = JSON.parse(await readFile(ownerPath, "utf8")) as { token?: unknown; }; @@ -334,6 +450,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 }); @@ -362,6 +479,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()) { @@ -379,6 +497,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 { @@ -392,9 +511,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, @@ -853,6 +991,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`); @@ -860,7 +1001,18 @@ export async function importAmbientAuth( await copyFile(source, temporary, constants.COPYFILE_EXCL); await chmod(temporary, 0o600); try { - await copyFile(temporary, destination, constants.COPYFILE_EXCL); + 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" && @@ -1183,24 +1335,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("/")); @@ -1736,7 +1889,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 { @@ -1745,18 +1898,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( @@ -1765,12 +1927,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()) { @@ -1842,6 +1998,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/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<{ diff --git a/sdk/typescript/tests-ts/api.test.ts b/sdk/typescript/tests-ts/api.test.ts index 13e73152..0ce7f5be 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< @@ -495,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, + }, + join(activeProject, "packages", "service"), + ); + + 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)), @@ -1030,6 +1118,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"); @@ -2164,6 +2305,87 @@ 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 quotedCredential = JSON.stringify({ + client_secret_value: "SYNTHETIC correct horse battery staple", + }); + const redactedFailure = `${REDACTED_CREDENTIALS} {"client_secret_value":"[redacted]"}`; + 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} ${quotedCredential}`, + }; + } + 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(redactedFailure); + + // `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: redactedFailure, + }); + + // 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"); @@ -2268,11 +2490,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", @@ -2331,6 +2563,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; @@ -3257,11 +3494,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); @@ -3272,11 +3511,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; @@ -3298,7 +3538,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; @@ -3331,7 +3571,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[] = []; @@ -3846,14 +4086,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( {}, @@ -3884,7 +4124,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 }); }); @@ -3901,7 +4153,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") { @@ -3913,7 +4165,8 @@ 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; } diff --git a/sdk/typescript/tests-ts/auth.test.ts b/sdk/typescript/tests-ts/auth.test.ts index 5e95da2f..08a2e434 100644 --- a/sdk/typescript/tests-ts/auth.test.ts +++ b/sdk/typescript/tests-ts/auth.test.ts @@ -100,6 +100,64 @@ 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("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({ @@ -127,6 +185,190 @@ 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("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("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("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-"), + ); + 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); @@ -236,15 +478,13 @@ grandchild.once("error", (error) => { }, ); - test.skipIf(process.platform !== "win32")( - "releases native login pipes when the Windows 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]; @@ -258,9 +498,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"; @@ -292,103 +532,137 @@ 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 Windows 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 Windows 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-")); + 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-")); diff --git a/sdk/typescript/tests-ts/cli-authentication.test.ts b/sdk/typescript/tests-ts/cli-authentication.test.ts index 40f12a27..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 = [ @@ -55,7 +70,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/cli-skills.test.ts b/sdk/typescript/tests-ts/cli-skills.test.ts index bb7f5c27..afae66f5 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, @@ -455,6 +457,122 @@ 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("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("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"], @@ -527,4 +645,95 @@ describe("CLI skill commands", () => { expect(stderr.text()).not.toContain("/private"); } }); + + 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( + 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 }); + } + }, + ); }); diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index 84b7d7c2..d16b6dba 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -21,11 +21,15 @@ import type { JsonObject, ScanPreflight, } from "../src/index.js"; +import { redactedErrorMessage } from "../src/errors.js"; import { BUNDLED_PLUGIN_VERSION, CodexSecurityError, DiffTarget, + InvalidTargetError, + OutputDirectoryError, OutputInsideProtectedRootError, + PluginPythonUnavailableError, ScanCostLimitExceededError, ScanInterruptedError, VERSION, @@ -2035,6 +2039,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(); @@ -2060,6 +2158,100 @@ 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]'); + expect( + redactedErrorMessage( + '{"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\\"}', + ), + ).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]', + ); + 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]", + ], + [ + "client_authorization_value=ApiKey SYNTHETIC_SUFFIXED_SECRET", + "client_authorization_value=ApiKey [redacted]", + ], + ["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]", + ], + ] 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', + }; + 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( + `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]"); + } + }); + 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 5e7d588b..16d6fc5b 100644 --- a/sdk/typescript/tests-ts/cost.test.ts +++ b/sdk/typescript/tests-ts/cost.test.ts @@ -226,6 +226,114 @@ 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("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("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", { diff --git a/sdk/typescript/tests-ts/multiscan.test.ts b/sdk/typescript/tests-ts/multiscan.test.ts index e739ed74..f357fac9 100644 --- a/sdk/typescript/tests-ts/multiscan.test.ts +++ b/sdk/typescript/tests-ts/multiscan.test.ts @@ -444,6 +444,22 @@ 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"; + 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"; + 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"; + 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, `id,repository,revision\nretry,${source.path},${source.revision}\n`, @@ -455,7 +471,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} 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!); }), ), @@ -467,7 +487,23 @@ 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(suffixedSecret); + 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(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"); }); test("resumes complete bundles, repairs missing output, and rejects manifest drift", async () => { diff --git a/sdk/typescript/tests-ts/runtime.test.ts b/sdk/typescript/tests-ts/runtime.test.ts index 613d8831..3d020c6f 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,9 @@ import { prepareCodexSecurityCredentialHome, preparePersistentScanRoot, requirePrivateCredentialHome, + requirePrivateCredentialFile, requirePrivateOutputDirectory, + requireSecureCredentialHome, requireSecureOutputAncestry, requireTrustedOutputAncestor, runWorkbench, @@ -387,6 +391,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 () => { @@ -740,14 +830,50 @@ 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"); 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( @@ -1228,6 +1354,165 @@ 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 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 () => { + 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"); @@ -1284,6 +1569,43 @@ 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) => { + const lock = join(path, ".codex-security-scan.lock"); + expect(existsSync(lock) && !existsSync(join(lock, "owner.json"))).toBe( + false, + ); + 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({ @@ -1352,6 +1674,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 () => {