Skip to content

Commit b19c029

Browse files
committed
fix(security): close two disclosure gaps and finish the slot-leak fix
The payer-pool gate only covered the workspace branch. A personal API key that omits workspaceId takes the account branch, where getHighestPrioritySubscription resolves an organization subscription from any member row regardless of role - so a plain member read the organization-wide credit and storage pool by dropping one query parameter. The account branch is now gated by the same authority, and the storage pool is not queried when it may not be disclosed. Forcing that branch self-scoped instead would have downgraded plan, period, and status, which is what a member needs to see whether the org is blocked. GET /api/v1/logs/executions/[executionId] emitted the workflow snapshot raw, carrying password sub-block values and oauth-input credential ids. It now shares the sanitizer the v2 read already used, extracted so there is one implementation rather than two. Env-var references are still preserved. cancelWorkflowGroupExecution itself was unguarded, so an unexpected throw from its transaction escaped ahead of every release site - the same reservation leak this branch set out to close, still open on the adjacent path. It now releases through the shared predicate and rethrows, because a failed transition means the cell state is unknown and a success-shaped answer would be a lie. The comment claiming the abort record cannot be taken back was false and now states the real reason: a refusal is always a terminal-or-absent state.
1 parent 123efba commit b19c029

10 files changed

Lines changed: 424 additions & 60 deletions

