fix: fail closed on the transferred upload chunk and on an unknown worker command

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', () => {

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);
   }
 }
 

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<void>): EngineHost {
+  const wasm = {
+    EngineHandle: class {
+      pushChunk(_handle: bigint, chunk: Uint8Array): Promise<void> {
+        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));
+  });
 });

packages/client/src/worker/engineHost.ts

@@ -26,6 +26,8 @@ export interface EngineHostLike {
   command(command: CommandDescriptor): Promise<void>;
   /** Opens a write handle for `size` plaintext bytes; the engine reserves them. */
   beginWrite(target: WriteTarget, size: number): Promise<WriteHandle>;
+  /** 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<void>;
   /** Closes the handle and journals its op; resolves with the durable op id. */
   commitWrite(handle: WriteHandle): Promise<bigint>;
@@ -80,17 +82,27 @@ export class EngineHost implements EngineHostLike {
     );
   }
 
-  async start(secret: ArrayBuffer): Promise<void> {
-    // 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<unknown>
+  ): Promise<void> {
+    const view = new Uint8Array(buffer);
     try {
-      await this.handle.start(view);
+      await use(view);
     } finally {
       view.fill(0);
     }
   }
 
+  start(secret: ArrayBuffer): Promise<void> {
+    return this.scrubbing(secret, (view) => this.handle.start(view));
+  }
+
   async command(command: CommandDescriptor): Promise<void> {
     await this.handle.command(buildCommand(this.wasm, command));
   }
@@ -112,10 +124,8 @@ export class EngineHost implements EngineHostLike {
     );
   }
 
-  async pushChunk(handle: WriteHandle, chunk: ArrayBuffer): Promise<void> {
-    // 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<void> {
+    return this.scrubbing(chunk, (view) => this.handle.pushChunk(handle, view));
   }
 
   commitWrite(handle: WriteHandle): Promise<bigint> {