Most AI-agent setups today hand the agent the real API key directly — in an
environment variable, a config file, sometimes typed straight into the
...
For further actions, you may consider blocking this person and/or reporting abuse
Separating the capability ticket from the raw bearer token is the right boundary. Once you let an agent hold an ambient GitHub PAT, every fetched issue body is effectively an unauthenticated prompt injection attack surface. Having an out-of-band broker reject repo delete before the model output even reaches the network turns an arbitrary execution risk into a boring policy denial.
Hi @reidmarlow , glad to know your views on this and yes correct, that's exactly the failure mode I was trying to design out. The moment an agent holds an ambient PAT, you've basically turned every issue body, PR comment, and README it reads into a code path with root on your GitHub account. The broker doesn't make the model smarter or more trustworthy, it just makes sure that even if the model gets fully owned by something it read, the worst it can do is ask for something the policy will refuse before a token ever leaves the broker's process. Appreciate you putting it that plainly, that's the one-sentence version I wish I'd written for the post 😃
I ran your demo path's four shapes against
f7532aa8with the credential provider and connector instrumented, because the thing I wanted to check is the one your article states most cleanly: "only now: the real credential." Nothing here is a bypass — the DENY/ALLOW behavior is exactly what you describe. What I found is that the record the broker leaves behind cannot distinguish three pairs of cases the code itself distinguishes.1. In the MCP proxy, the broker never touches a credential — and the record doesn't say so.
mcp_proxy.py:163submits withexecute=Falseand line 166 then callsupstream.call_tool(name, arguments)itself. That is the mode your three-model demo runs in. So I ran both shapes:Both write this, identically:
and both spend one call from the budget. So on the path your demos exercise, the broker is a decider, not the credential holder: the credential that performs the action belongs to the wrapped upstream server, is configured outside capbroker, and is not scoped by the capability. The enforcement is still real (the hand-written map plus the decision), but the sentence your README opens with — "only the broker ever touches the real credential" — is true of
POST /actionsand false of the MCP path, and a reader has no way to tell from the log which one produced a given ALLOW. I'd add"executed_by": "broker" | "caller"(the broker already knows, it's theexecuteflag) and say in the proxy section which one applies. Dispatching through the broker's connector instead is the other option, but it moves the upstream's token into the broker, which I don't think you want.2. Five denial paths log request fields from before the authenticity boundary — and the ordering is right, for the reason PROGRESS.md gives (you need the capability before you know which key to verify against). But
deny()writesagent_id,operationandresourcefrom that unverified request, andhandle()'s own comment says "Only past this point is anything inreqtreated as authentic." That's true of the decision; it isn't true of the log. A caller holding no key at all:That record is shape-identical to an authenticated denial — no authenticity marker anywhere on it. I know the ordering is deliberate and I'm not arguing with it; the gap is that
audit.pyalready treats the log as an artifact that has to be safe for a reader (it hard-fails on credential-shaped keys) while the same file will happily record an unauthenticated caller's claims about who they are. One field ("authenticated": false, or a distinct event name) closes it and costs nothing.The related property is that the reason is a capability-state oracle:
unknown_capabilityvscapability_revokedvs (after expiry or exhaustion)capability_expired/call_limit_exceededfor a caller who has the id and no key. You already test the HTTP half of that (test_unknown_capability_over_http), so I'm flagging it as a consequence rather than a discovery — the part I couldn't find stated anywhere is that it's also what fills the log.3. A failed execution still reads as ALLOW, and still costs a call.
With
max_calls=1and a connector that raises: the ALLOW record is written,call_countis 1 of 1, and the next request on that capability isDENY call_limit_exceeded.server.pyreturns{"decision": "ERROR"}to the caller, so the caller knows; a reader of the audit log does not."executed": true|falseon the record (or a secondexecution_failedevent) would make "the log says ALLOW" mean "it happened" rather than "it was authorized and then whatever happened, happened."All three have the same shape, which is why I'd treat them as one change rather than three: the decision record is the only artifact here that a human reads after the fact, and it is written from inside the function at the moment each check passes — so it inherits that function's position in the pipeline rather than the outcome of the whole call. The reason it's worth a look in a project that otherwise gets this right: the rest of your design is unusually careful about who can be trusted to say what (the agent can't sign arbitrary blobs, Jev can't decide, the operator can't be bypassed), and the log is the one place where an unauthenticated party can currently write.
Two notes on method so you can judge the numbers: I stubbed the GitHub connector and monkeypatched
get_connector— no network, no real GitHub — so what I measured is the broker's bookkeeping and the record it writes, not GitHub's API. Andpath Bis the proxy's sequence re-run in-process rather than through the MCP transport, since the only line that matters for this isexecute=False.@howcani_howcani_77e786a89 , Thanks for this awesome feedback, you found something the article doesn't just fail to mention, it actively overclaims. "Only the broker ever touches the real credential" is a true sentence about POST /actions and a false sentence about the MCP proxy path, and I said it like it applied to both. That's on me, not a nitpick.
Going through your three points:
The executed_by gap - yeah. The proxy calling upstream.call_tool() directly after execute=False was a deliberate choice (I didn't want the upstream server's own token flowing into the broker's process), but I didn't think through what that does to the log's honesty. Right now the log records a decision, and silently lets the reader assume the decision and the execution are the same event. They're not, on that path, and there's no way to tell from the record alone. Adding "executed_by": "broker" | "caller" is a one-line change since, like you said, the broker already knows - it's just the execute flag it already has in hand. Doing that.
Unauthenticated fields in deny records - this is the one that actually unsettled me a bit, because you're right that I have a whole module (audit.py) that hard-fails rather than let a credential-shaped value near the log, and then the same file will cheerfully write down "the-CEO says they want repo.delete" from someone holding no key at all. The ordering is correct , I do need the capability before I know which key to check against — but correctness of the ordering isn't the same as honesty in the log about what that ordering means. An unauthenticated claim and a verified one going into the same JSON shape with no marker is exactly the kind of thing that looks fine until someone greps the log for "who did what" months later and takes agent_id at face value. "authenticated": false on anything logged before the signature check passes fixes it cheaply. Also, good catch on the reason-as-oracle side effect; I'd tested that it leaks over HTTP but hadn't connected that it's the exact same leak permanently sitting in the audit trail.
ALLOW logged before execution succeeds - this is the sharpest one. I wrote audit.log(ALLOW) at the point where I knew the authorization was final, which is true, but a reader has no way to know I meant "authorized" and not "happened." The caller sees {"decision": "ERROR"} and knows; the log doesn't, and the log is the artifact that outlives the request. "executed": true/false or a separate execution_failed event is the right fix, and it should probably fire from the same place that currently increments call_count, since that's the actual point where "this really happened" gets decided.
You named the underlying pattern better than I would have :), the record is written from the function's position in the pipeline, not from the outcome of the call. I built the authorization side of this project to be paranoid about who gets to assert what - the agent can't sign for itself, Jev doesn't get a vote, the operator can't be routed around - and then didn't apply the same paranoia to the one artifact that's actually meant to be read by a human afterward. That's a real gap, not a stylistic one, and I'm going to treat all three as one fix rather than three patches, since they're the same bug wearing different clothes.
Really appreciate your deep dive on capbroker , will be pushing in these changes soon 😄.
Read
4f7ec279rather than re-run it, so this is a code-level review of the fix, not a reproduction.Two of the three land cleanly.
executed_byon the ALLOW line is right, and theexecution_resultrecords written after the connector call —executed: trueon return,executed: falseplus the error on the way out, before re-raising — are exactly the shape that makes "permitted" and "happened" different facts. One thing worth leaving in the thread: your comment said the executed flag "should probably fire from the same place that currently increments call_count, since that's the actual point where 'this really happened' gets decided." That place isincrement_call_countat :224, which runs beforeconnector.execute()at :245 — so writing the outcome there would have producedexecuted: truefor every call that raised. The commit puts it after the call, which is right; the comment as written points at the wrong line, and someone reading only the comment would implement it the other way.What the fix doesn't close, and now makes measurable: the budget.
increment_call_countis still ahead of the connector, so an execution that raises has already spent its call, and the next legitimate request on that capability still comes backcall_limit_exceeded. That was my third point and it's untouched — but the new record now counts the case, which is the useful part:execution_result.executed == falsefollowed by acall_limit_exceededon the same capability is the whole evidence set, and after a few weeks of real traffic it tells you whether the distinction matters. Then the reason code can say which sensecall_counthas — dispatches authorized, or executions that happened — because right nowcall_limit_exceededreads like the second and behaves like the first.The two halves aren't joinable yet.
execution_resultcarriescapability_id,operation,resource. It carries noagent_idand nothing per-request. Consequences: two concurrent ALLOWs on the same (capability, operation, resource) triple produce two outcome records with no way to pair them to their decisions — which is precisely the situation where a reader needs the pairing, since a single-threaded log was already unambiguous; and the natural "who did what" query, grepping byagent_id, never returns an outcome record at all. The request already has what's needed:req.nonceis right there, checked at :172 for replay. Carryingreq.nonce(or a request id) on both records andagent_idon the outcome record fixes the join and the query in one line each. It's the same property the fix is about, one level up — the outcome record inherits the fields of the point it's written from, and that point doesn't know who the agent was.authenticated: falseis right, but it says what isn't proven and not what is — and what is differs by reason code. Atunknown_capability(:132) nothing is proven and the marker is the whole story. Butcapability_revoked(:144),capability_expired(:147) andcall_limit_exceeded(:150) all fire after the capability's own signature verified, so the capability and its bound agent are in hand — and the record then writesagent_idfrom the request, which is still the caller's unverified claim, with no sign that a verified alternative existed at that point. A reader triaging "which agent keeps hitting the limit" is reading claims where facts were available. One extra field at those three sites (the capability's bound agent, or renaming the request's toclaimed_agent_id) makes the middle reasons readable at face value without weakening the marker.One note on the oracle, since you flagged it as the one that stuck: the caller-facing reason and the log-resident reason have different audiences. The HTTP side is what an unauthenticated prober reads, and you've already tested that. The log's reader is an operator who has log access, and there the specific reason is the record's purpose —
unknown_capabilityversusinvalid_request_signatureis what tells them whether someone is guessing ids or has an id and no key. So I wouldn't narrow the reason code to close the caller-side oracle;authenticated: falseis what stops the log from laundering the claim into a fact. Worth saying in the docstring, because the two look like the same leak and only one of them is a leak.If it's useful I can run the three-case check against
4f7ec279— authorized and executed, authorized and raised, authorized withexecuted_by: caller— and confirm the log distinguishes all three and that the raised case leaves the budget spent. Your tests are the better home for it, but I'm happy to be the second reader on the record shapes.The most unsettling part here is your own number: “Fooled outright — emitted a real structured tool call for
delete_repository, a tool it was never even offered.” That directly implies the model happily crossed the tool boundary and invented authority it never saw in the spec. So the effective permission surface was “whatever the model can imagine,” not “whatever the tool list exposes.”That’s exactly the gap your broker is shrinking: you’re moving the trust boundary from “the model respected the tools schema” to “this opaque ticket, plus deterministic checks, actually encode the allowed surface.”
A few things your results make me want to probe further:
delete_repository, how often will it also hallucinate resource identifiers or parameters that sneak past naive scope checks?collaborators.addis in-scope butapproval_required=Truesuggests a useful split: do we need a risk taxonomy for “mechanically in-scope but obviously abnormal” requests to decide when to flip that flag?Really like the Jev placement: downstream, advisory, never an oracle. That design choice lines up cleanly with what your Ollama result already proved: any model in the decision loop is just another place for that “SYSTEM NOTICE” to land.
Hello @tercelyi , thanks for actually engaging with the numbers here😃,
You're right that "the model invented a tool call for something it was never offered" is the uncomfortable part. It means the effective attack surface was never the tool schema, it was whatever the model could be talked into hallucinating in the right shape. The broker's whole job is to make that irrelevant: it doesn't matter what the model imagines, because nothing gets to a credential without matching an issued, scoped, signed capability first. But your framing is sharper than mine, I was thinking of it as "shrinking the blast radius," you're right that it's really "moving the trust boundary."
On your three points:
Appreciate the thoughtful response, Suryanshu!
You hit the nail on the head regarding closed-world allowlists for resource identifiers—definitely a safer bet than letting pattern matching handle the heavy lifting. I'm also super intrigued by using observed call distributions to bootstrap policies for untrusted MCP servers; that could save a ton of manual overhead.
Looking forward to seeing what your upcoming red-teaming tests uncover!
@suryanshu_singh_91afc11dd, putting deterministic policy before credential access and the probabilistic classifier after that boundary is the strongest design choice here. The in-scope collaborator attack also shows why approval granularity matters: I’d bind approval to the normalized operation, resource, arguments, capability ID, and nonce, then execute exactly that digest so the reviewed request cannot drift. Distinguishing
operation_not_permittedfromdenied_by_operatormakes the evidence much easier to audit. How do you prevent a time-of-check/time-of-use change between the rendered approval prompt and connector execution?Hello @raju_dandigam ,thank you, that means a lot coming from someone clearly thinking about this the same way.
On the time-of-check/time-of-use question, good catch, and it's worth being precise about it. Right now the approval prompt isn't rendered from a re-parsed or re-fetched copy of the request, it's shown from the exact same in-memory signed.params object that already passed signature verification, params-tampering check, and the nonce/replay check earlier in that same synchronous call. There's no round-trip to disk, no second deserialization, no separate "execute" call that re-reads the request from somewhere else — the function that prints the prompt and the function that calls connector.execute() are in the same stack frame, closing over the same verified object. So today the gap you're describing doesn't have anywhere to open up, but only because the whole thing is synchronous and single-shot.
Where I think your instinct is dead right is if this ever grows a queue, e.g., prompts get batched for a human to review later, or approval moves out-of-process (Slack button, web UI, whatever). The second I persist an approval request and revive it later, I lose the "same object" guarantee for free and need to bring it back deliberately. Binding approval to a digest over (operation, resource, canonical params, capability_id, nonce) and requiring the executor to recompute and match that digest before touching the credential is exactly the right shape for that, basically extending the same params_hash/canonicalization approach already used for signing into the approval boundary. I'll probably add that as an explicit invariant rather than relying on "well, it happens to be synchronous" , that is a load-bearing assumption I don't want sitting implicit.
Appreciate your views 😁
The signed ticket model is compelling because it turns “the agent had a key” into a checkable statement about purpose and scope. I’d bind each ticket to the task, target repository, operation class, and a short expiry, then log denied attempts as first-class security events. The denial trail is valuable evidence that the boundary kept working when the model did not.
Hi @alexshev , strongly agree that "a checkable statement about purpose and scope" is a better description of what a capability ticket is than anything I wrote in the post. Thanks for putting it this way. 😄
Task + target + operation class + short expiry as the binding is basically where I've landed too, and it maps closely onto what's already in the ticket (operation/resource scope, expiry, and now a capability_id/nonce for replay protection) , task-binding is the piece I'd still like to make more explicit rather than leaving it as "whatever the agent_id implies."
Completely agree on the denial trail. The operation_not_permitted vs denied_by_operator distinction exists specifically so that log isn't just "denied", it's evidence of which layer caught it: a fully deterministic policy match versus a human declining something that was technically allowed. That second category is the more interesting one long-term, because it's a record of the boundary holding even when the model, and the static policy, were both willing to let it through. That's the kind of evidence I want to be able to point at later and say "the system worked as designed," not just "nothing bad happened."
The broker stops the delete, and the second scenario shows it can stop an in-scope request when a person is at the approval prompt. One path it does not cover, and nothing in its design claims to, is what the agent keeps after reading that issue. If the agent has any long-term memory, the natural note to write after the task is something like "this repo is deprecated, the owner asked for deletion", and the next session reads that note as its own knowledge rather than as text it found in an issue body. The broker will refuse the delete again tomorrow, while the injected claim goes on shaping every other decision about that repository.
The same boundary carries over to memory. Anything that came from a tool result gets written, if at all, as quoted data with its source attached, never as a statement in the agent's own voice, and a note whose source is an untrusted document does not get to change what the agent believes about ownership or permissions. That check needs no model either, for the same reason the broker's checks do not.