diff --git a/apps/sim/app/api/v2/custom-tools/[id]/route.test.ts b/apps/sim/app/api/v2/custom-tools/[id]/route.test.ts index 5619a64b1cd..269daa147be 100644 --- a/apps/sim/app/api/v2/custom-tools/[id]/route.test.ts +++ b/apps/sim/app/api/v2/custom-tools/[id]/route.test.ts @@ -1,308 +1,163 @@ /** * @vitest-environment node - * - * Public v2 custom tool detail: the per-id get/update/delete the internal - * surface never had, and the rename guard that keeps a duplicate title from - * reaching the unique index. */ import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -const { - mockCheckRateLimit, - mockResolveWorkspaceAccess, - mockGetWorkspaceCustomTool, - mockGetWorkspaceCustomToolByTitle, - mockDeleteWorkspaceCustomTool, - mockUpdateWorkspaceCustomTool, -} = vi.hoisted(() => ({ - mockCheckRateLimit: vi.fn(), - mockResolveWorkspaceAccess: vi.fn(), - mockGetWorkspaceCustomTool: vi.fn(), - mockGetWorkspaceCustomToolByTitle: vi.fn(), - mockDeleteWorkspaceCustomTool: vi.fn(), - mockUpdateWorkspaceCustomTool: vi.fn(), -})) +const { mocks, MockV2ApiKeyUnauthenticatedError } = vi.hoisted(() => { + class MockV2ApiKeyUnauthenticatedError extends Error {} + return { + mocks: { + authenticate: vi.fn(), + preauthRate: vi.fn(), + operationRate: vi.fn(), + gate: vi.fn(), + get: vi.fn(), + update: vi.fn(), + remove: vi.fn(), + }, + MockV2ApiKeyUnauthenticatedError, + } +}) -vi.mock('@/app/api/v1/middleware', () => ({ - checkRateLimit: mockCheckRateLimit, - resolveWorkspaceAccess: mockResolveWorkspaceAccess, +vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => ({ + authenticateV2ApiKey: mocks.authenticate, + V2ApiKeyUnauthenticatedError: MockV2ApiKeyUnauthenticatedError, })) - -vi.mock('@/lib/workflows/custom-tools/operations', () => ({ - getWorkspaceCustomTool: mockGetWorkspaceCustomTool, - getWorkspaceCustomToolByTitle: mockGetWorkspaceCustomToolByTitle, - deleteWorkspaceCustomTool: mockDeleteWorkspaceCustomTool, - updateWorkspaceCustomTool: mockUpdateWorkspaceCustomTool, +vi.mock('@/lib/core/rate-limiter', () => ({ + RateLimiter: class { + checkRateLimitDirect = mocks.preauthRate + checkRateLimitDirectOrThrow = mocks.operationRate + }, + getRateLimit: vi.fn().mockReturnValue({ + maxTokens: 100, + refillRate: 100, + refillIntervalMs: 60_000, + }), })) - -vi.mock('@/app/api/v2/lib/gate', () => ({ - v2ApiGateError: vi.fn().mockResolvedValue(null), +vi.mock('@/lib/api/server/rate-limit-context', () => ({ + recordRateLimitSnapshot: vi.fn(), + getRateLimitHeaders: vi.fn().mockReturnValue(null), +})) +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: vi.fn().mockReturnValue('request-1'), + getClientIp: vi.fn().mockReturnValue('127.0.0.1'), +})) +vi.mock('@/app/api/v2/lib/gate', () => ({ v2ApiGateError: mocks.gate })) +vi.mock('@/lib/custom-tools/application/use-cases', () => ({ + getWorkspaceCustomToolUseCase: { operation: { id: 'custom_tools.read' }, execute: mocks.get }, + updateWorkspaceCustomToolUseCase: { + operation: { id: 'custom_tools.update' }, + execute: mocks.update, + }, + deleteWorkspaceCustomToolUseCase: { + operation: { id: 'custom_tools.delete' }, + execute: mocks.remove, + }, })) import { DELETE, GET, PATCH } from '@/app/api/v2/custom-tools/[id]/route' +const WORKSPACE_ID = 'workspace-1' +const PRINCIPAL = { kind: 'workspace_api_key' as const, workspaceId: WORKSPACE_ID, keyId: 'key-1' } +const AUTH = { + principal: PRINCIPAL, + rolloutUserId: 'owner-1', + rateLimitSubjectIds: ['workspace:workspace-1'] as const, + rateLimitSubscription: null, + keyType: 'workspace' as const, +} const RATE_LIMIT_OK = { allowed: true, - userId: 'user-1', - keyType: 'workspace', limit: 100, remaining: 99, - resetAt: new Date('2024-01-01T01:00:00Z'), -} - -const RATE_LIMIT_DENIED = { - allowed: false, - limit: 100, - remaining: 0, - resetAt: new Date('2024-01-01T01:00:00Z'), - retryAfterMs: 1000, + resetAt: new Date('2026-01-01T00:00:00Z'), + retryAfterMs: 0, } - -const ACCESS_DENIED = { status: 403, code: 'FORBIDDEN', message: 'Access denied' } - -const TOOL_SCHEMA = { - type: 'function', - function: { - name: 'lookup_order', - parameters: { type: 'object', properties: { orderId: { type: 'string' } } }, +const tool = { + id: 'tool-1', + workspaceId: WORKSPACE_ID, + userId: 'owner-1', + title: 'lookup_order', + schema: { + type: 'function', + function: { name: 'lookup_order', parameters: { type: 'object', properties: {} } }, }, + code: 'return { ok: true }', + createdAt: new Date('2026-01-01T00:00:00Z'), + updatedAt: new Date('2026-01-02T00:00:00Z'), } - -function buildTool(overrides: Record = {}) { - return { - id: 'tool_abc123', - workspaceId: 'workspace-1', - userId: 'user-1', - title: 'lookup_order', - schema: TOOL_SCHEMA, - code: 'return { ok: true }', - createdAt: new Date('2024-01-01T00:00:00Z'), - updatedAt: new Date('2024-01-02T00:00:00Z'), - ...overrides, - } -} - -const routeContext = () => ({ params: Promise.resolve({ id: 'tool_abc123' }) }) -const url = (query = 'workspaceId=workspace-1') => - `http://localhost:3000/api/v2/custom-tools/tool_abc123?${query}` - -const callGet = (query?: string) => GET(new NextRequest(url(query)), routeContext()) -const callDelete = (query?: string) => - DELETE(new NextRequest(url(query), { method: 'DELETE' }), routeContext()) - -function callPatch(body: unknown) { - return PATCH( - new NextRequest('http://localhost:3000/api/v2/custom-tools/tool_abc123', { - method: 'PATCH', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(body), - }), - routeContext() +const context = { params: Promise.resolve({ id: tool.id }) } + +function request(method: 'GET' | 'PATCH' | 'DELETE', body?: unknown) { + return new NextRequest( + `http://localhost:3000/api/v2/custom-tools/${tool.id}?workspaceId=${WORKSPACE_ID}`, + { + method, + headers: { + 'x-api-key': 'key', + ...(body === undefined ? {} : { 'Content-Type': 'application/json' }), + }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + } ) } -describe('GET /api/v2/custom-tools/[id]', () => { +describe('/api/v2/custom-tools/[id]', () => { beforeEach(() => { vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockGetWorkspaceCustomTool.mockResolvedValue(buildTool()) - }) - - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) - - const res = await callGet() - - expect(res.status).toBe(404) - expect(mockGetWorkspaceCustomTool).not.toHaveBeenCalled() - }) - - it('400s when workspaceId is missing', async () => { - const res = await callGet('') - expect(res.status).toBe(400) - expect(mockGetWorkspaceCustomTool).not.toHaveBeenCalled() - }) - - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue(ACCESS_DENIED) - const res = await callGet() - expect(res.status).toBe(403) - expect(mockGetWorkspaceCustomTool).not.toHaveBeenCalled() - }) - - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_DENIED) - const res = await callGet() - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') - }) - - it('404s when the tool is not in the workspace', async () => { - mockGetWorkspaceCustomTool.mockResolvedValue(null) - const res = await callGet() - expect(res.status).toBe(404) - expect((await res.json()).error.code).toBe('NOT_FOUND') - }) - - it('returns the public tool shape without internal scoping columns', async () => { - const res = await callGet() - const body = await res.json() - - expect(res.status).toBe(200) - expect(body.data.customTool).toEqual({ - id: 'tool_abc123', - title: 'lookup_order', - schema: TOOL_SCHEMA, - code: 'return { ok: true }', - createdAt: '2024-01-01T00:00:00.000Z', - updatedAt: '2024-01-02T00:00:00.000Z', - }) - expect(mockGetWorkspaceCustomTool).toHaveBeenCalledWith({ - workspaceId: 'workspace-1', - toolId: 'tool_abc123', + mocks.authenticate.mockResolvedValue(AUTH) + mocks.preauthRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.operationRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.gate.mockResolvedValue(null) + mocks.get.mockResolvedValue({ tool }) + mocks.update.mockResolvedValue({ tool }) + mocks.remove.mockResolvedValue({ tool }) + }) + + it('gets a custom tool through its semantic read operation', async () => { + const response = await GET(request('GET'), context) + + expect(response.status).toBe(200) + expect(mocks.get).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { workspaceId: WORKSPACE_ID, toolId: tool.id }, + request: expect.anything(), }) }) -}) - -describe('PATCH /api/v2/custom-tools/[id]', () => { - beforeEach(() => { - vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockGetWorkspaceCustomTool.mockResolvedValue(buildTool()) - mockGetWorkspaceCustomToolByTitle.mockResolvedValue(null) - mockUpdateWorkspaceCustomTool.mockResolvedValue(buildTool()) - }) - - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) - - const res = await callPatch({ workspaceId: 'workspace-1', code: 'return 1' }) - - expect(res.status).toBe(404) - expect(mockUpdateWorkspaceCustomTool).not.toHaveBeenCalled() - }) - - it('400s when no field to change is supplied', async () => { - const res = await callPatch({ workspaceId: 'workspace-1' }) - expect(res.status).toBe(400) - expect(mockUpdateWorkspaceCustomTool).not.toHaveBeenCalled() - }) - - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue(ACCESS_DENIED) - const res = await callPatch({ workspaceId: 'workspace-1', code: 'return 1' }) - expect(res.status).toBe(403) - expect(mockUpdateWorkspaceCustomTool).not.toHaveBeenCalled() - }) - - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_DENIED) - const res = await callPatch({ workspaceId: 'workspace-1', code: 'return 1' }) - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') - }) - it('404s when the tool is not in the workspace', async () => { - mockGetWorkspaceCustomTool.mockResolvedValue(null) - const res = await callPatch({ workspaceId: 'workspace-1', code: 'return 1' }) - expect(res.status).toBe(404) - expect(mockUpdateWorkspaceCustomTool).not.toHaveBeenCalled() - }) - - it('409s when renaming onto an existing title', async () => { - mockGetWorkspaceCustomToolByTitle.mockResolvedValue(buildTool({ id: 'tool_other' })) - - const res = await callPatch({ workspaceId: 'workspace-1', title: 'taken' }) - - expect(res.status).toBe(409) - expect((await res.json()).error.code).toBe('CONFLICT') - expect(mockUpdateWorkspaceCustomTool).not.toHaveBeenCalled() - }) + it('updates a custom tool through its semantic update operation', async () => { + const response = await PATCH( + request('PATCH', { workspaceId: WORKSPACE_ID, code: 'return 2' }), + context + ) - it('merges the partial body against the stored tool', async () => { - const res = await callPatch({ workspaceId: 'workspace-1', code: 'return 2' }) - - expect(res.status).toBe(200) - expect(mockUpdateWorkspaceCustomTool).toHaveBeenCalledWith({ - workspaceId: 'workspace-1', - toolId: 'tool_abc123', - title: 'lookup_order', - schema: TOOL_SCHEMA, - code: 'return 2', + expect(response.status).toBe(200) + expect(mocks.update).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { workspaceId: WORKSPACE_ID, toolId: tool.id, code: 'return 2', source: 'api' }, + request: expect.anything(), }) }) - it('404s rather than orphaning a tool deleted between the read and the write', async () => { - mockUpdateWorkspaceCustomTool.mockResolvedValue(null) - - const res = await callPatch({ workspaceId: 'workspace-1', code: 'return 2' }) - - expect(res.status).toBe(404) - expect((await res.json()).error.code).toBe('NOT_FOUND') - }) -}) - -describe('DELETE /api/v2/custom-tools/[id]', () => { - beforeEach(() => { - vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockGetWorkspaceCustomTool.mockResolvedValue(buildTool()) - mockDeleteWorkspaceCustomTool.mockResolvedValue(true) - }) - - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) + it('deletes a custom tool through its semantic delete operation', async () => { + const response = await DELETE(request('DELETE'), context) - const res = await callDelete() - - expect(res.status).toBe(404) - expect(mockDeleteWorkspaceCustomTool).not.toHaveBeenCalled() - }) - - it('400s when workspaceId is missing', async () => { - const res = await callDelete('') - expect(res.status).toBe(400) - expect(mockDeleteWorkspaceCustomTool).not.toHaveBeenCalled() - }) - - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue(ACCESS_DENIED) - const res = await callDelete() - expect(res.status).toBe(403) - expect(mockDeleteWorkspaceCustomTool).not.toHaveBeenCalled() + expect(response.status).toBe(200) + expect(await response.json()).toEqual({ data: { id: tool.id, deleted: true } }) + expect(mocks.remove).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { workspaceId: WORKSPACE_ID, toolId: tool.id, source: 'api' }, + request: expect.anything(), + }) }) - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_DENIED) - const res = await callDelete() - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') - }) + it('authenticates before validating an empty patch body', async () => { + mocks.authenticate.mockRejectedValueOnce(new MockV2ApiKeyUnauthenticatedError()) - it('404s when the tool is not in the workspace', async () => { - mockGetWorkspaceCustomTool.mockResolvedValue(null) - const res = await callDelete() - expect(res.status).toBe(404) - expect(mockDeleteWorkspaceCustomTool).not.toHaveBeenCalled() - }) + const response = await PATCH(request('PATCH', {}), context) - it('deletes the tool and acknowledges the id', async () => { - const res = await callDelete() - expect(res.status).toBe(200) - expect(await res.json()).toEqual({ data: { id: 'tool_abc123', deleted: true } }) - expect(mockDeleteWorkspaceCustomTool).toHaveBeenCalledWith({ - workspaceId: 'workspace-1', - toolId: 'tool_abc123', - }) + expect(response.status).toBe(401) + expect(mocks.update).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/app/api/v2/custom-tools/[id]/route.ts b/apps/sim/app/api/v2/custom-tools/[id]/route.ts index a20c9933873..6e04d53abc7 100644 --- a/apps/sim/app/api/v2/custom-tools/[id]/route.ts +++ b/apps/sim/app/api/v2/custom-tools/[id]/route.ts @@ -1,133 +1,65 @@ -import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { v2DeleteCustomToolContract, v2GetCustomToolContract, v2UpdateCustomToolContract, } from '@/lib/api/contracts/v2/custom-tools' import { - deleteWorkspaceCustomTool, - getWorkspaceCustomTool, - getWorkspaceCustomToolByTitle, - updateWorkspaceCustomTool, -} from '@/lib/workflows/custom-tools/operations' -import { withPublicApiRouteHandler } from '@/app/api/public-api-route-handler' -import { resolveWorkspaceAccess } from '@/app/api/v1/middleware' -import { toV2CustomTool, v2CustomToolWriteError } from '@/app/api/v2/custom-tools/utils' -import { v2Data, v2Error, v2WorkspaceAccessError } from '@/app/api/v2/lib/response' + defineV2JsonRoute, + v2ApiKeyAuth, + v2OrchestrationErrorPolicy, + v2RateLimits, +} from '@/lib/api/server/routes' +import { customToolOperations } from '@/lib/custom-tools/application/operations' +import { + deleteWorkspaceCustomToolUseCase, + getWorkspaceCustomToolUseCase, + updateWorkspaceCustomToolUseCase, +} from '@/lib/custom-tools/application/use-cases' +import { toV2CustomTool } from '@/app/api/v2/custom-tools/utils' export const dynamic = 'force-dynamic' export const revalidate = 0 -interface RouteContext { - params: Promise<{ id: string }> -} - /** GET /api/v2/custom-tools/[id] — Fetch a single custom tool. */ -export const GET = withPublicApiRouteHandler({ +export const GET = defineV2JsonRoute({ contract: v2GetCustomToolContract, - rateLimitEndpoint: 'custom-tool-detail', - handler: async ({ input, auth: { userId, rateLimit } }) => { - const { id } = input.params - const { workspaceId } = input.query - - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'read') - if (access) return v2WorkspaceAccessError(access) - - const tool = await getWorkspaceCustomTool({ workspaceId, toolId: id }) - if (!tool) return v2Error('NOT_FOUND', 'Custom tool not found') - - return v2Data({ customTool: toV2CustomTool(tool) }, { rateLimit }) - }, + operation: customToolOperations.read, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ params, query }) => ({ workspaceId: query.workspaceId, toolId: params.id }), + useCase: getWorkspaceCustomToolUseCase, + present: ({ tool }) => ({ data: { customTool: toV2CustomTool(tool) } }), }) -/** PATCH /api/v2/custom-tools/[id] — Update a custom tool. Omitted fields keep their values. */ -export const PATCH = withPublicApiRouteHandler({ +/** PATCH /api/v2/custom-tools/[id] — Update a custom tool. */ +export const PATCH = defineV2JsonRoute({ contract: v2UpdateCustomToolContract, - rateLimitEndpoint: 'custom-tool-detail', - handler: async ({ request, input, auth: { userId, rateLimit } }) => { - try { - const { id } = input.params - const { workspaceId, title, schema, code } = input.body - - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'write') - if (access) return v2WorkspaceAccessError(access) - - const current = await getWorkspaceCustomTool({ workspaceId, toolId: id }) - if (!current) return v2Error('NOT_FOUND', 'Custom tool not found') - - /** - * `upsertCustomTools` replaces title/schema/code wholesale and checks for a - * duplicate title only when inserting, so a rename onto an existing title - * would hit the `custom_tools_workspace_title_unique` index as a 500. Merge - * the partial body against the stored row and check the rename here. - */ - if (title !== undefined && title !== current.title) { - if (await getWorkspaceCustomToolByTitle({ workspaceId, title })) { - return v2Error( - 'CONFLICT', - `A custom tool titled "${title}" already exists in this workspace` - ) - } - } - - const updated = await updateWorkspaceCustomTool({ - workspaceId, - toolId: id, - title: title ?? current.title, - schema: schema ?? current.schema, - code: code ?? current.code, - }) - if (!updated) return v2Error('NOT_FOUND', 'Custom tool not found') - - recordAudit({ - workspaceId, - actorId: userId, - action: AuditAction.CUSTOM_TOOL_UPDATED, - resourceType: AuditResourceType.CUSTOM_TOOL, - resourceId: updated.id, - resourceName: updated.title, - description: `Updated custom tool "${updated.title}" via API`, - request, - }) - - return v2Data({ customTool: toV2CustomTool(updated) }, { rateLimit }) - } catch (error) { - const writeError = v2CustomToolWriteError(error) - if (writeError) return writeError - - throw error - } - }, + operation: customToolOperations.update, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ params, body }) => ({ + ...body, + toolId: params.id, + source: 'api' as const, + }), + useCase: updateWorkspaceCustomToolUseCase, + present: ({ tool }) => ({ data: { customTool: toV2CustomTool(tool) } }), }) /** DELETE /api/v2/custom-tools/[id] — Delete a custom tool. */ -export const DELETE = withPublicApiRouteHandler({ +export const DELETE = defineV2JsonRoute({ contract: v2DeleteCustomToolContract, - rateLimitEndpoint: 'custom-tool-detail', - handler: async ({ request, input, auth: { userId, rateLimit } }) => { - const { id } = input.params - const { workspaceId } = input.query - - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'write') - if (access) return v2WorkspaceAccessError(access) - - const tool = await getWorkspaceCustomTool({ workspaceId, toolId: id }) - if (!tool) return v2Error('NOT_FOUND', 'Custom tool not found') - - const deleted = await deleteWorkspaceCustomTool({ workspaceId, toolId: id }) - if (!deleted) return v2Error('NOT_FOUND', 'Custom tool not found') - - recordAudit({ - workspaceId, - actorId: userId, - action: AuditAction.CUSTOM_TOOL_DELETED, - resourceType: AuditResourceType.CUSTOM_TOOL, - resourceId: id, - resourceName: tool.title, - description: `Deleted custom tool "${tool.title}" via API`, - request, - }) - - return v2Data({ id, deleted: true as const }, { rateLimit }) - }, + operation: customToolOperations.delete, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ params, query }) => ({ + workspaceId: query.workspaceId, + toolId: params.id, + source: 'api' as const, + }), + useCase: deleteWorkspaceCustomToolUseCase, + present: ({ tool }) => ({ data: { id: tool.id, deleted: true as const } }), }) diff --git a/apps/sim/app/api/v2/custom-tools/route.test.ts b/apps/sim/app/api/v2/custom-tools/route.test.ts index 7ca46e81b0c..a3ee031338f 100644 --- a/apps/sim/app/api/v2/custom-tools/route.test.ts +++ b/apps/sim/app/api/v2/custom-tools/route.test.ts @@ -1,302 +1,178 @@ /** * @vitest-environment node - * - * Public v2 custom tools list/create: gate ordering, contract validation, and - * the workspace-scoped single-resource create that replaced the bulk upsert. */ import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -const { - mockCheckRateLimit, - mockResolveWorkspaceAccess, - mockListWorkspaceCustomTools, - mockGetWorkspaceCustomToolByTitle, - mockUpsertCustomTools, -} = vi.hoisted(() => ({ - mockCheckRateLimit: vi.fn(), - mockResolveWorkspaceAccess: vi.fn(), - mockListWorkspaceCustomTools: vi.fn(), - mockGetWorkspaceCustomToolByTitle: vi.fn(), - mockUpsertCustomTools: vi.fn(), -})) +const { mocks, MockV2ApiKeyUnauthenticatedError } = vi.hoisted(() => { + class MockV2ApiKeyUnauthenticatedError extends Error {} + return { + mocks: { + authenticate: vi.fn(), + preauthRate: vi.fn(), + operationRate: vi.fn(), + gate: vi.fn(), + list: vi.fn(), + create: vi.fn(), + }, + MockV2ApiKeyUnauthenticatedError, + } +}) -vi.mock('@/app/api/v1/middleware', () => ({ - checkRateLimit: mockCheckRateLimit, - resolveWorkspaceAccess: mockResolveWorkspaceAccess, +vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => ({ + authenticateV2ApiKey: mocks.authenticate, + V2ApiKeyUnauthenticatedError: MockV2ApiKeyUnauthenticatedError, })) - -vi.mock('@/lib/workflows/custom-tools/operations', () => ({ - listWorkspaceCustomTools: mockListWorkspaceCustomTools, - getWorkspaceCustomToolByTitle: mockGetWorkspaceCustomToolByTitle, - upsertCustomTools: mockUpsertCustomTools, +vi.mock('@/lib/core/rate-limiter', () => ({ + RateLimiter: class { + checkRateLimitDirect = mocks.preauthRate + checkRateLimitDirectOrThrow = mocks.operationRate + }, + getRateLimit: vi.fn().mockReturnValue({ + maxTokens: 100, + refillRate: 100, + refillIntervalMs: 60_000, + }), })) - -vi.mock('@/app/api/v2/lib/gate', () => ({ - v2ApiGateError: vi.fn().mockResolvedValue(null), +vi.mock('@/lib/api/server/rate-limit-context', () => ({ + recordRateLimitSnapshot: vi.fn(), + getRateLimitHeaders: vi.fn().mockReturnValue(null), +})) +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: vi.fn().mockReturnValue('request-1'), + getClientIp: vi.fn().mockReturnValue('127.0.0.1'), +})) +vi.mock('@/app/api/v2/lib/gate', () => ({ v2ApiGateError: mocks.gate })) +vi.mock('@/lib/custom-tools/application/use-cases', () => ({ + listWorkspaceCustomToolsUseCase: { + operation: { id: 'custom_tools.list' }, + execute: mocks.list, + }, + createWorkspaceCustomToolUseCase: { + operation: { id: 'custom_tools.create' }, + execute: mocks.create, + }, })) import { GET, POST } from '@/app/api/v2/custom-tools/route' +const WORKSPACE_ID = 'workspace-1' +const PRINCIPAL = { kind: 'workspace_api_key' as const, workspaceId: WORKSPACE_ID, keyId: 'key-1' } +const AUTH = { + principal: PRINCIPAL, + rolloutUserId: 'owner-1', + rateLimitSubjectIds: ['workspace:workspace-1'] as const, + rateLimitSubscription: null, + keyType: 'workspace' as const, +} const RATE_LIMIT_OK = { allowed: true, - userId: 'user-1', - keyType: 'workspace', limit: 100, remaining: 99, - resetAt: new Date('2024-01-01T01:00:00Z'), -} - -const RATE_LIMIT_DENIED = { - allowed: false, - limit: 100, - remaining: 0, - resetAt: new Date('2024-01-01T01:00:00Z'), - retryAfterMs: 1000, + resetAt: new Date('2026-01-01T00:00:00Z'), + retryAfterMs: 0, } - const TOOL_SCHEMA = { type: 'function', function: { name: 'lookup_order', - description: 'Look up an order by id', - parameters: { - type: 'object', - properties: { orderId: { type: 'string' } }, - required: ['orderId'], - }, + parameters: { type: 'object', properties: {} }, }, } - -function buildTool(overrides: Record = {}) { - return { - id: 'tool_abc123', - workspaceId: 'workspace-1', - userId: 'user-1', - title: 'lookup_order', - schema: TOOL_SCHEMA, - code: 'return { ok: true }', - createdAt: new Date('2024-01-01T00:00:00Z'), - updatedAt: new Date('2024-01-02T00:00:00Z'), - ...overrides, - } -} - -/** What the route forwards for a bare `?workspaceId=` list. */ -const DEFAULT_LIST_ARGS = { - search: undefined, - sortBy: 'createdAt', - sortOrder: 'desc', -} - -const callList = (query: string) => - GET(new NextRequest(`http://localhost:3000/api/v2/custom-tools?${query}`)) - -function callCreate(body: unknown) { - return POST( - new NextRequest('http://localhost:3000/api/v2/custom-tools', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(body), - }) - ) -} - -const VALID_BODY = { - workspaceId: 'workspace-1', +const tool = { + id: 'tool-1', + workspaceId: WORKSPACE_ID, + userId: 'owner-1', title: 'lookup_order', schema: TOOL_SCHEMA, code: 'return { ok: true }', + createdAt: new Date('2026-01-01T00:00:00Z'), + updatedAt: new Date('2026-01-02T00:00:00Z'), } -describe('GET /api/v2/custom-tools', () => { - beforeEach(() => { - vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockListWorkspaceCustomTools.mockResolvedValue([buildTool()]) - }) - - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) - - const res = await callList('workspaceId=workspace-1') - - expect(res.status).toBe(404) - expect(mockListWorkspaceCustomTools).not.toHaveBeenCalled() - }) - - it('400s when workspaceId is missing', async () => { - const res = await callList('') - expect(res.status).toBe(400) - expect((await res.json()).error.code).toBe('BAD_REQUEST') - expect(mockListWorkspaceCustomTools).not.toHaveBeenCalled() +function request(method: 'GET' | 'POST', url: string, body?: unknown) { + return new NextRequest(`http://localhost:3000${url}`, { + method, + headers: { + 'x-api-key': 'key', + ...(body === undefined ? {} : { 'Content-Type': 'application/json' }), + }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), }) +} - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue({ - status: 403, - code: 'FORBIDDEN', - message: 'Access denied', +describe('/api/v2/custom-tools', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.authenticate.mockResolvedValue(AUTH) + mocks.preauthRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.operationRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.gate.mockResolvedValue(null) + mocks.list.mockResolvedValue({ tools: [tool] }) + mocks.create.mockResolvedValue({ tool }) + }) + + it('lists custom tools through the authorized application use case', async () => { + const response = await GET(request('GET', `/api/v2/custom-tools?workspaceId=${WORKSPACE_ID}`)) + + expect(response.status).toBe(200) + expect((await response.json()).data[0]).toMatchObject({ id: 'tool-1', title: 'lookup_order' }) + expect(mocks.list).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { + workspaceId: WORKSPACE_ID, + search: undefined, + sortBy: 'createdAt', + sortOrder: 'desc', + }, + request: expect.anything(), }) - const res = await callList('workspaceId=workspace-1') - expect(res.status).toBe(403) - expect(mockListWorkspaceCustomTools).not.toHaveBeenCalled() - }) - - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_DENIED) - const res = await callList('workspaceId=workspace-1') - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') + expect(mocks.operationRate).toHaveBeenCalledWith( + 'v2:custom_tools.list:workspace:workspace-1', + expect.objectContaining({ maxTokens: 100 }) + ) }) - it('returns the public tool shape in the cursor envelope, workspace-scoped', async () => { - const res = await callList('workspaceId=workspace-1') - const body = await res.json() + it('creates exactly one custom tool with the v2 source and status', async () => { + const response = await POST( + request('POST', '/api/v2/custom-tools', { + workspaceId: WORKSPACE_ID, + title: tool.title, + schema: TOOL_SCHEMA, + code: tool.code, + }) + ) - expect(res.status).toBe(200) - expect(body.nextCursor).toBeNull() - expect(body.data).toEqual([ - { - id: 'tool_abc123', - title: 'lookup_order', + expect(response.status).toBe(201) + expect((await response.json()).data.customTool.id).toBe('tool-1') + expect(mocks.create).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { + workspaceId: WORKSPACE_ID, + title: tool.title, schema: TOOL_SCHEMA, - code: 'return { ok: true }', - createdAt: '2024-01-01T00:00:00.000Z', - updatedAt: '2024-01-02T00:00:00.000Z', + code: tool.code, + source: 'api', }, - ]) - expect(mockListWorkspaceCustomTools).toHaveBeenCalledWith({ - workspaceId: 'workspace-1', - ...DEFAULT_LIST_ARGS, - }) - }) - it('400s on a sort field outside the enum instead of letting it reach the query', async () => { - const res = await callList(`workspaceId=workspace-1&sortBy=name);--`) - - expect(res.status).toBe(400) - expect((await res.json()).error.code).toBe('BAD_REQUEST') - }) - - it('400s on a sort direction outside the enum', async () => { - const res = await callList(`workspaceId=workspace-1&sortOrder=sideways`) - - expect(res.status).toBe(400) - }) - - it('400s on an empty search rather than treating it as unsearched', async () => { - const res = await callList(`workspaceId=workspace-1&search=`) - - expect(res.status).toBe(400) - }) - - it('forwards search and sort into the query and still terminates pagination', async () => { - const res = await callList(`workspaceId=workspace-1&search=report&sortBy=title&sortOrder=asc`) - - expect(res.status).toBe(200) - expect((await res.json()).nextCursor).toBeNull() - }) -}) - -describe('POST /api/v2/custom-tools', () => { - beforeEach(() => { - vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockGetWorkspaceCustomToolByTitle.mockResolvedValue(null) - mockUpsertCustomTools.mockResolvedValue([buildTool()]) - }) - - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) - - const res = await callCreate(VALID_BODY) - - expect(res.status).toBe(404) - expect(mockUpsertCustomTools).not.toHaveBeenCalled() - }) - - it('400s when the schema is not an OpenAI function declaration', async () => { - const res = await callCreate({ ...VALID_BODY, schema: { type: 'nonsense' } }) - expect(res.status).toBe(400) - expect((await res.json()).error.code).toBe('BAD_REQUEST') - expect(mockUpsertCustomTools).not.toHaveBeenCalled() - }) - - it('400s when the body carries an unknown field', async () => { - const res = await callCreate({ ...VALID_BODY, bogus: true }) - expect(res.status).toBe(400) - expect(mockUpsertCustomTools).not.toHaveBeenCalled() - }) - - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue({ - status: 403, - code: 'FORBIDDEN', - message: 'Access denied', + request: expect.anything(), }) - const res = await callCreate(VALID_BODY) - expect(res.status).toBe(403) - expect(mockUpsertCustomTools).not.toHaveBeenCalled() - }) - - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_DENIED) - const res = await callCreate(VALID_BODY) - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') }) - it('409s on a duplicate title instead of hitting the unique index', async () => { - mockGetWorkspaceCustomToolByTitle.mockResolvedValue(buildTool()) + it('authenticates before validating a malformed create body', async () => { + mocks.authenticate.mockRejectedValueOnce(new MockV2ApiKeyUnauthenticatedError()) - const res = await callCreate(VALID_BODY) + const response = await POST(request('POST', '/api/v2/custom-tools', {})) - expect(res.status).toBe(409) - expect((await res.json()).error.code).toBe('CONFLICT') - expect(mockUpsertCustomTools).not.toHaveBeenCalled() + expect(response.status).toBe(401) + expect(mocks.create).not.toHaveBeenCalled() }) - it('409s when a concurrent create loses the title race inside the lib', async () => { - mockUpsertCustomTools.mockRejectedValue( - new Error('A tool with the title "v2_smoke_tool" already exists in this workspace') + it('rejects invalid list sort fields before application execution', async () => { + const response = await GET( + request('GET', `/api/v2/custom-tools?workspaceId=${WORKSPACE_ID}&sortBy=invalid`) ) - const res = await callCreate(VALID_BODY) - - expect(res.status).toBe(409) - expect((await res.json()).error.code).toBe('CONFLICT') - }) - - it('409s when the unique index rejects the loser of a title race', async () => { - const pgError = Object.assign(new Error('duplicate key value violates unique constraint'), { - code: '23505', - }) - mockUpsertCustomTools.mockRejectedValue(pgError) - - const res = await callCreate(VALID_BODY) - - expect(res.status).toBe(409) - expect((await res.json()).error.code).toBe('CONFLICT') - }) - - it('creates the tool and returns 201 with the single tool', async () => { - const res = await callCreate(VALID_BODY) - const body = await res.json() - - expect(res.status).toBe(201) - expect(body.data.customTool).toMatchObject({ id: 'tool_abc123', title: 'lookup_order' }) - expect(mockUpsertCustomTools).toHaveBeenCalledWith( - expect.objectContaining({ - workspaceId: 'workspace-1', - userId: 'user-1', - tools: [{ title: 'lookup_order', schema: TOOL_SCHEMA, code: 'return { ok: true }' }], - }) - ) + expect(response.status).toBe(400) + expect(mocks.list).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/app/api/v2/custom-tools/route.ts b/apps/sim/app/api/v2/custom-tools/route.ts index 37899f53a00..688aeba700e 100644 --- a/apps/sim/app/api/v2/custom-tools/route.ts +++ b/apps/sim/app/api/v2/custom-tools/route.ts @@ -1,88 +1,43 @@ -import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { v2CreateCustomToolContract, v2ListCustomToolsContract, } from '@/lib/api/contracts/v2/custom-tools' import { - getWorkspaceCustomToolByTitle, - listWorkspaceCustomTools, - upsertCustomTools, -} from '@/lib/workflows/custom-tools/operations' -import { withPublicApiRouteHandler } from '@/app/api/public-api-route-handler' -import { resolveWorkspaceAccess } from '@/app/api/v1/middleware' -import { toV2CustomTool, v2CustomToolWriteError } from '@/app/api/v2/custom-tools/utils' -import { v2CursorList, v2Data, v2Error, v2WorkspaceAccessError } from '@/app/api/v2/lib/response' + defineV2JsonRoute, + v2ApiKeyAuth, + v2OrchestrationErrorPolicy, + v2RateLimits, +} from '@/lib/api/server/routes' +import { customToolOperations } from '@/lib/custom-tools/application/operations' +import { + createWorkspaceCustomToolUseCase, + listWorkspaceCustomToolsUseCase, +} from '@/lib/custom-tools/application/use-cases' +import { toV2CustomTool } from '@/app/api/v2/custom-tools/utils' export const dynamic = 'force-dynamic' export const revalidate = 0 /** GET /api/v2/custom-tools — List custom tools in a workspace. */ -export const GET = withPublicApiRouteHandler({ +export const GET = defineV2JsonRoute({ contract: v2ListCustomToolsContract, - rateLimitEndpoint: 'custom-tools', - handler: async ({ input, auth: { userId, rateLimit } }) => { - const { workspaceId, search, sortBy, sortOrder } = input.query - - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'read') - if (access) return v2WorkspaceAccessError(access) - - const rows = await listWorkspaceCustomTools({ workspaceId, search, sortBy, sortOrder }) - - // The per-workspace tool set is small and bounded → a single full page. - return v2CursorList(rows.map(toV2CustomTool), null, { rateLimit }) - }, + operation: customToolOperations.list, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ query }) => query, + useCase: listWorkspaceCustomToolsUseCase, + present: ({ tools }) => ({ data: tools.map(toV2CustomTool), nextCursor: null }), }) /** POST /api/v2/custom-tools — Create a custom tool. */ -export const POST = withPublicApiRouteHandler({ +export const POST = defineV2JsonRoute({ contract: v2CreateCustomToolContract, - rateLimitEndpoint: 'custom-tools', - handler: async ({ request, input, auth: { requestId, userId, rateLimit } }) => { - try { - const { workspaceId, title, schema, code } = input.body - - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'write') - if (access) return v2WorkspaceAccessError(access) - - /** - * Titles are unique per workspace and tools resolve by title at call time, - * so a collision is reported rather than surfacing as a unique-index 500. - */ - if (await getWorkspaceCustomToolByTitle({ workspaceId, title })) { - return v2Error( - 'CONFLICT', - `A custom tool titled "${title}" already exists in this workspace` - ) - } - - const tools = await upsertCustomTools({ - tools: [{ title, schema, code }], - workspaceId, - userId, - requestId, - }) - const created = tools.find((tool) => tool.title === title) - if (!created) { - throw new Error(`Custom tool "${title}" missing after a successful write`) - } - - recordAudit({ - workspaceId, - actorId: userId, - action: AuditAction.CUSTOM_TOOL_CREATED, - resourceType: AuditResourceType.CUSTOM_TOOL, - resourceId: created.id, - resourceName: created.title, - description: `Created custom tool "${created.title}" via API`, - request, - }) - - return v2Data({ customTool: toV2CustomTool(created) }, { rateLimit, status: 201 }) - } catch (error) { - const writeError = v2CustomToolWriteError(error) - if (writeError) return writeError - - throw error - } - }, + operation: customToolOperations.create, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ body }) => ({ ...body, source: 'api' as const }), + useCase: createWorkspaceCustomToolUseCase, + present: ({ tool }) => ({ data: { customTool: toV2CustomTool(tool) } }), }) diff --git a/apps/sim/app/api/v2/mcp-servers/[id]/route.test.ts b/apps/sim/app/api/v2/mcp-servers/[id]/route.test.ts index 1d5b93a53de..7aba50ef58a 100644 --- a/apps/sim/app/api/v2/mcp-servers/[id]/route.test.ts +++ b/apps/sim/app/api/v2/mcp-servers/[id]/route.test.ts @@ -1,350 +1,181 @@ /** * @vitest-environment node - * - * Public v2 MCP server detail: gate ordering, contract validation, workspace - * access, and the thin-wrapper mapping onto `lib/mcp/orchestration`. */ +import type { mcpServers } from '@sim/db/schema' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { McpServerRow } from '@/lib/mcp/queries' -const { - mockCheckRateLimit, - mockResolveWorkspaceAccess, - mockGetWorkspaceMcpServer, - mockPerformUpdateMcpServer, - mockPerformDeleteMcpServer, -} = vi.hoisted(() => ({ - mockCheckRateLimit: vi.fn(), - mockResolveWorkspaceAccess: vi.fn(), - mockGetWorkspaceMcpServer: vi.fn(), - mockPerformUpdateMcpServer: vi.fn(), - mockPerformDeleteMcpServer: vi.fn(), -})) +const { mocks, MockV2ApiKeyUnauthenticatedError } = vi.hoisted(() => { + class MockV2ApiKeyUnauthenticatedError extends Error {} + return { + mocks: { + authenticate: vi.fn(), + preauthRate: vi.fn(), + operationRate: vi.fn(), + gate: vi.fn(), + get: vi.fn(), + update: vi.fn(), + remove: vi.fn(), + capture: vi.fn(), + }, + MockV2ApiKeyUnauthenticatedError, + } +}) -vi.mock('@/app/api/v1/middleware', () => ({ - checkRateLimit: mockCheckRateLimit, - resolveWorkspaceAccess: mockResolveWorkspaceAccess, +vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => ({ + authenticateV2ApiKey: mocks.authenticate, + V2ApiKeyUnauthenticatedError: MockV2ApiKeyUnauthenticatedError, })) - -vi.mock('@/lib/mcp/queries', () => ({ - getWorkspaceMcpServer: mockGetWorkspaceMcpServer, +vi.mock('@/lib/core/rate-limiter', () => ({ + RateLimiter: class { + checkRateLimitDirect = mocks.preauthRate + checkRateLimitDirectOrThrow = mocks.operationRate + }, + getRateLimit: vi.fn().mockReturnValue({ + maxTokens: 100, + refillRate: 100, + refillIntervalMs: 60_000, + }), })) - -vi.mock('@/lib/mcp/orchestration', () => ({ - performUpdateMcpServer: mockPerformUpdateMcpServer, - performDeleteMcpServer: mockPerformDeleteMcpServer, +vi.mock('@/lib/api/server/rate-limit-context', () => ({ + recordRateLimitSnapshot: vi.fn(), + getRateLimitHeaders: vi.fn().mockReturnValue(null), })) - -vi.mock('@/app/api/v2/lib/gate', () => ({ - v2ApiGateError: vi.fn().mockResolvedValue(null), +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: vi.fn().mockReturnValue('request-1'), + getClientIp: vi.fn().mockReturnValue('127.0.0.1'), +})) +vi.mock('@/app/api/v2/lib/gate', () => ({ v2ApiGateError: mocks.gate })) +vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mocks.capture })) +vi.mock('@/lib/mcp/application/use-cases', () => ({ + getMcpServerUseCase: { operation: { id: 'mcp_servers.read' }, execute: mocks.get }, + updateMcpServerUseCase: { operation: { id: 'mcp_servers.update' }, execute: mocks.update }, + deleteMcpServerUseCase: { operation: { id: 'mcp_servers.delete' }, execute: mocks.remove }, })) import { DELETE, GET, PATCH } from '@/app/api/v2/mcp-servers/[id]/route' +type McpServerRow = typeof mcpServers.$inferSelect +const WORKSPACE_ID = 'workspace-1' +const PRINCIPAL = { kind: 'workspace_api_key' as const, workspaceId: WORKSPACE_ID, keyId: 'key-1' } +const AUTH = { + principal: PRINCIPAL, + rolloutUserId: 'owner-1', + rateLimitSubjectIds: ['workspace:workspace-1'] as const, + rateLimitSubscription: null, + keyType: 'workspace' as const, +} const RATE_LIMIT_OK = { allowed: true, - userId: 'user-1', - keyType: 'workspace', limit: 100, remaining: 99, - resetAt: new Date('2024-01-01T01:00:00Z'), -} - -const RATE_LIMIT_DENIED = { - allowed: false, - limit: 100, - remaining: 0, - resetAt: new Date('2024-01-01T01:00:00Z'), - retryAfterMs: 1000, + resetAt: new Date('2026-01-01T00:00:00Z'), + retryAfterMs: 0, } - -const ACCESS_DENIED = { status: 403, code: 'FORBIDDEN', message: 'Access denied' } - -function buildRow(overrides: Partial = {}): McpServerRow { - return { - id: 'mcp-abc12345', - workspaceId: 'workspace-1', - createdBy: 'user-1', - name: 'Docs server', - description: null, - transport: 'streamable-http', - url: 'https://mcp.example.com/sse', - authType: 'headers', - oauthClientId: null, - oauthClientSecret: 'encrypted-secret', - headers: { Authorization: 'Bearer super-secret-token' }, - timeout: 30000, - retries: 3, - enabled: true, - lastConnected: null, - connectionStatus: 'disconnected', - lastError: null, - statusConfig: {}, - toolCount: 0, - lastToolsRefresh: null, - totalRequests: 0, - lastUsed: null, - deletedAt: null, - createdAt: new Date('2024-01-01T00:00:00Z'), - updatedAt: new Date('2024-01-02T00:00:00Z'), - ...overrides, - } as McpServerRow -} - -const routeContext = () => ({ params: Promise.resolve({ id: 'mcp-abc12345' }) }) - -const url = (query = 'workspaceId=workspace-1') => - `http://localhost:3000/api/v2/mcp-servers/mcp-abc12345?${query}` - -function callGet(query?: string) { - return GET(new NextRequest(url(query)), routeContext()) -} - -function callPatch(body: unknown) { - return PATCH( - new NextRequest('http://localhost:3000/api/v2/mcp-servers/mcp-abc12345', { - method: 'PATCH', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(body), - }), - routeContext() +const server = { + id: 'mcp-server-1', + workspaceId: WORKSPACE_ID, + createdBy: 'owner-1', + name: 'Docs server', + description: null, + transport: 'streamable-http', + url: 'https://mcp.example.com/sse', + authType: 'headers', + oauthClientId: null, + oauthClientSecret: null, + headers: {}, + timeout: 30_000, + retries: 3, + enabled: true, + lastConnected: null, + connectionStatus: 'connected', + lastError: null, + statusConfig: {}, + toolCount: 0, + lastToolsRefresh: null, + totalRequests: 0, + lastUsed: null, + deletedAt: null, + createdAt: new Date('2026-01-01T00:00:00Z'), + updatedAt: new Date('2026-01-02T00:00:00Z'), +} as McpServerRow +const context = { params: Promise.resolve({ id: server.id }) } + +function request(method: 'GET' | 'PATCH' | 'DELETE', body?: unknown) { + return new NextRequest( + `http://localhost:3000/api/v2/mcp-servers/${server.id}?workspaceId=${WORKSPACE_ID}`, + { + method, + headers: { + 'x-api-key': 'key', + ...(body === undefined ? {} : { 'Content-Type': 'application/json' }), + }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + } ) } -function callDelete(query?: string) { - return DELETE(new NextRequest(url(query), { method: 'DELETE' }), routeContext()) -} - -describe('GET /api/v2/mcp-servers/[id]', () => { - beforeEach(() => { - vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockGetWorkspaceMcpServer.mockResolvedValue(buildRow()) - }) - - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) - - const res = await callGet() - - expect(res.status).toBe(404) - expect((await res.json()).error.code).toBe('NOT_FOUND') - expect(mockGetWorkspaceMcpServer).not.toHaveBeenCalled() - }) - - it('400s when workspaceId is missing', async () => { - const res = await callGet('') - expect(res.status).toBe(400) - expect((await res.json()).error.code).toBe('BAD_REQUEST') - expect(mockGetWorkspaceMcpServer).not.toHaveBeenCalled() - }) - - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue(ACCESS_DENIED) - const res = await callGet() - expect(res.status).toBe(403) - expect(mockGetWorkspaceMcpServer).not.toHaveBeenCalled() - }) - - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_DENIED) - const res = await callGet() - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') - }) - - it('404s when the server does not exist in the workspace', async () => { - mockGetWorkspaceMcpServer.mockResolvedValue(null) - const res = await callGet() - expect(res.status).toBe(404) - expect((await res.json()).error.code).toBe('NOT_FOUND') - }) - - it('returns the public server shape without header values', async () => { - const res = await callGet() - const body = await res.json() - - expect(res.status).toBe(200) - expect(body.data.mcpServer).toMatchObject({ - id: 'mcp-abc12345', - hasHeaders: true, - headerNames: ['Authorization'], - hasOauthClientSecret: true, - }) - expect(JSON.stringify(body)).not.toContain('super-secret-token') - expect(JSON.stringify(body)).not.toContain('encrypted-secret') - expect(mockGetWorkspaceMcpServer).toHaveBeenCalledWith({ - workspaceId: 'workspace-1', - serverId: 'mcp-abc12345', - }) - }) -}) - -describe('PATCH /api/v2/mcp-servers/[id]', () => { +describe('/api/v2/mcp-servers/[id]', () => { beforeEach(() => { vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockPerformUpdateMcpServer.mockResolvedValue({ success: true, server: buildRow() }) - }) - - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) - - const res = await callPatch({ workspaceId: 'workspace-1', name: 'Renamed' }) - - expect(res.status).toBe(404) - expect(mockPerformUpdateMcpServer).not.toHaveBeenCalled() - }) - - it('400s when the body has an unknown field', async () => { - const res = await callPatch({ workspaceId: 'workspace-1', bogus: true }) - expect(res.status).toBe(400) - expect(mockPerformUpdateMcpServer).not.toHaveBeenCalled() - }) - - it('400s when the url carries an environment-variable template', async () => { - const res = await callPatch({ workspaceId: 'workspace-1', url: 'https://{{HOST}}/sse' }) - expect(res.status).toBe(400) - expect(mockPerformUpdateMcpServer).not.toHaveBeenCalled() - }) - - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue(ACCESS_DENIED) - const res = await callPatch({ workspaceId: 'workspace-1', name: 'Renamed' }) - expect(res.status).toBe(403) - expect(mockPerformUpdateMcpServer).not.toHaveBeenCalled() - }) - - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_DENIED) - const res = await callPatch({ workspaceId: 'workspace-1', name: 'Renamed' }) - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') - }) - - it('maps a not_found orchestration failure to 404', async () => { - mockPerformUpdateMcpServer.mockResolvedValue({ - success: false, - error: 'Server not found', - errorCode: 'not_found', + mocks.authenticate.mockResolvedValue(AUTH) + mocks.preauthRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.operationRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.gate.mockResolvedValue(null) + mocks.get.mockResolvedValue({ server }) + mocks.update.mockResolvedValue({ server }) + mocks.remove.mockResolvedValue({ server }) + }) + + it('gets an MCP server through the semantic read operation', async () => { + const response = await GET(request('GET'), context) + + expect(response.status).toBe(200) + expect(mocks.get).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { workspaceId: WORKSPACE_ID, serverId: server.id }, + request: expect.anything(), }) - const res = await callPatch({ workspaceId: 'workspace-1', name: 'Renamed' }) - expect(res.status).toBe(404) - expect((await res.json()).error.code).toBe('NOT_FOUND') }) - it('400s when the url is changed, since the id is derived from it', async () => { - mockGetWorkspaceMcpServer.mockResolvedValue(buildRow()) + it('updates an MCP server through the strict semantic update operation', async () => { + const response = await PATCH( + request('PATCH', { workspaceId: WORKSPACE_ID, name: 'New docs' }), + context + ) - const res = await callPatch({ - workspaceId: 'workspace-1', - url: 'https://different.example.com/sse', + expect(response.status).toBe(200) + expect(mocks.update).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { + workspaceId: WORKSPACE_ID, + serverId: server.id, + name: 'New docs', + source: 'api', + }, + request: expect.anything(), }) - - expect(res.status).toBe(400) - expect((await res.json()).error.message).toContain('url cannot be changed') - expect(mockPerformUpdateMcpServer).not.toHaveBeenCalled() }) - it('allows a url that matches the stored one, so a full-object PATCH still works', async () => { - mockGetWorkspaceMcpServer.mockResolvedValue(buildRow()) + it('deletes an MCP server without product analytics for workspace keys', async () => { + const response = await DELETE(request('DELETE'), context) - const res = await callPatch({ - workspaceId: 'workspace-1', - url: 'https://mcp.example.com/sse', - enabled: false, + expect(response.status).toBe(200) + expect(await response.json()).toEqual({ data: { id: server.id, deleted: true } }) + expect(mocks.remove).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { workspaceId: WORKSPACE_ID, serverId: server.id, source: 'api' }, + request: expect.anything(), }) - - expect(res.status).toBe(200) - expect(mockPerformUpdateMcpServer).toHaveBeenCalled() + expect(mocks.capture).not.toHaveBeenCalled() }) - it('updates the server and returns the public shape', async () => { - const res = await callPatch({ workspaceId: 'workspace-1', name: 'Renamed', enabled: false }) - const body = await res.json() + it('authenticates before parsing an invalid update body', async () => { + mocks.authenticate.mockRejectedValueOnce(new MockV2ApiKeyUnauthenticatedError()) - expect(res.status).toBe(200) - expect(body.data.mcpServer.id).toBe('mcp-abc12345') - expect(body.data.mcpServer.headers).toBeUndefined() - expect(mockPerformUpdateMcpServer).toHaveBeenCalledWith( - expect.objectContaining({ - workspaceId: 'workspace-1', - userId: 'user-1', - serverId: 'mcp-abc12345', - name: 'Renamed', - enabled: false, - }) - ) - }) -}) - -describe('DELETE /api/v2/mcp-servers/[id]', () => { - beforeEach(() => { - vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockPerformDeleteMcpServer.mockResolvedValue({ success: true, server: buildRow() }) - }) - - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) - - const res = await callDelete() - - expect(res.status).toBe(404) - expect(mockPerformDeleteMcpServer).not.toHaveBeenCalled() - }) + const response = await PATCH(request('PATCH', {}), context) - it('400s when workspaceId is missing', async () => { - const res = await callDelete('') - expect(res.status).toBe(400) - expect(mockPerformDeleteMcpServer).not.toHaveBeenCalled() - }) - - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue(ACCESS_DENIED) - const res = await callDelete() - expect(res.status).toBe(403) - expect(mockPerformDeleteMcpServer).not.toHaveBeenCalled() - }) - - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_DENIED) - const res = await callDelete() - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') - }) - - it('maps a not_found orchestration failure to 404', async () => { - mockPerformDeleteMcpServer.mockResolvedValue({ - success: false, - error: 'Server not found', - errorCode: 'not_found', - }) - const res = await callDelete() - expect(res.status).toBe(404) - expect((await res.json()).error.code).toBe('NOT_FOUND') - }) - - it('deletes the server and acknowledges the id', async () => { - const res = await callDelete() - expect(res.status).toBe(200) - expect(await res.json()).toEqual({ data: { id: 'mcp-abc12345', deleted: true } }) - expect(mockPerformDeleteMcpServer).toHaveBeenCalledWith( - expect.objectContaining({ - workspaceId: 'workspace-1', - userId: 'user-1', - serverId: 'mcp-abc12345', - }) - ) + expect(response.status).toBe(401) + expect(mocks.update).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/app/api/v2/mcp-servers/[id]/route.ts b/apps/sim/app/api/v2/mcp-servers/[id]/route.ts index 4e61d698811..3fcb02503bf 100644 --- a/apps/sim/app/api/v2/mcp-servers/[id]/route.ts +++ b/apps/sim/app/api/v2/mcp-servers/[id]/route.ts @@ -3,111 +3,72 @@ import { v2GetMcpServerContract, v2UpdateMcpServerContract, } from '@/lib/api/contracts/v2/mcp-servers' -import { performDeleteMcpServer, performUpdateMcpServer } from '@/lib/mcp/orchestration' -import { getWorkspaceMcpServer } from '@/lib/mcp/queries' -import { withPublicApiRouteHandler } from '@/app/api/public-api-route-handler' -import { resolveWorkspaceAccess } from '@/app/api/v1/middleware' -import { v2Data, v2Error, v2WorkspaceAccessError } from '@/app/api/v2/lib/response' -import { toV2McpServer, v2McpOrchestrationError } from '@/app/api/v2/mcp-servers/utils' +import { + defineV2JsonRoute, + v2ApiKeyAuth, + v2OrchestrationErrorPolicy, + v2RateLimits, +} from '@/lib/api/server/routes' +import { mcpServerOperations } from '@/lib/mcp/application/operations' +import { + deleteMcpServerUseCase, + getMcpServerUseCase, + updateMcpServerUseCase, +} from '@/lib/mcp/application/use-cases' +import { captureServerEvent } from '@/lib/posthog/server' +import { toV2McpServer } from '@/app/api/v2/mcp-servers/utils' export const dynamic = 'force-dynamic' export const revalidate = 0 -interface RouteContext { - params: Promise<{ id: string }> -} - /** GET /api/v2/mcp-servers/[id] — Fetch a single MCP server. */ -export const GET = withPublicApiRouteHandler({ +export const GET = defineV2JsonRoute({ contract: v2GetMcpServerContract, - rateLimitEndpoint: 'mcp-server-detail', - handler: async ({ input, auth: { userId, rateLimit } }) => { - const { id } = input.params - const { workspaceId } = input.query - - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'read') - if (access) return v2WorkspaceAccessError(access) - - const server = await getWorkspaceMcpServer({ workspaceId, serverId: id }) - if (!server) return v2Error('NOT_FOUND', 'MCP server not found') - - return v2Data({ mcpServer: toV2McpServer(server) }, { rateLimit }) - }, + operation: mcpServerOperations.read, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ params, query }) => ({ workspaceId: query.workspaceId, serverId: params.id }), + useCase: getMcpServerUseCase, + present: ({ server }) => ({ data: { mcpServer: toV2McpServer(server) } }), }) /** PATCH /api/v2/mcp-servers/[id] — Update an MCP server's configuration. */ -export const PATCH = withPublicApiRouteHandler({ +export const PATCH = defineV2JsonRoute({ contract: v2UpdateMcpServerContract, - rateLimitEndpoint: 'mcp-server-detail', - handler: async ({ request, input, auth: { userId, rateLimit } }) => { - const { id } = input.params - const { workspaceId, ...body } = input.body - - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'write') - if (access) return v2WorkspaceAccessError(access) - - /** - * A server's id is the hash of its workspace + URL, and this surface promises - * that identity. The lib will happily move `url` while the id keeps hashing - * the old one, which both breaks that promise and defeats the duplicate - * check on create (id-keyed, so it would not see the moved URL) — leaving two - * rows on one URL. Re-pointing a server at a different URL is a new server. - */ - if (body.url !== undefined) { - const current = await getWorkspaceMcpServer({ workspaceId, serverId: id }) - if (!current) return v2Error('NOT_FOUND', 'MCP server not found') - if (current.url !== body.url) { - return v2Error( - 'BAD_REQUEST', - 'url cannot be changed: an MCP server’s id is derived from its URL. Delete this server and create one at the new URL.' - ) - } - } - - const result = await performUpdateMcpServer({ - workspaceId, - userId, - serverId: id, - name: body.name, - description: body.description, - transport: body.transport, - url: body.url, - headers: body.headers, - timeout: body.timeout, - retries: body.retries, - enabled: body.enabled, - authType: body.authType, - oauthClientId: body.oauthClientId ?? null, - oauthClientIdProvided: body.oauthClientId !== undefined, - oauthClientSecret: body.oauthClientSecret, - oauthClientSecretProvided: body.oauthClientSecret !== undefined, - request, - }) - - if (!result.success || !result.server) { - return v2McpOrchestrationError(result.errorCode, result.error ?? 'Failed to update server') - } - - return v2Data({ mcpServer: toV2McpServer(result.server) }, { rateLimit }) - }, + operation: mcpServerOperations.update, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ params, body }) => ({ ...body, serverId: params.id, source: 'api' as const }), + useCase: updateMcpServerUseCase, + present: ({ server }) => ({ data: { mcpServer: toV2McpServer(server) } }), }) /** DELETE /api/v2/mcp-servers/[id] — Remove an MCP server from the workspace. */ -export const DELETE = withPublicApiRouteHandler({ +export const DELETE = defineV2JsonRoute({ contract: v2DeleteMcpServerContract, - rateLimitEndpoint: 'mcp-server-detail', - handler: async ({ request, input, auth: { userId, rateLimit } }) => { - const { id } = input.params - const { workspaceId } = input.query - - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'write') - if (access) return v2WorkspaceAccessError(access) - - const result = await performDeleteMcpServer({ workspaceId, userId, serverId: id, request }) - if (!result.success) { - return v2McpOrchestrationError(result.errorCode, result.error ?? 'Failed to delete server') - } - - return v2Data({ id, deleted: true as const }, { rateLimit }) + operation: mcpServerOperations.delete, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ params, query }) => ({ + workspaceId: query.workspaceId, + serverId: params.id, + source: 'api' as const, + }), + useCase: deleteMcpServerUseCase, + onSuccess: ({ principal, input, result }) => { + if (principal.kind !== 'personal_api_key') return + captureServerEvent( + principal.userId, + 'mcp_server_disconnected', + { + workspace_id: input.workspaceId, + server_name: result.server.name, + }, + { groups: { workspace: input.workspaceId } } + ) }, + present: ({ server }) => ({ data: { id: server.id, deleted: true as const } }), }) diff --git a/apps/sim/app/api/v2/mcp-servers/route.test.ts b/apps/sim/app/api/v2/mcp-servers/route.test.ts index cb0df3ac683..cf158e4cec2 100644 --- a/apps/sim/app/api/v2/mcp-servers/route.test.ts +++ b/apps/sim/app/api/v2/mcp-servers/route.test.ts @@ -1,365 +1,197 @@ /** * @vitest-environment node - * - * Public v2 MCP servers list/create: gate ordering, contract validation, the - * write-only `headers` projection, and the 409-on-duplicate-URL departure from - * the internal upsert. */ +import type { mcpServers } from '@sim/db/schema' import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -import type { McpServerRow } from '@/lib/mcp/queries' -const { - mockCheckRateLimit, - mockResolveWorkspaceAccess, - mockListWorkspaceMcpServers, - mockGetWorkspaceMcpServer, - mockGetMcpServerIdState, - mockPerformCreateMcpServer, -} = vi.hoisted(() => ({ - mockCheckRateLimit: vi.fn(), - mockResolveWorkspaceAccess: vi.fn(), - mockListWorkspaceMcpServers: vi.fn(), - mockGetWorkspaceMcpServer: vi.fn(), - mockGetMcpServerIdState: vi.fn(), - mockPerformCreateMcpServer: vi.fn(), -})) +const { mocks, MockV2ApiKeyUnauthenticatedError } = vi.hoisted(() => { + class MockV2ApiKeyUnauthenticatedError extends Error {} + return { + mocks: { + authenticate: vi.fn(), + preauthRate: vi.fn(), + operationRate: vi.fn(), + gate: vi.fn(), + list: vi.fn(), + create: vi.fn(), + capture: vi.fn(), + }, + MockV2ApiKeyUnauthenticatedError, + } +}) -vi.mock('@/app/api/v1/middleware', () => ({ - checkRateLimit: mockCheckRateLimit, - resolveWorkspaceAccess: mockResolveWorkspaceAccess, +vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => ({ + authenticateV2ApiKey: mocks.authenticate, + V2ApiKeyUnauthenticatedError: MockV2ApiKeyUnauthenticatedError, })) - -vi.mock('@/lib/mcp/queries', () => ({ - listWorkspaceMcpServers: mockListWorkspaceMcpServers, - getWorkspaceMcpServer: mockGetWorkspaceMcpServer, - getMcpServerIdState: mockGetMcpServerIdState, +vi.mock('@/lib/core/rate-limiter', () => ({ + RateLimiter: class { + checkRateLimitDirect = mocks.preauthRate + checkRateLimitDirectOrThrow = mocks.operationRate + }, + getRateLimit: vi.fn().mockReturnValue({ + maxTokens: 100, + refillRate: 100, + refillIntervalMs: 60_000, + }), })) - -vi.mock('@/lib/mcp/orchestration', () => ({ - performCreateMcpServer: mockPerformCreateMcpServer, +vi.mock('@/lib/api/server/rate-limit-context', () => ({ + recordRateLimitSnapshot: vi.fn(), + getRateLimitHeaders: vi.fn().mockReturnValue(null), })) - -vi.mock('@/app/api/v2/lib/gate', () => ({ - v2ApiGateError: vi.fn().mockResolvedValue(null), +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: vi.fn().mockReturnValue('request-1'), + getClientIp: vi.fn().mockReturnValue('127.0.0.1'), +})) +vi.mock('@/app/api/v2/lib/gate', () => ({ v2ApiGateError: mocks.gate })) +vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mocks.capture })) +vi.mock('@/lib/mcp/application/use-cases', () => ({ + listMcpServersUseCase: { operation: { id: 'mcp_servers.list' }, execute: mocks.list }, + createMcpServerUseCase: { operation: { id: 'mcp_servers.create' }, execute: mocks.create }, })) import { GET, POST } from '@/app/api/v2/mcp-servers/route' +type McpServerRow = typeof mcpServers.$inferSelect +const WORKSPACE_ID = 'workspace-1' +const PRINCIPAL = { kind: 'workspace_api_key' as const, workspaceId: WORKSPACE_ID, keyId: 'key-1' } +const AUTH = { + principal: PRINCIPAL, + rolloutUserId: 'owner-1', + rateLimitSubjectIds: ['workspace:workspace-1'] as const, + rateLimitSubscription: null, + keyType: 'workspace' as const, +} const RATE_LIMIT_OK = { allowed: true, - userId: 'user-1', - keyType: 'workspace', limit: 100, remaining: 99, - resetAt: new Date('2024-01-01T01:00:00Z'), -} - -function buildRow(overrides: Partial = {}): McpServerRow { - return { - id: 'mcp-abc12345', - workspaceId: 'workspace-1', - createdBy: 'user-1', - name: 'Docs server', - description: 'Internal docs', - transport: 'streamable-http', - url: 'https://mcp.example.com/sse', - authType: 'headers', - oauthClientId: null, - oauthClientSecret: null, - headers: { Authorization: 'Bearer super-secret-token' }, - timeout: 30000, - retries: 3, - enabled: true, - lastConnected: new Date('2024-01-02T00:00:00Z'), - connectionStatus: 'connected', - lastError: null, - statusConfig: {}, - toolCount: 4, - lastToolsRefresh: new Date('2024-01-02T00:00:00Z'), - totalRequests: 0, - lastUsed: null, - deletedAt: null, - createdAt: new Date('2024-01-01T00:00:00Z'), - updatedAt: new Date('2024-01-02T00:00:00Z'), - ...overrides, - } as McpServerRow + resetAt: new Date('2026-01-01T00:00:00Z'), + retryAfterMs: 0, } - -function callList(query: string) { - return GET(new NextRequest(`http://localhost:3000/api/v2/mcp-servers?${query}`)) -} - -function callCreate(body: unknown) { - return POST( - new NextRequest('http://localhost:3000/api/v2/mcp-servers', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(body), - }) - ) -} - -/** What the route forwards for a bare `?workspaceId=` list. */ -const DEFAULT_LIST_ARGS = { - search: undefined, - sortBy: 'createdAt', - sortOrder: 'desc', -} - -const VALID_BODY = { - workspaceId: 'workspace-1', +const server = { + id: 'mcp-server-1', + workspaceId: WORKSPACE_ID, + createdBy: 'owner-1', name: 'Docs server', + description: 'Internal docs', + transport: 'streamable-http', url: 'https://mcp.example.com/sse', + authType: 'headers', + oauthClientId: null, + oauthClientSecret: null, + headers: { Authorization: 'secret' }, + timeout: 30_000, + retries: 3, + enabled: true, + lastConnected: new Date('2026-01-02T00:00:00Z'), + connectionStatus: 'connected', + lastError: null, + statusConfig: {}, + toolCount: 4, + lastToolsRefresh: new Date('2026-01-02T00:00:00Z'), + totalRequests: 0, + lastUsed: null, + deletedAt: null, + createdAt: new Date('2026-01-01T00:00:00Z'), + updatedAt: new Date('2026-01-02T00:00:00Z'), +} as McpServerRow + +function request(method: 'GET' | 'POST', url: string, body?: unknown) { + return new NextRequest(`http://localhost:3000${url}`, { + method, + headers: { + 'x-api-key': 'key', + ...(body === undefined ? {} : { 'Content-Type': 'application/json' }), + }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + }) } -describe('GET /api/v2/mcp-servers', () => { +describe('/api/v2/mcp-servers', () => { beforeEach(() => { vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockListWorkspaceMcpServers.mockResolvedValue([buildRow()]) - }) - - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) - - const res = await callList('workspaceId=workspace-1') - - expect(res.status).toBe(404) - expect((await res.json()).error.code).toBe('NOT_FOUND') - expect(mockListWorkspaceMcpServers).not.toHaveBeenCalled() - }) - - it('400s when workspaceId is missing', async () => { - const res = await callList('') - expect(res.status).toBe(400) - expect((await res.json()).error.code).toBe('BAD_REQUEST') - expect(mockListWorkspaceMcpServers).not.toHaveBeenCalled() - }) - - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue({ - status: 403, - code: 'FORBIDDEN', - message: 'Access denied', - }) - const res = await callList('workspaceId=workspace-1') - expect(res.status).toBe(403) - expect((await res.json()).error).toMatchObject({ code: 'FORBIDDEN', message: 'Access denied' }) - expect(mockListWorkspaceMcpServers).not.toHaveBeenCalled() - }) - - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue({ - allowed: false, - limit: 100, - remaining: 0, - resetAt: new Date('2024-01-01T01:00:00Z'), - retryAfterMs: 1000, - }) - const res = await callList('workspaceId=workspace-1') - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') - }) - - it('returns the public server shape in the cursor envelope', async () => { - const res = await callList('workspaceId=workspace-1') - const body = await res.json() - - expect(res.status).toBe(200) - expect(body.nextCursor).toBeNull() - expect(body.data).toEqual([ - { - id: 'mcp-abc12345', - name: 'Docs server', - description: 'Internal docs', - transport: 'streamable-http', - authType: 'headers', - url: 'https://mcp.example.com/sse', - timeout: 30000, - retries: 3, - enabled: true, - connectionStatus: 'connected', - lastError: null, - toolCount: 4, - lastToolsRefresh: '2024-01-02T00:00:00.000Z', - lastConnected: '2024-01-02T00:00:00.000Z', - createdAt: '2024-01-01T00:00:00.000Z', - updatedAt: '2024-01-02T00:00:00.000Z', - hasHeaders: true, - headerNames: ['Authorization'], - hasOauthClientSecret: false, + mocks.authenticate.mockResolvedValue(AUTH) + mocks.preauthRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.operationRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.gate.mockResolvedValue(null) + mocks.list.mockResolvedValue({ servers: [server] }) + mocks.create.mockResolvedValue({ server, updated: false }) + }) + + it('lists MCP servers without exposing secret header values', async () => { + const response = await GET(request('GET', `/api/v2/mcp-servers?workspaceId=${WORKSPACE_ID}`)) + const body = await response.json() + + expect(response.status).toBe(200) + expect(body.data[0]).toMatchObject({ id: server.id, hasHeaders: true }) + expect(JSON.stringify(body)).not.toContain('secret') + expect(mocks.list).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { + workspaceId: WORKSPACE_ID, + search: undefined, + sortBy: 'createdAt', + sortOrder: 'desc', }, - ]) - expect(mockListWorkspaceMcpServers).toHaveBeenCalledWith({ - workspaceId: 'workspace-1', - ...DEFAULT_LIST_ARGS, - }) - }) - - it('never returns configured header values', async () => { - const res = await callList('workspaceId=workspace-1') - const raw = JSON.stringify(await res.json()) - - expect(raw).not.toContain('super-secret-token') - expect(raw).not.toContain('"headers":') - }) - it('400s on a sort field outside the enum instead of letting it reach the query', async () => { - const res = await callList(`workspaceId=workspace-1&sortBy=name);--`) - - expect(res.status).toBe(400) - expect((await res.json()).error.code).toBe('BAD_REQUEST') - }) - - it('400s on a sort direction outside the enum', async () => { - const res = await callList(`workspaceId=workspace-1&sortOrder=sideways`) - - expect(res.status).toBe(400) - }) - - it('400s on an empty search rather than treating it as unsearched', async () => { - const res = await callList(`workspaceId=workspace-1&search=`) - - expect(res.status).toBe(400) - }) - - it('forwards search and sort into the query and still terminates pagination', async () => { - const res = await callList(`workspaceId=workspace-1&search=report&sortBy=name&sortOrder=asc`) - - expect(res.status).toBe(200) - expect((await res.json()).nextCursor).toBeNull() - }) -}) - -describe('POST /api/v2/mcp-servers', () => { - beforeEach(() => { - vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockGetMcpServerIdState.mockResolvedValue(null) - mockPerformCreateMcpServer.mockResolvedValue({ - success: true, - serverId: 'mcp-abc12345', - updated: false, + request: expect.anything(), }) - mockGetWorkspaceMcpServer.mockResolvedValue(buildRow()) - }) - - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) - - const res = await callCreate(VALID_BODY) - - expect(res.status).toBe(404) - expect(mockPerformCreateMcpServer).not.toHaveBeenCalled() - }) - - it('400s when the body is missing a required field', async () => { - const res = await callCreate({ workspaceId: 'workspace-1', name: 'Docs server' }) - expect(res.status).toBe(400) - expect((await res.json()).error.code).toBe('BAD_REQUEST') - expect(mockPerformCreateMcpServer).not.toHaveBeenCalled() - }) - - it('400s when the url carries an environment-variable template', async () => { - const res = await callCreate({ ...VALID_BODY, url: 'https://{{MCP_HOST}}/sse' }) - expect(res.status).toBe(400) - expect((await res.json()).error.message).toContain('{{ENV_VAR}}') - expect(mockPerformCreateMcpServer).not.toHaveBeenCalled() }) - it('400s when the url is not an absolute http(s) URL', async () => { - const res = await callCreate({ ...VALID_BODY, url: 'file:///etc/passwd' }) - expect(res.status).toBe(400) - expect(mockPerformCreateMcpServer).not.toHaveBeenCalled() - }) - - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue({ - status: 403, - code: 'FORBIDDEN', - message: 'Access denied', - }) - const res = await callCreate(VALID_BODY) - expect(res.status).toBe(403) - expect(mockPerformCreateMcpServer).not.toHaveBeenCalled() - }) + it('strictly creates an MCP server with the v2 source and status', async () => { + const response = await POST( + request('POST', '/api/v2/mcp-servers', { + workspaceId: WORKSPACE_ID, + name: server.name, + url: server.url, + }) + ) - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue({ - allowed: false, - limit: 100, - remaining: 0, - resetAt: new Date('2024-01-01T01:00:00Z'), - retryAfterMs: 1000, + expect(response.status).toBe(201) + expect(mocks.create).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { + workspaceId: WORKSPACE_ID, + name: server.name, + url: server.url, + source: 'api', + }, + request: expect.anything(), }) - const res = await callCreate(VALID_BODY) - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') - }) - - it('409s on a duplicate URL without letting the lib upsert', async () => { - mockGetMcpServerIdState.mockResolvedValue({ deleted: false }) - - const res = await callCreate(VALID_BODY) - - expect(res.status).toBe(409) - expect((await res.json()).error.code).toBe('CONFLICT') - expect(mockPerformCreateMcpServer).not.toHaveBeenCalled() + expect(mocks.capture).not.toHaveBeenCalled() }) - it('409s when a concurrent create made the lib upsert instead of insert', async () => { - mockPerformCreateMcpServer.mockResolvedValue({ - success: true, - serverId: 'mcp-abc12345', - updated: true, + it('keeps product analytics surface-specific for personal API keys', async () => { + mocks.authenticate.mockResolvedValueOnce({ + ...AUTH, + principal: { kind: 'personal_api_key', userId: 'user-1', keyId: 'key-personal' }, + keyType: 'personal', }) - const res = await callCreate(VALID_BODY) + const response = await POST( + request('POST', '/api/v2/mcp-servers', { + workspaceId: WORKSPACE_ID, + name: server.name, + url: server.url, + }) + ) - expect(res.status).toBe(409) - expect((await res.json()).error.code).toBe('CONFLICT') + expect(response.status).toBe(201) + expect(mocks.capture).toHaveBeenCalledWith( + 'user-1', + 'mcp_server_connected', + expect.objectContaining({ workspace_id: WORKSPACE_ID }), + expect.anything() + ) }) - it('revives a soft-deleted URL instead of stranding it behind a 409', async () => { - mockGetMcpServerIdState.mockResolvedValue({ deleted: true }) - mockPerformCreateMcpServer.mockResolvedValue({ - success: true, - serverId: 'mcp-abc12345', - updated: true, - }) - - const res = await callCreate(VALID_BODY) + it('authenticates before parsing create input', async () => { + mocks.authenticate.mockRejectedValueOnce(new MockV2ApiKeyUnauthenticatedError()) - expect(res.status).toBe(201) - expect(mockPerformCreateMcpServer).toHaveBeenCalled() - }) - - it('creates the server and returns 201 with the public shape', async () => { - const res = await callCreate({ ...VALID_BODY, headers: { Authorization: 'Bearer tok' } }) - const body = await res.json() + const response = await POST(request('POST', '/api/v2/mcp-servers', {})) - expect(res.status).toBe(201) - expect(body.data.mcpServer).toMatchObject({ - id: 'mcp-abc12345', - name: 'Docs server', - hasHeaders: true, - headerNames: ['Authorization'], - }) - expect(body.data.mcpServer.headers).toBeUndefined() - expect(mockPerformCreateMcpServer).toHaveBeenCalledWith( - expect.objectContaining({ - workspaceId: 'workspace-1', - userId: 'user-1', - name: 'Docs server', - url: 'https://mcp.example.com/sse', - headers: { Authorization: 'Bearer tok' }, - }) - ) + expect(response.status).toBe(401) + expect(mocks.create).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/app/api/v2/mcp-servers/route.ts b/apps/sim/app/api/v2/mcp-servers/route.ts index c14c89233c7..7639b4d3122 100644 --- a/apps/sim/app/api/v2/mcp-servers/route.ts +++ b/apps/sim/app/api/v2/mcp-servers/route.ts @@ -2,107 +2,56 @@ import { v2CreateMcpServerContract, v2ListMcpServersContract, } from '@/lib/api/contracts/v2/mcp-servers' -import { performCreateMcpServer } from '@/lib/mcp/orchestration' import { - getMcpServerIdState, - getWorkspaceMcpServer, - listWorkspaceMcpServers, -} from '@/lib/mcp/queries' -import { generateMcpServerId } from '@/lib/mcp/utils' -import { withPublicApiRouteHandler } from '@/app/api/public-api-route-handler' -import { resolveWorkspaceAccess } from '@/app/api/v1/middleware' -import { v2CursorList, v2Data, v2Error, v2WorkspaceAccessError } from '@/app/api/v2/lib/response' -import { toV2McpServer, v2McpOrchestrationError } from '@/app/api/v2/mcp-servers/utils' + defineV2JsonRoute, + v2ApiKeyAuth, + v2OrchestrationErrorPolicy, + v2RateLimits, +} from '@/lib/api/server/routes' +import { mcpServerOperations } from '@/lib/mcp/application/operations' +import { createMcpServerUseCase, listMcpServersUseCase } from '@/lib/mcp/application/use-cases' +import { captureServerEvent } from '@/lib/posthog/server' +import { toV2McpServer } from '@/app/api/v2/mcp-servers/utils' export const dynamic = 'force-dynamic' export const revalidate = 0 /** GET /api/v2/mcp-servers — List MCP servers in a workspace. */ -export const GET = withPublicApiRouteHandler({ +export const GET = defineV2JsonRoute({ contract: v2ListMcpServersContract, - rateLimitEndpoint: 'mcp-servers', - handler: async ({ input, auth: { userId, rateLimit } }) => { - const { workspaceId, search, sortBy, sortOrder } = input.query - - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'read') - if (access) return v2WorkspaceAccessError(access) - - const rows = await listWorkspaceMcpServers({ workspaceId, search, sortBy, sortOrder }) - - // The per-workspace server set is small and bounded → a single full page. - return v2CursorList(rows.map(toV2McpServer), null, { rateLimit }) - }, + operation: mcpServerOperations.list, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ query }) => query, + useCase: listMcpServersUseCase, + present: ({ servers }) => ({ data: servers.map(toV2McpServer), nextCursor: null }), }) /** POST /api/v2/mcp-servers — Register a new MCP server. */ -export const POST = withPublicApiRouteHandler({ +export const POST = defineV2JsonRoute({ contract: v2CreateMcpServerContract, - rateLimitEndpoint: 'mcp-servers', - handler: async ({ request, input, auth: { userId, rateLimit } }) => { - const { workspaceId, ...body } = input.body - - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'write') - if (access) return v2WorkspaceAccessError(access) - - /** - * The server id is a deterministic hash of workspace + normalized URL, and - * `performCreateMcpServer` upserts onto it — a second registration of the - * same URL silently overwrites the first. The internal surface and the - * copilot rely on that; a public create must not, so the collision is - * detected here, before the lib is given a chance to clobber the row. - * - * Only a *live* row is a conflict. A soft-deleted one is revived by the lib - * rather than inserted alongside, and reporting it as a duplicate would - * strand that URL for good: the detail routes resolve live rows only, so it - * could be neither fetched, patched, nor re-created. - */ - const serverId = generateMcpServerId(workspaceId, body.url) - const idState = await getMcpServerIdState({ workspaceId, serverId }) - if (idState && !idState.deleted) { - return v2Error( - 'CONFLICT', - 'An MCP server with this URL already exists in this workspace. Update it with PATCH /api/v2/mcp-servers/{id}.' - ) - } - const revivingSoftDeleted = idState?.deleted === true - - const result = await performCreateMcpServer({ - workspaceId, - userId, - name: body.name, - description: body.description, - transport: body.transport, - url: body.url, - headers: body.headers, - timeout: body.timeout, - retries: body.retries, - enabled: body.enabled, - authType: body.authType, - oauthClientId: body.oauthClientId ?? null, - oauthClientIdProvided: body.oauthClientId !== undefined, - oauthClientSecret: body.oauthClientSecret, - oauthClientSecretProvided: body.oauthClientSecret !== undefined, - request, - }) - - if (!result.success || !result.serverId) { - return v2McpOrchestrationError(result.errorCode, result.error ?? 'Failed to register server') - } - - /** - * `updated` means the lib wrote onto an existing row. Reviving the - * soft-deleted row we already saw is the intended outcome; otherwise a - * concurrent create won the id race between the check above and the write. - */ - if (result.updated && !revivingSoftDeleted) { - return v2Error('CONFLICT', 'An MCP server with this URL already exists in this workspace.') - } - - const created = await getWorkspaceMcpServer({ workspaceId, serverId: result.serverId }) - if (!created) { - throw new Error(`MCP server ${result.serverId} missing after a successful registration`) - } - - return v2Data({ mcpServer: toV2McpServer(created) }, { rateLimit, status: 201 }) + operation: mcpServerOperations.create, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ body }) => ({ ...body, source: 'api' as const }), + useCase: createMcpServerUseCase, + onSuccess: ({ principal, input, result }) => { + if (principal.kind !== 'personal_api_key' || result.updated) return + captureServerEvent( + principal.userId, + 'mcp_server_connected', + { + workspace_id: input.workspaceId, + server_name: result.server.name, + transport: result.server.transport, + }, + { + groups: { workspace: input.workspaceId }, + setOnce: { first_mcp_connected_at: new Date().toISOString() }, + } + ) }, + present: ({ server }) => ({ data: { mcpServer: toV2McpServer(server) } }), }) diff --git a/apps/sim/app/api/v2/secrets/[name]/route.test.ts b/apps/sim/app/api/v2/secrets/[name]/route.test.ts index 15db1c86543..d77c511e782 100644 --- a/apps/sim/app/api/v2/secrets/[name]/route.test.ts +++ b/apps/sim/app/api/v2/secrets/[name]/route.test.ts @@ -4,227 +4,187 @@ import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -const { - mockCheckRateLimit, - mockResolveWorkspaceAccess, - mockCheckWorkspaceAccess, - mockGetWorkspaceEnvKeyAdminAccess, - mockListVisibleWorkspaceCredentials, - mockSetWorkspaceSecret, - mockSetPersonalSecret, - mockDeleteWorkspaceSecret, - mockDeletePersonalSecret, - mockRecordAudit, -} = vi.hoisted(() => ({ - mockCheckRateLimit: vi.fn(), - mockResolveWorkspaceAccess: vi.fn(), - mockCheckWorkspaceAccess: vi.fn(), - mockGetWorkspaceEnvKeyAdminAccess: vi.fn(), - mockListVisibleWorkspaceCredentials: vi.fn(), - mockSetWorkspaceSecret: vi.fn(), - mockSetPersonalSecret: vi.fn(), - mockDeleteWorkspaceSecret: vi.fn(), - mockDeletePersonalSecret: vi.fn(), - mockRecordAudit: vi.fn(), -})) - -vi.mock('@sim/audit', () => ({ - AuditAction: { - ENVIRONMENT_UPDATED: 'environment.updated', - ENVIRONMENT_DELETED: 'environment.deleted', - }, - AuditResourceType: { ENVIRONMENT: 'environment' }, - recordAudit: mockRecordAudit, -})) - -vi.mock('@/app/api/v1/middleware', () => ({ - checkRateLimit: mockCheckRateLimit, - resolveWorkspaceAccess: mockResolveWorkspaceAccess, -})) +const { mocks, MockV2ApiKeyUnauthenticatedError } = vi.hoisted(() => { + class MockV2ApiKeyUnauthenticatedError extends Error {} + return { + mocks: { + authenticate: vi.fn(), + preauthRate: vi.fn(), + operationRate: vi.fn(), + gate: vi.fn(), + set: vi.fn(), + remove: vi.fn(), + }, + MockV2ApiKeyUnauthenticatedError, + } +}) -vi.mock('@/lib/workspaces/permissions/utils', () => ({ - checkWorkspaceAccess: mockCheckWorkspaceAccess, +vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => ({ + authenticateV2ApiKey: mocks.authenticate, + V2ApiKeyUnauthenticatedError: MockV2ApiKeyUnauthenticatedError, })) - -vi.mock('@/lib/credentials/environment', () => ({ - getWorkspaceEnvKeyAdminAccess: mockGetWorkspaceEnvKeyAdminAccess, +vi.mock('@/lib/core/rate-limiter', () => ({ + RateLimiter: class { + checkRateLimitDirect = mocks.preauthRate + checkRateLimitDirectOrThrow = mocks.operationRate + }, + getRateLimit: vi.fn().mockReturnValue({ + maxTokens: 100, + refillRate: 100, + refillIntervalMs: 60_000, + }), })) - -vi.mock('@/lib/credentials/queries', () => ({ - listVisibleWorkspaceCredentials: mockListVisibleWorkspaceCredentials, +vi.mock('@/lib/api/server/rate-limit-context', () => ({ + recordRateLimitSnapshot: vi.fn(), + getRateLimitHeaders: vi.fn().mockReturnValue(null), })) - -vi.mock('@/lib/credentials/secret-values', () => ({ - setWorkspaceSecret: mockSetWorkspaceSecret, - setPersonalSecret: mockSetPersonalSecret, - deleteWorkspaceSecret: mockDeleteWorkspaceSecret, - deletePersonalSecret: mockDeletePersonalSecret, +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: vi.fn().mockReturnValue('request-1'), + getClientIp: vi.fn().mockReturnValue('127.0.0.1'), })) - -vi.mock('@/app/api/v2/lib/gate', () => ({ - v2ApiGateError: vi.fn().mockResolvedValue(null), +vi.mock('@/app/api/v2/lib/gate', () => ({ v2ApiGateError: mocks.gate })) +vi.mock('@/lib/secrets/application/use-cases', () => ({ + setSecretUseCase: { operation: { id: 'secrets.set' }, execute: mocks.set }, + deleteSecretUseCase: { operation: { id: 'secrets.delete' }, execute: mocks.remove }, })) +import { OrchestrationError } from '@/lib/core/orchestration/types' import { DELETE, PUT } from '@/app/api/v2/secrets/[name]/route' -const WORKSPACE_ID = '11111111-2222-4333-8444-555555555555' +const WORKSPACE_ID = 'workspace-1' +const SECRET_NAME = 'STRIPE_API_KEY' +const PRINCIPAL = { kind: 'personal_api_key' as const, userId: 'user-1', keyId: 'key-personal' } +const AUTH = { + principal: PRINCIPAL, + rolloutUserId: 'user-1', + rateLimitSubjectIds: ['user:user-1'] as const, + rateLimitSubscription: null, + keyType: 'personal' as const, +} const RATE_LIMIT_OK = { allowed: true, - userId: 'user-1', - keyType: 'workspace', limit: 100, remaining: 99, - resetAt: new Date('2024-01-01T01:00:00Z'), -} - -function secretCredential(scope: 'workspace' | 'personal') { - return { - id: 'secret-1', - workspaceId: WORKSPACE_ID, - type: scope === 'workspace' ? ('env_workspace' as const) : ('env_personal' as const), - displayName: 'STRIPE_API_KEY', - description: null, - providerId: null, - accountId: null, - envKey: 'STRIPE_API_KEY', - envOwnerUserId: scope === 'personal' ? 'user-1' : null, - createdBy: 'user-1', - createdAt: new Date('2024-01-01T00:00:00Z'), - updatedAt: new Date('2024-01-02T00:00:00Z'), - hasServiceAccountKey: false, - role: 'admin' as const, - } + resetAt: new Date('2026-01-01T00:00:00Z'), + retryAfterMs: 0, } - -const context = { params: Promise.resolve({ name: 'STRIPE_API_KEY' }) } - -function callSet(scope: 'workspace' | 'personal', value = 'super-secret-value') { - mockListVisibleWorkspaceCredentials.mockResolvedValue([secretCredential(scope)]) - return PUT( - new NextRequest('http://localhost:3000/api/v2/secrets/STRIPE_API_KEY', { - method: 'PUT', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ workspaceId: WORKSPACE_ID, scope, value }), - }), - context - ) +const secret = { + id: 'secret-1', + workspaceId: WORKSPACE_ID, + type: 'env_workspace' as const, + displayName: SECRET_NAME, + description: null, + providerId: null, + accountId: null, + envKey: SECRET_NAME, + envOwnerUserId: null, + createdBy: 'user-1', + createdAt: new Date('2026-01-01T00:00:00Z'), + updatedAt: new Date('2026-01-02T00:00:00Z'), + hasServiceAccountKey: false, + role: 'admin' as const, } - -function callDelete(scope: 'workspace' | 'personal') { - return DELETE( - new NextRequest( - `http://localhost:3000/api/v2/secrets/STRIPE_API_KEY?workspaceId=${WORKSPACE_ID}&scope=${scope}`, - { method: 'DELETE' } - ), - context +const context = { params: Promise.resolve({ name: SECRET_NAME }) } + +function request(method: 'PUT' | 'DELETE', body?: unknown) { + const scope = method === 'DELETE' ? '&scope=workspace' : '' + return new NextRequest( + `http://localhost:3000/api/v2/secrets/${SECRET_NAME}?workspaceId=${WORKSPACE_ID}${scope}`, + { + method, + headers: { + 'x-api-key': 'key', + ...(body === undefined ? {} : { 'Content-Type': 'application/json' }), + }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + } ) } -describe('PUT /api/v2/secrets/[name]', () => { +describe('/api/v2/secrets/[name]', () => { beforeEach(() => { vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockCheckWorkspaceAccess.mockResolvedValue({ hasAccess: true, canWrite: true, canAdmin: false }) - mockGetWorkspaceEnvKeyAdminAccess.mockResolvedValue({ - adminKeys: new Set(), - knownKeys: new Set(), - }) - mockSetWorkspaceSecret.mockResolvedValue({ created: true, updatedAt: new Date() }) - mockSetPersonalSecret.mockResolvedValue({ created: true, updatedAt: new Date() }) + mocks.authenticate.mockResolvedValue(AUTH) + mocks.preauthRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.operationRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.gate.mockResolvedValue(null) + mocks.set.mockResolvedValue({ secret, userId: 'user-1', created: true }) + mocks.remove.mockResolvedValue({ name: SECRET_NAME, scope: 'workspace' }) }) - it('sets a workspace secret and never echoes its value', async () => { - const res = await callSet('workspace') - const body = await res.json() - - expect(res.status).toBe(201) - expect(body.data.secret).toMatchObject({ name: 'STRIPE_API_KEY', scope: 'workspace' }) - expect(JSON.stringify(body)).not.toContain('super-secret-value') - expect(mockSetWorkspaceSecret).toHaveBeenCalledWith({ - workspaceId: WORKSPACE_ID, - name: 'STRIPE_API_KEY', - value: 'super-secret-value', - userId: 'user-1', - }) - }) + it('creates a write-only secret with a dynamic 201 status', async () => { + const response = await PUT( + request('PUT', { workspaceId: WORKSPACE_ID, scope: 'workspace', value: 'secret-value' }), + context + ) - it('updates an existing workspace secret only for a secret admin', async () => { - mockGetWorkspaceEnvKeyAdminAccess.mockResolvedValue({ - adminKeys: new Set(), - knownKeys: new Set(['STRIPE_API_KEY']), + expect(response.status).toBe(201) + expect(JSON.stringify(await response.json())).not.toContain('secret-value') + expect(mocks.set).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { + workspaceId: WORKSPACE_ID, + name: SECRET_NAME, + scope: 'workspace', + value: 'secret-value', + }, + request: expect.anything(), }) + }) - const forbidden = await callSet('workspace') - expect(forbidden.status).toBe(403) - expect(mockSetWorkspaceSecret).not.toHaveBeenCalled() + it('returns 200 when replacing an existing secret', async () => { + mocks.set.mockResolvedValueOnce({ secret, userId: 'user-1', created: false }) - mockGetWorkspaceEnvKeyAdminAccess.mockResolvedValue({ - adminKeys: new Set(['STRIPE_API_KEY']), - knownKeys: new Set(['STRIPE_API_KEY']), - }) - mockSetWorkspaceSecret.mockResolvedValue({ created: false, updatedAt: new Date() }) + const response = await PUT( + request('PUT', { workspaceId: WORKSPACE_ID, scope: 'workspace', value: 'replacement' }), + context + ) - const updated = await callSet('workspace') - expect(updated.status).toBe(200) + expect(response.status).toBe(200) }) - it('sets only the caller-owned personal secret catalog', async () => { - const res = await callSet('personal') + it('deletes a secret through the semantic delete operation', async () => { + const response = await DELETE(request('DELETE'), context) - expect(res.status).toBe(201) - expect(mockSetPersonalSecret).toHaveBeenCalledWith({ - userId: 'user-1', - name: 'STRIPE_API_KEY', - value: 'super-secret-value', + expect(response.status).toBe(200) + expect(await response.json()).toEqual({ + data: { name: SECRET_NAME, scope: 'workspace', deleted: true }, + }) + expect(mocks.remove).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { workspaceId: WORKSPACE_ID, name: SECRET_NAME, scope: 'workspace' }, + request: expect.anything(), }) - expect(mockSetWorkspaceSecret).not.toHaveBeenCalled() }) - it('rejects invalid names and empty values before storage', async () => { - const invalidContext = { params: Promise.resolve({ name: 'not-valid' }) } - const res = await PUT( - new NextRequest('http://localhost:3000/api/v2/secrets/not-valid', { - method: 'PUT', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ workspaceId: WORKSPACE_ID, scope: 'workspace', value: '' }), - }), - invalidContext - ) + it('renders typed application errors without leaking raw errors', async () => { + mocks.remove.mockRejectedValueOnce(new OrchestrationError('not_found', 'stored detail')) - expect(res.status).toBe(400) - expect(mockSetWorkspaceSecret).not.toHaveBeenCalled() - }) -}) + const response = await DELETE(request('DELETE'), context) -describe('DELETE /api/v2/secrets/[name]', () => { - beforeEach(() => { - vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockCheckWorkspaceAccess.mockResolvedValue({ hasAccess: true, canWrite: true, canAdmin: true }) - mockGetWorkspaceEnvKeyAdminAccess.mockResolvedValue({ - adminKeys: new Set(['STRIPE_API_KEY']), - knownKeys: new Set(['STRIPE_API_KEY']), + expect(response.status).toBe(404) + expect(await response.json()).toEqual({ + error: { code: 'NOT_FOUND', message: 'stored detail' }, }) - mockDeleteWorkspaceSecret.mockResolvedValue(true) - mockDeletePersonalSecret.mockResolvedValue(true) }) - it('deletes workspace secret metadata without returning a value', async () => { - const res = await callDelete('workspace') - const body = await res.json() + it('conceals unclassified application errors', async () => { + mocks.remove.mockRejectedValueOnce(new Error('database connection detail')) + + const response = await DELETE(request('DELETE'), context) + const body = await response.json() - expect(res.status).toBe(200) - expect(body.data).toEqual({ name: 'STRIPE_API_KEY', scope: 'workspace', deleted: true }) - expect(JSON.stringify(body)).not.toContain('value') + expect(response.status).toBe(500) + expect(body).toEqual({ error: { code: 'INTERNAL_ERROR', message: 'Internal server error' } }) + expect(JSON.stringify(body)).not.toContain('database connection detail') }) - it('returns 404 when the scoped secret does not exist', async () => { - mockDeletePersonalSecret.mockResolvedValue(false) + it('authenticates before parsing a malformed set request', async () => { + mocks.authenticate.mockRejectedValueOnce(new MockV2ApiKeyUnauthenticatedError()) - const res = await callDelete('personal') + const response = await PUT(request('PUT', {}), context) - expect(res.status).toBe(404) + expect(response.status).toBe(401) + expect(mocks.set).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/app/api/v2/secrets/[name]/route.ts b/apps/sim/app/api/v2/secrets/[name]/route.ts index ca559713c46..1eb74ae39da 100644 --- a/apps/sim/app/api/v2/secrets/[name]/route.ts +++ b/apps/sim/app/api/v2/secrets/[name]/route.ts @@ -1,168 +1,38 @@ -import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' -import type { NextResponse } from 'next/server' +import { v2DeleteSecretContract, v2SetSecretContract } from '@/lib/api/contracts/v2/secrets' import { - type V2Secret, - type V2SecretScope, - v2DeleteSecretContract, - v2SetSecretContract, -} from '@/lib/api/contracts/v2/secrets' -import { getWorkspaceEnvKeyAdminAccess } from '@/lib/credentials/environment' -import { listVisibleWorkspaceCredentials } from '@/lib/credentials/queries' -import { - deletePersonalSecret, - deleteWorkspaceSecret, - setPersonalSecret, - setWorkspaceSecret, -} from '@/lib/credentials/secret-values' -import { checkWorkspaceAccess } from '@/lib/workspaces/permissions/utils' -import { withPublicApiRouteHandler } from '@/app/api/public-api-route-handler' -import { resolveWorkspaceAccess } from '@/app/api/v1/middleware' -import { v2Data, v2Error, v2WorkspaceAccessError } from '@/app/api/v2/lib/response' -import { secretCredentialTypes, toV2Secret } from '@/app/api/v2/secrets/utils' + defineV2JsonRoute, + v2ApiKeyAuth, + v2OrchestrationErrorPolicy, + v2RateLimits, +} from '@/lib/api/server/routes' +import { secretOperations } from '@/lib/secrets/application/operations' +import { deleteSecretUseCase, setSecretUseCase } from '@/lib/secrets/application/use-cases' +import { toV2Secret } from '@/app/api/v2/secrets/utils' export const dynamic = 'force-dynamic' export const revalidate = 0 -interface RouteContext { - params: Promise<{ name: string }> -} - -/** Enforces the per-secret admin rule used by the existing workspace editor. */ -async function workspaceSecretAccessError(params: { - workspaceId: string - name: string - userId: string - canWrite: boolean - canAdmin: boolean -}): Promise { - const { workspaceId, name, userId, canWrite, canAdmin } = params - const { adminKeys, knownKeys } = await getWorkspaceEnvKeyAdminAccess({ - workspaceId, - envKeys: [name], - userId, - }) - - if (knownKeys.has(name)) { - return canAdmin || adminKeys.has(name) - ? null - : v2Error('FORBIDDEN', 'Credential admin permission required for this secret') - } - return canWrite ? null : v2Error('FORBIDDEN', 'Write permission required to set this secret') -} - -/** Reads metadata from the credential catalog; encrypted value columns are never selected. */ -async function getSecretMetadata(params: { - workspaceId: string - name: string - scope: V2SecretScope - userId: string - workspaceAccess: Awaited> -}): Promise { - const { workspaceId, name, scope, userId, workspaceAccess } = params - const rows = await listVisibleWorkspaceCredentials({ - workspaceId, - userId, - workspaceAccess, - types: [...secretCredentialTypes(scope)], - search: name, - sortBy: 'displayName', - sortOrder: 'asc', - }) - const row = rows.find( - (candidate) => - candidate.envKey === name && - (scope === 'workspace' - ? candidate.type === 'env_workspace' - : candidate.type === 'env_personal' && candidate.envOwnerUserId === userId) - ) - if (!row) throw new Error(`Secret metadata was not created for ${scope}:${name}`) - return toV2Secret(row, userId) -} - /** PUT /api/v2/secrets/[name] — Create or replace a write-only secret value. */ -export const PUT = withPublicApiRouteHandler({ +export const PUT = defineV2JsonRoute({ contract: v2SetSecretContract, - rateLimitEndpoint: 'secret-detail', - handler: async ({ request, input, auth: { userId, rateLimit } }) => { - const { name } = input.params - const { workspaceId, scope, value } = input.body - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'read') - if (access) return v2WorkspaceAccessError(access) - - const workspaceAccess = await checkWorkspaceAccess(workspaceId, userId) - if (scope === 'workspace') { - const permissionError = await workspaceSecretAccessError({ - workspaceId, - name, - userId, - canWrite: workspaceAccess.canWrite, - canAdmin: workspaceAccess.canAdmin, - }) - if (permissionError) return permissionError - } - - const result = - scope === 'workspace' - ? await setWorkspaceSecret({ workspaceId, name, value, userId }) - : await setPersonalSecret({ userId, name, value }) - const secret = await getSecretMetadata({ workspaceId, name, scope, userId, workspaceAccess }) - - recordAudit({ - workspaceId, - actorId: userId, - action: AuditAction.ENVIRONMENT_UPDATED, - resourceType: AuditResourceType.ENVIRONMENT, - resourceId: `${scope}:${name}`, - resourceName: name, - description: `${result.created ? 'Created' : 'Updated'} ${scope} secret "${name}"`, - metadata: { scope, name }, - request, - }) - - return v2Data({ secret }, { rateLimit, status: result.created ? 201 : 200 }) - }, + operation: secretOperations.set, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ params, body }) => ({ ...body, name: params.name }), + useCase: setSecretUseCase, + statusForResult: ({ created }) => (created ? 201 : 200), + present: ({ secret, userId }) => ({ data: { secret: toV2Secret(secret, userId) } }), }) /** DELETE /api/v2/secrets/[name] — Delete a secret without reading its value. */ -export const DELETE = withPublicApiRouteHandler({ +export const DELETE = defineV2JsonRoute({ contract: v2DeleteSecretContract, - rateLimitEndpoint: 'secret-detail', - handler: async ({ request, input, auth: { userId, rateLimit } }) => { - const { name } = input.params - const { workspaceId, scope } = input.query - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'read') - if (access) return v2WorkspaceAccessError(access) - - const workspaceAccess = await checkWorkspaceAccess(workspaceId, userId) - if (scope === 'workspace') { - const permissionError = await workspaceSecretAccessError({ - workspaceId, - name, - userId, - canWrite: workspaceAccess.canWrite, - canAdmin: workspaceAccess.canAdmin, - }) - if (permissionError) return permissionError - } - - const deleted = - scope === 'workspace' - ? await deleteWorkspaceSecret({ workspaceId, name }) - : await deletePersonalSecret({ userId, name }) - if (!deleted) return v2Error('NOT_FOUND', 'Secret not found') - - recordAudit({ - workspaceId, - actorId: userId, - action: AuditAction.ENVIRONMENT_DELETED, - resourceType: AuditResourceType.ENVIRONMENT, - resourceId: `${scope}:${name}`, - resourceName: name, - description: `Deleted ${scope} secret "${name}"`, - metadata: { scope, name }, - request, - }) - - return v2Data({ name, scope, deleted: true as const }, { rateLimit }) - }, + operation: secretOperations.delete, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ params, query }) => ({ ...query, name: params.name }), + useCase: deleteSecretUseCase, + present: ({ name, scope }) => ({ data: { name, scope, deleted: true as const } }), }) diff --git a/apps/sim/app/api/v2/secrets/route.test.ts b/apps/sim/app/api/v2/secrets/route.test.ts index 08b096ec84e..5c173b4d883 100644 --- a/apps/sim/app/api/v2/secrets/route.test.ts +++ b/apps/sim/app/api/v2/secrets/route.test.ts @@ -4,137 +4,134 @@ import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -const { - mockCheckRateLimit, - mockResolveWorkspaceAccess, - mockCheckWorkspaceAccess, - mockListVisibleWorkspaceCredentials, -} = vi.hoisted(() => ({ - mockCheckRateLimit: vi.fn(), - mockResolveWorkspaceAccess: vi.fn(), - mockCheckWorkspaceAccess: vi.fn(), - mockListVisibleWorkspaceCredentials: vi.fn(), -})) +const { mocks, MockV2ApiKeyUnauthenticatedError } = vi.hoisted(() => { + class MockV2ApiKeyUnauthenticatedError extends Error {} + return { + mocks: { + authenticate: vi.fn(), + preauthRate: vi.fn(), + operationRate: vi.fn(), + gate: vi.fn(), + list: vi.fn(), + }, + MockV2ApiKeyUnauthenticatedError, + } +}) -vi.mock('@/app/api/v1/middleware', () => ({ - checkRateLimit: mockCheckRateLimit, - resolveWorkspaceAccess: mockResolveWorkspaceAccess, +vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => ({ + authenticateV2ApiKey: mocks.authenticate, + V2ApiKeyUnauthenticatedError: MockV2ApiKeyUnauthenticatedError, })) - -vi.mock('@/lib/workspaces/permissions/utils', () => ({ - checkWorkspaceAccess: mockCheckWorkspaceAccess, +vi.mock('@/lib/core/rate-limiter', () => ({ + RateLimiter: class { + checkRateLimitDirect = mocks.preauthRate + checkRateLimitDirectOrThrow = mocks.operationRate + }, + getRateLimit: vi.fn().mockReturnValue({ + maxTokens: 100, + refillRate: 100, + refillIntervalMs: 60_000, + }), })) - -vi.mock('@/lib/credentials/queries', () => ({ - listVisibleWorkspaceCredentials: mockListVisibleWorkspaceCredentials, +vi.mock('@/lib/api/server/rate-limit-context', () => ({ + recordRateLimitSnapshot: vi.fn(), + getRateLimitHeaders: vi.fn().mockReturnValue(null), })) - -vi.mock('@/app/api/v2/lib/gate', () => ({ - v2ApiGateError: vi.fn().mockResolvedValue(null), +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: vi.fn().mockReturnValue('request-1'), + getClientIp: vi.fn().mockReturnValue('127.0.0.1'), +})) +vi.mock('@/app/api/v2/lib/gate', () => ({ v2ApiGateError: mocks.gate })) +vi.mock('@/lib/secrets/application/use-cases', () => ({ + listSecretsUseCase: { operation: { id: 'secrets.list' }, execute: mocks.list }, })) import { GET } from '@/app/api/v2/secrets/route' -const WORKSPACE_ID = '11111111-2222-4333-8444-555555555555' +const WORKSPACE_ID = 'workspace-1' +const PRINCIPAL = { kind: 'personal_api_key' as const, userId: 'user-1', keyId: 'key-personal' } +const AUTH = { + principal: PRINCIPAL, + rolloutUserId: 'user-1', + rateLimitSubjectIds: ['user:user-1'] as const, + rateLimitSubscription: null, + keyType: 'personal' as const, +} const RATE_LIMIT_OK = { allowed: true, - userId: 'user-1', - keyType: 'workspace', limit: 100, remaining: 99, - resetAt: new Date('2024-01-01T01:00:00Z'), + resetAt: new Date('2026-01-01T00:00:00Z'), + retryAfterMs: 0, } - -function secretCredential(overrides: Record = {}) { - return { - id: 'secret-1', - workspaceId: WORKSPACE_ID, - type: 'env_workspace' as const, - displayName: 'STRIPE_API_KEY', - description: null, - providerId: null, - accountId: null, - envKey: 'STRIPE_API_KEY', - envOwnerUserId: null, - createdBy: 'user-1', - createdAt: new Date('2024-01-01T00:00:00Z'), - updatedAt: new Date('2024-01-02T00:00:00Z'), - hasServiceAccountKey: false, - role: 'admin' as const, - ...overrides, - } +const secret = { + id: 'secret-1', + workspaceId: WORKSPACE_ID, + type: 'env_workspace' as const, + displayName: 'STRIPE_API_KEY', + description: null, + providerId: null, + accountId: null, + envKey: 'STRIPE_API_KEY', + envOwnerUserId: null, + createdBy: 'user-1', + createdAt: new Date('2026-01-01T00:00:00Z'), + updatedAt: new Date('2026-01-02T00:00:00Z'), + hasServiceAccountKey: false, + role: 'admin' as const, } -const callList = (query: string) => - GET(new NextRequest(`http://localhost:3000/api/v2/secrets?${query}`)) - describe('GET /api/v2/secrets', () => { beforeEach(() => { vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockCheckWorkspaceAccess.mockResolvedValue({ hasAccess: true, canWrite: true, canAdmin: true }) - mockListVisibleWorkspaceCredentials.mockResolvedValue([secretCredential()]) + mocks.authenticate.mockResolvedValue(AUTH) + mocks.preauthRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.operationRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.gate.mockResolvedValue(null) + mocks.list.mockResolvedValue({ secrets: [secret], userId: 'user-1' }) }) - it('lists metadata without a value field', async () => { - const res = await callList(`workspaceId=${WORKSPACE_ID}`) - const body = await res.json() + it('lists secret metadata without exposing values', async () => { + const response = await GET( + new NextRequest(`http://localhost:3000/api/v2/secrets?workspaceId=${WORKSPACE_ID}`, { + headers: { 'x-api-key': 'key' }, + }) + ) + const body = await response.json() - expect(res.status).toBe(200) + expect(response.status).toBe(200) expect(body).toEqual({ data: [ { name: 'STRIPE_API_KEY', scope: 'workspace', role: 'admin', - createdAt: '2024-01-01T00:00:00.000Z', - updatedAt: '2024-01-02T00:00:00.000Z', + createdAt: '2026-01-01T00:00:00.000Z', + updatedAt: '2026-01-02T00:00:00.000Z', }, ], nextCursor: null, }) expect(JSON.stringify(body)).not.toContain('value') - expect(mockListVisibleWorkspaceCredentials).toHaveBeenCalledWith( - expect.objectContaining({ types: ['env_workspace', 'env_personal'] }) - ) - }) - - it('does not expose another user personal secret', async () => { - mockListVisibleWorkspaceCredentials.mockResolvedValue([ - secretCredential({ - id: 'secret-2', - type: 'env_personal', - displayName: 'PRIVATE_KEY', - envKey: 'PRIVATE_KEY', - envOwnerUserId: 'user-2', - }), - ]) - - const res = await callList(`workspaceId=${WORKSPACE_ID}`) - - expect((await res.json()).data).toEqual([]) + expect(mocks.list).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { + workspaceId: WORKSPACE_ID, + scope: undefined, + search: undefined, + sortBy: 'name', + sortOrder: 'asc', + }, + request: expect.anything(), + }) }) - it('maps scope and sort filters to the credential catalog', async () => { - await callList( - `workspaceId=${WORKSPACE_ID}&scope=workspace&search=STRIPE&sortBy=name&sortOrder=desc` - ) - - expect(mockListVisibleWorkspaceCredentials).toHaveBeenCalledWith( - expect.objectContaining({ - types: ['env_workspace'], - search: 'STRIPE', - sortBy: 'displayName', - sortOrder: 'desc', - }) - ) - }) + it('authenticates before validating list input', async () => { + mocks.authenticate.mockRejectedValueOnce(new MockV2ApiKeyUnauthenticatedError()) - it('rejects missing workspace context', async () => { - const res = await callList('') + const response = await GET(new NextRequest('http://localhost:3000/api/v2/secrets')) - expect(res.status).toBe(400) - expect(mockListVisibleWorkspaceCredentials).not.toHaveBeenCalled() + expect(response.status).toBe(401) + expect(mocks.list).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/app/api/v2/secrets/route.ts b/apps/sim/app/api/v2/secrets/route.ts index a2ca345b892..62a1685dc88 100644 --- a/apps/sim/app/api/v2/secrets/route.ts +++ b/apps/sim/app/api/v2/secrets/route.ts @@ -1,37 +1,28 @@ import { v2ListSecretsContract } from '@/lib/api/contracts/v2/secrets' -import { listVisibleWorkspaceCredentials } from '@/lib/credentials/queries' -import { checkWorkspaceAccess } from '@/lib/workspaces/permissions/utils' -import { withPublicApiRouteHandler } from '@/app/api/public-api-route-handler' -import { resolveWorkspaceAccess } from '@/app/api/v1/middleware' -import { v2CursorList, v2WorkspaceAccessError } from '@/app/api/v2/lib/response' -import { secretCredentialTypes, toV2Secret } from '@/app/api/v2/secrets/utils' +import { + defineV2JsonRoute, + v2ApiKeyAuth, + v2OrchestrationErrorPolicy, + v2RateLimits, +} from '@/lib/api/server/routes' +import { secretOperations } from '@/lib/secrets/application/operations' +import { listSecretsUseCase } from '@/lib/secrets/application/use-cases' +import { toV2Secret } from '@/app/api/v2/secrets/utils' export const dynamic = 'force-dynamic' export const revalidate = 0 /** GET /api/v2/secrets — List secret names and metadata without reading their values. */ -export const GET = withPublicApiRouteHandler({ +export const GET = defineV2JsonRoute({ contract: v2ListSecretsContract, - rateLimitEndpoint: 'secrets', - handler: async ({ input, auth: { userId, rateLimit } }) => { - const { workspaceId, scope, search, sortBy, sortOrder } = input.query - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'read') - if (access) return v2WorkspaceAccessError(access) - - const workspaceAccess = await checkWorkspaceAccess(workspaceId, userId) - const credentials = await listVisibleWorkspaceCredentials({ - workspaceId, - userId, - workspaceAccess, - types: [...secretCredentialTypes(scope)], - search, - sortBy: sortBy === 'name' ? 'displayName' : sortBy, - sortOrder, - }) - const secrets = credentials - .filter((row) => row.type === 'env_workspace' || row.envOwnerUserId === userId) - .map((row) => toV2Secret(row, userId)) - - return v2CursorList(secrets, null, { rateLimit }) - }, + operation: secretOperations.list, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ query }) => query, + useCase: listSecretsUseCase, + present: ({ secrets, userId }) => ({ + data: secrets.map((secret) => toV2Secret(secret, userId)), + nextCursor: null, + }), }) diff --git a/apps/sim/app/api/v2/skills/[id]/route.test.ts b/apps/sim/app/api/v2/skills/[id]/route.test.ts index 834191497ff..9444c0d799f 100644 --- a/apps/sim/app/api/v2/skills/[id]/route.test.ts +++ b/apps/sim/app/api/v2/skills/[id]/route.test.ts @@ -1,331 +1,168 @@ /** * @vitest-environment node - * - * Public v2 skill detail: the get-by-id that has no internal equivalent, plus - * the per-id update/delete that replaced the bulk upsert. */ import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -const { - mockCheckRateLimit, - mockResolveWorkspaceAccess, - mockGetSkillById, - mockPerformUpdateSkill, - mockPerformDeleteSkill, -} = vi.hoisted(() => ({ - mockCheckRateLimit: vi.fn(), - mockResolveWorkspaceAccess: vi.fn(), - mockGetSkillById: vi.fn(), - mockPerformUpdateSkill: vi.fn(), - mockPerformDeleteSkill: vi.fn(), -})) +const { mocks, MockV2ApiKeyUnauthenticatedError } = vi.hoisted(() => { + class MockV2ApiKeyUnauthenticatedError extends Error {} + return { + mocks: { + authenticate: vi.fn(), + preauthRate: vi.fn(), + operationRate: vi.fn(), + gate: vi.fn(), + get: vi.fn(), + update: vi.fn(), + remove: vi.fn(), + capture: vi.fn(), + }, + MockV2ApiKeyUnauthenticatedError, + } +}) -vi.mock('@/app/api/v1/middleware', () => ({ - checkRateLimit: mockCheckRateLimit, - resolveWorkspaceAccess: mockResolveWorkspaceAccess, +vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => ({ + authenticateV2ApiKey: mocks.authenticate, + V2ApiKeyUnauthenticatedError: MockV2ApiKeyUnauthenticatedError, })) - -vi.mock('@/lib/workflows/skills/operations', () => ({ - getSkillById: mockGetSkillById, +vi.mock('@/lib/core/rate-limiter', () => ({ + RateLimiter: class { + checkRateLimitDirect = mocks.preauthRate + checkRateLimitDirectOrThrow = mocks.operationRate + }, + getRateLimit: vi.fn().mockReturnValue({ + maxTokens: 100, + refillRate: 100, + refillIntervalMs: 60_000, + }), })) - -vi.mock('@/lib/skills/orchestration', () => ({ - performUpdateSkill: mockPerformUpdateSkill, - performDeleteSkill: mockPerformDeleteSkill, +vi.mock('@/lib/api/server/rate-limit-context', () => ({ + recordRateLimitSnapshot: vi.fn(), + getRateLimitHeaders: vi.fn().mockReturnValue(null), })) - -vi.mock('@/app/api/v2/lib/gate', () => ({ - v2ApiGateError: vi.fn().mockResolvedValue(null), +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: vi.fn().mockReturnValue('request-1'), + getClientIp: vi.fn().mockReturnValue('127.0.0.1'), +})) +vi.mock('@/app/api/v2/lib/gate', () => ({ v2ApiGateError: mocks.gate })) +vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mocks.capture })) +vi.mock('@/lib/skills/application/use-cases', () => ({ + getSkillUseCase: { operation: { id: 'skills.read' }, execute: mocks.get }, + updateSkillUseCase: { operation: { id: 'skills.update' }, execute: mocks.update }, + deleteSkillUseCase: { operation: { id: 'skills.delete' }, execute: mocks.remove }, })) import { DELETE, GET, PATCH } from '@/app/api/v2/skills/[id]/route' +const WORKSPACE_ID = 'workspace-1' +const PRINCIPAL = { kind: 'personal_api_key' as const, userId: 'user-1', keyId: 'key-personal' } +const AUTH = { + principal: PRINCIPAL, + rolloutUserId: 'user-1', + rateLimitSubjectIds: ['user:user-1'] as const, + rateLimitSubscription: null, + keyType: 'personal' as const, +} const RATE_LIMIT_OK = { allowed: true, - userId: 'user-1', - keyType: 'workspace', limit: 100, remaining: 99, - resetAt: new Date('2024-01-01T01:00:00Z'), -} - -const RATE_LIMIT_DENIED = { - allowed: false, - limit: 100, - remaining: 0, - resetAt: new Date('2024-01-01T01:00:00Z'), - retryAfterMs: 1000, + resetAt: new Date('2026-01-01T00:00:00Z'), + retryAfterMs: 0, } - -const ACCESS_DENIED = { status: 403, code: 'FORBIDDEN', message: 'Access denied' } - -function buildSkill(overrides: Record = {}) { - return { - id: 'skl_abc123', - workspaceId: 'workspace-1', - userId: 'user-1', - name: 'refund-policy', - description: 'How to handle refunds', - content: '# Refund policy', - createdAt: new Date('2024-01-01T00:00:00Z'), - updatedAt: new Date('2024-01-02T00:00:00Z'), - ...overrides, - } +const skill = { + id: 'skill-1', + workspaceId: WORKSPACE_ID, + userId: 'user-1', + name: 'refund-policy', + description: 'How to handle refunds', + content: '# Refund policy', + createdAt: new Date('2026-01-01T00:00:00Z'), + updatedAt: new Date('2026-01-02T00:00:00Z'), } - -const routeContext = () => ({ params: Promise.resolve({ id: 'skl_abc123' }) }) -const url = (query = 'workspaceId=workspace-1') => - `http://localhost:3000/api/v2/skills/skl_abc123?${query}` - -const callGet = (query?: string) => GET(new NextRequest(url(query)), routeContext()) -const callDelete = (query?: string) => - DELETE(new NextRequest(url(query), { method: 'DELETE' }), routeContext()) - -function callPatch(body: unknown) { - return PATCH( - new NextRequest('http://localhost:3000/api/v2/skills/skl_abc123', { - method: 'PATCH', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(body), - }), - routeContext() +const context = { params: Promise.resolve({ id: skill.id }) } + +function request(method: 'GET' | 'PATCH' | 'DELETE', body?: unknown) { + return new NextRequest( + `http://localhost:3000/api/v2/skills/${skill.id}?workspaceId=${WORKSPACE_ID}`, + { + method, + headers: { + 'x-api-key': 'key', + ...(body === undefined ? {} : { 'Content-Type': 'application/json' }), + }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + } ) } -describe('GET /api/v2/skills/[id]', () => { +describe('/api/v2/skills/[id]', () => { beforeEach(() => { vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockGetSkillById.mockResolvedValue(buildSkill()) - }) - - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) - - const res = await callGet() - - expect(res.status).toBe(404) - expect(mockGetSkillById).not.toHaveBeenCalled() - }) - - it('400s when workspaceId is missing', async () => { - const res = await callGet('') - expect(res.status).toBe(400) - expect(mockGetSkillById).not.toHaveBeenCalled() - }) - - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue(ACCESS_DENIED) - const res = await callGet() - expect(res.status).toBe(403) - expect(mockGetSkillById).not.toHaveBeenCalled() - }) - - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_DENIED) - const res = await callGet() - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') - }) - - it('404s when the skill is not in the workspace', async () => { - mockGetSkillById.mockResolvedValue(null) - const res = await callGet() - expect(res.status).toBe(404) - expect((await res.json()).error.code).toBe('NOT_FOUND') - }) - - it('returns the single skill including its body', async () => { - const res = await callGet() - const body = await res.json() - - expect(res.status).toBe(200) - expect(body.data).toEqual({ - skill: { - id: 'skl_abc123', - name: 'refund-policy', - description: 'How to handle refunds', - content: '# Refund policy', - readOnly: false, - createdAt: '2024-01-01T00:00:00.000Z', - updatedAt: '2024-01-02T00:00:00.000Z', - }, - }) - expect(mockGetSkillById).toHaveBeenCalledWith({ - skillId: 'skl_abc123', - workspaceId: 'workspace-1', + mocks.authenticate.mockResolvedValue(AUTH) + mocks.preauthRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.operationRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.gate.mockResolvedValue(null) + mocks.get.mockResolvedValue({ skill }) + mocks.update.mockResolvedValue({ skill }) + mocks.remove.mockResolvedValue({ skill }) + }) + + it('gets a skill through the semantic read operation', async () => { + const response = await GET(request('GET'), context) + + expect(response.status).toBe(200) + expect((await response.json()).data.skill.content).toBe(skill.content) + expect(mocks.get).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { workspaceId: WORKSPACE_ID, skillId: skill.id }, + request: expect.anything(), }) }) -}) -describe('PATCH /api/v2/skills/[id]', () => { - beforeEach(() => { - vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockPerformUpdateSkill.mockResolvedValue({ - success: true, - skill: buildSkill({ description: 'Updated' }), - }) - }) - - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) - - const res = await callPatch({ workspaceId: 'workspace-1', description: 'Updated' }) - - expect(res.status).toBe(404) - expect(mockPerformUpdateSkill).not.toHaveBeenCalled() - }) - - it('400s when no field to change is supplied', async () => { - const res = await callPatch({ workspaceId: 'workspace-1' }) - expect(res.status).toBe(400) - expect((await res.json()).error.code).toBe('BAD_REQUEST') - expect(mockPerformUpdateSkill).not.toHaveBeenCalled() - }) - - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue(ACCESS_DENIED) - const res = await callPatch({ workspaceId: 'workspace-1', description: 'Updated' }) - expect(res.status).toBe(403) - expect(mockPerformUpdateSkill).not.toHaveBeenCalled() - }) - - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_DENIED) - const res = await callPatch({ workspaceId: 'workspace-1', description: 'Updated' }) - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') - }) - - it('403s when the caller is not a skill editor', async () => { - mockPerformUpdateSkill.mockResolvedValue({ - success: false, - error: 'Skill editor access required to modify "refund-policy"', - errorCode: 'forbidden', - }) - const res = await callPatch({ workspaceId: 'workspace-1', description: 'Updated' }) - expect(res.status).toBe(403) - expect((await res.json()).error.code).toBe('FORBIDDEN') - }) - - it('400s when the orchestration rejects a built-in skill', async () => { - mockPerformUpdateSkill.mockResolvedValue({ - success: false, - error: 'Built-in skills are read-only and cannot be modified', - errorCode: 'validation', - }) - const res = await callPatch({ workspaceId: 'workspace-1', description: 'Updated' }) - expect(res.status).toBe(400) - expect((await res.json()).error.message).toContain('Built-in') - }) - - it('gates on workspace read, leaving edit rights to the per-skill editor check', async () => { - await callPatch({ workspaceId: 'workspace-1', description: 'Updated' }) - expect(mockResolveWorkspaceAccess).toHaveBeenCalledWith( - expect.anything(), - 'user-1', - 'workspace-1', - 'read' + it('updates a skill and emits only surface analytics', async () => { + const response = await PATCH( + request('PATCH', { workspaceId: WORKSPACE_ID, content: '# Updated' }), + context ) - }) - - it('updates the skill and returns the single skill', async () => { - const res = await callPatch({ workspaceId: 'workspace-1', description: 'Updated' }) - const body = await res.json() - expect(res.status).toBe(200) - expect(body.data.skill.description).toBe('Updated') - expect(Array.isArray(body.data)).toBe(false) - expect(mockPerformUpdateSkill).toHaveBeenCalledWith( - expect.objectContaining({ - workspaceId: 'workspace-1', - userId: 'user-1', - skillId: 'skl_abc123', - description: 'Updated', + expect(response.status).toBe(200) + expect(mocks.update).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { + workspaceId: WORKSPACE_ID, + skillId: skill.id, + content: '# Updated', source: 'api', - }) + }, + request: expect.anything(), + }) + expect(mocks.capture).toHaveBeenCalledWith( + 'user-1', + 'skill_updated', + expect.objectContaining({ skill_id: skill.id }), + expect.anything() ) }) -}) - -describe('DELETE /api/v2/skills/[id]', () => { - beforeEach(() => { - vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockPerformDeleteSkill.mockResolvedValue({ success: true, skill: buildSkill() }) - }) - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) - - const res = await callDelete() - - expect(res.status).toBe(404) - expect(mockPerformDeleteSkill).not.toHaveBeenCalled() - }) - - it('400s when workspaceId is missing', async () => { - const res = await callDelete('') - expect(res.status).toBe(400) - expect(mockPerformDeleteSkill).not.toHaveBeenCalled() - }) + it('deletes a skill through the semantic delete operation', async () => { + const response = await DELETE(request('DELETE'), context) - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue(ACCESS_DENIED) - const res = await callDelete() - expect(res.status).toBe(403) - expect(mockPerformDeleteSkill).not.toHaveBeenCalled() - }) - - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_DENIED) - const res = await callDelete() - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') - }) - - it('400s when the skill is a read-only built-in', async () => { - mockPerformDeleteSkill.mockResolvedValue({ - success: false, - error: 'Built-in skills are read-only and cannot be modified', - errorCode: 'validation', + expect(response.status).toBe(200) + expect(await response.json()).toEqual({ data: { id: skill.id, deleted: true } }) + expect(mocks.remove).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { workspaceId: WORKSPACE_ID, skillId: skill.id, source: 'api' }, + request: expect.anything(), }) - const res = await callDelete() - expect(res.status).toBe(400) }) - it('gates on workspace read, leaving delete rights to the per-skill editor check', async () => { - await callDelete() - expect(mockResolveWorkspaceAccess).toHaveBeenCalledWith( - expect.anything(), - 'user-1', - 'workspace-1', - 'read' - ) - }) + it('authenticates before parsing an empty update body', async () => { + mocks.authenticate.mockRejectedValueOnce(new MockV2ApiKeyUnauthenticatedError()) - it('deletes the skill and acknowledges the id', async () => { - const res = await callDelete() - expect(res.status).toBe(200) - expect(await res.json()).toEqual({ data: { id: 'skl_abc123', deleted: true } }) - expect(mockPerformDeleteSkill).toHaveBeenCalledWith( - expect.objectContaining({ - workspaceId: 'workspace-1', - userId: 'user-1', - skillId: 'skl_abc123', - source: 'api', - }) - ) + const response = await PATCH(request('PATCH', {}), context) + + expect(response.status).toBe(401) + expect(mocks.update).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/app/api/v2/skills/[id]/route.ts b/apps/sim/app/api/v2/skills/[id]/route.ts index 00dae3b6bce..eb896725050 100644 --- a/apps/sim/app/api/v2/skills/[id]/route.ts +++ b/apps/sim/app/api/v2/skills/[id]/route.ts @@ -3,99 +3,91 @@ import { v2GetSkillContract, v2UpdateSkillContract, } from '@/lib/api/contracts/v2/skills' -import { performDeleteSkill, performUpdateSkill } from '@/lib/skills/orchestration' -import { getSkillById } from '@/lib/workflows/skills/operations' -import { withPublicApiRouteHandler } from '@/app/api/public-api-route-handler' -import { resolveWorkspaceAccess } from '@/app/api/v1/middleware' -import { v2Data, v2Error, v2WorkspaceAccessError } from '@/app/api/v2/lib/response' -import { toV2Skill, v2SkillOrchestrationError } from '@/app/api/v2/skills/utils' +import { + defineV2JsonRoute, + v2ApiKeyAuth, + v2OrchestrationErrorPolicy, + v2RateLimits, +} from '@/lib/api/server/routes' +import { captureServerEvent } from '@/lib/posthog/server' +import { skillOperations } from '@/lib/skills/application/operations' +import { + deleteSkillUseCase, + getSkillUseCase, + updateSkillUseCase, +} from '@/lib/skills/application/use-cases' +import { toV2Skill } from '@/app/api/v2/skills/utils' export const dynamic = 'force-dynamic' export const revalidate = 0 -interface RouteContext { - params: Promise<{ id: string }> -} - /** GET /api/v2/skills/[id] — Fetch a single skill, including its body. */ -export const GET = withPublicApiRouteHandler({ +export const GET = defineV2JsonRoute({ contract: v2GetSkillContract, - rateLimitEndpoint: 'skill-detail', - handler: async ({ input, auth: { userId, rateLimit } }) => { - const { id } = input.params - const { workspaceId } = input.query - - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'read') - if (access) return v2WorkspaceAccessError(access) - - const skill = await getSkillById({ skillId: id, workspaceId }) - if (!skill) return v2Error('NOT_FOUND', 'Skill not found') - - return v2Data({ skill: toV2Skill(skill) }, { rateLimit }) - }, + operation: skillOperations.read, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ params, query }) => ({ workspaceId: query.workspaceId, skillId: params.id }), + useCase: getSkillUseCase, + present: ({ skill }) => ({ data: { skill: toV2Skill(skill) } }), }) -/** PATCH /api/v2/skills/[id] — Update a skill. Omitted fields keep their values. */ -export const PATCH = withPublicApiRouteHandler({ +/** PATCH /api/v2/skills/[id] — Update a skill. */ +export const PATCH = defineV2JsonRoute({ contract: v2UpdateSkillContract, - rateLimitEndpoint: 'skill-detail', - handler: async ({ request, input, auth: { userId, rateLimit } }) => { - const { id } = input.params - const { workspaceId, name, description, content } = input.body - - /** - * Editing an existing skill is gated per skill, not per workspace: an - * explicit editor grant (or workspace admin) is the authority, and - * `performUpdateSkill` enforces it. Requiring workspace `write` here would - * reject a legitimate skill editor who only holds `read` — stricter than the - * UI and than what this endpoint documents. Creating still needs `write`. - */ - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'read') - if (access) return v2WorkspaceAccessError(access) - - const result = await performUpdateSkill({ - workspaceId, - userId, - skillId: id, - name, - description, - content, - source: 'api', - request, - }) - - if (!result.success || !result.skill) { - return v2SkillOrchestrationError(result.errorCode, result.error ?? 'Failed to update skill') - } - - return v2Data({ skill: toV2Skill(result.skill) }, { rateLimit }) + operation: skillOperations.update, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ params, body }) => ({ + ...body, + skillId: params.id, + source: 'api' as const, + }), + useCase: updateSkillUseCase, + onSuccess: ({ principal, input, result }) => { + if (principal.kind !== 'personal_api_key') return + captureServerEvent( + principal.userId, + 'skill_updated', + { + skill_id: result.skill.id, + skill_name: result.skill.name, + workspace_id: input.workspaceId, + source: 'api', + }, + { groups: { workspace: input.workspaceId } } + ) }, + present: ({ skill }) => ({ data: { skill: toV2Skill(skill) } }), }) /** DELETE /api/v2/skills/[id] — Delete a skill. */ -export const DELETE = withPublicApiRouteHandler({ +export const DELETE = defineV2JsonRoute({ contract: v2DeleteSkillContract, - rateLimitEndpoint: 'skill-detail', - handler: async ({ request, input, auth: { userId, rateLimit } }) => { - const { id } = input.params - const { workspaceId } = input.query - - // Gated per skill by `performDeleteSkill`, same as PATCH above. - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'read') - if (access) return v2WorkspaceAccessError(access) - - const result = await performDeleteSkill({ - workspaceId, - userId, - skillId: id, - source: 'api', - request, - }) - - if (!result.success) { - return v2SkillOrchestrationError(result.errorCode, result.error ?? 'Failed to delete skill') - } - - return v2Data({ id, deleted: true as const }, { rateLimit }) + operation: skillOperations.delete, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ params, query }) => ({ + workspaceId: query.workspaceId, + skillId: params.id, + source: 'api' as const, + }), + useCase: deleteSkillUseCase, + onSuccess: ({ principal, input, result }) => { + if (principal.kind !== 'personal_api_key') return + captureServerEvent( + principal.userId, + 'skill_deleted', + { + skill_id: result.skill.id, + workspace_id: input.workspaceId, + source: 'api', + }, + { groups: { workspace: input.workspaceId } } + ) }, + present: ({ skill }) => ({ data: { id: skill.id, deleted: true as const } }), }) diff --git a/apps/sim/app/api/v2/skills/route.test.ts b/apps/sim/app/api/v2/skills/route.test.ts index 8e1c5131c2e..9f7f634a2e5 100644 --- a/apps/sim/app/api/v2/skills/route.test.ts +++ b/apps/sim/app/api/v2/skills/route.test.ts @@ -1,290 +1,180 @@ /** * @vitest-environment node - * - * Public v2 skills list/create: gate ordering, contract validation, and the - * single-resource create that replaced the internal bulk upsert. */ import { NextRequest } from 'next/server' import { beforeEach, describe, expect, it, vi } from 'vitest' -const { mockCheckRateLimit, mockResolveWorkspaceAccess, mockListSkills, mockPerformCreateSkill } = - vi.hoisted(() => ({ - mockCheckRateLimit: vi.fn(), - mockResolveWorkspaceAccess: vi.fn(), - mockListSkills: vi.fn(), - mockPerformCreateSkill: vi.fn(), - })) +const { mocks, MockV2ApiKeyUnauthenticatedError } = vi.hoisted(() => { + class MockV2ApiKeyUnauthenticatedError extends Error {} + return { + mocks: { + authenticate: vi.fn(), + preauthRate: vi.fn(), + operationRate: vi.fn(), + gate: vi.fn(), + list: vi.fn(), + create: vi.fn(), + capture: vi.fn(), + }, + MockV2ApiKeyUnauthenticatedError, + } +}) -vi.mock('@/app/api/v1/middleware', () => ({ - checkRateLimit: mockCheckRateLimit, - resolveWorkspaceAccess: mockResolveWorkspaceAccess, +vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => ({ + authenticateV2ApiKey: mocks.authenticate, + V2ApiKeyUnauthenticatedError: MockV2ApiKeyUnauthenticatedError, })) - -vi.mock('@/lib/workflows/skills/operations', () => ({ - listSkills: mockListSkills, +vi.mock('@/lib/core/rate-limiter', () => ({ + RateLimiter: class { + checkRateLimitDirect = mocks.preauthRate + checkRateLimitDirectOrThrow = mocks.operationRate + }, + getRateLimit: vi.fn().mockReturnValue({ + maxTokens: 100, + refillRate: 100, + refillIntervalMs: 60_000, + }), })) - -vi.mock('@/lib/skills/orchestration', () => ({ - performCreateSkill: mockPerformCreateSkill, +vi.mock('@/lib/api/server/rate-limit-context', () => ({ + recordRateLimitSnapshot: vi.fn(), + getRateLimitHeaders: vi.fn().mockReturnValue(null), })) - -vi.mock('@/app/api/v2/lib/gate', () => ({ - v2ApiGateError: vi.fn().mockResolvedValue(null), +vi.mock('@/lib/core/utils/request', () => ({ + generateRequestId: vi.fn().mockReturnValue('request-1'), + getClientIp: vi.fn().mockReturnValue('127.0.0.1'), +})) +vi.mock('@/app/api/v2/lib/gate', () => ({ v2ApiGateError: mocks.gate })) +vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mocks.capture })) +vi.mock('@/lib/skills/application/use-cases', () => ({ + listSkillsUseCase: { operation: { id: 'skills.list' }, execute: mocks.list }, + createSkillUseCase: { operation: { id: 'skills.create' }, execute: mocks.create }, })) import { GET, POST } from '@/app/api/v2/skills/route' +const WORKSPACE_ID = 'workspace-1' +const PRINCIPAL = { kind: 'workspace_api_key' as const, workspaceId: WORKSPACE_ID, keyId: 'key-1' } +const AUTH = { + principal: PRINCIPAL, + rolloutUserId: 'owner-1', + rateLimitSubjectIds: ['workspace:workspace-1'] as const, + rateLimitSubscription: null, + keyType: 'workspace' as const, +} const RATE_LIMIT_OK = { allowed: true, - userId: 'user-1', - keyType: 'workspace', limit: 100, remaining: 99, - resetAt: new Date('2024-01-01T01:00:00Z'), -} - -const RATE_LIMIT_DENIED = { - allowed: false, - limit: 100, - remaining: 0, - resetAt: new Date('2024-01-01T01:00:00Z'), - retryAfterMs: 1000, -} - -function buildSkill(overrides: Record = {}) { - return { - id: 'skl_abc123', - workspaceId: 'workspace-1', - userId: 'user-1', - name: 'refund-policy', - description: 'How to handle refunds', - content: '# Refund policy\n\nAlways be kind.', - createdAt: new Date('2024-01-01T00:00:00Z'), - updatedAt: new Date('2024-01-02T00:00:00Z'), - ...overrides, - } -} - -function callList(query: string) { - return GET(new NextRequest(`http://localhost:3000/api/v2/skills?${query}`)) -} - -function callCreate(body: unknown) { - return POST( - new NextRequest('http://localhost:3000/api/v2/skills', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify(body), - }) - ) + resetAt: new Date('2026-01-01T00:00:00Z'), + retryAfterMs: 0, } - -const VALID_BODY = { - workspaceId: 'workspace-1', +const skill = { + id: 'skill-1', + workspaceId: WORKSPACE_ID, + userId: 'owner-1', name: 'refund-policy', description: 'How to handle refunds', content: '# Refund policy', + createdAt: new Date('2026-01-01T00:00:00Z'), + updatedAt: new Date('2026-01-02T00:00:00Z'), } -describe('GET /api/v2/skills', () => { - beforeEach(() => { - vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockListSkills.mockResolvedValue([buildSkill()]) - }) - - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) - - const res = await callList('workspaceId=workspace-1') - - expect(res.status).toBe(404) - expect((await res.json()).error.code).toBe('NOT_FOUND') - expect(mockListSkills).not.toHaveBeenCalled() - }) - - it('400s when workspaceId is missing', async () => { - const res = await callList('') - expect(res.status).toBe(400) - expect((await res.json()).error.code).toBe('BAD_REQUEST') - expect(mockListSkills).not.toHaveBeenCalled() - }) - - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue({ - status: 403, - code: 'FORBIDDEN', - message: 'Access denied', - }) - const res = await callList('workspaceId=workspace-1') - expect(res.status).toBe(403) - expect(mockListSkills).not.toHaveBeenCalled() - }) - - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_DENIED) - const res = await callList('workspaceId=workspace-1') - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') - }) - - it('returns summaries without skill bodies in the cursor envelope', async () => { - const res = await callList('workspaceId=workspace-1') - const body = await res.json() - - expect(res.status).toBe(200) - expect(body.nextCursor).toBeNull() - expect(body.data).toEqual([ - { - id: 'skl_abc123', - name: 'refund-policy', - description: 'How to handle refunds', - readOnly: false, - createdAt: '2024-01-01T00:00:00.000Z', - updatedAt: '2024-01-02T00:00:00.000Z', - }, - ]) - expect(mockListSkills).toHaveBeenCalledWith({ - workspaceId: 'workspace-1', - search: undefined, - sort: { sortBy: 'createdAt', sortOrder: 'desc' }, - }) - }) - it('400s on a sort field outside the enum instead of letting it reach the query', async () => { - const res = await callList(`workspaceId=workspace-1&sortBy=name);--`) - - expect(res.status).toBe(400) - expect((await res.json()).error.code).toBe('BAD_REQUEST') - }) - - it('400s on a sort direction outside the enum', async () => { - const res = await callList(`workspaceId=workspace-1&sortOrder=sideways`) - - expect(res.status).toBe(400) +function request(method: 'GET' | 'POST', url: string, body?: unknown) { + return new NextRequest(`http://localhost:3000${url}`, { + method, + headers: { + 'x-api-key': 'key', + ...(body === undefined ? {} : { 'Content-Type': 'application/json' }), + }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), }) +} - it('400s on an empty search rather than treating it as unsearched', async () => { - const res = await callList(`workspaceId=workspace-1&search=`) - - expect(res.status).toBe(400) - }) - - it('forwards search and sort into the query and still terminates pagination', async () => { - const res = await callList(`workspaceId=workspace-1&search=report&sortBy=name&sortOrder=asc`) - - expect(res.status).toBe(200) - expect((await res.json()).nextCursor).toBeNull() - }) -}) - -describe('POST /api/v2/skills', () => { +describe('/api/v2/skills', () => { beforeEach(() => { vi.clearAllMocks() - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_OK) - mockResolveWorkspaceAccess.mockResolvedValue(null) - mockPerformCreateSkill.mockResolvedValue({ success: true, skill: buildSkill() }) - }) - - it('returns 404 when the v2 API surface flag is off', async () => { - const { v2ApiGateError } = await import('@/app/api/v2/lib/gate') - const { v2Error } = await import('@/app/api/v2/lib/response') - vi.mocked(v2ApiGateError).mockResolvedValueOnce(v2Error('NOT_FOUND', 'Not found')) - - const res = await callCreate(VALID_BODY) - - expect(res.status).toBe(404) - expect(mockPerformCreateSkill).not.toHaveBeenCalled() - }) - - it('400s when the body is missing content', async () => { - const res = await callCreate({ - workspaceId: 'workspace-1', - name: 'refund-policy', - description: 'How to handle refunds', + mocks.authenticate.mockResolvedValue(AUTH) + mocks.preauthRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.operationRate.mockResolvedValue(RATE_LIMIT_OK) + mocks.gate.mockResolvedValue(null) + mocks.list.mockResolvedValue({ skills: [skill] }) + mocks.create.mockResolvedValue({ skill }) + }) + + it('lists skill summaries through the authorized application use case', async () => { + const response = await GET(request('GET', `/api/v2/skills?workspaceId=${WORKSPACE_ID}`)) + + expect(response.status).toBe(200) + expect((await response.json()).data[0]).not.toHaveProperty('content') + expect(mocks.list).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { + workspaceId: WORKSPACE_ID, + search: undefined, + sortBy: 'createdAt', + sortOrder: 'desc', + }, + request: expect.anything(), }) - expect(res.status).toBe(400) - expect((await res.json()).error.code).toBe('BAD_REQUEST') - expect(mockPerformCreateSkill).not.toHaveBeenCalled() }) - it('400s when the name is not kebab-case', async () => { - const res = await callCreate({ ...VALID_BODY, name: 'Refund Policy' }) - expect(res.status).toBe(400) - expect(mockPerformCreateSkill).not.toHaveBeenCalled() - }) + it('creates a skill with the v2 source and status', async () => { + const response = await POST( + request('POST', '/api/v2/skills', { + workspaceId: WORKSPACE_ID, + name: skill.name, + description: skill.description, + content: skill.content, + }) + ) - it('surfaces an access-denied failure in the v2 error envelope', async () => { - mockResolveWorkspaceAccess.mockResolvedValue({ - status: 403, - code: 'FORBIDDEN', - message: 'Access denied', + expect(response.status).toBe(201) + expect((await response.json()).data.skill.id).toBe(skill.id) + expect(mocks.create).toHaveBeenCalledWith({ + principal: PRINCIPAL, + input: { + workspaceId: WORKSPACE_ID, + name: skill.name, + description: skill.description, + content: skill.content, + source: 'api', + }, + request: expect.anything(), }) - const res = await callCreate(VALID_BODY) - expect(res.status).toBe(403) - expect(mockPerformCreateSkill).not.toHaveBeenCalled() + expect(mocks.capture).not.toHaveBeenCalled() }) - it('returns the rate-limit response when denied', async () => { - mockCheckRateLimit.mockResolvedValue(RATE_LIMIT_DENIED) - const res = await callCreate(VALID_BODY) - expect(res.status).toBe(429) - expect((await res.json()).error.code).toBe('RATE_LIMITED') - }) - - it('400s when the orchestration rejects a built-in skill name', async () => { - mockPerformCreateSkill.mockResolvedValue({ - success: false, - error: 'The skill name "deploy-workflow" is reserved by a built-in skill', - errorCode: 'validation', + it('keeps skill analytics on the personal-key v2 surface', async () => { + mocks.authenticate.mockResolvedValueOnce({ + ...AUTH, + principal: { kind: 'personal_api_key', userId: 'user-1', keyId: 'key-personal' }, + keyType: 'personal', }) - const res = await callCreate({ ...VALID_BODY, name: 'deploy-workflow' }) - const body = await res.json() + const response = await POST( + request('POST', '/api/v2/skills', { + workspaceId: WORKSPACE_ID, + name: skill.name, + description: skill.description, + content: skill.content, + }) + ) - expect(res.status).toBe(400) - expect(body.error.code).toBe('BAD_REQUEST') - expect(body.error.message).toContain('built-in') + expect(response.status).toBe(201) + expect(mocks.capture).toHaveBeenCalledWith( + 'user-1', + 'skill_created', + expect.objectContaining({ skill_id: skill.id, source: 'api' }), + expect.anything() + ) }) - it('409s when the skill name is already taken', async () => { - mockPerformCreateSkill.mockResolvedValue({ - success: false, - error: 'The skill name "refund-policy" is unavailable in this workspace', - errorCode: 'conflict', - }) - const res = await callCreate(VALID_BODY) - expect(res.status).toBe(409) - expect((await res.json()).error.code).toBe('CONFLICT') - }) + it('authenticates before parsing skill input', async () => { + mocks.authenticate.mockRejectedValueOnce(new MockV2ApiKeyUnauthenticatedError()) - it('creates the skill and returns 201 with the single skill, not the workspace list', async () => { - const res = await callCreate(VALID_BODY) - const body = await res.json() + const response = await POST(request('POST', '/api/v2/skills', {})) - expect(res.status).toBe(201) - expect(body.data).toEqual({ - skill: { - id: 'skl_abc123', - name: 'refund-policy', - description: 'How to handle refunds', - content: '# Refund policy\n\nAlways be kind.', - readOnly: false, - createdAt: '2024-01-01T00:00:00.000Z', - updatedAt: '2024-01-02T00:00:00.000Z', - }, - }) - expect(mockPerformCreateSkill).toHaveBeenCalledWith( - expect.objectContaining({ - workspaceId: 'workspace-1', - userId: 'user-1', - name: 'refund-policy', - description: 'How to handle refunds', - content: '# Refund policy', - source: 'api', - }) - ) + expect(response.status).toBe(401) + expect(mocks.create).not.toHaveBeenCalled() }) }) diff --git a/apps/sim/app/api/v2/skills/route.ts b/apps/sim/app/api/v2/skills/route.ts index 868c1c54979..effbf7f93e0 100644 --- a/apps/sim/app/api/v2/skills/route.ts +++ b/apps/sim/app/api/v2/skills/route.ts @@ -1,55 +1,52 @@ import { v2CreateSkillContract, v2ListSkillsContract } from '@/lib/api/contracts/v2/skills' -import { performCreateSkill } from '@/lib/skills/orchestration' -import { listSkills } from '@/lib/workflows/skills/operations' -import { withPublicApiRouteHandler } from '@/app/api/public-api-route-handler' -import { resolveWorkspaceAccess } from '@/app/api/v1/middleware' -import { v2CursorList, v2Data, v2WorkspaceAccessError } from '@/app/api/v2/lib/response' -import { toV2Skill, toV2SkillSummary, v2SkillOrchestrationError } from '@/app/api/v2/skills/utils' +import { + defineV2JsonRoute, + v2ApiKeyAuth, + v2OrchestrationErrorPolicy, + v2RateLimits, +} from '@/lib/api/server/routes' +import { captureServerEvent } from '@/lib/posthog/server' +import { skillOperations } from '@/lib/skills/application/operations' +import { createSkillUseCase, listSkillsUseCase } from '@/lib/skills/application/use-cases' +import { toV2Skill, toV2SkillSummary } from '@/app/api/v2/skills/utils' export const dynamic = 'force-dynamic' export const revalidate = 0 /** GET /api/v2/skills — List skills in a workspace, built-ins included. */ -export const GET = withPublicApiRouteHandler({ +export const GET = defineV2JsonRoute({ contract: v2ListSkillsContract, - rateLimitEndpoint: 'skills', - handler: async ({ input, auth: { userId, rateLimit } }) => { - const { workspaceId, search, sortBy, sortOrder } = input.query - - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'read') - if (access) return v2WorkspaceAccessError(access) - - const skills = await listSkills({ workspaceId, search, sort: { sortBy, sortOrder } }) - - // The per-workspace skill set is small and bounded → a single full page. - return v2CursorList(skills.map(toV2SkillSummary), null, { rateLimit }) - }, + operation: skillOperations.list, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ query }) => query, + useCase: listSkillsUseCase, + present: ({ skills }) => ({ data: skills.map(toV2SkillSummary), nextCursor: null }), }) /** POST /api/v2/skills — Create a skill. */ -export const POST = withPublicApiRouteHandler({ +export const POST = defineV2JsonRoute({ contract: v2CreateSkillContract, - rateLimitEndpoint: 'skills', - handler: async ({ request, input, auth: { userId, rateLimit } }) => { - const { workspaceId, name, description, content } = input.body - - const access = await resolveWorkspaceAccess(rateLimit, userId, workspaceId, 'write') - if (access) return v2WorkspaceAccessError(access) - - const result = await performCreateSkill({ - workspaceId, - userId, - name, - description, - content, - source: 'api', - request, - }) - - if (!result.success || !result.skill) { - return v2SkillOrchestrationError(result.errorCode, result.error ?? 'Failed to create skill') - } - - return v2Data({ skill: toV2Skill(result.skill) }, { rateLimit, status: 201 }) + operation: skillOperations.create, + auth: v2ApiKeyAuth, + rateLimit: v2RateLimits.publicApi, + errorPolicy: v2OrchestrationErrorPolicy, + mapInput: ({ body }) => ({ ...body, source: 'api' as const }), + useCase: createSkillUseCase, + onSuccess: ({ principal, input, result }) => { + if (principal.kind !== 'personal_api_key') return + captureServerEvent( + principal.userId, + 'skill_created', + { + skill_id: result.skill.id, + skill_name: result.skill.name, + workspace_id: input.workspaceId, + source: 'api', + }, + { groups: { workspace: input.workspaceId } } + ) }, + present: ({ skill }) => ({ data: { skill: toV2Skill(skill) } }), }) diff --git a/apps/sim/lib/api/contracts/v2/custom-tools.ts b/apps/sim/lib/api/contracts/v2/custom-tools.ts index c2e6221e776..248a505857f 100644 --- a/apps/sim/lib/api/contracts/v2/custom-tools.ts +++ b/apps/sim/lib/api/contracts/v2/custom-tools.ts @@ -128,6 +128,7 @@ export const v2CreateCustomToolContract = defineRouteContract({ response: { mode: 'json', schema: v2DataResponse(v2CustomToolDataSchema), + status: 201, }, }) diff --git a/apps/sim/lib/api/contracts/v2/mcp-servers.ts b/apps/sim/lib/api/contracts/v2/mcp-servers.ts index 96b40e38b13..181942bfbd8 100644 --- a/apps/sim/lib/api/contracts/v2/mcp-servers.ts +++ b/apps/sim/lib/api/contracts/v2/mcp-servers.ts @@ -196,6 +196,7 @@ export const v2CreateMcpServerContract = defineRouteContract({ response: { mode: 'json', schema: v2DataResponse(v2McpServerDataSchema), + status: 201, }, }) diff --git a/apps/sim/lib/api/contracts/v2/skills.ts b/apps/sim/lib/api/contracts/v2/skills.ts index 003151aef9f..50f50671a02 100644 --- a/apps/sim/lib/api/contracts/v2/skills.ts +++ b/apps/sim/lib/api/contracts/v2/skills.ts @@ -135,6 +135,7 @@ export const v2CreateSkillContract = defineRouteContract({ response: { mode: 'json', schema: v2DataResponse(v2SkillDataSchema), + status: 201, }, }) diff --git a/apps/sim/lib/api/server/routes/v2-json-route.ts b/apps/sim/lib/api/server/routes/v2-json-route.ts index 62a0c572814..283a1c6d64b 100644 --- a/apps/sim/lib/api/server/routes/v2-json-route.ts +++ b/apps/sim/lib/api/server/routes/v2-json-route.ts @@ -164,6 +164,12 @@ interface V2JsonRouteOptions }): void | Promise + onSuccess?(args: { + principal: V2ApiKeyAuthContext['principal'] + input: NoInfer + result: NoInfer + }): void | Promise + statusForResult?(result: NoInfer): number } export function defineV2JsonRoute< @@ -213,9 +219,10 @@ export function defineV2JsonRoute< if (!parsed.success) return parsed.response try { + const input = options.mapInput(parsed.data) const result = await options.useCase.execute({ principal: auth.principal, - input: options.mapInput(parsed.data), + input, request, }) const body = await options.present(result) @@ -224,8 +231,13 @@ export function defineV2JsonRoute< throw new Error('V2 JSON route response mode changed after initialization') } const validatedBody = responseSchema.schema.parse(body) + const responseStatus = options.statusForResult?.(result) ?? successStatus + if (!Number.isInteger(responseStatus) || responseStatus < 200 || responseStatus >= 300) { + throw new Error(`V2 JSON route produced invalid success status ${responseStatus}`) + } + await options.onSuccess?.({ principal: auth.principal, input, result }) return NextResponse.json(validatedBody, { - status: successStatus, + status: responseStatus, headers: { 'Cache-Control': 'private, no-store' }, }) } catch (error) { diff --git a/apps/sim/lib/copilot/application/execute-custom-tool-use-case.ts b/apps/sim/lib/copilot/application/execute-custom-tool-use-case.ts new file mode 100644 index 00000000000..0ba46365102 --- /dev/null +++ b/apps/sim/lib/copilot/application/execute-custom-tool-use-case.ts @@ -0,0 +1,8 @@ +import { createCopilotWorkspaceUseCaseExecutor } from '@/lib/copilot/application/execute-workspace-use-case' +import { CUSTOM_TOOL_DELEGATION_AUDIENCE } from '@/lib/custom-tools/application/authorization' +import { customToolOperations } from '@/lib/custom-tools/application/operations' + +export const executeCopilotCustomToolUseCase = createCopilotWorkspaceUseCaseExecutor({ + audience: CUSTOM_TOOL_DELEGATION_AUDIENCE, + operations: customToolOperations, +}) diff --git a/apps/sim/lib/copilot/application/execute-mcp-server-use-case.ts b/apps/sim/lib/copilot/application/execute-mcp-server-use-case.ts new file mode 100644 index 00000000000..7744e0e0b2a --- /dev/null +++ b/apps/sim/lib/copilot/application/execute-mcp-server-use-case.ts @@ -0,0 +1,8 @@ +import { createCopilotWorkspaceUseCaseExecutor } from '@/lib/copilot/application/execute-workspace-use-case' +import { MCP_SERVER_DELEGATION_AUDIENCE } from '@/lib/mcp/application/authorization' +import { mcpServerOperations } from '@/lib/mcp/application/operations' + +export const executeCopilotMcpServerUseCase = createCopilotWorkspaceUseCaseExecutor({ + audience: MCP_SERVER_DELEGATION_AUDIENCE, + operations: mcpServerOperations, +}) diff --git a/apps/sim/lib/copilot/application/execute-skill-use-case.ts b/apps/sim/lib/copilot/application/execute-skill-use-case.ts new file mode 100644 index 00000000000..8acce8e1e12 --- /dev/null +++ b/apps/sim/lib/copilot/application/execute-skill-use-case.ts @@ -0,0 +1,8 @@ +import { createCopilotWorkspaceUseCaseExecutor } from '@/lib/copilot/application/execute-workspace-use-case' +import { SKILL_DELEGATION_AUDIENCE } from '@/lib/skills/application/authorization' +import { skillOperations } from '@/lib/skills/application/operations' + +export const executeCopilotSkillUseCase = createCopilotWorkspaceUseCaseExecutor({ + audience: SKILL_DELEGATION_AUDIENCE, + operations: skillOperations, +}) diff --git a/apps/sim/lib/copilot/application/execute-workspace-use-case.test.ts b/apps/sim/lib/copilot/application/execute-workspace-use-case.test.ts new file mode 100644 index 00000000000..e2aaa045163 --- /dev/null +++ b/apps/sim/lib/copilot/application/execute-workspace-use-case.test.ts @@ -0,0 +1,99 @@ +/** + * @vitest-environment node + */ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { createCopilotWorkspaceUseCaseExecutor } from '@/lib/copilot/application/execute-workspace-use-case' + +const operation = { + id: 'skills.update', + minimumRole: 'read' as const, + workspaceApiKey: 'deny' as const, + principalKinds: ['delegated'] as const, +} + +describe('Copilot workspace application delegation', () => { + afterEach(() => { + vi.useRealTimers() + }) + + it('builds a bounded principal from trusted runtime context, never tool input identity', async () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-01-01T00:00:00Z')) + const execute = vi.fn().mockResolvedValue({ ok: true }) + const executeCopilotUseCase = createCopilotWorkspaceUseCaseExecutor({ + audience: 'sim:skills', + operations: { update: operation }, + }) + + await executeCopilotUseCase( + { + userId: 'trusted-user', + workspaceId: 'workspace-1', + chatId: 'chat-1', + executionId: 'execution-1', + toolCallId: 'call-1', + copilotToolExecution: true, + }, + { operation, execute }, + { userId: 'model-supplied-user', workspaceId: 'workspace-1' } + ) + + expect(execute).toHaveBeenCalledWith({ + principal: { + kind: 'delegated', + serviceId: 'copilot', + subjectUserId: 'trusted-user', + workspaceId: 'workspace-1', + delegationId: 'copilot-tool:call-1', + audience: 'sim:skills', + issuedAt: new Date('2026-01-01T00:00:00Z'), + expiresAt: new Date('2026-01-01T00:05:00Z'), + resourceScope: { chatId: 'chat-1', executionId: 'execution-1' }, + }, + input: { userId: 'model-supplied-user', workspaceId: 'workspace-1' }, + }) + }) + + it('fails fast for an untrusted execution context', async () => { + const execute = vi.fn() + const executeCopilotUseCase = createCopilotWorkspaceUseCaseExecutor({ + audience: 'sim:skills', + operations: { update: operation }, + }) + + expect(() => + executeCopilotUseCase( + { + userId: 'user-1', + workspaceId: 'workspace-1', + toolCallId: 'call-1', + copilotToolExecution: false, + }, + { operation, execute }, + { workspaceId: 'workspace-1' } + ) + ).toThrow('trusted Copilot execution context') + expect(execute).not.toHaveBeenCalled() + }) + + it('fails fast when a tool adapter tries an unregistered operation', () => { + const executeCopilotUseCase = createCopilotWorkspaceUseCaseExecutor({ + audience: 'sim:skills', + operations: { update: operation }, + }) + const unregistered = { ...operation, id: 'skills.unregistered' } + + expect(() => + executeCopilotUseCase( + { + userId: 'user-1', + workspaceId: 'workspace-1', + toolCallId: 'call-1', + copilotToolExecution: true, + }, + { operation: unregistered, execute: vi.fn() }, + { workspaceId: 'workspace-1' } + ) + ).toThrow('Unregistered Copilot workspace operation') + }) +}) diff --git a/apps/sim/lib/copilot/application/execute-workspace-use-case.ts b/apps/sim/lib/copilot/application/execute-workspace-use-case.ts new file mode 100644 index 00000000000..03774dee872 --- /dev/null +++ b/apps/sim/lib/copilot/application/execute-workspace-use-case.ts @@ -0,0 +1,34 @@ +import { + type CopilotWorkspaceDelegationContext, + createCopilotWorkspacePrincipal, +} from '@/lib/copilot/auth/workspace-application-delegation' +import type { OperationUseCase, WorkspaceOperation } from '@/lib/core/application' + +interface CopilotWorkspaceUseCaseExecutorOptions { + audience: string + operations: Readonly> +} + +/** Binds a domain registry to the trusted Copilot workspace execution runtime. */ +export function createCopilotWorkspaceUseCaseExecutor( + options: CopilotWorkspaceUseCaseExecutorOptions +) { + const registeredOperationIds = new Set( + Object.values(options.operations).map((operation) => operation.id) + ) + + return function executeCopilotWorkspaceUseCase( + context: CopilotWorkspaceDelegationContext | undefined, + useCase: OperationUseCase, + input: I + ): Promise { + if (!registeredOperationIds.has(useCase.operation.id)) { + throw new Error(`Unregistered Copilot workspace operation: ${useCase.operation.id}`) + } + + return useCase.execute({ + principal: createCopilotWorkspacePrincipal(context, { audience: options.audience }), + input, + }) + } +} diff --git a/apps/sim/lib/copilot/auth/workspace-application-delegation.ts b/apps/sim/lib/copilot/auth/workspace-application-delegation.ts new file mode 100644 index 00000000000..d2c40aae739 --- /dev/null +++ b/apps/sim/lib/copilot/auth/workspace-application-delegation.ts @@ -0,0 +1,46 @@ +import type { DelegatedPrincipal } from '@sim/auth/principal' + +const COPILOT_WORKSPACE_DELEGATION_TTL_MS = 5 * 60 * 1000 + +export interface CopilotWorkspaceDelegationContext { + userId: string + workspaceId?: string + chatId?: string + executionId?: string + toolCallId?: string + copilotToolExecution?: boolean +} + +interface CreateCopilotWorkspacePrincipalOptions { + audience: string +} + +/** Creates a delegated principal exclusively from server-authored Copilot execution context. */ +export function createCopilotWorkspacePrincipal( + context: CopilotWorkspaceDelegationContext | undefined, + options: CreateCopilotWorkspacePrincipalOptions +): DelegatedPrincipal { + if (!context) throw new Error('Workspace delegation requires a Copilot execution context') + if (!context.copilotToolExecution) { + throw new Error('Workspace delegation requires a trusted Copilot execution context') + } + if (!context.toolCallId) throw new Error('Workspace delegation requires a tool call ID') + if (!context.workspaceId) throw new Error('Workspace delegation requires a workspace ID') + if (!options.audience) throw new Error('Workspace delegation requires an audience') + + const issuedAt = new Date() + return { + kind: 'delegated', + serviceId: 'copilot', + subjectUserId: context.userId, + workspaceId: context.workspaceId, + delegationId: `copilot-tool:${context.toolCallId}`, + audience: options.audience, + issuedAt, + expiresAt: new Date(issuedAt.getTime() + COPILOT_WORKSPACE_DELEGATION_TTL_MS), + resourceScope: { + ...(context.chatId ? { chatId: context.chatId } : {}), + ...(context.executionId ? { executionId: context.executionId } : {}), + }, + } +} diff --git a/apps/sim/lib/copilot/tools/handlers/management/manage-application-use-cases.test.ts b/apps/sim/lib/copilot/tools/handlers/management/manage-application-use-cases.test.ts new file mode 100644 index 00000000000..e7ea463273b --- /dev/null +++ b/apps/sim/lib/copilot/tools/handlers/management/manage-application-use-cases.test.ts @@ -0,0 +1,160 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mocks, useCases } = vi.hoisted(() => ({ + mocks: { + custom: vi.fn(), + mcp: vi.fn(), + skill: vi.fn(), + capture: vi.fn(), + }, + useCases: { + saveCustom: { operation: { id: 'custom_tools.save' } }, + deleteCustom: { operation: { id: 'custom_tools.delete_available' } }, + listCustom: { operation: { id: 'custom_tools.list_available' } }, + updateCustom: { operation: { id: 'custom_tools.update_available' } }, + deleteMcp: { operation: { id: 'mcp_servers.delete' } }, + listMcp: { operation: { id: 'mcp_servers.list' } }, + reconfigureMcp: { operation: { id: 'mcp_servers.reconfigure' } }, + registerMcp: { operation: { id: 'mcp_servers.register' } }, + createSkill: { operation: { id: 'skills.create' } }, + deleteSkill: { operation: { id: 'skills.delete' } }, + listSkill: { operation: { id: 'skills.list_available' } }, + updateSkill: { operation: { id: 'skills.update' } }, + }, +})) + +vi.mock('@/lib/copilot/application/execute-custom-tool-use-case', () => ({ + executeCopilotCustomToolUseCase: mocks.custom, +})) +vi.mock('@/lib/copilot/application/execute-mcp-server-use-case', () => ({ + executeCopilotMcpServerUseCase: mocks.mcp, +})) +vi.mock('@/lib/copilot/application/execute-skill-use-case', () => ({ + executeCopilotSkillUseCase: mocks.skill, +})) +vi.mock('@/lib/custom-tools/application/use-cases', () => ({ + deleteAvailableCustomToolUseCase: useCases.deleteCustom, + listAvailableCustomToolsUseCase: useCases.listCustom, + saveWorkspaceCustomToolUseCase: useCases.saveCustom, + updateAvailableCustomToolUseCase: useCases.updateCustom, +})) +vi.mock('@/lib/mcp/application/use-cases', () => ({ + deleteMcpServerUseCase: useCases.deleteMcp, + listMcpServersUseCase: useCases.listMcp, + reconfigureMcpServerUseCase: useCases.reconfigureMcp, + registerMcpServerUseCase: useCases.registerMcp, +})) +vi.mock('@/lib/skills/application/use-cases', () => ({ + createSkillUseCase: useCases.createSkill, + deleteSkillUseCase: useCases.deleteSkill, + listAvailableSkillsUseCase: useCases.listSkill, + updateSkillUseCase: useCases.updateSkill, +})) +vi.mock('@/lib/posthog/server', () => ({ captureServerEvent: mocks.capture })) + +import type { ExecutionContext } from '@/lib/copilot/request/types' +import { executeManageCustomTool } from '@/lib/copilot/tools/handlers/management/manage-custom-tool' +import { executeManageMcpTool } from '@/lib/copilot/tools/handlers/management/manage-mcp-tool' +import { executeManageSkill } from '@/lib/copilot/tools/handlers/management/manage-skill' + +const context: ExecutionContext = { + userId: 'user-1', + workflowId: '', + workspaceId: 'workspace-1', + chatId: 'chat-1', + toolCallId: 'call-1', + copilotToolExecution: true, + userPermission: 'admin', +} + +describe('Copilot management application boundaries', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('creates custom tools through the shared use case using server workspace context', async () => { + mocks.custom.mockResolvedValue({ + tool: { id: 'tool-1', title: 'lookup_order' }, + }) + + const result = await executeManageCustomTool( + { + operation: 'add', + workspaceId: 'model-workspace', + title: 'lookup_order', + schema: { + type: 'function', + function: { name: 'lookup_order', parameters: {} }, + }, + code: 'return 1', + }, + context + ) + + expect(result).toMatchObject({ success: true, output: { toolId: 'tool-1' } }) + expect(mocks.custom).toHaveBeenCalledWith(context, useCases.saveCustom, { + workspaceId: context.workspaceId, + title: 'lookup_order', + schema: { + type: 'function', + function: { name: 'lookup_order', parameters: {} }, + }, + code: 'return 1', + source: 'tool_input', + }) + }) + + it('keeps Copilot MCP registration compatibility behind its semantic operation', async () => { + mocks.mcp.mockResolvedValue({ + serverId: 'legacy-result-id', + server: { + id: 'mcp-server-1', + name: 'Docs', + transport: 'streamable-http', + }, + updated: true, + }) + + const result = await executeManageMcpTool( + { + operation: 'add', + config: { name: 'Docs', url: 'https://mcp.example.com/sse' }, + }, + context + ) + + expect(result).toMatchObject({ success: true, output: { serverId: 'mcp-server-1' } }) + expect(mocks.mcp).toHaveBeenCalledWith( + context, + useCases.registerMcp, + expect.objectContaining({ workspaceId: context.workspaceId, source: 'tool_input' }) + ) + expect(mocks.capture).not.toHaveBeenCalled() + }) + + it('delegates skill-specific edit authorization to the shared application use case', async () => { + mocks.skill.mockResolvedValue({ + skill: { id: 'skill-1', name: 'refund-policy' }, + }) + + const result = await executeManageSkill( + { operation: 'edit', skillId: 'skill-1', content: '# Updated' }, + { ...context, userPermission: 'read' } + ) + + expect(result).toMatchObject({ success: true, output: { skillId: 'skill-1' } }) + expect(mocks.skill).toHaveBeenCalledWith( + expect.objectContaining({ workspaceId: context.workspaceId }), + useCases.updateSkill, + { + workspaceId: context.workspaceId, + skillId: 'skill-1', + content: '# Updated', + source: 'tool_input', + } + ) + }) +}) diff --git a/apps/sim/lib/copilot/tools/handlers/management/manage-custom-tool.ts b/apps/sim/lib/copilot/tools/handlers/management/manage-custom-tool.ts index d1491ff9af9..a0c5cbb9089 100644 --- a/apps/sim/lib/copilot/tools/handlers/management/manage-custom-tool.ts +++ b/apps/sim/lib/copilot/tools/handlers/management/manage-custom-tool.ts @@ -1,15 +1,15 @@ -import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit' import { createLogger } from '@sim/logger' -import { getErrorMessage, toError } from '@sim/utils/errors' +import { toError } from '@sim/utils/errors' +import { executeCopilotCustomToolUseCase } from '@/lib/copilot/application/execute-custom-tool-use-case' import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types' -import { copilotToolCanWrite, copilotWriteDeniedMessage } from '@/lib/copilot/tools/permissions' -import { captureServerEvent } from '@/lib/posthog/server' +import { asOrchestrationError } from '@/lib/core/orchestration/types' import { - deleteCustomTool, - getCustomToolById, - listCustomTools, - upsertCustomTools, -} from '@/lib/workflows/custom-tools/operations' + deleteAvailableCustomToolUseCase, + listAvailableCustomToolsUseCase, + saveWorkspaceCustomToolUseCase, + updateAvailableCustomToolUseCase, +} from '@/lib/custom-tools/application/use-cases' +import { captureServerEvent } from '@/lib/posthog/server' const logger = createLogger('CopilotToolExecutor') @@ -53,20 +53,14 @@ export async function executeManageCustomTool( return { success: false, error: "Missing required 'operation' argument" } } - const writeOps: string[] = ['add', 'edit', 'delete'] - if (writeOps.includes(operation) && !copilotToolCanWrite(context.userPermission)) { - return { - success: false, - error: copilotWriteDeniedMessage('manage_custom_tool', operation, context.userPermission), - } - } - try { if (operation === 'list') { - const toolsForUser = await listCustomTools({ - userId: context.userId, - workspaceId, - }) + if (!workspaceId) return { success: false, error: 'workspaceId is required' } + const { tools: toolsForUser } = await executeCopilotCustomToolUseCase( + context, + listAvailableCustomToolsUseCase, + { workspaceId } + ) return { success: true, @@ -98,45 +92,37 @@ export async function executeManageCustomTool( return { success: false, error: "Missing tool title or schema.function.name for 'add'" } } - const resultTools = await upsertCustomTools({ - tools: [{ title, schema: params.schema, code: params.code }], - workspaceId, - userId: context.userId, - }) - const created = resultTools.find((tool) => tool.title === title) - - recordAudit({ - workspaceId, - actorId: context.userId, - action: AuditAction.CUSTOM_TOOL_CREATED, - resourceType: AuditResourceType.CUSTOM_TOOL, - resourceId: created?.id, - resourceName: title, - description: `Created custom tool "${title}"`, - metadata: { source: 'tool_input' }, - }) - if (created?.id) { - captureServerEvent( - context.userId, - 'custom_tool_saved', - { - tool_id: created.id, - workspace_id: workspaceId, - tool_name: title, - source: 'tool_input', - }, - { groups: { workspace: workspaceId } } - ) - } + const { tool: created } = await executeCopilotCustomToolUseCase( + context, + saveWorkspaceCustomToolUseCase, + { + title, + schema: params.schema, + code: params.code, + source: 'tool_input', + workspaceId, + } + ) + captureServerEvent( + context.userId, + 'custom_tool_saved', + { + tool_id: created.id, + workspace_id: workspaceId, + tool_name: created.title, + source: 'tool_input', + }, + { groups: { workspace: workspaceId } } + ) return { success: true, output: { success: true, operation, - toolId: created?.id, - title, - message: `Created custom tool "${title}"`, + toolId: created.id, + title: created.title, + message: `Created custom tool "${created.title}"`, }, } } @@ -158,42 +144,25 @@ export async function executeManageCustomTool( } } - const existing = await getCustomToolById({ - toolId: params.toolId, - userId: context.userId, - workspaceId, - }) - if (!existing) { - return { success: false, error: `Custom tool not found: ${params.toolId}` } - } - - const mergedSchema = params.schema || (existing.schema as ManageCustomToolSchema) - const mergedCode = params.code || existing.code - const title = params.title || mergedSchema.function?.name || existing.title - - await upsertCustomTools({ - tools: [{ id: params.toolId, title, schema: mergedSchema, code: mergedCode }], - workspaceId, - userId: context.userId, - }) - - recordAudit({ - workspaceId, - actorId: context.userId, - action: AuditAction.CUSTOM_TOOL_UPDATED, - resourceType: AuditResourceType.CUSTOM_TOOL, - resourceId: params.toolId, - resourceName: title, - description: `Updated custom tool "${title}"`, - metadata: { source: 'tool_input' }, - }) + const { tool } = await executeCopilotCustomToolUseCase( + context, + updateAvailableCustomToolUseCase, + { + workspaceId, + toolId: params.toolId, + title: params.title || params.schema?.function?.name, + schema: params.schema, + code: params.code, + source: 'tool_input', + } + ) captureServerEvent( context.userId, 'custom_tool_saved', { - tool_id: params.toolId, + tool_id: tool.id, workspace_id: workspaceId, - tool_name: title, + tool_name: tool.title, source: 'tool_input', }, { groups: { workspace: workspaceId } } @@ -204,9 +173,9 @@ export async function executeManageCustomTool( output: { success: true, operation, - toolId: params.toolId, - title, - message: `Updated custom tool "${title}"`, + toolId: tool.id, + title: tool.title, + message: `Updated custom tool "${tool.title}"`, }, } } @@ -216,41 +185,35 @@ export async function executeManageCustomTool( if (toolIds.length === 0) { return { success: false, error: "'toolId' or 'toolIds' is required for operation 'delete'" } } - + if (!workspaceId) return { success: false, error: 'workspaceId is required' } const deleted: string[] = [] const notFound: string[] = [] for (const toolId of toolIds) { - const result = await deleteCustomTool({ - toolId, - userId: context.userId, - workspaceId, - }) - if (result) { + try { + await executeCopilotCustomToolUseCase(context, deleteAvailableCustomToolUseCase, { + toolId, + workspaceId, + source: 'tool_input', + }) deleted.push(toolId) - } else { - notFound.push(toolId) + } catch (error) { + const classified = asOrchestrationError(error) + if (classified?.code === 'not_found') { + notFound.push(toolId) + continue + } + throw error } } for (const toolId of deleted) { - recordAudit({ - workspaceId: workspaceId ?? null, - actorId: context.userId, - action: AuditAction.CUSTOM_TOOL_DELETED, - resourceType: AuditResourceType.CUSTOM_TOOL, - resourceId: toolId, - description: 'Deleted custom tool', - metadata: { source: 'tool_input' }, - }) - if (workspaceId) { - captureServerEvent( - context.userId, - 'custom_tool_deleted', - { tool_id: toolId, workspace_id: workspaceId, source: 'tool_input' }, - { groups: { workspace: workspaceId } } - ) - } + captureServerEvent( + context.userId, + 'custom_tool_deleted', + { tool_id: toolId, workspace_id: workspaceId, source: 'tool_input' }, + { groups: { workspace: workspaceId } } + ) } return { @@ -281,9 +244,13 @@ export async function executeManageCustomTool( error: toError(error).message, } ) + const classified = asOrchestrationError(error) return { success: false, - error: getErrorMessage(error, 'Failed to manage custom tool'), + error: + classified && classified.code !== 'internal' + ? classified.message + : 'Failed to manage custom tool', } } } diff --git a/apps/sim/lib/copilot/tools/handlers/management/manage-mcp-tool.ts b/apps/sim/lib/copilot/tools/handlers/management/manage-mcp-tool.ts index c13ba02c76e..5158176f27c 100644 --- a/apps/sim/lib/copilot/tools/handlers/management/manage-mcp-tool.ts +++ b/apps/sim/lib/copilot/tools/handlers/management/manage-mcp-tool.ts @@ -1,15 +1,15 @@ -import { db } from '@sim/db' -import { mcpServers } from '@sim/db/schema' import { createLogger } from '@sim/logger' -import { getErrorMessage, toError } from '@sim/utils/errors' -import { and, eq, isNull } from 'drizzle-orm' +import { toError } from '@sim/utils/errors' +import { executeCopilotMcpServerUseCase } from '@/lib/copilot/application/execute-mcp-server-use-case' import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types' -import { copilotToolCanWrite, copilotWriteDeniedMessage } from '@/lib/copilot/tools/permissions' +import { asOrchestrationError } from '@/lib/core/orchestration/types' import { - performCreateMcpServer, - performDeleteMcpServer, - performUpdateMcpServer, -} from '@/lib/mcp/orchestration' + deleteMcpServerUseCase, + listMcpServersUseCase, + reconfigureMcpServerUseCase, + registerMcpServerUseCase, +} from '@/lib/mcp/application/use-cases' +import { captureServerEvent } from '@/lib/posthog/server' const logger = createLogger('CopilotToolExecutor') @@ -46,20 +46,11 @@ export async function executeManageMcpTool( return { success: false, error: 'workspaceId is required' } } - const writeOps: string[] = ['add', 'edit', 'delete'] - if (writeOps.includes(operation) && !copilotToolCanWrite(context.userPermission)) { - return { - success: false, - error: copilotWriteDeniedMessage('manage_mcp_tool', operation, context.userPermission), - } - } - try { if (operation === 'list') { - const servers = await db - .select() - .from(mcpServers) - .where(and(eq(mcpServers.workspaceId, workspaceId), isNull(mcpServers.deletedAt))) + const { servers } = await executeCopilotMcpServerUseCase(context, listMcpServersUseCase, { + workspaceId, + }) return { success: true, @@ -85,9 +76,8 @@ export async function executeManageMcpTool( return { success: false, error: "config.name and config.url are required for 'add'" } } - const result = await performCreateMcpServer({ + const result = await executeCopilotMcpServerUseCase(context, registerMcpServerUseCase, { workspaceId, - userId: context.userId, name: config.name, description: '', transport: config.transport || 'streamable-http', @@ -98,11 +88,21 @@ export async function executeManageMcpTool( enabled: config.enabled, source: 'tool_input', }) - if (!result.success || !result.serverId) { - return { - success: false, - error: result.error || `Failed to add MCP server "${config.name}"`, - } + if (!result.updated) { + captureServerEvent( + context.userId, + 'mcp_server_connected', + { + workspace_id: workspaceId, + server_name: result.server.name, + transport: result.server.transport, + source: 'tool_input', + }, + { + groups: { workspace: workspaceId }, + setOnce: { first_mcp_connected_at: new Date().toISOString() }, + } + ) } return { @@ -110,7 +110,7 @@ export async function executeManageMcpTool( output: { success: true, operation, - serverId: result.serverId, + serverId: result.server.id, name: config.name, message: result.updated ? `Updated existing MCP server "${config.name}"` @@ -128,9 +128,8 @@ export async function executeManageMcpTool( return { success: false, error: "'config' is required for 'edit'" } } - const result = await performUpdateMcpServer({ + const result = await executeCopilotMcpServerUseCase(context, reconfigureMcpServerUseCase, { workspaceId, - userId: context.userId, serverId: params.serverId, name: config.name, transport: config.transport, @@ -138,10 +137,8 @@ export async function executeManageMcpTool( headers: config.headers, timeout: config.timeout, enabled: config.enabled, + source: 'tool_input', }) - if (!result.success || !result.server) { - return { success: false, error: `MCP server not found: ${params.serverId}` } - } return { success: true, @@ -160,15 +157,21 @@ export async function executeManageMcpTool( return { success: false, error: "'serverId' is required for 'delete'" } } - const result = await performDeleteMcpServer({ + const result = await executeCopilotMcpServerUseCase(context, deleteMcpServerUseCase, { workspaceId, - userId: context.userId, serverId: params.serverId, source: 'tool_input', }) - if (!result.success || !result.server) { - return { success: false, error: `MCP server not found: ${params.serverId}` } - } + captureServerEvent( + context.userId, + 'mcp_server_disconnected', + { + workspace_id: workspaceId, + server_name: result.server.name, + source: 'tool_input', + }, + { groups: { workspace: workspaceId } } + ) return { success: true, @@ -193,9 +196,13 @@ export async function executeManageMcpTool( error: toError(error).message, } ) + const classified = asOrchestrationError(error) return { success: false, - error: getErrorMessage(error, 'Failed to manage MCP server'), + error: + classified && classified.code !== 'internal' + ? classified.message + : 'Failed to manage MCP server', } } } diff --git a/apps/sim/lib/copilot/tools/handlers/management/manage-skill.ts b/apps/sim/lib/copilot/tools/handlers/management/manage-skill.ts index c68debf0af0..d17bac0bfd6 100644 --- a/apps/sim/lib/copilot/tools/handlers/management/manage-skill.ts +++ b/apps/sim/lib/copilot/tools/handlers/management/manage-skill.ts @@ -1,13 +1,15 @@ import { createLogger } from '@sim/logger' -import { getErrorMessage, toError } from '@sim/utils/errors' +import { toError } from '@sim/utils/errors' +import { executeCopilotSkillUseCase } from '@/lib/copilot/application/execute-skill-use-case' import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types' -import { copilotToolCanWrite, copilotWriteDeniedMessage } from '@/lib/copilot/tools/permissions' +import { asOrchestrationError } from '@/lib/core/orchestration/types' +import { captureServerEvent } from '@/lib/posthog/server' import { - performCreateSkill, - performDeleteSkill, - performUpdateSkill, -} from '@/lib/skills/orchestration' -import { listSkillsForUser } from '@/lib/workflows/skills/operations' + createSkillUseCase, + deleteSkillUseCase, + listAvailableSkillsUseCase, + updateSkillUseCase, +} from '@/lib/skills/application/use-cases' const logger = createLogger('CopilotToolExecutor') @@ -37,18 +39,11 @@ export async function executeManageSkill( return { success: false, error: 'workspaceId is required' } } - // Workspace write gates only creation; edits and deletes are gated per skill - // below (skill editor — explicit editor row or derived workspace admin). - if (operation === 'add' && !copilotToolCanWrite(context.userPermission)) { - return { - success: false, - error: copilotWriteDeniedMessage('manage_skill', operation, context.userPermission), - } - } - try { if (operation === 'list') { - const skills = await listSkillsForUser({ workspaceId, userId: context.userId }) + const { skills } = await executeCopilotSkillUseCase(context, listAvailableSkillsUseCase, { + workspaceId, + }) return { success: true, @@ -74,26 +69,33 @@ export async function executeManageSkill( } } - const result = await performCreateSkill({ + const { skill } = await executeCopilotSkillUseCase(context, createSkillUseCase, { workspaceId, - userId: context.userId, name: params.name, description: params.description, content: params.content, source: 'tool_input', }) - if (!result.success || !result.skill) { - return { success: false, error: result.error ?? 'Failed to create skill' } - } + captureServerEvent( + context.userId, + 'skill_created', + { + skill_id: skill.id, + skill_name: skill.name, + workspace_id: workspaceId, + source: 'tool_input', + }, + { groups: { workspace: workspaceId } } + ) return { success: true, output: { success: true, operation, - skillId: result.skill.id, - name: result.skill.name, - message: `Created skill "${result.skill.name}"`, + skillId: skill.id, + name: skill.name, + message: `Created skill "${skill.name}"`, }, } } @@ -109,28 +111,34 @@ export async function executeManageSkill( } } - // Partial update: omitted fields keep their current values server-side. - const result = await performUpdateSkill({ + const { skill } = await executeCopilotSkillUseCase(context, updateSkillUseCase, { workspaceId, - userId: context.userId, skillId: params.skillId, ...(params.name ? { name: params.name } : {}), ...(params.description ? { description: params.description } : {}), ...(params.content ? { content: params.content } : {}), source: 'tool_input', }) - if (!result.success || !result.skill) { - return { success: false, error: result.error ?? 'Failed to update skill' } - } + captureServerEvent( + context.userId, + 'skill_updated', + { + skill_id: skill.id, + skill_name: skill.name, + workspace_id: workspaceId, + source: 'tool_input', + }, + { groups: { workspace: workspaceId } } + ) return { success: true, output: { success: true, operation, - skillId: result.skill.id, - name: result.skill.name, - message: `Updated skill "${result.skill.name}"`, + skillId: skill.id, + name: skill.name, + message: `Updated skill "${skill.name}"`, }, } } @@ -140,22 +148,24 @@ export async function executeManageSkill( return { success: false, error: "'skillId' is required for 'delete'" } } - const result = await performDeleteSkill({ + const { skill } = await executeCopilotSkillUseCase(context, deleteSkillUseCase, { workspaceId, - userId: context.userId, skillId: params.skillId, source: 'tool_input', }) - if (!result.success) { - return { success: false, error: result.error ?? 'Failed to delete skill' } - } + captureServerEvent( + context.userId, + 'skill_deleted', + { skill_id: skill.id, workspace_id: workspaceId, source: 'tool_input' }, + { groups: { workspace: workspaceId } } + ) return { success: true, output: { success: true, operation, - skillId: params.skillId, + skillId: skill.id, message: 'Deleted skill', }, } @@ -173,9 +183,13 @@ export async function executeManageSkill( error: toError(error).message, } ) + const classified = asOrchestrationError(error) return { success: false, - error: getErrorMessage(error, 'Failed to manage skill'), + error: + classified && classified.code !== 'internal' + ? classified.message + : 'Failed to manage skill', } } } diff --git a/apps/sim/lib/custom-tools/application/authorization.ts b/apps/sim/lib/custom-tools/application/authorization.ts new file mode 100644 index 00000000000..84e65bdf921 --- /dev/null +++ b/apps/sim/lib/custom-tools/application/authorization.ts @@ -0,0 +1,13 @@ +import type { DelegatedPrincipal } from '@sim/auth/principal' +import type { WorkspaceDelegationPolicy } from '@/lib/core/application' + +export const CUSTOM_TOOL_DELEGATION_AUDIENCE = 'sim:custom-tools' + +export const customToolDelegationPolicy = { + audience: CUSTOM_TOOL_DELEGATION_AUDIENCE, + isWithinScope: (principal: DelegatedPrincipal) => principal.serviceId === 'copilot', +} as const satisfies WorkspaceDelegationPolicy<{ + workspaceId: string + workspaceOrganizationId: string | null + allowPersonalApiKeys: boolean +}> diff --git a/apps/sim/lib/custom-tools/application/operations.ts b/apps/sim/lib/custom-tools/application/operations.ts new file mode 100644 index 00000000000..eff480f5782 --- /dev/null +++ b/apps/sim/lib/custom-tools/application/operations.ts @@ -0,0 +1,68 @@ +import { defineWorkspaceOperation } from '@/lib/core/application' + +const ALL_PRINCIPAL_KINDS = [ + 'session', + 'personal_api_key', + 'workspace_api_key', + 'delegated', +] as const +const HUMAN_PRINCIPAL_KINDS = ['session', 'personal_api_key', 'delegated'] as const + +export const customToolOperations = { + list: defineWorkspaceOperation({ + id: 'custom_tools.list', + minimumRole: 'read', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + listAvailable: defineWorkspaceOperation({ + id: 'custom_tools.list_available', + minimumRole: 'read', + workspaceApiKey: 'deny', + principalKinds: HUMAN_PRINCIPAL_KINDS, + }), + read: defineWorkspaceOperation({ + id: 'custom_tools.read', + minimumRole: 'read', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + create: defineWorkspaceOperation({ + id: 'custom_tools.create', + minimumRole: 'write', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + save: defineWorkspaceOperation({ + id: 'custom_tools.save', + minimumRole: 'write', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + update: defineWorkspaceOperation({ + id: 'custom_tools.update', + minimumRole: 'write', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + updateAvailable: defineWorkspaceOperation({ + id: 'custom_tools.update_available', + minimumRole: 'write', + workspaceApiKey: 'deny', + principalKinds: HUMAN_PRINCIPAL_KINDS, + }), + delete: defineWorkspaceOperation({ + id: 'custom_tools.delete', + minimumRole: 'write', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + deleteAvailable: defineWorkspaceOperation({ + id: 'custom_tools.delete_available', + minimumRole: 'write', + workspaceApiKey: 'deny', + principalKinds: HUMAN_PRINCIPAL_KINDS, + }), +} as const + +export type CustomToolOperation = (typeof customToolOperations)[keyof typeof customToolOperations] diff --git a/apps/sim/lib/custom-tools/application/use-cases.test.ts b/apps/sim/lib/custom-tools/application/use-cases.test.ts new file mode 100644 index 00000000000..fb14babae18 --- /dev/null +++ b/apps/sim/lib/custom-tools/application/use-cases.test.ts @@ -0,0 +1,187 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mocks } = vi.hoisted(() => ({ + mocks: { + loadContext: vi.fn(), + resolvePermission: vi.fn(), + getByTitle: vi.fn(), + upsert: vi.fn(), + audit: vi.fn(), + }, +})) + +vi.mock('@/lib/uploads/contexts/workspace', () => ({ + loadActiveWorkspaceContext: mocks.loadContext, +})) +vi.mock('@sim/platform-authz/workspace', () => ({ + permissionSatisfies: (actual: string | null, required: string) => + actual === 'admin' || actual === required || (actual === 'write' && required === 'read'), + resolveEffectiveWorkspacePermission: mocks.resolvePermission, +})) +vi.mock('@sim/audit', () => ({ + AuditAction: { + CUSTOM_TOOL_CREATED: 'custom_tool.created', + CUSTOM_TOOL_UPDATED: 'custom_tool.updated', + CUSTOM_TOOL_DELETED: 'custom_tool.deleted', + }, + AuditResourceType: { CUSTOM_TOOL: 'custom_tool' }, + recordAudit: mocks.audit, +})) +vi.mock('@/lib/workflows/custom-tools/operations', () => ({ + deleteCustomTool: vi.fn(), + deleteWorkspaceCustomTool: vi.fn(), + getCustomToolById: vi.fn(), + getWorkspaceCustomTool: vi.fn(), + getWorkspaceCustomToolByTitle: mocks.getByTitle, + listCustomTools: vi.fn(), + listWorkspaceCustomTools: vi.fn(), + updateCustomTool: vi.fn(), + updateWorkspaceCustomTool: vi.fn(), + upsertCustomTools: mocks.upsert, +})) + +import { CUSTOM_TOOL_DELEGATION_AUDIENCE } from '@/lib/custom-tools/application/authorization' +import { + createWorkspaceCustomToolUseCase, + saveWorkspaceCustomToolUseCase, +} from '@/lib/custom-tools/application/use-cases' + +const workspace = { + workspaceId: 'workspace-1', + workspaceOrganizationId: null, + allowPersonalApiKeys: true, + billedAccountUserId: 'owner-1', +} +const tool = { + id: 'tool-1', + workspaceId: workspace.workspaceId, + userId: 'owner-1', + title: 'lookup_order', + schema: { type: 'function' }, + code: 'return 1', + createdAt: new Date('2026-01-01T00:00:00Z'), + updatedAt: new Date('2026-01-01T00:00:00Z'), +} + +describe('custom tool application use cases', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.loadContext.mockResolvedValue(workspace) + mocks.resolvePermission.mockResolvedValue('write') + mocks.getByTitle.mockResolvedValue(null) + mocks.upsert.mockResolvedValue([tool]) + }) + + it('uses compatibility attribution without impersonating a workspace-key audit actor', async () => { + const principal = { + kind: 'workspace_api_key' as const, + workspaceId: workspace.workspaceId, + keyId: 'workspace-key-1', + } + + const result = await createWorkspaceCustomToolUseCase.execute({ + principal, + input: { + workspaceId: workspace.workspaceId, + title: tool.title, + schema: tool.schema, + code: tool.code, + source: 'api', + }, + }) + + expect(result.tool.id).toBe(tool.id) + expect(mocks.resolvePermission).not.toHaveBeenCalled() + expect(mocks.upsert).toHaveBeenCalledWith({ + tools: [{ title: tool.title, schema: tool.schema, code: tool.code }], + workspaceId: workspace.workspaceId, + userId: workspace.billedAccountUserId, + }) + expect(mocks.audit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: null, + actorName: 'Workspace API key', + metadata: expect.objectContaining({ + operation: 'custom_tools.create', + actor: { + kind: 'workspace_api_key', + keyId: 'workspace-key-1', + workspaceId: workspace.workspaceId, + }, + }), + }) + ) + }) + + it('returns a typed conflict and does not audit a rejected create', async () => { + mocks.getByTitle.mockResolvedValueOnce(tool) + + await expect( + createWorkspaceCustomToolUseCase.execute({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { + workspaceId: workspace.workspaceId, + title: tool.title, + schema: tool.schema, + code: tool.code, + }, + }) + ).rejects.toMatchObject({ code: 'conflict' }) + + expect(mocks.upsert).not.toHaveBeenCalled() + expect(mocks.audit).not.toHaveBeenCalled() + }) + + it('normalizes the in-transaction duplicate-title error for strict creates', async () => { + mocks.upsert.mockRejectedValueOnce( + new Error(`A tool with the title "${tool.title}" already exists in this workspace`) + ) + + await expect( + createWorkspaceCustomToolUseCase.execute({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { + workspaceId: workspace.workspaceId, + title: tool.title, + schema: tool.schema, + code: tool.code, + }, + }) + ).rejects.toMatchObject({ code: 'conflict' }) + + expect(mocks.audit).not.toHaveBeenCalled() + }) + + it('normalizes the in-transaction duplicate-title error for compatibility saves', async () => { + mocks.upsert.mockRejectedValueOnce( + new Error(`A tool with the title "${tool.title}" already exists in this workspace`) + ) + + await expect( + saveWorkspaceCustomToolUseCase.execute({ + principal: { + kind: 'delegated', + serviceId: 'copilot', + subjectUserId: 'user-1', + workspaceId: workspace.workspaceId, + delegationId: 'delegation-1', + audience: CUSTOM_TOOL_DELEGATION_AUDIENCE, + issuedAt: new Date(Date.now() - 1_000), + expiresAt: new Date(Date.now() + 60_000), + }, + input: { + workspaceId: workspace.workspaceId, + title: tool.title, + schema: tool.schema, + code: tool.code, + source: 'tool_input', + }, + }) + ).rejects.toMatchObject({ code: 'conflict' }) + + expect(mocks.audit).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/lib/custom-tools/application/use-cases.ts b/apps/sim/lib/custom-tools/application/use-cases.ts new file mode 100644 index 00000000000..71a58139e83 --- /dev/null +++ b/apps/sim/lib/custom-tools/application/use-cases.ts @@ -0,0 +1,385 @@ +import { AuditAction, AuditResourceType } from '@sim/audit' +import { type Principal, resolvePrincipalAttribution } from '@sim/auth/principal' +import type { customTools } from '@sim/db/schema' +import { getErrorMessage, 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 { customToolDelegationPolicy } from '@/lib/custom-tools/application/authorization' +import { customToolOperations } from '@/lib/custom-tools/application/operations' +import { loadActiveWorkspaceContext } from '@/lib/uploads/contexts/workspace' +import { + type CustomToolSortBy, + deleteCustomTool, + deleteWorkspaceCustomTool, + getCustomToolById, + getWorkspaceCustomTool, + getWorkspaceCustomToolByTitle, + listCustomTools, + listWorkspaceCustomTools, + updateCustomTool, + updateWorkspaceCustomTool, + upsertCustomTools, +} from '@/lib/workflows/custom-tools/operations' + +type CustomToolRow = typeof customTools.$inferSelect +type CustomToolWriteSource = 'api' | 'settings' | 'tool_input' + +interface CustomToolWorkspaceContext { + workspaceId: string + workspaceOrganizationId: string | null + allowPersonalApiKeys: boolean + billedAccountUserId: string +} + +interface CustomToolContext extends CustomToolWorkspaceContext { + tool: CustomToolRow +} + +async function resolveWorkspaceContext(workspaceId: string): Promise { + const context = await loadActiveWorkspaceContext(workspaceId) + if (!context) throw new OrchestrationError('not_found', 'Workspace not found') + return context +} + +async function resolveWorkspaceToolContext( + workspaceId: string, + toolId: string +): Promise { + const workspace = await resolveWorkspaceContext(workspaceId) + const tool = await getWorkspaceCustomTool({ workspaceId: workspace.workspaceId, toolId }) + if (!tool) throw new OrchestrationError('not_found', 'Custom tool not found') + return { ...workspace, tool } +} + +function humanUserId(principal: Exclude): string { + return principal.kind === 'delegated' ? principal.subjectUserId : principal.userId +} + +async function resolveAvailableToolContext(args: { + principal: Exclude + workspaceId: string + toolId: string +}): Promise { + const workspace = await resolveWorkspaceContext(args.workspaceId) + const tool = await getCustomToolById({ + toolId: args.toolId, + userId: humanUserId(args.principal), + workspaceId: workspace.workspaceId, + }) + if (!tool) throw new OrchestrationError('not_found', 'Custom tool not found') + return { ...workspace, tool } +} + +function customToolConflict(error: unknown): never { + if (getPostgresErrorCode(error) === '23505') { + throw new OrchestrationError( + 'conflict', + 'A custom tool with that title already exists in this workspace' + ) + } + const message = getErrorMessage(error, '') + if (/already exists in this workspace/i.test(message)) { + throw new OrchestrationError('conflict', message) + } + throw error +} + +const authorizationOptions = { delegation: customToolDelegationPolicy } + +export interface ListWorkspaceCustomToolsInput { + workspaceId: string + search?: string + sortBy?: CustomToolSortBy + sortOrder?: ListSortOrder +} + +export const listWorkspaceCustomToolsUseCase = defineAuthorizedWorkspaceUseCase({ + operation: customToolOperations.list, + resolveContext: ({ input }: { input: ListWorkspaceCustomToolsInput }) => + resolveWorkspaceContext(input.workspaceId), + authorizationOptions, + async execute({ input, context }) { + const tools = await listWorkspaceCustomTools({ ...input, workspaceId: context.workspaceId }) + return { tools } + }, +}) + +export interface ListAvailableCustomToolsInput { + workspaceId: string +} + +export const listAvailableCustomToolsUseCase = defineAuthorizedWorkspaceUseCase({ + operation: customToolOperations.listAvailable, + resolveContext: ({ input }: { input: ListAvailableCustomToolsInput }) => + resolveWorkspaceContext(input.workspaceId), + authorizationOptions, + async execute({ principal, context }) { + const tools = await listCustomTools({ + userId: humanUserId(principal), + workspaceId: context.workspaceId, + }) + return { tools } + }, +}) + +export interface GetWorkspaceCustomToolInput { + workspaceId: string + toolId: string +} + +export const getWorkspaceCustomToolUseCase = defineAuthorizedWorkspaceUseCase({ + operation: customToolOperations.read, + resolveContext: ({ input }: { input: GetWorkspaceCustomToolInput }) => + resolveWorkspaceToolContext(input.workspaceId, input.toolId), + authorizationOptions, + async execute({ context }) { + return { tool: context.tool } + }, +}) + +export interface CreateWorkspaceCustomToolInput { + workspaceId: string + title: string + schema: unknown + code: string + source?: CustomToolWriteSource +} + +export const createWorkspaceCustomToolUseCase = defineAuthorizedWorkspaceUseCase({ + operation: customToolOperations.create, + resolveContext: ({ input }: { input: CreateWorkspaceCustomToolInput }) => + resolveWorkspaceContext(input.workspaceId), + authorizationOptions, + async execute({ principal, input, context }) { + if ( + await getWorkspaceCustomToolByTitle({ workspaceId: context.workspaceId, title: input.title }) + ) { + throw new OrchestrationError( + 'conflict', + `A custom tool titled "${input.title}" already exists in this workspace` + ) + } + + const attribution = resolvePrincipalAttribution(principal, { + workspaceBillingOwnerUserId: context.billedAccountUserId, + }) + try { + const tools = await upsertCustomTools({ + tools: [{ title: input.title, schema: input.schema, code: input.code }], + workspaceId: context.workspaceId, + userId: attribution.attributedUserId, + }) + const tool = tools.find((candidate) => candidate.title === input.title) + if (!tool) throw new Error(`Custom tool "${input.title}" missing after a successful write`) + return { tool } + } catch (error) { + return customToolConflict(error) + } + }, + projectAudit: ({ input, result }) => ({ + action: AuditAction.CUSTOM_TOOL_CREATED, + resourceType: AuditResourceType.CUSTOM_TOOL, + resourceId: result.tool.id, + resourceName: result.tool.title, + description: `Created custom tool "${result.tool.title}"`, + metadata: { source: input.source }, + }), +}) + +export const saveWorkspaceCustomToolUseCase = defineAuthorizedWorkspaceUseCase({ + operation: customToolOperations.save, + resolveContext: ({ input }: { input: CreateWorkspaceCustomToolInput }) => + resolveWorkspaceContext(input.workspaceId), + authorizationOptions, + async execute({ principal, input, context }) { + const attribution = resolvePrincipalAttribution(principal, { + workspaceBillingOwnerUserId: context.billedAccountUserId, + }) + try { + const tools = await upsertCustomTools({ + tools: [{ title: input.title, schema: input.schema, code: input.code }], + workspaceId: context.workspaceId, + userId: attribution.attributedUserId, + }) + const tool = tools.find((candidate) => candidate.title === input.title) + if (!tool) throw new Error(`Custom tool "${input.title}" missing after a successful save`) + return { tool } + } catch (error) { + return customToolConflict(error) + } + }, + projectAudit: ({ input, result }) => ({ + action: AuditAction.CUSTOM_TOOL_CREATED, + resourceType: AuditResourceType.CUSTOM_TOOL, + resourceId: result.tool.id, + resourceName: result.tool.title, + description: `Created custom tool "${result.tool.title}"`, + metadata: { source: input.source }, + }), +}) + +interface UpdateCustomToolFields { + title?: string + schema?: unknown + code?: string + source?: CustomToolWriteSource +} + +export interface UpdateWorkspaceCustomToolInput extends UpdateCustomToolFields { + workspaceId: string + toolId: string +} + +async function ensureTitleAvailable(context: CustomToolContext, title: string): Promise { + if (title === context.tool.title) return + if (context.tool.workspaceId === null) return + const collision = await getWorkspaceCustomToolByTitle({ + workspaceId: context.workspaceId, + title, + }) + if (collision && collision.id !== context.tool.id) { + throw new OrchestrationError( + 'conflict', + `A custom tool titled "${title}" already exists in this workspace` + ) + } +} + +export const updateWorkspaceCustomToolUseCase = defineAuthorizedWorkspaceUseCase({ + operation: customToolOperations.update, + resolveContext: ({ input }: { input: UpdateWorkspaceCustomToolInput }) => + resolveWorkspaceToolContext(input.workspaceId, input.toolId), + authorizationOptions, + async execute({ input, context }) { + const title = input.title ?? context.tool.title + await ensureTitleAvailable(context, title) + try { + const tool = await updateWorkspaceCustomTool({ + workspaceId: context.workspaceId, + toolId: context.tool.id, + title, + schema: input.schema ?? context.tool.schema, + code: input.code ?? context.tool.code, + }) + if (!tool) throw new OrchestrationError('not_found', 'Custom tool not found') + return { tool } + } catch (error) { + return customToolConflict(error) + } + }, + projectAudit: ({ input, result }) => ({ + action: AuditAction.CUSTOM_TOOL_UPDATED, + resourceType: AuditResourceType.CUSTOM_TOOL, + resourceId: result.tool.id, + resourceName: result.tool.title, + description: `Updated custom tool "${result.tool.title}"`, + metadata: { source: input.source }, + }), +}) + +export const updateAvailableCustomToolUseCase = defineAuthorizedWorkspaceUseCase({ + operation: customToolOperations.updateAvailable, + resolveContext: ({ + principal, + input, + }: { + principal: Exclude + input: UpdateWorkspaceCustomToolInput + }) => + resolveAvailableToolContext({ + principal, + workspaceId: input.workspaceId, + toolId: input.toolId, + }), + authorizationOptions, + async execute({ principal, input, context }) { + const title = input.title ?? context.tool.title + await ensureTitleAvailable(context, title) + try { + const tool = await updateCustomTool({ + workspaceId: context.workspaceId, + toolId: context.tool.id, + userId: humanUserId(principal), + title, + schema: input.schema ?? context.tool.schema, + code: input.code ?? context.tool.code, + }) + if (!tool) throw new OrchestrationError('not_found', 'Custom tool not found') + return { tool } + } catch (error) { + return customToolConflict(error) + } + }, + projectAudit: ({ input, result }) => ({ + action: AuditAction.CUSTOM_TOOL_UPDATED, + resourceType: AuditResourceType.CUSTOM_TOOL, + resourceId: result.tool.id, + resourceName: result.tool.title, + description: `Updated custom tool "${result.tool.title}"`, + metadata: { source: input.source }, + }), +}) + +export interface DeleteWorkspaceCustomToolInput { + workspaceId: string + toolId: string + source?: CustomToolWriteSource +} + +export const deleteWorkspaceCustomToolUseCase = defineAuthorizedWorkspaceUseCase({ + operation: customToolOperations.delete, + resolveContext: ({ input }: { input: DeleteWorkspaceCustomToolInput }) => + resolveWorkspaceToolContext(input.workspaceId, input.toolId), + authorizationOptions, + async execute({ context }) { + const deleted = await deleteWorkspaceCustomTool({ + workspaceId: context.workspaceId, + toolId: context.tool.id, + }) + if (!deleted) throw new OrchestrationError('not_found', 'Custom tool not found') + return { tool: context.tool } + }, + projectAudit: ({ input, result }) => ({ + action: AuditAction.CUSTOM_TOOL_DELETED, + resourceType: AuditResourceType.CUSTOM_TOOL, + resourceId: result.tool.id, + resourceName: result.tool.title, + description: `Deleted custom tool "${result.tool.title}"`, + metadata: { source: input.source }, + }), +}) + +export const deleteAvailableCustomToolUseCase = defineAuthorizedWorkspaceUseCase({ + operation: customToolOperations.deleteAvailable, + resolveContext: ({ + principal, + input, + }: { + principal: Exclude + input: DeleteWorkspaceCustomToolInput + }) => + resolveAvailableToolContext({ + principal, + workspaceId: input.workspaceId, + toolId: input.toolId, + }), + authorizationOptions, + async execute({ principal, context }) { + const deleted = await deleteCustomTool({ + workspaceId: context.workspaceId, + toolId: context.tool.id, + userId: humanUserId(principal), + }) + if (!deleted) throw new OrchestrationError('not_found', 'Custom tool not found') + return { tool: context.tool } + }, + projectAudit: ({ input, result }) => ({ + action: AuditAction.CUSTOM_TOOL_DELETED, + resourceType: AuditResourceType.CUSTOM_TOOL, + resourceId: result.tool.id, + resourceName: result.tool.title, + description: `Deleted custom tool "${result.tool.title}"`, + metadata: { source: input.source }, + }), +}) diff --git a/apps/sim/lib/mcp/application/authorization.ts b/apps/sim/lib/mcp/application/authorization.ts new file mode 100644 index 00000000000..bd3c575a4d1 --- /dev/null +++ b/apps/sim/lib/mcp/application/authorization.ts @@ -0,0 +1,13 @@ +import type { DelegatedPrincipal } from '@sim/auth/principal' +import type { WorkspaceDelegationPolicy } from '@/lib/core/application' + +export const MCP_SERVER_DELEGATION_AUDIENCE = 'sim:mcp-servers' + +export const mcpServerDelegationPolicy = { + audience: MCP_SERVER_DELEGATION_AUDIENCE, + isWithinScope: (principal: DelegatedPrincipal) => principal.serviceId === 'copilot', +} as const satisfies WorkspaceDelegationPolicy<{ + workspaceId: string + workspaceOrganizationId: string | null + allowPersonalApiKeys: boolean +}> diff --git a/apps/sim/lib/mcp/application/operations.ts b/apps/sim/lib/mcp/application/operations.ts new file mode 100644 index 00000000000..b0a6dcbc28f --- /dev/null +++ b/apps/sim/lib/mcp/application/operations.ts @@ -0,0 +1,55 @@ +import { defineWorkspaceOperation } from '@/lib/core/application' + +const ALL_PRINCIPAL_KINDS = [ + 'session', + 'personal_api_key', + 'workspace_api_key', + 'delegated', +] as const + +export const mcpServerOperations = { + list: defineWorkspaceOperation({ + id: 'mcp_servers.list', + minimumRole: 'read', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + read: defineWorkspaceOperation({ + id: 'mcp_servers.read', + minimumRole: 'read', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + create: defineWorkspaceOperation({ + id: 'mcp_servers.create', + minimumRole: 'write', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + register: defineWorkspaceOperation({ + id: 'mcp_servers.register', + minimumRole: 'write', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + update: defineWorkspaceOperation({ + id: 'mcp_servers.update', + minimumRole: 'write', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + reconfigure: defineWorkspaceOperation({ + id: 'mcp_servers.reconfigure', + minimumRole: 'write', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + delete: defineWorkspaceOperation({ + id: 'mcp_servers.delete', + minimumRole: 'write', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), +} as const + +export type McpServerOperation = (typeof mcpServerOperations)[keyof typeof mcpServerOperations] diff --git a/apps/sim/lib/mcp/application/use-cases.test.ts b/apps/sim/lib/mcp/application/use-cases.test.ts new file mode 100644 index 00000000000..4cf037aa241 --- /dev/null +++ b/apps/sim/lib/mcp/application/use-cases.test.ts @@ -0,0 +1,163 @@ +/** + * @vitest-environment node + */ +import type { mcpServers } from '@sim/db/schema' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { events, mocks } = vi.hoisted(() => ({ + events: [] as string[], + mocks: { + loadContext: vi.fn(), + resolvePermission: vi.fn(), + idState: vi.fn(), + create: vi.fn(), + effects: vi.fn(), + audit: vi.fn(), + }, +})) + +vi.mock('@/lib/uploads/contexts/workspace', () => ({ + loadActiveWorkspaceContext: mocks.loadContext, +})) +vi.mock('@sim/platform-authz/workspace', () => ({ + permissionSatisfies: (actual: string | null, required: string) => + actual === 'admin' || actual === required || (actual === 'write' && required === 'read'), + resolveEffectiveWorkspacePermission: mocks.resolvePermission, +})) +vi.mock('@sim/audit', () => ({ + AuditAction: { + MCP_SERVER_ADDED: 'mcp_server.added', + MCP_SERVER_UPDATED: 'mcp_server.updated', + MCP_SERVER_REMOVED: 'mcp_server.removed', + }, + AuditResourceType: { MCP_SERVER: 'mcp_server' }, + recordAudit: mocks.audit, +})) +vi.mock('@/lib/mcp/orchestration', () => ({ + applyMcpServerMutationEffects: mocks.effects, + createMcpServer: mocks.create, + deleteMcpServer: vi.fn(), + updateMcpServer: vi.fn(), +})) +vi.mock('@/lib/mcp/queries', () => ({ + getMcpServerIdState: mocks.idState, + getWorkspaceMcpServer: vi.fn(), + listWorkspaceMcpServers: vi.fn(), +})) + +import { createMcpServerUseCase } from '@/lib/mcp/application/use-cases' + +type McpServerRow = typeof mcpServers.$inferSelect +const workspace = { + workspaceId: 'workspace-1', + workspaceOrganizationId: null, + allowPersonalApiKeys: true, + billedAccountUserId: 'owner-1', +} +const server = { + id: 'mcp-server-1', + workspaceId: workspace.workspaceId, + createdBy: 'owner-1', + name: 'Docs server', + description: null, + transport: 'streamable-http', + url: 'https://mcp.example.com/sse', + authType: 'headers', + oauthClientId: null, + oauthClientSecret: null, + headers: {}, + timeout: 30_000, + retries: 3, + enabled: true, + lastConnected: null, + connectionStatus: 'connected', + lastError: null, + statusConfig: {}, + toolCount: 0, + lastToolsRefresh: null, + totalRequests: 0, + lastUsed: null, + deletedAt: null, + createdAt: new Date('2026-01-01T00:00:00Z'), + updatedAt: new Date('2026-01-02T00:00:00Z'), +} as McpServerRow + +describe('MCP server application use cases', () => { + beforeEach(() => { + vi.clearAllMocks() + events.length = 0 + mocks.loadContext.mockResolvedValue(workspace) + mocks.resolvePermission.mockResolvedValue('write') + mocks.idState.mockResolvedValue(null) + mocks.create.mockResolvedValue({ + success: true, + serverId: server.id, + server, + updated: false, + }) + mocks.audit.mockImplementation(() => events.push('audit')) + mocks.effects.mockImplementation(async () => events.push('effects')) + }) + + it('keeps strict creation, compatibility attribution, audit, and effects in order', async () => { + const principal = { + kind: 'workspace_api_key' as const, + workspaceId: workspace.workspaceId, + keyId: 'workspace-key-1', + } + + const result = await createMcpServerUseCase.execute({ + principal, + input: { + workspaceId: workspace.workspaceId, + name: server.name, + url: server.url, + source: 'api', + }, + }) + + expect(result.server.id).toBe(server.id) + expect(mocks.create).toHaveBeenCalledWith( + expect.objectContaining({ + workspaceId: workspace.workspaceId, + userId: workspace.billedAccountUserId, + existingServerBehavior: 'reject', + }) + ) + expect(events).toEqual(['audit', 'effects']) + expect(mocks.audit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: null, + metadata: expect.objectContaining({ operation: 'mcp_servers.create' }), + }) + ) + }) + + it('rejects an existing live URL before mutation and audit', async () => { + mocks.idState.mockResolvedValueOnce({ deleted: false }) + + await expect( + createMcpServerUseCase.execute({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { workspaceId: workspace.workspaceId, name: server.name, url: server.url }, + }) + ).rejects.toMatchObject({ code: 'conflict' }) + + expect(mocks.create).not.toHaveBeenCalled() + expect(mocks.audit).not.toHaveBeenCalled() + expect(mocks.effects).not.toHaveBeenCalled() + }) + + it('fails fast when a post-audit domain effect fails', async () => { + mocks.effects.mockRejectedValueOnce(new Error('cache unavailable')) + + await expect( + createMcpServerUseCase.execute({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { workspaceId: workspace.workspaceId, name: server.name, url: server.url }, + }) + ).rejects.toThrow('cache unavailable') + + expect(mocks.audit).toHaveBeenCalledOnce() + }) +}) diff --git a/apps/sim/lib/mcp/application/use-cases.ts b/apps/sim/lib/mcp/application/use-cases.ts new file mode 100644 index 00000000000..9137d97cbb0 --- /dev/null +++ b/apps/sim/lib/mcp/application/use-cases.ts @@ -0,0 +1,372 @@ +import { AuditAction, AuditResourceType } from '@sim/audit' +import { resolvePrincipalAttribution } from '@sim/auth/principal' +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 { mcpServerDelegationPolicy } from '@/lib/mcp/application/authorization' +import { mcpServerOperations } from '@/lib/mcp/application/operations' +import { + applyMcpServerMutationEffects, + createMcpServer, + deleteMcpServer, + type PerformMcpServerResult, + updateMcpServer as updateMcpServerRecord, +} from '@/lib/mcp/orchestration' +import { + getMcpServerIdState, + getWorkspaceMcpServer, + listWorkspaceMcpServers, + type McpServerRow, + type McpServerSortBy, +} from '@/lib/mcp/queries' +import type { McpAuthType } from '@/lib/mcp/types' +import { generateMcpServerId } from '@/lib/mcp/utils' +import { loadActiveWorkspaceContext } from '@/lib/uploads/contexts/workspace' + +type McpServerTransport = McpServerRow['transport'] +type McpWriteSource = 'api' | 'settings' | 'tool_input' + +interface McpWorkspaceContext { + workspaceId: string + workspaceOrganizationId: string | null + allowPersonalApiKeys: boolean + billedAccountUserId: string +} + +interface McpServerContext extends McpWorkspaceContext { + server: McpServerRow +} + +async function resolveWorkspaceContext(workspaceId: string): Promise { + const context = await loadActiveWorkspaceContext(workspaceId) + if (!context) throw new OrchestrationError('not_found', 'Workspace not found') + return context +} + +async function resolveServerContext( + workspaceId: string, + serverId: string +): Promise { + const workspace = await resolveWorkspaceContext(workspaceId) + const server = await getWorkspaceMcpServer({ workspaceId: workspace.workspaceId, serverId }) + if (!server) throw new OrchestrationError('not_found', 'MCP server not found') + return { ...workspace, server } +} + +function requireSuccessfulResult( + result: PerformMcpServerResult, + fallback: string +): PerformMcpServerResult & { server: McpServerRow } { + if (result.success && result.server) + return result as PerformMcpServerResult & { server: McpServerRow } + switch (result.errorCode) { + case 'not_found': + throw new OrchestrationError('not_found', 'MCP server not found') + case 'forbidden': + throw new OrchestrationError('forbidden', result.error ?? fallback) + case 'bad_gateway': + throw new OrchestrationError('validation', result.error ?? fallback) + case 'conflict': + throw new OrchestrationError('conflict', result.error ?? fallback) + default: + throw new Error(fallback) + } +} + +const authorizationOptions = { delegation: mcpServerDelegationPolicy } + +export interface ListMcpServersInput { + workspaceId: string + search?: string + sortBy?: McpServerSortBy + sortOrder?: ListSortOrder +} + +export const listMcpServersUseCase = defineAuthorizedWorkspaceUseCase({ + operation: mcpServerOperations.list, + resolveContext: ({ input }: { input: ListMcpServersInput }) => + resolveWorkspaceContext(input.workspaceId), + authorizationOptions, + async execute({ input, context }) { + const servers = await listWorkspaceMcpServers({ ...input, workspaceId: context.workspaceId }) + return { servers } + }, +}) + +export interface GetMcpServerInput { + workspaceId: string + serverId: string +} + +export const getMcpServerUseCase = defineAuthorizedWorkspaceUseCase({ + operation: mcpServerOperations.read, + resolveContext: ({ input }: { input: GetMcpServerInput }) => + resolveServerContext(input.workspaceId, input.serverId), + authorizationOptions, + async execute({ context }) { + return { server: context.server } + }, +}) + +export interface SaveMcpServerInput { + workspaceId: string + name: string + description?: string | null + transport?: McpServerTransport + url: string + headers?: Record + timeout?: number + retries?: number + enabled?: boolean + authType?: McpAuthType + oauthClientId?: string | null + oauthClientSecret?: string | null + source?: McpWriteSource +} + +async function saveMcpServer(args: { + principal: Parameters[0] + input: SaveMcpServerInput + context: McpWorkspaceContext + existingServerBehavior?: 'update' | 'reject' +}): Promise { + const attribution = resolvePrincipalAttribution(args.principal, { + workspaceBillingOwnerUserId: args.context.billedAccountUserId, + }) + const result = await createMcpServer({ + workspaceId: args.context.workspaceId, + userId: attribution.attributedUserId, + name: args.input.name, + description: args.input.description, + transport: args.input.transport, + url: args.input.url, + headers: args.input.headers, + timeout: args.input.timeout, + retries: args.input.retries, + enabled: args.input.enabled, + authType: args.input.authType, + oauthClientId: args.input.oauthClientId ?? null, + oauthClientIdProvided: args.input.oauthClientId !== undefined, + oauthClientSecret: args.input.oauthClientSecret, + oauthClientSecretProvided: args.input.oauthClientSecret !== undefined, + existingServerBehavior: args.existingServerBehavior, + }) + return requireSuccessfulResult(result, 'Failed to register MCP server') +} + +function createAudit( + input: SaveMcpServerInput, + result: PerformMcpServerResult & { server: McpServerRow } +) { + if (result.updated) return [] + return [ + { + action: AuditAction.MCP_SERVER_ADDED, + resourceType: AuditResourceType.MCP_SERVER, + resourceId: result.server.id, + resourceName: result.server.name, + description: `Added MCP server "${result.server.name}"`, + metadata: { + serverName: result.server.name, + transport: result.server.transport, + url: result.server.url, + timeout: result.server.timeout, + retries: result.server.retries, + source: input.source, + }, + }, + ] +} + +export const createMcpServerUseCase = defineAuthorizedWorkspaceUseCase({ + operation: mcpServerOperations.create, + resolveContext: ({ input }: { input: SaveMcpServerInput }) => + resolveWorkspaceContext(input.workspaceId), + authorizationOptions, + async execute({ principal, input, context }) { + const serverId = generateMcpServerId(context.workspaceId, input.url) + const idState = await getMcpServerIdState({ workspaceId: context.workspaceId, serverId }) + if (idState && !idState.deleted) { + throw new OrchestrationError( + 'conflict', + 'An MCP server with this URL already exists in this workspace. Update it with PATCH /api/v2/mcp-servers/{id}.' + ) + } + let result: PerformMcpServerResult & { server: McpServerRow } + try { + result = await saveMcpServer({ + principal, + input, + context, + existingServerBehavior: 'reject', + }) + } catch (error) { + if (getPostgresErrorCode(error) === '23505') { + throw new OrchestrationError( + 'conflict', + 'An MCP server with this URL already exists in this workspace.' + ) + } + throw error + } + if (result.updated && idState?.deleted !== true) { + throw new OrchestrationError( + 'conflict', + 'An MCP server with this URL already exists in this workspace.' + ) + } + return result + }, + projectAudit: ({ input, result }) => createAudit(input, result), + afterSuccess: ({ context, result }) => + applyMcpServerMutationEffects({ action: 'create', workspaceId: context.workspaceId, result }), +}) + +export const registerMcpServerUseCase = defineAuthorizedWorkspaceUseCase({ + operation: mcpServerOperations.register, + resolveContext: ({ input }: { input: SaveMcpServerInput }) => + resolveWorkspaceContext(input.workspaceId), + authorizationOptions, + execute: ({ principal, input, context }) => saveMcpServer({ principal, input, context }), + projectAudit: ({ input, result }) => createAudit(input, result), + afterSuccess: ({ context, result }) => + applyMcpServerMutationEffects({ action: 'create', workspaceId: context.workspaceId, result }), +}) + +export interface UpdateMcpServerInput { + workspaceId: string + serverId: string + name?: string + description?: string | null + transport?: McpServerTransport + url?: string + headers?: Record + timeout?: number + retries?: number + enabled?: boolean + authType?: McpAuthType + oauthClientId?: string | null + oauthClientSecret?: string | null + source?: McpWriteSource +} + +async function updateMcpServer(args: { + principal: Parameters[0] + input: UpdateMcpServerInput + context: McpServerContext +}): Promise { + const attribution = resolvePrincipalAttribution(args.principal, { + workspaceBillingOwnerUserId: args.context.billedAccountUserId, + }) + const result = await updateMcpServerRecord({ + workspaceId: args.context.workspaceId, + userId: attribution.attributedUserId, + serverId: args.context.server.id, + name: args.input.name, + description: args.input.description, + transport: args.input.transport, + url: args.input.url, + headers: args.input.headers, + timeout: args.input.timeout, + retries: args.input.retries, + enabled: args.input.enabled, + authType: args.input.authType, + oauthClientId: args.input.oauthClientId ?? null, + oauthClientIdProvided: args.input.oauthClientId !== undefined, + oauthClientSecret: args.input.oauthClientSecret, + oauthClientSecretProvided: args.input.oauthClientSecret !== undefined, + }) + return requireSuccessfulResult(result, 'Failed to update MCP server') +} + +function updateAudit( + input: UpdateMcpServerInput, + result: PerformMcpServerResult & { server: McpServerRow } +) { + return { + action: AuditAction.MCP_SERVER_UPDATED, + resourceType: AuditResourceType.MCP_SERVER, + resourceId: result.server.id, + resourceName: result.server.name, + description: `Updated MCP server "${result.server.name}"`, + metadata: { + serverName: result.server.name, + transport: result.server.transport, + url: result.server.url, + updatedFields: Object.keys(input).filter( + (key) => !['workspaceId', 'serverId', 'source'].includes(key) + ), + source: input.source, + }, + } +} + +export const updateMcpServerUseCase = defineAuthorizedWorkspaceUseCase({ + operation: mcpServerOperations.update, + resolveContext: ({ input }: { input: UpdateMcpServerInput }) => + resolveServerContext(input.workspaceId, input.serverId), + authorizationOptions, + async execute({ principal, input, context }) { + if (input.url !== undefined && input.url !== context.server.url) { + throw new OrchestrationError( + 'validation', + 'url cannot be changed: an MCP server’s id is derived from its URL. Delete this server and create one at the new URL.' + ) + } + return updateMcpServer({ principal, input, context }) + }, + projectAudit: ({ input, result }) => updateAudit(input, result), + afterSuccess: ({ context, result }) => + applyMcpServerMutationEffects({ action: 'update', workspaceId: context.workspaceId, result }), +}) + +export const reconfigureMcpServerUseCase = defineAuthorizedWorkspaceUseCase({ + operation: mcpServerOperations.reconfigure, + resolveContext: ({ input }: { input: UpdateMcpServerInput }) => + resolveServerContext(input.workspaceId, input.serverId), + authorizationOptions, + execute: ({ principal, input, context }) => updateMcpServer({ principal, input, context }), + projectAudit: ({ input, result }) => updateAudit(input, result), + afterSuccess: ({ context, result }) => + applyMcpServerMutationEffects({ action: 'update', workspaceId: context.workspaceId, result }), +}) + +export interface DeleteMcpServerInput { + workspaceId: string + serverId: string + source?: McpWriteSource +} + +export const deleteMcpServerUseCase = defineAuthorizedWorkspaceUseCase({ + operation: mcpServerOperations.delete, + resolveContext: ({ input }: { input: DeleteMcpServerInput }) => + resolveServerContext(input.workspaceId, input.serverId), + authorizationOptions, + async execute({ principal, input, context }) { + const attribution = resolvePrincipalAttribution(principal, { + workspaceBillingOwnerUserId: context.billedAccountUserId, + }) + const result = await deleteMcpServer({ + workspaceId: context.workspaceId, + userId: attribution.attributedUserId, + serverId: context.server.id, + }) + return requireSuccessfulResult(result, 'Failed to delete MCP server') + }, + projectAudit: ({ input, result }) => ({ + action: AuditAction.MCP_SERVER_REMOVED, + resourceType: AuditResourceType.MCP_SERVER, + resourceId: result.server.id, + resourceName: result.server.name, + description: `Removed MCP server "${result.server.name}"`, + metadata: { + serverName: result.server.name, + transport: result.server.transport, + url: result.server.url, + source: input.source, + }, + }), + afterSuccess: ({ context, result }) => + applyMcpServerMutationEffects({ action: 'delete', workspaceId: context.workspaceId, result }), +}) diff --git a/apps/sim/lib/mcp/orchestration/index.ts b/apps/sim/lib/mcp/orchestration/index.ts index 7fac0516176..dc2297c6a73 100644 --- a/apps/sim/lib/mcp/orchestration/index.ts +++ b/apps/sim/lib/mcp/orchestration/index.ts @@ -1,4 +1,8 @@ export { + applyMcpServerMutationEffects, + createMcpServer, + deleteMcpServer, + type McpServerMutationAction, type McpServerOrchestrationErrorCode, type PerformCreateMcpServerParams, type PerformDeleteMcpServerParams, @@ -7,6 +11,7 @@ export { performCreateMcpServer, performDeleteMcpServer, performUpdateMcpServer, + updateMcpServer, } from './server-lifecycle' export { type PerformCreateWorkflowMcpServerParams, diff --git a/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts b/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts index 29d746f3c7b..34b7fe2fbd8 100644 --- a/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts +++ b/apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts @@ -108,6 +108,7 @@ describe('MCP server lifecycle orchestration', () => { lastError: null, }) ) + expect(result.configurationChanged).toBe(true) expect(mockClearCache).toHaveBeenCalledWith('workspace-1') }) @@ -164,6 +165,16 @@ describe('MCP server lifecycle orchestration', () => { oauthClientSecret: 'secret-1', }, ]) + dbChainMockFns.limit.mockResolvedValueOnce([ + { + id: 'server-1', + workspaceId: 'workspace-1', + name: 'Example', + transport: 'streamable-http', + url: 'https://example.com/mcp', + authType: 'headers', + }, + ]) const result = await performCreateMcpServer({ workspaceId: 'workspace-1', diff --git a/apps/sim/lib/mcp/orchestration/server-lifecycle.ts b/apps/sim/lib/mcp/orchestration/server-lifecycle.ts index 2468900e386..c646cb0e5d2 100644 --- a/apps/sim/lib/mcp/orchestration/server-lifecycle.ts +++ b/apps/sim/lib/mcp/orchestration/server-lifecycle.ts @@ -21,7 +21,12 @@ import { captureServerEvent } from '@/lib/posthog/server' const logger = createLogger('McpServerOrchestration') -export type McpServerOrchestrationErrorCode = 'not_found' | 'forbidden' | 'bad_gateway' | 'internal' +export type McpServerOrchestrationErrorCode = + | 'not_found' + | 'forbidden' + | 'bad_gateway' + | 'conflict' + | 'internal' type McpServerTransport = (typeof mcpServers.$inferInsert)['transport'] @@ -48,6 +53,7 @@ export interface PerformCreateMcpServerParams extends ActorMetadata { oauthClientIdProvided?: boolean oauthClientSecret?: string | null oauthClientSecretProvided?: boolean + existingServerBehavior?: 'update' | 'reject' } export interface PerformUpdateMcpServerParams extends ActorMetadata { @@ -84,8 +90,11 @@ export interface PerformMcpServerResult { server?: typeof mcpServers.$inferSelect updated?: boolean authType?: McpAuthType + configurationChanged?: boolean } +export type McpServerMutationAction = 'create' | 'update' | 'delete' + type ValidateMcpServerUrlResult = | { ok: true; resolvedIP: string | null } | { ok: false; result: PerformMcpServerResult } @@ -109,8 +118,8 @@ async function validateMcpServerUrl(url: string): Promise ): Promise { const validation = await validateMcpServerUrl(params.url) if (!validation.ok) return validation.result @@ -144,6 +153,18 @@ export async function performCreateMcpServer( const urlChanged = existingServer ? existingServer.url !== params.url : true + if ( + existingServer && + existingServer.deletedAt === null && + params.existingServerBehavior === 'reject' + ) { + return { + success: false, + error: 'An MCP server with this URL already exists in this workspace', + errorCode: 'conflict', + } + } + let resolvedAuthType: McpAuthType = params.authType ?? 'headers' if (!params.authType) { if (existingServer && !urlChanged) { @@ -213,9 +234,13 @@ export async function performCreateMcpServer( await tx.update(mcpServers).set(updateValues).where(eq(mcpServers.id, serverId)) }) - await mcpService.clearCache(params.workspaceId) - await mcpService.evictServerConnections(serverId, 'config changed') - return { success: true, serverId, updated: true, authType: resolvedAuthType } + const [server] = await db + .select() + .from(mcpServers) + .where(and(eq(mcpServers.id, serverId), eq(mcpServers.workspaceId, params.workspaceId))) + .limit(1) + if (!server) throw new Error(`MCP server ${serverId} missing after a successful update`) + return { success: true, serverId, server, updated: true, authType: resolvedAuthType } } await db.insert(mcpServers).values({ @@ -239,61 +264,21 @@ export async function performCreateMcpServer( updatedAt: new Date(), }) - await mcpService.clearCache(params.workspaceId) - - try { - const { PlatformEvents } = await import('@/lib/core/telemetry') - PlatformEvents.mcpServerAdded({ - serverId, - serverName: params.name, - transport, - workspaceId: params.workspaceId, - }) - } catch {} - - const source = - params.source === 'settings' || params.source === 'tool_input' ? params.source : undefined - - captureServerEvent( - params.userId, - 'mcp_server_connected', - { workspace_id: params.workspaceId, server_name: params.name, transport, source }, - { - groups: { workspace: params.workspaceId }, - setOnce: { first_mcp_connected_at: new Date().toISOString() }, - } - ) - - recordAudit({ - workspaceId: params.workspaceId, - actorId: params.userId, - actorName: params.actorName ?? undefined, - actorEmail: params.actorEmail ?? undefined, - action: AuditAction.MCP_SERVER_ADDED, - resourceType: AuditResourceType.MCP_SERVER, - resourceId: serverId, - resourceName: params.name, - description: `Added MCP server "${params.name}"`, - metadata: { - serverName: params.name, - transport, - url: params.url, - timeout, - retries, - source, - }, - request: params.request, - }) - - return { success: true, serverId, updated: false, authType: resolvedAuthType } + const [server] = await db + .select() + .from(mcpServers) + .where(and(eq(mcpServers.id, serverId), eq(mcpServers.workspaceId, params.workspaceId))) + .limit(1) + if (!server) throw new Error(`MCP server ${serverId} missing after a successful insert`) + return { success: true, serverId, server, updated: false, authType: resolvedAuthType } } catch (error) { logger.error('Failed to create MCP server', { error }) - return { success: false, error: 'Failed to register MCP server', errorCode: 'internal' } + throw error } } -export async function performUpdateMcpServer( - params: PerformUpdateMcpServerParams +export async function updateMcpServer( + params: Omit ): Promise { if (params.url) { const validation = await validateMcpServerUrl(params.url) @@ -404,10 +389,103 @@ export async function performUpdateMcpServer( params.timeout !== undefined || params.retries !== undefined - if (shouldClearCache) { - await mcpService.clearCache(params.workspaceId) - await mcpService.evictServerConnections(params.serverId, 'config changed') + return { success: true, server, configurationChanged: shouldClearCache } + } catch (error) { + logger.error('Failed to update MCP server', { error }) + throw error + } +} + +export async function deleteMcpServer( + params: Omit +): Promise { + try { + await revokeMcpOauthTokens(params.serverId) + const [server] = await db + .delete(mcpServers) + .where( + and(eq(mcpServers.id, params.serverId), eq(mcpServers.workspaceId, params.workspaceId)) + ) + .returning() + + if (!server) return { success: false, error: 'Server not found', errorCode: 'not_found' } + + return { success: true, server } + } catch (error) { + logger.error('Failed to delete MCP server', { error }) + throw error + } +} + +function legacySource(source: string | undefined): 'settings' | 'tool_input' | undefined { + return source === 'settings' || source === 'tool_input' ? source : undefined +} + +/** Preserves the legacy internal registration result, analytics, audit, and effects contract. */ +export async function performCreateMcpServer( + params: PerformCreateMcpServerParams +): Promise { + try { + const result = await createMcpServer(params) + if (!result.success) return result + if (!result.server) throw new Error('Successful MCP registration is missing its server') + + await applyMcpServerMutationEffects({ + action: 'create', + workspaceId: params.workspaceId, + result, + }) + if (!result.updated) { + const source = legacySource(params.source) + captureServerEvent( + params.userId, + 'mcp_server_connected', + { + workspace_id: params.workspaceId, + server_name: result.server.name, + transport: result.server.transport, + source, + }, + { + groups: { workspace: params.workspaceId }, + setOnce: { first_mcp_connected_at: new Date().toISOString() }, + } + ) + recordAudit({ + workspaceId: params.workspaceId, + actorId: params.userId, + actorName: params.actorName ?? undefined, + actorEmail: params.actorEmail ?? undefined, + action: AuditAction.MCP_SERVER_ADDED, + resourceType: AuditResourceType.MCP_SERVER, + resourceId: result.server.id, + resourceName: result.server.name, + description: `Added MCP server "${result.server.name}"`, + metadata: { + serverName: result.server.name, + transport: result.server.transport, + url: result.server.url, + timeout: result.server.timeout, + retries: result.server.retries, + source, + }, + request: params.request, + }) } + return result + } catch (error) { + logger.error('Failed to register MCP server', { error }) + return { success: false, error: 'Failed to register MCP server', errorCode: 'internal' } + } +} + +/** Preserves the legacy internal update result, audit, and effects contract. */ +export async function performUpdateMcpServer( + params: PerformUpdateMcpServerParams +): Promise { + try { + const result = await updateMcpServer(params) + if (!result.success || !result.server) return result recordAudit({ workspaceId: params.workspaceId, @@ -416,51 +494,56 @@ export async function performUpdateMcpServer( actorEmail: params.actorEmail ?? undefined, action: AuditAction.MCP_SERVER_UPDATED, resourceType: AuditResourceType.MCP_SERVER, - resourceId: params.serverId, - resourceName: server.name || params.serverId, - description: `Updated MCP server "${server.name || params.serverId}"`, + resourceId: result.server.id, + resourceName: result.server.name, + description: `Updated MCP server "${result.server.name}"`, metadata: { - serverName: server.name, - transport: server.transport, - url: server.url, - updatedFields: Object.keys(updateData).filter((key) => key !== 'updatedAt'), + 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), }, request: params.request, }) - - return { success: true, server } + await applyMcpServerMutationEffects({ + action: 'update', + workspaceId: params.workspaceId, + result, + }) + return result } catch (error) { logger.error('Failed to update MCP server', { error }) return { success: false, error: 'Failed to update MCP server', errorCode: 'internal' } } } +/** Preserves the legacy internal delete result, analytics, audit, and effects contract. */ export async function performDeleteMcpServer( params: PerformDeleteMcpServerParams ): Promise { try { - await revokeMcpOauthTokens(params.serverId) - const [server] = await db - .delete(mcpServers) - .where( - and(eq(mcpServers.id, params.serverId), eq(mcpServers.workspaceId, params.workspaceId)) - ) - .returning() - - if (!server) return { success: false, error: 'Server not found', errorCode: 'not_found' } - - await mcpService.clearCache(params.workspaceId) - await mcpService.evictServerConnections(params.serverId, 'server deleted') - const source = - params.source === 'settings' || params.source === 'tool_input' ? params.source : undefined + const result = await deleteMcpServer(params) + if (!result.success || !result.server) return result + const source = legacySource(params.source) captureServerEvent( params.userId, 'mcp_server_disconnected', - { workspace_id: params.workspaceId, server_name: server.name, source }, + { + workspace_id: params.workspaceId, + server_name: result.server.name, + source, + }, { groups: { workspace: params.workspaceId } } ) - recordAudit({ workspaceId: params.workspaceId, actorId: params.userId, @@ -468,21 +551,57 @@ export async function performDeleteMcpServer( actorEmail: params.actorEmail ?? undefined, action: AuditAction.MCP_SERVER_REMOVED, resourceType: AuditResourceType.MCP_SERVER, - resourceId: params.serverId, - resourceName: server.name, - description: `Removed MCP server "${server.name}"`, + resourceId: result.server.id, + resourceName: result.server.name, + description: `Removed MCP server "${result.server.name}"`, metadata: { - serverName: server.name, - transport: server.transport, - url: server.url, + serverName: result.server.name, + transport: result.server.transport, + url: result.server.url, source, }, request: params.request, }) - - return { success: true, server } + await applyMcpServerMutationEffects({ + action: 'delete', + workspaceId: params.workspaceId, + result, + }) + return result } catch (error) { logger.error('Failed to delete MCP server', { error }) return { success: false, error: 'Failed to delete MCP server', errorCode: 'internal' } } } + +/** Applies shared cache, connection, and domain-telemetry effects after semantic audit. */ +export async function applyMcpServerMutationEffects(params: { + action: McpServerMutationAction + workspaceId: string + result: PerformMcpServerResult +}): Promise { + const { action, workspaceId, result } = params + if (!result.serverId && !result.server?.id) { + throw new Error(`MCP ${action} result is missing its server ID`) + } + const serverId = result.serverId ?? result.server!.id + + if (action === 'update' && !result.configurationChanged) return + await mcpService.clearCache(workspaceId) + if (action !== 'create' || result.updated) { + await mcpService.evictServerConnections( + serverId, + action === 'delete' ? 'server deleted' : 'config changed' + ) + } + + if (action === 'create' && result.updated === false && result.server) { + const { PlatformEvents } = await import('@/lib/core/telemetry') + PlatformEvents.mcpServerAdded({ + serverId, + serverName: result.server.name, + transport: result.server.transport, + workspaceId, + }) + } +} diff --git a/apps/sim/lib/mcp/queries.ts b/apps/sim/lib/mcp/queries.ts index 789a86a4454..f7fdf40af1c 100644 --- a/apps/sim/lib/mcp/queries.ts +++ b/apps/sim/lib/mcp/queries.ts @@ -1,9 +1,7 @@ import { db } from '@sim/db' import { mcpServers } from '@sim/db/schema' import { and, type Column, eq, isNull } from 'drizzle-orm' -import type { V2McpServerSortBy } from '@/lib/api/contracts/v2/mcp-servers' -import type { V2SortOrder } from '@/lib/api/contracts/v2/shared' -import { listOrderBy, searchFilter } from '@/lib/api/list-query' +import { type ListSortOrder, listOrderBy, searchFilter } from '@/lib/api/list-query' /** * Workspace-scoped MCP server reads. The lifecycle functions in @@ -12,6 +10,7 @@ import { listOrderBy, searchFilter } from '@/lib/api/list-query' */ export type McpServerRow = typeof mcpServers.$inferSelect +export type McpServerSortBy = 'name' | 'createdAt' | 'updatedAt' /** Live (non-soft-deleted) MCP servers in a workspace, newest first. */ /** @@ -23,14 +22,14 @@ const MCP_SERVER_SORTS = { name: [mcpServers.name, mcpServers.id], createdAt: [mcpServers.createdAt, mcpServers.id], updatedAt: [mcpServers.updatedAt, mcpServers.id], -} satisfies Record +} satisfies Record export async function listWorkspaceMcpServers(params: { workspaceId: string /** Case-insensitive substring match on the server name. */ search?: string - sortBy?: V2McpServerSortBy - sortOrder?: V2SortOrder + sortBy?: McpServerSortBy + sortOrder?: ListSortOrder }): Promise { const { sortBy = 'createdAt', sortOrder = 'desc' } = params return db diff --git a/apps/sim/lib/secrets/application/operations.ts b/apps/sim/lib/secrets/application/operations.ts new file mode 100644 index 00000000000..e7b60c59ac5 --- /dev/null +++ b/apps/sim/lib/secrets/application/operations.ts @@ -0,0 +1,26 @@ +import { defineWorkspaceOperation } from '@/lib/core/application' + +const HUMAN_API_PRINCIPAL_KINDS = ['session', 'personal_api_key'] as const + +export const secretOperations = { + list: defineWorkspaceOperation({ + id: 'secrets.list', + minimumRole: 'read', + workspaceApiKey: 'deny', + principalKinds: HUMAN_API_PRINCIPAL_KINDS, + }), + set: defineWorkspaceOperation({ + id: 'secrets.set', + minimumRole: 'read', + workspaceApiKey: 'deny', + principalKinds: HUMAN_API_PRINCIPAL_KINDS, + }), + delete: defineWorkspaceOperation({ + id: 'secrets.delete', + minimumRole: 'read', + workspaceApiKey: 'deny', + principalKinds: HUMAN_API_PRINCIPAL_KINDS, + }), +} as const + +export type SecretOperation = (typeof secretOperations)[keyof typeof secretOperations] diff --git a/apps/sim/lib/secrets/application/use-cases.test.ts b/apps/sim/lib/secrets/application/use-cases.test.ts new file mode 100644 index 00000000000..1bb47279bd4 --- /dev/null +++ b/apps/sim/lib/secrets/application/use-cases.test.ts @@ -0,0 +1,146 @@ +/** + * @vitest-environment node + */ +import type { Principal } from '@sim/auth/principal' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { SetSecretInput } from '@/lib/secrets/application/use-cases' + +const { mocks } = vi.hoisted(() => ({ + mocks: { + loadContext: vi.fn(), + resolvePermission: vi.fn(), + workspaceAccess: vi.fn(), + keyAccess: vi.fn(), + setWorkspace: vi.fn(), + listCredentials: vi.fn(), + audit: vi.fn(), + }, +})) + +vi.mock('@/lib/uploads/contexts/workspace', () => ({ + loadActiveWorkspaceContext: mocks.loadContext, +})) +vi.mock('@sim/platform-authz/workspace', () => ({ + permissionSatisfies: (actual: string | null, required: string) => + actual === 'admin' || actual === required || (actual === 'write' && required === 'read'), + resolveEffectiveWorkspacePermission: mocks.resolvePermission, +})) +vi.mock('@sim/audit', () => ({ + AuditAction: { + ENVIRONMENT_UPDATED: 'environment.updated', + ENVIRONMENT_DELETED: 'environment.deleted', + }, + AuditResourceType: { ENVIRONMENT: 'environment' }, + recordAudit: mocks.audit, +})) +vi.mock('@/lib/workspaces/permissions/utils', () => ({ + checkWorkspaceAccess: mocks.workspaceAccess, +})) +vi.mock('@/lib/credentials/environment', () => ({ + getWorkspaceEnvKeyAdminAccess: mocks.keyAccess, +})) +vi.mock('@/lib/credentials/queries', () => ({ + listVisibleWorkspaceCredentials: mocks.listCredentials, +})) +vi.mock('@/lib/credentials/secret-values', () => ({ + deletePersonalSecret: vi.fn(), + deleteWorkspaceSecret: vi.fn(), + setPersonalSecret: vi.fn(), + setWorkspaceSecret: mocks.setWorkspace, +})) + +import { setSecretUseCase } from '@/lib/secrets/application/use-cases' + +const workspace = { + workspaceId: 'workspace-1', + workspaceOrganizationId: null, + allowPersonalApiKeys: true, + billedAccountUserId: 'owner-1', +} +const secret = { + id: 'secret-1', + workspaceId: workspace.workspaceId, + type: 'env_workspace' as const, + displayName: 'STRIPE_API_KEY', + description: null, + providerId: null, + accountId: null, + envKey: 'STRIPE_API_KEY', + envOwnerUserId: null, + createdBy: 'user-1', + createdAt: new Date('2026-01-01T00:00:00Z'), + updatedAt: new Date('2026-01-02T00:00:00Z'), + hasServiceAccountKey: false, + role: 'admin' as const, +} + +describe('secret application use cases', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.loadContext.mockResolvedValue(workspace) + mocks.resolvePermission.mockResolvedValue('write') + mocks.workspaceAccess.mockResolvedValue({ hasAccess: true, canWrite: true, canAdmin: false }) + mocks.keyAccess.mockResolvedValue({ knownKeys: new Set(), adminKeys: new Set() }) + mocks.setWorkspace.mockResolvedValue({ created: true }) + mocks.listCredentials.mockResolvedValue([secret]) + }) + + it('rejects workspace keys before resolving or reading secret state', async () => { + const execute = setSecretUseCase.execute as (args: { + principal: Principal + input: SetSecretInput + }) => Promise + + await expect( + execute({ + principal: { + kind: 'workspace_api_key', + workspaceId: workspace.workspaceId, + keyId: 'workspace-key-1', + }, + input: { + workspaceId: workspace.workspaceId, + name: secret.envKey, + scope: 'workspace', + value: 'secret-value', + }, + }) + ).rejects.toMatchObject({ code: 'forbidden' }) + + expect(mocks.loadContext).not.toHaveBeenCalled() + expect(mocks.listCredentials).not.toHaveBeenCalled() + expect(mocks.setWorkspace).not.toHaveBeenCalled() + }) + + it('checks ACLs, writes through the manager, and audits without the secret value', async () => { + const result = await setSecretUseCase.execute({ + principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' }, + input: { + workspaceId: workspace.workspaceId, + name: secret.envKey, + scope: 'workspace', + value: 'secret-value', + }, + }) + + expect(result.created).toBe(true) + expect(mocks.keyAccess).toHaveBeenCalledWith({ + workspaceId: workspace.workspaceId, + envKeys: [secret.envKey], + userId: 'user-1', + }) + expect(mocks.setWorkspace).toHaveBeenCalledWith({ + workspaceId: workspace.workspaceId, + name: secret.envKey, + value: 'secret-value', + userId: 'user-1', + }) + expect(mocks.audit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: 'user-1', + metadata: expect.objectContaining({ operation: 'secrets.set', scope: 'workspace' }), + }) + ) + expect(JSON.stringify(mocks.audit.mock.calls)).not.toContain('secret-value') + }) +}) diff --git a/apps/sim/lib/secrets/application/use-cases.ts b/apps/sim/lib/secrets/application/use-cases.ts new file mode 100644 index 00000000000..f30efa998b3 --- /dev/null +++ b/apps/sim/lib/secrets/application/use-cases.ts @@ -0,0 +1,232 @@ +import { AuditAction, AuditResourceType } from '@sim/audit' +import type { Principal } from '@sim/auth/principal' +import type { V2SortOrder } from '@/lib/api/contracts/v2/shared' +import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' +import { OrchestrationError } from '@/lib/core/orchestration/types' +import { getWorkspaceEnvKeyAdminAccess } from '@/lib/credentials/environment' +import { + listVisibleWorkspaceCredentials, + type VisibleWorkspaceCredential, +} from '@/lib/credentials/queries' +import { + deletePersonalSecret, + deleteWorkspaceSecret, + setPersonalSecret, + setWorkspaceSecret, +} from '@/lib/credentials/secret-values' +import { secretOperations } from '@/lib/secrets/application/operations' +import { loadActiveWorkspaceContext } from '@/lib/uploads/contexts/workspace' +import { checkWorkspaceAccess } from '@/lib/workspaces/permissions/utils' + +export type SecretScope = 'workspace' | 'personal' +export type SecretSortBy = 'name' | 'createdAt' | 'updatedAt' + +interface SecretWorkspaceContext { + workspaceId: string + workspaceOrganizationId: string | null + allowPersonalApiKeys: boolean + billedAccountUserId: string +} + +async function resolveWorkspaceContext(workspaceId: string): Promise { + const context = await loadActiveWorkspaceContext(workspaceId) + if (!context) throw new OrchestrationError('not_found', 'Workspace not found') + return context +} + +function principalUserId( + principal: Extract +): string { + return principal.userId +} + +function credentialTypes(scope?: SecretScope) { + if (scope === 'workspace') return ['env_workspace'] as const + if (scope === 'personal') return ['env_personal'] as const + return ['env_workspace', 'env_personal'] as const +} + +async function listSecretMetadata(params: { + workspaceId: string + userId: string + scope?: SecretScope + search?: string + sortBy: SecretSortBy + sortOrder: V2SortOrder +}): Promise { + const workspaceAccess = await checkWorkspaceAccess(params.workspaceId, params.userId) + const rows = await listVisibleWorkspaceCredentials({ + workspaceId: params.workspaceId, + userId: params.userId, + workspaceAccess, + types: [...credentialTypes(params.scope)], + search: params.search, + sortBy: params.sortBy === 'name' ? 'displayName' : params.sortBy, + sortOrder: params.sortOrder, + }) + return rows.filter((row) => row.type === 'env_workspace' || row.envOwnerUserId === params.userId) +} + +async function requireWorkspaceSecretMutationAccess(params: { + workspaceId: string + name: string + userId: string +}): Promise { + const [workspaceAccess, keyAccess] = await Promise.all([ + checkWorkspaceAccess(params.workspaceId, params.userId), + getWorkspaceEnvKeyAdminAccess({ + workspaceId: params.workspaceId, + envKeys: [params.name], + userId: params.userId, + }), + ]) + + if (keyAccess.knownKeys.has(params.name)) { + if (!workspaceAccess.canAdmin && !keyAccess.adminKeys.has(params.name)) { + throw new OrchestrationError( + 'forbidden', + 'Credential admin permission required for this secret' + ) + } + return + } + if (!workspaceAccess.canWrite) { + throw new OrchestrationError('forbidden', 'Write permission required to set this secret') + } +} + +async function getSecretMetadata(params: { + workspaceId: string + userId: string + scope: SecretScope + name: string +}): Promise { + const rows = await listSecretMetadata({ + ...params, + search: params.name, + sortBy: 'name', + sortOrder: 'asc', + }) + const row = rows.find( + (candidate) => + candidate.envKey === params.name && + (params.scope === 'workspace' + ? candidate.type === 'env_workspace' + : candidate.type === 'env_personal' && candidate.envOwnerUserId === params.userId) + ) + if (!row) throw new Error(`Secret metadata was not created for ${params.scope}:${params.name}`) + return row +} + +const authorizationOptions = {} + +export interface ListSecretsInput { + workspaceId: string + scope?: SecretScope + search?: string + sortBy: SecretSortBy + sortOrder: V2SortOrder +} + +export const listSecretsUseCase = defineAuthorizedWorkspaceUseCase({ + operation: secretOperations.list, + resolveContext: ({ input }: { input: ListSecretsInput }) => + resolveWorkspaceContext(input.workspaceId), + authorizationOptions, + async execute({ principal, input, context }) { + const userId = principalUserId(principal) + const secrets = await listSecretMetadata({ + ...input, + workspaceId: context.workspaceId, + userId, + }) + return { secrets, userId } + }, +}) + +export interface SetSecretInput { + workspaceId: string + name: string + scope: SecretScope + value: string +} + +export const setSecretUseCase = defineAuthorizedWorkspaceUseCase({ + operation: secretOperations.set, + resolveContext: ({ input }: { input: SetSecretInput }) => + resolveWorkspaceContext(input.workspaceId), + authorizationOptions, + async execute({ principal, input, context }) { + const userId = principalUserId(principal) + if (input.scope === 'workspace') { + await requireWorkspaceSecretMutationAccess({ + workspaceId: context.workspaceId, + name: input.name, + userId, + }) + } + + const mutation = + input.scope === 'workspace' + ? await setWorkspaceSecret({ + workspaceId: context.workspaceId, + name: input.name, + value: input.value, + userId, + }) + : await setPersonalSecret({ userId, name: input.name, value: input.value }) + const secret = await getSecretMetadata({ + workspaceId: context.workspaceId, + userId, + scope: input.scope, + name: input.name, + }) + return { secret, userId, created: mutation.created } + }, + projectAudit: ({ input }) => ({ + action: AuditAction.ENVIRONMENT_UPDATED, + resourceType: AuditResourceType.ENVIRONMENT, + resourceId: `${input.scope}:${input.name}`, + resourceName: input.name, + description: `Set ${input.scope} secret "${input.name}"`, + metadata: { scope: input.scope, name: input.name }, + }), +}) + +export interface DeleteSecretInput { + workspaceId: string + name: string + scope: SecretScope +} + +export const deleteSecretUseCase = defineAuthorizedWorkspaceUseCase({ + operation: secretOperations.delete, + resolveContext: ({ input }: { input: DeleteSecretInput }) => + resolveWorkspaceContext(input.workspaceId), + authorizationOptions, + async execute({ principal, input, context }) { + const userId = principalUserId(principal) + if (input.scope === 'workspace') { + await requireWorkspaceSecretMutationAccess({ + workspaceId: context.workspaceId, + name: input.name, + userId, + }) + } + + const deleted = + input.scope === 'workspace' + ? await deleteWorkspaceSecret({ workspaceId: context.workspaceId, name: input.name }) + : await deletePersonalSecret({ userId, name: input.name }) + if (!deleted) throw new OrchestrationError('not_found', 'Secret not found') + return { name: input.name, scope: input.scope } + }, + projectAudit: ({ input }) => ({ + action: AuditAction.ENVIRONMENT_DELETED, + resourceType: AuditResourceType.ENVIRONMENT, + resourceId: `${input.scope}:${input.name}`, + resourceName: input.name, + description: `Deleted ${input.scope} secret "${input.name}"`, + metadata: { scope: input.scope, name: input.name }, + }), +}) diff --git a/apps/sim/lib/skills/application/authorization.ts b/apps/sim/lib/skills/application/authorization.ts new file mode 100644 index 00000000000..97aea84b6da --- /dev/null +++ b/apps/sim/lib/skills/application/authorization.ts @@ -0,0 +1,13 @@ +import type { DelegatedPrincipal } from '@sim/auth/principal' +import type { WorkspaceDelegationPolicy } from '@/lib/core/application' + +export const SKILL_DELEGATION_AUDIENCE = 'sim:skills' + +export const skillDelegationPolicy = { + audience: SKILL_DELEGATION_AUDIENCE, + isWithinScope: (principal: DelegatedPrincipal) => principal.serviceId === 'copilot', +} as const satisfies WorkspaceDelegationPolicy<{ + workspaceId: string + workspaceOrganizationId: string | null + allowPersonalApiKeys: boolean +}> diff --git a/apps/sim/lib/skills/application/operations.ts b/apps/sim/lib/skills/application/operations.ts new file mode 100644 index 00000000000..b19065816be --- /dev/null +++ b/apps/sim/lib/skills/application/operations.ts @@ -0,0 +1,50 @@ +import { defineWorkspaceOperation } from '@/lib/core/application' + +const ALL_PRINCIPAL_KINDS = [ + 'session', + 'personal_api_key', + 'workspace_api_key', + 'delegated', +] as const +const HUMAN_PRINCIPAL_KINDS = ['session', 'personal_api_key', 'delegated'] as const + +export const skillOperations = { + list: defineWorkspaceOperation({ + id: 'skills.list', + minimumRole: 'read', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + listAvailable: defineWorkspaceOperation({ + id: 'skills.list_available', + minimumRole: 'read', + workspaceApiKey: 'deny', + principalKinds: HUMAN_PRINCIPAL_KINDS, + }), + read: defineWorkspaceOperation({ + id: 'skills.read', + minimumRole: 'read', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + create: defineWorkspaceOperation({ + id: 'skills.create', + minimumRole: 'write', + workspaceApiKey: 'allow', + principalKinds: ALL_PRINCIPAL_KINDS, + }), + update: defineWorkspaceOperation({ + id: 'skills.update', + minimumRole: 'read', + workspaceApiKey: 'deny', + principalKinds: HUMAN_PRINCIPAL_KINDS, + }), + delete: defineWorkspaceOperation({ + id: 'skills.delete', + minimumRole: 'read', + workspaceApiKey: 'deny', + principalKinds: HUMAN_PRINCIPAL_KINDS, + }), +} as const + +export type SkillOperation = (typeof skillOperations)[keyof typeof skillOperations] diff --git a/apps/sim/lib/skills/application/use-cases.test.ts b/apps/sim/lib/skills/application/use-cases.test.ts new file mode 100644 index 00000000000..250c6d15dfe --- /dev/null +++ b/apps/sim/lib/skills/application/use-cases.test.ts @@ -0,0 +1,140 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mocks } = vi.hoisted(() => ({ + mocks: { + loadContext: vi.fn(), + resolvePermission: vi.fn(), + getById: vi.fn(), + update: vi.fn(), + audit: vi.fn(), + }, +})) + +vi.mock('@/lib/uploads/contexts/workspace', () => ({ + loadActiveWorkspaceContext: mocks.loadContext, +})) +vi.mock('@sim/platform-authz/workspace', () => ({ + permissionSatisfies: (actual: string | null, required: string) => + actual === 'admin' || actual === required || (actual === 'write' && required === 'read'), + resolveEffectiveWorkspacePermission: mocks.resolvePermission, +})) +vi.mock('@sim/audit', () => ({ + AuditAction: { + SKILL_CREATED: 'skill.created', + SKILL_UPDATED: 'skill.updated', + SKILL_DELETED: 'skill.deleted', + }, + AuditResourceType: { SKILL: 'skill' }, + recordAudit: mocks.audit, +})) +vi.mock('@/lib/skills/orchestration', () => ({ + createSkill: vi.fn(), + deleteSkillRecord: vi.fn(), + updateSkill: mocks.update, +})) +vi.mock('@/lib/workflows/skills/operations', () => ({ + getSkillById: mocks.getById, + listSkills: vi.fn(), + listSkillsForUser: vi.fn(), +})) + +import { updateSkillUseCase } from '@/lib/skills/application/use-cases' + +const workspace = { + workspaceId: 'workspace-1', + workspaceOrganizationId: null, + allowPersonalApiKeys: true, + billedAccountUserId: 'owner-1', +} +const skill = { + id: 'skill-1', + workspaceId: workspace.workspaceId, + userId: 'user-1', + name: 'refund-policy', + description: 'Refund rules', + content: '# Refunds', + createdAt: new Date('2026-01-01T00:00:00Z'), + updatedAt: new Date('2026-01-02T00:00:00Z'), +} + +describe('skill application use cases', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.loadContext.mockResolvedValue(workspace) + mocks.resolvePermission.mockResolvedValue('read') + mocks.getById.mockResolvedValue(skill) + mocks.update.mockResolvedValue({ ...skill, content: '# Updated' }) + }) + + it('rejects workspace keys before resolving protected skill state', async () => { + await expect( + updateSkillUseCase.execute({ + principal: { + kind: 'workspace_api_key', + workspaceId: workspace.workspaceId, + keyId: 'workspace-key-1', + }, + input: { workspaceId: workspace.workspaceId, skillId: skill.id, content: '# Updated' }, + }) + ).rejects.toMatchObject({ code: 'forbidden' }) + + expect(mocks.loadContext).not.toHaveBeenCalled() + expect(mocks.getById).not.toHaveBeenCalled() + expect(mocks.update).not.toHaveBeenCalled() + }) + + it('uses the subject identity and semantic audit for delegated Copilot updates', async () => { + const principal = { + kind: 'delegated' as const, + serviceId: 'copilot' as const, + subjectUserId: 'user-1', + workspaceId: workspace.workspaceId, + delegationId: 'copilot-tool:call-1', + audience: 'sim:skills', + issuedAt: new Date('2026-01-01T00:00:00Z'), + expiresAt: new Date('2099-01-01T00:00:00Z'), + resourceScope: { chatId: 'chat-1' }, + } + + await updateSkillUseCase.execute({ + principal, + input: { + workspaceId: workspace.workspaceId, + skillId: skill.id, + content: '# Updated', + source: 'tool_input', + }, + }) + + expect(mocks.resolvePermission).toHaveBeenCalledWith( + 'user-1', + workspace.workspaceId, + null, + undefined, + { forUpdate: undefined } + ) + expect(mocks.update).toHaveBeenCalledWith({ + workspaceId: workspace.workspaceId, + userId: 'user-1', + skillId: skill.id, + name: undefined, + description: undefined, + content: '# Updated', + }) + expect(mocks.audit).toHaveBeenCalledWith( + expect.objectContaining({ + actorId: 'user-1', + metadata: expect.objectContaining({ + operation: 'skills.update', + actor: expect.objectContaining({ + kind: 'delegated', + delegationId: principal.delegationId, + }), + }), + }) + ) + }) +}) diff --git a/apps/sim/lib/skills/application/use-cases.ts b/apps/sim/lib/skills/application/use-cases.ts new file mode 100644 index 00000000000..89899bb5bcc --- /dev/null +++ b/apps/sim/lib/skills/application/use-cases.ts @@ -0,0 +1,204 @@ +import { AuditAction, AuditResourceType } from '@sim/audit' +import { type Principal, resolvePrincipalAttribution } from '@sim/auth/principal' +import type { skill } from '@sim/db/schema' +import type { ListSortOrder } from '@/lib/api/list-query' +import { defineAuthorizedWorkspaceUseCase } from '@/lib/core/application' +import { OrchestrationError } from '@/lib/core/orchestration/types' +import { skillDelegationPolicy } from '@/lib/skills/application/authorization' +import { skillOperations } from '@/lib/skills/application/operations' +import { createSkill, deleteSkillRecord, updateSkill } from '@/lib/skills/orchestration' +import { loadActiveWorkspaceContext } from '@/lib/uploads/contexts/workspace' +import { + getSkillById, + listSkills, + listSkillsForUser, + type SkillSortBy, +} from '@/lib/workflows/skills/operations' + +type SkillRow = typeof skill.$inferSelect +type SkillWriteSource = 'api' | 'settings' | 'tool_input' + +interface SkillWorkspaceContext { + workspaceId: string + workspaceOrganizationId: string | null + allowPersonalApiKeys: boolean + billedAccountUserId: string +} + +interface SkillContext extends SkillWorkspaceContext { + skill: SkillRow +} + +async function resolveWorkspaceContext(workspaceId: string): Promise { + const context = await loadActiveWorkspaceContext(workspaceId) + if (!context) throw new OrchestrationError('not_found', 'Workspace not found') + return context +} + +async function resolveSkillContext(workspaceId: string, skillId: string): Promise { + const workspace = await resolveWorkspaceContext(workspaceId) + const row = await getSkillById({ workspaceId: workspace.workspaceId, skillId }) + if (!row) throw new OrchestrationError('not_found', 'Skill not found') + return { ...workspace, skill: row } +} + +function humanUserId(principal: Exclude): string { + return principal.kind === 'delegated' ? principal.subjectUserId : principal.userId +} + +const authorizationOptions = { delegation: skillDelegationPolicy } + +export interface ListSkillsInput { + workspaceId: string + search?: string + sortBy: SkillSortBy + sortOrder: ListSortOrder +} + +export const listSkillsUseCase = defineAuthorizedWorkspaceUseCase({ + operation: skillOperations.list, + resolveContext: ({ input }: { input: ListSkillsInput }) => + resolveWorkspaceContext(input.workspaceId), + authorizationOptions, + async execute({ input, context }) { + const skills = await listSkills({ + workspaceId: context.workspaceId, + search: input.search, + sort: { sortBy: input.sortBy, sortOrder: input.sortOrder }, + }) + return { skills } + }, +}) + +export interface ListAvailableSkillsInput { + workspaceId: string +} + +export const listAvailableSkillsUseCase = defineAuthorizedWorkspaceUseCase({ + operation: skillOperations.listAvailable, + resolveContext: ({ input }: { input: ListAvailableSkillsInput }) => + resolveWorkspaceContext(input.workspaceId), + authorizationOptions, + async execute({ principal, context }) { + const skills = await listSkillsForUser({ + workspaceId: context.workspaceId, + userId: humanUserId(principal), + }) + return { skills } + }, +}) + +export interface GetSkillInput { + workspaceId: string + skillId: string +} + +export const getSkillUseCase = defineAuthorizedWorkspaceUseCase({ + operation: skillOperations.read, + resolveContext: ({ input }: { input: GetSkillInput }) => + resolveSkillContext(input.workspaceId, input.skillId), + authorizationOptions, + async execute({ context }) { + return { skill: context.skill } + }, +}) + +export interface CreateSkillInput { + workspaceId: string + name: string + description: string + content: string + source?: SkillWriteSource +} + +export const createSkillUseCase = defineAuthorizedWorkspaceUseCase({ + operation: skillOperations.create, + resolveContext: ({ input }: { input: CreateSkillInput }) => + resolveWorkspaceContext(input.workspaceId), + authorizationOptions, + async execute({ principal, input, context }) { + const attribution = resolvePrincipalAttribution(principal, { + workspaceBillingOwnerUserId: context.billedAccountUserId, + }) + const row = await createSkill({ + workspaceId: context.workspaceId, + userId: attribution.attributedUserId, + name: input.name, + description: input.description, + content: input.content, + }) + return { skill: row } + }, + projectAudit: ({ input, result }) => ({ + action: AuditAction.SKILL_CREATED, + resourceType: AuditResourceType.SKILL, + resourceId: result.skill.id, + resourceName: result.skill.name, + description: `Created skill "${result.skill.name}"`, + metadata: { source: input.source }, + }), +}) + +export interface UpdateSkillInput { + workspaceId: string + skillId: string + name?: string + description?: string + content?: string + source?: SkillWriteSource +} + +export const updateSkillUseCase = defineAuthorizedWorkspaceUseCase({ + operation: skillOperations.update, + resolveContext: ({ input }: { input: UpdateSkillInput }) => + resolveSkillContext(input.workspaceId, input.skillId), + authorizationOptions, + async execute({ principal, input, context }) { + const row = await updateSkill({ + workspaceId: context.workspaceId, + userId: humanUserId(principal), + skillId: context.skill.id, + name: input.name, + description: input.description, + content: input.content, + }) + return { skill: row } + }, + projectAudit: ({ input, result }) => ({ + action: AuditAction.SKILL_UPDATED, + resourceType: AuditResourceType.SKILL, + resourceId: result.skill.id, + resourceName: result.skill.name, + description: `Updated skill "${result.skill.name}"`, + metadata: { source: input.source }, + }), +}) + +export interface DeleteSkillInput { + workspaceId: string + skillId: string + source?: SkillWriteSource +} + +export const deleteSkillUseCase = defineAuthorizedWorkspaceUseCase({ + operation: skillOperations.delete, + resolveContext: ({ input }: { input: DeleteSkillInput }) => + resolveSkillContext(input.workspaceId, input.skillId), + authorizationOptions, + async execute({ principal, context }) { + const row = await deleteSkillRecord({ + workspaceId: context.workspaceId, + userId: humanUserId(principal), + skillId: context.skill.id, + }) + return { skill: row } + }, + projectAudit: ({ input, result }) => ({ + action: AuditAction.SKILL_DELETED, + resourceType: AuditResourceType.SKILL, + resourceId: result.skill.id, + resourceName: result.skill.name, + description: `Deleted skill "${result.skill.name}"`, + metadata: { source: input.source }, + }), +}) diff --git a/apps/sim/lib/skills/orchestration/index.ts b/apps/sim/lib/skills/orchestration/index.ts index 48bf621ec27..fd07c58624d 100644 --- a/apps/sim/lib/skills/orchestration/index.ts +++ b/apps/sim/lib/skills/orchestration/index.ts @@ -1,4 +1,6 @@ export { + createSkill, + deleteSkillRecord, type PerformCreateSkillParams, type PerformDeleteSkillParams, type PerformSkillResult, @@ -9,4 +11,5 @@ export { type SkillOrchestrationErrorCode, type SkillWriteSource, statusForSkillOrchestrationError, + updateSkill, } from './skill-lifecycle' diff --git a/apps/sim/lib/skills/orchestration/skill-lifecycle.ts b/apps/sim/lib/skills/orchestration/skill-lifecycle.ts index 5cb13daf037..1fd56b048c6 100644 --- a/apps/sim/lib/skills/orchestration/skill-lifecycle.ts +++ b/apps/sim/lib/skills/orchestration/skill-lifecycle.ts @@ -9,7 +9,11 @@ import { skillDescriptionSchema, skillNameSchema, } from '@/lib/api/contracts/skills' -import type { OrchestrationErrorCode } from '@/lib/core/orchestration/types' +import { + asOrchestrationError, + OrchestrationError, + type OrchestrationErrorCode, +} from '@/lib/core/orchestration/types' import { captureServerEvent } from '@/lib/posthog/server' import { getSkillActorContext } from '@/lib/skills/access' import { getBuiltinSkillByName, isBuiltinSkillId } from '@/lib/workflows/skills/builtin-skills' @@ -18,18 +22,12 @@ import { deleteSkill, getSkillById, upsertSkills } from '@/lib/workflows/skills/ const logger = createLogger('SkillOrchestration') /** - * Single authority for skill create/update/delete. - * - * Before this module the API route owned the create-vs-update split, the - * built-in guard, the per-skill editor check, the field limits (which lived - * only in the route's Zod contract), and the audit — so the copilot's - * `manage_skill`, which calls `upsertSkills` directly, bypassed all of them. - * Every caller now goes through these functions and gets the same rules. + * Shared skill manager primitives and legacy orchestration adapters. * - * Workspace-level authorization stays with the caller: each surface has already - * established workspace access by the time it gets here (session middleware, - * the v2 `resolveWorkspaceAccess`, the copilot's permission context). What is - * owned here is everything *per skill*. + * The throwing primitives own field validation, built-in guards, conflicts, + * and per-skill editor checks. Authorized application use cases own workspace + * authorization and semantic audit. The `perform*` adapters preserve internal + * route result, audit, and analytics compatibility. */ /** @@ -241,12 +239,47 @@ function recordSkillEvent(params: { export async function performCreateSkill( params: PerformCreateSkillParams ): Promise { + try { + const skill = await createSkill(params) + recordSkillEvent({ + action: 'created', + workspaceId: params.workspaceId, + userId: params.userId, + skillId: skill.id, + skillName: skill.name, + actor: params, + }) + return { success: true, skill } + } catch (error) { + return skillFailureResult(error, 'Failed to create skill') + } +} + +function throwSkillFailure(result: PerformSkillResult): never { + throw new OrchestrationError( + result.errorCode ?? 'internal', + result.error ?? 'Skill operation failed' + ) +} + +function skillFailureResult(error: unknown, fallback: string): PerformSkillResult { + const classified = asOrchestrationError(error) + if (classified) { + return { success: false, error: classified.message, errorCode: classified.code } + } + logger.error(fallback, { error: getErrorMessage(error, fallback) }) + return { success: false, error: fallback, errorCode: 'internal' } +} + +export async function createSkill( + params: Omit +): Promise { const invalid = fieldError(skillNameSchema, params.name) ?? fieldError(skillDescriptionSchema, params.description) ?? fieldError(skillContentSchema, params.content) ?? builtinNameCollision(params.name) - if (invalid) return validationFailure(invalid) + if (invalid) throw new OrchestrationError('validation', invalid) let created: { id: string; name: string } | undefined try { @@ -258,37 +291,49 @@ export async function performCreateSkill( }) created = touched[0] } catch (error) { - return classifyUpsertError(error) + throwSkillFailure(classifyUpsertError(error)) } if (!created) { - logger.error('Skill create returned no touched row', { workspaceId: params.workspaceId }) - return { success: false, error: 'Failed to create skill', errorCode: 'internal' } + throw new Error(`Skill create returned no touched row for workspace ${params.workspaceId}`) } - recordSkillEvent({ - action: 'created', - workspaceId: params.workspaceId, - userId: params.userId, - skillId: created.id, - skillName: created.name, - actor: params, - }) - const row = await getSkillById({ skillId: created.id, workspaceId: params.workspaceId }) - if (!row) return { success: false, error: 'Failed to create skill', errorCode: 'internal' } - return { success: true, skill: row } + if (!row) throw new Error(`Skill ${created.id} missing after a successful create`) + return row } export async function performUpdateSkill( params: PerformUpdateSkillParams ): Promise { + try { + const skill = await updateSkill(params) + recordSkillEvent({ + action: 'updated', + workspaceId: params.workspaceId, + userId: params.userId, + skillId: skill.id, + skillName: skill.name, + actor: params, + }) + return { success: true, skill } + } catch (error) { + return skillFailureResult(error, 'Failed to update skill') + } +} + +export async function updateSkill( + params: Omit +): Promise { if ( params.name === undefined && params.description === undefined && params.content === undefined ) { - return validationFailure('At least one of name, description, or content is required') + throw new OrchestrationError( + 'validation', + 'At least one of name, description, or content is required' + ) } const invalid = @@ -299,10 +344,10 @@ export async function performUpdateSkill( ? fieldError(skillDescriptionSchema, params.description) : null) ?? (params.content !== undefined ? fieldError(skillContentSchema, params.content) : null) - if (invalid) return validationFailure(invalid) + if (invalid) throw new OrchestrationError('validation', invalid) const resolved = await resolveEditableSkill(params) - if (!resolved.ok) return resolved.result + if (!resolved.ok) throwSkillFailure(resolved.result) try { await upsertSkills({ @@ -319,41 +364,40 @@ export async function performUpdateSkill( returnSkills: false, }) } catch (error) { - return classifyUpsertError(error) + throwSkillFailure(classifyUpsertError(error)) } const row = await getSkillById({ skillId: params.skillId, workspaceId: params.workspaceId }) - if (!row) return { success: false, error: 'Skill not found', errorCode: 'not_found' } - - recordSkillEvent({ - action: 'updated', - workspaceId: params.workspaceId, - userId: params.userId, - skillId: row.id, - skillName: row.name, - actor: params, - }) - - return { success: true, skill: row } + if (!row) throw new OrchestrationError('not_found', 'Skill not found') + return row } export async function performDeleteSkill( params: PerformDeleteSkillParams ): Promise { + try { + const skill = await deleteSkillRecord(params) + recordSkillEvent({ + action: 'deleted', + workspaceId: params.workspaceId, + userId: params.userId, + skillId: skill.id, + skillName: skill.name, + actor: params, + }) + return { success: true, skill } + } catch (error) { + return skillFailureResult(error, 'Failed to delete skill') + } +} + +export async function deleteSkillRecord( + params: Omit +): Promise { const resolved = await resolveEditableSkill(params) - if (!resolved.ok) return resolved.result + if (!resolved.ok) throwSkillFailure(resolved.result) const deleted = await deleteSkill({ skillId: params.skillId, workspaceId: params.workspaceId }) - if (!deleted) return { success: false, error: 'Skill not found', errorCode: 'not_found' } - - recordSkillEvent({ - action: 'deleted', - workspaceId: params.workspaceId, - userId: params.userId, - skillId: params.skillId, - skillName: resolved.skill.name, - actor: params, - }) - - return { success: true, skill: resolved.skill } + if (!deleted) throw new OrchestrationError('not_found', 'Skill not found') + return resolved.skill } diff --git a/apps/sim/lib/workflows/custom-tools/operations.ts b/apps/sim/lib/workflows/custom-tools/operations.ts index 32e2ba8bf3c..aedc1f6cc2b 100644 --- a/apps/sim/lib/workflows/custom-tools/operations.ts +++ b/apps/sim/lib/workflows/custom-tools/operations.ts @@ -3,13 +3,13 @@ import { customTools } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { generateShortId } from '@sim/utils/id' import { and, type Column, desc, eq, isNull, or } from 'drizzle-orm' -import type { V2CustomToolSortBy } from '@/lib/api/contracts/v2/custom-tools' -import type { V2SortOrder } from '@/lib/api/contracts/v2/shared' -import { listOrderBy, searchFilter } from '@/lib/api/list-query' +import { type ListSortOrder, listOrderBy, searchFilter } from '@/lib/api/list-query' import { generateRequestId } from '@/lib/core/utils/request' const logger = createLogger('CustomToolsOperations') +export type CustomToolSortBy = 'title' | 'createdAt' | 'updatedAt' + /** * Internal function to create/update custom tools * Can be called from API routes or internal services @@ -148,14 +148,14 @@ const CUSTOM_TOOL_SORTS = { title: [customTools.title, customTools.id], createdAt: [customTools.createdAt, customTools.id], updatedAt: [customTools.updatedAt, customTools.id], -} satisfies Record +} satisfies Record export async function listWorkspaceCustomTools(params: { workspaceId: string /** Case-insensitive substring match on the tool title. */ search?: string - sortBy?: V2CustomToolSortBy - sortOrder?: V2SortOrder + sortBy?: CustomToolSortBy + sortOrder?: ListSortOrder }) { const { sortBy = 'createdAt', sortOrder = 'desc' } = params return db @@ -290,6 +290,36 @@ export async function getCustomToolByIdOrTitle(params: { return legacyTool[0] || null } +export async function updateCustomTool(params: { + toolId: string + userId: string + workspaceId: string + title: string + schema: unknown + code: string +}) { + const workspaceTool = await updateWorkspaceCustomTool(params) + if (workspaceTool) return workspaceTool + + const [legacyTool] = await db + .update(customTools) + .set({ + title: params.title, + schema: params.schema, + code: params.code, + updatedAt: new Date(), + }) + .where( + and( + eq(customTools.id, params.toolId), + isNull(customTools.workspaceId), + eq(customTools.userId, params.userId) + ) + ) + .returning() + return legacyTool ?? null +} + export async function deleteCustomTool(params: { toolId: string userId: string diff --git a/apps/sim/lib/workflows/skills/operations.ts b/apps/sim/lib/workflows/skills/operations.ts index daf82b2d285..bdc1347fe5f 100644 --- a/apps/sim/lib/workflows/skills/operations.ts +++ b/apps/sim/lib/workflows/skills/operations.ts @@ -3,9 +3,7 @@ import { skill, skillMember } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { generateId, generateShortId } from '@sim/utils/id' import { and, type Column, desc, eq, ne } from 'drizzle-orm' -import type { V2SortOrder } from '@/lib/api/contracts/v2/shared' -import type { V2SkillSortBy } from '@/lib/api/contracts/v2/skills' -import { listOrderBy, searchFilter } from '@/lib/api/list-query' +import { type ListSortOrder, listOrderBy, searchFilter } from '@/lib/api/list-query' import { generateRequestId } from '@/lib/core/utils/request' import { getEditableSkillIds } from '@/lib/skills/access' import { @@ -36,6 +34,7 @@ function builtinSkillRow(workspaceId: string, builtin: BuiltinSkill): typeof ski } type SkillRow = typeof skill.$inferSelect +export type SkillSortBy = 'name' | 'createdAt' | 'updatedAt' /** * Orderings for the public list's sortable fields, made total over the contract @@ -46,15 +45,15 @@ const SKILL_SORTS = { name: [skill.name, skill.id], createdAt: [skill.createdAt, skill.id], updatedAt: [skill.updatedAt, skill.id], -} satisfies Record +} satisfies Record /** The sort key {@link SKILL_SORTS} orders on, for one row. */ -function skillSortKey(row: SkillRow, sortBy: V2SkillSortBy): [string | number, string] { +function skillSortKey(row: SkillRow, sortBy: SkillSortBy): [string | number, string] { if (sortBy === 'name') return [row.name, row.id] return [(sortBy === 'createdAt' ? row.createdAt : row.updatedAt).getTime(), row.id] } -function compareSkills(a: SkillRow, b: SkillRow, sortBy: V2SkillSortBy): number { +function compareSkills(a: SkillRow, b: SkillRow, sortBy: SkillSortBy): number { const [aKey, aId] = skillSortKey(a, sortBy) const [bKey, bId] = skillSortKey(b, sortBy) if (aKey !== bKey) return aKey < bKey ? -1 : 1 @@ -86,7 +85,7 @@ export async function listSkills(params: { includeBuiltins?: boolean /** Case-insensitive substring match on the skill name. */ search?: string - sort?: { sortBy: V2SkillSortBy; sortOrder: V2SortOrder } + sort?: { sortBy: SkillSortBy; sortOrder: ListSortOrder } }): Promise { const sortBy = params.sort?.sortBy ?? 'createdAt' const sortOrder = params.sort?.sortOrder ?? 'desc'