diff --git a/.env.example b/.env.example index 7312479a..651187c6 100644 --- a/.env.example +++ b/.env.example @@ -28,10 +28,16 @@ OPENAI_API_KEY= # Messaging XMTP_ENV=dev +# Seconds an eventId is remembered for replay/duplicate protection. Defaults to 86400 (24h). +IDEMPOTENCY_TTL_SECONDS=86400 +# Milliseconds an offline transition is held before broadcasting user_offline, +# so a brief disconnect+reconnect blip produces no offline/online pair. Defaults to 5000 (5s). +PRESENCE_OFFLINE_GRACE_MS=5000 # Push Notifications (VAPID) +# The frontend fetches VAPID_PUBLIC_KEY from GET /push/vapid-public-key at +# runtime (#349) rather than a separate NEXT_PUBLIC_* build-time var, so the +# two can never drift out of sync. VAPID_PUBLIC_KEY= VAPID_PRIVATE_KEY= -VAPID_SUBJECT=mailto:admin@example.com -# Exposed to the browser via Next.js -NEXT_PUBLIC_VAPID_PUBLIC_KEY= \ No newline at end of file +VAPID_SUBJECT=mailto:admin@example.com \ No newline at end of file diff --git a/apps/backend/src/__tests__/dispatcher.test.ts b/apps/backend/src/__tests__/dispatcher.test.ts index 5a335b02..2a024d95 100644 --- a/apps/backend/src/__tests__/dispatcher.test.ts +++ b/apps/backend/src/__tests__/dispatcher.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { EventEmitter } from 'events'; import { EventDispatcher } from '../socket/dispatcher.js'; import type { AuthSocket } from '../middleware/socketAuth.js'; @@ -187,3 +187,115 @@ describe('EventDispatcher.listen — envelope routing', () => { expect(errors.length).toBeGreaterThan(0); }); }); + +describe('EventDispatcher — configurable idempotency TTL (#344)', () => { + const ORIGINAL_TTL = process.env.IDEMPOTENCY_TTL_SECONDS; + + afterEach(() => { + if (ORIGINAL_TTL === undefined) delete process.env.IDEMPOTENCY_TTL_SECONDS; + else process.env.IDEMPOTENCY_TTL_SECONDS = ORIGINAL_TTL; + }); + + it('defaults the Redis SET EX TTL to 86400 seconds when unset', async () => { + delete process.env.IDEMPOTENCY_TTL_SECONDS; + const { socket, trigger } = makeSocket(); + const redis = makeRedis('OK'); + const dispatcher = new EventDispatcher(makeIo(), socket, redis as never); + dispatcher.register('join_room', vi.fn()); + dispatcher.listen(); + + trigger('dispatch', { + eventId: 'evt-default-ttl', + type: 'join_room', + timestamp: Date.now(), + payload: {}, + }); + + await new Promise((r) => setTimeout(r, 10)); + expect(redis.set).toHaveBeenCalledWith( + 'event:idempotency:evt-default-ttl', + '1', + 'EX', + 86_400, + 'NX', + ); + }); + + it('reads the Redis SET EX TTL from IDEMPOTENCY_TTL_SECONDS when configured', async () => { + process.env.IDEMPOTENCY_TTL_SECONDS = '120'; + const { socket, trigger } = makeSocket(); + const redis = makeRedis('OK'); + const dispatcher = new EventDispatcher(makeIo(), socket, redis as never); + dispatcher.register('join_room', vi.fn()); + dispatcher.listen(); + + trigger('dispatch', { + eventId: 'evt-custom-ttl', + type: 'join_room', + timestamp: Date.now(), + payload: {}, + }); + + await new Promise((r) => setTimeout(r, 10)); + expect(redis.set).toHaveBeenCalledWith( + 'event:idempotency:evt-custom-ttl', + '1', + 'EX', + 120, + 'NX', + ); + }); + + it('falls back to the default TTL for an invalid (non-numeric) override', async () => { + process.env.IDEMPOTENCY_TTL_SECONDS = 'not-a-number'; + const { socket, trigger } = makeSocket(); + const redis = makeRedis('OK'); + const dispatcher = new EventDispatcher(makeIo(), socket, redis as never); + dispatcher.register('join_room', vi.fn()); + dispatcher.listen(); + + trigger('dispatch', { + eventId: 'evt-invalid-ttl', + type: 'join_room', + timestamp: Date.now(), + payload: {}, + }); + + await new Promise((r) => setTimeout(r, 10)); + expect(redis.set).toHaveBeenCalledWith( + 'event:idempotency:evt-invalid-ttl', + '1', + 'EX', + 86_400, + 'NX', + ); + }); + + it('rejects a duplicate eventId within the TTL window regardless of configured TTL', async () => { + process.env.IDEMPOTENCY_TTL_SECONDS = '300'; + const { socket, trigger } = makeSocket(); + // null == Redis SET NX found the key already present (still within TTL) + const redis = makeRedis(null); + const dispatcher = new EventDispatcher(makeIo(), socket, redis as never); + const handler = vi.fn(); + dispatcher.register('join_room', handler); + dispatcher.listen(); + + trigger('dispatch', { + eventId: 'evt-duplicate-within-ttl', + type: 'join_room', + timestamp: Date.now(), + payload: {}, + }); + + await new Promise((r) => setTimeout(r, 10)); + expect(handler).not.toHaveBeenCalled(); + expect(redis.set).toHaveBeenCalledWith( + 'event:idempotency:evt-duplicate-within-ttl', + '1', + 'EX', + 300, + 'NX', + ); + }); +}); diff --git a/apps/backend/src/__tests__/file.messages.test.ts b/apps/backend/src/__tests__/file.messages.test.ts index f12fec54..b68ecc55 100644 --- a/apps/backend/src/__tests__/file.messages.test.ts +++ b/apps/backend/src/__tests__/file.messages.test.ts @@ -35,7 +35,10 @@ const mockMemberFindFirst = vi.fn(); const mockMemberFindMany = vi.fn(); const mockMessageFindFirst = vi.fn(); const mockFileFindFirst = vi.fn(); +const mockMessageFindFirst = vi.fn(); const mockDevicesFindMany = vi.fn(); +const mockInsert = vi.fn(); +const mockFindMany = vi.fn(); const mockUpdate = vi.fn(); /** Every insert(table).values(rows) call, so envelope rows can be asserted. */ @@ -185,10 +188,13 @@ const CONVERSATION_ID = 'conv-1'; const FILE_ID = 'file-abc'; const MESSAGE_ID = 'msg-client-supplied'; -// The content is an E2EE envelope ciphertext. The server treats it as an -// opaque string — it must NOT parse or store the embedded fileKey. -const ENVELOPE_CIPHERTEXT = - 'encrypted:{"fileId":"file-abc","fileName":"photo.jpg","mimeType":"image/jpeg","size":204800,"fileKey":"SUPER_SECRET_KEY_NEVER_STORED"}'; +// The content is an E2EE envelope ciphertext for the message body. The server +// treats it as an opaque string. The file's symmetric encryption key must +// NEVER appear here — it only ever lives inside `envelopes[].ciphertext`. +const ENVELOPE_CIPHERTEXT = 'encrypted:{"fileId":"file-abc","fileName":"photo.jpg"}'; +const FILE_KEY_ENVELOPES = [ + { recipientDeviceId: RECIPIENT_DEVICE_ID, ciphertext: 'sealed:SUPER_SECRET_KEY_NEVER_STORED' }, +]; function readyFile( overrides: Partial<{ @@ -553,16 +559,60 @@ describe('send_file_message — validation and access control', () => { it('rejects when sender is not a member of the conversation', async () => { mockMemberFindFirst.mockResolvedValue(undefined); - const socket = makeSocket('non-member'); + await handler(basePayload({ messageId: '' })); + + expect(socket.emit).toHaveBeenCalledWith( + 'error', + expect.objectContaining({ + event: 'send_file_message', + message: expect.stringContaining('messageId'), + }), + ); + expect(mockInsert).not.toHaveBeenCalled(); + }); + + it('rejects when envelopes are missing (the file key has nowhere safe to travel)', async () => { + mockMemberFindFirst.mockResolvedValueOnce({ + id: 'm1', + userId: SENDER_ID, + conversationId: CONVERSATION_ID, + }); + + const socket = makeSocket(SENDER_ID); const io = makeIo(); const handler = await getHandler(socket, io); await handler({ conversationId: CONVERSATION_ID, - fileId: FILE_ID, - content: ENVELOPE_CIPHERTEXT, - contentType: 'file', }); + mockFileFindFirst.mockResolvedValueOnce(readyFile()); + // fetchSiblingDeviceIds finds one sibling device the payload didn't cover. + mockDevicesFindMany.mockResolvedValueOnce([{ id: 'device-sibling', userId: SENDER_ID }]); + + const socket = makeSocket(SENDER_ID); + const io = makeIo(); + const handler = await getHandler(socket, io); + + await handler(basePayload()); + + expect(socket.emit).toHaveBeenCalledWith( + 'error', + expect.objectContaining({ + event: 'device_set_mismatch', + missingDeviceIds: ['device-sibling'], + }), + ); + expect(mockInsert).not.toHaveBeenCalled(); + }); + + it('rejects when sender is not a member of the conversation', async () => { + mockMemberFindFirst.mockResolvedValueOnce(undefined); // no membership + + const socket = makeSocket('non-member'); + const io = makeIo(); + const handler = await getHandler(socket, io); + + await handler(basePayload()); expect(socket.emit).toHaveBeenCalledWith( 'error', diff --git a/apps/backend/src/__tests__/presence.test.ts b/apps/backend/src/__tests__/presence.test.ts index 53eb665d..15653500 100644 --- a/apps/backend/src/__tests__/presence.test.ts +++ b/apps/backend/src/__tests__/presence.test.ts @@ -1,10 +1,13 @@ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { setOnline, refreshPresence, setOffline, markDeviceOffline, isOnline, + scheduleOfflineBroadcast, + cancelPendingOfflineBroadcast, + __resetOfflineBroadcastsForTesting, } from '../services/presence.js'; class FakeRedis { @@ -96,3 +99,74 @@ describe('presence service', () => { expect(redis.deleted.has('presence:user:user-1:device:device-1')).toBe(true); }); }); + +describe('presence offline-broadcast debounce (#345)', () => { + const ORIGINAL_GRACE = process.env.PRESENCE_OFFLINE_GRACE_MS; + + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + __resetOfflineBroadcastsForTesting(); + vi.useRealTimers(); + if (ORIGINAL_GRACE === undefined) delete process.env.PRESENCE_OFFLINE_GRACE_MS; + else process.env.PRESENCE_OFFLINE_GRACE_MS = ORIGINAL_GRACE; + }); + + it('does not broadcast if cancelled (reconnect) before the grace window elapses', async () => { + process.env.PRESENCE_OFFLINE_GRACE_MS = '5000'; + const broadcast = vi.fn(); + + scheduleOfflineBroadcast('user-1', broadcast); + vi.advanceTimersByTime(4999); + const cancelled = cancelPendingOfflineBroadcast('user-1'); + + await vi.runAllTimersAsync(); + + expect(cancelled).toBe(true); + expect(broadcast).not.toHaveBeenCalled(); + }); + + it('broadcasts once the grace window fully elapses without a reconnect', async () => { + process.env.PRESENCE_OFFLINE_GRACE_MS = '5000'; + const broadcast = vi.fn(); + + scheduleOfflineBroadcast('user-1', broadcast); + await vi.advanceTimersByTimeAsync(5000); + + expect(broadcast).toHaveBeenCalledTimes(1); + expect(cancelPendingOfflineBroadcast('user-1')).toBe(false); + }); + + it('cancelling with nothing pending is a no-op that reports false', () => { + expect(cancelPendingOfflineBroadcast('user-with-no-pending-broadcast')).toBe(false); + }); + + it('re-scheduling for the same user replaces (does not stack) the pending broadcast', async () => { + process.env.PRESENCE_OFFLINE_GRACE_MS = '5000'; + const first = vi.fn(); + const second = vi.fn(); + + scheduleOfflineBroadcast('user-1', first); + vi.advanceTimersByTime(2000); + scheduleOfflineBroadcast('user-1', second); + + await vi.advanceTimersByTimeAsync(5000); + + expect(first).not.toHaveBeenCalled(); + expect(second).toHaveBeenCalledTimes(1); + }); + + it('defaults the grace window to 5000ms when PRESENCE_OFFLINE_GRACE_MS is unset', async () => { + delete process.env.PRESENCE_OFFLINE_GRACE_MS; + const broadcast = vi.fn(); + + scheduleOfflineBroadcast('user-1', broadcast); + vi.advanceTimersByTime(4999); + expect(broadcast).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(1); + expect(broadcast).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/backend/src/__tests__/push.routes.test.ts b/apps/backend/src/__tests__/push.routes.test.ts new file mode 100644 index 00000000..be9667a4 --- /dev/null +++ b/apps/backend/src/__tests__/push.routes.test.ts @@ -0,0 +1,90 @@ +/** + * Tests for GET /push/vapid-public-key (#349). + * + * The frontend must source the VAPID public key from the backend instead of + * a separately-configured build-time env var, so the two can never drift out + * of sync with the private key the backend actually signs pushes with. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import request from 'supertest'; +import express from 'express'; + +vi.mock('../db/index.js', () => ({ + db: { + insert: vi.fn(), + delete: vi.fn(), + update: vi.fn(), + }, +})); + +vi.mock('../db/schema.js', () => ({ + pushSubscriptions: { + deviceId: 'device_id', + endpoint: 'endpoint', + }, +})); + +vi.mock('drizzle-orm', () => ({ + and: vi.fn((...args: unknown[]) => ({ type: 'and', args })), + eq: vi.fn((col: unknown, val: unknown) => ({ type: 'eq', col, val })), + isNull: vi.fn((col: unknown) => ({ type: 'isNull', col })), +})); + +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-1', + deviceId: 'device-1', + }; + next(); + }, +})); + +const { pushRouter } = await import('../routes/push.js'); + +function makeApp() { + const app = express(); + app.use(express.json()); + app.use('/push', pushRouter); + return app; +} + +describe('GET /push/vapid-public-key', () => { + const ORIGINAL_PUBLIC_KEY = process.env['VAPID_PUBLIC_KEY']; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + if (ORIGINAL_PUBLIC_KEY === undefined) delete process.env['VAPID_PUBLIC_KEY']; + else process.env['VAPID_PUBLIC_KEY'] = ORIGINAL_PUBLIC_KEY; + }); + + it('returns the configured VAPID public key', async () => { + process.env['VAPID_PUBLIC_KEY'] = 'test-vapid-public-key'; + + const res = await request(makeApp()).get('/push/vapid-public-key'); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ configured: true, vapidPublicKey: 'test-vapid-public-key' }); + }); + + it('returns configured: false when VAPID is not set up on the backend', async () => { + delete process.env['VAPID_PUBLIC_KEY']; + + const res = await request(makeApp()).get('/push/vapid-public-key'); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ configured: false, vapidPublicKey: null }); + }); + + it('returns configured: false when the env var is set but empty', async () => { + process.env['VAPID_PUBLIC_KEY'] = ''; + + const res = await request(makeApp()).get('/push/vapid-public-key'); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ configured: false, vapidPublicKey: null }); + }); +}); diff --git a/apps/backend/src/config.ts b/apps/backend/src/config.ts index ccc89079..d5d5ed76 100644 --- a/apps/backend/src/config.ts +++ b/apps/backend/src/config.ts @@ -36,6 +36,11 @@ export const EnvSchema = z.object({ OBJECT_STORE_SECRET_KEY: z.string().min(1, 'OBJECT_STORE_SECRET_KEY is required'), OBJECT_STORE_REGION: z.string().min(1, 'OBJECT_STORE_REGION is required'), OBJECT_STORE_FORCE_PATH_STYLE: booleanEnv, + IDEMPOTENCY_TTL_SECONDS: z.coerce + .number() + .int('IDEMPOTENCY_TTL_SECONDS must be an integer') + .positive('IDEMPOTENCY_TTL_SECONDS must be positive') + .optional(), }); export type Env = z.infer; diff --git a/apps/backend/src/index.ts b/apps/backend/src/index.ts index d2dedf89..e37fc094 100644 --- a/apps/backend/src/index.ts +++ b/apps/backend/src/index.ts @@ -21,6 +21,8 @@ import { setOnline, unregisterPresenceSocket, deriveDevicePresence, + scheduleOfflineBroadcast, + cancelPendingOfflineBroadcast, } from './services/presence.js'; import { startHeartbeatTimer, clearHeartbeatTimer } from './services/heartbeat.js'; import { @@ -201,13 +203,19 @@ io.on('connection', async (socket: AuthSocket) => { await registerPresenceSocket(appRedis, userId, deviceId, socket.id); await cleanupStaleSockets(io, appRedis, userId, socket.id); + // #345 — a device reconnecting cancels any offline broadcast still + // pending from a very recent disconnect (same user, any device). When + // that happens the reconnect is invisible to observers too: no + // offline/online pair, since nothing was ever announced offline. + const cancelledPendingOffline = cancelPendingOfflineBroadcast(userId); + const becameOnline = await setOnline(appRedis, userId, deviceId); const connectUser = await db.query.users.findFirst({ where: eq(users.id, userId), columns: { presenceVisible: true }, }); const presenceVisible = connectUser?.presenceVisible ?? true; - if (becameOnline && presenceVisible) { + if (becameOnline && presenceVisible && !cancelledPendingOffline) { for (const m of memberships) { io.to(conversationRoom(m.conversationId)).emit('user_online', { userId }); io.to(conversationRoom(m.conversationId)).emit('presence_update', { @@ -313,33 +321,39 @@ io.on('connection', async (socket: AuthSocket) => { const presenceVisible = user?.presenceVisible ?? true; if (presenceVisible) { - const memberships = await db.query.conversationMembers.findMany({ - where: eq(conversationMembers.userId, userId), - columns: { conversationId: true }, - }); - - const { lastSeen } = await deriveDevicePresence(userId); - - for (const m of memberships) { - io.to(conversationRoom(m.conversationId)).emit('user_offline', { userId }); - io.to(conversationRoom(m.conversationId)).emit('presence_update', { - userId, - online: false, - ...(lastSeen ? { lastSeen } : {}), + // #345 — defer the broadcast by the configured grace window instead + // of announcing offline immediately. A reconnect within the window + // cancels this via cancelPendingOfflineBroadcast() in the connect + // handler above, so a brief blip never produces an offline/online pair. + scheduleOfflineBroadcast(userId, async () => { + const memberships = await db.query.conversationMembers.findMany({ + where: eq(conversationMembers.userId, userId), + columns: { conversationId: true }, }); - // Also emit to direct conversation room for backward compatibility - io.to(m.conversationId).emit('user_offline', { userId }); - io.to(m.conversationId).emit('presence_update', { + + const { lastSeen } = await deriveDevicePresence(userId); + + for (const m of memberships) { + io.to(conversationRoom(m.conversationId)).emit('user_offline', { userId }); + io.to(conversationRoom(m.conversationId)).emit('presence_update', { + userId, + online: false, + ...(lastSeen ? { lastSeen } : {}), + }); + // Also emit to direct conversation room for backward compatibility + io.to(m.conversationId).emit('user_offline', { userId }); + io.to(m.conversationId).emit('presence_update', { + userId, + online: false, + ...(lastSeen ? { lastSeen } : {}), + }); + } + await recordPresenceForCoMembers( userId, - online: false, - ...(lastSeen ? { lastSeen } : {}), - }); - } - await recordPresenceForCoMembers( - userId, - false, - memberships.map((m) => m.conversationId), - ); + false, + memberships.map((m) => m.conversationId), + ); + }); } } } diff --git a/apps/backend/src/routes/push.ts b/apps/backend/src/routes/push.ts index b4e31ca0..44252080 100644 --- a/apps/backend/src/routes/push.ts +++ b/apps/backend/src/routes/push.ts @@ -8,6 +8,24 @@ import { requireAuth, type AuthRequest } from '../middleware/auth.js'; export const pushRouter: IRouter = Router(); pushRouter.use(requireAuth); +/** + * Issue #349 — single source of truth for the VAPID public key, so the + * frontend never configures it independently of the backend's own + * VAPID_PUBLIC_KEY/VAPID_PRIVATE_KEY pair (a drift between the two silently + * breaks push delivery). `configured: false` lets the frontend skip push + * registration gracefully when the backend has no VAPID keys set up. + */ +pushRouter.get('/vapid-public-key', (_req: AuthRequest, res) => { + const vapidPublicKey = process.env['VAPID_PUBLIC_KEY']; + + if (!vapidPublicKey) { + res.status(200).json({ configured: false, vapidPublicKey: null }); + return; + } + + res.status(200).json({ configured: true, vapidPublicKey }); +}); + pushRouter.post('/subscriptions', async (req: AuthRequest, res) => { const deviceId = req.auth!.deviceId; const { endpoint, keys } = req.body; diff --git a/apps/backend/src/services/presence.ts b/apps/backend/src/services/presence.ts index f58c785c..7b168bac 100644 --- a/apps/backend/src/services/presence.ts +++ b/apps/backend/src/services/presence.ts @@ -36,6 +36,63 @@ import { devices, conversationMembers } from '../db/schema.js'; const PRESENCE_TTL = 90; // seconds const SOCKET_MAPPING_PREFIX = 'presence:sockets:'; +const DEFAULT_OFFLINE_GRACE_MS = 5_000; + +function getOfflineGraceMs(): number { + const raw = process.env.PRESENCE_OFFLINE_GRACE_MS; + if (!raw) return DEFAULT_OFFLINE_GRACE_MS; + const parsed = Number.parseInt(raw, 10); + return Number.isFinite(parsed) && parsed >= 0 ? parsed : DEFAULT_OFFLINE_GRACE_MS; +} + +// Per-user pending "went offline" broadcast timers (issue #345). A user +// going fully offline schedules its broadcast here instead of firing it +// immediately; a reconnect within the grace window cancels the timer so a +// brief disconnect+reconnect blip never produces an offline/online pair. +const pendingOfflineBroadcasts = new Map>(); + +/** + * Defer a "user went offline" broadcast by the configured grace window. + * Call only once the caller has confirmed the user has zero remaining + * live devices/sockets — this schedules the *broadcast*, not the + * underlying presence-state removal, which should already be immediate. + */ +export function scheduleOfflineBroadcast( + userId: string, + broadcast: () => void | Promise, +): void { + const existing = pendingOfflineBroadcasts.get(userId); + if (existing) clearTimeout(existing); + + const timer = setTimeout(() => { + pendingOfflineBroadcasts.delete(userId); + void broadcast(); + }, getOfflineGraceMs()); + + // Let a pending offline broadcast never hold the process open on its own. + timer.unref?.(); + pendingOfflineBroadcasts.set(userId, timer); +} + +/** + * Cancel a pending offline broadcast for a user, e.g. because a device + * reconnected within the grace window. Returns true if a pending + * broadcast was actually found and cancelled. + */ +export function cancelPendingOfflineBroadcast(userId: string): boolean { + const timer = pendingOfflineBroadcasts.get(userId); + if (!timer) return false; + clearTimeout(timer); + pendingOfflineBroadcasts.delete(userId); + return true; +} + +/** Test hook: clear all pending timers between test cases. */ +export function __resetOfflineBroadcastsForTesting(): void { + for (const timer of pendingOfflineBroadcasts.values()) clearTimeout(timer); + pendingOfflineBroadcasts.clear(); +} + type RedisWithOptionalHashRead = Redis & { hgetall?: (key: string) => Promise>; }; diff --git a/apps/backend/src/socket/dispatcher.ts b/apps/backend/src/socket/dispatcher.ts index 4d0c3930..4cdf9d74 100644 --- a/apps/backend/src/socket/dispatcher.ts +++ b/apps/backend/src/socket/dispatcher.ts @@ -10,7 +10,16 @@ import { type Handler = (payload: Record) => Promise; -const IDEMPOTENCY_TTL_SECONDS = 86_400; // 24 h +const DEFAULT_IDEMPOTENCY_TTL_SECONDS = 86_400; // 24 h + +// Read lazily (not at module load) so tests can override +// process.env.IDEMPOTENCY_TTL_SECONDS per-case without a module reset. +function getIdempotencyTtlSeconds(): number { + const raw = process.env.IDEMPOTENCY_TTL_SECONDS; + if (!raw) return DEFAULT_IDEMPOTENCY_TTL_SECONDS; + const parsed = Number.parseInt(raw, 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : DEFAULT_IDEMPOTENCY_TTL_SECONDS; +} export class EventDispatcher { private handlers = new Map(); @@ -81,7 +90,7 @@ export class EventDispatcher { if (this.redis) { const idempotencyKey = `event:idempotency:${envelope.eventId}`; const set = await this.redis - .set(idempotencyKey, '1', 'EX', IDEMPOTENCY_TTL_SECONDS, 'NX') + .set(idempotencyKey, '1', 'EX', getIdempotencyTtlSeconds(), 'NX') .catch(() => null); if (set === null) { // Already processed — acknowledge without re-running. diff --git a/apps/backend/src/socket/messaging.ts b/apps/backend/src/socket/messaging.ts index 81708de0..355173f1 100644 --- a/apps/backend/src/socket/messaging.ts +++ b/apps/backend/src/socket/messaging.ts @@ -332,6 +332,13 @@ export function registerMessagingHandlers(io: Server, socket: AuthSocket): void }); // ── send_file_message ────────────────────────────────────────────────────── + // Issue #347: routes through the same deliverMessage pipeline send_message + // uses, so file messages get identical per-device receipts, resume/sync + // backfill, and fan-out validation. `content` is the message-body envelope + // ciphertext (as before); `envelopes` carries the file's symmetric + // encryption key, individually sealed per recipient device — the key is + // never accepted or stored as a server-visible plaintext field, only + // inside each envelope's opaque ciphertext. socket.on( 'send_file_message', async (payload: { @@ -365,6 +372,14 @@ export function registerMessagingHandlers(io: Server, socket: AuthSocket): void return; } + if (!Array.isArray(envelopes) || envelopes.length === 0) { + socket.emit('error', { + event: 'send_file_message', + message: 'envelopes are required for file messages (they carry the encrypted file key)', + }); + return; + } + const membership = await db.query.conversationMembers.findFirst({ where: and( eq(conversationMembers.conversationId, conversationId), diff --git a/apps/web/next.config.ts b/apps/web/next.config.ts index b8df440d..b22af960 100644 --- a/apps/web/next.config.ts +++ b/apps/web/next.config.ts @@ -1,9 +1,5 @@ import type { NextConfig } from 'next'; -const nextConfig: NextConfig = { - env: { - NEXT_PUBLIC_VAPID_PUBLIC_KEY: process.env.NEXT_PUBLIC_VAPID_PUBLIC_KEY ?? '', - }, -}; +const nextConfig: NextConfig = {}; export default nextConfig; diff --git a/apps/web/src/hooks/usePushSubscription.test.ts b/apps/web/src/hooks/usePushSubscription.test.ts new file mode 100644 index 00000000..932443e7 --- /dev/null +++ b/apps/web/src/hooks/usePushSubscription.test.ts @@ -0,0 +1,77 @@ +/** + * Tests for fetchVapidPublicKey (#349). + * + * The frontend must fetch the VAPID public key from the backend instead of + * a build-time env var, and gracefully return null (skip push registration) + * whenever the backend has no key configured, the request fails, or the + * response can't be parsed — it must never throw. + */ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { fetchVapidPublicKey } from './usePushSubscription'; + +function mockFetchOnce(response: { ok: boolean; json: () => Promise }) { + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue(response as unknown as Response), + ); +} + +describe('fetchVapidPublicKey', () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('returns the key when the backend reports it is configured', async () => { + mockFetchOnce({ + ok: true, + json: () => Promise.resolve({ configured: true, vapidPublicKey: 'abc123' }), + }); + + const key = await fetchVapidPublicKey('token-1'); + + expect(key).toBe('abc123'); + expect(fetch).toHaveBeenCalledWith( + expect.stringContaining('/push/vapid-public-key'), + expect.objectContaining({ headers: { Authorization: 'Bearer token-1' } }), + ); + }); + + it('returns null when the backend reports VAPID is not configured', async () => { + mockFetchOnce({ + ok: true, + json: () => Promise.resolve({ configured: false, vapidPublicKey: null }), + }); + + expect(await fetchVapidPublicKey('token-1')).toBeNull(); + }); + + it('returns null when configured is true but the key is missing (defensive)', async () => { + mockFetchOnce({ + ok: true, + json: () => Promise.resolve({ configured: true, vapidPublicKey: null }), + }); + + expect(await fetchVapidPublicKey('token-1')).toBeNull(); + }); + + it('returns null on a non-2xx response instead of throwing', async () => { + mockFetchOnce({ ok: false, json: () => Promise.resolve({}) }); + + expect(await fetchVapidPublicKey('token-1')).toBeNull(); + }); + + it('returns null when fetch itself rejects (network failure) instead of throwing', async () => { + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network down'))); + + await expect(fetchVapidPublicKey('token-1')).resolves.toBeNull(); + }); + + it('returns null when the response body is not valid JSON', async () => { + mockFetchOnce({ + ok: true, + json: () => Promise.reject(new Error('invalid JSON')), + }); + + await expect(fetchVapidPublicKey('token-1')).resolves.toBeNull(); + }); +}); diff --git a/apps/web/src/hooks/usePushSubscription.ts b/apps/web/src/hooks/usePushSubscription.ts index a5fda1bd..ed1e3603 100644 --- a/apps/web/src/hooks/usePushSubscription.ts +++ b/apps/web/src/hooks/usePushSubscription.ts @@ -3,9 +3,6 @@ import { useCallback, useEffect, useState } from 'react'; import { API_BASE_URL } from '@/lib/api'; -// Loaded at build time — must be set in the environment. -const VAPID_PUBLIC_KEY = process.env.NEXT_PUBLIC_VAPID_PUBLIC_KEY ?? ''; - // Web Push requires the VAPID key as a Uint8Array in base64url encoding. function vapidKeyToUint8Array(base64url: string): Uint8Array { const padding = '='.repeat((4 - (base64url.length % 4)) % 4); @@ -18,6 +15,26 @@ function vapidKeyToUint8Array(base64url: string): Uint8Array { return bytes; } +/** + * Issue #349 — the VAPID public key comes from the backend, not a build-time + * env var. It must match the private key the backend signs pushes with; + * configuring it separately on the frontend risks silent drift between the + * two. Returns null (and lets the caller skip push registration) whenever + * the backend has no key configured or the request fails. + */ +export async function fetchVapidPublicKey(token: string): Promise { + try { + const res = await fetch(`${API_BASE_URL}/push/vapid-public-key`, { + headers: { Authorization: `Bearer ${token}` }, + }); + if (!res.ok) return null; + const data = (await res.json()) as { configured: boolean; vapidPublicKey: string | null }; + return data.configured && data.vapidPublicKey ? data.vapidPublicKey : null; + } catch { + return null; + } +} + async function postSubscription(sub: PushSubscription, token: string): Promise { const json = sub.toJSON(); await fetch(`${API_BASE_URL}/push/subscriptions`, { @@ -44,6 +61,7 @@ export type PushSubscriptionState = { export function usePushSubscription(token: string | null): PushSubscriptionState { const [registration, setRegistration] = useState(null); + const [vapidPublicKey, setVapidPublicKey] = useState(null); const [permission, setPermission] = useState(() => { if (typeof window !== 'undefined' && 'Notification' in window) { return Notification.permission; @@ -72,9 +90,23 @@ export function usePushSubscription(token: string | null): PushSubscriptionState }; }, []); + // Fetch the backend-configured VAPID public key once a token is available. + useEffect(() => { + if (!token) return; + + let active = true; + fetchVapidPublicKey(token).then((key) => { + if (active) setVapidPublicKey(key); + }); + + return () => { + active = false; + }; + }, [token]); + // Re-use an existing subscription if one already exists. useEffect(() => { - if (!registration || !token || !VAPID_PUBLIC_KEY) return; + if (!registration || !token || !vapidPublicKey) return; if (Notification.permission !== 'granted') return; let active = true; @@ -88,10 +120,10 @@ export function usePushSubscription(token: string | null): PushSubscriptionState return () => { active = false; }; - }, [registration, token]); + }, [registration, token, vapidPublicKey]); const requestSubscription = useCallback(async () => { - if (!registration || !token || !VAPID_PUBLIC_KEY) return; + if (!registration || !token || !vapidPublicKey) return; const result = await Notification.requestPermission(); setPermission(result); @@ -102,13 +134,13 @@ export function usePushSubscription(token: string | null): PushSubscriptionState if (!sub) { sub = await registration.pushManager.subscribe({ userVisibleOnly: true, - applicationServerKey: vapidKeyToUint8Array(VAPID_PUBLIC_KEY), + applicationServerKey: vapidKeyToUint8Array(vapidPublicKey), }); } await postSubscription(sub, token); setSubscribed(true); - }, [registration, token]); + }, [registration, token, vapidPublicKey]); return { permission, subscribed, requestSubscription }; } diff --git a/apps/web/vitest.config.ts b/apps/web/vitest.config.ts index fa9617e8..a3d41c8b 100644 --- a/apps/web/vitest.config.ts +++ b/apps/web/vitest.config.ts @@ -1,6 +1,12 @@ +import { fileURLToPath } from 'url'; import { defineConfig } from 'vitest/config'; export default defineConfig({ + resolve: { + alias: { + '@': fileURLToPath(new URL('./src', import.meta.url)), + }, + }, test: { environment: 'node', include: ['src/**/*.test.ts'],