refactor(permissions): make hasWorkspaceAdminAccess and getUserEntityPermissions thin wrappers of checkWorkspaceAccess by waleedlatif1 · Pull Request #5430 · simstudioai/sim

waleedlatif1 · 2026-07-05T17:45:05Z

Summary

Follow-up to fix(custom-blocks): dedupe redundant workspace lookup in POST admin check #5429 (custom-blocks dedup fix): the root cause of that bug was that hasWorkspaceAdminAccess and getUserEntityPermissions('workspace', ...) each independently re-implement the same getWorkspaceWithOwner + getEffectiveWorkspacePermission fetch that checkWorkspaceAccess already does. Any call site that uses two of these three functions for the same (userId, workspaceId) pair silently pays for redundant DB round-trips, and worse, the three implementations could drift apart over time
Consolidated: hasWorkspaceAdminAccess and getUserEntityPermissions's workspace branch are now thin wrappers around checkWorkspaceAccess — matching the pattern assertActiveWorkspaceAccess already used in this same file
Added permission: PermissionType | null to the WorkspaceAccess return shape so getUserEntityPermissions doesn't need its own second getEffectiveWorkspacePermission call to get the raw value
Zero behavior change: canAdmin is exactly permission === 'admin' (verified against PERMISSION_RANK — admin is the top rank, nothing else satisfies it)

Type of Change

Refactor / code quality

Testing

bunx tsc --noEmit clean, bunx biome check clean
616 existing tests across lib/workspaces, lib/credentials, lib/invitations, lib/copilot/vfs, and the affected API routes pass unmodified
2 test assertions updated for the new additive permission field (exact-shape .toEqual() checks)
Ran an independent security-focused review specifically checking argument-order correctness (checkWorkspaceAccess(workspaceId, userId) vs the wrapper functions' (userId, workspaceId) order) and privilege-escalation risk in the canAdmin delegation — no issues found

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)

…Permissions thin wrappers of checkWorkspaceAccess hasWorkspaceAdminAccess and getUserEntityPermissions('workspace', ...) each independently re-implemented the same getWorkspaceWithOwner + getEffectiveWorkspacePermission fetch that checkWorkspaceAccess already does — this is exactly the duplication that let the custom-blocks POST handler drift into two separate DB round-trips for one permission resolution (fixed separately). Centralizing all three onto one implementation means a future logic change or bug fix only has to happen once, and any future caller that (accidentally) calls two of these functions together is now at least drawing from one consistent definition of "effective permission" instead of two that could diverge. Adds a `permission: PermissionType | null` field to the WorkspaceAccess return shape so getUserEntityPermissions doesn't need a second independent getEffectiveWorkspacePermission call to get the raw value. No behavior change: verified analytically (canAdmin is exactly permission === 'admin' per PERMISSION_RANK) and confirmed by an independent security review focused on argument-order and privilege-escalation risks in this kind of refactor. 616 existing tests across lib/workspaces, lib/credentials, lib/invitations, lib/copilot/vfs, and the affected API routes pass unmodified (plus 2 test assertions updated for the new additive field).

vercel · 2026-07-05T17:45:11Z

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

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Jul 5, 2026 5:45pm

cursor · 2026-07-05T17:45:15Z

PR Summary

Low Risk
Refactor-only with intended parity; touches authorization helpers but does not change the underlying permission rules.

Overview
Workspace permission helpers now share one code path instead of repeating workspace load + effective permission resolution.

checkWorkspaceAccess returns a new permission field (PermissionType | null) alongside the existing flags. getUserEntityPermissions for entityType === 'workspace' reads that field via checkWorkspaceAccess, and hasWorkspaceAdminAccess delegates to canAdmin on the same result—matching assertActiveWorkspaceAccess’s pattern.

Tests that assert the full checkWorkspaceAccess shape were updated for the additive permission field.

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

greptile-apps · 2026-07-05T17:47:19Z

Greptile Summary

This PR consolidates three independent implementations of getWorkspaceWithOwner + getEffectiveWorkspacePermission into a single authoritative path through checkWorkspaceAccess, eliminating redundant DB round-trips and preventing future drift. A new permission: PermissionType | null field is added to WorkspaceAccess so callers get the raw permission value without a second fetch.

hasWorkspaceAdminAccess and getUserEntityPermissions's workspace branch are now thin wrappers around checkWorkspaceAccess, matching the pattern assertActiveWorkspaceAccess already used in the same file.
Two test assertions updated for the new additive permission field; the rest of the 616-test suite passes unmodified.

Confidence Score: 5/5

Safe to merge — the change is a pure consolidation with no behavior difference at any call site.

Both wrappers delegate to checkWorkspaceAccess with correct argument order (workspaceId, userId), canAdmin is semantically identical to the previous permission === 'admin' comparison because admin is the highest rank in PERMISSION_RANK, and the permission field addition to WorkspaceAccess is additive with no existing manual constructors of the type outside of checkWorkspaceAccess itself. TypeScript reports no errors and the full test suite passes.

No files require special attention.

Important Files Changed

