fix(signals): a removed own __proto__ reads undefined, not Object.prototype (#4672)

.changeset/proto-key-removal-4672.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/signals": patch
+---
+
+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.

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 | 19 | 30 |
+| 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×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.** |
@@ -171,7 +171,7 @@ Status legend: **live** stated and standing · **ruled** carries an explicit rul
 | 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 | 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.*… |

packages/signals/src/store/reconcile.ts

@@ -272,7 +272,16 @@ 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 `__proto__` reads absent, not the prototype (core R30,
+      // #4672).
+      if (!hasOwnP.call(incoming, key))
+        notifyKeyDiff(
+          nodes[key as any],
+          key,
+          old,
+          key === "__proto__" ? Object.create(null) : incoming,
+          false
+        );
     }
   }
   notifyFoldTail(t, old, incoming);

packages/signals/src/store/store.ts

@@ -1220,9 +1220,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
@@ -1241,7 +1242,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];
+      // 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

packages/signals/tests/store/proto-key-removal-4672.test.ts

@@ -0,0 +1,236 @@
+/**
+ * #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.
+ *
+ * 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 {
+  createProjection,
+  createRenderEffect,
+  createRoot,
+  createStore,
+  flush,
+  reconcile
+} from "../../src/index.js";
+
+const withOwn = (key: string, value: unknown, base: Record<string, unknown> = { 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;
+}
+
+const removals: Record<string, (key: string) => { q: any; remove: () => void }> = {
+  reconcile: key => {
+    const [q, setQ] = createStore<any>(withOwn(key, 0));
+    return { q, remove: () => setQ(reconcile({ id: "a" }, () => undefined) as any) };
+  },
+  "setter delete": key => {
+    const [q, setQ] = createStore<any>(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<any>(
+      (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<any>(() => (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<any>(() => (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<any>(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<any>({ id: "a" });
+    const seen = track(() => never["__proto__"]);
+    expect(never["__proto__"]).toBeUndefined();
+    expect(seen).toEqual([undefined]);
+  });
+
+  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(`${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<any>(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<any>(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<string, unknown> = {};
+    for (let i = 0; i < 40; i++) base["k" + i] = i;
+    const [q, setQ] = createStore<any>(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]);
+  });
+
+  // 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<any>({ id: "a" });
+    setQ(reconcile(withOwn("__proto__", 0), () => undefined) as any);
+    flush();
+    expect(q["__proto__"]).toBe(0);
+  });
+
+  it.fails("reverse, tracked: own __proto__ data landing reaches the tracked reader (R12)", () => {
+    const [q, setQ] = createStore<any>({ 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]);
+  });
+});