Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
import { createLogger } from '@sim/logger'
import { getErrorMessage, toError } from '@sim/utils/errors'
import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types'
import { copilotToolCanWrite, copilotWriteDeniedMessage } from '@/lib/copilot/tools/permissions'
import { captureServerEvent } from '@/lib/posthog/server'
import {
deleteCustomTool,
Expand Down Expand Up @@ -30,7 +31,6 @@ interface ManageCustomToolParams {
schema?: ManageCustomToolSchema
code?: string
title?: string
workspaceId?: string
}

export async function executeManageCustomTool(
Expand All @@ -39,22 +39,25 @@ export async function executeManageCustomTool(
): Promise<ToolCallResult> {
const params = rawParams as ManageCustomToolParams
const operation = String(params.operation || '').toLowerCase() as ManageCustomToolOperation
const workspaceId = params.workspaceId || context.workspaceId
/**
* Server-set context only. A model-supplied `params.workspaceId` used to win
* here, while the permission gate above is resolved for the CONTEXT
* workspace — so a caller could name another workspace and have it
* authorized against their own. `upsertCustomTools` does no authz of its own
* (it only scopes queries by the id it is handed), so nothing downstream
* caught it. Matches manage_mcp_tool and manage_skill.
*/
const workspaceId = context.workspaceId

if (!operation) {
return { success: false, error: "Missing required 'operation' argument" }
}

const writeOps: string[] = ['add', 'edit', 'delete']
if (
writeOps.includes(operation) &&
context.userPermission &&
context.userPermission !== 'write' &&
context.userPermission !== 'admin'
) {
if (writeOps.includes(operation) && !copilotToolCanWrite(context.userPermission)) {
return {
success: false,
error: `Permission denied: '${operation}' on manage_custom_tool requires write access. You have '${context.userPermission}' permission.`,
error: copilotWriteDeniedMessage('manage_custom_tool', operation, context.userPermission),
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { createLogger } from '@sim/logger'
import { getErrorMessage, toError } from '@sim/utils/errors'
import { and, eq, isNull } from 'drizzle-orm'
import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types'
import { copilotToolCanWrite, copilotWriteDeniedMessage } from '@/lib/copilot/tools/permissions'
import {
performCreateMcpServer,
performDeleteMcpServer,
Expand Down Expand Up @@ -46,15 +47,10 @@ export async function executeManageMcpTool(
}

const writeOps: string[] = ['add', 'edit', 'delete']
if (
writeOps.includes(operation) &&
context.userPermission &&
context.userPermission !== 'write' &&
context.userPermission !== 'admin'
) {
if (writeOps.includes(operation) && !copilotToolCanWrite(context.userPermission)) {
return {
success: false,
error: `Permission denied: '${operation}' on manage_mcp_tool requires write access. You have '${context.userPermission}' permission.`,
error: copilotWriteDeniedMessage('manage_mcp_tool', operation, context.userPermission),
}
}

Expand Down
10 changes: 3 additions & 7 deletions apps/sim/lib/copilot/tools/handlers/management/manage-skill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
import { createLogger } from '@sim/logger'
import { getErrorMessage, toError } from '@sim/utils/errors'
import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types'
import { copilotToolCanWrite, copilotWriteDeniedMessage } from '@/lib/copilot/tools/permissions'
import { captureServerEvent } from '@/lib/posthog/server'
import { getSkillActorContext } from '@/lib/skills/access'
import { isBuiltinSkillId } from '@/lib/workflows/skills/builtin-skills'
Expand Down Expand Up @@ -37,15 +38,10 @@ export async function executeManageSkill(

// Workspace write gates only creation; edits and deletes are gated per skill
// below (skill editor — explicit editor row or derived workspace admin).
if (
operation === 'add' &&
context.userPermission &&
context.userPermission !== 'write' &&
context.userPermission !== 'admin'
) {
if (operation === 'add' && !copilotToolCanWrite(context.userPermission)) {
return {
success: false,
error: `Permission denied: 'add' on manage_skill requires write access. You have '${context.userPermission}' permission.`,
error: copilotWriteDeniedMessage('manage_skill', operation, context.userPermission),
}
}

Expand Down
34 changes: 34 additions & 0 deletions apps/sim/lib/copilot/tools/handlers/materialize-file.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ const {
mockResolveStorageBillingContext: vi.fn(),
}))

vi.mock('@/lib/copilot/tools/handlers/access', () => ({
ensureWorkspaceAccess: vi.fn(),
}))

vi.mock('@/lib/copilot/tools/handlers/upload-file-reader', () => ({
findMothershipUploadRowByChatAndName: mockFindUpload,
}))
Expand Down Expand Up @@ -128,6 +132,36 @@ const mothershipRow = {
updatedAt: new Date('2026-01-01'),
}

describe('executeMaterializeFile - workspace write gate', () => {
beforeEach(() => {
vi.clearAllMocks()
resetDbChainMock()
})

it.each(['save', 'import', 'extract'])(
'refuses %s without workspace write access and touches no upload',
async (operation) => {
const { ensureWorkspaceAccess } = await import('@/lib/copilot/tools/handlers/access')
vi.mocked(ensureWorkspaceAccess).mockRejectedValueOnce(
new Error('Write access required for this workspace')
)

const result = await executeMaterializeFile({ fileNames: ['a.json'], operation }, context)

expect(result.success).toBe(false)
expect(result.error).toContain('Write access required')
expect(mockFindUpload).not.toHaveBeenCalled()
}
)

it('requires write, not merely read, access', async () => {
const { ensureWorkspaceAccess } = await import('@/lib/copilot/tools/handlers/access')
await executeMaterializeFile({ fileNames: ['a.json'], operation: 'save' }, context)

expect(ensureWorkspaceAccess).toHaveBeenCalledWith(context.workspaceId, context.userId, 'write')
})
})

describe('executeMaterializeFile - unsupported operation', () => {
beforeEach(() => {
vi.clearAllMocks()
Expand Down
10 changes: 10 additions & 0 deletions apps/sim/lib/copilot/tools/handlers/materialize-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
resolveStorageBillingContext,
} from '@/lib/billing/storage'
import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types'
import { ensureWorkspaceAccess } from '@/lib/copilot/tools/handlers/access'
import { findMothershipUploadRowByChatAndName } from '@/lib/copilot/tools/handlers/upload-file-reader'
import { canonicalWorkspaceFilePath, encodeVfsPathSegments } from '@/lib/copilot/vfs/path-utils'
import { getServePathPrefix } from '@/lib/uploads'
Expand Down Expand Up @@ -504,6 +505,15 @@ export async function executeMaterializeFile(
error: `Unsupported materialize_file operation "${operation}". Use "save", "import", or "extract". For CSV/TSV/JSON → use the table subagent; for documents → use the knowledge subagent.`,
}
}

// Every operation writes: save/extract create files, import creates a workflow.
// The handler-map path has no central permission gate.
try {
await ensureWorkspaceAccess(context.workspaceId, context.userId, 'write')
} catch (error) {
return { success: false, error: getErrorMessage(error, 'Workspace write access required') }
}

const succeeded: string[] = []
const failed: Array<{ fileName: string; error: string }> = []
const resources: NonNullable<ToolCallResult['resources']> = []
Expand Down
41 changes: 41 additions & 0 deletions apps/sim/lib/copilot/tools/permissions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* @vitest-environment node
*/
import { describe, expect, it } from 'vitest'
import { copilotToolCanWrite, copilotWriteDeniedMessage } from '@/lib/copilot/tools/permissions'

describe('copilotToolCanWrite', () => {
it('fails closed when the permission is absent', () => {
expect(copilotToolCanWrite(undefined)).toBe(false)
expect(copilotToolCanWrite(null)).toBe(false)
expect(copilotToolCanWrite('')).toBe(false)
})

it('denies read-only and unrecognized permissions', () => {
expect(copilotToolCanWrite('read')).toBe(false)
expect(copilotToolCanWrite('nonsense')).toBe(false)
})

it('allows write and admin', () => {
expect(copilotToolCanWrite('write')).toBe(true)
expect(copilotToolCanWrite('admin')).toBe(true)
})
})

describe('copilotWriteDeniedMessage', () => {
it('names the operation and the caller’s actual permission', () => {
expect(copilotWriteDeniedMessage('manage_custom_tool', 'delete', 'read')).toBe(
"Permission denied: 'delete' on manage_custom_tool requires write access. You have 'read' permission."
)
})

it('reports "none" rather than an empty string when permission is absent', () => {
expect(copilotWriteDeniedMessage('manage_skill', 'add', undefined)).toContain("You have 'none'")
})

it('omits the operation label when there is no operation', () => {
expect(copilotWriteDeniedMessage('knowledge_base', undefined, 'read')).toBe(
"Permission denied: knowledge_base requires write access. You have 'read' permission."
)
})
})
20 changes: 20 additions & 0 deletions apps/sim/lib/copilot/tools/permissions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { type PermissionType, permissionSatisfies } from '@sim/platform-authz/workspace'

/**
* Whether a copilot tool call may write. Fails closed: `userPermission` is
* optional on the execution context, and absent must deny. Shared by the
* server-tool router and the handler-map tools.
*/
export function copilotToolCanWrite(userPermission: string | null | undefined): boolean {
return permissionSatisfies((userPermission ?? null) as PermissionType | null, 'write')
}

/** Renders the denial message shared by both copilot execution paths. */
export function copilotWriteDeniedMessage(
toolName: string,
operation: string | undefined,
userPermission: string | null | undefined
): string {
const actionLabel = operation ? `'${operation}' on ` : ''
return `Permission denied: ${actionLabel}${toolName} requires write access. You have '${userPermission || 'none'}' permission.`
}
59 changes: 59 additions & 0 deletions apps/sim/lib/copilot/tools/server/knowledge/knowledge-base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ vi.mock('@/app/api/knowledge/utils', () => ({
checkKnowledgeBaseWriteAccess: mockCheckKnowledgeBaseWriteAccess,
}))

import { checkAttributedUsageLimits } from '@/lib/billing/core/billing-attribution'
import { knowledgeBaseServerTool } from '@/lib/copilot/tools/server/knowledge/knowledge-base'
import { createSingleDocument } from '@/lib/knowledge/documents/service'
import { getKnowledgeBaseById } from '@/lib/knowledge/service'
import { resolveWorkspaceFileReference } from '@/lib/uploads/contexts/workspace/workspace-file-manager'

const BILLING_ATTRIBUTION = {
actorUserId: 'external-admin',
Expand Down Expand Up @@ -173,3 +177,58 @@ describe('knowledge base connector Copilot operations', () => {
}
)
})

describe('knowledge base add_file usage gate', () => {
beforeEach(() => {
vi.clearAllMocks()
resetDbChainMock()
mockCheckKnowledgeBaseWriteAccess.mockResolvedValue({
hasAccess: true,
knowledgeBase: { id: 'knowledge-base-1', workspaceId: 'workspace-paid', name: 'Paid KB' },
})
vi.mocked(getKnowledgeBaseById).mockResolvedValue({
id: 'knowledge-base-1',
workspaceId: 'workspace-paid',
} as Awaited<ReturnType<typeof getKnowledgeBaseById>>)
})

function addFile() {
return knowledgeBaseServerTool.execute(
{
operation: 'add_file',
args: { knowledgeBaseId: 'knowledge-base-1', filePaths: ['files/report.pdf'] },
},
{
userId: 'external-admin',
workspaceId: 'workspace-paid',
billingAttribution: BILLING_ATTRIBUTION,
}
)
}

it('refuses to index when the payer is over its usage limit', async () => {
vi.mocked(checkAttributedUsageLimits).mockResolvedValue({
isExceeded: true,
message: 'Usage limit exceeded.',
} as Awaited<ReturnType<typeof checkAttributedUsageLimits>>)

const result = await addFile()

expect(result.success).toBe(false)
expect(result.message).toContain('Usage limit exceeded')
// The gate must precede any indexing work, matching the upload routes.
expect(resolveWorkspaceFileReference).not.toHaveBeenCalled()
expect(createSingleDocument).not.toHaveBeenCalled()
})

it('gates on the knowledge base workspace payer, not the caller', async () => {
vi.mocked(checkAttributedUsageLimits).mockResolvedValue({
isExceeded: false,
} as Awaited<ReturnType<typeof checkAttributedUsageLimits>>)
vi.mocked(resolveWorkspaceFileReference).mockResolvedValue(null)

await addFile()

expect(checkAttributedUsageLimits).toHaveBeenCalledWith(BILLING_ATTRIBUTION)
})
})
11 changes: 11 additions & 0 deletions apps/sim/lib/copilot/tools/server/knowledge/knowledge-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,17 @@ export const knowledgeBaseServerTool: BaseServerTool<KnowledgeBaseArgs, Knowledg

const kbWorkspaceId: string = targetKb.workspaceId
const billingAttribution = requireKnowledgeBillingAttribution(context, kbWorkspaceId)

// Gate the payer before accepting indexing work, same as the upload routes.
const usage = await checkAttributedUsageLimits(billingAttribution)
if (usage.isExceeded) {
return {
success: false,
message:
usage.message || 'Usage limit exceeded. Please upgrade your plan to continue.',
}
}

const added: Array<{ documentId: string; filename: string }> = []
const failedFiles: string[] = []

Expand Down
7 changes: 2 additions & 5 deletions apps/sim/lib/copilot/tools/server/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
UserTable,
WorkspaceFile,
} from '@/lib/copilot/generated/tool-catalog-v1'
import { copilotToolCanWrite } from '@/lib/copilot/tools/permissions'
import {
assertServerToolNotAborted,
type BaseServerTool,
Expand Down Expand Up @@ -139,10 +140,6 @@ const WRITE_ACTIONS: Record<string, string[]> = {
[enrichmentRunServerTool.name]: ['*'],
}

function isWritePermission(userPermission: string): boolean {
return userPermission === 'write' || userPermission === 'admin'
}

function isWriteAction(toolName: string, action: string | undefined): boolean {
const writeActions = WRITE_ACTIONS[toolName]
if (!writeActions) return false
Expand Down Expand Up @@ -211,7 +208,7 @@ export async function routeExecution(
if (WRITE_ACTIONS[toolName]) {
const p = payload as Record<string, unknown>
const action = (p?.operation ?? p?.action) as string | undefined
if (isWriteAction(toolName, action) && !isWritePermission(context?.userPermission ?? '')) {
if (isWriteAction(toolName, action) && !copilotToolCanWrite(context?.userPermission)) {
const actionLabel = action ? `'${action}' on ` : ''
throw new Error(
`Permission denied: ${actionLabel}${toolName} requires write access. You have '${context?.userPermission ?? 'none'}' permission.`
Expand Down
Loading
Loading