fix(archives): wrap the bare EOFError a truncated tar.gz raises

[Noor-ul-ain001]

Problem

tarfile wraps most decompression failures in TarError, but a gzip stream that ends before its end-of-stream marker escapes as a bare EOFError from the gzip layer. EOFError derives from neither TarError nor OSError, so it bypassed all three tar handlers added with tar archive support (#3874):

Site	Handler before
format probe in detect_archive_format	except tarfile.TarError
tarfile.open in safe_extract_tar	except (tarfile.TarError, OSError)
member iteration in safe_extract_tar	except (tarfile.TarError, OSError)

A truncated .tar.gz — an interrupted download, a partially written file — raised a raw EOFError straight through the caller's error_type, so callers catching ValueError / ExtensionError / PresetError never saw it.

In specify workflow add the effect is worse than a traceback. Typer treats a bare EOFError as a Ctrl-D abort, so the whole diagnostic vanishes:

$ specify workflow add ./pkg.tar.gz      # truncated

Aborted.

The ZIP twin, given the same treatment, reports properly:

$ specify workflow add ./pkg.zip         # truncated
Error: Invalid workflow archive: Invalid ZIP archive: /tmp/pkg.zip

Fix

Route all three sites through a shared _TAR_DECOMPRESSION_ERRORS tuple so they stay in sync:

_TAR_DECOMPRESSION_ERRORS = (tarfile.TarError, EOFError, zlib.error)

zlib.error is included alongside EOFError: it is likewise neither a TarError nor an OSError, and can surface from a corrupt deflate block.

OSError is deliberately kept only on the two safe_extract_tar sites, which use it to report genuine I/O failures. Adding it to the probe would silently swallow those into "format mismatch" instead of the existing clean Invalid archive error, so the probe catches the decompression tuple alone.

After the fix, the tar path matches its ZIP twin:

$ specify workflow add ./pkg.tar.gz      # truncated
Error: Invalid workflow archive: Archive format mismatch: expected tar.gz, got
invalid/unsupported data

and domain error types wrap correctly again:

ExtensionError -> Invalid tar.gz archive: /tmp/...
PresetError    -> Invalid tar.gz archive: /tmp/...

Tests

Six regression tests in tests/test_download_security.py. tarfile decompresses lazily, so the leak surfaced at different sites depending on how much of the stream survived — the tests pin explicit byte counts to cover both:

• 64 bytes — fails inside tarfile.open itself (covers the probe and the open site)
• 512 / 2048 bytes — opens fine, fails during member iteration

Plus one test asserting the caller's error_type is honored, and one covering the safe_extract_archive entry point.

Verified test-the-test: all 6 fail against unmodified _download_security.py (DID NOT RAISE / raw EOFError), all pass with the fix.

tests/test_download_security.py    188 passed
tests/test_extensions.py           431 passed, 8 skipped
tests/test_workflows.py + test_presets.py   1403 passed, 27 failed

The 27 failures are the pre-existing Windows symlink-elevation class (OSError: [WinError 1314] A required privilege is not held by the client) — identical count and identity before and after this change on the same machine.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Wraps tar/gzip decompression failures in caller-defined domain errors.

Changes:

• Adds shared handling for TarError, EOFError, and zlib.error.
• Adds truncated tar.gz regression coverage.

Show a summary per file

File	Description
src/specify_cli/_download_security.py	Handles decompression failures across tar probing and extraction.
tests/test_download_security.py	Tests truncated tar.gz behavior and error wrapping.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Balanced

[mnriem]

Please address Copilot feedback

[Noor-ul-ain001]

@mnriem Copilot feedback addressed in ef49acc.

It flagged that the zlib.error arm of the new error tuple had no test coverage — correct, and it turned out to be reachable at the two safe_extract_tar sites (tarfile wraps zlib.error during member-header reads, but not during the forward seek that skips member data). Added four regression tests, verified test-the-test.

It also surfaced that the PR description overstated the scope: the detect_archive_format probe cannot see a bare zlib.error (fuzzed 2800 corrupt archives — zero). That arm is defensive there, and the comment and tests now say so rather than implying coverage that can't exist.

Details in the review thread. tests/test_download_security.py: 193 passed.

[Copilot]

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[mnriem]

Please fix test & lint errors

[Noor-ul-ain001]

Thanks — fixed in a0beea1. Lint was already green (ruff check src tests passes); the failure was a single test of mine that was not portable.

What broke. Exactly one test failed, on macos-latest / 3.13:

FAILED tests/test_download_security.py::test_corrupt_deflate_fixture_raises_bare_zlib_error
  - gzip.BadGzipFile: CRC check failed 0xeff9dff3 != 0x93656ee0

The other five pytest jobs were fail-fast cancellations, not real failures — and the preceding commit (c303925, the fix itself) was green on all six. So the regression came entirely from the zlib.error coverage I added for the review comment above.

Why. That fixture built its corruption by XOR-ing 64 arbitrary bytes mid-stream. Whether that yields a structural deflate error turns out to be zlib-version dependent: on the macOS runner the mangled bytes still decoded successfully, so the stream instead failed the trailing gzip CRC check and raised BadGzipFile — which is an OSError, i.e. a type the pre-fix (TarError, OSError) handler already caught. The guard test exists precisely to catch that kind of degradation, and it did its job; my fixture was just the thing that was wrong.

The fix. Replace the XOR with a deflate block header whose BTYPE is the reserved value 0b11. Every zlib rejects that identically as "invalid block type", and it fails during decompression rather than at the CRC check, so no zlib version can turn it into a TarError or OSError. The stream is now assembled by hand (compressobj(..., -15) plus an explicit gzip header/trailer) so the invalid block lands a controlled 256 KiB into the first member's data — past the gzip read buffer, so the first header still reads clean and the failure surfaces from the forward seek in tarfile.next, which is the site the raw zlib.error actually escapes from.

A sweep over clean-prefix sizes confirms the margin is wide rather than balanced on a knife edge — with 512 KiB members, every prefix from 160 KiB up gives a bare zlib.error, versus the transition below ~131 KiB where tarfile still wraps it as ReadError. The hand-built header also zeroes the gzip mtime field, so the fixture is byte-identical across builds instead of embedding a timestamp.

I also strengthened the guard to assert what the fix actually depends on, rather than just the exception's name:

assert not isinstance(excinfo.value, tarfile.TarError)
assert not isinstance(excinfo.value, OSError)

That is the property that makes the zlib.error arm load-bearing, and it means the fixture cannot silently decay into an already-caught type again — which is the exact failure mode that produced this CI break.

Production code is unchanged from ef49acc — this commit is test-only.

Re-verified test-the-test by dropping the zlib.error arm from _TAR_DECOMPRESSION_ERRORS: the three wrapping tests fail with the raw zlib.error: Error -3 while decompressing data: invalid block type, and pass with it restored. tests/test_download_security.py: 193 passed. ruff check src tests: all checks passed.

[Copilot]

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[mnriem]

Thank you!