Skip to content

Commit 41818fb

Browse files
committed
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.
1 parent 217f28a commit 41818fb

6 files changed

Lines changed: 159 additions & 113 deletions

File tree

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
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 { 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+
const redis = getRedisClient()
82+
if (redis && createdKeys.length) await redis.del(...createdKeys)
83+
if (inheritedRedisUrl === undefined) process.env.REDIS_URL = undefined
84+
else process.env.REDIS_URL = inheritedRedisUrl
85+
})
86+
87+
describe.skipIf(!redisUrl)('workbench certification at the code boundary', () => {
88+
it.each([
89+
['code', false],
90+
['code', true],
91+
['shell', false],
92+
['shell', true],
93+
] as const)('%s with unprovenanced mounts %s', async (kind, unprovenanced) => {
94+
const sandboxId = `machine-${generateShortId(12)}`
95+
const key = `certification-${generateShortId(12)}`
96+
const identity = { providerId: 'e2b', sandboxId } as const
97+
createdKeys.push(
98+
`mothership:workbench-provenance:v1:${createHash('sha256')
99+
.update(JSON.stringify([key, identity.providerId, sandboxId]))
100+
.digest('hex')}`
101+
)
102+
mockFindSessionSandbox.mockResolvedValue(machine(sandboxId))
103+
await initializeSessionFileProvenance(key, identity)
104+
expect(await isSessionFileProvenanceClean(key, identity)).toBe(true)
105+
106+
const request = {
107+
code: 'print(1)',
108+
language: CodeLanguage.Python,
109+
timeoutMs: 30_000,
110+
session: { key, ...(unprovenanced ? { unprovenancedInputs: true } : {}) },
111+
}
112+
await observeSandboxSessionInputs(
113+
() => true,
114+
() =>
115+
kind === 'code'
116+
? executeInSandbox(request)
117+
: executeShellInSandbox({ ...request, envs: {} })
118+
)
119+
expect(await isSessionFileProvenanceClean(key, identity)).toBe(!unprovenanced)
120+
})
121+
})

‎apps/sim/lib/execution/remote-sandbox/session-input-certification.test.ts‎

Lines changed: 0 additions & 112 deletions
This file was deleted.

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

Lines changed: 5 additions & 1 deletion
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,7 +155,7 @@ vi.mock('@/lib/function-execution/sandbox-mounts', () => ({
153155
}) => ({
154156
contributingFiles: mockMountContributors(),
155157
renderedContributingFiles: mockRenderedMountContributors(),
156-
unprovenancedMountCount: mockMountContributors() ? 0 : planned.length,
158+
unprovenancedMountCount: mockUnprovenancedMountCount(),
157159
sandboxFiles: planned.map(({ mountPath }) => ({
158160
type: 'url' as const,
159161
path: mountPath,
@@ -260,6 +262,7 @@ describe('Function execution request', () => {
260262
beforeEach(() => {
261263
resetDbChainMock()
262264
mockMountContributors.mockReturnValue(undefined)
265+
mockUnprovenancedMountCount.mockReturnValue(0)
263266
mockRenderedMountContributors.mockReturnValue(undefined)
264267
mockUploadExecutionFile.mockImplementation(async (context, buffer, name, type) => ({
265268
id: 'execution-file-1',
@@ -2566,6 +2569,7 @@ describe('Function execution request', () => {
25662569
['no mounts', false],
25672570
] as const)('withholds workbench certification for %s', async (_label, mounted) => {
25682571
envFlagsMock.isMothershipSandboxEnabled = true
2572+
mockUnprovenancedMountCount.mockReturnValue(mounted ? 1 : 0)
25692573
hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({
25702574
success: true,
25712575
userId: 'user-123',

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,10 @@ export async function resolveUserFileMounts(args: {
281281
* Mounts whose own bytes have no provenance source (no principal to bind one, or a key with no
282282
* canonical metadata record). Workflow runs keep their legacy absence policy; a persistent
283283
* 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.
284288
*/
285289
unprovenancedMountCount: number
286290
}> {

‎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)