Skip to content

Commit a65da5e

Browse files
committed
cleanup
1 parent bc7ece0 commit a65da5e

File tree

2 files changed

+172
-2
lines changed

2 files changed

+172
-2
lines changed
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const mockGetAllowedMcpDomainsFromEnv = vi.fn<() => string[] | null>()
7+
const mockGetBaseUrl = vi.fn<() => string>()
8+
9+
vi.doMock('@/lib/core/config/feature-flags', () => ({
10+
getAllowedMcpDomainsFromEnv: mockGetAllowedMcpDomainsFromEnv,
11+
}))
12+
13+
vi.doMock('@/lib/core/utils/urls', () => ({
14+
getBaseUrl: mockGetBaseUrl,
15+
}))
16+
17+
const { McpDomainNotAllowedError, isMcpDomainAllowed, validateMcpDomain } = await import(
18+
'./domain-check'
19+
)
20+
21+
describe('McpDomainNotAllowedError', () => {
22+
it.concurrent('creates error with correct name and message', () => {
23+
const error = new McpDomainNotAllowedError('evil.com')
24+
25+
expect(error).toBeInstanceOf(Error)
26+
expect(error).toBeInstanceOf(McpDomainNotAllowedError)
27+
expect(error.name).toBe('McpDomainNotAllowedError')
28+
expect(error.message).toContain('evil.com')
29+
})
30+
})
31+
32+
describe('isMcpDomainAllowed', () => {
33+
beforeEach(() => {
34+
vi.clearAllMocks()
35+
})
36+
37+
describe('when no allowlist is configured', () => {
38+
beforeEach(() => {
39+
mockGetAllowedMcpDomainsFromEnv.mockReturnValue(null)
40+
})
41+
42+
it('allows any URL', () => {
43+
expect(isMcpDomainAllowed('https://any-server.com/mcp')).toBe(true)
44+
})
45+
46+
it('allows undefined URL', () => {
47+
expect(isMcpDomainAllowed(undefined)).toBe(true)
48+
})
49+
50+
it('allows empty string URL', () => {
51+
expect(isMcpDomainAllowed('')).toBe(true)
52+
})
53+
})
54+
55+
describe('when allowlist is configured', () => {
56+
beforeEach(() => {
57+
mockGetAllowedMcpDomainsFromEnv.mockReturnValue(['allowed.com', 'internal.company.com'])
58+
mockGetBaseUrl.mockReturnValue('https://platform.example.com')
59+
})
60+
61+
it('allows URLs on the allowlist', () => {
62+
expect(isMcpDomainAllowed('https://allowed.com/mcp')).toBe(true)
63+
expect(isMcpDomainAllowed('https://internal.company.com/tools')).toBe(true)
64+
})
65+
66+
it('rejects URLs not on the allowlist', () => {
67+
expect(isMcpDomainAllowed('https://evil.com/mcp')).toBe(false)
68+
})
69+
70+
it('rejects undefined URL (fail-closed)', () => {
71+
expect(isMcpDomainAllowed(undefined)).toBe(false)
72+
})
73+
74+
it('rejects empty string URL (fail-closed)', () => {
75+
expect(isMcpDomainAllowed('')).toBe(false)
76+
})
77+
78+
it('rejects malformed URLs', () => {
79+
expect(isMcpDomainAllowed('not-a-url')).toBe(false)
80+
})
81+
82+
it('matches case-insensitively', () => {
83+
expect(isMcpDomainAllowed('https://ALLOWED.COM/mcp')).toBe(true)
84+
})
85+
86+
it('always allows the platform hostname', () => {
87+
expect(isMcpDomainAllowed('https://platform.example.com/mcp')).toBe(true)
88+
})
89+
90+
it('allows platform hostname even when not in the allowlist', () => {
91+
mockGetAllowedMcpDomainsFromEnv.mockReturnValue(['other.com'])
92+
expect(isMcpDomainAllowed('https://platform.example.com/mcp')).toBe(true)
93+
})
94+
})
95+
96+
describe('when getBaseUrl is not configured', () => {
97+
beforeEach(() => {
98+
mockGetAllowedMcpDomainsFromEnv.mockReturnValue(['allowed.com'])
99+
mockGetBaseUrl.mockImplementation(() => {
100+
throw new Error('Not configured')
101+
})
102+
})
103+
104+
it('still allows URLs on the allowlist', () => {
105+
expect(isMcpDomainAllowed('https://allowed.com/mcp')).toBe(true)
106+
})
107+
108+
it('still rejects URLs not on the allowlist', () => {
109+
expect(isMcpDomainAllowed('https://evil.com/mcp')).toBe(false)
110+
})
111+
})
112+
})
113+
114+
describe('validateMcpDomain', () => {
115+
beforeEach(() => {
116+
vi.clearAllMocks()
117+
})
118+
119+
describe('when no allowlist is configured', () => {
120+
beforeEach(() => {
121+
mockGetAllowedMcpDomainsFromEnv.mockReturnValue(null)
122+
})
123+
124+
it('does not throw for any URL', () => {
125+
expect(() => validateMcpDomain('https://any-server.com/mcp')).not.toThrow()
126+
})
127+
128+
it('does not throw for undefined URL', () => {
129+
expect(() => validateMcpDomain(undefined)).not.toThrow()
130+
})
131+
})
132+
133+
describe('when allowlist is configured', () => {
134+
beforeEach(() => {
135+
mockGetAllowedMcpDomainsFromEnv.mockReturnValue(['allowed.com'])
136+
mockGetBaseUrl.mockReturnValue('https://platform.example.com')
137+
})
138+
139+
it('does not throw for allowed URLs', () => {
140+
expect(() => validateMcpDomain('https://allowed.com/mcp')).not.toThrow()
141+
})
142+
143+
it('throws McpDomainNotAllowedError for disallowed URLs', () => {
144+
expect(() => validateMcpDomain('https://evil.com/mcp')).toThrow(McpDomainNotAllowedError)
145+
})
146+
147+
it('throws for undefined URL (fail-closed)', () => {
148+
expect(() => validateMcpDomain(undefined)).toThrow(McpDomainNotAllowedError)
149+
})
150+
151+
it('throws for malformed URLs', () => {
152+
expect(() => validateMcpDomain('not-a-url')).toThrow(McpDomainNotAllowedError)
153+
})
154+
155+
it('includes the rejected domain in the error message', () => {
156+
expect(() => validateMcpDomain('https://evil.com/mcp')).toThrow(/evil\.com/)
157+
})
158+
159+
it('does not throw for platform hostname', () => {
160+
expect(() => validateMcpDomain('https://platform.example.com/mcp')).not.toThrow()
161+
})
162+
})
163+
})

apps/sim/lib/mcp/domain-check.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ function checkMcpDomain(url: string): string | null {
4545
* The platform's own hostname (from getBaseUrl) is always allowed.
4646
*/
4747
export function isMcpDomainAllowed(url: string | undefined): boolean {
48-
if (!url) return true
48+
if (!url) {
49+
return getAllowedMcpDomainsFromEnv() === null
50+
}
4951
return checkMcpDomain(url) === null
5052
}
5153

@@ -54,7 +56,12 @@ export function isMcpDomainAllowed(url: string | undefined): boolean {
5456
* The platform's own hostname (from getBaseUrl) is always allowed.
5557
*/
5658
export function validateMcpDomain(url: string | undefined): void {
57-
if (!url) return
59+
if (!url) {
60+
if (getAllowedMcpDomainsFromEnv() !== null) {
61+
throw new McpDomainNotAllowedError('(empty)')
62+
}
63+
return
64+
}
5865
const rejected = checkMcpDomain(url)
5966
if (rejected !== null) {
6067
throw new McpDomainNotAllowedError(rejected)

0 commit comments

Comments
 (0)