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

fix: redact secrets nested inside metadata in servers/show

Browse source 
The redaction checked only top-level keys. That was sufficient while values
were strings — a secret could not hide beneath one — but #1910 lets a value be
an object, so `{ trace: { accessToken: "…" } }` printed in full: `trace` is not
sensitive, so the whole subtree went out with it. `servers/show` output is
meant to be safe to paste into an issue, so this is a regression this PR
introduced rather than a pre-existing gap.

`redactMetadataValue` walks the payload: a key matching `isSensitiveHeader`
replaces its entire value (so a sensitive key holding an object cannot leak
through its members either), anything else is descended into, and arrays are
mapped elementwise since objects inside them can carry sensitive keys.
Recursion is bounded — the payload came from `JSON.parse`, so it is a finite
tree with no cycles.

Tests cover a secret two levels under a non-sensitive key and one inside an
array element.

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>
This commit is contained in:
cliffhall 2026-08-23 14:03:40 -04:00
commit 85e4a29990
2 changed files with 54 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

18
clients/cli/__tests__/servers-list.test.ts
View file

@ -226,6 +226,15 @@ describe("showServerEntry / servers/show", () => {
// the whole value, not walk into it (#1910).
"X-Api-Key": { primary: "sk-live-1", fallback: "sk-live-2" },
nested: { keep: true },
// A secret buried under a non-sensitive key. A top-level-only check
// would print `accessToken` in full.
trace: {
id: "t-1",
accessToken: "sk-live-3",
deeper: { refresh_token: "sk-live-4", ok: 1 },
},
// Objects inside an array are reached too.
attempts: [{ password: "hunter2" }, { attempt: 2 }],
},
env: [
{ key: "TOKEN", value: "secret" },
@ -250,8 +259,15 @@ describe("showServerEntry / servers/show", () => {
Authorization: "[redacted]",
"X-Custom": "ok",
"X-Api-Key": "[redacted]",
// A structured value under a non-sensitive key survives intact.
// A structured value under a non-sensitive key survives intact...
nested: { keep: true },
// ...but a sensitive key nested inside one does not.
trace: {
id: "t-1",
accessToken: "[redacted]",
deeper: { refresh_token: "[redacted]", ok: 1 },
},
attempts: [{ password: "[redacted]" }, { attempt: 2 }],
});
expect(sanitized.env).toEqual([
{ key: "TOKEN", value: "[redacted]" },

46
clients/cli/src/handlers/servers-list.ts
View file

@ -157,15 +157,10 @@ export function sanitizeServerSettings(
key: h.key,
value: isSensitiveHeader(h.key) ? REDACTED : h.value,
})),
// Metadata is a JSON object whose values may be any JSON (#1910), so the
// redaction replaces the whole value rather than a string — a sensitive
// key holding an object must not leak through its members.
metadata: Object.fromEntries(
Object.entries(settings.metadata ?? {}).map(([key, value]) => [
key,
isSensitiveHeader(key) ? REDACTED : value,
]),
),
// Metadata values may be any JSON (#1910), so the redaction has to descend:
// a top-level key that is not itself sensitive can hold a nested one that
// is. See `redactMetadataValue`.
metadata: redactMetadataValue(settings.metadata ?? {}),
env: (settings.env ?? []).map((e) => ({
key: e.key,
value: REDACTED,
@ -217,6 +212,39 @@ function sanitizeInitRecord(
return out;
}
/**
* Redact secret-bearing entries anywhere inside a `_meta` payload.
*
* The pre-#1910 version only had to check the top level, because a value was
* always a string — a secret could not hide below one. Now that a value may be
* an object or an array, `{ trace: { accessToken: "…" } }` would sail through a
* top-level-only check: `trace` is not sensitive, so the whole subtree,
* `accessToken` included, would print. `servers/show` output is meant to be
* safe to paste into an issue, so the walk has to reach every key.
*
* A key matching {@link isSensitiveHeader} replaces its **entire** value rather
* than being descended into, so a sensitive key holding an object cannot leak
* through its members either. Arrays are mapped elementwise: their indices are
* not names anyone can mark sensitive, but objects inside them can be.
*
* Recursion is bounded by the payload, which came from `JSON.parse` and so is a
* finite tree with no cycles.
*/
function redactMetadataValue(value: unknown): unknown {
if (Array.isArray(value)) {
return value.map(redactMetadataValue);
}
if (value !== null && typeof value === "object") {
return Object.fromEntries(
Object.entries(value).map(([key, member]) => [
key,
isSensitiveHeader(key) ? REDACTED : redactMetadataValue(member),
]),
);
}
return value;
}
function isSensitiveHeader(key: string): boolean {
const k = key.toLowerCase();
return (