Skip to content

Commit 1015ff2

Browse files
Copilotmnriem
Copilot
and
mnriem
authored
Fix GNU sparse skip in safe_extract_tarball; use response.geturl() for redirect-safe format detection and HTTPS re-check
Agent-Logs-Url: https://github.com/github/spec-kit/sessions/739d3f73-200b-417a-8a86-134329200560 Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com>
1 parent 05798a9 commit 1015ff2

5 files changed

Lines changed: 59 additions & 8 deletions

File tree

‎src/specify_cli/__init__.py‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2633,16 +2633,25 @@ def preset_add(
26332633

26342634
with tempfile.TemporaryDirectory() as tmpdir:
26352635
archive_fmt = _det_fmt(from_url)
2636+
final_url = from_url
26362637
try:
26372638
with urllib.request.urlopen(from_url, timeout=60) as response:
2639+
final_url = response.geturl()
26382640
if not archive_fmt:
26392641
content_type = response.headers.get("Content-Type", "")
2640-
archive_fmt = _det_fmt(from_url, content_type)
2642+
archive_fmt = _det_fmt(final_url, content_type)
26412643
archive_data = response.read()
26422644
except urllib.error.URLError as e:
26432645
console.print(f"[red]Error:[/red] Failed to download: {e}")
26442646
raise typer.Exit(1)
26452647

2648+
# Re-validate scheme after any redirect (scheme-downgrade guard).
2649+
_fp = _urlparse(final_url)
2650+
_fl = _fp.hostname in ("localhost", "127.0.0.1", "::1")
2651+
if _fp.scheme != "https" and not (_fp.scheme == "http" and _fl):
2652+
console.print(f"[red]Error:[/red] URL was redirected to a non-HTTPS URL: {final_url}")
2653+
raise typer.Exit(1)
2654+
26462655
if not archive_fmt:
26472656
console.print("[red]Error:[/red] Could not determine archive format from URL or Content-Type.")
26482657
console.print("Ensure the URL points to a .zip or .tar.gz/.tgz file.")
@@ -3652,15 +3661,24 @@ def extension_add(
36523661
download_dir = project_root / ".specify" / "extensions" / ".cache" / "downloads"
36533662
download_dir.mkdir(parents=True, exist_ok=True)
36543663
archive_fmt = detect_archive_format(from_url)
3664+
final_url = from_url
36553665
archive_path = None
36563666

36573667
try:
36583668
with urllib.request.urlopen(from_url, timeout=60) as response:
3669+
final_url = response.geturl()
36593670
if not archive_fmt:
36603671
content_type = response.headers.get("Content-Type", "")
3661-
archive_fmt = detect_archive_format(from_url, content_type)
3672+
archive_fmt = detect_archive_format(final_url, content_type)
36623673
archive_data = response.read()
36633674

3675+
# Re-validate scheme after any redirect (scheme-downgrade guard).
3676+
_fp = urlparse(final_url)
3677+
_fl = _fp.hostname in ("localhost", "127.0.0.1", "::1")
3678+
if _fp.scheme != "https" and not (_fp.scheme == "http" and _fl):
3679+
console.print(f"[red]Error:[/red] URL was redirected to a non-HTTPS URL: {final_url}")
3680+
raise typer.Exit(1)
3681+
36643682
if not archive_fmt:
36653683
console.print("[red]Error:[/red] Could not determine archive format from URL or Content-Type.")
36663684
console.print("Ensure the URL points to a .zip or .tar.gz/.tgz file.")

‎src/specify_cli/extensions.py‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,15 @@ def safe_extract_tarball(
173173
# Tar metadata member types to skip during validation — they carry no
174174
# extractable payload and are generated automatically by many common
175175
# archiving tools (e.g. PAX headers, GNU longname/longlink entries).
176+
# GNUTYPE_SPARSE is intentionally excluded: it carries a real file payload
177+
# and tarfile.TarInfo.isreg() returns True for it, so it passes the
178+
# regular-file check below and is extracted correctly.
176179
_TAR_METADATA_TYPES = (
177-
tarfile.XHDTYPE, # PAX extended header
178-
tarfile.XGLTYPE, # PAX global extended header
179-
tarfile.SOLARIS_XHDTYPE, # Solaris PAX extended header
180-
*tarfile.GNU_TYPES, # GNU longname / longlink / sparse
180+
tarfile.XHDTYPE, # PAX extended header
181+
tarfile.XGLTYPE, # PAX global extended header
182+
tarfile.SOLARIS_XHDTYPE, # Solaris PAX extended header
183+
tarfile.GNUTYPE_LONGNAME, # GNU long path name (metadata only)
184+
tarfile.GNUTYPE_LONGLINK, # GNU long link name (metadata only)
181185
)
182186

183187
try:
@@ -2153,21 +2157,34 @@ def download_extension(self, extension_id: str, target_dir: Optional[Path] = Non
21532157
version = ext_info.get("version", "unknown")
21542158

21552159
# Detect archive format from URL; resolve via Content-Type when needed.
2160+
# `final_url` may differ from `download_url` if the server redirects.
21562161
archive_fmt = detect_archive_format(download_url)
2162+
final_url = download_url
21572163

21582164
# Download the archive
21592165
try:
21602166
with self._open_url(download_url, timeout=60) as response:
2167+
final_url = response.geturl()
21612168
if not archive_fmt:
21622169
content_type = response.headers.get("Content-Type", "")
2163-
archive_fmt = detect_archive_format(download_url, content_type)
2170+
archive_fmt = detect_archive_format(final_url, content_type)
21642171
archive_data = response.read()
21652172

21662173
except urllib.error.URLError as e:
21672174
raise ExtensionError(f"Failed to download extension from {download_url}: {e}")
21682175
except IOError as e:
21692176
raise ExtensionError(f"Failed to read extension archive from {download_url}: {e}")
21702177

2178+
# Re-validate scheme after any redirect to guard against scheme-downgrade.
2179+
_final_parsed = urlparse(final_url)
2180+
_final_is_localhost = _final_parsed.hostname in ("localhost", "127.0.0.1", "::1")
2181+
if _final_parsed.scheme != "https" and not (
2182+
_final_parsed.scheme == "http" and _final_is_localhost
2183+
):
2184+
raise ExtensionError(
2185+
f"Extension download URL was redirected to a non-HTTPS URL: {final_url}"
2186+
)
2187+
21712188
# Choose file extension based on detected format.
21722189
if not archive_fmt:
21732190
raise ExtensionError(

‎src/specify_cli/presets.py‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2314,13 +2314,16 @@ def download_pack(
23142314
version = pack_info.get("version", "unknown")
23152315

23162316
# Detect archive format from URL; resolve via Content-Type when needed.
2317+
# `final_url` may differ from `download_url` if the server redirects.
23172318
archive_fmt = detect_archive_format(download_url)
2319+
final_url = download_url
23182320

23192321
try:
23202322
with self._open_url(download_url, timeout=60) as response:
2323+
final_url = response.geturl()
23212324
if not archive_fmt:
23222325
content_type = response.headers.get("Content-Type", "")
2323-
archive_fmt = detect_archive_format(download_url, content_type)
2326+
archive_fmt = detect_archive_format(final_url, content_type)
23242327
archive_data = response.read()
23252328

23262329
except urllib.error.URLError as e:
@@ -2330,6 +2333,16 @@ def download_pack(
23302333
except IOError as e:
23312334
raise PresetError(f"Failed to read preset archive from {download_url}: {e}")
23322335

2336+
# Re-validate scheme after any redirect to guard against scheme-downgrade.
2337+
_final_parsed = urlparse(final_url)
2338+
_final_is_localhost = _final_parsed.hostname in ("localhost", "127.0.0.1", "::1")
2339+
if _final_parsed.scheme != "https" and not (
2340+
_final_parsed.scheme == "http" and _final_is_localhost
2341+
):
2342+
raise PresetError(
2343+
f"Preset download URL was redirected to a non-HTTPS URL: {final_url}"
2344+
)
2345+
23332346
# Choose file extension based on detected format.
23342347
if not archive_fmt:
23352348
raise PresetError(

‎tests/test_extensions.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2759,6 +2759,7 @@ def test_download_extension_sends_auth_header(self, temp_dir, monkeypatch):
27592759

27602760
mock_response = MagicMock()
27612761
mock_response.read.return_value = zip_bytes
2762+
mock_response.geturl.return_value = "https://github.com/org/repo/releases/download/v1/test-ext.zip"
27622763
mock_response.__enter__ = lambda s: s
27632764
mock_response.__exit__ = MagicMock(return_value=False)
27642765

@@ -3661,6 +3662,7 @@ def test_download_extension_allows_bundled_with_url(self, temp_dir):
36613662

36623663
mock_response = MagicMock()
36633664
mock_response.read.return_value = b"fake zip data"
3665+
mock_response.geturl.return_value = "https://example.com/git-2.0.0.zip"
36643666
mock_response.__enter__ = lambda s: s
36653667
mock_response.__exit__ = MagicMock(return_value=False)
36663668

‎tests/test_presets.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,6 +1613,7 @@ def test_download_pack_sends_auth_header(self, project_dir, monkeypatch):
16131613

16141614
mock_response = MagicMock()
16151615
mock_response.read.return_value = zip_bytes
1616+
mock_response.geturl.return_value = "https://github.com/org/repo/releases/download/v1/test-pack.zip"
16161617
mock_response.__enter__ = lambda s: s
16171618
mock_response.__exit__ = MagicMock(return_value=False)
16181619

0 commit comments

Comments
 (0)