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 89e2d46051b..367a2bb6345 100644 --- a/apps/sim/app/api/mcp/workflow-servers/[id]/route.ts +++ b/apps/sim/app/api/mcp/workflow-servers/[id]/route.ts @@ -97,7 +97,7 @@ export const PATCH = withRouteHandler( )( async ( request: NextRequest, - { userId, userName, userEmail, workspaceId, requestId }, + { userId, userName, userEmail, workspaceId, requestId, permission }, { params } ) => { try { @@ -122,6 +122,7 @@ export const PATCH = withRouteHandler( name: body.name, description: body.description, isPublic: body.isPublic, + allowVisibilityChange: permission === 'admin', }) if (!result.success || !result.server) { const status = mcpOrchestrationStatus(result.errorCode) diff --git a/apps/sim/app/api/mcp/workflow-servers/route.ts b/apps/sim/app/api/mcp/workflow-servers/route.ts index 127384261ba..f7a8f90fb19 100644 --- a/apps/sim/app/api/mcp/workflow-servers/route.ts +++ b/apps/sim/app/api/mcp/workflow-servers/route.ts @@ -103,54 +103,60 @@ export const POST = withRouteHandler( withMcpAuth( 'write', 'deploy.mcp' - )(async (request: NextRequest, { userId, userName, userEmail, workspaceId, requestId }) => { - try { - const rawBody = await readMcpJsonBodyWithLimit(request) - const parsedBody = createWorkflowMcpServerBodySchema.safeParse(rawBody) + )( + async ( + request: NextRequest, + { userId, userName, userEmail, workspaceId, requestId, permission } + ) => { + try { + const rawBody = await readMcpJsonBodyWithLimit(request) + const parsedBody = createWorkflowMcpServerBodySchema.safeParse(rawBody) + + if (!parsedBody.success) { + return createMcpErrorResponse(parsedBody.error, 'Invalid request format', 400) + } - if (!parsedBody.success) { - return createMcpErrorResponse(parsedBody.error, 'Invalid request format', 400) - } + const body = parsedBody.data - const body = parsedBody.data - - logger.info(`[${requestId}] Creating workflow MCP server:`, { - name: body.name, - workspaceId, - workflowIds: body.workflowIds, - }) - - const result = await performCreateWorkflowMcpServer({ - workspaceId, - userId, - actorName: userName, - actorEmail: userEmail, - name: body.name, - description: body.description, - isPublic: body.isPublic, - workflowIds: body.workflowIds, - }) - if (!result.success || !result.server) { - return createMcpErrorResponse( - new Error(result.error || 'Failed to create workflow MCP server'), - result.error || 'Failed to create workflow MCP server', - mcpOrchestrationStatus(result.errorCode) - ) - } + logger.info(`[${requestId}] Creating workflow MCP server:`, { + name: body.name, + workspaceId, + workflowIds: body.workflowIds, + }) - const { server } = result - const addedTools = result.addedTools || [] + const result = await performCreateWorkflowMcpServer({ + workspaceId, + userId, + actorName: userName, + actorEmail: userEmail, + name: body.name, + description: body.description, + isPublic: body.isPublic, + allowVisibilityChange: permission === 'admin', + workflowIds: body.workflowIds, + }) + if (!result.success || !result.server) { + return createMcpErrorResponse( + new Error(result.error || 'Failed to create workflow MCP server'), + result.error || 'Failed to create workflow MCP server', + mcpOrchestrationStatus(result.errorCode) + ) + } - logger.info( - `[${requestId}] Successfully created workflow MCP server: ${body.name} (ID: ${server.id})` - ) + const { server } = result + const addedTools = result.addedTools || [] - return createMcpSuccessResponse({ server, addedTools }, 201) - } catch (error) { - const bodyErrorResponse = mcpBodyReadErrorResponse(error, request) - if (bodyErrorResponse) return bodyErrorResponse - logger.error(`[${requestId}] Error creating workflow MCP server:`, error) - return createMcpErrorResponse(toError(error), 'Failed to create workflow MCP server', 500) + logger.info( + `[${requestId}] Successfully created workflow MCP server: ${body.name} (ID: ${server.id})` + ) + + return createMcpSuccessResponse({ server, addedTools }, 201) + } catch (error) { + const bodyErrorResponse = mcpBodyReadErrorResponse(error, request) + if (bodyErrorResponse) return bodyErrorResponse + logger.error(`[${requestId}] Error creating workflow MCP server:`, error) + return createMcpErrorResponse(toError(error), 'Failed to create workflow MCP server', 500) + } } - }) + ) ) 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 7e8992c2866..be27186f16b 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 @@ -29,6 +29,7 @@ interface CreateWorkflowMcpServerModalProps { onOpenChange: (open: boolean) => void workspaceId: string workflowOptions?: ComboboxOption[] + canChangeVisibility: boolean } export function CreateWorkflowMcpServerModal({ @@ -36,6 +37,7 @@ export function CreateWorkflowMcpServerModal({ onOpenChange, workspaceId, workflowOptions, + canChangeVisibility, }: CreateWorkflowMcpServerModalProps) { const createServerMutation = useCreateWorkflowMcpServer() @@ -118,6 +120,7 @@ export function CreateWorkflowMcpServerModal({ setFormData({ ...formData, isPublic: value === 'public' })} + disabled={!canChangeVisibility} > Private Public 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 5320c8e3410..7b24593efdf 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 @@ -67,6 +67,7 @@ const logger = createLogger('WorkflowMcpServers') interface ServerDetailViewProps { canManage: boolean + canChangeVisibility: boolean workspaceId: string serverId: string onBack: () => void @@ -80,6 +81,7 @@ type McpClientType = 'sim' | 'cursor' | 'codex' | 'claude-code' | 'claude-deskto function ServerDetailView({ canManage, + canChangeVisibility, workspaceId, serverId, onBack, @@ -854,6 +856,7 @@ function ServerDetailView({ setEditServerIsPublic(value === 'public')} + disabled={!canChangeVisibility} > Private Public @@ -1002,6 +1005,7 @@ export function WorkflowMcpServers() { <> { @@ -1094,6 +1098,7 @@ export function WorkflowMcpServers() { onOpenChange={setShowAddModal} workspaceId={workspaceId} workflowOptions={workflowOptions} + canChangeVisibility={workspacePermissions.canAdmin} /> )} diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/mcp/mcp.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/mcp/mcp.tsx index 226ea8f03f8..374a5504fd6 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/mcp/mcp.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/mcp/mcp.tsx @@ -23,6 +23,7 @@ import { } from '@/lib/mcp/workflow-tool-schema' import type { InputFormatField } from '@/lib/workflows/types' import { useWorkflowScope } from '@/app/workspace/[workspaceId]/providers/workflow-scope-provider' +import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider' import { CreateWorkflowMcpServerModal } from '@/app/workspace/[workspaceId]/settings/components/workflow-mcp-servers/components/create-workflow-mcp-server-modal' import { useAddWorkflowMcpTool, @@ -133,6 +134,7 @@ export function McpDeploy({ onActiveServerChange, }: McpDeployProps) { const { workspaceId } = useWorkflowScope() + const { canAdmin } = useUserPermissionsContext() const [showCreateModal, setShowCreateModal] = useState(false) const { data: servers = [], isLoading: isLoadingServers } = useWorkflowMcpServers(workspaceId) @@ -513,6 +515,7 @@ export function McpDeploy({ open={showCreateModal} onOpenChange={setShowCreateModal} workspaceId={workspaceId} + canChangeVisibility={canAdmin} /> ) diff --git a/apps/sim/lib/mcp/application/workflow-deployments.ts b/apps/sim/lib/mcp/application/workflow-deployments.ts index 8e8ea2aac83..d0125df9b8d 100644 --- a/apps/sim/lib/mcp/application/workflow-deployments.ts +++ b/apps/sim/lib/mcp/application/workflow-deployments.ts @@ -211,6 +211,8 @@ export const createWorkflowMcpDeploymentServer = defineAuthorizedWorkspaceUseCas async execute({ principal, input, context }) { const result = await performCreateWorkflowMcpServer({ ...input, + // Safe only because the operation's `minimumRole` is `admin`. + allowVisibilityChange: true, workspaceId: context.workspaceId, userId: attribution(principal, context.billedAccountUserId), projectLegacyAudit: false, @@ -259,6 +261,8 @@ export const updateWorkflowMcpDeploymentServer = defineAuthorizedWorkspaceUseCas async execute({ principal, input, context }) { const result = await performUpdateWorkflowMcpServer({ ...input, + // Safe only because the operation's `minimumRole` is `admin`. + allowVisibilityChange: true, workspaceId: context.workspaceId, userId: attribution(principal, context.billedAccountUserId), projectLegacyAudit: false, diff --git a/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.test.ts b/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.test.ts index 5e19be724c6..9dca5401769 100644 --- a/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.test.ts +++ b/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.test.ts @@ -29,6 +29,7 @@ describe('workflow MCP lifecycle orchestration', () => { const result = await performCreateWorkflowMcpServer({ workspaceId: 'workspace-1', userId: 'user-1', + allowVisibilityChange: false, name: 'Too Many Tools', workflowIds: Array.from( { length: MAX_MCP_TOOLS_PER_SERVER + 1 }, @@ -47,6 +48,7 @@ describe('workflow MCP lifecycle orchestration', () => { const result = await performCreateWorkflowMcpServer({ workspaceId: 'workspace-1', userId: 'user-1', + allowVisibilityChange: false, name: 'Duplicate Tools', workflowIds: ['wf-1', 'wf-1'], }) @@ -86,6 +88,7 @@ describe('workflow MCP lifecycle orchestration', () => { const result = await performCreateWorkflowMcpServer({ workspaceId: 'workspace-1', userId: 'user-1', + allowVisibilityChange: false, name: 'Server', workflowIds: ['wf-1'], }) @@ -121,6 +124,7 @@ describe('workflow MCP lifecycle orchestration', () => { const result = await performCreateWorkflowMcpServer({ workspaceId: 'workspace-1', userId: 'user-1', + allowVisibilityChange: false, name: 'Server', workflowIds: ['wf-1'], }) @@ -158,6 +162,7 @@ describe('workflow MCP lifecycle orchestration', () => { const result = await performCreateWorkflowMcpServer({ workspaceId: 'workspace-1', userId: 'user-1', + allowVisibilityChange: false, name: 'Server', workflowIds: ['wf-1'], }) diff --git a/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.ts b/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.ts index 1cb3a5ffcf0..74c189035b2 100644 --- a/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.ts +++ b/apps/sim/lib/mcp/orchestration/workflow-mcp-lifecycle.ts @@ -63,7 +63,16 @@ interface ActorMetadata { publishEffects?: boolean } -export interface PerformCreateWorkflowMcpServerParams extends ActorMetadata { +interface VisibilityAuthority { + /** + * Whether the caller may set `isPublic`. A public server answers + * `/api/mcp/serve/{serverId}` with no Sim credential, so this is reserved for + * workspace admins. + */ + allowVisibilityChange: boolean +} + +export interface PerformCreateWorkflowMcpServerParams extends ActorMetadata, VisibilityAuthority { workspaceId: string userId: string name: string @@ -80,7 +89,7 @@ export interface PerformCreateWorkflowMcpServerResult { addedTools?: Array<{ workflowId: string; toolName: string }> } -export interface PerformUpdateWorkflowMcpServerParams extends ActorMetadata { +export interface PerformUpdateWorkflowMcpServerParams extends ActorMetadata, VisibilityAuthority { serverId: string workspaceId: string userId: string @@ -339,6 +348,14 @@ async function validateWorkflowMcpServerMembershipBudget( export async function performCreateWorkflowMcpServer( params: PerformCreateWorkflowMcpServerParams ): Promise { + if (params.isPublic && !params.allowVisibilityChange) { + return { + success: false, + error: 'Only workspace admins can create a public workflow MCP server', + errorCode: 'forbidden', + } + } + try { const name = params.name.trim() const workflowIds = params.workflowIds || [] @@ -558,9 +575,17 @@ export async function performUpdateWorkflowMcpServer( if (params.name !== undefined) updateData.name = params.name.trim() if (params.description !== undefined) updateData.description = params.description?.trim() || null - if (params.isPublic !== undefined) updateData.isPublic = params.isPublic + const lockedVisibility = params.allowVisibilityChange ? undefined : params.isPublic + if (params.allowVisibilityChange && params.isPublic !== undefined) { + updateData.isPublic = params.isPublic + } const updatedFields = auditUpdatedFields(updateData) + const serverInWorkspace = and( + eq(workflowMcpServer.id, params.serverId), + eq(workflowMcpServer.workspaceId, params.workspaceId), + isNull(workflowMcpServer.deletedAt) + ) try { const [server] = await db @@ -568,14 +593,29 @@ export async function performUpdateWorkflowMcpServer( .set(updateData) .where( and( - eq(workflowMcpServer.id, params.serverId), - eq(workflowMcpServer.workspaceId, params.workspaceId), - isNull(workflowMcpServer.deletedAt) + serverInWorkspace, + lockedVisibility !== undefined + ? eq(workflowMcpServer.isPublic, lockedVisibility) + : undefined ) ) .returning() if (!server) { + if (lockedVisibility !== undefined) { + const [existing] = await db + .select({ id: workflowMcpServer.id }) + .from(workflowMcpServer) + .where(serverInWorkspace) + .limit(1) + if (existing) { + return { + success: false, + error: 'Only workspace admins can change whether a workflow MCP server is public', + errorCode: 'forbidden', + } + } + } return { success: false, error: 'Server not found', errorCode: 'not_found' } } diff --git a/apps/sim/lib/mcp/orchestration/workflow-mcp-visibility.integration.ts b/apps/sim/lib/mcp/orchestration/workflow-mcp-visibility.integration.ts new file mode 100644 index 00000000000..b2d216d3b5c --- /dev/null +++ b/apps/sim/lib/mcp/orchestration/workflow-mcp-visibility.integration.ts @@ -0,0 +1,128 @@ +import { readTestDatabaseUrl } from '@sim/db/testing/test-infrastructure' +import { withUtcTimestamps } from '@sim/db/timestamps' +import { generateId } from '@sim/utils/id' +import { drizzle, type PostgresJsDatabase } from 'drizzle-orm/postgres-js' +import postgres from 'postgres' +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' + +const database = vi.hoisted(() => ({ current: undefined as PostgresJsDatabase | undefined })) +vi.mock('@sim/db', async () => ({ + ...(await import('@sim/db/schema')), + get db() { + if (!database.current) throw new Error('Test database not initialized') + return database.current + }, +})) + +import { + type PerformUpdateWorkflowMcpServerParams, + performCreateWorkflowMcpServer, + performUpdateWorkflowMcpServer, +} from '@/lib/mcp/orchestration/workflow-mcp-lifecycle' + +/** + * A public workflow MCP server runs its workflows for anyone holding the URL, so + * only a caller allowed to change visibility may set `isPublic`; everyone else + * keeps editing the server without touching it. + */ +describe('workflow MCP server visibility in PostgreSQL', () => { + const schema = `workflow_mcp_visibility_${generateId().replaceAll('-', '')}` + const connection = postgres( + readTestDatabaseUrl(), + withUtcTimestamps({ max: 2, connection: { search_path: schema }, onnotice: () => undefined }) + ) + const actor = { projectLegacyAudit: false, publishEffects: false } as const + + beforeAll(async () => { + await connection`CREATE SCHEMA ${connection(schema)}` + await connection`CREATE TABLE workflow_mcp_server ( + id text PRIMARY KEY, workspace_id text NOT NULL, created_by text NOT NULL, + name text NOT NULL, description text, is_public boolean NOT NULL DEFAULT false, + deleted_at timestamp, created_at timestamp NOT NULL DEFAULT now(), + updated_at timestamp NOT NULL DEFAULT now() + )` + database.current = drizzle(connection) + }) + + afterAll(async () => { + await connection`DROP SCHEMA ${connection(schema)} CASCADE` + await connection.end() + }) + + beforeEach(async () => { + await connection`DELETE FROM workflow_mcp_server` + await connection`INSERT INTO workflow_mcp_server (id, workspace_id, created_by, name, is_public) + VALUES ('private', 'ws', 'owner', 'Private', false), ('public', 'ws', 'owner', 'Public', true)` + }) + + async function visibility(id: string) { + const [row] = await connection<{ name: string; is_public: boolean }[]>` + SELECT name, is_public FROM workflow_mcp_server WHERE id = ${id}` + return row + } + + function update( + serverId: string, + allowVisibilityChange: boolean, + fields: Partial + ) { + return performUpdateWorkflowMcpServer({ + ...actor, + serverId, + workspaceId: 'ws', + userId: 'member', + allowVisibilityChange, + ...fields, + }) + } + + it('refuses to make a private server public without visibility authority', async () => { + const result = await update('private', false, { isPublic: true }) + expect(result).toMatchObject({ success: false, errorCode: 'forbidden' }) + expect(await visibility('private')).toEqual({ name: 'Private', is_public: false }) + }) + + it('refuses to make a public server private without visibility authority', async () => { + const result = await update('public', false, { name: 'Renamed', isPublic: false }) + expect(result).toMatchObject({ success: false, errorCode: 'forbidden' }) + expect(await visibility('public')).toEqual({ name: 'Public', is_public: true }) + }) + + it('saves other fields when the submitted visibility is unchanged', async () => { + const result = await update('public', false, { name: 'Renamed', isPublic: true }) + expect(result).toMatchObject({ success: true, server: { name: 'Renamed', isPublic: true } }) + expect(result.updatedFields).not.toContain('isPublic') + expect(await visibility('public')).toEqual({ name: 'Renamed', is_public: true }) + }) + + it('reports a missing server as not found, not forbidden', async () => { + const result = await update('missing', false, { isPublic: true }) + expect(result).toMatchObject({ success: false, errorCode: 'not_found' }) + }) + + it('does not reveal a server in another workspace', async () => { + const result = await update('private', false, { isPublic: true, workspaceId: 'other-ws' }) + expect(result).toMatchObject({ success: false, errorCode: 'not_found' }) + }) + + it('changes visibility with visibility authority', async () => { + const result = await update('private', true, { isPublic: true }) + expect(result).toMatchObject({ success: true, server: { isPublic: true } }) + expect(await visibility('private')).toEqual({ name: 'Private', is_public: true }) + }) + + it('refuses to create a public server without visibility authority', async () => { + const result = await performCreateWorkflowMcpServer({ + ...actor, + workspaceId: 'ws', + userId: 'member', + name: 'Exposed', + isPublic: true, + allowVisibilityChange: false, + }) + expect(result).toMatchObject({ success: false, errorCode: 'forbidden' }) + const [{ count }] = await connection<{ count: number }[]>` + SELECT count(*)::int AS count FROM workflow_mcp_server` + expect(count).toBe(2) + }) +})