diff --git a/CHANGELOG.md b/CHANGELOG.md index afc70f70..eed7310b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ # Changelog ## Unreleased +- `git ls-tree -r -l -z --full-tree` exact-tree metadata 검증을 Git의 실제 long-output grammar에 맞춰 강화. mode·object type·object identity의 구분자는 literal ASCII space로 고정하고, blob size는 ASCII decimal만 허용한다. Git이 `%(objectsize:padded)`를 최소 폭 7로 right-justify한다는 공식 계약에 따라 정확한 선행 ASCII padding과 deterministic fixture용 unpadded form만 통과시키며, 임의 under/over-padding·nonbreaking space·Arabic-Indic digit·trailing text는 실패-폐쇄한다. 모든 repeated ASCII space가 악성이라는 초기 가정은 실제 `-l` 출력과 충돌하므로 부분적으로 잘못된 피드백으로 분류하고, valid Unicode-normalization finding만 보존해 RED 회귀 테스트를 GREEN으로 전환했다. public/doctoring 문서와 APA 7th Git 근거를 함께 갱신했다. +- patch file-mode metadata 검증을 suffix 검사에서 full-line grammar로 강화. `old mode`·`new mode`·`new file mode`·`deleted file mode`는 정확히 하나의 6자리 mode token만 허용하고, `100644`·`100755`만 통과시킨다. exact `120000`·`160000`은 symlink/gitlink로 계속 명시적으로 차단하며, `new file mode 120000 100644`처럼 허용 suffix 뒤에 악성 mode를 숨긴 trailing-token 입력은 `malformed mode metadata`로 실패-폐쇄한다. 기존 RED 회귀 테스트를 GREEN으로 전환하고 public/doctoring 문서에 진단 분리와 보안 결정을 기록했다. +- untrusted patch를 exact repository/base/head/patch SHA-256와 allowlisted validation profile에 결합해 credential-free, no-network, read-only, non-root Docker sandbox에서 검증하는 reviewer 경계를 추가. text-only preflight가 malformed UTF-8·binary payload·symlink/gitlink mode(새/삭제/변경 mode뿐 아니라 기존 entry의 `index … 120000|160000`)·traversal·absolute/control-character/raw-backslash path·중복/과다 변경 파일·GitHub governance 경로를 Docker 실행 전에 실패-폐쇄한다. unified hunk header와 old/new line count를 정확히 소진해 다중 hunk·context·zero-count·`No newline` marker를 지원하면서 truncated/overlong hunk와 hunk 뒤 전통 diff section을 거부하고, `---`·`+++`·rename/copy source/target은 counted primary `diff --git` path identity와 일치해야 한다. Git source는 caller `.git`의 config/index/hooks/attributes를 직접 신뢰하지 않고 descriptor-safe gitfile·commondir·object-store resolution과 private bare control metadata를 사용하며, highest-precedence `* -export-ignore -export-subst`로 committed/local archive transforms를 제거해 failing test 누락과 blob substitution을 방지한다. exact `read-tree`·isolated status·bounded raw-tree archive·member allowlist·post-extraction manifest equality를 강제하고, Docker에는 writable host directory 대신 pre-created `/output/result.json` 한 파일만 전달한다. process-wide `RLIMIT_FSIZE`는 현실적인 검증 artifact를 허용하는 64 MiB로 제한하고 host result parser는 evidence를 독립적으로 16 KiB에 제한한다. immutable digest-pinned image·capability drop·seccomp·resource quotas·bounded timeout cleanup·exact structured result 재검증을 유지하고, beginner-readable 운영 문서와 Git 2.54/2.55·NIST SP 800-190·NIST SP 800-218·OCI Runtime Specification 1.3.0·SLSA 1.2 근거를 APA 7th doctoring에 기록했다. reviewer production statement/branch/docstring 100% gate와 committed/local attributes·linked worktree·descriptor race·archive/extraction·hunk/path/mode·single-result-file 악성 회귀 테스트를 유지한다. - `hourly-product-development`가 `NVIDIA_NIM_API_KEY`뿐 아니라 `NOEMA_MAINTAINER_APP_CLIENT_ID`와 `NOEMA_MAINTAINER_APP_PRIVATE_KEY` 존재를 checkout·OpenCode 설치·NVIDIA 호출 전에 검증한다. 게시 경로가 준비되지 않았으면 `maintainer_app_unavailable`로 실패 폐쇄하여 알려진 실패에 추론 비용을 쓰지 않으며, `dry_run`은 credential 없이 queue와 task contract를 검토하는 경로로 유지한다. 기존 reviewer App 및 `NOEMA_LLM_API_KEY`·`contextual-orchestrator` reviewer credential 경계는 변경하지 않는다. - zero open pull requests일 때만 `NVIDIA_NIM_API_KEY` 전용 OpenCode 1.17.13 세션을 실행하는 proposal-only `hourly-product-development` 루프를 추가. minute-47 schedule·non-cancelling single flight·OpenCode binary SHA-256 pin·NVIDIA NIM model fallback·후보 실패 시 clean reset·GitHub/OIDC credential 제거·reviewer key 비참조·full release verification·40-file/500,000-byte proposal budget·trusted one-PR packaging을 강제한다. 각 후보 실행은 900초와 30초 kill grace로 제한하고, 실패 후 `npm ci --ignore-scripts` 재설치는 별도 60초와 10초 kill grace로 제한한다. 재설치가 실패하거나 시간 초과되면 불완전한 dependency tree로 다음 후보를 실행하지 않고 실패 폐쇄한다. 세 후보의 실행·종료 2,790초, 두 번의 후보 간 재설치 140초, 300초 setup/diagnostic reserve를 합친 3,230초가 55분(3,300초) job budget에 들어가며 70초 여유를 남긴다. 마지막 후보가 실패하면 불필요한 reset·clean·재설치를 생략하고 안정적인 전체 후보 실패 진단으로 곧바로 종료한다. 모델 실행, 제안 코드 검증, publication credential을 각각 별도의 GitHub-hosted runner로 분리하고, immutable artifact의 exact ID·workflow-run ID·archive digest와 patch SHA-256·base SHA·file/byte count를 교차 검증하며 symlink(`120000`)와 gitlink(`160000`)를 세 경계 모두에서 차단한다. 제안 코드를 실행한 runner에는 Maintainer App secret/token을 절대 제공하지 않고, 세 번째 non-executing publisher에서만 late-bound repository-scoped App token을 발급한다. merge/release/deploy authority는 기존 `hourly-commercial-readiness` exact-head governance에 유지하며, 운영 Runbook과 OpenCode/NVIDIA/GitHub Actions/NIST SP 800-218 근거를 APA 7th doctoring에 기록했다. package version은 release·deployment·production KPI evidence를 발행하지 않으므로 유지한다. - `/health` liveness와 분리된 unauthenticated `GET`/`HEAD /ready` runtime readiness endpoint를 추가. GitHub Actions OIDC issuer·audience·organization/workflow binding·exact workflow ref·GitHub Cloud API origin·GitHub App identifiers·PKCS#8 private key를 외부 호출 없이 검증하며, 불완전한 설정은 secret/config value를 반사하지 않는 deterministic failure codes와 `503 ERR_SERVICE_NOT_READY`, `Retry-After`, no-store/nosniff/trace/latency headers로 실패-폐쇄한다. exact workflow named ref는 Git `check-ref-format`의 모호성·유효성 경계(`..`, `//`, dot-leading/`.lock` component, revision-expression 문자, trailing dot/slash 등)를 만족해야 하므로 GitHub가 실제로 표현할 수 없는 ref에서 false-ready가 발생하지 않는다. 배포 smoke contract가 liveness·runtime readiness·unauthenticated exchange challenge를 모두 요구하도록 확장하고 Kubernetes probe separation, RFC 9110, NIST SSDF, Git ref-format 근거를 APA 7th doctoring에 기록했다. diff --git a/docs/doctoring/quarantined-patch-validation.md b/docs/doctoring/quarantined-patch-validation.md new file mode 100644 index 00000000..348100e3 --- /dev/null +++ b/docs/doctoring/quarantined-patch-validation.md @@ -0,0 +1,191 @@ +# Quarantined patch-validation boundary + +## Decision + +Noema validates generated or externally supplied text patches only inside a credential-free, no-network container boundary. The runner accepts one exact repository/base/head/patch-digest tuple and one allowlisted profile, executes only the profile baked into an immutable image, and accepts one bounded structured result that repeats the request identity. + +A passed sandbox result is evidence for one authenticated Git revision, patch, image, and validation profile. It is not merge approval, model judgement, release provenance, deployment evidence, or a substitute for independent review. + +The current implementation requires verifiable Git metadata and a reachable content-addressed object database. A plain directory is not accepted as exact-revision evidence. + +## Threat model + +Patch content, repository source, Git control metadata, repository scripts, status output, exact-tree output, archive metadata, extracted filesystem objects, and container output are hostile. The boundary addresses: + +- malformed, binary, oversized, symlink, gitlink, traversal, absolute, control-character, backslash, aliasing, duplicate, or governance-path patch input; +- conflicting or incomplete `---`/`+++`, rename, copy, mode, index, similarity, and hunk metadata; +- patch-path replacement and descriptor races; +- tracked, staged, untracked, or ignored worktree drift; +- unbounded Git status or exact-tree stdout before semantic limits are applied; +- caller-worktree mutation after exact-head preflight; +- committed or local `export-ignore` and `export-subst` archive transforms; +- checkout-local configuration, hooks, indexes, remotes, linked-worktree records, common-directory records, and object-store substitution; +- special Git tree modes, malformed or Unicode-normalized `ls-tree` metadata, noncanonical field separators or object-size padding, excessive tree members, oversized paths, oversized blobs, and aggregate source expansion before archive allocation; +- tar links, devices, FIFOs, unsafe names, duplicate aliases, file-directory collisions, leaf gitlink-like directories, and extraction-size exhaustion; +- extraction-time or post-extraction substitution; +- checkout tokens, credential-bearing remotes, object storage, reflogs, and worktree pointers entering the container; +- container network, privilege, process, memory, CPU, descriptor, file-size, tmpfs, IPC, and wall-time abuse; +- writable host-directory abuse, stdout/stderr evidence smuggling, oversized result output, and identity-confused evidence; and +- accidental equivalence among validation evidence, review approval, and release authority. + +The boundary does not claim protection against a compromised host kernel, container runtime, trusted Git executable, validator image, image registry, workflow source, content-addressed object database, or privileged trusted caller. Those remain separate controls. + +## Fail-closed controls + +### Exact request and result binding + +The request binds repository full name, exact base SHA, exact head SHA, patch SHA-256, and an enumerated validation profile. The result must repeat those fields and the command baked into the profile. Unknown fields, malformed values, excessive values, a `passed` status with nonzero exit code, or any identity mismatch are rejected. + +The base SHA is an evidence binding. The current runner does not fetch or reconstruct the base commit and does not independently prove the base-to-head relationship. + +### Descriptor-safe Git control resolution + +A direct `git status` or `git archive` against caller-controlled `.git` state is not a sufficient trust boundary. Noema reads only the standard repository or linked-worktree control records through bounded, no-follow descriptors with strict UTF-8, single-line syntax, and device/inode stability. Symlinks, special objects, unsafe paths, malformed gitfiles, inaccessible common directories, and unavailable object stores fail closed. + +The runner creates private owner-only bare Git control metadata containing: + +- minimal bare-repository configuration; +- `HEAD` bound to the requested exact commit; +- `objects/info/alternates` bound to the resolved content-addressed object store; and +- highest-precedence `info/attributes` containing `* -export-ignore -export-subst`. + +System and global Git configuration, system attributes, hooks, fsmonitor, optional locks, and the untracked cache are disabled. Source-local configuration, remotes, indexes, hooks, and attributes do not become policy inputs. + +Using this isolated control directory, the trusted host runs `read-tree ` and a non-shell porcelain-v2 status comparison. Status stdout is consumed as a binary stream under one 30-second deadline. The host reads at most one byte: an observed byte proves drift, triggers immediate bounded child termination, and blocks validation. An empty stream is admissible only when the child exits zero. Launch, read, wait, timeout, termination, or nonzero-exit failure is never treated as clean. This design makes dirty-worktree evidence constant-space rather than proportional to the number or length of untracked and ignored paths. + +### Exact-tree preflight before archive allocation + +A clean worktree is insufficient because the source tree itself may be structurally unsupported or too large to serialize safely. Before `git archive`, the isolated control directory runs the equivalent of: + +```text +git ls-tree -r -l -z --full-tree +``` + +The NUL-delimited binary output is parsed incrementally under the same 30-second process deadline. The trusted host reads bounded chunks and retains at most one partial record. Every complete record must contain exactly one canonical repository-relative path and a long-format metadata prefix composed of a `100644` or `100755` mode, the literal object type `blob`, a valid SHA-1 or SHA-256 object identifier, and an ASCII-decimal byte size. + +Git documents the `-l` output as `%(objectmode) %(objecttype) %(objectname) %(objectsize:padded)%x09%(path)` and right-justifies the object-size field to a minimum width of seven. The three field separators before the size field therefore remain exactly one literal ASCII space, while a short size legitimately carries leading ASCII padding inside its own field. The parser also accepts an unpadded size for deterministic internal fixtures; no other padding width is accepted. + +The preflight rejects more than 20,000 records, paths above 4 KiB, records above the path ceiling plus fixed metadata allowance, aggregate tree metadata above 16 MiB, a blob above 64 MiB, aggregate blob bytes above 512 MiB, special or unsupported modes, tree or gitlink records, empty fixed fields, Unicode whitespace separators, undocumented leading-space counts, non-ASCII or nondecimal sizes, malformed, non-UTF-8, empty, or truncated output, duplicate paths, `.git` content, aliases, traversal, absolute paths, backslashes, and control characters. Git launch, read, wait, timeout, termination, decoding, or nonzero-exit failure fails closed. + +Project decision: exact-tree metadata is parsed as Git protocol syntax rather than normalized human text. Python's `str.split()` without an explicit separator collapses whitespace and recognizes Unicode separators, while `str.isdecimal()` accepts decimal characters outside ASCII. A first attempted hardening rejected every repeated ASCII space and consequently rejected Git's own documented padded `-l` output. That finding was partially valid—Unicode normalization and arbitrary padding were fail-open risks—but the blanket repeated-space premise was incorrect for this protocol. + +Noema now splits on the literal ASCII space at most three times. That preserves the complete size field while making mode, object type, and object identity separators exact. It strips only leading ASCII spaces from the size field, requires the remaining token to satisfy both `isascii()` and `isdecimal()`, and accepts either zero padding or exactly `max(0, 7 - len(size))` leading spaces. Empty fixed fields, nonbreaking-space separators, Arabic-Indic digits, trailing characters, under-padding, and over-padding fail closed. Canonical mode, object type, and object-identity failures retain the non-regular-object diagnostic; malformed structure and noncanonical padding use the malformed-metadata diagnostic; invalid numeric text retains the invalid-blob-size diagnostic. This preserves interoperability with Git's real output while keeping the accepted language explicit and test-bound. + +The child is terminated on the first violated path, record, member-count, metadata-byte, per-file, or aggregate-file limit. This order matters: archive member validation alone occurs after storage has already been allocated and written, and capture-based subprocess APIs can allocate the entire hostile output before semantic checks run. Incremental exact-tree preflight therefore bounds source cardinality, serialized metadata, retained memory, and blob bytes before archive serialization while proving that the committed tree contains only materializable regular blobs. + +### Isolated archive and extraction equality + +After exact-tree preflight, the isolated control directory performs: + +```text +git archive --format=tar --output= +``` + +Git documents that `git archive` honors `export-ignore` and `export-subst` and that `$GIT_DIR/info/attributes` has the highest precedence. The private attributes explicitly unset both transforms, preventing a committed or local attribute from omitting a failing test or rewriting committed blob bytes. + +The archive is independently hostile. Noema enumerates it before extraction and permits only canonical regular files and populated directories under the same member and byte ceilings. Absolute names, traversal, raw backslashes, control characters, `.git` content, aliases, duplicates, file-directory collisions, children below files, links, devices, FIFOs, other special entries, and empty leaf directories are rejected. + +Only the validated members are extracted into a fresh owner-only directory through Python's `data` filter. An `lstat` walk must exactly match the validated path, type, and regular-file-size manifest. Added, omitted, substituted, linked, special, or resized entries fail closed before Docker starts. + +Python documents extraction filters as mitigations, not complete authentication. Noema adds deterministic tree and archive limits, allowlisting, private extraction, pre/post manifest equality, trusted Git timeouts, and downstream container quotas as defense in depth. + +### Git metadata and credential isolation + +The caller's `.git` object must be a regular directory or regular linked-worktree file. A missing, symlinked, or special object is rejected. + +The committed snapshot contains no original Git control data. A type-compatible empty `.git` placeholder and nested read-only bind boundary prevent untrusted code from reading checkout credentials, remotes, local configuration, object storage, reflogs, or host worktree pointers. + +### Descriptor-safe patch intake + +The patch is read through no-follow descriptor operations with pre-open and post-open device/inode checks, regular-file enforcement, a 4 MiB ceiling, and exact SHA-256 comparison. The caller-controlled original pathname never enters Docker mount grammar; verified bytes are copied to a private owner-only file. + +The parser validates canonical primary paths and independent file, rename, and copy metadata families. Each family must be complete, exact source and target roles must match the active primary diff identity, duplicates are rejected within a family, rename and copy cannot conflict, and `/dev/null` is permitted only for canonical creation or deletion file headers. + +Project decision: file-mode metadata is parsed with a full-line grammar rather than a suffix predicate. A recognized mode directive must contain exactly one six-digit token, and the parser allowlists only `100644` and `100755`; the existing exact special-mode gate rejects `120000` and `160000` with a dedicated symlink/gitlink diagnostic. This closes the observed fail-open case where `new file mode 120000 100644` ended in an allowed suffix even though its leading token materialized a symlink. Malformed extra tokens are now distinguished from canonical-but-unsupported modes, preserving actionable diagnostics and exact regression evidence. + +Hunk counts are consumed exactly. Newline markers require immediately preceding valid content and cannot repeat. Extra content after declared counts, path metadata after a hunk, malformed quoting, noncanonical path aliases, and governance targets fail closed before Docker. + +### Container isolation + +The validator requires an immutable repository-scoped image digest and uses `--pull=never`. The container has no network, no Docker socket, a read-only root, read-only source and patch mounts, one pre-created writable result file, a non-root UID/GID, all capabilities dropped, `no-new-privileges`, seccomp, isolated IPC, and bounded PID, CPU, memory, swap, descriptor, process, core, file-size, tmpfs, and wall-time resources. + +The child environment contains only the minimum executable path, result path, and exact validation identity. Repository, reviewer-model, NVIDIA NIM, Cloudflare, OIDC, publication, and deployment credentials are absent. Timeout handling attempts bounded forced cleanup. + +### Single bounded result channel + +The container receives exactly one host file at `/output/result.json`, never a writable host output directory. Normal subprocess stdout and stderr are directed to `DEVNULL`; there is no stdout compatibility fallback in the evidence contract. + +The host independently reads the result through regular-file, no-follow, stable-descriptor, and 16 KiB byte-limit checks. The extra-fields-forbidden schema bounds status, exit code, duration, excerpts, reason-code count, and reason-code syntax. Missing, empty, malformed, oversized, inconsistent, or identity-mismatched evidence fails closed. + +The process-wide 64 MiB `RLIMIT_FSIZE` and the 16 KiB result ceiling protect different resources. The first bounds individual workspace artifacts created by an allowlisted validation tool; the second bounds the only host-writable trusted evidence input. + +## Standards rationale + +NIST SP 800-190 identifies image, registry, orchestrator, host, and workload risks and recommends trusted images, isolation, least privilege, vulnerability management, and resource controls. The immutable image reference, non-root execution, capability drop, no-network policy, read-only mounts, single-file result channel, and quotas align with those recommendations without claiming formal conformance. + +NIST SP 800-218 remains the final SSDF Version 1.1 baseline. NIST SP 800-218 Revision 1, describing SSDF Version 1.2, remains an Initial Public Draft in this decision record. Exact-head binding, deterministic failure evidence, test-first security regressions, and separation of development, review, and release authority operationalize SSDF verification practices. + +OCI Runtime Specification 1.3.0 defines the low-level namespace, mount, resource, capability, and process model used by container runtimes. Docker flags are implementation mechanisms for those controls, not security guarantees by themselves. + +SLSA Version 1.2 adds a Source Track alongside the Build Track. The exact-tree snapshot boundary improves source evidence, but this PR does not claim a SLSA level. Protected history, two-party review, build isolation, provenance, artifact verification, and release acceptance remain separate. + +## Verification contract + +Deterministic tests prove at least: + +- malformed patch encodings, payloads, paths, modes, headers, metadata families, and hunk counts fail closed; +- descriptor swaps, symlink substitutions, short reads, and byte-limit violations fail closed; +- malformed, multiline, unstable, or unavailable Git control records fail closed; +- exact-head mismatch, failed isolated status, and all worktree drift categories block Docker; +- dirty status output is detected after at most one byte and the Git child is terminated without accumulating path output; +- exact-tree stdout is parsed incrementally with shared deadlines, bounded chunks, one-record retention, path and metadata ceilings, and immediate termination on the first violated limit; +- exact-tree record count, modes, object types, object identities, literal ASCII fixed-field separators, unpadded and documented minimum-width-7 size forms, noncanonical padding, ASCII sizes, aggregate bytes, canonical paths, duplicates, process failures, and timeouts fail closed before archive allocation, including nonbreaking-space and Arabic-Indic-digit regressions; +- committed and local archive attributes cannot omit or rewrite exact-tree bytes; +- post-preflight worktree mutation cannot change the snapshot mounted in Docker; +- archive failure, malformed or empty archives, unsafe names, duplicates, links, special entries, leaf directories, member limits, and byte limits fail closed; +- post-extraction path, type, or size substitution fails closed; +- directory and linked-worktree Git metadata are replaced by type-compatible empty boundaries; +- only an immutable trusted image and allowlisted profile are accepted; +- only one bounded result file is host-writable and stdout/stderr are not evidence channels; +- malformed, oversized, inconsistent, or identity-mismatched result evidence fails closed; and +- production statement and branch coverage and public docstring coverage remain 100 percent. + +## Residual risks and next slices + +Before production activation, the repository still requires: + +- independent exact-head approval and every protected required check; +- a reproducible patch-validator image build; +- signature, vulnerability, SBOM, and provenance verification; +- a real no-network smoke test of the digest-pinned patch-validator image; +- integration into reviewer decision flow without conflating evidence and model judgement; +- retained evidence bound to workflow, run, source, image, request, and result; and +- image rotation, incident response, failure recovery, and rollback procedures. + +Until those gates pass, this remains a tested library and evidence contract, not an end-to-end release capability. + +## References + +Git Project. (2026, April 20). *git-archive documentation* (Version 2.54.0). https://git-scm.com/docs/git-archive + +Git Project. (2026, June 29). *gitattributes documentation* (Version 2.55.0). https://git-scm.com/docs/gitattributes + +Git Project. (2025, March 14). *gitrepository-layout documentation* (Version 2.49.0). https://git-scm.com/docs/gitrepository-layout + +Git Project. (n.d.). *git-ls-tree documentation* (Git 2.55.0). Retrieved August 6, 2026, from https://git-scm.com/docs/git-ls-tree + +Open Container Initiative. (2025, November 4). *OCI runtime-spec v1.3.0 release notice*. https://opencontainers.org/release-notices/v1-3-0-runtime-spec/ + +Python Software Foundation. (2026). *Text sequence type—str (Python 3.11.15 documentation).* https://docs.python.org/3.11/library/stdtypes.html#text-sequence-type-str + +Python Software Foundation. (2026). *tarfile—Read and write tar archive files (Python 3.11.15 documentation).* https://docs.python.org/3.11/library/tarfile.html + +SLSA Community. (2025, November 24). *Announcing SLSA v1.2*. The Linux Foundation. https://slsa.dev/blog/2025/11/announce-slsa-v1.2 + +SLSA Community. (2025). *SLSA specification (Version 1.2)*. The Linux Foundation. https://slsa.dev/spec/v1.2/ + +Souppaya, M., Morello, J., & Scarfone, K. (2017). *Application container security guide* (NIST Special Publication 800-190). National Institute of Standards and Technology. https://doi.org/10.6028/NIST.SP.800-190 + +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 + +Booth, H., Ogata, M., Kent, K., Souppaya, M., & Dodson, D. (2025). *Secure software development framework (SSDF) version 1.2: Recommendations for mitigating the risk of software vulnerabilities* (Initial Public Draft, NIST Special Publication 800-218 Revision 1). National Institute of Standards and Technology. https://doi.org/10.6028/NIST.SP.800-218r1.ipd diff --git a/docs/quarantined-patch-validation.md b/docs/quarantined-patch-validation.md new file mode 100644 index 00000000..3f42c357 --- /dev/null +++ b/docs/quarantined-patch-validation.md @@ -0,0 +1,177 @@ +# Quarantined patch validation + +Noema validates an untrusted text patch against one authenticated Git commit without exposing repository write credentials, reviewer-model credentials, NVIDIA NIM credentials, Cloudflare credentials, OIDC tokens, publication credentials, or the Docker socket to the code being tested. + +This boundary produces validation evidence only. It does not approve a pull request, judge a model response, authorize a release, or bypass repository protection. + +## Request contract + +The trusted caller supplies: + +- the repository full name; +- the exact base commit SHA; +- the exact head commit SHA; +- the SHA-256 digest of the patch bytes; and +- one approved validation profile. + +The current approved profile is: + +| Profile | Command inside the validator image | +|---|---| +| `node_release_verify` | `npm run release:verify` | + +Callers cannot provide an arbitrary command. + +The current runner requires a Git checkout with verifiable `.git` metadata. A directory without authenticated Git metadata is rejected; it is not treated as revision-bound source evidence. + +## Exact source identity + +Noema does not trust checkout-local Git configuration, hooks, indexes, remotes, attributes, or worktree-control files as policy inputs. It reads repository or linked-worktree control records through bounded, no-follow descriptor operations and resolves the content-addressed object store. Symlinks, special files, malformed UTF-8, multiline records, unstable descriptors, unsafe paths, and unavailable object directories fail closed. + +The runner creates private bare Git control metadata that: + +- points only to the resolved object store through `objects/info/alternates`; +- binds `HEAD` to the requested `head_sha`; +- disables system and global Git configuration, hooks, fsmonitor, optional locks, and the untracked cache; and +- installs highest-precedence `* -export-ignore -export-subst` attributes. + +Using that private control directory, Noema runs `read-tree` for the exact head and a porcelain-v2 status comparison against the caller worktree. Status output is not accumulated: the trusted host reads at most one byte from a binary pipe. Any byte proves tracked, staged, untracked, or ignored drift, causes immediate bounded child termination, and blocks validation. An empty stream is accepted only when the Git child exits zero within the shared deadline. A failed, timed-out, or malformed Git process is never interpreted as a clean result. + +### Prearchive exact-tree bounds + +Before allocating archive storage, Noema runs a configuration-isolated command equivalent to: + +```text +git ls-tree -r -l -z --full-tree +``` + +The binary stdout stream is parsed incrementally under one 30-second wall deadline. The host retains at most one bounded partial record instead of collecting the full command output. Every NUL-terminated record must describe a `100644` or `100755` blob with a valid SHA-1 or SHA-256 object identity, an ASCII-only decimal byte size, and one canonical repository-relative POSIX path. + +The three separators between mode, object type, object identity, and the long-format size field must be literal ASCII spaces. Git's `-l` format right-justifies object sizes to a minimum width of seven, so the size field may use exactly that documented leading ASCII padding; unpadded records remain accepted for deterministic fixtures. Other leading-space counts, Unicode whitespace, trailing characters, and non-ASCII decimal digits fail closed instead of being normalized by Python's Unicode-aware string helpers. The preflight rejects: + +- trees above 20,000 records; +- paths above 4 KiB and records above the path ceiling plus fixed metadata allowance; +- aggregate exact-tree metadata above 16 MiB; +- blobs above 64 MiB; +- aggregate blob bytes above 512 MiB; +- tree, gitlink, symlink, or other non-regular object modes; +- malformed, non-UTF-8, empty, or truncated records; +- absolute, traversing, aliased, control-character, backslash, duplicate, or `.git` paths; and +- any Git launch, read, exit, termination, or timeout failure. + +The child is terminated as soon as the first record, path, member-count, metadata-byte, per-file, or aggregate-file bound is violated. This gate runs before `git archive`, so an excessive or structurally unsupported tree cannot first consume archive storage or unbounded host memory. + +### Archive and extraction bounds + +After exact-tree preflight, the isolated control directory creates an exact-commit tar archive. The private attribute layer prevents committed or local `export-ignore` and `export-subst` rules from omitting files or rewriting blob bytes. + +The archive is independently treated as hostile. Before extraction, Noema allows only normalized regular files and populated directories. It rejects links, devices, FIFOs, special entries, `.git` content, aliases, duplicate names, file-directory collisions, children below files, empty gitlink-like leaf directories, excessive members, oversized files, and excessive aggregate bytes. + +Only the validated member list is extracted through Python's `data` filter into an owner-only temporary directory. Noema then walks the snapshot with `lstat` and requires exact path, type, and regular-file-size equality with the validated archive manifest. Docker receives this verified committed snapshot, not the mutable worktree. + +The snapshot contains only a type-compatible empty `.git` placeholder. Directory-style repositories receive an empty directory boundary; linked worktrees receive an empty regular-file boundary. Checkout credentials, remotes, local configuration, object storage, reflogs, and worktree pointers therefore do not enter the container. + +## Patch preflight + +The original patch is read as a stable, bounded, regular non-symlink file and matched to the request's SHA-256 digest. Its caller-controlled pathname is never passed to Docker; verified bytes are copied to an owner-only staging file and mounted read-only. + +A patch is rejected before Docker starts when it is: + +- empty, above 4 MiB, non-UTF-8, binary, unstable, symlinked, or not regular; +- missing or malformed `diff --git` headers; +- changing more than 100 files or repeating a target path; +- using noncanonical aliases such as repeated slashes, `.` components, a trailing slash, traversal, absolute paths, raw backslashes, malformed quoting, or control characters; +- creating, deleting, or retaining symlink or gitlink modes; +- declaring malformed, misplaced, conflicting, duplicated, or incomplete file, rename, copy, mode, index, similarity, or hunk metadata; +- redirecting `---`, `+++`, rename, or copy metadata away from the active primary source or target identity; +- using `/dev/null` outside canonical creation or deletion headers; or +- touching `.git/`, `.github/workflows/`, `.github/actions/`, `.gitmodules`, Dependabot configuration, or protected `CODEOWNERS` paths. + +File-mode metadata is parsed as a complete line containing one recognized directive and exactly one six-digit mode token. Only `100644` and `100755` are accepted. Exact `120000` and `160000` modes retain the dedicated symlink/gitlink rejection, while trailing or additional tokens such as `new file mode 120000 100644` are rejected as malformed instead of being accepted through suffix matching. + +Unified hunk line counts must be consumed exactly. Newline markers are accepted only once after valid hunk content. Truncated hunks, extra content after declared counts, and path metadata after a hunk fail closed. + +## Container boundary + +`NOEMA_PATCH_SANDBOX_IMAGE` must be an immutable reference in this repository namespace: + +```text +ghcr.io/contextualwisdomlab/noema-patch-validator@sha256:<64 lowercase hexadecimal characters> +``` + +The runner uses `--pull=never`. The trusted release workflow remains responsible for separately building, signing, scanning, attesting, and approving the image. + +The container runs with: + +- no network and no Docker socket; +- a read-only root filesystem; +- read-only source and patch mounts; +- a non-root host UID/GID; +- all capabilities dropped; +- `no-new-privileges`, seccomp, and isolated IPC; +- bounded PID, CPU, memory, swap, descriptor, process, core, file-size, tmpfs, and wall-time resources; and +- no GitHub, reviewer, NVIDIA NIM, Cloudflare, OIDC, publication, or deployment credential. + +## Result boundary + +The container receives one pre-created writable host file at `/output/result.json`; it does not receive a writable host directory. Normal stdout and stderr are discarded and are never accepted as evidence. + +The host reads the result through the same no-follow, inode/device-stable regular-file boundary with a separate 16 KiB ceiling. The schema: + +- rejects unknown fields; +- bounds status, exit code, duration, excerpts, and reason codes; +- requires `passed` to report exit code `0`; +- repeats repository, base SHA, head SHA, patch digest, and profile; and +- must report the exact command baked into the selected profile. + +A missing, empty, malformed, oversized, inconsistent, or identity-mismatched result fails closed. + +The process-wide file-size ceiling is 64 MiB so an allowlisted validation command can create bounded workspace artifacts without making the 16 KiB evidence file an alternate resource limit. + +## Python API + +```python +from pathlib import Path + +from noema_reviewer.patch_validation import ( + DockerPatchValidationRunner, + PatchValidationProfile, + PatchValidationRequest, +) + +request = PatchValidationRequest( + repository_full_name="ContextualWisdomLab/noema", + base_sha="0" * 40, + head_sha="1" * 40, + patch_sha256="2" * 64, + profile=PatchValidationProfile.NODE_RELEASE_VERIFY, +) + +result = DockerPatchValidationRunner().validate( + request=request, + source_root=Path("/trusted/git-checkout"), + patch_path=Path("/trusted/change.patch"), +) +``` + +The values above are placeholders. Production callers must calculate the real patch digest and bind authenticated exact commits. + +## Operational interpretation + +A passed result is evidence only for the bound repository, base, head, patch bytes, profile, source object database, and validator image. Merge still requires the live exact head, every required CI and security gate, resolved current review findings, an eligible independent approval, branch protection, provenance, and release acceptance. Queued or pending checks are not success. + +The feature fails closed when source identity, streamed Git evidence, tree bounds, archive materialization, extraction equality, patch syntax, Docker execution, result parsing, or request/result identity cannot be established. + +## Verification + +```bash +cd reviewer +python -m pytest +python -m interrogate -c pyproject.toml noema_reviewer +``` + +Repository CI requires 100 percent production statement and branch coverage and 100 percent public docstring coverage. Regression tests prove bounded streamed status and exact-tree reads, immediate child termination, shared deadlines, record and path ceilings, literal ASCII metadata separators, documented Git long-size padding, Unicode rejection, exact-tree parsing, canonical path identity, rename/copy families, Git control isolation, linked worktrees, worktree drift, archive and extraction boundaries, descriptor races, result-channel bounds, Docker isolation, and exact request/result binding. + +This PR does not yet build or publish the patch-validator image and does not activate patch validation in the reviewer decision flow. Those are separate follow-on gates. + +For design rationale and APA 7th references, see `docs/doctoring/quarantined-patch-validation.md`. diff --git a/reviewer/noema_reviewer/__init__.py b/reviewer/noema_reviewer/__init__.py index f27d01cd..6b918278 100644 --- a/reviewer/noema_reviewer/__init__.py +++ b/reviewer/noema_reviewer/__init__.py @@ -13,11 +13,24 @@ from .agent import PydanticAIReviewAgent, ReviewAgent, build_agent from .manifest import ReviewManifest from .models import Confidence, Finding, ReviewVerdict, Severity, Verdict +from .patch_validation import ( + DockerPatchValidationRunner, + PatchValidationProfile, + PatchValidationRequest, + PatchValidationResult, + PatchValidationStatus, + inspect_patch_bytes, +) __all__ = [ "Confidence", + "DockerPatchValidationRunner", "Finding", + "PatchValidationProfile", + "PatchValidationRequest", + "PatchValidationResult", + "PatchValidationStatus", "PydanticAIReviewAgent", "ReviewAgent", "ReviewManifest", @@ -25,4 +38,5 @@ "Severity", "Verdict", "build_agent", + "inspect_patch_bytes", ] diff --git a/reviewer/noema_reviewer/patch_validation.py b/reviewer/noema_reviewer/patch_validation.py new file mode 100644 index 00000000..ab420973 --- /dev/null +++ b/reviewer/noema_reviewer/patch_validation.py @@ -0,0 +1,1459 @@ +"""Credential-free patch validation in an immutable sandbox image. + +This module is deliberately narrower than a general-purpose CI runner. It +accepts one exact-head-bound, text-only Git patch and one allowlisted validation +profile. The source checkout and patch are mounted read-only; the validator +container receives no repository, reviewer, model, Cloudflare, Docker, or OIDC +credentials and has no network access. The container returns a bounded JSON +artifact that is revalidated against the request before it can influence a +review verdict. +""" + +from __future__ import annotations + +import hashlib +import os +import re +import select +import shlex +import shutil +import stat +import subprocess +import tarfile +import tempfile +import time +import uuid +from collections.abc import Callable +from enum import Enum +from pathlib import Path, PurePosixPath +from typing import Annotated, Any, Literal, Self + +from pydantic import BaseModel, ConfigDict, Field, ValidationError, model_validator + + +TRUSTED_PATCH_IMAGE_REPOSITORY = ( + "ghcr.io/contextualwisdomlab/noema-patch-validator" +) +TRUSTED_PATCH_IMAGE_RE = re.compile( + rf"^{re.escape(TRUSTED_PATCH_IMAGE_REPOSITORY)}@sha256:[0-9a-f]{{64}}$" +) +TRUSTED_GIT_EXECUTABLE = shutil.which("git") or "/usr/bin/git" +PATCH_SANDBOX_WALL_TIMEOUT_SECONDS = 1200 +GIT_STREAM_TIMEOUT_SECONDS = 30 +GIT_STREAM_TERMINATION_TIMEOUT_SECONDS = 5 +GIT_STREAM_READ_BYTES = 65_536 +MAX_PATCH_BYTES = 4 * 1024 * 1024 +MAX_CHANGED_FILES = 100 +MAX_SOURCE_ARCHIVE_MEMBERS = 20_000 +MAX_SOURCE_ARCHIVE_MEMBER_BYTES = 64 * 1024 * 1024 +MAX_SOURCE_ARCHIVE_FILE_BYTES = MAX_SOURCE_ARCHIVE_MEMBER_BYTES +MAX_SOURCE_ARCHIVE_TOTAL_BYTES = 512 * 1024 * 1024 +MAX_SOURCE_TREE_PATH_BYTES = 4096 +MAX_SOURCE_TREE_RECORD_BYTES = MAX_SOURCE_TREE_PATH_BYTES + 256 +MAX_SOURCE_TREE_METADATA_BYTES = 16 * 1024 * 1024 +GIT_LONG_OBJECT_SIZE_MINIMUM_WIDTH = 7 +MAX_GIT_CONTROL_FILE_BYTES = 4096 +MAX_DIAGNOSTIC_CHARS = 1000 +MAX_RESULT_EXCERPT_CHARS = 4000 +MAX_RESULT_JSON_BYTES = 16 * 1024 +MAX_RESULT_DURATION_MS = PATCH_SANDBOX_WALL_TIMEOUT_SECONDS * 1000 +SHA1_PATTERN = r"^[0-9a-f]{40}$" +SHA256_PATTERN = r"^[0-9a-f]{64}$" +REPOSITORY_PATTERN = r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$" +REASON_CODE_PATTERN = r"^[a-z][a-z0-9_]{0,63}$" +PATCH_MODE_PATTERN = re.compile( + r"^(?:old mode|new mode|new file mode|deleted file mode) (120000|160000)$", + re.MULTILINE, +) +FILE_MODE_METADATA_PATTERN = re.compile( + r"^(?:old mode|new mode|new file mode|deleted file mode) ([0-9]{6})$" +) +INDEX_MODE_PATTERN = re.compile( + r"^index [0-9a-fA-F]{4,64}\.\.[0-9a-fA-F]{4,64}(?: ([0-9]{6}))?$" +) +HUNK_HEADER_PATTERN = re.compile( + r"^@@ -(?P[0-9]+)(?:,(?P[0-9]+))? " + r"\+(?P[0-9]+)(?:,(?P[0-9]+))? @@(?: .*)?$" +) +PERCENT_METADATA_PATTERN = re.compile(r"^(?:similarity|dissimilarity) index [0-9]{1,3}%$") +GIT_OBJECT_ID_PATTERN = re.compile(r"^(?:[0-9a-f]{40}|[0-9a-f]{64})$") +FORBIDDEN_PATCH_PATHS = frozenset( + { + ".gitmodules", + ".github/CODEOWNERS", + ".github/dependabot.yml", + "CODEOWNERS", + "docs/CODEOWNERS", + } +) +FORBIDDEN_PATCH_PREFIXES = ( + ".git/", + ".github/actions/", + ".github/workflows/", +) +SECONDARY_PATCH_PATH_HEADERS = ( + ("--- ", "a/", True, "file", "source"), + ("+++ ", "b/", True, "file", "target"), + ("rename from ", None, False, "rename", "source"), + ("rename to ", None, False, "rename", "target"), + ("copy from ", None, False, "copy", "source"), + ("copy to ", None, False, "copy", "target"), +) + +ProcessRunner = Callable[..., subprocess.CompletedProcess[str]] +NameFactory = Callable[[], str] +GitMetadataKind = Literal["directory", "file"] +SecondaryPatchPathFamily = Literal["file", "rename", "copy"] +SecondaryPatchPathRole = Literal["source", "target"] +SourceArchiveEntryKind = Literal["directory", "file"] +SourceArchiveEntry = tuple[SourceArchiveEntryKind, int] +ReasonCode = Annotated[ + str, + Field(min_length=1, max_length=64, pattern=REASON_CODE_PATTERN), +] + + +class PatchValidationProfile(str, Enum): + """Approved test command profiles baked into the validator image.""" + + NODE_RELEASE_VERIFY = "node_release_verify" + + +class PatchValidationStatus(str, Enum): + """Terminal outcomes emitted by the validator image.""" + + PASSED = "passed" + FAILED = "failed" + BLOCKED = "blocked" + + +PROFILE_COMMANDS: dict[PatchValidationProfile, str] = { + PatchValidationProfile.NODE_RELEASE_VERIFY: "npm run release:verify", +} + + +class PatchValidationRequest(BaseModel): + """Exact revision and patch identity allowed to enter the sandbox.""" + + model_config = ConfigDict(extra="forbid") + + repository_full_name: str = Field(pattern=REPOSITORY_PATTERN) + base_sha: str = Field(pattern=SHA1_PATTERN) + head_sha: str = Field(pattern=SHA1_PATTERN) + patch_sha256: str = Field(pattern=SHA256_PATTERN) + profile: PatchValidationProfile + + +class PatchValidationResult(BaseModel): + """Bounded, exact-request-bound evidence returned by the sandbox.""" + + model_config = ConfigDict(extra="forbid") + + status: PatchValidationStatus + repository_full_name: str = Field(pattern=REPOSITORY_PATTERN) + base_sha: str = Field(pattern=SHA1_PATTERN) + head_sha: str = Field(pattern=SHA1_PATTERN) + patch_sha256: str = Field(pattern=SHA256_PATTERN) + profile: PatchValidationProfile + command_profile: str = Field(min_length=1, max_length=200) + exit_code: int = Field(ge=0, le=255) + duration_ms: int = Field(ge=0, le=MAX_RESULT_DURATION_MS) + stdout_excerpt: str = Field(max_length=MAX_RESULT_EXCERPT_CHARS) + stderr_excerpt: str = Field(max_length=MAX_RESULT_EXCERPT_CHARS) + reason_codes: list[ReasonCode] = Field(default_factory=list, max_length=20) + + @model_validator(mode="after") + def require_successful_exit_for_passed_status(self) -> Self: + """Reject evidence that claims success while reporting a failing command.""" + if self.status is PatchValidationStatus.PASSED and self.exit_code != 0: + raise ValueError("passed patch validation requires exit_code 0") + return self + + +class _PatchFileSystem: + """Injectable descriptor-safe filesystem operations for bounded reads.""" + + lstat = staticmethod(os.lstat) + open = staticmethod(os.open) + fstat = staticmethod(os.fstat) + read = staticmethod(os.read) + close = staticmethod(os.close) + + +DEFAULT_PATCH_FILE_SYSTEM = _PatchFileSystem() + + +def _bounded_detail(text: str) -> str: + """Return a single bounded diagnostic for an infrastructure failure.""" + compact = text.strip() or "no diagnostic output" + if len(compact) <= MAX_DIAGNOSTIC_CHARS: + return compact + omitted = len(compact) - MAX_DIAGNOSTIC_CHARS + return f"{compact[:MAX_DIAGNOSTIC_CHARS]} [truncated {omitted} characters]" + + +def _default_name() -> str: + """Return an unpredictable Docker-safe container name.""" + return f"noema-patch-{uuid.uuid4().hex}" + + +def _verified_image_reference() -> str: + """Return the workflow-verified immutable patch-validator image.""" + image = os.environ.get("NOEMA_PATCH_SANDBOX_IMAGE", "").strip() + if not TRUSTED_PATCH_IMAGE_RE.fullmatch(image): + raise RuntimeError( + "NOEMA_PATCH_SANDBOX_IMAGE must be a verified immutable " + f"{TRUSTED_PATCH_IMAGE_REPOSITORY}@sha256 reference" + ) + return image + + +def _validated_docker_mount_path(path: Path, label: str) -> Path: + """Reject path characters that can alter Docker's comma-delimited mount grammar.""" + if any(character in str(path) for character in (",", "\n", "\r")): + raise RuntimeError( + f"{label} contains characters unsafe for a Docker mount: {path}" + ) + return path + + +def _validated_directory(raw_path: str | Path, label: str) -> Path: + """Resolve one trusted bind-mount directory and reject Docker delimiters.""" + try: + resolved = Path(raw_path).resolve(strict=True) + except OSError as exc: + raise RuntimeError(f"{label} is unavailable: {exc}") from exc + if not resolved.is_dir(): + raise RuntimeError(f"{label} must be a directory: {resolved}") + return _validated_docker_mount_path(resolved, label) + + +def _absolute_without_following(raw_path: str | Path) -> Path: + """Return an absolute path without resolving its final symlink component.""" + return Path(os.path.abspath(os.fspath(raw_path))) + + +def _read_regular_patch( + raw_path: str | Path, + *, + file_system: Any = DEFAULT_PATCH_FILE_SYSTEM, + maximum_bytes: int = MAX_PATCH_BYTES, + label: str = "patch file", +) -> tuple[Path, bytes]: + """Read one stable bounded regular file without following its final path.""" + path = _absolute_without_following(raw_path) + try: + linked = file_system.lstat(path) + except OSError as exc: + raise RuntimeError(f"{label} is unavailable: {exc}") from exc + if not stat.S_ISREG(linked.st_mode) or stat.S_ISLNK(linked.st_mode): + raise RuntimeError(f"{label} must be a regular non-symlink file") + if linked.st_size <= 0: + raise RuntimeError(f"{label} must not be empty") + if linked.st_size > maximum_bytes: + raise RuntimeError(f"{label} exceeds {maximum_bytes} bytes") + + descriptor: int | None = None + try: + descriptor = file_system.open( + path, + os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0), + ) + opened = file_system.fstat(descriptor) + if not stat.S_ISREG(opened.st_mode): + raise RuntimeError(f"{label} changed during validation") + if opened.st_dev != linked.st_dev or opened.st_ino != linked.st_ino: + raise RuntimeError(f"{label} changed during validation") + + chunks: list[bytes] = [] + total = 0 + while True: + chunk = file_system.read( + descriptor, + min(65_536, maximum_bytes + 1 - total), + ) + if not chunk: + break + chunks.append(chunk) + total += len(chunk) + if total > maximum_bytes: + raise RuntimeError(f"{label} exceeds {maximum_bytes} bytes") + data = b"".join(chunks) + if not data: + raise RuntimeError(f"{label} must not be empty") + return path, data + except OSError as exc: + raise RuntimeError(f"{label} could not be read safely: {exc}") from exc + finally: + if descriptor is not None: + file_system.close(descriptor) + + +def _read_git_control_line(path: Path, label: str) -> str: + """Read one stable bounded UTF-8 Git control line without following symlinks.""" + try: + linked = os.lstat(path) + except OSError as exc: + raise RuntimeError(f"{label} is unavailable") from exc + if not stat.S_ISREG(linked.st_mode) or stat.S_ISLNK(linked.st_mode): + raise RuntimeError(f"{label} must be a regular non-symlink file") + if linked.st_size <= 0 or linked.st_size > MAX_GIT_CONTROL_FILE_BYTES: + raise RuntimeError(f"{label} has an invalid byte length") + + descriptor: int | None = None + try: + descriptor = os.open(path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0)) + opened = os.fstat(descriptor) + if ( + not stat.S_ISREG(opened.st_mode) + or opened.st_dev != linked.st_dev + or opened.st_ino != linked.st_ino + ): + raise RuntimeError(f"{label} changed during validation") + chunks: list[bytes] = [] + total = 0 + while True: + chunk = os.read( + descriptor, + min(4096, MAX_GIT_CONTROL_FILE_BYTES + 1 - total), + ) + if not chunk: + break + chunks.append(chunk) + total += len(chunk) + if total > MAX_GIT_CONTROL_FILE_BYTES: + raise RuntimeError(f"{label} has an invalid byte length") + data = b"".join(chunks) + except OSError as exc: + raise RuntimeError(f"{label} could not be read safely") from exc + finally: + if descriptor is not None: + os.close(descriptor) + + try: + text = data.decode("utf-8", errors="strict") + except UnicodeDecodeError as exc: + raise RuntimeError(f"{label} must be valid UTF-8") from exc + line = text.removesuffix("\n") + if not line or "\n" in line or "\r" in line or "\x00" in line: + raise RuntimeError(f"{label} must contain one unambiguous line") + return line + + +def _validated_git_directory(path: Path, label: str, *, require_exists: bool) -> Path: + """Return a normalized Git control directory or fail closed when required.""" + absolute = _absolute_without_following(path) + if any(character in str(absolute) for character in ("\x00", "\n", "\r")): + raise RuntimeError(f"{label} contains unsafe path characters") + if not require_exists: + return absolute + try: + metadata = os.lstat(absolute) + except OSError as exc: + raise RuntimeError(f"{label} is unavailable") from exc + if stat.S_ISLNK(metadata.st_mode) or not stat.S_ISDIR(metadata.st_mode): + raise RuntimeError(f"{label} must be a regular directory") + return absolute + + +def _source_object_directory( + source: Path, + metadata_kind: GitMetadataKind, + *, + require_exists: bool, +) -> Path: + """Resolve the primary object database without executing source-local Git config.""" + if metadata_kind == "directory": + git_directory = source / ".git" + else: + gitfile = _read_git_control_line(source / ".git", "source Git file") + if not gitfile.startswith("gitdir: ") or not gitfile.removeprefix("gitdir: "): + raise RuntimeError("source Git file has an invalid gitdir record") + raw_git_directory = Path(gitfile.removeprefix("gitdir: ")) + git_directory = ( + raw_git_directory + if raw_git_directory.is_absolute() + else source / raw_git_directory + ) + git_directory = _validated_git_directory( + git_directory, + "source Git directory", + require_exists=require_exists, + ) + + common_directory = git_directory + commondir_path = git_directory / "commondir" + try: + os.lstat(commondir_path) + except FileNotFoundError: + pass + except OSError as exc: + raise RuntimeError("source Git common-directory record is unavailable") from exc + else: + commondir = Path( + _read_git_control_line( + commondir_path, + "source Git common-directory record", + ) + ) + common_directory = ( + commondir if commondir.is_absolute() else git_directory / commondir + ) + common_directory = _validated_git_directory( + common_directory, + "source Git common directory", + require_exists=require_exists, + ) + + return _validated_git_directory( + common_directory / "objects", + "source Git object directory", + require_exists=require_exists, + ) + + +def _isolated_git_environment() -> dict[str, str]: + """Return a minimal environment that disables host Git configuration channels.""" + return { + "PATH": str(Path(TRUSTED_GIT_EXECUTABLE).parent), + "GIT_CONFIG_NOSYSTEM": "1", + "GIT_CONFIG_GLOBAL": os.devnull, + "GIT_OPTIONAL_LOCKS": "0", + "GIT_ATTR_NOSYSTEM": "1", + "GIT_NO_REPLACE_OBJECTS": "1", + "GIT_NO_LAZY_FETCH": "1", + } + + +def _remaining_process_timeout(deadline: float) -> float: + """Return the positive time remaining for one bounded Git child operation.""" + remaining = deadline - time.monotonic() + if remaining <= 0: + raise subprocess.TimeoutExpired("git", GIT_STREAM_TIMEOUT_SECONDS) + return remaining + + +def _start_git_stream(command: list[str]) -> Any: + """Start one configuration-isolated Git child with a binary stdout pipe.""" + return subprocess.Popen( + command, + stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=subprocess.DEVNULL, + shell=False, + close_fds=True, + env=_isolated_git_environment(), + ) + + +def _read_git_stream_chunk(process: Any, maximum_bytes: int, deadline: float) -> bytes: + """Read at most one bounded chunk after waiting within the shared deadline.""" + stdout = process.stdout + if stdout is None: + raise RuntimeError("Git child stdout pipe is unavailable") + ready, _writable, _exceptional = select.select( + [stdout], + [], + [], + _remaining_process_timeout(deadline), + ) + if not ready: + raise subprocess.TimeoutExpired("git", GIT_STREAM_TIMEOUT_SECONDS) + return os.read(stdout.fileno(), maximum_bytes) + + +def _wait_git_stream(process: Any, deadline: float) -> int: + """Wait for one Git child without exceeding its shared wall deadline.""" + return process.wait(timeout=_remaining_process_timeout(deadline)) + + +def _terminate_git_stream(process: Any) -> None: + """Terminate one unfinished Git child and escalate to kill within fixed bounds.""" + if process.poll() is not None: + return + process.terminate() + try: + process.wait(timeout=GIT_STREAM_TERMINATION_TIMEOUT_SECONDS) + except subprocess.TimeoutExpired: + process.kill() + process.wait(timeout=GIT_STREAM_TERMINATION_TIMEOUT_SECONDS) + + +def _close_git_stream(process: Any | None) -> None: + """Close the parent copy of one Git stdout pipe when present.""" + if process is None or process.stdout is None: + return + close = getattr(process.stdout, "close", None) + if close is not None: + close() + + +def _create_isolated_git_control( + source: Path, + head_sha: str, + staging_root: Path, + metadata_kind: GitMetadataKind, + *, + require_object_directory: bool, +) -> Path: + """Create private Git control metadata backed only by content-addressed objects.""" + object_directory = _source_object_directory( + source, + metadata_kind, + require_exists=require_object_directory, + ) + control = staging_root / "isolated-git-control" + objects_info = control / "objects" / "info" + info = control / "info" + refs = control / "refs" / "heads" + objects_info.mkdir(parents=True, mode=0o700) + info.mkdir(mode=0o700) + refs.mkdir(parents=True, mode=0o700) + (control / "config").write_text( + "[core]\nrepositoryformatversion = 0\nbare = true\n", + encoding="utf-8", + ) + (control / "HEAD").write_text(f"{head_sha}\n", encoding="ascii") + (objects_info / "alternates").write_text( + f"{object_directory}\n", + encoding="utf-8", + ) + (info / "attributes").write_text( + "* -export-ignore -export-subst\n", + encoding="utf-8", + ) + for control_file in ( + control / "config", + control / "HEAD", + objects_info / "alternates", + info / "attributes", + ): + control_file.chmod(0o600) + return control + + +def _validated_repository_path(raw_path: str) -> str: + """Return one canonical repository-relative path or reject governed targets.""" + if ( + not raw_path + or raw_path.startswith("/") + or "\\" in raw_path + or any(ord(character) < 32 or ord(character) == 127 for character in raw_path) + ): + raise ValueError("patch contains an unsafe repository path") + pure_path = PurePosixPath(raw_path) + normalized = pure_path.as_posix() + if ( + pure_path.is_absolute() + or normalized == "." + or normalized != raw_path + or any(part in ("", ".", "..") for part in pure_path.parts) + ): + raise ValueError("patch contains an unsafe repository path") + if normalized in FORBIDDEN_PATCH_PATHS or normalized.startswith( + FORBIDDEN_PATCH_PREFIXES + ): + raise ValueError(f"patch targets forbidden path: {normalized}") + return normalized + + +def _validated_patch_path(raw_path: str, prefix: str) -> str: + """Normalize one prefixed diff path and reject traversal or governance paths.""" + if not raw_path.startswith(prefix): + raise ValueError("patch contains a malformed diff path") + return _validated_repository_path(raw_path[len(prefix) :]) + + +def _decoded_secondary_path(raw_path: str) -> str: + """Decode one optional quoted metadata path without accepting escape sequences.""" + if "\\" in raw_path: + raise ValueError("patch contains an unsafe repository path") + if raw_path.startswith('"'): + try: + parts = shlex.split(raw_path) + except ValueError as exc: + raise ValueError("patch contains a malformed diff header") from exc + if len(parts) != 1: + raise ValueError("patch contains a malformed diff header") + return parts[0] + if '"' in raw_path: + raise ValueError("patch contains a malformed diff header") + return raw_path + + +def _validated_secondary_patch_header( + line: str, +) -> tuple[SecondaryPatchPathFamily, SecondaryPatchPathRole, str | None] | None: + """Return a normalized auxiliary metadata family, role, and optional path.""" + for marker, prefix, allows_dev_null, family, role in SECONDARY_PATCH_PATH_HEADERS: + if not line.startswith(marker): + continue + raw_path = _decoded_secondary_path(line[len(marker) :]) + if allows_dev_null and raw_path == "/dev/null": + return family, role, None + normalized = ( + _validated_repository_path(raw_path) + if prefix is None + else _validated_patch_path(raw_path, prefix) + ) + return family, role, normalized + return None + + +def inspect_patch_bytes(patch_bytes: bytes) -> tuple[str, ...]: + """Return changed paths after strict text, hunk, mode, path, and size validation.""" + if not patch_bytes: + raise ValueError("patch must not be empty") + if len(patch_bytes) > MAX_PATCH_BYTES: + raise ValueError(f"patch exceeds {MAX_PATCH_BYTES} bytes") + try: + text = patch_bytes.decode("utf-8", errors="strict") + except UnicodeDecodeError as exc: + raise ValueError("patch must be valid UTF-8") from exc + if "GIT binary patch" in text or "Binary files " in text: + raise ValueError("binary patch payloads are not allowed") + if PATCH_MODE_PATTERN.search(text): + raise ValueError("patch contains a symlink or gitlink mode") + lines = text.splitlines() + if not any(line.startswith("diff --git ") for line in lines): + raise ValueError("patch contains no diff headers") + + changed_paths: list[str] = [] + in_hunk = False + old_remaining = 0 + new_remaining = 0 + previous_hunk_content = False + newline_marker_seen = False + current_diff_has_hunk = False + current_source_path: str | None = None + current_target_path: str | None = None + secondary_paths: dict[ + SecondaryPatchPathFamily, + dict[SecondaryPatchPathRole, tuple[bool, str | None]], + ] = {} + + def reset_secondary_paths() -> None: + """Reset independent file-header, rename, and copy metadata families.""" + secondary_paths.clear() + for family in ("file", "rename", "copy"): + secondary_paths[family] = { + "source": (False, None), + "target": (False, None), + } + + def validate_secondary_pairs() -> None: + """Validate each complete metadata family and canonical `/dev/null` use.""" + complete_families: set[SecondaryPatchPathFamily] = set() + for family in ("file", "rename", "copy"): + source_seen, source_path = secondary_paths[family]["source"] + target_seen, target_path = secondary_paths[family]["target"] + if source_seen != target_seen: + raise ValueError("patch contains incomplete secondary path metadata") + if not source_seen: + continue + complete_families.add(family) + if source_path is None and target_path is None: + raise ValueError("patch contains invalid /dev/null path metadata") + if source_path is None: + if ( + family != "file" + or target_path != current_target_path + or current_source_path != current_target_path + ): + raise ValueError("patch contains noncanonical creation metadata") + elif target_path is None: + if ( + family != "file" + or source_path != current_source_path + or current_source_path != current_target_path + ): + raise ValueError("patch contains noncanonical deletion metadata") + if "rename" in complete_families and "copy" in complete_families: + raise ValueError("patch contains conflicting rename and copy metadata") + + reset_secondary_paths() + for line in lines: + if in_hunk: + if line == "\\ No newline at end of file": + if not previous_hunk_content or newline_marker_seen: + raise ValueError("patch contains a malformed hunk newline marker") + newline_marker_seen = True + previous_hunk_content = False + continue + if old_remaining == 0 and new_remaining == 0: + in_hunk = False + previous_hunk_content = False + newline_marker_seen = False + else: + if not line: + raise ValueError("patch contains a malformed hunk body") + marker = line[0] + if marker == " ": + old_remaining -= 1 + new_remaining -= 1 + elif marker == "-": + old_remaining -= 1 + elif marker == "+": + new_remaining -= 1 + else: + raise ValueError("patch contains a malformed hunk body") + if old_remaining < 0 or new_remaining < 0: + raise ValueError("patch hunk contains more lines than declared") + previous_hunk_content = True + newline_marker_seen = False + continue + + if line.startswith("diff --git "): + validate_secondary_pairs() + current_diff_has_hunk = False + reset_secondary_paths() + if "\\" in line: + raise ValueError("patch contains an unsafe repository path") + try: + parts = shlex.split(line) + except ValueError as exc: + raise ValueError("patch contains a malformed diff header") from exc + if len(parts) != 4 or parts[:2] != ["diff", "--git"]: + raise ValueError("patch contains a malformed diff header") + current_source_path = _validated_patch_path(parts[2], "a/") + current_target_path = _validated_patch_path(parts[3], "b/") + if current_target_path in changed_paths: + raise ValueError(f"patch repeats changed path: {current_target_path}") + changed_paths.append(current_target_path) + if len(changed_paths) > MAX_CHANGED_FILES: + raise ValueError(f"patch changes more than {MAX_CHANGED_FILES} files") + continue + + if line.startswith("@@"): + if current_source_path is None or current_target_path is None: + raise ValueError("patch hunk appears before a diff header") + validate_secondary_pairs() + match = HUNK_HEADER_PATTERN.fullmatch(line) + if match is None: + raise ValueError("patch contains a malformed hunk header") + old_remaining = int(match.group("old_count") or "1") + new_remaining = int(match.group("new_count") or "1") + in_hunk = True + previous_hunk_content = False + newline_marker_seen = False + current_diff_has_hunk = True + continue + + secondary_path = _validated_secondary_patch_header(line) + if secondary_path is not None: + if current_source_path is None or current_target_path is None: + raise ValueError("patch path metadata appears before a diff header") + if current_diff_has_hunk: + raise ValueError("patch contains path metadata after a hunk") + family, role, normalized_path = secondary_path + expected_path = current_source_path if role == "source" else current_target_path + if normalized_path is not None and normalized_path != expected_path: + raise ValueError( + "secondary patch path does not match the primary diff path" + ) + seen, _previous_path = secondary_paths[family][role] + if seen: + raise ValueError(f"patch repeats {role} path metadata") + secondary_paths[family][role] = (True, normalized_path) + continue + + if line.startswith("index "): + if current_source_path is None or current_diff_has_hunk: + raise ValueError("patch contains misplaced index metadata") + match = INDEX_MODE_PATTERN.fullmatch(line) + if match is None: + raise ValueError("patch contains malformed index metadata") + mode = match.group(1) + if mode in {"120000", "160000"}: + raise ValueError("patch contains a symlink or gitlink mode") + if mode is not None and mode not in {"100644", "100755"}: + raise ValueError("patch contains an unsupported index mode") + continue + + if line.startswith(("old mode ", "new mode ", "new file mode ", "deleted file mode ")): + if current_source_path is None or current_diff_has_hunk: + raise ValueError("patch contains misplaced mode metadata") + match = FILE_MODE_METADATA_PATTERN.fullmatch(line) + if match is None: + raise ValueError("patch contains malformed mode metadata") + if match.group(1) not in {"100644", "100755"}: + raise ValueError("patch contains an unsupported file mode") + continue + + if line.startswith(("similarity index ", "dissimilarity index ")): + if current_source_path is None or current_diff_has_hunk: + raise ValueError("patch contains misplaced similarity metadata") + if PERCENT_METADATA_PATTERN.fullmatch(line) is None: + raise ValueError("patch contains malformed similarity metadata") + percentage = int(line.rsplit(" ", 1)[1].removesuffix("%")) + if percentage > 100: + raise ValueError("patch contains malformed similarity metadata") + continue + + if line == "": + continue + if line == "\\ No newline at end of file": + raise ValueError("patch contains a malformed hunk newline marker") + if line.startswith((" ", "+", "-")): + raise ValueError("patch hunk contains more lines than declared") + raise ValueError("patch contains unbound trailing syntax") + + if in_hunk and (old_remaining != 0 or new_remaining != 0): + raise ValueError("patch hunk ended before its declared line counts") + validate_secondary_pairs() + return tuple(changed_paths) + + +def _result_matches_request( + result: PatchValidationResult, + request: PatchValidationRequest, +) -> bool: + """Return whether result identity and allowlisted command match the request.""" + observed = ( + result.repository_full_name, + result.base_sha, + result.head_sha, + result.patch_sha256, + result.profile, + result.command_profile, + ) + expected = ( + request.repository_full_name, + request.base_sha, + request.head_sha, + request.patch_sha256, + request.profile, + PROFILE_COMMANDS[request.profile], + ) + return observed == expected + + +def _git_metadata_kind(source: Path) -> GitMetadataKind | None: + """Return safe Git-control metadata shape or reject special-file redirection.""" + try: + metadata = os.lstat(source / ".git") + except FileNotFoundError: + return None + if stat.S_ISLNK(metadata.st_mode) or not ( + stat.S_ISDIR(metadata.st_mode) or stat.S_ISREG(metadata.st_mode) + ): + raise RuntimeError( + "source Git metadata must not be a symlink and must be a regular file or directory" + ) + return "directory" if stat.S_ISDIR(metadata.st_mode) else "file" + + +def _verify_source_head( + source: Path, + expected_head_sha: str, + metadata_kind: GitMetadataKind | None, +) -> None: + """Reject source whose exact authenticated Git tree differs from the request.""" + if metadata_kind is None: + raise RuntimeError("source Git metadata is required for exact-head validation") + with tempfile.TemporaryDirectory(prefix="noema-git-preflight-") as staging: + staging_root = Path(staging) + try: + control = _create_isolated_git_control( + source, + expected_head_sha, + staging_root, + metadata_kind, + require_object_directory=True, + ) + except RuntimeError as exc: + raise RuntimeError("source HEAD could not be verified") from exc + command_prefix = [ + TRUSTED_GIT_EXECUTABLE, + "-c", + "core.hooksPath=/dev/null", + "-c", + "core.fsmonitor=false", + "-c", + "core.untrackedCache=false", + f"--git-dir={control}", + f"--work-tree={source}", + ] + read_tree = subprocess.run( + [*command_prefix, "read-tree", expected_head_sha], + text=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + shell=False, + timeout=GIT_STREAM_TIMEOUT_SECONDS, + env=_isolated_git_environment(), + ) + if read_tree.returncode != 0: + raise RuntimeError( + "source HEAD does not match the exact validation request" + ) + + process: Any | None = None + try: + process = _start_git_stream( + [ + *command_prefix, + "status", + "--porcelain=v2", + "--untracked-files=all", + "--ignored=matching", + "--", + ".", + ":(exclude).git", + ] + ) + deadline = time.monotonic() + GIT_STREAM_TIMEOUT_SECONDS + first_byte = _read_git_stream_chunk(process, 1, deadline) + if first_byte: + raise RuntimeError("source worktree is not clean") + if _wait_git_stream(process, deadline) != 0: + raise RuntimeError("source HEAD could not be verified") + except RuntimeError: + if process is not None: + _terminate_git_stream(process) + raise + except (OSError, subprocess.TimeoutExpired) as exc: + if process is not None: + _terminate_git_stream(process) + raise RuntimeError("source HEAD could not be verified") from exc + finally: + _close_git_stream(process) + + +def _validated_source_archive_name(raw_name: str) -> str: + """Return one exact normalized archive path or reject aliasing and metadata.""" + candidate = raw_name[:-1] if raw_name.endswith("/") else raw_name + if ( + not candidate + or candidate.startswith("/") + or "\\" in candidate + or candidate == ".git" + or candidate.startswith(".git/") + or any(ord(character) < 32 or ord(character) == 127 for character in candidate) + ): + raise ValueError("source archive contains an unsafe member name") + pure_path = PurePosixPath(candidate) + normalized = pure_path.as_posix() + if ( + pure_path.is_absolute() + or any(part in ("", ".", "..") for part in pure_path.parts) + or normalized != candidate + ): + raise ValueError("source archive contains an unsafe member name") + return normalized + + +def _validated_exact_tree_record( + record: bytes, + observed_paths: set[str], + total_file_bytes: int, +) -> int: + """Validate one bounded binary `ls-tree` record and return aggregate blob bytes.""" + if len(record) > MAX_SOURCE_TREE_RECORD_BYTES: + raise ValueError("source exact tree record exceeds its byte limit") + metadata, separator, raw_path = record.partition(b"\t") + if not separator: + raise ValueError("source exact tree contains malformed metadata") + if len(raw_path) > MAX_SOURCE_TREE_PATH_BYTES: + raise ValueError("source exact tree path exceeds its path byte limit") + try: + metadata_text = metadata.decode("utf-8", errors="strict") + raw_path_text = raw_path.decode("utf-8", errors="strict") + except UnicodeDecodeError as exc: + raise ValueError("source exact tree must be valid UTF-8") from exc + fields = metadata_text.split(" ", 3) + if len(fields) != 4 or "" in fields[:3]: + raise ValueError("source exact tree contains malformed metadata") + mode, object_type, object_id, padded_size = fields + if ( + mode not in {"100644", "100755"} + or object_type != "blob" + or GIT_OBJECT_ID_PATTERN.fullmatch(object_id) is None + ): + raise ValueError("source exact tree contains a non-regular object") + raw_size = padded_size.lstrip(" ") + if not raw_size.isascii() or not raw_size.isdecimal(): + raise ValueError("source exact tree contains an invalid blob size") + padding = len(padded_size) - len(raw_size) + canonical_padding = max(0, GIT_LONG_OBJECT_SIZE_MINIMUM_WIDTH - len(raw_size)) + if padding not in {0, canonical_padding}: + raise ValueError("source exact tree contains malformed metadata") + size = int(raw_size) + if size > MAX_SOURCE_ARCHIVE_MEMBER_BYTES: + raise ValueError("source exact tree member exceeds its byte limit") + aggregate_file_bytes = total_file_bytes + size + if aggregate_file_bytes > MAX_SOURCE_ARCHIVE_TOTAL_BYTES: + raise ValueError("source exact tree exceeds its aggregate byte limit") + normalized = _validated_source_archive_name(raw_path_text) + if normalized in observed_paths: + raise ValueError("source exact tree repeats a member name") + observed_paths.add(normalized) + return aggregate_file_bytes + + +def _validated_exact_tree_output(raw_output: str) -> None: + """Reject malformed, special, aliased, oversized, or excessive exact-tree output.""" + try: + encoded = raw_output.encode("utf-8", errors="strict") + except UnicodeEncodeError as exc: + raise ValueError("source exact tree must be valid UTF-8") from exc + if not encoded or not encoded.endswith(b"\0"): + raise ValueError("source exact tree output is empty or truncated") + if len(encoded) > MAX_SOURCE_TREE_METADATA_BYTES: + raise ValueError("source exact tree metadata exceeds its aggregate byte limit") + observed_paths: set[str] = set() + total_file_bytes = 0 + records = encoded.split(b"\0")[:-1] + if len(records) > MAX_SOURCE_ARCHIVE_MEMBERS: + raise ValueError("source exact tree contains too many members") + for record in records: + total_file_bytes = _validated_exact_tree_record( + record, + observed_paths, + total_file_bytes, + ) + + +def _consume_exact_tree_stream(process: Any, deadline: float) -> None: + """Validate NUL records incrementally without retaining hostile tree output.""" + buffer = bytearray() + observed_paths: set[str] = set() + total_metadata_bytes = 0 + total_file_bytes = 0 + record_count = 0 + while True: + remaining_capacity = MAX_SOURCE_TREE_METADATA_BYTES + 1 - total_metadata_bytes + chunk = _read_git_stream_chunk( + process, + min(GIT_STREAM_READ_BYTES, remaining_capacity), + deadline, + ) + if not chunk: + break + total_metadata_bytes += len(chunk) + if total_metadata_bytes > MAX_SOURCE_TREE_METADATA_BYTES: + raise ValueError( + "source exact tree metadata exceeds its aggregate byte limit" + ) + buffer.extend(chunk) + while True: + delimiter = buffer.find(0) + if delimiter < 0: + break + record = bytes(buffer[:delimiter]) + del buffer[: delimiter + 1] + record_count += 1 + if record_count > MAX_SOURCE_ARCHIVE_MEMBERS: + raise ValueError("source exact tree contains too many members") + total_file_bytes = _validated_exact_tree_record( + record, + observed_paths, + total_file_bytes, + ) + if len(buffer) > MAX_SOURCE_TREE_RECORD_BYTES: + raise ValueError("source exact tree record exceeds its byte limit") + if buffer or record_count == 0: + raise ValueError("source exact tree output is empty or truncated") + if _wait_git_stream(process, deadline) != 0: + raise RuntimeError("source exact tree command failed") + + +def _verify_exact_tree_limits(control: Path, head_sha: str) -> None: + """Stream-check exact committed object bounds before archive serialization.""" + process: Any | None = None + try: + process = _start_git_stream( + [ + TRUSTED_GIT_EXECUTABLE, + "-c", + "core.hooksPath=/dev/null", + "-c", + "core.fsmonitor=false", + f"--git-dir={control}", + "ls-tree", + "-r", + "-l", + "-z", + "--full-tree", + head_sha, + ] + ) + deadline = time.monotonic() + GIT_STREAM_TIMEOUT_SECONDS + _consume_exact_tree_stream(process, deadline) + except ValueError as exc: + if process is not None: + _terminate_git_stream(process) + raise RuntimeError("source exact tree failed bounded validation") from exc + except (OSError, subprocess.TimeoutExpired, RuntimeError) as exc: + if process is not None: + _terminate_git_stream(process) + raise RuntimeError("source exact tree could not be inspected safely") from exc + finally: + _close_git_stream(process) + + +def _validated_source_archive_members( + archive: tarfile.TarFile, +) -> tuple[list[tarfile.TarInfo], dict[str, SourceArchiveEntry]]: + """Allowlist bounded regular-file and populated-directory archive entries.""" + members: list[tarfile.TarInfo] = [] + expected_entries: dict[str, SourceArchiveEntry] = {} + declared_paths: set[str] = set() + declared_directories: set[str] = set() + total_file_bytes = 0 + + for member in archive: + if len(members) >= MAX_SOURCE_ARCHIVE_MEMBERS: + raise ValueError("source archive contains too many members") + normalized = _validated_source_archive_name(member.name) + if normalized in declared_paths: + raise ValueError("source archive repeats a member name") + declared_paths.add(normalized) + + if member.isdir(): + entry: SourceArchiveEntry = ("directory", 0) + declared_directories.add(normalized) + elif member.isreg(): + if not 0 <= member.size <= MAX_SOURCE_ARCHIVE_MEMBER_BYTES: + raise ValueError("source archive member exceeds its byte limit") + total_file_bytes += member.size + if total_file_bytes > MAX_SOURCE_ARCHIVE_TOTAL_BYTES: + raise ValueError("source archive exceeds its aggregate byte limit") + entry = ("file", member.size) + else: + raise ValueError("source archive contains a non-regular member") + + parent = PurePosixPath(normalized).parent + while parent != PurePosixPath("."): + parent_name = parent.as_posix() + parent_entry = expected_entries.get(parent_name) + if parent_entry is not None and parent_entry[0] == "file": + raise ValueError("source archive places content below a regular file") + expected_entries.setdefault(parent_name, ("directory", 0)) + parent = parent.parent + + previous_entry = expected_entries.get(normalized) + if previous_entry is not None and ( + entry[0] == "file" or previous_entry[0] != "directory" + ): + raise ValueError("source archive contains a file-directory collision") + expected_entries[normalized] = entry + members.append(member) + + if not members: + raise ValueError("source archive must contain at least one member") + for directory in declared_directories: + prefix = f"{directory}/" + if not any( + path != directory and path.startswith(prefix) + for path in declared_paths + ): + raise ValueError("source archive contains an empty gitlink-like directory") + return members, expected_entries + + +def _verify_materialized_snapshot( + snapshot: Path, + expected_entries: dict[str, SourceArchiveEntry], +) -> None: + """Verify extracted paths, types, and sizes before the Docker bind mount.""" + observed_entries: dict[str, SourceArchiveEntry] = {} + for extracted_path in snapshot.rglob("*"): + relative_path = extracted_path.relative_to(snapshot).as_posix() + metadata = os.lstat(extracted_path) + if stat.S_ISLNK(metadata.st_mode) or not ( + stat.S_ISDIR(metadata.st_mode) or stat.S_ISREG(metadata.st_mode) + ): + raise ValueError("source snapshot contains a non-regular extracted entry") + observed_entries[relative_path] = ( + ("directory", 0) + if stat.S_ISDIR(metadata.st_mode) + else ("file", metadata.st_size) + ) + if observed_entries != expected_entries: + raise ValueError("source snapshot does not match the validated archive") + + +def _materialize_committed_source( + source: Path, + head_sha: str, + staging_root: Path, + metadata_kind: GitMetadataKind, +) -> Path: + """Materialize one private exact-commit snapshot without local Git controls.""" + archive_path = staging_root / "source.tar" + snapshot = staging_root / "source" + snapshot.mkdir(mode=0o700) + try: + control = _create_isolated_git_control( + source, + head_sha, + staging_root, + metadata_kind, + require_object_directory=True, + ) + except RuntimeError as exc: + raise RuntimeError("source commit snapshot could not be materialized") from exc + try: + _verify_exact_tree_limits(control, head_sha) + except RuntimeError as exc: + raise RuntimeError( + "source commit snapshot could not be materialized safely" + ) from exc + completed = subprocess.run( + [ + TRUSTED_GIT_EXECUTABLE, + "-c", + "core.hooksPath=/dev/null", + "-c", + "core.fsmonitor=false", + f"--git-dir={control}", + "archive", + "--format=tar", + f"--output={archive_path}", + head_sha, + ], + text=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + shell=False, + timeout=GIT_STREAM_TIMEOUT_SECONDS, + env=_isolated_git_environment(), + ) + if completed.returncode != 0: + raise RuntimeError("source commit snapshot could not be materialized") + try: + with tarfile.open(archive_path, mode="r:") as archive: + members, expected_entries = _validated_source_archive_members(archive) + archive.extractall(snapshot, members=members, filter="data") + _verify_materialized_snapshot(snapshot, expected_entries) + except (OSError, ValueError, tarfile.TarError) as exc: + raise RuntimeError( + "source commit snapshot could not be materialized safely" + ) from exc + finally: + archive_path.unlink(missing_ok=True) + + metadata_placeholder = snapshot / ".git" + if metadata_kind == "directory": + metadata_placeholder.mkdir(mode=0o700) + else: + metadata_placeholder.touch(mode=0o400) + return _validated_docker_mount_path(snapshot, "source snapshot") + + +def _create_git_metadata_mask( + staging_root: Path, + metadata_kind: GitMetadataKind | None, +) -> Path | None: + """Create an empty nested bind source that hides checkout control metadata.""" + if metadata_kind is None: + return None + metadata_mask = staging_root / "git-metadata-mask" + if metadata_kind == "directory": + metadata_mask.mkdir(mode=0o700) + else: + metadata_mask.touch(mode=0o400) + return metadata_mask + + +def _write_private_patch_copy(directory: Path, patch_bytes: bytes) -> Path: + """Create one owner-only immutable-by-policy patch copy for the bind mount.""" + staged_patch = directory / "input.patch" + staged_patch.write_bytes(patch_bytes) + staged_patch.chmod(0o400) + return staged_patch + + +def _read_result_payload( + result_path: Path, + _completed: subprocess.CompletedProcess[str] | None = None, + *, + file_system: Any = DEFAULT_PATCH_FILE_SYSTEM, +) -> bytes: + """Return evidence only from the descriptor-safe 16 KiB result file.""" + _resolved, result_bytes = _read_regular_patch( + result_path, + file_system=file_system, + maximum_bytes=MAX_RESULT_JSON_BYTES, + label="patch validation result", + ) + return result_bytes + + +class DockerPatchValidationRunner: + """Run one exact-bound patch through a hardened, no-network Docker profile.""" + + def __init__( + self, + *, + command_runner: ProcessRunner = subprocess.run, + cleanup_runner: ProcessRunner = subprocess.run, + name_factory: NameFactory = _default_name, + file_system: Any = DEFAULT_PATCH_FILE_SYSTEM, + ) -> None: + """Initialize injectable process, cleanup, name, and filesystem adapters.""" + self._command_runner = command_runner + self._cleanup_runner = cleanup_runner + self._name_factory = name_factory + self._file_system = file_system + + def validate( + self, + *, + request: PatchValidationRequest, + source_root: str | Path, + patch_path: str | Path, + ) -> PatchValidationResult: + """Validate one patch and return exact-request-bound structured evidence.""" + source = _validated_directory(source_root, "source root") + _resolved_patch, patch_bytes = _read_regular_patch( + patch_path, + file_system=self._file_system, + ) + inspect_patch_bytes(patch_bytes) + observed_digest = hashlib.sha256(patch_bytes).hexdigest() + if observed_digest != request.patch_sha256: + raise RuntimeError( + "patch file digest does not match the validation request" + ) + image = _verified_image_reference() + metadata_kind = _git_metadata_kind(source) + _verify_source_head(source, request.head_sha, metadata_kind) + if metadata_kind is None: + raise RuntimeError("source Git metadata is required for exact-head validation") + container_name = self._name_factory() + uid = os.getuid() + gid = os.getgid() + if uid <= 0 or gid <= 0: + raise RuntimeError("patch validation requires a non-root runner UID and GID") + child_environment = {"PATH": os.environ.get("PATH", os.defpath)} + + with tempfile.TemporaryDirectory(prefix="noema-patch-validation-") as staging: + staging_root = _validated_docker_mount_path(Path(staging), "staging root") + source_mount = _materialize_committed_source( + source, + request.head_sha, + staging_root, + metadata_kind, + ) + staged_patch = _write_private_patch_copy(staging_root, patch_bytes) + git_metadata_mask = _create_git_metadata_mask(staging_root, metadata_kind) + result_path = staging_root / "result.json" + result_path.touch(mode=0o600) + git_metadata_mount = ( + [] + if git_metadata_mask is None + else [ + "--mount=type=bind," + f"src={git_metadata_mask},dst=/input/.git,readonly" + ] + ) + command = [ + "docker", + "run", + "--rm", + f"--name={container_name}", + "--pull=never", + "--network=none", + "--read-only", + "--cap-drop=ALL", + "--security-opt=no-new-privileges=true", + "--security-opt=seccomp=builtin", + "--pids-limit=256", + "--memory=2g", + "--memory-swap=2g", + "--cpus=2", + "--ipc=none", + "--ulimit=nofile=1024:1024", + "--ulimit=nproc=256:256", + "--ulimit=core=0:0", + ( + "--ulimit=fsize=" + f"{MAX_SOURCE_ARCHIVE_FILE_BYTES}:" + f"{MAX_SOURCE_ARCHIVE_FILE_BYTES}" + ), + f"--user={uid}:{gid}", + ( + "--tmpfs=/workspace:" + f"rw,nosuid,nodev,size=1073741824,mode=0700,uid={uid},gid={gid}" + ), + "--tmpfs=/tmp:rw,noexec,nosuid,nodev,size=67108864,mode=1777", + f"--mount=type=bind,src={source_mount},dst=/input,readonly", + *git_metadata_mount, + ( + "--mount=type=bind," + f"src={staged_patch},dst=/patch/input.patch,readonly" + ), + ( + "--mount=type=bind," + f"src={result_path},dst=/output/result.json" + ), + "--workdir=/workspace", + "--env=HOME=/workspace/home", + "--env=XDG_CACHE_HOME=/workspace/cache", + "--env=NOEMA_RESULT_PATH=/output/result.json", + f"--env=NOEMA_REPOSITORY={request.repository_full_name}", + f"--env=NOEMA_BASE_SHA={request.base_sha}", + f"--env=NOEMA_HEAD_SHA={request.head_sha}", + f"--env=NOEMA_PATCH_SHA256={request.patch_sha256}", + f"--env=NOEMA_PATCH_PROFILE={request.profile.value}", + "--entrypoint=/opt/noema/bin/validate-patch", + image, + ] + try: + completed = self._command_runner( + command, + text=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + shell=False, + timeout=PATCH_SANDBOX_WALL_TIMEOUT_SECONDS, + env=child_environment, + ) + except subprocess.TimeoutExpired as exc: + self._cleanup_runner( + ["docker", "rm", "-f", container_name], + text=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=False, + shell=False, + timeout=30, + env=child_environment, + ) + raise RuntimeError( + "patch validation sandbox timed out after " + f"{PATCH_SANDBOX_WALL_TIMEOUT_SECONDS} seconds" + ) from exc + except OSError as exc: + raise RuntimeError( + f"patch validation sandbox could not start Docker: {exc}" + ) from exc + + if completed.returncode != 0: + stderr = getattr(completed, "stderr", "") or "" + stdout = getattr(completed, "stdout", "") or "" + detail = _bounded_detail(stderr or stdout) + raise RuntimeError( + f"patch validation sandbox exited {completed.returncode}: {detail}" + ) + result_payload = _read_result_payload( + result_path, + completed, + file_system=self._file_system, + ) + try: + result = PatchValidationResult.model_validate_json(result_payload) + except (ValidationError, ValueError) as exc: + raise RuntimeError( + "patch validation sandbox returned invalid structured evidence" + ) from exc + if not _result_matches_request(result, request): + raise RuntimeError( + "patch validation sandbox result does not match the request" + ) + return result diff --git a/reviewer/tests/test_patch_validation.py b/reviewer/tests/test_patch_validation.py new file mode 100644 index 00000000..373be401 --- /dev/null +++ b/reviewer/tests/test_patch_validation.py @@ -0,0 +1,717 @@ +"""Tests for credential-free, allowlisted patch validation.""" + +from __future__ import annotations + +import hashlib +import os +import re +import subprocess +from pathlib import Path +from types import SimpleNamespace + +import pytest +from pydantic import ValidationError + +from noema_reviewer import patch_validation +from noema_reviewer.patch_validation import ( + DockerPatchValidationRunner, + PatchValidationProfile, + PatchValidationRequest, + PatchValidationResult, + PatchValidationStatus, + inspect_patch_bytes, +) + + +TEST_IMAGE = ( + f"{patch_validation.TRUSTED_PATCH_IMAGE_REPOSITORY}" + f"@sha256:{'a' * 64}" +) +BASE_SHA = "1" * 40 +HEAD_SHA = "2" * 40 + + +def _patch(content: str = "+safe change\n") -> bytes: + """Return a minimal text-only Git patch for one permitted source file.""" + return ( + "diff --git a/src/example.ts b/src/example.ts\n" + "index 1111111..2222222 100644\n" + "--- a/src/example.ts\n" + "+++ b/src/example.ts\n" + "@@ -1 +1 @@\n" + "-old value\n" + f"{content}" + ).encode() + + +def _request( + patch_bytes: bytes, + *, + head_sha: str = HEAD_SHA, +) -> PatchValidationRequest: + """Build a request bound to the exact test patch and commit identities.""" + return PatchValidationRequest( + repository_full_name="ContextualWisdomLab/noema", + base_sha=BASE_SHA, + head_sha=head_sha, + patch_sha256=hashlib.sha256(patch_bytes).hexdigest(), + profile=PatchValidationProfile.NODE_RELEASE_VERIFY, + ) + + +def _run_git(source: Path, *arguments: str) -> str: + """Run one deterministic non-shell Git command for a test repository.""" + completed = subprocess.run( + [patch_validation.TRUSTED_GIT_EXECUTABLE, "-C", str(source), *arguments], + check=True, + shell=False, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=30, + ) + return completed.stdout.strip() + + +def _write_inputs(tmp_path: Path, patch_bytes: bytes) -> tuple[Path, Path, str]: + """Create an authenticated clean Git source and regular patch file.""" + source = tmp_path / "source" + source.mkdir() + _run_git(source, "init", "-q") + _run_git(source, "config", "user.email", "test@example.invalid") + _run_git(source, "config", "user.name", "Noema Test") + source_file = source / "src" / "example.ts" + source_file.parent.mkdir() + source_file.write_text("old value\n", encoding="utf-8") + _run_git(source, "add", "src/example.ts") + _run_git(source, "commit", "-qm", "fixture") + head_sha = _run_git(source, "rev-parse", "HEAD") + patch_path = tmp_path / "proposal.patch" + patch_path.write_bytes(patch_bytes) + return source, patch_path, head_sha + + +def _result_json(request: PatchValidationRequest) -> str: + """Return one exact-binding successful container result.""" + return PatchValidationResult( + status=PatchValidationStatus.PASSED, + repository_full_name=request.repository_full_name, + base_sha=request.base_sha, + head_sha=request.head_sha, + patch_sha256=request.patch_sha256, + profile=request.profile, + command_profile="npm run release:verify", + exit_code=0, + duration_ms=42, + stdout_excerpt="all tests passed", + stderr_excerpt="", + reason_codes=[], + ).model_dump_json() + + +def _mount_source(command: list[str], destination: str) -> Path: + """Return the host source for one exact Docker bind destination.""" + suffix = f",dst={destination}" + mount = next( + argument + for argument in command + if argument.startswith("--mount=") and suffix in argument + ) + return Path(mount.split("src=", 1)[1].split(",dst=", 1)[0]) + + +def _write_container_result(command: list[str], payload: str) -> None: + """Write structured evidence through the production single-file channel.""" + _mount_source(command, "/output/result.json").write_text( + payload, + encoding="utf-8", + ) + + +def _metadata( + *, + mode: int | None = None, + size: int = 4, + device: int = 11, + inode: int = 13, +): + """Return synthetic stat metadata for descriptor-race tests.""" + return SimpleNamespace( + st_mode=patch_validation.stat.S_IFREG | 0o600 if mode is None else mode, + st_size=size, + st_dev=device, + st_ino=inode, + ) + + +def _file_system(**overrides): + """Return injectable patch filesystem operations with deterministic reads.""" + chunks = iter([b"safe", b""]) + defaults = { + "lstat": lambda _path: _metadata(), + "open": lambda _path, _flags: 7, + "fstat": lambda _descriptor: _metadata(), + "read": lambda _descriptor, _size: next(chunks), + "close": lambda _descriptor: None, + } + defaults.update(overrides) + return SimpleNamespace(**defaults) + + +def test_request_rejects_ambiguous_identity_and_arbitrary_profile() -> None: + """Repository, commit, digest, and test profile are closed wire contracts.""" + patch_bytes = _patch() + valid = _request(patch_bytes) + assert valid.profile is PatchValidationProfile.NODE_RELEASE_VERIFY + + invalid_cases = ( + {"repository_full_name": "single-component"}, + {"base_sha": "ABC"}, + {"head_sha": "f" * 39}, + {"patch_sha256": "0" * 63}, + {"profile": "bash -lc 'curl attacker.invalid'"}, + ) + for override in invalid_cases: + values = valid.model_dump() + values.update(override) + with pytest.raises(ValidationError): + PatchValidationRequest.model_validate(values) + + +def test_patch_inspector_accepts_bounded_regular_source_patch() -> None: + """A text-only source change yields the normalized changed-path tuple.""" + assert inspect_patch_bytes(_patch()) == ("src/example.ts",) + quoted = b'diff --git "a/src/file name.ts" "b/src/file name.ts"\n' + assert inspect_patch_bytes(quoted) == ("src/file name.ts",) + + +@pytest.mark.parametrize( + ("patch_bytes", "message"), + ( + (b"", "must not be empty"), + (b"\xff", "valid UTF-8"), + ( + b"diff --git a/link b/link\nnew file mode 120000\n", + "symlink or gitlink", + ), + ( + b"diff --git a/submodule b/submodule\ndeleted file mode 160000\n", + "symlink or gitlink", + ), + ( + b"diff --git a/.github/workflows/pwn.yml b/.github/workflows/pwn.yml\n", + "forbidden path", + ), + ( + b"diff --git a/.github/actions/pwn/action.yml b/.github/actions/pwn/action.yml\n", + "forbidden path", + ), + ( + b"diff --git a/.git/config b/.git/config\n", + "forbidden path", + ), + ( + b"diff --git a/.gitmodules b/.gitmodules\n", + "forbidden path", + ), + ( + b"diff --git a/../outside b/../outside\n", + "unsafe repository path", + ), + ( + b"diff --git a//absolute b//absolute\n", + "unsafe repository path", + ), + ( + b"diff --git a/src\\evil b/src\\evil\n", + "unsafe repository path", + ), + ( + b"diff --git a/src/\x01evil b/src/\x01evil\n", + "unsafe repository path", + ), + ( + b"diff --git a/src/a.bin b/src/a.bin\nGIT binary patch\n", + "binary patch", + ), + ( + b"diff --git a/src/a.bin b/src/a.bin\nBinary files differ\n", + "binary patch", + ), + (b"ordinary text only\n", "no diff headers"), + (b'diff --git "a/src/x b/src/x\n', "malformed diff header"), + (b"diff --git a/src/x\n", "malformed diff header"), + (b"diff --git c/src/x b/src/x\n", "malformed diff path"), + (b"diff --git a/ b/\n", "unsafe repository path"), + ), +) +def test_patch_inspector_rejects_unsafe_patch_shapes( + patch_bytes: bytes, + message: str, +) -> None: + """Malformed text, modes, paths, governance files, and binaries fail closed.""" + with pytest.raises(ValueError, match=message): + inspect_patch_bytes(patch_bytes) + + +def test_patch_inspector_rejects_size_duplicates_and_file_count() -> None: + """Patch bytes, duplicate paths, and file cardinality have explicit limits.""" + with pytest.raises(ValueError, match="exceeds"): + inspect_patch_bytes(b"x" * (patch_validation.MAX_PATCH_BYTES + 1)) + + duplicate = ( + b"diff --git a/src/x b/src/x\n" + b"diff --git a/src/x b/src/x\n" + ) + with pytest.raises(ValueError, match="repeats changed path"): + inspect_patch_bytes(duplicate) + + many = b"".join( + f"diff --git a/src/f{index} b/src/f{index}\n".encode() + for index in range(patch_validation.MAX_CHANGED_FILES + 1) + ) + with pytest.raises(ValueError, match="more than"): + inspect_patch_bytes(many) + + +def test_internal_diagnostics_and_names_are_bounded_and_unique() -> None: + """Infrastructure helpers emit deterministic bounds and Docker-safe names.""" + assert patch_validation._bounded_detail("") == "no diagnostic output" + assert patch_validation._bounded_detail(" short ") == "short" + first = patch_validation._default_name() + second = patch_validation._default_name() + assert re.fullmatch(r"noema-patch-[0-9a-f]{32}", first) + assert first != second + + +@pytest.mark.parametrize("kind", ["missing", "file", "unsafe"]) +def test_runner_rejects_invalid_source_mount(tmp_path, monkeypatch, kind: str) -> None: + """Missing, non-directory, and Docker-ambiguous source roots fail closed.""" + patch_bytes = _patch() + request = _request(patch_bytes) + patch_path = tmp_path / "proposal.patch" + patch_path.write_bytes(patch_bytes) + if kind == "missing": + source = tmp_path / "missing" + message = "unavailable" + elif kind == "file": + source = tmp_path / "source-file" + source.write_text("x", encoding="utf-8") + message = "must be a directory" + else: + source = tmp_path / "unsafe,source" + source.mkdir() + message = "unsafe for a Docker mount" + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + + with pytest.raises(RuntimeError, match=message): + DockerPatchValidationRunner().validate( + request=request, + source_root=source, + patch_path=patch_path, + ) + + +def test_descriptor_safe_patch_reader_rejects_invalid_metadata(tmp_path) -> None: + """Every pre-open and post-open patch metadata anomaly is rejected.""" + path = tmp_path / "proposal.patch" + path.write_bytes(b"safe") + + cases = ( + ( + _file_system(lstat=lambda _path: (_ for _ in ()).throw(FileNotFoundError("gone"))), + "unavailable", + ), + ( + _file_system(lstat=lambda _path: _metadata(mode=patch_validation.stat.S_IFDIR)), + "regular non-symlink", + ), + ( + _file_system(lstat=lambda _path: _metadata(size=0)), + "must not be empty", + ), + ( + _file_system( + lstat=lambda _path: _metadata(size=patch_validation.MAX_PATCH_BYTES + 1) + ), + "exceeds", + ), + ( + _file_system(fstat=lambda _descriptor: _metadata(mode=patch_validation.stat.S_IFDIR)), + "changed during validation", + ), + ( + _file_system(fstat=lambda _descriptor: _metadata(device=99)), + "changed during validation", + ), + ( + _file_system(fstat=lambda _descriptor: _metadata(inode=99)), + "changed during validation", + ), + ) + for file_system, message in cases: + with pytest.raises(RuntimeError, match=message): + patch_validation._read_regular_patch(path, file_system=file_system) + + +def test_descriptor_safe_patch_reader_bounds_reads_and_closes(tmp_path) -> None: + """Read growth, empty descriptors, and I/O errors close assigned descriptors.""" + path = tmp_path / "proposal.patch" + path.write_bytes(b"safe") + closed: list[int] = [] + + oversized_chunks = iter([b"x" * (patch_validation.MAX_PATCH_BYTES + 1)]) + oversized = _file_system( + read=lambda _descriptor, _size: next(oversized_chunks), + close=lambda descriptor: closed.append(descriptor), + ) + with pytest.raises(RuntimeError, match="exceeds"): + patch_validation._read_regular_patch(path, file_system=oversized) + assert closed == [7] + + closed.clear() + empty = _file_system( + read=lambda _descriptor, _size: b"", + close=lambda descriptor: closed.append(descriptor), + ) + with pytest.raises(RuntimeError, match="must not be empty"): + patch_validation._read_regular_patch(path, file_system=empty) + assert closed == [7] + + closed.clear() + read_error = _file_system( + read=lambda _descriptor, _size: (_ for _ in ()).throw(OSError("read failed")), + close=lambda descriptor: closed.append(descriptor), + ) + with pytest.raises(RuntimeError, match="could not be read safely"): + patch_validation._read_regular_patch(path, file_system=read_error) + assert closed == [7] + + close_calls: list[int] = [] + open_error = _file_system( + open=lambda _path, _flags: (_ for _ in ()).throw(OSError("open failed")), + close=lambda descriptor: close_calls.append(descriptor), + ) + with pytest.raises(RuntimeError, match="could not be read safely"): + patch_validation._read_regular_patch(path, file_system=open_error) + assert close_calls == [] + + +def test_descriptor_safe_patch_reader_returns_exact_bytes(tmp_path) -> None: + """A stable descriptor returns its exact bytes and is always closed.""" + path = tmp_path / "proposal.patch" + path.write_bytes(b"safe") + chunks = iter([b"sa", b"fe", b""]) + closed: list[int] = [] + file_system = _file_system( + read=lambda _descriptor, _size: next(chunks), + close=lambda descriptor: closed.append(descriptor), + ) + resolved, data = patch_validation._read_regular_patch( + path, + file_system=file_system, + ) + assert resolved == path.absolute() + assert data == b"safe" + assert closed == [7] + + +def test_runner_launches_exact_hardened_profile_without_parent_secrets( + tmp_path, + monkeypatch, +) -> None: + """The model patch runs in one immutable, networkless, credential-free image.""" + patch_bytes = _patch() + source, patch_path, head_sha = _write_inputs(tmp_path, patch_bytes) + request = _request(patch_bytes, head_sha=head_sha) + calls: list[tuple[list[str], dict[str, object]]] = [] + + def fake_run(args, **kwargs): + """Capture private mounts and write exact-bound file evidence.""" + command = list(args) + source_snapshot = _mount_source(command, "/input,readonly") + assert source_snapshot != source.resolve() + assert (source_snapshot / "src" / "example.ts").read_text( + encoding="utf-8" + ) == "old value\n" + _write_container_result(command, _result_json(request)) + calls.append((command, kwargs)) + return SimpleNamespace(returncode=0, stdout="", stderr="") + + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + monkeypatch.setenv("GH_TOKEN", "github-secret") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "model-secret") + monkeypatch.setenv("NVIDIA_NIM_API_KEY", "nim-secret") + monkeypatch.setenv("PATH", "/trusted/bin") + runner = DockerPatchValidationRunner( + command_runner=fake_run, + cleanup_runner=fake_run, + name_factory=lambda: "fixed-patch-validator", + ) + + result = runner.validate( + request=request, + source_root=source, + patch_path=patch_path, + ) + + assert result.status is PatchValidationStatus.PASSED + assert result.patch_sha256 == request.patch_sha256 + assert len(calls) == 1 + command, kwargs = calls[0] + assert command[:3] == ["docker", "run", "--rm"] + for required in ( + "--pull=never", + "--network=none", + "--read-only", + "--cap-drop=ALL", + "--security-opt=no-new-privileges=true", + "--security-opt=seccomp=builtin", + "--pids-limit=256", + "--memory=2g", + "--memory-swap=2g", + "--cpus=2", + "--ipc=none", + "--entrypoint=/opt/noema/bin/validate-patch", + ): + assert required in command + patch_mount = next( + part + for part in command + if part.startswith("--mount=") and ",dst=/patch/input.patch,readonly" in part + ) + assert str(patch_path.resolve()) not in patch_mount + destinations = tuple( + part.split(",dst=", 1)[1].split(",", 1)[0] + for part in command + if part.startswith("--mount=") and ",dst=" in part + ) + assert "/output" not in destinations + assert destinations.count("/output/result.json") == 1 + assert f"--env=NOEMA_REPOSITORY={request.repository_full_name}" in command + assert f"--env=NOEMA_BASE_SHA={request.base_sha}" in command + assert f"--env=NOEMA_HEAD_SHA={request.head_sha}" in command + assert f"--env=NOEMA_PATCH_SHA256={request.patch_sha256}" in command + assert "--env=NOEMA_PATCH_PROFILE=node_release_verify" in command + assert command[-1] == TEST_IMAGE + assert kwargs["shell"] is False + assert kwargs["timeout"] == patch_validation.PATCH_SANDBOX_WALL_TIMEOUT_SECONDS + assert kwargs["env"] == {"PATH": "/trusted/bin"} + assert kwargs["stdout"] is subprocess.DEVNULL + assert kwargs["stderr"] is subprocess.DEVNULL + assert not any("docker.sock" in part for part in command) + assert "github-secret" not in repr((command, kwargs)) + assert "model-secret" not in repr((command, kwargs)) + assert "nim-secret" not in repr((command, kwargs)) + + +def test_runner_rejects_patch_digest_mismatch_before_docker(tmp_path, monkeypatch) -> None: + """A substituted patch never reaches the container runtime.""" + patch_bytes = _patch() + source, patch_path, head_sha = _write_inputs(tmp_path, patch_bytes) + request = _request(patch_bytes, head_sha=head_sha) + patch_path.write_bytes(_patch("+substituted\n")) + called = False + + def should_not_run(_args, **_kwargs): + """Record an erroneous attempt to start Docker.""" + nonlocal called + called = True + raise AssertionError("Docker must not start") + + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + runner = DockerPatchValidationRunner(command_runner=should_not_run) + with pytest.raises(RuntimeError, match="digest does not match"): + runner.validate(request=request, source_root=source, patch_path=patch_path) + assert called is False + + +def test_runner_rejects_symlink_patch_before_read(tmp_path, monkeypatch) -> None: + """A symlink cannot redirect patch validation to an attacker-selected file.""" + patch_bytes = _patch() + source, _unused_patch_path, head_sha = _write_inputs(tmp_path, patch_bytes) + request = _request(patch_bytes, head_sha=head_sha) + target = tmp_path / "target.patch" + target.write_bytes(patch_bytes) + patch_path = tmp_path / "symlink-proposal.patch" + patch_path.symlink_to(target) + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + + with pytest.raises(RuntimeError, match="regular non-symlink"): + DockerPatchValidationRunner().validate( + request=request, + source_root=source, + patch_path=patch_path, + ) + + +def test_runner_rejects_unverified_image(tmp_path, monkeypatch) -> None: + """A mutable or foreign image reference cannot replace the reviewed sandbox.""" + patch_bytes = _patch() + source, patch_path, head_sha = _write_inputs(tmp_path, patch_bytes) + request = _request(patch_bytes, head_sha=head_sha) + + for invalid in ( + "", + "ghcr.io/contextualwisdomlab/noema-patch-validator:latest", + f"docker.io/library/node@sha256:{'a' * 64}", + ): + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", invalid) + with pytest.raises(RuntimeError, match="verified immutable"): + DockerPatchValidationRunner().validate( + request=request, + source_root=source, + patch_path=patch_path, + ) + + +def test_runner_rejects_container_result_bound_to_another_head( + tmp_path, + monkeypatch, +) -> None: + """A structurally valid result for another revision is artifact substitution.""" + patch_bytes = _patch() + source, patch_path, head_sha = _write_inputs(tmp_path, patch_bytes) + request = _request(patch_bytes, head_sha=head_sha) + mismatched = PatchValidationResult.model_validate_json(_result_json(request)) + mismatched.head_sha = "3" * 40 + + def fake_run(args, **_kwargs): + """Write a result whose head binding differs from the request.""" + _write_container_result(list(args), mismatched.model_dump_json()) + return SimpleNamespace(returncode=0, stdout="", stderr="") + + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + with pytest.raises(RuntimeError, match="does not match the request"): + DockerPatchValidationRunner(command_runner=fake_run).validate( + request=request, + source_root=source, + patch_path=patch_path, + ) + + +def test_runner_rejects_invalid_structured_evidence_and_missing_docker( + tmp_path, + monkeypatch, +) -> None: + """Malformed file evidence and a missing Docker client become visible failures.""" + patch_bytes = _patch() + source, patch_path, head_sha = _write_inputs(tmp_path, patch_bytes) + request = _request(patch_bytes, head_sha=head_sha) + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + + def invalid_json(args, **_kwargs): + """Write invalid structured evidence through the bounded file channel.""" + _write_container_result(list(args), "not-json") + return SimpleNamespace(returncode=0, stdout="", stderr="") + + with pytest.raises(RuntimeError, match="invalid structured evidence"): + DockerPatchValidationRunner(command_runner=invalid_json).validate( + request=request, + source_root=source, + patch_path=patch_path, + ) + + def missing_docker(_args, **_kwargs): + """Simulate an unavailable Docker client.""" + raise FileNotFoundError("docker missing") + + with pytest.raises(RuntimeError, match="could not start Docker"): + DockerPatchValidationRunner(command_runner=missing_docker).validate( + request=request, + source_root=source, + patch_path=patch_path, + ) + + +def test_runner_cleans_up_timed_out_container(tmp_path, monkeypatch) -> None: + """A host wall timeout force-removes the unpredictable container name.""" + patch_bytes = _patch() + source, patch_path, head_sha = _write_inputs(tmp_path, patch_bytes) + request = _request(patch_bytes, head_sha=head_sha) + cleanup_calls: list[list[str]] = [] + + def timed_out(args, **kwargs): + """Simulate a validator exceeding the host wall-clock budget.""" + raise subprocess.TimeoutExpired(args, kwargs["timeout"]) + + def cleanup(args, **_kwargs): + """Capture forced removal of the timed-out sandbox.""" + cleanup_calls.append(list(args)) + return SimpleNamespace(returncode=0, stdout="", stderr="") + + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + runner = DockerPatchValidationRunner( + command_runner=timed_out, + cleanup_runner=cleanup, + name_factory=lambda: "timed-out-patch-validator", + ) + + with pytest.raises(RuntimeError, match="timed out"): + runner.validate(request=request, source_root=source, patch_path=patch_path) + assert cleanup_calls == [ + ["docker", "rm", "-f", "timed-out-patch-validator"], + ] + + +@pytest.mark.parametrize( + ("stdout", "stderr", "expected"), + ( + ("", "x" * 5000, "truncated"), + ("", "", "no diagnostic output"), + ("stdout failure", "", "stdout failure"), + ), +) +def test_runner_bounds_nonzero_container_diagnostic( + tmp_path, + monkeypatch, + stdout: str, + stderr: str, + expected: str, +) -> None: + """Attacker-controlled or silent process diagnostics remain bounded.""" + patch_bytes = _patch() + source, patch_path, head_sha = _write_inputs(tmp_path, patch_bytes) + request = _request(patch_bytes, head_sha=head_sha) + + def failed(_args, **_kwargs): + """Return the selected non-zero sandbox diagnostic.""" + return SimpleNamespace(returncode=9, stdout=stdout, stderr=stderr) + + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + with pytest.raises(RuntimeError) as captured: + DockerPatchValidationRunner(command_runner=failed).validate( + request=request, + source_root=source, + patch_path=patch_path, + ) + assert "exited 9" in str(captured.value) + assert expected in str(captured.value) + assert len(str(captured.value)) < 1500 + + +def test_runner_uses_default_path_when_parent_path_is_absent( + tmp_path, + monkeypatch, +) -> None: + """Docker receives only a deterministic PATH even when the parent lacks one.""" + patch_bytes = _patch() + source, patch_path, head_sha = _write_inputs(tmp_path, patch_bytes) + request = _request(patch_bytes, head_sha=head_sha) + observed: dict[str, object] = {} + + def successful(args, **kwargs): + """Capture the child environment and write bounded file evidence.""" + observed.update(kwargs) + _write_container_result(list(args), _result_json(request)) + return SimpleNamespace(returncode=0, stdout="", stderr="") + + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + monkeypatch.delenv("PATH", raising=False) + result = DockerPatchValidationRunner(command_runner=successful).validate( + request=request, + source_root=source, + patch_path=patch_path, + ) + assert result.status is PatchValidationStatus.PASSED + assert observed["env"] == {"PATH": os.defpath} diff --git a/reviewer/tests/test_patch_validation_archive_boundaries.py b/reviewer/tests/test_patch_validation_archive_boundaries.py new file mode 100644 index 00000000..da812d55 --- /dev/null +++ b/reviewer/tests/test_patch_validation_archive_boundaries.py @@ -0,0 +1,335 @@ +"""Adversarial archive-boundary regressions for exact source snapshots.""" + +from __future__ import annotations + +import io +import stat +import tarfile +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from noema_reviewer import patch_validation + + +def _archive_runner( + entries: list[tuple[tarfile.TarInfo, bytes | None]], +): + """Return a fake Git runner for exact-tree preflight and one tar archive.""" + + def run(command, **_kwargs): + """Expose a bounded tree, then write the requested controlled archive.""" + command_list = list(command) + if "ls-tree" in command_list: + return SimpleNamespace( + returncode=0, + stdout=f"100644 blob {'a' * 40} 1\tfixture.txt\0", + ) + if "archive" not in command_list: + raise AssertionError(f"unexpected Git command: {command_list}") + output = next( + argument.removeprefix("--output=") + for argument in command_list + if argument.startswith("--output=") + ) + with tarfile.open(output, mode="w") as archive: + for member, payload in entries: + archive.addfile( + member, + None if payload is None else io.BytesIO(payload), + ) + return SimpleNamespace(returncode=0) + + return run + + +def _regular_member(name: str, payload: bytes) -> tuple[tarfile.TarInfo, bytes]: + """Build one regular archive member with an exact declared size.""" + member = tarfile.TarInfo(name) + member.size = len(payload) + member.mode = 0o640 + return member, payload + + +def _directory_member(name: str) -> tuple[tarfile.TarInfo, None]: + """Build one explicit archive directory member.""" + member = tarfile.TarInfo(name) + member.type = tarfile.DIRTYPE + member.mode = 0o750 + return member, None + + +def _materialize( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + entries: list[tuple[tarfile.TarInfo, bytes | None]], +) -> Path: + """Materialize one controlled archive through the production boundary.""" + staging = tmp_path / "staging" + staging.mkdir() + isolated_control = staging / "isolated-control" + isolated_control.mkdir() + monkeypatch.setattr( + patch_validation, + "_create_isolated_git_control", + lambda *_args, **_kwargs: isolated_control, + ) + monkeypatch.setattr( + patch_validation, + "_verify_exact_tree_limits", + lambda *_args, **_kwargs: None, + ) + monkeypatch.setattr( + patch_validation.subprocess, + "run", + _archive_runner(entries), + ) + return patch_validation._materialize_committed_source( + tmp_path, + "2" * 40, + staging, + "directory", + ) + + +@pytest.mark.parametrize( + ("member_type", "link_name"), + [ + (tarfile.SYMTYPE, "target.txt"), + (tarfile.LNKTYPE, "target.txt"), + (tarfile.FIFOTYPE, ""), + (tarfile.CHRTYPE, ""), + (tarfile.BLKTYPE, ""), + ], +) +def test_snapshot_rejects_non_regular_archive_members( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + member_type: bytes, + link_name: str, +) -> None: + """Links, devices, and FIFOs cannot enter the Docker-mounted snapshot.""" + member = tarfile.TarInfo("unsafe-entry") + member.type = member_type + member.linkname = link_name + + with pytest.raises(RuntimeError, match="materialized safely"): + _materialize(tmp_path, monkeypatch, [(member, None)]) + + +@pytest.mark.parametrize( + "unsafe_name", + [ + "../escape.txt", + "/absolute.txt", + "unsafe\\name.txt", + "control\nname.txt", + ".git/config", + ], +) +def test_snapshot_rejects_unsafe_archive_member_names( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + unsafe_name: str, +) -> None: + """Archive names must remain normalized repository-relative POSIX paths.""" + with pytest.raises(RuntimeError, match="materialized safely"): + _materialize( + tmp_path, + monkeypatch, + [_regular_member(unsafe_name, b"unsafe")], + ) + + +def test_snapshot_rejects_empty_archive( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An empty source archive cannot be treated as usable committed source.""" + with pytest.raises(RuntimeError, match="materialized safely"): + _materialize(tmp_path, monkeypatch, []) + + +def test_snapshot_rejects_duplicate_archive_member_names( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Duplicate names cannot overwrite earlier validated archive entries.""" + duplicate = _regular_member("src/example.txt", b"first") + replacement = _regular_member("src/example.txt", b"second") + + with pytest.raises(RuntimeError, match="materialized safely"): + _materialize(tmp_path, monkeypatch, [duplicate, replacement]) + + +def test_snapshot_rejects_content_below_regular_file( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An archive cannot place a child beneath a path already declared as a file.""" + with pytest.raises(RuntimeError, match="materialized safely"): + _materialize( + tmp_path, + monkeypatch, + [ + _regular_member("parent", b"file"), + _regular_member("parent/child.txt", b"child"), + ], + ) + + +def test_snapshot_rejects_implicit_directory_replaced_by_file( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A later file cannot replace an implicit directory created by a child path.""" + with pytest.raises(RuntimeError, match="materialized safely"): + _materialize( + tmp_path, + monkeypatch, + [ + _regular_member("parent/child.txt", b"child"), + _regular_member("parent", b"file"), + ], + ) + + +def test_snapshot_rejects_leaf_directory_gitlink_shape( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A leaf directory entry is rejected as an unmaterialized gitlink shape.""" + with pytest.raises(RuntimeError, match="materialized safely"): + _materialize( + tmp_path, + monkeypatch, + [_directory_member("third_party/dependency/")], + ) + + +def test_snapshot_rejects_excessive_archive_member_count( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The snapshot refuses archives whose member count exceeds its bound.""" + monkeypatch.setattr(patch_validation, "MAX_SOURCE_ARCHIVE_MEMBERS", 1) + + with pytest.raises(RuntimeError, match="materialized safely"): + _materialize( + tmp_path, + monkeypatch, + [ + _regular_member("one.txt", b"1"), + _regular_member("two.txt", b"2"), + ], + ) + + +def test_snapshot_rejects_oversized_archive_member( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """One oversized file cannot exhaust extraction storage.""" + monkeypatch.setattr(patch_validation, "MAX_SOURCE_ARCHIVE_MEMBER_BYTES", 1) + + with pytest.raises(RuntimeError, match="materialized safely"): + _materialize( + tmp_path, + monkeypatch, + [_regular_member("large.txt", b"12")], + ) + + +def test_snapshot_rejects_excessive_total_archive_bytes( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Aggregate declared file bytes remain below a deterministic limit.""" + monkeypatch.setattr(patch_validation, "MAX_SOURCE_ARCHIVE_TOTAL_BYTES", 3) + + with pytest.raises(RuntimeError, match="materialized safely"): + _materialize( + tmp_path, + monkeypatch, + [ + _regular_member("one.txt", b"12"), + _regular_member("two.txt", b"34"), + ], + ) + + +def test_snapshot_verifies_extracted_member_type( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Post-extraction verification rejects a substituted symbolic link.""" + original_extractall = tarfile.TarFile.extractall + + def substitute_symlink(archive, path, *args, **kwargs): + """Extract normally, then replace a regular member before verification.""" + original_extractall(archive, path, *args, **kwargs) + extracted = Path(path) / "src" / "example.txt" + extracted.unlink() + extracted.symlink_to("missing-target") + + monkeypatch.setattr(tarfile.TarFile, "extractall", substitute_symlink) + + with pytest.raises(RuntimeError, match="materialized safely"): + _materialize( + tmp_path, + monkeypatch, + [ + _directory_member("src/"), + _regular_member("src/example.txt", b"trusted"), + ], + ) + + +def test_snapshot_verifies_extracted_member_size( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Post-extraction verification rejects content whose size changed.""" + original_extractall = tarfile.TarFile.extractall + + def substitute_size(archive, path, *args, **kwargs): + """Extract normally, then alter a regular member before verification.""" + original_extractall(archive, path, *args, **kwargs) + extracted = Path(path) / "src" / "example.txt" + extracted.write_bytes(b"changed-size") + + monkeypatch.setattr(tarfile.TarFile, "extractall", substitute_size) + + with pytest.raises(RuntimeError, match="materialized safely"): + _materialize( + tmp_path, + monkeypatch, + [ + _directory_member("src/"), + _regular_member("src/example.txt", b"trusted"), + ], + ) + + +def test_snapshot_accepts_only_bounded_regular_tree( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A bounded directory and regular file tree remains usable by Docker.""" + snapshot = _materialize( + tmp_path, + monkeypatch, + [ + _directory_member("src/"), + _regular_member("src/example.txt", b"trusted"), + ], + ) + + directory_mode = snapshot.joinpath("src").lstat().st_mode + file_path = snapshot / "src" / "example.txt" + file_mode = file_path.lstat().st_mode + assert stat.S_ISDIR(directory_mode) + assert stat.S_ISREG(file_mode) + assert not stat.S_ISLNK(file_mode) + assert file_path.read_bytes() == b"trusted" diff --git a/reviewer/tests/test_patch_validation_canonical_paths.py b/reviewer/tests/test_patch_validation_canonical_paths.py new file mode 100644 index 00000000..5d088c75 --- /dev/null +++ b/reviewer/tests/test_patch_validation_canonical_paths.py @@ -0,0 +1,39 @@ +"""Canonical repository-path regressions for patch preflight.""" + +from __future__ import annotations + +import pytest + +from noema_reviewer.patch_validation import inspect_patch_bytes + + +@pytest.mark.parametrize( + "path", + ( + "src//example.ts", + "src/./example.ts", + "src/example.ts/", + ".", + ), +) +def test_noncanonical_primary_paths_are_rejected(path: str) -> None: + """Primary diff paths must not normalize to a different filesystem identity.""" + patch_bytes = ( + f"diff --git a/{path} b/{path}\n" + f"--- a/{path}\n" + f"+++ b/{path}\n" + ).encode() + + with pytest.raises(ValueError, match="unsafe repository path"): + inspect_patch_bytes(patch_bytes) + + +def test_canonical_path_with_spaces_remains_supported() -> None: + """An exact quoted path with ordinary spaces remains a valid identity.""" + patch_bytes = ( + b'diff --git "a/src/file name.ts" "b/src/file name.ts"\n' + b'--- "a/src/file name.ts"\n' + b'+++ "b/src/file name.ts"\n' + ) + + assert inspect_patch_bytes(patch_bytes) == ("src/file name.ts",) diff --git a/reviewer/tests/test_patch_validation_coverage_edges.py b/reviewer/tests/test_patch_validation_coverage_edges.py new file mode 100644 index 00000000..aa32468e --- /dev/null +++ b/reviewer/tests/test_patch_validation_coverage_edges.py @@ -0,0 +1,406 @@ +"""Focused branch regressions for patch-validation fail-closed boundaries.""" + +from __future__ import annotations + +import hashlib +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from noema_reviewer import patch_validation +from noema_reviewer.patch_validation import ( + DockerPatchValidationRunner, + PatchValidationProfile, + PatchValidationRequest, + inspect_patch_bytes, +) + + +TEST_IMAGE = ( + f"{patch_validation.TRUSTED_PATCH_IMAGE_REPOSITORY}" + f"@sha256:{'a' * 64}" +) + + +def _diff(*metadata: bytes, source: bytes = b"src/x", target: bytes = b"src/x") -> bytes: + """Build one metadata-only Git diff with exact primary path identity.""" + return ( + b"diff --git a/" + + source + + b" b/" + + target + + b"\n" + + b"".join(metadata) + ) + + +def _tree_record( + path: str = "fixture.txt", + *, + mode: str = "100644", + object_type: str = "blob", + object_id: str = "a" * 40, + size: str = "1", +) -> str: + """Build one NUL-terminated `git ls-tree -l` record.""" + return f"{mode} {object_type} {object_id} {size}\t{path}\0" + + +def _ordinary_patch() -> bytes: + """Return one canonical one-line text patch.""" + return ( + b"diff --git a/src/x b/src/x\n" + b"--- a/src/x\n" + b"+++ b/src/x\n" + b"@@ -1 +1 @@\n" + b"-old\n" + b"+new\n" + ) + + +def test_git_control_reader_rejects_empty_file(tmp_path: Path) -> None: + """An empty Git control file cannot be interpreted as one control line.""" + control_file = tmp_path / "git-control" + control_file.touch() + + with pytest.raises(RuntimeError, match="invalid byte length"): + patch_validation._read_git_control_line(control_file, "test control") + + +def test_git_control_reader_closes_safely_when_open_fails( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A descriptor-open failure remains bounded before any descriptor exists.""" + control_file = tmp_path / "git-control" + control_file.write_text("gitdir: objects\n", encoding="utf-8") + + def fail_open(*_args, **_kwargs): + """Emulate a descriptor-open race without returning a descriptor.""" + raise OSError("open failed") + + monkeypatch.setattr(patch_validation.os, "open", fail_open) + + with pytest.raises(RuntimeError, match="could not be read safely"): + patch_validation._read_git_control_line(control_file, "test control") + + +def test_git_directory_rejects_regular_file(tmp_path: Path) -> None: + """A regular file cannot stand in for a required Git directory.""" + candidate = tmp_path / "not-a-directory" + candidate.write_text("not a directory", encoding="utf-8") + + with pytest.raises(RuntimeError, match="regular directory"): + patch_validation._validated_git_directory( + candidate, + "test Git directory", + require_exists=True, + ) + + +@pytest.mark.parametrize( + ("patch_bytes", "message"), + ( + ( + _diff(b"--- a/src/x\n"), + "incomplete secondary path metadata", + ), + ( + _diff(b"--- /dev/null\n", b"+++ /dev/null\n"), + "invalid /dev/null path metadata", + ), + ( + _diff( + b"--- /dev/null\n", + b"+++ b/src/new\n", + source=b"src/old", + target=b"src/new", + ), + "noncanonical creation metadata", + ), + ( + _diff( + b"--- a/src/old\n", + b"+++ /dev/null\n", + source=b"src/old", + target=b"src/new", + ), + "noncanonical deletion metadata", + ), + ( + _diff( + b"rename from src/old\n", + b"rename to src/new\n", + b"copy from src/old\n", + b"copy to src/new\n", + source=b"src/old", + target=b"src/new", + ), + "conflicting rename and copy metadata", + ), + ), +) +def test_secondary_metadata_families_fail_closed( + patch_bytes: bytes, + message: str, +) -> None: + """Incomplete, ambiguous, and noncanonical metadata families are rejected.""" + with pytest.raises(ValueError, match=message): + inspect_patch_bytes(patch_bytes) + + +@pytest.mark.parametrize( + ("patch_bytes", "message"), + ( + ( + b"@@ -1 +1 @@\n-old\n+new\n" + b"diff --git a/src/x b/src/x\n", + "hunk appears before a diff header", + ), + ( + b"--- a/src/x\n" + b"diff --git a/src/x b/src/x\n", + "path metadata appears before a diff header", + ), + ( + _diff( + b"--- a/src/x\n", + b"--- a/src/x\n", + b"+++ b/src/x\n", + ), + "repeats source path metadata", + ), + ( + b"index 1111..2222 100644\n" + b"diff --git a/src/x b/src/x\n", + "misplaced index metadata", + ), + ( + _diff(b"index nope\n"), + "malformed index metadata", + ), + ( + _diff(b"index 1111..2222 100600\n"), + "unsupported index mode", + ), + ( + b"new file mode 100644\n" + b"diff --git a/src/x b/src/x\n", + "misplaced mode metadata", + ), + ( + _diff(b"new file mode 100600\n"), + "unsupported file mode", + ), + ( + b"similarity index 100%\n" + b"diff --git a/src/x b/src/x\n", + "misplaced similarity metadata", + ), + ( + _diff(b"similarity index nope\n"), + "malformed similarity metadata", + ), + ( + _diff(b"similarity index 101%\n"), + "malformed similarity metadata", + ), + ( + _diff(b"\\ No newline at end of file\n"), + "malformed hunk newline marker", + ), + ), +) +def test_patch_metadata_placement_and_format_fail_closed( + patch_bytes: bytes, + message: str, +) -> None: + """Misplaced, duplicate, malformed, and unsupported metadata is rejected.""" + with pytest.raises(ValueError, match=message): + inspect_patch_bytes(patch_bytes) + + +def test_patch_inspector_accepts_unbound_blank_separator() -> None: + """An empty separator line does not create an unbound syntax channel.""" + assert inspect_patch_bytes(_diff(b"\n")) == ("src/x",) + + +@pytest.mark.parametrize("raw_output", ("", "not NUL terminated")) +def test_exact_tree_rejects_empty_or_truncated_output(raw_output: str) -> None: + """Exact-tree evidence must be nonempty and explicitly NUL terminated.""" + with pytest.raises(ValueError, match="empty or truncated"): + patch_validation._validated_exact_tree_output(raw_output) + + +def test_exact_tree_rejects_excessive_member_count( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Prearchive inspection rejects a tree beyond the member ceiling.""" + monkeypatch.setattr(patch_validation, "MAX_SOURCE_ARCHIVE_MEMBERS", 1) + + with pytest.raises(ValueError, match="too many members"): + patch_validation._validated_exact_tree_output( + _tree_record("one.txt") + _tree_record("two.txt") + ) + + +@pytest.mark.parametrize( + ("raw_output", "message"), + ( + ("metadata-without-tab\0", "malformed metadata"), + ("100644 blob\tfixture.txt\0", "malformed metadata"), + (_tree_record(mode="160000"), "non-regular object"), + (_tree_record(object_type="tree"), "non-regular object"), + (_tree_record(object_id="not-an-object-id"), "non-regular object"), + (_tree_record(size="unknown"), "invalid blob size"), + ), +) +def test_exact_tree_rejects_malformed_or_special_records( + raw_output: str, + message: str, +) -> None: + """Malformed metadata, special objects, and unknown sizes fail closed.""" + with pytest.raises(ValueError, match=message): + patch_validation._validated_exact_tree_output(raw_output) + + +def test_exact_tree_rejects_aggregate_size( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Aggregate blob bytes are bounded before archive serialization starts.""" + monkeypatch.setattr(patch_validation, "MAX_SOURCE_ARCHIVE_TOTAL_BYTES", 1) + + with pytest.raises(ValueError, match="aggregate byte limit"): + patch_validation._validated_exact_tree_output( + _tree_record("one.txt") + _tree_record("two.txt") + ) + + +def test_exact_tree_rejects_duplicate_path() -> None: + """Two object records cannot alias the same archive destination.""" + with pytest.raises(ValueError, match="repeats a member name"): + patch_validation._validated_exact_tree_output( + _tree_record("same.txt") + _tree_record("same.txt") + ) + + +def test_exact_tree_accepts_executable_sha256_blob() -> None: + """Canonical executable blobs and SHA-256 object identities remain valid.""" + patch_validation._validated_exact_tree_output( + _tree_record(mode="100755", object_id="b" * 64) + ) + + +def test_exact_tree_preflight_wraps_process_failure( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """An operating-system launch failure cannot be mistaken for valid evidence.""" + + def fail_popen(*_args, **_kwargs): + """Emulate a trusted Git executable launch failure.""" + raise OSError("Git unavailable") + + monkeypatch.setattr(patch_validation.subprocess, "Popen", fail_popen) + + with pytest.raises(RuntimeError, match="could not be inspected safely"): + patch_validation._verify_exact_tree_limits(tmp_path, "1" * 40) + + +def test_exact_tree_preflight_rejects_nonzero_git( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """A failed exact-tree command produces no admissible tree evidence.""" + process = SimpleNamespace(stdout=None, poll=lambda: 1) + chunks = iter( + ( + f"100644 blob {'a' * 40} 1\tfixture.txt\0".encode(), + b"", + ) + ) + monkeypatch.setattr( + patch_validation, + "_start_git_stream", + lambda _command: process, + ) + monkeypatch.setattr( + patch_validation, + "_read_git_stream_chunk", + lambda *_args, **_kwargs: next(chunks), + ) + monkeypatch.setattr( + patch_validation, + "_wait_git_stream", + lambda *_args, **_kwargs: 1, + ) + + with pytest.raises(RuntimeError, match="could not be inspected safely"): + patch_validation._verify_exact_tree_limits(tmp_path, "1" * 40) + + +def test_materialization_wraps_isolated_control_failure( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Failure to build isolated Git controls cannot fall back to local config.""" + source = tmp_path / "source" + source.mkdir() + staging = tmp_path / "staging" + staging.mkdir() + + def fail_control(*_args, **_kwargs): + """Emulate unavailable authenticated object storage.""" + raise RuntimeError("objects unavailable") + + monkeypatch.setattr( + patch_validation, + "_create_isolated_git_control", + fail_control, + ) + + with pytest.raises(RuntimeError, match="snapshot could not be materialized"): + patch_validation._materialize_committed_source( + source, + "1" * 40, + staging, + "directory", + ) + + +def test_metadata_mask_is_absent_without_git_metadata(tmp_path: Path) -> None: + """A checkout without Git metadata requires no nested Docker mask source.""" + assert patch_validation._create_git_metadata_mask(tmp_path, None) is None + + +def test_runner_rejects_missing_git_metadata_after_verified_preflight( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, +) -> None: + """Exact-head validation still requires an authenticated Git metadata shape.""" + source = tmp_path / "source" + source.mkdir() + patch_bytes = _ordinary_patch() + patch_path = tmp_path / "proposal.patch" + patch_path.write_bytes(patch_bytes) + request = PatchValidationRequest( + repository_full_name="ContextualWisdomLab/noema", + base_sha="1" * 40, + head_sha="2" * 40, + patch_sha256=hashlib.sha256(patch_bytes).hexdigest(), + profile=PatchValidationProfile.NODE_RELEASE_VERIFY, + ) + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + monkeypatch.setattr( + patch_validation, + "_verify_source_head", + lambda *_args, **_kwargs: None, + ) + + with pytest.raises(RuntimeError, match="Git metadata is required"): + DockerPatchValidationRunner().validate( + request=request, + source_root=source, + patch_path=patch_path, + ) diff --git a/reviewer/tests/test_patch_validation_exact_tree_and_output.py b/reviewer/tests/test_patch_validation_exact_tree_and_output.py new file mode 100644 index 00000000..c2f462b0 --- /dev/null +++ b/reviewer/tests/test_patch_validation_exact_tree_and_output.py @@ -0,0 +1,226 @@ +"""Regressions for raw Git-tree snapshots and a single bounded result file.""" + +from __future__ import annotations + +import hashlib +import subprocess +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from noema_reviewer import patch_validation +from noema_reviewer.patch_validation import ( + DockerPatchValidationRunner, + PatchValidationProfile, + PatchValidationRequest, + PatchValidationResult, + PatchValidationStatus, +) + + +TEST_IMAGE = ( + f"{patch_validation.TRUSTED_PATCH_IMAGE_REPOSITORY}" + f"@sha256:{'a' * 64}" +) + + +def _run_git(source: Path, *arguments: str) -> str: + """Run one bounded non-shell Git command in a temporary repository.""" + completed = subprocess.run( + [patch_validation.TRUSTED_GIT_EXECUTABLE, "-C", str(source), *arguments], + check=True, + shell=False, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=30, + ) + return completed.stdout.strip() + + +def _repository(tmp_path: Path) -> Path: + """Create one committed test repository with deterministic identity.""" + source = tmp_path / "source" + source.mkdir() + _run_git(source, "init", "-q") + _run_git(source, "config", "user.name", "Noema Test") + _run_git(source, "config", "user.email", "noema-test@example.invalid") + return source + + +def _commit(source: Path, message: str = "fixture") -> str: + """Commit every fixture path and return the exact head SHA.""" + _run_git(source, "add", "--all") + _run_git(source, "commit", "-qm", message) + return _run_git(source, "rev-parse", "HEAD") + + +def _materialize(source: Path, head_sha: str, tmp_path: Path) -> Path: + """Materialize one exact committed source snapshot through production code.""" + staging = tmp_path / "staging" + staging.mkdir() + return patch_validation._materialize_committed_source( + source, + head_sha, + staging, + "directory", + ) + + +def _patch() -> bytes: + """Return one ordinary bounded patch for Docker-boundary testing.""" + return ( + "diff --git a/src/example.ts b/src/example.ts\n" + "--- a/src/example.ts\n" + "+++ b/src/example.ts\n" + "@@ -1 +1 @@\n" + "-old\n" + "+new\n" + ).encode() + + +def _request(patch_bytes: bytes, head_sha: str) -> PatchValidationRequest: + """Build an exact request for one authenticated committed source tree.""" + return PatchValidationRequest( + repository_full_name="ContextualWisdomLab/noema", + base_sha="1" * 40, + head_sha=head_sha, + patch_sha256=hashlib.sha256(patch_bytes).hexdigest(), + profile=PatchValidationProfile.NODE_RELEASE_VERIFY, + ) + + +def _result_json(request: PatchValidationRequest) -> str: + """Return exact-request-bound successful structured evidence.""" + return PatchValidationResult( + status=PatchValidationStatus.PASSED, + repository_full_name=request.repository_full_name, + base_sha=request.base_sha, + head_sha=request.head_sha, + patch_sha256=request.patch_sha256, + profile=request.profile, + command_profile="npm run release:verify", + exit_code=0, + duration_ms=1, + stdout_excerpt="passed", + stderr_excerpt="", + reason_codes=[], + ).model_dump_json() + + +def _mount_source(command: list[str], destination: str) -> Path: + """Return the host source path for one exact Docker bind destination.""" + suffix = f",dst={destination}" + mount = next( + argument + for argument in command + if argument.startswith("--mount=") and suffix in argument + ) + return Path(mount.split("src=", 1)[1].split(",dst=", 1)[0]) + + +def _mount_destinations(command: list[str]) -> tuple[str, ...]: + """Return exact Docker bind destinations without prefix collisions.""" + return tuple( + argument.split(",dst=", 1)[1].split(",", 1)[0] + for argument in command + if argument.startswith("--mount=") and ",dst=" in argument + ) + + +def test_exact_snapshot_ignores_committed_export_ignore( + tmp_path: Path, +) -> None: + """A committed export-ignore rule cannot hide a tracked failing test.""" + source = _repository(tmp_path) + hidden = source / "tests" / "failing_test.py" + hidden.parent.mkdir() + hidden.write_text("raise AssertionError('must remain visible')\n", encoding="utf-8") + (source / ".gitattributes").write_text( + "tests/failing_test.py export-ignore\n", + encoding="utf-8", + ) + head_sha = _commit(source) + + snapshot = _materialize(source, head_sha, tmp_path) + + assert (snapshot / "tests" / "failing_test.py").read_bytes() == hidden.read_bytes() + + +def test_exact_snapshot_ignores_committed_export_subst( + tmp_path: Path, +) -> None: + """A committed export-subst rule cannot rewrite raw tracked blob bytes.""" + source = _repository(tmp_path) + version = source / "src" / "version.txt" + version.parent.mkdir() + version.write_text("$Format:%H$\n", encoding="utf-8") + (source / ".gitattributes").write_text( + "src/version.txt export-subst\n", + encoding="utf-8", + ) + head_sha = _commit(source) + + snapshot = _materialize(source, head_sha, tmp_path) + + assert (snapshot / "src" / "version.txt").read_bytes() == b"$Format:%H$\n" + + +def test_exact_snapshot_ignores_untracked_git_info_attributes( + tmp_path: Path, +) -> None: + """Repository-local info attributes cannot alter exact committed source bytes.""" + source = _repository(tmp_path) + hidden = source / "tests" / "failing_test.py" + hidden.parent.mkdir() + hidden.write_text("raise AssertionError('must remain visible')\n", encoding="utf-8") + head_sha = _commit(source) + info_attributes = source / ".git" / "info" / "attributes" + info_attributes.parent.mkdir(parents=True, exist_ok=True) + info_attributes.write_text( + "tests/failing_test.py export-ignore\n", + encoding="utf-8", + ) + + snapshot = _materialize(source, head_sha, tmp_path) + + assert (snapshot / "tests" / "failing_test.py").read_bytes() == hidden.read_bytes() + + +def test_runner_mounts_only_one_size_limited_result_file( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Untrusted code receives one host file and a realistic finite file ceiling.""" + source = _repository(tmp_path) + (source / "src").mkdir() + (source / "src" / "example.ts").write_text("old\n", encoding="utf-8") + head_sha = _commit(source) + patch_bytes = _patch() + patch_path = tmp_path / "proposal.patch" + patch_path.write_bytes(patch_bytes) + request = _request(patch_bytes, head_sha) + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + + def successful(command, **_kwargs): + """Write evidence only through the single pre-created result-file mount.""" + command_list = list(command) + result_path = _mount_source(command_list, "/output/result.json") + destinations = _mount_destinations(command_list) + assert "/output" not in destinations + assert destinations.count("/output/result.json") == 1 + assert ( + f"--ulimit=fsize={patch_validation.MAX_SOURCE_ARCHIVE_FILE_BYTES}:" + f"{patch_validation.MAX_SOURCE_ARCHIVE_FILE_BYTES}" + ) in command_list + result_path.write_text(_result_json(request), encoding="utf-8") + return SimpleNamespace(returncode=0) + + result = DockerPatchValidationRunner(command_runner=successful).validate( + request=request, + source_root=source, + patch_path=patch_path, + ) + + assert result.status is PatchValidationStatus.PASSED diff --git a/reviewer/tests/test_patch_validation_exact_tree_canonical_metadata.py b/reviewer/tests/test_patch_validation_exact_tree_canonical_metadata.py new file mode 100644 index 00000000..a7f2a038 --- /dev/null +++ b/reviewer/tests/test_patch_validation_exact_tree_canonical_metadata.py @@ -0,0 +1,51 @@ +"""Canonical exact-tree metadata regressions for the patch-validation boundary.""" + +from __future__ import annotations + +import pytest + +from noema_reviewer import patch_validation + + +@pytest.mark.parametrize( + ("metadata", "message"), + ( + (f"100644 blob {'a' * 40} 1", "malformed metadata"), + (f"100644\u00a0blob {'a' * 40} 1", "malformed metadata"), + (f"100644 blob {'a' * 40} 1", "malformed metadata"), + (f"100644 blob {'a' * 40} 1", "malformed metadata"), + (f"100644 blob {'a' * 40} \u0661", "invalid blob size"), + (f"100644 blob {'a' * 40} 1x", "invalid blob size"), + ), +) +def test_exact_tree_metadata_requires_canonical_ascii_fields( + metadata: str, + message: str, +) -> None: + """Unicode digits or noncanonical separators cannot masquerade as Git metadata.""" + record = f"{metadata}\tfixture.txt".encode("utf-8") + + with pytest.raises(ValueError, match=message): + patch_validation._validated_exact_tree_record(record, set(), 0) + + +@pytest.mark.parametrize( + ("raw_size", "expected_size"), + ( + ("1", 1), + (" 1", 1), + (" 10", 10), + ("1234567", 1_234_567), + ), +) +def test_exact_tree_metadata_accepts_git_ascii_size_forms( + raw_size: str, + expected_size: int, +) -> None: + """Unpadded fixtures and Git's exact minimum-width padding remain supported.""" + record = f"100644 blob {'a' * 40} {raw_size}\tfixture.txt".encode("ascii") + + assert ( + patch_validation._validated_exact_tree_record(record, set(), 0) + == expected_size + ) diff --git a/reviewer/tests/test_patch_validation_git_control_descriptor_edge.py b/reviewer/tests/test_patch_validation_git_control_descriptor_edge.py new file mode 100644 index 00000000..7364010a --- /dev/null +++ b/reviewer/tests/test_patch_validation_git_control_descriptor_edge.py @@ -0,0 +1,39 @@ +"""Descriptor-state regression for bounded Git control-line reads.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from noema_reviewer import patch_validation + + +def test_git_control_reader_skips_close_for_absent_descriptor_sentinel( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The defensive cleanup branch tolerates an opener returning no descriptor.""" + control_file = tmp_path / "git-control" + control_file.write_text("gitdir: objects\n", encoding="utf-8") + metadata = control_file.lstat() + chunks = iter((b"gitdir: objects\n", b"")) + + monkeypatch.setattr(patch_validation.os, "open", lambda *_args, **_kwargs: None) + monkeypatch.setattr(patch_validation.os, "fstat", lambda _descriptor: metadata) + monkeypatch.setattr( + patch_validation.os, + "read", + lambda _descriptor, _size: next(chunks), + ) + + def fail_close(_descriptor) -> None: + """Fail if cleanup tries to close the absent descriptor sentinel.""" + raise AssertionError("an absent descriptor must not be closed") + + monkeypatch.setattr(patch_validation.os, "close", fail_close) + + assert ( + patch_validation._read_git_control_line(control_file, "test control") + == "gitdir: objects" + ) diff --git a/reviewer/tests/test_patch_validation_git_control_isolation.py b/reviewer/tests/test_patch_validation_git_control_isolation.py new file mode 100644 index 00000000..7b914b19 --- /dev/null +++ b/reviewer/tests/test_patch_validation_git_control_isolation.py @@ -0,0 +1,297 @@ +"""Regression tests for isolating exact-commit archives from local Git metadata.""" + +from __future__ import annotations + +import hashlib +import json +import os +import stat +import subprocess +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from noema_reviewer import patch_validation +from noema_reviewer.patch_validation import ( + DockerPatchValidationRunner, + PatchValidationProfile, + PatchValidationRequest, + PatchValidationStatus, +) + + +TEST_IMAGE = ( + f"{patch_validation.TRUSTED_PATCH_IMAGE_REPOSITORY}" + f"@sha256:{'a' * 64}" +) + + +def _run_git(source: Path, *arguments: str) -> str: + """Run one deterministic local Git command and return stripped stdout.""" + completed = subprocess.run( + [patch_validation.TRUSTED_GIT_EXECUTABLE, "-C", str(source), *arguments], + check=True, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + return completed.stdout.strip() + + +def _patch_bytes() -> bytes: + """Return a bounded text patch for an ordinary repository source file.""" + return ( + "diff --git a/other.txt b/other.txt\n" + "index 1111111..2222222 100644\n" + "--- a/other.txt\n" + "+++ b/other.txt\n" + "@@ -1 +1 @@\n" + "-old\n" + "+new\n" + ).encode("utf-8") + + +def _mount_source(command: list[str], destination: str) -> Path: + """Return the source path for one exact Docker bind-mount destination.""" + suffix = f",dst={destination},readonly" + mount = next( + argument + for argument in command + if argument.startswith("--mount=type=bind,src=") and argument.endswith(suffix) + ) + return Path(mount.removeprefix("--mount=type=bind,src=").removesuffix(suffix)) + + +def _output_source(command: list[str]) -> Path: + """Return the host source for the validator's writable result-file mount.""" + suffix = ",dst=/output/result.json" + mount = next( + argument + for argument in command + if argument.startswith("--mount=type=bind,src=") and argument.endswith(suffix) + ) + return Path(mount.removeprefix("--mount=type=bind,src=").removesuffix(suffix)) + + +def _repository(tmp_path: Path) -> tuple[Path, str]: + """Create one exact committed repository for isolated-status tests.""" + source = tmp_path / "source" + source.mkdir() + _run_git(source, "init", "-q") + _run_git(source, "config", "user.name", "Noema Test") + _run_git(source, "config", "user.email", "noema-test@example.invalid") + (source / "kept.txt").write_text("committed bytes\n", encoding="utf-8") + (source / "other.txt").write_text("old\n", encoding="utf-8") + _run_git(source, "add", "kept.txt", "other.txt") + _run_git(source, "commit", "-m", "test exact source") + return source, _run_git(source, "rev-parse", "HEAD") + + +def test_local_git_info_attributes_cannot_rewrite_exact_commit_snapshot( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Host-local Git attributes must not omit bytes from the requested commit.""" + source, head_sha = _repository(tmp_path) + info_directory = source / ".git" / "info" + info_directory.mkdir(exist_ok=True) + (info_directory / "attributes").write_text( + "kept.txt export-ignore\n", + encoding="utf-8", + ) + + patch_bytes = _patch_bytes() + patch_path = tmp_path / "proposal.patch" + patch_path.write_bytes(patch_bytes) + request = PatchValidationRequest( + repository_full_name="ContextualWisdomLab/noema", + base_sha="1" * 40, + head_sha=head_sha, + patch_sha256=hashlib.sha256(patch_bytes).hexdigest(), + profile=PatchValidationProfile.NODE_RELEASE_VERIFY, + ) + + def fake_run(command: list[str], **_kwargs: object) -> SimpleNamespace: + """Require Docker to receive the commit byte hidden by hostile metadata.""" + mounted_source = _mount_source(command, "/input") + assert (mounted_source / "kept.txt").read_text(encoding="utf-8") == ( + "committed bytes\n" + ) + result_path = _output_source(command) + result = { + "status": "passed", + "repository_full_name": request.repository_full_name, + "base_sha": request.base_sha, + "head_sha": request.head_sha, + "patch_sha256": request.patch_sha256, + "profile": request.profile.value, + "command_profile": "npm run release:verify", + "exit_code": 0, + "duration_ms": 1, + "stdout_excerpt": "verified", + "stderr_excerpt": "", + "reason_codes": [], + } + result_path.write_text(json.dumps(result), encoding="utf-8") + return SimpleNamespace(returncode=0, stdout="", stderr="") + + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + result = DockerPatchValidationRunner( + command_runner=fake_run, + cleanup_runner=fake_run, + name_factory=lambda: "isolated-git-control-test", + ).validate( + request=request, + source_root=source, + patch_path=patch_path, + ) + + assert result.status is PatchValidationStatus.PASSED + + +def test_git_control_reader_rejects_unavailable_nonregular_and_ambiguous_files( + tmp_path: Path, +) -> None: + """Missing, directory, malformed UTF-8, and multiline controls fail closed.""" + with pytest.raises(RuntimeError, match="unavailable"): + patch_validation._read_git_control_line(tmp_path / "missing", "control") + + directory = tmp_path / "directory" + directory.mkdir() + with pytest.raises(RuntimeError, match="regular non-symlink"): + patch_validation._read_git_control_line(directory, "control") + + malformed = tmp_path / "malformed" + malformed.write_bytes(b"\xff") + with pytest.raises(RuntimeError, match="valid UTF-8"): + patch_validation._read_git_control_line(malformed, "control") + + ambiguous = tmp_path / "ambiguous" + ambiguous.write_text("one\ntwo\n", encoding="utf-8") + with pytest.raises(RuntimeError, match="one unambiguous line"): + patch_validation._read_git_control_line(ambiguous, "control") + + +@pytest.mark.parametrize("failure_kind", ["changed", "open", "read", "oversized"]) +def test_git_control_reader_rejects_descriptor_anomalies( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + failure_kind: str, +) -> None: + """Descriptor swaps, I/O errors, and growth beyond the bound fail closed.""" + control = tmp_path / "control" + control.write_text("gitdir: target\n", encoding="utf-8") + real_fstat = os.fstat + + if failure_kind == "changed": + monkeypatch.setattr( + patch_validation.os, + "fstat", + lambda descriptor: SimpleNamespace( + st_mode=stat.S_IFREG | 0o600, + st_dev=real_fstat(descriptor).st_dev, + st_ino=real_fstat(descriptor).st_ino + 1, + ), + ) + message = "changed during validation" + elif failure_kind == "open": + monkeypatch.setattr( + patch_validation.os, + "open", + lambda *_args: (_ for _ in ()).throw(OSError("open failed")), + ) + message = "could not be read safely" + elif failure_kind == "read": + monkeypatch.setattr( + patch_validation.os, + "read", + lambda *_args: (_ for _ in ()).throw(OSError("read failed")), + ) + message = "could not be read safely" + else: + monkeypatch.setattr( + patch_validation.os, + "read", + lambda *_args: b"x" * (patch_validation.MAX_GIT_CONTROL_FILE_BYTES + 1), + ) + message = "invalid byte length" + + with pytest.raises(RuntimeError, match=message): + patch_validation._read_git_control_line(control, "control") + + +def test_git_directory_and_gitfile_records_fail_closed( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Unsafe directory names, missing objects, and malformed records are rejected.""" + with pytest.raises(RuntimeError, match="unsafe path characters"): + patch_validation._validated_git_directory( + tmp_path / "unsafe\npath", + "control", + require_exists=False, + ) + with pytest.raises(RuntimeError, match="unavailable"): + patch_validation._validated_git_directory( + tmp_path / "missing", + "control", + require_exists=True, + ) + + source = tmp_path / "worktree" + source.mkdir() + (source / ".git").write_text("not-a-gitdir\n", encoding="utf-8") + with pytest.raises(RuntimeError, match="invalid gitdir record"): + patch_validation._source_object_directory( + source, + "file", + require_exists=False, + ) + + git_directory = tmp_path / "git-directory" + git_directory.mkdir() + (git_directory / "commondir").write_text("..\n", encoding="utf-8") + (source / ".git").write_text(f"gitdir: {git_directory}\n", encoding="utf-8") + real_lstat = os.lstat + + def fail_commondir(path: os.PathLike[str] | str): + """Raise a non-missing OS error only for the common-directory record.""" + if Path(path) == git_directory / "commondir": + raise PermissionError("denied") + return real_lstat(path) + + monkeypatch.setattr(patch_validation.os, "lstat", fail_commondir) + with pytest.raises(RuntimeError, match="common-directory record is unavailable"): + patch_validation._source_object_directory( + source, + "file", + require_exists=False, + ) + + +def test_isolated_status_failure_cannot_be_treated_as_clean( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A failed streamed status command cannot authenticate source cleanliness.""" + source, head_sha = _repository(tmp_path) + failed_process = SimpleNamespace(stdout=None, poll=lambda: 1) + monkeypatch.setattr( + patch_validation, + "_start_git_stream", + lambda _command: failed_process, + ) + monkeypatch.setattr( + patch_validation, + "_read_git_stream_chunk", + lambda *_args, **_kwargs: b"", + ) + monkeypatch.setattr( + patch_validation, + "_wait_git_stream", + lambda *_args, **_kwargs: 1, + ) + + with pytest.raises(RuntimeError, match="source HEAD could not be verified"): + patch_validation._verify_source_head(source, head_sha, "directory") diff --git a/reviewer/tests/test_patch_validation_git_metadata_mask.py b/reviewer/tests/test_patch_validation_git_metadata_mask.py new file mode 100644 index 00000000..c51bc59f --- /dev/null +++ b/reviewer/tests/test_patch_validation_git_metadata_mask.py @@ -0,0 +1,198 @@ +"""Git-control metadata isolation tests for the patch sandbox.""" + +from __future__ import annotations + +import hashlib +import subprocess +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from noema_reviewer import patch_validation +from noema_reviewer.patch_validation import ( + DockerPatchValidationRunner, + PatchValidationProfile, + PatchValidationRequest, + PatchValidationResult, + PatchValidationStatus, +) + + +TEST_IMAGE = ( + f"{patch_validation.TRUSTED_PATCH_IMAGE_REPOSITORY}" + f"@sha256:{'a' * 64}" +) + + +def _patch() -> bytes: + """Return one ordinary source patch.""" + return ( + "diff --git a/src/example.ts b/src/example.ts\n" + "--- a/src/example.ts\n" + "+++ b/src/example.ts\n" + "@@ -1 +1 @@\n" + "-old\n" + "+new\n" + ).encode() + + +def _initialize_repository(path: Path) -> str: + """Create one committed Git repository and return its exact HEAD.""" + path.mkdir() + subprocess.run(["git", "init", "-q", str(path)], check=True) + subprocess.run( + ["git", "-C", str(path), "config", "user.email", "test@example.invalid"], + check=True, + ) + subprocess.run( + ["git", "-C", str(path), "config", "user.name", "Noema Test"], + check=True, + ) + source = path / "src" + source.mkdir() + (source / "example.ts").write_text("old\n", encoding="utf-8") + subprocess.run(["git", "-C", str(path), "add", "src/example.ts"], check=True) + subprocess.run( + ["git", "-C", str(path), "commit", "-qm", "fixture"], + check=True, + ) + subprocess.run( + [ + "git", + "-C", + str(path), + "remote", + "add", + "origin", + "https://x-access-token:repository-secret@example.invalid/noema.git", + ], + check=True, + ) + return subprocess.run( + ["git", "-C", str(path), "rev-parse", "HEAD"], + check=True, + text=True, + stdout=subprocess.PIPE, + ).stdout.strip() + + +def _request(patch_bytes: bytes, head_sha: str) -> PatchValidationRequest: + """Build one exact-head-bound request.""" + return PatchValidationRequest( + repository_full_name="ContextualWisdomLab/noema", + base_sha="1" * 40, + head_sha=head_sha, + patch_sha256=hashlib.sha256(patch_bytes).hexdigest(), + profile=PatchValidationProfile.NODE_RELEASE_VERIFY, + ) + + +def _result_json(request: PatchValidationRequest) -> str: + """Return one exact-request-bound successful result.""" + return PatchValidationResult( + status=PatchValidationStatus.PASSED, + repository_full_name=request.repository_full_name, + base_sha=request.base_sha, + head_sha=request.head_sha, + patch_sha256=request.patch_sha256, + profile=request.profile, + command_profile="npm run release:verify", + exit_code=0, + duration_ms=1, + stdout_excerpt="passed", + stderr_excerpt="", + reason_codes=[], + ).model_dump_json() + + +def _mount_source(command: list[str], destination: str) -> Path: + """Return the host source for one Docker bind destination.""" + suffix = f",dst={destination}" + mount = next( + item + for item in command + if item.startswith("--mount=") and suffix in item + ) + return Path(mount.split("src=", 1)[1].split(",dst=", 1)[0]) + + +@pytest.mark.parametrize("checkout_kind", ["repository", "worktree"]) +def test_runner_masks_git_control_metadata_from_untrusted_code( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + checkout_kind: str, +) -> None: + """Repository credentials and worktree pointers are hidden by a nested mount.""" + repository = tmp_path / "repository" + head = _initialize_repository(repository) + if checkout_kind == "repository": + source = repository + else: + source = tmp_path / "worktree" + subprocess.run( + ["git", "-C", str(repository), "worktree", "add", "-q", "--detach", str(source), head], + check=True, + ) + patch_bytes = _patch() + patch_path = tmp_path / "proposal.patch" + patch_path.write_bytes(patch_bytes) + request = _request(patch_bytes, head) + observed_masks: list[Path] = [] + + def successful(command, **_kwargs): + """Inspect the metadata mask and write bounded result evidence.""" + command_list = list(command) + metadata_mask = _mount_source(command_list, "/input/.git,readonly") + result_path = _mount_source(command_list, "/output/result.json") + observed_masks.append(metadata_mask) + assert metadata_mask != source / ".git" + assert "repository-secret" not in repr(command_list) + if (source / ".git").is_dir(): + assert metadata_mask.is_dir() + assert list(metadata_mask.iterdir()) == [] + else: + assert metadata_mask.is_file() + assert metadata_mask.read_bytes() == b"" + result_path.write_text( + _result_json(request), + encoding="utf-8", + ) + return SimpleNamespace(returncode=0) + + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + result = DockerPatchValidationRunner(command_runner=successful).validate( + request=request, + source_root=source, + patch_path=patch_path, + ) + assert result.status is PatchValidationStatus.PASSED + assert len(observed_masks) == 1 + assert not observed_masks[0].exists() + + +def test_runner_rejects_symlinked_git_control_metadata_before_docker( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A symlink cannot redirect the trusted Git preflight outside the source root.""" + source = tmp_path / "source" + source.mkdir() + external_git = tmp_path / "external-git" + external_git.mkdir() + (source / ".git").symlink_to(external_git, target_is_directory=True) + patch_bytes = _patch() + patch_path = tmp_path / "proposal.patch" + patch_path.write_bytes(patch_bytes) + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + + def should_not_run(_args, **_kwargs): + """Fail if symlinked Git metadata reaches Docker.""" + raise AssertionError("Docker must not start") + + with pytest.raises(RuntimeError, match="Git metadata must not be a symlink"): + DockerPatchValidationRunner(command_runner=should_not_run).validate( + request=_request(patch_bytes, "2" * 40), + source_root=source, + patch_path=patch_path, + ) diff --git a/reviewer/tests/test_patch_validation_hardening.py b/reviewer/tests/test_patch_validation_hardening.py new file mode 100644 index 00000000..e9dfaa2e --- /dev/null +++ b/reviewer/tests/test_patch_validation_hardening.py @@ -0,0 +1,282 @@ +"""Hardening regressions for exact-source and bounded patch validation.""" + +from __future__ import annotations + +import hashlib +import subprocess +from pathlib import Path +from types import SimpleNamespace + +import pytest +from pydantic import ValidationError + +from noema_reviewer import patch_validation +from noema_reviewer.patch_validation import ( + DockerPatchValidationRunner, + PatchValidationProfile, + PatchValidationRequest, + PatchValidationResult, + PatchValidationStatus, + inspect_patch_bytes, +) + + +TEST_IMAGE = ( + f"{patch_validation.TRUSTED_PATCH_IMAGE_REPOSITORY}" + f"@sha256:{'a' * 64}" +) +BASE_SHA = "1" * 40 + + +def _patch() -> bytes: + """Return a minimal valid text patch.""" + return ( + "diff --git a/src/example.ts b/src/example.ts\n" + "index 1111111..2222222 100644\n" + "--- a/src/example.ts\n" + "+++ b/src/example.ts\n" + "@@ -1 +1 @@\n" + "-old value\n" + "+new value\n" + ).encode() + + +def _git_repository(tmp_path: Path) -> tuple[Path, str]: + """Create a clean repository and return its exact committed HEAD.""" + repository = tmp_path / "repository" + repository.mkdir() + subprocess.run(["git", "init", "-q", str(repository)], check=True) + subprocess.run( + ["git", "-C", str(repository), "config", "user.email", "test@example.invalid"], + check=True, + ) + subprocess.run( + ["git", "-C", str(repository), "config", "user.name", "Noema Test"], + check=True, + ) + source = repository / "src" + source.mkdir() + (source / "example.ts").write_text("old value\n", encoding="utf-8") + subprocess.run(["git", "-C", str(repository), "add", "src/example.ts"], check=True) + subprocess.run( + ["git", "-C", str(repository), "commit", "-qm", "fixture"], + check=True, + ) + head = subprocess.run( + ["git", "-C", str(repository), "rev-parse", "HEAD"], + check=True, + text=True, + stdout=subprocess.PIPE, + ).stdout.strip() + return repository, head + + +def _request(patch_bytes: bytes, head_sha: str) -> PatchValidationRequest: + """Build one exact-head-bound validation request.""" + return PatchValidationRequest( + repository_full_name="ContextualWisdomLab/noema", + base_sha=BASE_SHA, + head_sha=head_sha, + patch_sha256=hashlib.sha256(patch_bytes).hexdigest(), + profile=PatchValidationProfile.NODE_RELEASE_VERIFY, + ) + + +def _successful_result(request: PatchValidationRequest) -> str: + """Return exact-request-bound successful JSON evidence.""" + return PatchValidationResult( + status=PatchValidationStatus.PASSED, + repository_full_name=request.repository_full_name, + base_sha=request.base_sha, + head_sha=request.head_sha, + patch_sha256=request.patch_sha256, + profile=request.profile, + command_profile="npm run release:verify", + exit_code=0, + duration_ms=1, + stdout_excerpt="passed", + stderr_excerpt="", + reason_codes=[], + ).model_dump_json() + + +def _mount_source(command: list[str], destination: str) -> Path: + """Return the host source path for one Docker bind-mount destination.""" + suffix = f",dst={destination}" + mount = next(part for part in command if part.startswith("--mount=") and suffix in part) + source = mount.split("src=", 1)[1].split(",dst=", 1)[0] + return Path(source) + + +def test_patch_inspector_rejects_auxiliary_governance_paths() -> None: + """Traditional and rename headers cannot bypass the safe diff header.""" + patches = ( + ( + b"diff --git a/src/x b/src/x\n" + b"--- a/src/x\n" + b"+++ b/.github/workflows/pwn.yml\n" + ), + ( + b"diff --git a/src/x b/src/x\n" + b"similarity index 100%\n" + b"rename from src/x\n" + b"rename to .github/actions/pwn/action.yml\n" + ), + ( + b"diff --git a/src/x b/src/x\n" + b"similarity index 100%\n" + b"copy from src/x\n" + b"copy to .git/config\n" + ), + ) + for patch_bytes in patches: + with pytest.raises(ValueError, match="forbidden path"): + inspect_patch_bytes(patch_bytes) + + +def test_result_requires_consistent_status_and_bounded_reason_codes() -> None: + """Successful evidence cannot carry a failing exit code or unbounded labels.""" + patch_bytes = _patch() + request = _request(patch_bytes, "2" * 40) + values = PatchValidationResult( + status=PatchValidationStatus.PASSED, + repository_full_name=request.repository_full_name, + base_sha=request.base_sha, + head_sha=request.head_sha, + patch_sha256=request.patch_sha256, + profile=request.profile, + command_profile="npm run release:verify", + exit_code=0, + duration_ms=1, + stdout_excerpt="passed", + stderr_excerpt="", + reason_codes=[], + ).model_dump() + + inconsistent = dict(values, exit_code=1) + with pytest.raises(ValidationError): + PatchValidationResult.model_validate(inconsistent) + + unbounded = dict(values, reason_codes=["x" * 129]) + with pytest.raises(ValidationError): + PatchValidationResult.model_validate(unbounded) + + +def test_runner_rejects_source_revision_mismatch_before_docker( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A clean checkout must match the request head before untrusted execution.""" + repository, head = _git_repository(tmp_path) + patch_bytes = _patch() + patch_path = tmp_path / "proposal.patch" + patch_path.write_bytes(patch_bytes) + request = _request(patch_bytes, "f" * 40 if head != "f" * 40 else "e" * 40) + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + + def should_not_run(_args, **_kwargs): + """Fail if a mismatched checkout reaches Docker.""" + raise AssertionError("Docker must not start for a mismatched source revision") + + with pytest.raises(RuntimeError, match="source HEAD does not match"): + DockerPatchValidationRunner(command_runner=should_not_run).validate( + request=request, + source_root=repository, + patch_path=patch_path, + ) + + +@pytest.mark.parametrize("dirty_kind", ["tracked", "untracked"]) +def test_runner_rejects_non_exact_source_worktree_before_docker( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + dirty_kind: str, +) -> None: + """Tracked and untracked source drift cannot enter exact-head validation.""" + repository, head = _git_repository(tmp_path) + if dirty_kind == "tracked": + (repository / "src" / "example.ts").write_text( + "attacker replacement\n", + encoding="utf-8", + ) + else: + (repository / "src" / "injected.ts").write_text( + "attacker addition\n", + encoding="utf-8", + ) + patch_bytes = _patch() + patch_path = tmp_path / "proposal.patch" + patch_path.write_bytes(patch_bytes) + request = _request(patch_bytes, head) + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + + def should_not_run(_args, **_kwargs): + """Fail if a dirty checkout reaches Docker.""" + raise AssertionError("Docker must not start for a dirty source worktree") + + with pytest.raises(RuntimeError, match="source worktree is not clean"): + DockerPatchValidationRunner(command_runner=should_not_run).validate( + request=request, + source_root=repository, + patch_path=patch_path, + ) + + +def test_runner_mounts_private_patch_copy_and_bounded_result_file( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Docker bounds evidence without choking realistic profile artifacts.""" + repository, head = _git_repository(tmp_path) + patch_bytes = _patch() + original_patch = tmp_path / "proposal.patch" + original_patch.write_bytes(patch_bytes) + request = _request(patch_bytes, head) + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + monkeypatch.setenv("PATH", "/trusted/bin") + observed_mounts: list[tuple[Path, Path]] = [] + + def fake_run(command, **kwargs): + """Inspect private mounts and write exact-bound result evidence.""" + command_list = list(command) + staged_patch = _mount_source(command_list, "/patch/input.patch,readonly") + result_path = _mount_source(command_list, "/output/result.json") + observed_mounts.append((staged_patch, result_path)) + assert staged_patch != original_patch + assert staged_patch.read_bytes() == patch_bytes + assert kwargs["stdout"] is subprocess.DEVNULL + assert kwargs["stderr"] is subprocess.DEVNULL + mount_destinations = [ + argument.split(",dst=", 1)[1].split(",", 1)[0] + for argument in command_list + if argument.startswith("--mount=") and ",dst=" in argument + ] + assert "/output" not in mount_destinations + assert "/output/result.json" in mount_destinations + realistic_profile_artifact_bytes = 32 * 1024 + assert ( + patch_validation.MAX_RESULT_JSON_BYTES + < realistic_profile_artifact_bytes + <= patch_validation.MAX_SOURCE_ARCHIVE_FILE_BYTES + ) + assert ( + f"--ulimit=fsize={patch_validation.MAX_SOURCE_ARCHIVE_FILE_BYTES}:" + f"{patch_validation.MAX_SOURCE_ARCHIVE_FILE_BYTES}" + ) in command_list + result_path.write_text( + _successful_result(request), + encoding="utf-8", + ) + return SimpleNamespace(returncode=0) + + result = DockerPatchValidationRunner(command_runner=fake_run).validate( + request=request, + source_root=repository, + patch_path=original_patch, + ) + + assert result.status is PatchValidationStatus.PASSED + assert len(observed_mounts) == 1 + staged_patch, result_path = observed_mounts[0] + assert not staged_patch.exists() + assert not result_path.exists() diff --git a/reviewer/tests/test_patch_validation_mode_boundaries.py b/reviewer/tests/test_patch_validation_mode_boundaries.py new file mode 100644 index 00000000..842624c3 --- /dev/null +++ b/reviewer/tests/test_patch_validation_mode_boundaries.py @@ -0,0 +1,55 @@ +"""Git file-mode regressions for patch preflight.""" + +from __future__ import annotations + +import pytest + +from noema_reviewer.patch_validation import inspect_patch_bytes + + +@pytest.mark.parametrize("mode", ["120000", "160000"]) +def test_existing_symlink_and_gitlink_index_modes_are_rejected(mode: str) -> None: + """An existing special entry cannot bypass checks through an `index` header.""" + patch_bytes = ( + b"diff --git a/vendor/component b/vendor/component\n" + + f"index {'1' * 40}..{'2' * 40} {mode}\n".encode() + + b"--- a/vendor/component\n" + + b"+++ b/vendor/component\n" + + b"@@ -1 +1 @@\n" + + b"-Subproject commit 1111111111111111111111111111111111111111\n" + + b"+Subproject commit 2222222222222222222222222222222222222222\n" + ) + + with pytest.raises(ValueError, match="symlink or gitlink mode"): + inspect_patch_bytes(patch_bytes) + + +def test_regular_index_mode_is_accepted() -> None: + """A normal existing regular-file mode remains valid patch metadata.""" + patch_bytes = ( + b"diff --git a/src/example.ts b/src/example.ts\n" + + f"index {'1' * 40}..{'2' * 40} 100644\n".encode() + + b"--- a/src/example.ts\n" + + b"+++ b/src/example.ts\n" + + b"@@ -1 +1 @@\n" + + b"-old\n" + + b"+new\n" + ) + + assert inspect_patch_bytes(patch_bytes) == ("src/example.ts",) + + +def test_trailing_mode_tokens_cannot_hide_a_symlink_mode() -> None: + """A Git-tolerated trailing token cannot hide a symlink creation mode.""" + patch_bytes = ( + b"diff --git a/link b/link\n" + b"new file mode 120000 100644\n" + b"index 0000000..ce01362\n" + b"--- /dev/null\n" + b"+++ b/link\n" + b"@@ -0,0 +1 @@\n" + b"+target\n" + ) + + with pytest.raises(ValueError, match="malformed mode metadata"): + inspect_patch_bytes(patch_bytes) diff --git a/reviewer/tests/test_patch_validation_object_alternates_boundary.py b/reviewer/tests/test_patch_validation_object_alternates_boundary.py new file mode 100644 index 00000000..8bb983ed --- /dev/null +++ b/reviewer/tests/test_patch_validation_object_alternates_boundary.py @@ -0,0 +1,142 @@ +"""Fail-closed regressions for Git alternate object-database metadata.""" + +from __future__ import annotations + +import hashlib +import os +import subprocess +from pathlib import Path + +import pytest + +from noema_reviewer import patch_validation +from noema_reviewer.patch_validation import ( + DockerPatchValidationRunner, + PatchValidationProfile, + PatchValidationRequest, +) + + +TEST_IMAGE = ( + f"{patch_validation.TRUSTED_PATCH_IMAGE_REPOSITORY}" + f"@sha256:{'a' * 64}" +) + + +def _run_git(source: Path, *arguments: str) -> str: + """Run one bounded non-shell Git command and return stripped stdout.""" + completed = subprocess.run( + [patch_validation.TRUSTED_GIT_EXECUTABLE, "-C", str(source), *arguments], + check=True, + shell=False, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=30, + ) + return completed.stdout.strip() + + +def _repository(tmp_path: Path, name: str) -> Path: + """Create one ordinary temporary Git repository with deterministic identity.""" + source = tmp_path / name + source.mkdir() + _run_git(source, "init", "-q") + _run_git(source, "config", "user.name", "Noema Test") + _run_git(source, "config", "user.email", "noema-test@example.invalid") + return source + + +@pytest.mark.parametrize("metadata_name", ("alternates", "http-alternates")) +def test_source_object_directory_rejects_alternate_metadata( + tmp_path: Path, + metadata_name: str, +) -> None: + """A source object store cannot borrow objects or URLs outside its boundary.""" + source = _repository(tmp_path, "source") + metadata_path = source / ".git" / "objects" / "info" / metadata_name + metadata_path.parent.mkdir(parents=True, exist_ok=True) + metadata_path.write_text("/outside/object-store\n", encoding="utf-8") + + with pytest.raises(RuntimeError, match="alternate object database"): + patch_validation._source_object_directory( + source, + "directory", + require_exists=True, + ) + + +def test_source_object_directory_rejects_unreadable_alternate_metadata( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An I/O error while checking alternate metadata cannot be treated as absence.""" + source = _repository(tmp_path, "source") + alternates_path = source / ".git" / "objects" / "info" / "alternates" + real_lstat = os.lstat + + def deny_alternates(path: os.PathLike[str] | str): + """Deny only the source-local alternates record.""" + if Path(path) == alternates_path: + raise PermissionError("denied") + return real_lstat(path) + + monkeypatch.setattr(patch_validation.os, "lstat", deny_alternates) + + with pytest.raises(RuntimeError, match="alternate object metadata is unavailable"): + patch_validation._source_object_directory( + source, + "directory", + require_exists=True, + ) + + +def test_runner_rejects_exact_head_borrowed_from_external_object_store( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A clean worktree cannot import another repository's private object graph.""" + lender = _repository(tmp_path, "lender") + private_file = lender / "private.txt" + private_file.write_text("private bytes\n", encoding="utf-8") + _run_git(lender, "add", "private.txt") + _run_git(lender, "commit", "-qm", "private fixture") + borrowed_head = _run_git(lender, "rev-parse", "HEAD") + + source = _repository(tmp_path, "source") + alternates_path = source / ".git" / "objects" / "info" / "alternates" + alternates_path.write_text( + f"{lender / '.git' / 'objects'}\n", + encoding="utf-8", + ) + _run_git(source, "checkout", "--detach", "-q", borrowed_head) + + patch_bytes = ( + "diff --git a/private.txt b/private.txt\n" + "--- a/private.txt\n" + "+++ b/private.txt\n" + "@@ -1 +1 @@\n" + "-private bytes\n" + "+public bytes\n" + ).encode("utf-8") + patch_path = tmp_path / "proposal.patch" + patch_path.write_bytes(patch_bytes) + request = PatchValidationRequest( + repository_full_name="ContextualWisdomLab/noema", + base_sha="1" * 40, + head_sha=borrowed_head, + patch_sha256=hashlib.sha256(patch_bytes).hexdigest(), + profile=PatchValidationProfile.NODE_RELEASE_VERIFY, + ) + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + + def should_not_run(*_args: object, **_kwargs: object): + """Expose any attempt to launch Docker with borrowed source objects.""" + raise AssertionError("Docker must not receive borrowed source objects") + + with pytest.raises(RuntimeError, match="alternate object database"): + DockerPatchValidationRunner(command_runner=should_not_run).validate( + request=request, + source_root=source, + patch_path=patch_path, + ) diff --git a/reviewer/tests/test_patch_validation_path_consistency.py b/reviewer/tests/test_patch_validation_path_consistency.py new file mode 100644 index 00000000..c1bce900 --- /dev/null +++ b/reviewer/tests/test_patch_validation_path_consistency.py @@ -0,0 +1,93 @@ +"""Path-identity regressions for Git patch preflight.""" + +from __future__ import annotations + +import pytest + +from noema_reviewer.patch_validation import inspect_patch_bytes + + +@pytest.mark.parametrize( + "patch_bytes", + ( + ( + b"diff --git a/src/declared.ts b/src/declared.ts\n" + b"--- a/src/actual.ts\n" + b"+++ b/src/declared.ts\n" + b"@@ -1 +1 @@\n-old\n+new\n" + ), + ( + b"diff --git a/src/declared.ts b/src/declared.ts\n" + b"--- a/src/declared.ts\n" + b"+++ b/src/actual.ts\n" + b"@@ -1 +1 @@\n-old\n+new\n" + ), + ( + b"diff --git a/src/old.ts b/src/new.ts\n" + b"similarity index 100%\n" + b"rename from src/other.ts\n" + b"rename to src/new.ts\n" + ), + ( + b"diff --git a/src/old.ts b/src/new.ts\n" + b"similarity index 100%\n" + b"rename from src/old.ts\n" + b"rename to src/other.ts\n" + ), + ( + b"diff --git a/src/old.ts b/src/new.ts\n" + b"similarity index 100%\n" + b"copy from src/other.ts\n" + b"copy to src/new.ts\n" + ), + ( + b"diff --git a/src/old.ts b/src/new.ts\n" + b"similarity index 100%\n" + b"copy from src/old.ts\n" + b"copy to src/other.ts\n" + ), + ), +) +def test_secondary_paths_must_match_primary_diff_identity(patch_bytes: bytes) -> None: + """Auxiliary paths cannot redirect one counted diff entry to another safe file.""" + with pytest.raises(ValueError, match="does not match the primary diff path"): + inspect_patch_bytes(patch_bytes) + + +@pytest.mark.parametrize( + ("patch_bytes", "expected_target"), + ( + ( + b"diff --git a/src/old.ts b/src/new.ts\n" + b"similarity index 90%\n" + b"rename from src/old.ts\n" + b"rename to src/new.ts\n" + b"--- a/src/old.ts\n" + b"+++ b/src/new.ts\n" + b"@@ -1 +1 @@\n-old\n+new\n", + "src/new.ts", + ), + ( + b"diff --git a/src/new.ts b/src/new.ts\n" + b"new file mode 100644\n" + b"--- /dev/null\n" + b"+++ b/src/new.ts\n" + b"@@ -0,0 +1 @@\n+new\n", + "src/new.ts", + ), + ( + b"diff --git a/src/old.ts b/src/old.ts\n" + b"deleted file mode 100644\n" + b"--- a/src/old.ts\n" + b"+++ /dev/null\n" + b"@@ -1 +0,0 @@\n-old\n", + "src/old.ts", + ), + ), +) +def test_consistent_rename_create_and_delete_paths_are_accepted( + patch_bytes: bytes, + expected_target: str, +) -> None: + """Canonical rename, creation, and deletion metadata remains supported.""" + assert inspect_patch_bytes(patch_bytes) == (expected_target,) diff --git a/reviewer/tests/test_patch_validation_prearchive_and_result_channel.py b/reviewer/tests/test_patch_validation_prearchive_and_result_channel.py new file mode 100644 index 00000000..dfef1bfe --- /dev/null +++ b/reviewer/tests/test_patch_validation_prearchive_and_result_channel.py @@ -0,0 +1,128 @@ +"""Pre-serialization source limits and single-file result-channel regressions.""" + +from __future__ import annotations + +import stat +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from noema_reviewer import patch_validation + + +class _ResultFileSystem: + """Record bounded descriptor reads while emulating one stable regular file.""" + + def __init__(self, payload: bytes, *, declared_size: int | None = None) -> None: + """Store one payload and initialize descriptor-read observations.""" + self.payload = payload + self.offset = 0 + self.requested_sizes: list[int] = [] + self.metadata = SimpleNamespace( + st_mode=stat.S_IFREG | 0o600, + st_size=len(payload) if declared_size is None else declared_size, + st_dev=1, + st_ino=2, + ) + + def lstat(self, _path: Path) -> SimpleNamespace: + """Return stable path metadata.""" + return self.metadata + + def open(self, _path: Path, _flags: int) -> int: + """Return one deterministic descriptor.""" + return 7 + + def fstat(self, _descriptor: int) -> SimpleNamespace: + """Return metadata for the opened descriptor.""" + return self.metadata + + def read(self, _descriptor: int, size: int) -> bytes: + """Return at most the requested payload bytes and record the bound.""" + self.requested_sizes.append(size) + chunk = self.payload[self.offset : self.offset + size] + self.offset += len(chunk) + return chunk + + def close(self, _descriptor: int) -> None: + """Close the deterministic descriptor without side effects.""" + + +def test_result_reader_never_uses_stdout_fallback(tmp_path: Path) -> None: + """An empty result file fails even when a runner returns forged JSON stdout.""" + result_path = tmp_path / "result.json" + result_path.touch(mode=0o600) + completed = SimpleNamespace(returncode=0, stdout='{"status":"passed"}') + + with pytest.raises(RuntimeError, match="result.*must not be empty"): + patch_validation._read_result_payload(result_path, completed) + + +def test_result_reader_stops_at_sixteen_kibibytes_plus_one() -> None: + """A post-stat growth race cannot escape the 16 KiB descriptor read budget.""" + payload = b"x" * (patch_validation.MAX_RESULT_JSON_BYTES + 1) + file_system = _ResultFileSystem( + payload, + declared_size=patch_validation.MAX_RESULT_JSON_BYTES, + ) + + with pytest.raises(RuntimeError, match="result.*exceeds"): + patch_validation._read_result_payload( + Path("/bounded/result.json"), + file_system=file_system, + ) + + assert file_system.requested_sizes + assert max(file_system.requested_sizes) <= patch_validation.MAX_RESULT_JSON_BYTES + 1 + + +def test_exact_tree_limits_are_checked_before_git_archive( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An over-limit blob is rejected before archive bytes reach runner storage.""" + source = tmp_path / "source" + source.mkdir() + staging = tmp_path / "staging" + staging.mkdir() + isolated_control = staging / "isolated-control" + isolated_control.mkdir() + archive_started = False + + monkeypatch.setattr( + patch_validation, + "_create_isolated_git_control", + lambda *_args, **_kwargs: isolated_control, + ) + + def fake_run(command, **_kwargs): + """Expose an oversized exact-tree entry and forbid archive execution.""" + nonlocal archive_started + command_list = list(command) + if "ls-tree" in command_list: + return SimpleNamespace( + returncode=0, + stdout=( + "100644 blob " + f"{'a' * 40} " + f"{patch_validation.MAX_SOURCE_ARCHIVE_MEMBER_BYTES + 1}" + "\toversized.bin\0" + ), + ) + if "archive" in command_list: + archive_started = True + return SimpleNamespace(returncode=0) + raise AssertionError(f"unexpected Git command: {command_list}") + + monkeypatch.setattr(patch_validation.subprocess, "run", fake_run) + + with pytest.raises(RuntimeError, match="tree|byte limit|materialized"): + patch_validation._materialize_committed_source( + source, + "1" * 40, + staging, + "directory", + ) + + assert archive_started is False diff --git a/reviewer/tests/test_patch_validation_provenance_and_hunk_edges.py b/reviewer/tests/test_patch_validation_provenance_and_hunk_edges.py new file mode 100644 index 00000000..e18b5f59 --- /dev/null +++ b/reviewer/tests/test_patch_validation_provenance_and_hunk_edges.py @@ -0,0 +1,181 @@ +"""Exact-head provenance and unified-hunk edge regressions.""" + +from __future__ import annotations + +import hashlib +import subprocess +from pathlib import Path + +import pytest + +from noema_reviewer import patch_validation +from noema_reviewer.patch_validation import ( + DockerPatchValidationRunner, + PatchValidationProfile, + PatchValidationRequest, + inspect_patch_bytes, +) + + +TEST_IMAGE = ( + f"{patch_validation.TRUSTED_PATCH_IMAGE_REPOSITORY}" + f"@sha256:{'a' * 64}" +) +BASE_SHA = "1" * 40 + + +def _patch() -> bytes: + """Return one ordinary exact-file modification patch.""" + return ( + b"diff --git a/src/example.ts b/src/example.ts\n" + b"index 1111111..2222222 100644\n" + b"--- a/src/example.ts\n" + b"+++ b/src/example.ts\n" + b"@@ -1 +1 @@\n" + b"-old\n" + b"+new\n" + ) + + +def _request(patch_bytes: bytes, head_sha: str) -> PatchValidationRequest: + """Build one exact-head-bound validation request.""" + return PatchValidationRequest( + repository_full_name="ContextualWisdomLab/noema", + base_sha=BASE_SHA, + head_sha=head_sha, + patch_sha256=hashlib.sha256(patch_bytes).hexdigest(), + profile=PatchValidationProfile.NODE_RELEASE_VERIFY, + ) + + +def _git_repository(tmp_path: Path) -> tuple[Path, str]: + """Create a clean committed source repository and return its head SHA.""" + repository = tmp_path / "repository" + repository.mkdir() + subprocess.run(["git", "init", "-q", str(repository)], check=True) + subprocess.run( + ["git", "-C", str(repository), "config", "user.email", "test@example.invalid"], + check=True, + ) + subprocess.run( + ["git", "-C", str(repository), "config", "user.name", "Noema Test"], + check=True, + ) + source = repository / "src" + source.mkdir() + (source / "example.ts").write_text("old\n", encoding="utf-8") + subprocess.run(["git", "-C", str(repository), "add", "src/example.ts"], check=True) + subprocess.run( + ["git", "-C", str(repository), "commit", "-qm", "fixture"], + check=True, + ) + head_sha = subprocess.run( + ["git", "-C", str(repository), "rev-parse", "HEAD"], + check=True, + text=True, + stdout=subprocess.PIPE, + ).stdout.strip() + return repository, head_sha + + +def test_metadata_free_source_cannot_claim_an_exact_git_head( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An arbitrary mutable directory cannot be labeled with a Git head SHA.""" + source = tmp_path / "metadata-free-source" + (source / "src").mkdir(parents=True) + (source / "src" / "example.ts").write_text("old\n", encoding="utf-8") + patch_bytes = _patch() + patch_path = tmp_path / "proposal.patch" + patch_path.write_bytes(patch_bytes) + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + + def should_not_run(*_args, **_kwargs): + """Fail if unauthenticated source bytes reach Docker.""" + raise AssertionError("Docker must not run for metadata-free exact-head input") + + with pytest.raises(RuntimeError, match="Git metadata"): + DockerPatchValidationRunner(command_runner=should_not_run).validate( + request=_request(patch_bytes, "2" * 40), + source_root=source, + patch_path=patch_path, + ) + + +@pytest.mark.parametrize(("uid", "gid"), [(0, 1000), (1000, 0), (0, 0)]) +def test_root_host_identity_cannot_become_the_sandbox_identity( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + uid: int, + gid: int, +) -> None: + """A root UID or GID must fail before Docker can launch the validator.""" + source, head_sha = _git_repository(tmp_path) + patch_bytes = _patch() + patch_path = tmp_path / "proposal.patch" + patch_path.write_bytes(patch_bytes) + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + monkeypatch.setattr(patch_validation.os, "getuid", lambda: uid) + monkeypatch.setattr(patch_validation.os, "getgid", lambda: gid) + + def should_not_run(*_args, **_kwargs): + """Fail if a root-derived sandbox identity reaches Docker.""" + raise AssertionError("Docker must not run with a root UID or GID") + + with pytest.raises(RuntimeError, match="non-root"): + DockerPatchValidationRunner(command_runner=should_not_run).validate( + request=_request(patch_bytes, head_sha), + source_root=source, + patch_path=patch_path, + ) + + +@pytest.mark.parametrize( + "patch_bytes", + [ + ( + b"diff --git a/src/example.ts b/src/example.ts\n" + b"--- a/src/example.ts\n" + b"+++ b/src/example.ts\n" + b"@@ -1 +1 @@\n" + b"\\ No newline at end of file\n" + b"-old\n" + b"+new\n" + ), + ( + b"diff --git a/src/example.ts b/src/example.ts\n" + b"--- a/src/example.ts\n" + b"+++ b/src/example.ts\n" + b"@@ -1 +1 @@\n" + b"-old\n" + b"\\ No newline at end of file\n" + b"\\ No newline at end of file\n" + b"+new\n" + ), + ( + b"diff --git a/src/example.ts b/src/example.ts\n" + b"--- a/src/example.ts\n" + b"+++ b/src/example.ts\n" + b"@@ -1 +1 @@\n" + b"-old\n" + b"+new\n" + b"+extra\n" + ), + ( + b"diff --git a/src/example.ts b/src/example.ts\n" + b"--- a/src/example.ts\n" + b"+++ b/src/example.ts\n" + b"@@ -1 +1 @@\n" + b"-old\n" + b"+new\n" + b"unbound trailing syntax\n" + ), + ], +) +def test_unified_hunk_markers_and_trailing_content_fail_closed( + patch_bytes: bytes, +) -> None: + """Misplaced markers, count overrun, and unbound trailing text are rejected.""" + with pytest.raises(ValueError, match="hunk|trailing|syntax"): + inspect_patch_bytes(patch_bytes) diff --git a/reviewer/tests/test_patch_validation_security_boundaries.py b/reviewer/tests/test_patch_validation_security_boundaries.py new file mode 100644 index 00000000..10adf103 --- /dev/null +++ b/reviewer/tests/test_patch_validation_security_boundaries.py @@ -0,0 +1,418 @@ +"""Adversarial regression tests for patch-validation trust boundaries.""" + +from __future__ import annotations + +import hashlib +import subprocess +from pathlib import Path +from types import SimpleNamespace + +import pytest +from pydantic import ValidationError + +from noema_reviewer import patch_validation +from noema_reviewer.patch_validation import ( + DockerPatchValidationRunner, + PatchValidationProfile, + PatchValidationRequest, + PatchValidationResult, + PatchValidationStatus, + inspect_patch_bytes, +) + + +BASE_SHA = "1" * 40 +HEAD_SHA = "2" * 40 +TEST_IMAGE = ( + f"{patch_validation.TRUSTED_PATCH_IMAGE_REPOSITORY}" + f"@sha256:{'a' * 64}" +) + + +def _safe_patch() -> bytes: + """Return a minimal patch whose declared target is ordinary source code.""" + return ( + "diff --git a/src/example.ts b/src/example.ts\n" + "index 1111111..2222222 100644\n" + "--- a/src/example.ts\n" + "+++ b/src/example.ts\n" + "@@ -1 +1 @@\n" + "-old\n" + "+new\n" + ).encode() + + +def _request( + patch_bytes: bytes, + *, + head_sha: str = HEAD_SHA, +) -> PatchValidationRequest: + """Build an exact request for one test patch.""" + return PatchValidationRequest( + repository_full_name="ContextualWisdomLab/noema", + base_sha=BASE_SHA, + head_sha=head_sha, + patch_sha256=hashlib.sha256(patch_bytes).hexdigest(), + profile=PatchValidationProfile.NODE_RELEASE_VERIFY, + ) + + +def _result_json(request: PatchValidationRequest) -> str: + """Return one exact-request-bound successful result document.""" + return PatchValidationResult( + status=PatchValidationStatus.PASSED, + repository_full_name=request.repository_full_name, + base_sha=request.base_sha, + head_sha=request.head_sha, + patch_sha256=request.patch_sha256, + profile=request.profile, + command_profile="npm run release:verify", + exit_code=0, + duration_ms=1, + stdout_excerpt="passed", + stderr_excerpt="", + reason_codes=[], + ).model_dump_json() + + +def _run_git(source: Path, *arguments: str) -> str: + """Run one deterministic non-shell Git command for a test repository.""" + completed = subprocess.run( + [patch_validation.TRUSTED_GIT_EXECUTABLE, "-C", str(source), *arguments], + check=True, + shell=False, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=30, + ) + return completed.stdout.strip() + + +def _write_inputs( + tmp_path: Path, + patch_bytes: bytes, + *, + patch_name: str = "proposal.patch", +) -> tuple[Path, Path, str]: + """Create one authenticated clean Git source and patch input.""" + source = tmp_path / "source" + source.mkdir() + _run_git(source, "init", "-q") + _run_git(source, "config", "user.email", "test@example.invalid") + _run_git(source, "config", "user.name", "Noema Test") + source_file = source / "src" / "example.ts" + source_file.parent.mkdir() + source_file.write_text("old\n", encoding="utf-8") + _run_git(source, "add", "src/example.ts") + _run_git(source, "commit", "-qm", "fixture") + head_sha = _run_git(source, "rev-parse", "HEAD") + patch_path = tmp_path / patch_name + patch_path.write_bytes(patch_bytes) + return source, patch_path, head_sha + + +def _mount_source(command: list[str], destination: str) -> Path: + """Return the host source path for one exact Docker bind destination.""" + suffix = f",dst={destination}" + mount = next( + part + for part in command + if part.startswith("--mount=") and suffix in part + ) + return Path(mount.split("src=", 1)[1].split(",dst=", 1)[0]) + + +@pytest.mark.parametrize( + "patch_bytes", + ( + ( + b"diff --git a/src/example.ts b/src/example.ts\n" + b"--- a/src/example.ts\n" + b"+++ b/.github/workflows/pwn.yml\n" + ), + ( + b"diff --git a/src/example.ts b/src/example.ts\n" + b"rename from src/example.ts\n" + b"rename to .github/actions/pwn/action.yml\n" + ), + ( + b"diff --git a/src/example.ts b/src/example.ts\n" + b"copy from src/example.ts\n" + b"copy to docs/CODEOWNERS\n" + ), + ), +) +def test_patch_inspector_rejects_hidden_governance_targets(patch_bytes: bytes) -> None: + """Secondary Git headers cannot redirect a safe diff header into governance.""" + with pytest.raises(ValueError, match="forbidden path"): + inspect_patch_bytes(patch_bytes) + + +@pytest.mark.parametrize( + ("patch_bytes", "message"), + ( + ( + b"diff --git a/src/x b/src/x\n" + b"--- a/src/x\n" + b"+++ b/src\\evil\n", + "unsafe repository path", + ), + ( + b"diff --git a/src/x b/src/x\n" + b"--- a/src/x\n" + b'+++ "b/src/unterminated\n', + "malformed diff header", + ), + ( + b"diff --git a/src/x b/src/x\n" + b"--- a/src/x\n" + b'+++ "b/src/x" "b/src/y"\n', + "malformed diff header", + ), + ( + b"diff --git a/src/x b/src/x\n" + b"--- a/src/x\n" + b'+++ b/src/"x\n', + "malformed diff header", + ), + ), +) +def test_patch_inspector_rejects_malformed_secondary_paths( + patch_bytes: bytes, + message: str, +) -> None: + """Quoted and escaped auxiliary path syntax is validated fail closed.""" + with pytest.raises(ValueError, match=message): + inspect_patch_bytes(patch_bytes) + + +def test_patch_inspector_accepts_quoted_secondary_paths_and_dev_null() -> None: + """Valid quoted names and Git's canonical deletion sentinel remain supported.""" + quoted = ( + b'diff --git "a/src/file name.ts" "b/src/file name.ts"\n' + b'--- "a/src/file name.ts"\n' + b'+++ "b/src/file name.ts"\n' + ) + deleted = ( + b"diff --git a/src/x b/src/x\n" + b"--- a/src/x\n" + b"+++ /dev/null\n" + ) + assert inspect_patch_bytes(quoted) == ("src/file name.ts",) + assert inspect_patch_bytes(deleted) == ("src/x",) + + +def test_patch_inspector_rejects_traditional_governance_section_after_hunk() -> None: + """A second traditional diff cannot hide after a completed safe Git hunk.""" + patch_bytes = ( + b"diff --git a/src/x b/src/x\n" + b"--- a/src/x\n" + b"+++ b/src/x\n" + b"@@ -1 +1 @@\n" + b"-old\n" + b"+new\n" + b"--- a/.github/workflows/pwn.yml\n" + b"+++ b/.github/workflows/pwn.yml\n" + b"@@ -1 +1 @@\n" + b"-safe\n" + b"+pwned\n" + ) + + with pytest.raises(ValueError, match="forbidden path"): + inspect_patch_bytes(patch_bytes) + + +def test_patch_inspector_keeps_path_like_removed_content_inside_hunk() -> None: + """A removed source line beginning with three dashes is hunk content, not a path.""" + patch_bytes = ( + b"diff --git a/src/x b/src/x\n" + b"--- a/src/x\n" + b"+++ b/src/x\n" + b"@@ -1 +1 @@\n" + b"--- not/a/header\n" + b"+replacement\n" + ) + + assert inspect_patch_bytes(patch_bytes) == ("src/x",) + + +def test_patch_inspector_accepts_context_multiple_hunks_and_no_newline_marker() -> None: + """Counted context, multiple hunks, zero ranges, and newline markers are valid.""" + patch_bytes = ( + b"diff --git a/src/x b/src/x\n" + b"--- a/src/x\n" + b"+++ b/src/x\n" + b"@@ -1,2 +1,2 @@ first\n" + b" unchanged\n" + b"-old\n" + b"+new\n" + b"\\ No newline at end of file\n" + b"@@ -10,0 +11,1 @@ second\n" + b"+added\n" + ) + + assert inspect_patch_bytes(patch_bytes) == ("src/x",) + + +@pytest.mark.parametrize( + ("patch_bytes", "message"), + ( + ( + b"@@ -1 +1 @@\n-old\n+new\n" + b"diff --git a/src/x b/src/x\n", + "before a diff header", + ), + ( + b"diff --git a/src/x b/src/x\n@@@ -1 +1 @@@\n", + "malformed hunk header", + ), + ( + b"diff --git a/src/x b/src/x\n@@ -1 +1 @@\n\n", + "malformed hunk body", + ), + ( + b"diff --git a/src/x b/src/x\n@@ -1 +1 @@\n?invalid\n", + "malformed hunk body", + ), + ( + b"diff --git a/src/x b/src/x\n@@ -0,0 +1 @@\n-old\n+new\n", + "more lines than declared", + ), + ( + b"diff --git a/src/x b/src/x\n@@ -1,1 +1,0 @@\n", + "ended before", + ), + ( + b"diff --git a/src/x b/src/x\n@@ -1,0 +1,1 @@\n", + "ended before", + ), + ( + b"diff --git a/src/x b/src/x\n" + b"@@ -1 +1 @@\n-old\n+new\n" + b"--- a/src/y\n+++ b/src/y\n", + "path metadata after a hunk", + ), + ), +) +def test_patch_inspector_rejects_malformed_or_smuggled_hunks( + patch_bytes: bytes, + message: str, +) -> None: + """Malformed counts, bodies, truncation, and late path metadata fail closed.""" + with pytest.raises(ValueError, match=message): + inspect_patch_bytes(patch_bytes) + + +def test_runner_stages_docker_ambiguous_original_patch_path( + tmp_path, + monkeypatch, +) -> None: + """A comma-bearing caller path is replaced by a private safe mount source.""" + patch_bytes = _safe_patch() + source, patch_path, head_sha = _write_inputs( + tmp_path, + patch_bytes, + patch_name="proposal,readonly=false.patch", + ) + request = _request(patch_bytes, head_sha=head_sha) + observed: list[tuple[Path, Path]] = [] + + def successful(command, **kwargs): + """Verify safe staging and write the bounded result artifact.""" + command_list = list(command) + staged_patch = _mount_source(command_list, "/patch/input.patch,readonly") + result_path = _mount_source(command_list, "/output/result.json") + observed.append((staged_patch, result_path)) + assert staged_patch != patch_path + assert "," not in str(staged_patch) + assert staged_patch.read_bytes() == patch_bytes + assert str(patch_path) not in repr(command) + assert kwargs["stdout"] is subprocess.DEVNULL + assert kwargs["stderr"] is subprocess.DEVNULL + result_path.write_text(_result_json(request), encoding="utf-8") + return SimpleNamespace(returncode=0, stdout="", stderr="") + + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + result = DockerPatchValidationRunner(command_runner=successful).validate( + request=request, + source_root=source, + patch_path=patch_path, + ) + assert result.status is PatchValidationStatus.PASSED + staged_patch, result_path = observed[0] + assert not staged_patch.exists() + assert not result_path.exists() + + +def test_request_and_result_models_reject_unknown_fields() -> None: + """Unknown wire fields fail closed instead of being silently discarded.""" + request = _request(_safe_patch()) + request_values = request.model_dump() + request_values["arbitrary_command"] = "curl attacker.invalid" + with pytest.raises(ValidationError): + PatchValidationRequest.model_validate(request_values) + + result_values = { + "status": PatchValidationStatus.PASSED, + "repository_full_name": request.repository_full_name, + "base_sha": request.base_sha, + "head_sha": request.head_sha, + "patch_sha256": request.patch_sha256, + "profile": request.profile, + "command_profile": "npm run release:verify", + "exit_code": 0, + "duration_ms": 1, + "stdout_excerpt": "ok", + "stderr_excerpt": "", + "reason_codes": [], + "unreviewed_evidence": True, + } + with pytest.raises(ValidationError): + PatchValidationResult.model_validate(result_values) + + +def test_result_model_bounds_duration_and_reason_codes() -> None: + """Result metadata cannot smuggle unbounded integers or diagnostic strings.""" + request = _request(_safe_patch()) + values = { + "status": PatchValidationStatus.BLOCKED, + "repository_full_name": request.repository_full_name, + "base_sha": request.base_sha, + "head_sha": request.head_sha, + "patch_sha256": request.patch_sha256, + "profile": request.profile, + "command_profile": "npm run release:verify", + "exit_code": 1, + "duration_ms": patch_validation.PATCH_SANDBOX_WALL_TIMEOUT_SECONDS * 1000 + 1, + "stdout_excerpt": "", + "stderr_excerpt": "", + "reason_codes": ["x"], + } + with pytest.raises(ValidationError): + PatchValidationResult.model_validate(values) + + values["duration_ms"] = 1 + values["reason_codes"] = ["x" * 65] + with pytest.raises(ValidationError): + PatchValidationResult.model_validate(values) + + +def test_runner_rejects_oversized_result_file(tmp_path, monkeypatch) -> None: + """The writable result-file mount cannot return oversized evidence.""" + patch_bytes = _safe_patch() + source, patch_path, head_sha = _write_inputs(tmp_path, patch_bytes) + + def oversized(command, **_kwargs): + """Write a regular result file just beyond the accepted byte ceiling.""" + result_path = _mount_source(list(command), "/output/result.json") + result_path.write_bytes(b"x" * (patch_validation.MAX_RESULT_JSON_BYTES + 1)) + return SimpleNamespace(returncode=0, stdout="", stderr="") + + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + with pytest.raises(RuntimeError, match="result exceeds"): + DockerPatchValidationRunner(command_runner=oversized).validate( + request=_request(patch_bytes, head_sha=head_sha), + source_root=source, + patch_path=patch_path, + ) diff --git a/reviewer/tests/test_patch_validation_source_integrity.py b/reviewer/tests/test_patch_validation_source_integrity.py new file mode 100644 index 00000000..4f8d0e0f --- /dev/null +++ b/reviewer/tests/test_patch_validation_source_integrity.py @@ -0,0 +1,271 @@ +"""Source-checkout integrity regressions for patch validation.""" + +from __future__ import annotations + +import hashlib +import subprocess +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from noema_reviewer import patch_validation +from noema_reviewer.patch_validation import ( + DockerPatchValidationRunner, + PatchValidationProfile, + PatchValidationRequest, + PatchValidationResult, + PatchValidationStatus, +) + + +TEST_IMAGE = ( + f"{patch_validation.TRUSTED_PATCH_IMAGE_REPOSITORY}" + f"@sha256:{'a' * 64}" +) + + +def _patch() -> bytes: + """Return one ordinary text patch for a source-integrity test.""" + return ( + "diff --git a/src/example.ts b/src/example.ts\n" + "--- a/src/example.ts\n" + "+++ b/src/example.ts\n" + "@@ -1 +1 @@\n" + "-old\n" + "+new\n" + ).encode() + + +def _request( + patch_bytes: bytes, + *, + head_sha: str = "2" * 40, +) -> PatchValidationRequest: + """Build one exact-head-bound request for source-integrity testing.""" + return PatchValidationRequest( + repository_full_name="ContextualWisdomLab/noema", + base_sha="1" * 40, + head_sha=head_sha, + patch_sha256=hashlib.sha256(patch_bytes).hexdigest(), + profile=PatchValidationProfile.NODE_RELEASE_VERIFY, + ) + + +def _run_git(source: Path, *arguments: str) -> str: + """Run one bounded non-shell Git command for a temporary test repository.""" + completed = subprocess.run( + [patch_validation.TRUSTED_GIT_EXECUTABLE, "-C", str(source), *arguments], + check=True, + shell=False, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + timeout=30, + ) + return completed.stdout.strip() + + +def _mount_source(command: list[str], destination: str) -> Path: + """Return the host bind source for one exact Docker mount destination.""" + suffix = f",dst={destination}" + mount = next( + argument + for argument in command + if argument.startswith("--mount=") and suffix in argument + ) + return Path(mount.split("src=", 1)[1].split(",dst=", 1)[0]) + + +def _result_json(request: PatchValidationRequest) -> str: + """Return one successful result document bound to the exact request.""" + return PatchValidationResult( + status=PatchValidationStatus.PASSED, + repository_full_name=request.repository_full_name, + base_sha=request.base_sha, + head_sha=request.head_sha, + patch_sha256=request.patch_sha256, + profile=request.profile, + command_profile="npm run release:verify", + exit_code=0, + duration_ms=1, + stdout_excerpt="passed", + stderr_excerpt="", + reason_codes=[], + ).model_dump_json() + + +def _isolated_control( + staging: Path, + monkeypatch: pytest.MonkeyPatch, +) -> Path: + """Install one ambient-repository-free isolated-control test double.""" + control = staging / "isolated-control" + control.mkdir() + monkeypatch.setattr( + patch_validation, + "_create_isolated_git_control", + lambda *_args, **_kwargs: control, + ) + return control + + +def _bounded_tree_result() -> SimpleNamespace: + """Return one valid exact-tree record for archive-focused test doubles.""" + return SimpleNamespace( + returncode=0, + stdout=f"100644 blob {'a' * 40} 1\tfixture.txt\0", + ) + + +def test_runner_rejects_unverifiable_git_metadata_before_docker( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An invalid Git control directory cannot masquerade as an exact checkout.""" + source = tmp_path / "source" + source.mkdir() + (source / ".git").mkdir() + patch_bytes = _patch() + patch_path = tmp_path / "proposal.patch" + patch_path.write_bytes(patch_bytes) + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + + def should_not_run(_args, **_kwargs): + """Fail if unverifiable source metadata reaches Docker.""" + raise AssertionError("Docker must not start") + + with pytest.raises(RuntimeError, match="source HEAD could not be verified"): + DockerPatchValidationRunner(command_runner=should_not_run).validate( + request=_request(patch_bytes), + source_root=source, + patch_path=patch_path, + ) + + +def test_snapshot_materialization_rejects_git_archive_failure( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A failed exact-commit archive cannot fall back to the mutable worktree.""" + staging = tmp_path / "staging" + staging.mkdir() + _isolated_control(staging, monkeypatch) + + def failed_archive(command, **_kwargs): + """Pass exact-tree preflight but fail the subsequent archive command.""" + command_list = list(command) + if "ls-tree" in command_list: + return _bounded_tree_result() + if "archive" in command_list: + return SimpleNamespace(returncode=1) + raise AssertionError(f"unexpected Git command: {command_list}") + + monkeypatch.setattr(patch_validation.subprocess, "run", failed_archive) + + with pytest.raises(RuntimeError, match="snapshot could not be materialized"): + patch_validation._materialize_committed_source( + tmp_path, + "2" * 40, + staging, + "directory", + ) + + +def test_snapshot_materialization_rejects_invalid_archive( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Malformed archive bytes fail closed and the transient archive is removed.""" + staging = tmp_path / "staging" + staging.mkdir() + _isolated_control(staging, monkeypatch) + + def corrupt_archive(command, **_kwargs): + """Pass preflight, then write invalid bytes at Git's archive output path.""" + command_list = list(command) + if "ls-tree" in command_list: + return _bounded_tree_result() + if "archive" not in command_list: + raise AssertionError(f"unexpected Git command: {command_list}") + output = next( + argument.removeprefix("--output=") + for argument in command_list + if argument.startswith("--output=") + ) + Path(output).write_bytes(b"not a tar archive") + return SimpleNamespace(returncode=0) + + monkeypatch.setattr(patch_validation.subprocess, "run", corrupt_archive) + + with pytest.raises(RuntimeError, match="materialized safely"): + patch_validation._materialize_committed_source( + tmp_path, + "2" * 40, + staging, + "directory", + ) + assert not (staging / "source.tar").exists() + + +def test_runner_mounts_committed_snapshot_after_post_preflight_mutation( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Docker must receive committed bytes even when the worktree changes after preflight.""" + source = tmp_path / "source" + source.mkdir() + _run_git(source, "init") + _run_git(source, "config", "user.name", "Noema Test") + _run_git(source, "config", "user.email", "noema-test@example.invalid") + tracked = source / "src" / "example.ts" + tracked.parent.mkdir() + tracked.write_text("trusted\n", encoding="utf-8") + _run_git(source, "add", "src/example.ts") + _run_git(source, "commit", "-m", "trusted source") + head_sha = _run_git(source, "rev-parse", "HEAD") + + patch_bytes = _patch() + patch_path = tmp_path / "proposal.patch" + patch_path.write_bytes(patch_bytes) + request = _request(patch_bytes, head_sha=head_sha) + monkeypatch.setenv("NOEMA_PATCH_SANDBOX_IMAGE", TEST_IMAGE) + + verified = patch_validation._verify_source_head + + def mutate_after_preflight( + source_path: Path, + expected_head_sha: str, + metadata_kind: patch_validation.GitMetadataKind | None, + ) -> None: + """Mutate tracked content immediately after the trusted status check.""" + verified(source_path, expected_head_sha, metadata_kind) + tracked.write_text("attacker-controlled\n", encoding="utf-8") + + monkeypatch.setattr( + patch_validation, + "_verify_source_head", + mutate_after_preflight, + ) + + def inspect_snapshot(command, **_kwargs): + """Require a private exact-commit source mount and emit bounded evidence.""" + command_list = list(command) + mounted_source = _mount_source(command_list, "/input,readonly") + result_path = _mount_source(command_list, "/output/result.json") + assert mounted_source != source + assert (mounted_source / "src" / "example.ts").read_text( + encoding="utf-8" + ) == "trusted\n" + result_path.write_text( + _result_json(request), + encoding="utf-8", + ) + return SimpleNamespace(returncode=0) + + result = DockerPatchValidationRunner(command_runner=inspect_snapshot).validate( + request=request, + source_root=source, + patch_path=patch_path, + ) + assert result.status is PatchValidationStatus.PASSED diff --git a/reviewer/tests/test_patch_validation_streaming_edges.py b/reviewer/tests/test_patch_validation_streaming_edges.py new file mode 100644 index 00000000..529a4c01 --- /dev/null +++ b/reviewer/tests/test_patch_validation_streaming_edges.py @@ -0,0 +1,571 @@ +"""Branch-complete regressions for bounded Git child-process streaming.""" + +from __future__ import annotations + +import subprocess +import time +from pathlib import Path +from types import SimpleNamespace +from typing import Any + +import pytest + +from noema_reviewer import patch_validation + + +class _FakeStdout: + """Expose one deterministic descriptor and observable close state.""" + + def __init__(self) -> None: + """Initialize an open fake stdout pipe.""" + self.closed = False + + def fileno(self) -> int: + """Return a stable descriptor number for monkeypatched reads.""" + return 91 + + def close(self) -> None: + """Record parent-side pipe closure.""" + self.closed = True + + +class _FakeProcess: + """Model bounded poll, wait, terminate, kill, and stdout behavior.""" + + def __init__( + self, + *, + final_returncode: int = 0, + stdout: Any | None = None, + ) -> None: + """Initialize one running process with configurable terminal status.""" + self.stdout = _FakeStdout() if stdout is None else stdout + self.returncode: int | None = None + self.final_returncode = final_returncode + self.terminated = False + self.killed = False + self.wait_timeouts: list[float | None] = [] + + def poll(self) -> int | None: + """Return the current process state without changing it.""" + return self.returncode + + def wait(self, timeout: float | None = None) -> int: + """Complete the process and return its configured status.""" + self.wait_timeouts.append(timeout) + self.returncode = self.final_returncode + return self.returncode + + def terminate(self) -> None: + """Record graceful termination and a signal-like status.""" + self.terminated = True + self.returncode = -15 + + def kill(self) -> None: + """Record forced termination and a signal-like status.""" + self.killed = True + self.returncode = -9 + + +class _EscalatingProcess(_FakeProcess): + """Require terminate-to-kill escalation on the first bounded wait.""" + + def __init__(self) -> None: + """Initialize one process that ignores graceful termination once.""" + super().__init__() + self.wait_calls = 0 + + def wait(self, timeout: float | None = None) -> int: + """Time out once after terminate and complete after kill.""" + self.wait_timeouts.append(timeout) + self.wait_calls += 1 + if self.wait_calls == 1: + raise subprocess.TimeoutExpired("git", timeout) + self.returncode = -9 + return self.returncode + + +def _tree_record(path: str = "fixture.txt") -> bytes: + """Return one canonical NUL-terminated exact-tree blob record.""" + return f"100644 blob {'a' * 40} 1\t{path}\0".encode() + + +def _isolated_control( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> Path: + """Install one deterministic isolated-control factory for source checks.""" + control = tmp_path / "isolated-control" + control.mkdir() + monkeypatch.setattr( + patch_validation, + "_create_isolated_git_control", + lambda *_args, **_kwargs: control, + ) + return control + + +def test_remaining_process_timeout_accepts_positive_and_rejects_expired( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Shared process deadlines return positive time or raise deterministically.""" + monkeypatch.setattr(patch_validation.time, "monotonic", lambda: 10.0) + assert patch_validation._remaining_process_timeout(15.0) == 5.0 + with pytest.raises(subprocess.TimeoutExpired): + patch_validation._remaining_process_timeout(10.0) + + +def test_start_git_stream_uses_binary_isolated_process_contract( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Git streaming starts without shell, stdin, stderr, or ambient config.""" + marker = object() + observed: dict[str, Any] = {} + + def fake_popen(command, **kwargs): + """Capture the exact process contract and return one marker.""" + observed["command"] = command + observed["kwargs"] = kwargs + return marker + + monkeypatch.setattr(patch_validation.subprocess, "Popen", fake_popen) + + assert patch_validation._start_git_stream(["git", "status"]) is marker + assert observed["command"] == ["git", "status"] + assert observed["kwargs"]["stdin"] is subprocess.DEVNULL + assert observed["kwargs"]["stdout"] is subprocess.PIPE + assert observed["kwargs"]["stderr"] is subprocess.DEVNULL + assert observed["kwargs"]["shell"] is False + assert observed["kwargs"]["close_fds"] is True + assert observed["kwargs"]["env"]["GIT_CONFIG_NOSYSTEM"] == "1" + + +def test_read_git_stream_chunk_rejects_missing_stdout() -> None: + """A child without the required stdout pipe fails closed.""" + process = SimpleNamespace(stdout=None) + with pytest.raises(RuntimeError, match="stdout pipe is unavailable"): + patch_validation._read_git_stream_chunk( + process, + 1, + time.monotonic() + 1, + ) + + +def test_read_git_stream_chunk_rejects_select_timeout( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A pipe that never becomes readable consumes no unbounded wait time.""" + process = _FakeProcess() + monkeypatch.setattr( + patch_validation.select, + "select", + lambda *_args, **_kwargs: ([], [], []), + ) + with pytest.raises(subprocess.TimeoutExpired): + patch_validation._read_git_stream_chunk( + process, + 1, + time.monotonic() + 1, + ) + + +def test_wait_git_stream_uses_remaining_deadline( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Child waiting receives only the positive time left on the shared deadline.""" + process = _FakeProcess(final_returncode=7) + monkeypatch.setattr(patch_validation.time, "monotonic", lambda: 4.0) + assert patch_validation._wait_git_stream(process, 9.0) == 7 + assert process.wait_timeouts == [5.0] + + +def test_terminate_git_stream_skips_already_finished_process() -> None: + """An already-finished child is not signalled again.""" + process = _FakeProcess() + process.returncode = 0 + patch_validation._terminate_git_stream(process) + assert not process.terminated + assert not process.killed + + +def test_terminate_git_stream_completes_after_graceful_signal() -> None: + """A cooperative child is terminated and boundedly reaped.""" + process = _FakeProcess() + patch_validation._terminate_git_stream(process) + assert process.terminated + assert not process.killed + assert process.wait_timeouts == [ + patch_validation.GIT_STREAM_TERMINATION_TIMEOUT_SECONDS + ] + + +def test_terminate_git_stream_escalates_to_kill() -> None: + """A child ignoring terminate is killed and reaped within fixed bounds.""" + process = _EscalatingProcess() + patch_validation._terminate_git_stream(process) + assert process.terminated + assert process.killed + assert process.wait_timeouts == [ + patch_validation.GIT_STREAM_TERMINATION_TIMEOUT_SECONDS, + patch_validation.GIT_STREAM_TERMINATION_TIMEOUT_SECONDS, + ] + + +def test_close_git_stream_handles_absent_and_optional_close() -> None: + """Parent pipe cleanup tolerates absent processes, pipes, and close methods.""" + patch_validation._close_git_stream(None) + patch_validation._close_git_stream(SimpleNamespace(stdout=None)) + patch_validation._close_git_stream(SimpleNamespace(stdout=object())) + process = _FakeProcess() + stdout = process.stdout + patch_validation._close_git_stream(process) + assert stdout.closed + + +def test_verify_source_head_wraps_control_creation_failure( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Unavailable authenticated objects cannot fall back to caller Git controls.""" + + def fail_control(*_args, **_kwargs): + """Emulate failure to construct isolated Git metadata.""" + raise RuntimeError("objects unavailable") + + monkeypatch.setattr( + patch_validation, + "_create_isolated_git_control", + fail_control, + ) + with pytest.raises(RuntimeError, match="source HEAD could not be verified"): + patch_validation._verify_source_head( + tmp_path, + "1" * 40, + "directory", + ) + + +def test_verify_source_head_rejects_read_tree_failure( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Failure to populate the exact-head index blocks status inspection.""" + _isolated_control(tmp_path, monkeypatch) + monkeypatch.setattr( + patch_validation.subprocess, + "run", + lambda *_args, **_kwargs: SimpleNamespace(returncode=1), + ) + with pytest.raises(RuntimeError, match="does not match the exact validation request"): + patch_validation._verify_source_head( + tmp_path, + "1" * 40, + "directory", + ) + + +def test_verify_source_head_accepts_clean_zero_exit_stream( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An empty status stream and zero exit authenticate one clean exact head.""" + _isolated_control(tmp_path, monkeypatch) + process = _FakeProcess() + monkeypatch.setattr( + patch_validation.subprocess, + "run", + lambda *_args, **_kwargs: SimpleNamespace(returncode=0), + ) + monkeypatch.setattr( + patch_validation, + "_start_git_stream", + lambda _command: process, + ) + monkeypatch.setattr( + patch_validation, + "_read_git_stream_chunk", + lambda *_args, **_kwargs: b"", + ) + monkeypatch.setattr( + patch_validation, + "_wait_git_stream", + lambda *_args, **_kwargs: 0, + ) + + patch_validation._verify_source_head( + tmp_path, + "1" * 40, + "directory", + ) + assert process.stdout.closed + + +def test_verify_source_head_wraps_stream_start_failure( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An operating-system launch failure cannot authenticate a clean source.""" + _isolated_control(tmp_path, monkeypatch) + monkeypatch.setattr( + patch_validation.subprocess, + "run", + lambda *_args, **_kwargs: SimpleNamespace(returncode=0), + ) + monkeypatch.setattr( + patch_validation, + "_start_git_stream", + lambda _command: (_ for _ in ()).throw(OSError("status unavailable")), + ) + + with pytest.raises(RuntimeError, match="source HEAD could not be verified"): + patch_validation._verify_source_head( + tmp_path, + "1" * 40, + "directory", + ) + + +@pytest.mark.parametrize( + "failure", + ( + OSError("status read failed"), + subprocess.TimeoutExpired("git", 30), + ), +) +def test_verify_source_head_terminates_stream_read_failure( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + failure: BaseException, +) -> None: + """A status read error terminates the child and fails exact-head verification.""" + _isolated_control(tmp_path, monkeypatch) + process = _FakeProcess() + monkeypatch.setattr( + patch_validation.subprocess, + "run", + lambda *_args, **_kwargs: SimpleNamespace(returncode=0), + ) + monkeypatch.setattr( + patch_validation, + "_start_git_stream", + lambda _command: process, + ) + + def fail_read(*_args, **_kwargs): + """Raise the configured status-stream failure.""" + raise failure + + monkeypatch.setattr(patch_validation, "_read_git_stream_chunk", fail_read) + + with pytest.raises(RuntimeError, match="source HEAD could not be verified"): + patch_validation._verify_source_head( + tmp_path, + "1" * 40, + "directory", + ) + assert process.terminated + assert process.stdout.closed + + +def test_exact_tree_record_rejects_record_byte_ceiling( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """One record is rejected before unbounded metadata or path decoding.""" + monkeypatch.setattr(patch_validation, "MAX_SOURCE_TREE_RECORD_BYTES", 3) + with pytest.raises(ValueError, match="record exceeds its byte limit"): + patch_validation._validated_exact_tree_record( + b"abcd", + set(), + 0, + ) + + +@pytest.mark.parametrize( + "record", + ( + b"\xff\tpath", + f"100644 blob {'a' * 40} 1\t".encode() + b"\xff", + ), +) +def test_exact_tree_record_rejects_invalid_utf8(record: bytes) -> None: + """Metadata and path bytes must decode as strict UTF-8.""" + with pytest.raises(ValueError, match="valid UTF-8"): + patch_validation._validated_exact_tree_record(record, set(), 0) + + +def test_exact_tree_output_rejects_unencodable_text() -> None: + """A surrogate-bearing compatibility input cannot become binary evidence.""" + with pytest.raises(ValueError, match="valid UTF-8"): + patch_validation._validated_exact_tree_output("\ud800\0") + + +def test_exact_tree_output_rejects_aggregate_metadata_ceiling( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Compatibility parsing enforces the same aggregate metadata byte bound.""" + monkeypatch.setattr(patch_validation, "MAX_SOURCE_TREE_METADATA_BYTES", 1) + with pytest.raises(ValueError, match="metadata exceeds its aggregate byte limit"): + patch_validation._validated_exact_tree_output( + _tree_record().decode(), + ) + + +def test_consume_exact_tree_stream_accepts_partial_records( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """NUL records split across bounded chunks are validated incrementally.""" + process = _FakeProcess() + record = _tree_record() + chunks = iter((record[:17], record[17:], b"")) + monkeypatch.setattr( + patch_validation, + "_read_git_stream_chunk", + lambda *_args, **_kwargs: next(chunks), + ) + monkeypatch.setattr( + patch_validation, + "_wait_git_stream", + lambda *_args, **_kwargs: 0, + ) + patch_validation._consume_exact_tree_stream( + process, + time.monotonic() + 1, + ) + + +def test_consume_exact_tree_stream_rejects_unterminated_record_ceiling( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A record without a delimiter cannot grow beyond its byte ceiling.""" + process = _FakeProcess() + monkeypatch.setattr(patch_validation, "MAX_SOURCE_TREE_RECORD_BYTES", 4) + monkeypatch.setattr( + patch_validation, + "_read_git_stream_chunk", + lambda *_args, **_kwargs: b"12345", + ) + with pytest.raises(ValueError, match="record exceeds its byte limit"): + patch_validation._consume_exact_tree_stream( + process, + time.monotonic() + 1, + ) + + +def test_consume_exact_tree_stream_rejects_member_count( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Streaming validation stops as soon as the member ceiling is exceeded.""" + process = _FakeProcess() + monkeypatch.setattr(patch_validation, "MAX_SOURCE_ARCHIVE_MEMBERS", 1) + chunks = iter((_tree_record("one.txt") + _tree_record("two.txt"), b"")) + monkeypatch.setattr( + patch_validation, + "_read_git_stream_chunk", + lambda *_args, **_kwargs: next(chunks), + ) + with pytest.raises(ValueError, match="too many members"): + patch_validation._consume_exact_tree_stream( + process, + time.monotonic() + 1, + ) + + +@pytest.mark.parametrize( + "chunks", + ( + (b"",), + (b"truncated", b""), + ), +) +def test_consume_exact_tree_stream_rejects_empty_or_truncated_output( + monkeypatch: pytest.MonkeyPatch, + chunks: tuple[bytes, ...], +) -> None: + """An empty stream or final partial record is not admissible evidence.""" + process = _FakeProcess() + values = iter(chunks) + monkeypatch.setattr( + patch_validation, + "_read_git_stream_chunk", + lambda *_args, **_kwargs: next(values), + ) + with pytest.raises(ValueError, match="empty or truncated"): + patch_validation._consume_exact_tree_stream( + process, + time.monotonic() + 1, + ) + + +def test_consume_exact_tree_stream_rejects_nonzero_exit( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Valid records from a failed Git child remain inadmissible.""" + process = _FakeProcess(final_returncode=1) + chunks = iter((_tree_record(), b"")) + monkeypatch.setattr( + patch_validation, + "_read_git_stream_chunk", + lambda *_args, **_kwargs: next(chunks), + ) + monkeypatch.setattr( + patch_validation, + "_wait_git_stream", + lambda *_args, **_kwargs: 1, + ) + with pytest.raises(RuntimeError, match="exact tree command failed"): + patch_validation._consume_exact_tree_stream( + process, + time.monotonic() + 1, + ) + + +def test_verify_exact_tree_limits_accepts_valid_stream_and_closes_pipe( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A bounded valid exact-tree stream succeeds and closes parent stdout.""" + process = _FakeProcess() + chunks = iter((_tree_record(), b"")) + monkeypatch.setattr( + patch_validation, + "_start_git_stream", + lambda _command: process, + ) + monkeypatch.setattr( + patch_validation, + "_read_git_stream_chunk", + lambda *_args, **_kwargs: next(chunks), + ) + monkeypatch.setattr( + patch_validation, + "_wait_git_stream", + lambda *_args, **_kwargs: 0, + ) + + patch_validation._verify_exact_tree_limits(tmp_path, "1" * 40) + assert process.stdout.closed + + +def test_verify_exact_tree_limits_wraps_stream_timeout( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A timed-out exact-tree stream is terminated and fails closed.""" + process = _FakeProcess() + monkeypatch.setattr( + patch_validation, + "_start_git_stream", + lambda _command: process, + ) + monkeypatch.setattr( + patch_validation, + "_consume_exact_tree_stream", + lambda *_args, **_kwargs: (_ for _ in ()).throw( + subprocess.TimeoutExpired("git", 30) + ), + ) + + with pytest.raises(RuntimeError, match="could not be inspected safely"): + patch_validation._verify_exact_tree_limits(tmp_path, "1" * 40) + assert process.terminated + assert process.stdout.closed diff --git a/reviewer/tests/test_patch_validation_streaming_final_edges.py b/reviewer/tests/test_patch_validation_streaming_final_edges.py new file mode 100644 index 00000000..7a109bfd --- /dev/null +++ b/reviewer/tests/test_patch_validation_streaming_final_edges.py @@ -0,0 +1,111 @@ +"""Final branch regressions for streamed exact-head and archive evidence.""" + +from __future__ import annotations + +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from noema_reviewer import patch_validation + + +def _install_source_preflight( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Install one successful isolated-control and read-tree preflight.""" + control = tmp_path / "isolated-control" + control.mkdir() + monkeypatch.setattr( + patch_validation, + "_create_isolated_git_control", + lambda *_args, **_kwargs: control, + ) + monkeypatch.setattr( + patch_validation.subprocess, + "run", + lambda *_args, **_kwargs: SimpleNamespace(returncode=0), + ) + + +def test_source_head_runtime_start_failure_has_no_process_to_terminate( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A runtime failure before process assignment is re-raised without cleanup.""" + _install_source_preflight(tmp_path, monkeypatch) + monkeypatch.setattr( + patch_validation, + "_start_git_stream", + lambda _command: (_ for _ in ()).throw(RuntimeError("invalid stream")), + ) + + with pytest.raises(RuntimeError, match="invalid stream"): + patch_validation._verify_source_head( + tmp_path, + "1" * 40, + "directory", + ) + + +def test_exact_tree_value_failure_has_no_process_to_terminate( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A bounded-validation error before assignment still receives safe wrapping.""" + monkeypatch.setattr( + patch_validation, + "_start_git_stream", + lambda _command: (_ for _ in ()).throw(ValueError("invalid tree")), + ) + + with pytest.raises(RuntimeError, match="failed bounded validation"): + patch_validation._verify_exact_tree_limits(tmp_path, "1" * 40) + + +def test_exact_tree_record_rejects_oversized_blob( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """One blob above the source-file ceiling is rejected before archiving.""" + monkeypatch.setattr(patch_validation, "MAX_SOURCE_ARCHIVE_MEMBER_BYTES", 0) + record = f"100644 blob {'a' * 40} 1\tfixture.txt".encode() + + with pytest.raises(ValueError, match="member exceeds its byte limit"): + patch_validation._validated_exact_tree_record(record, set(), 0) + + +def test_materialization_rejects_archive_command_failure_after_tree_preflight( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A nonzero archive command cannot yield a committed source snapshot.""" + source = tmp_path / "source-root" + source.mkdir() + staging = tmp_path / "staging-root" + staging.mkdir() + control = staging / "isolated-control" + control.mkdir() + monkeypatch.setattr( + patch_validation, + "_create_isolated_git_control", + lambda *_args, **_kwargs: control, + ) + monkeypatch.setattr( + patch_validation, + "_verify_exact_tree_limits", + lambda *_args, **_kwargs: None, + ) + monkeypatch.setattr( + patch_validation.subprocess, + "run", + lambda *_args, **_kwargs: SimpleNamespace(returncode=1), + ) + + with pytest.raises(RuntimeError, match="snapshot could not be materialized"): + patch_validation._materialize_committed_source( + source, + "1" * 40, + staging, + "directory", + ) diff --git a/reviewer/tests/test_patch_validation_streaming_git_output.py b/reviewer/tests/test_patch_validation_streaming_git_output.py new file mode 100644 index 00000000..dad0dccc --- /dev/null +++ b/reviewer/tests/test_patch_validation_streaming_git_output.py @@ -0,0 +1,150 @@ +"""Streaming regressions for hostile Git status and exact-tree output.""" + +from __future__ import annotations + +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from noema_reviewer import patch_validation + + +class _FakeStdout: + """Expose one stable descriptor identity for a fake child stdout pipe.""" + + def fileno(self) -> int: + """Return a deterministic descriptor used by monkeypatched reads.""" + return 91 + + +class _FakeProcess: + """Record bounded termination and wait behavior for one fake Git child.""" + + def __init__(self, *, returncode: int = 0) -> None: + """Initialize a running child with a fake stdout descriptor.""" + self.stdout = _FakeStdout() + self.returncode: int | None = None + self.final_returncode = returncode + self.terminated = False + self.killed = False + + def poll(self) -> int | None: + """Return the current child state without changing it.""" + return self.returncode + + def wait(self, timeout: float | None = None) -> int: + """Complete the child and return its configured exit status.""" + del timeout + self.returncode = self.final_returncode + return self.returncode + + def terminate(self) -> None: + """Record graceful early termination.""" + self.terminated = True + self.returncode = -15 + + def kill(self) -> None: + """Record forced early termination.""" + self.killed = True + self.returncode = -9 + + +def _ready(*_args, **_kwargs): + """Report the fake stdout descriptor as immediately readable.""" + return ([_args[0][0]], [], []) + + +def test_exact_tree_reader_stops_at_aggregate_output_ceiling( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Exact-tree output is terminated after at most the configured ceiling plus one.""" + process = _FakeProcess() + requested_sizes: list[int] = [] + chunks = iter((b"x" * 33,)) + monkeypatch.setattr(patch_validation, "MAX_SOURCE_TREE_METADATA_BYTES", 32) + monkeypatch.setattr( + patch_validation.subprocess, + "Popen", + lambda *_args, **_kwargs: process, + ) + monkeypatch.setattr( + patch_validation.subprocess, + "run", + lambda *_args, **_kwargs: (_ for _ in ()).throw( + AssertionError("exact-tree output must not use subprocess.run capture") + ), + ) + monkeypatch.setattr(patch_validation.select, "select", _ready) + + def bounded_read(_descriptor: int, size: int) -> bytes: + """Record the requested size and return one over-limit chunk.""" + requested_sizes.append(size) + return next(chunks) + + monkeypatch.setattr(patch_validation.os, "read", bounded_read) + + with pytest.raises(RuntimeError, match="bounded validation"): + patch_validation._verify_exact_tree_limits(tmp_path, "1" * 40) + + assert requested_sizes == [33] + assert process.terminated + assert not process.killed + + +def test_dirty_status_reads_one_byte_then_terminates( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The first dirty-worktree byte is sufficient to stop status collection.""" + source = tmp_path / "source" + source.mkdir() + staging_control = tmp_path / "isolated-control" + staging_control.mkdir() + process = _FakeProcess() + requested_sizes: list[int] = [] + monkeypatch.setattr( + patch_validation, + "_create_isolated_git_control", + lambda *_args, **_kwargs: staging_control, + ) + + def bounded_run(command, **_kwargs): + """Permit only the exact-head index population command.""" + if "read-tree" not in list(command): + raise AssertionError("status output must use bounded streaming") + return SimpleNamespace(returncode=0) + + monkeypatch.setattr(patch_validation.subprocess, "run", bounded_run) + monkeypatch.setattr( + patch_validation.subprocess, + "Popen", + lambda *_args, **_kwargs: process, + ) + monkeypatch.setattr(patch_validation.select, "select", _ready) + + def one_byte(_descriptor: int, size: int) -> bytes: + """Return the first dirty byte and prove the read is capped at one.""" + requested_sizes.append(size) + return b"?" + + monkeypatch.setattr(patch_validation.os, "read", one_byte) + + with pytest.raises(RuntimeError, match="worktree is not clean"): + patch_validation._verify_source_head(source, "2" * 40, "directory") + + assert requested_sizes == [1] + assert process.terminated + assert not process.killed + + +def test_exact_tree_rejects_oversized_path_before_decoding( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """One hostile path cannot create an unbounded record buffer.""" + monkeypatch.setattr(patch_validation, "MAX_SOURCE_TREE_PATH_BYTES", 4) + record = f"100644 blob {'a' * 40} 1\tlong-path.txt\0" + + with pytest.raises(ValueError, match="path byte limit"): + patch_validation._validated_exact_tree_output(record)