From 3c4985ca85307dee40b1448b1dfbc7a64512c591 Mon Sep 17 00:00:00 2001 From: Rohan Poudel Date: Mon, 3 Aug 2026 15:48:31 -0600 Subject: [PATCH 1/2] fix(contract): adopt the registered target kind when sealing a draft The scan prompt tells the agent to copy CODEX_SECURITY_TARGET_KIND verbatim instead of inferring scan.target.kind from the checkout, because only the workbench knows how the target was registered. A draft that inferred it anyway failed _validate_completion_binding with "scan.target.kind: must match the workbench target", which discarded a scan whose analysis had already finished and whose findings, coverage, and manifest were complete on disk. A clean worktree is the ordinary way to reach this. The workbench registers it as git_revision, since a worktree with no uncommitted changes is content- identical to its revision, while the checkout still looks like a worktree to the agent. Bulk-scan checkouts are always clean, so any campaign could hit it. scan.target.kind carries no draft-owned information when the registration allows a single kind, so take the registered kind during draft population rather than rejecting the scan. This runs only for unsealed drafts, next to the coordinate replacement that already treats workbench-owned target fields as authoritative, so the pruning there now sees the registered kind and drops coordinates that kind must not carry. Sealed scans keep verifying unchanged. Fixes #62 Fixes #50 --- .../scripts/finalize_scan_contract.py | 28 ++++++++++++++++++- sdk/typescript/tests-ts/scan-recovery.test.ts | 27 ++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py b/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py index 31190058..147a1e3a 100644 --- a/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py +++ b/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py @@ -1094,19 +1094,45 @@ def _populate_unsealed_manifest_envelope( target = scan.get("target") if isinstance(target, dict): - _populate_unsealed_target_binding(target, completion_binding["target"]) + _populate_unsealed_target_binding( + target, + completion_binding["target"], + completion_binding.get("allowedTargetKinds"), + ) scope = scan.get("scope") if isinstance(scope, dict): scope.update(copy.deepcopy(completion_binding["scope"])) +def _populate_unsealed_target_kind(target: dict[str, Any], allowed_kinds: Any) -> None: + """Adopt the registered target kind when the workbench allows exactly one. + + The scan prompt tells the agent to copy CODEX_SECURITY_TARGET_KIND verbatim rather + than infer the kind from the checkout, because only the workbench knows how the + target was registered. A draft that infers it anyway used to discard the whole + completed scan at the seal step. A clean worktree is the common way to hit this: + the workbench registers it as git_revision, while the checkout still looks like a + worktree to the agent. The kind carries no draft-owned information when the + registration allows a single value, so take that value instead of failing. + """ + + if not isinstance(allowed_kinds, list) or len(allowed_kinds) != 1: + return + registered_kind = allowed_kinds[0] + if not isinstance(registered_kind, str) or target.get("kind") == registered_kind: + return + target["kind"] = registered_kind + + def _populate_unsealed_target_binding( target: dict[str, Any], target_binding: dict[str, Any], + allowed_kinds: Any = None, ) -> None: """Replace workbench-owned target coordinates without retaining incompatible drafts.""" + _populate_unsealed_target_kind(target, allowed_kinds) target_kind = target.get("kind") required_coordinates = ( TARGET_REQUIRED_COORDINATE_FIELDS.get(target_kind, set()) diff --git a/sdk/typescript/tests-ts/scan-recovery.test.ts b/sdk/typescript/tests-ts/scan-recovery.test.ts index 2da5fb98..73042ddb 100644 --- a/sdk/typescript/tests-ts/scan-recovery.test.ts +++ b/sdk/typescript/tests-ts/scan-recovery.test.ts @@ -315,6 +315,33 @@ describe("malformed scan artifact recovery", () => { } }); + test("seals a clean worktree draft that inferred the worktree target kind", async () => { + const fixture = await startDraftScan("clean"); + const manifestPath = join(fixture.scanDir, "scan-manifest.json"); + const draft = await readJson<{ + scan: { target: { kind: string; snapshotDigest?: string } }; + }>(manifestPath); + draft.scan.target.kind = "git_worktree"; + draft.scan.target.snapshotDigest = `codex-security-snapshot/v1:sha256:${"a".repeat(64)}`; + await writeJson(manifestPath, draft); + const revision = spawnSync( + "git", + ["-C", fixture.repository, "rev-parse", "HEAD"], + { encoding: "utf8" }, + ); + expect(revision.status, revision.stderr).toBe(0); + + const completed = await completeScan(fixture); + + expect(completed.progress.status).toBe("complete"); + const sealed = await readJson<{ + scan: { target: { kind: string; revision: string } }; + }>(manifestPath); + expect(sealed.scan.target.kind).toBe("git_revision"); + expect(sealed.scan.target.revision).toBe(revision.stdout.trim()); + expect(sealed.scan.target).not.toHaveProperty("snapshotDigest"); + }); + test("seals a prepared scan without publishing it before acceptance", async () => { const fixture = await startDraftScan(); From 042cf3622f9cfd34a62dde35dd4c3143cde2f1b8 Mon Sep 17 00:00:00 2001 From: Rohan Poudel Date: Mon, 3 Aug 2026 18:01:34 -0600 Subject: [PATCH 2/2] fix(contract): keep target kinds the binding cannot rebind Adopting the registered target kind also reinterprets the coordinates that kind requires, and the pruning beside it keeps a coordinate the kind requires even when the completion binding does not carry one. The draft's own value then survives under the new kind's meaning. Commit and range diffs are the reachable case. The workbench registers them as git_diff and authors only base and head revisions, never a snapshotDigest, so a draft that labelled itself git_worktree or directory_snapshot had the digest it computed for whole-worktree or directory contents sealed as the diff digest instead of the mismatch being reported. Take the registered kind only when the binding supplies every coordinate that kind requires. That still normalizes the clean worktree this change was written for, where the binding owns the revision behind git_revision, along with dirty worktrees, directory snapshots, and working-tree diffs, whose registrations all author the snapshot digest. Every other kind change keeps the draft value so _validate_completion_binding reports the mismatch it already describes. --- .../scripts/finalize_scan_contract.py | 20 ++- sdk/typescript/tests-ts/scan-recovery.test.ts | 121 +++++++++++++++++- 2 files changed, 135 insertions(+), 6 deletions(-) diff --git a/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py b/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py index c71b1014..b56b87d4 100644 --- a/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py +++ b/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py @@ -1126,8 +1126,12 @@ def _populate_unsealed_manifest_envelope( scope.update(copy.deepcopy(completion_binding["scope"])) -def _populate_unsealed_target_kind(target: dict[str, Any], allowed_kinds: Any) -> None: - """Adopt the registered target kind when the workbench allows exactly one. +def _populate_unsealed_target_kind( + target: dict[str, Any], + target_binding: dict[str, Any], + allowed_kinds: Any, +) -> None: + """Adopt the registered target kind when the binding owns every coordinate it needs. The scan prompt tells the agent to copy CODEX_SECURITY_TARGET_KIND verbatim rather than infer the kind from the checkout, because only the workbench knows how the @@ -1136,6 +1140,14 @@ def _populate_unsealed_target_kind(target: dict[str, Any], allowed_kinds: Any) - the workbench registers it as git_revision, while the checkout still looks like a worktree to the agent. The kind carries no draft-owned information when the registration allows a single value, so take that value instead of failing. + + Rewriting the kind also reinterprets the coordinates that kind requires, and a + coordinate the binding does not own survives the replacement below with whatever the + draft computed under its own reading of the target. A commit or range diff is the + case that matters: the workbench registers git_diff but authors no snapshotDigest, + so adopting the kind would seal a whole-worktree or directory digest as the diff + digest. Leave the kind alone whenever the binding cannot restate every coordinate + the registered kind requires, and let the binding check reject the draft instead. """ if not isinstance(allowed_kinds, list) or len(allowed_kinds) != 1: @@ -1143,6 +1155,8 @@ def _populate_unsealed_target_kind(target: dict[str, Any], allowed_kinds: Any) - registered_kind = allowed_kinds[0] if not isinstance(registered_kind, str) or target.get("kind") == registered_kind: return + if not TARGET_REQUIRED_COORDINATE_FIELDS.get(registered_kind, set()) <= target_binding.keys(): + return target["kind"] = registered_kind @@ -1153,7 +1167,7 @@ def _populate_unsealed_target_binding( ) -> None: """Replace workbench-owned target coordinates without retaining incompatible drafts.""" - _populate_unsealed_target_kind(target, allowed_kinds) + _populate_unsealed_target_kind(target, target_binding, allowed_kinds) target_kind = target.get("kind") required_coordinates = ( TARGET_REQUIRED_COORDINATE_FIELDS.get(target_kind, set()) diff --git a/sdk/typescript/tests-ts/scan-recovery.test.ts b/sdk/typescript/tests-ts/scan-recovery.test.ts index 6a23460e..c3ad41d7 100644 --- a/sdk/typescript/tests-ts/scan-recovery.test.ts +++ b/sdk/typescript/tests-ts/scan-recovery.test.ts @@ -117,7 +117,12 @@ async function workbench(fixture: ScanFixture, args: readonly string[]) { } async function startDraftScan( - repositoryKind: "directory" | "clean" | "dirty" | "nested" = "directory", + repositoryKind: + | "directory" + | "clean" + | "dirty" + | "nested" + | "range-diff" = "directory", ): Promise { const root = await realpath( await mkdtemp(join(tmpdir(), "codex-security-scan-recovery-")), @@ -155,6 +160,27 @@ async function startDraftScan( if (repositoryKind === "dirty") { await writeFile(join(target, "src", "extract.py"), "# changed fixture\n"); } + if (repositoryKind === "range-diff") { + await writeFile(join(target, "src", "extract.py"), "# second fixture\n"); + for (const args of [ + ["-C", target, "add", "--", "src/extract.py"], + [ + "-C", + target, + "-c", + "user.name=Codex Security", + "-c", + "user.email=codex-security@example.invalid", + "commit", + "--quiet", + "-m", + "fixture change", + ], + ]) { + const result = spawnSync("git", args, { encoding: "utf8" }); + expect(result.status, result.stderr).toBe(0); + } + } if (repositoryKind === "nested") { const nested = join(target, "nested"); await mkdir(nested); @@ -185,7 +211,10 @@ async function startDraftScan( config: {}, mode: "standard", repository: target, - target: { kind: "repository", paths: [] }, + target: + repositoryKind === "range-diff" + ? { kind: "refs", paths: [], base: "HEAD~1", head: "HEAD" } + : { kind: "repository", paths: [] }, }), ]); fixture.scanId = String(registration["scanId"]); @@ -209,7 +238,9 @@ async function startDraftScan( ? "directory_snapshot" : repositoryKind === "clean" ? "git_revision" - : "git_worktree"; + : repositoryKind === "range-diff" + ? "git_diff" + : "git_worktree"; delete manifest.scan.sealedAt; delete manifest.scan.artifacts; await writeJson(manifestPath, manifest); @@ -342,6 +373,90 @@ describe("malformed scan artifact recovery", () => { expect(sealed.scan.target).not.toHaveProperty("snapshotDigest"); }); + test("seals a dirty worktree draft that inferred the revision target kind", async () => { + const fixture = await startDraftScan("dirty"); + const manifestPath = join(fixture.scanDir, "scan-manifest.json"); + const draft = await readJson<{ + scan: { target: { kind: string; snapshotDigest?: string } }; + }>(manifestPath); + draft.scan.target.kind = "git_revision"; + draft.scan.target.snapshotDigest = `codex-security-snapshot/v1:sha256:${"a".repeat(64)}`; + await writeJson(manifestPath, draft); + const registered = ( + fixture.registration["contract"] as { + target: { requiredSnapshotDigest: string }; + } + ).target.requiredSnapshotDigest; + + const completed = await completeScan(fixture); + + expect(completed.progress.status).toBe("complete"); + const sealed = await readJson<{ + scan: { target: { kind: string; snapshotDigest: string } }; + }>(manifestPath); + expect(sealed.scan.target.kind).toBe("git_worktree"); + expect(sealed.scan.target.snapshotDigest).toBe(registered); + }); + + test("rejects a range diff draft whose inferred kind kept an unbindable digest", async () => { + const fixture = await startDraftScan("range-diff"); + const manifestPath = join(fixture.scanDir, "scan-manifest.json"); + const draft = await readJson<{ + scan: { target: { kind: string; snapshotDigest?: string } }; + }>(manifestPath); + draft.scan.target.kind = "git_worktree"; + draft.scan.target.snapshotDigest = `codex-security-snapshot/v1:sha256:${"b".repeat(64)}`; + await writeJson(manifestPath, draft); + + await expect(completeScan(fixture)).rejects.toThrow( + "scan.target.kind: must match the workbench target", + ); + expect( + ( + await readJson<{ scan: { target: { snapshotDigest?: string } } }>( + manifestPath, + ) + ).scan.target.snapshotDigest, + ).toBe(`codex-security-snapshot/v1:sha256:${"b".repeat(64)}`); + }); + + test("seals a range diff draft that reported the registered diff kind", async () => { + const fixture = await startDraftScan("range-diff"); + const manifestPath = join(fixture.scanDir, "scan-manifest.json"); + const draft = await readJson<{ + scan: { target: { snapshotDigest: string } }; + }>(manifestPath); + const digest = draft.scan.target.snapshotDigest; + const revisions = spawnSync( + "git", + ["-C", fixture.repository, "rev-parse", "HEAD~1", "HEAD"], + { encoding: "utf8" }, + ); + expect(revisions.status, revisions.stderr).toBe(0); + const revisionList = revisions.stdout.trim().split("\n"); + const base = revisionList[0]!; + const head = revisionList[1]!; + + const completed = await completeScan(fixture); + + expect(completed.progress.status).toBe("complete"); + const sealed = await readJson<{ + scan: { + target: { + kind: string; + baseRevision: string; + headRevision: string; + snapshotDigest: string; + }; + }; + }>(manifestPath); + expect(sealed.scan.target.kind).toBe("git_diff"); + expect(sealed.scan.target.baseRevision).toBe(base); + expect(sealed.scan.target.headRevision).toBe(head); + expect(sealed.scan.target.snapshotDigest).toBe(digest); + expect(sealed.scan.target).not.toHaveProperty("revision"); + }); + test("seals a prepared scan without publishing it before acceptance", async () => { const fixture = await startDraftScan();