Skip to content

Commit f6b626a

Browse files
waleedlatif1waleedlatif1
committed
fix(audit-log): lazily resolve actor name/email when missing
1 parent 3c470ab commit f6b626a

File tree

2 files changed

+191
-74
lines changed

2 files changed

+191
-74
lines changed

‎apps/sim/lib/audit/log.test.ts‎

Lines changed: 145 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,24 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { auditMock, databaseMock, loggerMock } from '@sim/testing'
4+
import { auditMock, databaseMock, drizzleOrmMock, loggerMock } from '@sim/testing'
55
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
66

77
vi.mock('@sim/db', () => ({
88
...databaseMock,
99
auditLog: { id: 'id', workspaceId: 'workspace_id' },
1010
}))
11+
vi.mock('@sim/db/schema', () => ({
12+
user: { id: 'id', name: 'name', email: 'email' },
13+
}))
14+
vi.mock('drizzle-orm', () => drizzleOrmMock)
1115
vi.mock('@sim/logger', () => loggerMock)
1216
vi.mock('nanoid', () => ({ nanoid: () => 'test-id-123' }))
1317

1418
import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
1519

20+
const flush = () => new Promise((resolve) => setTimeout(resolve, 10))
21+
1622
describe('AuditAction', () => {
1723
it('contains all expected action categories', () => {
1824
expect(AuditAction.WORKFLOW_CREATED).toBe('workflow.created')
@@ -45,12 +51,22 @@ describe('AuditResourceType', () => {
4551

4652
describe('recordAudit', () => {
4753
const mockInsert = databaseMock.db.insert
54+
const mockSelect = databaseMock.db.select
4855
let mockValues: ReturnType<typeof vi.fn>
56+
let mockLimit: ReturnType<typeof vi.fn>
4957

5058
beforeEach(() => {
5159
vi.clearAllMocks()
5260
mockValues = vi.fn(() => Promise.resolve())
5361
mockInsert.mockReturnValue({ values: mockValues })
62+
mockLimit = vi.fn(() => Promise.resolve([]))
63+
mockSelect.mockReturnValue({
64+
from: vi.fn(() => ({
65+
where: vi.fn(() => ({
66+
limit: mockLimit,
67+
})),
68+
})),
69+
})
5470
})
5571

5672
afterEach(() => {
@@ -61,15 +77,16 @@ describe('recordAudit', () => {
6177
recordAudit({
6278
workspaceId: 'ws-1',
6379
actorId: 'user-1',
80+
actorName: 'Test User',
81+
actorEmail: 'test@example.com',
6482
action: AuditAction.WORKFLOW_CREATED,
6583
resourceType: AuditResourceType.WORKFLOW,
6684
resourceId: 'wf-1',
6785
})
6886

69-
await vi.waitFor(() => {
70-
expect(mockInsert).toHaveBeenCalledTimes(1)
71-
})
87+
await flush()
7288

89+
expect(mockInsert).toHaveBeenCalledTimes(1)
7390
expect(mockValues).toHaveBeenCalledWith(
7491
expect.objectContaining({
7592
id: 'test-id-123',
@@ -96,9 +113,7 @@ describe('recordAudit', () => {
96113
description: 'Created folder "My Folder"',
97114
})
98115

99-
await vi.waitFor(() => {
100-
expect(mockValues).toHaveBeenCalledTimes(1)
101-
})
116+
await flush()
102117

103118
expect(mockValues).toHaveBeenCalledWith(
104119
expect.objectContaining({
@@ -110,26 +125,6 @@ describe('recordAudit', () => {
110125
)
111126
})
112127

113-
it('sets optional fields to undefined when not provided', async () => {
114-
recordAudit({
115-
workspaceId: 'ws-1',
116-
actorId: 'user-1',
117-
action: AuditAction.WORKSPACE_DELETED,
118-
resourceType: AuditResourceType.WORKSPACE,
119-
})
120-
121-
await vi.waitFor(() => {
122-
expect(mockValues).toHaveBeenCalledTimes(1)
123-
})
124-
125-
const insertedValues = mockValues.mock.calls[0][0]
126-
expect(insertedValues.resourceId).toBeUndefined()
127-
expect(insertedValues.actorName).toBeUndefined()
128-
expect(insertedValues.actorEmail).toBeUndefined()
129-
expect(insertedValues.resourceName).toBeUndefined()
130-
expect(insertedValues.description).toBeUndefined()
131-
})
132-
133128
it('extracts IP address from x-forwarded-for header', async () => {
134129
const request = new Request('https://example.com', {
135130
headers: {
@@ -141,14 +136,14 @@ describe('recordAudit', () => {
141136
recordAudit({
142137
workspaceId: 'ws-1',
143138
actorId: 'user-1',
139+
actorName: 'Test',
140+
actorEmail: 'test@test.com',
144141
action: AuditAction.MEMBER_INVITED,
145142
resourceType: AuditResourceType.WORKSPACE,
146143
request,
147144
})
148145

149-
await vi.waitFor(() => {
150-
expect(mockValues).toHaveBeenCalledTimes(1)
151-
})
146+
await flush()
152147

153148
expect(mockValues).toHaveBeenCalledWith(
154149
expect.objectContaining({
@@ -166,14 +161,14 @@ describe('recordAudit', () => {
166161
recordAudit({
167162
workspaceId: 'ws-1',
168163
actorId: 'user-1',
164+
actorName: 'Test',
165+
actorEmail: 'test@test.com',
169166
action: AuditAction.API_KEY_CREATED,
170167
resourceType: AuditResourceType.API_KEY,
171168
request,
172169
})
173170

174-
await vi.waitFor(() => {
175-
expect(mockValues).toHaveBeenCalledTimes(1)
176-
})
171+
await flush()
177172

178173
expect(mockValues).toHaveBeenCalledWith(
179174
expect.objectContaining({
@@ -187,13 +182,13 @@ describe('recordAudit', () => {
187182
recordAudit({
188183
workspaceId: 'ws-1',
189184
actorId: 'user-1',
185+
actorName: 'Test',
186+
actorEmail: 'test@test.com',
190187
action: AuditAction.ENVIRONMENT_UPDATED,
191188
resourceType: AuditResourceType.ENVIRONMENT,
192189
})
193190

194-
await vi.waitFor(() => {
195-
expect(mockValues).toHaveBeenCalledTimes(1)
196-
})
191+
await flush()
197192

198193
expect(mockValues).toHaveBeenCalledWith(expect.objectContaining({ metadata: {} }))
199194
})
@@ -202,14 +197,14 @@ describe('recordAudit', () => {
202197
recordAudit({
203198
workspaceId: 'ws-1',
204199
actorId: 'user-1',
200+
actorName: 'Test',
201+
actorEmail: 'test@test.com',
205202
action: AuditAction.WEBHOOK_CREATED,
206203
resourceType: AuditResourceType.WEBHOOK,
207204
metadata: { provider: 'github', workflowId: 'wf-1' },
208205
})
209206

210-
await vi.waitFor(() => {
211-
expect(mockValues).toHaveBeenCalledTimes(1)
212-
})
207+
await flush()
213208

214209
expect(mockValues).toHaveBeenCalledWith(
215210
expect.objectContaining({
@@ -219,28 +214,138 @@ describe('recordAudit', () => {
219214
})
220215

221216
it('does not throw when the database insert fails', async () => {
222-
mockValues.mockReturnValue(Promise.reject(new Error('DB connection lost')))
217+
mockValues.mockImplementation(() => Promise.reject(new Error('DB connection lost')))
223218

224219
expect(() => {
225220
recordAudit({
226221
workspaceId: 'ws-1',
227222
actorId: 'user-1',
223+
actorName: 'Test',
224+
actorEmail: 'test@test.com',
228225
action: AuditAction.WORKFLOW_DELETED,
229226
resourceType: AuditResourceType.WORKFLOW,
230227
})
231228
}).not.toThrow()
229+
230+
await flush()
232231
})
233232

234233
it('does not block — returns void synchronously', () => {
235234
const result = recordAudit({
236235
workspaceId: 'ws-1',
237236
actorId: 'user-1',
237+
actorName: 'Test',
238+
actorEmail: 'test@test.com',
238239
action: AuditAction.CHAT_DEPLOYED,
239240
resourceType: AuditResourceType.CHAT,
240241
})
241242

242243
expect(result).toBeUndefined()
243244
})
245+
246+
describe('lazy actor resolution', () => {
247+
it('looks up user when actorName and actorEmail are both undefined', async () => {
248+
mockLimit.mockResolvedValue([{ name: 'Resolved Name', email: 'resolved@example.com' }])
249+
250+
recordAudit({
251+
workspaceId: 'ws-1',
252+
actorId: 'user-1',
253+
action: AuditAction.DOCUMENT_UPLOADED,
254+
resourceType: AuditResourceType.DOCUMENT,
255+
resourceId: 'doc-1',
256+
})
257+
258+
await flush()
259+
260+
expect(mockSelect).toHaveBeenCalledTimes(1)
261+
expect(mockValues).toHaveBeenCalledWith(
262+
expect.objectContaining({
263+
actorName: 'Resolved Name',
264+
actorEmail: 'resolved@example.com',
265+
})
266+
)
267+
})
268+
269+
it('skips lookup when actorName is provided (even if null)', async () => {
270+
recordAudit({
271+
workspaceId: 'ws-1',
272+
actorId: 'user-1',
273+
actorName: null,
274+
actorEmail: null,
275+
action: AuditAction.DOCUMENT_UPLOADED,
276+
resourceType: AuditResourceType.DOCUMENT,
277+
})
278+
279+
await flush()
280+
281+
expect(mockSelect).not.toHaveBeenCalled()
282+
})
283+
284+
it('skips lookup when actorName and actorEmail are provided', async () => {
285+
recordAudit({
286+
workspaceId: 'ws-1',
287+
actorId: 'user-1',
288+
actorName: 'Already Known',
289+
actorEmail: 'known@example.com',
290+
action: AuditAction.WORKFLOW_CREATED,
291+
resourceType: AuditResourceType.WORKFLOW,
292+
})
293+
294+
await flush()
295+
296+
expect(mockSelect).not.toHaveBeenCalled()
297+
expect(mockValues).toHaveBeenCalledWith(
298+
expect.objectContaining({
299+
actorName: 'Already Known',
300+
actorEmail: 'known@example.com',
301+
})
302+
)
303+
})
304+
305+
it('inserts without actor info when lookup fails', async () => {
306+
mockLimit.mockRejectedValue(new Error('DB down'))
307+
308+
recordAudit({
309+
workspaceId: 'ws-1',
310+
actorId: 'user-1',
311+
action: AuditAction.KNOWLEDGE_BASE_CREATED,
312+
resourceType: AuditResourceType.KNOWLEDGE_BASE,
313+
})
314+
315+
await flush()
316+
317+
expect(mockSelect).toHaveBeenCalledTimes(1)
318+
expect(mockValues).toHaveBeenCalledWith(
319+
expect.objectContaining({
320+
actorId: 'user-1',
321+
actorName: undefined,
322+
actorEmail: undefined,
323+
})
324+
)
325+
})
326+
327+
it('sets actor info to null when user is not found', async () => {
328+
mockLimit.mockResolvedValue([])
329+
330+
recordAudit({
331+
workspaceId: 'ws-1',
332+
actorId: 'deleted-user',
333+
action: AuditAction.WORKFLOW_DELETED,
334+
resourceType: AuditResourceType.WORKFLOW,
335+
})
336+
337+
await flush()
338+
339+
expect(mockSelect).toHaveBeenCalledTimes(1)
340+
expect(mockValues).toHaveBeenCalledWith(
341+
expect.objectContaining({
342+
actorId: 'deleted-user',
343+
actorName: undefined,
344+
actorEmail: undefined,
345+
})
346+
)
347+
})
348+
})
244349
})
245350

246351
describe('auditMock sync', () => {

0 commit comments

Comments
 (0)