[amat-osto]

Adds support for v3.asset.file.repository_archive, allowing scans to target source archives instead of live repositories.

What it does

• Handles repository archive assets alongside live repositories.
• Downloads content_url archives or extracts embedded content bytes into the shared /code volume.
• Supports ZIP, tar and compressed tar variants, and 7z with path, symlink, size, uncompressed-size, and member-count guards.
• Converts archive-library failures to ArchiveDownloadError; invalid archives are logged and skipped without aborting the scan.
• Uses staging extraction and moves files into place only after successful extraction.

Verification

• Built locally with oxo agent build -f ostorlab.yaml -o ostorlab --force. The resulting image was agent_ostorlab_inject_asset:v2.0.3 (616c3b8061a7).
• Ran a live OXO repository-archive scan using the GitHub ZIP archive for commit 8ca1f7aa62dece9142146d356fc3ddb3ab06e735.
• Scan 139 completed with status Done.
• Injector logs confirmed the archive was extracted into /code.
• Injector logs confirmed the emitted selector was v3.asset.file.repository_archive.
• Unit tests, Ruff lint, and formatting checks pass in CI.

[ostorlab-ai-pr-review]

📊 PR Complexity Assessment: Moderate (Score: 3/5)

This PR adds a new asset type to the inject_asset agent: v3.asset.file.repository_archive, letting scans target source archives instead of live repositories. It introduces a new 331-line repository_archive.py module that downloads archives from content_url or extracts embedded content bytes into the shared /code volume, with format sniffing for ZIP, tar (gzip/bzip2/xz/lzma variants), and 7z, plus extensive defenses against untrusted archive content: path-traversal rejection, symlink audits, download/uncompressed-size/member-count/header-size caps, and staged extraction that only moves files into place after success. The agent dispatch is extended to handle the new selector, failures are logged-and-skipped (single bad asset must not abort the scan), and the provider exception hierarchy gains a common Error base (backward compatible). Roughly 900 LOC across 6 files including ~500 lines of well-targeted tests, plus a new py7zr dependency. The change is functionally additive and isolated from the existing repository-checkout flow, but the new code is a security-sensitive untrusted-input processing surface (zip bombs, Zip Slip, symlink escapes), so focused security review of the guard logic and the catch-all error boundary is warranted before merge; the PR also validates a real end-to-end OXO scan in its description.

🔍 Complexity Drivers

• New 331-line security-sensitive module (repository_archive.py) parsing and extracting untrusted archive bytes from downloads or embedded message content
• Supports three archive families (zip, tar + compression variants, 7z) each requiring separately wired guards
• Manual binary parsing of ZIP64 EOCD/locator structures and 7z signature headers for pre-extraction caps
• Multiple layered anti-bomb defenses (download size, uncompressed size, member count, 7z header size, symlink audit) whose correctness is security-relevant
• Deliberately broad except Exception boundaries translating heterogeneous library failures to ArchiveDownloadError; must pass through intended errors without masking genuine bugs
• Refactor of the shared provider exception hierarchy (new Error base, CloneError reparented) touched by existing handlers
• New third-party runtime dependency (py7zr) and implicit ostorlab >= 2.10.0 requirement for the new proto
• Substantial new test surface (~500 LOC) whose fidelity determines confidence in the guard wiring

🎯 Suggested Attention Areas

