diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index 9fa44d3809..bcffb24c62 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -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) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index 157bac6c46..1d2066912b 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -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 diff --git a/tests/test_extensions.py b/tests/test_extensions.py index d668019087..a2d7a1b5c7 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -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 ===== diff --git a/tests/test_presets.py b/tests/test_presets.py index 80f2ddab58..e43fa5f99a 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -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."""