Repository navigation
fix(mcp): the authenticated role never reaches MCPDispatcher — every tool call is evaluated as "user" #13821
Description
Activity
Scoped, not started — this is a five-layer change, and the layer everyone assumes exists does not
I picked this up to unblock #13228 stage 3 and stopped after tracing it. Recording the analysis so
whoever takes it does not repeat the search.There is no trusted identity path into the workflow
The obvious approach is "thread the role like
author_idis already threaded". That does not work,
becauseauthor_idstops at message storage:$ grep -rn author_id --include=*.py chat_workflow/ (no output)It reaches
_store_and_log_user_messageinapi/chat.pyand goes no further. So there is no
existing server-side identity arriving inchat_workflow/to hang a role on — the path has to be
built, not extended.LLMIterationContexthas nouser_role, nouser_id, no authenticated identity of any kind. The
only identity-shaped thing it carries isagent_context.agent_id, which comes from
build_governed_identity(context_bag)— and that bag is caller-supplied, which this issue's own
AC rules out as a source.The five layers
api/chat.py—current_userexists here (Depends(get_current_user), ~8 call sites) but
process_chat_messageaccepts onlyauthor_id.process_chat_message→ the workflow manager.ChatWorkflowManager→LLMIterationContextconstruction (manager.py:3418,graph.py:414).LLMIterationContext→ needs auser_rolefield._process_tool_calls(tool_handler.py:3677) →_dispatch_tool_call(:3232).
Layer 5 is one line.
_dispatch_tool_callalready takesrole: str = "user"and already
forwards it todispatcher.dispatch(role=role)(:3429);_process_tool_callsalready hasctx.
The moment the context carries a role, connecting them isrole=ctx.user_role. Everything above it
is the actual work.Two things to get right
- Trusted, not supplied. The value must come from
get_current_user, never from
context["user_role"].build_governed_identity's docstring already warns about this shape for
agent_id: a caller-supplied identity may only add restrictions, never lift them. A role read
from the bag would lift them. - The streaming path is separate.
api/chat.py:908callsprocess_chat_messagefrom inside a
generator with a different argument list than the non-streaming path. Threading one and not the
other yields a role that depends on whether the client asked for streaming.
Consequence for #13228 stage 3
Until this lands,
dispatch()observesrole="user"and nothing else, so stage 2's shadow inventory
has rows for exactly one role. The question stage 3 needs answered — "which working calls would
default-deny break for admin/operator/analyst?" — is unanswerable from it.Stage 3 can still proceed on
ROLE_PERMISSIONSas the source of truth now that #13820 has made it
authoritative; that is a judgement call recorded on #13228, not a blocker.Also worth knowing
This gap means the admin-only MCP gate has never distinguished anybody: an admin is evaluated as
userand denied the redis tools they are entitled to. Over-restrictive rather than permissive, which
is why nobody reported it — and why fixing this will grant access that was previously refused, so it
wants a deliberate look rather than a quiet merge.#13228 stage 3 (the default-deny flip) is gated on this issue. While stage 2 shadow-reports which calls would be denied, every call is evaluated as
"user"per this report — so the inventory stage 3 gets planned from describes the wrong caller population. Planning the flip on it would deny admin-only tools to admins. Raising the coupling here so this is not treated as merely related: it is a hard blocker. Detail on #13228.PR #13981 opened — threads the authenticated role from the chat endpoint through
_process_tool_callstoMCPDispatcher.dispatch().Three of the four ACs are delivered and tested (362 passing; the admin/user pair runs against the real dispatcher gate, and the trust-boundary behaviour is mutation-tested).
AC 4 — "#13228's shadow inventory shows more than one distinct role" — is deliberately not checked. It is an operational observation, not a code change: it needs this merged, code-synced to a running system, and real traffic from more than one role. Noted on #13228 so stage 3 is not planned before that happens.
Delivered by PR #13981, merged as
aa8a39715.Verified in the base branch
$ git log origin/Dev_new_gui --oneline --grep=13821 aa8a39715 fix(mcp): thread the authenticated role to MCPDispatcher (#13821) (#13981) $ git show origin/Dev_new_gui:.../tool_handler.py | grep -c "role=role" -> 4 $ git show origin/Dev_new_gui:.../session_role.py | grep -c apply/resolve -> 2 $ git show origin/Dev_new_gui:.../delegation.py | grep -c auth_role -> 6Acceptance criteria
AC status the authenticated role reaches MCPDispatcher.dispatch()from the live chat pathdelivered test: an admin session reaches an admin-only MCP tool, a user session does not delivered — against the real dispatcher gate, not a stand-in the role comes from the server-side session, never a caller-supplied context key delivered — apply_auth_rolealways writes or strips, so a clientauth_rolenever survives#13228's shadow inventory shows more than one distinct role not met — operational, see below The fourth AC is deliberately unchecked
It needs this merged, code-synced to a running system, and real traffic from more than one role. It is an observation about a live system, not a property of the code, and it cannot be produced from the repo. Noted on #13228 so stage 3 is not planned before it exists.
Closing on the three code ACs. If the intent was that this issue stays open until the inventory is observed, reopen — but the plumbing is delivered either way.
What review changed
The first pass threaded the role through the main chat path only. Review found three more seams still on the
"user"default — the compose shim, the delegation subagent, and the direct/approval endpoint — each silently reproducing this exact bug on its own route. All were fail-safe (denying, never granting), which is why none had surfaced.A second pass caught that changing the delegation engine signature broke a monkeypatched fake in
tests/orchestration/, outside the directory I had swept. The lesson worth keeping: after changing a shared callable signature, sweep by the type, not the directory.Recorded decision: a delegated subagent inherits the delegating user's role. It acts for that user, and defaulting it would deny an admin the tools they are entitled to — the same bug, deferred until delegation ships. The value can only originate from the trusted server-side overlay.
Follow-up filed
#13982 —
send_direct_chat_responsehas no ownership check, so any authenticated user can post a direct/approval response into anychat_id. Found while tracing entrypoints for this fix; pre-existing and out of scope here.- added a commit that references this issue
on Aug 18, 2026
Found while reviewing #13228 stage 2 (PR #13818).
Problem
MCPDispatcher.dispatch()takesrole: str = "user", and in production nothing ever passes it.The default always wins, so every MCP tool call in the chat path is evaluated as
role="user"regardless of who is signed in.
The chain:
chat_workflow/tool_handler.py—_process_tool_callshas noroleparameter, and its call to_dispatch_tool_callomits it, so therole: str = "user"default at the_dispatch_tool_callsignature is what reaches the dispatcher.
chat_workflow/manager.pyandchat_workflow/graph.py— likewise pass norole.
#2629 wired
roleas far as_dispatch_tool_calland stopped there.Impact
Today, with the legacy blocklist: an admin is treated as a
userand is denied the admin-onlyredis tools they are entitled to. Over-restrictive rather than permissive, which is why nobody has
reported it — but it means the admin-only gate has never actually distinguished anybody.
For #13228 stage 3: the shadow inventory that the enforcement flip is meant to be planned from
can only ever contain
role="user"rows. The question "which working calls would default-denybreak for admin / operator / analyst?" is unanswerable from that data.
Proposed approach
Thread the authenticated role from the request context through
_process_tool_callsto_dispatch_tool_call. The session already carries an authenticated identity; this is a plumbinggap, not a missing source of truth. Take care that it is the server-side identity and not a
client-supplied field — see the trust-boundary note in
build_governed_identity.Acceptance criteria
MCPDispatcher.dispatch()from the live chat pathBlocks: #13228 (stage 3)