Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions apps/sim/app/api/v1/admin/organizations/[id]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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,
Expand All @@ -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,
})

Expand Down
9 changes: 8 additions & 1 deletion apps/sim/lib/credentials/orchestration/index.test.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions apps/sim/lib/credentials/orchestration/index.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions apps/sim/lib/mcp/orchestration/server-lifecycle.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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 })
Expand Down
Original file line number Diff line number Diff line change
@@ -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', () => ({
Expand All @@ -14,6 +14,7 @@ vi.mock('@sim/audit', () => ({
MCP_TOOL: 'mcp_tool',
},
recordAudit: vi.fn(),
auditUpdatedFields: auditMock.auditUpdatedFields,
}))
vi.mock('@sim/db', () => ({
...dbChainMock,
Expand Down
6 changes: 3 additions & 3 deletions apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions packages/audit/src/index.ts
Original file line number Diff line number Diff line change
@@ -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'
Comment thread
waleedlatif1 marked this conversation as resolved.
46 changes: 46 additions & 0 deletions packages/audit/src/updated-fields.test.ts
Original file line number Diff line number Diff line change
@@ -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))
}
})
})
15 changes: 15 additions & 0 deletions packages/audit/src/updated-fields.ts
Original file line number Diff line number Diff line change
@@ -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')
}
6 changes: 6 additions & 0 deletions packages/testing/src/mocks/audit.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Comment thread
waleedlatif1 marked this conversation as resolved.
AuditAction: {
API_KEY_CREATED: 'api_key.created',
API_KEY_UPDATED: 'api_key.updated',
Expand Down
Loading