diff --git a/apps/sim/app/api/chat/manage/[id]/password/route.ts b/apps/sim/app/api/chat/manage/[id]/password/route.ts index ddc88e9594e..79b74c4506d 100644 --- a/apps/sim/app/api/chat/manage/[id]/password/route.ts +++ b/apps/sim/app/api/chat/manage/[id]/password/route.ts @@ -17,7 +17,7 @@ const PRIVATE_NO_STORE = { 'Cache-Control': 'private, no-store' } as const /** * GET endpoint that reveals a chat deployment's current password. - * Restricted to workspace admins (checkChatAccess requires admin permission + * Restricted to workspace editors (checkChatAccess requires write permission * on the workflow's workspace); each reveal is recorded in the audit log. */ export const GET = withRouteHandler( diff --git a/apps/sim/app/api/chat/manage/[id]/route.test.ts b/apps/sim/app/api/chat/manage/[id]/route.test.ts index f0ec9e9d695..9a0c1791ef2 100644 --- a/apps/sim/app/api/chat/manage/[id]/route.test.ts +++ b/apps/sim/app/api/chat/manage/[id]/route.test.ts @@ -23,13 +23,17 @@ import { import { NextRequest } from 'next/server' import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' -const { mockCheckChatAccess, mockCheckNeedsRedeployment, mockValidateChatDeployAuth } = vi.hoisted( - () => ({ - mockCheckChatAccess: vi.fn(), - mockCheckNeedsRedeployment: vi.fn(), - mockValidateChatDeployAuth: vi.fn(), - }) -) +const { + mockCheckChatAccess, + mockCanExposePublicly, + mockCheckNeedsRedeployment, + mockValidateChatDeployAuth, +} = vi.hoisted(() => ({ + mockCheckChatAccess: vi.fn(), + mockCanExposePublicly: vi.fn(), + mockCheckNeedsRedeployment: vi.fn(), + mockValidateChatDeployAuth: vi.fn(), +})) const mockCreateSuccessResponse = workflowsApiUtilsMockFns.mockCreateSuccessResponse const mockCreateErrorResponse = workflowsApiUtilsMockFns.mockCreateErrorResponse @@ -47,6 +51,10 @@ vi.mock('@/lib/core/security/encryption', () => encryptionMock) vi.mock('@/app/api/chat/utils', () => ({ checkChatAccess: mockCheckChatAccess, })) +vi.mock('@/lib/deployments/public-exposure', () => ({ + canExposePublicly: mockCanExposePublicly, +})) + vi.mock('@/ee/access-control/utils/permission-check', () => { class ChatDeployAuthNotAllowedError extends Error { constructor() { @@ -78,6 +86,9 @@ afterAll(() => { describe('Chat Edit API Route', () => { beforeEach(() => { vi.clearAllMocks() + // Existing chat suites deploy with authType public; default to admin so they + // keep testing what they were written to test. + mockCanExposePublicly.mockResolvedValue(true) resetDbChainMock() mockPerformChatUndeploy.mockResolvedValue({ success: true }) diff --git a/apps/sim/app/api/chat/manage/[id]/route.ts b/apps/sim/app/api/chat/manage/[id]/route.ts index 8088df80d29..e8dfcd1ef7d 100644 --- a/apps/sim/app/api/chat/manage/[id]/route.ts +++ b/apps/sim/app/api/chat/manage/[id]/route.ts @@ -12,6 +12,7 @@ import { isDev } from '@/lib/core/config/env-flags' import { encryptSecret } from '@/lib/core/security/encryption' import { getEmailDomain } from '@/lib/core/utils/urls' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { canExposePublicly } from '@/lib/deployments/public-exposure' import { checkNeedsRedeployment } from '@/lib/workflows/deployment-status' import { getWorkflowDeploymentSummary, @@ -127,6 +128,12 @@ export const PATCH = withRouteHandler( // mode actually changes, so a grandfathered mode already saved on this chat // can still be re-saved (e.g. a title-only edit) without a 403. if (authType && authType !== existingChatRecord.authType && chatWorkspaceId) { + // Only the transition *to* public is admin-gated. Leaving an already-public + // chat as-is, or moving it off public, does not increase exposure. + if (authType === 'public' && !(await canExposePublicly(session.user.id, chatWorkspaceId))) { + return createErrorResponse('Only admins can make a chat public', 403) + } + try { await validateChatDeployAuth(session.user.id, chatWorkspaceId, authType) } catch (error) { diff --git a/apps/sim/app/api/chat/route.test.ts b/apps/sim/app/api/chat/route.test.ts index 9772b567188..62f0080f2d5 100644 --- a/apps/sim/app/api/chat/route.test.ts +++ b/apps/sim/app/api/chat/route.test.ts @@ -16,8 +16,13 @@ import { import { NextRequest } from 'next/server' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' -const { mockCheckWorkflowAccessForChatCreation, mockValidateChatDeployAuth } = vi.hoisted(() => ({ +const { + mockCheckWorkflowAccessForChatCreation, + mockCanExposePublicly, + mockValidateChatDeployAuth, +} = vi.hoisted(() => ({ mockCheckWorkflowAccessForChatCreation: vi.fn(), + mockCanExposePublicly: vi.fn(), mockValidateChatDeployAuth: vi.fn(), })) @@ -31,6 +36,10 @@ vi.mock('@/app/api/chat/utils', () => ({ checkWorkflowAccessForChatCreation: mockCheckWorkflowAccessForChatCreation, })) +vi.mock('@/lib/deployments/public-exposure', () => ({ + canExposePublicly: mockCanExposePublicly, +})) + vi.mock('@/ee/access-control/utils/permission-check', () => { class ChatDeployAuthNotAllowedError extends Error { constructor() { @@ -53,6 +62,9 @@ describe('Chat API Route', () => { beforeEach(() => { vi.clearAllMocks() + // Existing chat suites deploy with authType public; default to admin so they + // keep testing what they were written to test. + mockCanExposePublicly.mockResolvedValue(true) setEnv({ NODE_ENV: 'development', NEXT_PUBLIC_APP_URL: 'http://localhost:3000' }) mockCreateSuccessResponse.mockImplementation((data) => { @@ -251,6 +263,65 @@ describe('Chat API Route', () => { ) }) + it('returns 403 when a non-admin deploys a public chat', async () => { + authMockFns.mockGetSession.mockResolvedValue({ + user: { id: 'user-id', email: 'user@example.com' }, + }) + + dbChainMockFns.limit.mockResolvedValueOnce([]) + mockCheckWorkflowAccessForChatCreation.mockResolvedValue({ + hasAccess: true, + workflow: { userId: 'user-id', workspaceId: 'workspace-1', isDeployed: true }, + }) + // Write access is enough to deploy a chat, but not to make one public. + mockCanExposePublicly.mockResolvedValue(false) + + const response = await POST( + new NextRequest('http://localhost:3000/api/chat', { + method: 'POST', + body: JSON.stringify({ + workflowId: 'workflow-123', + identifier: 'test-chat', + title: 'Test Chat', + authType: 'public', + customizations: { primaryColor: '#000000', welcomeMessage: 'Hello' }, + }), + }) + ) + + expect(response.status).toBe(403) + expect(mockCanExposePublicly).toHaveBeenCalledWith('user-id', 'workspace-1') + }) + + it('lets a non-admin deploy a password-protected chat', async () => { + authMockFns.mockGetSession.mockResolvedValue({ + user: { id: 'user-id', email: 'user@example.com' }, + }) + + dbChainMockFns.limit.mockResolvedValueOnce([]) + mockCheckWorkflowAccessForChatCreation.mockResolvedValue({ + hasAccess: true, + workflow: { userId: 'user-id', workspaceId: 'workspace-1', isDeployed: true }, + }) + mockCanExposePublicly.mockResolvedValue(false) + + const response = await POST( + new NextRequest('http://localhost:3000/api/chat', { + method: 'POST', + body: JSON.stringify({ + workflowId: 'workflow-123', + identifier: 'test-chat', + title: 'Test Chat', + authType: 'password', + password: 'test-password', + customizations: { primaryColor: '#000000', welcomeMessage: 'Hello' }, + }), + }) + ) + + expect(response.status).not.toBe(403) + }) + it('returns 403 when the chat auth type is blocked by the permission group', async () => { authMockFns.mockGetSession.mockResolvedValue({ user: { id: 'user-id', email: 'user@example.com' }, diff --git a/apps/sim/app/api/chat/route.ts b/apps/sim/app/api/chat/route.ts index e916d48b2da..d8470deeee1 100644 --- a/apps/sim/app/api/chat/route.ts +++ b/apps/sim/app/api/chat/route.ts @@ -8,6 +8,7 @@ import { createChatContract } from '@/lib/api/contracts/chats' import { getValidationErrorMessage, parseRequest } from '@/lib/api/server' import { getSession } from '@/lib/auth' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { canExposePublicly } from '@/lib/deployments/public-exposure' import { performChatDeploy } from '@/lib/workflows/orchestration' import { checkWorkflowAccessForChatCreation } from '@/app/api/chat/utils' import { createErrorResponse, createSuccessResponse } from '@/app/api/workflows/utils' @@ -113,6 +114,13 @@ export const POST = withRouteHandler(async (request: NextRequest) => { } if (workflowRecord.workspaceId) { + if ( + authType === 'public' && + !(await canExposePublicly(session.user.id, workflowRecord.workspaceId)) + ) { + return createErrorResponse('Only admins can deploy a public chat', 403) + } + try { await validateChatDeployAuth(session.user.id, workflowRecord.workspaceId, authType) } catch (error) { diff --git a/apps/sim/app/api/chat/utils.ts b/apps/sim/app/api/chat/utils.ts index 5b17f3cb6e8..76669a0db2b 100644 --- a/apps/sim/app/api/chat/utils.ts +++ b/apps/sim/app/api/chat/utils.ts @@ -28,7 +28,7 @@ export async function checkWorkflowAccessForChatCreation( const authorization = await authorizeWorkflowByWorkspacePermission({ workflowId, userId, - action: 'admin', + action: 'write', }) if (!authorization.workflow) { @@ -71,7 +71,7 @@ export async function checkChatAccess( const authorization = await authorizeWorkflowByWorkspacePermission({ workflowId: chatRecord.workflowId, userId, - action: 'admin', + action: 'write', }) return authorization.allowed diff --git a/apps/sim/app/api/mcp/workflow-servers/[id]/route.ts b/apps/sim/app/api/mcp/workflow-servers/[id]/route.ts index d765c4985f7..3a17405a9a3 100644 --- a/apps/sim/app/api/mcp/workflow-servers/[id]/route.ts +++ b/apps/sim/app/api/mcp/workflow-servers/[id]/route.ts @@ -9,6 +9,7 @@ import { workflowMcpServerParamsSchema, } from '@/lib/api/contracts/workflow-mcp-servers' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { canExposePublicly, increasesPublicExposure } from '@/lib/deployments/public-exposure' import { mcpBodyReadErrorResponse, readMcpJsonBodyWithLimit, @@ -109,6 +110,43 @@ export const PATCH = withRouteHandler( logger.info(`[${requestId}] Updating workflow MCP server: ${serverId}`) + /** + * `withMcpAuth('write')` covers managing the server, but a public + * server skips authentication on the serve path, so publishing one is + * admin-only. Only the transition is gated: the edit form resubmits the + * server's current visibility alongside whatever field changed, so a + * `write` member must still be able to rename an already-public server. + * + * This route calls the orchestration layer directly rather than the + * application use case, so `updateWorkflowMcpDeploymentServer`'s gate + * does not apply here and the rule has to be repeated. Delete this copy + * once the route goes through that use case. + */ + if (body.isPublic === true) { + const [current] = await db + .select({ isPublic: workflowMcpServer.isPublic }) + .from(workflowMcpServer) + .where( + and( + eq(workflowMcpServer.id, serverId), + eq(workflowMcpServer.workspaceId, workspaceId), + isNull(workflowMcpServer.deletedAt) + ) + ) + .limit(1) + + if ( + increasesPublicExposure(body.isPublic, current?.isPublic) && + !(await canExposePublicly(userId, workspaceId)) + ) { + return createMcpErrorResponse( + new Error('Only admins can make an MCP server public'), + 'Only admins can make an MCP server public', + 403 + ) + } + } + const result = await performUpdateWorkflowMcpServer({ serverId, workspaceId, diff --git a/apps/sim/app/api/mcp/workflow-servers/public-exposure.test.ts b/apps/sim/app/api/mcp/workflow-servers/public-exposure.test.ts new file mode 100644 index 00000000000..4291c828d4b --- /dev/null +++ b/apps/sim/app/api/mcp/workflow-servers/public-exposure.test.ts @@ -0,0 +1,169 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { mocks } = vi.hoisted(() => ({ + mocks: { + /** Mutable so a test can decide whether the server is already public. */ + currentServer: { id: 'srv-1', workspaceId: 'workspace-1', isPublic: false } as Record< + string, + unknown + >, + body: {} as Record, + getUserEntityPermissions: vi.fn(), + createServer: vi.fn(), + updateServer: vi.fn(), + errorResponse: vi.fn(), + }, +})) + +/** + * `withMcpAuth('write')` is the only authorization these routes carry, so the + * stub grants exactly that: a `write` caller who has already passed auth. What + * is under test is whether the public-exposure gate runs *after* it. + */ +vi.mock('@/lib/mcp/middleware', () => ({ + withMcpAuth: () => (handler: unknown) => (request: unknown, routeContext: unknown) => + (handler as (r: unknown, c: Record, rc: unknown) => Promise)( + request, + { + userId: 'editor-1', + userName: 'Editor', + userEmail: 'editor@example.com', + workspaceId: 'workspace-1', + requestId: 'req-1', + }, + routeContext + ), + readMcpJsonBodyWithLimit: async () => mocks.body, + mcpBodyReadErrorResponse: () => null, +})) + +vi.mock('@/lib/mcp/utils', () => ({ + createMcpErrorResponse: (_error: unknown, message: string, status: number) => + mocks.errorResponse({ message, status }) ?? { message, status }, + createMcpSuccessResponse: (data: unknown) => ({ status: 200, data }), + mcpOrchestrationStatus: () => 500, +})) + +vi.mock('@/lib/mcp/orchestration', () => ({ + performCreateWorkflowMcpServer: mocks.createServer, + performUpdateWorkflowMcpServer: mocks.updateServer, + performDeleteWorkflowMcpServer: vi.fn(), +})) + +vi.mock('@/lib/workspaces/permissions/utils', () => ({ + getUserEntityPermissions: mocks.getUserEntityPermissions, +})) + +vi.mock('@sim/db', () => { + const chain: Record = {} + for (const method of ['select', 'from', 'where', 'limit']) { + chain[method] = vi.fn(() => chain) + } + ;(chain as { then?: unknown }).then = (resolve: (rows: unknown[]) => unknown) => + resolve([mocks.currentServer]) + return { db: chain } +}) + +vi.mock('@sim/db/schema', () => ({ + workflowMcpServer: { id: {}, workspaceId: {}, deletedAt: {}, isPublic: {} }, + workflowMcpTool: {}, +})) + +vi.mock('drizzle-orm', () => ({ + and: vi.fn(), + eq: vi.fn(), + inArray: vi.fn(), + isNull: vi.fn(), + sql: vi.fn(), +})) + +import { PATCH } from '@/app/api/mcp/workflow-servers/[id]/route' +import { POST } from '@/app/api/mcp/workflow-servers/route' + +const request = {} as never +const routeContext = { params: Promise.resolve({ id: 'srv-1' }) } as never + +/** + * These routes call the orchestration layer directly rather than the + * application use case, so the use case's admin gate does not cover them. A + * public server needs no authentication to invoke, so a `write` member must not + * be able to publish one through the settings UI, which is what these hit. + */ +describe('workflow MCP server REST routes gate public exposure', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.currentServer.isPublic = false + mocks.createServer.mockResolvedValue({ + success: true, + server: { id: 'srv-1', name: 'srv' }, + addedTools: [], + }) + mocks.updateServer.mockResolvedValue({ + success: true, + server: { id: 'srv-1', name: 'renamed' }, + updatedFields: ['name'], + }) + }) + + it('rejects a write member creating a public server', async () => { + mocks.getUserEntityPermissions.mockResolvedValue('write') + mocks.body = { name: 'srv', isPublic: true } + + const response = (await POST(request, routeContext)) as { status: number } + + expect(response.status).toBe(403) + expect(mocks.createServer).not.toHaveBeenCalled() + }) + + it('allows an admin creating a public server', async () => { + mocks.getUserEntityPermissions.mockResolvedValue('admin') + mocks.body = { name: 'srv', isPublic: true } + + await POST(request, routeContext) + + expect(mocks.createServer).toHaveBeenCalledWith(expect.objectContaining({ isPublic: true })) + }) + + it('allows a write member creating a private server', async () => { + mocks.getUserEntityPermissions.mockResolvedValue('write') + mocks.body = { name: 'srv', isPublic: false } + + await POST(request, routeContext) + + expect(mocks.createServer).toHaveBeenCalled() + }) + + it('rejects a write member flipping a private server to public', async () => { + mocks.getUserEntityPermissions.mockResolvedValue('write') + mocks.body = { isPublic: true } + + const response = (await PATCH(request, routeContext)) as { status: number } + + expect(response.status).toBe(403) + expect(mocks.updateServer).not.toHaveBeenCalled() + }) + + it('allows a write member renaming an already-public server', async () => { + mocks.currentServer.isPublic = true + mocks.getUserEntityPermissions.mockResolvedValue('write') + mocks.body = { name: 'renamed', isPublic: true } + + await PATCH(request, routeContext) + + expect(mocks.updateServer).toHaveBeenCalledWith(expect.objectContaining({ name: 'renamed' })) + }) + + it('allows a write member making a public server private', async () => { + mocks.currentServer.isPublic = true + mocks.getUserEntityPermissions.mockResolvedValue('write') + mocks.body = { isPublic: false } + + await PATCH(request, routeContext) + + expect(mocks.updateServer).toHaveBeenCalled() + expect(mocks.getUserEntityPermissions).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/app/api/mcp/workflow-servers/route.ts b/apps/sim/app/api/mcp/workflow-servers/route.ts index 10398e6eeb4..0b9010629b6 100644 --- a/apps/sim/app/api/mcp/workflow-servers/route.ts +++ b/apps/sim/app/api/mcp/workflow-servers/route.ts @@ -6,6 +6,7 @@ import { and, eq, inArray, isNull, sql } from 'drizzle-orm' import type { NextRequest } from 'next/server' import { createWorkflowMcpServerBodySchema } from '@/lib/api/contracts/workflow-mcp-servers' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { canExposePublicly, increasesPublicExposure } from '@/lib/deployments/public-exposure' import { mcpBodyReadErrorResponse, readMcpJsonBodyWithLimit, @@ -115,6 +116,25 @@ export const POST = withRouteHandler( workflowIds: body.workflowIds, }) + /** + * `withMcpAuth('write')` covers managing the server, but a public + * server skips authentication on the serve path, so publishing one is + * admin-only. This route calls the orchestration layer directly rather + * than the application use case, so `createWorkflowMcpDeploymentServer`'s + * gate does not apply here and the rule has to be repeated. Delete this + * copy once the route goes through that use case. + */ + if ( + increasesPublicExposure(body.isPublic, false) && + !(await canExposePublicly(userId, workspaceId)) + ) { + return createMcpErrorResponse( + new Error('Only admins can make an MCP server public'), + 'Only admins can make an MCP server public', + 403 + ) + } + const result = await performCreateWorkflowMcpServer({ workspaceId, userId, diff --git a/apps/sim/app/api/tools/deployments/deploy/route.ts b/apps/sim/app/api/tools/deployments/deploy/route.ts index 0795475b549..fcdf49b7e48 100644 --- a/apps/sim/app/api/tools/deployments/deploy/route.ts +++ b/apps/sim/app/api/tools/deployments/deploy/route.ts @@ -39,7 +39,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { const { workflowId, workspaceId, name, description } = parsed.data.body - const access = await authorizeDeploymentWorkflow(auth.userId, workflowId, workspaceId, 'admin') + const access = await authorizeDeploymentWorkflow(auth.userId, workflowId, workspaceId, 'write') if (!access.ok) return access.response await assertWorkflowMutable(workflowId) diff --git a/apps/sim/app/api/tools/deployments/promote/route.ts b/apps/sim/app/api/tools/deployments/promote/route.ts index 523a5630a32..52f18676354 100644 --- a/apps/sim/app/api/tools/deployments/promote/route.ts +++ b/apps/sim/app/api/tools/deployments/promote/route.ts @@ -39,7 +39,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { const { workflowId, workspaceId, version } = parsed.data.body - const access = await authorizeDeploymentWorkflow(auth.userId, workflowId, workspaceId, 'admin') + const access = await authorizeDeploymentWorkflow(auth.userId, workflowId, workspaceId, 'write') if (!access.ok) return access.response await assertWorkflowMutable(workflowId) diff --git a/apps/sim/app/api/tools/deployments/routes.test.ts b/apps/sim/app/api/tools/deployments/routes.test.ts index f0313a779b3..29ec7f92528 100644 --- a/apps/sim/app/api/tools/deployments/routes.test.ts +++ b/apps/sim/app/api/tools/deployments/routes.test.ts @@ -112,7 +112,7 @@ describe('POST /api/tools/deployments/deploy', () => { expect(mockPerformFullDeploy).not.toHaveBeenCalled() }) - it('requires admin permission on the workflow workspace', async () => { + it('requires write permission on the workflow workspace', async () => { workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ allowed: false, status: 403, @@ -129,7 +129,7 @@ describe('POST /api/tools/deployments/deploy', () => { expect(workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith({ workflowId: WORKFLOW_ID, userId: 'user-1', - action: 'admin', + action: 'write', }) expect(mockPerformFullDeploy).not.toHaveBeenCalled() }) @@ -221,6 +221,28 @@ describe('POST /api/tools/deployments/undeploy', () => { expect(mockPerformFullUndeploy).not.toHaveBeenCalled() }) + it('requires write permission on the workflow workspace', async () => { + workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: false, + status: 403, + message: 'Insufficient permissions', + workflow: WORKFLOW_RECORD, + workspacePermission: 'read', + }) + + const response = await undeployPost( + makePost('undeploy', { workflowId: WORKFLOW_ID, workspaceId: 'ws-1' }) + ) + + expect(response.status).toBe(403) + expect(workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith({ + workflowId: WORKFLOW_ID, + userId: 'user-1', + action: 'write', + }) + expect(mockPerformFullUndeploy).not.toHaveBeenCalled() + }) + it('undeploys a deployed workflow', async () => { const response = await undeployPost( makePost('undeploy', { workflowId: WORKFLOW_ID, workspaceId: 'ws-1' }) @@ -254,6 +276,28 @@ describe('POST /api/tools/deployments/promote', () => { }) }) + it('requires write permission on the workflow workspace', async () => { + workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ + allowed: false, + status: 403, + message: 'Insufficient permissions', + workflow: WORKFLOW_RECORD, + workspacePermission: 'read', + }) + + const response = await promotePost( + makePost('promote', { workflowId: WORKFLOW_ID, workspaceId: 'ws-1', version: 3 }) + ) + + expect(response.status).toBe(403) + expect(workflowAuthzMockFns.mockAuthorizeWorkflowByWorkspacePermission).toHaveBeenCalledWith({ + workflowId: WORKFLOW_ID, + userId: 'user-1', + action: 'write', + }) + expect(mockPerformActivateVersion).not.toHaveBeenCalled() + }) + it('promotes the given version to live', async () => { const response = await promotePost( makePost('promote', { workflowId: WORKFLOW_ID, workspaceId: 'ws-1', version: 3 }) diff --git a/apps/sim/app/api/tools/deployments/undeploy/route.ts b/apps/sim/app/api/tools/deployments/undeploy/route.ts index 942cf9f4895..7d7f167419b 100644 --- a/apps/sim/app/api/tools/deployments/undeploy/route.ts +++ b/apps/sim/app/api/tools/deployments/undeploy/route.ts @@ -38,7 +38,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => { const { workflowId, workspaceId } = parsed.data.body - const access = await authorizeDeploymentWorkflow(auth.userId, workflowId, workspaceId, 'admin') + const access = await authorizeDeploymentWorkflow(auth.userId, workflowId, workspaceId, 'write') if (!access.ok) return access.response if (!access.workflow.isDeployed) { diff --git a/apps/sim/app/api/tools/deployments/utils.ts b/apps/sim/app/api/tools/deployments/utils.ts index 2812b86d39a..567cefb97f8 100644 --- a/apps/sim/app/api/tools/deployments/utils.ts +++ b/apps/sim/app/api/tools/deployments/utils.ts @@ -45,15 +45,15 @@ export async function authenticateDeploymentToolRequest( /** * Verifies the user holds the required workspace permission on the target * workflow and that the workflow belongs to the calling workspace. Deployment - * mutations require `admin`, reads require `read`, matching the UI deploy + * mutations require `write`, reads require `read`, matching the UI deploy * routes. The workspace binding keeps workflow-driven executions (schedules, - * webhooks) from reaching into other workspaces the actor administers. + * webhooks) from reaching into other workspaces the actor can edit. */ export async function authorizeDeploymentWorkflow( userId: string, workflowId: string, workspaceId: string, - action: 'read' | 'admin' + action: 'read' | 'write' ): Promise< { ok: true; workflow: AuthorizedDeploymentWorkflow } | { ok: false; response: NextResponse } > { diff --git a/apps/sim/app/api/v1/workflows/[id]/deploy/route.test.ts b/apps/sim/app/api/v1/workflows/[id]/deploy/route.test.ts index 7ec6df59634..ac5aa0d3875 100644 --- a/apps/sim/app/api/v1/workflows/[id]/deploy/route.test.ts +++ b/apps/sim/app/api/v1/workflows/[id]/deploy/route.test.ts @@ -2,7 +2,7 @@ * @vitest-environment node * * Tests for POST/DELETE /api/v1/workflows/[id]/deploy — verifies auth, - * workspace admin permission enforcement, optional body handling, and the + * workspace write permission enforcement, optional body handling, and the * mapping of orchestration results to v1 API responses. */ @@ -121,7 +121,7 @@ describe('POST /api/v1/workflows/[id]/deploy', () => { expect(mockPerformFullDeploy).not.toHaveBeenCalled() }) - it('masks missing admin permission as 404', async () => { + it('masks missing write permission as 404', async () => { mockValidateWorkspaceAccess.mockResolvedValue( NextResponse.json({ error: 'Access denied' }, { status: 403 }) ) @@ -133,7 +133,7 @@ describe('POST /api/v1/workflows/[id]/deploy', () => { expect.objectContaining({ allowed: true }), 'user-1', 'ws-1', - 'admin' + 'write' ) expect(mockPerformFullDeploy).not.toHaveBeenCalled() }) @@ -277,7 +277,7 @@ describe('DELETE /api/v1/workflows/[id]/deploy', () => { ) }) - it('masks missing admin permission as 404', async () => { + it('masks missing write permission as 404', async () => { mockValidateWorkspaceAccess.mockResolvedValue( NextResponse.json({ error: 'Access denied' }, { status: 403 }) ) diff --git a/apps/sim/app/api/v1/workflows/[id]/rollback/route.test.ts b/apps/sim/app/api/v1/workflows/[id]/rollback/route.test.ts index 2327f71325b..4066ba58ea1 100644 --- a/apps/sim/app/api/v1/workflows/[id]/rollback/route.test.ts +++ b/apps/sim/app/api/v1/workflows/[id]/rollback/route.test.ts @@ -217,7 +217,7 @@ describe('POST /api/v1/workflows/[id]/rollback', () => { expect(response.status).toBe(404) }) - it('masks missing admin permission as 404', async () => { + it('masks missing write permission as 404', async () => { mockValidateWorkspaceAccess.mockResolvedValue( NextResponse.json({ error: 'Access denied' }, { status: 403 }) ) @@ -229,7 +229,7 @@ describe('POST /api/v1/workflows/[id]/rollback', () => { expect.objectContaining({ allowed: true }), 'user-1', 'ws-1', - 'admin' + 'write' ) expect(mockPerformActivateVersion).not.toHaveBeenCalled() }) diff --git a/apps/sim/app/api/v1/workflows/utils.ts b/apps/sim/app/api/v1/workflows/utils.ts index f2cb6d059a9..80a42693e11 100644 --- a/apps/sim/app/api/v1/workflows/utils.ts +++ b/apps/sim/app/api/v1/workflows/utils.ts @@ -11,7 +11,7 @@ function workflowNotFoundResponse(): NextResponse { /** * Resolves the target workflow for a v1 deployment mutation: loads the active - * record and verifies the caller's admin permission on its workspace. Access + * record and verifies the caller's write permission on its workspace. Access * failures are masked as 404, matching the v1 workflow read surface so * unauthorized callers cannot probe workflow existence. */ @@ -25,7 +25,7 @@ export async function resolveV1DeploymentWorkflow( return { ok: false, response: workflowNotFoundResponse() } } - const accessError = await validateWorkspaceAccess(rateLimit, userId, target.workspaceId, 'admin') + const accessError = await validateWorkspaceAccess(rateLimit, userId, target.workspaceId, 'write') if (accessError) { return { ok: false, response: workflowNotFoundResponse() } } diff --git a/apps/sim/app/api/workspaces/[id]/api-keys/route.test.ts b/apps/sim/app/api/workspaces/[id]/api-keys/route.test.ts index 39eb2394e0b..70d1fc31d4f 100644 --- a/apps/sim/app/api/workspaces/[id]/api-keys/route.test.ts +++ b/apps/sim/app/api/workspaces/[id]/api-keys/route.test.ts @@ -10,19 +10,24 @@ import { } from '@sim/testing' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' -const { mockGetApiKeyDisplayFormat, mockGetUserEntityPermissions, mockGetWorkspaceById } = - vi.hoisted(() => ({ - mockGetApiKeyDisplayFormat: vi.fn(), - mockGetUserEntityPermissions: vi.fn(), - mockGetWorkspaceById: vi.fn(), - })) +const { + mockGetApiKeyDisplayFormat, + mockGetUserEntityPermissions, + mockGetWorkspaceById, + mockPerformCreateWorkspaceApiKey, +} = vi.hoisted(() => ({ + mockGetApiKeyDisplayFormat: vi.fn(), + mockGetUserEntityPermissions: vi.fn(), + mockGetWorkspaceById: vi.fn(), + mockPerformCreateWorkspaceApiKey: vi.fn(), +})) vi.mock('@/lib/api-key/auth', () => ({ getApiKeyDisplayFormat: mockGetApiKeyDisplayFormat, })) vi.mock('@/lib/api-key/orchestration', () => ({ - performCreateWorkspaceApiKey: vi.fn(), + performCreateWorkspaceApiKey: mockPerformCreateWorkspaceApiKey, })) vi.mock('@/lib/workspaces/permissions/utils', () => ({ @@ -30,7 +35,7 @@ vi.mock('@/lib/workspaces/permissions/utils', () => ({ getWorkspaceById: mockGetWorkspaceById, })) -import { GET } from '@/app/api/workspaces/[id]/api-keys/route' +import { GET, POST } from '@/app/api/workspaces/[id]/api-keys/route' const mockGetSession = authMockFns.mockGetSession @@ -81,3 +86,73 @@ describe('GET /api/workspaces/[id]/api-keys', () => { expect(mockGetApiKeyDisplayFormat).toHaveBeenCalledWith('sim_plaintext_legacy_secret') }) }) + +/** + * Deploying a workflow only requires workspace `write`, but a workspace API key + * can invoke every deployed workflow in the workspace, so minting one stays + * admin-only. These pin that boundary. + */ +describe('POST /api/workspaces/[id]/api-keys', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + mockGetSession.mockResolvedValue({ user: { id: 'editor-1' } }) + mockGetWorkspaceById.mockResolvedValue({ id: 'workspace-1' }) + }) + + afterAll(() => { + resetDbChainMock() + }) + + it.each(['write', 'read'] as const)( + 'rejects a %s member without reaching key creation', + async (permission) => { + mockGetUserEntityPermissions.mockResolvedValue(permission) + + const response = await POST(createMockRequest('POST', { name: 'deploy-key' }), { + params: Promise.resolve({ id: 'workspace-1' }), + }) + + expect(response.status).toBe(403) + expect(mockPerformCreateWorkspaceApiKey).not.toHaveBeenCalled() + } + ) + + it('allows an admin to create a workspace key', async () => { + mockGetSession.mockResolvedValue({ user: { id: 'admin-1' } }) + mockGetUserEntityPermissions.mockResolvedValue('admin') + mockPerformCreateWorkspaceApiKey.mockResolvedValue({ + success: true, + key: { + id: 'key-2', + name: 'deploy-key', + key: 'sim_plaintext_new_secret', + createdAt: new Date('2026-08-01T00:00:00.000Z'), + }, + }) + + const response = await POST(createMockRequest('POST', { name: 'deploy-key' }), { + params: Promise.resolve({ id: 'workspace-1' }), + }) + + expect(response.status).toBe(200) + expect(mockPerformCreateWorkspaceApiKey).toHaveBeenCalledWith( + expect.objectContaining({ workspaceId: 'workspace-1', userId: 'admin-1', name: 'deploy-key' }) + ) + }) + + it('maps a forbidden orchestration result to 403', async () => { + mockGetUserEntityPermissions.mockResolvedValue('admin') + mockPerformCreateWorkspaceApiKey.mockResolvedValue({ + success: false, + error: 'Admin permission is required to create a workspace API key', + errorCode: 'forbidden', + }) + + const response = await POST(createMockRequest('POST', { name: 'deploy-key' }), { + params: Promise.resolve({ id: 'workspace-1' }), + }) + + expect(response.status).toBe(403) + }) +}) diff --git a/apps/sim/app/api/workspaces/[id]/api-keys/route.ts b/apps/sim/app/api/workspaces/[id]/api-keys/route.ts index ea9521e37b6..5b897fe8dd5 100644 --- a/apps/sim/app/api/workspaces/[id]/api-keys/route.ts +++ b/apps/sim/app/api/workspaces/[id]/api-keys/route.ts @@ -119,7 +119,8 @@ export const POST = withRouteHandler( actorEmail: session.user.email, }) if (!result.success || !result.key) { - const status = result.errorCode === 'conflict' ? 409 : 500 + const status = + result.errorCode === 'conflict' ? 409 : result.errorCode === 'forbidden' ? 403 : 500 return NextResponse.json({ error: result.error }, { status }) } diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.test.ts index 0067ef31f47..8deeb774159 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.test.ts @@ -275,6 +275,7 @@ describe('chunked parse — property test over randomized documents', () => { } } expect(failures).toEqual([]) - // 400 docs each parsed+serialized twice — generous timeout so it can't flake under parallel load. - }, 30000) + // 400 docs each parsed+serialized twice. Runs ~13s alone, but this is a CPU-bound synchronous + // loop competing with every other worker in a full-suite run, where 30s was not enough headroom. + }, 90000) }) diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/workflow-mcp-servers/components/create-workflow-mcp-server-modal/create-workflow-mcp-server-modal.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/workflow-mcp-servers/components/create-workflow-mcp-server-modal/create-workflow-mcp-server-modal.tsx index 2417477fb83..81cc2c68389 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/workflow-mcp-servers/components/create-workflow-mcp-server-modal/create-workflow-mcp-server-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/workflow-mcp-servers/components/create-workflow-mcp-server-modal/create-workflow-mcp-server-modal.tsx @@ -12,8 +12,10 @@ import { ChipModalHeader, ChipSelect, type ComboboxOption, + Tooltip, } from '@sim/emcn' import { createLogger } from '@sim/logger' +import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider' import { useCreateWorkflowMcpServer } from '@/hooks/queries/workflow-mcp-servers' const logger = createLogger('CreateWorkflowMcpServerModal') @@ -38,6 +40,13 @@ export function CreateWorkflowMcpServerModal({ workflowOptions, }: CreateWorkflowMcpServerModalProps) { const createServerMutation = useCreateWorkflowMcpServer() + /** + * A public server is callable with no authentication, so publishing one is + * admin-only — the same boundary the public workflow API and public chats + * enforce. The server rejects it regardless; this keeps a `write` member from + * being offered an option that would only 403 on save. + */ + const canSetPublicAccess = useUserPermissionsContext().canAdmin const [formData, setFormData] = useState({ ...INITIAL_FORM_DATA }) const [selectedWorkflowIds, setSelectedWorkflowIds] = useState([]) @@ -115,13 +124,25 @@ export function CreateWorkflowMcpServerModal({ )}
- setFormData({ ...formData, isPublic: value === 'public' })} - > - API Key - Public - + + + + + setFormData({ ...formData, isPublic: value === 'public' }) + } + disabled={!canSetPublicAccess} + > + API Key + Public + + + + {!canSetPublicAccess && ( + Only admins can change public access + )} + {formData.isPublic && ( No authentication required diff --git a/apps/sim/app/workspace/[workspaceId]/settings/components/workflow-mcp-servers/workflow-mcp-servers.tsx b/apps/sim/app/workspace/[workspaceId]/settings/components/workflow-mcp-servers/workflow-mcp-servers.tsx index d635b71894b..a3839bd7773 100644 --- a/apps/sim/app/workspace/[workspaceId]/settings/components/workflow-mcp-servers/workflow-mcp-servers.tsx +++ b/apps/sim/app/workspace/[workspaceId]/settings/components/workflow-mcp-servers/workflow-mcp-servers.tsx @@ -19,6 +19,7 @@ import { Code, type ComboboxOption, Label, + Tooltip, useCopyToClipboard, } from '@sim/emcn' import { ArrowLeft, Check, Clipboard, Plus, Server } from '@sim/emcn/icons' @@ -86,6 +87,7 @@ function ServerDetailView({ onDelete, isDeleting, }: ServerDetailViewProps) { + const workspacePermissions = useUserPermissionsContext() const { data, isLoading, error } = useWorkflowMcpServer(workspaceId, serverId) const { data: deployedWorkflows = [], isLoading: isLoadingWorkflows } = useDeployedWorkflows(workspaceId) @@ -99,7 +101,15 @@ function ServerDetailView({ const existingKeyNames = (apiKeysData?.workspaceKeys ?? []).map((key) => key.name) const allowPersonalApiKeys = false - const canManageWorkspaceKeys = canManage + /** Managing this server only needs `write`, but minting a workspace key needs `admin`. */ + const canManageWorkspaceKeys = workspacePermissions.canAdmin + /** + * A public server is callable with no authentication, so changing its access + * is admin-only — the same boundary the public workflow API and public chats + * enforce. The server rejects the change regardless; this keeps a `write` + * member from being offered an action that would only 403. + */ + const canSetPublicAccess = workspacePermissions.canAdmin const defaultKeyType = 'workspace' const addToWorkspaceMutation = useCreateMcpServer() @@ -609,7 +619,7 @@ function ServerDetailView({ {!server.isPublic && (

Replace $SIM_API_KEY with your API key - {canManage && ( + {canManageWorkspaceKeys && ( <> , or{' '}