fix(copilot): gate post-tool output writes behind write permission (#5241)

* fix(copilot): gate post-tool output writes behind write permission

The Copilot/Mothership executor runs three post-tool output-redirection
sinks (maybeWriteOutputToFile, maybeWriteOutputToTable,
maybeWriteReadCsvToTable) that persist a tool's result into the
workspace. They were gated only on identity (workspaceId + userId), not
on permission. Because function_execute/user_table/read are read-allowed
for execution (absent from WRITE_ACTIONS in tools/server/router.ts), a
read-only collaborator could drive the agent to durably create/overwrite
workspace files and insert/overwrite table rows via output declarations —
a function-level authorization bypass (CWE-862) that the dedicated write
tools correctly reject.

Add a shared denyOutputWriteWithoutWritePermission guard built on the
canonical permissionSatisfies predicate and apply it to all three sinks,
once a write is actually intended, so read-only principals get the same
Permission denied outcome as the dedicated mutation tools.

* fix(copilot): move file output write-permission gate after no-op skip branches

Address Cursor review: in maybeWriteOutputToFile the gate ran before the
sandbox-export skip branch (which returns the result unchanged without
writing), so a read-only caller with a sandbox files payload was denied
even though no workspace write would occur. Move the check to immediately
before writeWorkspaceFileByPath so it only fires when a write is actually
performed.

apps/sim/lib/copilot/request/tools/files.test.ts

@@ -1,13 +1,33 @@
 /**
  * @vitest-environment node
  */
-import { describe, expect, it } from 'vitest'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { mockWriteWorkspaceFileByPath } = vi.hoisted(() => ({
+  mockWriteWorkspaceFileByPath: vi.fn(),
+}))
+
+vi.mock('@/lib/copilot/vfs/resource-writer', () => ({
+  writeWorkspaceFileByPath: mockWriteWorkspaceFileByPath,
+}))
+
+vi.mock('@/lib/copilot/request/otel', () => ({
+  withCopilotSpan: (
+    _name: string,
+    _attrs: Record<string, unknown> | undefined,
+    fn: (span: unknown) => Promise<unknown>
+  ) => fn({ setAttribute: vi.fn(), setAttributes: vi.fn(), addEvent: vi.fn() }),
+}))
+
+import { FunctionExecute } from '@/lib/copilot/generated/tool-catalog-v1'
 import {
   extractTabularData,
+  maybeWriteOutputToFile,
   normalizeOutputWorkspaceFileName,
   serializeOutputForFile,
   unwrapFunctionExecuteOutput,
 } from '@/lib/copilot/request/tools/files'
+import type { ExecutionContext } from '@/lib/copilot/request/types'
 
 describe('unwrapFunctionExecuteOutput', () => {
   it('unwraps the function_execute envelope { result, stdout }', () => {
@@ -87,6 +107,65 @@ describe('normalizeOutputWorkspaceFileName', () => {
   })
 })
 
+describe('maybeWriteOutputToFile', () => {
+  function buildContext(overrides: Partial<ExecutionContext> = {}): ExecutionContext {
+    return {
+      userId: 'user-1',
+      workflowId: 'wf-1',
+      workspaceId: 'workspace-1',
+      userPermission: 'write',
+      ...overrides,
+    }
+  }
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mockWriteWorkspaceFileByPath.mockResolvedValue({
+      id: 'file-1',
+      name: 'report.csv',
+      vfsPath: 'files/report.csv',
+      mode: 'overwrite',
+    })
+  })
+
+  it('denies a read-only principal without writing the file', async () => {
+    const result = await maybeWriteOutputToFile(
+      FunctionExecute.id,
+      { outputs: { files: [{ path: 'files/report.csv', mode: 'overwrite' }] } },
+      { success: true, output: { result: 'name,age\nAlice,30', stdout: '' } },
+      buildContext({ userPermission: 'read' })
+    )
+
+    expect(result.success).toBe(false)
+    expect(result.error).toContain('requires write access')
+    expect(mockWriteWorkspaceFileByPath).not.toHaveBeenCalled()
+  })
+
+  it('does not deny a read-only principal when no workspace write occurs (sandbox export active)', async () => {
+    const result = await maybeWriteOutputToFile(
+      FunctionExecute.id,
+      { outputs: { files: [{ path: 'files/report.csv', mode: 'overwrite' }] } },
+      { success: true, output: { result: { files: [{ path: 'report.csv' }] }, stdout: '' } },
+      buildContext({ userPermission: 'read' })
+    )
+
+    expect(result.success).toBe(true)
+    expect(mockWriteWorkspaceFileByPath).not.toHaveBeenCalled()
+  })
+
+  it('writes the output file for a write principal', async () => {
+    const result = await maybeWriteOutputToFile(
+      FunctionExecute.id,
+      { outputs: { files: [{ path: 'files/report.csv', mode: 'overwrite' }] } },
+      { success: true, output: { result: 'name,age\nAlice,30', stdout: '' } },
+      buildContext()
+    )
+
+    expect(result.success).toBe(true)
+    expect(mockWriteWorkspaceFileByPath).toHaveBeenCalledTimes(1)
+  })
+})
+
 describe('extractTabularData', () => {
   it('extracts rows directly from an array input', () => {
     expect(extractTabularData([{ a: 1 }, { a: 2 }])).toEqual([{ a: 1 }, { a: 2 }])

apps/sim/lib/copilot/request/tools/files.ts

@@ -6,6 +6,7 @@ import { TraceAttr } from '@/lib/copilot/generated/trace-attributes-v1'
 import { TraceEvent } from '@/lib/copilot/generated/trace-events-v1'
 import { TraceSpan } from '@/lib/copilot/generated/trace-spans-v1'
 import { withCopilotSpan } from '@/lib/copilot/request/otel'
+import { denyOutputWriteWithoutWritePermission } from '@/lib/copilot/request/tools/permissions'
 import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types'
 import { decodeVfsPathSegments } from '@/lib/copilot/vfs/path-utils'
 import { writeWorkspaceFileByPath } from '@/lib/copilot/vfs/resource-writer'
@@ -228,6 +229,9 @@ export async function maybeWriteOutputToFile(
     return result
   }
 
+  const denied = denyOutputWriteWithoutWritePermission(context)
+  if (denied) return denied
+
   // Only span the actual write path (where we upload to storage). Fast
   // no-op returns above don't need a span — they'd just pad the trace
   // with empty work.

apps/sim/lib/copilot/request/tools/permissions.ts

@@ -0,0 +1,28 @@
+import { type PermissionType, permissionSatisfies } from '@sim/platform-authz/workspace'
+import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types'
+
+/**
+ * Guards a post-tool output-redirection sink against read-only principals.
+ *
+ * `function_execute`, `user_table`, and `read` are read-allowed for execution
+ * (they don't mutate the workspace themselves), so the router's `WRITE_ACTIONS`
+ * gate in `tools/server/router.ts` lets read-only collaborators run them. But
+ * their output-redirection declarations (`outputs.files`, `outputTable`)
+ * durably persist to the workspace — creating/overwriting files and table rows.
+ * Those writes must satisfy the same write gate as the dedicated mutation tools.
+ *
+ * Returns a denial `ToolCallResult` when the caller lacks write access (so the
+ * agent surfaces the same `Permission denied` outcome it gets from `create_file`
+ * / `user_table` writes), or `null` when the write may proceed.
+ */
+export function denyOutputWriteWithoutWritePermission(
+  context: ExecutionContext
+): ToolCallResult | null {
+  if (permissionSatisfies(context.userPermission as PermissionType | undefined, 'write')) {
+    return null
+  }
+  return {
+    success: false,
+    error: `Permission denied: writing tool output to the workspace requires write access. You have '${context.userPermission ?? 'none'}' permission.`,
+  }
+}

apps/sim/lib/copilot/request/tools/tables.test.ts

@@ -61,6 +61,7 @@ function buildContext(overrides: Partial<ExecutionContext> = {}): ExecutionConte
     userId: 'user-1',
     workflowId: 'wf-1',
     workspaceId: 'workspace-1',
+    userPermission: 'write',
     ...overrides,
   }
 }
@@ -86,6 +87,20 @@ describe('maybeWriteOutputToTable', () => {
     expect(mockReplaceTableRows).not.toHaveBeenCalled()
   })
 
+  it('denies a read-only principal without touching the table', async () => {
+    const result = await maybeWriteOutputToTable(
+      FunctionExecute.id,
+      { outputTable: 'tbl_1' },
+      { success: true, output: { result: [{ name: 'Alice' }] } },
+      buildContext({ userPermission: 'read' })
+    )
+
+    expect(result.success).toBe(false)
+    expect(result.error).toContain('requires write access')
+    expect(mockGetTableById).not.toHaveBeenCalled()
+    expect(mockReplaceTableRows).not.toHaveBeenCalled()
+  })
+
   it('replaces rows through the service with name keys remapped to column ids', async () => {
     const result = await maybeWriteOutputToTable(
       FunctionExecute.id,
@@ -179,6 +194,20 @@ describe('maybeWriteReadCsvToTable', () => {
     expect(mockReplaceTableRows).not.toHaveBeenCalled()
   })
 
+  it('denies a read-only principal without touching the table', async () => {
+    const result = await maybeWriteReadCsvToTable(
+      ReadTool.id,
+      { outputTable: 'tbl_1', path: 'files/people.csv' },
+      { success: true, output: { content: 'name,age\nAlice,30' } },
+      buildContext({ userPermission: 'read' })
+    )
+
+    expect(result.success).toBe(false)
+    expect(result.error).toContain('requires write access')
+    expect(mockGetTableById).not.toHaveBeenCalled()
+    expect(mockReplaceTableRows).not.toHaveBeenCalled()
+  })
+
   it('imports CSV content through the service with id-keyed rows', async () => {
     const result = await maybeWriteReadCsvToTable(
       ReadTool.id,

apps/sim/lib/copilot/request/tools/tables.ts

@@ -8,6 +8,7 @@ import { TraceAttr } from '@/lib/copilot/generated/trace-attributes-v1'
 import { TraceEvent } from '@/lib/copilot/generated/trace-events-v1'
 import { TraceSpan } from '@/lib/copilot/generated/trace-spans-v1'
 import { withCopilotSpan } from '@/lib/copilot/request/otel'
+import { denyOutputWriteWithoutWritePermission } from '@/lib/copilot/request/tools/permissions'
 import type { ExecutionContext, ToolCallResult } from '@/lib/copilot/request/types'
 import type { RowData, TableDefinition } from '@/lib/table'
 import { buildIdByName, rowDataNameToId } from '@/lib/table/column-keys'
@@ -63,6 +64,9 @@ export async function maybeWriteOutputToTable(
   const outputTable = params?.outputTable as string | undefined
   if (!outputTable) return result
 
+  const denied = denyOutputWriteWithoutWritePermission(context)
+  if (denied) return denied
+
   return withCopilotSpan(
     TraceSpan.CopilotToolsWriteOutputTable,
     {
@@ -178,6 +182,9 @@ export async function maybeWriteReadCsvToTable(
   const outputTable = params?.outputTable as string | undefined
   if (!outputTable) return result
 
+  const denied = denyOutputWriteWithoutWritePermission(context)
+  if (denied) return denied
+
   return withCopilotSpan(
     TraceSpan.CopilotToolsWriteCsvToTable,
     {