Skip to content

Commit 815a960

Browse files
waleedlatif1waleedlatif1
authored
improvement(api): share the API key workspace policy messages and align MCP discover (#6439)
* improvement(api): share the API key workspace policy messages and align MCP discover * fix(mcp): keep public servers listed for personal keys in discover
1 parent 9cd2e7f commit 815a960

13 files changed

Lines changed: 163 additions & 37 deletions

File tree

‎apps/sim/app/api/jobs/[jobId]/route.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { toError } from '@sim/utils/errors'
33
import { type NextRequest, NextResponse } from 'next/server'
44
import { getJobStatusContract } from '@/lib/api/contracts/common'
55
import { parseRequest } from '@/lib/api/server'
6+
import { WORKSPACE_KEY_SCOPE_DENIED } from '@/lib/api-key/policy-messages'
67
import { checkHybridAuth } from '@/lib/auth/hybrid'
78
import { getJobQueue } from '@/lib/core/async-jobs'
89
import { generateRequestId } from '@/lib/core/utils/request'
@@ -54,7 +55,7 @@ export const GET = withRouteHandler(
5455
const { getWorkflowById } = await import('@/lib/workflows/utils')
5556
const workflow = await getWorkflowById(metadataToCheck.workflowId as string)
5657
if (!workflow?.workspaceId || workflow.workspaceId !== authResult.workspaceId) {
57-
return createErrorResponse('API key is not authorized for this workspace', 403)
58+
return createErrorResponse(WORKSPACE_KEY_SCOPE_DENIED, 403)
5859
}
5960
}
6061
} else if (metadataToCheck?.userId && metadataToCheck.userId !== authenticatedUserId) {

‎apps/sim/app/api/mcp/discover/route.test.ts‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { dbChainMockFns, hybridAuthMockFns, resetDbChainMock } from '@sim/testing'
5+
import { NextRequest } from 'next/server'
6+
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
const { mockListAccessibleWorkspaceRowsForUser } = vi.hoisted(() => ({
9+
mockListAccessibleWorkspaceRowsForUser: vi.fn(),
10+
}))
11+
12+
vi.mock('@/lib/workspaces/utils', () => ({
13+
listAccessibleWorkspaceRowsForUser: mockListAccessibleWorkspaceRowsForUser,
14+
}))
15+
16+
import { GET } from '@/app/api/mcp/discover/route'
17+
18+
function workspaceRow(id: string) {
19+
return {
20+
workspace: { id, name: `${id} name`, archivedAt: null },
21+
permissionType: 'write' as const,
22+
}
23+
}
24+
25+
function serverRow(
26+
id: string,
27+
{ isPublic = false, workspaceAllowsPersonalApiKeys = true } = {}
28+
): Record<string, unknown> {
29+
return {
30+
id,
31+
name: `${id} name`,
32+
description: null,
33+
workspaceId: 'ws-1',
34+
workspaceName: 'ws-1 name',
35+
isPublic,
36+
workspaceAllowsPersonalApiKeys,
37+
createdAt: new Date('2026-01-01T00:00:00.000Z'),
38+
toolCount: 1,
39+
}
40+
}
41+
42+
function discoverRequest() {
43+
return new NextRequest('http://localhost:3000/api/mcp/discover', {
44+
method: 'GET',
45+
headers: { 'X-API-Key': 'sk_test_123' },
46+
})
47+
}
48+
49+
function authAs(auth: Record<string, unknown>) {
50+
hybridAuthMockFns.mockCheckHybridAuth.mockResolvedValueOnce({
51+
success: true,
52+
userId: 'user-1',
53+
...auth,
54+
})
55+
}
56+
57+
describe('MCP Discover Route', () => {
58+
beforeEach(() => {
59+
vi.clearAllMocks()
60+
resetDbChainMock()
61+
mockListAccessibleWorkspaceRowsForUser.mockResolvedValue([workspaceRow('ws-1')])
62+
})
63+
64+
it('hides private servers whose workspace disallows personal api keys', async () => {
65+
authAs({ authType: 'api_key', apiKeyType: 'personal' })
66+
dbChainMockFns.orderBy.mockResolvedValueOnce([
67+
serverRow('allowed', { workspaceAllowsPersonalApiKeys: true }),
68+
serverRow('blocked', { workspaceAllowsPersonalApiKeys: false }),
69+
])
70+
71+
const response = await GET(discoverRequest())
72+
const body = await response.json()
73+
74+
expect(response.status).toBe(200)
75+
expect(body.servers.map((server: { id: string }) => server.id)).toEqual(['allowed'])
76+
})
77+
78+
it('keeps public servers listed for a personal key even when the workspace disallows them', async () => {
79+
authAs({ authType: 'api_key', apiKeyType: 'personal' })
80+
dbChainMockFns.orderBy.mockResolvedValueOnce([
81+
serverRow('public', { isPublic: true, workspaceAllowsPersonalApiKeys: false }),
82+
serverRow('private', { isPublic: false, workspaceAllowsPersonalApiKeys: false }),
83+
])
84+
85+
const response = await GET(discoverRequest())
86+
const body = await response.json()
87+
88+
expect(response.status).toBe(200)
89+
expect(body.servers.map((server: { id: string }) => server.id)).toEqual(['public'])
90+
})
91+
92+
it('does not filter for a session caller', async () => {
93+
authAs({ authType: 'session' })
94+
dbChainMockFns.orderBy.mockResolvedValueOnce([
95+
serverRow('blocked', { workspaceAllowsPersonalApiKeys: false }),
96+
])
97+
98+
const response = await GET(discoverRequest())
99+
const body = await response.json()
100+
101+
expect(response.status).toBe(200)
102+
expect(body.servers).toHaveLength(1)
103+
})
104+
105+
it('does not filter for a workspace key', async () => {
106+
authAs({ authType: 'api_key', apiKeyType: 'workspace', workspaceId: 'ws-1' })
107+
dbChainMockFns.orderBy.mockResolvedValueOnce([
108+
serverRow('blocked', { workspaceAllowsPersonalApiKeys: false }),
109+
])
110+
111+
const response = await GET(discoverRequest())
112+
const body = await response.json()
113+
114+
expect(response.status).toBe(200)
115+
expect(body.servers).toHaveLength(1)
116+
})
117+
})

‎apps/sim/app/api/mcp/discover/route.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
5454
description: workflowMcpServer.description,
5555
workspaceId: workflowMcpServer.workspaceId,
5656
workspaceName: workspace.name,
57+
isPublic: workflowMcpServer.isPublic,
58+
workspaceAllowsPersonalApiKeys: workspace.allowPersonalApiKeys,
5759
createdAt: workflowMcpServer.createdAt,
5860
toolCount: sql<number>`(
5961
SELECT COUNT(*)::int
@@ -75,7 +77,17 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
7577

7678
const baseUrl = getBaseUrl()
7779

78-
const formattedServers = servers.map((server) => ({
80+
/**
81+
* `/api/mcp/serve/[serverId]` rejects a personal key on a workspace that
82+
* disabled them, so listing those servers would advertise unusable
83+
* endpoints. Public servers skip that check entirely, so they stay listed.
84+
*/
85+
const visibleServers =
86+
auth.apiKeyType === 'personal'
87+
? servers.filter((server) => server.isPublic || server.workspaceAllowsPersonalApiKeys)
88+
: servers
89+
90+
const formattedServers = visibleServers.map((server) => ({
7991
id: server.id,
8092
name: server.name,
8193
description: server.description,

‎apps/sim/app/api/mcp/serve/[serverId]/route.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ vi.mock('@/lib/core/execution-limits', () => ({
6969
getMaxExecutionTimeout: () => 10_000,
7070
}))
7171

72+
import { PERSONAL_KEY_DENIED } from '@/lib/api-key/policy-messages'
7273
import {
7374
MAX_PRIVATE_TOOL_METADATA_OVERHEAD_BYTES,
7475
PRIVATE_TOOL_METADATA_REQUEST_HEADER,
@@ -349,7 +350,7 @@ describe('MCP Serve Route', () => {
349350
const body = await response.json()
350351

351352
expect(response.status).toBe(403)
352-
expect(body.error).toBe('Personal API keys are not allowed for this workspace')
353+
expect(body.error).toBe(PERSONAL_KEY_DENIED)
353354
expect(fetchMock).not.toHaveBeenCalled()
354355
expect(mockGenerateInternalToken).not.toHaveBeenCalled()
355356
})

‎apps/sim/app/api/mcp/serve/[serverId]/route.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {
3434
mcpServeRouteParamsSchema,
3535
mcpToolCallParamsSchema,
3636
} from '@/lib/api/contracts/mcp'
37+
import { PERSONAL_KEY_DENIED } from '@/lib/api-key/policy-messages'
3738
import { AuthType, checkHybridAuth } from '@/lib/auth/hybrid'
3839
import { generateInternalToken } from '@/lib/auth/internal'
3940
import {
@@ -433,10 +434,7 @@ async function authorizeMcpServeRequest(
433434
const isPersonalApiKey = auth.authType === AuthType.API_KEY && auth.apiKeyType === 'personal'
434435
if (isPersonalApiKey && !server.workspaceAllowsPersonalApiKeys) {
435436
return {
436-
response: NextResponse.json(
437-
{ error: 'Personal API keys are not allowed for this workspace' },
438-
{ status: 403 }
439-
),
437+
response: NextResponse.json({ error: PERSONAL_KEY_DENIED }, { status: 403 }),
440438
}
441439
}
442440

‎apps/sim/app/api/v1/middleware.ts‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { type NextRequest, NextResponse } from 'next/server'
44
import type { ZodError } from 'zod'
55
import { getValidationErrorMessage, isZodError, validationErrorResponse } from '@/lib/api/server'
66
import { buildRateLimitHeaders, recordRateLimitSnapshot } from '@/lib/api/server/rate-limit-context'
7+
import { PERSONAL_KEY_DENIED, WORKSPACE_KEY_SCOPE_DENIED } from '@/lib/api-key/policy-messages'
78
import { getHighestPrioritySubscription } from '@/lib/billing/core/subscription'
89
import type { SubscriptionPlan } from '@/lib/core/rate-limiter'
910
import { getRateLimit, RateLimiter } from '@/lib/core/rate-limiter'
@@ -197,8 +198,8 @@ export function createRateLimitResponse(result: RateLimitResult): NextResponse {
197198
* Enforces two policies:
198199
* - A workspace-scoped key may only target its own workspace.
199200
* - A personal key is rejected when the workspace has disabled personal API
200-
* keys (`allowPersonalApiKeys = false`), matching the workflow-execution
201-
* surface in `app/api/workflows/middleware.ts`.
201+
* keys (`allowPersonalApiKeys = false`). Other surfaces enforcing the same
202+
* policy share `PERSONAL_KEY_DENIED`.
202203
*/
203204
export async function checkWorkspaceScope(
204205
rateLimit: RateLimitResult,
@@ -209,19 +210,13 @@ export async function checkWorkspaceScope(
209210
rateLimit.workspaceId &&
210211
rateLimit.workspaceId !== requestedWorkspaceId
211212
) {
212-
return NextResponse.json(
213-
{ error: 'API key is not authorized for this workspace' },
214-
{ status: 403 }
215-
)
213+
return NextResponse.json({ error: WORKSPACE_KEY_SCOPE_DENIED }, { status: 403 })
216214
}
217215

218216
if (rateLimit.keyType === 'personal') {
219217
const settings = await getWorkspaceBillingSettings(requestedWorkspaceId)
220218
if (!settings?.allowPersonalApiKeys) {
221-
return NextResponse.json(
222-
{ error: 'Personal API keys are not allowed for this workspace' },
223-
{ status: 403 }
224-
)
219+
return NextResponse.json({ error: PERSONAL_KEY_DENIED }, { status: 403 })
225220
}
226221
}
227222

‎apps/sim/app/api/workflows/[id]/execute/route.async.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@ vi.mock('@sim/utils/id', () => ({
194194
),
195195
}))
196196

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

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

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

18131814
expect(response.status).toBe(403)
18141815
await expect(response.json()).resolves.toEqual({
1815-
error: 'Personal API keys are not allowed for this workspace',
1816+
error: PERSONAL_KEY_DENIED,
18161817
})
18171818
expect(mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalled()
18181819
expect(mockPreprocessExecution).not.toHaveBeenCalled()

‎apps/sim/app/api/workflows/[id]/execute/route.ts‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
WORKFLOW_EXECUTION_ID_HEADER,
1414
WORKFLOW_EXECUTION_TIMEOUT_SECONDS_HEADER,
1515
} from '@/lib/api/contracts/workflows'
16+
import { PERSONAL_KEY_DENIED, WORKSPACE_KEY_SCOPE_DENIED } from '@/lib/api-key/policy-messages'
1617
import { AuthType, checkHybridAuth, hasExternalApiCredentials } from '@/lib/auth/hybrid'
1718
import { releaseExecutionSlot } from '@/lib/billing/calculations/usage-reservation'
1819
import {
@@ -1106,21 +1107,15 @@ async function handleExecutePost(
11061107
}
11071108
if (auth.authType === AuthType.API_KEY) {
11081109
if (auth.apiKeyType === 'workspace' && auth.workspaceId !== workflowWorkspaceId) {
1109-
return NextResponse.json(
1110-
{ error: 'API key is not authorized for this workspace' },
1111-
{ status: 403 }
1112-
)
1110+
return NextResponse.json({ error: WORKSPACE_KEY_SCOPE_DENIED }, { status: 403 })
11131111
}
11141112

11151113
if (auth.apiKeyType === 'personal') {
11161114
const workspaceSettings = workflowWorkspaceId
11171115
? await getWorkspaceBillingSettings(workflowWorkspaceId)
11181116
: null
11191117
if (!workspaceSettings?.allowPersonalApiKeys) {
1120-
return NextResponse.json(
1121-
{ error: 'Personal API keys are not allowed for this workspace' },
1122-
{ status: 403 }
1123-
)
1118+
return NextResponse.json({ error: PERSONAL_KEY_DENIED }, { status: 403 })
11241119
}
11251120
}
11261121
}

‎apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { and, eq, inArray } from 'drizzle-orm'
88
import { type NextRequest, NextResponse } from 'next/server'
99
import { cancelWorkflowExecutionContract } from '@/lib/api/contracts/workflows'
1010
import { parseRequest } from '@/lib/api/server'
11+
import { WORKSPACE_KEY_SCOPE_DENIED } from '@/lib/api-key/policy-messages'
1112
import { checkHybridAuth } from '@/lib/auth/hybrid'
1213
import { releaseExecutionSlot } from '@/lib/billing/calculations/usage-reservation'
1314
import { getJobQueue } from '@/lib/core/async-jobs'
@@ -344,10 +345,7 @@ export const POST = withRouteHandler(
344345
auth.apiKeyType === 'workspace' &&
345346
workflowAuthorization.workflow?.workspaceId !== auth.workspaceId
346347
) {
347-
return NextResponse.json(
348-
{ error: 'API key is not authorized for this workspace' },
349-
{ status: 403 }
350-
)
348+
return NextResponse.json({ error: WORKSPACE_KEY_SCOPE_DENIED }, { status: 403 })
351349
}
352350

353351
const execution = await db

‎apps/sim/app/api/workflows/[id]/route.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { eq, sql } from 'drizzle-orm'
1212
import { type NextRequest, NextResponse } from 'next/server'
1313
import { updateWorkflowContract } from '@/lib/api/contracts/workflows'
1414
import { parseRequest } from '@/lib/api/server'
15+
import { WORKSPACE_KEY_SCOPE_DENIED } from '@/lib/api-key/policy-messages'
1516
import { AuthType, checkHybridAuth, checkSessionOrInternalAuth } from '@/lib/auth/hybrid'
1617
import { generateRequestId } from '@/lib/core/utils/request'
1718
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -51,10 +52,7 @@ export const GET = withRouteHandler(
5152
}
5253

5354
if (auth.apiKeyType === 'workspace' && auth.workspaceId !== workflowData.workspaceId) {
54-
return NextResponse.json(
55-
{ error: 'API key is not authorized for this workspace' },
56-
{ status: 403 }
57-
)
55+
return NextResponse.json({ error: WORKSPACE_KEY_SCOPE_DENIED }, { status: 403 })
5856
}
5957

6058
if (isInternalCall && !userId) {

0 commit comments

Comments
 (0)