From ec01cfe9043d5306739fa55c7630ffd36b948e16 Mon Sep 17 00:00:00 2001 From: mldangelo Date: Thu, 30 Jul 2026 10:34:21 -0700 Subject: [PATCH] fix: report local scan failures without connectivity advice classifyConnectionFailure matches bare words anywhere in a message, and the unauthorized, forbidden and rate-limited branches of scanFailureMessage replace the message entirely with fixed advice. A local failure whose text merely contained one of those words was therefore reported as a credential problem, and its own text reached neither stderr nor the JSON error field. The reported trigger is "permission denied", the standard strerror for EACCES: a read-only TMPDIR surfaced as "The stored ChatGPT credentials cannot access the configured model." Skip classification for failures that cannot originate from the model transport -- InvalidTargetError, OutputDirectoryError, ConfigurationError, PluginPythonUnavailableError, and filesystem syscall errno codes -- so they keep their own message. Network errno codes are deliberately excluded from that set because they are genuine transport failures. Advice still replaces the underlying text for real transport failures. Upstream authentication and authorization errors can name the organization or project, and cli-authentication.test.ts pins that they must not reach stderr. Fixes #36 --- sdk/typescript/src/cli.ts | 55 ++++++++++++++++ sdk/typescript/tests-ts/cli.test.ts | 97 +++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+) diff --git a/sdk/typescript/src/cli.ts b/sdk/typescript/src/cli.ts index 1abe8302..c29da603 100644 --- a/sdk/typescript/src/cli.ts +++ b/sdk/typescript/src/cli.ts @@ -56,7 +56,11 @@ import { import { formatUsd } from "./cost.js"; import { CodexSecurityError, + ConfigurationError, + InvalidTargetError, + OutputDirectoryError, OutputInsideProtectedRootError, + PluginPythonUnavailableError, ScanInterruptedError, } from "./errors.js"; import type { SeverityLevel } from "./models.js"; @@ -2674,10 +2678,61 @@ async function runScan( return { exitCode: blockingCount > 0 ? 1 : 0, data: result.toJSON() }; } +// Filesystem and OS syscall failures cannot originate from the model transport, +// so they must never be rewritten as connectivity or credential advice. Network +// errno codes are deliberately absent: they are genuine transport failures. +const LOCAL_SYSCALL_CODES = new Set([ + "EACCES", + "EBUSY", + "EEXIST", + "EFBIG", + "EIO", + "EISDIR", + "ELOOP", + "EMFILE", + "ENAMETOOLONG", + "ENFILE", + "ENOENT", + "ENOMEM", + "ENOSPC", + "ENOTDIR", + "ENOTEMPTY", + "EPERM", + "EROFS", + "EXDEV", +]); + +function isLocalScanFailure(error: unknown): boolean { + if ( + error instanceof InvalidTargetError || + error instanceof OutputDirectoryError || + error instanceof ConfigurationError || + error instanceof PluginPythonUnavailableError + ) { + return true; + } + return ( + typeof error === "object" && + error !== null && + "code" in error && + typeof (error as { code: unknown }).code === "string" && + LOCAL_SYSCALL_CODES.has((error as { code: string }).code) + ); +} + function scanFailureMessage( error: unknown, authentication: ScanAuthentication | null, ): string { + // A local failure keeps its own message. Classification matches bare words + // such as "permission denied" anywhere in the text, so an EACCES from a + // read-only TMPDIR would otherwise be reported as a credential problem. + // + // The advice branches below still replace the underlying text rather than + // appending it. That is deliberate: upstream authentication and authorization + // errors can name the organization or project, which must not reach stderr or + // the JSON error field. + if (isLocalScanFailure(error)) return cliErrorMessage(error); switch (classifyConnectionFailure(error)) { case "unauthorized": return authentication?.method === "api_key" diff --git a/sdk/typescript/tests-ts/cli.test.ts b/sdk/typescript/tests-ts/cli.test.ts index 84b7d7c2..6fc37032 100644 --- a/sdk/typescript/tests-ts/cli.test.ts +++ b/sdk/typescript/tests-ts/cli.test.ts @@ -25,7 +25,10 @@ import { BUNDLED_PLUGIN_VERSION, CodexSecurityError, DiffTarget, + InvalidTargetError, + OutputDirectoryError, OutputInsideProtectedRootError, + PluginPythonUnavailableError, ScanCostLimitExceededError, ScanInterruptedError, VERSION, @@ -2035,6 +2038,100 @@ describe("CLI", () => { } }); + test("reports local input and filesystem failures without connectivity advice", async () => { + // https://github.com/openai/codex-security/issues/36 -- classification + // matches bare words such as "permission denied" anywhere in the message, + // so local failures were reported as credential or connectivity problems + // and their own text was discarded. + const failures: Array<[string, unknown]> = [ + [ + "EACCES from a read-only TMPDIR", + Object.assign( + new Error( + "EACCES: permission denied, mkdtemp '/tmp/openai-codex-security-home-XXXXXX'", + ), + { code: "EACCES" }, + ), + ], + [ + "EPERM writing the scan directory", + Object.assign( + new Error("EPERM: operation not permitted, mkdir '/out/scan'"), + { code: "EPERM" }, + ), + ], + [ + "output directory rejected", + new OutputDirectoryError( + "Scan output directory must not be accessible to other users (chmod 700): /out", + ), + ], + [ + "path target naming a 403 directory", + new InvalidTargetError("Path target does not exist: src/403/client.ts"), + ], + [ + "git ref naming a forbidden branch", + new InvalidTargetError("unknown Git ref: origin/forbidden-paths"), + ], + [ + "python interpreter unavailable", + new PluginPythonUnavailableError( + "The configured plugin Python interpreter is unavailable or unusable: /usr/bin/python3", + ), + ], + ]; + + for (const [, failure] of failures) { + const stdout = capture(); + const stderr = capture(); + const deps = dependencies(); + deps.createSecurity = () => ({ + run: async () => { + throw failure; + }, + preflight: async () => fakePreflight(), + close: async () => {}, + }); + + expect( + await main(["scan", "."], stdout.stream, stderr.stream, deps), + ).toBe(2); + expect(stderr.text()).toContain((failure as Error).message); + expect(stderr.text()).not.toContain("cannot access the configured model"); + expect(stderr.text()).not.toContain("Authentication failed"); + expect(stderr.text()).not.toContain("reached its rate limit"); + } + }); + + test("keeps model authorization advice for genuine transport failures", async () => { + // The bypass must not swallow real 401/403 handling, and the advice must + // still replace upstream text that can name the organization or project. + for (const [detail, expected] of [ + ["401 invalid API key for org-private", "Authentication failed"], + [ + "403 model access denied for org-private", + "cannot access the configured model", + ], + ] as const) { + const stderr = capture(); + const deps = dependencies(); + deps.createSecurity = () => ({ + run: async () => { + throw new CodexSecurityError(detail); + }, + preflight: async () => fakePreflight(), + close: async () => {}, + }); + + expect( + await main(["scan", "."], capture().stream, stderr.stream, deps), + ).toBe(2); + expect(stderr.text()).toContain(expected); + expect(stderr.text()).not.toContain("org-private"); + } + }); + test("redacts credentials in underlying network errors", async () => { const stdout = capture(); const stderr = capture();