fix(billing,payments,security): harden payment, billing and platform correctness

[akansnoro]

Closes #1683
Closes #1684
Closes #1685
Closes #1686

Summary

One focused hardening pass across the four Stellar Wave iteration-10 issues. Each
section below names the concrete defect fixed, because in every case the
original code looked correct at a glance and failed in a way that silently
produced wrong money or unsafe behaviour.

---

#1683 — Improve crypto payment resilience

Reverted transactions were reported as successful. Every on-chain path called
tx.wait() with no arguments and then only asserted that a hash came back. A
receipt with status === 0 means mined but reverted, so a failed payment was
surfaced to the user as confirmed.

New src/services/transactionConfirmation.ts centralises three concerns and is
now used by Superfluid creation, Sablier creation, Sablier approval and ERC-20
approval:

• requires chain-appropriate confirmation depth (1 on L2, 2 on L1);
• rejects receipt.status === 0;
• enforces its own timeout. This mattered: ethers v5's
  ContractTransaction.wait(confirmations) does not forward a timeout argument
  at all, so delegating the bound to it left every contract-based payment able
  to hang forever — and because processQueue is sequential, one stuck
  transaction stalled the whole offline retry queue behind it.

The RPC provider could send a request to the wrong endpoint. The base ethers
provider has one mutable connection.url, and the fallback logic swapped it per
call. getBalancesAcrossChains fans out in parallel, so concurrent calls
interleaved and requests could be routed to a node on a different chain. Access
is now serialised.

Healthy nodes were being evicted from the circuit. Every error counted as a
failure, including legitimate JSON-RPC errors such as reverts and insufficient
funds — a working node answering correctly was treated as unreachable.
isTransportFailure now separates transport problems from application-level
answers. Half-open circuits also required a single success to close; they now
require a threshold, matching the documented behaviour.

Fractional amounts could never pass the spend cap. The check was
ethers.BigNumber.from(amount).gt(limit), and BigNumber.from("12.5") throws.
The throw was swallowed by the surrounding catch, so every fractional amount
was recorded as a generic failure. Replaced with exact fixed-point comparison
(src/utils/decimalAmount.ts) — not parseFloat, which would reintroduce float
error.

Balances used hard-coded decimals. checkBalance assumed USDC ? 6 : 18,
under- or over-reporting any other ERC-20 and triggering spurious fallbacks.
Decimals are now read from the contract. All four raw JsonRpcProvider
constructions also bypassed the timeout/circuit-breaker/fallback layer entirely.

A crash between broadcast and receipt could charge the user twice. The queue
removed an entry only after execution, so an app crash mid-flight left the
item pending and the next pass re-broadcast it. A broadcastedAt marker is now
persisted before the payload reaches the wallet. Items whose outcome is
genuinely unknown are retired with a "needs review" notification rather than
re-sent, and the marker is only cleared for failures that prove nothing was
submitted (wallet rejection). Also added exponential backoff, staleness measured
from the last attempt rather than creation, reference-counted connectivity
listeners, and removal of unhandled rejections that can terminate the app.

Smart fallback ordering was inert. walletStore read attempt.success and
attempt.createdAt, neither of which exists on PaymentAttempt (it has
status and attemptedAt). Every field resolved to undefined, the monitor's
look-back window discarded all attempts, and every method scored a 100% success
rate. Also fixed verifyPaymentMethod leaving isLoading stuck when
verification returned false.

---

#1684 — Strengthen subscription billing correctness

Editing a line on a discounted invoice increased the amount payable.
createInvoice subtracted the discount but never persisted it, and
updateInvoice recomputed subtotal + tax with no discount at all. New
src/utils/invoiceTotals.ts is the single arithmetic path for both, clamps the
discount to the subtotal so a total can never go negative, and rounds each
component to minor units.

Average invoice amount was Infinity. totalRevenue / paidInvoices || 0
returns Infinity when no invoice is paid, because Infinity is truthy.

MRR was inflated 12× for annual plans. subscriptionStore compared raw
strings and omitted annual entirely, so a 120/year plan reported 120 MRR
instead of 10 — and this is the store that populates stats for the app.
analyticsService and analyticsSlice had the same omission plus a hard-coded
4.345 weeks/month while BILLING_CONVERSIONS uses 52/12. All four now use
src/utils/billingCycleMath.ts.

Revenue was re-recognised in full on every run. In
monthlyRevenueRecognitionJob, a 'partial' recognition was never marked
complete, so the next daily run re-recognised the entire elapsed portion,
compounding daily. The job now recognises only the increment since the last run,
tracking cumulative per-entry progress via a new optional repository method.
When that method is absent it degrades to recognising each entry once at full
amount — slower, but never over-counting. Deferred revenue is clamped at zero.

Group invoices priced the wrong window. generateInvoice derived periodDays
from the period but aggregateCharges computed its cutoff from Date.now(), so
a historical invoice priced a different set of charges than the period it claims.
aggregateCharges now takes an explicit (periodStart, periodEnd).
ownerDiscount was unclamped (a negative value increased the bill, >100 gave a
negative payable amount) and tax was hard-coded to 0.

Usage billing summed all history but labelled it one month.
calculateUsageBilling aggregated the unbounded history array while reporting
the current calendar month, over-billing every account with prior history each
cycle. Period end was also new Date(y, m+1, 0) — midnight starting the last
day, excluding the final day. Additionally, auto idempotency keys built purely
from the event timestamp collided for same-millisecond events, and the second was
silently dropped and never rated.

Also fixed a RunHistory field typo in batchChargeService that made every
batch charge throw on recordRun.

