fix(workflows): validate requires keys and reject phantom permissions gate

docs/reference/workflows.md

@@ -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` is an advisory pre-condition block (spec-kit version, integrations), not a runtime gate, so it does **not** restrict what a step can do. In particular there is no `requires.permissions` capability gate: it is rejected by validation precisely because it would imply a sandbox that does not exist. Review any catalog or downloaded workflow before running it, and use a `gate` step to require explicit approval before sensitive or destructive shell commands.
+
 ## Expressions
 
 Steps can reference inputs and previous step outputs using `{{ expression }}` syntax:

src/specify_cli/workflows/engine.py

@@ -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`` (recognized keys only;
+        # see ``_RECOGNIZED_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.
         self.requires: dict[str, Any] = data.get("requires", {})
 
         # Inputs
@@ -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: the advisory
+# pre-conditions documented for workflows (``speckit_version`` and
+# ``integrations``). This is the *workflow* schema only — the bundle manifest's
+# ``requires`` (see ``bundler/models/manifest.py``) is a separate schema that
+# also carries ``tools``/``mcp``; those are not workflow ``requires`` keys.
+# Any other key — notably ``permissions`` — is rejected by ``validate_workflow``
+# so it is never mistaken for an enforced runtime control.
+_RECOGNIZED_REQUIRES_KEYS = frozenset({"speckit_version", "integrations"})
+
 # 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."""
@@ -177,6 +190,39 @@ 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 recognized;
+    # reject anything else so authoring typos surface here instead of being
+    # 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.
+    #
+    # Guard on ``is not None`` rather than truthiness: a falsy but non-mapping
+    # value (e.g. ``requires: []`` or ``requires: ''``) is still an authoring
+    # error and must surface, whereas ``requires:`` (YAML null) is treated as
+    # an omitted block.
+    if definition.requires is not None:
+        if not isinstance(definition.requires, dict):
+            errors.append(
+                "'requires' must be a mapping (or omitted)."
+            )
+        else:
+            for key in definition.requires:
+                if key == "permissions":
+                    errors.append(
+                        "'requires.permissions' is not a recognized 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 _RECOGNIZED_REQUIRES_KEYS:
+                    errors.append(
+                        f"Unknown 'requires' key {key!r}. Recognized keys: "
+                        f"{', '.join(sorted(_RECOGNIZED_REQUIRES_KEYS))}."
+                    )
+
     # -- Steps ------------------------------------------------------------
     if not isinstance(definition.steps, list):
         errors.append("'steps' must be a list.")

tests/test_workflows.py

@@ -2048,6 +2048,107 @@ 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)
+
+    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 on specific markers from the intended message (the offending
+        # key and the `gate` remediation) so the test fails if the validation
+        # path or wording drifts, rather than passing on any error that merely
+        # happens to contain "permissions" and "not".
+        assert any("requires.permissions" in e and "gate" in e for e in errors)
+
+    def test_requires_empty_sequence_is_rejected_as_non_mapping(self):
+        """A falsy-but-non-mapping ``requires`` (e.g. an empty list) is still an
+        authoring error: validation must guard on ``is not None`` rather than
+        truthiness, otherwise ``requires: []`` would silently pass.
+        """
+        from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow
+
+        definition = WorkflowDefinition.from_string("""
+workflow:
+  id: "test"
+  name: "Test"
+  version: "1.0.0"
+requires: []
+steps:
+  - id: step-one
+    command: speckit.specify
+""")
+        errors = validate_workflow(definition)
+        assert any("'requires' must be a mapping" in e for e in errors)
+
 
 # ===== Workflow Engine Tests =====
 

workflows/PUBLISHING.md

@@ -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`), **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