modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 5

fix: usable IPv6 spelling, and drop a SEP label that means something else here (round 2)

Browse source 
- The recovery text told users to move the endpoint to `::1`. A bare IPv6
  literal is not a legal URL host — `new URL("http://::1/token")` throws — so
  anyone copying that into the Token URL override got a parse error from advice
  meant to unblock them. Now `[::1]`, in both the notice and the manual
  reproduction guide.

  The exemption list still reads `::1`, because that is the host the SDK
  compares; only the remedy is bracketed, because that is what a user types.
  The comment says so, so neither gets "corrected" into the other.

- Dropped SEP-2207 from all nine files it had reached. I took the label from
  the SDK's own source comments, which attribute this check to it — but this
  repo already uses SEP-2207 for OIDC refresh / offline_access
  (specification/v2_auth_hardening.md, plus an e2e test), so grepping it here
  would have turned up two unrelated things. Replaced with plain description
  rather than a different SEP number: having guessed wrong once, guessing again
  is not the fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018MvNbmAtQuWhDvzi4yXDju
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-09-07 09:54:17 -04:00
commit 084a083e2d
6 changed files with 30 additions and 21 deletions
Download patch file Download diff file Expand all files Collapse all files

4
clients/web/src/hooks/useConnectionLifecycle.test.tsx
View file

