diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index c22445ebe0..dabde70bbc 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -432,9 +432,13 @@ def _validate(self): f"Invalid hook '{hook_name}': " "expected a mapping or list of mappings" ) - if not entry.get("command"): + command = entry.get("command") + # #5080: same rule as validate_events() in events/__init__.py. + # A truthy non-string (e.g. command: [foo]) passed a bare + # truthiness check and crashed hook registration mid-install. + if not isinstance(command, str) or not command.strip(): raise ValidationError( - f"Hook '{hook_name}' missing required 'command' field" + f"Hook '{hook_name}' missing required 'command' string" ) if "priority" in entry: priority = entry["priority"] diff --git a/tests/specify_cli/extensions/test_command_add.py b/tests/specify_cli/extensions/test_command_add.py index 44423b5fdc..f898fa052f 100644 --- a/tests/specify_cli/extensions/test_command_add.py +++ b/tests/specify_cli/extensions/test_command_add.py @@ -40,6 +40,34 @@ class TestExtensionAddCLI: """CLI tests for ``specify extension add``.""" + @pytest.mark.parametrize("command", [ + pytest.param(["speckit.test-ext.hello"], id="list"), + pytest.param({"name": "speckit.test-ext.hello"}, id="mapping"), + ]) + def test_add_dev_rejects_invalid_hook_command_before_writing_files( + self, extension_dir, project_dir, command + ): + """Malformed hook commands must leave no installed artifacts (#5080).""" + manifest_path = extension_dir / "extension.yml" + data = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) + data["hooks"]["after_tasks"]["command"] = command + manifest_path.write_text(yaml.safe_dump(data), encoding="utf-8") + agent_dir = project_dir / ".claude" + (agent_dir / "skills").mkdir(parents=True) + before = set(agent_dir.rglob("*")) + + with patch.object(Path, "cwd", return_value=project_dir): + result = CliRunner().invoke(app, ["extension", "add", "--dev", str(extension_dir)]) + + assert result.exit_code == 1, result.output + assert isinstance(result.exception, SystemExit), repr(result.exception) + output = strip_ansi(result.output) + assert "after_tasks" in output and "'command'" in output, output + assert "Traceback" not in output + assert not (project_dir / ".specify/extensions/test-ext").exists() + assert set(agent_dir.rglob("*")) == before + assert ExtensionRegistry(project_dir / ".specify/extensions").get("test-ext") is None + def test_add_dev_links_copilot_agent_when_supported( self, extension_dir, project_dir, temp_dir ): diff --git a/tests/test_extensions.py b/tests/test_extensions.py index f2a75d08a4..07142e9c24 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -878,6 +878,26 @@ def test_non_dict_hook_entry_raises_validation_error(self, temp_dir, valid_manif with pytest.raises(ValidationError, match="Invalid hook 'after_tasks'"): ExtensionManifest(manifest_path) + @pytest.mark.parametrize("as_list", [False, True]) + @pytest.mark.parametrize("command", [ + pytest.param(["speckit.test-ext.hello"], id="list"), + pytest.param({"name": "speckit.test-ext.hello"}, id="mapping"), + pytest.param(123, id="integer"), + pytest.param(True, id="boolean"), + pytest.param(" ", id="blank"), + ]) + def test_hook_command_must_be_a_string( + self, temp_dir, valid_manifest_data, command, as_list + ): + """Reject non-string commands before hook registration can fail (#5080).""" + entry = {"command": command} + valid_manifest_data["hooks"]["after_tasks"] = [entry] if as_list else entry + manifest_path = temp_dir / "extension.yml" + manifest_path.write_text(yaml.safe_dump(valid_manifest_data), encoding="utf-8") + + with pytest.raises(ValidationError, match="Hook 'after_tasks' missing required 'command' string"): + ExtensionManifest(manifest_path) + def test_hook_single_mapping_still_accepted(self, extension_dir): """Existing single-mapping hook manifests parse unchanged (regression).""" manifest_path = extension_dir / "extension.yml"