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
9 changes: 9 additions & 0 deletions apps/sim/lib/auth/oauth-access-token.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ function row(overrides: Record<string, unknown> = {}) {
id: 'token-1',
userId: 'user-1',
clientId: 'sim-cli',
clientName: 'Sim CLI',
scopes: ['offline_access', 'api:read'],
resource: null,
expiresAt: new Date(Date.now() + 60_000),
Expand Down Expand Up @@ -80,6 +81,7 @@ describe('verifyOAuthAccessToken', () => {
kind: 'oauth_access_token',
userId: 'user-1',
clientId: 'sim-cli',
clientName: 'Sim CLI',
tokenId: 'token-1',
scopes: ['offline_access', 'api:read'],
expiresAt: expect.any(Date),
Expand All @@ -92,6 +94,13 @@ describe('verifyOAuthAccessToken', () => {
)
})

it('does not invent a display name for an unnamed OAuth client', async () => {
queueTableRows(schemaMock.oauthAccessToken, [row({ clientName: null })])
const principal = await verifyOAuthAccessToken('sim_oat_secret')
expect(principal).not.toHaveProperty('clientName')
expect(principal.clientId).toBe('sim-cli')
})

it('refuses a credential that is not one of ours without a database read', async () => {
expect(await reason('sim_abc')).toBe('malformed')
expect(await reason('sim_oat_')).toBe('malformed')
Expand Down
4 changes: 3 additions & 1 deletion apps/sim/lib/auth/oauth-access-token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { db } from '@sim/db'
import { oauthAccessToken, oauthClient, user } from '@sim/db/schema'
import { createLogger, setRequestAuth } from '@sim/logger'
import { sha256Hex } from '@sim/security/hash'
import { eq } from 'drizzle-orm'
import { eq, sql } from 'drizzle-orm'
import { isAccountBlocked } from '@/lib/auth/ban'
import {
OAUTH_ACCESS_TOKEN_PREFIX,
Expand Down Expand Up @@ -99,6 +99,7 @@ export async function verifyOAuthAccessToken(
id: oauthAccessToken.id,
userId: oauthAccessToken.userId,
clientId: oauthAccessToken.clientId,
clientName: sql<string | null>`left(${oauthClient.name}, 256)`,
scopes: oauthAccessToken.scopes,
resource: oauthAccessToken.resource,
expiresAt: oauthAccessToken.expiresAt,
Expand Down Expand Up @@ -142,6 +143,7 @@ export async function verifyOAuthAccessToken(
kind: 'oauth_access_token',
userId: row.userId,
clientId: row.clientId,
...(row.clientName ? { clientName: row.clientName } : {}),
tokenId: row.id,
scopes: row.scopes,
expiresAt: row.expiresAt,
Expand Down
8 changes: 8 additions & 0 deletions apps/sim/lib/auth/oauth-principal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,12 @@ describe('oauth_access_token principal', () => {
parsePrincipal({ ...serialized, principal: { ...serialized.principal, expiresAt: 'soon' } })
).toThrow('expiresAt must be an ISO timestamp')
})

it('keeps display metadata out of persisted workflow authority', () => {
const named = { ...principal, clientName: 'Registered app' }
const serialized = serializePrincipal(named)
expect(serialized.principal).not.toHaveProperty('clientName')
expect(parsePrincipal(serialized)).toEqual(principal)
expect(toPrincipalActor(named)).toEqual(toPrincipalActor(principal))
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
oauthConsent,
organization,
organizationSearchIntegration,
organizationSearchMcpInvocation,
rateLimitBucket,
user,
workspace,
Expand All @@ -35,9 +36,19 @@ import { generateId } from '@sim/utils/id'
import { isPlainRecord } from '@sim/utils/object'
import { and, eq, inArray } from 'drizzle-orm'
import { NextRequest } from 'next/server'
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest'

const fixtures = vi.hoisted(() => ({ storageRoot: '' }))
const fixtures = vi.hoisted(() => ({
storageRoot: '',
afterResponse: [] as Array<() => Promise<void>>,
}))
vi.mock('@/lib/core/utils/after-response', () => ({
afterResponse: (task: () => Promise<void>) => fixtures.afterResponse.push(task),
}))

async function flushAfterResponse() {
for (const task of fixtures.afterResponse.splice(0)) await task()
}
vi.mock('@/lib/uploads/core/setup.server', () => ({
get UPLOAD_DIR_SERVER() {
return fixtures.storageRoot
Expand Down Expand Up @@ -401,6 +412,8 @@ describe('organization Search MCP with real ingestion and current access', () =>
bobOAuth = await connect(OAUTH_ACCESS_TOKEN_PREFIX + oauthTokens.bob, true)
})

afterEach(flushAfterResponse)

afterAll(async () => {
await Promise.all(clients.map((client) => client.close()))
await db.delete(oauthClient).where(eq(oauthClient.clientId, oauthClientId))
Expand Down Expand Up @@ -508,6 +521,74 @@ describe('organization Search MCP with real ingestion and current access', () =>
expect(await applicationSearch(bobPrincipal)).toEqual([])
})

it('persists content-free per-client tool outcomes separately from search counters', async () => {
await db
.delete(organizationSearchMcpInvocation)
.where(eq(organizationSearchMcpInvocation.organizationId, organizationId))
const clientName = 'MCP fixture client'.repeat(20)
await db
.update(oauthClient)
.set({ name: clientName })
.where(eq(oauthClient.clientId, oauthClientId))
try {
await aliceOAuth.listTools()
expect(fixtures.afterResponse).toHaveLength(0)
await search(aliceOAuth)
await value(aliceOAuth, 'read_document', { documentId })
expect((await call(bob, 'read_document', { documentId })).isError).toBe(true)
expect(fixtures.afterResponse).toHaveLength(3)
await db
.update(oauthClient)
.set({ name: 'Renamed client' })
.where(eq(oauthClient.clientId, oauthClientId))
await flushAfterResponse()
const rows = await db
.select()
.from(organizationSearchMcpInvocation)
.where(eq(organizationSearchMcpInvocation.organizationId, organizationId))
.orderBy(organizationSearchMcpInvocation.createdAt)
.limit(10)
expect(rows).toHaveLength(3)
expect(rows).toMatchObject([
{
organizationId,
userId: aliceId,
authKind: 'oauth_access_token',
oauthClientId,
clientName: clientName.slice(0, 256),
toolName: 'search',
outcome: 'success',
},
{
organizationId,
userId: aliceId,
authKind: 'oauth_access_token',
oauthClientId,
clientName: clientName.slice(0, 256),
toolName: 'read_document',
outcome: 'success',
},
{
organizationId,
userId: bobId,
authKind: 'personal_api_key',
oauthClientId: null,
clientName: null,
toolName: 'read_document',
outcome: 'error',
},
])
expect(rows.every((row) => row.durationMs >= 0)).toBe(true)
expect(JSON.stringify(rows)).not.toContain(documentId)
expect(JSON.stringify(rows)).not.toContain(oauthTokens.alice)
} finally {
await db
.update(oauthClient)
.set({ name: 'Search MCP OAuth fixture' })
.where(eq(oauthClient.clientId, oauthClientId))
}
})

it('enforces current document and organization access on Search OAuth clients', async () => {
expect((await aliceOAuth.listTools()).tools).toHaveLength(3)
expect(await search(aliceOAuth)).toEqual(await search(alice))
Expand Down
87 changes: 87 additions & 0 deletions apps/sim/lib/knowledge/mcp/activity.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/** @vitest-environment node */
import { beforeEach, describe, expect, it, vi } from 'vitest'

const mocks = vi.hoisted(() => ({
values: vi.fn(),
insert: vi.fn(),
execute: vi.fn(),
transaction: vi.fn(),
}))
vi.mock('@sim/db', () => ({ db: { transaction: mocks.transaction } }))

import {
recordOrganizationSearchMcpActivity,
type SearchMcpActivityInput,
} from '@/lib/knowledge/mcp/activity'

const activity: SearchMcpActivityInput = {
organizationId: 'org',
userId: 'actor',
authKind: 'personal_api_key',
oauthClientId: null,
clientName: null,
toolName: 'read_document',
outcome: 'success',
durationMs: 42,
createdAt: new Date('2026-01-01T00:00:00Z'),
}

beforeEach(() => {
vi.clearAllMocks()
mocks.insert.mockReturnValue({ values: mocks.values })
mocks.values.mockResolvedValue(undefined)
mocks.execute.mockResolvedValue(undefined)
mocks.transaction.mockImplementation((callback) =>
callback({ execute: mocks.execute, insert: mocks.insert })
)
})

describe('persistent MCP activity', () => {
it('stores an API-key call without inventing an application name', async () => {
await recordOrganizationSearchMcpActivity(activity)
expect(mocks.values).toHaveBeenCalledExactlyOnceWith({
id: expect.any(String),
...activity,
clientName: null,
})
})

it('only persists the allowlisted metadata when extra content is present', async () => {
const input = {
...activity,
query: 'private question',
content: 'private document',
token: 'private token',
}
await recordOrganizationSearchMcpActivity(input)
expect(mocks.values).toHaveBeenCalledExactlyOnceWith({
id: expect.any(String),
...activity,
clientName: null,
})
})

it('sets the transaction deadline before attempting the insert', async () => {
const ready = Promise.withResolvers<void>()
mocks.execute.mockReturnValueOnce(ready.promise)
const recording = recordOrganizationSearchMcpActivity(activity)
expect(mocks.insert).not.toHaveBeenCalled()
expect(JSON.stringify(mocks.execute.mock.calls[0])).toContain(
"SET LOCAL statement_timeout = '2s'"
)
ready.resolve()
await recording
expect(mocks.insert).toHaveBeenCalledOnce()
})

it('does not insert when the deadline could not be established', async () => {
mocks.execute.mockRejectedValueOnce(new Error('unavailable'))
await expect(recordOrganizationSearchMcpActivity(activity)).resolves.toBeUndefined()
expect(mocks.insert).not.toHaveBeenCalled()
})

it('does not propagate storage failures into the request lifecycle', async () => {
mocks.values.mockRejectedValueOnce(new Error('offline'))
await expect(recordOrganizationSearchMcpActivity(activity)).resolves.toBeUndefined()
})
})
48 changes: 48 additions & 0 deletions apps/sim/lib/knowledge/mcp/activity.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { db } from '@sim/db'
import { organizationSearchMcpInvocation } from '@sim/db/schema'
import { createLogger } from '@sim/logger'
import { getErrorMessage } from '@sim/utils/errors'
import { generateId } from '@sim/utils/id'
import { sql } from 'drizzle-orm'

const logger = createLogger('OrganizationSearchMcpActivity')

export type SearchMcpActivityInput = Pick<
typeof organizationSearchMcpInvocation.$inferInsert,
| 'organizationId'
| 'userId'
| 'authKind'
| 'oauthClientId'
| 'clientName'
| 'toolName'
| 'outcome'
| 'durationMs'
| 'createdAt'
>

/** Stores content-free metadata from an admitted MCP request, independently of tool success. */
export async function recordOrganizationSearchMcpActivity(
input: SearchMcpActivityInput
): Promise<void> {
try {
await db.transaction(async (tx) => {
await tx.execute(sql`SET LOCAL statement_timeout = '2s'`)
await tx.insert(organizationSearchMcpInvocation).values({
id: generateId(),
organizationId: input.organizationId,
userId: input.userId,
authKind: input.authKind,
oauthClientId: input.oauthClientId,
clientName: input.clientName,
toolName: input.toolName,
outcome: input.outcome,
durationMs: input.durationMs,
createdAt: input.createdAt,
})
})
} catch (error) {
logger.warn('Failed to record organization Search MCP activity', {
error: getErrorMessage(error),
})
}
}
Loading
Loading