fix: preserve authoritative security scan targets (#103)

Author: ianw-oai

sdk/typescript/_bundled_plugin/references/scan-contract.md

@@ -31,6 +31,8 @@ A sealed manifest records the completed timestamp and hashes for the canonical d
 Choose the target kind based on the reviewed content, not the scan invocation:
 `git_worktree` for a checked-out Git workspace, `directory_snapshot` for a non-Git directory, `git_diff` for a Git-backed change set, and `git_revision` for an exact immutable Git tree.
 
+For a workbench-backed scan, use the recorded target contract instead of inferring the kind from the checkout. A clean Git checkout has `allowedKinds: ["git_revision"]`: use its recorded revision and omit `snapshotDigest`. A dirty checkout has `allowedKinds: ["git_worktree"]`: copy `requiredSnapshotDigest` exactly.
+
 | Kind | Required snapshot fields |
 | --- | --- |
 | `git_revision` | `revision` |

sdk/typescript/_bundled_plugin/scripts/workbench_cli.py

@@ -210,6 +210,10 @@ def parse_args(description: str) -> argparse.Namespace:
     update_progress.add_argument("--deep-review-pass", type=positive_int)
     update_progress.add_argument("--claim-token")
 
+    prepare_scan_completion = subparsers.add_parser("prepare-scan-completion")
+    prepare_scan_completion.add_argument("--scan-id", required=True)
+    prepare_scan_completion.add_argument("--claim-token")
+
     complete_scan = subparsers.add_parser("complete-scan")
     complete_scan.add_argument("--scan-id", required=True)
     complete_scan.add_argument("--claim-token")

sdk/typescript/_bundled_plugin/scripts/workbench_db.py

@@ -86,6 +86,7 @@
     safe_segment,
     scan_diff_identity,
     scan_target_identity,
+    stored_diff_target,
 )
 from workbench_schema import MIGRATIONS, normalize_pre_release_migrations, sql_statements
 from workbench_source_excerpt import finding_source_excerpt
@@ -504,19 +505,6 @@ def expected_target_kinds(scan: sqlite3.Row) -> list[str]:
     return ["git_worktree"]
 
 
-def stored_diff_target(row: sqlite3.Row) -> dict[str, str] | None:
-    if not row["diff_target_kind"]:
-        return None
-    target = {
-        "baseRevision": row["diff_base_revision"],
-        "headRevision": row["diff_head_revision"],
-        "kind": row["diff_target_kind"],
-    }
-    if row["diff_content_digest"]:
-        target["contentDigest"] = row["diff_content_digest"]
-    return target
-
-
 def requested_scan_paths(scan: sqlite3.Row) -> list[str]:
     if "recipe_json" in scan.keys() and scan["recipe_json"] is not None:
         recipe = json.loads(scan["recipe_json"], parse_constant=reject_non_finite_json)
@@ -1374,18 +1362,24 @@ def pin_legacy_manifest_digest(
         raise
 
 
-def complete_scan(connection: sqlite3.Connection, args: argparse.Namespace) -> dict[str, Any]:
+def complete_scan(
+    connection: sqlite3.Connection, args: argparse.Namespace, *, prepare_only: bool = False
+) -> dict[str, Any]:
     scan_id = require_uuid(args.scan_id, "scan-id")
-    cost_json = parse_scan_cost(args.cost_json)
+    cost_json = None if prepare_only else parse_scan_cost(args.cost_json)
     with scan_completion_lock(scan_id):
-        return complete_scan_locked(connection, scan_id, args.claim_token, cost_json)
+        return complete_scan_locked(
+            connection, scan_id, args.claim_token, cost_json, prepare_only=prepare_only
+        )
 
 
 def complete_scan_locked(
     connection: sqlite3.Connection,
     scan_id: str,
     claim_token: str | None,
     cost_json: str | None,
+    *,
+    prepare_only: bool = False,
 ) -> dict[str, Any]:
     scan = require_scan(connection, scan_id)
     if scan["status"] == "complete":
@@ -1412,9 +1406,9 @@ def complete_scan_locked(
     )
     if scan["recipe_json"] is None:
         deep_scan.require_deep_scan_ready_for_parent_completion(connection, scan)
-    warnings = []
+    warnings = json.loads(scan["completion_warnings_json"])
     warning = scan_target_warning(scan)
-    if warning is not None:
+    if warning is not None and warning not in warnings:
         warnings.append(warning)
     scan_dir = require_canonical_scan_directory(Path(scan["scan_dir"]))
     completion_timestamp = now()
@@ -1443,9 +1437,23 @@ def complete_scan_locked(
         for kind, filename in ARTIFACTS.items()
     }
     manifest_digest = published_manifest_digest(scan_dir, manifest)
+    if prepare_only:
+        connection.execute("BEGIN IMMEDIATE")
+        try:
+            updated = connection.execute(
+                "UPDATE scans SET completion_warnings_json = ? WHERE id = ? AND status = 'running'",
+                (json.dumps(warnings), scan["id"]),
+            )
+            if updated.rowcount != 1:
+                raise SystemExit("Only a running scan can be prepared for completion.")
+            connection.commit()
+        except BaseException:
+            connection.rollback()
+            raise
+        return scan_context(connection, scan["id"])
     connection.execute("BEGIN IMMEDIATE")
     try:
-        timestamp = completion_timestamp
+        timestamp = manifest["scan"]["completedAt"]
         scan = require_scan(connection, scan["id"])
         if scan["status"] == "complete":
             connection.commit()
@@ -1609,7 +1617,14 @@ def register_cli_scan(connection: sqlite3.Connection, args: argparse.Namespace)
     except BaseException:
         connection.rollback()
         raise
-    return {"scanDir": str(scan_dir), "scanId": scan_id, "targetId": target_id}
+    scan = require_scan(connection, scan_id)
+    return {
+        "contract": scan_contract(scan),
+        "scanDir": str(scan_dir),
+        "scanId": scan_id,
+        "targetId": target_id,
+        "targetRevision": scan["target_revision"],
+    }
 
 
 def parse_scan_recipe(value: str, repository: Path) -> dict[str, Any]:
@@ -3596,8 +3611,10 @@ def main() -> None:
                 require_scan=require_scan,
                 scan_context=scan_context,
             )
