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

Merge branch 'v2/main' into v2/feat/2234-skills-extension

Browse source
This commit is contained in:
Cliff Hall 2026-09-05 00:26:12 -04:00 • committed by GitHub
commit 63f89fae10
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 222 additions and 42 deletions
Download patch file Download diff file Expand all files Collapse all files

46
clients/tui/package-lock.json generated
View file

@ -1816,9 +1816,9 @@
}
},
"node_modules/baseline-browser-mapping": {
"version": "2.10.38",
"resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.10.38.tgz",
"integrity": "sha512-31/02mVB4yuQU6adKk5SlY6m+mxDwUq5KZkyYgnLrrKl7TEm1+3PyDtDBz2kOv/wxZz41GHsvV1A/u6RmiyBvw==",
"version": "2.11.21",
"resolved": "https://registry.npmjs.org/baseline-browser-mapping/-/baseline-browser-mapping-2.11.21.tgz",
"integrity": "sha512-uh8vpY/1/YyFkunIDFH/12p7/7VdPKA1hejMVEbdkEaWnUz0Hesvx5EbiU6XxjyHZIOju+ZMbQJkRh+es3/spQ==",
"dev": true,
"license": "Apache-2.0",
"bin": {
@ -1843,9 +1843,9 @@
}
},
"node_modules/browserslist": {
"version": "4.28.2",
"resolved": "https://registry.npmjs.org/browserslist/-/browserslist-4.28.2.tgz",
"integrity": "sha512-48xSriZYYg+8qXna9kwqjIVzuQxi+KYWp2+5nCYnYKPTr0LvD89Jqk2Or5ogxz0NUMfIjhh2lIUX/LyX9B4oIg==",
"version": "4.28.9",
"resolved": "https://registry.npmjs.org/browserslist/-/browserslist-4.28.9.tgz",
"integrity": "sha512-EWazOblFYUvlGZcfGhPUPmYh3nikUxBVb+y9MJun5f3hBi812X+8MSQTujLBtgK3cf51fJWbWfOjyeO954d+Eg==",
"dev": true,
"funding": [
{
@ -1863,11 +1863,11 @@
],
"license": "MIT",
"dependencies": {
"baseline-browser-mapping": "^2.10.12",
"caniuse-lite": "^1.0.30001782",
"electron-to-chromium": "^1.5.328",
"node-releases": "^2.0.36",
"update-browserslist-db": "^1.2.3"
"baseline-browser-mapping": "^2.11.20",
"caniuse-lite": "^1.0.30001810",
"electron-to-chromium": "^1.5.420",
"node-releases": "^2.0.54",
"update-browserslist-db": "^1.3.2"
},
"bin": {
"browserslist": "cli.js"
@ -1903,9 +1903,9 @@
}
},
"node_modules/caniuse-lite": {
"version": "1.0.30001799",
"resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001799.tgz",
"integrity": "sha512-hG1bReV+OUU+MOqK4t/ZWI0tZOyz3rqS9XuhOUz1cIcbwBKjOyJEJuw9ER5JuNyqxNk8u/JUVbGibBOL1yrjFw==",
"version": "1.0.30001810",
"resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001810.tgz",
"integrity": "sha512-TITQPUkaz+aVk5GL6NhOdwk1aEaNTSDPsGFWrTuhKGtjTF70jL/Oht2W4c6rXUe5fu7Ie19VIahAXHIIiWWNeg==",
"dev": true,
"funding": [
{
@ -2099,9 +2099,9 @@
}
},
"node_modules/electron-to-chromium": {
"version": "1.5.376",
"resolved": "https://registry.npmjs.org/electron-to-chromium/-/electron-to-chromium-1.5.376.tgz",
"integrity": "sha512-cUVA7/RvbFTEuw/i3obUwDTRIXojaxkResf+ibByPFxjc6XK3VNtcQXV0NSbAlJ0FMjcJGgftVVB4Qo184EXvA==",
"version": "1.5.422",
"resolved": "https://registry.npmjs.org/electron-to-chromium/-/electron-to-chromium-1.5.422.tgz",
"integrity": "sha512-UvA/32XqrLDdZSn7Jllo1AYNcWji/G0d5M0GTViE7KoGBiMunw3a34Sb2KO4ZZyrSEhqsxFoVhWWJshdyfKqJA==",
"dev": true,
"license": "ISC"
},
@ -3366,9 +3366,9 @@
"peer": true
},
"node_modules/node-releases": {
"version": "2.0.48",
"resolved": "https://registry.npmjs.org/node-releases/-/node-releases-2.0.48.tgz",
"integrity": "sha512-1uz8041X6LoI6ZSdZacM9lVY28vuzDlSKitnpbSNK0RfKoIJkX29NBPVEFXhnuSuEOA9Ww0xnPJ+ILWbGAv8DA==",
"version": "2.0.54",
"resolved": "https://registry.npmjs.org/node-releases/-/node-releases-2.0.54.tgz",
"integrity": "sha512-YHs7BmmcsdAI5Ozuf8JZo6PT0mv2GIWC9vMfvUC3dp65M8hn7Ux8CPL+2oBI7juNuj9d0ndhTcznq2ODBps9cQ==",
"dev": true,
"license": "MIT",
"engines": {
@ -4151,9 +4151,9 @@
"license": "MIT"
},
"node_modules/update-browserslist-db": {
"version": "1.2.3",
"resolved": "https://registry.npmjs.org/update-browserslist-db/-/update-browserslist-db-1.2.3.tgz",
"integrity": "sha512-Js0m9cx+qOgDxo0eMiFGEueWztz+d4+M3rGlmKPT+T4IS/jP4ylw3Nwpu6cpTTP8R1MAC1kF4VbdLt3ARf209w==",
"version": "1.3.2",
"resolved": "https://registry.npmjs.org/update-browserslist-db/-/update-browserslist-db-1.3.2.tgz",
"integrity": "sha512-UQ+MSxlhRm1bzjhU+DcuXfjFO1FzNtqhK5+9Yvlp90ItDLk5vT932A0rFu619nf7RVS+Y/VeaUW1jaRDqZ8VJw==",
"dev": true,
"funding": [
{

68
clients/web/src/test/core/auth/revocation.test.ts
View file

@ -207,6 +207,62 @@ describe("buildRevocationRequest", () => {
expect(body(init).has("client_secret")).toBe(false);
});
// #2222 asked whether `encodeURIComponent` is safe here, since §2.3.1 names
// the form-urlencoded algorithm (RFC 6749 Appendix B) and this is not it. It
// is safe, and these cases are what say so: each credential is checked on the
// wire *and* round-tripped through a real form-urldecoder, which is the
// decoder a compliant authorization server runs. Before this, no case in
// either suite could distinguish the two algorithms — the fixture decoded
// with `decodeURIComponent`, the encoder's own inverse, so every input passed
// by construction.
describe.each([
// The character the report turned on, and the answer to it:
// `encodeURIComponent` escapes `+` as `%2B` rather than leaving it bare, so
// a form-urldecoder never gets the chance to read it as a space. Base64
// secrets contain `+` routinely, which is what makes this the case worth
// pinning rather than reasoning about.
{ what: "a plus", id: "cid", secret: "ab+cd", wire: "cid:ab%2Bcd" },
// A space is the one place the two algorithms visibly differ — `%20` here,
// `+` under `URLSearchParams`. Both decode to a space at a compliant
// server, and only `%20` also survives a server that decodes with
// `decodeURIComponent` alone, which is why the encoder was left as it is.
{ what: "a space", id: "cid", secret: "ab cd", wire: "cid:ab%20cd" },
// The case the original encoding change was made for: an unencoded `:` in
// the id would move the separator and split the credential in the wrong
// place. It must keep working.
{ what: "a colon", id: "c:id", secret: "sec", wire: "c%3Aid:sec" },
// `%` is what makes a raw credential undecodable rather than merely
// mis-decoded — an unescaped one starts an escape sequence that isn't.
{ what: "a percent", id: "cid", secret: "s%ec", wire: "cid:s%25ec" },
])("a credential containing $what", ({ id, secret, wire }) => {
const basic = (): string => {
const { init } = buildRevocationRequest({
endpoint: REVOKE_URL,
token: "r",
tokenTypeHint: "refresh_token",
clientInformation: { client_id: id, client_secret: secret },
supportedAuthMethods: ["client_secret_basic"],
});
return String(headerOf(init, "Authorization")).slice("Basic ".length);
};
it("is percent-encoded on the wire", () => {
expect(atob(basic())).toBe(wire);
});
it("survives a compliant server's form-urldecode", () => {
const decoded = atob(basic());
const separator = decoded.indexOf(":");
// `+` to space *before* percent-decoding, exactly as `test-servers`'
// `/oauth/revoke` now does. Written out here rather than imported so the
// assertion does not lean on the fixture it exists to corroborate.
const formUrlDecode = (v: string): string =>
decodeURIComponent(v.replace(/\+/g, "%20"));
expect(formUrlDecode(decoded.slice(0, separator))).toBe(id);
expect(formUrlDecode(decoded.slice(separator + 1))).toBe(secret);
});
});
it("sends the secret in the body for client_secret_post", () => {
const { init } = buildRevocationRequest({
endpoint: REVOKE_URL,
@ -291,7 +347,16 @@ describe("revokeToken", () => {
// persisted client id or secret and makes `encodeURIComponent` throw. Every
// caller has already cleared its local state by the time this runs, so a
// rejection here would break the documented best-effort guarantee.
//
// The stub is a real `Response` rather than a bare `vi.fn()`: an unencodable
// credential must fail *before* the request goes out, and against a stub that
// returns nothing the assertion would hold either way — reading `.ok` off
// `undefined` throws into the same `catch`. Asserting the fetch was never
// called is what makes this about the encoder (#2222).
it("reports an unencodable credential as failed rather than throwing", async () => {
const fetchFn = vi.fn<typeof fetch>(
async () => new Response(null, { status: 200 }),
);
const outcome = await revokeToken({
endpoint: REVOKE_URL,
token: "r",
@ -302,9 +367,10 @@ describe("revokeToken", () => {
client_secret: `bad${String.fromCharCode(0xd800)}`,
},
supportedAuthMethods: ["client_secret_basic"],
fetchFn: vi.fn<typeof fetch>(),
fetchFn,
});
expect(fetchFn).not.toHaveBeenCalled();
expect(outcome).toMatchObject({ status: "failed", endpoint: REVOKE_URL });
});

85
clients/web/src/test/integration/auth/revocation-e2e.test.ts
View file

@ -39,6 +39,16 @@ const REDIRECT_URL = "http://localhost:3000/oauth/callback";
/** A second registered client, used to prove tokens are not cross-revocable. */
const OTHER_CLIENT_ID = "test-2144-other";
const OTHER_CLIENT_SECRET = "test-2144-other-secret";
/**
* A third client whose secret carries the character #2222 was filed about — a
* `+`, which the two encoding algorithms treat differently and which base64
* secrets (the kind an authorization server most often hands out) contain
* routinely. The Inspector's encoding turns out to handle it, but nothing here
* demonstrated that before, so this client is what makes the claim testable
* rather than argued.
*/
const PLUS_CLIENT_ID = "test-2222-plus";
const PLUS_CLIENT_SECRET = "aG9sZA+bXk/beer=";
function base64Url(buffer: Buffer): string {
return buffer
@ -71,6 +81,11 @@ describe("OAuth token revocation (RFC 7009)", () => {
clientSecret: OTHER_CLIENT_SECRET,
redirectUris: [REDIRECT_URL],
},
{
clientId: PLUS_CLIENT_ID,
clientSecret: PLUS_CLIENT_SECRET,
redirectUris: [REDIRECT_URL],
},
],
}),
});
@ -93,7 +108,10 @@ describe("OAuth token revocation (RFC 7009)", () => {
}, 30_000);
/** Run a real authorization-code exchange and return the issued tokens. */
async function authorize(): Promise<{
async function authorize(
clientId: string = CLIENT_ID,
clientSecret: string = CLIENT_SECRET,
): Promise<{
access_token: string;
refresh_token: string;
}> {
@ -105,7 +123,7 @@ describe("OAuth token revocation (RFC 7009)", () => {
headers: { "Content-Type": "application/x-www-form-urlencoded" },
redirect: "manual",
body: new URLSearchParams({
client_id: CLIENT_ID,
client_id: clientId,
redirect_uri: REDIRECT_URL,
response_type: "code",
scope: "mcp",
@ -125,8 +143,8 @@ describe("OAuth token revocation (RFC 7009)", () => {
grant_type: "authorization_code",
code: code!,
redirect_uri: REDIRECT_URL,
client_id: CLIENT_ID,
client_secret: CLIENT_SECRET,
client_id: clientId,
client_secret: clientSecret,
code_verifier: verifier,
}),
});
@ -146,10 +164,14 @@ describe("OAuth token revocation (RFC 7009)", () => {
return response.status !== 401;
}
async function seededStorage(tokens: {
access_token: string;
refresh_token?: string;
}): Promise<NodeOAuthStorage> {
async function seededStorage(
tokens: {
access_token: string;
refresh_token?: string;
},
clientId: string = CLIENT_ID,
clientSecret: string = CLIENT_SECRET,
): Promise<NodeOAuthStorage> {
const storage = new NodeOAuthStorage(join(storageDir, "oauth.json"));
await storage.clear(serverUrl);
// Issuer-bound and matching the discovered metadata: an unkeyed grant
@ -164,8 +186,8 @@ describe("OAuth token revocation (RFC 7009)", () => {
// server with `oauth.clientId` uses. That is the slot the revocation path
// must read first, or a confidential client sends no authentication at all.
await storage.savePreregisteredClientInformation(serverUrl, {
client_id: CLIENT_ID,
client_secret: CLIENT_SECRET,
client_id: clientId,
client_secret: clientSecret,
});
return storage;
}
@ -269,4 +291,47 @@ describe("OAuth token revocation (RFC 7009)", () => {
status: "revoked",
});
});
// #2222. The fixture used to decode with `decodeURIComponent`, the exact
// inverse of the encoder under test, so the round trip succeeded for every
// input and no case in this file could tell a correct encoder from a wrong
// one. `/oauth/revoke` now runs the form-urldecode a compliant authorization
// server runs, which is what gives this case teeth: it passes only because
// `encodeURIComponent` escapes `+` as `%2B`, and would fail against an
// encoder that emitted a bare one.
it("revokes a grant whose client secret contains a plus", async () => {
const tokens = await authorize(PLUS_CLIENT_ID, PLUS_CLIENT_SECRET);
expect(await tokenAccepted(tokens.access_token)).toBe(true);
const outcome = await clearAndRevoke(
await seededStorage(tokens, PLUS_CLIENT_ID, PLUS_CLIENT_SECRET),
);
expect(outcome).toMatchObject({
status: "revoked",
tokenTypeHint: "refresh_token",
});
expect(await tokenAccepted(tokens.access_token)).toBe(false);
});
// The guard on the fixture itself, and the one case that separates the two
// decoders. An *unencoded* credential is what the SDK's `applyBasicAuth`
// sends, and a compliant server form-urldecodes it anyway — so a `+` in the
// secret comes back as a space and the credential is refused. Under the old
// `decodeURIComponent` fixture this same request was accepted (a string with
// no `%` in it decodes to itself), which is precisely why that fixture could
// not fail on an encoding mistake. Revert `/oauth/revoke` and this test goes
// red.
it("refuses an unencoded Basic credential whose secret contains a plus", async () => {
const raw = `${PLUS_CLIENT_ID}:${PLUS_CLIENT_SECRET}`;
const response = await fetch(`${serverUrl}/oauth/revoke`, {
method: "POST",
headers: {
"Content-Type": "application/x-www-form-urlencoded",
Authorization: `Basic ${Buffer.from(raw).toString("base64")}`,
},
body: new URLSearchParams({ token: "anything" }),
});
expect(response.status).toBe(401);
});
});

20
core/auth/revocation.ts
View file

@ -163,6 +163,26 @@ export function buildRevocationRequest(params: RevocationRequestParams): {
// there is no precedent to match, and the raw form is ambiguous for a
// client id containing `:` and makes `btoa` throw outright on a
// non-Latin-1 secret. Encoding is what the server decodes.
//
// ⚠️ `encodeURIComponent` is **not** the form-urlencoded algorithm §2.3.1
// names (RFC 6749 Appendix B), and #2222 was filed on the assumption that
// the difference breaks a secret containing `+`. It does not, and the
// reasoning is worth keeping because the next reader will have the same
// doubt: `encodeURIComponent` **escapes** `+` as `%2B`. The characters it
// leaves bare are exactly `!'()*-._~` plus alphanumerics, and a
// form-urldecoder passes every one of them through unchanged. It can
// therefore never emit the one character the two algorithms disagree
// about, so its output decodes identically under both — verified over
// every code point up to U+2FFF, and pinned by the round-trip cases in
// `revocation.test.ts`.
//
// Switching to `URLSearchParams` would be the literal algorithm and a
// small *regression*: it encodes a space as `+`, which a compliant server
// reads back as a space but a lenient one — decoding with
// `decodeURIComponent` alone, having never implemented the `+` rule —
// reads as a literal `+`. `%20` is understood by both. So the encoding
// here is the one that survives either server, which matters more than
// matching the spec's wording for a header no server sees twice.
const credentials = `${encodeURIComponent(client.client_id)}:${encodeURIComponent(client.client_secret)}`;
headers.Authorization = `Basic ${base64Encode(credentials)}`;
} else if (method === "client_secret_post" && client.client_secret) {

12
package-lock.json generated
View file

@ -2856,9 +2856,9 @@
"license": "MIT"
},
"node_modules/fast-uri": {
"version": "3.1.5",
"resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.5.tgz",
"integrity": "sha512-gHwA1O9LDIcKunMKhObS/HimwtehO1nPUECKAu5TpKgaO19fcWEl4bliWe1jWxVFvIXztJjjQ4L8XQ1EU9f7Jw==",
"version": "3.1.7",
"resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.7.tgz",
"integrity": "sha512-dOvZVzjdZdz7phd9v6jCbwxrBW3fK6n8Rc0CtdmM4bumzMnxywBYhuph6J819RRw/ku+rLbelwfMunktuzVVHg==",
"funding": [
{
"type": "github",
@ -4383,9 +4383,9 @@
}
},
"node_modules/qs": {
"version": "6.15.3",
"resolved": "https://registry.npmjs.org/qs/-/qs-6.15.3.tgz",
"integrity": "sha512-O9gl3zCl5h5blw1KGUzQKhA5oUXSl8rwUIM5o0S3nCXMliSvy5Dzx7/DJcI+SwgICv+IneSZwhBh1oSyEHA71A==",
"version": "6.16.0",
"resolved": "https://registry.npmjs.org/qs/-/qs-6.16.0.tgz",
"integrity": "sha512-h6fhOIaRrID2CbEY2fqs+7t+UXZo+MLAnU5gRIq85uFtdiUPCdsApMlHhXogKVM4HM2DVbIjGNTTYH2OcmP1vA==",
"license": "BSD-3-Clause",
"dependencies": {
"es-define-property": "^1.0.1",

33
test-servers/src/test-server-oauth.ts
View file

@ -819,8 +819,8 @@ async function authenticateRevocationClient(
// turn into a 500 — so a bad credential would be reported as a server
// fault rather than as the `invalid_client` 401 this endpoint means.
try {
clientId = decodeURIComponent(decoded.slice(0, separator));
clientSecret = decodeURIComponent(decoded.slice(separator + 1));
clientId = formUrlDecode(decoded.slice(0, separator));
clientSecret = formUrlDecode(decoded.slice(separator + 1));
} catch {
return null;
}
@ -841,6 +841,35 @@ async function authenticateRevocationClient(
return ok ? clientId : null;
}
/**
* Decode one half of a Basic credential the way a compliant authorization
* server does — the `application/x-www-form-urlencoded` algorithm RFC 6749
* §2.3.1 names, not `decodeURIComponent`.
*
* The distinction is the whole point of this helper, and #2222 is what it cost
* to learn: this fixture used to decode with `decodeURIComponent`, the exact
* inverse of the encoder it was testing. The round trip then succeeded for
* **every** input — so no test here could have failed on an encoding mistake,
* and the suite's apparent coverage of client authentication was really a
* statement that the encoder is self-consistent. (The encoder was in fact
* fine; that was established by reasoning and a sweep over the code-point
* space, not by anything this fixture asserted, which is the gap being
* closed.)
*
* A form-urldecoder reads a bare `+` as a space, so that substitution happens
* **before** percent-decoding; doing it after would turn a legitimate escaped
* `%2B` into a space too. `%20` still decodes to a space, which is why the
* Inspector's `encodeURIComponent` output — which escapes `+` and spaces both,
* and never emits a bare `+` — round-trips through this decoder unchanged.
*
* `decodeURIComponent` remains the right primitive for the percent half, and it
* still throws on a malformed escape — which the caller catches, keeping a bad
* credential a 401 rather than an Express 500.
*/
function formUrlDecode(value: string): string {
return decodeURIComponent(value.replace(/\+/g, "%20"));
}
/**
* Set up Dynamic Client Registration endpoint
*/