Skip to content

Commit 2f8d064

Browse files
committed
test(permission-groups): pin the governed subject end to end
Four seams, each mutation-verified against the fallback it replaces: `insertDispatch` and `runWorkflowColumn` store and forward the subject verbatim (an explicit null survives a non-null attribution); the enrichment cell gates on the payload subject, so a workspace-key dispatch runs ungated and a pre-0315-shaped payload — subject null, attribution intact — stays ungated rather than reconstructing a gate from the payer; and `deleteUserAccount` cancels the account's non-terminal dispatches ahead of the user delete. The 0315 backfill itself is SQL and is covered by review, not by a test.
1 parent eacf218 commit 2f8d064

6 files changed

Lines changed: 411 additions & 0 deletions
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { resetDbChainMock } from '@sim/testing'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const mocks = vi.hoisted(() => ({
8+
getTableById: vi.fn(),
9+
getRowById: vi.fn(),
10+
updateRow: vi.fn(),
11+
pickNextEligibleGroupForRow: vi.fn(),
12+
stashCellContextForResume: vi.fn(),
13+
writeWorkflowGroupState: vi.fn(async () => 'wrote'),
14+
markWorkflowGroupPickedUp: vi.fn(async () => 'wrote'),
15+
createWorkflowCellProgressWriter: vi.fn(),
16+
buildCancelledExecution: vi.fn(),
17+
classifyWorkflowCellTerminalResult: vi.fn(),
18+
getEnrichment: vi.fn(),
19+
runEnrichment: vi.fn(),
20+
skippedEnrichmentDetail: vi.fn(() => ({})),
21+
checkAttributedUsageLimits: vi.fn(async () => ({ isExceeded: false })),
22+
loadTableRowSecretProvenance: vi.fn(async () => ({ scope: null, entries: [] })),
23+
}))
24+
25+
vi.mock('@/lib/table/service', () => ({ getTableById: mocks.getTableById }))
26+
vi.mock('@/lib/table/rows/service', () => ({
27+
getRowById: mocks.getRowById,
28+
updateRow: mocks.updateRow,
29+
}))
30+
vi.mock('@/lib/table/cell-write', () => ({
31+
writeWorkflowGroupState: mocks.writeWorkflowGroupState,
32+
markWorkflowGroupPickedUp: mocks.markWorkflowGroupPickedUp,
33+
createWorkflowCellProgressWriter: mocks.createWorkflowCellProgressWriter,
34+
buildCancelledExecution: mocks.buildCancelledExecution,
35+
}))
36+
vi.mock('@/lib/table/workflow-cell-result', () => ({
37+
classifyWorkflowCellTerminalResult: mocks.classifyWorkflowCellTerminalResult,
38+
}))
39+
vi.mock('@/enrichments/registry', () => ({ getEnrichment: mocks.getEnrichment }))
40+
vi.mock('@/enrichments/run', () => ({
41+
runEnrichment: mocks.runEnrichment,
42+
skippedEnrichmentDetail: mocks.skippedEnrichmentDetail,
43+
}))
44+
vi.mock('@/lib/billing/core/billing-attribution', () => ({
45+
assertBillingAttributionSnapshot: vi.fn((value) => value),
46+
checkAttributedUsageLimits: mocks.checkAttributedUsageLimits,
47+
toBillingContext: vi.fn(() => ({})),
48+
}))
49+
vi.mock('@/lib/table/rows/secret-provenance', () => ({
50+
createExactEmptyTableRowSecretProvenance: vi.fn(() => undefined),
51+
createTableRowSecretProvenanceFromRegistry: vi.fn(() => undefined),
52+
loadTableRowSecretProvenance: mocks.loadTableRowSecretProvenance,
53+
}))
54+
vi.mock('@/executor/utils/resolved-secret-trace-registry', () => ({
55+
ResolvedSecretTraceRegistry: class {
56+
async importCrossingProvenance() {}
57+
},
58+
}))
59+
vi.mock('@/lib/table/events', () => ({ appendTableEvent: vi.fn() }))
60+
61+
import { runRowCascadeLoop } from '@/background/workflow-column-execution'
62+
63+
const GROUP = {
64+
id: 'group-1',
65+
type: 'enrichment' as const,
66+
enrichmentId: 'company-lookup',
67+
workflowId: '',
68+
outputs: [{ columnName: 'col-out', blockId: '', path: '' }],
69+
inputMappings: [{ columnName: 'col-in', inputName: 'domain' }],
70+
}
71+
72+
const TABLE = {
73+
id: 'table-1',
74+
workspaceId: 'workspace-1',
75+
schema: { columns: [{ id: 'col-in', name: 'Domain', type: 'string' }], workflowGroups: [GROUP] },
76+
}
77+
78+
function payload(capabilityGovernedUserId: string | null, triggeredByUserId?: string) {
79+
return {
80+
tableId: 'table-1',
81+
tableName: 'Table',
82+
rowId: 'row-1',
83+
groupId: 'group-1',
84+
workflowId: '',
85+
workspaceId: 'workspace-1',
86+
executionId: 'exec-1',
87+
capabilityGovernedUserId,
88+
...(triggeredByUserId ? { triggeredByUserId } : {}),
89+
billingAttribution: {
90+
/** The meter's subject: the payer a workspace-key run attributes to. */
91+
actorUserId: triggeredByUserId ?? 'billing-owner',
92+
workspaceId: 'workspace-1',
93+
organizationId: null,
94+
billedAccountUserId: 'billing-owner',
95+
billingEntity: { type: 'user' as const, id: 'billing-owner' },
96+
billingPeriod: { start: '2026-07-01T00:00:00.000Z', end: '2026-08-01T00:00:00.000Z' },
97+
payerSubscription: null,
98+
},
99+
}
100+
}
101+
102+
/** The `userId` the cell handed the enrichment run — the per-tool gate subject. */
103+
function gatedUserId(): unknown {
104+
expect(mocks.runEnrichment).toHaveBeenCalledTimes(1)
105+
return (mocks.runEnrichment.mock.calls[0][2] as { userId?: unknown }).userId
106+
}
107+
108+
describe('enrichment cell capability subject', () => {
109+
beforeEach(() => {
110+
vi.clearAllMocks()
111+
resetDbChainMock()
112+
mocks.getTableById.mockResolvedValue(TABLE)
113+
mocks.getRowById.mockResolvedValue({
114+
id: 'row-1',
115+
data: { 'col-in': 'example.com' },
116+
executions: {},
117+
updatedAt: new Date('2026-08-01T00:00:00.000Z'),
118+
})
119+
mocks.checkAttributedUsageLimits.mockResolvedValue({ isExceeded: false })
120+
mocks.markWorkflowGroupPickedUp.mockResolvedValue('wrote')
121+
mocks.writeWorkflowGroupState.mockResolvedValue('wrote')
122+
mocks.pickNextEligibleGroupForRow.mockResolvedValue(null)
123+
mocks.getEnrichment.mockReturnValue({
124+
id: 'company-lookup',
125+
inputs: [{ id: 'domain', required: true }],
126+
providers: [],
127+
})
128+
mocks.runEnrichment.mockResolvedValue({ result: {}, cost: 0, detail: {} })
129+
})
130+
131+
/**
132+
* A workspace-key write is actorless: nobody's permission group governs it,
133+
* and the billing owner beside it on the payload is a bystander. Handing that
134+
* bystander to the enrichment would run their tool denylist against a request
135+
* they never made.
136+
*/
137+
it('runs a workspace-key dispatch ungated even though the payload names a payer', async () => {
138+
await runRowCascadeLoop(payload(null, 'billing-owner') as never)
139+
expect(gatedUserId()).toBeNull()
140+
})
141+
142+
it('governs a session-triggered dispatch by the acting person', async () => {
143+
await runRowCascadeLoop(payload('acting-user', 'acting-user') as never)
144+
expect(gatedUserId()).toBe('acting-user')
145+
})
146+
147+
/**
148+
* The shape a pre-0315 dispatch row has after the column is added: no governed
149+
* subject, attribution intact. New code reads that as actorless, which is why
150+
* the migration backfills the legacy subject onto non-terminal old rows rather
151+
* than letting the reader reconstruct it here.
152+
*/
153+
it('does not fall back to the attribution when the governed subject is absent', async () => {
154+
await runRowCascadeLoop(payload(null, 'legacy-trigger-user') as never)
155+
expect(gatedUserId()).toBeNull()
156+
})
157+
})

apps/sim/background/workflow-column-execution.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,8 @@ describe('table workflow usage-limit clear', () => {
283283
data: {},
284284
workspaceId: 'workspace-1',
285285
executionsPatch: { 'group-1': null },
286+
/** Clearing a pre-stamp writes no values, so no acting person governs it. */
287+
capabilityGovernedUserId: null,
286288
cancellationGuard: { groupId: 'group-1', executionId: 'execution-1' },
287289
})
288290
})

apps/sim/lib/table/cell-write.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,8 @@ describe('writeWorkflowGroupState', () => {
157157
workspaceId: TABLE.workspaceId,
158158
executionsPatch: { [GROUP.id]: RUNNING_STATE },
159159
cancellationGuard: { groupId: GROUP.id, executionId: CONTEXT.executionId },
160+
/** A cell result carries no acting person down to the write layer. */
161+
capabilityGovernedUserId: null,
160162
secretProvenance,
161163
},
162164
TABLE,
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { dbChainMockFns, resetDbChainMock } from '@sim/testing'
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
vi.mock('@/lib/table/events', () => ({
8+
appendTableEvent: vi.fn(),
9+
}))
10+
vi.mock('@/lib/table/service', () => ({
11+
getTableById: vi.fn(),
12+
}))
13+
14+
import { insertDispatch } from '@/lib/table/dispatcher'
15+
16+
const BASE = {
17+
tableId: 'table-1',
18+
workspaceId: 'workspace-1',
19+
requestId: 'req-1',
20+
mode: 'all' as const,
21+
scope: { groupIds: ['group-1'] },
22+
isManualRun: true,
23+
}
24+
25+
/** The values `insertDispatch` handed to the single `db.insert(...).values(...)`. */
26+
function insertedRow(): Record<string, unknown> {
27+
expect(dbChainMockFns.values).toHaveBeenCalledTimes(1)
28+
return dbChainMockFns.values.mock.calls[0][0] as Record<string, unknown>
29+
}
30+
31+
describe('insertDispatch governed subject', () => {
32+
beforeEach(() => {
33+
vi.clearAllMocks()
34+
resetDbChainMock()
35+
})
36+
37+
/**
38+
* The bug this replaces: an optional field defaulting to `triggeredByUserId`
39+
* meant a workspace-key auto-dispatch stored the workspace billed account as
40+
* its gate subject — a bystander whose tool denylist would then run against
41+
* a request nobody meant to govern.
42+
*/
43+
it('stores null for an actorless run even when the attribution names a user', async () => {
44+
await insertDispatch({
45+
...BASE,
46+
triggeredByUserId: 'billing-owner',
47+
capabilityGovernedUserId: null,
48+
})
49+
const row = insertedRow()
50+
expect(row.triggeredByUserId).toBe('billing-owner')
51+
expect(row.capabilityGovernedUserId).toBeNull()
52+
})
53+
54+
it('stores the acting person for a session-triggered run', async () => {
55+
await insertDispatch({
56+
...BASE,
57+
triggeredByUserId: 'user-1',
58+
capabilityGovernedUserId: 'user-1',
59+
})
60+
const row = insertedRow()
61+
expect(row.capabilityGovernedUserId).toBe('user-1')
62+
})
63+
64+
/**
65+
* The two fields are independent: a delegated run can be metered to the payer
66+
* while staying governed by the person who asked for it.
67+
*/
68+
it('keeps the gate subject independent of the meter subject', async () => {
69+
await insertDispatch({
70+
...BASE,
71+
triggeredByUserId: 'billing-owner',
72+
capabilityGovernedUserId: 'requesting-user',
73+
})
74+
const row = insertedRow()
75+
expect(row.triggeredByUserId).toBe('billing-owner')
76+
expect(row.capabilityGovernedUserId).toBe('requesting-user')
77+
})
78+
79+
/**
80+
* A row written before the column existed reads `capability_governed_user_id`
81+
* as NULL with `triggered_by_user_id` still set. Under the new semantics that
82+
* shape means "actorless, ungated" — which is why the 0315 migration
83+
* backfills the legacy subject onto non-terminal pre-migration rows rather
84+
* than letting them fall through to it.
85+
*/
86+
it('never reconstructs the gate subject from the attribution', async () => {
87+
await insertDispatch({
88+
...BASE,
89+
triggeredByUserId: 'user-1',
90+
capabilityGovernedUserId: null,
91+
})
92+
expect(insertedRow().capabilityGovernedUserId).toBeNull()
93+
})
94+
})
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const mocks = vi.hoisted(() => ({
7+
getTableById: vi.fn(),
8+
insertDispatch: vi.fn(async () => 'tdsp_1'),
9+
readDispatch: vi.fn(async () => null),
10+
cancelDispatchById: vi.fn(),
11+
bulkClearWorkflowGroupCells: vi.fn(async () => false),
12+
runDispatcherToCompletion: vi.fn(),
13+
resolveTableDispatchConcurrency: vi.fn(async () => 5),
14+
}))
15+
16+
vi.mock('@/lib/table/service', () => ({ getTableById: mocks.getTableById }))
17+
vi.mock('@/lib/table/dispatcher', () => ({
18+
bulkClearWorkflowGroupCells: mocks.bulkClearWorkflowGroupCells,
19+
cancelDispatchById: mocks.cancelDispatchById,
20+
insertDispatch: mocks.insertDispatch,
21+
readDispatch: mocks.readDispatch,
22+
runDispatcherToCompletion: mocks.runDispatcherToCompletion,
23+
}))
24+
vi.mock('@/lib/table/dispatch-concurrency', () => ({
25+
resolveTableDispatchConcurrency: mocks.resolveTableDispatchConcurrency,
26+
}))
27+
28+
import { runWorkflowColumn } from '@/lib/table/workflow-columns'
29+
30+
const TABLE = {
31+
id: 'table-1',
32+
workspaceId: 'workspace-1',
33+
schema: { columns: [], workflowGroups: [{ id: 'group-1', outputs: [] }] },
34+
}
35+
36+
const BASE = {
37+
tableId: 'table-1',
38+
workspaceId: 'workspace-1',
39+
groupIds: ['group-1'],
40+
mode: 'new' as const,
41+
isManualRun: false,
42+
requestId: 'req-1',
43+
}
44+
45+
/** The dispatch row `runWorkflowColumn` asked the dispatcher to insert. */
46+
function inserted(): Record<string, unknown> {
47+
expect(mocks.insertDispatch).toHaveBeenCalledTimes(1)
48+
return mocks.insertDispatch.mock.calls[0][0] as Record<string, unknown>
49+
}
50+
51+
describe('runWorkflowColumn governed subject', () => {
52+
beforeEach(() => {
53+
vi.clearAllMocks()
54+
mocks.getTableById.mockResolvedValue(TABLE)
55+
mocks.insertDispatch.mockResolvedValue('tdsp_1')
56+
mocks.readDispatch.mockResolvedValue(null)
57+
mocks.bulkClearWorkflowGroupCells.mockResolvedValue(false)
58+
mocks.resolveTableDispatchConcurrency.mockResolvedValue(5)
59+
})
60+
61+
/**
62+
* The row-write auto-fire case: a workspace API key wrote the row, so the
63+
* attribution names the workspace billed account. Forwarding that as the gate
64+
* subject — which an optional field with a fallback did — puts a bystander's
65+
* tool denylist on a run nobody governs.
66+
*/
67+
it('forwards an explicit null past a non-null attribution', async () => {
68+
await runWorkflowColumn({
69+
...BASE,
70+
triggeredByUserId: 'billing-owner',
71+
capabilityGovernedUserId: null,
72+
})
73+
const row = inserted()
74+
expect(row.triggeredByUserId).toBe('billing-owner')
75+
expect(row.capabilityGovernedUserId).toBeNull()
76+
})
77+
78+
it('forwards the acting person for a session-initiated run', async () => {
79+
await runWorkflowColumn({
80+
...BASE,
81+
triggeredByUserId: 'user-1',
82+
capabilityGovernedUserId: 'user-1',
83+
})
84+
expect(inserted().capabilityGovernedUserId).toBe('user-1')
85+
})
86+
})

0 commit comments

Comments
 (0)