fix: close cross-tenant billing/tenant security holes + string-enum API contract#1271
Merged
Conversation
… + string-enum API contract Deep audit of the billing/subscription/tenant system — backend. Security (P0, cross-tenant): - Invoice read/PDF/mutation (issue/pay/void), AssignSubscription, Get/CaptureUsage now root-gate via MultitenancyConstants.Root.Id (operator acts cross-tenant; tenants pinned to self). GetInvoices list scopes non-root callers to their own tenant. - GenerateInvoices is now root-only (ForbiddenException for non-root). - RoleService.FilterRootPermissions stripped only a "Permissions.Root." prefix that matched NO real root permission, letting a non-root admin grant root permissions to a role (privilege escalation). Now filtered by the IsRoot permission catalog. Correctness: - Usage/overage invoice was skipped when a subscription invoice shared the month (GenerateInvoiceForPeriodAsync idempotency lacked Purpose==Usage). - Same-plan renewal now advances Subscription.EndUtc (Subscription.Extend) to track the renewed ValidUpto, fixing the dashboard term drift. - IdentityDbInitializer now checks the admin CreateAsync result (silent failure previously marked a tenant provisioned with no usable admin login). - Invoice.Void idempotent; invoice pageSize clamped <=100; AdjustTenantValidity blocks the root tenant. API contract: - Enums serialize as string names globally (JsonStringEnumConverter via ConfigureHttpJsonOptions); [Flags] AuditTag/BodyCapture stay numeric via NumericEnumConverter. Every bug has a failing-first test (BillingTenantIsolationTests, RolePrivilegeEscalationTests, RenewTenantTests, AdjustTenantValidityTests, InvoiceTests). Integration suite: 699 passed, 0 failed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tions, expiry UX Frontend half of the billing/tenant audit. Enum contract: mirror the API's string enums as string unions across both apps (audits, chat, files, billing); AuditTag stays numeric (bitwise flags). Route-mock fixtures updated to emit string enum values. Admin: permission-gate every plan/invoice/tenant action affordance (previously shown to View-only users and 403-ing on submit); fix invoice-detail stuck "Loading..." on an error state. Dashboard: "Valid for"/validity now reads grace/expired from tenant status (was a healthy day-count off subscription.endUtc); persistent Expired banner; overview no longer masks an API error as a no-plan first-run; real server-side invoice pagination; removed dead Suspended/Cancelled rendering; subscription term now tracks start->end, not the calendar month. Playwright updated/added: admin specs green; dashboard 136 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced May 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Deep audit of the billing / subscription / tenant-management system across the backend and both React apps, plus the fixes. Every backend bug has a failing-first test.
Security (P0 — cross-tenant)
BillingDbContextextends a raw, non-tenant-filteredDbContext(for operator cross-tenant visibility), so every handler must scope explicitly. Several didn't:AssignSubscription/CaptureUsageSnapshotstenantIdGetUsageSnapshotsGenerateInvoicesRoleService.FilterRootPermissions"Permissions.Root."prefix that matched no real root permission, so a non-root admin could grant their role root permissionsIsRootpermission catalogAll gated on
MultitenancyConstants.Root.Id. Existing isolation tests missed this because they always authenticate with a matching tenant header — they never tested tenant-A's token acting on tenant-B's data.Verified non-issue: the
tenant-header-vs-JWT-claim concern is not exploitable — Finbuckle'sClaimStrategyis registered first and binds the resolved tenant to the JWT claim for non-root callers (proven by the existingTenantAdmin_HeaderOverride_Should_BeIgnored). No "fix" added.Correctness
GenerateInvoiceForPeriodAsyncidempotency lackedPurpose==Usage) — lost revenue.ValidUptobut notSubscription.EndUtc— fixed viaSubscription.Extend(the dashboard term drift).IdentityDbInitializerignored the adminCreateAsyncresult — a silent failure marked a tenant "provisioned" with no usable admin login.Invoice.Voidis now idempotent; invoicepageSizeclamped ≤100;AdjustTenantValidityblocks the root tenant.API contract
JsonStringEnumConverterviaConfigureHttpJsonOptions).[Flags]AuditTag/BodyCapturestay numeric via aNumericEnumConverter. Both frontends mirror the contract as string unions (AuditTagstays numeric/bitwise).Frontend
Tests
BillingTenantIsolationTests(5 cross-tenant mutation/usage/generate cases + usage-skip),RolePrivilegeEscalationTests,RenewTenant_Should_Advance_SubscriptionEndUtc,AdjustValidity_Should_Return400_When_TargetIsRootTenant,Void_Should_Be_Idempotent.Deferred (documented — features/edge cases, not bugs)
ConnectionStringin the tenant-list DTO (operator-only; contract change), WebSocket teardown on expiry (protected realtime infra, edge case), manual-issue notification (tenant-context nuance). Out of scope: payment gateway, proration, dunning, refunds, tenant suspend/delete, self-serve upgrade.🤖 Generated with Claude Code