Skip to content

Commit 55ccc5d

Browse files
authored
feat(search-mcp): persist client-attributed tool activity (#7892)
* feat(search-mcp): persist client-attributed tool activity * fix(search-mcp): snapshot client attribution during admission
1 parent 212a3a9 commit 55ccc5d

16 files changed

Lines changed: 27868 additions & 51 deletions

File tree

‎apps/sim/lib/auth/oauth-access-token.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ function row(overrides: Record<string, unknown> = {}) {
1818
id: 'token-1',
1919
userId: 'user-1',
2020
clientId: 'sim-cli',
21+
clientName: 'Sim CLI',
2122
scopes: ['offline_access', 'api:read'],
2223
resource: null,
2324
expiresAt: new Date(Date.now() + 60_000),
@@ -80,6 +81,7 @@ describe('verifyOAuthAccessToken', () => {
8081
kind: 'oauth_access_token',
8182
userId: 'user-1',
8283
clientId: 'sim-cli',
84+
clientName: 'Sim CLI',
8385
tokenId: 'token-1',
8486
scopes: ['offline_access', 'api:read'],
8587
expiresAt: expect.any(Date),
@@ -92,6 +94,13 @@ describe('verifyOAuthAccessToken', () => {
9294
)
9395
})
9496

97+
it('does not invent a display name for an unnamed OAuth client', async () => {
98+
queueTableRows(schemaMock.oauthAccessToken, [row({ clientName: null })])
99+
const principal = await verifyOAuthAccessToken('sim_oat_secret')
100+
expect(principal).not.toHaveProperty('clientName')
101+
expect(principal.clientId).toBe('sim-cli')
102+
})
103+
95104
it('refuses a credential that is not one of ours without a database read', async () => {
96105
expect(await reason('sim_abc')).toBe('malformed')
97106
expect(await reason('sim_oat_')).toBe('malformed')

‎apps/sim/lib/auth/oauth-access-token.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { db } from '@sim/db'
33
import { oauthAccessToken, oauthClient, user } from '@sim/db/schema'
44
import { createLogger, setRequestAuth } from '@sim/logger'
55
import { sha256Hex } from '@sim/security/hash'
6-
import { eq } from 'drizzle-orm'
6+
import { eq, sql } from 'drizzle-orm'
77
import { isAccountBlocked } from '@/lib/auth/ban'
88
import {
99
OAUTH_ACCESS_TOKEN_PREFIX,
@@ -99,6 +99,7 @@ export async function verifyOAuthAccessToken(
9999
id: oauthAccessToken.id,
100100
userId: oauthAccessToken.userId,
101101
clientId: oauthAccessToken.clientId,
102+
clientName: sql<string | null>`left(${oauthClient.name}, 256)`,
102103
scopes: oauthAccessToken.scopes,
103104
resource: oauthAccessToken.resource,
104105
expiresAt: oauthAccessToken.expiresAt,
@@ -142,6 +143,7 @@ export async function verifyOAuthAccessToken(
142143
kind: 'oauth_access_token',
143144
userId: row.userId,
144145
clientId: row.clientId,
146+
...(row.clientName ? { clientName: row.clientName } : {}),
145147
tokenId: row.id,
146148
scopes: row.scopes,
147149
expiresAt: row.expiresAt,

‎apps/sim/lib/auth/oauth-principal.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,12 @@ describe('oauth_access_token principal', () => {
6666
parsePrincipal({ ...serialized, principal: { ...serialized.principal, expiresAt: 'soon' } })
6767
).toThrow('expiresAt must be an ISO timestamp')
6868
})
69+
70+
it('keeps display metadata out of persisted workflow authority', () => {
71+
const named = { ...principal, clientName: 'Registered app' }
72+
const serialized = serializePrincipal(named)
73+
expect(serialized.principal).not.toHaveProperty('clientName')
74+
expect(parsePrincipal(serialized)).toEqual(principal)
75+
expect(toPrincipalActor(named)).toEqual(toPrincipalActor(principal))
76+
})
6977
})

‎apps/sim/lib/knowledge/__integration__/organization-mcp-search.integration.ts‎

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
oauthConsent,
2727
organization,
2828
organizationSearchIntegration,
29+
organizationSearchMcpInvocation,
2930
rateLimitBucket,
3031
user,
3132
workspace,
@@ -35,9 +36,19 @@ import { generateId } from '@sim/utils/id'
3536
import { isPlainRecord } from '@sim/utils/object'
3637
import { and, eq, inArray } from 'drizzle-orm'
3738
import { NextRequest } from 'next/server'
38-
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
39+
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest'
3940

40-
const fixtures = vi.hoisted(() => ({ storageRoot: '' }))
41+
const fixtures = vi.hoisted(() => ({
42+
storageRoot: '',
43+
afterResponse: [] as Array<() => Promise<void>>,
44+
}))
45+
vi.mock('@/lib/core/utils/after-response', () => ({
46+
afterResponse: (task: () => Promise<void>) => fixtures.afterResponse.push(task),
47+
}))
48+
49+
async function flushAfterResponse() {
50+
for (const task of fixtures.afterResponse.splice(0)) await task()
51+
}
4152
vi.mock('@/lib/uploads/core/setup.server', () => ({
4253
get UPLOAD_DIR_SERVER() {
4354
return fixtures.storageRoot
@@ -401,6 +412,8 @@ describe('organization Search MCP with real ingestion and current access', () =>
401412
bobOAuth = await connect(OAUTH_ACCESS_TOKEN_PREFIX + oauthTokens.bob, true)
402413
})
403414

415+
afterEach(flushAfterResponse)
416+
404417
afterAll(async () => {
405418
await Promise.all(clients.map((client) => client.close()))
406419
await db.delete(oauthClient).where(eq(oauthClient.clientId, oauthClientId))
@@ -508,6 +521,74 @@ describe('organization Search MCP with real ingestion and current access', () =>
508521
expect(await applicationSearch(bobPrincipal)).toEqual([])
509522
})
510523

524+
it('persists content-free per-client tool outcomes separately from search counters', async () => {
525+
await db
526+
.delete(organizationSearchMcpInvocation)
527+
.where(eq(organizationSearchMcpInvocation.organizationId, organizationId))
528+
const clientName = 'MCP fixture client'.repeat(20)
529+
await db
530+
.update(oauthClient)
531+
.set({ name: clientName })
532+
.where(eq(oauthClient.clientId, oauthClientId))
533+
try {
534+
await aliceOAuth.listTools()
535+
expect(fixtures.afterResponse).toHaveLength(0)
536+
await search(aliceOAuth)
537+
await value(aliceOAuth, 'read_document', { documentId })
538+
expect((await call(bob, 'read_document', { documentId })).isError).toBe(true)
539+
expect(fixtures.afterResponse).toHaveLength(3)
540+
await db
541+
.update(oauthClient)
542+
.set({ name: 'Renamed client' })
543+
.where(eq(oauthClient.clientId, oauthClientId))
544+
await flushAfterResponse()
545+
const rows = await db
546+
.select()
547+
.from(organizationSearchMcpInvocation)
548+
.where(eq(organizationSearchMcpInvocation.organizationId, organizationId))
549+
.orderBy(organizationSearchMcpInvocation.createdAt)
550+
.limit(10)
551+
expect(rows).toHaveLength(3)
552+
expect(rows).toMatchObject([
553+
{
554+
organizationId,
555+
userId: aliceId,
556+
authKind: 'oauth_access_token',
557+
oauthClientId,
558+
clientName: clientName.slice(0, 256),
559+
toolName: 'search',
560+
outcome: 'success',
561+
},
562+
{
563+
organizationId,
564+
userId: aliceId,
565+
authKind: 'oauth_access_token',
566+
oauthClientId,
567+
clientName: clientName.slice(0, 256),
568+
toolName: 'read_document',
569+
outcome: 'success',
570+
},
571+
{
572+
organizationId,
573+
userId: bobId,
574+
authKind: 'personal_api_key',
575+
oauthClientId: null,
576+
clientName: null,
577+
toolName: 'read_document',
578+
outcome: 'error',
579+
},
580+
])
581+
expect(rows.every((row) => row.durationMs >= 0)).toBe(true)
582+
expect(JSON.stringify(rows)).not.toContain(documentId)
583+
expect(JSON.stringify(rows)).not.toContain(oauthTokens.alice)
584+
} finally {
585+
await db
586+
.update(oauthClient)
587+
.set({ name: 'Search MCP OAuth fixture' })
588+
.where(eq(oauthClient.clientId, oauthClientId))
589+
}
590+
})
591+
511592
it('enforces current document and organization access on Search OAuth clients', async () => {
512593
expect((await aliceOAuth.listTools()).tools).toHaveLength(3)
513594
expect(await search(aliceOAuth)).toEqual(await search(alice))
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/** @vitest-environment node */
2+
import { beforeEach, describe, expect, it, vi } from 'vitest'
3+
4+
const mocks = vi.hoisted(() => ({
5+
values: vi.fn(),
6+
insert: vi.fn(),
7+
execute: vi.fn(),
8+
transaction: vi.fn(),
9+
}))
10+
vi.mock('@sim/db', () => ({ db: { transaction: mocks.transaction } }))
11+
12+
import {
13+
recordOrganizationSearchMcpActivity,
14+
type SearchMcpActivityInput,
15+
} from '@/lib/knowledge/mcp/activity'
16+
17+
const activity: SearchMcpActivityInput = {
18+
organizationId: 'org',
19+
userId: 'actor',
20+
authKind: 'personal_api_key',
21+
oauthClientId: null,
22+
clientName: null,
23+
toolName: 'read_document',
24+
outcome: 'success',
25+
durationMs: 42,
26+
createdAt: new Date('2026-01-01T00:00:00Z'),
27+
}
28+
29+
beforeEach(() => {
30+
vi.clearAllMocks()
31+
mocks.insert.mockReturnValue({ values: mocks.values })
32+
mocks.values.mockResolvedValue(undefined)
33+
mocks.execute.mockResolvedValue(undefined)
34+
mocks.transaction.mockImplementation((callback) =>
35+
callback({ execute: mocks.execute, insert: mocks.insert })
36+
)
37+
})
38+
39+
describe('persistent MCP activity', () => {
40+
it('stores an API-key call without inventing an application name', async () => {
41+
await recordOrganizationSearchMcpActivity(activity)
42+
expect(mocks.values).toHaveBeenCalledExactlyOnceWith({
43+
id: expect.any(String),
44+
...activity,
45+
clientName: null,
46+
})
47+
})
48+
49+
it('only persists the allowlisted metadata when extra content is present', async () => {
50+
const input = {
51+
...activity,
52+
query: 'private question',
53+
content: 'private document',
54+
token: 'private token',
55+
}
56+
await recordOrganizationSearchMcpActivity(input)
57+
expect(mocks.values).toHaveBeenCalledExactlyOnceWith({
58+
id: expect.any(String),
59+
...activity,
60+
clientName: null,
61+
})
62+
})
63+
64+
it('sets the transaction deadline before attempting the insert', async () => {
65+
const ready = Promise.withResolvers<void>()
66+
mocks.execute.mockReturnValueOnce(ready.promise)
67+
const recording = recordOrganizationSearchMcpActivity(activity)
68+
expect(mocks.insert).not.toHaveBeenCalled()
69+
expect(JSON.stringify(mocks.execute.mock.calls[0])).toContain(
70+
"SET LOCAL statement_timeout = '2s'"
71+
)
72+
ready.resolve()
73+
await recording
74+
expect(mocks.insert).toHaveBeenCalledOnce()
75+
})
76+
77+
it('does not insert when the deadline could not be established', async () => {
78+
mocks.execute.mockRejectedValueOnce(new Error('unavailable'))
79+
await expect(recordOrganizationSearchMcpActivity(activity)).resolves.toBeUndefined()
80+
expect(mocks.insert).not.toHaveBeenCalled()
81+
})
82+
83+
it('does not propagate storage failures into the request lifecycle', async () => {
84+
mocks.values.mockRejectedValueOnce(new Error('offline'))
85+
await expect(recordOrganizationSearchMcpActivity(activity)).resolves.toBeUndefined()
86+
})
87+
})
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { db } from '@sim/db'
2+
import { organizationSearchMcpInvocation } from '@sim/db/schema'
3+
import { createLogger } from '@sim/logger'
4+
import { getErrorMessage } from '@sim/utils/errors'
5+
import { generateId } from '@sim/utils/id'
6+
import { sql } from 'drizzle-orm'
7+
8+
const logger = createLogger('OrganizationSearchMcpActivity')
9+
10+
export type SearchMcpActivityInput = Pick<
11+
typeof organizationSearchMcpInvocation.$inferInsert,
12+
| 'organizationId'
13+
| 'userId'
14+
| 'authKind'
15+
| 'oauthClientId'
16+
| 'clientName'
17+
| 'toolName'
18+
| 'outcome'
19+
| 'durationMs'
20+
| 'createdAt'
21+
>
22+
23+
/** Stores content-free metadata from an admitted MCP request, independently of tool success. */
24+
export async function recordOrganizationSearchMcpActivity(
25+
input: SearchMcpActivityInput
26+
): Promise<void> {
27+
try {
28+
await db.transaction(async (tx) => {
29+
await tx.execute(sql`SET LOCAL statement_timeout = '2s'`)
30+
await tx.insert(organizationSearchMcpInvocation).values({
31+
id: generateId(),
32+
organizationId: input.organizationId,
33+
userId: input.userId,
34+
authKind: input.authKind,
35+
oauthClientId: input.oauthClientId,
36+
clientName: input.clientName,
37+
toolName: input.toolName,
38+
outcome: input.outcome,
39+
durationMs: input.durationMs,
40+
createdAt: input.createdAt,
41+
})
42+
})
43+
} catch (error) {
44+
logger.warn('Failed to record organization Search MCP activity', {
45+
error: getErrorMessage(error),
46+
})
47+
}
48+
}

0 commit comments

Comments
 (0)