From 8783ba841db33672b271af6e15cb52b1adca4c22 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 4 Nov 2026 13:27:37 -0800 Subject: [PATCH 1/3] fix(files): let Mothership tool runs write workspace files `tools execute` authorizes the chat's delegated principal for `sim:tool-execution` and hands it to the tool layer unchanged, but every workspace-file use case admits only `sim:workspace-files`. Every File tool run directly from a Mothership chat therefore failed with a 403 "Delegated workspace access is no longer valid", which the agent relayed to the user as lost workspace access. The File tool handler now rebinds a delegated caller to the workspace-files audience (keeping subject, workspace, expiry ceiling and resource scope), as function-execute and payload materialization already do; the file use cases still check the subject's current workspace role. `image_generate` outside a workflow run returned only a URL for its copilot-storage upload, so the file-output processor downloaded and re-uploaded it, storing every image twice under two keys and returning `imageUrl` and `image` that disagreed. It now returns the stored file. --- .../delegated-caller-write.integration.ts | 153 ++++++++++++++++++ apps/sim/lib/internal/file/execute-tool.ts | 42 ++++- apps/sim/lib/internal/image/operations.ts | 1 + 3 files changed, 194 insertions(+), 2 deletions(-) create mode 100644 apps/sim/lib/internal/file/delegated-caller-write.integration.ts diff --git a/apps/sim/lib/internal/file/delegated-caller-write.integration.ts b/apps/sim/lib/internal/file/delegated-caller-write.integration.ts new file mode 100644 index 00000000000..03a15197606 --- /dev/null +++ b/apps/sim/lib/internal/file/delegated-caller-write.integration.ts @@ -0,0 +1,153 @@ +/** + * Mothership runs a file tool through `tools execute`, whose use case authorizes the chat's + * delegated principal for `sim:tool-execution`. The File write that follows must still reach the + * workspace-file use cases, which admit only `sim:workspace-files`, and must still refuse a + * subject or workspace the original delegation does not cover. Exercises the real handler, + * application authorization, metadata and local storage against Postgres. + */ +import { mkdtempSync } from 'node:fs' +import { rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import path from 'node:path' +import type { SubjectDelegatedPrincipal } from '@sim/auth/principal' +import { db } from '@sim/db' +import { auditLog, organization, user, workspace, workspaceFiles } from '@sim/db/schema' +import { deleteWorkspaceFixture } from '@sim/db/testing/workspace-fixtures' +import { generateId } from '@sim/utils/id' +import { and, eq, inArray } from 'drizzle-orm' +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' + +const fixtureStorage = vi.hoisted(() => ({ root: '' })) +vi.mock('@/lib/uploads/core/setup.server', () => ({ + get UPLOAD_DIR_SERVER() { + return fixtureStorage.root + }, +})) + +import { executeFileTool } from '@/lib/internal/file/execute-tool' +import { + createKnowledgeAclFixtureIds, + seedKnowledgeAclFixture, +} from '@/lib/knowledge/__integration__/seed-source-access-fixture' +import { uploadCopilotFile } from '@/lib/uploads/contexts/copilot/file-manager' + +const fixtures: ReturnType[] = [] + +async function seed() { + const ids = createKnowledgeAclFixtureIds() + fixtures.push(ids) + await seedKnowledgeAclFixture(ids) + return ids +} + +type Fixture = Awaited> + +/** The chat principal `tools execute` hands to the tool layer. */ +function toolExecutionPrincipal(subjectUserId: string, workspaceId: string) { + const issuedAt = new Date() + return { + kind: 'delegated', + serviceId: 'copilot', + subjectUserId, + workspaceId, + delegationId: generateId(), + audience: 'sim:tool-execution', + issuedAt, + expiresAt: new Date(issuedAt.getTime() + 60_000), + resourceScope: { chatId: generateId() }, + } satisfies SubjectDelegatedPrincipal +} + +/** A generated image as `image_generate` stores it outside a workflow run. */ +async function generatedImage(ownerId: string) { + return uploadCopilotFile({ + buffer: Buffer.from(`image bytes ${generateId()}`), + fileName: `${generateId()}.webp`, + contentType: 'image/webp', + userId: ownerId, + }) +} + +async function writeCopy( + ids: Fixture, + principal: SubjectDelegatedPrincipal, + fileInput: Awaited> +) { + const fileName = `saved-${generateId()}.webp` + const response = await executeFileTool({ + toolId: 'file_write', + input: { operation: 'write', workspaceId: ids.workspaceId, fileName, fileInput }, + headers: new Headers(), + requestId: generateId(), + context: { + workflowId: '', + workspaceId: ids.workspaceId, + userId: principal.subjectUserId, + callerPrincipal: principal, + }, + }) + const [saved] = await db + .select({ id: workspaceFiles.id, sizeBytes: workspaceFiles.sizeBytes }) + .from(workspaceFiles) + .where( + and( + eq(workspaceFiles.workspaceId, ids.workspaceId), + eq(workspaceFiles.originalName, fileName), + eq(workspaceFiles.context, 'workspace') + ) + ) + return { status: response.status, saved } +} + +beforeAll(() => { + fixtureStorage.root = mkdtempSync(path.join(tmpdir(), 'sim-delegated-caller-write-')) +}) + +afterAll(async () => { + try { + for (const ids of fixtures) { + await db.delete(auditLog).where(eq(auditLog.workspaceId, ids.workspaceId)) + await db + .delete(workspaceFiles) + .where(inArray(workspaceFiles.userId, [ids.aliceId, ids.bobId])) + await deleteWorkspaceFixture(db, eq(workspace.id, ids.workspaceId)) + await db.delete(organization).where(eq(organization.id, ids.organizationId)) + await db.delete(user).where(inArray(user.id, [ids.aliceId, ids.bobId])) + } + await rm(fixtureStorage.root, { recursive: true, force: true }) + } finally { + await db.$client.end() + } +}) + +describe('a File write run directly from a Mothership chat', () => { + it('saves a generated image into Workspace Files', async () => { + const ids = await seed() + const image = await generatedImage(ids.aliceId) + + const result = await writeCopy(ids, toolExecutionPrincipal(ids.aliceId, ids.workspaceId), image) + + expect(result.status).toBe(200) + expect(result.saved?.sizeBytes).toBe(image.size) + }) + + it('still refuses a member whose role cannot write', async () => { + const ids = await seed() + const image = await generatedImage(ids.bobId) + + const result = await writeCopy(ids, toolExecutionPrincipal(ids.bobId, ids.workspaceId), image) + + expect(result.status).toBeGreaterThanOrEqual(400) + expect(result.saved).toBeUndefined() + }) + + it('still refuses a delegation minted for another workspace', async () => { + const ids = await seed() + const image = await generatedImage(ids.aliceId) + + const result = await writeCopy(ids, toolExecutionPrincipal(ids.aliceId, generateId()), image) + + expect(result.status).toBeGreaterThanOrEqual(400) + expect(result.saved).toBeUndefined() + }) +}) diff --git a/apps/sim/lib/internal/file/execute-tool.ts b/apps/sim/lib/internal/file/execute-tool.ts index 88c3950d70b..d86d830d24e 100644 --- a/apps/sim/lib/internal/file/execute-tool.ts +++ b/apps/sim/lib/internal/file/execute-tool.ts @@ -1,4 +1,8 @@ -import { resolvePrincipalAttribution, resolvePrincipalSubject } from '@sim/auth/principal' +import { + type Principal, + resolvePrincipalAttribution, + resolvePrincipalSubject, +} from '@sim/auth/principal' import { createLogger } from '@sim/logger' import { getErrorMessage, toError } from '@sim/utils/errors' import { z } from 'zod' @@ -25,6 +29,7 @@ import { import { parseInternalToolInput } from '@/lib/internal/tool-operations/parse-input' import type { InternalToolOperationHandler } from '@/lib/internal/tool-operations/types' import { WORKSPACE_FILES_DELEGATION_AUDIENCE } from '@/lib/workspace-files/application/authorization' +import { rebindWorkspaceFileDelegatedPrincipal } from '@/lib/workspace-files/application/delegated-principal' import { searchWorkspaceFileContent } from '@/lib/workspace-files/application/search-workspace-file-content' import { FILE_SEARCH_DEFAULT_MAX_RESULTS, @@ -79,6 +84,35 @@ const fileSearchInputSchema = z }) .strict() +/** + * A delegated caller reaches a file tool through a use case that authorized it for + * its own audience (`sim:tool-execution` for a direct tool call), while every + * workspace-file use case admits only `sim:workspace-files`. Rebinding keeps the + * subject, workspace, expiry ceiling, and resource scope, so the file use cases + * still check the subject's current workspace access. + */ +function bindWorkspaceFilePrincipal( + principal: Principal, + workspaceId: string, + requestId: string +): Principal { + if ( + principal.kind !== 'delegated' || + principal.audience === WORKSPACE_FILES_DELEGATION_AUDIENCE + ) { + return principal + } + const scope = principal.resourceScope + return rebindWorkspaceFileDelegatedPrincipal({ + principal, + workspaceId, + delegationId: `file-tool:${requestId}`, + ...(scope?.fileId ? { fileId: scope.fileId } : {}), + ...(scope?.chatId ? { chatId: scope.chatId } : {}), + ...(scope?.executionId ? { executionId: scope.executionId } : {}), + }) +} + export const executeFileTool: InternalToolOperationHandler = async (request) => { request.signal?.throwIfAborted() if (!FILE_MANAGE_TOOL_IDS.has(request.toolId)) { @@ -117,7 +151,11 @@ export const executeFileTool: InternalToolOperationHandler = async (request) => try { const principal = request.context.callerPrincipal && !request.context.executorDelegationOrigin - ? request.context.callerPrincipal + ? bindWorkspaceFilePrincipal( + request.context.callerPrincipal, + workspaceId, + request.requestId + ) : await createExecutorPrincipalFromExecutionContext({ context: request.context, audience: WORKSPACE_FILES_DELEGATION_AUDIENCE, diff --git a/apps/sim/lib/internal/image/operations.ts b/apps/sim/lib/internal/image/operations.ts index 82f61d11c66..2831ff9517d 100644 --- a/apps/sim/lib/internal/image/operations.ts +++ b/apps/sim/lib/internal/image/operations.ts @@ -654,6 +654,7 @@ async function storeGeneratedImage( return { content: imageUrl, imageUrl, + imageFile: fileInfo, fileName: safeFileName, contentType: imageResult.contentType, provider: imageResult.provider, From 74c4281f4c9abce5549dd5d41b8dce0237ad2b21 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 4 Nov 2026 13:41:14 -0800 Subject: [PATCH 2/3] improvement(files): sniff generated images once and refuse foreign delegations with 403 Generated images are now content-sniffed before they are stored, so every output field references the same verified object on both the workflow and chat paths; skipping the re-upload had otherwise skipped the sniff it did. A delegation minted for another workspace is no longer rebound (which threw and surfaced a 500); the workspace-file use cases refuse it with their ordinary 403. The regression test now uses the shared principal factory and uploads mock. --- .../delegated-caller-write.integration.ts | 29 +++---- apps/sim/lib/internal/file/execute-tool.ts | 17 ++-- apps/sim/lib/internal/image/operations.ts | 83 +++++++------------ .../sim/lib/internal/tool-operations/types.ts | 5 +- 4 files changed, 54 insertions(+), 80 deletions(-) diff --git a/apps/sim/lib/internal/file/delegated-caller-write.integration.ts b/apps/sim/lib/internal/file/delegated-caller-write.integration.ts index 03a15197606..08f240595f2 100644 --- a/apps/sim/lib/internal/file/delegated-caller-write.integration.ts +++ b/apps/sim/lib/internal/file/delegated-caller-write.integration.ts @@ -13,16 +13,13 @@ import type { SubjectDelegatedPrincipal } from '@sim/auth/principal' import { db } from '@sim/db' import { auditLog, organization, user, workspace, workspaceFiles } from '@sim/db/schema' import { deleteWorkspaceFixture } from '@sim/db/testing/workspace-fixtures' +import { createDelegatedPrincipal } from '@sim/testing' +import { setUploadDirServer, uploadsSetupMock } from '@sim/testing/mocks/uploads-setup.mock' import { generateId } from '@sim/utils/id' import { and, eq, inArray } from 'drizzle-orm' import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' -const fixtureStorage = vi.hoisted(() => ({ root: '' })) -vi.mock('@/lib/uploads/core/setup.server', () => ({ - get UPLOAD_DIR_SERVER() { - return fixtureStorage.root - }, -})) +vi.mock('@/lib/uploads/core/setup.server', () => uploadsSetupMock) import { executeFileTool } from '@/lib/internal/file/execute-tool' import { @@ -44,18 +41,12 @@ type Fixture = Awaited> /** The chat principal `tools execute` hands to the tool layer. */ function toolExecutionPrincipal(subjectUserId: string, workspaceId: string) { - const issuedAt = new Date() - return { - kind: 'delegated', - serviceId: 'copilot', + return createDelegatedPrincipal({ subjectUserId, workspaceId, - delegationId: generateId(), audience: 'sim:tool-execution', - issuedAt, - expiresAt: new Date(issuedAt.getTime() + 60_000), resourceScope: { chatId: generateId() }, - } satisfies SubjectDelegatedPrincipal + }) } /** A generated image as `image_generate` stores it outside a workflow run. */ @@ -99,8 +90,10 @@ async function writeCopy( return { status: response.status, saved } } +const uploadRoot = mkdtempSync(path.join(tmpdir(), 'sim-delegated-caller-write-')) + beforeAll(() => { - fixtureStorage.root = mkdtempSync(path.join(tmpdir(), 'sim-delegated-caller-write-')) + setUploadDirServer(uploadRoot) }) afterAll(async () => { @@ -114,7 +107,7 @@ afterAll(async () => { await db.delete(organization).where(eq(organization.id, ids.organizationId)) await db.delete(user).where(inArray(user.id, [ids.aliceId, ids.bobId])) } - await rm(fixtureStorage.root, { recursive: true, force: true }) + await rm(uploadRoot, { recursive: true, force: true }) } finally { await db.$client.end() } @@ -137,7 +130,7 @@ describe('a File write run directly from a Mothership chat', () => { const result = await writeCopy(ids, toolExecutionPrincipal(ids.bobId, ids.workspaceId), image) - expect(result.status).toBeGreaterThanOrEqual(400) + expect(result.status).toBe(403) expect(result.saved).toBeUndefined() }) @@ -147,7 +140,7 @@ describe('a File write run directly from a Mothership chat', () => { const result = await writeCopy(ids, toolExecutionPrincipal(ids.aliceId, generateId()), image) - expect(result.status).toBeGreaterThanOrEqual(400) + expect(result.status).toBe(403) expect(result.saved).toBeUndefined() }) }) diff --git a/apps/sim/lib/internal/file/execute-tool.ts b/apps/sim/lib/internal/file/execute-tool.ts index d86d830d24e..15764b6d1db 100644 --- a/apps/sim/lib/internal/file/execute-tool.ts +++ b/apps/sim/lib/internal/file/execute-tool.ts @@ -85,11 +85,9 @@ const fileSearchInputSchema = z .strict() /** - * A delegated caller reaches a file tool through a use case that authorized it for - * its own audience (`sim:tool-execution` for a direct tool call), while every - * workspace-file use case admits only `sim:workspace-files`. Rebinding keeps the - * subject, workspace, expiry ceiling, and resource scope, so the file use cases - * still check the subject's current workspace access. + * A delegated caller arrives with its calling use case's audience (`sim:tool-execution`), + * but workspace-file use cases admit only `sim:workspace-files`; they still check the + * subject's access. A delegation for another workspace is left for them to refuse. */ function bindWorkspaceFilePrincipal( principal: Principal, @@ -98,7 +96,8 @@ function bindWorkspaceFilePrincipal( ): Principal { if ( principal.kind !== 'delegated' || - principal.audience === WORKSPACE_FILES_DELEGATION_AUDIENCE + principal.audience === WORKSPACE_FILES_DELEGATION_AUDIENCE || + principal.workspaceId !== workspaceId ) { return principal } @@ -107,9 +106,9 @@ function bindWorkspaceFilePrincipal( principal, workspaceId, delegationId: `file-tool:${requestId}`, - ...(scope?.fileId ? { fileId: scope.fileId } : {}), - ...(scope?.chatId ? { chatId: scope.chatId } : {}), - ...(scope?.executionId ? { executionId: scope.executionId } : {}), + fileId: scope?.fileId, + chatId: scope?.chatId, + executionId: scope?.executionId, }) } diff --git a/apps/sim/lib/internal/image/operations.ts b/apps/sim/lib/internal/image/operations.ts index 2831ff9517d..0feef97bf1f 100644 --- a/apps/sim/lib/internal/image/operations.ts +++ b/apps/sim/lib/internal/image/operations.ts @@ -1,7 +1,7 @@ import { createLogger } from '@sim/logger' import { getErrorMessage } from '@sim/utils/errors' import { interruptibleSleep } from '@sim/utils/helpers' -import { isRecordLike } from '@sim/utils/object' +import { isRecordLike, omit } from '@sim/utils/object' import { getUserFacingErrorMessage } from '@/lib/core/errors/database-query-error' import { getMaxExecutionTimeout } from '@/lib/core/execution-limits' import { @@ -23,6 +23,7 @@ import { recordServiceMeteringFailure } from '@/lib/mothership/billing/service-o import { type FalAICostMetadata, getFalAICostMetadata } from '@/lib/tools/falai-pricing' import { uploadCopilotFile } from '@/lib/uploads/contexts/copilot' import { uploadExecutionFile } from '@/lib/uploads/contexts/execution' +import { resolveStoredFileMetadata } from '@/lib/uploads/utils/stored-file-metadata' const logger = createLogger('ImageOperations') const MAX_IMAGE_BYTES = MAX_REMOTE_IMAGE_BYTES @@ -595,8 +596,11 @@ async function storeGeneratedImage( context: ImageOperationContext ): Promise { context.signal?.throwIfAborted() - const timestamp = Date.now() - const safeFileName = imageResult.fileName || `image-${imageResult.provider}-${timestamp}.png` + const { fileName, mimeType: contentType } = resolveStoredFileMetadata( + imageResult.fileName || `image-${imageResult.provider}-${Date.now()}.png`, + imageResult.contentType, + imageResult.buffer + ) const executionContext = context.workspaceId && context.workflowId && context.executionId ? { @@ -606,57 +610,32 @@ async function storeGeneratedImage( } : null - if (executionContext) { - const imageFile = await uploadExecutionFile( - executionContext, - imageResult.buffer, - safeFileName, - imageResult.contentType, - context.userId - ) - context.signal?.throwIfAborted() - - return { - content: imageFile.url, - imageUrl: imageFile.url, - imageFile, - fileName: safeFileName, - contentType: imageResult.contentType, - provider: imageResult.provider, - model: imageResult.model, - metadata: { - provider: imageResult.provider, - model: imageResult.model, - description: imageResult.description, - revisedPrompt: imageResult.revisedPrompt, - seed: imageResult.seed, - jobId: imageResult.jobId, - contentType: imageResult.contentType, - }, - __falaiCostDollars: imageResult.falaiCost?.costDollars, - __falaiBilling: imageResult.falaiCost, - } - } - - const fileInfo = await uploadCopilotFile({ - buffer: imageResult.buffer, - fileName: safeFileName, - contentType: imageResult.contentType, - userId: context.userId, - }) + // A stored file output is kept as is; a bare URL would be downloaded and stored again. + const imageFile = executionContext + ? await uploadExecutionFile( + executionContext, + imageResult.buffer, + fileName, + contentType, + context.userId + ) + : omit( + await uploadCopilotFile({ + buffer: imageResult.buffer, + fileName, + contentType, + userId: context.userId, + }), + ['mimeType'] + ) context.signal?.throwIfAborted() - const imageUrl = fileInfo.url - logger.info(`[${context.requestId}] Stored generated image fallback`, { - fileName: safeFileName, - size: imageResult.buffer.length, - }) return { - content: imageUrl, - imageUrl, - imageFile: fileInfo, - fileName: safeFileName, - contentType: imageResult.contentType, + content: imageFile.url, + imageUrl: imageFile.url, + imageFile, + fileName, + contentType, provider: imageResult.provider, model: imageResult.model, metadata: { @@ -666,7 +645,7 @@ async function storeGeneratedImage( revisedPrompt: imageResult.revisedPrompt, seed: imageResult.seed, jobId: imageResult.jobId, - contentType: imageResult.contentType, + contentType, }, __falaiCostDollars: imageResult.falaiCost?.costDollars, __falaiBilling: imageResult.falaiCost, diff --git a/apps/sim/lib/internal/tool-operations/types.ts b/apps/sim/lib/internal/tool-operations/types.ts index 605899e1adf..9a6db19cab7 100644 --- a/apps/sim/lib/internal/tool-operations/types.ts +++ b/apps/sim/lib/internal/tool-operations/types.ts @@ -22,7 +22,10 @@ export interface InternalToolOperationContext { executorDelegationOrigin?: ExecutorDelegationOrigin /** Trusted source block for saved MCP operation restrictions. */ mcpBlockId?: string - /** Authenticated direct caller, supplied by an authorized use case, never tool input. */ + /** + * Authenticated direct caller, supplied by an authorized use case, never tool input. A + * delegated principal carries that use case's audience; rebind it before another use case. + */ callerPrincipal?: Principal copilotToolExecution?: boolean copilotInteractionMode?: 'interactive' | 'headless' From aa7c133ec0a9a2603d29abc3792fdf6e29822c65 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 4 Nov 2026 15:09:35 -0800 Subject: [PATCH 3/3] chore(files): import the principal factory directly in the delegated write test --- .../sim/lib/internal/file/delegated-caller-write.integration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/sim/lib/internal/file/delegated-caller-write.integration.ts b/apps/sim/lib/internal/file/delegated-caller-write.integration.ts index 08f240595f2..d234c9c713f 100644 --- a/apps/sim/lib/internal/file/delegated-caller-write.integration.ts +++ b/apps/sim/lib/internal/file/delegated-caller-write.integration.ts @@ -13,7 +13,7 @@ import type { SubjectDelegatedPrincipal } from '@sim/auth/principal' import { db } from '@sim/db' import { auditLog, organization, user, workspace, workspaceFiles } from '@sim/db/schema' import { deleteWorkspaceFixture } from '@sim/db/testing/workspace-fixtures' -import { createDelegatedPrincipal } from '@sim/testing' +import { createDelegatedPrincipal } from '@sim/testing/factories/principal.factory' import { setUploadDirServer, uploadsSetupMock } from '@sim/testing/mocks/uploads-setup.mock' import { generateId } from '@sim/utils/id' import { and, eq, inArray } from 'drizzle-orm'