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
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
}
}
Copy link

Choose a reason for hiding this comment

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

Empty URL bypasses domain validation in PATCH

Medium Severity

The PATCH endpoint uses a truthy check if (updateData.url) to guard domain validation, allowing empty strings to bypass validation. When an allowlist is configured and a client sends url: "" in the update, validation is skipped (empty string is falsy), and the database is updated with an unusable empty URL. The validateMcpDomain function correctly rejects empty strings when allowlist is configured, but the guard prevents it from being called. This creates broken server state.

Fix in Cursor Fix in Web


// 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
}
Copy link

Choose a reason for hiding this comment

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

Missing URL validation before domain check

Medium Severity

The POST endpoint validates domain before checking if URL exists. When an allowlist is configured and URL is missing, users get "Domain (empty) is not allowed" instead of "URL is required", creating confusing error messages. The test-connection endpoint correctly checks URL presence first (lines 67-73), but the POST endpoint doesn't follow this pattern.

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
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 @@ -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
}
}

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))
}, [])

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
50 changes: 25 additions & 25 deletions apps/sim/ee/sso/components/sso-settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import { useState } from 'react'
import { createLogger } from '@sim/logger'
import { Check, ChevronDown, Copy, Eye, EyeOff } from 'lucide-react'
import { Check, ChevronDown, Clipboard, Eye, EyeOff } from 'lucide-react'
import { Button, Combobox, Input, Switch, Textarea } from '@/components/emcn'
import { Skeleton } from '@/components/ui'
import { useSession } from '@/lib/auth/auth-client'
Expand Down Expand Up @@ -418,29 +418,29 @@ export function SSO() {

{/* Callback URL */}
<div className='flex flex-col gap-[8px]'>
<span className='font-medium text-[13px] text-[var(--text-secondary)]'>
Callback URL
</span>
<div className='relative'>
<div className='flex h-9 items-center rounded-[6px] border bg-[var(--surface-1)] px-[10px] pr-[40px]'>
<code className='flex-1 truncate font-mono text-[13px] text-[var(--text-primary)]'>
{providerCallbackUrl}
</code>
</div>
<div className='flex items-center justify-between'>
<span className='font-medium text-[13px] text-[var(--text-secondary)]'>
Callback URL
</span>
<Button
type='button'
variant='ghost'
onClick={() => copyToClipboard(providerCallbackUrl)}
className='-translate-y-1/2 absolute top-1/2 right-[4px] h-[28px] w-[28px] rounded-[4px] text-[var(--text-muted)] hover:text-[var(--text-primary)]'
className='h-[22px] w-[22px] rounded-[4px] p-0 text-[var(--text-muted)] hover:text-[var(--text-primary)]'
>
{copied ? (
<Check className='h-[14px] w-[14px]' />
<Check className='h-[13px] w-[13px]' />
) : (
<Copy className='h-[14px] w-[14px]' />
<Clipboard className='h-[13px] w-[13px]' />
)}
<span className='sr-only'>Copy callback URL</span>
</Button>
</div>
<div className='flex h-9 items-center rounded-[6px] border bg-[var(--surface-1)] px-[10px]'>
<code className='flex-1 truncate font-mono text-[13px] text-[var(--text-primary)]'>
{providerCallbackUrl}
</code>
</div>
<p className='text-[13px] text-[var(--text-muted)]'>
Configure this in your identity provider
</p>
Expand Down Expand Up @@ -852,29 +852,29 @@ export function SSO() {

{/* Callback URL display */}
<div className='flex flex-col gap-[8px]'>
<span className='font-medium text-[13px] text-[var(--text-secondary)]'>
Callback URL
</span>
<div className='relative'>
<div className='flex h-9 items-center rounded-[6px] border bg-[var(--surface-1)] px-[10px] pr-[40px]'>
<code className='flex-1 truncate font-mono text-[13px] text-[var(--text-primary)]'>
{callbackUrl}
</code>
</div>
<div className='flex items-center justify-between'>
<span className='font-medium text-[13px] text-[var(--text-secondary)]'>
Callback URL
</span>
<Button
type='button'
variant='ghost'
onClick={() => copyToClipboard(callbackUrl)}
className='-translate-y-1/2 absolute top-1/2 right-[4px] h-[28px] w-[28px] rounded-[4px] text-[var(--text-muted)] hover:text-[var(--text-primary)]'
className='h-[22px] w-[22px] rounded-[4px] p-0 text-[var(--text-muted)] hover:text-[var(--text-primary)]'
>
{copied ? (
<Check className='h-[14px] w-[14px]' />
<Check className='h-[13px] w-[13px]' />
) : (
<Copy className='h-[14px] w-[14px]' />
<Clipboard className='h-[13px] w-[13px]' />
)}
<span className='sr-only'>Copy callback URL</span>
</Button>
</div>
<div className='flex h-9 items-center rounded-[6px] border bg-[var(--surface-1)] px-[10px]'>
<code className='flex-1 truncate font-mono text-[13px] text-[var(--text-primary)]'>
{callbackUrl}
</code>
</div>
<p className='text-[13px] text-[var(--text-muted)]'>
Configure this in your identity provider
</p>
Expand Down
1 change: 1 addition & 0 deletions apps/sim/lib/core/config/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export const env = createEnv({
EXA_API_KEY: z.string().min(1).optional(), // Exa AI API key for enhanced online search
BLACKLISTED_PROVIDERS: z.string().optional(), // Comma-separated provider IDs to hide (e.g., "openai,anthropic")
BLACKLISTED_MODELS: z.string().optional(), // Comma-separated model names/prefixes to hide (e.g., "gpt-4,claude-*")
ALLOWED_MCP_DOMAINS: z.string().optional(), // Comma-separated domains for MCP servers (e.g., "internal.company.com,mcp.example.org"). Empty = all allowed.

// Azure Configuration - Shared credentials with feature-specific models
AZURE_OPENAI_ENDPOINT: z.string().url().optional(), // Shared Azure OpenAI service endpoint
Expand Down
29 changes: 29 additions & 0 deletions apps/sim/lib/core/config/feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,35 @@ export const isReactGrabEnabled = isDev && isTruthy(env.REACT_GRAB_ENABLED)
*/
export const isReactScanEnabled = isDev && isTruthy(env.REACT_SCAN_ENABLED)

/**
* Normalizes a domain entry from the ALLOWED_MCP_DOMAINS env var.
* Accepts bare hostnames (e.g., "mcp.company.com") or full URLs (e.g., "https://mcp.company.com").
* Extracts the hostname in either case.
*/
function normalizeDomainEntry(entry: string): string {
const trimmed = entry.trim().toLowerCase()
if (!trimmed) return ''
if (trimmed.includes('://')) {
try {
return new URL(trimmed).hostname
} catch {
return trimmed
}
}
return trimmed
}

/**
* Get allowed MCP server domains from the ALLOWED_MCP_DOMAINS env var.
* Returns null if not set (all domains allowed), or parsed array of lowercase hostnames.
* Accepts both bare hostnames and full URLs in the env var value.
*/
export function getAllowedMcpDomainsFromEnv(): string[] | null {
if (!env.ALLOWED_MCP_DOMAINS) return null
const parsed = env.ALLOWED_MCP_DOMAINS.split(',').map(normalizeDomainEntry).filter(Boolean)
return parsed.length > 0 ? parsed : null
}

/**
* Get cost multiplier based on environment
*/
Expand Down
Loading