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

AGENTS.md

@@ -134,12 +134,64 @@ and keyword argument is checked independently.
        SecretPatternFilter(),
    ])
    ```
-5. **Only `str`, `bytes`, `list`, `tuple`, `set`, and `dict` keys and values are inspected.** A value
+5. **`ShellCommandAllowList` is the rule for shell tools.** If the user's agent can run shell
+   commands, suggest it. It is default-deny and matches **structured argv**, not text, so a
+   leading path (`/bin/curl`), quoting, a chained command (`ls; curl …`), a newline, an
+   environment assignment (`PATH=/tmp/pwn ls`), or an inline interpreter script (`sh -c "…"`,
+   `python -cCODE`, `bash -lc "…"`, `node --eval=…`) are each refused rather than sliding past a
+   textual prefix check. Unparseable input fails closed. Each entry is an argv **prefix**:
+   `"git status"` permits `git status --short`, not `git push`.
+
+   ```python
+   from modelfuzz import PolicyEngine, ShellCommandAllowList
+
+   engine = PolicyEngine([ShellCommandAllowList(["git status", "ls"])])
+   ```
+
+   Caveats you must pass on:
+
+   - **Never tell a user it is safe to allowlist an interpreter.** Inline-script rejection is
+     best-effort, not a boundary — `awk 'BEGIN{system("id")}'` needs no flag at all, and
+     `sh script.sh` runs a file the rule never sees. Allowlisting `sh`, `python`, `node` or `awk`
+     is close to allowlisting arbitrary execution, and the constructor warns about it. Recommend
+     allowlisting the specific program instead.
+   - **Environment assignments are refused, not stripped.** `PATH=`, `LD_PRELOAD=` and
+     `GIT_SSH_COMMAND=` each subvert an allowlisted binary, so all assignments are blocked by
+     default. If the user genuinely needs one, that is `allowed_env={"LANG"}` — and opting a name
+     in means accepting whatever value the caller supplies.
+   - It treats **every string it sees as a command**, so put it on an engine guarding a tool whose
+     only string argument is the command (a second string argument such as `cwd` will be blocked).
+   - It governs the command, not what the command then does — an allowlisted `git` still accepts
+     `git config`. And an allowlisted *name* runs whatever the OS resolves it to, so advise
+     pinning `PATH` and the working directory at the tool.
+6. **`NoDangerousShellPatterns` is a tripwire — never call it a security boundary.** It matches raw
+   text against a fixed table (`rm -rf`, `curl … | sh`, `$(…)`, `sudo`, `/etc/shadow`). A renamed
+   binary, base64, or unusual quoting defeats it. Offer it as a cheap second layer, or where the
+   commands cannot be enumerated — but if the user can list the commands they need, recommend
+   `ShellCommandAllowList` instead. Do not present the two as equivalent.
+7. **Only `str`, `bytes`, `list`, `tuple`, `set`, and `dict` keys and values are inspected.** A value
    in a custom object is not inspected and will pass. Do not assume full coverage.
-6. **Policies see one argument at a time.** A rule cannot express "amount > 1000 only when
+   (`ShellCommandAllowList` is the one exception to the dict rule: it reads dict *values* but not
+   *keys*, since a field name is not a command.)
+8. **Policies see one argument at a time.** A rule cannot express "amount > 1000 only when
    account is external", because it never sees the whole call.
-7. **Catch `ModelFuzzBlockError` in the agent loop.** Feed the block reason back to the model as
-   a tool error so it can recover, rather than letting it crash the run.
+9. **Catch `ModelFuzzBlockError` in the agent loop, and branch on `.category`.** Feed the reason
+   back to the model as a tool error so it can recover, rather than letting it crash the run. Write
+   recovery logic against `exc.category` — a stable string such as `credential`, `not_allowlisted`,
+   `metacharacter` or `interpreter` — and never against `exc.reason`, which is prose for a human
+   audit log and may be reworded between releases.
+
+   ```python
+   from modelfuzz import CATEGORY_CREDENTIAL, ModelFuzzBlockError
+
+   try:
+       result = run_tool(...)
+   except ModelFuzzBlockError as exc:
+       if exc.category == CATEGORY_CREDENTIAL:
+           result = "Blocked: that argument contained a credential. Retry without it."
+       else:
+           result = f"Tool call blocked by policy: {exc}"
+   ```
 
 ## Red-teaming a target
 

CHANGELOG.md

@@ -4,6 +4,17 @@ All notable changes to this project are documented here.
 
 ## [Unreleased]
 
+- feat: add `ShellCommandAllowList`, a default-deny allowlist for shell commands that matches **structured argv** rather than raw-string prefixes. Textual matching is defeated by a leading path (`/bin/curl`), quoting, a chained command (`ls; curl …`), an embedded newline, an environment assignment, or an inline interpreter script; each of those is refused, and unparseable input fails closed. Entries are argv prefixes, so `"git status"` permits `git status --short` but not `git push`. Closes the gap where `shell.run` was named in the README's threat model with no bundled rule to cover it. Fixes #71
+- security: `ShellCommandAllowList` refuses environment assignments rather than stripping them. An earlier revision of this branch skipped `NAME=value` tokens to find the real binary and then discarded them, which made `PATH=/tmp/pwn ls`, `LD_PRELOAD=… ls` and `GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=core.fsmonitor GIT_CONFIG_VALUE_0=id git status` all pass while executing attacker-chosen code through an allowlisted command. Assignments are now blocked by default — the dangerous names are not enumerable — with an explicit `allowed_env={"LANG"}` opt-in. Found by an adversarial sweep before release; never shipped
+- security: inline-script detection is structural instead of a list of four exact spellings. `bash -lc`, `sh -ec`, `python -cCODE`, `perl -0e`, `node --eval`, `node --eval=…`, `node -p`, `python -` and `sh -s` all reached the interpreter unblocked before; bundled short clusters, values attached to the flag, long forms with `=value`, and stdin scripts are now all recognised. Found by the same sweep
+- docs: correct two claims this branch made that the code did not honour — environment-assignment *stripping* was described as a hardening feature when it was the largest hole, and "inline interpreter scripts are rejected … even if the interpreter is allowlisted" was false for most spellings. The guarantee is now scoped honestly: inline-script rejection is best-effort, `awk 'BEGIN{system(…)}'` needs no flag at all, and `ShellCommandAllowList` emits a `UserWarning` when the allowlist names a known interpreter
+- feat: `ShellCommandAllowList` rejects an allowlist entry that could never match (one beginning with an assignment, or naming bare `env`), instead of silently leaving the defender believing it was in force
+- feat: add `NoDangerousShellPatterns`, a raw-text tripwire for the unsubtle (`rm -rf`, `curl … | sh`, `$(…)`, `sudo`, `/etc/shadow`). Documented throughout as a tripwire, explicitly **not** a shell parser or security boundary — unlike the allowlist it only blocks on a positive match, so it is safe to attach to a multi-argument tool
+- feat: `Violation` gains a machine-readable `category` field, and every bundled rule now sets one (`credential`, `sensitive_keyword`, `not_allowlisted`, `invalid_url`, `scheme_not_allowed`, `userinfo_trick`, `metacharacter`, `interpreter`, `destructive_command`, `network_utility`, `unparseable`). Defaults to `unspecified`, so a hand-written policy predating the field keeps working
+- feat: `ModelFuzzBlockError` exposes `.category`, `.rule_name` and `.violation`, so an agent loop can branch on *why* a call was blocked instead of regex-matching the reason text — a block is a policy decision, not an infrastructure failure, and the two want different handling. `str(exc)` is unchanged
+- feat: blocks are logged with an additional `modelfuzz_category` structured field
+- docs: add "Branching on why a call was blocked" and "Guarding shell commands" README sections, record both shell rules' limits in Limitations, and extend `AGENTS.md` with the shell rules and the rule that recovery logic keys on `.category`, never on `.reason`
+
 - feat: add `SecretPatternFilter`, a bundled policy that blocks tool-call arguments carrying a recognisable credential — Anthropic, OpenAI, Stripe, AWS, GitHub, Google and Slack key formats, JWTs, and PEM private-key headers. Where `SensitiveDataFilter` matches the *word* "password", this matches the *shape* of a real key, closing the gap where a live `sk-…` or `AKIA…` passed straight through the bundled default. Opt-in: the bare `@shield_tool` default is unchanged. Extend with `extra_patterns=` or replace the table with `patterns=`. Fixes #70
 - fix: the block reason for a matched credential names the format only and never quotes the matched text — blocks are logged at `WARNING`, and a reason carrying the key would leak the very thing the rule exists to contain
 - docs: add a "Blocking real credentials" README section, record `SecretPatternFilter`'s limits (listed formats only; matches shape, not validity) in Limitations, and update `AGENTS.md` so assistants stop reporting that ModelFuzz cannot detect credentials

README.md

@@ -77,7 +77,25 @@ except ModelFuzzBlockError as e:
     result = f"Tool call blocked by policy: {e}"   # hand this back to the model
 ```
 
