diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index c20160c167..882379d2f1 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -1184,21 +1184,54 @@ def _get_installed_command_name_map( return installed_names + @staticmethod + def _normalize_shadow_name(name: str) -> str: + """Normalize a command/alias name to its on-disk output form. + + Agent integrations (Cline, Forge, Junie) and the SKILL.md output-name + computation (``CommandRegistrar._compute_output_name``) all collapse + dots to hyphens and prefix a bare name with ``speckit-``, so + ``speckit.taskstoissues``, ``taskstoissues``, and + ``speckit-taskstoissues`` are distinct alias spellings that land on + the same on-disk command name. Normalize before comparing so all of + them are caught, not just the exact dotted spelling. + """ + hyphenated = name.replace(".", "-") + if not hyphenated.startswith("speckit-"): + hyphenated = f"speckit-{hyphenated}" + return hyphenated + def _validate_install_conflicts(self, manifest: ExtensionManifest) -> None: - """Reject installs that would shadow core or installed extension commands.""" + """Reject installs that would shadow core or installed extension commands. + + Primary command names are already namespace-checked against + ``CORE_COMMAND_NAMES`` in ``_collect_manifest_command_names``, but + aliases are intentionally free-form (see the comment there) and so + can only be caught here, by comparing declared names' normalized + on-disk form (see ``_normalize_shadow_name``) against core command + names rather than relying on ``_get_installed_command_name_map``, + which only knows about installed extensions. + """ declared_names = self._collect_manifest_command_names(manifest) installed_names = self._get_installed_command_name_map( exclude_extension_id=manifest.id ) + core_shadow_names = { + self._normalize_shadow_name(f"speckit.{name}") for name in CORE_COMMAND_NAMES + } + + collisions = [] + for name in sorted(declared_names): + if name in installed_names: + collisions.append( + f"{name} (already provided by extension '{installed_names[name]}')" + ) + elif self._normalize_shadow_name(name) in core_shadow_names: + collisions.append(f"{name} (conflicts with core command)") - collisions = [ - f"{name} (already provided by extension '{installed_names[name]}')" - for name in sorted(declared_names) - if name in installed_names - ] if collisions: raise ValidationError( - "Extension commands conflict with installed extensions:\n- " + "Extension commands conflict with core or installed extension commands:\n- " + "\n- ".join(collisions) ) diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 800f5ce00e..add93b2fac 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -3126,6 +3126,94 @@ def test_install_rejects_command_collision_with_installed_extension(self, temp_d with pytest.raises(ValidationError, match="already provided by extension 'ext-one'"): manager.install_from_directory(second_dir, "0.1.0", register_commands=False) + def test_install_rejects_alias_shadowing_core_command(self, temp_dir, project_dir): + """An alias equal to a core command's qualified name must not install. + + Regression test for #4555: a primary name is namespace-checked + against CORE_COMMAND_NAMES, but aliases are intentionally free-form + and previously went unchecked against core commands entirely, so an + extension could claim e.g. 'speckit.taskstoissues' as an alias and + shadow the core command of the same name. + """ + import yaml + + ext_dir = temp_dir / "probe-ext" + ext_dir.mkdir() + (ext_dir / "commands").mkdir() + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "probe", + "name": "Probe", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.probe.taskstoissues", + "file": "commands/cmd.md", + "aliases": ["speckit.taskstoissues"], + } + ] + }, + } + + (ext_dir / "extension.yml").write_text(yaml.dump(manifest_data)) + (ext_dir / "commands" / "cmd.md").write_text("---\ndescription: Test\n---\n\nBody") + + manager = ExtensionManager(project_dir) + with pytest.raises(ValidationError, match="conflicts with core command"): + manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) + + @pytest.mark.parametrize("alias", ["taskstoissues", "speckit-taskstoissues"]) + def test_install_rejects_equivalent_alias_shadowing_core_command( + self, temp_dir, project_dir, alias + ): + """Plain and hyphenated alias spellings must be rejected too. + + Regression test for the reviewer follow-up on #4555: agent-specific + name transformation (``CommandRegistrar._compute_output_name`` and the + Cline/Forge/Junie formatters) collapses ``speckit.taskstoissues``, + ``taskstoissues``, and ``speckit-taskstoissues`` to the same on-disk + command name, so all three spellings must be rejected, not just the + exact dotted one. + """ + import yaml + + ext_dir = temp_dir / "probe-ext" + ext_dir.mkdir() + (ext_dir / "commands").mkdir() + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "probe", + "name": "Probe", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.probe.taskstoissues", + "file": "commands/cmd.md", + "aliases": [alias], + } + ] + }, + } + + (ext_dir / "extension.yml").write_text(yaml.dump(manifest_data)) + (ext_dir / "commands" / "cmd.md").write_text("---\ndescription: Test\n---\n\nBody") + + manager = ExtensionManager(project_dir) + with pytest.raises(ValidationError, match="conflicts with core command"): + manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) + def test_remove_extension(self, extension_dir, project_dir): """Test removing an installed extension.""" manager = ExtensionManager(project_dir)