Skip to content

Commit 0e6983a

Browse files
fix: address 7 critical/high security findings from deep audit
Proxy (cli/proxy.py): - V11: Add target command allowlist — reject unlisted binaries - V13: Wire dead audit log to AuditLog.log() for persistence - V14: Drop non-JSON messages instead of forwarding (smuggling fix) - V15: Skip forwarding blocked tool calls to target server MCP server (integrations/mcp/__init__.py): - V12: Validate tool handler kwargs against input_schema before dispatch Policy engine (governance/policy.py): - V26: Default to deny when no policies loaded (fail-closed) - V27: Treat rule evaluation exceptions as match (fail-closed) All 1852 tests pass, 0 failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1251446 commit 0e6983a

File tree

3 files changed

+72
-11
lines changed

3 files changed

+72
-11
lines changed

packages/agent-mesh/src/agentmesh/cli/proxy.py

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import asyncio
1717
import json
18+
import os
1819
import sys
1920
from datetime import datetime
2021
from typing import Any, Dict, Optional, List
@@ -29,6 +30,14 @@
2930

3031
logger = logging.getLogger(__name__)
3132

33+
# Allowlist of binaries the proxy may spawn as MCP targets.
34+
# Extend via AGENTMESH_PROXY_ALLOWED_TARGETS env var (comma-separated).
35+
_DEFAULT_ALLOWED_TARGETS = frozenset({
36+
"npx", "node", "python", "python3", "uvx", "uv",
37+
"npx.cmd", "node.exe", "python.exe", "python3.exe",
38+
"echo", "cat", "test", # Common for testing
39+
})
40+
3241

3342
class MCPProxy:
3443
"""
@@ -58,6 +67,9 @@ def __init__(
5867
self.policy_level = policy
5968
self.enable_footer = enable_footer
6069

70+
# V11: Validate target command against allowlist
71+
self._validate_target_command(target_command)
72+
6173
# Create proxy identity
6274
logger.info("Initializing AgentMesh proxy identity...")
6375
self.identity = AgentIdentity.create(
@@ -80,6 +92,23 @@ def __init__(
8092

8193
logger.info("Proxy initialized with trust score: %d/1000", self.trust_score)
8294

95+
@staticmethod
96+
def _validate_target_command(target_command: List[str]) -> None:
97+
"""Validate target command binary against the allowlist (V11)."""
98+
if not target_command:
99+
raise ValueError("target_command must not be empty")
100+
binary = os.path.basename(target_command[0])
101+
env_extra = os.environ.get("AGENTMESH_PROXY_ALLOWED_TARGETS", "")
102+
allowed = _DEFAULT_ALLOWED_TARGETS | frozenset(
103+
t.strip() for t in env_extra.split(",") if t.strip()
104+
)
105+
if binary not in allowed:
106+
raise ValueError(
107+
f"Target binary '{binary}' is not in the allowed list: "
108+
f"{sorted(allowed)}. Set AGENTMESH_PROXY_ALLOWED_TARGETS "
109+
f"to extend the allowlist."
110+
)
111+
83112
def _load_default_policies(self):
84113
"""Load default policies based on policy level."""
85114
if self.policy_level == "strict":
@@ -195,14 +224,18 @@ async def _read_from_client(self):
195224
try:
196225
message = json.loads(line.strip())
197226
except json.JSONDecodeError:
198-
# Not a JSON message, pass through
199-
self._write_to_target(line)
227+
# V14: Drop non-JSON messages — never forward unvalidated content
228+
logger.warning("Dropping non-JSON client message (potential smuggling)")
200229
continue
201230

202231
# Intercept tool calls
203232
if message.get("method") == "tools/call":
204233
message = await self._handle_tool_call(message)
205234

235+
# V15: Don't forward blocked tool calls to target
236+
if isinstance(message, dict) and message.get("_agentmesh_blocked"):
237+
continue
238+
206239
# Forward to target
207240
self._write_to_target(json.dumps(message) + "\n")
208241

@@ -354,9 +387,9 @@ def _audit_log_tool_call(
354387
decision: Any
355388
):
356389
"""Log tool call to audit trail."""
357-
{
390+
entry = {
358391
"timestamp": datetime.utcnow().isoformat() + "Z",
359-
"agent": self.identity.did,
392+
"agent": str(self.identity.did),
360393
"action": "mcp_tool_call",
361394
"tool": tool_name,
362395
"arguments": arguments,
@@ -367,8 +400,14 @@ def _audit_log_tool_call(
367400
"trust_score": self.trust_score,
368401
}
369402

370-
# In production, would write to persistent audit log
371-
# For now, just track in memory
403+
# V13: Actually persist to audit log
404+
self.audit_log.log(
405+
event_type="mcp_tool_call",
406+
agent_did=str(self.identity.did),
407+
action=tool_name,
408+
data=entry,
409+
outcome="allowed" if decision.allowed else "denied",
410+
)
372411
logger.debug("Audit: %s - %s", tool_name, decision.action)
373412

374413
def _update_trust_score(self, tool_name: str, allowed: bool):

packages/agent-mesh/src/agentmesh/governance/policy.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,16 @@ def evaluate(self, context: dict) -> bool:
8686
# In production, would use a proper expression parser
8787
return self._eval_expression(self.condition, context)
8888
except Exception:
89-
logger.debug("Policy rule evaluation failed for '%s'", self.name, exc_info=True)
90-
return False
89+
# V27: Fail-closed — treat evaluation errors as a match so
90+
# the rule's action (typically "deny") takes effect. This
91+
# prevents attackers from crafting inputs that trigger
92+
# exceptions to bypass policy rules.
93+
logger.warning(
94+
"Policy rule evaluation error for '%s' — treating as MATCH (fail-closed)",
95+
self.name,
96+
exc_info=True,
97+
)
98+
return True
9199

92100
def _eval_expression(self, expr: str, context: dict) -> bool:
93101
"""Evaluate a simple expression."""
@@ -779,13 +787,15 @@ def evaluate(
779787
if applicable:
780788
default = applicable[0].default_action
781789
else:
782-
default = "allow" # No policies = default allow
790+
# V26: Fail-closed — no policies loaded means deny by default.
791+
# Operators must explicitly load an allow policy.
792+
default = "deny"
783793

784794
elapsed = (datetime.utcnow() - start).total_seconds() * 1000
785795
return PolicyDecision(
786796
allowed=(default == "allow"),
787797
action=default,
788-
reason="No matching rules, using default",
798+
reason="No matching rules, using default" if applicable else "No policies loaded (deny by default)",
789799
evaluated_at=start,
790800
evaluation_ms=elapsed,
791801
)

packages/agent-mesh/src/agentmesh/integrations/mcp/__init__.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,19 @@ async def invoke_tool(
268268
# Execute tool
269269
call.trust_verified = True
270270
try:
271-
result = await tool.handler(**arguments)
271+
# V12: Validate arguments against input_schema before dispatch
272+
allowed_keys = set(tool.input_schema.get("properties", {}).keys())
273+
if allowed_keys:
274+
sanitized = {k: v for k, v in arguments.items() if k in allowed_keys}
275+
stripped = set(arguments.keys()) - allowed_keys
276+
if stripped:
277+
logger.warning(
278+
"Stripped unexpected kwargs from %s call by %s: %s",
279+
tool_name, caller_did, stripped,
280+
)
281+
else:
282+
sanitized = arguments
283+
result = await tool.handler(**sanitized)
272284
call.success = True
273285
call.result = result
274286
tool.total_calls += 1

0 commit comments

Comments
 (0)