Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
12 changes: 12 additions & 0 deletions apps/sim/app/api/mcp/servers/[id]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { mcpServers } from '@sim/db/schema'
import { createLogger } from '@sim/logger'
import { and, eq, isNull } from 'drizzle-orm'
import type { NextRequest } from 'next/server'
import { McpDomainNotAllowedError, validateMcpDomain } from '@/lib/mcp/domain-check'
import { getParsedBody, withMcpAuth } from '@/lib/mcp/middleware'
import { mcpService } from '@/lib/mcp/service'
import { createMcpErrorResponse, createMcpSuccessResponse } from '@/lib/mcp/utils'
Expand All @@ -29,6 +30,17 @@ export const PATCH = withMcpAuth<{ id: string }>('write')(
// Remove workspaceId from body to prevent it from being updated
const { workspaceId: _, ...updateData } = body

if (updateData.url) {
try {
validateMcpDomain(updateData.url)
} catch (e) {
if (e instanceof McpDomainNotAllowedError) {
return createMcpErrorResponse(e, e.message, 403)
}
throw e
}
}

// Get the current server to check if URL is changing
const [currentServer] = await db
.select({ url: mcpServers.url })
Expand Down
10 changes: 10 additions & 0 deletions apps/sim/app/api/mcp/servers/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { mcpServers } from '@sim/db/schema'
import { createLogger } from '@sim/logger'
import { and, eq, isNull } from 'drizzle-orm'
import type { NextRequest } from 'next/server'
import { McpDomainNotAllowedError, validateMcpDomain } from '@/lib/mcp/domain-check'
import { getParsedBody, withMcpAuth } from '@/lib/mcp/middleware'
import { mcpService } from '@/lib/mcp/service'
import {
Expand Down Expand Up @@ -72,6 +73,15 @@ export const POST = withMcpAuth('write')(
)
}

try {
validateMcpDomain(body.url)
} catch (e) {
if (e instanceof McpDomainNotAllowedError) {
return createMcpErrorResponse(e, e.message, 403)
}
throw e
}
Comment thread
waleedlatif1 marked this conversation as resolved.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Domain validation runs on optional URLs

Medium Severity

Domain validation executes unconditionally on body.url even when undefined, causing the creation endpoint to reject servers without URLs when an MCP domain allowlist is configured. This contradicts the deliberate design choice to allow creating servers without URLs (evidenced by line 86 generating random UUIDs when URL is missing), and creates inconsistency with the PATCH endpoint which only validates domains when a URL is actually provided.

Fix in Cursor Fix in Web


const serverId = body.url ? generateMcpServerId(workspaceId, body.url) : crypto.randomUUID()

const [existingServer] = await db
Expand Down
10 changes: 10 additions & 0 deletions apps/sim/app/api/mcp/servers/test-connection/route.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { createLogger } from '@sim/logger'
import type { NextRequest } from 'next/server'
import { McpClient } from '@/lib/mcp/client'
import { McpDomainNotAllowedError, validateMcpDomain } from '@/lib/mcp/domain-check'
import { getParsedBody, withMcpAuth } from '@/lib/mcp/middleware'
import { resolveMcpConfigEnvVars } from '@/lib/mcp/resolve-config'
import type { McpTransport } from '@/lib/mcp/types'
Expand Down Expand Up @@ -71,6 +72,15 @@ export const POST = withMcpAuth('write')(
)
}

try {
validateMcpDomain(body.url)
} catch (e) {
if (e instanceof McpDomainNotAllowedError) {
return createMcpErrorResponse(e, e.message, 403)
}
throw e
}

