From 2f92db9593bb79463097d3a3d62953c0b79e711e Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 4 Aug 2026 12:26:37 +0100 Subject: [PATCH 1/3] feat(artifacts): add bounded outer zip inspection --- .../AUTH_HANDOFF.md | 10 +- .../CHUNK_MAP.md | 4 +- .../STATUS.md | 23 +- .../WS-ART-001-04A2-outer-zip-safety.md | 103 ++++- ...001-PLAN3-v01-end-to-end-reconciliation.md | 2 +- ...S-ART-001-04A2-internal-review-evidence.md | 55 +++ .../WS-ART-001-04A2-pr-trust-bundle.md | 98 ++++ backend/app/adapters/artifacts/__init__.py | 21 + backend/app/core/config.py | 25 + .../app/modules/artifacts/guide_formats.py | 20 +- .../modules/artifacts/submission_archive.py | 430 ++++++++++++++++++ backend/app/modules/artifacts/zip_safety.py | 83 ++++ backend/scripts/run_test_lanes.py | 1 + backend/tests/test_config.py | 33 +- backend/tests/test_submission_archive.py | 412 +++++++++++++++++ docs/spec_artifact_storage_service.md | 26 ++ 16 files changed, 1305 insertions(+), 41 deletions(-) create mode 100644 .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-internal-review-evidence.md create mode 100644 .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-pr-trust-bundle.md create mode 100644 backend/app/modules/artifacts/submission_archive.py create mode 100644 backend/app/modules/artifacts/zip_safety.py create mode 100644 backend/tests/test_submission_archive.py diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/AUTH_HANDOFF.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/AUTH_HANDOFF.md index 590bd8303..db923bac7 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/AUTH_HANDOFF.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/AUTH_HANDOFF.md @@ -13,9 +13,10 @@ activation custody, and availability. ## 2026-08-02 Reconciliation XINT-002-01 already registered the one contributor action and removed the six -obsolete upload-session actions. AUTH-04B has an approved corrected contract, -but its implementation/activation has not merged; guide binding/read remain -planned. No ART status may describe that planning merge as runtime activation. +obsolete upload-session actions. AUTH-04B implementation merged in PR #245; +fixed-service guide binding/read are active, and ART-03C completed the verified +guide-pipeline cutover. Submission actions remain governed by the later split +activation order below. The remaining AUTH order requires one correction before submission work can go live: split XINT-06 into `06A` (pre-submit materializer only, after hidden @@ -25,7 +26,8 @@ from activating while its mandatory fixed materializer still denies. ## Guide Source Sequence -1. Existing guide-source actions remain planned and unavailable. +1. Guide-source ingest and fixed-service binding/read are active through their + merged AUTH chunks; contributor submission actions remain unavailable. 2. ART-03A implements hidden `artifact.guide_source.ingest` behavior and its exact resource/guard/surface manifest. 3. AUTH activates only that exact action through a separately reviewed AUTH diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/CHUNK_MAP.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/CHUNK_MAP.md index e7e48f2ad..8b309166e 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/CHUNK_MAP.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/CHUNK_MAP.md @@ -24,8 +24,8 @@ they cross multiple L1 boundaries. |---|---|---:|---| | `WS-ART-001-PLAN3` | Reconcile the complete remaining v0.1 custody chain and AUTH/REV/CON handoffs. | L1 | Planning only; proposed | | `WS-ART-001-03C` | Clean-cut legacy guide identity/excerpts and make the verified same-generation pipeline live. | L1 | Merged PR #249 | -| `WS-ART-001-04A1` | Remove legacy multi-step contributor intake reachability and schema without adding the replacement route. | L1 | Implemented; internal review passed; external PR gates pending | -| `WS-ART-001-04A2` | Add bounded one-outer-ZIP intake and archive-safety inspection in private scratch. | L1 | Proposed after 04A1 | +| `WS-ART-001-04A1` | Remove legacy multi-step contributor intake reachability and schema without adding the replacement route. | L1 | Merged PR #264 | +| `WS-ART-001-04A2` | Add bounded one-outer-ZIP intake and archive-safety inspection in private scratch. | L1 | Implemented; internal review passed; external PR gates pending | | `WS-ART-001-04A3` | Add canonical semantic manifest, executable normalization, and unchanged-work gate. | L1 | Proposed after 04A2 | | `WS-ART-001-04B` | Run non-bypassable platform and locked-guide prechecks against that exact scratch tree and persist bounded evidence. | L1 | Proposed after 04A3 | | `WS-ART-001-04C1` | Reauthorize and atomically persist capacity plus durable put intent, then write the checked ZIP once. | L1 | Proposed after XINT-06A | diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md index 52ed58649..67f7ac510 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md @@ -66,10 +66,10 @@ normalizes regular-file executable intent into the semantic manifest; and it requires fresh AUTH prepared capabilities at durable put intent and atomic Submission/binding consumption. -## Current Planning Correction +## Completed Guide Pipeline And Current Submission Work -ART-03A and AUTH `WS-XINT-002-04A` are merged. Before ART-03B implementation, -the guide-content boundary is being corrected explicitly: verified binding, +ART-03A and AUTH `WS-XINT-002-04A` are merged. The guide-content boundary was +implemented explicitly as verified binding, full-read materialization, format classification, isolated extraction, canonical extraction provenance, incremental complex-format support, and same-generation sufficiency continuation are separate PR-sized contracts. @@ -77,7 +77,8 @@ same-generation sufficiency continuation are separate PR-sized contracts. material port, all-items-required semantics, deterministic 12 MiB assembly, normalized report-to-extraction provenance, and the hidden pre-submit identifier/generation continuation. AUTH binding/read production activation -remains unmerged; ART-03C remains blocked on that implementation. +merged through PR #245, and ART-03C's verified-pipeline cutover subsequently +merged. After 03B3A merged, the original complex-format chunk was found too broad for one dependency and parser-security review. It is replaced by 03B3B1 dependency @@ -103,12 +104,14 @@ omission facts on the shared OOXML boundary. 03B3B3C merged through PR #235 and adds bounded PPTX slide/notes extraction. 03B3B3D merged through PR #238 and adds bounded XLSX cell extraction. 03B3B4 merged through PR #239 and adds only bounded PNG/JPEG/WebP structural metadata. 03B4 merged through PR #240 and adds -the hidden same-generation sufficiency continuation. AUTH binding/read actions -remain planned. - -AUTH `WS-XINT-002-04B` follows the complete hidden split-03B series and -activates only fixed-service binding and guide read. ART-03C then removes the -legacy identity/excerpt path and makes the verified pipeline authoritative. +the hidden same-generation sufficiency continuation. ART-03C and ART-04A1 are +merged. ART-04A1 merged through PR #264. ART-04A2 is implemented on its +bounded branch with internal L1 reviews passed; hosted PR gates and human merge +remain pending. + +AUTH `WS-XINT-002-04B` activated only fixed-service binding and guide read. +ART-03C removed the legacy identity/excerpt path and made the verified pipeline +authoritative. ## Gate diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-04A2-outer-zip-safety.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-04A2-outer-zip-safety.md index 26d8c4520..8af01b5bb 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-04A2-outer-zip-safety.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-04A2-outer-zip-safety.md @@ -1,6 +1,6 @@ # Chunk Contract: WS-ART-001-04A2 — Bounded Outer-ZIP Safety -Parent initiative: `WS-ART-001` | Risk: L1 | Status: Proposed after 04A1 +Parent initiative: `WS-ART-001` | Risk: L1 | Status: Implemented; internal review passed ## Goal @@ -9,13 +9,57 @@ complete file/directory tree without provider I/O or a public route. ## Allowed Files -ART scratch intake/archive safety capability, bounded configuration use, -adversarial ZIP tests/fuzz fixtures, docs, and scoped coverage evidence. +- `backend/app/modules/artifacts/submission_archive.py`: dedicated contributor + outer-ZIP inspector and closed process-local result/failure types; +- `backend/app/modules/artifacts/zip_safety.py`: neutral bounded ZIP-directory + facts shared by guide and submission consumers; +- `backend/app/modules/artifacts/guide_formats.py`: import the moved neutral + `zip_directory_facts()` probe only, without changing guide classification; +- `backend/app/core/config.py` and + `backend/app/adapters/artifacts/__init__.py`: conservative ZIP-safety settings + and composition only; +- `backend/tests/test_submission_archive.py`, `backend/tests/test_config.py`, + and narrowly necessary + additions to `backend/tests/test_artifact_preparation.py`: adversarial + inspection, fixed configuration, and scratch cleanup proof; +- `backend/scripts/run_test_lanes.py` and + `backend/tests/test_ci_test_lanes.py`: exact semantic-lane custody for the + two new focused modules only; +- this contract, the PLAN3 focused-test mapping, the ART status/chunk map, + artifact-storage specification, and + scoped review/coverage evidence; `AUTH_HANDOFF.md` only for current merged + dependency wording. ## Not Allowed Changes Semantic manifest/change comparison, project checker, durable admission, provider I/O, nested archive extraction, larger limits, or AUTH activation. +Do not reuse guide-format archive recursion: contributor backslashes are +rejected before normalization and nested ZIP members remain opaque regular +files. Do not add multipart/request parsing in this hidden chunk. + +## Internal Handoff And Limits + +04A2 must use the existing `PreparedArtifact.inspect(...)` / +`PreparedArtifactInspector` seam. It returns one immutable, non-durable +`SubmissionArchiveInspectionResult` containing only bounded +`SubmissionArchiveEntry` structural facts (normalized POSIX path, closed entry +type, actual byte count, and bounded archive totals) plus closed redacted +failure codes. It must never expose or retain `ZipInfo`, a reader, scratch path +or handle, raw bytes, provider facts, prepared authorization, semantic hashes, +file hashes, executable normalization, or durable identity. + +Neutral bounded EOCD/ZIP64/multi-disk directory probing already present as +`zip_directory_facts()` must be reused or moved to a neutral ART module rather +than copied. The guide detector itself must not be reused because its recursive +nested-archive and backslash-normalization semantics are intentionally +different. + +The implementation owns conservative startup-fixed limits for maximum entry +count, normalized path bytes/depth, central-directory bytes, actual bytes per +entry, actual total expanded bytes, compression ratio, and inspection time. +Defaults may not raise the existing 512 MiB source ceiling. Limit configuration +is validated once at startup and is identical for processes sharing scratch. ## Acceptance Criteria @@ -24,10 +68,59 @@ symlink/special/encrypted/malformed entries, duplicates/NFC/case-fold collisions, bombs, and every configured limit breach; nested ZIPs stay opaque; all outcomes clean scratch and disclose no path/handle. +Path validation also rejects empty, `.` and `..` segments, drive/root forms, +ambiguous trailing dot/space segments, file/directory normalized collisions, +and ancestry conflicts such as a regular file `a` plus `a/b`. Implicit +directories are derived deterministically without becoming duplicate entries. + +Every regular-file member is fully bounded-read during inspection. Actual reads, +not central-directory declarations, enforce per-entry/aggregate byte and ratio +limits and verify CRC, truncation, local-header consistency, and malformed data +descriptors. Multi-disk/spanned archives and unsupported ZIP64 structures are +rejected; supported bounded ZIP64 metadata must not weaken any limit. ZIP +metadata is never used to create filesystem entries in this chunk. + +Cancellation, timeout, malformed input, and every rejection must close the +archive reader and release the enclosing scratch preparation. Stable internal +failure codes distinguish invalid outer ZIP, unsafe path/type, collision, +encryption, malformed/truncated content, bomb/limit breach, and timeout without +including submitted paths or parser/provider details. +v0.1 accepts stored and raw-DEFLATE members only; other compression methods +fail closed and are not silently delegated to provider or checker code. + ## Verification Commands -Focused archive/scratch/fuzz tests, Ruff, stale scans, hosted gates, 90% owned -subsystem and 78% repository coverage. +```bash +(cd backend && .venv/bin/python -m ruff check app tests scripts) +(cd backend && .venv/bin/python -m pytest -q \ + tests/test_submission_archive.py tests/test_config.py tests/test_guide_formats.py) +(cd backend && .venv/bin/python -m pytest -q tests/test_ci_test_lanes.py) +(cd backend && .venv/bin/python -m pytest \ + tests/test_submission_archive.py \ + --cov=app.modules.artifacts.submission_archive --cov-report=term-missing \ + --cov-fail-under=90) +(cd backend && .venv/bin/python -m pytest \ + tests/test_submission_archive.py tests/test_guide_formats.py \ + --cov=app.modules.artifacts.zip_safety --cov-report=term-missing \ + --cov-fail-under=90) +python3 scripts/check_stale_artifact_contracts.py +python3 scripts/check_stale_authorization_docs.py +python3 scripts/check_stale_workstream_wording.py +python3 scripts/check_markdown_links.py +git diff --check +``` + +The exact PR head must pass hosted `Backend / test` at the repository-wide 78 +percent floor and `Agent Gates / agent-gates`. Semantic-lane inventory must +collect the new focused module exactly once; no test, coverage, lint, or +documentation gate may be skipped or weakened. + +Focused tests explicitly cover CRC/truncation and declared-size mismatch, +local-header/central-directory disagreement, data descriptors, ZIP64 and +multi-disk markers, central-directory abuse, high entry count and compression +ratio, all path/collision classes, Unix special modes, encrypted entries, +nested-ZIP opacity, cancellation/timeout cleanup, result redaction, and proof +that no provider or durable service is reachable. ## Required Reviewers diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-PLAN3-v01-end-to-end-reconciliation.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-PLAN3-v01-end-to-end-reconciliation.md index 7db3146fb..6ae39e9bd 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-PLAN3-v01-end-to-end-reconciliation.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/chunks/WS-ART-001-PLAN3-v01-end-to-end-reconciliation.md @@ -48,7 +48,7 @@ git diff --check | Chunk | Required focused test module(s) | |---|---| | `04A1` | `tests/test_artifact_architecture.py`, `tests/test_alembic.py` | -| `04A2` | `tests/test_submission_archive.py`, `tests/test_artifact_scratch_manager.py` | +| `04A2` | `tests/test_submission_archive.py` (including the canonical `PreparedArtifact.inspect(...)` scratch-custody and cleanup seam) | | `04A3` | `tests/test_submission_manifest.py`, `tests/test_submission_change_gate.py` | | `04B` | `tests/test_submission_precheck.py`, `tests/test_checker_materialization.py` | | `04C1`-`04C2` | `tests/test_submission_bundle_admission.py`, `tests/test_artifact_verification.py`, `tests/test_artifact_recovery.py` | diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-internal-review-evidence.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-internal-review-evidence.md new file mode 100644 index 000000000..bbe4cb7cd --- /dev/null +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-internal-review-evidence.md @@ -0,0 +1,55 @@ +# WS-ART-001-04A2 Internal Review Evidence + +## Evidence gate + +Result: PASS. + +- Scope remains hidden and process-local: no public route, provider I/O, + durable admission, Submission, checker, semantic manifest, or AUTH activation. +- One outer ZIP is inspected through `PreparedArtifact.inspect(...)` while + canonical scratch custody remains with `ArtifactScratchManager`. +- Exact envelope and record coverage rejects prefixes, suffixes, gaps, + overlaps, central-directory disagreement, local/central disagreement, + descriptor corruption, stored extra bytes, and deflate unused tails. +- Paths, normalized collisions, ancestry conflicts, encryption, special Unix + entries, directory payloads, expansion ratios, actual bytes, and deadlines + fail closed with redacted stable internal codes. +- Nested ZIP members remain opaque regular files. Only stored and raw-DEFLATE + member compression is accepted in v0.1. +- Ruff, focused tests, focused 90-percent subsystem coverage, semantic-lane + inventory, stale scans, Markdown links, and diff checks pass locally. + +## Reviewer results + +- Architecture: PASS after startup cross-limit validation and a distinct + collision failure token were added. +- Security: PASS WITH LOW RISKS after exact byte-envelope/record coverage, + stored-size equality, and exact deflate consumption were added. Its remaining + aggregate-budget observation was also resolved during each member read. +- QA: PASS WITH LOW RISKS after exact envelope, hidden directory payload, and + complete adversarial seam cleanup proofs were added. +- Product/operations: PASS after local filenames for every entry, including + directories, were bound exactly to the central-directory name. +- Senior engineering: PASS WITH LOW RISKS after the neutral ZIP helper move, + directory payload rejection, and finite deadline validation. +- CI integrity: PASS WITH LOW RISKS; no workflow, threshold, package-script, or + dependency weakening. Hosted Backend and Agent Gates remain required. +- Documentation: PASS. +- Reuse/dedup: PASS after the ZIP directory probe moved to neutral + `zip_safety.py`. +- Test delta: PASS WITH LOW RISKS after all requested adversarial and + configuration-mapping cases were added. + +## Findings resolved + +- Safe central names cannot hide unsafe local-header names. +- No byte before, between, inside, or after ZIP records escapes accounting. +- Directory entries cannot carry hidden payloads. +- Stored and deflated members cannot hide bytes beyond logical content. +- ZIP64 is bounded while multi-disk/spanned layouts remain rejected. +- Configuration limits are validated at startup and capped at 512 MiB. + +## Residual risk + +The behavior has no public caller yet. Hosted PR shards, CodeRabbit, and human +review of the exact commit remain required before merge. diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-pr-trust-bundle.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-pr-trust-bundle.md new file mode 100644 index 000000000..193c3ee1c --- /dev/null +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-pr-trust-bundle.md @@ -0,0 +1,98 @@ +# WS-ART-001-04A2 PR Trust Bundle + +## Chunk + +`WS-ART-001-04A2 — Bounded Outer-ZIP Safety` (L1) + +## Goal and human-approved intent + +Inspect one contributor outer ZIP completely and safely inside canonical +private scratch, returning only bounded structural facts for the later semantic +manifest chunk. The ZIP's required contents remain governed by the locked +Project Guide. This chunk creates no route, checker, provider write, admission, +or Submission. + +## What changed and why + +- Added immutable structural result, entry, limit, and redacted failure types. +- Added exact ZIP envelope, central-directory, local-record, descriptor, + compression-stream, path, collision, type, and quota validation. +- Moved the neutral bounded ZIP directory probe into `zip_safety.py` for guide + and submission reuse without guide-specific recursion. +- Added startup-fixed settings and composition for outer-ZIP limits. +- Registered the focused test module in the semantic CI lane. +- Updated the ART contract/status and canonical storage specification. + +The later manifest, checker, reviewer, and client chain cannot trust a ZIP if +bytes can hide outside its enumerated tree or scratch ownership leaks. + +## Design chosen and alternatives rejected + +`SubmissionArchiveInspector` implements the existing +`PreparedArtifactInspector` seam. It reads scratch-owned bytes without +extracting them and returns sorted process-local facts only. It accepts stored +and raw-DEFLATE members, treats nested ZIPs as opaque files, and proves +continuous byte coverage through the exact EOCD/comment boundary. + +Rejected: guide-detector reuse, recursive nested ZIP inspection, direct temp +paths, a second scratch manager, provider writes, caller-owned limits, +self-extracting envelopes, and compression methods without exact-consumption +proof. + +## Scope control and product behavior + +No public/multipart route, durable model or migration, provider I/O, AUTH +activation, checker invocation, semantic hash, executable normalization, +unchanged-work comparison, or Submission behavior was added. Hidden inspection +returns a bounded tree or one stable redacted internal failure. Rejection has no +capacity, provider, review, contribution, payment, or reputation effect. + +## Acceptance criteria proof + +- Every regular member is fully read and CRC-checked through `zipfile`; exact + stored/deflate range consumption is independently proven. +- Prefixes, suffixes, gaps, overlaps, local/central mismatches, malformed data + descriptors, multi-disk layouts, traversal, collisions, special entries, + encryption, directory payloads, and bombs reject. +- Cancellation, timeout, and rejection prove scratch release through + `PreparedArtifact.inspect(...)`. +- Nested archives remain opaque; results expose no reader, path, bytes, + provider fact, authorization handle, hash, or durable identity. + +## Tests/checks run and test delta + +- Repository-wide backend Ruff: passed. +- Focused archive/config/guide/CI-lane tests: passed. +- `submission_archive.py` and `zip_safety.py`: each exceeds 90-percent focused + coverage. +- Stale artifact/auth/wording scans, Markdown links, and `git diff --check`: + passed. +- No tests were removed, skipped, or weakened. Semantic lane inventory includes + the new test module exactly once. + +## CI integrity and reviewer results + +No workflow, threshold, dependency, package-script, or skip behavior changed. +The repository-wide 78-percent hosted floor and Backend/Agent Gates remain +required. Architecture, product/operations, documentation, and reuse reviews +pass. Security, QA, senior engineering, CI integrity, and test-delta reviews +pass with only documented low residual risks; all actionable findings were +resolved. + +## External review + +CodeRabbit and hosted GitHub checks have not run yet. Findings must be triaged +against the exact PR head before merge. + +## Remaining risks and follow-up + +Only stored and raw-DEFLATE ZIP members are supported in v0.1. 04A3 adds the +canonical semantic manifest, executable normalization, and unchanged-work gate +only after this PR merges. + +## Human review focus and merge ownership + +Review byte-range accounting, local/central name binding, deflate EOF/unused +data, path collisions, and absence of provider/durable/public reachability. +Only the human repository owner may approve and merge after hosted checks and +external review pass. diff --git a/backend/app/adapters/artifacts/__init__.py b/backend/app/adapters/artifacts/__init__.py index 1791ebce1..c4d7a8a99 100644 --- a/backend/app/adapters/artifacts/__init__.py +++ b/backend/app/adapters/artifacts/__init__.py @@ -26,6 +26,7 @@ ArtifactPreparationService, ArtifactScratchManager, ) +from app.modules.artifacts.submission_archive import SubmissionArchiveLimits from app.modules.artifacts.schemas import ( ArtifactInternalAuthority, ) @@ -104,6 +105,26 @@ def artifact_preparation_limits(settings: Settings) -> ArtifactPreparationLimits ) +def submission_archive_limits(settings: Settings) -> SubmissionArchiveLimits: + """Map validated settings to the fixed outer-ZIP safety contract.""" + return SubmissionArchiveLimits( + maximum_entries=settings.artifact_submission_zip_maximum_entries, + maximum_path_bytes=settings.artifact_submission_zip_maximum_path_bytes, + maximum_path_depth=settings.artifact_submission_zip_maximum_path_depth, + maximum_central_directory_bytes=( + settings.artifact_submission_zip_maximum_central_directory_bytes + ), + maximum_entry_bytes=settings.artifact_submission_zip_maximum_entry_bytes, + maximum_expanded_bytes=settings.artifact_submission_zip_maximum_expanded_bytes, + maximum_compression_ratio=( + settings.artifact_submission_zip_maximum_compression_ratio + ), + maximum_inspection_seconds=( + settings.artifact_submission_zip_maximum_inspection_seconds + ), + ) + + def create_artifact_scratch_manager(settings: Settings) -> ArtifactScratchManager: """Construct a scratch manager from the canonical settings mapping.""" if settings.artifact_scratch_root is None: diff --git a/backend/app/core/config.py b/backend/app/core/config.py index d34448280..4b1ec66e1 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -156,6 +156,24 @@ class Settings(BaseSettings): artifact_admission_project_maximum_bytes: int | None = Field(default=None, gt=0) artifact_admission_deployment_maximum_bytes: int | None = Field(default=None, gt=0) artifact_stream_buffer_bytes: int = Field(default=1024 * 1024, gt=0, le=1024 * 1024) + artifact_submission_zip_maximum_entries: int = Field(default=2_000, gt=0, le=100_000) + artifact_submission_zip_maximum_path_bytes: int = Field(default=1024, gt=0, le=4096) + artifact_submission_zip_maximum_path_depth: int = Field(default=32, gt=0, le=256) + artifact_submission_zip_maximum_central_directory_bytes: int = Field( + default=8 * 1024 * 1024, gt=0, le=64 * 1024 * 1024 + ) + artifact_submission_zip_maximum_entry_bytes: int = Field( + default=128 * 1024 * 1024, gt=0, le=512 * 1024 * 1024 + ) + artifact_submission_zip_maximum_expanded_bytes: int = Field( + default=512 * 1024 * 1024, gt=0, le=512 * 1024 * 1024 + ) + artifact_submission_zip_maximum_compression_ratio: int = Field( + default=100, gt=0, le=10_000 + ) + artifact_submission_zip_maximum_inspection_seconds: float = Field( + default=300.0, gt=0.0, le=1800.0 + ) artifact_operation_lock_timeout_seconds: float = Field( default=1800.0, gt=0.0, @@ -457,6 +475,13 @@ def validate_artifact_storage(self) -> Settings: """ if self.artifact_scratch_maximum_concurrency > self.artifact_scratch_maximum_files: raise ValueError("artifact scratch concurrency cannot exceed its file limit") + if ( + self.artifact_submission_zip_maximum_entry_bytes + > self.artifact_submission_zip_maximum_expanded_bytes + ): + raise ValueError( + "artifact submission ZIP entry limit cannot exceed expanded limit" + ) if ( self.artifact_preparation_total_deadline_seconds + self.artifact_scratch_cleanup_margin_seconds diff --git a/backend/app/modules/artifacts/guide_formats.py b/backend/app/modules/artifacts/guide_formats.py index 0620da858..d65a386da 100644 --- a/backend/app/modules/artifacts/guide_formats.py +++ b/backend/app/modules/artifacts/guide_formats.py @@ -10,6 +10,8 @@ import zipfile from xml.etree import ElementTree +from app.modules.artifacts.zip_safety import zip_directory_facts + DETECTOR_NAME = "workstream.guide_format" DETECTOR_VERSION = "1" @@ -34,24 +36,6 @@ } -def zip_directory_facts(source: BinaryIO | BytesIO) -> tuple[int, int]: - """Read bounded EOCD facts 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 - - @dataclass(frozen=True, slots=True) class GuideFormatLimits: """Fixed startup-owned limits for structural guide inspection.""" diff --git a/backend/app/modules/artifacts/submission_archive.py b/backend/app/modules/artifacts/submission_archive.py new file mode 100644 index 000000000..7cbefcede --- /dev/null +++ b/backend/app/modules/artifacts/submission_archive.py @@ -0,0 +1,430 @@ +"""Bounded structural inspection of one contributor outer ZIP.""" + +from __future__ import annotations + +from dataclasses import dataclass +from enum import StrEnum +import math +from pathlib import PurePosixPath +import stat +import struct +import time +from typing import BinaryIO +import unicodedata +import zipfile +import zlib + +from app.modules.artifacts.zip_safety import zip_directory_layout + + +_READ_BYTES = 1024 * 1024 + + +class SubmissionArchiveFailureCode(StrEnum): + """Stable, non-sensitive outer-ZIP rejection categories.""" + + MALFORMED = "submission_archive_malformed" + LIMIT_EXCEEDED = "submission_archive_limit_exceeded" + UNSAFE_ENTRY = "submission_archive_unsafe_entry" + COLLISION = "submission_archive_collision" + ENCRYPTED = "submission_archive_encrypted" + INTEGRITY_FAILURE = "submission_archive_integrity_failure" + TIMEOUT = "submission_archive_timeout" + + +class SubmissionArchiveRejectedError(ValueError): + """Reject an archive without exposing attacker-controlled metadata.""" + + def __init__(self, code: SubmissionArchiveFailureCode) -> None: + self.code = code + super().__init__(code.value) + + +class SubmissionArchiveEntryType(StrEnum): + FILE = "file" + DIRECTORY = "directory" + + +@dataclass(frozen=True, slots=True) +class SubmissionArchiveEntry: + """One normalized structural entry; no scratch or ZIP handles escape.""" + + normalized_path: str + entry_type: SubmissionArchiveEntryType + byte_count: int + + +@dataclass(frozen=True, slots=True) +class SubmissionArchiveInspectionResult: + """Non-durable structural facts for the later semantic-manifest chunk.""" + + entries: tuple[SubmissionArchiveEntry, ...] + entry_count: int + file_count: int + directory_count: int + total_expanded_bytes: int + + +@dataclass(frozen=True, slots=True) +class SubmissionArchiveLimits: + """Startup-fixed limits for one outer ZIP inspection.""" + + maximum_entries: int = 2_000 + maximum_path_bytes: int = 1024 + maximum_path_depth: int = 32 + maximum_central_directory_bytes: int = 8 * 1024 * 1024 + maximum_entry_bytes: int = 128 * 1024 * 1024 + maximum_expanded_bytes: int = 512 * 1024 * 1024 + maximum_compression_ratio: int = 100 + maximum_inspection_seconds: float = 300.0 + + def __post_init__(self) -> None: + integers = ( + self.maximum_entries, + self.maximum_path_bytes, + self.maximum_path_depth, + self.maximum_central_directory_bytes, + self.maximum_entry_bytes, + self.maximum_expanded_bytes, + self.maximum_compression_ratio, + ) + if any(type(value) is not int or value <= 0 for value in integers): + raise ValueError("submission archive limits are invalid") + if self.maximum_entries > 100_000 or self.maximum_path_depth > 256: + raise ValueError("submission archive inventory limits are invalid") + if self.maximum_expanded_bytes > 512 * 1024 * 1024: + raise ValueError("submission archive expansion exceeds 512 MiB") + if self.maximum_entry_bytes > self.maximum_expanded_bytes: + raise ValueError("submission archive entry limit exceeds total limit") + if not isinstance(self.maximum_inspection_seconds, (int, float)) or isinstance( + self.maximum_inspection_seconds, bool + ) or not math.isfinite(self.maximum_inspection_seconds) or self.maximum_inspection_seconds <= 0: + raise ValueError("submission archive deadline is invalid") + + +class SubmissionArchiveInspector: + """Fully read one outer ZIP and return only bounded structural facts.""" + + def __init__(self, limits: SubmissionArchiveLimits) -> None: + self._limits = limits + + def inspect(self, reader: BinaryIO) -> SubmissionArchiveInspectionResult: + started = time.monotonic() + try: + reader.seek(0) + if reader.read(4) not in {b"PK\x03\x04", b"PK\x05\x06", b"PK\x06\x06"}: + self._reject(SubmissionArchiveFailureCode.MALFORMED) + layout = zip_directory_layout(reader, allow_zip64=True) + if ( + layout.entry_count > self._limits.maximum_entries + or layout.directory_bytes > self._limits.maximum_central_directory_bytes + ): + self._reject(SubmissionArchiveFailureCode.LIMIT_EXCEEDED) + reader.seek(0) + with zipfile.ZipFile(reader, allowZip64=True) as archive: + infos = archive.infolist() + if len(infos) != layout.entry_count: + self._reject(SubmissionArchiveFailureCode.MALFORMED) + self._validate_record_coverage( + archive, infos, directory_offset=layout.directory_offset + ) + entries = self._read_entries(archive, infos, started=started) + except SubmissionArchiveRejectedError: + raise + except (OSError, ValueError, zipfile.BadZipFile, RuntimeError): + self._reject(SubmissionArchiveFailureCode.MALFORMED) + files = sum(entry.entry_type is SubmissionArchiveEntryType.FILE for entry in entries) + directories = len(entries) - files + total = sum(entry.byte_count for entry in entries) + return SubmissionArchiveInspectionResult( + entries=tuple(sorted(entries, key=lambda entry: entry.normalized_path)), + entry_count=len(entries), + file_count=files, + directory_count=directories, + total_expanded_bytes=total, + ) + + def _read_entries( + self, archive: zipfile.ZipFile, infos: list[zipfile.ZipInfo], *, started: float + ) -> list[SubmissionArchiveEntry]: + results: list[SubmissionArchiveEntry] = [] + paths: dict[str, SubmissionArchiveEntryType] = {} + source_paths: set[str] = set() + total = 0 + for info in infos: + self._check_deadline(started) + path, entry_type = self._validated_path(info) + folded = unicodedata.normalize("NFC", path).casefold() + if folded in source_paths or ( + folded in paths and entry_type is SubmissionArchiveEntryType.FILE + ): + self._reject(SubmissionArchiveFailureCode.COLLISION) + parts = PurePosixPath(path).parts + for index in range(1, len(parts)): + ancestor = unicodedata.normalize("NFC", "/".join(parts[:index])).casefold() + if paths.get(ancestor) is SubmissionArchiveEntryType.FILE: + self._reject(SubmissionArchiveFailureCode.COLLISION) + if ancestor not in paths: + implicit_path = "/".join(parts[:index]) + paths[ancestor] = SubmissionArchiveEntryType.DIRECTORY + results.append( + SubmissionArchiveEntry( + implicit_path, + SubmissionArchiveEntryType.DIRECTORY, + 0, + ) + ) + if len(paths) > self._limits.maximum_entries: + self._reject(SubmissionArchiveFailureCode.LIMIT_EXCEEDED) + if entry_type is SubmissionArchiveEntryType.FILE: + prefix = folded + "/" + if any(existing.startswith(prefix) for existing in paths): + self._reject(SubmissionArchiveFailureCode.COLLISION) + paths[folded] = entry_type + source_paths.add(folded) + if entry_type is SubmissionArchiveEntryType.DIRECTORY and any( + entry.normalized_path == path for entry in results + ): + continue + actual = ( + self._read_member( + archive, + info, + started=started, + remaining_expanded_bytes=self._limits.maximum_expanded_bytes - total, + ) + if entry_type is SubmissionArchiveEntryType.FILE + else 0 + ) + total += actual + if total > self._limits.maximum_expanded_bytes: + self._reject(SubmissionArchiveFailureCode.LIMIT_EXCEEDED) + results.append(SubmissionArchiveEntry(path, entry_type, actual)) + return results + + def _validated_path( + self, info: zipfile.ZipInfo + ) -> tuple[str, SubmissionArchiveEntryType]: + raw = info.filename + if ( + not raw + or "\\" in raw + or "\x00" in raw + or raw.startswith("/") + or any(ord(character) < 32 or ord(character) == 127 for character in raw) + ): + self._reject(SubmissionArchiveFailureCode.UNSAFE_ENTRY) + is_directory = info.is_dir() + if is_directory and (info.file_size != 0 or info.compress_size != 0): + self._reject(SubmissionArchiveFailureCode.UNSAFE_ENTRY) + path = raw[:-1] if is_directory and raw.endswith("/") else raw + raw_parts = path.split("/") + pure = PurePosixPath(path) + parts = pure.parts + if ( + not parts + or any( + part in {"", ".", ".."} or part.endswith((" ", ".")) + for part in raw_parts + ) + or ":" in parts[0] + or len(parts) > self._limits.maximum_path_depth + or len(path.encode("utf-8")) > self._limits.maximum_path_bytes + ): + self._reject(SubmissionArchiveFailureCode.UNSAFE_ENTRY) + mode = info.external_attr >> 16 + kind = stat.S_IFMT(mode) + if kind not in {0, stat.S_IFREG, stat.S_IFDIR}: + self._reject(SubmissionArchiveFailureCode.UNSAFE_ENTRY) + if (kind == stat.S_IFDIR) != is_directory and kind != 0: + self._reject(SubmissionArchiveFailureCode.UNSAFE_ENTRY) + if info.flag_bits & 0x1: + self._reject(SubmissionArchiveFailureCode.ENCRYPTED) + return unicodedata.normalize("NFC", path), ( + SubmissionArchiveEntryType.DIRECTORY + if is_directory + else SubmissionArchiveEntryType.FILE + ) + + def _read_member( + self, + archive: zipfile.ZipFile, + info: zipfile.ZipInfo, + *, + started: float, + remaining_expanded_bytes: int, + ) -> int: + maximum_output = min( + self._limits.maximum_entry_bytes, remaining_expanded_bytes + ) + if info.file_size > maximum_output: + self._reject(SubmissionArchiveFailureCode.LIMIT_EXCEEDED) + if info.file_size and ( + info.compress_size == 0 + or info.file_size > info.compress_size * self._limits.maximum_compression_ratio + ): + self._reject(SubmissionArchiveFailureCode.LIMIT_EXCEEDED) + self._validate_compressed_stream( + archive, info, started=started, maximum_output_bytes=maximum_output + ) + actual = 0 + try: + with archive.open(info, "r") as member: + while chunk := member.read(_READ_BYTES): + self._check_deadline(started) + actual += len(chunk) + if actual > maximum_output: + self._reject(SubmissionArchiveFailureCode.LIMIT_EXCEEDED) + except (EOFError, OSError, RuntimeError, zipfile.BadZipFile): + self._reject(SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + if actual != info.file_size: + self._reject(SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + return actual + + def _validate_record_coverage( + self, + archive: zipfile.ZipFile, + infos: list[zipfile.ZipInfo], + *, + directory_offset: int, + ) -> None: + """Prove local records cover every byte before the central directory.""" + expected_offset = 0 + for info in sorted(infos, key=lambda item: item.header_offset): + if info.header_offset != expected_offset: + self._reject(SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + expected_offset = self._validate_local_record(archive, info) + if expected_offset != directory_offset: + self._reject(SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + + def _validate_local_record(self, archive: zipfile.ZipFile, info: zipfile.ZipInfo) -> int: + """Bind central facts to one exact local record and return its end.""" + source = archive.fp + if source is None: + self._reject(SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + prior = source.tell() + try: + source.seek(info.header_offset) + header = source.read(30) + if len(header) != 30 or header[:4] != b"PK\x03\x04": + self._reject(SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + flags, compression = struct.unpack_from(" 0xFFFFFFFF or info.compress_size > 0xFFFFFFFF: + if len(descriptor) < offset + 20: + self._reject(SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + descriptor_compressed, descriptor_expanded = struct.unpack_from( + " 0xFFFFFFFF or info.compress_size > 0xFFFFFFFF + else 12 + ) + elif ( + crc != info.CRC + or compressed not in {info.compress_size, 0xFFFFFFFF} + or expanded not in {info.file_size, 0xFFFFFFFF} + ): + self._reject(SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + return payload_end + except (OSError, struct.error): + self._reject(SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + finally: + source.seek(prior) + + def _validate_compressed_stream( + self, + archive: zipfile.ZipFile, + info: zipfile.ZipInfo, + *, + started: float, + maximum_output_bytes: int, + ) -> None: + """Consume the exact compressed range without accepting trailing payload.""" + if info.compress_type == zipfile.ZIP_STORED: + if info.compress_size != info.file_size: + self._reject(SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + return + if info.compress_type != zipfile.ZIP_DEFLATED: + self._reject(SubmissionArchiveFailureCode.MALFORMED) + source = archive.fp + if source is None: + self._reject(SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + prior = source.tell() + actual = 0 + try: + source.seek(info.header_offset) + header = source.read(30) + name_bytes, extra_bytes = struct.unpack_from(" maximum_output_bytes: + self._reject(SubmissionArchiveFailureCode.LIMIT_EXCEEDED) + pending = decompressor.unconsumed_tail + actual += len(decompressor.flush()) + if ( + not decompressor.eof + or decompressor.unused_data + or decompressor.unconsumed_tail + or actual != info.file_size + ): + self._reject(SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + except (OSError, struct.error, zlib.error): + self._reject(SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + finally: + source.seek(prior) + + def _check_deadline(self, started: float) -> None: + if time.monotonic() - started > self._limits.maximum_inspection_seconds: + self._reject(SubmissionArchiveFailureCode.TIMEOUT) + + @staticmethod + def _reject(code: SubmissionArchiveFailureCode) -> None: + raise SubmissionArchiveRejectedError(code) diff --git a/backend/app/modules/artifacts/zip_safety.py b/backend/app/modules/artifacts/zip_safety.py new file mode 100644 index 000000000..f9daa6248 --- /dev/null +++ b/backend/app/modules/artifacts/zip_safety.py @@ -0,0 +1,83 @@ +"""Neutral bounded ZIP directory primitives shared by ART consumers.""" + +from __future__ import annotations + +from dataclasses import dataclass +import struct +from typing import BinaryIO +import zipfile + + +_EOCD = b"PK\x05\x06" +_ZIP64_LOCATOR = b"PK\x06\x07" +_ZIP64_EOCD = b"PK\x06\x06" + + +@dataclass(frozen=True, slots=True) +class ZipDirectoryLayout: + entry_count: int + directory_bytes: int + directory_offset: int + end_record_offset: int + + +def zip_directory_facts( + source: BinaryIO, *, allow_zip64: bool = False +) -> tuple[int, int]: + """Read bounded EOCD facts before ZipFile allocates its inventory.""" + layout = zip_directory_layout(source, allow_zip64=allow_zip64) + return layout.entry_count, layout.directory_bytes + + +def zip_directory_layout( + source: BinaryIO, *, allow_zip64: bool = False +) -> ZipDirectoryLayout: + """Return the exact central-directory envelope after bounded tail parsing.""" + source.seek(0, 2) + size = source.tell() + source.seek(max(0, size - 65_557)) + tail = source.read(65_557) + marker = tail.rfind(_EOCD) + if marker < 0 or len(tail) - marker < 22: + raise zipfile.BadZipFile("missing end of central directory") + absolute_marker = max(0, size - 65_557) + marker + comment_bytes = int.from_bytes(tail[marker + 20 : marker + 22], "little") + if marker + 22 + comment_bytes != len(tail): + raise zipfile.BadZipFile("trailing archive content") + disk, directory_disk, disk_entries, entries, directory_bytes, directory_offset = ( + struct.unpack_from(" No assert settings.api_admin_mutation_rate_window_seconds == 60 assert settings.api_authorization_read_rate_limit == 120 assert settings.api_authorization_read_rate_window_seconds == 60 + limits = submission_archive_limits(settings) + assert limits.maximum_expanded_bytes == 512 * 1024 * 1024 + assert limits.maximum_entry_bytes == 128 * 1024 * 1024 + + +def test_submission_archive_settings_map_to_fixed_validated_limits() -> None: + settings = Settings( + artifact_submission_zip_maximum_entries=12, + artifact_submission_zip_maximum_path_bytes=120, + artifact_submission_zip_maximum_path_depth=6, + artifact_submission_zip_maximum_central_directory_bytes=300, + artifact_submission_zip_maximum_entry_bytes=100, + artifact_submission_zip_maximum_expanded_bytes=200, + artifact_submission_zip_maximum_compression_ratio=8, + artifact_submission_zip_maximum_inspection_seconds=9.0, + ) + limits = submission_archive_limits(settings) + assert limits.maximum_entries == 12 + assert limits.maximum_path_bytes == 120 + assert limits.maximum_path_depth == 6 + assert limits.maximum_central_directory_bytes == 300 + assert limits.maximum_entry_bytes == 100 + assert limits.maximum_expanded_bytes == 200 + assert limits.maximum_compression_ratio == 8 + assert limits.maximum_inspection_seconds == 9.0 + + with pytest.raises(ValidationError, match="entry limit"): + Settings( + artifact_submission_zip_maximum_entry_bytes=2, + artifact_submission_zip_maximum_expanded_bytes=1, + ) def test_rate_limit_secret_is_canonical_and_redacted() -> None: diff --git a/backend/tests/test_submission_archive.py b/backend/tests/test_submission_archive.py new file mode 100644 index 000000000..d0574bb3d --- /dev/null +++ b/backend/tests/test_submission_archive.py @@ -0,0 +1,412 @@ +from __future__ import annotations + +import asyncio +from io import BytesIO +from pathlib import Path +import stat +import struct +import threading +from typing import BinaryIO +import zipfile + +import pytest + +from app.modules.artifacts.preparation import ArtifactPreparationService, ArtifactScratchManager +from app.modules.artifacts.submission_archive import ( + SubmissionArchiveEntryType, + SubmissionArchiveFailureCode, + SubmissionArchiveInspector, + SubmissionArchiveLimits, + SubmissionArchiveRejectedError, +) +from app.modules.artifacts.zip_safety import zip_directory_facts +from tests.artifact_store_helpers import artifact_byte_stream, artifact_preparation_limits + + +def archive_bytes(entries: dict[str, bytes], *, compression: int = zipfile.ZIP_STORED) -> bytes: + output = BytesIO() + with zipfile.ZipFile(output, "w", compression=compression, allowZip64=True) as archive: + for name, value in entries.items(): + archive.writestr(name, value) + return output.getvalue() + + +def inspect(data: bytes, limits: SubmissionArchiveLimits | None = None): + return SubmissionArchiveInspector(limits or SubmissionArchiveLimits()).inspect(BytesIO(data)) + + +def rejection(data: bytes, code: SubmissionArchiveFailureCode) -> None: + with pytest.raises(SubmissionArchiveRejectedError) as caught: + inspect(data) + assert caught.value.code is code + assert str(caught.value) == code.value + + +def test_outer_zip_returns_sorted_typed_structure_and_implicit_directories() -> None: + nested = archive_bytes({"inside.txt": b"ordinary nested bytes"}) + result = inspect(archive_bytes({"z.txt": b"z", "folder/a.zip": nested})) + + assert [(entry.normalized_path, entry.entry_type, entry.byte_count) for entry in result.entries] == [ + ("folder", SubmissionArchiveEntryType.DIRECTORY, 0), + ("folder/a.zip", SubmissionArchiveEntryType.FILE, len(nested)), + ("z.txt", SubmissionArchiveEntryType.FILE, 1), + ] + assert result.entry_count == 3 + assert result.file_count == 2 + assert result.directory_count == 1 + assert result.total_expanded_bytes == len(nested) + 1 + + +@pytest.mark.parametrize( + "name", + ( + "../escape", + "/absolute", + "C:/drive", + "a\\b", + "a/./b", + "a//b", + "trailing. ", + ), +) +def test_unsafe_paths_fail_with_one_redacted_code(name: str) -> None: + rejection(archive_bytes({name: b"x"}), SubmissionArchiveFailureCode.UNSAFE_ENTRY) + + +@pytest.mark.parametrize( + "entries", + ( + {"A.txt": b"a", "a.txt": b"b"}, + {"café.txt": b"a", "café.txt": b"b"}, + {"node": b"file", "node/child": b"child"}, + {"node/child": b"child", "node": b"file"}, + ), +) +def test_collision_and_ancestry_confusion_fail_closed(entries: dict[str, bytes]) -> None: + rejection(archive_bytes(entries), SubmissionArchiveFailureCode.COLLISION) + + +def test_symlink_entry_is_rejected() -> None: + output = BytesIO() + info = zipfile.ZipInfo("link") + info.create_system = 3 + info.external_attr = (stat.S_IFLNK | 0o777) << 16 + with zipfile.ZipFile(output, "w") as archive: + archive.writestr(info, b"target") + rejection(output.getvalue(), SubmissionArchiveFailureCode.UNSAFE_ENTRY) + + +@pytest.mark.parametrize("kind", (stat.S_IFIFO, stat.S_IFCHR, stat.S_IFBLK, stat.S_IFSOCK)) +def test_other_special_entries_are_rejected(kind: int) -> None: + output = BytesIO() + info = zipfile.ZipInfo("special") + info.create_system = 3 + info.external_attr = (kind | 0o600) << 16 + with zipfile.ZipFile(output, "w") as archive: + archive.writestr(info, b"content") + rejection(output.getvalue(), SubmissionArchiveFailureCode.UNSAFE_ENTRY) + + +def test_encrypted_flag_is_rejected_before_member_open() -> None: + data = bytearray(archive_bytes({"private.txt": b"content"})) + local = data.index(b"PK\x03\x04") + central = data.index(b"PK\x01\x02") + data[local + 6 : local + 8] = (1).to_bytes(2, "little") + data[central + 8 : central + 10] = (1).to_bytes(2, "little") + rejection(bytes(data), SubmissionArchiveFailureCode.ENCRYPTED) + + +def test_actual_expansion_and_ratio_are_bounded() -> None: + data = archive_bytes({"large.txt": b"0" * 4096}, compression=zipfile.ZIP_DEFLATED) + with pytest.raises(SubmissionArchiveRejectedError) as caught: + inspect(data, SubmissionArchiveLimits(maximum_entry_bytes=4096, maximum_expanded_bytes=4096, maximum_compression_ratio=2)) + assert caught.value.code is SubmissionArchiveFailureCode.LIMIT_EXCEEDED + + +def test_corrupt_member_is_fully_read_and_rejected() -> None: + data = bytearray(archive_bytes({"payload.txt": b"unique payload bytes"})) + offset = data.index(b"unique payload bytes") + data[offset] ^= 0xFF + rejection(bytes(data), SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + + +def test_local_header_name_disagreement_is_rejected_during_full_read() -> None: + data = bytearray(archive_bytes({"payload.txt": b"content"})) + local_name = data.index(b"payload.txt") + data[local_name] = ord("x") + rejection(bytes(data), SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + + +def test_directory_local_header_cannot_hide_unsafe_path() -> None: + data = bytearray(archive_bytes({"safe/": b""})) + local_name = data.index(b"safe/") + data[local_name : local_name + 5] = b"../x/" + rejection(bytes(data), SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + + +def test_declared_uncompressed_size_mismatch_is_rejected() -> None: + data = bytearray(archive_bytes({"payload.txt": b"content"})) + central = data.index(b"PK\x01\x02") + data[central + 24 : central + 28] = (99).to_bytes(4, "little") + rejection(bytes(data), SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + + +def test_data_descriptor_archive_is_read_without_special_casing() -> None: + class UnseekableBuffer(BytesIO): + def seekable(self) -> bool: + return False + + def seek(self, *_args: object, **_kwargs: object) -> int: + raise OSError("not seekable") + + output = UnseekableBuffer() + with zipfile.ZipFile(output, "w", compression=zipfile.ZIP_DEFLATED) as archive: + archive.writestr("descriptor.txt", b"descriptor content") + result = inspect(output.getvalue()) + assert result.total_expanded_bytes == len(b"descriptor content") + + malformed = bytearray(output.getvalue()) + descriptor = malformed.index(b"PK\x07\x08") + malformed[descriptor + 4] ^= 0xFF + rejection(bytes(malformed), SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + + +def test_malformed_non_zip_and_multidisk_are_rejected() -> None: + rejection(b"not a zip", SubmissionArchiveFailureCode.MALFORMED) + data = bytearray(archive_bytes({"a": b"a"})) + marker = data.rfind(b"PK\x05\x06") + data[marker + 4 : marker + 6] = (1).to_bytes(2, "little") + rejection(bytes(data), SubmissionArchiveFailureCode.MALFORMED) + + +@pytest.mark.parametrize("wrapped", ("prefix", "suffix")) +def test_additional_bytes_outside_exact_zip_envelope_are_rejected(wrapped: str) -> None: + data = archive_bytes({"a": b"a"}) + candidate = b"JUNK" + data if wrapped == "prefix" else data + b"JUNK" + rejection(candidate, SubmissionArchiveFailureCode.MALFORMED) + + +def test_pk_prefixed_payload_and_inter_record_gap_are_rejected() -> None: + data = archive_bytes({"a": b"a"}) + rejection(b"PK\x03\x04JUNK" + data, SubmissionArchiveFailureCode.MALFORMED) + + gap = b"HIDDEN" + mutated = bytearray(data) + central = mutated.index(b"PK\x01\x02") + mutated[central:central] = gap + eocd = mutated.rfind(b"PK\x05\x06") + old_offset = int.from_bytes(mutated[eocd + 16 : eocd + 20], "little") + mutated[eocd + 16 : eocd + 20] = (old_offset + len(gap)).to_bytes(4, "little") + rejection(bytes(mutated), SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + + +def test_directory_entry_cannot_hide_payload_bytes() -> None: + rejection(archive_bytes({"directory/": b"hidden"}), SubmissionArchiveFailureCode.UNSAFE_ENTRY) + + +def test_stored_member_cannot_hide_bytes_beyond_declared_file_size() -> None: + import binascii + + data = bytearray(archive_bytes({"file": b"abcXYZ"})) + local = data.index(b"PK\x03\x04") + central = data.index(b"PK\x01\x02") + crc = binascii.crc32(b"abc") + for offset in (local + 14, central + 16): + data[offset : offset + 4] = crc.to_bytes(4, "little") + for offset in (local + 22, central + 24): + data[offset : offset + 4] = (3).to_bytes(4, "little") + rejection(bytes(data), SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + + +def test_deflate_member_cannot_hide_unused_compressed_tail() -> None: + data = bytearray( + archive_bytes({"file": b"deflated content"}, compression=zipfile.ZIP_DEFLATED) + ) + local = data.index(b"PK\x03\x04") + central = data.index(b"PK\x01\x02") + compressed = int.from_bytes(data[local + 18 : local + 22], "little") + tail = b"UNUSED" + data[central:central] = tail + shifted_central = central + len(tail) + for offset in (local + 18, shifted_central + 20): + data[offset : offset + 4] = (compressed + len(tail)).to_bytes(4, "little") + eocd = data.rfind(b"PK\x05\x06") + data[eocd + 16 : eocd + 20] = shifted_central.to_bytes(4, "little") + rejection(bytes(data), SubmissionArchiveFailureCode.INTEGRITY_FAILURE) + + +def test_unsupported_zip_compression_method_fails_closed() -> None: + rejection( + archive_bytes({"file": b"content"}, compression=zipfile.ZIP_BZIP2), + SubmissionArchiveFailureCode.MALFORMED, + ) + + +def test_limit_configuration_rejects_invalid_or_oversized_values() -> None: + with pytest.raises(ValueError, match="512 MiB"): + SubmissionArchiveLimits(maximum_expanded_bytes=512 * 1024 * 1024 + 1) + with pytest.raises(ValueError, match="entry limit"): + SubmissionArchiveLimits(maximum_entry_bytes=2, maximum_expanded_bytes=1) + with pytest.raises(ValueError, match="limits are invalid"): + SubmissionArchiveLimits(maximum_entries=0) + with pytest.raises(ValueError, match="inventory"): + SubmissionArchiveLimits(maximum_entries=100_001) + with pytest.raises(ValueError, match="deadline"): + SubmissionArchiveLimits(maximum_inspection_seconds=False) + with pytest.raises(ValueError, match="deadline"): + SubmissionArchiveLimits(maximum_inspection_seconds=float("inf")) + + +def test_entry_inventory_and_total_expansion_limits_are_enforced() -> None: + data = archive_bytes({"a": b"aa", "b": b"bb"}) + with pytest.raises(SubmissionArchiveRejectedError) as inventory: + inspect(data, SubmissionArchiveLimits(maximum_entries=1)) + assert inventory.value.code is SubmissionArchiveFailureCode.LIMIT_EXCEEDED + with pytest.raises(SubmissionArchiveRejectedError) as directory: + inspect(data, SubmissionArchiveLimits(maximum_central_directory_bytes=1)) + assert directory.value.code is SubmissionArchiveFailureCode.LIMIT_EXCEEDED + with pytest.raises(SubmissionArchiveRejectedError) as entry: + inspect(data, SubmissionArchiveLimits(maximum_entry_bytes=1, maximum_expanded_bytes=4)) + assert entry.value.code is SubmissionArchiveFailureCode.LIMIT_EXCEEDED + with pytest.raises(SubmissionArchiveRejectedError) as total: + inspect(data, SubmissionArchiveLimits(maximum_entry_bytes=2, maximum_expanded_bytes=3)) + assert total.value.code is SubmissionArchiveFailureCode.LIMIT_EXCEEDED + + +def test_explicit_directory_is_structural_and_deadline_is_bounded( + monkeypatch: pytest.MonkeyPatch, +) -> None: + result = inspect(archive_bytes({"folder/file": b"x", "folder/": b""})) + assert result.entries[0].entry_type is SubmissionArchiveEntryType.DIRECTORY + monkeypatch.setattr("app.modules.artifacts.submission_archive.time.monotonic", lambda: 2.0) + inspector = SubmissionArchiveInspector( + SubmissionArchiveLimits(maximum_inspection_seconds=1.0) + ) + with zipfile.ZipFile(BytesIO(archive_bytes({"a": b"a"}))) as archive: + with pytest.raises(SubmissionArchiveRejectedError) as caught: + inspector._read_entries( # noqa: SLF001 - deterministic deadline boundary + archive, archive.infolist(), started=0.0 + ) + assert caught.value.code is SubmissionArchiveFailureCode.TIMEOUT + + +def test_zip64_directory_probe_is_explicitly_bounded() -> None: + zip64_record = ( + b"PK\x06\x06" + + struct.pack(" None: + manager = ArtifactScratchManager( + root=tmp_path / "scratch", limits=artifact_preparation_limits() + ) + prepared = await ArtifactPreparationService(manager).prepare( + artifact_byte_stream(archive_bytes({"work.txt": b"complete"})), + media_type="application/zip", + ) + async with prepared: + result = await prepared.inspect(SubmissionArchiveInspector(SubmissionArchiveLimits())) + assert result.file_count == 1 + assert not hasattr(result, "reader") + assert not hasattr(result, "scratch_path") + assert (await manager.usage()).reservation_count == 0 + assert list((tmp_path / "scratch" / "files").iterdir()) == [] + manager.close() + + +@pytest.mark.asyncio +async def test_rejected_inspection_is_redacted_and_prepared_scratch_is_released( + tmp_path: Path, +) -> None: + manager = ArtifactScratchManager( + root=tmp_path / "scratch", limits=artifact_preparation_limits() + ) + prepared = await ArtifactPreparationService(manager).prepare( + artifact_byte_stream(archive_bytes({"../../secret-name": b"secret-value"})), + media_type="application/zip", + ) + with pytest.raises(SubmissionArchiveRejectedError) as caught: + async with prepared: + await prepared.inspect(SubmissionArchiveInspector(SubmissionArchiveLimits())) + assert "secret-name" not in str(caught.value) + assert "secret-value" not in str(caught.value) + assert (await manager.usage()).reservation_count == 0 + manager.close() + + +@pytest.mark.asyncio +async def test_timeout_through_prepared_inspection_releases_scratch( + tmp_path: Path, +) -> None: + manager = ArtifactScratchManager( + root=tmp_path / "scratch", limits=artifact_preparation_limits() + ) + prepared = await ArtifactPreparationService(manager).prepare( + artifact_byte_stream(archive_bytes({"work.txt": b"complete"})), + media_type="application/zip", + ) + class DeterministicTimeoutInspector(SubmissionArchiveInspector): + def _check_deadline(self, started: float) -> None: + del started + self._reject(SubmissionArchiveFailureCode.TIMEOUT) + + with pytest.raises(SubmissionArchiveRejectedError) as caught: + async with prepared: + await prepared.inspect( + DeterministicTimeoutInspector( + SubmissionArchiveLimits(maximum_inspection_seconds=1.0) + ) + ) + assert caught.value.code is SubmissionArchiveFailureCode.TIMEOUT + assert (await manager.usage()).reservation_count == 0 + manager.close() + + +@pytest.mark.asyncio +async def test_cancelled_prepared_inspection_finishes_then_releases_scratch( + tmp_path: Path, +) -> None: + manager = ArtifactScratchManager( + root=tmp_path / "scratch", limits=artifact_preparation_limits() + ) + prepared = await ArtifactPreparationService(manager).prepare( + artifact_byte_stream(archive_bytes({"work.txt": b"complete"})), + media_type="application/zip", + ) + entered = threading.Event() + release = threading.Event() + + class BlockingInspector: + def inspect(self, reader: BinaryIO) -> int: + entered.set() + assert release.wait(5) + return len(reader.read()) + + async with prepared: + task = asyncio.create_task(prepared.inspect(BlockingInspector())) + assert await asyncio.to_thread(entered.wait, 5) + task.cancel() + release.set() + with pytest.raises(asyncio.CancelledError): + await task + assert (await manager.usage()).reservation_count == 0 + manager.close() diff --git a/docs/spec_artifact_storage_service.md b/docs/spec_artifact_storage_service.md index cf5e0ac80..7dd31d469 100644 --- a/docs/spec_artifact_storage_service.md +++ b/docs/spec_artifact_storage_service.md @@ -858,6 +858,22 @@ The preparation settings use the standard `WORKSTREAM_` environment prefix: | `WORKSTREAM_ARTIFACT_STREAM_BUFFER_BYTES` | `1048576` | Bounded streaming buffer, limited to at most 1 MiB. | | `WORKSTREAM_ARTIFACT_OPERATION_LOCK_TIMEOUT_SECONDS` | `1800` | Maximum wait for a private cross-process artifact-store operation lock before failing closed. | +Contributor outer-ZIP inspection uses additional startup-fixed bounds. These +limits govern scratch inspection only; they do not activate submission routes +or durable admission. Raising the 512 MiB ceiling requires a separately +approved operational change. + +| Environment variable | Default | Maximum | Contract | +|---|---:|---:|---| +| `WORKSTREAM_ARTIFACT_SUBMISSION_ZIP_MAXIMUM_ENTRIES` | `2000` | `100000` | Central-directory entries plus deterministic implicit directories. | +| `WORKSTREAM_ARTIFACT_SUBMISSION_ZIP_MAXIMUM_PATH_BYTES` | `1024` | `4096` | UTF-8 bytes in one normalized POSIX path. | +| `WORKSTREAM_ARTIFACT_SUBMISSION_ZIP_MAXIMUM_PATH_DEPTH` | `32` | `256` | Segments in one normalized path. | +| `WORKSTREAM_ARTIFACT_SUBMISSION_ZIP_MAXIMUM_CENTRAL_DIRECTORY_BYTES` | `8388608` | `67108864` | Bounded central-directory allocation guard. | +| `WORKSTREAM_ARTIFACT_SUBMISSION_ZIP_MAXIMUM_ENTRY_BYTES` | `134217728` | `536870912` | Actual expanded bytes for one regular file. | +| `WORKSTREAM_ARTIFACT_SUBMISSION_ZIP_MAXIMUM_EXPANDED_BYTES` | `536870912` | `536870912` | Actual expanded bytes across the complete outer archive. | +| `WORKSTREAM_ARTIFACT_SUBMISSION_ZIP_MAXIMUM_COMPRESSION_RATIO` | `100` | `10000` | Maximum expanded-to-compressed ratio for one file. | +| `WORKSTREAM_ARTIFACT_SUBMISSION_ZIP_MAXIMUM_INSPECTION_SECONDS` | `300` | `1800` | Complete synchronous inspector deadline inside the preparation deadline. | + Enabled artifact storage also requires an explicit durable-byte policy. None of these limits has a default, and startup fails unless all four are positive: @@ -1065,6 +1081,8 @@ normalized file/directory paths, entry type, file SHA-256/byte count, and the normalized executable flag for each regular file. Nested archives remain opaque in v0.1. Exact archive or semantic-manifest equality with the immediate prior immutable Submission rejects before checker/provider I/O. +The v0.1 structural safety boundary accepts only stored and raw-DEFLATE member +compression; other ZIP compression methods fail closed before admission. For a regular Unix-created ZIP entry with valid mode metadata, `executable` is true when any execute bit is present. It defaults false for non-Unix or invalid @@ -1093,6 +1111,14 @@ contributor outcome. It creates no durable artifact, admission, Submission, compensation, contribution, or reputation effect. Scratch is cleaned and process loss requires reupload without manager/operator approval. +The hidden 04A2 structural inspector uses the closed internal failure tokens +`submission_archive_malformed`, `submission_archive_limit_exceeded`, +`submission_archive_unsafe_entry`, `submission_archive_encrypted`, +`submission_archive_collision`, `submission_archive_integrity_failure`, and +`submission_archive_timeout`. +They contain no submitted paths or parser details. Their eventual HTTP/API +mapping remains deferred to the submission-admission route chunk. + Only a passing result is handed immediately to generic durable admission in the same process. Workstream writes the outer ZIP once, independently reads it back, and publishes a `ready` bindable admission only after exact verification. Existing put From 881d5c66becb2a7d028e74351b9b1b6f5bdc25d6 Mon Sep 17 00:00:00 2001 From: Abiorh001 Date: Tue, 4 Aug 2026 12:38:25 +0100 Subject: [PATCH 2/3] fix(artifacts): preserve guide zip failure semantics --- ...S-ART-001-04A2-external-review-response.md | 34 +++++++++++++++++++ .../WS-ART-001-04A2-pr-trust-bundle.md | 7 ++-- backend/app/modules/artifacts/zip_safety.py | 4 +-- 3 files changed, 41 insertions(+), 4 deletions(-) create mode 100644 .agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-external-review-response.md diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-external-review-response.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-external-review-response.md new file mode 100644 index 000000000..95393c3cd --- /dev/null +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-external-review-response.md @@ -0,0 +1,34 @@ +# WS-ART-001-04A2 External Review Response + +## Comments addressed + +- Hosted `shared_foundations` found two guide-OOXML regression failures after + the neutral ZIP probe move. EOCD per-disk versus total entry-count mismatch + was being classified as multi-disk before the existing guide layer could + preserve its stable `ooxml_directory_conflict` and `ooxml_zip64` outcomes. +- The neutral probe now reserves `multi-disk` for non-zero disk identifiers, + preserves the existing guide classification order, and retains submission + exact-record validation separately. + +## Comments deferred + +- CodeRabbit was rate-limited and produced no substantive review comments. + No automated finding is being treated as approved evidence. + +## Human decisions needed + +None for this correction. Human approval and merge remain required after the +replacement hosted run passes. + +## Commands rerun + +```text +ruff check app/modules/artifacts/zip_safety.py tests/test_submission_archive.py tests/test_guide_ooxml.py +pytest -q tests/test_guide_ooxml.py tests/test_submission_archive.py tests/test_guide_formats.py +``` + +Result: Ruff passed; 119 focused tests passed. + +## Remaining risks + +The exact correction commit still requires fresh hosted Backend and Agent Gates. diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-pr-trust-bundle.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-pr-trust-bundle.md index 193c3ee1c..eb598c771 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-pr-trust-bundle.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-pr-trust-bundle.md @@ -81,8 +81,11 @@ resolved. ## External review -CodeRabbit and hosted GitHub checks have not run yet. Findings must be triaged -against the exact PR head before merge. +CodeRabbit was rate-limited and produced no substantive comments. The first +hosted `shared_foundations` shard exposed two guide-OOXML stable-classification +regressions in the moved neutral ZIP probe; the helper was corrected and 119 +focused guide/submission tests pass. Fresh hosted checks are required on that +correction commit. ## Remaining risks and follow-up diff --git a/backend/app/modules/artifacts/zip_safety.py b/backend/app/modules/artifacts/zip_safety.py index f9daa6248..0ec4d9736 100644 --- a/backend/app/modules/artifacts/zip_safety.py +++ b/backend/app/modules/artifacts/zip_safety.py @@ -44,10 +44,10 @@ def zip_directory_layout( comment_bytes = int.from_bytes(tail[marker + 20 : marker + 22], "little") if marker + 22 + comment_bytes != len(tail): raise zipfile.BadZipFile("trailing archive content") - disk, directory_disk, disk_entries, entries, directory_bytes, directory_offset = ( + disk, directory_disk, _disk_entries, entries, directory_bytes, directory_offset = ( struct.unpack_from(" Date: Tue, 4 Aug 2026 12:49:44 +0100 Subject: [PATCH 3/3] fix(artifacts): address external review findings --- .../WS-ART-001-immutable-artifact-storage/STATUS.md | 5 +++-- .../WS-ART-001-04A2-external-review-response.md | 13 +++++++++---- backend/app/core/config.py | 7 +++++++ backend/tests/test_config.py | 5 +++++ backend/tests/test_submission_archive.py | 2 +- 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md index 67f7ac510..93b2814ce 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/STATUS.md @@ -6,8 +6,9 @@ ART-03A and every split ART-03B chunk through 03B4 are merged. The complete verified guide binding, materialization, classification, extraction, and hidden same-generation sufficiency continuation exist on `main`. -AUTH-04B production implementation merged in PR #245 at `6babf81b`. The fixed -guide binding and guide-reader services are live, and +AUTH-04A activated `artifact.guide_source.ingest`. AUTH-04B production +implementation merged in PR #245 at `6babf81b`. The fixed guide binding and +guide-reader services are live, and `artifact.guide_source.binding.create` plus `artifact.guide_source.read` are active under `XINT_002_04B`. ART-03C's dependency is satisfied. diff --git a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-external-review-response.md b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-external-review-response.md index 95393c3cd..936b1ead9 100644 --- a/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-external-review-response.md +++ b/.agent-loop/initiatives/WS-ART-001-immutable-artifact-storage/reviews/WS-ART-001-04A2-external-review-response.md @@ -9,16 +9,21 @@ - The neutral probe now reserves `multi-disk` for non-zero disk identifiers, preserves the existing guide classification order, and retains submission exact-record validation separately. +- CodeRabbit's completed correction-head review identified three valid small + fixes: align the ingest activation ledger, enforce inspection deadline below + the preparation deadline, and make NFC/NFD test literals explicit. All were + applied. Its hosted-status wording comment was also reconciled here. ## Comments deferred -- CodeRabbit was rate-limited and produced no substantive review comments. - No automated finding is being treated as approved evidence. +- The first-head CodeRabbit attempt was rate-limited. The correction-head + review completed; no comment was deferred. ## Human decisions needed -None for this correction. Human approval and merge remain required after the -replacement hosted run passes. +None. Human approval and merge remain required. The first hosted +`shared_foundations` run completed with the two documented guide failures; the +replacement Backend run and all five shards remain pending on the latest head. ## Commands rerun diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 4b1ec66e1..bb3dcb91e 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -482,6 +482,13 @@ def validate_artifact_storage(self) -> Settings: raise ValueError( "artifact submission ZIP entry limit cannot exceed expanded limit" ) + if ( + self.artifact_submission_zip_maximum_inspection_seconds + >= self.artifact_preparation_total_deadline_seconds + ): + raise ValueError( + "artifact submission ZIP inspection deadline must fit within preparation" + ) if ( self.artifact_preparation_total_deadline_seconds + self.artifact_scratch_cleanup_margin_seconds diff --git a/backend/tests/test_config.py b/backend/tests/test_config.py index e6c1b8ac9..7a16b0476 100644 --- a/backend/tests/test_config.py +++ b/backend/tests/test_config.py @@ -91,6 +91,11 @@ def test_submission_archive_settings_map_to_fixed_validated_limits() -> None: artifact_submission_zip_maximum_entry_bytes=2, artifact_submission_zip_maximum_expanded_bytes=1, ) + with pytest.raises(ValidationError, match="inspection deadline"): + Settings( + artifact_submission_zip_maximum_inspection_seconds=1800, + artifact_preparation_total_deadline_seconds=1800, + ) def test_rate_limit_secret_is_canonical_and_redacted() -> None: diff --git a/backend/tests/test_submission_archive.py b/backend/tests/test_submission_archive.py index d0574bb3d..3cfc0773c 100644 --- a/backend/tests/test_submission_archive.py +++ b/backend/tests/test_submission_archive.py @@ -77,7 +77,7 @@ def test_unsafe_paths_fail_with_one_redacted_code(name: str) -> None: "entries", ( {"A.txt": b"a", "a.txt": b"b"}, - {"café.txt": b"a", "café.txt": b"b"}, + {"caf\u00e9.txt": b"a", "cafe\u0301.txt": b"b"}, {"node": b"file", "node/child": b"child"}, {"node/child": b"child", "node": b"file"}, ),