From 1880f12e2198cd68ba01c41d3589a48696aa5a38 Mon Sep 17 00:00:00 2001 From: Olorunfemi20 Date: Thu, 30 Jul 2026 08:35:43 +0100 Subject: [PATCH 1/2] @ feat(backend): group file sharing over MLS A file shared with a group is encrypted once, and the random file key that protects it is distributed inside the MLS group message that references the file. Every member derives the same key from group state, the server never holds one, and object storage keeps a single ciphertext instead of one copy per recipient device. - GET /files/:fileId now gates MLS group files on the requesting device holding an epoch window that covers the reference, using the same reason codes as the message history path. A device removed at epoch M keeps what it could already decrypt and loses everything shared from M onwards, so the server stops handing out ciphertext whose key was only distributed post-rekey - access considers every message referencing the file, so re-sharing a file into a later epoch is how a group deliberately makes an older file readable to members who joined since; the response reports which epoch granted access - POST /uploads requires the uploading device to hold an active leaf when the conversation has an MLS group, and returns the current epoch so the client knows which epoch to encrypt the file key to - non-MLS conversations are untouched: the group lookup is skipped and the response carries a null epoch Adds docs/mls-group-files.md covering the single-ciphertext model, the access rules, and the removed-member behaviour. --- apps/backend/docs/mls-group-files.md | 158 +++++++++ .../src/__tests__/mlsGroupFiles.test.ts | 325 ++++++++++++++++++ apps/backend/src/__tests__/uploads.test.ts | 13 +- apps/backend/src/routes/files.ts | 84 ++++- apps/backend/src/routes/uploads.ts | 19 +- 5 files changed, 586 insertions(+), 13 deletions(-) create mode 100644 apps/backend/docs/mls-group-files.md create mode 100644 apps/backend/src/__tests__/mlsGroupFiles.test.ts diff --git a/apps/backend/docs/mls-group-files.md b/apps/backend/docs/mls-group-files.md new file mode 100644 index 00000000..5916f3e5 --- /dev/null +++ b/apps/backend/docs/mls-group-files.md @@ -0,0 +1,158 @@ +# Group file sharing over MLS + +Sharing a file with a group encrypts it **once**. The random file key that +protects it is distributed by putting it inside the MLS group message that +references the file, so every member derives the same key from group state and +the server never holds one. + +This is the same file subsystem DMs use — presigned upload, a `files` row, a +presigned download — with single-ciphertext delivery instead of one envelope per +recipient device. + +## Why one ciphertext + +The obvious alternative is to encrypt the file separately for each recipient +device. In a 30-person group with 3 devices each that is 90 copies of the same +bytes, and it grows again on every new device. + +MLS already solves this: the group agrees on epoch secrets, so a message +encrypted to the group is readable by exactly the devices in the tree at that +epoch. Putting the file key in such a message gives every member the key without +the server ever learning it, and object storage holds one object instead of +ninety. + +## Flow + +```text +Sender device Backend Other members + | | | + |-- generate random fileKey ---------| | + |-- encrypt file with fileKey -------| | + | | | + |-- POST /uploads ------------------>| | + | { conversationId, size, | | + | mimeType, sha256 } | | + |<- { fileId, uploadUrl, mlsEpoch }--| | + | | | + |-- PUT ciphertext to uploadUrl ---->| (object storage) | + |-- POST /uploads/:id/confirm ------>| | + | | | + |-- send_message ------------------->| | + | { contentType: "image", | | + | fileId, | | + | ciphertext: MLS(fileKey, ...), | | + | mlsEpoch } |--- new_message ------------->| + | | | + | |<-- GET /files/:fileId -------| + | |--- { url, mlsEpoch } ------->| + | | | + | | decrypt with the fileKey | + | | recovered from the message| +``` + +1. The sender generates a random file key, encrypts the file locally, and + uploads the ciphertext to a presigned slot. +2. `POST /uploads` returns `mlsEpoch` — the group's current epoch — so the + client knows which epoch to encrypt the accompanying message to. +3. The sender sends one message carrying `fileId` and a `ciphertext` that + contains the file key, encrypted to the group's epoch secrets. No per-device + envelopes: see [mls-group-membership.md](./mls-group-membership.md) for the + send-path rules. +4. Every member decrypts that message with group state, recovers the same file + key, downloads the single ciphertext and decrypts it locally. + +## What the server stores + +Nothing new. The existing `files` row records the object's location, size, MIME +type and hash. The file key is not a column and never appears in a request body: +it exists only inside the message ciphertext, which the server cannot read. + +`messages.mls_epoch` on the referencing message is what ties a file to an epoch, +so no separate epoch column is needed on `files`. + +## Access control + +`GET /files/:fileId` grants a short-lived presigned download URL when **both** +hold: + +1. The caller is a member of a conversation the file was shared into. +2. For files whose references are all MLS group messages, the calling **device** + holds an epoch window that covers at least one of those references. + +The second rule mirrors the message read path exactly. A device that could not +decrypt the message carrying the file key has no use for the ciphertext, so the +server does not hand it out. Denials carry the same reason codes the history +endpoint uses: + +```json +{ "error": "This device has no key for this file", "reason": "mls_no_key_before_join" } +``` + +| Code | Meaning | +| -------------------------- | ------------------------------------------- | +| `mls_no_key_before_join` | Shared before this device joined the group. | +| `mls_no_key_after_removal` | Shared after this device was removed. | +| `mls_not_a_group_member` | This device holds no leaf in the group. | + +### Removed members lose access going forward + +When a member is removed, the commit that removes them rekeys the group. Every +file shared from that epoch on has its key distributed in messages encrypted to +secrets the removed device does not have — so it could not open those files even +if it obtained the ciphertext. The epoch check makes the server refuse to hand +over the ciphertext in the first place, closing the gap between "cannot decrypt" +and "cannot download". + +Files shared **before** the removal are still served to that device while it +remains a conversation member. It already held those file keys; withholding +bytes it has already been able to read would be theatre rather than forward +secrecy. Removing the user from the conversation entirely cuts off all of it, +via the membership check. + +### Re-sharing an old file to newer members + +A file can be referenced by more than one message. Re-sending it in a later +epoch — with the same file key, re-encrypted to the new epoch's secrets — is how +a group deliberately makes an older file available to members who joined since. + +Access is therefore granted if **any** reachable reference falls inside the +device's window, and the response reports which epoch granted it: + +```json +{ "url": "https://...", "mlsEpoch": 6 } +``` + +When no reference is readable, the denial reports the reason from the earliest +reference, so the message is about the original share rather than an incidental +re-share. + +## Uploads + +`POST /uploads` requires the uploading device to hold an active leaf when the +conversation has an MLS group. A device that cannot send into the group cannot +distribute a file key either, so letting it claim an upload slot would only +create an orphaned object. + +```json +{ "fileId": "uuid", "uploadUrl": "https://...", "mlsEpoch": 7 } +``` + +`mlsEpoch` is `null` for conversations with no MLS group; nothing else about the +upload path changes for DMs. + +## Thumbnails + +A thumbnail is a separate `files` row with `isThumbnail: true` and its own +ciphertext. Clients may encrypt it under the same file key or a distinct one — +either way the key travels in the same MLS group message, and the download route +applies the identical epoch check because the thumbnail is referenced by the same +message. + +## Implementation references + +- download + epoch gate: `apps/backend/src/routes/files.ts` +- upload slot: `apps/backend/src/routes/uploads.ts` +- epoch window lookup: `apps/backend/src/services/mlsGroups.ts` +- reason codes: `apps/backend/src/lib/mlsVisibility.ts` +- send-path rules for MLS messages: `apps/backend/src/lib/validateMessagePayload.ts` +- tests: `apps/backend/src/__tests__/mlsGroupFiles.test.ts` diff --git a/apps/backend/src/__tests__/mlsGroupFiles.test.ts b/apps/backend/src/__tests__/mlsGroupFiles.test.ts new file mode 100644 index 00000000..1e977f43 --- /dev/null +++ b/apps/backend/src/__tests__/mlsGroupFiles.test.ts @@ -0,0 +1,325 @@ +/** + * Tests for group file sharing over MLS (#371). + * + * The model: the file is encrypted once, to a random file key, and that key is + * distributed by putting it inside the MLS group message that references the + * file. Every member derives the same key from group state, the server never + * sees it, and a device removed from the group stops being able to fetch the + * ciphertext for anything shared from the removal epoch on. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import request from 'supertest'; +import express from 'express'; + +// ── Mocks ───────────────────────────────────────────────────────────────────── + +const mockFileFindFirst = vi.fn(); +const mockMessageFindMany = vi.fn(); +const mockMemberFindFirst = vi.fn(); +const mockInsert = vi.fn(); +const mockGetConversationEpochWindow = vi.fn(); +const mockGetGroupByConversation = vi.fn(); +const mockIsActiveMember = vi.fn(); +const mockGeneratePresignedGet = vi.fn(); + +vi.mock('../db/index.js', () => ({ + db: { + query: { + files: { findFirst: mockFileFindFirst }, + messages: { findMany: mockMessageFindMany }, + conversationMembers: { findFirst: mockMemberFindFirst }, + }, + insert: mockInsert, + update: vi.fn(), + }, +})); + +vi.mock('../db/schema.js', () => ({ + files: { id: 'id' }, + messages: { fileId: 'fileId' }, + conversationMembers: { conversationId: 'conversationId', userId: 'userId' }, + fileStatusEnum: vi.fn(), +})); + +vi.mock('drizzle-orm', () => ({ + and: vi.fn((...args: unknown[]) => ({ op: 'and', args })), + eq: vi.fn((col: unknown, val: unknown) => ({ op: 'eq', col, val })), +})); + +vi.mock('../lib/storage.js', () => ({ + generatePresignedGet: mockGeneratePresignedGet, + generatePresignedPut: vi.fn(async (key: string) => `https://storage.example.com/${key}`), + generateStorageKey: vi.fn(() => 'uploads/conv-1/hash'), +})); + +vi.mock('../services/mlsGroups.js', () => ({ + getConversationEpochWindow: mockGetConversationEpochWindow, + getGroupByConversation: mockGetGroupByConversation, + isActiveMember: mockIsActiveMember, +})); + +const USER_ID = 'user-1'; +const DEVICE_ID = 'device-1'; +const CONVERSATION_ID = '33333333-3333-4333-8333-333333333333'; + +vi.mock('../middleware/auth.js', () => ({ + requireAuth: (req: express.Request, _res: express.Response, next: express.NextFunction) => { + (req as express.Request & { auth: { userId: string; deviceId: string } }).auth = { + userId: USER_ID, + deviceId: DEVICE_ID, + }; + next(); + }, +})); + +const { filesRouter } = await import('../routes/files.js'); +const { uploadsRouter } = await import('../routes/uploads.js'); + +function makeApp() { + const app = express(); + app.use(express.json()); + app.use('/files', filesRouter); + app.use('/uploads', uploadsRouter); + return app; +} + +const READY_FILE = { + id: 'file-1', + storageKey: 'uploads/conv-1/hash', + status: 'ready', + deletedAt: null, +}; + +function groupMessage(mlsEpoch: number | null, conversationId = CONVERSATION_ID) { + return { id: `msg-${mlsEpoch}`, conversationId, mlsEpoch }; +} + +beforeEach(() => { + vi.clearAllMocks(); + mockFileFindFirst.mockResolvedValue(READY_FILE); + mockMemberFindFirst.mockResolvedValue({ id: 'membership-1' }); + mockGeneratePresignedGet.mockResolvedValue('https://storage.example.com/signed'); +}); + +// ── Download: single ciphertext, MLS-gated ──────────────────────────────────── + +describe('GET /files/:fileId — MLS group files', () => { + const url = '/files/file-1'; + + it('serves the one stored ciphertext to a member inside the epoch window', async () => { + mockMessageFindMany.mockResolvedValue([groupMessage(7)]); + mockGetConversationEpochWindow.mockResolvedValue({ + hasGroup: true, + window: { joinedAtEpoch: 5, removedAtEpoch: null }, + }); + + const res = await request(makeApp()).get(url); + + expect(res.status).toBe(200); + expect(res.body.url).toBe('https://storage.example.com/signed'); + expect(res.body.mlsEpoch).toBe(7); + // One object in storage, shared by the whole group — not one per member. + expect(mockGeneratePresignedGet).toHaveBeenCalledWith('uploads/conv-1/hash', 300); + }); + + it('serves the same storage key regardless of which member asks', async () => { + mockMessageFindMany.mockResolvedValue([groupMessage(7)]); + mockGetConversationEpochWindow.mockResolvedValue({ + hasGroup: true, + window: { joinedAtEpoch: 0, removedAtEpoch: null }, + }); + + await request(makeApp()).get(url); + await request(makeApp()).get(url); + + const keys = mockGeneratePresignedGet.mock.calls.map((c) => c[0]); + expect(new Set(keys).size).toBe(1); + }); + + it('denies a device removed from the group access to a later-epoch file', async () => { + // Removed by the commit that produced epoch 6; the file was shared at 7. + mockMessageFindMany.mockResolvedValue([groupMessage(7)]); + mockGetConversationEpochWindow.mockResolvedValue({ + hasGroup: true, + window: { joinedAtEpoch: 1, removedAtEpoch: 6 }, + }); + + const res = await request(makeApp()).get(url); + + expect(res.status).toBe(403); + expect(res.body.reason).toBe('mls_no_key_after_removal'); + expect(mockGeneratePresignedGet).not.toHaveBeenCalled(); + }); + + it('still serves a file shared before the removal epoch', async () => { + mockMessageFindMany.mockResolvedValue([groupMessage(5)]); + mockGetConversationEpochWindow.mockResolvedValue({ + hasGroup: true, + window: { joinedAtEpoch: 1, removedAtEpoch: 6 }, + }); + + const res = await request(makeApp()).get(url); + + // The device already held that file key; withholding the ciphertext now + // would be theatre, not forward secrecy. + expect(res.status).toBe(200); + }); + + it('denies a device that joined after the file was shared', async () => { + mockMessageFindMany.mockResolvedValue([groupMessage(2)]); + mockGetConversationEpochWindow.mockResolvedValue({ + hasGroup: true, + window: { joinedAtEpoch: 5, removedAtEpoch: null }, + }); + + const res = await request(makeApp()).get(url); + + expect(res.status).toBe(403); + expect(res.body.reason).toBe('mls_no_key_before_join'); + }); + + it('denies a device that holds no leaf in the group', async () => { + mockMessageFindMany.mockResolvedValue([groupMessage(2)]); + mockGetConversationEpochWindow.mockResolvedValue({ hasGroup: true, window: null }); + + const res = await request(makeApp()).get(url); + + expect(res.status).toBe(403); + expect(res.body.reason).toBe('mls_not_a_group_member'); + }); + + it('grants access when the file was re-shared into an epoch the device holds', async () => { + // Originally shared at epoch 2, re-shared at 6 for members who joined later. + mockMessageFindMany.mockResolvedValue([groupMessage(2), groupMessage(6)]); + mockGetConversationEpochWindow.mockResolvedValue({ + hasGroup: true, + window: { joinedAtEpoch: 5, removedAtEpoch: null }, + }); + + const res = await request(makeApp()).get(url); + + expect(res.status).toBe(200); + expect(res.body.mlsEpoch).toBe(6); + }); + + it('reports the reason from the original share when no reference is readable', async () => { + mockMessageFindMany.mockResolvedValue([groupMessage(2), groupMessage(3)]); + mockGetConversationEpochWindow.mockResolvedValue({ + hasGroup: true, + window: { joinedAtEpoch: 9, removedAtEpoch: null }, + }); + + const res = await request(makeApp()).get(url); + + expect(res.status).toBe(403); + expect(res.body.reason).toBe('mls_no_key_before_join'); + }); + + it('leaves non-MLS file sharing unchanged', async () => { + mockMessageFindMany.mockResolvedValue([groupMessage(null)]); + + const res = await request(makeApp()).get(url); + + expect(res.status).toBe(200); + expect(res.body.mlsEpoch).toBeNull(); + expect(mockGetConversationEpochWindow).not.toHaveBeenCalled(); + }); + + it('does not apply the epoch gate when the file also has a non-MLS reference', async () => { + mockMessageFindMany.mockResolvedValue([groupMessage(2), groupMessage(null)]); + + const res = await request(makeApp()).get(url); + + expect(res.status).toBe(200); + expect(mockGetConversationEpochWindow).not.toHaveBeenCalled(); + }); + + it('rejects a non-member before any MLS lookup happens', async () => { + mockMessageFindMany.mockResolvedValue([groupMessage(7)]); + mockMemberFindFirst.mockResolvedValue(undefined); + + const res = await request(makeApp()).get(url); + + expect(res.status).toBe(403); + expect(res.body.error).toMatch(/Not authorized/i); + expect(mockGetConversationEpochWindow).not.toHaveBeenCalled(); + }); + + it('returns 404 for a soft-deleted file', async () => { + mockFileFindFirst.mockResolvedValue({ ...READY_FILE, deletedAt: new Date() }); + + const res = await request(makeApp()).get(url); + + expect(res.status).toBe(404); + }); + + it('returns 404 when no message references the file', async () => { + mockMessageFindMany.mockResolvedValue([]); + + const res = await request(makeApp()).get(url); + + expect(res.status).toBe(404); + }); + + it('never returns a file key — only the ciphertext location', async () => { + mockMessageFindMany.mockResolvedValue([groupMessage(7)]); + mockGetConversationEpochWindow.mockResolvedValue({ + hasGroup: true, + window: { joinedAtEpoch: 0, removedAtEpoch: null }, + }); + + const res = await request(makeApp()).get(url); + + expect(Object.keys(res.body).sort()).toEqual(['mlsEpoch', 'url']); + }); +}); + +// ── Upload ──────────────────────────────────────────────────────────────────── + +describe('POST /uploads — MLS group conversations', () => { + const body = { + conversationId: CONVERSATION_ID, + size: 1024, + mimeType: 'image/png', + sha256: 'abc123', + }; + + beforeEach(() => { + mockInsert.mockReturnValue({ + values: vi.fn().mockReturnValue({ + returning: vi.fn().mockResolvedValue([{ id: 'file-1' }]), + }), + }); + }); + + it('returns the current epoch so the client knows what to encrypt the key to', async () => { + mockGetGroupByConversation.mockResolvedValue({ id: 'mls-group-1', currentEpoch: 7 }); + mockIsActiveMember.mockResolvedValue(true); + + const res = await request(makeApp()).post('/uploads').send(body); + + expect(res.status).toBe(201); + expect(res.body).toMatchObject({ fileId: 'file-1', mlsEpoch: 7 }); + }); + + it('refuses a device that holds no leaf in the group', async () => { + mockGetGroupByConversation.mockResolvedValue({ id: 'mls-group-1', currentEpoch: 7 }); + mockIsActiveMember.mockResolvedValue(false); + + const res = await request(makeApp()).post('/uploads').send(body); + + expect(res.status).toBe(403); + expect(mockInsert).not.toHaveBeenCalled(); + }); + + it('reports a null epoch for a conversation with no MLS group', async () => { + mockGetGroupByConversation.mockResolvedValue(null); + + const res = await request(makeApp()).post('/uploads').send(body); + + expect(res.status).toBe(201); + expect(res.body.mlsEpoch).toBeNull(); + expect(mockIsActiveMember).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/backend/src/__tests__/uploads.test.ts b/apps/backend/src/__tests__/uploads.test.ts index d57f3204..0d4b4144 100644 --- a/apps/backend/src/__tests__/uploads.test.ts +++ b/apps/backend/src/__tests__/uploads.test.ts @@ -52,9 +52,20 @@ vi.mock('../lib/storage.js', () => ({ generateStorageKey: vi.fn(() => 'uploads/conv-123/abc123def456'), })); +// These cases cover non-MLS conversations, so the group lookup finds nothing +// and the upload path behaves exactly as before (#371). MLS group uploads are +// covered in mlsGroupFiles.test.ts. +vi.mock('../services/mlsGroups.js', () => ({ + getGroupByConversation: vi.fn().mockResolvedValue(null), + isActiveMember: vi.fn().mockResolvedValue(false), +})); + vi.mock('../middleware/auth.js', () => ({ requireAuth: (req: express.Request, _res: express.Response, next: express.NextFunction) => { - (req as express.Request & { auth?: { userId: string } }).auth = { userId: 'user-abc' }; + (req as express.Request & { auth?: { userId: string; deviceId: string } }).auth = { + userId: 'user-abc', + deviceId: 'device-abc', + }; next(); }, })); diff --git a/apps/backend/src/routes/files.ts b/apps/backend/src/routes/files.ts index 0ab21468..dff3b1eb 100644 --- a/apps/backend/src/routes/files.ts +++ b/apps/backend/src/routes/files.ts @@ -5,15 +5,26 @@ import { db } from '../db/index.js'; import { messages, conversationMembers, files } from '../db/schema.js'; import { requireAuth, type AuthRequest } from '../middleware/auth.js'; import { generatePresignedGet } from '../lib/storage.js'; +import { mlsUnavailableReason, type MlsUnavailableReason } from '../lib/mlsVisibility.js'; +import { getConversationEpochWindow } from '../services/mlsGroups.js'; export const filesRouter: IRouter = Router(); filesRouter.use(requireAuth); // ── GET /files/:fileId ───────────────────────────────────────────────────────── // Issues a short-lived presigned GET URL so the client can download ciphertext -// and decrypt it locally (#166). Access is gated on conversation membership. +// and decrypt it locally (#166). Access is gated on conversation membership, +// and — for files shared into an MLS group (#371) — on the requesting device +// holding the epoch the file key was distributed in. +// +// A group file is encrypted once, to a random file key, and that key travels +// inside the MLS group message that references it. The server stores the single +// ciphertext and never sees the key, so "who can open this file" is decided by +// who could decrypt the message that carried the key. This route mirrors that +// decision rather than inventing a second, weaker rule. filesRouter.get('/:fileId', async (req: AuthRequest, res) => { const userId = req.auth!.userId; + const deviceId = req.auth!.deviceId as string | undefined; const fileId = req.params['fileId'] as string; if (!fileId) { @@ -30,33 +41,84 @@ filesRouter.get('/:fileId', async (req: AuthRequest, res) => { return; } - // Find the message that references this file and check conversation membership - const message = await db.query.messages.findFirst({ + // All messages that reference this file. Usually one, but a file re-shared + // into a later epoch has several — and that is precisely how a group makes an + // older file readable to members who joined later, so every reference has to + // be considered rather than just the first. + const referencing = await db.query.messages.findMany({ where: eq(messages.fileId, fileId), + columns: { id: true, conversationId: true, mlsEpoch: true }, }); - if (!message) { + if (referencing.length === 0) { res.status(404).json({ error: 'File not referenced by any message' }); return; } // Check if the user is a member of the conversation where the file was shared - const membership = await db.query.conversationMembers.findFirst({ - where: and( - eq(conversationMembers.conversationId, message.conversationId), - eq(conversationMembers.userId, userId), + const conversationIds = [...new Set(referencing.map((m) => m.conversationId))]; + const memberships = await Promise.all( + conversationIds.map((conversationId) => + db.query.conversationMembers.findFirst({ + where: and( + eq(conversationMembers.conversationId, conversationId), + eq(conversationMembers.userId, userId), + ), + columns: { id: true }, + }), ), - }); + ); + + const memberOf = new Set(conversationIds.filter((_, index) => memberships[index] !== undefined)); + + const reachable = referencing.filter((m) => memberOf.has(m.conversationId)); - if (!membership) { + if (reachable.length === 0) { res.status(403).json({ error: 'Not authorized to access this file' }); return; } + // ── MLS epoch check (#371) ────────────────────────────────────────────────── + // Access is granted if *any* reachable reference sits inside this device's + // epoch window. A device removed from the group at epoch M keeps whatever it + // could already decrypt and loses everything shared from M on: the removal + // commit rekeys the group, so it never receives the file keys distributed + // afterwards, and this check stops the server handing out the ciphertext they + // protect. + const mlsReferences = reachable.filter((m) => m.mlsEpoch !== null); + const nonMlsReferences = reachable.length - mlsReferences.length; + + let grantedEpoch: number | null = null; + let denialReason: MlsUnavailableReason | null = null; + + if (nonMlsReferences === 0 && mlsReferences.length > 0) { + for (const reference of mlsReferences) { + const { window } = await getConversationEpochWindow(reference.conversationId, deviceId); + const reason = mlsUnavailableReason(reference.mlsEpoch!, window); + + if (reason === null) { + grantedEpoch = reference.mlsEpoch; + break; + } + + // Report the reason from the earliest reference so the message is about + // the original share rather than an incidental re-share. + denialReason ??= reason; + } + + if (grantedEpoch === null) { + res.status(403).json({ + error: 'This device has no key for this file', + reason: denialReason, + }); + return; + } + } + try { // Short-lived URL: 5 minutes const presignedUrl = await generatePresignedGet(file.storageKey, 300); - res.json({ url: presignedUrl }); + res.json({ url: presignedUrl, mlsEpoch: grantedEpoch }); } catch { res.status(500).json({ error: 'Failed to generate download URL' }); } diff --git a/apps/backend/src/routes/uploads.ts b/apps/backend/src/routes/uploads.ts index 3a0ae36c..d883f84d 100644 --- a/apps/backend/src/routes/uploads.ts +++ b/apps/backend/src/routes/uploads.ts @@ -6,6 +6,7 @@ import { db } from '../db/index.js'; import { files, conversationMembers } from '../db/schema.js'; import { requireAuth, type AuthRequest } from '../middleware/auth.js'; import { generatePresignedPut, generateStorageKey } from '../lib/storage.js'; +import { getGroupByConversation, isActiveMember } from '../services/mlsGroups.js'; export const uploadsRouter: IRouter = Router(); @@ -65,6 +66,22 @@ uploadsRouter.post('/', async (req: AuthRequest, res) => { return; } + // ── MLS group uploads (#371) ──────────────────────────────────────────────── + // The file is encrypted once to a random file key, and that key is delivered + // by putting it inside the MLS group message that references the file — so + // the uploading device has to be able to send into the group in the first + // place. `mlsEpoch` comes back with the slot so the client knows which epoch + // to encrypt that message to. + const group = await getGroupByConversation(conversationId); + const deviceId = req.auth!.deviceId as string | undefined; + + if (group) { + if (!deviceId || !(await isActiveMember(group.id, deviceId))) { + res.status(403).json({ error: 'Device is not a member of this conversation MLS group' }); + return; + } + } + const storageKey = generateStorageKey(conversationId, sha256); const uploadUrl = await generatePresignedPut(storageKey, mimeType); @@ -82,7 +99,7 @@ uploadsRouter.post('/', async (req: AuthRequest, res) => { }) .returning({ id: files.id }); - res.status(201).json({ fileId: file!.id, uploadUrl }); + res.status(201).json({ fileId: file!.id, uploadUrl, mlsEpoch: group?.currentEpoch ?? null }); }); // POST /uploads/:fileId/confirm — mark file as ready after client PUT succeeds From 46ad41f4b08ba3827abf27e9f73c7f27dee27485 Mon Sep 17 00:00:00 2001 From: Olorunfemi20 Date: Thu, 30 Jul 2026 08:56:29 +0100 Subject: [PATCH 2/2] @ fix(backend): treat a null membership lookup as not-a-member The file download membership filter compared against undefined, so a driver reporting "no row" as null would have been read as a match and handed a non-member a presigned URL. Uses a truthiness check and pins it with a test. --- apps/backend/src/__tests__/mlsGroupFiles.test.ts | 13 +++++++++++++ apps/backend/src/routes/files.ts | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/apps/backend/src/__tests__/mlsGroupFiles.test.ts b/apps/backend/src/__tests__/mlsGroupFiles.test.ts index 1e977f43..4ba764d1 100644 --- a/apps/backend/src/__tests__/mlsGroupFiles.test.ts +++ b/apps/backend/src/__tests__/mlsGroupFiles.test.ts @@ -246,6 +246,19 @@ describe('GET /files/:fileId — MLS group files', () => { expect(mockGetConversationEpochWindow).not.toHaveBeenCalled(); }); + it('treats a null membership lookup as not-a-member', async () => { + // Guards the membership filter against a driver that reports "no row" as + // null rather than undefined — a truthiness slip here would hand a + // non-member a download URL. + mockMessageFindMany.mockResolvedValue([groupMessage(7)]); + mockMemberFindFirst.mockResolvedValue(null); + + const res = await request(makeApp()).get(url); + + expect(res.status).toBe(403); + expect(mockGeneratePresignedGet).not.toHaveBeenCalled(); + }); + it('returns 404 for a soft-deleted file', async () => { mockFileFindFirst.mockResolvedValue({ ...READY_FILE, deletedAt: new Date() }); diff --git a/apps/backend/src/routes/files.ts b/apps/backend/src/routes/files.ts index dff3b1eb..ec1427ed 100644 --- a/apps/backend/src/routes/files.ts +++ b/apps/backend/src/routes/files.ts @@ -69,7 +69,7 @@ filesRouter.get('/:fileId', async (req: AuthRequest, res) => { ), ); - const memberOf = new Set(conversationIds.filter((_, index) => memberships[index] !== undefined)); + const memberOf = new Set(conversationIds.filter((_, index) => Boolean(memberships[index]))); const reachable = referencing.filter((m) => memberOf.has(m.conversationId));