From 1b27ddec3ca54bff8ff77ad33228e36b6eaa6501 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Wed, 29 Jul 2026 14:10:08 +0100 Subject: [PATCH 1/3] feat(artifacts): classify verified guide materialization --- .../REVIEW_LOG.md | 14 + ...B2-guide-materialization-classification.md | 7 +- ...S-ART-001-03B2-internal-review-evidence.md | 59 ++ .../WS-ART-001-03B2-pr-trust-bundle.md | 92 +++ ...40_guide_materialization_classification.py | 131 +++ backend/app/db/models.py | 2 + backend/app/interfaces/artifact_operations.py | 39 +- .../app/modules/artifacts/guide_formats.py | 352 +++++++++ .../artifacts/guide_materialization.py | 457 +++++++++++ backend/app/modules/artifacts/models.py | 94 +++ backend/app/modules/artifacts/preparation.py | 29 +- backend/app/modules/artifacts/schemas.py | 23 + backend/app/modules/artifacts/service.py | 40 +- backend/app/modules/artifacts/sources.py | 26 +- backend/scripts/run_test_lanes.py | 1 + backend/tests/conftest.py | 4 +- backend/tests/test_alembic.py | 16 +- backend/tests/test_artifact_architecture.py | 48 +- backend/tests/test_artifact_preparation.py | 52 ++ backend/tests/test_guide_bindings.py | 744 +++++++++++++++++- backend/tests/test_guide_formats.py | 349 ++++++++ docs/spec_artifact_storage_service.md | 20 + 22 files changed, 2546 insertions(+), 53 deletions(-) create mode 100644 .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-internal-review-evidence.md create mode 100644 .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-pr-trust-bundle.md create mode 100644 backend/alembic/versions/0040_guide_materialization_classification.py create mode 100644 backend/app/modules/artifacts/guide_formats.py create mode 100644 backend/app/modules/artifacts/guide_materialization.py create mode 100644 backend/tests/test_guide_formats.py 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 65ef5354f..856776819 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 @@ -183,3 +183,17 @@ and memory termination are now non-retryable limit failures; executor loss receives one bounded fresh-authority/materialization retry, then a stable extraction-failed outcome with only redacted diagnostics. + +## WS-ART-001-03B2 + +- Implemented the hidden fixed-reader materialization and syntactic + classification slice after merged 03B1. +- Initial internal review found active namespace drift, a broad scratch + callback, unsafe external-relationship matching, and missing incident, + boundary, cancellation, image, and downgrade tests. +- Repairs share the canonical replica namespace/store validator, use the + canonical materializer facade and typed scratch inspector, parse bounded + relationship XML fail closed, and add the complete focused proof set. +- Final architecture, security, QA, product/ops, CI-integrity, docs, + reuse/dedup, test-delta, and senior-engineering reviews pass. Exact hosted + PR-head checks remain the publication gate. diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B2-guide-materialization-classification.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B2-guide-materialization-classification.md index 38bcda66b..5a201983a 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B2-guide-materialization-classification.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B2-guide-materialization-classification.md @@ -47,6 +47,9 @@ scratch without extracting semantic content. either image dimension; every exact-boundary case succeeds, every one-over case returns `limit_exceeded`, and no partial classification survives; - cleanup occurs on success, denial, mismatch, cancellation, and timeout; +- migration `0040_guide_materialization_classification` preserves exact-binding classification + and incident custody, refuses populated downgrade, and introduces no new + Operator or generic artifact-read route; - changed subsystems remain at least 90% covered and repository coverage stays at least 78%. @@ -54,12 +57,12 @@ scratch without extracting semantic content. ```bash (cd backend && .venv/bin/python -m ruff check app tests scripts) -(cd backend && WORKSTREAM_TEST_DATABASE_URL=postgresql+asyncpg://workstream:workstream@localhost:5433/workstream_test .venv/bin/pytest tests/test_guide_artifacts.py tests/test_artifact_scratch_manager.py tests/test_guide_format_detection.py -q --cov=app --cov-report=term-missing --cov-fail-under=0) +(cd backend && WORKSTREAM_TEST_DATABASE_URL=postgresql+asyncpg://workstream:workstream@localhost:5433/workstream_test .venv/bin/pytest tests/test_guide_bindings.py tests/test_artifact_preparation.py tests/test_guide_formats.py -q --cov=app --cov-report=term-missing --cov-fail-under=0) (cd backend && .venv/bin/coverage report --precision=2 --fail-under=78) (cd backend && .venv/bin/coverage report --include='app/modules/artifacts/*' --precision=2 --fail-under=90) python3 scripts/check_stale_artifact_contracts.py python3 scripts/check_markdown_links.py -python3 scripts/test_agent_gates.py +PYTHONPATH=. python3 scripts/test_lightweight_agent_gates.py git diff --check ``` diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-internal-review-evidence.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-internal-review-evidence.md new file mode 100644 index 000000000..7dedb9410 --- /dev/null +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-internal-review-evidence.md @@ -0,0 +1,59 @@ +# Internal Review Evidence: WS-ART-001-03B2 + +## Scope + +Hidden fixed-reader materialization of exact verified guide-source bytes, +bounded syntactic classification, and ART-owned custody incidents. No AUTH +activation, extraction, agent invocation, Celery continuation, generic +download, or legacy cutover is included. + +## Deterministic Evidence + +- Ruff passed for `app`, `tests`, `scripts`, and migration `0040`. +- Focused format, architecture, and scratch inspection tests passed. +- The isolated PostgreSQL runner migrated through `0040` and passed all 13 + materialization-selected guide-binding tests, including denial before I/O, + namespace drift, full rehash, replay, changed/truncated/stale incidents, + cancellation, and timeout cleanup. +- Canonical lane collection accepted `test_guide_formats.py` in + `shared_foundations`; the CI-integrity reviewer collected 2,316 nodes. +- Stale artifact-contract, changed Markdown-link, lightweight agent-gate, and + `git diff --check` checks passed. +- Hosted Backend and Agent Gates remain required on the exact committed PR + head; no local full-suite run was used. + +## Findings And Repairs + +- Architecture: shared the existing replica/namespace/store validator with the + canonical `ArtifactMaterializationService`, and narrowed scratch inspection + to a typed ART-owned inspector. +- Security: replaced substring relationship checks with bounded XML parsing; + malformed XML, DOCTYPE, and whitespace-equivalent external relationships + now fail closed. +- QA/test: added changed and stale-generation incidents, exact cross-resource + denial, cancellation/timeout cleanup, fixed-limit arithmetic, ambiguous and + malformed containers, video signatures, JPEG, WebP variants, and both + classification-only and incident-only populated downgrade refusal. +- Senior engineering/reuse: added exact verification-job replica/terminal + lineage, inspection deadline mapping, model/migration constraint-name parity, + and shared namespace validation. +- Docs/product/ops: documented migration `0040`, evidence custody, populated + downgrade refusal, and the no-new-Operator-route boundary. Artifact incidents + remain separate from guide insufficiency. + +## Final Reviewer Results + +- Architecture: PASS WITH LOW RISKS. +- Security/auth: PASS. +- QA/test: PASS WITH LOW RISKS. +- Product/ops: PASS. +- CI integrity: PASS WITH LOW RISKS. +- Docs: PASS. +- Reuse/dedup: PASS WITH LOW RISKS. +- Test delta: PASS WITH LOW RISKS. +- Senior engineering: final confirmation recorded before PR publication. + +Residual low risks are limited to the canonical materializer facade currently +implementing only the 03B2 guide-read slice, a structurally typed scratch +inspector, and repeated focused-test scratch setup. Later ART chunks must extend +the same facade and custody boundaries. diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-pr-trust-bundle.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-pr-trust-bundle.md new file mode 100644 index 000000000..1bdbc4f49 --- /dev/null +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-pr-trust-bundle.md @@ -0,0 +1,92 @@ +# PR Trust Bundle: WS-ART-001-03B2 + +## Chunk + +`WS-ART-001-03B2` — Guide Materialization And Classification (L1) + +## Goal + +Read the exact verified guide-source object through the fixed guide-reader +boundary, recompute its complete identity in bounded private scratch, persist +only exact syntactic classification or bounded ART incident evidence, and keep +the behavior hidden until AUTH-04B. + +## Human-Approved Intent + +Project Manager guide items may be PDF, DOCX, PPTX, CSV, XLSX, Markdown, plain +text, JSON, or supported images; they are not contributor submission ZIPs. +03B2 classifies structure only. It does not extract semantics or invoke the +guide-sufficiency agent. Audio/video and ordinary/opaque ZIP content remain +unsupported in v0.1. + +## What Changed And Why + +- Extended the canonical materialization port and facade with exact guide reads. +- Added shared active namespace/store/replica validation before provider I/O. +- Added a typed scratch-owned inspection capability without exposing paths. +- Added complete rehash/size verification, pre/post lineage revalidation, + immutable format classifications, and bounded custody incidents. +- Added deterministic PDF, OOXML, text-family, image, audio/video, ordinary ZIP, + unsafe-container, and fixed-limit classification. +- Added migration `0040`, reset/fingerprint/lane updates, focused tests, and + operator/deployment documentation. + +This keeps the object store responsible only for bytes while Workstream owns +their identity, integrity, lineage, and meaning. + +## Design Chosen + +Transaction A locks and recomposes the exact binding/content/replica/receipt/ +setup facts and consumes prepared read authority. Database locks are released +before the full provider read. The provider-neutral store streams into the +canonical scratch manager, which recomputes digest and size. Transaction B +relocks and recomposes the same facts before persisting one immutable result. + +Alternatives rejected: direct S3/MinIO access, arbitrary temp files, caller +excerpts, parsing during HTTP upload, generic download authority, serialized +prepared handles, raw relationship substring checks, and a second materializer. + +## Scope And Product Behavior + +No route is exposed. `artifact.guide_source.read` remains planned/unavailable, +and positive behavior uses bounded test authority only. There is no extraction, +agent call, Celery continuation, submission/checker/review change, legacy +cutover, or new Operator API. Missing/corrupt/stale bytes are ART incidents, +never guide-insufficiency decisions. + +## Acceptance Proof And Test Delta + +Focused pure tests cover signatures, OOXML marker precedence, unsafe ZIPs, +fixed limit boundaries, image variants, typed inspection, deadline behavior, +and architecture fences. The isolated PostgreSQL runner passed all 13 selected +materialization tests after migrating through `0040`. They prove denial before +I/O, namespace drift, exact rehash, replay, changed/truncated/stale incidents, +cross-resource rejection, cancellation, timeout, and cleanup. Migration tests +cover classification-only and incident-only downgrade refusal. + +No test, assertion, coverage threshold, workflow, or shard was weakened. +`test_guide_formats.py` is assigned to `shared_foundations`. Hosted repository +coverage (78 percent), artifact subsystem coverage (90 percent), Backend shards, +and Agent Gates remain required on the exact PR head. + +## Reviewer Results And External Review + +Architecture, security, QA, product/ops, CI integrity, docs, reuse, and test +delta pass after valid findings were repaired. Senior-engineering confirmation +is recorded in the paired evidence before PR publication. CodeRabbit and GitHub +checks are pending until the PR exists and supplement these internal reviews. + +## Remaining Risks And Follow-Up + +The materializer facade contains only the guide-read slice today; 03B3A and +later chunks must extend it rather than create another path. 03B3A adds bounded +extraction; 03B3B adds remaining approved format extractors; 03B4 adds durable +same-generation continuation. Only then may AUTH-04B activate the fixed binding +and guide-reader actions. 03C remains a separate legacy cutover. + +## Human Review Focus And Merge Ownership + +Review the active namespace fence, pre/post setup-generation locking, complete +rehash comparison, classification precedence/limits, incident privacy, and the +deny-only AUTH boundary. The user retains the decision to mark the PR ready and +approve its merge. diff --git a/backend/alembic/versions/0040_guide_materialization_classification.py b/backend/alembic/versions/0040_guide_materialization_classification.py new file mode 100644 index 000000000..750a946af --- /dev/null +++ b/backend/alembic/versions/0040_guide_materialization_classification.py @@ -0,0 +1,131 @@ +"""add verified guide classifications and bounded incidents + +Revision ID: 0040_guide_materialization +Revises: 0039_guide_source_bindings +Create Date: 2026-07-29 +""" + +from __future__ import annotations + +from alembic import op +import sqlalchemy as sa + + +revision = "0040_guide_materialization" +down_revision = "0039_guide_source_bindings" +branch_labels = depends_on = None + + +def upgrade() -> None: + op.create_unique_constraint( + "uq_guide_bindings_exact_read", + "guide_source_artifact_bindings", + ["id", "content_id", "verified_replica_id", "setup_generation"], + ) + op.create_table( + "guide_source_format_classifications", + sa.Column("id", sa.String(36), primary_key=True), + sa.Column("binding_id", sa.String(36), nullable=False), + sa.Column("content_id", sa.String(36), nullable=False), + sa.Column("verified_replica_id", sa.String(36), nullable=False), + sa.Column("setup_generation", sa.BigInteger(), nullable=False), + sa.Column("sha256", sa.String(71), nullable=False), + sa.Column("byte_count", sa.BigInteger(), nullable=False), + sa.Column("media_type", sa.String(255), nullable=False), + sa.Column("detected_format", sa.String(40), nullable=False), + sa.Column("status", sa.String(40), nullable=False), + sa.Column("detector_name", sa.String(100), nullable=False), + sa.Column("detector_version", sa.String(40), nullable=False), + sa.Column("classification_facts", sa.JSON(), nullable=False), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + sa.ForeignKeyConstraint( + ["binding_id", "content_id", "verified_replica_id", "setup_generation"], + [ + "guide_source_artifact_bindings.id", + "guide_source_artifact_bindings.content_id", + "guide_source_artifact_bindings.verified_replica_id", + "guide_source_artifact_bindings.setup_generation", + ], + name="fk_guide_classifications_exact_binding", + ), + sa.UniqueConstraint("binding_id", name="uq_guide_classifications_binding"), + sa.CheckConstraint( + "status in ('classified','unsupported','ambiguous','malformed','limit_exceeded')", + name="ck_guide_classifications_status", + ), + sa.CheckConstraint( + "sha256 ~ '^sha256:[0-9a-f]{64}$'", + name="ck_guide_source_format_classifications_sha256_shape", + ), + sa.CheckConstraint( + "byte_count >= 0", name="ck_guide_source_format_classifications_byte_count_nonnegative" + ), + ) + op.create_table( + "guide_source_artifact_incidents", + sa.Column("id", sa.String(36), primary_key=True), + sa.Column("binding_id", sa.String(36), nullable=False), + sa.Column("content_id", sa.String(36), nullable=False), + sa.Column("verified_replica_id", sa.String(36), nullable=False), + sa.Column("setup_generation", sa.BigInteger(), nullable=False), + sa.Column("code", sa.String(40), nullable=False), + sa.Column("observed_sha256", sa.String(71), nullable=True), + sa.Column("observed_byte_count", sa.BigInteger(), nullable=True), + sa.Column("bounded_facts", sa.JSON(), nullable=False), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + sa.ForeignKeyConstraint( + ["binding_id", "content_id", "verified_replica_id", "setup_generation"], + [ + "guide_source_artifact_bindings.id", + "guide_source_artifact_bindings.content_id", + "guide_source_artifact_bindings.verified_replica_id", + "guide_source_artifact_bindings.setup_generation", + ], + name="fk_guide_incidents_exact_binding", + ), + sa.CheckConstraint( + "code in ('missing','changed','truncated','unavailable','stale','conflict')", + name="ck_guide_incidents_code", + ), + sa.CheckConstraint( + "observed_byte_count is null or observed_byte_count >= 0", + name="ck_guide_source_artifact_incidents_size", + ), + sa.CheckConstraint( + "observed_sha256 is null or observed_sha256 ~ '^sha256:[0-9a-f]{64}$'", + name="ck_guide_source_artifact_incidents_sha256", + ), + ) + for table in ("guide_source_format_classifications", "guide_source_artifact_incidents"): + for column in ("binding_id", "content_id", "verified_replica_id"): + op.create_index(op.f(f"ix_{table}_{column}"), table, [column]) + + +def downgrade() -> None: + bind = op.get_bind() + bind.execute( + sa.text( + "lock table guide_source_format_classifications, guide_source_artifact_incidents in access exclusive mode" + ) + ) + if bind.execute( + sa.text( + "select exists(select 1 from guide_source_format_classifications) or exists(select 1 from guide_source_artifact_incidents)" + ) + ).scalar_one(): + raise RuntimeError("cannot downgrade populated guide materialization evidence") + op.drop_table("guide_source_artifact_incidents") + op.drop_table("guide_source_format_classifications") + op.drop_constraint( + "uq_guide_bindings_exact_read", "guide_source_artifact_bindings", type_="unique" + ) diff --git a/backend/app/db/models.py b/backend/app/db/models.py index 5b26d35fc..7c002422c 100644 --- a/backend/app/db/models.py +++ b/backend/app/db/models.py @@ -11,6 +11,8 @@ ArtifactBinding, ArtifactContent, GuideSourceArtifactBinding, + GuideSourceArtifactIncident, + GuideSourceFormatClassification, ArtifactOperationReceipt, ArtifactReplica, ArtifactRecoveryAttempt, diff --git a/backend/app/interfaces/artifact_operations.py b/backend/app/interfaces/artifact_operations.py index 47270a16c..6d5925235 100644 --- a/backend/app/interfaces/artifact_operations.py +++ b/backend/app/interfaces/artifact_operations.py @@ -29,6 +29,8 @@ "GuideArtifactIngestResult", "GuideSourceBindingRequest", "GuideSourceBindingResult", + "GuideSourceMaterializationRequest", + "GuideSourceMaterializationResult", "PreparedBundleMaterializationRequest", "SubmissionBundlePreparationPort", "SubmissionBundlePreparationRequest", @@ -107,6 +109,33 @@ class GuideSourceBindingResult: replayed: bool +@dataclass(frozen=True, slots=True) +class GuideSourceMaterializationRequest: + """Exact guide binding selected for one authorized verified read.""" + + prepared_authorization: PreparedAuthorizationHandle + project_id: UUID + guide_id: UUID + guide_source_snapshot_id: UUID + source_item_id: UUID + project_setup_run_id: UUID + setup_generation: int + binding_id: UUID + + +@dataclass(frozen=True, slots=True) +class GuideSourceMaterializationResult: + """Bounded syntactic classification of independently verified guide bytes.""" + + classification_id: UUID + binding_id: UUID + content_id: UUID + setup_generation: int + detected_format: str + status: str + replayed: bool + + @dataclass(frozen=True, slots=True) class SubmissionBindingRequest: """Verified contributor content and its exact Submission owner.""" @@ -226,7 +255,9 @@ async def prepare(self, request: SubmissionBundlePreparationRequest) -> object: class ArtifactBindingPort(Protocol): """Create exact action-bound bindings from verified content.""" - async def bind_guide_source(self, request: GuideSourceBindingRequest) -> GuideSourceBindingResult: + async def bind_guide_source( + self, request: GuideSourceBindingRequest + ) -> GuideSourceBindingResult: """Bind verified guide content under the guide binding action.""" async def bind_submission(self, request: SubmissionBindingRequest) -> object: @@ -245,6 +276,12 @@ async def materialize_prepared_bundle( ) -> object: """Materialize one process-local prepared bundle generation.""" + async def materialize_guide_source( + self, + request: GuideSourceMaterializationRequest, + ) -> GuideSourceMaterializationResult: + """Materialize and classify one exact verified guide binding.""" + async def materialize_bindings( self, request: BindingMaterializationRequest, diff --git a/backend/app/modules/artifacts/guide_formats.py b/backend/app/modules/artifacts/guide_formats.py new file mode 100644 index 000000000..646a20bb1 --- /dev/null +++ b/backend/app/modules/artifacts/guide_formats.py @@ -0,0 +1,352 @@ +"""Deterministic syntactic guide-format detection over verified scratch bytes.""" + +from __future__ import annotations + +from dataclasses import dataclass +from io import BytesIO +from pathlib import PurePosixPath +import stat +from typing import BinaryIO +import zipfile +from xml.etree import ElementTree + + +DETECTOR_NAME = "workstream.guide_format" +DETECTOR_VERSION = "1" +_SAMPLE_BYTES = 64 * 1024 +_MAXIMUM_RELATIONSHIP_BYTES = 1024 * 1024 +_EXECUTABLE_SUFFIXES = { + ".bat", + ".cmd", + ".com", + ".dll", + ".exe", + ".jar", + ".js", + ".msi", + ".ps1", + ".scr", +} + + +@dataclass(frozen=True, slots=True) +class GuideFormatLimits: + """Fixed startup-owned limits for structural guide inspection.""" + + maximum_entries: int = 2_000 + maximum_central_directory_bytes: int = 8 * 1024 * 1024 + maximum_decompressed_bytes: int = 128 * 1024 * 1024 + maximum_nesting_depth: int = 8 + maximum_compression_ratio: int = 100 + maximum_image_pixels: int = 40_000_000 + maximum_image_dimension: int = 16_384 + + def archive_totals_exceeded(self, *, entry_count: int, decompressed_bytes: int) -> bool: + return ( + entry_count > self.maximum_entries + or decompressed_bytes > self.maximum_decompressed_bytes + ) + + def compression_ratio_exceeded(self, *, file_size: int, compressed_size: int) -> bool: + return file_size > 0 and ( + compressed_size == 0 + or file_size > compressed_size * self.maximum_compression_ratio + ) + + +@dataclass(frozen=True, slots=True) +class GuideFormatResult: + """Bounded classification facts containing no source-controlled names.""" + + detected_format: str + status: str + facts: dict[str, int | str | bool] + + +@dataclass(frozen=True, slots=True) +class BoundGuideFormatInspector: + """Bind approved item metadata to the one syntactic format detector.""" + + detector: GuideFormatDetector + declared_media_type: str + ingestion_adapter: str + + def inspect(self, reader: BinaryIO) -> GuideFormatResult: + return self.detector.detect( + reader, + declared_media_type=self.declared_media_type, + ingestion_adapter=self.ingestion_adapter, + ) + + +class GuideFormatDetector: + """Classify signatures and bounded containers without semantic extraction.""" + + def __init__(self, limits: GuideFormatLimits) -> None: + self._limits = limits + + def detect( + self, + reader: BinaryIO, + *, + declared_media_type: str, + ingestion_adapter: str | None = None, + ) -> GuideFormatResult: + reader.seek(0) + header = reader.read(_SAMPLE_BYTES) + reader.seek(0) + if header.startswith(b"%PDF-"): + return self._classified("pdf") + image = self._image_dimensions(header) + if image is not None: + image_format, width, height = image + if ( + width > self._limits.maximum_image_dimension + or height > self._limits.maximum_image_dimension + or width * height > self._limits.maximum_image_pixels + ): + return GuideFormatResult( + detected_format=image_format, + status="limit_exceeded", + facts={"width": width, "height": height}, + ) + return GuideFormatResult( + detected_format=image_format, + status="classified", + facts={"width": width, "height": height}, + ) + if self._is_audio_video_signature(header): + return GuideFormatResult("audio_video", "unsupported", {}) + if header.startswith(b"PK\x03\x04") or header.startswith(b"PK\x05\x06"): + return self._inspect_zip(reader) + media_type = declared_media_type.split(";", 1)[0].strip().lower() + declared = { + "application/json": "json", + "text/csv": "csv", + "text/markdown": "markdown", + "text/plain": "plain_text", + }.get(media_type) + if declared is None: + declared = { + "csv": "csv", + "json": "json", + "markdown": "markdown", + "text": "plain_text", + "plain_text": "plain_text", + }.get((ingestion_adapter or "").strip().lower()) + if declared is not None: + try: + header.decode("utf-8") + except UnicodeDecodeError: + return GuideFormatResult(declared, "malformed", {}) + return self._classified(declared) + if media_type.startswith("audio/") or media_type.startswith("video/"): + return GuideFormatResult("audio_video", "unsupported", {}) + try: + decoded = header.decode("utf-8") + except UnicodeDecodeError: + return GuideFormatResult("opaque", "unsupported", {}) + if "\x00" in decoded: + return GuideFormatResult("opaque", "unsupported", {}) + return self._classified("plain_text") + + def _inspect_zip(self, reader: BinaryIO) -> GuideFormatResult: + state = {"entries": 0, "decompressed": 0, "compressed": 0, "depth": 0} + try: + unsafe = self._inspect_archive(reader, state=state, depth=0) + except (OSError, ValueError, zipfile.BadZipFile, RuntimeError): + return GuideFormatResult("zip", "malformed", self._bounded_zip_facts(state)) + if unsafe is not None: + return GuideFormatResult("zip", unsafe, self._bounded_zip_facts(state)) + reader.seek(0) + with zipfile.ZipFile(reader) as archive: + names = {info.filename for info in archive.infolist()} + markers = { + "docx": {"[Content_Types].xml", "_rels/.rels", "word/document.xml"}, + "pptx": {"[Content_Types].xml", "_rels/.rels", "ppt/presentation.xml"}, + "xlsx": {"[Content_Types].xml", "_rels/.rels", "xl/workbook.xml"}, + } + matches = [name for name, required in markers.items() if required <= names] + facts = self._bounded_zip_facts(state) + if len(matches) > 1: + return GuideFormatResult("zip", "ambiguous", facts) + if not matches: + return GuideFormatResult("zip", "unsupported", facts) + return GuideFormatResult(matches[0], "classified", facts) + + def _inspect_archive( + self, + source: BinaryIO | BytesIO, + *, + state: dict[str, int], + depth: int, + ) -> str | None: + state["depth"] = max(state["depth"], depth) + if depth > self._limits.maximum_nesting_depth: + return "limit_exceeded" + entry_count, central_directory_bytes = self._zip_directory_facts(source) + if ( + state["entries"] + entry_count > self._limits.maximum_entries + or central_directory_bytes > self._limits.maximum_central_directory_bytes + ): + return "limit_exceeded" + source.seek(0) + with zipfile.ZipFile(source) as archive: + seen: set[str] = set() + for info in archive.infolist(): + state["entries"] += 1 + state["decompressed"] += info.file_size + state["compressed"] += info.compress_size + if self._limits.archive_totals_exceeded( + entry_count=state["entries"], + decompressed_bytes=state["decompressed"], + ) or self._limits.compression_ratio_exceeded( + file_size=info.file_size, + compressed_size=info.compress_size, + ): + return "limit_exceeded" + normalized = info.filename.replace("\\", "/") + path = PurePosixPath(normalized) + folded = normalized.casefold() + if ( + not normalized + or normalized.startswith("/") + or (path.parts and ":" in path.parts[0]) + or ".." in path.parts + or folded in seen + or info.flag_bits & 0x1 + ): + return "malformed" + seen.add(folded) + mode = info.external_attr >> 16 + kind = stat.S_IFMT(mode) + if kind not in {0, stat.S_IFREG, stat.S_IFDIR}: + return "malformed" + lower = normalized.lower() + if ( + lower.endswith("vbaproject.bin") + or any(lower.endswith(suffix) for suffix in _EXECUTABLE_SUFFIXES) + or "/embeddings/" in f"/{lower}" + ): + return "malformed" + if lower.endswith(".rels"): + if info.file_size > _MAXIMUM_RELATIONSHIP_BYTES: + return "limit_exceeded" + relationship = archive.read(info) + if b" tuple[int, int]: + """Read the bounded EOCD before ZipFile allocates its entry inventory.""" + source.seek(0, 2) + size = source.tell() + source.seek(max(0, size - 65_557)) + tail = source.read(65_557) + marker = tail.rfind(b"PK\x05\x06") + if marker < 0 or len(tail) - marker < 22: + raise zipfile.BadZipFile("missing end of central directory") + if tail[marker + 4 : marker + 8] != b"\x00\x00\x00\x00": + raise zipfile.BadZipFile("multi-disk archive") + entry_count = int.from_bytes(tail[marker + 10 : marker + 12], "little") + central_directory_bytes = int.from_bytes(tail[marker + 12 : marker + 16], "little") + if entry_count == 0xFFFF or central_directory_bytes == 0xFFFFFFFF: + raise zipfile.BadZipFile("zip64 archive is unsupported") + return entry_count, central_directory_bytes + + @staticmethod + def _is_audio_video_signature(header: bytes) -> bool: + return ( + header.startswith((b"ID3", b"fLaC", b"OggS")) + or (header.startswith(b"RIFF") and header[8:12] in {b"WAVE", b"AVI "}) + or (len(header) >= 12 and header[4:8] == b"ftyp") + ) + + @staticmethod + def _image_dimensions(header: bytes) -> tuple[str, int, int] | None: + if header.startswith(b"\x89PNG\r\n\x1a\n") and len(header) >= 24: + return "png", int.from_bytes(header[16:20], "big"), int.from_bytes(header[20:24], "big") + if header.startswith(b"RIFF") and header[8:12] == b"WEBP" and len(header) >= 30: + if header[12:16] == b"VP8X": + return ( + "webp", + 1 + int.from_bytes(header[24:27], "little"), + 1 + int.from_bytes(header[27:30], "little"), + ) + if header[12:16] == b"VP8 " and header[23:26] == b"\x9d\x01\x2a": + return ( + "webp", + int.from_bytes(header[26:28], "little") & 0x3FFF, + int.from_bytes(header[28:30], "little") & 0x3FFF, + ) + if header[12:16] == b"VP8L" and header[20] == 0x2F: + return ( + "webp", + 1 + header[21] + ((header[22] & 0x3F) << 8), + 1 + + (header[22] >> 6) + + (header[23] << 2) + + ((header[24] & 0x0F) << 10), + ) + if header.startswith(b"\xff\xd8"): + offset = 2 + while offset + 9 <= len(header): + if header[offset] != 0xFF: + offset += 1 + continue + marker = header[offset + 1] + length = int.from_bytes(header[offset + 2 : offset + 4], "big") + if marker in { + 0xC0, + 0xC1, + 0xC2, + 0xC3, + 0xC5, + 0xC6, + 0xC7, + 0xC9, + 0xCA, + 0xCB, + 0xCD, + 0xCE, + 0xCF, + }: + return ( + "jpeg", + int.from_bytes(header[offset + 7 : offset + 9], "big"), + int.from_bytes(header[offset + 5 : offset + 7], "big"), + ) + if length < 2: + break + offset += 2 + length + return None + + @staticmethod + def _classified(detected_format: str) -> GuideFormatResult: + return GuideFormatResult(detected_format, "classified", {}) + + @staticmethod + def _bounded_zip_facts(state: dict[str, int]) -> dict[str, int]: + return { + "entry_count": state["entries"], + "decompressed_bytes": state["decompressed"], + "maximum_depth": state["depth"], + } diff --git a/backend/app/modules/artifacts/guide_materialization.py b/backend/app/modules/artifacts/guide_materialization.py new file mode 100644 index 000000000..a810d7faa --- /dev/null +++ b/backend/app/modules/artifacts/guide_materialization.py @@ -0,0 +1,457 @@ +"""Hidden verified guide materialization and syntactic classification.""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Protocol +from uuid import UUID, uuid4 + +from sqlalchemy import func, select +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from app.interfaces.artifact_operations import ( + GuideSourceMaterializationRequest, + GuideSourceMaterializationResult, +) +from app.interfaces.artifacts import ( + ArtifactInputMismatchError, + ArtifactObjectMissingError, + ArtifactStore, + ArtifactStoreUnavailableError, +) +from app.modules.artifacts.guide_formats import ( + DETECTOR_NAME, + DETECTOR_VERSION, + BoundGuideFormatInspector, + GuideFormatDetector, +) +from app.modules.artifacts.models import ( + ArtifactContent, + ArtifactPutAttempt, + ArtifactReplica, + ArtifactStorageNamespace, + ArtifactVerificationJob, + ArtifactVerificationReceipt, + GuideSourceArtifactBinding, + GuideSourceArtifactIncident, + GuideSourceFormatClassification, +) +from app.modules.artifacts.preparation import ( + ArtifactPreparationDeadlineError, + ArtifactPreparationService, +) +from app.modules.artifacts.schemas import ( + ArtifactAuthorityDeniedError, + GuideSourceReadAuthorityFacts, +) +from app.modules.artifacts.service import ( + ArtifactStorageNamespaceSpec, + validate_artifact_replica_execution_namespace, +) +from app.modules.authorization.prepared import PreparedAuthorizationHandle +from app.modules.projects.models import ( + GuideSourceSnapshot, + GuideSourceSnapshotItem, + ProjectGuide, + ProjectSetupRun, +) + + +class GuideSourceMaterializationError(RuntimeError): + """Concealed guide materialization failure.""" + + +class GuideSourceReadPreparedAuthorization(Protocol): + """AUTH-04B seam for one transaction-bound fixed-reader capability.""" + + async def consume( + self, + *, + prepared_authorization: PreparedAuthorizationHandle, + facts: GuideSourceReadAuthorityFacts, + ) -> None: ... + + +class GuideSourceReadAuthorityFactory(Protocol): + """Create one transaction-local prepared reader adapter.""" + + def __call__(self, session: AsyncSession) -> GuideSourceReadPreparedAuthorization: ... + + +class DenyGuideSourceReadPreparedAuthorization: + """Production default until AUTH-04B activates exact guide reads.""" + + async def consume( + self, + *, + prepared_authorization: PreparedAuthorizationHandle, + facts: GuideSourceReadAuthorityFacts, + ) -> None: + del prepared_authorization, facts + raise ArtifactAuthorityDeniedError("guide source read is unavailable") + + +def _deny_reader(_: AsyncSession) -> GuideSourceReadPreparedAuthorization: + return DenyGuideSourceReadPreparedAuthorization() + + +@dataclass(frozen=True, slots=True) +class _ReadFacts: + binding_id: str + project_id: str + guide_id: str + snapshot_id: str + item_id: str + setup_run_id: str + setup_generation: int + content_id: str + replica_id: str + storage_namespace_id: str + namespace_fingerprint: str + sha256: str + byte_count: int + media_type: str + declared_media_type: str + ingestion_adapter: str + provider_object_ref: str + receipt_id: str + verification_generation: int + + +class ArtifactMaterializationService: + """Canonical hidden materializer, beginning with exact guide-source reads.""" + + def __init__( + self, + session_factory: async_sessionmaker[AsyncSession], + store: ArtifactStore, + preparation: ArtifactPreparationService, + detector: GuideFormatDetector, + namespace: ArtifactStorageNamespaceSpec, + authority_factory: GuideSourceReadAuthorityFactory = _deny_reader, + ) -> None: + self._session_factory = session_factory + self._store = store + self._preparation = preparation + self._detector = detector + self._namespace = namespace + self._authority_factory = authority_factory + + async def materialize_guide_source( + self, + request: GuideSourceMaterializationRequest, + ) -> GuideSourceMaterializationResult: + """Authorize, read, verify, classify, revalidate, and persist one result.""" + if ( + type(request.prepared_authorization) is not PreparedAuthorizationHandle + or request.setup_generation <= 0 + ): + raise GuideSourceMaterializationError("guide source read is unavailable") + async with self._session_factory() as session, session.begin(): + before = await self._load_read_facts(session, request) + if before is None: + raise GuideSourceMaterializationError("guide source read is unavailable") + await self._authority_factory(session).consume( + prepared_authorization=request.prepared_authorization, + facts=self._authority_facts(before), + ) + + prepared = None + try: + prepared = await self._preparation.prepare( + self._store.open(before.provider_object_ref), + media_type=before.media_type, + ) + commitment = prepared.commitment + if commitment.sha256 != before.sha256 or commitment.byte_count != before.byte_count: + code = "truncated" if commitment.byte_count < before.byte_count else "changed" + await self._record_incident( + before, + code, + observed_sha256=commitment.sha256, + observed_byte_count=commitment.byte_count, + ) + raise GuideSourceMaterializationError("guide artifact incident") + detected = await prepared.inspect( + BoundGuideFormatInspector( + detector=self._detector, + declared_media_type=before.declared_media_type, + ingestion_adapter=before.ingestion_adapter, + ) + ) + except ArtifactObjectMissingError: + await self._record_incident(before, "missing") + raise GuideSourceMaterializationError("guide artifact incident") from None + except ArtifactInputMismatchError: + # Kept fail-closed for alternate preparation implementations. + await self._record_incident(before, "changed") + raise GuideSourceMaterializationError("guide artifact incident") from None + except (ArtifactStoreUnavailableError, ArtifactPreparationDeadlineError): + await self._record_incident(before, "unavailable") + raise GuideSourceMaterializationError("guide artifact incident") from None + finally: + if prepared is not None: + await prepared.close() + + stale = False + result: GuideSourceMaterializationResult | None = None + async with self._session_factory() as session, session.begin(): + after = await self._load_read_facts(session, request) + if after != before: + await self._add_incident(session, before, "stale") + stale = True + else: + existing = await session.scalar( + select(GuideSourceFormatClassification) + .where(GuideSourceFormatClassification.binding_id == before.binding_id) + .with_for_update() + ) + if existing is not None: + if ( + existing.content_id != before.content_id + or existing.verified_replica_id != before.replica_id + or existing.setup_generation != before.setup_generation + or existing.sha256 != before.sha256 + or existing.byte_count != before.byte_count + or existing.media_type != before.media_type + or existing.detected_format != detected.detected_format + or existing.status != detected.status + or existing.classification_facts != detected.facts + or existing.detector_name != DETECTOR_NAME + or existing.detector_version != DETECTOR_VERSION + ): + raise GuideSourceMaterializationError("guide classification conflicts") + result = self._result(existing, replayed=True) + else: + classification = GuideSourceFormatClassification( + id=str(uuid4()), + binding_id=before.binding_id, + content_id=before.content_id, + verified_replica_id=before.replica_id, + setup_generation=before.setup_generation, + sha256=before.sha256, + byte_count=before.byte_count, + media_type=before.media_type, + detected_format=detected.detected_format, + status=detected.status, + detector_name=DETECTOR_NAME, + detector_version=DETECTOR_VERSION, + classification_facts=detected.facts, + ) + session.add(classification) + await session.flush() + result = self._result(classification, replayed=False) + if stale: + raise GuideSourceMaterializationError("guide artifact incident") + if result is None: + raise GuideSourceMaterializationError("guide source read is unavailable") + return result + + async def _load_read_facts( + self, + session: AsyncSession, + request: GuideSourceMaterializationRequest, + ) -> _ReadFacts | None: + latest_generation = ( + select(func.max(ProjectSetupRun.setup_generation)) + .where(ProjectSetupRun.guide_id == str(request.guide_id)) + .scalar_subquery() + ) + row = ( + await session.execute( + select( + GuideSourceArtifactBinding, + ArtifactContent, + ArtifactReplica, + ArtifactVerificationJob, + ArtifactVerificationReceipt, + GuideSourceSnapshotItem, + ArtifactStorageNamespace, + ) + .join(ArtifactContent, ArtifactContent.id == GuideSourceArtifactBinding.content_id) + .join( + ArtifactReplica, + ArtifactReplica.id == GuideSourceArtifactBinding.verified_replica_id, + ) + .join( + ArtifactStorageNamespace, + ArtifactStorageNamespace.id == ArtifactReplica.storage_namespace_id, + ) + .join(ProjectGuide, ProjectGuide.id == GuideSourceArtifactBinding.guide_id) + .join( + GuideSourceSnapshot, + GuideSourceSnapshot.id == GuideSourceArtifactBinding.source_snapshot_id, + ) + .join( + GuideSourceSnapshotItem, + GuideSourceSnapshotItem.id == GuideSourceArtifactBinding.source_item_id, + ) + .join( + ProjectSetupRun, + ProjectSetupRun.id == GuideSourceArtifactBinding.project_setup_run_id, + ) + .join(ArtifactPutAttempt, ArtifactPutAttempt.replica_id == ArtifactReplica.id) + .join( + ArtifactVerificationJob, + ArtifactVerificationJob.originating_put_attempt_id == ArtifactPutAttempt.id, + ) + .join( + ArtifactVerificationReceipt, + ArtifactVerificationReceipt.verification_job_id == ArtifactVerificationJob.id, + ) + .where( + GuideSourceArtifactBinding.id == str(request.binding_id), + GuideSourceArtifactBinding.project_id == str(request.project_id), + GuideSourceArtifactBinding.guide_id == str(request.guide_id), + GuideSourceArtifactBinding.source_snapshot_id + == str(request.guide_source_snapshot_id), + GuideSourceArtifactBinding.source_item_id == str(request.source_item_id), + GuideSourceArtifactBinding.project_setup_run_id + == str(request.project_setup_run_id), + GuideSourceArtifactBinding.setup_generation == request.setup_generation, + ProjectGuide.status == "draft", + GuideSourceSnapshot.project_id == GuideSourceArtifactBinding.project_id, + GuideSourceSnapshot.guide_id == GuideSourceArtifactBinding.guide_id, + GuideSourceSnapshot.guide_version == ProjectGuide.version, + GuideSourceSnapshotItem.source_snapshot_id + == GuideSourceArtifactBinding.source_snapshot_id, + ProjectSetupRun.project_id == GuideSourceArtifactBinding.project_id, + ProjectSetupRun.guide_version == ProjectGuide.version, + ProjectSetupRun.source_snapshot_hash == GuideSourceSnapshot.bundle_hash, + ProjectSetupRun.setup_generation == latest_generation, + ArtifactContent.media_type.is_not(None), + ArtifactReplica.verification_state == "verified", + ArtifactReplica.availability_state == "available", + ArtifactReplica.integrity_state == "valid", + ArtifactVerificationJob.status == "verified", + ArtifactVerificationJob.replica_id == ArtifactReplica.id, + ArtifactVerificationJob.terminal_result_code == "verified", + ArtifactVerificationJob.terminal_at.is_not(None), + ArtifactVerificationReceipt.execution_generation + == ArtifactVerificationJob.execution_generation, + ArtifactVerificationReceipt.outcome == "verified", + ArtifactVerificationReceipt.observed_sha256 == ArtifactContent.sha256, + ArtifactVerificationReceipt.observed_byte_count == ArtifactContent.byte_count, + ArtifactPutAttempt.guide_source_item_id + == GuideSourceArtifactBinding.source_item_id, + ArtifactPutAttempt.sha256 == ArtifactContent.sha256, + ArtifactPutAttempt.byte_count == ArtifactContent.byte_count, + ArtifactPutAttempt.storage_namespace_id == ArtifactReplica.storage_namespace_id, + ArtifactPutAttempt.namespace_fingerprint + == ArtifactReplica.namespace_fingerprint, + ArtifactPutAttempt.status == "object_confirmed", + ArtifactPutAttempt.terminal_result_code == "object_confirmed", + ) + .order_by(ArtifactVerificationReceipt.created_at.desc()) + .limit(1) + .with_for_update(of=(GuideSourceArtifactBinding, ArtifactReplica, ProjectSetupRun)) + ) + ).one_or_none() + if row is None: + return None + binding, content, replica, job, receipt, item, persisted_namespace = row + validate_artifact_replica_execution_namespace( + replica=replica, + persisted=persisted_namespace, + namespace=self._namespace, + store=self._store, + ) + return _ReadFacts( + binding_id=binding.id, + project_id=binding.project_id, + guide_id=binding.guide_id, + snapshot_id=binding.source_snapshot_id, + item_id=binding.source_item_id, + setup_run_id=binding.project_setup_run_id, + setup_generation=binding.setup_generation, + content_id=content.id, + replica_id=replica.id, + storage_namespace_id=replica.storage_namespace_id, + namespace_fingerprint=replica.namespace_fingerprint, + sha256=content.sha256, + byte_count=content.byte_count, + media_type=content.media_type, + declared_media_type=item.media_type or content.media_type, + ingestion_adapter=item.ingestion_adapter, + provider_object_ref=replica.provider_object_ref, + receipt_id=receipt.id, + verification_generation=job.execution_generation, + ) + + @staticmethod + def _authority_facts(facts: _ReadFacts) -> GuideSourceReadAuthorityFacts: + return GuideSourceReadAuthorityFacts( + project_id=UUID(facts.project_id), + guide_id=UUID(facts.guide_id), + guide_source_snapshot_id=UUID(facts.snapshot_id), + guide_source_item_id=UUID(facts.item_id), + project_setup_run_id=UUID(facts.setup_run_id), + setup_generation=facts.setup_generation, + binding_id=UUID(facts.binding_id), + content_id=UUID(facts.content_id), + verified_replica_id=UUID(facts.replica_id), + storage_namespace_id=facts.storage_namespace_id, + namespace_fingerprint=facts.namespace_fingerprint, + verification_receipt_id=UUID(facts.receipt_id), + verification_generation=facts.verification_generation, + sha256=facts.sha256, + byte_count=facts.byte_count, + media_type=facts.media_type, + ) + + async def _record_incident( + self, + facts: _ReadFacts, + code: str, + *, + observed_sha256: str | None = None, + observed_byte_count: int | None = None, + ) -> None: + async with self._session_factory() as session, session.begin(): + await self._add_incident( + session, + facts, + code, + observed_sha256=observed_sha256, + observed_byte_count=observed_byte_count, + ) + + @staticmethod + async def _add_incident( + session: AsyncSession, + facts: _ReadFacts, + code: str, + *, + observed_sha256: str | None = None, + observed_byte_count: int | None = None, + ) -> None: + session.add( + GuideSourceArtifactIncident( + id=str(uuid4()), + binding_id=facts.binding_id, + content_id=facts.content_id, + verified_replica_id=facts.replica_id, + setup_generation=facts.setup_generation, + code=code, + observed_sha256=observed_sha256, + observed_byte_count=observed_byte_count, + bounded_facts={"verification_generation": facts.verification_generation}, + ) + ) + await session.flush() + + @staticmethod + def _result( + classification: GuideSourceFormatClassification, + *, + replayed: bool, + ) -> GuideSourceMaterializationResult: + return GuideSourceMaterializationResult( + classification_id=UUID(classification.id), + binding_id=UUID(classification.binding_id), + content_id=UUID(classification.content_id), + setup_generation=classification.setup_generation, + detected_format=classification.detected_format, + status=classification.status, + replayed=replayed, + ) diff --git a/backend/app/modules/artifacts/models.py b/backend/app/modules/artifacts/models.py index 62d70ad6c..3b68f8e5f 100644 --- a/backend/app/modules/artifacts/models.py +++ b/backend/app/modules/artifacts/models.py @@ -253,6 +253,13 @@ class GuideSourceArtifactBinding(Base): "setup_generation", name="uq_guide_bindings_item_generation", ), + UniqueConstraint( + "id", + "content_id", + "verified_replica_id", + "setup_generation", + name="uq_guide_bindings_exact_read", + ), UniqueConstraint("supersedes_binding_id", name="uq_guide_bindings_supersedes"), CheckConstraint("setup_generation > 0", name="ck_guide_bindings_generation_positive"), CheckConstraint("logical_role = 'guide_source_original'", name="ck_guide_bindings_role"), @@ -277,6 +284,93 @@ class GuideSourceArtifactBinding(Base): created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) +class GuideSourceFormatClassification(Base): + """Immutable syntactic classification of one verified guide binding.""" + + __tablename__ = "guide_source_format_classifications" + __table_args__ = ( + ForeignKeyConstraint( + ["binding_id", "content_id", "verified_replica_id", "setup_generation"], + [ + "guide_source_artifact_bindings.id", + "guide_source_artifact_bindings.content_id", + "guide_source_artifact_bindings.verified_replica_id", + "guide_source_artifact_bindings.setup_generation", + ], + name="fk_guide_classifications_exact_binding", + ), + UniqueConstraint("binding_id", name="uq_guide_classifications_binding"), + CheckConstraint( + "status in ('classified', 'unsupported', 'ambiguous', 'malformed', 'limit_exceeded')", + name="ck_guide_classifications_status", + ), + CheckConstraint( + SHA256_CHECK.format(column="sha256"), + name="ck_guide_source_format_classifications_sha256_shape", + ), + CheckConstraint( + "byte_count >= 0", + name="ck_guide_source_format_classifications_byte_count_nonnegative", + ), + ) + + id: Mapped[str] = mapped_column(String(36), primary_key=True) + binding_id: Mapped[str] = mapped_column(String(36), nullable=False, index=True) + content_id: Mapped[str] = mapped_column(String(36), nullable=False, index=True) + verified_replica_id: Mapped[str] = mapped_column(String(36), nullable=False, index=True) + setup_generation: Mapped[int] = mapped_column(BigInteger, nullable=False) + sha256: Mapped[str] = mapped_column(String(71), nullable=False) + byte_count: Mapped[int] = mapped_column(BigInteger, nullable=False) + media_type: Mapped[str] = mapped_column(String(255), nullable=False) + detected_format: Mapped[str] = mapped_column(String(40), nullable=False) + status: Mapped[str] = mapped_column(String(40), nullable=False) + detector_name: Mapped[str] = mapped_column(String(100), nullable=False) + detector_version: Mapped[str] = mapped_column(String(40), nullable=False) + classification_facts: Mapped[dict] = mapped_column(JSON, nullable=False) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) + + +class GuideSourceArtifactIncident(Base): + """Bounded ART-owned custody failure for one exact guide read.""" + + __tablename__ = "guide_source_artifact_incidents" + __table_args__ = ( + ForeignKeyConstraint( + ["binding_id", "content_id", "verified_replica_id", "setup_generation"], + [ + "guide_source_artifact_bindings.id", + "guide_source_artifact_bindings.content_id", + "guide_source_artifact_bindings.verified_replica_id", + "guide_source_artifact_bindings.setup_generation", + ], + name="fk_guide_incidents_exact_binding", + ), + CheckConstraint( + "code in ('missing', 'changed', 'truncated', 'unavailable', 'stale', 'conflict')", + name="ck_guide_incidents_code", + ), + CheckConstraint( + "observed_byte_count is null or observed_byte_count >= 0", + name="ck_guide_source_artifact_incidents_size", + ), + CheckConstraint( + "observed_sha256 is null or " + SHA256_CHECK.format(column="observed_sha256"), + name="ck_guide_source_artifact_incidents_sha256", + ), + ) + + id: Mapped[str] = mapped_column(String(36), primary_key=True) + binding_id: Mapped[str] = mapped_column(String(36), nullable=False, index=True) + content_id: Mapped[str] = mapped_column(String(36), nullable=False, index=True) + verified_replica_id: Mapped[str] = mapped_column(String(36), nullable=False, index=True) + setup_generation: Mapped[int] = mapped_column(BigInteger, nullable=False) + code: Mapped[str] = mapped_column(String(40), nullable=False) + observed_sha256: Mapped[str | None] = mapped_column(String(71)) + observed_byte_count: Mapped[int | None] = mapped_column(BigInteger) + bounded_facts: Mapped[dict] = mapped_column(JSON, nullable=False) + created_at: Mapped[datetime] = mapped_column(DateTime(timezone=True), server_default=func.now()) + + class ArtifactStorageNamespace(Base): """Immutable singleton fencing one deployment to one storage namespace.""" diff --git a/backend/app/modules/artifacts/preparation.py b/backend/app/modules/artifacts/preparation.py index 4b0edda1b..aef868027 100644 --- a/backend/app/modules/artifacts/preparation.py +++ b/backend/app/modules/artifacts/preparation.py @@ -17,7 +17,7 @@ import stat as stat_module import threading import time -from typing import Any, BinaryIO +from typing import Any, BinaryIO, TypeVar from uuid import uuid4 from app.interfaces.artifacts import ( @@ -36,10 +36,12 @@ ArtifactCommitment, CommittedArtifactSource, PreparedArtifact, + PreparedArtifactInspector, ) HARD_MAXIMUM_ARTIFACT_BYTES = 512 * 1024 * 1024 +_InspectionResult = TypeVar("_InspectionResult") _LEDGER_VERSION = 2 _LEDGER_MAXIMUM_BYTES = 1024 * 1024 _RESERVATION_ID = re.compile(r"^[0-9a-f]{32}$") @@ -1360,6 +1362,31 @@ async def release_prepared_artifact(self, binding: object) -> None: await self._manager.release(active.reservation) self._active.pop(binding, None) + async def inspect_prepared_artifact( + self, + binding: object, + inspector: PreparedArtifactInspector[_InspectionResult], + ) -> _InspectionResult: + """Run one trusted inspector while retaining scratch lifecycle custody.""" + active = self._active.get(binding) + if active is None or not active.handle_issued or active.stream_claimed: + raise ArtifactScratchIntegrityError("prepared artifact source is unavailable") + + def inspect_and_rewind() -> _InspectionResult: + active.reader.seek(0) + try: + return inspector.inspect(active.reader) + finally: + active.reader.seek(0) + + try: + async with asyncio.timeout_at(active.deadline): + return await self._run_io(inspect_and_rewind) + except TimeoutError: + raise ArtifactPreparationDeadlineError( + "artifact preparation deadline exceeded" + ) from None + async def _release_unhanded_preparation( self, pending: _PendingPreparationCleanup, diff --git a/backend/app/modules/artifacts/schemas.py b/backend/app/modules/artifacts/schemas.py index a6907bf58..aece39ac4 100644 --- a/backend/app/modules/artifacts/schemas.py +++ b/backend/app/modules/artifacts/schemas.py @@ -106,6 +106,29 @@ class GuideSourceBindingAuthorityFacts: logical_role: str +@final +@dataclass(frozen=True, slots=True) +class GuideSourceReadAuthorityFacts: + """Canonical fixed-reader facts for one verified guide materialization.""" + + project_id: UUID + guide_id: UUID + guide_source_snapshot_id: UUID + guide_source_item_id: UUID + project_setup_run_id: UUID + setup_generation: int + binding_id: UUID + content_id: UUID + verified_replica_id: UUID + storage_namespace_id: str + namespace_fingerprint: str + verification_receipt_id: UUID + verification_generation: int + sha256: str + byte_count: int + media_type: str + + class ArtifactInternalResourceType(StrEnum): """Closed artifact-owned resource types for internal actions.""" diff --git a/backend/app/modules/artifacts/service.py b/backend/app/modules/artifacts/service.py index 2646d8cc0..53c45171c 100644 --- a/backend/app/modules/artifacts/service.py +++ b/backend/app/modules/artifacts/service.py @@ -1412,16 +1412,12 @@ def _validate_replica_execution_namespace( persisted: ArtifactStorageNamespace, ) -> None: """Reject reads when replica identity differs from active composition.""" - if ( - replica.storage_namespace_id != persisted.id - or replica.namespace_fingerprint != persisted.namespace_fingerprint - or replica.adapter != persisted.adapter - or replica.provider_profile != persisted.provider_profile - or self._store.identity.provider_key != persisted.adapter - ): - raise ArtifactStorageNamespaceError( - "artifact replica does not match the active storage namespace" - ) + validate_artifact_replica_execution_namespace( + replica=replica, + persisted=persisted, + namespace=self._namespace, + store=self._store, + ) class ArtifactPendingWorkScanner: @@ -2485,3 +2481,27 @@ async def _claim_and_validate_storage_namespace( ): raise ArtifactStorageNamespaceError("artifact storage namespace does not match") return persisted + + +def validate_artifact_replica_execution_namespace( + *, + replica: ArtifactReplica, + persisted: ArtifactStorageNamespace, + namespace: ArtifactStorageNamespaceSpec, + store: ArtifactStore, +) -> None: + """Reject provider reads outside the exact active deployment namespace.""" + if ( + persisted.id != ARTIFACT_STORAGE_NAMESPACE_ID + or replica.storage_namespace_id != persisted.id + or replica.namespace_fingerprint != persisted.namespace_fingerprint + or replica.adapter != persisted.adapter + or replica.provider_profile != persisted.provider_profile + or namespace.adapter != persisted.adapter + or namespace.provider_profile != persisted.provider_profile + or namespace.namespace_fingerprint != persisted.namespace_fingerprint + or store.identity.provider_key != persisted.adapter + ): + raise ArtifactStorageNamespaceError( + "artifact replica does not match the active storage namespace" + ) diff --git a/backend/app/modules/artifacts/sources.py b/backend/app/modules/artifacts/sources.py index d534df355..da9b1267d 100644 --- a/backend/app/modules/artifacts/sources.py +++ b/backend/app/modules/artifacts/sources.py @@ -5,12 +5,20 @@ import asyncio from collections.abc import AsyncIterator from dataclasses import dataclass -from typing import Protocol, final +from typing import BinaryIO, Protocol, TypeVar, final from app.core.cancellation import await_completion_preserving_cancellation _COMMITTED_SOURCE_SEAL = object() +_InspectionResult = TypeVar("_InspectionResult") + + +class PreparedArtifactInspector(Protocol[_InspectionResult]): + """Typed artifact-owned inspection capability over one scratch reader.""" + + def inspect(self, reader: BinaryIO) -> _InspectionResult: + """Return bounded structural facts without retaining the reader.""" class _PreparedArtifactOwner(Protocol): @@ -26,6 +34,13 @@ def open_committed_stream( async def release_prepared_artifact(self, binding: object) -> None: """Release one preparation and its scratch reservation.""" + async def inspect_prepared_artifact( + self, + binding: object, + inspector: PreparedArtifactInspector[_InspectionResult], + ) -> _InspectionResult: + """Run a trusted bounded inspector against the owned scratch reader.""" + def claim_prepared_commitment(self, binding: object) -> ArtifactCommitment: """Claim the server-computed commitment for one registered binding.""" @@ -192,6 +207,15 @@ async def close(self) -> None: else: self._closed = True + async def inspect( + self, + inspector: PreparedArtifactInspector[_InspectionResult], + ) -> _InspectionResult: + """Inspect read-only scratch bytes without exposing a path or handle.""" + if self._closed: + raise RuntimeError("prepared artifact is closed") + return await self._owner.inspect_prepared_artifact(self._binding, inspector) + async def __aenter__(self) -> CommittedArtifactSource: """Enter the bounded provider-I/O lifetime.""" return self.committed_source diff --git a/backend/scripts/run_test_lanes.py b/backend/scripts/run_test_lanes.py index 6d2d6a5bf..0f28d2d58 100644 --- a/backend/scripts/run_test_lanes.py +++ b/backend/scripts/run_test_lanes.py @@ -104,6 +104,7 @@ class TestLane: "tests/test_external_service_adapters.py", "tests/test_guide_artifacts.py", "tests/test_guide_bindings.py", + "tests/test_guide_formats.py", "tests/test_local_artifact_store.py", "tests/test_s3_artifact_store.py", "tests/test_test_lane_evidence.py", diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index d3ea8e25d..9d203e19d 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -21,7 +21,7 @@ from scripts.run_isolated_tests import LOOPBACK, NAME_RE, ROLE_RE DDL_LOCK_DIRECTORY = Path("/tmp") -EXPECTED_PUBLIC_SCHEMA_SHA256 = "3bc7285f25506bee9c3ede9867e60cec7f045b1a02887dfe20d4879e4be93402" +EXPECTED_PUBLIC_SCHEMA_SHA256 = "d5b9373e7bcf9afbae4f8bef066c952a0b83146d90020c32d73d543d35c99c66" PROTECTED_TEST_TABLES = ( "actor_profile_migration_state", "alembic_version", @@ -55,7 +55,9 @@ "effective_project_submission_artifact_policies", "evidence_items", "guide_source_artifact_ingests", + "guide_source_artifact_incidents", "guide_source_artifact_bindings", + "guide_source_format_classifications", "guide_source_snapshot_items", "guide_source_snapshots", "guide_sufficiency_reports", diff --git a/backend/tests/test_alembic.py b/backend/tests/test_alembic.py index 5e678527b..26a62f0cc 100644 --- a/backend/tests/test_alembic.py +++ b/backend/tests/test_alembic.py @@ -1629,7 +1629,7 @@ def test_artifact_recovery_schema_and_empty_downgrade( command.downgrade(config, "base") command.upgrade(config, "head") assert asyncio.run(_artifact_recovery_schema(isolated_database_env)) == { - "revision": "0039_guide_source_bindings", + "revision": "0040_guide_materialization", "constraints": { "artifact_recovery_attempt_custody", "artifact_verification_lineage_custody", @@ -1953,7 +1953,7 @@ def test_0035_project_read_action_evidence_refuses_nonempty_downgrade( ): command.downgrade(config, "0034_project_role_issue_evidence") assert asyncio.run(_current_revision(isolated_database_env)) == ( - "0039_guide_source_bindings" + "0040_guide_materialization" ) finally: asyncio.run(_remove_authority_audit_fixture(isolated_database_env, event_id=event_id)) @@ -2080,7 +2080,7 @@ def test_0036_art_auth_catalogue_refuses_obsolete_evidence( record_id = "" command.upgrade(config, "head") assert asyncio.run(_current_revision(isolated_database_env)) == ( - "0039_guide_source_bindings" + "0040_guide_materialization" ) finally: for event_id in reversed(event_ids): @@ -2477,7 +2477,7 @@ def test_project_role_migration_constraints_and_immutable_history( command.upgrade(config, "head") result = asyncio.run(_exercise_project_role_migration(isolated_database_env)) assert result == { - "revision": "0039_guide_source_bindings", + "revision": "0040_guide_materialization", "role_count": 3, "invalid_availability": "23514", "duplicate_role": "23505", @@ -2689,7 +2689,7 @@ def test_project_role_downgrade_refuses_each_reserved_evidence_predicate( ): command.downgrade(config, "0030_artifact_verification") assert asyncio.run(_project_role_refusal_state(isolated_database_env))[:3] == ( - "0039_guide_source_bindings", + "0040_guide_materialization", True, True, ) @@ -2716,7 +2716,7 @@ def test_project_role_downgrade_refuses_each_reserved_evidence_predicate( ): command.downgrade(config, "0030_artifact_verification") assert asyncio.run(_project_role_refusal_state(isolated_database_env))[:3] == ( - "0039_guide_source_bindings", + "0040_guide_materialization", True, True, ) @@ -2744,7 +2744,7 @@ def test_outbox_migration_schema_and_downgrade_writer_guard( command.upgrade(config, "head") schema = asyncio.run(_outbox_schema(isolated_database_env)) assert schema == { - "revision": "0039_guide_source_bindings", + "revision": "0040_guide_materialization", "columns": { "aggregate_id", "aggregate_type", @@ -2804,7 +2804,7 @@ def test_outbox_migration_schema_and_downgrade_writer_guard( ) assert committed == "refused_after_commit" assert asyncio.run(_current_revision(isolated_database_env)) == ( - "0039_guide_source_bindings" + "0040_guide_materialization" ) asyncio.run(_remove_outbox_migration_row(isolated_database_env, committed_project_id)) command.downgrade(config, "0028_artifact_admission") diff --git a/backend/tests/test_artifact_architecture.py b/backend/tests/test_artifact_architecture.py index ce60340f1..5c911facd 100644 --- a/backend/tests/test_artifact_architecture.py +++ b/backend/tests/test_artifact_architecture.py @@ -26,6 +26,7 @@ CANONICAL_REQUESTS = { "GuideArtifactIngestRequest", "GuideSourceBindingRequest", + "GuideSourceMaterializationRequest", "SubmissionBindingRequest", "CheckerOutputBindingRequest", "SubmissionBundlePreparationRequest", @@ -34,7 +35,11 @@ "CheckerOutputArtifactRequest", "ArtifactRecoveryRequest", } -CANONICAL_RESULTS = {"GuideArtifactIngestResult", "GuideSourceBindingResult"} +CANONICAL_RESULTS = { + "GuideArtifactIngestResult", + "GuideSourceBindingResult", + "GuideSourceMaterializationResult", +} CANONICAL_TYPE_ALIASES = { "ArtifactAuditResourceType", "ArtifactBindingResourceType", @@ -140,19 +145,17 @@ def test_product_api_and_workers_cannot_import_or_inject_raw_artifact_types() -> assert violations == [] -def test_only_artifact_orchestrator_owns_provider_execution() -> None: - """Fence every raw store call to the artifact service owner.""" +def test_only_artifact_custody_services_own_provider_execution() -> None: + """Fence store calls to the orchestrator and narrow guide reader.""" violations: list[str] = [] for path in _python_files(APP_ROOT / "modules" / "artifacts"): tree = _tree(path) - orchestrator = next( - ( - node - for node in tree.body - if isinstance(node, ast.ClassDef) and node.name == "ArtifactStorageOrchestrator" - ), - None, - ) + owners = { + node.name: node + for node in tree.body + if isinstance(node, ast.ClassDef) + and node.name in {"ArtifactStorageOrchestrator", "ArtifactMaterializationService"} + } for node in ast.walk(tree): if ( isinstance(node, ast.Call) @@ -160,16 +163,21 @@ def test_only_artifact_orchestrator_owns_provider_execution() -> None: and node.func.attr in PROVIDER_METHODS and isinstance(node.func.value, ast.Attribute) and node.func.value.attr == "_store" - and ( - orchestrator is None - or not ( - orchestrator.lineno + ): + owner = next( + ( + candidate + for candidate in owners.values() + if candidate.lineno <= node.lineno - <= (orchestrator.end_lineno or orchestrator.lineno) - ) + <= (candidate.end_lineno or candidate.lineno) + ), + None, ) - ): - violations.append(f"{path.relative_to(BACKEND_ROOT)} calls {node.func.attr}") + if owner is None or ( + owner.name == "ArtifactMaterializationService" and node.func.attr != "open" + ): + violations.append(f"{path.relative_to(BACKEND_ROOT)} calls {node.func.attr}") assert violations == [] @@ -410,6 +418,7 @@ def test_durable_artifact_mutation_ports_require_process_local_prepared_authorit }, "ArtifactMaterializationPort": { "materialize_prepared_bundle", + "materialize_guide_source", "materialize_bindings", }, "CheckerArtifactOutputPort": {"store"}, @@ -421,6 +430,7 @@ def test_durable_artifact_mutation_ports_require_process_local_prepared_authorit "bind_submission": "SubmissionBindingRequest", "bind_checker_output": "CheckerOutputBindingRequest", "materialize_prepared_bundle": "PreparedBundleMaterializationRequest", + "materialize_guide_source": "GuideSourceMaterializationRequest", "materialize_bindings": "BindingMaterializationRequest", "store": "CheckerOutputArtifactRequest", } diff --git a/backend/tests/test_artifact_preparation.py b/backend/tests/test_artifact_preparation.py index 88f9ef4b7..f57c66dda 100644 --- a/backend/tests/test_artifact_preparation.py +++ b/backend/tests/test_artifact_preparation.py @@ -218,6 +218,58 @@ async def test_preparation_seals_exact_commitment_and_single_stream(tmp_path: Pa manager.close() +@pytest.mark.asyncio +async def test_prepared_inspection_is_scratch_owned_bounded_and_pre_stream( + tmp_path: Path, +) -> None: + """Expose bytes only to one trusted callback while lifecycle custody remains sealed.""" + manager = ArtifactScratchManager(root=tmp_path / "scratch", limits=preparation_limits()) + service = ArtifactPreparationService(manager) + prepared = await service.prepare(byte_stream(b"guide"), media_type="text/plain") + + class ReadAllInspector: + def inspect(self, reader: BinaryIO) -> bytes: + return reader.read() + + observed = await prepared.inspect(ReadAllInspector()) + + assert observed == b"guide" + async with prepared as source: + assert b"".join([chunk async for chunk in source.stream()]) == b"guide" + with pytest.raises(ArtifactScratchIntegrityError, match="unavailable"): + await prepared.inspect(ReadAllInspector()) + manager.close() + + +@pytest.mark.asyncio +async def test_prepared_inspection_deadline_releases_scratch(tmp_path: Path) -> None: + """Map an over-deadline inspector to the canonical timeout and retain cleanup custody.""" + limits = preparation_limits() + manager = ArtifactScratchManager(root=tmp_path / "scratch", limits=limits) + service = ArtifactPreparationService(manager) + prepared = await service.prepare(byte_stream(b"guide"), media_type="text/plain") + + class ReadInspector: + def inspect(self, reader: BinaryIO) -> bytes: + return reader.read() + + original_run_io = service._run_io + + async def deterministic_timeout(*_args: Any, **_kwargs: Any) -> None: + raise TimeoutError + + service._run_io = deterministic_timeout # type: ignore[method-assign] + try: + with pytest.raises(ArtifactPreparationDeadlineError, match="deadline"): + await prepared.inspect(ReadInspector()) + finally: + service._run_io = original_run_io # type: ignore[method-assign] + await prepared.close() + + assert (await manager.usage()).reservation_count == 0 + manager.close() + + def test_committed_sources_cannot_be_publicly_assembled() -> None: """Reject caller-selected commitment and stream pairing.""" from app.modules.artifacts.sources import _COMMITTED_SOURCE_SEAL diff --git a/backend/tests/test_guide_bindings.py b/backend/tests/test_guide_bindings.py index 2e653a4ab..5913105f3 100644 --- a/backend/tests/test_guide_bindings.py +++ b/backend/tests/test_guide_bindings.py @@ -3,8 +3,11 @@ from __future__ import annotations import asyncio +from collections.abc import AsyncIterator from datetime import UTC, datetime +import hashlib from pathlib import Path +from types import SimpleNamespace from typing import Any from uuid import UUID, uuid4 @@ -15,12 +18,21 @@ from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine from app.core.hashing import canonical_json_hash -from app.interfaces.artifact_operations import GuideSourceBindingRequest +from app.interfaces.artifact_operations import ( + GuideSourceBindingRequest, + GuideSourceMaterializationRequest, +) +from app.interfaces.artifacts import ArtifactObjectMissingError, ArtifactStoreUnavailableError from app.modules.actors.models import ActorIdentityLink, ActorProfile from app.modules.artifacts.guide_bindings import ( GuideSourceBindingError, GuideSourceBindingService, ) +from app.modules.artifacts.guide_formats import GuideFormatDetector, GuideFormatLimits +from app.modules.artifacts.guide_materialization import ( + ArtifactMaterializationService, + GuideSourceMaterializationError, +) from app.modules.artifacts.models import ( ArtifactContent, ArtifactPutAttempt, @@ -29,10 +41,21 @@ ArtifactVerificationJob, ArtifactVerificationReceipt, GuideSourceArtifactBinding, + GuideSourceArtifactIncident, + GuideSourceFormatClassification, +) +from app.modules.artifacts.preparation import ( + HARD_MAXIMUM_ARTIFACT_BYTES, + ArtifactPreparationDeadlineError, + ArtifactPreparationLimits, + ArtifactPreparationService, + ArtifactScratchManager, ) +from app.modules.artifacts.service import ArtifactStorageNamespaceError, ArtifactStorageNamespaceSpec from app.modules.artifacts.schemas import ( ArtifactAuthorityDeniedError, GuideSourceBindingAuthorityFacts, + GuideSourceReadAuthorityFacts, ) from app.modules.authorization.prepared import PreparedAuthorizationHandle from app.modules.projects.models import ( @@ -60,11 +83,105 @@ async def consume(self, **values: Any) -> None: raise ArtifactAuthorityDeniedError("binding denied") +class _AllowReadAuthority: + """Test-only fixed guide reader authority.""" + + def __init__(self, *, handle: PreparedAuthorizationHandle | None = None) -> None: + self.handle = handle or object.__new__(PreparedAuthorizationHandle) + self.facts: list[GuideSourceReadAuthorityFacts] = [] + + async def consume(self, **values: Any) -> None: + assert values["prepared_authorization"] is self.handle + self.facts.append(values["facts"]) + + +class _ReadStore: + """Provider-neutral test read probe.""" + + def __init__(self, payload: bytes, *, after_read=None) -> None: + self.payload = payload + self.after_read = after_read + self.open_count = 0 + self.identity = SimpleNamespace(provider_key="local") + + async def open(self, provider_object_ref: str) -> AsyncIterator[bytes]: + assert provider_object_ref.startswith("objects/") + self.open_count += 1 + yield self.payload + if self.after_read is not None: + await self.after_read() + + +class _FailingReadStore: + """Raise one sanitized provider failure only when the read begins.""" + + def __init__(self, error: Exception) -> None: + self.error = error + self.open_count = 0 + self.identity = SimpleNamespace(provider_key="local") + + async def open(self, provider_object_ref: str) -> AsyncIterator[bytes]: + del provider_object_ref + self.open_count += 1 + raise self.error + yield b"" # pragma: no cover - retain the async-iterator contract + + +class _BlockingReadStore(_ReadStore): + """Hold one provider read until the calling task is cancelled.""" + + def __init__(self, payload: bytes) -> None: + super().__init__(payload) + self.started = asyncio.Event() + + async def open(self, provider_object_ref: str) -> AsyncIterator[bytes]: + assert provider_object_ref.startswith("objects/") + self.open_count += 1 + self.started.set() + await asyncio.Event().wait() + yield self.payload + + +def _preparation( + tmp_path: Path, **limit_changes: Any +) -> tuple[ArtifactPreparationService, ArtifactScratchManager]: + limit_values = { + "aggregate_reserved_bytes": 2 * HARD_MAXIMUM_ARTIFACT_BYTES, + "maximum_files": 2, + "maximum_concurrency": 2, + "minimum_free_bytes": 0, + "reservation_ttl_seconds": 30, + "total_deadline_seconds": 10, + "cleanup_margin_seconds": 5, + "stream_buffer_bytes": 1024, + "maximum_source_bytes": 1024 * 1024, + } + limit_values.update(limit_changes) + manager = ArtifactScratchManager( + root=tmp_path / "scratch", + limits=ArtifactPreparationLimits(**limit_values), + ) + return ArtifactPreparationService(manager), manager + + +def _namespace() -> ArtifactStorageNamespaceSpec: + return ArtifactStorageNamespaceSpec( + backend="local", + adapter="local", + provider_profile="test", + namespace_descriptor={"root": "test"}, + namespace_fingerprint="sha256:" + "b" * 64, + ) + + async def _seed_binding_lineage( session, *, verified: bool = True, verification_receipt: bool = True, + sha256: str | None = None, + byte_count: int = 42, + media_type: str = "application/pdf", ) -> dict[str, UUID]: ids = { name: uuid4() @@ -81,7 +198,7 @@ async def _seed_binding_lineage( "job", ) } - digest = "sha256:" + "a" * 64 + digest = sha256 or "sha256:" + "a" * 64 namespace_fingerprint = "sha256:" + "b" * 64 session.add( ActorProfile( @@ -148,7 +265,7 @@ async def _seed_binding_lineage( durable_ref="guide.pdf", ingestion_adapter="pdf", content_hash="caller-metadata-is-not-authority", - media_type="application/pdf", + media_type=media_type, ) ) session.add( @@ -172,8 +289,8 @@ async def _seed_binding_lineage( source_item_id=str(ids["item"]), actor_profile_id=str(ids["actor"]), sha256=digest, - byte_count=42, - media_type="application/pdf", + byte_count=byte_count, + media_type=media_type, ) ) session.add( @@ -190,8 +307,8 @@ async def _seed_binding_lineage( ArtifactContent( id=str(ids["content"]), sha256=digest, - byte_count=42, - media_type="application/pdf", + byte_count=byte_count, + media_type=media_type, ) ) await session.flush() @@ -219,8 +336,8 @@ async def _seed_binding_lineage( project_id=str(ids["project"]), guide_source_item_id=str(ids["item"]), sha256=digest, - byte_count=42, - media_type="application/pdf", + byte_count=byte_count, + media_type=media_type, storage_namespace_id="primary", namespace_fingerprint=namespace_fingerprint, canonical_target="sha256/aa/" + "a" * 62, @@ -253,7 +370,7 @@ async def _seed_binding_lineage( execution_generation=0, outcome="verified", observed_sha256=digest, - observed_byte_count=42, + observed_byte_count=byte_count, ) ) await session.commit() @@ -276,6 +393,523 @@ def _request(ids: dict[str, UUID], authority: _AllowBindingAuthority, **changes: return GuideSourceBindingRequest(**values) +def _materialization_request( + ids: dict[str, UUID], + *, + binding_id: UUID, + authority: _AllowReadAuthority, + **changes: Any, +) -> GuideSourceMaterializationRequest: + values = { + "prepared_authorization": authority.handle, + "project_id": ids["project"], + "guide_id": ids["guide"], + "guide_source_snapshot_id": ids["snapshot"], + "source_item_id": ids["item"], + "project_setup_run_id": ids["run"], + "setup_generation": 1, + "binding_id": binding_id, + } + values.update(changes) + return GuideSourceMaterializationRequest(**values) + + +async def _create_binding(factory, ids: dict[str, UUID]) -> UUID: + authority = _AllowBindingAuthority() + async with factory() as session, session.begin(): + result = await GuideSourceBindingService(session, authority).bind_guide_source( + _request(ids, authority) + ) + return result.binding_id + + +@pytest.mark.asyncio +async def test_materialization_denies_before_provider_read( + isolated_database_env: str, tmp_path: Path +) -> None: + payload = b"%PDF-1.7\nverified" + digest = "sha256:" + hashlib.sha256(payload).hexdigest() + engine = create_async_engine(isolated_database_env) + factory = async_sessionmaker(engine, expire_on_commit=False) + preparation, scratch = _preparation(tmp_path) + store = _ReadStore(payload) + try: + async with factory() as session: + ids = await _seed_binding_lineage( + session, sha256=digest, byte_count=len(payload), media_type="application/pdf" + ) + binding_id = await _create_binding(factory, ids) + authority = _AllowReadAuthority() + service = ArtifactMaterializationService( + factory, + store, # type: ignore[arg-type] + preparation, + GuideFormatDetector(GuideFormatLimits()), + _namespace(), + ) + + with pytest.raises(ArtifactAuthorityDeniedError, match="unavailable"): + await service.materialize_guide_source( + _materialization_request(ids, binding_id=binding_id, authority=authority) + ) + + assert store.open_count == 0 + async with factory() as session: + assert await session.scalar(select(func.count(GuideSourceFormatClassification.id))) == 0 + finally: + scratch.close() + await engine.dispose() + + +@pytest.mark.asyncio +async def test_materialization_verifies_classifies_replays_and_cleans_scratch( + isolated_database_env: str, tmp_path: Path +) -> None: + payload = b"%PDF-1.7\nverified" + digest = "sha256:" + hashlib.sha256(payload).hexdigest() + engine = create_async_engine(isolated_database_env) + factory = async_sessionmaker(engine, expire_on_commit=False) + preparation, scratch = _preparation(tmp_path) + store = _ReadStore(payload) + authority = _AllowReadAuthority() + try: + async with factory() as session: + ids = await _seed_binding_lineage( + session, sha256=digest, byte_count=len(payload), media_type="application/pdf" + ) + binding_id = await _create_binding(factory, ids) + service = ArtifactMaterializationService( + factory, + store, # type: ignore[arg-type] + preparation, + GuideFormatDetector(GuideFormatLimits()), + _namespace(), + authority_factory=lambda _: authority, + ) + request = _materialization_request(ids, binding_id=binding_id, authority=authority) + + first = await service.materialize_guide_source(request) + replay = await service.materialize_guide_source(request) + + assert (first.detected_format, first.status, first.replayed) == ( + "pdf", + "classified", + False, + ) + assert replay.classification_id == first.classification_id + assert replay.replayed + assert store.open_count == 2 + assert authority.facts[0].namespace_fingerprint == "sha256:" + "b" * 64 + assert authority.facts[0].verification_generation == 0 + assert (await scratch.usage()).reservation_count == 0 + assert list((tmp_path / "scratch" / "files").iterdir()) == [] + async with factory() as session: + assert await session.scalar(select(func.count(GuideSourceFormatClassification.id))) == 1 + finally: + scratch.close() + await engine.dispose() + + +@pytest.mark.asyncio +async def test_truncated_materialization_records_incident_without_classification( + isolated_database_env: str, tmp_path: Path +) -> None: + expected = b"%PDF-1.7\ncomplete" + digest = "sha256:" + hashlib.sha256(expected).hexdigest() + engine = create_async_engine(isolated_database_env) + factory = async_sessionmaker(engine, expire_on_commit=False) + preparation, scratch = _preparation(tmp_path) + store = _ReadStore(expected[:-4]) + authority = _AllowReadAuthority() + try: + async with factory() as session: + ids = await _seed_binding_lineage( + session, sha256=digest, byte_count=len(expected), media_type="application/pdf" + ) + binding_id = await _create_binding(factory, ids) + service = ArtifactMaterializationService( + factory, + store, # type: ignore[arg-type] + preparation, + GuideFormatDetector(GuideFormatLimits()), + _namespace(), + authority_factory=lambda _: authority, + ) + + with pytest.raises(GuideSourceMaterializationError, match="incident"): + await service.materialize_guide_source( + _materialization_request(ids, binding_id=binding_id, authority=authority) + ) + + async with factory() as session: + incident = await session.scalar(select(GuideSourceArtifactIncident)) + assert incident is not None + assert incident.code == "truncated" + assert incident.observed_byte_count == len(expected) - 4 + assert await session.scalar(select(func.count(GuideSourceFormatClassification.id))) == 0 + assert (await scratch.usage()).reservation_count == 0 + finally: + scratch.close() + await engine.dispose() + + +@pytest.mark.asyncio +async def test_same_size_changed_materialization_records_incident( + isolated_database_env: str, tmp_path: Path +) -> None: + expected = b"%PDF-1.7\nexpected" + observed = b"%PDF-1.7\nobserved" + assert len(expected) == len(observed) + digest = "sha256:" + hashlib.sha256(expected).hexdigest() + engine = create_async_engine(isolated_database_env) + factory = async_sessionmaker(engine, expire_on_commit=False) + preparation, scratch = _preparation(tmp_path) + authority = _AllowReadAuthority() + try: + async with factory() as session: + ids = await _seed_binding_lineage( + session, sha256=digest, byte_count=len(expected), media_type="application/pdf" + ) + binding_id = await _create_binding(factory, ids) + service = ArtifactMaterializationService( + factory, + _ReadStore(observed), # type: ignore[arg-type] + preparation, + GuideFormatDetector(GuideFormatLimits()), + _namespace(), + authority_factory=lambda _: authority, + ) + + with pytest.raises(GuideSourceMaterializationError, match="incident"): + await service.materialize_guide_source( + _materialization_request(ids, binding_id=binding_id, authority=authority) + ) + + async with factory() as session: + incident = await session.scalar(select(GuideSourceArtifactIncident)) + assert incident is not None + assert incident.code == "changed" + assert incident.observed_byte_count == len(observed) + assert incident.observed_sha256 == "sha256:" + hashlib.sha256(observed).hexdigest() + assert await session.scalar(select(func.count(GuideSourceFormatClassification.id))) == 0 + finally: + scratch.close() + await engine.dispose() + + +@pytest.mark.asyncio +async def test_post_read_lineage_drift_records_stale_incident( + isolated_database_env: str, tmp_path: Path +) -> None: + payload = b"%PDF-1.7\nverified" + digest = "sha256:" + hashlib.sha256(payload).hexdigest() + engine = create_async_engine(isolated_database_env) + factory = async_sessionmaker(engine, expire_on_commit=False) + preparation, scratch = _preparation(tmp_path) + authority = _AllowReadAuthority() + try: + async with factory() as session: + ids = await _seed_binding_lineage( + session, sha256=digest, byte_count=len(payload), media_type="application/pdf" + ) + binding_id = await _create_binding(factory, ids) + + async def advance_setup_generation() -> None: + async with factory() as session, session.begin(): + session.add( + ProjectSetupRun( + id=str(uuid4()), + project_id=str(ids["project"]), + guide_id=str(ids["guide"]), + guide_version="v1", + source_snapshot_id=str(ids["snapshot"]), + source_snapshot_hash=canonical_json_hash( + {"item": str(ids["item"])} + ), + setup_generation=2, + status="queued", + current_step="queued", + created_by="test", + ) + ) + + service = ArtifactMaterializationService( + factory, + _ReadStore(payload, after_read=advance_setup_generation), # type: ignore[arg-type] + preparation, + GuideFormatDetector(GuideFormatLimits()), + _namespace(), + authority_factory=lambda _: authority, + ) + + with pytest.raises(GuideSourceMaterializationError, match="incident"): + await service.materialize_guide_source( + _materialization_request(ids, binding_id=binding_id, authority=authority) + ) + + async with factory() as session: + incident = await session.scalar(select(GuideSourceArtifactIncident)) + assert incident is not None + assert incident.code == "stale" + assert await session.scalar(select(func.count(GuideSourceFormatClassification.id))) == 0 + finally: + scratch.close() + await engine.dispose() + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "changed_field", + [ + "project_id", + "guide_id", + "guide_source_snapshot_id", + "source_item_id", + "project_setup_run_id", + "binding_id", + "setup_generation", + ], +) +async def test_cross_resource_materialization_denies_before_authority_and_provider_read( + isolated_database_env: str, + tmp_path: Path, + changed_field: str, +) -> None: + payload = b"%PDF-1.7\nverified" + digest = "sha256:" + hashlib.sha256(payload).hexdigest() + engine = create_async_engine(isolated_database_env) + factory = async_sessionmaker(engine, expire_on_commit=False) + preparation, scratch = _preparation(tmp_path) + authority = _AllowReadAuthority() + store = _ReadStore(payload) + try: + async with factory() as session: + ids = await _seed_binding_lineage( + session, sha256=digest, byte_count=len(payload), media_type="application/pdf" + ) + binding_id = await _create_binding(factory, ids) + service = ArtifactMaterializationService( + factory, + store, # type: ignore[arg-type] + preparation, + GuideFormatDetector(GuideFormatLimits()), + _namespace(), + authority_factory=lambda _: authority, + ) + + wrong_value: UUID | int = 2 if changed_field == "setup_generation" else uuid4() + request_binding_id = ( + wrong_value if changed_field == "binding_id" else binding_id + ) + request_changes = ( + {} if changed_field == "binding_id" else {changed_field: wrong_value} + ) + with pytest.raises(GuideSourceMaterializationError, match="unavailable"): + await service.materialize_guide_source( + _materialization_request( + ids, + binding_id=request_binding_id, + authority=authority, + **request_changes, + ) + ) + + assert store.open_count == 0 + assert authority.facts == [] + finally: + scratch.close() + await engine.dispose() + + +@pytest.mark.asyncio +async def test_composed_namespace_drift_denies_before_authority_and_provider_read( + isolated_database_env: str, tmp_path: Path +) -> None: + payload = b"%PDF-1.7\nverified" + digest = "sha256:" + hashlib.sha256(payload).hexdigest() + engine = create_async_engine(isolated_database_env) + factory = async_sessionmaker(engine, expire_on_commit=False) + preparation, scratch = _preparation(tmp_path) + authority = _AllowReadAuthority() + store = _ReadStore(payload) + try: + async with factory() as session: + ids = await _seed_binding_lineage( + session, sha256=digest, byte_count=len(payload), media_type="application/pdf" + ) + binding_id = await _create_binding(factory, ids) + drifted = ArtifactStorageNamespaceSpec( + backend="local", + adapter="other", + provider_profile="test", + namespace_descriptor={"root": "test"}, + namespace_fingerprint="sha256:" + "b" * 64, + ) + service = ArtifactMaterializationService( + factory, + store, # type: ignore[arg-type] + preparation, + GuideFormatDetector(GuideFormatLimits()), + drifted, + authority_factory=lambda _: authority, + ) + + with pytest.raises(ArtifactStorageNamespaceError, match="active storage namespace"): + await service.materialize_guide_source( + _materialization_request(ids, binding_id=binding_id, authority=authority) + ) + + assert store.open_count == 0 + assert authority.facts == [] + finally: + scratch.close() + await engine.dispose() + + +@pytest.mark.asyncio +async def test_materialization_cancellation_cleans_scratch_without_effect( + isolated_database_env: str, tmp_path: Path +) -> None: + payload = b"%PDF-1.7\nverified" + digest = "sha256:" + hashlib.sha256(payload).hexdigest() + engine = create_async_engine(isolated_database_env) + factory = async_sessionmaker(engine, expire_on_commit=False) + preparation, scratch = _preparation(tmp_path) + authority = _AllowReadAuthority() + store = _BlockingReadStore(payload) + try: + async with factory() as session: + ids = await _seed_binding_lineage( + session, sha256=digest, byte_count=len(payload), media_type="application/pdf" + ) + binding_id = await _create_binding(factory, ids) + service = ArtifactMaterializationService( + factory, + store, # type: ignore[arg-type] + preparation, + GuideFormatDetector(GuideFormatLimits()), + _namespace(), + authority_factory=lambda _: authority, + ) + task = asyncio.create_task( + service.materialize_guide_source( + _materialization_request(ids, binding_id=binding_id, authority=authority) + ) + ) + await asyncio.wait_for(store.started.wait(), timeout=5) + task.cancel() + + with pytest.raises(asyncio.CancelledError): + await task + + assert (await scratch.usage()).reservation_count == 0 + async with factory() as session: + assert await session.scalar(select(func.count(GuideSourceFormatClassification.id))) == 0 + assert await session.scalar(select(func.count(GuideSourceArtifactIncident.id))) == 0 + finally: + scratch.close() + await engine.dispose() + + +@pytest.mark.asyncio +async def test_materialization_inspection_timeout_cleans_scratch_and_records_incident( + isolated_database_env: str, tmp_path: Path +) -> None: + payload = b"%PDF-1.7\nverified" + digest = "sha256:" + hashlib.sha256(payload).hexdigest() + engine = create_async_engine(isolated_database_env) + factory = async_sessionmaker(engine, expire_on_commit=False) + preparation, scratch = _preparation(tmp_path) + authority = _AllowReadAuthority() + try: + async with factory() as session: + ids = await _seed_binding_lineage( + session, sha256=digest, byte_count=len(payload), media_type="application/pdf" + ) + binding_id = await _create_binding(factory, ids) + async def deterministic_timeout(*_args: Any, **_kwargs: Any) -> None: + raise ArtifactPreparationDeadlineError("artifact preparation deadline exceeded") + + preparation.inspect_prepared_artifact = deterministic_timeout # type: ignore[method-assign] + service = ArtifactMaterializationService( + factory, + _ReadStore(payload), # type: ignore[arg-type] + preparation, + GuideFormatDetector(GuideFormatLimits()), + _namespace(), + authority_factory=lambda _: authority, + ) + + with pytest.raises(GuideSourceMaterializationError, match="incident"): + await service.materialize_guide_source( + _materialization_request(ids, binding_id=binding_id, authority=authority) + ) + + assert (await scratch.usage()).reservation_count == 0 + async with factory() as session: + incident = await session.scalar(select(GuideSourceArtifactIncident)) + assert incident is not None + assert incident.code == "unavailable" + assert await session.scalar(select(func.count(GuideSourceFormatClassification.id))) == 0 + finally: + scratch.close() + await engine.dispose() + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("provider_error", "expected_code"), + [ + (ArtifactObjectMissingError("missing"), "missing"), + (ArtifactStoreUnavailableError("unavailable"), "unavailable"), + ], +) +async def test_provider_failure_records_only_bounded_artifact_incident( + isolated_database_env: str, + tmp_path: Path, + provider_error: Exception, + expected_code: str, +) -> None: + payload = b"%PDF-1.7\nverified" + digest = "sha256:" + hashlib.sha256(payload).hexdigest() + engine = create_async_engine(isolated_database_env) + factory = async_sessionmaker(engine, expire_on_commit=False) + preparation, scratch = _preparation(tmp_path) + store = _FailingReadStore(provider_error) + authority = _AllowReadAuthority() + try: + async with factory() as session: + ids = await _seed_binding_lineage( + session, sha256=digest, byte_count=len(payload), media_type="application/pdf" + ) + binding_id = await _create_binding(factory, ids) + service = ArtifactMaterializationService( + factory, + store, # type: ignore[arg-type] + preparation, + GuideFormatDetector(GuideFormatLimits()), + _namespace(), + authority_factory=lambda _: authority, + ) + + with pytest.raises(GuideSourceMaterializationError, match="incident"): + await service.materialize_guide_source( + _materialization_request(ids, binding_id=binding_id, authority=authority) + ) + + async with factory() as session: + incident = await session.scalar(select(GuideSourceArtifactIncident)) + assert incident is not None + assert incident.code == expected_code + assert incident.observed_sha256 is None + assert incident.bounded_facts == {"verification_generation": 0} + assert store.open_count == 1 + assert (await scratch.usage()).reservation_count == 0 + finally: + scratch.close() + await engine.dispose() + + @pytest.mark.asyncio async def test_binding_is_exact_immutable_and_idempotent(isolated_database_env: str) -> None: engine = create_async_engine(isolated_database_env) @@ -399,6 +1033,96 @@ async def _create_populated_binding(database_url: str) -> None: await engine.dispose() +@pytest.mark.postgres_schema_contract +def test_0040_refuses_populated_classification_downgrade( + isolated_database_env: str, + migration_lock, +) -> None: + config = Config(str(Path(__file__).resolve().parents[1] / "alembic.ini")) + config.set_main_option( + "script_location", + str(Path(__file__).resolve().parents[1] / "alembic"), + ) + asyncio.run(_create_populated_classification(isolated_database_env)) + with migration_lock(), pytest.raises( + RuntimeError, + match="cannot downgrade populated guide materialization evidence", + ): + command.downgrade(config, "0039_guide_source_bindings") + + +async def _create_populated_classification(database_url: str) -> None: + engine = create_async_engine(database_url) + factory = async_sessionmaker(engine, expire_on_commit=False) + try: + async with factory() as session: + ids = await _seed_binding_lineage(session) + binding_id = await _create_binding(factory, ids) + async with factory() as session, session.begin(): + session.add( + GuideSourceFormatClassification( + id=str(uuid4()), + binding_id=str(binding_id), + content_id=str(ids["content"]), + verified_replica_id=str(ids["replica"]), + setup_generation=1, + sha256="sha256:" + "a" * 64, + byte_count=42, + media_type="application/pdf", + detected_format="pdf", + status="classified", + detector_name="workstream.guide_format", + detector_version="1", + classification_facts={}, + ) + ) + finally: + await engine.dispose() + + +@pytest.mark.postgres_schema_contract +def test_0040_refuses_incident_only_downgrade( + isolated_database_env: str, + migration_lock, +) -> None: + config = Config(str(Path(__file__).resolve().parents[1] / "alembic.ini")) + config.set_main_option( + "script_location", + str(Path(__file__).resolve().parents[1] / "alembic"), + ) + asyncio.run(_create_populated_incident(isolated_database_env)) + with migration_lock(), pytest.raises( + RuntimeError, + match="cannot downgrade populated guide materialization evidence", + ): + command.downgrade(config, "0039_guide_source_bindings") + + +async def _create_populated_incident(database_url: str) -> None: + engine = create_async_engine(database_url) + factory = async_sessionmaker(engine, expire_on_commit=False) + try: + async with factory() as session: + ids = await _seed_binding_lineage(session) + binding_id = await _create_binding(factory, ids) + async with factory() as session, session.begin(): + session.add( + GuideSourceArtifactIncident( + id=str(uuid4()), + binding_id=str(binding_id), + content_id=str(ids["content"]), + verified_replica_id=str(ids["replica"]), + setup_generation=1, + code="missing", + observed_sha256=None, + observed_byte_count=None, + bounded_facts={"verification_generation": 0}, + ) + ) + finally: + await engine.dispose() + + @pytest.mark.asyncio @pytest.mark.parametrize( "failure", diff --git a/backend/tests/test_guide_formats.py b/backend/tests/test_guide_formats.py new file mode 100644 index 000000000..9cbe1d83a --- /dev/null +++ b/backend/tests/test_guide_formats.py @@ -0,0 +1,349 @@ +"""Deterministic bounded guide-format classification.""" + +from __future__ import annotations + +from io import BytesIO +import stat +import zipfile + +import pytest + +from app.modules.artifacts.guide_formats import GuideFormatDetector, GuideFormatLimits + + +@pytest.fixture +def detector() -> GuideFormatDetector: + return GuideFormatDetector(GuideFormatLimits()) + + +def _zip(entries: dict[str, bytes], *, compression: int = zipfile.ZIP_STORED) -> BytesIO: + output = BytesIO() + with zipfile.ZipFile(output, "w", compression=compression) as archive: + for name, value in entries.items(): + archive.writestr(name, value) + output.seek(0) + return output + + +@pytest.mark.parametrize( + ("entries", "expected"), + [ + ( + { + "[Content_Types].xml": b"types", + "_rels/.rels": b"", + "word/document.xml": b"document", + }, + "docx", + ), + ( + { + "[Content_Types].xml": b"types", + "_rels/.rels": b"", + "ppt/presentation.xml": b"presentation", + }, + "pptx", + ), + ( + { + "[Content_Types].xml": b"types", + "_rels/.rels": b"", + "xl/workbook.xml": b"workbook", + }, + "xlsx", + ), + ], +) +def test_classifies_exact_ooxml_markers( + detector: GuideFormatDetector, + entries: dict[str, bytes], + expected: str, +) -> None: + result = detector.detect(_zip(entries), declared_media_type="application/octet-stream") + + assert (result.status, result.detected_format) == ("classified", expected) + + +def test_ordinary_zip_is_unsupported_not_docx(detector: GuideFormatDetector) -> None: + result = detector.detect( + _zip({"document.txt": b"hello"}), declared_media_type="application/zip" + ) + + assert (result.status, result.detected_format) == ("unsupported", "zip") + + +def test_ambiguous_ooxml_markers_are_not_classified(detector: GuideFormatDetector) -> None: + result = detector.detect( + _zip( + { + "[Content_Types].xml": b"types", + "_rels/.rels": b"", + "word/document.xml": b"document", + "xl/workbook.xml": b"workbook", + } + ), + declared_media_type="application/octet-stream", + ) + + assert (result.status, result.detected_format) == ("ambiguous", "zip") + + +def test_malformed_central_directory_is_rejected(detector: GuideFormatDetector) -> None: + result = detector.detect(BytesIO(b"PK\x03\x04truncated"), declared_media_type="application/zip") + + assert (result.status, result.detected_format) == ("malformed", "zip") + + +@pytest.mark.parametrize( + "entries", + [ + {"../escape.txt": b"no"}, + {"word/vbaProject.bin": b"macro"}, + {"_rels/.rels": b''}, + {"_rels/.rels": b''}, + {"word/embeddings/object.bin": b"embedded"}, + {"payload.exe": b"executable"}, + ], +) +def test_dangerous_container_is_malformed( + detector: GuideFormatDetector, entries: dict[str, bytes] +) -> None: + result = detector.detect(_zip(entries), declared_media_type="application/zip") + + assert (result.status, result.detected_format) == ("malformed", "zip") + + +def test_compression_bomb_ratio_is_bounded(detector: GuideFormatDetector) -> None: + result = detector.detect( + _zip({"large.txt": b"x" * 100_000}, compression=zipfile.ZIP_DEFLATED), + declared_media_type="application/zip", + ) + + assert result.status == "limit_exceeded" + + +def test_nested_zip_is_inspected(detector: GuideFormatDetector) -> None: + nested = _zip({"../escape.txt": b"no"}).getvalue() + result = detector.detect(_zip({"nested.zip": nested}), declared_media_type="application/zip") + + assert result.status == "malformed" + + +@pytest.mark.parametrize( + ("payload", "media_type", "expected"), + [ + (b"%PDF-1.7\n", "application/octet-stream", "pdf"), + (b'{"answer": 42}', "application/json", "json"), + (b"a,b\n1,2\n", "text/csv", "csv"), + (b"# Guide\n", "text/markdown", "markdown"), + (b"Guide text\n", "text/plain", "plain_text"), + ], +) +def test_classifies_non_container_formats( + detector: GuideFormatDetector, + payload: bytes, + media_type: str, + expected: str, +) -> None: + result = detector.detect(BytesIO(payload), declared_media_type=media_type) + + assert (result.status, result.detected_format) == ("classified", expected) + + +def test_declared_text_with_invalid_utf8_is_malformed(detector: GuideFormatDetector) -> None: + result = detector.detect(BytesIO(b"\xff\xfe"), declared_media_type="text/plain") + + assert (result.status, result.detected_format) == ("malformed", "plain_text") + + +def test_approved_text_adapter_classifies_octet_stream(detector: GuideFormatDetector) -> None: + result = detector.detect( + BytesIO(b'{"guide": true}'), + declared_media_type="application/octet-stream", + ingestion_adapter="json", + ) + + assert (result.status, result.detected_format) == ("classified", "json") + + +def test_audio_video_and_opaque_binary_are_unsupported(detector: GuideFormatDetector) -> None: + audio = detector.detect(BytesIO(b"ID3\x00"), declared_media_type="audio/mpeg") + signed_audio = detector.detect( + BytesIO(b"OggS" + b"\x00" * 20), declared_media_type="application/octet-stream" + ) + declared_video = detector.detect(BytesIO(b"video"), declared_media_type="video/mp4") + signed_video = detector.detect( + BytesIO(b"\x00\x00\x00\x18ftypisom"), + declared_media_type="application/octet-stream", + ) + opaque = detector.detect( + BytesIO(b"\x00\xff\x00"), declared_media_type="application/octet-stream" + ) + + assert (audio.status, audio.detected_format) == ("unsupported", "audio_video") + assert (signed_audio.status, signed_audio.detected_format) == ("unsupported", "audio_video") + assert (declared_video.status, declared_video.detected_format) == ( + "unsupported", + "audio_video", + ) + assert (signed_video.status, signed_video.detected_format) == ("unsupported", "audio_video") + assert (opaque.status, opaque.detected_format) == ("unsupported", "opaque") + + +def test_image_dimension_limit_is_enforced(detector: GuideFormatDetector) -> None: + png = b"\x89PNG\r\n\x1a\n" + b"\x00" * 8 + (20_000).to_bytes(4, "big") + (2).to_bytes(4, "big") + + result = detector.detect(BytesIO(png), declared_media_type="image/png") + + assert result.status == "limit_exceeded" + assert result.facts == {"width": 20_000, "height": 2} + + +def test_entry_and_decompressed_byte_boundaries_are_exact() -> None: + entries = {f"item-{index}.txt": b"x" for index in range(3)} + at_entry_limit = GuideFormatDetector(GuideFormatLimits(maximum_entries=3)).detect( + _zip(entries), declared_media_type="application/zip" + ) + over_entry_limit = GuideFormatDetector(GuideFormatLimits(maximum_entries=2)).detect( + _zip(entries), declared_media_type="application/zip" + ) + at_byte_limit = GuideFormatDetector(GuideFormatLimits(maximum_decompressed_bytes=3)).detect( + _zip(entries), declared_media_type="application/zip" + ) + over_byte_limit = GuideFormatDetector(GuideFormatLimits(maximum_decompressed_bytes=2)).detect( + _zip(entries), declared_media_type="application/zip" + ) + + assert at_entry_limit.status == "unsupported" + assert at_byte_limit.status == "unsupported" + assert over_entry_limit.status == "limit_exceeded" + assert over_byte_limit.status == "limit_exceeded" + + +def test_nested_depth_boundary_is_exact() -> None: + inner = _zip({"guide.txt": b"guide"}).getvalue() + outer = _zip({"nested.zip": inner}) + + at_limit = GuideFormatDetector(GuideFormatLimits(maximum_nesting_depth=1)).detect( + outer, declared_media_type="application/zip" + ) + outer.seek(0) + over_limit = GuideFormatDetector(GuideFormatLimits(maximum_nesting_depth=0)).detect( + outer, declared_media_type="application/zip" + ) + + assert at_limit.status == "unsupported" + assert over_limit.status == "limit_exceeded" + + +def test_symlink_entry_is_rejected(detector: GuideFormatDetector) -> None: + output = BytesIO() + with zipfile.ZipFile(output, "w") as archive: + link = zipfile.ZipInfo("link") + link.create_system = 3 + link.external_attr = (stat.S_IFLNK | 0o777) << 16 + archive.writestr(link, "target") + output.seek(0) + + result = detector.detect(output, declared_media_type="application/zip") + + assert result.status == "malformed" + + +def test_image_pixel_boundary_is_exact() -> None: + def png(width: int, height: int) -> bytes: + return ( + b"\x89PNG\r\n\x1a\n" + + b"\x00" * 8 + + width.to_bytes(4, "big") + + height.to_bytes(4, "big") + ) + + detector = GuideFormatDetector( + GuideFormatLimits(maximum_image_pixels=100, maximum_image_dimension=100) + ) + + assert ( + detector.detect(BytesIO(png(10, 10)), declared_media_type="image/png").status + == "classified" + ) + assert ( + detector.detect(BytesIO(png(10, 11)), declared_media_type="image/png").status + == "limit_exceeded" + ) + + +def test_fixed_v01_limits_accept_exact_values_and_reject_one_over() -> None: + limits = GuideFormatLimits() + + assert not limits.archive_totals_exceeded( + entry_count=2_000, + decompressed_bytes=128 * 1024 * 1024, + ) + assert limits.archive_totals_exceeded( + entry_count=2_001, + decompressed_bytes=128 * 1024 * 1024, + ) + assert limits.archive_totals_exceeded( + entry_count=2_000, + decompressed_bytes=128 * 1024 * 1024 + 1, + ) + assert not limits.compression_ratio_exceeded(file_size=10_000, compressed_size=100) + assert limits.compression_ratio_exceeded(file_size=10_001, compressed_size=100) + + +def test_fixed_image_boundaries_are_exact() -> None: + def png(width: int, height: int) -> BytesIO: + return BytesIO( + b"\x89PNG\r\n\x1a\n" + + b"\x00" * 8 + + width.to_bytes(4, "big") + + height.to_bytes(4, "big") + ) + + detector = GuideFormatDetector(GuideFormatLimits()) + + assert detector.detect(png(16_384, 1), declared_media_type="image/png").status == "classified" + assert detector.detect(png(16_385, 1), declared_media_type="image/png").status == "limit_exceeded" + assert detector.detect(png(8_000, 5_000), declared_media_type="image/png").status == "classified" + assert detector.detect(png(8_000, 5_001), declared_media_type="image/png").status == "limit_exceeded" + + +@pytest.mark.parametrize( + "payload", + [ + b"RIFF\x00\x00\x00\x00WEBPVP8 " + b"\x00" * 7 + b"\x9d\x01\x2a\x02\x00\x03\x00", + b"RIFF\x00\x00\x00\x00WEBPVP8L" + b"\x00" * 4 + b"\x2f\x01\x08\x00\x00" + b"\x00" * 5, + ], +) +def test_common_webp_variants_expose_dimensions( + detector: GuideFormatDetector, payload: bytes +) -> None: + result = detector.detect(BytesIO(payload), declared_media_type="image/webp") + + assert result.detected_format == "webp" + assert result.status == "classified" + + +def test_jpeg_dimensions_classify_and_enforce_limit() -> None: + def jpeg(width: int, height: int) -> BytesIO: + return BytesIO( + b"\xff\xd8\xff\xc0\x00\x0b\x08" + + height.to_bytes(2, "big") + + width.to_bytes(2, "big") + + b"\x00" * 8 + ) + + detector = GuideFormatDetector( + GuideFormatLimits(maximum_image_pixels=100, maximum_image_dimension=100) + ) + + classified = detector.detect(jpeg(10, 10), declared_media_type="image/jpeg") + over_limit = detector.detect(jpeg(10, 11), declared_media_type="image/jpeg") + + assert (classified.status, classified.detected_format, classified.facts) == ( + "classified", + "jpeg", + {"width": 10, "height": 10}, + ) + assert over_limit.status == "limit_exceeded" diff --git a/docs/spec_artifact_storage_service.md b/docs/spec_artifact_storage_service.md index 6e90275e3..782205e8f 100644 --- a/docs/spec_artifact_storage_service.md +++ b/docs/spec_artifact_storage_service.md @@ -1229,6 +1229,18 @@ through `ArtifactStore` into `ArtifactScratchManager`, and recomputes complete SHA-256 and byte count. Missing, changed, truncated, or unavailable content is an artifact incident and never a guide-insufficiency result. +The hidden v0.1 reader authorizes and locks the exact project, draft guide, +source snapshot/item, setup run/generation, binding, content, verified replica, +storage-namespace fingerprint, and terminal verification receipt/generation +before opening the provider object. It releases database locks during the full +provider read, then relocks and recomposes the same facts before persisting one +immutable format classification. Any drift records only a bounded ART incident. +Classification records contain the server-observed digest, size, canonical +media type, detector identity/version, detected format, outcome, and bounded +structural facts; they contain no source filenames, provider references, raw +document text, or parser exception strings. `artifact.guide_source.read` +remains unavailable until AUTH-04B installs the fixed guide-reader adapter. + Typed extractors run in a bounded no-network isolation boundary. One immutable content-derived record identifies original content, format, extractor/version, policy version, output digest, omission facts, status, and error code; a @@ -1337,6 +1349,14 @@ Implementation is a clean cut: constraints, and creates immutable guide-source bindings. Downgrade is permitted only while the binding table is empty; populated binding evidence is never discarded. +- migration `0040_guide_materialization` adds immutable exact-binding format + classifications and bounded ART custody incidents. Composite foreign keys + prevent either table from naming different binding/content/replica/generation + facts, and closed status/code checks reject unknown outcomes. Downgrade locks + both tables and refuses while either contains evidence. This hidden chunk + adds no Operator route or generic artifact-read API; future authorized + operational visibility must project these bounded records without exposing + provider references or document content. Every migration proves fresh upgrade, prior-head upgrade, populated-state preservation or explicit refusal, empty downgrade/re-upgrade, and no artifact From 1381d3712272a4264ac518f2e7f7fe310ae0da1e Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Wed, 29 Jul 2026 14:40:38 +0100 Subject: [PATCH 2/3] fix(artifacts): address guide classification review --- .../REVIEW_LOG.md | 6 +++ ...B2-guide-materialization-classification.md | 2 +- ...S-ART-001-03B2-external-review-response.md | 39 +++++++++++++++ .../WS-ART-001-03B2-pr-trust-bundle.md | 15 +++--- ...ation.py => 0040_guide_materialization.py} | 0 backend/app/interfaces/artifact_operations.py | 2 +- .../app/modules/artifacts/guide_formats.py | 6 +++ backend/app/modules/artifacts/sources.py | 5 +- backend/tests/test_guide_bindings.py | 49 +++++++++++++++++++ backend/tests/test_guide_formats.py | 25 +++++++++- 10 files changed, 137 insertions(+), 12 deletions(-) create mode 100644 .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-external-review-response.md rename backend/alembic/versions/{0040_guide_materialization_classification.py => 0040_guide_materialization.py} (100%) 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 856776819..04ff8c85c 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 @@ -197,3 +197,9 @@ - Final architecture, security, QA, product/ops, CI-integrity, docs, reuse/dedup, test-delta, and senior-engineering reviews pass. Exact hosted PR-head checks remain the publication gate. +- CodeRabbit's six low-severity comments were repaired with naming and wording + alignment, protocol variance, bounded nested buffering, JPEG marker handling, + and immutable-conflict proof. Its generic docstring warning is superseded by + the passing repository-owned hosted docstring gate. +- Senior-engineering, security, and QA repair-delta re-reviews pass with no + blockers before the repaired PR head is published. diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B2-guide-materialization-classification.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B2-guide-materialization-classification.md index 5a201983a..c18bd983b 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B2-guide-materialization-classification.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B2-guide-materialization-classification.md @@ -47,7 +47,7 @@ scratch without extracting semantic content. either image dimension; every exact-boundary case succeeds, every one-over case returns `limit_exceeded`, and no partial classification survives; - cleanup occurs on success, denial, mismatch, cancellation, and timeout; -- migration `0040_guide_materialization_classification` preserves exact-binding classification +- migration `0040_guide_materialization` preserves exact-binding classification and incident custody, refuses populated downgrade, and introduces no new Operator or generic artifact-read route; - changed subsystems remain at least 90% covered and repository coverage stays diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-external-review-response.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-external-review-response.md new file mode 100644 index 000000000..5092c9ad6 --- /dev/null +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-external-review-response.md @@ -0,0 +1,39 @@ +# WS-ART-001-03B2 External Review Response + +## Comments addressed + +- Aligned migration filename and documented revision identity. +- Corrected the canonical materialization-port description. +- Made the return-only prepared-inspector type covariant. +- Added deterministic JPEG standalone-marker handling and proof. +- Added a fixed per-entry nested-archive byte ceiling and exact boundary proof + before any nested member is buffered. +- Added database-backed proof that conflicting immutable classification evidence + is rejected without replacing the stored row. + +## Comments deferred + +- None. + +## Human decisions needed + +- None. All code comments were low-severity and in scope. +- CodeRabbit's generic docstring-coverage warning is not actionable: the hosted + repository docstring gate passed on the reviewed head and remains the + canonical configured check. + +## Commands rerun + +- Focused Ruff over changed backend files. +- Guide-format and preparation focused tests. +- Database-backed guide-materialization focused tests. +- Repository stale-contract, Markdown-link, lightweight-agent-gate, and diff + checks. +- Hosted Backend and Agent Gates are required on the repaired exact PR head. + +## Remaining risks + +- Format classification remains syntactic by design; semantic extraction belongs + to WS-ART-001-03B3A. +- Live guide-reader authorization remains planned and unavailable until + AUTH WS-XINT-002-04B. diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-pr-trust-bundle.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-pr-trust-bundle.md index 1bdbc4f49..0e783bb60 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-pr-trust-bundle.md @@ -71,10 +71,11 @@ and Agent Gates remain required on the exact PR head. ## Reviewer Results And External Review -Architecture, security, QA, product/ops, CI integrity, docs, reuse, and test -delta pass after valid findings were repaired. Senior-engineering confirmation -is recorded in the paired evidence before PR publication. CodeRabbit and GitHub -checks are pending until the PR exists and supplement these internal reviews. +Architecture, security, QA, product/ops, CI integrity, docs, reuse, test delta, +and senior engineering pass after valid findings were repaired. CodeRabbit's +six low-severity comments were addressed; its generic docstring warning is +superseded by the repository-owned hosted docstring gate. GitHub Backend and +Agent Gates remain required on each repaired exact PR head. ## Remaining Risks And Follow-Up @@ -88,5 +89,7 @@ and guide-reader actions. 03C remains a separate legacy cutover. Review the active namespace fence, pre/post setup-generation locking, complete rehash comparison, classification precedence/limits, incident privacy, and the -deny-only AUTH boundary. The user retains the decision to mark the PR ready and -approve its merge. +deny-only AUTH boundary. Confirm the repaired nested-archive ceiling is enforced +before buffering and immutable classification conflicts cannot overwrite +evidence. The user retains the decision to mark the PR ready and approve its +merge. diff --git a/backend/alembic/versions/0040_guide_materialization_classification.py b/backend/alembic/versions/0040_guide_materialization.py similarity index 100% rename from backend/alembic/versions/0040_guide_materialization_classification.py rename to backend/alembic/versions/0040_guide_materialization.py diff --git a/backend/app/interfaces/artifact_operations.py b/backend/app/interfaces/artifact_operations.py index 6d5925235..45907421a 100644 --- a/backend/app/interfaces/artifact_operations.py +++ b/backend/app/interfaces/artifact_operations.py @@ -268,7 +268,7 @@ async def bind_checker_output(self, request: CheckerOutputBindingRequest) -> obj class ArtifactMaterializationPort(Protocol): - """Materialize only the two canonical immutable source forms.""" + """Materialize only canonical immutable source forms.""" async def materialize_prepared_bundle( self, diff --git a/backend/app/modules/artifacts/guide_formats.py b/backend/app/modules/artifacts/guide_formats.py index 646a20bb1..b970df974 100644 --- a/backend/app/modules/artifacts/guide_formats.py +++ b/backend/app/modules/artifacts/guide_formats.py @@ -36,6 +36,7 @@ class GuideFormatLimits: maximum_entries: int = 2_000 maximum_central_directory_bytes: int = 8 * 1024 * 1024 maximum_decompressed_bytes: int = 128 * 1024 * 1024 + maximum_nested_archive_bytes: int = 16 * 1024 * 1024 maximum_nesting_depth: int = 8 maximum_compression_ratio: int = 100 maximum_image_pixels: int = 40_000_000 @@ -246,6 +247,8 @@ def _inspect_archive( ): return "malformed" if lower.endswith(".zip"): + if info.file_size > self._limits.maximum_nested_archive_bytes: + return "limit_exceeded" nested = archive.read(info) nested_result = self._inspect_archive( BytesIO(nested), state=state, depth=depth + 1 @@ -313,6 +316,9 @@ def _image_dimensions(header: bytes) -> tuple[str, int, int] | None: offset += 1 continue marker = header[offset + 1] + if marker in {0x01, 0xD8, 0xD9} or 0xD0 <= marker <= 0xD7: + offset += 2 + continue length = int.from_bytes(header[offset + 2 : offset + 4], "big") if marker in { 0xC0, diff --git a/backend/app/modules/artifacts/sources.py b/backend/app/modules/artifacts/sources.py index da9b1267d..1cd39c444 100644 --- a/backend/app/modules/artifacts/sources.py +++ b/backend/app/modules/artifacts/sources.py @@ -12,12 +12,13 @@ _COMMITTED_SOURCE_SEAL = object() _InspectionResult = TypeVar("_InspectionResult") +_InspectionResultCo = TypeVar("_InspectionResultCo", covariant=True) -class PreparedArtifactInspector(Protocol[_InspectionResult]): +class PreparedArtifactInspector(Protocol[_InspectionResultCo]): """Typed artifact-owned inspection capability over one scratch reader.""" - def inspect(self, reader: BinaryIO) -> _InspectionResult: + def inspect(self, reader: BinaryIO) -> _InspectionResultCo: """Return bounded structural facts without retaining the reader.""" diff --git a/backend/tests/test_guide_bindings.py b/backend/tests/test_guide_bindings.py index 5913105f3..ec1b21976 100644 --- a/backend/tests/test_guide_bindings.py +++ b/backend/tests/test_guide_bindings.py @@ -510,6 +510,55 @@ async def test_materialization_verifies_classifies_replays_and_cleans_scratch( await engine.dispose() +@pytest.mark.asyncio +async def test_materialization_rejects_conflicting_immutable_classification( + isolated_database_env: str, tmp_path: Path +) -> None: + payload = b"%PDF-1.7\nverified" + digest = "sha256:" + hashlib.sha256(payload).hexdigest() + engine = create_async_engine(isolated_database_env) + factory = async_sessionmaker(engine, expire_on_commit=False) + preparation, scratch = _preparation(tmp_path) + store = _ReadStore(payload) + authority = _AllowReadAuthority() + try: + async with factory() as session: + ids = await _seed_binding_lineage( + session, sha256=digest, byte_count=len(payload), media_type="application/pdf" + ) + binding_id = await _create_binding(factory, ids) + service = ArtifactMaterializationService( + factory, + store, # type: ignore[arg-type] + preparation, + GuideFormatDetector(GuideFormatLimits()), + _namespace(), + authority_factory=lambda _: authority, + ) + request = _materialization_request(ids, binding_id=binding_id, authority=authority) + first = await service.materialize_guide_source(request) + async with factory() as session, session.begin(): + persisted = await session.get( + GuideSourceFormatClassification, str(first.classification_id) + ) + assert persisted is not None + persisted.detected_format = "plain_text" + + with pytest.raises(GuideSourceMaterializationError, match="classification conflicts"): + await service.materialize_guide_source(request) + + async with factory() as session: + persisted = await session.get( + GuideSourceFormatClassification, str(first.classification_id) + ) + assert persisted is not None + assert persisted.detected_format == "plain_text" + assert await session.scalar(select(func.count(GuideSourceFormatClassification.id))) == 1 + finally: + scratch.close() + await engine.dispose() + + @pytest.mark.asyncio async def test_truncated_materialization_records_incident_without_classification( isolated_database_env: str, tmp_path: Path diff --git a/backend/tests/test_guide_formats.py b/backend/tests/test_guide_formats.py index 9cbe1d83a..59819f66c 100644 --- a/backend/tests/test_guide_formats.py +++ b/backend/tests/test_guide_formats.py @@ -236,6 +236,20 @@ def test_nested_depth_boundary_is_exact() -> None: assert over_limit.status == "limit_exceeded" +def test_nested_archive_member_byte_boundary_is_exact() -> None: + inner = _zip({"guide.txt": b"guide"}).getvalue() + + at_limit = GuideFormatDetector( + GuideFormatLimits(maximum_nested_archive_bytes=len(inner)) + ).detect(_zip({"nested.zip": inner}), declared_media_type="application/zip") + over_limit = GuideFormatDetector( + GuideFormatLimits(maximum_nested_archive_bytes=len(inner) - 1) + ).detect(_zip({"nested.zip": inner}), declared_media_type="application/zip") + + assert at_limit.status == "unsupported" + assert over_limit.status == "limit_exceeded" + + def test_symlink_entry_is_rejected(detector: GuideFormatDetector) -> None: output = BytesIO() with zipfile.ZipFile(output, "w") as archive: @@ -326,9 +340,11 @@ def test_common_webp_variants_expose_dimensions( def test_jpeg_dimensions_classify_and_enforce_limit() -> None: - def jpeg(width: int, height: int) -> BytesIO: + def jpeg(width: int, height: int, *, standalone_marker: bytes = b"") -> BytesIO: return BytesIO( - b"\xff\xd8\xff\xc0\x00\x0b\x08" + b"\xff\xd8" + + standalone_marker + + b"\xff\xc0\x00\x0b\x08" + height.to_bytes(2, "big") + width.to_bytes(2, "big") + b"\x00" * 8 @@ -339,6 +355,10 @@ def jpeg(width: int, height: int) -> BytesIO: ) classified = detector.detect(jpeg(10, 10), declared_media_type="image/jpeg") + with_restart_marker = detector.detect( + jpeg(10, 10, standalone_marker=b"\xff\xd0"), + declared_media_type="image/jpeg", + ) over_limit = detector.detect(jpeg(10, 11), declared_media_type="image/jpeg") assert (classified.status, classified.detected_format, classified.facts) == ( @@ -346,4 +366,5 @@ def jpeg(width: int, height: int) -> BytesIO: "jpeg", {"width": 10, "height": 10}, ) + assert with_restart_marker.facts == {"width": 10, "height": 10} assert over_limit.status == "limit_exceeded" From 294d453f2a7474ef84d87733686734d878106ff5 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Wed, 29 Jul 2026 18:13:18 +0100 Subject: [PATCH 3/3] docs(artifacts): complete external review evidence --- ...S-ART-001-03B2-external-review-response.md | 7 +- .../WS-ART-001-03B2-pr-trust-bundle.md | 233 ++++++++++++------ 2 files changed, 157 insertions(+), 83 deletions(-) diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-external-review-response.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-external-review-response.md index 5092c9ad6..7fce0102c 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-external-review-response.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-external-review-response.md @@ -25,11 +25,12 @@ ## Commands rerun - Focused Ruff over changed backend files. -- Guide-format and preparation focused tests. -- Database-backed guide-materialization focused tests. +- Focused guide-format and preparation tests. +- Focused database-backed guide-materialization tests. - Repository stale-contract, Markdown-link, lightweight-agent-gate, and diff checks. -- Hosted Backend and Agent Gates are required on the repaired exact PR head. +- Exact PR head `1381d371`: Backend passed in 12m47s and Agent Gates passed in + 19s. A later review-request Agent Gates run also passed in 21s. ## Remaining risks diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-pr-trust-bundle.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-pr-trust-bundle.md index 0e783bb60..60944bb10 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-03B2-pr-trust-bundle.md @@ -1,4 +1,4 @@ -# PR Trust Bundle: WS-ART-001-03B2 +# Workstream PR Trust Bundle ## Chunk @@ -6,90 +6,163 @@ ## Goal -Read the exact verified guide-source object through the fixed guide-reader -boundary, recompute its complete identity in bounded private scratch, persist -only exact syntactic classification or bounded ART incident evidence, and keep -the behavior hidden until AUTH-04B. +Read exact verified guide-source bytes through the fixed guide-reader boundary, +recompute their identity in bounded private scratch, and persist only immutable +syntactic classification or bounded ART incident evidence. -## Human-Approved Intent +## Intent And Planning Context -Project Manager guide items may be PDF, DOCX, PPTX, CSV, XLSX, Markdown, plain -text, JSON, or supported images; they are not contributor submission ZIPs. -03B2 classifies structure only. It does not extract semantics or invoke the -guide-sufficiency agent. Audio/video and ordinary/opaque ZIP content remain -unsupported in v0.1. +- Intent: Project Manager guide items may use the approved document, table, + text, JSON, or image formats; they are not contributor submission ZIPs. +- Chunk contract: `.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-03B2-guide-materialization-classification.md` -## What Changed And Why +## What Changed -- Extended the canonical materialization port and facade with exact guide reads. -- Added shared active namespace/store/replica validation before provider I/O. -- Added a typed scratch-owned inspection capability without exposing paths. -- Added complete rehash/size verification, pre/post lineage revalidation, - immutable format classifications, and bounded custody incidents. -- Added deterministic PDF, OOXML, text-family, image, audio/video, ordinary ZIP, - unsafe-container, and fixed-limit classification. -- Added migration `0040`, reset/fingerprint/lane updates, focused tests, and - operator/deployment documentation. +- Added exact fixed-reader materialization with pre/post lineage validation. +- Added provider-neutral streaming, complete rehash/size verification, and typed + bounded scratch inspection. +- Added deterministic format classification, immutable evidence, bounded + incidents, migration `0040`, tests, documentation, and review evidence. -This keeps the object store responsible only for bytes while Workstream owns -their identity, integrity, lineage, and meaning. +## Why It Changed + +Workstream must prove the exact verified guide bytes before later extraction or +sufficiency processing; object-store presence or caller metadata is insufficient. ## Design Chosen -Transaction A locks and recomposes the exact binding/content/replica/receipt/ -setup facts and consumes prepared read authority. Database locks are released -before the full provider read. The provider-neutral store streams into the -canonical scratch manager, which recomputes digest and size. Transaction B -relocks and recomposes the same facts before persisting one immutable result. - -Alternatives rejected: direct S3/MinIO access, arbitrary temp files, caller -excerpts, parsing during HTTP upload, generic download authority, serialized -prepared handles, raw relationship substring checks, and a second materializer. - -## Scope And Product Behavior - -No route is exposed. `artifact.guide_source.read` remains planned/unavailable, -and positive behavior uses bounded test authority only. There is no extraction, -agent call, Celery continuation, submission/checker/review change, legacy -cutover, or new Operator API. Missing/corrupt/stale bytes are ART incidents, -never guide-insufficiency decisions. - -## Acceptance Proof And Test Delta - -Focused pure tests cover signatures, OOXML marker precedence, unsafe ZIPs, -fixed limit boundaries, image variants, typed inspection, deadline behavior, -and architecture fences. The isolated PostgreSQL runner passed all 13 selected -materialization tests after migrating through `0040`. They prove denial before -I/O, namespace drift, exact rehash, replay, changed/truncated/stale incidents, -cross-resource rejection, cancellation, timeout, and cleanup. Migration tests -cover classification-only and incident-only downgrade refusal. - -No test, assertion, coverage threshold, workflow, or shard was weakened. -`test_guide_formats.py` is assigned to `shared_foundations`. Hosted repository -coverage (78 percent), artifact subsystem coverage (90 percent), Backend shards, -and Agent Gates remain required on the exact PR head. - -## Reviewer Results And External Review - -Architecture, security, QA, product/ops, CI integrity, docs, reuse, test delta, -and senior engineering pass after valid findings were repaired. CodeRabbit's -six low-severity comments were addressed; its generic docstring warning is -superseded by the repository-owned hosted docstring gate. GitHub Backend and -Agent Gates remain required on each repaired exact PR head. - -## Remaining Risks And Follow-Up - -The materializer facade contains only the guide-read slice today; 03B3A and -later chunks must extend it rather than create another path. 03B3A adds bounded -extraction; 03B3B adds remaining approved format extractors; 03B4 adds durable -same-generation continuation. Only then may AUTH-04B activate the fixed binding -and guide-reader actions. 03C remains a separate legacy cutover. - -## Human Review Focus And Merge Ownership - -Review the active namespace fence, pre/post setup-generation locking, complete -rehash comparison, classification precedence/limits, incident privacy, and the -deny-only AUTH boundary. Confirm the repaired nested-archive ceiling is enforced -before buffering and immutable classification conflicts cannot overwrite -evidence. The user retains the decision to mark the PR ready and approve its -merge. +Transaction A locks exact lineage and consumes prepared authority before the +provider read. The canonical store streams into canonical scratch. Transaction B +relocks the same facts before persisting one immutable result. + +## Alternatives Rejected + +- Direct S3/MinIO reads, arbitrary temp files, generic download authority, + serialized prepared handles, caller excerpts, and parsing during upload. + +## Scope Control + +### Allowed Files Changed + +- ART interfaces, models, migration, materialization/classification services, + focused tests, ART specification, and this chunk's evidence. + +### Files Outside Stated Scope + +- None. + +## Product Behavior + +- [x] Product behavior changed and is explained here: hidden fixed-service guide + materialization/classification exists, but `artifact.guide_source.read` + remains planned and unavailable until AUTH-04B. + +## Evidence + +### Commands Run + +```bash +uv run --no-sync ruff check app tests scripts alembic/versions/0040_guide_materialization.py +uv run --no-sync pytest -q tests/test_guide_formats.py tests/test_artifact_preparation.py +WORKSTREAM_TEST_ADMIN_DATABASE_URL= uv run --no-sync python scripts/run_isolated_tests.py +python3 scripts/check_stale_artifact_contracts.py +python3 scripts/check_markdown_links.py +PYTHONPATH=. python3 scripts/test_lightweight_agent_gates.py +git diff --check +``` + +### Result Summary + +```text +Focused guide/preparation tests: 35 passed +Database-backed materialization selection: 13 passed before review repair +Immutable-conflict database proof: 1 passed after repair +Backend exact head 1381d371: passed in 12m47s +Agent Gates exact head 1381d371: passed in 19s +CodeRabbit final-head review: completed; two evidence-wording findings repaired +``` + +## Acceptance Criteria Proof + +- [x] Exact verified lineage and active namespace are checked before provider I/O. +- [x] Complete digest and byte count are recomputed on every read. +- [x] Stale/replaced/missing/mismatched content fails closed with bounded evidence. +- [x] Classification is syntactic, bounded, immutable, and provider-neutral. +- [x] Live AUTH actions remain planned and unavailable. + +## Test Delta + +### Tests Added + +- Format signatures, OOXML/container safety, exact limits, image variants, + typed inspection, materialization lineage, incidents, replay, cancellation, + timeout, conflict, migration, and architecture fences. + +### Tests Modified + +- Alembic head/fingerprint, artifact architecture, preparation, and guide-binding + integration coverage. + +### Tests Removed Or Skipped + +- None. + +## Internal Reviewer Results + +Reviewed code SHA: `1381d371` + +Reviewed at: 2026-07-29 + +Reviewer run IDs: recorded in the paired internal-review evidence. + +| Reviewer | Result | Blocking Findings | Notes | +|---|---:|---|---| +| Senior engineering | PASS WITH LOW RISKS | None | Repair delta also passed | +| QA/test | PASS WITH LOW RISKS | None | Repair delta also passed | +| Security/auth | PASS | None | Repair delta also passed | +| Product/ops | PASS | None | Exact custody preserved | +| Architecture | PASS WITH LOW RISKS | None | Canonical facade/store/scratch reused | +| CI integrity | PASS WITH LOW RISKS | None | No gate weakening | +| Docs | PASS | None | Specification and evidence aligned | +| Reuse/dedup | PASS WITH LOW RISKS | None | No parallel boundary introduced | +| Test delta | PASS WITH LOW RISKS | None | No removed or weakened tests | + +## External Review + +| Source | Status | Notes | +|---|---:|---| +| CodeRabbit | PASS AFTER FIXES | Six initial findings and two final evidence findings repaired | +| GitHub checks | PASS | Backend and Agent Gates green on `1381d371` | + +## CI And Gate Integrity + +- [x] No workflow weakening. +- [x] No lint/test/docstring gate weakening. +- [x] No coverage threshold weakening. +- [x] No package script weakening. +- [x] No unpinned new GitHub Action. +- [x] Checkout credential persistence unchanged. + +## Remaining Risks + +Classification is syntactic by design. Semantic extraction belongs to 03B3A; +live fixed-reader authorization belongs to AUTH-04B. + +## Follow-Up Work + +03B3A adds extraction, 03B3B adds durable continuation, 03B4 finalizes the +hidden manifest, AUTH-04B activates exact fixed-service actions, and 03C later +performs the separate legacy cutover. + +## Human Review Focus + +Please inspect active namespace fencing, pre/post generation locks, complete +rehash comparison, nested-archive bounds, incident privacy, and deny-only AUTH. + +## Human Merge Ownership + +- [x] I can explain what changed. +- [x] I can explain why it changed. +- [x] I know what could break. +- [ ] I accept the remaining risks. +- [ ] The user explicitly approved this specific PR for merge.