Skip to content

Commit b440da7

Browse files
committed
fix(sandbox): unify output handling and avoid response copies
1 parent 05b84e9 commit b440da7

4 files changed

Lines changed: 103 additions & 18 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,10 @@ export interface SandboxSessionRequest {
9393
/** Extra environment variables present on every execution in the session. */
9494
envs?: Record<string, string>
9595
/**
96-
* Ephemeral callback credentials for model-output redaction after execution. They expire with
97-
* the tool lease and do not contribute to durable machine or exported-file provenance.
96+
* Selects ephemeral callback credentials present in the returned JSON for model redaction.
97+
* They expire with the tool lease and do not contribute to machine or exported-file provenance.
9898
*/
99-
outputProvenance?: DurableSecretProvenance
99+
outputProvenance?: (value: unknown) => DurableSecretProvenance
100100
/**
101101
* This execution mounts bytes whose secret provenance is unknown, so the machine's input
102102
* history must not stay certified clean even when the caller's own inputs are.

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

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,11 @@ import {
9191
MAX_BLOCK_MOUNTED_FILES,
9292
SANDBOX_OUTPUT_DIR,
9393
} from '@/lib/execution/remote-sandbox/sandbox-paths'
94-
import type { SandboxCollectedFile, SandboxFile } from '@/lib/execution/remote-sandbox/types'
94+
import type {
95+
SandboxCollectedFile,
96+
SandboxFile,
97+
SandboxSessionRequest,
98+
} from '@/lib/execution/remote-sandbox/types'
9599
import { isExecutionResourceLimitError } from '@/lib/execution/resource-errors'
96100
import { MAX_FUNCTION_REFERENCES } from '@/lib/function-execution/limits'
97101
import type { SandboxExportedFile } from '@/lib/function-execution/output'
@@ -1030,7 +1034,7 @@ interface FunctionRouteExecutionContext {
10301034
runtimeFileSecretTraceRegistry?: ResolvedSecretTraceRegistry
10311035
runtimeInputProvenanceUnrecorded?: boolean
10321036
resolvedSecretTraceRegistry?: ResolvedSecretTraceRegistry
1033-
sessionOutputProvenance?: DurableSecretProvenance
1037+
sessionOutputProvenance?: SandboxSessionRequest['outputProvenance']
10341038
}
10351039

10361040
/** Keeps bound file provenance in both ordinary Function results and exported artifact bytes. */
@@ -1277,15 +1281,10 @@ async function functionJsonResponse<T>(
12771281
context: FunctionRouteExecutionContext,
12781282
init?: ResponseInit
12791283
) {
1280-
/**
1281-
* Narrow callback receipts to the returned JSON before compaction. Scanning serialized bytes
1282-
* avoids activating secret-only traversal limits on large results that contain no credential.
1283-
*/
12841284
if (context.sessionOutputProvenance && context.resolvedSecretTraceRegistry) {
12851285
await importDurableSecretProvenance(
12861286
context.resolvedSecretTraceRegistry,
1287-
context.sessionOutputProvenance,
1288-
JSON.stringify(body)
1287+
context.sessionOutputProvenance(body)
12891288
)
12901289
}
12911290
const responseBody = {
@@ -1555,13 +1554,14 @@ function exportUnchangedNote(sandboxPath?: string): string {
15551554
}
15561555

15571556
function exportFailure(
1557+
context: FunctionRouteExecutionContext,
15581558
error: string,
15591559
status: number,
15601560
stdout: string,
15611561
executionTime: number,
15621562
cost: FunctionExecutionCost | undefined
1563-
): NextResponse {
1564-
return NextResponse.json(
1563+
) {
1564+
return functionJsonResponse(
15651565
{
15661566
success: false,
15671567
error,
@@ -1572,6 +1572,7 @@ function exportFailure(
15721572
...(cost ? { cost } : {}),
15731573
},
15741574
},
1575+
context,
15751576
{ status }
15761577
)
15771578
}
@@ -1661,6 +1662,7 @@ async function maybeExportSandboxFileToWorkspace(args: {
16611662

16621663
if (!outputPath) {
16631664
return exportFailure(
1665+
routeContext,
16641666
'outputSandboxPath requires outputPath. Set outputPath to the destination workspace file, e.g. "files/result.csv".',
16651667
400,
16661668
stdout,
@@ -1674,6 +1676,7 @@ async function maybeExportSandboxFileToWorkspace(args: {
16741676

16751677
if (!resolvedWorkspaceId || routeContext.principal.kind !== 'delegated') {
16761678
return exportFailure(
1679+
routeContext,
16771680
'Workspace context required to save sandbox file to workspace',
16781681
400,
16791682
stdout,
@@ -1684,6 +1687,7 @@ async function maybeExportSandboxFileToWorkspace(args: {
16841687

16851688
if (exportedFileContent === undefined) {
16861689
return exportFailure(
1690+
routeContext,
16871691
`Sandbox file "${outputSandboxPath}" was not found or could not be read`,
16881692
500,
16891693
stdout,
@@ -1707,6 +1711,7 @@ async function maybeExportSandboxFileToWorkspace(args: {
17071711
const outputBytes = Buffer.byteLength(exportedFileContent, isBinary ? 'base64' : 'utf-8')
17081712
if (outputBytes > MAX_SANDBOX_OUTPUT_BYTES) {
17091713
return exportFailure(
1714+
routeContext,
17101715
`Sandbox output files exceed ${MAX_SANDBOX_OUTPUT_BYTES} bytes total`,
17111716
400,
17121717
stdout,
@@ -1791,6 +1796,7 @@ async function maybeExportSandboxFileToWorkspace(args: {
17911796
})
17921797
} catch (error) {
17931798
return exportFailure(
1799+
routeContext,
17941800
getErrorMessage(error, 'Failed to export sandbox file'),
17951801
workspaceFileExportErrorStatus(error),
17961802
stdout,
@@ -1817,6 +1823,7 @@ async function maybeExportSandboxFilesToWorkspace(args: {
18171823
if (sandboxFiles.length === 0) return null
18181824
if (sandboxFiles.length > MAX_SANDBOX_OUTPUT_FILES) {
18191825
return exportFailure(
1826+
args.routeContext,
18201827
`Too many sandbox output files requested (${sandboxFiles.length}). Maximum is ${MAX_SANDBOX_OUTPUT_FILES}.`,
18211828
400,
18221829
args.stdout,
@@ -1852,6 +1859,7 @@ async function maybeExportSandboxFilesToWorkspace(args: {
18521859
(args.workflowId ? (await getWorkflowById(args.workflowId))?.workspaceId : undefined)
18531860
if (!resolvedWorkspaceId || args.routeContext.principal.kind !== 'delegated') {
18541861
return exportFailure(
1862+
args.routeContext,
18551863
'Workspace context required to save sandbox files to workspace',
18561864
400,
18571865
args.stdout,
@@ -1867,6 +1875,7 @@ async function maybeExportSandboxFilesToWorkspace(args: {
18671875
const content = args.exportedFiles?.[sandboxPath]
18681876
if (content === undefined) {
18691877
return exportFailure(
1878+
args.routeContext,
18701879
`Sandbox file "${sandboxPath}" was not found or could not be read`,
18711880
500,
18721881
args.stdout,
@@ -1888,6 +1897,7 @@ async function maybeExportSandboxFilesToWorkspace(args: {
18881897
totalOutputBytes += size
18891898
if (totalOutputBytes > MAX_SANDBOX_OUTPUT_BYTES) {
18901899
return exportFailure(
1900+
args.routeContext,
18911901
`Sandbox output files exceed ${MAX_SANDBOX_OUTPUT_BYTES} bytes total`,
18921902
400,
18931903
args.stdout,
@@ -1940,6 +1950,7 @@ async function maybeExportSandboxFilesToWorkspace(args: {
19401950
validationPaths = validations.map((validation) => validation.vfsPath)
19411951
} catch (error) {
19421952
return exportFailure(
1953+
args.routeContext,
19431954
getErrorMessage(error, 'Invalid sandbox output destination'),
19441955
workspaceFileExportErrorStatus(error),
19451956
args.stdout,
@@ -1952,6 +1963,7 @@ async function maybeExportSandboxFilesToWorkspace(args: {
19521963
)
19531964
if (duplicateDestination) {
19541965
return exportFailure(
1966+
args.routeContext,
19551967
`Duplicate sandbox output destination: ${duplicateDestination}`,
19561968
400,
19571969
args.stdout,
@@ -2009,6 +2021,7 @@ async function maybeExportSandboxFilesToWorkspace(args: {
20092021
}
20102022
} catch (error) {
20112023
return exportFailure(
2024+
args.routeContext,
20122025
getErrorMessage(error, 'Failed to export sandbox files'),
20132026
workspaceFileExportErrorStatus(error),
20142027
args.stdout,
@@ -2157,7 +2170,8 @@ async function collectSandboxOutputFiles(args: {
21572170
// reporting success without them would read as "your script wrote nothing".
21582171
if (!resolvedWorkspaceId || !args.workflowId || !args.executionId) {
21592172
return {
2160-
response: exportFailure(
2173+
response: await exportFailure(
2174+
routeContext,
21612175
'Workspace, workflow, and execution context are required to return files from the sandbox.',
21622176
400,
21632177
args.stdout,
@@ -2188,7 +2202,8 @@ async function collectSandboxOutputFiles(args: {
21882202
) {
21892203
await discardUploadedExecutionFiles(files)
21902204
return {
2191-
response: exportFailure(
2205+
response: await exportFailure(
2206+
routeContext,
21922207
`Sandbox output file "${name}" contains a resolved secret value and was not returned. Write the file without embedding secret values, or export it to a workspace file where its provenance can be recorded.`,
21932208
400,
21942209
args.stdout,

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,47 @@ describe('persistent workbench output confidentiality', () => {
512512
expect(JSON.stringify(output.projected.result)).not.toContain(canary)
513513
expect(JSON.stringify(output.projected.result)).toContain('{{TOKEN}}')
514514
})
515+
it('redacts session credentials when an export write fails', async () => {
516+
io.write.mockRejectedValueOnce(new Error('Write unavailable'))
517+
const current = context()
518+
const raw = await inResourceScope(() =>
519+
executeFunctionExecute(
520+
{
521+
code: 'printf "%s" "$SIM_API_KEY" > session-key.txt; cat session-key.txt; printf data > export.txt',
522+
language: 'shell',
523+
outputs: {
524+
files: [{ path: 'files/export.txt', sandboxPath: 'export.txt' }],
525+
},
526+
},
527+
current
528+
)
529+
)
530+
const credential = await readFile(workerPath('session-key.txt'), 'utf8')
531+
const projected = inspectToolResultForCopilot(
532+
raw,
533+
current.resolvedSecretTraceRegistry,
534+
'function_execute'
535+
)
536+
expect(raw.success).toBe(false)
537+
expect(projected.safe).toBe(true)
538+
expect(JSON.stringify(projected.result)).not.toContain(credential)
539+
expect(JSON.stringify(projected.result)).toContain('{{SIM_API_KEY}}')
540+
})
541+
it.each([
542+
'{ nested: [process.env.SIM_API_KEY] }',
543+
'{ nested: [{ [process.env.SIM_API_KEY]: true }] }',
544+
])('redacts session credentials in returned %s', async (value) => {
545+
const result = await run(
546+
`(await import("node:fs")).writeFileSync("session-key.txt", process.env.SIM_API_KEY); return ${value}`,
547+
[],
548+
'javascript'
549+
)
550+
const credential = await readFile(workerPath('session-key.txt'), 'utf8')
551+
expect(result.raw.success).toBe(true)
552+
expect(result.projected.safe).toBe(true)
553+
expect(JSON.stringify(result.projected.result)).not.toContain(credential)
554+
expect(JSON.stringify(result.projected.result)).toContain('{{SIM_API_KEY}}')
555+
})
515556
it('keeps ordinary binary exports usable when only callback authentication is present', async () => {
516557
const result = await inResourceScope(() =>
517558
executeFunctionExecute(

‎apps/sim/lib/mothership/tools/sandbox-session.ts‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,44 @@ import { resolve } from 'node:path'
44
import { createLogger } from '@sim/logger'
55
import { getErrorMessage } from '@sim/utils/errors'
66
import { generateId } from '@sim/utils/id'
7+
import { toRecord } from '@sim/utils/object'
78
import { env } from '@/lib/core/config/env'
89
import { encryptSecret } from '@/lib/core/security/encryption'
910
import { getBaseUrl } from '@/lib/core/utils/urls'
10-
import type { DurableSecretProvenance } from '@/lib/execution/durable-secret-provenance'
11+
import {
12+
type DurableSecretProvenance,
13+
EXACT_EMPTY_DURABLE_SECRET_PROVENANCE,
14+
} from '@/lib/execution/durable-secret-provenance'
1115
import type { SandboxSessionRequest } from '@/lib/execution/remote-sandbox/types'
1216
import { WorkbenchBootstrap } from '@/lib/mothership/generated/workbench'
1317
import { fetchGo } from '@/lib/mothership/request/go/fetch'
1418
import { mothershipRequestHeaders } from '@/lib/mothership/request/headers'
1519
import { getMothershipBaseURL } from '@/lib/mothership/server/agent-url'
1620
import { sandboxResourceEndpoint } from '@/lib/mothership/tools/sandbox-resources'
1721
import { getSimConnection } from '@/lib/mothership/transport/connection'
22+
import {
23+
containsResolvedSecret,
24+
createResolvedSecretMatcher,
25+
type ResolvedSecretMatcher,
26+
} from '@/executor/utils/resolved-secret-content-projection'
1827

1928
const logger = createLogger('MothershipSandboxSession')
2029

30+
/** Scans parsed sandbox JSON in place, including keys, without allocating a second payload. */
31+
function containsSessionCredential(value: unknown, matcher: ResolvedSecretMatcher): boolean {
32+
if (typeof value === 'string') return containsResolvedSecret(value, matcher)
33+
if (value === null || typeof value !== 'object') return false
34+
if (Array.isArray(value)) return value.some((item) => containsSessionCredential(item, matcher))
35+
const record = toRecord(value)
36+
for (const key in record) {
37+
if (!Object.hasOwn(record, key)) continue
38+
if (containsResolvedSecret(key, matcher) || containsSessionCredential(record[key], matcher)) {
39+
return true
40+
}
41+
}
42+
return false
43+
}
44+
2145
/** Public runtime and private bootstrap share an immutable release directory. */
2246
async function workbenchCli(
2347
userId: string,
@@ -81,13 +105,13 @@ export async function buildMothershipSandboxSession(args: {
81105
if (getSimConnection().mode === 'checkpoint') return { key: args.sessionKey }
82106
const cli = await workbenchCli(args.userId, args.signal)
83107
let cliEnvs: Record<string, string> | undefined
84-
let outputProvenance: DurableSecretProvenance | undefined
108+
let outputProvenance: SandboxSessionRequest['outputProvenance']
85109
try {
86110
const apiKey = `mothership-sandbox:${generateId()}`
87111
const endpoint = env.MOTHERSHIP_SANDBOX_CLI_ENDPOINT?.trim() || getBaseUrl()
88112
const scopedEndpoint = await sandboxResourceEndpoint(endpoint, args, apiKey)
89113
if (scopedEndpoint !== endpoint) {
90-
outputProvenance = {
114+
const provenance: DurableSecretProvenance = {
91115
status: 'exact',
92116
entries: [
93117
{
@@ -98,6 +122,11 @@ export async function buildMothershipSandboxSession(args: {
98122
},
99123
],
100124
}
125+
const matcher = createResolvedSecretMatcher([{ plaintext: apiKey, replacement: '' }])
126+
outputProvenance = (value) =>
127+
matcher && containsSessionCredential(value, matcher)
128+
? provenance
129+
: EXACT_EMPTY_DURABLE_SECRET_PROVENANCE
101130
cliEnvs = {
102131
SIM_API_KEY: apiKey,
103132
...(args.organizationId

0 commit comments

Comments
 (0)