Skip to content

feat: add URL-mode elicitation support#1423

Merged
olaservo merged 6 commits into
mainfrom
add-url-elicitation
Jun 4, 2026
Merged

feat: add URL-mode elicitation support#1423
olaservo merged 6 commits into
mainfrom
add-url-elicitation

Conversation

@cliffhall
Copy link
Copy Markdown
Member

@cliffhall cliffhall commented Jun 3, 2026

Supersedes #1108 — same changes, but with the branch pushed directly to the modelcontextprotocol/inspector repo so the Claude review workflow can run (it cannot review PRs from forks).

Summary

  • Adds client-side support for URL-mode elicitation (MCP spec 2025-11-25), allowing servers to direct users to external URLs for out-of-band interactions (auth flows, payments, sensitive data entry)
  • Announces both form and url elicitation capabilities during client initialization (elicitation: { form: {}, url: {} })
  • Logs notifications/elicitation/complete notifications in the Inspector's notification panel

Changes

  • ElicitationTab.tsx: Replaced ElicitationRequestData with a discriminated union (ElicitationFormRequestData | ElicitationUrlRequestData)
  • App.tsx: Updated onElicitationRequest handler to branch on request.params.mode and build the appropriate request data shape
  • ElicitationRequest.tsx: Added URL-mode rendering with server message, "Open URL" button that shows a consent dialog displaying the URL as plain text before navigating, a separate "Accept" button that resolves with { action: "accept" } (no content field, per spec), and Decline/Cancel buttons; existing form-mode code untouched
  • useConnection.ts: Declared elicitation: { form: {}, url: {} } capability; imported and registered ElicitationCompleteNotificationSchema in the notification handler array
  • Tests: Added URL-mode tests in ElicitationRequest.test.tsx (8 tests) and ElicitationTab.test.tsx (1 test); updated capability assertion in useConnection.test.tsx

How Has This Been Tested?

Against Everything Server PR 3334

Url Elicitation Tool Entry

url-elicitation-tool-entry

Url Elicitation Tool

url-elicitation-tool

Url Elicitation Request

url-elicitation-request

Url Elicitation Open URL Dialog

url-elicitation-open-dlalog

Url Elicitation Tool Success

url-elicitation-success

Url Elicitation Tool Error Path

url-elicitation-error-path

Test plan

  • npm run build — no type errors
  • cd client && npm run lint — no lint issues
  • cd client && npx jest — 494 tests pass
  • Manual: npm run dev, connect to an MCP server, verify form elicitation still works
  • Manual: verify a URL-mode elicitation renders the message, consent dialog, and buttons correctly

🤖 Generated with Claude Code

cliffhall and others added 5 commits February 23, 2026 13:33
The MCP spec (2025-11-25) added URL-mode elicitation alongside form mode,
allowing servers to direct users to external URLs for out-of-band
interactions (auth flows, payments, sensitive data). This implements
client-side support for rendering and handling URL-mode elicitation
requests, and announces the capability during initialization.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the clickable URL link with a consent dialog that displays the
URL as plain text when "Open URL" is clicked, letting the user confirm
before navigating. Add a separate "Accept" button that resolves the
elicitation without the content field, per the MCP spec.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a tool call returns an MCP error (e.g. -32042 URLElicitationRequired),
the catch block was only capturing error.message (a string), discarding the
structured error.code and error.data fields.

Now McpError instances push the full JSON-RPC error structure to history:
  { error: { code, message, data } }

This means the Response panel will show the complete error including
data.elicitations for -32042, rather than just a message string.
@cliffhall
Copy link
Copy Markdown
Member Author

@claude review

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Jun 3, 2026

Claude finished @cliffhall's task in 2m 35s —— View job


Code Review: URL-mode elicitation support

  • Read changed files
  • Verify MCP spec compliance (SEP-1036, 2025-11-25)
  • Review for bugs, security, and best practices
  • Post review feedback

Nice work — the discriminated-union refactor is clean, form mode is left untouched, the consent dialog is a good touch, and the capability declaration / notification registration line up with SEP‑1036. I checked the implementation against the SDK (@modelcontextprotocol/sdk@1.25.2, which has the mode/url/elicitationId union and ElicitationCompleteNotificationSchema) and against the SEP‑1036 spec text. A few findings, one of which is a security/spec gap worth addressing before merge.


🔴 1. URL scheme is not validated — spec requires HTTPS-only (ElicitationRequest.tsx)

handleConfirmOpen passes the server-supplied url straight to window.open:

