diff --git a/apps/sim/app/api/v1/admin/organizations/[id]/route.ts b/apps/sim/app/api/v1/admin/organizations/[id]/route.ts index d8e1c9c450c..20f6e948f94 100644 --- a/apps/sim/app/api/v1/admin/organizations/[id]/route.ts +++ b/apps/sim/app/api/v1/admin/organizations/[id]/route.ts @@ -30,7 +30,13 @@ * Response: AdminSingleResponse<{ success, organizationId, slug, membersRemoved, workspacesDetached }> */ -import { AuditAction, AuditResourceType, recordAudit, recordAuditBatch } from '@sim/audit' +import { + AuditAction, + AuditResourceType, + auditUpdatedFields, + recordAudit, + recordAuditBatch, +} from '@sim/audit' import { db } from '@sim/db' import { member, organization, subscription } from '@sim/db/schema' import { createLogger } from '@sim/logger' @@ -178,9 +184,8 @@ export const PATCH = withRouteHandler( .where(eq(organization.id, organizationId)) .returning() - logger.info(`Admin API: Updated organization ${organizationId}`, { - fields: Object.keys(updateData).filter((k) => k !== 'updatedAt'), - }) + const updatedFields = auditUpdatedFields(updateData) + logger.info(`Admin API: Updated organization ${organizationId}`, { updatedFields }) recordAudit({ workspaceId: null, @@ -190,7 +195,7 @@ export const PATCH = withRouteHandler( resourceId: organizationId, resourceName: updated.name, description: `Admin API updated organization "${updated.name}"`, - metadata: { fields: Object.keys(updateData).filter((k) => k !== 'updatedAt') }, + metadata: { updatedFields }, request, }) diff --git a/apps/sim/lib/credentials/orchestration/index.test.ts b/apps/sim/lib/credentials/orchestration/index.test.ts index 800e0a5e741..8f5a9dcc6f5 100644 --- a/apps/sim/lib/credentials/orchestration/index.test.ts +++ b/apps/sim/lib/credentials/orchestration/index.test.ts @@ -1,7 +1,13 @@ /** * @vitest-environment node */ -import { dbChainMockFns, queueTableRows, resetDbChainMock, schemaMock } from '@sim/testing' +import { + auditMock, + dbChainMockFns, + queueTableRows, + resetDbChainMock, + schemaMock, +} from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' const { @@ -26,6 +32,7 @@ vi.mock('@sim/audit', () => ({ AuditAction: { CREDENTIAL_UPDATED: 'credential.updated' }, AuditResourceType: { CREDENTIAL: 'credential' }, recordAudit: mockRecordAudit, + auditUpdatedFields: auditMock.auditUpdatedFields, })) vi.mock('@/lib/credentials/access', () => ({ getCredentialActorContext: mockGetCredentialActorContext, diff --git a/apps/sim/lib/credentials/orchestration/index.ts b/apps/sim/lib/credentials/orchestration/index.ts index f1937d7fa80..ff0a3bd283f 100644 --- a/apps/sim/lib/credentials/orchestration/index.ts +++ b/apps/sim/lib/credentials/orchestration/index.ts @@ -1,4 +1,4 @@ -import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' +import { AuditAction, AuditResourceType, auditUpdatedFields, recordAudit } from '@sim/audit' import { db } from '@sim/db' import { credential, environment, webhook, workspaceEnvironment } from '@sim/db/schema' import { createLogger } from '@sim/logger' @@ -336,7 +336,7 @@ export async function performUpdateCredential( .where(and(eq(webhook.provider, 'slack'), eq(webhook.routingKey, params.credentialId))) } - const updatedFields = Object.keys(updates).filter((key) => key !== 'updatedAt') + const updatedFields = auditUpdatedFields(updates) recordAudit({ workspaceId: access.credential.workspaceId, actorId: params.userId, diff --git a/apps/sim/lib/mcp/orchestration/server-lifecycle.ts b/apps/sim/lib/mcp/orchestration/server-lifecycle.ts index 2899aa659b8..7a9656a4a91 100644 --- a/apps/sim/lib/mcp/orchestration/server-lifecycle.ts +++ b/apps/sim/lib/mcp/orchestration/server-lifecycle.ts @@ -1,4 +1,4 @@ -import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' +import { AuditAction, AuditResourceType, auditUpdatedFields, recordAudit } from '@sim/audit' import { db, mcpServers } from '@sim/db' import { mcpServerOauth } from '@sim/db/schema' import { createLogger } from '@sim/logger' @@ -400,7 +400,7 @@ export async function updateMcpServer( success: true, server, configurationChanged: shouldClearCache, - updatedFields: Object.keys(updateData).filter((key) => key !== 'updatedAt'), + updatedFields: auditUpdatedFields(updateData), } } catch (error) { logger.error('Failed to update MCP server', { error }) diff --git a/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.test.ts b/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.test.ts index f6437a111e8..e1e2ed0dc1c 100644 --- a/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.test.ts +++ b/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.test.ts @@ -1,7 +1,7 @@ /** * @vitest-environment node */ -import { dbChainMock, dbChainMockFns, resetDbChainMock, schemaMock } from '@sim/testing' +import { auditMock, dbChainMock, dbChainMockFns, resetDbChainMock, schemaMock } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('@sim/audit', () => ({ @@ -14,6 +14,7 @@ vi.mock('@sim/audit', () => ({ MCP_TOOL: 'mcp_tool', }, recordAudit: vi.fn(), + auditUpdatedFields: auditMock.auditUpdatedFields, })) vi.mock('@sim/db', () => ({ ...dbChainMock, diff --git a/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.ts b/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.ts index a10d236b473..4dd1cad048e 100644 --- a/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.ts +++ b/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.ts @@ -1,4 +1,4 @@ -import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' +import { AuditAction, AuditResourceType, auditUpdatedFields, recordAudit } from '@sim/audit' import { db, workflow, workflowMcpServer, workflowMcpTool } from '@sim/db' import { createLogger } from '@sim/logger' import { generateId } from '@sim/utils/id' @@ -549,7 +549,7 @@ export async function performUpdateWorkflowMcpServer( if (params.description !== undefined) updateData.description = params.description?.trim() || null if (params.isPublic !== undefined) updateData.isPublic = params.isPublic - const updatedFields = Object.keys(updateData).filter((key) => key !== 'updatedAt') + const updatedFields = auditUpdatedFields(updateData) try { const [server] = await db @@ -936,7 +936,7 @@ export async function performUpdateWorkflowMcpTool( updateData.parameterSchema = applyDescriptionOverrides(baseSchema, overrides) } - const updatedFields = Object.keys(updateData).filter((key) => key !== 'updatedAt') + const updatedFields = auditUpdatedFields(updateData) const tool = await db.transaction(async (tx) => { await acquireWorkflowMcpServerLock(tx, params.serverId) diff --git a/packages/audit/src/index.ts b/packages/audit/src/index.ts index 2813a231de4..41dd4b7f24e 100644 --- a/packages/audit/src/index.ts +++ b/packages/audit/src/index.ts @@ -1,3 +1,4 @@ export { recordAudit, recordAuditBatch } from './log' export type { AuditActionType, AuditResourceTypeValue } from './types' export { AuditAction, AuditResourceType } from './types' +export { auditUpdatedFields } from './updated-fields' diff --git a/packages/audit/src/updated-fields.test.ts b/packages/audit/src/updated-fields.test.ts new file mode 100644 index 00000000000..c6e8c277384 --- /dev/null +++ b/packages/audit/src/updated-fields.test.ts @@ -0,0 +1,46 @@ +import { auditMock } from '@sim/testing' +import { describe, expect, it } from 'vitest' +import { auditUpdatedFields } from './updated-fields' + +describe('auditUpdatedFields', () => { + it('returns the written columns', () => { + expect(auditUpdatedFields({ name: 'Renamed', url: 'https://example.com' })).toEqual([ + 'name', + 'url', + ]) + }) + + it('drops updatedAt, which every write moves', () => { + expect(auditUpdatedFields({ name: 'Renamed', updatedAt: new Date() })).toEqual(['name']) + }) + + it('keeps columns explicitly written as null — clearing a value is a change', () => { + expect(auditUpdatedFields({ lastConnected: null, lastError: null })).toEqual([ + 'lastConnected', + 'lastError', + ]) + }) + + it('returns an empty list when only updatedAt was written', () => { + expect(auditUpdatedFields({ updatedAt: new Date() })).toEqual([]) + }) + + /** + * `auditMock` carries its own copy because `@sim/audit` devDepends on + * `@sim/testing` — importing the real helper there would close a package + * cycle. The copy is pinned from this side instead, where the dependency + * already runs the safe direction. + */ + it('stays in step with the copy @sim/testing hands to mocked callers', () => { + const cases: object[] = [ + { name: 'Renamed', url: 'https://example.com' }, + { name: 'Renamed', updatedAt: new Date() }, + { lastConnected: null, lastError: null }, + { updatedAt: new Date() }, + {}, + ] + for (const updateValues of cases) { + expect(auditMock.auditUpdatedFields(updateValues)).toEqual(auditUpdatedFields(updateValues)) + } + }) +}) diff --git a/packages/audit/src/updated-fields.ts b/packages/audit/src/updated-fields.ts new file mode 100644 index 00000000000..dcefa61376a --- /dev/null +++ b/packages/audit/src/updated-fields.ts @@ -0,0 +1,15 @@ +/** + * Columns a write touched, ready for an audit row's `updatedFields` metadata. + * + * Pass the update object the write actually applied — never the caller's + * params. A param is not a write: an unchanged value still arrives, and a + * writer often sets columns nobody asked for (a status reset forced by some + * other change). Deriving names from input is what makes audit rows name fields + * that were never written and omit the ones that were. + * + * `updatedAt` is excluded because every write moves it, so it is noise in every + * row. Keeping that rule here means it is one edit if the set ever grows. + */ +export function auditUpdatedFields(updateValues: object): string[] { + return Object.keys(updateValues).filter((key) => key !== 'updatedAt') +} diff --git a/packages/testing/src/mocks/audit.mock.ts b/packages/testing/src/mocks/audit.mock.ts index fc7d43b2311..78827f39e8d 100644 --- a/packages/testing/src/mocks/audit.mock.ts +++ b/packages/testing/src/mocks/audit.mock.ts @@ -28,6 +28,12 @@ export const auditMockFns = { export const auditMock = { recordAudit: auditMockFns.mockRecordAudit, recordAuditBatch: auditMockFns.mockRecordAuditBatch, + /** + * Real implementation, not a stub: callers under test derive their audit + * metadata through it, so stubbing it would erase what the test asserts. + */ + auditUpdatedFields: (updateValues: object): string[] => + Object.keys(updateValues).filter((key) => key !== 'updatedAt'), AuditAction: { API_KEY_CREATED: 'api_key.created', API_KEY_UPDATED: 'api_key.updated',