diff --git a/CHANGELOG.md b/CHANGELOG.md index 707c18532e..068e67d8fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,23 @@ +### Sandboxed verification emits a versioned, binary-safe trusted result bundle + +- `scripts/ci/sandboxed_verify.py --result-file ` now keeps command + stdout, command stderr, and the wrapper-controlled JSON envelope in three + exclusive sibling files. The stream files preserve arbitrary and large + binary bytes exactly; their SHA-256 digests and byte lengths are bound into + the `sandboxed_verify.execution.v1` envelope with argv, exit code, explicit + completed/timeout/copy-rejection/internal-error state, runtime identity, + requested network mode, and allowed environment names. Result-directory + traversal uses directory file descriptors with no-follow semantics for every ancestor, and every bundle + file uses exclusive creation, closing the nested-symlink and substitution + races in the first result-file implementation. A bounded evidence-write + failure returns 125 without a traceback when the command succeeded, preserves + an existing command/timeout/copy-rejection failure code, and cannot skip + temporary sandbox cleanup unless `--keep-sandbox` explicitly requests + retention. The envelope explicitly records that this helper supplies a copied + workspace and scrubbed environment, not OS process isolation or enforced + network policy. Legacy stdout-marker mode remains available for human-only + calls. Refs #2086, #2088. + ### Failed-check finding names the Strix sandbox instead of the gateway - `opencode-review-dispatch.yml`'s `emit_strix_provider_failure_finding` rendered one fixed finding for every `STRIX_PROVIDER_UNAVAILABLE` line, whose Root cause read "The contextual-orchestrator gateway or its discovered provider pool was unavailable for this run". `#1953` had just given the Strix sandbox bootstrap failure its own second verdict token (`STRIX_SANDBOX_UNAVAILABLE`) precisely because that attribution is wrong for it -- the sandbox container never reaches its Caido proxy, so the run dies before the gateway serves anything -- and this consumer re-applied the wrong attribution one step downstream, into the review findings and the failure census. The emitter now branches on the second token: a sandbox verdict gets a finding that names Strix's sandbox, says the verdict does not name the gateway, and tells the reader not to change gateway or provider configuration on its strength. A `STRIX_PROVIDER_UNAVAILABLE` line without the token keeps its existing text verbatim, so the gateway class has no regression surface. No test covered this finding text at all before (`gateway or its discovered provider pool` matched nothing under `tests/`); `tests/test_opencode_dispatch_strix_sandbox_finding.py` now runs the production emitter from the published run block and pins both directions plus the no-signal case. Refs #1953, #1935. diff --git a/docs/pr-review-and-merge-procedure.md b/docs/pr-review-and-merge-procedure.md index e34c3957cb..df351d3861 100644 --- a/docs/pr-review-and-merge-procedure.md +++ b/docs/pr-review-and-merge-procedure.md @@ -215,10 +215,23 @@ the specific environment variable names required and record why they were needed. The central helper is `python3 scripts/ci/sandboxed_verify.py --repo-root -- `; reviews should cite its `SANDBOXED_VERIFY_RESULT` -line when the helper is used. Use `--network required`, `--allow-env NAME`, -and `--evidence-note "why"` only for repository-required verification. This -helper does not replace the existing bash, task, webfetch, websearch, lsp, -CodeGraph, DeepWiki, Context7, or web_search review policy. +line when the helper is used. For machine handoff, pass `--result-file +`: the helper exclusively creates that versioned envelope plus +`.stdout` and `.stderr`. Those files preserve all +command-controlled stdout and stderr bytes exactly, including marker-shaped or +JSON-shaped content; only the wrapper-authored envelope is trusted control +data. The envelope records hashes, byte lengths, argv, exit code, +`completed`/`timed_out`/`copy_rejected`/`internal_error` state, runtime +identity, allowed environment names, and the requested network mode. It also +says explicitly that this copy-and-scrub helper provides no OS process isolation and does not +enforce network policy. Trusted result paths reject symlink ancestors and +existing bundle files; evidence-write failure is bounded, returns 125 only +when the command succeeded, preserves an existing command/timeout/copy failure +status, and never skips sandbox cleanup unless `--keep-sandbox` explicitly +requests retention. Use `--network required`, `--allow-env NAME`, and +`--evidence-note "why"` only for repository-required verification. This helper +does not replace the existing bash, task, webfetch, websearch, lsp, CodeGraph, +DeepWiki, Context7, or web_search review policy. Scratch PoC files are not committed. For web applications with both backend and frontend surfaces, the preferred diff --git a/scripts/ci/sandboxed_verify.py b/scripts/ci/sandboxed_verify.py index 94797c2038..e17fa00a9c 100644 --- a/scripts/ci/sandboxed_verify.py +++ b/scripts/ci/sandboxed_verify.py @@ -3,9 +3,12 @@ from __future__ import annotations import argparse +import hashlib import json import os +import platform import re +import secrets import shutil import subprocess import sys @@ -94,8 +97,7 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: """Parse CLI arguments for the sandboxed verification wrapper.""" parser = argparse.ArgumentParser( description=( - "Copy the repository into a temporary workspace and run a verification " - "command with a scrubbed environment." + "Copy the repository into a temporary workspace and run a verification command with a scrubbed environment." ) ) parser.add_argument("--repo-root", default=".", help="Repository root to copy into the sandbox.") @@ -129,6 +131,15 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: default="", help="Short reviewer note explaining why network or allowed env variables are needed.", ) + parser.add_argument( + "--result-file", + type=Path, + help=( + "Write the trusted result envelope to a new file and exact command " + "streams to sibling .stdout/.stderr files instead of mixing evidence " + "with command output." + ), + ) parser.add_argument("command", nargs=argparse.REMAINDER, help="Verification command after --.") args = parser.parse_args(argv) if args.command and args.command[0] == "--": @@ -164,7 +175,13 @@ def scrubbed_env(sandbox_root: Path, allow_env: Sequence[str] = ()) -> dict[str, "XDG_DATA_HOME": str(sandbox_root / "xdg-data"), } ) - for path_key in ("HOME", "TMPDIR", "XDG_CACHE_HOME", "XDG_CONFIG_HOME", "XDG_DATA_HOME"): + for path_key in ( + "HOME", + "TMPDIR", + "XDG_CACHE_HOME", + "XDG_CONFIG_HOME", + "XDG_DATA_HOME", + ): Path(env[path_key]).mkdir(parents=True, exist_ok=True) return env @@ -208,7 +225,12 @@ def _reject_escaping_symlinks(destination: Path) -> None: for path in root.rglob("*"): if path.is_symlink(): _resolve_symlink_components( - path.relative_to(root).parts, root, root, set(), [MAXIMUM_SYMLINK_HOPS], path + path.relative_to(root).parts, + root, + root, + set(), + [MAXIMUM_SYMLINK_HOPS], + path, ) @@ -274,12 +296,8 @@ def _resolve_symlink_components( hops_remaining[0] -= 1 target = Path(os.readlink(step)) if target.is_absolute(): - raise ValueError( - f"workspace symlink escapes the sandbox root: {step} -> {target}" - ) - resolved = _resolve_symlink_components( - target.parts, resolved, root, active, hops_remaining, candidate - ) + raise ValueError(f"workspace symlink escapes the sandbox root: {step} -> {target}") + resolved = _resolve_symlink_components(target.parts, resolved, root, active, hops_remaining, candidate) active.discard(step) return resolved @@ -314,9 +332,7 @@ def _ignore(directory: str, names: list[str]) -> set[str]: default_ignored = default_ignore(directory, names) extra_ignored = extra_ignore(directory, names) protected = { - name - for name in default_ignored - if name in DEFAULT_ENV_TEMPLATE_ALLOWLIST and name not in extra_ignored + name for name in default_ignored if name in DEFAULT_ENV_TEMPLATE_ALLOWLIST and name not in extra_ignored } return (default_ignored | extra_ignored) - protected @@ -335,13 +351,14 @@ def copy_workspace(repo_root: Path, sandbox_root: Path, extra_ignores: Sequence[ return destination -def run_command(command: Sequence[str], cwd: Path, env: dict[str, str], timeout: int) -> subprocess.CompletedProcess[str]: +def run_command( + command: Sequence[str], cwd: Path, env: dict[str, str], timeout: int +) -> subprocess.CompletedProcess[bytes]: """Run the verification command and capture output for review evidence.""" return subprocess.run( list(command), cwd=cwd, env=env, - text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, timeout=timeout, @@ -359,6 +376,152 @@ def timeout_output_text(value: str | bytes | None) -> str: return value +def _output_bytes(value: str | bytes | None) -> bytes: + """Normalize captured command output without altering subprocess bytes.""" + if value is None: + return b"" + if isinstance(value, bytes): + return value + return value.encode() + + +def _forward_bytes(stream: object, output: bytes) -> None: + """Forward command bytes exactly when the active stream exposes a buffer.""" + if not output: + return + binary_stream = getattr(stream, "buffer", None) + if binary_stream is not None: + stream.flush() # type: ignore[attr-defined] + binary_stream.write(output) + binary_stream.flush() + return + stream.write(output.decode(errors="replace")) # type: ignore[attr-defined] + stream.flush() # type: ignore[attr-defined] + + +def _open_result_parent(parent: Path) -> int: + """Open/create ``parent`` component-wise without following symlinks.""" + directory_flags = os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) | getattr(os, "O_NOFOLLOW", 0) + if parent.is_absolute(): + directory_fd = os.open(os.path.sep, directory_flags) + components = parent.parts[1:] + else: + directory_fd = os.open(".", directory_flags) + components = parent.parts + try: + for component in components: + if component in ("", "."): + continue + if component == "..": + raise ValueError(f"result file parent is not a regular directory: {parent}") + try: + next_fd = os.open(component, directory_flags, dir_fd=directory_fd) + except FileNotFoundError: + try: + os.mkdir(component, mode=0o700, dir_fd=directory_fd) + except FileExistsError: + pass + try: + next_fd = os.open(component, directory_flags, dir_fd=directory_fd) + except OSError as exc: + raise ValueError(f"result file parent is not a regular directory: {parent}") from exc + except OSError as exc: + raise ValueError(f"result file parent is not a regular directory: {parent}") from exc + os.close(directory_fd) + directory_fd = next_fd + return directory_fd + except BaseException: + os.close(directory_fd) + raise + + +def _write_all(file_descriptor: int, content: bytes) -> None: + """Write all ``content`` to an already-open file descriptor.""" + offset = 0 + while offset < len(content): + offset += os.write(file_descriptor, content[offset:]) + + +def _write_result_bundle( + result_file: Path, + rendered_result: bytes, + stdout_bytes: bytes, + stderr_bytes: bytes, +) -> None: + """Create both streams before atomically publishing the trusted envelope.""" + parent_fd = _open_result_parent(result_file.parent) + stdout_name = result_file.name + ".stdout" + stderr_name = result_file.name + ".stderr" + stream_bundle = ( + (stdout_name, stdout_bytes), + (stderr_name, stderr_bytes), + ) + created_names: list[str] = [] + staged_envelope_name = f".{result_file.name}.{secrets.token_hex(16)}.tmp" + staged_envelope_created = False + file_flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_NOFOLLOW", 0) + try: + try: + os.stat(result_file.name, dir_fd=parent_fd, follow_symlinks=False) + except FileNotFoundError: + pass + else: + raise ValueError(f"result file already exists: {result_file}") + for file_name, content in stream_bundle: + try: + file_descriptor = os.open(file_name, file_flags, 0o600, dir_fd=parent_fd) + except FileExistsError as exc: + raise ValueError( + f"result bundle file already exists: {result_file.parent / file_name}" + ) from exc + created_names.append(file_name) + try: + _write_all(file_descriptor, content) + os.fsync(file_descriptor) + finally: + os.close(file_descriptor) + + staged_descriptor = os.open( + staged_envelope_name, + file_flags, + 0o600, + dir_fd=parent_fd, + ) + staged_envelope_created = True + try: + _write_all(staged_descriptor, rendered_result) + os.fsync(staged_descriptor) + finally: + os.close(staged_descriptor) + try: + os.link( + staged_envelope_name, + result_file.name, + src_dir_fd=parent_fd, + dst_dir_fd=parent_fd, + follow_symlinks=False, + ) + except FileExistsError as exc: + raise ValueError(f"result file already exists: {result_file}") from exc + created_names.append(result_file.name) + os.unlink(staged_envelope_name, dir_fd=parent_fd) + staged_envelope_created = False + except BaseException: + if staged_envelope_created: + try: + os.unlink(staged_envelope_name, dir_fd=parent_fd) + except FileNotFoundError: + pass + for file_name in created_names: + try: + os.unlink(file_name, dir_fd=parent_fd) + except FileNotFoundError: + pass + raise + finally: + os.close(parent_fd) + + def emit_result( *, command: Sequence[str], @@ -370,8 +533,15 @@ def emit_result( allowed_env: Sequence[str], network: str, evidence_note: str, + result_file: Path | None = None, + result_state: str = "completed", + timed_out: bool = False, + stdout_bytes: bytes = b"", + stderr_bytes: bytes = b"", ) -> None: - """Print a machine-readable execution evidence summary.""" + """Write a versioned execution envelope and its exact command streams.""" + stdout_name = result_file.name + ".stdout" if result_file is not None else None + stderr_name = result_file.name + ".stderr" if result_file is not None else None payload = { "allowed_env": sorted(set(allowed_env)), "command": list(command), @@ -379,11 +549,38 @@ def emit_result( "elapsed_seconds": round(elapsed_seconds, 3), "evidence_note": evidence_note, "exit_code": exit_code, + "helper_id": "ContextualWisdomLab/.github:sandboxed_verify", + "isolation": { + "network_enforced": False, + "os_process_isolation": "none", + "workspace": "copy+scrubbed-env", + }, "network": network, + "result_state": result_state, + "runtime": { + "implementation": platform.python_implementation(), + "python_version": platform.python_version(), + }, "sandbox": str(sandbox_root) if kept else "(removed)", "sandboxed": True, + "schema": "sandboxed_verify.execution.v1", + "stderr": { + "file": stderr_name, + "sha256": hashlib.sha256(stderr_bytes).hexdigest(), + "size_bytes": len(stderr_bytes), + }, + "stdout": { + "file": stdout_name, + "sha256": hashlib.sha256(stdout_bytes).hexdigest(), + "size_bytes": len(stdout_bytes), + }, + "timed_out": timed_out, } - print(f"{RESULT_MARKER} {json.dumps(payload, sort_keys=True)}") + rendered = f"{RESULT_MARKER} {json.dumps(payload, sort_keys=True)}\n" + if result_file is None: + print(rendered, end="") + return + _write_result_bundle(result_file, rendered.encode(), stdout_bytes, stderr_bytes) def main(argv: Sequence[str] | None = None) -> int: @@ -393,52 +590,77 @@ def main(argv: Sequence[str] | None = None) -> int: start = time.monotonic() exit_code = 1 copied_repo = sandbox / "repo" + result_state = "internal_error" + command_stdout = b"" + command_stderr = b"" try: try: copied_repo = copy_workspace(Path(args.repo_root), sandbox, args.ignore) except ValueError as exc: print(f"sandboxed-verify: workspace copy rejected: {exc}", file=sys.stderr) exit_code = 125 - return exit_code - env = scrubbed_env(sandbox, args.allow_env) - print(f"sandboxed-verify: cwd={copied_repo}") - print(f"sandboxed-verify: command={' '.join(args.command)}") - if args.allow_env: - print(f"sandboxed-verify: allowed env names={','.join(sorted(set(args.allow_env)))}") - if args.network != "default": - print(f"sandboxed-verify: network={args.network}") - try: - completed = run_command(args.command, copied_repo, env, args.timeout) - if completed.stdout: - print(completed.stdout, end="") - if completed.stderr: - print(completed.stderr, end="", file=sys.stderr) - exit_code = completed.returncode - except subprocess.TimeoutExpired as exc: - stdout = timeout_output_text(exc.stdout) - stderr = timeout_output_text(exc.stderr) - if stdout: - print(stdout, end="" if stdout.endswith("\n") else "\n") - if stderr: - print(stderr, end="" if stderr.endswith("\n") else "\n", file=sys.stderr) - print(f"sandboxed-verify: command timed out after {args.timeout}s", file=sys.stderr) - exit_code = 124 - return exit_code + result_state = "copy_rejected" + else: + env = scrubbed_env(sandbox, args.allow_env) + print(f"sandboxed-verify: cwd={copied_repo}") + print(f"sandboxed-verify: command={' '.join(args.command)}") + if args.allow_env: + print(f"sandboxed-verify: allowed env names={','.join(sorted(set(args.allow_env)))}") + if args.network != "default": + print(f"sandboxed-verify: network={args.network}") + try: + completed = run_command(args.command, copied_repo, env, args.timeout) + command_stdout = _output_bytes(completed.stdout) + command_stderr = _output_bytes(completed.stderr) + _forward_bytes(sys.stdout, command_stdout) + _forward_bytes(sys.stderr, command_stderr) + exit_code = completed.returncode + result_state = "completed" + except subprocess.TimeoutExpired as exc: + command_stdout = _output_bytes(exc.stdout) + command_stderr = _output_bytes(exc.stderr) + _forward_bytes(sys.stdout, command_stdout) + _forward_bytes(sys.stderr, command_stderr) + print( + f"sandboxed-verify: command timed out after {args.timeout}s", + file=sys.stderr, + ) + exit_code = 124 + result_state = "timed_out" finally: elapsed = time.monotonic() - start - emit_result( - command=args.command, - copied_repo=copied_repo, - sandbox_root=sandbox, - exit_code=exit_code, - elapsed_seconds=elapsed, - kept=args.keep_sandbox, - allowed_env=args.allow_env, - network=args.network, - evidence_note=args.evidence_note, - ) - if not args.keep_sandbox: - shutil.rmtree(sandbox, ignore_errors=True) + try: + emit_result( + command=args.command, + copied_repo=copied_repo, + sandbox_root=sandbox, + exit_code=exit_code, + elapsed_seconds=elapsed, + kept=args.keep_sandbox, + allowed_env=args.allow_env, + network=args.network, + evidence_note=args.evidence_note, + result_file=args.result_file, + result_state=result_state, + timed_out=result_state == "timed_out", + stdout_bytes=command_stdout, + stderr_bytes=command_stderr, + ) + except (OSError, ValueError) as exc: + diagnostic = str(exc).splitlines()[0][:240] + print( + f"sandboxed-verify: result evidence rejected: {diagnostic}", + file=sys.stderr, + ) + # Evidence rejection is the primary failure only when the command + # itself succeeded. Preserve an existing command, timeout, or copy + # rejection status so callers do not lose the causal exit code. + if exit_code == 0: + exit_code = 125 + finally: + if not args.keep_sandbox: + shutil.rmtree(sandbox, ignore_errors=True) + return exit_code if __name__ == "__main__": diff --git a/tests/test_sandboxed_verify.py b/tests/test_sandboxed_verify.py index d89b9bb956..489b32442c 100644 --- a/tests/test_sandboxed_verify.py +++ b/tests/test_sandboxed_verify.py @@ -1,7 +1,9 @@ +import hashlib import json import runpy import shutil import sys +import threading from pathlib import Path import pytest @@ -190,7 +192,9 @@ def test_copy_workspace_rejects_absolute_symlink_escaping_sandbox_root(tmp_path) sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", []) -def test_copy_workspace_rejects_relative_symlink_escaping_via_parent_traversal(tmp_path): +def test_copy_workspace_rejects_relative_symlink_escaping_via_parent_traversal( + tmp_path, +): """A relative, ``..``-laden symlink target that exits the copied tree is also rejected.""" outside = tmp_path / "outside-secret.txt" outside.write_text("host-only-content", encoding="utf-8") @@ -263,7 +267,9 @@ def test_copy_workspace_rejects_unresolvable_symlink_cycle(tmp_path): sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", []) -def test_copy_workspace_accepts_the_same_symlink_referenced_twice_non_recursively(tmp_path): +def test_copy_workspace_accepts_the_same_symlink_referenced_twice_non_recursively( + tmp_path, +): """A symlink resolved twice in one chain, not as part of a loop, is accepted. ``link -> shared/../shared/file.txt`` references ``shared`` twice, but @@ -339,7 +345,9 @@ def test_copy_workspace_keeps_symlink_dangling_from_a_missing_internal_target(tm assert not (copied / "dangling.txt").exists() -def test_copy_workspace_accepts_internal_symlink_when_sandbox_root_is_reached_via_symlinked_ancestor(tmp_path): +def test_copy_workspace_accepts_internal_symlink_when_sandbox_root_is_reached_via_symlinked_ancestor( + tmp_path, +): """A benign internal symlink is accepted even when an *ancestor* of the sandbox root is itself reached through a symlink (for example a symlinked default temp directory, unrelated to anything the copied repository controls). @@ -369,7 +377,9 @@ def test_copy_workspace_accepts_internal_symlink_when_sandbox_root_is_reached_vi assert (copied / "link.txt").read_text(encoding="utf-8") == "payload" -def test_copy_workspace_still_rejects_escape_when_sandbox_root_is_reached_via_symlinked_ancestor(tmp_path): +def test_copy_workspace_still_rejects_escape_when_sandbox_root_is_reached_via_symlinked_ancestor( + tmp_path, +): """A genuinely escaping symlink is still rejected when the sandbox root is itself reached through a symlinked ancestor -- walking from the resolved root (this fix) must not weaken the escape check itself. @@ -456,6 +466,48 @@ def test_timeout_output_text_normalizes_subprocess_payloads(): assert sandboxed_verify.timeout_output_text("text-output") == "text-output" +def test_forward_bytes_flushes_text_before_binary_output(): + """Buffered wrapper diagnostics must precede forwarded command bytes.""" + events = [] + + class BinaryStream: + def write(self, output): + events.append(("binary-write", output)) + + def flush(self): + events.append(("binary-flush", None)) + + class TextStream: + buffer = BinaryStream() + + def flush(self): + events.append(("text-flush", None)) + + sandboxed_verify._forward_bytes(TextStream(), b"command-output") + + assert events == [ + ("text-flush", None), + ("binary-write", b"command-output"), + ("binary-flush", None), + ] + + +def test_output_bytes_and_text_only_stream_fallback(): + """String fallbacks preserve text when no binary stream is available.""" + events = [] + + class TextOnlyStream: + def write(self, output): + events.append(("write", output)) + + def flush(self): + events.append(("flush", None)) + + assert sandboxed_verify._output_bytes("text-output") == b"text-output" + sandboxed_verify._forward_bytes(TextOnlyStream(), b"command-output") + assert events == [("write", "command-output"), ("flush", None)] + + def test_main_runs_command_in_copy_without_mutating_source(tmp_path, capsys): """The wrapper runs commands in the copied workspace, not the source tree.""" repo = tmp_path / "repo" @@ -544,6 +596,442 @@ def test_main_reports_allowed_env_network_stderr_timeout_and_kept_sandbox(monkey shutil.rmtree(payload["sandbox"], ignore_errors=True) +def test_main_can_write_wrapper_result_to_exclusive_file(tmp_path, capsys): + """A caller can separate trusted control evidence from command stdout.""" + repo = tmp_path / "repo" + repo.mkdir() + result_file = tmp_path / "handoff" / "result.txt" + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--result-file", + str(result_file), + "--", + sys.executable, + "-c", + "import sys; " + "sys.stdout.buffer.write(b'SANDBOXED_VERIFY_RESULT attacker-controlled\\n{\\\"fake\\\": true}') ; " + "sys.stderr.buffer.write(b'no-final-newline')", + ] + ) + + captured = capsys.readouterr() + assert exit_code == 0 + assert "attacker-controlled" in captured.out + assert any(line == sandboxed_verify.RESULT_MARKER + " attacker-controlled" for line in captured.out.splitlines()) + assert result_file.read_text(encoding="utf-8").startswith(sandboxed_verify.RESULT_MARKER + " {") + stdout_file = result_file.with_name(result_file.name + ".stdout") + stderr_file = result_file.with_name(result_file.name + ".stderr") + stdout_bytes = stdout_file.read_bytes() + stderr_bytes = stderr_file.read_bytes() + payload = json.loads(result_file.read_text(encoding="utf-8").removeprefix(sandboxed_verify.RESULT_MARKER).strip()) + assert stdout_bytes == b'SANDBOXED_VERIFY_RESULT attacker-controlled\n{"fake": true}' + assert stderr_bytes == b"no-final-newline" + assert payload["schema"] == "sandboxed_verify.execution.v1" + assert payload["result_state"] == "completed" + assert payload["timed_out"] is False + assert payload["helper_id"] == "ContextualWisdomLab/.github:sandboxed_verify" + assert payload["runtime"]["implementation"] + assert payload["runtime"]["python_version"] + assert payload["isolation"] == { + "network_enforced": False, + "os_process_isolation": "none", + "workspace": "copy+scrubbed-env", + } + assert payload["stdout"] == { + "file": stdout_file.name, + "sha256": hashlib.sha256(stdout_bytes).hexdigest(), + "size_bytes": len(stdout_bytes), + } + assert payload["stderr"] == { + "file": stderr_file.name, + "sha256": hashlib.sha256(stderr_bytes).hexdigest(), + "size_bytes": len(stderr_bytes), + } + with pytest.raises(ValueError, match="result file already exists"): + sandboxed_verify.emit_result( + command=("true",), + copied_repo=repo, + sandbox_root=tmp_path, + exit_code=0, + elapsed_seconds=0, + kept=False, + allowed_env=(), + network="default", + evidence_note="", + result_file=result_file, + ) + + +def test_result_envelope_is_not_visible_before_streams_are_complete(monkeypatch, tmp_path): + """The envelope path becomes visible only after both streams are complete.""" + result_file = tmp_path / "evidence" / "result.json" + first_stream_started = threading.Event() + release_first_stream = threading.Event() + envelope_write_started = threading.Event() + release_envelope_write = threading.Event() + original_write_all = sandboxed_verify._write_all + + def pause_publication(file_descriptor, content): + if content == b"stdout": + first_stream_started.set() + assert release_first_stream.wait(timeout=2) + elif content == b"envelope": + envelope_write_started.set() + assert release_envelope_write.wait(timeout=2) + original_write_all(file_descriptor, content) + + monkeypatch.setattr(sandboxed_verify, "_write_all", pause_publication) + writer = threading.Thread( + target=sandboxed_verify._write_result_bundle, + args=(result_file, b"envelope", b"stdout", b"stderr"), + ) + writer.start() + assert first_stream_started.wait(timeout=2) + try: + assert not result_file.exists() + finally: + release_first_stream.set() + + assert envelope_write_started.wait(timeout=2) + try: + assert not result_file.exists() + finally: + release_envelope_write.set() + writer.join(timeout=2) + + assert not writer.is_alive() + assert result_file.read_bytes() == b"envelope" + assert result_file.with_name(result_file.name + ".stdout").read_bytes() == b"stdout" + assert result_file.with_name(result_file.name + ".stderr").read_bytes() == b"stderr" + + +def test_result_parent_relative_and_component_failures(monkeypatch, tmp_path): + """Relative traversal handles dot, parent, and creation-race branches.""" + + class Parent: + def __init__(self, *parts): + self.parts = parts + + def is_absolute(self): + return False + + def __str__(self): + return "/".join(self.parts) + + directory_fd = sandboxed_verify._open_result_parent(Parent(".")) + sandboxed_verify.os.close(directory_fd) + with pytest.raises(ValueError, match="parent is not a regular directory"): + sandboxed_verify._open_result_parent(Parent("..")) + + monkeypatch.chdir(tmp_path) + original_mkdir = sandboxed_verify.os.mkdir + + def racing_mkdir(path, mode=0o777, *, dir_fd=None): + original_mkdir(path, mode=mode, dir_fd=dir_fd) + raise FileExistsError + + monkeypatch.setattr(sandboxed_verify.os, "mkdir", racing_mkdir) + directory_fd = sandboxed_verify._open_result_parent(Path("raced")) + sandboxed_verify.os.close(directory_fd) + + original_open = sandboxed_verify.os.open + open_attempts = 0 + + def failing_retry(path, flags, mode=0o777, *, dir_fd=None): + nonlocal open_attempts + if path == "denied": + open_attempts += 1 + if open_attempts == 1: + raise FileNotFoundError + raise OSError("denied") + return original_open(path, flags, mode, dir_fd=dir_fd) + + monkeypatch.setattr(sandboxed_verify.os, "mkdir", original_mkdir) + monkeypatch.setattr(sandboxed_verify.os, "open", failing_retry) + with pytest.raises(ValueError, match="parent is not a regular directory"): + sandboxed_verify._open_result_parent(Path("denied")) + + +def test_result_bundle_rejects_existing_stream(tmp_path): + """A pre-existing stream prevents publication and remains untouched.""" + result_file = tmp_path / "result.json" + stdout_file = result_file.with_name(result_file.name + ".stdout") + stdout_file.write_bytes(b"occupied") + + with pytest.raises(ValueError, match="result bundle file already exists"): + sandboxed_verify._write_result_bundle( + result_file, + b"envelope", + b"stdout", + b"stderr", + ) + + assert stdout_file.read_bytes() == b"occupied" + assert not result_file.exists() + + +def test_result_bundle_cleans_raced_publication(monkeypatch, tmp_path): + """A final-name race removes only this writer's private bundle files.""" + result_file = tmp_path / "result.json" + original_unlink = sandboxed_verify.os.unlink + + def reject_publication(*_args, **_kwargs): + raise FileExistsError + + def unlink_then_report_missing(path, *, dir_fd=None): + original_unlink(path, dir_fd=dir_fd) + raise FileNotFoundError + + monkeypatch.setattr(sandboxed_verify.os, "link", reject_publication) + monkeypatch.setattr(sandboxed_verify.os, "unlink", unlink_then_report_missing) + with pytest.raises(ValueError, match="result file already exists"): + sandboxed_verify._write_result_bundle( + result_file, + b"envelope", + b"stdout", + b"stderr", + ) + + assert list(tmp_path.iterdir()) == [] + + +def test_result_file_rejects_symlinked_parent(tmp_path): + """The trusted handoff must not follow a caller-controlled parent symlink.""" + target = tmp_path / "target" + target.mkdir() + link = tmp_path / "link" + link.symlink_to(target, target_is_directory=True) + + with pytest.raises(ValueError, match="parent is not a regular directory"): + sandboxed_verify.emit_result( + command=("true",), + copied_repo=tmp_path, + sandbox_root=tmp_path, + exit_code=0, + elapsed_seconds=0, + kept=False, + allowed_env=(), + network="default", + evidence_note="", + result_file=link / "result.json", + ) + + +def test_result_file_rejects_existing_symlink_ancestor(tmp_path): + """An existing nested directory must not hide a symlink ancestor.""" + target = tmp_path / "target" + nested = target / "nested" + nested.mkdir(parents=True) + link = tmp_path / "link" + link.symlink_to(target, target_is_directory=True) + + with pytest.raises(ValueError, match="parent is not a regular directory"): + sandboxed_verify.emit_result( + command=("true",), + copied_repo=tmp_path, + sandbox_root=tmp_path, + exit_code=0, + elapsed_seconds=0, + kept=False, + allowed_env=(), + network="default", + evidence_note="", + result_file=link / "nested" / "result.json", + ) + + assert not (nested / "result.json").exists() + + +def test_result_bundle_preserves_large_binary_streams(tmp_path, capfdbinary): + """Dedicated handoff files preserve large invalid UTF-8 output exactly.""" + repo = tmp_path / "repo" + repo.mkdir() + result_file = tmp_path / "evidence" / "result.json" + stdout_bytes = (b"\xff\x00marker\n" * 131_072) + b"tail" + stderr_bytes = b"\xfejson:{not-json}\r\nend" + command = ( + "import sys; " + "sys.stdout.buffer.write((b'\\xff\\x00marker\\n' * 131072) + b'tail'); " + "sys.stderr.buffer.write(b'\\xfejson:{not-json}\\r\\nend')" + ) + + assert ( + sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--result-file", + str(result_file), + "--", + sys.executable, + "-c", + command, + ] + ) + == 0 + ) + capfdbinary.readouterr() + + assert result_file.with_name(result_file.name + ".stdout").read_bytes() == stdout_bytes + assert result_file.with_name(result_file.name + ".stderr").read_bytes() == stderr_bytes + + +def test_result_file_distinguishes_timeout_from_exit_124(tmp_path, capsys): + """A real timeout and a command exit 124 have different result states.""" + repo = tmp_path / "repo" + repo.mkdir() + timeout_result = tmp_path / "timeout" / "result.json" + exit_result = tmp_path / "exit" / "result.json" + + assert ( + sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--timeout", + "1", + "--result-file", + str(timeout_result), + "--", + sys.executable, + "-c", + "import time; print('partial', flush=True); time.sleep(2)", + ] + ) + == 124 + ) + assert ( + sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--result-file", + str(exit_result), + "--", + sys.executable, + "-c", + "raise SystemExit(124)", + ] + ) + == 124 + ) + capsys.readouterr() + + timeout_payload = json.loads( + timeout_result.read_text(encoding="utf-8").removeprefix(sandboxed_verify.RESULT_MARKER).strip() + ) + exit_payload = json.loads( + exit_result.read_text(encoding="utf-8").removeprefix(sandboxed_verify.RESULT_MARKER).strip() + ) + assert timeout_payload["result_state"] == "timed_out" + assert timeout_payload["timed_out"] is True + assert exit_payload["result_state"] == "completed" + assert exit_payload["timed_out"] is False + assert timeout_result.with_name(timeout_result.name + ".stdout").read_bytes() == b"partial\n" + + +def test_result_file_failure_is_bounded_and_always_cleans_sandbox(monkeypatch, tmp_path, capsys): + """A successful command with a handoff collision returns 125 and cleans up.""" + repo = tmp_path / "repo" + repo.mkdir() + sandbox = tmp_path / "sandbox" + result_file = tmp_path / "result.json" + result_file.write_text("occupied", encoding="utf-8") + + def make_sandbox(*, prefix): + assert prefix == "sandboxed-verify-" + sandbox.mkdir() + return str(sandbox) + + monkeypatch.setattr(sandboxed_verify.tempfile, "mkdtemp", make_sandbox) + + assert ( + sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--result-file", + str(result_file), + "--", + "true", + ] + ) + == 125 + ) + captured = capsys.readouterr() + assert "Traceback" not in captured.err + assert "result file already exists" in captured.err + assert not sandbox.exists() + + +@pytest.mark.parametrize( + ("command", "expected_exit_code"), + [ + ((sys.executable, "-c", "raise SystemExit(2)"), 2), + ((sys.executable, "-c", "raise SystemExit(124)"), 124), + ], +) +def test_result_file_failure_preserves_command_failure( + command, expected_exit_code, tmp_path, capsys +): + """Evidence rejection must not mask the command's nonzero exit status.""" + repo = tmp_path / "repo" + repo.mkdir() + result_file = tmp_path / "result.json" + result_file.write_text("occupied", encoding="utf-8") + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--result-file", + str(result_file), + "--", + *command, + ] + ) + + captured = capsys.readouterr() + assert exit_code == expected_exit_code + assert "result file already exists" in captured.err + + +def test_result_file_failure_preserves_timeout_status(monkeypatch, tmp_path, capsys): + """Evidence rejection must not mask the wrapper's timeout status.""" + repo = tmp_path / "repo" + repo.mkdir() + result_file = tmp_path / "result.json" + result_file.write_text("occupied", encoding="utf-8") + + def time_out(*_args, **_kwargs): + raise sandboxed_verify.subprocess.TimeoutExpired( + cmd=("slow-command",), timeout=1, output=b"partial-out", stderr=b"partial-err" + ) + + monkeypatch.setattr(sandboxed_verify, "run_command", time_out) + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--timeout", + "1", + "--result-file", + str(result_file), + "--", + "slow-command", + ] + ) + + captured = capsys.readouterr() + assert exit_code == 124 + assert "command timed out after 1s" in captured.err + assert "result file already exists" in captured.err + + def test_main_reports_a_clean_failure_when_the_workspace_copy_is_rejected(tmp_path, capsys): """A symlink-escape rejection from ``copy_workspace`` must not surface as an uncaught traceback. @@ -561,9 +1049,7 @@ def test_main_reports_a_clean_failure_when_the_workspace_copy_is_rejected(tmp_pa repo.mkdir() (repo / "escape-link").symlink_to(outside) - exit_code = sandboxed_verify.main( - ["--repo-root", str(repo), "--", "true"] - ) + exit_code = sandboxed_verify.main(["--repo-root", str(repo), "--", "true"]) captured = capsys.readouterr() assert exit_code == 125 @@ -575,6 +1061,36 @@ def test_main_reports_a_clean_failure_when_the_workspace_copy_is_rejected(tmp_pa assert payload["exit_code"] == 125 +def test_copy_rejection_is_recorded_in_trusted_result_bundle(tmp_path): + """A rejected source tree must still produce explicit trusted evidence.""" + outside = tmp_path / "outside-secret.txt" + outside.write_text("host-only-content", encoding="utf-8") + repo = tmp_path / "repo" + repo.mkdir() + (repo / "escape-link").symlink_to(outside) + result_file = tmp_path / "evidence" / "result.json" + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--result-file", + str(result_file), + "--", + "true", + ] + ) + + assert exit_code == 125 + wrapper_result = json.loads( + result_file.read_text(encoding="utf-8").removeprefix(sandboxed_verify.RESULT_MARKER).strip() + ) + assert wrapper_result["result_state"] == "copy_rejected" + assert wrapper_result["timed_out"] is False + assert result_file.with_name(result_file.name + ".stdout").read_bytes() == b"" + assert result_file.with_name(result_file.name + ".stderr").read_bytes() == b"" + + def test_parse_args_rejects_invalid_inputs(): """The CLI rejects invocations without a command or with invalid options.""" with pytest.raises(SystemExit): @@ -589,7 +1105,19 @@ def test_module_main_entrypoint(monkeypatch, tmp_path): """The script entrypoint exits with the verification command status.""" repo = tmp_path / "repo" repo.mkdir() - monkeypatch.setattr(sys, "argv", ["sandboxed_verify.py", "--repo-root", str(repo), "--", sys.executable, "-c", "raise SystemExit(0)"]) + monkeypatch.setattr( + sys, + "argv", + [ + "sandboxed_verify.py", + "--repo-root", + str(repo), + "--", + sys.executable, + "-c", + "raise SystemExit(0)", + ], + ) module = sys.modules.pop("scripts.ci.sandboxed_verify", None) with pytest.raises(SystemExit) as exc_info: try: