From 5570dda4a120787e0f556bf5f68336398cdac630 Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Fri, 7 Aug 2026 15:06:53 +0200 Subject: [PATCH 1/6] fix: wipe the transferred upload chunk in the engine worker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The chunk reaches EngineHost.pushChunk by transfer, so the worker realm is its terminal owner. wasm-bindgen copies the view into WASM memory before it returns, so scrub the JS-side plaintext once the push settles, on the failure path too — mirroring the existing start secret scrub. Closes #1037 --- packages/client/src/worker/engineHost.test.ts | 37 +++++++++++++++++++ packages/client/src/worker/engineHost.ts | 13 +++++-- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/packages/client/src/worker/engineHost.test.ts b/packages/client/src/worker/engineHost.test.ts index bfa3f101d0..9d3eeff0a6 100644 --- a/packages/client/src/worker/engineHost.test.ts +++ b/packages/client/src/worker/engineHost.test.ts @@ -42,6 +42,18 @@ function recordingWasm(): { wasm: EngineWasm; constructed: Constructed[] } { return { wasm, constructed }; } +/** A host whose WASM `pushChunk` hands the view it was given to `onPush`. */ +function pushingHost(onPush: (chunk: Uint8Array) => Promise): EngineHost { + const wasm = { + EngineHandle: class { + pushChunk(_handle: bigint, chunk: Uint8Array): Promise { + return onPush(chunk); + } + }, + } as unknown as EngineWasm; + return new EngineHost(wasm, {}, { apiBaseUrl: 'https://api.example.test' }); +} + describe('EngineHost', () => { it('hands the engine the API base URL so cold start can log in', () => { const { wasm, constructed } = recordingWasm(); @@ -92,4 +104,29 @@ describe('EngineHost', () => { expect(constructed[0].acceleratorBaseUrl).toBeUndefined(); expect(constructed[0].publicGateways).toBeUndefined(); }); + + it('wipes the transferred upload chunk once WASM has copied it', async () => { + const plaintext = Uint8Array.of(1, 2, 3, 4); + let copied: Uint8Array | undefined; + const host = pushingHost((chunk) => { + copied = Uint8Array.from(chunk); + return Promise.resolve(); + }); + + await host.pushChunk(7n, plaintext.buffer as ArrayBuffer); + + expect(copied).toEqual(Uint8Array.of(1, 2, 3, 4)); + expect(plaintext).toEqual(new Uint8Array(4)); + }); + + it('wipes the transferred upload chunk when the push rejects', async () => { + const plaintext = Uint8Array.of(5, 6, 7, 8); + const host = pushingHost(() => Promise.reject(new Error('staging full'))); + + await expect(host.pushChunk(7n, plaintext.buffer as ArrayBuffer)).rejects.toThrow( + 'staging full' + ); + + expect(plaintext).toEqual(new Uint8Array(4)); + }); }); diff --git a/packages/client/src/worker/engineHost.ts b/packages/client/src/worker/engineHost.ts index ac5c2d02d5..821b18af8e 100644 --- a/packages/client/src/worker/engineHost.ts +++ b/packages/client/src/worker/engineHost.ts @@ -26,6 +26,7 @@ export interface EngineHostLike { command(command: CommandDescriptor): Promise; /** Opens a write handle for `size` plaintext bytes; the engine reserves them. */ beginWrite(target: WriteTarget, size: number): Promise; + /** Takes ownership of `chunk`: the host scrubs the plaintext once it lands. */ pushChunk(handle: WriteHandle, chunk: ArrayBuffer): Promise; /** Closes the handle and journals its op; resolves with the durable op id. */ commitWrite(handle: WriteHandle): Promise; @@ -113,9 +114,15 @@ export class EngineHost implements EngineHostLike { } async pushChunk(handle: WriteHandle, chunk: ArrayBuffer): Promise { - // The handle copies into WASM memory synchronously; a view over the - // transferred buffer is safe here. - await this.handle.pushChunk(handle, new Uint8Array(chunk)); + // The chunk arrives by transfer, so the worker is its terminal owner: the + // handle copies it into WASM memory synchronously, and the JS-side + // plaintext is scrubbed rather than left for the collector. + const view = new Uint8Array(chunk); + try { + await this.handle.pushChunk(handle, view); + } finally { + view.fill(0); + } } commitWrite(handle: WriteHandle): Promise { From 357d6704fe3d129968d9bb82ed3aa9f7870e4e01 Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Fri, 7 Aug 2026 15:14:48 +0200 Subject: [PATCH 2/6] refactor: share one scrub-after-use helper for the host's plaintext buffers --- packages/client/src/worker/engineHost.ts | 32 +++++++++++++----------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/client/src/worker/engineHost.ts b/packages/client/src/worker/engineHost.ts index 821b18af8e..fd60faaa0c 100644 --- a/packages/client/src/worker/engineHost.ts +++ b/packages/client/src/worker/engineHost.ts @@ -81,17 +81,27 @@ export class EngineHost implements EngineHostLike { ); } - async start(secret: ArrayBuffer): Promise { - // The engine copies the secret into its `Zeroizing` store; scrub the - // worker's transferred copy immediately after so no plaintext lingers. - const view = new Uint8Array(secret); + /** + * Runs `use` over `buffer`, scrubbing it once the call settles — including + * when it rejects. Buffers reaching the host arrive by transfer, making the + * worker their terminal owner, and the engine below copies what it keeps. + */ + private async scrubbing( + buffer: ArrayBuffer, + use: (view: Uint8Array) => Promise + ): Promise { + const view = new Uint8Array(buffer); try { - await this.handle.start(view); + await use(view); } finally { view.fill(0); } } + start(secret: ArrayBuffer): Promise { + return this.scrubbing(secret, (view) => this.handle.start(view)); + } + async command(command: CommandDescriptor): Promise { await this.handle.command(buildCommand(this.wasm, command)); } @@ -113,16 +123,8 @@ export class EngineHost implements EngineHostLike { ); } - async pushChunk(handle: WriteHandle, chunk: ArrayBuffer): Promise { - // The chunk arrives by transfer, so the worker is its terminal owner: the - // handle copies it into WASM memory synchronously, and the JS-side - // plaintext is scrubbed rather than left for the collector. - const view = new Uint8Array(chunk); - try { - await this.handle.pushChunk(handle, view); - } finally { - view.fill(0); - } + pushChunk(handle: WriteHandle, chunk: ArrayBuffer): Promise { + return this.scrubbing(chunk, (view) => this.handle.pushChunk(handle, view)); } commitWrite(handle: WriteHandle): Promise { From 06467cbe97b32ff03730f23c0b2aa4cf7cadf51b Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Fri, 7 Aug 2026 15:19:38 +0200 Subject: [PATCH 3/6] fix: fail closed on an unknown or wrong-typed command in the worker codec buildCommand had no default arm, so an unknown kind fell out as undefined and only failed by accident of the wasm-bindgen glue. A known kind carrying a wrong-typed field was worse: wasm-bindgen coerced a numeric newName through USVString and renamed the node, rather than rejecting the command. Every field the builders read is now checked against the type the protocol declares, and the default arm binds the descriptor to never so a new command kind without a builder is a compile error. Closes #1051 --- .../client/src/worker/commandCodec.test.ts | 53 +++++++++++ packages/client/src/worker/commandCodec.ts | 95 ++++++++++++++----- 2 files changed, 123 insertions(+), 25 deletions(-) diff --git a/packages/client/src/worker/commandCodec.test.ts b/packages/client/src/worker/commandCodec.test.ts index f93b55e349..ccfaa51b72 100644 --- a/packages/client/src/worker/commandCodec.test.ts +++ b/packages/client/src/worker/commandCodec.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { fakeWasmEnums } from '../testkit.js'; import { buildCommand, readEvent, readSnapshot } from './commandCodec.js'; +import type { CommandDescriptor } from './protocol.js'; import type { EngineWasm, WasmEvent, WasmSnapshotView } from './engineWasm.js'; /** @@ -54,6 +55,58 @@ describe('buildCommand', () => { expect(calls).toEqual([[2n ** 60n]]); }); + + /** Every builder succeeds, so only the codec's own checks can reject. */ + const permissiveWasm = { + ...fakeWasmEnums, + NodeId: { fromBytes: (bytes: Uint8Array) => ({ bytes }) }, + Command: new Proxy({}, { get: () => () => ({}) }), + } as unknown as EngineWasm; + + const refuses = + (descriptor: unknown): (() => unknown) => + () => + buildCommand(permissiveWasm, descriptor as CommandDescriptor); + + it('fails closed on an unknown command kind', () => { + expect(refuses({ kind: 'telepathy' })).toThrow('unknown command kind: telepathy'); + }); + + it('rejects a wrong-typed string field rather than letting wasm-bindgen coerce it', () => { + expect(refuses({ kind: 'rename', node: new Uint8Array(16), newName: 12345 })).toThrow( + 'invalid command field newName: number' + ); + expect( + refuses({ kind: 'create', parent: new Uint8Array(16), name: null, nodeKind: 'file' }) + ).toThrow('invalid command field name: null'); + }); + + it('rejects a wrong-typed byte-array field', () => { + expect( + refuses({ + kind: 'grant', + node: new Uint8Array(16), + recipientIdentityPublicKey: 'deadbeef', + permission: 'read', + }) + ).toThrow('invalid command field recipientIdentityPublicKey: string'); + expect(refuses({ kind: 'delete', node: [1, 2, 3] })).toThrow('invalid command field node'); + }); + + it('rejects an unknown node kind or permission rather than defaulting one', () => { + expect( + refuses({ kind: 'create', parent: new Uint8Array(16), name: 'a', nodeKind: 'symlink' }) + ).toThrow('invalid command field nodeKind: string'); + expect( + refuses({ kind: 'createInviteLink', node: new Uint8Array(16), permission: 'admin' }) + ).toThrow('invalid command field permission: string'); + }); + + it('rejects an op id that is not the engine bigint', () => { + expect(refuses({ kind: 'cancelUpload', opId: 7 })).toThrow( + 'invalid command field opId: number' + ); + }); }); describe('readEvent', () => { diff --git a/packages/client/src/worker/commandCodec.ts b/packages/client/src/worker/commandCodec.ts index f53fe2467b..d3f190c251 100644 --- a/packages/client/src/worker/commandCodec.ts +++ b/packages/client/src/worker/commandCodec.ts @@ -15,7 +15,6 @@ import type { NodeKind, OpProgressPhase, PendingClass, - Permission, SnapshotDescriptor, Staleness, } from './protocol.js'; @@ -28,71 +27,117 @@ import type { WasmSnapshotView, } from './engineWasm.js'; -function nodeId(wasm: EngineWasm, bytes: Uint8Array): WasmNodeId { - return wasm.NodeId.fromBytes(bytes); +/** + * A descriptor crosses a realm boundary as plain data, so its fields arrive + * untrusted however they are typed here: a version-skewed peer can carry a + * wrong-typed one, and wasm-bindgen would coerce it — a `12345` newName + * marshalled as `"12345"` — rather than reject it. Every field the builders + * read is checked, so the only wrong-typed field is a rejected command. + */ +function invalidField(field: string, value: unknown): Error { + return new Error(`invalid command field ${field}: ${value === null ? 'null' : typeof value}`); +} + +function bytes(value: unknown, field: string): Uint8Array { + if (!(value instanceof Uint8Array)) throw invalidField(field, value); + return value; } -function nodeKind(wasm: EngineWasm, kind: NodeKind): number { - return kind === 'file' ? wasm.NodeKind.File : wasm.NodeKind.Folder; +function text(value: unknown, field: string): string { + if (typeof value !== 'string') throw invalidField(field, value); + return value; } -function permission(wasm: EngineWasm, level: Permission): number { - return level === 'read' ? wasm.Permission.Read : wasm.Permission.Write; +function opId(value: unknown, field: string): bigint { + if (typeof value !== 'bigint') throw invalidField(field, value); + return value; +} + +function nodeId(wasm: EngineWasm, value: unknown, field: string): WasmNodeId { + return wasm.NodeId.fromBytes(bytes(value, field)); +} + +function nodeKind(wasm: EngineWasm, value: unknown): number { + if (value === 'file') return wasm.NodeKind.File; + if (value === 'folder') return wasm.NodeKind.Folder; + throw invalidField('nodeKind', value); +} + +function permission(wasm: EngineWasm, value: unknown): number { + if (value === 'read') return wasm.Permission.Read; + if (value === 'write') return wasm.Permission.Write; + throw invalidField('permission', value); } export function buildCommand(wasm: EngineWasm, descriptor: CommandDescriptor): WasmCommand { switch (descriptor.kind) { case 'create': return wasm.Command.create( - nodeId(wasm, descriptor.parent), - descriptor.name, + nodeId(wasm, descriptor.parent, 'parent'), + text(descriptor.name, 'name'), nodeKind(wasm, descriptor.nodeKind) ); case 'delete': - return wasm.Command.delete(nodeId(wasm, descriptor.node)); + return wasm.Command.delete(nodeId(wasm, descriptor.node, 'node')); case 'rename': - return wasm.Command.rename(nodeId(wasm, descriptor.node), descriptor.newName); + return wasm.Command.rename( + nodeId(wasm, descriptor.node, 'node'), + text(descriptor.newName, 'newName') + ); case 'relink': - return wasm.Command.relink(nodeId(wasm, descriptor.node), nodeId(wasm, descriptor.newParent)); + return wasm.Command.relink( + nodeId(wasm, descriptor.node, 'node'), + nodeId(wasm, descriptor.newParent, 'newParent') + ); case 'cancelUpload': - return wasm.Command.cancelUpload(descriptor.opId); + return wasm.Command.cancelUpload(opId(descriptor.opId, 'opId')); case 'setFocus': return wasm.Command.setFocus( - descriptor.node === null ? undefined : nodeId(wasm, descriptor.node) + descriptor.node === null ? undefined : nodeId(wasm, descriptor.node, 'node') ); case 'manualRefresh': return wasm.Command.manualRefresh(); case 'importContact': - return wasm.Command.importContact(descriptor.contactCode); + return wasm.Command.importContact(bytes(descriptor.contactCode, 'contactCode')); case 'grant': return wasm.Command.grant( - nodeId(wasm, descriptor.node), - descriptor.recipientIdentityPublicKey, + nodeId(wasm, descriptor.node, 'node'), + bytes(descriptor.recipientIdentityPublicKey, 'recipientIdentityPublicKey'), permission(wasm, descriptor.permission) ); case 'revoke': return wasm.Command.revoke( - nodeId(wasm, descriptor.node), - descriptor.recipientIdentityPublicKey + nodeId(wasm, descriptor.node, 'node'), + bytes(descriptor.recipientIdentityPublicKey, 'recipientIdentityPublicKey') ); case 'downgrade': return wasm.Command.downgrade( - nodeId(wasm, descriptor.node), - descriptor.recipientIdentityPublicKey + nodeId(wasm, descriptor.node, 'node'), + bytes(descriptor.recipientIdentityPublicKey, 'recipientIdentityPublicKey') ); case 'createInviteLink': return wasm.Command.createInviteLink( - nodeId(wasm, descriptor.node), + nodeId(wasm, descriptor.node, 'node'), permission(wasm, descriptor.permission) ); case 'acceptShare': - return wasm.Command.acceptShare(descriptor.sealedSharePointer); + return wasm.Command.acceptShare(bytes(descriptor.sealedSharePointer, 'sealedSharePointer')); case 'rotateNow': - return wasm.Command.rotateNow(nodeId(wasm, descriptor.node)); + return wasm.Command.rotateNow(nodeId(wasm, descriptor.node, 'node')); case 'siweLogin': - return wasm.Command.siweLogin(descriptor.message, descriptor.signature); + return wasm.Command.siweLogin( + text(descriptor.message, 'message'), + bytes(descriptor.signature, 'signature') + ); case 'logout': return wasm.Command.logout(); + default: { + // Fail closed: an unmapped kind means a peer built against a different + // protocol, not a command to guess at. The `never` binding makes adding + // a kind without a builder a compile error. + const unmapped: never = descriptor; + throw new Error(`unknown command kind: ${String((unmapped as CommandDescriptor).kind)}`); + } } } From f725340e44233f82a2956472cca895dd75ff35e7 Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Fri, 7 Aug 2026 15:23:12 +0200 Subject: [PATCH 4/6] refactor: use the relay's exhaustiveness-bound idiom for the unknown command --- packages/client/src/worker/commandCodec.ts | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/packages/client/src/worker/commandCodec.ts b/packages/client/src/worker/commandCodec.ts index d3f190c251..6383242c58 100644 --- a/packages/client/src/worker/commandCodec.ts +++ b/packages/client/src/worker/commandCodec.ts @@ -31,8 +31,8 @@ import type { * A descriptor crosses a realm boundary as plain data, so its fields arrive * untrusted however they are typed here: a version-skewed peer can carry a * wrong-typed one, and wasm-bindgen would coerce it — a `12345` newName - * marshalled as `"12345"` — rather than reject it. Every field the builders - * read is checked, so the only wrong-typed field is a rejected command. + * marshalled as `"12345"` — rather than reject it. Hence the checkers below + * take `unknown`, and every field a builder reads passes through one. */ function invalidField(field: string, value: unknown): Error { return new Error(`invalid command field ${field}: ${value === null ? 'null' : typeof value}`); @@ -69,6 +69,15 @@ function permission(wasm: EngineWasm, value: unknown): number { throw invalidField('permission', value); } +/** + * Exhaustiveness bound: adding a command kind without a builder fails the + * build, and a sender off the union gets a refusal rather than the `undefined` + * command the wasm glue merely happens to reject. + */ +function unknownCommand(descriptor: never): Error { + return new Error(`unknown command kind: ${String((descriptor as CommandDescriptor).kind)}`); +} + export function buildCommand(wasm: EngineWasm, descriptor: CommandDescriptor): WasmCommand { switch (descriptor.kind) { case 'create': @@ -131,13 +140,8 @@ export function buildCommand(wasm: EngineWasm, descriptor: CommandDescriptor): W ); case 'logout': return wasm.Command.logout(); - default: { - // Fail closed: an unmapped kind means a peer built against a different - // protocol, not a command to guess at. The `never` binding makes adding - // a kind without a builder a compile error. - const unmapped: never = descriptor; - throw new Error(`unknown command kind: ${String((unmapped as CommandDescriptor).kind)}`); - } + default: + throw unknownCommand(descriptor); } } From ca37eddd22e7054dc9d8af366d47baf1db461dc6 Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Fri, 7 Aug 2026 15:30:46 +0200 Subject: [PATCH 5/6] test: cover the second literal of the node kind and permission mappers --- .../client/src/worker/commandCodec.test.ts | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/packages/client/src/worker/commandCodec.test.ts b/packages/client/src/worker/commandCodec.test.ts index ccfaa51b72..3977f9196e 100644 --- a/packages/client/src/worker/commandCodec.test.ts +++ b/packages/client/src/worker/commandCodec.test.ts @@ -56,6 +56,34 @@ describe('buildCommand', () => { expect(calls).toEqual([[2n ** 60n]]); }); + it('maps the second literal of each mirror enum, not just the first', () => { + const calls: unknown[][] = []; + const record = (...args: unknown[]): object => { + calls.push(args); + return {}; + }; + const wasm = { + ...fakeWasmEnums, + NodeId: { fromBytes: (bytes: Uint8Array) => ({ bytes }) }, + Command: { create: record, createInviteLink: record }, + } as unknown as EngineWasm; + + buildCommand(wasm, { + kind: 'create', + parent: new Uint8Array(16), + name: 'docs', + nodeKind: 'folder', + }); + buildCommand(wasm, { + kind: 'createInviteLink', + node: new Uint8Array(16), + permission: 'write', + }); + + expect(calls[0][2]).toBe(fakeWasmEnums.NodeKind.Folder); + expect(calls[1][1]).toBe(fakeWasmEnums.Permission.Write); + }); + /** Every builder succeeds, so only the codec's own checks can reject. */ const permissiveWasm = { ...fakeWasmEnums, From 45f85cc4465310a67d5571a138bbee7ddceb48e5 Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Sat, 8 Aug 2026 02:15:01 +0200 Subject: [PATCH 6/6] docs: state why the worker scrubs the transferred upload chunk The comment narrated the scrub; the load-bearing fact is that the transfer makes the host the terminal owner, which is what licenses it to zero a buffer at all. Co-Authored-By: Claude Opus 5 --- packages/client/src/worker/engineHost.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/client/src/worker/engineHost.ts b/packages/client/src/worker/engineHost.ts index fd60faaa0c..0560e6ad5a 100644 --- a/packages/client/src/worker/engineHost.ts +++ b/packages/client/src/worker/engineHost.ts @@ -26,7 +26,8 @@ export interface EngineHostLike { command(command: CommandDescriptor): Promise; /** Opens a write handle for `size` plaintext bytes; the engine reserves them. */ beginWrite(target: WriteTarget, size: number): Promise; - /** Takes ownership of `chunk`: the host scrubs the plaintext once it lands. */ + /** Takes ownership of `chunk`: the host is its terminal owner, so it scrubs the + * plaintext to bound the lifetime of a copy no caller can reach. */ pushChunk(handle: WriteHandle, chunk: ArrayBuffer): Promise; /** Closes the handle and journals its op; resolves with the durable op id. */ commitWrite(handle: WriteHandle): Promise;