// Build initial config for resolution
const initialConfig = {
id: `test-${requestId}`,
Expand Down
14 changes: 14 additions & 0 deletions apps/sim/app/api/settings/allowed-integrations/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { NextResponse } from 'next/server'
import { getSession } from '@/lib/auth'
import { getAllowedIntegrationsFromEnv } from '@/lib/core/config/feature-flags'

export async function GET() {
const session = await getSession()
if (!session?.user?.id) {
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
}

return NextResponse.json({
allowedIntegrations: getAllowedIntegrationsFromEnv(),
})
}
27 changes: 27 additions & 0 deletions apps/sim/app/api/settings/allowed-mcp-domains/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { NextResponse } from 'next/server'
import { getSession } from '@/lib/auth'
import { getAllowedMcpDomainsFromEnv } from '@/lib/core/config/feature-flags'
import { getBaseUrl } from '@/lib/core/utils/urls'

export async function GET() {
const session = await getSession()
if (!session?.user?.id) {
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
}

const configuredDomains = getAllowedMcpDomainsFromEnv()
if (configuredDomains === null) {
return NextResponse.json({ allowedMcpDomains: null })
}

try {
const platformHostname = new URL(getBaseUrl()).hostname.toLowerCase()
if (!configuredDomains.includes(platformHostname)) {
return NextResponse.json({
allowedMcpDomains: [...configuredDomains, platformHostname],
})
}
} catch {}

return NextResponse.json({ allowedMcpDomains: configuredDomains })
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { ParallelTool } from '@/app/workspace/[workspaceId]/w/[workflowId]/compo
import { getDisplayValue } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block'
import { getBlock } from '@/blocks/registry'
import type { CopilotToolCall } from '@/stores/panel'
import { useCopilotStore } from '@/stores/panel'
import { useCopilotStore, usePanelStore } from '@/stores/panel'
import type { SubAgentContentBlock } from '@/stores/panel/copilot/types'
import { useWorkflowStore } from '@/stores/workflows/workflow/store'

Expand Down Expand Up @@ -341,16 +341,20 @@ export function OptionsSelector({
const [hoveredIndex, setHoveredIndex] = useState(-1)
const [chosenKey, setChosenKey] = useState<string | null>(selectedOptionKey)
const containerRef = useRef<HTMLDivElement>(null)
const activeTab = usePanelStore((s) => s.activeTab)

const isLocked = chosenKey !== null

// Handle keyboard navigation - only for the active options selector
// Handle keyboard navigation - only for the active options selector when copilot is active
useEffect(() => {
if (isInteractionDisabled || !enableKeyboardNav || isLocked) return

const handleKeyDown = (e: KeyboardEvent) => {
if (e.defaultPrevented) return

// Only handle keyboard shortcuts when the copilot panel is active
if (activeTab !== 'copilot') return

const activeElement = document.activeElement
const isInputFocused =
activeElement?.tagName === 'INPUT' ||
Expand Down Expand Up @@ -387,7 +391,15 @@ export function OptionsSelector({

document.addEventListener('keydown', handleKeyDown)
return () => document.removeEventListener('keydown', handleKeyDown)
}, [isInteractionDisabled, enableKeyboardNav, isLocked, sortedOptions, hoveredIndex, onSelect])
}, [
isInteractionDisabled,
enableKeyboardNav,
isLocked,
sortedOptions,
hoveredIndex,
onSelect,
activeTab,
])

if (sortedOptions.length === 0) return null

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,18 @@ export function isBlockProtected(blockId: string, blocks: Record<string, BlockSt

/**
* Checks if an edge is protected from modification.
* An edge is protected if either its source or target block is protected.
* An edge is protected only if its target block is protected.
* Outbound connections from locked blocks are allowed to be modified.
*
* @param edge - The edge to check (must have source and target)
* @param blocks - Record of all blocks in the workflow
* @returns True if the edge is protected
* @returns True if the edge is protected (target is locked)
*/
export function isEdgeProtected(
edge: { source: string; target: string },
blocks: Record<string, BlockState>
): boolean {
return isBlockProtected(edge.source, blocks) || isBlockProtected(edge.target, blocks)
return isBlockProtected(edge.target, blocks)
}

/**
Expand Down
10 changes: 5 additions & 5 deletions apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2523,7 +2523,7 @@ const WorkflowContent = React.memo(() => {
.filter((change: any) => change.type === 'remove')
.map((change: any) => change.id)
.filter((edgeId: string) => {
// Prevent removing edges connected to protected blocks
// Prevent removing edges targeting protected blocks
const edge = edges.find((e) => e.id === edgeId)
if (!edge) return true
return !isEdgeProtected(edge, blocks)
Expand Down Expand Up @@ -2595,7 +2595,7 @@ const WorkflowContent = React.memo(() => {

if (!sourceNode || !targetNode) return

// Prevent connections to/from protected blocks
// Prevent connections to protected blocks (outbound from locked blocks is allowed)
if (isEdgeProtected(connection, blocks)) {
addNotification({
level: 'info',
Expand Down Expand Up @@ -3357,12 +3357,12 @@ const WorkflowContent = React.memo(() => {
/** Stable delete handler to avoid creating new function references per edge. */
const handleEdgeDelete = useCallback(
(edgeId: string) => {
// Prevent removing edges connected to protected blocks
// Prevent removing edges targeting protected blocks
const edge = edges.find((e) => e.id === edgeId)
if (edge && isEdgeProtected(edge, blocks)) {
addNotification({
level: 'info',
message: 'Cannot remove connections from locked blocks',
message: 'Cannot remove connections to locked blocks',
workflowId: activeWorkflowId || undefined,
})
return
Expand Down Expand Up @@ -3420,7 +3420,7 @@ const WorkflowContent = React.memo(() => {

// Handle edge deletion first (edges take priority if selected)
if (selectedEdges.size > 0) {
// Get all selected edge IDs and filter out edges connected to protected blocks
// Get all selected edge IDs and filter out edges targeting protected blocks
const edgeIds = Array.from(selectedEdges.values()).filter((edgeId) => {
const edge = edges.find((e) => e.id === edgeId)
if (!edge) return true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,13 +223,11 @@ export function Integrations({ onOpenChange, registerCloseHandler }: Integration
}
}

// Group services by provider, filtering by permission config
const groupedServices = services.reduce(
(acc, service) => {
// Filter based on allowedIntegrations
if (
permissionConfig.allowedIntegrations !== null &&
!permissionConfig.allowedIntegrations.includes(service.id)
!permissionConfig.allowedIntegrations.includes(service.id.replace(/-/g, '_'))
Comment thread
waleedlatif1 marked this conversation as resolved.
Outdated
) {
return acc
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,21 @@ interface McpServer {

const logger = createLogger('McpSettings')

/**
* Checks if a URL's hostname is in the allowed domains list.
* Returns true if no allowlist is configured (null) or the domain matches.
*/
function isDomainAllowed(url: string | undefined, allowedDomains: string[] | null): boolean {
if (allowedDomains === null) return true
if (!url) return true
try {
const hostname = new URL(url).hostname.toLowerCase()
return allowedDomains.includes(hostname)
} catch {
return false
}
Comment thread
waleedlatif1 marked this conversation as resolved.
}

const DEFAULT_FORM_DATA: McpServerFormData = {
name: '',
transport: 'streamable-http',
Expand Down Expand Up @@ -390,6 +405,15 @@ export function MCP({ initialServerId }: MCPProps) {
} = useMcpServerTest()
const availableEnvVars = useAvailableEnvVarKeys(workspaceId)

const [allowedMcpDomains, setAllowedMcpDomains] = useState<string[] | null>(null)

useEffect(() => {
fetch('/api/settings/allowed-mcp-domains')
.then((res) => res.json())
.then((data) => setAllowedMcpDomains(data.allowedMcpDomains ?? null))
.catch(() => setAllowedMcpDomains(null))
Comment thread
waleedlatif1 marked this conversation as resolved.
}, [])

const urlInputRef = useRef<HTMLInputElement>(null)

const [showAddForm, setShowAddForm] = useState(false)
Expand Down Expand Up @@ -1006,10 +1030,12 @@ export function MCP({ initialServerId }: MCPProps) {
const showNoResults = searchTerm.trim() && filteredServers.length === 0 && servers.length > 0

const isFormValid = formData.name.trim() && formData.url?.trim()
const isSubmitDisabled = serversLoading || isAddingServer || !isFormValid
const isAddDomainBlocked = !isDomainAllowed(formData.url, allowedMcpDomains)
const isSubmitDisabled = serversLoading || isAddingServer || !isFormValid || isAddDomainBlocked
const testButtonLabel = getTestButtonLabel(testResult, isTestingConnection)

const isEditFormValid = editFormData.name.trim() && editFormData.url?.trim()
const isEditDomainBlocked = !isDomainAllowed(editFormData.url, allowedMcpDomains)
const editTestButtonLabel = getTestButtonLabel(editTestResult, isEditTestingConnection)
const hasEditChanges = useMemo(() => {
if (editFormData.name !== editOriginalData.name) return true
Expand Down Expand Up @@ -1299,6 +1325,11 @@ export function MCP({ initialServerId }: MCPProps) {
onChange={(e) => handleEditInputChange('url', e.target.value)}
onScroll={setEditUrlScrollLeft}
/>
{isEditDomainBlocked && (
<p className='mt-[4px] text-[12px] text-[var(--text-error)]'>
Domain not permitted by server policy
</p>
)}
</FormField>

<div className='flex flex-col gap-[8px]'>
Expand Down Expand Up @@ -1351,7 +1382,7 @@ export function MCP({ initialServerId }: MCPProps) {
<Button
variant='default'
onClick={handleEditTestConnection}
disabled={isEditTestingConnection || !isEditFormValid}
disabled={isEditTestingConnection || !isEditFormValid || isEditDomainBlocked}
>
{editTestButtonLabel}
</Button>
Expand All @@ -1361,7 +1392,9 @@ export function MCP({ initialServerId }: MCPProps) {
</Button>
<Button
onClick={handleSaveEdit}
disabled={!hasEditChanges || isUpdatingServer || !isEditFormValid}
disabled={
!hasEditChanges || isUpdatingServer || !isEditFormValid || isEditDomainBlocked
}
variant='tertiary'
>
{isUpdatingServer ? 'Saving...' : 'Save'}
Expand Down Expand Up @@ -1434,6 +1467,11 @@ export function MCP({ initialServerId }: MCPProps) {
onChange={(e) => handleInputChange('url', e.target.value)}
onScroll={(scrollLeft) => handleUrlScroll(scrollLeft)}
/>
{isAddDomainBlocked && (
<p className='mt-[4px] text-[12px] text-[var(--text-error)]'>
Domain not permitted by server policy
</p>
)}
</FormField>

<div className='flex flex-col gap-[8px]'>
Expand Down Expand Up @@ -1479,7 +1517,7 @@ export function MCP({ initialServerId }: MCPProps) {
<Button
variant='default'
onClick={handleTestConnection}
disabled={isTestingConnection || !isFormValid}
disabled={isTestingConnection || !isFormValid || isAddDomainBlocked}
>
{testButtonLabel}
</Button>
Expand All @@ -1489,7 +1527,9 @@ export function MCP({ initialServerId }: MCPProps) {
Cancel
</Button>
<Button onClick={handleAddServer} disabled={isSubmitDisabled} variant='tertiary'>
{isSubmitDisabled && isFormValid ? 'Adding...' : 'Add Server'}
{isSubmitDisabled && isFormValid && !isAddDomainBlocked
? 'Adding...'
: 'Add Server'}
</Button>
</div>
</div>
Expand Down
Loading