fix(ibac): Bypass transport-layer calls and let agents take self-actions#450
Merged
huang195 merged 6 commits intoMay 30, 2026
Merged
Conversation
All three listeners (forwardproxy, reverseproxy, extproc) build a
pipeline.Context but skip the Method field, leaving every plugin to
read it as the empty string. The bug was masked because no plugin
acted on pctx.Method until ibac's describeAction() formatted it into
the judge prompt — where it produced a leading-space artifact in
session events (" http://...") that operators reading abctl couldn't
explain.
Populate Method consistently:
- forwardproxy.handleRequest / handleConnect: from r.Method
- reverseproxy.handleRequest: from r.Method
- extproc.handle{Inbound,Outbound}{,Body}: from the :method HTTP/2
pseudo-header, matching the existing :scheme / :path / :authority
pattern.
Required prerequisite for ibac's body-less GET bypass.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
Body-less GET requests carry no action payload to evaluate — typical patterns include MCP Streamable HTTP's server→client SSE channel, agent-card fetches, and OAuth metadata probes. Sending these to the LLM judge is a category error: there is nothing to judge, and the judge either denies for lack of context or returns a non-deterministic verdict that depends on noise in the prompt. Symptom seen on a real agent (exgentic-a2a-tool-calling): the MCP client opens GET /mcp for SSE alongside its POST endpoint. Without this bypass, ibac denies the SSE open with no_intent at startup, and later — once a user A2A turn populates intent — with "blocked" because the judge correctly notes "no specific task or calculation is provided" (the SSE open has no payload). The client then loops on reconnect, never establishing the streaming channel. Add step 5b to IBAC.OnRequest: skip body-less GETs with reason "transport_stream" before the intent check. Side-effect requests (POST/PUT/DELETE/PATCH) always have bodies and reach the judge as before; an attacker cannot smuggle an action through a body-less GET without a request body to put it in. The bypass relies on the listener-side prerequisite committed separately to populate pctx.Method. Tests: - body-less GET → skip/transport_stream, judge not called - GET with body → judged (the body is the action) - body-less POST/PUT/DELETE/PATCH → judged (deliberately not bypassed) makePCtx default updated from GET-with-no-body (which was an unusual shape masking the prior listener bug) to a realistic POST-with-body, matching the contract IBAC actually models. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughListeners (extproc, forwardproxy, reverseproxy) populate pipeline.Context.Method for inbound/outbound header and body phases. IBAC.OnRequest adds an early bypass that skips judge evaluation when the outbound request body is empty and the method is GET, HEAD, or OPTIONS. Tests and helper defaults were updated. ChangesHTTP Method Propagation and Transport Stream Bypass
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
authbridge/authlib/plugins/ibac/plugin.go (1)
279-279: 💤 Low valueRenumber steps for clarity.
Steps 6 (line 279: "Pull the user's most recent declared intent") and 6 (line 315: "Build action description and call judge") are distinct operations. The second should be step 7, and subsequent steps (currently 7 and 8 at lines 319 and 359) should be renumbered to 8 and 9 for consistency.
📝 Proposed step renumbering
- // 6. Build action description and call judge. + // 7. Build action description and call judge. action := describeAction(pctx, p.cfg.JudgeInference) verdict, reason, err := p.judge.Evaluate(ctx, intentText, action) - // 7. Fail closed on judge errors. Two flavors, distinguished + // 8. Fail closed on judge errors. Two flavors, distinguished // via the ErrJudgeUncertain sentinel so operator dashboards // don't conflate model-output bugs with infra outages:- // 8. Apply verdict. + // 9. Apply verdict. if verdict == "deny" {Also applies to: 315-385
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@authbridge/authlib/plugins/ibac/plugin.go` at line 279, Update the step numbering in the comment block in plugin.go where the steps list contains two "6." entries: change the second "6. Build action description and call judge" (the comment that mentions "Build action description and call judge") to "7.", and increment the following steps currently labeled "7." and "8." to "8." and "9." respectively so the sequence is continuous; locate the multi-step comment containing "Pull the user's most recent declared intent" and the subsequent steps (the block covering the later comments around the "Build action description and call judge" and the next two steps) and adjust the numeric prefixes only.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@authbridge/authlib/plugins/ibac/plugin.go`:
- Line 279: Update the step numbering in the comment block in plugin.go where
the steps list contains two "6." entries: change the second "6. Build action
description and call judge" (the comment that mentions "Build action description
and call judge") to "7.", and increment the following steps currently labeled
"7." and "8." to "8." and "9." respectively so the sequence is continuous;
locate the multi-step comment containing "Pull the user's most recent declared
intent" and the subsequent steps (the block covering the later comments around
the "Build action description and call judge" and the next two steps) and adjust
the numeric prefixes only.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9dcea7ae-4ab3-451d-8bd2-8a55df4470c1
📒 Files selected for processing (5)
authbridge/authlib/listener/extproc/server.goauthbridge/authlib/listener/forwardproxy/server.goauthbridge/authlib/listener/reverseproxy/server.goauthbridge/authlib/plugins/ibac/plugin.goauthbridge/authlib/plugins/ibac/plugin_test.go
Three points raised during review:
1. Widen transport_stream bypass to GET, HEAD, OPTIONS. HEAD shares
GET semantics by RFC 9110 §9.3.2 (servers MUST answer HEAD
identically to GET, sans body); OPTIONS is CORS preflight or
capability discovery, never an action. The threat-model argument
("attacker can't smuggle a payload through a body-less request
without a body to put it in") extends verbatim to all three.
Extracted to an isTransportRetrieval() helper, mirroring
isMCPHousekeeping()'s shape.
2. Document the server-side query-string caveat. A REST-violating
server that handles side-effect operations through GET query
strings (e.g. ?action=delete&id=42) bypasses IBAC by design —
defending against that needs to live in the server. Made the
contract self-documenting in the step-5b comment so future
readers don't have to re-derive it.
3. Add a no-leading-whitespace assertion on describeAction. The
listener-side pctx.Method wiring is a prerequisite for the
transport_stream bypass; if it ever regresses, the visible
symptom is a leading-space artifact in session events
(" http://...") and the bypass silently stops firing for GET/
HEAD/OPTIONS. The new test asserts describeAction's output
starts with the HTTP method, locking in that contract.
Also widened the body-less retrieval test to a table-driven case
covering GET / HEAD / OPTIONS in one place.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@authbridge/authlib/plugins/ibac/plugin.go`:
- Around line 506-522: The doc comment originally intended for isMCPHousekeeping
is now attached to isTransportRetrieval because the new function was inserted
immediately after the comment; fix by restoring separation: either insert a
blank line between the existing doc comment block and the isTransportRetrieval
function so the comment remains associated with isMCPHousekeeping, or move
isTransportRetrieval to follow the isMCPHousekeeping function; ensure the
comment block immediately precedes isMCPHousekeeping to reattach it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a970de40-9700-4c19-bca9-69730ed7179b
📒 Files selected for processing (2)
authbridge/authlib/plugins/ibac/plugin.goauthbridge/authlib/plugins/ibac/plugin_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- authbridge/authlib/plugins/ibac/plugin_test.go
CodeRabbit flagged a pre-existing inconsistency exposed by the step-5b insert: two consecutive comment blocks in OnRequest were both numbered "6" (intent-fetch and judge-build), then "7" and "8" followed. Renumber the post-bypass steps to 6, 7, 8, 9 so the sequence reads continuously. Pure comment change; no behavior delta. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
pdettori
approved these changes
May 30, 2026
pdettori
left a comment
pdettori
left a comment
Member
There was a problem hiding this comment.
APPROVE — clean PR, no issues found.
Areas reviewed: Go (listeners, IBAC plugin), Tests
Commits: 4 commits, all DCO signed-off
CI: all green
The two-commit split (method threading → bypass logic) is clean. The threat model is explicitly documented, the bypass condition (isTransportRetrieval — GET/HEAD/OPTIONS + empty body) is narrow and defensible, and test coverage is comprehensive with positive, negative, and invariant cases.
Assisted-By: Claude Code
When IBAC reaches step 6 without a recorded user intent — either
because Session is nil (no inbound A2A turn has populated the active
bucket yet) or because the session contains no extractable user-role
A2A fragment — the previous behavior was always to fail closed with
a 403. That's the right answer for deployments where every outbound
is supposed to be user-driven, but it's the wrong answer for
deployments where agents legitimately take self-actions:
- Initialization flows that do real tools/call setup before any user
turn (housekeeping bypass covers protocol setup but not body-having
side-effect calls)
- Machine-to-machine A2A flows where the inbound side isn't a
human-shaped user turn
- Headless cron-driven or event-triggered agents where there's never
a "user" in the IBAC sense
Add no_intent_policy with two values:
- "allow" (default): Skip with reason "no_user_context", Continue.
Treats the request as a legitimate non-user-driven action. The
Invocation carries a sub_reason ("no_session" or "no_intent") so
operators can still distinguish the two states in abctl.
- "deny": Reject 403 with the existing "no_session" / "no_intent"
reasons. Preserves the prior fail-closed semantics for operators
who want them.
Default is "allow" because IBAC's threat model targets prompt-
injection making the agent emit user-misaligned actions — that
requires there to be a user. When there isn't one, IBAC has nothing
to align against and shouldn't be in the middle of the decision.
Tests:
- TestConfigure_AppliesDefaults extended to assert default
- TestConfigure_NoIntentPolicy_{RejectsUnknownValue,AcceptsExplicitValues}
- TestOnRequest_NoIntent_PolicyDeny_FailsClosed (legacy behavior
reachable via explicit config)
- TestOnRequest_NoIntent_PolicyAllow_Bypasses (new default)
- TestOnRequest_NilSession_PolicyDeny_FailsClosed
- TestOnRequest_NilSession_PolicyAllow_Bypasses
- newConfiguredIBACDeny test helper for the strict variant
Threat-model trade-off worth being explicit about: the default flip
means an attacker who compromises the agent before the first user
turn (e.g. via a poisoned tool-list response triggering bootstrap
LLM reasoning) is no longer caught by IBAC during that pre-user
window. That's outside IBAC's threat model anyway — credential
theft and pre-deployment compromise need different controls (mTLS,
SPIFFE attestation, policy on the credential issuer). Operators
who deploy IBAC against purely user-driven agents and want hard
fail-closed semantics flip to no_intent_policy: deny.
Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: Hai Huang <huang195@gmail.com>
huang195
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@authbridge/authlib/plugins/ibac/plugin.go`:
- Around line 155-157: The default for the no-intent policy is currently set to
allow which weakens existing deployments; change the initialization in the
plugin constructor so that c.NoIntentPolicy defaults to the fail-closed value
(use NoIntentPolicyDeny instead of NoIntentPolicyAllow) when c.NoIntentPolicy ==
""; update any related comments and ensure the constant NoIntentPolicyDeny is
defined and used wherever NoIntentPolicyAllow was previously assumed to enforce
a deny-by-default posture (look for c.NoIntentPolicy and NoIntentPolicyAllow
symbols).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 52ebb493-e378-43ad-b03f-14b7553e1e5e
📒 Files selected for processing (2)
authbridge/authlib/plugins/ibac/plugin.goauthbridge/authlib/plugins/ibac/plugin_test.go
CodeRabbit flagged that the // isMCPHousekeeping ... and // isTransportRetrieval ... doc comments were running together with no blank-line separator, so godoc merged them into a single block attached to isTransportRetrieval — leaving isMCPHousekeeping with no doc. Move isTransportRetrieval to follow isMCPHousekeeping (the cleaner of the two CodeRabbit-suggested fixes). Each function now has its own contiguous doc block, properly attached. Pure structural change; no behavior delta. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
github-project-automation
Bot
moved this from New /:ToDo
to Done
in Kagenti Issue Prioritization
This was referenced May 30, 2026
huang195
added a commit
to huang195/kagenti-extensions
that referenced
this pull request
May 31, 2026
…+ posture docs Four review-feedback follow-ups in one commit, scoped to IBAC plus its demo manifest and reference docs. Build + test pass. 1. Must-fix rossoctl#1 — TestOnRequest_InferenceBypassByDefault assertion was passing for the wrong reason. With makePCtx populating MCPExtension{IsAction:true} by default, the test's InferenceExtension{IsAction:false} (zero value) caused step 4 (classification gate) to skip with protocol_mechanics before step 5 (inference-policy) could run. A regression in the inference-policy logic would have gone uncaught. Fix: set IsAction:true on the Inference extension (matches what inference-parser does in production). Add an explicit assertion on inv.Reason == "inference_bypass" so the test verifies the right step fired. Same fix shape as the earlier 944838d correction to TestOnRequest_InferenceJudgedWhenEnabled, which missed this sibling. 2. Must-fix rossoctl#2 — IBAC demo's plain-HTTP exfiltration scenario (the headline ibac-plugin.md threat model) silently passed through the new defense-in-depth gate, since no parser claims plain HTTP and step 4 short-circuits with !anyAction. Fix: introduce unclassified_policy as a config field on the ibac plugin, parallel in shape to the existing no_intent_policy. - Default "passthrough": current behavior. Defense in depth — IBAC only judges what a parser classified. - "judge": falls through to inference policy + intent + judge even when no parser claimed the request. Catches plain-HTTP outbound at the cost of one judge round-trip per unclassified request. Demo's k8s/ibac-patch.yaml sets unclassified_policy: "judge" so the headline scenario keeps working. Production deployments using MCP/A2A/inference exclusively get cleaner defense-in-depth behavior without changing config; deployments that want plain- HTTP egress coverage opt in. 3. Inline suggestion — RequiresAny: ["mcp-parser", "a2a-parser", "inference-parser"] was misleading. RequiresAny is a same-chain check; IBAC runs OUTBOUND, but a2a-parser runs INBOUND in every in-tree config. Including it would let an outbound chain [a2a-parser, ibac] pass validation while a2a-parser populates nothing IBAC reads. Fix: drop a2a-parser. RequiresAny: ["mcp-parser", "inference-parser"]. Doc-comment in Capabilities() explains the cross-chain a2a dependency stays runtime, governed by no_intent_policy. 4. Suggestion — operator-facing default-posture documentation. docs/ibac-plugin.md gains a "Default security posture" subsection under Limitations covering both fail-open knobs (unclassified_policy, no_intent_policy) and how operators choose between them. Config-table rows added for both. Threat-coverage notes added: plain-HTTP exfil is covered when unclassified_policy: "judge"; MCP-shaped exfil is covered by default. The PR's compatibility section is intentionally NOT amended for no_intent_policy — that flag landed in rossoctl#450 (already in main), not rossoctl#452, and the durable home for the operator guidance is the plugin's reference doc. Tests: - TestOnRequest_InferenceBypassByDefault now asserts inv.Reason. - New TestOnRequest_UnclassifiedPolicy_Judge covers the policy=judge branch (unclassified body-having POST → reaches judge → reject). - New TestConfigure_UnclassifiedPolicy_DefaultsToPassthrough. - New TestConfigure_UnclassifiedPolicy_RejectsUnknownValue. - New TestConfigure_UnclassifiedPolicy_AcceptsExplicitValues (parameterized over {passthrough, judge}). - Existing tests pass unchanged — default behavior preserved. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
huang195
added a commit
that referenced
this pull request
Jun 1, 2026
After PR #450 landed the body-less GET/HEAD/OPTIONS bypass, one class of MCP traffic still reaches the judge and gets denied: the MCP Streamable HTTP session-termination signal — DELETE to the MCP endpoint with the Mcp-Session-Id header. The MCP client SDK fires this at end-of-conversation to release server-side session state. Symptom seen on a real agent (exgentic-a2a-tool-calling-gsm8k): after the user's task completes successfully, the agent issues DELETE /mcp with Mcp-Session-Id; the judge sees the bare request line (no body) and reasonably answers something like "Action involves deleting data, which is not strictly necessary for user intent." 403 on routine protocol cleanup. Extend the transport_stream bypass to cover this shape. The Mcp-Session-Id header is set by the client SDK (never user input), so it's a precise distinguisher between transport cleanup and a real "DELETE /api/users/42" action call — the latter has no MCP session header and still reaches the judge. Refactor: replace the single-condition isTransportRetrieval check at step 5b with a wider isTransportShaped helper that wraps both patterns. Keeps the existing isTransportRetrieval(method) helper for testability and readability. Tests: - TestOnRequest_TransportStream_MCPSessionTerminate (positive: body-less DELETE + Mcp-Session-Id → skip/transport_stream) - TestOnRequest_TransportStream_BodylessDELETEWithoutHeaderIsJudged (negative: body-less DELETE without the header → judged, the header is the load-bearing distinguisher) - Existing TestOnRequest_TransportStreamBypass_BodylessPOSTIsJudged still validates body-less DELETE without Mcp-Session-Id reaches the judge — comment cross-references the new test. Threat model unchanged: an attacker can't smuggle a payload through a body-less request, and the Mcp-Session-Id header is set by the client SDK rather than user input, so the bypass condition isn't attacker-controllable in any way that bypasses other action shapes. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
huang195
added a commit
to huang195/kagenti-extensions
that referenced
this pull request
Jun 1, 2026
…+ posture docs Four review-feedback follow-ups in one commit, scoped to IBAC plus its demo manifest and reference docs. Build + test pass. 1. Must-fix rossoctl#1 — TestOnRequest_InferenceBypassByDefault assertion was passing for the wrong reason. With makePCtx populating MCPExtension{IsAction:true} by default, the test's InferenceExtension{IsAction:false} (zero value) caused step 4 (classification gate) to skip with protocol_mechanics before step 5 (inference-policy) could run. A regression in the inference-policy logic would have gone uncaught. Fix: set IsAction:true on the Inference extension (matches what inference-parser does in production). Add an explicit assertion on inv.Reason == "inference_bypass" so the test verifies the right step fired. Same fix shape as the earlier 944838d correction to TestOnRequest_InferenceJudgedWhenEnabled, which missed this sibling. 2. Must-fix rossoctl#2 — IBAC demo's plain-HTTP exfiltration scenario (the headline ibac-plugin.md threat model) silently passed through the new defense-in-depth gate, since no parser claims plain HTTP and step 4 short-circuits with !anyAction. Fix: introduce unclassified_policy as a config field on the ibac plugin, parallel in shape to the existing no_intent_policy. - Default "passthrough": current behavior. Defense in depth — IBAC only judges what a parser classified. - "judge": falls through to inference policy + intent + judge even when no parser claimed the request. Catches plain-HTTP outbound at the cost of one judge round-trip per unclassified request. Demo's k8s/ibac-patch.yaml sets unclassified_policy: "judge" so the headline scenario keeps working. Production deployments using MCP/A2A/inference exclusively get cleaner defense-in-depth behavior without changing config; deployments that want plain- HTTP egress coverage opt in. 3. Inline suggestion — RequiresAny: ["mcp-parser", "a2a-parser", "inference-parser"] was misleading. RequiresAny is a same-chain check; IBAC runs OUTBOUND, but a2a-parser runs INBOUND in every in-tree config. Including it would let an outbound chain [a2a-parser, ibac] pass validation while a2a-parser populates nothing IBAC reads. Fix: drop a2a-parser. RequiresAny: ["mcp-parser", "inference-parser"]. Doc-comment in Capabilities() explains the cross-chain a2a dependency stays runtime, governed by no_intent_policy. 4. Suggestion — operator-facing default-posture documentation. docs/ibac-plugin.md gains a "Default security posture" subsection under Limitations covering both fail-open knobs (unclassified_policy, no_intent_policy) and how operators choose between them. Config-table rows added for both. Threat-coverage notes added: plain-HTTP exfil is covered when unclassified_policy: "judge"; MCP-shaped exfil is covered by default. The PR's compatibility section is intentionally NOT amended for no_intent_policy — that flag landed in rossoctl#450 (already in main), not rossoctl#452, and the durable home for the operator guidance is the plugin's reference doc. Tests: - TestOnRequest_InferenceBypassByDefault now asserts inv.Reason. - New TestOnRequest_UnclassifiedPolicy_Judge covers the policy=judge branch (unclassified body-having POST → reaches judge → reject). - New TestConfigure_UnclassifiedPolicy_DefaultsToPassthrough. - New TestConfigure_UnclassifiedPolicy_RejectsUnknownValue. - New TestConfigure_UnclassifiedPolicy_AcceptsExplicitValues (parameterized over {passthrough, judge}). - Existing tests pass unchanged — default behavior preserved. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
9 tasks
This was referenced Jun 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three related IBAC behavior changes that together stop the plugin from blocking legitimate non-user-driven agent traffic, while keeping its core threat model intact:
pctx.Methodin all three listeners (forwardproxy, reverseproxy, extproc) — they were leaving it empty, masking a real listener bug and breaking any plugin that wants to act on HTTP method.transport_streambypass for body-less retrieval-shaped requests (GET/HEAD/OPTIONS with no body), so the MCP Streamable HTTP server→client SSE channel, agent-card fetches, OAuth metadata probes, and CORS preflights are not sent to the LLM judge.no_intent_policyconfig field, default"allow", controlling what happens when IBAC reaches the intent check without a recorded user intent. Allow = treat as agent self-action and Skip; deny = preserve the prior 403 fail-closed semantics.Why
On a real agent (
exgentic-a2a-tool-calling-gsm8kin team1), IBAC was denying multiple classes of legitimate traffic:no_intentdenials at agent startup, before any user turntransport_streambypassblockeddenials of the SSE GET after a user turn — judge correctly noting "no specific task or calculation is provided" because the SSE open carries no payloadtransport_streambypassjudge_unavailabledenials when Ollama hiccupstransport_streambypass" http://..."(leading space)pctx.Method, sodescribeAction()rendered the empty method as just whitespacepctx.Methodin all listenerstools/callwith its own credentials before any user turn (initialization, machine-to-machine, headless cron)no_intent_policy: allow(default)All four denies in that pod's session timeline disappear with this PR.
What changes
Listener prerequisite
authlib/listener/{forwardproxy,reverseproxy,extproc}/server.go— populatepctx.Methodat every pctx construction site. HTTP listeners readr.Method; ext_proc reads the:methodHTTP/2 pseudo-header (matching the existing:scheme/:path/:authoritypattern).transport_stream bypass
authlib/plugins/ibac/plugin.go— new step 5b inOnRequest:Where
isTransportRetrievalcoversGET,HEAD,OPTIONS. Placed after the existingmcp_housekeepingcheck, before the intent fetch.Threat model: an attacker can't smuggle a payload through a body-less GET/HEAD/OPTIONS — there's no request body to put the action in. POST/PUT/DELETE/PATCH (the methods that carry action semantics) always reach the judge.
Caveat: servers that handle side-effect operations through GET query strings (e.g.
?action=delete&id=42) violate REST semantics and would bypass IBAC here — by design. Defending against that needs to live in the server, not in IBAC.no_intent_policy
New config field with two values:
"allow"(default): Skip with reason"no_user_context", Continue. Treats the request as a legitimate non-user-driven action. The Invocation carries asub_reason("no_session"or"no_intent") so operators can still distinguish the two states in abctl."deny": Reject 403 with the existing"no_session"/"no_intent"reasons. Preserves the prior fail-closed semantics.Default is
"allow"because IBAC's threat model targets prompt-injection making the agent emit user-misaligned actions — that requires there to be a user. When there isn't one (agent self-actions during init, machine-to-machine A2A, headless cron-driven flows), IBAC has nothing to align against.The trade-off: an attacker who compromises the agent before the first user turn (e.g. via a poisoned tool-list response triggering bootstrap LLM reasoning) is no longer caught by IBAC during that pre-user window. That's outside IBAC's threat model anyway — credential theft and pre-deployment compromise need different controls (mTLS, SPIFFE attestation, policy on the credential issuer). Operators who deploy IBAC against purely user-driven agents and want hard fail-closed semantics flip to
no_intent_policy: deny.Test plan
go test ./...inauthbridge/authlib— all packages passdescribeActionhas no leading whitespace (regression lock on listener Method wiring)no_intent_policydefaults to"allow""allow"and"deny"authbridge:latesttoteam1/exgentic-a2a-tool-calling-gsm8kand confirm:skiprowstools/calltraffic is still judgedNotes
pctx.Methodfix is a prerequisite for the GET/HEAD/OPTIONS check — without it, the bypass never matches becauseMethod == ""everywhere. Split into a separate commit for review clarity.Assisted-By: Claude (Anthropic AI) noreply@anthropic.com
Summary by CodeRabbit
Bug Fixes
New Features
Performance
Tests