Skip to content

feat: wire dependency resolution into the MCP-application-proxy mint site - #1948

Open
serguei-gorokhov wants to merge 1 commit into
rd/8-chained-resolutionfrom
rd/9-mcp-mint-site
Open

serguei-gorokhov wants to merge 1 commit into
rd/8-chained-resolutionfrom
rd/9-mcp-mint-site

Conversation

@serguei-gorokhov

@serguei-gorokhov serguei-gorokhov commented Sep 9, 2026

Copy link
Copy Markdown

Applicable issues

Description of changes

ApplicationMcpProxyController's enhancement chain never included ResolveResourceDependenciesFn — an application invoked over /v1/deployments/{id}/mcp got no dependency grants however it declared and however consented it was (a documented v1 limitation). Adding the function to the chain as-is is not a one-line change:

  • The chain is typed over ObjectNode, not RequestObject, and this controller mints its per-request key after the chain runs (inside McpUpstreamAuthInjector, at header-injection time) — a naive wiring would NPE on a null proxyApiKeyData on every call.
  • ResolveResourceDependenciesFn is genericized (<T> extends BaseRequestFunction<T>) so the one implementation joins both RequestObject-typed chains (the three conversation mint sites) and the ObjectNode-typed MCP chain — the request body was never read by this function to begin with. No adapter class.
  • McpProxyController gains a preAssignsPerRequestKey() hook (gated on the app's own mcp.forwardPerRequestKey, default true): when set, the per-request key is created before the enhancement chain runs (so the chain can bake a grant into it) and persisted to Redis only after the chain succeeds — the same create/chain/assign order DeploymentPostController already uses. The body-handling dispatch moves from the Vert.x event loop onto the task executor to match, since the chain now does blocking consent/permission/Redis work.
  • McpUpstreamAuthInjector's key mint is made idempotent: handleProxyRequest re-enters on every 429 retry, connection failure, and same-origin 307/308 redirect (the common Starlette/FastMCP trailing-slash case), and the old unconditional mint would replace the grant-bearing key with a fresh, grant-free one on the very first such re-entry — and orphan the previous Redis-backed key, since only the current key is invalidated on completion. It now reuses an already-assigned key instead.
  • A required dependency with nowhere to bake a grant into (forwardPerRequestKey disabled) is treated exactly like an unconsented one — still hard-fails the call rather than silently succeeding for an app that can never receive the access it declared as required.

Behavior changes flagged for review

  • Mint-timing restructure in a shared controller. Request-handling order and threading change in McpProxyController: key mint moved before the enhancement chain (persisted only after it succeeds), body-handling dispatch moved off the Vert.x event loop onto the task executor, and the injector's mint made idempotent. This PR had the deepest review pass of the series for exactly that reason.
  • Two known edge cases, recorded for the team (not fixed here):
    1. A required dependency combined with forwardPerRequestKey: false is a permanent, unvalidated 403 with no write-time signal — a write-time cross-field check belongs in ResourceDependencyValidator as separate follow-up.
    2. tools/call allow-list enforcement (isToolCallAllowed) is gated on a client-controlled Content-Type header, a pre-existing gap this PR does not introduce but now places a second security-relevant function (this one) behind the same gate; the grant-baking side is fail-closed under that bypass.
  • Shared side effect on the ToolSet MCP proxy: the idempotent-mint change in McpUpstreamAuthInjector also covers the ToolSet path — a ToolSet upstream now reuses its per-request key across 429 retries / connection failures / same-origin redirects instead of minting a fresh one per re-entry. No grant-semantics impact (ToolSet has no resourceDependencies), but the key-lifecycle behavior change is real; the story's idempotency acceptance criterion covers both controllers.

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

🤖 Generated with Claude Code

…site

ApplicationMcpProxyController's enhancement chain never included
ResolveResourceDependenciesFn -- an application invoked over
/v1/deployments/{id}/mcp got no dependency grants however it declared and
however consented it was (documented v1 limitation, P-6). Adding the
function to the chain as-is is not a one-line change: the chain is typed
over ObjectNode, not RequestObject, and this controller mints its
per-request key *after* the chain runs (inside McpUpstreamAuthInjector, at
header-injection time) -- so a naive wiring would NPE on a null
proxyApiKeyData on every call.

ResolveResourceDependenciesFn is genericized (<T> extends
BaseRequestFunction<T>) so the one implementation joins both
RequestObject-typed chains (the three conversation mint sites) and the
ObjectNode-typed MCP chain -- the request body was never read by this
function to begin with.

McpProxyController gains a preAssignsPerRequestKey() hook (gated on the
app's own mcp.forwardPerRequestKey, default true): when set, the
per-request key is created before the enhancement chain runs (so the
chain can bake a grant into it) and persisted to Redis only after the
chain succeeds -- the same create/chain/assign order
DeploymentPostController already uses. The body-handling dispatch moves
from the Vert.x event loop onto the task executor to match, since the
chain now does blocking consent/permission/Redis work.

McpUpstreamAuthInjector's key mint is made idempotent: handleProxyRequest
re-enters on every 429 retry, connection failure, and same-origin 307/308
redirect (the common Starlette/FastMCP trailing-slash case), and the old
unconditional mint would replace the grant-bearing key with a fresh,
grant-free one on the very first such re-entry -- and orphan the previous
Redis-backed key, since only the current key is invalidated on
completion. It now reuses an already-assigned key instead.

A required dependency with nowhere to bake a grant into (forwardPerRequestKey
disabled) is treated exactly like an unconsented one -- still hard-fails
the call rather than silently succeeding for an app that can never
receive the access it declared as required.

Known edge cases, not fixed here (recorded for the team): a required
dependency combined with forwardPerRequestKey disabled is a permanent,
unvalidated 403 with no write-time signal -- a write-time cross-field
check belongs in ResourceDependencyValidator as separate follow-up.
tools/call allow-list enforcement (isToolCallAllowed) is gated on a
client-controlled Content-Type header, a pre-existing gap this PR does
not introduce but now places a second security-relevant function (this
one) behind the same gate; the grant-baking side is fail-closed under
that bypass.

Spec: documentation repo, offline-access-delegation/implementation-specs/pr5b-mcp-mint-site.md

Co-Authored-By: Claude Code <noreply@anthropic.com>
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