Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion apps/sim/app/api/jobs/[jobId]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { toError } from '@sim/utils/errors'
import { type NextRequest, NextResponse } from 'next/server'
import { getJobStatusContract } from '@/lib/api/contracts/common'
import { parseRequest } from '@/lib/api/server'
import { WORKSPACE_KEY_SCOPE_DENIED } from '@/lib/api-key/policy-messages'
import { checkHybridAuth } from '@/lib/auth/hybrid'
import { getJobQueue } from '@/lib/core/async-jobs'
import { generateRequestId } from '@/lib/core/utils/request'
Expand Down Expand Up @@ -54,7 +55,7 @@ export const GET = withRouteHandler(
const { getWorkflowById } = await import('@/lib/workflows/utils')
const workflow = await getWorkflowById(metadataToCheck.workflowId as string)
if (!workflow?.workspaceId || workflow.workspaceId !== authResult.workspaceId) {
return createErrorResponse('API key is not authorized for this workspace', 403)
return createErrorResponse(WORKSPACE_KEY_SCOPE_DENIED, 403)
}
}
} else if (metadataToCheck?.userId && metadataToCheck.userId !== authenticatedUserId) {
Expand Down
117 changes: 117 additions & 0 deletions apps/sim/app/api/mcp/discover/route.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
/**
* @vitest-environment node
*/
import { dbChainMockFns, hybridAuthMockFns, resetDbChainMock } from '@sim/testing'
import { NextRequest } from 'next/server'
import { beforeEach, describe, expect, it, vi } from 'vitest'

const { mockListAccessibleWorkspaceRowsForUser } = vi.hoisted(() => ({
mockListAccessibleWorkspaceRowsForUser: vi.fn(),
}))

vi.mock('@/lib/workspaces/utils', () => ({
listAccessibleWorkspaceRowsForUser: mockListAccessibleWorkspaceRowsForUser,
}))

import { GET } from '@/app/api/mcp/discover/route'

function workspaceRow(id: string) {
return {
workspace: { id, name: `${id} name`, archivedAt: null },
permissionType: 'write' as const,
}
}

function serverRow(
id: string,
{ isPublic = false, workspaceAllowsPersonalApiKeys = true } = {}
): Record<string, unknown> {
return {
id,
name: `${id} name`,
description: null,
workspaceId: 'ws-1',
workspaceName: 'ws-1 name',
isPublic,
workspaceAllowsPersonalApiKeys,
createdAt: new Date('2026-01-01T00:00:00.000Z'),
toolCount: 1,
}
}

function discoverRequest() {
return new NextRequest('http://localhost:3000/api/mcp/discover', {
method: 'GET',
headers: { 'X-API-Key': 'sk_test_123' },
})
}

function authAs(auth: Record<string, unknown>) {
hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({
success: true,
userId: 'user-1',
...auth,
})
}

describe('MCP Discover Route', () => {
beforeEach(() => {
vi.clearAllMocks()
resetDbChainMock()
mockListAccessibleWorkspaceRowsForUser.mockResolvedValue([workspaceRow('ws-1')])
})

it('hides private servers whose workspace disallows personal api keys', async () => {
authAs({ authType: 'api_key', apiKeyType: 'personal' })
dbChainMockFns.orderBy.mockResolvedValueOnce([
serverRow('allowed', { workspaceAllowsPersonalApiKeys: true }),
serverRow('blocked', { workspaceAllowsPersonalApiKeys: false }),
])

const response = await GET(discoverRequest())
const body = await response.json()

expect(response.status).toBe(200)
expect(body.servers.map((server: { id: string }) => server.id)).toEqual(['allowed'])
})

it('keeps public servers listed for a personal key even when the workspace disallows them', async () => {
authAs({ authType: 'api_key', apiKeyType: 'personal' })
dbChainMockFns.orderBy.mockResolvedValueOnce([
serverRow('public', { isPublic: true, workspaceAllowsPersonalApiKeys: false }),
serverRow('private', { isPublic: false, workspaceAllowsPersonalApiKeys: false }),
])

const response = await GET(discoverRequest())
const body = await response.json()

expect(response.status).toBe(200)
expect(body.servers.map((server: { id: string }) => server.id)).toEqual(['public'])
})

it('does not filter for a session caller', async () => {
authAs({ authType: 'session' })
dbChainMockFns.orderBy.mockResolvedValueOnce([
serverRow('blocked', { workspaceAllowsPersonalApiKeys: false }),
])

const response = await GET(discoverRequest())
const body = await response.json()

expect(response.status).toBe(200)
expect(body.servers).toHaveLength(1)
})

it('does not filter for a workspace key', async () => {
authAs({ authType: 'api_key', apiKeyType: 'workspace', workspaceId: 'ws-1' })
dbChainMockFns.orderBy.mockResolvedValueOnce([
serverRow('blocked', { workspaceAllowsPersonalApiKeys: false }),
])

const response = await GET(discoverRequest())
const body = await response.json()

expect(response.status).toBe(200)
expect(body.servers).toHaveLength(1)
})
})
14 changes: 13 additions & 1 deletion apps/sim/app/api/mcp/discover/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
description: workflowMcpServer.description,
workspaceId: workflowMcpServer.workspaceId,
workspaceName: workspace.name,
isPublic: workflowMcpServer.isPublic,
workspaceAllowsPersonalApiKeys: workspace.allowPersonalApiKeys,
createdAt: workflowMcpServer.createdAt,
toolCount: sql<number>`(
SELECT COUNT(*)::int
Expand All @@ -75,7 +77,17 @@ export const GET = withRouteHandler(async (request: NextRequest) => {

const baseUrl = getBaseUrl()

const formattedServers = servers.map((server) => ({
/**
* `/api/mcp/serve/[serverId]` rejects a personal key on a workspace that
* disabled them, so listing those servers would advertise unusable
* endpoints. Public servers skip that check entirely, so they stay listed.
*/
const visibleServers =
auth.apiKeyType === 'personal'
? servers.filter((server) => server.isPublic || server.workspaceAllowsPersonalApiKeys)
: servers

const formattedServers = visibleServers.map((server) => ({
id: server.id,
name: server.name,
description: server.description,
Expand Down
3 changes: 2 additions & 1 deletion apps/sim/app/api/mcp/serve/[serverId]/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ vi.mock('@/lib/core/execution-limits', () => ({
getMaxExecutionTimeout: () => 10_000,
}))

import { PERSONAL_KEY_DENIED } from '@/lib/api-key/policy-messages'
import {
MAX_PRIVATE_TOOL_METADATA_OVERHEAD_BYTES,
PRIVATE_TOOL_METADATA_REQUEST_HEADER,
Expand Down Expand Up @@ -349,7 +350,7 @@ describe('MCP Serve Route', () => {
const body = await response.json()

expect(response.status).toBe(403)
expect(body.error).toBe('Personal API keys are not allowed for this workspace')
expect(body.error).toBe(PERSONAL_KEY_DENIED)
expect(fetchMock).not.toHaveBeenCalled()
expect(mockGenerateInternalToken).not.toHaveBeenCalled()
})
Expand Down
6 changes: 2 additions & 4 deletions apps/sim/app/api/mcp/serve/[serverId]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
mcpServeRouteParamsSchema,
mcpToolCallParamsSchema,
} from '@/lib/api/contracts/mcp'
import { PERSONAL_KEY_DENIED } from '@/lib/api-key/policy-messages'
import { AuthType, checkHybridAuth } from '@/lib/auth/hybrid'
import { generateInternalToken } from '@/lib/auth/internal'
import {
Expand Down Expand Up @@ -433,10 +434,7 @@ async function authorizeMcpServeRequest(
const isPersonalApiKey = auth.authType === AuthType.API_KEY && auth.apiKeyType === 'personal'
if (isPersonalApiKey && !server.workspaceAllowsPersonalApiKeys) {
return {
response: NextResponse.json(
{ error: 'Personal API keys are not allowed for this workspace' },
{ status: 403 }
),
response: NextResponse.json({ error: PERSONAL_KEY_DENIED }, { status: 403 }),
}
}

Expand Down
15 changes: 5 additions & 10 deletions apps/sim/app/api/v1/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { type NextRequest, NextResponse } from 'next/server'
import type { ZodError } from 'zod'
import { getValidationErrorMessage, isZodError, validationErrorResponse } from '@/lib/api/server'
import { buildRateLimitHeaders, recordRateLimitSnapshot } from '@/lib/api/server/rate-limit-context'
import { PERSONAL_KEY_DENIED, WORKSPACE_KEY_SCOPE_DENIED } from '@/lib/api-key/policy-messages'
import { getHighestPrioritySubscription } from '@/lib/billing/core/subscription'
import type { SubscriptionPlan } from '@/lib/core/rate-limiter'
import { getRateLimit, RateLimiter } from '@/lib/core/rate-limiter'
Expand Down Expand Up @@ -197,8 +198,8 @@ export function createRateLimitResponse(result: RateLimitResult): NextResponse {
* Enforces two policies:
* - A workspace-scoped key may only target its own workspace.
* - A personal key is rejected when the workspace has disabled personal API
* keys (`allowPersonalApiKeys = false`), matching the workflow-execution
* surface in `app/api/workflows/middleware.ts`.
* keys (`allowPersonalApiKeys = false`). Other surfaces enforcing the same
* policy share `PERSONAL_KEY_DENIED`.
*/
export async function checkWorkspaceScope(
rateLimit: RateLimitResult,
Expand All @@ -209,19 +210,13 @@ export async function checkWorkspaceScope(
rateLimit.workspaceId &&
rateLimit.workspaceId !== requestedWorkspaceId
) {
return NextResponse.json(
{ error: 'API key is not authorized for this workspace' },
{ status: 403 }
)
return NextResponse.json({ error: WORKSPACE_KEY_SCOPE_DENIED }, { status: 403 })
}

if (rateLimit.keyType === 'personal') {
const settings = await getWorkspaceBillingSettings(requestedWorkspaceId)
if (!settings?.allowPersonalApiKeys) {
return NextResponse.json(
{ error: 'Personal API keys are not allowed for this workspace' },
{ status: 403 }
)
return NextResponse.json({ error: PERSONAL_KEY_DENIED }, { status: 403 })
}
}

Expand Down
5 changes: 3 additions & 2 deletions apps/sim/app/api/workflows/[id]/execute/route.async.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ vi.mock('@sim/utils/id', () => ({
),
}))

import { PERSONAL_KEY_DENIED, WORKSPACE_KEY_SCOPE_DENIED } from '@/lib/api-key/policy-messages'
import { storeLargeValue } from '@/lib/execution/payloads/store'
import { POST } from './route'

Expand Down Expand Up @@ -1791,7 +1792,7 @@ describe('workflow execute async route', () => {

expect(response.status).toBe(403)
await expect(response.json()).resolves.toEqual({
error: 'API key is not authorized for this workspace',
error: WORKSPACE_KEY_SCOPE_DENIED,
})
expect(mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalled()
expect(mockPreprocessExecution).not.toHaveBeenCalled()
Expand All @@ -1812,7 +1813,7 @@ describe('workflow execute async route', () => {

expect(response.status).toBe(403)
await expect(response.json()).resolves.toEqual({
error: 'Personal API keys are not allowed for this workspace',
error: PERSONAL_KEY_DENIED,
})
expect(mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalled()
expect(mockPreprocessExecution).not.toHaveBeenCalled()
Expand Down
11 changes: 3 additions & 8 deletions apps/sim/app/api/workflows/[id]/execute/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
WORKFLOW_EXECUTION_ID_HEADER,
WORKFLOW_EXECUTION_TIMEOUT_SECONDS_HEADER,
} from '@/lib/api/contracts/workflows'
import { PERSONAL_KEY_DENIED, WORKSPACE_KEY_SCOPE_DENIED } from '@/lib/api-key/policy-messages'
import { AuthType, checkHybridAuth, hasExternalApiCredentials } from '@/lib/auth/hybrid'
import { releaseExecutionSlot } from '@/lib/billing/calculations/usage-reservation'
import {
Expand Down Expand Up @@ -1106,21 +1107,15 @@ async function handleExecutePost(
}
if (auth.authType === AuthType.API_KEY) {
if (auth.apiKeyType === 'workspace' && auth.workspaceId !== workflowWorkspaceId) {
return NextResponse.json(
{ error: 'API key is not authorized for this workspace' },
{ status: 403 }
)
return NextResponse.json({ error: WORKSPACE_KEY_SCOPE_DENIED }, { status: 403 })
}

if (auth.apiKeyType === 'personal') {
const workspaceSettings = workflowWorkspaceId
? await getWorkspaceBillingSettings(workflowWorkspaceId)
: null
if (!workspaceSettings?.allowPersonalApiKeys) {
return NextResponse.json(
{ error: 'Personal API keys are not allowed for this workspace' },
{ status: 403 }
)
return NextResponse.json({ error: PERSONAL_KEY_DENIED }, { status: 403 })
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { and, eq, inArray } from 'drizzle-orm'
import { type NextRequest, NextResponse } from 'next/server'
import { cancelWorkflowExecutionContract } from '@/lib/api/contracts/workflows'
import { parseRequest } from '@/lib/api/server'
import { WORKSPACE_KEY_SCOPE_DENIED } from '@/lib/api-key/policy-messages'
import { checkHybridAuth } from '@/lib/auth/hybrid'
import { releaseExecutionSlot } from '@/lib/billing/calculations/usage-reservation'
import { getJobQueue } from '@/lib/core/async-jobs'
Expand Down Expand Up @@ -344,10 +345,7 @@ export const POST = withRouteHandler(
auth.apiKeyType === 'workspace' &&
workflowAuthorization.workflow?.workspaceId !== auth.workspaceId
) {
return NextResponse.json(
{ error: 'API key is not authorized for this workspace' },
{ status: 403 }
)
return NextResponse.json({ error: WORKSPACE_KEY_SCOPE_DENIED }, { status: 403 })
}

const execution = await db
Expand Down
6 changes: 2 additions & 4 deletions apps/sim/app/api/workflows/[id]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { eq, sql } from 'drizzle-orm'
import { type NextRequest, NextResponse } from 'next/server'
import { updateWorkflowContract } from '@/lib/api/contracts/workflows'
import { parseRequest } from '@/lib/api/server'
import { WORKSPACE_KEY_SCOPE_DENIED } from '@/lib/api-key/policy-messages'
import { AuthType, checkHybridAuth, checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
import { generateRequestId } from '@/lib/core/utils/request'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
Expand Down Expand Up @@ -51,10 +52,7 @@ export const GET = withRouteHandler(
}

if (auth.apiKeyType === 'workspace' && auth.workspaceId !== workflowData.workspaceId) {
return NextResponse.json(
{ error: 'API key is not authorized for this workspace' },
{ status: 403 }
)
return NextResponse.json({ error: WORKSPACE_KEY_SCOPE_DENIED }, { status: 403 })
}

if (isInternalCall && !userId) {
Expand Down
3 changes: 2 additions & 1 deletion apps/sim/app/api/workflows/middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ vi.mock('@/lib/api-key/service', () => ({
updateApiKeyLastUsed: vi.fn(),
}))

import { WORKSPACE_KEY_SCOPE_DENIED } from '@/lib/api-key/policy-messages'
import { validateWorkflowAccess } from '@/app/api/workflows/middleware'

function makeRequest() {
Expand Down Expand Up @@ -53,7 +54,7 @@ describe('validateWorkflowAccess (requireDeployment=false)', () => {
const result = await validateWorkflowAccess(makeRequest(), 'wf-1', false)

expect(result.error).toEqual({
message: 'API key is not authorized for this workspace',
message: WORKSPACE_KEY_SCOPE_DENIED,
status: 403,
})
expect(workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission).not.toHaveBeenCalled()
Expand Down
3 changes: 2 additions & 1 deletion apps/sim/app/api/workflows/middleware.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { createLogger } from '@sim/logger'
import { authorizeWorkflowByWorkspacePermission } from '@sim/platform-authz/workflow'
import type { NextRequest } from 'next/server'
import { WORKSPACE_KEY_SCOPE_DENIED } from '@/lib/api-key/policy-messages'
import {
type ApiKeyAuthResult,
authenticateApiKeyFromHeader,
Expand Down Expand Up @@ -57,7 +58,7 @@ export async function validateWorkflowAccess(
if (auth.apiKeyType === 'workspace' && auth.workspaceId !== workflow.workspaceId) {
return {
error: {
message: 'API key is not authorized for this workspace',
message: WORKSPACE_KEY_SCOPE_DENIED,
status: 403,
},
}
Expand Down
8 changes: 8 additions & 0 deletions apps/sim/lib/api-key/policy-messages.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/**
* Shared 403 messages so surfaces gating on API-key workspace scope or
* `workspace.allowPersonalApiKeys` reject with identical wording.
*/

export const WORKSPACE_KEY_SCOPE_DENIED = 'API key is not authorized for this workspace'

export const PERSONAL_KEY_DENIED = 'Personal API keys are not allowed for this workspace'
Loading