Skip to content

Commit d98bd8c

Browse files
committed
fix(settings): fix handlePaste silent swallow, quote-strip bug, and credential sync efficiency
- Move e.preventDefault() inside parsedVars guard in handlePaste so KEY= lines don't silently discard input (mirrors handleWorkspacePaste fix from same PR) - Add value.length >= 2 guard before quote-stripping to prevent single-char values like KEY=\" from being stripped to empty and silently dropped - Introduce createWorkspaceEnvCredentials and deleteWorkspaceEnvCredentials for delta-aware credential sync (O(k) instead of O(n*m) for env var mutations) - Fix createWorkspaceEnvCredentials early-return bug that skipped credential record creation when workspace had zero members - Update credentials/[id] DELETE to use deleteWorkspaceEnvCredentials instead of full syncWorkspaceEnvCredentials - Optimize syncWorkspaceEnvCredentials to fetch workspace+member IDs in parallel once instead of once per credential
1 parent f252c9e commit d98bd8c

File tree

5 files changed

+130
-35
lines changed

5 files changed

+130
-35
lines changed

apps/sim/app/api/credentials/[id]/route.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ import { getSession } from '@/lib/auth'
1010
import { encryptSecret } from '@/lib/core/security/encryption'
1111
import { getCredentialActorContext } from '@/lib/credentials/access'
1212
import {
13+
deleteWorkspaceEnvCredentials,
1314
syncPersonalEnvCredentialsForUser,
14-
syncWorkspaceEnvCredentials,
1515
} from '@/lib/credentials/environment'
1616
import { captureServerEvent } from '@/lib/posthog/server'
1717

@@ -317,10 +317,9 @@ export async function DELETE(
317317
set: { variables: current, updatedAt: new Date() },
318318
})
319319

