From 167b9b4dab49092f928cc42718a8a21e5fe5e930 Mon Sep 17 00:00:00 2001
From: cliffhall <cliff@futurescale.com>
Date: Sun, 23 Aug 2026 17:54:26 -0400
Subject: [PATCH] fix: validate every reserved `_meta` member against the SDK's
 own schema
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`progressToken` was not the only reserved key. The pinned SDK's
`RequestMetaSchema` also constrains `io.modelcontextprotocol/related-task` to
`{ taskId: string }`, and a conforming server rejects the *whole request* over
one bad reserved member rather than ignoring that key. Before #1910 every value
was a string, so only `progressToken` could be wrong; the widening made all of
them reachable.

Replace both hand-rolled checks with the SDK's schemas — `RequestMetaSchema`
for the payload, `ProgressTokenSchema` for the single value. The schema is
loose, so unknown keys pass through untouched, and zod reports the offending
top-level key as `issue.path[0]`, so only that member is dropped and its
siblings survive. A member the SDK constrains in a later release is then
covered without anyone remembering to add it — which is the drift that produced
the `isInteger`/`isSafeInteger` bug one round ago.

`name` was hard-coded, and react-ace uses it as the editor container's DOM id,
so two instances emitted duplicate ids. Derived from the per-instance id, and
deliberately distinct from the id given to the textarea — that one is what the
wrapper's `<label for>` targets, so reusing it would just move the collision.

The docs table, the docs prose and the form hint all claimed "any JSON" without
qualification, presenting a configuration as valid that the client then drops.
All three now name the reserved keys and say what happens to a bad one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0177mPHAdECD18nLLCTwR5rh
Signed-off-by: cliffhall <cliff@futurescale.com>
---
 .../JsonObjectInput/JsonObjectInput.test.tsx  | 20 ++++
 .../JsonObjectInput/JsonObjectInput.tsx       |  6 +-
 .../ServerSettingsForm/ServerSettingsForm.tsx |  7 +-
 .../integration/mcp/inspectorClient.test.ts   | 76 +++++++++++++++
 core/mcp/inspectorClient.ts                   | 92 +++++++++++--------
 docs/mcp-server-configuration.md              | 30 +++---
 6 files changed, 179 insertions(+), 52 deletions(-)

