Skip to content

Commit e5c61c2

Browse files
committed
fix(copilot): apply the workspace-scope guard across every model-steerable copilot surface
Extends requireCopilotWorkspace to the remaining copilot tools that resolved their target workspace from model-supplied arguments: get_credentials (a workflowId could steer the credential listing to any workspace the user can access) and publish_custom_block (a workflowId could deploy/undeploy custom blocks from another workspace's workflow). The handlers already protected downstream by the application adapter (create workflow, generate API key, list/create workspace MCP servers) now use the same guard so a mismatch is rejected uniformly at the surface, and the getDefaultWorkspaceId fallback is deleted entirely — no copilot path picks a workspace for the model anymore.
1 parent bf3c6e1 commit e5c61c2

9 files changed

Lines changed: 73 additions & 39 deletions

File tree

apps/sim/lib/copilot/tools/handlers/access.ts

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import { authorizeWorkflowByWorkspacePermission } from '@sim/platform-authz/work
22
import { OrchestrationError } from '@/lib/core/orchestration/types'
33
import type { getWorkflowById } from '@/lib/workflows/utils'
44
import { checkWorkspaceAccess, type WorkspaceAccess } from '@/lib/workspaces/permissions/utils'
5-
import { listAccessibleWorkspaceRowsForUser } from '@/lib/workspaces/utils'
65

76
type WorkflowRecord = NonNullable<Awaited<ReturnType<typeof getWorkflowById>>>
87

@@ -40,19 +39,6 @@ export async function ensureWorkflowAccess(
4039
return { workflow: result.workflow, workspaceId: result.workflow.workspaceId }
4140
}
4241

43-
export async function getDefaultWorkspaceId(userId: string): Promise<string> {
44-
const accessibleRows = await listAccessibleWorkspaceRowsForUser(userId)
45-
const mostRecent = accessibleRows
46-
.map((row) => row.workspace)
47-
.sort((a, b) => b.createdAt.getTime() - a.createdAt.getTime())[0]
48-
49-
if (!mostRecent) {
50-
throw new Error('No workspace found for user')
51-
}
52-
53-
return mostRecent.id
54-
}
55-
5642
export async function ensureWorkspaceAccess(
5743
workspaceId: string,
5844
userId: string,

apps/sim/lib/copilot/tools/handlers/deployment/custom-block.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,21 @@ describe('executeDeployCustomBlock', () => {
157157
})
158158
})
159159

