fix(workspaces): serialize the workspace.create gate against permission-group writes

The interim fix hoisted the whole capability gate out of the creation
transaction. That removed the second pooled-connection checkout the
`DbTxTripwire` fires on, but left the revocation window open: an admin could
revoke `workspace.create` between the gate and the insert. The original
placement did not close it either — creation took `organization-mutation:<org>`
while permission-group writes take `permission_group:<org>`, so the two never
contended.

Split the decision along the line that actually matters:

- ENTITLEMENT (is this organization governed by permission groups at all)
  stays outside the transaction. It bottoms out in `isOrganizationOnEnterprisePlan`,
  which is `cache(resolveOrganizationEnterprisePlan)` and admits no options
  object by design — an executor argument would miss the memo on every call, and
  bypassing the cache resolves a lapsed read to `config: null`, meaning every
  capability ALLOWED. `permission_group:<org>` never serialized subscription
  changes, so holding it across this read excluded nothing. A concurrent lapse
  now applies the group config for one more request (refuses, does not permit);
  a concurrent grant skips it for one more request, the same answer the route's
  own preflight gave microseconds earlier.

- The CAPABILITY itself is re-read INSIDE the transaction, on the transaction
  executor, under `acquirePermissionGroupOrgLock` — the same advisory lock every
  permission-group mutation takes. That is what makes the check-to-insert window
  closed rather than merely narrow. `resolveDefaultGroup` gains an executor
  parameter; nothing else in the chain is threaded.

The pre-transaction capability read is REMOVED rather than kept as a fast-fail:
both callers of `createWorkspace` already refuse on `creationPolicy.canCreate`
(`app/api/workspaces/route.ts`), so it re-read the same value microseconds later
and caught only what the under-lock read now catches definitively. Net effect is
one fewer read on the enterprise create path, not one more.

LOCK ORDER: `organization-mutation:<org>` -> `user-billing-identity:<user>` ->
`<user>:<org>` -> `permission_group:<org>`. The new lock is taken last, and
safely so: it is a leaf. Every transaction that holds it — the five
`organizations/[id]/permission-groups` route transactions — acquires no further
advisory lock afterwards, so no holder can be waiting on any of the three above
it and no cycle can form. It is also taken only AFTER live membership is
confirmed, so a caller who turns out not to belong to the organization never
serializes against its admins, and only when the regime is active, so
non-enterprise and unaffiliated creators take nothing new.

`createDefaultPersonalWorkspaceInTransaction`, which runs inside the enterprise
owner claim's external transaction, passes both organization ids as `null`, so
it is ungoverned by construction, takes no permission-group lock, and cannot
deadlock against the locks that transaction already holds.

`acquirePermissionGroupOrgLock` moves to `lib/permission-groups/locks.ts` so
`lib/workspaces/policy.ts` can acquire it without importing from `app/api/**`;
the five route call sites are repointed rather than re-exported.

Contention this adds, honestly: workspace creates in an Enterprise organization
with Access Control now serialize against that organization's permission-group
admin writes, and against each other, on one org-wide key. Personal creates by
members of such an organization take that lock where they previously took none.
Permission-group writes are low-frequency admin actions and the lock is held for
one indexed single-row read, so the expected wait is negligible — but it is a
real new serialization point, not a free one.

Author: waleedlatif1

apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/members/bulk/route.ts

@@ -11,8 +11,8 @@ import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
 import { getSession } from '@/lib/auth'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { PERMISSION_GROUP_MEMBER_CONSTRAINTS } from '@/lib/permission-groups/constraints'
