Skip to content

Commit 4f19bd2

Browse files
jawwad-aliclaude
andcommitted
fix(workflows): shell step validate() rejects non-string run
ShellStep.validate() only checked that 'run' was present, so run: (null) or a GitHub-Actions-style list validated clean; execute() then str()-coerces the value and invokes it under shell=True, literally running 'None' or "['echo', 'hi']" as a command. Add a type check after the presence check, mirroring the command-step (#3262) and gate options validation. Expression strings ('{{ ... }}') are strings, so they stay valid. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent bba473c commit 4f19bd2

2 files changed

Lines changed: 30 additions & 0 deletions

File tree

src/specify_cli/workflows/steps/shell/__init__.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,16 @@ def validate(self, config: dict[str, Any]) -> list[str]:
9090
errors.append(
9191
f"Shell step {config.get('id', '?')!r} is missing 'run' field."
9292
)
93+
elif not isinstance(config["run"], str):
94+
# execute() str()-coerces run and invokes it under shell=True, so a
95+
# null or list 'run' would run the Python repr ('None', "['echo']")
96+
# as a command. Reject non-strings at validation, mirroring the
97+
# command-step input/options and gate options type checks. An
98+
# expression like "{{ ... }}" is still a str, so it stays valid.
99+
errors.append(
100+
f"Shell step {config.get('id', '?')!r}: 'run' must be a string, "
101+
f"got {type(config['run']).__name__}."
102+
)
93103
output_format = config.get("output_format")
94104
if output_format is not None and output_format != "json":
95105
errors.append(

tests/test_workflows.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,6 +1183,26 @@ def test_validate_missing_run(self):
11831183
errors = step.validate({"id": "test"})
11841184
assert any("missing 'run'" in e for e in errors)
11851185

1186+
@pytest.mark.parametrize("bad_run", [None, ["echo", "hi"], 42])
1187+
def test_validate_rejects_non_string_run(self, bad_run):
1188+
"""A non-string 'run' must be rejected at validation.
1189+
1190+
execute() str()-coerces run and invokes it under shell=True, so a
1191+
null or list run would otherwise run the Python repr as a command.
1192+
"""
1193+
from specify_cli.workflows.steps.shell import ShellStep
1194+
1195+
step = ShellStep()
1196+
errors = step.validate({"id": "s", "run": bad_run})
1197+
assert any("'run' must be a string" in e for e in errors)
1198+
1199+
def test_validate_accepts_string_and_expression_run(self):
1200+
from specify_cli.workflows.steps.shell import ShellStep
1201+
1202+
step = ShellStep()
1203+
assert step.validate({"id": "s", "run": "echo hi"}) == []
1204+
assert step.validate({"id": "s", "run": "{{ steps.x.output }}"}) == []
1205+
11861206

11871207
def test_output_format_json_exposes_data(self, tmp_path):
11881208
from specify_cli.workflows.steps.shell import ShellStep

0 commit comments

Comments
 (0)