From 44a9b6ee04892b66495759b76271e59149372087 Mon Sep 17 00:00:00 2001 From: Sylvain Duchesne Date: Mon, 10 Aug 2026 09:10:51 +0200 Subject: [PATCH] =?UTF-8?q?test:=20quatre=20tests=20prouvaient=20autre=20c?= =?UTF-8?q?hose=20que=20ce=20qu'ils=20annon=C3=A7aient?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lot D de la revue adverse. Aucun changement de comportement de la bibliothèque : ce sont les tests qui mentaient, et deux faux `ng` qui fabriquaient un état que le vrai broker ne produit pas. **« Un tiers résout l'inbox d'un autre utilisateur » prouvait le CACHE.** `userInbox` indexe par (compte, portée) sans regarder qui demande, donc Bob tombait sur l'entrée que la session d'Alice venait de chauffer. Rien de la persistance n'était exercé — le faux ne servait même pas la requête `docInbox` — si bien que dans une seconde SESSION, ou une seconde page de navigateur comme en pilote la suite applicative, Bob aurait obtenu une inbox DIFFÉRENTE et son dépôt serait parti où personne ne lit. C'est la panne que cette bibliothèque a déjà payée une fois. Deux causes, toutes deux dans les faux : la requête `docInbox` n'était servie nulle part, et le faux de `cross-user-access` prenait le **nom de graphe** pour le sujet dans un `INSERT DATA { GRAPH { … } }` — donc le pointeur du shim n'était jamais retrouvé et chaque résolution à froid créait un nouveau shim. Les deux corrigées, et les tests qui franchissent une frontière d'identité purgent maintenant le cache à la frontière. **Mon propre index d'inbox avait le même défaut**, découvert en faisant ça : `isKnownInbox` ne répondait que par sa mémoire, parce qu'aucun faux ne servait la requête. La moitié durable n'était pas exercée — exactement la faute que ce lot corrigeait ailleurs. **« Connecting drains BOTH levels » n'observait pas le second niveau.** Rejoué contre un `connectedUser` qui ne draine que les inbox de l'utilisateur, il restait vert. La raison n'est pas un test faible : le second niveau n'a **aucun producteur**. Le seul appel qui dépose un cap est `inbox.share(doc, toUser)`, qui résout l'inbox d'un UTILISATEUR, jamais celle d'un document. Drainer une inbox de document n'applique donc rien. Le test dit désormais ce qu'il prouve, et l'anticipation est nommée comme telle : en amont `AddInboxCap` est générique sur les repos et `InboxMsgContent::Link` existe, donc viser cela est légitime — annoncer que c'est exercé ne l'était pas. **« La liste de Bob ne contient pas la note d'Alice » n'avait pas de contrôle positif.** Bob n'écrivait jamais de note publique : sa liste était vide quoi qu'il arrive. Il en écrit une maintenant, et la vérification symétrique est ajoutée. Au passage, `showScope` lisait le DOM avant le rendu — le gestionnaire `change` de l'application lance `refresh()` sans l'attendre. 189 tests unitaires, e2e 40/40 et applicatif 12/12. --- packages/sdk/e2e/notebook.ts | 41 ++++++++--- packages/sdk/test/cross-user-access.test.ts | 78 +++++++++++++++++++-- packages/sdk/test/inbox.test.ts | 21 ++++++ packages/sdk/test/isolation-active.test.ts | 21 ++++++ 4 files changed, 145 insertions(+), 16 deletions(-) diff --git a/packages/sdk/e2e/notebook.ts b/packages/sdk/e2e/notebook.ts index 3f0915b..95e4bb3 100644 --- a/packages/sdk/e2e/notebook.ts +++ b/packages/sdk/e2e/notebook.ts @@ -116,10 +116,25 @@ async function signIn(ctx: BrowserContext, appUrl: string, id: string): Promise< // ── the acts, expressed as the application expresses them ─────────────────── -/** Show the notes of `scope` — the list is per-scope, so acting on a note means - * looking at the right shelf first. */ -async function showScope(a: Actor, scope: string): Promise { +/** + * Show the notes of `scope` — the list is per-scope, so acting on a note means looking at + * the right shelf first. + * + * The wait is not decoration: the application's `change` handler runs `void refresh()`, + * un-awaited, so reading `textContent` straight after `selectOption` reads the PREVIOUS + * shelf. A suite that asserts "Bob's list does not contain Alice's note" against a list + * that has not re-rendered is green whether isolation holds or not — found adversarially, + * 2026-08-10. + */ +async function showScope(a: Actor, scope: string, settle = "les notes"): Promise { await a.frame.locator('[data-testid="scope"]').selectOption(scope); + // The list is rebuilt wholesale; waiting for the marker the caller expects (or for the + // list to be empty) is the only signal the application offers. + await a.frame + .locator(`[data-testid="notes"]:has-text("${settle}"), [data-testid="notes"]:empty`) + .first() + .waitFor({ timeout: 60000 }) + .catch(() => {}); } async function writeNote(a: Actor, scope: string, title: string, body: string): Promise { @@ -243,7 +258,7 @@ async function main(): Promise { // 3. A note opened for messages: anyone deposits, only its owner reads. Bob addresses // the NOTE — he never names an inbox, and no application should have to. await journey("Bob leaves a message on Alice's note, and only Alice reads it", async () => { - await showScope(alice, "public"); // her public shelf, where "Courses" lives + await showScope(alice, "public", "Courses"); // her public shelf await openForMessages(alice, "Courses"); // Bob has to REOPEN so the address published on the note is visible to his session. const bob2 = await reopen(ctx!, url, bob); @@ -257,13 +272,21 @@ async function main(): Promise { // 4. Each actor lists their OWN notes and nothing else — the boundary, seen from // the only place that matters: what the screen shows. await journey("each actor's list holds their own notes, and no one else's", async () => { - await showScope(alice, "public"); + // POSITIVE CONTROL. Bob writes a public note of his own first — without it his list + // is empty whatever the boundary does, and "it does not contain Alice's note" is + // true for the wrong reason. The assertion has to be able to fail. + await writeNote(bob, "public", "Vélo", "réviser les freins"); + await showScope(bob, "public", "Vélo"); + const bobList = (await bob.frame.locator('[data-testid="notes"]').textContent()) ?? ""; + + await showScope(alice, "public", "Courses"); await alice.frame.locator('li:has-text("Courses")').waitFor({ timeout: 60000 }); const aliceList = (await alice.frame.locator('[data-testid="notes"]').textContent()) ?? ""; - await showScope(bob, "public"); - const bobList = (await bob.frame.locator('[data-testid="notes"]').textContent()) ?? ""; - check("Alice sees her own note", aliceList.includes("Courses"), aliceList.slice(0, 80)); - check("Bob's own list does not contain Alice's note", !bobList.includes("Courses"), bobList.slice(0, 80) || "(vide)"); + + check("Alice sees her own note", aliceList.includes("Courses"), aliceList.slice(0, 60)); + check("Bob sees HIS own note — the control that lets the next check fail", bobList.includes("Vélo"), bobList.slice(0, 60)); + check("Bob's list does not contain Alice's note", !bobList.includes("Courses"), bobList.slice(0, 60)); + check("Alice's list does not contain Bob's note", !aliceList.includes("Vélo"), aliceList.slice(0, 60)); }); } finally { await ctx?.close().catch(() => {}); diff --git a/packages/sdk/test/cross-user-access.test.ts b/packages/sdk/test/cross-user-access.test.ts index c8c4789..f9e717f 100644 --- a/packages/sdk/test/cross-user-access.test.ts +++ b/packages/sdk/test/cross-user-access.test.ts @@ -99,7 +99,16 @@ function makeFakeNg() { } return undefined; } - const body = query.replace(/^\s*INSERT DATA\s*\{/, "").replace(/\}\s*$/, ""); + // `INSERT DATA { GRAPH { … } }` — the shape the store-ROOT pointer write uses. + // Without this arm the first `<…>` in the body is the GRAPH NAME, so the pointer was + // stored with the graph as its subject and its predicate as its object. The pointer + // SELECT then found nothing, every cold `resolveShimDoc` forked a NEW shim, and the + // suite never noticed because the module cache carried the previous answer. Added + // 2026-08-10; the other fakes had it already. + const gm = query.match(/GRAPH\s+<([^>]+)>\s*\{([\s\S]*)\}/); + const body = gm + ? gm[2]! + : query.replace(/^\s*INSERT DATA\s*\{/, "").replace(/\}\s*$/, ""); const sm = body.match(/<([^>]+)>/); if (!sm) return undefined; const s = sm[1]!; @@ -183,6 +192,27 @@ function makeFakeNg() { if (query.includes(`<${SHIM}:link>`)) { return { results: { bindings: quads.filter((q) => q.g === anchor && q.p === `${SHIM}:link`).map((q) => ({ c: { value: q.o } })) } }; } + // Shim `isInbox` SELECT — the emulated "the broker knows this is an inbox". Absent at + // first, so `isKnownInbox` answered from its in-memory set alone: the durable half was + // never exercised, which is the very fault this pass was fixing elsewhere. + if (query.includes(`${SHIM}:isInbox`)) { + return { results: { bindings: quads + .filter((q) => q.g === anchor && q.p === `${SHIM}:isInbox`) + .map((q) => ({ i: { value: q.o } })) } }; + } + // Shim `docInbox:` SELECT — WHICH inbox a virtual user owns. Absent until + // 2026-08-10, so `userInbox` never found a persisted address and answered from the + // module cache alone: two actors in one JS realm agreed, two SESSIONS would not have. + // The suite's "a third party resolves another user's inbox" was proving the cache. + if (query.includes(`${SHIM}:docInbox`)) { + const pm = query.match(/<(urn:ng-eventually:shim:docInbox:[a-z]+)>/); + const pred = pm ? pm[1]! : ""; + const sm = query.match(/<([^>]+)>\s+ q.g === anchor && q.p === pred && (subj === null || q.s === subj)) + .map((q) => ({ d: { value: q.o } })) } }; + } // Header-branch `exposedReadCap` SELECT — what a PUBLIC store serves to anyone. if (query.includes(`<${SHIM}:exposedReadCap>`)) { return { results: { bindings: quads.filter((q) => q.g === anchor && q.p === `${SHIM}:exposedReadCap`).map((q) => ({ c: { value: q.o } })) } }; @@ -516,6 +546,11 @@ test("a document has its own inbox: anyone deposits, only the owner reads", asyn // handed, and the only thing an application circulates. The document is in a public // store, so the store serves him its read cap; the address is not passed to him, // because if it had to be there would be no way for an app to get it. + // + // The registry cache is dropped first: Bob is another session, and an address he can + // only find because Alice's session warmed a module map is an address no second browser + // page would find. + resetRegistryCache(); setCurrentUser("bob"); const bobTarget = await documentInboxAddress(doc); expect(bobTarget).toBe(aliceInbox); // …and it is the SAME inbox alice reads @@ -537,6 +572,7 @@ test("opening an inbox on someone else's document is refused, not silently forke const aliceInbox = await openDocumentInbox(doc); // Bob can READ the document (it is in a public store) — and reading is not ownership. + resetRegistryCache(); // another session, not a warmed cache setCurrentUser("bob"); await expect(openDocumentInbox(doc)).rejects.toThrow(/already has an inbox|you may only open an inbox/i); // The address he resolves is still alice's, so his deposits reach her. @@ -551,6 +587,7 @@ test("a fresh document has NO inbox — one belongs to one document, and only it // Not "the owner's inbox by default": upstream an inbox belongs to exactly ONE repo // (the verifier routes by `inboxes: PubKey → RepoId`), so pointing several documents // at one inbox is a relation the model cannot express. + resetRegistryCache(); // another session, not a warmed cache setCurrentUser("bob"); expect(await documentInboxAddress(doc)).toBeUndefined(); // …and depositing THROWS rather than vanishing — a lost deposit is the bug this @@ -565,6 +602,7 @@ test("opening an inbox publishes ONE address, and re-opening does not accumulate const dedicated = await openDocumentInbox(doc); expect(await openDocumentInbox(doc)).toBe(dedicated); // idempotent + resetRegistryCache(); // another session, not a warmed cache setCurrentUser("bob"); expect(await documentInboxAddress(doc)).toBe(dedicated); // The deposit reaches the owner, addressed by the document alone. @@ -587,21 +625,37 @@ test("the inbox address is machinery: it never surfaces as the document's data", expect(Object.keys(props)).toEqual([SECRET]); }); -test("connecting drains BOTH levels: the user's inbox and its documents'", async () => { +// CONNECTING APPLIES WHAT WAS DEPOSITED — and the honest scope of that claim. +// +// This was called "connecting drains BOTH levels" and asserted nothing about the second. +// An adversarial review replayed it against a `connectedUser` that drained ONLY the +// user's own two inboxes: all three assertions still passed. The reason is not a weak +// test, it is that the second level currently has **no producer**: the one call that +// deposits a cap is `inbox.share(doc, toUser)`, which resolves `userInbox(toUser, +// "protected")` (`surface/inbox.ts`) — a USER's inbox, never a document's. Nothing +// published can address a cap to a document's inbox, so draining one applies nothing and +// there is nothing to observe. +// +// That the drain covers document inboxes is therefore an ANTICIPATION, and a legitimate +// one: upstream `AddInboxCap` is generic over repos (no `is_store` check, +// `engine/verifier/src/verifier.rs:1916-1930`) and `InboxMsgContent::Link` exists as a +// variant. What is NOT legitimate is a test title asserting a property no code exercises. +// So this test states what it proves; the anticipation is named, not dressed up. +test("connecting applies the Links waiting for me, and leaves consumer deposits alone", async () => { inject(); setCurrentUser("alice"); const protDoc = await createEntityDoc("alice", "protected"); const pubDoc = await createEntityDoc("alice", "public"); const docInbox = await openDocumentInbox(pubDoc); - const aliceInbox = await userInbox("alice", "protected"); // Two deposits, one at each level, both made by someone else. + resetRegistryCache(); setCurrentUser("carol"); const carolDoc = await createEntityDoc("carol", "protected"); - await share(carolDoc, "alice"); // a Link, to alice herself - await post(docInbox, { payload: { onTheDocument: true }, ts: 2 }); + await share(carolDoc, "alice"); // a Link, into ALICE's own inbox — the only cap path + await post(docInbox, { payload: { onTheDocument: true }, ts: 2 }); // consumer data - // Alice connects: one call, both queues. + // Alice connects: one call, and she calls nothing to "receive". setCurrentUser("alice"); await connectedUser(); @@ -612,10 +666,20 @@ test("connecting drains BOTH levels: the user's inbox and its documents'", async }); // The same resolution property one level up: a user's own inbox. -test("a third party resolves another user's inbox (the wallet level)", async () => { +// +// REGRESSION (2026-08-10, found adversarially). This test used to pass on the module +// CACHE: `userInbox` keys by (account, scope) regardless of who is asking, so Bob hit the +// entry Alice had just warmed. Nothing about persistence was exercised — the fake did not +// even answer the shim query — so in a second SESSION (or a second browser page, which is +// what the applicative e2e runs) Bob would have got a DIFFERENT inbox, and his deposit +// would have gone where nobody reads. That is the exact failure this library already paid +// for once. Dropping the cache between the two actors is what makes it a real test. +test("a third party resolves another user's inbox, from the shim and not from a cache", async () => { inject(); setCurrentUser("alice"); const aliceView = await userInbox("alice", "protected"); + + resetRegistryCache(); // Bob is another session: nothing of Alice's is in memory setCurrentUser("bob"); const bobView = await userInbox("alice", "protected"); expect(bobView).toBe(aliceView); diff --git a/packages/sdk/test/inbox.test.ts b/packages/sdk/test/inbox.test.ts index fb866f8..b1b72ff 100644 --- a/packages/sdk/test/inbox.test.ts +++ b/packages/sdk/test/inbox.test.ts @@ -118,7 +118,28 @@ function makeFakeNg() { }); const sparql_query = mock(async (...a: unknown[]) => { + const query = a[1] as string; const anchor = a[3] as string | undefined; + // Shim `isInbox` SELECT — the emulated "the broker knows this is an inbox". Absent at + // first, so `isKnownInbox` answered from its in-memory set alone: the durable half was + // never exercised, which is the very fault this pass was fixing elsewhere. + if (query.includes("urn:ng-eventually:shim:isInbox")) { + return { results: { bindings: quads + .filter((q) => q.g === anchor && q.p === "urn:ng-eventually:shim:isInbox") + .map((q) => ({ i: { value: q.o } })) } }; + } + // Shim `docInbox:` SELECT — WHICH inbox a virtual user owns. Without it + // `userInbox` never finds a persisted address and answers from the module cache, so + // a test comparing two actors compares one cached value with itself. + if (query.includes("urn:ng-eventually:shim:docInbox")) { + const pm = query.match(/<(urn:ng-eventually:shim:docInbox:[a-z]+)>/); + const pred = pm ? pm[1]! : ""; + const sm = query.match(/<([^>]+)>\s+ q.g === anchor && q.p === pred && (subj === null || q.s === subj)) + .map((q) => ({ d: { value: q.o } })) } }; + } const bySubject = new Map>(); for (const q of quads) { if (q.g !== anchor) continue; diff --git a/packages/sdk/test/isolation-active.test.ts b/packages/sdk/test/isolation-active.test.ts index a006dcb..d5a2e16 100644 --- a/packages/sdk/test/isolation-active.test.ts +++ b/packages/sdk/test/isolation-active.test.ts @@ -152,6 +152,27 @@ function makeFakeNg() { }); return { results: { bindings } }; } + // Shim `isInbox` SELECT — the emulated "the broker knows this is an inbox". Absent at + // first, so `isKnownInbox` answered from its in-memory set alone: the durable half was + // never exercised, which is the very fault this pass was fixing elsewhere. + if (query.includes(`${SHIM}:isInbox`)) { + return { results: { bindings: quads + .filter((q) => q.g === anchor && q.p === `${SHIM}:isInbox`) + .map((q) => ({ i: { value: q.o } })) } }; + } + // Shim `docInbox:` SELECT — WHICH inbox a virtual user owns. Absent until + // 2026-08-10, so `userInbox` never found a persisted address and answered from the + // module cache alone: two actors in one JS realm agreed, two SESSIONS would not have. + // The suite's "a third party resolves another user's inbox" was proving the cache. + if (query.includes(`${SHIM}:docInbox`)) { + const pm = query.match(/<(urn:ng-eventually:shim:docInbox:[a-z]+)>/); + const pred = pm ? pm[1]! : ""; + const sm = query.match(/<([^>]+)>\s+ q.g === anchor && q.p === pred && (subj === null || q.s === subj)) + .map((q) => ({ d: { value: q.o } })) } }; + } // User-branch `link` SELECT (the emulated AddLink records). // User-branch `inboxCap` SELECT (the emulated AddInboxCap records). if (query.includes(`<${SHIM}:inboxCap>`)) {