Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion src/specify_cli/extensions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -647,9 +647,32 @@ def __init__(self, extensions_dir: Path):

def _load(self) -> dict:
"""Load registry from disk."""
if not self.registry_path.exists():
# A dangling symlink's target doesn't exist, so Path.exists() (which
# follows symlinks) returns False even though the path itself is
# present as a broken/corrupted entry. Treat any symlink as "present"
# so a dangling one is not mistaken for "no registry at all" — that
# would let every on-disk extension directory be scanned as an
# unregistered, enabled extension (the fail-open path this guard is
# meant to close).
if not self.registry_path.is_symlink() and not self.registry_path.exists():
return {"schema_version": self.SCHEMA_VERSION, "extensions": {}}

# The registry path exists lexically. Require a readable regular file
# before parsing: a directory, a dangling symlink, or a symlink to a
# non-file cannot be parsed, and silently starting fresh here would
# reopen the fail-open directory scan. Fail closed instead so callers
# (e.g. runtime resolution) surface the tampered/broken registry
# rather than treating every on-disk extension as enabled. This raises
# the same error open() would: EISDIR for a directory (as before) and
# ENOENT for a dangling symlink whose target is missing.
if not self.registry_path.is_file():
code = errno.EISDIR if self.registry_path.is_dir() else errno.ENOENT
raise OSError(
code,
"Extension registry is not a readable regular file",
str(self.registry_path),
)

try:
with open(self.registry_path, "r", encoding="utf-8") as f:
data = json.load(f)
Expand Down
89 changes: 52 additions & 37 deletions src/specify_cli/presets/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5420,44 +5420,59 @@ def _find_in_subdirs(base_dir: Path) -> Optional[Path]:
})

# Priority 3: Extension-provided templates (always "replace")
for _priority, ext_id, ext_meta in self._get_all_extensions_by_priority():
ext_dir = self.extensions_dir / ext_id
if not ext_dir.is_dir():
continue
# Try convention-based lookup first
candidate = _find_in_subdirs(ext_dir)
# If not found and this is a command, check extension manifest
if candidate is None and template_type == "command":
ext_manifest_path = ext_dir / "extension.yml"
if ext_manifest_path.exists():
try:
from ..extensions import ExtensionManifest, ValidationError as ExtValidationError
ext_manifest = ExtensionManifest(ext_manifest_path)
for cmd in ext_manifest.commands:
if cmd.get("name") == template_name:
cmd_file = cmd.get("file")
if cmd_file:
c = ext_dir / cmd_file
if c.exists():
candidate = c
# Extensions (priority 3) and core (priority 4) sit below the override
# (priority 1) and preset (priority 2) tiers. Once one of those higher
# tiers has already contributed an effective "replace" base, no lower
# tier can change the composed result (resolve_content stops at the
# highest-priority "replace" layer). Skip enumerating the extension
# tier in that case so a corrupt, unreadable, or otherwise broken
# lower extension registry cannot fail resolution that a winning
# override or preset has already decided — matching the priority
# parity of the runtime resolvers. Only the extension tier is gated
# here: its enumeration is the sole lower-tier step that reads the
# extension registry and can therefore fail on a tampered/broken one.
base_already_won = any(
layer["strategy"] == "replace" for layer in layers
)
if not base_already_won:
for _priority, ext_id, ext_meta in self._get_all_extensions_by_priority():
ext_dir = self.extensions_dir / ext_id
if not ext_dir.is_dir():
continue
# Try convention-based lookup first
candidate = _find_in_subdirs(ext_dir)
# If not found and this is a command, check extension manifest
if candidate is None and template_type == "command":
ext_manifest_path = ext_dir / "extension.yml"
if ext_manifest_path.exists():
try:
from ..extensions import ExtensionManifest, ValidationError as ExtValidationError
ext_manifest = ExtensionManifest(ext_manifest_path)
for cmd in ext_manifest.commands:
if cmd.get("name") == template_name:
cmd_file = cmd.get("file")
if cmd_file:
c = ext_dir / cmd_file
if c.exists():
candidate = c
break
except (ExtValidationError, yaml.YAMLError):
# Invalid extension manifest — fall back to
# convention-based lookup (already attempted above).
pass
if candidate:
if ext_meta:
version = ext_meta.get("version", "?")
source = f"extension:{ext_id} v{version}"
else:
source = f"extension:{ext_id} (unregistered)"
layers.append({
"path": candidate,
"source": source,
"strategy": "replace",
"extension_id": ext_id,
"extension_dir": ext_dir,
})
except (ExtValidationError, yaml.YAMLError):
# Invalid extension manifest — fall back to
# convention-based lookup (already attempted above).
pass
if candidate:
if ext_meta:
version = ext_meta.get("version", "?")
source = f"extension:{ext_id} v{version}"
else:
source = f"extension:{ext_id} (unregistered)"
layers.append({
"path": candidate,
"source": source,
"strategy": "replace",
"extension_id": ext_id,
"extension_dir": ext_dir,
})

