refactor(mcp): replace MCP server standing delegation with session-key registration by aterga · Pull Request #4086 · dfinity/internet-identity

aterga · 2026-07-03T12:26:54Z

Motivation

The mcp_ API mixed two delegation concepts: (1) short-term per-app delegations issued to the MCP server, and (2) a standing delegation for the MCP server itself — a 2-hop chain (canister → ephemeral browser key → server session key) minted at connect time via the generic account-delegation endpoints and form-POSTed to the server. Concept (2) was conceptually unnecessary and had real drawbacks: it was an unrevocable bearer credential (targets: none, up to 1 week) usable at arbitrary canisters, and Settings' toggle-off didn't actually revoke anything (the frontend never called mcp_set_access(false)).

This PR removes the standing delegation entirely. The MCP server's own session-key principal is registered with the backend as a time-boxed, revocable grant, and all server authentication reduces to: look up caller() in the grant map, check it hasn't expired. mcp_prepare_delegation / mcp_get_delegation are now unambiguously the short-term per-app delegation API. On top of that base the PR also (a) hardens the server-facing methods so the only way to reach them is with the registered session key, (b) lets the trusted server finish its own OAuth flow via a finish_url handoff, and (c) threads a session access level (read-only / full) through registration. The MCP feature is in preview, so the legacy index is wiped (abandoned) rather than migrated.

Changes

Backend

New mcp_register(anchor, session_key, grant_ttl_ns, opt permissions) (full auth): binds self_authenticating(session_key) → { anchor, expires_at, permissions } in a new stable grant map (memory 27), TTL clamped to [10 min, 30 days]. Requires an enabled config with a trusted URL, so every grant stays revocable. At most one session per identity — registering replaces the previous grant via a session_principal pointer in StorableMcpConfig (upgrade- and rollback-safe minicbor field, with tests). One key serves one identity; cross-identity reuse of a live key is rejected without leaking the other anchor.
Server-facing methods are gated behind a sealed McpSession capability whose sole constructor is the grant-lookup gate (caller() in the grant map + unexpired + config enabled + trusted URL set + session_principal == caller). mcp_get_accounts / mcp_prepare_delegation / mcp_get_delegation are methods on that capability, so the only way to reach them is as the registered session key's principal — there is no unguarded path.
The session key is fetched by the frontend from the trusted server's callback (origin-attested: only the trusted origin is contacted, and the server only answers for a state it issued). Nothing from the unauthenticated connect link is ever registered, preserving the property that a session only materializes with the trusted server's cooperation — a crafted link binds nothing.
Session access level. mcp_register takes opt Permissions (variant { queries; all }). A queries session makes every per-app delegation it later mints queries-only (the issued Delegation carries permissions = "queries", which the IC rejects for update calls at ingress); all / omitted is full, update-capable access. Fixed for the whole session.
mcp_set_access / mcp_access_enabled removed. Legacy reverse index (memory 25) abandoned per the deprecated-index precedent, de-authorizing all old bindings at upgrade.
mcp_prepare_account_delegation / mcp_get_account_delegation renamed to mcp_prepare_delegation / mcp_get_delegation; per-app cap raised from 5 min to 1 hour and additionally bounded by the grant's absolute expiry (threaded into account_management, and re-checked mid-call so an await can't let a delegation outlive the grant). Auth = the capability gate above; expired grants are lazily cleaned up on the update path.
Revocation: mcp_set_config deletes the grant when MCP is disabled or the trusted URL changes (deliberate exact-string compare — revoking on any edit is the safe direction), atomically in the same message. Ownership is validated so a stale pointer can never delete another identity's session.

Frontend

