Address follow-up security workflow review

Author: PascalThuet

.github/bandit-baseline.json

@@ -0,0 +1,31 @@
+{
+  "results": [
+    {
+      "code": "34                 run_cmd,\n35                 shell=True,\n36                 capture_output=True,\n37                 text=True,\n38                 cwd=cwd,\n39                 timeout=300,\n40             )\n41             output = {\n42                 \"exit_code\": proc.returncode,\n43                 \"stdout\": proc.stdout,\n",
+      "col_offset": 19,
+      "end_col_offset": 13,
+      "filename": "src/specify_cli/workflows/steps/shell/__init__.py",
+      "issue_confidence": "HIGH",
+      "issue_cwe": {
+        "id": 78,
+        "link": "https://cwe.mitre.org/data/definitions/78.html"
+      },
+      "issue_severity": "HIGH",
+      "issue_text": "subprocess call with shell=True identified, security issue.",
+      "line_number": 35,
+      "line_range": [
+        33,
+        34,
+        35,
+        36,
+        37,
+        38,
+        39,
+        40
+      ],
+      "more_info": "https://bandit.readthedocs.io/en/1.9.4/plugins/b602_subprocess_popen_with_shell_equals_true.html",
+      "test_id": "B602",
+      "test_name": "subprocess_popen_with_shell_equals_true"
+    }
+  ]
+}

.github/workflows/security.yml

@@ -29,7 +29,7 @@ jobs:
 
       - name: Run pip-audit
         run: |
-          uv export --quiet --extra test --frozen --format requirements.txt --no-emit-project --output-file /tmp/spec-kit-audit-requirements.txt
+          uv export --quiet --extra test --format requirements.txt --no-emit-project --output-file /tmp/spec-kit-audit-requirements.txt
           uvx --from pip-audit==2.10.0 pip-audit -r /tmp/spec-kit-audit-requirements.txt --progress-spinner off
 
   static-analysis:
@@ -48,4 +48,4 @@ jobs:
           python-version: "3.13"
 
       - name: Run Bandit
-        run: uvx --from bandit==1.9.4 bandit -r src -lll
+        run: uvx --from bandit==1.9.4 bandit -r src -lll --baseline .github/bandit-baseline.json

CONTRIBUTING.md

@@ -84,12 +84,12 @@ Run this when you change agent metadata, context update scripts, or integration
 #### Security checks
 
 ```bash
-uv export --quiet --extra test --frozen --format requirements.txt --no-emit-project --output-file /tmp/spec-kit-audit-requirements.txt
+uv export --quiet --extra test --format requirements.txt --no-emit-project --output-file /tmp/spec-kit-audit-requirements.txt
 uvx --from pip-audit==2.10.0 pip-audit -r /tmp/spec-kit-audit-requirements.txt --progress-spinner off
-uvx --from bandit==1.9.4 bandit -r src -lll
+uvx --from bandit==1.9.4 bandit -r src -lll --baseline .github/bandit-baseline.json
 ```
 
-Run these before changing dependency metadata, workflow execution code, subprocess usage, or security-sensitive paths. The dependency audit uses the locked runtime and `test` extra dependency set used by CI and contributors.
+Run these before changing dependency metadata, workflow execution code, subprocess usage, or security-sensitive paths. The dependency audit resolves the runtime and `test` extra dependency set used by CI and contributors.
 
 ### Manual testing
 

src/specify_cli/__init__.py