diff --git a/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.test.tsx b/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.test.tsx
index 2d72e026..85643475 100644
--- a/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.test.tsx
+++ b/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.test.tsx
@@ -158,6 +158,26 @@ describe("JsonObjectInput", () => {
       return el;
     }
 
+    it("gives two instances distinct DOM ids", () => {
+      // react-ace uses `name` as the container id; a fixed value would put
+      // duplicate ids in the document as soon as a second editor renders.
+      renderWithMantine(
+        <>
+          <JsonObjectInput ariaLabel="First" value={{}} onChange={() => {}} />
+          <JsonObjectInput ariaLabel="Second" value={{}} onChange={() => {}} />
+        </>,
+      );
+      const containerIds = Array.from(
+        document.querySelectorAll(".ace_editor"),
+      ).map((n) => n.id);
+      const inputIds = Array.from(
+        document.querySelectorAll("textarea.ace_text-input"),
+      ).map((n) => n.id);
+      expect(containerIds).toHaveLength(2);
+      expect(new Set([...containerIds, ...inputIds]).size).toBe(4);
+      expect(containerIds.concat(inputIds).every((id) => id !== "")).toBe(true);
+    });
+
     it("lets the visible label focus the editor", () => {
       renderWithMantine(
         <JsonObjectInput
diff --git a/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.tsx b/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.tsx
index 13056f49..a65b850d 100644
--- a/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.tsx
+++ b/clients/web/src/components/elements/JsonObjectInput/JsonObjectInput.tsx
@@ -187,7 +187,11 @@ export function JsonObjectInput({
       <AceEditor
         mode="json"
         theme={colorScheme === "dark" ? "github_dark" : "github"}
-        name="json-object-input"
+        // react-ace uses `name` as the editor container's DOM id, so a fixed
+        // value would collide the moment two of these render — and it must also
+        // differ from the id given to the textarea below, which is the one the
+        // wrapper's `<label for>` points at.
+        name={`${wrapperId}-editor`}
         value={draft}
         onChange={handleChange}
         width="100%"
diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx
index 4abb41e7..2e03a93b 100644
--- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx
+++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx
@@ -732,7 +732,12 @@ export function ServerSettingsForm({
               Metadata merged into the <Code>_meta</Code> field of every MCP
               request to this server, as a JSON object. Values may be any JSON —
               objects, arrays, numbers, booleans, <Code>null</Code> — not just
-              strings. Leave it as <Code>{"{}"}</Code> to send none.
+              strings. Leave it as <Code>{"{}"}</Code> to send none. The keys
+              the protocol reserves keep their own shape:{" "}
+              <Code>progressToken</Code> must be a string or an integer, and{" "}
+              <Code>io.modelcontextprotocol/related-task</Code> an object with a
+              string <Code>taskId</Code>. A reserved key that does not match is
+              dropped from the request rather than sent.
             </HintText>
             <JsonObjectInput
               ariaLabel="Request metadata JSON"
diff --git a/clients/web/src/test/integration/mcp/inspectorClient.test.ts b/clients/web/src/test/integration/mcp/inspectorClient.test.ts
index 82e1bcf2..8bd2a3bf 100644
--- a/clients/web/src/test/integration/mcp/inspectorClient.test.ts
+++ b/clients/web/src/test/integration/mcp/inspectorClient.test.ts
@@ -1469,6 +1469,82 @@ describe("InspectorClient", () => {
       },
     );
 
+    it.each([
+      ["an array", [1]],
+      ["a string", "t1"],
+      ["an object with a non-string taskId", { taskId: 5 }],
+    ])(
+      "drops a related-task member that is %s, keeping its siblings",
+      async (_label, badRelatedTask) => {
+        // `progressToken` is not the only reserved member: the pinned SDK also
+        // constrains `io.modelcontextprotocol/related-task` to
+        // `{ taskId: string }`. A bad one makes a conforming server reject the
+        // whole request, so it is dropped the same way.
+        client = new InspectorClient(
+          {
+            type: "stdio",
+            command: serverCommand.command,
+            args: serverCommand.args,
+          },
+          {
+            environment: { transport: createTransportNode },
+            progress: false,
+            defaultMetadata: {
+              "io.modelcontextprotocol/related-task": badRelatedTask,
+              keep: "me",
+            },
+          },
+        );
+        const messageLogState = new MessageLogState(client);
+        await client.connect();
+        await client.listTools();
+
+        const req = messageLogState
+          .getMessages()
+          .find(
+            (m) =>
+              m.direction === "request" &&
+              (m.message as { method?: string }).method === "tools/list",
+          );
+        const meta = metaOf(req!);
+        expect(meta).not.toHaveProperty("io.modelcontextprotocol/related-task");
+        expect(meta.keep).toBe("me");
+        messageLogState.destroy();
+      },
+    );
+
+    it("keeps a well-formed related-task member", async () => {
+      client = new InspectorClient(
+        {
+          type: "stdio",
+          command: serverCommand.command,
+          args: serverCommand.args,
+        },
+        {
+          environment: { transport: createTransportNode },
+          progress: false,
+          defaultMetadata: {
+            "io.modelcontextprotocol/related-task": { taskId: "task-1" },
+          },
+        },
+      );
+      const messageLogState = new MessageLogState(client);
+      await client.connect();
+      await client.listTools();
+
+      const req = messageLogState
+        .getMessages()
+        .find(
+          (m) =>
+            m.direction === "request" &&
+            (m.message as { method?: string }).method === "tools/list",
+        );
+      expect(metaOf(req!)["io.modelcontextprotocol/related-task"]).toEqual({
+        taskId: "task-1",
+      });
+      messageLogState.destroy();
+    });
+
     it("warns with the rejected progressToken's type, never its value", async () => {
       // This logger is persisted by real clients (the TUI writes it to
       // `~/.mcp-inspector/auth.log`), and an invalid `progressToken` can now be
diff --git a/core/mcp/inspectorClient.ts b/core/mcp/inspectorClient.ts
index 98290f1e..09c8fa89 100644
--- a/core/mcp/inspectorClient.ts
+++ b/core/mcp/inspectorClient.ts
@@ -1,4 +1,10 @@
 import { Client } from "@modelcontextprotocol/client";
+// The protocol's own schemas for the reserved `_meta` members, so the client
+// validates against the SDK rather than a restatement of it that can drift.
+import {
+  ProgressTokenSchema,
+  RequestMetaSchema,
+} from "@modelcontextprotocol/core";
 import type {
   MCPServerConfig,
   StderrLogEntry,
@@ -382,23 +388,53 @@ function describeJsonType(value: unknown): string {
 /**
  * Whether a `_meta.progressToken` value is one the protocol allows.
  *
- * The spec (and the SDK's `ProgressTokenSchema`,
- * `z.union([z.string(), z.number().int()])`) constrains this reserved member to
- * a string or an **integer** — a float is as invalid as an object. Everything
- * else in `_meta` may be any JSON (#1910), which is exactly why this one key
- * needs its own check.
- *
- * `isSafeInteger`, not `isInteger`: zod 4's `.int()` rejects anything past
- * `Number.MAX_SAFE_INTEGER`, where the float64 representation is no longer the
- * integer it was written as. `Number.isInteger(2 ** 53)` is `true`, so the
- * looser check would pass a token the server then rejects — verified against
- * the pinned zod.
+ * Delegates to the SDK's own `ProgressTokenSchema` rather than restating it.
+ * A hand-rolled check drifts: this was `Number.isInteger` until review pointed
+ * out that zod 4's `.int()` rejects anything past `Number.MAX_SAFE_INTEGER`,
+ * so an unsafe integer passed here and was rejected by the server instead.
  */
 function isProgressToken(value: unknown): value is ProgressToken {
-  return (
-    typeof value === "string" ||
-    (typeof value === "number" && Number.isSafeInteger(value))
-  );
+  return ProgressTokenSchema.safeParse(value).success;
+}
+
+/**
+ * Drop the reserved `_meta` members whose value the protocol will not accept.
+ *
+ * Most of `_meta` is arbitrary JSON (#1910), but a few keys are **reserved**
+ * and carry their own schema — `progressToken` (`string | integer`) and
+ * `io.modelcontextprotocol/related-task` (`{ taskId: string }`) in the pinned
+ * SDK. Before the widening every value was a string, so only `progressToken`
+ * could even be wrong; now any of them can, and one bad reserved member makes a
+ * conforming server reject the **whole request** rather than ignore the key.
+ *
+ * Validated with the SDK's `RequestMetaSchema` instead of a hand-maintained
+ * list, which is the point: the schema is loose, so unknown keys pass through
+ * untouched, and a member the SDK constrains in a later release is covered here
+ * without anyone remembering to add it. Zod reports the offending top-level key
+ * as `issue.path[0]`, so only that member is removed — siblings survive, and
+ * the request stays well-formed because every reserved member is optional.
+ */
+function dropInvalidReservedMeta(
+  meta: RequestMetadata,
+  logger: InspectorLogger,
+): RequestMetadata {
+  const result = RequestMetaSchema.safeParse(meta);
+  if (result.success) return meta;
+
+  const cleaned = { ...meta };
+  for (const issue of result.error.issues) {
+    const key = issue.path[0];
+    if (typeof key !== "string" || !(key in cleaned)) continue;
+    // The key and the reason, never the value: `_meta` can carry credentials,
+    // and this logger is persisted by real clients (the TUI writes it to
+    // `~/.mcp-inspector/auth.log`).
+    logger.warn(
+      { key, received: describeJsonType(cleaned[key]) },
+      "Dropping reserved `_meta` member — it does not match the protocol schema.",
+    );
+    delete cleaned[key];
+  }
+  return cleaned;
 }
 
 const MODERN_RECONNECT_BASE_MS = 500;
@@ -1085,27 +1121,11 @@ export class InspectorClient extends InspectorClientEventTarget {
       ...(logMeta ?? {}),
       ...(callMetadata ?? {}),
     };
-    // `_meta` values are arbitrary JSON (#1910), but `progressToken` is a
-    // reserved member the spec constrains to `string | integer`
-    // (`ProgressTokenSchema = z.union([z.string(), z.number().int()])`). Now
-    // that a value can be an object, an array or a float, a bad one would ride
-    // out on the wire and a conforming server would reject the whole request —
-    // so drop the member rather than send something invalid. It is optional, so
-    // the request stays well-formed and simply asks for no progress. Warn,
-    // because a silently dropped key the user typed is worse than a noisy one.
-    if ("progressToken" in merged && !isProgressToken(merged.progressToken)) {
-      // Log the rejected value's *type*, never the value. `_meta` now carries
-      // arbitrary JSON, so an invalid `progressToken` can be an object holding
-      // credentials — and this logger is persisted by real clients (the TUI
-      // writes it to `~/.mcp-inspector/auth.log`). The type is what makes the
-      // warning actionable anyway; the value would only leak.
-      this.logger.warn(
-        { received: describeJsonType(merged.progressToken) },
-        "Dropping `_meta.progressToken` — expected a string or an integer.",
-      );
-      delete merged.progressToken;
-    }
-    return Object.keys(merged).length > 0 ? merged : undefined;
+    // Reserved members carry their own schemas even though the rest of `_meta`
+    // is arbitrary JSON, and one bad reserved value makes a conforming server
+    // reject the whole request. See `dropInvalidReservedMeta`.
+    const sanitized = dropInvalidReservedMeta(merged, this.logger);
+    return Object.keys(sanitized).length > 0 ? sanitized : undefined;
   }
 
   /**
diff --git a/docs/mcp-server-configuration.md b/docs/mcp-server-configuration.md
index 67f98b12..9005411d 100644
--- a/docs/mcp-server-configuration.md
+++ b/docs/mcp-server-configuration.md
@@ -163,21 +163,21 @@ The file is the familiar MCP client-config shape — an `mcpServers` object keye
 
 These have no analog in the broader `mcp.json` ecosystem. Each is **omitted on write when it equals its default**, so a round-trip through the Inspector keeps the file diff minimal.
 
-| Field                                  | Default    | Meaning                                                                                                                                            |
-| -------------------------------------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `protocolEra`                          | `"legacy"` | `"legacy"` \| `"auto"` \| `"modern"` — which protocol era to negotiate, orthogonal to the transport                                                |
-| `modernLogLevel`                       | `"debug"`  | Per-request log level stamped on modern connections, or `"off"`. Legacy connections ignore it                                                      |
-| `roots`                                | —          | Roots advertised via the `roots` client capability; each is `{ uri, name? }`                                                                       |
-| `metadata`                             | —          | Default `_meta` payload merged into every outgoing request — a JSON object, values may be any JSON. Web and TUI only today; see the CLI note below |
-| `connectionTimeout` / `requestTimeout` | —          | Timeouts in ms                                                                                                                                     |
-| `taskTtl`                              | `60000`    | TTL in ms for tasks created via "Run as task" (`DEFAULT_TASK_TTL_MS`)                                                                              |
-| `autoRefreshOnListChanged`             | `false`    | Refresh lists automatically on `*/list_changed` instead of only flagging the indicator                                                             |
-| `paginatedLists`                       | `false`    | Fetch tools/resources/prompts one page at a time instead of auto-aggregating                                                                       |
-| `advertisedExtensions`                 | —          | Per-extension overrides for what the Inspector declares in `capabilities.extensions`                                                               |
-| `maxFetchRequests`                     | `1000`     | Network-log retention for this server (`DEFAULT_MAX_FETCH_REQUESTS`); `0` means unlimited                                                          |
-| `oauth`                                | —          | `{ clientId, clientSecret, scopes, authorizationParams, authorizationUrl, tokenUrl, enterpriseManaged, onInsufficientScope }`                      |
+| Field                                  | Default    | Meaning                                                                                                                                                                                     |
+| -------------------------------------- | ---------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `protocolEra`                          | `"legacy"` | `"legacy"` \| `"auto"` \| `"modern"` — which protocol era to negotiate, orthogonal to the transport                                                                                         |
+| `modernLogLevel`                       | `"debug"`  | Per-request log level stamped on modern connections, or `"off"`. Legacy connections ignore it                                                                                               |
+| `roots`                                | —          | Roots advertised via the `roots` client capability; each is `{ uri, name? }`                                                                                                                |
+| `metadata`                             | —          | Default `_meta` payload merged into every outgoing request — a JSON object; values may be any JSON except for the reserved keys noted below. Web and TUI only today; see the CLI note below |
+| `connectionTimeout` / `requestTimeout` | —          | Timeouts in ms                                                                                                                                                                              |
+| `taskTtl`                              | `60000`    | TTL in ms for tasks created via "Run as task" (`DEFAULT_TASK_TTL_MS`)                                                                                                                       |
+| `autoRefreshOnListChanged`             | `false`    | Refresh lists automatically on `*/list_changed` instead of only flagging the indicator                                                                                                      |
+| `paginatedLists`                       | `false`    | Fetch tools/resources/prompts one page at a time instead of auto-aggregating                                                                                                                |
+| `advertisedExtensions`                 | —          | Per-extension overrides for what the Inspector declares in `capabilities.extensions`                                                                                                        |
+| `maxFetchRequests`                     | `1000`     | Network-log retention for this server (`DEFAULT_MAX_FETCH_REQUESTS`); `0` means unlimited                                                                                                   |
+| `oauth`                                | —          | `{ clientId, clientSecret, scopes, authorizationParams, authorizationUrl, tokenUrl, enterpriseManaged, onInsufficientScope }`                                                               |
 
-`metadata` is a JSON **object**, and its values may be any JSON — an object, an array, a number, a boolean, `null` — not only a string. Nothing in the MCP spec restricts `_meta` value types, and the SDK models the field as a passthrough object, so the Inspector does not narrow it either ([#1910](https://github.com/modelcontextprotocol/inspector/issues/1910)). Edit it in Server Settings → Request Metadata, which is a JSON editor rather than the key/value rows `headers` and `env` use; text that is not a JSON object is flagged inline and not applied.
+`metadata` is a JSON **object**, and its values may be any JSON — an object, an array, a number, a boolean, `null` — not only a string. The MCP spec does not restrict `_meta` value types in general, and the SDK models the field as a passthrough object, so the Inspector does not narrow it either ([#1910](https://github.com/modelcontextprotocol/inspector/issues/1910)). Edit it in Server Settings → Request Metadata, which is a JSON editor rather than the key/value rows `headers` and `env` use; text that is not a JSON object is flagged inline and not applied.
 
 ```jsonc
 "metadata": {
@@ -187,6 +187,8 @@ These have no analog in the broader `mcp.json` ecosystem. Each is **omitted on w
 }
 ```
 
+> **A few keys are reserved and keep their own shape.** "Any JSON" holds for keys you invent, not for the ones the protocol defines. In the current SDK those are `progressToken` (a string or an integer) and `io.modelcontextprotocol/related-task` (`{ "taskId": "<string>" }`). A reserved key whose value does not match is **dropped from the outgoing request**, with a warning naming the key — because a conforming server rejects the _entire_ request over one bad reserved member, rather than ignoring that key. Everything else in the payload is sent unchanged.
+
 > **Not yet applied by the CLI.** The web client and the TUI both send this payload on every request. The CLI does not — it never passes the persisted value to its client, so a `metadata` entry is currently inert for `--cli` runs ([#2093](https://github.com/modelcontextprotocol/inspector/issues/2093)). Use the per-invocation `--metadata` flag there in the meantime.
 
 > **Reading an older file.** Before #1910 this field was a `[{ "key": …, "value": … }]` pair array. That shape is still **read**, so an existing `mcp.json` keeps working unchanged; it is never written back, so the field is rewritten as an object the next time the Inspector saves the entry.