Skip to content

Commit 3297baf

Browse files
committed
fix(security): authorize MCP subagent IDs, oauth workspace, credential admin demotion
- handleSubagentToolCall and handleDirectToolCall now authorize user-supplied workflowId/workspaceId via authorizeWorkflowByWorkspacePermission / ensureWorkspaceAccess before forwarding downstream; resolvedWorkspaceId is derived from the authorized workflow record instead of trusted from the body - executeOAuthGetAuthLink verifies caller membership (write level) on the target workspaceId before generating the OAuth link or writing pendingCredentialDraft, closing the cross-workspace credential injection path - POST /api/credentials/[id]/members wraps role updates in a transaction that counts active admins and rejects demotion of the last admin (mirrors the existing DELETE guard in the same file) - GET /api/credentials/[id]/members returns uniform 404 for both missing and inaccessible credentials to remove the existence oracle
1 parent ae680af commit 3297baf

4 files changed

Lines changed: 185 additions & 98 deletions

File tree

apps/sim/app/api/credentials/[id]/members/route.ts

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export const GET = withRouteHandler(async (_request: NextRequest, context: Route
5858
.limit(1)
5959

6060
if (!cred) {
61-
return NextResponse.json({ members: [] }, { status: 200 })
61+
return NextResponse.json({ error: 'Not found' }, { status: 404 })
6262
}
6363

6464
const callerPerm = await getUserEntityPermissions(
@@ -67,7 +67,7 @@ export const GET = withRouteHandler(async (_request: NextRequest, context: Route
6767
cred.workspaceId
6868
)
6969
if (callerPerm === null) {
70-
return NextResponse.json({ error: 'Forbidden' }, { status: 403 })
70+
return NextResponse.json({ error: 'Not found' }, { status: 404 })
7171
}
7272

7373
const members = await db
@@ -120,10 +120,34 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Route
120120
.limit(1)
121121

122122
if (existing) {
123-
await db
124-
.update(credentialMember)
125-
.set({ role, status: 'active', updatedAt: now })
126-
.where(eq(credentialMember.id, existing.id))
123+
const ok = await db.transaction(async (tx) => {
124+
const [current] = await tx
125+
.select({ role: credentialMember.role })
126+
.from(credentialMember)
127+
.where(eq(credentialMember.id, existing.id))
128+
.limit(1)
129+
if (current?.role === 'admin' && role !== 'admin') {
130+
const activeAdmins = await tx
131+
.select({ id: credentialMember.id })
132+
.from(credentialMember)
133+
.where(
134+
and(
135+
eq(credentialMember.credentialId, credentialId),
136+
eq(credentialMember.role, 'admin'),
137+
eq(credentialMember.status, 'active')
138+
)
139+
)
140+
if (activeAdmins.length <= 1) return false
141+
}
142+
await tx
143+
.update(credentialMember)
144+
.set({ role, status: 'active', updatedAt: now })
145+
.where(eq(credentialMember.id, existing.id))
146+
return true
147+
})
148+
if (!ok) {
149+
return NextResponse.json({ error: 'Cannot demote the last admin' }, { status: 400 })
150+
}
127151
return NextResponse.json({ success: true })
128152
}
129153

apps/sim/app/api/mcp/copilot/route.ts

Lines changed: 63 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { createRequestId } from '@/lib/copilot/request/http'
2727
import { runHeadlessCopilotLifecycle } from '@/lib/copilot/request/lifecycle/headless'
2828
import { orchestrateSubagentStream } from '@/lib/copilot/request/subagent'
2929
import { ensureHandlersRegistered, executeTool } from '@/lib/copilot/tool-executor'
30+
import { ensureWorkspaceAccess } from '@/lib/copilot/tools/handlers/access'
3031
import { prepareExecutionContext } from '@/lib/copilot/tools/handlers/context'
3132
import { DIRECT_TOOL_DEFS, SUBAGENT_TOOL_DEFS } from '@/lib/copilot/tools/mcp/definitions'
3233
import { env } from '@/lib/core/config/env'
@@ -445,9 +446,32 @@ async function handleDirectToolCall(
445446
userId: string
446447
): Promise<CallToolResult> {
447448
try {
449+
const rawWorkflowId = (args.workflowId as string) || ''
450+
if (rawWorkflowId) {
451+
const authorization = await authorizeWorkflowByWorkspacePermission({
452+
workflowId: rawWorkflowId,
453+
userId,
454+
action: 'read',
455+
})
456+
if (!authorization.allowed) {
457+
return {
458+
content: [
459+
{
460+
type: 'text',
461+
text: JSON.stringify(
462+
{ success: false, error: 'Workflow not found or access denied' },
463+
null,
464+
2
465+
),
466+
},
467+
],
468+
isError: true,
469+
}
470+
}
471+
}
448472
const execContext = await prepareExecutionContext(
449473
userId,
450-
(args.workflowId as string) || '',
474+
rawWorkflowId,
451475
(args.chatId as string) || undefined
452476
)
453477

@@ -642,21 +666,55 @@ async function handleSubagentToolCall(
642666
context.plan = args.plan
643667
}
644668

669+
// Authorize user-supplied workflowId / workspaceId before forwarding downstream
670+
const rawWorkflowId = args.workflowId as string | undefined
671+
const rawWorkspaceId = args.workspaceId as string | undefined
672+
let resolvedWorkflowId: string | undefined
673+
let resolvedWorkspaceId: string | undefined
674+
675+
if (rawWorkflowId) {
676+
const authorization = await authorizeWorkflowByWorkspacePermission({
677+
workflowId: rawWorkflowId,
678+
userId,
679+
action: 'read',
680+
})
681+
if (!authorization.allowed) {
682+
return {
683+
content: [
684+
{
685+
type: 'text',
686+
text: JSON.stringify(
687+
{ success: false, error: 'Workflow not found or access denied' },
688+
null,
689+
2
690+
),
691+
},
692+
],
693+
isError: true,
694+
}
695+
}
696+
resolvedWorkflowId = rawWorkflowId
697+
resolvedWorkspaceId = authorization.workflow?.workspaceId || undefined
698+
} else if (rawWorkspaceId) {
699+
await ensureWorkspaceAccess(rawWorkspaceId, userId, 'read')
700+
resolvedWorkspaceId = rawWorkspaceId
701+
}
702+
645703
const result = await orchestrateSubagentStream(
646704
toolDef.agentId,
647705
{
648706
message: requestText,
649-
workflowId: args.workflowId,
650-
workspaceId: args.workspaceId,
707+
workflowId: resolvedWorkflowId,
708+
workspaceId: resolvedWorkspaceId,
651709
context,
652710
model: DEFAULT_COPILOT_MODEL,
653711
headless: true,
654712
source: 'mcp',
655713
},
656714
{
657715
userId,
658-
workflowId: args.workflowId as string | undefined,
659-
workspaceId: args.workspaceId as string | undefined,
716+
workflowId: resolvedWorkflowId,
717+
workspaceId: resolvedWorkspaceId,
660718
simRequestId,
661719
abortSignal,
662720
}

0 commit comments

Comments
 (0)