fix(workspaces): gate workspace creation before the transaction, not inside it (#7493)

* fix(workspaces): gate workspace creation before the transaction, not inside it

`lockWorkspaceCreationContext` called `isOrganizationCapabilityWithheld` with
no executor, so it ran on the global `db` pool while the creation transaction
already held a pooled connection and three advisory locks. That is the pool
deadlock `packages/db/tx-tripwire.ts` exists to detect — it is firing in
staging right now, four times per request, 144 lines across 36 requests in six
hours, all on POST /api/workspaces.

It also added up to four SEQUENTIAL reads to a lock hold that serializes every
organization mutation, which is what pushed concurrent creates past the 5s
lock_timeout and answered them as a generic 500.

Nothing is given up by moving the gate out. The re-read was never serialized
against permission-group writes: those take `permission_group:<org>` while
creation takes `organization-mutation:<org>`, which are different advisory-lock
ids and never contend. What the check actually provides is recency against the
caller's earlier preflight, and that holds identically microseconds earlier.
The governing organization is `organizationId ?? observedOrganizationId`, both
known before the transaction, and `lockWorkspaceCreationContext` still refuses
to commit unless live membership still equals `observedOrganizationId` — so a
verdict computed outside can never be applied to a different organization.

The comment justifying the old placement was also wrong: for POST
/api/workspaces the preflight and the insert are the same request, not separate
ones.

Rejected alternatives: threading an executor through the resolver would touch
seven functions across four files, requires exporting the uncached
`resolveOrganizationEnterprisePlan`, and bypassing the React cache is a
fail-OPEN semantic change. `runOutsideTransactionContext` only exits the
ambient context — it would silence the tripwire while leaving the second pooled
connection checkout in place.

Adds create.test.ts pinning the ordering, because no existing test can catch a
regression here: vitest.setup.ts mocks @sim/db globally, so the real pool
instrumentation never runs and the tripwire cannot fire in any apps/sim test.

* fix(workspaces): reattach the lock helper's TSDoc and correct it

The new gate was inserted between `lockWorkspaceCreationContext`'s doc block
and the function, orphaning the doc onto the gate and leaving the lock helper
undocumented. The doc was also stale: it described serializing 'the final
creation-policy check', which no longer happens there. It now states what the
function does enforce — the membership invariant the hoisted gate's verdict
depends on.

* 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.

* fix(workspaces): carry the governing organization instead of a derived boolean

Replaces `permissionGroupsGovernCreation: boolean` plus a separately
re-derived organization id with the organization id itself, so the
capability gate has one field carrying one fact and no branch TypeScript
cannot see through.

- `resolveGoverningPermissionGroupOrganization` replaces both
  `isWorkspaceCreationGovernedByPermissionGroups` and
  `governingOrganizationIdFor`; the id is derived once, before the
  transaction, and reused inside it.
- The creation policy now carries the resolved organization, so the create
  route does not re-issue the entitlement read that React's `cache()` memo
  does not span in an App Route.
- `lockWorkspaceCreationContext` takes the permission-group lock after the
  organization's own `FOR UPDATE` revalidation and owner lookup, so an
  org-wide key is no longer held across a blocking row-lock wait.
- `resolveDefaultGroup` and `getEntitledOrganizationPermissionConfig`
  require their executor, so a caller cannot silently reintroduce the
  second pooled-connection checkout.
- `acquirePermissionGroupOrgLock` skips the redundant `lock_timeout`
  `set_config` when the caller already bounded it at the same value.

* docs(workspaces): cut the third copy of the personal-workspace rationale

The escape-hatch reasoning already lives at the preflight that enforces it,
and again in the test. A pointer is enough here.

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/app/api/workspaces/route.ts

@@ -2,6 +2,7 @@ import { AuditAction, AuditResourceType, recordAudit } from '@sim/audit'
 import { db } from '@sim/db'
 import { type WorkspaceMode, workflow } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
+import { getPostgresErrorCode } from '@sim/utils/errors'
 import { and, eq, isNull } from 'drizzle-orm'
 import { type NextRequest, NextResponse } from 'next/server'
 import { listWorkspacesQuerySchema } from '@/lib/api/contracts'
@@ -157,6 +158,7 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
       workspaceMode: creationPolicy.workspaceMode,
       billedAccountUserId: creationPolicy.billedAccountUserId,
       observedOrganizationId: creationPolicy.observedOrganizationId,
+      governingPermissionGroupOrganizationId: creationPolicy.governingPermissionGroupOrganizationId,
     })
 
     captureServerEvent(
@@ -207,6 +209,20 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
         { status: 409 }
       )
     }
