diff --git a/src/specify_cli/workflows/steps/command/__init__.py b/src/specify_cli/workflows/steps/command/__init__.py index 7a6d893ed0..e28f305172 100644 --- a/src/specify_cli/workflows/steps/command/__init__.py +++ b/src/specify_cli/workflows/steps/command/__init__.py @@ -30,6 +30,21 @@ class CommandStep(StepBase): def execute(self, config: dict[str, Any], context: StepContext) -> StepResult: command = config.get("command", "") + # validate() rejects a non-string 'command', but the engine does not + # auto-validate before execute(); an unvalidated run would pass the value + # to build_command_invocation() (via _try_dispatch) and crash there with a + # raw AttributeError (command_name.startswith(...) on a list/int/None). + # Fail the step with the same contract error instead, mirroring the + # 'input'/'options' guards below. + if not isinstance(command, str): + return StepResult( + status=StepStatus.FAILED, + error=( + f"Command step {config.get('id', '?')!r}: 'command' must be a " + f"string, got {type(command).__name__}." + ), + ) + input_data = config.get("input", {}) # validate() rejects a non-mapping input, but the engine does not # auto-validate before execute(); a workflow that skipped validation can @@ -179,6 +194,17 @@ def validate(self, config: dict[str, Any]) -> list[str]: errors.append( f"Command step {config.get('id', '?')!r} is missing 'command' field." ) + elif not isinstance(config["command"], str): + # execute() passes 'command' straight to the integration's + # build_command_invocation(), which does command_name.startswith(...); + # a non-string (null, list, int) crashes there with a raw + # AttributeError once dispatch is attempted. Reject it at validation, + # mirroring the prompt-step 'prompt' and shell-step 'run' type checks. + # An expression like "{{ ... }}" is still a str, so it stays valid. + errors.append( + f"Command step {config.get('id', '?')!r}: 'command' must be a " + f"string, got {type(config['command']).__name__}." + ) # execute() iterates input.items() and options.update(step_options); a # non-mapping here would raise at run time. Validate the shape like the # sibling steps (switch 'cases', fan-out 'step') so it is reported, not diff --git a/tests/test_workflows.py b/tests/test_workflows.py index b1d9841535..4eb6d5e25c 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1026,6 +1026,41 @@ def test_validate_rejects_non_mapping_input_and_options(self): assert res_opt.status is StepStatus.FAILED assert "'options' must be a mapping" in (res_opt.error or "") + def test_validate_rejects_non_string_command(self): + from specify_cli.workflows.steps.command import CommandStep + + step = CommandStep() + # execute() passes 'command' to build_command_invocation(), which does + # command_name.startswith(...); a non-string crashes there with a raw + # AttributeError. validate() must report it, like prompt-step 'prompt'. + for bad in (None, ["a", "b"], 5, {"x": 1}): + errs = step.validate({"id": "c", "command": bad}) + assert any("'command' must be a string" in e for e in errs), bad + # a string command (incl. an expression) is still accepted + assert step.validate({"id": "c", "command": "/x"}) == [] + assert step.validate({"id": "c", "command": "{{ inputs.cmd }}"}) == [] + + def test_execute_non_string_command_fails_cleanly(self): + from unittest.mock import patch + from specify_cli.workflows.steps.command import CommandStep + from specify_cli.workflows.base import StepContext, StepStatus + + step = CommandStep() + # The engine may skip validate(); a non-string 'command' must FAIL the + # step with the contract error rather than reaching _try_dispatch and + # crashing build_command_invocation with a raw AttributeError. Force a + # resolvable integration + installed CLI so, absent the guard, dispatch + # would actually be attempted and the crash would fire. + ctx = StepContext(default_integration="claude") + with patch("specify_cli.workflows.steps.command.shutil.which", + return_value="/usr/bin/claude"): + for bad in (None, ["a", "b"], 5, {"x": 1}): + result = step.execute( + {"id": "c", "command": bad, "input": {}}, ctx + ) + assert result.status is StepStatus.FAILED, bad + assert "'command' must be a string" in (result.error or ""), bad + def test_step_override_integration(self): from unittest.mock import patch from specify_cli.workflows.steps.command import CommandStep