From f629e8e8811d0691145dc7c7049f96cfe13df81b Mon Sep 17 00:00:00 2001 From: chelsealong Date: Sat, 12 Sep 2026 11:08:04 +0000 Subject: [PATCH 1/2] fix(extensions): reject aliases that shadow core commands _validate_install_conflicts only compared declared command/alias names against installed extensions, never against core command names, so an extension could claim a core command's fully-qualified name (e.g. 'speckit.taskstoissues') as an alias and shadow it silently. Primary names are already namespace-checked against CORE_COMMAND_NAMES, but aliases are intentionally free-form, so this can only be caught in the install-conflict check by comparing against the qualified core names directly. Fixes #4555 --- src/specify_cli/extensions/__init__.py | 26 ++++++++++++---- tests/test_extensions.py | 42 ++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index c20160c167..ef74c3b86d 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -1185,17 +1185,31 @@ def _get_installed_command_name_map( return installed_names 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 directly + against the fully-qualified core command names (``speckit.``) + 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_command_names = {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 name in core_command_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- " diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 800f5ce00e..b825039b85 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -3126,6 +3126,48 @@ 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) + def test_remove_extension(self, extension_dir, project_dir): """Test removing an installed extension.""" manager = ExtensionManager(project_dir) From 4a6ac69fce5e2d769a4cf7759d8535954af5d13b Mon Sep 17 00:00:00 2001 From: chelsealong Date: Tue, 15 Sep 2026 16:48:19 +0000 Subject: [PATCH 2/2] fix(extensions): reject normalized-equivalent alias spellings that shadow core commands Copilot review on #4558 pointed out that agent-specific output-name normalization (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 the exact-dotted-string check missed the plain and hyphenated alias spellings. Also fixed the error heading, which said "conflict with installed extensions" even for a core-only collision. --- src/specify_cli/extensions/__init__.py | 33 ++++++++++++++---- tests/test_extensions.py | 46 ++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index ef74c3b86d..882379d2f1 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -1184,22 +1184,41 @@ 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. 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 directly - against the fully-qualified core command names (``speckit.``) - rather than relying on ``_get_installed_command_name_map``, which - only knows about installed extensions. + 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_command_names = {f"speckit.{name}" for name in CORE_COMMAND_NAMES} + core_shadow_names = { + self._normalize_shadow_name(f"speckit.{name}") for name in CORE_COMMAND_NAMES + } collisions = [] for name in sorted(declared_names): @@ -1207,12 +1226,12 @@ def _validate_install_conflicts(self, manifest: ExtensionManifest) -> None: collisions.append( f"{name} (already provided by extension '{installed_names[name]}')" ) - elif name in core_command_names: + elif self._normalize_shadow_name(name) in core_shadow_names: collisions.append(f"{name} (conflicts with core command)") 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 b825039b85..add93b2fac 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -3168,6 +3168,52 @@ def test_install_rejects_alias_shadowing_core_command(self, temp_dir, project_di 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)