Skip to content

fix(plugin-auth): OTP 冷却按声明值真正生效 —— 发送历史保留时长不再被硬编码 1 小时截断 (#4808) - #4869

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-4808-otp-cooldown-ttl
Aug 3, 2026
Merged

fix(plugin-auth): OTP 冷却按声明值真正生效 —— 发送历史保留时长不再被硬编码 1 小时截断 (#4808)#4869
os-zhuang merged 1 commit into
mainfrom
claude/issue-4808-otp-cooldown-ttl

Conversation

@os-zhuang

@os-zhuang os-zhuang commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #4808

现状核实(先验证,再动手)

在 origin/main(cb5a75e,已含 PR #4806)上确认:

为什么选方向 1(TTL 跟随配置),而不是方向 2(拒绝 > 3600)

关键认识是:这一小时根本不属于冷却,它是「每小时上限」的窗口,被借给了另一个维度。两个维度需要两个窗口,合用一个常量才是缺陷本身 —— 不是实现能力的固有上限。

  • 对本项目的长期正确性:方向 2 会把一条并不存在的实现限制永久写进公开契约(「冷却不能超过 1 小时」),以后想放开还要走一次变更;而算法上没有任何东西要求它是 1 小时。方向 1 把两个窗口拆开,是结构性修复而非补丁,符合 ADR-0049「声明即强制」与 Prime Directive [WIP] Fix error in step four of the action run #5(不要临时补丁)。
  • 让 AI 写出的配置难以出错:两个方向都能消除静默截断,但方向 1 单独使用会让某类错误变得更糟 —— 一个荒谬的数值以前静默退化成 1 小时,现在会被如实执行成一个超长冷却,并把条目按同样长度钉在共享缓存里。所以上限不是装饰,是让方向 1 安全的必要条件,而且它必须显式拒绝。

改动

保留时长跟随配置。 历史保留 max(1 小时, cooldownSeconds) —— 「两个维度里还用得着它的那个更长的窗」;TTL 同步跟随,记录因此活得比它所度量的冷却更久。

每小时上限仍在它自己的滚动一小时内计数(新增 withinHour),因此超长冷却不会反过来把 maxPerHour 收得比声明的更严。这一条今天是 belt-and-braces(落在更宽保留窗内的条目必然也落在冷却窗内,而冷却判定在前先行返回),代码里已如实注明 —— 写出来是为了让上限的窗口不会在下次任一窗口变动时,悄悄变成「冷却保留多久就算多久」。

上限是拒绝,不是又一次截断。 MAX_COOLDOWN_SECONDS = 86400(24 小时),超出/负数/非有限值一律由 assertOtpCooldownSeconds() 抛错,错误信息给出值、上限和改法。把截断点挪到更高的数字只是把同一个缺陷往外推一个量级。设上限的理由:一条号码的历史会在共享缓存里驻留整个冷却期,而超过一天的封锁已经不是发送节流而是账号锁定(另一套机制、另一套管控);它同时把「误填成毫秒」挡在门外(5 分钟以上的意图都会被拒)。

校验放在配置处,不是首次发送处。 guard 由 AuthManager.getOtpSendGuard() 惰性构造,只在 guard 构造函数里校验的话,一个配置错误会表现为 /phone-number/send-otp 的 500。因此 AuthManager 构造函数(即 AuthPlugin.init())也调用同一个校验函数 —— 一条消息、两个接缝、没有第二份规则。

验收对照

验收项 落实
> 1 小时要么生效、要么明确拒绝 ≤ 24h 真正生效;> 24h 启动即拒。静默截断已消除
默认配置行为不变(有测试证明) DEFAULT config is unchanged: 60s cooldown, 5 per rolling hour, 1h retention —— 不传任何 phoneOtp,断言 60s 冷却、5 条/小时上限、滚动窗生效,且每次写入的 TTL 仍是 3600
方案 1 需有「超过 1 小时后仍被拒」的测试 a 2-hour cooldown STILL rejects after the 1-hour mark —— 冷却设 2 小时,在 59 分 / 61 分(跨过旧的 1 小时边界) / 119 分推进时钟,三次均被拒,并断言 61 分处 retryAfterSeconds ≈ 59 分;直到 121 分才放行

测试

packages/plugins/plugin-auth 全量 29 files / 658 tests 全绿,typecheck 干净。

新增 7 条测试(含跨节点共享存储下的长冷却、TTL 确实等于 7200 而非 3600、超长冷却不收紧每小时上限、超限/毫秒误填/负数/非有限值的拒绝、AuthManager 启动期拒绝)。

反向验证:把 retentionMs 临时改回 HOUR_MS 以模拟修复前的行为,新增测试中的 3 条如期失败(2-hour cooldown STILL rejects…、跨节点那条、TTL 那条),而「默认配置不变」那条在改前改后都通过 —— 说明测试确实锁住了缺陷,且主路径未被误伤。

changeset 等级:patch

按仓内同族做法(#4757 / #3456 / #3712 / #4728「守卫开始按声明值真正生效」均为 patch)。同一 guard 上的 #4790(PR #4806)同样是 patch,且其 changeset 明确列出了新增导出(createLazyCounterStore() / counterStoreFromKv())—— 本 PR 的两个新增导出(MAX_COOLDOWN_SECONDS、assertOtpCooldownSeconds())同为纯增量,不改任何既有签名。

关于「开始拒绝以前被接受的配置」这一行为收紧:被拒的那些值从未按声明工作过(要么被静默截断到 1 小时,要么被 Math.max(0, …) 静默钳成 0 即关闭冷却),因此不存在依赖其旧行为的部署 —— 这是修复的一部分,而非独立的破坏性变更。

范围

packages/spec 零改动;未触碰 packages/lint、skills/**、content/docs/**(含 content/docs/releases/)。content/docs/permissions/authentication.mdx 描述的是默认值(60s + 5/小时),本 PR 未改默认值,无需同步。

🤖 Generated with Claude Code

https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny

Generated by Claude Code

@claude
fix(plugin-auth): retain the OTP send history for as long as the cool…
37f0754 
…down it measures (#4808)

`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
#4790, a different defect; behaviour identical before and after #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.

Co-Authored-By: Claude Opus 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 11:29am

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/m labels Aug 3, 2026
@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.

9 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/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 11:30
@os-zhuang
os-zhuang enabled auto-merge August 3, 2026 11:30
@os-zhuang Claude

os-zhuang commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

复核通过 —— ACCEPT,已标 ready 并送合并队列

这一单最有价值的部分是它纠正了 issue 本身,以及拒绝了 issue 倾向的那个方向并给出了更好的理由。两点都记录在案。

1. issue 只点到了一半 —— 真正截断冷却的是剪枝,不是 TTL

我立单时写的是「历史 TTL 硬编码 1 小时」。dev 核实后发现:

真正截断冷却的是 checkAndRecord 里的剪枝 filter(t => now - t < HOUR_MS) —— 即使 TTL 无限,last 也会在 1 小时处消失。

两处都改了。只修 TTL 会得到一个看起来修好、实则照旧的 PR —— 而且它会通过一条只检查存储过期时间的测试。这正是本仓一直在关的那类「修了个相邻的东西」。

2. 拒绝方向 2 的理由比 issue 里我写的更准

我在 issue 里把方向 2(配置校验期拒绝 > 3600)描述为「诚实、成本最低」。dev 不采纳,理由是:

那个 1 小时根本不属于冷却 —— 它是「每小时上限」的窗口被借给了另一个维度。两个维度合用一个常量才是缺陷本身,而非实现的固有上限;方向 2 会把一条并不存在的限制永久写进公开契约。

这个判断对,我接受。把一个「本来就不该存在的耦合」固化成契约上的能力上限,是用文档掩盖设计问题。

3. 两个窗口被显式分开 —— 这是我准备提的问题,它先答了

放宽保留时长最容易顺带犯的错,是让「每小时 5 次」的计数窗跟着变宽。代码注释里明确写死了:

History is now retained for max(1 hour, cooldownSeconds) … while the hourly cap keeps counting over its own rolling hour (HOUR_MS), so a long cooldown does not quietly make maxPerHour stricter than declared either.

新增 withinHour 把两者彻底拆开。修一个 declared ≠ enforced 时,没有在另一个维度上造出新的。

4. 上限是拒绝,不是又一次截断

MAX_COOLDOWN_SECONDS = 86400,超出即抛。理由写得很干脆:

Not a truncation point — 把静默截断挪到一个更大的数字,只是把 #4808 的缺陷往外推一个数量级。

而且校验从 AuthManager 构造函数(即 AuthPlugin.init())调用,配置错误在启动期暴露,而不是变成首次 /phone-number/send-otp 的 500。一条规则、两个接缝、不复制第二份 —— 这个安排是对的。

顺带一个巧思:24 小时上限同时能抓住把 cooldownSeconds 按毫秒填的常见笔误(300000 → 拒绝)。

5. 反向验证 + 主路径未误伤,两者分开证明了

把 retentionMs 临时改回 HOUR_MS 后 3 条失败(含 a 2-hour cooldown STILL rejects after the 1-hour mark — the defect itself),恢复后全绿。关键是:DEFAULT config is unchanged: 60s cooldown, 5 per rolling hour, 1h retention 这条在改前改后都通过 —— 这正是验收第 2 条要的证据,而不是"我觉得没影响"。

时钟推进测试覆盖 59 / 61 / 119 分钟三点均被拒,并校验 61 分处 retryAfterSeconds ≈ 59 分,直到 121 分才放行。

6. 测试抓到了作者自己

过程中测试发现 dev 用 cooldownSeconds: 60_000 举「毫秒误填」的例子是错的(60000 秒 = 16.7 小时,低于 24 小时上限,不会被拒),已改为 300_000 并同步修正了源码注释里同样错误的算术。测试对作者生效,是它在做本职工作。

核对

Generated by Claude Code

@os-zhuang
os-zhuang added this pull request to the merge queue Aug 3, 2026
Merged via the queue into main with commit 5046afe Aug 3, 2026
21 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-4808-otp-cooldown-ttl branch August 3, 2026 11:37
This was referenced Aug 3, 2026
Closed
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

phoneOtp.cooldownSeconds 配成大于 1 小时会被静默截断 —— OtpSendGuard 的历史 TTL 硬编码 1 小时

2 participants

@os-zhuang @claude