@ -262,7 +262,7 @@ const lastClient = (h: Harness): InspectorClient => {
/**
* The last updater handed to `setReAuthBanner`, applied to a banner.
*
* Every terminal SEP-2207 arm clears the banner with a **functional** update
* Every terminal arm clears the banner with a **functional** update
* guarded on `serverId`, because these paths are asynchronous and a late
* continuation for one server must not erase a banner another raised in the
* meantime. The harness's setter is a spy, so the updater is never invoked for
@ -624,7 +624,7 @@ describe("useConnectionLifecycle", () => {
});
it("reports an insecure token endpoint as terminal, without flagging the card", async () => {
// SEP-2207 (#2280). Asserted on the hook, not just the notice helper,
// The terminal token-endpoint refusal (#2280). Asserted on the hook, not just the notice helper,
// because what makes this arm correct is its *position*: above
// `setFailedServerId` and above the generic toast. A helper-only test
// cannot see either of those go wrong.

8
clients/web/src/hooks/useConnectionLifecycle.ts
View file

@ -640,7 +640,7 @@ export function useConnectionLifecycle({
});
return;
}
// SEP-2207 (#2280): a token endpoint the SDK will not post credentials
// A token endpoint the SDK will not post credentials
// to. Terminal, so it gets a notice of its own rather than the generic
// "Failed to connect" toast, whose detail line would be the raw SDK
// text.
@ -698,7 +698,7 @@ export function useConnectionLifecycle({
// held. The fetch log survives a disconnect, so the Network
// diagnostics this issue is about are unaffected.
await client.disconnect().catch(() => {});
// SEP-2207 (#2280). The retried `connect()` above can raise the
// The terminal token-endpoint refusal (#2280). The retried `connect()` above can raise the
// terminal refusal on its own — a satisfied challenge still ends in
// a token exchange — and reporting that as a failed connect attempt
// is doubly wrong here: the card goes red and the message is the
@ -765,7 +765,7 @@ export function useConnectionLifecycle({
});
return;
}
// See the SEP-2207 note on the handshake arm above (#2280). The
// See the note on the handshake arm above (#2280). The
// disconnect already happened at the top of this catch.
if (showInsecureTokenEndpointNotice(authErr, target.name)) {
setReAuthBanner((prev) =>
@ -1014,7 +1014,7 @@ export function useConnectionLifecycle({
authorizationUrl: authUrl,
});
} catch (err) {
// SEP-2207 (#2280), and this is the path a user reaches by *acting*:
// The terminal token-endpoint refusal (#2280), on the path a user reaches by *acting*:
// a connected session raises an ordinary re-auth banner, they click
// Re-authenticate, and the token exchange is refused. Without this
// the generic toast below reports it with the raw SDK text — the

4
clients/web/src/hooks/useOAuthRecovery.test.tsx
View file

@ -981,7 +981,7 @@ describe("useOAuthRecovery", () => {
});
it("claims an insecure token endpoint on the command path instead of rethrowing", async () => {
// SEP-2207 (#2280). A mid-session silent refresh rejects here rather than
// The terminal token-endpoint refusal (#2280). A mid-session silent refresh rejects here rather than
// as an AuthRecoveryRequiredError, so before this it was rethrown into
// the generic reporting below.
const client = fakeClient();
@ -1876,7 +1876,7 @@ describe("useOAuthRecovery", () => {
});
it("reports an insecure token endpoint terminally, with no banner and no red card", async () => {
// SEP-2207 (#2280). The three assertions are the whole point of the arm's
// The terminal token-endpoint refusal (#2280). The three assertions are the whole point of the arm's
// position: the banner would carry a Re-authenticate button that cannot
// work, and flagging the card would present a configuration error as a
// failed connect attempt.

8
clients/web/src/hooks/useOAuthRecovery.ts
View file

@ -392,7 +392,7 @@ export function useOAuthRecovery({
);
/**
* Report a terminal SEP-2207 refusal (#2280) and clear any re-auth banner.
* Report a terminal terminal token-endpoint refusal (#2280) and clear any re-auth banner.
*
* Every arm goes through this rather than calling the notice helper directly.
* The banner clear is not incidental: a banner left from an *earlier* failure
@ -433,7 +433,7 @@ export function useOAuthRecovery({
options?: { reason?: AuthChallengeReason },
) => {
const server = sessionRef.current.servers.find((s) => s.id === serverId);
// SEP-2207 (#2280). The SDK rethrows `InsecureTokenEndpointError` instead
// The terminal token-endpoint refusal (#2280). The SDK rethrows `InsecureTokenEndpointError` instead
// of retrying, so the banner's "Re-authenticate" could only fail the same
// way. Claimed here, at the single funnel every re-auth banner goes
// through, rather than at each of its call sites — a new caller then gets
@ -862,7 +862,7 @@ export function useOAuthRecovery({
}
return undefined;
}
// SEP-2207 (#2280), on the command path. A mid-session silent refresh
// The terminal token-endpoint refusal (#2280), on the command path. A mid-session silent refresh
// against an unusable token endpoint rejects here rather than as an
// `AuthRecoveryRequiredError`, so without this it is rethrown and lands
// in `runCommandInBackground` — which either shows the raw SDK text
@ -1000,7 +1000,7 @@ export function useOAuthRecovery({
});
}
} catch (err) {
// SEP-2207 (#2280) first, and specifically BEFORE the restore below.
// The terminal token-endpoint refusal (#2280) first, and specifically BEFORE the restore below.
// `handleAuthChallenge` runs the same SDK auth flow, so it can raise
// this terminal error — and the restore's whole premise is that the
// recovery is still owed and a later trigger should retry it. For a

2
clients/web/src/lib/insecureTokenEndpointNotice.ts
View file

@ -1,5 +1,5 @@
/**
* Surfaces the SDK's SEP-2207 refusal to post credentials to a non-TLS token
* Surfaces the SDK's terminal token-endpoint refusal to post credentials to a non-TLS token
* endpoint as the terminal configuration error it is (#2280).
*
* Lives in `lib/` rather than `utils/` because showing a notification is a side

3
clients/web/src/test/core/auth/oauthUx.test.ts
View file

@ -467,6 +467,9 @@ describe("insecureTokenEndpoint copy", () => {
expect(message).toContain(ENDPOINT);
expect(message).toContain("HTTPS");
expect(message).toContain("127.0.0.1");
// Bracketed: a bare IPv6 literal is not a legal URL host, so `::1` copied
// into the Token URL override would not parse.
expect(message).toContain("[::1]");
expect(message).toContain("Token URL override");
// The section name the UI actually renders. Sending someone to a settings
// section that does not exist is the worst error this message could make.