diff --git a/clients/web/src/test/core/mcp/serverList.test.ts b/clients/web/src/test/core/mcp/serverList.test.ts index acca524f..1a0f79cc 100644 --- a/clients/web/src/test/core/mcp/serverList.test.ts +++ b/clients/web/src/test/core/mcp/serverList.test.ts @@ -1432,6 +1432,23 @@ describe("normalizeStoredMetadata (#1910)", () => { }); }); + it.each([ + ["the object form", { __proto__: { polluted: true } } as unknown], + [ + "a hand-edited file's object form", + JSON.parse('{"__proto__":{"polluted":true}}'), + ], + ["the legacy pair form", [{ key: "__proto__", value: { polluted: true } }]], + ])("stores a `__proto__` key from %s as an own property", (_label, input) => { + // Plain assignment hits the prototype setter: the entry vanishes and, for + // an object value, the result gets a caller-controlled prototype. Both + // branches must define rather than set — the object branch regressed once + // when it changed from a spread to a per-key loop. + const out = normalizeStoredMetadata(input); + expect(Object.getPrototypeOf(out)).toBe(Object.prototype); + expect(({} as Record).polluted).toBeUndefined(); + }); + it("stores a legacy `__proto__` key as an own property", () => { // Plain assignment would hit the prototype setter: the entry would vanish // (and an object value would mutate the prototype) instead of being stored. diff --git a/core/mcp/serverList.ts b/core/mcp/serverList.ts index b21d736b..e30a3efd 100644 --- a/core/mcp/serverList.ts +++ b/core/mcp/serverList.ts @@ -242,16 +242,7 @@ export function normalizeStoredMetadata( // migration as `null`. Only a genuinely absent `value` becomes `""`. const resolved = value === undefined ? "" : value; if (!keepSerializable(key, resolved)) continue; - // `defineProperty` rather than `out[key] = …` because a legacy key of - // `__proto__` would otherwise hit the prototype setter — silently - // dropping the entry (or, for an object value, mutating the prototype) - // instead of storing an own property under that name. - Object.defineProperty(out, key, { - value: resolved, - writable: true, - enumerable: true, - configurable: true, - }); + defineMetadataKey(out, key, resolved); } return out; } @@ -264,7 +255,7 @@ export function normalizeStoredMetadata( // value does not cost the rest. const out: RequestMetadata = {}; for (const [key, value] of Object.entries(metadata as RequestMetadata)) { - if (keepSerializable(key, value)) out[key] = value; + if (keepSerializable(key, value)) defineMetadataKey(out, key, value); } return out; } @@ -275,6 +266,34 @@ export function normalizeStoredMetadata( return {}; } +/** + * Store a metadata key as an **own** property. + * + * `out[key] = value` invokes the prototype setter for the key `"__proto__"`, + * so the entry vanishes on reload and — for an object value — the returned + * metadata is given a caller-controlled prototype. `_meta` keys come from a + * file a user (or something that wrote that file) controls, so `"__proto__"` + * is a key that can genuinely arrive. + * + * Both branches of `normalizeStoredMetadata` go through this rather than each + * spelling it out. The object branch was originally a spread — safe, because + * spread defines rather than sets — and rewriting it into a per-key loop for + * value filtering silently reintroduced the hazard the legacy branch had + * already fixed. One mechanism is what stops that recurring. + */ +function defineMetadataKey( + out: RequestMetadata, + key: string, + value: JsonValue, +): void { + Object.defineProperty(out, key, { + value, + writable: true, + enumerable: true, + configurable: true, + }); +} + /** * Whether a metadata value can be sent as written, warning by key when it * cannot.