From 4f19bd260da976c0aeb411d4528b07fbcafb62ae Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Sun, 5 Jul 2026 19:34:03 +0500 Subject: [PATCH] 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 --- .../workflows/steps/shell/__init__.py | 10 ++++++++++ tests/test_workflows.py | 20 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/specify_cli/workflows/steps/shell/__init__.py b/src/specify_cli/workflows/steps/shell/__init__.py index 2a65fca444..3ef94892ef 100644 --- a/src/specify_cli/workflows/steps/shell/__init__.py +++ b/src/specify_cli/workflows/steps/shell/__init__.py @@ -90,6 +90,16 @@ def validate(self, config: dict[str, Any]) -> list[str]: errors.append( f"Shell step {config.get('id', '?')!r} is missing 'run' field." ) + elif not isinstance(config["run"], str): + # execute() str()-coerces run and invokes it under shell=True, so a + # null or list 'run' would run the Python repr ('None', "['echo']") + # as a command. Reject non-strings at validation, mirroring the + # command-step input/options and gate options type checks. An + # expression like "{{ ... }}" is still a str, so it stays valid. + errors.append( + f"Shell step {config.get('id', '?')!r}: 'run' must be a string, " + f"got {type(config['run']).__name__}." + ) output_format = config.get("output_format") if output_format is not None and output_format != "json": errors.append( diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 2fdbf887b3..117583864f 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1183,6 +1183,26 @@ def test_validate_missing_run(self): errors = step.validate({"id": "test"}) assert any("missing 'run'" in e for e in errors) + @pytest.mark.parametrize("bad_run", [None, ["echo", "hi"], 42]) + def test_validate_rejects_non_string_run(self, bad_run): + """A non-string 'run' must be rejected at validation. + + execute() str()-coerces run and invokes it under shell=True, so a + null or list run would otherwise run the Python repr as a command. + """ + from specify_cli.workflows.steps.shell import ShellStep + + step = ShellStep() + errors = step.validate({"id": "s", "run": bad_run}) + assert any("'run' must be a string" in e for e in errors) + + def test_validate_accepts_string_and_expression_run(self): + from specify_cli.workflows.steps.shell import ShellStep + + step = ShellStep() + assert step.validate({"id": "s", "run": "echo hi"}) == [] + assert step.validate({"id": "s", "run": "{{ steps.x.output }}"}) == [] + def test_output_format_json_exposes_data(self, tmp_path): from specify_cli.workflows.steps.shell import ShellStep