fix: second tour adverse — mes correctifs avaient quatre trous, dont un qui les annulait

J'ai relancé un adversaire sur les correctifs du commit précédent, comme la règle
l'exige. Il en a trouvé quatre. Le premier annulait la garde que je venais d'écrire.

**Le registre de propriété était écrivable depuis la surface publiée.** `assertMayWrite`
lit la propriété dans l'index de store de l'appelant — et `caps.open` marquait les
documents de STRUCTURE (les trois stores, les inbox) comme « créés par moi ». Donc un
porteur pouvait, par le `docs.sparqlUpdate` publié, insérer `contains "<n'importe quel
document>"` dans son propre index et s'en déclarer propriétaire. Démontré : Bob écrit dans
le document protégé d'Alice, et détourne l'inbox d'un de ses documents — exactement le
vecteur que le commit précédent prétendait fermer. `open` classe désormais sans marquer :
un document de structure n'est possédé par personne au sens de la paternité, donc les deux
moitiés de la garde répondent non, ce qui est correct.

**`inbox.post` acceptait n'importe quel NURI.** Déplacer `depositInto` hors de la surface
ne suffisait pas : `post` atteint la même porte, qui saute les deux gardes par
conception. Bob, ne détenant rien, écrivait quatre triplets dans le document d'Alice. En
amont la confusion est impossible — `InboxPost` scelle vers une CLÉ d'inbox et le broker
route par `inboxes: PubKey → RepoId` ; adresser un document n'est pas refusé, c'est
inexprimable. Le shim tient maintenant un index des inbox, l'équivalent émulé de ce que
le broker sait par construction, et `post` refuse ce qui n'en est pas une.

**Le filtre de lecture fuyait encore par les clés dunder.** `DeepSignalSet` expose la
collection brute sur `__raw__` / `__meta__` : `[...view]` rendait zéro élément pendant que
`view.__raw__` rendait le Set complet, tous utilisateurs confondus. Mon en-tête affirmait
qu'« une propriété simple ne porte aucun élément » — faux pour ce type.

**Et il cassait des membres légitimes** : ma liste blanche couvrait la moitié des
helpers d'itération, si bien que `toArray`, `reduce`, `first`, `take`, `drop`, `flatMap`
levaient sur les données du porteur lui-même. Tous filtrés désormais ; le refus ne vaut
que pour l'inconnu.

**Deux tests réparés à la source plutôt qu'en affaiblissant les gardes.** Le faux
`doc_create` de `inbox.test.ts` rendait une CONSTANTE — tous les documents créés étaient
le même NURI, donc la garde de propriété n'avait rien à distinguer et deux tests lisaient
l'inbox d'Alice sous l'identité de Bob sans que rien ne proteste. Et le harnais e2e
utilisait un document ordinaire comme inbox.

Enfin, mon propre cache d'inbox a reproduit la faute que la revue avait relevée ailleurs :
un mémo qui survit à sa session. Rattaché à `resetRegistryCache`.