+import { acquirePermissionGroupOrgLock } from '@/lib/permission-groups/locks'
 import {
-  acquirePermissionGroupOrgLock,
   authorizeOrgAccessControl,
   findScopeConflicts,
   formatScopeConflictError,

apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/members/route.ts

@@ -11,10 +11,10 @@ import { getValidationErrorMessage, parseRequest } from '@/lib/api/server'
 import { getSession } from '@/lib/auth'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { PERMISSION_GROUP_MEMBER_CONSTRAINTS } from '@/lib/permission-groups/constraints'
+import { acquirePermissionGroupOrgLock } from '@/lib/permission-groups/locks'
 import { isOrganizationMember } from '@/lib/workspaces/permissions/utils'
 import {
   type AllMembersConflict,
-  acquirePermissionGroupOrgLock,
   authorizeOrgAccessControl,
   findAllMembersWorkspaceConflict,
   findScopeConflicts,

apps/sim/app/api/organizations/[id]/permission-groups/[groupId]/route.ts

@@ -15,9 +15,9 @@ import {
   type PermissionGroupConfig,
   parsePermissionGroupConfig,
 } from '@/lib/permission-groups/fields'
+import { acquirePermissionGroupOrgLock } from '@/lib/permission-groups/locks'
 import {
   type AllMembersConflict,
-  acquirePermissionGroupOrgLock,
   authorizeOrgAccessControl,
   findAllMembersWorkspaceConflict,
   findScopeConflicts,

apps/sim/app/api/organizations/[id]/permission-groups/route.ts

@@ -21,9 +21,9 @@ import {
   type PermissionGroupConfig,
   parsePermissionGroupConfig,
 } from '@/lib/permission-groups/fields'
+import { acquirePermissionGroupOrgLock } from '@/lib/permission-groups/locks'
 import {
   type AllMembersConflict,
-  acquirePermissionGroupOrgLock,
   authorizeOrgAccessControl,
   findAllMembersWorkspaceConflict,
   findWorkspacesNotInOrganization,

apps/sim/app/api/organizations/[id]/permission-groups/utils.ts

@@ -41,36 +41,6 @@ export async function authorizeOrgAccessControl(
   return null
 }
 
-const PERMISSION_GROUP_LOCK_TIMEOUT_MS = 5_000
-
-/**
- * Serialize all permission-group membership and scope writes for an organization
- * via a transaction-scoped Postgres advisory lock. Callers acquire it at the top
- * of the transaction that both checks (`findScopeConflicts`) and mutates, so a
- * concurrent member add or scope change can't commit in the check-to-write
- * window and leave a user governed by two groups on the same workspace.
- *
- * The invariant (one effective group per user per workspace) spans users and
- * groups in ways a unique constraint can't express, and these are low-frequency
- * admin writes, so a single org-scoped lock is simpler and more obviously
- * correct than fine-grained per-user/per-group locks with acquire-ordering.
- *
- * `pg_advisory_xact_lock` auto-releases at transaction end (safe on pooled
- * connections), and `lock_timeout` bounds the wait (raising SQLSTATE 55P03)
- * instead of hanging if a holder is stuck.
- */
-export async function acquirePermissionGroupOrgLock(
-  tx: DbOrTx,
-  organizationId: string
-): Promise<void> {
-  await tx.execute(
-    sql`select set_config('lock_timeout', ${`${PERMISSION_GROUP_LOCK_TIMEOUT_MS}ms`}, true)`
-  )
-  await tx.execute(
-    sql`select pg_advisory_xact_lock(hashtextextended(${`permission_group:${organizationId}`}, 0))`
-  )
-}
-
 /** Load a permission group only if it belongs to the given organization. */
 export async function loadGroupInOrganization(
   groupId: string,

apps/sim/lib/permission-groups/capability-assertions.ts

@@ -1,11 +1,15 @@
+import type { DbOrTx } from '@/lib/db/types'
 import {
   CAPABILITY_RULES,
   refuseCapability,
   type StaticPermissionGroupCapability,
 } from '@/lib/permission-groups/capabilities'
 import { resolvePermissionGroupConfig } from '@/lib/permission-groups/config-scope.server'
 import type { PermissionGroupConfig } from '@/lib/permission-groups/fields'
-import { getUserPermissionConfigForOrganization } from '@/lib/permission-groups/resolve.server'
+import {
+  getEntitledOrganizationPermissionConfig,
+  getUserPermissionConfigForOrganization,
+} from '@/lib/permission-groups/resolve.server'
 
 /**
  * Re-exported so a caller that gates inline reaches the refusal sentence and the
@@ -90,3 +94,29 @@ export async function isOrganizationCapabilityWithheld(
     await getUserPermissionConfigForOrganization(organizationId)
   )
 }
+
+/**
+ * {@link isOrganizationCapabilityWithheld} for an organization whose regime the
+ * caller has ALREADY established with `isOrganizationPermissionRegimeActive`,
+ * reading the group on the given executor.
+ *
+ * For the one caller that must answer this question inside the transaction that
+ * commits the governed act, under `acquirePermissionGroupOrgLock`, so an admin
+ * cannot revoke the capability in the check-to-write window. Splitting the
+ * entitlement half off is what lets the remaining read run on the transaction's
+ * own connection instead of checking out a second pooled one.
+ *
+ * Passing an executor without having checked entitlement would read a stale
+ * default group as authoritative for an organization the regime no longer
+ * governs, so that check is the caller's obligation, not an optional one.
+ */
+export async function isEntitledOrganizationCapabilityWithheld(
+  organizationId: string,
+  capability: StaticPermissionGroupCapability,
+  executor: DbOrTx
+): Promise<boolean> {
+  return capabilityDeniedBy(
+    capability,
+    await getEntitledOrganizationPermissionConfig(organizationId, executor)
+  )
+}

apps/sim/lib/permission-groups/locks.ts

@@ -0,0 +1,49 @@
+import { sql } from 'drizzle-orm'
+import type { DbOrTx } from '@/lib/db/types'
+
+const PERMISSION_GROUP_LOCK_TIMEOUT_MS = 5_000
+
+/**
+ * Serialize all permission-group membership, scope, and config writes for an
+ * organization via a transaction-scoped Postgres advisory lock. Callers acquire
+ * it at the top of the transaction that both checks (`findScopeConflicts`) and
+ * mutates, so a concurrent member add or scope change can't commit in the
+ * check-to-write window and leave a user governed by two groups on the same
+ * workspace.
+ *
+ * The invariant (one effective group per user per workspace) spans users and
+ * groups in ways a unique constraint can't express, and these are low-frequency
+ * admin writes, so a single org-scoped lock is simpler and more obviously
+ * correct than fine-grained per-user/per-group locks with acquire-ordering.
+ *
+ * Readers take it too, when the value they read decides whether a write in the
+ * same transaction may commit — workspace creation re-reads the default group's
+ * `workspace.create` capability under this lock, which is the only thing that
+ * makes the check-to-insert window closed rather than merely narrow.
+ *
+ * `pg_advisory_xact_lock` auto-releases at transaction end (safe on pooled
+ * connections), and `lock_timeout` bounds the wait (raising SQLSTATE 55P03)
+ * instead of hanging if a holder is stuck.
+ *
+ * LOCK ORDER: this is a LEAF lock. Every transaction that holds it — the five
+ * `organizations/[id]/permission-groups` route transactions, and the workspace
+ * creation transaction — acquires no further advisory lock afterwards. That is
+ * what makes it safe for workspace creation to take it *last*, after
+ * `organization-mutation`, `user-billing-identity`, and the membership lock: a
+ * deadlock needs a holder of this lock to wait on one of those, and no such
+ * holder exists. Keep it a leaf.
+ *
+ * Lives in `lib/` rather than beside the routes because `lib/workspaces/policy.ts`
+ * acquires it, and `lib/` must not import from `app/api/**`.
+ */
+export async function acquirePermissionGroupOrgLock(
+  tx: DbOrTx,
+  organizationId: string
+): Promise<void> {
+  await tx.execute(
+    sql`select set_config('lock_timeout', ${`${PERMISSION_GROUP_LOCK_TIMEOUT_MS}ms`}, true)`
+  )
+  await tx.execute(
+    sql`select pg_advisory_xact_lock(hashtextextended(${`permission_group:${organizationId}`}, 0))`
+  )
+}

apps/sim/lib/permission-groups/resolve.server.ts

@@ -22,6 +22,7 @@ import {
   isAccessControlEnabled,
   isHosted,
 } from '@/lib/core/config/env-flags'
+import type { DbOrTx } from '@/lib/db/types'
 import {
   DEFAULT_PERMISSION_GROUP_CONFIG,
   type PermissionGroupConfig,
@@ -90,11 +91,19 @@ function inactiveUserAccessControlContext(organizationId: string | null): UserAc
   }
 }
 
-/** The organization's single default group (`isDefault`), or `null`. */
+/**
+ * The organization's single default group (`isDefault`), or `null`.
+ *
+ * Takes an executor so a caller that must read the group under
+ * `acquirePermissionGroupOrgLock` — inside the transaction that holds it — can
+ * do so on the transaction's own connection instead of checking out a second
+ * one from the pool.
+ */
 async function resolveDefaultGroup(
-  organizationId: string
+  organizationId: string,
+  executor: DbOrTx = db
 ): Promise<ResolvedPermissionGroup | null> {
-  const [defaultGroup] = await db
+  const [defaultGroup] = await executor
     .select({
       id: permissionGroup.id,
       name: permissionGroup.name,
@@ -281,16 +290,50 @@ export async function getUserPermissionConfig(
 export async function getUserPermissionConfigForOrganization(
   organizationId: string
 ): Promise<PermissionGroupConfig | null> {
-  if (!isHosted && !isAccessControlEnabled) {
+  if (!(await isOrganizationPermissionRegimeActive(organizationId))) {
     return mergeEnvAllowlist(null)
   }
+  return getEntitledOrganizationPermissionConfig(organizationId)
+}
 
-  /** `'throw'` for the same reason as in {@link resolveUserAccessControlContextForOrganization}. */
-  const isEnterprise = await isOrganizationOnEnterprisePlan(organizationId, 'throw')
-  if (!isEnterprise) {
-    return mergeEnvAllowlist(null)
-  }
+/**
+ * Whether permission groups govern `organizationId` at all — the deployment
+ * enables Access Control, and the organization holds the Enterprise entitlement
+ * that turns the regime on.
+ *
+ * Split out of {@link getUserPermissionConfigForOrganization} so a caller that
+ * must re-read the *group* under `acquirePermissionGroupOrgLock` can settle this
+ * half BEFORE opening its transaction. The entitlement read cannot move into a
+ * transaction: {@link isOrganizationOnEnterprisePlan} is `cache()`d on its
+ * argument list, so it admits no executor, and giving it one would both miss the
+ * memo on every call and — because an unentitled organization resolves to
+ * `config: null`, meaning every capability ALLOWED — turn a read failure into a
+ * fail-open. The lock never serialized this half either way: it guards
+ * permission-group writes, not subscription changes.
+ *
+ * `'throw'` for the same reason as in
+ * {@link resolveUserAccessControlContextForOrganization}.
+ */
+export async function isOrganizationPermissionRegimeActive(
+  organizationId: string
+): Promise<boolean> {
+  if (!isHosted && !isAccessControlEnabled) return false
+  return isOrganizationOnEnterprisePlan(organizationId, 'throw')
+}
 
-  const resolved = await resolveDefaultGroup(organizationId)
+/**
+ * The organization-level permission config for an organization already known to
+ * be governed — the second half of {@link getUserPermissionConfigForOrganization},
+ * callable on a transaction executor.
+ *
+ * Callers MUST have established {@link isOrganizationPermissionRegimeActive}
+ * first; this function does not re-check entitlement, and reading it as though
+ * it did would apply an unentitled organization's stale default group.
+ */
+export async function getEntitledOrganizationPermissionConfig(
+  organizationId: string,
+  executor: DbOrTx = db
+): Promise<PermissionGroupConfig | null> {
+  const resolved = await resolveDefaultGroup(organizationId, executor)
   return mergeEnvAllowlist(resolved?.config ?? null)
 }