harden: reject shell=True in run_command (#3132)

run_command() forwarded shell= straight to subprocess.run, so a caller
passing shell=True would invoke a shell. Reject shell=True with ValueError
(keeping the parameter for signature compatibility) and drop shell= from
both subprocess.run calls.

Enable ruff S602/S604/S605 to flag any future shell=True reintroduction,
annotate the one intentional workflow shell sink with # noqa: S602, and
document the shell-step execution risk in workflows/PUBLISHING.md.

pyproject.toml

@@ -74,3 +74,13 @@ precision = 2
 show_missing = true
 skip_covered = false
 
+[tool.ruff.lint]
+# Lock in subprocess security posture: any reintroduction of shell=True
+# (or os.system / popen2) must be acknowledged with an explicit `# noqa`
+# pointing at the rule, making the deviation visible in review.
+extend-select = [
+    "S602",  # subprocess-popen-with-shell-equals-true
+    "S604",  # call-with-shell-equals-true
+    "S605",  # start-process-with-a-shell
+]
+

src/specify_cli/_utils.py

@@ -65,14 +65,31 @@ def dump_frontmatter(data: dict[str, Any]) -> str:
     return yaml.safe_dump(data, sort_keys=False, allow_unicode=True).strip()
 
 
-def run_command(cmd: list[str], check_return: bool = True, capture: bool = False, shell: bool = False) -> str | None:
-    """Run a shell command and optionally capture output."""
+def run_command(
+    cmd: list[str],
+    check_return: bool = True,
+    capture: bool = False,
+    shell: bool = False,
+) -> str | None:
+    """Run a command without invoking a shell and optionally capture output.
+
+    The ``shell`` parameter is kept in the signature so existing keyword
+    callers (and the re-export from ``specify_cli``) don't raise ``TypeError``,
+    but only the default ``shell=False`` is honoured. ``shell=True`` is
+    rejected with ``ValueError`` rather than silently ignored, so the
+    unsupported mode fails loudly instead of running with a different meaning.
+    """
+    if shell:
+        raise ValueError(
+            "run_command() does not support shell=True; pass argv as a list"
+        )
+
     try:
         if capture:
-            result = subprocess.run(cmd, check=check_return, capture_output=True, text=True, shell=shell)
+            result = subprocess.run(cmd, check=check_return, capture_output=True, text=True)
             return result.stdout.strip()
         else:
-            subprocess.run(cmd, check=check_return, shell=shell)
+            subprocess.run(cmd, check=check_return)
             return None
     except subprocess.CalledProcessError as e:
         if check_return:

src/specify_cli/workflows/steps/shell/__init__.py

@@ -31,7 +31,7 @@ class ShellStep(StepBase):
         # control commands; catalog-installed workflows should be reviewed
         # before use (see PUBLISHING.md for security guidance).
         try:
-            proc = subprocess.run(
+            proc = subprocess.run(  # noqa: S602 -- intentional shell=True (see NOTE above)
                 run_cmd,
                 shell=True,
                 capture_output=True,

tests/test_utils.py

@@ -0,0 +1,15 @@
+"""Tests for specify_cli._utils.run_command."""
+
+from __future__ import annotations
+
+import inspect
+
+import pytest
+
+from specify_cli import run_command
+
+
+def test_run_command_rejects_shell_execution_compatibly():
+    assert inspect.signature(run_command).parameters["shell"].default is False
+    with pytest.raises(ValueError, match="does not support shell=True"):
+        run_command(["echo", "blocked"], shell=True)  # noqa: S604

workflows/PUBLISHING.md

@@ -272,6 +272,17 @@ When releasing a new version:
 - **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
 
+#### Security: shell steps execute arbitrary code
+
+Workflow `shell` steps execute their `run` field through `/bin/sh` (POSIX) or the platform shell. There is no sandbox between the step and the user's machine: a malicious or buggy `run` block can read environment variables, modify files outside the project, exfiltrate data, or escalate privileges.
+
+Catalog-listed workflows are reviewed at submission time (see [Verification Process](#verification-process)), but you should still treat every install as code-execution from an untrusted source until you have read the `workflow.yml`:
+
+- **Before installing a workflow**, fetch the raw YAML and audit every `shell` step's `run` field directly. `specify workflow info <name>` only shows metadata (name, version, inputs, step IDs/types) — not the shell content that would actually execute.
+- **Prefer explicit commands over interpolation** in `run` blocks: `{{ inputs.something }}` substitutions should be quoted and constrained via `enum` so a malicious input can't inject shell syntax.
+- **Limit privilege**: shell steps inherit the user's environment. Workflows that need elevated access (sudo, secrets, GitHub tokens) should call them out explicitly in the README so reviewers can spot the requirement.
+- **Authors**: if your workflow has shell steps that look risky out of context (deletions, network calls, credential reads), document the rationale in your README. Maintainers will reject submissions whose shell steps can't be justified at review time.
+
 ### Integration Flexibility
 
 - **Set `integration` at workflow level** — use the `workflow.integration` field as the default