fix(plugin-auth): 每号码 OTP 发送预算改用惰性解析的共享计数存储 —— 多节点下不再按节点数倍增 (#4790)

.changeset/auth-otp-budget-shared-counter-store.md

@@ -0,0 +1,41 @@
+---
+"@objectstack/plugin-auth": patch
+---
+
+fix(plugin-auth): 每号码 OTP 发送预算改用惰性解析的共享计数存储 —— 多节点下不再按节点数倍增 (#4790)
+
+#2780 的「每号码 OTP 发送预算」（60s 冷却 + 每小时 5 条）此前**只有宿主显式提供
+better-auth `secondaryStorage` 时才跨节点共享**：`AuthManager.getOtpSendGuard()` 唯一的
+存储来源就是 `AuthManagerOptions.secondaryStorage`，而标准 `serve` 组合里没有任何一处
+提供它（#4788 之后 `AuthPlugin` 也明确不再从 cache 服务派生它）。于是预算落在**每个进程
+一份**：N 个节点的部署，一个号码实际能收到的是声明值的 N 倍，而且**没有任何信号**告诉你
+它没兑现（ADR-0049 声明 ≠ 强制）。这里的计价单位是**真金白银的短信**。
+
+这是 #4772 那条限流洞的同类，但是独立的一处：#4788 修的是 better-auth 自己的 `rateLimit`
+计数器（走 `rateLimit.customStorage`），OTP 预算是 ObjectStack 在 `AuthManager` 里自己实现
+的另一套计数，行为未被 #4788 改变。
+
+**修法：复用 #4788 建好的那条路径，而不是再写一份。** `rate-limit-storage.ts` 中把「惰性
+解析 → 绑定即宣告 → 解析不到就降级到有界的进程内存储并响亮告警」抽成
+`createLazyCounterStore()`（`createLazyCacheRateLimitStorage()` 现在就是它的一层薄封装），
+OTP 预算经由新的 `AuthManagerOptions.sharedCounterStore` 接同一条路径：
+
+- **存储在每次发送校验时才解析**，因此 `CacheServicePlugin` 晚于 `AuthPlugin` 注册也照样
+  绑定得上（插件启动顺序不再决定任何事）—— 这正是 #4772 冻结结论造成的那个洞；
+- 配了 cache 的多节点部署，每号码预算**现在真的是一份**，换节点不会重新获得冷却额度；
+- 没有 cache 服务的部署**仍然限额**，只是降级为进程内计数，并在第一次真正计数时打一条
+  点名代价的 warn（「an N-node deployment can send up to N× the configured number of PAID
+  SMS to one number」）—— 降级不是关闭，两种情况在日志里可区分（绑定打 info，降级打 warn）。
+
+**刻意不引入 `secondaryStorage` 来修它**（#4785）：那会把会话的记录之处搬进缓存，静默废掉
+ADR-0069 D4 的三个会话管控。宿主自己提供的 `secondaryStorage` 对这个预算仍然优先且行为不变。
+
+冷却与滚动小时窗的语义**未做任何改动**：计数依旧是按号码的时间戳滚动窗口，只是换了它所在的
+存储。（固定窗口计数器无法表达「距上一次发送满 N 秒」，把它改成定窗会在窗口边界放行两倍突发
+——用一种倍增换另一种倍增。）
+
+对使用者的影响：
+
+- 新增 `AuthManagerOptions.sharedCounterStore`，`AuthPlugin` 自动填充，一般宿主无需感知；
+- 新增导出 `createLazyCounterStore()` 与 `counterStoreFromKv()`；
+- `OtpSendGuard` 新增 `resolveStore` 选项，原有的 `storage`（字符串 KV）选项保持可用。

packages/plugins/plugin-auth/src/auth-manager.test.ts

@@ -1734,6 +1734,88 @@ describe('AuthManager', () => {
       await manager.assertPhoneOtpSendAllowed(PHONE);
     });
 
+    // ── #4790 — the budget is only global if its STORE is ─────────────────
+    describe('where the per-number budget is counted (#4790)', () => {
+      const makeCache = () => {
+        const store = new Map<string, unknown>();
+        return {
+          store,
+          get: vi.fn(async (k: string) => (store.has(k) ? store.get(k) : undefined)),
+          set: vi.fn(async (k: string, v: unknown, _ttl?: number) => { store.set(k, v); }),
+        };
+      };
+      const bootNode = async (sharedCounterStore: any) => {
+        const { manager } = await bootOtp({ sharedCounterStore });
+        manager.setSmsService(fakeSms().service);
+        return manager;
+      };
+
+      it('spends ONE budget across nodes when a shared counter store is wired', async () => {
+        const { createLazyCounterStore } = await import('./rate-limit-storage.js');
+        const cache = makeCache();
+        // Two nodes: separate managers, separate resolvers, one cache.
+        const nodeStore = () =>
+          createLazyCounterStore({ resolveCache: async () => cache as any, subject: 'otp-budget' });
+        const nodeA = await bootNode(nodeStore());
+        const nodeB = await bootNode(nodeStore());
+
+        await nodeA.assertPhoneOtpSendAllowed(PHONE);
+        // Rotating nodes no longer buys a fresh cooldown.
+        await expect(nodeB.assertPhoneOtpSendAllowed(PHONE))
+          .rejects.toThrow(/Too many verification codes/);
+        expect(cache.store.size).toBe(1);
+        expect([...cache.store.keys()][0]).toContain(PHONE);
+      });
+
+      it('resolves the store per check, so a cache registered after boot still binds', async () => {
+        const { createLazyCounterStore } = await import('./rate-limit-storage.js');
+        const cache = makeCache();
+        let registered = false;
+        const manager = await bootNode(
+          createLazyCounterStore({
+            resolveCache: async () => (registered ? (cache as any) : undefined),
+            subject: 'otp-budget',
+          }),
+        );
+        registered = true; // CacheServicePlugin comes up after plugin-auth.
+        await manager.assertPhoneOtpSendAllowed(PHONE);
+        expect(cache.store.size).toBe(1);
+      });
+
+      it('a host-supplied secondaryStorage keeps owning the budget', async () => {
+        const kv = new Map<string, string>();
+        const secondaryStorage = {
+          get: async (k: string) => kv.get(k) ?? null,
+          set: async (k: string, v: string) => { kv.set(k, v); },
+          delete: async (k: string) => { kv.delete(k); },
+        };
+        const cache = makeCache();
+        const { manager } = await bootOtp({
+          secondaryStorage,
+          sharedCounterStore: async () => cache as any,
+        });
+        manager.setSmsService(fakeSms().service);
+        await manager.assertPhoneOtpSendAllowed(PHONE);
+        expect(kv.size).toBe(1);
+        expect(cache.store.size).toBe(0);
+      });
+
+      it('without any shared store the budget is per-manager — degraded, still enforced', async () => {
+        const { manager: nodeA } = await bootOtp();
+        const { manager: nodeB } = await bootOtp();
+        nodeA.setSmsService(fakeSms().service);
+        nodeB.setSmsService(fakeSms().service);
+
+        await nodeA.assertPhoneOtpSendAllowed(PHONE);
+        // Enforced on its own node…
+        await expect(nodeA.assertPhoneOtpSendAllowed(PHONE))
+          .rejects.toThrow(/Too many verification codes/);
+        // …and not on the other one: exactly the N× multiplication #4790 is
+        // about, which is why AuthPlugin warns loudly when it has to do this.
+        await nodeB.assertPhoneOtpSendAllowed(PHONE);
+      });
+    });
+
     it('features.phoneNumberOtp requires plugin + deliverable SMS', async () => {
       const { manager } = await bootOtp();
       expect((manager.getPublicConfig() as any).features.phoneNumberOtp).toBe(false);

packages/plugins/plugin-auth/src/auth-manager.ts

@@ -26,6 +26,7 @@ import { isPlaceholderEmail } from './placeholder-email.js';
 import { reconcileMembership, type MembershipPolicy } from './reconcile-membership.js';
 import type { TenancyService } from './tenancy-service.js';
 import { OtpSendGuard } from './otp-send-guard.js';
+import type { CounterStore } from './rate-limit-storage.js';
 import {
   PHONE_SMS_TOPICS,
   builtinPhoneSmsBody,
@@ -620,6 +621,26 @@ export interface AuthManagerOptions extends Partial<AuthConfig> {
    * drop the store.
    */
   rateLimitStorage?: NonNullable<BetterAuthOptions['rateLimit']>['customStorage'];
+
+  /**
+   * ADR-0069 D2 (#4790) — the store ObjectStack's OWN cross-node counters
+   * count in, resolved LAZILY (called per count, not at construction). Today
+   * that is the #2780 per-number OTP send budget; {@link rateLimitStorage}
+   * covers better-auth's own per-IP counters, which never pass through here.
+   *
+   * `AuthPlugin` supplies `createLazyCounterStore(...)` over the kernel `cache`
+   * service (`rate-limit-storage.ts`) — the same resolution the rate-limit
+   * counters use, so plugin start order decides nothing and a deployment
+   * WITHOUT a cache still counts, per process, and is told so. Absent → the
+   * budget is per-process silently, which is the pre-#4790 behaviour and is
+   * why this option exists.
+   *
+   * NOT `secondaryStorage`: handing better-auth one of those also relocates the
+   * session of record into the cache and silently disables the ADR-0069 D4
+   * session controls (#4785). A host that supplies `secondaryStorage`
+   * deliberately still wins for this budget — see `getOtpSendGuard`.
+   */
+  sharedCounterStore?: () => Promise<CounterStore>;
 }
 
 /**
@@ -2511,12 +2532,22 @@ export class AuthManager {
   private getOtpSendGuard(): OtpSendGuard {
     if (!this._otpSendGuard) {
       const otpCfg = this.config.phoneOtp ?? {};
+      // WHERE the budget is counted decides whether it is a budget at all
+      // (#4790): counted per process, a declared "5 per hour" is 5×N across N
+      // nodes. Precedence, most deliberate first:
+      //  1. a host-supplied `secondaryStorage` — an explicit cross-node KV;
+      //  2. `sharedCounterStore` — AuthPlugin's lazily-resolved kernel `cache`,
+      //     which also announces the degraded (no cache) case loudly;
+      //  3. neither → the guard's own bounded per-process store.
+      const storeOption = this.config.secondaryStorage
+        ? { storage: this.config.secondaryStorage }
+        : this.config.sharedCounterStore
+          ? { resolveStore: this.config.sharedCounterStore }
+          : {};
       this._otpSendGuard = new OtpSendGuard({
         ...(otpCfg.cooldownSeconds != null ? { cooldownSeconds: otpCfg.cooldownSeconds } : {}),
         ...(otpCfg.maxPerHour != null ? { maxPerHour: otpCfg.maxPerHour } : {}),
-        // Share better-auth's cross-node KV when wired (ADR-0069 D2) so the
-        // per-number budget is enforced against ONE store across nodes.
-        ...(this.config.secondaryStorage ? { storage: this.config.secondaryStorage } : {}),
+        ...storeOption,
       });
     }
     return this._otpSendGuard;

packages/plugins/plugin-auth/src/auth-plugin.test.ts

@@ -1319,5 +1319,69 @@ describe('AuthPlugin', () => {
       const { manager } = await bootWith(async () => cache);
       expect((manager as any).config.secondaryStorage).toBeUndefined();
     });
+
+    // ── #4790 — the SECOND counter with the same hole: ObjectStack's own
+    // per-number OTP send budget (#2780). It shared state only through a
+    // host-supplied `secondaryStorage`, which the standard `serve` composition
+    // never supplies, so the declared per-number cap was cap×N across N nodes —
+    // in paid SMS. Same cure, same resolution path, deliberately NOT
+    // `secondaryStorage` (#4785).
+    describe('per-number OTP send budget (#2780 / #4790)', () => {
+      const PHONE = '+8613800000000';
+      const deliverableSms = { async send() { return { id: 'x', status: 'sent' }; }, isConfigured: () => true };
+
+      it('counts the budget in a cache registered AFTER auth init', async () => {
+        const cache = makeCache();
+        let registered = false;
+        const { ctx, manager } = await bootWith(async () => (registered ? cache : undefined));
+        expect((manager as any).config.sharedCounterStore).toBeTypeOf('function');
+
+        // CacheServicePlugin comes up 21ms later.
+        registered = true;
+        manager.setSmsService(deliverableSms as any);
+
+        await manager.assertPhoneOtpSendAllowed(PHONE);
+        // The send landed in the SHARED store — this is the assertion the
+        // pre-#4790 wiring could not satisfy in any standard composition.
+        expect([...cache.store.keys()]).toEqual([`phone-otp-sends:${PHONE}`]);
+        await expect(manager.assertPhoneOtpSendAllowed(PHONE))
+          .rejects.toThrow(/Too many verification codes/);
+
+        // Bound → an info line and NO warning: the operator must be able to
+        // tell "shared" from "degraded" without reading the code.
+        const info = (ctx.logger.info as any).mock.calls.map((c: any[]) => String(c[0]));
+        expect(info.some((m: string) => m.includes('per-number OTP send budget (#2780) bound to the kernel cache service'))).toBe(true);
+        expect((ctx.logger.warn as any).mock.calls
+          .map((c: any[]) => String(c[0]))
+          .filter((m: string) => m.includes('per-number OTP send budget'))).toEqual([]);
+      });
+
+      it('warns loudly at counting time when there is no cache — degraded, never disabled', async () => {
+        const { ctx, manager } = await bootWith(async () => undefined);
+        manager.setSmsService(deliverableSms as any);
+        const otpWarnings = () =>
+          (ctx.logger.warn as any).mock.calls
+            .map((c: any[]) => String(c[0]))
+            .filter((m: string) => m.includes('per-number OTP send budget'));
+
+        // Nothing is said at boot — a deployment that never sends an OTP is not
+        // warned about a store it never needs.
+        expect(otpWarnings()).toEqual([]);
+
+        await manager.assertPhoneOtpSendAllowed(PHONE);
+        // Still enforced, in-process.
+        await expect(manager.assertPhoneOtpSendAllowed(PHONE))
+          .rejects.toThrow(/Too many verification codes/);
+
+        expect(otpWarnings()).toHaveLength(1);
+        expect(otpWarnings()[0]).toContain('PAID SMS');
+        expect(otpWarnings()[0]).toContain('no `cache` service registered at all');
+        // Degraded → a warning and NO "bound" info line; the mirror image of
+        // the cache-present case above.
+        expect((ctx.logger.info as any).mock.calls
+          .map((c: any[]) => String(c[0]))
+          .filter((m: string) => m.includes('per-number OTP send budget'))).toEqual([]);
+      });
+    });
   });
 });

packages/plugins/plugin-auth/src/auth-plugin.ts

@@ -41,6 +41,7 @@ import { MANAGED_EXTENSION_EDITABLE_FIELDS } from './managed-extension-fields.js
 import { runSetInitialPassword } from './set-initial-password.js';
 import { runRegisterSsoProviderFromForm, runRegisterSamlProviderFromForm, runRequestDomainVerification, runVerifyDomain } from './register-sso-provider.js';
 import { runResendVerificationEmail } from './send-verification-email.js';
+import type { CounterStore } from './rate-limit-storage.js';
 import {
   authIdentityObjects,
   authPluginManifestHeader,
@@ -356,24 +357,51 @@ export class AuthPlugin implements Plugin {
     // Whether the session of record should live in the cache at all is #4785.
     // A host that supplies `secondaryStorage` itself still wins and keeps
     // better-auth's own `storage: 'secondary-storage'` counters.
-    if (!authConfig.secondaryStorage) {
-      const { createLazyCacheRateLimitStorage } = await import('./rate-limit-storage.js');
-      authConfig.rateLimitStorage = createLazyCacheRateLimitStorage({
-        // The `cache` service is registered ASYNC — `getService` throws for it,
-        // so resolve via `getServiceAsync` and treat any failure (not
-        // registered, or not yet ready) as "no shared cache, ask again later".
-        resolveCache: async () => {
-          let cache: any;
-          try {
-            cache = await (ctx as { getServiceAsync?: (n: string) => Promise<unknown> })
-              .getServiceAsync?.('cache');
-          } catch {
-            return undefined;
-          }
-          if (cache && typeof cache.get === 'function' && typeof cache.set === 'function') return cache;
-          return undefined;
-        },
+    //
+    // The SAME resolution carries ObjectStack's own per-number OTP send budget
+    // (#2780) below — that counter had the identical hole (#4790): it counted
+    // in `secondaryStorage` when a host supplied one and per-process otherwise,
+    // which under the standard `serve` composition (nobody supplies one) meant
+    // every node granted the same phone number its own cooldown + hourly cap.
+    // The currency there is paid SMS, so an N-node deployment was billed for up
+    // to N× the declared budget.
+    //
+    // The `cache` service is registered ASYNC — `getService` throws for it, so
+    // resolve via `getServiceAsync` and treat any failure (not registered, or
+    // not yet ready) as "no shared cache, ask again later".
+    const resolveCache = async (): Promise<CounterStore | undefined> => {
+      let cache: any;
+      try {
+        cache = await (ctx as { getServiceAsync?: (n: string) => Promise<unknown> })
+          .getServiceAsync?.('cache');
+      } catch {
+        return undefined;
+      }
+      if (cache && typeof cache.get === 'function' && typeof cache.set === 'function') return cache;
+      return undefined;
+    };
+
+    {
+      const { createLazyCacheRateLimitStorage, createLazyCounterStore } = await import(
+        './rate-limit-storage.js'
+      );
+      if (!authConfig.secondaryStorage) {
+        authConfig.rateLimitStorage = createLazyCacheRateLimitStorage({
+          resolveCache,
+          logger: ctx.logger,
+        });
+      }
+      // #4790 — ObjectStack's own counters. Wired even when the host supplied a
+      // `secondaryStorage` (AuthManager prefers that store for the budget and
+      // then never calls this resolver), so the two decisions stay independent.
+      authConfig.sharedCounterStore = createLazyCounterStore({
+        resolveCache,
         logger: ctx.logger,
+        subject: 'per-number OTP send budget (#2780)',
+        degradedImpact:
+          'The budget is still enforced, but PER NODE: every node grants the same phone number its own ' +
+          'cooldown and hourly cap, so an N-node deployment can send up to N× the configured number of ' +
+          'PAID SMS to one number (#4790)',
       });
     }