Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 3.8k
fix(security): authorize MCP subagent IDs, oauth workspace, credential admin demotion#4551
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -58,7 +58,7 @@ export const GET = withRouteHandler(async (_request: NextRequest, context: Route | ||
| .limit(1) | ||
| if (!cred) { | ||
| return NextResponse.json({ members: [] }, { status: 200 }) | ||
| return NextResponse.json({ error: 'Not found' }, { status: 404 }) | ||
| } | ||
| const callerPerm = await getUserEntityPermissions( | ||
| @@ -67,7 +67,7 @@ export const GET = withRouteHandler(async (_request: NextRequest, context: Route | ||
| cred.workspaceId | ||
| ) | ||
| if (callerPerm === null) { | ||
| return NextResponse.json({ error: 'Forbidden' }, { status: 403 }) | ||
| return NextResponse.json({ error: 'Not found' }, { status: 404 }) | ||
| } | ||
| const members = await db | ||
| @@ -120,10 +120,36 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Route | ||
| .limit(1) | ||
| if (existing) { | ||
| await db | ||
| .update(credentialMember) | ||
| .set({ role, status: 'active', updatedAt: now }) | ||
| .where(eq(credentialMember.id, existing.id)) | ||
| const ok = await db.transaction(async (tx) => { | ||
| const [current] = await tx | ||
| .select({ role: credentialMember.role, status: credentialMember.status }) | ||
| .from(credentialMember) | ||
| .where(eq(credentialMember.id, existing.id)) | ||
| .limit(1) | ||
| .for('update') | ||
| if (current?.role === 'admin' && current?.status === 'active' && role !== 'admin') { | ||
| const activeAdmins = await tx | ||
| .select({ id: credentialMember.id }) | ||
| .from(credentialMember) | ||
| .where( | ||
| and( | ||
| eq(credentialMember.credentialId, credentialId), | ||
| eq(credentialMember.role, 'admin'), | ||
| eq(credentialMember.status, 'active') | ||
| ) | ||
| ) | ||
| .for('update') | ||
| if (activeAdmins.length <= 1) return false | ||
| } | ||
| await tx | ||
| .update(credentialMember) | ||
| .set({ role, status: 'active', updatedAt: now }) | ||
| .where(eq(credentialMember.id, existing.id)) | ||
| return true | ||
| }) | ||
waleedlatif1 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| if (!ok) { | ||
| return NextResponse.json({ error: 'Cannot demote the last admin' }, { status: 400 }) | ||
| } | ||
| return NextResponse.json({ success: true }) | ||
| } | ||
| @@ -195,6 +221,7 @@ export const DELETE = withRouteHandler(async (request: NextRequest, context: Rou | ||
| eq(credentialMember.status, 'active') | ||
| ) | ||
| ) | ||
| .for('update') | ||
| if (activeAdmins.length <= 1) { | ||
| return false | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -27,6 +27,7 @@ import { createRequestId } from '@/lib/copilot/request/http' | ||
| import { runHeadlessCopilotLifecycle } from '@/lib/copilot/request/lifecycle/headless' | ||
| import { orchestrateSubagentStream } from '@/lib/copilot/request/subagent' | ||
| import { ensureHandlersRegistered, executeTool } from '@/lib/copilot/tool-executor' | ||
| import { ensureWorkspaceAccess } from '@/lib/copilot/tools/handlers/access' | ||
| import { prepareExecutionContext } from '@/lib/copilot/tools/handlers/context' | ||
| import { DIRECT_TOOL_DEFS, SUBAGENT_TOOL_DEFS } from '@/lib/copilot/tools/mcp/definitions' | ||
| import { env } from '@/lib/core/config/env' | ||
| @@ -445,10 +446,36 @@ async function handleDirectToolCall( | ||
| userId: string | ||
| ): Promise<CallToolResult> { | ||
| try { | ||
| const rawWorkflowId = (args.workflowId as string) || '' | ||
| let resolvedWorkspaceId: string | undefined | ||
| if (rawWorkflowId) { | ||
| const authorization = await authorizeWorkflowByWorkspacePermission({ | ||
| workflowId: rawWorkflowId, | ||
| userId, | ||
| action: 'read', | ||
| }) | ||
| if (!authorization.allowed) { | ||
| return { | ||
| content: [ | ||
| { | ||
| type: 'text', | ||
| text: JSON.stringify( | ||
| { success: false, error: 'Workflow not found or access denied' }, | ||
| null, | ||
| 2 | ||
| ), | ||
| }, | ||
| ], | ||
| isError: true, | ||
| } | ||
| } | ||
| resolvedWorkspaceId = authorization.workflow?.workspaceId || undefined | ||
| } | ||
waleedlatif1 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| const execContext = await prepareExecutionContext( | ||
| userId, | ||
| (args.workflowId as string) || '', | ||
| (args.chatId as string) || undefined | ||
| rawWorkflowId, | ||
| (args.chatId as string) || undefined, | ||
| { workspaceId: resolvedWorkspaceId } | ||
| ) | ||
| const toolCall = { | ||
| @@ -642,21 +669,55 @@ async function handleSubagentToolCall( | ||
| context.plan = args.plan | ||
| } | ||
| // Authorize user-supplied workflowId / workspaceId before forwarding downstream | ||
| const rawWorkflowId = args.workflowId as string | undefined | ||
| const rawWorkspaceId = args.workspaceId as string | undefined | ||
| let resolvedWorkflowId: string | undefined | ||
| let resolvedWorkspaceId: string | undefined | ||
| if (rawWorkflowId) { | ||
| const authorization = await authorizeWorkflowByWorkspacePermission({ | ||
| workflowId: rawWorkflowId, | ||
| userId, | ||
| action: 'read', | ||
| }) | ||
| if (!authorization.allowed) { | ||
| return { | ||
| content: [ | ||
| { | ||
| type: 'text', | ||
| text: JSON.stringify( | ||
| { success: false, error: 'Workflow not found or access denied' }, | ||
| null, | ||
| 2 | ||
| ), | ||
| }, | ||
| ], | ||
| isError: true, | ||
| } | ||
| } | ||
| resolvedWorkflowId = rawWorkflowId | ||
| resolvedWorkspaceId = authorization.workflow?.workspaceId || undefined | ||
| } else if (rawWorkspaceId) { | ||
| await ensureWorkspaceAccess(rawWorkspaceId, userId, 'read') | ||
| resolvedWorkspaceId = rawWorkspaceId | ||
| } | ||
| const result = await orchestrateSubagentStream( | ||
| toolDef.agentId, | ||
| { | ||
| message: requestText, | ||
| workflowId: args.workflowId, | ||
| workspaceId: args.workspaceId, | ||
| workflowId: resolvedWorkflowId, | ||
| workspaceId: resolvedWorkspaceId, | ||
| context, | ||
| model: DEFAULT_COPILOT_MODEL, | ||
| headless: true, | ||
| source: 'mcp', | ||
| }, | ||
| { | ||
| userId, | ||
| workflowId: args.workflowId as string | undefined, | ||
| workspaceId: args.workspaceId as string | undefined, | ||
| workflowId: resolvedWorkflowId, | ||
| workspaceId: resolvedWorkspaceId, | ||
| simRequestId, | ||
| abortSignal, | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.