mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(execution): give each execution file a unique storage key (#6480)
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.
This commit is contained in:
@@ -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(),
|
||||
}))
|
||||
@@ -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
|
||||
@@ -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 }) => ({
|
||||
|
||||
@@ -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
|
||||
)
|
||||
|
||||
@@ -24,7 +24,7 @@ const {
|
||||
mockIsUsingCloudStorageUploads,
|
||||
mockGetUserEntityPermissions,
|
||||
mockGenerateWorkspaceFileKey,
|
||||
mockGenerateExecutionAttachmentKey,
|
||||
mockGenerateUniqueExecutionFileKey,
|
||||
mockInsertFileMetadata,
|
||||
mockCheckStorageQuotaForBillingContext,
|
||||
mockDecrementStorageUsageForBillingContext,
|
||||
@@ -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}`
|
||||
),
|
||||
@@ -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', () => ({
|
||||
@@ -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')
|
||||
|
||||
|
||||
@@ -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'
|
||||
@@ -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
|
||||
)
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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()
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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')
|
||||
@@ -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}`)
|
||||
|
||||
@@ -3,8 +3,8 @@
|
||||
*/
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
generateExecutionAttachmentKey,
|
||||
generateExecutionFileKey,
|
||||
generateLargeValuePayloadKey,
|
||||
generateUniqueExecutionFileKey,
|
||||
} from '@/lib/uploads/contexts/execution/utils'
|
||||
|
||||
const context = {
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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}`
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -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')) {
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user