189 tests unitaires (six régressions de plus), e2e 40/40 et applicatif 10/10 — après un
échec réseau non reproductible, relancé sans modification.
This commit is contained in:
Sylvain Duchesne
2026-08-07 14:26:32 +02:00
parent 0b936d2119
commit b5f05472d9
9 changed files with 227 additions and 7 deletions
@@ -22,6 +22,7 @@ import { test, expect, mock, afterAll } from "bun:test";
import {
createEntityDoc,
resetRegistryCache,
resolveWriteGraph,
userInbox,
} from "../src/shared-wallet/account-registry";
import { documentInboxAddress, openDocumentInbox } from "../src/emulated-verifier/branch-registers";
@@ -416,6 +417,44 @@ test("connecting a user that does not exist provisions nothing", async () => {
expect(getCaps().isEnforcing()).toBe(false);
});
// REGRESSION (second adversarial pass). `inbox.post` is a published door that skips both
// guards by design — the deposit is the one write that legitimately crosses. It accepted
// ANY NURI, so it wrote into a document its caller could not even read. Upstream the
// confusion cannot arise: a deposit carries an inbox key, not a document reference.
test("a deposit is addressed to an inbox, never to a document", async () => {
inject();
setCurrentUser("alice");
const protDoc = await createEntityDoc("alice", "protected");
await write(protDoc, SECRET, "alice's own");
setCurrentUser("bob");
await expect(post(protDoc, { payload: { x: 1 }, ts: 1 })).rejects.toThrow(/not an inbox/i);
setCurrentUser("alice");
expect(await readValues([protDoc], SECRET)).toEqual(["alice's own"]); // untouched
});
// REGRESSION (second adversarial pass). The write guard reads ownership from the store
// index — and the holder's own store document was marked "created by me", so it was
// writable through the PUBLISHED `docs.sparqlUpdate`. One insert into it and you were
// the owner of anything you cared to name.
test("a holder cannot write into their own store index and forge ownership", async () => {
inject();
setCurrentUser("alice");
const protDoc = await createEntityDoc("alice", "protected");
await write(protDoc, SECRET, "alice's own");
setCurrentUser("bob");
await createEntityDoc("bob", "protected"); // bob has his own stores
const bobStore = await resolveWriteGraph("bob", "protected");
await expect(
sparqlUpdate(SESSION.sessionId, `INSERT DATA { <${SHIM}:index> <${SHIM}:contains> "${protDoc}" }`, bobStore, "forge"),
).rejects.toThrow(/WRITE cap/i);
// …and he is still refused the write itself — here by rule 1 (he cannot even reach
// alice's protected document), which fires before the ownership guard. Both say no.
await expect(write(protDoc, SECRET, "bob was here")).rejects.toThrow(/refused/i);
});
// WRITING IS OWNERSHIP — the two regressions that replaced the old write guard.
//
// It used to ask "was this cap served to me by a public store?", which was wrong in both
+20 -3
View File
@@ -73,7 +73,13 @@ function makeFakeNg() {
for (const cb of subs.get(anchor) ?? []) cb({ V0: { Patch: { doc: anchor } } });
};
const doc_create = mock(async (..._a: unknown[]) => "did:ng:o:new");
// Distinct NURIs, one per creation — as a real broker does. It returned the CONSTANT
// `"did:ng:o:new"` until 2026-08-07, so every document the library made was the same
// one: two users' inboxes collided, and the ownership guard could not fire because
// there was nothing to tell apart. An adversarial review measured it. A fake that
// produces a state the real system never produces makes its suite green and blind.
let created = 0;
const doc_create = mock(async (..._a: unknown[]) => `did:ng:o:new${++created}`);
// Parses one deposit: `<subj> a <Deposit> ; <payload> "..." ; <ts> "..." [; <from> "..."] .`
//
@@ -202,9 +208,14 @@ test("(c) post rejects a spoofed `from` (naming another principal); self/null al
expect(froms).toEqual(["alice", null]);
});
test("from is optional — omitting it defaults to the current user", async () => {
// Bob DEPOSITS, alice READS. The asymmetry is the model — anyone deposits, only the
// owner reads — so a test that reads back under the depositor is testing a path no
// application has. It passed until 2026-08-07 only because the fake `doc_create` handed
// out one NURI for every document, so the ownership guard had nothing to tell apart.
test("from is optional — omitting it defaults to the depositor", async () => {
setCurrentUser("bob");
await post(TARGET, { payload: { hi: 1 }, ts: 200 });
setCurrentUser("alice");
const deposits = await read(TARGET);
expect(deposits[0]!.from).toBe("bob");
});
@@ -212,6 +223,7 @@ test("from is optional — omitting it defaults to the current user", async () =
test("from: null makes an anonymous deposit even when a current user is set", async () => {
setCurrentUser("bob");
await post(TARGET, { from: null, payload: { hi: 1 }, ts: 200 });
setCurrentUser("alice");
const deposits = await read(TARGET);
expect(deposits[0]!.from).toBeNull();
});
@@ -225,8 +237,13 @@ test("read returns deposits sorted by ts ascending and materialize is an alias",
});
test("read is scoped to one inbox — deposits in another inbox are not returned", async () => {
// The OTHER inbox is obtained from the system, not invented. A made-up NURI would be
// a target no deposit can legitimately reach (`inbox.post` refuses what is not an
// inbox), so the test would have been proving something the model does not allow.
const otherInbox = await userInbox("bob", "protected");
expect(otherInbox).not.toBe(TARGET);
await post(TARGET, { from: null, payload: "mine", ts: 1 });
await post("did:ng:o:other-inbox", { from: null, payload: "theirs", ts: 2 });
await post(otherInbox, { from: null, payload: "theirs", ts: 2 });
const deposits = await read(TARGET);
expect(deposits.map((d) => d.payload)).toEqual(["mine"]);
});
+28
View File
@@ -95,6 +95,34 @@ test("every item-yielding member is filtered, not just iteration", () => {
expect(view.has(MINE)).toBe(false);
});
// REGRESSION (second adversarial pass). `DeepSignalSet` exposes the underlying
// collection on dunder keys; the view forwarded non-function properties untouched, so
// `view.__raw__` handed back every identity's items while `[...view]` showed none.
test("a dunder escape hatch cannot reach past the view", () => {
const set = new Set<Item>([MINE]) as any;
set.__raw__ = set;
const { caps, become } = setup("alice");
const view = makeReadFilteredView(set, caps) as any;
become("bob");
expect([...view]).toEqual([]);
expect(() => view.__raw__).toThrow(/raw collection/i);
});
// REGRESSION (second adversarial pass). The first whitelist covered half of the reactive
// set's iterator helpers and threw on the rest, so a holder's calls on their OWN data
// crashed. Filtering is the answer for all of them; refusing is only for the unknown.
test("every iterator helper is filtered, and none of them throws on one's own data", () => {
const set = new Set<Item>([MINE, FOREIGN]);
const { caps } = setup("alice"); // alice holds MINE only
const view = makeReadFilteredView(set, caps) as any;
expect(view.toArray().map((i: Item) => i.id)).toEqual(["a"]);
expect(view.first().id).toBe("a");
expect(view.take(1).map((i: Item) => i.id)).toEqual(["a"]);
expect(view.drop(1)).toEqual([]);
expect(view.flatMap((i: Item) => [i.id])).toEqual(["a"]);
expect(view.reduce((acc: string, i: Item) => acc + i.id, "")).toBe("a");
});
// An unknown member must REFUSE, not forward: forwarding is a silent leak, and this
// view's one job is that it cannot show more than the holder may read.
test("an unfiltered member throws rather than leaking", () => {