From e489a1c4538a16b8b1137d0932a89cad5dce1de7 Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Sun, 9 Aug 2026 23:18:05 +0200 Subject: [PATCH 1/3] fix: name the download outcome and seal the Core Kit store at rest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A save returned a bare boolean, which could not tell a browser that never fetched from a stream the broker gave up on. `whenStreamIdle` resolves true for any ticket a body ever claimed, so a read that died after its first byte reported success and left a truncated file with no banner; the batch loop then read the same false as a refusal and dropped every file after it. `save` now returns 'saved' | 'refused' | 'failed', and the hook subscribes to `onStreamError` for its own ticket so a broker-abandoned read sets the error. `saveAll` owns the loop: it stops only at a refusal and names the files whose reads failed. The Core Kit store now reaches localStorage as AES-GCM ciphertext under a non-extractable key kept in IndexedDB, minted under a Web Lock so tabs that cold-start together share one. A value this device cannot open — a store written before the seal, or one whose key was evicted — is dropped on read, which costs one re-login rather than a wedge. Logout takes the wrapping key with the store. Also replaces the real-timer deadline in the broker's port-replacement test with fake timers; the ~15 ms of margin it left made it flaky. --- apps/web/src/auth/CoreKitProvider.test.tsx | 72 ++++--- apps/web/src/auth/coreKit.test.ts | 61 ++++-- apps/web/src/auth/coreKit.ts | 47 ++-- apps/web/src/auth/sealedStore.test.ts | 157 ++++++++++++++ apps/web/src/auth/sealedStore.ts | 202 ++++++++++++++++++ .../file-browser/FileBrowserActions.tsx | 11 +- apps/web/src/hooks/useFileDownload.test.ts | 130 +++++++++-- apps/web/src/hooks/useFileDownload.ts | 73 +++++-- apps/web/src/test/storeFakes.ts | 54 +++++ packages/client/src/media/broker.test.ts | 28 ++- 10 files changed, 725 insertions(+), 110 deletions(-) create mode 100644 apps/web/src/auth/sealedStore.test.ts create mode 100644 apps/web/src/auth/sealedStore.ts create mode 100644 apps/web/src/test/storeFakes.ts diff --git a/apps/web/src/auth/CoreKitProvider.test.tsx b/apps/web/src/auth/CoreKitProvider.test.tsx index 6f8b88ad4..a5e82fd7c 100644 --- a/apps/web/src/auth/CoreKitProvider.test.tsx +++ b/apps/web/src/auth/CoreKitProvider.test.tsx @@ -2,13 +2,15 @@ import { act, renderHook, waitFor } from '@testing-library/react'; import { COREKIT_STATUS } from '@web3auth/mpc-core-kit'; import type { ReactNode } from 'react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { sealedTestStore } from '../test/storeFakes'; import { createCoreKitSession } from './coreKit'; +import type { SealedStore } from './sealedStore'; import { CoreKitProvider, useCoreKit } from './CoreKitProvider'; const STORE_KEY = 'corekit_store'; /** Every store shape the SDK's read throws on, since it parses and then indexes. */ const UNREADABLE_STORES: [string, string][] = [ - // What an evicted or half-flushed store looks like: the parse itself throws. + // What an evicted or half-flushed write looks like: the parse itself throws. ['a truncated write', '{"sessionId":"a-sessio'], // Parses cleanly, then the index step throws because `null` has no keys. ['a null literal', 'null'], @@ -18,6 +20,12 @@ const LOGGED_IN_STORE = '{"sessionId":"a-fresh-session-id"}'; /** The SDK's own feature check is a bare `fetch`, so a restore fails offline. */ const OFFLINE = new Error('Failed to fetch'); +/** The `IAsyncStorage` surface the SDK drives, which is what the seal implements. */ +interface AsyncStore { + getItem(key: string): Promise; + setItem(key: string, value: string): Promise; +} + // The SDK reads its store as `JSON.parse(raw || '{}')[key]` (`AsyncStorage.get`, // which `init` calls for `sessionId`) on both the restore and the login path, so // one unreadable blob defeats every later login too. The fake reproduces that @@ -28,27 +36,28 @@ const sdk = vi.hoisted(() => ({ })); vi.mock('@web3auth/mpc-core-kit', async (importOriginal) => { const actual = await importOriginal(); - const readStore = (): unknown => { - const parsed = JSON.parse(window.localStorage.getItem(STORE_KEY) || '{}') as Record< - string, - unknown - >; - return parsed.sessionId; - }; return { ...actual, Web3AuthMPCCoreKit: class { readonly _storageKey = STORE_KEY; + private readonly storage: AsyncStore; + constructor(options: { storage: AsyncStore }) { + this.storage = options.storage; + } get status(): string { return sdk.status; } + private async readStore(): Promise { + const raw = (await this.storage.getItem(STORE_KEY)) || '{}'; + void (JSON.parse(raw) as Record).sessionId; + } async init(): Promise { - readStore(); + await this.readStore(); if (sdk.initFailure) throw sdk.initFailure; } async loginWithOAuth(): Promise { - readStore(); - window.localStorage.setItem(STORE_KEY, LOGGED_IN_STORE); + await this.readStore(); + await this.storage.setItem(STORE_KEY, LOGGED_IN_STORE); sdk.status = actual.COREKIT_STATUS.LOGGED_IN; } commitChanges(): Promise { @@ -63,26 +72,31 @@ const ENV = { VITE_WEB3AUTH_VERIFIER: 'verifier', } satisfies Partial; -/** The provider over a real Core Kit session, so the store it owns is the real one. */ -function mount() { - return renderHook(() => useCoreKit(), { - wrapper: ({ children }: { children: ReactNode }) => ( - createCoreKitSession(ENV)}>{children} - ), - }); -} - describe('CoreKitProvider', () => { + let store: SealedStore; + + /** The provider over a real Core Kit session, so the store it owns is the real one. */ + function mount() { + return renderHook(() => useCoreKit(), { + wrapper: ({ children }: { children: ReactNode }) => ( + createCoreKitSession(ENV, store)}> + {children} + + ), + }); + } + beforeEach(() => { sdk.status = COREKIT_STATUS.NOT_INITIALIZED; sdk.initFailure = undefined; window.localStorage.clear(); + store = sealedTestStore(); }); it.each(UNREADABLE_STORES)( 'discards %s the restore could not read, and does not pass it off as a signed-out tab', - async (_shape, store) => { - window.localStorage.setItem(STORE_KEY, store); + async (_shape, seeded) => { + await store.setItem(STORE_KEY, seeded); const { result } = mount(); await waitFor(() => expect(result.current.status).toBe('ready')); @@ -95,8 +109,8 @@ describe('CoreKitProvider', () => { it.each(UNREADABLE_STORES)( 'leaves a login after a restore that failed on %s able to establish a session', - async (_shape, store) => { - window.localStorage.setItem(STORE_KEY, store); + async (_shape, seeded) => { + await store.setItem(STORE_KEY, seeded); const { result } = mount(); await waitFor(() => expect(result.current.status).toBe('ready')); @@ -105,29 +119,29 @@ describe('CoreKitProvider', () => { }); expect(result.current.session?.isLoggedIn()).toBe(true); - expect(window.localStorage.getItem(STORE_KEY)).toBe(LOGGED_IN_STORE); + await expect(store.getItem(STORE_KEY)).resolves.toBe(LOGGED_IN_STORE); } ); it('keeps a readable store when the restore failed for some other reason', async () => { - window.localStorage.setItem(STORE_KEY, SIGNED_OUT_STORE); + await store.setItem(STORE_KEY, SIGNED_OUT_STORE); sdk.initFailure = OFFLINE; const { result } = mount(); await waitFor(() => expect(result.current.status).toBe('ready')); expect(result.current.error).toMatch(/could not be restored/); - expect(window.localStorage.getItem(STORE_KEY)).toBe(SIGNED_OUT_STORE); + await expect(store.getItem(STORE_KEY)).resolves.toBe(SIGNED_OUT_STORE); }); it('keeps a store the restore read cleanly, session in it or not', async () => { - window.localStorage.setItem(STORE_KEY, SIGNED_OUT_STORE); + await store.setItem(STORE_KEY, SIGNED_OUT_STORE); const { result } = mount(); await waitFor(() => expect(result.current.status).toBe('ready')); expect(result.current.error).toBeNull(); expect(result.current.session?.isLoggedIn()).toBe(false); - expect(window.localStorage.getItem(STORE_KEY)).toBe(SIGNED_OUT_STORE); + await expect(store.getItem(STORE_KEY)).resolves.toBe(SIGNED_OUT_STORE); }); }); diff --git a/apps/web/src/auth/coreKit.test.ts b/apps/web/src/auth/coreKit.test.ts index e3a449144..ba25a0a91 100644 --- a/apps/web/src/auth/coreKit.test.ts +++ b/apps/web/src/auth/coreKit.test.ts @@ -1,9 +1,14 @@ import { COREKIT_STATUS } from '@web3auth/mpc-core-kit'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { createCoreKitSession } from './coreKit'; +import { MemoryKeys, sealedTestStore } from '../test/storeFakes'; +import type { SealedStore } from './sealedStore'; +import { createCoreKitSession, sealedCoreKitStore } from './coreKit'; const STORE_KEY = 'corekit_store'; +/** What the SDK writes under its one key; nothing here is real key material. */ +const SESSION = '{"sessionId":"not-a-real-session-id"}'; + // The SDK reaches for the Web3Auth network on construction; the seam is what // lets the persistence options it is handed be read back, and what drives the // login and logout outcomes a device can actually land in. @@ -48,6 +53,12 @@ const ENV = { const REFUSED = new Error('the session server is unreachable'); describe('the Core Kit store', () => { + let keys: MemoryKeys; + let store: SealedStore; + + /** A session over a store this test can seed and read back. */ + const session = () => createCoreKitSession(ENV, store); + beforeEach(() => { sdk.options = undefined; sdk.status = COREKIT_STATUS.LOGGED_IN; @@ -55,58 +66,74 @@ describe('the Core Kit store', () => { sdk.logoutError = undefined; sdk.logoutCalls = 0; window.localStorage.clear(); + keys = new MemoryKeys(); + store = sealedTestStore(keys); + }); + + it('hands the SDK the sealed store, so nothing it writes lands in the clear', async () => { + session(); + await store.setItem(STORE_KEY, SESSION); + + expect(sdk.options?.storage).toBe(store); + expect(window.localStorage.getItem(STORE_KEY)).not.toContain('sessionId'); }); - it('persists origin-wide, so a tab that did not log in can still be promoted to leader', () => { - createCoreKitSession(ENV); + it('keeps the ciphertext origin-wide, so a tab that did not log in can still be promoted', async () => { + // A store written before the seal is dropped on read, which is only + // observable if the default store is this origin's `localStorage`. + window.localStorage.setItem(STORE_KEY, SESSION); - expect(sdk.options?.storage).toBe(window.localStorage); + await expect(sealedCoreKitStore().getItem(STORE_KEY)).resolves.toBeNull(); + + expect(window.localStorage.getItem(STORE_KEY)).toBeNull(); }); it('caps how long a persisted session stays restorable at eight hours', () => { - createCoreKitSession(ENV); + session(); expect(sdk.options?.sessionTime).toBe(28_800); }); it('is cleared on logout, which the SDK leaves standing', async () => { - const session = createCoreKitSession(ENV); - window.localStorage.setItem(STORE_KEY, '{"sessionId":"a-session-id"}'); + const created = session(); + await store.setItem(STORE_KEY, SESSION); - await session.logout(); + await created.logout(); expect(sdk.logoutCalls).toBe(1); expect(window.localStorage.getItem(STORE_KEY)).toBeNull(); + expect(keys.held).toBeNull(); }); it('is cleared when the SDK refuses to log out, and the refusal still surfaces', async () => { - const session = createCoreKitSession(ENV); + const created = session(); sdk.logoutError = REFUSED; - window.localStorage.setItem(STORE_KEY, '{"sessionId":"a-session-id"}'); + await store.setItem(STORE_KEY, SESSION); - await expect(session.logout()).rejects.toThrow(REFUSED); + await expect(created.logout()).rejects.toThrow(REFUSED); expect(window.localStorage.getItem(STORE_KEY)).toBeNull(); + expect(keys.held).toBeNull(); }); it('is cleared on a sign-out the SDK has no session to end', async () => { - const session = createCoreKitSession(ENV); + const created = session(); sdk.status = COREKIT_STATUS.NOT_INITIALIZED; - window.localStorage.setItem(STORE_KEY, '{"sessionId":"a-session-id"}'); + await store.setItem(STORE_KEY, SESSION); - await session.logout(); + await created.logout(); expect(sdk.logoutCalls).toBe(0); expect(window.localStorage.getItem(STORE_KEY)).toBeNull(); }); it('is cleared when a login stops short of a session and the rollback is refused', async () => { - const session = createCoreKitSession(ENV); + const created = session(); sdk.statusAfterLogin = COREKIT_STATUS.REQUIRED_SHARE; sdk.logoutError = REFUSED; - window.localStorage.setItem(STORE_KEY, '{"sessionId":"a-session-id"}'); + await store.setItem(STORE_KEY, SESSION); - await expect(session.login('google')).rejects.toThrow(/needs approval or a recovery phrase/); + await expect(created.login('google')).rejects.toThrow(/needs approval or a recovery phrase/); expect(sdk.logoutCalls).toBe(1); expect(window.localStorage.getItem(STORE_KEY)).toBeNull(); diff --git a/apps/web/src/auth/coreKit.ts b/apps/web/src/auth/coreKit.ts index bb0840495..68e97e87e 100644 --- a/apps/web/src/auth/coreKit.ts +++ b/apps/web/src/auth/coreKit.ts @@ -9,6 +9,7 @@ import { COREKIT_STATUS, WEB3AUTH_NETWORK, Web3AuthMPCCoreKit } from '@web3auth/ import { tssLib } from '@toruslabs/tss-dkls-lib'; import { environment, loginEnv } from '../engine/config'; import type { LoginSecretExporter } from '../engine/loginHandoff'; +import { indexedDbWrappingKeys, SealedStore } from './sealedStore'; /** How a session was established; also the `authStore` login method. */ export type CoreKitLoginMethod = 'google' | 'email'; @@ -34,7 +35,7 @@ export interface CoreKitSession extends LoginSecretExporter { class Web3AuthSession implements CoreKitSession { constructor( private readonly coreKit: Web3AuthMPCCoreKit, - private readonly store: Storage, + private readonly store: SealedStore, private readonly verifier: string, private readonly clientId: string ) {} @@ -46,7 +47,7 @@ class Web3AuthSession implements CoreKitSession { // Only an unreadable store wedges the next login through that same read. // A restore that failed for any other reason — the SDK's feature check // has no network — must leave a good store standing. - if (!this.storeIsReadable()) this.clearStore(); + if (!(await this.storeIsReadable())) await this.clearStore(); throw failure; } } @@ -69,7 +70,7 @@ class Web3AuthSession implements CoreKitSession { // device approval are not built yet, so fail rather than half-log-in, and // end the partial session rather than leave it resident on the device. await this.coreKit.logout().catch(() => undefined); - this.clearStore(); + await this.clearStore(); throw new Error('this device needs approval or a recovery phrase before it can sign in'); } await this.coreKit.commitChanges(); @@ -87,7 +88,7 @@ class Web3AuthSession implements CoreKitSession { try { if (this.isLoggedIn()) await this.coreKit.logout(); } finally { - this.clearStore(); + await this.clearStore(); } } @@ -95,17 +96,18 @@ class Web3AuthSession implements CoreKitSession { * The SDK's own logout blanks its session id in place and leaves the rest of * its store standing — a device factor share among it, once MFA is reachable. * So every path that leaves this device without a usable session clears it - * here, whether the session ended, was refused, or was never readable. + * here, whether the session ended, was refused, or was never readable, and + * takes the wrapping key with it. */ - private clearStore(): void { - this.store.removeItem(this.coreKit._storageKey); + private clearStore(): Promise { + return this.store.purge(this.coreKit._storageKey); } /** The SDK reads its store as `JSON.parse(raw || '{}')[key]`; nothing else opens. */ - private storeIsReadable(): boolean { - const raw = this.store.getItem(this.coreKit._storageKey); - if (!raw) return true; + private async storeIsReadable(): Promise { try { + const raw = await this.store.getItem(this.coreKit._storageKey); + if (!raw) return true; const parsed: unknown = JSON.parse(raw); return typeof parsed === 'object' && parsed !== null; } catch { @@ -127,16 +129,25 @@ class Web3AuthSession implements CoreKitSession { */ const SESSION_SECONDS = 8 * 60 * 60; +/** + * This origin's Core Kit store: the ciphertext origin-wide in `localStorage`, + * the key that opens it in IndexedDB. + * + * Origin-wide by decision, not by default: a tab promoted to leader re-exports + * the login secret from its own restored Core Kit session + * (`EngineClient.promote`), so a per-tab store would strand every tab that did + * not itself log in. + */ +export function sealedCoreKitStore(): SealedStore { + return new SealedStore(window.localStorage, indexedDbWrappingKeys(), navigator.locks); +} + /** Builds this tab's Core Kit session from the build-time environment. */ -export function createCoreKitSession(env: Partial): CoreKitSession { +export function createCoreKitSession( + env: Partial, + store: SealedStore = sealedCoreKitStore() +): CoreKitSession { const { clientId, verifier } = loginEnv(env); - // Origin-wide by decision, not by default: a tab promoted to leader re-exports - // the login secret from its own restored Core Kit session - // (`EngineClient.promote`), so a per-tab store would strand every tab that did - // not itself log in. What sits in it is a secp256k1 scalar that both addresses - // and decrypts a Web3Auth-held record holding the shares an export needs, so - // it is bearer key material and `SESSION_SECONDS` is its only other bound. - const store = window.localStorage; const coreKit = new Web3AuthMPCCoreKit({ web3AuthClientId: clientId, diff --git a/apps/web/src/auth/sealedStore.test.ts b/apps/web/src/auth/sealedStore.test.ts new file mode 100644 index 000000000..679fcdc73 --- /dev/null +++ b/apps/web/src/auth/sealedStore.test.ts @@ -0,0 +1,157 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import { MemoryKeys, SerialLocks, sealedTestStore as sealed } from '../test/storeFakes'; + +const KEY = 'corekit_store'; + +/** Stands in for what the SDK writes; nothing here is real key material. */ +const STORE_VALUE = '{"sessionId":"not-a-real-session-id"}'; + +const rawEnvelope = (): { v: number; iv: string; ct: string } => + JSON.parse(window.localStorage.getItem(KEY) ?? 'null') as { v: number; iv: string; ct: string }; + +beforeEach(() => { + window.localStorage.clear(); +}); + +describe('sealing the Core Kit store', () => { + it('opens what it sealed', async () => { + const store = sealed(new MemoryKeys()); + + await store.setItem(KEY, STORE_VALUE); + + await expect(store.getItem(KEY)).resolves.toBe(STORE_VALUE); + }); + + it('leaves ciphertext in storage, never the value it was handed', async () => { + await sealed(new MemoryKeys()).setItem(KEY, STORE_VALUE); + + const raw = window.localStorage.getItem(KEY) ?? ''; + expect(raw).not.toContain('sessionId'); + expect(raw).not.toContain('not-a-real-session-id'); + expect(rawEnvelope().v).toBe(1); + }); + + it('mints a wrapping key whose bytes cannot leave WebCrypto', async () => { + const keys = new MemoryKeys(); + + await sealed(keys).setItem(KEY, STORE_VALUE); + + const wrapping = keys.held; + expect(wrapping?.extractable).toBe(false); + await expect(crypto.subtle.exportKey('raw', wrapping as CryptoKey)).rejects.toThrow(); + }); + + it('seals each write under its own nonce', async () => { + const store = sealed(new MemoryKeys()); + + await store.setItem(KEY, STORE_VALUE); + const first = rawEnvelope(); + await store.setItem(KEY, STORE_VALUE); + const second = rawEnvelope(); + + expect(second.iv).not.toBe(first.iv); + expect(second.ct).not.toBe(first.ct); + }); + + it('reads back what another store on this origin sealed', async () => { + const keys = new MemoryKeys(); + const locks = new SerialLocks(); + + await sealed(keys, locks).setItem(KEY, STORE_VALUE); + + await expect(sealed(keys, locks).getItem(KEY)).resolves.toBe(STORE_VALUE); + }); + + it('mints one wrapping key however many tabs cold-start at once', async () => { + const keys = new MemoryKeys(); + const locks = new SerialLocks(); + const first = sealed(keys, locks); + const second = sealed(keys, locks); + + await Promise.all([first.setItem(KEY, STORE_VALUE), second.setItem(KEY, STORE_VALUE)]); + + expect(keys.writes).toBe(1); + await expect(first.getItem(KEY)).resolves.toBe(STORE_VALUE); + }); +}); + +describe('a sealed store it cannot open', () => { + it('drops a store written before the seal rather than leaving it readable', async () => { + window.localStorage.setItem(KEY, STORE_VALUE); + const keys = new MemoryKeys(); + + await expect(sealed(keys).getItem(KEY)).resolves.toBeNull(); + + expect(window.localStorage.getItem(KEY)).toBeNull(); + expect(keys.writes).toBe(0); + }); + + it('drops a value sealed under a wrapping key this device no longer has', async () => { + const keys = new MemoryKeys(); + await sealed(keys).setItem(KEY, STORE_VALUE); + keys.held = null; + + await expect(sealed(keys).getItem(KEY)).resolves.toBeNull(); + + expect(window.localStorage.getItem(KEY)).toBeNull(); + }); + + it('refuses ciphertext that was edited under the key that seals it', async () => { + const keys = new MemoryKeys(); + await sealed(keys).setItem(KEY, STORE_VALUE); + const envelope = rawEnvelope(); + const flipped = envelope.ct.startsWith('A') + ? `B${envelope.ct.slice(1)}` + : `A${envelope.ct.slice(1)}`; + window.localStorage.setItem(KEY, JSON.stringify({ ...envelope, ct: flipped })); + + await expect(sealed(keys).getItem(KEY)).resolves.toBeNull(); + }); + + it('fails the read rather than dropping a session when the key store is unreachable', async () => { + const keys = new MemoryKeys(); + await sealed(keys).setItem(KEY, STORE_VALUE); + const stored = window.localStorage.getItem(KEY); + keys.refusal = new Error('the wrapping-key database is shut'); + + await expect(sealed(keys).getItem(KEY)).rejects.toThrow('the wrapping-key database is shut'); + + expect(window.localStorage.getItem(KEY)).toBe(stored); + }); + + it('retries the key store after a refusal rather than caching it', async () => { + const keys = new MemoryKeys(); + const store = sealed(keys); + keys.refusal = new Error('the wrapping-key database is shut'); + await expect(store.setItem(KEY, STORE_VALUE)).rejects.toThrow(); + + keys.refusal = null; + await store.setItem(KEY, STORE_VALUE); + + await expect(store.getItem(KEY)).resolves.toBe(STORE_VALUE); + }); +}); + +describe('purging the sealed store', () => { + it('takes the value and the key that opens it', async () => { + const keys = new MemoryKeys(); + const store = sealed(keys); + await store.setItem(KEY, STORE_VALUE); + + await store.purge(KEY); + + expect(window.localStorage.getItem(KEY)).toBeNull(); + expect(keys.held).toBeNull(); + }); + + it('clears the value even when the key store refuses to give up the key', async () => { + const keys = new MemoryKeys(); + const store = sealed(keys); + await store.setItem(KEY, STORE_VALUE); + keys.clear = () => Promise.reject(new Error('the wrapping-key database is shut')); + + await store.purge(KEY); + + expect(window.localStorage.getItem(KEY)).toBeNull(); + }); +}); diff --git a/apps/web/src/auth/sealedStore.ts b/apps/web/src/auth/sealedStore.ts new file mode 100644 index 000000000..830102d39 --- /dev/null +++ b/apps/web/src/auth/sealedStore.ts @@ -0,0 +1,202 @@ +/** + * The Core Kit store, sealed at rest. + * + * What the SDK keeps under `corekit_store` is a secp256k1 scalar that both + * addresses and decrypts the Web3Auth record holding the login secret — the + * root of the hierarchy in `blueprint/core.md`, so no rotation demotes it. This + * seals it under an AES-GCM key WebCrypto will not export, which takes it away + * from readers of storage at rest — a copied profile, a backup, forensics. It + * does not take it away from script on this origin, which can still call the + * key handle: that the key bytes never exist outside WebCrypto is the whole + * control, and is why it is minted here rather than derived anywhere else. + */ + +import type { LockManagerLike } from '@cipherbox/client'; + +const DB_NAME = 'cipherbox-corekit'; +const DB_VERSION = 1; +const KEY_STORE = 'wrapping-keys'; +const KEY_ID = 'corekit-store'; + +/** Tabs cold-start together; the loser of an unserialised race would overwrite the key. */ +const WRAPPING_KEY_LOCK = 'cipherbox-corekit-wrapping-key'; + +const IV_BYTES = 12; + +/** Bumped when the envelope shape changes. An older one is dropped, never migrated. */ +const ENVELOPE_VERSION = 1; + +/** Where the wrapping key lives. Its handle is storable; its bytes are not. */ +export interface WrappingKeyStore { + read(): Promise; + write(key: CryptoKey): Promise; + clear(): Promise; +} + +/** + * `IAsyncStorage` for the Core Kit SDK, which awaits every store read and write. + * A value it cannot open is dropped rather than surfaced, so an evicted key or a + * store written before this wrapper costs one re-login, not a wedged app. + */ +export class SealedStore { + private wrapping: Promise | null = null; + + constructor( + private readonly storage: Storage, + private readonly keys: WrappingKeyStore, + private readonly locks: LockManagerLike + ) {} + + async getItem(key: string): Promise { + const raw = this.storage.getItem(key); + if (raw === null) return null; + const envelope = decodeEnvelope(raw); + if (envelope === null) { + // A store written before this wrapper is a bearer capability sitting in + // the clear, so reading it is also the chance to be rid of it. + this.storage.removeItem(key); + return null; + } + // Resolved before the decrypt, so a key store that is merely unreachable + // fails the read rather than discarding a session it could still open. + const wrapping = await this.wrappingKey(); + try { + const opened = await crypto.subtle.decrypt( + { name: 'AES-GCM', iv: envelope.iv }, + wrapping, + envelope.sealed + ); + return new TextDecoder().decode(opened); + } catch { + this.storage.removeItem(key); + return null; + } + } + + async setItem(key: string, value: string): Promise { + const wrapping = await this.wrappingKey(); + const iv = crypto.getRandomValues(new Uint8Array(IV_BYTES)); + const sealed = await crypto.subtle.encrypt( + { name: 'AES-GCM', iv }, + wrapping, + new TextEncoder().encode(value) + ); + this.storage.setItem(key, encodeEnvelope(iv, new Uint8Array(sealed))); + } + + /** + * Drops the sealed value and the key that opens it. The value goes first: once + * it is gone the key opens nothing, so a key store that refuses still leaves + * this device with no session to steal. + */ + async purge(key: string): Promise { + this.storage.removeItem(key); + this.wrapping = null; + await this.keys.clear().catch(() => undefined); + } + + private async wrappingKey(): Promise { + const pending = (this.wrapping ??= this.loadOrMint()); + try { + return await pending; + } catch (failure) { + // A rejected load must not be remembered, or every later read replays it. + if (this.wrapping === pending) this.wrapping = null; + throw failure; + } + } + + private async loadOrMint(): Promise { + let key: CryptoKey | undefined; + await this.locks.request(WRAPPING_KEY_LOCK, { mode: 'exclusive' }, async () => { + key = (await this.keys.read()) ?? undefined; + if (key !== undefined) return; + const minted = await crypto.subtle.generateKey({ name: 'AES-GCM', length: 256 }, false, [ + 'encrypt', + 'decrypt', + ]); + await this.keys.write(minted); + key = minted; + }); + if (key === undefined) throw new Error('the Core Kit store has no wrapping key'); + return key; + } +} + +/** The wrapping key in IndexedDB: the handle survives a structured clone, the bytes do not. */ +export function indexedDbWrappingKeys(): WrappingKeyStore { + return { + read: async () => { + const held: unknown = await transact('readonly', (store) => store.get(KEY_ID)); + return held instanceof CryptoKey ? held : null; + }, + write: async (key) => { + await transact('readwrite', (store) => store.put(key, KEY_ID)); + }, + clear: async () => { + await transact('readwrite', (store) => store.delete(KEY_ID)); + }, + }; +} + +function openDatabase(): Promise { + return new Promise((resolve, reject) => { + const request = indexedDB.open(DB_NAME, DB_VERSION); + request.onupgradeneeded = () => request.result.createObjectStore(KEY_STORE); + request.onsuccess = () => resolve(request.result); + request.onerror = () => reject(request.error ?? new Error('the wrapping-key database is shut')); + }); +} + +async function transact( + mode: IDBTransactionMode, + run: (store: IDBObjectStore) => IDBRequest +): Promise { + const database = await openDatabase(); + try { + return await new Promise((resolve, reject) => { + const request = run(database.transaction(KEY_STORE, mode).objectStore(KEY_STORE)); + request.onsuccess = () => resolve(request.result); + request.onerror = () => reject(request.error ?? new Error('the wrapping-key store refused')); + }); + } finally { + database.close(); + } +} + +interface Envelope { + iv: Uint8Array; + sealed: Uint8Array; +} + +function encodeEnvelope(iv: Uint8Array, sealed: Uint8Array): string { + return JSON.stringify({ v: ENVELOPE_VERSION, iv: toBase64(iv), ct: toBase64(sealed) }); +} + +/** `null` for anything this build does not recognise, which the caller drops. */ +function decodeEnvelope(raw: string): Envelope | null { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return null; + } + if (typeof parsed !== 'object' || parsed === null) return null; + const { v, iv, ct } = parsed as { v?: unknown; iv?: unknown; ct?: unknown }; + if (v !== ENVELOPE_VERSION || typeof iv !== 'string' || typeof ct !== 'string') return null; + try { + return { iv: fromBase64(iv), sealed: fromBase64(ct) }; + } catch { + return null; + } +} + +function toBase64(bytes: Uint8Array): string { + let binary = ''; + for (const byte of bytes) binary += String.fromCharCode(byte); + return btoa(binary); +} + +function fromBase64(text: string): Uint8Array { + return Uint8Array.from(atob(text), (character) => character.charCodeAt(0)); +} diff --git a/apps/web/src/components/file-browser/FileBrowserActions.tsx b/apps/web/src/components/file-browser/FileBrowserActions.tsx index a32e01747..3cfe214fa 100644 --- a/apps/web/src/components/file-browser/FileBrowserActions.tsx +++ b/apps/web/src/components/file-browser/FileBrowserActions.tsx @@ -88,12 +88,11 @@ export function FileBrowserActions({ const downloadSelection = async (): Promise => { setDownloading(true); try { - // One save at a time, and none after a refusal: a browser that blocks the - // second download blocks every one after it. - for (const row of selection.rows) { - if (row.kind !== 'file') continue; - if (!(await downloads.save(row.id, row.name, row.bytes))) break; - } + await downloads.saveAll( + selection.rows + .filter((row) => row.kind === 'file') + .map((row) => ({ node: row.id, name: row.name, size: row.bytes })) + ); } finally { setDownloading(false); } diff --git a/apps/web/src/hooks/useFileDownload.test.ts b/apps/web/src/hooks/useFileDownload.test.ts index 71064ac54..4e184605d 100644 --- a/apps/web/src/hooks/useFileDownload.test.ts +++ b/apps/web/src/hooks/useFileDownload.test.ts @@ -3,7 +3,7 @@ import type { EngineClient, MediaService } from '@cipherbox/client'; import { act, renderHook, waitFor } from '@testing-library/react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { EngineProvider } from '../providers/EngineProvider'; -import { useFileDownload } from './useFileDownload'; +import { useFileDownload, type SaveOutcome, type SaveRequest } from './useFileDownload'; /** The pipe this tab gets; `null` is the browser without a Service Worker. */ const mediaControl = { create: (): MediaService | null => null }; @@ -13,6 +13,9 @@ vi.mock('../engine/createMediaService', () => ({ const NODE = new Uint8Array(16).fill(3); +const batch = (names: readonly string[]): SaveRequest[] => + names.map((name) => ({ node: NODE, name, size: 12n })); + /** * A pipe whose tickets only go idle when the test says so, which is what a * transfer still in flight looks like to the hook. @@ -21,6 +24,7 @@ function fakePipe() { const live = new Set(); const minted: string[] = []; const waiting = new Map void>(); + const listeners = new Set<(failure: { url: string; message: string }) => void>(); const service = { streaming: true, @@ -32,6 +36,10 @@ function fakePipe() { live.add(url); return url; }, + onStreamError: (listener: (failure: { url: string; message: string }) => void) => { + listeners.add(listener); + return () => listeners.delete(listener); + }, whenStreamIdle: (url: string) => new Promise((resolve) => { waiting.set(url, resolve); @@ -43,6 +51,7 @@ function fakePipe() { service, live, minted, + listeners, /** The transfer for this ticket ended, or the browser never began it. */ finish: async (url: string, read = true): Promise => { await act(async () => { @@ -50,6 +59,17 @@ function fakePipe() { await Promise.resolve(); }); }, + /** + * The broker gave up on this ticket's body. It settles the wait as read, + * which is what the broker itself does once a body has claimed the ticket. + */ + abandon: async (url: string, message: string): Promise => { + await act(async () => { + for (const listener of listeners) listener({ url, message }); + waiting.get(url)?.(true); + await Promise.resolve(); + }); + }, }; } @@ -95,10 +115,10 @@ describe('bounding the tickets a streamed save leaves live', () => { mediaControl.create = () => pipe.service; const { result } = mount(fakeEngine()); - let saved: boolean | null = null; + let saved: SaveOutcome | null = null; await act(async () => { - void result.current.save(NODE, 'notes.txt', 12n).then((ok) => { - saved = ok; + void result.current.save(NODE, 'notes.txt', 12n).then((outcome) => { + saved = outcome; }); await Promise.resolve(); }); @@ -110,7 +130,7 @@ describe('bounding the tickets a streamed save leaves live', () => { await pipe.finish('/stream/ticket-1'); - await waitFor(() => expect(saved).toBe(true)); + await waitFor(() => expect(saved).toBe('saved')); expect([...pipe.live]).toEqual([]); }); @@ -119,20 +139,55 @@ describe('bounding the tickets a streamed save leaves live', () => { mediaControl.create = () => pipe.service; const { result } = mount(fakeEngine()); - let saved: boolean | null = null; + let saved: SaveOutcome | null = null; await act(async () => { - void result.current.save(NODE, 'notes.txt', 12n).then((ok) => { - saved = ok; + void result.current.save(NODE, 'notes.txt', 12n).then((outcome) => { + saved = outcome; }); await Promise.resolve(); }); await pipe.finish('/stream/ticket-1', false); - await waitFor(() => expect(saved).toBe(false)); + await waitFor(() => expect(saved).toBe('refused')); expect(pipe.live.size).toBe(0); expect(result.current.error).toBe('the browser did not start the download'); }); + it('reports a stream the broker gave up on as a failure, not a completed save', async () => { + const pipe = fakePipe(); + mediaControl.create = () => pipe.service; + const { result } = mount(fakeEngine()); + + let saved: SaveOutcome | null = null; + await act(async () => { + void result.current.save(NODE, 'notes.txt', 12n).then((outcome) => { + saved = outcome; + }); + await Promise.resolve(); + }); + await pipe.abandon('/stream/ticket-1', 'the record is gone'); + + await waitFor(() => expect(saved).toBe('failed')); + expect(result.current.error).toBe('the record is gone'); + expect(pipe.live.size).toBe(0); + }); + + it('drops the failure listener with the ticket it watched', async () => { + const pipe = fakePipe(); + mediaControl.create = () => pipe.service; + const { result } = mount(fakeEngine()); + + await act(async () => { + void result.current.save(NODE, 'notes.txt', 12n); + await Promise.resolve(); + }); + expect(pipe.listeners.size).toBe(1); + + await pipe.finish('/stream/ticket-1'); + + await waitFor(() => expect(pipe.listeners.size).toBe(0)); + }); + it('leaves one live ticket however many files a caller saves in a loop', async () => { const pipe = fakePipe(); mediaControl.create = () => pipe.service; @@ -141,10 +196,9 @@ describe('bounding the tickets a streamed save leaves live', () => { const names = ['a.bin', 'b.bin', 'c.bin', 'd.bin', 'e.bin']; let done = false; await act(async () => { - void (async () => { - for (const name of names) await result.current.save(NODE, name, 12n); + void result.current.saveAll(batch(names)).then(() => { done = true; - })(); + }); await Promise.resolve(); }); @@ -174,6 +228,52 @@ describe('bounding the tickets a streamed save leaves live', () => { }); }); +describe('saving a selection', () => { + it('carries a failed file forward and names it, rather than dropping the rest', async () => { + const pipe = fakePipe(); + mediaControl.create = () => pipe.service; + const { result } = mount(fakeEngine()); + + let done = false; + await act(async () => { + void result.current.saveAll(batch(['a.bin', 'b.bin', 'c.bin'])).then(() => { + done = true; + }); + await Promise.resolve(); + }); + + await pipe.finish('/stream/ticket-1'); + await pipe.abandon('/stream/ticket-2', 'the record is gone'); + expect(pipe.minted).toHaveLength(3); + await pipe.finish('/stream/ticket-3'); + + await waitFor(() => expect(done).toBe(true)); + expect(clicked).toEqual(['/stream/ticket-1', '/stream/ticket-2', '/stream/ticket-3']); + expect(result.current.error).toBe('could not download b.bin'); + }); + + it('stops at the file the browser refused, since it will refuse the rest too', async () => { + const pipe = fakePipe(); + mediaControl.create = () => pipe.service; + const { result } = mount(fakeEngine()); + + let done = false; + await act(async () => { + void result.current.saveAll(batch(['a.bin', 'b.bin', 'c.bin'])).then(() => { + done = true; + }); + await Promise.resolve(); + }); + + await pipe.finish('/stream/ticket-1'); + await pipe.finish('/stream/ticket-2', false); + + await waitFor(() => expect(done).toBe(true)); + expect(pipe.minted).toHaveLength(2); + expect(result.current.error).toBe('the browser did not start the download'); + }); +}); + describe('the buffered fallback', () => { const originalCreate = URL.createObjectURL; const originalRevoke = URL.revokeObjectURL; @@ -183,14 +283,16 @@ describe('the buffered fallback', () => { URL.revokeObjectURL = originalRevoke; }); - it('reads through the facade and reports a refusal instead of saving', async () => { + it('reports a read the facade refused as this file failing, not as a refusal', async () => { const engine = fakeEngine(() => Promise.reject(new Error('the record is gone'))); const { result } = mount(engine); + let saved: SaveOutcome | null = null; await act(async () => { - await result.current.save(NODE, 'notes.txt', 12n); + saved = await result.current.save(NODE, 'notes.txt', 12n); }); + expect(saved).toBe('failed'); expect(clicked).toEqual([]); expect(result.current.error).toBe('the record is gone'); diff --git a/apps/web/src/hooks/useFileDownload.ts b/apps/web/src/hooks/useFileDownload.ts index 6df400420..79126c24a 100644 --- a/apps/web/src/hooks/useFileDownload.ts +++ b/apps/web/src/hooks/useFileDownload.ts @@ -24,16 +24,29 @@ const STREAM_START_MS = 30_000; const NEVER_FETCHED = 'the browser did not start the download'; +/** + * How a save ended. `refused` is the save never being attempted, which will hold + * for the next file too; `failed` is this one file's read giving out, which says + * nothing about the next. A stream that dies after its first byte still goes + * idle having been read, so only the broker's failure tells it from a whole file. + */ +export type SaveOutcome = 'saved' | 'refused' | 'failed'; + +/** One file of a batch save. */ +export interface SaveRequest { + readonly node: Uint8Array; + /** The name the file lands under on disk. */ + readonly name: string; + /** The engine's byte count; `null` forces the buffered read. */ + readonly size: bigint | null; +} + export interface FileDownload { error: string | null; - /** - * Resolves once the file's bytes have stopped moving, with whether it reached - * the browser at all — false is a refusal, and a caller working through a - * selection should stop rather than repeat it per file. - * - * @param size the engine's byte count; `null` forces the buffered read. - */ - save(node: Uint8Array, name: string, size: bigint | null): Promise; + /** Resolves once the file's bytes have stopped moving. */ + save(node: Uint8Array, name: string, size: bigint | null): Promise; + /** Saves each file in turn, stopping at a refusal and naming what failed. */ + saveAll(files: readonly SaveRequest[]): Promise; /** Drops a failure the user has moved on from. */ clearError(): void; } @@ -55,10 +68,10 @@ export function useFileDownload(): FileDownload { }, [media]); const save = useCallback( - async (node: Uint8Array, name: string, size: bigint | null): Promise => { + async (node: Uint8Array, name: string, size: bigint | null): Promise => { if (client === null) { setError('the engine is not ready yet'); - return false; + return 'refused'; } setError(null); @@ -66,12 +79,26 @@ export function useFileDownload(): FileDownload { const ticket = streamTicket(media, node, size, OPAQUE); if (ticket !== null) { tickets.current.add(ticket); + // Subscribed before the fetch it watches: a body that dies on its + // first window fails before a later subscribe would be listening. + let abandoned: string | null = null; + const unsubscribe = media.onStreamError((failure) => { + if (failure.url === ticket) abandoned ??= failure.message; + }); saveToDisk(ticket, name); try { const read = await media.whenStreamIdle(ticket, STREAM_START_MS); - if (!read) setError(NEVER_FETCHED); - return read; + if (abandoned !== null) { + setError(abandoned); + return 'failed'; + } + if (!read) { + setError(NEVER_FETCHED); + return 'refused'; + } + return 'saved'; } finally { + unsubscribe(); tickets.current.delete(ticket); media.revokeStreamUrl(ticket); } @@ -83,16 +110,32 @@ export function useFileDownload(): FileDownload { const url = URL.createObjectURL(new Blob([bytes], { type: OPAQUE })); saveToDisk(url, name); setTimeout(() => URL.revokeObjectURL(url), REVOKE_AFTER_MS); - return true; + return 'saved'; } catch (failure: unknown) { setError(errorMessage(failure)); - return false; + return 'failed'; } }, [client, media] ); - return { error, save, clearError: useCallback(() => setError(null), []) }; + const saveAll = useCallback( + async (files: readonly SaveRequest[]): Promise => { + const failed: string[] = []; + for (const file of files) { + const outcome = await save(file.node, file.name, file.size); + // A browser that blocks the second download blocks every one after it. + if (outcome === 'refused') break; + if (outcome === 'failed') failed.push(file.name); + } + // A per-file failure is reported here or nowhere: each save clears the + // banner the one before it set. + if (failed.length > 0) setError(`could not download ${failed.join(', ')}`); + }, + [save] + ); + + return { error, save, saveAll, clearError: useCallback(() => setError(null), []) }; } function saveToDisk(url: string, name: string): void { diff --git a/apps/web/src/test/storeFakes.ts b/apps/web/src/test/storeFakes.ts new file mode 100644 index 000000000..72b5c086d --- /dev/null +++ b/apps/web/src/test/storeFakes.ts @@ -0,0 +1,54 @@ +/** + * The browser seams the sealed Core Kit store sits on: jsdom has neither + * IndexedDB nor the Web Locks API, so both are substituted while WebCrypto — + * the part under test — stays real. + */ + +import type { LockGrant, LockManagerLike } from '@cipherbox/client'; +import { SealedStore, type WrappingKeyStore } from '../auth/sealedStore'; + +export class MemoryKeys implements WrappingKeyStore { + held: CryptoKey | null = null; + writes = 0; + /** Set to model a key store that is present but unreachable. */ + refusal: Error | null = null; + + read(): Promise { + return this.refusal ? Promise.reject(this.refusal) : Promise.resolve(this.held); + } + + write(key: CryptoKey): Promise { + this.writes += 1; + this.held = key; + return Promise.resolve(); + } + + clear(): Promise { + this.held = null; + return Promise.resolve(); + } +} + +/** One holder at a time, which is the only guarantee the wrapping key needs. */ +export class SerialLocks implements LockManagerLike { + private tail: Promise = Promise.resolve(); + + request( + name: string, + _options: unknown, + callback: (lock: LockGrant | null) => Promise + ): Promise { + const granted: LockGrant = { name }; + const run = this.tail.then(() => callback(granted)); + this.tail = run.catch(() => undefined); + return run; + } +} + +/** A sealed store over this origin's real `localStorage`. */ +export function sealedTestStore( + keys: WrappingKeyStore = new MemoryKeys(), + locks: LockManagerLike = new SerialLocks() +): SealedStore { + return new SealedStore(window.localStorage, keys, locks); +} diff --git a/packages/client/src/media/broker.test.ts b/packages/client/src/media/broker.test.ts index a05004839..52bcc92d3 100644 --- a/packages/client/src/media/broker.test.ts +++ b/packages/client/src/media/broker.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { MediaBroker, type MediaBrokerOptions, type MediaReader } from './broker.js'; import { EngineRequestError } from '../correlatedTransport.js'; @@ -617,17 +617,23 @@ describe('MediaBroker.whenIdle', () => { it('re-arms rather than settling when the port is replaced mid-save', async () => { // A killed worker re-brokers and re-opens; retiring the ticket here would - // 404 the retry the pipe is about to make. - const h = harness(20, { lingerMs: 10_000 }); - const idle = watch(h.broker.whenIdle(h.ticket, 40)); + // 404 the retry the pipe is about to make. Fake timers, because a real one + // overrunning the deadline expires the waiter and asserts the wrong thing. + vi.useFakeTimers(); + try { + const h = harness(20, { lingerMs: 10_000 }); + const idle = watch(h.broker.whenIdle(h.ticket, 40)); - await new Promise((resolve) => setTimeout(resolve, 25)); - const next = new MessageChannel(); - h.broker.serve(next.port1); - await new Promise((resolve) => setTimeout(resolve, 25)); + await vi.advanceTimersByTimeAsync(25); + const next = new MessageChannel(); + h.broker.serve(next.port1); + await vi.advanceTimersByTimeAsync(25); - expect(idle()).toBeNull(); - next.port1.close(); - next.port2.close(); + expect(idle()).toBeNull(); + next.port1.close(); + next.port2.close(); + } finally { + vi.useRealTimers(); + } }); }); From 36f5ee3134c3e1d15f507d0e52238a4d503e1643 Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Sun, 9 Aug 2026 23:42:29 +0200 Subject: [PATCH 2/3] fix: report the failed read from the broker and bind the sealed store's context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings from the simplify, security and crypto-privacy passes. The download failure now travels on `whenIdle`'s own result rather than through a per-save `onStreamError` subscription. The broker holds the message and the waiters are already indexed by ticket, so recording it there costs six lines and removes three defects the consumer-side patch carried: a latch that a re-opened ticket could never clear, a match on a URL the package documents as having several forms, and a dependence on `fail` notifying synchronously after it resolves the waiter — which nothing asserted, and whose regression direction was to report a truncated file as saved. `saveAll` no longer overwrites the message that stopped the batch: a failure before a refusal used to lose the refusal, which is the actionable half. On the sealed store: - `restore()` treated an unreachable key store as a corrupt one and purged the ciphertext, which is the exact case `getItem` throws rather than drops for. Only a parse failure condemns the store now. - A decrypt failure re-reads the key store once before giving up, so a tab that logged out and back in elsewhere does not force a second re-login here. - The seal binds the storage key and the envelope version as AAD. The version travels outside the sealed bytes, so a v1 ciphertext relabelled `v2` would otherwise open under a future build's semantics. - `indexedDB.open` rejects on `blocked`; it runs inside the wrapping-key lock, and an open that never settles would queue every tab on the origin at login. - The header no longer claims a copied profile is covered. `extractable: false` bars export to script, not presence on disk, and a whole-profile copy carries the IndexedDB key with it. - `createCoreKitSession` takes its store rather than defaulting to one, so the production composition sits at the composition root in `main.tsx`. --- apps/web/src/auth/coreKit.test.ts | 31 +++++++-- apps/web/src/auth/coreKit.ts | 26 +++++--- apps/web/src/auth/sealedStore.test.ts | 32 +++++++++ apps/web/src/auth/sealedStore.ts | 65 ++++++++++++++----- .../file-browser/FileBrowserActions.test.tsx | 11 ++-- .../file-browser/FileBrowserActions.tsx | 18 ++--- apps/web/src/hooks/useFileDownload.test.ts | 62 ++++++------------ apps/web/src/hooks/useFileDownload.ts | 37 +++++------ apps/web/src/main.tsx | 6 +- packages/client/src/media/broker.test.ts | 56 +++++++++++++--- packages/client/src/media/broker.ts | 34 +++++++--- packages/client/src/media/service.test.ts | 7 +- packages/client/src/media/service.ts | 6 +- 13 files changed, 261 insertions(+), 130 deletions(-) diff --git a/apps/web/src/auth/coreKit.test.ts b/apps/web/src/auth/coreKit.test.ts index ba25a0a91..5340e9ea3 100644 --- a/apps/web/src/auth/coreKit.test.ts +++ b/apps/web/src/auth/coreKit.test.ts @@ -2,7 +2,7 @@ import { COREKIT_STATUS } from '@web3auth/mpc-core-kit'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { MemoryKeys, sealedTestStore } from '../test/storeFakes'; import type { SealedStore } from './sealedStore'; -import { createCoreKitSession, sealedCoreKitStore } from './coreKit'; +import { createCoreKitSession } from './coreKit'; const STORE_KEY = 'corekit_store'; @@ -18,6 +18,7 @@ const sdk = vi.hoisted(() => ({ statusAfterLogin: 'LOGGED_IN', logoutError: undefined as Error | undefined, logoutCalls: 0, + initFailure: undefined as Error | undefined, })); vi.mock('@web3auth/mpc-core-kit', async (importOriginal) => { const actual = await importOriginal(); @@ -31,6 +32,9 @@ vi.mock('@web3auth/mpc-core-kit', async (importOriginal) => { get status(): string { return sdk.status; } + init(): Promise { + return sdk.initFailure ? Promise.reject(sdk.initFailure) : Promise.resolve(); + } async loginWithOAuth(): Promise { sdk.status = sdk.statusAfterLogin; } @@ -65,6 +69,7 @@ describe('the Core Kit store', () => { sdk.statusAfterLogin = COREKIT_STATUS.LOGGED_IN; sdk.logoutError = undefined; sdk.logoutCalls = 0; + sdk.initFailure = undefined; window.localStorage.clear(); keys = new MemoryKeys(); store = sealedTestStore(keys); @@ -78,12 +83,26 @@ describe('the Core Kit store', () => { expect(window.localStorage.getItem(STORE_KEY)).not.toContain('sessionId'); }); - it('keeps the ciphertext origin-wide, so a tab that did not log in can still be promoted', async () => { - // A store written before the seal is dropped on read, which is only - // observable if the default store is this origin's `localStorage`. - window.localStorage.setItem(STORE_KEY, SESSION); + it('is left standing when a restore failed only because the key store was unreachable', async () => { + // Seeded through a store of its own, so the session's has no key in hand + // and has to reach the one that is about to refuse. + await sealedTestStore(keys).setItem(STORE_KEY, SESSION); + const stored = window.localStorage.getItem(STORE_KEY); + keys.refusal = new Error('the wrapping-key database is shut'); + sdk.initFailure = REFUSED; + + await expect(session().restore()).rejects.toThrow(REFUSED); + + expect(window.localStorage.getItem(STORE_KEY)).toBe(stored); + expect(keys.held).not.toBeNull(); + }); + + it('is cleared when a restore found something the SDK could not parse', async () => { + const created = session(); + await store.setItem(STORE_KEY, '{"sessionId":"a-truncated-writ'); + sdk.initFailure = REFUSED; - await expect(sealedCoreKitStore().getItem(STORE_KEY)).resolves.toBeNull(); + await expect(created.restore()).rejects.toThrow(REFUSED); expect(window.localStorage.getItem(STORE_KEY)).toBeNull(); }); diff --git a/apps/web/src/auth/coreKit.ts b/apps/web/src/auth/coreKit.ts index 68e97e87e..d7e5bc656 100644 --- a/apps/web/src/auth/coreKit.ts +++ b/apps/web/src/auth/coreKit.ts @@ -47,7 +47,7 @@ class Web3AuthSession implements CoreKitSession { // Only an unreadable store wedges the next login through that same read. // A restore that failed for any other reason — the SDK's feature check // has no network — must leave a good store standing. - if (!(await this.storeIsReadable())) await this.clearStore(); + if (await this.storeIsCorrupt()) await this.clearStore(); throw failure; } } @@ -103,16 +103,26 @@ class Web3AuthSession implements CoreKitSession { return this.store.purge(this.coreKit._storageKey); } - /** The SDK reads its store as `JSON.parse(raw || '{}')[key]`; nothing else opens. */ - private async storeIsReadable(): Promise { + /** + * Whether the store opens but holds something the SDK's own read throws on — + * it reads as `JSON.parse(raw || '{}')[key]`, and nothing else opens. + */ + private async storeIsCorrupt(): Promise { + let raw: string | null; try { - const raw = await this.store.getItem(this.coreKit._storageKey); - if (!raw) return true; - const parsed: unknown = JSON.parse(raw); - return typeof parsed === 'object' && parsed !== null; + raw = await this.store.getItem(this.coreKit._storageKey); } catch { + // A store this device cannot reach is not a corrupt one, and purging it + // would destroy a session the next attempt could still open. return false; } + if (!raw) return false; + try { + const parsed: unknown = JSON.parse(raw); + return typeof parsed !== 'object' || parsed === null; + } catch { + return true; + } } _UNSAFE_exportTssKey(): Promise { @@ -145,7 +155,7 @@ export function sealedCoreKitStore(): SealedStore { /** Builds this tab's Core Kit session from the build-time environment. */ export function createCoreKitSession( env: Partial, - store: SealedStore = sealedCoreKitStore() + store: SealedStore ): CoreKitSession { const { clientId, verifier } = loginEnv(env); diff --git a/apps/web/src/auth/sealedStore.test.ts b/apps/web/src/auth/sealedStore.test.ts index 679fcdc73..766bf7093 100644 --- a/apps/web/src/auth/sealedStore.test.ts +++ b/apps/web/src/auth/sealedStore.test.ts @@ -62,6 +62,20 @@ describe('sealing the Core Kit store', () => { await expect(sealed(keys, locks).getItem(KEY)).resolves.toBe(STORE_VALUE); }); + it('re-reads the key store rather than trusting a memo another tab replaced', async () => { + const keys = new MemoryKeys(); + const locks = new SerialLocks(); + const reader = sealed(keys, locks); + await reader.setItem(KEY, STORE_VALUE); + await reader.getItem(KEY); + + // The other tab logs out and back in, which re-keys the store under it. + keys.held = null; + await sealed(keys, locks).setItem(KEY, STORE_VALUE); + + await expect(reader.getItem(KEY)).resolves.toBe(STORE_VALUE); + }); + it('mints one wrapping key however many tabs cold-start at once', async () => { const keys = new MemoryKeys(); const locks = new SerialLocks(); @@ -96,6 +110,24 @@ describe('a sealed store it cannot open', () => { expect(window.localStorage.getItem(KEY)).toBeNull(); }); + it('refuses ciphertext transplanted from another storage key', async () => { + const keys = new MemoryKeys(); + await sealed(keys).setItem(KEY, STORE_VALUE); + const envelope = window.localStorage.getItem(KEY) ?? ''; + window.localStorage.setItem('another_store', envelope); + + await expect(sealed(keys).getItem('another_store')).resolves.toBeNull(); + }); + + it('drops an envelope from a version this build does not know', async () => { + const keys = new MemoryKeys(); + await sealed(keys).setItem(KEY, STORE_VALUE); + window.localStorage.setItem(KEY, JSON.stringify({ ...rawEnvelope(), v: 2 })); + + await expect(sealed(keys).getItem(KEY)).resolves.toBeNull(); + expect(window.localStorage.getItem(KEY)).toBeNull(); + }); + it('refuses ciphertext that was edited under the key that seals it', async () => { const keys = new MemoryKeys(); await sealed(keys).setItem(KEY, STORE_VALUE); diff --git a/apps/web/src/auth/sealedStore.ts b/apps/web/src/auth/sealedStore.ts index 830102d39..1dcef66d7 100644 --- a/apps/web/src/auth/sealedStore.ts +++ b/apps/web/src/auth/sealedStore.ts @@ -4,11 +4,14 @@ * What the SDK keeps under `corekit_store` is a secp256k1 scalar that both * addresses and decrypts the Web3Auth record holding the login secret — the * root of the hierarchy in `blueprint/core.md`, so no rotation demotes it. This - * seals it under an AES-GCM key WebCrypto will not export, which takes it away - * from readers of storage at rest — a copied profile, a backup, forensics. It - * does not take it away from script on this origin, which can still call the - * key handle: that the key bytes never exist outside WebCrypto is the whole - * control, and is why it is minted here rather than derived anywhere else. + * seals it under an AES-GCM key WebCrypto will not export. + * + * What that buys is narrow, and worth stating so nothing is relaxed on the + * strength of it: it defeats a reader of `localStorage` alone — a scraping + * extension, a partial backup, a grep over a disk image. `extractable: false` + * bars export to script, not presence on disk, so a whole-profile copy carries + * the IndexedDB key with it; and script on this origin can open that database + * and call the handle without going through this module at all. */ import type { LockManagerLike } from '@cipherbox/client'; @@ -59,31 +62,46 @@ export class SealedStore { } // Resolved before the decrypt, so a key store that is merely unreachable // fails the read rather than discarding a session it could still open. - const wrapping = await this.wrappingKey(); - try { - const opened = await crypto.subtle.decrypt( - { name: 'AES-GCM', iv: envelope.iv }, - wrapping, - envelope.sealed - ); - return new TextDecoder().decode(opened); - } catch { - this.storage.removeItem(key); - return null; + let opened = await this.unseal(key, envelope, await this.wrappingKey()); + if (opened === null) { + // Another tab can have re-keyed the store since this one resolved its + // key, so a memo is not evidence the value is unopenable. + this.wrapping = null; + opened = await this.unseal(key, envelope, await this.wrappingKey()); } + if (opened === null) this.storage.removeItem(key); + return opened; } async setItem(key: string, value: string): Promise { const wrapping = await this.wrappingKey(); const iv = crypto.getRandomValues(new Uint8Array(IV_BYTES)); const sealed = await crypto.subtle.encrypt( - { name: 'AES-GCM', iv }, + { name: 'AES-GCM', iv, additionalData: context(key) }, wrapping, new TextEncoder().encode(value) ); this.storage.setItem(key, encodeEnvelope(iv, new Uint8Array(sealed))); } + /** `null` for anything the key does not authenticate under this envelope. */ + private async unseal( + key: string, + envelope: Envelope, + wrapping: CryptoKey + ): Promise { + try { + const opened = await crypto.subtle.decrypt( + { name: 'AES-GCM', iv: envelope.iv, additionalData: context(key) }, + wrapping, + envelope.sealed + ); + return new TextDecoder().decode(opened); + } catch { + return null; + } + } + /** * Drops the sealed value and the key that opens it. The value goes first: once * it is gone the key opens nothing, so a key store that refuses still leaves @@ -145,6 +163,9 @@ function openDatabase(): Promise { request.onupgradeneeded = () => request.result.createObjectStore(KEY_STORE); request.onsuccess = () => resolve(request.result); request.onerror = () => reject(request.error ?? new Error('the wrapping-key database is shut')); + // An open that neither succeeds nor errors would strand the wrapping-key + // lock, and every tab on the origin queues behind it at login. + request.onblocked = () => reject(new Error('the wrapping-key database is held open')); }); } @@ -169,6 +190,16 @@ interface Envelope { sealed: Uint8Array; } +/** + * What the ciphertext is authenticated against. The version travels outside the + * sealed bytes, so binding it here is what stops a v1 envelope relabelled `v2` + * from opening under a future build's semantics; the storage key stops one slot's + * ciphertext from being transplanted into another. + */ +function context(key: string): Uint8Array { + return new TextEncoder().encode(`cipherbox:sealed-store:v${ENVELOPE_VERSION}:${key}`); +} + function encodeEnvelope(iv: Uint8Array, sealed: Uint8Array): string { return JSON.stringify({ v: ENVELOPE_VERSION, iv: toBase64(iv), ct: toBase64(sealed) }); } diff --git a/apps/web/src/components/file-browser/FileBrowserActions.test.tsx b/apps/web/src/components/file-browser/FileBrowserActions.test.tsx index fbd9c00e3..8d54bf489 100644 --- a/apps/web/src/components/file-browser/FileBrowserActions.test.tsx +++ b/apps/web/src/components/file-browser/FileBrowserActions.test.tsx @@ -746,13 +746,14 @@ describe('the vault browser read path over the streaming pipe', () => { /** Whether the browser opens the save the ticket was minted for. */ let fetched = true; /** Set by a test that drives the transfers itself, keyed by ticket url. */ - let transfers: Map void> | null = null; + let transfers: Map void> | null = + null; const streamListeners = new Set<(failure: MediaStreamFailure) => void>(); /** The browser finished reading this ticket. */ const endTransfer = async (url: string): Promise => { await act(async () => { - transfers?.get(url)?.(fetched); + transfers?.get(url)?.({ read: fetched, failure: null }); await Promise.resolve(); }); }; @@ -793,8 +794,10 @@ describe('the vault browser read path over the streaming pipe', () => { }, whenStreamIdle: (url: string) => { const held = transfers; - if (held === null) return Promise.resolve(fetched); - return new Promise((resolve) => held.set(url, resolve)); + if (held === null) return Promise.resolve({ read: fetched, failure: null }); + return new Promise<{ read: boolean; failure: string | null }>((resolve) => + held.set(url, resolve) + ); }, onStreamError: (listener: (failure: MediaStreamFailure) => void) => { streamListeners.add(listener); diff --git a/apps/web/src/components/file-browser/FileBrowserActions.tsx b/apps/web/src/components/file-browser/FileBrowserActions.tsx index 3cfe214fa..77251c283 100644 --- a/apps/web/src/components/file-browser/FileBrowserActions.tsx +++ b/apps/web/src/components/file-browser/FileBrowserActions.tsx @@ -7,7 +7,7 @@ import { useState } from 'react'; import { toHex } from '@cipherbox/client'; import { useContextMenu } from '../../hooks/useContextMenu'; -import { useFileDownload } from '../../hooks/useFileDownload'; +import { useFileDownload, type SaveRequest } from '../../hooks/useFileDownload'; import { useVaultActions, type BatchOutcome } from '../../hooks/useVaultActions'; import type { ListingRow } from '../../vault/listing'; import { previewKind } from '../../vault/previewKind'; @@ -22,6 +22,12 @@ import { NamePromptDialog } from './NamePromptDialog'; import { SelectionActionBar } from './SelectionActionBar'; import { TextEditorDialog } from './TextEditorDialog'; +const saveRequest = (row: ListingRow): SaveRequest => ({ + node: row.id, + name: row.name, + size: row.bytes, +}); + type Dialog = | { kind: 'create' } | { kind: 'rename' | 'details' | 'preview' | 'edit'; row: ListingRow } @@ -88,11 +94,7 @@ export function FileBrowserActions({ const downloadSelection = async (): Promise => { setDownloading(true); try { - await downloads.saveAll( - selection.rows - .filter((row) => row.kind === 'file') - .map((row) => ({ node: row.id, name: row.name, size: row.bytes })) - ); + await downloads.saveAll(selection.rows.filter((row) => row.kind === 'file').map(saveRequest)); } finally { setDownloading(false); } @@ -110,7 +112,7 @@ export function FileBrowserActions({ } items.push({ label: 'download', - onSelect: () => void downloads.save(row.id, row.name, row.bytes), + onSelect: () => void downloads.save(saveRequest(row)), }); } items.push( @@ -235,7 +237,7 @@ export function FileBrowserActions({ void downloads.save(dialog.row.id, dialog.row.name, dialog.row.bytes)} + onDownload={() => void downloads.save(saveRequest(dialog.row))} /> )} diff --git a/apps/web/src/hooks/useFileDownload.test.ts b/apps/web/src/hooks/useFileDownload.test.ts index 4e184605d..41c59ad1b 100644 --- a/apps/web/src/hooks/useFileDownload.test.ts +++ b/apps/web/src/hooks/useFileDownload.test.ts @@ -13,8 +13,9 @@ vi.mock('../engine/createMediaService', () => ({ const NODE = new Uint8Array(16).fill(3); -const batch = (names: readonly string[]): SaveRequest[] => - names.map((name) => ({ node: NODE, name, size: 12n })); +const file = (name: string): SaveRequest => ({ node: NODE, name, size: 12n }); + +const batch = (names: readonly string[]): SaveRequest[] => names.map(file); /** * A pipe whose tickets only go idle when the test says so, which is what a @@ -23,8 +24,7 @@ const batch = (names: readonly string[]): SaveRequest[] => function fakePipe() { const live = new Set(); const minted: string[] = []; - const waiting = new Map void>(); - const listeners = new Set<(failure: { url: string; message: string }) => void>(); + const waiting = new Map void>(); const service = { streaming: true, @@ -36,12 +36,8 @@ function fakePipe() { live.add(url); return url; }, - onStreamError: (listener: (failure: { url: string; message: string }) => void) => { - listeners.add(listener); - return () => listeners.delete(listener); - }, whenStreamIdle: (url: string) => - new Promise((resolve) => { + new Promise((resolve: (outcome: { read: boolean; failure: string | null }) => void) => { waiting.set(url, resolve); }), revokeStreamUrl: (url: string) => live.delete(url), @@ -51,22 +47,20 @@ function fakePipe() { service, live, minted, - listeners, /** The transfer for this ticket ended, or the browser never began it. */ finish: async (url: string, read = true): Promise => { await act(async () => { - waiting.get(url)?.(read); + waiting.get(url)?.({ read, failure: null }); await Promise.resolve(); }); }, /** - * The broker gave up on this ticket's body. It settles the wait as read, - * which is what the broker itself does once a body has claimed the ticket. + * The broker gave up on this ticket's body. It still went idle having been + * read, which is exactly what makes the failure the only signal. */ - abandon: async (url: string, message: string): Promise => { + abandon: async (url: string, failure: string): Promise => { await act(async () => { - for (const listener of listeners) listener({ url, message }); - waiting.get(url)?.(true); + waiting.get(url)?.({ read: true, failure }); await Promise.resolve(); }); }, @@ -117,7 +111,7 @@ describe('bounding the tickets a streamed save leaves live', () => { let saved: SaveOutcome | null = null; await act(async () => { - void result.current.save(NODE, 'notes.txt', 12n).then((outcome) => { + void result.current.save(file('notes.txt')).then((outcome) => { saved = outcome; }); await Promise.resolve(); @@ -141,7 +135,7 @@ describe('bounding the tickets a streamed save leaves live', () => { let saved: SaveOutcome | null = null; await act(async () => { - void result.current.save(NODE, 'notes.txt', 12n).then((outcome) => { + void result.current.save(file('notes.txt')).then((outcome) => { saved = outcome; }); await Promise.resolve(); @@ -160,7 +154,7 @@ describe('bounding the tickets a streamed save leaves live', () => { let saved: SaveOutcome | null = null; await act(async () => { - void result.current.save(NODE, 'notes.txt', 12n).then((outcome) => { + void result.current.save(file('notes.txt')).then((outcome) => { saved = outcome; }); await Promise.resolve(); @@ -172,22 +166,6 @@ describe('bounding the tickets a streamed save leaves live', () => { expect(pipe.live.size).toBe(0); }); - it('drops the failure listener with the ticket it watched', async () => { - const pipe = fakePipe(); - mediaControl.create = () => pipe.service; - const { result } = mount(fakeEngine()); - - await act(async () => { - void result.current.save(NODE, 'notes.txt', 12n); - await Promise.resolve(); - }); - expect(pipe.listeners.size).toBe(1); - - await pipe.finish('/stream/ticket-1'); - - await waitFor(() => expect(pipe.listeners.size).toBe(0)); - }); - it('leaves one live ticket however many files a caller saves in a loop', async () => { const pipe = fakePipe(); mediaControl.create = () => pipe.service; @@ -202,10 +180,10 @@ describe('bounding the tickets a streamed save leaves live', () => { await Promise.resolve(); }); - for (let file = 1; file <= names.length; file += 1) { + for (let nth = 1; nth <= names.length; nth += 1) { expect(pipe.live.size).toBe(1); - expect(pipe.minted).toHaveLength(file); - await pipe.finish(`/stream/ticket-${file}`); + expect(pipe.minted).toHaveLength(nth); + await pipe.finish(`/stream/ticket-${nth}`); } await waitFor(() => expect(done).toBe(true)); @@ -218,7 +196,7 @@ describe('bounding the tickets a streamed save leaves live', () => { const { result, unmount } = mount(fakeEngine()); await act(async () => { - void result.current.save(NODE, 'notes.txt', 12n); + void result.current.save(file('notes.txt')); await Promise.resolve(); }); expect(pipe.live.size).toBe(1); @@ -289,7 +267,7 @@ describe('the buffered fallback', () => { let saved: SaveOutcome | null = null; await act(async () => { - saved = await result.current.save(NODE, 'notes.txt', 12n); + saved = await result.current.save(file('notes.txt')); }); expect(saved).toBe('failed'); @@ -310,8 +288,8 @@ describe('the buffered fallback', () => { vi.useFakeTimers(); try { await act(async () => { - await result.current.save(NODE, 'a.bin', null); - await result.current.save(NODE, 'b.bin', null); + await result.current.save({ node: NODE, name: 'a.bin', size: null }); + await result.current.save({ node: NODE, name: 'b.bin', size: null }); }); expect(clicked).toEqual(['blob:fake/1', 'blob:fake/2']); expect(revoked).toEqual([]); diff --git a/apps/web/src/hooks/useFileDownload.ts b/apps/web/src/hooks/useFileDownload.ts index 79126c24a..5478962b2 100644 --- a/apps/web/src/hooks/useFileDownload.ts +++ b/apps/web/src/hooks/useFileDownload.ts @@ -27,12 +27,12 @@ const NEVER_FETCHED = 'the browser did not start the download'; /** * How a save ended. `refused` is the save never being attempted, which will hold * for the next file too; `failed` is this one file's read giving out, which says - * nothing about the next. A stream that dies after its first byte still goes - * idle having been read, so only the broker's failure tells it from a whole file. + * nothing about the next. `saved` means the broker did not give up on the read, + * which is as much as this tab can know once the bytes are the browser's. */ export type SaveOutcome = 'saved' | 'refused' | 'failed'; -/** One file of a batch save. */ +/** One file to save. */ export interface SaveRequest { readonly node: Uint8Array; /** The name the file lands under on disk. */ @@ -44,7 +44,7 @@ export interface SaveRequest { export interface FileDownload { error: string | null; /** Resolves once the file's bytes have stopped moving. */ - save(node: Uint8Array, name: string, size: bigint | null): Promise; + save(file: SaveRequest): Promise; /** Saves each file in turn, stopping at a refusal and naming what failed. */ saveAll(files: readonly SaveRequest[]): Promise; /** Drops a failure the user has moved on from. */ @@ -68,7 +68,7 @@ export function useFileDownload(): FileDownload { }, [media]); const save = useCallback( - async (node: Uint8Array, name: string, size: bigint | null): Promise => { + async ({ node, name, size }: SaveRequest): Promise => { if (client === null) { setError('the engine is not ready yet'); return 'refused'; @@ -79,26 +79,19 @@ export function useFileDownload(): FileDownload { const ticket = streamTicket(media, node, size, OPAQUE); if (ticket !== null) { tickets.current.add(ticket); - // Subscribed before the fetch it watches: a body that dies on its - // first window fails before a later subscribe would be listening. - let abandoned: string | null = null; - const unsubscribe = media.onStreamError((failure) => { - if (failure.url === ticket) abandoned ??= failure.message; - }); - saveToDisk(ticket, name); try { - const read = await media.whenStreamIdle(ticket, STREAM_START_MS); - if (abandoned !== null) { - setError(abandoned); + saveToDisk(ticket, name); + const idle = await media.whenStreamIdle(ticket, STREAM_START_MS); + if (idle.failure !== null) { + setError(idle.failure); return 'failed'; } - if (!read) { + if (!idle.read) { setError(NEVER_FETCHED); return 'refused'; } return 'saved'; } finally { - unsubscribe(); tickets.current.delete(ticket); media.revokeStreamUrl(ticket); } @@ -123,14 +116,16 @@ export function useFileDownload(): FileDownload { async (files: readonly SaveRequest[]): Promise => { const failed: string[] = []; for (const file of files) { - const outcome = await save(file.node, file.name, file.size); + const outcome = await save(file); // A browser that blocks the second download blocks every one after it. if (outcome === 'refused') break; if (outcome === 'failed') failed.push(file.name); } - // A per-file failure is reported here or nowhere: each save clears the - // banner the one before it set. - if (failed.length > 0) setError(`could not download ${failed.join(', ')}`); + if (failed.length === 0) return; + // Each save clears the banner the one before it set, so the batch reports + // here or nowhere; whatever stopped it keeps the last word. + const summary = `could not download ${failed.join(', ')}`; + setError((stopped) => (stopped === null ? summary : `${summary}; ${stopped}`)); }, [save] ); diff --git a/apps/web/src/main.tsx b/apps/web/src/main.tsx index c1efa3908..e88e0d78e 100644 --- a/apps/web/src/main.tsx +++ b/apps/web/src/main.tsx @@ -18,7 +18,7 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { BrowserRouter } from 'react-router-dom'; import { WagmiProvider } from 'wagmi'; import { App } from './App'; -import { createCoreKitSession } from './auth/coreKit'; +import { createCoreKitSession, sealedCoreKitStore } from './auth/coreKit'; import { CoreKitProvider } from './auth/CoreKitProvider'; import { createEngineClient } from './engine/createEngineClient'; import { installIntrospection } from './engine/introspection'; @@ -42,7 +42,9 @@ createRoot(rootElement).render( installIntrospection(createEngineClient(secrets))} > - createCoreKitSession(import.meta.env)}> + createCoreKitSession(import.meta.env, sealedCoreKitStore())} + > diff --git a/packages/client/src/media/broker.test.ts b/packages/client/src/media/broker.test.ts index 52bcc92d3..eee544712 100644 --- a/packages/client/src/media/broker.test.ts +++ b/packages/client/src/media/broker.test.ts @@ -1,6 +1,11 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; -import { MediaBroker, type MediaBrokerOptions, type MediaReader } from './broker.js'; +import { + MediaBroker, + type IdleOutcome, + type MediaBrokerOptions, + type MediaReader, +} from './broker.js'; import { EngineRequestError } from '../correlatedTransport.js'; import type { StreamHandle } from '../worker/protocol.js'; import type { MediaRequest, MediaResponse } from './protocol.js'; @@ -563,12 +568,12 @@ describe('MediaBroker', () => { describe('MediaBroker.whenIdle', () => { /** Records the settlement without awaiting it, so pending can be asserted. */ - function watch(promise: Promise): () => boolean | null { - let outcome: boolean | null = null; - void promise.then((read) => { - outcome = read; + function watch(promise: Promise): () => IdleOutcome | null { + let settled: IdleOutcome | null = null; + void promise.then((outcome) => { + settled = outcome; }); - return () => outcome; + return () => settled; } const startRead = async (h: Harness): Promise => { @@ -597,12 +602,12 @@ describe('MediaBroker.whenIdle', () => { h.send({ type: 'cb:media:close', requestId: 1 }); await waitFor(() => idle() !== null, 'the ticket to go idle'); - expect(idle()).toBe(true); + expect(idle()).toEqual({ read: true, failure: null }); }); it('reports a ticket no body ever claims as unread', async () => { const h = harness(20); - expect(await h.broker.whenIdle(h.ticket, 1)).toBe(false); + expect(await h.broker.whenIdle(h.ticket, 1)).toEqual({ read: false, failure: null }); }); it('settles a waiter when the ticket is revoked out from under the read', async () => { @@ -612,7 +617,40 @@ describe('MediaBroker.whenIdle', () => { h.broker.revoke(h.ticket); await waitFor(() => idle() !== null, 'the revoked ticket to settle'); - expect(idle()).toBe(true); + expect(idle()).toEqual({ read: true, failure: null }); + }); + + it('reports the read it gave up on, which a body that died still went idle from', async () => { + const h = harness(20, { lingerMs: 10_000 }); + const idle = watch(h.broker.whenIdle(h.ticket, 10_000)); + await startRead(h); + h.reader.failure = new Error('the record is gone'); + + h.send({ type: 'cb:media:pull', requestId: 1 }); + await waitFor(() => idle() !== null, 'the failed read to settle'); + + // Read, because a body claimed it — which is exactly why the boolean alone + // cannot tell a truncated transfer from a whole one. + expect(idle()).toEqual({ read: true, failure: 'the record is gone' }); + }); + + it('carries no failure to a waiter armed after the one it settled', async () => { + const h = harness(20, { lingerMs: 10_000 }); + const idle = watch(h.broker.whenIdle(h.ticket, 10_000)); + await startRead(h); + h.reader.failure = new Error('the record is gone'); + h.send({ type: 'cb:media:pull', requestId: 1 }); + await waitFor(() => idle() !== null, 'the failed read to settle'); + + // The pipe re-opens the ticket, as it does after a dropped handle. + const retried = watch(h.broker.whenIdle(h.ticket, 10_000)); + h.reader.failure = null; + h.send({ type: 'cb:media:open', requestId: 2, ticket: h.ticket, range: null }); + await waitFor(() => h.received.length === 4, 'the second head'); + h.send({ type: 'cb:media:close', requestId: 2 }); + await waitFor(() => retried() !== null, 'the retried ticket to go idle'); + + expect(retried()).toEqual({ read: true, failure: null }); }); it('re-arms rather than settling when the port is replaced mid-save', async () => { diff --git a/packages/client/src/media/broker.ts b/packages/client/src/media/broker.ts index 70ab676dc..2eab0659c 100644 --- a/packages/client/src/media/broker.ts +++ b/packages/client/src/media/broker.ts @@ -50,12 +50,21 @@ interface Pin { linger: ReturnType | null; } +/** How a ticket's reading ended, for a holder that waited it out. */ +export interface IdleOutcome { + /** Whether a body ever claimed the ticket. */ + readonly read: boolean; + /** Why this broker gave up on the last body, or `null` if it did not. */ + readonly failure: string | null; +} + /** A holder waiting for a ticket to stop being read. */ interface IdleWaiter { - readonly resolve: (read: boolean) => void; + readonly resolve: (outcome: IdleOutcome) => void; readonly startWithinMs: number; /** Set once a body claims the ticket, which is what the deadline waits for. */ read: boolean; + failure: string | null; timer: ReturnType | null; } @@ -132,19 +141,22 @@ export class MediaBroker { } /** - * Resolves once no response body is reading `ticket`, with whether one ever - * did. A ticket is a bearer capability to plaintext, so its holder needs this - * to know when dropping it can no longer cut a transfer short. + * Resolves once no response body is reading `ticket`, with how the reading + * ended. A ticket is a bearer capability to plaintext, so its holder needs + * this to know when dropping it can no longer cut a transfer short — and a + * body that died after its first byte still went idle having been read, so + * only the failure tells a short transfer from a whole one. * * @param startWithinMs how long to wait for a body to claim the ticket. A * claimed ticket has no deadline — a reader between windows is still reading. */ - whenIdle(ticket: string, startWithinMs: number): Promise { + whenIdle(ticket: string, startWithinMs: number): Promise { return new Promise((resolve) => { const waiter: IdleWaiter = { resolve, startWithinMs, read: (this.pins.get(ticket)?.cursors ?? 0) > 0, + failure: null, timer: null, }; this.arm(ticket, waiter); @@ -167,7 +179,7 @@ export class MediaBroker { const waiters = this.idleWaiters.get(ticket); waiters?.delete(waiter); if (waiters?.size === 0) this.idleWaiters.delete(ticket); - waiter.resolve(waiter.read); + settle(waiter); } private settleIdle(ticket: string): void { @@ -176,7 +188,7 @@ export class MediaBroker { this.idleWaiters.delete(ticket); for (const waiter of waiters) { if (waiter.timer !== null) clearTimeout(waiter.timer); - waiter.resolve(waiter.read); + settle(waiter); } } @@ -436,8 +448,10 @@ export class MediaBroker { private fail(port: MessagePortLike, requestId: number, cursor: Cursor, error: unknown): void { if (!this.isCurrent(requestId, cursor)) return; - this.drop(requestId); const message = errorMessage(error); + // Recorded before the drop, which is what settles the waiters. + for (const waiter of this.idleWaiters.get(cursor.ticket) ?? []) waiter.failure = message; + this.drop(requestId); post(port, { type: 'cb:media:error', requestId, message }); this.onFailure?.({ ticket: cursor.ticket, @@ -447,6 +461,10 @@ export class MediaBroker { } } +function settle(waiter: IdleWaiter): void { + waiter.resolve({ read: waiter.read, failure: waiter.failure }); +} + function post(port: MessagePortLike, response: MediaResponse): void { port.postMessage(response); } diff --git a/packages/client/src/media/service.test.ts b/packages/client/src/media/service.test.ts index c96864861..de2d81e0b 100644 --- a/packages/client/src/media/service.test.ts +++ b/packages/client/src/media/service.test.ts @@ -206,8 +206,11 @@ describe('MediaService', () => { service.revokeStreamUrl(url); // A deadline long enough that only an immediate resolve can finish the test. - expect(await service.whenStreamIdle(url, 60_000)).toBe(false); - expect(await service.whenStreamIdle('https://elsewhere.example/stream/x', 60_000)).toBe(false); + const unread = { read: false, failure: null }; + expect(await service.whenStreamIdle(url, 60_000)).toEqual(unread); + expect(await service.whenStreamIdle('https://elsewhere.example/stream/x', 60_000)).toEqual( + unread + ); }); it('re-brokers a fresh channel and closes the old one when the worker asks for a port', async () => { diff --git a/packages/client/src/media/service.ts b/packages/client/src/media/service.ts index 39d6fb7e2..04a236c7b 100644 --- a/packages/client/src/media/service.ts +++ b/packages/client/src/media/service.ts @@ -5,7 +5,7 @@ */ import { fanOut } from '../correlatedTransport.js'; -import { MediaBroker, type MediaFailure, type MediaReader } from './broker.js'; +import { MediaBroker, type IdleOutcome, type MediaFailure, type MediaReader } from './broker.js'; import { MEDIA_PORT_OFFER, MEDIA_PORT_REQUEST, @@ -121,10 +121,10 @@ export class MediaService { } /** {@link MediaBroker.whenIdle} for a ticket named by its URL. */ - whenStreamIdle(url: string, startWithinMs: number): Promise { + whenStreamIdle(url: string, startWithinMs: number): Promise { const ticket = ticketFromUrl(url, this.origin); if (ticket === null || this.registry.lookup(ticket) === undefined) - return Promise.resolve(false); + return Promise.resolve({ read: false, failure: null }); return this.broker.whenIdle(ticket, startWithinMs); } From fa1283ef411f8f3a5b7deec09bdfdebf26e107b8 Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Mon, 10 Aug 2026 19:51:19 +0000 Subject: [PATCH 3/3] test: pin the shared-ticket failure the idle outcome must keep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bodies can hold one ticket. When one fails and the last ends tidily, the outcome still names the failure — the truncated bytes the first one left are not made whole by the second finishing. whenIdle documents that the boolean cannot tell a short transfer from a whole one and that only the failure can, so discarding it there would reintroduce exactly the confusion the field exists to prevent. Behaviour is unchanged; the test pins a choice that was implicit. --- packages/client/src/media/broker.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/packages/client/src/media/broker.test.ts b/packages/client/src/media/broker.test.ts index eee544712..d03c640d4 100644 --- a/packages/client/src/media/broker.test.ts +++ b/packages/client/src/media/broker.test.ts @@ -653,6 +653,30 @@ describe('MediaBroker.whenIdle', () => { expect(retried()).toEqual({ read: true, failure: null }); }); + it('keeps the failure of one shared-ticket cursor when the last one ends clean', async () => { + // Two bodies on one ticket: the failed one truncated what it was reading, + // and the survivor ending tidily does not make those bytes whole. Naming + // the outcome saved here is the confusion `failure` exists to prevent. + const h = harness(20, { lingerMs: 10_000 }); + const idle = watch(h.broker.whenIdle(h.ticket, 10_000)); + + h.send({ type: 'cb:media:open', requestId: 1, ticket: h.ticket, range: null }); + await waitFor(() => h.received.length === 1, 'the first head'); + h.send({ type: 'cb:media:open', requestId: 2, ticket: h.ticket, range: null }); + await waitFor(() => h.received.length === 2, 'the second head'); + + h.reader.failure = new Error('the record is gone'); + h.send({ type: 'cb:media:pull', requestId: 1 }); + await waitFor(() => h.received.length === 3, 'the first cursor to fail'); + expect(idle()).toBeNull(); + + h.reader.failure = null; + h.send({ type: 'cb:media:close', requestId: 2 }); + await waitFor(() => idle() !== null, 'the last cursor to settle the ticket'); + + expect(idle()).toEqual({ read: true, failure: 'the record is gone' }); + }); + it('re-arms rather than settling when the port is replaced mid-save', async () => { // A killed worker re-brokers and re-opens; retiring the ticket here would // 404 the retry the pipe is about to make. Fake timers, because a real one