File tree

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { NextRequest, NextResponse } from 'next/server'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const mocks = vi.hoisted(() => ({
8+
checkRateLimit: vi.fn(),
9+
validateWorkspaceAccess: vi.fn(),
10+
getPublicWorkflowLog: vi.fn(),
11+
getUserLimits: vi.fn(),
12+
}))
13+
14+
vi.mock('@/app/api/v1/middleware', () => ({
15+
checkRateLimit: mocks.checkRateLimit,
16+
createRateLimitResponse: () => NextResponse.json({ error: 'Rate limit' }, { status: 429 }),
17+
validateWorkspaceAccess: mocks.validateWorkspaceAccess,
18+
}))
19+
20+
vi.mock('@/lib/logs/public-queries', () => ({
21+
getPublicWorkflowLog: mocks.getPublicWorkflowLog,
22+
}))
23+
24+
vi.mock('@/app/api/v1/logs/meta', () => ({
25+
getUserLimits: mocks.getUserLimits,
26+
createApiResponse: <T, L>(data: T, limits: L) => ({ body: { ...data, limits }, headers: {} }),
27+
}))
28+
29+
/**
30+
* Overrides the global stub, whose empty `subBlocks` would let the sanitizer
31+
* no-op and make this suite pass against an unsanitized route.
32+
*/
33+
vi.mock('@/blocks/registry', () => ({
34+
getBlock: vi.fn(() => ({
35+
name: 'Gmail',
36+
subBlocks: [
37+
{ id: 'credential', type: 'oauth-input' },
38+
{ id: 'apiKey', type: 'short-input', password: true },
39+
{ id: 'envApiKey', type: 'short-input', password: true },
40+
{ id: 'subject', type: 'short-input' },
41+
],
42+
outputs: {},
43+
})),
44+
getAllBlocks: vi.fn(() => []),
45+
getLatestBlock: vi.fn(() => undefined),
46+
getBlockRegistry: vi.fn(() => ({})),
47+
getBlockByToolName: vi.fn(() => undefined),
48+
}))
49+
50+
import { GET } from '@/app/api/v1/logs/executions/[executionId]/route'
51+
52+
const rateLimit = {
53+
allowed: true,
54+
userId: 'user-1',
55+
limit: 100,
56+
remaining: 99,
57+
resetAt: new Date('2026-08-11T00:00:00Z'),
58+
}
59+
60+
function snapshot() {
61+
return {
62+
blocks: {
63+
'block-1': {
64+
id: 'block-1',
65+
type: 'gmail',
66+
subBlocks: {
67+
credential: { id: 'credential', type: 'oauth-input', value: 'credential-row-id' },
68+
apiKey: { id: 'apiKey', type: 'short-input', value: 'literal-secret-value' },
69+
envApiKey: { id: 'envApiKey', type: 'short-input', value: '{{GMAIL_API_KEY}}' },
70+
subject: { id: 'subject', type: 'short-input', value: 'Weekly digest' },
71+
},
72+
},
73+
},
74+
edges: [],
75+
}
76+
}
77+
78+
function requestFor(executionId: string) {
79+
return {
80+
request: new NextRequest(`http://localhost:3000/api/v1/logs/executions/${executionId}`),
81+
context: { params: Promise.resolve({ executionId }) },
82+
}
83+
}
84+
85+
describe('GET /api/v1/logs/executions/[executionId]', () => {
86+
beforeEach(() => {
87+
vi.clearAllMocks()
88+
mocks.checkRateLimit.mockResolvedValue(rateLimit)
89+
mocks.validateWorkspaceAccess.mockResolvedValue(null)
90+
mocks.getUserLimits.mockResolvedValue({ usage: { plan: 'free' } })
91+
mocks.getPublicWorkflowLog.mockResolvedValue({
92+
workflowId: 'workflow-1',
93+
workspaceId: 'workspace-1',
94+
workflowState: snapshot(),
95+
trigger: 'api',
96+
startedAt: new Date('2026-08-11T00:00:00Z'),
97+
endedAt: new Date('2026-08-11T00:00:01Z'),
98+
totalDurationMs: 1000,
99+
costTotal: '0.01',
100+
})
101+
})
102+
103+
it('redacts credentials from the snapshot while preserving env-var references', async () => {
104+
const { request, context } = requestFor('execution-1')
105+
const response = await GET(request, context)
106+
const body = await response.json()
107+
108+
expect(response.status).toBe(200)
109+
110+
const subBlocks = body.workflowState.blocks['block-1'].subBlocks
111+
expect(subBlocks.credential.value).toBeNull()
112+
expect(subBlocks.apiKey.value).toBeNull()
113+
expect(subBlocks.envApiKey.value).toBe('{{GMAIL_API_KEY}}')
114+
expect(subBlocks.subject.value).toBe('Weekly digest')
115+
expect(JSON.stringify(body)).not.toContain('literal-secret-value')
116+
expect(JSON.stringify(body)).not.toContain('credential-row-id')
117+
})
118+
119+
it('keeps the surrounding response shape intact', async () => {
120+
const { request, context } = requestFor('execution-1')
121+
const body = await (await GET(request, context)).json()
122+
123+
expect(body).toMatchObject({
124+
executionId: 'execution-1',
125+
workflowId: 'workflow-1',
126+
executionMetadata: {
127+
trigger: 'api',
128+
startedAt: '2026-08-11T00:00:00.000Z',
129+
endedAt: '2026-08-11T00:00:01.000Z',
130+
totalDurationMs: 1000,
131+
cost: { total: 0.01 },
132+
},
133+
limits: { usage: { plan: 'free' } },
134+
})
135+
})
136+
137+
it('reports a missing snapshot as not found', async () => {
138+
mocks.getPublicWorkflowLog.mockResolvedValueOnce({
139+
workflowId: 'workflow-1',
140+
workspaceId: 'workspace-1',
141+
workflowState: null,
142+
trigger: 'api',
143+
startedAt: new Date('2026-08-11T00:00:00Z'),
144+
endedAt: null,
145+
totalDurationMs: 1000,
146+
costTotal: null,
147+
})
148+
149+
const { request, context } = requestFor('execution-1')
150+
const response = await GET(request, context)
151+
152+
expect(response.status).toBe(404)
153+
expect(await response.json()).toEqual({ error: 'Workflow state snapshot not found' })
154+
})
155+
})

