fix(billing): v4 hardening — finish v3 oversights + cohérence API (#3584)

* fix(billing): v4 hardening — finish v3 oversights + cohérence API constants

- weeklyReset cron: applyJitter (les 3 autres l'ont)
- constants extraction: dollarsToUnitRatio + maxUnitsPerOperation + getDefaultPlanId
- reset.service: drop isoWeekKey re-export (lib leak)
- usage.service: 80% threshold no longer skipped on >100% jump (drop break)
- usage.service + init: thresholdFields config validation at boot (meterMode-gated)
- README: full config knobs table
- nits: cron-utils fractional-input guard restored, Date.now comment, errors.js extension comment
- alertCrossed JSDoc: document last-threshold-emitted semantics on multi-crossing

* fix(billing): address Opus review — requireQuota uses getDefaultPlanId

- middleware: consolidate plan fallback via getDefaultPlanId()
- usage.service: invariant comment on threshold loop ordering
- usage.service: stale-memory comment on updatedDoc[field]
- README: thresholdPercents notes complete
- errors.js: drop YAGNI extension comment

* fix(billing): address CodeRabbit/Codacy/Copilot v4 review comments

- constants: add type guards to getDollarsToUnitRatio (0/NaN → 1000),
  getMaxUnitsPerOperation (invalid → Infinity), getDefaultPlanId (empty → 'free')
- constants: guard getAlertThresholdPercents against string env-override
  (e.g. DEVKIT_NODE_billing_alerts_thresholdPercents=80 — coerce to [80])
- weeklyReset cron: add mongooseService.loadModels() before connect()
  to match retry-pending-extras-debit.cron.js pattern (Copilot LYE-)
- service: add random suffix to extras_checkout idempotency key to reduce
  collision risk on concurrent clicks (Codacy LXAx / CodeRabbit LXwS)
- README: point defaults source to billing.constants.js, fix maxUnitsPerOperation
  default (10000 from dev config, Infinity from constant fallback), note
  ratioVersion is not wrapped in a constant (Copilot LYFL, CodeRabbit LXwQ)
- tests: guard tests for new constant fallback behaviours (0/NaN/empty)
- tests: add getAlertThresholdPercents string coercion test
- tests: weeklyReset cron — replace export parity checks with contract test
  (applyJitter(getCronJitterMaxMs()) produces valid delay in [0, maxMs))
- tests: 0%→150% regression — add event emit assertions (LXwU)

* fix(billing): use crypto.randomBytes for idempotency key nonce

Math.random() in the idempotency key was flagged by Codacy as insecure
random in a security-sensitive context. Switch to node:crypto randomBytes(4)
which produces a cryptographically secure 8-char hex suffix.

Author: PierreBrisorgueil

modules/billing/README.md

@@ -105,7 +105,24 @@ Consumers should NOT retry on `applied: true` — the outbox handles eventual co
 
 ## Meter hardening configuration
 
-Defaults live in `modules/billing/config/billing.development.config.js` and can be overridden by downstream project config:
+### Configuration knobs
+
+| Knob | Type | Devkit default | Notes |
+|------|------|----------------|-------|
+| `billing.meter.runBase` | number | 1 | METER_RUN_BASE base unit cost |
+| `billing.meter.fallbackPlanId` | string \| null | null | Fallback plan when active not resolvable |
+| `billing.meter.dollarsToUnitRatio` | number | 1000 | Dollar → unit conversion. DOWNSTREAM-OVERRIDE-REQUIRED. Constant fallback: `getDollarsToUnitRatio()` |
+| `billing.meter.maxUnitsPerOperation` | number | 10000 | Cap per single attribute call (dev config). Constant fallback: `Infinity` via `getMaxUnitsPerOperation()` |
+| `billing.meter.ratioVersion` | string \| null | '2026.05' | DOWNSTREAM-OVERRIDE-REQUIRED — pricing version namespace. Read directly from config, no constant wrapper |
+| `billing.outbox.maxRetryAttempts` | number | 5 | Outbox retry limit before exhausted |
+| `billing.outbox.retryIntervalSec` | number | 300 | Cron retry interval |
+| `billing.crons.jitterMaxMs` | number | 60000 | Cron startup jitter max. Constant fallback: `getCronJitterMaxMs()` |
+| `billing.planChange.preserveUsageDefault` | boolean | true | forceRotateForPlanChange default |
+| `billing.alerts.thresholdPercents` | number[] | [80, 100] | Schema-supported only — others warn at boot, alert silently skipped. Constant fallback: `getAlertThresholdPercents()` |
+| `billing.events.extrasExhausted` | string | 'billing.extras_debit.exhausted' | Event name for downstream alerting |
+| `billing.defaultPlan` | string | 'free' | Default plan ID for fallback. Constant fallback: `getDefaultPlanId()` |
+
+Canonical constant fallbacks live in `modules/billing/lib/billing.constants.js`. Downstream project overrides go in `modules/billing/config/billing.development.config.js`:
 
 ```js
 billing: {

modules/billing/billing.init.js

@@ -7,6 +7,7 @@ import AnalyticsService from '../../lib/services/analytics.js';
 import billingEvents from './lib/events.js';
 import BillingPlanService from './services/billing.plan.service.js';
 import BillingUsageRepository from './repositories/billing.usage.repository.js';
+import { getAlertThresholdPercents } from './lib/billing.constants.js';
 
 /**
  * Billing module initialisation.
@@ -26,6 +27,19 @@ export default async (app) => {
     }
   }
 
+  // Validate alert threshold percents (meterMode only) — warn on configured values with no schema field.
+  // Only 80 and 100 have matching alertedAtN fields in BillingUsage; other values are silently skipped.
+  if (config?.billing?.meterMode) {
+    const SUPPORTED_THRESHOLD_PERCENTS = new Set([80, 100]);
+    for (const threshold of getAlertThresholdPercents()) {
+      if (!SUPPORTED_THRESHOLD_PERCENTS.has(threshold)) {
+        console.warn(
+          `[billing] Configured alert threshold ${threshold}% is not in schema-supported set [80, 100] — alert will be silently skipped`,
+        );
+      }
+    }
+  }
+
   // Update analytics group properties when a subscription plan changes
   billingEvents.on('plan.changed', ({ organizationId, newPlan }) => {
     try {

modules/billing/crons/billing.weeklyReset.js

@@ -13,17 +13,27 @@
 
 process.env.NODE_ENV = process.env.NODE_ENV || 'development';
 
-const [{ default: config }, { default: mongooseService }] = await Promise.all([
+const [
+  { default: config },
+  { default: mongooseService },
+  { applyJitter },
+  { getCronJitterMaxMs },
+] = await Promise.all([
   import('../../../config/index.js'),
   import('../../../lib/services/mongoose.js'),
+  import('../lib/billing.cron-utils.js'),
+  import('../lib/billing.constants.js'),
 ]);
 
 if (!config?.billing?.meterMode) {
   console.log('[billing.weeklyReset] meterMode disabled — skipping.');
   process.exit(0);
 }
 
+await applyJitter(getCronJitterMaxMs());
+
 try {
+  await mongooseService.loadModels();
   await mongooseService.connect();
 
   const { default: BillingResetService } = await import('../services/billing.reset.service.js');

modules/billing/lib/billing.constants.js

@@ -78,12 +78,50 @@ export const getPlanChangePreserveUsageDefault = () =>
  * @returns {number[]} Sorted threshold percentages.
  */
 export const getAlertThresholdPercents = () => {
-  const thresholds = config?.billing?.alerts?.thresholdPercents ?? DEFAULT_ALERT_THRESHOLD_PERCENTS;
+  const raw = config?.billing?.alerts?.thresholdPercents ?? DEFAULT_ALERT_THRESHOLD_PERCENTS;
+  // Guard against env-override delivering a string (e.g. DEVKIT_NODE_billing_alerts_thresholdPercents=80)
+  const thresholds = Array.isArray(raw) ? raw : [raw].map(Number).filter(Number.isFinite);
   return thresholds
+    .map(Number)
     .filter((threshold) => Number.isFinite(threshold) && threshold > 0)
     .sort((a, b) => b - a);
 };
 
+/**
+ * @function getDollarsToUnitRatio
+ * @description Resolve the configured conversion factor from dollar amounts to meter units.
+ * Returns the raw config value when valid; falls back to 1000.
+ * @returns {number} Dollar-to-unit ratio (e.g. 1000 means $1 = 1000 units).
+ */
+export const getDollarsToUnitRatio = () => {
+  const ratio = Number(config?.billing?.meter?.dollarsToUnitRatio);
+  return Number.isFinite(ratio) && ratio > 0 ? ratio : 1000;
+};
+
+/**
+ * @function getMaxUnitsPerOperation
+ * @description Resolve the configured per-operation unit cap. Infinity means no cap.
+ * Returns the raw config value when valid (positive finite or Infinity); falls back to Infinity.
+ * @returns {number} Maximum units allowed for a single attribute call.
+ */
+export const getMaxUnitsPerOperation = () => {
+  const raw = config?.billing?.meter?.maxUnitsPerOperation;
+  if (raw === undefined || raw === null) return Infinity;
+  const cap = Number(raw);
+  return Number.isFinite(cap) && cap > 0 ? cap : Infinity;
+};
+
+/**
+ * @function getDefaultPlanId
+ * @description Resolve the default plan ID used as a fallback when no active subscription exists.
+ * Returns the raw config value when non-empty string; falls back to 'free'.
+ * @returns {string} Default plan identifier.
+ */
+export const getDefaultPlanId = () => {
+  const planId = config?.billing?.defaultPlan;
+  return typeof planId === 'string' && planId.trim() ? planId : 'free';
+};
+
 /**
  * @function getExtrasExhaustedEventName
  * @description Resolve the event name emitted when extras debit retries are exhausted.

modules/billing/lib/billing.cron-utils.js

@@ -12,7 +12,7 @@ import { randomInt } from 'node:crypto';
 export const applyJitter = async (maxMs) => {
   if (!Number.isFinite(maxMs) || maxMs <= 0) return 0;
   const jitterMaxMs = Math.floor(maxMs);
-  if (jitterMaxMs <= 0) return 0;
+  if (jitterMaxMs <= 0) return 0; // guard fractional inputs (e.g. 0.4) — randomInt(0,0) throws
   const delayMs = randomInt(0, jitterMaxMs);
   await new Promise((resolve) => setTimeout(resolve, delayMs));
   return delayMs;

modules/billing/middlewares/billing.requireQuota.js

@@ -7,6 +7,7 @@ import BillingExtraBalanceRepository from '../repositories/billing.extraBalance.
 import BillingPlanService from '../services/billing.plan.service.js';
 
 import { activeStatuses } from '../lib/constants.js';
+import { getDefaultPlanId } from '../lib/billing.constants.js';
 import config from '../../../config/index.js';
 import responses from '../../../lib/helpers/responses.js';
 
@@ -81,7 +82,7 @@ function requireQuota(resource, action) {
           // Don't create the doc here — let incrementMeter do it on first attribution.
           // Fall back to the plan quota so first-run requests are not blocked.
           // Reuse the `subscription` already fetched by the degraded-mode gate above.
-          const planId = subscription?.plan ?? config?.billing?.defaultPlan ?? 'free';
+          const planId = subscription?.plan ?? getDefaultPlanId();
           const activePlan = await BillingPlanService.getActivePlan(planId);
 
           // Plan missing (seeding / version bump in progress) → fail safe with 503

modules/billing/services/billing.meter.service.js

@@ -6,7 +6,13 @@ import BillingPlanService from './billing.plan.service.js';
 import BillingUsageService from './billing.usage.service.js';
 import BillingExtraService from './billing.extra.service.js';
 import BillingMeterOutboxRepository from '../repositories/billing.meter.outbox.repository.js';
-import { getMeterFallbackPlanId, getMeterRunBase, METER_RUN_BASE } from '../lib/billing.constants.js';
+import {
+  getMeterFallbackPlanId,
+  getMeterRunBase,
+  getDollarsToUnitRatio,
+  getMaxUnitsPerOperation,
+  METER_RUN_BASE,
+} from '../lib/billing.constants.js';
 
 export { METER_RUN_BASE };
 
@@ -35,7 +41,7 @@ const unitsFromCosts = async (costs, planId, ratioVersion) => {
     return { totalUnits: getMeterRunBase(), breakdown: {} };
   }
 
-  const dollarsToUnitRatio = config?.billing?.meter?.dollarsToUnitRatio ?? 1000;
+  const dollarsToUnitRatio = getDollarsToUnitRatio();
 
   const hasBillableCost = Object.values(costs).some(
     (cost) => typeof cost === 'number' && Number.isFinite(cost) && cost > 0,
@@ -223,7 +229,7 @@ const attribute = async (history, organizationId, options = {}) => {
     ({ totalUnits, breakdown } = await unitsFromCosts(costs, planId, ratioVersion));
   }
 
-  const maxUnits = config?.billing?.meter?.maxUnitsPerOperation ?? Infinity;
+  const maxUnits = getMaxUnitsPerOperation();
   const cappedUnits = Math.min(totalUnits, maxUnits);
   const isCapped = cappedUnits < totalUnits;
   const cappedBreakdown = isCapped ? capBreakdown(breakdown, cappedUnits, totalUnits) : breakdown;

modules/billing/services/billing.reset.service.js

@@ -7,7 +7,7 @@ import BillingSubscriptionRepository from '../repositories/billing.subscription.
 import BillingPlanService from './billing.plan.service.js';
 import billingEvents from '../lib/events.js';
 import { isoWeekKey } from '../lib/billing.isoWeek.js';
-import { getPlanChangePreserveUsageDefault } from '../lib/billing.constants.js';
+import { getPlanChangePreserveUsageDefault, getDefaultPlanId } from '../lib/billing.constants.js';
 import { isDuplicateKeyError } from '../lib/billing.errors.js';
 
 /**
@@ -42,7 +42,7 @@ const resetWeek = async (orgId, periodStart) => {
 
   // Step 2 — Fetch the active plan to snapshot quota/planVersion — lean projection (plan only, no populate).
   const subscription = await BillingSubscriptionRepository.findPlan(orgId);
-  const planId = subscription?.plan ?? config?.billing?.defaultPlan ?? 'free';
+  const planId = subscription?.plan ?? getDefaultPlanId();
   const activePlan = await BillingPlanService.getActivePlan(planId);
   const meterQuota = activePlan?.meterQuota ?? 0;
   const planVersion = activePlan?.version ?? null;
@@ -102,7 +102,7 @@ const forceRotateForPlanChange = async (organizationId, options = {}) => {
   if (!existingDoc) return null;
 
   const subscription = await BillingSubscriptionRepository.findPlan(organizationId);
-  const planId = subscription?.plan ?? config?.billing?.defaultPlan ?? 'free';
+  const planId = subscription?.plan ?? getDefaultPlanId();
   const activePlan = await BillingPlanService.getActivePlan(planId);
   const newQuota = activePlan?.meterQuota ?? 0;
   const newVersion = activePlan?.version ?? null;
@@ -181,5 +181,4 @@ export default {
   resetWeek,
   forceRotateForPlanChange,
   resetAllDue,
-  isoWeekKey,
 };

modules/billing/services/billing.service.js

@@ -1,6 +1,7 @@
 /**
  * Module dependencies
  */
+import { randomBytes } from 'node:crypto';
 import config from '../../../config/index.js';
 import getStripe from '../lib/stripe.js';
 import BillingPlansService from './billing.plans.service.js';
@@ -193,8 +194,9 @@ const createExtrasCheckout = async (organization, packId, successUrl, cancelUrl)
 
   const subscription = await _ensureStripeCustomer(stripe, organization);
 
-  // Use a timestamped idempotency key (debounce double-click within ~1s granularity)
-  const idempotencyKey = `extras_checkout_${String(organization._id)}_${packId}_${Date.now()}`;
+  // Per-intent idempotency key: timestamp + crypto-random suffix reduces collision risk under concurrent clicks.
+  // Full deduplication would require a caller-provided stable intent id — deferred to a future improvement.
+  const idempotencyKey = `extras_checkout_${String(organization._id)}_${packId}_${Date.now()}_${randomBytes(4).toString('hex')}`;
 
   const extrasCheckoutParams = {
     customer: subscription.stripeCustomerId,

modules/billing/services/billing.usage.service.js

@@ -8,7 +8,7 @@ import BillingMeterOutboxRepository from '../repositories/billing.meter.outbox.r
 import BillingPlanService from './billing.plan.service.js';
 import billingEvents from '../lib/events.js';
 import { currentWeekKey } from '../lib/billing.isoWeek.js';
-import { getAlertThresholdPercents } from '../lib/billing.constants.js';
+import { getAlertThresholdPercents, getDefaultPlanId } from '../lib/billing.constants.js';
 import { isDuplicateKeyError } from '../lib/billing.errors.js';
 
 /**
@@ -70,6 +70,8 @@ const reset = (organizationId) => UsageRepository.reset(organizationId, currentM
  * @param {Object} breakdown - Feature-keyed breakdown: { featureKey: units }.
  * @param {string} idempotencyKey - Unique key for replay protection (usually history._id).
  * @returns {Promise<{applied: boolean, meterUsed: number, meterQuota: number, extrasConsumed: number, alertCrossed: string|null}>}
+ *   `alertCrossed` is the last threshold emitted this call (lowest value when multiple thresholds crossed in one jump,
+ *   e.g. 0%→150% emits both 80 and 100 — alertCrossed='80'). Informational only; events are the authoritative signal.
  */
 // biome-ignore lint/correctness/useQwikValidLexicalScope: false positive — Node.js service, not Qwik
 const incrementMeter = async (organizationId, units, breakdown, idempotencyKey) => {
@@ -82,7 +84,7 @@ const incrementMeter = async (organizationId, units, breakdown, idempotencyKey)
 
   // Fetch active plan for quota snapshot — lean projection (plan field only, no populate)
   const subscription = await BillingSubscriptionRepository.findPlan(organizationId);
-  const planId = subscription?.plan ?? config?.billing?.defaultPlan ?? 'free';
+  const planId = subscription?.plan ?? getDefaultPlanId();
   const activePlan = await BillingPlanService.getActivePlan(planId);
   const meterQuota = activePlan?.meterQuota ?? 0;
   const planVersion = activePlan?.version ?? null;
@@ -133,12 +135,14 @@ const incrementMeter = async (organizationId, units, breakdown, idempotencyKey)
 
   if (effectiveQuota > 0) {
     const pct = (newMeterUsed / effectiveQuota) * 100;
+    // loop runs DESC (e.g. [100, 80] from getAlertThresholdPercents()); alertCrossed retains the last (lowest) marked threshold by design.
     for (const threshold of getAlertThresholdPercents()) {
       const field = thresholdFields[threshold];
       if (!field) {
         console.warn(`[billing.usage] threshold ${threshold}% has no schema field (only 80/100 are supported) — skipping`);
         continue;
       }
+      // updatedDoc is pre-mark snapshot; DB-side dedup enforced by markThreshold conditional update.
       if (pct < threshold || updatedDoc[field]) continue;
 
       let marked = false;
@@ -157,7 +161,6 @@ const incrementMeter = async (organizationId, units, breakdown, idempotencyKey)
           meterUsed: newMeterUsed,
           meterQuota: effectiveQuota,
         });
-        break;
       }
     }
   }