Skip to content

Commit 32f4c5c

Browse files
committed
fix(billing): deny the payer pool to actor-less workspace API keys
The first pass gated `credits` and `storage` on billing authority for personal API keys but let a `workspace_api_key` principal through unconditionally, which left the excluded role a way back in. Any workspace `admin` may mint a workspace API key, and a workspace `admin` is deliberately not a billing manager, so an admin who reads `null` as themselves could mint a key and read the full pool with it. On an organization-hosted workspace that pool is the organization's, spanning workspaces the admin has no standing in. Billing authority is payer identity or an organization admin role — a property of a person. A workspace API key is deliberately actor-less, so it can never satisfy it and now reads both fields as `null`. Attributing the key to its creator was rejected: it would launder the same workspace-admin role, it breaks when the creator's authority is revoked while the key lives on, and substituting a key's owner for the acting principal is what the application operation boundary forbids. The reasoning sits in TSDoc at the decision point. The key keeps the plan, period, and standing it needs to monitor a workspace, including `limit_exceeded` and `billing_blocked`. No in-repo caller reads `credits` or `storage` from this endpoint. The payer storage pool is now read only once disclosure is authorized, so a caller who may not see it no longer triggers the query at all.
1 parent 53c560c commit 32f4c5c

5 files changed

Lines changed: 104 additions & 38 deletions

File tree

apps/docs/openapi-v2-billing.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
"get": {
3737
"operationId": "getBillingStatus",
3838
"summary": "Get Billing Status",
39-
"description": "Return the current plan, billing standing, credit allowance, and storage quota. `credits` and `storage` report the payer's pooled allowances and are null unless the caller can manage that payer's billing. Billing history lives at `GET /api/v2/billing/logs`. Without a Stripe subscription — notably on the free plan — there is no real billing period: `period` is the open interval 1970-01-01 to 9999-12-31 and `credits.used` is lifetime consumption, not consumption since a period start.",
39+
"description": "Return the current plan, billing standing, credit allowance, and storage quota. `credits` and `storage` report the payer's pooled allowances and are null unless the caller can manage that payer's billing; they are always null for a workspace API key. Billing history lives at `GET /api/v2/billing/logs`. Without a Stripe subscription — notably on the free plan — there is no real billing period: `period` is the open interval 1970-01-01 to 9999-12-31 and `credits.used` is lifetime consumption, not consumption since a period start.",
4040
"tags": ["Billing"],
4141
"parameters": [
4242
{
@@ -567,7 +567,7 @@
567567
"type": "null"
568568
}
569569
],
570-
"description": "The payer's credit usage and allowance — periodic on a paid plan, lifetime on the free plan, where the counter never resets. Null when the caller cannot manage that payer's billing."
570+
"description": "The payer's credit usage and allowance — periodic on a paid plan, lifetime on the free plan, where the counter never resets. Null when the caller cannot manage that payer's billing. Always null for a workspace API key."
571571
},
572572
"storage": {
573573
"anyOf": [
@@ -597,7 +597,7 @@
597597
"type": "null"
598598
}
599599
],
600-
"description": "The payer's storage consumption and quota, or null when the caller cannot manage that payer's billing."
600+
"description": "The payer's storage consumption and quota, or null when the caller cannot manage that payer's billing. Always null for a workspace API key."
601601
}
602602
},
603603
"required": ["workspaceId", "period", "plan", "status", "credits", "storage"],

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,12 @@ export const v2BillingStatusQuerySchema = z.object({
3838
*
3939
* `credits` and `storage` report the resolved payer's pooled allowances, which
4040
* 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
42-
* holder, an admin of the hosting organization, or a workspace API key, which
43-
* an admin of that workspace provisioned. Any other workspace member reads
44-
* both as `null` while still seeing the plan, period, and standing that the
45-
* workspace already surfaces to them.
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`.
4647
*/
4748
export const v2BillingStatusDataSchema = z
4849
.object({
@@ -88,7 +89,7 @@ export const v2BillingStatusDataSchema = z
8889
})
8990
.nullable()
9091
.describe(
91-
"The payer's credit usage and allowance — periodic on a paid plan, lifetime on the free plan, where the counter never resets. Null when the caller cannot manage that payer's billing."
92+
"The payer's credit usage and allowance — periodic on a paid plan, lifetime on the free plan, where the counter never resets. Null when the caller cannot manage that payer's billing. Always null for a workspace API key."
9293
),
9394
storage: z
9495
.object({
@@ -98,7 +99,7 @@ export const v2BillingStatusDataSchema = z
9899
})
99100
.nullable()
100101
.describe(
101-
"The payer's storage consumption and quota, or null when the caller cannot manage that payer's billing."
102+
"The payer's storage consumption and quota, or null when the caller cannot manage that payer's billing. Always null for a workspace API key."
102103
),
103104
})
104105
.meta({

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ const routes = [
8080
operationId: 'getBillingStatus',
8181
summary: 'Get Billing Status',
8282
description:
83-
"Return the current plan, billing standing, credit allowance, and storage quota. `credits` and `storage` report the payer's pooled allowances and are null unless the caller can manage that payer's billing. Billing history lives at `GET /api/v2/billing/logs`. Without a Stripe subscription — notably on the free plan — there is no real billing period: `period` is the open interval 1970-01-01 to 9999-12-31 and `credits.used` is lifetime consumption, not consumption since a period start.",
83+
"Return the current plan, billing standing, credit allowance, and storage quota. `credits` and `storage` report the payer's pooled allowances and are null unless the caller can manage that payer's billing; they are always null for a workspace API key. Billing history lives at `GET /api/v2/billing/logs`. Without a Stripe subscription — notably on the free plan — there is no real billing period: `period` is the open interval 1970-01-01 to 9999-12-31 and `credits.used` is lifetime consumption, not consumption since a period start.",
8484
errors: [...WORKSPACE_ERRORS, 'NotFound'],
8585
success: { description: 'The current billing and storage status.' },
8686
}),

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

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -167,18 +167,53 @@ describe('billing application use cases', () => {
167167
})
168168

169169
expect(result.workspaceId).toBe('workspace-1')
170-
expect(result.storage).toEqual({
171-
usedBytes: 5_242_880,
172-
limitBytes: 1_073_741_824,
173-
percentUsed: 0.48828125,
174-
})
170+
expect(result).toMatchObject({ plan: 'free', status: 'active' })
175171
expect(mocks.resolveSystemAttribution).toHaveBeenCalledWith('workspace-1')
176-
expect(mocks.resolveStorageContext).toHaveBeenCalledWith('workspace-1')
177172
expect(mocks.resolvePermission).not.toHaveBeenCalled()
178173
expect(mocks.resolveAttribution).not.toHaveBeenCalled()
179174
expect(mocks.recordAudit).not.toHaveBeenCalled()
180175
})
181176

177+
/**
178+
* A workspace API key is actor-less, and any workspace `admin` may mint one,
179+
* so granting it the pool would launder the exact role the projection
180+
* excludes — across the whole organization on an organization-hosted
181+
* workspace.
182+
*/
183+
it('withholds the payer pool from an actor-less workspace key', async () => {
184+
const result = await getBillingStatus.execute({
185+
principal: workspacePrincipal,
186+
input: {},
187+
})
188+
189+
expect(result.credits).toBeNull()
190+
expect(result.storage).toBeNull()
191+
expect(mocks.canManageWorkspaceBilling).not.toHaveBeenCalled()
192+
})
193+
194+
it('never reads the payer storage pool it may not disclose', async () => {
195+
await getBillingStatus.execute({ principal: workspacePrincipal, input: {} })
196+
await getBillingStatus.execute({
197+
principal: personalPrincipal,
198+
input: { workspaceId: 'workspace-1' },
199+
})
200+
201+
expect(mocks.resolveStorageContext).not.toHaveBeenCalled()
202+
expect(mocks.getStorageUsageForContext).not.toHaveBeenCalled()
203+
})
204+
205+
it('still reports a workspace key an exceeded pooled limit it cannot read', async () => {
206+
mocks.checkAttributedBlocks.mockResolvedValue({ blocked: true })
207+
208+
const result = await getBillingStatus.execute({
209+
principal: workspacePrincipal,
210+
input: {},
211+
})
212+
213+
expect(result.status).toBe('billing_blocked')
214+
expect(result.credits).toBeNull()
215+
})
216+
182217
it('withholds the payer pool from a workspace member who cannot manage billing', async () => {
183218
mocks.resolvePermission.mockResolvedValue('read')
184219
mocks.canManageWorkspaceBilling.mockResolvedValue(false)
@@ -236,13 +271,6 @@ describe('billing application use cases', () => {
236271
})
237272
})
238273

239-
it('never consults human billing authority for a workspace key', async () => {
240-
const result = await getBillingStatus.execute({ principal: workspacePrincipal, input: {} })
241-
242-
expect(result.credits).not.toBeNull()
243-
expect(mocks.canManageWorkspaceBilling).not.toHaveBeenCalled()
244-
})
245-
246274
it('always reports the account-scoped pool the caller owns', async () => {
247275
mocks.canManageWorkspaceBilling.mockResolvedValue(false)
248276
mocks.getSubscription.mockResolvedValue({ plan: 'pro' })

apps/sim/lib/billing/application/get-billing-status.ts

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { defineAuthorizedBillingReadUseCase } from '@/lib/billing/application/authorized-billing-read-use-case'
2-
import { billingOperations } from '@/lib/billing/application/operations'
2+
import { type BillingReadPrincipal, billingOperations } from '@/lib/billing/application/operations'
33
import {
44
checkBillingBlocked,
55
checkBillingEntityBlocked,
@@ -13,7 +13,10 @@ import {
1313
} from '@/lib/billing/core/billing-attribution'
1414
import { getHighestPrioritySubscription } from '@/lib/billing/core/subscription'
1515
import { deriveBillingContext } from '@/lib/billing/core/usage-log'
16-
import { canUserManageWorkspaceBilling } from '@/lib/billing/core/workspace-billing-authority'
16+
import {
17+
canUserManageWorkspaceBilling,
18+
type WorkspaceBillingAuthorityContext,
19+
} from '@/lib/billing/core/workspace-billing-authority'
1720
import { dollarsToCredits } from '@/lib/billing/credits/conversion'
1821
import {
1922
getStorageLimitForBillingContext,
@@ -42,9 +45,9 @@ export interface BillingStorageStatus {
4245
/**
4346
* `credits` and `storage` describe the resolved payer's pooled allowances, not
4447
* the caller's own consumption, so they are only projected to a caller who may
45-
* manage that payer's billing. Every other workspace member reads them as
46-
* `null` while still seeing the plan and standing the workspace UI already
47-
* shows them.
48+
* manage that payer's billing — see {@link canReadPayerPool}. Every other
49+
* caller reads them as `null` while still seeing the plan and standing the
50+
* workspace UI already shows them.
4851
*/
4952
export interface BillingStatusResult {
5053
workspaceId: string | null
@@ -63,29 +66,63 @@ function storageStatus(usedBytes: number, limitBytes: number): BillingStorageSta
6366
}
6467
}
6568

69+
/**
70+
* Resolves whether a caller may read the resolved payer's pooled allowances.
71+
*
72+
* Only a human principal can hold billing authority: it is payer identity, or
73+
* an admin role in the hosting organization, and never a workspace role. A
74+
* workspace API key is deliberately actor-less — `WorkspaceApiKeyPrincipal`
75+
* carries a key and a workspace but no user — so there is no identity to
76+
* evaluate that authority against, and it is therefore never a billing
77+
* manager.
78+
*
79+
* Attributing the key to whoever created it would close the gap on paper and
80+
* open a worse one. Any workspace `admin` may mint a workspace key, and a
81+
* workspace `admin` is not a billing manager, so the key would launder exactly
82+
* the role this projection excludes. The pool is the payer's, not the
83+
* workspace's — organization-wide on an organization-hosted workspace — so it
84+
* spans workspaces that admin has no standing in at all. Substituting a key's
85+
* owner for the acting principal is also what the application operation
86+
* boundary forbids outright, and the creator's authority can be revoked while
87+
* the key keeps working.
88+
*
89+
* A workspace key still reads the plan, period, and standing it needs to
90+
* monitor the workspace, including `limit_exceeded` and `billing_blocked`.
91+
*/
92+
function canReadPayerPool(
93+
principal: BillingReadPrincipal,
94+
workspace: WorkspaceBillingAuthorityContext
95+
): Promise<boolean> {
96+
if (principal.kind !== 'personal_api_key') return Promise.resolve(false)
97+
return canUserManageWorkspaceBilling(workspace, principal.userId)
98+
}
99+
100+
/** Reads the payer's storage pool. Only called once disclosure is authorized. */
101+
async function resolvePayerStorage(workspaceId: string): Promise<BillingStorageStatus> {
102+
const storageContext = await resolveStorageBillingContext(workspaceId)
103+
const usedBytes = await getStorageUsageForBillingContext(storageContext)
104+
return storageStatus(usedBytes, getStorageLimitForBillingContext(storageContext))
105+
}
106+
66107
export const getBillingStatus = defineAuthorizedBillingReadUseCase({
67108
operation: billingOperations.readStatus,
68109
requestedWorkspaceId: (input: GetBillingStatusInput) => input.workspaceId,
69110
execute: async ({ principal, scope }): Promise<BillingStatusResult> => {
70111
if (scope.kind === 'workspace') {
71-
const [attribution, storageContext, canViewPayerPool] = await Promise.all([
112+
const [attribution, canViewPayerPool] = await Promise.all([
72113
principal.kind === 'personal_api_key'
73114
? resolveBillingAttribution({
74115
actorUserId: principal.userId,
75116
workspaceId: scope.workspace.workspaceId,
76117
})
77118
: resolveSystemBillingAttribution(scope.workspace.workspaceId),
78-
resolveStorageBillingContext(scope.workspace.workspaceId),
79-
principal.kind === 'personal_api_key'
80-
? canUserManageWorkspaceBilling(scope.workspace, principal.userId)
81-
: Promise.resolve(true),
119+
canReadPayerPool(principal, scope.workspace),
82120
])
83-
const [usage, block, storageUsedBytes] = await Promise.all([
121+
const [usage, block, storage] = await Promise.all([
84122
checkUsageStatus(attribution.billedAccountUserId, toUsageLimitSubscription(attribution)),
85123
checkAttributedBillingBlocks(attribution),
86-
getStorageUsageForBillingContext(storageContext),
124+
canViewPayerPool ? resolvePayerStorage(scope.workspace.workspaceId) : null,
87125
])
88-
const storageLimitBytes = getStorageLimitForBillingContext(storageContext)
89126
return {
90127
workspaceId: scope.workspace.workspaceId,
91128
period: attribution.billingPeriod,
@@ -98,7 +135,7 @@ export const getBillingStatus = defineAuthorizedBillingReadUseCase({
98135
remaining: dollarsToCredits(usage.limit - usage.currentUsage),
99136
}
100137
: null,
101-
storage: canViewPayerPool ? storageStatus(storageUsedBytes, storageLimitBytes) : null,
138+
storage,
102139
}
103140
}
104141

0 commit comments

Comments
 (0)