Skip to content

Commit 499162d

Browse files
committed
fix(execution): give each execution file a unique storage key
Execution file keys were built as execution/{workspaceId}/{workflowId}/{executionId}/{fileName}, so two files sharing a display name within one execution resolved to the same key and URL. The second upload overwrote the first in object storage and updated its workspace_files row instead of inserting, silently losing a file. Any trigger that ingests multiple attachments hits this — repeated screenshot names, mail clients that reuse inline-image names, or a loop emitting the same output name. generateUniqueExecutionFileKey now allocates a unique directory segment (.../{executionId}/{uuid}/{fileName}) and uploadExecutionFile uses it, so every execution file gets its own key. The uniquifier is its own path segment rather than a filename prefix because presigned URLs carry no content-disposition: the key's final segment is the name a consumer sees, and a prefix would rename every download. The deterministic generator is renamed to generateLargeValuePayloadKey and takes the payload id instead of a free-form file name, so no user-supplied name can reach a key without a uniquifier. Its output is unchanged — determinism is load-bearing there, since the cleanup job matches those keys by LIKE pattern and the trace store recovers workflowId by segment position. Every reader tolerates the extra segment: key parsers use parts.length >= 5 with fixed indices, storage providers write a preserved key verbatim, and local-disk storage already creates the dirname recursively.
1 parent 4475686 commit 499162d

9 files changed

Lines changed: 76 additions & 93 deletions

File tree

apps/sim/app/api/files/multipart/route.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,19 +52,19 @@ vi.mock('@/lib/uploads/providers/blob/client', () => ({
5252
}))
5353

5454
vi.mock('@/lib/uploads/contexts/execution/utils', () => ({
55-
generateExecutionAttachmentKey: mockGenerateExecutionAttachmentKey,
55+
generateUniqueExecutionFileKey: mockGenerateUniqueExecutionFileKey,
5656
}))
5757

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