apps/sim/app/api/v1/logs/executions/[executionId]/route.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { v1GetExecutionContract } from '@/lib/api/contracts/v1/logs'
44
import { parseRequest } from '@/lib/api/server'
55
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
66
import { getPublicWorkflowLog } from '@/lib/logs/public-queries'
7+
import { sanitizeExecutionSnapshotState } from '@/lib/logs/snapshot-sanitizer'
78
import { createApiResponse, getUserLimits } from '@/app/api/v1/logs/meta'
89
import {
910
checkRateLimit,
@@ -50,14 +51,21 @@ export const GET = withRouteHandler(
5051
return NextResponse.json({ error: 'Workflow execution not found' }, { status: 404 })
5152
}
5253

53-
if (!workflowLog.workflowState) {
54+
/**
55+
* The stored snapshot carries `password: true` sub-block values and `oauth-input`
56+
* credential ids, so it is redacted before it reaches this public wire — the same
57+
* treatment the v2 run detail applies. A snapshot the sanitizer cannot walk projects
58+
* as `null`, which keeps the pre-existing "not found" outcome for an absent one.
59+
*/
60+
const workflowState = sanitizeExecutionSnapshotState(workflowLog.workflowState)
61+
if (!workflowState) {
5462
return NextResponse.json({ error: 'Workflow state snapshot not found' }, { status: 404 })
5563
}
5664

5765
const response = {
5866
executionId,
5967
workflowId: workflowLog.workflowId,
60-
workflowState: workflowLog.workflowState,
68+
workflowState,
6169
executionMetadata: {
6270
trigger: workflowLog.trigger,
6371
startedAt: workflowLog.startedAt.toISOString(),
@@ -70,9 +78,7 @@ export const GET = withRouteHandler(
7078
}
7179

7280
logger.debug(`Successfully fetched execution data for: ${executionId}`)
73-
logger.debug(
74-
`Workflow state contains ${countWorkflowStateBlocks(workflowLog.workflowState)} blocks`
75-
)
81+
logger.debug(`Workflow state contains ${countWorkflowStateBlocks(workflowState)} blocks`)
7682

7783
// Get user's workflow execution limits and usage
7884
const limits = await getUserLimits(userId)

apps/sim/lib/api/contracts/v2/billing.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,16 @@ export const v2BillingStatusQuerySchema = z.object({
3737
* and source analytics deliberately live outside this status resource.
3838
*
3939
* `credits` and `storage` report the resolved payer's pooled allowances, which
40-
* are shared across every workspace that payer funds. They are populated only
41-
* for a caller who may manage that payer's billing: the billed account holder,
42-
* or an admin of the hosting organization. Billing authority is a property of
43-
* a person, so an actor-less workspace API key never qualifies. Every other
44-
* caller reads both as `null` while still seeing the plan, period, and
45-
* standing that the workspace already surfaces to them — enough to monitor for
46-
* `limit_exceeded` and `billing_blocked`.
40+
* are shared across every workspace and member that payer funds. They are
41+
* populated only for a caller who may manage that payer's billing: the billed
42+
* account holder, or an admin of the owning organization. Billing authority is
43+
* a property of a person, so an actor-less workspace API key never qualifies.
44+
* This holds on both scopes — omitting `workspaceId` resolves the payer from
45+
* the caller's own subscriptions and organization memberships, and plain
46+
* membership is not authority over the organization's pool. Every other caller
47+
* reads both as `null` while still seeing the plan, period, and standing of
48+
* the payer that funds them — enough to monitor for `limit_exceeded` and
49+
* `billing_blocked`.
4750
*/
4851
export const v2BillingStatusDataSchema = z
4952
.object({

apps/sim/lib/billing/application/billing-use-cases.test.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@ const mocks = vi.hoisted(() => ({
2525
getWorkspaceUsageLogs: vi.fn(),
2626
recordAudit: vi.fn(),
2727
canUserManageWorkspaceBilling: vi.fn(),
28+
canUserManageBillingEntity: vi.fn(),
2829
}))
2930

3031
vi.mock('@/lib/billing/core/workspace-billing-authority', () => ({
3132
canUserManageWorkspaceBilling: mocks.canUserManageWorkspaceBilling,
33+
canUserManageBillingEntity: mocks.canUserManageBillingEntity,
3234
}))
3335

3436
vi.mock('@/lib/workspaces/application/workspace-context', () => ({
@@ -100,6 +102,7 @@ describe('billing application use cases', () => {
100102
mocks.loadWorkspace.mockResolvedValue(workspaceContext)
101103
mocks.resolvePermission.mockResolvedValue('read')
102104
mocks.canUserManageWorkspaceBilling.mockResolvedValue(false)
105+
mocks.canUserManageBillingEntity.mockResolvedValue(false)
103106
mocks.checkUsageStatus.mockResolvedValue({ currentUsage: 1, limit: 10, isExceeded: false })
104107
mocks.checkAttributedBlocks.mockResolvedValue({ blocked: false })
105108
mocks.toUsageLimitSubscription.mockReturnValue(null)
@@ -289,6 +292,81 @@ describe('billing application use cases', () => {
289292
expect(result.storage).not.toBeNull()
290293
})
291294

295+
/**
296+
* `getHighestPrioritySubscription` resolves an organization subscription from
297+
* any `member` row regardless of role, so dropping `workspaceId` must not
298+
* hand a plain member the organization-wide pool the workspace branch
299+
* withholds.
300+
*/
301+
it('withholds the organization pool from an account caller who cannot manage it', async () => {
302+
mocks.getSubscription.mockResolvedValue({ plan: 'team', referenceId: 'organization-1' })
303+
mocks.deriveBillingContext.mockReturnValue({
304+
billingEntity: { type: 'organization', id: 'organization-1' },
305+
billingPeriod: {
306+
start: new Date('2026-01-01T00:00:00Z'),
307+
end: new Date('2026-02-01T00:00:00Z'),
308+
},
309+
})
310+
mocks.canUserManageBillingEntity.mockResolvedValue(false)
311+
mocks.checkBillingBlocked.mockResolvedValue({ blocked: false })
312+
mocks.checkBillingEntityBlocked.mockResolvedValue({ blocked: false })
313+
314+
const result = await getBillingStatus.execute({ principal: personalPrincipal, input: {} })
315+
316+
expect(result.credits).toBeNull()
317+
expect(result.storage).toBeNull()
318+
expect(result).toMatchObject({ workspaceId: null, plan: 'team', status: 'active' })
319+
expect(mocks.canUserManageBillingEntity).toHaveBeenCalledWith(
320+
{ type: 'organization', id: 'organization-1' },
321+
'user-1'
322+
)
323+
expect(mocks.getUserStorageUsage).not.toHaveBeenCalled()
324+
expect(mocks.getUserStorageLimit).not.toHaveBeenCalled()
325+
})
326+
327+
it('still reports an exceeded organization limit to an account caller who cannot read it', async () => {
328+
mocks.getSubscription.mockResolvedValue({ plan: 'team', referenceId: 'organization-1' })
329+
mocks.deriveBillingContext.mockReturnValue({
330+
billingEntity: { type: 'organization', id: 'organization-1' },
331+
billingPeriod: {
332+
start: new Date('2026-01-01T00:00:00Z'),
333+
end: new Date('2026-02-01T00:00:00Z'),
334+
},
335+
})
336+
mocks.canUserManageBillingEntity.mockResolvedValue(false)
337+
mocks.checkBillingBlocked.mockResolvedValue({ blocked: false })
338+
mocks.checkBillingEntityBlocked.mockResolvedValue({ blocked: false })
339+
mocks.checkUsageStatus.mockResolvedValue({ currentUsage: 40, limit: 10, isExceeded: true })
340+
341+
const result = await getBillingStatus.execute({ principal: personalPrincipal, input: {} })
342+
343+
expect(result.status).toBe('limit_exceeded')
344+
expect(result.credits).toBeNull()
345+
})
346+
347+
it('projects the organization pool to an account caller who administers it', async () => {
348+
mocks.getSubscription.mockResolvedValue({ plan: 'team', referenceId: 'organization-1' })
349+
mocks.deriveBillingContext.mockReturnValue({
350+
billingEntity: { type: 'organization', id: 'organization-1' },
351+
billingPeriod: {
352+
start: new Date('2026-01-01T00:00:00Z'),
353+
end: new Date('2026-02-01T00:00:00Z'),
354+
},
355+
})
356+
mocks.canUserManageBillingEntity.mockResolvedValue(true)
357+
mocks.checkBillingBlocked.mockResolvedValue({ blocked: false })
358+
mocks.checkBillingEntityBlocked.mockResolvedValue({ blocked: false })
359+
360+
const result = await getBillingStatus.execute({ principal: personalPrincipal, input: {} })
361+
362+
expect(result.credits).toEqual({ used: 200, limit: 2_000, remaining: 1_800 })
363+
expect(result.storage).toEqual({
364+
usedBytes: 5_242_880,
365+
limitBytes: 1_073_741_824,
366+
percentUsed: 0.48828125,
367+
})
368+
})
369+
292370
it('uses the personal principal as account authority', async () => {
293371
mocks.getSubscription.mockResolvedValue({ plan: 'pro' })
294372
mocks.deriveBillingContext.mockReturnValue({

0 commit comments

Comments
 (0)