diff --git a/packages/sdk/e2e/sdk-entry.ts b/packages/sdk/e2e/sdk-entry.ts index e968ce1..fd3f00f 100644 --- a/packages/sdk/e2e/sdk-entry.ts +++ b/packages/sdk/e2e/sdk-entry.ts @@ -474,9 +474,12 @@ const identity = new IdentityStore( }, // spoof guard: depositing as another principal throws. async inboxSpoofGuard() { - const s = await sessionReady; - const target = await docs.docCreate(s.session_id, "Graph", "data:graph", "store", undefined); + // A REAL inbox, obtained from the system. It used to be a plain `docs.docCreate` + // document — a state the library never produces, and `inbox.post` now refuses it + // (a deposit is addressed to an inbox, never to a document). The step is about the + // `from` spoof guard; it should not also assert something the model forbids. setCurrentUser("alice"); + const target = await registryInternals.userInbox("alice", "protected"); let threw = false; try { await inbox.post(target, { payload: { x: 1 }, from: "bob" }); diff --git a/packages/sdk/src/emulated-verifier/branch-registers.ts b/packages/sdk/src/emulated-verifier/branch-registers.ts index 0e8f2ae..edb19ed 100644 --- a/packages/sdk/src/emulated-verifier/branch-registers.ts +++ b/packages/sdk/src/emulated-verifier/branch-registers.ts @@ -57,6 +57,7 @@ import { userInbox, createDoc, ensureAccount, + recordInbox, type VirtualUserRecord, } from "../shared-wallet/account-registry"; import type { InboxScope, Nuri, ReadCap, Scope } from "../model/types"; @@ -489,6 +490,10 @@ export async function openDocumentInbox(doc: Nuri): Promise { const record = await ensureAccount(holder); const store = record.docPrivate; getCaps().open(inbox, "private"); // its owner holds it, like any document of theirs + // …and the shim records that it IS an inbox, so a depositor can find that out without + // holding anything of it. See `recordInbox`: upstream a deposit cannot address a plain + // document at all, and this is what stands in for that impossibility. + await recordInbox(inbox); if (store) { try { await registerUpdate( diff --git a/packages/sdk/src/emulated-verifier/caps.ts b/packages/sdk/src/emulated-verifier/caps.ts index c19357d..cd9da33 100644 --- a/packages/sdk/src/emulated-verifier/caps.ts +++ b/packages/sdk/src/emulated-verifier/caps.ts @@ -280,7 +280,21 @@ export class CapRegistry { * arming their guard here would be enforcement this batch does not do. */ open(nuri: Nuri, scope: Scope): ReadCap { - const cap = this.mint(nuri); + // `file`, NOT `mint` — and the difference is a hole that was open for one commit. + // + // Every caller of this method files a STRUCTURAL document: one of the holder's three + // store documents, or an inbox. Those are not authored content, they are registers — + // written only through `emulated-verifier/register-write.ts`. Minting them marked + // them "created by me", which let the write guard through, which let a holder append + // `contains ""` to their own store index through the PUBLISHED + // `docs.sparqlUpdate` and forge ownership of it. `ownsDocument` reads that very + // index, so the guard was fully bypassable from the surface. + // + // Found by re-running the adversary on the fix (2026-08-07). Filing without minting + // closes it at the source: a structural document is owned by nobody in the authorship + // sense, so both halves of `assertMayWrite` say no, which is correct. + const cap = mintCap(nuri); + this.file(cap); if (scope === "public") this.markInPublicStore(nuri); return cap; } diff --git a/packages/sdk/src/emulated-verifier/read-filter.ts b/packages/sdk/src/emulated-verifier/read-filter.ts index c0edbc4..ec5b389 100644 --- a/packages/sdk/src/emulated-verifier/read-filter.ts +++ b/packages/sdk/src/emulated-verifier/read-filter.ts @@ -111,11 +111,26 @@ export function makeReadFilteredView(set: S, caps: CapRegistry // at all, so "yes it is in there" would be an answer the target cannot give. if (prop === "has") return (item: unknown) => keep(item) && (target as Set).has(item); // The reactive-set extras: they iterate, so they must iterate the filtered items. + // The list is `iteratorHelperKeys` from `@ng-org/alien-deepsignals` — an earlier + // pass whitelisted half of it and threw on the rest, so a holder's calls on their + // OWN data crashed (`toArray`, `reduce`, `first`…). Filtering is the answer for all + // of them; refusing is only for what is not on this list. if (prop === "map") return (fn: (v: unknown, i: number) => unknown) => kept(target).map(fn); if (prop === "filter") return (fn: (v: unknown, i: number) => boolean) => kept(target).filter(fn); if (prop === "find") return (fn: (v: unknown, i: number) => boolean) => kept(target).find(fn); if (prop === "some") return (fn: (v: unknown, i: number) => boolean) => kept(target).some(fn); if (prop === "every") return (fn: (v: unknown, i: number) => boolean) => kept(target).every(fn); + if (prop === "toArray") return () => kept(target); + if (prop === "first") return () => kept(target)[0]; + if (prop === "take") return (n: number) => kept(target).slice(0, n); + if (prop === "drop") return (n: number) => kept(target).slice(n); + if (prop === "flatMap") return (fn: (v: unknown, i: number) => unknown) => kept(target).flatMap(fn as never); + if (prop === "reduce") { + return (fn: (acc: unknown, v: unknown, i: number) => unknown, init?: unknown) => + init === undefined + ? kept(target).reduce(fn as never) + : kept(target).reduce(fn as never, init); + } if (prop === "getById" || prop === "getBy") { const inner = Reflect.get(target, prop, target) as ((...a: unknown[]) => unknown) | undefined; if (typeof inner !== "function") return inner; @@ -134,6 +149,19 @@ export function makeReadFilteredView(set: S, caps: CapRegistry return typeof fn === "function" ? fn.bind(target) : fn; } + // RAW ESCAPE HATCHES. `DeepSignalSet` exposes the underlying collection on + // dunder keys (`__raw__`, `__meta__` — `RAW_KEY` in `@ng-org/alien-deepsignals`), + // and the header used to claim "a plain property carries no items". It does here: + // `view.__raw__` handed back the unfiltered Set, every identity's items in it. + // Found by re-running the adversary on the fix (2026-08-07). Any dunder key is + // refused, because that is the convention the escape hatches follow. + if (typeof prop === "string" && prop.startsWith("__")) { + throw new Error( + `[ng-eventually] read filter: \`${prop}\` reaches past the view to the raw ` + + "collection, which holds every identity's items. There is no filtered form of it.", + ); + } + const v = Reflect.get(target, prop, target); if (typeof v !== "function") return v; // UNKNOWN function member: refuse rather than forward. See the header — forwarding diff --git a/packages/sdk/src/shared-wallet/account-registry.ts b/packages/sdk/src/shared-wallet/account-registry.ts index 1cb014b..3b9156e 100644 --- a/packages/sdk/src/shared-wallet/account-registry.ts +++ b/packages/sdk/src/shared-wallet/account-registry.ts @@ -124,6 +124,7 @@ export const P = { inboxCap: `${SHIM}:inboxCap`, // user branch → an inbox this user may READ inboxAddress: `${SHIM}:inboxAddress`, // header branch → WHERE to deposit for this document exposedReadCap: `${SHIM}:exposedReadCap`, // header branch → the cap a PUBLIC store serves to anyone + isInbox: `${SHIM}:isInbox`, // shim → this NURI IS an inbox (see `assertIsInbox`) } as const; // Fixed subject of the per-(account×scope) index document. The index doc plays // the role of the future store-container: it lists the NURIs of the entity @@ -311,6 +312,11 @@ export function resetRegistryCache(): void { inboxInFlight.clear(); shimDocNuri = null; shimDocInFlight = null; + // The inbox index is session state like the rest: a NURI confirmed as an inbox in one + // session must not be taken for one in the next. (Left out at first, and the full unit + // suite went red on a NURI collision between two files — a memo that outlives its + // session is exactly the fault this review found elsewhere in the tests.) + knownInboxes.clear(); } // --- SPARQL result helpers ------------------------------------------------ @@ -711,6 +717,68 @@ export async function ensureAccount(id: string): Promise { // --- resolvers ------------------------------------------------------------ +/** Fixed subject of the shim's inbox INDEX — see {@link recordInbox}. */ +const INBOX_INDEX_SUBJECT = `${SHIM}:inboxes`; + +/** + * Record that `nuri` IS an inbox, in the shim — the emulated counterpart of what the + * broker knows by construction upstream. + * + * Upstream you cannot address a repo with a deposit: `InboxPost` seals to an inbox + * PUBKEY, and the broker routes it through `inboxes: PubKey → RepoId` + * (`engine/verifier/src/verifier.rs:105,1677`). Depositing into a plain document is not + * refused there — it is unrepresentable. Here an inbox is a document like any other, so + * "is this an inbox?" has to be asked of something, and the shim is where the emulation + * keeps what the network would know. + * + * Written through the PHYSICAL door: which NURIs are inboxes is not one virtual user's + * business, exactly like the account records beside it. + */ +export async function recordInbox(nuri: Nuri): Promise { + const s = await session(); + const shimDoc = await resolveShimDoc(); + try { + await physicalUpdate( + s.sessionId, + `INSERT DATA { <${INBOX_INDEX_SUBJECT}> <${P.isInbox}> "${escapeLiteral(nuri)}" }`, + shimDoc, + "recordInbox", + ); + } catch (error) { + console.error(accessLogPrefix() + " recordInbox failed:", error); + } + knownInboxes.add(nuri); +} + +/** Inboxes this session has already confirmed — the index only ever grows. */ +const knownInboxes = new Set(); + +/** + * Is `nuri` an inbox? Asked of the shim, through the physical door — a depositor is not + * the inbox's owner and holds nothing of it, which is the whole point of an inbox. + */ +export async function isKnownInbox(nuri: Nuri): Promise { + if (knownInboxes.has(nuri)) return true; + const s = await session(); + const shimDoc = await resolveShimDoc(); + try { + const res = await physicalQuery( + s.sessionId, + `SELECT ?i WHERE { <${INBOX_INDEX_SUBJECT}> <${P.isInbox}> ?i }`, + undefined, + shimDoc, + "isKnownInbox", + ); + for (const row of readBindings(res)) { + const v = bindingValue(row, "i"); + if (v && isNuri(v)) knownInboxes.add(v); + } + } catch (error) { + console.error(accessLogPrefix() + " isKnownInbox failed:", error); + } + return knownInboxes.has(nuri); +} + /** The index document NURI of an account for a scope (the store-container). */ export function storeOf(record: VirtualUserRecord, scope: Scope): Nuri { return scope === "public" @@ -877,6 +945,7 @@ export async function userInbox(id: string, scope: InboxScope): Promise { const doc = await createDoc(); fileOwnInbox(id, doc); + await recordInbox(doc); try { await physicalUpdate( s.sessionId, diff --git a/packages/sdk/src/surface/inbox.ts b/packages/sdk/src/surface/inbox.ts index 05158ca..321ba06 100644 --- a/packages/sdk/src/surface/inbox.ts +++ b/packages/sdk/src/surface/inbox.ts @@ -32,7 +32,7 @@ import { subscribeDoc } from "./subscribe"; import { ensureRepoOpen } from "../emulated-verifier/open-repo"; import { getCaps, getCurrentUser, getStoreRegistryDeps } from "../shared-wallet/bootstrap"; import { addLink, documentInboxAddress, isOwnInbox } from "../emulated-verifier/branch-registers"; -import { userInbox } from "../shared-wallet/account-registry"; +import { userInbox, isKnownInbox } from "../shared-wallet/account-registry"; import { escapeLiteral } from "./sparql"; import { hasReadCap, toNuri } from "../model/nuri"; import { @@ -179,6 +179,23 @@ export async function post(targetInboxLike: NuriLike, opts: PostOptions): Promis <${P.payload}> "${payloadLiteral}" ; <${P.ts}> "${ts}"${fromTriple} . }`; + // The target must BE an inbox, and this is the one check standing between a deposit + // and an arbitrary write into someone else's document. + // + // Upstream the question does not arise: `InboxPost` seals to an inbox PUBKEY and the + // broker routes it by `inboxes: PubKey → RepoId` — addressing a plain repo with a + // deposit is not refused, it is unrepresentable. Here an inbox is a document like any + // other, so without this `inbox.post(someoneElsesDocument, …)` wrote four triples into + // it, through a published door that skips both guards by design. Found by re-running + // the adversary on the fix that un-published `depositInto` (2026-08-07) — moving that + // function was not enough, because `post` reaches the same door. + if (!(await isKnownInbox(targetInbox))) { + throw new Error( + "[ng-eventually] inbox.post: refused — this is not an inbox. A deposit is addressed " + + "to an inbox, never to a document; upstream the two cannot even be confused, " + + `because a deposit carries an inbox key and not a document reference. ${JSON.stringify(targetInbox)}`, + ); + } // A deposit crosses the boundary on purpose — see `register-write.depositInto`. await depositInto(sid, update, targetInbox, "deposit"); // Domain-level diagnostic (on top of docs.ts's generic access-path WRITE log): diff --git a/packages/sdk/test/cross-user-access.test.ts b/packages/sdk/test/cross-user-access.test.ts index f3e61d8..c8c4789 100644 --- a/packages/sdk/test/cross-user-access.test.ts +++ b/packages/sdk/test/cross-user-access.test.ts @@ -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 diff --git a/packages/sdk/test/inbox.test.ts b/packages/sdk/test/inbox.test.ts index d20acda..fb866f8 100644 --- a/packages/sdk/test/inbox.test.ts +++ b/packages/sdk/test/inbox.test.ts @@ -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: ` a ; "..." ; "..." [; "..."] .` // @@ -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"]); }); diff --git a/packages/sdk/test/read-filter.test.ts b/packages/sdk/test/read-filter.test.ts index c5b5c37..1dfda37 100644 --- a/packages/sdk/test/read-filter.test.ts +++ b/packages/sdk/test/read-filter.test.ts @@ -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([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([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", () => {