From f2c311d1b82b874e2369aab1535a393ef472bda4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 23:16:06 +0000 Subject: [PATCH 1/2] fix(signals): removed own __proto__/constructor data reads undefined, not the prototype (#4672) A store whose own `__proto__` (or `constructor`) data was removed told the key's node the backing's inherited value, so the hot read served `Object.prototype` wrapped as a store (or `Object`) to tracked and untracked readers. Core R30: inherited pollution keys are never served. Now the node is told `undefined`, the same answer a store that never had the key gives: - notifyWrites (setter delete, a projection's draft delete): an absent pollution key's node gets `undefined`. An own key pays nothing, because the existing hasOwn result is reused. - reconcile's removed-key branch (reconcile, and a derive's returned value): a pollution key diffs against an empty record. - the get trap's existing pollution-key branch: a tracked reader subscribes like any absent key (R12), and its node is born absent. Own data that lands later reaches the reader (F-10). `__proto__` assignment stays inert (R30). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0117BoWNSSgtemsSYsaX2hXE --- .changeset/proto-key-removal-4672.md | 5 + packages/signals/docs/RULES-INDEX.md | 8 +- packages/signals/src/store/reconcile.ts | 11 +- packages/signals/src/store/store.ts | 26 +- .../store/proto-key-removal-4672.test.ts | 230 ++++++++++++++++++ 5 files changed, 267 insertions(+), 13 deletions(-) create mode 100644 .changeset/proto-key-removal-4672.md create mode 100644 packages/signals/tests/store/proto-key-removal-4672.test.ts diff --git a/.changeset/proto-key-removal-4672.md b/.changeset/proto-key-removal-4672.md new file mode 100644 index 0000000000..031634fec2 --- /dev/null +++ b/.changeset/proto-key-removal-4672.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +A store whose own `__proto__` (or `constructor`) data is removed — by `reconcile`, a setter's `delete`, or a projection — now reads `undefined`, as a store that never had the key does, for tracked and untracked readers alike (#4672, core R30). The key's node used to be told the backing's inherited value, so the read served `Object.prototype` wrapped as a store (or `Object`). A tracked reader of an absent pollution key now subscribes like any absent key (R12), so own data landing later reaches it. `__proto__` assignment stays inert. diff --git a/packages/signals/docs/RULES-INDEX.md b/packages/signals/docs/RULES-INDEX.md index 91ffb76a02..a8771555f6 100644 --- a/packages/signals/docs/RULES-INDEX.md +++ b/packages/signals/docs/RULES-INDEX.md @@ -28,7 +28,7 @@ Status legend: **live** stated and standing · **ruled** carries an explicit rul | C | 4 | 0 | 3 | 1 | | INV | 11 | 4 | 7 | 4 | | RUL | 13 | 3 | 6 | 6 | -| R (CS) | 59 | 15 | 16 | 32 | +| R (CS) | 59 | 15 | 19 | 31 | | R (OL) | 37 | 4 | 1 | 32 | | R (OS) | 46 | 0 | 0 | 46 | | R (PJ) | 37 | 2 | 5 | 32 | @@ -153,7 +153,7 @@ Status legend: **live** stated and standing · **ruled** carries an explicit rul | CS-R9 | live | `docs/rules-mining/core-store.md:42` | store.ts×2 target.ts×1 | — | Proxy identity per logical slot is stable across writes and reconciles** (mapArray keyed flows reuse rows across refetch/reconcile). | | CS-R10 | live | `docs/rules-mining/core-store.md:48` | — | next-smoke.test.ts×1 | Per-property tracking; same-value writes (direct or functional path setter returning prev) do not re-trigger.** | | CS-R11 | live | `docs/rules-mining/core-store.md:52` | — | visibility-oracle-store.states.ts×3 visibility-oracle.states.ts×2 | Per-path tracking: reading `state.user.firstName` subscribes to that leaf; reading the reference `store[0]` does not subscribe to `store[0].i`.** | -| CS-R12 | live | `docs/rules-mining/core-store.md:56` | store.ts×1 | — | Reading an absent key subscribes to that key: other-key changes don't trigger; defining it later (assignment or defineProperty) does.** | +| CS-R12 | live | `docs/rules-mining/core-store.md:56` | store.ts×2 | proto-key-removal-4672.test.ts×1 | Reading an absent key subscribes to that key: other-key changes don't trigger; defining it later (assignment or defineProperty) does.** | | CS-R13 | live | `docs/rules-mining/core-store.md:60` | store.ts×2 target.ts×1 | — | `in` tracks presence, not value: undefined-write doesn't retrigger; delete does; adding absent key does. `in`/`has` never invokes source getters.** | | CS-R14 | live | `docs/rules-mining/core-store.md:64` | — | — | `Object.keys` / `for…in` subscribe to key-set membership (root and nested) — distinct from property nodes.** Aligned: key-set node. | | CS-R15 | live | `docs/rules-mining/core-store.md:68` | store.ts×1 | — | Array structural tracking is uniform across idioms: indexed length loop, `for…of`, mapArray ($TRACK) all re-run exactly once per flush on add/update/removal.** | @@ -167,11 +167,11 @@ Status legend: **live** stated and standing · **ruled** carries an explicit rul | CS-R23 | live | `docs/rules-mining/core-store.md:103` | — | — | The proxy is immutable from outside the setter: direct assignment and delete are silently ignored (no change, no notify, no TypeError — traps report success while discarding).** | | CS-R24 | live | `docs/rules-mining/core-store.md:107` | — | next-smoke.test.ts×1 | Writes batch like signals: inside the setter draft, reads are read-your-writes (values, length, `in` sync); outside the setter, ALL reads — value, `in`, length — return pre-write state until flush(). … | | CS-R25 | live | `docs/rules-mining/core-store.md:112` | — | next-smoke.test.ts×1 | Writes to properties with ZERO observers still batch (no effects anywhere; pre-write value visible between setState and flush).** | -| CS-R26 | live | `docs/rules-mining/core-store.md:117` | — | — | Setting a key to undefined is not deletion: key stays present (`in` true, no key-set notify); only delete / storePath.DELETE removes.** | +| CS-R26 | live | `docs/rules-mining/core-store.md:117` | — | proto-key-removal-4672.test.ts×1 | Setting a key to undefined is not deletion: key stays present (`in` true, no key-set notify); only delete / storePath.DELETE removes.** | | CS-R27 | live | `docs/rules-mining/core-store.md:121` | store.ts×2 | — | The setter may return a replacement value that swaps the root wholesale; symbol keys on the replacement preserved.** | | CS-R28 | live | `docs/rules-mining/core-store.md:125` | reconcile.ts×1 | — | `storePath` addressing: string keys, numeric indices, index arrays, predicate filters ((value, index)), ranges, trailing functional setters address and update intended paths + trigger per-path subscri… | | CS-R29 | live | `docs/rules-mining/core-store.md:129` | store.ts×1 | overlay.test.ts×1 | Merge/replacement preserve accessor descriptors and keep getters LIVE (re-evaluated per read, reactive reads track), for pre-existing and new keys.** | -| CS-R30 | live | `docs/rules-mining/core-store.md:134` | store.ts×3 | — | Prototype pollution fully guarded: `__proto__` assignment inert; reading `constructor` on the draft returns undefined; storePath refuses `__proto__`/`constructor`/`prototype` segments; skips unsafe ow… | +| CS-R30 | live | `docs/rules-mining/core-store.md:134` | store.ts×5 | proto-key-removal-4672.test.ts×3 | Prototype pollution fully guarded: `__proto__` assignment inert; reading `constructor` on the draft returns undefined; storePath refuses `__proto__`/`constructor`/`prototype` segments; skips unsafe ow… | | CS-R31 | live | `docs/rules-mining/core-store.md:138` | store.ts×1 | a34-writes-then-derivations.test.ts×1 createMemo.test.ts×6 derived-write-then-derivation-3733.test.ts×1 held-derivation-not-a-proposal-3612.test.ts×3 createStore.test.ts×3 reconcile-resend-identity.test.ts×1 | Derived-store manual writes apply before the derivation.** A manual `setStore` lands at once, staged with any live transaction. When a source of the fold changes, in the same update or later, the fold… | | CS-R32 | live | `docs/rules-mining/core-store.md:147` | — | visibility-oracle-store.states.ts×1 | A setter-staged replacement followed by reconcile lands the reconciled value — staged writes fold into the diff.** Aligned: O7's resolution (a test already exists). | | CS-R33 | live | `docs/rules-mining/core-store.md:151` | — | visibility-oracle-store.states.ts×4 | Action/async lane semantics on store properties: a write held by an action makes isPending true for that property (per-property, not whole-store) while showing the committed value; applies on settle.*… | diff --git a/packages/signals/src/store/reconcile.ts b/packages/signals/src/store/reconcile.ts index ea52e4d279..3843500982 100644 --- a/packages/signals/src/store/reconcile.ts +++ b/packages/signals/src/store/reconcile.ts @@ -34,6 +34,7 @@ import { sameKey, sameRow, targetsEqual, + UNSAFE_KEYS, unwrapValue, userWriting } from "./store.js"; @@ -272,7 +273,15 @@ function applyAdopt(t: StoreTarget, incoming: any, keyFn: KeyFn | null, proj = f // Keys the incoming object no longer has still have nodes. if (nodes !== null && nodesHit < t.nc) { for (const key of Reflect.ownKeys(nodes)) { - if (!hasOwnP.call(incoming, key)) notifyKeyDiff(nodes[key as any], key, old, incoming, false); + // A removed pollution key reads absent, not the prototype's (#4672). + if (!hasOwnP.call(incoming, key)) + notifyKeyDiff( + nodes[key as any], + key, + old, + UNSAFE_KEYS.has(key) ? Object.create(null) : incoming, + false + ); } } notifyFoldTail(t, old, incoming); diff --git a/packages/signals/src/store/store.ts b/packages/signals/src/store/store.ts index 31ed4a11eb..46aec00a8d 100644 --- a/packages/signals/src/store/store.ts +++ b/packages/signals/src/store/store.ts @@ -393,7 +393,10 @@ export function getNode( // accessor-ness right before creating the node — pass the verdict through // so creation skips the second descriptor scan. -1 = unknown (other // callers), 0/1 = probed. - accKnown: -1 | 0 | 1 = -1 + accKnown: -1 | 0 | 1 = -1, + // An inherited pollution key (core R30): born absent, never with the + // prototype's value (#4672). + absent?: true ): Signal { const nodes = (target.n ??= Object.create(null)); let node: Signal | undefined = nodes[key]; @@ -403,14 +406,14 @@ export function getNode( // receiver. const pb = target.pb; const created: Signal = (node = slotSignal( - committed(target)[key as any], + absent ? undefined : committed(target)[key as any], slotNodeEquals, target, key, accKnown === -1 ? isOwnAccessor(pb ?? target.v, key) : accKnown === 1 )); if (pb !== null && !(created as any).acc) - bornStaged(target, created, target.del?.has(key) ? undefined : pb[key as any], key); + bornStaged(target, created, absent || target.del?.has(key) ? undefined : pb[key as any], key); // Attribution-only: name store property nodes by path segment so // attribution chains and wide-scope warnings read "store.todos" (or // "todos.title" when the store was declared with a name), not @@ -1214,9 +1217,10 @@ function notifyWrites(t: StoreTarget): void { for (const key of keys) { const node = nodes[key as any]; if (node === undefined) continue; + let own = false; if ( (node as any).acc === true || - (hasOwn.call(pb, key) && lookupGetter.call(pb, key) !== undefined) + ((own = hasOwn.call(pb, key)) && lookupGetter.call(pb, key) !== undefined) ) { // Accessor keys: the node is linked for shape-change notification, // its value is never served (the getter runs with the proxy @@ -1235,7 +1239,10 @@ function notifyWrites(t: StoreTarget): void { if (od?.get || od?.set || !isEqual(od?.value, nd.value)) setSignal(node, () => nd.value); continue; } - const nv = t.del?.has(key) ? undefined : pb[key as any]; + // An absent key's node is told `undefined`, never an inherited + // pollution key's value (core R30, #4672); an own key pays nothing. + // (An overlay's written keys are own on it or in `del`.) + const nv = own || !(t.del?.has(key) || UNSAFE_KEYS.has(key)) ? pb[key as any] : undefined; setSignal(node, () => nv); if (t._fam !== null) { // The derive's write is a derivation; a user setter's is a proposal @@ -2003,7 +2010,7 @@ function optRead(target: StoreTarget): boolean { } /** Pollution keys are never served from the prototype (core R30). */ -const UNSAFE_KEYS = new Set(["__proto__", "prototype", "constructor"]); +export const UNSAFE_KEYS = new Set(["__proto__", "prototype", "constructor"]); // --------------------------------------------------------------------------- // traps @@ -2211,8 +2218,11 @@ const traps: ProxyHandler & ThisType = { (key === "constructor" || key === "__proto__" || key === "prototype") && !hasOwn.call(src, key) && !(viewOvl && hasOwn.call(target.v, key)) - ) - return undefined; + ) { + // Absent like any absent key: a tracked reader subscribes (R12, #4672). + getObserver() && !inDraft(target) && readNode(getNode(target, key, 0, true)); + return; + } let v = (src as any)[key]; if ( v === undefined ? !hasOwn.call(src, key) && !(viewOvl && hasOwn.call(target.v, key)) : false diff --git a/packages/signals/tests/store/proto-key-removal-4672.test.ts b/packages/signals/tests/store/proto-key-removal-4672.test.ts new file mode 100644 index 0000000000..0ec18803cf --- /dev/null +++ b/packages/signals/tests/store/proto-key-removal-4672.test.ts @@ -0,0 +1,230 @@ +/** + * #4672 — a store with own `__proto__` data (JSON rows can carry it) whose + * data was then removed read `Object.prototype` through the store: a + * store-wrapped object with 12 own keys, to tracked and untracked readers + * alike. The key's node had been told the backing's inherited value. Own + * `constructor` data leaked `Object` the same way. + * + * Core R30: inherited pollution keys are never served, so once the own data + * is gone the read answers what a store that never had it answers + * (`undefined`). F-10: the change is visible at the flush to every channel, + * so tracked readers re-run onto it. R12: reading an absent key subscribes, + * so the reverse direction (absent, then own data lands) reaches tracked + * readers too. R30 also keeps `__proto__` assignment inert, so a write after + * the removal neither lands nor pollutes `Object.prototype`. + */ +import { describe, expect, it } from "vitest"; +import { + createProjection, + createRenderEffect, + createRoot, + createStore, + flush, + reconcile +} from "../../src/index.js"; + +const withOwn = (key: string, value: unknown, base: Record = { id: "a" }) => { + const o: any = { ...base }; + Object.defineProperty(o, key, { value, enumerable: true, writable: true, configurable: true }); + return o; +}; + +function track(read: () => unknown) { + const seen: unknown[] = []; + createRoot(() => + createRenderEffect(read, v => { + seen.push(v); + }) + ); + flush(); + return seen; +} + +describe("#4672 own pollution-key data removed", () => { + it("the issue's repro: reconcile removal reads undefined, tracked and untracked", () => { + const [q, setQ] = createStore(withOwn("__proto__", 0)); + const seen = track(() => q["__proto__"]); + expect(q["__proto__"]).toBe(0); + expect(seen).toEqual([0]); + + setQ(reconcile({ id: "a" }, () => undefined) as any); + flush(); + + expect(q["__proto__"]).toBeUndefined(); + expect(seen).toEqual([0, undefined]); + expect(Object.keys(q)).toEqual(["id"]); + }); + + it("matches a store that never had own __proto__ data", () => { + const [never] = createStore({ id: "a" }); + const seen = track(() => never["__proto__"]); + expect(never["__proto__"]).toBeUndefined(); + expect(seen).toEqual([undefined]); + }); + + for (const key of ["__proto__", "constructor", "prototype"]) { + describe(`key ${key}`, () => { + const removals: Record { q: any; remove: () => void }> = { + reconcile: () => { + const [q, setQ] = createStore(withOwn(key, 0)); + return { q, remove: () => setQ(reconcile({ id: "a" }, () => undefined) as any) }; + }, + "setter delete": () => { + const [q, setQ] = createStore(withOwn(key, 0)); + return { + q, + remove: () => + setQ((s: any) => { + delete s[key]; + }) + }; + }, + projection: () => { + const [src, setSrc] = createStore({ has: true }); + const q = createProjection( + (draft: any) => { + if (src.has) withOwnOn(draft, key, 0); + else delete draft[key]; + }, + withOwn(key, 0) + ); + return { + q, + remove: () => + setSrc(s => { + s.has = false; + }) + }; + }, + "projection (returned value)": () => { + const [src, setSrc] = createStore({ has: true }); + const q = createProjection(() => (src.has ? withOwn(key, 0) : { id: "a" }), { + id: "a" + }); + return { + q, + remove: () => + setSrc(s => { + s.has = false; + }) + }; + }, + "derived store (returned value)": () => { + const [src, setSrc] = createStore({ has: true }); + const [q] = createStore(() => (src.has ? withOwn(key, 0) : { id: "a" }), { + id: "a" + }); + return { + q, + remove: () => + setSrc(s => { + s.has = false; + }) + }; + } + }; + + for (const [path, make] of Object.entries(removals)) { + it(`${path}: removal reads undefined and tells the tracked reader`, () => { + const { q, remove } = make(); + const seen = track(() => q[key]); + expect(q[key]).toBe(0); + + remove(); + flush(); + + expect(q[key]).toBeUndefined(); + expect(seen).toEqual([0, undefined]); + expect(Object.keys(q)).toEqual(["id"]); + }); + + it(`${path}: removal with no tracked reader reads undefined`, () => { + const { q, remove } = make(); + expect(q[key]).toBe(0); + remove(); + flush(); + expect(q[key]).toBeUndefined(); + }); + } + + it("reverse: absent, then own data lands through reconcile — tracked readers see it", () => { + const [q, setQ] = createStore({ id: "a" }); + const seen = track(() => q[key]); + expect(q[key]).toBeUndefined(); + + setQ(reconcile(withOwn(key, 0), () => undefined) as any); + flush(); + + expect(q[key]).toBe(0); + expect(seen).toEqual([undefined, 0]); + }); + + it("setting it to undefined keeps a present own key (R26), never the inherited value", () => { + const [q, setQ] = createStore(withOwn(key, 0)); + const seen = track(() => q[key]); + setQ((s: any) => { + s[key] = undefined; + }); + flush(); + // `__proto__` assignment is inert (R30): the own data stays. + const expected = key === "__proto__" ? 0 : undefined; + expect(q[key]).toBe(expected); + expect(seen.at(-1)).toBe(expected); + }); + }); + } + + it("a wide record (overlay pending backing): own __proto__ survives sibling writes, then removal reads undefined", () => { + const base: Record = {}; + for (let i = 0; i < 40; i++) base["k" + i] = i; + const [q, setQ] = createStore(withOwn("__proto__", 0, base)); + const seen = track(() => [q["__proto__"], q.k1]); + for (let r = 0; r < 3; r++) { + setQ((s: any) => { + s.k1 = 100 + r; + }); + flush(); + } + expect(q["__proto__"]).toBe(0); + setQ((s: any) => { + delete s["__proto__"]; + s.k2 = 5; + }); + flush(); + expect(q["__proto__"]).toBeUndefined(); + setQ((s: any) => { + s.k3 = 5; + }); + flush(); + expect(q["__proto__"]).toBeUndefined(); + expect(seen.map((v: any) => v[0])).toEqual([0, 0, 0, 0, undefined]); + }); + + it("a write after removal: __proto__ stays inert and pollutes nothing; constructor/prototype land as data", () => { + for (const key of ["__proto__", "constructor", "prototype"]) { + const [q, setQ] = createStore(withOwn(key, 0)); + const seen = track(() => q[key]); + setQ((s: any) => { + delete s[key]; + }); + flush(); + setQ((s: any) => { + s[key] = { polluted4672: true }; + }); + flush(); + if (key === "__proto__") { + expect(q[key]).toBeUndefined(); + expect(seen).toEqual([0, undefined]); + } else { + expect(q[key]).toEqual({ polluted4672: true }); + expect(seen.at(-1)).toEqual({ polluted4672: true }); + } + expect(({} as any).polluted4672).toBeUndefined(); + expect(Object.prototype.hasOwnProperty.call(Object.prototype, "polluted4672")).toBe(false); + } + }); +}); + +function withOwnOn(o: any, key: string, value: unknown) { + Object.defineProperty(o, key, { value, enumerable: true, writable: true, configurable: true }); +} From 4fbe2a719d5a88a30a8d84fc56793e3a63acef27 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 23:54:44 +0000 Subject: [PATCH 2/2] fix(signals): narrow #4672 to __proto__ after #4674 With #4674 on next, a plain read of an inherited `constructor` returns the source's class, and a removed own `constructor` already reads it, as an object that never had one does. Ruling: narrow the fix to `__proto__`. - notifyWrites and reconcile's removed-key branch tell a removed `__proto__` node `undefined`. An own key does no extra work in notifyWrites (the existing hasOwn result is reused), and the reconcile check sits inside the existing removed-key branch. - Dropped: the get-trap subscription and getNode's `absent` parameter (the reverse direction is pinned `it.fails`), and every constructor-specific change and test, apart from one pin that a removed own `constructor` reads the inherited class, and a pin that `prototype` reads undefined. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0117BoWNSSgtemsSYsaX2hXE --- .changeset/proto-key-removal-4672.md | 2 +- packages/signals/docs/RULES-INDEX.md | 8 +- packages/signals/src/store/reconcile.ts | 6 +- packages/signals/src/store/store.ts | 26 +- .../store/proto-key-removal-4672.test.ts | 294 +++++++++--------- 5 files changed, 168 insertions(+), 168 deletions(-) diff --git a/.changeset/proto-key-removal-4672.md b/.changeset/proto-key-removal-4672.md index 031634fec2..905b0f0fb2 100644 --- a/.changeset/proto-key-removal-4672.md +++ b/.changeset/proto-key-removal-4672.md @@ -2,4 +2,4 @@ "@solidjs/signals": patch --- -A store whose own `__proto__` (or `constructor`) data is removed — by `reconcile`, a setter's `delete`, or a projection — now reads `undefined`, as a store that never had the key does, for tracked and untracked readers alike (#4672, core R30). The key's node used to be told the backing's inherited value, so the read served `Object.prototype` wrapped as a store (or `Object`). A tracked reader of an absent pollution key now subscribes like any absent key (R12), so own data landing later reaches it. `__proto__` assignment stays inert. +A store whose own `__proto__` data is removed — by `reconcile`, a setter's `delete`, or a projection — now reads `undefined`, as a store that never had it does, for tracked and untracked readers alike (#4672, core R30). The key's node used to be told the backing's inherited value, so the read served `Object.prototype` wrapped as a store. Tracked readers are notified of the change; `__proto__` assignment stays inert. diff --git a/packages/signals/docs/RULES-INDEX.md b/packages/signals/docs/RULES-INDEX.md index a3e6ecdb5b..324fdb079f 100644 --- a/packages/signals/docs/RULES-INDEX.md +++ b/packages/signals/docs/RULES-INDEX.md @@ -28,7 +28,7 @@ Status legend: **live** stated and standing · **ruled** carries an explicit rul | C | 4 | 0 | 3 | 1 | | INV | 11 | 4 | 7 | 4 | | RUL | 13 | 3 | 6 | 6 | -| R (CS) | 59 | 15 | 21 | 29 | +| R (CS) | 59 | 15 | 20 | 30 | | R (OL) | 37 | 4 | 1 | 32 | | R (OS) | 46 | 0 | 0 | 46 | | R (PJ) | 37 | 2 | 5 | 32 | @@ -153,7 +153,7 @@ Status legend: **live** stated and standing · **ruled** carries an explicit rul | CS-R9 | live | `docs/rules-mining/core-store.md:42` | store.ts×2 target.ts×1 | — | Proxy identity per logical slot is stable across writes and reconciles** (mapArray keyed flows reuse rows across refetch/reconcile). | | CS-R10 | live | `docs/rules-mining/core-store.md:48` | — | next-smoke.test.ts×1 | Per-property tracking; same-value writes (direct or functional path setter returning prev) do not re-trigger.** | | CS-R11 | live | `docs/rules-mining/core-store.md:52` | — | visibility-oracle-store.states.ts×3 visibility-oracle.states.ts×2 | Per-path tracking: reading `state.user.firstName` subscribes to that leaf; reading the reference `store[0]` does not subscribe to `store[0].i`.** | -| CS-R12 | live | `docs/rules-mining/core-store.md:56` | store.ts×2 | proto-key-removal-4672.test.ts×1 | Reading an absent key subscribes to that key: other-key changes don't trigger; defining it later (assignment or defineProperty) does.** | +| CS-R12 | live | `docs/rules-mining/core-store.md:56` | store.ts×1 | proto-key-removal-4672.test.ts×1 | Reading an absent key subscribes to that key: other-key changes don't trigger; defining it later (assignment or defineProperty) does.** | | CS-R13 | live | `docs/rules-mining/core-store.md:60` | store.ts×2 target.ts×1 | — | `in` tracks presence, not value: undefined-write doesn't retrigger; delete does; adding absent key does. `in`/`has` never invokes source getters.** | | CS-R14 | live | `docs/rules-mining/core-store.md:64` | — | — | `Object.keys` / `for…in` subscribe to key-set membership (root and nested) — distinct from property nodes.** Aligned: key-set node. | | CS-R15 | live | `docs/rules-mining/core-store.md:68` | store.ts×1 | — | Array structural tracking is uniform across idioms: indexed length loop, `for…of`, mapArray ($TRACK) all re-run exactly once per flush on add/update/removal.** | @@ -167,11 +167,11 @@ Status legend: **live** stated and standing · **ruled** carries an explicit rul | CS-R23 | live | `docs/rules-mining/core-store.md:103` | — | — | The proxy is immutable from outside the setter: direct assignment and delete are silently ignored (no change, no notify, no TypeError — traps report success while discarding).** | | CS-R24 | live | `docs/rules-mining/core-store.md:107` | — | next-smoke.test.ts×1 | Writes batch like signals: inside the setter draft, reads are read-your-writes (values, length, `in` sync); outside the setter, ALL reads — value, `in`, length — return pre-write state until flush(). … | | CS-R25 | live | `docs/rules-mining/core-store.md:112` | — | next-smoke.test.ts×1 | Writes to properties with ZERO observers still batch (no effects anywhere; pre-write value visible between setState and flush).** | -| CS-R26 | live | `docs/rules-mining/core-store.md:117` | — | proto-key-removal-4672.test.ts×1 | Setting a key to undefined is not deletion: key stays present (`in` true, no key-set notify); only delete / storePath.DELETE removes.** | +| CS-R26 | live | `docs/rules-mining/core-store.md:117` | — | — | Setting a key to undefined is not deletion: key stays present (`in` true, no key-set notify); only delete / storePath.DELETE removes.** | | CS-R27 | live | `docs/rules-mining/core-store.md:121` | store.ts×2 | — | The setter may return a replacement value that swaps the root wholesale; symbol keys on the replacement preserved.** | | CS-R28 | live | `docs/rules-mining/core-store.md:125` | reconcile.ts×1 | — | `storePath` addressing: string keys, numeric indices, index arrays, predicate filters ((value, index)), ranges, trailing functional setters address and update intended paths + trigger per-path subscri… | | CS-R29 | live | `docs/rules-mining/core-store.md:129` | store.ts×1 | overlay.test.ts×1 | Merge/replacement preserve accessor descriptors and keep getters LIVE (re-evaluated per read, reactive reads track), for pre-existing and new keys.** | -| CS-R30 | live | `docs/rules-mining/core-store.md:134` | store.ts×5 | proto-key-removal-4672.test.ts×3 store-prototype-4670.test.ts×2 | Prototype pollution fully guarded: `__proto__` assignment inert; reading `constructor` on the draft returns undefined; storePath refuses `__proto__`/`constructor`/`prototype` segments; skips unsafe ow… | +| CS-R30 | live | `docs/rules-mining/core-store.md:134` | reconcile.ts×1 store.ts×4 | proto-key-removal-4672.test.ts×3 store-prototype-4670.test.ts×2 | Prototype pollution fully guarded: `__proto__` assignment inert; reading `constructor` on the draft returns undefined; storePath refuses `__proto__`/`constructor`/`prototype` segments; skips unsafe ow… | | CS-R31 | live | `docs/rules-mining/core-store.md:138` | store.ts×1 | a34-writes-then-derivations.test.ts×1 createMemo.test.ts×6 derived-write-then-derivation-3733.test.ts×1 held-derivation-not-a-proposal-3612.test.ts×3 createStore.test.ts×3 reconcile-resend-identity.test.ts×1 | Derived-store manual writes apply before the derivation.** A manual `setStore` lands at once, staged with any live transaction. When a source of the fold changes, in the same update or later, the fold… | | CS-R32 | live | `docs/rules-mining/core-store.md:147` | — | visibility-oracle-store.states.ts×1 | A setter-staged replacement followed by reconcile lands the reconciled value — staged writes fold into the diff.** Aligned: O7's resolution (a test already exists). | | CS-R33 | live | `docs/rules-mining/core-store.md:151` | — | visibility-oracle-store.states.ts×4 | Action/async lane semantics on store properties: a write held by an action makes isPending true for that property (per-property, not whole-store) while showing the committed value; applies on settle.*… | diff --git a/packages/signals/src/store/reconcile.ts b/packages/signals/src/store/reconcile.ts index 3843500982..1eed03ee3e 100644 --- a/packages/signals/src/store/reconcile.ts +++ b/packages/signals/src/store/reconcile.ts @@ -34,7 +34,6 @@ import { sameKey, sameRow, targetsEqual, - UNSAFE_KEYS, unwrapValue, userWriting } from "./store.js"; @@ -273,13 +272,14 @@ function applyAdopt(t: StoreTarget, incoming: any, keyFn: KeyFn | null, proj = f // Keys the incoming object no longer has still have nodes. if (nodes !== null && nodesHit < t.nc) { for (const key of Reflect.ownKeys(nodes)) { - // A removed pollution key reads absent, not the prototype's (#4672). + // A removed `__proto__` reads absent, not the prototype (core R30, + // #4672). if (!hasOwnP.call(incoming, key)) notifyKeyDiff( nodes[key as any], key, old, - UNSAFE_KEYS.has(key) ? Object.create(null) : incoming, + key === "__proto__" ? Object.create(null) : incoming, false ); } diff --git a/packages/signals/src/store/store.ts b/packages/signals/src/store/store.ts index 53a1611653..ade0477285 100644 --- a/packages/signals/src/store/store.ts +++ b/packages/signals/src/store/store.ts @@ -393,10 +393,7 @@ export function getNode( // accessor-ness right before creating the node — pass the verdict through // so creation skips the second descriptor scan. -1 = unknown (other // callers), 0/1 = probed. - accKnown: -1 | 0 | 1 = -1, - // An inherited pollution key (core R30): born absent, never with the - // prototype's value (#4672). - absent?: true + accKnown: -1 | 0 | 1 = -1 ): Signal { const nodes = (target.n ??= Object.create(null)); let node: Signal | undefined = nodes[key]; @@ -406,14 +403,14 @@ export function getNode( // receiver. const pb = target.pb; const created: Signal = (node = slotSignal( - absent ? undefined : committed(target)[key as any], + committed(target)[key as any], slotNodeEquals, target, key, accKnown === -1 ? isOwnAccessor(pb ?? target.v, key) : accKnown === 1 )); if (pb !== null && !(created as any).acc) - bornStaged(target, created, absent || target.del?.has(key) ? undefined : pb[key as any], key); + bornStaged(target, created, target.del?.has(key) ? undefined : pb[key as any], key); // Attribution-only: name store property nodes by path segment so // attribution chains and wide-scope warnings read "store.todos" (or // "todos.title" when the store was declared with a name), not @@ -1245,10 +1242,10 @@ function notifyWrites(t: StoreTarget): void { if (od?.get || od?.set || !isEqual(od?.value, nd.value)) setSignal(node, () => nd.value); continue; } - // An absent key's node is told `undefined`, never an inherited - // pollution key's value (core R30, #4672); an own key pays nothing. - // (An overlay's written keys are own on it or in `del`.) - const nv = own || !(t.del?.has(key) || UNSAFE_KEYS.has(key)) ? pb[key as any] : undefined; + // A removed `__proto__` is told `undefined`, never the prototype (core + // R30, #4672); an own key pays nothing. (An overlay's written keys are + // own on it or in `del`.) + const nv = own || !(t.del?.has(key) || key === "__proto__") ? pb[key as any] : undefined; setSignal(node, () => nv); if (t._fam !== null) { // The derive's write is a derivation; a user setter's is a proposal @@ -2016,7 +2013,7 @@ function optRead(target: StoreTarget): boolean { } /** Pollution keys are never served from the prototype (core R30). */ -export const UNSAFE_KEYS = new Set(["__proto__", "prototype", "constructor"]); +const UNSAFE_KEYS = new Set(["__proto__", "prototype", "constructor"]); // --------------------------------------------------------------------------- // traps @@ -2228,11 +2225,8 @@ const traps: ProxyHandler & ThisType = { : key === "__proto__" || key === "prototype") && !hasOwn.call(src, key) && !(viewOvl && hasOwn.call(target.v, key)) - ) { - // Absent like any absent key: a tracked reader subscribes (R12, #4672). - getObserver() && !inDraft(target) && readNode(getNode(target, key, 0, true)); - return; - } + ) + return undefined; let v = (src as any)[key]; if ( v === undefined ? !hasOwn.call(src, key) && !(viewOvl && hasOwn.call(target.v, key)) : false diff --git a/packages/signals/tests/store/proto-key-removal-4672.test.ts b/packages/signals/tests/store/proto-key-removal-4672.test.ts index 0ec18803cf..bdf162e3d0 100644 --- a/packages/signals/tests/store/proto-key-removal-4672.test.ts +++ b/packages/signals/tests/store/proto-key-removal-4672.test.ts @@ -2,16 +2,13 @@ * #4672 — a store with own `__proto__` data (JSON rows can carry it) whose * data was then removed read `Object.prototype` through the store: a * store-wrapped object with 12 own keys, to tracked and untracked readers - * alike. The key's node had been told the backing's inherited value. Own - * `constructor` data leaked `Object` the same way. + * alike. The key's node had been told the backing's inherited value. * - * Core R30: inherited pollution keys are never served, so once the own data - * is gone the read answers what a store that never had it answers - * (`undefined`). F-10: the change is visible at the flush to every channel, - * so tracked readers re-run onto it. R12: reading an absent key subscribes, - * so the reverse direction (absent, then own data lands) reaches tracked - * readers too. R30 also keeps `__proto__` assignment inert, so a write after - * the removal neither lands nor pollutes `Object.prototype`. + * Core R30: an inherited `__proto__` is never served, so once the own data is + * gone the read answers what a store that never had it answers: `undefined`. + * F-10: the change is visible at the flush to every channel, so tracked + * readers re-run onto it. R30 also keeps `__proto__` assignment inert, so a + * write after the removal neither lands nor pollutes `Object.prototype`. */ import { describe, expect, it } from "vitest"; import { @@ -40,7 +37,69 @@ function track(read: () => unknown) { return seen; } -describe("#4672 own pollution-key data removed", () => { +const removals: Record { q: any; remove: () => void }> = { + reconcile: key => { + const [q, setQ] = createStore(withOwn(key, 0)); + return { q, remove: () => setQ(reconcile({ id: "a" }, () => undefined) as any) }; + }, + "setter delete": key => { + const [q, setQ] = createStore(withOwn(key, 0)); + return { + q, + remove: () => + setQ((s: any) => { + delete s[key]; + }) + }; + }, + "projection draft delete": key => { + const [src, setSrc] = createStore({ has: true }); + const q = createProjection( + (draft: any) => { + if (src.has) + Object.defineProperty(draft, key, { + value: 0, + enumerable: true, + writable: true, + configurable: true + }); + else delete draft[key]; + }, + withOwn(key, 0) + ); + return { + q, + remove: () => + setSrc(s => { + s.has = false; + }) + }; + }, + "projection returned value": key => { + const [src, setSrc] = createStore({ has: true }); + const q = createProjection(() => (src.has ? withOwn(key, 0) : { id: "a" }), { id: "a" }); + return { + q, + remove: () => + setSrc(s => { + s.has = false; + }) + }; + }, + "derived store returned value": key => { + const [src, setSrc] = createStore({ has: true }); + const [q] = createStore(() => (src.has ? withOwn(key, 0) : { id: "a" }), { id: "a" }); + return { + q, + remove: () => + setSrc(s => { + s.has = false; + }) + }; + } +}; + +describe("#4672 own __proto__ data removed", () => { it("the issue's repro: reconcile removal reads undefined, tracked and untracked", () => { const [q, setQ] = createStore(withOwn("__proto__", 0)); const seen = track(() => q["__proto__"]); @@ -62,118 +121,54 @@ describe("#4672 own pollution-key data removed", () => { expect(seen).toEqual([undefined]); }); - for (const key of ["__proto__", "constructor", "prototype"]) { - describe(`key ${key}`, () => { - const removals: Record { q: any; remove: () => void }> = { - reconcile: () => { - const [q, setQ] = createStore(withOwn(key, 0)); - return { q, remove: () => setQ(reconcile({ id: "a" }, () => undefined) as any) }; - }, - "setter delete": () => { - const [q, setQ] = createStore(withOwn(key, 0)); - return { - q, - remove: () => - setQ((s: any) => { - delete s[key]; - }) - }; - }, - projection: () => { - const [src, setSrc] = createStore({ has: true }); - const q = createProjection( - (draft: any) => { - if (src.has) withOwnOn(draft, key, 0); - else delete draft[key]; - }, - withOwn(key, 0) - ); - return { - q, - remove: () => - setSrc(s => { - s.has = false; - }) - }; - }, - "projection (returned value)": () => { - const [src, setSrc] = createStore({ has: true }); - const q = createProjection(() => (src.has ? withOwn(key, 0) : { id: "a" }), { - id: "a" - }); - return { - q, - remove: () => - setSrc(s => { - s.has = false; - }) - }; - }, - "derived store (returned value)": () => { - const [src, setSrc] = createStore({ has: true }); - const [q] = createStore(() => (src.has ? withOwn(key, 0) : { id: "a" }), { - id: "a" - }); - return { - q, - remove: () => - setSrc(s => { - s.has = false; - }) - }; - } - }; - - for (const [path, make] of Object.entries(removals)) { - it(`${path}: removal reads undefined and tells the tracked reader`, () => { - const { q, remove } = make(); - const seen = track(() => q[key]); - expect(q[key]).toBe(0); - - remove(); - flush(); - - expect(q[key]).toBeUndefined(); - expect(seen).toEqual([0, undefined]); - expect(Object.keys(q)).toEqual(["id"]); - }); - - it(`${path}: removal with no tracked reader reads undefined`, () => { - const { q, remove } = make(); - expect(q[key]).toBe(0); - remove(); - flush(); - expect(q[key]).toBeUndefined(); - }); - } - - it("reverse: absent, then own data lands through reconcile — tracked readers see it", () => { - const [q, setQ] = createStore({ id: "a" }); - const seen = track(() => q[key]); - expect(q[key]).toBeUndefined(); - - setQ(reconcile(withOwn(key, 0), () => undefined) as any); - flush(); - - expect(q[key]).toBe(0); - expect(seen).toEqual([undefined, 0]); - }); + for (const [path, make] of Object.entries(removals)) { + it(`${path}: reads undefined and tells the tracked reader`, () => { + const { q, remove } = make("__proto__"); + const seen = track(() => q["__proto__"]); + expect(q["__proto__"]).toBe(0); + remove(); + flush(); + expect(q["__proto__"]).toBeUndefined(); + expect(seen).toEqual([0, undefined]); + expect(Object.keys(q)).toEqual(["id"]); + }); - it("setting it to undefined keeps a present own key (R26), never the inherited value", () => { - const [q, setQ] = createStore(withOwn(key, 0)); - const seen = track(() => q[key]); - setQ((s: any) => { - s[key] = undefined; - }); - flush(); - // `__proto__` assignment is inert (R30): the own data stays. - const expected = key === "__proto__" ? 0 : undefined; - expect(q[key]).toBe(expected); - expect(seen.at(-1)).toBe(expected); - }); + it(`${path}: with no tracked reader reads undefined`, () => { + const { q, remove } = make("__proto__"); + expect(q["__proto__"]).toBe(0); + remove(); + flush(); + expect(q["__proto__"]).toBeUndefined(); }); } + it("setting it to undefined is inert (R30): the own data stays", () => { + const [q, setQ] = createStore(withOwn("__proto__", 0)); + const seen = track(() => q["__proto__"]); + setQ((s: any) => { + s["__proto__"] = undefined; + }); + flush(); + expect(q["__proto__"]).toBe(0); + expect(seen).toEqual([0]); + }); + + it("a write after removal stays inert and pollutes nothing", () => { + const [q, setQ] = createStore(withOwn("__proto__", 0)); + const seen = track(() => q["__proto__"]); + setQ((s: any) => { + delete s["__proto__"]; + }); + flush(); + setQ((s: any) => { + s["__proto__"] = { polluted4672: true }; + }); + flush(); + expect(q["__proto__"]).toBeUndefined(); + expect(seen).toEqual([0, undefined]); + expect(({} as any).polluted4672).toBeUndefined(); + }); + it("a wide record (overlay pending backing): own __proto__ survives sibling writes, then removal reads undefined", () => { const base: Record = {}; for (let i = 0; i < 40; i++) base["k" + i] = i; @@ -200,31 +195,42 @@ describe("#4672 own pollution-key data removed", () => { expect(seen.map((v: any) => v[0])).toEqual([0, 0, 0, 0, undefined]); }); - it("a write after removal: __proto__ stays inert and pollutes nothing; constructor/prototype land as data", () => { - for (const key of ["__proto__", "constructor", "prototype"]) { - const [q, setQ] = createStore(withOwn(key, 0)); - const seen = track(() => q[key]); - setQ((s: any) => { - delete s[key]; - }); - flush(); - setQ((s: any) => { - s[key] = { polluted4672: true }; - }); - flush(); - if (key === "__proto__") { - expect(q[key]).toBeUndefined(); - expect(seen).toEqual([0, undefined]); - } else { - expect(q[key]).toEqual({ polluted4672: true }); - expect(seen.at(-1)).toEqual({ polluted4672: true }); - } - expect(({} as any).polluted4672).toBeUndefined(); - expect(Object.prototype.hasOwnProperty.call(Object.prototype, "polluted4672")).toBe(false); - } + // The reverse direction is outside #4672: a tracked reader of an absent + // `__proto__` gets no node (the get trap answers it before subscribing), so + // own data landing later reaches untracked reads only. Pinned as it stands. + it("reverse, untracked: own __proto__ data landing reads as data", () => { + const [q, setQ] = createStore({ id: "a" }); + setQ(reconcile(withOwn("__proto__", 0), () => undefined) as any); + flush(); + expect(q["__proto__"]).toBe(0); }); -}); -function withOwnOn(o: any, key: string, value: unknown) { - Object.defineProperty(o, key, { value, enumerable: true, writable: true, configurable: true }); -} + it.fails("reverse, tracked: own __proto__ data landing reaches the tracked reader (R12)", () => { + const [q, setQ] = createStore({ id: "a" }); + const seen = track(() => q["__proto__"]); + setQ(reconcile(withOwn("__proto__", 0), () => undefined) as any); + flush(); + expect(seen).toEqual([undefined, 0]); + }); + + // Pins for the sibling keys (#4674 ruling): a removed own `constructor` + // reads the inherited class, as an object that never had one does; own + // `prototype` data never leaked (no built-in prototype has one). + it("pin: a removed own constructor reads the inherited class", () => { + const { q, remove } = removals.reconcile("constructor"); + const seen = track(() => q.constructor); + remove(); + flush(); + expect(q.constructor).toBe(Object); + expect(seen).toEqual([0, Object]); + }); + + it("pin: a removed own prototype reads undefined", () => { + const { q, remove } = removals["setter delete"]("prototype"); + const seen = track(() => q.prototype); + remove(); + flush(); + expect(q.prototype).toBeUndefined(); + expect(seen).toEqual([0, undefined]); + }); +});