From 0a5bc435640ff68ac96f4bf0f901d30b4ec86320 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Wed, 26 Aug 2026 23:08:54 -0700
Subject: [PATCH 1/5] fix(credentials): write an env value and its credential
 row together
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Every writer of a workspace or personal environment map read-modify-writes a
single jsonb column, so they take an advisory lock on the map to serialize.
`deleteCredentialRecord` took none, and did the read, the edit and the
write-back outside a transaction: a secret written concurrently was read
before that write and silently dropped by the write-back.

The credential row was also written after its map transaction committed, in
four places. The delete direction left a row describing a value that was gone;
the create direction was worse than a stale row, because it cannot be repaired
by retrying — the key is in the map by then, so the next attempt reads it as
pre-existing, computes an empty `newKeys`, and never creates the row. Both
helpers already accept `executor`, and `setWorkspaceSecret` has been passing
the transaction since the parameter landed; these four were never migrated.

The personal reconcile stays outside its transaction: it opens its own and
takes the user-identity fence, so nesting it would have two transactions
taking two locks in opposite orders. It reconciles against the stored keys,
so a failure there is repaired by the next one rather than entrenched.

Also folds the four copies of the lock into one helper, since this would have
been the fifth.
---
 .../api/workspaces/[id]/environment/route.ts  |  42 ++++---
 apps/sim/lib/credentials/env-locks.ts         |  32 +++++
 .../credentials/orchestration/index.test.ts   |  63 +++++++++-
 .../lib/credentials/orchestration/index.ts    | 118 +++++++++++-------
 apps/sim/lib/credentials/secret-values.ts     |  21 +---
 apps/sim/lib/environment/utils.ts             |  30 +++--
 6 files changed, 216 insertions(+), 90 deletions(-)
 create mode 100644 apps/sim/lib/credentials/env-locks.ts

diff --git a/apps/sim/app/api/workspaces/[id]/environment/route.ts b/apps/sim/app/api/workspaces/[id]/environment/route.ts
index 98194c979b0..7b81f89df36 100644
--- a/apps/sim/app/api/workspaces/[id]/environment/route.ts
+++ b/apps/sim/app/api/workspaces/[id]/environment/route.ts
@@ -4,7 +4,7 @@ import { workspaceEnvironment } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { getErrorMessage } from '@sim/utils/errors'
 import { generateId } from '@sim/utils/id'
-import { eq, sql } from 'drizzle-orm'
+import { eq } from 'drizzle-orm'
 import { type NextRequest, NextResponse } from 'next/server'
 import {
   removeWorkspaceEnvironmentContract,
@@ -15,6 +15,7 @@ import { getSession } from '@/lib/auth'
 import { encryptSecret } from '@/lib/core/security/encryption'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
+import { lockWorkspaceEnvMap } from '@/lib/credentials/env-locks'
 import {
   createWorkspaceEnvCredentials,
   deleteWorkspaceEnvCredentials,
@@ -39,8 +40,6 @@ const logger = createLogger('WorkspaceEnvironmentAPI')
  * fast (SQLSTATE 55P03) rather than hanging, even if the deployment lacks a
  * server-side `lock_timeout`. Transaction-scoped via `set_config(..., true)`.
  */
-const WORKSPACE_ENV_LOCK_TIMEOUT_MS = 5_000
-
 /**
  * Restricts decrypted workspace env values to administrators. Members (including
  * read-only) receive the variable names with empty values so editor autocomplete
@@ -237,11 +236,8 @@ export const PUT = withRouteHandler(
         })
       ).then((entries) => Object.fromEntries(entries))
 
-      const { existingEncrypted, merged } = await db.transaction(async (tx) => {
-        await tx.execute(
-          sql`SELECT set_config('lock_timeout', ${`${WORKSPACE_ENV_LOCK_TIMEOUT_MS}ms`}, true)`
-        )
-        await tx.execute(sql`SELECT pg_advisory_xact_lock(hashtextextended(${workspaceId}, 0))`)
+      const { merged } = await db.transaction(async (tx) => {
+        await lockWorkspaceEnvMap(tx, workspaceId)
 
         const [existingRow] = await tx
           .select()
@@ -269,12 +265,24 @@ export const PUT = withRouteHandler(
             set: { variables: mergedVars, updatedAt: new Date() },
           })
 
-        return { existingEncrypted: existing, merged: mergedVars }
+        /**
+         * Inside the transaction because a value committed without its
+         * credential row cannot be repaired by retrying: the key is in the map
+         * by then, so the next attempt reads it as pre-existing, computes an
+         * empty `newKeys`, and never creates the row.
+         */
+        const newKeys = Object.keys(variables).filter((k) => !(k in existing))
+        await createWorkspaceEnvCredentials({
+          workspaceId,
+          newKeys,
+          actingUserId: userId,
+          executor: tx,
+        })
+
+        return { merged: mergedVars }
       })
 
       invalidateEffectiveDecryptedEnvCache({ workspaceId })