@@ -397,21 +397,24 @@ def callback(
         console.print()
 
 def run_command(cmd: list[str], check_return: bool = True, capture: bool = False, shell: bool = False) -> Optional[str]:
-    """Run a shell command and optionally capture output."""
+    """Run a command without invoking a shell and optionally capture output."""
+    if shell:
+        raise ValueError(
+            "run_command does not support shell=True; use a reviewed "
+            "subprocess.run call for shell-specific behavior."
+        )
+
     try:
         if capture:
-            # shell=True is only available to callers that opt in explicitly.
-            result = subprocess.run(  # nosec B602
+            result = subprocess.run(
                 cmd,
                 check=check_return,
                 capture_output=True,
                 text=True,
-                shell=shell,
             )
             return result.stdout.strip()
         else:
-            # shell=True is only available to callers that opt in explicitly.
-            subprocess.run(cmd, check=check_return, shell=shell)  # nosec B602
+            subprocess.run(cmd, check=check_return)
             return None
     except subprocess.CalledProcessError as e:
         if check_return:

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

@@ -32,7 +32,7 @@ def execute(self, config: dict[str, Any], context: StepContext) -> StepResult:
         try:
             proc = subprocess.run(
                 run_cmd,
-                shell=True,  # nosec B602
+                shell=True,
                 capture_output=True,
                 text=True,
                 cwd=cwd,

tests/test_security_workflow.py

@@ -2,26 +2,32 @@
 
 from __future__ import annotations
 
+import json
 import re
 from pathlib import Path
 
+import pytest
 import yaml
 
 
 REPO_ROOT = Path(__file__).resolve().parent.parent
 SECURITY_WORKFLOW = REPO_ROOT / ".github" / "workflows" / "security.yml"
 CONTRIBUTING = REPO_ROOT / "CONTRIBUTING.md"
+BANDIT_BASELINE = REPO_ROOT / ".github" / "bandit-baseline.json"
 
 AUDIT_REQUIREMENTS = "/tmp/spec-kit-audit-requirements.txt"
-EXPORT_TEST_DEPS = (
-    "uv export --quiet --extra test --frozen --format requirements.txt "
+EXPORT_TEST_EXTRA_DEPS = (
+    "uv export --quiet --extra test --format requirements.txt "
     f"--no-emit-project --output-file {AUDIT_REQUIREMENTS}"
 )
 PIP_AUDIT = (
     "uvx --from pip-audit==2.10.0 pip-audit "
     f"-r {AUDIT_REQUIREMENTS} --progress-spinner off"
 )
-BANDIT = "uvx --from bandit==1.9.4 bandit -r src -lll"
+BANDIT = (
+    "uvx --from bandit==1.9.4 bandit -r src -lll "
+    "--baseline .github/bandit-baseline.json"
+)
 
 
 def _load_security_workflow() -> dict:
@@ -39,11 +45,13 @@ def _step_run(job_name: str, step_name: str) -> str:
 class TestSecurityWorkflow:
     """Guard the security workflow against review-feedback regressions."""
 
-    def test_dependency_audit_uses_locked_test_extra_export(self):
+    def test_dependency_audit_uses_test_extra_export_without_lockfile_flags(self):
         run = _step_run("dependency-audit", "Run pip-audit")
 
-        assert EXPORT_TEST_DEPS in run
+        assert EXPORT_TEST_EXTRA_DEPS in run
         assert PIP_AUDIT in run
+        assert "--frozen" not in run
+        assert "--locked" not in run
         assert "uvx pip-audit ." not in run
 
     def test_security_tools_are_pinned(self):
@@ -61,10 +69,38 @@ def test_bandit_does_not_globally_skip_b602(self):
         assert run == BANDIT
         assert "--skip" not in run
         assert "--skip B602" not in workflow_text
+        assert "--baseline .github/bandit-baseline.json" in run
+
+    def test_bandit_baseline_only_ignores_shell_step_b602(self):
+        baseline = json.loads(BANDIT_BASELINE.read_text(encoding="utf-8"))
+        results = baseline["results"]
+
+        assert len(results) == 1
+        assert results[0]["test_id"] == "B602"
+        assert (
+            results[0]["filename"]
+            == "src/specify_cli/workflows/steps/shell/__init__.py"
+        )
+
+    def test_b602_is_not_suppressed_in_source(self):
+        source_text = "\n".join(
+            path.read_text(encoding="utf-8")
+            for path in (REPO_ROOT / "src").rglob("*.py")
+        )
+
+        assert "# nosec B602" not in source_text
+
+    def test_run_command_rejects_shell_true(self):
+        from specify_cli import run_command
+
+        with pytest.raises(ValueError, match="shell=True"):
+            run_command(["echo", "hello"], shell=True)
 
     def test_contributing_documents_security_commands(self):
         contributing_text = CONTRIBUTING.read_text(encoding="utf-8")
 
-        assert EXPORT_TEST_DEPS in contributing_text
+        assert EXPORT_TEST_EXTRA_DEPS in contributing_text
         assert PIP_AUDIT in contributing_text
         assert BANDIT in contributing_text
+        assert "--frozen" not in contributing_text
+        assert "--locked" not in contributing_text