Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions apps/sim/lib/copilot/tools/handlers/access.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { authorizeWorkflowByWorkspacePermission } from '@sim/platform-authz/work
import { OrchestrationError } from '@/lib/core/orchestration/types'
import type { getWorkflowById } from '@/lib/workflows/utils'
import { checkWorkspaceAccess, type WorkspaceAccess } from '@/lib/workspaces/permissions/utils'
import { listAccessibleWorkspaceRowsForUser } from '@/lib/workspaces/utils'

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

Expand Down Expand Up @@ -40,19 +39,6 @@ export async function ensureWorkflowAccess(
return { workflow: result.workflow, workspaceId: result.workflow.workspaceId }
}

export async function getDefaultWorkspaceId(userId: string): Promise<string> {
const accessibleRows = await listAccessibleWorkspaceRowsForUser(userId)
const mostRecent = accessibleRows
.map((row) => row.workspace)
.sort((a, b) => b.createdAt.getTime() - a.createdAt.getTime())[0]

if (!mostRecent) {
throw new Error('No workspace found for user')
}

return mostRecent.id
}

export async function ensureWorkspaceAccess(
workspaceId: string,
userId: string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,21 @@ describe('executeDeployCustomBlock', () => {
})
})

it('rejects a workflowId whose workspace differs from the execution workspace', async () => {
ensureWorkflowAccessMock.mockResolvedValue({
workflow: { id: 'wf-other', workspaceId: 'ws-other', name: 'Other', isDeployed: true },
})

const result = await executeDeployCustomBlock(
{ workflowId: 'wf-other', name: 'Enrich Lead' },
context
)

expect(result.success).toBe(false)
expect(result.error).toContain('does not match the Copilot execution workspace')
expect(publishCustomBlockMock).not.toHaveBeenCalled()
})

