Skip to content

improvement(governance): org-ws-credential roles clarity#5134

Open
icecrasher321 wants to merge 3 commits into
stagingfrom
improvement/gov-model-guarantees
Open

improvement(governance): org-ws-credential roles clarity#5134
icecrasher321 wants to merge 3 commits into
stagingfrom
improvement/gov-model-guarantees

Conversation

@icecrasher321

Copy link
Copy Markdown
Collaborator

Summary

Org Admins are auto Workspace Admins. And workspace admins are auto credential admins.

Type of Change

  • Other: UX improvement

Testing

Tested manually

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

vercel Bot commented Jun 19, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Jun 19, 2026 4:20am

Request Review

@cursor

cursor Bot commented Jun 19, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Changes effective permission resolution across workspaces, credentials, env secrets, and workflow authorization—security-critical paths where incorrect inheritance could grant or deny access broadly.

Overview
This PR implements a governance inheritance chain: organization Owners/Admins are treated as workspace Admins on every workspace in that org (without a per-workspace invite or permission row), and workspace Admins are treated as credential Admins on shared credentials (OAuth, service accounts, workspace env secrets)—personal env secrets stay private.

Backend enforcement adds getEffectiveWorkspacePermission / canAdmin on workspace access, merges org-admin workspaces into workspace listing (listAccessibleWorkspaceRowsForUser), and mirrors the same rules in workflow-authz. Credential admin checks and workflow credential access now grant admin/use when the caller is a workspace admin even without an explicit credentialMember row. Credential member APIs merge in workspace admins with roleSource: 'workspace-admin', block demoting/removing them, and stop bulk-seeding every workspace admin as DB credential admins on create/sync.

Product guardrails: PATCH workspace permissions rejects changing org owner/admin roles; org roster shows org admins as admin on all org workspaces. Member and credential UIs disable role changes for inherited roles and show RoleLockTooltip explanations. Permissions docs and FAQs describe credential access and the fixed inherited roles.

Tests cover credential actor context, org-admin workspace access, and accessible workspace listing elevation.

Reviewed by Cursor Bugbot for commit fc753a8. Configure here.

Comment thread apps/sim/app/api/credentials/route.ts
@greptile-apps

greptile-apps Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR implements a role-inheritance governance model: org owners/admins are derived workspace admins, and workspace admins are derived credential admins. It centralises the resolution logic in a new @sim/platform-authz package so the Next.js app and the realtime Bun server share a single source of truth.

  • Platform-authz package: Introduces resolveEffectiveWorkspacePermission, permissionSatisfies, isOrgAdminRole, and related predicates shared across both apps.
  • Workspace permissions: checkWorkspaceAccess now returns canAdmin, and getUsersWithPermissions surfaces org-admin–derived members in member lists.
  • Credential access: getCredentialActorContext derives admin status from workspace role, the credential list uses a leftJoin + accessClause to expose shared credentials to workspace admins, and the "promote workspace owner" orphan-prevention logic is removed in favour of the governance model.

Confidence Score: 4/5

Safe to merge with one fix: the last-admin guard in the credential member DELETE endpoint should account for derived workspace admins before blocking removal.

The governance model and shared resolver are well-structured. One functional regression was found: the credential member DELETE handler's 'last explicit admin' guard was not updated to account for the new model where workspace admins are derived credential admins but no longer receive explicit admin rows at credential-creation time. This means the last explicit credential admin (typically the creator) cannot be removed even when workspace-derived admins still have full access, creating a permanently stuck state.

apps/sim/app/api/credentials/[id]/members/route.ts — the activeAdmins.length <= 1 check inside the DELETE transaction needs to also check for derived workspace admins before returning false.

Important Files Changed

