Skip to content

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

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-4790-otp-budget-shared-cache
Aug 3, 2026
Merged

fix(plugin-auth): 每号码 OTP 发送预算改用惰性解析的共享计数存储 —— 多节点下不再按节点数倍增 (#4790)#4806
os-zhuang merged 1 commit into
mainfrom
claude/issue-4790-otp-budget-shared-cache

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #4790

一、先核实(PM 指令 #1):issue 成立,现象与描述一致

按要求先验证再动手,结论是该洞真实存在,且存储来源确实就是 AuthManagerOptions.secondaryStorage:

  1. 唯一存储来源 —— AuthManager.getOtpSendGuard()(auth-manager.ts,改动前)构造 OtpSendGuard 时,只有这一处会传 store:

    ...(this.config.secondaryStorage ? { storage: this.config.secondaryStorage } : {}),

    OtpSendGuard 内部 load()/save() 是二选一分支:有 storage 走共享 KV,没有就落到实例内的 private readonly local = new Map()没有第三条路径,也没有任何地方从 cache 服务取过它。

  2. 默认组合下确实为空 —— 全仓检索 secondaryStorage 的赋值点,packages/plugins/plugin-auth/src 之外只剩各包的 CHANGELOG.md 命中,没有任何宿主代码提供它;packages/cli/src/commands/serve.tsnew AuthPlugin({...}) 不传;而 fix(plugin-auth): 限流计数器惰性解析 kernel cache —— 误报的告警,与它掩盖的共享限流功能洞 #4788 之后 AuthPlugin.init() 明确不再从 kernel cache 派生 secondaryStorage(注释写明:那会把会话的记录之处搬进缓存,废掉 ADR-0069 D4)。所以标准 serve 组合下 storage === undefined,预算落在每进程一份local Map 里。

  3. 无任何信号 —— 这条路径上一句 warn 都没有,配置里能写每号码预算,多节点下不兑现,而运维看不到任何提示(ADR-0049 声明 ≠ 强制)。

  4. 确属 fix(plugin-auth): 限流计数器惰性解析 kernel cache —— 误报的告警,与它掩盖的共享限流功能洞 #4788 未覆盖的独立一处 —— fix(plugin-auth): 限流计数器惰性解析 kernel cache —— 误报的告警,与它掩盖的共享限流功能洞 #4788 改的是 better-auth 的 rateLimit.customStorage;OTP 预算是 ObjectStack 自己在 AuthManager 里的另一套计数,行为未被 fix(plugin-auth): 限流计数器惰性解析 kernel cache —— 误报的告警,与它掩盖的共享限流功能洞 #4788 改变。

结论:issue 不需要关单,按原方向修。

二、修法:复用 #4788 那条路径,不写第二份

rate-limit-storage.ts 里把「惰性解析 → 绑定即宣告 → 解析不到就降级到有界进程内存储并响亮告警」这半边抽成 createLazyCounterStore();createLazyCacheRateLimitStorage() 现在是它的一层薄封装(行为、日志文案对限流侧保持不变,#4788 的断言原样通过)。OTP 预算经新增的 AuthManagerOptions.sharedCounterStore 接同一条路径,由 AuthPlugin同一个 resolveCache 闭包填充。

一处对 PM 指令 #2 的偏离,请复核

指令说复用 createLazyCacheRateLimitStorage() / incrementFixedWindow。我复用了前者的解析路径(抽出共用),但没有把 OTP 的计数改成 incrementFixedWindow 的定窗计数,理由:

所以时间戳滚动窗口的算法原样保留,只换了它所在的存储——这正是本单的缺陷所在。若维护者更希望统一到定窗计数(接受上述语义变化),这部分可以单独再改。

三、验收对应

验收项 证据
配了 cache 的多节点部署,预算真正跨节点共享 otp-send-guard.test.ts "is ONE budget across nodes when the cache is shared":两个 guard(两套 resolver)共用一个 cache,A 发过之后 B 立刻被拒;小时额度也是一份
cache 在 auth 之后注册,计数仍落到共享 cache auth-plugin.test.ts "counts the budget in a cache registered AFTER auth init":init 时 cache 不在注册表,之后再注册,第一次发送就落进共享 store(phone-otp-sends:+86...),并打出 bound 的 info
没有 cache 的部署仍然限额,降级不是关闭 auth-plugin.test.ts "warns loudly at counting time…":第二次发送仍被 429;warn 只在真正计数时打一次,含 PAID SMSno cache service registered at all
两种情况必须能区分 绑定 → info 且无 warn;降级 → warn 且无 info(两条断言分别钉在上面两个用例里)

四、#2814 短信配额闸的核对(PM 指令 #4)

不是同一套计数,无需另立 issue。 packages/services/service-sms/src/ 下(sms-service.ts / sms-plugin.ts / transports/)检索 quota / daily 均无命中,#2814 目前尚未实施——daily_quota / daily_quota_per_tenant 都还不存在,因此不存在「建立在进程内计数上、上线即失效」的第二个闸。本 PR 抽出的 createLazyCounterStore() 正好是 #2814 落地时该用的那条路径(#2814 正文第 4 点要求的就是这个降级策略)。

五、验证

pnpm --filter @objectstack/plugin-auth test   → Test Files 29 passed (29) / Tests 649 passed (649)
pnpm --filter @objectstack/plugin-auth typecheck → tsc --noEmit,无输出(通过)
pnpm --filter @objectstack/plugin-auth build  → ESM/CJS/DTS build success

已加 changeset:.changeset/auth-otp-budget-shared-counter-store.md


Generated by Claude Code

…store (#4790)

#2780's per-number OTP budget (60s cooldown + 5/hour) was shared across nodes
ONLY when a host supplied better-auth's `secondaryStorage`. Nothing in the
standard `serve` composition supplies one — and since #4788, AuthPlugin
deliberately does not derive it from the kernel cache either — so the budget
was counted per process: an N-node deployment granted one phone number N
cooldowns and N hourly caps, in paid SMS, with no signal that the declared
limit was not the enforced one (ADR-0049).

Same defect class as #4772's rate-limit counters, and now the same cure rather
than a second implementation of it. The lazy-resolution half of
`createLazyCacheRateLimitStorage` is extracted as `createLazyCounterStore()`:
resolve the `cache` service when a counter is CONSUMED (strictly after
`kernel:ready`, so plugin start order decides nothing), memoise the handle,
fall back to the bounded in-process store when there is genuinely no cache —
and say which of the two happened, once. The OTP guard reaches it through the
new `AuthManagerOptions.sharedCounterStore`, filled by AuthPlugin from the
same `resolveCache` closure the rate-limit counters use.

Deliberately NOT `secondaryStorage` (#4785): that also relocates the session of
record into the cache and silently disables the ADR-0069 D4 session controls. A
host-supplied `secondaryStorage` still wins for this budget, unchanged.

The cooldown / rolling-hour semantics are untouched — only where the timestamps
live changed. A fixed-window counter cannot express "N seconds since the last
send", and converting the hourly cap to one would admit a 2× burst across the
window boundary: trading one multiplication for another.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
@vercel

vercel Bot commented Aug 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 3, 2026 7:54am

Request Review

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth.

10 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx (via @objectstack/plugin-auth)
  • content/docs/deployment/production-readiness.mdx (via @objectstack/plugin-auth)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/plugin-auth)
  • content/docs/permissions/authentication.mdx (via @objectstack/plugin-auth)
  • content/docs/permissions/sso.mdx (via @objectstack/plugin-auth)
  • content/docs/plugins/index.mdx (via @objectstack/plugin-auth)
  • content/docs/plugins/packages.mdx (via @objectstack/plugin-auth)
  • content/docs/protocol/kernel/http-protocol.mdx (via @objectstack/plugin-auth)
  • content/docs/releases/implementation-status.mdx (via @objectstack/plugin-auth)
  • content/docs/releases/v9.mdx (via @objectstack/plugin-auth)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@os-zhuang
os-zhuang marked this pull request as ready for review August 3, 2026 08:08
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 3, 2026
Merged via the queue into main with commit 8bd437f Aug 3, 2026
21 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-4790-otp-budget-shared-cache branch August 3, 2026 08:15
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Aug 4, 2026
…down it measures (objectstack-ai#4808) (objectstack-ai#4869)

`OtpSendGuard` enforces two dimensions with two different windows — a
per-number cooldown (`cooldownSeconds`) and a per-number rolling-hour cap
(`maxPerHour`) — but both were pruned, and stored with a TTL, at a flat
one hour: the cap's window, borrowed for the cooldown.

So `phoneOtp.cooldownSeconds` above 3600 was accepted, with no validation
error and no warning, and then served as one hour, because the record the
cooldown is measured from had already been dropped. A declared 2-hour
cooldown was really 1 hour — half the declared anti-abuse strength on a
PAID channel, silently (ADR-0049, declared != enforced). Same guard as
objectstack-ai#4790, a different defect; behaviour identical before and after objectstack-ai#4806.

History is now retained for `max(1 hour, cooldownSeconds)` — the longer of
the two windows — with the TTL following it, so the entry outlives what it
measures. The hourly cap keeps counting over its own rolling hour, so a
long cooldown cannot make `maxPerHour` stricter than declared either.

The bound is a rejection, not a higher truncation point:
`cooldownSeconds` over MAX_COOLDOWN_SECONDS (86400 / 24h), negative or
non-finite throws from `assertOtpCooldownSeconds()`, called from the
`AuthManager` constructor so a bad config fails at boot rather than as a
500 on the first `/phone-number/send-otp`. Moving the truncation further
out would only be the same defect one order of magnitude away.

Default config is unchanged and pinned by a test: 60s cooldown, 5 per
rolling hour, 3600s retention and TTL.


Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

每号码 OTP 发送预算(#2780)也只在进程内计数 —— 与 #4772 的限流洞同类,多节点下可按节点数倍增

2 participants