fix: keep the object branch's __proto__ handling when filtering per key
A regression from the previous commit. The object branch was a spread, which
*defines* properties and is therefore safe; rewriting it into a per-key loop to
filter unserializable values turned it into `out[key] = value`, which *sets* —
so a persisted `"__proto__"` key vanished on reload and, with an object value,
gave the returned metadata a caller-controlled prototype. The legacy branch had
already been fixed for exactly this.
Fixed at the root rather than the site: both branches now go through one
`defineMetadataKey`, so a change to how one stores a key cannot leave the other
behind. That split is what let the earlier fix fail to protect this path.
The test uses `JSON.parse('{"__proto__":…}')`, not an object literal — a
literal `__proto__:` key sets the prototype at construction and creates no own
property, so a test written that way passes against the broken code. Confirmed
by reverting the line: only the parsed case fails.
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:
parent
56d8644032
commit
bc433bbf72
2 changed files with 47 additions and 11 deletions
|
|
@ -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<string, unknown>).polluted).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
it("stores a legacy `__proto__` key as an own property", () => {
|
it("stores a legacy `__proto__` key as an own property", () => {
|
||||||
// Plain assignment would hit the prototype setter: the entry would vanish
|
// Plain assignment would hit the prototype setter: the entry would vanish
|
||||||
// (and an object value would mutate the prototype) instead of being stored.
|
// (and an object value would mutate the prototype) instead of being stored.
|
||||||
|
|
|
||||||
|
|
@ -242,16 +242,7 @@ export function normalizeStoredMetadata(
|
||||||
// migration as `null`. Only a genuinely absent `value` becomes `""`.
|
// migration as `null`. Only a genuinely absent `value` becomes `""`.
|
||||||
const resolved = value === undefined ? "" : value;
|
const resolved = value === undefined ? "" : value;
|
||||||
if (!keepSerializable(key, resolved)) continue;
|
if (!keepSerializable(key, resolved)) continue;
|
||||||
// `defineProperty` rather than `out[key] = …` because a legacy key of
|
defineMetadataKey(out, key, resolved);
|
||||||
// `__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,
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
return out;
|
return out;
|
||||||
}
|
}
|
||||||
|
|
@ -264,7 +255,7 @@ export function normalizeStoredMetadata(
|
||||||
// value does not cost the rest.
|
// value does not cost the rest.
|
||||||
const out: RequestMetadata = {};
|
const out: RequestMetadata = {};
|
||||||
for (const [key, value] of Object.entries(metadata as 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;
|
return out;
|
||||||
}
|
}
|
||||||
|
|
@ -275,6 +266,34 @@ export function normalizeStoredMetadata(
|
||||||
return {};
|
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
|
* Whether a metadata value can be sent as written, warning by key when it
|
||||||
* cannot.
|
* cannot.
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue