Skip to content

Commit 1c8dce1

Browse files
committed
fix(sandbox): preserve workbench availability for unrecorded API responses
1 parent 9a2b64d commit 1c8dce1

3 files changed

Lines changed: 154 additions & 88 deletions

File tree

‎apps/sim/lib/mothership/tools/handlers/workbench-confidentiality.live.test.ts‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ import {
1818
mothershipGoFetchMock,
1919
mothershipGoFetchMockFns,
2020
} from '@sim/testing/mocks/mothership-go-fetch.mock'
21+
import {
22+
mothershipWorkspaceTargetMock,
23+
mothershipWorkspaceTargetMockFns,
24+
} from '@sim/testing/mocks/mothership-workspace-target.mock'
2125
import { redisConfigMockFns } from '@sim/testing/mocks/redis-config.mock'
2226
import {
2327
remoteSandboxProviderMock,
@@ -30,6 +34,7 @@ import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
3034

3135
const io = vi.hoisted(() => ({ mount: vi.fn(), find: vi.fn(), write: vi.fn() }))
3236
vi.mock('@/tools', () => toolsMock)
37+
vi.mock('@/lib/mothership/application/workspace-target', () => mothershipWorkspaceTargetMock)
3338
vi.mock('@/lib/mothership/tools/secret-mount-materializer.server', () => ({
3439
materializeCopilotCodeSecrets: io.mount,
3540
CopilotCodeSecretAccessError: class extends Error {},
@@ -54,6 +59,7 @@ vi.mock('@/lib/mothership/vfs/resource-writer', () => ({
5459
}))
5560

5661
import { functionExecuteBodySchema } from '@/lib/api/contracts'
62+
import * as inProcessTransport from '@/lib/api/server/routes/in-process-transport'
5763
import { encryptSecret } from '@/lib/core/security/encryption'
5864
import {
5965
PRIVATE_TOOL_METADATA_REQUEST_HEADER,
@@ -73,12 +79,15 @@ import { inspectToolResultForCopilot } from '@/lib/mothership/request/tools/reso
7379
import type { ToolExecutionContext } from '@/lib/mothership/tool-executor/types'
7480
import { executeFunctionExecute } from '@/lib/mothership/tools/handlers/function-execute'
7581
import { executeRunCode } from '@/lib/mothership/tools/handlers/run-code'
82+
import { proxySandboxResourceRequest } from '@/lib/mothership/tools/sandbox-resource-transport'
7683
import {
7784
readSandboxResourceScope,
7885
withSandboxResourceScope,
7986
} from '@/lib/mothership/tools/sandbox-resources'
8087
import { buildMothershipSandboxSession } from '@/lib/mothership/tools/sandbox-session'
8188
import { chatSandboxSessionKey } from '@/lib/mothership/tools/sandbox-session-key'
89+
import { reportTableRowDelivery } from '@/lib/table/application/row-delivery-observer'
90+
import { reportWorkspaceFileDelivery } from '@/lib/workspace-files/application/file-delivery-observer'
8291
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
8392
import { buildFunctionExecuteBody, functionExecuteTool } from '@/tools/function/execute'
8493
import type { CodeExecutionInput } from '@/tools/function/types'
@@ -320,6 +329,123 @@ function inResourceScope<T>(action: () => Promise<T>) {
320329
)
321330
}
322331

332+
async function sandboxApi(path: string, handler: () => Promise<Response>, method = 'GET') {
333+
mothershipWorkspaceTargetMockFns.mockResolveInvocationWorkspace.mockResolvedValue(scope)
334+
vi.spyOn(inProcessTransport, 'matchV2Route').mockReturnValue({
335+
pattern: path,
336+
params: { fileId: 'fixture', tableId: 'fixture' },
337+
literals: 3,
338+
load: async () => ({ GET: handler, POST: handler }),
339+
})
340+
return inResourceScope(async () => {
341+
const session = await buildMothershipSandboxSession({
342+
...scope,
343+
sessionKey: chatSandboxSessionKey(chatId),
344+
})
345+
const endpoint = session.envs!.SIM_ENDPOINT
346+
return proxySandboxResourceRequest(
347+
new Request(`${endpoint}${path}`, {
348+
method,
349+
headers: { 'x-api-key': session.envs!.SIM_API_KEY },
350+
}),
351+
endpoint.split('/').at(-1)!
352+
)
353+
})
354+
}
355+
356+
describe('sandbox API provenance admission', () => {
357+
it('keeps ordinary API mutations usable for later code output and generated CLI input', async () => {
358+
const response = await sandboxApi(
359+
'/api/v2/custom-tools',
360+
async () => Response.json({ data: { id: 'fixture-tool', title: 'fixture' } }),
361+
'POST'
362+
)
363+
expect(response.status).toBe(200)
364+
const result = await run('printf "[]" > operations.json; printf "ready"')
365+
expect(result.projected.safe).toBe(true)
366+
expect(result.projected.result).toMatchObject({ success: true, output: { stdout: 'ready' } })
367+
expect(
368+
(await readCliInputFile(chatSandboxSessionKey(chatId), 'operations.json')).toString()
369+
).toBe('[]')
370+
})
371+
372+
it('retains earlier secret protection after an API response without provenance', async () => {
373+
await run('printf "%s" "$TOKEN" > saved.txt', ['TOKEN'])
374+
await sandboxApi('/api/v2/custom-tools', async () => Response.json({ data: [] }))
375+
const result = await run('cat saved.txt')
376+
expect(result.projected.safe).toBe(true)
377+
expect(result.projected.result).toMatchObject({
378+
success: true,
379+
output: { stdout: '{{TOKEN}}' },
380+
})
381+
await expect(readCliInputFile(chatSandboxSessionKey(chatId), 'saved.txt')).rejects.toThrow(
382+
'protected workbench values'
383+
)
384+
})
385+
386+
it.each(['file', 'table'] as const)(
387+
'imports explicit %s delivery evidence before later output',
388+
async (source) => {
389+
const response = await sandboxApi(
390+
`/api/v2/${source === 'file' ? 'files/fixture' : 'tables/fixture/rows'}`,
391+
async () => {
392+
if (source === 'file') {
393+
await reportWorkspaceFileDelivery({
394+
status: 'exact',
395+
entries: [
396+
{
397+
name: 'TOKEN',
398+
encryptedValue: catalog[0].encryptedValue,
399+
sourceUserId: scope.userId,
400+
sourceWorkspaceId: scope.workspaceId,
401+
},
402+
],
403+
})
404+
} else {
405+
await reportTableRowDelivery(
406+
{
407+
version: 1,
408+
complete: true,
409+
scope,
410+
entries: [{ name: 'TOKEN', encryptedValue: catalog[0].encryptedValue }],
411+
},
412+
[{ value: canary }]
413+
)
414+
}
415+
return new Response(canary)
416+
}
417+
)
418+
await machine.writeFile('/home/user/delivered.txt', await response.text())
419+
const result = await run('cat delivered.txt')
420+
expect(result.projected.safe).toBe(true)
421+
expect(result.projected.result).toMatchObject({
422+
success: true,
423+
output: { stdout: '{{TOKEN}}' },
424+
})
425+
}
426+
)
427+
428+
it.each(['file', 'table'] as const)(
429+
'preserves an explicit unknown %s delivery as unknown',
430+
async (source) => {
431+
await sandboxApi(
432+
`/api/v2/${source === 'file' ? 'files/fixture' : 'tables/fixture/rows'}`,
433+
async () => {
434+
if (source === 'file') await reportWorkspaceFileDelivery({ status: 'unknown' })
435+
else
436+
await reportTableRowDelivery({ version: 1, complete: false, entries: [] }, [
437+
{ value: 'unknown' },
438+
])
439+
return new Response('unknown')
440+
}
441+
)
442+
const result = await run('printf "ready"')
443+
expect(result.projected.safe).toBe(false)
444+
expect(JSON.stringify(result.projected.result)).not.toContain('ready')
445+
}
446+
)
447+
})
448+
323449
describe('persistent workbench output confidentiality', () => {
324450
it.each(['javascript', 'shell'] as const)(
325451
'redacts session credentials in %s output while preserving routing metadata',

‎apps/sim/lib/mothership/tools/sandbox-resource-transport.test.ts‎

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,6 @@ describe('private sandbox v2 resource transport', () => {
9090
token
9191
)
9292
expect(await response.json()).toEqual({ data: { inserted: 1 } })
93-
expect(recordInput).toHaveBeenCalledWith('mothership-chat:chat', false)
94-
expect(recordInput.mock.invocationCallOrder[0]).toBeLessThan(
95-
fetcher.mock.invocationCallOrder[0]!
96-
)
9793
expect(fetcher).toHaveBeenCalledTimes(1)
9894
expect(recordEffects).toHaveBeenCalledWith(token, scope, [
9995
{
@@ -261,14 +257,6 @@ vi.mock('@/lib/execution/remote-sandbox/session-file-provenance', () => ({
261257
recordExistingSessionFileInput: recordInput,
262258
}))
263259

264-
it('refuses delivery before dispatch when provenance cannot be recorded', async () => {
265-
recordInput.mockRejectedValueOnce(new Error('storage unavailable'))
266-
await expect(proxySandboxResourceRequest(request('/api/v2/tables/table'), token)).rejects.toThrow(
267-
'storage unavailable'
268-
)
269-
expect(fetcher).not.toHaveBeenCalled()
270-
})
271-
272260
it('does not poison public scratch after authenticated static catalog discovery', async () => {
273261
routeMatcher.mockReturnValue({
274262
params: { toolId: 'function_execute' },
@@ -333,40 +321,3 @@ it('keeps the server identity out of callback response headers and body', async
333321
expect(JSON.stringify([...response.headers])).not.toContain('server-only-identity')
334322
expect(await response.text()).not.toContain('server-only-identity')
335323
})
336-
337-
it.each(['builtin', 'custom'] as const)(
338-
'preserves public research scratch only for producer-classified %s block catalog content',
339-
async (source) => {
340-
routeMatcher.mockReturnValue({ params: {}, load: async () => ({ GET: fetcher }) })
341-
fetcher.mockResolvedValue(
342-
Response.json({
343-
data: [
344-
{
345-
id: 'agent',
346-
name: 'Agent',
347-
description: 'Build an agent',
348-
category: 'blocks',
349-
source,
350-
triggerAllowed: false,
351-
triggerCapable: false,
352-
triggerIds: [],
353-
toolIds: [],
354-
operationIds: [],
355-
preview: false,
356-
tags: [],
357-
},
358-
],
359-
nextCursor: null,
360-
})
361-
)
362-
expect((await proxySandboxResourceRequest(request('/api/v2/blocks'), token)).status).toBe(200)
363-
if (source === 'builtin') expect(recordInput).not.toHaveBeenCalled()
364-
else expect(recordInput).toHaveBeenCalledWith('mothership-chat:chat', false)
365-
}
366-
)
367-
it('does not trust a source label in a malformed catalog result', async () => {
368-
routeMatcher.mockReturnValue({ params: {}, load: async () => ({ GET: fetcher }) })
369-
fetcher.mockResolvedValue(Response.json({ data: [{ source: 'builtin' }], nextCursor: null }))
370-
expect((await proxySandboxResourceRequest(request('/api/v2/blocks'), token)).status).toBe(200)
371-
expect(recordInput).toHaveBeenCalledWith('mothership-chat:chat', false)
372-
})

‎apps/sim/lib/mothership/tools/sandbox-resource-transport.ts‎

Lines changed: 28 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,16 @@
11
import { createLogger } from '@sim/logger'
22
import { generateId } from '@sim/utils/id'
33
import { NextRequest } from 'next/server'
4-
import {
5-
v2GetBlockContract,
6-
v2GetToolContract,
7-
v2ListBlocksContract,
8-
v2ListConnectorTypesContract,
9-
v2ListToolsContract,
10-
} from '@/lib/api/contracts/v2/catalog'
114
import { v2DownloadFileContract, v2ReadFileTextContract } from '@/lib/api/contracts/v2/files'
125
import { markCopilotRequest } from '@/lib/api/server/routes/copilot-request'
136
import { matchV2Route } from '@/lib/api/server/routes/in-process-transport'
147
import { withWorkspaceInvocationScope } from '@/lib/core/application/workspace-invocation-scope'
158
import { asOrchestrationError, statusForOrchestrationError } from '@/lib/core/orchestration/types'
169
import { getInternalApiBaseUrl } from '@/lib/core/utils/urls'
17-
import type { DurableSecretProvenance } from '@/lib/execution/durable-secret-provenance'
10+
import {
11+
type DurableSecretProvenance,
12+
durableSecretProvenanceFromEnvelope,
13+
} from '@/lib/execution/durable-secret-provenance'
1814
import { recordExistingSessionFileInput } from '@/lib/execution/remote-sandbox/session-file-provenance'
1915
import { createResourceEffectTransport } from '@/lib/mothership/agent-cli/resource-effects'
2016
import { resolveInvocationWorkspace } from '@/lib/mothership/application/workspace-target'
@@ -24,6 +20,7 @@ import {
2420
recordSandboxResourceEffects,
2521
} from '@/lib/mothership/tools/sandbox-resources'
2622
import { chatSandboxSessionKey } from '@/lib/mothership/tools/sandbox-session-key'
23+
import { observeTableRowDelivery } from '@/lib/table/application/row-delivery-observer'
2724
import { observeWorkspaceFileDelivery } from '@/lib/workspace-files/application/file-delivery-observer'
2825

2926
const logger = createLogger('MothershipSandboxResourceTransport')
@@ -122,23 +119,6 @@ async function proxyAuthorizedSandboxRequest(
122119
if (!(result instanceof Response)) throw new Error('Invalid sandbox API response')
123120
return result
124121
}
125-
// Only these producer-owned catalog responses contain no workspace data or execution output.
126-
const publicCatalog =
127-
method === 'GET' &&
128-
[v2ListToolsContract, v2GetToolContract, v2ListConnectorTypesContract].some(
129-
(contract) =>
130-
contract.path.replace(/\[([^\]]+)\]/g, (_match, key) =>
131-
encodeURIComponent(matched.params[key] ?? '')
132-
) === path
133-
)
134-
const blockCatalog =
135-
method === 'GET' &&
136-
[v2ListBlocksContract, v2GetBlockContract].find(
137-
(contract) =>
138-
contract.path.replace(/\[([^\]]+)\]/g, (_match, key) =>
139-
encodeURIComponent(matched.params[key] ?? '')
140-
) === path
141-
)
142122
const recordInput = (provenance: boolean | DurableSecretProvenance) =>
143123
recordExistingSessionFileInput(chatSandboxSessionKey(scope.chatId), provenance)
144124
const fileRead =
@@ -151,21 +131,30 @@ async function proxyAuthorizedSandboxRequest(
151131
)
152132
const deliver = async () => {
153133
dispatched = true
154-
if (!publicCatalog && !fileRead && !blockCatalog) await recordInput(false)
155-
let observed = false
156-
const result = await observeWorkspaceFileDelivery(async (provenance) => {
157-
await recordInput(provenance?.status === 'exact' ? provenance : false)
158-
observed = true
159-
}, dispatch)
134+
let fileObserved = false
135+
let rowsObserved = false
136+
const result = await observeTableRowDelivery(
137+
async (provenance, _values, extras) => {
138+
await recordInput(
139+
extras.unprovenancedErrorText ? false : durableSecretProvenanceFromEnvelope(provenance)
140+
)
141+
rowsObserved = true
142+
},
143+
() =>
144+
observeWorkspaceFileDelivery(async (provenance) => {
145+
await recordInput(provenance?.status === 'exact' ? provenance : false)
146+
fileObserved = true
147+
}, dispatch)
148+
)
160149
try {
161-
if (fileRead && !observed && result.ok && result.body) await recordInput(false)
162-
if (blockCatalog && result.ok && result.body) {
163-
const parsed = blockCatalog.response.schema.safeParse(await result.clone().json())
164-
const data = parsed.success ? parsed.data.data : undefined
165-
const safe = Array.isArray(data)
166-
? data.every((block) => block.source === 'builtin')
167-
: data?.source === 'builtin'
168-
if (!safe) await recordInput(false)
150+
if (fileRead && !fileObserved && result.ok && result.body) await recordInput(false)
151+
else if (!fileObserved && !rowsObserved) {
152+
/** Missing producer evidence is unrecorded, not proof that the machine received a secret. */
153+
logger.warn('Sandbox API response has no recorded secret provenance', {
154+
method,
155+
route: matched.pattern,
156+
toolCallId: scope.toolCallId,
157+
})
169158
}
170159
} catch (error) {
171160
await result.body?.cancel().catch(() => {})

0 commit comments

Comments
 (0)