# Priority 4: Core templates (always "replace")
core = None
Expand Down
32 changes: 32 additions & 0 deletions tests/test_extensions.py
Original file line number Diff line number Diff line change
Expand Up @@ -1316,6 +1316,38 @@ def test_load_starts_fresh_for_non_utf8_registry(self, temp_dir):
assert registry.list() == {}
assert not registry.is_installed("test-ext")

def test_load_fails_closed_for_dangling_registry_symlink(self, temp_dir):
"""A dangling ``.registry`` symlink must fail closed, not start fresh.

``Path.exists()`` follows symlinks, so a broken symlink returns
``False`` and would be mistaken for an absent registry — after which
callers scan every on-disk extension directory as an unregistered,
enabled extension (the fail-open path). Detect the entry lexically and
require a readable regular file so a dangling symlink raises instead.
"""
extensions_dir = temp_dir / "extensions"
extensions_dir.mkdir()
registry_path = extensions_dir / ExtensionRegistry.REGISTRY_FILE
if not can_create_symlink(extensions_dir):
pytest.skip("platform/user cannot create symlinks")
os.symlink(extensions_dir / "does-not-exist.json", registry_path)
assert registry_path.is_symlink()
assert not registry_path.exists()

with pytest.raises(OSError):
ExtensionRegistry(extensions_dir)

def test_load_fails_closed_for_directory_registry(self, temp_dir):
"""A directory at the ``.registry`` path is not a readable regular
file and must fail closed rather than be treated as an absent
registry."""
extensions_dir = temp_dir / "extensions"
extensions_dir.mkdir()
(extensions_dir / ExtensionRegistry.REGISTRY_FILE).mkdir()

with pytest.raises(OSError):
ExtensionRegistry(extensions_dir)


# ===== ExtensionManager Tests =====

Expand Down
45 changes: 45 additions & 0 deletions tests/test_presets.py
Original file line number Diff line number Diff line change
Expand Up @@ -12088,6 +12088,51 @@ def test_layers_read_strategy_from_manifest(self, project_dir, temp_dir, valid_p
# Core layer should be replace
assert layers[1]["strategy"] == "replace"

def test_winning_replace_preset_skips_broken_extension_registry(
self, project_dir, temp_dir, valid_pack_data
):
"""A winning ``replace`` preset must not be invalidated by a broken
lower extension registry.

Extensions sit below presets in the priority stack, so once a
higher-priority ``replace`` preset resolves the template, the
extension tier is irrelevant. ``collect_all_layers`` must therefore
skip enumerating extensions in that case, so a corrupt/unreadable
extension registry (which fails closed when read) cannot break
resolution the preset has already decided.
"""
# Highest-priority preset provides spec-template with default replace.
pack_dir = _create_pack(
temp_dir, valid_pack_data, "win-pack", "# From Pack\n"
)
PresetManager(project_dir).install_from_directory(pack_dir, "0.1.5")

# A broken extension registry: a directory at the ``.registry`` path
# is not a readable regular file and makes ExtensionRegistry() raise.
extensions_dir = project_dir / ".specify" / "extensions"
(extensions_dir / ".registry").mkdir(parents=True)
# Also drop an extension that would otherwise provide the template, to
# prove it is never consulted.
ext_templates = extensions_dir / "ext-a" / "templates"
ext_templates.mkdir(parents=True)
(ext_templates / "spec-template.md").write_text("# From Extension\n")

resolver = PresetResolver(project_dir)

# Enumerating the extension tier would raise on the broken registry.
with pytest.raises(OSError):
resolver._get_all_extensions_by_priority()

# collect_all_layers/resolve_content must not raise, and must resolve
# to the winning preset content — the extension tier is skipped.
layers = resolver.collect_all_layers("spec-template")
assert "win-pack" in layers[0]["source"]
assert layers[0]["strategy"] == "replace"
assert not any(
str(layer["source"]).startswith("extension:") for layer in layers
)
assert resolver.resolve_content("spec-template") == "# From Pack\n"


class TestRemoveReconciliation:
"""Test that removing a preset re-registers the next layer's command."""
Expand Down