Harden legacy file upload validation

Author: IM.codes

server/src/routes/file-transfer.ts

@@ -19,6 +19,7 @@ export const fileTransferRoutes = new Hono<{ Bindings: Env; Variables: { userId:
 // URL more than once, so tokens are resource-bound and short-lived with a small
 // use budget instead of being consumed on the first GET.
 const DOWNLOAD_TOKEN_MAX_USES = 5;
+const MULTIPART_UPLOAD_OVERHEAD_BYTES = 1024 * 1024;
 const downloadTokens = new Map<string, {
   serverId: string;
   attachmentId: string;
@@ -68,6 +69,18 @@ fileTransferRoutes.post('/:id/upload', async (c) => {
   const role = await resolveServerRole(c.env.DB, serverId, userId);
   if (role === 'none') return c.json({ error: 'forbidden' }, 403);
 
+  const contentLengthHeader = c.req.header('content-length');
+  const contentLength = contentLengthHeader ? Number.parseInt(contentLengthHeader, 10) : Number.NaN;
+  if (
+    Number.isFinite(contentLength)
+    && contentLength > FILE_TRANSFER_LIMITS.MAX_FILE_SIZE + MULTIPART_UPLOAD_OVERHEAD_BYTES
+  ) {
+    return c.json({
+      error: 'file_too_large',
+      maxBytes: FILE_TRANSFER_LIMITS.MAX_FILE_SIZE,
+    }, 413);
+  }
+
   // Parse multipart
   const formData = await c.req.formData().catch(() => null);
   if (!formData) return c.json({ error: 'invalid_body' }, 400);

server/test/file-transfer-routes.test.ts

@@ -0,0 +1,118 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { Hono } from 'hono';
+import { FILE_TRANSFER_LIMITS } from '../../shared/transport/file-transfer.js';
+
+const { sendFileTransferRequestMock, isDaemonConnectedMock } = vi.hoisted(() => ({
+  sendFileTransferRequestMock: vi.fn(),
+  isDaemonConnectedMock: vi.fn(),
+}));
+
+vi.mock('../src/security/authorization.js', () => ({
+  requireAuth: () => async (c: { req: { header: (name: string) => string | undefined }; set: (key: string, value: string) => void }, next: () => Promise<void>) => {
+    if (!c.req.header('Authorization')) {
+      return new Response(JSON.stringify({ error: 'unauthorized' }), {
+        status: 401,
+        headers: { 'Content-Type': 'application/json' },
+      });
+    }
+    c.set('userId', 'user-1');
+    return next();
+  },
+  resolveServerRole: vi.fn().mockResolvedValue('owner'),
+}));
+
+vi.mock('../src/ws/bridge.js', () => ({
+  WsBridge: {
+    get: () => ({
+      isDaemonConnected: isDaemonConnectedMock,
+      sendFileTransferRequest: sendFileTransferRequestMock,
+    }),
+  },
+}));
+
+vi.mock('../src/util/logger.js', () => ({
+  default: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() },
+}));
+
+vi.mock('../src/security/crypto.js', () => ({
+  randomHex: (bytes: number) => 'a'.repeat(bytes * 2),
+}));
+
+import { fileTransferRoutes } from '../src/routes/file-transfer.js';
+
+function makeApp(): Hono {
+  const app = new Hono();
+  app.use('/*', async (c, next) => {
+    (c as never as { env: { DB: unknown } }).env = { DB: {} };
+    return next();
+  });
+  app.route('/api/server', fileTransferRoutes);
+  return app;
+}
+
+describe('file-transfer upload route', () => {
+  beforeEach(() => {
+    sendFileTransferRequestMock.mockReset();
+    isDaemonConnectedMock.mockReset();
+    isDaemonConnectedMock.mockReturnValue(true);
+    sendFileTransferRequestMock.mockResolvedValue({
+      type: 'file.upload_done',
+      attachment: {
+        id: 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.txt',
+        source: 'upload',
+        daemonPath: '/tmp/upload.txt',
+        downloadable: true,
+      },
+    });
+  });
+
+  it('rejects oversized legacy uploads from content-length before daemon relay', async () => {
+    const res = await makeApp().request('/api/server/srv-1/upload', {
+      method: 'POST',
+      headers: {
+        Authorization: 'Bearer test',
+        'Content-Type': 'multipart/form-data; boundary=x',
+        'Content-Length': String(FILE_TRANSFER_LIMITS.MAX_FILE_SIZE + 1024 * 1024 + 1),
+      },
+      body: '--x\r\n',
+    });
+
+    expect(res.status).toBe(413);
+    await expect(res.json()).resolves.toEqual({
+      error: 'file_too_large',
+      maxBytes: FILE_TRANSFER_LIMITS.MAX_FILE_SIZE,
+    });
+    expect(sendFileTransferRequestMock).not.toHaveBeenCalled();
+  });
+
+  it('relays a valid legacy upload with base64 content', async () => {
+    const form = new FormData();
+    form.append('file', new File(['hello'], 'hello.txt', { type: 'text/plain' }));
+
+    const res = await makeApp().request('/api/server/srv-1/upload', {
+      method: 'POST',
+      headers: { Authorization: 'Bearer test' },
+      body: form,
+    });
+
+    expect(res.status).toBe(200);
+    await expect(res.json()).resolves.toMatchObject({
+      ok: true,
+      attachment: {
+        serverId: 'srv-1',
+        daemonPath: '/tmp/upload.txt',
+      },
+    });
+    expect(sendFileTransferRequestMock).toHaveBeenCalledWith(
+      expect.any(String),
+      expect.objectContaining({
+        type: 'file.upload',
+        originalName: 'hello.txt',
+        mime: 'text/plain',
+        size: 5,
+        content: Buffer.from('hello').toString('base64'),
+      }),
+      FILE_TRANSFER_LIMITS.UPLOAD_TIMEOUT_MS,
+    );
+  });
+});

src/daemon/file-transfer-handler.ts

@@ -157,6 +157,12 @@ export async function handleFileUpload(cmd: Record<string, unknown>, serverLink:
     }
 
     const buffer = Buffer.from(content, 'base64');
+    if (buffer.length > FILE_TRANSFER_LIMITS.MAX_FILE_SIZE) {
+      throw new Error('file_too_large');
+    }
+    if (typeof msg.size === 'number' && buffer.length !== msg.size) {
+      throw new Error('size_mismatch');
+    }
     await writeFile(resolved, buffer);
 
     // Write metadata sidecar for recovery after daemon restart

test/daemon/file-transfer-handler.test.ts

@@ -4,12 +4,24 @@ import { tmpdir } from 'node:os';
 import path from 'node:path';
 import { FILE_TRANSFER_LIMITS } from '../../shared/transport/file-transfer.js';
 
-async function loadFileTransferHandler(fakeHome: string) {
+async function loadFileTransferHandler(fakeHome: string, options?: { maxFileSize?: number }) {
   vi.resetModules();
   vi.doMock('node:os', async (importOriginal) => {
     const actual = await importOriginal<typeof import('node:os')>();
     return { ...actual, homedir: () => fakeHome };
   });
+  if (options?.maxFileSize !== undefined) {
+    vi.doMock('../../shared/transport/file-transfer.js', async (importOriginal) => {
+      const actual = await importOriginal<typeof import('../../shared/transport/file-transfer.js')>();
+      return {
+        ...actual,
+        FILE_TRANSFER_LIMITS: {
+          ...actual.FILE_TRANSFER_LIMITS,
+          MAX_FILE_SIZE: options.maxFileSize,
+        },
+      };
+    });
+  }
   vi.doMock('../../src/util/logger.js', () => ({
     default: {
       info: vi.fn(),
@@ -47,6 +59,7 @@ describe('file-transfer local handle hardening', () => {
   afterEach(async () => {
     vi.restoreAllMocks();
     vi.doUnmock('node:os');
+    vi.doUnmock('../../shared/transport/file-transfer.js');
     vi.doUnmock('../../src/util/logger.js');
     vi.resetModules();
     await rm(rootDir, { recursive: true, force: true });
@@ -188,4 +201,48 @@ describe('file-transfer local handle hardening', () => {
       message: 'expired',
     });
   });
+
+  it('rejects legacy uploads over the active single-frame cap', async () => {
+    const transfer = await loadFileTransferHandler(fakeHome, { maxFileSize: 4 });
+    const failed = createServerLinkMock();
+
+    await transfer.handleFileUpload(
+      {
+        type: 'file.upload',
+        uploadId: 'upload-too-large',
+        filename: 'safe.txt',
+        size: 5,
+        content: Buffer.from('hello').toString('base64'),
+      },
+      failed.serverLink as never,
+    );
+
+    expect(failed.sent[0]).toMatchObject({
+      type: 'file.upload_error',
+      uploadId: 'upload-too-large',
+      message: 'file_too_large',
+    });
+  });
+
+  it('rejects legacy upload payloads whose decoded byte count does not match the declared size', async () => {
+    const transfer = await loadFileTransferHandler(fakeHome);
+    const failed = createServerLinkMock();
+
+    await transfer.handleFileUpload(
+      {
+        type: 'file.upload',
+        uploadId: 'upload-size-mismatch',
+        filename: 'safe.txt',
+        size: 99,
+        content: Buffer.from('hello').toString('base64'),
+      },
+      failed.serverLink as never,
+    );
+
+    expect(failed.sent[0]).toMatchObject({
+      type: 'file.upload_error',
+      uploadId: 'upload-size-mismatch',
+      message: 'size_mismatch',
+    });
+  });
 });