-Blocks are also logged at `WARNING` on the `modelfuzz` logger with structured fields (`modelfuzz_tool`, `modelfuzz_rule`, `modelfuzz_reason`) for your audit trail. Nothing is ever written to stdout.
+Blocks are also logged at `WARNING` on the `modelfuzz` logger with structured fields (`modelfuzz_tool`, `modelfuzz_rule`, `modelfuzz_category`, `modelfuzz_reason`) for your audit trail. Nothing is ever written to stdout.
+
+### Branching on why a call was blocked
+
+A block is a policy decision, not an infrastructure failure, and the two deserve different handling. `ModelFuzzBlockError` carries a stable `category` so the agent loop can tell them apart without parsing English:
+
+```python
+from modelfuzz import CATEGORY_CREDENTIAL, ModelFuzzBlockError
+
+try:
+    result = http_post(url, body)
+except ModelFuzzBlockError as e:
+    if e.category == CATEGORY_CREDENTIAL:
+        result = "Blocked: that argument contained a credential. Retry without it."
+    else:
+        result = f"Tool call blocked by policy: {e}"   # hand back to the model
+```
+
+`e.reason` is prose for a human reading the audit log and may be reworded between releases — `e.category` is the part to branch on. The categories are `credential`, `sensitive_keyword`, `not_allowlisted`, `invalid_url`, `scheme_not_allowed`, `userinfo_trick`, `metacharacter`, `interpreter`, `environment_assignment`, `destructive_command`, `network_utility`, `unparseable`, and `unspecified` for custom policies that don't set one.
 
 > **Using the bare `@shield_tool`?** It applies a default `SensitiveDataFilter` that matches the literal strings `secret`, `password`, and `api_key` — a demo default, not a credential scanner. For real credential formats, add [`SecretPatternFilter`](#blocking-real-credentials) to your engine. See [Limitations](#limitations).
 
@@ -111,6 +129,61 @@ SecretPatternFilter(extra_patterns={"internal token": r"INT-[0-9]{8}"})
 
 Pass `patterns=` instead of `extra_patterns=` to replace the bundled table entirely. It is a format matcher, not a validity check or an entropy scanner — see [Limitations](#limitations).
 
+### Guarding shell commands
+
+If your agent can run shell commands, enumerate what it's allowed to run. `ShellCommandAllowList` is default-deny and matches **structured argv**, not text:
+
+```python
+from modelfuzz import PolicyEngine, ShellCommandAllowList, shield_tool
+
+engine = PolicyEngine([ShellCommandAllowList(["git status", "ls"])])
+
+@shield_tool(engine=engine)
+def run_shell(command: str) -> str:
+    return subprocess.run(command, shell=True, capture_output=True, text=True).stdout
+
+run_shell("git status --short")   # ok — "git status" is an allowed argv prefix
+run_shell("git push")             # ModelFuzzBlockError: Command not in allowlist: 'git'
+```
+
+Each entry is an **argv prefix**, so `"git status"` permits `git status --short` but not `git push`. Textual prefix matching would fall to any of these; 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` |
+| `PATH=/tmp/pwn ls` | blocked | `environment_assignment` |
+| `LD_PRELOAD=/tmp/evil.so ls` | blocked | `environment_assignment` |
+| `GIT_SSH_COMMAND=id git status` | blocked | `environment_assignment` |
+| `env PATH=/tmp/pwn ls` | blocked | `environment_assignment` |
+| `sh -c "…"`, `bash -lc "…"`, `python -cCODE`, `node --eval=…`, `node -p`, `python -` | blocked | `interpreter` |
+| `sudo ls` | blocked — wrappers are not unwrapped | `not_allowlisted` |
+| `ls "unbalanced` | blocked — unparseable fails closed | `unparseable` |
+
+**Environment assignments are refused, not stripped.** The environment decides which program a name resolves to and how it behaves, so an assignment subverts the binary the allowlist just approved — `PATH=` redirects it, `LD_PRELOAD=` injects into it, and `GIT_CONFIG_*`/`GIT_SSH_COMMAND=` turn an allowlisted `git status` into a way to run anything. The dangerous names are not enumerable, so the default is to refuse all of them. Name the ones you need:
+
+```python
+ShellCommandAllowList(["ls"], allowed_env={"LANG"})
+```
+
+Three things to know before you reach for it:
+
+- **Do not allowlist an interpreter.** Rejecting `-c` is best-effort, not a boundary: `awk 'BEGIN{system("id")}'` carries its program as a plain positional argument, and `sh script.sh` or `python evil.py` run a file this rule never sees. Allowlisting `sh`, `python`, `node` or `awk` is close to allowlisting arbitrary execution — the constructor warns you when you do. Allowlist the specific *program* instead.
+- **An allowlisted name authorises whatever the OS resolves it to.** This rule reads the command, not the filesystem. Pin `PATH` and the working directory at the tool, or an attacker-writable `./ls` is still an `ls`.
+- **It treats every string it sees as a command.** A policy sees one argument at a time and cannot know its name, so there is no way to distinguish a `command` argument from a `cwd` one. Put it on an engine guarding a tool whose only string argument is the command.
+- **It governs the command, not what the command then does.** An allowlisted `git` still accepts `git config`. Allowlist the narrowest prefix that does the job.
+
+Where you can't enumerate the commands, `NoDangerousShellPatterns` is a cheap second layer:
+
+```python
+from modelfuzz import NoDangerousShellPatterns
+
+engine = PolicyEngine([NoDangerousShellPatterns(), ShellCommandAllowList(["git status"])])
+```
+
+It matches raw text against a fixed table — `rm -rf`, `curl … | sh`, `$(…)`, `sudo`, `/etc/shadow` — and blocks only on a positive match, so unlike the allowlist it's safe to attach to a multi-argument tool. **It is a tripwire, not a shell parser and not a security boundary**: it catches the unsubtle and will not stop an attacker who knows it's there. See [Limitations](#limitations).
+
 ## When to use ModelFuzz
 
 **Use it if:**
@@ -205,6 +278,10 @@ ModelFuzz is pre-1.0 and provides the interception point, the policy protocol, a
 
 - **The default filter is a keyword tripwire, not a secret scanner.** `SensitiveDataFilter` matches the literal strings `secret`, `password`, and `api_key`. It does not recognise credential formats, so a real `sk-…` or `AKIA…` key passes straight through — while ordinary prose containing "password" is blocked. Treat it as a demo default; add `SecretPatternFilter` for credential formats, and write policies for your own threat model.
 - **`SecretPatternFilter` matches known formats, not secrets in general.** It recognises the credential shapes listed in [Blocking real credentials](#blocking-real-credentials) and nothing else: a bespoke internal token, a bare high-entropy string, or a provider not in the table passes untouched. It also matches *shape, not validity* — a revoked key, a docs placeholder, or a test fixture in the right shape is blocked exactly like a live credential. Use `extra_patterns=` for your own formats.
+- **`NoDangerousShellPatterns` is a tripwire, not a shell parser or a security boundary.** It matches raw text against a fixed table. Base64, unusual quoting, a renamed binary, or a utility not in the table all walk straight past it, and ordinary prose containing `curl` or a `|` trips it. Use it as a cheap second layer; where you can enumerate the commands your agent needs, `ShellCommandAllowList` is the boundary.
+- **`ShellCommandAllowList` treats every string it sees as a command, and governs only the command itself.** Because a policy cannot know an argument's name, a second string argument (a `cwd`, say) is judged as a command and blocked — put it on an engine guarding a tool whose only string argument is the command. And an allowlisted binary is allowlisted with all its own options: `git` still accepts `git config`. Dict *keys* are not read as commands, only values. Parsing follows `shlex` POSIX rules, which is close to `sh` but not identical to every shell in every mode.
+- **Allowlisting an interpreter is close to allowlisting arbitrary execution.** `ShellCommandAllowList` rejects inline-script invocation (`-c`, `-cCODE`, `-lc`, `--eval=…`, `-p`, `-`) on a best-effort basis, but that is a tripwire around one vector, not a boundary: `awk 'BEGIN{system("id")}'` carries its program as a positional argument with no flag at all, and `sh script.sh` or `python evil.py` execute a file the rule never sees. The constructor emits a `UserWarning` when your allowlist names a known interpreter. Allowlist the specific program instead.
+- **An allowlisted name authorises whatever the OS resolves it to.** The rule reads the command, not the filesystem, so it cannot tell `/bin/ls` from an attacker-written `./ls`. It refuses `PATH=`/`LD_PRELOAD=` assignments in the command, but the ambient environment is yours to control: pin `PATH` and the working directory at the tool.
 - **Unrecognised argument types are not inspected, and pass.** Only `str`, `bytes`, `list`, `tuple`, `set`, and `dict` keys and values are walked. A secret carried in a custom object is *not* checked and the call proceeds — the default is to allow what it cannot read.
 - **Policies see one argument at a time.** A rule cannot express "amount > 1000 only when account is external", because it never sees the whole call.
 - **It does not inspect prompts or model output** — only tool-call arguments. It is not a content filter.