Skip to content

fix(security): enforce write permission on target workspace in folder duplicate#3478

Open
MaxwellCalkin wants to merge 1 commit intosimstudioai:mainfrom
MaxwellCalkin:fix/folder-duplicate-target-workspace-auth
Open

fix(security): enforce write permission on target workspace in folder duplicate#3478
MaxwellCalkin wants to merge 1 commit intosimstudioai:mainfrom
MaxwellCalkin:fix/folder-duplicate-target-workspace-auth

Conversation

@MaxwellCalkin
Copy link

Summary

The POST /api/folders/[id]/duplicate endpoint only verified the user's permissions on the source workspace. When a different workspaceId was supplied in the request body, the entire folder tree (and its nested sub-folders) was created in the target workspace without checking that the user had write access.

This is an authorization bypass: any authenticated user who can read a folder in workspace A could duplicate its entire folder structure into workspace B, even if they have no access to workspace B at all.

The companion duplicateWorkflow() helper (lib/workflows/persistence/duplicate.ts, lines 131-139) already enforces a target-workspace permission check, so the individual workflow copies inside the duplicated tree would fail. But the folder structure itself was created unconditionally.

Changes

  • apps/sim/app/api/folders/[id]/duplicate/route.ts — When targetWorkspaceId differs from the source folder's workspace, call getUserEntityPermissions on the target and reject with 403 if the user lacks write or admin access.
  • apps/sim/app/api/folders/[id]/duplicate/route.test.ts — New test file covering: unauthenticated rejection, read-only source access rejection, cross-workspace authorization enforcement, and same-workspace skip of the extra check.

Test plan

  • Verify existing folder duplicate (same workspace) still works
  • Verify cross-workspace duplicate with write/admin access succeeds
  • Verify cross-workspace duplicate with read-only or no access returns 403
  • Run route.test.ts in CI

AI Disclosure: This PR was authored by an AI (Claude, Anthropic). See https://maxcalkin.com/ai for more information.

🤖 Generated with Claude Code

… duplicate

The folder duplicate endpoint only checked permissions on the source
workspace. When a different workspaceId was provided in the request body,
folders (and their entire nested structure) were created in the target
workspace without verifying the user had write access to it.

The workflow duplicate helper (lib/workflows/persistence/duplicate.ts)
already enforces this check, but the folder duplicate route bypassed it
for folder creation.

Add a permission check on the target workspace when it differs from the
source, requiring write or admin access before proceeding. Add tests for
cross-workspace authorization scenarios.

> **AI Disclosure:** This PR was authored by an AI (Claude, Anthropic).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@cursor
Copy link

cursor bot commented Mar 9, 2026

PR Summary

Medium Risk
Touches authorization logic for cross-workspace folder duplication, which is security-sensitive and could inadvertently block valid duplicates if permission checks are incorrect.

Overview
Closes an authorization gap in POST /api/folders/[id]/duplicate by validating write/admin permission on the target workspace when workspaceId differs from the source folder’s workspace, returning 403 with a clear error message when denied.

Adds a new Vitest suite for the route covering unauthenticated/insufficient source access, cross-workspace denial (read/no access), and ensuring the extra permission check is skipped for same-workspace duplication.

Written by Cursor Bugbot for commit 27684e0. This will update automatically on new commits. Configure here.

@vercel
Copy link

vercel bot commented Mar 9, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Mar 9, 2026 2:31am

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 9, 2026

Greptile Summary

This PR closes an authorization bypass where POST /api/folders/[id]/duplicate checked write permissions only on the source workspace while allowing callers to redirect the entire duplicated folder tree into an arbitrary workspaceId without any permission check on the target. The fix adds a single, well-placed guard: when targetWorkspaceId differs from sourceFolder.workspaceId, it calls getUserEntityPermissions on the target workspace and rejects with 403 unless the user holds write or admin access — consistent with the pattern already used in duplicateWorkflow().

Strengths:

  • The security fix in route.ts is logically correct and the permission check is in the right place (after source-permission is confirmed but before the DB transaction begins).
  • The four denial-case tests (unauthenticated, read-only source, read-only target, no target access) are well-structured and verify the authorization logic.

Concern:

  • The two success-path tests (lines 199–216 and 218–236) have a mock defect: minSelectMock returns [{minSortOrder: 0}] for all tx.select() calls, including the recursive child-folder query in duplicateFolderStructure. This causes infinite recursion (eventually a stack overflow) that is silently caught and returns 500. Neither test asserts on the response status, so they pass despite the request failing, reducing confidence in the happy-path coverage.

Confidence Score: 4/5

  • The security fix is correct and safe to merge; the only concern is insufficient test coverage for the happy path due to mock infrastructure issues.
  • The production code change is minimal, well-targeted, and correctly implements the authorization check for cross-workspace duplication. The four denial-case tests are solid and validate the security guard effectively. However, the two success-path tests contain a mock defect (minSelectMock returns [{minSortOrder: 0}] for all queries, including child-folder queries) that causes stack overflow inside the transaction. Because neither test asserts on the response status, they pass silently despite the endpoint returning 500, meaning the happy-path scenario is not actually validated. This reduces confidence in regression coverage for successful duplication.
  • apps/sim/app/api/folders/[id]/duplicate/route.test.ts — the success-path tests need response status assertions and a corrected tx.select mock that returns an empty array for child-folder queries.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Route as duplicate route
    participant Perms as getUserEntityPermissions
    participant DB as Database

    Client->>Route: POST body with name and optional workspaceId
    Route->>Route: getSession — check authentication
    alt Unauthenticated
        Route-->>Client: 401
    end
    Route->>DB: look up source folder by id
    alt Folder missing
        Route-->>Client: 404
    end
    Route->>Perms: check source workspace access
    alt Read-only or no access on source
        Route-->>Client: 403 Access denied
    end
    Note over Route: resolve targetWorkspaceId
    alt targetWorkspaceId differs from source workspaceId
        Route->>Perms: check target workspace access (NEW)
        alt Read-only or no access on target
            Route-->>Client: 403 Write or admin access required
        end
    end
    Route->>DB: transaction — insert root folder then child folders recursively
    DB-->>Route: newFolderId and folderMapping
    Route->>DB: duplicate workflows for each folder in mapping
    DB-->>Route: workflow stats
    Route-->>Client: 201 with duplicated folder details
Loading

Last reviewed commit: 27684e0

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