Filename	Overview
apps/sim/lib/workspaces/permissions/utils.ts	Adds `permission: PermissionType
apps/sim/lib/workspaces/permissions/utils.test.ts	Updates two exact-shape `toEqual` assertions for `checkWorkspaceAccess` to include the new `permission` field; no test logic removed or weakened.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant hasWorkspaceAdminAccess
    participant getUserEntityPermissions
    participant checkWorkspaceAccess
    participant getWorkspaceWithOwner
    participant getEffectiveWorkspacePermission

    note over hasWorkspaceAdminAccess,getUserEntityPermissions: Before: each called DB directly
    note over hasWorkspaceAdminAccess,getUserEntityPermissions: After: thin wrappers

    Caller->>hasWorkspaceAdminAccess: (userId, workspaceId)
    hasWorkspaceAdminAccess->>checkWorkspaceAccess: (workspaceId, userId)
    checkWorkspaceAccess->>getWorkspaceWithOwner: (workspaceId)
    getWorkspaceWithOwner-->>checkWorkspaceAccess: "WorkspaceWithOwner | null"
    checkWorkspaceAccess->>getEffectiveWorkspacePermission: (userId, ws)
    getEffectiveWorkspacePermission-->>checkWorkspaceAccess: "PermissionType | null"
    checkWorkspaceAccess-->>hasWorkspaceAdminAccess: "WorkspaceAccess { canAdmin, permission, ... }"
    hasWorkspaceAdminAccess-->>Caller: boolean (canAdmin)

    Caller->>getUserEntityPermissions: (userId, 'workspace', entityId)
    getUserEntityPermissions->>checkWorkspaceAccess: (entityId, userId)
    checkWorkspaceAccess->>getWorkspaceWithOwner: (entityId)
    getWorkspaceWithOwner-->>checkWorkspaceAccess: "WorkspaceWithOwner | null"
    checkWorkspaceAccess->>getEffectiveWorkspacePermission: (userId, ws)
    getEffectiveWorkspacePermission-->>checkWorkspaceAccess: "PermissionType | null"
    checkWorkspaceAccess-->>getUserEntityPermissions: "WorkspaceAccess { permission, ... }"
    getUserEntityPermissions-->>Caller: "PermissionType | null (permission)"

%%{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 Caller
    participant hasWorkspaceAdminAccess
    participant getUserEntityPermissions
    participant checkWorkspaceAccess
    participant getWorkspaceWithOwner
    participant getEffectiveWorkspacePermission

    note over hasWorkspaceAdminAccess,getUserEntityPermissions: Before: each called DB directly
    note over hasWorkspaceAdminAccess,getUserEntityPermissions: After: thin wrappers

    Caller->>hasWorkspaceAdminAccess: (userId, workspaceId)
    hasWorkspaceAdminAccess->>checkWorkspaceAccess: (workspaceId, userId)
    checkWorkspaceAccess->>getWorkspaceWithOwner: (workspaceId)
    getWorkspaceWithOwner-->>checkWorkspaceAccess: "WorkspaceWithOwner | null"
    checkWorkspaceAccess->>getEffectiveWorkspacePermission: (userId, ws)
    getEffectiveWorkspacePermission-->>checkWorkspaceAccess: "PermissionType | null"
    checkWorkspaceAccess-->>hasWorkspaceAdminAccess: "WorkspaceAccess { canAdmin, permission, ... }"
    hasWorkspaceAdminAccess-->>Caller: boolean (canAdmin)

    Caller->>getUserEntityPermissions: (userId, 'workspace', entityId)
    getUserEntityPermissions->>checkWorkspaceAccess: (entityId, userId)
    checkWorkspaceAccess->>getWorkspaceWithOwner: (entityId)
    getWorkspaceWithOwner-->>checkWorkspaceAccess: "WorkspaceWithOwner | null"
    checkWorkspaceAccess->>getEffectiveWorkspacePermission: (userId, ws)
    getEffectiveWorkspacePermission-->>checkWorkspaceAccess: "PermissionType | null"
    checkWorkspaceAccess-->>getUserEntityPermissions: "WorkspaceAccess { permission, ... }"
    getUserEntityPermissions-->>Caller: "PermissionType | null (permission)"

_{Reviews (1): Last reviewed commit: "refactor(permissions): make hasWorkspace..." | Re-trigger Greptile}

waleedlatif1 merged commit 20e8654 into staging Jul 5, 2026
18 checks passed

waleedlatif1 deleted the workspace-permissions-vend-consolidation branch July 5, 2026 17:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor(permissions): make hasWorkspaceAdminAccess and getUserEntityPermissions thin wrappers of checkWorkspaceAccess#5430

refactor(permissions): make hasWorkspaceAdminAccess and getUserEntityPermissions thin wrappers of checkWorkspaceAccess#5430
waleedlatif1 merged 1 commit into
stagingfrom
workspace-permissions-vend-consolidation

waleedlatif1 commented Jul 5, 2026

Uh oh!

vercel Bot commented Jul 5, 2026

Uh oh!

cursor Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Jul 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

waleedlatif1 commented Jul 5, 2026

Summary

Type of Change

Testing

Checklist

Uh oh!

vercel Bot commented Jul 5, 2026

Uh oh!

cursor Bot commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

greptile-apps Bot commented Jul 5, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cursor Bot commented Jul 5, 2026 •

edited

Loading