fix: consolidate contributor runtime-security hardening (#152)

* Redact persisted scan failure credentials

* fix: bound login and skill child shutdown

* fix: bound authentication and skill output

* fix: bound configured plugin traversal

* fix: bound cost session events

* fix: fail the scan on every turn.failed payload

A failed turn was only honored when its error payload was an object with a
string message. Every other shape fell through the event loop, and because a
turn.completed had already been observed the scan was returned to the caller
as successful and recorded with complete-scan.

Treat any turn.failed as terminal. Only error.message is reused, because that
is the single shape the previous code already surfaced; no other shape is
forwarded or stringified, since this message reaches fail-scan --message and
is stored in scans.failure_message without redaction.

* fix: keep scan cleanup failures from replacing the scan result

The scan finally block awaited knowledge-base cleanup, target-paths removal, and
the credential home release without guarding any of them. A rejection in a
finally block replaces the pending exception, so a cleanup failure discarded the
outcome the try and catch blocks had already produced and the caller was handed
an unrelated filesystem error instead of the reason the scan failed. The catch
block had already recorded the real message through fail-scan, so the persisted
scan history and the reported error disagreed.

Removing the temporary inputs is best effort and is now reported through
onWarning instead of deciding the result. Releasing the credential home lock is
not: it only marks itself done once the lock directory is gone, so a failed
release leaves an owner.json naming a live process that stale-lock recovery will
not reclaim. That failure still propagates, and is downgraded to a warning only
when the scan already failed and its error is the one worth keeping.

The failed-scan marker is recorded on entry to the catch block because
requireOpen and throwIfAborted raise a different error for the same failed scan,
the release keeps its own finally so reporting the earlier failures cannot skip
it, and the warning formatter is total so no hostile reason can throw in place of
the result.

* fix: prioritize active preflight config

* fix: make bounded authentication shutdown deterministic

Co-authored-by: mangeshraut712 <mbr63@drexel.edu>

* test: restrict POSIX process signal propagation to supported platforms

* fix: close runtime credential and subprocess review gaps

* 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

* fix: retain credential redaction for local scan failures

* fix: harden credential redaction and authentication boundaries

* fix: exclude unrelated delegated cost sessions safely

* fix: keep credential homes private under trusted ancestry

Reject non-sticky or untrusted parents for durable auth state, re-check
on every credential-home use path, harden auth.json, and pin identity
for the lock session so shared-volume rename races cannot steal auth.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix: preserve safe credential login and shared ancestry contracts

* test: use private credential homes for authentication fixtures

* fix: redact PEM credentials and revalidate Windows home ACLs

* test: synchronously flush synthetic ChatGPT login prompts

* fix: atomically publish ambient auth credentials

* fix: redact JSON-quoted credentials before persistent failures

* fix: redact recursively escaped credential strings

* fix: close credential redaction and home identity races

* fix: seal credential locking and redact custom authorization schemes

* fix: bound login instructions and redact incomplete credentials

* fix: preserve padded authorization credentials during redaction

* fix: redact camel-case credential fields before persistence

* fix: settle terminated skill processes and portable credential imports

* fix: redact truncated private-key diagnostics completely

* fix: redact key-value custom authorization credentials

---------

Co-authored-by: Matthew Bright <matthew.bright@hotmail.com>
Co-authored-by: GautamSharma99 <gautamsharma99067@gmail.com>
Co-authored-by: Hai Duong <haiduong.nguyen2712@gmail.com>
Co-authored-by: mangeshraut712 <mbr63@drexel.edu>
Co-authored-by: mldangelo <michael.l.dangelo@gmail.com>
Co-authored-by: batmanknows <122247678+batmnnn@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Ian Webster <ianw@openai.com>

Author: 9 people

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.

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<void>) | 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<ScanOptions, "onWarning" | "onObserverError">,
+  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<void> {
   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<string, unknown>,
+    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;
   }