it('returns a clean admin-permission error when workflow access is denied', async () => {
ensureWorkflowAccessMock.mockRejectedValue(new Error('Unauthorized workflow access'))

Expand Down
10 changes: 8 additions & 2 deletions apps/sim/lib/copilot/tools/handlers/deployment/custom-block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
resolveCopilotWorkspaceFileReference,
} from '@/lib/copilot/application/execute-file-use-case'
import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types'
import { requireCopilotWorkspace } from '@/lib/copilot/tools/server/workspace-scope'
import { canonicalizeVfsPath } from '@/lib/copilot/vfs/path-utils'
import { isFeatureEnabled } from '@/lib/core/config/feature-flags'
import { buildStorageKeySegment } from '@/lib/uploads/core/storage-key'
Expand Down Expand Up @@ -147,10 +148,15 @@ export async function executeDeployCustomBlock(
error: "Managing a custom block requires admin permission on the workflow's workspace",
}
}
const workspaceId = workflowRecord.workspaceId
if (!workspaceId) {
if (!workflowRecord.workspaceId) {
return { success: false, error: 'Workflow must belong to a workspace' }
}
let workspaceId: string
try {
workspaceId = requireCopilotWorkspace(context, workflowRecord.workspaceId)
} catch (error) {
return { success: false, error: toError(error).message }
}

const ws = await getWorkspaceWithOwner(workspaceId)
const organizationId = ws?.organizationId
Expand Down
11 changes: 3 additions & 8 deletions apps/sim/lib/copilot/tools/handlers/deployment/manage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
messageForCopilotWorkflowError,
} from '@/lib/copilot/application/execute-workflow-use-case'
import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types'
import { requireCopilotWorkspace } from '@/lib/copilot/tools/server/workspace-scope'
import { generateRequestId } from '@/lib/core/utils/request'
import {
createWorkflowMcpDeploymentServer,
Expand Down Expand Up @@ -138,10 +139,7 @@ export async function executeListWorkspaceMcpServers(
context: ExecutionContext
): Promise<ToolCallResult> {
try {
const workspaceId = params.workspaceId || context.workspaceId
if (!workspaceId) {
return { success: false, error: 'workspaceId is required' }
}
const workspaceId = requireCopilotWorkspace(context, params.workspaceId)
const result = await executeCopilotMcpServerUseCase(context, listWorkflowMcpDeployments, {
workspaceId,
})
Expand All @@ -163,10 +161,7 @@ export async function executeCreateWorkspaceMcpServer(
context: ExecutionContext
): Promise<ToolCallResult> {
try {
const workspaceId = params.workspaceId || context.workspaceId
if (!workspaceId) {
return { success: false, error: 'workspaceId is required' }
}
const workspaceId = requireCopilotWorkspace(context, params.workspaceId)

const name = params.name?.trim()
if (!name) {
Expand Down
5 changes: 3 additions & 2 deletions apps/sim/lib/copilot/tools/handlers/vfs-mutate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { fileOperations } from '@/lib/workspace-files/application/operations'
const mocks = vi.hoisted(() => ({
ensureWorkspaceAccess: vi.fn(),
ensureWorkflowAccess: vi.fn(),
getDefaultWorkspaceId: vi.fn(),
getWorkspaceFileByName: vi.fn(),
resolveWorkspaceFileReference: vi.fn(),
findWorkspaceFileFolderIdByPath: vi.fn(),
Expand Down Expand Up @@ -57,7 +56,6 @@ vi.mock('@sim/db', () => ({ ...dbChainMock, ...schemaMock }))
vi.mock('@/lib/copilot/tools/handlers/access', () => ({
ensureWorkspaceAccess: mocks.ensureWorkspaceAccess,
ensureWorkflowAccess: mocks.ensureWorkflowAccess,
getDefaultWorkspaceId: mocks.getDefaultWorkspaceId,
}))

vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({
Expand Down Expand Up @@ -86,6 +84,9 @@ vi.mock('@/lib/copilot/tools/server/files/file-folder-application', () => ({
...(fileId ? { resourceScope: { fileId } } : {}),
})),
ensureCopilotFileFolderPath: mocks.ensureCopilotFileFolderPath,
}))

vi.mock('@/lib/copilot/tools/server/workspace-scope', () => ({
requireCopilotWorkspace: vi.fn((context) => context.workspaceId),
}))

Expand Down
2 changes: 1 addition & 1 deletion apps/sim/lib/copilot/tools/handlers/vfs-mutate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
} from '@/lib/copilot/application/execute-workflow-use-case'
import { messageForCopilotTableError } from '@/lib/copilot/auth/table-delegation'
import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types'
import { requireCopilotWorkspace } from '@/lib/copilot/tools/server/files/file-folder-application'
import { requireCopilotWorkspace } from '@/lib/copilot/tools/server/workspace-scope'
import { decodeVfsPathSegments, encodeVfsPathSegments } from '@/lib/copilot/vfs/path-utils'
import { asOrchestrationError } from '@/lib/core/orchestration/types'
import { PlatformEvents } from '@/lib/core/telemetry'
Expand Down
16 changes: 10 additions & 6 deletions apps/sim/lib/copilot/tools/handlers/workflow/mutations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import type { ExecutionContext } from '@/lib/copilot/request/types'
const { mocks } = vi.hoisted(() => ({
mocks: {
apiKey: vi.fn(),
defaultWorkspace: vi.fn(),
executeWorkflowUseCase: vi.fn(),
hasExecutionResult: vi.fn(),
},
Expand All @@ -23,10 +22,6 @@ vi.mock('@/lib/copilot/application/execute-api-key-use-case', () => ({
executeCopilotApiKeyUseCase: mocks.apiKey,
}))

vi.mock('@/lib/copilot/tools/handlers/access', () => ({
getDefaultWorkspaceId: mocks.defaultWorkspace,
}))

vi.mock('@/lib/workflows/sanitization/json-sanitizer', () => ({
sanitizeForCopilot: vi.fn((state) => state),
}))
Expand Down Expand Up @@ -61,7 +56,6 @@ const context = {
describe('workflow mutation Copilot adapters', () => {
beforeEach(() => {
vi.clearAllMocks()
mocks.defaultWorkspace.mockResolvedValue('workspace-1')
mocks.hasExecutionResult.mockReturnValue(false)
})

Expand Down Expand Up @@ -93,6 +87,16 @@ describe('workflow mutation Copilot adapters', () => {
)
})

it('rejects a create-workflow workspaceId that names a different workspace', async () => {
const result = await executeCreateWorkflow(
{ name: 'New Workflow', workspaceId: 'workspace-other' },
context
)

expect(result.success).toBe(false)
expect(mocks.executeWorkflowUseCase).not.toHaveBeenCalled()
})

it('calls the compound variable command once', async () => {
mocks.executeWorkflowUseCase.mockResolvedValue({ updated: 2 })
const operations = [
Expand Down
8 changes: 3 additions & 5 deletions apps/sim/lib/copilot/tools/handlers/workflow/mutations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
messageForCopilotWorkflowError,
} from '@/lib/copilot/application/execute-workflow-use-case'
import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types'
import { requireCopilotWorkspace } from '@/lib/copilot/tools/server/workspace-scope'
import { decodeVfsPathSegments, encodeVfsPathSegments } from '@/lib/copilot/vfs/path-utils'
import { PlatformEvents } from '@/lib/core/telemetry'
import { createWorkflow } from '@/lib/workflows/application/create-workflow'
Expand All @@ -25,7 +26,6 @@ import {
import { sanitizeForCopilot } from '@/lib/workflows/sanitization/json-sanitizer'
import { hasExecutionResult } from '@/executor/utils/errors'
import type { WorkflowState } from '@/stores/workflows/workflow/types'
import { getDefaultWorkspaceId } from '../access'

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

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

const workspaceId =
params.workspaceId || context.workspaceId || (await getDefaultWorkspaceId(context.userId))
const workspaceId = requireCopilotWorkspace(context, params.workspaceId)
assertWorkflowMutationNotAborted(context)

const result = await executeCopilotApiKeyUseCase(context, createCopilotWorkspaceApiKey, {
Expand Down
19 changes: 0 additions & 19 deletions apps/sim/lib/copilot/tools/server/files/file-folder-application.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,8 @@
import { executeCopilotFileUseCase } from '@/lib/copilot/application/execute-file-use-case'
import type { CopilotFileDelegationContext } from '@/lib/copilot/auth/file-delegation'
import { OrchestrationError } from '@/lib/core/orchestration/types'
import { findWorkspaceFileFolderIdByPath } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager'
import { createWorkspaceFileFolderOperation } from '@/lib/workspace-files/application/workspace-file-folders'

/**
* Returns the execution workspace only. Model-provided workspace ids may assert the same value,
* but can never select a different workspace or trigger a default-workspace fallback.
*/
export function requireCopilotWorkspace(
context: CopilotFileDelegationContext,
assertedWorkspaceId?: string
): string {
if (!context.workspaceId) throw new Error('Copilot execution workspace is required')
if (assertedWorkspaceId && assertedWorkspaceId !== context.workspaceId) {
throw new OrchestrationError(
'validation',
'Workspace ID does not match the Copilot execution workspace'
)
}
return context.workspaceId
}

/** Creates missing parent folders through the shared folder application operation. */
export async function ensureCopilotFileFolderPath(
context: CopilotFileDelegationContext,
Expand Down
6 changes: 2 additions & 4 deletions apps/sim/lib/copilot/tools/server/files/file-folders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ import {
type BaseServerTool,
type ServerToolContext,
} from '@/lib/copilot/tools/server/base-tool'
import {
ensureCopilotFileFolderPath,
requireCopilotWorkspace,
} from '@/lib/copilot/tools/server/files/file-folder-application'
import { ensureCopilotFileFolderPath } from '@/lib/copilot/tools/server/files/file-folder-application'
import { requireCopilotWorkspace } from '@/lib/copilot/tools/server/workspace-scope'
import { decodeVfsPathSegments } from '@/lib/copilot/vfs/path-utils'
import { asOrchestrationError, OrchestrationError } from '@/lib/core/orchestration/types'
import {
Expand Down
31 changes: 31 additions & 0 deletions apps/sim/lib/copilot/tools/server/user/get-credentials.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const {
getUserPermissionConfigMock,
getAccessibleOAuthCredentialsMock,
checkWorkspaceAccessMock,
verifyWorkflowAccessMock,
} = vi.hoisted(() => ({
getAllOAuthServicesMock: vi.fn(),
decodeJwtMock: vi.fn(),
Expand All @@ -33,6 +34,7 @@ const {
getUserPermissionConfigMock: vi.fn(),
getAccessibleOAuthCredentialsMock: vi.fn(),
checkWorkspaceAccessMock: vi.fn(),
verifyWorkflowAccessMock: vi.fn(),
}))

const getPersonalAndWorkspaceEnvMock = environmentUtilsMockFns.mockGetPersonalAndWorkspaceEnv
Expand Down Expand Up @@ -93,6 +95,11 @@ vi.mock('jose', () => ({
decodeJwt: decodeJwtMock,
}))

vi.mock('@/lib/copilot/auth/permissions', () => ({
verifyWorkflowAccess: verifyWorkflowAccessMock,
createPermissionError: (action: string) => `Permission denied: ${action}`,
}))

import { getCredentialsServerTool } from './get-credentials'

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

it('resolves the workspace from a workflow in the execution workspace', async () => {
verifyWorkflowAccessMock.mockResolvedValue({ hasAccess: true, workspaceId: 'workspace-1' })
getUserPermissionConfigMock.mockResolvedValue({ allowedIntegrations: null })

await getCredentialsServerTool.execute(
{ workflowId: 'wf-1' },
{ userId: 'user-1', workspaceId: 'workspace-1' }
)

expect(verifyWorkflowAccessMock).toHaveBeenCalledWith('user-1', 'wf-1')
expect(getUserPermissionConfigMock).toHaveBeenCalledWith('user-1', 'workspace-1')
})

it('rejects a workflowId whose workspace differs from the execution workspace', async () => {
verifyWorkflowAccessMock.mockResolvedValue({ hasAccess: true, workspaceId: 'workspace-other' })

await expect(
getCredentialsServerTool.execute(
{ workflowId: 'wf-other' },
{ userId: 'user-1', workspaceId: 'workspace-1' }
)
).rejects.toThrow('Workspace ID does not match the Copilot execution workspace')
})

it('rejects unauthenticated callers without touching the database', async () => {
await expect(getCredentialsServerTool.execute({}, undefined)).rejects.toThrow(
'Authentication required'
Expand Down
3 changes: 2 additions & 1 deletion apps/sim/lib/copilot/tools/server/user/get-credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { eq } from 'drizzle-orm'
import { decodeJwt } from 'jose'
import { createPermissionError, verifyWorkflowAccess } from '@/lib/copilot/auth/permissions'
import type { BaseServerTool } from '@/lib/copilot/tools/server/base-tool'
import { requireCopilotWorkspace } from '@/lib/copilot/tools/server/workspace-scope'
import { getAllowedIntegrationsFromEnv } from '@/lib/core/config/env-flags'
import { OrchestrationError } from '@/lib/core/orchestration/types'
import { getAccessibleOAuthCredentials } from '@/lib/credentials/environment'
Expand Down Expand Up @@ -54,7 +55,7 @@ export const getCredentialsServerTool: BaseServerTool<GetCredentialsParams, any>
throw new OrchestrationError('forbidden', errorMessage)
}

workspaceId = wId
workspaceId = requireCopilotWorkspace(context, wId)
}

const userId = authenticatedUserId
Expand Down
Loading
Loading