Skip to content

Commit 01fedd0

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(billing): address sandbox usage review feedback
1 parent 6393c4e commit 01fedd0

11 files changed

Lines changed: 243 additions & 41 deletions

File tree

apps/sim/app/api/function/execute/route.test.ts

Lines changed: 107 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ import {
1515
} from '@sim/testing'
1616
import { NextRequest } from 'next/server'
1717
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
18+
import {
19+
BILLING_ATTRIBUTION_HEADER,
20+
type BillingAttributionSnapshot,
21+
serializeBillingAttributionHeader,
22+
} from '@/lib/billing/core/billing-attribution'
1823
import { INTERNAL_EXECUTION_DEADLINE_HEADER } from '@/lib/execution/execution-deadline-header'
1924
import {
2025
MOUNTED_WORKSPACE_FILES_PROVENANCE_KEY,
@@ -39,6 +44,22 @@ function grantedAccess(workspaceId: string) {
3944
}
4045
}
4146

47+
function billingAttributionHeaders(workspaceId = 'workspace-1'): Record<string, string> {
48+
const attribution: BillingAttributionSnapshot = {
49+
actorUserId: 'user-123',
50+
workspaceId,
51+
organizationId: null,
52+
billedAccountUserId: 'user-123',
53+
billingEntity: { type: 'user', id: 'user-123' },
54+
billingPeriod: {
55+
start: '2026-08-01T00:00:00.000Z',
56+
end: '2026-09-01T00:00:00.000Z',
57+
},
58+
payerSubscription: null,
59+
}
60+
return { [BILLING_ATTRIBUTION_HEADER]: serializeBillingAttributionHeader(attribution) }
61+
}
62+
4263
const {
4364
mockExecuteInSandbox,
4465
mockExecuteInIsolatedVM,
@@ -318,26 +339,8 @@ describe('Function Execute API Route', () => {
318339
expect(mockWriteWorkspaceFileByPath).not.toHaveBeenCalled()
319340
})
320341

321-
it('rejects an export whose workspace is derived from a body-supplied workflowId', async () => {
342+
it('fails closed before deriving an export workspace from incomplete workflow context', async () => {
322343
envFlagsMock.isRemoteSandboxEnabled = true
323-
mockExecuteInSandbox.mockResolvedValueOnce({
324-
result: 'done',
325-
stdout: 'ok',
326-
sandboxId: 'sandbox-123',
327-
exportedFiles: { '/tmp/out.txt': 'owned by attacker' },
328-
})
329-
workflowsUtilsMock.getWorkflowById.mockResolvedValueOnce({
330-
id: 'workflow-victim',
331-
workspaceId: 'workspace-victim',
332-
})
333-
mockResolveWorkspaceAccess.mockResolvedValue({
334-
exists: true,
335-
hasAccess: false,
336-
canWrite: false,
337-
canAdmin: false,
338-
workspace: { id: 'workspace-victim' },
339-
permission: null,
340-
})
341344

342345
const req = createMockRequest('POST', {
343346
code: 'print("done")',
@@ -350,7 +353,12 @@ describe('Function Execute API Route', () => {
350353

351354
const response = await POST(req)
352355

353-
expect(response.status).toBe(403)
356+
expect(response.status).toBe(503)
357+
await expect(response.json()).resolves.toMatchObject({
358+
retryable: false,
359+
code: 'sandbox_usage_attribution_invalid',
360+
})
361+
expect(mockExecuteInSandbox).not.toHaveBeenCalled()
354362
expect(mockWriteWorkspaceFileByPath).not.toHaveBeenCalled()
355363
})
356364

@@ -537,6 +545,85 @@ describe('Function Execute API Route', () => {
537545
expect(mockExecuteInIsolatedVM).not.toHaveBeenCalled()
538546
})
539547

548+
it.each([
549+
{
550+
label: 'execution ID',
551+
body: { workflowId: 'workflow-1', workspaceId: 'workspace-1' },
552+
},
553+
{
554+
label: 'trusted billing attribution',
555+
body: {
556+
workflowId: 'workflow-1',
557+
workspaceId: 'workspace-1',
558+
executionId: 'execution-1',
559+
},
560+
},
561+
])(
562+
'fails before remote sandbox creation when workflow usage lacks $label',
563+
async ({ body }) => {
564+
envFlagsMock.isRemoteSandboxEnabled = true
565+
566+
const response = await POST(
567+
createMockRequest('POST', { code: 'print("ready")', language: 'python', ...body })
568+
)
569+
570+
expect(response.status).toBe(503)
571+
await expect(response.json()).resolves.toMatchObject({
572+
success: false,
573+
retryable: false,
574+
code: 'sandbox_usage_attribution_invalid',
575+
})
576+
expect(mockExecuteInSandbox).not.toHaveBeenCalled()
577+
}
578+
)
579+
580+
it('passes complete trusted workflow attribution to remote sandbox billing', async () => {
581+
envFlagsMock.isRemoteSandboxEnabled = true
582+
583+
const response = await POST(
584+
createMockRequest(
585+
'POST',
586+
{
587+
code: 'print("ready")',
588+
language: 'python',
589+
workflowId: 'workflow-1',
590+
workspaceId: 'workspace-1',
591+
executionId: 'execution-1',
592+
},
593+
billingAttributionHeaders()
594+
)
595+
)
596+
597+
expect(response.status).toBe(200)
598+
expect(mockExecuteInSandbox).toHaveBeenCalledWith(
599+
expect.objectContaining({
600+
usageContext: expect.objectContaining({
601+
workspaceId: 'workspace-1',
602+
workflowId: 'workflow-1',
603+
executionId: 'execution-1',
604+
billingAttribution: expect.objectContaining({ actorUserId: 'user-123' }),
605+
}),
606+
})
607+
)
608+
})
609+
610+
it('allows a non-workflow remote call without sandbox usage attribution', async () => {
611+
envFlagsMock.isRemoteSandboxEnabled = true
612+
613+
const response = await POST(
614+
createMockRequest('POST', {
615+
code: 'print("ready")',
616+
language: 'python',
617+
workspaceId: 'workspace-1',
618+
})
619+
)
620+
621+
expect(response.status).toBe(200)
622+
expect(mockExecuteInSandbox).toHaveBeenCalledWith(
623+
expect.objectContaining({ usageContext: undefined })
624+
)
625+
})
626+
540627
it('forces import-free JavaScript into the remote runtime when a Sim sandbox is selected', async () => {
541628
envFlagsMock.isRemoteSandboxEnabled = true
542629

apps/sim/lib/billing/core/usage-log.test.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ describe('recordUsage', () => {
8686
})
8787

8888
it('commits canonical usage rows with deterministic event keys and billing scope', async () => {
89-
await recordUsage({
89+
const insertedCost = await recordUsage({
9090
userId: 'external-actor',
9191
workspaceId: 'workspace-1',
9292
billingEntity: { type: 'organization', id: 'workspace-org' },
@@ -124,9 +124,34 @@ describe('recordUsage', () => {
124124
expect(mockOnConflictDoNothing.mock.calls[0][0]).toMatchObject({
125125
target: usageLog.eventKey,
126126
})
127+
expect(insertedCost).toBeCloseTo(0.3, 8)
127128
expect(mockGetHighestPrioritySubscription).not.toHaveBeenCalled()
128129
})
129130

131+
it('returns zero when idempotency skips a duplicate event', async () => {
132+
mockReturning.mockResolvedValueOnce([])
133+
134+
const insertedCost = await recordUsage({
135+
userId: 'user-1',
136+
billingEntity: { type: 'user', id: 'user-1' },
137+
billingPeriod: {
138+
start: new Date('2026-05-01T00:00:00.000Z'),
139+
end: new Date('2026-06-01T00:00:00.000Z'),
140+
},
141+
entries: [
142+
{
143+
category: 'tool',
144+
source: 'workflow',
145+
description: 'Code sandbox',
146+
cost: 0.1,
147+
eventKey: 'sandbox-event',
148+
},
149+
],
150+
})
151+
152+
expect(insertedCost).toBe(0)
153+
})
154+
130155
it('uses pre-resolved billing context without loading subscriptions', async () => {
131156
await recordUsage({
132157
userId: 'user-1',

apps/sim/lib/billing/core/usage-log.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ export async function getStampedPeriodRangeUsageCostByUser(
381381
* as the pre-cutover period baseline and for low-frequency billing trackers,
382382
* but usage writes no longer contend on the user_stats row.
383383
*/
384-
export async function recordUsage(params: RecordUsageParams): Promise<void> {
384+
export async function recordUsage(params: RecordUsageParams): Promise<number> {
385385
// The usage ledger is written regardless of BILLING_ENABLED so it is the
386386
// single, universal source of truth for cost (including self-hosted, where
387387
// it powers the logs-page cost display). Billing *enforcement* (Stripe /
@@ -400,7 +400,7 @@ export async function recordUsage(params: RecordUsageParams): Promise<void> {
400400
const validEntries = entries.filter((e) => e.cost > 0)
401401

402402
if (validEntries.length === 0) {
403-
return
403+
return 0
404404
}
405405

406406
if (workspaceId && (!billingEntity || !billingPeriod)) {
@@ -473,6 +473,8 @@ export async function recordUsage(params: RecordUsageParams): Promise<void> {
473473
entryCount: validEntries.length,
474474
sources: [...new Set(validEntries.map((e) => e.source))],
475475
})
476+
477+
return insertedCost
476478
}
477479

478480
/**

apps/sim/lib/billing/sandbox-usage-outbox.test.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,14 +81,28 @@ async function runHandler(value: SandboxUsageOutboxPayloadV1): Promise<void> {
8181
await sandboxUsageOutboxHandlers[SANDBOX_USAGE_OUTBOX_EVENT_TYPE](value, context)
8282
}
8383

84+
function costTotalUpdate(index: number): { text: string; params: unknown[] } {
85+
const updates = dbChainMockFns.set.mock.calls
86+
.map(([value]) => value)
87+
.filter(
88+
(value): value is { costTotal: { strings: string[]; values: unknown[] } } =>
89+
typeof value === 'object' && value !== null && 'costTotal' in value
90+
)
91+
const update = updates[index].costTotal
92+
return {
93+
text: update.strings.join(''),
94+
params: update.values.filter((value) => typeof value === 'string' || typeof value === 'number'),
95+
}
96+
}
97+
8498
afterAll(resetDbChainMock)
8599

86100
describe('sandbox usage outbox finalizer', () => {
87101
beforeEach(() => {
88102
vi.clearAllMocks()
89103
resetDbChainMock()
90104
queueTableRows(usageLog, [{ cost: '0.02' }])
91-
mockRecordUsage.mockResolvedValue(undefined)
105+
mockRecordUsage.mockResolvedValue(0.00046)
92106
mockTerminateById.mockResolvedValue('terminated')
93107
})
94108

@@ -114,6 +128,10 @@ describe('sandbox usage outbox finalizer', () => {
114128
)
115129
expect(dbChainMockFns.execute).toHaveBeenCalledOnce()
116130
expect(dbChainMockFns.update).toHaveBeenCalled()
131+
expect(costTotalUpdate(0)).toEqual({
132+
text: 'GREATEST(COALESCE(, 0) + ::numeric, ::numeric)',
133+
params: ['workflowExecutionLogs.costTotal', '0.00046', '0.02'],
134+
})
117135
})
118136

119137
it('terminates and checkpoints a sandbox whose terminal timestamp is missing', async () => {
@@ -143,11 +161,20 @@ describe('sandbox usage outbox finalizer', () => {
143161
})
144162

145163
it('uses the same ledger event key when finalization is replayed', async () => {
164+
mockRecordUsage.mockResolvedValueOnce(0.00046).mockResolvedValueOnce(0)
165+
queueTableRows(usageLog, [{ cost: '0.02' }])
166+
146167
await runHandler(payload())
147168
await runHandler(payload())
148169

149170
const firstEventKey = mockRecordUsage.mock.calls[0][0].entries[0].eventKey
150171
const secondEventKey = mockRecordUsage.mock.calls[1][0].entries[0].eventKey
151172
expect(firstEventKey).toBe(secondEventKey)
173+
expect(costTotalUpdate(0).params).toEqual([
174+
'workflowExecutionLogs.costTotal',
175+
'0.00046',
176+
'0.02',
177+
])
178+
expect(costTotalUpdate(1).params).toEqual(['workflowExecutionLogs.costTotal', '0', '0.02'])
152179
})
153180
})

apps/sim/lib/billing/sandbox-usage-outbox.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ async function finalizeSandboxUsage(
277277

278278
await db.transaction(async (tx) => {
279279
await tx.execute(sql`select pg_advisory_xact_lock(hashtextextended(${payload.executionId}, 0))`)
280-
await recordUsage({
280+
const insertedCost = await recordUsage({
281281
userId: payload.billingAttribution.actorUserId,
282282
entries: [
283283
{
@@ -323,7 +323,7 @@ async function finalizeSandboxUsage(
323323
await tx
324324
.update(workflowExecutionLogs)
325325
.set({
326-
costTotal: sql`GREATEST(COALESCE(${workflowExecutionLogs.costTotal}, 0), ${ledgerCost})`,
326+
costTotal: sql`GREATEST(COALESCE(${workflowExecutionLogs.costTotal}, 0) + ${insertedCost.toString()}::numeric, ${ledgerCost}::numeric)`,
327327
})
328328
.where(eq(workflowExecutionLogs.executionId, payload.executionId))
329329
})

apps/sim/lib/execution/non-retryable-error.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,19 @@ export class SandboxUsagePersistenceError extends NonRetryableExecutionError {
3939
}
4040
}
4141

42+
/** A workflow sandbox request lacks the trusted context required for durable billing. */
43+
export class SandboxUsageAttributionError extends NonRetryableExecutionError {
44+
readonly code = 'sandbox_usage_attribution_invalid' as const
45+
46+
constructor(options?: ErrorOptions) {
47+
super(
48+
'Sim could not establish trusted billing attribution for this Function sandbox. The sandbox was not created.',
49+
options
50+
)
51+
this.name = 'SandboxUsageAttributionError'
52+
}
53+
}
54+
4255
export function isNonRetryableExecutionError(error: unknown): boolean {
4356
return Boolean(
4457
findCause(error, (cause): cause is NonRetryableExecutionError => {
@@ -64,3 +77,9 @@ export function isSandboxUsagePersistenceError(
6477
): error is SandboxUsagePersistenceError {
6578
return Boolean(findCause(error, (cause) => cause instanceof SandboxUsagePersistenceError))
6679
}
80+
81+
export function isSandboxUsageAttributionError(
82+
error: unknown
83+
): error is SandboxUsageAttributionError {
84+
return Boolean(findCause(error, (cause) => cause instanceof SandboxUsageAttributionError))
85+
}

apps/sim/lib/execution/remote-sandbox/conformance.test.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -650,8 +650,17 @@ describe.each(PROVIDERS)('sandbox conformance [%s]', (provider) => {
650650
}
651651

652652
await expect(
653-
executeInSandbox({ code: 'sleep()', language: CodeLanguage.Python, timeoutMs: 1000 })
653+
executeInSandbox({
654+
code: 'sleep()',
655+
language: CodeLanguage.Python,
656+
timeoutMs: 1000,
657+
usageContext,
658+
})
654659
).rejects.toMatchObject({ name: 'AbortError', message: 'timeout' })
660+
expect(mockReleaseAndProcessSandboxUsage).toHaveBeenCalledWith(
661+
'sandbox-usage-event',
662+
expect.objectContaining({ outcome: 'timeout' })
663+
)
655664
})
656665

657666
it('normalizes JavaScript code budget expiry to a typed timeout abort', async () => {
@@ -994,8 +1003,12 @@ describe.each(PROVIDERS)('sandbox conformance [%s]', (provider) => {
9941003
}
9951004

9961005
await expect(
997-
executeShellInSandbox({ code: 'sleep infinity', timeoutMs: 1000 })
1006+
executeShellInSandbox({ code: 'sleep infinity', timeoutMs: 1000, usageContext })
9981007
).rejects.toMatchObject({ name: 'AbortError', message: 'timeout' })
1008+
expect(mockReleaseAndProcessSandboxUsage).toHaveBeenCalledWith(
1009+
'sandbox-usage-event',
1010+
expect.objectContaining({ outcome: 'timeout' })
1011+
)
9991012
})
10001013

10011014
it('preserves a user process exit code 124 as an ordinary failure', async () => {

apps/sim/lib/execution/remote-sandbox/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,7 @@ async function executeInSandboxWithinBudget(
686686
throw error
687687
}
688688
throwIfAborted(signal)
689+
if (execution.timedOut) outcome = 'timeout'
689690
throwIfSandboxTimedOut(execution)
690691

691692
if (execution.error) {
@@ -822,6 +823,7 @@ async function executeShellInSandboxWithinBudget(
822823
throw error
823824
}
824825
throwIfAborted(signal)
826+
if (result.timedOut) outcome = 'timeout'
825827
throwIfSandboxTimedOut(result)
826828

827829
const stdout = [result.stdout, result.stderr].filter(Boolean).join('\n')

0 commit comments

Comments
 (0)