6060
const {
6161
mockCheckStorageQuota,
62-
mockGenerateExecutionAttachmentKey,
62+
mockGenerateUniqueExecutionFileKey,
6363
mockInitiateS3MultipartUpload,
6464
mockResolveStorageBillingContext,
6565
} = vi.hoisted(() => ({
6666
mockCheckStorageQuota: vi.fn(),
67-
mockGenerateExecutionAttachmentKey: vi.fn(),
67+
mockGenerateUniqueExecutionFileKey: vi.fn(),
6868
mockInitiateS3MultipartUpload: vi.fn(),
6969
mockResolveStorageBillingContext: vi.fn(),
7070
}))
@@ -250,7 +250,7 @@ describe('POST /api/files/multipart action=initiate quota enforcement', () => {
250250
mockResolveStorageBillingContext.mockResolvedValue(STORAGE_CONTEXT)
251251
mockCheckStorageQuota.mockResolvedValue({ allowed: true })
252252
mockInitiateS3MultipartUpload.mockResolvedValue({ uploadId: 'up-1', key: 'k/file.bin' })
253-
mockGenerateExecutionAttachmentKey.mockImplementation(
253+
mockGenerateUniqueExecutionFileKey.mockImplementation(
254254
(
255255
context: { workspaceId: string; workflowId: string; executionId: string },
256256
fileName: string
@@ -311,7 +311,7 @@ describe('POST /api/files/multipart action=initiate quota enforcement', () => {
311311
})
312312

313313
it('allocates distinct multipart keys for duplicate execution attachment names', async () => {
314-
mockGenerateExecutionAttachmentKey
314+
mockGenerateUniqueExecutionFileKey
315315
.mockReturnValueOnce('execution/ws-1/wf-1/exec-1/one-output.bin')
316316
.mockReturnValueOnce('execution/ws-1/wf-1/exec-1/two-output.bin')
317317
mockInitiateS3MultipartUpload.mockImplementation(async ({ customKey }) => ({

apps/sim/app/api/files/multipart/route.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,10 +215,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
215215
{ status: 400 }
216216
)
217217
}
218-
const { generateExecutionAttachmentKey } = await import(
218+
const { generateUniqueExecutionFileKey } = await import(
219219
'@/lib/uploads/contexts/execution/utils'
220220
)
221-
customKey = generateExecutionAttachmentKey(
221+
customKey = generateUniqueExecutionFileKey(
222222
{ workspaceId, workflowId, executionId },
223223
fileName
224224
)

apps/sim/app/api/files/presigned/route.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ const {
2424
mockIsUsingCloudStorageUploads,
2525
mockGetUserEntityPermissions,
2626
mockGenerateWorkspaceFileKey,
27-
mockGenerateExecutionAttachmentKey,
27+
mockGenerateUniqueExecutionFileKey,
2828
mockInsertFileMetadata,
2929
mockCheckStorageQuotaForBillingContext,
3030
mockDecrementStorageUsageForBillingContext,
@@ -52,7 +52,7 @@ const {
5252
mockGenerateWorkspaceFileKey: vi.fn(
5353
(workspaceId: string, fileName: string) => `workspace/${workspaceId}/${fileName}`
5454
),
55-
mockGenerateExecutionAttachmentKey: vi.fn(
55+
mockGenerateUniqueExecutionFileKey: vi.fn(
5656
(ctx: { workspaceId: string; workflowId: string; executionId: string }, fileName: string) =>
5757
`execution/${ctx.workspaceId}/${ctx.workflowId}/${ctx.executionId}/attachment-${fileName}`
5858
),
@@ -110,7 +110,7 @@ vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({
110110
}))
111111

112112
vi.mock('@/lib/uploads/contexts/execution/utils', () => ({
113-
generateExecutionAttachmentKey: mockGenerateExecutionAttachmentKey,
113+
generateUniqueExecutionFileKey: mockGenerateUniqueExecutionFileKey,
114114
}))
115115

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

apps/sim/app/api/files/presigned/route.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { getSession } from '@/lib/auth'
1111
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1212
import { CopilotFiles } from '@/lib/uploads'
1313
import { getServeStoragePrefix } from '@/lib/uploads/config'
14-
import { generateExecutionAttachmentKey } from '@/lib/uploads/contexts/execution/utils'
14+
import { generateUniqueExecutionFileKey } from '@/lib/uploads/contexts/execution/utils'
1515
import { generateKnowledgeBaseFileKey } from '@/lib/uploads/contexts/knowledge-base/knowledge-base-file-manager'
1616
import { generateWorkspaceFileKey } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
1717
import { generatePresignedUploadUrl, hasCloudStorage } from '@/lib/uploads/core/storage-service'
@@ -222,7 +222,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
222222
throw new ValidationError(fileValidationError.message)
223223
}
224224

225-
const customKey = generateExecutionAttachmentKey(
225+
const customKey = generateUniqueExecutionFileKey(
226226
{ workspaceId, workflowId, executionId },
227227
fileName
228228
)

apps/sim/lib/execution/payloads/store.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
isValidLargeValueKey,
1717
readLargeValueRefFromStorage,
1818
} from '@/lib/execution/payloads/materialization.server'
19-
import { generateExecutionFileKey } from '@/lib/uploads/contexts/execution/utils'
19+
import { generateLargeValuePayloadKey } from '@/lib/uploads/contexts/execution/utils'
2020

2121
const logger = createLogger('LargeExecutionPayloadStore')
2222

@@ -75,10 +75,7 @@ async function persistValue(
7575
return undefined
7676
}
7777

78-
const key = generateExecutionFileKey(
79-
{ workspaceId, workflowId, executionId },
80-
`large-value-${id}.json`
81-
)
78+
const key = generateLargeValuePayloadKey({ workspaceId, workflowId, executionId }, id)
8279

8380
try {
8481
const { StorageService } = await import('@/lib/uploads')

apps/sim/lib/uploads/contexts/execution/execution-file-manager.test.ts

Lines changed: 18 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,13 @@ vi.mock('@/lib/uploads/providers/s3/client', () => ({
2323

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

26-
describe('uploadExecutionFile replacement compatibility', () => {
26+
const context = {
27+
workspaceId: 'workspace-1',
28+
workflowId: 'workflow-1',
29+
executionId: 'execution-1',
30+
}
31+
32+
describe('uploadExecutionFile key allocation', () => {
2733
beforeEach(() => {
2834
vi.clearAllMocks()
2935
resetDbChainMock()
@@ -35,57 +41,27 @@ describe('uploadExecutionFile replacement compatibility', () => {
3541
type: contentType,
3642
}))
3743
mockGetPresignedUrlWithConfig.mockResolvedValue('https://example.com/download')
44+
dbChainMockFns.limit.mockResolvedValue([])
45+
dbChainMockFns.returning.mockResolvedValue([{ id: 'file-1' }])
3846
})
3947

40-
it('allows changed bytes and content type at the same execution-scoped key', async () => {
41-
const context = {
42-
workspaceId: 'workspace-1',
43-
workflowId: 'workflow-1',
44-
executionId: 'execution-1',
45-
}
46-
const key = 'execution/workspace-1/workflow-1/execution-1/result.txt'
47-
const existingMetadata = {
48-
id: 'file-1',
49-
key,
50-
userId: 'user-1',
51-
workspaceId: 'workspace-1',
52-
folderId: null,
53-
context: 'execution',
54-
originalName: key,
55-
displayName: key,
56-
contentType: 'text/plain',
57-
size: 3,
58-
deletedAt: null,
59-
}
60-
61-
dbChainMockFns.limit
62-
.mockResolvedValueOnce([])
63-
.mockResolvedValueOnce([])
64-
.mockResolvedValueOnce([existingMetadata])
65-
dbChainMockFns.returning.mockResolvedValueOnce([existingMetadata])
66-
48+
it('gives same-named files in one execution distinct keys', async () => {
6749
const first = await uploadExecutionFile(
6850
context,
69-
Buffer.from('old'),
70-
'result.txt',
71-
'text/plain',
51+
Buffer.alloc(13575),
52+
'image.png',
53+
'image/png',
7254
'user-1'
7355
)
7456
const second = await uploadExecutionFile(
7557
context,
76-
Buffer.from('{"new":true}'),
77-
'result.txt',
78-
'application/json',
58+
Buffer.alloc(37226),
59+
'image.png',
60+
'image/png',
7961
'user-1'
8062
)
8163

82-
expect(first.key).toBe(key)
83-
expect(second).toMatchObject({
84-
key,
85-
size: 12,
86-
type: 'application/json',
87-
})
88-
expect(mockUploadToS3).toHaveBeenCalledTimes(2)
89-
expect(dbChainMockFns.insert).toHaveBeenCalledTimes(1)
64+
expect(first.key).not.toBe(second.key)
65+
expect(dbChainMockFns.insert).toHaveBeenCalledTimes(2)
9066
})
9167
})

apps/sim/lib/uploads/contexts/execution/execution-file-manager.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ import { getErrorMessage } from '@sim/utils/errors'
33
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
44
import { isUserFileWithMetadata } from '@/lib/core/utils/user-file'
55
import type { ExecutionContext } from '@/lib/uploads/contexts/execution/utils'
6-
import { generateExecutionFileKey, generateFileId } from '@/lib/uploads/contexts/execution/utils'
6+
import {
7+
generateFileId,
8+
generateUniqueExecutionFileKey,
9+
} from '@/lib/uploads/contexts/execution/utils'
710
import type { UserFile } from '@/executor/types'
811

912
const logger = createLogger('ExecutionFileStorage')
@@ -78,7 +81,7 @@ export async function uploadExecutionFile(
7881
bufferSize: fileBuffer.length,
7982
})
8083

81-
const storageKey = generateExecutionFileKey(context, fileName)
84+
const storageKey = generateUniqueExecutionFileKey(context, fileName)
8285
const fileId = generateFileId()
8386

8487
logger.info(`Generated storage key: "${storageKey}" for file: ${fileName}`)

apps/sim/lib/uploads/contexts/execution/utils.test.ts

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
*/
44
import { describe, expect, it } from 'vitest'
55
import {
6-
generateExecutionAttachmentKey,
7-
generateExecutionFileKey,
6+
generateLargeValuePayloadKey,
7+
generateUniqueExecutionFileKey,
88
} from '@/lib/uploads/contexts/execution/utils'
99

1010
const context = {
@@ -14,25 +14,20 @@ const context = {
1414
}
1515

1616
describe('execution storage keys', () => {
17-
it('retains deterministic keys for internal execution artifacts', () => {
18-
expect(generateExecutionFileKey(context, 'result.json')).toBe(
19-
'execution/workspace-1/workflow-1/execution-1/result.json'
20-
)
21-
expect(generateExecutionFileKey(context, 'result.json')).toBe(
22-
'execution/workspace-1/workflow-1/execution-1/result.json'
23-
)
17+
it('retains deterministic keys for large-value payloads', () => {
18+
const key = 'execution/workspace-1/workflow-1/execution-1/large-value-lv_abc123.json'
19+
20+
expect(generateLargeValuePayloadKey(context, 'lv_abc123')).toBe(key)
21+
expect(generateLargeValuePayloadKey(context, 'lv_abc123')).toBe(key)
2422
})
2523

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

30-
expect(first).toMatch(
31-
/^execution\/workspace-1\/workflow-1\/execution-1\/[0-9a-f-]+-report-final\.pdf$/
32-
)
33-
expect(second).toMatch(
34-
/^execution\/workspace-1\/workflow-1\/execution-1\/[0-9a-f-]+-report-final\.pdf$/
35-
)
29+
expect(first).toMatch(shape)
30+
expect(second).toMatch(shape)
3631
expect(first).not.toBe(second)
3732
})
3833
})

apps/sim/lib/uploads/contexts/execution/utils.ts

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,28 +13,42 @@ export interface ExecutionContext {
1313
}
1414

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

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

4054
/**
@@ -45,8 +59,7 @@ export function generateFileId(): string {
4559
}
4660

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

6679
/**
6780
* Check if a file is from execution storage based on its key pattern
68-
* Execution files have keys in format: execution/workspaceId/workflowId/executionId/filename
6981
*/
7082
export function isExecutionFile(file: UserFile): boolean {
7183
if (!file.key) {

0 commit comments

Comments
 (0)