Skip to content

Commit 9a5709e

Browse files
committed
fix(extensions): enforce trusted catalog identity boundaries
Catalog entries can originate from remote/community sources and cached state, so install and info resolution must not coerce malformed fields into usable names or IDs. This keeps display-name lookup aligned with rendering, requires whole-string ID validation, and prevents default download cache writes through symlinked project paths.
1 parent 4c8abf3 commit 9a5709e

4 files changed

Lines changed: 178 additions & 9 deletions

File tree

src/specify_cli/extensions/__init__.py

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ def _validate(self):
239239
raise ValidationError(f"Missing extension.{field}")
240240

241241
# Validate extension ID format
242-
if not EXTENSION_ID_PATTERN.match(ext["id"]):
242+
if not EXTENSION_ID_PATTERN.fullmatch(ext["id"]):
243243
raise ValidationError(
244244
f"Invalid extension ID '{ext['id']}': "
245245
"must be lowercase alphanumeric with hyphens only"
@@ -2050,6 +2050,54 @@ def __init__(self, project_root: Path):
20502050
self.cache_file = self.cache_dir / "catalog.json"
20512051
self.cache_metadata_file = self.cache_dir / "catalog-metadata.json"
20522052

2053+
def _ensure_default_download_cache_dir(self) -> Path:
2054+
"""Create the default download cache without following symlink parents."""
2055+
root = self.project_root.resolve()
2056+
target_dir = self.cache_dir / "downloads"
2057+
try:
2058+
rel = target_dir.relative_to(self.project_root)
2059+
except ValueError:
2060+
raise ExtensionError("Default extension download cache escapes project root") from None
2061+
2062+
current = self.project_root
2063+
for part in rel.parts:
2064+
current = current / part
2065+
try:
2066+
label = current.relative_to(self.project_root).as_posix()
2067+
except ValueError:
2068+
label = str(current)
2069+
2070+
if current.is_symlink():
2071+
raise ExtensionError(
2072+
f"Refusing to use symlinked extension download cache path: {label}"
2073+
)
2074+
if current.exists():
2075+
if not current.is_dir():
2076+
raise ExtensionError(
2077+
f"Extension download cache path is not a directory: {label}"
2078+
)
2079+
try:
2080+
current.resolve().relative_to(root)
2081+
except (OSError, ValueError):
2082+
raise ExtensionError(
2083+
f"Extension download cache path escapes project root: {label}"
2084+
) from None
2085+
continue
2086+
2087+
current.mkdir()
2088+
if current.is_symlink():
2089+
raise ExtensionError(
2090+
f"Refusing to use symlinked extension download cache path: {label}"
2091+
)
2092+
try:
2093+
current.resolve().relative_to(root)
2094+
except (OSError, ValueError):
2095+
raise ExtensionError(
2096+
f"Extension download cache path escapes project root: {label}"
2097+
) from None
2098+
2099+
return target_dir
2100+
20532101
def _make_request(self, url: str):
20542102
"""Build a urllib Request, adding auth headers when a provider matches.
20552103
@@ -2611,7 +2659,7 @@ def download_extension(
26112659
"""
26122660
import urllib.error
26132661

2614-
if not isinstance(extension_id, str) or not EXTENSION_ID_PATTERN.match(
2662+
if not isinstance(extension_id, str) or not EXTENSION_ID_PATTERN.fullmatch(
26152663
extension_id
26162664
):
26172665
raise ExtensionError(
@@ -2648,8 +2696,9 @@ def download_extension(
26482696

26492697
# Determine target path
26502698
if target_dir is None:
2651-
target_dir = self.cache_dir / "downloads"
2652-
target_dir.mkdir(parents=True, exist_ok=True)
2699+
target_dir = self._ensure_default_download_cache_dir()
2700+
else:
2701+
target_dir.mkdir(parents=True, exist_ok=True)
26532702

26542703
version = _safe_version_token(ext_info.get("version"))
26552704
zip_filename = f"{extension_id}-{version}.zip"

src/specify_cli/extensions/_commands.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,11 @@ def _resolve_catalog_extension(
204204

205205
# Try by display name - search using argument as query, then filter for exact match
206206
search_results = catalog.search(query=argument)
207-
name_matches = [ext for ext in search_results if str(ext.get("name", "")).lower() == argument.lower()]
207+
name_matches = [
208+
ext
209+
for ext in search_results
210+
if _catalog_id(ext) and _catalog_str(ext, "name").lower() == argument.lower()
211+
]
208212

209213
if len(name_matches) == 1:
210214
return (name_matches[0], None)
@@ -221,10 +225,10 @@ def _resolve_catalog_extension(
221225
table.add_column("Catalog", style="dim")
222226
for ext in name_matches:
223227
table.add_row(
224-
_escape_markup(str(ext.get("id", ""))),
225-
_escape_markup(str(ext.get("name", ""))),
226-
_escape_markup(str(ext.get("version", ""))),
227-
_escape_markup(str(ext.get("_catalog_name", ""))),
228+
_escape_markup(_catalog_id(ext)),
229+
_escape_markup(_catalog_str(ext, "name", "(unnamed)")),
230+
_escape_markup(_catalog_str(ext, "version", "?")),
231+
_escape_markup(_catalog_str(ext, "_catalog_name")),
228232
)
229233
console.print(table)
230234
console.print("\nPlease rerun using the extension ID:")

tests/test_extension_catalog_robustness.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,58 @@ def fail_open_url(*args, **kwargs):
393393
catalog.download_extension(malicious_id)
394394

395395

396+
@pytest.mark.parametrize(
397+
("argument", "catalog_entry"),
398+
[
399+
(
400+
"none",
401+
{
402+
"id": "null-name",
403+
"name": None,
404+
"version": "1.0.0",
405+
"description": "JSON null name",
406+
},
407+
),
408+
(
409+
"123",
410+
{
411+
"id": "numeric-name",
412+
"name": 123,
413+
"version": "1.0.0",
414+
"description": "Numeric name",
415+
},
416+
),
417+
(
418+
"Hidden Name",
419+
{
420+
"id": "../bad",
421+
"name": "Hidden Name",
422+
"version": "1.0.0",
423+
"description": "Invalid ID",
424+
},
425+
),
426+
],
427+
)
428+
def test_info_display_name_resolution_ignores_malformed_catalog_entries(
429+
project_dir, monkeypatch, argument, catalog_entry
430+
):
431+
"""Display-name lookup must follow the same untrusted-entry rules as rendering.
432+
433+
JSON null / numeric names are unnamed, and invalid IDs are not installable
434+
or addressable even when their display name matches exactly.
435+
"""
436+
monkeypatch.chdir(project_dir)
437+
438+
monkeypatch.setattr(ExtensionManager, "list_installed", lambda self: [])
439+
monkeypatch.setattr(ExtensionCatalog, "get_extension_info", lambda self, ext_id: None)
440+
monkeypatch.setattr(ExtensionCatalog, "search", lambda self, **kwargs: [catalog_entry])
441+
442+
result = runner.invoke(app, ["extension", "info", argument], obj={"project_root": project_dir})
443+
444+
assert result.exit_code == 1, result.output
445+
assert f"Extension '{argument}' not found" in result.output
446+
447+
396448
def test_info_json_null_fields_render_placeholders_not_none(project_dir, monkeypatch):
397449
"""`extension info` must coalesce JSON null fields, not print "None".
398450

tests/test_extensions.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,21 @@ def test_invalid_extension_id(self, temp_dir, valid_manifest_data):
362362
with pytest.raises(ValidationError, match="Invalid extension ID"):
363363
ExtensionManifest(manifest_path)
364364

365+
def test_extension_id_with_trailing_newline_is_invalid(self, temp_dir, valid_manifest_data):
366+
"""A trailing newline must not pass the extension ID regex anchor."""
367+
import yaml
368+
369+
valid_manifest_data["extension"]["id"] = "valid-id\n"
370+
valid_manifest_data["provides"]["commands"] = []
371+
valid_manifest_data["hooks"] = {}
372+
373+
manifest_path = temp_dir / "extension.yml"
374+
with open(manifest_path, "w") as f:
375+
yaml.dump(valid_manifest_data, f)
376+
377+
with pytest.raises(ValidationError, match="Invalid extension ID"):
378+
ExtensionManifest(manifest_path)
379+
365380
def test_invalid_version(self, temp_dir, valid_manifest_data):
366381
"""Test manifest with invalid semantic version."""
367382
import yaml
@@ -5878,6 +5893,55 @@ def test_download_extension_raises_no_url_for_non_bundled(self, temp_dir):
58785893
with pytest.raises(ExtensionError, match="has no download URL"):
58795894
catalog.download_extension("some-ext")
58805895

5896+
def test_download_extension_rejects_trailing_newline_id(self, temp_dir):
5897+
"""download_extension must require the whole extension ID to match."""
5898+
project_dir = temp_dir / "project"
5899+
project_dir.mkdir()
5900+
(project_dir / ".specify").mkdir()
5901+
5902+
catalog = ExtensionCatalog(project_dir)
5903+
5904+
with pytest.raises(ExtensionError, match="Invalid extension ID"):
5905+
catalog.download_extension("valid-id\n")
5906+
5907+
def test_download_extension_rejects_symlinked_default_cache(self, temp_dir):
5908+
"""The default download cache must not follow symlinked project paths."""
5909+
from unittest.mock import patch
5910+
5911+
if not can_create_symlink(temp_dir):
5912+
pytest.skip("symlinks are not available in this environment")
5913+
5914+
project_dir = temp_dir / "project"
5915+
project_dir.mkdir()
5916+
extensions_dir = project_dir / ".specify" / "extensions"
5917+
extensions_dir.mkdir(parents=True)
5918+
outside_cache = temp_dir / "outside-cache"
5919+
outside_cache.mkdir()
5920+
5921+
try:
5922+
os.symlink(outside_cache, extensions_dir / ".cache")
5923+
except OSError:
5924+
pytest.skip("directory symlinks are not available in this environment")
5925+
5926+
catalog = ExtensionCatalog(project_dir)
5927+
ext_info = {
5928+
"name": "Test Extension",
5929+
"id": "test-ext",
5930+
"version": "1.0.0",
5931+
"description": "Test",
5932+
"download_url": "https://example.com/test-ext.zip",
5933+
}
5934+
5935+
def fail_open_url(*args, **kwargs):
5936+
raise AssertionError("download should not be attempted with symlinked cache")
5937+
5938+
with patch.object(catalog, "get_extension_info", return_value=ext_info), \
5939+
patch.object(catalog, "_open_url", side_effect=fail_open_url):
5940+
with pytest.raises(ExtensionError, match="symlinked extension download cache"):
5941+
catalog.download_extension("test-ext")
5942+
5943+
assert not (outside_cache / "downloads" / "test-ext-1.0.0.zip").exists()
5944+
58815945

58825946
class TestExtensionUpdateCLI:
58835947
"""CLI integration tests for extension update command."""

0 commit comments

Comments
 (0)