• agent/repository_archive.py (Path traversal and symlink escape defenses): Verify _check_member_paths correctly rejects ../ traversal and absolute-path members for zip/tar/7z, and that check_extracted_symlinks runs for the 7z branch after extraction but before any file is moved out of staging. Scrutinize the resolve()-based containment comparisons for edge cases: destination directories that do not yet exist at check time, symlink chains, and members that could collide with or overwrite the staging temp directory or pre-existing files in /code. Confirm tar relies on filter="data" (raises rather than sanitizes) and zip on extractall sanitization, and that the lexical check plus library behavior cannot be split into a bypass.
• agent/repository_archive.py (Archive-bomb resource caps and manual binary parsing): Manually verify the binary offsets in _check_zip_member_count (EOCD member count at +10, ZIP64 locator at EOCD-20, ZIP64 EOCD total count at +32) and in _check_7z_header_size (next-header size at +20) for malformed/truncated inputs, and confirm they fail closed rather than silently passing when structures are absent. Check that download-size, uncompressed-size, member-count, and 7z-header caps are all enforced BEFORE extraction for every format branch, that lazy tar member enumeration (_collect_tar_members) truly caps enumeration memory, and that format sniffing fails closed (e.g., plain gzip that is not a tar, unknown signatures).
• agent/agent_inject_asset.py (Exception boundary, error hierarchy, and scan continuity): Confirm every failure path from _extract, download_archive, and extract_content surfaces only as ArchiveDownloadError — including the catch-all except Exception boundaries (truncated gzip raises EOFError, corrupt deflate zlib.error, xz lzma.LZMAError) — and that _emit_asset logs-and-skips on it without emitting a partial asset or aborting start() for other assets. Verify dispatch precedence (content_url over content), empty-string/empty-bytes fallthrough to the missing-payload error, that emitted raw bytes equal the original asset, and that the errors.py hierarchy change (CloneError now under Error) is backward compatible with existing CloneError handlers.
• tests/repository_archive_test.py (Fidelity of corruption and limit tests): Assess whether the corruption tests genuinely exercise the catch-all boundary (truncated tar.gz triggering EOFError, corrupt deflate, corrupt 7z payload) and whether guard wiring is covered per format. Note gaps to probe: the ZIP64 member-count fallback path in _check_zip_member_count (only the 0xFFFF branch), missing-EOCD behavior, rename/move failure from staging into the destination (e.g., name collision), and 7z symlink-in-archive end-to-end behavior (audit is only unit-tested directly, not through a real 7z extraction).
• requirement.txt (New dependency and implicit version floor): py7zr is a new third-party dependency pulled into the scan image specifically to parse untrusted archive input — verify it is intentional (vs. handling 7z via tar/zip paths), reasonably maintained, and acceptable for the supply-chain posture. Also confirm the unpinned ostorlab[agent] requirement effectively enforces >= 2.10.0 (first release with the repository_archive proto); an environment resolving an older ostorlab would fail message parsing at runtime, so check whether CI or documentation pins/guards that floor.

⚠️ Sensitive Paths Touched

• agent/repository_archive.py
• agent/agent_inject_asset.py
• requirement.txt

⚡ Auto-Merge Assessment

❌ Not recommended for auto-merge — manual peer review required.

[ostorlab-ai-pr-review]

Code Review Summary

Total Issues Found: 5
Critical Issues (Bugs): 1
Security Issues: 1
Suggestions: 3

This PR adds repository archive download/extraction support (tar, zip, 7z) with security guards (member-count caps, size limits, path checks, tar filter="data") plus a new ArchiveDownloadError type and test suite. The design intent is sound and the zip/tar handling is solid, but the patch needs fixes before merge:

1. Critical (Bug): The exception tuples in extract_content/download_archive do not cover realistic corruption errors from the archive libraries (EOFError from truncated gzip, zlib.error from corrupt deflate streams, lzma.LZMAError, py7zr CrcError/DecompressionError). These escape all handlers up through _emit_asset and crash the entire agent start(), aborting injection of every asset in the scan — directly violating the documented skip-on-failure contract.
2. Security: The 7z branch lacks any symlink containment. py7zr materializes symlink members as real symlinks, so a crafted archive (e.g., link -> ../../.. followed by link/evil) passes the lexical filename check and writes outside the destination, with the escaping symlink also relocated into /code.
3. Suggestion: The new ArchiveDownloadError creates a second exception root in providers/errors.py, contradicting the module docstring and the single-base convention.
4. Suggestion: The 7z branch's guard wiring (member count, size cap, path check) has zero test coverage, so regressions would pass CI.
5. Suggestion: The members parameter should be annotated with collections.abc.Sequence[str] per project convention.

Key themes are fail-closed archive extraction, exception-boundary translation (library errors must be normalized to domain errors at the _extract boundary), and format-parity in both defenses and tests.

[ostorlab-ai-pr-review]

Re-review: 2 remaining findings (1 bug, 1 security)

