Skip to content

Commit 21817ce

Browse files
authored
feat: port unsafe-command-allowlist risk flag from mcp-audit (#15)
Detects MCP command allowlists (e.g. ALLOW_COMMANDS) that name a binary with its own argument-level execution primitive, where checking argv[0] alone does not restrict execution (git -c alias, find -exec, python -c). Ported from the merged mcp-audit detection. - New data module allowlist_bypass_binaries.py (16 binaries, token-based key matching that excludes denylists and full-argv allowlists) - Wired into McpAuditDetector._identify_risks - risk_definitions entry (critical) + OWASP LLM06 mapping - Classified CONFIRMED in verdicts (a declared, re-checkable config fact) - 11 tests incl. denylist/toggle/full-argv negative cases 396 tests pass, ruff + mypy clean.
1 parent ea50c2e commit 21817ce

6 files changed

Lines changed: 250 additions & 0 deletions

File tree

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
"""
2+
Argument-injection-prone binaries for command-allowlist bypass detection.
3+
4+
Many MCP servers implement a "command allowlist" by checking only argv[0]
5+
(the binary name) against a list like ALLOW_COMMANDS=git, without inspecting
6+
the rest of argv. That's not a real restriction if the allowlisted binary
7+
itself exposes an argument or config-driven way to run arbitrary commands.
8+
9+
This list is intentionally not closed — add entries as new primitives surface.
10+
"""
11+
12+
# Binary name (as it would appear in argv[0]) -> one-line description of its
13+
# argument/config-level execution primitive.
14+
ARG_INJECTION_PRONE_BINARIES = {
15+
"git": "git -c alias.<x>=!<cmd> defines a shell alias; -c core.fsmonitor=<cmd> or "
16+
"-c diff.external=<cmd> also execute arbitrary commands",
17+
"find": "-exec/-execdir runs an arbitrary command on matched files",
18+
"python": "-c '<code>' executes arbitrary Python, including os.system()/subprocess calls",
19+
"python3": "-c '<code>' executes arbitrary Python, including os.system()/subprocess calls",
20+
"perl": "-e '<code>' executes arbitrary Perl, including system()/exec()",
21+
"awk": 'system("<cmd>") inside a BEGIN block executes an arbitrary command',
22+
"gawk": 'system("<cmd>") inside a BEGIN block executes an arbitrary command',
23+
"sed": "GNU sed's e command/flag (s/.../cmd/e or e cmd) executes an arbitrary shell command",
24+
"tar": "--checkpoint-action=exec=<cmd> or --to-command=<cmd> executes an arbitrary command",
25+
"npx": "-c '<script>' runs an arbitrary shell command via the underlying package runner",
26+
"node": "-e '<code>' executes arbitrary JavaScript, including child_process calls",
27+
"bash": "-c '<cmd>' runs an arbitrary command string; allowlisting a shell voids the allowlist",
28+
"sh": "-c '<cmd>' runs an arbitrary command string; allowlisting a shell voids the allowlist",
29+
"env": "env <cmd> execs an arbitrary binary, so the argv[0] check only ever sees env",
30+
"xargs": "xargs <cmd> execs an arbitrary command with input-supplied arguments",
31+
"ssh": "-o ProxyCommand=<cmd>, and -o PermitLocalCommand=yes -o LocalCommand=<cmd>, "
32+
"execute arbitrary commands on the local host",
33+
}
34+
35+
# Allowlist-style env var keys are recognized by splitting the key on "_" and
36+
# matching whole tokens — NOT by substring. Substring matching is what makes
37+
# DISALLOW_COMMANDS (a denylist) look like an allowlist, since "DISALLOW"
38+
# contains "ALLOW".
39+
_DENY_TOKENS = {
40+
"DISALLOW",
41+
"DISALLOWED",
42+
"DENY",
43+
"DENIED",
44+
"BLOCK",
45+
"BLOCKED",
46+
"FORBID",
47+
"FORBIDDEN",
48+
"EXCLUDE",
49+
"EXCLUDED",
50+
"DENYLIST",
51+
"BLOCKLIST",
52+
"BLACKLIST",
53+
"NO",
54+
"NOT",
55+
"NEVER",
56+
}
57+
58+
_ALLOW_TOKENS = {"ALLOW", "ALLOWED", "ALLOWLIST", "WHITELIST", "PERMIT", "PERMITTED"}
59+
60+
_SUBJECT_TOKENS = {
61+
"COMMAND",
62+
"COMMANDS",
63+
"CMD",
64+
"CMDS",
65+
"PATTERN",
66+
"PATTERNS",
67+
"BINARY",
68+
"BINARIES",
69+
"EXECUTABLE",
70+
"EXECUTABLES",
71+
}
72+
73+
# A key must END in one of these, so that a toggle like ALLOW_COMMAND_LOGGING
74+
# (which ends in LOGGING) is not mistaken for a list of allowed commands.
75+
_LIST_TAIL_TOKENS = {"ALLOWLIST", "WHITELIST", "LIST", "ALLOWED", "ALLOW"}
76+
77+
# Characters that separate entries in an allowlist value. Whitespace is
78+
# deliberately NOT a separator: an entry with arguments (e.g. "git status") is
79+
# a full-argument-vector allowlist, which is the safe pattern, not a bare
80+
# binary-name allowlist.
81+
_VALUE_SEPARATORS = [",", ";", "|", "\n"]
82+
83+
84+
def is_allowlist_env_key(key: str) -> bool:
85+
"""Check whether an env var key looks like a command-allowlist declaration."""
86+
tokens = [t for t in key.upper().replace("-", "_").split("_") if t]
87+
if not tokens:
88+
return False
89+
90+
# A deny-style key (DISALLOW_COMMANDS, COMMAND_DENYLIST, NO_ALLOW_COMMANDS)
91+
# describes the opposite of this risk.
92+
if any(t in _DENY_TOKENS for t in tokens):
93+
return False
94+
95+
if not any(t in _ALLOW_TOKENS for t in tokens):
96+
return False
97+
if not any(t in _SUBJECT_TOKENS for t in tokens):
98+
return False
99+
100+
return tokens[-1] in _SUBJECT_TOKENS or tokens[-1] in _LIST_TAIL_TOKENS
101+
102+
103+
def _split_entries(value: str) -> list[str]:
104+
"""Split an allowlist value into individual entries."""
105+
entries = [value]
106+
for sep in _VALUE_SEPARATORS:
107+
next_entries = []
108+
for e in entries:
109+
next_entries.extend(e.split(sep))
110+
entries = next_entries
111+
return [e.strip() for e in entries if e.strip()]
112+
113+
114+
def _normalize_entry(entry: str) -> str:
115+
"""
116+
Reduce an allowlist entry to the bare binary name it allows.
117+
118+
Returns "" for entries that don't name a bare binary — notably entries that
119+
carry arguments ("git status"), which are full-argument-vector allowlists
120+
and are not bypassable by argv[0] alone.
121+
"""
122+
entry = entry.strip().strip("\"'")
123+
# ALLOW_PATTERNS values are often anchored regexes (^git$).
124+
entry = entry.lstrip("^").rstrip("$").strip()
125+
if not entry or any(c.isspace() for c in entry):
126+
return ""
127+
return entry.replace("\\", "/").rsplit("/", 1)[-1].lower()
128+
129+
130+
def detect_unsafe_command_allowlist(env: dict) -> list[str]:
131+
"""
132+
Scan env vars for allowlist-style keys whose values reference a binary
133+
known to have argument-level execution primitives.
134+
135+
Returns a sorted list of matched binary names (empty if none found).
136+
"""
137+
if not env:
138+
return []
139+
140+
matched = set()
141+
for key, value in env.items():
142+
if not is_allowlist_env_key(key):
143+
continue
144+
if isinstance(value, (list, tuple)):
145+
value = ",".join(str(v) for v in value)
146+
if not isinstance(value, str):
147+
continue
148+
for entry in _split_entries(value):
149+
binary = _normalize_entry(entry)
150+
if binary in ARG_INJECTION_PRONE_BINARIES:
151+
matched.add(binary)
152+
153+
return sorted(matched)
154+
155+
156+
def get_reason(binary: str) -> str:
157+
"""Get the one-line execution-primitive description for a binary."""
158+
return ARG_INJECTION_PRONE_BINARIES.get(binary.lower(), "")

src/ai_surface/data/mcp/owasp_llm.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
"filesystem-access": ["LLM06"],
2828
"filesystem-write": ["LLM06"],
2929
"network-access": ["LLM06"],
30+
"unsafe-command-allowlist": ["LLM06"],
3031
"secrets-in-env": ["LLM02", "LLM07"],
3132
"secrets-detected": ["LLM02", "LLM07"],
3233
"admin-credentials": ["LLM02", "LLM06"],

src/ai_surface/data/mcp/risk_definitions.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,21 @@
2424
"allowlist of commands and run in a sandbox."
2525
),
2626
},
27+
"unsafe-command-allowlist": {
28+
"severity": "critical",
29+
"description": (
30+
"MCP is configured with a command allowlist (e.g. ALLOW_COMMANDS) that "
31+
"names a binary with its own argument-level execution primitive. "
32+
"Checking argv[0] alone does not restrict execution when the allowlisted "
33+
"binary can be told to run arbitrary commands (git -c alias, find -exec, "
34+
"python -c)."
35+
),
36+
"remediation": (
37+
"Do not rely on binary-name allowlists alone; allowlist the full argument "
38+
"vector, strip dangerous flags (-c, -exec, -e, --checkpoint-action), or run "
39+
"the binary in a sandbox with no ambient credentials or network access."
40+
),
41+
},
2742
"filesystem-access": {
2843
"severity": "high",
2944
"description": (

src/ai_surface/detectors/mcp_audit.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
from typing import Any
2727

2828
from ..data.mcp import owasp_llm, registry
29+
from ..data.mcp.allowlist_bypass_binaries import detect_unsafe_command_allowlist
2930
from ..data.mcp.risk_definitions import get_risk_flag_info
3031
from ..data.mcp.secret_patterns import detect_secrets
3132
from ..types import CATEGORY_MCP_SERVER as _CATEGORY
@@ -160,6 +161,12 @@ def _identify_risks(
160161
if command and (command.startswith("./") or command.startswith("/")):
161162
risks.append("local-binary")
162163

164+
# A command allowlist (e.g. ALLOW_COMMANDS) that names a binary with its own
165+
# argument-level execution primitive does not actually restrict execution:
166+
# allowlisting argv[0] alone is bypassable (git -c alias, find -exec, python -c).
167+
if detect_unsafe_command_allowlist(env):
168+
risks.append("unsafe-command-allowlist")
169+
163170
return risks
164171

165172

src/ai_surface/verdicts.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
"broad-permissions",
4444
"high-blast-radius",
4545
"excessive-agency",
46+
"unsafe-command-allowlist",
4647
}
4748

4849
#: Flags that are inference, reputation, or absence-of-signal. Named explicitly

tests/test_allowlist_bypass.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
"""Tests for unsafe-command-allowlist detection (argv[0] allowlist bypass).
2+
3+
Ported from mcp-audit. Detects command allowlists (e.g. ALLOW_COMMANDS) that
4+
name a binary with its own argument-level execution primitive, which makes the
5+
argv[0]-only allowlist bypassable (git -c alias, find -exec, python -c).
6+
"""
7+
from __future__ import annotations
8+
9+
from ai_surface.data.mcp.allowlist_bypass_binaries import (
10+
detect_unsafe_command_allowlist,
11+
is_allowlist_env_key,
12+
)
13+
14+
15+
# --- true positives ---------------------------------------------------------- #
16+
def test_bare_risky_binary_flags():
17+
assert detect_unsafe_command_allowlist({"ALLOW_COMMANDS": "git,ls"}) == ["git"]
18+
19+
20+
def test_variant_key_and_separator():
21+
assert detect_unsafe_command_allowlist({"ALLOWED_COMMANDS": "find;cat"}) == ["find"]
22+
23+
24+
def test_prefixed_key_and_path_form():
25+
assert detect_unsafe_command_allowlist(
26+
{"MCP_ALLOW_COMMANDS": "/usr/bin/python"}
27+
) == ["python"]
28+
29+
30+
def test_added_shell_and_exec_binaries():
31+
got = detect_unsafe_command_allowlist({"ALLOW_COMMANDS": "bash,sh,env,xargs,ssh"})
32+
assert got == ["bash", "env", "sh", "ssh", "xargs"]
33+
34+
35+
def test_mixed_full_command_and_bare_binary():
36+
# "git status" is a full-argv allowlist (safe); bare "find" is bypassable.
37+
assert detect_unsafe_command_allowlist({"ALLOW_COMMANDS": "git status, find"}) == [
38+
"find"
39+
]
40+
41+
42+
# --- false positives we must not raise --------------------------------------- #
43+
def test_denylist_key_not_flagged():
44+
assert detect_unsafe_command_allowlist({"DISALLOW_COMMANDS": "git"}) == []
45+
assert is_allowlist_env_key("DISALLOW_COMMANDS") is False
46+
47+
48+
def test_deny_and_block_prefixes_not_flagged():
49+
assert detect_unsafe_command_allowlist({"DENY_COMMANDS": "git"}) == []
50+
assert detect_unsafe_command_allowlist({"BLOCK_COMMANDS": "find"}) == []
51+
52+
53+
def test_unrelated_toggle_not_flagged():
54+
# ALLOW_COMMAND_LOGGING is a boolean toggle, not a command list.
55+
assert detect_unsafe_command_allowlist({"ALLOW_COMMAND_LOGGING": "true"}) == []
56+
assert is_allowlist_env_key("ALLOW_COMMAND_LOGGING") is False
57+
58+
59+
def test_full_argv_allowlist_not_flagged():
60+
assert detect_unsafe_command_allowlist({"ALLOW_COMMANDS": "git status"}) == []
61+
62+
63+
def test_clean_allowlist_not_flagged():
64+
assert detect_unsafe_command_allowlist({"ALLOW_COMMANDS": "ls,cat,echo"}) == []
65+
66+
67+
def test_empty_env():
68+
assert detect_unsafe_command_allowlist({}) == []

0 commit comments

Comments
 (0)