fix(mcp): redact audit URLs and drop unwritten columns from updatedFields

Two review findings on the new upsert audit.

The upsert assigns every column unconditionally, so `description` is present
on updateValues but undefined when the registration omits it. Drizzle skips
undefined in .set(), so deriving keys without checking values made the audit
claim a column the write never touched. Filter by value; null stays, since
clearing a value is a write.

MCP URLs carry tokens in their query string — that is why a silent rewrite of
one matters — and audit rows are readable by org admins who need no workspace
MCP access. Newly auditing rewrites would persist those tokens verbatim, so
every MCP audit row now records the URL through sanitizeUrlForLog, which
strips query and fragment. Applied to the add, update and delete rows alike:
redacting only the new path would leave the same credential in the row a
first registration already writes. A null url stays null rather than becoming
an empty string.

Author: waleedlatif1

apps/sim/lib/mcp/application/use-cases.ts

@@ -4,6 +4,7 @@ import { getPostgresErrorCode } from '@sim/utils/errors'
 import type { ListSortOrder } from '@/lib/api/list-query'
 import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application'
 import { OrchestrationError } from '@/lib/core/orchestration/types'
+import { sanitizeUrlForLog } from '@/lib/core/utils/logging'
 import { mcpServerDelegationPolicy } from '@/lib/mcp/application/authorization'
 import { mcpServerOperations } from '@/lib/mcp/application/operations'
 import {
@@ -197,7 +198,7 @@ function createAudit(
       metadata: {
         serverName: result.server.name,
         transport: result.server.transport,
-        url: result.server.url,
+        url: result.server.url ? sanitizeUrlForLog(result.server.url) : null,
         timeout: result.server.timeout,
         retries: result.server.retries,
         source: input.source,
@@ -321,7 +322,7 @@ function updateAudit(
     metadata: {
       serverName: result.server.name,
       transport: result.server.transport,
-      url: result.server.url,
+      url: result.server.url ? sanitizeUrlForLog(result.server.url) : null,
       updatedFields: result.updatedFields ?? [],
       source: input.source,
     },
@@ -389,7 +390,7 @@ export const deleteMcpServerUseCase = defineAuthorizedWorkspaceUseCase({
     metadata: {
       serverName: result.server.name,
       transport: result.server.transport,
-      url: result.server.url,
+      url: result.server.url ? sanitizeUrlForLog(result.server.url) : null,
       source: input.source,
     },
   }),

apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts

@@ -70,6 +70,8 @@ describe('MCP server lifecycle orchestration', () => {
     auditMockFns.mockRecordAudit.mock.calls.at(-1)?.[0].metadata.updatedFields
   const auditAction = (): string | undefined =>
     auditMockFns.mockRecordAudit.mock.calls.at(-1)?.[0].action
+  const auditMetadata = (): Record<string, unknown> | undefined =>
+    auditMockFns.mockRecordAudit.mock.calls.at(-1)?.[0].metadata
 
   beforeEach(() => {
     vi.clearAllMocks()
@@ -286,6 +288,12 @@ describe('MCP server lifecycle orchestration', () => {
     expect(result.revived).toBe(false)
     expect(auditAction()).toBe(AuditAction.MCP_SERVER_UPDATED)
     expect(auditUpdatedFields()).toEqual(expect.arrayContaining(['url', 'headers']))
+    // The registration omitted `description`, and Drizzle skips undefined in
+    // .set(), so the audit must not claim that column was written.
+    expect(auditUpdatedFields()).not.toContain('description')
+    // A query string routinely carries the endpoint's token, and audit rows are
+    // readable by org admins who need no workspace MCP access.
+    expect(auditMetadata()?.url).toBe('https://example.com/mcp')
   })
 
   it('audits a re-registration that revives a soft-deleted server as an addition', async () => {

apps/sim/lib/mcp/orchestration/server-lifecycle.ts

@@ -6,6 +6,7 @@ import { generateId } from '@sim/utils/id'
 import { and, eq, isNull } from 'drizzle-orm'
 import type { NextRequest } from 'next/server'
 import { encryptSecret } from '@/lib/core/security/encryption'
+import { sanitizeUrlForLog } from '@/lib/core/utils/logging'
 import {
   McpDnsResolutionError,
   McpDomainNotAllowedError,
@@ -245,7 +246,16 @@ export async function createMcpServer(
         if (params.oauthClientSecretProvided) {
           updateValues.oauthClientSecret = oauthClientSecretEncrypted
         }
-        updatedFields = Object.keys(updateValues).filter((key) => key !== 'updatedAt')
+        /**
+         * Drizzle skips `undefined` in `.set()`, and this object assigns every
+         * column unconditionally — `description` is present but undefined when
+         * the registration omits it. Keys must therefore be filtered by value,
+         * or the audit claims a column the write never touched. `null` stays:
+         * clearing a value is a write.
+         */
+        updatedFields = Object.entries(updateValues)
+          .filter(([key, value]) => key !== 'updatedAt' && value !== undefined)
+          .map(([key]) => key)
         await tx.update(mcpServers).set(updateValues).where(eq(mcpServers.id, serverId))
       })
 
@@ -501,7 +511,7 @@ export async function performCreateMcpServer(
       metadata: {
         serverName: result.server.name,
         transport: result.server.transport,
-        url: result.server.url,
+        url: result.server.url ? sanitizeUrlForLog(result.server.url) : null,
         timeout: result.server.timeout,
         retries: result.server.retries,
         source,
@@ -537,7 +547,7 @@ export async function performUpdateMcpServer(
       metadata: {
         serverName: result.server.name,
         transport: result.server.transport,
-        url: result.server.url,
+        url: result.server.url ? sanitizeUrlForLog(result.server.url) : null,
         updatedFields: result.updatedFields ?? [],
       },
       request: params.request,
@@ -586,7 +596,7 @@ export async function performDeleteMcpServer(
       metadata: {
         serverName: result.server.name,
         transport: result.server.transport,
-        url: result.server.url,
+        url: result.server.url ? sanitizeUrlForLog(result.server.url) : null,
         source,
       },
       request: params.request,