---

#1686 — Harden platform security

Forgeable pagination cursors → SQL identifier injection. pagination.ts fell
back to the literal 'subtrackr-cursor-secret', published in the repo. Cursors
carry a field that is interpolated into ORDER BY/WHERE, so a known secret
let anyone forge one. The secret is now required in production, signatures are
compared with timingSafeEqual, and any sort/field identifier outside
[A-Za-z0-9_] is rejected.

Forgeable webhook signatures. createApiServer fell back to
'default-dev-key-change-in-prod' for the key that signs every outbound
webhook. Removed; production startup now fails instead.

Weak webhook signature scheme. eventReplayWorker emitted
sha256=<31-bit rolling hash> — labelled SHA-256, not SHA-256. Replaced with a
real keyed HMAC.

API keys hashed with unsalted SHA-256. Now a keyed HMAC, so a leaked key
table cannot be attacked offline. Remains deterministic so lookups still work.

SSRF. Zapier hook targetUrl was stored verbatim and later fetched
server-side, reaching cloud metadata (169.254.169.254), loopback and RFC1918
hosts. New backend/shared/security/outboundUrlGuard.ts validates scheme,
embedded credentials, and private/loopback/link-local/CGNAT/malformed-IP-literal
hosts. Applied at registration and delivery, so already-stored targets are
covered too.

100× rate-limit bypass. The tier came from the client-supplied
x-subscription-tier header, so anyone could request enterprise and get
10,000 req/hr instead of 100. Now resolved from the server-side record, with an
explicit TRUST_TIER_HEADER=true opt-in for trusted-proxy deployments.

Unbounded request bodies. maxBytes was optional and 31 of 32 routes omitted
it, so any route could buffer the whole body into memory before parsing; the
HubSpot webhook accumulated chunks before its signature check. A 256 KiB default
now applies everywhere. The gateway's blanket 10mb JSON limit is reduced to
256kb.

Timing-unsafe secret comparison + unvalidated revalidate path. The
REVALIDATE_SECRET check short-circuited on the first differing byte, and
path went straight into res.revalidate(). Now constant-time and
allowlisted.

---

#1685 — Expand automated test coverage

New suites for the new modules (transactionConfirmation, decimalAmount,
invoiceTotals, billingCycleMath, outboundUrlGuard) plus extensions to the
transaction-queue and RPC-provider suites covering idempotency, backoff and
circuit-breaker behaviour. Each new test names the regression it locks in.

The existing groupBilling test was updated to the explicit period signature; its
previous assertion encoded the Date.now()-relative window bug.

---

Deployment notes

Three secrets are now required in production and the process fails to start
without them:

Variable	Purpose
CURSOR_HMAC_SECRET	Cursor HMAC key
WEBHOOK_SIGNING_KEY	Outbound webhook signing key
API_KEY_HASH_PEPPER	API key digest pepper

All need random values of at least 16 characters. Outside production an
ephemeral per-process value is generated with a warning — which means cursors
and hashed keys do not survive a restart in dev. Documented in .env.example.

Behaviour change: rate-limit tiers are no longer self-declared by clients.
A deployment behind an authenticating proxy that sets x-subscription-tier
should set TRUST_TIER_HEADER=true and ensure the proxy strips that header from
inbound traffic. Without a server-side tier record, callers resolve to FREE.

Verification

npm run lint, npx tsc --noEmit and npm run format:check were not run
locally — node_modules is not installed in this environment and installing
dependencies was out of scope for this task. The changed src/ files were
manually checked against the configured Prettier rules (printWidth 100,
trailingComma: es5, single quotes) and for unused imports, since
prettier/prettier is configured as an ESLint error.

[gitguardian]

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

Since your pull request originates from a forked repository, GitGuardian is not able to associate the secrets uncovered with secret incidents on your GitGuardian dashboard.
Skipping this check run and merging your pull request will create secret incidents on your GitGuardian dashboard.

🔎 Detected hardcoded secret in your pull request

GitGuardian id	GitGuardian status	Secret	Commit	Filename
38302542	Triggered	Zapier Webhook URL	3a87a76	backend/shared/security/tests/outboundUrlGuard.test.ts	View secret

🛠 Guidelines to remediate hardcoded secrets

1. Understand the implications of revoking this secret by investigating where it is used in your code.
2. Replace and store your secret safely. Learn here the best practices.
3. Revoke and rotate this secret.
4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

• following these best practices for managing and storing secrets including API keys and other credentials
• install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

---

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}

[akansnoro]

Not a real secret — remediated in #1711.

GitGuardian id 38302542 · File backend/shared/security/__tests__/outboundUrlGuard.test.ts · R75

The finding is a test fixture, not a credential:

const result = validateOutboundUrl('https://hooks.zapier.com/hooks/catch/1/2/');

It is a URL passed to validateOutboundUrl to assert that a public https
host is accepted — there is no live endpoint, no account, and nothing to
revoke.

The reason it still fired is that the /hooks/catch/<n>/<n>/ path shape is
byte-for-byte what a real Zapier hook URL looks like, so the detector cannot
tell the two apart. Replaced in #1711 with a non-/hooks/catch/ path on the
same host:

const result = validateOutboundUrl('https://hooks.zapier.com/example-endpoint');

The assertion still needs only "public host over https", and keeping the real
hooks.zapier.com hostname preserves the realistic-host coverage. No secret
was ever exposed, so no rotation or history rewrite is required — but the fix
does prevent this recurring and creating noise on every future PR that touches
the file.

Thanks for the catch.