mcpAuthorize no longer mints anything: fetch {public_key, finish_url?} from the callback with {state}, call mcp_register (with the chosen access level), notify the server with {state, expiration, permissions}. The ephemeral key, 2-hop chain construction, and form-POST are gone; the page owns its terminal screens (no #status redirect-back contract) and the analytics funnel completes in one JS lifetime — the sole exception is the optional finish_url handoff below.
Finish redirect. If the trusted server's key response carries a finish_url, II validates it is on the callback's own origin and, after registering, navigates the consenting tab there — letting the server complete its own flow (e.g. mint an OAuth authorization code and redirect back to an MCP client). Absent / null / "" → II's own close screen. This is the only redirect-back, and only ever to the trusted origin.
Connect-link contract: public_key removed (tolerated-and-ignored), ttl max raised to 30 days; consent picker gains a 30-day preset.
Read-only opt-in behind the READ_ONLY_MODE feature flag (merged from fix(fe): gate read-only access UI behind a feature flag (default off) #4087, default off): while off, the toggle is hidden and the connection is full access, so a queries-only delegation — which unreleased agent stacks don't round-trip — is never sent; the completion POST reports permissions = "all". The connect success screen shows a redirect message when handing the tab to finish_url (merged from Show redirect message on MCP sign-in success screen #4088).
/mcp-only form-action 'self' https: CSP relaxation removed from the asset canister — every route is back on the tight SPA-wide policy (the callback fetches are covered by connect-src).

Protocol / docs

New server-side contract documented in docs/mcp-server-guide.md (CORS JSON callback: key request + completion notification; the finish_url handoff; signed mcp_* calls; read-only sessions; session lifecycle; target_origin derivation caveat). The "serving MCP clients over OAuth" guidance specifies Consent-Bound Completion for /oauth/finish: require the initiator sid cookie and a one-time finish_secret (delivered only to the consenting browser, in finish_url's query) and a proven on-chain grant, which closes the split-browser authorization-code injection (H3) that a cookie alone leaves open. Breaking for deployed servers — needs a lockstep server deploy.

Tests

Integration (tests/integration/mcp.rs, rewritten against the built wasm in PocketIC): happy path + principal parity with the regular account-delegation API, 1-hour cap, delegation never outliving the grant (incl. the mid-call expiry refusal), grant expiry via time-advance, revocation by toggle-off and by URL change (and no resurrection on re-enable), replacement on re-register, registration requiring an enabled config and full auth, cross-identity key-reuse rejection incl. expired-takeover + stale-pointer safety, grant TTL clamping both ends, grant persistence across canister upgrade, server-facing methods rejecting non-session callers (the sealed capability), account-number threading regressions, account listing. Read-only delegations rejected at ingress in tests/integration/delegation_ingress.rs.
Storable unit tests: StorableMcpGrant roundtrip; StorableMcpConfig roundtrip, old-bytes decode (no session_principal), rollback safety. Backend unit suite incl. candid↔.did compatibility.
Asset canister: CSP unit tests + http integration tests updated with the relaxation removed.
Frontend: mcp/utils.test.ts covers access-level wiring, the completion POST's permissions, finish_url validation (same-origin accept; cross-origin / plain-http / opaque-origin / malformed reject; null/"" tolerated), and the two H3 preconditions the server-side fix depends on (P1 — exactly one key request per connect, never auto-retried; P2 — finish_secret never written to a log sink); /mcp load tests updated for the fragment contract incl. legacy public_key tolerance; e2e Playwright covers the finish_url handoff, a finish_secret no-leak assertion across the connect, and read-only-off full access. tsc + svelte-check, eslint, and prettier are clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

Generated by Claude Code

…ion-key registration The /mcp connect flow used to mint a standing delegation chain (canister -> ephemeral browser key -> MCP server session key) and form-POST it to the server, alongside binding an II-derived principal in a reverse index. The standing delegation was an unrevocable bearer credential (targets: none, up to 1 week) and conceptually redundant: being the authorized caller is all the server-facing mcp_* methods need. Now the frontend fetches the server's session public key from the trusted callback (origin-attested: only the trusted origin is contacted, and the server only answers for a state it issued — nothing from the unauthenticated connect link is ever registered) and registers it via the new mcp_register method: a grant `session principal -> (anchor, expiry)` in a new stable map, mirrored by a session_principal pointer in the synced McpConfig. Every server-facing call authorizes by looking up caller()'s grant and checking expiry. - mcp_set_access / mcp_access_enabled removed; mcp_register added - mcp_prepare_account_delegation / mcp_get_account_delegation renamed to mcp_prepare_delegation / mcp_get_delegation; per-app delegation cap raised from 5 minutes to 1 hour, additionally capped by the grant's absolute expiry (threaded into account_management so an await can't drift past it) - grants live 10 min - 30 days (user-chosen, clamped both ends); at most one session per identity — registering replaces the previous grant - revocation: mcp_set_config deletes the grant when MCP is disabled or the trusted URL changes (deliberate exact-string compare), atomically with the config write; caller_grant() re-checks config.enabled as defense in depth; expired grants are lazily cleaned up on the update path - legacy reverse index (memory 25) abandoned per the deprecated-index precedent, de-authorizing all old bindings at upgrade; new grant map at memory 27; StorableMcpConfig gains an optional session_principal field (upgrade- and rollback-safe minicbor evolution, with tests) - /mcp form-action CSP relaxation removed (delivery is fetch-based now); the redirect-back status contract is gone — the page owns its terminal screens, and the funnel completes in one JS lifetime - docs/mcp-server-guide.md documents the new server-side contract Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

Adds a Mermaid sequence diagram covering the full session lifecycle — connect handshake (key fetch + mcp_register), per-app delegation minting, and end-of-life via expiry or config-change revocation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

Reconciles the MCP session-registration refactor (PR #4086) with the read-only-delegations feature (#4016), which independently restructured the same MCP storage and threaded a queries-only `permissions` option through the account-delegation path. Resolution: - Fold #4016's `read_only` onto the session grant: StorableMcpGrant now carries { anchor_number, expires_at_ns, read_only }; the mcp_access storable and its map are dropped. Read-only is a property of the whole session, chosen once at mcp_register and applied to every per-app delegation it mints. - mcp_register gains a `permissions : opt Permissions` argument (derived to a bool on the grant); mcp_prepare_delegation / mcp_get_delegation thread it into DelegationAccess so minted delegations carry permissions="queries". - account_management::prepare_account_delegation keeps both the refactor's max_expiration cap and #4016's access argument. - Grant map moves to memory index 29; indices 25/27/28 abandoned per #4016's convention (preview feature, no migration). - Frontend: mcpAuthorize passes the consent screen's access level to mcp_register; McpAuthorizeView's read-only toggle is preserved. - Ported #4016's delegation_ingress.rs and mcp/utils.test.ts to the registration flow; added a read-only MCP integration test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

The mocked fetch was async with no await (require-await); return a resolved Promise instead. Fixes the frontend-checks CI failure on the merge commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

Copilot

Pull request overview

This PR refactors the /mcp flow to remove the MCP server “standing delegation” mechanism and replace it with explicit registration of a server session-key principal as a revocable, time-boxed grant. This simplifies authentication for server-facing MCP calls while improving revocability and reducing the blast radius of long-lived bearer credentials.

Changes:

Replace standing delegation with mcp_register-backed session grants (stable storage) and grant-based authorization on caller().
Rename MCP delegation endpoints (mcp_prepare_delegation / mcp_get_delegation), cap per-app delegations at 1 hour, and ensure they never outlive the session grant.
Update frontend connect flow and CSP to use callback JSON fetches (key request + completion), and add/refresh protocol documentation and tests.

Reviewed changes

Copilot reviewed 26 out of 28 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/internet_identity/tests/integration/mcp.rs	Rewritten integration coverage for grant-based MCP
src/internet_identity/tests/integration/delegation_ingress.rs	Update ingress tests to new MCP API
src/internet_identity/src/storage/storable/mcp_grant.rs	New stable storable type for MCP grants
src/internet_identity/src/storage/storable/mcp_config.rs	Add `session_principal` forward pointer + tests
src/internet_identity/src/storage/storable/mcp_access.rs	Remove deprecated access-grant storable
src/internet_identity/src/storage/storable.rs	Wire new `mcp_grant` module
src/internet_identity/src/storage.rs	Add stable map for grants; abandon old MCP regions
src/internet_identity/src/mcp.rs	Implement registration + grant-based auth + cleanup
src/internet_identity/src/main.rs	Expose `mcp_register`; rename MCP endpoints
src/internet_identity/src/account_management.rs	Thread absolute expiry cap into delegation prepare
src/internet_identity/internet_identity.did	Update candid: new methods/types, removed legacy
src/internet_identity_interface/src/internet_identity/types.rs	Add `McpRegistration`; update MCP docs/types
src/internet_identity_frontend/src/main.rs	Remove `/mcp` CSP form-action relaxation
src/frontend/tests/e2e-playwright/routes/mcp.spec.ts	Update e2e flow to session-key registration
src/frontend/tests/e2e-playwright/fixtures/mcp.ts	Replace POST-nav fixture with CORS JSON callback
src/frontend/src/routes/(new-styling)/mcp/views/McpErrorView.svelte	Remove redirect-back error terminal screen
src/frontend/src/routes/(new-styling)/mcp/views/McpConnectingView.svelte	Update connecting copy for registration flow
src/frontend/src/routes/(new-styling)/mcp/views/McpAuthorizeView.svelte	Extend TTL presets up to 30 days
src/frontend/src/routes/(new-styling)/mcp/utils.ts	Implement callback key-fetch + `mcp_register`
src/frontend/src/routes/(new-styling)/mcp/utils.test.ts	Update unit tests for new connect behavior
src/frontend/src/routes/(new-styling)/mcp/load.test.ts	Update fragment parsing; tolerate legacy `public_key`
src/frontend/src/routes/(new-styling)/mcp/+page.ts	Remove `public_key`/status parsing; validate callback/state
src/frontend/src/routes/(new-styling)/mcp/+page.svelte	Keep flow on-page; no redirect-back contract
src/frontend/src/lib/utils/analytics/mcpAuthorizeFunnel.ts	Update funnel semantics for single-page completion
src/frontend/src/lib/generated/internet_identity_types.d.ts	Regenerate types for new MCP API surface
src/frontend/src/lib/generated/internet_identity_idl.js	Regenerate IDL factory for new MCP API surface
src/canister_tests/src/api/internet_identity/api_v2.rs	Update test API helpers for `mcp_register` etc.
docs/mcp-server-guide.md	New server protocol documentation (callback + grants)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

A pre-merge comment block still described the session-grant map as living at stable memory index 27 and duplicated the index-25 deprecation marker. The merge moved the grant map to index 29; the accurate abandoned-index block (25/27/28 → 29) already lives next to MCP_GRANT_MEMORY_INDEX. Remove the stale duplicate so stable-memory maintenance can't misread which index is live. Addresses a Copilot review comment on storage.rs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

…ability Make the session-key check the sole, un-bypassable entry point for every server-facing mcp_* method (mcp_get_accounts, mcp_prepare_delegation, mcp_get_delegation). The three operations become methods on a new McpSession capability whose only constructor is `authorize_mcp_session` (renamed from `caller_grant`) and whose grant field is private, so main.rs cannot construct one and must pass the caller check to reach any operation. This turns "reachable only via the registered session key" into a type-level invariant that holds across the endpoint boundary — and one a newly added server-facing method inherits for free — rather than an in-function convention that could be forgotten. Each endpoint now shows its gate explicitly (`authorize_mcp_session()?` for the query methods, `authorize_mcp_session_for_update()?` for the prepare update, which keeps the eager expired-grant cleanup). The candid interface and observable behavior are unchanged. Add an integration test asserting all three server-facing endpoints reject every non-session caller — the identity's own owner principal, an unrelated principal, and an unregistered session key — while a live session is in place, closing the coverage gap for mcp_get_delegation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

…rver-guide / II #4086) (#27) * feat(auth): migrate II connect to session-key registration + device grant Rework the Internet Identity MCP connect flow to the session-key registration model (dfinity/internet-identity#4086), replacing the old standing-delegation-chain flow. Connect handshake (Phase 1): - /oauth/connect/callback now serves II's two cross-origin JSON POSTs: a key request {state} -> {public_key} (a fresh Ed25519 session keypair minted per connection) and a completion notification {state, expiration} -> mark the grant live. No delegation chain is received or verified, and the server never calls mcp_register (II's frontend does). - The connect link carries no key material; ttl is in seconds (3600). Acting for the user (Phase 2): - Sign mcp_get_accounts / mcp_prepare_delegation / mcp_get_delegation directly with the session key; mint per-app delegations to a separate per-app key. Method names per #4086 (were *_account_delegation). - Treat any Unauthorized as "session over -> reconnect" (no retry). OAuth completion: - II's handshake has no redirect back, so model the client login as the OAuth 2.0 Device Authorization Grant (RFC 8628): a device_authorization endpoint + token polling (authorization_pending/slow_down), PKCE bound to the poll. Also: delete src/delegation.rs (self-chain verification no longer needed); management tools act as a stable per-user "management identity" (the user's default account at this server's origin); update README. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014T9N8USDfNK5yzznPGg7Ym * fix(auth): address PR review — PKCE method, callback tolerance, rename - device_authorization: reject code_challenge_method other than S256 (and require code_challenge when a method is named), instead of silently ignoring it and failing with a confusing S256 mismatch at token time. - connect_callback: make the completion notification best-effort — only the key-request POST strictly validates state; a completion POST for an unknown or already-consumed state (e.g. the grant went live via the signed-call fallback before the completion POST arrived) returns 204 instead of 403, and never creates a session for an unknown state. - management.rs: rename standing_agent -> management_agent (the standing delegation concept is gone; it builds from management_identity). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014T9N8USDfNK5yzznPGg7Ym --------- Co-authored-by: Claude <noreply@anthropic.com>

…sh_url The MCP server's key-request response may now carry an optional `finish_url`: after registering the session key and (best effort) POSTing the completion, the II frontend navigates the connecting tab there instead of parking on its own success screen. This is what lets a server complete a browser-based flow of its own around the connect — concretely, the OAuth 2.1 authorization-code flow that real-world MCP clients (claude.ai, Cursor, VS Code) require: the server's finish endpoint mints the authorization code and redirects to the client's redirect_uri. Trust story is unchanged: `finish_url` arrives over the same origin-attested channel as the session key (the trusted server's callback response, never the attacker-craftable connect link) and must be an absolute URL on the callback's own origin — the origin check (scheme included) also excludes plain-http and opaque-origin schemes like javascript:. A response that fails validation fails the connect before anything is registered. `null` and `""` count as absent (how common stacks serialize an omitted optional), not as errors. The page reaches the terminal close screen before handing the tab over, so a non-navigating finish response or a bfcache Back-restore shows the truthful "You're signed in" state rather than a stranded spinner. No canister change. Unit tests cover the validation matrix (same-origin ok, cross-origin / plain-http same-host / javascript: / malformed rejected before mcp_register; null and "" tolerated); the e2e stand-in server can now advertise a finish_url and the new spec drives the full redirect. The server guide documents the field, the ordering caveat (arrival is a UX signal, not proof of registration — check the candid Ok, not transport status), and a new "Serving MCP clients over OAuth" section with the authorize -> II connect -> finish -> token wiring. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

Copilot

Pull request overview

Copilot reviewed 26 out of 28 changed files in this pull request and generated 2 comments.

…-call grant expiry Address two Copilot review findings on the session-authorization path: 1. `authorize_mcp_session` now re-verifies the full config relationship on every call, not just the master toggle: the config must be enabled, still name a trusted server URL, and its forward pointer must name the caller. All write paths already maintain these invariants transactionally (single message, no awaits), so this is behavior-neutral today — it makes the one-session-per-identity invariant enforced on the read path rather than assumed from write-path discipline, so a future regression in a write path can't leave a revoked, disabled, or superseded session usable. 2. `prepare_account_delegation` refuses to sign when the absolute `max_expiration` cap has already passed by the time its internal await resolves (an MCP grant can expire mid-call: the liveness gate runs before the await). Previously this "succeeded" with an already-expired delegation and wasted a signature-map entry; now it returns Unauthorized — exactly the session-over signal the server guide documents. The generic account- delegation path passes no cap and is unaffected. No dedicated integration test: the window only opens when the async path actually yields (e.g. first-call salt initialization), which PocketIC cannot be made to do deterministically from the outside; the guard is covered by the existing suites remaining green (75 tests across mcp/delegation/accounts). No candid or observable-behavior change for well-formed sessions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

…-only, origin derivation) Address gaps the MCP-server design review surfaced that belong in II's normative server guide: - OAuth session-fixation (H3): document that `state` alone can't bind the browser that starts /oauth/authorize to the one that completes /oauth/finish, and require a server-side session cookie across the flow before minting the code. II's consent screen only ever identifies the server origin, never the OAuth client, so this binding is the server's responsibility. - Read-only sessions (H2): state that read-only is the connect *default*; that it disables every update call at ingress including the whole management- canister surface (canister_status/cycles reads are update calls too); and how to detect it (inspect the `permissions` field on a returned SignedDelegation), since the access level is not in the handshake. - Origin derivation / principal parity (H1): document that principals derive from the target_origin domain (modulo II's icp0.io/icp.net -> ic0.app remap; pass a bare https://host) and are NOT guaranteed equal to a browser sign-in when the app uses a custom derivation origin (ii-alternative-origins) the MCP methods can't reveal — with a user-facing explanation and reconcile path. - Device grant (M2): recommend omitting it (no current client uses it; adds a phishing surface with no PKCE binding). - Refresh tokens: make them optional rather than mandated — the grant is the hard revocable ceiling and short per-app delegations already bound exposure. - account_number = null: describe it as the anchor's current (mutable) default at the origin, not a fixed "synthetic" account; reinforce echoing the resolved account_number from prepare into get. Guide-only change; no canister or frontend behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

…on POST The completion notification now carries `permissions` ("queries" | "all") alongside `expiration`, so the MCP server learns whether the session is read-only at connect time instead of having to mint a probe delegation and inspect its `permissions` field. Best-effort like the rest of the completion POST: if it never arrives the server still falls back to reading the level off any minted delegation, so this adds a fast path without a new dependency. - accessLevel.ts: add toPermissionsString (maps the AccessLevel to the wire "queries"/"all", the same vocabulary a delegation's permissions field uses). - mcp/utils.ts: include permissions in the completion POST body. - Guide: document the field in §2b and rewrite the read-only detection guidance to use it (completion POST first, delegation-permissions fallback). - Tests: unit test asserts the completion body's permissions for both levels; e2e fixture surfaces permissions on McpCompletion and the connect spec asserts the default (read-only) session reports "queries". No canister change; the access level was already persisted on the grant via mcp_register's opt permissions argument. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

…-checked toggle bug) The e2e permissions assertion added in 235fe4b caught a real bug: a default MCP connect (read-only) registered a FULL-access grant — the completion POST reported permissions "all", not "queries". Root cause: McpAuthorizeView was the only flow to set AccessLevelToggle's `prompt` equal to its default access level ("read-only"), which makes the checkbox start *checked*. A native checkbox's DOM default is unchecked, and the toggle's function-binding is only sound for an initially-*unchecked* opt-in (as proven by the CLI and authorize flows, which both set `prompt` to the non-default level); an initially-checked toggle didn't hold its value, so the level flowing to `onAuthorize`/`mcp_register` came out full-access despite the box appearing ticked. Fix: match the CLI/authorize pattern — keep the read-only default as the state value and make the opt-in a "Full access" checkbox that starts unchecked (`prompt="full-access"`). Same security default (read-only), no pre-checked box, and it uses the toggle in its supported initially-unchecked shape. The e2e now asserts the "Full access" box starts unchecked (like the CLI flow) and keeps the strict `completion.permissions === "queries"` check, which now holds. No canister change. The shared AccessLevelToggle's inability to represent an initially-checked opt-in is a latent sharp edge; no current flow relies on it after this change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

…p-delegation-refactor-x5b6ek Conflicts were in the two files #4087 and this branch both touched: - McpAuthorizeView.svelte: adopt main's feature-flag gating. The access-level toggle now renders only under `{#if $READ_ONLY_MODE}` (default off) and `effectiveAccessLevel` forces "full-access" while the flag is off — so a connect never sends read-only ("queries") permissions until the flag is on, which is the intended behavior (no released agent round-trips a queries-only delegation yet). This supersedes this branch's prompt="full-access" tweak. Kept this branch's 30-day TTL preset/range (the MCP grant cap is 30 days). - mcp.spec.ts: keep this branch's session-key-model tests ("registers the server's session key", plus the finish_url test) and apply #4087's flag-off expectation — the toggle is hidden (toHaveCount(0)) and the completion POST reports permissions "all". The read-only→"queries" mapping stays covered by the utils unit tests, which drive mcpAuthorize directly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

# Motivation The terminal MCP sign-in success screen (`McpCloseWindowView`) told the user "You can close this window." That framing doesn't fit the flow: after the session key is registered, the tab is either redirected back to the agent (when the trusted server returns a `finish_url`) or it comes to rest on this screen. The copy should reflect what's actually happening in each case. # Changes - `McpCloseWindowView` now takes a `redirecting` prop and shows conditional secondary text: - **Redirect case** (server returned a `finish_url`): `Redirecting back to the agent…` - **Resting case** (no `finish_url`): `You can return to your agent.` — no false promise of a redirect that won't fire. - **Stall fallback**: since `+page.svelte` sets the `close` phase *before* `window.location.assign` (so users aren't stranded on the spinner when the navigation never replaces the document — a 204/attachment `finish_url`, or a bfcache back-navigation), the view starts a 5s timer on mount while redirecting. A successful redirect unmounts the view and clears the timer; if it stalls, the timer reveals a hint: `If nothing happens, you can return to your agent.` - `+page.svelte` sets `redirecting` on the terminal `close` phase from whether `mcpAuthorize` returned a `finish_url`, and passes it to the view. - Updated the component comment to describe the current session-key registration flow (previously it referenced the old delegation-delivery flow and claimed there was no redirect). The CLI close-window screen is intentionally left unchanged: it returns to the terminal via a loopback callback, so "close this window" is still correct there. # Tests Copy/UI-only change. Existing e2e tests assert the unchanged "You're signed in" heading in both the redirect and no-redirect scenarios and continue to pass. Locale catalogs are regenerated by the build's `lingui extract` step, which picks up the new source strings. --------- Co-authored-by: Claude <noreply@anthropic.com>

The server-side H3 fix ("Consent-Bound Completion") relies on two properties of the II connect frontend. They hold today; these tests pin them so a future refactor of the connect flow can't silently regress them: - P1 (single, non-retried key request): assert mcpAuthorize issues exactly one key-request POST per connect, and that a rejected key request aborts without retrying or registering — recovery is a fresh /oauth/authorize (new connect-state), never a retry of the same one. A single-use connect-state makes any retry either brick the user or reopen the attacker race. - P2 (finish_url stays out of logs): finish_url carries the server's one-time finish_secret, so assert mcpAuthorize returns it to the caller (to navigate to) but never writes it to any console sink. Test-only; no behavior change. 14/14 mcp utils tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

The server's key response carries a finish_url whose query holds a one-time finish_secret — the "Consent-Bound Completion" credential the MCP server delivers only to the consenting browser. Property P2 of the H3 mitigation requires II to navigate the tab there and surface that secret nowhere else. The utils unit test pins the mcpAuthorize path, but can't reach the +page.svelte funnel/error surface where a leak to a console or telemetry sink would actually happen. Give the e2e fixture's finish_url a distinctive, greppable secret and expose it as `finishSecret`. Add a spec that observes console output, uncaught page errors, and every outbound request (URL + body) across a full connect, excluding only the legitimate finish navigation, and asserts the secret appears in none of them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

…d Completion An adversarial review found the initiator-cookie mitigation the guide described was only partial: it binds authorize↔finish to one browser but not to whoever performed the II consent, leaving a split-browser authorization-code injection open (attacker initiates and holds the cookie, victim consents, attacker completes /oauth/finish and redeems a token for the victim). Document the full fix instead. /oauth/finish must require two independent proofs — the sid cookie (initiator) and a one-time finish_secret delivered only in the key-request response and carried through in finish_url's query (consenter) — plus a provably registered session (a signed mcp_get_accounts returning Ok, not a bare completion POST). Because connect_state is single-use, both proofs co-reside only in the legitimate same-browser flow. Spell out the II property this relies on (exactly one key request per connect, never auto-retried; recovery is a fresh /oauth/authorize) and the finish_secret leak-proofing (query-only, Referrer-Policy: no-referrer, out of logs). Note the same-browser / hosted-redirect residual is closed separately by hosted-redirect allow-listing (loopback/native are safe either way). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

Copilot

Pull request overview

Copilot reviewed 28 out of 30 changed files in this pull request and generated 1 comment.

…-delegation metrics When the absolute `max_expiration` cap binds — e.g. an MCP session grant near expiry — the minted delegation's lifetime is shorter than the requested `session_duration_ns`, but the PrepareDelegationEvent metric was fed the uncapped `session_duration_ns`, overstating the real signed lifetime. Record `expiration - time()` instead. Without a cap the two are equal, so the regular delegation path's metric is unchanged; only the MCP-capped case is corrected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

parseMcpServerUrl (the https-only callback/origin gate) and isOriginTrusted (the per-identity trust match) decide which servers the /mcp connect flow will talk to, yet neither had a unit test — only incidental e2e coverage. Add focused tests: - mcpServer.test.ts: parseMcpServerUrl accepts and normalizes https (verbatim URL vs. stripped origin, ports, trailing slash, whitespace) and rejects http, http loopback, non-http(s) schemes, and malformed/relative input; probeMcpServer confirms MCP via RFC 9728 metadata (both well-known paths, trailing-slash tolerance, wrong-resource rejection), falls back to the initialize handshake (plain JSON and SSE frame), and fails closed on network/CORS errors. - mcpConfig.test.ts: originOf and isOriginTrusted across enabled/disabled, missing URL, and host/scheme/port/subdomain/unparsable mismatches. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

…ery-gate expiry, CSP) Follows the coverage audit of the II<>MCP integration. Adds tests for paths that were confirmed unexercised: Frontend (vitest): - mcpServer.ts: parseMcpServerUrl userinfo anti-spoof (origin/host reflect the real host, not embedded userinfo); probeMcpServer rejects RFC 9728 metadata lacking an authorization_servers array. - mcp/utils.ts: mcpAuthorize failure paths — key response missing/ non-string public_key (registers nothing), mcp_register Err propagates with no completion POST, and a failing completion POST is swallowed (best-effort) while a finish_url is still returned; non-string finish_url rejected. - mcp/+page.ts load: callback validation — malformed, non-http(s) scheme, http accepted structurally (the https-only gate is the component's), and present-but-empty callback/state. Backend (integration, PocketIC): - mcp_query_methods_reject_expired_grant: expiry is enforced on the pure query gate (get_accounts / get_delegation), not only the update path. - mcp_get_delegation_rejects_mismatched_expiration_and_session_key: get fails closed on any mismatched lookup input, not just the account. - mcp_read_only_grant_stays_queries_only_across_upgrade: the read-only flag persists across a canister upgrade (still mints queries-only after). Asset canister (unit): pin connect-src to exactly `'self' https:` (trailing `;`, so no extra scheme-source slips in) and assert form-action stays `'self' http://127.0.0.1:*` in both dev and prod CSP. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

…d read-only UI Adds the confirmed end-to-end gaps the audit flagged (Playwright; validated via CI): - Connect failure returns to the connect screen: the server rejects the key request, so handleAuthorize's catch surfaces the error and re-opens the connect screen (retry possible) rather than stranding the connecting spinner or reaching the success screen. - Removing the trusted server in Settings (the revoke path the add-only fixture never hit): the server row gives way to the URL input, and a subsequent connect lands on the untrusted screen (trust re-verified at connect time). - Disabling the master toggle blocks connecting while the URL stays saved. - READ_ONLY_MODE on: the connect screen shows the access-level toggle, read-only is the default (opt-out) and reports `permissions="queries"`; unchecking flips `effectiveAccessLevel` to full access (`"all"`). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

zeropath-ai · 2026-07-06T07:14:03Z

✅ No security or compliance issues detected. Reviewed everything up to d6e9a45.

Security Overview

🔎 Scanned files: 32 changed file(s)
🔗 Scan Link: https://zeropath.com/app/repositories/6d3e3b61-710b-4b2b-9c0b-43458b6314b6?scanId=67855ee9-f006-4b04-a997-58c04078ced4&codeScanTypes=PrScan&tab=issues

Detected Code Changes

The diff is too large to display a summary of code changes.

Copilot

Pull request overview

Copilot reviewed 30 out of 32 changed files in this pull request and generated 2 comments.

The connect flow's callback requests followed redirects by default, so a 307/308 from the server would re-POST the body — the single-use `state` (key request) or the completion payload — to the redirected origin. Set `redirect: "error"` so a redirect is a hard failure before anything is registered, and `credentials: "omit"` so no ambient credentials ride these cross-origin calls, matching the fail-closed fetches elsewhere in the repo (e.g. validateDerivationOrigin). Unit test pins both options on both callback requests. Addresses Copilot review comments on the key-request and completion fetches. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

…#4089) The #4089 merge made the /mcp connect flow always show the access-level toggle and default to read-only (opt-out), but left the "Allow access registers the server's session key" e2e asserting the old flagged-off behavior: it expects the toggle checked (correct now) yet still asserts the completion reports `permissions = "all"`. With read-only checked by default and no unchecking, the connect reports `"queries"`, so the assertion (and the desktop e2e shard) fails. Assert the read-only default (`"queries"`) with accurate comments, and reconcile the read-only tests I added: the flag no longer gates /mcp, so drop the now-no-op `enableReadOnlyMode` helper; remove the "With read-only mode enabled" test (now identical to the default-connect assertion above); keep "Unchecking read-only mode connects with full access" as the opt-out (`"all"`) path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf

claude added 4 commits July 3, 2026 12:23

aterga requested a review from Copilot July 3, 2026 16:03

Copilot started reviewing on behalf of aterga July 3, 2026 16:03

View session

Copilot AI reviewed Jul 3, 2026

View reviewed changes

Comment thread src/internet_identity/src/storage.rs Resolved

aterga mentioned this pull request Jul 3, 2026

Migrate II connect to session-key registration + device grant (mcp-server-guide / II #4086) dfinity/imcp2#27

Merged

aterga mentioned this pull request Jul 3, 2026

Restore authorization-code flow via II finish_url redirect (so Claude.ai can connect) dfinity/imcp2#28

Merged

aterga requested a review from Copilot July 4, 2026 17:54

Copilot started reviewing on behalf of aterga July 4, 2026 17:55

View session

Copilot AI reviewed Jul 4, 2026

View reviewed changes

Comment thread src/internet_identity/src/mcp.rs Resolved

Comment thread src/internet_identity/src/account_management.rs Resolved

claude added 4 commits July 4, 2026 18:10

aterga mentioned this pull request Jul 4, 2026

Comply with peer design-review outcomes (H3 Consent-Bound Completion, H2 read-only, H1 derivation, M2 drop device grant) dfinity/imcp2#32

Merged

claude and others added 4 commits July 4, 2026 22:30

aterga mentioned this pull request Jul 5, 2026

Bump ic-agent/ic-transport-types to 0.48.0 and forward read-only delegation permissions dfinity/imcp2#33

Merged

aterga requested a review from Copilot July 5, 2026 11:57

Copilot started reviewing on behalf of aterga July 5, 2026 11:57

View session

Copilot AI reviewed Jul 5, 2026

View reviewed changes

Comment thread src/internet_identity/src/account_management.rs Resolved

aterga mentioned this pull request Jul 5, 2026

/status: fix stale "II /mcp delegation flow enabled" check dfinity/imcp2#38

Merged

claude added 3 commits July 5, 2026 20:06

aterga marked this pull request as ready for review July 6, 2026 07:12

aterga requested a review from a team as a code owner July 6, 2026 07:12

aterga requested a review from Copilot July 6, 2026 07:13

Copilot started reviewing on behalf of aterga July 6, 2026 07:13

View session

Copilot AI reviewed Jul 6, 2026

View reviewed changes

Comment thread src/frontend/src/routes/(new-styling)/mcp/utils.ts Resolved

Comment thread src/frontend/src/routes/(new-styling)/mcp/utils.ts Resolved

aterga requested a review from sea-snake July 6, 2026 14:51

sea-snake approved these changes Jul 6, 2026

View reviewed changes

aterga enabled auto-merge (squash) July 6, 2026 17:09

sea-snake and others added 2 commits July 6, 2026 19:55

Merge branch 'main' into claude/mcp-delegation-refactor-x5b6ek

526429b

aterga merged commit 37670ec into main Jul 6, 2026
46 checks passed

aterga deleted the claude/mcp-delegation-refactor-x5b6ek branch July 6, 2026 18:19

aterga mentioned this pull request Jul 31, 2026

MCP: revoke backend access when a trusted server is removed/disabled #4064

Open

Navigation Menu

Repository navigation

refactor(mcp): replace MCP server standing delegation with session-key registration - #4086

refactor(mcp): replace MCP server standing delegation with session-key registration#4086
aterga merged 23 commits into
mainfrom
claude/mcp-delegation-refactor-x5b6ek

aterga commented Jul 3, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

zeropath-ai Bot commented Jul 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

aterga commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

zeropath-ai Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

aterga commented Jul 3, 2026 •

edited

Loading

zeropath-ai Bot commented Jul 6, 2026 •

edited

Loading