From 1529c72219482610788dd438eccf73b243aad327 Mon Sep 17 00:00:00 2001 From: ivanpaghubasan Date: Mon, 28 Sep 2026 14:59:54 +0800 Subject: [PATCH] feat(rules): add side-effect bounds rules for OpenAI, Pydantic AI, MCP, LangChain (batch 1) --- ARCHITECTURE.md | 17 ++ CLAUDE.md | 40 ++-- COVERAGE.md | 38 +++- internal/rules/policies_test.go | 200 ++++++++++++++++ .../langchain/side_effect_bounds.yaml | 110 +++++++++ .../rules-fixture/mcp/side_effect_bounds.yaml | 110 +++++++++ .../openai_sdk/side_effect_bounds.yaml | 213 ++++++++++++++++++ .../pydantic_ai/side_effect_bounds.yaml | 115 ++++++++++ 8 files changed, 819 insertions(+), 24 deletions(-) create mode 100644 testdata/rules-fixture/langchain/side_effect_bounds.yaml create mode 100644 testdata/rules-fixture/mcp/side_effect_bounds.yaml create mode 100644 testdata/rules-fixture/openai_sdk/side_effect_bounds.yaml create mode 100644 testdata/rules-fixture/pydantic_ai/side_effect_bounds.yaml diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 39d58c0a..779ef3e0 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -1146,6 +1146,8 @@ Shipped rules (one row per YAML rule entry): | OAI-019 | tool | openai_sdk | medium | `openai_sdk/idempotency.yaml` | TypeScript mutating tool has no idempotency key | | OAI-022 | tool | openai_sdk | low | `openai_sdk/tool_definition.yaml` | TypeScript tool has no description | | OAI-024 | tool | openai_sdk | medium | `openai_sdk/network.yaml` | TypeScript tool builds outbound URL from a non-literal value | +| OAI-030 | tool | openai_sdk | high | `openai_sdk/side_effect_bounds.yaml` | Side-effecting tool lets the model choose the recipient or amount with no visible bound | +| OAI-031 | tool | openai_sdk | high | `openai_sdk/side_effect_bounds.yaml` | TypeScript side-effecting tool lets the model choose the recipient or amount with no visible bound | | OAI-101 | agent | openai_sdk | high | `openai_sdk/agent_safety.yaml` | Agent has no input_guardrails AND wires shell or filesystem-touching tools | | OAI-102 | agent | openai_sdk | high | `openai_sdk/agent_safety.yaml` | Agent uses tool_use_behavior="stop_on_first_tool" | | OAI-103 | agent | openai_sdk | high | `openai_sdk/agent_safety.yaml` | tool_choice="required" combined with reset_tool_choice=False | @@ -1198,6 +1200,7 @@ Shipped rules (one row per YAML rule entry): | MCP-012 | tool | mcp | high | `mcp/shell_safety.yaml` | TypeScript MCP tool spawns a subprocess | | MCP-013 | tool | mcp | high | `mcp/ssrf.yaml` | TypeScript MCP tool fetches a caller-controlled URL (SSRF) | | MCP-014 | tool | mcp | high | `mcp/code_execution.yaml` | TypeScript MCP tool evaluates dynamic code (eval / new Function) | +| MCP-030 | tool | mcp | high | `mcp/side_effect_bounds.yaml` | Side-effecting tool lets the model choose the recipient or amount with no visible bound | | LC-001 | tool | langchain | low | `langchain/tool_definition.yaml` | LangChain tool has no description | | LC-002 | tool | langchain | medium | `langchain/tool_definition.yaml` | LangChain tool parameters are not type-annotated | | LC-003 | tool | langchain | high | `langchain/shell_safety.yaml` | LangChain tool body spawns a subprocess | @@ -1209,6 +1212,7 @@ Shipped rules (one row per YAML rule entry): | LC-012 | tool | langchain | high | `langchain/code_execution.yaml` | TypeScript LangChain tool evaluates dynamic code | | LC-013 | tool | langchain | high | `langchain/ssrf.yaml` | TypeScript LangChain tool fetches a caller-controlled URL (SSRF) | | LC-014 | tool | langchain | medium | `langchain/tool_behavior.yaml` | TypeScript LangChain tool returns output directly (`returnDirect`) | +| LC-025 | tool | langchain | high | `langchain/side_effect_bounds.yaml` | Side-effecting tool lets the model choose the recipient or amount with no visible bound | | LC-101 | agent | langchain | high | `langchain/agent_safety.yaml` | LangChain agent wires a code-execution or shell built-in tool | | LC-102 | agent | langchain | medium | `langchain/agent_safety.yaml` | LangChain AgentExecutor has no max_iterations limit | | LC-111 | agent | langchain | medium | `langchain/agent_safety.yaml` | TypeScript LangChain AgentExecutor has no maxIterations limit | @@ -1223,6 +1227,19 @@ Shipped rules (one row per YAML rule entry): > category allow-list (`internal/rules/loader.go`); `SDKMCP` already routed to > the `mcp` category via `LoadFor`, so no other wiring changed. +> **This table is a known-incomplete index, not the enumeration of shipped +> rules.** It predates several rule packs (Pydantic AI has none listed here, +> and the OpenAI/MCP/LangChain sections above are missing dozens of rules +> shipped since this table was last fully reconciled) — re-derive the true +> count with `grep -rhoE '^\s*-\s*id:\s*\S+' testdata/rules-fixture/*/*.yaml +> | wc -l` rather than counting rows here, and treat +> [`COVERAGE.md`](COVERAGE.md)'s per-SDK rule-ID lists as the more current +> reference. PYD-014 (added alongside OAI-030/031, MCP-030, and LC-025 — see +> `openai_sdk/side_effect_bounds.yaml` and siblings) is intentionally not +> added as an isolated row here, since no other Pydantic AI row exists to +> anchor it. Backfilling this table fully is tracked as separate cleanup, not +> part of any single rule change. + ### Step 4c — Origin classification ([internal/pathclass/](internal/pathclass/)) After every finding is assembled (rule findings, META, and — when opted in — diff --git a/CLAUDE.md b/CLAUDE.md index 034af87a..b13c5dfc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -350,25 +350,29 @@ When changing a rule (add / remove / edit severity, confidence, match, text): 6. Commit and push the rules repo **and** the rulebook (the user pushes engine commits manually; confirm before pushing any of the three). -> **Rulebook status (2026-09-09):** the fixture and production both carry -> **210** rules across ten SDK categories (`autogen`, `claude_sdk`, +> **Rulebook status (2026-09-28):** the fixture and production both carry +> **282** rules across ten SDK categories (`autogen`, `claude_sdk`, > `claude_skill`, `crewai`, `google_adk`, `langchain`, `mcp`, `openai_sdk`, -> `pydantic_ai`, `vercel_ai`) — in sync as of **OAI-116** (OpenAI Agents SDK, -> TypeScript: `hostedMcpTool({...})` with no `allowedTools` allow-list, the TS -> sibling of OAI-115), which closes the OpenAI Agents SDK half of Class 1 in -> `docs/decisions/tool-allowlist-scope.md`. The count grew by one rule since -> the 209 figure implied by the prior note (which itself undercounted — -> re-derive, don't trust a cached figure) from `grep -rhoE -> '^\s*-\s*id:\s*\S+' testdata/rules-fixture/*/*.yaml | wc -l`. **Known gap in -> this note's own claim:** the prior version of this note said -> `check_rulebook.py` reports 0 warnings against the pack; running it now -> shows that has drifted — it currently reports 32 pre-existing *errors* -> (mostly severity/confidence drift between shipped rules and their rationale -> docs, e.g. `CSDK-103`, `CSDK-204/205`, `LC-101`, plus several -> documented-but-removed rule IDs) across docs unrelated to OAI-116, none -> introduced by this update (confirmed via a clean-tree run before this -> change: 33 errors, one of which — OAI-116 undocumented — this update -> closes). Fixing those 32 is a separate, not-yet-scoped cleanup. +> `pydantic_ai`, `vercel_ai`) — in sync as of Batch 1 of the side-effect-bounds +> rule (competitive-analysis backlog rank 12, P0): **OAI-030/031**, **PYD-014**, +> **MCP-030**, **LC-025** — a side-effecting tool (send/notify/refund/charge/ +> pay/payout/transfer/issue) with a free-form recipient or amount parameter, +> no visible bound in the body, and no SDK-native approval gate. Batch 2 +> (crewai, google_adk, autogen, vercel_ai, claude_sdk TS) is a tracked +> follow-up, not yet done. The count is up from **277**, the figure the prior +> version of this note recorded (itself already corrected once from a stale +> 210/209 — re-derive, don't trust a cached figure, from `grep -rhoE +> '^\s*-\s*id:\s*\S+' testdata/rules-fixture/*/*.yaml | wc -l`). +> **`check_rulebook.py` gate status:** running +> `python tools/check_rulebook.py --rules-repo ../trustabl-rules` from the +> rulebook repo reports **99 pre-existing errors**, none introduced by this +> batch (confirmed both before writing the four new rationale docs and after — +> the count did not move; the 5 new rules pass COVERAGE/CONSISTENCY/PLACEMENT +> cleanly). This is already worse than the 32 the prior version of this note +> recorded on 2026-09-09, meaning the pre-existing drift grew in the interim +> from unrelated changes. Fixing it remains a separate, not-yet-scoped +> cleanup — re-run the gate rather than trusting this number, since it has +> already drifted twice. The rule-authoring contract (required fields, ID conventions, per-scope `applies_to` values, framing discipline) lives in diff --git a/COVERAGE.md b/COVERAGE.md index 8d7f5c6b..838cbe95 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -4,7 +4,33 @@ Coverage matrix for Trustabl's static analysis: which agent SDKs (and which languages) we currently scan, analyse, and detect against. This file is the at-a-glance reference; `ARCHITECTURE.md` has the implementation detail. -_Last reviewed: 2026-09-21 (pending commit) — outreach-feedback investigation +_Last reviewed: 2026-09-28 (pending commit; HEAD 1f90791) — Batch 1 of the +side-effect-bounds rule (competitive-analysis backlog rank 12, P0): new rules +**OAI-030** (Python), **OAI-031** (TypeScript), **PYD-014**, **MCP-030**, and +**LC-025** — each flags a send/notify or refund/charge/pay/payout/transfer/ +issue tool whose recipient or amount parameter is free-form model input with +no visible bound (no schema constraint, no `MAX_`/`ALLOWED`/allow-list +marker in the body) and no SDK-native approval gate +(`needs_approval`/`needsApproval` for OAI, `requires_approval` for PYD, +`interrupt(` for LC, `ctx.elicit(` for MCP). Distinct from the idempotency +family (OAI-009/019, PYD-007, MCP-007, LC-017): a duplicate call repeats an +action within a bound the caller already accepted, this rule is about a +single call moving further than the caller intended. Severity high, +confidence 0.5 — the pack's floor, tied with OAI-019 — deliberately below the +idempotency family's 0.55/0.5 (see the confidence-gap section in +`openai_sdk/side_effect_bounds.md` in the rulebook for the full defense). +Rules-only; no schema/predicate/`schema_version` change (existing predicates +`name_has_prefix`, `param_name_matches`, `has_body_text`, +`tool_decorator_kwarg_present`/`value` cover it). CSDK Python is a +deliberate, documented gap for this rule: the idiomatic `@tool("name", +"desc", {"to": str})` + `async def fn(args)` shape gives discovery +`param_names=["args"]`, so a static parameter-name match can never see `to` +or `amount` — CSDK-006 (idempotency) has the identical blind spot already. +Batch 2 (crewai, google_adk, autogen, vercel_ai, claude_sdk TS) is tracked as +a follow-up, not done in this review. Rule count: **282** (was 277 as of the +prior figure below; re-derive with `grep -rhoE '^\s*-\s*id:\s*\S+' +testdata/rules-fixture/*/*.yaml | wc -l`, don't trust a cached figure). +Prior review 2026-09-21 (pending commit) — outreach-feedback investigation (Aug–Sep 2026 batch) fixed two confirmed rule bugs. **CSKILL-080/081** (claude_skill/skill_quality_text.yaml) moved from raw-substring `skill_*_has_text` matching onto a new sentence-scoped, word-boundary @@ -60,9 +86,9 @@ Legend: ✅ full · ◐ partial · ❌ none · — N/A |---|---|---|---|---| | **Claude Agent SDK** | Python | ✅ dep-scan + file inventory + `.claude/` & `.claude-plugin/` components | ✅ tools, agents, subagents (canonical + flat-collection shape fallback), skills (`SKILL.md`), slash commands, plugin manifests, settings, `ClaudeAgentOptions` session config | ✅ tool CSDK-001..009, 107, 108 (008 = `**kwargs` without input_schema, 009 = SSRF); agent CSDK-101..105; subagent CSDK-110, 111 (fire on pure-markdown collections); repo CSDK-201..205 (`defaultMode` / `permission_mode` bypass; 203 = SDK code without an agent-guidance doc: AGENTS.md/CLAUDE.md; 204 = no explicit `max_turns` on `ClaudeAgentOptions`; 205 = `acceptEdits` with no `disallowed_tools` deny-list); skill CSKILL-001 (unrestricted shell), 002/003 (dynamic-context exec + egress/secret), 010/011 (bundled-script egress + credential-read), 030 (hardcoded secret in bundled file), 020 (external-URL fetch), 040 (injection markers incl. hidden-Unicode), 050 (model-invocable + side-effecting), 060 (description claims read-only but grants side-effecting tools) | | **Claude Agent SDK** | TypeScript | ✅ dep-scan (`@anthropic-ai/claude-agent-sdk`) + file inventory + `.claude/` components | ✅ tools (`tool()` factory), agents (main thread `QueryMainAgent` per `query()` call + sub-agents inline-in-query + typed-const `AgentDefinition`), MCP servers (createSdkMcpServer + 4 config literals) | ◐ tool CSDK-010 (shell), 011 (eval/new Function), 012 (fs-write), 013 (SSRF / dynamic URL), 014 (no description), 016 (mutating tool no idempotency key); agent CSDK-120 (permissionMode bypass), 121..124 (`AgentDefinition` grants Bash / WebSearch / write built-ins / WebFetch), 130 (`query()` main agent grants Bash), 131 (`query()` main agent grants write/fetch built-ins); META-004 no longer fires | -| **OpenAI Agents SDK** | Python | ✅ dep-scan + file inventory | ✅ tools, hosted tools (11 classes), agents, MCP servers (3 transports + alias, kwargs captured incl. `tool_filter`), guardrails, sessions, `Runner.run`/`run_sync`/`run_streamed` call sites (`max_turns`) | ✅ tool OAI-001..015, 018 (018 = SSRF / caller-controlled URL); agent OAI-101..104, 106, 109, 110, 111, 112 (`Runner.run` family with no `max_turns`), 115 (`HostedMCPTool` with no `allowed_tools`), 118 (MCP server with no `tool_filter`); repo OAI-201, 202 (202 = SDK code without an agent-guidance doc: AGENTS.md/CLAUDE.md) | -| **OpenAI Agents SDK** | TypeScript | ✅ dep-scan (`@openai/agents` substring catches `-core` / `-openai`) + file inventory | ✅ tools (`tool({...})` factory), agents (`new Agent({...})` + `Agent.create(...)`), hosted tools (9 factories across `@openai/agents-core` and `@openai/agents-openai`), MCP servers (3 transports + `MCPServers` wrapper, kwargs captured incl. `toolFilter`), guardrails (4 `defineX` factories), sessions (`MemorySession` / `OpenAIConversationsSession` / `OpenAIResponsesCompactionSession`) | ✅ tool OAI-016 (fetch without AbortSignal timeout), OAI-017 (eval / new Function), OAI-019 (mutating tool without idempotency), OAI-022 (no description), OAI-024 (dynamic URL / SSRF); agent OAI-105 (content hosted-tool without inputGuardrails), OAI-116 (`hostedMcpTool` with no `allowedTools` allow-list), OAI-117 (`hostedMcpTool` with no `requireApproval`), OAI-119 (MCP server with no `toolFilter`); all with fire/silent cases in the per-rule harness | -| **MCP** | Python | ✅ tool registrations + config files | ◐ tool registrations only (no server-side resource/prompt discovery) | ✅ dedicated `mcp/` pack: tool MCP-001..010 (001 no description, 002 untyped params, 003 ambiguous name, 004 network timeout, 005 path safety, 006 error contract, 007 idempotency, 008 SSRF, 009 code-exec, 010 shell). `mcp_tool` coverage now lives ONLY in this pack — stripped from the CSDK rules' `applies_to` so a pure-MCP repo is covered and a mixed Claude+MCP repo does not double-fire | +| **OpenAI Agents SDK** | Python | ✅ dep-scan + file inventory | ✅ tools, hosted tools (11 classes), agents, MCP servers (3 transports + alias, kwargs captured incl. `tool_filter`), guardrails, sessions, `Runner.run`/`run_sync`/`run_streamed` call sites (`max_turns`) | ✅ tool OAI-001..015, 018 (018 = SSRF / caller-controlled URL), 030 (side-effecting tool with a free-form recipient/amount and no visible bound); agent OAI-101..104, 106, 109, 110, 111, 112 (`Runner.run` family with no `max_turns`), 115 (`HostedMCPTool` with no `allowed_tools`), 118 (MCP server with no `tool_filter`); repo OAI-201, 202 (202 = SDK code without an agent-guidance doc: AGENTS.md/CLAUDE.md) | +| **OpenAI Agents SDK** | TypeScript | ✅ dep-scan (`@openai/agents` substring catches `-core` / `-openai`) + file inventory | ✅ tools (`tool({...})` factory), agents (`new Agent({...})` + `Agent.create(...)`), hosted tools (9 factories across `@openai/agents-core` and `@openai/agents-openai`), MCP servers (3 transports + `MCPServers` wrapper, kwargs captured incl. `toolFilter`), guardrails (4 `defineX` factories), sessions (`MemorySession` / `OpenAIConversationsSession` / `OpenAIResponsesCompactionSession`) | ✅ tool OAI-016 (fetch without AbortSignal timeout), OAI-017 (eval / new Function), OAI-019 (mutating tool without idempotency), OAI-022 (no description), OAI-024 (dynamic URL / SSRF), OAI-031 (side-effecting tool with a free-form recipient/amount and no visible bound — the TS sibling of the Python row's OAI-030); agent OAI-105 (content hosted-tool without inputGuardrails), OAI-116 (`hostedMcpTool` with no `allowedTools` allow-list), OAI-117 (`hostedMcpTool` with no `requireApproval`), OAI-119 (MCP server with no `toolFilter`); all with fire/silent cases in the per-rule harness | +| **MCP** | Python | ✅ tool registrations + config files | ◐ tool registrations only (no server-side resource/prompt discovery) | ✅ dedicated `mcp/` pack: tool MCP-001..010 (001 no description, 002 untyped params, 003 ambiguous name, 004 network timeout, 005 path safety, 006 error contract, 007 idempotency, 008 SSRF, 009 code-exec, 010 shell), MCP-030 (side-effecting tool with a free-form recipient/amount and no visible bound). `mcp_tool` coverage now lives ONLY in this pack — stripped from the CSDK rules' `applies_to` so a pure-MCP repo is covered and a mixed Claude+MCP repo does not double-fire | | **MCP** | TypeScript | ✅ dep-scan (`@modelcontextprotocol/sdk`) + file inventory | ✅ server authoring: `new McpServer(...)` receiver tracked, `registerTool` / legacy `tool` registrations → `KindMCPTool` (reuses `tsZodParamNames` / `tsHandlerFacts`). Distinct from the Claude client-config `createSdkMcpServer` discovery. Low-level `Server` + `setRequestHandler` not extracted (gap) | ✅ tool MCP-011..014 (011 no description, 012 shell, 013 SSRF, 014 eval / new Function); shared `mcp/` pack, `language: typescript` | | **MCP** | Go | ✅ dep-scan (`go.mod`: mark3labs/mcp-go, official go-sdk, metoro-io/mcp-golang) + file inventory | ◐ tools (`mcp.NewTool("n", mcp.WithDescription(...), mcp.WithString(...))` — mark3labs, full name/desc/params; `mcp.AddTool(server, &mcp.Tool{Name, Description}, fn)` — official go-sdk, name/desc). metoro `RegisterTool`, the official handler-struct param schema, and the `s.AddTool` registration edge are v1 gaps | ◐ field-based language:go: MCP-015 (no description), MCP-016 (ambiguous name). Body-fact rules (shell/SSRF/timeout) need Go AST predicate branches (fast-follow); untyped-params is N/A (Go is statically typed) | | **MCP** | C#/.NET | ✅ dep-scan (`ModelContextProtocol` in `Directory.Packages.props` / `packages.config`; variable-named `.csproj` is a best-effort gap) + file inventory | ◐ tools (`[McpServerTool]`-attributed methods, official ModelContextProtocol SDK; name = method name, description from a co-located `[Description(...)]`, typed params). `[McpServerTool(Name=...)]` override, Semantic Kernel `[KernelFunction]`, and AutoGen `[Function]` are gaps | ◐ field-based language:csharp: MCP-017 (no description), MCP-018 (ambiguous name). Body-fact rules need C# AST predicate branches (fast-follow); untyped-params N/A (C# is statically typed) | @@ -71,12 +97,12 @@ Legend: ✅ full · ◐ partial · ❌ none · — N/A | **Google ADK** | Python | ✅ dep-scan (`google-adk`) + file inventory | ✅ LlmAgent (+ Agent alias), SequentialAgent, ParallelAgent, LoopAgent, LanggraphAgent; FunctionTool wrapping; 13 built-in hosted tools; sub_agents edges | ✅ tool ADK-001..007, 009..012 (009 = print to stdout, 010 = subprocess, 011 = eval/exec/compile, 012 = SSRF); agent ADK-008, 101..108, 110..111; repo ADK-201 (SDK code without an agent-guidance doc: AGENTS.md/CLAUDE.md) | | **Google ADK** | TypeScript | ✅ dep-scan (`@google/adk`) + file inventory | ✅ tools (`new FunctionTool({...})`), agents (5 constructors: LlmAgent + SequentialAgent + ParallelAgent + LoopAgent + RoutedAgent), hosted tools (13 classes), subAgents edges | ◐ tool ADK-013 (no description), 015 (eval / new Function), 016 (SSRF / dynamic URL); agent ADK-109 (LlmAgent no description); first ADK TS pack; META-004 no longer fires | | **Google ADK** | Go / Java / Kotlin | ❌ | ❌ | ❌ | -| **LangChain / LangGraph** | Python | ✅ dep-scan (`langchain` / `langgraph` needles, all manifests) + file inventory | ✅ tools (`@tool` decorator — import-gated to disambiguate from the Claude SDK's `@tool`; `StructuredTool` / `Tool` factories + `.from_function`), agents (`create_react_agent`, `create_agent`, `AgentExecutor` + `AgentExecutor.from_agent_and_tools`; positional `tools` captured), raw `StateGraph` graphs (`StateGraph(...)` → `AgentDef` Class `StateGraph`, import-bound; `compile()` kwargs captured; `ToolNode` / `bind_tools` tools resolved), dangerous built-ins (`PythonREPLTool` / `PythonAstREPLTool` / `ShellTool` / `Requests*` → `HostedToolDef` edges) | ✅ tool LC-001 (no description), LC-002 (untyped params), LC-003 (shell), LC-004 (code-exec), LC-005 (SSRF), LC-006 (`return_direct`); agent LC-101 (code-exec/shell built-in — incl. raw `StateGraph`), LC-102 (AgentExecutor no `max_iterations`); repo LC-201 (no agent-guidance doc) | +| **LangChain / LangGraph** | Python | ✅ dep-scan (`langchain` / `langgraph` needles, all manifests) + file inventory | ✅ tools (`@tool` decorator — import-gated to disambiguate from the Claude SDK's `@tool`; `StructuredTool` / `Tool` factories + `.from_function`), agents (`create_react_agent`, `create_agent`, `AgentExecutor` + `AgentExecutor.from_agent_and_tools`; positional `tools` captured), raw `StateGraph` graphs (`StateGraph(...)` → `AgentDef` Class `StateGraph`, import-bound; `compile()` kwargs captured; `ToolNode` / `bind_tools` tools resolved), dangerous built-ins (`PythonREPLTool` / `PythonAstREPLTool` / `ShellTool` / `Requests*` → `HostedToolDef` edges) | ✅ tool LC-001 (no description), LC-002 (untyped params), LC-003 (shell), LC-004 (code-exec), LC-005 (SSRF), LC-006 (`return_direct`), LC-025 (side-effecting tool with a free-form recipient/amount and no visible bound); agent LC-101 (code-exec/shell built-in — incl. raw `StateGraph`), LC-102 (AgentExecutor no `max_iterations`); repo LC-201 (no agent-guidance doc) | | **LangChain / LangGraph** | TypeScript | ✅ dep-scan (`@langchain/*` / `langchain` / `langgraph`) + file inventory | ✅ tools (`tool(fn, {...})` factory — import-gated, config from arg 1; `DynamicStructuredTool` / `DynamicTool`), agents (`createReactAgent`, `createAgent`, `new AgentExecutor`) | ◐ tool LC-010 (no description), LC-011 (shell), LC-012 (code-exec), LC-013 (SSRF), LC-014 (`returnDirect`); agent LC-111 (AgentExecutor no `maxIterations`). Provider hosted tools (`shell()` / `bash_*` / `applyPatch`) and the raw `StateGraph` graph agent are documented gaps | | **CrewAI** | Python | ✅ dep-scan (`crewai` / `crewai-tools`) + file inventory | ✅ agents (`Agent(...)`, import-gated to `crewai` so it doesn't collide with OpenAI/ADK `Agent`), tools (`@tool` decorator routed in `kindFromDecorators` by import binding; `Tool(fn)` factory), dangerous built-ins (`CodeInterpreterTool` / `FileReadTool` / scrape+RAG tools → `HostedToolDef`) | ✅ tool CREW-001..006 (no-desc, untyped, code-exec, shell, SSRF, idempotency), CREW-108 (`result_as_answer`); agent CREW-101..104 (`allow_code_execution`, `code_execution_mode=unsafe`, wired `CodeInterpreterTool`, `allow_delegation`), CREW-106 (unconstrained `FileReadTool`), CREW-107 (URL-fetching tools), CREW-110 (no explicit `max_iter`); repo CREW-201. `class X(BaseTool)` and `Crew(...)` orchestration are v1 gaps | | **AutoGen / AG2** | Python | ✅ dep-scan (`pyautogen` / `ag2` / `autogen-agentchat`) + file inventory | ✅ two import gates (AG2/0.2 `autogen`; Microsoft v0.4 `autogen_agentchat`/`_core`/`_ext`): agents `ConversableAgent` / `UserProxyAgent` / `AssistantAgent` / `GroupChat` / `GroupChatManager` / `CodeExecutorAgent`; tools `register_function(fn,...)` + stacked `@x.register_for_llm` / `@x.register_for_execution` attribute decorators; nested `code_execution_config` dict captured | ✅ agent AG2-001 (`use_docker=False`), 002 (`human_input_mode=NEVER` + code exec), 004 (`GroupChat` no `max_round`), 005 (`AssistantAgent` code exec), 006 (no `max_consecutive_auto_reply`); tool AG2-007..012 (no-desc, untyped, shell, code-exec, SSRF, timeout); repo AG2-201. AG2-003 (v0.4 executor-class), the `register_function` caller/executor edge, and AG2 `@tool` are v1 gaps | | **Vercel AI SDK** | TypeScript | ✅ dep-scan (quoted `"ai"` key + `@ai-sdk/`) + file inventory | ✅ tools (`tool({...})` / `dynamicTool({...})` single-object factory, import-gated to `ai`), agents (call-based `generateText` / `streamText` / `generateObject` / `streamObject` carrying a `tools` record + class `ToolLoopAgent` / `Experimental_Agent`; **`tools` is an object/record**, walked by property value, not an array), provider hosted tools (`.tools.*()` → `HostedToolDef`) | ◐ tool VAI-001..005 (shell, code-exec, SSRF, no-desc, untyped), VAI-011 (HTTP call without timeout); agent VAI-006 (provider shell/computer/code-exec tool), VAI-007 (no loop bound), VAI-008 (`toolChoice:'required'` + dangerous tool); repo VAI-012. VAI-009/010 (name rules — Vercel tools carry no `Name`) are gaps; `.js`/`.mjs`/`.cjs` apps are now AST-parsed via the shared TS-family pipeline (ES `import` and CommonJS `require()`) | -| **Pydantic AI** | Python | ✅ dep-scan (`pydantic-ai` / `pydantic-ai-slim`) + file inventory | ✅ agents (`Agent(...)` → Class `PydanticAgent`, import-gated to `pydantic_ai`), tools (`@agent.tool` / `@agent.tool_plain` attribute decorators — routed in `kindFromDecorators`, disambiguated from the Claude SDK's `@agent.tool` by import; `Tool(fn)` factory), native tools (`capabilities=[NativeTool(CodeExecutionTool())]` / `builtin_tools=[...]` unwrapped → `HostedToolDef`, inner kwargs such as `force_download` captured), FileUrl-family `force_download` stamped on the agent, `agent.run`/`run_sync`/`run_stream` call sites (`usage_limits`) | ✅ tool PYD-001..007 (no-desc, untyped, shell, code-exec, SSRF, timeout, idempotency); agent PYD-101 (no `output_type` validation), 102 (`CodeExecutionTool`), 103 (`WebFetchTool` / `UrlContextTool` / `WebSearchTool`), 104 (`force_download=True` / `"allow-local"` on FileUrl-family or `WebFetchTool`), 105 (`end_strategy='exhaustive'`), 106 (no explicit `usage_limits`); repo PYD-201. The bare-`tools=[fn]` ToolDef shape and `RunContext` param-strip for PYD-002 remain gaps | +| **Pydantic AI** | Python | ✅ dep-scan (`pydantic-ai` / `pydantic-ai-slim`) + file inventory | ✅ agents (`Agent(...)` → Class `PydanticAgent`, import-gated to `pydantic_ai`), tools (`@agent.tool` / `@agent.tool_plain` attribute decorators — routed in `kindFromDecorators`, disambiguated from the Claude SDK's `@agent.tool` by import; `Tool(fn)` factory), native tools (`capabilities=[NativeTool(CodeExecutionTool())]` / `builtin_tools=[...]` unwrapped → `HostedToolDef`, inner kwargs such as `force_download` captured), FileUrl-family `force_download` stamped on the agent, `agent.run`/`run_sync`/`run_stream` call sites (`usage_limits`) | ✅ tool PYD-001..007 (no-desc, untyped, shell, code-exec, SSRF, timeout, idempotency), PYD-014 (side-effecting tool with a free-form recipient/amount and no visible bound); agent PYD-101 (no `output_type` validation), 102 (`CodeExecutionTool`), 103 (`WebFetchTool` / `UrlContextTool` / `WebSearchTool`), 104 (`force_download=True` / `"allow-local"` on FileUrl-family or `WebFetchTool`), 105 (`end_strategy='exhaustive'`), 106 (no explicit `usage_limits`); repo PYD-201. The bare-`tools=[fn]` ToolDef shape and `RunContext` param-strip for PYD-002 remain gaps | | **NVIDIA NeMo Agent Toolkit** | Python | ◐ dep-scan (`nvidia-nat`, catches bracketed extras) + file inventory | ❌ | ❌ | | **OpenShell** | Python | ✅ shell-invocation discovery + `openshell/*.yaml` policy files surfaced | ✅ `KindShellInvocation` tools → `RepoInventory.HasShellInvocations` (the "openshell" risk surface; not an SDK, never in `SDKsDetected`) | ❌ rules moved to closed-source companion project (no rule fires; no META finding — openshell is not treated as an unaudited SDK) | diff --git a/internal/rules/policies_test.go b/internal/rules/policies_test.go index 6a0f789f..5154aeb3 100644 --- a/internal/rules/policies_test.go +++ b/internal/rules/policies_test.go @@ -726,6 +726,40 @@ def create_order(customer_id: str, amount: float, idempotency_key: str) -> dict: return {"ok": True} `, wantFires: false}, + // ─── PYD-014 side-effect bounds (recipient / amount, no visible cap) ──── + {name: "PYD-014 fires on send tool with free-form recipient", ruleID: "PYD-014", kind: models.KindPydanticAITool, src: ` +def send_email(to: str, body: str) -> str: + """Send an email.""" + return "ok" +`, wantFires: true}, + {name: "PYD-014 fires on charge tool with free-form amount", ruleID: "PYD-014", kind: models.KindPydanticAITool, src: ` +def charge_card(customer_id: str, amount: float) -> str: + """Charge a card.""" + return "ok" +`, wantFires: true}, + {name: "PYD-014 silent when amount has a conint bound", ruleID: "PYD-014", kind: models.KindPydanticAITool, src: ` +def charge_card(customer_id: str, amount: conint(le=50000)) -> str: + """Charge a card.""" + return "ok" +`, wantFires: false}, + {name: "PYD-014 silent when the name matches but the param does not", ruleID: "PYD-014", kind: models.KindPydanticAITool, src: ` +def charge_customer(customer_id: str) -> str: + """Charge, amount decided server-side.""" + return "ok" +`, wantFires: false}, + {name: "PYD-014 silent when requires_approval=True gates the call", ruleID: "PYD-014", kind: models.KindPydanticAITool, toolConfig: map[string]string{"requires_approval": "True"}, src: ` +def send_email(to: str, body: str) -> str: + """Send an email.""" + return "ok" +`, wantFires: false}, + // Explicit requires_approval=False is exactly as un-gated as omitting the + // kwarg. + {name: "PYD-014 fires when requires_approval=False explicitly", ruleID: "PYD-014", kind: models.KindPydanticAITool, toolConfig: map[string]string{"requires_approval": "False"}, src: ` +def send_email(to: str, body: str) -> str: + """Send an email.""" + return "ok" +`, wantFires: true}, + // ─── CSDK-001 missing docstring ───────────────────────────────────────── {name: "CSDK-001 fires on missing docstring", ruleID: "CSDK-001", kind: models.KindClaudeSDKTool, src: ` def fetch_data(x: str) -> dict: @@ -1103,6 +1137,68 @@ def create_order(customer_id: str, amount: float, idempotency_key: str) -> dict: `, toolConfig: nil, wantFires: false}, + // ─── OAI-030 side-effect bounds (recipient / amount, no visible cap) ──── + {name: "OAI-030 fires on send tool with free-form recipient", ruleID: "OAI-030", kind: models.KindOpenAITool, src: ` +def send_email(to: str, subject: str, body: str) -> str: + """Send an email.""" + return "ok" +`, + toolConfig: nil, wantFires: true}, + {name: "OAI-030 fires on refund tool with free-form amount", ruleID: "OAI-030", kind: models.KindOpenAITool, src: ` +def refund_payment(charge_id: str, amount: int) -> str: + """Refund a payment.""" + return "ok" +`, + toolConfig: nil, wantFires: true}, + {name: "OAI-030 silent when amount has a Field(le=) bound", ruleID: "OAI-030", kind: models.KindOpenAITool, src: ` +def refund_payment(charge_id: str, amount: int = Field(le=50000)) -> str: + """Refund a payment.""" + return "ok" +`, + toolConfig: nil, wantFires: false}, + {name: "OAI-030 silent when body enforces a MAX_ constant", ruleID: "OAI-030", kind: models.KindOpenAITool, src: ` +def refund_payment(charge_id: str, amount: int) -> str: + """Refund a payment.""" + if amount > MAX_REFUND_CENTS: + raise ValueError("too much") + return "ok" +`, + toolConfig: nil, wantFires: false}, + {name: "OAI-030 silent when amount is not a model-supplied param", ruleID: "OAI-030", kind: models.KindOpenAITool, src: ` +def charge_customer(customer_id: str) -> str: + """Charge, amount decided server-side.""" + return "ok" +`, + toolConfig: nil, wantFires: false}, + {name: "OAI-030 silent on a non-mutating tool name", ruleID: "OAI-030", kind: models.KindOpenAITool, src: ` +def get_user_data(user_id: str) -> dict: + """Get user data.""" + return {} +`, + toolConfig: nil, wantFires: false}, + {name: "OAI-030 silent with an allow-list check in the body", ruleID: "OAI-030", kind: models.KindOpenAITool, src: ` +def notify_channel(recipient: str, message: str) -> str: + """Notify a channel.""" + if recipient not in ALLOWED_CHANNELS: + raise ValueError("bad recipient") + return "ok" +`, + toolConfig: nil, wantFires: false}, + {name: "OAI-030 silent when needs_approval=True gates the call", ruleID: "OAI-030", kind: models.KindOpenAITool, src: ` +def send_email(to: str, subject: str, body: str) -> str: + """Send an email.""" + return "ok" +`, + toolConfig: map[string]string{"needs_approval": "True"}, wantFires: false}, + // Explicit needs_approval=False is exactly as un-gated as omitting the + // kwarg, mirroring OAI-014's treatment of the same kwarg. + {name: "OAI-030 fires when needs_approval=False explicitly", ruleID: "OAI-030", kind: models.KindOpenAITool, src: ` +def send_email(to: str, subject: str, body: str) -> str: + """Send an email.""" + return "ok" +`, + toolConfig: map[string]string{"needs_approval": "False"}, wantFires: true}, + // ─── OAI-010 print to stdout ───────────────────────────────────────────── {name: "OAI-010 fires on print()", ruleID: "OAI-010", kind: models.KindOpenAITool, src: ` def fetch(x: str) -> dict: @@ -1356,6 +1452,41 @@ def create_order(customer_id: str, amount: float, idempotency_key: str) -> dict: `, toolConfig: nil, wantFires: false}, + // ─── MCP-030 side-effect bounds (recipient / amount, no visible cap) ──── + {name: "MCP-030 fires on notify tool with free-form recipient", ruleID: "MCP-030", kind: models.KindMCPTool, src: ` +def notify_user(recipient: str, message: str) -> str: + """Notify a user.""" + return "ok" +`, + toolConfig: nil, wantFires: true}, + {name: "MCP-030 fires on refund tool with free-form amount", ruleID: "MCP-030", kind: models.KindMCPTool, src: ` +def refund_payment(charge_id: str, amount: int) -> str: + """Refund a payment.""" + return "ok" +`, + toolConfig: nil, wantFires: true}, + {name: "MCP-030 silent when amount has an allow-list bound", ruleID: "MCP-030", kind: models.KindMCPTool, src: ` +def refund_payment(charge_id: str, amount: int) -> str: + """Refund a payment.""" + if amount > MAX_REFUND_CENTS: + raise ValueError("too much") + return "ok" +`, + toolConfig: nil, wantFires: false}, + {name: "MCP-030 silent when the handler elicits confirmation", ruleID: "MCP-030", kind: models.KindMCPTool, src: ` +def refund_payment(charge_id: str, amount: int) -> str: + """Refund a payment.""" + ctx.elicit("confirm this refund?") + return "ok" +`, + toolConfig: nil, wantFires: false}, + {name: "MCP-030 silent when the name matches but the param does not", ruleID: "MCP-030", kind: models.KindMCPTool, src: ` +def charge_customer(customer_id: str) -> str: + """Charge, amount decided server-side.""" + return "ok" +`, + toolConfig: nil, wantFires: false}, + {name: "MCP-008 fires on dynamic URL", ruleID: "MCP-008", kind: models.KindMCPTool, src: ` import requests def fetch(path: str) -> dict: @@ -1864,6 +1995,45 @@ def calc(expr: str) -> int: "} });\n", }, + // ── OAI-031: TS side-effect bounds (recipient / amount, no visible cap) ── + { + name: "OAI-031 fires on send tool with free-form recipient", ruleID: "OAI-031", + kind: models.KindOpenAITool, lang: models.LanguageTypeScript, wantFires: true, + src: "import { tool } from \"@openai/agents\";\nimport { z } from \"zod\";\n" + + "export const t = tool({ name: \"send_email\", description: \"Send\", parameters: z.object({ to: z.string(), body: z.string() }), execute: async ({ to, body }) => \"ok\" });\n", + }, + { + name: "OAI-031 fires on charge tool with free-form amount", ruleID: "OAI-031", + kind: models.KindOpenAITool, lang: models.LanguageTypeScript, wantFires: true, + src: "import { tool } from \"@openai/agents\";\nimport { z } from \"zod\";\n" + + "export const t = tool({ name: \"charge_customer\", description: \"Charge\", parameters: z.object({ amount: z.number() }), execute: async ({ amount }) => \"ok\" });\n", + }, + { + name: "OAI-031 silent when the amount has a zod .max() bound", ruleID: "OAI-031", + kind: models.KindOpenAITool, lang: models.LanguageTypeScript, wantFires: false, + src: "import { tool } from \"@openai/agents\";\nimport { z } from \"zod\";\n" + + "export const t = tool({ name: \"charge_customer\", description: \"Charge\", parameters: z.object({ amount: z.number().max(500) }), execute: async ({ amount }) => \"ok\" });\n", + }, + { + name: "OAI-031 silent when the name matches but the param does not (payload, not a money verb)", ruleID: "OAI-031", + kind: models.KindOpenAITool, lang: models.LanguageTypeScript, wantFires: false, + src: "import { tool } from \"@openai/agents\";\nimport { z } from \"zod\";\n" + + "export const t = tool({ name: \"payload_transform\", description: \"x\", parameters: z.object({ amount: z.number() }), execute: async ({ amount }) => \"ok\" });\n", + }, + { + name: "OAI-031 silent when needsApproval: true gates the call", ruleID: "OAI-031", + kind: models.KindOpenAITool, lang: models.LanguageTypeScript, wantFires: false, + src: "import { tool } from \"@openai/agents\";\nimport { z } from \"zod\";\n" + + "export const t = tool({ name: \"send_email\", description: \"Send\", parameters: z.object({ to: z.string(), body: z.string() }), needsApproval: true, execute: async ({ to, body }) => \"ok\" });\n", + }, + // Explicit needsApproval: false is exactly as un-gated as omitting it. + { + name: "OAI-031 fires when needsApproval: false explicitly", ruleID: "OAI-031", + kind: models.KindOpenAITool, lang: models.LanguageTypeScript, wantFires: true, + src: "import { tool } from \"@openai/agents\";\nimport { z } from \"zod\";\n" + + "export const t = tool({ name: \"send_email\", description: \"Send\", parameters: z.object({ to: z.string(), body: z.string() }), needsApproval: false, execute: async ({ to, body }) => \"ok\" });\n", + }, + // ── OAI-022: TS tool has no description ── { name: "OAI-022 fires on empty description", ruleID: "OAI-022", @@ -2390,6 +2560,36 @@ def get_payment(charge_id: str) -> dict: return {"ok": True} `, wantFires: false}, + // ─── LC-025 side-effect bounds (recipient / amount, no visible cap) ──── + {name: "LC-025 fires on send tool with free-form recipient", ruleID: "LC-025", kind: models.KindLangChainTool, src: ` +def send_email(to: str, body: str) -> str: + """Send an email.""" + return "ok" +`, wantFires: true}, + {name: "LC-025 fires on charge tool with free-form amount", ruleID: "LC-025", kind: models.KindLangChainTool, src: ` +def charge_card(customer_id: str, amount_cents: int) -> str: + """Charge a card.""" + return "ok" +`, wantFires: true}, + {name: "LC-025 silent when amount has an ALLOWED bound", ruleID: "LC-025", kind: models.KindLangChainTool, src: ` +def charge_card(customer_id: str, amount_cents: int) -> str: + """Charge a card.""" + if amount_cents > ALLOWED_MAX_CENTS: + raise ValueError("too much") + return "ok" +`, wantFires: false}, + {name: "LC-025 silent when the tool calls interrupt() for approval", ruleID: "LC-025", kind: models.KindLangChainTool, src: ` +def charge_card(customer_id: str, amount_cents: int) -> str: + """Charge a card.""" + interrupt("confirm this charge?") + return "ok" +`, wantFires: false}, + {name: "LC-025 silent when the name matches but the param does not", ruleID: "LC-025", kind: models.KindLangChainTool, src: ` +def charge_customer(customer_id: str) -> str: + """Charge, amount decided server-side.""" + return "ok" +`, wantFires: false}, + // ─── PYD-010 / PYD-011: Pydantic AI description quality ───────────────── // PYD-010 is has_description_text (placeholder markers); PYD-011 pairs // description_length_lt with has_docstring so an ABSENT docstring stays diff --git a/testdata/rules-fixture/langchain/side_effect_bounds.yaml b/testdata/rules-fixture/langchain/side_effect_bounds.yaml new file mode 100644 index 00000000..7fe3d5c5 --- /dev/null +++ b/testdata/rules-fixture/langchain/side_effect_bounds.yaml @@ -0,0 +1,110 @@ +policy: + id: langchain_side_effect_bounds + name: LangChain unbounded side-effect parameters + category: langchain + description: > + Rules that flag a side-effecting LangChain tool (send/notify/refund/ + charge/...) whose recipient or amount parameter is free-form model input + with no visible bound — no allow-list constraining who a message can go + to, no cap or enum constraining how much money moves. This is a different + failure mode from LC-017 (idempotency): a duplicate call repeats an + action within a bound the caller already accepted, while an unbounded + recipient or amount lets a single call move further than the caller + intended. + +rules: + - id: LC-025 + title: Side-effecting tool lets the model choose the recipient or amount with no visible bound + severity: high + confidence: 0.5 + language: python + applies_to: + - langchain_tool + scope: tool + match: + all: + - any: + - all: + - name_has_prefix: + - send_ + - notify_ + - param_name_matches: + contains: + - recipient + exact: + - to + - cc + - bcc + - email + - email_address + - phone + - phone_number + - to_email + - to_address + - to_number + - all: + - name_has_prefix: + - refund_ + - charge_ + - pay_ + - payout_ + - transfer_ + - issue_ + - param_name_matches: + contains: + - amount + exact: + - total + - price + suffixes: + - cents + - not: + has_body_text: + - "(le=" + - " le=" + - "(lt=" + - " lt=" + - "conint(" + - "condecimal(" + - "confloat(" + - "Literal[" + - "MAX_" + - "_MAX" + - "_LIMIT" + - "_limit" + - "ALLOWED" + - "allowed_" + - "ALLOWLIST" + - "allow_list" + - "WHITELIST" + - "whitelist" + - "endswith(\"@" + - "endswith('@" + - "interrupt(" + explanation: > + This LangChain tool's name signals a send/notify or a refund/charge/pay + side effect, and it takes a free-form recipient or amount parameter + with no visible bound: no allow-list constraining who the message + reaches, and no cap, range, or enum constraining how much money moves. + A ReAct agent's tool arguments come from the model's own reasoning, + which prompt injection, a misread task, or an upstream tool's poisoned + observation can steer — and that model can pass any address or any + amount into this call, with no checkpoint before it executes. This is a + distinct failure from LC-017 (idempotency): a duplicate call repeats an + action inside a bound the caller already accepted; this rule is about + a single call moving further — a wider blast radius, not a repeated + one. A tool body that calls LangGraph's interrupt(...) to pause and + wait for a human decision before acting is a gate and silences this + rule. + fix: > + Do not let the model supply the recipient or the amount unconstrained. + Derive the recipient from trusted context (a value bound to the + conversation or the authenticated user) rather than a model parameter, + or constrain it to an allow-list / domain allow-list validated before + the call executes. Cap the amount with a Pydantic field constraint on + the tool's args_schema (Annotated[int, Field(le=...)], conint(le=...)) + AND enforce the same cap server-side, since a static bound can be + bypassed by a differently-shaped call. For anything above a low, + pre-approved threshold, call interrupt(...) from inside the tool to + require a human to approve the specific recipient or amount before the + side effect runs. diff --git a/testdata/rules-fixture/mcp/side_effect_bounds.yaml b/testdata/rules-fixture/mcp/side_effect_bounds.yaml new file mode 100644 index 00000000..dd5d3e5f --- /dev/null +++ b/testdata/rules-fixture/mcp/side_effect_bounds.yaml @@ -0,0 +1,110 @@ +policy: + id: mcp_side_effect_bounds + name: MCP unbounded side-effect parameters + category: mcp + description: > + Rules that flag a side-effecting MCP tool (send/notify/refund/charge/...) + whose recipient or amount parameter is free-form model input with no + visible bound — no allow-list constraining who a message can go to, no + cap or enum constraining how much money moves. This is a different + failure mode from MCP-007 (idempotency): a duplicate call repeats an + action within a bound the caller already accepted, while an unbounded + recipient or amount lets a single call move further than the caller + intended. + +rules: + - id: MCP-030 + title: Side-effecting tool lets the model choose the recipient or amount with no visible bound + severity: high + confidence: 0.5 + language: python + applies_to: + - mcp_tool + scope: tool + match: + all: + - any: + - all: + - name_has_prefix: + - send_ + - notify_ + - param_name_matches: + contains: + - recipient + exact: + - to + - cc + - bcc + - email + - email_address + - phone + - phone_number + - to_email + - to_address + - to_number + - all: + - name_has_prefix: + - refund_ + - charge_ + - pay_ + - payout_ + - transfer_ + - issue_ + - param_name_matches: + contains: + - amount + exact: + - total + - price + suffixes: + - cents + - not: + has_body_text: + - "(le=" + - " le=" + - "(lt=" + - " lt=" + - "conint(" + - "condecimal(" + - "confloat(" + - "Literal[" + - "MAX_" + - "_MAX" + - "_LIMIT" + - "_limit" + - "ALLOWED" + - "allowed_" + - "ALLOWLIST" + - "allow_list" + - "WHITELIST" + - "whitelist" + - "endswith(\"@" + - "endswith('@" + - "elicit(" + explanation: > + This MCP tool's name signals a send/notify or a refund/charge/pay side + effect, and it takes a free-form recipient or amount parameter with no + visible bound: no allow-list constraining who the message reaches, and + no cap, range, or enum constraining how much money moves. A client + talks to this server on behalf of a model whose reasoning can be + steered by prompt injection, by its own misreading of the task, or by + an upstream tool's poisoned output — and that model can pass any + address or any amount into this call, with the handler acting on it + directly. This is a distinct failure from MCP-007 (idempotency): a + duplicate call repeats an action inside a bound the caller already + accepted; this rule is about a single call moving further — a wider + blast radius, not a repeated one. A handler that calls ctx.elicit(...) + to ask the human operator for confirmation before acting is a gate and + silences this rule. + fix: > + Do not let the model supply the recipient or the amount unconstrained. + Derive the recipient from trusted context (a value bound to the + authenticated session, not a tool argument) rather than a model + parameter, or constrain it to an allow-list / domain allow-list + validated before the call executes. Cap the amount with a Pydantic + field constraint on the tool's input model (Annotated[int, + Field(le=...)], conint(le=...)) AND enforce the same cap server-side, + since a static bound can be bypassed by a differently-shaped call. For + anything above a low, pre-approved threshold, use the MCP elicitation + capability (ctx.elicit(...)) to require the human on the other end of + the client to confirm before the handler proceeds. diff --git a/testdata/rules-fixture/openai_sdk/side_effect_bounds.yaml b/testdata/rules-fixture/openai_sdk/side_effect_bounds.yaml new file mode 100644 index 00000000..46244525 --- /dev/null +++ b/testdata/rules-fixture/openai_sdk/side_effect_bounds.yaml @@ -0,0 +1,213 @@ +policy: + id: openai_sdk_side_effect_bounds + name: OpenAI Agents SDK unbounded side-effect parameters + category: openai_sdk + description: > + Rules that flag a side-effecting OpenAI Agents SDK tool (send/notify/ + refund/charge/...) whose recipient or amount parameter is free-form model + input with no visible bound — no allow-list constraining who a message can + go to, no cap or enum constraining how much money moves. This is a + different failure mode from OAI-009/019 (idempotency): a duplicate call + repeats an action within a bound the caller already accepted, while an + unbounded recipient or amount lets a single call move further than the + caller intended. Retry/duplication is OAI-009/019's job; volume is + OAI-101/104's job (max_turns). + +rules: + - id: OAI-030 + title: Side-effecting tool lets the model choose the recipient or amount with no visible bound + severity: high + confidence: 0.5 + language: python + applies_to: + - openai_tool + scope: tool + match: + all: + - any: + - all: + - name_has_prefix: + - send_ + - notify_ + - param_name_matches: + contains: + - recipient + exact: + - to + - cc + - bcc + - email + - email_address + - phone + - phone_number + - to_email + - to_address + - to_number + - all: + - name_has_prefix: + - refund_ + - charge_ + - pay_ + - payout_ + - transfer_ + - issue_ + - param_name_matches: + contains: + - amount + exact: + - total + - price + suffixes: + - cents + - not: + has_body_text: + - "(le=" + - " le=" + - "(lt=" + - " lt=" + - "conint(" + - "condecimal(" + - "confloat(" + - "Literal[" + - "MAX_" + - "_MAX" + - "_LIMIT" + - "_limit" + - "ALLOWED" + - "allowed_" + - "ALLOWLIST" + - "allow_list" + - "WHITELIST" + - "whitelist" + - "endswith(\"@" + - "endswith('@" + - any: + - not: + tool_decorator_kwarg_present: + - needs_approval + - tool_decorator_kwarg_value: + kwarg: needs_approval + value: "False" + explanation: > + This @function_tool's name signals a send/notify or a refund/charge/pay + side effect, and it takes a free-form recipient or amount parameter with + no visible bound: no allow-list constraining who the message reaches, no + cap, range, or enum constraining how much money moves, and no + needs_approval gate. A model steered by prompt injection, by its own + misreading of the task, or by an upstream tool's poisoned output can + pass any address or any amount to this call, and the tool will act on + it. This is a distinct failure from OAI-009/019 (idempotency): a + duplicate call repeats an action inside a bound the caller already + accepted; this rule is about a single call moving further — a wider + blast radius, not a repeated one. Passing needs_approval=True or a + per-call approval callable is a gate and silences this rule, exactly as + it does for OAI-014. + fix: > + Do not let the model supply the recipient or the amount unconstrained. + Derive the recipient from trusted context (the authenticated user's + on-file address, a ticket's assignee) rather than a model parameter, or + constrain it to an allow-list / domain allow-list validated before the + call executes. Cap the amount with a schema bound (Annotated[int, + Field(le=...)], conint(le=...), or a plain range check) AND enforce the + same cap server-side, since a static bound can be bypassed by a + differently-shaped call. For anything above a low, pre-approved + threshold, pass needs_approval=True and route the approval through a + human. + + - id: OAI-031 + title: TypeScript side-effecting tool lets the model choose the recipient or amount with no visible bound + severity: high + confidence: 0.5 + language: typescript + applies_to: + - openai_tool + scope: tool + match: + all: + - any: + - all: + - name_has_prefix: + - send + - notify + - param_name_matches: + contains: + - recipient + exact: + - to + - cc + - bcc + - email + - emailAddress + - toEmail + - toAddress + - phone + - phoneNumber + - toNumber + - all: + - name_has_prefix: + - refund + - charge + - payout + - transfer + - issue + - param_name_matches: + contains: + - amount + exact: + - total + - price + suffixes: + - cents + - not: + has_body_text: + - ".max(" + - ".lte(" + - ".lt(" + - "maximum" + - "z.enum(" + - "nativeEnum(" + - "\"enum\"" + - "enum:" + - "MAX_" + - "_MAX" + - "maxAmount" + - "_LIMIT" + - "Limit" + - "ALLOWED" + - "allowedDomains" + - "AllowList" + - "allowList" + - "WHITELIST" + - "whitelist" + - "endsWith(\"@" + - "endsWith('@" + - any: + - not: + tool_decorator_kwarg_present: + - needsApproval + - tool_decorator_kwarg_value: + kwarg: needsApproval + value: "false" + explanation: > + This OpenAI Agents SDK tool authored in TypeScript has a name that + signals a send/notify or a refund/charge/pay side effect, and it takes a + free-form recipient or amount parameter with no visible bound: no + allow-list constraining who the message reaches, no cap, max, or enum + constraining how much money moves, and no needsApproval gate. A model + steered by prompt injection, by its own misreading of the task, or by an + upstream tool's poisoned output can pass any address or any amount to + this call. This is a distinct failure from OAI-019 (idempotency): a + duplicate call repeats an action inside a bound the caller already + accepted; this rule is about a single call moving further. Passing + needsApproval: true or a per-call approval function is a gate and + silences this rule, the same needsApproval option OAI-014 checks on the + Python side of this SDK. + fix: > + Do not let the model supply the recipient or the amount unconstrained. + Derive the recipient from trusted context rather than a model + parameter, or constrain it with a zod allow-list / domain check + validated before the call executes. Cap the amount with a zod bound + (z.number().max(...)) or an enum AND enforce the same cap server-side, + since a static bound in the schema can be bypassed by a differently- + shaped call. For anything above a low, pre-approved threshold, set + needsApproval: true and route the approval through a human. diff --git a/testdata/rules-fixture/pydantic_ai/side_effect_bounds.yaml b/testdata/rules-fixture/pydantic_ai/side_effect_bounds.yaml new file mode 100644 index 00000000..39658681 --- /dev/null +++ b/testdata/rules-fixture/pydantic_ai/side_effect_bounds.yaml @@ -0,0 +1,115 @@ +policy: + id: pydantic_ai_side_effect_bounds + name: Pydantic AI unbounded side-effect parameters + category: pydantic_ai + description: > + Rules that flag a side-effecting Pydantic AI tool (send/notify/refund/ + charge/...) whose recipient or amount parameter is free-form model input + with no visible bound — no allow-list constraining who a message can go + to, no cap or enum constraining how much money moves. This is a different + failure mode from PYD-007 (idempotency): a duplicate call repeats an + action within a bound the caller already accepted, while an unbounded + recipient or amount lets a single call move further than the caller + intended. + +rules: + - id: PYD-014 + title: Side-effecting tool lets the model choose the recipient or amount with no visible bound + severity: high + confidence: 0.5 + language: python + applies_to: + - pydantic_ai_tool + scope: tool + match: + all: + - any: + - all: + - name_has_prefix: + - send_ + - notify_ + - param_name_matches: + contains: + - recipient + exact: + - to + - cc + - bcc + - email + - email_address + - phone + - phone_number + - to_email + - to_address + - to_number + - all: + - name_has_prefix: + - refund_ + - charge_ + - pay_ + - payout_ + - transfer_ + - issue_ + - param_name_matches: + contains: + - amount + exact: + - total + - price + suffixes: + - cents + - not: + has_body_text: + - "(le=" + - " le=" + - "(lt=" + - " lt=" + - "conint(" + - "condecimal(" + - "confloat(" + - "Literal[" + - "MAX_" + - "_MAX" + - "_LIMIT" + - "_limit" + - "ALLOWED" + - "allowed_" + - "ALLOWLIST" + - "allow_list" + - "WHITELIST" + - "whitelist" + - "endswith(\"@" + - "endswith('@" + - any: + - not: + tool_decorator_kwarg_present: + - requires_approval + - tool_decorator_kwarg_value: + kwarg: requires_approval + value: "False" + explanation: > + This Pydantic AI tool's name signals a send/notify or a refund/charge/ + pay side effect, and it takes a free-form recipient or amount parameter + with no visible bound: no allow-list constraining who the message + reaches, no cap, range, or enum constraining how much money moves, and + no requires_approval gate. A model steered by prompt injection, by its + own misreading of the task, or by an upstream tool's poisoned output can + pass any address or any amount to this call, and the tool will act on + it. This is a distinct failure from PYD-007 (idempotency): a duplicate + call repeats an action inside a bound the caller already accepted; this + rule is about a single call moving further — a wider blast radius, not + a repeated one. Setting requires_approval=True on @agent.tool / + @agent.tool_plain routes the call through Pydantic AI's deferred-tool + approval flow and is a gate that silences this rule. + fix: > + Do not let the model supply the recipient or the amount unconstrained. + Derive the recipient from trusted context (the authenticated user's + on-file address, the run's originating conversation) rather than a + model parameter, or constrain it to an allow-list / domain allow-list + validated before the call executes. Cap the amount with a Pydantic + field constraint (Annotated[int, Field(le=...)], conint(le=...)) AND + enforce the same cap server-side, since a static bound can be bypassed + by a differently-shaped call. For anything above a low, pre-approved + threshold, set requires_approval=True and handle the resulting + DeferredToolRequests in your run loop so a human signs off before the + call executes.