Skip to content

Fix critical governance bypass and LLM classifier gaps - #35

Closed
requie wants to merge 2 commits into
mainfrom
claude/pr28-p29-review-arbrx0
Closed

Fix critical governance bypass and LLM classifier gaps#35
requie wants to merge 2 commits into
mainfrom
claude/pr28-p29-review-arbrx0

Conversation

@requie

@requie requie commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR closes two security gaps and adds cost controls to the adversarial LLM classifier:

  1. [Critical] Tool arguments could shadow governance rule fields, allowing a prompt-injected agent to bypass sensitive-tool approval (GOV-001).
  2. [High] The LLM classifier skipped the two multi-agent channels where the regex taxonomy is weakest, leaving cascade-compromise attacks undetected.
  3. [Feature] LLM classification cost is now bounded by concurrency limits and per-evaluation target caps, with verdict caching to avoid re-billing.

Key Changes

Security Fixes

Tool argument shadowing (governance.py, base.py)

  • Untrusted tool arguments now nest under action["arguments"] instead of spreading flat into the action dict
  • Prevents an argument named "tool" or "type" from overwriting the trusted fields that governance rules match on
  • GOV-001's sensitive-tool gate now evaluates the real tool name, not a forgery
  • GOV-003 (financial threshold) updated to read action["arguments"]["amount"]
  • New test suite (test_governance_action_shape.py) validates the nesting and confirms the bypass is closed

LLM classifier now covers multi-agent channels (adversarial_llm.py)

  • Added shared_memory and broadcast_messages to the targets fed to the classifier
  • Uses the same content-key fallbacks and channel labels as the regex scanner for consistency
  • Closes the gap where cascade-compromise attacks (T-CASCADE) reached neither strong detector

Cost Controls (adversarial_llm.py)

  • Bounded concurrency: max_concurrency (default 8) limits in-flight API calls
  • Per-evaluation target cap: max_targets_per_evaluation (default 200) prevents cost amplification on attacker-writable channels
  • Verdict caching: Verdicts cached per (channel, content-hash) so growing adapter buffers don't re-bill already-classified text
  • Fail-open handling: Neutral verdicts (network errors, missing keys) are never cached, since they record outages not judgments
  • Truncation reporting: details["llm_classifier"]["targets_dropped"] surfaces when a scan is capped, so it never reads as full coverage

Test Coverage

  • Extended existing test to verify both multi-agent channels are classified
  • New tests for content-key fallbacks (summary/text for shared memory, text for broadcast)
  • New tests for channel labeling (broadcast uses broadcast_channels to match regex scanner)
  • New test suite (TestCostBounds) validates concurrency limits, target caps, verdict caching, and fail-open behavior

Migration Notes

Custom PolicyRule conditions that read tool-call arguments from the top level of context["action"] must be updated to read from action["arguments"]. Built-in rules have been updated; this only affects custom policy code.

Implementation Details

  • Verdict cache uses OrderedDict with oldest-first eviction at the size limit
  • Cache key is (channel, sha256(text)) to handle the same text on different channels as distinct questions
  • Semaphore-based concurrency control in _classify_targets ensures bounded parallelism
  • Logging warns when targets are truncated, with counts for scanned and dropped

… LLM channel gap

Three related fixes to the runtime integrity path.

1. Untrusted tool arguments could shadow the fields governance matches
   on. The pre_tool_use buffer entry spread the agent's own arguments
   over the trusted fields, and that entry becomes context["action"], so
   an argument named "tool" overwrote the real tool name: a
   prompt-injected agent calling payment_execute with {"tool": "noop"}
   skipped GOV-001's sensitive-tool gate entirely. Under enforce=True
   the dangerous tool ran; in observe-only mode the signed attestation
   recorded governance:pass for a check that never happened. The same
   trick overrode "type". Arguments now nest under action["arguments"],
   the shape the decision-record path already used, so no
   agent-controlled key can collide with a trusted field. GOV-003 reads
   the nested amount.

2. The LLM classifier skipped the two multi-agent channels. The regex
   scanner covers seven channels; aevaluate fed only five to the
   classifier, so shared_memory and broadcast_messages reached neither
   strong detector (the regex taxonomy scores ~0 on the action-oriented
   injections that travel through them). That left the cross-agent
   cascade path covered only by the detector that cannot see it. Both
   channels are now classified, using the regex scanner's content keys
   and channel labels.

3. Classification cost is bounded. Calls ran sequentially with no
   ceiling while the multi-agent buffers cap at 1000 entries per
   channel. Calls now run with bounded concurrency and a per-evaluation
   target ceiling; truncation is logged and reported so a capped scan
   never reads as full coverage. Verdicts are cached per (channel,
   content hash) with oldest-first eviction, and fail-open verdicts are
   never cached since they record an outage rather than a judgment.

Adds regression tests for the bypass, both channel gaps, and every cost
bound. Full suite green (640 passed), ruff and mypy --strict clean.
@cursor

cursor Bot commented Jul 25, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@requie requie self-assigned this Jul 25, 2026
@requie requie added bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request labels Jul 25, 2026
@requie requie closed this Jul 25, 2026
@requie
requie deleted the claude/pr28-p29-review-arbrx0 branch July 25, 2026 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bound LLM classification cost in AdversarialLLMLayer.aevaluate Nest tool arguments under arguments in the action dict; rework GOV-003 to match

1 participant