320-
await syncWorkspaceEnvCredentials({
320+
await deleteWorkspaceEnvCredentials({
321321
workspaceId: access.credential.workspaceId,
322-
envKeys: Object.keys(current),
323-
actingUserId: session.user.id,
322+
removedKeys: [access.credential.envKey],
324323
})
325324

326325
captureServerEvent(

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

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ import { AuditAction, AuditResourceType, recordAudit } from '@/lib/audit/log'
99
import { getSession } from '@/lib/auth'
1010
import { encryptSecret } from '@/lib/core/security/encryption'
1111
import { generateRequestId } from '@/lib/core/utils/request'
12-
import { syncWorkspaceEnvCredentials } from '@/lib/credentials/environment'
12+
import {
13+
createWorkspaceEnvCredentials,
14+
deleteWorkspaceEnvCredentials,
15+
} from '@/lib/credentials/environment'
1316
import { getPersonalAndWorkspaceEnv } from '@/lib/environment/utils'
1417
import { getUserEntityPermissions, getWorkspaceById } from '@/lib/workspaces/permissions/utils'
1518

@@ -126,11 +129,8 @@ export async function PUT(request: NextRequest, { params }: { params: Promise<{
126129
set: { variables: merged, updatedAt: new Date() },
127130
})
128131

129-
await syncWorkspaceEnvCredentials({
130-
workspaceId,
131-
envKeys: Object.keys(merged),
132-
actingUserId: userId,
133-
})
132+
const newKeys = Object.keys(variables).filter((k) => !(k in existingEncrypted))
133+
await createWorkspaceEnvCredentials({ workspaceId, newKeys, actingUserId: userId })
134134

135135
recordAudit({
136136
workspaceId,
@@ -215,11 +215,7 @@ export async function DELETE(
215215
set: { variables: current, updatedAt: new Date() },
216216
})
217217

218-
await syncWorkspaceEnvCredentials({
219-
workspaceId,
220-
envKeys: Object.keys(current),
221-
actingUserId: userId,
222-
})
218+
await deleteWorkspaceEnvCredentials({ workspaceId, removedKeys: keys })
223219

224220
recordAudit({
225221
workspaceId,

apps/sim/app/workspace/[workspaceId]/settings/components/credentials/credentials-manager.tsx

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,10 @@ function parseEnvVarLine(line: string): UIEnvironmentVariable | null {
157157
value = value.trim()
158158

159159
if (
160-
(value.startsWith('"') && value.endsWith('"')) ||
161-
(value.startsWith("'") && value.endsWith("'")) ||
162-
(value.startsWith('`') && value.endsWith('`'))
160+
value.length >= 2 &&
161+
((value.startsWith('"') && value.endsWith('"')) ||
162+
(value.startsWith("'") && value.endsWith("'")) ||
163+
(value.startsWith('`') && value.endsWith('`')))
163164
) {
164165
value = value.slice(1, -1)
165166
}
@@ -904,7 +905,9 @@ export function CredentialsManager() {
904905

905906
/**
906907
* Paste handler for personal env var rows.
907-
* Always prevents default: KV patterns destructure into new rows; plain values overwrite the field.
908+
* Only prevents default when it actually handles the paste: KV patterns destructure into new rows,
909+
* plain values overwrite the field. Falls through to native paste if pattern is detected but all
910+
* values are empty (e.g. KEY=), avoiding silently swallowed input.
908911
*/
909912
const handlePaste = (e: React.ClipboardEvent<HTMLInputElement>, index: number) => {
910913
const text = e.clipboardData.getData('text').trim()
@@ -913,21 +916,24 @@ export function CredentialsManager() {
913916
const lines = text.split('\n').filter((line) => line.trim())
914917
if (lines.length === 0) return
915918

916-
e.preventDefault()
917-
918919
const inputType = (e.target as HTMLInputElement).getAttribute('data-input-type') as
919920
| 'key'
920921
| 'value'
921922

922923
if (inputType) {
923924
const hasValidEnvVarPattern = lines.some((line) => parseEnvVarLine(line) !== null)
924925
if (!hasValidEnvVarPattern) {
926+
e.preventDefault()
925927
handleSingleValuePaste(text, index, inputType)
926928
return
927929
}
928930
}
929931

930-
handleKeyValuePaste(lines)
932+
const parsedVars = parseValidEnvVars(lines)
933+
if (parsedVars.length > 0) {
934+
e.preventDefault()
935+
handleKeyValuePaste(lines)
936+
}
931937
}
932938

933939
/**

apps/sim/lib/credentials/environment.ts

Lines changed: 104 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,10 @@ async function upsertCredentialAdminMember(credentialId: string, adminUserId: st
100100

101101
async function ensureWorkspaceCredentialMemberships(
102102
credentialId: string,
103-
workspaceId: string,
103+
memberUserIds: string[],
104104
ownerUserId: string
105105
) {
106-
const workspaceMemberUserIds = await getWorkspaceMemberUserIds(workspaceId)
107-
if (!workspaceMemberUserIds.length) return
106+
if (!memberUserIds.length) return
108107

109108
const existingMemberships = await db
110109
.select({
@@ -117,14 +116,14 @@ async function ensureWorkspaceCredentialMemberships(
117116
.where(
118117
and(
119118
eq(credentialMember.credentialId, credentialId),
120-
inArray(credentialMember.userId, workspaceMemberUserIds)
119+
inArray(credentialMember.userId, memberUserIds)
121120
)
122121
)
123122

124123
const byUserId = new Map(existingMemberships.map((row) => [row.userId, row]))
125124
const now = new Date()
126125

127-
for (const memberUserId of workspaceMemberUserIds) {
126+
for (const memberUserId of memberUserIds) {
128127
const targetRole = memberUserId === ownerUserId ? 'admin' : 'member'
129128
const existing = byUserId.get(memberUserId)
130129
if (existing) {
@@ -164,11 +163,14 @@ export async function syncWorkspaceEnvCredentials(params: {
164163
actingUserId: string
165164
}) {
166165
const { workspaceId, envKeys, actingUserId } = params
167-
const [workspaceRow] = await db
168-
.select({ ownerId: workspace.ownerId })
169-
.from(workspace)
170-
.where(eq(workspace.id, workspaceId))
171-
.limit(1)
166+
const [[workspaceRow], memberUserIds] = await Promise.all([
167+
db
168+
.select({ ownerId: workspace.ownerId })
169+
.from(workspace)
170+
.where(eq(workspace.id, workspaceId))
171+
.limit(1),
172+
getWorkspaceMemberUserIds(workspaceId),
173+
])
172174

173175
if (!workspaceRow) return
174176

@@ -217,7 +219,7 @@ export async function syncWorkspaceEnvCredentials(params: {
217219
}
218220

219221
for (const credentialId of credentialIdsToEnsureMembership) {
220-
await ensureWorkspaceCredentialMemberships(credentialId, workspaceId, workspaceRow.ownerId)
222+
await ensureWorkspaceCredentialMemberships(credentialId, memberUserIds, workspaceRow.ownerId)
221223
}
222224

223225
if (normalizedKeys.length > 0) {
@@ -238,6 +240,97 @@ export async function syncWorkspaceEnvCredentials(params: {
238240
.where(and(eq(credential.workspaceId, workspaceId), eq(credential.type, 'env_workspace')))
239241
}
240242

243+
/**
244+
* Creates credential records and bulk-inserts memberships for newly added workspace env keys.
245+
* Use this instead of `syncWorkspaceEnvCredentials` when the caller knows exactly which keys are new.
246+
*/
247+
export async function createWorkspaceEnvCredentials(params: {
248+
workspaceId: string
249+
newKeys: string[]
250+
actingUserId: string
251+
}): Promise<void> {
252+
const { workspaceId, newKeys, actingUserId } = params
253+
const keys = Array.from(new Set(newKeys.filter(Boolean)))
254+
if (keys.length === 0) return
255+
256+
const [[workspaceRow], memberUserIds] = await Promise.all([
257+
db
258+
.select({ ownerId: workspace.ownerId })
259+
.from(workspace)
260+
.where(eq(workspace.id, workspaceId))
261+
.limit(1),
262+
getWorkspaceMemberUserIds(workspaceId),
263+
])
264+
265+
if (!workspaceRow) return
266+
267+
const ownerUserId = workspaceRow.ownerId
268+
const now = new Date()
269+
const createdIds: string[] = []
270+
271+
for (const envKey of keys) {
272+
const createdId = generateId()
273+
try {
274+
await db.insert(credential).values({
275+
id: createdId,
276+
workspaceId,
277+
type: 'env_workspace',
278+
displayName: envKey,
279+
envKey,
280+
createdBy: actingUserId,
281+
createdAt: now,
282+
updatedAt: now,
283+
})
284+
createdIds.push(createdId)
285+
} catch (error: unknown) {
286+
const code = getPostgresErrorCode(error)
287+
if (code !== '23505') throw error
288+
}
289+
}
290+
291+
if (createdIds.length === 0 || memberUserIds.length === 0) return
292+
293+
// Bulk-insert memberships for all new credentials × all workspace members in one query
294+
const membershipValues = createdIds.flatMap((credentialId) =>
295+
memberUserIds.map((memberUserId) => ({
296+
id: generateId(),
297+
credentialId,
298+
userId: memberUserId,
299+
role: (memberUserId === ownerUserId ? 'admin' : 'member') as 'admin' | 'member',
300+
status: 'active' as const,
301+
joinedAt: now,
302+
invitedBy: ownerUserId,
303+
createdAt: now,
304+
updatedAt: now,
305+
}))
306+
)
307+
308+
await db.insert(credentialMember).values(membershipValues).onConflictDoNothing()
309+
}
310+
311+
/**
312+
* Deletes credential records (and their memberships via cascade) for removed workspace env keys.
313+
* Use this instead of `syncWorkspaceEnvCredentials` when the caller knows exactly which keys were deleted.
314+
*/
315+
export async function deleteWorkspaceEnvCredentials(params: {
316+
workspaceId: string
317+
removedKeys: string[]
318+
}): Promise<void> {
319+
const { workspaceId, removedKeys } = params
320+
const keys = removedKeys.filter(Boolean)
321+
if (keys.length === 0) return
322+
323+
await db
324+
.delete(credential)
325+
.where(
326+
and(
327+
eq(credential.workspaceId, workspaceId),
328+
eq(credential.type, 'env_workspace'),
329+
inArray(credential.envKey, keys)
330+
)
331+
)
332+
}
333+
241334
export async function syncPersonalEnvCredentialsForUser(params: {
242335
userId: string
243336
envKeys: string[]

apps/sim/lib/environment/utils.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import { generateId } from '@sim/utils/id'
55
import { eq, inArray } from 'drizzle-orm'
66
import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
77
import {
8+
createWorkspaceEnvCredentials,
89
getAccessibleEnvCredentials,
910
syncPersonalEnvCredentialsForUser,
10-
syncWorkspaceEnvCredentials,
1111
} from '@/lib/credentials/environment'
1212

1313
const logger = createLogger('EnvironmentUtils')
@@ -305,7 +305,8 @@ export async function upsertWorkspaceEnvVars(
305305
set: { variables: merged, updatedAt: new Date() },
306306
})
307307

308-
await syncWorkspaceEnvCredentials({ workspaceId, envKeys: Object.keys(merged), actingUserId })
308+
const newKeys = Object.keys(newVars).filter((k) => !(k in existingWsEncrypted))
309+
await createWorkspaceEnvCredentials({ workspaceId, newKeys, actingUserId })
309310

310311
return updatedKeys
311312
}

0 commit comments

Comments
 (0)