diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/REVIEW_LOG.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/REVIEW_LOG.md index 802ca8d3..9fd6905f 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/REVIEW_LOG.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/REVIEW_LOG.md @@ -215,3 +215,29 @@ fixture-only resource probes, and expanded boundary/migration evidence. - Architecture, security, senior engineering, product/ops, QA, docs, reuse, CI-integrity, and test-delta repair reviews pass with no blockers. + +## WS-ART-001-03B3B1 + +- Selected the minimal complex-format dependency set: `pypdf==6.14.2`, + `defusedxml==0.7.1`, and `Pillow==12.3.0`; no package or lock change occurs. +- Initial security, QA, senior-engineering, and product/ops review found that + version-only declarations did not bind installed wheel hashes, excluded + OOXML packages and optional groups could bypass the gate, format scopes were + global, and wheel identity/platform coherence was under-specified. +- Repairs require exact hash-bound PyPI wheel references, scan runtime, + optional, and dependency-group declarations, reject the prohibited OOXML + package set, validate wheel identity/tags/platform, and scope imports per + parser module. +- Focused proof passes with 34 tests and 92.94 percent checker coverage. + Architecture, security, QA, product/ops, senior engineering, CI integrity, + reuse/dedup, and test-delta reviews pass after repair. Docs confirmation and + exact hosted PR-head behavior remain publication evidence. +- The first approval-authorized hosted run reached the canonical lane inventory + and correctly failed because the new focused test module had not been assigned + to a lane. The repair adds it to `shared_foundations`; no lane, test, or + coverage policy is weakened. +- CodeRabbit's later review found three valid issues: ambiguous final-head + wording, unstable type failures, and `COMMENTED` reviews erasing approval. + All are repaired with regression proof. Its review-event checkout concern is + rejected because hosted review-event run `30600808957` passed exact-head + approval and executed the PR's lane inventory before its later failure. diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md index 44824ca3..0d0252ef 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md @@ -67,6 +67,12 @@ approval, 03B3B2 PDF, 03B3B3A shared OOXML security, separate 03B3B3B DOCX, 03B3B3C PPTX, and 03B3B3D XLSX adapters, and 03B3B4 image metadata. No dependency is installed until the human owner approves 03B3B1's exact pinned allowlist. +03B3B1 implementation is active as a dependency-decision and CI-only chunk. +It proposes exact hashed wheels for `pypdf`, `defusedxml`, and `Pillow`, with +no package, lock, runtime import, or parser behavior change. Its approval gate +requires independent protected GitHub review of the exact final PR head before +merge; repository-authored evidence alone is not authority. + AUTH `WS-XINT-002-04B` follows the complete hidden split-03B series and activates only fixed-service binding and guide read. ART-03C then removes the legacy identity/excerpt path and makes the verified pipeline authoritative. diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B1-parser-dependency-approval.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B1-parser-dependency-approval.md index 8078ff62..7ab47819 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B1-parser-dependency-approval.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B3B1-parser-dependency-approval.md @@ -31,6 +31,7 @@ P2 ```text backend/config/guide_extractor_dependencies.json backend/scripts/check_guide_extractor_dependencies.py +backend/scripts/run_test_lanes.py backend/tests/test_guide_extractor_dependencies.py .github/workflows/backend.yml .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/** @@ -71,11 +72,11 @@ containing real customer data, AUTH/Celery/submission changes. ## Verification commands ```bash -(cd backend && python scripts/check_guide_extractor_dependencies.py) -(cd backend && uv run pytest -q tests/test_guide_extractor_dependencies.py) +(cd backend && python3 scripts/check_guide_extractor_dependencies.py) +(cd backend && PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python3 -m pytest -q -p pytest_cov.plugin tests/test_guide_extractor_dependencies.py --cov=scripts.check_guide_extractor_dependencies --cov-report=term-missing --cov-fail-under=90) python3 scripts/check_markdown_links.py python3 scripts/check_stale_artifact_contracts.py -python3 scripts/test_agent_gates.py +python3 -m unittest -v scripts.test_lightweight_agent_gates git diff --check ``` diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B3B1-external-review-response.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B3B1-external-review-response.md new file mode 100644 index 00000000..945ffd0f --- /dev/null +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B3B1-external-review-response.md @@ -0,0 +1,44 @@ +# WS-ART-001-03B3B1 External Review Response + +## Comments addressed + +- GitHub Backend run `30600808957` passed the exact-head approval gate, then + failed canonical collection with + `missing_lane_modules:tests/test_guide_extractor_dependencies.py`. +- The focused test module is now assigned to the existing + `shared_foundations` semantic lane. No new lane or coverage exception was + introduced. + +## Comments deferred + +- None. + +## Human decisions needed + +- A fresh independent approval of the exact final PR head is pending. + `abiorh-claw` approved `66ac70e6` before the CI repair, so that approval was + correctly dismissed when the head changed. + +## CodeRabbit + +- The initial check returned `pass` with `Review rate limited` and no comments. +- The repaired-head review posted four findings. Three valid findings are + addressed: exact-final-head wording, stable type-validation failures, and + preserving an active approval across a later `COMMENTED` review. +- The workflow-checkout finding is rejected with direct hosted evidence: review + event run `30600808957` passed exact-head approval against `66ac70e6`, executed + the PR's new code, and reached canonical lane collection. The checkout also + uses `fetch-depth: 0`, so both PR parents required by the diff were present. + +## Commands rerun + +- Focused dependency gate and tests. +- Stable malformed-value and approval-then-comment regression tests. +- Canonical semantic-lane collect-only validation. +- Ruff, markdown links, stale-contract checks, lightweight agent gates, and + diff checks. + +## Remaining risks + +- Hosted Backend and Agent Gates must pass on the repaired exact head after a + fresh independent approval. diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B3B1-pr-trust-bundle.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B3B1-pr-trust-bundle.md new file mode 100644 index 00000000..5da41563 --- /dev/null +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B3B1-pr-trust-bundle.md @@ -0,0 +1,131 @@ +# WS-ART-001-03B3B1 PR Trust Bundle + +## Chunk + +`WS-ART-001-03B3B1` — Parser Dependency Approval. + +## Goal + +Approve and enforce the smallest exact PDF, OOXML-safety, and image-metadata +dependency set without installing packages or changing runtime behavior. + +## Human-approved intent + +The merged 03B3B split contract requires dependency approval before any PDF, +OOXML, or image parser implementation. The human owner explicitly started this +chunk after PR #228 merged. + +## What changed + +- Added a canonical allowlist for `pypdf==6.14.2`, `defusedxml==0.7.1`, and + `Pillow==12.3.0`, including exact approved wheel URLs and SHA-256 hashes. +- Added a deterministic checker for manifest shape, wheel identity/platform, + hash-bound future declarations, prohibited packages, format-scoped imports, + and independent exact-head GitHub approval. +- Added focused failure tests and wired the gate before backend installation. +- Registered the focused test module in the canonical `shared_foundations` + semantic lane after hosted inventory validation exposed the missing entry. +- Updated the ART specification, chunk command, status, and review evidence. + +## Why it changed + +Untrusted document parsers are a supply-chain boundary. A package name or +version alone cannot prove which bytes will later be installed, and a +repository-authored approval file cannot independently authorize itself. + +## Design chosen + +- PDF: pure-Python `pypdf`. +- OOXML safety: pure-Python `defusedxml` plus later ART-owned bounded container + and format adapters. +- Image metadata: native `Pillow`, restricted to two named Python 3.11/3.12 + manylinux x86_64 wheels. +- Future runtime declarations must use the exact approved wheel URL with its + SHA-256 fragment. Optional/dependency-group parser declarations are denied. +- Allowlist changes require a current approving GitHub review from an + independent human owner/member/collaborator on the exact PR head. + +## Alternatives rejected + +- `python-docx`, `python-pptx`, `openpyxl`, `lxml`, and `XlsxWriter`: larger, + unnecessary graphs for the bounded v0.1 OOXML extraction design. +- Version-only pins: do not bind installed artifact bytes. +- Repository-local approval as authority: forgeable by the proposing PR. +- `pull_request_target`: an unnecessary privileged workflow boundary. + +## Scope control + +No `pyproject.toml`, lockfile, dependency install, application import, parser +adapter, AUTH, Celery, submission, or product lifecycle change is included. + +## Product behavior + +None. Project Manager guide-source handling remains distinct from contributor +one-ZIP submission handling. All parser behavior remains unimplemented. + +## Acceptance criteria proof + +- Canonical schema covers version, license, maintenance, advisories, + transitive graph, native-code, malformed-input, network, cancellation, + import, format, source, wheel URL, and hash facts. +- Three closed scopes are present exactly once. +- Plain pins, wrong hashes/URLs/wheels/tags/scopes, prohibited packages, + optional-group bypasses, cross-format imports, self/bot/stale/dismissed + approvals, and GitHub API failure all reject in focused tests. + +## Tests/checks run + +- Manifest checker: pass. +- Focused pytest: 34 pass; checker coverage 92.94 percent. +- Ruff lint and format: pass. +- Compile, markdown links, stale artifact/wording, lightweight agent gates, + and `git diff --check`: pass. + +## Test delta + +One new focused test module; no tests removed, skipped, or weakened. + +## CI integrity + +The gate runs before package installation. Existing backend tests, semantic +lanes, provider proof, and 78/90 percent coverage floors are unchanged. Token +permissions remain read-only: `contents: read` and `pull-requests: read`. + +## Reviewer results + +Architecture, security, QA, product/ops, senior engineering, CI integrity, +reuse/dedup, and test delta pass after valid findings were repaired. Docs +review is rerun on the staged evidence. + +## External review + +CodeRabbit's later substantive review produced three valid repaired findings +and one workflow-checkout finding rejected by direct hosted evidence. The first approval-authorized +Backend run found one missing semantic-lane inventory entry, now repaired. +Exact repaired-head hosted results remain required external evidence. + +## Remaining risks + +- Live GitHub review-event/check semantics require hosted proof. +- Review submitted/dismissed events rerun the full Backend job; this is an + accepted low operational cost for the v0.1 fail-closed required check. +- `defusedxml` is a stable, slow-release security utility; its isolation and + bounded OOXML use remain mandatory in later chunks. + +## Follow-up work + +After this chunk merges, `WS-ART-001-03B3B2` may install only the approved PDF +wheel and implement bounded PDF extraction. Later OOXML and image chunks remain +separate and require explicit starts. + +## Human review focus + +Confirm the three-package minimum, wheel URL/hash selection, native Pillow +platform bounds, prohibited package set, and independent exact-head approval +semantics. + +## Human merge ownership + +Only the human owner may approve and merge this PR. The PR must receive an +independent qualifying GitHub approval on its exact final head; this agent will +not merge it. diff --git a/.github/workflows/backend.yml b/.github/workflows/backend.yml index 1e6b2a07..6accf551 100644 --- a/.github/workflows/backend.yml +++ b/.github/workflows/backend.yml @@ -2,12 +2,17 @@ name: Backend on: pull_request: + pull_request_review: + types: + - submitted + - dismissed push: branches: - main permissions: contents: read + pull-requests: read env: MINIO_IMAGE: quay.io/minio/minio:latest@sha256:14cea493d9a34af32f524e538b8346cf79f3321eff8e708c1e2960462bd8936e @@ -55,6 +60,23 @@ jobs: echo "job_start_epoch=${job_start_epoch}" >> "${GITHUB_OUTPUT}" echo "tree_sha=${tree_sha}" >> "${GITHUB_OUTPUT}" + - name: Guide extractor dependency approval + working-directory: backend + env: + GITHUB_TOKEN: ${{ github.token }} + WORKSTREAM_PR_AUTHOR: ${{ github.event.pull_request.user.login }} + WORKSTREAM_PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + WORKSTREAM_PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + WORKSTREAM_PR_NUMBER: ${{ github.event.pull_request.number }} + shell: bash + run: | + set -euo pipefail + approval_args=() + if [[ "${GITHUB_EVENT_NAME}" != "push" ]]; then + approval_args+=(--require-pr-approval) + fi + python scripts/check_guide_extractor_dependencies.py "${approval_args[@]}" + - name: Install backend and exact Ruff working-directory: backend run: | diff --git a/backend/config/guide_extractor_dependencies.json b/backend/config/guide_extractor_dependencies.json new file mode 100644 index 00000000..23561f14 --- /dev/null +++ b/backend/config/guide_extractor_dependencies.json @@ -0,0 +1,127 @@ +{ + "advisory_snapshot": { + "checked_at": "2026-07-30", + "provider": "OSV", + "query_url": "https://api.osv.dev/v1/query" + }, + "dependencies": [ + { + "advisories": [], + "approved_artifacts": [ + { + "filename": "pypdf-6.14.2-py3-none-any.whl", + "sha256": "3f07891af76dc002657e04993ab9b4de81de29f9013b9761d0b7968bff12e946", + "url": "https://files.pythonhosted.org/packages/49/e6/136aa8993a2ae7214e0b0ef2edaa0d2e08d1d4e4982635b08a835ff31ec8/pypdf-6.14.2-py3-none-any.whl" + } + ], + "cancellation_timeout_implications": "Pure-Python parsing remains confined to the existing killable extraction child; cancellation never relies on cooperative parser shutdown.", + "direct": true, + "format_scopes": [ + "pdf" + ], + "import_names": [ + "pypdf" + ], + "license": "BSD-3-Clause", + "maintenance": { + "assessment": "active", + "release_uploaded_at": "2026-06-23T14:18:28.867545Z", + "source_url": "https://github.com/py-pdf/pypdf" + }, + "malformed_input_history": "A mature untrusted-PDF parser with recurring malformed-input and denial-of-service hardening; ART still rejects active features and enforces page, byte, CPU, memory, and wall-clock limits outside the library.", + "name": "pypdf", + "native_code": false, + "network_behavior": "No network access is required for approved PDF text extraction; the extraction child remains no-network.", + "source_index": "https://pypi.org/simple", + "transitive_dependencies": [], + "version": "6.14.2" + }, + { + "advisories": [], + "approved_artifacts": [ + { + "filename": "defusedxml-0.7.1-py2.py3-none-any.whl", + "sha256": "a352e7e428770286cc899e2542b6cdaedb2b4953ff269a210103ec58f6198a61", + "url": "https://files.pythonhosted.org/packages/07/6c/aa3f2f849e01cb6a001cd8554a88d4c77c5c1a31c95bdf1cf9301e6d9ef4/defusedxml-0.7.1-py2.py3-none-any.whl" + } + ], + "cancellation_timeout_implications": "Pure-Python XML safety checks run only inside the existing killable extraction child and remain subject to container, CPU, memory, and wall-clock limits.", + "direct": true, + "format_scopes": [ + "ooxml" + ], + "import_names": [ + "defusedxml" + ], + "license": "PSF-2.0", + "maintenance": { + "assessment": "stable_security_utility", + "release_uploaded_at": "2021-03-08T10:59:24.450195Z", + "source_url": "https://github.com/tiran/defusedxml" + }, + "malformed_input_history": "Purpose-built rejection of XML entity expansion and external entity hazards; ART additionally rejects DTD/entity declarations before adapter dispatch and bounds the surrounding ZIP container.", + "name": "defusedxml", + "native_code": false, + "network_behavior": "No network access is required; external entities and external relationships remain rejected and the extraction child remains no-network.", + "source_index": "https://pypi.org/simple", + "transitive_dependencies": [], + "version": "0.7.1" + }, + { + "advisories": [], + "approved_artifacts": [ + { + "filename": "pillow-12.3.0-cp311-cp311-manylinux_2_27_x86_64.manylinux_2_28_x86_64.whl", + "python_version": "3.11", + "sha256": "23d27a3e0307ec2244cc51e7287b919aa68d097504ebe19df4e76a98a3eea5bd", + "url": "https://files.pythonhosted.org/packages/3b/2d/ede717bc1144f63886c21fd349bb95860b0d1a21149ff16f2bb362b612b6/pillow-12.3.0-cp311-cp311-manylinux_2_27_x86_64.manylinux_2_28_x86_64.whl" + }, + { + "filename": "pillow-12.3.0-cp312-cp312-manylinux_2_27_x86_64.manylinux_2_28_x86_64.whl", + "python_version": "3.12", + "sha256": "78cb2c6865a35ab8ff8b75fd122f6033b92a62c82801110e48ddd6c936a45d91", + "url": "https://files.pythonhosted.org/packages/84/21/a35af28dcc61f37ed850a2d64c65c701321dfbf25085e469d5559360cbbf/pillow-12.3.0-cp312-cp312-manylinux_2_27_x86_64.manylinux_2_28_x86_64.whl" + } + ], + "cancellation_timeout_implications": "Native decoders can block or crash, so all header inspection and metadata extraction remains in the killable child with pre-decode dimension checks and OS resource limits.", + "direct": true, + "format_scopes": [ + "image_metadata" + ], + "import_names": [ + "PIL" + ], + "license": "MIT-CMU", + "maintenance": { + "assessment": "active", + "release_uploaded_at": "2026-07-01T11:53:53.487322Z", + "source_url": "https://github.com/python-pillow/Pillow" + }, + "malformed_input_history": "Native image codecs have a material malformed-input and decompression-bomb history; ART uses metadata-only output, fixed format allowlists, pre-allocation dimension limits, and subprocess termination.", + "name": "Pillow", + "native_code": true, + "network_behavior": "No network access is required for local PNG, JPEG, or WebP metadata; the extraction child remains no-network.", + "source_index": "https://pypi.org/simple", + "transitive_dependencies": [], + "version": "12.3.0" + } + ], + "installation_policy": { + "allow_source_distributions": false, + "approved_python_versions": [ + "3.11", + "3.12" + ], + "approved_runtime_platform": "manylinux x86_64", + "index": "https://pypi.org/simple", + "require_hashes": true + }, + "prohibited_packages": [ + "lxml", + "openpyxl", + "python-docx", + "python-pptx", + "XlsxWriter" + ], + "schema_version": 1 +} diff --git a/backend/scripts/check_guide_extractor_dependencies.py b/backend/scripts/check_guide_extractor_dependencies.py new file mode 100644 index 00000000..8affd45f --- /dev/null +++ b/backend/scripts/check_guide_extractor_dependencies.py @@ -0,0 +1,528 @@ +#!/usr/bin/env python3 +"""Validate the approved guide-extractor dependency boundary.""" + +from __future__ import annotations + +import argparse +import ast +import hashlib +import json +import os +import re +import subprocess +import sys +import tomllib +import urllib.error +import urllib.request +from pathlib import Path +from typing import Any + +BACKEND_ROOT = Path(__file__).resolve().parents[1] +REPOSITORY_ROOT = BACKEND_ROOT.parent +ALLOWLIST_PATH = BACKEND_ROOT / "config" / "guide_extractor_dependencies.json" +PYPROJECT_PATH = BACKEND_ROOT / "pyproject.toml" +ALLOWLIST_REPOSITORY_PATH = ALLOWLIST_PATH.relative_to(REPOSITORY_ROOT).as_posix() +APPROVED_ASSOCIATIONS = frozenset({"OWNER", "MEMBER", "COLLABORATOR"}) +APPROVED_SCOPES = frozenset({"pdf", "ooxml", "image_metadata"}) +STATE_BEARING_REVIEW_STATES = frozenset({"APPROVED", "CHANGES_REQUESTED", "DISMISSED"}) +PARSER_MODULES = ( + "guide_pdf.py", + "guide_ooxml.py", + "guide_docx.py", + "guide_pptx.py", + "guide_xlsx.py", + "guide_images.py", +) +PARSER_MODULE_SCOPES = { + "guide_pdf.py": frozenset({"pdf"}), + "guide_ooxml.py": frozenset({"ooxml"}), + "guide_docx.py": frozenset({"ooxml"}), + "guide_pptx.py": frozenset({"ooxml"}), + "guide_xlsx.py": frozenset({"ooxml"}), + "guide_images.py": frozenset({"image_metadata"}), +} +SHA256_PATTERN = re.compile(r"[0-9a-f]{64}") +VERSION_PATTERN = re.compile(r"[0-9]+(?:\.[0-9]+)+(?:[a-z0-9.-]*)?", re.IGNORECASE) +PACKAGE_NAME_PATTERN = re.compile(r"[-_.]+") + + +class DependencyGateError(RuntimeError): + """Raised when dependency approval evidence fails closed.""" + + +def require(condition: bool, code: str) -> None: + """Raise one stable failure code when ``condition`` is false.""" + if not condition: + raise DependencyGateError(code) + + +def _object(value: Any, code: str) -> dict[str, Any]: + require(isinstance(value, dict), code) + return value + + +def _string(value: Any, code: str) -> str: + require(isinstance(value, str) and bool(value.strip()), code) + return value + + +def _string_list(value: Any, code: str) -> list[str]: + require(isinstance(value, list) and bool(value), code) + require(all(isinstance(item, str) and item.strip() for item in value), code) + require(len(value) == len(set(value)), code) + return value + + +def normalize_package_name(value: str) -> str: + """Return the Python packaging canonical project name.""" + return PACKAGE_NAME_PATTERN.sub("-", value).lower() + + +def load_manifest(path: Path = ALLOWLIST_PATH) -> tuple[dict[str, Any], bytes]: + """Load JSON while preserving the exact bytes used for the digest.""" + try: + raw = path.read_bytes() + data = json.loads(raw) + except (OSError, json.JSONDecodeError) as exc: + raise DependencyGateError("guide_dependency_allowlist_unreadable") from exc + require(isinstance(data, dict), "guide_dependency_allowlist_not_object") + canonical = (json.dumps(data, indent=2, sort_keys=True) + "\n").encode() + require(raw == canonical, "guide_dependency_allowlist_not_canonical") + return data, raw + + +def validate_manifest(data: dict[str, Any]) -> dict[str, dict[str, Any]]: + """Validate the closed v0.1 dependency allowlist schema.""" + require( + set(data) + == { + "advisory_snapshot", + "dependencies", + "installation_policy", + "prohibited_packages", + "schema_version", + }, + "guide_dependency_allowlist_top_level_shape_invalid", + ) + require(data["schema_version"] == 1, "guide_dependency_allowlist_schema_unsupported") + + advisory = _object(data["advisory_snapshot"], "guide_dependency_advisory_invalid") + require( + set(advisory) == {"checked_at", "provider", "query_url"}, + "guide_dependency_advisory_shape_invalid", + ) + require( + re.fullmatch( + r"\d{4}-\d{2}-\d{2}", + _string(advisory["checked_at"], "guide_dependency_advisory_date_invalid"), + ) + is not None, + "guide_dependency_advisory_date_invalid", + ) + require(advisory["provider"] == "OSV", "guide_dependency_advisory_provider_invalid") + require( + advisory["query_url"] == "https://api.osv.dev/v1/query", + "guide_dependency_advisory_url_invalid", + ) + + policy = _object(data["installation_policy"], "guide_dependency_policy_invalid") + require( + set(policy) + == { + "allow_source_distributions", + "approved_python_versions", + "approved_runtime_platform", + "index", + "require_hashes", + }, + "guide_dependency_policy_shape_invalid", + ) + require(policy["allow_source_distributions"] is False, "guide_dependency_sdist_allowed") + require(policy["require_hashes"] is True, "guide_dependency_hashes_not_required") + require(policy["index"] == "https://pypi.org/simple", "guide_dependency_index_invalid") + require( + policy["approved_python_versions"] == ["3.11", "3.12"], + "guide_dependency_python_versions_invalid", + ) + require( + policy["approved_runtime_platform"] == "manylinux x86_64", + "guide_dependency_platform_invalid", + ) + prohibited_packages = _string_list( + data["prohibited_packages"], "guide_dependency_prohibited_packages_invalid" + ) + prohibited_canonical = {normalize_package_name(item) for item in prohibited_packages} + require( + len(prohibited_canonical) == len(prohibited_packages), + "guide_dependency_prohibited_packages_duplicate", + ) + + dependencies = data["dependencies"] + require(isinstance(dependencies, list) and dependencies, "guide_dependency_entries_missing") + by_name: dict[str, dict[str, Any]] = {} + observed_scopes: set[str] = set() + observed_imports: set[str] = set() + for entry_value in dependencies: + entry = _object(entry_value, "guide_dependency_entry_invalid") + require( + set(entry) + == { + "advisories", + "approved_artifacts", + "cancellation_timeout_implications", + "direct", + "format_scopes", + "import_names", + "license", + "maintenance", + "malformed_input_history", + "name", + "native_code", + "network_behavior", + "source_index", + "transitive_dependencies", + "version", + }, + "guide_dependency_entry_shape_invalid", + ) + name = _string(entry["name"], "guide_dependency_name_invalid") + canonical_name = normalize_package_name(name) + require(canonical_name not in by_name, "guide_dependency_name_duplicate") + version = _string(entry["version"], "guide_dependency_version_invalid") + require(VERSION_PATTERN.fullmatch(version) is not None, "guide_dependency_version_unpinned") + require(entry["direct"] is True, "guide_dependency_must_be_direct") + require(isinstance(entry["native_code"], bool), "guide_dependency_native_flag_invalid") + require(entry["source_index"] == policy["index"], "guide_dependency_source_index_invalid") + _string(entry["license"], "guide_dependency_license_missing") + _string(entry["malformed_input_history"], "guide_dependency_history_missing") + _string(entry["network_behavior"], "guide_dependency_network_behavior_missing") + _string( + entry["cancellation_timeout_implications"], + "guide_dependency_cancellation_implications_missing", + ) + + scopes = _string_list(entry["format_scopes"], "guide_dependency_scope_invalid") + require(set(scopes) <= APPROVED_SCOPES, "guide_dependency_scope_unknown") + require(not observed_scopes.intersection(scopes), "guide_dependency_scope_ambiguous") + observed_scopes.update(scopes) + + imports = _string_list(entry["import_names"], "guide_dependency_imports_invalid") + require(not observed_imports.intersection(imports), "guide_dependency_import_ambiguous") + observed_imports.update(imports) + + transitive = entry["transitive_dependencies"] + require(isinstance(transitive, list), "guide_dependency_transitive_graph_invalid") + require( + all(isinstance(item, str) and item.strip() for item in transitive), + "guide_dependency_transitive_graph_invalid", + ) + require(len(transitive) == len(set(transitive)), "guide_dependency_transitive_duplicate") + + advisories = entry["advisories"] + require(isinstance(advisories, list), "guide_dependency_advisories_invalid") + require( + all(isinstance(item, str) and item.strip() for item in advisories), + "guide_dependency_advisories_invalid", + ) + + maintenance = _object(entry["maintenance"], "guide_dependency_maintenance_invalid") + require( + set(maintenance) == {"assessment", "release_uploaded_at", "source_url"}, + "guide_dependency_maintenance_shape_invalid", + ) + _string(maintenance["assessment"], "guide_dependency_maintenance_assessment_missing") + release_uploaded_at = _string( + maintenance["release_uploaded_at"], "guide_dependency_release_timestamp_invalid" + ) + require( + release_uploaded_at.endswith("Z"), + "guide_dependency_release_timestamp_invalid", + ) + require( + _string(maintenance["source_url"], "guide_dependency_source_url_missing").startswith( + "https://" + ), + "guide_dependency_source_url_invalid", + ) + + artifacts = entry["approved_artifacts"] + require(isinstance(artifacts, list) and artifacts, "guide_dependency_artifacts_missing") + filenames: set[str] = set() + for artifact_value in artifacts: + artifact = _object(artifact_value, "guide_dependency_artifact_invalid") + expected_artifact_keys = {"filename", "sha256", "url"} + if entry["native_code"]: + expected_artifact_keys.add("python_version") + require( + set(artifact) == expected_artifact_keys, "guide_dependency_artifact_shape_invalid" + ) + filename = _string(artifact["filename"], "guide_dependency_artifact_name_invalid") + require(filename.endswith(".whl"), "guide_dependency_artifact_not_wheel") + require(filename not in filenames, "guide_dependency_artifact_duplicate") + filenames.add(filename) + artifact_sha256 = _string(artifact["sha256"], "guide_dependency_artifact_hash_invalid") + require( + SHA256_PATTERN.fullmatch(artifact_sha256) is not None, + "guide_dependency_artifact_hash_invalid", + ) + wheel_prefix = f"{canonical_name.replace('-', '_')}-{version}-" + require( + filename.lower().startswith(wheel_prefix), + "guide_dependency_artifact_identity_mismatch", + ) + url = _string(artifact["url"], "guide_dependency_artifact_url_invalid") + require( + url.startswith("https://files.pythonhosted.org/packages/") + and url.endswith(f"/{filename}"), + "guide_dependency_artifact_url_invalid", + ) + if entry["native_code"]: + python_version = _string( + artifact["python_version"], "guide_dependency_artifact_python_invalid" + ) + compact_python = python_version.replace(".", "") + require( + python_version in policy["approved_python_versions"] + and f"-cp{compact_python}-cp{compact_python}-" in filename + and "manylinux" in filename + and filename.endswith("x86_64.whl"), + "guide_dependency_artifact_platform_mismatch", + ) + else: + require( + filename.endswith("-none-any.whl"), + "guide_dependency_artifact_platform_mismatch", + ) + by_name[canonical_name] = entry + + require(observed_scopes == APPROVED_SCOPES, "guide_dependency_scope_incomplete") + require( + not prohibited_canonical.intersection(by_name), + "guide_dependency_prohibited_package_approved", + ) + return by_name + + +def _dependency_name(requirement: str) -> str: + return normalize_package_name(re.split(r"[<>=!~\s;\[]", requirement.strip(), 1)[0]) + + +def _approved_requirements(entry: dict[str, Any]) -> set[str]: + """Build the only hash-bound direct references approved for one package.""" + requirements: set[str] = set() + for artifact in entry["approved_artifacts"]: + requirement = f"{entry['name']} @ {artifact['url']}#sha256={artifact['sha256']}" + if python_version := artifact.get("python_version"): + requirement += f'; python_version == "{python_version}"' + requirements.add(requirement) + return requirements + + +def validate_declared_dependencies( + allowlist: dict[str, dict[str, Any]], prohibited_packages: list[str] +) -> None: + """Require runtime parser declarations to bind exact approved wheel bytes.""" + with PYPROJECT_PATH.open("rb") as handle: + pyproject = tomllib.load(handle) + project = pyproject["project"] + runtime_requirements = project.get("dependencies", []) + optional_requirements = [ + item for group in project.get("optional-dependencies", {}).values() for item in group + ] + dependency_group_requirements = [ + item for group in pyproject.get("dependency-groups", {}).values() for item in group + ] + prohibited = {normalize_package_name(item) for item in prohibited_packages} + for requirement in [ + *runtime_requirements, + *optional_requirements, + *dependency_group_requirements, + ]: + require( + _dependency_name(requirement) not in prohibited, + "guide_dependency_prohibited_package_declared", + ) + for requirement in [*optional_requirements, *dependency_group_requirements]: + require( + _dependency_name(requirement) not in allowlist, + "guide_dependency_parser_non_runtime_declaration", + ) + requirements_by_name: dict[str, set[str]] = {} + for requirement in runtime_requirements: + requirements_by_name.setdefault(_dependency_name(requirement), set()).add(requirement) + for name, entry in allowlist.items(): + requirements = requirements_by_name.get(name) + if requirements is None: + continue + require( + requirements == _approved_requirements(entry), + "guide_dependency_declared_artifact_mismatch", + ) + + +def validate_parser_imports(allowlist: dict[str, dict[str, Any]]) -> None: + """Reject undeclared third-party imports in format-specific parser modules.""" + parser_root = BACKEND_ROOT / "app" / "modules" / "artifacts" + for module_name in PARSER_MODULES: + module_path = parser_root / module_name + if not module_path.exists(): + continue + approved_imports = { + import_name.split(".", 1)[0] + for entry in allowlist.values() + if set(entry["format_scopes"]) & PARSER_MODULE_SCOPES[module_name] + for import_name in entry["import_names"] + } + tree = ast.parse(module_path.read_text(), filename=str(module_path)) + for node in ast.walk(tree): + imported: str | None = None + if isinstance(node, ast.Import): + for alias in node.names: + root = alias.name.split(".", 1)[0] + require( + root in sys.stdlib_module_names + or root == "app" + or root in approved_imports, + "guide_dependency_parser_import_undeclared", + ) + elif isinstance(node, ast.ImportFrom) and node.level == 0 and node.module: + imported = node.module.split(".", 1)[0] + if imported is not None: + require( + imported in sys.stdlib_module_names + or imported == "app" + or imported in approved_imports, + "guide_dependency_parser_import_undeclared", + ) + + +def allowlist_changed(base_sha: str, head_sha: str) -> bool: + """Return whether the exact allowlist bytes differ across the PR range.""" + require(bool(re.fullmatch(r"[0-9a-f]{40}", base_sha)), "guide_dependency_base_sha_invalid") + require(bool(re.fullmatch(r"[0-9a-f]{40}", head_sha)), "guide_dependency_head_sha_invalid") + result = subprocess.run( + ["git", "diff", "--quiet", f"{base_sha}...{head_sha}", "--", ALLOWLIST_REPOSITORY_PATH], + cwd=REPOSITORY_ROOT, + check=False, + ) + require(result.returncode in {0, 1}, "guide_dependency_git_diff_failed") + return result.returncode == 1 + + +def validate_reviews( + reviews: list[dict[str, Any]], *, author: str, head_sha: str +) -> dict[str, Any]: + """Return one current independent approval or fail closed.""" + require(bool(author), "guide_dependency_pr_author_missing") + require(bool(re.fullmatch(r"[0-9a-f]{40}", head_sha)), "guide_dependency_head_sha_invalid") + latest_by_reviewer: dict[str, dict[str, Any]] = {} + for review in reviews: + require(isinstance(review, dict), "guide_dependency_review_shape_invalid") + user = review.get("user") + require(isinstance(user, dict), "guide_dependency_review_user_missing") + login = user.get("login") + require(isinstance(login, str) and login, "guide_dependency_review_login_missing") + if review.get("state") not in STATE_BEARING_REVIEW_STATES: + continue + latest_by_reviewer[login.casefold()] = review + + candidates: list[dict[str, Any]] = [] + for login, review in latest_by_reviewer.items(): + user = review["user"] + if login == author.casefold(): + continue + if user.get("type") != "User" or login.endswith("[bot]"): + continue + if review.get("author_association") not in APPROVED_ASSOCIATIONS: + continue + if review.get("state") != "APPROVED": + continue + if review.get("commit_id") != head_sha: + continue + candidates.append(review) + require(bool(candidates), "guide_dependency_independent_head_approval_missing") + return candidates[-1] + + +def fetch_reviews(repository: str, pull_number: int, token: str) -> list[dict[str, Any]]: + """Read every PR review using the minimal read-only GitHub token.""" + require(bool(re.fullmatch(r"[^/]+/[^/]+", repository)), "guide_dependency_repository_invalid") + require(pull_number > 0, "guide_dependency_pull_number_invalid") + require(bool(token), "guide_dependency_github_token_missing") + reviews: list[dict[str, Any]] = [] + page = 1 + while True: + request = urllib.request.Request( + f"https://api.github.com/repos/{repository}/pulls/{pull_number}/reviews?per_page=100&page={page}", + headers={ + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {token}", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "workstream-guide-dependency-gate", + }, + ) + try: + with urllib.request.urlopen(request, timeout=20) as response: + payload = json.load(response) + except (OSError, urllib.error.HTTPError, json.JSONDecodeError) as exc: + raise DependencyGateError("guide_dependency_github_reviews_unavailable") from exc + require(isinstance(payload, list), "guide_dependency_github_reviews_invalid") + reviews.extend(payload) + if len(payload) < 100: + return reviews + page += 1 + + +def validate_pr_approval(raw_allowlist: bytes) -> None: + """Validate live review authority when this PR changes the allowlist.""" + base_sha = os.environ.get("WORKSTREAM_PR_BASE_SHA", "") + head_sha = os.environ.get("WORKSTREAM_PR_HEAD_SHA", "") + if not allowlist_changed(base_sha, head_sha): + return + repository = os.environ.get("GITHUB_REPOSITORY", "") + author = os.environ.get("WORKSTREAM_PR_AUTHOR", "") + pull_number_value = os.environ.get("WORKSTREAM_PR_NUMBER", "") + require(pull_number_value.isdigit(), "guide_dependency_pull_number_invalid") + reviews = fetch_reviews( + repository, + int(pull_number_value), + os.environ.get("GITHUB_TOKEN", ""), + ) + approval = validate_reviews(reviews, author=author, head_sha=head_sha) + digest = hashlib.sha256(raw_allowlist).hexdigest() + print( + "Guide dependency approval: " + f"review={approval.get('id')} head={head_sha} allowlist_sha256={digest}" + ) + + +def parse_args() -> argparse.Namespace: + """Parse the small CI-facing command surface.""" + parser = argparse.ArgumentParser() + parser.add_argument( + "--require-pr-approval", + action="store_true", + help="Require live exact-head review authority when the PR changes the allowlist.", + ) + return parser.parse_args() + + +def main() -> int: + """Run static validation and the optional live PR approval gate.""" + args = parse_args() + try: + data, raw = load_manifest() + allowlist = validate_manifest(data) + validate_declared_dependencies(allowlist, data["prohibited_packages"]) + validate_parser_imports(allowlist) + if args.require_pr_approval: + validate_pr_approval(raw) + except DependencyGateError as exc: + print(str(exc), file=sys.stderr) + return 1 + print(f"Guide extractor dependency gate passed ({hashlib.sha256(raw).hexdigest()}).") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/backend/scripts/run_test_lanes.py b/backend/scripts/run_test_lanes.py index abc23acd..4678db93 100644 --- a/backend/scripts/run_test_lanes.py +++ b/backend/scripts/run_test_lanes.py @@ -106,6 +106,7 @@ class TestLane: "tests/test_guide_bindings.py", "tests/test_guide_formats.py", "tests/test_guide_extraction.py", + "tests/test_guide_extractor_dependencies.py", "tests/test_local_artifact_store.py", "tests/test_s3_artifact_store.py", "tests/test_test_lane_evidence.py", diff --git a/backend/tests/test_guide_extractor_dependencies.py b/backend/tests/test_guide_extractor_dependencies.py new file mode 100644 index 00000000..57e31820 --- /dev/null +++ b/backend/tests/test_guide_extractor_dependencies.py @@ -0,0 +1,379 @@ +"""Tests for the guide-extractor dependency approval gate.""" + +from __future__ import annotations + +import copy +import json +import subprocess +from pathlib import Path +from urllib.error import HTTPError + +import pytest + +from scripts import check_guide_extractor_dependencies as gate + +HEAD = "a" * 40 +OTHER_HEAD = "b" * 40 + + +def _manifest() -> dict[str, object]: + data, _ = gate.load_manifest() + return data + + +def _review( + login: str = "maintainer", + *, + state: str = "APPROVED", + commit_id: str = HEAD, + association: str = "MEMBER", + user_type: str = "User", + review_id: int = 1, +) -> dict[str, object]: + return { + "author_association": association, + "commit_id": commit_id, + "id": review_id, + "state": state, + "user": {"login": login, "type": user_type}, + } + + +def test_repository_allowlist_is_canonical_and_valid() -> None: + data, raw = gate.load_manifest() + + allowlist = gate.validate_manifest(data) + + assert set(allowlist) == {"pypdf", "defusedxml", "pillow"} + assert raw == (json.dumps(data, indent=2, sort_keys=True) + "\n").encode() + assert { + scope for entry in allowlist.values() for scope in entry["format_scopes"] + } == gate.APPROVED_SCOPES + + +def test_unreadable_allowlist_fails_closed(tmp_path: Path) -> None: + invalid = tmp_path / "allowlist.json" + invalid.write_text("not-json") + + with pytest.raises(gate.DependencyGateError, match="guide_dependency_allowlist_unreadable"): + gate.load_manifest(invalid) + + +@pytest.mark.parametrize( + ("mutation", "failure"), + [ + ( + lambda data: data["dependencies"][0].update(version=">=6"), + "guide_dependency_version_unpinned", + ), + ( + lambda data: data["dependencies"][0]["approved_artifacts"][0].update(sha256="0" * 63), + "guide_dependency_artifact_hash_invalid", + ), + ( + lambda data: data["dependencies"][0]["approved_artifacts"][0].update(sha256=None), + "guide_dependency_artifact_hash_invalid", + ), + ( + lambda data: data["dependencies"][0]["maintenance"].update(release_uploaded_at=None), + "guide_dependency_release_timestamp_invalid", + ), + ( + lambda data: data["dependencies"][0]["approved_artifacts"][0].update( + filename="other-6.14.2-py3-none-any.whl" + ), + "guide_dependency_artifact_identity_mismatch", + ), + ( + lambda data: data["dependencies"][0]["approved_artifacts"][0].update( + filename="pypdf-6.13.0-py3-none-any.whl" + ), + "guide_dependency_artifact_identity_mismatch", + ), + ( + lambda data: data["dependencies"][2]["approved_artifacts"][0].update( + python_version="3.13" + ), + "guide_dependency_artifact_platform_mismatch", + ), + ( + lambda data: data["dependencies"][0]["approved_artifacts"][0].update( + url="https://example.invalid/pypdf.whl" + ), + "guide_dependency_artifact_url_invalid", + ), + ( + lambda data: data["dependencies"][0].update(format_scopes=["submission_zip"]), + "guide_dependency_scope_unknown", + ), + ( + lambda data: data["dependencies"][0].update(source_index="https://example.invalid"), + "guide_dependency_source_index_invalid", + ), + ( + lambda data: data["installation_policy"].update(allow_source_distributions=True), + "guide_dependency_sdist_allowed", + ), + ], +) +def test_manifest_rejects_unpinned_hash_drifted_or_wrong_scope_entries( + mutation: object, failure: str +) -> None: + data = copy.deepcopy(_manifest()) + mutation(data) # type: ignore[operator] + + with pytest.raises(gate.DependencyGateError, match=failure): + gate.validate_manifest(data) + + +def test_declared_parser_dependency_must_use_exact_approved_pin( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + pyproject = tmp_path / "pyproject.toml" + pyproject.write_text('[project]\ndependencies = ["pypdf>=6"]\n') + monkeypatch.setattr(gate, "PYPROJECT_PATH", pyproject) + data = _manifest() + allowlist = gate.validate_manifest(data) + + with pytest.raises( + gate.DependencyGateError, match="guide_dependency_declared_artifact_mismatch" + ): + gate.validate_declared_dependencies(allowlist, data["prohibited_packages"]) + + +def test_exact_declared_parser_pin_passes(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + data = _manifest() + allowlist = gate.validate_manifest(data) + approved_requirement = next(iter(gate._approved_requirements(allowlist["pypdf"]))) + pyproject = tmp_path / "pyproject.toml" + pyproject.write_text(f"[project]\ndependencies = [{approved_requirement!r}]\n") + monkeypatch.setattr(gate, "PYPROJECT_PATH", pyproject) + + gate.validate_declared_dependencies(allowlist, data["prohibited_packages"]) + + +@pytest.mark.parametrize("table", ["project.optional-dependencies", "dependency-groups"]) +def test_optional_or_dependency_group_parser_declaration_is_forbidden( + table: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + pyproject = tmp_path / "pyproject.toml" + pyproject.write_text(f'[project]\ndependencies = []\n[{table}]\ndev = ["pypdf>=6"]\n') + monkeypatch.setattr(gate, "PYPROJECT_PATH", pyproject) + data = _manifest() + + with pytest.raises( + gate.DependencyGateError, match="guide_dependency_parser_non_runtime_declaration" + ): + gate.validate_declared_dependencies( + gate.validate_manifest(data), data["prohibited_packages"] + ) + + +def test_explicitly_prohibited_parser_package_is_rejected( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + pyproject = tmp_path / "pyproject.toml" + pyproject.write_text('[project]\ndependencies = ["python-docx==1.2.0"]\n') + monkeypatch.setattr(gate, "PYPROJECT_PATH", pyproject) + data = _manifest() + + with pytest.raises( + gate.DependencyGateError, match="guide_dependency_prohibited_package_declared" + ): + gate.validate_declared_dependencies( + gate.validate_manifest(data), data["prohibited_packages"] + ) + + +def test_parser_module_rejects_undeclared_third_party_import( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + parser_root = tmp_path / "app" / "modules" / "artifacts" + parser_root.mkdir(parents=True) + (parser_root / "guide_pdf.py").write_text("import arbitrary_pdf_plugin\n") + monkeypatch.setattr(gate, "BACKEND_ROOT", tmp_path) + allowlist = gate.validate_manifest(_manifest()) + + with pytest.raises(gate.DependencyGateError, match="guide_dependency_parser_import_undeclared"): + gate.validate_parser_imports(allowlist) + + +def test_parser_module_allows_stdlib_project_and_approved_imports( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + parser_root = tmp_path / "app" / "modules" / "artifacts" + parser_root.mkdir(parents=True) + (parser_root / "guide_pdf.py").write_text( + "import json\nfrom app.core import config\nfrom pypdf import PdfReader\n" + ) + monkeypatch.setattr(gate, "BACKEND_ROOT", tmp_path) + + gate.validate_parser_imports(gate.validate_manifest(_manifest())) + + +@pytest.mark.parametrize( + ("module_name", "source"), + [ + ("guide_images.py", "import pypdf\n"), + ("guide_pdf.py", "from PIL import Image\n"), + ], +) +def test_parser_module_rejects_approved_dependency_from_wrong_format_scope( + module_name: str, source: str, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + parser_root = tmp_path / "app" / "modules" / "artifacts" + parser_root.mkdir(parents=True) + (parser_root / module_name).write_text(source) + monkeypatch.setattr(gate, "BACKEND_ROOT", tmp_path) + + with pytest.raises(gate.DependencyGateError, match="guide_dependency_parser_import_undeclared"): + gate.validate_parser_imports(gate.validate_manifest(_manifest())) + + +def test_exact_head_independent_member_approval_passes() -> None: + approval = gate.validate_reviews([_review()], author="contributor", head_sha=HEAD) + + assert approval["id"] == 1 + + +@pytest.mark.parametrize( + "reviews", + [ + [], + [_review("contributor")], + [_review("coderabbitai[bot]", user_type="Bot")], + [_review(commit_id=OTHER_HEAD)], + [_review(association="NONE")], + [_review(state="DISMISSED")], + ], +) +def test_missing_self_bot_stale_untrusted_or_dismissed_approval_fails( + reviews: list[dict[str, object]], +) -> None: + with pytest.raises( + gate.DependencyGateError, + match="guide_dependency_independent_head_approval_missing", + ): + gate.validate_reviews(reviews, author="contributor", head_sha=HEAD) + + +def test_latest_review_by_same_reviewer_must_remain_approved() -> None: + reviews = [ + _review(review_id=1), + _review(state="CHANGES_REQUESTED", review_id=2), + ] + + with pytest.raises( + gate.DependencyGateError, + match="guide_dependency_independent_head_approval_missing", + ): + gate.validate_reviews(reviews, author="contributor", head_sha=HEAD) + + +def test_later_commented_review_does_not_erase_current_approval() -> None: + reviews = [ + _review(review_id=1), + _review(state="COMMENTED", review_id=2), + ] + + approval = gate.validate_reviews(reviews, author="contributor", head_sha=HEAD) + + assert approval["id"] == 1 + + +def test_changed_allowlist_requires_live_approval( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(gate, "allowlist_changed", lambda _base, _head: True) + monkeypatch.setattr(gate, "fetch_reviews", lambda *_args: []) + monkeypatch.setenv("WORKSTREAM_PR_BASE_SHA", OTHER_HEAD) + monkeypatch.setenv("WORKSTREAM_PR_HEAD_SHA", HEAD) + monkeypatch.setenv("WORKSTREAM_PR_AUTHOR", "contributor") + monkeypatch.setenv("WORKSTREAM_PR_NUMBER", "42") + monkeypatch.setenv("GITHUB_REPOSITORY", "Flow-Research/workstream") + monkeypatch.setenv("GITHUB_TOKEN", "read-only-test-token") + + with pytest.raises( + gate.DependencyGateError, + match="guide_dependency_independent_head_approval_missing", + ): + gate.validate_pr_approval(b"changed allowlist") + + +def test_unchanged_allowlist_needs_no_live_pr_metadata( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(gate, "allowlist_changed", lambda _base, _head: False) + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + + gate.validate_pr_approval(b"unchanged allowlist") + + +def test_github_api_failure_fails_closed(monkeypatch: pytest.MonkeyPatch) -> None: + def unavailable(*_args: object, **_kwargs: object) -> object: + raise HTTPError("https://api.github.test", 503, "unavailable", {}, None) + + monkeypatch.setattr(gate.urllib.request, "urlopen", unavailable) + + with pytest.raises( + gate.DependencyGateError, + match="guide_dependency_github_reviews_unavailable", + ): + gate.fetch_reviews("Flow-Research/workstream", 42, "read-only-test-token") + + +def test_github_review_fetch_returns_valid_payload(monkeypatch: pytest.MonkeyPatch) -> None: + class Response: + def __enter__(self) -> "Response": + return self + + def __exit__(self, *_args: object) -> None: + return None + + def read(self) -> bytes: + return json.dumps([_review()]).encode() + + monkeypatch.setattr(gate.urllib.request, "urlopen", lambda *_args, **_kwargs: Response()) + + reviews = gate.fetch_reviews("Flow-Research/workstream", 42, "read-only-test-token") + + assert reviews == [_review()] + + +def test_changed_allowlist_with_current_approval_passes( + monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + monkeypatch.setattr(gate, "allowlist_changed", lambda _base, _head: True) + monkeypatch.setattr(gate, "fetch_reviews", lambda *_args: [_review()]) + monkeypatch.setenv("WORKSTREAM_PR_BASE_SHA", OTHER_HEAD) + monkeypatch.setenv("WORKSTREAM_PR_HEAD_SHA", HEAD) + monkeypatch.setenv("WORKSTREAM_PR_AUTHOR", "contributor") + monkeypatch.setenv("WORKSTREAM_PR_NUMBER", "42") + monkeypatch.setenv("GITHUB_REPOSITORY", "Flow-Research/workstream") + monkeypatch.setenv("GITHUB_TOKEN", "read-only-test-token") + + gate.validate_pr_approval(b"approved allowlist") + + assert "allowlist_sha256=" in capsys.readouterr().out + + +def test_allowlist_changed_uses_exact_pr_range(monkeypatch: pytest.MonkeyPatch) -> None: + observed: list[list[str]] = [] + + def run(command: list[str], **_kwargs: object) -> subprocess.CompletedProcess[str]: + observed.append(command) + return subprocess.CompletedProcess(command, 1) + + monkeypatch.setattr(gate.subprocess, "run", run) + + assert gate.allowlist_changed(OTHER_HEAD, HEAD) is True + assert observed == [ + [ + "git", + "diff", + "--quiet", + f"{OTHER_HEAD}...{HEAD}", + "--", + "backend/config/guide_extractor_dependencies.json", + ] + ] diff --git a/docs/spec_artifact_storage_service.md b/docs/spec_artifact_storage_service.md index 8000f7c6..bfa75e9e 100644 --- a/docs/spec_artifact_storage_service.md +++ b/docs/spec_artifact_storage_service.md @@ -1252,6 +1252,28 @@ generation extraction reaches the agent, delimited as untrusted source data with no tool, secret, provider, or instruction authority. Unsupported, ambiguous, malformed, stale, or failed required material stops setup internally. +Complex-format parser dependencies are a closed, CI-owned supply-chain +boundary. v0.1 approves only `pypdf==6.14.2` for PDF, `defusedxml==0.7.1` for +the shared OOXML safety boundary, and `Pillow==12.3.0` for PNG/JPEG/WebP +structural metadata. The OOXML adapters do not add `python-docx`, +`python-pptx`, `openpyxl`, `lxml`, or `XlsxWriter`. The canonical allowlist +records exact wheel URLs and hashes, licenses, imports, format scopes, native-code and +runtime-isolation facts, maintenance evidence, and an advisory snapshot. It +approves no source distribution and only the named Python 3.11/3.12 manylinux +x86_64 Pillow wheels. + +Changing that allowlist requires an independent protected GitHub approval on +the exact PR head after the final byte change. The reviewer must be a human +repository owner, member, or collaborator other than the PR author; bot, +self-authored, dismissed, stale-head, or unavailable review evidence fails +closed. Repository evidence may mirror the decision for audit but cannot grant +authority. On `main`, the protected merged history is the approval baseline; +later parser chunks must use hash-bound direct wheel references and +format-scoped approved imports without changing the baseline silently. +`python-docx`, `python-pptx`, `openpyxl`, `lxml`, and `XlsxWriter` are an +explicitly prohibited parser set across runtime, optional, and dependency-group +declarations. + For 03B3A the isolation contract is descriptor-only parsing after trusted imports, enforced by a default-deny Linux libseccomp profile with an explicit syscall allowlist plus fixed resource limits: 32 MiB input, 4 MiB