diff --git a/CHANGELOG.md b/CHANGELOG.md index 55a2a2f211..46d1cbeaab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1245,6 +1245,11 @@ Semantic Versioning where the repository publishes a release. ### Added +- Add a reusable POSIX subprocess boundary with continuously drained bounded + stdout/stderr suffixes, finite reader joins, post-leader process-group cleanup, + and stream or file-suffix UTF-8 decoding that stays inside the declared byte + budget. Consumer sandbox integrations remain in separate stacked changes. + - Refresh the live product and technical gap baseline against the current open-PR queue after ContextualWisdomLab/.github#1252 merged, with SHA-bound snapshot rows, a same-session open/close delta, ADR Figma File ID N/A, and @@ -1268,6 +1273,21 @@ Semantic Versioning where the repository publishes a release. ### Changed +- Bound backend and frontend combined service logs plus E2E command output, + stop a service before running E2E when readiness evidence overflows, preserve + timeout and prior-failure precedence, and publish separate resource and + capture-finalization fields with bounded log tails. + +- Route sandboxed verification commands through the bounded subprocess layer, + reject copied-tree symlinks that leave the sandbox, and publish distinct + output-limit, unsupported-platform, missing/non-executable command, and + path-boundary evidence without exposing host paths or uncaught tracebacks. +- Classify missing or non-executable backend, frontend, and E2E commands with + stable exit codes and operator recovery actions while still cleaning up + services that started before the failure. +- Reject non-HTTP readiness URLs during argument parsing with the exact option + to correct, instead of starting services and exposing a runtime traceback. + - Require the PR Review Merge Scheduler to observe both GitHub's aggregate `APPROVED` decision and the latest effective non-author, non-OpenCode formal approval bound to the exact live head before direct merge or auto-merge. diff --git a/docs/doctoring/sandboxed-output-resource-bounds.md b/docs/doctoring/sandboxed-output-resource-bounds.md new file mode 100644 index 0000000000..3e2e919692 --- /dev/null +++ b/docs/doctoring/sandboxed-output-resource-bounds.md @@ -0,0 +1,187 @@ +# Sandboxed subprocess output resource bounds + +## Decision + +The central CI library now provides one reusable POSIX subprocess boundary that +continuously drains child stdout and stderr into fixed-size final-suffix buffers. +A stream that exceeds its declared byte budget terminates the isolated process +group. Exit code `123` is reserved for sandbox consumers that adopt this library; +consumer integration remains in separately reviewable stacked changes. + +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. Each of the two normal-path output-reader joins has a +finite 30-second bound. Because sibling readers are finalized sequentially so +one failure cannot skip the other, the worst-case two-reader finalization bound +is 60 seconds. + +## 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 same-group descendant can retain an inherited stdout or stderr descriptor after +the direct child exits. The runner therefore signals the isolated process group +again after reaping the direct child and before joining its readers. A real +sentinel regression proves that such a descendant is stopped before it can act. +If a descendant deliberately creates a different session, every reader +finalization still has a 30-second bound, continues to finalize sibling readers, +and re-raises the first failure instead of holding the job until its 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 on the first stream overflow; +6. kills the group on timeout and again after direct-child exit so same-group + descendants cannot retain inherited pipes; 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 + +The second stack layer adopts the library in `sandboxed_verify.py` for +short-lived verification commands. Long-running `sandboxed_web_e2e.py` service +evidence remains a separate layer. Keeping those integrations separate prevents +a shared process primitive, workspace symlink policy, and E2E result schema from +becoming one monolithic review. + +The third stack layer adopts the same bounded drainer for each backend and +frontend combined stdout/stderr pipe. A capture retains the final suffix in +memory and writes only its bounded rendered form to the private sandbox log when +the stream closes, so the evidence file cannot exceed its declared budget. + +Service overflow is checked during readiness, after E2E execution, and after +service shutdown. It takes precedence over an otherwise successful command or +readiness result, while a true E2E timeout remains `124`. A verbose but healthy +service is intentionally stopped once it exceeds the default 4 MiB combined-log +contract; projects that need more evidence must select an explicit supported +budget. `tail_text()` reads no more than 65,536 bytes from the end of the already +bounded file and keeps the truncation marker visible. + +The result separates `output_limited`, `output_limit_unsupported`, and +`service_capture_failed`. A nonzero E2E or readiness code remains authoritative +even when a late service overflow also sets `output_limited=true`; the Boolean +retains the secondary resource evidence without erasing the original failure. +If service finalization fails, the wrapper performs another best-effort group +kill, bounded reap, and capture join before publishing failure evidence. + +The verification consumer maps an executable lookup failure to exit code `127`, +publishes its normal machine-readable failed result, and tells the operator to +install the executable or correct `PATH`. Provider and host path details do not +escape through an uncaught traceback. + +A path that exists but is a directory or lacks execute permission is distinct: +the consumer returns exit code `126` and tells the operator to select an +executable file or correct its permissions. The stable failed result remains +available without exposing the operating-system exception traceback. + +The web E2E consumer applies the same `126`/`127` machine-readable failure +boundary to backend, frontend, and E2E command launch. Services that started +before a later launch failure still pass through the ordinary bounded cleanup +path. + +Backend and frontend readiness URLs are rejected during argument parsing unless +they are empty or use `http://` or `https://`. The parser names the option the +operator must correct before any workspace copy or service launch occurs. + +## Security and availability properties + +- Parent retained memory is bounded independently for stdout and stderr. +- 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 same-group descendant that outlives the direct child is terminated before + reader finalization; a different-session descendant cannot create an unbounded + reader join. +- Structured argv and `shell=False` remain unchanged. +- Environment scrubbing, workspace copying, readiness polling, and + machine-readable consumer evidence remain independent follow-up controls. +- Non-POSIX environments fail closed rather than using unmanaged capture. +- UTF-8 replacement decoding cannot expand published text beyond the configured + byte budget. +- Bounded regular-file suffix reads account for both the truncation marker and + replacement-decoding expansion inside the caller's declared byte budget. + +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; +- a real same-group descendant that inherits the pipes, outlives the direct + child, and is prevented from writing a delayed sentinel; +- 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; +- UTF-8 replacement expansion within the declared byte budget; +- 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; +- 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 inherited descriptor outside the managed +process group into a deterministic failure, but it does not discover or +terminate arbitrary processes in another session. Isolation beyond that +boundary remains the responsibility of the surrounding container or runner. +After the direct child is reaped, final same-group cleanup uses its numeric +process-group identifier immediately. POSIX does not provide a retained +process-group handle, so an extremely narrow identifier-reuse race remains a +platform limitation; a stronger isolation boundary belongs in the surrounding +container or runner. + +## Rollback + +Rollback must restore a different proven stdout/stderr memory bound before any +consumer relies on complete pipe capture. Reverting the process-group cleanup, +bounded suffix, or finite reader join independently would recreate an unbounded +or lingering-child path around the remaining controls. + +## 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 + +The Open Group, & IEEE. (2024). *The Open Group Base Specifications Issue 8: +Definitions* (IEEE Std 1003.1-2024). https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/V1_chap03.html diff --git a/docs/doctoring/sandboxed-verification-symlink-boundary.md b/docs/doctoring/sandboxed-verification-symlink-boundary.md new file mode 100644 index 0000000000..ef1c646bf6 --- /dev/null +++ b/docs/doctoring/sandboxed-verification-symlink-boundary.md @@ -0,0 +1,61 @@ +# Sandboxed verification symlink boundary + +## Incident + +The review verifier copied an untrusted checkout with `shutil.copytree(..., +symlinks=True)`. That preserves symbolic links rather than copying their +targets. A pull request could therefore add an absolute link, or a relative +link containing enough parent traversal, that a verification command followed +outside the temporary repository. Environment scrubbing did not close that +filesystem boundary. + +## Decision + +After applying the copy ignore policy, and before running the untrusted command, +the verifier walks the exact copied tree without following directory links and +validates every symbolic link. An absolute target is rejected because the copied +link would still point at a host path. A relative target is accepted only when +its fully resolved path remains beneath the copied repository. Safe internal +relative links remain links so project semantics are preserved. Links under +ignored paths such as `node_modules` never enter the copy and are not evaluated. + +The validation happens before the untrusted command starts. Rejection is +fail-closed with stable exit code `122`, `path_boundary_rejected=true` in the +machine-readable result, and a generic diagnostic that does not disclose the +resolved host target. It produces no verification success evidence. This is +filesystem containment, not an operating-system sandbox claim; the existing +network-mode field remains evidence metadata rather than enforcement. + +The walk is intentionally a pre-execution copy validation, not a continuous +kernel-enforced filesystem sandbox. A command may create a new symlink after +validation. The wrapper therefore does not claim to contain a hostile process +that can mutate its copied workspace during execution; that stronger boundary +belongs to the surrounding runner or container. The control closes exposure +introduced by attacker-supplied links already present in the copied checkout. +Repository-internal symlink cycles remain inside the boundary but can make a +later verification tool that follows links recurse. Projects must remove such +cycles or configure the verification tool not to follow them; containment does +not imply that every internal graph is operationally valid. + +## Test-first evidence + +`tests/test_sandboxed_verify_symlink_boundary.py` first reproduced the defect: +an escaping repository link copied successfully instead of raising. The +accepted tests require rejection of both relative traversal and absolute links, +including an absolute link back into the original checkout, while retaining a +safe repository-internal relative link. + +## Failure, recovery, and rollback + +Repositories that intentionally contain absolute or escaping links must replace +them with bounded relative links before review verification. A rollback is safe +only if an independently reviewed replacement proves that no path available to +the copied command can resolve outside the copy. Dereferencing untrusted links +during the copy is not an acceptable fallback because it can read the external +target while constructing the sandbox. + +## APA 7th reference + +Python Software Foundation. (2026). *shutil—High-level file operations* +(Python 3.14.6 documentation). Retrieved August 24, 2026, from +https://docs.python.org/3.14/library/shutil.html#shutil.copytree diff --git a/scripts/ci/bounded_subprocess.py b/scripts/ci/bounded_subprocess.py new file mode 100644 index 0000000000..462658521e --- /dev/null +++ b/scripts/ci/bounded_subprocess.py @@ -0,0 +1,468 @@ +"""Run POSIX child processes with continuously drained bounded output pipes.""" + +from __future__ import annotations + +import math +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 _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 not _process_groups_supported(): + 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[-limit:] + marker = TRUNCATION_MARKER.encode("utf-8") + if limit <= len(marker): + return marker[:limit] + suffix_budget = limit - len(marker) + suffix = buffer[-suffix_budget:] + return marker + suffix + + +def _decode_bounded_bytes(buffer: bytes, limit: int) -> str: + """Decode evidence without expanding beyond its retained UTF-8 byte budget.""" + + decoded = buffer.decode("utf-8", errors="replace") + encoded = decoded.encode("utf-8") + if len(encoded) <= limit: + return decoded + + marker = TRUNCATION_MARKER if buffer.startswith( + TRUNCATION_MARKER.encode("utf-8") + ) else "" + suffix_budget = limit - len(marker.encode("utf-8")) + suffix = encoded[-suffix_budget:] + return marker + suffix.decode("utf-8", errors="ignore") + + +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 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.""" + + 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 _decode_bounded_bytes(evidence, self._limit) + + 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) + evidence = _render_bounded_bytes(data, read_limit, truncated) + text = _decode_bounded_bytes(evidence, read_limit) + 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 finite numeric subprocess timeout. + + Rejects ``bool``, non-numeric values, non-positive numbers, ``NaN``, and + either signed infinity: none of those can ever reach the timeout/cleanup + path, so a command given one of them would otherwise run unbounded. + """ + + if ( + isinstance(timeout, bool) + or not isinstance(timeout, (int, float)) + or timeout <= 0 + or not math.isfinite(timeout) + ): + raise ValueError("timeout must be a positive number") + return timeout + + +def kill_process_group(process: subprocess.Popen[bytes]) -> None: + """Kill an isolated group, including descendants after its leader exits.""" + + 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) + 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: + 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() + + kill_process_group(process) + + _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 94797c2038..759388d2a2 100644 --- a/scripts/ci/sandboxed_verify.py +++ b/scripts/ci/sandboxed_verify.py @@ -14,6 +14,11 @@ from collections.abc import Callable, 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", @@ -86,10 +91,21 @@ ".env.template", ) RESULT_MARKER = "SANDBOXED_VERIFY_RESULT" +PATH_BOUNDARY_EXIT_CODE = 122 +COMMAND_NOT_EXECUTABLE_EXIT_CODE = 126 +COMMAND_NOT_FOUND_EXIT_CODE = 127 ENV_NAME_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") MAXIMUM_SYMLINK_HOPS = 40 +class RepositoryPathBoundaryError(ValueError): + """Report a copied repository link that escapes its sandbox boundary.""" + + +class RepositoryRootError(ValueError): + """Report that the requested repository root cannot be copied.""" + + def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: """Parse CLI arguments for the sandboxed verification wrapper.""" parser = argparse.ArgumentParser( @@ -100,6 +116,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", @@ -137,6 +159,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}") @@ -169,97 +198,72 @@ def scrubbed_env(sandbox_root: Path, allow_env: Sequence[str] = ()) -> dict[str, return env -def _reject_escaping_symlinks(destination: Path) -> None: - """Fail closed if any symlink copied into the workspace resolves outside it. - - ``shutil.copytree(..., symlinks=True)`` preserves the exact target string - of every symlink instead of dereferencing it, so a repository can carry a - symlink whose (possibly absolute, possibly ``..``-laden) target resolves - outside the copied tree. A command later run against the copy — under OS - sandboxing or, in ``--isolation disabled`` debugging mode, directly on the - host — must never be able to follow such a link to read or write a file - outside the workspace boundary, defeating the isolation this module - exists to provide. Every symlink under ``destination`` is walked hop by - hop purely lexically (see ``_reject_escaping_symlink_chain``), so a link - whose own target was itself excluded from the copy by ``DEFAULT_IGNORE`` - or ``extra_ignores`` -- or is simply broken -- is not confused with one - that escapes; the first symlink found to actually escape, or whose chain - cannot be resolved, aborts the whole copy rather than being silently - dropped or repaired, since a repository author who plants one such link - cannot be assumed not to have planted others. - - Walking starts from ``root`` -- ``destination`` fully resolved -- rather - than ``destination`` itself, and every symlink found is then checked - with ``path.relative_to(root)``. When some *ancestor* of ``destination`` - is itself reached through a symlink (for example a temp directory whose - default OS location is a symlink, unrelated to anything the copied - repository controls), ``destination`` and ``root`` are different, only - lexically equal-looking strings for the same real location. Walking from - the unresolved ``destination`` would then yield paths still prefixed - with that unresolved string, which are never actually relative to - ``root`` -- so ``relative_to`` raises before this function's own escape - check ever runs, rejecting an entirely legitimate copy that contains no - escaping symlink at all. Walking from ``root`` instead guarantees every - yielded path already shares ``root``'s own resolved prefix, so - ``relative_to`` only ever fails for the cases this function exists to - reject. +def _validate_contained_symlink_cycle(candidate: Path, source_root: Path) -> None: + """Reject a symlink chain that escapes ``source_root`` or cannot be resolved. + + Thin entry point over ``_resolve_repository_symlink_components``, which + does the actual component-by-component walk starting fresh (no symlink + yet in progress, the full hop budget available). """ - root = destination.resolve(strict=True) - for path in root.rglob("*"): - if path.is_symlink(): - _resolve_symlink_components( - path.relative_to(root).parts, root, root, set(), [MAXIMUM_SYMLINK_HOPS], path - ) + _resolve_repository_symlink_components( + candidate.relative_to(source_root).parts, + source_root, + source_root, + set(), + [MAXIMUM_SYMLINK_HOPS], + candidate, + ) -def _resolve_symlink_components( +def _resolve_repository_symlink_components( parts: Sequence[str], resolved: Path, - root: Path, + source_root: Path, active: set[Path], hops_remaining: list[int], candidate: Path, ) -> Path: """Resolve ``parts`` one component at a time, raising on escape or cycle. - Uses ``os.readlink`` at every hop instead of ``Path.resolve()``, which - requires the fully-resolved path to exist (``strict=True``) or is - unreliable for detecting a cycle across Python versions (``strict=False``, - the default) -- either way conflating a symlink escape with a symlink - that merely points at a target this function never had to check for - existence. A dangling target -- for example one whose file was excluded - from the copy by ``DEFAULT_IGNORE`` -- is therefore accepted as long as - it still resolves inside ``root``: verification must still run despite - the broken link. Only an absolute target, a component that steps outside - ``root``, or a chain that revisits a symlink it is *currently in the - middle of following* (an unresolvable cycle) raises. - - Each path component is checked individually, and a component found to be - a symlink is resolved via a recursive call, rather than resolving a whole - target string in one ``os.path.normpath`` call -- a target can itself - contain an intermediate component that is a symlink, for example - ``some-alias/../secret`` where ``some-alias`` is itself a relative, - entirely-legitimate-looking internal symlink. Collapsing that whole - string lexically in one step would cancel ``some-alias`` against the - following ``..`` textually, silently ignoring that following - ``some-alias`` for real can land somewhere shallower or deeper than one - directory level. Recursion is what makes the cycle check precise: a - symlink is added to ``active`` only while its own target is being - resolved and removed again as soon as that resolution returns - successfully, so the *same* symlink referenced twice in one chain -- - once fully resolved before the second reference is ever reached, not a - real loop -- is accepted, while a symlink that (directly or through - others) points back to itself while still being resolved is rejected. A - hop budget, shared across the whole recursive walk, bounds the total - number of symlinks followed so a chain that never repeats still fails - closed instead of walking forever; only actually dereferencing a symlink - spends one unit of that budget, so a chain of exactly - ``MAXIMUM_SYMLINK_HOPS`` real, resolvable symlinks is accepted. + ``validate_repository_symlinks`` calls ``_validate_contained_symlink_cycle`` + for every symlink it finds, which enters this function. It walks by hand + one path component at a time using ``os.readlink``, instead of asking + ``Path.resolve()`` to follow the chain or collapsing each hop's whole + target in a single ``os.path.normpath`` call: either of those would miss + a target that itself contains an intermediate component that is a + symlink -- for example ``some-alias/../secret`` where ``some-alias`` is + an entirely-legitimate-looking internal symlink on its own -- and + ``Path.resolve()``'s behavior on an actual cycle is not reliable evidence + either way, raising ``RuntimeError`` on some Python versions but silently + returning a partially-resolved path, with no error at all, on others. + Processing one component at a time and recursing into a symlink's own + target -- rather than collapsing the whole string lexically, which would + cancel ``some-alias`` against a following ``..`` textually without ever + re-examining whether ``some-alias`` needs its own resolution first -- + closes that gap without ever calling ``Path.resolve()``. + + Recursion is also what makes the cycle check precise: a symlink is added + to ``active`` only while its own target is being resolved and removed + again as soon as that resolution returns successfully, so the *same* + symlink referenced twice in one chain -- once fully resolved before the + second reference is ever reached, not a real loop -- is accepted, while a + symlink that (directly or through others) points back to itself while + still being resolved raises ``RepositoryPathBoundaryError``: there is no + well-defined resolved position to hand back to a caller that needs to + keep resolving components after such a symlink, so a genuine cycle can + never be treated as merely "contained" once resolution is allowed to + continue past it. A hop budget, shared across the whole recursive walk, + bounds the total number of symlinks followed so a chain that never + repeats still fails closed instead of walking forever; only actually + dereferencing a symlink spends one unit of that budget, so a chain of + exactly ``MAXIMUM_SYMLINK_HOPS`` real, resolvable symlinks is accepted. """ for component in parts: if component == "..": - if resolved == root: - raise ValueError(f"workspace symlink escapes the sandbox root: {candidate}") + if resolved == source_root: + raise RepositoryPathBoundaryError( + f"symlink escapes repository verification sandbox: {candidate}" + ) resolved = resolved.parent continue step = resolved / component @@ -267,23 +271,55 @@ def _resolve_symlink_components( resolved = step continue if step in active: - raise ValueError(f"workspace symlink could not be resolved: {candidate}") + raise RepositoryPathBoundaryError( + f"symlink chain could not be resolved: {candidate}" + ) if hops_remaining[0] <= 0: - raise ValueError(f"workspace symlink could not be resolved: {candidate}") + raise RepositoryPathBoundaryError( + f"symlink chain exceeds the supported hop limit: {candidate}" + ) active.add(step) hops_remaining[0] -= 1 target = Path(os.readlink(step)) if target.is_absolute(): - raise ValueError( - f"workspace symlink escapes the sandbox root: {step} -> {target}" + raise RepositoryPathBoundaryError( + f"symlink escapes repository verification sandbox via absolute target: " + f"{step} -> {target}" ) - resolved = _resolve_symlink_components( - target.parts, resolved, root, active, hops_remaining, candidate + resolved = _resolve_repository_symlink_components( + target.parts, resolved, source_root, active, hops_remaining, candidate ) active.discard(step) return resolved +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. Every + symlink is validated with ``_validate_contained_symlink_cycle``'s + component-by-component walk rather than ``Path.resolve()``: resolving a + multi-hop chain silently follows any absolute hop partway through instead + of just the first one, and resolving a genuine cycle is not reliable + evidence either way -- it raises ``RuntimeError`` on some Python versions + but silently returns a partially-resolved path, with no error at all, on + others. Neither behavior is something this boundary check can depend on; + the manual walk is deterministic across Python versions, checks every + hop rather than just the first, and rejects rather than tolerates a + symlink chain that cannot be resolved to a real, bounded target. + """ + 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 candidate.is_symlink(): + _validate_contained_symlink_cycle(candidate, source_root) + + def _ignore_with_env_template_allowlist( default_patterns: Sequence[str], extra_patterns: Sequence[str] ) -> Callable[[str, list[str]], set[str]]: @@ -327,26 +363,28 @@ def copy_workspace(repo_root: Path, sandbox_root: Path, extra_ignores: Sequence[ """Copy the repository into the sandbox and return the copied root.""" source = repo_root.resolve() if not source.is_dir(): - raise ValueError(f"repo root is not a directory: {source}") + raise RepositoryRootError(f"repo root is not a directory: {source}") destination = sandbox_root / "repo" ignore = _ignore_with_env_template_allowlist(DEFAULT_IGNORE, tuple(extra_ignores)) shutil.copytree(source, destination, ignore=ignore, symlinks=True) - _reject_escaping_symlinks(destination) + validate_repository_symlinks(destination) return destination -def run_command(command: Sequence[str], cwd: Path, env: dict[str, str], timeout: int) -> subprocess.CompletedProcess[str]: - """Run the verification command and capture output for review evidence.""" - return subprocess.run( - list(command), +def run_command( + command: Sequence[str], + cwd: Path, + env: dict[str, str], + timeout: int, + output_limit_bytes: int = bounded_subprocess.DEFAULT_COMMAND_OUTPUT_LIMIT_BYTES, +) -> bounded_subprocess.BoundedCompletedProcess: + """Run one verification command with continuously drained bounded output.""" + return bounded_subprocess.run_bounded_command( + command, cwd=cwd, env=env, - text=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, timeout=timeout, - check=False, - shell=False, + evidence_limit_bytes=output_limit_bytes, ) @@ -370,6 +408,10 @@ def emit_result( allowed_env: Sequence[str], network: str, evidence_note: str, + output_limit_bytes: int = bounded_subprocess.DEFAULT_COMMAND_OUTPUT_LIMIT_BYTES, + output_limited: bool = False, + output_limit_unsupported: bool = False, + path_boundary_rejected: bool = False, ) -> None: """Print a machine-readable execution evidence summary.""" payload = { @@ -380,9 +422,14 @@ def emit_result( "evidence_note": evidence_note, "exit_code": exit_code, "network": network, + "output_limit_bytes": output_limit_bytes, + "output_limited": output_limited, + "output_limit_unsupported": output_limit_unsupported, + "path_boundary_rejected": path_boundary_rejected, "sandbox": str(sandbox_root) if kept else "(removed)", "sandboxed": True, } + print() print(f"{RESULT_MARKER} {json.dumps(payload, sort_keys=True)}") @@ -392,13 +439,29 @@ def main(argv: Sequence[str] | None = None) -> int: sandbox = Path(tempfile.mkdtemp(prefix="sandboxed-verify-")) start = time.monotonic() exit_code = 1 + output_limited = False + output_limit_unsupported = False + path_boundary_rejected = False copied_repo = sandbox / "repo" try: try: copied_repo = copy_workspace(Path(args.repo_root), sandbox, args.ignore) - except ValueError as exc: - print(f"sandboxed-verify: workspace copy rejected: {exc}", file=sys.stderr) - exit_code = 125 + except RepositoryPathBoundaryError: + path_boundary_rejected = True + copied_repo = Path("(not-created)") + print( + "sandboxed-verify: repository path boundary rejected", + file=sys.stderr, + ) + exit_code = PATH_BOUNDARY_EXIT_CODE + return exit_code + except RepositoryRootError: + copied_repo = Path("(not-created)") + print( + "sandboxed-verify: repository root is not a directory", + file=sys.stderr, + ) + exit_code = 1 return exit_code env = scrubbed_env(sandbox, args.allow_env) print(f"sandboxed-verify: cwd={copied_repo}") @@ -408,12 +471,52 @@ def main(argv: Sequence[str] | None = None) -> int: if args.network != "default": print(f"sandboxed-verify: network={args.network}") try: - completed = run_command(args.command, copied_repo, env, args.timeout) + completed = run_command( + args.command, + copied_repo, + env, + args.timeout, + args.output_limit_bytes, + ) if completed.stdout: print(completed.stdout, end="") if completed.stderr: print(completed.stderr, end="", file=sys.stderr) - exit_code = completed.returncode + output_limited = completed.output_limited + if output_limited: + print( + "sandboxed-verify: command output exceeded " + f"{args.output_limit_bytes} bytes", + file=sys.stderr, + ) + exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE + else: + exit_code = completed.returncode + except FileNotFoundError: + print( + "sandboxed-verify: install the executable or correct command PATH", + file=sys.stderr, + ) + exit_code = COMMAND_NOT_FOUND_EXIT_CODE + except (PermissionError, IsADirectoryError): + print( + "sandboxed-verify: select an executable file or correct its permissions", + file=sys.stderr, + ) + exit_code = COMMAND_NOT_EXECUTABLE_EXIT_CODE + except bounded_subprocess.OutputLimitUnsupportedError: + output_limit_unsupported = True + print( + "sandboxed-verify: bounded child output is unavailable on this platform", + file=sys.stderr, + ) + exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE + except (OSError, RuntimeError): + print( + "sandboxed-verify: bounded output capture failed", + file=sys.stderr, + ) + exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE except subprocess.TimeoutExpired as exc: stdout = timeout_output_text(exc.stdout) stderr = timeout_output_text(exc.stderr) @@ -421,6 +524,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 @@ -436,6 +540,10 @@ def main(argv: Sequence[str] | None = None) -> int: allowed_env=args.allow_env, network=args.network, evidence_note=args.evidence_note, + output_limit_bytes=args.output_limit_bytes, + output_limited=output_limited, + output_limit_unsupported=output_limit_unsupported, + path_boundary_rejected=path_boundary_rejected, ) if not args.keep_sandbox: shutil.rmtree(sandbox, ignore_errors=True) diff --git a/scripts/ci/sandboxed_web_e2e.py b/scripts/ci/sandboxed_web_e2e.py index b0376c0822..9c8f8d5e3a 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 ipaddress import json import os @@ -21,14 +22,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 SANDBOX_MOUNT = "/workspace" @@ -40,14 +43,24 @@ def redirect_request(self, req, fp, code, msg, headers, newurl): raise urllib.error.HTTPError(req.full_url, code, msg, headers, fp) +class CommandExecutableNotFoundError(RuntimeError): + """Report that one declared service or E2E executable is unavailable.""" + + +class CommandNotExecutableError(RuntimeError): + """Report that one declared service or E2E path cannot be executed.""" + + @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: @@ -67,6 +80,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( "--isolation", @@ -106,42 +131,115 @@ 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") - 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}") - for flag, value in ( + for option, url in ( + ("--backend-ready-url", args.backend_ready_url), + ("--frontend-ready-url", args.frontend_ready_url), + ): + if url and not url.lower().startswith(("http://", "https://")): + parser.error(f"{option} must start with http:// or https://") + for option, command in ( ("--backend-cmd", args.backend_cmd), ("--frontend-cmd", args.frontend_cmd), ("--e2e-cmd", args.e2e_cmd), ): - _require_parseable_command(parser, flag, value) + try: + tokens = shlex.split(command) + except ValueError as error: + parser.error(f"{option} is invalid: {error}") + if not tokens: + parser.error(f"{option} must not be empty") + 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 _require_parseable_command(parser: argparse.ArgumentParser, flag: str, value: str) -> None: - """Reject a command that fails to shell-tokenize or tokenizes to nothing. - - ``isolated_command`` performs this exact ``shlex.split`` validation - itself, but only reaches it when isolation is enabled. With - ``--isolation disabled`` (the explicit, documented "trusted local - debugging" escape hatch), a command string bypasses ``isolated_command`` - entirely and is handed straight to ``start_service``/``run_shell``, which - call ``shlex.split`` directly with no ``except`` around either call. A - blank command, or one with an unmatched shell-quote character, then - raised an uncaught ``ValueError`` from deep inside ``main`` instead of - the clean, coded CLI failure every other bad input in this module - produces. Validating here, in ``parse_args``, runs for both isolation - modes -- disabled included -- so a malformed command is always rejected - the same way, through argparse's own clean-exit path, before ``main`` - ever tries to run it. - """ +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" try: - tokens = shlex.split(value) - except ValueError as exc: - parser.error(f"{flag} is not a valid shell command: {exc}") - else: - if not tokens: - parser.error(f"{flag} must not be blank") + process = subprocess.Popen( + shlex.split(command), + cwd=cwd, + env=env, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + bufsize=0, + start_new_session=True, + shell=False, + ) + except FileNotFoundError as error: + raise CommandExecutableNotFoundError from error + except (PermissionError, IsADirectoryError) as error: + raise CommandNotExecutableError from error + 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 + ) BIND_ROOTS = ("/usr", "/bin", "/sbin", "/lib", "/lib64", "/opt") @@ -446,24 +544,6 @@ def isolated_command( return shlex.join([*args, *argv]) -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.""" - 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, - stderr=subprocess.STDOUT, - start_new_session=True, - shell=False, - ) - log_file.close() - return Service(label=label, command=command, process=process, log_path=log_path) - - def _require_loopback_ip_text(ip_text: str, hostname: str) -> None: """Reject a literal or resolved address that is not loopback.""" try: @@ -560,7 +640,7 @@ def require_unoccupied_readiness_port(url: str) -> None: 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. The opener is built with an explicitly empty ``ProxyHandler({})`` so this loopback-only poll can never be routed through an ``HTTP_PROXY`` / @@ -577,7 +657,7 @@ def wait_for_url(url: str, timeout: int, service: Service) -> bool: deadline = time.monotonic() + timeout opener = urllib.request.build_opener(NoRedirectHandler(), urllib.request.ProxyHandler({})) 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 @@ -589,42 +669,78 @@ 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( - shlex.split(command), - cwd=cwd, - env=env, - text=True, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - timeout=timeout, - check=False, - shell=False, - ) +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.""" + try: + return bounded_subprocess.run_bounded_command( + shlex.split(command), + cwd=cwd, + env=env, + timeout=timeout, + evidence_limit_bytes=output_limit_bytes, + ) + except FileNotFoundError as error: + raise CommandExecutableNotFoundError from error + except (PermissionError, IsADirectoryError) as error: + raise CommandNotExecutableError from error 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. + + Graceful ``SIGTERM`` is attempted only while the direct leader is still + alive. Regardless of whether the leader was already reaped before this + call, exited on its own, or had to be force-killed, a final same-group + cleanup runs before the bounded capture is joined -- analogous to + ``bounded_subprocess.run_bounded_command``'s unconditional final + ``kill_process_group`` call. Without it, a same-group descendant that + outlives an already-exited leader would keep the inherited log pipe open + and never let the capture reach EOF. + + ``poll()`` returning ``None`` only means the leader was alive at that + instant; it can still exit in the narrow window before ``os.killpg`` + runs, which raises ``ProcessLookupError`` because the process group is + already gone. The leader is a genuine zombie at that point -- exited, + but not yet reaped by this parent -- so the exception handler still + reaps it with ``wait()`` instead of leaving it unreaped until this + wrapper process itself exits. + """ + 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) - - -def tail_text(path: Path, max_lines: int = 80) -> str: - """Return the final lines of a service log.""" + service.process.wait(timeout=10) + except subprocess.TimeoutExpired: + bounded_subprocess.kill_process_group(service.process) + service.process.wait(timeout=10) + bounded_subprocess.kill_process_group(service.process) + if service.capture is not None: + service.capture.join(timeout=10) + + +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 max_lines <= 0: + raise ValueError("max_lines must be positive") if not path.exists(): return "" - lines = path.read_text(encoding="utf-8", errors="replace").splitlines() - return "\n".join(lines[-max_lines:]) + bounded_text = bounded_subprocess.read_bounded_suffix(path, max_bytes) + lines = bounded_text.text.splitlines() + tail = "\n".join(lines[-max_lines:]) + if bounded_text.truncated and bounded_subprocess.TRUNCATION_MARKER.strip() not in tail: + return f"{bounded_subprocess.TRUNCATION_MARKER.strip()}\n{tail}" + return tail def emit_result( @@ -636,6 +752,10 @@ def emit_result( frontend_ready: bool, exit_code: int, elapsed_seconds: float, + output_limited: bool, + output_limit_unsupported: bool, + service_capture_failed: bool, + path_boundary_rejected: bool = False, ) -> None: """Print a machine-readable web E2E execution evidence summary.""" payload = { @@ -649,15 +769,27 @@ def emit_result( "exit_code": exit_code, "frontend_cmd": args.frontend_cmd, "frontend_ready": frontend_ready, - "network": args.network, "isolation": args.isolation, "isolation_backend": getattr(args, "isolation_backend", "unknown"), + "network": args.network, + "output_limit_bytes": args.output_limit_bytes, + "output_limited": output_limited, + "output_limit_unsupported": output_limit_unsupported, + "path_boundary_rejected": path_boundary_rejected, "sandbox": str(sandbox_root) if args.keep_sandbox else "(removed)", "sandboxed": True, + "service_capture_failed": service_capture_failed, + "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) @@ -669,112 +801,223 @@ def main(argv: Sequence[str] | None = None) -> int: backend_ready = False frontend_ready = False exit_code = 1 + output_limited = False + output_limit_unsupported = False + service_capture_failed = False + service_limit_reported = False + path_boundary_rejected = False start = time.monotonic() try: try: copied_repo = sandboxed_verify.copy_workspace(Path(args.repo_root), sandbox, args.ignore) - except ValueError as exc: - print(f"sandboxed-web-e2e: workspace copy rejected: {exc}", file=sys.stderr) - exit_code = 125 - return exit_code - env = sandboxed_verify.scrubbed_env(sandbox, args.allow_env) - try: - backend = isolation_backend(args.isolation) - except RuntimeError as exc: - print(f"sandboxed-web-e2e: {exc}", file=sys.stderr) - args.isolation_backend = "unavailable" - exit_code = 126 - return exit_code - args.isolation_backend = backend or "disabled" - 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}") - command_env = _sandbox_environment(env, sandbox) if backend else env - try: - backend_cmd = ( - isolated_command( - args.backend_cmd, - backend=backend, - cwd=copied_repo, - sandbox_root=sandbox, - env=env, + env = sandboxed_verify.scrubbed_env(sandbox, args.allow_env) + try: + backend = isolation_backend(args.isolation) + except RuntimeError as exc: + print(f"sandboxed-web-e2e: {exc}", file=sys.stderr) + args.isolation_backend = "unavailable" + exit_code = 126 + return exit_code + args.isolation_backend = backend or "disabled" + 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}") + command_env = _sandbox_environment(env, sandbox) if backend else env + try: + backend_cmd = ( + isolated_command( + args.backend_cmd, + backend=backend, + cwd=copied_repo, + sandbox_root=sandbox, + env=env, + ) + if backend + else args.backend_cmd + ) + frontend_cmd = ( + isolated_command( + args.frontend_cmd, + backend=backend, + cwd=copied_repo, + sandbox_root=sandbox, + env=env, + ) + if backend + else args.frontend_cmd + ) + e2e_cmd = ( + isolated_command( + args.e2e_cmd, + backend=backend, + cwd=copied_repo, + sandbox_root=sandbox, + env=env, + ) + if backend + else args.e2e_cmd + ) + except (RuntimeError, ValueError) as exc: + print(f"sandboxed-web-e2e: isolation rejected command: {exc}", file=sys.stderr) + exit_code = 126 + return exit_code + try: + if args.backend_ready_url: + require_loopback_readiness_url(args.backend_ready_url) + require_unoccupied_readiness_port(args.backend_ready_url) + if args.frontend_ready_url: + require_loopback_readiness_url(args.frontend_ready_url) + require_unoccupied_readiness_port(args.frontend_ready_url) + except ValueError as exc: + print(f"sandboxed-web-e2e: invalid readiness URL: {exc}", file=sys.stderr) + exit_code = 125 + return exit_code + services.append( + start_service( + "backend", + backend_cmd, + copied_repo, + command_env, + logs_dir, + args.service_log_limit_bytes, ) - if backend - else args.backend_cmd ) - frontend_cmd = ( - isolated_command( - args.frontend_cmd, - backend=backend, - cwd=copied_repo, - sandbox_root=sandbox, - env=env, + services.append( + start_service( + "frontend", + frontend_cmd, + copied_repo, + command_env, + logs_dir, + args.service_log_limit_bytes, ) - if backend - else args.frontend_cmd ) - e2e_cmd = ( - isolated_command( - args.e2e_cmd, - backend=backend, - cwd=copied_repo, - sandbox_root=sandbox, - env=env, + try: + 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]) + except ValueError as exc: + print(f"sandboxed-web-e2e: invalid readiness URL: {exc}", file=sys.stderr) + exit_code = 125 + return exit_code + 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, ) - if backend - else args.e2e_cmd + 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( + e2e_cmd, + copied_repo, + command_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 CommandExecutableNotFoundError: + print( + "sandboxed-web-e2e: install each executable or correct command PATH", + file=sys.stderr, ) - except (RuntimeError, ValueError) as exc: - print(f"sandboxed-web-e2e: isolation rejected command: {exc}", file=sys.stderr) - exit_code = 126 - return exit_code - try: - if args.backend_ready_url: - require_loopback_readiness_url(args.backend_ready_url) - require_unoccupied_readiness_port(args.backend_ready_url) - if args.frontend_ready_url: - require_loopback_readiness_url(args.frontend_ready_url) - require_unoccupied_readiness_port(args.frontend_ready_url) - except ValueError as exc: - print(f"sandboxed-web-e2e: invalid readiness URL: {exc}", file=sys.stderr) - exit_code = 125 - return exit_code - services.append(start_service("backend", backend_cmd, copied_repo, command_env, logs_dir)) - services.append(start_service("frontend", frontend_cmd, copied_repo, command_env, logs_dir)) - try: - 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]) - except ValueError as exc: - print(f"sandboxed-web-e2e: invalid readiness URL: {exc}", file=sys.stderr) - exit_code = 125 - return exit_code - 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(e2e_cmd, copied_repo, command_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 + exit_code = sandboxed_verify.COMMAND_NOT_FOUND_EXIT_CODE + except CommandNotExecutableError: + print( + "sandboxed-web-e2e: select executable files or correct their permissions", + file=sys.stderr, + ) + exit_code = sandboxed_verify.COMMAND_NOT_EXECUTABLE_EXIT_CODE + except bounded_subprocess.OutputLimitUnsupportedError: + output_limit_unsupported = True + print( + "sandboxed-web-e2e: bounded child output is unavailable on this platform", + file=sys.stderr, + ) + exit_code = bounded_subprocess.OUTPUT_LIMIT_EXIT_CODE + except sandboxed_verify.RepositoryPathBoundaryError: + path_boundary_rejected = True + copied_repo = Path("(not-created)") + print( + "sandboxed-web-e2e: repository path boundary rejected", + file=sys.stderr, + ) + exit_code = sandboxed_verify.PATH_BOUNDARY_EXIT_CODE + except sandboxed_verify.RepositoryRootError: + copied_repo = Path("(not-created)") + print( + "sandboxed-web-e2e: repository root is not a directory", + file=sys.stderr, + ) + exit_code = 1 + except (OSError, RuntimeError): + print( + "sandboxed-web-e2e: bounded output capture failed", + 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): + 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) + if service.capture is not None: + with contextlib.suppress(OSError, RuntimeError, subprocess.SubprocessError): + service.capture.join(timeout=10) + service_capture_failed = True + if exit_code == 0: + 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 == 0: + 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 ---") @@ -787,9 +1030,14 @@ 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, + output_limit_unsupported=output_limit_unsupported, + service_capture_failed=service_capture_failed, + path_boundary_rejected=path_boundary_rejected, ) if not args.keep_sandbox: shutil.rmtree(sandbox, ignore_errors=True) + return exit_code if __name__ == "__main__": diff --git a/tests/test_bounded_subprocess.py b/tests/test_bounded_subprocess.py new file mode 100644 index 0000000000..b0e35b11b9 --- /dev/null +++ b/tests/test_bounded_subprocess.py @@ -0,0 +1,338 @@ +"""Real-process contracts for bounded sandbox subprocess output.""" + +from __future__ import annotations + +import io +import os +import sys +import time +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 == bounded.TRUNCATION_MARKER[:2] + + invalid_path = tmp_path / "invalid.log" + invalid_path.write_bytes(b"\xff" * 4097) + invalid = bounded.read_bounded_suffix(invalid_path, 4096) + assert invalid.truncated is True + assert invalid.text.startswith(bounded.TRUNCATION_MARKER) + assert len(invalid.text.encode("utf-8")) <= 4096 + + exact_path = tmp_path / "exact-invalid.log" + exact_path.write_bytes(b"\xff" * 4096) + exact = bounded.read_bounded_suffix(exact_path, 4096) + assert exact.truncated is False + assert len(exact.text.encode("utf-8")) <= 4096 + + +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 + + +def test_run_bounded_command_reaps_descendant_after_direct_child_exits( + tmp_path: Path, +) -> None: + """A same-group descendant cannot retain inherited pipes past parent exit.""" + + sentinel = tmp_path / "escaped-descendant-ran" + descendant = ( + "import pathlib,time; " + "time.sleep(0.75); " + f"pathlib.Path({str(sentinel)!r}).write_text('escaped', encoding='utf-8')" + ) + started = time.monotonic() + result = bounded.run_bounded_command( + [ + sys.executable, + "-c", + ( + "import subprocess,sys; " + f"subprocess.Popen([sys.executable, '-c', {descendant!r}]); " + "print('direct child exited')" + ), + ], + cwd=tmp_path, + env=_environment(), + timeout=10, + evidence_limit_bytes=4096, + ) + + assert time.monotonic() - started < 5 + assert result.returncode == 0 + assert result.stdout == "direct child exited\n" + time.sleep(1) + assert not sentinel.exists() + + +def test_capture_text_remains_inside_utf8_byte_budget_after_replacement() -> None: + """Invalid leading suffix bytes cannot expand retained decoded evidence.""" + + capture = bounded.start_bounded_capture( + io.BytesIO(b"x" * 4095 + b"\xff"), + evidence_limit_bytes=4096, + on_limit=lambda: None, + ) + capture.join(timeout=5) + + assert "�" in capture.text + assert len(capture.text.encode("utf-8")) <= 4096 + + +@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, + ) + 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) + + +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 0000000000..509b60a2c4 --- /dev/null +++ b/tests/test_bounded_subprocess_capture_startup.py @@ -0,0 +1,159 @@ +"""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 0000000000..f991cc2b77 --- /dev/null +++ b/tests/test_bounded_subprocess_contract.py @@ -0,0 +1,351 @@ +"""Branch-complete contracts for bounded subprocess helpers and failures.""" + +from __future__ import annotations + +import io +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, + float("nan"), + float("inf"), + float("-inf"), + ]: + 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, "_process_groups_supported", lambda: False) + 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_cleans_descendants_and_handles_disappearing_groups( + monkeypatch, +) -> None: + """Cleanup signals a group after leader exit and tolerates a vanished group.""" + + 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 == [(10, bounded.signal.SIGKILL)] + + 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_share_one_limit_kill_before_final_cleanup( + monkeypatch, + tmp_path: Path, +) -> None: + """Limit callbacks share one kill before the mandatory descendant cleanup.""" + + 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, 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), + ) + monkeypatch.setattr(bounded, "kill_process_group", lambda _process: None) + + 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 7d1ec0a431..0a45c702d4 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 0000000000..4dc88be454 --- /dev/null +++ b/tests/test_sandboxed_entrypoint_and_cleanup_coverage.py @@ -0,0 +1,93 @@ +import subprocess + +from scripts.ci import bounded_subprocess, sandboxed_web_e2e + + +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: + pid = 12345 + + 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, + ) + monkeypatch.setattr( + bounded_subprocess, + "kill_process_group", + lambda _process: None, + ) + + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(repo), + "--isolation", + "disabled", + "--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": false' in captured.out + assert '"output_limit_unsupported": false' in captured.out + assert '"service_capture_failed": 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 0000000000..b8c323b872 --- /dev/null +++ b/tests/test_sandboxed_service_capture_startup.py @@ -0,0 +1,77 @@ +"""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.py b/tests/test_sandboxed_verify.py index d89b9bb956..bbfd86d159 100644 --- a/tests/test_sandboxed_verify.py +++ b/tests/test_sandboxed_verify.py @@ -9,6 +9,17 @@ from scripts.ci import sandboxed_verify +def test_direct_file_import_bootstraps_the_repository_path() -> None: + """Direct-file loading covers the installed workflow entrypoint boundary.""" + + namespace = runpy.run_path( + str(Path(sandboxed_verify.__file__)), + run_name="sandboxed_verify_import_probe", + ) + + assert namespace["RESULT_MARKER"] == sandboxed_verify.RESULT_MARKER + + def test_scrubbed_env_uses_sandbox_paths_and_drops_secrets(monkeypatch, tmp_path): """Sandbox env keeps basic runtime variables but drops credentials.""" monkeypatch.setenv("PATH", "/usr/bin") @@ -168,6 +179,29 @@ def test_copy_workspace_rejects_missing_repo_root(tmp_path): sandboxed_verify.copy_workspace(tmp_path / "missing", tmp_path / "sandbox", []) +def test_main_reports_invalid_repo_root_without_boundary_evidence(tmp_path, capsys): + """An invalid root is a generic input failure, not a symlink rejection.""" + missing = tmp_path / "host-secret-root" + + exit_code = sandboxed_verify.main( + ["--repo-root", str(missing), "--", "verify"] + ) + captured = capsys.readouterr() + result_line = [ + line + for line in captured.out.splitlines() + if line.startswith(sandboxed_verify.RESULT_MARKER) + ][-1] + payload = json.loads(result_line.removeprefix(sandboxed_verify.RESULT_MARKER).strip()) + + assert exit_code == 1 + assert payload["exit_code"] == 1 + assert payload["path_boundary_rejected"] is False + assert "repository root is not a directory" in captured.err + assert str(missing) not in captured.err + assert "Traceback" not in captured.err + + def test_copy_workspace_rejects_absolute_symlink_escaping_sandbox_root(tmp_path): """A workspace symlink pointing at a host path outside the copy fails the whole copy closed. @@ -186,7 +220,7 @@ def test_copy_workspace_rejects_absolute_symlink_escaping_sandbox_root(tmp_path) repo.mkdir() (repo / "escape-link").symlink_to(outside) - with pytest.raises(ValueError, match="workspace symlink escapes the sandbox root"): + with pytest.raises(ValueError, match="symlink escapes repository verification sandbox via absolute target"): sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", []) @@ -200,7 +234,7 @@ def test_copy_workspace_rejects_relative_symlink_escaping_via_parent_traversal(t # Once copied to sandbox/repo/escape-link, two ".." segments reach tmp_path. (repo / "escape-link").symlink_to(Path("../../outside-secret.txt")) - with pytest.raises(ValueError, match="workspace symlink escapes the sandbox root"): + with pytest.raises(ValueError, match="symlink escapes repository verification sandbox"): sandboxed_verify.copy_workspace(repo, sandbox, []) @@ -219,7 +253,7 @@ def test_copy_workspace_rejects_directory_symlink_escaping_sandbox_root(tmp_path repo.mkdir() (repo / "escape-dir").symlink_to(outside_dir, target_is_directory=True) - with pytest.raises(ValueError, match="workspace symlink escapes the sandbox root"): + with pytest.raises(ValueError, match="symlink escapes repository verification sandbox via absolute target"): sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", []) @@ -241,7 +275,7 @@ def test_copy_workspace_rejects_escape_via_intermediate_directory_alias(tmp_path (repo / "self-alias").symlink_to(".", target_is_directory=True) (repo / "link").symlink_to("self-alias/../outside-secret.txt") - with pytest.raises(ValueError, match="workspace symlink escapes the sandbox root"): + with pytest.raises(ValueError, match="symlink escapes repository verification sandbox"): sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", []) @@ -259,7 +293,7 @@ def test_copy_workspace_rejects_unresolvable_symlink_cycle(tmp_path): (repo / "a").symlink_to("b") (repo / "b").symlink_to("a") - with pytest.raises(ValueError, match="workspace symlink could not be resolved"): + with pytest.raises(ValueError, match="symlink chain could not be resolved"): sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", []) @@ -383,7 +417,7 @@ def test_copy_workspace_still_rejects_escape_when_sandbox_root_is_reached_via_sy repo.mkdir() (repo / "evil.txt").symlink_to("/etc/passwd") - with pytest.raises(ValueError, match="workspace symlink escapes the sandbox root"): + with pytest.raises(ValueError, match="symlink escapes repository verification sandbox via absolute target"): sandboxed_verify.copy_workspace(repo, linked_root, []) @@ -401,7 +435,7 @@ def test_copy_workspace_rejects_symlink_chain_past_the_hop_limit(tmp_path): (repo / f"hop-{index}").symlink_to(f"hop-{index + 1}") (repo / f"hop-{chain_length}").write_text("payload", encoding="utf-8") - with pytest.raises(ValueError, match="workspace symlink could not be resolved"): + with pytest.raises(ValueError, match="symlink chain exceeds the supported hop limit"): sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", []) @@ -554,6 +588,9 @@ def test_main_reports_a_clean_failure_when_the_workspace_copy_is_rejected(tmp_pa on stderr and Python's default uncaught-exception exit status, instead of the clean ``sandboxed-verify: ...`` message and coded exit this module uses for every other config-time rejection (e.g. the timeout path's 124). + A path-boundary rejection is now its own typed ``RepositoryPathBoundaryError``, + classified with the dedicated ``PATH_BOUNDARY_EXIT_CODE`` rather than the + generic workspace-copy-rejected code. """ outside = tmp_path / "outside-secret.txt" outside.write_text("host-only-content", encoding="utf-8") @@ -566,13 +603,13 @@ def test_main_reports_a_clean_failure_when_the_workspace_copy_is_rejected(tmp_pa ) captured = capsys.readouterr() - assert exit_code == 125 + assert exit_code == sandboxed_verify.PATH_BOUNDARY_EXIT_CODE assert "Traceback" not in captured.err - assert "workspace copy rejected" in captured.err - assert "workspace symlink escapes the sandbox root" in captured.err + assert "repository path boundary rejected" in captured.err result_line = [line for line in captured.out.splitlines() if line.startswith(sandboxed_verify.RESULT_MARKER)][-1] payload = json.loads(result_line.removeprefix(sandboxed_verify.RESULT_MARKER).strip()) - assert payload["exit_code"] == 125 + assert payload["exit_code"] == sandboxed_verify.PATH_BOUNDARY_EXIT_CODE + assert payload["path_boundary_rejected"] is True def test_parse_args_rejects_invalid_inputs(): diff --git a/tests/test_sandboxed_verify_output_limits.py b/tests/test_sandboxed_verify_output_limits.py new file mode 100644 index 0000000000..5fe48a78e8 --- /dev/null +++ b/tests/test_sandboxed_verify_output_limits.py @@ -0,0 +1,269 @@ +"""Real-command contracts for sandboxed verification output ceilings.""" + +from __future__ import annotations + +import json +import os +import sys +from pathlib import Path + +import pytest + +from scripts.ci import bounded_subprocess as bounded +from scripts.ci import sandboxed_verify + + +def _result_payload(output: str) -> dict[str, object]: + """Parse the final sandbox result marker from captured standard output.""" + + marker = f"{sandboxed_verify.RESULT_MARKER} " + result_line = next( + line for line in reversed(output.splitlines()) if line.startswith(marker) + ) + return json.loads(result_line.removeprefix(marker)) + + +def _repository(tmp_path: Path) -> Path: + """Create one minimal repository directory accepted by the copy boundary.""" + + repository = tmp_path / "repository" + repository.mkdir() + (repository / "README.md").write_text("sandbox fixture\n", encoding="utf-8") + return repository + + +def test_normal_command_preserves_output_and_reports_declared_limit( + tmp_path: Path, + capsys, +) -> None: + """Ordinary Unicode output remains visible with deterministic limit evidence.""" + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--output-limit-bytes", + "4096", + "--", + sys.executable, + "-c", + "import sys; print('정상'); print('경고', file=sys.stderr)", + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == 0 + assert "정상" in captured.out + assert "경고" in captured.err + assert payload["output_limit_bytes"] == 4096 + assert payload["output_limited"] is False + + +@pytest.mark.parametrize("descriptor", [1, 2]) +def test_excessive_stdout_or_stderr_returns_resource_limit_code( + tmp_path: Path, + capsys, + descriptor: int, +) -> None: + """A real output flood is bounded and classified as exit 123.""" + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--output-limit-bytes", + "4096", + "--", + sys.executable, + "-c", + ( + "import os\n" + f"descriptor={descriptor}\n" + "chunk=b'x'*1024\n" + "while True:\n" + " os.write(descriptor, chunk)\n" + ), + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + combined = captured.out + captured.err + + assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE + assert bounded.TRUNCATION_MARKER.strip() in combined + assert "output exceeded 4096 bytes" in captured.err + assert payload["output_limited"] is True + assert len(combined.encode("utf-8")) < 20_000 + + +def test_timeout_retains_precedence_and_bounded_partial_output( + tmp_path: Path, + capsys, +) -> None: + """A timeout remains exit 124 while its partial output stays byte-bounded.""" + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--timeout", + "1", + "--output-limit-bytes", + "4096", + "--", + sys.executable, + "-c", + "import os,time; os.write(1,b'before\\n'); time.sleep(30)", + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == 124 + assert "before" in captured.out + assert "timed out after 1s" in captured.err + assert payload["output_limited"] is False + + +@pytest.mark.parametrize( + "capture_error", + [RuntimeError("host descriptor detail"), OSError("reader failed")], +) +def test_stuck_capture_returns_bounded_failure_without_traceback( + monkeypatch, + tmp_path: Path, + capsys, + capture_error: BaseException, +) -> None: + """A stuck reader becomes stable resource evidence instead of a traceback.""" + repository = _repository(tmp_path) + monkeypatch.setattr( + sandboxed_verify, + "run_command", + lambda *args, **kwargs: (_ for _ in ()).throw(capture_error), + ) + + exit_code = sandboxed_verify.main( + ["--repo-root", str(repository), "--", "verify"] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE + assert payload["output_limited"] is False + assert "bounded output capture failed" in captured.err + assert "host descriptor detail" not in captured.err + assert "Traceback" not in captured.err + + +def test_missing_executable_returns_stable_failed_evidence( + tmp_path: Path, + capsys, +) -> None: + """A missing command gives an actionable result instead of a traceback.""" + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--", + "missing-verification-executable", + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == sandboxed_verify.COMMAND_NOT_FOUND_EXIT_CODE + assert payload["exit_code"] == sandboxed_verify.COMMAND_NOT_FOUND_EXIT_CODE + assert "install the executable or correct command PATH" in captured.err + assert "Traceback" not in captured.err + + +@pytest.mark.parametrize("candidate_kind", ["file", "directory"]) +def test_non_executable_command_returns_stable_failed_evidence( + tmp_path: Path, + capsys, + candidate_kind: str, +) -> None: + """A present but unusable command tells the operator how to recover.""" + + candidate = tmp_path / "verification-candidate" + if candidate_kind == "file": + candidate.write_text("not executable\n", encoding="utf-8") + else: + candidate.mkdir() + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--", + str(candidate), + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == sandboxed_verify.COMMAND_NOT_EXECUTABLE_EXIT_CODE + assert payload["exit_code"] == sandboxed_verify.COMMAND_NOT_EXECUTABLE_EXIT_CODE + assert "select an executable file or correct its permissions" in captured.err + assert "Traceback" not in captured.err + + +def test_unsupported_resource_limit_fails_closed( + monkeypatch, + tmp_path: Path, + capsys, +) -> None: + """The wrapper never falls back to unbounded pipes on unsupported platforms.""" + + def fail_run(*args, **kwargs): + del args, kwargs + raise bounded.OutputLimitUnsupportedError("unsupported") + + monkeypatch.setattr(sandboxed_verify, "run_command", fail_run) + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--output-limit-bytes", + "4096", + "--", + sys.executable, + "-c", + "print('never runs')", + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE + assert "bounded child output is unavailable" in captured.err + assert payload["output_limited"] is False + assert payload["output_limit_unsupported"] is True + + +@pytest.mark.parametrize( + "value", + ["4095", str(bounded.MAXIMUM_OUTPUT_LIMIT_BYTES + 1)], +) +def test_cli_rejects_output_budgets_outside_supported_range( + tmp_path: Path, + value: str, +) -> None: + """Unsafe output budgets fail argument parsing before workspace execution.""" + + repository = _repository(tmp_path) + with pytest.raises(SystemExit) as raised: + sandboxed_verify.parse_args( + [ + "--repo-root", + str(repository), + "--output-limit-bytes", + value, + "--", + os.devnull, + ] + ) + assert raised.value.code == 2 diff --git a/tests/test_sandboxed_verify_symlink_boundary.py b/tests/test_sandboxed_verify_symlink_boundary.py new file mode 100644 index 0000000000..b35016f9d8 --- /dev/null +++ b/tests/test_sandboxed_verify_symlink_boundary.py @@ -0,0 +1,292 @@ +"""Security contracts for sandboxed verification symlink handling.""" + +import json + +import subprocess +import tempfile +from pathlib import Path + +import pytest + +from scripts.ci import sandboxed_verify + + +def test_copy_workspace_rejects_symlink_that_escapes_repository(tmp_path: Path) -> None: + """An untrusted repository symlink must not expose a host-side path.""" + repo = tmp_path / "repo" + repo.mkdir() + outside = tmp_path / "runner-secret.txt" + outside.write_text("host-only", encoding="utf-8") + (repo / "escape").symlink_to("../runner-secret.txt") + + with pytest.raises(ValueError, match="symlink escapes repository"): + sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", ()) + + +def test_copy_workspace_rejects_absolute_symlink_into_original_checkout( + tmp_path: Path, +) -> None: + """An absolute link must not reconnect the copy to its source checkout.""" + repo = tmp_path / "repo" + repo.mkdir() + target = repo / "target.txt" + target.write_text("mutable source", encoding="utf-8") + (repo / "absolute-alias.txt").symlink_to(target) + + with pytest.raises(ValueError, match="absolute target"): + sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", ()) + + +def test_copy_workspace_preserves_repository_internal_symlink(tmp_path: Path) -> None: + """A relative symlink whose resolved target stays in the repository is safe.""" + repo = tmp_path / "repo" + repo.mkdir() + (repo / "target.txt").write_text("review me", encoding="utf-8") + (repo / "alias.txt").symlink_to("target.txt") + + copied = sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", ()) + + assert (copied / "alias.txt").is_symlink() + assert (copied / "alias.txt").read_text(encoding="utf-8") == "review me" + + +def test_copy_workspace_accepts_the_same_symlink_referenced_twice_non_recursively( + tmp_path: Path, +) -> None: + """A symlink resolved twice in one chain, not as part of a loop, is accepted. + + ``link -> shared/../shared/file.txt`` references ``shared`` twice, but + the first reference is fully resolved (and its bookkeeping cleared) + before the second one is ever reached -- this is not a cycle, just an + ordinary path that happens to name the same symlink in two places, and + the OS itself resolves it without issue. A cycle check that treats + "already resolved once, earlier" the same as "currently being resolved" + would reject this valid path. + """ + repo = tmp_path / "repo" + repo.mkdir() + (repo / "real_dir").mkdir() + (repo / "real_dir" / "file.txt").write_text("payload", encoding="utf-8") + (repo / "shared").symlink_to("real_dir", target_is_directory=True) + (repo / "link").symlink_to("shared/../shared/file.txt") + + copied = sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", ()) + + assert (copied / "link").is_symlink() + assert (copied / "link").read_text(encoding="utf-8") == "payload" + + +def test_copy_workspace_rejects_repository_internal_symlink_cycle(tmp_path: Path) -> None: + """A two-link symlink cycle fails closed instead of hanging or being tolerated. + + ``Path.resolve(strict=False)`` raises an uncaught ``RuntimeError`` for a + symlink loop on some Python versions but silently returns a + partially-resolved path on others -- neither is reliable evidence this + check can depend on. A genuine cycle has no well-defined resolved + position once resolution is allowed to continue past it into further + path components, so it is rejected the same way an escape is, rather + than being tolerated as merely "contained". + """ + repo = tmp_path / "repo" + repo.mkdir() + (repo / "cycle-a").symlink_to("cycle-b") + (repo / "cycle-b").symlink_to("cycle-a") + + with pytest.raises(ValueError, match="symlink chain could not be resolved"): + sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", ()) + + +def test_main_rejects_repository_internal_symlink_cycle_without_traceback( + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """The CLI reports a clean path-boundary rejection for a self-contained cycle. + + Before the fix, ``main`` would have surfaced the ``RuntimeError`` raised + by ``Path.resolve`` deep inside ``copy_workspace`` as an uncaught + traceback, never reaching this repository's normal, coded rejection + payload at all. + """ + repo = tmp_path / "repo" + repo.mkdir() + (repo / "cycle-a").symlink_to("cycle-b") + (repo / "cycle-b").symlink_to("cycle-a") + + exit_code = sandboxed_verify.main(["--repo-root", str(repo), "--", "verify"]) + captured = capsys.readouterr() + lines = [ + line + for line in captured.out.splitlines() + if line.startswith(sandboxed_verify.RESULT_MARKER) + ] + payload = json.loads(lines[0].removeprefix(sandboxed_verify.RESULT_MARKER)) + + assert exit_code == sandboxed_verify.PATH_BOUNDARY_EXIT_CODE + assert payload["exit_code"] == sandboxed_verify.PATH_BOUNDARY_EXIT_CODE + assert payload["path_boundary_rejected"] is True + assert payload["cwd"] == "(not-created)" + assert "Traceback" not in captured.err + + +def test_validate_contained_symlink_cycle_returns_at_non_symlink_target() -> None: + """The manual hop-walk stops as soon as a chain reaches a real file.""" + with tempfile.TemporaryDirectory() as tmp_dir: + source_root = Path(tmp_dir) + real_file = source_root / "real.txt" + real_file.write_text("data", encoding="utf-8") + + assert ( + sandboxed_verify._validate_contained_symlink_cycle(real_file, source_root) + is None + ) + + +def test_validate_contained_symlink_cycle_rejects_absolute_hop_mid_chain() -> None: + """An absolute target anywhere in the chain is rejected, not just at hop one.""" + with tempfile.TemporaryDirectory() as tmp_dir: + source_root = Path(tmp_dir) + (source_root / "cycle-a").symlink_to("cycle-b") + (source_root / "cycle-b").symlink_to(source_root / "cycle-a") + + with pytest.raises(ValueError, match="absolute target"): + sandboxed_verify._validate_contained_symlink_cycle( + source_root / "cycle-a", source_root + ) + + +def test_validate_contained_symlink_cycle_rejects_lexical_escape_mid_chain() -> None: + """A relative hop that lexically steps outside source_root is rejected.""" + with tempfile.TemporaryDirectory() as tmp_dir: + source_root = Path(tmp_dir) / "repo" + source_root.mkdir() + (source_root / "cycle-a").symlink_to("cycle-b") + (source_root / "cycle-b").symlink_to("../outside") + + with pytest.raises(ValueError, match="symlink escapes repository"): + sandboxed_verify._validate_contained_symlink_cycle( + source_root / "cycle-a", source_root + ) + + +def test_validate_contained_symlink_cycle_fails_closed_past_hop_limit( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A chain that never repeats within the hop budget is rejected, not hung.""" + monkeypatch.setattr(sandboxed_verify, "MAXIMUM_SYMLINK_HOPS", 2) + with tempfile.TemporaryDirectory() as tmp_dir: + source_root = Path(tmp_dir) + (source_root / "cycle-a").symlink_to("cycle-b") + (source_root / "cycle-b").symlink_to("cycle-c") + (source_root / "cycle-c").symlink_to("cycle-a") + + with pytest.raises(ValueError, match="exceeds the supported hop limit"): + sandboxed_verify._validate_contained_symlink_cycle( + source_root / "cycle-a", source_root + ) + + +def test_validate_contained_symlink_cycle_accepts_a_chain_of_exactly_the_hop_limit( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A chain of exactly MAXIMUM_SYMLINK_HOPS real symlinks is still accepted. + + Each iteration checks one position and only advances past it if it is + itself a further symlink, so resolving a chain of N real symlinks needs + N+1 checks: one per hop, plus one to confirm the final landing position + is a real, non-symlink target. A chain of exactly the hop limit is + something the OS itself can resolve and must not be rejected. + """ + monkeypatch.setattr(sandboxed_verify, "MAXIMUM_SYMLINK_HOPS", 3) + with tempfile.TemporaryDirectory() as tmp_dir: + source_root = Path(tmp_dir) + (source_root / "hop-0").symlink_to("hop-1") + (source_root / "hop-1").symlink_to("hop-2") + (source_root / "hop-2").symlink_to("real.txt") + (source_root / "real.txt").write_text("payload", encoding="utf-8") + + assert ( + sandboxed_verify._validate_contained_symlink_cycle( + source_root / "hop-0", source_root + ) + is None + ) + + +def test_copy_workspace_does_not_validate_ignored_symlinks(tmp_path: Path) -> None: + """A link excluded from the copy is outside the command's path boundary.""" + repo = tmp_path / "repo" + ignored = repo / "node_modules" + ignored.mkdir(parents=True) + outside = tmp_path / "package-cache" + outside.mkdir() + (ignored / "external-package").symlink_to(outside, target_is_directory=True) + + copied = sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", ()) + + assert not (copied / "node_modules").exists() + + +def test_main_classifies_repository_path_boundary_without_traceback( + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """A rejected repository link must emit stable, non-sensitive evidence.""" + repo = tmp_path / "repo" + repo.mkdir() + sensitive_target = tmp_path / "runner-secret.txt" + sensitive_target.write_text("host-only", encoding="utf-8") + (repo / "escape").symlink_to(sensitive_target) + + exit_code = sandboxed_verify.main( + ["--repo-root", str(repo), "--", "verify"] + ) + captured = capsys.readouterr() + lines = [ + line + for line in captured.out.splitlines() + if line.startswith(sandboxed_verify.RESULT_MARKER) + ] + payload = json.loads(lines[0].removeprefix(sandboxed_verify.RESULT_MARKER)) + + assert exit_code == sandboxed_verify.PATH_BOUNDARY_EXIT_CODE + assert payload["exit_code"] == sandboxed_verify.PATH_BOUNDARY_EXIT_CODE + assert payload["path_boundary_rejected"] is True + assert payload["cwd"] == "(not-created)" + assert "repository path boundary rejected" in captured.err + assert str(sensitive_target) not in captured.err + assert str(repo) not in captured.err + assert "Traceback" not in captured.err + + +def test_timeout_without_partial_streams_still_emits_failed_evidence( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + """A silent timeout must retain deterministic fail-closed evidence.""" + repo = tmp_path / "repo" + repo.mkdir() + + def timeout_runner(*_args: object, **_kwargs: object) -> None: + raise subprocess.TimeoutExpired(["verify"], 1) + + monkeypatch.setattr(sandboxed_verify, "run_command", timeout_runner) + + assert ( + sandboxed_verify.main( + ["--repo-root", str(repo), "--timeout", "1", "--", "verify"] + ) + == 124 + ) + lines = [ + line + for line in capsys.readouterr().out.splitlines() + if line.startswith(sandboxed_verify.RESULT_MARKER) + ] + assert len(lines) == 1 + payload = json.loads(lines[0].removeprefix(sandboxed_verify.RESULT_MARKER)) + assert payload["exit_code"] == 124 + assert payload["output_limit_bytes"] == 1_048_576 + assert payload["output_limited"] is False + assert payload["output_limit_unsupported"] is False + assert payload["sandboxed"] is True diff --git a/tests/test_sandboxed_web_e2e.py b/tests/test_sandboxed_web_e2e.py index 1b1cdf3722..fdfbcb98cd 100644 --- a/tests/test_sandboxed_web_e2e.py +++ b/tests/test_sandboxed_web_e2e.py @@ -1,11 +1,14 @@ import json +import io import os import re import runpy +import shlex import shutil import socket import subprocess import sys +import time from pathlib import Path import pytest @@ -110,7 +113,7 @@ def test_sandboxed_web_e2e_runs_services_and_does_not_mutate_source(tmp_path, ca def test_wait_helpers_and_service_cleanup_edges(monkeypatch, tmp_path): """Small helper branches handle empty URLs, loopback readiness, and hard cleanup.""" - exited = subprocess.Popen([sys.executable, "-c", ""], text=True) + exited = subprocess.Popen([sys.executable, "-c", ""], text=True, start_new_session=True) exited.wait(timeout=5) exited_service = sandboxed_web_e2e.Service("done", "true", exited, tmp_path / "missing.log") @@ -153,13 +156,98 @@ def fake_killpg(pid, sig): monkeypatch.setattr(sandboxed_web_e2e.os, "killpg", fake_killpg, raising=False) monkeypatch.setattr(sandboxed_web_e2e.signal, "SIGKILL", sandboxed_web_e2e.signal.SIGTERM, raising=False) sandboxed_web_e2e.stop_service(slow_service) - assert len(killed) == 2 + # SIGTERM, the force kill after TimeoutExpired, and the final unconditional + # same-group cleanup that now always runs before the capture is joined. + assert len(killed) == 3 killed.clear() slow_service = sandboxed_web_e2e.Service("slow", "sleep", SlowProcess(), tmp_path / "slow.log") monkeypatch.setattr(sandboxed_web_e2e.os, "killpg", lambda pid, sig: killed.append((pid, sig)), raising=False) sandboxed_web_e2e.stop_service(slow_service) - assert len(killed) == 2 + assert len(killed) == 3 + + +def test_stop_service_reaps_leader_that_exits_between_poll_and_killpg(monkeypatch, tmp_path): + """A leader that exits in the poll()-to-killpg() race is still reaped. + + ``poll()`` returning ``None`` only proves the leader was alive at that + instant; if it exits before ``os.killpg`` runs, the whole process group + is already gone and ``killpg`` raises ``ProcessLookupError``. The leader + is then a genuine zombie -- exited, but never ``wait()``-ed on by this + parent -- so the exception handler must still reap it instead of + silently leaving it unreaped until the wrapper process itself exits. + """ + + class RaceProcess: + pid = 24680 + + def __init__(self): + self.wait_calls = 0 + + def poll(self): + return None + + def wait(self, timeout): + self.wait_calls += 1 + return 0 + + race_service = sandboxed_web_e2e.Service("race", "sleep", RaceProcess(), tmp_path / "race.log") + monkeypatch.setattr( + sandboxed_web_e2e.os, + "killpg", + lambda pid, sig: (_ for _ in ()).throw(ProcessLookupError), + raising=False, + ) + + sandboxed_web_e2e.stop_service(race_service) + + assert race_service.process.wait_calls >= 1 + + +def test_stop_service_reaps_descendant_after_leader_already_exited(tmp_path: Path) -> None: + """A same-group descendant that inherits the log pipe cannot outlive a + leader that has already exited and been reaped by the time cleanup runs. + + Before the fix, ``stop_service`` skipped ``os.killpg`` entirely whenever + ``service.process.poll()`` was no longer ``None``, so an already-exited + leader's same-group descendant (holding the inherited log pipe open) + kept running and delayed the bounded capture join until its own timeout. + """ + sentinel = tmp_path / "escaped-descendant-ran" + descendant_source = ( + "import pathlib, time; time.sleep(2); " + f"pathlib.Path({str(sentinel)!r}).write_text('escaped', encoding='utf-8')" + ) + leader_source = ( + "import subprocess, sys; " + f"subprocess.Popen([sys.executable, '-c', {descendant_source!r}]); " + "print('leader exited')" + ) + command = shlex.join([sys.executable, "-c", leader_source]) + logs_dir = tmp_path / "logs" + logs_dir.mkdir() + service = sandboxed_web_e2e.start_service( + "leader", + command, + tmp_path, + {"PATH": os.environ.get("PATH", "")}, + logs_dir, + ) + + # Let the direct leader exit and be reaped before cleanup ever runs, so + # stop_service observes an already-finished process (poll() is not None). + service.process.wait(timeout=10) + assert service.process.poll() is not None + + started = time.monotonic() + sandboxed_web_e2e.stop_service(service) + assert time.monotonic() - started < 5 + + # The descendant must have been killed with the rest of the group before + # it could complete its sleep and write the sentinel. + time.sleep(2.5) + assert not sentinel.exists() + assert "leader exited" in service.log_path.read_text(encoding="utf-8") def test_start_service_and_run_shell_capture_bash_contract(monkeypatch, tmp_path): @@ -169,6 +257,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 @@ -177,12 +266,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) @@ -191,14 +290,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 popen_calls[0][1]["shell"] is False 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 run_calls[0][1]["shell"] is False - 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): @@ -331,7 +430,7 @@ def poll(self): def test_wait_for_url_rejects_non_loopback_and_confused_deputy_targets(tmp_path): """Readiness polling must fail closed on public, metadata, and userinfo targets.""" - exited = subprocess.Popen([sys.executable, "-c", ""], text=True) + exited = subprocess.Popen([sys.executable, "-c", ""], text=True, start_new_session=True) exited.wait(timeout=5) exited_service = sandboxed_web_e2e.Service("done", "true", exited, tmp_path / "missing.log") @@ -366,7 +465,7 @@ def test_require_loopback_readiness_url_rejects_malformed_port(): def test_localhost_resolution_must_stay_loopback(monkeypatch, tmp_path): """Literal localhost is allowed only when every resolved address is loopback.""" - exited = subprocess.Popen([sys.executable, "-c", ""], text=True) + exited = subprocess.Popen([sys.executable, "-c", ""], text=True, start_new_session=True) exited.wait(timeout=5) exited_service = sandboxed_web_e2e.Service("done", "true", exited, tmp_path / "missing.log") @@ -550,12 +649,11 @@ def test_main_reports_a_clean_failure_when_the_workspace_copy_is_rejected(monkey """A symlink-escape rejection from the shared ``copy_workspace`` helper must not surface as an uncaught traceback here either. - This script calls ``sandboxed_verify.copy_workspace`` directly with no - ``except`` around it -- the same gap ``sandboxed_verify.py``'s own - ``main()`` had (a rejected copy propagated as a raw Python traceback and - Python's default uncaught-exception status instead of this module's own - clean ``sandboxed-web-e2e: ...`` message and coded exit, e.g. the 125 - already used for an invalid readiness URL below). + This script calls ``sandboxed_verify.copy_workspace`` and relies on the + typed ``sandboxed_verify.RepositoryPathBoundaryError`` it raises for a + copied-tree symlink that escapes the sandbox, classified with the + dedicated ``sandboxed_verify.PATH_BOUNDARY_EXIT_CODE`` rather than a + generic uncaught-exception status or the readiness-URL 125. """ outside = tmp_path / "outside-secret.txt" outside.write_text("host-only-content", encoding="utf-8") @@ -581,14 +679,14 @@ def test_main_reports_a_clean_failure_when_the_workspace_copy_is_rejected(monkey ) captured = capsys.readouterr() - assert exit_code == 125 + assert exit_code == sandboxed_web_e2e.sandboxed_verify.PATH_BOUNDARY_EXIT_CODE assert not started assert "Traceback" not in captured.err - assert "workspace copy rejected" in captured.err - assert "workspace symlink escapes the sandbox root" in captured.err + assert "repository path boundary rejected" in captured.err result_line = [line for line in captured.out.splitlines() if line.startswith(sandboxed_web_e2e.RESULT_MARKER)][-1] payload = json.loads(result_line.removeprefix(sandboxed_web_e2e.RESULT_MARKER).strip()) - assert payload["exit_code"] == 125 + assert payload["exit_code"] == sandboxed_web_e2e.sandboxed_verify.PATH_BOUNDARY_EXIT_CODE + assert payload["path_boundary_rejected"] is True def test_main_reports_malformed_backend_port_before_starting_services(monkeypatch, tmp_path, capsys): @@ -706,11 +804,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) @@ -718,11 +816,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)) @@ -777,6 +878,7 @@ def test_main_runs_required_isolation_with_mapped_environment(monkeypatch, tmp_p repo.mkdir() wrapped = [] started = [] + ran = [] class DoneProcess: def poll(self): @@ -786,21 +888,23 @@ def fake_isolated(command, **kwargs): wrapped.append((command, kwargs)) return f"wrapped {command}" - 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") started.append((label, command, cwd, env)) - return sandboxed_web_e2e.Service(label, command, DoneProcess(), log_path) + return sandboxed_web_e2e.Service( + label, command, DoneProcess(), log_path, log_limit_bytes=log_limit_bytes + ) + + def fake_run_shell(command, cwd, env, timeout, output_limit_bytes): + ran.append((command, cwd, env, timeout, output_limit_bytes)) + return subprocess.CompletedProcess(command, 0) monkeypatch.setattr(sandboxed_web_e2e, "isolation_backend", lambda mode: "/usr/bin/bwrap") monkeypatch.setattr(sandboxed_web_e2e, "isolated_command", fake_isolated) monkeypatch.setattr(sandboxed_web_e2e, "start_service", fake_start) monkeypatch.setattr(sandboxed_web_e2e, "wait_for_url", lambda url, timeout, service: True) - monkeypatch.setattr( - sandboxed_web_e2e, - "run_shell", - lambda command, cwd, env, timeout: subprocess.CompletedProcess(command, 0), - ) + monkeypatch.setattr(sandboxed_web_e2e, "run_shell", fake_run_shell) monkeypatch.setattr(sandboxed_web_e2e, "stop_service", lambda service: None) exit_code = sandboxed_web_e2e.main( @@ -822,6 +926,13 @@ def fake_start(label, command, cwd, env, logs_dir): assert [item[0] for item in started] == ["backend", "frontend"] assert all(item[1].startswith("wrapped ") for item in started) assert started[0][3]["HOME"].startswith("/workspace/") + # The E2E command actually executed by run_shell must be the isolated_command + # output, not the raw --e2e-cmd string -- a splice that reintroduces the + # unwrapped command here would silently let it escape the sandbox even though + # isolated_command was still (uselessly) called to compute "wrapped e2e". + assert len(ran) == 1 + assert ran[0][0] == "wrapped e2e" + assert ran[0][2]["HOME"].startswith("/workspace/") result_line = [line for line in captured.out.splitlines() if line.startswith(sandboxed_web_e2e.RESULT_MARKER)][-1] payload = json.loads(result_line.removeprefix(sandboxed_web_e2e.RESULT_MARKER).strip()) assert payload["isolation"] == "required" @@ -905,7 +1016,7 @@ def test_main_reports_coded_failure_for_whitespace_only_command(monkeypatch, tmp assert exc_info.value.code == 2 assert not started - assert "--backend-cmd must not be blank" in captured.err + assert "--backend-cmd must not be empty" in captured.err assert "Traceback" not in captured.err assert "Traceback" not in captured.out @@ -951,7 +1062,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) @@ -1095,10 +1207,12 @@ 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): started.append(label) log_path = logs_dir / f"{label}.log" - return sandboxed_web_e2e.Service(label, command, DoneProcess(), log_path) + return sandboxed_web_e2e.Service( + label, command, DoneProcess(), log_path, log_limit_bytes=log_limit_bytes + ) monkeypatch.setattr(sandboxed_web_e2e, "start_service", fake_start) monkeypatch.setattr( @@ -1142,12 +1256,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) @@ -1178,7 +1294,8 @@ def fake_run_shell(command, cwd, env, timeout): assert "e2e-err" in captured.err assert "e2e command timed out after 3s" in captured.err - def fake_run_shell_with_newlines(command, cwd, env, timeout): + def fake_run_shell_with_newlines(command, cwd, env, timeout, output_limit_bytes): + del output_limit_bytes raise subprocess.TimeoutExpired(command, timeout, output=b"e2e-out\n", stderr=b"e2e-err\n") monkeypatch.setattr(sandboxed_web_e2e, "run_shell", fake_run_shell_with_newlines) @@ -1199,7 +1316,8 @@ def fake_run_shell_with_newlines(command, cwd, env, timeout): ] ) == 124 - def fake_run_shell_without_output(command, cwd, env, timeout): + def fake_run_shell_without_output(command, cwd, env, timeout, output_limit_bytes): + del output_limit_bytes raise subprocess.TimeoutExpired(command, timeout) monkeypatch.setattr(sandboxed_web_e2e, "run_shell", fake_run_shell_without_output) @@ -1264,7 +1382,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) @@ -1912,49 +2031,115 @@ def test_parse_args_rejects_invalid_inputs(): ) -def test_parse_args_rejects_blank_backend_frontend_e2e_commands(capsys): - """A blank command on any of the three flags is rejected during parse_args. +@pytest.mark.parametrize( + "option", + ["--backend-ready-url", "--frontend-ready-url"], +) +def test_parse_args_rejects_non_http_readiness_urls(option, capsys): + """Invalid readiness schemes fail in argument parsing without a traceback.""" - This validation is independent of ``--isolation`` -- unlike - ``isolated_command``'s own blank-command check, which only ever runs - when isolation is enabled -- so a blank command is rejected the same way - whether or not isolation is later requested as ``disabled``. - """ - base = ["--backend-cmd", "backend", "--frontend-cmd", "frontend", "--e2e-cmd", "e2e"] - for flag in ("--backend-cmd", "--frontend-cmd", "--e2e-cmd"): - argv = list(base) - argv[base.index(flag) + 1] = " " - with pytest.raises(SystemExit) as exc_info: - sandboxed_web_e2e.parse_args(argv) - assert exc_info.value.code == 2 - assert f"{flag} must not be blank" in capsys.readouterr().err - - -def test_parse_args_rejects_malformed_quoting_in_commands(capsys): - """A command with an unmatched shell-quote character is rejected during parse_args. - - Previously, with isolation disabled, this exact input reached - ``shlex.split`` uncaught deep inside ``start_service``/``run_shell`` and - crashed with a raw ``ValueError`` traceback instead of a clean CLI - failure. Validating in ``parse_args`` catches it up front for both - isolation modes. - """ - base = ["--backend-cmd", "backend", "--frontend-cmd", "frontend", "--e2e-cmd", "e2e"] - for flag in ("--backend-cmd", "--frontend-cmd", "--e2e-cmd"): - argv = list(base) - argv[base.index(flag) + 1] = "echo 'unterminated" - with pytest.raises(SystemExit) as exc_info: - sandboxed_web_e2e.parse_args(argv) - assert exc_info.value.code == 2 - assert f"{flag} is not a valid shell command" in capsys.readouterr().err + with pytest.raises(SystemExit) as raised: + sandboxed_web_e2e.parse_args( + [ + "--backend-cmd", + "backend", + "--frontend-cmd", + "frontend", + "--e2e-cmd", + "e2e", + option, + "file:///runner/private", + ] + ) + captured = capsys.readouterr() + + assert raised.value.code == 2 + assert f"{option} must start with http:// or https://" in captured.err + assert "Traceback" not in captured.err + + +@pytest.mark.parametrize( + "option", + ["--backend-ready-url", "--frontend-ready-url"], +) +def test_parse_args_accepts_uppercase_readiness_url_schemes(option): + """An uppercase HTTP(S) scheme is a valid readiness URL, not a rejected one.""" + args = sandboxed_web_e2e.parse_args( + [ + "--backend-cmd", + "backend", + "--frontend-cmd", + "frontend", + "--e2e-cmd", + "e2e", + option, + "HTTP://127.0.0.1:8000/health", + ] + ) + assert getattr(args, option.lstrip("-").replace("-", "_")) == "HTTP://127.0.0.1:8000/health" + + +@pytest.mark.parametrize( + "option", + ["--backend-cmd", "--frontend-cmd", "--e2e-cmd"], +) +def test_parse_args_rejects_empty_commands(option, capsys): + """An empty or whitespace-only command fails in argument parsing without a traceback.""" + commands = {"--backend-cmd": "backend", "--frontend-cmd": "frontend", "--e2e-cmd": "e2e"} + commands[option] = " " + + with pytest.raises(SystemExit) as raised: + sandboxed_web_e2e.parse_args( + [ + "--backend-cmd", + commands["--backend-cmd"], + "--frontend-cmd", + commands["--frontend-cmd"], + "--e2e-cmd", + commands["--e2e-cmd"], + ] + ) + captured = capsys.readouterr() + + assert raised.value.code == 2 + assert f"{option} must not be empty" in captured.err + assert "Traceback" not in captured.err + + +@pytest.mark.parametrize( + "option", + ["--backend-cmd", "--frontend-cmd", "--e2e-cmd"], +) +def test_parse_args_rejects_malformed_command_quoting(option, capsys): + """An unmatched quote in a command fails in argument parsing without a traceback.""" + commands = {"--backend-cmd": "backend", "--frontend-cmd": "frontend", "--e2e-cmd": "e2e"} + commands[option] = 'unterminated "quote' + + with pytest.raises(SystemExit) as raised: + sandboxed_web_e2e.parse_args( + [ + "--backend-cmd", + commands["--backend-cmd"], + "--frontend-cmd", + commands["--frontend-cmd"], + "--e2e-cmd", + commands["--e2e-cmd"], + ] + ) + captured = capsys.readouterr() + + assert raised.value.code == 2 + assert f"{option} is invalid" in captured.err + assert "Traceback" not in captured.err def test_main_disabled_isolation_reports_clean_failure_for_blank_command(tmp_path, capsys): """Disabled isolation still fails a blank command closed, not with a traceback. - This is the exact bug this validation fixes: with ``--isolation - disabled``, a blank command used to bypass ``isolated_command`` entirely - and reach ``shlex.split`` inside ``start_service`` uncaught. + ``parse_args`` rejects a blank backend/frontend/E2E command up front (see + ``test_parse_args_rejects_empty_commands`` above) for both isolation + modes, so ``--isolation disabled`` cannot bypass it to reach + ``shlex.split`` uncaught deep inside ``start_service``. """ repo = tmp_path / "repo" repo.mkdir() @@ -1977,7 +2162,7 @@ def test_main_disabled_isolation_reports_clean_failure_for_blank_command(tmp_pat captured = capsys.readouterr() assert exc_info.value.code == 2 - assert "--frontend-cmd must not be blank" in captured.err + assert "--frontend-cmd must not be empty" in captured.err assert "Traceback" not in captured.err assert "Traceback" not in captured.out @@ -1985,10 +2170,11 @@ def test_main_disabled_isolation_reports_clean_failure_for_blank_command(tmp_pat def test_main_disabled_isolation_reports_clean_failure_for_malformed_quoting(tmp_path, capsys): """Disabled isolation still fails malformed shell-quoting closed, not with a traceback. - This is the exact bug this validation fixes: with ``--isolation - disabled``, unmatched shell-quote characters used to bypass - ``isolated_command`` entirely and raise an uncaught ``ValueError`` from - ``shlex.split`` inside ``run_shell``. + ``parse_args`` rejects an unmatched shell-quote character in the + backend/frontend/E2E commands up front (see + ``test_parse_args_rejects_malformed_command_quoting`` above) for both + isolation modes, so ``--isolation disabled`` cannot bypass it to reach an + uncaught ``ValueError`` from ``shlex.split`` inside ``run_shell``. """ repo = tmp_path / "repo" repo.mkdir() @@ -2011,7 +2197,7 @@ def test_main_disabled_isolation_reports_clean_failure_for_malformed_quoting(tmp captured = capsys.readouterr() assert exc_info.value.code == 2 - assert "--e2e-cmd is not a valid shell command" in captured.err + assert "--e2e-cmd is invalid" in captured.err assert "Traceback" not in captured.err assert "Traceback" not in captured.out 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 0000000000..6faee55a50 --- /dev/null +++ b/tests/test_sandboxed_web_e2e_branch_contract.py @@ -0,0 +1,695 @@ +"""Branch-complete contracts for bounded sandbox web E2E orchestration.""" + +from __future__ import annotations + +import json +import subprocess +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_verify +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" * 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) + + +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://127.0.0.1/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://127.0.0.1/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://127.0.0.1/health" + assert timeout == 2 + return Response() + + clean_running = _service(tmp_path, process=_RunningProcess()) + monkeypatch.setattr( + sandboxed_web_e2e.urllib.request, + "build_opener", + lambda handler, proxy_handler: Opener(), + ) + assert sandboxed_web_e2e.wait_for_url( + "https://127.0.0.1/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, proxy_handler: FailingOpener(), + ) + + assert not sandboxed_web_e2e.wait_for_url( + "https://127.0.0.1/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) + # Once for the force kill after TimeoutExpired, once more for the final + # unconditional same-group cleanup that now always runs before join. + assert forced == [timeout_process, 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_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, + 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), + "--isolation", + "disabled", + "--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() + captures = [] + + class Capture: + """Record the cleanup retry after service termination fails.""" + + output_limited = False + + def __init__(self) -> None: + self.join_calls = 0 + + def join(self, timeout=None) -> None: + """Record the bounded retry timeout.""" + + del timeout + self.join_calls += 1 + + def fake_start(label, command, cwd, env, logs_dir, log_limit_bytes): + """Return completed service doubles.""" + + del command, cwd, env + capture = Capture() + captures.append(capture) + return sandboxed_web_e2e.Service( + label=label, + command=label, + process=cast(subprocess.Popen[bytes], _DoneProcess()), + log_path=logs_dir / f"{label}.log", + capture=capture, + 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")), + ) + forced: list[object] = [] + monkeypatch.setattr( + bounded, + "kill_process_group", + lambda process: forced.append(process), + ) + + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(repository), + "--isolation", + "disabled", + "--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 False + assert _result(captured.out)["output_limit_unsupported"] is False + assert _result(captured.out)["service_capture_failed"] is True + assert len(forced) == 2 + assert [capture.join_calls for capture in captures] == [1, 1] + + +def test_main_classifies_web_symlink_boundary_without_host_target( + tmp_path: Path, + capsys, +) -> None: + """Web E2E rejects a copied symlink without disclosing its host target.""" + repository = tmp_path / "repository" + repository.mkdir() + sensitive_target = tmp_path / "runner-secret.txt" + sensitive_target.write_text("host-only", encoding="utf-8") + (repository / "escape").symlink_to(sensitive_target) + + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(repository), + "--backend-cmd", + "backend", + "--frontend-cmd", + "frontend", + "--e2e-cmd", + "e2e", + ] + ) + captured = capsys.readouterr() + result = _result(captured.out) + + assert exit_code == sandboxed_verify.PATH_BOUNDARY_EXIT_CODE + assert result["path_boundary_rejected"] is True + assert result["cwd"] == "(not-created)" + assert "repository path boundary rejected" in captured.err + assert str(sensitive_target) not in captured.err + assert str(repository) not in captured.err + assert "Traceback" not in captured.err + + +def test_main_reports_web_invalid_root_without_boundary_evidence( + tmp_path: Path, + capsys, +) -> None: + """Web E2E distinguishes an absent repository root from a path escape.""" + missing = tmp_path / "missing-repository" + + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(missing), + "--backend-cmd", + "backend", + "--frontend-cmd", + "frontend", + "--e2e-cmd", + "e2e", + ] + ) + captured = capsys.readouterr() + result = _result(captured.out) + + assert exit_code == 1 + assert result["path_boundary_rejected"] is False + assert result["cwd"] == "(not-created)" + assert "repository root is not a directory" in captured.err + assert str(missing) not in captured.err + + +@pytest.mark.parametrize( + "capture_error", + [RuntimeError("host descriptor detail"), OSError("reader failed")], +) +def test_main_maps_command_capture_failure_to_bounded_resource_evidence( + monkeypatch, + tmp_path: Path, + capsys, + capture_error: BaseException, +) -> None: + """A command-side stuck reader is reported without leaking its exception.""" + repository = tmp_path / "repository" + repository.mkdir() + + def fake_start(label, command, cwd, env, logs_dir, log_limit_bytes): + """Return completed services so the E2E command path is reached.""" + + del cwd, env + return sandboxed_web_e2e.Service( + label=label, + command=command, + 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, **kwargs: (_ for _ in ()).throw(capture_error), + ) + + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(repository), + "--isolation", + "disabled", + "--backend-cmd", + "backend", + "--frontend-cmd", + "frontend", + "--e2e-cmd", + "e2e", + ] + ) + captured = capsys.readouterr() + + assert exit_code == bounded.OUTPUT_LIMIT_EXIT_CODE + assert "bounded output capture failed" in captured.err + assert "host descriptor detail" not in captured.err + assert "Traceback" not in captured.err + + +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), + "--isolation", + "disabled", + "--backend-cmd", + "backend", + "--frontend-cmd", + "frontend", + "--e2e-cmd", + "e2e", + "--e2e-timeout", + "1", + ] + ) == 124 + + +def test_late_service_overflow_preserves_nonzero_e2e_exit( + monkeypatch, + tmp_path: Path, + capsys, +) -> None: + """A late service overflow must not hide an earlier command failure.""" + repository = tmp_path / "repository" + repository.mkdir() + + def fake_start(label, command, cwd, env, logs_dir, log_limit_bytes): + """Return completed service doubles for the late-limit branch.""" + + del command, cwd, env, log_limit_bytes + 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=4096, + ) + + 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=7, + stdout="", + stderr="", + output_limited=False, + ), + ) + monkeypatch.setattr(sandboxed_web_e2e, "stop_service", lambda service: None) + limit_checks = iter([False, True]) + monkeypatch.setattr( + sandboxed_web_e2e, + "_services_output_limited", + lambda services: next(limit_checks), + ) + + assert sandboxed_web_e2e.main( + [ + "--repo-root", + str(repository), + "--isolation", + "disabled", + "--backend-cmd", + "backend", + "--frontend-cmd", + "frontend", + "--e2e-cmd", + "e2e", + ] + ) == 7 + payload = _result(capsys.readouterr().out) + assert payload["output_limited"] is True + assert payload["service_capture_failed"] is False 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 0000000000..39749bb74b --- /dev/null +++ b/tests/test_sandboxed_web_e2e_output_limits.py @@ -0,0 +1,460 @@ +"""Real-process contracts for bounded sandbox web E2E output.""" + +from __future__ import annotations + +import json +import socket +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_verify +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 _free_ports(count: int) -> list[int]: + """Return distinct localhost ports reserved at the same time.""" + + listeners = [socket.socket() for _ in range(count)] + try: + ports = [] + for listener in listeners: + listener.bind(("127.0.0.1", 0)) + ports.append(int(listener.getsockname()[1])) + return ports + finally: + for listener in listeners: + listener.close() + + +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.""" + + 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() + log_limit_bytes = 4096 + 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, + log_limit_bytes, + ) + try: + service.process.wait(timeout=10) + 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) + + +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)), + "--isolation", + "disabled", + "--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["output_limit_unsupported"] is False + assert payload["service_capture_failed"] is False + 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)), + "--isolation", + "disabled", + "--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 payload["output_limit_unsupported"] is False + assert payload["service_capture_failed"] is False + 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.""" + + backend_port, frontend_port = _free_ports(2) + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--isolation", + "disabled", + "--backend-cmd", + _http_service_command(backend_port, "backend-ready"), + "--frontend-cmd", + _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", + "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_unsupported"] is False + assert payload["service_capture_failed"] 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 == f"{bounded.TRUNCATION_MARKER.strip()}\n�\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)), + "--isolation", + "disabled", + "--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 False + assert payload["output_limit_unsupported"] is True + assert payload["service_capture_failed"] is False + + +@pytest.mark.parametrize("missing_role", ["backend", "frontend", "e2e"]) +def test_missing_executable_returns_stable_failed_evidence( + tmp_path: Path, + capsys, + missing_role: str, +) -> None: + """Every command role reports a missing executable without a traceback. + + Isolation is disabled so the command reaches ``start_service``'s own + ``FileNotFoundError`` classification directly -- with isolation enabled + ``isolated_command`` would reject an unresolvable executable earlier as a + generic isolation rejection (see ``test_main_reports_rejected_isolated_command``). + """ + + commands = { + "backend": _command("import time; time.sleep(30)"), + "frontend": _command("import time; time.sleep(30)"), + "e2e": _command("print('ready')"), + } + commands[missing_role] = "missing-web-e2e-executable" + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--isolation", + "disabled", + "--backend-cmd", + commands["backend"], + "--frontend-cmd", + commands["frontend"], + "--e2e-cmd", + commands["e2e"], + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == sandboxed_verify.COMMAND_NOT_FOUND_EXIT_CODE + assert payload["exit_code"] == sandboxed_verify.COMMAND_NOT_FOUND_EXIT_CODE + assert "install each executable or correct command PATH" in captured.err + assert "Traceback" not in captured.err + + +@pytest.mark.parametrize("command_role", ["backend", "frontend", "e2e"]) +@pytest.mark.parametrize("candidate_kind", ["file", "directory"]) +def test_non_executable_command_returns_stable_failed_evidence( + tmp_path: Path, + capsys, + command_role: str, + candidate_kind: str, +) -> None: + """Every web command role classifies a present but unusable executable. + + Isolation is disabled so the command reaches ``start_service``'s own + ``PermissionError``/``IsADirectoryError`` classification directly -- with + isolation enabled ``isolated_command`` would reject the same candidate + earlier as a generic isolation rejection. + """ + + candidate = tmp_path / "web-command-candidate" + if candidate_kind == "file": + candidate.write_text("not executable\n", encoding="utf-8") + else: + candidate.mkdir() + commands = { + "backend": _command("import time; time.sleep(30)"), + "frontend": _command("import time; time.sleep(30)"), + "e2e": _command("print('ready')"), + } + commands[command_role] = str(candidate) + + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--isolation", + "disabled", + "--backend-cmd", + commands["backend"], + "--frontend-cmd", + commands["frontend"], + "--e2e-cmd", + commands["e2e"], + ] + ) + captured = capsys.readouterr() + payload = _result_payload(captured.out) + + assert exit_code == sandboxed_verify.COMMAND_NOT_EXECUTABLE_EXIT_CODE + assert payload["exit_code"] == sandboxed_verify.COMMAND_NOT_EXECUTABLE_EXIT_CODE + assert "select executable files or correct their permissions" in captured.err + assert "Traceback" not in captured.err + + +@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) + base = [ + "--repo-root", + str(repository), + "--backend-cmd", + _command("pass"), + "--frontend-cmd", + _command("pass"), + "--e2e-cmd", + _command("pass"), + ] + 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.""" + + log_limit_bytes = 4096 + exit_code = sandboxed_web_e2e.main( + [ + "--repo-root", + str(_repository(tmp_path)), + "--isolation", + "disabled", + "--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", + str(log_limit_bytes), + "--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 <= log_limit_bytes + finally: + shutil.rmtree(sandbox_path, ignore_errors=True)