mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
fix(catalogs): validate extension and preset catalog payload shape (#2621)
* fix(catalogs): validate extension and preset catalog payload shape
`ExtensionCatalog._fetch_single_catalog` and
`PresetCatalog._fetch_single_catalog` only check that the `extensions` /
`presets` key is *present* in the parsed catalog JSON. They don't check
that the value is a JSON object, and they don't check that the root is
a JSON object at all. A malformed (or compromised) upstream catalog
returning:
{"schema_version": "1.0", "extensions": []}
passes both `"extensions" not in catalog_data` and the subsequent
`response.read()` JSON parse, gets cached on disk, and then crashes
deep inside `_get_merged_extensions` (resp. `_get_merged_packs`) with:
AttributeError: 'list' object has no attribute 'items'
instead of the existing user-facing
`ExtensionError("Invalid catalog format from <url>")` /
`PresetError("Invalid preset catalog format")` that the surrounding
code is clearly trying to produce.
The sibling integration-catalog reader already validates this — see
`src/specify_cli/integrations/catalog.py` where the fetch path
explicitly checks both `isinstance(catalog_data, dict)` and
`isinstance(catalog_data.get("integrations"), dict)` before returning.
This change mirrors that pattern in the extension and preset readers so
the three catalog fetchers stay consistent and a malformed upstream
surfaces as the user-facing error instead of a raw Python traceback.
Adds parametrized regression tests covering:
- root payload is not a JSON object (list, str, int, null)
- root is a dict but `extensions` / `presets` value is the wrong type
(list, str, null, int)
All eight bad-payload shapes now raise the expected catalog error.
* fix(catalogs): skip non-mapping entries during extension and preset merge
Addresses Copilot review feedback on this PR.
`_fetch_single_catalog` now validates that the ``extensions`` / ``presets``
value is a mapping, but it doesn't (and shouldn't) validate every entry
inside that mapping. A payload like:
{"schema_version": "1.0", "extensions": {"good": {...}, "bad": []}}
passes the fetch-level guard, then later crashes inside
``_get_merged_extensions`` (resp. ``_get_merged_packs``) at
``{**ext_data, ...}`` with ``TypeError: 'list' object is not a mapping``.
The sibling integration-catalog reader at
``src/specify_cli/integrations/catalog.py:245`` handles this with a
per-entry ``isinstance(integ_data, dict)`` skip during merge, so one
malformed entry doesn't poison an otherwise valid catalog. This change
mirrors that pattern in the extension and preset mergers and adds
regression tests asserting that valid entries continue to merge while
malformed siblings are silently dropped.
* fix(catalogs): validate cached extension and preset payload shape
Addresses Copilot review feedback on this PR (round 2).
The earlier commits in this branch added payload-shape validation on the
network fetch path. The cache-hit path still returned
``json.loads(cache_file.read_text())`` directly without re-checking the
shape, so a cache poisoned by an older spec-kit version (or a manual
edit, or an upstream that briefly served a bad payload before the
network guards landed) would re-crash every invocation of
``_get_merged_extensions`` / ``_get_merged_packs`` with
``AttributeError: 'list' object has no attribute 'items'`` despite the
cache being "valid" by age.
Extracts the shape validation into ``_validate_catalog_payload`` on both
``ExtensionCatalog`` and ``PresetCatalog``, and calls it from both the
cache-load and network-fetch branches of ``_fetch_single_catalog``. If
the cached payload fails validation, the cache read is treated like a
``json.JSONDecodeError`` — the cached value is discarded and the
function falls through to the network fetch, which refreshes the cache
with a clean payload on success. Never propagates ``AttributeError`` to
the caller.
Regression tests parametrize the four root-bad-type variants plus three
``extensions``/``presets``-bad-type variants per file, asserting that a
poisoned cache silently recovers via network refetch and returns the
freshly-fetched payload.
* fix(catalogs): include URL in missing-keys error to match sibling branches
Addresses Copilot review feedback on this PR (round 3).
``_validate_catalog_payload`` advertises in its docstring that the
catalog URL is included in error messages "so the user can tell which
catalog in a multi-catalog stack is malformed" — but the missing-keys
branch raised ``PresetError("Invalid preset catalog format")`` without
the URL, breaking that contract and making multi-catalog debugging
harder. The root-bad-type and nested-bad-type branches in the same
helper already include the URL; this commit brings the middle branch
in line.
For consistency, the same fix is applied to the legacy single-catalog
fetch paths in ``ExtensionCatalog.fetch_catalog`` and
``PresetCatalog.fetch_catalog`` (where the URL was likewise dropped
from the missing-keys error).
The existing regex matchers in the regression tests target the
``"Invalid (preset )?catalog format"`` prefix, which is preserved
verbatim before the ``from <url>`` suffix — no test changes needed.
* fix(catalogs): broaden cache except tuples and reuse validator in fetch_catalog
Addresses Copilot review feedback on this PR (round 4):
1. ``ExtensionCatalog.fetch_catalog`` and ``PresetCatalog.fetch_catalog``
— the legacy single-catalog methods — still only checked key
presence. A payload like ``42`` (root non-object) crashed with
``TypeError: argument of type 'int' is not iterable`` during the
``"schema_version" in catalog_data`` check, and an entry mapping of
the wrong type crashed downstream. Both now reuse
``_validate_catalog_payload`` so the network-side behaviour of the
legacy methods stays consistent with the multi-catalog
``_fetch_single_catalog`` path. (Copilot #3335623482, #3335623556.)
2. The cache-read ``except`` tuples in ``_fetch_single_catalog`` and
``fetch_catalog`` were too narrow. ``read_text`` can raise
``OSError`` (permissions / disk / handle limit) or ``UnicodeError``
(cache file written by an older client in a different encoding)
in addition to ``json.JSONDecodeError``. Without those in the
tuple, an unreadable cache crashed the caller instead of falling
through to the network refetch the cache contract documents. Both
sites now catch ``(json.JSONDecodeError, OSError, UnicodeError,
<DomainError>)``. (Copilot #3335623588, #3335623608.)
3. While here, pinned ``encoding="utf-8"`` on every cache ``read_text``
call so cache files written by an older Windows client (with a
non-UTF-8 default locale) decode the same way on a newer client.
Regression tests:
- ``test_fetch_catalog_rejects_malformed_payload`` — 7 parametrized
payloads per file covering root-non-object + nested-bad-type
variants asserting ``fetch_catalog`` raises the named domain error.
- ``test_fetch_catalog_recovers_from_unreadable_cache`` — writes
``b"\xff\xfe\x00not-utf-8"`` to the cache file and asserts
``fetch_catalog`` silently falls through to the mocked network and
returns the freshly-fetched payload.
* fix(catalogs): harden cache-validity checks and pin UTF-8 on writes
The cache-best-effort contract added in 7f44b25 was incomplete on two
points raised by Copilot:
1. The cache-validity helpers (is_cache_valid /
_is_url_cache_valid, plus the inline metadata-age check inside
_fetch_single_catalog for per-URL caches) read the metadata file
without specifying an encoding and only caught
json.JSONDecodeError / ValueError / KeyError /
TypeError. A metadata file written by a tool using the system
locale codec, or one whose handle is briefly unavailable, would
raise UnicodeDecodeError / OSError and propagate past the
read-side try/except in fetch_catalog — the very crash the
read-side guard was meant to prevent. The validity checks now read
with encoding="utf-8" and treat OSError / UnicodeError
as cache-invalid, matching the documented contract.
2. The network-fetch path wrote the cache and metadata files with bare
write_text(...), picking up the platform default encoding. The
read path was already pinned to UTF-8 (and the
integrations/catalog.py:193-203 sibling writes UTF-8 too), so
on hosts whose default codec isn't UTF-8 the write/read pair could
disagree and force an unnecessary refetch on every invocation. All
four write_text calls now pass encoding="utf-8" so the
cache survives a round trip on any platform.
Also rewords the misleading # Fetch from network comment in
extensions.fetch_catalog — it sat above the cache-check block,
which read as if the cache step had been skipped.
Tests
-----
Adds two parametrized regression tests per catalog:
* test_fetch_catalog_recovers_from_unreadable_metadata plants
non-UTF-8 bytes in the metadata file, asserts is_cache_valid()
returns False (rather than raising), and confirms
fetch_catalog falls through to the network instead of crashing.
* test_fetch_catalog_writes_cache_as_utf8 round-trips a payload
containing a non-ASCII identifier (café) through the public
fetch path and reads the cache back with
read_text(encoding="utf-8"), catching encoding drift at the
byte level rather than relying on the system codec to happen to be
UTF-8.
Both pairs follow the established sibling-file symmetry — the
extension and preset suites stay in lock-step.
* test(catalogs): assert UTF-8 write encoding by recording write_text kwargs
Copilot's review on this PR caught that test_fetch_catalog_writes_cache_as_utf8
claimed to validate UTF-8 at the byte level but actually only round-tripped a
non-ASCII string through json.dumps/read_text. Because json.dumps defaults to
ensure_ascii=True, 'café' was serialized as the all-ASCII escape 'caf\u00e9'
before reaching write_text — the bytes on disk were identical regardless of the
encoding kwarg, so a locale-encoded write would have round-tripped just fine.
The drift guard the test name advertised was not actually being enforced.
Rewriting these tests to observe the production code's argument directly:
each test now monkey-patches pathlib.Path.write_text with a recorder that
captures the encoding kwarg for every call, runs the production fetch, and
asserts every write into the cache directory passed encoding='utf-8'. That is
the substantive thing the regression guard cares about — non-ASCII payload
tricks were the wrong lever to pull, because json.dumps was masking the
encoding choice before write_text ever ran.
Both tests verified locally against the current production code (492 passed in
the extensions+presets suites) and confirmed to fail against a synthetic
no-encoding write (the recorder records None instead of 'utf-8', the assertion
catches it). Same change applied symmetrically to test_extensions.py and
test_presets.py to keep the sibling files in lockstep with the production
code paths in extensions.py and presets.py.
* fix(catalogs): catch AttributeError on non-mapping cache metadata; drop stale line refs
Copilot's review on the previous push pointed out that the
cache-validity helpers still had a gap: metadata.get("cached_at", "")
assumes metadata is a dict, but json.loads happily parses a
file containing [] / "oops" / 42 / true / null into
a non-mapping. The except tuple covered json.JSONDecodeError,
OSError, UnicodeError, ValueError, KeyError and
TypeError but not AttributeError, so a valid-JSON-but-non-dict
metadata payload would still crash the caller instead of degrading to
"cache invalid" as the docstring promised.
This affected four cache-validity sites — symmetric across the two
catalog modules:
* extensions.py — inline per-URL metadata-age check in
_fetch_single_catalog
* extensions.py — is_cache_valid (legacy default-URL path)
* presets.py — _is_url_cache_valid
* presets.py — is_cache_valid
All four except tuples now include AttributeError with a comment
naming the exact failure (metadata.get(...) on a non-mapping) so
the next reader doesn't have to reconstruct the reasoning.
Separately, Copilot flagged that several comments hard-coded a line
range from a sibling file
(integrations/catalog.py:193-203) — those references will go stale
the moment that file changes. Replaced the hard-coded ranges with
file-only references (integrations/catalog.py) so the pointer
stays accurate as that file evolves. Same change applied to both
modules.
Tests
-----
test_is_cache_valid_handles_non_mapping_metadata is added to both
test_extensions.py and test_presets.py, parametrized over the
five JSON non-mapping root types ([], "oops", 42,
true, null). Each variant plants the metadata file with that
exact content and asserts is_cache_valid() returns False
without raising. The parametrize covers every JSON type the public
spec allows at the root, so a regression that drops AttributeError
from any except tuple is caught against every observable shape rather
than relying on the next reviewer to remember the .get /
non-mapping interaction.
pytest tests/test_extensions.py tests/test_presets.py — 502
passed (was 492 before; the parametrize adds five vectors per file).
* fix(catalogs): make cache writes best-effort to match read-side contract
This commit is contained in:
Quratulain-bilal
@@ -3087,6 +3087,424 @@ class TestExtensionCatalog:
|
||||
|
||||
assert captured["req"].get_header("Authorization") == "Bearer ghp_testtoken"
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"payload",
|
||||
[
|
||||
# Root is not a JSON object.
|
||||
[],
|
||||
"oops",
|
||||
42,
|
||||
None,
|
||||
# Root is fine but ``extensions`` is the wrong type.
|
||||
{"schema_version": "1.0", "extensions": []},
|
||||
{"schema_version": "1.0", "extensions": "oops"},
|
||||
{"schema_version": "1.0", "extensions": None},
|
||||
{"schema_version": "1.0", "extensions": 42},
|
||||
],
|
||||
)
|
||||
def test_fetch_single_catalog_rejects_malformed_payload(self, temp_dir, payload):
|
||||
"""Malformed catalog payloads raise ExtensionError, not AttributeError.
|
||||
|
||||
Without this guard, a payload like ``{"extensions": []}`` would pass the
|
||||
key-presence check and then crash with ``AttributeError: 'list' object
|
||||
has no attribute 'items'`` deep inside ``_get_merged_extensions``. The
|
||||
sibling integration catalog reader already validates both the root
|
||||
object and the nested mapping (see ``integrations/catalog.py``); the
|
||||
extension catalog must stay consistent.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
catalog = self._make_catalog(temp_dir)
|
||||
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(payload).encode()
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
entry = CatalogEntry(
|
||||
url="https://example.com/catalog.json",
|
||||
name="default",
|
||||
priority=1,
|
||||
install_allowed=True,
|
||||
)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response):
|
||||
with pytest.raises(ExtensionError, match="Invalid catalog format"):
|
||||
catalog._fetch_single_catalog(entry, force_refresh=True)
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"cached_payload",
|
||||
[
|
||||
[],
|
||||
"oops",
|
||||
42,
|
||||
None,
|
||||
{"schema_version": "1.0", "extensions": []},
|
||||
{"schema_version": "1.0", "extensions": "oops"},
|
||||
{"schema_version": "1.0", "extensions": None},
|
||||
],
|
||||
)
|
||||
def test_fetch_single_catalog_rejects_malformed_cached_payload(
|
||||
self, temp_dir, cached_payload
|
||||
):
|
||||
"""A poisoned cache silently falls back to the network instead of
|
||||
crashing — cached payloads pass through the same shape validation
|
||||
as freshly-fetched ones.
|
||||
|
||||
Without this, a cache poisoned by an older spec-kit version (or a
|
||||
manual edit, or an upstream that briefly served a bad payload
|
||||
before the network guards landed) would re-crash every invocation
|
||||
of ``_get_merged_extensions`` despite the cache being "valid" by
|
||||
age. The recovery contract is: if the cached payload fails
|
||||
validation, drop it and refetch — never propagate
|
||||
``AttributeError`` to the caller.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
catalog = self._make_catalog(temp_dir)
|
||||
|
||||
# Poison the default-URL cache. ``DEFAULT_CATALOG_URL`` is the
|
||||
# branch that goes through ``is_cache_valid()`` (the non-default
|
||||
# branch uses per-URL hashed cache files but the same code path
|
||||
# below).
|
||||
catalog.cache_dir.mkdir(parents=True, exist_ok=True)
|
||||
catalog.cache_file.write_text(json.dumps(cached_payload))
|
||||
catalog.cache_metadata_file.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"cached_at": datetime.now(timezone.utc).isoformat(),
|
||||
"catalog_url": ExtensionCatalog.DEFAULT_CATALOG_URL,
|
||||
}
|
||||
)
|
||||
)
|
||||
|
||||
# Network refetch returns a valid payload so the recovery path
|
||||
# can complete.
|
||||
valid = {
|
||||
"schema_version": "1.0",
|
||||
"extensions": {"foo": {"name": "Foo", "version": "1.0.0"}},
|
||||
}
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(valid).encode()
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
entry = CatalogEntry(
|
||||
url=ExtensionCatalog.DEFAULT_CATALOG_URL,
|
||||
name="default",
|
||||
priority=1,
|
||||
install_allowed=True,
|
||||
)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response):
|
||||
result = catalog._fetch_single_catalog(entry, force_refresh=False)
|
||||
|
||||
# The poisoned cache was discarded and the network payload returned.
|
||||
assert result == valid
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"payload",
|
||||
[
|
||||
# Root is not a JSON object.
|
||||
[],
|
||||
"oops",
|
||||
42,
|
||||
None,
|
||||
# Root is fine but ``extensions`` is the wrong type.
|
||||
{"schema_version": "1.0", "extensions": []},
|
||||
{"schema_version": "1.0", "extensions": "oops"},
|
||||
{"schema_version": "1.0", "extensions": None},
|
||||
],
|
||||
)
|
||||
def test_fetch_catalog_rejects_malformed_payload(self, temp_dir, payload):
|
||||
"""Legacy ``fetch_catalog`` reuses the same shape-validation helper.
|
||||
|
||||
Before this change ``fetch_catalog`` only checked key presence — so
|
||||
a payload like ``42`` would crash with
|
||||
``TypeError: argument of type 'int' is not iterable`` during the
|
||||
``"schema_version" in catalog_data`` check, and an entry mapping
|
||||
of the wrong type would crash downstream. Reusing
|
||||
``_validate_catalog_payload`` keeps the network-side behaviour of
|
||||
the legacy single-catalog method consistent with the multi-catalog
|
||||
``_fetch_single_catalog`` path.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
catalog = self._make_catalog(temp_dir)
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(payload).encode()
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response):
|
||||
with pytest.raises(ExtensionError, match="Invalid catalog format"):
|
||||
catalog.fetch_catalog(force_refresh=True)
|
||||
|
||||
def test_fetch_catalog_recovers_from_unreadable_cache(self, temp_dir):
|
||||
"""An unreadable / wrong-encoded cache file silently refetches.
|
||||
|
||||
The cache contract is best-effort: a JSON-decode failure, an OS
|
||||
read failure (permissions / disk / handle limit), or an invalid
|
||||
text encoding on a cache file written by an older client must
|
||||
all fall through to the network fetch rather than crash the
|
||||
caller. Covers Copilot's review point that the previous
|
||||
``except (json.JSONDecodeError,)`` was too narrow.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
catalog = self._make_catalog(temp_dir)
|
||||
# Write invalid UTF-8 bytes to the cache file so ``read_text``
|
||||
# raises ``UnicodeDecodeError`` (a subclass of ``UnicodeError``).
|
||||
catalog.cache_dir.mkdir(parents=True, exist_ok=True)
|
||||
catalog.cache_file.write_bytes(b"\xff\xfe\x00not-utf-8")
|
||||
catalog.cache_metadata_file.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"cached_at": datetime.now(timezone.utc).isoformat(),
|
||||
"catalog_url": ExtensionCatalog.DEFAULT_CATALOG_URL,
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
valid = {
|
||||
"schema_version": "1.0",
|
||||
"extensions": {"foo": {"name": "Foo", "version": "1.0.0"}},
|
||||
}
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(valid).encode()
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response):
|
||||
result = catalog.fetch_catalog(force_refresh=False)
|
||||
|
||||
# Recovered via network rather than crashing on the unreadable cache.
|
||||
assert result == valid
|
||||
|
||||
def test_fetch_catalog_recovers_from_unreadable_metadata(self, temp_dir):
|
||||
"""A wrongly-encoded metadata file degrades to a cache miss.
|
||||
|
||||
``is_cache_valid`` is consulted *before* the cache payload is
|
||||
read; if the metadata file itself can't be decoded (e.g. it was
|
||||
written on a Windows host whose default codec isn't UTF-8) the
|
||||
validity check must return ``False`` rather than propagate
|
||||
``UnicodeDecodeError``. Without that guard, a corrupted metadata
|
||||
file would crash every invocation instead of falling through to
|
||||
a network refetch.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
catalog = self._make_catalog(temp_dir)
|
||||
catalog.cache_dir.mkdir(parents=True, exist_ok=True)
|
||||
catalog.cache_file.write_text("{}", encoding="utf-8")
|
||||
# Bytes that are not valid UTF-8 — ``read_text(encoding="utf-8")``
|
||||
# will raise ``UnicodeDecodeError`` (subclass of ``UnicodeError``).
|
||||
catalog.cache_metadata_file.write_bytes(b"\xff\xfe\x00bad")
|
||||
|
||||
# is_cache_valid must absorb the decode failure, not crash.
|
||||
assert catalog.is_cache_valid() is False
|
||||
|
||||
valid = {
|
||||
"schema_version": "1.0",
|
||||
"extensions": {"foo": {"name": "Foo", "version": "1.0.0"}},
|
||||
}
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(valid).encode()
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response):
|
||||
result = catalog.fetch_catalog(force_refresh=False)
|
||||
|
||||
assert result == valid
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"non_mapping_metadata",
|
||||
[
|
||||
"[]", # JSON array
|
||||
'"oops"', # JSON string
|
||||
"42", # JSON number
|
||||
"true", # JSON bool
|
||||
"null", # JSON null
|
||||
],
|
||||
)
|
||||
def test_is_cache_valid_handles_non_mapping_metadata(
|
||||
self, temp_dir, non_mapping_metadata
|
||||
):
|
||||
"""Metadata that parses to a non-mapping degrades to cache-invalid.
|
||||
|
||||
The cache-validity check calls ``metadata.get("cached_at", "")``
|
||||
immediately after ``json.loads``. If the metadata file is valid
|
||||
JSON but parses to a non-mapping (``[]``, ``"oops"``, ``42``,
|
||||
``true``, ``null``), ``.get`` raises ``AttributeError`` — which
|
||||
previously slipped past the except tuple and crashed the
|
||||
caller. The contract documented on ``is_cache_valid`` says any
|
||||
decode/shape failure should return ``False`` so ``fetch_catalog``
|
||||
falls through to a network refetch. This test pins that
|
||||
contract across every JSON non-mapping root type so a regression
|
||||
in the except clause can't silently re-introduce the crash.
|
||||
"""
|
||||
catalog = self._make_catalog(temp_dir)
|
||||
catalog.cache_dir.mkdir(parents=True, exist_ok=True)
|
||||
catalog.cache_file.write_text("{}", encoding="utf-8")
|
||||
catalog.cache_metadata_file.write_text(
|
||||
non_mapping_metadata, encoding="utf-8"
|
||||
)
|
||||
|
||||
# Must not raise — the contract is "any decode/shape failure → False".
|
||||
assert catalog.is_cache_valid() is False
|
||||
|
||||
def test_fetch_catalog_writes_cache_as_utf8(self, temp_dir, monkeypatch):
|
||||
"""Cache + metadata writes pass ``encoding="utf-8"``, observably.
|
||||
|
||||
The earlier version of this test claimed to assert UTF-8 at the
|
||||
byte level but actually only round-tripped a non-ASCII string
|
||||
through ``json.dumps`` and ``read_text(encoding="utf-8")``.
|
||||
Because ``json.dumps`` defaults to ``ensure_ascii=True``, "café"
|
||||
was serialized as the all-ASCII escape ``caf\\u00e9`` before it
|
||||
ever reached ``write_text`` — the bytes on disk were identical
|
||||
regardless of the encoding kwarg, so a locale-encoded write
|
||||
would have round-tripped just fine. The drift Copilot's review
|
||||
flagged wasn't actually being caught.
|
||||
|
||||
Fix: directly observe the ``encoding`` argument passed to every
|
||||
``write_text`` call made against the cache directory. This is
|
||||
the production code's encoding choice, which is exactly what
|
||||
the regression guard cares about; non-ASCII payload tricks are
|
||||
unnecessary because the assertion is about the kwarg, not the
|
||||
bytes.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
from pathlib import Path as _PathCls
|
||||
|
||||
catalog = self._make_catalog(temp_dir)
|
||||
payload = {
|
||||
"schema_version": "1.0",
|
||||
"extensions": {"foo": {"name": "Foo", "version": "1.0.0"}},
|
||||
}
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(payload).encode("utf-8")
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Record every ``write_text`` call's encoding kwarg so the
|
||||
# assertion observes the production writer's argument directly.
|
||||
recorded: list[dict] = []
|
||||
real_write_text = _PathCls.write_text
|
||||
|
||||
def recording_write_text(self, data, *args, **kwargs):
|
||||
recorded.append(
|
||||
{"path": str(self), "encoding": kwargs.get("encoding")}
|
||||
)
|
||||
return real_write_text(self, data, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(_PathCls, "write_text", recording_write_text)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response):
|
||||
catalog.fetch_catalog(force_refresh=True)
|
||||
|
||||
# Filter to writes inside the catalog's cache directory so
|
||||
# unrelated writes from other machinery don't pollute the
|
||||
# assertion.
|
||||
cache_writes = [
|
||||
r for r in recorded if str(catalog.cache_dir) in r["path"]
|
||||
]
|
||||
assert cache_writes, "fetch_catalog made no writes to the cache dir"
|
||||
for record in cache_writes:
|
||||
assert record["encoding"] == "utf-8", (
|
||||
f"write_text on {record['path']} used encoding "
|
||||
f"{record['encoding']!r}; expected 'utf-8'"
|
||||
)
|
||||
|
||||
def test_fetch_catalog_survives_unwritable_cache(self, temp_dir, monkeypatch):
|
||||
"""An unwritable cache dir doesn't fail a successful fetch.
|
||||
|
||||
Cache writes are best-effort, mirroring the read side and the
|
||||
``integrations/catalog.py`` precedent: if ``mkdir``/``write_text``
|
||||
raises ``OSError`` (read-only checkout, permissions), the
|
||||
already-fetched-and-validated payload must still be returned
|
||||
rather than surfacing the cache failure to the caller.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
from pathlib import Path as _PathCls
|
||||
|
||||
catalog = self._make_catalog(temp_dir)
|
||||
valid = {
|
||||
"schema_version": "1.0",
|
||||
"extensions": {"foo": {"name": "Foo", "version": "1.0.0"}},
|
||||
}
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(valid).encode()
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Simulate an unwritable cache dir: every write_text under the
|
||||
# cache directory raises PermissionError (an OSError subclass).
|
||||
real_write_text = _PathCls.write_text
|
||||
|
||||
def failing_write_text(self, data, *args, **kwargs):
|
||||
if str(catalog.cache_dir) in str(self):
|
||||
raise PermissionError("cache dir is read-only")
|
||||
return real_write_text(self, data, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(_PathCls, "write_text", failing_write_text)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response):
|
||||
# Legacy single-catalog path.
|
||||
assert catalog.fetch_catalog(force_refresh=True) == valid
|
||||
|
||||
# Multi-catalog path.
|
||||
entry = CatalogEntry(
|
||||
url="https://example.com/catalog.json",
|
||||
name="default",
|
||||
priority=1,
|
||||
install_allowed=True,
|
||||
)
|
||||
assert catalog._fetch_single_catalog(entry, force_refresh=True) == valid
|
||||
|
||||
def test_get_merged_extensions_skips_non_mapping_entries(self, temp_dir):
|
||||
"""Per-entry guard: one malformed entry shouldn't poison the merge.
|
||||
|
||||
``_fetch_single_catalog`` validates that ``extensions`` is a mapping,
|
||||
but it doesn't (and shouldn't) validate every entry inside it — a
|
||||
single bad entry in an otherwise-valid catalog should be skipped, not
|
||||
crash the whole resolve path. Mirrors the per-entry skip in
|
||||
``integrations/catalog.py``: a malformed entry returns no error,
|
||||
valid entries continue to merge normally.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
catalog = self._make_catalog(temp_dir)
|
||||
# Mix of valid entry, list-shaped entry, and string-shaped entry.
|
||||
payload = {
|
||||
"schema_version": "1.0",
|
||||
"extensions": {
|
||||
"good": {"name": "Good", "version": "1.0.0"},
|
||||
"bad-list": [],
|
||||
"bad-str": "oops",
|
||||
},
|
||||
}
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(payload).encode()
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
entry = CatalogEntry(
|
||||
url="https://example.com/catalog.json",
|
||||
name="default",
|
||||
priority=1,
|
||||
install_allowed=True,
|
||||
)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response), \
|
||||
patch.object(catalog, "get_active_catalogs", return_value=[entry]):
|
||||
merged = catalog._get_merged_extensions(force_refresh=True)
|
||||
|
||||
# Only the well-formed entry survives; the two malformed entries are
|
||||
# silently dropped rather than raising or crashing.
|
||||
assert [ext["id"] for ext in merged] == ["good"]
|
||||
|
||||
def test_download_extension_sends_auth_header(self, temp_dir, monkeypatch):
|
||||
"""download_extension passes Authorization header when a provider is configured."""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
@@ -1514,6 +1514,421 @@ class TestPresetCatalog:
|
||||
|
||||
assert captured["req"].get_header("Authorization") == "Bearer ghp_testtoken"
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"payload",
|
||||
[
|
||||
# Root is not a JSON object.
|
||||
[],
|
||||
"oops",
|
||||
42,
|
||||
None,
|
||||
# Root is fine but ``presets`` is the wrong type.
|
||||
{"schema_version": "1.0", "presets": []},
|
||||
{"schema_version": "1.0", "presets": "oops"},
|
||||
{"schema_version": "1.0", "presets": None},
|
||||
{"schema_version": "1.0", "presets": 42},
|
||||
],
|
||||
)
|
||||
def test_fetch_single_catalog_rejects_malformed_payload(self, project_dir, payload):
|
||||
"""Malformed catalog payloads raise PresetError, not AttributeError.
|
||||
|
||||
Without this guard, a payload like ``{"presets": []}`` would pass the
|
||||
key-presence check and then crash with ``AttributeError: 'list' object
|
||||
has no attribute 'items'`` deep inside ``_get_merged_packs``. The
|
||||
sibling integration catalog reader already validates both the root
|
||||
object and the nested mapping (see ``integrations/catalog.py``); the
|
||||
preset catalog must stay consistent.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
catalog = PresetCatalog(project_dir)
|
||||
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(payload).encode()
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
entry = PresetCatalogEntry(
|
||||
url="https://example.com/catalog.json",
|
||||
name="default",
|
||||
priority=1,
|
||||
install_allowed=True,
|
||||
)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response):
|
||||
with pytest.raises(PresetError, match="Invalid preset catalog format"):
|
||||
catalog._fetch_single_catalog(entry, force_refresh=True)
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"cached_payload",
|
||||
[
|
||||
[],
|
||||
"oops",
|
||||
42,
|
||||
None,
|
||||
{"schema_version": "1.0", "presets": []},
|
||||
{"schema_version": "1.0", "presets": "oops"},
|
||||
{"schema_version": "1.0", "presets": None},
|
||||
],
|
||||
)
|
||||
def test_fetch_single_catalog_rejects_malformed_cached_payload(
|
||||
self, project_dir, cached_payload
|
||||
):
|
||||
"""A poisoned cache silently falls back to the network instead of
|
||||
crashing — cached payloads pass through the same shape validation
|
||||
as freshly-fetched ones.
|
||||
|
||||
Without this, a cache poisoned by an older spec-kit version (or a
|
||||
manual edit, or an upstream that briefly served a bad payload
|
||||
before the network guards landed) would re-crash every invocation
|
||||
of ``_get_merged_packs`` despite the cache being "valid" by age.
|
||||
The recovery contract is: if the cached payload fails validation,
|
||||
drop it and refetch — never propagate ``AttributeError`` to the
|
||||
caller.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
catalog = PresetCatalog(project_dir)
|
||||
|
||||
# Poison the default-URL cache. ``DEFAULT_CATALOG_URL`` and
|
||||
# non-default URLs both flow through the same cache-load branch.
|
||||
cache_file, metadata_file = catalog._get_cache_paths(
|
||||
catalog.DEFAULT_CATALOG_URL
|
||||
)
|
||||
cache_file.parent.mkdir(parents=True, exist_ok=True)
|
||||
cache_file.write_text(json.dumps(cached_payload))
|
||||
metadata_file.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"cached_at": datetime.now(timezone.utc).isoformat(),
|
||||
"catalog_url": catalog.DEFAULT_CATALOG_URL,
|
||||
}
|
||||
)
|
||||
)
|
||||
|
||||
# Network refetch returns a valid payload so the recovery path
|
||||
# can complete.
|
||||
valid = {
|
||||
"schema_version": "1.0",
|
||||
"presets": {"foo": {"name": "Foo", "version": "1.0.0"}},
|
||||
}
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(valid).encode()
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
entry = PresetCatalogEntry(
|
||||
url=catalog.DEFAULT_CATALOG_URL,
|
||||
name="default",
|
||||
priority=1,
|
||||
install_allowed=True,
|
||||
)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response):
|
||||
result = catalog._fetch_single_catalog(entry, force_refresh=False)
|
||||
|
||||
# The poisoned cache was discarded and the network payload returned.
|
||||
assert result == valid
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"payload",
|
||||
[
|
||||
# Root is not a JSON object.
|
||||
[],
|
||||
"oops",
|
||||
42,
|
||||
None,
|
||||
# Root is fine but ``presets`` is the wrong type.
|
||||
{"schema_version": "1.0", "presets": []},
|
||||
{"schema_version": "1.0", "presets": "oops"},
|
||||
{"schema_version": "1.0", "presets": None},
|
||||
],
|
||||
)
|
||||
def test_fetch_catalog_rejects_malformed_payload(self, project_dir, payload):
|
||||
"""Legacy ``fetch_catalog`` reuses the same shape-validation helper.
|
||||
|
||||
Before this change ``fetch_catalog`` only checked key presence —
|
||||
so a payload like ``42`` would crash with
|
||||
``TypeError: argument of type 'int' is not iterable`` during the
|
||||
``"schema_version" in catalog_data`` check, and an entry mapping
|
||||
of the wrong type would crash downstream. Reusing
|
||||
``_validate_catalog_payload`` keeps the network-side behaviour of
|
||||
the legacy single-catalog method consistent with the multi-catalog
|
||||
``_fetch_single_catalog`` path.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
catalog = PresetCatalog(project_dir)
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(payload).encode()
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response):
|
||||
with pytest.raises(PresetError, match="Invalid preset catalog format"):
|
||||
catalog.fetch_catalog(force_refresh=True)
|
||||
|
||||
def test_fetch_catalog_recovers_from_unreadable_cache(self, project_dir):
|
||||
"""An unreadable / wrong-encoded cache file silently refetches.
|
||||
|
||||
The cache contract is best-effort: a JSON-decode failure, an OS
|
||||
read failure (permissions / disk / handle limit), or an invalid
|
||||
text encoding on a cache file written by an older client must
|
||||
all fall through to the network fetch rather than crash the
|
||||
caller. Covers Copilot's review point that the previous
|
||||
``except (json.JSONDecodeError, OSError)`` was missing
|
||||
``UnicodeError``.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
catalog = PresetCatalog(project_dir)
|
||||
catalog.cache_dir.mkdir(parents=True, exist_ok=True)
|
||||
# Invalid UTF-8 bytes so ``read_text`` raises ``UnicodeDecodeError``
|
||||
# (a subclass of ``UnicodeError``).
|
||||
catalog.cache_file.write_bytes(b"\xff\xfe\x00not-utf-8")
|
||||
catalog.cache_metadata_file.write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"cached_at": datetime.now(timezone.utc).isoformat(),
|
||||
"catalog_url": catalog.get_catalog_url(),
|
||||
}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
valid = {
|
||||
"schema_version": "1.0",
|
||||
"presets": {"foo": {"name": "Foo", "version": "1.0.0"}},
|
||||
}
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(valid).encode()
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response):
|
||||
result = catalog.fetch_catalog(force_refresh=False)
|
||||
|
||||
# Recovered via network rather than crashing on the unreadable cache.
|
||||
assert result == valid
|
||||
|
||||
def test_fetch_catalog_recovers_from_unreadable_metadata(self, project_dir):
|
||||
"""A wrongly-encoded metadata file degrades to a cache miss.
|
||||
|
||||
``is_cache_valid`` is consulted *before* the cache payload is
|
||||
read; if the metadata file itself can't be decoded (e.g. it was
|
||||
written on a host whose default codec isn't UTF-8) the validity
|
||||
check must return ``False`` rather than propagate
|
||||
``UnicodeDecodeError``. Without that guard, a corrupted metadata
|
||||
file would crash every invocation instead of falling through to
|
||||
a network refetch.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
catalog = PresetCatalog(project_dir)
|
||||
catalog.cache_dir.mkdir(parents=True, exist_ok=True)
|
||||
catalog.cache_file.write_text("{}", encoding="utf-8")
|
||||
# Bytes that are not valid UTF-8 — ``read_text(encoding="utf-8")``
|
||||
# will raise ``UnicodeDecodeError`` (subclass of ``UnicodeError``).
|
||||
catalog.cache_metadata_file.write_bytes(b"\xff\xfe\x00bad")
|
||||
|
||||
# is_cache_valid must absorb the decode failure, not crash.
|
||||
assert catalog.is_cache_valid() is False
|
||||
|
||||
valid = {
|
||||
"schema_version": "1.0",
|
||||
"presets": {"foo": {"name": "Foo", "version": "1.0.0"}},
|
||||
}
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(valid).encode()
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response):
|
||||
result = catalog.fetch_catalog(force_refresh=False)
|
||||
|
||||
assert result == valid
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"non_mapping_metadata",
|
||||
[
|
||||
"[]", # JSON array
|
||||
'"oops"', # JSON string
|
||||
"42", # JSON number
|
||||
"true", # JSON bool
|
||||
"null", # JSON null
|
||||
],
|
||||
)
|
||||
def test_is_cache_valid_handles_non_mapping_metadata(
|
||||
self, project_dir, non_mapping_metadata
|
||||
):
|
||||
"""Metadata that parses to a non-mapping degrades to cache-invalid.
|
||||
|
||||
The cache-validity check calls ``metadata.get("cached_at", "")``
|
||||
immediately after ``json.loads``. If the metadata file is valid
|
||||
JSON but parses to a non-mapping (``[]``, ``"oops"``, ``42``,
|
||||
``true``, ``null``), ``.get`` raises ``AttributeError`` — which
|
||||
previously slipped past the except tuple and crashed the
|
||||
caller. The contract documented on ``is_cache_valid`` says any
|
||||
decode/shape failure should return ``False`` so ``fetch_catalog``
|
||||
falls through to a network refetch. This test pins that
|
||||
contract across every JSON non-mapping root type.
|
||||
"""
|
||||
catalog = PresetCatalog(project_dir)
|
||||
catalog.cache_dir.mkdir(parents=True, exist_ok=True)
|
||||
catalog.cache_file.write_text("{}", encoding="utf-8")
|
||||
catalog.cache_metadata_file.write_text(
|
||||
non_mapping_metadata, encoding="utf-8"
|
||||
)
|
||||
|
||||
# Must not raise — the contract is "any decode/shape failure → False".
|
||||
assert catalog.is_cache_valid() is False
|
||||
|
||||
def test_fetch_catalog_writes_cache_as_utf8(self, project_dir, monkeypatch):
|
||||
"""Cache + metadata writes pass ``encoding="utf-8"``, observably.
|
||||
|
||||
The earlier version of this test claimed to assert UTF-8 at the
|
||||
byte level but actually only round-tripped a non-ASCII string
|
||||
through ``json.dumps`` and ``read_text(encoding="utf-8")``.
|
||||
Because ``json.dumps`` defaults to ``ensure_ascii=True``, "café"
|
||||
was serialized as the all-ASCII escape ``caf\\u00e9`` before it
|
||||
ever reached ``write_text`` — the bytes on disk were identical
|
||||
regardless of the encoding kwarg. The drift Copilot's review
|
||||
flagged wasn't actually being caught.
|
||||
|
||||
Fix: directly observe the ``encoding`` argument passed to every
|
||||
``write_text`` call made against the cache directory. This is
|
||||
the production code's encoding choice, which is exactly what
|
||||
the regression guard cares about.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
from pathlib import Path as _PathCls
|
||||
|
||||
catalog = PresetCatalog(project_dir)
|
||||
payload = {
|
||||
"schema_version": "1.0",
|
||||
"presets": {"foo": {"name": "Foo", "version": "1.0.0"}},
|
||||
}
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(payload).encode("utf-8")
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Record every ``write_text`` call's encoding kwarg so the
|
||||
# assertion observes the production writer's argument directly.
|
||||
recorded: list[dict] = []
|
||||
real_write_text = _PathCls.write_text
|
||||
|
||||
def recording_write_text(self, data, *args, **kwargs):
|
||||
recorded.append(
|
||||
{"path": str(self), "encoding": kwargs.get("encoding")}
|
||||
)
|
||||
return real_write_text(self, data, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(_PathCls, "write_text", recording_write_text)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response):
|
||||
catalog.fetch_catalog(force_refresh=True)
|
||||
|
||||
cache_writes = [
|
||||
r for r in recorded if str(catalog.cache_dir) in r["path"]
|
||||
]
|
||||
assert cache_writes, "fetch_catalog made no writes to the cache dir"
|
||||
for record in cache_writes:
|
||||
assert record["encoding"] == "utf-8", (
|
||||
f"write_text on {record['path']} used encoding "
|
||||
f"{record['encoding']!r}; expected 'utf-8'"
|
||||
)
|
||||
|
||||
def test_fetch_catalog_survives_unwritable_cache(self, project_dir, monkeypatch):
|
||||
"""An unwritable cache dir doesn't fail a successful fetch.
|
||||
|
||||
Cache writes are best-effort, mirroring the read side and the
|
||||
``integrations/catalog.py`` precedent: if ``mkdir``/``write_text``
|
||||
raises ``OSError`` (read-only checkout, permissions), the
|
||||
already-fetched-and-validated payload must still be returned —
|
||||
not swallowed into the broad except and re-raised as a
|
||||
``PresetError``.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
from pathlib import Path as _PathCls
|
||||
|
||||
catalog = PresetCatalog(project_dir)
|
||||
valid = {
|
||||
"schema_version": "1.0",
|
||||
"presets": {"foo": {"name": "Foo", "version": "1.0.0"}},
|
||||
}
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(valid).encode()
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
# Simulate an unwritable cache dir: every write_text under the
|
||||
# cache directory raises PermissionError (an OSError subclass).
|
||||
real_write_text = _PathCls.write_text
|
||||
|
||||
def failing_write_text(self, data, *args, **kwargs):
|
||||
if str(catalog.cache_dir) in str(self):
|
||||
raise PermissionError("cache dir is read-only")
|
||||
return real_write_text(self, data, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(_PathCls, "write_text", failing_write_text)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response):
|
||||
# Legacy single-catalog path.
|
||||
assert catalog.fetch_catalog(force_refresh=True) == valid
|
||||
|
||||
# Multi-catalog path.
|
||||
entry = PresetCatalogEntry(
|
||||
url=catalog.DEFAULT_CATALOG_URL,
|
||||
name="default",
|
||||
priority=1,
|
||||
install_allowed=True,
|
||||
)
|
||||
assert (
|
||||
catalog._fetch_single_catalog(entry, force_refresh=True) == valid
|
||||
)
|
||||
|
||||
def test_get_merged_packs_skips_non_mapping_entries(self, project_dir):
|
||||
"""Per-entry guard: one malformed entry shouldn't poison the merge.
|
||||
|
||||
``_fetch_single_catalog`` validates that ``presets`` is a mapping,
|
||||
but it doesn't (and shouldn't) validate every entry inside it — a
|
||||
single bad entry in an otherwise-valid catalog should be skipped,
|
||||
not crash the whole resolve path. Mirrors the per-entry skip in
|
||||
``integrations/catalog.py``: a malformed entry returns no error,
|
||||
valid entries continue to merge normally.
|
||||
"""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
catalog = PresetCatalog(project_dir)
|
||||
payload = {
|
||||
"schema_version": "1.0",
|
||||
"presets": {
|
||||
"good": {"name": "Good", "version": "1.0.0"},
|
||||
"bad-list": [],
|
||||
"bad-str": "oops",
|
||||
},
|
||||
}
|
||||
mock_response = MagicMock()
|
||||
mock_response.read.return_value = json.dumps(payload).encode()
|
||||
mock_response.__enter__ = lambda s: s
|
||||
mock_response.__exit__ = MagicMock(return_value=False)
|
||||
|
||||
entry = PresetCatalogEntry(
|
||||
url="https://example.com/catalog.json",
|
||||
name="default",
|
||||
priority=1,
|
||||
install_allowed=True,
|
||||
)
|
||||
|
||||
with patch.object(catalog, "_open_url", return_value=mock_response), \
|
||||
patch.object(catalog, "get_active_catalogs", return_value=[entry]):
|
||||
merged = catalog._get_merged_packs(force_refresh=True)
|
||||
|
||||
# Only the well-formed entry survives; the two malformed entries are
|
||||
# silently dropped rather than raising or crashing.
|
||||
assert list(merged.keys()) == ["good"]
|
||||
|
||||
def test_download_pack_sends_auth_header(self, project_dir, monkeypatch):
|
||||
"""download_pack passes Authorization header when configured."""
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
Reference in New Issue
Block a user