Skip to content

Commit 1fce014

Browse files
fix(mcp): treat setting a URL on a URL-less server as a destination change
1 parent b2d3e67 commit 1fce014

3 files changed

Lines changed: 29 additions & 7 deletions

File tree

‎apps/sim/lib/mcp/application/use-cases.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,8 +392,8 @@ async function updateMcpServer(args: {
392392
}
393393
const changesDestination =
394394
args.input.url !== undefined &&
395-
!!args.context.server.url &&
396-
!isSameMcpServerDestination(args.context.server.url, args.input.url)
395+
(!args.context.server.url ||
396+
!isSameMcpServerDestination(args.context.server.url, args.input.url))
397397
if (changesDestination) {
398398
await authorizeWorkspaceOperation(
399399
args.principal,

‎apps/sim/lib/mcp/orchestration/server-lifecycle.test.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,29 @@ describe('MCP server lifecycle orchestration', () => {
261261
expect(mockRevokeOauthTokens).not.toHaveBeenCalled()
262262
})
263263

264+
it('refuses a non-admin setting a URL on a server that has none', async () => {
265+
dbChainMockFns.limit.mockResolvedValueOnce([
266+
{
267+
url: null,
268+
authType: 'headers',
269+
headers: {},
270+
oauthClientId: null,
271+
oauthClientSecret: null,
272+
},
273+
])
274+
275+
const result = await performUpdateMcpServer({
276+
workspaceId: 'workspace-1',
277+
userId: 'user-1',
278+
serverId: 'server-1',
279+
allowDestinationChange: false,
280+
url: 'https://other-host.example.com/mcp',
281+
})
282+
283+
expect(result).toMatchObject({ success: false, errorCode: 'forbidden' })
284+
expect(dbChainMockFns.set).not.toHaveBeenCalled()
285+
})
286+
264287
it('refuses a non-admin save when the URL changed after it was checked', async () => {
265288
dbChainMockFns.limit.mockResolvedValueOnce([
266289
{

‎apps/sim/lib/mcp/orchestration/server-lifecycle.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -435,19 +435,18 @@ export async function updateMcpServer(
435435

436436
if (!currentServer) return { success: false, error: 'Server not found', errorCode: 'not_found' }
437437

438-
const checkedUrl =
439-
!params.allowDestinationChange && params.url !== undefined ? currentServer.url : null
438+
const guardedUrl = params.allowDestinationChange ? undefined : params.url
440439
if (
441-
checkedUrl &&
442-
params.url !== undefined &&
443-
!isSameMcpServerDestination(checkedUrl, params.url)
440+
guardedUrl !== undefined &&
441+
(!currentServer.url || !isSameMcpServerDestination(currentServer.url, guardedUrl))
444442
) {
445443
return {
446444
success: false,
447445
error: 'Only workspace admins can point an MCP server at a different URL',
448446
errorCode: 'forbidden',
449447
}
450448
}
449+
const checkedUrl = guardedUrl !== undefined ? currentServer.url : null
451450

452451
if (
453452
params.oauthClientId &&

0 commit comments

Comments
 (0)