diff --git a/apps/sim/lib/mcp/application/use-cases.ts b/apps/sim/lib/mcp/application/use-cases.ts index b656ea30cf5..bbc6519e335 100644 --- a/apps/sim/lib/mcp/application/use-cases.ts +++ b/apps/sim/lib/mcp/application/use-cases.ts @@ -315,9 +315,7 @@ function updateAudit( serverName: result.server.name, transport: result.server.transport, url: result.server.url, - updatedFields: Object.keys(input).filter( - (key) => !['workspaceId', 'serverId', 'source'].includes(key) - ), + updatedFields: result.updatedFields ?? [], source: input.source, }, } diff --git a/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts b/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts index 073c5a1d0fa..cdcd59d2f58 100644 --- a/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts +++ b/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts @@ -3,6 +3,7 @@ */ import { auditMock, + auditMockFns, dbChainMock, dbChainMockFns, encryptionMock, @@ -64,6 +65,9 @@ import { } from '@/lib/mcp/orchestration/server-lifecycle' describe('MCP server lifecycle orchestration', () => { + const auditUpdatedFields = (): string[] | undefined => + auditMockFns.mockRecordAudit.mock.calls.at(-1)?.[0].metadata.updatedFields + beforeEach(() => { vi.clearAllMocks() resetDbChainMock() @@ -151,6 +155,48 @@ describe('MCP server lifecycle orchestration', () => { ) // ...and revoke the now-orphaned OAuth tokens rather than leaving them stored and valid. expect(mockRevokeOauthTokens).toHaveBeenCalledWith('server-1', 'workspace-1') + // The reset columns are the point of this audit row — an auditor needs to see + // that the connection was invalidated, not just that authType was touched. + expect(auditUpdatedFields()).toEqual( + expect.arrayContaining(['authType', 'connectionStatus', 'lastConnected', 'lastError']) + ) + }) + + it('audits only the columns an edit wrote, not the params it was handed', async () => { + dbChainMockFns.limit.mockResolvedValueOnce([ + { + url: 'https://example.com/mcp', + authType: 'headers', + oauthClientId: null, + oauthClientSecret: null, + }, + ]) + dbChainMockFns.returning.mockResolvedValueOnce([ + { + id: 'server-1', + workspaceId: 'workspace-1', + name: 'Renamed', + transport: 'streamable-http', + url: 'https://example.com/mcp', + authType: 'headers', + }, + ]) + + // A rename from the settings modal: the route always sends the OAuth params. + const result = await performUpdateMcpServer({ + workspaceId: 'workspace-1', + userId: 'user-1', + serverId: 'server-1', + name: 'Renamed', + oauthClientId: null, + oauthClientIdProvided: false, + oauthClientSecretProvided: false, + }) + + expect(result.success).toBe(true) + // `updatedAt` is excluded deliberately — it moves on every write, so it would + // be noise in every audit row. + expect(auditUpdatedFields()).toEqual(['name']) }) it('resets to disconnected when a create/upsert flips an existing OAuth server to headers', async () => { diff --git a/apps/sim/lib/mcp/orchestration/server-lifecycle.ts b/apps/sim/lib/mcp/orchestration/server-lifecycle.ts index 6f378ffafd5..2899aa659b8 100644 --- a/apps/sim/lib/mcp/orchestration/server-lifecycle.ts +++ b/apps/sim/lib/mcp/orchestration/server-lifecycle.ts @@ -91,6 +91,13 @@ export interface PerformMcpServerResult { updated?: boolean authType?: McpAuthType configurationChanged?: boolean + /** + * Fields the update's SET clause wrote, minus `updatedAt`, for audit. Only + * the writer knows these: a param is not a write, and callers cannot see the + * `connectionStatus`/`lastConnected`/`lastError` reset that an auth or + * credential change forces. Record this instead of deriving names from input. + */ + updatedFields?: string[] } export type McpServerMutationAction = 'create' | 'update' | 'delete' @@ -389,7 +396,12 @@ export async function updateMcpServer( params.timeout !== undefined || params.retries !== undefined - return { success: true, server, configurationChanged: shouldClearCache } + return { + success: true, + server, + configurationChanged: shouldClearCache, + updatedFields: Object.keys(updateData).filter((key) => key !== 'updatedAt'), + } } catch (error) { logger.error('Failed to update MCP server', { error }) throw error @@ -501,15 +513,7 @@ export async function performUpdateMcpServer( serverName: result.server.name, transport: result.server.transport, url: result.server.url, - updatedFields: Object.entries(params) - .filter( - ([key, value]) => - value !== undefined && - !['workspaceId', 'userId', 'serverId', 'actorName', 'actorEmail', 'request'].includes( - key - ) - ) - .map(([key]) => key), + updatedFields: result.updatedFields ?? [], }, request: params.request, })