diff --git a/docs/reference/workflows.md b/docs/reference/workflows.md index e7e921e1e9..b06ae93f88 100644 --- a/docs/reference/workflows.md +++ b/docs/reference/workflows.md @@ -193,6 +193,14 @@ steps: args: "{{ inputs.spec }}" ``` +Workflows that include any `type: shell` step must also declare: + +```yaml +requires: + permissions: + shell: true +``` + This produces the following execution flow: ```mermaid diff --git a/src/specify_cli/workflows/engine.py b/src/specify_cli/workflows/engine.py index d6a73bbeb0..6769552043 100644 --- a/src/specify_cli/workflows/engine.py +++ b/src/specify_cli/workflows/engine.py @@ -47,8 +47,8 @@ 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) + # Requirements declared by the workflow. Some permissions are enforced + # during validation and execution. self.requires: dict[str, Any] = data.get("requires", {}) # Inputs @@ -128,6 +128,10 @@ def validate_workflow(definition: WorkflowDefinition) -> list[str]: f"semantic versioning (expected X.Y.Z)." ) + # -- Requirements ----------------------------------------------------- + if not isinstance(definition.requires, dict): + errors.append("'requires' must be a mapping (or omitted).") + # -- Inputs ----------------------------------------------------------- if not isinstance(definition.inputs, dict): errors.append("'inputs' must be a mapping (or omitted).") @@ -150,12 +154,68 @@ def validate_workflow(definition: WorkflowDefinition) -> list[str]: if not definition.steps: errors.append("Workflow has no steps defined.") + errors.extend(_validate_shell_permissions(definition)) + seen_ids: set[str] = set() _validate_steps(definition.steps, seen_ids, errors) return errors +def _validate_shell_permissions(definition: WorkflowDefinition) -> list[str]: + """Validate explicit opt-in for workflows that run shell commands.""" + if not _workflow_uses_shell(definition.steps): + return [] + if _allows_shell_steps(definition.requires): + return [] + return [ + "Workflow uses shell steps but must declare " + "'requires.permissions.shell: true' before shell commands can run." + ] + + +def _allows_shell_steps(requires: Any) -> bool: + """Return whether the workflow explicitly opts into shell execution.""" + if not isinstance(requires, dict): + return False + permissions = requires.get("permissions") + if not isinstance(permissions, dict): + return False + return permissions.get("shell") is True + + +def _workflow_uses_shell(steps: Any) -> bool: + """Return True when any top-level or nested step has type 'shell'.""" + if not isinstance(steps, list): + return False + + for step_config in steps: + if not isinstance(step_config, dict): + continue + + if step_config.get("type", "command") == "shell": + return True + + for nested_key in ("then", "else", "steps"): + if _workflow_uses_shell(step_config.get(nested_key)): + return True + + cases = step_config.get("cases") + if isinstance(cases, dict): + for case_steps in cases.values(): + if _workflow_uses_shell(case_steps): + return True + + if _workflow_uses_shell(step_config.get("default")): + return True + + fan_step = step_config.get("step") + if isinstance(fan_step, dict) and _workflow_uses_shell([fan_step]): + return True + + return False + + def _validate_steps( steps: list[dict[str, Any]], seen_ids: set[str], @@ -400,6 +460,10 @@ def execute( """ from . import STEP_REGISTRY + shell_permission_errors = _validate_shell_permissions(definition) + if shell_permission_errors: + raise ValueError(shell_permission_errors[0]) + state = RunState( run_id=run_id, workflow_id=definition.id, diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 4c042fc7d5..7fa6a1b7c0 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1283,6 +1283,61 @@ def test_invalid_step_type(self): errors = validate_workflow(definition) assert any("invalid type" in e.lower() for e in errors) + def test_shell_step_requires_explicit_permission(self): + from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow + + definition = WorkflowDefinition.from_string(""" +workflow: + id: "test" + name: "Test" + version: "1.0.0" +steps: + - id: run-tests + type: shell + run: "pytest" +""") + errors = validate_workflow(definition) + assert any("requires.permissions.shell: true" in e for e in errors) + + def test_shell_step_with_permission_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: + permissions: + shell: true +steps: + - id: run-tests + type: shell + run: "pytest" +""") + errors = validate_workflow(definition) + assert errors == [] + + def test_nested_shell_step_requires_explicit_permission(self): + from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow + + definition = WorkflowDefinition.from_string(""" +workflow: + id: "test" + name: "Test" + version: "1.0.0" +steps: + - id: branch + type: if + condition: "{{ true }}" + then: + - id: run-tests + type: shell + run: "pytest" +""") + errors = validate_workflow(definition) + assert any("requires.permissions.shell: true" in e for e in errors) + def test_nested_step_validation(self): from specify_cli.workflows.engine import WorkflowDefinition, validate_workflow @@ -1392,6 +1447,9 @@ def test_execute_with_gate_pauses(self, project_dir): id: "gated" name: "Gated" version: "1.0.0" +requires: + permissions: + shell: true steps: - id: step-one type: shell @@ -1423,6 +1481,9 @@ def test_execute_with_shell_step(self, project_dir): id: "shell-test" name: "Shell Test" version: "1.0.0" +requires: + permissions: + shell: true steps: - id: echo type: shell @@ -1435,6 +1496,26 @@ def test_execute_with_shell_step(self, project_dir): assert state.status == RunStatus.COMPLETED assert "workflow-output" in state.step_results["echo"]["output"]["stdout"] + def test_execute_shell_without_permission_raises(self, project_dir): + from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition + + yaml_str = """ +schema_version: "1.0" +workflow: + id: "shell-test" + name: "Shell Test" + version: "1.0.0" +steps: + - id: echo + type: shell + run: "echo workflow-output" +""" + definition = WorkflowDefinition.from_string(yaml_str) + engine = WorkflowEngine(project_dir) + + with pytest.raises(ValueError, match="requires.permissions.shell: true"): + engine.execute(definition) + def test_execute_with_if_then(self, project_dir): from specify_cli.workflows.engine import WorkflowEngine, WorkflowDefinition from specify_cli.workflows.base import RunStatus @@ -1445,6 +1526,9 @@ def test_execute_with_if_then(self, project_dir): id: "branching" name: "Branching" version: "1.0.0" +requires: + permissions: + shell: true inputs: scope: type: string @@ -1569,6 +1653,9 @@ def test_list_after_execution(self, project_dir): id: "list-test" name: "List Test" version: "1.0.0" +requires: + permissions: + shell: true steps: - id: step-one type: shell @@ -1767,6 +1854,9 @@ def test_full_sequential_workflow(self, project_dir): name: "E2E Test" version: "1.0.0" integration: claude +requires: + permissions: + shell: true inputs: feature: type: string @@ -1814,6 +1904,9 @@ def test_switch_workflow(self, project_dir): id: "switch-test" name: "Switch Test" version: "1.0.0" +requires: + permissions: + shell: true inputs: action: type: string diff --git a/workflows/PUBLISHING.md b/workflows/PUBLISHING.md index ce0d251826..5df90dcd81 100644 --- a/workflows/PUBLISHING.md +++ b/workflows/PUBLISHING.md @@ -84,6 +84,14 @@ steps: on_reject: abort ``` +If your workflow contains any `type: shell` step, it must also opt in explicitly: + +```yaml +requires: + permissions: + shell: true +``` + **Validation Checklist**: - ✅ `id` is lowercase alphanumeric with hyphens (single-character IDs are allowed) @@ -91,6 +99,7 @@ steps: - ✅ `description` is concise - ✅ All step IDs are unique - ✅ Step types are valid: `command`, `prompt`, `shell`, `gate`, `if`, `switch`, `while`, `do-while`, `fan-out`, `fan-in` +- ✅ Shell steps are explicitly enabled with `requires.permissions.shell: true` - ✅ Required fields present per step type (e.g., `condition` for `if`, `expression` for `switch`) - ✅ Input types are valid: `string`, `number`, `boolean` - ✅ Step IDs do not contain `:` (reserved for engine-generated nested IDs like `parentId:childId`) diff --git a/workflows/README.md b/workflows/README.md index 31f736ff76..cd494a1bda 100644 --- a/workflows/README.md +++ b/workflows/README.md @@ -106,9 +106,15 @@ Send an arbitrary inline prompt to an integration CLI (no command file needed): ### Shell Steps -Run a shell command and capture output: +Run a shell command and capture output. Workflows that contain shell steps +must opt in explicitly at the top level: ```yaml +requires: + permissions: + shell: true + +steps: - id: run-tests type: shell run: "cd {{ inputs.project_dir }} && npm test"