[mnriem]

Description

Bound decompressed tar.gz reads and seeks before GNU/PAX metadata parsing in both archive format detection and extraction. The shared 54 MiB stream limit includes headers, padding, metadata, and file contents while preserving the existing member-size and total-file-size limits.

Report limit violations through the caller's error type, preserve caller-owned streams, and document the limit in the shared catalog reference.

Testing

• Tested locally with uv run specify --help
• Ran existing tests with uv sync && uv run pytest
• Tested with a sample project (if applicable)

The full suite used the documented worktree-local interpreter instead of the template's uv run pytest invocation:

• LC_ALL=en_US.UTF-8 .venv/bin/python -m pytest tests -n 4 -q --tb=short — passed: 13,691 passed, 130 skipped, 63 warnings.
• uv run specify --help — passed.

A separate sample project is not applicable to this shared archive-helper change.

AI Disclosure

• I did not use AI assistance for this contribution
• I did use AI assistance (fill in the disclosure below)

AI disclosure: GitHub Copilot using GPT-6.1 Sol performed autonomous, user-directed implementation and validation and generated the code, tests, documentation, and PR description. Reasoning effort was not exposed to the session. A GitHub Copilot security-review subagent assisted with the initial assessment; its model was not reported. No line-by-line human review is claimed.

[Copilot]

🟡 Changes recommended

Tiny archives can trigger excessive memory use without a dedicated GNU/PAX metadata limit, while exception-handling flaws can reject valid ZIPs and obscure format-mismatch and size-limit diagnostics.

3 open issues

• Limit GNU/PAX metadata payload size before reading · New
• Preserve tar stream limit errors from OSError handlers · New
• Handle malformed gzip headers during format probing · New

What changed in this PR

Hardens Spec Kit’s shared archive helpers by bounding decompressed tar.gz access before GNU/PAX metadata parsing during format detection and extraction.

Changes:

• Adds a 54 MiB decompressed-stream budget alongside existing member and total-file-size limits.
• Adds coverage for oversized metadata, read/seek boundaries, and caller-owned stream handling.
• Documents the stream limit in the shared catalog reference.

AI disclosure: GitHub Copilot performed this review autonomously for the requester; model/settings unavailable. Review text and suggested fixes are AI-generated.

File	Description
tests/​test_download_security.py	Adds metadata-budget, stream-boundary, and stream-ownership coverage.
src/​specify_cli/​_download_security.py	Adds a bounded gzip reader for tar detection and extraction.
docs/​reference/​catalogs.md	Documents the 54 MiB limit and its scope.

_{Review effort: Max (auto)}

[mnriem]

Pushed commit 373ca76: GNU/PAX metadata payloads are now capped at 64 KiB before recursive parsing reads them, size-limit diagnostics are preserved for OSError-derived caller errors, and malformed gzip headers no longer hide format mismatches or reject otherwise valid ZIP archives. The targeted archive and installer suites passed all 1,791 tests. AI disclosure: posted on behalf of @mnriem by GitHub Copilot using GPT-6.1 Sol in autonomous, user-directed mode; reasoning effort was not exposed. The code, tests, documentation, and this summary were AI-generated.

[Copilot]

🟡 Changes recommended

GNU sparse maps bypass the metadata cap and allow large in-memory lists, while extension and workflow wrappers conceal archive-limit errors.

2 open issues

• Bound GNU sparse maps to prevent metadata-cap bypass · New
• Preserve archive limit errors through command wrappers · New

3 resolved since last review

• Limit GNU/PAX metadata payload size before reading
• Handle malformed gzip headers during format probing
• Preserve tar stream limit errors from OSError handlers

_{Review effort: Max (auto)}

[mnriem]

Pushed commit 66ab41a: old GNU sparse and GNU sparse 1.0 archives are rejected before their separately stored maps are parsed, while bounded PAX 0.0/0.1 sparse maps remain supported. Extension URL installs and suffixless workflow downloads now preserve archive safety diagnostics instead of substituting authentication advice or falling through to YAML parsing. All 1,892 targeted archive and installer tests passed. AI disclosure: posted on behalf of @mnriem by GitHub Copilot using GPT-6.1 Sol in autonomous, user-directed mode; reasoning effort was not exposed. The code, tests, documentation, and this summary were AI-generated.

[Copilot]

🔵 Needs a closer look

Archive validation still permits quadratic memory growth from global PAX headers, unhandled recursion errors from long metadata chains, and metadata-limit bypasses via negative sizes.

1 open issue

• Preserve archive limit errors through command wrappers

3 previously missed issues

In code that hasn't changed since last review

Bound cumulative global PAX metadata to prevent quadratic memory growth

src/​specify_cli/​_download_security.py:124

Interleaving global PAX headers with tiny files accumulates archive.pax_headers, which tarfile copies into every retained member. This allows quadratic memory growth during validation while staying under the 54 MiB stream, 64 KiB payload, and 512-entry limits. Bound cumulative global PAX metadata per archive before parsing, raise _TarLimitError when that budget is exceeded, and add an interleaved-header regression case.

Reject negative base-256 metadata sizes before bounded reads

src/​specify_cli/​_download_security.py:120

On supported Python patch releases where tarfile._block() forwards negative sizes, a base-256 metadata size of -512 passes this guard and becomes a negative read length. _BoundedTarGzipFile.read() treats that as an unsized read, allowing more than 64 KiB to reach the metadata parser. Reject sizes outside 0..MAX_TAR_METADATA_BYTES and add a base-256 negative-size regression case.

Bound PAX header chain depth to prevent raw RecursionError

src/​specify_cli/​_download_security.py:125

A chain of 400 tiny PAX headers causes this recursive call to raise a raw RecursionError, even though the archive is far below both new byte limits. That bypasses the caller's error_type and normal CLI error handling. Bound metadata chain depth on the archive, raise _TarLimitError at that bound, and add a long-chain regression case.

1 resolved since last review

• Bound GNU sparse maps to prevent metadata-cap bypass

_{Review effort: Max (auto)}

[mnriem]

Pushed commit 36566e4: global PAX payloads now have an 8 KiB cumulative per-archive budget, metadata chains are limited to 32 headers, and negative base-256 metadata sizes are rejected before parsing. Regression coverage includes interleaved global headers, exact budget and depth boundaries, independent archive state, and legacy negative-read behavior. The extension/workflow wrapper fix from 66ab41a remains in place; caller-level tests now also verify preservation of all three new safety diagnostics. All 1,937 targeted archive and installer tests passed. AI disclosure: posted on behalf of @mnriem by GitHub Copilot using GPT-6.1 Sol in autonomous, user-directed mode; reasoning effort was not exposed. The code, tests, documentation, and this summary were AI-generated.

[Copilot]

🔵 Needs a closer look

Per-member PAX metadata lacks a cumulative budget, allowing an accepted 73 KiB archive within all new limits to drive peak memory usage to about 666 MiB.

1 open issue

• Preserve archive limit errors through command wrappers

_{Review effort: Max (auto)}