fix(uploads): gate execution-context uploads behind write/admin permission by waleedlatif1 · Pull Request #5404 · simstudioai/sim · GitHub
Skip to content

fix(uploads): gate execution-context uploads behind write/admin permission#5404

Merged
waleedlatif1 merged 1 commit into
stagingfrom
fix/execution-upload-workspace-permission-v2
Jul 3, 2026
Merged

fix(uploads): gate execution-context uploads behind write/admin permission#5404
waleedlatif1 merged 1 commit into
stagingfrom
fix/execution-upload-workspace-permission-v2

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • The multipart-upload fallback route (/api/files/upload) had no workspace-permission check for execution-context uploads, while the primary presigned-upload route (/api/files/presigned) already requires write/admin for the same upload type.
  • Added the same write/admin gate to the fallback route's execution branch, and now require workspaceId (in addition to workflowId/executionId) before proceeding, matching the presigned route's validation.

Note: self-hosted deployments without cloud storage configured will now require write/admin workspace permission for execution-context uploads via this fallback path, matching the existing requirement on the primary presigned-upload path.

Type of Change

  • Bug fix

Testing

  • Added 5 new tests covering: missing workspaceId, read-only member rejected, no-permission member rejected, write-permission member allowed, admin-permission member allowed.
  • bun run vitest run app/api/files/upload/route.test.ts — 19/19 passing.
  • bun run check:api-validation passes.

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)

Supersedes #5403, whose source branch (worktree-deepsec-fresh) had a polluted commit history (~100 unrelated commits from a bad rebase/merge). Same code, clean single-commit branch off current staging.

@waleedlatif1
fix(uploads): gate execution-context uploads behind write/admin permi…
db30831 
…ssion

Fallback multipart upload route (/api/files/upload) had no workspace
permission check for execution-context uploads, unlike the primary
presigned-upload route which requires write/admin. Mirror that gate
so both paths enforce the same access control.
@waleedlatif1 waleedlatif1 mentioned this pull request Jul 3, 2026
Closed
6 tasks
@vercel

vercel Bot commented Jul 3, 2026

Copy link
Copy Markdown

@cursor

cursor Bot commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown

PR Summary

Medium Risk
Tightens authorization on a file-upload API path; read-only users and missing workspaceId will be blocked where they previously could succeed on the fallback route.

Overview
Aligns the multipart fallback upload route (POST /api/files/upload) with the presigned path for context=execution uploads.

The execution branch now requires workspaceId alongside workflowId and executionId (400 if missing) and checks workspace permission via getUserEntityPermissions—only write or admin may upload; read-only or no access get 403 and never call uploadExecutionFile. workspaceId is no longer passed through as an empty string when omitted.

Tests add mocks for uploadExecutionFile and five cases covering validation, permission denials, and allowed write/admin uploads.

Reviewed by Cursor Bugbot for commit db30831. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR closes a permission gap in the multipart-upload fallback route (/api/files/upload) where execution-context uploads could proceed without any workspace authorization check, while the primary presigned route already required write/admin. The fix mirrors the presigned route's gate exactly.

  • route.ts: Adds workspaceId as a required field for the execution branch and calls getUserEntityPermissions before uploading, returning HTTP 403 for read or no-permission callers.
  • route.test.ts: Adds 5 targeted tests covering the missing-workspaceId 400, read-permission 403, null-permission 403, and write/admin 200 paths — all 19 tests pass.

Confidence Score: 5/5

Safe to merge — the change is a narrow, well-tested addition that closes a real authorization gap without touching any unrelated paths.

The execution-context permission gate is implemented correctly, mirrors the presigned route exactly, and is covered by five dedicated tests exercising every relevant permission state. The getUserEntityPermissions import was already present, no new dependencies are introduced, and the workspaceId || '' fallback that previously bypassed the empty-string guard is cleanly removed.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/api/files/upload/route.ts Adds workspaceId to the execution-context required-parameter check and inserts a getUserEntityPermissions write/admin gate, matching the presigned route. Logic is correct and the import was already present.
apps/sim/app/api/files/upload/route.test.ts Adds 5 well-structured tests covering missing workspaceId (400), read (403), null (403), write (200), and admin (200) paths; mock setup is consistent with the rest of the test file.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Client
    participant UploadRoute as /api/files/upload (fallback)
    participant Auth as getSession
    participant Perms as getUserEntityPermissions
    participant Storage as uploadExecutionFile

    Client->>UploadRoute: "POST multipart/form-data (context=execution)"
    UploadRoute->>Auth: getSession()
    Auth-->>UploadRoute: session (or 401)
    UploadRoute->>UploadRoute: Validate form fields
    UploadRoute->>UploadRoute: Check workflowId + executionId + workspaceId present
    note right of UploadRoute: NEW: workspaceId now required
    UploadRoute->>Perms: getUserEntityPermissions(userId, 'workspace', workspaceId)
    Perms-->>UploadRoute: "'read' | 'write' | 'admin' | null"
    note right of UploadRoute: NEW: write/admin gate added here
    alt "permission == write or admin"
        UploadRoute->>Storage: uploadExecutionFile(ctx, buffer, name, type, userId)
        Storage-->>UploadRoute: userFile
        UploadRoute-->>Client: 200 file metadata
    else "permission == read or null"
        UploadRoute-->>Client: 403 Write or Admin access required
    end
Loading 
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Client
    participant UploadRoute as /api/files/upload (fallback)
    participant Auth as getSession
    participant Perms as getUserEntityPermissions
    participant Storage as uploadExecutionFile

    Client->>UploadRoute: "POST multipart/form-data (context=execution)"
    UploadRoute->>Auth: getSession()
    Auth-->>UploadRoute: session (or 401)
    UploadRoute->>UploadRoute: Validate form fields
    UploadRoute->>UploadRoute: Check workflowId + executionId + workspaceId present
    note right of UploadRoute: NEW: workspaceId now required
    UploadRoute->>Perms: getUserEntityPermissions(userId, 'workspace', workspaceId)
    Perms-->>UploadRoute: "'read' | 'write' | 'admin' | null"
    note right of UploadRoute: NEW: write/admin gate added here
    alt "permission == write or admin"
        UploadRoute->>Storage: uploadExecutionFile(ctx, buffer, name, type, userId)
        Storage-->>UploadRoute: userFile
        UploadRoute-->>Client: 200 file metadata
    else "permission == read or null"
        UploadRoute-->>Client: 403 Write or Admin access required
    end
Loading

Reviews (1): Last reviewed commit: "fix(uploads): gate execution-context upl..." | Re-trigger Greptile

@waleedlatif1 waleedlatif1 merged commit 88330b4 into staging Jul 3, 2026
18 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/execution-upload-workspace-permission-v2 branch July 3, 2026 23:12
@waleedlatif1 waleedlatif1 mentioned this pull request Jul 3, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1