Skip to content

feat(rules): add side-effect bounds rules for OpenAI, Pydantic AI, MCP, LangChain (batch 1) - #231

Merged
ivanpaghubasan merged 2 commits into
mainfrom
feat/p0-side-effect-bounds-batch1
Sep 29, 2026
Merged

ivanpaghubasan merged 2 commits into
mainfrom
feat/p0-side-effect-bounds-batch1

Conversation

@ivanpaghubasan

Copy link
Copy Markdown
Collaborator

Summary

Adds a new rule family that flags tools letting the model choose the recipient or amount of a side effect (email, notify, refund, charge, payout) with no visible bound. This is batch 1 of 2, covering four SDKs and five rules:

Rule | SDK | Language -- | -- | -- OAI-030 | OpenAI Agents SDK | Python OAI-031 | OpenAI Agents SDK | TypeScript PYD-014 | Pydantic AI | Python MCP-030 | MCP | Python LC-025 | LangChain | Python

Batch 2 (CrewAI, Google ADK, AutoGen, Vercel AI, Claude SDK TypeScript) will follow in a separate PR set once this one is reviewed.

Background

This comes from rank 12 of a competitive-analysis rule backlog (P0). The source framed it as a fix for the "3,400 duplicate notifications" failure mode and suggested critical severity. Verification against the current catalog showed those premises don't hold, so the rule was redesigned rather than implemented as written:

  • Different failure mode. Duplicate sends come from retries and loops, and a recipient allow-list or amount cap does nothing against 3,400 sends to an allowed recipient. That case is already covered by the idempotency rules and the turn/iteration-cap rules. This rule targets the case where a model, steered by prompt injection or its own error, chooses the destination or size of an irreversible side effect (OWASP LLM06, with LLM01 as the usual trigger).
  • Name alone is too noisy. The rule pairs a side-effect name prefix with a matching parameter: a recipient parameter for send/notify, an amount parameter for money verbs. charge_customer(customer_id) with the amount set server-side stays silent.
  • Not critical. Partial mitigations exist (approval hooks, tool guardrails, provider-side limits, validation in helpers), so the rule is high severity rather than critical.

What changed

  • New side_effect_bounds.yaml topic file in each of the four SDK packs under testdata/rules-fixture/. A separate topic file was chosen over extending idempotency.yaml because the concern is different (who receives the effect, or how much it moves, rather than duplicate execution).
  • Fire and silent test cases in internal/rules/policies_test.go for each rule, including a gate set to False still firing.
  • Updates to ARCHITECTURE.md, COVERAGE.md, and the rule count in CLAUDE.md.
  • No new predicate and no schema change. The rules use existing predicates only.

Design notes

  • Bound markers. A rule stays silent when the tool body or schema shows a visible bound: a schema constraint (le=, Field(le=…), .max(…), Literal[…]), a named cap, or an allow-list.
  • Approval gates. Each SDK's own gate silences the rule where one is visible at tool scope (for example needs_approval, requires_approval, interrupt(, elicit(). A gate explicitly set to False does not silence it.
  • Confidence 0.5. This is the pack's floor and the first high rule below 0.6. Confidence is low because "no cap" is a weaker claim than a missing key parameter, since a bound can live in a helper, a service layer, or the provider. Scoring multiplies severity by confidence, so this halves the surface penalty.

Known limitations

  • No data-flow check. The rule does not verify that the flagged parameter actually reaches the send or charge call.
  • Invisible gates. Guardrails attached after definition, agent-level approval hooks, and bounds defined in a separate schema class are not visible to the rule, and can produce false positives.
  • Claude SDK Python is not covered. The idiomatic @tool("name", "desc", {"to": str}) form records the parameter as args, so parameter-name rules can't see to or amount. This is a discovery gap in the engine and is tracked separately.

Verification

  • go build ./... and go test ./... pass.
  • RULES_REPO=../trustabl-rules scripts/check-rules-sync.sh reports the fixture in sync with production (113 files compared).

Related PRs

Merge order is engine, then rules, then rulebook. The paired PRs use the same branch name, feat/p0-side-effect-bounds-batch1:

  • Rules: trustabl/agent-reliability-rules (link)
  • Rulebook: trustabl/trustabl-rulebook (link)

Follow-ups (not in this PR)

  • Batch 2 of this rule family.
  • Claude SDK Python discovery: extract @tool schema-dict keys into ParamNames. The same gap makes CSDK-006 fire on every mutating-named Claude tool, even when an idempotency key is present in the schema.
  • notify_ is not among the idempotency rule prefixes, so duplicate notify_* sends are not covered there.

Resolves conflicts in CLAUDE.md and COVERAGE.md by combining Batch 1
side-effect-bounds work with main's agent-observability rules.
Recounted rule total: 298 (293 on main + 5 from Batch 1).
@ivanpaghubasan
ivanpaghubasan merged commit e5cef47 into main Sep 29, 2026
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants