Skip to content

Commit d328092

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(selectors): harden exact reference handling
1 parent 8075e62 commit d328092

10 files changed

Lines changed: 476 additions & 74 deletions

File tree

apps/sim/lib/environment/utils.test.ts

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import {
5454
getExecutionEnvironment,
5555
getPersonalAndWorkspaceEnv,
5656
invalidateEffectiveDecryptedEnvCache,
57+
resolveEffectiveEnvironmentVariables,
5758
upsertWorkspaceEnvVars,
5859
WorkspaceEnvAccessError,
5960
} from '@/lib/environment/utils'
@@ -175,6 +176,172 @@ describe('getEffectiveEnvironmentVariableNames', () => {
175176
})
176177
})
177178

179+
describe('resolveEffectiveEnvironmentVariables', () => {
180+
beforeEach(() => {
181+
vi.clearAllMocks()
182+
resetDbChainMock()
183+
invalidateEffectiveDecryptedEnvCache({ userId: 'resolver-user' })
184+
mockCheckWorkspaceAccess.mockResolvedValue({
185+
exists: true,
186+
hasAccess: true,
187+
canWrite: true,
188+
canAdmin: false,
189+
})
190+
mockGetAccessibleEnvCredentials.mockResolvedValue([])
191+
encryptionMockFns.mockDecryptSecret.mockReset()
192+
})
193+
194+
it('decrypts only unique requested accessible values with workspace precedence', async () => {
195+
mockGetAccessibleEnvCredentials.mockResolvedValue([
196+
{
197+
type: 'env_workspace',
198+
envKey: 'VISIBLE_SHARED',
199+
envOwnerUserId: null,
200+
updatedAt: new Date('2026-01-01T00:00:00.000Z'),
201+
unredacted: true,
202+
},
203+
{
204+
type: 'env_workspace',
205+
envKey: 'HIDDEN_SHARED',
206+
envOwnerUserId: null,
207+
updatedAt: new Date('2026-01-01T00:00:00.000Z'),
208+
unredacted: false,
209+
},
210+
{
211+
type: 'env_workspace',
212+
envKey: 'DUPLICATE',
213+
envOwnerUserId: null,
214+
updatedAt: new Date('2026-01-01T00:00:00.000Z'),
215+
unredacted: false,
216+
},
217+
{
218+
type: 'env_workspace',
219+
envKey: 'BROKEN',
220+
envOwnerUserId: null,
221+
updatedAt: new Date('2026-01-01T00:00:00.000Z'),
222+
unredacted: false,
223+
},
224+
{
225+
type: 'env_personal',
226+
envKey: 'SHARED_PERSONAL',
227+
envOwnerUserId: 'owner-2',
228+
updatedAt: new Date('2026-01-01T00:00:00.000Z'),
229+
},
230+
])
231+
queueTableRows(environment, [
232+
{
233+
variables: {
234+
OWN_PERSONAL: 'own-cipher',
235+
DUPLICATE: 'personal-shadow-cipher',
236+
UNREQUESTED_PERSONAL: 'unrequested-personal-cipher',
237+
},
238+
},
239+
])
240+
queueTableRows(workspaceEnvironment, [
241+
{
242+
variables: {
243+
VISIBLE_SHARED: 'visible-cipher',
244+
HIDDEN_SHARED: 'hidden-cipher',
245+
DUPLICATE: 'workspace-cipher',
246+
BROKEN: 'broken-cipher',
247+
INACCESSIBLE: 'inaccessible-cipher',
248+
UNREQUESTED_WORKSPACE: 'unrequested-workspace-cipher',
249+
},
250+
},
251+
])
252+
queueTableRows(environment, [
253+
{ userId: 'owner-2', variables: { SHARED_PERSONAL: 'shared-personal-cipher' } },
254+
])
255+
encryptionMockFns.mockDecryptSecret.mockImplementation(async (encryptedValue: string) => {
256+
if (encryptedValue === 'broken-cipher') throw new Error('cannot decrypt')
257+
return { decrypted: `plain:${encryptedValue}` }
258+
})
259+
260+
await expect(
261+
resolveEffectiveEnvironmentVariables('resolver-user', 'workspace-1', [
262+
'OWN_PERSONAL',
263+
'SHARED_PERSONAL',
264+
'VISIBLE_SHARED',
265+
'HIDDEN_SHARED',
266+
'DUPLICATE',
267+
'DUPLICATE',
268+
'BROKEN',
269+
'MISSING',
270+
'INACCESSIBLE',
271+
'constructor',
272+
])
273+
).resolves.toEqual({
274+
OWN_PERSONAL: {
275+
value: 'plain:own-cipher',
276+
scope: 'personal',
277+
visible: true,
278+
},
279+
SHARED_PERSONAL: {
280+
value: 'plain:shared-personal-cipher',
281+
scope: 'personal',
282+
visible: false,
283+
},
284+
VISIBLE_SHARED: {
285+
value: 'plain:visible-cipher',
286+
scope: 'workspace',
287+
visible: true,
288+
},
289+
HIDDEN_SHARED: {
290+
value: 'plain:hidden-cipher',
291+
scope: 'workspace',
292+
visible: false,
293+
},
294+
DUPLICATE: {
295+
value: 'plain:workspace-cipher',
296+
scope: 'workspace',
297+
visible: false,
298+
},
299+
})
300+
expect(encryptionMockFns.mockDecryptSecret.mock.calls.map(([value]) => value)).toEqual([
301+
'own-cipher',
302+
'shared-personal-cipher',
303+
'visible-cipher',
304+
'hidden-cipher',
305+
'workspace-cipher',
306+
'broken-cipher',
307+
])
308+
})
309+
310+
it('performs a fresh lookup without reading or warming the snapshot cache', async () => {
311+
encryptionMockFns.mockDecryptSecret.mockImplementation(async (encryptedValue: string) => ({
312+
decrypted: `plain:${encryptedValue}`,
313+
}))
314+
315+
queueTableRows(environment, [{ variables: { ROTATING: 'first-cipher' } }])
316+
queueTableRows(workspaceEnvironment, [{ variables: {} }])
317+
await expect(
318+
resolveEffectiveEnvironmentVariables('resolver-user', 'workspace-1', ['ROTATING'])
319+
).resolves.toEqual({
320+
ROTATING: { value: 'plain:first-cipher', scope: 'personal', visible: true },
321+
})
322+
323+
queueTableRows(environment, [{ variables: { ROTATING: 'snapshot-cipher' } }])
324+
queueTableRows(workspaceEnvironment, [{ variables: {} }])
325+
await expect(
326+
getEffectiveEnvironmentSnapshot('resolver-user', 'workspace-1')
327+
).resolves.toMatchObject({ personalDecrypted: { ROTATING: 'plain:snapshot-cipher' } })
328+
329+
queueTableRows(environment, [{ variables: { ROTATING: 'fresh-cipher' } }])
330+
queueTableRows(workspaceEnvironment, [{ variables: {} }])
331+
await expect(
332+
resolveEffectiveEnvironmentVariables('resolver-user', 'workspace-1', ['ROTATING'])
333+
).resolves.toEqual({
334+
ROTATING: { value: 'plain:fresh-cipher', scope: 'personal', visible: true },
335+
})
336+
337+
await expect(
338+
getEffectiveEnvironmentSnapshot('resolver-user', 'workspace-1')
339+
).resolves.toMatchObject({ personalDecrypted: { ROTATING: 'plain:snapshot-cipher' } })
340+
expect(encryptionMockFns.mockDecryptSecret).toHaveBeenCalledTimes(3)
341+
expect(mockCheckWorkspaceAccess).toHaveBeenCalledTimes(3)
342+
})
343+
})
344+
178345
describe('getPersonalAndWorkspaceEnv access filtering', () => {
179346
beforeEach(() => {
180347
vi.clearAllMocks()

apps/sim/lib/environment/utils.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,67 @@ export async function getEffectiveEnvironmentVariableNames(
279279
].sort()
280280
}
281281

282+
export interface ResolvedEnvironmentVariable {
283+
value: string
284+
scope: 'personal' | 'workspace'
285+
visible: boolean
286+
}
287+
288+
/**
289+
* Resolves only the requested environment variables through a fresh ACL-aware lookup.
290+
*
291+
* This deliberately neither reads nor populates the runtime environment snapshot cache.
292+
* Workspace values take precedence over personal values, matching normal resolution. Missing,
293+
* inaccessible, and undecryptable values are all omitted so callers cannot distinguish them.
294+
*/
295+
export async function resolveEffectiveEnvironmentVariables(
296+
userId: string,
297+
workspaceId: string | undefined,
298+
requestedNames: readonly string[]
299+
): Promise<Record<string, ResolvedEnvironmentVariable>> {
300+
const names = [...new Set(requestedNames)]
301+
if (names.length === 0) return {}
302+
303+
const { personalEncrypted, workspaceEncrypted, personalOwners, workspaceUnredactedKeys } =
304+
await loadAccessibleEncryptedEnvironment(userId, workspaceId)
305+
const visibleWorkspaceNames = new Set(workspaceUnredactedKeys)
306+
307+
const resolvedEntries = await Promise.all(
308+
names.map(async (name) => {
309+
const fromWorkspace = Object.hasOwn(workspaceEncrypted, name)
310+
const fromPersonal = Object.hasOwn(personalEncrypted, name)
311+
const encrypted = fromWorkspace
312+
? workspaceEncrypted[name]
313+
: fromPersonal
314+
? personalEncrypted[name]
315+
: undefined
316+
if (encrypted === undefined) return null
317+
318+
try {
319+
const { decrypted } = await decryptSecret(encrypted)
320+
return [
321+
name,
322+
{
323+
value: decrypted,
324+
scope: fromWorkspace ? 'workspace' : 'personal',
325+
visible: fromWorkspace
326+
? visibleWorkspaceNames.has(name)
327+
: personalOwners[name] === userId,
328+
},
329+
] as const
330+
} catch {
331+
return null
332+
}
333+
})
334+
)
335+
336+
return Object.fromEntries(
337+
resolvedEntries.filter(
338+
(entry): entry is readonly [string, ResolvedEnvironmentVariable] => entry !== null
339+
)
340+
)
341+
}
342+
282343
export async function getPersonalAndWorkspaceEnv(
283344
userId: string,
284345
workspaceId?: string,

apps/sim/lib/selectors/application/execute-selector.test.ts

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -170,10 +170,22 @@ describe('executeSelector', () => {
170170
expect(logged).not.toContain('context')
171171
})
172172

173-
it('restores exact detail-id repeats from reference provenance before sanitization', async () => {
173+
it.each([
174+
{
175+
name: 'restores exact detail-id repeats after sanitization',
176+
referenceName: 'GOOGLE_FILE_ID',
177+
resolvedId: 'resolved-file-id',
178+
},
179+
{
180+
name: 'restores a reference whose spelling overlaps its resolved ID',
181+
referenceName: 'ID',
182+
resolvedId: 'ID',
183+
},
184+
])('$name', async ({ referenceName, resolvedId }) => {
174185
const { sanitizeSelectorResult } = await vi.importActual<
175186
typeof import('@/lib/selectors/server/sanitize')
176187
>('@/lib/selectors/server/sanitize')
188+
const originalId = `{{${referenceName}}}`
177189

178190
mocks.resolveScope.mockImplementationOnce(async () => {
179191
mocks.events.push('canonical-scope')
@@ -188,16 +200,16 @@ describe('executeSelector', () => {
188200
})
189201
mocks.resolveReferences.mockImplementationOnce(async ({ protectedValues }) => {
190202
mocks.events.push('reference-resolution')
191-
protectedValues.add('resolved-file-id')
203+
protectedValues.add(resolvedId)
192204
return {
193205
context: { oauthCredential: 'credential-1' },
194-
request: { kind: 'detail', id: 'resolved-file-id' },
206+
request: { kind: 'detail', id: resolvedId },
195207
references: new Map([
196208
[
197209
'request.id',
198210
{
199211
field: 'request.id',
200-
name: 'GOOGLE_FILE_ID',
212+
name: referenceName,
201213
scope: 'workspace',
202214
visible: false,
203215
},
@@ -210,38 +222,38 @@ describe('executeSelector', () => {
210222
return {
211223
kind: 'detail',
212224
item: {
213-
id: 'resolved-file-id',
214-
label: 'resolved-file-id',
215-
meta: { resourceId: 'resolved-file-id', mimeType: 'application/pdf' },
225+
id: resolvedId,
226+
label: resolvedId,
227+
meta: { resourceId: resolvedId, mimeType: 'application/pdf' },
216228
},
217229
}
218230
})
219-
mocks.sanitize.mockImplementationOnce((result, protectedValues) => {
231+
mocks.sanitize.mockImplementationOnce((result, protectedValues, options) => {
220232
mocks.events.push('sanitization')
221233
expect(result).toEqual({
222234
kind: 'detail',
223235
item: {
224-
id: '{{GOOGLE_FILE_ID}}',
225-
label: '{{GOOGLE_FILE_ID}}',
226-
meta: { resourceId: '{{GOOGLE_FILE_ID}}', mimeType: 'application/pdf' },
236+
id: resolvedId,
237+
label: resolvedId,
238+
meta: { resourceId: resolvedId, mimeType: 'application/pdf' },
227239
},
228240
})
229-
expect(JSON.stringify(result)).not.toContain('resolved-file-id')
230-
expect(protectedValues.contains('resolved-file-id')).toBe(true)
231-
return sanitizeSelectorResult(result, protectedValues)
241+
expect(protectedValues.contains(resolvedId)).toBe(true)
242+
expect(options).toEqual({ allowedDetailExactProtectedValue: resolvedId })
243+
return sanitizeSelectorResult(result, protectedValues, options)
232244
})
233245

234246
await expect(
235247
execute({
236248
selectorKey: 'google.drive',
237-
request: { kind: 'detail', id: '{{GOOGLE_FILE_ID}}' },
249+
request: { kind: 'detail', id: originalId },
238250
})
239251
).resolves.toEqual({
240252
kind: 'detail',
241253
item: {
242-
id: '{{GOOGLE_FILE_ID}}',
243-
label: '{{GOOGLE_FILE_ID}}',
244-
meta: { resourceId: '{{GOOGLE_FILE_ID}}', mimeType: 'application/pdf' },
254+
id: originalId,
255+
label: originalId,
256+
meta: { resourceId: originalId, mimeType: 'application/pdf' },
245257
},
246258
})
247259
})

0 commit comments

Comments
 (0)