-        elif args.command == "complete-scan":
-            result = complete_scan(connection, args)
+        elif args.command in {"prepare-scan-completion", "complete-scan"}:
+            result = complete_scan(
+                connection, args, prepare_only=args.command == "prepare-scan-completion"
+            )
         elif args.command == "cancel-scan":
             result = cancel_scan(connection, args)
         elif args.command == "fail-scan":

sdk/typescript/_bundled_plugin/scripts/workbench_scan_start.py

@@ -72,6 +72,19 @@ def scan_diff_identity(
     )
 
 
+def stored_diff_target(row: sqlite3.Row) -> dict[str, str] | None:
+    if not row["diff_target_kind"]:
+        return None
+    target = {
+        "baseRevision": row["diff_base_revision"],
+        "headRevision": row["diff_head_revision"],
+        "kind": row["diff_target_kind"],
+    }
+    if row["diff_content_digest"]:
+        target["contentDigest"] = row["diff_content_digest"]
+    return target
+
+
 def archive_scan(
     connection: sqlite3.Connection,
     args: argparse.Namespace,

sdk/typescript/src/api.ts

@@ -303,6 +303,7 @@ export class CodexSecurity {
     let targetPathsFile: string | null = null;
     let knowledgeBase: PreparedKnowledgeBase | null = null;
     let costTracker: ScanCostTracker | null = null;
+    let completionCost: ScanCost | null = null;
     let activeScan: {
       id: string;
       options: WorkbenchCommandOptions;
@@ -567,15 +568,50 @@ export class CodexSecurity {
       ]);
       const scanId = registration["scanId"];
       const targetId = registration["targetId"];
+      const contract = registration["contract"];
+      const contractTarget = isRecord(contract)
+        ? contract["target"]
+        : undefined;
+      const allowedKinds = isRecord(contractTarget)
+        ? contractTarget["allowedKinds"]
+        : undefined;
+      const targetKind =
+        Array.isArray(allowedKinds) && allowedKinds.length === 1
+          ? allowedKinds[0]
+          : undefined;
+      const diffTarget = isRecord(contract)
+        ? contract["diffTarget"]
+        : undefined;
+      const snapshotDigest =
+        targetKind === "git_diff" && isRecord(diffTarget)
+          ? diffTarget["contentDigest"]
+          : isRecord(contractTarget)
+            ? contractTarget["requiredSnapshotDigest"]
+            : undefined;
+      const registeredRevision = registration["targetRevision"];
       if (
         typeof scanId !== "string" ||
         typeof targetId !== "string" ||
-        registration["scanDir"] !== scanDir
+        registration["scanDir"] !== scanDir ||
+        typeof targetKind !== "string" ||
+        ![
+          "git_revision",
+          "git_worktree",
+          "git_diff",
+          "directory_snapshot",
+        ].includes(targetKind) ||
+        (snapshotDigest !== undefined && typeof snapshotDigest !== "string") ||
+        ((targetKind === "git_worktree" ||
+          targetKind === "directory_snapshot") &&
+          typeof snapshotDigest !== "string") ||
+        typeof registeredRevision !== "string"
       ) {
         throw new CodexSecurityError(
           "The Codex Security workbench returned an invalid scan registration.",
         );
       }
+      const targetRevision =
+        registeredRevision === "unversioned" ? null : registeredRevision;
       activeScan = { id: scanId, options: workbenchOptions };
       checkOpen();
       const feedback = await workbench(
@@ -642,6 +678,13 @@ export class CodexSecurity {
         CODEX_SECURITY_SCAN_ID: scanId,
         CODEX_SECURITY_TARGET_ID: targetId,
         CODEX_SECURITY_TARGET_DISPLAY_NAME: basename(repo),
+        CODEX_SECURITY_TARGET_KIND: targetKind,
+        ...(targetRevision === null
+          ? {}
+          : { CODEX_SECURITY_TARGET_REVISION: targetRevision }),
+        ...(typeof snapshotDigest === "string"
+          ? { CODEX_SECURITY_TARGET_SNAPSHOT_DIGEST: snapshotDigest }
+          : {}),
         ...(knowledgeBase === null
           ? {}
           : { CODEX_SECURITY_KNOWLEDGE_BASE: knowledgeBase.path }),
@@ -703,6 +746,7 @@ export class CodexSecurity {
         scanDir,
         pluginRoot: runtime.plugin.installedRoot,
         expectation,
+        workbenchValidated: true,
         model,
         onThreadStarted: (threadId) => tracker.start(threadId),
         onFinalize: async (usage) => {
@@ -716,30 +760,12 @@ export class CodexSecurity {
               "Scan completed, but its cost limit could not be verified because model pricing or token usage is unavailable.",
             );
           }
-          const cost = snapshot.cost;
-          const completion = await workbench(workbenchOptions, [
-            "complete-scan",
+          completionCost = snapshot.cost;
+          await workbench(workbenchOptions, [
+            "prepare-scan-completion",
             "--scan-id",
             scanId,
-            ...(cost === null ? [] : ["--cost-json", JSON.stringify(cost)]),
           ]);
-          activeScan = null;
-          const completedScan = completion["scan"];
-          if (
-            isRecord(completedScan) &&
-            Array.isArray(completedScan["warnings"])
-          ) {
-            for (const warning of completedScan["warnings"]) {
-              if (typeof warning === "string") {
-                notifyObserver(
-                  "onWarning",
-                  options.onWarning,
-                  options.onObserverError,
-                  warning,
-                );
-              }
-            }
-          }
           return snapshot.usage;
         },
         onScanStarted: options.onScanStarted,
@@ -748,6 +774,28 @@ export class CodexSecurity {
         onObserverError: options.onObserverError,
       });
       checkOpen();
+      const completion = await workbench(workbenchOptions, [
+        "complete-scan",
+        "--scan-id",
+        scanId,
+        ...(completionCost === null
+          ? []
+          : ["--cost-json", JSON.stringify(completionCost)]),
+      ]);
+      activeScan = null;
+      const completedScan = completion["scan"];
+      if (isRecord(completedScan) && Array.isArray(completedScan["warnings"])) {
+        for (const warning of completedScan["warnings"]) {
+          if (typeof warning === "string") {
+            notifyObserver(
+              "onWarning",
+              options.onWarning,
+              options.onObserverError,
+              warning,
+            );
+          }
+        }
+      }
       return result;
     } catch (error) {
       const snapshot = await costTracker?.stop().catch(() => null);
@@ -1154,6 +1202,7 @@ interface ScanEventRunOptions {
   scanDir: string;
   pluginRoot: string;
   expectation: ScanExpectation;
+  workbenchValidated?: boolean;
   model?: string;
   onFinalize?: (usage: unknown) => Promise<unknown>;
   onThreadStarted?: (threadId: string) => void;
@@ -1273,6 +1322,7 @@ export async function runScanEvents(
       options.pluginRoot,
       options.expectation,
       options.signal,
+      options.workbenchValidated,
     );
     if (options.signal.aborted) {
       throw new ScanInterruptedError(
@@ -1326,6 +1376,9 @@ async function scanPrompt(
     'Use exactly "$CODEX_SECURITY_SCAN_ID" as the scan ID in the manifest, findings, and coverage.',
     'Use exactly "$CODEX_SECURITY_TARGET_ID" as scan.target.targetId; do not derive a different target ID.',
     'Use exactly "$CODEX_SECURITY_TARGET_DISPLAY_NAME" as scan.target.displayName; do not infer a display name from the Git remote.',
+    'Use exactly "$CODEX_SECURITY_TARGET_KIND" as scan.target.kind; do not infer the target kind from the checkout.',
+    'When "$CODEX_SECURITY_TARGET_REVISION" is set, use its exact value as scan.target.revision.',
+    'When "$CODEX_SECURITY_TARGET_SNAPSHOT_DIGEST" is set, use its exact value as scan.target.snapshotDigest. For git_revision, omit scan.target.snapshotDigest.',
     'Use exactly "codex-security-plugin" as scan.producer.name.',
     ...(hasConfigPath
       ? [
@@ -1416,6 +1469,7 @@ async function collectResult(
   pluginRoot: string,
   expectation: ScanExpectation,
   signal: AbortSignal,
+  workbenchValidated = false,
 ): Promise<ScanResult> {
   const required = [
     "scan-manifest.json",
@@ -1440,6 +1494,7 @@ async function collectResult(
   const { manifest, findings, coverage } = await loadContract(scanDir, {
     pluginRoot,
     expectation,
+    workbenchValidated,
     signal,
   });
   let sarifPath: string | null = null;