diff --git a/apps/backend/drizzle/0001_audit_logs.sql b/apps/backend/drizzle/0001_audit_logs.sql new file mode 100644 index 00000000..36f39282 --- /dev/null +++ b/apps/backend/drizzle/0001_audit_logs.sql @@ -0,0 +1,35 @@ +CREATE TYPE "public"."audit_action" AS ENUM('device_linked', 'device_revoked', 'logout_everywhere', 'key_bundle_drained', 'auth_failed', 'file_access_denied', 'group_member_added', 'group_member_removed');--> statement-breakpoint +CREATE TABLE "audit_logs" ( + "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, + "action" "audit_action" NOT NULL, + "actor_user_id" uuid, + "actor_device_id" uuid, + "subject_user_id" uuid, + "target_type" text, + "target_id" text, + "ip_address" text, + "user_agent" text, + "metadata" jsonb, + "created_at" timestamp DEFAULT now() NOT NULL +); +--> statement-breakpoint +CREATE INDEX "audit_logs_subject_created_idx" ON "audit_logs" USING btree ("subject_user_id","created_at");--> statement-breakpoint +CREATE INDEX "audit_logs_actor_created_idx" ON "audit_logs" USING btree ("actor_user_id","created_at");--> statement-breakpoint +CREATE INDEX "audit_logs_action_created_idx" ON "audit_logs" USING btree ("action","created_at");--> statement-breakpoint +-- Append-only enforcement (#376). Enforced in the database rather than by +-- convention: the log is only useful to an incident responder if the +-- application account an attacker would already have reached cannot rewrite +-- or erase it. Retention pruning is therefore a deliberate, privileged +-- operation — drop the trigger, prune, recreate it — not something a stray +-- UPDATE or DELETE can do. The actor/subject columns carry no foreign keys +-- for the same reason: a cascade would delete the history along with the +-- account it incriminates. +CREATE OR REPLACE FUNCTION audit_logs_reject_mutation() RETURNS trigger AS $$ +BEGIN + RAISE EXCEPTION 'audit_logs is append-only; % is not permitted', TG_OP + USING ERRCODE = 'restrict_violation'; +END; +$$ LANGUAGE plpgsql;--> statement-breakpoint +CREATE TRIGGER audit_logs_no_mutation + BEFORE UPDATE OR DELETE OR TRUNCATE ON "audit_logs" + FOR EACH STATEMENT EXECUTE FUNCTION audit_logs_reject_mutation(); \ No newline at end of file diff --git a/apps/backend/src/__tests__/auditLog.test.ts b/apps/backend/src/__tests__/auditLog.test.ts new file mode 100644 index 00000000..8c9a71d5 --- /dev/null +++ b/apps/backend/src/__tests__/auditLog.test.ts @@ -0,0 +1,368 @@ +/** + * #376 — audit logging for security-relevant events. + * + * Covers the two invariants that matter: every recorded event carries an + * actor, device and timestamp, and no message content ever reaches a row. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import express from 'express'; +import request from 'supertest'; + +const insertedRows: Array> = []; +const mockValues = vi.fn(async (row: Record) => { + insertedRows.push(row); +}); +const mockInsert = vi.fn(() => ({ values: mockValues })); +const mockAuditFindMany = vi.fn(); + +vi.mock('../db/index.js', () => ({ + db: { + insert: mockInsert, + query: { + auditLogs: { findMany: mockAuditFindMany }, + }, + }, +})); + +let currentAuth: { userId: string; deviceId: string } | undefined = { + userId: 'user-alice', + deviceId: 'device-alice', +}; + +vi.mock('../middleware/auth.js', () => ({ + requireAuth: (req: express.Request, _res: express.Response, next: express.NextFunction) => { + (req as express.Request & { auth?: typeof currentAuth }).auth = currentAuth; + next(); + }, +})); + +const { + recordAuditEvent, + sanitizeMetadata, + queryAuditLog, + encodeAuditCursor, + decodeAuditCursor, + actorFromRequest, +} = await import('../services/auditLog.js'); +const { auditLogsRouter } = await import('../routes/auditLogs.js'); + +function makeApp() { + const app = express(); + app.use(express.json()); + app.use('/audit-logs', auditLogsRouter); + return app; +} + +/** + * Every string reachable from a drizzle condition tree. The tree is cyclic + * (columns point back at their table), so it cannot simply be stringified. + */ +function collectStrings(value: unknown, seen = new WeakSet()): string[] { + if (typeof value === 'string') return [value]; + if (typeof value !== 'object' || value === null) return []; + if (seen.has(value)) return []; + seen.add(value); + return Object.values(value).flatMap((entry) => collectStrings(entry, seen)); +} + +beforeEach(() => { + vi.clearAllMocks(); + insertedRows.length = 0; + currentAuth = { userId: 'user-alice', deviceId: 'device-alice' }; + mockInsert.mockReturnValue({ values: mockValues }); +}); + +// ─── recording ──────────────────────────────────────────────────────────────── + +describe('recordAuditEvent', () => { + it('AC1 — records actor, device, subject and target for a security event', async () => { + await recordAuditEvent({ + action: 'device_revoked', + actorUserId: 'user-alice', + actorDeviceId: 'device-alice', + targetType: 'device', + targetId: 'device-bob', + ipAddress: '203.0.113.7', + userAgent: 'clicked-web/1.0', + metadata: { selfRevocation: false, remainingActiveDevices: 2 }, + }); + + expect(insertedRows).toHaveLength(1); + expect(insertedRows[0]).toMatchObject({ + action: 'device_revoked', + actorUserId: 'user-alice', + actorDeviceId: 'device-alice', + // Subject defaults to the actor when the event is about their own account. + subjectUserId: 'user-alice', + targetType: 'device', + targetId: 'device-bob', + ipAddress: '203.0.113.7', + metadata: { selfRevocation: false, remainingActiveDevices: 2 }, + }); + }); + + it('keeps the subject distinct when the event was done to someone else', async () => { + await recordAuditEvent({ + action: 'key_bundle_drained', + actorUserId: 'user-mallory', + actorDeviceId: 'device-mallory', + subjectUserId: 'user-alice', + targetType: 'device', + targetId: 'device-alice', + }); + + expect(insertedRows[0]).toMatchObject({ + actorUserId: 'user-mallory', + subjectUserId: 'user-alice', + }); + }); + + it('records an unauthenticated failure with a null actor', async () => { + await recordAuditEvent({ + action: 'auth_failed', + targetType: 'wallet', + targetId: 'GABC', + metadata: { reason: 'signature_verification_failed' }, + }); + + expect(insertedRows[0]).toMatchObject({ + action: 'auth_failed', + actorUserId: null, + actorDeviceId: null, + subjectUserId: null, + }); + }); + + it('never lets a failed audit write break the action it observes', async () => { + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + mockInsert.mockImplementation(() => { + throw new Error('audit table unavailable'); + }); + + await expect( + recordAuditEvent({ action: 'device_revoked', actorUserId: 'user-alice' }), + ).resolves.toBeUndefined(); + expect(error).toHaveBeenCalled(); + + error.mockRestore(); + }); + + it('derives actor and request context from an authenticated request', async () => { + const captured: Array> = []; + const app = express(); + app.get('/probe', (req, res) => { + (req as express.Request & { auth?: typeof currentAuth }).auth = currentAuth; + captured.push(actorFromRequest(req)); + res.json({ ok: true }); + }); + + await request(app).get('/probe').set('User-Agent', 'clicked-web/2.0'); + + expect(captured[0]).toMatchObject({ + actorUserId: 'user-alice', + actorDeviceId: 'device-alice', + userAgent: 'clicked-web/2.0', + }); + expect(captured[0]?.ipAddress).toBeTruthy(); + }); +}); + +// ─── redaction ──────────────────────────────────────────────────────────────── + +describe('AC2 — no message content reaches the log', () => { + it('redacts content-shaped keys however they are spelled', () => { + const sanitized = sanitizeMetadata({ + ciphertext: 'AAECAwQ=', + cipherText: 'AAECAwQ=', + content: 'hello there', + messageBody: 'hello there', + plaintext: 'hello there', + envelope_ciphertext: 'AAECAwQ=', + token: 'ey.jwt.value', + preKey: 'secret-key-material', + conversationId: 'conv-1', + memberCount: 4, + }); + + expect(sanitized).toEqual({ + ciphertext: '[redacted]', + cipherText: '[redacted]', + content: '[redacted]', + messageBody: '[redacted]', + plaintext: '[redacted]', + envelope_ciphertext: '[redacted]', + token: '[redacted]', + preKey: '[redacted]', + conversationId: 'conv-1', + memberCount: 4, + }); + }); + + it('redacts content nested one level down as well', () => { + const sanitized = sanitizeMetadata({ + message: { ciphertext: 'AAECAwQ=' }, + device: { id: 'device-1', ciphertext: 'AAECAwQ=' }, + }); + + expect(sanitized).toEqual({ + message: '[redacted]', + device: { id: 'device-1', ciphertext: '[redacted]' }, + }); + }); + + it('bounds strings, arrays, key counts and nesting depth', () => { + const sanitized = sanitizeMetadata({ + long: 'x'.repeat(1000), + list: Array.from({ length: 100 }, (_, i) => i), + deep: { level2: { level3: { smuggled: 'x'.repeat(1000) } } }, + ...Object.fromEntries(Array.from({ length: 40 }, (_, i) => [`k${i}`, i])), + }); + + expect((sanitized?.['long'] as string).length).toBeLessThanOrEqual(257); + expect(sanitized?.['list']).toHaveLength(20); + expect(sanitized?.['deep']).toEqual({ level2: '[object]' }); + expect(Object.keys(sanitized ?? {}).length).toBeLessThanOrEqual(20); + }); + + it('drops values an incident responder cannot use, and empty metadata', () => { + expect(sanitizeMetadata({ fn: () => undefined, missing: undefined, kept: 1 })).toEqual({ + kept: 1, + }); + expect(sanitizeMetadata(undefined)).toBeNull(); + expect(sanitizeMetadata({})).toBeNull(); + }); + + it('sanitizes on the write path, not just when called directly', async () => { + await recordAuditEvent({ + action: 'file_access_denied', + actorUserId: 'user-mallory', + metadata: { ciphertext: 'AAECAwQ=', reason: 'not_a_member' }, + }); + + expect(insertedRows[0]?.['metadata']).toEqual({ + ciphertext: '[redacted]', + reason: 'not_a_member', + }); + }); +}); + +// ─── querying ───────────────────────────────────────────────────────────────── + +describe('AC3 — logs are queryable for an account', () => { + const rows = [ + { + id: 'evt-2', + action: 'device_revoked', + actorUserId: 'user-alice', + actorDeviceId: 'device-alice', + subjectUserId: 'user-alice', + targetType: 'device', + targetId: 'device-old', + ipAddress: '203.0.113.7', + userAgent: 'clicked-web/1.0', + metadata: { selfRevocation: false }, + createdAt: new Date('2026-07-02T10:00:00Z'), + }, + { + id: 'evt-1', + action: 'key_bundle_drained', + actorUserId: 'user-mallory', + actorDeviceId: 'device-mallory', + subjectUserId: 'user-alice', + targetType: 'device', + targetId: 'device-alice', + ipAddress: '198.51.100.4', + userAgent: null, + metadata: { oneTimePreKeysRemaining: 0, exhausted: true }, + createdAt: new Date('2026-07-01T10:00:00Z'), + }, + ]; + + it('returns the account history newest first, with a page cursor', async () => { + mockAuditFindMany.mockResolvedValue(rows); + + const result = await queryAuditLog({ userId: 'user-alice', limit: 10 }); + + expect(result.events).toHaveLength(2); + expect(result.hasMore).toBe(false); + expect(result.nextCursor).toBeNull(); + expect(mockAuditFindMany).toHaveBeenCalledWith(expect.objectContaining({ limit: 11 })); + }); + + it('reports another page when one more row than the limit came back', async () => { + mockAuditFindMany.mockResolvedValue(rows); + + const result = await queryAuditLog({ userId: 'user-alice', limit: 1 }); + + expect(result.events).toHaveLength(1); + expect(result.hasMore).toBe(true); + expect(result.nextCursor).toBe(encodeAuditCursor(rows[0]!)); + }); + + it('round-trips a cursor, tie-breaking on id within a millisecond', () => { + const cursor = encodeAuditCursor(rows[0]!); + expect(decodeAuditCursor(cursor)).toEqual({ + createdAt: rows[0]!.createdAt, + id: 'evt-2', + }); + expect(decodeAuditCursor('garbage')).toBeNull(); + }); + + it('serves the caller their own history, marking what was done to them', async () => { + mockAuditFindMany.mockResolvedValue(rows); + + const res = await request(makeApp()).get('/audit-logs'); + + expect(res.status).toBe(200); + expect(res.body.events).toHaveLength(2); + expect(res.body.events[0]).toMatchObject({ + id: 'evt-2', + action: 'device_revoked', + direction: 'performed', + }); + // Mallory drained Alice's bundle — Alice's own log must show it. + expect(res.body.events[1]).toMatchObject({ + id: 'evt-1', + action: 'key_bundle_drained', + actorUserId: 'user-mallory', + direction: 'received', + }); + }); + + it('has no parameter for reading someone else’s log', async () => { + mockAuditFindMany.mockResolvedValue([]); + + await request(makeApp()).get('/audit-logs?userId=user-bob&subjectUserId=user-bob'); + + // Whatever the query string claims, the scope is the authenticated caller. + expect(mockAuditFindMany).toHaveBeenCalledTimes(1); + const call = mockAuditFindMany.mock.calls[0]![0] as { where: unknown }; + const bound = collectStrings(call.where); + expect(bound).toContain('user-alice'); + expect(bound).not.toContain('user-bob'); + }); + + it('rejects an unknown action filter instead of silently ignoring it', async () => { + const res = await request(makeApp()).get('/audit-logs?action=not_an_action'); + + expect(res.status).toBe(400); + expect(mockAuditFindMany).not.toHaveBeenCalled(); + }); + + it('accepts a known action filter', async () => { + mockAuditFindMany.mockResolvedValue([]); + + const res = await request(makeApp()).get('/audit-logs?action=auth_failed'); + + expect(res.status).toBe(200); + expect(mockAuditFindMany).toHaveBeenCalledTimes(1); + }); + + it('clamps the page size so one request cannot drain the table', async () => { + mockAuditFindMany.mockResolvedValue([]); + + await request(makeApp()).get('/audit-logs?limit=100000'); + + expect(mockAuditFindMany).toHaveBeenCalledWith(expect.objectContaining({ limit: 201 })); + }); +}); diff --git a/apps/backend/src/__tests__/devices.revoke.test.ts b/apps/backend/src/__tests__/devices.revoke.test.ts index f9e0055d..315c8d1e 100644 --- a/apps/backend/src/__tests__/devices.revoke.test.ts +++ b/apps/backend/src/__tests__/devices.revoke.test.ts @@ -51,6 +51,18 @@ vi.mock('../services/deviceRevocation.js', () => ({ vi.mock('../lib/socket.js', () => ({ getSocketServer: vi.fn(() => null) })); +// #376 — revocation is a security event; assert it reaches the audit log. +const mockRecordAuditEvent = vi.fn().mockResolvedValue(undefined); +vi.mock('../services/auditLog.js', () => ({ + recordAuditEvent: mockRecordAuditEvent, + actorFromRequest: (req: { auth?: { userId: string; deviceId: string } }) => ({ + actorUserId: req.auth?.userId ?? null, + actorDeviceId: req.auth?.deviceId ?? null, + ipAddress: '127.0.0.1', + userAgent: 'test', + }), +})); + vi.mock('drizzle-orm', () => ({ eq: vi.fn((col: unknown, val: unknown) => ({ op: 'eq', col, val })), and: vi.fn((...args: unknown[]) => ({ op: 'and', args })), @@ -176,6 +188,27 @@ describe('DELETE /devices/:id', () => { expect(mockMarkDeviceRevoked).toHaveBeenCalledWith('device-2'); expect(mockPublish).toHaveBeenCalledWith('device_revoked:device-2', '1'); }); + + it('records the revocation in the audit log with actor, device and context (#376)', async () => { + mockDeviceFindFirst.mockResolvedValue(ACTIVE_DEVICE); + setupActiveCount(2); + + await request(makeApp()).delete('/devices/device-2'); + + expect(mockRecordAuditEvent).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'device_revoked', + actorUserId: 'owner-user-id', + actorDeviceId: 'device-1', + targetType: 'device', + targetId: 'device-2', + metadata: expect.objectContaining({ + selfRevocation: false, + remainingActiveDevices: 1, + }), + }), + ); + }); }); describe('POST /devices/logout-everywhere', () => { @@ -200,4 +233,40 @@ describe('POST /devices/logout-everywhere', () => { expect(res.body).toEqual({ revokedCount: 0 }); expect(mockPublish).not.toHaveBeenCalled(); }); + + it('audits the account-wide action and each device it revoked (#376)', async () => { + mockFindMany.mockResolvedValue([{ id: 'device-2' }, { id: 'device-3' }]); + + await request(makeApp()).post('/devices/logout-everywhere'); + + expect(mockRecordAuditEvent).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'logout_everywhere', + actorUserId: 'owner-user-id', + metadata: expect.objectContaining({ revokedCount: 2, retainedDeviceId: 'device-1' }), + }), + ); + for (const id of ['device-2', 'device-3']) { + expect(mockRecordAuditEvent).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'device_revoked', + targetId: id, + metadata: { viaLogoutEverywhere: true }, + }), + ); + } + }); + + it('audits the attempt even when there was nothing to revoke (#376)', async () => { + mockFindMany.mockResolvedValue([]); + + await request(makeApp()).post('/devices/logout-everywhere'); + + expect(mockRecordAuditEvent).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'logout_everywhere', + metadata: expect.objectContaining({ revokedCount: 0 }), + }), + ); + }); }); diff --git a/apps/backend/src/db/schema.ts b/apps/backend/src/db/schema.ts index 738e7fb0..6153a34b 100644 --- a/apps/backend/src/db/schema.ts +++ b/apps/backend/src/db/schema.ts @@ -369,6 +369,73 @@ export const pushSubscriptions = pgTable('push_subscriptions', { export type PushSubscription = typeof pushSubscriptions.$inferSelect; export type NewPushSubscription = typeof pushSubscriptions.$inferInsert; +// ─── Audit log (#376) ───────────────────────────────────────────────────────── +// +// Append-only record of security-relevant events, for incident response. +// Nothing here may contain message content: an audit trail that leaks +// plaintext would undo the end-to-end encryption it exists to protect. Rows +// carry identifiers, counts and outcomes only — `services/auditLog.ts` +// strips anything content-shaped before it reaches the database. +// +// Append-only is enforced in the database itself (see the migration's +// `audit_logs_no_mutation` trigger), not just by convention, because the +// value of the log to an incident responder depends on it not being editable +// by the same application account an attacker would already have reached. +// +// `actorUserId` is who did it; `subjectUserId` is whose account it happened +// to. They differ for exactly the events that matter most — someone else's +// device fetching your key bundle, a failed sign-in against your wallet — and +// the account-scoped query indexes on the subject so a user's own history +// includes what was done *to* them, not just by them. + +export const auditActionEnum = pgEnum('audit_action', [ + 'device_linked', + 'device_revoked', + 'logout_everywhere', + 'key_bundle_drained', + 'auth_failed', + 'file_access_denied', + 'group_member_added', + 'group_member_removed', +]); + +export type AuditAction = (typeof auditActionEnum.enumValues)[number]; + +export const auditLogs = pgTable( + 'audit_logs', + { + id: uuid('id').primaryKey().defaultRandom(), + action: auditActionEnum('action').notNull(), + // Deliberately *not* foreign keys. An audit row must record what was true + // when it was written and stay that way: a cascade would delete history + // along with the account it incriminates, and ON DELETE SET NULL would + // issue an UPDATE that the append-only trigger correctly refuses. Ids are + // stored plain, and a responder resolves them (or finds them gone) at + // read time. Nullable because a failed sign-in has no established actor. + actorUserId: uuid('actor_user_id'), + actorDeviceId: uuid('actor_device_id'), + subjectUserId: uuid('subject_user_id'), + /** Kind of thing acted on: 'device', 'file', 'conversation', 'wallet'. */ + targetType: text('target_type'), + targetId: text('target_id'), + ipAddress: text('ip_address'), + userAgent: text('user_agent'), + /** Sanitised, bounded key/value context. Never message content. */ + metadata: jsonb('metadata').$type>(), + createdAt: timestamp('created_at').notNull().defaultNow(), + }, + (table) => [ + // Account-scoped queries are the primary read path. + index('audit_logs_subject_created_idx').on(table.subjectUserId, table.createdAt), + index('audit_logs_actor_created_idx').on(table.actorUserId, table.createdAt), + // "Show me every failed auth in the last hour" during an incident. + index('audit_logs_action_created_idx').on(table.action, table.createdAt), + ], +); + +export type AuditLog = typeof auditLogs.$inferSelect; +export type NewAuditLog = typeof auditLogs.$inferInsert; + // ─── Relations ──────────────────────────────────────────────────────────────── export const usersRelations = relations(users, ({ many }) => ({ diff --git a/apps/backend/src/middleware/auth.ts b/apps/backend/src/middleware/auth.ts index 1af30423..70efcedb 100644 --- a/apps/backend/src/middleware/auth.ts +++ b/apps/backend/src/middleware/auth.ts @@ -3,6 +3,7 @@ import { eq, and } from 'drizzle-orm'; import { verifyToken, type JwtPayload } from '../lib/jwt.js'; import { db } from '../db/index.js'; import { devices } from '../db/schema.js'; +import { recordAuditEvent, requestContext } from '../services/auditLog.js'; export interface AuthRequest extends Request { auth?: JwtPayload; @@ -41,6 +42,20 @@ export async function requireAuth( }); if (!device || device.revokedAt) { + // Audited (#376): the token's signature was valid, so this is a real + // credential being replayed after the device lost its authorisation — + // unlike a malformed or expired token, which any scanner produces and + // which would let an unauthenticated caller flood the audit table. + void recordAuditEvent({ + action: 'auth_failed', + subjectUserId: payload.userId, + actorDeviceId: device ? payload.deviceId : null, + targetType: 'device', + targetId: payload.deviceId, + ...requestContext(req), + metadata: { reason: device ? 'device_revoked' : 'device_not_found' }, + }); + res.status(401).json({ error: 'Device not found or has been revoked' }); return; } diff --git a/apps/backend/src/routes/auditLogs.ts b/apps/backend/src/routes/auditLogs.ts new file mode 100644 index 00000000..9dfebf38 --- /dev/null +++ b/apps/backend/src/routes/auditLogs.ts @@ -0,0 +1,72 @@ +/** + * GET /audit-logs — a user's own security history (#376). + * + * Scoped to the caller: there is no parameter for whose log to read, so a + * compromised token cannot be used to enumerate anyone else's security events. + * Returns events where the caller was either the actor or the subject, newest + * first, cursor-paginated. + */ +import { Router, type Router as RouterType } from 'express'; +import { requireAuth, type AuthRequest } from '../middleware/auth.js'; +import { auditActionEnum, type AuditAction } from '../db/schema.js'; +import { + DEFAULT_AUDIT_PAGE_SIZE, + MAX_AUDIT_PAGE_SIZE, + queryAuditLog, +} from '../services/auditLog.js'; + +export const auditLogsRouter: RouterType = Router(); + +auditLogsRouter.use(requireAuth); + +const VALID_ACTIONS = new Set(auditActionEnum.enumValues); + +auditLogsRouter.get('/', async (req: AuthRequest, res) => { + const userId = req.auth!.userId; + + const rawAction = typeof req.query['action'] === 'string' ? req.query['action'] : undefined; + if (rawAction && !VALID_ACTIONS.has(rawAction)) { + res.status(400).json({ + error: 'Unknown action filter', + allowed: auditActionEnum.enumValues, + }); + return; + } + + const rawLimit = Number.parseInt(req.query['limit'] as string, 10); + const limit = Number.isFinite(rawLimit) && rawLimit > 0 ? rawLimit : DEFAULT_AUDIT_PAGE_SIZE; + + const cursor = typeof req.query['cursor'] === 'string' ? req.query['cursor'] : undefined; + + try { + const { events, nextCursor, hasMore } = await queryAuditLog({ + userId, + ...(rawAction ? { action: rawAction as AuditAction } : {}), + ...(cursor ? { cursor } : {}), + limit: Math.min(limit, MAX_AUDIT_PAGE_SIZE), + }); + + res.json({ + events: events.map((event) => ({ + id: event.id, + action: event.action, + actorUserId: event.actorUserId, + actorDeviceId: event.actorDeviceId, + subjectUserId: event.subjectUserId, + targetType: event.targetType, + targetId: event.targetId, + ipAddress: event.ipAddress, + userAgent: event.userAgent, + metadata: event.metadata, + createdAt: event.createdAt, + // Whether this was something the account did, or something done to it. + direction: event.actorUserId === userId ? 'performed' : 'received', + })), + // Null once the end is reached, so a client pages until it is null. + nextCursor, + hasMore, + }); + } catch { + res.status(500).json({ error: 'Failed to read audit log' }); + } +}); diff --git a/apps/backend/src/routes/auth.ts b/apps/backend/src/routes/auth.ts index 5a187d46..339316ce 100644 --- a/apps/backend/src/routes/auth.ts +++ b/apps/backend/src/routes/auth.ts @@ -9,6 +9,7 @@ import { eq, and } from 'drizzle-orm'; import { createNonce, consumeNonce } from '../lib/nonce.js'; import { signToken } from '../lib/jwt.js'; import { validate } from '../middleware/validate.js'; +import { recordAuditEvent, requestContext } from '../services/auditLog.js'; import { ipIdentifier, rateLimit } from '../middleware/rateLimit.js'; import { ChallengeSchema, @@ -56,9 +57,22 @@ authRouter.post( const platform = device?.platform; const registrationId = device?.registrationId; + // Every failed sign-in is audited (#376). The wallet address is the only + // identity available before verification succeeds, and it is a public + // value, so it is safe to record as the target. + const auditFailure = (reason: string) => + recordAuditEvent({ + action: 'auth_failed', + ...requestContext(req), + targetType: 'wallet', + targetId: walletAddress, + metadata: { reason }, + }); + // Validate and consume nonce const valid = consumeNonce(walletAddress, nonce); if (!valid) { + void auditFailure('invalid_or_expired_nonce'); res.status(401).json({ error: 'Invalid or expired nonce' }); return; } @@ -79,10 +93,12 @@ authRouter.post( keypair.verify(freighterMessageBytes, base64SignatureBytes); if (!isValidSignature) { + void auditFailure('signature_verification_failed'); res.status(401).json({ error: 'Signature verification failed' }); return; } } catch { + void auditFailure('malformed_signature_or_wallet'); res.status(401).json({ error: 'Invalid signature or wallet address' }); return; } @@ -116,6 +132,17 @@ authRouter.post( if (existingDevice) { if (existingDevice.revokedAt) { + // A revoked device still holding valid wallet credentials is the + // single most interesting failed sign-in there is. + void recordAuditEvent({ + action: 'auth_failed', + ...requestContext(req), + subjectUserId: userId, + actorDeviceId: existingDevice.id, + targetType: 'device', + targetId: existingDevice.id, + metadata: { reason: 'device_revoked' }, + }); res.status(401).json({ error: 'Device has been revoked' }); return; } diff --git a/apps/backend/src/routes/conversations.ts b/apps/backend/src/routes/conversations.ts index 21c91f74..62b5378e 100644 --- a/apps/backend/src/routes/conversations.ts +++ b/apps/backend/src/routes/conversations.ts @@ -17,6 +17,7 @@ import { invalidateConversationCaches } from '../lib/conversationCache.js'; import { serializeMessage, type MessageLike } from '../lib/messages.js'; import { getSocketServer } from '../lib/socket.js'; import { MAX_MESSAGES_LIMIT, DEFAULT_MESSAGES_LIMIT } from '../constants.js'; +import { actorFromRequest, recordAuditEvent } from '../services/auditLog.js'; export const conversationsRouter: IRouter = Router(); @@ -355,6 +356,18 @@ conversationsRouter.post('/:id/members', async (req: AuthRequest, res) => { conversationId, }); + // Group membership defines who can decrypt what from here on, so the + // change is a security event for both parties: the requester who made it + // and the account that was added (#376). + void recordAuditEvent({ + action: 'group_member_added', + ...actorFromRequest(req), + subjectUserId: newUserId, + targetType: 'conversation', + targetId: conversationId, + metadata: { memberCount: members.length }, + }); + res.status(201).json({ id: newMembership.id, conversationId: newMembership.conversationId, @@ -783,6 +796,20 @@ conversationsRouter.delete('/:id/leave', async (req: AuthRequest, res) => { await invalidateConversationCaches(members.map((member) => member.userId)); + void recordAuditEvent({ + action: 'group_member_removed', + ...actorFromRequest(req), + subjectUserId: userId, + targetType: 'conversation', + targetId: conversationId, + metadata: { + // Leaving as the last member deletes the conversation outright, which + // is a materially different outcome to a departure. + conversationDeleted: members.length === 1, + memberCountBefore: members.length, + }, + }); + res.status(204).send(); }); diff --git a/apps/backend/src/routes/devices.ts b/apps/backend/src/routes/devices.ts index 3cd1d39f..2d41286d 100644 --- a/apps/backend/src/routes/devices.ts +++ b/apps/backend/src/routes/devices.ts @@ -25,6 +25,7 @@ import { SignedPreKeyEntrySchema, PreKeyEntrySchema, verifyEd25519Signature } fr import { conversationRoom } from '../services/roomManager.js'; import { redis } from '../lib/redis.js'; import { markDeviceRevoked } from '../services/deviceRevocation.js'; +import { actorFromRequest, recordAuditEvent } from '../services/auditLog.js'; export const devicesRouter: RouterType = Router(); @@ -177,6 +178,21 @@ devicesRouter.delete('/:id', async (req: AuthRequest, res) => { const revokedAt = await revokeDeviceRow(deviceId); void emitDeviceChangeEvent(callerId, 'device_revoked'); + void recordAuditEvent({ + action: 'device_revoked', + ...actorFromRequest(req), + targetType: 'device', + targetId: deviceId, + metadata: { + deviceName: device.deviceName, + platform: device.platform, + // Revoking from the device being revoked reads very differently from + // revoking a device you no longer hold. + selfRevocation: deviceId === req.auth!.deviceId, + remainingActiveDevices: activeCount - 1, + }, + }); + res.json({ id: deviceId, revokedAt: revokedAt.toISOString() }); }); @@ -199,12 +215,29 @@ devicesRouter.post('/logout-everywhere', async (req: AuthRequest, res) => { for (const { id } of toRevoke) { await revokeDeviceRow(id); + void recordAuditEvent({ + action: 'device_revoked', + ...actorFromRequest(req), + targetType: 'device', + targetId: id, + metadata: { viaLogoutEverywhere: true }, + }); } if (toRevoke.length > 0) { void emitDeviceChangeEvent(userId, 'device_revoked'); } + // Recorded even when nothing was revoked: an account-wide security action + // was still invoked, and a responder wants to see the attempt. + void recordAuditEvent({ + action: 'logout_everywhere', + ...actorFromRequest(req), + targetType: 'user', + targetId: userId, + metadata: { revokedCount: toRevoke.length, retainedDeviceId: currentDeviceId }, + }); + res.json({ revokedCount: toRevoke.length }); }); @@ -429,6 +462,26 @@ devicesRouter.post( return; } + void recordAuditEvent({ + action: 'device_linked', + ...actorFromRequest(req), + targetType: 'device', + targetId: row.id, + metadata: { + deviceName: body.deviceName ?? null, + platform: body.platform ?? null, + // Re-activating a revoked identity key is not the same event as + // linking a brand-new device, and the difference matters after a + // revocation that was meant to lock someone out. + reactivatedRevokedDevice: Boolean(existing), + }, + }); + + res.status(201).json({ id: row.id, createdAt: row.createdAt }); + } catch (err) { + console.error('Failed to register device:', err); + res.status(500).json({ error: 'Failed to register device' }); + } if ( !verifyWalletSignature(walletAddress, deviceLinkMessage(userId, body.nonce), body.signature) ) { diff --git a/apps/backend/src/routes/files.ts b/apps/backend/src/routes/files.ts index 33d0ef55..208688ce 100644 --- a/apps/backend/src/routes/files.ts +++ b/apps/backend/src/routes/files.ts @@ -5,6 +5,7 @@ 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 { actorFromRequest, recordAuditEvent } from '../services/auditLog.js'; import { rateLimit } from '../middleware/rateLimit.js'; export const filesRouter: IRouter = Router(); @@ -50,6 +51,16 @@ filesRouter.get('/:fileId', rateLimit('file_download'), async (req: AuthRequest, }); if (!membership) { + // A non-member reaching for a file id is the clearest signal of an + // attempt to read someone else's attachments (#376). + void recordAuditEvent({ + action: 'file_access_denied', + ...actorFromRequest(req), + targetType: 'file', + targetId: fileId, + metadata: { conversationId: message.conversationId, reason: 'not_a_member' }, + }); + res.status(403).json({ error: 'Not authorized to access this file' }); return; } diff --git a/apps/backend/src/routes/users.ts b/apps/backend/src/routes/users.ts index 76815c33..dfed56a5 100644 --- a/apps/backend/src/routes/users.ts +++ b/apps/backend/src/routes/users.ts @@ -9,6 +9,7 @@ import { redis } from '../lib/redis.js'; import { isOnline, deriveDevicePresence } from '../services/presence.js'; import { getSocketServer } from '../lib/socket.js'; import { conversationRoom } from '../services/roomManager.js'; +import { actorFromRequest, recordAuditEvent } from '../services/auditLog.js'; import { prekeyConsumedTotal } from '../lib/metrics.js'; export const usersRouter: RouterType = Router(); @@ -297,8 +298,40 @@ usersRouter.get( return { keyId: candidate.keyId, publicKey: candidate.publicKey }; }); + // A one-time prekey was consumed and cannot be handed out again (#376). + // Draining a device's supply forces every later session with it down from + // 4-DH to 3-DH, and it happens quietly, so the count left is the signal an + // incident responder actually needs. Subject is the device owner — the + // account this was done *to* — while the actor is whoever fetched it. if (claimedOneTimePreKey) { - prekeyConsumedTotal.inc(); + // The remaining count is the useful part but only a nice-to-have: if the + // count query fails, still record that a prekey was consumed rather than + // losing the event, and never fail the bundle fetch over bookkeeping. + let remaining: number | null = null; + try { + const [remainingRow] = await db + .select({ remaining: sql`count(*)::int` }) + .from(devicePrekeys) + .where( + and( + eq(devicePrekeys.deviceId, deviceId), + eq(devicePrekeys.keyType, 'one_time'), + eq(devicePrekeys.consumed, false), + ), + ); + remaining = remainingRow?.remaining ?? 0; + } catch { + // Leave it null — the event itself is what must not be lost. + } + + void recordAuditEvent({ + action: 'key_bundle_drained', + ...actorFromRequest(req), + subjectUserId: targetUserId, + targetType: 'device', + targetId: deviceId, + metadata: { oneTimePreKeysRemaining: remaining, exhausted: remaining === 0 }, + }); } res.json({ diff --git a/apps/backend/src/services/auditLog.ts b/apps/backend/src/services/auditLog.ts new file mode 100644 index 00000000..0a6e10f6 --- /dev/null +++ b/apps/backend/src/services/auditLog.ts @@ -0,0 +1,257 @@ +/** + * Security audit log (#376). + * + * Records who did what, to whom, and when — for device linking and revocation, + * "log out everywhere", key-bundle drains, failed authentication, denied file + * access and group membership changes. The audience is an incident responder + * reconstructing a compromise after the fact. + * + * Two rules shape everything here: + * + * 1. **No message content, ever.** An audit trail that leaks plaintext would + * undo the end-to-end encryption it exists to protect. `sanitizeMetadata` + * drops content-shaped keys and bounds everything else, so a careless + * caller cannot widen the blast radius by spreading a request body into + * the metadata. + * 2. **Recording must never break the action.** A failed audit write is + * logged to stderr and swallowed. Failing a device revocation because the + * audit table is unavailable would make the security control less + * reliable than the thing it observes. + * + * Append-only is enforced by a database trigger (see the migration), not by + * convention: the log is only worth having if the application account an + * attacker reaches cannot rewrite it. + */ +import type { Request } from 'express'; +import { and, desc, eq, lt, or, type SQL } from 'drizzle-orm'; +import { db } from '../db/index.js'; +import { auditLogs, type AuditAction, type AuditLog } from '../db/schema.js'; +import type { AuthRequest } from '../middleware/auth.js'; + +/** + * Keys never written to the log, matched case-insensitively as substrings. + * A denylist rather than an allowlist because callers legitimately attach + * varied identifiers and counts; the caps below bound whatever survives. + */ +const FORBIDDEN_KEY_PATTERNS = [ + 'ciphertext', + 'plaintext', + 'content', + 'message', + 'body', + 'text', + 'envelope', + 'payload', + 'token', + 'secret', + 'password', + 'signature', + 'privatekey', + 'prekey', +]; + +/** Bounds on what a single row may carry. */ +const MAX_METADATA_KEYS = 20; +const MAX_STRING_LENGTH = 256; +const MAX_ARRAY_LENGTH = 20; +const MAX_USER_AGENT_LENGTH = 256; + +function isForbiddenKey(key: string): boolean { + const normalized = key.toLowerCase().replace(/[_-]/g, ''); + return FORBIDDEN_KEY_PATTERNS.some((pattern) => normalized.includes(pattern)); +} + +function sanitizeValue(value: unknown, depth: number): unknown { + if (value === null || typeof value === 'boolean' || typeof value === 'number') { + return value; + } + + if (typeof value === 'string') { + return value.length > MAX_STRING_LENGTH ? `${value.slice(0, MAX_STRING_LENGTH)}…` : value; + } + + if (Array.isArray(value)) { + if (depth <= 0) return `[array:${value.length}]`; + return value.slice(0, MAX_ARRAY_LENGTH).map((entry) => sanitizeValue(entry, depth - 1)); + } + + if (typeof value === 'object') { + // Nested objects are where a whole request body sneaks in, so they are + // summarised rather than recursed past one level. + if (depth <= 0) return '[object]'; + return sanitizeRecord(value as Record, depth - 1); + } + + // Functions, symbols, undefined: nothing an incident responder can use. + return undefined; +} + +function sanitizeRecord(input: Record, depth: number): Record { + const output: Record = {}; + + for (const [key, value] of Object.entries(input)) { + if (Object.keys(output).length >= MAX_METADATA_KEYS) break; + if (isForbiddenKey(key)) { + output[key] = '[redacted]'; + continue; + } + + const sanitized = sanitizeValue(value, depth); + if (sanitized !== undefined) { + output[key] = sanitized; + } + } + + return output; +} + +/** + * Strip content-shaped keys and bound size. Exported so the guarantee is + * directly testable rather than only observable through a database write. + */ +export function sanitizeMetadata( + metadata: Record | undefined, +): Record | null { + if (!metadata) return null; + const sanitized = sanitizeRecord(metadata, 1); + return Object.keys(sanitized).length > 0 ? sanitized : null; +} + +export interface AuditEvent { + action: AuditAction; + /** Who performed the action, when known. */ + actorUserId?: string | null; + actorDeviceId?: string | null; + /** Whose account the event concerns. Defaults to the actor. */ + subjectUserId?: string | null; + targetType?: string | null; + targetId?: string | null; + ipAddress?: string | null; + userAgent?: string | null; + metadata?: Record; +} + +/** Client address and user agent, for the "was this me?" question. */ +export function requestContext(req: Request): { + ipAddress: string | null; + userAgent: string | null; +} { + const userAgent = req.get('user-agent'); + return { + ipAddress: req.ip ?? null, + userAgent: userAgent ? userAgent.slice(0, MAX_USER_AGENT_LENGTH) : null, + }; +} + +/** Actor identity plus request context, for a route behind `requireAuth`. */ +export function actorFromRequest( + req: AuthRequest, +): Pick { + return { + actorUserId: req.auth?.userId ?? null, + actorDeviceId: req.auth?.deviceId ?? null, + ...requestContext(req), + }; +} + +/** + * Append one event. Never throws and never rejects — callers may `void` this + * without risking an unhandled rejection taking down the process. + */ +export async function recordAuditEvent(event: AuditEvent): Promise { + try { + await db.insert(auditLogs).values({ + action: event.action, + actorUserId: event.actorUserId ?? null, + actorDeviceId: event.actorDeviceId ?? null, + subjectUserId: event.subjectUserId ?? event.actorUserId ?? null, + targetType: event.targetType ?? null, + targetId: event.targetId ?? null, + ipAddress: event.ipAddress ?? null, + userAgent: event.userAgent ?? null, + metadata: sanitizeMetadata(event.metadata), + }); + } catch (err) { + // Deliberately swallowed — see the module comment. + console.error('[audit] failed to record event', event.action, err); + } +} + +export interface AuditQuery { + /** Account whose history is being read. */ + userId: string; + action?: AuditAction; + /** Opaque cursor from a previous page's `nextCursor`. */ + cursor?: string; + limit?: number; +} + +export const DEFAULT_AUDIT_PAGE_SIZE = 50; +export const MAX_AUDIT_PAGE_SIZE = 200; + +export function encodeAuditCursor(row: Pick): string { + return `${row.createdAt.getTime()}:${row.id}`; +} + +export function decodeAuditCursor(raw: string): { createdAt: Date; id: string } | null { + const separator = raw.indexOf(':'); + if (separator === -1) return null; + + const millis = Number(raw.slice(0, separator)); + const id = raw.slice(separator + 1); + if (!Number.isFinite(millis) || !id) return null; + + return { createdAt: new Date(millis), id }; +} + +/** + * Read one account's history, newest first. An event is "theirs" if they were + * the actor or the subject — a responder investigating an account needs the + * key-bundle fetch someone else performed against it just as much as the + * device that account revoked itself. + * + * Ordered by (createdAt, id) descending so the cursor is stable when several + * events share a millisecond. + */ +export async function queryAuditLog({ + userId, + action, + cursor, + limit = DEFAULT_AUDIT_PAGE_SIZE, +}: AuditQuery): Promise<{ events: AuditLog[]; nextCursor: string | null; hasMore: boolean }> { + const pageSize = Math.min(Math.max(1, limit), MAX_AUDIT_PAGE_SIZE); + const decoded = cursor ? decodeAuditCursor(cursor) : null; + + const conditions: Array = [ + or(eq(auditLogs.subjectUserId, userId), eq(auditLogs.actorUserId, userId)), + ]; + + if (action) { + conditions.push(eq(auditLogs.action, action)); + } + + if (decoded) { + conditions.push( + or( + lt(auditLogs.createdAt, decoded.createdAt), + and(eq(auditLogs.createdAt, decoded.createdAt), lt(auditLogs.id, decoded.id)), + ), + ); + } + + const rows = await db.query.auditLogs.findMany({ + where: and(...conditions), + orderBy: [desc(auditLogs.createdAt), desc(auditLogs.id)], + limit: pageSize + 1, + }); + + const hasMore = rows.length > pageSize; + const events = hasMore ? rows.slice(0, pageSize) : rows; + const last = events[events.length - 1]; + + return { + events, + nextCursor: hasMore && last ? encodeAuditCursor(last) : null, + hasMore, + }; +} diff --git a/docs/security/audit-logging.md b/docs/security/audit-logging.md new file mode 100644 index 00000000..eb18763d --- /dev/null +++ b/docs/security/audit-logging.md @@ -0,0 +1,103 @@ +# Security audit logging + +An append-only record of security-relevant events, written for an incident +responder reconstructing a compromise after the fact: which device was linked, +when the account was locked down, who fetched whose key bundle, where the +failed sign-ins came from. + +Implementation: `apps/backend/src/services/auditLog.ts`, `audit_logs` table, +migration `0001_audit_logs.sql`. + +## Recorded events + +| Action | Written when | Actor | Subject | +| ---------------------- | ---------------------------------------------------------------- | --------------- | ----------------- | +| `device_linked` | `POST /devices` registers or re-activates a device | linking user | same | +| `device_revoked` | `DELETE /devices/:id`, and once per device in log-out-everywhere | revoking user | same | +| `logout_everywhere` | `POST /devices/logout-everywhere`, including a zero-device run | requesting user | same | +| `key_bundle_drained` | a one-time prekey is consumed by a bundle fetch | fetching user | **device owner** | +| `auth_failed` | bad nonce, bad signature, or a revoked device presenting a token | none / device | wallet or account | +| `file_access_denied` | `GET /files/:fileId` by a non-member of the conversation | requesting user | same | +| `group_member_added` | `POST /conversations/:id/members` | requester | **added member** | +| `group_member_removed` | `DELETE /conversations/:id/leave` | leaving user | same | + +Every row carries the actor's user and device ids, the subject account, a +target type and id, the client IP, the user agent and a timestamp. + +### Actor versus subject + +`actorUserId` is who did it; `subjectUserId` is whose account it happened to. +They differ for exactly the events that matter most — someone else's device +draining your key bundle, a failed sign-in against your wallet, being added to +a group. The account-scoped query matches on **either**, so a user's history +includes what was done _to_ them, not only what they did. + +### What is not recorded + +Malformed or expired bearer tokens are not audited. Any internet scanner +produces them by the thousand, so recording them would hand an unauthenticated +caller a write amplification into the audit table. A token with a _valid +signature_ whose device has been revoked or deleted is a different matter — a +real credential being replayed after losing its authorisation — and that is +recorded. + +## No message content, ever + +An audit trail that leaks plaintext would undo the encryption it exists to +protect. `sanitizeMetadata` therefore runs on every write, not merely by +convention at the call sites: + +- Keys matching `ciphertext`, `plaintext`, `content`, `message`, `body`, + `text`, `envelope`, `payload`, `token`, `secret`, `password`, `signature`, + `privateKey` or `prekey` are replaced with `[redacted]`. Matching is + case-insensitive and ignores `_`/`-`, so `cipher_text` and `messageBody` are + caught too. +- Strings are truncated at 256 characters, arrays at 20 entries, objects at 20 + keys, and nesting stops after one level — nested objects are where a whole + request body would otherwise sneak in. +- Functions, symbols and `undefined` are dropped. + +Metadata is meant for identifiers, counts and outcomes: `remainingActiveDevices`, +`oneTimePreKeysRemaining`, `reason: 'device_revoked'`, `conversationDeleted`. + +## Append-only + +Enforced by a database trigger, not convention: + +```sql +CREATE TRIGGER audit_logs_no_mutation + BEFORE UPDATE OR DELETE OR TRUNCATE ON audit_logs + FOR EACH STATEMENT EXECUTE FUNCTION audit_logs_reject_mutation(); +``` + +The log is only worth having if the application account an attacker has already +reached cannot rewrite or erase it. For the same reason the actor and subject +columns carry **no foreign keys**: a cascade would delete the history along with +the account it incriminates, and `ON DELETE SET NULL` would issue an `UPDATE` +that the trigger correctly refuses. Ids are stored plain and resolved at read +time. + +Retention pruning is consequently a deliberate, privileged operation: drop the +trigger, prune, recreate it. That is the intended friction. + +## Recording never breaks the action + +A failed audit write is logged to stderr and swallowed. Failing a device +revocation because the audit table is unavailable would make the security +control less reliable than the thing it observes. `recordAuditEvent` never +throws and never rejects, so call sites can `void` it safely. + +## Querying + +``` +GET /audit-logs?action=&cursor=&limit= +``` + +Scoped to the authenticated caller — there is no parameter for whose log to +read, so a stolen token cannot enumerate anyone else's security events. Returns +newest first, cursor-paginated on `(createdAt, id)` so the cursor is stable when +several events share a millisecond. Page size defaults to 50 and is clamped to 200. An unknown `action` filter is a 400 rather than a silently ignored +parameter. + +Each event carries a `direction` of `performed` or `received`, so a client can +present "you did this" separately from "this was done to your account".