-      const newKeys = Object.keys(variables).filter((k) => !(k in existingEncrypted))
-      await createWorkspaceEnvCredentials({ workspaceId, newKeys, actingUserId: userId })
 
       recordAudit({
         workspaceId,
@@ -370,10 +378,7 @@ export const DELETE = withRouteHandler(
       }
 
       const result = await db.transaction(async (tx) => {
-        await tx.execute(
-          sql`SELECT set_config('lock_timeout', ${`${WORKSPACE_ENV_LOCK_TIMEOUT_MS}ms`}, true)`
-        )
-        await tx.execute(sql`SELECT pg_advisory_xact_lock(hashtextextended(${workspaceId}, 0))`)
+        await lockWorkspaceEnvMap(tx, workspaceId)
 
         const [existingRow] = await tx
           .select()
@@ -400,6 +405,12 @@ export const DELETE = withRouteHandler(
           .set({ variables: current, updatedAt: new Date() })
           .where(eq(workspaceEnvironment.workspaceId, workspaceId))
 
+        await deleteWorkspaceEnvCredentials({
+          workspaceId,
+          removedKeys: keys,
+          executor: tx,
+        })
+
         return { remainingKeysCount: Object.keys(current).length }
       })
 
@@ -408,7 +419,6 @@ export const DELETE = withRouteHandler(
       }
 
       invalidateEffectiveDecryptedEnvCache({ workspaceId })
-      await deleteWorkspaceEnvCredentials({ workspaceId, removedKeys: keys })
 
       recordAudit({
         workspaceId,
diff --git a/apps/sim/lib/credentials/env-locks.ts b/apps/sim/lib/credentials/env-locks.ts
new file mode 100644
index 00000000000..9bb425ffabf
--- /dev/null
+++ b/apps/sim/lib/credentials/env-locks.ts
@@ -0,0 +1,32 @@
+import { sql } from 'drizzle-orm'
+import type { DbOrTx } from '@/lib/db/types'
+
+const ENV_MAP_LOCK_TIMEOUT_MS = 5_000
+
+/**
+ * Serializes every writer of one environment variables map.
+ *
+ * Both maps are a single jsonb column that every writer read-modify-writes, so
+ * without this two concurrent writers each persist their own copy of the map
+ * and the later commit silently drops the earlier one's key. The lock is
+ * transaction-scoped, so it releases on commit or rollback with no unlock path
+ * to miss, and it must be taken before the read that the write is derived from.
+ *
+ * The keys are the bare workspace or user id, matching every writer that
+ * already takes this lock — a prefixed key would be a different lock and would
+ * serialize against nothing.
+ */
+async function lockEnvMap(tx: DbOrTx, lockKey: string): Promise<void> {
+  await tx.execute(sql`SELECT set_config('lock_timeout', ${`${ENV_MAP_LOCK_TIMEOUT_MS}ms`}, true)`)
+  await tx.execute(sql`SELECT pg_advisory_xact_lock(hashtextextended(${lockKey}, 0))`)
+}
+
+/** Serializes writers of one workspace's environment variables map. */
+export async function lockWorkspaceEnvMap(tx: DbOrTx, workspaceId: string): Promise<void> {
+  await lockEnvMap(tx, workspaceId)
+}
+
+/** Serializes writers of one user's personal environment variables map. */
+export async function lockPersonalEnvMap(tx: DbOrTx, userId: string): Promise<void> {
+  await lockEnvMap(tx, userId)
+}
diff --git a/apps/sim/lib/credentials/orchestration/index.test.ts b/apps/sim/lib/credentials/orchestration/index.test.ts
index 5aea33b7d95..2c0bab8b3ba 100644
--- a/apps/sim/lib/credentials/orchestration/index.test.ts
+++ b/apps/sim/lib/credentials/orchestration/index.test.ts
@@ -20,6 +20,8 @@ const {
   mockGetClientCredentialAccountDescriptor,
   mockDeleteConnectionCredential,
   mockDeleteOrphanedOAuthAccount,
+  mockDeleteWorkspaceEnvCredentials,
+  mockSyncPersonalEnvCredentialsForUser,
 } = vi.hoisted(() => ({
   mockRecordAudit: vi.fn(),
   mockGetCredentialActorContext: vi.fn(),
@@ -31,6 +33,8 @@ const {
   mockGetClientCredentialAccountDescriptor: vi.fn(() => undefined),
   mockDeleteConnectionCredential: vi.fn(),
   mockDeleteOrphanedOAuthAccount: vi.fn(),
+  mockDeleteWorkspaceEnvCredentials: vi.fn(),
+  mockSyncPersonalEnvCredentialsForUser: vi.fn(),
 }))
 
 vi.mock('@sim/audit', () => ({
@@ -57,8 +61,8 @@ vi.mock('@/lib/credentials/deletion', () => ({
   deleteOrphanedOAuthAccount: mockDeleteOrphanedOAuthAccount,
 }))
 vi.mock('@/lib/credentials/environment', () => ({
-  deleteWorkspaceEnvCredentials: vi.fn(),
-  syncPersonalEnvCredentialsForUser: vi.fn(),
+  deleteWorkspaceEnvCredentials: mockDeleteWorkspaceEnvCredentials,
+  syncPersonalEnvCredentialsForUser: mockSyncPersonalEnvCredentialsForUser,
 }))
 vi.mock('@/lib/credentials/atlassian-service-account', () => ({
   AtlassianValidationError: class AtlassianValidationError extends Error {},
@@ -713,6 +717,61 @@ describe('deleteCredentialRecord', () => {
     expect(mockDeleteConnectionCredential).not.toHaveBeenCalled()
   })
 
+  /**
+   * The whole variables map is read, edited and written back here, so a
+   * concurrent secret write is lost unless this holds the same advisory lock
+   * every other writer of that map takes.
+   */
+  it('removes a workspace env value under the map lock, with the row', async () => {
+    await deleteCredentialRecord({
+      credential: {
+        id: 'cred-1',
+        workspaceId: 'ws-1',
+        type: 'env_workspace',
+        envKey: 'STRIPE_API_KEY',
+        providerId: null,
+      } as never,
+      reason: 'user_delete',
+    })
+
+    expect(dbChainMockFns.transaction).toHaveBeenCalled()
+    const locked = dbChainMockFns.execute.mock.calls.some(([statement]) => {
+      const { sql, params } = (
+        statement as { toSQL: () => { sql: string; params: unknown[] } }
+      ).toSQL()
+      return sql.includes('pg_advisory_xact_lock') && params.includes('ws-1')
+    })
+    expect(locked).toBe(true)
+    // Passed the transaction, so the row cannot outlive the value it describes.
+    expect(mockDeleteWorkspaceEnvCredentials).toHaveBeenCalledWith(
+      expect.objectContaining({ workspaceId: 'ws-1', removedKeys: ['STRIPE_API_KEY'] })
+    )
+    expect(mockDeleteWorkspaceEnvCredentials.mock.calls[0][0].executor).toBeDefined()
+  })
+
+  it('removes a personal env value under the map lock', async () => {
+    await deleteCredentialRecord({
+      credential: {
+        id: 'cred-1',
+        workspaceId: 'ws-1',
+        type: 'env_personal',
+        envKey: 'MY_KEY',
+        envOwnerUserId: 'user-1',
+        providerId: null,
+      } as never,
+      reason: 'user_delete',
+    })
+
+    expect(dbChainMockFns.transaction).toHaveBeenCalled()
+    const locked = dbChainMockFns.execute.mock.calls.some(([statement]) => {
+      const { sql, params } = (
+        statement as { toSQL: () => { sql: string; params: unknown[] } }
+      ).toSQL()
+      return sql.includes('pg_advisory_xact_lock') && params.includes('user-1')
+    })
+    expect(locked).toBe(true)
+  })
+
   it('revokes the backing OAuth grant of a deleted oauth credential', async () => {
     mockDeleteConnectionCredential.mockResolvedValueOnce(true)
 
diff --git a/apps/sim/lib/credentials/orchestration/index.ts b/apps/sim/lib/credentials/orchestration/index.ts
index 7b449c3a5cd..9334e9c0c9a 100644
--- a/apps/sim/lib/credentials/orchestration/index.ts
+++ b/apps/sim/lib/credentials/orchestration/index.ts
@@ -30,6 +30,7 @@ import {
   deleteOrphanedOAuthAccount,
 } from '@/lib/credentials/deletion'
 import { slackCustomBotDisplayName } from '@/lib/credentials/display-name'
+import { lockPersonalEnvMap, lockWorkspaceEnvMap } from '@/lib/credentials/env-locks'
 import {
   deleteWorkspaceEnvCredentials,
   syncPersonalEnvCredentialsForUser,
@@ -565,25 +566,39 @@ export async function deleteCredentialRecord(
     if (!credentialRow.envKey || !credentialRow.envOwnerUserId) {
       throw new Error('Personal environment credential is missing its source identity')
     }
-    const [personalRow] = await db
-      .select({ variables: environment.variables })
-      .from(environment)
-      .where(eq(environment.userId, credentialRow.envOwnerUserId))
-      .limit(1)
-    const current = { ...((personalRow?.variables as Record<string, string> | null) ?? {}) }
-    delete current[credentialRow.envKey]
-    await db
-      .insert(environment)
-      .values({
-        id: credentialRow.envOwnerUserId,
-        userId: credentialRow.envOwnerUserId,
-        variables: current,
-        updatedAt: new Date(),
-      })
-      .onConflictDoUpdate({
-        target: [environment.userId],
-        set: { variables: current, updatedAt: new Date() },
-      })
+    const { envKey, envOwnerUserId } = credentialRow
+    /**
+     * Same read-modify-write on the personal map, under the same lock its
+     * other writers take. The credential reconcile stays outside: it opens its
+     * own transaction and takes the user-identity fence, so nesting it here
+     * would have two transactions taking two locks in opposite orders. It is a
+     * reconcile against the stored keys, so a failure is repaired by the next
+     * one rather than entrenched.
+     */
+    const current = await db.transaction(async (tx) => {
+      await lockPersonalEnvMap(tx, envOwnerUserId)
+
+      const [personalRow] = await tx
+        .select({ variables: environment.variables })
+        .from(environment)
+        .where(eq(environment.userId, envOwnerUserId))
+        .limit(1)
+      const variables = { ...((personalRow?.variables as Record<string, string> | null) ?? {}) }
+      delete variables[envKey]
+      await tx
+        .insert(environment)
+        .values({
+          id: envOwnerUserId,
+          userId: envOwnerUserId,
+          variables,
+          updatedAt: new Date(),
+        })
+        .onConflictDoUpdate({
+          target: [environment.userId],
+          set: { variables, updatedAt: new Date() },
+        })
+      return variables
+    })
     await syncPersonalEnvCredentialsForUser({
       userId: credentialRow.envOwnerUserId,
       envKeys: Object.keys(current),
@@ -595,33 +610,46 @@ export async function deleteCredentialRecord(
     if (!credentialRow.envKey) {
       throw new Error('Workspace environment credential is missing its source identity')
     }
-    const [workspaceRow] = await db
-      .select({
-        id: workspaceEnvironment.id,
-        createdAt: workspaceEnvironment.createdAt,
-        variables: workspaceEnvironment.variables,
-      })
-      .from(workspaceEnvironment)
-      .where(eq(workspaceEnvironment.workspaceId, credentialRow.workspaceId))
-      .limit(1)
-    const current = { ...((workspaceRow?.variables as Record<string, string> | null) ?? {}) }
-    delete current[credentialRow.envKey]
-    await db
-      .insert(workspaceEnvironment)
-      .values({
-        id: workspaceRow?.id ?? generateId(),
-        workspaceId: credentialRow.workspaceId,
-        variables: current,
-        createdAt: workspaceRow?.createdAt ?? new Date(),
-        updatedAt: new Date(),
-      })
-      .onConflictDoUpdate({
-        target: [workspaceEnvironment.workspaceId],
-        set: { variables: current, updatedAt: new Date() },
+    const { envKey, workspaceId } = credentialRow
+    /**
+     * The whole variables map is read, edited and written back, so this has to
+     * hold the same lock every other writer of that map takes — without it a
+     * secret written concurrently is read before the write and dropped by this
+     * write-back. The credential row goes in the same transaction so the row
+     * and the value it describes cannot outlive each other.
+     */
+    await db.transaction(async (tx) => {
+      await lockWorkspaceEnvMap(tx, workspaceId)
+
+      const [workspaceRow] = await tx
+        .select({
+          id: workspaceEnvironment.id,
+          createdAt: workspaceEnvironment.createdAt,
+          variables: workspaceEnvironment.variables,
+        })
+        .from(workspaceEnvironment)
+        .where(eq(workspaceEnvironment.workspaceId, workspaceId))
+        .limit(1)
+      const current = { ...((workspaceRow?.variables as Record<string, string> | null) ?? {}) }
+      delete current[envKey]
+      await tx
+        .insert(workspaceEnvironment)
+        .values({
+          id: workspaceRow?.id ?? generateId(),
+          workspaceId,
+          variables: current,
+          createdAt: workspaceRow?.createdAt ?? new Date(),
+          updatedAt: new Date(),
+        })
+        .onConflictDoUpdate({
+          target: [workspaceEnvironment.workspaceId],
+          set: { variables: current, updatedAt: new Date() },
+        })
+      await deleteWorkspaceEnvCredentials({
+        workspaceId,
+        removedKeys: [envKey],
+        executor: tx,
       })
-    await deleteWorkspaceEnvCredentials({
-      workspaceId: credentialRow.workspaceId,
-      removedKeys: [credentialRow.envKey],
     })
     return true
   }
diff --git a/apps/sim/lib/credentials/secret-values.ts b/apps/sim/lib/credentials/secret-values.ts
index aae67ca55a6..9d73cf6a629 100644
--- a/apps/sim/lib/credentials/secret-values.ts
+++ b/apps/sim/lib/credentials/secret-values.ts
@@ -1,31 +1,22 @@
 import { db } from '@sim/db'
 import { credential, environment, workspaceEnvironment } from '@sim/db/schema'
 import { generateId } from '@sim/utils/id'
-import { and, eq, sql } from 'drizzle-orm'
+import { and, eq } from 'drizzle-orm'
 import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
+import { lockPersonalEnvMap, lockWorkspaceEnvMap } from '@/lib/credentials/env-locks'
 import {
   createWorkspaceEnvCredentials,
   deletePersonalEnvCredentialForUser,
   deleteWorkspaceEnvCredentials,
   upsertPersonalEnvCredentialForUser,
 } from '@/lib/credentials/environment'
-import type { DbOrTx } from '@/lib/db/types'
 import { invalidateEffectiveDecryptedEnvCache } from '@/lib/environment/utils'
 
-const SECRET_MAP_LOCK_TIMEOUT_MS = 5_000
-
 export interface SecretMutationResult {
   created: boolean
   updatedAt: Date
 }
 
-async function lockSecretMap(tx: DbOrTx, lockKey: string): Promise<void> {
-  await tx.execute(
-    sql`SELECT set_config('lock_timeout', ${`${SECRET_MAP_LOCK_TIMEOUT_MS}ms`}, true)`
-  )
-  await tx.execute(sql`SELECT pg_advisory_xact_lock(hashtextextended(${lockKey}, 0))`)
-}
-
 /**
  * Decrypts the stored values for the requested workspace secret names.
  *
@@ -84,7 +75,7 @@ export async function setWorkspaceSecret(params: {
   const updatedAt = new Date()
 
   const created = await db.transaction(async (tx) => {
-    await lockSecretMap(tx, workspaceId)
+    await lockWorkspaceEnvMap(tx, workspaceId)
     const [row] = await tx
       .select({
         id: workspaceEnvironment.id,
@@ -201,7 +192,7 @@ export async function setPersonalSecret(params: {
   const updatedAt = new Date()
 
   const created = await db.transaction(async (tx) => {
-    await lockSecretMap(tx, userId)
+    await lockPersonalEnvMap(tx, userId)
     const [row] = await tx
       .select({ id: environment.id, variables: environment.variables })
       .from(environment)
@@ -248,7 +239,7 @@ export async function deleteWorkspaceSecret(params: {
   const { workspaceId, name } = params
 
   const deleted = await db.transaction(async (tx) => {
-    await lockSecretMap(tx, workspaceId)
+    await lockWorkspaceEnvMap(tx, workspaceId)
     const [row] = await tx
       .select({ variables: workspaceEnvironment.variables })
       .from(workspaceEnvironment)
@@ -284,7 +275,7 @@ export async function deletePersonalSecret(params: {
   const { userId, name } = params
 
   const deleted = await db.transaction(async (tx) => {
-    await lockSecretMap(tx, userId)
+    await lockPersonalEnvMap(tx, userId)
     const [row] = await tx
       .select({ variables: environment.variables })
       .from(environment)
diff --git a/apps/sim/lib/environment/utils.ts b/apps/sim/lib/environment/utils.ts
index a0e033ae34e..b8a0da05de0 100644
--- a/apps/sim/lib/environment/utils.ts
+++ b/apps/sim/lib/environment/utils.ts
@@ -4,9 +4,10 @@ import { environment, workspaceEnvironment } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { getErrorMessage } from '@sim/utils/errors'
 import { generateId } from '@sim/utils/id'
-import { eq, inArray, sql } from 'drizzle-orm'
+import { eq, inArray } from 'drizzle-orm'
 import { LRUCache } from 'lru-cache'
 import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
+import { lockWorkspaceEnvMap } from '@/lib/credentials/env-locks'
 import {
   createWorkspaceEnvCredentials,
   getAccessibleEnvCredentials,
@@ -512,11 +513,8 @@ export async function upsertWorkspaceEnvVars(
 
   // Read-modify-write on a single jsonb column, so serialize against the
   // route's identically-locked transaction or concurrent writers lose keys.
-  const existingEncrypted = await db.transaction(async (tx) => {
-    await tx.execute(
-      sql`SELECT set_config('lock_timeout', ${`${WORKSPACE_ENV_LOCK_TIMEOUT_MS}ms`}, true)`
-    )
-    await tx.execute(sql`SELECT pg_advisory_xact_lock(hashtextextended(${workspaceId}, 0))`)
+  await db.transaction(async (tx) => {
+    await lockWorkspaceEnvMap(tx, workspaceId)
 
     const [existingRow] = await tx
       .select()
@@ -540,15 +538,23 @@ export async function upsertWorkspaceEnvVars(
         set: { variables: merged, updatedAt: new Date() },
       })
 
-    return existing
+    // Derived from the stored variables, not from the credential rows: a legacy
+    // secret present in the jsonb map without a credential row is NOT new, and
+    // minting an ACL for it would make the caller its secret-admin.
+    //
+    // Written inside this transaction because a value committed without its
+    // credential row cannot be repaired by retrying: the key is in the map by
+    // then, so the next attempt reads it as pre-existing and creates nothing.
+    const newKeys = updatedKeys.filter((key) => !(key in existing))
+    await createWorkspaceEnvCredentials({
+      workspaceId,
+      newKeys,
+      actingUserId,
+      executor: tx,
+    })
   })
 
   invalidateEffectiveDecryptedEnvCache({ workspaceId })
-  // Derived from the stored variables, not from the credential rows: a legacy
-  // secret present in the jsonb map without a credential row is NOT new, and
-  // minting an ACL for it would make the caller its secret-admin.
-  const newKeys = updatedKeys.filter((key) => !(key in existingEncrypted))
-  await createWorkspaceEnvCredentials({ workspaceId, newKeys, actingUserId })
 
   recordAudit({
     workspaceId,

From a62dd795d4ecc941fb58ec778e00b3e47d120acb Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Wed, 26 Aug 2026 23:08:55 -0700
Subject: [PATCH 2/5] fix(workflows): say when a block is dropped before
 persistence
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`workflow_blocks.name` is NOT NULL, so a block missing `type` or `name` has
to be dropped — but it was dropped silently. A block with no edges left no
trace anywhere: not in the returned warnings, not in a log line. The client
sanitizer warns on the identical condition; this is its server counterpart,
and the warnings array it feeds is already returned by the internal PUT, the
v2 write and the importer.
---
 .../persistence/prepare-state.test.ts          |  7 +++++--
 .../lib/workflows/persistence/prepare-state.ts | 18 +++++++++++++++---
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/apps/sim/lib/workflows/persistence/prepare-state.test.ts b/apps/sim/lib/workflows/persistence/prepare-state.test.ts
index 98405fe82cf..292e8233a3c 100644
--- a/apps/sim/lib/workflows/persistence/prepare-state.test.ts
+++ b/apps/sim/lib/workflows/persistence/prepare-state.test.ts
@@ -36,8 +36,8 @@ describe('prepareWorkflowStateForPersistence', () => {
     expect(warnings.some((w) => w.includes('dangling'))).toBe(true)
   })
 
-  it('drops blocks missing type or name', () => {
-    const { state } = prepareWorkflowStateForPersistence({
+  it('drops blocks missing type or name, and says which', () => {
+    const { state, warnings } = prepareWorkflowStateForPersistence({
       blocks: {
         ok: block({ id: 'ok' }),
         noType: block({ id: 'noType', type: '' }),
@@ -47,6 +47,9 @@ describe('prepareWorkflowStateForPersistence', () => {
     })
 
     expect(Object.keys(state.blocks)).toEqual(['ok'])
+    // A block with no edges left no other trace of having been dropped.
+    expect(warnings).toContain('Dropped block "noType": missing type or name')
+    expect(warnings).toContain('Dropped block "noName": missing type or name')
   })
 
   it('backfills the columns the normalized tables require', () => {
diff --git a/apps/sim/lib/workflows/persistence/prepare-state.ts b/apps/sim/lib/workflows/persistence/prepare-state.ts
index e6d4ec74a86..6593bb10a93 100644
--- a/apps/sim/lib/workflows/persistence/prepare-state.ts
+++ b/apps/sim/lib/workflows/persistence/prepare-state.ts
@@ -25,8 +25,9 @@ export interface PrepareWorkflowStateResult {
  *
  * The steps are order-dependent:
  * 1. Strip secrets from inline agent-tool definitions.
- * 2. Drop blocks missing `type`/`name` and backfill the columns the tables
- *    require, so a partial block cannot violate a NOT NULL constraint.
+ * 2. Drop blocks missing `type`/`name` — reporting each one — and backfill the
+ *    columns the tables require, so a partial block cannot violate a NOT NULL
+ *    constraint.
  * 3. Drop edges whose endpoints no longer resolve — `workflow_edges` has
  *    foreign keys onto `workflow_blocks`, so a dangling edge would otherwise
  *    abort the whole transaction with an opaque database error.
@@ -42,8 +43,18 @@ export function prepareWorkflowStateForPersistence(state: {
   )
 
   const blocks: Record<string, BlockState> = {}
+  const droppedBlockWarnings: string[] = []
   for (const [blockId, block] of Object.entries(sanitizedBlocks)) {
-    if (!block.type || !block.name) continue
+    /**
+     * Reported, not just skipped: `workflow_blocks.name` is NOT NULL so the
+     * drop has to happen, but a block with no edges left no trace anywhere and
+     * simply vanished from the saved workflow. The client-side sanitizer warns
+     * on the identical condition; this is its server-side counterpart.
+     */
+    if (!block.type || !block.name) {
+      droppedBlockWarnings.push(`Dropped block "${blockId}": missing type or name`)
+      continue
+    }
     blocks[blockId] = {
       ...block,
       enabled: block.enabled !== undefined ? block.enabled : true,
@@ -65,6 +76,7 @@ export function prepareWorkflowStateForPersistence(state: {
     },
     warnings: [
       ...sanitizationWarnings,
+      ...droppedBlockWarnings,
       ...validatedEdges.dropped.map(({ edge, reason }) => `Dropped edge "${edge.id}": ${reason}`),
     ],
   }

From 29db9159b09c34fe26515c6480c87e1b6a04695c Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Wed, 26 Aug 2026 23:08:55 -0700
Subject: [PATCH 3/5] improvement(chat): stop loading a transcript the v2 route
 never reads
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Nothing caps a chat transcript — no per-chat message limit on write, no
pruning — and the v2 route keys continuity by `chatId`, so it read the whole
thing on every resumed turn and dropped it. Opt out there. The load stays the
default because the copilot send path does consume it.
---
 apps/sim/app/api/v2/chat/route.ts      |  1 +
 apps/sim/lib/copilot/chat/lifecycle.ts | 23 +++++++++++++++++++----
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/apps/sim/app/api/v2/chat/route.ts b/apps/sim/app/api/v2/chat/route.ts
index 7c5f097f068..cc646312f44 100644
--- a/apps/sim/app/api/v2/chat/route.ts
+++ b/apps/sim/app/api/v2/chat/route.ts
@@ -177,6 +177,7 @@ export const POST = withRouteHandler(
       // Chat block do, both of which post a single message with a chat id.
       const resolvedChat = await resolveOrCreateChat({
         ...(conversationId ? { chatId: conversationId } : {}),
+        includeTranscript: false,
         userId,
         workspaceId,
         model: MOTHERSHIP_CHAT_DEFAULT_MODEL,
diff --git a/apps/sim/lib/copilot/chat/lifecycle.ts b/apps/sim/lib/copilot/chat/lifecycle.ts
index b6701675be3..34dbed2bdc2 100644
--- a/apps/sim/lib/copilot/chat/lifecycle.ts
+++ b/apps/sim/lib/copilot/chat/lifecycle.ts
@@ -212,7 +212,8 @@ export async function getAccessibleCopilotChat(
  */
 export async function getAccessibleCopilotChatWithMessages(
   chatId: string,
-  userId: string
+  userId: string,
+  options?: { includeTranscript?: boolean }
 ): Promise<CopilotChatDetailRow | null> {
   const [chat] = await db
     .select(copilotChatDetailColumns)
@@ -223,7 +224,14 @@ export async function getAccessibleCopilotChatWithMessages(
   const authorized = await authorizeCopilotChatRow(chat, chatId, userId)
   if (!authorized) return null
 
-  const messages = await loadCopilotChatMessages(chatId)
+  /**
+   * The transcript is unbounded — no per-chat message cap on write and no
+   * pruning — so a caller that only needs the chat's scope should not pay to
+   * materialize it. Every check `resolveOrCreateChat` runs reads detail
+   * columns only, so an empty list stays a truthful "not loaded" rather than
+   * "no messages" for the callers that opt out.
+   */
+  const messages = options?.includeTranscript === false ? [] : await loadCopilotChatMessages(chatId)
   return { ...authorized, messages }
 }
 
@@ -245,15 +253,22 @@ export async function resolveOrCreateChat(params: {
   model: string
   type?: 'mothership' | 'copilot'
   title?: string
+  /**
+   * Skips loading the transcript on the resume path. For a caller that keys
+   * continuity by `chatId` alone and never reads `conversationHistory`.
+   */
+  includeTranscript?: boolean
 }): Promise<ChatLoadResult> {
-  const { chatId, userId, workflowId, workspaceId, model, type, title } = params
+  const { chatId, userId, workflowId, workspaceId, model, type, title, includeTranscript } = params
 
   if (workspaceId) {
     await assertActiveWorkspaceAccess(workspaceId, userId)
   }
 
   if (chatId) {
-    const chat = await getAccessibleCopilotChatWithMessages(chatId, userId)
+    const chat = await getAccessibleCopilotChatWithMessages(chatId, userId, {
+      includeTranscript,
+    })
 
     if (chat) {
       if (workflowId && chat.workflowId !== workflowId) {

From 5c4dbed1e096a70759ad63440d4a43c1199bfa77 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Wed, 26 Aug 2026 23:27:13 -0700
Subject: [PATCH 4/5] fix(credentials): serialize every personal env map writer

Exporting the personal lock while two writers skipped it left the map
unserialized: `upsertPersonalEnvVars` merged against a read taken outside any
lock, and the settings PUT replaced the map wholesale. A wholesale replace
landing between another writer's read and its write-back is discarded whole,
so it takes the lock too.

The delete path now removes the key's mirrors directly instead of reconciling
against a key list. The reconcile prunes every mirror absent from that list,
so a secret added between the read and the prune lost its mirror while its
value survived. `setPersonalSecret` already takes the map lock and then the
user-identity fence inside it, so the targeted delete introduces no new lock
order.
---
 apps/sim/app/api/environment/route.ts         | 37 +++++++++------
 .../credentials/orchestration/index.test.ts   | 15 +++++--
 .../lib/credentials/orchestration/index.ts    | 22 +++++----
 apps/sim/lib/environment/utils.ts             | 45 +++++++++++++------
 4 files changed, 77 insertions(+), 42 deletions(-)

diff --git a/apps/sim/app/api/environment/route.ts b/apps/sim/app/api/environment/route.ts
index 75162c53e0b..ee9ce281393 100644
--- a/apps/sim/app/api/environment/route.ts
+++ b/apps/sim/app/api/environment/route.ts
@@ -12,6 +12,7 @@ import { getSession } from '@/lib/auth'
 import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
+import { lockPersonalEnvMap } from '@/lib/credentials/env-locks'
 import { syncPersonalEnvCredentialsForUser } from '@/lib/credentials/environment'
 import type { EnvironmentVariable } from '@/lib/environment/api'
 import { captureServerEvent } from '@/lib/posthog/server'
@@ -53,21 +54,31 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
       })
     ).then((entries) => Object.fromEntries(entries))
 
-    await db
-      .insert(environment)
-      .values({
-        id: generateId(),
-        userId: session.user.id,
-        variables: encryptedVariables,
-        updatedAt: new Date(),
-      })
-      .onConflictDoUpdate({
-        target: [environment.userId],
-        set: {
+    /**
+     * A wholesale replace still takes the map lock: without it this can land
+     * between another writer's read and its write-back, and that writer then
+     * persists a map derived from the pre-replace state, discarding this one
+     * entirely. The reconcile below matches the replace, so it stays outside.
+     */
+    await db.transaction(async (tx) => {
+      await lockPersonalEnvMap(tx, session.user.id)
+
+      await tx
+        .insert(environment)
+        .values({
+          id: generateId(),
+          userId: session.user.id,
           variables: encryptedVariables,
           updatedAt: new Date(),
-        },
-      })
+        })
+        .onConflictDoUpdate({
+          target: [environment.userId],
+          set: {
+            variables: encryptedVariables,
+            updatedAt: new Date(),
+          },
+        })
+    })
 
     await syncPersonalEnvCredentialsForUser({
       userId: session.user.id,
diff --git a/apps/sim/lib/credentials/orchestration/index.test.ts b/apps/sim/lib/credentials/orchestration/index.test.ts
index 2c0bab8b3ba..54510cd4902 100644
--- a/apps/sim/lib/credentials/orchestration/index.test.ts
+++ b/apps/sim/lib/credentials/orchestration/index.test.ts
@@ -21,7 +21,7 @@ const {
   mockDeleteConnectionCredential,
   mockDeleteOrphanedOAuthAccount,
   mockDeleteWorkspaceEnvCredentials,
-  mockSyncPersonalEnvCredentialsForUser,
+  mockDeletePersonalEnvCredentialForUser,
 } = vi.hoisted(() => ({
   mockRecordAudit: vi.fn(),
   mockGetCredentialActorContext: vi.fn(),
@@ -34,7 +34,7 @@ const {
   mockDeleteConnectionCredential: vi.fn(),
   mockDeleteOrphanedOAuthAccount: vi.fn(),
   mockDeleteWorkspaceEnvCredentials: vi.fn(),
-  mockSyncPersonalEnvCredentialsForUser: vi.fn(),
+  mockDeletePersonalEnvCredentialForUser: vi.fn(),
 }))
 
 vi.mock('@sim/audit', () => ({
@@ -62,7 +62,7 @@ vi.mock('@/lib/credentials/deletion', () => ({
 }))
 vi.mock('@/lib/credentials/environment', () => ({
   deleteWorkspaceEnvCredentials: mockDeleteWorkspaceEnvCredentials,
-  syncPersonalEnvCredentialsForUser: mockSyncPersonalEnvCredentialsForUser,
+  deletePersonalEnvCredentialForUser: mockDeletePersonalEnvCredentialForUser,
 }))
 vi.mock('@/lib/credentials/atlassian-service-account', () => ({
   AtlassianValidationError: class AtlassianValidationError extends Error {},
@@ -770,6 +770,15 @@ describe('deleteCredentialRecord', () => {
       return sql.includes('pg_advisory_xact_lock') && params.includes('user-1')
     })
     expect(locked).toBe(true)
+    /**
+     * Targeted, not a reconcile against a key list: a list read before the
+     * prune can miss a secret added since, and prune that secret's mirror
+     * while its value survives.
+     */
+    expect(mockDeletePersonalEnvCredentialForUser).toHaveBeenCalledWith(
+      expect.objectContaining({ userId: 'user-1', envKey: 'MY_KEY' })
+    )
+    expect(mockDeletePersonalEnvCredentialForUser.mock.calls[0][0].executor).toBeDefined()
   })
 
   it('revokes the backing OAuth grant of a deleted oauth credential', async () => {
diff --git a/apps/sim/lib/credentials/orchestration/index.ts b/apps/sim/lib/credentials/orchestration/index.ts
index 9334e9c0c9a..aae2b276e45 100644
--- a/apps/sim/lib/credentials/orchestration/index.ts
+++ b/apps/sim/lib/credentials/orchestration/index.ts
@@ -32,8 +32,8 @@ import {
 import { slackCustomBotDisplayName } from '@/lib/credentials/display-name'
 import { lockPersonalEnvMap, lockWorkspaceEnvMap } from '@/lib/credentials/env-locks'
 import {
+  deletePersonalEnvCredentialForUser,
   deleteWorkspaceEnvCredentials,
-  syncPersonalEnvCredentialsForUser,
 } from '@/lib/credentials/environment'
 import type { ServiceAccountFieldId } from '@/lib/credentials/service-account-fields'
 import {
@@ -569,13 +569,15 @@ export async function deleteCredentialRecord(
     const { envKey, envOwnerUserId } = credentialRow
     /**
      * Same read-modify-write on the personal map, under the same lock its
-     * other writers take. The credential reconcile stays outside: it opens its
-     * own transaction and takes the user-identity fence, so nesting it here
-     * would have two transactions taking two locks in opposite orders. It is a
-     * reconcile against the stored keys, so a failure is repaired by the next
-     * one rather than entrenched.
+     * other writers take, with the mirrors removed in the same transaction.
+     *
+     * Targeted rather than a reconcile: the reconcile prunes every mirror
+     * absent from a caller-supplied key list, so a secret added between the
+     * read and the prune lost its mirror while its value survived. Deleting
+     * this one key's mirrors cannot strand another secret, and the lock order
+     * — map, then user identity — is the one `setPersonalSecret` already takes.
      */
-    const current = await db.transaction(async (tx) => {
+    await db.transaction(async (tx) => {
       await lockPersonalEnvMap(tx, envOwnerUserId)
 
       const [personalRow] = await tx
@@ -597,11 +599,7 @@ export async function deleteCredentialRecord(
           target: [environment.userId],
           set: { variables, updatedAt: new Date() },
         })
-      return variables
-    })
-    await syncPersonalEnvCredentialsForUser({
-      userId: credentialRow.envOwnerUserId,
-      envKeys: Object.keys(current),
+      await deletePersonalEnvCredentialForUser({ userId: envOwnerUserId, envKey, executor: tx })
     })
     return true
   }
diff --git a/apps/sim/lib/environment/utils.ts b/apps/sim/lib/environment/utils.ts
index b8a0da05de0..b75bcd009c9 100644
--- a/apps/sim/lib/environment/utils.ts
+++ b/apps/sim/lib/environment/utils.ts
@@ -7,7 +7,7 @@ import { generateId } from '@sim/utils/id'
 import { eq, inArray } from 'drizzle-orm'
 import { LRUCache } from 'lru-cache'
 import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
-import { lockWorkspaceEnvMap } from '@/lib/credentials/env-locks'
+import { lockPersonalEnvMap, lockWorkspaceEnvMap } from '@/lib/credentials/env-locks'
 import {
   createWorkspaceEnvCredentials,
   getAccessibleEnvCredentials,
@@ -435,20 +435,37 @@ export async function upsertPersonalEnvVars(
     newlyEncrypted[key] = encrypted
   }
 
-  const finalEncrypted = { ...existingEncrypted, ...newlyEncrypted }
+  /**
+   * The read above only decides which values changed; the merge has to be made
+   * against a read taken under the lock, or a key written concurrently is
+   * absent from this map and dropped by the write-back.
+   */
+  const finalEncrypted = await db.transaction(async (tx) => {
+    await lockPersonalEnvMap(tx, userId)
 
-  await db
-    .insert(environment)
-    .values({
-      id: generateId(),
-      userId,
-      variables: finalEncrypted,
-      updatedAt: new Date(),
-    })
-    .onConflictDoUpdate({
-      target: [environment.userId],
-      set: { variables: finalEncrypted, updatedAt: new Date() },
-    })
+    const [currentRow] = await tx
+      .select({ variables: environment.variables })
+      .from(environment)
+      .where(eq(environment.userId, userId))
+      .limit(1)
+    const current = (currentRow?.variables as Record<string, string>) || {}
+    const finalEncrypted = { ...current, ...newlyEncrypted }
+
+    await tx
+      .insert(environment)
+      .values({
+        id: generateId(),
+        userId,
+        variables: finalEncrypted,
+        updatedAt: new Date(),
+      })
+      .onConflictDoUpdate({
+        target: [environment.userId],
+        set: { variables: finalEncrypted, updatedAt: new Date() },
+      })
+
+    return finalEncrypted
+  })
 
   invalidateEffectiveDecryptedEnvCache({ userId })
   await syncPersonalEnvCredentialsForUser({

From d7b1874b1a4cb44044167f40fbd8403ed1797e98 Mon Sep 17 00:00:00 2001
From: Waleed Latif <walif6@gmail.com>
Date: Thu, 27 Aug 2026 10:54:01 -0700
Subject: [PATCH 5/5] fix(credentials): chunk the credential-ACL write the env
 save now depends on
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`createWorkspaceEnvCredentials` wrote keys x members membership rows in one
statement, and neither side is bounded by the request contract. Past 65535
bind parameters that throws — previously a partial success, because the value
had already committed, but this now runs inside the value's transaction, so it
rolls the save back instead, deterministically, on every retry. A 50-member
workspace saving 150 keys reaches it. Chunked the same way the two personal
paths in this file already are.

Also from the audit:

- invalidate the decrypted-env cache after `deleteCredentialRecord` removes an
  env value, matching the dedicated delete paths; without it a deleted secret
  stayed resolvable for the cache TTL
- correct the comment claiming the personal reconcile "matches the replace" —
  it prunes against this request's key list, so a secret added after the commit
  still loses its mirror. Naming the gap instead of asserting it away
- name the one behavior change the in-transaction re-read introduces: a key
  whose submitted value already matched is not re-encrypted, so a concurrent
  write for that key now survives rather than being overwritten
- drop the lock-timeout constant and TSDoc left behind when the lock moved into
  the shared helper, and stop shadowing `finalEncrypted`
---
 apps/sim/app/api/environment/route.ts         |  9 ++-
 .../api/workspaces/[id]/environment/route.ts  |  5 --
 apps/sim/lib/credentials/environment.test.ts  | 39 ++++++++++++
 apps/sim/lib/credentials/environment.ts       | 61 +++++++++++--------
 .../lib/credentials/orchestration/index.ts    |  4 ++
 apps/sim/lib/environment/utils.ts             | 15 +++--
 6 files changed, 95 insertions(+), 38 deletions(-)

diff --git a/apps/sim/app/api/environment/route.ts b/apps/sim/app/api/environment/route.ts
index ee9ce281393..96aa61c9b87 100644
--- a/apps/sim/app/api/environment/route.ts
+++ b/apps/sim/app/api/environment/route.ts
@@ -58,7 +58,14 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
      * A wholesale replace still takes the map lock: without it this can land
      * between another writer's read and its write-back, and that writer then
      * persists a map derived from the pre-replace state, discarding this one
-     * entirely. The reconcile below matches the replace, so it stays outside.
+     * entirely.
+     *
+     * The reconcile below stays outside because it opens its own transaction.
+     * That leaves a known gap: it prunes mirrors against this request's key
+     * list, so a secret added after the commit loses its mirror while its
+     * value survives. Closing it means having the reconcile read the map
+     * itself rather than trust a caller's list, across all four of its
+     * callers.
      */
     await db.transaction(async (tx) => {
       await lockPersonalEnvMap(tx, session.user.id)
diff --git a/apps/sim/app/api/workspaces/[id]/environment/route.ts b/apps/sim/app/api/workspaces/[id]/environment/route.ts
index 7b81f89df36..4673fa1e47b 100644
--- a/apps/sim/app/api/workspaces/[id]/environment/route.ts
+++ b/apps/sim/app/api/workspaces/[id]/environment/route.ts
@@ -35,11 +35,6 @@ import {
 
 const logger = createLogger('WorkspaceEnvironmentAPI')
 
-/**
- * Bounds the workspace-environment advisory-lock wait so a stuck holder fails
- * fast (SQLSTATE 55P03) rather than hanging, even if the deployment lacks a
- * server-side `lock_timeout`. Transaction-scoped via `set_config(..., true)`.
- */
 /**
  * Restricts decrypted workspace env values to administrators. Members (including
  * read-only) receive the variable names with empty values so editor autocomplete
diff --git a/apps/sim/lib/credentials/environment.test.ts b/apps/sim/lib/credentials/environment.test.ts
index 349a94d5db0..11ae69d8ac1 100644
--- a/apps/sim/lib/credentials/environment.test.ts
+++ b/apps/sim/lib/credentials/environment.test.ts
@@ -15,6 +15,7 @@ vi.mock('@/lib/billing/organizations/billing-identity-lock', () => ({
 }))
 
 import {
+  createWorkspaceEnvCredentials,
   getPersonalEnvKeyRawAccess,
   getWorkspaceEnvKeyAdminAccess,
   syncPersonalEnvCredentialsForUser,
@@ -239,3 +240,41 @@ describe('syncPersonalEnvCredentialsForUser', () => {
     ])
   })
 })
+
+describe('createWorkspaceEnvCredentials', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    resetDbChainMock()
+  })
+
+  /**
+   * The membership row count is keys × members, and neither is bounded by the
+   * request contract. A single statement past Postgres's 65535 bind parameters
+   * throws — and because this now runs inside the value's transaction, that
+   * would roll back the save on every retry rather than half-committing it.
+   */
+  it('splits a keys x members write too wide for one statement', async () => {
+    const keys = Array.from({ length: 40 }, (_, i) => `KEY_${i}`)
+    queueTableRows(workspace, [{ ownerId: 'owner-1' }])
+    queueTableRows(
+      permissions,
+      Array.from({ length: 60 }, (_, i) => ({ userId: `member-${i}` }))
+    )
+    // Every chunk of the credential insert reports its rows back as created.
+    dbChainMockFns.returning.mockImplementation(() =>
+      Promise.resolve(keys.map((_, i) => ({ id: `credential-${i}` })))
+    )
+
+    await createWorkspaceEnvCredentials({
+      workspaceId: 'ws-1',
+      newKeys: keys,
+      actingUserId: 'member-0',
+    })
+
+    const rowsPerCall = dbChainMockFns.values.mock.calls.map(([rows]) =>
+      Array.isArray(rows) ? rows.length : 1
+    )
+    expect(rowsPerCall.length).toBeGreaterThan(1)
+    expect(Math.max(...rowsPerCall)).toBeLessThanOrEqual(500)
+  })
+})
diff --git a/apps/sim/lib/credentials/environment.ts b/apps/sim/lib/credentials/environment.ts
index eb7a739fefb..ad808306d30 100644
--- a/apps/sim/lib/credentials/environment.ts
+++ b/apps/sim/lib/credentials/environment.ts
@@ -17,7 +17,7 @@ import {
   hasWorkspaceAdminAccess,
 } from '@/lib/workspaces/permissions/utils'
 
-const PERSONAL_ENV_CREDENTIAL_WRITE_CHUNK_SIZE = 500
+const ENV_CREDENTIAL_WRITE_CHUNK_SIZE = 500
 
 export interface WorkspaceMembership {
   ownerId: string | null
@@ -437,27 +437,35 @@ export async function createWorkspaceEnvCredentials(params: {
 
   const now = params.updatedAt ?? new Date()
 
-  const inserted = await executor
-    .insert(credential)
-    .values(
-      keys.map((envKey) => ({
-        id: generateId(),
-        workspaceId,
-        type: 'env_workspace' as const,
-        displayName: envKey,
-        envKey,
-        createdBy: actingUserId,
-        createdAt: now,
-        updatedAt: now,
-      }))
-    )
-    .onConflictDoNothing()
-    .returning({ id: credential.id })
-  const createdIds = inserted.map((row) => row.id)
+  const credentialValues = keys.map((envKey) => ({
+    id: generateId(),
+    workspaceId,
+    type: 'env_workspace' as const,
+    displayName: envKey,
+    envKey,
+    createdBy: actingUserId,
+    createdAt: now,
+    updatedAt: now,
+  }))
+  const createdIds: string[] = []
+  for (const values of chunkArray(credentialValues, ENV_CREDENTIAL_WRITE_CHUNK_SIZE)) {
+    const inserted = await executor
+      .insert(credential)
+      .values(values)
+      .onConflictDoNothing()
+      .returning({ id: credential.id })
+    createdIds.push(...inserted.map((row) => row.id))
+  }
 
   if (createdIds.length === 0 || memberUserIds.length === 0) return
 
-  // Bulk-insert memberships for all new credentials × all workspace members in one query
+  /**
+   * Chunked because the row count is keys × members and neither side is
+   * bounded: a wide enough save exceeds Postgres's 65535 bind parameters and
+   * throws. Unchunked that was a partial success — the value was already
+   * committed — but this now runs inside the value's transaction, so it would
+   * roll the save back, deterministically, on every retry.
+   */
   const membershipValues = createdIds.flatMap((credentialId) =>
     memberUserIds.map((memberUserId) => ({
       id: generateId(),
@@ -472,7 +480,9 @@ export async function createWorkspaceEnvCredentials(params: {
     }))
   )
 
-  await executor.insert(credentialMember).values(membershipValues).onConflictDoNothing()
+  for (const values of chunkArray(membershipValues, ENV_CREDENTIAL_WRITE_CHUNK_SIZE)) {
+    await executor.insert(credentialMember).values(values).onConflictDoNothing()
+  }
 }
 
 /**
@@ -529,7 +539,7 @@ export async function upsertPersonalEnvCredentialForUser(params: {
       createdAt: updatedAt,
       updatedAt,
     }))
-    for (const values of chunkArray(credentialValues, PERSONAL_ENV_CREDENTIAL_WRITE_CHUNK_SIZE)) {
+    for (const values of chunkArray(credentialValues, ENV_CREDENTIAL_WRITE_CHUNK_SIZE)) {
       await tx.insert(credential).values(values).onConflictDoNothing()
     }
 
@@ -570,7 +580,7 @@ export async function upsertPersonalEnvCredentialForUser(params: {
       createdAt: updatedAt,
       updatedAt,
     }))
-    for (const values of chunkArray(membershipValues, PERSONAL_ENV_CREDENTIAL_WRITE_CHUNK_SIZE)) {
+    for (const values of chunkArray(membershipValues, ENV_CREDENTIAL_WRITE_CHUNK_SIZE)) {
       await tx
         .insert(credentialMember)
         .values(values)
@@ -696,7 +706,7 @@ export async function syncPersonalEnvCredentialsForUser(params: {
           updatedAt: now,
         }))
       )
-      for (const values of chunkArray(credentialValues, PERSONAL_ENV_CREDENTIAL_WRITE_CHUNK_SIZE)) {
+      for (const values of chunkArray(credentialValues, ENV_CREDENTIAL_WRITE_CHUNK_SIZE)) {
         await tx.insert(credential).values(values).onConflictDoNothing()
       }
 
@@ -724,10 +734,7 @@ export async function syncPersonalEnvCredentialsForUser(params: {
           createdAt: now,
           updatedAt: now,
         }))
-        for (const values of chunkArray(
-          membershipValues,
-          PERSONAL_ENV_CREDENTIAL_WRITE_CHUNK_SIZE
-        )) {
+        for (const values of chunkArray(membershipValues, ENV_CREDENTIAL_WRITE_CHUNK_SIZE)) {
           await tx
             .insert(credentialMember)
             .values(values)
diff --git a/apps/sim/lib/credentials/orchestration/index.ts b/apps/sim/lib/credentials/orchestration/index.ts
index aae2b276e45..47cc51e4927 100644
--- a/apps/sim/lib/credentials/orchestration/index.ts
+++ b/apps/sim/lib/credentials/orchestration/index.ts
@@ -601,6 +601,9 @@ export async function deleteCredentialRecord(
         })
       await deletePersonalEnvCredentialForUser({ userId: envOwnerUserId, envKey, executor: tx })
     })
+    // The value is gone; without this it stays resolvable from the cache for
+    // its TTL, as the dedicated delete paths already recognise.
+    invalidateEffectiveDecryptedEnvCache({ userId: envOwnerUserId })
     return true
   }
 
@@ -649,6 +652,7 @@ export async function deleteCredentialRecord(
         executor: tx,
       })
     })
+    invalidateEffectiveDecryptedEnvCache({ workspaceId })
     return true
   }
 
diff --git a/apps/sim/lib/environment/utils.ts b/apps/sim/lib/environment/utils.ts
index b75bcd009c9..ea10b6d1546 100644
--- a/apps/sim/lib/environment/utils.ts
+++ b/apps/sim/lib/environment/utils.ts
@@ -21,7 +21,6 @@ import {
 } from '@/lib/workspaces/permissions/utils'
 
 const logger = createLogger('EnvironmentUtils')
-const WORKSPACE_ENV_LOCK_TIMEOUT_MS = 5_000
 const EFFECTIVE_ENVIRONMENT_CACHE_TTL_MS = 2_000
 const EFFECTIVE_ENVIRONMENT_CACHE_MAX_ENTRIES = 1_000
 
@@ -439,6 +438,12 @@ export async function upsertPersonalEnvVars(
    * The read above only decides which values changed; the merge has to be made
    * against a read taken under the lock, or a key written concurrently is
    * absent from this map and dropped by the write-back.
+   *
+   * One consequence worth naming: a key whose submitted value already matched
+   * the earlier read is not re-encrypted, so a value written concurrently for
+   * that key now survives instead of being overwritten with the identical
+   * plaintext. `added`/`updated` describe the earlier read and are reporting
+   * only — the keys actually written are exactly the re-encrypted ones.
    */
   const finalEncrypted = await db.transaction(async (tx) => {
     await lockPersonalEnvMap(tx, userId)
@@ -449,22 +454,22 @@ export async function upsertPersonalEnvVars(
       .where(eq(environment.userId, userId))
       .limit(1)
     const current = (currentRow?.variables as Record<string, string>) || {}
-    const finalEncrypted = { ...current, ...newlyEncrypted }
+    const merged = { ...current, ...newlyEncrypted }
 
     await tx
       .insert(environment)
       .values({
         id: generateId(),
         userId,
-        variables: finalEncrypted,
+        variables: merged,
         updatedAt: new Date(),
       })
       .onConflictDoUpdate({
         target: [environment.userId],
-        set: { variables: finalEncrypted, updatedAt: new Date() },
+        set: { variables: merged, updatedAt: new Date() },
       })
 
-    return finalEncrypted
+    return merged
   })
 
   invalidateEffectiveDecryptedEnvCache({ userId })