+    /**
+     * A lock timeout is contention, not a fault: creation serializes on the
+     * organization's mutation locks and now also on `permission_group:<org>`,
+     * so a concurrent create or a permission-group admin write can exhaust the
+     * `lock_timeout` and abort this transaction. Answer 503 like the
+     * permission-group routes do, rather than letting it reach the generic 500
+     * below — the caller should retry, and a 500 tells them the opposite.
+     */
+    if (getPostgresErrorCode(error) === '55P03') {
+      return NextResponse.json(
+        { error: 'This organization is being updated by another request. Please try again.' },
+        { status: 503 }
+      )
+    }
     logger.error('Error creating workspace:', error)
     return NextResponse.json({ error: 'Failed to create workspace' }, { status: 500 })
   }
@@ -220,6 +236,7 @@ async function createDefaultWorkspace(
     workspaceMode: WorkspaceMode
     billedAccountUserId: string
     observedOrganizationId: string | null
+    governingPermissionGroupOrganizationId: string | null
   }
 ) {
   const firstName = userName?.split(' ')[0] || null
@@ -231,6 +248,7 @@ async function createDefaultWorkspace(
     workspaceMode: creationPolicy.workspaceMode,
     billedAccountUserId: creationPolicy.billedAccountUserId,
     observedOrganizationId: creationPolicy.observedOrganizationId,
+    governingPermissionGroupOrganizationId: creationPolicy.governingPermissionGroupOrganizationId,
   })
 }
 

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,20 @@ export async function isOrganizationCapabilityWithheld(
     await getUserPermissionConfigForOrganization(organizationId)
   )
 }
+
+/**
+ * {@link isOrganizationCapabilityWithheld} for an organization whose regime the
+ * caller has ALREADY established, reading the group on the given executor. That
+ * establishment is the caller's obligation, not an optional one — see
+ * {@link getEntitledOrganizationPermissionConfig}.
+ */
+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,62 @@
+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. The key string is the contention
+ * identity: any change to its format silently stops contending with in-flight
+ * holders.
+ *
+ * `lockTimeoutAlreadyBounded` skips the `set_config` round trip for a caller
+ * that has already bounded `lock_timeout` transaction-locally at the same
+ * 5000ms — every advisory lock in `lib/billing/organizations/membership.ts`
+ * does, and workspace creation takes those first. It stays a separate statement
+ * rather than being folded into the `pg_advisory_xact_lock` select: target-list
+ * evaluation order is unspecified, so the bound might not be in force when the
+ * lock is requested.
+ *
+ * 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,
+  options?: { lockTimeoutAlreadyBounded?: boolean }
+): Promise<void> {
+  if (!options?.lockTimeoutAlreadyBounded) {
+    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`.
+ *
+ * The executor is required, not defaulted: a caller that must read the group
+ * under `acquirePermissionGroupOrgLock` has to read it on the transaction's own
+ * connection, and a default would let that caller silently check out a second
+ * pooled connection while advisory locks are held.
+ */
 async function resolveDefaultGroup(
-  organizationId: string
+  organizationId: string,
+  executor: DbOrTx
 ): Promise<ResolvedPermissionGroup | null> {
-  const [defaultGroup] = await db
+  const [defaultGroup] = await executor
     .select({
       id: permissionGroup.id,
       name: permissionGroup.name,
@@ -181,7 +190,7 @@ export async function resolveWorkspaceGroup(
     }
   }
 
-  return resolveDefaultGroup(organizationId)
+  return resolveDefaultGroup(organizationId, db)
 }
 
 /**
@@ -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, db)
+}
 
-  /** `'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
+): Promise<PermissionGroupConfig | null> {
+  const resolved = await resolveDefaultGroup(organizationId, executor)
   return mergeEnvAllowlist(resolved?.config ?? null)
 }