diff --git a/packages/client/src/worker/commandCodec.test.ts b/packages/client/src/worker/commandCodec.test.ts index f93b55e349..3977f9196e 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,86 @@ 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, + 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..6383242c58 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,121 @@ 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. 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}`); +} + +function bytes(value: unknown, field: string): Uint8Array { + if (!(value instanceof Uint8Array)) throw invalidField(field, value); + return value; +} + +function text(value: unknown, field: string): string { + if (typeof value !== 'string') throw invalidField(field, value); + return value; +} + +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 nodeKind(wasm: EngineWasm, kind: NodeKind): number { - return kind === 'file' ? wasm.NodeKind.File : wasm.NodeKind.Folder; +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); } -function permission(wasm: EngineWasm, level: Permission): number { - return level === 'read' ? wasm.Permission.Read : wasm.Permission.Write; +/** + * 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': 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: + throw unknownCommand(descriptor); } } 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..0560e6ad5a 100644 --- a/packages/client/src/worker/engineHost.ts +++ b/packages/client/src/worker/engineHost.ts @@ -26,6 +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 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; @@ -80,17 +82,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)); } @@ -112,10 +124,8 @@ 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)); + pushChunk(handle: WriteHandle, chunk: ArrayBuffer): Promise { + return this.scrubbing(chunk, (view) => this.handle.pushChunk(handle, view)); } commitWrite(handle: WriteHandle): Promise {