mirror of
https://github.com/github/spec-kit.git
synced 2026-08-03 06:26:30 +08:00
harden: remove shell parameter from run_command() (#3716)
run_command() enforces a list[str] argv contract, so a shell parameter served no purpose beyond keeping an unnecessary shell-injection surface that a future refactor could re-enable. Remove the parameter (and its now-dead ValueError guard) so shell=False is the only possible behavior. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 74a1bd02-f6cd-412a-b5a8-a7767a5e058d
This commit is contained in:
@@ -69,21 +69,14 @@ 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.
|
||||
Commands are always executed with ``shell=False`` and must be passed as an
|
||||
argv ``list[str]``. There is deliberately no ``shell`` parameter: the
|
||||
argv-list contract makes shell interpolation impossible by construction, so
|
||||
the shell-injection surface cannot be re-enabled at a call site.
|
||||
"""
|
||||
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)
|
||||
|
||||
@@ -9,7 +9,13 @@ 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
|
||||
def test_run_command_has_no_shell_parameter():
|
||||
"""The shell-injection surface is removed at the API level.
|
||||
|
||||
``run_command`` must never accept a ``shell`` parameter: the argv-list
|
||||
contract makes shell interpolation impossible by construction, and there is
|
||||
no runtime mode to re-enable it. Passing ``shell=`` is a hard ``TypeError``.
|
||||
"""
|
||||
assert "shell" not in inspect.signature(run_command).parameters
|
||||
with pytest.raises(TypeError):
|
||||
run_command(["echo", "blocked"], shell=True) # type: ignore[call-arg] # noqa: S604
|
||||
|
||||
Reference in New Issue
Block a user