Skip to content

Commit 3a741cb

Browse files
committed
fix(sandbox): preserve completed table mutation outcomes
1 parent 1c8dce1 commit 3a741cb

2 files changed

Lines changed: 65 additions & 5 deletions

File tree

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,44 @@ describe('sandbox API provenance admission', () => {
425425
}
426426
)
427427

428+
it('preserves mutation completion and withholds its body when provenance storage fails', async () => {
429+
const mutationPath = join(root, 'mutation.json')
430+
const response = await sandboxApi(
431+
'/api/v2/tables/fixture/rows',
432+
async () => {
433+
await writeFile(mutationPath, JSON.stringify({ committed: true, completed: false }))
434+
const evalCommand = redis.eval.bind(redis)
435+
vi.spyOn(redis, 'eval').mockImplementation((...args) => {
436+
if (String(args[2]).startsWith('mothership:workbench-provenance:v2:')) {
437+
return Promise.reject(new Error('Synthetic provenance storage failure'))
438+
}
439+
return evalCommand(...args)
440+
})
441+
await reportTableRowDelivery(
442+
{
443+
version: 1,
444+
complete: true,
445+
scope,
446+
entries: [{ name: 'TOKEN', encryptedValue: catalog[0].encryptedValue }],
447+
},
448+
[{ value: canary }]
449+
)
450+
await writeFile(mutationPath, JSON.stringify({ committed: true, completed: true }))
451+
return Response.json({ data: { value: canary } }, { status: 201 })
452+
},
453+
'POST'
454+
)
455+
expect(JSON.parse(await readFile(mutationPath, 'utf8'))).toEqual({
456+
committed: true,
457+
completed: true,
458+
})
459+
expect(response.status).toBe(502)
460+
const body = await response.text()
461+
expect(body).not.toContain(canary)
462+
expect(body).toContain('completed with HTTP 201')
463+
expect(body).toContain('Do not retry a mutation automatically')
464+
})
465+
428466
it.each(['file', 'table'] as const)(
429467
'preserves an explicit unknown %s delivery as unknown',
430468
async (source) => {

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

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import { getInternalApiBaseUrl } from '@/lib/core/utils/urls'
1010
import {
1111
type DurableSecretProvenance,
1212
durableSecretProvenanceFromEnvelope,
13+
EXACT_EMPTY_DURABLE_SECRET_PROVENANCE,
14+
mergeDurableSecretProvenance,
1315
} from '@/lib/execution/durable-secret-provenance'
1416
import { recordExistingSessionFileInput } from '@/lib/execution/remote-sandbox/session-file-provenance'
1517
import { createResourceEffectTransport } from '@/lib/mothership/agent-cli/resource-effects'
@@ -98,6 +100,7 @@ async function proxyAuthorizedSandboxRequest(
98100
const forwarded = new Request(target, init)
99101
const effects: ResourceChange[] = []
100102
let response: Response | undefined
103+
let rowProvenance: DurableSecretProvenance | undefined
101104
let dispatched = false
102105
/** Invoke the same handler with private request identity; the declared use case still authorizes current domain access. */
103106
const matched = matchV2Route(path)
@@ -132,13 +135,14 @@ async function proxyAuthorizedSandboxRequest(
132135
const deliver = async () => {
133136
dispatched = true
134137
let fileObserved = false
135-
let rowsObserved = false
136138
const result = await observeTableRowDelivery(
137139
async (provenance, _values, extras) => {
138-
await recordInput(
139-
extras.unprovenancedErrorText ? false : durableSecretProvenanceFromEnvelope(provenance)
140+
rowProvenance = mergeDurableSecretProvenance(
141+
rowProvenance ?? EXACT_EMPTY_DURABLE_SECRET_PROVENANCE,
142+
extras.unprovenancedErrorText
143+
? { status: 'unknown' }
144+
: durableSecretProvenanceFromEnvelope(provenance)
140145
)
141-
rowsObserved = true
142146
},
143147
() =>
144148
observeWorkspaceFileDelivery(async (provenance) => {
@@ -148,7 +152,7 @@ async function proxyAuthorizedSandboxRequest(
148152
)
149153
try {
150154
if (fileRead && !fileObserved && result.ok && result.body) await recordInput(false)
151-
else if (!fileObserved && !rowsObserved) {
155+
else if (!fileObserved && !rowProvenance) {
152156
/** Missing producer evidence is unrecorded, not proof that the machine received a secret. */
153157
logger.warn('Sandbox API response has no recorded secret provenance', {
154158
method,
@@ -208,6 +212,24 @@ async function proxyAuthorizedSandboxRequest(
208212
}
209213
})
210214
)
215+
if (rowProvenance) {
216+
// Row mutations have already committed; admission must not interrupt completion or effects.
217+
try {
218+
await recordInput(rowProvenance)
219+
} catch {
220+
await response.body?.cancel().catch(() => {})
221+
logger.warn('Sandbox response provenance could not be recorded after API completion', {
222+
toolCallId: scope.toolCallId,
223+
status: response.status,
224+
})
225+
response = Response.json(
226+
{
227+
error: `API request completed with HTTP ${response.status}, but its result could not be returned safely. Do not retry a mutation automatically; read the resource to check its current state.`,
228+
},
229+
{ status: 502 }
230+
)
231+
}
232+
}
211233
return new Response(request.method === 'HEAD' ? null : response.body, {
212234
status: response.status,
213235
statusText: response.statusText,

0 commit comments

Comments
 (0)