Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 10 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
]

25 changes: 21 additions & 4 deletions src/specify_cli/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion src/specify_cli/workflows/steps/shell/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def execute(self, config: dict[str, Any], context: StepContext) -> StepResult:
# 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,
Expand Down
15 changes: 15 additions & 0 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -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
11 changes: 11 additions & 0 deletions workflows/PUBLISHING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down