Markus Fritsche
8708d93039
@testdesigner (Fable) designs an executable spec; @py (Haiku) grinds until green. Both are bullpen-lurker@ instances (.model fable / haiku) + dispatcher roster + bp timeouts + extended invoker allowlist. First ticket = reviewer finding #7 (_show shell-injection). Fable wrote a sharp black-box spec (tests/test_show_shell_safe.py) that PROVES the injection by pasting and executing the suggested command — 6 tests, and it independently caught the @architect debate-path variant too. Fix: _show now escapes the double-quote-context metacharacters (backslash, backtick, dollar, quote) so no command substitution or quote-breakout survives a copy-paste. 6/6 green. HONEST RESULT: the spec half worked; the GRINDER half did not — @py hit the 20-turn cap fighting remote-edit-over-sic (no Edit tool, re-running pytest across sic each cycle). So I closed the bug directly against Fable spec. Lesson: grinders need a LOCAL workspace (Edit + local pytest) and a higher turn budget, not remote-edit; and @testdesigner s reply was silently lost (reviewer #2 live). Grinder-harness rework + #2 are the follow-ups before @py is real. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EWpfhDgYNA21tETDP9ueBE
146 lines
5.7 KiB
Python
146 lines
5.7 KiB
Python
"""Reviewer finding #7 — bin/bullpen-dispatcher _show() must produce a SHELL-SAFE
|
|
copy-paste command.
|
|
|
|
_show() sanitises a request body into the `sic hertz ...` command the dispatcher
|
|
tells the user to run. It strips double-quotes and newlines but NOT backticks,
|
|
`$(...)` command substitution, or backslashes — so a body that merely *quotes*
|
|
third-party text turns the suggested command into a shell injection the user is
|
|
instructed to paste and run.
|
|
|
|
These tests are BLACK-BOX. For each payload they:
|
|
1. run the dispatcher one-shot (`--once <body>`) and grab the suggested `sic …` line,
|
|
2. execute that line exactly as a copy-pasting user would — but with `sic` replaced
|
|
by a harmless recorder stub, so nothing ever reaches the fleet,
|
|
3. assert the injected `touch <marker>` did NOT fire (no live command substitution
|
|
/ quote breakout) and the literal body text survived to `sic` inert.
|
|
|
|
They FAIL on the current code (the payload executes / the command won't parse) and
|
|
pass only once _show() properly shell-quotes the body.
|
|
"""
|
|
import os
|
|
import re
|
|
import shutil
|
|
import subprocess
|
|
import sys
|
|
import tempfile
|
|
from pathlib import Path
|
|
|
|
REPO = Path(__file__).resolve().parents[1]
|
|
DISPATCHER = REPO / "bin" / "bullpen-dispatcher"
|
|
LIB = REPO / "lib"
|
|
BASH = shutil.which("bash") or "/bin/bash"
|
|
|
|
|
|
def _suggested_command(body):
|
|
"""Run `bullpen-dispatcher --once <body>` and return the emitted `sic …` line."""
|
|
env = dict(os.environ, PYTHONPATH=str(LIB))
|
|
proc = subprocess.run(
|
|
[sys.executable, str(DISPATCHER), "--once", body],
|
|
capture_output=True, text=True, env=env, timeout=60,
|
|
)
|
|
assert proc.returncode == 0, f"dispatcher --once failed: {proc.stderr}"
|
|
cmds = [ln.strip() for ln in proc.stdout.splitlines() if re.match(r"\s*sic\b", ln)]
|
|
assert cmds, f"no suggested `sic` command in dispatcher output:\n{proc.stdout}"
|
|
return cmds[-1]
|
|
|
|
|
|
def _run_paste(make_body):
|
|
"""make_body(marker) -> a request body that, IF the suggested command is unsafe,
|
|
runs `touch <marker>`.
|
|
|
|
Returns (marker_fired, sic_argv, body, command):
|
|
marker_fired True if the injected command executed (the bug),
|
|
sic_argv the argument list the pasted command actually delivered to `sic`
|
|
(None if `sic` was never reached, e.g. an unterminated quote),
|
|
body/command the exact strings involved, for assertion messages.
|
|
"""
|
|
work = Path(tempfile.mkdtemp(prefix="showsafe_"))
|
|
try:
|
|
marker = work / "PWNED"
|
|
body = make_body(str(marker))
|
|
command = _suggested_command(body)
|
|
|
|
bindir = work / "bin"
|
|
bindir.mkdir()
|
|
argv_file = work / "sic_argv"
|
|
stub = bindir / "sic"
|
|
stub.write_text(
|
|
"#!/usr/bin/env bash\n"
|
|
f'printf "%s\\0" "$@" > "{argv_file}"\n'
|
|
"exit 0\n"
|
|
)
|
|
stub.chmod(0o755)
|
|
|
|
script = work / "paste.sh"
|
|
script.write_text(command + "\n")
|
|
env = dict(os.environ, PATH=f"{bindir}:/usr/bin:/bin")
|
|
subprocess.run([BASH, str(script)], cwd=str(work), env=env,
|
|
capture_output=True, text=True, timeout=60)
|
|
|
|
argv = None
|
|
if argv_file.exists():
|
|
parts = argv_file.read_bytes().split(b"\0")[:-1]
|
|
argv = [p.decode("utf-8", "surrogateescape") for p in parts]
|
|
return marker.exists(), argv, body, command
|
|
finally:
|
|
shutil.rmtree(work, ignore_errors=True)
|
|
|
|
|
|
def _assert_safe(fired, argv, body, command):
|
|
assert not fired, (
|
|
f"INJECTION EXECUTED: the suggested command ran the injected payload.\n"
|
|
f" suggested: {command}"
|
|
)
|
|
assert argv is not None, (
|
|
f"the suggested command did not parse / never reached `sic` "
|
|
f"(corrupted quoting).\n suggested: {command}"
|
|
)
|
|
assert body in "".join(argv), (
|
|
f"the request body was not delivered to `sic` as inert literal text "
|
|
f"(it was mangled or partly executed).\n"
|
|
f" body: {body!r}\n sic argv: {argv!r}\n suggested: {command}"
|
|
)
|
|
|
|
|
|
def test_backtick_command_substitution_does_not_execute():
|
|
# a body quoting third-party text that happens to contain backticks
|
|
fired, argv, body, cmd = _run_paste(
|
|
lambda m: f"please fetch https://x.test and summarize: `touch {m}`")
|
|
_assert_safe(fired, argv, body, cmd)
|
|
|
|
|
|
def test_dollar_paren_command_substitution_does_not_execute():
|
|
fired, argv, body, cmd = _run_paste(
|
|
lambda m: f"review this code $(touch {m})")
|
|
_assert_safe(fired, argv, body, cmd)
|
|
|
|
|
|
def test_nested_dollar_paren_command_substitution_does_not_execute():
|
|
fired, argv, body, cmd = _run_paste(
|
|
lambda m: f"look at $(echo $(touch {m})) for me")
|
|
_assert_safe(fired, argv, body, cmd)
|
|
|
|
|
|
def test_trailing_backslash_does_not_break_the_quoting():
|
|
# a trailing backslash escapes the naive closing double-quote, leaving the
|
|
# copy-paste command unterminated / corrupt (marker independent of body).
|
|
fired, argv, body, cmd = _run_paste(
|
|
lambda m: "please fetch the url now \\")
|
|
_assert_safe(fired, argv, body, cmd)
|
|
|
|
|
|
def test_backslash_then_shell_metachars_do_not_execute():
|
|
# backslash-escaped quote followed by shell code: if the backslash survives it
|
|
# neutralises the closing quote and the trailing command runs.
|
|
fired, argv, body, cmd = _run_paste(
|
|
lambda m: f"read this \\ ; touch {m} ; echo done")
|
|
_assert_safe(fired, argv, body, cmd)
|
|
|
|
|
|
def test_debate_conversant_path_is_also_shell_safe():
|
|
# the debate branch embeds the SAME _show(body) into a `room_say … body="…"`
|
|
# command, so it carries the identical exposure.
|
|
fired, argv, body, cmd = _run_paste(
|
|
lambda m: f"let's debate the design `touch {m}`")
|
|
_assert_safe(fired, argv, body, cmd)
|