Fixes from the previous round verified at head 67eb588: the exception-boundary translation, the 7z post-extraction symlink audit, the Sequence[str] annotation, and the new 7z guard tests are all in place. The missing tests/providers/errors_test.py issue is already tracked in the errors.py thread (#3981739117) and is not duplicated here.

Two newly confirmed issues remain:

• agent/repository_archive.py:36 (Bug): the lzma-alone signature requires a zero dictionary size, which no real encoder emits, so every valid lzma-compressed tar falls through to "Unsupported repository archive format" — the advertised lzma support is non-functional.
• agent/repository_archive.py:168 (Security): the member cap fires only after ZipFile/py7zr materialize their full member lists at open, so a crafted archive under the 5 GB cap can exhaust memory before _check_member_count runs; the "lazy enumeration" guarantee holds only for tar.

Event is COMMENT — not mergeable until these are addressed.

[ostorlab-ai-pr-review]

Review follow-up

1 new issue found (maintainability / test design). No critical bugs or security issues in this pass.

• tests/repository_archive_test.py:339 — the 7z symlink-escape audit is unit-tested through a private seam. Make it reachable via the public surface: expose the helper as a public module-level function, or drive it through extract_content with the py7zr extraction call mocked at the external library boundary so an escaping symlink is materialized before the audit runs — while keeping the assertion that the resulting ArchiveDownloadError leaves the destination empty.

Note: the missing tests/providers/errors_test.py follow-up (the two error-hierarchy assertions and the one-test-file-per-module convention) is already tracked on the agent/providers/errors.py thread from commit 645f046 and remains unresolved at the PR head, so it is not duplicated here.

[ostorlab-ai-pr-review]

Re-review at head f46c40c. The previously reported fixes verified: boundary exception translation to ArchiveDownloadError, the 7z symlink audit via the public check_extracted_symlinks seam, LZMA-alone signature detection, the pre-materialization ZIP EOCD/ZIP64 and 7z header-size caps, and the Sequence[str] annotation. Two issues remain open:

1. Security — the new EOCD guard's record selection diverges from zipfile's own parser (65,557-byte scan window with no record validation vs zipfile's 65,558-byte window plus comment-length-to-EOF acceptance rule), leaving a craftable bypass at file_size - 65_558 that re-opens the exact pre-materialization memory-exhaustion window the guard was added to close. Align with CPython _EndRecData's acceptance rule.
2. Coverage — tests/providers/errors_test.py, described as added in 645f046, is still absent at head, so the Error re-parenting shared-contract change ships with zero test coverage; the ZIP64 sub-branch of the EOCD guard is also untested.

Details in the inline comments.

[ostorlab-ai-pr-review]

Commit the missing tests/providers/errors_test.py — it is still absent at head f46c40c (fetching it returns 404 and it is not in the PR's changed-file list), even though the 645f046 reply described it as added. Pin the two properties that reply promised: every persistence failure is catchable as Error, and ArchiveDownloadError is not a CloneError (which is what keeps the existing except CloneError in _emit_asset correctly scoped to repository failures only). The Error re-parenting is a shared-contract change currently shipped with zero test coverage, and the project convention requires one test file per source module (agent/providers/errors.py → tests/providers/errors_test.py). If the intent was to reuse an existing module instead, fold those two assertions into a tests/providers test file.

[ostorlab-ai-pr-review]

Align this guard's record selection with zipfile's own parser (CPython _EndRecData) so the guard and ZipFile.__init__ can never disagree about the authoritative EOCD. The guard scans the last 65,557 bytes with no validation of the found record, while zipfile scans the last 65,558 bytes and only accepts a record whose declared comment length reaches exactly EOF. A crafted archive whose EOCD starts precisely at file_size - 65_558 is invisible to this guard (the signature's first byte falls outside the 65,557-byte window, so rfind cannot match it and the guard returns without enforcing) yet is accepted by zipfile, which then materializes the full >1M-entry central directory before _check_member_count can fire — re-opening the exact pre-materialization memory window this check was added to close. Use the same 65,558-byte window and treat a record as authoritative only when EOCD start + 22 + declared comment size == archive_size (mirroring zipfile's acceptance rule, including the empty-comment fast path), keeping the ZIP64 fallback; records the parser will reject can simply be skipped here since ZipFile then fails closed with BadZipFile. Also add a regression test for the ZIP64 sub-branch (count == 0xFFFF → locator → ZIP64 EOCD), which currently has no coverage.