160+
it('rejects a workflowId whose workspace differs from the execution workspace', async () => {
161+
ensureWorkflowAccessMock.mockResolvedValue({
162+
workflow: { id: 'wf-other', workspaceId: 'ws-other', name: 'Other', isDeployed: true },
163+
})
164+
165+
const result = await executeDeployCustomBlock(
166+
{ workflowId: 'wf-other', name: 'Enrich Lead' },
167+
context
168+
)
169+
170+
expect(result.success).toBe(false)
171+
expect(result.error).toContain('does not match the Copilot execution workspace')
172+
expect(publishCustomBlockMock).not.toHaveBeenCalled()
173+
})
174+
160175
it('returns a clean admin-permission error when workflow access is denied', async () => {
161176
ensureWorkflowAccessMock.mockRejectedValue(new Error('Unauthorized workflow access'))
162177

apps/sim/lib/copilot/tools/handlers/deployment/custom-block.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ import {
99
resolveCopilotWorkspaceFileReference,
1010
} from '@/lib/copilot/application/execute-file-use-case'
1111
import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types'
12+
import { requireCopilotWorkspace } from '@/lib/copilot/tools/server/workspace-scope'
1213
import { canonicalizeVfsPath } from '@/lib/copilot/vfs/path-utils'
1314
import { isFeatureEnabled } from '@/lib/core/config/feature-flags'
15+
import { OrchestrationError } from '@/lib/core/orchestration/types'
1416
import { buildStorageKeySegment } from '@/lib/uploads/core/storage-key'
1517
import { uploadFile } from '@/lib/uploads/core/storage-service'
1618
import { isImageFileType } from '@/lib/uploads/utils/file-utils'
@@ -147,10 +149,11 @@ export async function executeDeployCustomBlock(
147149
error: "Managing a custom block requires admin permission on the workflow's workspace",
148150
}
149151
}
150-
const workspaceId = workflowRecord.workspaceId
151-
if (!workspaceId) {
152+
const rawWorkspaceId = workflowRecord.workspaceId
153+
if (!rawWorkspaceId) {
152154
return { success: false, error: 'Workflow must belong to a workspace' }
153155
}
156+
const workspaceId = requireCopilotWorkspace(context, rawWorkspaceId)
154157

155158
const ws = await getWorkspaceWithOwner(workspaceId)
156159
const organizationId = ws?.organizationId
@@ -303,7 +306,7 @@ export async function executeDeployCustomBlock(
303306
})
304307
return { success: true, output: { ...customBlockOutput(block, 'deploy'), updated: false } }
305308
} catch (error) {
306-
if (error instanceof CustomBlockValidationError) {
309+
if (error instanceof CustomBlockValidationError || error instanceof OrchestrationError) {
307310
return { success: false, error: error.message }
308311
}
309312
logger.error('Custom block deployment failed', { error })

apps/sim/lib/copilot/tools/handlers/deployment/manage.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
messageForCopilotWorkflowError,
66
} from '@/lib/copilot/application/execute-workflow-use-case'
77
import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types'
8+
import { requireCopilotWorkspace } from '@/lib/copilot/tools/server/workspace-scope'
89
import { generateRequestId } from '@/lib/core/utils/request'
910
import {
1011
createWorkflowMcpDeploymentServer,
@@ -138,10 +139,7 @@ export async function executeListWorkspaceMcpServers(
138139
context: ExecutionContext
139140
): Promise<ToolCallResult> {
140141
try {
141-
const workspaceId = params.workspaceId || context.workspaceId
142-
if (!workspaceId) {
143-
return { success: false, error: 'workspaceId is required' }
144-
}
142+
const workspaceId = requireCopilotWorkspace(context, params.workspaceId || undefined)
145143
const result = await executeCopilotMcpServerUseCase(context, listWorkflowMcpDeployments, {
146144
workspaceId,
147145
})
@@ -163,10 +161,7 @@ export async function executeCreateWorkspaceMcpServer(
163161
context: ExecutionContext
164162
): Promise<ToolCallResult> {
165163
try {
166-
const workspaceId = params.workspaceId || context.workspaceId
167-
if (!workspaceId) {
168-
return { success: false, error: 'workspaceId is required' }
169-
}
164+
const workspaceId = requireCopilotWorkspace(context, params.workspaceId || undefined)
170165

171166
const name = params.name?.trim()
172167
if (!name) {

apps/sim/lib/copilot/tools/handlers/vfs-mutate.test.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import { fileOperations } from '@/lib/workspace-files/application/operations'
1111
const mocks = vi.hoisted(() => ({
1212
ensureWorkspaceAccess: vi.fn(),
1313
ensureWorkflowAccess: vi.fn(),
14-
getDefaultWorkspaceId: vi.fn(),
1514
getWorkspaceFileByName: vi.fn(),
1615
resolveWorkspaceFileReference: vi.fn(),
1716
findWorkspaceFileFolderIdByPath: vi.fn(),
@@ -57,7 +56,6 @@ vi.mock('@sim/db', () => ({ ...dbChainMock, ...schemaMock }))
5756
vi.mock('@/lib/copilot/tools/handlers/access', () => ({
5857
ensureWorkspaceAccess: mocks.ensureWorkspaceAccess,
5958
ensureWorkflowAccess: mocks.ensureWorkflowAccess,
60-
getDefaultWorkspaceId: mocks.getDefaultWorkspaceId,
6159
}))
6260

6361
vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({

apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import type { ExecutionContext } from '@/lib/copilot/request/types'
77
const { mocks } = vi.hoisted(() => ({
88
mocks: {
99
apiKey: vi.fn(),
10-
defaultWorkspace: vi.fn(),
1110
executeWorkflowUseCase: vi.fn(),
1211
hasExecutionResult: vi.fn(),
1312
},
@@ -23,10 +22,6 @@ vi.mock('@/lib/copilot/application/execute-api-key-use-case', () => ({
2322
executeCopilotApiKeyUseCase: mocks.apiKey,
2423
}))
2524

26-
vi.mock('@/lib/copilot/tools/handlers/access', () => ({
27-
getDefaultWorkspaceId: mocks.defaultWorkspace,
28-
}))
29-
3025
vi.mock('@/lib/workflows/sanitization/json-sanitizer', () => ({
3126
sanitizeForCopilot: vi.fn((state) => state),
3227
}))
@@ -61,7 +56,6 @@ const context = {
6156
describe('workflow mutation Copilot adapters', () => {
6257
beforeEach(() => {
6358
vi.clearAllMocks()
64-
mocks.defaultWorkspace.mockResolvedValue('workspace-1')
6559
mocks.hasExecutionResult.mockReturnValue(false)
6660
})
6761

@@ -93,6 +87,16 @@ describe('workflow mutation Copilot adapters', () => {
9387
)
9488
})
9589

90+
it('rejects a create-workflow workspaceId that names a different workspace', async () => {
91+
const result = await executeCreateWorkflow(
92+
{ name: 'New Workflow', workspaceId: 'workspace-other' },
93+
context
94+
)
95+
96+
expect(result.success).toBe(false)
97+
expect(mocks.executeWorkflowUseCase).not.toHaveBeenCalled()
98+
})
99+
96100
it('calls the compound variable command once', async () => {
97101
mocks.executeWorkflowUseCase.mockResolvedValue({ updated: 2 })
98102
const operations = [

apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
messageForCopilotWorkflowError,
88
} from '@/lib/copilot/application/execute-workflow-use-case'
99
import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types'
10+
import { requireCopilotWorkspace } from '@/lib/copilot/tools/server/workspace-scope'
1011
import { decodeVfsPathSegments, encodeVfsPathSegments } from '@/lib/copilot/vfs/path-utils'
1112
import { PlatformEvents } from '@/lib/core/telemetry'
1213
import { createWorkflow } from '@/lib/workflows/application/create-workflow'
@@ -25,7 +26,6 @@ import {
2526
import { sanitizeForCopilot } from '@/lib/workflows/sanitization/json-sanitizer'
2627
import { hasExecutionResult } from '@/executor/utils/errors'
2728
import type { WorkflowState } from '@/stores/workflows/workflow/types'
28-
import { getDefaultWorkspaceId } from '../access'
2929

3030
function stripBinaryFields(value: unknown): unknown {
3131
if (value === null || value === undefined) return value
@@ -152,8 +152,7 @@ export async function executeCreateWorkflow(
152152
if (name.length > 200) {
153153
return { success: false, error: 'Workflow name must be 200 characters or less' }
154154
}
155-
const workspaceId =
156-
params?.workspaceId || context.workspaceId || (await getDefaultWorkspaceId(context.userId))
155+
const workspaceId = requireCopilotWorkspace(context, params?.workspaceId || undefined)
157156

158157
const folderPath = typeof params?.folderPath === 'string' ? params.folderPath.trim() : ''
159158
const folderId =
@@ -363,8 +362,7 @@ export async function executeGenerateApiKey(
363362
return { success: false, error: 'API key name must be 200 characters or less' }
364363
}
365364

366-
const workspaceId =
367-
params.workspaceId || context.workspaceId || (await getDefaultWorkspaceId(context.userId))
365+
const workspaceId = requireCopilotWorkspace(context, params.workspaceId || undefined)
368366
assertWorkflowMutationNotAborted(context)
369367

370368
const result = await executeCopilotApiKeyUseCase(context, createCopilotWorkspaceApiKey, {

apps/sim/lib/copilot/tools/server/user/get-credentials.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const {
2525
getUserPermissionConfigMock,
2626
getAccessibleOAuthCredentialsMock,
2727
checkWorkspaceAccessMock,
28+
verifyWorkflowAccessMock,
2829
} = vi.hoisted(() => ({
2930
getAllOAuthServicesMock: vi.fn(),
3031
decodeJwtMock: vi.fn(),
@@ -33,6 +34,7 @@ const {
3334
getUserPermissionConfigMock: vi.fn(),
3435
getAccessibleOAuthCredentialsMock: vi.fn(),
3536
checkWorkspaceAccessMock: vi.fn(),
37+
verifyWorkflowAccessMock: vi.fn(),
3638
}))
3739

3840
const getPersonalAndWorkspaceEnvMock = environmentUtilsMockFns.mockGetPersonalAndWorkspaceEnv
@@ -93,6 +95,11 @@ vi.mock('jose', () => ({
9395
decodeJwt: decodeJwtMock,
9496
}))
9597

98+
vi.mock('@/lib/copilot/auth/permissions', () => ({
99+
verifyWorkflowAccess: verifyWorkflowAccessMock,
100+
createPermissionError: (action: string) => `Permission denied: ${action}`,
101+
}))
102+
96103
import { getCredentialsServerTool } from './get-credentials'
97104

98105
/**
@@ -385,6 +392,30 @@ describe('getCredentialsServerTool', () => {
385392
expect(result.oauth.connected.credentials).toEqual([])
386393
})
387394

395+
it('resolves the workspace from a workflow in the execution workspace', async () => {
396+
verifyWorkflowAccessMock.mockResolvedValue({ hasAccess: true, workspaceId: 'workspace-1' })
397+
getUserPermissionConfigMock.mockResolvedValue({ allowedIntegrations: null })
398+
399+
await getCredentialsServerTool.execute(
400+
{ workflowId: 'wf-1' },
401+
{ userId: 'user-1', workspaceId: 'workspace-1' }
402+
)
403+
404+
expect(verifyWorkflowAccessMock).toHaveBeenCalledWith('user-1', 'wf-1')
405+
expect(getUserPermissionConfigMock).toHaveBeenCalledWith('user-1', 'workspace-1')
406+
})
407+
408+
it('rejects a workflowId whose workspace differs from the execution workspace', async () => {
409+
verifyWorkflowAccessMock.mockResolvedValue({ hasAccess: true, workspaceId: 'workspace-other' })
410+
411+
await expect(
412+
getCredentialsServerTool.execute(
413+
{ workflowId: 'wf-other' },
414+
{ userId: 'user-1', workspaceId: 'workspace-1' }
415+
)
416+
).rejects.toThrow('Workspace ID does not match the Copilot execution workspace')
417+
})
418+
388419
it('rejects unauthenticated callers without touching the database', async () => {
389420
await expect(getCredentialsServerTool.execute({}, undefined)).rejects.toThrow(
390421
'Authentication required'

apps/sim/lib/copilot/tools/server/user/get-credentials.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { eq } from 'drizzle-orm'
66
import { decodeJwt } from 'jose'
77
import { createPermissionError, verifyWorkflowAccess } from '@/lib/copilot/auth/permissions'
88
import type { BaseServerTool } from '@/lib/copilot/tools/server/base-tool'
9+
import { requireCopilotWorkspace } from '@/lib/copilot/tools/server/workspace-scope'
910
import { getAllowedIntegrationsFromEnv } from '@/lib/core/config/env-flags'
1011
import { OrchestrationError } from '@/lib/core/orchestration/types'
1112
import { getAccessibleOAuthCredentials } from '@/lib/credentials/environment'
@@ -54,7 +55,10 @@ export const getCredentialsServerTool: BaseServerTool<GetCredentialsParams, any>
5455
throw new OrchestrationError('forbidden', errorMessage)
5556
}
5657

57-
workspaceId = wId
58+
// A model-supplied workflowId may only re-assert the chat's workspace —
59+
// it can never steer the credential listing to another workspace. A
60+
// legacy workflow with no workspace contributes no workspace scope.
61+
workspaceId = wId ? requireCopilotWorkspace(context, wId) : undefined
5862
}
5963

6064
const userId = authenticatedUserId

0 commit comments

Comments
 (0)