const handleConfirmOpen = () => {
  window.open(requestData.url, "_blank", "noopener,noreferrer");
  setShowUrlConfirm(false);
};

SEP‑1036's Security Implications → URL Security Requirements is explicit:

Protocol Restrictions: Only HTTPS URLs are allowed for URL elicitation
Clients must … Validate URLs for SSRF protection

Right now a malicious or buggy server can supply http://, file://, data:, or javascript: and the Inspector will hand it to window.open. The plain-text consent dialog mitigates blind clicks, but the client should refuse to open (or at minimum visibly flag) anything that isn't https:. Suggested approach: parse with new URL(...) in a try/catch, require parsed.protocol === "https:", and if it fails, render an error/warning state instead of the Open button rather than calling window.open.

Fix this →


🟡 2. Display the target domain prominently (ElicitationRequest.tsx)

SEP‑1036: "Clearly display target domains to users." The dialog shows the full URL with break-all, which is good, but for a long URL the actual host can be visually buried — and look-alikes like https://trusted.com@evil.com or https://trusted.com.evil.com/... are exactly the case the spec wants surfaced. Consider also rendering the parsed origin/host on its own emphasized line above the full URL. (Minor / enhancement.)


🟡 3. elicitationId is captured but never correlated

ElicitationCompleteNotificationSchema is registered so completions show up in the notification panel (good for an Inspector), and elicitationId is stored on the pending request — but it's never used to correlate a completion notification back to its request. Per spec this is only a MAY (auto-continue), and the manual Accept button satisfies the SHOULD ("provide a manual way for the user to continue"), so this is acceptable as-is. Flagging only so it's a conscious choice — a short code comment noting that completion is surfaced via the panel and resolved manually would help future readers. Also note the spec's "Clients MUST ignore notifications referencing unknown or already-completed IDs" — currently every such notification is logged unconditionally, which is fine for a debugging tool but technically broader than the spec's MUST-ignore.


🟢 Nits / positives

  • Good fix: the old form path had a latent off-by-one — outer id: nextRequestId.current++ vs inner id: nextRequestId.current read the post-increment value. Both now use requestId. 👍
  • window.open(..., "noopener,noreferrer") — correct hardening. 👍
  • Hooks are called unconditionally before the isUrlMode early return, and useEffect's [isUrlMode, requestData] deps are stable (props-derived object ref), so no re-run loop. Correct.
  • The as ElicitationFormRequestData casts are safe given the runtime guard. Since this is a discriminated union you could narrow on the discriminant instead, but because mode is optional (mode?: "form") on the form variant the cast is a pragmatic choice. Acceptable.
  • UX consideration (subjective): the Accept button resolves { action: "accept" } regardless of whether the user ever opened the URL. Per spec, accept means the user consented to the interaction, so for a testing tool letting you exercise the response independently is reasonable — just confirming it's intentional.

Summary: Solid, spec-aligned implementation. The one item I'd treat as blocking is #1 (HTTPS-only validation) since it's a MUST in SEP‑1036's security requirements; the rest are optional polish.
• branch add-url-elicitation

Address Claude review on #1423:
- Validate the server-supplied URL before opening it; refuse non-https
  schemes (http/file/javascript/data) and malformed URLs, disabling the
  "Open URL" button and showing an error state instead of calling
  window.open (SEP-1036 URL Security Requirements, a spec MUST).
- Display the parsed destination host prominently in the consent dialog
  to surface look-alike/embedded-credential URLs.
- Document that elicitation/complete is surfaced via the notification
  panel and resolved manually (no auto-correlation), a conscious choice
  per the spec's MAY/SHOULD.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cliffhall
Copy link
Copy Markdown
Member Author

Thanks for the thorough review! Addressed in f75bd62.

🔴 1 — HTTPS-only validation (blocking): Fixed. The URL-mode handler now parses requestData.url with new URL() in a try/catch and requires parsed.protocol === "https:". Anything else — http:, file:, javascript:, data:, or a malformed URL — renders a visible error state and disables the "Open URL" button, so window.open is never reached. handleConfirmOpen also passes the parsed href and guards on parsedUrl defensively. Added parameterized tests covering http:, javascript:, file:, data:, and a malformed URL.

🟡 2 — Display target domain prominently: Done. The consent dialog now shows the parsed parsedUrl.host on its own emphasized line above the full URL. Since new URL() resolves the real host, look-alikes like https://trusted.com@evil.com correctly surface evil.com. Added a test asserting the host line.

