Skip to content

Commit 7b84a79

Browse files
committed
fix(security): eliminate workspace env lost-update race with atomic JSONB ops
PUT: use `variables || excluded.variables` in onConflictDoUpdate so concurrent writes merge atomically in the DB instead of last-writer-wins at the application layer. DELETE: replace the read-modify-write upsert with a single UPDATE that removes keys via the JSONB `-` operator, preventing concurrent deletes from resurrecting previously-removed secrets.
1 parent 398dc65 commit 7b84a79

1 file changed

Lines changed: 14 additions & 16 deletions

File tree

  • apps/sim/app/api/workspaces/[id]/environment

apps/sim/app/api/workspaces/[id]/environment/route.ts

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { db } from '@sim/db'
33
import { workspaceEnvironment } from '@sim/db/schema'
44
import { createLogger } from '@sim/logger'
55
import { generateId } from '@sim/utils/id'
6-
import { eq } from 'drizzle-orm'
6+
import { eq, sql } from 'drizzle-orm'
77
import { type NextRequest, NextResponse } from 'next/server'
88
import {
99
removeWorkspaceEnvironmentContract,
@@ -113,23 +113,24 @@ export const PUT = withRouteHandler(
113113
})
114114
).then((entries) => Object.fromEntries(entries))
115115

116-
const merged = { ...existingEncrypted, ...encryptedIncoming }
117-
118-
// Upsert by unique workspace_id
119116
await db
120117
.insert(workspaceEnvironment)
121118
.values({
122119
id: generateId(),
123120
workspaceId,
124-
variables: merged,
121+
variables: encryptedIncoming,
125122
createdAt: new Date(),
126123
updatedAt: new Date(),
127124
})
128125
.onConflictDoUpdate({
129126
target: [workspaceEnvironment.workspaceId],
130-
set: { variables: merged, updatedAt: new Date() },
127+
set: {
128+
variables: sql`${workspaceEnvironment.variables} || excluded.variables`,
129+
updatedAt: new Date(),
130+
},
131131
})
132132

133+
const merged = { ...existingEncrypted, ...encryptedIncoming }
133134
const newKeys = Object.keys(variables).filter((k) => !(k in existingEncrypted))
134135
await createWorkspaceEnvCredentials({ workspaceId, newKeys, actingUserId: userId })
135136

@@ -203,18 +204,15 @@ export const DELETE = withRouteHandler(
203204
}
204205

205206
await db
206-
.insert(workspaceEnvironment)
207-
.values({
208-
id: wsRows[0]?.id || generateId(),
209-
workspaceId,
210-
variables: current,
211-
createdAt: wsRows[0]?.createdAt || new Date(),
207+
.update(workspaceEnvironment)
208+
.set({
209+
variables: sql`${workspaceEnvironment.variables} - ARRAY[${sql.join(
210+
keys.map((k) => sql`${k}`),
211+
sql`, `
212+
)}]::text[]`,
212213
updatedAt: new Date(),
213214
})
214-
.onConflictDoUpdate({
215-
target: [workspaceEnvironment.workspaceId],
216-
set: { variables: current, updatedAt: new Date() },
217-
})
215+
.where(eq(workspaceEnvironment.workspaceId, workspaceId))
218216

219217
await deleteWorkspaceEnvCredentials({ workspaceId, removedKeys: keys })
220218

0 commit comments

Comments
 (0)