diff --git a/.github/workflows/agent-review-runtime-quality-ci.yml b/.github/workflows/agent-review-runtime-quality-ci.yml index 6c6efc3dd1..992aef9710 100644 --- a/.github/workflows/agent-review-runtime-quality-ci.yml +++ b/.github/workflows/agent-review-runtime-quality-ci.yml @@ -2,9 +2,13 @@ name: Agent Review Runtime Quality CI on: pull_request: + types: [opened, synchronize, reopened, ready_for_review] branches: [main] paths: - ".github/workflows/agent-review-runtime-quality-ci.yml" + - "scripts/ci/sandboxed_verify.py" + - "tests/test_sandboxed_verify.py" + - "tests/test_required_workflow_queue_contract.py" - ".github/workflows/noema-review.yml" - ".github/actions/noema-review/two_phase.py" - "tests/test_noema_reviewer_token_lifetime.py" @@ -116,6 +120,7 @@ permissions: jobs: agent_review_runtime_quality: name: agent-review-runtime-quality + if: github.event.pull_request.draft == false runs-on: ubuntu-24.04 timeout-minutes: 25 env: @@ -152,6 +157,7 @@ jobs: opencode_suite=false strix_suite=false queue_suite=false + sandbox_suite=false review_repair_suite=false commercial_readiness_suite=false exact_artifact_suite=false @@ -163,6 +169,7 @@ jobs: opencode_suite=true strix_suite=true queue_suite=true + sandbox_suite=true review_repair_suite=true commercial_readiness_suite=true exact_artifact_suite=true @@ -217,6 +224,13 @@ jobs: scripts/ci/current_head_run_coalescer.py) queue_suite=true ;; + tests/test_required_workflow_queue_contract.py) + queue_suite=true + ;; + scripts/ci/sandboxed_verify.py|\ + tests/test_sandboxed_verify.py) + sandbox_suite=true + ;; .github/workflows/pr-review-fix-scheduler.yml|\ scripts/ci/pr_review_fix_scheduler.py|\ scripts/ci/pr_review_merge_scheduler.py|\ @@ -297,6 +311,7 @@ jobs: echo "opencode=$opencode_suite" echo "strix=$strix_suite" echo "queue=$queue_suite" + echo "sandbox=$sandbox_suite" echo "review_repair=$review_repair_suite" echo "commercial_readiness=$commercial_readiness_suite" echo "exact_artifact=$exact_artifact_suite" @@ -322,7 +337,7 @@ jobs: -r "${RUNNER_TEMP}/strix-quality-requirements.txt" - name: Install exact review dependencies - if: steps.affected_suites.outputs.noema == 'true' || steps.affected_suites.outputs.opencode == 'true' || steps.affected_suites.outputs.review_repair == 'true' || steps.affected_suites.outputs.exact_artifact == 'true' + if: steps.affected_suites.outputs.noema == 'true' || steps.affected_suites.outputs.opencode == 'true' || steps.affected_suites.outputs.sandbox == 'true' || steps.affected_suites.outputs.review_repair == 'true' || steps.affected_suites.outputs.exact_artifact == 'true' run: >- python -m pip install --disable-pip-version-check --require-hashes -r requirements-opencode-review-ci-hashes.txt @@ -385,8 +400,27 @@ jobs: if: steps.affected_suites.outputs.queue == 'true' run: | set -euo pipefail - python -m pytest -q tests/test_current_head_coalescer_self_cancellation.py - python -m compileall -q tests/test_current_head_coalescer_self_cancellation.py + python -m pytest -q \ + tests/test_current_head_coalescer_self_cancellation.py \ + tests/test_required_workflow_queue_contract.py + python -m compileall -q \ + tests/test_current_head_coalescer_self_cancellation.py \ + tests/test_required_workflow_queue_contract.py + + - name: Verify sandbox evidence contracts + if: steps.affected_suites.outputs.sandbox == 'true' + run: | + set -euo pipefail + python -m coverage erase + python -m coverage run --branch -m pytest -q tests/test_sandboxed_verify.py + python -m coverage report \ + --include=scripts/ci/sandboxed_verify.py \ + --show-missing \ + --fail-under=100 + python -m interrogate --fail-under 100 scripts/ci/sandboxed_verify.py + python -m compileall -q \ + scripts/ci/sandboxed_verify.py \ + tests/test_sandboxed_verify.py - name: Verify scheduler and contextual-orchestrator review-repair contracts if: steps.affected_suites.outputs.review_repair == 'true' diff --git a/.github/workflows/codeql-pr.yml b/.github/workflows/codeql-pr.yml index c21c8446df..c190bb7a0a 100644 --- a/.github/workflows/codeql-pr.yml +++ b/.github/workflows/codeql-pr.yml @@ -55,7 +55,7 @@ permissions: jobs: detect-languages: name: Detect CodeQL languages - if: github.event.action != 'closed' + if: github.event.action != 'closed' && github.event.pull_request.draft == false runs-on: ubuntu-24.04 permissions: contents: read diff --git a/.github/workflows/python-security.yml b/.github/workflows/python-security.yml index 8453895027..dfaee6dd89 100644 --- a/.github/workflows/python-security.yml +++ b/.github/workflows/python-security.yml @@ -45,7 +45,9 @@ permissions: jobs: detect-python: name: Detect Python - if: github.event.action != 'closed' + if: >- + github.event.action != 'closed' && + (github.event_name != 'pull_request' || github.event.pull_request.draft == false) runs-on: ubuntu-24.04 outputs: has_python: ${{ steps.detect.outputs.has_python }} diff --git a/.github/workflows/sast-semgrep.yml b/.github/workflows/sast-semgrep.yml index 12b7013da3..e9adce6dd0 100644 --- a/.github/workflows/sast-semgrep.yml +++ b/.github/workflows/sast-semgrep.yml @@ -47,7 +47,7 @@ jobs: # here and consumed through `needs`. See # docs/doctoring/required-workflow-path-filter-boundary.md. # Fails OPEN: an unreadable, empty, or truncated file list scans everything. - if: github.event.action != 'closed' + if: github.event.action != 'closed' && (github.event_name != 'pull_request' || github.event.pull_request.draft == false) runs-on: ubuntu-24.04 timeout-minutes: 5 permissions: diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 500e22b4ab..638323217c 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -70,7 +70,7 @@ jobs: # here and consumed through `needs`. See # docs/doctoring/required-workflow-path-filter-boundary.md. # Fails OPEN: an unreadable, empty, or truncated file list scans everything. - if: github.event.action != 'closed' + if: github.event.action != 'closed' && github.event.pull_request.draft == false runs-on: ubuntu-24.04 timeout-minutes: 5 permissions: @@ -431,7 +431,7 @@ jobs: # push, schedule, and manual backstops remain in secret-scan.yml. gitleaks: name: gitleaks (secret scan) - if: github.event.action != 'closed' && github.repository == 'ContextualWisdomLab/.github' + if: github.event.action != 'closed' && github.event.pull_request.draft == false && github.repository == 'ContextualWisdomLab/.github' runs-on: ubuntu-24.04 permissions: contents: read diff --git a/CHANGELOG.md b/CHANGELOG.md index 707c18532e..2035da40cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,23 @@ +### Sandboxed verification emits a versioned, binary-safe trusted result bundle + +- `scripts/ci/sandboxed_verify.py --result-file ` now keeps command + stdout, command stderr, and the wrapper-controlled JSON envelope in three + exclusive sibling files. The stream files preserve arbitrary and large + binary bytes exactly; their SHA-256 digests and byte lengths are bound into + the `sandboxed_verify.execution.v1` envelope with argv, exit code, explicit + completed/timeout/copy-rejection/internal-error state, runtime identity, + requested network mode, and allowed environment names. Result-directory + traversal uses directory file descriptors with no-follow semantics for every ancestor, and every bundle + file uses exclusive creation, closing the nested-symlink and substitution + races in the first result-file implementation. A bounded evidence-write + failure returns 125 without a traceback when the command succeeded, preserves + an existing command/timeout/copy-rejection failure code, and cannot skip + temporary sandbox cleanup unless `--keep-sandbox` explicitly requests + retention. The envelope explicitly records that this helper supplies a copied + workspace and scrubbed environment, not OS process isolation or enforced + network policy. Legacy stdout-marker mode remains available for human-only + calls. Refs #2086, #2088. + ### Failed-check finding names the Strix sandbox instead of the gateway - `opencode-review-dispatch.yml`'s `emit_strix_provider_failure_finding` rendered one fixed finding for every `STRIX_PROVIDER_UNAVAILABLE` line, whose Root cause read "The contextual-orchestrator gateway or its discovered provider pool was unavailable for this run". `#1953` had just given the Strix sandbox bootstrap failure its own second verdict token (`STRIX_SANDBOX_UNAVAILABLE`) precisely because that attribution is wrong for it -- the sandbox container never reaches its Caido proxy, so the run dies before the gateway serves anything -- and this consumer re-applied the wrong attribution one step downstream, into the review findings and the failure census. The emitter now branches on the second token: a sandbox verdict gets a finding that names Strix's sandbox, says the verdict does not name the gateway, and tells the reader not to change gateway or provider configuration on its strength. A `STRIX_PROVIDER_UNAVAILABLE` line without the token keeps its existing text verbatim, so the gateway class has no regression surface. No test covered this finding text at all before (`gateway or its discovered provider pool` matched nothing under `tests/`); `tests/test_opencode_dispatch_strix_sandbox_finding.py` now runs the production emitter from the published run block and pins both directions plus the no-signal case. Refs #1953, #1935. @@ -168,6 +188,18 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- **Stop Draft PR pushes from consuming five required-workflow runner lanes.** + Every independent entry job in Runtime Quality, CodeQL, Security Scan + (including its document-sensitive Gitleaks gate), Python Security, and SAST + now skips while a pull request is Draft. Existing + `ready_for_review` triggers create fresh exact-head evidence after review + admission; Runtime Quality now explicitly subscribes to that event as well. + Runtime Quality's path selector now executes the 100% branch-coverage and + public-doc gate whenever `sandboxed_verify.py` or its contract changes; + the selector contract slices the actual trigger block instead of matching + paths vacuously elsewhere in the workflow. + Push, schedule, and repository-dispatch coverage remains intact. + A contract pins both pull-request-only and mixed-event guards. - **Pin `opencode-review-dispatch.yml` off the starved floating `ubuntu-latest` image.** The 2026-09-01 floating-image fix (see that entry below) pinned `strix.yml`, `opencode-review.yml`, and `noema-review.yml` -- the three required-check diff --git a/docs/doctoring/draft-required-workflow-admission-20260912.md b/docs/doctoring/draft-required-workflow-admission-20260912.md new file mode 100644 index 0000000000..c9688f2a8e --- /dev/null +++ b/docs/doctoring/draft-required-workflow-admission-20260912.md @@ -0,0 +1,68 @@ +# Draft required-workflow admission + +## Problem and exact evidence + +Pull request #2106 head `24bb6591ab7df23558cb793b4af60c567ff9da97` +generated Security Scan `34688578677`, CodeQL PR `34688578675`, SAST +`34688578674`, Python Security `34688578683`, and Runtime Quality +`34688578679` while the pull request was Draft. Restoring Ready at the same +head generated a second set. The first four runs were cancelled after queued +jobs had already entered admission; Runtime Quality had already consumed a +runner and completed. This is same-head lifecycle queue waste, not stale source +or a test failure. + +The first successor canary exposed two independent omissions. Security Scan +skipped its `changed-scope` path but admitted independent Gitleaks job +`103542086113`. After that guard was repaired, Ready restoration generated four +security workflows but no Runtime Quality run because that workflow relied on +the default `pull_request` activity set, which excludes `ready_for_review`. +Current-head review then found that sandbox evidence paths started Runtime +Quality without selecting or executing a sandbox contract suite. RED +`3648b848` requires non-vacuous selection and GREEN `d1473882` adds the +100% branch-coverage/public-doc gate plus queue-contract execution. A second +RED `b6715554` proves the selector test's literal `\\n` split retained the +entire workflow; GREEN `2c00900e` restricts assertions to the actual trigger +block. + +## Constraints and selected repair + +The workflows must keep `ready_for_review`, PR-keyed concurrency, and their +existing close-event behavior. Security workflows that also run on push, +schedule, or `repository_dispatch` must not lose those non-PR paths. Trigger +filters alone are insufficient for organization required workflows, so the +repair uses the existing job-level policy boundary: + +- pull-request-only workflows require `pull_request.draft == false` on every + independent entry job, including both Security Scan `changed-scope` and its + document-sensitive `gitleaks` gate; +- mixed-event workflows allow every non-PR event and require non-Draft state + only for pull-request events; +- downstream jobs remain unchanged and naturally skip through `needs` when the + admission job skips. +- Runtime Quality explicitly subscribes to `ready_for_review`, so its Draft + skip cannot strand the exact head when review admission opens. +- Sandbox verifier changes select a dedicated suite with 100% branch coverage, + 100% public documentation, compilation, and the queue selector contract. +- Trigger-path assertions exclude the workflow `jobs` block, preventing a + matching path elsewhere from satisfying admission tests. + +No new workflow, dependency, scheduler, token, or status context is added. + +## Alternatives rejected + +- Removing `ready_for_review` would strand Draft-origin PRs without fresh + evidence when they become reviewable. +- Adding head SHA to concurrency would not prevent the same-head lifecycle + duplication and would weaken close-event cancellation. +- Cancelling the duplicate later still spends queue admission and runner time. + +## Verification and follow-up + +`tests/test_required_workflow_queue_contract.py` binds all five workflows and +all independent entry-job guards while preserving the existing close-event +contract, requiring Runtime Quality Ready admission, executing sandbox evidence +contracts, and limiting trigger assertions to the actual trigger block. The +proposal is not +complete until exact-head hosted Checks and independent review pass, it merges +through ordinary protection, and a post-merge Draft→Ready canary shows skipped +Draft jobs followed by one fresh Ready generation. diff --git a/docs/pr-review-and-merge-procedure.md b/docs/pr-review-and-merge-procedure.md index e34c3957cb..df351d3861 100644 --- a/docs/pr-review-and-merge-procedure.md +++ b/docs/pr-review-and-merge-procedure.md @@ -215,10 +215,23 @@ the specific environment variable names required and record why they were needed. The central helper is `python3 scripts/ci/sandboxed_verify.py --repo-root -- `; reviews should cite its `SANDBOXED_VERIFY_RESULT` -line when the helper is used. Use `--network required`, `--allow-env NAME`, -and `--evidence-note "why"` only for repository-required verification. This -helper does not replace the existing bash, task, webfetch, websearch, lsp, -CodeGraph, DeepWiki, Context7, or web_search review policy. +line when the helper is used. For machine handoff, pass `--result-file +`: the helper exclusively creates that versioned envelope plus +`.stdout` and `.stderr`. Those files preserve all +command-controlled stdout and stderr bytes exactly, including marker-shaped or +JSON-shaped content; only the wrapper-authored envelope is trusted control +data. The envelope records hashes, byte lengths, argv, exit code, +`completed`/`timed_out`/`copy_rejected`/`internal_error` state, runtime +identity, allowed environment names, and the requested network mode. It also +says explicitly that this copy-and-scrub helper provides no OS process isolation and does not +enforce network policy. Trusted result paths reject symlink ancestors and +existing bundle files; evidence-write failure is bounded, returns 125 only +when the command succeeded, preserves an existing command/timeout/copy failure +status, and never skips sandbox cleanup unless `--keep-sandbox` explicitly +requests retention. Use `--network required`, `--allow-env NAME`, and +`--evidence-note "why"` only for repository-required verification. This helper +does not replace the existing bash, task, webfetch, websearch, lsp, CodeGraph, +DeepWiki, Context7, or web_search review policy. Scratch PoC files are not committed. For web applications with both backend and frontend surfaces, the preferred diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 1cc9e20313..b5ee9e34ea 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -78,6 +78,7 @@ flowchart LR | Gap ID | 현재 관측 | 구매자 영향 | 우선 구현/검증 | |---|---|---|---| +| GAP-DRAFT-RUNNER-ADMISSION | 같은 exact head의 Draft push가 다섯 required workflow runner lane을 먼저 점유하고, Ready 전환이 그 generation을 취소한 뒤 새 generation을 만들었다. 첫 수리 canary에서는 Security Scan의 독립 `gitleaks` job만 여전히 runner queue에 진입했고, 그 guard 수리 뒤에는 Runtime Quality가 `ready_for_review`를 구독하지 않아 Ready generation 자체가 누락됐다. 후속 검토에서는 sandbox evidence path가 workflow를 시작해도 실제 sandbox suite가 선택되지 않는 경로와, trigger assertion이 literal `\\n`으로 전체 workflow를 검사하던 vacuous 계약을 확인했다. | 검토할 수 없는 Proposed 변경이 scarce runner capacity를 소모하거나 Ready exact head에 필수 품질 evidence가 영구 누락돼 merge-ready PR의 검증을 지연한다. | 모든 독립 entry job에서 Draft를 job-level skip하고 `ready_for_review`에서 fresh exact-head Checks를 생성한다. Runtime Quality는 sandbox evidence를 100% branch-coverage/public-doc gate로 실행하고 queue 계약을 함께 검증하며, trigger assertion은 실제 trigger block에 한정한다. Security Scan의 `changed-scope`와 document-sensitive `gitleaks`를 포함하고 mixed-event workflow의 push/schedule/dispatch는 유지한다. Proposed successor에서 RED→GREEN 및 hosted evidence를 검증한다. | | G-01 | 열린 PR은 107개다. metadata 상태는 BLOCKED=17, BEHIND=16, DIRTY=74, draft 13개다. 상태는 independent exact-head approval과 terminal required Checks를 자동으로 의미하지 않는다 | 안전하게 출시할 변경과 대기 중인 변경을 구별할 수 없다 | PR마다 current head, reviews, threads, required Checks, merge-result tree를 재수집하고 보호 조건 미충족이면 merge하지 않는다 | | G-02 | protected `main`은 `826b92394c63deb6981c3a8d16a724d71f85a0d7`이며, BEHIND/stacked PR의 predecessor evidence를 current-head approval로 승격할 수 없다 | 리뷰가 호출돼도 승인 증거가 생성되지 않아 자동화가 멈춘다 | current-head quality와 OpenCode/Noema/Strix를 재실행하고, exact SHA·run ID·review commit SHA를 한 receipt에 묶는다 | | G-03 | #1297은 Strix per-repository serialization과 scoped close cleanup을, #1345/#1347은 normalizer/web-E2E 안전성을 다룬다. 각 PR의 provider failure와 source/control-plane failure를 구분해야 한다 | 취약점 0건이어도 CI 인프라 결함이 보안 결과처럼 보이고 큐가 막힌다 | D3 교착 증거를 별도 수집하고, vulnerability marker는 절대 neutralize하지 않으며, 정상 gate 복구 후 exact-head hosted evidence를 재생성한다 | diff --git a/scripts/ci/sandboxed_verify.py b/scripts/ci/sandboxed_verify.py index 94797c2038..e17fa00a9c 100644 --- a/scripts/ci/sandboxed_verify.py +++ b/scripts/ci/sandboxed_verify.py @@ -3,9 +3,12 @@ from __future__ import annotations import argparse +import hashlib import json import os +import platform import re +import secrets import shutil import subprocess import sys @@ -94,8 +97,7 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: """Parse CLI arguments for the sandboxed verification wrapper.""" parser = argparse.ArgumentParser( description=( - "Copy the repository into a temporary workspace and run a verification " - "command with a scrubbed environment." + "Copy the repository into a temporary workspace and run a verification command with a scrubbed environment." ) ) parser.add_argument("--repo-root", default=".", help="Repository root to copy into the sandbox.") @@ -129,6 +131,15 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: default="", help="Short reviewer note explaining why network or allowed env variables are needed.", ) + parser.add_argument( + "--result-file", + type=Path, + help=( + "Write the trusted result envelope to a new file and exact command " + "streams to sibling .stdout/.stderr files instead of mixing evidence " + "with command output." + ), + ) parser.add_argument("command", nargs=argparse.REMAINDER, help="Verification command after --.") args = parser.parse_args(argv) if args.command and args.command[0] == "--": @@ -164,7 +175,13 @@ def scrubbed_env(sandbox_root: Path, allow_env: Sequence[str] = ()) -> dict[str, "XDG_DATA_HOME": str(sandbox_root / "xdg-data"), } ) - for path_key in ("HOME", "TMPDIR", "XDG_CACHE_HOME", "XDG_CONFIG_HOME", "XDG_DATA_HOME"): + for path_key in ( + "HOME", + "TMPDIR", + "XDG_CACHE_HOME", + "XDG_CONFIG_HOME", + "XDG_DATA_HOME", + ): Path(env[path_key]).mkdir(parents=True, exist_ok=True) return env @@ -208,7 +225,12 @@ def _reject_escaping_symlinks(destination: Path) -> None: for path in root.rglob("*"): if path.is_symlink(): _resolve_symlink_components( - path.relative_to(root).parts, root, root, set(), [MAXIMUM_SYMLINK_HOPS], path + path.relative_to(root).parts, + root, + root, + set(), + [MAXIMUM_SYMLINK_HOPS], + path, ) @@ -274,12 +296,8 @@ def _resolve_symlink_components( hops_remaining[0] -= 1 target = Path(os.readlink(step)) if target.is_absolute(): - raise ValueError( - f"workspace symlink escapes the sandbox root: {step} -> {target}" - ) - resolved = _resolve_symlink_components( - target.parts, resolved, root, active, hops_remaining, candidate - ) + raise ValueError(f"workspace symlink escapes the sandbox root: {step} -> {target}") + resolved = _resolve_symlink_components(target.parts, resolved, root, active, hops_remaining, candidate) active.discard(step) return resolved @@ -314,9 +332,7 @@ def _ignore(directory: str, names: list[str]) -> set[str]: default_ignored = default_ignore(directory, names) extra_ignored = extra_ignore(directory, names) protected = { - name - for name in default_ignored - if name in DEFAULT_ENV_TEMPLATE_ALLOWLIST and name not in extra_ignored + name for name in default_ignored if name in DEFAULT_ENV_TEMPLATE_ALLOWLIST and name not in extra_ignored } return (default_ignored | extra_ignored) - protected @@ -335,13 +351,14 @@ def copy_workspace(repo_root: Path, sandbox_root: Path, extra_ignores: Sequence[ return destination -def run_command(command: Sequence[str], cwd: Path, env: dict[str, str], timeout: int) -> subprocess.CompletedProcess[str]: +def run_command( + command: Sequence[str], cwd: Path, env: dict[str, str], timeout: int +) -> subprocess.CompletedProcess[bytes]: """Run the verification command and capture output for review evidence.""" return subprocess.run( list(command), cwd=cwd, env=env, - text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, timeout=timeout, @@ -359,6 +376,152 @@ def timeout_output_text(value: str | bytes | None) -> str: return value +def _output_bytes(value: str | bytes | None) -> bytes: + """Normalize captured command output without altering subprocess bytes.""" + if value is None: + return b"" + if isinstance(value, bytes): + return value + return value.encode() + + +def _forward_bytes(stream: object, output: bytes) -> None: + """Forward command bytes exactly when the active stream exposes a buffer.""" + if not output: + return + binary_stream = getattr(stream, "buffer", None) + if binary_stream is not None: + stream.flush() # type: ignore[attr-defined] + binary_stream.write(output) + binary_stream.flush() + return + stream.write(output.decode(errors="replace")) # type: ignore[attr-defined] + stream.flush() # type: ignore[attr-defined] + + +def _open_result_parent(parent: Path) -> int: + """Open/create ``parent`` component-wise without following symlinks.""" + directory_flags = os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) | getattr(os, "O_NOFOLLOW", 0) + if parent.is_absolute(): + directory_fd = os.open(os.path.sep, directory_flags) + components = parent.parts[1:] + else: + directory_fd = os.open(".", directory_flags) + components = parent.parts + try: + for component in components: + if component in ("", "."): + continue + if component == "..": + raise ValueError(f"result file parent is not a regular directory: {parent}") + try: + next_fd = os.open(component, directory_flags, dir_fd=directory_fd) + except FileNotFoundError: + try: + os.mkdir(component, mode=0o700, dir_fd=directory_fd) + except FileExistsError: + pass + try: + next_fd = os.open(component, directory_flags, dir_fd=directory_fd) + except OSError as exc: + raise ValueError(f"result file parent is not a regular directory: {parent}") from exc + except OSError as exc: + raise ValueError(f"result file parent is not a regular directory: {parent}") from exc + os.close(directory_fd) + directory_fd = next_fd + return directory_fd + except BaseException: + os.close(directory_fd) + raise + + +def _write_all(file_descriptor: int, content: bytes) -> None: + """Write all ``content`` to an already-open file descriptor.""" + offset = 0 + while offset < len(content): + offset += os.write(file_descriptor, content[offset:]) + + +def _write_result_bundle( + result_file: Path, + rendered_result: bytes, + stdout_bytes: bytes, + stderr_bytes: bytes, +) -> None: + """Create both streams before atomically publishing the trusted envelope.""" + parent_fd = _open_result_parent(result_file.parent) + stdout_name = result_file.name + ".stdout" + stderr_name = result_file.name + ".stderr" + stream_bundle = ( + (stdout_name, stdout_bytes), + (stderr_name, stderr_bytes), + ) + created_names: list[str] = [] + staged_envelope_name = f".{result_file.name}.{secrets.token_hex(16)}.tmp" + staged_envelope_created = False + file_flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_NOFOLLOW", 0) + try: + try: + os.stat(result_file.name, dir_fd=parent_fd, follow_symlinks=False) + except FileNotFoundError: + pass + else: + raise ValueError(f"result file already exists: {result_file}") + for file_name, content in stream_bundle: + try: + file_descriptor = os.open(file_name, file_flags, 0o600, dir_fd=parent_fd) + except FileExistsError as exc: + raise ValueError( + f"result bundle file already exists: {result_file.parent / file_name}" + ) from exc + created_names.append(file_name) + try: + _write_all(file_descriptor, content) + os.fsync(file_descriptor) + finally: + os.close(file_descriptor) + + staged_descriptor = os.open( + staged_envelope_name, + file_flags, + 0o600, + dir_fd=parent_fd, + ) + staged_envelope_created = True + try: + _write_all(staged_descriptor, rendered_result) + os.fsync(staged_descriptor) + finally: + os.close(staged_descriptor) + try: + os.link( + staged_envelope_name, + result_file.name, + src_dir_fd=parent_fd, + dst_dir_fd=parent_fd, + follow_symlinks=False, + ) + except FileExistsError as exc: + raise ValueError(f"result file already exists: {result_file}") from exc + created_names.append(result_file.name) + os.unlink(staged_envelope_name, dir_fd=parent_fd) + staged_envelope_created = False + except BaseException: + if staged_envelope_created: + try: + os.unlink(staged_envelope_name, dir_fd=parent_fd) + except FileNotFoundError: + pass + for file_name in created_names: + try: + os.unlink(file_name, dir_fd=parent_fd) + except FileNotFoundError: + pass + raise + finally: + os.close(parent_fd) + + def emit_result( *, command: Sequence[str], @@ -370,8 +533,15 @@ def emit_result( allowed_env: Sequence[str], network: str, evidence_note: str, + result_file: Path | None = None, + result_state: str = "completed", + timed_out: bool = False, + stdout_bytes: bytes = b"", + stderr_bytes: bytes = b"", ) -> None: - """Print a machine-readable execution evidence summary.""" + """Write a versioned execution envelope and its exact command streams.""" + stdout_name = result_file.name + ".stdout" if result_file is not None else None + stderr_name = result_file.name + ".stderr" if result_file is not None else None payload = { "allowed_env": sorted(set(allowed_env)), "command": list(command), @@ -379,11 +549,38 @@ def emit_result( "elapsed_seconds": round(elapsed_seconds, 3), "evidence_note": evidence_note, "exit_code": exit_code, + "helper_id": "ContextualWisdomLab/.github:sandboxed_verify", + "isolation": { + "network_enforced": False, + "os_process_isolation": "none", + "workspace": "copy+scrubbed-env", + }, "network": network, + "result_state": result_state, + "runtime": { + "implementation": platform.python_implementation(), + "python_version": platform.python_version(), + }, "sandbox": str(sandbox_root) if kept else "(removed)", "sandboxed": True, + "schema": "sandboxed_verify.execution.v1", + "stderr": { + "file": stderr_name, + "sha256": hashlib.sha256(stderr_bytes).hexdigest(), + "size_bytes": len(stderr_bytes), + }, + "stdout": { + "file": stdout_name, + "sha256": hashlib.sha256(stdout_bytes).hexdigest(), + "size_bytes": len(stdout_bytes), + }, + "timed_out": timed_out, } - print(f"{RESULT_MARKER} {json.dumps(payload, sort_keys=True)}") + rendered = f"{RESULT_MARKER} {json.dumps(payload, sort_keys=True)}\n" + if result_file is None: + print(rendered, end="") + return + _write_result_bundle(result_file, rendered.encode(), stdout_bytes, stderr_bytes) def main(argv: Sequence[str] | None = None) -> int: @@ -393,52 +590,77 @@ def main(argv: Sequence[str] | None = None) -> int: start = time.monotonic() exit_code = 1 copied_repo = sandbox / "repo" + result_state = "internal_error" + command_stdout = b"" + command_stderr = b"" try: try: copied_repo = copy_workspace(Path(args.repo_root), sandbox, args.ignore) except ValueError as exc: print(f"sandboxed-verify: workspace copy rejected: {exc}", file=sys.stderr) exit_code = 125 - return exit_code - env = scrubbed_env(sandbox, args.allow_env) - print(f"sandboxed-verify: cwd={copied_repo}") - print(f"sandboxed-verify: command={' '.join(args.command)}") - if args.allow_env: - print(f"sandboxed-verify: allowed env names={','.join(sorted(set(args.allow_env)))}") - if args.network != "default": - print(f"sandboxed-verify: network={args.network}") - try: - completed = run_command(args.command, copied_repo, env, args.timeout) - if completed.stdout: - print(completed.stdout, end="") - if completed.stderr: - print(completed.stderr, end="", file=sys.stderr) - exit_code = completed.returncode - except subprocess.TimeoutExpired as exc: - stdout = timeout_output_text(exc.stdout) - stderr = timeout_output_text(exc.stderr) - if stdout: - print(stdout, end="" if stdout.endswith("\n") else "\n") - if stderr: - print(stderr, end="" if stderr.endswith("\n") else "\n", file=sys.stderr) - print(f"sandboxed-verify: command timed out after {args.timeout}s", file=sys.stderr) - exit_code = 124 - return exit_code + result_state = "copy_rejected" + else: + env = scrubbed_env(sandbox, args.allow_env) + print(f"sandboxed-verify: cwd={copied_repo}") + print(f"sandboxed-verify: command={' '.join(args.command)}") + if args.allow_env: + print(f"sandboxed-verify: allowed env names={','.join(sorted(set(args.allow_env)))}") + if args.network != "default": + print(f"sandboxed-verify: network={args.network}") + try: + completed = run_command(args.command, copied_repo, env, args.timeout) + command_stdout = _output_bytes(completed.stdout) + command_stderr = _output_bytes(completed.stderr) + _forward_bytes(sys.stdout, command_stdout) + _forward_bytes(sys.stderr, command_stderr) + exit_code = completed.returncode + result_state = "completed" + except subprocess.TimeoutExpired as exc: + command_stdout = _output_bytes(exc.stdout) + command_stderr = _output_bytes(exc.stderr) + _forward_bytes(sys.stdout, command_stdout) + _forward_bytes(sys.stderr, command_stderr) + print( + f"sandboxed-verify: command timed out after {args.timeout}s", + file=sys.stderr, + ) + exit_code = 124 + result_state = "timed_out" finally: elapsed = time.monotonic() - start - emit_result( - command=args.command, - copied_repo=copied_repo, - sandbox_root=sandbox, - exit_code=exit_code, - elapsed_seconds=elapsed, - kept=args.keep_sandbox, - allowed_env=args.allow_env, - network=args.network, - evidence_note=args.evidence_note, - ) - if not args.keep_sandbox: - shutil.rmtree(sandbox, ignore_errors=True) + try: + emit_result( + command=args.command, + copied_repo=copied_repo, + sandbox_root=sandbox, + exit_code=exit_code, + elapsed_seconds=elapsed, + kept=args.keep_sandbox, + allowed_env=args.allow_env, + network=args.network, + evidence_note=args.evidence_note, + result_file=args.result_file, + result_state=result_state, + timed_out=result_state == "timed_out", + stdout_bytes=command_stdout, + stderr_bytes=command_stderr, + ) + except (OSError, ValueError) as exc: + diagnostic = str(exc).splitlines()[0][:240] + print( + f"sandboxed-verify: result evidence rejected: {diagnostic}", + file=sys.stderr, + ) + # Evidence rejection is the primary failure only when the command + # itself succeeded. Preserve an existing command, timeout, or copy + # rejection status so callers do not lose the causal exit code. + if exit_code == 0: + exit_code = 125 + finally: + if not args.keep_sandbox: + shutil.rmtree(sandbox, ignore_errors=True) + return exit_code if __name__ == "__main__": diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 19fe6b0f7f..530bdbcc01 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -1058,6 +1058,41 @@ def test_pr_keyed_scan_workflows_pin_cancellation_as_a_value() -> None: assert "github.event_name" not in group_value +def test_required_heavy_jobs_wait_until_pull_request_is_ready() -> None: + """Draft pushes must not consume runners before review admission.""" + pull_request_only_jobs = ( + ("agent-review-runtime-quality-ci.yml", "agent_review_runtime_quality"), + ("codeql-pr.yml", "detect-languages"), + ("security-scan.yml", "changed-scope"), + ("security-scan.yml", "gitleaks"), + ) + mixed_event_jobs = { + "python-security.yml": "detect-python", + "sast-semgrep.yml": "changed-scope", + } + + for filename, job_name in pull_request_only_jobs: + workflow = workflow_text(filename) + job_match = re.search( + rf"(?ms)^ {re.escape(job_name)}:\n(.*?)(?=^ [a-zA-Z0-9_-]+:\s*$|\Z)", + workflow, + ) + assert job_match is not None + job = job_match.group(1) + assert "github.event.pull_request.draft == false" in job + + for filename, job_name in mixed_event_jobs.items(): + workflow = workflow_text(filename) + job_match = re.search( + rf"(?ms)^ {re.escape(job_name)}:\n(.*?)(?=^ [a-zA-Z0-9_-]+:\s*$|\Z)", + workflow, + ) + assert job_match is not None + job = job_match.group(1) + assert "github.event_name != 'pull_request'" in job + assert "github.event.pull_request.draft == false" in job + + def test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs() -> None: """Close events should cancel old runs without starting expensive jobs.""" workflows = ( @@ -2103,3 +2138,35 @@ def test_scorecard_medium_plus_governance_has_owner_and_runbook() -> None: assert "latest head commit" in runbook assert "cancel superseded runs" in runbook assert "Every central workflow failure must print the actionable reason" in runbook + +def test_runtime_quality_reenters_when_draft_becomes_ready() -> None: + """A same-head Ready transition must create fresh Runtime Quality evidence.""" + workflow = workflow_text("agent-review-runtime-quality-ci.yml") + trigger = workflow.split("\nconcurrency:", 1)[0] + + assert "types: [opened, synchronize, reopened, ready_for_review]" in trigger + + +def test_runtime_quality_executes_sandbox_evidence_changes() -> None: + """Sandbox evidence changes must receive non-vacuous Runtime Quality checks.""" + workflow = workflow_text("agent-review-runtime-quality-ci.yml") + trigger = workflow.split("\nconcurrency:", 1)[0] + selector = workflow_step(workflow, "Select affected contract suites") + + assert "\njobs:" not in trigger + assert '- "scripts/ci/sandboxed_verify.py"' in trigger + assert '- "tests/test_sandboxed_verify.py"' in trigger + assert '- "tests/test_required_workflow_queue_contract.py"' in trigger + assert "sandbox_suite=false" in selector + assert "sandbox_suite=true" in selector + assert 'echo "sandbox=$sandbox_suite"' in selector + + sandbox_step = workflow_step(workflow, "Verify sandbox evidence contracts") + assert "steps.affected_suites.outputs.sandbox == 'true'" in sandbox_step + assert "tests/test_sandboxed_verify.py" in sandbox_step + assert "--include=scripts/ci/sandboxed_verify.py" in sandbox_step + assert "--fail-under=100" in sandbox_step + assert "python -m interrogate --fail-under 100" in sandbox_step + + queue_step = workflow_step(workflow, "Verify queue ownership contract") + assert "tests/test_required_workflow_queue_contract.py" in queue_step diff --git a/tests/test_sandboxed_verify.py b/tests/test_sandboxed_verify.py index d89b9bb956..489b32442c 100644 --- a/tests/test_sandboxed_verify.py +++ b/tests/test_sandboxed_verify.py @@ -1,7 +1,9 @@ +import hashlib import json import runpy import shutil import sys +import threading from pathlib import Path import pytest @@ -190,7 +192,9 @@ def test_copy_workspace_rejects_absolute_symlink_escaping_sandbox_root(tmp_path) sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", []) -def test_copy_workspace_rejects_relative_symlink_escaping_via_parent_traversal(tmp_path): +def test_copy_workspace_rejects_relative_symlink_escaping_via_parent_traversal( + tmp_path, +): """A relative, ``..``-laden symlink target that exits the copied tree is also rejected.""" outside = tmp_path / "outside-secret.txt" outside.write_text("host-only-content", encoding="utf-8") @@ -263,7 +267,9 @@ def test_copy_workspace_rejects_unresolvable_symlink_cycle(tmp_path): sandboxed_verify.copy_workspace(repo, tmp_path / "sandbox", []) -def test_copy_workspace_accepts_the_same_symlink_referenced_twice_non_recursively(tmp_path): +def test_copy_workspace_accepts_the_same_symlink_referenced_twice_non_recursively( + tmp_path, +): """A symlink resolved twice in one chain, not as part of a loop, is accepted. ``link -> shared/../shared/file.txt`` references ``shared`` twice, but @@ -339,7 +345,9 @@ def test_copy_workspace_keeps_symlink_dangling_from_a_missing_internal_target(tm assert not (copied / "dangling.txt").exists() -def test_copy_workspace_accepts_internal_symlink_when_sandbox_root_is_reached_via_symlinked_ancestor(tmp_path): +def test_copy_workspace_accepts_internal_symlink_when_sandbox_root_is_reached_via_symlinked_ancestor( + tmp_path, +): """A benign internal symlink is accepted even when an *ancestor* of the sandbox root is itself reached through a symlink (for example a symlinked default temp directory, unrelated to anything the copied repository controls). @@ -369,7 +377,9 @@ def test_copy_workspace_accepts_internal_symlink_when_sandbox_root_is_reached_vi assert (copied / "link.txt").read_text(encoding="utf-8") == "payload" -def test_copy_workspace_still_rejects_escape_when_sandbox_root_is_reached_via_symlinked_ancestor(tmp_path): +def test_copy_workspace_still_rejects_escape_when_sandbox_root_is_reached_via_symlinked_ancestor( + tmp_path, +): """A genuinely escaping symlink is still rejected when the sandbox root is itself reached through a symlinked ancestor -- walking from the resolved root (this fix) must not weaken the escape check itself. @@ -456,6 +466,48 @@ def test_timeout_output_text_normalizes_subprocess_payloads(): assert sandboxed_verify.timeout_output_text("text-output") == "text-output" +def test_forward_bytes_flushes_text_before_binary_output(): + """Buffered wrapper diagnostics must precede forwarded command bytes.""" + events = [] + + class BinaryStream: + def write(self, output): + events.append(("binary-write", output)) + + def flush(self): + events.append(("binary-flush", None)) + + class TextStream: + buffer = BinaryStream() + + def flush(self): + events.append(("text-flush", None)) + + sandboxed_verify._forward_bytes(TextStream(), b"command-output") + + assert events == [ + ("text-flush", None), + ("binary-write", b"command-output"), + ("binary-flush", None), + ] + + +def test_output_bytes_and_text_only_stream_fallback(): + """String fallbacks preserve text when no binary stream is available.""" + events = [] + + class TextOnlyStream: + def write(self, output): + events.append(("write", output)) + + def flush(self): + events.append(("flush", None)) + + assert sandboxed_verify._output_bytes("text-output") == b"text-output" + sandboxed_verify._forward_bytes(TextOnlyStream(), b"command-output") + assert events == [("write", "command-output"), ("flush", None)] + + def test_main_runs_command_in_copy_without_mutating_source(tmp_path, capsys): """The wrapper runs commands in the copied workspace, not the source tree.""" repo = tmp_path / "repo" @@ -544,6 +596,442 @@ def test_main_reports_allowed_env_network_stderr_timeout_and_kept_sandbox(monkey shutil.rmtree(payload["sandbox"], ignore_errors=True) +def test_main_can_write_wrapper_result_to_exclusive_file(tmp_path, capsys): + """A caller can separate trusted control evidence from command stdout.""" + repo = tmp_path / "repo" + repo.mkdir() + result_file = tmp_path / "handoff" / "result.txt" + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--result-file", + str(result_file), + "--", + sys.executable, + "-c", + "import sys; " + "sys.stdout.buffer.write(b'SANDBOXED_VERIFY_RESULT attacker-controlled\\n{\\\"fake\\\": true}') ; " + "sys.stderr.buffer.write(b'no-final-newline')", + ] + ) + + captured = capsys.readouterr() + assert exit_code == 0 + assert "attacker-controlled" in captured.out + assert any(line == sandboxed_verify.RESULT_MARKER + " attacker-controlled" for line in captured.out.splitlines()) + assert result_file.read_text(encoding="utf-8").startswith(sandboxed_verify.RESULT_MARKER + " {") + stdout_file = result_file.with_name(result_file.name + ".stdout") + stderr_file = result_file.with_name(result_file.name + ".stderr") + stdout_bytes = stdout_file.read_bytes() + stderr_bytes = stderr_file.read_bytes() + payload = json.loads(result_file.read_text(encoding="utf-8").removeprefix(sandboxed_verify.RESULT_MARKER).strip()) + assert stdout_bytes == b'SANDBOXED_VERIFY_RESULT attacker-controlled\n{"fake": true}' + assert stderr_bytes == b"no-final-newline" + assert payload["schema"] == "sandboxed_verify.execution.v1" + assert payload["result_state"] == "completed" + assert payload["timed_out"] is False + assert payload["helper_id"] == "ContextualWisdomLab/.github:sandboxed_verify" + assert payload["runtime"]["implementation"] + assert payload["runtime"]["python_version"] + assert payload["isolation"] == { + "network_enforced": False, + "os_process_isolation": "none", + "workspace": "copy+scrubbed-env", + } + assert payload["stdout"] == { + "file": stdout_file.name, + "sha256": hashlib.sha256(stdout_bytes).hexdigest(), + "size_bytes": len(stdout_bytes), + } + assert payload["stderr"] == { + "file": stderr_file.name, + "sha256": hashlib.sha256(stderr_bytes).hexdigest(), + "size_bytes": len(stderr_bytes), + } + with pytest.raises(ValueError, match="result file already exists"): + sandboxed_verify.emit_result( + command=("true",), + copied_repo=repo, + sandbox_root=tmp_path, + exit_code=0, + elapsed_seconds=0, + kept=False, + allowed_env=(), + network="default", + evidence_note="", + result_file=result_file, + ) + + +def test_result_envelope_is_not_visible_before_streams_are_complete(monkeypatch, tmp_path): + """The envelope path becomes visible only after both streams are complete.""" + result_file = tmp_path / "evidence" / "result.json" + first_stream_started = threading.Event() + release_first_stream = threading.Event() + envelope_write_started = threading.Event() + release_envelope_write = threading.Event() + original_write_all = sandboxed_verify._write_all + + def pause_publication(file_descriptor, content): + if content == b"stdout": + first_stream_started.set() + assert release_first_stream.wait(timeout=2) + elif content == b"envelope": + envelope_write_started.set() + assert release_envelope_write.wait(timeout=2) + original_write_all(file_descriptor, content) + + monkeypatch.setattr(sandboxed_verify, "_write_all", pause_publication) + writer = threading.Thread( + target=sandboxed_verify._write_result_bundle, + args=(result_file, b"envelope", b"stdout", b"stderr"), + ) + writer.start() + assert first_stream_started.wait(timeout=2) + try: + assert not result_file.exists() + finally: + release_first_stream.set() + + assert envelope_write_started.wait(timeout=2) + try: + assert not result_file.exists() + finally: + release_envelope_write.set() + writer.join(timeout=2) + + assert not writer.is_alive() + assert result_file.read_bytes() == b"envelope" + assert result_file.with_name(result_file.name + ".stdout").read_bytes() == b"stdout" + assert result_file.with_name(result_file.name + ".stderr").read_bytes() == b"stderr" + + +def test_result_parent_relative_and_component_failures(monkeypatch, tmp_path): + """Relative traversal handles dot, parent, and creation-race branches.""" + + class Parent: + def __init__(self, *parts): + self.parts = parts + + def is_absolute(self): + return False + + def __str__(self): + return "/".join(self.parts) + + directory_fd = sandboxed_verify._open_result_parent(Parent(".")) + sandboxed_verify.os.close(directory_fd) + with pytest.raises(ValueError, match="parent is not a regular directory"): + sandboxed_verify._open_result_parent(Parent("..")) + + monkeypatch.chdir(tmp_path) + original_mkdir = sandboxed_verify.os.mkdir + + def racing_mkdir(path, mode=0o777, *, dir_fd=None): + original_mkdir(path, mode=mode, dir_fd=dir_fd) + raise FileExistsError + + monkeypatch.setattr(sandboxed_verify.os, "mkdir", racing_mkdir) + directory_fd = sandboxed_verify._open_result_parent(Path("raced")) + sandboxed_verify.os.close(directory_fd) + + original_open = sandboxed_verify.os.open + open_attempts = 0 + + def failing_retry(path, flags, mode=0o777, *, dir_fd=None): + nonlocal open_attempts + if path == "denied": + open_attempts += 1 + if open_attempts == 1: + raise FileNotFoundError + raise OSError("denied") + return original_open(path, flags, mode, dir_fd=dir_fd) + + monkeypatch.setattr(sandboxed_verify.os, "mkdir", original_mkdir) + monkeypatch.setattr(sandboxed_verify.os, "open", failing_retry) + with pytest.raises(ValueError, match="parent is not a regular directory"): + sandboxed_verify._open_result_parent(Path("denied")) + + +def test_result_bundle_rejects_existing_stream(tmp_path): + """A pre-existing stream prevents publication and remains untouched.""" + result_file = tmp_path / "result.json" + stdout_file = result_file.with_name(result_file.name + ".stdout") + stdout_file.write_bytes(b"occupied") + + with pytest.raises(ValueError, match="result bundle file already exists"): + sandboxed_verify._write_result_bundle( + result_file, + b"envelope", + b"stdout", + b"stderr", + ) + + assert stdout_file.read_bytes() == b"occupied" + assert not result_file.exists() + + +def test_result_bundle_cleans_raced_publication(monkeypatch, tmp_path): + """A final-name race removes only this writer's private bundle files.""" + result_file = tmp_path / "result.json" + original_unlink = sandboxed_verify.os.unlink + + def reject_publication(*_args, **_kwargs): + raise FileExistsError + + def unlink_then_report_missing(path, *, dir_fd=None): + original_unlink(path, dir_fd=dir_fd) + raise FileNotFoundError + + monkeypatch.setattr(sandboxed_verify.os, "link", reject_publication) + monkeypatch.setattr(sandboxed_verify.os, "unlink", unlink_then_report_missing) + with pytest.raises(ValueError, match="result file already exists"): + sandboxed_verify._write_result_bundle( + result_file, + b"envelope", + b"stdout", + b"stderr", + ) + + assert list(tmp_path.iterdir()) == [] + + +def test_result_file_rejects_symlinked_parent(tmp_path): + """The trusted handoff must not follow a caller-controlled parent symlink.""" + target = tmp_path / "target" + target.mkdir() + link = tmp_path / "link" + link.symlink_to(target, target_is_directory=True) + + with pytest.raises(ValueError, match="parent is not a regular directory"): + sandboxed_verify.emit_result( + command=("true",), + copied_repo=tmp_path, + sandbox_root=tmp_path, + exit_code=0, + elapsed_seconds=0, + kept=False, + allowed_env=(), + network="default", + evidence_note="", + result_file=link / "result.json", + ) + + +def test_result_file_rejects_existing_symlink_ancestor(tmp_path): + """An existing nested directory must not hide a symlink ancestor.""" + target = tmp_path / "target" + nested = target / "nested" + nested.mkdir(parents=True) + link = tmp_path / "link" + link.symlink_to(target, target_is_directory=True) + + with pytest.raises(ValueError, match="parent is not a regular directory"): + sandboxed_verify.emit_result( + command=("true",), + copied_repo=tmp_path, + sandbox_root=tmp_path, + exit_code=0, + elapsed_seconds=0, + kept=False, + allowed_env=(), + network="default", + evidence_note="", + result_file=link / "nested" / "result.json", + ) + + assert not (nested / "result.json").exists() + + +def test_result_bundle_preserves_large_binary_streams(tmp_path, capfdbinary): + """Dedicated handoff files preserve large invalid UTF-8 output exactly.""" + repo = tmp_path / "repo" + repo.mkdir() + result_file = tmp_path / "evidence" / "result.json" + stdout_bytes = (b"\xff\x00marker\n" * 131_072) + b"tail" + stderr_bytes = b"\xfejson:{not-json}\r\nend" + command = ( + "import sys; " + "sys.stdout.buffer.write((b'\\xff\\x00marker\\n' * 131072) + b'tail'); " + "sys.stderr.buffer.write(b'\\xfejson:{not-json}\\r\\nend')" + ) + + assert ( + sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--result-file", + str(result_file), + "--", + sys.executable, + "-c", + command, + ] + ) + == 0 + ) + capfdbinary.readouterr() + + assert result_file.with_name(result_file.name + ".stdout").read_bytes() == stdout_bytes + assert result_file.with_name(result_file.name + ".stderr").read_bytes() == stderr_bytes + + +def test_result_file_distinguishes_timeout_from_exit_124(tmp_path, capsys): + """A real timeout and a command exit 124 have different result states.""" + repo = tmp_path / "repo" + repo.mkdir() + timeout_result = tmp_path / "timeout" / "result.json" + exit_result = tmp_path / "exit" / "result.json" + + assert ( + sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--timeout", + "1", + "--result-file", + str(timeout_result), + "--", + sys.executable, + "-c", + "import time; print('partial', flush=True); time.sleep(2)", + ] + ) + == 124 + ) + assert ( + sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--result-file", + str(exit_result), + "--", + sys.executable, + "-c", + "raise SystemExit(124)", + ] + ) + == 124 + ) + capsys.readouterr() + + timeout_payload = json.loads( + timeout_result.read_text(encoding="utf-8").removeprefix(sandboxed_verify.RESULT_MARKER).strip() + ) + exit_payload = json.loads( + exit_result.read_text(encoding="utf-8").removeprefix(sandboxed_verify.RESULT_MARKER).strip() + ) + assert timeout_payload["result_state"] == "timed_out" + assert timeout_payload["timed_out"] is True + assert exit_payload["result_state"] == "completed" + assert exit_payload["timed_out"] is False + assert timeout_result.with_name(timeout_result.name + ".stdout").read_bytes() == b"partial\n" + + +def test_result_file_failure_is_bounded_and_always_cleans_sandbox(monkeypatch, tmp_path, capsys): + """A successful command with a handoff collision returns 125 and cleans up.""" + repo = tmp_path / "repo" + repo.mkdir() + sandbox = tmp_path / "sandbox" + result_file = tmp_path / "result.json" + result_file.write_text("occupied", encoding="utf-8") + + def make_sandbox(*, prefix): + assert prefix == "sandboxed-verify-" + sandbox.mkdir() + return str(sandbox) + + monkeypatch.setattr(sandboxed_verify.tempfile, "mkdtemp", make_sandbox) + + assert ( + sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--result-file", + str(result_file), + "--", + "true", + ] + ) + == 125 + ) + captured = capsys.readouterr() + assert "Traceback" not in captured.err + assert "result file already exists" in captured.err + assert not sandbox.exists() + + +@pytest.mark.parametrize( + ("command", "expected_exit_code"), + [ + ((sys.executable, "-c", "raise SystemExit(2)"), 2), + ((sys.executable, "-c", "raise SystemExit(124)"), 124), + ], +) +def test_result_file_failure_preserves_command_failure( + command, expected_exit_code, tmp_path, capsys +): + """Evidence rejection must not mask the command's nonzero exit status.""" + repo = tmp_path / "repo" + repo.mkdir() + result_file = tmp_path / "result.json" + result_file.write_text("occupied", encoding="utf-8") + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--result-file", + str(result_file), + "--", + *command, + ] + ) + + captured = capsys.readouterr() + assert exit_code == expected_exit_code + assert "result file already exists" in captured.err + + +def test_result_file_failure_preserves_timeout_status(monkeypatch, tmp_path, capsys): + """Evidence rejection must not mask the wrapper's timeout status.""" + repo = tmp_path / "repo" + repo.mkdir() + result_file = tmp_path / "result.json" + result_file.write_text("occupied", encoding="utf-8") + + def time_out(*_args, **_kwargs): + raise sandboxed_verify.subprocess.TimeoutExpired( + cmd=("slow-command",), timeout=1, output=b"partial-out", stderr=b"partial-err" + ) + + monkeypatch.setattr(sandboxed_verify, "run_command", time_out) + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--timeout", + "1", + "--result-file", + str(result_file), + "--", + "slow-command", + ] + ) + + captured = capsys.readouterr() + assert exit_code == 124 + assert "command timed out after 1s" in captured.err + assert "result file already exists" in captured.err + + def test_main_reports_a_clean_failure_when_the_workspace_copy_is_rejected(tmp_path, capsys): """A symlink-escape rejection from ``copy_workspace`` must not surface as an uncaught traceback. @@ -561,9 +1049,7 @@ def test_main_reports_a_clean_failure_when_the_workspace_copy_is_rejected(tmp_pa repo.mkdir() (repo / "escape-link").symlink_to(outside) - exit_code = sandboxed_verify.main( - ["--repo-root", str(repo), "--", "true"] - ) + exit_code = sandboxed_verify.main(["--repo-root", str(repo), "--", "true"]) captured = capsys.readouterr() assert exit_code == 125 @@ -575,6 +1061,36 @@ def test_main_reports_a_clean_failure_when_the_workspace_copy_is_rejected(tmp_pa assert payload["exit_code"] == 125 +def test_copy_rejection_is_recorded_in_trusted_result_bundle(tmp_path): + """A rejected source tree must still produce explicit trusted evidence.""" + outside = tmp_path / "outside-secret.txt" + outside.write_text("host-only-content", encoding="utf-8") + repo = tmp_path / "repo" + repo.mkdir() + (repo / "escape-link").symlink_to(outside) + result_file = tmp_path / "evidence" / "result.json" + + exit_code = sandboxed_verify.main( + [ + "--repo-root", + str(repo), + "--result-file", + str(result_file), + "--", + "true", + ] + ) + + assert exit_code == 125 + wrapper_result = json.loads( + result_file.read_text(encoding="utf-8").removeprefix(sandboxed_verify.RESULT_MARKER).strip() + ) + assert wrapper_result["result_state"] == "copy_rejected" + assert wrapper_result["timed_out"] is False + assert result_file.with_name(result_file.name + ".stdout").read_bytes() == b"" + assert result_file.with_name(result_file.name + ".stderr").read_bytes() == b"" + + def test_parse_args_rejects_invalid_inputs(): """The CLI rejects invocations without a command or with invalid options.""" with pytest.raises(SystemExit): @@ -589,7 +1105,19 @@ def test_module_main_entrypoint(monkeypatch, tmp_path): """The script entrypoint exits with the verification command status.""" repo = tmp_path / "repo" repo.mkdir() - monkeypatch.setattr(sys, "argv", ["sandboxed_verify.py", "--repo-root", str(repo), "--", sys.executable, "-c", "raise SystemExit(0)"]) + monkeypatch.setattr( + sys, + "argv", + [ + "sandboxed_verify.py", + "--repo-root", + str(repo), + "--", + sys.executable, + "-c", + "raise SystemExit(0)", + ], + ) module = sys.modules.pop("scripts.ci.sandboxed_verify", None) with pytest.raises(SystemExit) as exc_info: try: