Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/specify_cli/workflows/steps/command/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
35 changes: 35 additions & 0 deletions tests/test_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down