Conversation
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/mcp/server/runner.py">
<violation number="1" location="src/mcp/server/runner.py:231">
P2: Notification handlers in a stateful Streamable HTTP session can still read the session-creating request's token after a later authenticated notification. Scope `push_auth_context_from_request(ctx.request)`/`pop_auth_context()` around `_on_notify`'s middleware call too, so `get_access_token()` matches that notification's HTTP request.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| # including its response envelope. The pipeline never patches it up after | ||
| # the fact. | ||
| result = _dump_result(await call(ctx)) | ||
| auth_token = push_auth_context_from_request(ctx.request) |
There was a problem hiding this comment.
P2: Notification handlers in a stateful Streamable HTTP session can still read the session-creating request's token after a later authenticated notification. Scope push_auth_context_from_request(ctx.request)/pop_auth_context() around _on_notify's middleware call too, so get_access_token() matches that notification's HTTP request.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/server/runner.py, line 231:
<comment>Notification handlers in a stateful Streamable HTTP session can still read the session-creating request's token after a later authenticated notification. Scope `push_auth_context_from_request(ctx.request)`/`pop_auth_context()` around `_on_notify`'s middleware call too, so `get_access_token()` matches that notification's HTTP request.</comment>
<file context>
@@ -227,7 +228,11 @@ async def _inner(ctx: ServerRequestContext[LifespanT, Any]) -> HandlerResult:
# including its response envelope. The pipeline never patches it up after
# the fact.
- result = _dump_result(await call(ctx))
+ auth_token = push_auth_context_from_request(ctx.request)
+ try:
+ result = _dump_result(await call(ctx))
</file context>
|
Thanks for the PR, and sorry it sat here without a proper review. The underlying bug has since been fixed by #2298 and #2710. We're closing most of the open PR backlog. v2 is out and changed a lot of the SDK, so many older PRs no longer apply as written, and we're a small team that realistically doesn't have the capacity to work through the rest. If this still matters to you on v2, the most useful thing you can do is open an issue (or comment on the existing one) with your use case and a repro. Hearing why it matters to you is what we use to decide what to prioritise. |
Fixes #2208.
Stateful Streamable HTTP sessions start the server task group on the first request. Anyio copies the request contextvars into that long-lived task, so get_access_token() could keep returning the token from the session-creating request even when later requests send a different Authorization header.
This change pushes the authenticated user into the auth contextvar at request-dispatch time (using the ServerMessageMetadata.request_context threaded through the transport), and resets it after the handler returns.
Tests: