Skip to content

fix(custom-blocks): dedupe redundant workspace lookup in POST admin check#5429

Merged
waleedlatif1 merged 1 commit into
stagingfrom
custom-blocks-admin-check-dedup
Jul 5, 2026
Merged

fix(custom-blocks): dedupe redundant workspace lookup in POST admin check#5429
waleedlatif1 merged 1 commit into
stagingfrom
custom-blocks-admin-check-dedup

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Found while auditing for the same pattern fixed in fix(sidebar): prefetch folders and workspace permissions on cold load #5426: the POST /api/custom-blocks handler called hasWorkspaceAdminAccess(userId, workspaceId) then separately called getWorkspaceWithOwner(workspaceId) again just to read organizationId — two independent DB fetches of the same workspace row for the same request
  • Consolidated into a single checkWorkspaceAccess call, matching the exact pattern the GET handler in this same file already uses a few lines above
  • access.canAdmin is logically identical to hasWorkspaceAdminAccess()'s result — verified against PERMISSION_RANK/permissionSatisfies in @sim/platform-authz/workspace: admin is the top rank (3), so permissionSatisfies(p, 'admin') can only be true when p === 'admin', same as the old check
  • Same error responses, same status codes, same order of checks (admin check still runs before the organization-membership check) — zero behavior change, one fewer DB round-trip per publish request

Type of Change

  • Refactor / minor perf

Testing

  • bunx tsc --noEmit clean
  • bunx biome check clean
  • bun run check:react-query passes
  • No existing test file for this route (none broken); logic equivalence verified analytically against the permission-rank system

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)

@waleedlatif1
fix(custom-blocks): dedupe redundant workspace lookup in POST admin c…
8a3e4ea 
…heck

hasWorkspaceAdminAccess + a separate getWorkspaceWithOwner call each
independently re-fetched the workspace row for the same (userId,
workspaceId) pair. Consolidated into a single checkWorkspaceAccess
call, matching the pattern the GET handler in this same file already
uses. access.canAdmin is logically identical to hasWorkspaceAdminAccess's
result (admin is the top PERMISSION_RANK, nothing else satisfies it) —
no behavior change, one fewer DB round-trip per publish request.
@vercel

vercel Bot commented Jul 5, 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 Jul 5, 2026 5:37pm

Request Review

@cursor

cursor Bot commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown

PR Summary

Low Risk
Refactor-only permission and org lookup consolidation on a single API route with equivalent admin checks and unchanged error handling.

Overview
The POST handler for publishing custom blocks no longer calls hasWorkspaceAdminAccess and then getWorkspaceWithOwner separately. It now uses a single checkWorkspaceAccess call—the same pattern as GET in this route—so admin authorization and organizationId come from one workspace fetch.

Authorization still gates on admin (access.canAdmin), and responses/status codes for missing org membership are unchanged; the change is one fewer DB round-trip per publish with no intended behavior change.

Reviewed by Cursor Bugbot for commit 8a3e4ea. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR eliminates a redundant database round-trip in the POST /api/custom-blocks handler by consolidating two separate workspace lookups — hasWorkspaceAdminAccess followed by getWorkspaceWithOwner — into a single checkWorkspaceAccess call, exactly mirroring the pattern the GET handler in the same file already uses.

  • Behavioral equivalence confirmed: hasWorkspaceAdminAccess internally checks getEffectiveWorkspacePermission(...) === 'admin'; checkWorkspaceAccess sets canAdmin = permissionSatisfies(permission, 'admin'), which evaluates to PERMISSION_RANK[have] >= 3. Since admin is rank 3 and is the top of the hierarchy, the only value that satisfies it is admin itself — the two expressions are identical in all reachable cases.
  • Error handling unchanged: workspace-not-found still yields 403 (from !access.canAdmin), insufficient-permission yields 403, missing organizationId yields 400 — same status codes in the same order as before.

Confidence Score: 5/5

Safe to merge — the refactor removes one DB round-trip with no change in auth logic, error codes, or response shape.

The permission check is logically identical: admin sits at rank 3 (the top of PERMISSION_RANK) so permissionSatisfies(p, 'admin') can only be true when p === 'admin', matching the old strict equality. All error paths and status codes are preserved in the same order.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/api/custom-blocks/route.ts POST handler consolidated to use checkWorkspaceAccess (matching GET handler), removing a redundant getWorkspaceWithOwner call; equivalence of canAdmin vs the old strict-equality check on 'admin' is confirmed by the PERMISSION_RANK definition.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Client
    participant POST as POST /api/custom-blocks
    participant CWA as checkWorkspaceAccess
    participant DB as Database

    Note over POST,DB: Before (2 DB fetches)
    Client->>POST: publish request
    POST->>DB: hasWorkspaceAdminAccess → getWorkspaceWithOwner
    DB-->>POST: "workspace row #1"
    POST->>DB: getWorkspaceWithOwner (again for organizationId)
    DB-->>POST: "workspace row #2"
    POST-->>Client: response

    Note over POST,DB: After (1 DB fetch)
    Client->>POST: publish request
    POST->>CWA: checkWorkspaceAccess(workspaceId, userId)
    CWA->>DB: getWorkspaceWithOwner (once)
    DB-->>CWA: workspace row
    CWA->>DB: resolveEffectiveWorkspacePermission
    DB-->>CWA: permission
    CWA-->>POST: access.canAdmin + access.workspace.organizationId
    POST-->>Client: response
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/custom-blocks
    participant CWA as checkWorkspaceAccess
    participant DB as Database

    Note over POST,DB: Before (2 DB fetches)
    Client->>POST: publish request
    POST->>DB: hasWorkspaceAdminAccess → getWorkspaceWithOwner
    DB-->>POST: "workspace row #1"
    POST->>DB: getWorkspaceWithOwner (again for organizationId)
    DB-->>POST: "workspace row #2"
    POST-->>Client: response

    Note over POST,DB: After (1 DB fetch)
    Client->>POST: publish request
    POST->>CWA: checkWorkspaceAccess(workspaceId, userId)
    CWA->>DB: getWorkspaceWithOwner (once)
    DB-->>CWA: workspace row
    CWA->>DB: resolveEffectiveWorkspacePermission
    DB-->>CWA: permission
    CWA-->>POST: access.canAdmin + access.workspace.organizationId
    POST-->>Client: response
Loading

Reviews (1): Last reviewed commit: "fix(custom-blocks): dedupe redundant wor..." | Re-trigger Greptile

@waleedlatif1 waleedlatif1 mentioned this pull request Jul 5, 2026
Merged
6 tasks
@waleedlatif1 waleedlatif1 merged commit 732e4ec into staging Jul 5, 2026
18 checks passed
@waleedlatif1 waleedlatif1 deleted the custom-blocks-admin-check-dedup branch July 5, 2026 17:59
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