Skip to content

Add a subscription narrowing hook and stop sending unrequested change notifications - #3197

Open
maxisbey wants to merge 1 commit into
mainfrom
subscriptions-narrow-hook
Open

Add a subscription narrowing hook and stop sending unrequested change notifications#3197
maxisbey wants to merge 1 commit into
mainfrom
subscriptions-narrow-hook

Conversation

@maxisbey

Copy link
Copy Markdown
Contributor

Two gaps on the 2026-07-28 subscriptions/listen server path, fixed together because either one alone leaves the other's exposure open.

Motivation and Context

No per-caller authorization on subscribe. A client can name any resource URI in its subscriptions/listen filter and the server honors it verbatim — there was no hook to narrow or refuse a subscription per caller, so a caller your resources/read handler would deny could still subscribe to that URI and observe its change events (URIs and activity timing; change notifications carry no content). This adds an optional narrow hook on ListenHandler (and MCPServer(narrow_subscriptions=...)) that runs once, before the acknowledgment, with the request context and the requested filter, and returns the filter to grant — or raises MCPError to refuse the request in-band. The grant is intersected with the request, so a hook can narrow but never widen (the spec's never-send-unrequested-types rule holds by construction), and any other exception fails closed rather than granting. The acknowledgment reports the grant, so clients see what they actually got. Default behavior (no hook) is unchanged.

Unrequested change notifications on modern connections. The modern-era standalone channel (NotifyOnlyOutbound) forwarded every notification, so ctx.session.send_tool_list_changed() on a 2026-07-28 stdio connection wrote a bare, unstamped notifications/tools/list_changed that no subscription had requested — bypassing subscribe-time filtering entirely. That channel now drops the four change-notification methods with a debug log; at this era they reach clients only through listen streams, which stamp and filter them. Publish via ctx.notify_* / the SubscriptionBus.

Docs: the multi-tenant warning on the subscriptions page is replaced with a section on the hook (existence-blind pattern policies, refusal, and the note that the decision holds for the stream's lifetime), plus a migration-guide entry for the notification-routing change.

How Has This Been Tested?

Unit tests for the hook (narrowed ack + delivery, cannot widen, MCPError refusal pre-ack, fail-closed on other exceptions) and for the dropped notification methods. Also driven end to end over real stdio with raw JSON-RPC frames: narrowed ack, only stamped stream frames after a publish (no bare list_changed), in-band refusal, and the fail-closed error all confirmed on the wire.

Breaking Changes

ctx.session.send_tool_list_changed() / send_prompt_list_changed() / send_resource_list_changed() / send_resource_updated() are now dropped (debug-logged) on 2026-07-28 stdio connections instead of being written as bare frames; pre-2026 connections are unaffected, and modern HTTP already had no standalone channel to write to. Migrate to the notify_* bus methods — documented in docs/migration.md.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

The spec is silent on filter authorization; the acknowledgment's honored subset is the only place a server can report narrowing, and the only MUST is against over-delivery, so narrowing is safe unspecified territory. The schema.ts doc comment on the acknowledgment currently frames omission as support-only; a clarification licensing authz-driven narrowing would be worth proposing upstream.

AI Disclaimer

… notifications

Two server-side gaps on the 2026-07-28 subscriptions/listen path, closed together
because either alone leaves the other's leak open.

Nothing let a server decide per caller what a subscription may watch: any client
could name any resource URI in its filter and the server honored it verbatim.
ListenHandler now takes an optional `narrow` hook (`MCPServer(narrow_subscriptions=...)`)
that runs once before the acknowledgment and returns the filter to grant, or
raises MCPError to refuse the request. The grant is intersected with the request
so a hook can narrow but never widen, and any other exception fails closed rather
than granting.

Separately, the modern-era standalone channel forwarded every notification, so
`ctx.session.send_tool_list_changed()` on a 2026-07-28 stdio connection wrote a
bare, unstamped list_changed frame that no subscription had requested. That
channel now drops the four change-notification methods; they reach clients only
through listen streams, which stamp and filter them.
@github-actions

Copy link
Copy Markdown
Contributor

📚 Documentation preview

Preview https://pr-3197.mcp-python-docs.pages.dev
Deployment https://e527fc85.mcp-python-docs.pages.dev
Commit b07b2b3
Triggered by @maxisbey
Updated 2026-07-27 23:19:17 UTC

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 8 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="docs/migration.md">

<violation number="1" location="docs/migration.md:2826">
P2: The three replacement notification calls after `notify_tools_changed()` omit `await`. Copying them into an async handler creates unexecuted coroutines and publishes no change event; show `await` for every `Context.notify_*` call.</violation>
</file>

<file name="src/mcp/server/subscriptions.py">

<violation number="1" location="src/mcp/server/subscriptions.py:233">
P1: A narrowing hook can widen the effective request by mutating the `SubscriptionFilter` it receives. The later intersection uses `params.notifications` again, so an appended URI is treated as client-requested and can be acknowledged and delivered. Passing the hook an independent copy preserves the original request as the authoritative upper bound.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

subscription_id = ctx.request_id
if subscription_id is None:
raise MCPError(INVALID_REQUEST, "subscriptions/listen requires a request id")
granted = await self._grant(ctx, params.notifications)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: A narrowing hook can widen the effective request by mutating the SubscriptionFilter it receives. The later intersection uses params.notifications again, so an appended URI is treated as client-requested and can be acknowledged and delivered. Passing the hook an independent copy preserves the original request as the authoritative upper bound.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/server/subscriptions.py, line 233:

<comment>A narrowing hook can widen the effective request by mutating the `SubscriptionFilter` it receives. The later intersection uses `params.notifications` again, so an appended URI is treated as client-requested and can be acknowledged and delivered. Passing the hook an independent copy preserves the original request as the authoritative upper bound.</comment>

<file context>
@@ -190,9 +230,10 @@ async def __call__(
         subscription_id = ctx.request_id
         if subscription_id is None:
             raise MCPError(INVALID_REQUEST, "subscriptions/listen requires a request id")
+        granted = await self._grant(ctx, params.notifications)
         if len(self._streams) >= self._max_subscriptions:
             raise MCPError(INTERNAL_ERROR, "Subscription limit reached")
</file context>
Suggested change
granted = await self._grant(ctx, params.notifications)
granted = await self._grant(ctx, params.notifications.model_copy(deep=True))

Comment thread docs/migration.md

On a 2026-07-28 connection, `notifications/tools/list_changed`, `notifications/prompts/list_changed`, `notifications/resources/list_changed`, and `notifications/resources/updated` reach a client only through a `subscriptions/listen` stream it opened — the spec forbids sending a notification type a subscription did not request. The v1-style session helpers (`ctx.session.send_tool_list_changed()`, `send_prompt_list_changed()`, `send_resource_list_changed()`, `send_resource_updated(uri)`) push a bare copy onto the connection's standalone channel instead, so on such a connection they are dropped with a debug log: silently on streamable HTTP (there is no standalone channel), and on stdio, where earlier v2 releases wrote the bare notification to the shared pipe, it is now dropped too. On pre-2026 connections the helpers behave as in v1.

Migrate to publishing on the subscription bus, which stamps and filters per stream: `await ctx.notify_tools_changed()`, `notify_prompts_changed()`, `notify_resources_changed()`, and `notify_resource_updated(uri)` on `MCPServer`'s `Context`, or `await bus.publish(...)` on a low-level `Server`'s own `SubscriptionBus` — see [Subscriptions](handlers/subscriptions.md). A stream only ever receives the kinds and URIs the server acknowledged for it; to decide per caller what that is, pass a `narrow_subscriptions` hook (`MCPServer(narrow_subscriptions=...)`, or `ListenHandler(bus, narrow=...)` on the low-level `Server`), covered on the same page.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The three replacement notification calls after notify_tools_changed() omit await. Copying them into an async handler creates unexecuted coroutines and publishes no change event; show await for every Context.notify_* call.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/migration.md, line 2826:

<comment>The three replacement notification calls after `notify_tools_changed()` omit `await`. Copying them into an async handler creates unexecuted coroutines and publishes no change event; show `await` for every `Context.notify_*` call.</comment>

<file context>
@@ -2819,6 +2819,12 @@ async def book_table(date: str, answer: Annotated[Confirmation, Resolve(ask_to_c
+
+On a 2026-07-28 connection, `notifications/tools/list_changed`, `notifications/prompts/list_changed`, `notifications/resources/list_changed`, and `notifications/resources/updated` reach a client only through a `subscriptions/listen` stream it opened — the spec forbids sending a notification type a subscription did not request. The v1-style session helpers (`ctx.session.send_tool_list_changed()`, `send_prompt_list_changed()`, `send_resource_list_changed()`, `send_resource_updated(uri)`) push a bare copy onto the connection's standalone channel instead, so on such a connection they are dropped with a debug log: silently on streamable HTTP (there is no standalone channel), and on stdio, where earlier v2 releases wrote the bare notification to the shared pipe, it is now dropped too. On pre-2026 connections the helpers behave as in v1.
+
+Migrate to publishing on the subscription bus, which stamps and filters per stream: `await ctx.notify_tools_changed()`, `notify_prompts_changed()`, `notify_resources_changed()`, and `notify_resource_updated(uri)` on `MCPServer`'s `Context`, or `await bus.publish(...)` on a low-level `Server`'s own `SubscriptionBus` — see [Subscriptions](handlers/subscriptions.md). A stream only ever receives the kinds and URIs the server acknowledged for it; to decide per caller what that is, pass a `narrow_subscriptions` hook (`MCPServer(narrow_subscriptions=...)`, or `ListenHandler(bus, narrow=...)` on the low-level `Server`), covered on the same page.
+
 ### Servers validate `Mcp-Param-*` headers against the request body ([SEP-2243](https://github.com/modelcontextprotocol/modelcontextprotocol/pull/2243))
</file context>
Suggested change
Migrate to publishing on the subscription bus, which stamps and filters per stream: `await ctx.notify_tools_changed()`, `notify_prompts_changed()`, `notify_resources_changed()`, and `notify_resource_updated(uri)` on `MCPServer`'s `Context`, or `await bus.publish(...)` on a low-level `Server`'s own `SubscriptionBus` — see [Subscriptions](handlers/subscriptions.md). A stream only ever receives the kinds and URIs the server acknowledged for it; to decide per caller what that is, pass a `narrow_subscriptions` hook (`MCPServer(narrow_subscriptions=...)`, or `ListenHandler(bus, narrow=...)` on the low-level `Server`), covered on the same page.
Migrate to publishing on the subscription bus, which stamps and filters per stream: `await ctx.notify_tools_changed()`, `await ctx.notify_prompts_changed()`, `await ctx.notify_resources_changed()`, and `await ctx.notify_resource_updated(uri)` on `MCPServer`'s `Context`, or `await bus.publish(...)` on a low-level `Server`'s own `SubscriptionBus` — see [Subscriptions](handlers/subscriptions.md). A stream only ever receives the kinds and URIs the server acknowledged for it; to decide per caller what that is, pass a `narrow_subscriptions` hook (`MCPServer(narrow_subscriptions=...)`, or `ListenHandler(bus, narrow=...)` on the low-level `Server`), covered on the same page.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beyond the inline aliasing nit, two other candidates were examined and ruled out: (1) the narrow hook running before the max_subscriptions cap check — the request is still rejected pre-ack, so an at-capacity hook invocation is a wasted policy call, not a grant; (2) NotifyOnlyOutbound's method-name drop swallowing stamped listen-stream frames — ListenHandler always sends with related_request_id, which routes through the request-scoped outbound, never this connection-scoped channel.

Extended reasoning...

One nit was found (in-place mutation of the request filter by a widening hook can defeat the intersection); it is posted inline and is defense-in-depth only — no correctly written hook is affected. The two candidates above were verified against the code: the cap check ordering is benign because the INTERNAL_ERROR rejection still precedes the ack, and the stamped-frame concern is impossible because session.send_notification with a related_request_id uses the request-scoped outbound (src/mcp/server/session.py:109), so subscription stream frames never traverse NotifyOnlyOutbound.

Comment on lines +233 to +236
granted = await self._grant(ctx, params.notifications)
if len(self._streams) >= self._max_subscriptions:
raise MCPError(INTERNAL_ERROR, "Subscription limit reached")
honored = _honored_subset(params.notifications)
honored = _honored_subset(params.notifications, granted)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The "can narrow but never widen" guarantee has an aliasing hole: _grant passes the live params.notifications object to the narrow hook (line 233), and _honored_subset then intersects against that same object (line 236), so a hook that widens the filter by in-place mutation (setting a kind flag or appending/rewriting URIs) and returns it makes the intersection a no-op — unrequested kinds and URIs leak into the ack and the delivery filter. Fix is a one-line defensive snapshot, e.g. requested = params.notifications.model_copy(deep=True) before invoking the hook, and passing the snapshot as the "requested" side of _honored_subset.

Extended reasoning...

The bug

ListenHandler.__call__ in src/mcp/server/subscriptions.py does:

granted = await self._grant(ctx, params.notifications)   # line 233 — hook receives the LIVE request filter
...
honored = _honored_subset(params.notifications, granted)  # line 236 — intersects against the SAME object

SubscriptionFilter is an ordinary mutable pydantic model (MCPModel in src/mcp-types/mcp_types/_types.py sets no frozen=True; the config even uses extra='allow'). So a narrow hook that mutates the filter in place and returns it makes the requested and granted arguments of _honored_subset the same (or equally mutated) data, and the intersection (requested.X and granted.X for the kind flags, uri in granted_uris for URIs) passes every added kind and URI straight through.

Step-by-step proof

  1. Client sends subscriptions/listen with SubscriptionFilter(tools_list_changed=True, resource_subscriptions=["r://a"]).
  2. The server's hook is written in the common edit-in-place style, e.g. it canonicalizes URIs or adds a per-tenant default:
    async def narrow(ctx, requested):
        requested.prompts_list_changed = True                # or: rewrites URIs to canonical forms
        requested.resource_subscriptions.append("r://extra")
        return requested
  3. _grant returns that object — which is params.notifications, now mutated.
  4. _honored_subset(params.notifications, granted) computes:
    • prompts_list_changed: requested.prompts_list_changed and granted.prompts_list_changedTrue and True → honored, though the client never asked for it.
    • URIs: [uri for uri in requested.resource_subscriptions if uri in granted_uris]["r://a", "r://extra"] — the never-requested URI survives.
  5. honored feeds both the acknowledgment (the client is told it got things it never asked for) and deliver()'s event_matches filter — the stream now delivers notifications/prompts/list_changed and ResourceUpdated(uri="r://extra"), violating the 2026-07-28 MUST NOT ("the server MUST NOT send notification types the client has not explicitly requested"). Note the SDK client admits any ResourceUpdated once any URI subscription was honored (src/mcp/client/subscriptions.py), so the unrequested events surface to the application.

A verifier reproduced this empirically: the in-place widen yields honored = {tools_list_changed=True, prompts_list_changed=True, resource_subscriptions=['r://a','r://extra']}, while the identical widen via a fresh SubscriptionFilter is correctly blocked.

Why existing safeguards miss it

  • test_narrow_hook_can_only_narrow_never_widen only exercises a hook that returns a fresh filter, where the intersection works. The mutation path is untested.
  • Nothing in the hook's contract forbids mutation — the NarrowSubscription docstring ("Return the filter to grant - typically the request itself, possibly with kinds cleared or resource_subscriptions cut down") actively invites returning an edited request object.
  • The fail-closed machinery in _grant only covers exceptions, not aliasing.

Impact and severity

The hook is trusted, server-authored code, and every correct usage is safe: the no-hook default, the documented model_copy(update=...) example, and even in-place narrowing hooks (a narrowed object intersected with itself is still a subset of the original request). Only a hook that itself widens in place — which already violates its documented contract — escapes. So nothing breaks for correctly written servers; this is a defense-in-depth gap in an advertised "holds by construction" invariant rather than a client-triggerable failure. That is why this is a nit, not merge-blocking.

Fix

One line at the top of __call__: snapshot the request before the hook runs, e.g.

requested = params.notifications.model_copy(deep=True)
granted = await self._grant(ctx, params.notifications)
honored = _honored_subset(requested, granted)

(or snapshot the three flags plus a frozenset of the URIs). With the snapshot as the requested side, no mutation the hook performs can widen the intersection, and the docstring's claim becomes literally true. A companion test with an in-place-widening hook would pin it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant