Skip to content

Commit 0fed079

Browse files
icecrasher321icecrasher321
committed
fix(sandbox): preserve broker error identity
1 parent d5b9e48 commit 0fed079

4 files changed

Lines changed: 98 additions & 62 deletions

File tree

‎apps/sim/lib/execution/isolated-vm-worker.cjs‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -951,6 +951,11 @@ async function executeTask(request, executionId) {
951951
// Capture the bridge reference in a closure so hardening can unset the
952952
// global without breaking already-installed brokers.
953953
const __ref = globalThis.__brokerRef;
954+
const __brokerErrors = new WeakMap();
955+
const __rememberBrokerError = __brokerErrors.set.bind(__brokerErrors);
956+
Object.defineProperty(globalThis, '__readBrokerErrorId', {
957+
value: __brokerErrors.get.bind(__brokerErrors), writable: false, configurable: false
958+
});
954959
globalThis.__brokers = globalThis.__brokers || {};
955960
for (const name of ${brokerNamesJson}) {
956961
globalThis.__brokers[name] = async (args) => {
@@ -963,7 +968,9 @@ async function executeTask(request, executionId) {
963968
let response;
964969
try { response = JSON.parse(responseJson); } catch { throw new Error('Invalid broker response'); }
965970
if (typeof response.error === 'string') {
966-
throw new Error(response.error || 'Broker call failed');
971+
const error = new Error(response.error || 'Broker call failed');
972+
__rememberBrokerError(error, response.brokerErrorId);
973+
throw error;
967974
}
968975
return response.resultJson === undefined || response.resultJson === null
969976
? null
@@ -1016,7 +1023,7 @@ async function executeTask(request, executionId) {
10161023
userResult = { success: true }
10171024
} else {
10181025
const wrappedUserCode = `
1019-
(async () => {
1026+
(async () => { const __readBrokerErrorId = globalThis.__readBrokerErrorId;
10201027
try {
10211028
await (async () => {
10221029
${code}
@@ -1029,6 +1036,7 @@ async function executeTask(request, executionId) {
10291036
message: error && error.message ? error.message : String(error),
10301037
name: error && error.name ? error.name : 'Error',
10311038
stack: error && error.stack ? error.stack : '',
1039+
brokerErrorId: __readBrokerErrorId(error),
10321040
},
10331041
});
10341042
}
@@ -1056,7 +1064,12 @@ async function executeTask(request, executionId) {
10561064
return {
10571065
result: null,
10581066
stdout: stdoutChunks.join(''),
1059-
error: convertToCompatibleError(userResult.errorInfo, code),
1067+
error: {
1068+
...convertToCompatibleError(userResult.errorInfo, code),
1069+
...(Number.isSafeInteger(userResult.errorInfo.brokerErrorId)
1070+
? { brokerErrorId: userResult.errorInfo.brokerErrorId }
1071+
: {}),
1072+
},
10601073
timings,
10611074
}
10621075
}
@@ -1233,7 +1246,13 @@ process.on('message', async (msg) => {
12331246
if (pending) {
12341247
clearTimeout(pending.timeout)
12351248
pendingBrokerCalls.delete(msg.brokerId)
1236-
pending.resolve(JSON.stringify({ error: msg.error, resultJson: msg.resultJson }))
1249+
pending.resolve(
1250+
JSON.stringify({
1251+
error: msg.error,
1252+
resultJson: msg.resultJson,
1253+
...(typeof msg.error === 'string' ? { brokerErrorId: msg.brokerId } : {}),
1254+
})
1255+
)
12371256
}
12381257
}
12391258
} catch (err) {

‎apps/sim/lib/execution/isolated-vm.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ interface IsolatedVMTaskRequest {
6666
* Host-side broker handler invoked when isolate code calls a broker.
6767
* Registered per-request via `executeInIsolatedVM(..., { brokers })`.
6868
*/
69-
export type IsolatedVMBrokerHandler = (args: unknown) => Promise<unknown>
69+
export type IsolatedVMBrokerHandler = (args: unknown, brokerId: number) => Promise<unknown>
7070

7171
export interface IsolatedVMExecutionOptions {
7272
/** Broker name → handler. Must cover every broker listed in `request.task.brokers`. */
@@ -104,6 +104,8 @@ interface IsolatedVMTaskTimings {
104104
}
105105

106106
interface IsolatedVMError {
107+
/** Existing broker call ID, set only when a task propagates that broker's error object. */
108+
brokerErrorId?: number
107109
message: string
108110
name: string
109111
stack?: string
@@ -735,7 +737,7 @@ function handleBrokerMessage(
735737
}
736738

737739
Promise.resolve()
738-
.then(() => pending.runInOutboundScope(() => handler(args)))
740+
.then(() => pending.runInOutboundScope(() => handler(args, brokerId)))
739741
.then((resultValue) => {
740742
if (pending.cancelled) {
741743
sendResponse({ error: 'Execution cancelled' })

‎apps/sim/lib/execution/sandbox/run-task.ts‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,16 @@ export async function runSandboxTask<TInput extends SandboxTaskInput>(
6666
}
6767
const brokers: Record<string, IsolatedVMBrokerHandler> = {}
6868
const runInCallerContext = AsyncLocalStorage.snapshot()
69-
let brokerRefusal: ContentRefusedError | undefined
69+
let brokerRefusals: Map<number, ContentRefusedError> | undefined
7070
for (const broker of task.brokers) {
71-
brokers[broker.name] = async (args) => {
71+
brokers[broker.name] = async (args, brokerId) => {
7272
try {
7373
return await runInCallerContext(() => broker.handle(brokerContext, args))
7474
} catch (error) {
75-
if (error instanceof ContentRefusedError) brokerRefusal = error
75+
if (error instanceof ContentRefusedError) {
76+
brokerRefusals ??= new Map()
77+
brokerRefusals.set(brokerId, error)
78+
}
7679
throw error
7780
}
7881
}
@@ -133,8 +136,11 @@ export async function runSandboxTask<TInput extends SandboxTaskInput>(
133136
if (result.error.stack) err.stack = result.error.stack
134137
throw err
135138
}
136-
// IPC carries strings; retain a matching host refusal without reclassifying user-thrown errors.
137-
if (brokerRefusal && result.error.message === brokerRefusal.message) throw brokerRefusal
139+
const brokerRefusal =
140+
result.error.brokerErrorId === undefined
141+
? undefined
142+
: brokerRefusals?.get(result.error.brokerErrorId)
143+
if (brokerRefusal) throw brokerRefusal
138144
throw new SandboxUserCodeError(
139145
result.error.message,
140146
result.error.name || 'SandboxTaskError',

‎apps/sim/lib/mothership/__integration__/content-delivery.integration.ts‎

Lines changed: 60 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -432,58 +432,67 @@ describe('content reaching Chat is admitted per item', () => {
432432
expect(read.body).toContain(fixture.files.entries)
433433
})
434434

435-
it.each(['uncached', 'cached', 'recovered', 'user-error', 'subsequent-error'] as const)(
436-
'keeps generated-document refusals per item (%s archive)',
437-
async (kind) => {
438-
const input = fixture.files.unknown
439-
const read = `await getFileBase64('${input}');`
440-
const code =
441-
kind === 'recovered'
442-
? `try { ${read} } catch {} pdf.addPage().drawText('safe fallback');`
443-
: kind === 'user-error'
444-
? `const error = new Error('File cannot be sent to a model because its secret provenance is unavailable'); error.name = 'ContentRefusedError'; throw error;`
445-
: kind === 'subsequent-error'
446-
? `try { ${read} } catch {} throw new Error('fixture compiler failure');`
447-
: `${read} pdf.addPage().drawText('fixture render');`
448-
const document = await uploadWorkspaceFile(
449-
ws(),
450-
fixture.ids.aliceId,
451-
Buffer.from(`const archiveFixture = '${generateId()}'; ${code}`),
452-
`archive-refusal-${generateId()}.pdf`,
453-
'text/x-python-pdf',
454-
{ notifyWorkspaceChange: false, secretProvenance: { status: 'exact', entries: [] } }
455-
)
456-
if (kind === 'cached') {
457-
const warmed = await downloadWorkspaceFileItems.execute({
458-
principal: createSessionPrincipal({ userId: fixture.ids.aliceId }),
459-
input: { workspaceId: ws(), fileIds: [document.id], folderIds: [] },
460-
})
461-
expect(warmed.renderedDocuments.get(document.id)?.subarray(0, 4).toString()).toBe('%PDF')
462-
}
463-
const route = `/api/v2/files/bulk-download?workspaceId=${ws()}&fileIds=`
464-
const mixed = await cli('GET', `${route}${fixture.files.clean},${document.id}`)
465-
if (kind === 'user-error' || kind === 'subsequent-error') {
466-
expect(mixed.status).toBe(500)
467-
expect(mixed.body).toContain('INTERNAL_ERROR')
468-
return
469-
}
470-
expect(mixed.status, mixed.body).toBe(200)
471-
expect(mixed.body).toContain(CANARY.clean)
472-
expect(mixed.body).not.toContain(CANARY.unknown)
473-
expect(mixed.registry.isPermanentlyIncomplete()).toBe(false)
474-
if (kind === 'recovered') {
475-
expect(mixed.body).toContain('%PDF')
476-
expect(mixed.body).not.toContain('sim-download-report.json')
477-
} else {
478-
expect(mixed.body).not.toContain('%PDF')
479-
expect(mixed.body).toContain('sim-download-report.json')
480-
expect(mixed.body).toContain(document.id)
481-
const alone = await cli('GET', `${route}${document.id}`)
482-
expect(alone.status, alone.body).toBe(403)
483-
expect(alone.body).not.toContain(CANARY.unknown)
484-
}
435+
it.each([
436+
'uncached',
437+
'cached',
438+
'recovered',
439+
'user-error',
440+
'subsequent-error',
441+
'copied-error',
442+
'mutated-refusal',
443+
] as const)('keeps generated-document refusals per item (%s archive)', async (kind) => {
444+
const input = fixture.files.unknown
445+
const read = `await getFileBase64('${input}');`
446+
const code =
447+
kind === 'recovered'
448+
? `try { ${read} } catch {} pdf.addPage().drawText('safe fallback');`
449+
: kind === 'user-error'
450+
? `const error = new Error('File cannot be sent to a model because its secret provenance is unavailable'); error.name = 'ContentRefusedError'; throw error;`
451+
: kind === 'subsequent-error'
452+
? `try { ${read} } catch {} throw new Error('fixture compiler failure');`
453+
: kind === 'copied-error'
454+
? `try { ${read} } catch (error) { throw Object.assign(new Error(error.message), error); }`
455+
: kind === 'mutated-refusal'
456+
? `try { ${read} } catch (error) { error.message = 'fixture reworded refusal'; throw error; }`
457+
: `${read} pdf.addPage().drawText('fixture render');`
458+
const document = await uploadWorkspaceFile(
459+
ws(),
460+
fixture.ids.aliceId,
461+
Buffer.from(`const archiveFixture = '${generateId()}'; ${code}`),
462+
`archive-refusal-${generateId()}.pdf`,
463+
'text/x-python-pdf',
464+
{ notifyWorkspaceChange: false, secretProvenance: { status: 'exact', entries: [] } }
465+
)
466+
if (kind === 'cached') {
467+
const warmed = await downloadWorkspaceFileItems.execute({
468+
principal: createSessionPrincipal({ userId: fixture.ids.aliceId }),
469+
input: { workspaceId: ws(), fileIds: [document.id], folderIds: [] },
470+
})
471+
expect(warmed.renderedDocuments.get(document.id)?.subarray(0, 4).toString()).toBe('%PDF')
485472
}
486-
)
473+
const route = `/api/v2/files/bulk-download?workspaceId=${ws()}&fileIds=`
474+
const mixed = await cli('GET', `${route}${fixture.files.clean},${document.id}`)
475+
if (kind === 'user-error' || kind === 'subsequent-error' || kind === 'copied-error') {
476+
expect(mixed.status).toBe(500)
477+
expect(mixed.body).toContain('INTERNAL_ERROR')
478+
return
479+
}
480+
expect(mixed.status, mixed.body).toBe(200)
481+
expect(mixed.body).toContain(CANARY.clean)
482+
expect(mixed.body).not.toContain(CANARY.unknown)
483+
expect(mixed.registry.isPermanentlyIncomplete()).toBe(false)
484+
if (kind === 'recovered') {
485+
expect(mixed.body).toContain('%PDF')
486+
expect(mixed.body).not.toContain('sim-download-report.json')
487+
} else {
488+
expect(mixed.body).not.toContain('%PDF')
489+
expect(mixed.body).toContain('sim-download-report.json')
490+
expect(mixed.body).toContain(document.id)
491+
const alone = await cli('GET', `${route}${document.id}`)
492+
expect(alone.status, alone.body).toBe(403)
493+
expect(alone.body).not.toContain(CANARY.unknown)
494+
}
495+
})
487496

488497
it('answers files search with locations and withheld unreadable previews (#5, misroute)', async () => {
489498
const read = await cli(

0 commit comments

Comments
 (0)