diff --git a/CHANGELOG.md b/CHANGELOG.md index b27b8245c..6c7978fa5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,11 @@ Semantic Versioning where the repository publishes a release. ### Changed +- Route sandboxed verification commands through the bounded subprocess layer, + reject copied-tree symlinks that leave the sandbox, and publish distinct + output-limit, unsupported-platform, missing/non-executable command, and + path-boundary evidence without exposing host paths or uncaught tracebacks. + - Emit completed repository pull-list requests as they finish in the five-minute agent-mention sweep, while retaining the four-worker ceiling, rotation, and exact-name dispatch ledger, so one slow repository cannot hide ready sibling diff --git a/docs/doctoring/sandboxed-output-resource-bounds.md b/docs/doctoring/sandboxed-output-resource-bounds.md index 83bbf27b2..d9b7d1b69 100644 --- a/docs/doctoring/sandboxed-output-resource-bounds.md +++ b/docs/doctoring/sandboxed-output-resource-bounds.md @@ -56,11 +56,21 @@ A truncation marker is included inside, not in addition to, the declared retaine ## Deferred consumer integration -This first stack layer does not change `sandboxed_verify.py` or -`sandboxed_web_e2e.py`. The next layers adopt the library for short-lived -verification commands and long-running service evidence respectively. Keeping -those integrations separate prevents a shared process primitive, workspace -symlink policy, and E2E result schema from becoming one monolithic review. +The second stack layer adopts the library in `sandboxed_verify.py` for +short-lived verification commands. Long-running `sandboxed_web_e2e.py` service +evidence remains a separate layer. Keeping those integrations separate prevents +a shared process primitive, workspace symlink policy, and E2E result schema from +becoming one monolithic review. + +The verification consumer maps an executable lookup failure to exit code `127`, +publishes its normal machine-readable failed result, and tells the operator to +install the executable or correct `PATH`. Provider and host path details do not +escape through an uncaught traceback. + +A path that exists but is a directory or lacks execute permission is distinct: +the consumer returns exit code `126` and tells the operator to select an +executable file or correct its permissions. The stable failed result remains +available without exposing the operating-system exception traceback. ## Security and availability properties diff --git a/docs/doctoring/sandboxed-verification-symlink-boundary.md b/docs/doctoring/sandboxed-verification-symlink-boundary.md new file mode 100644 index 000000000..ef1c646bf --- /dev/null +++ b/docs/doctoring/sandboxed-verification-symlink-boundary.md @@ -0,0 +1,61 @@ +# Sandboxed verification symlink boundary + +## Incident + +The review verifier copied an untrusted checkout with `shutil.copytree(..., +symlinks=True)`. That preserves symbolic links rather than copying their +targets. A pull request could therefore add an absolute link, or a relative +link containing enough parent traversal, that a verification command followed +outside the temporary repository. Environment scrubbing did not close that +filesystem boundary. + +## Decision + +After applying the copy ignore policy, and before running the untrusted command, +the verifier walks the exact copied tree without following directory links and +validates every symbolic link. An absolute target is rejected because the copied +link would still point at a host path. A relative target is accepted only when +its fully resolved path remains beneath the copied repository. Safe internal +relative links remain links so project semantics are preserved. Links under +ignored paths such as `node_modules` never enter the copy and are not evaluated. + +The validation happens before the untrusted command starts. Rejection is +fail-closed with stable exit code `122`, `path_boundary_rejected=true` in the +machine-readable result, and a generic diagnostic that does not disclose the +resolved host target. It produces no verification success evidence. This is +filesystem containment, not an operating-system sandbox claim; the existing +network-mode field remains evidence metadata rather than enforcement. + +The walk is intentionally a pre-execution copy validation, not a continuous +kernel-enforced filesystem sandbox. A command may create a new symlink after +validation. The wrapper therefore does not claim to contain a hostile process +that can mutate its copied workspace during execution; that stronger boundary +belongs to the surrounding runner or container. The control closes exposure +introduced by attacker-supplied links already present in the copied checkout. +Repository-internal symlink cycles remain inside the boundary but can make a +later verification tool that follows links recurse. Projects must remove such +cycles or configure the verification tool not to follow them; containment does +not imply that every internal graph is operationally valid. + +## Test-first evidence + +`tests/test_sandboxed_verify_symlink_boundary.py` first reproduced the defect: +an escaping repository link copied successfully instead of raising. The +accepted tests require rejection of both relative traversal and absolute links, +including an absolute link back into the original checkout, while retaining a +safe repository-internal relative link. + +## Failure, recovery, and rollback + +Repositories that intentionally contain absolute or escaping links must replace +them with bounded relative links before review verification. A rollback is safe +only if an independently reviewed replacement proves that no path available to +the copied command can resolve outside the copy. Dereferencing untrusted links +during the copy is not an acceptable fallback because it can read the external +target while constructing the sandbox. + +## APA 7th reference + +Python Software Foundation. (2026). *shutil—High-level file operations* +(Python 3.14.6 documentation). Retrieved August 24, 2026, from +https://docs.python.org/3.14/library/shutil.html#shutil.copytree diff --git a/scripts/ci/sandboxed_verify.py b/scripts/ci/sandboxed_verify.py index aace18d45..c5218b8ed 100644 --- a/scripts/ci/sandboxed_verify.py +++ b/scripts/ci/sandboxed_verify.py @@ -14,6 +14,11 @@ from collections.abc import Sequence from pathlib import Path +if __package__ in (None, ""): + sys.path.insert(0, str(Path(__file__).resolve().parents[2])) + +from scripts.ci import bounded_subprocess + DEFAULT_IGNORE = ( ".git", @@ -56,9 +61,20 @@ "PYTHONPATH", ) RESULT_MARKER = "SANDBOXED_VERIFY_RESULT" +PATH_BOUNDARY_EXIT_CODE = 122 +COMMAND_NOT_EXECUTABLE_EXIT_CODE = 126 +COMMAND_NOT_FOUND_EXIT_CODE = 127 ENV_NAME_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") +class RepositoryPathBoundaryError(ValueError): + """Report a copied repository link that escapes its sandbox boundary.""" + + +class RepositoryRootError(ValueError): + """Report that the requested repository root cannot be copied.""" + + def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: """Parse CLI arguments for the sandboxed verification wrapper.""" parser = argparse.ArgumentParser( @@ -69,6 +85,12 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: ) parser.add_argument("--repo-root", default=".", help="Repository root to copy into the sandbox.") parser.add_argument("--timeout", type=int, default=300, help="Command timeout in seconds.") + parser.add_argument( + "--output-limit-bytes", + type=int, + default=bounded_subprocess.DEFAULT_COMMAND_OUTPUT_LIMIT_BYTES, + help="Maximum retained stdout and stderr bytes per stream.", + ) parser.add_argument( "--keep-sandbox", action="store_true", @@ -106,6 +128,13 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: parser.error("provide a verification command after --") if args.timeout <= 0: parser.error("--timeout must be positive") + try: + args.output_limit_bytes = bounded_subprocess.validate_output_limit( + args.output_limit_bytes, + "--output-limit-bytes", + ) + except ValueError as error: + parser.error(str(error)) for name in args.allow_env: if not ENV_NAME_RE.match(name): parser.error(f"--allow-env must be an environment variable name: {name}") @@ -138,29 +167,62 @@ def scrubbed_env(sandbox_root: Path, allow_env: Sequence[str] = ()) -> dict[str, return env +def validate_repository_symlinks(source: Path) -> None: + """Reject symlinks that could escape the copied repository sandbox. + + Relative links are retained only when their resolved target stays beneath + ``source``. Absolute links are rejected even when they currently name a + path beneath ``source`` because preserving them would point the sandboxed + command back at the original checkout instead of the isolated copy. + """ + source_root = source.resolve(strict=True) + for current_root, directory_names, file_names in os.walk(source_root, followlinks=False): + current = Path(current_root) + for name in (*directory_names, *file_names): + candidate = current / name + if not candidate.is_symlink(): + continue + target = Path(os.readlink(candidate)) + if target.is_absolute(): + raise RepositoryPathBoundaryError( + f"symlink escapes repository verification sandbox via absolute target: " + f"{candidate} -> {target}" + ) + resolved_target = (candidate.parent / target).resolve(strict=False) + try: + resolved_target.relative_to(source_root) + except ValueError as exc: + raise RepositoryPathBoundaryError( + f"symlink escapes repository verification sandbox: {candidate} -> {target}" + ) from exc + + def copy_workspace(repo_root: Path, sandbox_root: Path, extra_ignores: Sequence[str]) -> Path: """Copy the repository into the sandbox and return the copied root.""" source = repo_root.resolve() if not source.is_dir(): - raise ValueError(f"repo root is not a directory: {source}") + raise RepositoryRootError(f"repo root is not a directory: {source}") destination = sandbox_root / "repo" ignore = shutil.ignore_patterns(*(DEFAULT_IGNORE + tuple(extra_ignores))) shutil.copytree(source, destination, ignore=ignore, symlinks=True) + validate_repository_symlinks(destination) return destination -def run_command(command: Sequence[str], cwd: Path, env: dict[str, str], timeout: int) -> subprocess.CompletedProcess[str]: - """Run the verification command and capture output for review evidence.""" - return subprocess.run( - list(command), +def run_command( + command: Sequence[str], + cwd: Path, + env: dict[str, str], + timeout: int, + output_limit_bytes: int = bounded_subprocess.DEFAULT_COMMAND_OUTPUT_LIMIT_BYTES, +) -> bounded_subprocess.BoundedCompletedProcess: + """Run one verification command with continuously drained bounded output.""" + return bounded_subprocess.run_bounded_command( + command, cwd=cwd, env=env, - text=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, timeout=timeout, - check=False, - shell=False, + evidence_limit_bytes=output_limit_bytes, ) @@ -184,6 +246,10 @@ def emit_result( allowed_env: Sequence[str], network: str, evidence_note: str, + output_limit_bytes: int = bounded_subprocess.DEFAULT_COMMAND_OUTPUT_LIMIT_BYTES, + output_limited: bool = False, + output_limit_unsupported: bool = False, + path_boundary_rejected: bool = False, ) -> None: """Print a machine-readable execution evidence summary.""" payload = { @@ -194,9 +260,14 @@ def emit_result( "evidence_note": evidence_note, "exit_code": exit_code, "network": network, + "output_limit_bytes": output_limit_bytes, + "output_limited": output_limited, + "output_limit_unsupported": output_limit_unsupported, + "path_boundary_rejected": path_boundary_rejected, "sandbox": str(sandbox_root) if kept else "(removed)", "sandboxed": True, } + print() print(f"{RESULT_MARKER} {json.dumps(payload, sort_keys=True)}") @@ -206,9 +277,30 @@ def main(argv: Sequence[str] | None = None) -> int: sandbox = Path(tempfile.mkdtemp(prefix="sandboxed-verify-")) start = time.monotonic() exit_code = 1 + output_limited = False + output_limit_unsupported = False + path_boundary_rejected = False copied_repo = sandbox / "repo" try: - copied_repo = copy_workspace(Path(args.repo_root), sandbox, args.ignore) + try: + copied_repo = copy_workspace(Path(args.repo_root), sandbox, args.ignore) + except RepositoryPathBoundaryError: + path_boundary_rejected = True + copied_repo = Path("(not-created)") + print( + "sandboxed-verify: repository path boundary rejected", + file=sys.stderr, + ) + exit_code = PATH_BOUNDARY_EXIT_CODE + return exit_code + except RepositoryRootError: + copied_repo = Path("(not-created)") + print( + "sandboxed-verify: repository root is not a directory", + file=sys.stderr, + ) + exit_code = 1 + 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)}") @@ -217,12 +309,52 @@ def main(argv: Sequence[str] | None = None) -> int: if args.network != "default": print(f"sandboxed-verify: network={args.network}") try: - completed = run_command(args.command, copied_repo, env, args.timeout) + completed = run_command( + args.command, + copied_repo, + env, + args.timeout, + args.output_limit_bytes, + ) if completed.stdout: print(completed.stdout, end="") if completed.stderr: print(completed.stderr, end="", file=sys.stderr) - exit_code = completed.returncode + output_limited = completed.output_limited + if output_limited: + print( + "sandboxed-verify: command output exceeded " + f"{args.output_limit_bytes} bytes", + file=sys.stderr, + ) + exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE + else: + exit_code = completed.returncode + except FileNotFoundError: + print( + "sandboxed-verify: install the executable or correct command PATH", + file=sys.stderr, + ) + exit_code = COMMAND_NOT_FOUND_EXIT_CODE + except (PermissionError, IsADirectoryError): + print( + "sandboxed-verify: select an executable file or correct its permissions", + file=sys.stderr, + ) + exit_code = COMMAND_NOT_EXECUTABLE_EXIT_CODE + except bounded_subprocess.OutputLimitUnsupportedError: + output_limit_unsupported = True + print( + "sandboxed-verify: bounded child output is unavailable on this platform", + file=sys.stderr, + ) + exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE + except RuntimeError: + print( + "sandboxed-verify: bounded output capture failed", + file=sys.stderr, + ) + exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE except subprocess.TimeoutExpired as exc: stdout = timeout_output_text(exc.stdout) stderr = timeout_output_text(exc.stderr) @@ -230,6 +362,7 @@ def main(argv: Sequence[str] | None = None) -> int: print(stdout, end="" if stdout.endswith("\n") else "\n") if stderr: print(stderr, end="" if stderr.endswith("\n") else "\n", file=sys.stderr) + output_limited = bool(getattr(exc, "output_limited", False)) print(f"sandboxed-verify: command timed out after {args.timeout}s", file=sys.stderr) exit_code = 124 return exit_code @@ -245,6 +378,10 @@ def main(argv: Sequence[str] | None = None) -> int: allowed_env=args.allow_env, network=args.network, evidence_note=args.evidence_note, + output_limit_bytes=args.output_limit_bytes, + output_limited=output_limited, + output_limit_unsupported=output_limit_unsupported, + path_boundary_rejected=path_boundary_rejected, ) if not args.keep_sandbox: shutil.rmtree(sandbox, ignore_errors=True) diff --git a/tests/test_repository_branch_coverage_execution_sandboxes.py b/tests/test_repository_branch_coverage_execution_sandboxes.py index f8912272a..65de0a774 100644 --- a/tests/test_repository_branch_coverage_execution_sandboxes.py +++ b/tests/test_repository_branch_coverage_execution_sandboxes.py @@ -131,7 +131,11 @@ def test_sandboxed_verify_timeout_with_no_streams_is_bounded( repo.mkdir() def timeout_runner( - command: list[str], _cwd: Path, _env: dict[str, str], timeout: int + command: list[str], + _cwd: Path, + _env: dict[str, str], + timeout: int, + _output_limit_bytes: int, ) -> subprocess.CompletedProcess[str]: raise subprocess.TimeoutExpired(command, timeout, output=None, stderr=None) diff --git a/tests/test_sandboxed_verify.py b/tests/test_sandboxed_verify.py index c711f3489..25111acca 100644 --- a/tests/test_sandboxed_verify.py +++ b/tests/test_sandboxed_verify.py @@ -9,6 +9,17 @@ from scripts.ci import sandboxed_verify +def test_direct_file_import_bootstraps_the_repository_path() -> None: + """Direct-file loading covers the installed workflow entrypoint boundary.""" + + namespace = runpy.run_path( + str(Path(sandboxed_verify.__file__)), + run_name="sandboxed_verify_import_probe", + ) + + assert namespace["RESULT_MARKER"] == sandboxed_verify.RESULT_MARKER + + def test_scrubbed_env_uses_sandbox_paths_and_drops_secrets(monkeypatch, tmp_path): """Sandbox env keeps basic runtime variables but drops credentials.""" monkeypatch.setenv("PATH", "/usr/bin") @@ -80,6 +91,29 @@ def test_copy_workspace_rejects_missing_repo_root(tmp_path): sandboxed_verify.copy_workspace(tmp_path / "missing", tmp_path / "sandbox", []) +def test_main_reports_invalid_repo_root_without_boundary_evidence(tmp_path, capsys): + """An invalid root is a generic input failure, not a symlink rejection.""" + missing = tmp_path / "host-secret-root" + + exit_code = sandboxed_verify.main( + ["--repo-root", str(missing), "--", "verify"] + ) + captured = capsys.readouterr() + result_line = [ + line + for line in captured.out.splitlines() + if line.startswith(sandboxed_verify.RESULT_MARKER) + ][-1] + payload = json.loads(result_line.removeprefix(sandboxed_verify.RESULT_MARKER).strip()) + + assert exit_code == 1 + assert payload["exit_code"] == 1 + assert payload["path_boundary_rejected"] is False + assert "repository root is not a directory" in captured.err + assert str(missing) not in captured.err + assert "Traceback" not in captured.err + + def test_timeout_output_text_normalizes_subprocess_payloads(): """Timeout output normalization handles subprocess bytes and missing streams.""" assert sandboxed_verify.timeout_output_text(None) == "" diff --git a/tests/test_sandboxed_verify_output_limits.py b/tests/test_sandboxed_verify_output_limits.py new file mode 100644 index 000000000..23faf2df4 --- /dev/null +++ b/tests/test_sandboxed_verify_output_limits.py @@ -0,0 +1,266 @@ +"""Real-command contracts for sandboxed verification output ceilings.""" + +from __future__ import annotations + +import json +import os +import sys +from pathlib import Path + +import pytest + +from scripts.ci import bounded_subprocess as bounded +from scripts.ci import sandboxed_verify + + +def _result_payload(output: str) -> dict[str, object]: + """Parse the final sandbox result marker from captured standard output.""" + + marker = f"{sandboxed_verify.RESULT_MARKER} " + result_line = next( + line for line in reversed(output.splitlines()) if line.startswith(marker) + ) + return json.loads(result_line.removeprefix(marker)) + + +def _repository(tmp_path: Path) -> Path: + """Create one minimal repository directory accepted by the copy boundary.""" + + repository = tmp_path / "repository" + repository.mkdir() + (repository / "README.md").write_text("sandbox fixture\n", encoding="utf-8") + return repository + + +def test_normal_command_preserves_output_and_reports_declared_limit( + tmp_path: Path, + capsys, +) -> None: + """Ordinary Unicode output remains visible with deterministic limit evidence.""" + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--output-limit-bytes", + "4096", + "--", + sys.executable, + "-c", + "import sys; print('정상'); print('경고', file=sys.stderr)", + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == 0 + assert "정상" in captured.out + assert "경고" in captured.err + assert payload["output_limit_bytes"] == 4096 + assert payload["output_limited"] is False + + +@pytest.mark.parametrize("descriptor", [1, 2]) +def test_excessive_stdout_or_stderr_returns_resource_limit_code( + tmp_path: Path, + capsys, + descriptor: int, +) -> None: + """A real output flood is bounded and classified as exit 123.""" + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--output-limit-bytes", + "4096", + "--", + sys.executable, + "-c", + ( + "import os\n" + f"descriptor={descriptor}\n" + "chunk=b'x'*1024\n" + "while True:\n" + " os.write(descriptor, chunk)\n" + ), + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + combined = captured.out + captured.err + + assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE + assert bounded.TRUNCATION_MARKER.strip() in combined + assert "output exceeded 4096 bytes" in captured.err + assert payload["output_limited"] is True + assert len(combined.encode("utf-8")) < 20_000 + + +def test_timeout_retains_precedence_and_bounded_partial_output( + tmp_path: Path, + capsys, +) -> None: + """A timeout remains exit 124 while its partial output stays byte-bounded.""" + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--timeout", + "1", + "--output-limit-bytes", + "4096", + "--", + sys.executable, + "-c", + "import os,time; os.write(1,b'before\\n'); time.sleep(30)", + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == 124 + assert "before" in captured.out + assert "timed out after 1s" in captured.err + assert payload["output_limited"] is False + + +def test_stuck_capture_returns_bounded_failure_without_traceback( + monkeypatch, + tmp_path: Path, + capsys, +) -> None: + """A stuck reader becomes stable resource evidence instead of a traceback.""" + repository = _repository(tmp_path) + monkeypatch.setattr( + sandboxed_verify, + "run_command", + lambda *args, **kwargs: (_ for _ in ()).throw( + RuntimeError("host descriptor detail") + ), + ) + + exit_code = sandboxed_verify.main( + ["--repo-root", str(repository), "--", "verify"] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE + assert payload["output_limited"] is False + assert "bounded output capture failed" in captured.err + assert "host descriptor detail" not in captured.err + assert "Traceback" not in captured.err + + +def test_missing_executable_returns_stable_failed_evidence( + tmp_path: Path, + capsys, +) -> None: + """A missing command gives an actionable result instead of a traceback.""" + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--", + "missing-verification-executable", + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == sandboxed_verify.COMMAND_NOT_FOUND_EXIT_CODE + assert payload["exit_code"] == sandboxed_verify.COMMAND_NOT_FOUND_EXIT_CODE + assert "install the executable or correct command PATH" in captured.err + assert "Traceback" not in captured.err + + +@pytest.mark.parametrize("candidate_kind", ["file", "directory"]) +def test_non_executable_command_returns_stable_failed_evidence( + tmp_path: Path, + capsys, + candidate_kind: str, +) -> None: + """A present but unusable command tells the operator how to recover.""" + + candidate = tmp_path / "verification-candidate" + if candidate_kind == "file": + candidate.write_text("not executable\n", encoding="utf-8") + else: + candidate.mkdir() + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--", + str(candidate), + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == sandboxed_verify.COMMAND_NOT_EXECUTABLE_EXIT_CODE + assert payload["exit_code"] == sandboxed_verify.COMMAND_NOT_EXECUTABLE_EXIT_CODE + assert "select an executable file or correct its permissions" in captured.err + assert "Traceback" not in captured.err + + +def test_unsupported_resource_limit_fails_closed( + monkeypatch, + tmp_path: Path, + capsys, +) -> None: + """The wrapper never falls back to unbounded pipes on unsupported platforms.""" + + def fail_run(*args, **kwargs): + del args, kwargs + raise bounded.OutputLimitUnsupportedError("unsupported") + + monkeypatch.setattr(sandboxed_verify, "run_command", fail_run) + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--output-limit-bytes", + "4096", + "--", + sys.executable, + "-c", + "print('never runs')", + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE + assert "bounded child output is unavailable" in captured.err + assert payload["output_limited"] is False + assert payload["output_limit_unsupported"] is True + + +@pytest.mark.parametrize( + "value", + ["4095", str(bounded.MAXIMUM_OUTPUT_LIMIT_BYTES + 1)], +) +def test_cli_rejects_output_budgets_outside_supported_range( + tmp_path: Path, + value: str, +) -> None: + """Unsafe output budgets fail argument parsing before workspace execution.""" + + repository = _repository(tmp_path) + with pytest.raises(SystemExit) as raised: + sandboxed_verify.parse_args( + [ + "--repo-root", + str(repository), + "--output-limit-bytes", + value, + "--", + os.devnull, + ] + ) + assert raised.value.code == 2 diff --git a/tests/test_sandboxed_verify_symlink_boundary.py b/tests/test_sandboxed_verify_symlink_boundary.py new file mode 100644 index 000000000..a5d34276e --- /dev/null +++ b/tests/test_sandboxed_verify_symlink_boundary.py @@ -0,0 +1,129 @@ +"""Security contracts for sandboxed verification symlink handling.""" + +import json + +import subprocess +from pathlib import Path + +import pytest + +from scripts.ci import sandboxed_verify + + +def test_copy_workspace_rejects_symlink_that_escapes_repository(tmp_path: Path) -> None: + """An untrusted repository symlink must not expose a host-side path.""" + repo = tmp_path / "repo" + repo.mkdir() + outside = tmp_path / "runner-secret.txt" + outside.write_text("host-only", encoding="utf-8") + (repo / "escape").symlink_to("../runner-secret.txt") + + with pytest.raises(ValueError, match="symlink escapes repository"): + sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", ()) + + +def test_copy_workspace_rejects_absolute_symlink_into_original_checkout( + tmp_path: Path, +) -> None: + """An absolute link must not reconnect the copy to its source checkout.""" + repo = tmp_path / "repo" + repo.mkdir() + target = repo / "target.txt" + target.write_text("mutable source", encoding="utf-8") + (repo / "absolute-alias.txt").symlink_to(target) + + with pytest.raises(ValueError, match="absolute target"): + sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", ()) + + +def test_copy_workspace_preserves_repository_internal_symlink(tmp_path: Path) -> None: + """A relative symlink whose resolved target stays in the repository is safe.""" + repo = tmp_path / "repo" + repo.mkdir() + (repo / "target.txt").write_text("review me", encoding="utf-8") + (repo / "alias.txt").symlink_to("target.txt") + + copied = sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", ()) + + assert (copied / "alias.txt").is_symlink() + assert (copied / "alias.txt").read_text(encoding="utf-8") == "review me" + + +def test_copy_workspace_does_not_validate_ignored_symlinks(tmp_path: Path) -> None: + """A link excluded from the copy is outside the command's path boundary.""" + repo = tmp_path / "repo" + ignored = repo / "node_modules" + ignored.mkdir(parents=True) + outside = tmp_path / "package-cache" + outside.mkdir() + (ignored / "external-package").symlink_to(outside, target_is_directory=True) + + copied = sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", ()) + + assert not (copied / "node_modules").exists() + + +def test_main_classifies_repository_path_boundary_without_traceback( + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """A rejected repository link must emit stable, non-sensitive evidence.""" + repo = tmp_path / "repo" + repo.mkdir() + sensitive_target = tmp_path / "runner-secret.txt" + sensitive_target.write_text("host-only", encoding="utf-8") + (repo / "escape").symlink_to(sensitive_target) + + exit_code = sandboxed_verify.main( + ["--repo-root", str(repo), "--", "verify"] + ) + captured = capsys.readouterr() + lines = [ + line + for line in captured.out.splitlines() + if line.startswith(sandboxed_verify.RESULT_MARKER) + ] + payload = json.loads(lines[0].removeprefix(sandboxed_verify.RESULT_MARKER)) + + assert exit_code == sandboxed_verify.PATH_BOUNDARY_EXIT_CODE + assert payload["exit_code"] == sandboxed_verify.PATH_BOUNDARY_EXIT_CODE + assert payload["path_boundary_rejected"] is True + assert payload["cwd"] == "(not-created)" + assert "repository path boundary rejected" in captured.err + assert str(sensitive_target) not in captured.err + assert str(repo) not in captured.err + assert "Traceback" not in captured.err + + +def test_timeout_without_partial_streams_still_emits_failed_evidence( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """A silent timeout must retain deterministic fail-closed evidence.""" + repo = tmp_path / "repo" + repo.mkdir() + + def timeout_runner(*_args: object, **_kwargs: object) -> None: + raise subprocess.TimeoutExpired(["verify"], 1) + + monkeypatch.setattr(sandboxed_verify, "run_command", timeout_runner) + + assert ( + sandboxed_verify.main( + ["--repo-root", str(repo), "--timeout", "1", "--", "verify"] + ) + == 124 + ) + lines = [ + line + for line in capsys.readouterr().out.splitlines() + if line.startswith(sandboxed_verify.RESULT_MARKER) + ] + assert len(lines) == 1 + payload = json.loads(lines[0].removeprefix(sandboxed_verify.RESULT_MARKER)) + assert payload["exit_code"] == 124 + assert payload["output_limit_bytes"] == 1_048_576 + assert payload["output_limited"] is False + assert payload["output_limit_unsupported"] is False + assert payload["sandboxed"] is True