Skip to content

fix(execution): give each execution file a unique storage key - #6480

Merged
waleedlatif1 merged 1 commit into
stagingfrom
worktree-execution-file-key-collision
Aug 10, 2026
Merged

fix(execution): give each execution file a unique storage key#6480
waleedlatif1 merged 1 commit into
stagingfrom
worktree-execution-file-key-collision

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Execution file keys were execution/{workspaceId}/{workflowId}/{executionId}/{fileName}, so two files sharing a display name in 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 ingesting 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: presigned URLs carry no content-disposition, so the key's final segment is the name a consumer sees, and a prefix would rename every download
  • Renamed the deterministic generator to generateLargeValuePayloadKey and made it take 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

Compatibility

  • Existing 5-segment keys keep resolving: every parser uses parts.length >= 5 with fixed indices (matchesExecutionFilePattern, extractWorkspaceIdFromExecutionKey, verifyExecutionFileAccess on both its modern and legacy branches, workflowIdFromStorageKey)
  • All three storage providers write a preserved key verbatim; local-disk storage already creates the dirname recursively, and sanitizeFileKey permits the UUID charset in non-leaf segments
  • The original_name unique index on workspace_files is scoped to context = 'workspace', so execution rows now insert per file rather than conflicting
  • Retention deletes by exact key read from the log row, so both files are now cleaned up instead of one
  • No prefix scan or bucket listing exists over the execution/ prefix

Type of Change

  • Bug fix

Testing

Unit tests cover unique key allocation for duplicate names, the filename-as-final-segment invariant, and the unchanged large-value key. Verified the new tests fail against the old key builder. Full suite and type-check pass.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

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.
@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 10, 2026 12:24am

Request Review

@cursor

cursor Bot commented Aug 10, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes storage key layout for new execution files across upload APIs and internal uploads, but existing 5-segment keys remain valid per pattern checks; risk is moderate due to breadth of touch points, not security-critical paths.

Overview
Fixes silent loss of execution files when multiple uploads share the same display name in one run (duplicate image.png, repeated tool outputs, etc.).

Key generation replaces deterministic execution/.../{fileName} keys with generateUniqueExecutionFileKey, which inserts a UUID path segment so the sanitized filename stays the final segment (execution/.../{uuid}/{fileName}). Large-value JSON payloads move to generateLargeValuePayloadKey(context, id) with the same deterministic large-value-{id}.json shape as before.

Call sites now use the unique helper for uploadExecutionFile, presigned execution uploads, and multipart initiate; the large-value store uses the renamed payload key helper.

Tests shift from “same key replacement” behavior to asserting distinct keys and two metadata inserts for duplicate names, plus updated key-shape expectations.

Reviewed by Cursor Bugbot for commit 499162d. Configure here.

@greptile-apps

greptile-apps Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR prevents same-named files within one execution from overwriting each other while preserving deterministic keys for large-value payloads.

  • Adds a generated directory segment to execution file storage keys while retaining the sanitized filename as the final segment.
  • Applies unique key allocation to direct, presigned, and multipart execution uploads.
  • Renames and narrows the deterministic key generator to large-value payload IDs.
  • Adds tests for duplicate filenames and unchanged large-value key behavior.

Confidence Score: 5/5

The PR appears safe to merge, with no actionable regressions identified in execution-file storage, access, serving, or cleanup.

Unique keys are confined to create-only execution-file paths, while deterministic large-value keys remain unchanged and existing consumers accept or preserve the additional path segment.

Important Files Changed

Filename Overview
apps/sim/lib/uploads/contexts/execution/utils.ts Separates unique execution-file key generation from deterministic large-value payload keys without breaking known key parsers.
apps/sim/lib/uploads/contexts/execution/execution-file-manager.ts Allocates a fresh storage key for each directly uploaded execution file, preventing same-name replacement.
apps/sim/lib/execution/payloads/store.ts Uses the payload-specific deterministic generator while preserving the existing large-value storage key format.
apps/sim/app/api/files/presigned/route.ts Gives every presigned execution upload a unique create-only key.
apps/sim/app/api/files/multipart/route.ts Gives every initiated multipart execution upload a unique storage key.

Reviews (1): Last reviewed commit: "fix(execution): give each execution file..." | Re-trigger Greptile

@waleedlatif1
waleedlatif1 merged commit 04fd63d into staging Aug 10, 2026
30 checks passed
@waleedlatif1
waleedlatif1 deleted the worktree-execution-file-key-collision branch August 10, 2026 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant