diff --git a/.project/concepts/app-contract/polyfill-surface/contract_polyfill-surface.md b/.project/concepts/app-contract/polyfill-surface/contract_polyfill-surface.md index 8a2c1ab..92e75c6 100644 --- a/.project/concepts/app-contract/polyfill-surface/contract_polyfill-surface.md +++ b/.project/concepts/app-contract/polyfill-surface/contract_polyfill-surface.md @@ -260,7 +260,9 @@ Where a call must first find out whether something already exists — a document **TWO COPIES OF THIS PACKAGE IN ONE APPLICATION MISBEHAVE SILENTLY, and nothing here can detect it.** What this package remembers it keeps in the package itself, not in any handle you hold: which documents are subscribed and who is listening for them, which identity the session settled on, which documents are open. None of that is shared between two copies. An application that ends up with two therefore runs two of everything — a document subscribed through one copy is invisible to the other, so changes to it simply never arrive; and the identity settled in one is not the identity the other acts as, so the same call writes as one user or the other depending on which copy it reached. Nothing raises, nothing warns, and no call can report it: from every entry point a second copy is indistinguishable from the first, and the symptoms surface far from the cause as missing updates and writes attributed to the wrong person. This is a packaging property, not a behaviour to code around — **declare this package once, as one dependency of the application**, and let your own code and every library built on it resolve to that copy. A library that builds on this package should declare it a *peer* rather than a dependency for exactly this reason, so that the application remains the one place it is named. -**No unfiltered read through `useShape`, and no way round it through the object it hands back.** What the call returns is NextGraph's own result, filtered: `data` and `promise` carry only items in documents you hold the key for, `isLoading` and the two page commands pass through, and `subscription` - the ORM's own handle, which reads and writes with no capability filter at all - is refused out loud rather than handed on. Any other member of that object is refused too unless what is behind it is a plain scalar, which is how a member a later `@ng-org/orm` adds is decided before anyone has seen it: whatever can carry an item is an object or a function, and both refuse. Inside `data`, every member that yields items is filtered, mutations pass through, and an ordered result addresses and counts the items you may read: `size`, `length`, an index, `Object.keys`, `in` and `JSON.stringify` all agree, and none of them says how many you may not. **Reading a member's DESCRIPTOR answers exactly what reading the member answers**, which is not a detail: `Object.getOwnPropertyDescriptor` - and the spread, `Object.entries` and console inspection built on it - reads a property without going through the view's own read, so a refused member throws there too rather than handing its value over. A document reached through that view alone, read nowhere else first, does not appear. +**No unfiltered read through `useShape`, and no way round it through the object it hands back.** What the call returns is NextGraph's own result, filtered: `data` and `promise` carry only items in documents you hold the key for, `isLoading` and the two page commands pass through, and `subscription` - the ORM's own handle, which reads and writes with no capability filter at all - is refused out loud rather than handed on. Any other member of that object is refused too unless what is behind it is a plain scalar, which is how a member a later `@ng-org/orm` adds is decided before anyone has seen it: whatever can carry an item is an object or a function, and both refuse. Inside `data`, every member that yields items is filtered, and an ordered result addresses and counts the items you may read: `size`, `length`, an index, `Object.keys`, `in` and `JSON.stringify` all agree, and none of them says how many you may not. **Reading a member's DESCRIPTOR answers exactly what reading the member answers**, which is not a detail: `Object.getOwnPropertyDescriptor` - and the spread, `Object.entries` and console inspection built on it - reads a property without going through the view's own read, so a refused member throws there too rather than handing its value over. **A mutation reaches the collection underneath, and neither hands back nor acts past what the view shows.** Writing is not broken - `add` reaches the document and the reactivity it drives - but `add` hands back the VIEW rather than the collection, because `Set.prototype.add` returns the collection so calls can be chained and chaining onto the unfiltered one is the whole leak; `delete` removes only an item the view shows, which is the same question `has` already declines to answer about; and `clear` empties the view rather than the collection every identity shares. A document reached through that view alone, read nowhere else first, does not appear. + +**`isLoading` does not mean what its name says, and the fault is upstream's.** `@ng-org/orm`'s declaration describes it as "`true` when no data is available yet"; the hook that ships computes it as the subscription's `isReady`, which is the inverse. Both artifacts were read and they disagree. This package forwards the value it is handed rather than correcting it, because a layer that quietly improves the target teaches you a behaviour you would have to unlearn. **Branch on `data === undefined`**, which is unambiguous, and treat `isLoading` as unusable until upstream settles it. **`getObjects` does not fetch a public store's key for you either**, for the same reason as the two shape reads below: it goes through the same ORM query and spends no round-trip asking. Read the document once by naming it — `docs.sparqlQuery` anchored on it — and it appears. @@ -292,7 +294,9 @@ What each level means here, in this package's own terms: - **The two arguments are the SDK's own types**, where the signature took `unknown` for both. `useShape(shapeType: ShapeType, conf: RdfOrmConfig | string | undefined)`, which is what the surface already said the pair was. A caller passing the SDK's shape and config compiles unchanged; one passing something else now does not, which is why this counts. - **`size` and `length` count what you may read, and so does every other way of asking.** An ordered config (`orderBy`) makes the collection an array, which says how many items it holds in four places - `length`, `Object.keys`, `in`, and the descriptor of each index - and only the first was filtered. `data[0]` now names the first item you may read rather than the first the engine returned, and the positions past them do not exist. - **A member's DESCRIPTOR answers what reading it answers.** `Object.getOwnPropertyDescriptor(result, "data")` returned the collection UNFILTERED, and `...(result, "subscription")` the handle that reading it refuses: that call does not go through the view's own read. Neither does a spread, `Object.entries`, `JSON.stringify` or a console inspection, so all of them were reading round the filter. They now read through it - which also means `{...result}` and `console.log(result)` throw on `subscription`, where they used to print it. Log `data`. -- **Unchanged**: the read still spends no round-trip asking a public store for a key, a document you have read nowhere else still does not appear, mutations of the live collection still pass through, and nothing about `useShapeQuery`, `getObjects` or `insertObject` moves. +- **A mutation no longer hands back the collection, nor acts past the view.** `add` returned it - `Set.prototype.add` returns `this` so calls chain, measured on the plain `Set` and on the real `DeepSignalSet` alike - so `view.add(x)` gave every identity's items to a caller whose every way IN was filtered, through the first member the ORM's own example reaches for. It now hands back the view. `delete` refused nothing and removed an item the view declines to confirm the existence of; it now removes only what the view shows. `clear` emptied the collection every identity shares; it now empties the view. **Writing still reaches the document** - that is the one thing that must not change, and `e2e/orm-live-read.ts` measures it against the deployed broker. +- **An ordered result yields indices where it used to yield items.** `keys()` handed back items, `entries()` handed back `[item, item]` and `forEach` handed the item as its second argument - the Set contract, applied to an array. No leak, and wrong output on your own data. +- **Unchanged**: the read still spends no round-trip asking a public store for a key, a document you have read nowhere else still does not appear, and nothing about `useShapeQuery`, `getObjects` or `insertObject` moves. `1.0.0-dev.10` is a **major**, and all of it is one rule: **"is this document open in this session" now has exactly one answer.** This package kept three records of that fact - a set of live bootstrap subscriptions, a per-document barrier state, and the subscription layer's own listeners - and the first two outlived the third. Only one remains, the state of the document's one real subscription; the other two are gone rather than reconciled, because two records of one fact can disagree and these did. Nothing is added, renamed or removed from the entry. diff --git a/packages/polyfill/src/emulated-verifier/read-filter.ts b/packages/polyfill/src/emulated-verifier/read-filter.ts index 7b80fb3..c676016 100644 --- a/packages/polyfill/src/emulated-verifier/read-filter.ts +++ b/packages/polyfill/src/emulated-verifier/read-filter.ts @@ -225,9 +225,27 @@ export function makeReadFilteredView(set: S, caps: CapRegistry const indexOf = (prop: string | symbol): number | null => arrayLike && typeof prop === "string" && INDEX.test(prop) ? Number(prop) : null; + /** + * The view itself, held in a box because it does not exist yet when the traps below are + * written. It is assigned on the line after the proxy is built, and a trap only runs on + * a property access, so nothing can read it before. + * + * It is needed because a member may hand the COLLECTION back - `Set.prototype.add` + * returns `this` so calls chain, and the real `DeepSignalSet` does the same (both + * measured). Handing that back is handing over every identity's items, so what goes + * back is the view. Never `receiver`: the descriptor trap reads a member with no + * receiver of its own, and a member reached that way must not be answered differently. + */ + const self: { view: S | null } = { view: null }; + const viewOf = (prop: string | symbol): S => { + // Unreachable in practice; refusing is the safe direction if it ever is not. + if (self.view === null) throw refusalOf(prop, "the collection view"); + return self.view; + }; + /** What the view answers for one member. The `get` trap and the descriptor trap BOTH * call this, so neither can answer what the other refuses. */ - const read = (target: object, prop: string | symbol, receiver: unknown): unknown => { + const read = (target: object, prop: string | symbol): unknown => { if (prop === Symbol.iterator) return function* () { yield* kept(target); }; // HOW MANY, under both spellings. `data` is a `DeepSignalSet` for an unordered read // and an array-like `DeepSignal` once the config asks for an @@ -242,13 +260,28 @@ export function makeReadFilteredView(set: S, caps: CapRegistry const i = indexOf(prop); if (i !== null) return kept(target)[i]; } - // A Set yields the item for both halves of a `[key, value]` pair; `DeepSignalSet` - // follows the same shape, so `keys`/`values`/`entries` are the Set contract. - if (prop === "values" || prop === "keys") return () => kept(target)[Symbol.iterator](); - if (prop === "entries") return () => kept(target).map((i) => [i, i] as const)[Symbol.iterator](); + // WHAT A POSITION IS, and it is not the same question on the two shapes. A Set yields + // the item for both halves of a `[key, value]` pair and hands `forEach` the item + // twice; an array yields INDICES from `keys()`, `[index, item]` from `entries()`, and + // hands `forEach` the index second. Answering an ordered result with the Set contract + // gave the caller the wrong output on their own data - no leak, just wrong. + if (prop === "values") return () => kept(target)[Symbol.iterator](); + if (prop === "keys") { + return () => + (arrayLike ? kept(target).map((_, i) => i) : kept(target))[Symbol.iterator](); + } + if (prop === "entries") { + return () => + kept(target) + .map((item, i) => (arrayLike ? ([i, item] as const) : ([item, item] as const))) + [Symbol.iterator](); + } if (prop === "forEach") { return (cb: (v: unknown, v2: unknown, s: unknown) => void) => { - for (const item of kept(target)) cb(item, item, receiver); + const items = kept(target); + for (let i = 0; i < items.length; i++) { + cb(items[i], arrayLike ? i : items[i], viewOf(prop)); + } }; } // `has(item)` is filtered, not forwarded: the caller already holds the item, so the @@ -285,13 +318,45 @@ export function makeReadFilteredView(set: S, caps: CapRegistry }; } - // MUTATIONS forward untouched. They take an item and return void or a boolean, so - // they yield nothing to leak — and the view must not break writes or the underlying - // reactivity. (Caught by `test/read-filter.test.ts` when the blanket refusal below - // was first written: refusing everything unknown also refused `add`.) - if (prop === "add" || prop === "delete" || prop === "clear") { + // MUTATIONS reach the collection underneath - the view must not break writes or the + // reactivity they drive - but under one rule, which the three of them used to break in + // three different ways: a mutation may not reach past what the view shows, and may not + // hand back what the view hides. + // + // They were forwarded bound to the target until 2026-09-24, on the written ground that + // "they take an item and return void or a boolean, so they yield nothing to leak". RUN + // rather than reasoned about, that ground is false: `Set.prototype.add` returns THIS so + // calls can be chained, so `view.add(x)` handed back the unfiltered collection - and + // `add` is the first member the SDK's own example reaches for. `delete` really does + // return a boolean and `clear` really does return undefined (both measured), but they + // ACTED past the view: `delete` removed an item `has` refuses to even confirm, and + // `clear` emptied the collection every identity shares. + if (prop === "add") { const fn = Reflect.get(target, prop, target); - return typeof fn === "function" ? fn.bind(target) : fn; + if (typeof fn !== "function") return fn; + return (...args: unknown[]): unknown => { + const out = (fn as (...a: unknown[]) => unknown).apply(target, args); + if (out === target) return viewOf(prop); // chaining lands on the view + if (cannotReachAnItem(out)) return out; + throw refusalOf(prop, "the collection view"); + }; + } + if (prop === "delete") { + const fn = Reflect.get(target, prop, target); + if (typeof fn !== "function") return fn; + // The same question `has` already refuses to answer, with a worse consequence. The + // two disagreed: `has(item)` said no while `delete(item)` removed it. + return (item: unknown): unknown => + keep(item) ? (fn as (i: unknown) => unknown).call(target, item) : false; + } + if (prop === "clear") { + const del = Reflect.get(target, "delete", target); + if (typeof del !== "function") return Reflect.get(target, prop, target); + // Emptying the view empties THE VIEW. `clear()` on the target empties the collection + // every identity shares, which destroys what the holder is not even allowed to see. + return (): void => { + for (const item of kept(target)) (del as (i: unknown) => unknown).call(target, item); + }; } // RAW ESCAPE HATCHES. `DeepSignalSet` exposes the underlying collection on @@ -313,14 +378,14 @@ export function makeReadFilteredView(set: S, caps: CapRegistry return answerUnnamedMember(target, prop, "the collection view"); }; - return new Proxy(set, { - get: (target, prop, receiver) => read(target, prop, receiver), + const proxy = new Proxy(set, { + get: (target, prop) => read(target, prop), getOwnPropertyDescriptor(target, prop) { // An index past what may be read names no position of this view, and saying so is // what keeps `has`, `ownKeys` and the descriptor telling one story. const i = indexOf(prop); if (i !== null && i >= kept(target).length) return undefined; - return filteredDescriptor(target, prop, () => read(target, prop, target)); + return filteredDescriptor(target, prop, () => read(target, prop)); }, has(target, prop) { const i = indexOf(prop); @@ -340,6 +405,8 @@ export function makeReadFilteredView(set: S, caps: CapRegistry return keys; }, }) as S; + self.view = proxy; + return proxy; } /** diff --git a/packages/polyfill/src/model/types.ts b/packages/polyfill/src/model/types.ts index 6859cac..cdc0c27 100644 --- a/packages/polyfill/src/model/types.ts +++ b/packages/polyfill/src/model/types.ts @@ -109,7 +109,12 @@ export interface ShapeResult { * `emulated-verifier/read-filter.ts` - and declares it here because refusing a member * requires knowing it is there. */ readonly subscription: unknown; - /** Whether the initial payload is still outstanding. */ + /** + * What the SDK's declaration calls "`true` when no data is available yet" - and what the + * SHIPPED hook computes as `ormSubscription.isReady`, which is the inverse. Both were + * read; they disagree, the disagreement is upstream's, and nothing here corrects it: + * this package forwards the value it is handed. Read `data === undefined` instead. + */ readonly isLoading: boolean; /** Load the next page. Present only under a paginated config. */ readonly nextPage?: (() => void) | undefined; diff --git a/packages/polyfill/test/read-filter.test.ts b/packages/polyfill/test/read-filter.test.ts index e936db6..e97dafa 100644 --- a/packages/polyfill/test/read-filter.test.ts +++ b/packages/polyfill/test/read-filter.test.ts @@ -186,3 +186,58 @@ test("the array form counts and addresses the readable items, by every route", ( // target cannot configure away - but it is not a position and does not enumerate. expect(Object.getOwnPropertyNames(view)).toContain("length"); }); + +// REGRESSION (fourth adversarial pass, 2026-09-24). The three mutations were forwarded +// bound to the TARGET, on the written ground that "they take an item and return void or a +// boolean, so they yield nothing to leak". RUN rather than reasoned about, that ground is +// false for one of the three: `Set.prototype.add` returns THIS so calls can be chained, so +// `view.add(x)` handed back the unfiltered collection - and `add` is the first member the +// ORM's own documentation example reaches for. The other two returned what the ground +// said, and ACTED past the view instead. +test("a mutation neither hands back nor reaches past what the view hides", () => { + const set = new Set([MINE, FOREIGN]); + const { caps, become } = setup("alice"); // alice holds MINE, not FOREIGN + const view = makeReadFilteredView(set, caps); + + // `add` hands back the VIEW, so chaining lands on the filtered collection. + const chained = view.add({ id: "c", "@graph": "did:ng:o:alice" }); + expect(chained).not.toBe(set); + expect([...chained].map((i) => i.id)).toEqual(["a", "c"]); + expect(set.has(FOREIGN)).toBe(true); // the write reached the real set, and only added + + // `delete` acts only on what the view shows - the same question `has` already refuses. + expect(view.has(FOREIGN)).toBe(false); + expect(view.delete(FOREIGN)).toBe(false); + expect(set.has(FOREIGN)).toBe(true); + expect(view.delete(MINE)).toBe(true); + expect(set.has(MINE)).toBe(false); + + // `clear` empties the VIEW, not the collection every identity shares. + view.clear(); + expect([...view]).toEqual([]); + expect([...set].map((i) => i.id)).toEqual(["n"]); + become("bob"); + expect(set.size).toBe(1); // FOREIGN survived a holder emptying "their" set +}); + +// An `orderBy` result is an ARRAY, and an array's positions are indices. Answering it with +// the Set contract gave a holder the wrong output on their own data: `keys()` yielded items +// instead of indices, `entries()` yielded `[item, item]`, and `forEach`'s second argument +// was the item. No leak, just wrong - and wrong output on the owner path is the other half +// of what this view owes. +test("the array form yields indices where a Set yields items", () => { + const items: Item[] = [MINE, FOREIGN, LINKED]; + const { caps } = setup("alice"); // holds MINE and LINKED + const view = makeReadFilteredView(items, caps); + + expect([...view.keys()]).toEqual([0, 1]); + expect([...view.entries()]).toEqual([[0, MINE], [1, LINKED]]); + expect([...view.values()]).toEqual([MINE, LINKED]); + const seen: Array<[string, unknown]> = []; + view.forEach((item, i) => seen.push([item.id, i])); + expect(seen).toEqual([["a", 0], ["p", 1]]); + // The Set form is unchanged: both halves of a pair are the item. + const asSet = makeReadFilteredView(new Set([MINE, FOREIGN]), caps); + expect([...asSet.keys()]).toEqual([MINE]); + expect([...asSet.entries()]).toEqual([[MINE, MINE]]); +}); diff --git a/packages/polyfill/test/use-shape-fake.ts b/packages/polyfill/test/use-shape-fake.ts index 24553a7..6ef2e0a 100644 --- a/packages/polyfill/test/use-shape-fake.ts +++ b/packages/polyfill/test/use-shape-fake.ts @@ -23,7 +23,11 @@ * - `promise` resolves to that same collection, and is `undefined` when `conf` is; * - `subscription` is the handle, carrying the items three ways of its own - * `signalObject`, `readyPromise` and the raw arrays a change listener is handed; - * - `isLoading` is a boolean, the one member that carries nothing; + * - `isLoading` is a boolean, the one member that carries nothing - reproduced with the + * shipped EXPRESSION, `ormSubscription.isReady`, which is the INVERSE of what the + * declaration beside it describes ("`true` when no data is available yet"). The + * inversion is upstream's, and a fake that quietly corrected it would be measuring a + * hook nobody ships - which is the exact blindness this file exists to remove; * - the two page commands exist only under a paginated config, as `WithMaybePagination` * `Omit`s them otherwise and the shipped hook leaves them `undefined`. * @@ -60,8 +64,9 @@ export interface FakeShapeHook { * A fake `useShape` over an in-memory row list. * * `loaded` (the default) answers as a hook whose payload has arrived. Turn it off for the - * state the declaration describes as "still loading": `data` and `promise` present but the - * data not there yet. + * state the declaration describes as "still loading": `promise` present but `data` not + * there yet. Note which way `isLoading` then reads - see the note above; it follows the + * shipped expression, not the shipped prose. */ export function makeUseShapeHook( options: { loaded?: boolean; paginated?: boolean } = {}, @@ -104,7 +109,9 @@ export function makeUseShapeHook( data: loaded ? collection : undefined, promise: Promise.resolve(collection), subscription, - isLoading: !loaded, + // Upstream, verbatim: `!ormSubscription ? false : ormSubscription.isReady`. There is + // a subscription here, so this is `isReady` - and `isReady` is `loaded`. + isLoading: loaded, nextPage: paginated ? (): void => void pages.push("next") : undefined, previousPage: paginated ? (): void => void pages.push("previous") : undefined, }; diff --git a/packages/polyfill/test/use-shape.test.ts b/packages/polyfill/test/use-shape.test.ts index 5545d69..d552be5 100644 --- a/packages/polyfill/test/use-shape.test.ts +++ b/packages/polyfill/test/use-shape.test.ts @@ -159,8 +159,12 @@ test("useShape: the holder reads their own items through every member the SDK of expect([...(result.data as Iterable)].map((o) => o["@graph"])).toEqual([hers, hers]); expect((result.data as { size: number }).size).toBe(2); expect([...((await result.promise) as Iterable)]).toHaveLength(2); - // The load state is a boolean and passes on its own, with no entry naming it. - expect(result.isLoading).toBe(false); + // The load state is a boolean and passes on its own, with no entry naming it. It reads + // `true` on a payload that HAS arrived, because the shipped hook computes it as + // `ormSubscription.isReady` while the declaration beside it says the opposite. That + // inversion is upstream's; the fake reproduces it rather than correcting it, and this + // assertion is what stops anyone here quietly making the target nicer than it is. + expect(result.isLoading).toBe(true); // The page commands forward: they return `void`, and what they load lands in a // collection only ever read back through the filter. result.nextPage?.(); @@ -198,6 +202,61 @@ test("useShape: an undefined config answers undefined rather than refusing", asy expect(result.isLoading).toBe(false); }); +// -- A MUTATION MAY NOT REACH PAST WHAT THE VIEW SHOWS --------------------- + +// THE RIGHTS CASE, second kind, and it is the member the SDK's own example reaches for +// first (`expenses.add({...})`). `Set.prototype.add` RETURNS THE COLLECTION so calls can +// be chained - measured, not assumed - so a mutation forwarded bound to the target handed +// the unfiltered collection back as its return value. The view filtered every way IN and +// gave the whole set away on the way OUT. +test("useShape: adding an item does not hand back the unfiltered collection", async () => { + const hook = makeUseShapeHook(); + inject(hook); + const hers = await documentOf("alice"); + const his = await documentOf("bob"); + hook.items.push(noteIn(hers, "a1"), noteIn(his, "b1")); + + setCurrentUser("bob"); + const data = useShape(NoteShape, EVERYTHING).data as { + add(item: BaseType): unknown; + [Symbol.iterator](): Iterator; + }; + expect([...data].map((o) => o["@graph"])).toEqual([his]); // the way in is filtered + + const handedBack = data.add(noteIn(his, "b2")); + expect(handedBack).not.toBe(hook.raw()); + // Chaining still works, and what it chains onto is the VIEW: still bob's documents only. + expect([...(handedBack as Iterable)].map((o) => o["@graph"])).toEqual([his, his]); + // and the write really did reach the collection underneath. + expect([...(hook.raw() as Iterable)]).toHaveLength(3); +}); + +// `has(item)` has always refused to answer about an item the holder may not read. Acting +// on one is the same question with a worse consequence, and the two disagreed: `has` said +// no while `delete` removed it. +test("useShape: deleting reaches only what the view shows", async () => { + const hook = makeUseShapeHook(); + inject(hook); + const hers = await documentOf("alice"); + const his = await documentOf("bob"); + const hersItem = noteIn(hers, "a1"); + hook.items.push(hersItem, noteIn(his, "b1")); + + setCurrentUser("bob"); + const data = useShape(NoteShape, EVERYTHING).data as { + delete(item: BaseType): boolean; + has(item: BaseType): boolean; + clear(): void; + }; + expect(data.has(hersItem)).toBe(false); + expect(data.delete(hersItem)).toBe(false); + expect([...(hook.raw() as Iterable)]).toHaveLength(2); // hers survives + + // and emptying the view empties the view, not the collection every identity shares. + data.clear(); + expect([...(hook.raw() as Iterable)].map((o) => o["@graph"])).toEqual([hers]); +}); + // -- SAFE FOR A SHAPE NOBODY HAS SEEN -------------------------------------- /** What a later `@ng-org/orm` might hand back: the six members of `alpha.21` plus two