Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions docs/reference/workflows.md
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,8 @@ specify workflow run speckit -i spec="Build a kanban board with drag-and-drop ta
| `fan-out` | Dispatch a step for each item in a list |
| `fan-in` | Aggregate results from a fan-out step |

> **Security note:** a `shell` step runs a local command with **your** privileges. There is no capability sandbox — `requires` (e.g. `requires.permissions`) is an advisory pre-condition block, not a runtime gate, so it does **not** restrict what a step can do. Review any catalog or downloaded workflow before running it, and use a `gate` step to require explicit approval before sensitive or destructive shell commands.
Comment thread
mnriem marked this conversation as resolved.
Outdated

## Expressions

Steps can reference inputs and previous step outputs using `{{ expression }}` syntax:
Expand Down
45 changes: 43 additions & 2 deletions src/specify_cli/workflows/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,12 @@ def __init__(self, data: dict[str, Any], source_path: Path | None = None) -> Non
if not isinstance(self.default_options, dict):
self.default_options = {}

# Requirements (declared but not yet enforced at runtime;
# enforcement is a planned enhancement)
# Advisory pre-conditions (spec-kit version / integrations a workflow
# expects). Validated by ``validate_workflow`` (recognised keys only;
# see ``_RECOGNISED_REQUIRES_KEYS``) but NOT enforced at run time — they
# are not a security boundary. In particular there is no
# ``requires.permissions`` capability gate: shell steps always run with
# the user's privileges.
Comment thread
mnriem marked this conversation as resolved.
self.requires: dict[str, Any] = data.get("requires", {})

# Inputs
Expand Down Expand Up @@ -87,6 +91,15 @@ def from_string(cls, content: str) -> WorkflowDefinition:
# ID format: lowercase alphanumeric with hyphens
_ID_PATTERN = re.compile(r"^[a-z0-9][a-z0-9-]*[a-z0-9]$|^[a-z0-9]$")

# Keys accepted under a workflow's ``requires`` block. These mirror the
# pre-conditions documented in workflows/PUBLISHING.md and resolved by the
# bundler (``speckit_version``, ``integrations``, ``tools``, ``mcp``). Any other
# key — notably ``permissions`` — is rejected by ``validate_workflow`` so it is
# never mistaken for an enforced runtime control.
_RECOGNISED_REQUIRES_KEYS = frozenset(
{"speckit_version", "integrations", "tools", "mcp"}
)
Comment thread
mnriem marked this conversation as resolved.
Outdated

# Valid step types (matching STEP_REGISTRY keys)
def _get_valid_step_types() -> set[str]:
"""Return valid step types from the registry, with a built-in fallback."""
Expand Down Expand Up @@ -177,6 +190,34 @@ def validate_workflow(definition: WorkflowDefinition) -> list[str]:
f"Input {input_name!r} has invalid default: {exc}"
)

# -- Requires ---------------------------------------------------------
# ``requires`` declares advisory pre-conditions (the spec-kit version and
# integrations a workflow expects). Only a fixed set of keys is recognised;
# reject anything else so authoring typos surface here instead of being
Comment thread
mnriem marked this conversation as resolved.
# silently ignored at runtime. In particular ``requires.permissions`` is
# rejected explicitly: it reads like a runtime capability gate, but no such
# gate exists — a ``shell`` step always runs with the user's privileges, so
# declaring it would give a false sense of sandboxing.
if definition.requires:
if not isinstance(definition.requires, dict):
errors.append(
"'requires' must be a mapping (or omitted)."
)
else:
Comment thread
mnriem marked this conversation as resolved.
Outdated
for key in definition.requires:
if key == "permissions":
errors.append(
"'requires.permissions' is not a recognised or "
"enforced capability gate — shell steps always run "
"with the user's privileges. Remove it and gate "
"sensitive steps with a 'gate' step instead."
)
elif key not in _RECOGNISED_REQUIRES_KEYS:
errors.append(
f"Unknown 'requires' key {key!r}. Recognised keys: "
f"{', '.join(sorted(_RECOGNISED_REQUIRES_KEYS))}."
)

# -- Steps ------------------------------------------------------------
if not isinstance(definition.steps, list):
errors.append("'steps' must be a list.")
Expand Down
77 changes: 77 additions & 0 deletions tests/test_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -2048,6 +2048,83 @@ def test_invalid_input_type(self):
errors = validate_workflow(definition)
assert any("invalid type" in e.lower() for e in errors)

def test_requires_with_recognized_keys_is_valid(self):
from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow

definition = WorkflowDefinition.from_string("""
workflow:
id: "test"
name: "Test"
version: "1.0.0"
requires:
speckit_version: ">=0.7.2"
integrations:
any: ["claude", "gemini"]
steps:
- id: step-one
command: speckit.specify
""")
errors = validate_workflow(definition)
assert errors == []

def test_requires_must_be_mapping(self):
from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow

definition = WorkflowDefinition.from_string("""
workflow:
id: "test"
name: "Test"
version: "1.0.0"
requires: "claude"
steps:
- id: step-one
command: speckit.specify
""")
errors = validate_workflow(definition)
assert any("'requires' must be a mapping" in e for e in errors)

Comment thread
mnriem marked this conversation as resolved.
def test_requires_unknown_key_is_rejected(self):
from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow

definition = WorkflowDefinition.from_string("""
workflow:
id: "test"
name: "Test"
version: "1.0.0"
requires:
speckit_version: ">=0.7.2"
typo_key: true
steps:
- id: step-one
command: speckit.specify
""")
errors = validate_workflow(definition)
assert any("typo_key" in e and "requires" in e for e in errors)

def test_requires_permissions_is_rejected_as_not_enforced(self):
"""A `requires.permissions` block looks like a runtime capability gate
but no such gate exists — shell steps always run with the user's
privileges. Reject it explicitly so authors are not misled into
believing the declaration sandboxes execution.
"""
from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow

definition = WorkflowDefinition.from_string("""
workflow:
id: "test"
name: "Test"
version: "1.0.0"
requires:
permissions:
shell: true
steps:
- id: run
type: shell
run: "echo hi"
""")
errors = validate_workflow(definition)
assert any("permissions" in e and "not" in e.lower() for e in errors)
Comment thread
mnriem marked this conversation as resolved.
Outdated


# ===== Workflow Engine Tests =====

Expand Down
1 change: 1 addition & 0 deletions workflows/PUBLISHING.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ When releasing a new version:

### Shell Steps

- **Shell runs with the user's privileges** — a `shell` step executes a local command directly; there is no capability sandbox. `requires` is an advisory pre-condition block (recognised keys: `speckit_version`, `integrations`, `tools`, `mcp`), **not** a runtime permission gate — there is no `requires.permissions`. Gate sensitive commands explicitly with a `gate` step.
- **Avoid destructive commands** — don't delete files or directories without explicit confirmation via a gate
- **Quote variables** — use proper quoting in shell commands to handle spaces
- **Check exit codes** — shell step failures stop the workflow; make sure commands are robust
Expand Down