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

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)

…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

vercelBot commented Jul 5, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
docsSkippedSkippedJul 5, 2026 5:37pm

Request Review

@cursor

cursorBot commented Jul 5, 2026

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

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

FilenameOverview
apps/sim/app/api/custom-blocks/route.tsPOST 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 merged commit 732e4ec into stagingJul 5, 2026
18 checks passed
@waleedlatif1
waleedlatif1 deleted the custom-blocks-admin-check-dedup branch July 5, 2026 17:59
Sign up for freeto 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

@waleedlatif1