diff --git a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py index c39c9635..624c68c3 100644 --- a/sdk/typescript/_bundled_plugin/scripts/workbench_target.py +++ b/sdk/typescript/_bundled_plugin/scripts/workbench_target.py @@ -10,8 +10,9 @@ import stat import subprocess import sys +import tempfile from pathlib import Path -from typing import Any +from typing import IO, Any # Some plugin hosts launch Python with safe-path isolation enabled. sys.path.insert(0, str(Path(__file__).resolve().parent)) @@ -40,12 +41,55 @@ def git_bytes( return completed.stdout if completed.returncode == 0 else None +def git_digest_field( + digest: Any, + label: bytes, + target: Path, + *args: str, + git_dir: Path | None = None, + work_tree: Path | None = None, +) -> bool: + """Frame Git output into ``digest`` without holding the output in memory. + + ``update_digest_field`` prefixes every value with its total length, so the byte + count has to be known before any of the content is hashed. Spooling stdout to a + private temporary file records that length from a single Git invocation, and the file + is then hashed in chunks. The framing is byte for byte what ``update_digest_field`` + writes, so digests stay comparable with recorded ones. + + Returns whether Git succeeded; the digest is left untouched when it did not. + """ + # The spool lives in the process temporary directory, never in the scan directory or + # the repository, and is owner-only. On POSIX it is unlinked before Git writes to it, + # so the patch is never reachable by name and cannot outlive this process. + with tempfile.TemporaryFile() as spool: + completed = git_command( + target, + *args, + text=False, + git_dir=git_dir, + work_tree=work_tree, + stdout=spool, + ) + if completed.returncode != 0: + return False + size = os.fstat(spool.fileno()).st_size + spool.seek(0) + digest.update(len(label).to_bytes(4, "big")) + digest.update(label) + digest.update(size.to_bytes(8, "big")) + for chunk in iter(lambda: spool.read(1024 * 1024), b""): + digest.update(chunk) + return True + + def git_command( target: Path, *args: str, text: bool, git_dir: Path | None = None, work_tree: Path | None = None, + stdout: IO[bytes] | None = None, ) -> subprocess.CompletedProcess[str] | subprocess.CompletedProcess[bytes]: if (git_dir is None) != (work_tree is None): raise ValueError("git_dir and work_tree must be provided together") @@ -62,7 +106,8 @@ def git_command( return subprocess.run( full_command, check=False, - capture_output=True, + stdout=subprocess.PIPE if stdout is None else stdout, + stderr=subprocess.PIPE, env=environment, text=text, ) @@ -93,7 +138,13 @@ def worktree_content_digest_for_context( git_dir: Path | None = None, work_tree: Path | None = None, ) -> str: - tracked = git_bytes( + digest = hashlib.sha256() + update_digest_field(digest, b"format", b"codex-security-snapshot/v1") + # The tracked patch is streamed: `git diff --binary` over a large changed binary + # produces a patch several times the file size, which must not be buffered. + tracked = git_digest_field( + digest, + b"tracked-diff", repository, "diff", "--binary", @@ -118,11 +169,8 @@ def worktree_content_digest_for_context( git_dir=git_dir, work_tree=work_tree, ) - if tracked is None or untracked is None: + if not tracked or untracked is None: raise SystemExit("Could not snapshot the selected working-tree changes.") - digest = hashlib.sha256() - update_digest_field(digest, b"format", b"codex-security-snapshot/v1") - update_digest_field(digest, b"tracked-diff", tracked) for raw_path in sorted(path for path in untracked.split(b"\0") if path): relative_path = os.fsdecode(raw_path) path = (work_tree or repository) / relative_path diff --git a/sdk/typescript/tests-ts/workbench-content-digest.test.ts b/sdk/typescript/tests-ts/workbench-content-digest.test.ts new file mode 100644 index 00000000..eca86216 --- /dev/null +++ b/sdk/typescript/tests-ts/workbench-content-digest.test.ts @@ -0,0 +1,259 @@ +import { execFileSync } from "node:child_process"; +import { + mkdir, + mkdtemp, + readdir, + realpath, + rm, + symlink, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, test } from "bun:test"; +import { PLUGIN_ROOT } from "./plugin-root.js"; + +const temporaryDirectories: string[] = []; +const testPosix = process.platform === "win32" ? test.skip : test; + +// The streaming helper spools `git diff --binary` to a temporary file so the framed +// value length is known before hashing. This probe recomputes the same digest with the +// buffered framing the helper replaced, which pins the compatibility requirement: +// recorded digests are compared against freshly computed ones when a selection is +// revalidated, so the two must agree byte for byte. +const digestProbe = [ + "import json, sys", + "from pathlib import Path", + "sys.path.insert(0, sys.argv[1])", + "import workbench_target as target", + "try:", + " import resource", + "except ImportError:", + " resource = None", + "def peak():", + " if resource is None:", + " return None", + " value = resource.getrusage(resource.RUSAGE_SELF).ru_maxrss", + " return value if sys.platform == 'darwin' else value * 1024", + "repository = Path(sys.argv[2])", + "before = peak()", + "streamed = target.worktree_content_digest(repository)", + "after = peak()", + "def buffered_field(digest, label, tree, *args, git_dir=None, work_tree=None):", + " value = target.git_bytes(tree, *args, git_dir=git_dir, work_tree=work_tree)", + " if value is None:", + " return False", + " target.update_digest_field(digest, label, value)", + " return True", + "target.git_digest_field = buffered_field", + "buffered = target.worktree_content_digest(repository)", + "patch = target.git_bytes(repository, 'diff', '--binary', '--full-index', '--no-ext-diff', '--no-textconv', '--ignore-submodules=none', 'HEAD', '--', '.')", + "print(json.dumps({", + " 'streamed': streamed,", + " 'buffered': buffered,", + " 'sentinel': target.clean_worktree_content_digest(),", + " 'patchBytes': len(patch),", + " 'peakRssIncreaseBytes': None if before is None else after - before,", + "}))", +].join("\n"); + +// The streaming helper reports Git failure as `False` rather than the `None` the +// buffered helper returned, so the caller's guard has to reject a falsy value. A guard +// that still tests `is None` accepts `False` and records a digest over an empty patch, +// which is a wrong snapshot rather than a visible error. +const failureProbe = [ + "import json, sys", + "from pathlib import Path", + "sys.path.insert(0, sys.argv[1])", + "import workbench_target as target", + "repository = Path(sys.argv[2])", + "try:", + " result = {'digest': target.worktree_content_digest(repository)}", + "except SystemExit as exc:", + " result = {'exit': str(exc)}", + "print(json.dumps(result))", +].join("\n"); + +interface DigestProbeResult { + streamed: string; + buffered: string; + sentinel: string; + patchBytes: number; + peakRssIncreaseBytes: number | null; +} + +interface FailureProbeResult { + digest?: string; + exit?: string; +} + +afterEach(async () => { + await Promise.all( + temporaryDirectories + .splice(0) + .map((directory) => rm(directory, { recursive: true, force: true })), + ); +}); + +async function temporaryDirectory(): Promise { + const directory = await realpath( + await mkdtemp(join(tmpdir(), "codex-security-content-digest-")), + ); + temporaryDirectories.push(directory); + return directory; +} + +function git(repository: string, ...args: string[]): string { + return execFileSync("git", args, { + cwd: repository, + encoding: "utf8", + }).trim(); +} + +async function repository(): Promise { + const root = await temporaryDirectory(); + const path = join(root, "repo"); + await mkdir(path, { recursive: true }); + git(path, "init", "-b", "main"); + git(path, "config", "user.email", "test@example.com"); + git(path, "config", "user.name", "Test"); + await writeFile(join(path, "README.md"), "baseline\n"); + git(path, "add", "."); + git(path, "commit", "-m", "initial"); + return path; +} + +// Deterministic incompressible bytes, so the binary patch cannot be shrunk by zlib. +function incompressible(size: number, seed: number): Uint8Array { + const bytes = new Uint8Array(size); + let state = seed; + for (let index = 0; index < size; index += 1) { + state = (Math.imul(state, 1103515245) + 12345) >>> 0; + bytes[index] = (state >>> 16) & 0xff; + } + return bytes; +} + +async function runProbe(source: string, target: string): Promise { + const python = Bun.which("python3") ?? Bun.which("python") ?? Bun.which("py"); + expect(python).not.toBeNull(); + if (python === null) { + throw new Error( + "A Python interpreter is required for content digest tests.", + ); + } + + // A private temporary directory proves the spool file does not outlive the digest. + const spoolDirectory = await temporaryDirectory(); + const result = Bun.spawnSync( + [python, "-I", "-B", "-c", source, join(PLUGIN_ROOT, "scripts"), target], + { + stdout: "pipe", + stderr: "pipe", + env: { + ...process.env, + TMPDIR: spoolDirectory, + TMP: spoolDirectory, + TEMP: spoolDirectory, + }, + }, + ); + expect(new TextDecoder().decode(result.stderr)).toBe(""); + expect(result.exitCode).toBe(0); + expect(await readdir(spoolDirectory)).toEqual([]); + return new TextDecoder().decode(result.stdout); +} + +async function runDigestProbe(target: string): Promise { + return JSON.parse(await runProbe(digestProbe, target)) as DigestProbeResult; +} + +describe("bundled workbench content digests", () => { + test("streams the tracked patch without changing the recorded digest", async () => { + const clean = await repository(); + const cleanResult = await runDigestProbe(clean); + + expect(cleanResult.streamed).toBe(cleanResult.buffered); + // The hardcoded clean-worktree sentinel must still describe an unchanged tree. + expect(cleanResult.streamed).toBe(cleanResult.sentinel); + + const text = await repository(); + await writeFile(join(text, "README.md"), "baseline\nmodified line\n"); + const textResult = await runDigestProbe(text); + + expect(textResult.streamed).toBe(textResult.buffered); + expect(textResult.streamed).not.toBe(textResult.sentinel); + + const binary = await repository(); + await writeFile(join(binary, "payload.bin"), incompressible(64 * 1024, 7)); + git(binary, "add", "."); + git(binary, "commit", "-m", "payload"); + await writeFile(join(binary, "payload.bin"), incompressible(64 * 1024, 11)); + const binaryResult = await runDigestProbe(binary); + + expect(binaryResult.streamed).toBe(binaryResult.buffered); + expect(binaryResult.patchBytes).toBeGreaterThan(64 * 1024); + + const untracked = await repository(); + await writeFile(join(untracked, "README.md"), "baseline\nedited\n"); + await writeFile(join(untracked, "new.txt"), "untracked content\n"); + await mkdir(join(untracked, "nested")); + await writeFile(join(untracked, "nested", "inner.txt"), "inner\n"); + if (process.platform !== "win32") { + await symlink("README.md", join(untracked, "link.txt")); + } + const untrackedResult = await runDigestProbe(untracked); + + expect(untrackedResult.streamed).toBe(untrackedResult.buffered); + }); + + test("refuses to record a digest when Git cannot emit the tracked patch", async () => { + // An unborn branch has no HEAD, so `git diff HEAD` exits non-zero and writes no + // patch. Streaming has to reject that as firmly as buffering did: the spool would + // otherwise be hashed as an empty value and pass for a clean worktree. + const root = await temporaryDirectory(); + const unborn = join(root, "repo"); + await mkdir(unborn, { recursive: true }); + git(unborn, "init", "-b", "main"); + git(unborn, "config", "user.email", "test@example.com"); + git(unborn, "config", "user.name", "Test"); + await writeFile(join(unborn, "README.md"), "unborn\n"); + + const result = JSON.parse( + await runProbe(failureProbe, unborn), + ) as FailureProbeResult; + + expect(result.digest).toBeUndefined(); + expect(result.exit).toBe( + "Could not snapshot the selected working-tree changes.", + ); + }); + + // Roughly 4 MiB of incompressible content, which Git expands into a binary patch + // several times that size. Kept small deliberately: the fixture costs about a + // second, and the buffering regression it guards is proportional, not threshold-based. + testPosix("hashes a large binary patch without buffering it", async () => { + const large = await repository(); + await writeFile( + join(large, "large.bin"), + incompressible(4 * 1024 * 1024, 3), + ); + git(large, "add", "."); + git(large, "commit", "-m", "large payload"); + await writeFile( + join(large, "large.bin"), + incompressible(4 * 1024 * 1024, 5), + ); + + const result = await runDigestProbe(large); + + expect(result.streamed).toBe(result.buffered); + expect(result.patchBytes).toBeGreaterThan(4 * 1024 * 1024); + expect(result.peakRssIncreaseBytes).not.toBeNull(); + // Buffering the patch would grow the process by at least the patch size; streaming + // holds one chunk at a time. + expect(result.peakRssIncreaseBytes as number).toBeLessThan( + result.patchBytes / 2, + ); + }); +});