Skip to content

Commit 80464fb

Browse files
jawwad-aliclaude
andcommitted
fix(presets): re-validate catalog URL after redirects (HTTPS parity)
PresetCatalog._fetch_single_catalog opened the catalog URL and trusted the payload without re-validating response.geturl() after redirects. _open_url follows redirects (stripping auth only on an HTTPS->HTTP downgrade), so an https:// catalog entry that 30x-redirects to http://attacker/... was still fetched and trusted. The catalog payload supplies each preset's download_url + sha256, so a redirected payload can drive install of an arbitrary archive that passes verify_archive_sha256. Add the post-redirect geturl() re-validation via _validate_catalog_url, mirroring integrations/catalog.py, workflows/catalog.py, and bundler adapters — and presets/_commands.py, which already does this on its --from download path. This is the lone preset catalog-fetch site missing the guard. Test: an HTTPS URL whose response.geturl() reports http:// is rejected (PresetValidationError). Completed four existing fetch-test mocks that predated this behavior to report geturl() like a real urllib response. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent faeb956 commit 80464fb

2 files changed

Lines changed: 49 additions & 0 deletions

File tree

src/specify_cli/presets/__init__.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2187,6 +2187,16 @@ def _fetch_single_catalog(self, entry: PresetCatalogEntry, force_refresh: bool =
21872187

21882188
try:
21892189
with self._open_url(entry.url, timeout=10) as response:
2190+
# Re-validate the URL after any redirects: _open_url follows
2191+
# redirects (stripping auth only on an HTTPS->HTTP downgrade), so
2192+
# without this an https:// catalog entry that 30x-redirects to
2193+
# http://attacker/... would be fetched and trusted. The catalog
2194+
# payload supplies each preset's download_url + sha256, so a
2195+
# redirected payload defeats verify_archive_sha256. Mirrors the
2196+
# integrations/workflows catalog fetchers.
2197+
final_url = response.geturl()
2198+
if final_url != entry.url:
2199+
self._validate_catalog_url(final_url)
21902200
catalog_data = json.loads(response.read())
21912201

21922202
self._validate_catalog_payload(catalog_data, entry.url)

tests/test_presets.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1754,6 +1754,39 @@ def fake_open(req, timeout=None):
17541754

17551755
assert captured["req"].get_header("Authorization") == "Bearer ghp_testtoken"
17561756

1757+
def test_fetch_single_catalog_revalidates_redirected_url(self, project_dir):
1758+
"""An HTTPS catalog URL that redirects to http:// must be rejected AFTER
1759+
the redirect. _open_url follows redirects (auth stripped on downgrade),
1760+
so without re-validating response.geturl() the http payload would still
1761+
be fetched and trusted — and it supplies each preset's download_url +
1762+
sha256, defeating verify_archive_sha256. Parity with the
1763+
integrations/workflows catalog fetchers."""
1764+
catalog = PresetCatalog(project_dir)
1765+
1766+
class _Resp:
1767+
def __enter__(self):
1768+
return self
1769+
1770+
def __exit__(self, *a):
1771+
return False
1772+
1773+
def read(self):
1774+
return json.dumps({"schema_version": "1.0", "presets": {}}).encode()
1775+
1776+
def geturl(self):
1777+
return "http://evil.test/catalog.json" # downgraded via redirect
1778+
1779+
catalog._open_url = lambda url, timeout=None: _Resp()
1780+
1781+
entry = PresetCatalogEntry(
1782+
url="https://good.example/catalog.json",
1783+
name="c",
1784+
priority=1,
1785+
install_allowed=True,
1786+
)
1787+
with pytest.raises(PresetValidationError, match="HTTPS"):
1788+
catalog._fetch_single_catalog(entry, force_refresh=True)
1789+
17571790
@pytest.mark.parametrize(
17581791
"payload",
17591792
[
@@ -1787,6 +1820,9 @@ def test_fetch_single_catalog_rejects_malformed_payload(self, project_dir, paylo
17871820
mock_response.read.return_value = json.dumps(payload).encode()
17881821
mock_response.__enter__ = lambda s: s
17891822
mock_response.__exit__ = MagicMock(return_value=False)
1823+
# A real urllib response reports the final URL (== request URL with no
1824+
# redirect); the fetcher re-validates it after redirects.
1825+
mock_response.geturl.return_value = "https://example.com/catalog.json"
17901826

17911827
entry = PresetCatalogEntry(
17921828
url="https://example.com/catalog.json",
@@ -1856,6 +1892,7 @@ def test_fetch_single_catalog_rejects_malformed_cached_payload(
18561892
mock_response.read.return_value = json.dumps(valid).encode()
18571893
mock_response.__enter__ = lambda s: s
18581894
mock_response.__exit__ = MagicMock(return_value=False)
1895+
mock_response.geturl.return_value = catalog.DEFAULT_CATALOG_URL
18591896

18601897
entry = PresetCatalogEntry(
18611898
url=catalog.DEFAULT_CATALOG_URL,
@@ -2101,6 +2138,7 @@ def test_fetch_catalog_survives_unwritable_cache(self, project_dir, monkeypatch)
21012138
mock_response.read.return_value = json.dumps(valid).encode()
21022139
mock_response.__enter__ = lambda s: s
21032140
mock_response.__exit__ = MagicMock(return_value=False)
2141+
mock_response.geturl.return_value = catalog.DEFAULT_CATALOG_URL
21042142

21052143
# Simulate an unwritable cache dir: every write_text under the
21062144
# cache directory raises PermissionError (an OSError subclass).
@@ -2153,6 +2191,7 @@ def test_get_merged_packs_skips_non_mapping_entries(self, project_dir):
21532191
mock_response.read.return_value = json.dumps(payload).encode()
21542192
mock_response.__enter__ = lambda s: s
21552193
mock_response.__exit__ = MagicMock(return_value=False)
2194+
mock_response.geturl.return_value = "https://example.com/catalog.json"
21562195

21572196
entry = PresetCatalogEntry(
21582197
url="https://example.com/catalog.json",

0 commit comments

Comments
 (0)