From faca1f145f237ce7b561218d707a40ae33471b88 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 12:17:22 +0900 Subject: [PATCH 01/15] test(security): expose sandbox symlink escape --- .../test_sandboxed_verify_symlink_boundary.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 tests/test_sandboxed_verify_symlink_boundary.py diff --git a/tests/test_sandboxed_verify_symlink_boundary.py b/tests/test_sandboxed_verify_symlink_boundary.py new file mode 100644 index 000000000..ea953f7e4 --- /dev/null +++ b/tests/test_sandboxed_verify_symlink_boundary.py @@ -0,0 +1,32 @@ +"""Security contracts for sandboxed verification symlink handling.""" + +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(outside) + + with pytest.raises(ValueError, match="symlink escapes repository"): + 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" From 856d92f529f55943a32bc34f19953ca5d38e1ee9 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 12:18:46 +0900 Subject: [PATCH 02/15] fix(security): reject sandbox-escaping symlinks --- scripts/ci/sandboxed_verify.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/scripts/ci/sandboxed_verify.py b/scripts/ci/sandboxed_verify.py index aace18d45..f27c0c102 100644 --- a/scripts/ci/sandboxed_verify.py +++ b/scripts/ci/sandboxed_verify.py @@ -138,11 +138,42 @@ 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 ValueError( + 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 ValueError( + 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}") + validate_repository_symlinks(source) destination = sandbox_root / "repo" ignore = shutil.ignore_patterns(*(DEFAULT_IGNORE + tuple(extra_ignores))) shutil.copytree(source, destination, ignore=ignore, symlinks=True) From ea6ac85b4ab865e10bfa3aa1aac608e5975fc2ea Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 12:19:00 +0900 Subject: [PATCH 03/15] test(security): cover absolute and relative symlink escapes --- tests/test_sandboxed_verify_symlink_boundary.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/test_sandboxed_verify_symlink_boundary.py b/tests/test_sandboxed_verify_symlink_boundary.py index ea953f7e4..f5c645cde 100644 --- a/tests/test_sandboxed_verify_symlink_boundary.py +++ b/tests/test_sandboxed_verify_symlink_boundary.py @@ -13,12 +13,26 @@ def test_copy_workspace_rejects_symlink_that_escapes_repository(tmp_path: Path) repo.mkdir() outside = tmp_path / "runner-secret.txt" outside.write_text("host-only", encoding="utf-8") - (repo / "escape").symlink_to(outside) + (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" From c0aefd32de74d43292625afd65b5e524e67cee90 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 12:19:46 +0900 Subject: [PATCH 04/15] docs(security): record verifier symlink boundary --- ...sandboxed-verification-symlink-boundary.md | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 docs/doctoring/sandboxed-verification-symlink-boundary.md diff --git a/docs/doctoring/sandboxed-verification-symlink-boundary.md b/docs/doctoring/sandboxed-verification-symlink-boundary.md new file mode 100644 index 000000000..aa017f21a --- /dev/null +++ b/docs/doctoring/sandboxed-verification-symlink-boundary.md @@ -0,0 +1,47 @@ +# 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 + +Before copying, the verifier walks the source tree without following directory +links and validates every symbolic link. An absolute target is rejected because +the copied link would still point at the original host path. A relative target +is accepted only when its fully resolved path remains beneath the source +repository. Safe repository-internal relative links remain links in the copied +tree so project semantics are preserved. + +The validation happens before the untrusted command starts. Rejection is +fail-closed and 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. + +## 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 documentation). Retrieved August 11, 2026, from +https://docs.python.org/3.14/library/shutil.html#shutil.copytree From ee903b875a0794f196060335231d9165e45000ea Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 12:21:38 +0900 Subject: [PATCH 05/15] test(security): close verifier branch coverage --- .../test_sandboxed_verify_symlink_boundary.py | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_sandboxed_verify_symlink_boundary.py b/tests/test_sandboxed_verify_symlink_boundary.py index f5c645cde..01dccb888 100644 --- a/tests/test_sandboxed_verify_symlink_boundary.py +++ b/tests/test_sandboxed_verify_symlink_boundary.py @@ -1,5 +1,6 @@ """Security contracts for sandboxed verification symlink handling.""" +import subprocess from pathlib import Path import pytest @@ -44,3 +45,24 @@ def test_copy_workspace_preserves_repository_internal_symlink(tmp_path: Path) -> assert (copied / "alias.txt").is_symlink() assert (copied / "alias.txt").read_text(encoding="utf-8") == "review me" + + +def test_timeout_without_partial_streams_still_emits_failed_evidence( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> 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 + ) From 6f597306a7414e4ab027af42c8d8672f8da8de39 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 12:37:37 +0900 Subject: [PATCH 06/15] test(security): expose ignored-link false positive --- tests/test_sandboxed_verify_symlink_boundary.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_sandboxed_verify_symlink_boundary.py b/tests/test_sandboxed_verify_symlink_boundary.py index 01dccb888..1cd1739c3 100644 --- a/tests/test_sandboxed_verify_symlink_boundary.py +++ b/tests/test_sandboxed_verify_symlink_boundary.py @@ -47,6 +47,20 @@ def test_copy_workspace_preserves_repository_internal_symlink(tmp_path: Path) -> 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_timeout_without_partial_streams_still_emits_failed_evidence( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, From ea2672a4151aed6228e963d81f6d90e70d4a8fc7 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 12:38:17 +0900 Subject: [PATCH 07/15] fix(security): validate exact sandbox copy --- scripts/ci/sandboxed_verify.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ci/sandboxed_verify.py b/scripts/ci/sandboxed_verify.py index f27c0c102..97b9f31f1 100644 --- a/scripts/ci/sandboxed_verify.py +++ b/scripts/ci/sandboxed_verify.py @@ -173,10 +173,10 @@ def copy_workspace(repo_root: Path, sandbox_root: Path, extra_ignores: Sequence[ source = repo_root.resolve() if not source.is_dir(): raise ValueError(f"repo root is not a directory: {source}") - validate_repository_symlinks(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 From 46b223795872a4f684088be2e94c6b32be552095 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 12:38:43 +0900 Subject: [PATCH 08/15] docs(security): scope validation to copied tree --- .../sandboxed-verification-symlink-boundary.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/doctoring/sandboxed-verification-symlink-boundary.md b/docs/doctoring/sandboxed-verification-symlink-boundary.md index aa017f21a..0fa28c7f0 100644 --- a/docs/doctoring/sandboxed-verification-symlink-boundary.md +++ b/docs/doctoring/sandboxed-verification-symlink-boundary.md @@ -11,12 +11,13 @@ filesystem boundary. ## Decision -Before copying, the verifier walks the source tree without following directory -links and validates every symbolic link. An absolute target is rejected because -the copied link would still point at the original host path. A relative target -is accepted only when its fully resolved path remains beneath the source -repository. Safe repository-internal relative links remain links in the copied -tree so project semantics are preserved. +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 and produces no verification success evidence. This is filesystem From b4547a55a732f3f2f6b64e5924bca11e10871599 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 13:14:27 +0900 Subject: [PATCH 09/15] test(security): expose unbounded sandbox output --- tests/test_bounded_subprocess.py | 275 ++++++++++ ...test_bounded_subprocess_capture_startup.py | 160 ++++++ tests/test_bounded_subprocess_contract.py | 345 +++++++++++++ ...ory_branch_coverage_execution_sandboxes.py | 13 +- ...ndboxed_entrypoint_and_cleanup_coverage.py | 97 ++++ .../test_sandboxed_service_capture_startup.py | 78 +++ tests/test_sandboxed_verify_output_limits.py | 180 +++++++ tests/test_sandboxed_web_e2e.py | 53 +- .../test_sandboxed_web_e2e_branch_contract.py | 469 ++++++++++++++++++ tests/test_sandboxed_web_e2e_output_limits.py | 302 +++++++++++ 10 files changed, 1953 insertions(+), 19 deletions(-) create mode 100644 tests/test_bounded_subprocess.py create mode 100644 tests/test_bounded_subprocess_capture_startup.py create mode 100644 tests/test_bounded_subprocess_contract.py create mode 100644 tests/test_sandboxed_entrypoint_and_cleanup_coverage.py create mode 100644 tests/test_sandboxed_service_capture_startup.py create mode 100644 tests/test_sandboxed_verify_output_limits.py create mode 100644 tests/test_sandboxed_web_e2e_branch_contract.py create mode 100644 tests/test_sandboxed_web_e2e_output_limits.py diff --git a/tests/test_bounded_subprocess.py b/tests/test_bounded_subprocess.py new file mode 100644 index 000000000..741c43b7a --- /dev/null +++ b/tests/test_bounded_subprocess.py @@ -0,0 +1,275 @@ +"""Real-process contracts for bounded sandbox subprocess output.""" + +from __future__ import annotations + +import io +import os +import sys +from pathlib import Path + +import pytest + +from scripts.ci import bounded_subprocess as bounded + + +def _environment() -> dict[str, str]: + """Return a minimal child environment that can launch the current Python.""" + + return {"PATH": os.environ.get("PATH", ""), "PYTHONIOENCODING": "utf-8"} + + +def test_read_bounded_suffix_preserves_unicode_and_marks_partial_suffix( + tmp_path: Path, +) -> None: + """Suffix reads are byte-bounded and tolerate a cut UTF-8 code point.""" + + short_path = tmp_path / "short.log" + short_path.write_text("ordinary 한글\n", encoding="utf-8") + short = bounded.read_bounded_suffix(short_path, 4096) + assert short.text == "ordinary 한글\n" + assert short.truncated is False + assert short.stored_bytes == len("ordinary 한글\n".encode("utf-8")) + + partial_path = tmp_path / "partial.log" + partial_path.write_bytes(b"prefix-" + "가".encode("utf-8")) + partial = bounded.read_bounded_suffix(partial_path, 2) + assert partial.truncated is True + assert partial.stored_bytes == len(partial_path.read_bytes()) + assert partial.text.startswith(bounded.TRUNCATION_MARKER) + assert "�" in partial.text + + +def test_bounded_capture_retains_final_suffix_and_writes_bounded_file( + tmp_path: Path, +) -> None: + """The stream drainer retains only a bounded final suffix for evidence.""" + + destination = tmp_path / "captured.log" + limit_calls: list[str] = [] + capture = bounded.start_bounded_capture( + io.BytesIO(b"prefix-" + b"x" * 5000 + b"-final"), + evidence_limit_bytes=4096, + on_limit=lambda: limit_calls.append("limited"), + destination=destination, + ) + capture.join(timeout=5) + + assert capture.output_limited is True + assert capture.total_bytes == 5013 + assert limit_calls == ["limited"] + assert capture.text.startswith(bounded.TRUNCATION_MARKER) + assert capture.text.endswith("-final") + assert destination.stat().st_size <= 4096 + assert destination.read_text(encoding="utf-8").endswith("-final") + + +def test_run_bounded_command_preserves_ordinary_unicode_output(tmp_path: Path) -> None: + """Normal child output and return codes remain unchanged below the budget.""" + + result = bounded.run_bounded_command( + [ + sys.executable, + "-c", + "import sys; print('안녕'); print('경고', file=sys.stderr)", + ], + cwd=tmp_path, + env=_environment(), + timeout=10, + evidence_limit_bytes=4096, + ) + + assert result.args[0] == sys.executable + assert result.returncode == 0 + assert result.stdout == "안녕\n" + assert result.stderr == "경고\n" + assert result.output_limited is False + + +@pytest.mark.parametrize("stream_descriptor", [1, 2]) +def test_run_bounded_command_caps_real_stdout_and_stderr( + tmp_path: Path, + stream_descriptor: int, +) -> None: + """A real output flood is killed while the retained stream stays bounded.""" + + result = bounded.run_bounded_command( + [ + sys.executable, + "-c", + ( + "import os\n" + f"descriptor={stream_descriptor}\n" + "chunk=b'x'*1024\n" + "while True:\n" + " os.write(descriptor, chunk)\n" + ), + ], + cwd=tmp_path, + env=_environment(), + timeout=10, + evidence_limit_bytes=4096, + ) + + selected = result.stdout if stream_descriptor == 1 else result.stderr + assert result.output_limited is True + assert selected.startswith(bounded.TRUNCATION_MARKER) + assert len(selected.encode("utf-8")) <= 4096 + assert result.returncode != 0 + + +def test_timeout_raises_with_only_bounded_output(tmp_path: Path) -> None: + """Timeout evidence is bounded even when the child was actively writing.""" + + with pytest.raises(bounded.BoundedTimeoutExpired) as raised: + bounded.run_bounded_command( + [ + sys.executable, + "-c", + ( + "import os,time\n" + "os.write(1,b'before-timeout\\n')\n" + "os.write(2,b'warning-before-timeout\\n')\n" + "time.sleep(30)\n" + ), + ], + cwd=tmp_path, + env=_environment(), + timeout=1, + evidence_limit_bytes=4096, + ) + + assert raised.value.timeout == 1 + assert raised.value.stdout == "before-timeout\n" + assert raised.value.stderr == "warning-before-timeout\n" + assert raised.value.output_limited is False + + +def test_validate_output_limit_rejects_unsafe_values() -> None: + """Configured byte budgets are integer, bounded, and never Boolean.""" + + assert bounded.validate_output_limit(4096, "test limit") == 4096 + assert ( + bounded.validate_output_limit( + bounded.MAXIMUM_OUTPUT_LIMIT_BYTES, + "test limit", + ) + == bounded.MAXIMUM_OUTPUT_LIMIT_BYTES + ) + for value in [ + True, + 1.5, + "4096", + 4095, + bounded.MAXIMUM_OUTPUT_LIMIT_BYTES + 1, + ]: + with pytest.raises(ValueError, match="test limit"): + bounded.validate_output_limit(value, "test limit") # type: ignore[arg-type] + + +def test_supported_platform_gate_fails_closed(monkeypatch) -> None: + """Unsupported platforms cannot silently fall back to unmanaged children.""" + + monkeypatch.setattr(bounded.os, "name", "nt") + with pytest.raises(bounded.OutputLimitUnsupportedError): + bounded.require_supported_platform() + + monkeypatch.setattr(bounded.os, "name", "posix") + bounded.require_supported_platform() + + +def test_capture_surfaces_reader_failure_and_join_timeout( + monkeypatch, +) -> None: + """Reader failures and stuck drains are explicit rather than silently ignored.""" + + class FailingStream: + """Raise one deterministic error from the background reader.""" + + def read(self, size: int) -> bytes: + """Reject the read request.""" + + del size + raise OSError("read failed") + + def close(self) -> None: + """Provide the binary-stream close interface.""" + + capture = bounded.start_bounded_capture( + FailingStream(), # type: ignore[arg-type] + evidence_limit_bytes=4096, + on_limit=lambda: None, + ) + with pytest.raises(OSError, match="read failed"): + capture.join(timeout=5) + + class NeverFinishesThread: + """Simulate one drain thread that remains alive after join.""" + + def join(self, timeout: float | None = None) -> None: + """Accept the join call without completing.""" + + del timeout + + def is_alive(self) -> bool: + """Report a stuck reader.""" + + return True + + capture = bounded.BoundedOutputCapture( + io.BytesIO(b"safe"), + evidence_limit_bytes=4096, + on_limit=lambda: None, + ) + monkeypatch.setattr(capture, "_thread", NeverFinishesThread()) + with pytest.raises(RuntimeError, match="did not finish"): + capture.join(timeout=0) + + +def test_join_captures_applies_finite_timeout_and_joins_every_reader() -> None: + """Normal-path capture finalization cannot wait forever on inherited pipe FDs.""" + + observed: list[tuple[str, float]] = [] + + class Capture: + """Record the timeout and optionally expose one stuck-reader failure.""" + + def __init__(self, label: str, *, fail: bool = False) -> None: + self.label = label + self.fail = fail + + def join(self, timeout: float) -> None: + """Require a positive finite timeout and retain sibling finalization.""" + + observed.append((self.label, timeout)) + if self.fail: + raise RuntimeError("bounded output drain did not finish") + + with pytest.raises(RuntimeError, match="did not finish"): + bounded._join_captures( # noqa: SLF001 - internal safety contract + (Capture("first", fail=True), Capture("second", fail=True)) # type: ignore[arg-type] + ) + + assert [label for label, _timeout in observed] == ["first", "second"] + assert all(timeout > 0 for _label, timeout in observed) + assert len({timeout for _label, timeout in observed}) == 1 + + +def test_run_bounded_command_rejects_empty_command_and_timeout(tmp_path: Path) -> None: + """The reusable runner validates execution controls before creating children.""" + + with pytest.raises(ValueError, match="command"): + bounded.run_bounded_command( + [], + cwd=tmp_path, + env=_environment(), + timeout=10, + evidence_limit_bytes=4096, + ) + with pytest.raises(ValueError, match="timeout"): + bounded.run_bounded_command( + [sys.executable, "-c", "pass"], + cwd=tmp_path, + env=_environment(), + timeout=0, + evidence_limit_bytes=4096, + ) diff --git a/tests/test_bounded_subprocess_capture_startup.py b/tests/test_bounded_subprocess_capture_startup.py new file mode 100644 index 000000000..e1254cf58 --- /dev/null +++ b/tests/test_bounded_subprocess_capture_startup.py @@ -0,0 +1,160 @@ +"""Regression contracts for bounded command capture-startup cleanup.""" + +from __future__ import annotations + +import io +from pathlib import Path +from typing import cast + +import pytest + +from scripts.ci import bounded_subprocess as bounded + + +class _TrackedStream(io.BytesIO): + """Binary pipe double whose closed state remains observable.""" + + +class _Process: + """Minimal running process double with two parent-side output pipes.""" + + pid = 4242 + + def __init__(self) -> None: + """Create open stdout and stderr streams and cleanup counters.""" + + self.stdout = _TrackedStream(b"stdout") + self.stderr = _TrackedStream(b"stderr") + self.returncode: int | None = None + self.wait_calls = 0 + + def poll(self) -> int | None: + """Return the current fake process status.""" + + return self.returncode + + def wait(self, timeout=None) -> int: + """Record reaping and return a killed-process status.""" + + del timeout + self.wait_calls += 1 + self.returncode = -9 + return self.returncode + + +class _Capture: + """Capture double that closes its owned stream when finalized.""" + + output_limited = False + text = "" + + def __init__(self, stream: _TrackedStream, *, fail_join: bool = False) -> None: + """Remember the owned stream and optional cleanup failure.""" + + self.stream = stream + self.fail_join = fail_join + self.join_calls = 0 + + def join(self, timeout=None) -> None: + """Finalize the owned stream and optionally report a secondary error.""" + + del timeout + self.join_calls += 1 + self.stream.close() + if self.fail_join: + raise RuntimeError("secondary capture cleanup failure") + + +@pytest.mark.parametrize("failure_call", [1, 2]) +def test_capture_startup_failure_kills_reaps_finalizes_and_closes( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + failure_call: int, +) -> None: + """Either capture-start failure must leave no process, reader, or pipe alive.""" + + process = _Process() + killed: list[_Process] = [] + captures: list[_Capture] = [] + startup_calls = 0 + + def fake_start(stream, **_kwargs): + """Fail at the selected capture start and return earlier captures.""" + + nonlocal startup_calls + startup_calls += 1 + if startup_calls == failure_call: + raise OSError("capture startup failed") + capture = _Capture(cast(_TrackedStream, stream)) + captures.append(capture) + return capture + + monkeypatch.setattr(bounded, "require_supported_platform", lambda: None) + monkeypatch.setattr(bounded.subprocess, "Popen", lambda *args, **kwargs: process) + monkeypatch.setattr( + bounded, + "kill_process_group", + lambda candidate: killed.append(cast(_Process, candidate)), + ) + monkeypatch.setattr(bounded, "start_bounded_capture", fake_start) + + with pytest.raises(OSError, match="capture startup failed"): + bounded.run_bounded_command( + ["tool"], + cwd=tmp_path, + env={}, + timeout=10, + evidence_limit_bytes=4096, + ) + + assert killed == [process] + assert process.wait_calls == 1 + assert process.stdout.closed + assert process.stderr.closed + assert all(capture.join_calls == 2 for capture in captures) + + +def test_capture_startup_preserves_original_error_when_cleanup_fails( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Secondary join errors cannot replace the capture-start root cause.""" + + process = _Process() + capture = _Capture(process.stdout, fail_join=True) + startup_calls = 0 + killed: list[object] = [] + + def fake_start(_stream, **_kwargs): + """Return stdout capture and fail while starting stderr capture.""" + + nonlocal startup_calls + startup_calls += 1 + if startup_calls == 1: + return capture + raise OSError("primary capture startup failure") + + monkeypatch.setattr(bounded, "require_supported_platform", lambda: None) + monkeypatch.setattr(bounded.subprocess, "Popen", lambda *args, **kwargs: process) + monkeypatch.setattr( + bounded, + "kill_process_group", + lambda candidate: killed.append(candidate), + ) + monkeypatch.setattr(bounded, "start_bounded_capture", fake_start) + + with pytest.raises(OSError, match="primary capture startup failure"): + bounded.run_bounded_command( + ["tool"], + cwd=tmp_path, + env={}, + timeout=10, + evidence_limit_bytes=4096, + ) + + assert killed == [process] + assert process.wait_calls == 1 + assert capture.join_calls == 2 + assert process.stdout.closed + assert process.stderr.closed + diff --git a/tests/test_bounded_subprocess_contract.py b/tests/test_bounded_subprocess_contract.py new file mode 100644 index 000000000..32a00d371 --- /dev/null +++ b/tests/test_bounded_subprocess_contract.py @@ -0,0 +1,345 @@ +"""Branch-complete contracts for bounded subprocess helpers and failures.""" + +from __future__ import annotations + +import io +import os +from collections.abc import Callable +from pathlib import Path + +import pytest + +from scripts.ci import bounded_subprocess as bounded + + +def test_read_limit_and_timeout_validation_reject_all_unsafe_types() -> None: + """Private validators reject Boolean, nonnumeric, nonpositive, and huge values.""" + + for value in [False, "2", 0, bounded.MAXIMUM_OUTPUT_LIMIT_BYTES + 1]: + with pytest.raises(ValueError, match="maximum_bytes"): + bounded._validate_read_limit(value) + for value in [False, "1", 0, -1]: + with pytest.raises(ValueError, match="timeout"): + bounded._validated_timeout(value) + + +def test_supported_platform_requires_posix_killpg(monkeypatch) -> None: + """POSIX naming without process-group termination still fails closed.""" + + monkeypatch.setattr(bounded.os, "name", "posix") + monkeypatch.delattr(bounded.os, "killpg") + with pytest.raises(bounded.OutputLimitUnsupportedError): + bounded.require_supported_platform() + + +def test_capture_notifies_once_across_multiple_overflowing_chunks() -> None: + """Repeated chunks beyond the ceiling retain a suffix but notify only once.""" + + class ChunkStream: + """Return one deterministic chunk for each background read.""" + + def __init__(self) -> None: + self.chunks = [b"a" * 3000, b"b" * 3000, b"c" * 1000, b""] + + def read(self, size: int) -> bytes: + """Return the next chunk within the requested reader contract.""" + + assert size == bounded.READ_CHUNK_BYTES + return self.chunks.pop(0) + + def close(self) -> None: + """Provide the binary-stream close interface.""" + + notifications: list[str] = [] + capture = bounded.start_bounded_capture( + ChunkStream(), # type: ignore[arg-type] + evidence_limit_bytes=4096, + on_limit=lambda: notifications.append("limited"), + ) + capture.join(timeout=5) + + assert notifications == ["limited"] + assert capture.output_limited + assert capture.total_bytes == 7000 + assert capture.text.endswith("c" * 1000) + + +def test_capture_destination_failures_propagate_without_masking_read_error( + tmp_path: Path, +) -> None: + """Evidence-write errors surface, while an earlier read error keeps precedence.""" + + blocked_parent = tmp_path / "blocked" + blocked_parent.write_text("not a directory", encoding="utf-8") + destination = blocked_parent / "capture.log" + + capture = bounded.start_bounded_capture( + io.BytesIO(b"safe"), + evidence_limit_bytes=4096, + on_limit=lambda: None, + destination=destination, + ) + with pytest.raises((FileExistsError, NotADirectoryError)): + capture.join(timeout=5) + + class ReadFailure: + """Fail before the destination writer also encounters its path error.""" + + def read(self, size: int) -> bytes: + """Raise the primary reader failure.""" + + del size + raise OSError("primary read failure") + + def close(self) -> None: + """Provide the binary-stream close interface.""" + + capture = bounded.start_bounded_capture( + ReadFailure(), # type: ignore[arg-type] + evidence_limit_bytes=4096, + on_limit=lambda: None, + destination=destination, + ) + with pytest.raises(OSError, match="primary read failure"): + capture.join(timeout=5) + + +def test_command_normalization_rejects_empty_executable() -> None: + """A present but empty executable token is not a runnable command.""" + + with pytest.raises(ValueError, match="command"): + bounded._normalized_command([""]) + + +def test_kill_process_group_handles_finished_and_disappearing_processes( + monkeypatch, +) -> None: + """Termination is idempotent when a process already exited or disappeared.""" + + calls: list[tuple[int, int]] = [] + monkeypatch.setattr( + bounded.os, + "killpg", + lambda pid, signal_number: calls.append((pid, signal_number)), + ) + + class FinishedProcess: + """Represent one already-reaped child.""" + + pid = 10 + + def poll(self) -> int: + """Return a completed status.""" + + return 0 + + bounded.kill_process_group(FinishedProcess()) # type: ignore[arg-type] + assert calls == [] + + class RunningProcess: + """Represent one child that disappears before the signal is delivered.""" + + pid = 11 + + def poll(self): + """Report an apparently running child.""" + + return None + + def missing_process(pid: int, signal_number: int) -> None: + """Simulate the race between poll and group signaling.""" + + del pid, signal_number + raise ProcessLookupError + + monkeypatch.setattr(bounded.os, "killpg", missing_process) + bounded.kill_process_group(RunningProcess()) # type: ignore[arg-type] + + +def test_run_rejects_missing_subprocess_pipes(monkeypatch, tmp_path: Path) -> None: + """A broken Popen contract is killed and rejected before reader creation.""" + + class MissingPipesProcess: + """Expose no stdout or stderr pipe despite the requested configuration.""" + + pid = 12 + stdout = None + stderr = None + returncode = -9 + + def poll(self): + """Report a running child until the fake kill path executes.""" + + return None + + def wait(self, timeout=None) -> int: + """Return the fake terminal status.""" + + del timeout + return self.returncode + + process = MissingPipesProcess() + monkeypatch.setattr(bounded, "require_supported_platform", lambda: None) + monkeypatch.setattr(bounded.subprocess, "Popen", lambda *args, **kwargs: process) + killed: list[object] = [] + monkeypatch.setattr(bounded, "kill_process_group", lambda candidate: killed.append(candidate)) + + with pytest.raises(RuntimeError, match="pipes"): + bounded.run_bounded_command( + ["tool"], + cwd=tmp_path, + env={}, + timeout=1, + evidence_limit_bytes=4096, + ) + assert killed == [process] + + +def test_two_overflow_callbacks_kill_the_process_group_once( + monkeypatch, + tmp_path: Path, +) -> None: + """Simultaneous stdout/stderr limit notifications share one kill transition.""" + + callbacks: list[Callable[[], None]] = [] + + class FakePipe: + """Stand in for one requested subprocess pipe.""" + + class FakeProcess: + """Invoke both capture callbacks while the parent waits.""" + + pid = 13 + stdout = FakePipe() + stderr = FakePipe() + returncode = -9 + + def poll(self): + """Report a running process during callback delivery.""" + + return None + + def wait(self, timeout=None) -> int: + """Deliver both overflow callbacks and return the terminal status.""" + + del timeout + if callbacks: + callbacks[0]() + callbacks[1]() + return self.returncode + + class FakeCapture: + """Return fixed limited evidence without background threads.""" + + output_limited = True + text = bounded.TRUNCATION_MARKER + + def join(self, timeout=None) -> None: + """Complete immediately.""" + + del timeout + + process = FakeProcess() + monkeypatch.setattr(bounded, "require_supported_platform", lambda: None) + monkeypatch.setattr(bounded.subprocess, "Popen", lambda *args, **kwargs: process) + + def fake_capture(stream, *, evidence_limit_bytes, on_limit, destination=None): + """Record each overflow callback supplied by the command runner.""" + + del stream, evidence_limit_bytes, destination + callbacks.append(on_limit) + return FakeCapture() + + monkeypatch.setattr(bounded, "start_bounded_capture", fake_capture) + kills: list[object] = [] + monkeypatch.setattr(bounded, "kill_process_group", lambda candidate: kills.append(candidate)) + + result = bounded.run_bounded_command( + ["tool"], + cwd=tmp_path, + env={}, + timeout=1, + evidence_limit_bytes=4096, + ) + + assert result.output_limited + assert kills == [process] + + +def test_run_joins_both_stream_captures_when_one_join_fails( + monkeypatch, + tmp_path: Path, +) -> None: + """A reader failure cannot leave the sibling drain thread unjoined.""" + + class FakePipe: + """Stand in for one requested subprocess pipe.""" + + class FakeProcess: + """Complete immediately with both requested pipes present.""" + + pid = 14 + stdout = FakePipe() + stderr = FakePipe() + returncode = 0 + + def poll(self): + """Return the completed status.""" + + return self.returncode + + def wait(self, timeout=None) -> int: + """Complete immediately.""" + + del timeout + return self.returncode + + joins: list[str] = [] + + class FakeCapture: + """Record join order and optionally raise one deterministic error.""" + + output_limited = False + text = "" + + def __init__(self, label: str, error: BaseException | None) -> None: + self.label = label + self.error = error + + def join(self, timeout=None) -> None: + """Record finalization before surfacing the configured error.""" + + del timeout + joins.append(self.label) + if self.error is not None: + raise self.error + + captures = iter( + [ + FakeCapture("stdout", OSError("stdout drain failed")), + FakeCapture("stderr", None), + ] + ) + monkeypatch.setattr(bounded, "require_supported_platform", lambda: None) + monkeypatch.setattr( + bounded.subprocess, + "Popen", + lambda *args, **kwargs: FakeProcess(), + ) + monkeypatch.setattr( + bounded, + "start_bounded_capture", + lambda *args, **kwargs: next(captures), + ) + + with pytest.raises(OSError, match="stdout drain failed"): + bounded.run_bounded_command( + ["tool"], + cwd=tmp_path, + env={}, + timeout=1, + evidence_limit_bytes=4096, + ) + + assert joins == ["stdout", "stderr"] + diff --git a/tests/test_repository_branch_coverage_execution_sandboxes.py b/tests/test_repository_branch_coverage_execution_sandboxes.py index f8912272a..be4666d9a 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) @@ -196,6 +200,7 @@ def start_service( _cwd: Path, _env: dict[str, str], logs_dir: Path, + _log_limit_bytes: int, ) -> sandboxed_web_e2e.Service: log_path = logs_dir / f"{label}.log" log_path.write_text("", encoding="utf-8") @@ -204,7 +209,11 @@ def start_service( ) def timeout_runner( - command: str, _cwd: Path, _env: dict[str, str], timeout: int + command: 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_entrypoint_and_cleanup_coverage.py b/tests/test_sandboxed_entrypoint_and_cleanup_coverage.py new file mode 100644 index 000000000..0e04fb38d --- /dev/null +++ b/tests/test_sandboxed_entrypoint_and_cleanup_coverage.py @@ -0,0 +1,97 @@ +import runpy +import subprocess +import sys +from pathlib import Path + +from scripts.ci import bounded_subprocess, sandboxed_verify, sandboxed_web_e2e + + +def test_sandboxed_verify_direct_file_import_bootstraps_repository_path(): + """Direct-file loading executes the repository-path bootstrap branch.""" + + 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_web_e2e_reports_bounded_capture_finalization_failure( + monkeypatch, + tmp_path, + capsys, +): + """A service-capture finalization failure remains a bounded hard failure.""" + + repo = tmp_path / "repo" + repo.mkdir() + + class DoneProcess: + def poll(self): + return 0 + + def fake_start( + label, + command, + cwd, + env, + logs_dir, + log_limit_bytes=bounded_subprocess.DEFAULT_SERVICE_LOG_LIMIT_BYTES, + ): + del cwd, env + log_path = logs_dir / f"{label}.log" + log_path.write_text(f"{label} ready\n", encoding="utf-8") + return sandboxed_web_e2e.Service( + label=label, + command=command, + process=DoneProcess(), + log_path=log_path, + log_limit_bytes=log_limit_bytes, + ) + + monkeypatch.setattr(sandboxed_web_e2e, "start_service", fake_start) + monkeypatch.setattr(sandboxed_web_e2e, "wait_for_url", lambda *args: True) + monkeypatch.setattr( + sandboxed_web_e2e, + "run_shell", + lambda *args, **kwargs: subprocess.CompletedProcess( + args=["e2e"], + returncode=0, + stdout="ok\n", + stderr="", + ), + ) + + def fail_capture_finalization(service): + raise OSError(f"cannot finalize {service.label}") + + monkeypatch.setattr( + sandboxed_web_e2e, + "stop_service", + fail_capture_finalization, + ) + + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(repo), + "--backend-cmd", + "backend", + "--frontend-cmd", + "frontend", + "--backend-ready-url", + "http://127.0.0.1:8000/health", + "--frontend-ready-url", + "http://127.0.0.1:3000/", + "--e2e-cmd", + "e2e", + ] + ) + captured = capsys.readouterr() + + assert exit_code == bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE + assert captured.err.count("bounded service capture failed") == 2 + assert f'"exit_code": {bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE}' in captured.out + assert '"output_limited": true' in captured.out + diff --git a/tests/test_sandboxed_service_capture_startup.py b/tests/test_sandboxed_service_capture_startup.py new file mode 100644 index 000000000..6da7a9ac5 --- /dev/null +++ b/tests/test_sandboxed_service_capture_startup.py @@ -0,0 +1,78 @@ +"""Failure contracts for bounded sandbox service capture startup.""" + +from __future__ import annotations + +import io +from pathlib import Path + +import pytest + +from scripts.ci import bounded_subprocess as bounded +from scripts.ci import sandboxed_web_e2e + + +class _RunningProcess: + """Minimal process double active until explicitly stopped and waited.""" + + pid = 200 + + def __init__(self) -> None: + self.stdout = io.BytesIO(b"") + self.returncode: int | None = None + self.waited = False + + def poll(self) -> int | None: + """Return the current process state.""" + + return self.returncode + + def wait(self, timeout=None) -> int: + """Record reaping and return the terminal status.""" + + del timeout + self.waited = True + self.returncode = -9 + return self.returncode + + +def test_capture_startup_failure_stops_reaps_and_closes_the_service_pipe( + monkeypatch, + tmp_path: Path, +) -> None: + """A failed drainer cannot leave a child or parent-side pipe uncollected.""" + + process = _RunningProcess() + monkeypatch.setattr(bounded, "require_supported_platform", lambda: None) + monkeypatch.setattr( + sandboxed_web_e2e.subprocess, + "Popen", + lambda *args, **kwargs: process, + ) + monkeypatch.setattr( + bounded, + "start_bounded_capture", + lambda *args, **kwargs: (_ for _ in ()).throw( + RuntimeError("capture startup failed") + ), + ) + stopped: list[object] = [] + monkeypatch.setattr( + bounded, + "kill_process_group", + lambda candidate: stopped.append(candidate), + ) + + with pytest.raises(RuntimeError, match="capture startup failed"): + sandboxed_web_e2e.start_service( + "backend", + "tool", + tmp_path, + {}, + tmp_path, + 4096, + ) + + assert stopped == [process] + assert process.waited is True + assert process.stdout.closed is True + diff --git a/tests/test_sandboxed_verify_output_limits.py b/tests/test_sandboxed_verify_output_limits.py new file mode 100644 index 000000000..2dd0951e7 --- /dev/null +++ b/tests/test_sandboxed_verify_output_limits.py @@ -0,0 +1,180 @@ +"""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, redacted, 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_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 True + + +def test_cli_rejects_output_budgets_outside_supported_range( + tmp_path: Path, +) -> None: + """Unsafe output budgets fail argument parsing before workspace execution.""" + + repository = _repository(tmp_path) + for value in ["4095", str(bounded.MAXIMUM_OUTPUT_LIMIT_BYTES + 1)]: + 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_web_e2e.py b/tests/test_sandboxed_web_e2e.py index 6e092c293..6ab04edd5 100644 --- a/tests/test_sandboxed_web_e2e.py +++ b/tests/test_sandboxed_web_e2e.py @@ -1,4 +1,5 @@ import json +import io import os import runpy import socket @@ -159,6 +160,7 @@ def test_start_service_and_run_shell_capture_bash_contract(monkeypatch, tmp_path class FakeProcess: pid = 42 + stdout = io.BytesIO(b"") def poll(self): return 0 @@ -167,12 +169,22 @@ def fake_popen(*args, **kwargs): popen_calls.append((args, kwargs)) return FakeProcess() - def fake_run(*args, **kwargs): + def fake_bounded_run(*args, **kwargs): run_calls.append((args, kwargs)) - return subprocess.CompletedProcess(args[0], 7, stdout="out", stderr="err") + return sandboxed_web_e2e.bounded_subprocess.BoundedCompletedProcess( + args=("npm", "test"), + returncode=7, + stdout="out", + stderr="err", + output_limited=False, + ) monkeypatch.setattr(sandboxed_web_e2e.subprocess, "Popen", fake_popen) - monkeypatch.setattr(sandboxed_web_e2e.subprocess, "run", fake_run) + monkeypatch.setattr( + sandboxed_web_e2e.bounded_subprocess, + "run_bounded_command", + fake_bounded_run, + ) service = sandboxed_web_e2e.start_service("backend", "npm run dev", tmp_path, {"PATH": "/bin"}, tmp_path) completed = sandboxed_web_e2e.run_shell("npm test", tmp_path, {"PATH": "/bin"}, 5) @@ -181,14 +193,14 @@ def fake_run(*args, **kwargs): assert service.command == "npm run dev" assert service.log_path == tmp_path / "backend.log" assert popen_calls[0][0] == (["npm", "run", "dev"],) - assert "shell" not in popen_calls[0][1] assert "executable" not in popen_calls[0][1] assert popen_calls[0][1]["start_new_session"] is True + assert popen_calls[0][1]["shell"] is False + service.capture.join(timeout=5) assert completed.returncode == 7 assert run_calls[0][0] == (["npm", "test"],) assert run_calls[0][1]["timeout"] == 5 - assert "shell" not in run_calls[0][1] - assert "executable" not in run_calls[0][1] + assert run_calls[0][1]["evidence_limit_bytes"] > 0 def test_wait_for_url_handles_success_retry_and_log_tail(monkeypatch, tmp_path): @@ -273,11 +285,11 @@ class DoneProcess: def poll(self): return 0 - def fake_start(label, command, cwd, env, logs_dir): + def fake_start(label, command, cwd, env, logs_dir, log_limit_bytes): log_path = logs_dir / f"{label}.log" log_path.write_text(f"{label} ready\n", encoding="utf-8") service = sandboxed_web_e2e.Service(label, command, DoneProcess(), log_path) - started.append((label, command, cwd, "SANDBOXED_VERIFY" in env)) + started.append((label, command, cwd, "SANDBOXED_VERIFY" in env, log_limit_bytes)) return service monkeypatch.setattr(sandboxed_web_e2e, "start_service", fake_start) @@ -285,11 +297,14 @@ def fake_start(label, command, cwd, env, logs_dir): monkeypatch.setattr( sandboxed_web_e2e, "run_shell", - lambda command, cwd, env, timeout: subprocess.CompletedProcess( - command, - 0, - stdout="e2e-out\n", - stderr="e2e-err\n", + lambda command, cwd, env, timeout, output_limit_bytes: ( + sandboxed_web_e2e.bounded_subprocess.BoundedCompletedProcess( + args=(command,), + returncode=0, + stdout="e2e-out\n", + stderr="e2e-err\n", + output_limited=False, + ) ), ) monkeypatch.setattr(sandboxed_web_e2e, "stop_service", lambda service: stopped.append(service.label)) @@ -345,7 +360,8 @@ class DoneProcess: def poll(self): return 0 - def fake_start(label, command, cwd, env, logs_dir): + def fake_start(label, command, cwd, env, logs_dir, log_limit_bytes): + del log_limit_bytes log_path = logs_dir / f"{label}.log" log_path.write_text(f"{label} not ready\n", encoding="utf-8") return sandboxed_web_e2e.Service(label, command, DoneProcess(), log_path) @@ -393,12 +409,14 @@ class DoneProcess: def poll(self): return 0 - def fake_start(label, command, cwd, env, logs_dir): + def fake_start(label, command, cwd, env, logs_dir, log_limit_bytes): + del log_limit_bytes log_path = logs_dir / f"{label}.log" log_path.write_text(f"{label} tail\n", encoding="utf-8") return sandboxed_web_e2e.Service(label, command, DoneProcess(), log_path) - def fake_run_shell(command, cwd, env, timeout): + def fake_run_shell(command, cwd, env, timeout, output_limit_bytes): + del output_limit_bytes raise subprocess.TimeoutExpired(command, timeout, output=b"e2e-out", stderr=b"e2e-err") monkeypatch.setattr(sandboxed_web_e2e, "start_service", fake_start) @@ -469,7 +487,8 @@ def test_sandboxed_web_e2e_reports_e2e_timeout(monkeypatch, tmp_path, capsys): repo = tmp_path / "repo" repo.mkdir() - def fake_run_shell(command, cwd, env, timeout): + def fake_run_shell(command, cwd, env, timeout, output_limit_bytes): + del output_limit_bytes raise subprocess.TimeoutExpired(command, timeout, output="e2e-out", stderr="e2e-err") monkeypatch.setattr(sandboxed_web_e2e, "run_shell", fake_run_shell) diff --git a/tests/test_sandboxed_web_e2e_branch_contract.py b/tests/test_sandboxed_web_e2e_branch_contract.py new file mode 100644 index 000000000..d9678a2a7 --- /dev/null +++ b/tests/test_sandboxed_web_e2e_branch_contract.py @@ -0,0 +1,469 @@ +"""Branch-complete contracts for bounded sandbox web E2E orchestration.""" + +from __future__ import annotations + +import json +import subprocess +import sys +import urllib.error +from pathlib import Path +from typing import cast + +import pytest + +from scripts.ci import bounded_subprocess as bounded +from scripts.ci import sandboxed_web_e2e + + +def _result(output: str) -> dict[str, object]: + """Parse one final web E2E result marker.""" + + marker = f"{sandboxed_web_e2e.RESULT_MARKER} " + line = next(line for line in output.splitlines() if line.startswith(marker)) + return json.loads(line.removeprefix(marker)) + + +class _DoneProcess: + """Minimal process double that has already completed.""" + + pid = 100 + returncode = 0 + + def poll(self) -> int: + """Return the completed status.""" + + return self.returncode + + def wait(self, timeout=None) -> int: + """Return immediately.""" + + del timeout + return self.returncode + + +class _RunningProcess: + """Minimal running process double for cleanup branches.""" + + pid = 101 + returncode = None + + def poll(self): + """Report that the process remains active.""" + + return self.returncode + + def wait(self, timeout=None) -> int: + """Complete when the fake process is explicitly waited.""" + + del timeout + self.returncode = 0 + return 0 + + +def _service(tmp_path: Path, *, process=None, log_limit_bytes: int = 4096): + """Create one service double with no background capture.""" + + return sandboxed_web_e2e.Service( + label="service", + command="service", + process=cast(subprocess.Popen[bytes], process or _DoneProcess()), + log_path=tmp_path / "service.log", + log_limit_bytes=log_limit_bytes, + ) + + +def test_start_service_rejects_missing_output_pipe( + monkeypatch, + tmp_path: Path, +) -> None: + """A broken Popen pipe contract is killed and rejected.""" + + class MissingPipeProcess(_RunningProcess): + """Return no stdout despite the requested PIPE configuration.""" + + stdout = None + + process = MissingPipeProcess() + monkeypatch.setattr(bounded, "require_supported_platform", lambda: None) + monkeypatch.setattr( + sandboxed_web_e2e.subprocess, + "Popen", + lambda *args, **kwargs: process, + ) + killed: list[object] = [] + monkeypatch.setattr( + bounded, + "kill_process_group", + lambda candidate: killed.append(candidate), + ) + + with pytest.raises(RuntimeError, match="pipe"): + sandboxed_web_e2e.start_service( + "backend", + "tool", + tmp_path, + {}, + tmp_path, + 4096, + ) + assert killed == [process] + + +def test_service_limit_fallback_handles_missing_small_and_large_files( + tmp_path: Path, +) -> None: + """Legacy/fake services classify file-only evidence deterministically.""" + + service = _service(tmp_path) + assert not sandboxed_web_e2e.service_output_limited(service) + service.log_path.write_bytes(b"safe") + assert not sandboxed_web_e2e.service_output_limited(service) + service.log_path.write_bytes(b"x" * 4097) + assert sandboxed_web_e2e.service_output_limited(service) + + +def test_wait_for_url_handles_empty_invalid_exited_limited_and_success( + monkeypatch, + tmp_path: Path, +) -> None: + """Readiness polling preserves every validation and termination branch.""" + + service = _service(tmp_path) + assert sandboxed_web_e2e.wait_for_url("", 1, service) + with pytest.raises(ValueError, match="http"): + sandboxed_web_e2e.wait_for_url("file:///tmp/ready", 1, service) + assert not sandboxed_web_e2e.wait_for_url( + "https://example.invalid/ready", + 1, + service, + ) + + running = _service(tmp_path, process=_RunningProcess()) + running.log_path.write_bytes(b"x" * 4097) + assert not sandboxed_web_e2e.wait_for_url( + "https://example.invalid/ready", + 1, + running, + ) + running.log_path.unlink() + + class Response: + """Context-managed readiness response.""" + + status = 204 + + def __enter__(self): + """Return the response.""" + + return self + + def __exit__(self, exc_type, exc, traceback) -> None: + """Close without suppressing exceptions.""" + + del exc_type, exc, traceback + + class Opener: + """Return one successful response.""" + + def open(self, url: str, timeout: int): + """Validate the poll request and return readiness.""" + + assert url == "https://ready.example/health" + assert timeout == 2 + return Response() + + clean_running = _service(tmp_path, process=_RunningProcess()) + monkeypatch.setattr( + sandboxed_web_e2e.urllib.request, + "build_opener", + lambda handler: Opener(), + ) + assert sandboxed_web_e2e.wait_for_url( + "https://ready.example/health", + 1, + clean_running, + ) + + +def test_wait_for_url_retries_url_errors_until_deadline( + monkeypatch, + tmp_path: Path, +) -> None: + """Transient URL errors sleep and eventually produce a bounded false result.""" + + class FailingOpener: + """Raise one deterministic URL error per poll.""" + + def open(self, url: str, timeout: int): + """Reject the readiness request.""" + + del url, timeout + raise urllib.error.URLError("not ready") + + timeline = iter([0.0, 0.0, 2.0]) + sleeps: list[int] = [] + monkeypatch.setattr(sandboxed_web_e2e.time, "monotonic", lambda: next(timeline)) + monkeypatch.setattr(sandboxed_web_e2e.time, "sleep", lambda seconds: sleeps.append(seconds)) + monkeypatch.setattr( + sandboxed_web_e2e.urllib.request, + "build_opener", + lambda handler: FailingOpener(), + ) + + assert not sandboxed_web_e2e.wait_for_url( + "https://ready.example/health", + 1, + _service(tmp_path, process=_RunningProcess()), + ) + assert sleeps == [1] + + +def test_redirect_handler_raises_http_error() -> None: + """Readiness redirects are never followed.""" + + handler = sandboxed_web_e2e.NoRedirectHandler() + request = type("Request", (), {"full_url": "https://ready.example"})() + with pytest.raises(urllib.error.HTTPError): + handler.redirect_request(request, None, 302, "redirect", {}, "https://other") + + +def test_stop_service_handles_finished_lookup_race_timeout_and_capture( + monkeypatch, + tmp_path: Path, +) -> None: + """Cleanup covers normal, disappearing, force-kill, and capture-finalization paths.""" + + joined: list[float | None] = [] + + class Capture: + """Record finalization of one fake background drain.""" + + output_limited = False + + def join(self, timeout=None) -> None: + """Record the requested join timeout.""" + + joined.append(timeout) + + finished = _service(tmp_path) + finished.capture = cast(bounded.BoundedOutputCapture, Capture()) + sandboxed_web_e2e.stop_service(finished) + assert joined == [10] + + disappearing = _service(tmp_path, process=_RunningProcess()) + monkeypatch.setattr( + sandboxed_web_e2e.os, + "killpg", + lambda pid, signal_number: (_ for _ in ()).throw(ProcessLookupError()), + ) + sandboxed_web_e2e.stop_service(disappearing) + + class TimeoutProcess(_RunningProcess): + """Timeout once before completing after force kill.""" + + def __init__(self) -> None: + self.waits = 0 + + def wait(self, timeout=None) -> int: + """Raise once, then return the terminal status.""" + + del timeout + self.waits += 1 + if self.waits == 1: + raise subprocess.TimeoutExpired("service", 10) + self.returncode = -9 + return self.returncode + + timeout_process = TimeoutProcess() + timed = _service(tmp_path, process=timeout_process) + monkeypatch.setattr(sandboxed_web_e2e.os, "killpg", lambda pid, sig: None) + forced: list[object] = [] + monkeypatch.setattr( + bounded, + "kill_process_group", + lambda process: forced.append(process), + ) + sandboxed_web_e2e.stop_service(timed) + assert forced == [timeout_process] + + +def test_tail_text_rejects_nonpositive_line_count(tmp_path: Path) -> None: + """A caller cannot request an ambiguous or unbounded line selection.""" + + log_path = tmp_path / "service.log" + log_path.write_text("line\n", encoding="utf-8") + with pytest.raises(ValueError, match="max_lines"): + sandboxed_web_e2e.tail_text(log_path, max_lines=0) + + +def test_timeout_precedence_survives_limited_partial_output( + monkeypatch, + tmp_path: Path, + capsys, +) -> None: + """A timed-out E2E remains 124 even when its bounded stream was truncated.""" + + repository = tmp_path / "repository" + repository.mkdir() + + def fake_start(label, command, cwd, env, logs_dir, log_limit_bytes): + """Return already-running service doubles without real children.""" + + del command, cwd, env + return sandboxed_web_e2e.Service( + label=label, + command=label, + process=cast(subprocess.Popen[bytes], _RunningProcess()), + log_path=logs_dir / f"{label}.log", + log_limit_bytes=log_limit_bytes, + ) + + def timeout_run(command, cwd, env, timeout, output_limit_bytes): + """Raise bounded timeout evidence.""" + + del command, cwd, env, output_limit_bytes + raise bounded.BoundedTimeoutExpired( + ["e2e"], + timeout, + stdout=bounded.TRUNCATION_MARKER, + stderr="", + output_limited=True, + ) + + monkeypatch.setattr(sandboxed_web_e2e, "start_service", fake_start) + monkeypatch.setattr(sandboxed_web_e2e, "wait_for_url", lambda *args: True) + monkeypatch.setattr(sandboxed_web_e2e, "run_shell", timeout_run) + monkeypatch.setattr(sandboxed_web_e2e, "stop_service", lambda service: None) + + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(repository), + "--backend-cmd", + "backend", + "--frontend-cmd", + "frontend", + "--e2e-cmd", + "e2e", + "--e2e-timeout", + "1", + ] + ) + captured = capsys.readouterr() + + assert exit_code == 124 + assert "timed out after 1s" in captured.err + assert _result(captured.out)["output_limited"] is True + + +def test_capture_finalization_failure_maps_to_resource_exit( + monkeypatch, + tmp_path: Path, + capsys, +) -> None: + """A service capture failure cannot leave a successful result envelope.""" + + repository = tmp_path / "repository" + repository.mkdir() + + def fake_start(label, command, cwd, env, logs_dir, log_limit_bytes): + """Return completed service doubles.""" + + del command, cwd, env + return sandboxed_web_e2e.Service( + label=label, + command=label, + process=cast(subprocess.Popen[bytes], _DoneProcess()), + log_path=logs_dir / f"{label}.log", + log_limit_bytes=log_limit_bytes, + ) + + monkeypatch.setattr(sandboxed_web_e2e, "start_service", fake_start) + monkeypatch.setattr(sandboxed_web_e2e, "wait_for_url", lambda *args: True) + monkeypatch.setattr( + sandboxed_web_e2e, + "run_shell", + lambda *args: bounded.BoundedCompletedProcess( + args=("e2e",), + returncode=0, + stdout="", + stderr="", + output_limited=False, + ), + ) + monkeypatch.setattr( + sandboxed_web_e2e, + "stop_service", + lambda service: (_ for _ in ()).throw(RuntimeError("capture failed")), + ) + + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(repository), + "--backend-cmd", + "backend", + "--frontend-cmd", + "frontend", + "--e2e-cmd", + "e2e", + ] + ) + captured = capsys.readouterr() + + assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE + assert "bounded service capture failed" in captured.err + assert _result(captured.out)["output_limited"] is True + + +def test_timeout_precedence_survives_cleanup_and_late_service_limit( + monkeypatch, + tmp_path: Path, +) -> None: + """Timeout 124 remains authoritative through late capture failures and overflow.""" + repository = tmp_path / "repository" + repository.mkdir() + + def fake_start(label, command, cwd, env, logs_dir, log_limit_bytes): + del command, cwd, env + return sandboxed_web_e2e.Service( + label=label, + command=label, + process=cast(subprocess.Popen[bytes], _DoneProcess()), + log_path=logs_dir / f"{label}.log", + log_limit_bytes=log_limit_bytes, + ) + + def timeout_run(*args): + del args + raise subprocess.TimeoutExpired(["e2e"], 1) + + limit_checks = iter([False, True]) + monkeypatch.setattr(sandboxed_web_e2e, "start_service", fake_start) + monkeypatch.setattr(sandboxed_web_e2e, "wait_for_url", lambda *args: True) + monkeypatch.setattr(sandboxed_web_e2e, "run_shell", timeout_run) + monkeypatch.setattr( + sandboxed_web_e2e, + "stop_service", + lambda service: (_ for _ in ()).throw(RuntimeError(service.label)), + ) + monkeypatch.setattr( + sandboxed_web_e2e, + "_services_output_limited", + lambda services: next(limit_checks), + ) + + assert sandboxed_web_e2e.main( + [ + "--repo-root", + str(repository), + "--backend-cmd", + "backend", + "--frontend-cmd", + "frontend", + "--e2e-cmd", + "e2e", + "--e2e-timeout", + "1", + ] + ) == 124 diff --git a/tests/test_sandboxed_web_e2e_output_limits.py b/tests/test_sandboxed_web_e2e_output_limits.py new file mode 100644 index 000000000..de53d6206 --- /dev/null +++ b/tests/test_sandboxed_web_e2e_output_limits.py @@ -0,0 +1,302 @@ +"""Real-process contracts for bounded sandbox web E2E output.""" + +from __future__ import annotations + +import json +import shlex +import shutil +import sys +from pathlib import Path + +import pytest + +from scripts.ci import bounded_subprocess as bounded +from scripts.ci import sandboxed_web_e2e + + +def _command(source: str) -> str: + """Return one shell-style command that safely launches the current Python.""" + + return shlex.join([sys.executable, "-c", source]) + + +def _repository(tmp_path: Path) -> Path: + """Create one minimal repository accepted by the sandbox copy boundary.""" + + repository = tmp_path / "repository" + repository.mkdir() + (repository / "README.md").write_text("web E2E fixture\n", encoding="utf-8") + return repository + + +def _result_payload(output: str) -> dict[str, object]: + """Parse the final machine-readable web E2E result marker.""" + + marker = f"{sandboxed_web_e2e.RESULT_MARKER} " + line = next( + item for item in reversed(output.splitlines()) if item.startswith(marker) + ) + return json.loads(line.removeprefix(marker)) + + +def test_start_service_enforces_real_log_file_ceiling(tmp_path: Path) -> None: + """A long-running child cannot grow its combined service log past the ceiling.""" + + logs_directory = tmp_path / "logs" + logs_directory.mkdir() + service = sandboxed_web_e2e.start_service( + "backend", + _command( + "import os\n" + "chunk=b'x'*1024\n" + "while True:\n" + " os.write(1,chunk)\n" + ), + tmp_path, + {"PATH": ""}, + logs_directory, + 4096, + ) + try: + service.process.wait(timeout=10) + assert service.log_path.stat().st_size <= 4097 + assert sandboxed_web_e2e.service_output_limited(service) + finally: + sandboxed_web_e2e.stop_service(service) + + +def test_service_log_overflow_returns_resource_limit_before_e2e( + tmp_path: Path, + capsys, +) -> None: + """Readiness cannot convert a backend log flood into an ordinary E2E run.""" + + sentinel = tmp_path / "e2e-ran" + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--backend-cmd", + _command( + "import os\n" + "chunk=b'x'*1024\n" + "while True:\n" + " os.write(1,chunk)\n" + ), + "--backend-ready-url", + "http://127.0.0.1:1/ready", + "--frontend-cmd", + _command("import time; time.sleep(30)"), + "--e2e-cmd", + _command( + f"from pathlib import Path; Path({str(sentinel)!r}).touch()" + ), + "--service-log-limit-bytes", + "4096", + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE + assert "service output exceeded 4096 bytes" in captured.err + assert payload["output_limited"] is True + assert payload["service_log_limit_bytes"] == 4096 + assert not sentinel.exists() + + +def test_e2e_output_overflow_is_bounded_and_returns_123( + tmp_path: Path, + capsys, +) -> None: + """The short-lived E2E command uses the same kernel-enforced output boundary.""" + + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--backend-cmd", + _command("import time; time.sleep(30)"), + "--frontend-cmd", + _command("import time; time.sleep(30)"), + "--e2e-cmd", + _command( + "import os\n" + "chunk=b'y'*1024\n" + "while True:\n" + " os.write(2,chunk)\n" + ), + "--output-limit-bytes", + "4096", + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE + assert bounded.TRUNCATION_MARKER.strip() in captured.err + assert "E2E output exceeded 4096 bytes" in captured.err + assert payload["output_limit_bytes"] == 4096 + assert payload["output_limited"] is True + assert len((captured.out + captured.err).encode("utf-8")) < 25_000 + + +def test_normal_services_and_e2e_preserve_existing_success_contract( + tmp_path: Path, + capsys, +) -> None: + """Ordinary services, Unicode output, cleanup, and evidence remain unchanged.""" + + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--backend-cmd", + _command("import time; print('backend-ready', flush=True); time.sleep(30)"), + "--frontend-cmd", + _command("import time; print('frontend-ready', flush=True); time.sleep(30)"), + "--e2e-cmd", + _command("print('통합 성공')"), + "--output-limit-bytes", + "4096", + "--service-log-limit-bytes", + "4096", + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == 0 + assert "통합 성공" in captured.out + assert "backend-ready" in captured.out + assert "frontend-ready" in captured.out + assert payload["output_limited"] is False + assert payload["output_limit_bytes"] == 4096 + assert payload["service_log_limit_bytes"] == 4096 + + +def test_tail_text_uses_bounded_suffix_and_tolerates_partial_utf8( + monkeypatch, + tmp_path: Path, +) -> None: + """Service evidence delegates to a byte-bounded suffix before line selection.""" + + log_path = tmp_path / "service.log" + log_path.write_bytes(b"ignored" + "가".encode("utf-8")) + observed: dict[str, object] = {} + + def fake_suffix(path: Path, maximum_bytes: int) -> bounded.BoundedText: + observed["path"] = path + observed["maximum_bytes"] = maximum_bytes + return bounded.BoundedText( + text=f"{bounded.TRUNCATION_MARKER}�\nlast-line\n", + truncated=True, + stored_bytes=10_000, + ) + + monkeypatch.setattr(bounded, "read_bounded_suffix", fake_suffix) + + tail = sandboxed_web_e2e.tail_text( + log_path, + max_lines=2, + max_bytes=4096, + ) + + assert observed == {"path": log_path, "maximum_bytes": 4096} + assert tail == "�\nlast-line" + + +def test_unsupported_resource_boundary_fails_closed( + monkeypatch, + tmp_path: Path, + capsys, +) -> None: + """Service startup cannot silently continue without file-size enforcement.""" + + def fail_start(*args, **kwargs): + del args, kwargs + raise bounded.OutputLimitUnsupportedError("unsupported") + + monkeypatch.setattr(sandboxed_web_e2e, "start_service", fail_start) + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--backend-cmd", + _command("pass"), + "--frontend-cmd", + _command("pass"), + "--e2e-cmd", + _command("pass"), + ] + ) + 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 True + + +def test_cli_rejects_unsafe_command_and_service_budgets(tmp_path: Path) -> None: + """Both output budgets fail parsing outside the explicit safe range.""" + + repository = _repository(tmp_path) + base = [ + "--repo-root", + str(repository), + "--backend-cmd", + _command("pass"), + "--frontend-cmd", + _command("pass"), + "--e2e-cmd", + _command("pass"), + ] + for option, value in [ + ("--output-limit-bytes", "4095"), + ( + "--service-log-limit-bytes", + str(bounded.MAXIMUM_OUTPUT_LIMIT_BYTES + 1), + ), + ]: + with pytest.raises(SystemExit) as raised: + sandboxed_web_e2e.parse_args([*base, option, value]) + assert raised.value.code == 2 + + +def test_kept_sandbox_service_file_never_exceeds_kernel_ceiling( + tmp_path: Path, + capsys, +) -> None: + """Persisted debugging sandboxes retain only the bounded service artifact.""" + + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--backend-cmd", + _command( + "import os\n" + "chunk=b'z'*1024\n" + "while True:\n" + " os.write(1,chunk)\n" + ), + "--frontend-cmd", + _command("import time; time.sleep(30)"), + "--e2e-cmd", + _command("pass"), + "--service-log-limit-bytes", + "4096", + "--keep-sandbox", + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + sandbox_path = Path(str(payload["sandbox"])) + + try: + assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE + assert (sandbox_path / "logs" / "backend.log").stat().st_size <= 4097 + finally: + shutil.rmtree(sandbox_path, ignore_errors=True) + From 52e128145eddf2b6d1a2137c7e6406103c54db17 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 13:15:15 +0900 Subject: [PATCH 10/15] fix(security): bound sandbox evidence streams --- CHANGELOG.md | 2 + .../sandboxed-output-resource-bounds.md | 110 +++++ scripts/ci/bounded_subprocess.py | 430 ++++++++++++++++++ scripts/ci/sandboxed_verify.py | 73 ++- scripts/ci/sandboxed_web_e2e.py | 306 ++++++++++--- 5 files changed, 845 insertions(+), 76 deletions(-) create mode 100644 docs/doctoring/sandboxed-output-resource-bounds.md create mode 100644 scripts/ci/bounded_subprocess.py diff --git a/CHANGELOG.md b/CHANGELOG.md index bf30091dd..589795bb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Bound sandbox verification stdout/stderr and long-running web-service evidence to declared final-suffix budgets, terminate isolated POSIX process groups on overflow, and classify resource exhaustion with stable exit code 123 while preserving timeout 124 and readiness 125 precedence. +- Read service evidence with a bounded seek-from-end operation and apply finite reader-finalization deadlines so excessive output, inherited pipe descriptors, or full log files cannot exhaust parent memory, disk, or workflow runtime before evidence publication. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. - Allowed commas and ASCII parentheses in the bounded Strix changed-file path policy so legal tracked Packrat fixtures can receive exact-head security analysis, while rejecting raw `..` components before normalization and keeping controls, backslashes, whitespace ambiguity, and shell punctuation fail-closed. - Bound each review-agent invocation key to the wrapper's complete canonical payload, including the base branch and requesting actor; altered fields with a valid-format key now fail before durable-leader election or forwarding, and wrapper write permission is job-scoped. diff --git a/docs/doctoring/sandboxed-output-resource-bounds.md b/docs/doctoring/sandboxed-output-resource-bounds.md new file mode 100644 index 000000000..cc38587c7 --- /dev/null +++ b/docs/doctoring/sandboxed-output-resource-bounds.md @@ -0,0 +1,110 @@ +# Sandboxed subprocess output resource bounds + +## Decision + +The central sandbox wrappers continuously drain child stdout and stderr into fixed-size final-suffix buffers before publication-stage processing. A stream that exceeds its declared byte budget terminates the isolated POSIX process group and produces stable exit code `123`. Timeout remains `124`; service-readiness failure remains `125` unless a more specific output limit occurred. + +The default retained budgets are: + +- 1,048,576 bytes for each short-lived command stream; and +- 4,194,304 bytes for each backend or frontend combined service stream. + +Configurations below 4,096 bytes or above 67,108,864 bytes are rejected before repository code executes. Every normal-path output-reader join also has a finite 30-second bound. + +## Why complete capture was unsafe + +Python's `subprocess.PIPE` creates operating-system pipes for child standard streams. Waiting without concurrently reading can deadlock when a pipe fills, while `communicate()` solves that deadlock by accumulating the complete streams in parent memory. Neither behavior supplies an evidence-size ceiling. Long-running services that write directly to ordinary files similarly consume disk until the process or runner fails, and reading the complete file merely moves that unbounded allocation into parent memory. + +The control plane therefore uses `Popen` directly, starts one reader thread per pipe immediately, reads fixed 64 KiB chunks, and retains only a locked final suffix. The first byte beyond a stream budget marks the result and kills the entire child process group created with `start_new_session=True`. Reader threads continue through EOF and are joined before bounded text is decoded or published. + +A descendant can intentionally create a new session while retaining an inherited stdout or stderr descriptor. The original process group can then terminate while the escaped descendant keeps the pipe open. For that reason, every ordinary reader finalization passes the 30-second join bound to each capture, continues to finalize sibling readers, and then re-raises the first failure. A reader still alive after that bound produces the explicit `bounded output drain did not finish` failure instead of holding the job until its outer workflow timeout. + +## Rejected process-wide file limit + +POSIX file-size resource limits apply to every regular file written by the child process. A repository verification command may legitimately create coverage databases, compiled assets, archives, package artifacts, temporary databases, or generated fixtures larger than its log budget. Applying `RLIMIT_FSIZE` to the child would therefore change application and build behavior rather than only bounding evidence. The implemented boundary constrains stdout/stderr retention and leaves ordinary repository file semantics unchanged. + +## Short-lived command boundary + +`bounded_subprocess.run_bounded_command()`: + +1. validates a structured, nonempty argument vector and positive timeout; +2. requires POSIX process-group termination and launches with `shell=False` and `start_new_session=True`; +3. connects stdout and stderr to independent binary pipes; +4. drains both pipes concurrently into separate bounded final-suffix buffers; +5. kills the process group exactly once when either stream exceeds its budget; +6. kills the group on timeout and joins both readers through the finite normal-path bound; and +7. returns or raises only bounded evidence. + +A truncation marker is included inside, not in addition to, the declared retained byte budget. Reader errors and reader-join timeouts are explicit failures. + +## Long-running service boundary + +Each backend and frontend uses one combined stdout/stderr pipe and the same bounded drainer. The capture retains the final suffix in memory and writes only its bounded rendered form to the private sandbox log file when the stream closes. The evidence file therefore cannot exceed the declared service-log budget. + +Service overflow is checked during readiness, after E2E execution, and after service shutdown. It takes precedence over an ordinary command or readiness result, but a true E2E timeout remains `124`. `tail_text()` reads no more than 65,536 bytes from the end of the already bounded file, retains the configured final line count, and publishes only that bounded suffix. Credential redaction remains a separate active integration line and is not claimed by this slice. + +A realistic regression gives the flooding backend an actual readiness URL and configures the E2E command to create a sentinel file. The overflow result must be emitted while the sentinel remains absent, proving that readiness handling cannot silently execute an ordinary E2E command before acknowledging the service evidence limit. + +## Security and availability properties + +- Parent retained memory is bounded independently for stdout and stderr. +- Service evidence disk use is bounded per service. +- Child pipes are continuously drained, preventing a full pipe from blocking the child indefinitely. +- Process-group termination covers ordinary descendants that retain inherited pipe descriptors. +- A descendant that escapes the original group cannot create an unbounded reader join. +- Structured argv and `shell=False` remain unchanged. +- Environment scrubbing, timeout enforcement, process cleanup, SSRF-safe readiness polling, and machine-readable evidence remain independent controls. The bounded capture is intentionally composable with the separately reviewed credential-redaction line. +- Non-POSIX environments fail closed rather than using unmanaged capture. +- Output overflow cannot be converted into success by the child process or into an E2E execution by readiness short-circuiting. + +MITRE CWE-770 identifies unbounded memory and other resource consumption as an availability weakness and recommends explicit minimum/maximum expectations, throttling, quotas, and safe failure when limits are reached. This implementation sets explicit per-stream ceilings, a finite finalization bound, and a stable failure result. NIST SP 800-218 supplies the secure-development framework used to define, test, and retain this control as reviewable evidence. + +No formal CWE, NIST, or POSIX conformity is claimed. + +## Verification contract + +Real subprocess tests exercise: + +- ordinary Korean Unicode stdout and stderr; +- infinite stdout and stderr floods; +- timeout with partial output; +- final-suffix retention and one overflow callback; +- bounded persisted service evidence; +- service overflow before or during readiness/E2E, including a sentinel proof that E2E never ran; +- ordinary backend/frontend/E2E success and cleanup; +- partial UTF-8 suffix decoding; +- bounded file reads; +- unsupported-platform failure; +- invalid budgets; +- reader exceptions, stuck-reader joins, a common finite join bound, and sibling finalization after the first failure; +- retained redaction of credentials in output, commands, notes, structured JSON, and service tails; and +- deterministic result fields and exit-code precedence. + +The exact pull-request head must additionally pass the complete central test suite, 100% production statement and branch coverage for the changed surface, production docstrings, Secret Scan, CodeQL, Semgrep, Python Security, dependency and supply-chain checks, OpenCode, Noema, CodeRabbit, independent current-head approval, and branch protection. + +## Limitations + +This slice does not limit: + +- repository workspace-copy size; +- application/build artifacts written outside standard streams; +- CPU time beyond the existing command timeouts; +- address space, process count, network traffic, or external service response size; or +- output generated by an unrelated process that does not inherit the managed service pipes. + +The reader buffers intentionally retain the final suffix rather than the complete beginning of an oversized stream because terminal diagnostics normally contain the most actionable failure evidence. Complete oversized logs are not retained as artifacts. + +The finite reader join converts an escaped inherited descriptor into a deterministic failure, but it does not discover or terminate arbitrary processes outside the original process group. Isolation beyond that boundary remains the responsibility of the surrounding container or runner. + +## Rollback + +Rollback must restore a different proven memory-and-disk bound for every short-lived and long-running publication path. Reverting only the process-group kill, service capture, suffix reader, or finite reader join would recreate an unbounded path around the remaining controls. Before rollback, operators must demonstrate realistic flood tests, bounded retained memory and files, finite finalization, timeout behavior, cleanup and exact-head independent review. + +## APA 7 references + +MITRE Corporation. (2026). *CWE-770: Allocation of resources without limits or throttling* (CWE Version 4.20). https://cwe.mitre.org/data/definitions/770.html + +Python Software Foundation. (2026). *subprocess—Subprocess management* (Python 3.14.6 documentation). https://docs.python.org/3.14/library/subprocess.html + +Souppaya, M., Scarfone, K., & Dodson, D. (2022). *Secure software development framework (SSDF) version 1.1: Recommendations for mitigating the risk of software vulnerabilities* (NIST Special Publication 800-218). National Institute of Standards and Technology. https://doi.org/10.6028/NIST.SP.800-218 + diff --git a/scripts/ci/bounded_subprocess.py b/scripts/ci/bounded_subprocess.py new file mode 100644 index 000000000..9987db66e --- /dev/null +++ b/scripts/ci/bounded_subprocess.py @@ -0,0 +1,430 @@ +"""Run POSIX child processes with continuously drained bounded output pipes.""" + +from __future__ import annotations + +import os +import signal +import subprocess +import threading +from collections.abc import Callable, Mapping, Sequence +from contextlib import suppress +from dataclasses import dataclass +from pathlib import Path +from typing import BinaryIO + + +OUTPUT_LIMIT_EXIT_CODE = 123 +DEFAULT_COMMAND_OUTPUT_LIMIT_BYTES = 1_048_576 +DEFAULT_SERVICE_LOG_LIMIT_BYTES = 4_194_304 +MAXIMUM_OUTPUT_LIMIT_BYTES = 67_108_864 +MINIMUM_OUTPUT_LIMIT_BYTES = 4_096 +READ_CHUNK_BYTES = 65_536 +READER_JOIN_TIMEOUT_SECONDS = 30.0 +TRUNCATION_MARKER = "...[output truncated]...\n" + + +class OutputLimitUnsupportedError(RuntimeError): + """Report that the operating system cannot isolate a child process group.""" + + +@dataclass(frozen=True) +class BoundedText: + """One bounded decoded file suffix and its original stored byte size.""" + + text: str + truncated: bool + stored_bytes: int + + +@dataclass(frozen=True) +class BoundedCompletedProcess: + """A completed child result whose output was drained into bounded buffers.""" + + args: tuple[str, ...] + returncode: int + stdout: str + stderr: str + output_limited: bool + + +class BoundedTimeoutExpired(subprocess.TimeoutExpired): + """A subprocess timeout carrying only bounded stdout and stderr evidence.""" + + def __init__( + self, + command: Sequence[str], + timeout: int | float, + *, + stdout: str, + stderr: str, + output_limited: bool, + ) -> None: + """Create timeout evidence with stable text stream attributes.""" + + super().__init__(tuple(command), timeout, output=stdout, stderr=stderr) + self.output_limited = output_limited + + +def validate_output_limit(value: object, label: str) -> int: + """Return one configured output budget inside the supported safety range.""" + + if ( + isinstance(value, bool) + or not isinstance(value, int) + or value < MINIMUM_OUTPUT_LIMIT_BYTES + or value > MAXIMUM_OUTPUT_LIMIT_BYTES + ): + raise ValueError( + f"{label} must be an integer from {MINIMUM_OUTPUT_LIMIT_BYTES} " + f"through {MAXIMUM_OUTPUT_LIMIT_BYTES}" + ) + return value + + +def _validate_read_limit(value: object) -> int: + """Return one positive bounded suffix-read size.""" + + if ( + isinstance(value, bool) + or not isinstance(value, int) + or value <= 0 + or value > MAXIMUM_OUTPUT_LIMIT_BYTES + ): + raise ValueError( + "maximum_bytes must be a positive integer no greater than " + f"{MAXIMUM_OUTPUT_LIMIT_BYTES}" + ) + return value + + +def require_supported_platform() -> None: + """Fail before execution when process-group termination is unavailable.""" + + if os.name != "posix" or not hasattr(os, "killpg"): + raise OutputLimitUnsupportedError( + "POSIX process-group support is required for bounded child output" + ) + + +def _render_bounded_bytes(buffer: bytes, limit: int, truncated: bool) -> bytes: + """Return evidence bytes no larger than the configured stream budget.""" + + if not truncated: + return buffer + marker = TRUNCATION_MARKER.encode("utf-8") + suffix_budget = max(0, limit - len(marker)) + suffix = buffer[-suffix_budget:] if suffix_budget else b"" + return marker + suffix + + +class BoundedOutputCapture: + """Continuously drain one binary pipe into a bounded final-suffix buffer.""" + + def __init__( + self, + stream: BinaryIO, + *, + evidence_limit_bytes: int, + on_limit: Callable[[], None], + destination: Path | None = None, + ) -> None: + """Start one background drain with an optional bounded evidence file.""" + + self._stream = stream + self._limit = validate_output_limit( + evidence_limit_bytes, + "evidence output limit", + ) + self._on_limit = on_limit + self._destination = destination + self._buffer = bytearray() + self._total_bytes = 0 + self._output_limited = False + self._error: BaseException | None = None + self._lock = threading.Lock() + self._thread = threading.Thread( + target=self._drain, + name="bounded-output-drain", + daemon=True, + ) + self._thread.start() + + @property + def output_limited(self) -> bool: + """Return whether this stream exceeded its configured byte budget.""" + + with self._lock: + return self._output_limited + + @property + def total_bytes(self) -> int: + """Return the complete byte count observed while draining the stream.""" + + with self._lock: + return self._total_bytes + + @property + def text(self) -> str: + """Return the bounded final suffix decoded with replacement semantics.""" + + with self._lock: + evidence = _render_bounded_bytes( + bytes(self._buffer), + self._limit, + self._output_limited, + ) + return evidence.decode("utf-8", errors="replace") + + def _append(self, chunk: bytes) -> bool: + """Append one chunk and report the first transition into limited state.""" + + should_notify = False + with self._lock: + self._total_bytes += len(chunk) + self._buffer.extend(chunk) + overflow = len(self._buffer) - self._limit + if overflow > 0: + del self._buffer[:overflow] + if self._total_bytes > self._limit and not self._output_limited: + self._output_limited = True + should_notify = True + return should_notify + + def _write_destination(self) -> None: + """Write at most the configured evidence budget to the destination file.""" + + if self._destination is None: + return + self._destination.parent.mkdir(parents=True, exist_ok=True) + with self._lock: + evidence = _render_bounded_bytes( + bytes(self._buffer), + self._limit, + self._output_limited, + ) + self._destination.write_bytes(evidence) + + def _drain(self) -> None: + """Drain until EOF, killing the child once on the first byte overflow.""" + + try: + while True: + chunk = self._stream.read(READ_CHUNK_BYTES) + if not chunk: + break + if self._append(chunk): + self._on_limit() + except BaseException as error: # noqa: BLE001 - propagated by join() + self._error = error + finally: + try: + self._stream.close() + self._write_destination() + except BaseException as error: # noqa: BLE001 - propagated by join() + if self._error is None: + self._error = error + + def join(self, timeout: float | None = None) -> None: + """Wait for EOF and re-raise any background capture failure.""" + + self._thread.join(timeout) + if self._thread.is_alive(): + raise RuntimeError("bounded output drain did not finish") + if self._error is not None: + raise self._error + + +def start_bounded_capture( + stream: BinaryIO, + *, + evidence_limit_bytes: int, + on_limit: Callable[[], None], + destination: Path | None = None, +) -> BoundedOutputCapture: + """Start one bounded background drain for a binary subprocess stream.""" + + return BoundedOutputCapture( + stream, + evidence_limit_bytes=evidence_limit_bytes, + on_limit=on_limit, + destination=destination, + ) + + +def read_bounded_suffix(path: Path, maximum_bytes: int) -> BoundedText: + """Read at most the final byte budget from one regular evidence file.""" + + read_limit = _validate_read_limit(maximum_bytes) + stored_bytes = path.stat().st_size + truncated = stored_bytes > read_limit + with path.open("rb") as captured_file: + if truncated: + captured_file.seek(stored_bytes - read_limit) + data = captured_file.read(read_limit) + decoded = data.decode("utf-8", errors="replace") + text = f"{TRUNCATION_MARKER}{decoded}" if truncated else decoded + return BoundedText( + text=text, + truncated=truncated, + stored_bytes=stored_bytes, + ) + + +def _normalized_command(arguments: Sequence[object]) -> tuple[str, ...]: + """Return one non-empty immutable structured command.""" + + command = tuple(str(argument) for argument in arguments) + if not command or not command[0]: + raise ValueError("command must contain one executable") + return command + + +def _validated_timeout(timeout: object) -> int | float: + """Return one positive numeric subprocess timeout.""" + + if ( + isinstance(timeout, bool) + or not isinstance(timeout, (int, float)) + or timeout <= 0 + ): + raise ValueError("timeout must be a positive number") + return timeout + + +def kill_process_group(process: subprocess.Popen[bytes]) -> None: + """Kill one isolated POSIX child process group exactly when still running.""" + + if process.poll() is not None: + return + try: + os.killpg(process.pid, signal.SIGKILL) + except ProcessLookupError: + return + + +def _join_captures( + captures: Sequence[BoundedOutputCapture], + timeout: float = READER_JOIN_TIMEOUT_SECONDS, +) -> None: + """Finalize every stream reader within one finite per-reader deadline.""" + + first_error: BaseException | None = None + for capture in captures: + try: + capture.join(timeout) + except BaseException as error: # noqa: BLE001 - re-raised after sibling join + if first_error is None: + first_error = error + if first_error is not None: + raise first_error + + +def _cleanup_capture_startup_failure( + process: subprocess.Popen[bytes], + captures: Sequence[BoundedOutputCapture], + streams: Sequence[BinaryIO], +) -> None: + """Best-effort terminate, reap, finalize, and close partial startup state.""" + + with suppress(BaseException): + kill_process_group(process) + with suppress(BaseException): + process.wait(timeout=10) + for capture in captures: + with suppress(BaseException): + capture.join(timeout=10) + for stream in streams: + with suppress(BaseException): + stream.close() + for capture in captures: + with suppress(BaseException): + capture.join(timeout=10) + + +def run_bounded_command( + arguments: Sequence[object], + *, + cwd: Path, + env: Mapping[str, str], + timeout: int | float, + evidence_limit_bytes: int, +) -> BoundedCompletedProcess: + """Run a structured command while continuously draining bounded pipe suffixes.""" + + require_supported_platform() + command = _normalized_command(arguments) + timeout_seconds = _validated_timeout(timeout) + evidence_limit = validate_output_limit( + evidence_limit_bytes, + "evidence output limit", + ) + process = subprocess.Popen( + list(command), + cwd=cwd, + env=dict(env), + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + bufsize=0, + shell=False, + start_new_session=True, + ) + if process.stdout is None or process.stderr is None: + kill_process_group(process) + process.wait() + raise RuntimeError("subprocess pipes were not created") + + limit_triggered = threading.Event() + + def stop_for_limit() -> None: + """Kill the process group only for the first overflowing stream.""" + + if not limit_triggered.is_set(): + limit_triggered.set() + kill_process_group(process) + + captures: list[BoundedOutputCapture] = [] + streams = (process.stdout, process.stderr) + try: + stdout_capture = start_bounded_capture( + process.stdout, + evidence_limit_bytes=evidence_limit, + on_limit=stop_for_limit, + ) + captures.append(stdout_capture) + stderr_capture = start_bounded_capture( + process.stderr, + evidence_limit_bytes=evidence_limit, + on_limit=stop_for_limit, + ) + captures.append(stderr_capture) + except BaseException: # noqa: BLE001 - preserve the startup root cause + _cleanup_capture_startup_failure(process, captures, streams) + raise + timed_out = False + try: + process.wait(timeout=timeout_seconds) + except subprocess.TimeoutExpired: + timed_out = True + kill_process_group(process) + process.wait() + + _join_captures((stdout_capture, stderr_capture)) + output_limited = ( + stdout_capture.output_limited or stderr_capture.output_limited + ) + if timed_out: + raise BoundedTimeoutExpired( + command, + timeout_seconds, + stdout=stdout_capture.text, + stderr=stderr_capture.text, + output_limited=output_limited, + ) + return BoundedCompletedProcess( + args=command, + returncode=process.returncode, + stdout=stdout_capture.text, + stderr=stderr_capture.text, + output_limited=output_limited, + ) + diff --git a/scripts/ci/sandboxed_verify.py b/scripts/ci/sandboxed_verify.py index 97b9f31f1..0d3a79dcc 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", @@ -69,6 +74,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 +117,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}") @@ -180,18 +198,20 @@ 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]: - """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, ) @@ -215,6 +235,8 @@ 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, ) -> None: """Print a machine-readable execution evidence summary.""" payload = { @@ -225,9 +247,12 @@ def emit_result( "evidence_note": evidence_note, "exit_code": exit_code, "network": network, + "output_limit_bytes": output_limit_bytes, + "output_limited": output_limited, "sandbox": str(sandbox_root) if kept else "(removed)", "sandboxed": True, } + print() print(f"{RESULT_MARKER} {json.dumps(payload, sort_keys=True)}") @@ -237,6 +262,7 @@ def main(argv: Sequence[str] | None = None) -> int: sandbox = Path(tempfile.mkdtemp(prefix="sandboxed-verify-")) start = time.monotonic() exit_code = 1 + output_limited = False copied_repo = sandbox / "repo" try: copied_repo = copy_workspace(Path(args.repo_root), sandbox, args.ignore) @@ -248,12 +274,34 @@ 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 = bool(getattr(completed, "output_limited", False)) + 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 bounded_subprocess.OutputLimitUnsupportedError: + output_limited = True + print( + "sandboxed-verify: bounded child output is unavailable on this platform", + 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) @@ -261,6 +309,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 @@ -276,6 +325,8 @@ 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, ) if not args.keep_sandbox: shutil.rmtree(sandbox, ignore_errors=True) diff --git a/scripts/ci/sandboxed_web_e2e.py b/scripts/ci/sandboxed_web_e2e.py index ae0c3105a..d013965b3 100644 --- a/scripts/ci/sandboxed_web_e2e.py +++ b/scripts/ci/sandboxed_web_e2e.py @@ -3,6 +3,7 @@ from __future__ import annotations import argparse +import contextlib import json import os import signal @@ -17,14 +18,16 @@ from collections.abc import Sequence from dataclasses import dataclass from pathlib import Path +from typing import BinaryIO if __package__ in (None, ""): sys.path.insert(0, str(Path(__file__).resolve().parents[2])) -from scripts.ci import sandboxed_verify +from scripts.ci import bounded_subprocess, sandboxed_verify RESULT_MARKER = "SANDBOXED_WEB_E2E_RESULT" +DEFAULT_TAIL_BYTES = 65_536 class NoRedirectHandler(urllib.request.HTTPRedirectHandler): @@ -37,12 +40,14 @@ def redirect_request(self, req, fp, code, msg, headers, newurl): @dataclass class Service: - """A long-running web service process and its log file.""" + """A long-running web service process and its bounded combined log capture.""" label: str command: str - process: subprocess.Popen[str] + process: subprocess.Popen[bytes] log_path: Path + capture: bounded_subprocess.BoundedOutputCapture | None = None + log_limit_bytes: int = bounded_subprocess.DEFAULT_SERVICE_LOG_LIMIT_BYTES def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: @@ -62,6 +67,18 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: parser.add_argument("--frontend-ready-url", default="", help="Frontend readiness URL to poll before E2E.") parser.add_argument("--startup-timeout", type=int, default=120, help="Seconds to wait for readiness URLs.") parser.add_argument("--e2e-timeout", type=int, default=600, help="Seconds to allow the E2E command to run.") + parser.add_argument( + "--output-limit-bytes", + type=int, + default=bounded_subprocess.DEFAULT_COMMAND_OUTPUT_LIMIT_BYTES, + help="Maximum retained stdout and stderr bytes for the E2E command.", + ) + parser.add_argument( + "--service-log-limit-bytes", + type=int, + default=bounded_subprocess.DEFAULT_SERVICE_LOG_LIMIT_BYTES, + help="Maximum retained combined log bytes for each long-running service.", + ) parser.add_argument("--keep-sandbox", action="store_true", help="Keep the temporary sandbox after execution.") parser.add_argument( "--allow-env", @@ -92,31 +109,97 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: parser.error("--startup-timeout must be positive") if args.e2e_timeout <= 0: parser.error("--e2e-timeout must be positive") + try: + args.output_limit_bytes = bounded_subprocess.validate_output_limit( + args.output_limit_bytes, + "--output-limit-bytes", + ) + args.service_log_limit_bytes = bounded_subprocess.validate_output_limit( + args.service_log_limit_bytes, + "--service-log-limit-bytes", + ) + except ValueError as error: + parser.error(str(error)) for name in args.allow_env: if not sandboxed_verify.ENV_NAME_RE.match(name): parser.error(f"--allow-env must be an environment variable name: {name}") return args -def start_service(label: str, command: str, cwd: Path, env: dict[str, str], logs_dir: Path) -> Service: - """Start a service command in its own process group.""" +def _cleanup_failed_service_start( + process: subprocess.Popen[bytes], + stream: BinaryIO, +) -> None: + """Best-effort stop, reap, and close after bounded capture startup fails.""" + with contextlib.suppress(OSError, subprocess.SubprocessError): + bounded_subprocess.kill_process_group(process) + with contextlib.suppress(OSError, subprocess.SubprocessError): + process.wait(timeout=10) + with contextlib.suppress(OSError): + stream.close() + + +def start_service( + label: str, + command: str, + cwd: Path, + env: dict[str, str], + logs_dir: Path, + log_limit_bytes: int = bounded_subprocess.DEFAULT_SERVICE_LOG_LIMIT_BYTES, +) -> Service: + """Start one service group and continuously drain its combined bounded log.""" + bounded_subprocess.require_supported_platform() + log_limit = bounded_subprocess.validate_output_limit( + log_limit_bytes, + "service log limit", + ) log_path = logs_dir / f"{label}.log" - log_file = log_path.open("w", encoding="utf-8") process = subprocess.Popen( shlex.split(command), cwd=cwd, env=env, - text=True, - stdout=log_file, + stdout=subprocess.PIPE, stderr=subprocess.STDOUT, + bufsize=0, start_new_session=True, + shell=False, + ) + if process.stdout is None: + bounded_subprocess.kill_process_group(process) + process.wait() + raise RuntimeError("service output pipe was not created") + try: + capture = bounded_subprocess.start_bounded_capture( + process.stdout, + evidence_limit_bytes=log_limit, + on_limit=lambda: bounded_subprocess.kill_process_group(process), + destination=log_path, + ) + except BaseException: + _cleanup_failed_service_start(process, process.stdout) + raise + return Service( + label=label, + command=command, + process=process, + log_path=log_path, + capture=capture, + log_limit_bytes=log_limit, + ) + + +def service_output_limited(service: Service) -> bool: + """Return whether one service exceeded its declared combined log budget.""" + if service.capture is not None: + return service.capture.output_limited + return ( + service.log_path.exists() + and service.log_path.stat().st_size > service.log_limit_bytes ) - log_file.close() - return Service(label=label, command=command, process=process, log_path=log_path) def wait_for_url(url: str, timeout: int, service: Service) -> bool: - """Poll a readiness URL until it responds or the service exits.""" + """Poll a readiness URL until it responds, exits, or exceeds its log budget.""" if not url: return True if not (url.startswith("http://") or url.startswith("https://")): @@ -124,7 +207,7 @@ def wait_for_url(url: str, timeout: int, service: Service) -> bool: deadline = time.monotonic() + timeout opener = urllib.request.build_opener(NoRedirectHandler()) while time.monotonic() < deadline: - if service.process.poll() is not None: + if service_output_limited(service) or service.process.poll() is not None: return False try: with opener.open(url, timeout=2) as response: # nosec B310 @@ -135,40 +218,50 @@ def wait_for_url(url: str, timeout: int, service: Service) -> bool: return False -def run_shell(command: str, cwd: Path, env: dict[str, str], timeout: int) -> subprocess.CompletedProcess[str]: - """Run a shell command and capture its output.""" - return subprocess.run( +def run_shell( + command: 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 shell-style command without a shell and with bounded pipe drains.""" + return bounded_subprocess.run_bounded_command( shlex.split(command), cwd=cwd, env=env, - text=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, timeout=timeout, - check=False, + evidence_limit_bytes=output_limit_bytes, ) def stop_service(service: Service) -> None: - """Terminate a service process group and wait briefly for cleanup.""" - if service.process.poll() is not None: - return - try: - os.killpg(service.process.pid, signal.SIGTERM) - service.process.wait(timeout=10) - except (ProcessLookupError, subprocess.TimeoutExpired): + """Terminate a service process group and finalize its bounded log evidence.""" + if service.process.poll() is None: try: - os.killpg(service.process.pid, signal.SIGKILL) + os.killpg(service.process.pid, signal.SIGTERM) + service.process.wait(timeout=10) except ProcessLookupError: - return - service.process.wait(timeout=10) + pass + except subprocess.TimeoutExpired: + bounded_subprocess.kill_process_group(service.process) + service.process.wait(timeout=10) + if service.capture is not None: + service.capture.join(timeout=10) -def tail_text(path: Path, max_lines: int = 80) -> str: - """Return the final lines of a service log.""" +def tail_text( + path: Path, + max_lines: int = 80, + max_bytes: int = DEFAULT_TAIL_BYTES, +) -> str: + """Return final lines after a byte-bounded service evidence read.""" if not path.exists(): return "" - lines = path.read_text(encoding="utf-8", errors="replace").splitlines() + if max_lines <= 0: + raise ValueError("max_lines must be positive") + bounded_text = bounded_subprocess.read_bounded_suffix(path, max_bytes) + lines = bounded_text.text.splitlines() return "\n".join(lines[-max_lines:]) @@ -181,6 +274,7 @@ def emit_result( frontend_ready: bool, exit_code: int, elapsed_seconds: float, + output_limited: bool, ) -> None: """Print a machine-readable web E2E execution evidence summary.""" payload = { @@ -195,12 +289,21 @@ def emit_result( "frontend_cmd": args.frontend_cmd, "frontend_ready": frontend_ready, "network": args.network, + "output_limit_bytes": args.output_limit_bytes, + "output_limited": output_limited, "sandbox": str(sandbox_root) if args.keep_sandbox else "(removed)", "sandboxed": True, + "service_log_limit_bytes": args.service_log_limit_bytes, } + print() print(f"{RESULT_MARKER} {json.dumps(payload, sort_keys=True)}") +def _services_output_limited(services: Sequence[Service]) -> bool: + """Return whether any started service exceeded its combined log budget.""" + return any(service_output_limited(service) for service in services) + + def main(argv: Sequence[str] | None = None) -> int: """Run backend, frontend, and E2E commands inside a sandbox copy.""" args = parse_args(argv) @@ -212,44 +315,115 @@ def main(argv: Sequence[str] | None = None) -> int: backend_ready = False frontend_ready = False exit_code = 1 + output_limited = False + service_limit_reported = False start = time.monotonic() try: - copied_repo = sandboxed_verify.copy_workspace(Path(args.repo_root), sandbox, args.ignore) - env = sandboxed_verify.scrubbed_env(sandbox, args.allow_env) - print(f"sandboxed-web-e2e: cwd={copied_repo}") - if args.allow_env: - print(f"sandboxed-web-e2e: allowed env names={','.join(sorted(set(args.allow_env)))}") - if args.network != "default": - print(f"sandboxed-web-e2e: network={args.network}") - services.append(start_service("backend", args.backend_cmd, copied_repo, env, logs_dir)) - services.append(start_service("frontend", args.frontend_cmd, copied_repo, env, logs_dir)) - backend_ready = wait_for_url(args.backend_ready_url, args.startup_timeout, services[0]) - frontend_ready = wait_for_url(args.frontend_ready_url, args.startup_timeout, services[1]) - if not backend_ready or not frontend_ready: - print("sandboxed-web-e2e: service readiness failed", file=sys.stderr) - exit_code = 125 - return exit_code try: - completed = run_shell(args.e2e_cmd, copied_repo, env, args.e2e_timeout) - if completed.stdout: - print(completed.stdout, end="") - if completed.stderr: - print(completed.stderr, end="", file=sys.stderr) - exit_code = completed.returncode - return exit_code - except subprocess.TimeoutExpired as exc: - stdout = sandboxed_verify.timeout_output_text(exc.stdout) - stderr = sandboxed_verify.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-web-e2e: e2e command timed out after {args.e2e_timeout}s", file=sys.stderr) - exit_code = 124 - return exit_code + copied_repo = sandboxed_verify.copy_workspace(Path(args.repo_root), sandbox, args.ignore) + env = sandboxed_verify.scrubbed_env(sandbox, args.allow_env) + print(f"sandboxed-web-e2e: cwd={copied_repo}") + if args.allow_env: + print(f"sandboxed-web-e2e: allowed env names={','.join(sorted(set(args.allow_env)))}") + if args.network != "default": + print(f"sandboxed-web-e2e: network={args.network}") + services.append( + start_service( + "backend", + args.backend_cmd, + copied_repo, + env, + logs_dir, + args.service_log_limit_bytes, + ) + ) + services.append( + start_service( + "frontend", + args.frontend_cmd, + copied_repo, + env, + logs_dir, + args.service_log_limit_bytes, + ) + ) + backend_ready = wait_for_url(args.backend_ready_url, args.startup_timeout, services[0]) + frontend_ready = wait_for_url(args.frontend_ready_url, args.startup_timeout, services[1]) + if _services_output_limited(services): + output_limited = True + service_limit_reported = True + print( + "sandboxed-web-e2e: service output exceeded " + f"{args.service_log_limit_bytes} bytes", + file=sys.stderr, + ) + exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE + elif not backend_ready or not frontend_ready: + print("sandboxed-web-e2e: service readiness failed", file=sys.stderr) + exit_code = 125 + else: + try: + completed = run_shell( + args.e2e_cmd, + copied_repo, + env, + args.e2e_timeout, + args.output_limit_bytes, + ) + if completed.stdout: + print(completed.stdout, end="") + if completed.stderr: + print(completed.stderr, end="", file=sys.stderr) + output_limited = bool(getattr(completed, "output_limited", False)) + if output_limited: + print( + "sandboxed-web-e2e: E2E 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 subprocess.TimeoutExpired as exc: + stdout = sandboxed_verify.timeout_output_text(exc.stdout) + stderr = sandboxed_verify.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) + output_limited = bool(getattr(exc, "output_limited", False)) + print(f"sandboxed-web-e2e: e2e command timed out after {args.e2e_timeout}s", file=sys.stderr) + exit_code = 124 + except bounded_subprocess.OutputLimitUnsupportedError: + output_limited = True + print( + "sandboxed-web-e2e: bounded child output is unavailable on this platform", + file=sys.stderr, + ) + exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE finally: for service in reversed(services): - stop_service(service) + try: + stop_service(service) + except (OSError, RuntimeError, subprocess.SubprocessError): + output_limited = True + if exit_code != 124: + exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE + print( + "sandboxed-web-e2e: bounded service capture failed", + file=sys.stderr, + ) + if _services_output_limited(services): + output_limited = True + if exit_code != 124: + exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE + if not service_limit_reported: + print( + "sandboxed-web-e2e: service output exceeded " + f"{args.service_log_limit_bytes} bytes", + file=sys.stderr, + ) + for service in reversed(services): log_tail = tail_text(service.log_path) if log_tail: print(f"--- {service.label} log tail ---") @@ -262,9 +436,11 @@ def main(argv: Sequence[str] | None = None) -> int: frontend_ready=frontend_ready, exit_code=exit_code, elapsed_seconds=time.monotonic() - start, + output_limited=output_limited, ) if not args.keep_sandbox: shutil.rmtree(sandbox, ignore_errors=True) + return exit_code if __name__ == "__main__": From 7b29a202ec8d511cac7b3d3650cfb87e4c3367fc Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 14:21:06 +0900 Subject: [PATCH 11/15] fix(sandbox): preserve capture ownership evidence --- CHANGELOG.md | 1 + scripts/ci/bounded_subprocess.py | 13 ++++++++++--- scripts/ci/sandboxed_verify.py | 8 ++++++-- tests/test_sandboxed_verify_output_limits.py | 4 ++-- tests/test_sandboxed_verify_symlink_boundary.py | 15 +++++++++++++++ 5 files changed, 34 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 589795bb3..d5c2b4fa9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Semantic Versioning where the repository publishes a release. - Bound sandbox verification stdout/stderr and long-running web-service evidence to declared final-suffix budgets, terminate isolated POSIX process groups on overflow, and classify resource exhaustion with stable exit code 123 while preserving timeout 124 and readiness 125 precedence. - Read service evidence with a bounded seek-from-end operation and apply finite reader-finalization deadlines so excessive output, inherited pipe descriptors, or full log files cannot exhaust parent memory, disk, or workflow runtime before evidence publication. +- Reject absolute and repository-escaping symbolic links in the copied verification workspace before the untrusted command runs, while retaining safe repository-internal relative links and ignoring excluded paths. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. - Allowed commas and ASCII parentheses in the bounded Strix changed-file path policy so legal tracked Packrat fixtures can receive exact-head security analysis, while rejecting raw `..` components before normalization and keeping controls, backslashes, whitespace ambiguity, and shell punctuation fail-closed. - Bound each review-agent invocation key to the wrapper's complete canonical payload, including the base branch and requesting actor; altered fields with a valid-format key now fail before durable-leader election or forwarding, and wrapper write permission is job-scoped. diff --git a/scripts/ci/bounded_subprocess.py b/scripts/ci/bounded_subprocess.py index 9987db66e..38f24cad8 100644 --- a/scripts/ci/bounded_subprocess.py +++ b/scripts/ci/bounded_subprocess.py @@ -112,8 +112,8 @@ def _render_bounded_bytes(buffer: bytes, limit: int, truncated: bool) -> bytes: if not truncated: return buffer marker = TRUNCATION_MARKER.encode("utf-8") - suffix_budget = max(0, limit - len(marker)) - suffix = buffer[-suffix_budget:] if suffix_budget else b"" + suffix_budget = limit - len(marker) + suffix = buffer[-suffix_budget:] return marker + suffix @@ -149,6 +149,11 @@ def __init__( ) self._thread.start() + @property + def stream(self) -> BinaryIO: + """Return the binary stream owned by this background capture.""" + return self._stream + @property def output_limited(self) -> bool: """Return whether this stream exceeded its configured byte budget.""" @@ -333,7 +338,10 @@ def _cleanup_capture_startup_failure( for capture in captures: with suppress(BaseException): capture.join(timeout=10) + owned_streams = {id(capture.stream) for capture in captures} for stream in streams: + if id(stream) in owned_streams: + continue with suppress(BaseException): stream.close() for capture in captures: @@ -427,4 +435,3 @@ def stop_for_limit() -> None: stderr=stderr_capture.text, output_limited=output_limited, ) - diff --git a/scripts/ci/sandboxed_verify.py b/scripts/ci/sandboxed_verify.py index 0d3a79dcc..2502263eb 100644 --- a/scripts/ci/sandboxed_verify.py +++ b/scripts/ci/sandboxed_verify.py @@ -237,6 +237,7 @@ def emit_result( evidence_note: str, output_limit_bytes: int = bounded_subprocess.DEFAULT_COMMAND_OUTPUT_LIMIT_BYTES, output_limited: bool = False, + output_limit_unsupported: bool = False, ) -> None: """Print a machine-readable execution evidence summary.""" payload = { @@ -249,6 +250,7 @@ def emit_result( "network": network, "output_limit_bytes": output_limit_bytes, "output_limited": output_limited, + "output_limit_unsupported": output_limit_unsupported, "sandbox": str(sandbox_root) if kept else "(removed)", "sandboxed": True, } @@ -263,6 +265,7 @@ def main(argv: Sequence[str] | None = None) -> int: start = time.monotonic() exit_code = 1 output_limited = False + output_limit_unsupported = False copied_repo = sandbox / "repo" try: copied_repo = copy_workspace(Path(args.repo_root), sandbox, args.ignore) @@ -285,7 +288,7 @@ def main(argv: Sequence[str] | None = None) -> int: print(completed.stdout, end="") if completed.stderr: print(completed.stderr, end="", file=sys.stderr) - output_limited = bool(getattr(completed, "output_limited", False)) + output_limited = completed.output_limited if output_limited: print( "sandboxed-verify: command output exceeded " @@ -296,7 +299,7 @@ def main(argv: Sequence[str] | None = None) -> int: else: exit_code = completed.returncode except bounded_subprocess.OutputLimitUnsupportedError: - output_limited = True + output_limit_unsupported = True print( "sandboxed-verify: bounded child output is unavailable on this platform", file=sys.stderr, @@ -327,6 +330,7 @@ def main(argv: Sequence[str] | None = None) -> int: evidence_note=args.evidence_note, output_limit_bytes=args.output_limit_bytes, output_limited=output_limited, + output_limit_unsupported=output_limit_unsupported, ) if not args.keep_sandbox: shutil.rmtree(sandbox, ignore_errors=True) diff --git a/tests/test_sandboxed_verify_output_limits.py b/tests/test_sandboxed_verify_output_limits.py index 2dd0951e7..448197cb7 100644 --- a/tests/test_sandboxed_verify_output_limits.py +++ b/tests/test_sandboxed_verify_output_limits.py @@ -155,7 +155,8 @@ def fail_run(*args, **kwargs): assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE assert "bounded child output is unavailable" in captured.err - assert payload["output_limited"] is True + assert payload["output_limited"] is False + assert payload["output_limit_unsupported"] is True def test_cli_rejects_output_budgets_outside_supported_range( @@ -177,4 +178,3 @@ def test_cli_rejects_output_budgets_outside_supported_range( ] ) assert raised.value.code == 2 - diff --git a/tests/test_sandboxed_verify_symlink_boundary.py b/tests/test_sandboxed_verify_symlink_boundary.py index 1cd1739c3..b9ce1e1cf 100644 --- a/tests/test_sandboxed_verify_symlink_boundary.py +++ b/tests/test_sandboxed_verify_symlink_boundary.py @@ -1,5 +1,7 @@ """Security contracts for sandboxed verification symlink handling.""" +import json + import subprocess from pathlib import Path @@ -64,6 +66,7 @@ def test_copy_workspace_does_not_validate_ignored_symlinks(tmp_path: Path) -> No 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" @@ -80,3 +83,15 @@ def timeout_runner(*_args: object, **_kwargs: object) -> None: ) == 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 From ce370cf8df6c41581fc06019efe2364798248dec Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 15:21:33 +0900 Subject: [PATCH 12/15] fix(sandbox): classify path and cleanup failures --- CHANGELOG.md | 1 + .../sandboxed-output-resource-bounds.md | 6 +++- ...sandboxed-verification-symlink-boundary.md | 15 ++++++++-- scripts/ci/sandboxed_verify.py | 24 +++++++++++++-- scripts/ci/sandboxed_web_e2e.py | 6 ++++ tests/test_bounded_subprocess.py | 1 + .../test_sandboxed_verify_symlink_boundary.py | 30 +++++++++++++++++++ .../test_sandboxed_web_e2e_branch_contract.py | 7 +++++ 8 files changed, 83 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5c2b4fa9..6d7d99c87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Semantic Versioning where the repository publishes a release. - Bound sandbox verification stdout/stderr and long-running web-service evidence to declared final-suffix budgets, terminate isolated POSIX process groups on overflow, and classify resource exhaustion with stable exit code 123 while preserving timeout 124 and readiness 125 precedence. - Read service evidence with a bounded seek-from-end operation and apply finite reader-finalization deadlines so excessive output, inherited pipe descriptors, or full log files cannot exhaust parent memory, disk, or workflow runtime before evidence publication. - Reject absolute and repository-escaping symbolic links in the copied verification workspace before the untrusted command runs, while retaining safe repository-internal relative links and ignoring excluded paths. +- Classify copied-workspace path rejection with stable exit code 126 and machine-readable evidence, and force-kill each service process group if bounded capture finalization fails so cleanup errors cannot leave orphaned descendants. - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. - Allowed commas and ASCII parentheses in the bounded Strix changed-file path policy so legal tracked Packrat fixtures can receive exact-head security analysis, while rejecting raw `..` components before normalization and keeping controls, backslashes, whitespace ambiguity, and shell punctuation fail-closed. - Bound each review-agent invocation key to the wrapper's complete canonical payload, including the base branch and requesting actor; altered fields with a valid-format key now fail before durable-leader election or forwarding, and wrapper write permission is job-scoped. diff --git a/docs/doctoring/sandboxed-output-resource-bounds.md b/docs/doctoring/sandboxed-output-resource-bounds.md index cc38587c7..6b00cb5d8 100644 --- a/docs/doctoring/sandboxed-output-resource-bounds.md +++ b/docs/doctoring/sandboxed-output-resource-bounds.md @@ -43,6 +43,11 @@ Each backend and frontend uses one combined stdout/stderr pipe and the same boun Service overflow is checked during readiness, after E2E execution, and after service shutdown. It takes precedence over an ordinary command or readiness result, but a true E2E timeout remains `124`. `tail_text()` reads no more than 65,536 bytes from the end of the already bounded file, retains the configured final line count, and publishes only that bounded suffix. Credential redaction remains a separate active integration line and is not claimed by this slice. +If orderly service or capture finalization raises, the wrapper makes a second +best-effort process-group kill and bounded reap before publishing resource +failure evidence. A capture error therefore cannot silently leave the backend +or frontend process group running after the ordinary shutdown path aborts. + A realistic regression gives the flooding backend an actual readiness URL and configures the E2E command to create a sentinel file. The overflow result must be emitted while the sentinel remains absent, proving that readiness handling cannot silently execute an ordinary E2E command before acknowledging the service evidence limit. ## Security and availability properties @@ -107,4 +112,3 @@ MITRE Corporation. (2026). *CWE-770: Allocation of resources without limits or t Python Software Foundation. (2026). *subprocess—Subprocess management* (Python 3.14.6 documentation). https://docs.python.org/3.14/library/subprocess.html Souppaya, M., Scarfone, K., & Dodson, D. (2022). *Secure software development framework (SSDF) version 1.1: Recommendations for mitigating the risk of software vulnerabilities* (NIST Special Publication 800-218). National Institute of Standards and Technology. https://doi.org/10.6028/NIST.SP.800-218 - diff --git a/docs/doctoring/sandboxed-verification-symlink-boundary.md b/docs/doctoring/sandboxed-verification-symlink-boundary.md index 0fa28c7f0..356c4c7c8 100644 --- a/docs/doctoring/sandboxed-verification-symlink-boundary.md +++ b/docs/doctoring/sandboxed-verification-symlink-boundary.md @@ -20,9 +20,18 @@ 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 and 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. +fail-closed with stable exit code `126`, `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. ## Test-first evidence diff --git a/scripts/ci/sandboxed_verify.py b/scripts/ci/sandboxed_verify.py index 2502263eb..42f9bd77c 100644 --- a/scripts/ci/sandboxed_verify.py +++ b/scripts/ci/sandboxed_verify.py @@ -61,9 +61,14 @@ "PYTHONPATH", ) RESULT_MARKER = "SANDBOXED_VERIFY_RESULT" +PATH_BOUNDARY_EXIT_CODE = 126 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.""" + + def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: """Parse CLI arguments for the sandboxed verification wrapper.""" parser = argparse.ArgumentParser( @@ -173,7 +178,7 @@ def validate_repository_symlinks(source: Path) -> None: continue target = Path(os.readlink(candidate)) if target.is_absolute(): - raise ValueError( + raise RepositoryPathBoundaryError( f"symlink escapes repository verification sandbox via absolute target: " f"{candidate} -> {target}" ) @@ -181,7 +186,7 @@ def validate_repository_symlinks(source: Path) -> None: try: resolved_target.relative_to(source_root) except ValueError as exc: - raise ValueError( + raise RepositoryPathBoundaryError( f"symlink escapes repository verification sandbox: {candidate} -> {target}" ) from exc @@ -238,6 +243,7 @@ def emit_result( 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 = { @@ -251,6 +257,7 @@ def emit_result( "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, } @@ -266,9 +273,19 @@ def main(argv: Sequence[str] | None = None) -> int: 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 + print( + "sandboxed-verify: repository path boundary rejected", + file=sys.stderr, + ) + exit_code = PATH_BOUNDARY_EXIT_CODE + 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)}") @@ -331,6 +348,7 @@ def main(argv: Sequence[str] | None = None) -> int: 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/scripts/ci/sandboxed_web_e2e.py b/scripts/ci/sandboxed_web_e2e.py index d013965b3..cbf6fae80 100644 --- a/scripts/ci/sandboxed_web_e2e.py +++ b/scripts/ci/sandboxed_web_e2e.py @@ -406,6 +406,12 @@ def main(argv: Sequence[str] | None = None) -> int: try: stop_service(service) except (OSError, RuntimeError, subprocess.SubprocessError): + with contextlib.suppress(OSError, subprocess.SubprocessError): + bounded_subprocess.kill_process_group(service.process) + with contextlib.suppress(OSError, subprocess.SubprocessError): + wait = getattr(service.process, "wait", None) + if wait is not None: + wait(timeout=10) output_limited = True if exit_code != 124: exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE diff --git a/tests/test_bounded_subprocess.py b/tests/test_bounded_subprocess.py index 741c43b7a..2b4d6db78 100644 --- a/tests/test_bounded_subprocess.py +++ b/tests/test_bounded_subprocess.py @@ -220,6 +220,7 @@ def is_alive(self) -> bool: evidence_limit_bytes=4096, on_limit=lambda: None, ) + assert capture.stream is capture._stream # noqa: SLF001 - ownership contract monkeypatch.setattr(capture, "_thread", NeverFinishesThread()) with pytest.raises(RuntimeError, match="did not finish"): capture.join(timeout=0) diff --git a/tests/test_sandboxed_verify_symlink_boundary.py b/tests/test_sandboxed_verify_symlink_boundary.py index b9ce1e1cf..45d7feb8a 100644 --- a/tests/test_sandboxed_verify_symlink_boundary.py +++ b/tests/test_sandboxed_verify_symlink_boundary.py @@ -63,6 +63,36 @@ def test_copy_workspace_does_not_validate_ignored_symlinks(tmp_path: Path) -> No 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 "repository path boundary rejected" in captured.err + assert str(sensitive_target) 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, diff --git a/tests/test_sandboxed_web_e2e_branch_contract.py b/tests/test_sandboxed_web_e2e_branch_contract.py index d9678a2a7..ef7e1af9f 100644 --- a/tests/test_sandboxed_web_e2e_branch_contract.py +++ b/tests/test_sandboxed_web_e2e_branch_contract.py @@ -396,6 +396,12 @@ def fake_start(label, command, cwd, env, logs_dir, log_limit_bytes): "stop_service", lambda service: (_ for _ in ()).throw(RuntimeError("capture failed")), ) + forced: list[object] = [] + monkeypatch.setattr( + bounded, + "kill_process_group", + lambda process: forced.append(process), + ) exit_code = sandboxed_web_e2e.main( [ @@ -414,6 +420,7 @@ def fake_start(label, command, cwd, env, logs_dir, log_limit_bytes): assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE assert "bounded service capture failed" in captured.err assert _result(captured.out)["output_limited"] is True + assert len(forced) == 2 def test_timeout_precedence_survives_cleanup_and_late_service_limit( From 92990d21766f289cc9917de1037a1c7d34ddd675 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 15:29:36 +0900 Subject: [PATCH 13/15] fix(sandbox): validate missing-log budgets --- scripts/ci/sandboxed_web_e2e.py | 4 ++-- tests/test_sandboxed_web_e2e_branch_contract.py | 7 +++++++ tests/test_sandboxed_web_e2e_output_limits.py | 3 +-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/scripts/ci/sandboxed_web_e2e.py b/scripts/ci/sandboxed_web_e2e.py index cbf6fae80..e1d8fb80f 100644 --- a/scripts/ci/sandboxed_web_e2e.py +++ b/scripts/ci/sandboxed_web_e2e.py @@ -256,10 +256,10 @@ def tail_text( max_bytes: int = DEFAULT_TAIL_BYTES, ) -> str: """Return final lines after a byte-bounded service evidence read.""" - if not path.exists(): - return "" if max_lines <= 0: raise ValueError("max_lines must be positive") + if not path.exists(): + return "" bounded_text = bounded_subprocess.read_bounded_suffix(path, max_bytes) lines = bounded_text.text.splitlines() return "\n".join(lines[-max_lines:]) diff --git a/tests/test_sandboxed_web_e2e_branch_contract.py b/tests/test_sandboxed_web_e2e_branch_contract.py index ef7e1af9f..11789ff90 100644 --- a/tests/test_sandboxed_web_e2e_branch_contract.py +++ b/tests/test_sandboxed_web_e2e_branch_contract.py @@ -296,6 +296,13 @@ def test_tail_text_rejects_nonpositive_line_count(tmp_path: Path) -> None: sandboxed_web_e2e.tail_text(log_path, max_lines=0) +def test_tail_text_validates_line_count_before_missing_file(tmp_path: Path) -> None: + """A missing evidence file cannot bypass the configured line-budget contract.""" + + with pytest.raises(ValueError, match="max_lines"): + sandboxed_web_e2e.tail_text(tmp_path / "missing.log", max_lines=0) + + def test_timeout_precedence_survives_limited_partial_output( monkeypatch, tmp_path: Path, diff --git a/tests/test_sandboxed_web_e2e_output_limits.py b/tests/test_sandboxed_web_e2e_output_limits.py index de53d6206..a3d2a7186 100644 --- a/tests/test_sandboxed_web_e2e_output_limits.py +++ b/tests/test_sandboxed_web_e2e_output_limits.py @@ -59,7 +59,7 @@ def test_start_service_enforces_real_log_file_ceiling(tmp_path: Path) -> None: ) try: service.process.wait(timeout=10) - assert service.log_path.stat().st_size <= 4097 + assert service.log_path.stat().st_size <= 4096 assert sandboxed_web_e2e.service_output_limited(service) finally: sandboxed_web_e2e.stop_service(service) @@ -299,4 +299,3 @@ def test_kept_sandbox_service_file_never_exceeds_kernel_ceiling( assert (sandbox_path / "logs" / "backend.log").stat().st_size <= 4097 finally: shutil.rmtree(sandbox_path, ignore_errors=True) - From 4cbf3f6b97d59dc9f7265bb88176c0451a73d3ce Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 15:33:14 +0900 Subject: [PATCH 14/15] test(sandbox): isolate platform capability probes --- scripts/ci/bounded_subprocess.py | 8 +++++++- tests/test_bounded_subprocess_contract.py | 4 +--- tests/test_sandboxed_web_e2e_output_limits.py | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/scripts/ci/bounded_subprocess.py b/scripts/ci/bounded_subprocess.py index 38f24cad8..086c3e295 100644 --- a/scripts/ci/bounded_subprocess.py +++ b/scripts/ci/bounded_subprocess.py @@ -97,10 +97,16 @@ def _validate_read_limit(value: object) -> int: return value +def _process_groups_supported() -> bool: + """Return whether isolated POSIX process-group termination is available.""" + + return os.name == "posix" and hasattr(os, "killpg") + + def require_supported_platform() -> None: """Fail before execution when process-group termination is unavailable.""" - if os.name != "posix" or not hasattr(os, "killpg"): + if not _process_groups_supported(): raise OutputLimitUnsupportedError( "POSIX process-group support is required for bounded child output" ) diff --git a/tests/test_bounded_subprocess_contract.py b/tests/test_bounded_subprocess_contract.py index 32a00d371..052b886e2 100644 --- a/tests/test_bounded_subprocess_contract.py +++ b/tests/test_bounded_subprocess_contract.py @@ -26,8 +26,7 @@ def test_read_limit_and_timeout_validation_reject_all_unsafe_types() -> None: def test_supported_platform_requires_posix_killpg(monkeypatch) -> None: """POSIX naming without process-group termination still fails closed.""" - monkeypatch.setattr(bounded.os, "name", "posix") - monkeypatch.delattr(bounded.os, "killpg") + monkeypatch.setattr(bounded, "_process_groups_supported", lambda: False) with pytest.raises(bounded.OutputLimitUnsupportedError): bounded.require_supported_platform() @@ -342,4 +341,3 @@ def join(self, timeout=None) -> None: ) assert joins == ["stdout", "stderr"] - diff --git a/tests/test_sandboxed_web_e2e_output_limits.py b/tests/test_sandboxed_web_e2e_output_limits.py index a3d2a7186..fb6ed9fd9 100644 --- a/tests/test_sandboxed_web_e2e_output_limits.py +++ b/tests/test_sandboxed_web_e2e_output_limits.py @@ -296,6 +296,6 @@ def test_kept_sandbox_service_file_never_exceeds_kernel_ceiling( try: assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE - assert (sandbox_path / "logs" / "backend.log").stat().st_size <= 4097 + assert (sandbox_path / "logs" / "backend.log").stat().st_size <= 4096 finally: shutil.rmtree(sandbox_path, ignore_errors=True) From dd2132b5aedc56859ffa81568d42ab9e81e52b65 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 11 Aug 2026 15:50:12 +0900 Subject: [PATCH 15/15] test(sandbox): synchronize service readiness --- tests/test_sandboxed_verify_output_limits.py | 30 ++++---- .../test_sandboxed_web_e2e_branch_contract.py | 2 + tests/test_sandboxed_web_e2e_output_limits.py | 74 ++++++++++++++----- 3 files changed, 76 insertions(+), 30 deletions(-) diff --git a/tests/test_sandboxed_verify_output_limits.py b/tests/test_sandboxed_verify_output_limits.py index 448197cb7..7661f809b 100644 --- a/tests/test_sandboxed_verify_output_limits.py +++ b/tests/test_sandboxed_verify_output_limits.py @@ -159,22 +159,26 @@ def fail_run(*args, **kwargs): 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) - for value in ["4095", str(bounded.MAXIMUM_OUTPUT_LIMIT_BYTES + 1)]: - 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 + 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_web_e2e_branch_contract.py b/tests/test_sandboxed_web_e2e_branch_contract.py index 11789ff90..9d220b2eb 100644 --- a/tests/test_sandboxed_web_e2e_branch_contract.py +++ b/tests/test_sandboxed_web_e2e_branch_contract.py @@ -118,6 +118,8 @@ def test_service_limit_fallback_handles_missing_small_and_large_files( assert not sandboxed_web_e2e.service_output_limited(service) service.log_path.write_bytes(b"safe") assert not sandboxed_web_e2e.service_output_limited(service) + service.log_path.write_bytes(b"x" * 4096) + assert not sandboxed_web_e2e.service_output_limited(service) service.log_path.write_bytes(b"x" * 4097) assert sandboxed_web_e2e.service_output_limited(service) diff --git a/tests/test_sandboxed_web_e2e_output_limits.py b/tests/test_sandboxed_web_e2e_output_limits.py index fb6ed9fd9..3cbf898d6 100644 --- a/tests/test_sandboxed_web_e2e_output_limits.py +++ b/tests/test_sandboxed_web_e2e_output_limits.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import socket import shlex import shutil import sys @@ -29,6 +30,28 @@ def _repository(tmp_path: Path) -> Path: return repository +def _free_port() -> int: + """Return one currently available localhost port for a short-lived service.""" + + with socket.socket() as listener: + listener.bind(("127.0.0.1", 0)) + return int(listener.getsockname()[1]) + + +def _http_service_command(port: int, label: str) -> str: + """Return a bounded-test HTTP service that emits its readiness label.""" + + return _command( + "import http.server\n" + "import socketserver\n" + "socketserver.TCPServer.allow_reuse_address=True\n" + f"server=socketserver.TCPServer(('127.0.0.1',{port})," + "http.server.SimpleHTTPRequestHandler)\n" + f"print({label!r},flush=True)\n" + "server.serve_forever()\n" + ) + + def _result_payload(output: str) -> dict[str, object]: """Parse the final machine-readable web E2E result marker.""" @@ -44,6 +67,7 @@ def test_start_service_enforces_real_log_file_ceiling(tmp_path: Path) -> None: logs_directory = tmp_path / "logs" logs_directory.mkdir() + log_limit_bytes = 4096 service = sandboxed_web_e2e.start_service( "backend", _command( @@ -55,11 +79,11 @@ def test_start_service_enforces_real_log_file_ceiling(tmp_path: Path) -> None: tmp_path, {"PATH": ""}, logs_directory, - 4096, + log_limit_bytes, ) try: service.process.wait(timeout=10) - assert service.log_path.stat().st_size <= 4096 + assert service.log_path.stat().st_size <= log_limit_bytes assert sandboxed_web_e2e.service_output_limited(service) finally: sandboxed_web_e2e.stop_service(service) @@ -147,14 +171,20 @@ def test_normal_services_and_e2e_preserve_existing_success_contract( ) -> None: """Ordinary services, Unicode output, cleanup, and evidence remain unchanged.""" + backend_port = _free_port() + frontend_port = _free_port() exit_code = sandboxed_web_e2e.main( [ "--repo-root", str(_repository(tmp_path)), "--backend-cmd", - _command("import time; print('backend-ready', flush=True); time.sleep(30)"), + _http_service_command(backend_port, "backend-ready"), "--frontend-cmd", - _command("import time; print('frontend-ready', flush=True); time.sleep(30)"), + _http_service_command(frontend_port, "frontend-ready"), + "--backend-ready-url", + f"http://127.0.0.1:{backend_port}/README.md", + "--frontend-ready-url", + f"http://127.0.0.1:{frontend_port}/README.md", "--e2e-cmd", _command("print('통합 성공')"), "--output-limit-bytes", @@ -238,7 +268,21 @@ def fail_start(*args, **kwargs): assert payload["output_limited"] is True -def test_cli_rejects_unsafe_command_and_service_budgets(tmp_path: Path) -> None: +@pytest.mark.parametrize( + ("option", "value"), + [ + ("--output-limit-bytes", "4095"), + ( + "--service-log-limit-bytes", + str(bounded.MAXIMUM_OUTPUT_LIMIT_BYTES + 1), + ), + ], +) +def test_cli_rejects_unsafe_command_and_service_budgets( + tmp_path: Path, + option: str, + value: str, +) -> None: """Both output budgets fail parsing outside the explicit safe range.""" repository = _repository(tmp_path) @@ -252,16 +296,9 @@ def test_cli_rejects_unsafe_command_and_service_budgets(tmp_path: Path) -> None: "--e2e-cmd", _command("pass"), ] - for option, value in [ - ("--output-limit-bytes", "4095"), - ( - "--service-log-limit-bytes", - str(bounded.MAXIMUM_OUTPUT_LIMIT_BYTES + 1), - ), - ]: - with pytest.raises(SystemExit) as raised: - sandboxed_web_e2e.parse_args([*base, option, value]) - assert raised.value.code == 2 + with pytest.raises(SystemExit) as raised: + sandboxed_web_e2e.parse_args([*base, option, value]) + assert raised.value.code == 2 def test_kept_sandbox_service_file_never_exceeds_kernel_ceiling( @@ -270,6 +307,7 @@ def test_kept_sandbox_service_file_never_exceeds_kernel_ceiling( ) -> None: """Persisted debugging sandboxes retain only the bounded service artifact.""" + log_limit_bytes = 4096 exit_code = sandboxed_web_e2e.main( [ "--repo-root", @@ -286,7 +324,7 @@ def test_kept_sandbox_service_file_never_exceeds_kernel_ceiling( "--e2e-cmd", _command("pass"), "--service-log-limit-bytes", - "4096", + str(log_limit_bytes), "--keep-sandbox", ] ) @@ -296,6 +334,8 @@ def test_kept_sandbox_service_file_never_exceeds_kernel_ceiling( try: assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE - assert (sandbox_path / "logs" / "backend.log").stat().st_size <= 4096 + assert ( + sandbox_path / "logs" / "backend.log" + ).stat().st_size <= log_limit_bytes finally: shutil.rmtree(sandbox_path, ignore_errors=True)