refactor: send cmd verbatim, drop /bin/sh -c wrap
All checks were successful
ci/woodpecker/push/unit Pipeline was successful

commands.run / stream now forward the user-supplied command string as
`cmd` instead of repackaging it as `cmd=/bin/sh args=["-c", cmd]`.
Callers that want shell semantics or argv splitting handle it
themselves; stream still accepts an optional `args` list for explicit
argv. Updated docstrings and payload tests accordingly.
This commit is contained in:
2026-06-26 17:33:32 +06:00
parent d8ff68f014
commit ad454cd909
3 changed files with 53 additions and 54 deletions

View File

@ -120,8 +120,7 @@ def _build_exec_payload(
tag: str | None, tag: str | None,
) -> dict: ) -> dict:
payload: dict = { payload: dict = {
"cmd": "/bin/sh", "cmd": cmd,
"args": ["-c", cmd],
"background": background, "background": background,
} }
if timeout is not None and not background: if timeout is not None and not background:
@ -156,7 +155,7 @@ def _decode_exec_run(
def _build_stream_start(cmd: str, args: builtins.list[str] | None) -> dict: def _build_stream_start(cmd: str, args: builtins.list[str] | None) -> dict:
if args: if args:
return {"type": "start", "cmd": cmd, "args": args} return {"type": "start", "cmd": cmd, "args": args}
return {"type": "start", "cmd": "/bin/sh", "args": ["-c", cmd]} return {"type": "start", "cmd": cmd}
def _decode_exec_response(data: dict) -> CommandResult: def _decode_exec_response(data: dict) -> CommandResult:
@ -218,7 +217,9 @@ class Commands:
"""Execute a shell command inside the capsule. """Execute a shell command inside the capsule.
Args: Args:
cmd (str): Shell command string to execute. cmd (str): Command string to execute. Sent verbatim to the
server — no shell wrapping is applied. Provide a full
shell-quoted command if you need shell features.
background (bool): If ``True``, launch the process in the background (bool): If ``True``, launch the process in the
background and return a :class:`CommandHandle` immediately. background and return a :class:`CommandHandle` immediately.
Defaults to ``False``. Defaults to ``False``.
@ -308,10 +309,11 @@ class Commands:
"""Execute a command via WebSocket, streaming output as events. """Execute a command via WebSocket, streaming output as events.
Args: Args:
cmd (str): Command to execute. cmd (str): Command to execute. Sent verbatim to the server as
args (list[str] | None): Additional arguments for the command. the full command string.
When omitted, *cmd* is interpreted as a shell command args (list[str] | None): Optional explicit argument vector. When
string and executed via ``/bin/sh -c``. provided, *cmd* is treated as the program and *args* as its
argv tail. When omitted, *cmd* is forwarded as-is.
Yields: Yields:
StreamEvent: Successive events including :class:`StreamStartEvent`, StreamEvent: Successive events including :class:`StreamStartEvent`,
@ -378,7 +380,9 @@ class AsyncCommands:
"""Execute a shell command inside the capsule. """Execute a shell command inside the capsule.
Args: Args:
cmd (str): Shell command string to execute. cmd (str): Command string to execute. Sent verbatim to the
server — no shell wrapping is applied. Provide a full
shell-quoted command if you need shell features.
background (bool): If ``True``, launch the process in the background (bool): If ``True``, launch the process in the
background and return a :class:`CommandHandle` immediately. background and return a :class:`CommandHandle` immediately.
Defaults to ``False``. Defaults to ``False``.
@ -470,10 +474,11 @@ class AsyncCommands:
"""Execute a command via WebSocket, streaming output as events. """Execute a command via WebSocket, streaming output as events.
Args: Args:
cmd (str): Command to execute. cmd (str): Command to execute. Sent verbatim to the server as
args (list[str] | None): Additional arguments for the command. the full command string.
When omitted, *cmd* is interpreted as a shell command args (list[str] | None): Optional explicit argument vector. When
string and executed via ``/bin/sh -c``. provided, *cmd* is treated as the program and *args* as its
argv tail. When omitted, *cmd* is forwarded as-is.
Yields: Yields:
StreamEvent: Successive events including :class:`StreamStartEvent`, StreamEvent: Successive events including :class:`StreamStartEvent`,

View File

@ -145,8 +145,8 @@ class TestRunPayload:
route = respx.post(EXEC_URL).respond(200, json={"stdout": "hi", "exit_code": 0}) route = respx.post(EXEC_URL).respond(200, json={"stdout": "hi", "exit_code": 0})
result = _make_commands().run("echo hi") result = _make_commands().run("echo hi")
body = json.loads(route.calls[0].request.content) body = json.loads(route.calls[0].request.content)
assert body["cmd"] == "/bin/sh" assert body["cmd"] == "echo hi"
assert body["args"] == ["-c", "echo hi"] assert "args" not in body
assert body["background"] is False assert body["background"] is False
assert body["timeout_sec"] == 30 assert body["timeout_sec"] == 30
assert result.stdout == "hi" assert result.stdout == "hi"
@ -361,12 +361,12 @@ def _patch_async_ws(monkeypatch, ws: _AsyncFakeWS) -> None:
class TestStream: class TestStream:
def test_stream_sends_shell_wrapped_start(self, monkeypatch): def test_stream_sends_cmd_verbatim(self, monkeypatch):
ws = _FakeWS([{"type": "exit", "exit_code": 0}]) ws = _FakeWS([{"type": "exit", "exit_code": 0}])
_patch_sync_ws(monkeypatch, ws) _patch_sync_ws(monkeypatch, ws)
list(_make_commands().stream("echo hi")) list(_make_commands().stream("echo hi"))
start = json.loads(ws.sent[0]) start = json.loads(ws.sent[0])
assert start == {"type": "start", "cmd": "/bin/sh", "args": ["-c", "echo hi"]} assert start == {"type": "start", "cmd": "echo hi"}
def test_stream_with_explicit_args(self, monkeypatch): def test_stream_with_explicit_args(self, monkeypatch):
ws = _FakeWS([{"type": "exit", "exit_code": 0}]) ws = _FakeWS([{"type": "exit", "exit_code": 0}])
@ -480,7 +480,8 @@ class TestAsyncCommands:
events = [e async for e in _make_async_commands().stream("echo out")] events = [e async for e in _make_async_commands().stream("echo out")]
assert [e.type for e in events] == ["start", "stdout", "exit"] assert [e.type for e in events] == ["start", "stdout", "exit"]
start = json.loads(ws.sent[0]) start = json.loads(ws.sent[0])
assert start["cmd"] == "/bin/sh" assert start["cmd"] == "echo out"
assert "args" not in start
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_async_connect(self, monkeypatch): async def test_async_connect(self, monkeypatch):

View File

@ -988,14 +988,14 @@ class TestAsyncGit:
assert len(branches) == 2 assert len(branches) == 2
# ════════════════════════════════<EFBFBD><EFBFBD>═════════════════════════════════ # ═════════════════════════════════════════════════════════════════
# Command payload tests — verify /bin/sh -c wrapping # Command payload tests — verify cmd passed verbatim, no sh -c wrap
# ════════════════════════════<EFBFBD><EFBFBD><EFBFBD>══════════════════════<EFBFBD><EFBFBD><EFBFBD>══════════════ # ══════════════════════════════════════════════════════════════════
class TestCommandPayloadWrapping: class TestCommandPayloadWrapping:
"""Verify that Commands.run sends cmd=/bin/sh args=['-c', cmd_string] """Verify that Commands.run sends the full command string as ``cmd``
so the server-side wrapper expands "${@}" into proper argv.""" with no auto ``/bin/sh -c`` wrapping or args splitting."""
@respx.mock @respx.mock
def test_simple_command(self): def test_simple_command(self):
@ -1005,15 +1005,13 @@ class TestCommandPayloadWrapping:
git = _make_git() git = _make_git()
git.init("/repo") git.init("/repo")
body = json.loads(route.calls[0].request.content) body = json.loads(route.calls[0].request.content)
assert body["cmd"] == "/bin/sh" assert "args" not in body
assert body["args"] == ["-c", git_cmd_from_body(body)] assert "git" in body["cmd"]
# args[1] should contain the actual git command assert "init" in body["cmd"]
assert body["args"][0] == "-c"
assert "git" in body["args"][1]
@respx.mock @respx.mock
def test_command_with_pipes(self): def test_command_with_pipes(self):
"""Pipes and redirects work because /bin/sh interprets them.""" """Shell metacharacters are forwarded verbatim — caller owns shell semantics."""
from wrenn.client import WrennClient from wrenn.client import WrennClient
from wrenn.commands import Commands from wrenn.commands import Commands
@ -1023,8 +1021,8 @@ class TestCommandPayloadWrapping:
route = respx.post(EXEC_URL).respond(200, json=_exec_response(stdout="3\n")) route = respx.post(EXEC_URL).respond(200, json=_exec_response(stdout="3\n"))
commands.run("cat /etc/passwd | wc -l") commands.run("cat /etc/passwd | wc -l")
body = json.loads(route.calls[0].request.content) body = json.loads(route.calls[0].request.content)
assert body["cmd"] == "/bin/sh" assert body["cmd"] == "cat /etc/passwd | wc -l"
assert body["args"] == ["-c", "cat /etc/passwd | wc -l"] assert "args" not in body
@respx.mock @respx.mock
def test_command_with_semicolons(self): def test_command_with_semicolons(self):
@ -1037,8 +1035,8 @@ class TestCommandPayloadWrapping:
route = respx.post(EXEC_URL).respond(200, json=_exec_response()) route = respx.post(EXEC_URL).respond(200, json=_exec_response())
commands.run("cd /tmp; ls -la && echo done") commands.run("cd /tmp; ls -la && echo done")
body = json.loads(route.calls[0].request.content) body = json.loads(route.calls[0].request.content)
assert body["cmd"] == "/bin/sh" assert body["cmd"] == "cd /tmp; ls -la && echo done"
assert body["args"] == ["-c", "cd /tmp; ls -la && echo done"] assert "args" not in body
@respx.mock @respx.mock
def test_command_with_env_vars(self): def test_command_with_env_vars(self):
@ -1051,8 +1049,8 @@ class TestCommandPayloadWrapping:
route = respx.post(EXEC_URL).respond(200, json=_exec_response()) route = respx.post(EXEC_URL).respond(200, json=_exec_response())
commands.run("FOO=bar echo $FOO") commands.run("FOO=bar echo $FOO")
body = json.loads(route.calls[0].request.content) body = json.loads(route.calls[0].request.content)
assert body["cmd"] == "/bin/sh" assert body["cmd"] == "FOO=bar echo $FOO"
assert body["args"] == ["-c", "FOO=bar echo $FOO"] assert "args" not in body
@respx.mock @respx.mock
def test_command_with_subshell(self): def test_command_with_subshell(self):
@ -1065,8 +1063,8 @@ class TestCommandPayloadWrapping:
route = respx.post(EXEC_URL).respond(200, json=_exec_response()) route = respx.post(EXEC_URL).respond(200, json=_exec_response())
commands.run("echo $(date +%s)") commands.run("echo $(date +%s)")
body = json.loads(route.calls[0].request.content) body = json.loads(route.calls[0].request.content)
assert body["cmd"] == "/bin/sh" assert body["cmd"] == "echo $(date +%s)"
assert body["args"] == ["-c", "echo $(date +%s)"] assert "args" not in body
@respx.mock @respx.mock
def test_command_with_quotes_and_spaces(self): def test_command_with_quotes_and_spaces(self):
@ -1079,10 +1077,8 @@ class TestCommandPayloadWrapping:
route = respx.post(EXEC_URL).respond(200, json=_exec_response()) route = respx.post(EXEC_URL).respond(200, json=_exec_response())
commands.run("""echo "hello 'world'" | grep -o "'[^']*'" """) commands.run("""echo "hello 'world'" | grep -o "'[^']*'" """)
body = json.loads(route.calls[0].request.content) body = json.loads(route.calls[0].request.content)
assert body["cmd"] == "/bin/sh" assert "args" not in body
assert body["args"][0] == "-c" assert "hello 'world'" in body["cmd"]
# The command string is passed verbatim — shell interprets it
assert "hello 'world'" in body["args"][1]
@respx.mock @respx.mock
def test_heredoc_style_command(self): def test_heredoc_style_command(self):
@ -1095,20 +1091,18 @@ class TestCommandPayloadWrapping:
route = respx.post(EXEC_URL).respond(200, json=_exec_response()) route = respx.post(EXEC_URL).respond(200, json=_exec_response())
commands.run("python3 -c 'import sys; print(sys.version)'") commands.run("python3 -c 'import sys; print(sys.version)'")
body = json.loads(route.calls[0].request.content) body = json.loads(route.calls[0].request.content)
assert body["cmd"] == "/bin/sh" assert body["cmd"] == "python3 -c 'import sys; print(sys.version)'"
assert body["args"] == ["-c", "python3 -c 'import sys; print(sys.version)'"] assert "args" not in body
@respx.mock @respx.mock
def test_git_shlex_joined_command(self): def test_git_shlex_joined_command(self):
"""Git module uses shlex.join — verify it passes through correctly.""" """Git module uses shlex.join — verify it passes through as cmd."""
route = respx.post(EXEC_URL).respond(200, json=_exec_response()) route = respx.post(EXEC_URL).respond(200, json=_exec_response())
git = _make_git() git = _make_git()
git.clone("https://github.com/user/repo.git", "/tmp/repo", depth=1) git.clone("https://github.com/user/repo.git", "/tmp/repo", depth=1)
body = json.loads(route.calls[0].request.content) body = json.loads(route.calls[0].request.content)
assert body["cmd"] == "/bin/sh" assert "args" not in body
assert body["args"][0] == "-c" shell_cmd = body["cmd"]
# shlex.join produces: git clone --depth 1 https://... /tmp/repo
shell_cmd = body["args"][1]
assert "git" in shell_cmd assert "git" in shell_cmd
assert "clone" in shell_cmd assert "clone" in shell_cmd
assert "--depth" in shell_cmd assert "--depth" in shell_cmd
@ -1125,13 +1119,12 @@ class TestCommandPayloadWrapping:
route = respx.post(EXEC_URL).respond(200, json={"pid": 42, "tag": "bg-1"}) route = respx.post(EXEC_URL).respond(200, json={"pid": 42, "tag": "bg-1"})
commands.run("tail -f /var/log/syslog", background=True) commands.run("tail -f /var/log/syslog", background=True)
body = json.loads(route.calls[0].request.content) body = json.loads(route.calls[0].request.content)
assert body["cmd"] == "/bin/sh" assert body["cmd"] == "tail -f /var/log/syslog"
assert body["args"] == ["-c", "tail -f /var/log/syslog"] assert "args" not in body
assert body["background"] is True assert body["background"] is True
def git_cmd_from_body(body: dict) -> str: def git_cmd_from_body(body: dict) -> str:
"""Extract the shell command string from a wrapped payload.""" """Extract the command string from the payload."""
assert body["cmd"] == "/bin/sh" assert "args" not in body
assert body["args"][0] == "-c" return body["cmd"]
return body["args"][1]