Add bundled shell-command policy (e.g. ShellCommandAllowList / NoDangerousShellPatterns) — URL is covered, shell exec isn't

[higagan]

Problem

The README's own motivating threat list for @shield_tool includes shell.run alongside http_post:

If your agent calls tools that act on the outside world — http_post, shell.run, file read/write, database queries, sending email — then wrap those tools with @shield_tool.

But the only strong bundled policy today is URLAllowList. There is no equivalent bundled rule for shell commands — a user whose agent can call shell.run/subprocess-style tools has to hand-write their own policy from scratch, with no guidance on what a reasonable default even looks like (shell metacharacters, command allowlisting, etc.).

Arbitrary shell execution via prompt injection is at least as severe a threat as URL exfiltration, and it's explicitly named as an in-scope use case, so the gap is inconsistent with the rest of the product's positioning.

Proposal

Add a bundled shell-command policy, default-deny in the same spirit as URLAllowList:

• NoDangerousShellPatterns — blocks shell metacharacters (;, |, &&, $(, backticks), redirection, and common exfil/destructive patterns (curl, wget, rm -rf, piping to sh/bash) — a quick raw-string tripwire. Must be documented explicitly as a tripwire, not a shell parser or security boundary, the same way SensitiveDataFilter is documented today (see Limitations).
• ShellCommandAllowList — default-deny allowlist of permitted commands, mirroring URLAllowList's allowed-domains model. Per discussion, this should match on a structured (binary, argv) representation rather than raw-string prefix matching — raw-string matching is defeated by quoting, whitespace, environment expansion, and interpreter invocation (sh -c "...", env FOO=bar cmd). Scope during implementation: what "resolved" means (symlink/$PATH handling, whether sh -c/bash -c/env wrappers are unwrapped and their inner command checked or rejected outright).
• Add a category field to Violation (e.g. metacharacter, interpreter, destructive_command, network_utility, not_allowlisted) alongside the existing free-text rule_name/reason, so an agent loop can branch on the block programmatically instead of parsing prose. This is a general Violation improvement, not shell-specific — worth applying consistently to URLAllowList/SensitiveDataFilter too.

Follow the same nested-container walk (dict/list/tuple/set) and self-referential-container guard already used in URLAllowList/SensitiveDataFilter for consistency.

Why this matters

Closes the gap between the README's stated threat model (URL exfiltration and shell execution) and what's actually shipped (URL only). Without it, users adopting ModelFuzz specifically for command-injection risk get no bundled help and have to write correct shell-safety logic themselves — which is easy to get wrong.

Acceptance criteria

• NoDangerousShellPatterns ships as a raw-string tripwire, documented in README Limitations as not a parser/security boundary
• ShellCommandAllowList matches structured (binary, argv) commands, not raw-string prefixes
• Violation gains a machine-readable category field
• Test coverage includes: shell quoting variants, nested containers, interpreter/wrapper invocation (sh -c, bash -c, env), and fail-closed behavior on unparseable/unrecognized input

[newjsouza]

I came across this issue while evaluating runtime safety for an autonomous agent stack that exposes shell, HTTP, file, and delegation tools. The gap described here is particularly relevant: URL allowlisting and keyword-based sensitive-data filtering do not provide a clear default boundary for shell-capable tools.\n\nOne design concern worth making explicit is that should remain documented as a tripwire rather than a shell parser or security boundary. A complementary could use a structured command representation (binary plus normalized argv/prefix) where possible, instead of relying only on raw-string matching; otherwise quoting, whitespace, environment expansion, and interpreter invocation may create surprising bypasses.\n\nFor an agent integration, I would also find it useful if the policy exposed a machine-readable reason/category for blocks (for example: metacharacter, interpreter, destructive command, network utility, or command not allowlisted), so the agent loop can surface a safe tool error and recover without treating the block as an infrastructure failure.\n\nThis seems like a strong direction, especially if the tests cover shell quoting, nested containers, interpreter variants, and fail-closed behavior. I will be following the design discussion.

[newjsouza]

Correction to my previous comment: the two proposed policy names are NoDangerousShellPatterns and ShellCommandAllowList.

The point I intended to make is that NoDangerousShellPatterns should be documented as a tripwire, not as a complete shell parser or security boundary. ShellCommandAllowList could complement it with structured command matching (binary plus normalized argv/prefix) where possible, rather than relying only on raw-string matching. This would make quoting, whitespace, environment expansion, and interpreter invocation easier to reason about and test.

[higagan]

Thanks — this is exactly the kind of design pressure this needs before implementation. Folding it into the proposal:

1. NoDangerousShellPatterns is a tripwire, not a boundary. Will document it the same way the README already does for SensitiveDataFilter — a quick raw-string/metacharacter check, explicitly not a shell parser, with the same "know its limits" framing in the Limitations section.

2. ShellCommandAllowList should match structured commands, not raw strings. Agreed raw-string prefix matching breaks on quoting, whitespace, env expansion, and interpreter invocation (sh -c "...", env FOO=bar cmd, etc.). It should accept a normalized (binary, argv) shape — e.g. take shlex.split-style input or require the tool to pass structured args rather than a single string — and allowlist on the resolved binary + argv prefix. Will scope exactly what "resolved" means (symlinks? $PATH lookup? interpreter unwrapping for sh -c?) as part of the implementation, since that's where the real bypass risk lives.

3. Machine-readable block reason/category. Good catch — Violation already carries rule_name/reason as free text; this would add a category field (e.g. metacharacter, interpreter, destructive_command, network_utility, not_allowlisted) so an agent loop can branch on it programmatically instead of parsing prose. Worth doing on Violation generally, not just for shell policies, for consistency with URLAllowList/SensitiveDataFilter.

Updating the issue body's acceptance criteria to reflect this. Test coverage will need to include: shell quoting variants, nested containers, interpreter/wrapper invocation (sh -c, bash -c, env), and fail-closed behavior on unparseable input — as you flagged.

[higagan]

Implemented in #74 — all four acceptance criteria are covered.

@newjsouza your three points shaped the design; thanks for pushing on them before any code existed:

1. NoDangerousShellPatterns is documented as a tripwire, not a boundary — in the class docstring, the README body, the README Limitations section, and AGENTS.md. There's also a test that pins the weakness (a binary outside the table passes straight through) and asserts the allowlist catches what the tripwire misses, so the "not a boundary" claim can't quietly rot into a false one.

2. ShellCommandAllowList matches structured (binary, argv), as you suggested. Concretely, the things raw-string prefix matching would have let through, each now with its own test: a leading path (/bin/curl, ./curl), quoting and whitespace variants, environment assignment (FOO=bar curl … and env FOO=bar curl … both resolve to curl), chained commands, and inline interpreter scripts. On interpreter invocation I went further than "reason about it" — sh -c "…" is refused even when sh itself is allowlisted, since permitting an interpreter must not silently permit everything it can run. Unparseable input fails closed.

   Your instinct about quoting/whitespace was right in a way I didn't expect: while writing tests I found that shlex treats a newline as ordinary whitespace, so ls\ncurl evil.com flattened into one innocent-looking argv that the allowlist accepted — while a real shell would run two commands. That's now caught on the raw string before the split erases it.

3. Violation has a category field, and — the part that turned out to matter most — ModelFuzzBlockError exposes it too. Your framing was that the agent loop should "surface a safe tool error and recover without treating the block as an infrastructure failure"; the loop catches the exception, so a log field alone wouldn't have delivered that. exc.category is the stable string to branch on, exc.reason stays prose for the audit log:

   except ModelFuzzBlockError as exc:
       if exc.category == CATEGORY_CREDENTIAL:
           ...

   Categories: credential, sensitive_keyword, not_allowlisted, invalid_url, scheme_not_allowed, userinfo_trick, metacharacter, interpreter, destructive_command, network_utility, unparseable, defaulting to unspecified for custom policies. Applied across all the existing rules too, not just the shell ones.

One design call I'd welcome a second opinion on: ShellCommandAllowList reads dict values but not dict keys, unlike the other rules. Because it's default-deny, reading keys made {"cmd": "ls"} block on a command named cmd — nearly every dict argument would fail on its own field names. Values-only felt clearly right, but it is a divergence, so it's commented at the call site and tested.

Review welcome if you have the time.