diff --git a/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py b/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py index 0d545f27..b56b87d4 100644 --- a/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py +++ b/sdk/typescript/_bundled_plugin/scripts/finalize_scan_contract.py @@ -1115,19 +1115,59 @@ 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], + 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 + 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. + + 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: + return + 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 + + 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, 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 77d93cd8..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); @@ -315,6 +346,117 @@ 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 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();