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
10 changes: 5 additions & 5 deletions apps/sim/app/api/files/multipart/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,19 @@ vi.mock('@/lib/uploads/providers/blob/client', () => ({
}))

vi.mock('@/lib/uploads/contexts/execution/utils', () => ({
generateExecutionAttachmentKey: mockGenerateExecutionAttachmentKey,
generateUniqueExecutionFileKey: mockGenerateUniqueExecutionFileKey,
}))

vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock)

const {
mockCheckStorageQuota,
mockGenerateExecutionAttachmentKey,
mockGenerateUniqueExecutionFileKey,
mockInitiateS3MultipartUpload,
mockResolveStorageBillingContext,
} = vi.hoisted(() => ({
mockCheckStorageQuota: vi.fn(),
mockGenerateExecutionAttachmentKey: vi.fn(),
mockGenerateUniqueExecutionFileKey: vi.fn(),
mockInitiateS3MultipartUpload: vi.fn(),
mockResolveStorageBillingContext: vi.fn(),
}))
Expand Down Expand Up @@ -250,7 +250,7 @@ describe('POST /api/files/multipart action=initiate quota enforcement', () => {
mockResolveStorageBillingContext.mockResolvedValue(STORAGE_CONTEXT)
mockCheckStorageQuota.mockResolvedValue({ allowed: true })
mockInitiateS3MultipartUpload.mockResolvedValue({ uploadId: 'up-1', key: 'k/file.bin' })
mockGenerateExecutionAttachmentKey.mockImplementation(
mockGenerateUniqueExecutionFileKey.mockImplementation(
(
context: { workspaceId: string; workflowId: string; executionId: string },
fileName: string
Expand Down Expand Up @@ -311,7 +311,7 @@ describe('POST /api/files/multipart action=initiate quota enforcement', () => {
})

it('allocates distinct multipart keys for duplicate execution attachment names', async () => {
mockGenerateExecutionAttachmentKey
mockGenerateUniqueExecutionFileKey
.mockReturnValueOnce('execution/ws-1/wf-1/exec-1/one-output.bin')
.mockReturnValueOnce('execution/ws-1/wf-1/exec-1/two-output.bin')
mockInitiateS3MultipartUpload.mockImplementation(async ({ customKey }) => ({
Expand Down
4 changes: 2 additions & 2 deletions apps/sim/app/api/files/multipart/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
{ status: 400 }
)
}
const { generateExecutionAttachmentKey } = await import(
const { generateUniqueExecutionFileKey } = await import(
'@/lib/uploads/contexts/execution/utils'
)
customKey = generateExecutionAttachmentKey(
customKey = generateUniqueExecutionFileKey(
{ workspaceId, workflowId, executionId },
fileName
)
Expand Down
8 changes: 4 additions & 4 deletions apps/sim/app/api/files/presigned/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const {
mockIsUsingCloudStorageUploads,
mockGetUserEntityPermissions,
mockGenerateWorkspaceFileKey,
mockGenerateExecutionAttachmentKey,
mockGenerateUniqueExecutionFileKey,
mockInsertFileMetadata,
mockCheckStorageQuotaForBillingContext,
mockDecrementStorageUsageForBillingContext,
Expand Down Expand Up @@ -52,7 +52,7 @@ const {
mockGenerateWorkspaceFileKey: vi.fn(
(workspaceId: string, fileName: string) => `workspace/${workspaceId}/${fileName}`
),
mockGenerateExecutionAttachmentKey: vi.fn(
mockGenerateUniqueExecutionFileKey: vi.fn(
(ctx: { workspaceId: string; workflowId: string; executionId: string }, fileName: string) =>
`execution/${ctx.workspaceId}/${ctx.workflowId}/${ctx.executionId}/attachment-${fileName}`
),
Expand Down Expand Up @@ -110,7 +110,7 @@ vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({
}))

vi.mock('@/lib/uploads/contexts/execution/utils', () => ({
generateExecutionAttachmentKey: mockGenerateExecutionAttachmentKey,
generateUniqueExecutionFileKey: mockGenerateUniqueExecutionFileKey,
}))

vi.mock('@/lib/uploads/server/metadata', () => ({
Expand Down Expand Up @@ -752,7 +752,7 @@ describe('/api/files/presigned', () => {
describe('execution uploads', () => {
it('allocates distinct create-only keys for duplicate attachment names', async () => {
setupFileApiMocks({ cloudEnabled: true, storageProvider: 's3' })
mockGenerateExecutionAttachmentKey
mockGenerateUniqueExecutionFileKey
.mockReturnValueOnce('execution/ws-1/wf-1/exec-1/one-output.txt')
.mockReturnValueOnce('execution/ws-1/wf-1/exec-1/two-output.txt')

Expand Down
4 changes: 2 additions & 2 deletions apps/sim/app/api/files/presigned/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { getSession } from '@/lib/auth'
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
import { CopilotFiles } from '@/lib/uploads'
import { getServeStoragePrefix } from '@/lib/uploads/config'
import { generateExecutionAttachmentKey } from '@/lib/uploads/contexts/execution/utils'
import { generateUniqueExecutionFileKey } from '@/lib/uploads/contexts/execution/utils'
import { generateKnowledgeBaseFileKey } from '@/lib/uploads/contexts/knowledge-base/knowledge-base-file-manager'
import { generateWorkspaceFileKey } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
import { generatePresignedUploadUrl, hasCloudStorage } from '@/lib/uploads/core/storage-service'
Expand Down Expand Up @@ -222,7 +222,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
throw new ValidationError(fileValidationError.message)
}

const customKey = generateExecutionAttachmentKey(
const customKey = generateUniqueExecutionFileKey(
{ workspaceId, workflowId, executionId },
fileName
)
Expand Down
7 changes: 2 additions & 5 deletions apps/sim/lib/execution/payloads/store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
isValidLargeValueKey,
readLargeValueRefFromStorage,
} from '@/lib/execution/payloads/materialization.server'
import { generateExecutionFileKey } from '@/lib/uploads/contexts/execution/utils'
import { generateLargeValuePayloadKey } from '@/lib/uploads/contexts/execution/utils'

const logger = createLogger('LargeExecutionPayloadStore')

Expand Down Expand Up @@ -75,10 +75,7 @@ async function persistValue(
return undefined
}

const key = generateExecutionFileKey(
{ workspaceId, workflowId, executionId },
`large-value-${id}.json`
)
const key = generateLargeValuePayloadKey({ workspaceId, workflowId, executionId }, id)

try {
const { StorageService } = await import('@/lib/uploads')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,13 @@ vi.mock('@/lib/uploads/providers/s3/client', () => ({

import { uploadExecutionFile } from '@/lib/uploads/contexts/execution/execution-file-manager'

describe('uploadExecutionFile replacement compatibility', () => {
const context = {
workspaceId: 'workspace-1',
workflowId: 'workflow-1',
executionId: 'execution-1',
}

describe('uploadExecutionFile key allocation', () => {
beforeEach(() => {
vi.clearAllMocks()
resetDbChainMock()
Expand All @@ -35,57 +41,27 @@ describe('uploadExecutionFile replacement compatibility', () => {
type: contentType,
}))
mockGetPresignedUrlWithConfig.mockResolvedValue('https://example.com/download')
dbChainMockFns.limit.mockResolvedValue([])
dbChainMockFns.returning.mockResolvedValue([{ id: 'file-1' }])
})

it('allows changed bytes and content type at the same execution-scoped key', async () => {
const context = {
workspaceId: 'workspace-1',
workflowId: 'workflow-1',
executionId: 'execution-1',
}
const key = 'execution/workspace-1/workflow-1/execution-1/result.txt'
const existingMetadata = {
id: 'file-1',
key,
userId: 'user-1',
workspaceId: 'workspace-1',
folderId: null,
context: 'execution',
originalName: key,
displayName: key,
contentType: 'text/plain',
size: 3,
deletedAt: null,
}

dbChainMockFns.limit
.mockResolvedValueOnce([])
.mockResolvedValueOnce([])
.mockResolvedValueOnce([existingMetadata])
dbChainMockFns.returning.mockResolvedValueOnce([existingMetadata])

it('gives same-named files in one execution distinct keys', async () => {
const first = await uploadExecutionFile(
context,
Buffer.from('old'),
'result.txt',
'text/plain',
Buffer.alloc(13575),
'image.png',
'image/png',
'user-1'
)
const second = await uploadExecutionFile(
context,
Buffer.from('{"new":true}'),
'result.txt',
'application/json',
Buffer.alloc(37226),
'image.png',
'image/png',
'user-1'
)

expect(first.key).toBe(key)
expect(second).toMatchObject({
key,
size: 12,
type: 'application/json',
})
expect(mockUploadToS3).toHaveBeenCalledTimes(2)
expect(dbChainMockFns.insert).toHaveBeenCalledTimes(1)
expect(first.key).not.toBe(second.key)
expect(dbChainMockFns.insert).toHaveBeenCalledTimes(2)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import { getErrorMessage } from '@sim/utils/errors'
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
import { isUserFileWithMetadata } from '@/lib/core/utils/user-file'
import type { ExecutionContext } from '@/lib/uploads/contexts/execution/utils'
import { generateExecutionFileKey, generateFileId } from '@/lib/uploads/contexts/execution/utils'
import {
generateFileId,
generateUniqueExecutionFileKey,
} from '@/lib/uploads/contexts/execution/utils'
import type { UserFile } from '@/executor/types'

const logger = createLogger('ExecutionFileStorage')
Expand Down Expand Up @@ -78,7 +81,7 @@ export async function uploadExecutionFile(
bufferSize: fileBuffer.length,
})

const storageKey = generateExecutionFileKey(context, fileName)
const storageKey = generateUniqueExecutionFileKey(context, fileName)
const fileId = generateFileId()

logger.info(`Generated storage key: "${storageKey}" for file: ${fileName}`)
Expand Down
31 changes: 13 additions & 18 deletions apps/sim/lib/uploads/contexts/execution/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
*/
import { describe, expect, it } from 'vitest'
import {
generateExecutionAttachmentKey,
generateExecutionFileKey,
generateLargeValuePayloadKey,
generateUniqueExecutionFileKey,
} from '@/lib/uploads/contexts/execution/utils'

const context = {
Expand All @@ -14,25 +14,20 @@ const context = {
}

describe('execution storage keys', () => {
it('retains deterministic keys for internal execution artifacts', () => {
expect(generateExecutionFileKey(context, 'result.json')).toBe(
'execution/workspace-1/workflow-1/execution-1/result.json'
)
expect(generateExecutionFileKey(context, 'result.json')).toBe(
'execution/workspace-1/workflow-1/execution-1/result.json'
)
it('retains deterministic keys for large-value payloads', () => {
const key = 'execution/workspace-1/workflow-1/execution-1/large-value-lv_abc123.json'

expect(generateLargeValuePayloadKey(context, 'lv_abc123')).toBe(key)
expect(generateLargeValuePayloadKey(context, 'lv_abc123')).toBe(key)
})

it('allocates unique create-only keys for duplicate browser attachment names', () => {
const first = generateExecutionAttachmentKey(context, 'report final.pdf')
const second = generateExecutionAttachmentKey(context, 'report final.pdf')
it('allocates unique keys for duplicate file names, keeping the name as the final segment', () => {
const first = generateUniqueExecutionFileKey(context, 'report final.pdf')
const second = generateUniqueExecutionFileKey(context, 'report final.pdf')
const shape = /^execution\/workspace-1\/workflow-1\/execution-1\/[0-9a-f-]+\/report-final\.pdf$/

expect(first).toMatch(
/^execution\/workspace-1\/workflow-1\/execution-1\/[0-9a-f-]+-report-final\.pdf$/
)
expect(second).toMatch(
/^execution\/workspace-1\/workflow-1\/execution-1\/[0-9a-f-]+-report-final\.pdf$/
)
expect(first).toMatch(shape)
expect(second).toMatch(shape)
expect(first).not.toBe(second)
})
})
38 changes: 25 additions & 13 deletions apps/sim/lib/uploads/contexts/execution/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,28 +13,42 @@ export interface ExecutionContext {
}

/**
* Generate execution-scoped storage key with explicit prefix
* Format: execution/workspace_id/workflow_id/execution_id/filename
* Generate the deterministic storage key for a large-value execution payload.
* Format: execution/workspace_id/workflow_id/execution_id/large-value-<id>.json
*
* Takes the payload id rather than a file name so no user-supplied name can
* reach a key without a uniquifier — that is what silently overwrote same-named
* files before {@link generateUniqueExecutionFileKey} existed. Determinism is
* load-bearing here: the cleanup job matches these keys by LIKE pattern and
* re-storing the same payload must be idempotent.
*/
export function generateExecutionFileKey(context: ExecutionContext, fileName: string): string {
export function generateLargeValuePayloadKey(context: ExecutionContext, id: string): string {
const { workspaceId, workflowId, executionId } = context
const safeFileName = sanitizeFileName(fileName)
const safeFileName = sanitizeFileName(`large-value-${id}.json`)
return `execution/${workspaceId}/${workflowId}/${executionId}/${safeFileName}`
}

/**
* Generates a unique execution-scoped key for browser attachments. Browser
* uploads are create-only, and a single execution may contain multiple files
* with the same display name. Internal execution artifacts intentionally keep
* using {@link generateExecutionFileKey}'s deterministic replacement semantics.
* Generate a collision-free execution-scoped storage key.
* Format: execution/workspace_id/workflow_id/execution_id/unique_id/filename
*
* One execution routinely carries several files sharing a display name (two
* `image.png` screenshots in one Slack message, repeated tool outputs in a
* loop), which the deterministic key would overwrite. The unique id is its own
* path segment rather than a filename prefix so the last segment stays the
* original name — presigned URLs carry no content-disposition, so that segment
* is what a consumer sees.
*
* Large-value payloads, whose ids are already unique, keep using
* {@link generateLargeValuePayloadKey}.
*/
export function generateExecutionAttachmentKey(
export function generateUniqueExecutionFileKey(
context: ExecutionContext,
fileName: string
): string {
const { workspaceId, workflowId, executionId } = context
const safeFileName = sanitizeFileName(fileName)
return `execution/${workspaceId}/${workflowId}/${executionId}/${generateId()}-${safeFileName}`
return `execution/${workspaceId}/${workflowId}/${executionId}/${generateId()}/${safeFileName}`
}

/**
Expand All @@ -45,8 +59,7 @@ export function generateFileId(): string {
}

/**
* Check if a key matches execution file pattern
* Execution files have keys in format: execution/workspaceId/workflowId/executionId/filename
* Execution keys: execution/workspaceId/workflowId/executionId/[uniqueId/]filename
*/
function matchesExecutionFilePattern(key: string): boolean {
if (!key || key.startsWith('/api/') || key.startsWith('http')) {
Expand All @@ -65,7 +78,6 @@ function matchesExecutionFilePattern(key: string): boolean {

/**
* Check if a file is from execution storage based on its key pattern
* Execution files have keys in format: execution/workspaceId/workflowId/executionId/filename
*/
export function isExecutionFile(file: UserFile): boolean {
if (!file.key) {
Expand Down
Loading