Skip to content

Commit e09970a

Browse files
authored
fix(sandbox): withhold workbench certification after an unprovenanced file mount (#8456)
* fix(sandbox): withhold workbench certification after an unprovenanced file mount A persistent chat workbench stays "clean" only while every input it received was classified secret-free, and the scratch-file read hands its bytes to the model on that basis. File mounts resolved from platform file objects were counted only when a provenance source existed, so a mount whose key has no canonical metadata record (or no principal to bind one) left the machine certified. Both the `files` parameter and a mount marker in context variables reach this resolver from model-supplied Function parameters. The resolver now reports how many mounts had no provenance source. When a workbench session receives any, the session request carries `unprovenancedInputs` and the code boundary records the machine as unknown. Workflow runs have no session and keep their existing absence policy. * test(sandbox): certify workbench history against real Redis and close the mount bypass - Replace the certification unit test, which restated the history script in a fake Redis, with an integration suite that runs the real code boundary against the real script in a disposable Redis. - Have the route test's mocked mount resolver return a fixed count per test rather than restating the counting rule. - Strip model-supplied `_sandboxFiles` from Copilot Function calls. Only resolved inputs may populate it, and a supplied URL mount would skip their provenance. - Document that public storage contexts always count as unprovenanced mounts. * test(sandbox): restore only the env the certification suite changed The suite's cleanup assigned undefined to REDIS_URL when it had been unset, which stores the string "undefined" for later suites in the worker, and asked for a Redis client even when the suite was skipped. * test(sandbox): close the shared Redis client after the certification suite
1 parent c873ed2 commit e09970a

9 files changed

Lines changed: 260 additions & 11 deletions

File tree

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,9 @@ async function executeInSandboxWithinBudget(
891891
await recordSessionFileInput(
892892
req.session.key,
893893
{ providerId: created.providerId, sandboxId },
894-
sandboxSessionInputsSafe() && !Object.keys(selected?.envs ?? {}).length
894+
sandboxSessionInputsSafe() &&
895+
!req.session.unprovenancedInputs &&
896+
!Object.keys(selected?.envs ?? {}).length
895897
)
896898
await provisionWithinBudget(sandbox, selected, signal)
897899
await writeSandboxInputs(sandbox, req.sandboxFiles, {
@@ -1078,7 +1080,9 @@ async function executeShellInSandboxWithinBudget(
10781080
await recordSessionFileInput(
10791081
req.session.key,
10801082
{ providerId: created.providerId, sandboxId },
1081-
sandboxSessionInputsSafe() && !Object.keys(selected?.envs ?? {}).length
1083+
sandboxSessionInputsSafe() &&
1084+
!req.session.unprovenancedInputs &&
1085+
!Object.keys(selected?.envs ?? {}).length
10821086
)
10831087
await provisionWithinBudget(sandbox, selected, signal)
10841088
await writeSandboxInputs(sandbox, req.sandboxFiles, {
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/**
2+
* The persistent workbench's input history is what lets a scratch file reach the model. This runs
3+
* the real code boundary against the real history script in a disposable Redis, so a mount the
4+
* caller could not classify has to leave the machine uncertified. Only the sandbox provider is a
5+
* stand-in: it hands back an existing machine that executes nothing.
6+
*
7+
* Set TEST_REDIS_URL to an isolated local Redis service.
8+
*/
9+
import { createHash } from 'node:crypto'
10+
import {
11+
remoteSandboxProviderMock,
12+
remoteSandboxProviderMockFns,
13+
} from '@sim/testing/mocks/remote-sandbox-provider.mock'
14+
import { generateShortId } from '@sim/utils/id'
15+
import { afterAll, describe, expect, it, vi } from 'vitest'
16+
17+
const { redisUrl, inheritedRedisUrl, mockFindSessionSandbox } = await vi.hoisted(async () => {
18+
const { readTestRedisUrl } = await import('@sim/db/testing/test-infrastructure')
19+
const url = readTestRedisUrl()
20+
const inheritedRedisUrl = process.env.REDIS_URL
21+
/** The real Redis module reads this at import. */
22+
if (url) process.env.REDIS_URL = url
23+
return { redisUrl: url, inheritedRedisUrl, mockFindSessionSandbox: vi.fn() }
24+
})
25+
26+
vi.mock('@/lib/execution/remote-sandbox/provider', () => remoteSandboxProviderMock)
27+
vi.mock('@/lib/execution/remote-sandbox/resolve', () => ({
28+
resolveWorkspaceSandbox: async () => null,
29+
provisionRuntimeDependencies: async () => {},
30+
repairMissingSandboxImage: async () => null,
31+
RUNTIME_INSTALL_TIMEOUT_MS: 60_000,
32+
}))
33+
34+
import { closeRedisConnection, getRedisClient } from '@/lib/core/config/redis'
35+
import { CodeLanguage } from '@/lib/execution/languages'
36+
import {
37+
executeInSandbox,
38+
executeShellInSandbox,
39+
SIM_RESULT_PREFIX,
40+
} from '@/lib/execution/remote-sandbox'
41+
import { observeSandboxSessionInputs } from '@/lib/execution/remote-sandbox/execution-observer'
42+
import {
43+
initializeSessionFileProvenance,
44+
isSessionFileProvenanceClean,
45+
} from '@/lib/execution/remote-sandbox/session-file-provenance'
46+
import type { SandboxHandle, SandboxProvider } from '@/lib/execution/remote-sandbox/types'
47+
48+
remoteSandboxProviderMockFns.mockResolveProvider.mockImplementation(
49+
() =>
50+
({
51+
id: 'e2b',
52+
dependencyStrategy: 'prebuilt',
53+
resolveLifetimeMs: (ms: number) => ms,
54+
create: async () => {
55+
throw new Error('The certification fixture only reuses an existing machine')
56+
},
57+
findSessionSandbox: mockFindSessionSandbox,
58+
}) satisfies SandboxProvider
59+
)
60+
61+
function machine(sandboxId: string): SandboxHandle {
62+
return {
63+
sandboxId,
64+
runCode: async () => ({ text: `${SIM_RESULT_PREFIX}{"ok":true}`, stdout: '', stderr: '' }),
65+
runCommand: async () => ({ stdout: '', stderr: '', exitCode: 0 }),
66+
extendLifetime: async () => {},
67+
getFileSize: async () => 0,
68+
readFile: async () => '',
69+
readFileWithLimit: async () => ({ content: '', byteLength: 0 }),
70+
writeFile: async () => {},
71+
removeFile: async () => {},
72+
listFiles: async () => [],
73+
kill: async () => {},
74+
}
75+
}
76+
77+
/** Machine-history keys this suite created, so cleanup never touches another suite's state. */
78+
const createdKeys: string[] = []
79+
80+
afterAll(async () => {
81+
if (redisUrl && createdKeys.length) {
82+
const redis = getRedisClient()
83+
if (redis) await redis.del(...createdKeys)
84+
}
85+
if (redisUrl) await closeRedisConnection()
86+
// Only restore what the hoisted setup changed; assigning undefined would store the string "undefined".
87+
if (!redisUrl) return
88+
if (inheritedRedisUrl === undefined) Reflect.deleteProperty(process.env, 'REDIS_URL')
89+
else process.env.REDIS_URL = inheritedRedisUrl
90+
})
91+
92+
describe.skipIf(!redisUrl)('workbench certification at the code boundary', () => {
93+
it.each([
94+
['code', false],
95+
['code', true],
96+
['shell', false],
97+
['shell', true],
98+
] as const)('%s with unprovenanced mounts %s', async (kind, unprovenanced) => {
99+
const sandboxId = `machine-${generateShortId(12)}`
100+
const key = `certification-${generateShortId(12)}`
101+
const identity = { providerId: 'e2b', sandboxId } as const
102+
createdKeys.push(
103+
`mothership:workbench-provenance:v1:${createHash('sha256')
104+
.update(JSON.stringify([key, identity.providerId, sandboxId]))
105+
.digest('hex')}`
106+
)
107+
mockFindSessionSandbox.mockResolvedValue(machine(sandboxId))
108+
await initializeSessionFileProvenance(key, identity)
109+
expect(await isSessionFileProvenanceClean(key, identity)).toBe(true)
110+
111+
const request = {
112+
code: 'print(1)',
113+
language: CodeLanguage.Python,
114+
timeoutMs: 30_000,
115+
session: { key, ...(unprovenanced ? { unprovenancedInputs: true } : {}) },
116+
}
117+
await observeSandboxSessionInputs(
118+
() => true,
119+
() =>
120+
kind === 'code'
121+
? executeInSandbox(request)
122+
: executeShellInSandbox({ ...request, envs: {} })
123+
)
124+
expect(await isSessionFileProvenanceClean(key, identity)).toBe(!unprovenanced)
125+
})
126+
})

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ export interface SandboxSessionRequest {
9191
cli?: { path: string; content: string; runtime?: { path: string; content: string } }
9292
/** Extra environment variables present on every execution in the session. */
9393
envs?: Record<string, string>
94+
/**
95+
* This execution mounts bytes whose secret provenance is unknown, so the machine's input
96+
* history must not stay certified clean even when the caller's own inputs are.
97+
*/
98+
unprovenancedInputs?: boolean
9499
}
95100

96101
export interface SandboxShellExecutionRequest {

‎apps/sim/lib/function-execution/execute-request.test.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,15 @@ const {
4747
mockWriteWorkspaceFileByPath,
4848
mockUploadExecutionFile,
4949
mockMountContributors,
50+
mockUnprovenancedMountCount,
5051
mockRenderedMountContributors,
5152
} = vi.hoisted(() => ({
5253
mockExecuteInIsolatedVM: vi.fn(),
5354
mockValidateWorkspaceFileWriteTarget: vi.fn(),
5455
mockWriteWorkspaceFileByPath: vi.fn(),
5556
mockUploadExecutionFile: vi.fn(),
5657
mockMountContributors: vi.fn(),
58+
mockUnprovenancedMountCount: vi.fn(),
5759
mockRenderedMountContributors: vi.fn(),
5860
}))
5961

@@ -153,6 +155,7 @@ vi.mock('@/lib/function-execution/sandbox-mounts', () => ({
153155
}) => ({
154156
contributingFiles: mockMountContributors(),
155157
renderedContributingFiles: mockRenderedMountContributors(),
158+
unprovenancedMountCount: mockUnprovenancedMountCount(),
156159
sandboxFiles: planned.map(({ mountPath }) => ({
157160
type: 'url' as const,
158161
path: mountPath,
@@ -259,6 +262,7 @@ describe('Function execution request', () => {
259262
beforeEach(() => {
260263
resetDbChainMock()
261264
mockMountContributors.mockReturnValue(undefined)
265+
mockUnprovenancedMountCount.mockReturnValue(0)
262266
mockRenderedMountContributors.mockReturnValue(undefined)
263267
mockUploadExecutionFile.mockImplementation(async (context, buffer, name, type) => ({
264268
id: 'execution-file-1',
@@ -2560,6 +2564,33 @@ describe('Function execution request', () => {
25602564
expect(mockWriteWorkspaceFileByPath).not.toHaveBeenCalled()
25612565
})
25622566

2567+
it.each([
2568+
['a mount with no provenance source', true],
2569+
['no mounts', false],
2570+
] as const)('withholds workbench certification for %s', async (_label, mounted) => {
2571+
envFlagsMock.isMothershipSandboxEnabled = true
2572+
mockUnprovenancedMountCount.mockReturnValue(mounted ? 1 : 0)
2573+
hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({
2574+
success: true,
2575+
userId: 'user-123',
2576+
authType: 'internal_jwt',
2577+
sandboxProfile: 'mothership',
2578+
})
2579+
const response = await POST(
2580+
createMockRequest('POST', {
2581+
code: 'x',
2582+
language: 'python',
2583+
workspaceId: 'workspace-1',
2584+
sandboxSessionKey: 'chat-session',
2585+
...(mounted ? { contextVariables: { doc: MOUNT_REF } } : {}),
2586+
})
2587+
)
2588+
expect(response.status).toBe(200)
2589+
const session = mockExecuteInSandbox.mock.calls.at(-1)?.[0].session
2590+
expect(session.key).toBe('chat-session')
2591+
expect(session.unprovenancedInputs === true).toBe(mounted)
2592+
})
2593+
25632594
it('gives overlapping calls in one persistent workbench distinct automatic export directories', async () => {
25642595
envFlagsMock.isMothershipSandboxEnabled = true
25652596
hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({

‎apps/sim/lib/function-execution/execute-request.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2385,7 +2385,7 @@ export async function executeFunctionRequest(
23852385
// would leave `{{OTHER_SECRET}}` resolving, which is a hole, not a scope.
23862386
const envVars = scopeEnvironmentVariables(rawEnvVars, secretScope, mountedSecrets)
23872387
const admittedChatOwner = activeSandboxChatOwner()
2388-
const mothershipSession =
2388+
const admittedSession =
23892389
usesMothershipSandbox &&
23902390
!selectedSandboxId &&
23912391
sandboxSessionKey &&
@@ -2716,6 +2716,10 @@ export async function executeFunctionRequest(
27162716
)
27172717
}
27182718
const { sandboxFiles: userFileMounts, manifest: mountManifest } = resolvedMounts
2719+
const mothershipSession =
2720+
admittedSession && resolvedMounts.unprovenancedMountCount > 0
2721+
? { ...admittedSession, unprovenancedInputs: true }
2722+
: admittedSession
27192723
const sandboxFiles = mergeSandboxFileMounts(_sandboxFiles, userFileMounts)
27202724

27212725
// Every `<block.file.path>` marker becomes the path its file was mounted at,

‎apps/sim/lib/function-execution/sandbox-mounts.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,42 @@ describe('resolveUserFileMounts', () => {
214214
expect(mockDownloadServableFileFromStorage).not.toHaveBeenCalled()
215215
})
216216

217+
/**
218+
* A persistent workbench certifies its machine from these counts: a mount whose bytes have no
219+
* provenance source can hold resolved secret plaintext nobody recorded, so it must be reported.
220+
*/
221+
it.each([
222+
['has no metadata record', null, 1],
223+
['has a canonical metadata record', 'recorded', 0],
224+
] as const)('reports a mounted file whose key %s', async (_label, metadata, expected) => {
225+
const file = executionFile()
226+
mockGetFileMetadataByKey.mockResolvedValue(
227+
metadata
228+
? {
229+
id: 'canonical-file-id',
230+
key: file.key,
231+
context: 'execution',
232+
workspaceId: WORKSPACE_ID,
233+
userId: 'user-1',
234+
contentUpdatedAt: new Date('2026-01-01T00:00:00Z'),
235+
}
236+
: null
237+
)
238+
const result = await resolveUserFileMounts({
239+
planned: planUserFileMounts([file]),
240+
context: { ...executionContext, principal: createSessionPrincipal() },
241+
})
242+
expect(result.unprovenancedMountCount).toBe(expected)
243+
})
244+
245+
it('reports every mount as unprovenanced when no principal can bind its source', async () => {
246+
const result = await resolveUserFileMounts({
247+
planned: planUserFileMounts([executionFile()]),
248+
context: executionContext,
249+
})
250+
expect(result.unprovenancedMountCount).toBe(1)
251+
})
252+
217253
it('preserves contributors introduced when an inline mount renders generated source', async () => {
218254
const contributor = {
219255
fileId: 'image-file',

‎apps/sim/lib/function-execution/sandbox-mounts.ts‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,18 @@ export async function resolveUserFileMounts(args: {
277277
manifest: SandboxMountManifestEntry[]
278278
contributingFiles?: readonly WorkspaceFileSecretProvenanceIdentity[]
279279
renderedContributingFiles?: readonly WorkspaceFileSecretProvenanceIdentity[]
280+
/**
281+
* Mounts whose own bytes have no provenance source (no principal to bind one, or a key with no
282+
* canonical metadata record). Workflow runs keep their legacy absence policy; a persistent
283+
* workbench must not certify a machine that received one.
284+
*
285+
* Storage contexts other than workspace and execution (chat, copilot, knowledge-base, logs, and
286+
* the other public contexts) never have a source, so they always count here and taint a
287+
* workbench. That is conservative by design.
288+
*/
289+
unprovenancedMountCount: number
280290
}> {
291+
let unprovenancedMountCount = 0
281292
const sandboxFiles: SandboxFile[] = []
282293
const manifest: SandboxMountManifestEntry[] = []
283294
const budget = createSandboxMountBudget()
@@ -297,14 +308,16 @@ export async function resolveUserFileMounts(args: {
297308
for (const { userFile, mountPath } of args.planned) {
298309
const storageContext = resolveTrustedFileContext(userFile.key, userFile.context)
299310
await assertUserFileContentAccess(userFile, args.context)
300-
if (args.context.principal && args.context.workspaceId) {
301-
const source = await resolveStoredFileProvenanceSource(userFile, {
302-
...args.context,
303-
principal: args.context.principal,
304-
workspaceId: args.context.workspaceId,
305-
})
306-
if (source) addContributor(source.identity)
307-
}
311+
const source =
312+
args.context.principal && args.context.workspaceId
313+
? await resolveStoredFileProvenanceSource(userFile, {
314+
...args.context,
315+
principal: args.context.principal,
316+
workspaceId: args.context.workspaceId,
317+
})
318+
: undefined
319+
if (source) addContributor(source.identity)
320+
else unprovenancedMountCount += 1
308321

309322
await pushSandboxFileMount(
310323
sandboxFiles,
@@ -361,6 +374,7 @@ export async function resolveUserFileMounts(args: {
361374
return {
362375
sandboxFiles,
363376
manifest,
377+
unprovenancedMountCount,
364378
...(contributingFiles.size > 0 ? { contributingFiles: [...contributingFiles.values()] } : {}),
365379
...(renderedContributingFiles.size > 0
366380
? { renderedContributingFiles: [...renderedContributingFiles.values()] }

‎apps/sim/lib/mothership/tools/handlers/function-execute-provenance.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,33 @@ describe('Function physical-session input certification', () => {
5757
)
5858
})
5959

60+
describe('Function sandbox mounts', () => {
61+
it('never forwards a model-supplied sandbox mount, which would bypass input provenance', async () => {
62+
mocks.execute.mockImplementation(async (_tool, params) => ({
63+
success: true,
64+
output: { sandboxFiles: params._sandboxFiles ?? [] },
65+
}))
66+
const result = await executeFunctionExecute(
67+
{
68+
code: 'print(open("/tmp/sim/inputs/x").read())',
69+
language: 'python',
70+
_sandboxFiles: [{ type: 'url', path: '/tmp/sim/inputs/x', url: 'https://storage.test/x' }],
71+
},
72+
{
73+
userId: 'user',
74+
workflowId: '',
75+
workspaceId: 'workspace',
76+
chatId: 'chat',
77+
resolvedSecretTraceRegistry: new ResolvedSecretTraceRegistry([], {
78+
userId: 'user',
79+
workspaceId: 'workspace',
80+
}),
81+
}
82+
)
83+
expect(result.output).toEqual({ sandboxFiles: [] })
84+
})
85+
})
86+
6087
describe('Generic Secrets function execution', () => {
6188
it('mounts the authorized environment and propagates echoed secrets into model redaction', async () => {
6289
setEnv({ ENCRYPTION_KEY: 'a'.repeat(64) })

‎apps/sim/lib/mothership/tools/handlers/function-execute.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,8 @@ export async function executeFunctionExecute(
499499
'internalSandboxProfile',
500500
// Server-derived below — a model-supplied value must never select a session.
501501
'sandboxSessionKey',
502+
// Server-derived from resolved inputs; a model-supplied mount would skip their provenance.
503+
'_sandboxFiles',
502504
PRIVATE_SECRET_PROVENANCE_FIELD,
503505
])
504506
// One persistent session sandbox per chat: files and installed packages

0 commit comments

Comments
 (0)