mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
fix(extensions): tolerate non-string catalog name in display-name lookup (#3747)
* fix(extensions): tolerate non-string catalog name in display-name lookup
_resolve_catalog_extension() filters catalog search results by display
name with `ext["name"].lower() == argument.lower()`. Extension catalog
JSON is user-editable, so a hand-authored non-string name (e.g.
`name: 123`) crashes the filter with `AttributeError: 'int' object has
no attribute 'lower'`, taking down `extension info <name>` and
`extension add <name>`. A missing `name` key would likewise KeyError.
Coerce defensively with `str(ext.get("name", "")).lower()`, matching the
ambiguous-match display block just below (which already str()-coerces
name for the same reason). A bad-named entry simply doesn't match,
yielding a clean not-found error instead of a traceback.
Adds a regression test invoking `extension info <name>` against a
mocked catalog whose search result has `name: 123`; it fails pre-fix
with AttributeError.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -166,9 +166,17 @@ def _resolve_catalog_extension(
|
||||
if ext_info:
|
||||
return (ext_info, None)
|
||||
|
||||
# Try by display name - search using argument as query, then filter for exact match
|
||||
search_results = catalog.search(query=argument)
|
||||
name_matches = [ext for ext in search_results if ext["name"].lower() == argument.lower()]
|
||||
# Try by display name - search using argument as query, then filter for exact match.
|
||||
# Coerce name defensively: catalog JSON is user-editable, so a hand-authored
|
||||
# non-string/missing name must not crash the match (the ambiguous-match display
|
||||
# below already str()-coerces name for the same reason).
|
||||
search_results = catalog.search()
|
||||
argument_lower = argument.lower()
|
||||
name_matches = [
|
||||
ext
|
||||
for ext in search_results
|
||||
if str(ext.get("name", "")).lower() == argument_lower
|
||||
]
|
||||
|
||||
if len(name_matches) == 1:
|
||||
return (name_matches[0], None)
|
||||
|
||||
@@ -6812,6 +6812,55 @@ class TestExtensionAddCLI:
|
||||
f"but was called with '{download_called_with[0]}'"
|
||||
)
|
||||
|
||||
def test_info_by_name_tolerates_non_string_catalog_name(self, tmp_path):
|
||||
"""Display-name resolution must not crash on a non-string catalog name.
|
||||
|
||||
Catalog JSON is user-editable, so ``catalog.search()`` may return an
|
||||
entry whose ``name`` is a non-string (e.g. ``name: 123``). The
|
||||
display-name filter calls ``.lower()`` on it; without coercion this
|
||||
raises ``AttributeError`` and takes down ``extension info``/``add``.
|
||||
The entry with the bad name must simply not match, yielding a clean
|
||||
"not found" rather than a traceback.
|
||||
"""
|
||||
from typer.testing import CliRunner
|
||||
from unittest.mock import patch, MagicMock
|
||||
from specify_cli import app
|
||||
|
||||
runner = CliRunner()
|
||||
|
||||
project_dir = tmp_path / "test-project"
|
||||
project_dir.mkdir()
|
||||
(project_dir / ".specify").mkdir()
|
||||
(project_dir / ".specify" / "extensions").mkdir(parents=True)
|
||||
|
||||
# Catalog search returns an entry with a non-string name.
|
||||
mock_catalog = MagicMock()
|
||||
mock_catalog.get_extension_info.return_value = None # ID lookup fails
|
||||
mock_catalog.search.return_value = [
|
||||
{
|
||||
"id": "acme-thing",
|
||||
"name": 123,
|
||||
"version": "1.0.0",
|
||||
"description": "A thing",
|
||||
"_install_allowed": True,
|
||||
}
|
||||
]
|
||||
|
||||
with patch("specify_cli.extensions.ExtensionCatalog", return_value=mock_catalog), \
|
||||
patch.object(Path, "cwd", return_value=project_dir):
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["extension", "info", "Some Name"],
|
||||
catch_exceptions=True,
|
||||
)
|
||||
|
||||
# Must not crash with AttributeError; the bad-named entry just doesn't
|
||||
# match, so resolution ends as a clean not-found error exit.
|
||||
assert not isinstance(result.exception, AttributeError), (
|
||||
f"non-string catalog name crashed resolution: {result.exception!r}"
|
||||
)
|
||||
assert result.exit_code != 0
|
||||
|
||||
def test_add_bundled_extension_not_found_gives_clear_error(self, tmp_path):
|
||||
"""extension add should give a clear error when a bundled extension is not found locally."""
|
||||
from typer.testing import CliRunner
|
||||
|
||||
Reference in New Issue
Block a user