Skip to content

Commit dbbdb4b

Browse files
mnriemCopilot
andcommitted
fix(extensions): use zipfile.is_zipfile for --from content guard
Replace the weak zip_data.startswith(b"PK") prefix check with zipfile.is_zipfile() on a BytesIO so any non-ZIP payload (not just those lacking the PK magic) is rejected with the friendly error before install_from_zip can raise BadZipFile. Addresses PR review feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3623d5f commit dbbdb4b

2 files changed

Lines changed: 10 additions & 5 deletions

File tree

src/specify_cli/extensions/_commands.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,7 @@ def extension_add(
482482

483483
elif from_url:
484484
# Install from URL (ZIP file)
485+
import io
485486
import urllib.error
486487

487488
console.print(f"Downloading from {safe_url}...")
@@ -515,7 +516,7 @@ def extension_add(
515516
) as response:
516517
zip_data = response.read()
517518

518-
if not zip_data.startswith(b"PK"):
519+
if not zipfile.is_zipfile(io.BytesIO(zip_data)):
519520
console.print(
520521
f"[red]Error:[/red] {safe_url} did not return a ZIP archive "
521522
f"(got {len(zip_data)} bytes). This usually means the request "

tests/test_extensions.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@
4040
version_satisfies,
4141
)
4242

43+
# Minimal valid ZIP (empty end-of-central-directory record). Passes
44+
# zipfile.is_zipfile() so --from download tests exercise the content guard.
45+
_MINIMAL_ZIP_BYTES = b"PK\x05\x06" + b"\x00" * 18
46+
4347

4448
def can_create_symlink(tmp_path: Path) -> bool:
4549
"""Return True when the current platform/user can create file symlinks."""
@@ -5378,7 +5382,7 @@ def fake_install_from_zip(self_obj, zip_path, speckit_version, priority=10, forc
53785382
runner = CliRunner()
53795383
with patch.object(Path, "cwd", return_value=project_dir), \
53805384
patch("typer.confirm", return_value=True), \
5381-
patch("specify_cli.authentication.http.open_url", return_value=FakeResponse(b"PK\x03\x04zip-bytes")), \
5385+
patch("specify_cli.authentication.http.open_url", return_value=FakeResponse(_MINIMAL_ZIP_BYTES)), \
53825386
patch.object(ExtensionManager, "install_from_zip", fake_install_from_zip), \
53835387
patch.object(ExtensionRegistry, "get", return_value={}):
53845388
result = runner.invoke(
@@ -5514,7 +5518,7 @@ def fake_open_url(url, timeout=10, extra_headers=None, redirect_validator=None):
55145518
return FakeResponse(body)
55155519
seen["url"] = url
55165520
seen["headers"] = extra_headers
5517-
return FakeResponse(b"PK\x03\x04payload")
5521+
return FakeResponse(_MINIMAL_ZIP_BYTES)
55185522

55195523
def fake_install(self_obj, zip_path, speckit_version, priority=10, force=False):
55205524
return SimpleNamespace(
@@ -5615,7 +5619,7 @@ def fake_install_from_zip(self_obj, zip_path, speckit_version, priority=10, forc
56155619
runner = CliRunner()
56165620
with patch.object(Path, "cwd", return_value=project_dir), \
56175621
patch("typer.confirm", return_value=True), \
5618-
patch("specify_cli.authentication.http.open_url", return_value=FakeResponse(b"PK\x03\x04zip-bytes")), \
5622+
patch("specify_cli.authentication.http.open_url", return_value=FakeResponse(_MINIMAL_ZIP_BYTES)), \
56195623
patch.object(ExtensionManager, "install_from_zip", fake_install_from_zip):
56205624
result = runner.invoke(
56215625
app,
@@ -5624,7 +5628,7 @@ def fake_install_from_zip(self_obj, zip_path, speckit_version, priority=10, forc
56245628
)
56255629

56265630
assert result.exit_code == 0
5627-
assert installed["zip_bytes"] == b"PK\x03\x04zip-bytes"
5631+
assert installed["zip_bytes"] == _MINIMAL_ZIP_BYTES
56285632
assert installed["zip_path"].resolve().is_relative_to(downloads_dir.resolve())
56295633
assert installed["zip_path"].name.startswith("extension-url-download-")
56305634
assert not installed["zip_path"].exists()

0 commit comments

Comments
 (0)