diff --git a/backend/__tests__/unit/services/billingService.test.js b/backend/__tests__/unit/services/billingService.test.js index 6331b9139f..1589a37b06 100644 --- a/backend/__tests__/unit/services/billingService.test.js +++ b/backend/__tests__/unit/services/billingService.test.js @@ -1,5 +1,6 @@ /** - * billingService — the only writer of `entitlements.pro`. + * billingService — the Stripe-derived writer of `entitlements.pro`. + * The admin entitlements PATCH is the other authorized writer. * * These tests exist because every bug here is either "we gave away the product" * or "we took money and withheld it". The four properties defended: diff --git a/backend/routes/admin/users.ts b/backend/routes/admin/users.ts index 4052b6bc6b..4eeae94012 100644 --- a/backend/routes/admin/users.ts +++ b/backend/routes/admin/users.ts @@ -174,7 +174,7 @@ router.patch('/:userId/role', auth, adminAuth, async (req: any, res: any) => { }); // PATCH /api/admin/users/:userId/entitlements — grant/revoke capability -// flags (today just cloudAgents, the hosted-agent gate; see User.entitlements). +// flags (cloudAgents for hosted agents and pro for paid-tier access; see User.entitlements). // This is the missing admin lever for the staged open-registration plan: // new signups default to BYO-only, and an admin flips cloudAgents here. router.patch('/:userId/entitlements', adminWriteLimiter, auth, adminAuth, async (req: any, res: any) => { diff --git a/backend/services/billingService.ts b/backend/services/billingService.ts index 6cc9ba1856..bae1975704 100644 --- a/backend/services/billingService.ts +++ b/backend/services/billingService.ts @@ -1,5 +1,6 @@ /** - * Stripe billing — the only writer of `entitlements.pro`. + * Stripe billing — handles Stripe-derived updates to `entitlements.pro`. + * The admin entitlements PATCH is the other authorized writer. * * Design rules, each of which exists because the obvious alternative is a way * to lose money or give it away: diff --git a/docs/development/agent-experience-audit.md b/docs/development/agent-experience-audit.md index 2eafbb20c3..22fbbfbf9d 100644 --- a/docs/development/agent-experience-audit.md +++ b/docs/development/agent-experience-audit.md @@ -4175,3 +4175,15 @@ Gates that never appear in metadata decided several more, including the client a **Repair:** a catalogue verdict now requires a second, independent pass that tries to refute it. That pass answers from vendor docs or measurement what the read token actually *reaches*: tools with side effects, proxies to third parties, token forwarding, plan or allowlist gates. The first pass's metadata is the starting point, not the verdict. **Lesson:** a scope's name is the vendor's label for a permission set, and the permission set is what the bar is about. Measure what the credential can reach, and refute your own first reading before a capture or an account is spent on it. + +## 81. "The ONLY writer of entitlements.pro" was false, and a security claim was built on it (2026-10-15, lily-shen) + +*Origin: TASK-268 (#2237), the plan line that gives Pro owners 200 hosted turns a day instead of 30.* + +`routes/billing.ts` documented the Stripe webhook as "the ONLY writer of entitlements.pro", and the User model said Pro is written only "from a signature-verified Stripe webhook". The admin route `PATCH /api/admin/users/:userId/entitlements` has accepted `pro` since before billing existed. Its own comment says that until billing exists, "this route IS the subscription". So there were two authorized writers, and the comments named one. + +I repeated the false claim in #2237's code comment and PR body, and in the review focus I sent with it: "can anything other than the signed webhook make an owner read as Pro?" Rhea answered by reading the handler, not the comment. The admin PATCH grants Pro, and the cap resolver honours it (HOLD 5536742280). Nothing was exploitable, because the admin path is authorized and meant to lift the line. But a security reviewer handed my framing would have checked only the webhook and signed off on a claim the code does not make. + +**Repair:** #2237 corrects the writer claims it touched; TASK-316 corrects the `billingService.ts` header, the billing service test header, and the admin route's capability list. Both the verified Stripe webhook and the admin entitlements PATCH are authorized writers; no ordinary client response sets Pro. + +**Lesson:** "the only writer of X" is a claim about every route, so it decays as soon as one more route writes X. Before repeating it, and especially before putting it in a review focus, search for the field and inspect assignment sites across routes and services. `git grep "entitlements.pro"` finds reads and comments but misses this admin write: it assigns `patch.pro = pro` and then spreads `patch` into `target.entitlements`. A comment's "only" is a hypothesis, not a fact.