From 3dba00de7033b5600b6814f9f21f10272c4ab5ca Mon Sep 17 00:00:00 2001 From: Pau Date: Fri, 31 Jul 2026 11:26:40 +0200 Subject: [PATCH 01/16] feat(audit): filter the audit log by resource, and by resource + action Adds `resource` and `resource_action` materialized search columns (each with a `set` skip index) to `audit_trail_events`, exposes them as repeated `resources` / `actions` query params on GET /api/v1/audit-trail, and wires two selectors into the dashboard audit log. An action is only meaningful attached to a resource, so the Action selector stays disabled until exactly one resource is picked, and the endpoint rejects `actions` sent without `resources`. Co-Authored-By: Claude Opus 5 (1M context) --- packages/audit/lib/audit.ts | 12 ++- .../audit/lib/migrate.integration.test.ts | 19 ++++- ...0001_add_resource_action_search_columns.ts | 23 ++++++ packages/audit/lib/store.integration.test.ts | 74 ++++++++++++++++++- packages/audit/lib/store.ts | 16 +++- .../getAuditTrail.integration.test.ts | 44 +++++++++-- .../v1/audit-trail/getAuditTrail.ts | 23 +++++- packages/types/lib/audit-trail/api.ts | 8 +- .../components/patterns/FilterMultiSelect.tsx | 8 +- packages/webapp/src/hooks/useAudit.tsx | 19 ++++- packages/webapp/src/pages/Audit/Show.tsx | 35 ++++++++- packages/webapp/src/pages/Audit/constants.ts | 59 +++++++++++++++ 12 files changed, 310 insertions(+), 30 deletions(-) create mode 100644 packages/audit/lib/migrations/20260731000001_add_resource_action_search_columns.ts create mode 100644 packages/webapp/src/pages/Audit/constants.ts diff --git a/packages/audit/lib/audit.ts b/packages/audit/lib/audit.ts index e848558497..f3ab38fe2c 100644 --- a/packages/audit/lib/audit.ts +++ b/packages/audit/lib/audit.ts @@ -66,21 +66,25 @@ export class AuditClient { /** * Account-scoped, most-recent-first. `cursor` is the opaque `nextCursor` of a previous page; - * `from`/`to` optionally bound the window and combine with the cursor. Empty when audit isn't wired - * to a backend. + * `from`/`to` and the resource filters optionally narrow the set and combine with the cursor. Empty + * when audit isn't wired to a backend. */ async listAuditTrailEvents({ accountId, limit, cursor, from, - to + to, + resources, + actions }: { accountId: number; limit: number; cursor?: string | undefined; from?: string | undefined; to?: string | undefined; + resources?: string[] | undefined; + actions?: string[] | undefined; }): Promise> { let before: AuditTrailCursor | undefined; if (cursor) { @@ -91,7 +95,7 @@ export class AuditClient { before = decoded; } - return (await this.reader.list({ accountId, limit, before, from, to })).map((page) => ({ + return (await this.reader.list({ accountId, limit, before, from, to, resources, actions })).map((page) => ({ events: page.events, nextCursor: page.nextCursor ? encodeCursor(page.nextCursor) : null })); diff --git a/packages/audit/lib/migrate.integration.test.ts b/packages/audit/lib/migrate.integration.test.ts index e864bcfcc6..9f899514f0 100644 --- a/packages/audit/lib/migrate.integration.test.ts +++ b/packages/audit/lib/migrate.integration.test.ts @@ -41,7 +41,10 @@ describe('audit migrate', () => { // Exact match: audit runs its own migration set, so none of the usage tables may appear here. expect(await tables()).toEqual(['audit_trail_events', 'migrations']); - expect(await appliedMigrations()).toEqual([expect.stringMatching(/^20260729000001_create_audit_trail_events\.[jt]s$/)]); + expect(await appliedMigrations()).toEqual([ + expect.stringMatching(/^20260729000001_create_audit_trail_events\.[jt]s$/), + expect.stringMatching(/^20260731000001_add_resource_action_search_columns\.[jt]s$/) + ]); }); it('does not re-apply an already-applied migration', async () => { @@ -49,7 +52,19 @@ describe('audit migrate', () => { // Deliberately no FINAL: re-applying inserts a second row for the same name, which FINAL would hide. const res = await admin.query({ query: `SELECT count() AS count FROM ${database}.migrations`, format: 'JSONEachRow' }); - expect(Number((await res.json<{ count: string }>())[0]!.count)).toBe(1); + expect(Number((await res.json<{ count: string }>())[0]!.count)).toBe(2); + }); + + it('creates a set index for each of the resource search columns', async () => { + const res = await admin.query({ + query: `SELECT name, type_full AS type FROM system.data_skipping_indices WHERE database = {db:String} AND table = 'audit_trail_events' ORDER BY name`, + format: 'JSONEachRow', + query_params: { db: database } + }); + expect(await res.json<{ name: string; type: string }>()).toEqual([ + { name: 'idx_resource', type: 'set(0)' }, + { name: 'idx_resource_action', type: 'set(0)' } + ]); }); it('rejects an event whose accountId is missing, malformed or not a real account id', async () => { diff --git a/packages/audit/lib/migrations/20260731000001_add_resource_action_search_columns.ts b/packages/audit/lib/migrations/20260731000001_add_resource_action_search_columns.ts new file mode 100644 index 0000000000..24fba4fa75 --- /dev/null +++ b/packages/audit/lib/migrations/20260731000001_add_resource_action_search_columns.ts @@ -0,0 +1,23 @@ +// Non-throwing extracts, unlike the ORDER BY keys: a blob these can't read is still a storable event, +// so it must not be rejected at insert. +// +// `resource_action` is a separate column because two `set` indexes AND-ed prune per column, not per +// pair — a granule holding `connection.created` and `api_key.deleted` passes both `resource='connection'` +// and `action='deleted'` while containing no `connection.deleted`. There is no standalone `action` +// column: filtering by action alone isn't offered, so it would index nothing anyone queries. +// +// ALTER only fills new parts; older ones evaluate the expression on read (correct, just unpruned) and +// pick up the column when they next merge. No MATERIALIZE step: the table is empty at this migration, +// and on a large one it would be a blocking mutation at metering boot. +export const sql = [ + ` + ALTER TABLE {database:Identifier}.audit_trail_events + ADD COLUMN IF NOT EXISTS resource LowCardinality(String) MATERIALIZED JSONExtractString(event, 'resource'), + ADD COLUMN IF NOT EXISTS resource_action LowCardinality(String) MATERIALIZED concat(JSONExtractString(event, 'resource'), '.', JSONExtractString(event, 'action')) + `, + ` + ALTER TABLE {database:Identifier}.audit_trail_events + ADD INDEX IF NOT EXISTS idx_resource resource TYPE set(0) GRANULARITY 1, + ADD INDEX IF NOT EXISTS idx_resource_action resource_action TYPE set(0) GRANULARITY 1 + ` +]; diff --git a/packages/audit/lib/store.integration.test.ts b/packages/audit/lib/store.integration.test.ts index 7658418dc2..79772e1bb4 100644 --- a/packages/audit/lib/store.integration.test.ts +++ b/packages/audit/lib/store.integration.test.ts @@ -18,7 +18,19 @@ let client: ClickHouseClient; let store: ClickhouseAuditStore; // Known ids so the read assertions below are deterministic. -async function insertEvent({ id, accountId, occurredAt }: { id: string; accountId: number; occurredAt: string }) { +async function insertEvent({ + id, + accountId, + occurredAt, + resource = 'connection', + action = 'deleted' +}: { + id: string; + accountId: number; + occurredAt: string; + resource?: string; + action?: string; +}) { const event = { id, version: '2026-07-16', @@ -26,8 +38,8 @@ async function insertEvent({ id, accountId, occurredAt }: { id: string; accountI accountId, environment: null, actor: { type: 'user', id: '5', display: 'a@b.co' }, - resource: 'connection', - action: 'deleted', + resource, + action, targets: [{ type: 'connection', id: '10' }], context: {}, outcome: 'success' @@ -55,6 +67,12 @@ beforeAll(async () => { await insertEvent({ id: '22222222-2222-2222-2222-222222222222', accountId: 1, occurredAt: at(1000) }); await insertEvent({ id: '33333333-3333-3333-3333-333333333333', accountId: 1, occurredAt: at(2000) }); await insertEvent({ id: '99999999-9999-9999-9999-999999999999', accountId: 2, occurredAt: at(1500) }); + + // account 3: one event per resource/action pair the filter tests select on + await insertEvent({ id: 'aaaaaaaa-0000-0000-0000-000000000001', accountId: 3, occurredAt: at(0), resource: 'connection', action: 'deleted' }); + await insertEvent({ id: 'aaaaaaaa-0000-0000-0000-000000000002', accountId: 3, occurredAt: at(1000), resource: 'connection', action: 'updated' }); + await insertEvent({ id: 'aaaaaaaa-0000-0000-0000-000000000003', accountId: 3, occurredAt: at(2000), resource: 'api_key', action: 'deleted' }); + await insertEvent({ id: 'aaaaaaaa-0000-0000-0000-000000000004', accountId: 3, occurredAt: at(3000), resource: 'sync', action: 'enabled' }); }); afterAll(async () => { @@ -94,6 +112,56 @@ describe('ClickhouseAuditStore.list', () => { }); }); +describe('ClickhouseAuditStore.list resource filters', () => { + const resourceActionOf = (events: { resource: string; action: string }[]) => events.map((e) => `${e.resource}.${e.action}`).sort(); + + it('filters by resource', async () => { + const { events } = (await store.list({ accountId: 3, limit: 10, resources: ['connection'] })).unwrap(); + expect(resourceActionOf(events)).toEqual(['connection.deleted', 'connection.updated']); + expect(events.every((e) => e.accountId === 3)).toBe(true); + }); + + it('filters by several resources at once', async () => { + const { events } = (await store.list({ accountId: 3, limit: 10, resources: ['api_key', 'sync'] })).unwrap(); + expect(resourceActionOf(events)).toEqual(['api_key.deleted', 'sync.enabled']); + }); + + it('narrows a resource to a single action', async () => { + const { events } = (await store.list({ accountId: 3, limit: 10, resources: ['connection'], actions: ['deleted'] })).unwrap(); + expect(resourceActionOf(events)).toEqual(['connection.deleted']); + }); + + // The pairs are the cross product, so an action belonging to another resource must not widen the match. + it('matches actions against their own resource only', async () => { + const onSync = (await store.list({ accountId: 3, limit: 10, resources: ['sync'], actions: ['enabled'] })).unwrap(); + expect(resourceActionOf(onSync.events)).toEqual(['sync.enabled']); + + // Same action, a resource that never records it: the pair matches nothing rather than falling + // back to either half. + const onConnection = (await store.list({ accountId: 3, limit: 10, resources: ['connection'], actions: ['enabled'] })).unwrap(); + expect(onConnection.events).toHaveLength(0); + }); + + it('ignores actions given without a resource, since a pair needs both halves', async () => { + const { events } = (await store.list({ accountId: 3, limit: 10, actions: ['deleted'] })).unwrap(); + expect(resourceActionOf(events)).toEqual(['api_key.deleted', 'connection.deleted', 'connection.updated', 'sync.enabled']); + }); + + it('combines with the date window and paginates', async () => { + const page1 = (await store.list({ accountId: 3, limit: 1, resources: ['connection'], from: at(0), to: at(1000) })).unwrap(); + expect(resourceActionOf(page1.events)).toEqual(['connection.updated']); + expect(page1.nextCursor).not.toBeNull(); + + const page2 = (await store.list({ accountId: 3, limit: 1, resources: ['connection'], from: at(0), to: at(1000), before: page1.nextCursor! })).unwrap(); + expect(resourceActionOf(page2.events)).toEqual(['connection.deleted']); + }); + + it('returns nothing for a resource that was never recorded', async () => { + const { events } = (await store.list({ accountId: 3, limit: 10, resources: ['team'] })).unwrap(); + expect(events).toHaveLength(0); + }); +}); + describe('AuditClient.record through ClickhouseAuditStore', () => { it('writes an emitted event that reads back with the id + version stamped at emit', async () => { const event: AuditEvent = { diff --git a/packages/audit/lib/store.ts b/packages/audit/lib/store.ts index a4f28adc15..91b54253c3 100644 --- a/packages/audit/lib/store.ts +++ b/packages/audit/lib/store.ts @@ -24,6 +24,9 @@ export interface ListAuditTrailEventsParams { before?: AuditTrailCursor | undefined; from?: string | undefined; to?: string | undefined; + resources?: string[] | undefined; + /** Narrows `resources`; ignored on its own, since a match needs both halves of `resource.action`. */ + actions?: string[] | undefined; } export interface AuditTrailPage { @@ -94,9 +97,20 @@ export class ClickhouseAuditStore implements AuditWriter, AuditBatchWriter, Audi } } - async list({ accountId, limit, before, from, to }: ListAuditTrailEventsParams): Promise> { + async list({ accountId, limit, before, from, to, resources, actions }: ListAuditTrailEventsParams): Promise> { const params: Record = { account_id: accountId, limit: limit + 1 }; const conditions = ['account_id = {account_id:Int64}']; + if (resources?.length) { + if (actions?.length) { + // Every requested pair, matched against the materialized concatenation. Two separate + // conditions would prune per column and let a granule through on a pair it doesn't hold. + conditions.push('resource_action IN {resource_actions:Array(String)}'); + params['resource_actions'] = resources.flatMap((resource) => actions.map((action) => `${resource}.${action}`)); + } else { + conditions.push('resource IN {resources:Array(String)}'); + params['resources'] = resources; + } + } if (from) { conditions.push('occurred_at >= parseDateTime64BestEffortOrNull({from:String}, 3)'); params['from'] = from; diff --git a/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.integration.test.ts b/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.integration.test.ts index f806994443..b308b0a69a 100644 --- a/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.integration.test.ts +++ b/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.integration.test.ts @@ -5,7 +5,7 @@ import { seeders } from '@nangohq/shared'; import { authenticateUser, isSuccess, runServer } from '../../../utils/tests.js'; -import type { AuditEvent } from '@nangohq/audit'; +import type { AuditEvent, AuditResourceAction } from '@nangohq/audit'; let api: Awaited>; let auditClient: ReturnType; @@ -18,17 +18,16 @@ async function authAdmin() { return { session, account, env }; } -function auditEvent(accountId: number, occurredAt: string): AuditEvent { +function auditEvent(accountId: number, occurredAt: string, resourceAction: AuditResourceAction = { resource: 'connection', action: 'deleted' }): AuditEvent { return { occurredAt, accountId, environment: null, actor: { type: 'user', id: '5', display: 'a@b.co' }, - resource: 'connection', - action: 'deleted', targets: [{ type: 'connection', id: '10' }], context: {}, - outcome: 'success' + outcome: 'success', + ...resourceAction }; } @@ -111,6 +110,41 @@ describe('GET /api/v1/audit-trail', () => { expect(event.action).toBe('deleted'); }); + it('rejects `actions` sent without `resources` with 400', async () => { + const { session } = await authAdmin(); + const res = await api.fetch('/api/v1/audit-trail', { method: 'GET', session, query: { actions: ['deleted'] } }); + expect(res.res.status).toBe(400); + }); + + it('filters by resource, and by a resource narrowed to an action', async () => { + const { session, account } = await authAdmin(); + (await emitter.record(auditEvent(account.id, '2026-07-16T10:00:00.000Z', { resource: 'connection', action: 'deleted' }))).unwrap(); + (await emitter.record(auditEvent(account.id, '2026-07-16T10:00:01.000Z', { resource: 'connection', action: 'updated' }))).unwrap(); + (await emitter.record(auditEvent(account.id, '2026-07-16T10:00:02.000Z', { resource: 'api_key', action: 'deleted' }))).unwrap(); + + const byResource = await api.fetch('/api/v1/audit-trail', { method: 'GET', session, query: { resources: ['connection'] } }); + expect(byResource.res.status).toBe(200); + isSuccess(byResource.json); + expect(byResource.json.data.map((e) => `${e.resource}.${e.action}`).sort()).toEqual(['connection.deleted', 'connection.updated']); + + const byPair = await api.fetch('/api/v1/audit-trail', { method: 'GET', session, query: { resources: ['connection'], actions: ['deleted'] } }); + expect(byPair.res.status).toBe(200); + isSuccess(byPair.json); + expect(byPair.json.data.map((e) => `${e.resource}.${e.action}`)).toEqual(['connection.deleted']); + }); + + it('filters by several resources at once', async () => { + const { session, account } = await authAdmin(); + (await emitter.record(auditEvent(account.id, '2026-07-16T10:00:00.000Z', { resource: 'connection', action: 'deleted' }))).unwrap(); + (await emitter.record(auditEvent(account.id, '2026-07-16T10:00:01.000Z', { resource: 'api_key', action: 'deleted' }))).unwrap(); + (await emitter.record(auditEvent(account.id, '2026-07-16T10:00:02.000Z', { resource: 'team', action: 'updated' }))).unwrap(); + + const res = await api.fetch('/api/v1/audit-trail', { method: 'GET', session, query: { resources: ['connection', 'api_key'] } }); + expect(res.res.status).toBe(200); + isSuccess(res.json); + expect(res.json.data.map((e) => e.resource).sort()).toEqual(['api_key', 'connection']); + }); + it('paginates via the opaque cursor', async () => { const { session, account } = await authAdmin(); // 26 events one second apart (oldest → newest) — one more than the fixed page size of 25. diff --git a/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.ts b/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.ts index 87d80176e2..e62caf4741 100644 --- a/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.ts +++ b/packages/server/lib/controllers/v1/audit-trail/getAuditTrail.ts @@ -10,15 +10,30 @@ import type { GetAuditTrail } from '@nangohq/types'; const PAGE_SIZE = 25; +// Resources and actions are combined into a cross product downstream, so the two caps multiply. +const MAX_FILTER_VALUES = 50; + +// A repeated query param (`?resources=a&resources=b`), which Express hands over as a string when it +// appears once. Values aren't checked against the audit vocabulary — it has no runtime form to check +// against — so an unknown one simply matches nothing. +const repeatedParam = z + .union([z.string().min(1), z.array(z.string().min(1)).max(MAX_FILTER_VALUES)]) + .transform((value) => (Array.isArray(value) ? value : [value])); + const queryStringValidation = z .object({ cursor: z.string().optional(), from: z.iso.datetime().optional(), - to: z.iso.datetime().optional() + to: z.iso.datetime().optional(), + resources: repeatedParam.optional(), + actions: repeatedParam.optional() }) // Account-scoped endpoint (no `env`). Not strict: any stray query param is stripped rather than 400'd, so a read never fails over an extra key. // Surface an inverted range as a 400 rather than a silently empty result. - .refine((q) => !q.from || !q.to || new Date(q.from) <= new Date(q.to), { message: '`from` must be before or equal to `to`', path: ['from'] }); + .refine((q) => !q.from || !q.to || new Date(q.from) <= new Date(q.to), { message: '`from` must be before or equal to `to`', path: ['from'] }) + // An action is only meaningful attached to a resource, so reject the pairless form rather than + // dropping it and returning a wider result set than the caller asked for. + .refine((q) => !q.actions?.length || Boolean(q.resources?.length), { message: '`actions` requires `resources`', path: ['actions'] }); export const getAuditTrail = asyncWrapper(async (req, res) => { const query = queryStringValidation.safeParse(req.query); @@ -28,9 +43,9 @@ export const getAuditTrail = asyncWrapper(async (req, res) => { } const { account } = res.locals; - const { cursor, from, to } = query.data; + const { cursor, from, to, resources, actions } = query.data; - const result = await audit.listAuditTrailEvents({ accountId: account.id, limit: PAGE_SIZE, cursor, from, to }); + const result = await audit.listAuditTrailEvents({ accountId: account.id, limit: PAGE_SIZE, cursor, from, to, resources, actions }); if (result.isErr()) { if (result.error instanceof InvalidAuditCursorError) { res.status(400).send({ error: { code: 'invalid_query_params', message: 'Invalid cursor' } }); diff --git a/packages/types/lib/audit-trail/api.ts b/packages/types/lib/audit-trail/api.ts index bd3566b1bd..662fdc38f1 100644 --- a/packages/types/lib/audit-trail/api.ts +++ b/packages/types/lib/audit-trail/api.ts @@ -26,11 +26,15 @@ export type GetAuditTrail = ApiEndpoint<{ Method: 'GET'; Path: '/api/v1/audit-trail'; Querystring: { - // Account-scoped endpoint: no `env`. `cursor` encodes position only, not the filter window — resend the - // same `from`/`to` on every page or subsequent pages paginate the unfiltered set past the cursor. + // Account-scoped endpoint: no `env`. `cursor` encodes position only, not the filter window — resend + // every other param on each page or subsequent pages paginate the unfiltered set past the cursor. cursor?: string; from?: string; to?: string; + // Repeated params (`?resources=connection&resources=sync`). `actions` narrows `resources` and is + // rejected without it: the pair is matched as a single `resource.action` value, which needs both. + resources?: AuditResource[]; + actions?: AuditAction[]; }; Success: { data: ApiAuditTrailEvent[]; diff --git a/packages/webapp/src/components/patterns/FilterMultiSelect.tsx b/packages/webapp/src/components/patterns/FilterMultiSelect.tsx index e685ed3aa6..f6810593df 100644 --- a/packages/webapp/src/components/patterns/FilterMultiSelect.tsx +++ b/packages/webapp/src/components/patterns/FilterMultiSelect.tsx @@ -26,6 +26,7 @@ interface FilterMultiSelectProps { width?: string; open?: boolean; onOpenChange?: (open: boolean) => void; + disabled?: boolean; } export function FilterMultiSelect({ @@ -40,7 +41,8 @@ export function FilterMultiSelect({ max, width = 'w-56', open: controlledOpen, - onOpenChange: controlledOnOpenChange + onOpenChange: controlledOnOpenChange, + disabled = false }: FilterMultiSelectProps) { const [internalOpen, setInternalOpen] = useState(false); const [search, setSearch] = useState(''); @@ -48,7 +50,7 @@ export function FilterMultiSelect({ const searchInputRef = useRef(null); const isControlled = controlledOpen !== undefined; - const open = isControlled ? controlledOpen : internalOpen; + const open = (isControlled ? controlledOpen : internalOpen) && !disabled; const setOpen = useCallback( (val: boolean) => { @@ -179,7 +181,7 @@ export function FilterMultiSelect({ return ( -