Skip to content

Commit 9ae8ba8

Browse files
fix(sandbox): preserve workbench output provenance (#8485)
* fix(sandbox): preserve workbench output provenance * fix(sandbox): preserve boundary compatibility
1 parent 41adb36 commit 9ae8ba8

28 files changed

Lines changed: 1085 additions & 282 deletions

‎apps/sim/lib/credentials/secret-values.test.ts‎

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,11 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
1616

1717
vi.mock('@/lib/core/security/encryption', () => encryptionMock)
1818
const mockEncryptSecret = encryptionMockFns.mockEncryptSecret
19-
const mockDecryptSecret = encryptionMockFns.mockDecryptSecret
2019
vi.mock('@/lib/credentials/environment', () => credentialsEnvironmentMock)
2120

2221
import {
2322
deletePersonalSecret,
2423
deleteWorkspaceSecret,
25-
readWorkspaceSecretValues,
2624
setWorkspaceSecret,
2725
updateWorkspaceSecretMetadata,
2826
} from '@/lib/credentials/secret-values'
@@ -116,51 +114,6 @@ describe('secret value storage', () => {
116114
})
117115
})
118116

119-
describe('readWorkspaceSecretValues', () => {
120-
beforeEach(() => {
121-
resetDbChainMock()
122-
mockDecryptSecret.mockImplementation(async (encrypted: string) => ({
123-
decrypted: `decrypted:${encrypted}`,
124-
}))
125-
})
126-
127-
it('decrypts only the requested names and omits absent or undecryptable ones', async () => {
128-
queueTableRows(schemaMock.workspaceEnvironment, [
129-
{
130-
id: 'env-1',
131-
variables: {
132-
VISIBLE_KEY: 'encrypted-visible',
133-
BROKEN_KEY: 'encrypted-broken',
134-
OTHER_KEY: 'encrypted-other',
135-
},
136-
},
137-
])
138-
mockDecryptSecret.mockImplementation(async (encrypted: string) => {
139-
if (encrypted === 'encrypted-broken') throw new Error('cannot decrypt')
140-
return { decrypted: `decrypted:${encrypted}` }
141-
})
142-
143-
await expect(
144-
readWorkspaceSecretValues({
145-
workspaceId: 'workspace-1',
146-
names: ['VISIBLE_KEY', 'BROKEN_KEY', 'MISSING_KEY'],
147-
})
148-
).resolves.toEqual({ VISIBLE_KEY: 'decrypted:encrypted-visible' })
149-
expect(mockDecryptSecret).not.toHaveBeenCalledWith('encrypted-other')
150-
})
151-
152-
it('never reads an inherited prototype member for a missing key', async () => {
153-
queueTableRows(schemaMock.workspaceEnvironment, [
154-
{ id: 'env-1', variables: { OTHER_KEY: 'encrypted-other' } },
155-
])
156-
157-
await expect(
158-
readWorkspaceSecretValues({ workspaceId: 'workspace-1', names: ['constructor', 'toString'] })
159-
).resolves.toEqual({})
160-
expect(mockDecryptSecret).not.toHaveBeenCalled()
161-
})
162-
})
163-
164117
/**
165118
* The row-queue mocks resolve whatever was queued regardless of the predicate, so
166119
* the only way to pin a WHERE clause is to read the condition tree the `eq`/`and`

‎apps/sim/lib/credentials/secret-values.ts‎

Lines changed: 1 addition & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { db } from '@sim/db'
22
import { credential, environment, workspaceEnvironment } from '@sim/db/schema'
33
import { generateId } from '@sim/utils/id'
44
import { and, eq } from 'drizzle-orm'
5-
import { decryptSecret, encryptSecret } from '@/lib/core/security/encryption'
5+
import { encryptSecret } from '@/lib/core/security/encryption'
66
import { lockPersonalEnvMap, lockWorkspaceEnvMap } from '@/lib/credentials/env-locks'
77
import {
88
createWorkspaceEnvCredentials,
@@ -17,44 +17,6 @@ export interface SecretMutationResult {
1717
updatedAt: Date
1818
}
1919

20-
/**
21-
* Decrypts the stored values for the requested workspace secret names.
22-
*
23-
* Exists for exactly one read path: rows a workspace marked visible (unredacted),
24-
* whose values already print into every run log the caller can open. Every other
25-
* secret read stays metadata-only — callers gate on the flag BEFORE asking. A
26-
* name that is absent or fails to decrypt is omitted rather than failing the
27-
* batch, since the value is optional on the wire.
28-
*/
29-
export async function readWorkspaceSecretValues(params: {
30-
workspaceId: string
31-
names: readonly string[]
32-
}): Promise<Record<string, string>> {
33-
if (params.names.length === 0) return {}
34-
35-
const [row] = await db
36-
.select({ variables: workspaceEnvironment.variables })
37-
.from(workspaceEnvironment)
38-
.where(eq(workspaceEnvironment.workspaceId, params.workspaceId))
39-
.limit(1)
40-
const variables = (row?.variables as Record<string, string> | null) ?? {}
41-
42-
const values: Record<string, string> = {}
43-
await Promise.all(
44-
params.names.map(async (name) => {
45-
const encrypted = Object.hasOwn(variables, name) ? variables[name] : undefined
46-
if (!encrypted) return
47-
try {
48-
const { decrypted } = await decryptSecret(encrypted)
49-
values[name] = decrypted
50-
} catch {
51-
// Omitted from the result; the caller's wire shape treats the value as optional.
52-
}
53-
})
54-
)
55-
return values
56-
}
57-
5820
/** Stores one workspace secret without decrypting any existing value. */
5921
export async function setWorkspaceSecret(params: {
6022
workspaceId: string

‎apps/sim/lib/execution/remote-sandbox/execution-observer.ts‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { AsyncLocalStorage } from 'node:async_hooks'
2+
import type { DurableSecretProvenance } from '@/lib/execution/durable-secret-provenance'
23
import type { SessionProcessIdentity } from '@/lib/execution/remote-sandbox/session-process'
34

45
interface SandboxExecutionObserver {
5-
sessionInputsSafe?(): boolean
6+
sessionInputProvenance?(): boolean | DurableSecretProvenance
67
hold(work: Promise<unknown>): void
78
unsettled(processId?: string): void
89
claimProcess?(process: SessionProcessIdentity): Promise<void>
@@ -49,20 +50,25 @@ export async function prepareSandboxSessionAccess(
4950
}
5051

5152
/** The trusted tool adapter supplies current input evidence while preserving execution ownership. */
52-
export function observeSandboxSessionInputs<T>(safe: () => boolean, execute: () => T): T {
53+
export function observeSandboxSessionInputs<T>(
54+
safe: () => boolean | DurableSecretProvenance,
55+
execute: () => T
56+
): T {
5357
const current = executionObserver.getStore()
5458
return executionObserver.run(
5559
{
5660
hold: (work) => current?.hold(work),
5761
unsettled: (id) => current?.unsettled(id),
5862
...current,
59-
sessionInputsSafe: safe,
63+
sessionInputProvenance: safe,
6064
},
6165
execute
6266
)
6367
}
6468

65-
/** Unobserved arbitrary code cannot certify scratch files as safe. */
66-
export function sandboxSessionInputsSafe(): boolean {
67-
return executionObserver.getStore()?.sessionInputsSafe?.() === true
69+
/** Trusted input evidence is sampled immediately before the machine receives the bytes. */
70+
export function sandboxSessionInputProvenance(): DurableSecretProvenance {
71+
const value = executionObserver.getStore()?.sessionInputProvenance?.()
72+
if (typeof value === 'object') return value
73+
return value === true ? { status: 'exact', entries: [] } : { status: 'unknown' }
6874
}

‎apps/sim/lib/execution/remote-sandbox/index.ts‎

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
prepareSandboxSessionAccess,
2121
reportUnsettledSandboxProcess,
2222
retainSandboxExecution,
23-
sandboxSessionInputsSafe,
23+
sandboxSessionInputProvenance,
2424
} from '@/lib/execution/remote-sandbox/execution-observer'
2525
import { withSandboxFilePublication } from '@/lib/execution/remote-sandbox/file-publication'
2626
import {
@@ -55,7 +55,10 @@ import {
5555
SESSION_SANDBOX_IDLE_MS,
5656
} from '@/lib/execution/remote-sandbox/session'
5757
import { sessionCommandPath } from '@/lib/execution/remote-sandbox/session-cli'
58-
import { recordSessionFileInput } from '@/lib/execution/remote-sandbox/session-file-provenance'
58+
import {
59+
readSessionSecretProvenance,
60+
recordSessionFileInput,
61+
} from '@/lib/execution/remote-sandbox/session-file-provenance'
5962
import { withSandboxSessionLock } from '@/lib/execution/remote-sandbox/session-lock'
6063
import type {
6164
CreateSandboxOptions,
@@ -842,6 +845,18 @@ async function provisionWithinBudget(
842845
throwIfAborted(signal)
843846
}
844847

848+
/** Confidentiality checks also run on provider failures, before their diagnostics can escape. */
849+
async function acceptSessionOutputHistory(
850+
session: SandboxSessionRequest | undefined,
851+
machine: { providerId: SandboxProviderId; sandboxId: string }
852+
): Promise<void> {
853+
if (!session) return
854+
const provenance = await readSessionSecretProvenance(session.key, machine)
855+
if (session.acceptOutputProvenance) await session.acceptOutputProvenance(provenance)
856+
else if (provenance.status !== 'exact' || provenance.entries.length > 0)
857+
throw new Error('Workbench output withheld because its secret provenance is unavailable')
858+
}
859+
845860
async function executeInSandboxWithinBudget(
846861
// The budget wrapper always injects the signal; the required-signal type states that
847862
// invariant instead of a cast hiding it.
@@ -887,13 +902,13 @@ async function executeInSandboxWithinBudget(
887902
// the finally below. Dependencies land before the inputs so user code and its
888903
// mounts always see a complete environment.
889904
//
890-
if (req.session)
905+
if (lease.session && req.session)
891906
await recordSessionFileInput(
892907
req.session.key,
893908
{ providerId: created.providerId, sandboxId },
894-
sandboxSessionInputsSafe() &&
895-
!req.session.unprovenancedInputs &&
896-
!Object.keys(selected?.envs ?? {}).length
909+
req.session.unprovenancedInputs || Object.keys(selected?.envs ?? {}).length
910+
? { status: 'unknown' }
911+
: (req.session.inputProvenance?.() ?? sandboxSessionInputProvenance())
897912
)
898913
await provisionWithinBudget(sandbox, selected, signal)
899914
await writeSandboxInputs(sandbox, req.sandboxFiles, {
@@ -1025,8 +1040,15 @@ async function executeInSandboxWithinBudget(
10251040
if (cost && billableOutputError) {
10261041
attachTrustedSandboxOutputCost(billableOutputError, cost)
10271042
}
1028-
await privateInputFiles?.cleanup()
1029-
await lease.release()
1043+
try {
1044+
await privateInputFiles?.cleanup()
1045+
await lease.release()
1046+
} finally {
1047+
await acceptSessionOutputHistory(lease.session ? req.session : undefined, {
1048+
providerId: created.providerId,
1049+
sandboxId,
1050+
})
1051+
}
10301052
}
10311053
}
10321054

@@ -1076,13 +1098,13 @@ async function executeShellInSandboxWithinBudget(
10761098
// Inside the try so a failed install or mount still releases the sandbox via
10771099
// the finally below. The install shares the caller's budget rather than adding
10781100
// to it — see the note in `executeInSandbox`.
1079-
if (req.session)
1101+
if (lease.session && req.session)
10801102
await recordSessionFileInput(
10811103
req.session.key,
10821104
{ providerId: created.providerId, sandboxId },
1083-
sandboxSessionInputsSafe() &&
1084-
!req.session.unprovenancedInputs &&
1085-
!Object.keys(selected?.envs ?? {}).length
1105+
req.session.unprovenancedInputs || Object.keys(selected?.envs ?? {}).length
1106+
? { status: 'unknown' }
1107+
: (req.session.inputProvenance?.() ?? sandboxSessionInputProvenance())
10861108
)
10871109
await provisionWithinBudget(sandbox, selected, signal)
10881110
await writeSandboxInputs(sandbox, req.sandboxFiles, {
@@ -1192,8 +1214,15 @@ async function executeShellInSandboxWithinBudget(
11921214
if (cost && billableOutputError) {
11931215
attachTrustedSandboxOutputCost(billableOutputError, cost)
11941216
}
1195-
await privateInputFiles?.cleanup()
1196-
await lease.release()
1217+
try {
1218+
await privateInputFiles?.cleanup()
1219+
await lease.release()
1220+
} finally {
1221+
await acceptSessionOutputHistory(lease.session ? req.session : undefined, {
1222+
providerId: created.providerId,
1223+
sandboxId,
1224+
})
1225+
}
11971226
}
11981227
}
11991228

0 commit comments

Comments
 (0)