Skip to content

Commit 2a3a4e9

Browse files
committed
fix(mothership): enforce ownership check on workflow resource attachments (#4468)
* fix(mothership): enforce ownership check on workflow resource attachments * fix(mothership): fix table and knowledgebase BOLA in resource attachment resolution * fix(mothership): apply workspace scope to table in processContextsServer * fix(mothership): verify workspace membership before resolving workspace branch * fix(data-drains): use const for timeoutId in sleepUntilAborted * fix(test): mock db.select and drizzle and for workspace permissions check * fix(mothership): always derive workspace from workflow record in workflow branch
1 parent 3f2f5cc commit 2a3a4e9

4 files changed

Lines changed: 45 additions & 20 deletions

File tree

apps/sim/lib/copilot/chat/post.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,18 @@ vi.mock('@sim/db', () => ({
9393
})),
9494
})),
9595
})),
96+
select: vi.fn(() => ({
97+
from: vi.fn(() => ({
98+
where: vi.fn(() => ({
99+
limit: vi.fn().mockResolvedValue([{ permissionType: 'write' }]),
100+
})),
101+
})),
102+
})),
96103
},
97104
}))
98105

99106
vi.mock('drizzle-orm', () => ({
107+
and: vi.fn(() => ({})),
100108
eq: vi.fn(() => ({})),
101109
sql: (strings: TemplateStringsArray, ...values: unknown[]) => ({ strings, values }),
102110
}))

apps/sim/lib/copilot/chat/post.ts

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { type Context as OtelContext, context as otelContextApi } from '@opentelemetry/api'
22
import { db } from '@sim/db'
3-
import { copilotChats } from '@sim/db/schema'
3+
import { copilotChats, permissions } from '@sim/db/schema'
44
import { createLogger } from '@sim/logger'
55
import { generateId } from '@sim/utils/id'
6-
import { eq, sql } from 'drizzle-orm'
6+
import { and, eq, sql } from 'drizzle-orm'
77
import { type NextRequest, NextResponse } from 'next/server'
88
import { z } from 'zod'
99
import { isZodError, validationErrorResponse } from '@/lib/api/server'
@@ -506,14 +506,12 @@ async function resolveBranch(params: {
506506
}
507507

508508
const resolvedWorkflowId = resolved.workflowId
509-
let resolvedWorkspaceId = requestedWorkspaceId
510-
if (!resolvedWorkspaceId) {
511-
try {
512-
const workflow = await getWorkflowById(resolvedWorkflowId)
513-
resolvedWorkspaceId = workflow?.workspaceId ?? undefined
514-
} catch {
515-
// best effort; downstream calls can still proceed
516-
}
509+
let resolvedWorkspaceId: string | undefined
510+
try {
511+
const workflow = await getWorkflowById(resolvedWorkflowId)
512+
resolvedWorkspaceId = workflow?.workspaceId ?? requestedWorkspaceId
513+
} catch {
514+
resolvedWorkspaceId = requestedWorkspaceId
517515
}
518516

519517
const selectedModel = model || DEFAULT_MODEL
@@ -569,6 +567,22 @@ async function resolveBranch(params: {
569567
return createBadRequestResponse('workspaceId is required when workflowId is not provided')
570568
}
571569

570+
const [permissionRow] = await db
571+
.select({ permissionType: permissions.permissionType })
572+
.from(permissions)
573+
.where(
574+
and(
575+
eq(permissions.userId, authenticatedUserId),
576+
eq(permissions.entityType, 'workspace'),
577+
eq(permissions.entityId, requestedWorkspaceId)
578+
)
579+
)
580+
.limit(1)
581+
582+
if (!permissionRow) {
583+
return createBadRequestResponse('Workspace not found or access denied')
584+
}
585+
572586
return {
573587
kind: 'workspace',
574588
workspaceId: requestedWorkspaceId,

apps/sim/lib/copilot/chat/process-contents.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ export async function processContextsServer(
116116
currentWorkspaceId
117117
)
118118
}
119-
if (ctx.kind === 'table' && ctx.tableId) {
120-
const result = await resolveTableResource(ctx.tableId)
119+
if (ctx.kind === 'table' && ctx.tableId && currentWorkspaceId) {
120+
const result = await resolveTableResource(ctx.tableId, currentWorkspaceId)
121121
if (!result) return null
122122
return { type: 'table', tag: ctx.label ? `@${ctx.label}` : '@', content: result.content }
123123
}
@@ -701,18 +701,18 @@ export async function resolveActiveResourceContext(
701701
resourceType: string,
702702
resourceId: string,
703703
workspaceId: string,
704-
_userId: string,
704+
userId: string,
705705
chatId?: string
706706
): Promise<AgentContext | null> {
707707
try {
708708
switch (resourceType) {
709709
case 'workflow': {
710710
const ctx = await processWorkflowFromDb(
711711
resourceId,
712-
undefined,
712+
userId,
713713
'@active_resource',
714714
'current_workflow',
715-
undefined,
715+
workspaceId,
716716
chatId
717717
)
718718
if (!ctx) return null
@@ -721,15 +721,15 @@ export async function resolveActiveResourceContext(
721721
case 'knowledgebase': {
722722
const ctx = await processKnowledgeFromDb(
723723
resourceId,
724-
undefined,
724+
userId,
725725
'@active_resource',
726726
workspaceId
727727
)
728728
if (!ctx) return null
729729
return { type: 'active_resource', tag: '@active_resource', content: ctx.content }
730730
}
731731
case 'table': {
732-
return await resolveTableResource(resourceId)
732+
return await resolveTableResource(resourceId, workspaceId)
733733
}
734734
case 'file': {
735735
return await resolveFileResource(resourceId, workspaceId)
@@ -745,9 +745,13 @@ export async function resolveActiveResourceContext(
745745
return null
746746
}
747747
}
748-
async function resolveTableResource(tableId: string): Promise<AgentContext | null> {
748+
async function resolveTableResource(
749+
tableId: string,
750+
workspaceId: string
751+
): Promise<AgentContext | null> {
749752
const table = await getTableById(tableId)
750753
if (!table) return null
754+
if (table.workspaceId !== workspaceId) return null
751755
return {
752756
type: 'active_resource',
753757
tag: '@active_resource',

apps/sim/lib/data-drains/destinations/webhook.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,11 @@ function sign(body: Buffer, secret: string, timestamp: number): string {
9999
function sleepUntilAborted(ms: number, signal: AbortSignal): Promise<void> {
100100
if (signal.aborted) return Promise.resolve()
101101
return new Promise((resolve) => {
102-
let timeoutId: ReturnType<typeof setTimeout>
103102
const onAbort = () => {
104103
clearTimeout(timeoutId)
105104
resolve()
106105
}
107-
timeoutId = setTimeout(() => {
106+
const timeoutId = setTimeout(() => {
108107
signal.removeEventListener('abort', onAbort)
109108
resolve()
110109
}, ms)

0 commit comments

Comments
 (0)