fix(files): require workspace write permission for execution-context uploads

[waleedlatif1]

Summary

• The execution-context branch of /api/files/upload only checked for a valid session — unlike the sibling knowledge-base/workspace/workspace-logos branches in the same file, and unlike the equivalent execution branch in /api/files/presigned, it never verified the caller has write/admin access to the target workspace
• Added the same getUserEntityPermissions write/admin gate already used by every other branch in this route
• workspaceId is now required for execution uploads instead of silently defaulting to an empty string when omitted

Type of Change

• Bug fix

Testing

• Added test coverage: caller without write/admin access is rejected (403) before upload proceeds, caller with write access succeeds, caller with admin access succeeds, request missing workspaceId is rejected (400) instead of defaulting
• bun run check:api-validation passes
• Full route test file passes (18/18)
• Typecheck and biome clean on touched files

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)

[vercel]

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Jul 3, 2026 10:46pm

[cursor]

PR Summary

Medium Risk
Tightens auth on a file upload path that previously allowed any authenticated user with execution IDs; legitimate clients must send workspaceId and hold write/admin access.

Overview
Closes an authorization gap on /api/files/upload for context=execution: uploads now require workspaceId (no empty-string default) and getUserEntityPermissions must be write or admin, matching workspace/knowledge-base branches and presigned execution flows.

403 is returned when the caller only has read access; missing workspaceId yields 400 before any permission or storage call. Tests cover reject/allow paths and hoist a shared createUploadRequest helper with vi.unstubAllGlobals in afterEach.

^{Reviewed by Cursor Bugbot for commit bc4d2d1. Configure here.}

[greptile-apps]

Greptile Summary

This PR closes a missing authorization check on the execution branch of /api/files/upload: previously only a valid session was required, while every other context in the same route required write/admin workspace permissions. It also makes workspaceId required for execution uploads rather than silently defaulting to an empty string.

• route.ts: Adds getUserEntityPermissions write/admin gate and a hard workspaceId presence check to the execution upload path, matching the pattern already used by knowledge-base, workspace, and workspace-logos.
• route.test.ts: Adds a four-case "Execution Context Authorization" suite (missing workspaceId → 400, read-only → 403, write → 200, admin → 200); promotes createUploadRequest to module scope and adds vi.unstubAllGlobals() cleanup.

Confidence Score: 4/5

Safe to merge — the change is a narrowly scoped authorization fix with thorough test coverage and no observable behavior change for callers who already have write/admin access.

The route fix is correct and well-tested. The only notable point is that the permission check is placed inside the per-file loop, meaning multi-file execution uploads incur one extra DB call per file. This is consistent with how knowledge-base and workspace contexts already behave, so it is an inherited pattern rather than a new regression, but worth a tidy-up to keep the auth gate from scaling with batch size.

Both changed files look good. route.ts has the minor loop-placement note on the permission check; route.test.ts is clean.

Important Files Changed

Filename	Overview
apps/sim/app/api/files/upload/route.ts	Adds write/admin permission gate and mandatory workspaceId check to the execution upload branch, matching the pattern used by every other context in the same route. Minor concern: getUserEntityPermissions is called once per file inside the for loop, resulting in repeated DB calls for multi-file uploads, consistent with the pre-existing pattern for knowledge-base and workspace contexts.
apps/sim/app/api/files/upload/route.test.ts	Adds a new 'Execution Context Authorization' test suite covering 4 cases: missing workspaceId → 400, read permission → 403, write access → 200, admin access → 200. Also promotes createUploadRequest to module scope and adds vi.unstubAllGlobals() cleanup to both afterEach blocks.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Client
    participant POST as POST /api/files/upload
    participant Auth as getSession
    participant Perms as getUserEntityPermissions
    participant Upload as uploadExecutionFile

    Client->>POST: "multipart/form-data (file, context=execution, workspaceId, workflowId, executionId)"
    POST->>Auth: getSession()
    Auth-->>POST: session or null

    alt session missing
        POST-->>Client: 401 Unauthorized
    end

    POST->>POST: "parse & validate form fields"

    alt workflowId / executionId / workspaceId missing
        POST-->>Client: 400 InvalidRequestError
    end

    POST->>Perms: getUserEntityPermissions(userId, workspace, workspaceId)
    Perms-->>POST: "read | write | admin | null"

    alt "permission != write AND != admin"
        POST-->>Client: 403 Write or Admin access required
    end

    POST->>Upload: uploadExecutionFile(workspaceId, workflowId, executionId, buffer)
    Upload-->>POST: userFile result
    POST-->>Client: 200 upload result

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 POST as POST /api/files/upload
    participant Auth as getSession
    participant Perms as getUserEntityPermissions
    participant Upload as uploadExecutionFile

    Client->>POST: "multipart/form-data (file, context=execution, workspaceId, workflowId, executionId)"
    POST->>Auth: getSession()
    Auth-->>POST: session or null

    alt session missing
        POST-->>Client: 401 Unauthorized
    end

    POST->>POST: "parse & validate form fields"

    alt workflowId / executionId / workspaceId missing
        POST-->>Client: 400 InvalidRequestError
    end

    POST->>Perms: getUserEntityPermissions(userId, workspace, workspaceId)
    Perms-->>POST: "read | write | admin | null"

    alt "permission != write AND != admin"
        POST-->>Client: 403 Write or Admin access required
    end

    POST->>Upload: uploadExecutionFile(workspaceId, workflowId, executionId, buffer)
    Upload-->>POST: userFile result
    POST-->>Client: 200 upload result

Loading

Comments Outside Diff (1)

1. apps/sim/app/api/files/upload/route.ts, line 95-126 (link)

   Permission check runs once per file inside the loop. For execution uploads with multiple files, getUserEntityPermissions makes a separate DB round-trip for every file in the batch, even though the workspaceId and userId are invariant across the loop. Hoisting the check above the loop would match the semantics while eliminating the redundant calls.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

_{Reviews (1): Last reviewed commit: "fix(files): require workspace write perm..." | Re-trigger Greptile}

[waleedlatif1]

Superseded by #5403 — same fix, that PR has the more complete test coverage and an added note about the self-hosted/no-cloud-storage deployment behavior change. Closing this duplicate to avoid two open PRs for the same change.