Repository navigation
feat: subscription billing correctness (#1684) - #1882
Merged
github-actions[bot] merged 1 commit intoOct 22, 2026
Merged
github-actions[bot] merged 1 commit into
github-actions[bot] merged 1 commit into
Conversation
Re-lands the part of the reverted Smartdevs17#1703 squash that belongs to issue 1684 only. The remainder of the squash is re-landed by the other branches of the split, so this diff contains this issue's work exclusively.
github-actions Bot
pushed a commit
that referenced
this pull request
Oct 22, 2026
## Context This is the third of four pull requests that re-land the contents of the squash-merged PR #1703 one issue at a time. #1703 was merged as a single commit (`5afe754`) covering four unrelated issues. The companion pull request (the revert) undoes that squash so each issue can be merged on its own. This pull request restores **only** the part of #1703 that belongs to: - #1686 — platform security ## What this changes ### Rate-limit tier cannot be spoofed (`backend/server.ts`, `backend/services/shared/rateLimitingService.ts`) The caller's tier was read from the client-supplied `x-subscription-tier` header verbatim, so any caller presenting a key could send `X-Subscription-Tier: enterprise` and receive the 10,000 req/hr allowance instead of the FREE tier's 100 req/hr — a 100× rate-limit bypass requiring no privilege at all. The tier now comes from the server-side record for that key (`rateLimitingService.getTierForApiKey`). The header is honoured only when the explicit `TRUST_TIER_HEADER=true` opt-in is configured, which suits deployments where an authenticating gateway sits in front of this process and strips the header from inbound requests. ### Outbound URL guard (`backend/shared/security/outboundUrlGuard.ts`, new) `validateOutboundUrl()` rejects URLs that resolve to private, loopback, link-local or otherwise non-routable addresses, including DNS names that resolve to them. `isPrivateHostname()` and `assertSafeIdentifier()` are the building blocks. Used by: - `backend/integrations/zapier/ZapierIntegrationService.ts` and `zapierRouter.ts` — the Zapier integration fetches a user-supplied URL, which was a server-side request forgery vector. - `backend/services/webhook/eventReplayWorker.ts` — the replay worker re-delivers to a stored webhook target. ### Cursor pagination (`backend/services/shared/pagination.ts`, new) `encodeCursor` / `decodeCursor` sign the cursor payload with a secret, so a client cannot craft one; `buildCursorClause` and `buildPage` provide keyset pagination, and `parseFieldSelection` / `selectFields` let a caller restrict the returned columns. ### API key rotation (`backend/services/auth/domain/ApiKeyRotationService.ts`) Key rotation records the previous key's state so it can be revoked and audited. ### Webhook body limits and raw payload handling (`backend/server/createApiServer.ts`) The two provider webhook registrations now cap the raw body size (`limit: MAX_JSON_BODY`), so a webhook POST cannot be used to exhaust memory. The registrations remain body-parser-only — no handler, no state change — and the HMAC verification still happens inside the Stripe and Paddle routers, which fail closed with 503 when unconfigured and 401 on a bad signature. ### Revalidation allow-list (`developer-portal/pages/api/revalidate.ts`) `path` is caller-supplied and goes straight into `res.revalidate()`, so an arbitrary value let a holder of the secret force revalidation of unrelated routes. Only the documented set of paths is accepted now, and the secret is compared in constant time. ### Gateway (`backend/gateway/index.ts`) and configuration (`.env.example`) The new toggles (`TRUST_TIER_HEADER`, the pagination cursor secret, the webhook body limit) are documented in `.env.example`. ## Merge order This is the fourth of five pull requests in the split of #1703. It is based on `main` **after** the revert pull request ([#1880](#1880)) and the #1683 ([#1881](#1881)) and #1684 ([#1882](#1882)) pull requests were merged, so its diff contains the #1686 work and nothing else. The last pull request (#1685) is opened on top of this one and must be merged after it. The issues referenced above (#1683–#1686) have been deleted from the tracker, so they are referenced as plain text rather than with closing keywords.
github-actions Bot
pushed a commit
that referenced
this pull request
Oct 22, 2026
## Context This is the last of four pull requests that re-land the contents of the squash-merged PR #1703 one issue at a time. #1703 was merged as a single commit (`5afe754`) covering four unrelated issues. The companion pull request (the revert) undoes that squash so each issue can be merged on its own. This pull request restores **only** the part of #1703 that belongs to: - #1685 — automated test coverage ## What this changes Seven new test suites that cover the behaviour introduced by the other three issues: | Suite | Covers | | --- | --- | | `src/utils/__tests__/decimalAmount.test.ts` | decimal-safe normalisation and comparison, including values exactly at a limit | | `src/utils/__tests__/billingCycleMath.test.ts` | `monthlyEquivalent` / `yearlyEquivalent` for weekly, monthly and yearly cycles | | `src/utils/__tests__/invoiceTotals.test.ts` | `calculateInvoiceTotals`: rounding, an itemised discount subtracted exactly once, and the discount clamp | | `src/services/__tests__/transactionConfirmation.test.ts` | confirmation thresholds, timeouts, reverts and the definitive-failure classification | | `src/services/__tests__/rpcProvider.test.ts` | circuit-breaker transitions, the half-open success threshold and endpoint failover | | `src/store/_tests_/transactionQueueStore.test.ts` | the broadcast marker, exponential backoff and the refusal to re-broadcast an unknown-outcome transaction | | `backend/shared/security/__tests__/outboundUrlGuard.test.ts` | rejection of private, loopback and link-local outbound targets | ## Merge order This is the fifth and last pull request in the split of #1703. It is based on `main` **after** the revert pull request ([#1880](#1880)) and the #1683 ([#1881](#1881)), #1684 ([#1882](#1882)) and #1686 ([#1883](#1883)) pull requests were merged, so its diff contains the #1685 work and nothing else. The suites exercise the modules added by the #1683, #1684 and #1686 pull requests, so this pull request has to come last. Merging it completes the split. After all five pull requests are merged, `main` is identical to its state before the split. The issues referenced above (#1683–#1686) have been deleted from the tracker, so they are referenced as plain text rather than with closing keywords.
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.
Context
This is the second of four pull requests that re-land the contents of the squash-merged PR #1703 one issue at a time.
#1703 was merged as a single commit (
5afe754) covering four unrelated issues. The companion pull request (the revert) undoes that squash so each issue can be merged on its own. This pull request restores only the part of #1703 that belongs to:What this changes
One place that computes invoice totals (
src/utils/invoiceTotals.ts)createInvoiceandupdateInvoiceeach rolled their own arithmetic and the two did not agree —createInvoicesubtracted the discount,updateInvoicedid not. Editing one line on a discounted invoice therefore silently increased the amount payable, and because the discount was never persisted on the record there was nothing left to subtract on the second pass.calculateInvoiceTotals()is now the single source of truth:kind: 'discount') are kept out of the subtotal and summed intodiscountinstead, so an itemised discount is subtracted exactly once;Invoice service (
src/services/invoiceService.ts)Both the create and the update path go through the shared totals function, and the discount is persisted on the record so a second pass subtracts the same amount.
Billing cycle maths (
src/utils/billingCycleMath.ts, new)monthlyEquivalent,yearlyEquivalent,isAnnualCycleandnormalizeBillingCyclereplace the ad-hoc cycle conversions that were scattered across the store and the analytics service, which disagreed about what a weekly or yearly cycle was worth.Revenue recognition (
backend/billing/jobs/monthlyRevenueRecognitionJob.ts,backend/billing/jobs/index.ts)The job now tracks cumulative per-entry recognition progress (
recordEntryProgress,recognisedToDate). Previously each run re-recognised the whole elapsed portion of an entry, so recognised revenue was inflated on every run and the deferred balance grew without bound.Metering and group billing (
backend/services/billing/meteringService.ts,groupBilling.ts,interfaces.ts)aggregateCharges(group, from, to)takes an explicit window.Analytics (
src/services/analyticsService.ts,src/store/slices/analyticsSlice.ts,src/store/subscriptionStore.ts,backend/services/analytics/subscriptionAnalyticsService.ts)Monthly and yearly equivalents now come from the shared billing-cycle helpers, so the dashboard, the store and the backend report the same numbers.
Batch charges (
backend/services/batchChargeService.ts)Uses the shared rounding helper.
Test (
src/services/__tests__/groupBilling.test.ts)Updated for the explicit-window
aggregateChargessignature.Merge order
This is the third of five pull requests in the split of #1703. It is based on
mainafter the revert pull request (#1880) and the #1683 pull request (#1881) were merged, so its diff contains the #1684 work and nothing else.The remaining two pull requests (#1686, #1685) are opened on top of this one, in that order, and must be merged after it.
The issues referenced above (#1683–#1686) have been deleted from the tracker, so they are referenced as plain text rather than with closing keywords.