Skip to content

Commit 0c510e1

Browse files
committed
Route the probe's four shell calls through one audited chokepoint
Promotion into src/ put four pre-existing shell=True calls under agentseam-no-shell-true for the first time: security.yml scans src/ and tests/, never tools/, so the rule had not seen this code. On main, src/ has no shell=True and tools/ has exactly these four. The rule is working; the code is not new. Its prescribed fix -- pass an argv list -- would be wrong here. reference_agent is Claude Code's protocol made executable, and Claude Code runs a hook by handing a command string to a shell; an argv list would emulate a vendor that does not exist, and every tested-basis row is measured against this driver. The transform trial runs the command as the hook rewrote it, so measuring the rewrite means running the rewrite. So the exception is real, and now lives in one place instead of four: _shell.py's run_shell() carries the single nosemgrep and the trust-boundary rationale beside it. The rule stays global and unmodified. Verified narrow: a canary shell=True added elsewhere in the same package still fails semgrep, and passes again once removed. semgrep exit 0 on CI's own invocation; 1615 passed, 4 skipped; ruff and ruff format clean. Probe re-checked end to end: agreement exits 0, a forced disagreement prints DISAGREES and exits 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Claude <noreply@anthropic.com>
1 parent c59bce6 commit 0c510e1

3 files changed

Lines changed: 37 additions & 9 deletions

File tree

src/agentseam/probe/_shell.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
"""The one place this package hands a command string to a shell.
2+
3+
`agentseam-no-shell-true` bans `shell=True` across `src/`, and it is right to: the runtime
4+
path sits inline in a developer's agent loop, on payloads an attacker can influence. The
5+
probe is not that path. It is a deliberate command-runner, and the shell is the thing under
6+
measurement rather than an implementation detail:
7+
8+
- `reference_agent` is Claude Code's protocol made executable, and Claude Code runs a hook by
9+
handing a command string to a shell. An argv list would emulate a vendor that does not
10+
exist, and every `tested`-basis row is measured against this driver.
11+
- The `transform` trial runs the command *as the hook rewrote it*. Measuring the rewrite means
12+
running the rewrite.
13+
- The driver template is a command line the operator typed on their own machine.
14+
15+
So the exception is real, and it is confined to this function rather than scattered across four
16+
call sites: one place to audit, and the rule keeps biting everywhere else in `src/`.
17+
"""
18+
19+
from __future__ import annotations
20+
21+
import subprocess
22+
23+
24+
def run_shell(command, *, check=False, capture_output=True, **kwargs):
25+
"""`subprocess.run(command, shell=True)`, defaulted to capture and never to raise."""
26+
# nosemgrep: agentseam-no-shell-true -- the audited exception; see this module's docstring.
27+
return subprocess.run( # noqa: S602
28+
command, shell=True, check=check, capture_output=capture_output, **kwargs
29+
)

src/agentseam/probe/experiment_driver.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
import json
1212
import subprocess
1313

14+
from . import _shell
15+
1416

1517
def drive_real(command, workspace, *, trigger):
1618
"""Drive a real agent CLI in the scratch workspace.
@@ -26,9 +28,7 @@ def drive_real(command, workspace, *, trigger):
2628
prompt = "Run this exact shell command and nothing else: %s" % trigger
2729
filled = command.replace("{prompt}", json.dumps(prompt))
2830
try:
29-
proc = subprocess.run( # noqa: S602
30-
filled, shell=True, cwd=workspace, capture_output=True, text=True, timeout=300, check=False
31-
)
31+
proc = _shell.run_shell(filled, cwd=workspace, text=True, timeout=300)
3232
except subprocess.TimeoutExpired as exc:
3333
return {
3434
"returncode": None,

src/agentseam/probe/reference_agent.py

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
import subprocess
3434
import sys
3535

36+
from . import _shell
37+
3638
CLAUDE_CODE_TIMEOUT_SECONDS = 60
3739

3840
ALLOW, DENY, ASK = "allow", "deny", "ask"
@@ -145,14 +147,11 @@ def _gate(config_path, event, payload, *, cwd, timeout):
145147
for hook_command in _hook_commands(config_path, event):
146148
state["invoked"] = True
147149
try:
148-
proc = subprocess.run( # noqa: S602 - the command under test is ours, in a scratch dir
150+
proc = _shell.run_shell(
149151
hook_command,
150-
shell=True,
151152
input=blob,
152-
capture_output=True,
153153
cwd=cwd,
154154
timeout=timeout if timeout is not None else CLAUDE_CODE_TIMEOUT_SECONDS,
155-
check=False,
156155
)
157156
except subprocess.TimeoutExpired:
158157
state.update({"timed_out": True, "reason": "hook timed out, non-blocking", "exit": None})
@@ -200,7 +199,7 @@ def run_turn(config_path, *, command, cwd, session_id="reference-run", timeout=N
200199
)
201200
gates.setdefault("PreToolUse", pre)
202201
if pre["decision"] == ALLOW:
203-
subprocess.run(pre["updated"] or command, shell=True, cwd=cwd, capture_output=True, check=False) # noqa: S602
202+
_shell.run_shell(pre["updated"] or command, cwd=cwd)
204203
runs += 1
205204

206205
stop = _gate(
@@ -234,7 +233,7 @@ def run_pre_tool(config_path, *, command, cwd, session_id="reference-run", timeo
234233
)
235234
ran = False
236235
if state["decision"] == ALLOW:
237-
subprocess.run(state["updated"] or command, shell=True, cwd=cwd, capture_output=True, check=False) # noqa: S602
236+
_shell.run_shell(state["updated"] or command, cwd=cwd)
238237
ran = True
239238
return {
240239
"decision": state["decision"],

0 commit comments

Comments
 (0)