Filename Overview
apps/sim/app/api/credentials/[id]/members/route.ts Adds workspace-admin protection to POST/DELETE and enriches GET with derived members, but the existing last-admin guard still only counts explicit credential-member rows and will block legitimate removals once workspace admins are no longer auto-enrolled as explicit admins.
packages/platform-authz/src/workspace.ts New shared resolver; correctly implements owner → admin short-circuit, explicit permission lookup, and org-admin derivation with a single scoped query per org.
packages/platform-authz/src/predicates.ts Introduces shared permission ordering constants and pure predicates (permissionSatisfies, isOrgAdminRole); replaces scattered hand-written role comparisons.
apps/sim/lib/credentials/access.ts Derives credential admin status from workspace role via deriveCredentialAdmin; removes complex orphan-prevention logic that is no longer needed under the governance model.
apps/sim/lib/workspaces/permissions/utils.ts Delegates workspace permission resolution to resolveEffectiveWorkspacePermission and adds canAdmin to WorkspaceAccess; enriches getUsersWithPermissions with org-admin derived rows.
apps/sim/lib/workspaces/utils.ts Adds getOrgAdminWorkspaceRows and listAccessibleWorkspaceRowsForUser; the org-admin lookup uses .limit(1) without a role filter before the limit (already flagged in a prior review thread).
apps/sim/app/api/credentials/route.ts Switches credential list to a leftJoin with an accessClause that exposes shared credentials to workspace admins; credential creation no longer grants explicit admin to all workspace admins, which is consistent with the new governance model.
apps/sim/app/api/workspaces/[id]/permissions/route.ts Adds a guard that blocks workspace permission updates for org admins; rejects the whole batch atomically if any target is an org admin.
apps/sim/app/api/auth/oauth/credentials/route.ts Switches to leftJoin + conditional isNotNull(credentialMember.id) filter so workspace admins see all OAuth/service-account credentials; uses getCredentialActorContext for per-credential checks.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    OrgOwner([Org Owner])
    OrgAdmin([Org Admin])
    WsOwner([Workspace Owner])
    WsAdmin([Workspace Admin - explicit])
    WsMember([Workspace Member])

    OrgOwner -- "derived workspace admin" --> WsAdmin
    OrgAdmin -- "derived workspace admin" --> WsAdmin
    WsOwner -- "implicit workspace admin" --> WsAdmin

    WsAdmin -- "derived credential admin\n(shared creds only)" --> CredAdmin([Credential Admin])
    WsMember -- "explicit credential member\n(via credentialMember table)" --> CredMember([Credential Member])

    CredAdmin --> CanManage[Can manage credential\n & its members]
    CredMember --> CanUse[Can use credential]

    style OrgOwner fill:#d4edda
    style OrgAdmin fill:#d4edda
    style WsAdmin fill:#cce5ff
    style CredAdmin fill:#fff3cd
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"}}}%%
flowchart TD
    OrgOwner([Org Owner])
    OrgAdmin([Org Admin])
    WsOwner([Workspace Owner])
    WsAdmin([Workspace Admin - explicit])
    WsMember([Workspace Member])

    OrgOwner -- "derived workspace admin" --> WsAdmin
    OrgAdmin -- "derived workspace admin" --> WsAdmin
    WsOwner -- "implicit workspace admin" --> WsAdmin

    WsAdmin -- "derived credential admin\n(shared creds only)" --> CredAdmin([Credential Admin])
    WsMember -- "explicit credential member\n(via credentialMember table)" --> CredMember([Credential Member])

    CredAdmin --> CanManage[Can manage credential\n & its members]
    CredMember --> CanUse[Can use credential]

    style OrgOwner fill:#d4edda
    style OrgAdmin fill:#d4edda
    style WsAdmin fill:#cce5ff
    style CredAdmin fill:#fff3cd
Loading

Comments Outside Diff (1)

  1. apps/sim/app/api/credentials/[id]/members/route.ts, line 347-363 (link)

    P1 Last-admin guard counts only explicit members, ignoring derived workspace admins

    The activeAdmins query counts rows in credentialMember with role = 'admin'. Under the old model every workspace admin received an explicit admin row at credential-creation time, so this count was always ≥ 2. Under the new model only the creator gets an explicit admin row — other workspace admins are derived admins and hold role = 'member' in credentialMember.

    Consequence: if the creator is the only explicit admin, any attempt to remove them returns 409 "Cannot remove the last admin from a credential", even though workspace-derived admins still have full credential admin access and the credential is not orphaned. The removal is permanently blocked.

    The fix is to short-circuit the guard when the credential has at least one workspace admin: check isSharedCredentialType(admin.credentialType) and, if true, query whether the workspace has any effective admin (owner or org admin) before returning false.

Reviews (2): Last reviewed commit: "improvement(credentials): code cleanup" | Re-trigger Greptile

Comment thread apps/sim/lib/workspaces/utils.ts
@icecrasher321 icecrasher321 requested a review from a team as a code owner June 19, 2026 04:14
@icecrasher321

Copy link
Copy Markdown
Collaborator Author

@greptile

@icecrasher321

Copy link
Copy Markdown
Collaborator Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3ab3a0c. Configure here.

return NextResponse.json({ error: 'userId query parameter required' }, { status: 400 })
}

const admin = await requireWorkspaceAdminMembership(credentialId, session.user.id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last admin ignores derived admins

Medium Severity

Demote and remove handlers still treat only explicit credential_member rows with role admin as admins. New credentials no longer seed workspace admins as explicit admins, so demoting or removing the last explicit admin can fail with “last admin” errors even when workspace admins still have derived credential admin access.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3ab3a0c. Configure here.

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