mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(audit-log): lazily resolve actor name/email when missing (#3262)
This commit is contained in:
+145
-40
@@ -1,18 +1,24 @@
|
||||
/**
|
||||
* @vitest-environment node
|
||||
*/
|
||||
import { auditMock, databaseMock, loggerMock } from '@sim/testing'
|
||||
import { auditMock, databaseMock, drizzleOrmMock, loggerMock } from '@sim/testing'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
vi.mock('@sim/db', () => ({
|
||||
...databaseMock,
|
||||
auditLog: { id: 'id', workspaceId: 'workspace_id' },
|
||||
}))
|
||||
vi.mock('@sim/db/schema', () => ({
|
||||
user: { id: 'id', name: 'name', email: 'email' },
|
||||
}))
|
||||
vi.mock('drizzle-orm', () => drizzleOrmMock)
|
||||
vi.mock('@sim/logger', () => loggerMock)
|
||||
vi.mock('nanoid', () => ({ nanoid: () => 'test-id-123' }))
|
||||
|
||||
import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
|
||||
|
||||
const flush = () => new Promise((resolve) => setTimeout(resolve, 10))
|
||||
|
||||
describe('AuditAction', () => {
|
||||
it('contains all expected action categories', () => {
|
||||
expect(AuditAction.WORKFLOW_CREATED).toBe('workflow.created')
|
||||
@@ -45,12 +51,22 @@ describe('AuditResourceType', () => {
|
||||
|
||||
describe('recordAudit', () => {
|
||||
const mockInsert = databaseMock.db.insert
|
||||
const mockSelect = databaseMock.db.select
|
||||
let mockValues: ReturnType<typeof vi.fn>
|
||||
let mockLimit: ReturnType<typeof vi.fn>
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockValues = vi.fn(() => Promise.resolve())
|
||||
mockInsert.mockReturnValue({ values: mockValues })
|
||||
mockLimit = vi.fn(() => Promise.resolve([]))
|
||||
mockSelect.mockReturnValue({
|
||||
from: vi.fn(() => ({
|
||||
where: vi.fn(() => ({
|
||||
limit: mockLimit,
|
||||
})),
|
||||
})),
|
||||
})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
@@ -61,15 +77,16 @@ describe('recordAudit', () => {
|
||||
recordAudit({
|
||||
workspaceId: 'ws-1',
|
||||
actorId: 'user-1',
|
||||
actorName: 'Test User',
|
||||
actorEmail: 'test@example.com',
|
||||
action: AuditAction.WORKFLOW_CREATED,
|
||||
resourceType: AuditResourceType.WORKFLOW,
|
||||
resourceId: 'wf-1',
|
||||
})
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockInsert).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
await flush()
|
||||
|
||||
expect(mockInsert).toHaveBeenCalledTimes(1)
|
||||
expect(mockValues).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
id: 'test-id-123',
|
||||
@@ -96,9 +113,7 @@ describe('recordAudit', () => {
|
||||
description: 'Created folder "My Folder"',
|
||||
})
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockValues).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
await flush()
|
||||
|
||||
expect(mockValues).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
@@ -110,26 +125,6 @@ describe('recordAudit', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it('sets optional fields to undefined when not provided', async () => {
|
||||
recordAudit({
|
||||
workspaceId: 'ws-1',
|
||||
actorId: 'user-1',
|
||||
action: AuditAction.WORKSPACE_DELETED,
|
||||
resourceType: AuditResourceType.WORKSPACE,
|
||||
})
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockValues).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
const insertedValues = mockValues.mock.calls[0][0]
|
||||
expect(insertedValues.resourceId).toBeUndefined()
|
||||
expect(insertedValues.actorName).toBeUndefined()
|
||||
expect(insertedValues.actorEmail).toBeUndefined()
|
||||
expect(insertedValues.resourceName).toBeUndefined()
|
||||
expect(insertedValues.description).toBeUndefined()
|
||||
})
|
||||
|
||||
it('extracts IP address from x-forwarded-for header', async () => {
|
||||
const request = new Request('https://example.com', {
|
||||
headers: {
|
||||
@@ -141,14 +136,14 @@ describe('recordAudit', () => {
|
||||
recordAudit({
|
||||
workspaceId: 'ws-1',
|
||||
actorId: 'user-1',
|
||||
actorName: 'Test',
|
||||
actorEmail: 'test@test.com',
|
||||
action: AuditAction.MEMBER_INVITED,
|
||||
resourceType: AuditResourceType.WORKSPACE,
|
||||
request,
|
||||
})
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockValues).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
await flush()
|
||||
|
||||
expect(mockValues).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
@@ -166,14 +161,14 @@ describe('recordAudit', () => {
|
||||
recordAudit({
|
||||
workspaceId: 'ws-1',
|
||||
actorId: 'user-1',
|
||||
actorName: 'Test',
|
||||
actorEmail: 'test@test.com',
|
||||
action: AuditAction.API_KEY_CREATED,
|
||||
resourceType: AuditResourceType.API_KEY,
|
||||
request,
|
||||
})
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockValues).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
await flush()
|
||||
|
||||
expect(mockValues).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
@@ -187,13 +182,13 @@ describe('recordAudit', () => {
|
||||
recordAudit({
|
||||
workspaceId: 'ws-1',
|
||||
actorId: 'user-1',
|
||||
actorName: 'Test',
|
||||
actorEmail: 'test@test.com',
|
||||
action: AuditAction.ENVIRONMENT_UPDATED,
|
||||
resourceType: AuditResourceType.ENVIRONMENT,
|
||||
})
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockValues).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
await flush()
|
||||
|
||||
expect(mockValues).toHaveBeenCalledWith(expect.objectContaining({ metadata: {} }))
|
||||
})
|
||||
@@ -202,14 +197,14 @@ describe('recordAudit', () => {
|
||||
recordAudit({
|
||||
workspaceId: 'ws-1',
|
||||
actorId: 'user-1',
|
||||
actorName: 'Test',
|
||||
actorEmail: 'test@test.com',
|
||||
action: AuditAction.WEBHOOK_CREATED,
|
||||
resourceType: AuditResourceType.WEBHOOK,
|
||||
metadata: { provider: 'github', workflowId: 'wf-1' },
|
||||
})
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockValues).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
await flush()
|
||||
|
||||
expect(mockValues).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
@@ -219,28 +214,138 @@ describe('recordAudit', () => {
|
||||
})
|
||||
|
||||
it('does not throw when the database insert fails', async () => {
|
||||
mockValues.mockReturnValue(Promise.reject(new Error('DB connection lost')))
|
||||
mockValues.mockImplementation(() => Promise.reject(new Error('DB connection lost')))
|
||||
|
||||
expect(() => {
|
||||
recordAudit({
|
||||
workspaceId: 'ws-1',
|
||||
actorId: 'user-1',
|
||||
actorName: 'Test',
|
||||
actorEmail: 'test@test.com',
|
||||
action: AuditAction.WORKFLOW_DELETED,
|
||||
resourceType: AuditResourceType.WORKFLOW,
|
||||
})
|
||||
}).not.toThrow()
|
||||
|
||||
await flush()
|
||||
})
|
||||
|
||||
it('does not block — returns void synchronously', () => {
|
||||
const result = recordAudit({
|
||||
workspaceId: 'ws-1',
|
||||
actorId: 'user-1',
|
||||
actorName: 'Test',
|
||||
actorEmail: 'test@test.com',
|
||||
action: AuditAction.CHAT_DEPLOYED,
|
||||
resourceType: AuditResourceType.CHAT,
|
||||
})
|
||||
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
|
||||
describe('lazy actor resolution', () => {
|
||||
it('looks up user when actorName and actorEmail are both undefined', async () => {
|
||||
mockLimit.mockResolvedValue([{ name: 'Resolved Name', email: 'resolved@example.com' }])
|
||||
|
||||
recordAudit({
|
||||
workspaceId: 'ws-1',
|
||||
actorId: 'user-1',
|
||||
action: AuditAction.DOCUMENT_UPLOADED,
|
||||
resourceType: AuditResourceType.DOCUMENT,
|
||||
resourceId: 'doc-1',
|
||||
})
|
||||
|
||||
await flush()
|
||||
|
||||
expect(mockSelect).toHaveBeenCalledTimes(1)
|
||||
expect(mockValues).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
actorName: 'Resolved Name',
|
||||
actorEmail: 'resolved@example.com',
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('skips lookup when actorName is provided (even if null)', async () => {
|
||||
recordAudit({
|
||||
workspaceId: 'ws-1',
|
||||
actorId: 'user-1',
|
||||
actorName: null,
|
||||
actorEmail: null,
|
||||
action: AuditAction.DOCUMENT_UPLOADED,
|
||||
resourceType: AuditResourceType.DOCUMENT,
|
||||
})
|
||||
|
||||
await flush()
|
||||
|
||||
expect(mockSelect).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('skips lookup when actorName and actorEmail are provided', async () => {
|
||||
recordAudit({
|
||||
workspaceId: 'ws-1',
|
||||
actorId: 'user-1',
|
||||
actorName: 'Already Known',
|
||||
actorEmail: 'known@example.com',
|
||||
action: AuditAction.WORKFLOW_CREATED,
|
||||
resourceType: AuditResourceType.WORKFLOW,
|
||||
})
|
||||
|
||||
await flush()
|
||||
|
||||
expect(mockSelect).not.toHaveBeenCalled()
|
||||
expect(mockValues).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
actorName: 'Already Known',
|
||||
actorEmail: 'known@example.com',
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('inserts without actor info when lookup fails', async () => {
|
||||
mockLimit.mockRejectedValue(new Error('DB down'))
|
||||
|
||||
recordAudit({
|
||||
workspaceId: 'ws-1',
|
||||
actorId: 'user-1',
|
||||
action: AuditAction.KNOWLEDGE_BASE_CREATED,
|
||||
resourceType: AuditResourceType.KNOWLEDGE_BASE,
|
||||
})
|
||||
|
||||
await flush()
|
||||
|
||||
expect(mockSelect).toHaveBeenCalledTimes(1)
|
||||
expect(mockValues).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
actorId: 'user-1',
|
||||
actorName: undefined,
|
||||
actorEmail: undefined,
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('sets actor info to null when user is not found', async () => {
|
||||
mockLimit.mockResolvedValue([])
|
||||
|
||||
recordAudit({
|
||||
workspaceId: 'ws-1',
|
||||
actorId: 'deleted-user',
|
||||
action: AuditAction.WORKFLOW_DELETED,
|
||||
resourceType: AuditResourceType.WORKFLOW,
|
||||
})
|
||||
|
||||
await flush()
|
||||
|
||||
expect(mockSelect).toHaveBeenCalledTimes(1)
|
||||
expect(mockValues).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
actorId: 'deleted-user',
|
||||
actorName: undefined,
|
||||
actorEmail: undefined,
|
||||
})
|
||||
)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('auditMock sync', () => {
|
||||
|
||||
+45
-33
@@ -1,5 +1,7 @@
|
||||
import { auditLog, db } from '@sim/db'
|
||||
import { user } from '@sim/db/schema'
|
||||
import { createLogger } from '@sim/logger'
|
||||
import { eq } from 'drizzle-orm'
|
||||
import { nanoid } from 'nanoid'
|
||||
|
||||
const logger = createLogger('AuditLog')
|
||||
@@ -185,41 +187,51 @@ interface AuditLogParams {
|
||||
|
||||
/**
|
||||
* Records an audit log entry. Fire-and-forget — never throws or blocks the caller.
|
||||
* If actorName and actorEmail are both undefined (not provided by the caller),
|
||||
* resolves them from the user table before inserting.
|
||||
*/
|
||||
export function recordAudit(params: AuditLogParams): void {
|
||||
try {
|
||||
const ipAddress =
|
||||
params.request?.headers.get('x-forwarded-for')?.split(',')[0].trim() ??
|
||||
params.request?.headers.get('x-real-ip') ??
|
||||
undefined
|
||||
const userAgent = params.request?.headers.get('user-agent') ?? undefined
|
||||
insertAuditLog(params).catch((error) => {
|
||||
logger.error('Failed to record audit log', { error, action: params.action })
|
||||
})
|
||||
}
|
||||
|
||||
db.insert(auditLog)
|
||||
.values({
|
||||
id: nanoid(),
|
||||
workspaceId: params.workspaceId || null,
|
||||
actorId: params.actorId,
|
||||
action: params.action,
|
||||
resourceType: params.resourceType,
|
||||
resourceId: params.resourceId,
|
||||
actorName: params.actorName ?? undefined,
|
||||
actorEmail: params.actorEmail ?? undefined,
|
||||
resourceName: params.resourceName,
|
||||
description: params.description,
|
||||
metadata: params.metadata ?? {},
|
||||
ipAddress,
|
||||
userAgent,
|
||||
})
|
||||
.then(() => {
|
||||
logger.debug('Audit log recorded', {
|
||||
action: params.action,
|
||||
resourceType: params.resourceType,
|
||||
})
|
||||
})
|
||||
.catch((error) => {
|
||||
logger.error('Failed to record audit log', { error, action: params.action })
|
||||
})
|
||||
} catch (error) {
|
||||
logger.error('Failed to initiate audit log', { error, action: params.action })
|
||||
async function insertAuditLog(params: AuditLogParams): Promise<void> {
|
||||
const ipAddress =
|
||||
params.request?.headers.get('x-forwarded-for')?.split(',')[0].trim() ??
|
||||
params.request?.headers.get('x-real-ip') ??
|
||||
undefined
|
||||
const userAgent = params.request?.headers.get('user-agent') ?? undefined
|
||||
|
||||
let { actorName, actorEmail } = params
|
||||
|
||||
if (actorName === undefined && actorEmail === undefined && params.actorId) {
|
||||
try {
|
||||
const [row] = await db
|
||||
.select({ name: user.name, email: user.email })
|
||||
.from(user)
|
||||
.where(eq(user.id, params.actorId))
|
||||
.limit(1)
|
||||
actorName = row?.name ?? undefined
|
||||
actorEmail = row?.email ?? undefined
|
||||
} catch (error) {
|
||||
logger.debug('Failed to resolve actor info', { error, actorId: params.actorId })
|
||||
}
|
||||
}
|
||||
|
||||
await db.insert(auditLog).values({
|
||||
id: nanoid(),
|
||||
workspaceId: params.workspaceId || null,
|
||||
actorId: params.actorId,
|
||||
action: params.action,
|
||||
resourceType: params.resourceType,
|
||||
resourceId: params.resourceId,
|
||||
actorName: actorName ?? undefined,
|
||||
actorEmail: actorEmail ?? undefined,
|
||||
resourceName: params.resourceName,
|
||||
description: params.description,
|
||||
metadata: params.metadata ?? {},
|
||||
ipAddress,
|
||||
userAgent,
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user