fix: consolidate contributor runtime-security hardening

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;
   }