🟡 3 — elicitationId correlation: Kept as-is (manual resolution via Accept), which is intentional for a debugging tool, but added a code comment at the notification-handler registration documenting that completion is surfaced via the panel and resolved manually — noting the auto-correlate path is a spec MAY and the manual Accept satisfies the SHOULD. Re: the "MUST ignore unknown/already-completed IDs" point — for an Inspector we deliberately log every completion notification unconditionally so nothing is hidden from the user during debugging; happy to revisit if you'd prefer stricter filtering.

🟢 Nits: Thanks for confirming the off-by-one fix, the noopener,noreferrer hardening, and the hooks ordering. The "Accept resolves independently of opening the URL" behavior is intentional for a testing tool, as you noted.

Verification: npm run build-client clean, eslint clean, full client suite 531 tests pass (ElicitationRequest now 24).

@cliffhall
Copy link
Copy Markdown
Member Author

@claude review

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Jun 3, 2026

Claude finished @cliffhall's task in 1m 29s —— View job


Code Review: URL-mode elicitation (round 2)

  • Read updated changed files
  • Verify HTTPS-only fix (Progress notifications #1 from prior review) is correctly implemented
  • Review for bugs, security, and best practices
  • Post review feedback

Re-reviewed the changes in f75bd62b against the prior feedback. The blocking issue (#1) is resolved correctly and the two 🟡 items were addressed well. Looks ready to merge.

#1 — HTTPS-only validation (was blocking): resolved

ElicitationRequest.tsx:52-68 parses the server URL with new URL() in a try/catch and rejects anything where protocol !== "https:", nulling out parsedUrl so the affordance is gone:

  • The Open URL button is disabled={!!urlError} (line 92), so setShowUrlConfirm(true) can't fire → the dialog never opens → window.open is unreachable.
  • handleConfirmOpen independently guards on if (!parsedUrl) return (line 65) — good defense-in-depth.
  • It passes the parsed parsedUrl.href (line 66) rather than the raw string. Correct.
  • The parameterized tests cover http:, javascript:, file:, data:, and a malformed URL, each asserting the error state + disabled button + no window.open. Thorough. 👍

#2 — Display target domain: resolved

The consent dialog now renders parsedUrl.host on its own emphasized line (lines 127-137). Because new URL() resolves the real authority, the look-alike case https://trusted.com@evil.com correctly surfaces evil.com as the destination host — exactly what SEP-1036 asks for. Test added.

#3elicitationId correlation: documented

The code comment at useConnection.ts:756-761 clearly records that completion is surfaced via the panel and resolved manually, noting auto-correlate is a spec MAY and manual Accept satisfies the SHOULD. Good for future readers.

🟢 Notes (all optional, non-blocking)

  • SSRF framing: SEP-1036 lists "validate URLs for SSRF protection," but here the URL is opened with window.open(..., "_blank") — a user-initiated navigation in a separate browser tab, not a server-side fetch — so classic SSRF (blocking localhost/169.254.169.254/internal IPs) doesn't really apply. The https:-only restriction plus the explicit consent dialog is the right guard for a client-side navigation. No action needed; just confirming the choice is sound.
  • Re-parse per render: the new URL() validation runs on every render of a URL-mode request. It's cheap and the component is rarely re-rendered, so not worth memoizing — flagging only for completeness.
  • I did not re-run the suite in this job (fresh checkout, deps not installed), but the diff matches the author's reported 531 passing and the new tests read correctly.

Summary: All prior findings addressed; no new issues. Clean, spec-aligned, and well-covered by tests. 👍
• branch add-url-elicitation

@cliffhall
Copy link
Copy Markdown
Member Author

Thanks for the round-2 pass! 🙏

Agreed on all three notes — no code changes needed:

  • SSRF framing: Good call-out. Since this is a user-initiated window.open(..., "_blank") navigation in a separate tab (not a server-side fetch), classic SSRF guards (localhost/169.254.169.254/internal IPs) don't apply. The https:-only restriction plus the explicit consent dialog showing the resolved host is the appropriate guard for a client-side navigation, so I'll leave it there.
  • Re-parse per render: Intentionally left un-memoized — new URL() is cheap and a pending elicitation re-renders rarely, so a useMemo would add noise for no real benefit.
  • Test run: Confirmed locally — full client suite is 531 passing (ElicitationRequest at 24), build-client and eslint both clean.

Will let a maintainer take it from here for merge.

@olaservo olaservo merged commit 5c782ad into main Jun 4, 2026
7 of 8 checks passed
@olaservo olaservo deleted the add-url-elicitation branch June 4, 2026 01:09
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.

3 participants