Skip to content

fix(workflows): reject a non-string 'command' in command-step#3596

Merged
mnriem merged 1 commit into
github:mainfrom
Noor-ul-ain001:fix/command-nonstring-command-crash
Jul 21, 2026
Merged

fix(workflows): reject a non-string 'command' in command-step#3596
mnriem merged 1 commit into
github:mainfrom
Noor-ul-ain001:fix/command-nonstring-command-crash

Conversation

@Noor-ul-ain001

Copy link
Copy Markdown
Contributor

Summary

CommandStep.validate only checked that a command field is present, never its type. But the workflow engine does not auto-validate a step before calling execute, so on an unvalidated run a non-string commandnull, a list, an int — was passed straight through _try_dispatch to the integration's build_command_invocation, which does:

stem = command_name
if stem.startswith("speckit."):   # AttributeError if command_name isn't a str

Once a resolvable integration with an installed CLI is found, this crashes the entire workflow run with a raw AttributeError instead of failing just the step.

Confirmed directly:

['a', 'b'] -> AttributeError 'list' object has no attribute 'startswith'
None       -> AttributeError 'NoneType' object has no attribute 'startswith'
42         -> AttributeError 'int' object has no attribute 'startswith'

This is the same class of gap that #3582 closed for the prompt-step prompt field — validate() checked presence only while execute() fed the value to the CLI layer.

Fix

Guard both paths, matching the sibling steps already in this file:

  • validate() rejects a non-string command, mirroring prompt-step prompt (fix(workflows): reject a non-string prompt in prompt-step validate() #3582) and shell-step run.
  • execute() fails the step cleanly with the same contract error before dispatch, mirroring the existing input/options guards in CommandStep.execute — so an unvalidated run FAILs the step (honouring continue_on_error) instead of crashing the run.

An expression like {{ inputs.cmd }} is still a str, so it stays valid.

Tests

Added to TestCommandStep:

  • test_validate_rejects_non_string_command (null / list / int / dict, plus accepts a plain string and an expression)
  • test_execute_non_string_command_fails_cleanly (patches shutil.which so dispatch would actually be attempted absent the guard — proving the guard prevents the crash)

Verified test-the-test: with the source fix stashed, validate() misses the bad value and execute() crashes at build_command_invocation with the raw AttributeError; with the fix, all 13 TestCommandStep tests pass. The 20 unrelated test_workflows.py failures are the pre-existing Windows symlink-guard tests that require elevation.

🤖 Generated with Claude Code

`CommandStep.validate` only checked that a `command` field is *present*,
never its type. On an unvalidated run (the engine does not auto-validate
before `execute`) a non-string `command` — null, a list, an int — was
passed straight through `_try_dispatch` to the integration's
`build_command_invocation`, which does `command_name.startswith("speckit.")`
and crashes the whole workflow with a raw `AttributeError` once a
resolvable integration with an installed CLI is found.

Guard both paths, mirroring the sibling steps:
- `validate()` rejects a non-string `command` (like prompt-step `prompt`
  github#3582 and shell-step `run`).
- `execute()` fails the step cleanly with the same contract error before
  dispatch (like the existing `input`/`options` guards in this file), so
  an unvalidated run FAILs the step instead of crashing the run.

An expression like `{{ inputs.cmd }}` is still a string, so it stays valid.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds robust type validation for workflow command steps, preventing non-string commands from crashing dispatch.

Changes:

  • Rejects non-string command values during validation and execution.
  • Adds coverage for validation and clean step failure behavior.
Show a summary per file
File Description
src/specify_cli/workflows/steps/command/__init__.py Adds command type guards.
tests/test_workflows.py Tests invalid command handling.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Medium

@mnriem
mnriem merged commit 70c547c into github:main Jul 21, 2026
12 checks passed
@mnriem

mnriem commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants