naudachu
e330a11e8f
The guard tested whether the command string contained `tea` between whitespace. In a repository whose subject is the CLI, that blocked prose: an issue title, a commit message quoting a raw call, `grep -rn " tea "` and `echo tea`. The block message told the operator to add --login to git commit, which cannot be done — the only way past was to reword the sentence. The command is now tokenized (heredoc bodies dropped, line continuations folded, backticks and newlines treated as boundaries) and only words in command position count: the first word, and the first after a shell operator, past VAR=value assignments and prefix words. Quoting is what saves the prose — a title is one token, and a token is never a command. Every invocation in the line is checked and rewritten, not just the first: a half-rewritten line left the second call with an unset variable and no login. The whitelist is now per-invocation too, so quoting "tea logins list" beside a real call no longer launders it. An untokenizable line (unbalanced quotes) falls back to the old substring test, which over-matches and therefore blocks. Closes #29 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
212 lines
8.4 KiB
Python
212 lines
8.4 KiB
Python
#!/usr/bin/env python3
|
|
"""
|
|
What the guard guards: `tea` the command, not `tea` the word.
|
|
|
|
python3 -m unittest discover -s tests -v
|
|
|
|
The bug these tests hold down: the guard asked whether the string contained
|
|
`tea` surrounded by whitespace, so in a repository *about* the CLI it blocked
|
|
prose. An issue title, a commit message quoting a raw call, `grep -rn " tea "`
|
|
and `echo tea` were all refused, with a message telling the operator to add
|
|
`--login` to `git commit`. The advice could not be followed — the only way
|
|
past was to reword the sentence.
|
|
|
|
Two lines are held at once here, and neither may move without the other: the
|
|
four false positives pass, and every shape that really runs the CLI — after
|
|
`&&`, after a pipe, in a subshell, in a substitution, twice in one line — is
|
|
still blocked or still rewritten. A test that only proved the first would be
|
|
satisfied by deleting the guard.
|
|
|
|
No network and no `tea` binary: the hook is pure decision-making, so the
|
|
fixture is a directory with a pin in it and a JSON payload on stdin.
|
|
"""
|
|
import json
|
|
import os
|
|
import subprocess
|
|
import sys
|
|
import tempfile
|
|
import unittest
|
|
|
|
REPO = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
|
GUARD = os.path.join(REPO, "hooks", "tea-guard.sh")
|
|
|
|
sys.path.insert(0, os.path.join(REPO, "skills", "auth", "scripts"))
|
|
import pin # noqa: E402
|
|
|
|
LOGIN = "fixture/user"
|
|
|
|
ALLOW, BLOCK, REWRITE = "allow", "block", "rewrite"
|
|
|
|
|
|
class GuardCase(unittest.TestCase):
|
|
"""One temp project with one pinned login; the hook run as the harness
|
|
runs it."""
|
|
|
|
def setUp(self):
|
|
self._tmp = tempfile.TemporaryDirectory(prefix="tea-guard-")
|
|
self.root = os.path.realpath(self._tmp.name)
|
|
self.addCleanup(self._tmp.cleanup)
|
|
path = pin.settings_path(self.root)
|
|
os.makedirs(os.path.dirname(path), exist_ok=True)
|
|
with open(path, "w") as f:
|
|
f.write(json.dumps({"env": {pin.ENV_KEY: LOGIN}}))
|
|
|
|
def run_guard(self, cmd):
|
|
env = dict(os.environ)
|
|
env.pop("PYTHONPATH", None)
|
|
env[pin.PROJECT_DIR_ENV] = self.root
|
|
p = subprocess.run([sys.executable, GUARD],
|
|
input=json.dumps({"tool_input": {"command": cmd},
|
|
"cwd": self.root}),
|
|
cwd=self.root, env=env,
|
|
capture_output=True, text=True)
|
|
return p
|
|
|
|
def verdict(self, cmd):
|
|
p = self.run_guard(cmd)
|
|
if p.returncode == 2:
|
|
return BLOCK, p.stderr
|
|
self.assertEqual(p.returncode, 0, p.stderr)
|
|
if not p.stdout.strip():
|
|
return ALLOW, ""
|
|
got = json.loads(p.stdout)["hookSpecificOutput"]["updatedInput"]["command"]
|
|
return REWRITE, got
|
|
|
|
def assertVerdict(self, cmd, expected):
|
|
kind, detail = self.verdict(cmd)
|
|
self.assertEqual(kind, expected,
|
|
"%r → %s (%s)" % (cmd, kind, detail.strip()))
|
|
return detail
|
|
|
|
|
|
# --------------------------------------------------------------------------
|
|
# the four false positives, verbatim from the report
|
|
# --------------------------------------------------------------------------
|
|
|
|
class TestProseAboutTheCliRuns(GuardCase):
|
|
|
|
def test_an_issue_title_may_name_the_command(self):
|
|
self.assertVerdict(
|
|
'python3 skills/issue/scripts/issue_new.py --type bug '
|
|
'--title "Warn that tea pulls create needs the repo checkout" '
|
|
'--label comp/use --severity low', ALLOW)
|
|
|
|
def test_a_commit_message_may_quote_a_raw_call(self):
|
|
self.assertVerdict(
|
|
"git add -A && git commit -F- <<'EOF'\n"
|
|
"feat: close issues through a script\n"
|
|
"\n"
|
|
"Единственным способом сменить state был сырой вызов\n"
|
|
"tea api -X PATCH ... repos/OWNER/REPO/issues/N\n"
|
|
"EOF", ALLOW)
|
|
|
|
def test_a_one_line_commit_message_may_too(self):
|
|
self.assertVerdict('git commit -m "route it through tea api"', ALLOW)
|
|
|
|
def test_searching_the_repository_for_the_word(self):
|
|
for cmd in ('grep -rn " tea " docs/',
|
|
'grep -rn "tea api" skills/',
|
|
'echo tea'):
|
|
self.assertVerdict(cmd, ALLOW)
|
|
|
|
def test_the_word_as_a_bare_argument_is_still_an_argument(self):
|
|
"""`echo tea` was the smallest case in the report; these are the same
|
|
shape with the word in other argument positions."""
|
|
for cmd in ('ls tea', 'cat notes/tea', 'python3 x.py tea api'):
|
|
self.assertVerdict(cmd, ALLOW)
|
|
|
|
|
|
# --------------------------------------------------------------------------
|
|
# and the real thing is still guarded
|
|
# --------------------------------------------------------------------------
|
|
|
|
class TestRealInvocationsStayGuarded(GuardCase):
|
|
|
|
def test_a_bare_call_without_a_login_is_blocked(self):
|
|
detail = self.assertVerdict("tea issues list", BLOCK)
|
|
self.assertIn("--login", detail)
|
|
|
|
def test_the_placeholder_is_rewritten_to_the_pin(self):
|
|
got = self.assertVerdict(
|
|
'tea issues list --login "$GITEA_LOGIN" --state open', REWRITE)
|
|
self.assertIn(LOGIN, got)
|
|
self.assertNotIn("GITEA_LOGIN", got)
|
|
|
|
def test_a_login_named_by_hand_is_blocked(self):
|
|
detail = self.assertVerdict("tea issues list --login somebody", BLOCK)
|
|
self.assertIn("do not name the login", detail)
|
|
|
|
def test_another_variable_is_not_the_placeholder(self):
|
|
self.assertVerdict('tea issues list --login "$OTHER"', BLOCK)
|
|
|
|
def test_compound_commands_are_read_segment_by_segment(self):
|
|
for cmd in ('cd /tmp && tea issues list',
|
|
'echo x | tea api -X GET repos/x/y',
|
|
'( tea issues list )',
|
|
'cd /tmp; tea issues list',
|
|
'FOO=1 tea issues list',
|
|
'sudo tea issues list',
|
|
'xargs tea issues list'):
|
|
self.assertVerdict(cmd, BLOCK)
|
|
|
|
def test_substitutions_are_read_too(self):
|
|
for cmd in ('echo $(tea whoami)',
|
|
'x=$(tea whoami)',
|
|
'echo `tea whoami`'):
|
|
self.assertVerdict(cmd, BLOCK)
|
|
|
|
def test_a_guarded_call_beside_prose_that_mentions_the_word(self):
|
|
"""The two halves of the bug in one line: the guard must ignore the
|
|
argument and still catch the call."""
|
|
self.assertVerdict(
|
|
'git commit -m "route it through tea api" && tea issues list',
|
|
BLOCK)
|
|
|
|
def test_an_absolute_path_to_the_binary_is_the_binary(self):
|
|
self.assertVerdict("/usr/local/bin/tea issues list", BLOCK)
|
|
|
|
def test_every_call_in_the_line_is_rewritten(self):
|
|
"""A half-rewritten line leaves the second call with an unset variable
|
|
and therefore no login at all."""
|
|
got = self.assertVerdict(
|
|
'tea issues list --login "$GITEA_LOGIN" && '
|
|
'tea pulls list --login "$GITEA_LOGIN"', REWRITE)
|
|
self.assertEqual(got.count(LOGIN), 2)
|
|
self.assertNotIn("GITEA_LOGIN", got)
|
|
|
|
def test_a_second_unguarded_call_is_not_covered_by_the_first(self):
|
|
self.assertVerdict(
|
|
'tea issues list --login "$GITEA_LOGIN" && tea pulls list', BLOCK)
|
|
|
|
def test_prose_naming_the_whitelisted_form_does_not_launder_a_call(self):
|
|
"""`tea logins list` is allowed because it uses no identity. Quoting
|
|
that phrase must not turn the call beside it into a whitelisted one."""
|
|
self.assertVerdict(
|
|
'echo "run tea logins list first" && tea issues list', BLOCK)
|
|
|
|
|
|
class TestTheWhitelistStillApplies(GuardCase):
|
|
|
|
def test_login_enumeration_needs_no_pin(self):
|
|
for cmd in ("tea logins list", "tea logins ls",
|
|
"tea --version", "tea --help"):
|
|
self.assertVerdict(cmd, ALLOW)
|
|
|
|
def test_a_whitelisted_call_next_to_a_guarded_one_does_not_excuse_it(self):
|
|
self.assertVerdict("tea logins list && tea issues list", BLOCK)
|
|
|
|
|
|
class TestUnparseableLinesFailClosed(GuardCase):
|
|
"""An unbalanced quote means the shell's reading and ours may differ. The
|
|
old substring test decides — it over-matches, and over-matching blocks."""
|
|
|
|
def test_an_unterminated_quote_around_a_call_still_blocks(self):
|
|
self.assertVerdict('tea issues list --state "open', BLOCK)
|
|
|
|
def test_an_unterminated_quote_with_no_call_is_still_allowed(self):
|
|
self.assertVerdict('echo "unterminated', ALLOW)
|
|
|
|
|
|
if __name__ == "__main__":
|
|
unittest.main()
|