Skip to content

fix(plugin-auth): a rate limit admits no more than its limit inside one process, and the auth counters count in a registered cache service - #29216

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-fk1k2s
Nov 2, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-fk1k2s

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Steve · project thread

Fixes #29053

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:plugin-auth 的限流计数在同一进程内按键串行,同一个键的更新一次只做一个,每次都读到上一次写下的计数,所以固定窗口限额是多少就只放行多少;按手机号的 OTP 发送记录同样如此。计数器改为通过插件上下文真正提供的内核访问器去拿 cache 服务(值注册、工厂注册都能拿到),注册了真正的 cache 就计在 cache 里。
  • 为什么改:按 plugin-auth rate limits over-admit a concurrent burst in ONE process (incrementFixedWindow reads then writes across an await: 20 of 20 admitted), and the shared cache store is never reached because PluginContext has no getServiceAsync #29053 与维护者裁决(「A+C 同意」):限流必须在并发下守住限额,并且要计在已注册的共享 cache 里(ADR-0069 D2)。原来两条都不成立:同进程内两次更新可以交错,cache 解析走的是插件上下文上并不存在的方法,从来没拿到过 cache。
  • 风险与回滚:同一个键的更新会排队(进程内存兜底下是微任务级,远端 cache 下每次要等上一次的读写往返),不同键互不影响。没有 cache 插件时内核自带的 degraded 内存 cache 不当共享库用,计数仍走有界的进程内兜底并照旧告警。跨节点仍没有原子自增(契约在 spec,本 PR 不动)。回滚即撤销本 PR。
  • 先看:packages/plugins/plugin-auth/src/per-key-serial.ts(串行规则本身)、packages/plugins/plugin-auth/src/kernel-cache.ts(cache 解析和 degraded 跳过)、packages/plugins/plugin-auth/src/rate-limit-storage.ts 的 incrementFixedWindow。
  • 两分钟验证:pnpm --filter @objectstack/plugin-auth exec vitest run src/rate-limit-concurrency.test.ts src/kernel-cache.test.ts,预期 2 个测试文件、19 个用例全部通过。
  • 需要你做的:无,席位落地。

What changes

The rule, for every counter in @objectstack/plugin-auth (ADR-0069 D2): inside one process a limit admits no more than its limit however many requests for one key arrive at once, and when a cache service is registered the counters count in it.

  • Per-key serialisation (src/per-key-serial.ts, new, internal, not re-exported): serializePerKey(key, task) runs the updates of one key one at a time, in call order. Other keys run concurrently. A failing update releases the next one, and its error reaches only its own caller. A key has an entry only while an update for it is in flight.
  • incrementFixedWindow (src/rate-limit-storage.ts) runs its read, compute and write through it. That covers every caller of the one fixed-window counter: better-auth's rateLimit.customStorage (createLazyCacheRateLimitStorage), the signed-out invitation preview, cacheSecondaryStorage().increment, and the subpath's external callers. Its docblock now states the in-process rule and keeps the cross-node caveat.
  • OtpSendGuard.checkAndRecord (src/otp-send-guard.ts) runs its read, decide and write per number through the same serialiser. The body moved unchanged into a private checkAndRecordOnce; the fail-open catch stays where it was. This is a declared in-place fix: the same defect class, a reader the card names (the OTP budget), the same file surface and the same test family.
  • Cache resolution (src/kernel-cache.ts, new, internal; src/auth-plugin.ts calls it): resolveKernelCache(ctx) asks ctx.getKernel().getServiceAsync('cache'), which answers a value registration and a factory registration alike. On a host with no getServiceAsync (LiteKernel, which has no service factories either) it asks ctx.getService('cache'). No PluginContext member is added and no packages/core or packages/spec file changes.
  • The kernel's own stand-in is not a shared store. An ObjectKernel with no cache plugin injects an in-memory cache that self-describes as degraded (ADR-0076 D12, read with readServiceSelfInfo). It is as process-local as the package's own fallback, and that fallback (InProcessCounterStore) is the store this package bounds for these counters: it prunes expired keys and caps how many it holds. resolveKernelCache answers undefined for it, so the bounded per-process fallback and its one-shot "no shared cache" warning stay in charge. A value without get/set answers undefined too.
  • Changeset: .changeset/29053-auth-counters-per-key.md, @objectstack/plugin-auth patch.

Why

better-auth's own consume contract says the check and the increment happen in one step, so that simultaneous requests cannot all pass a stale read. ADR-0069 D2 promises one count across nodes when a shared cache is registered. Neither held. In one process nothing ordered two updates of one key across the awaits between the read and the write. And the cache was resolved through a getServiceAsync member that the plugin context does not carry, so the optional call answered undefined on every host and the counters never reached a registered cache.

Tests

  • src/rate-limit-concurrency.test.ts (new, 9 cases).
    • A concurrent burst against one key admits exactly the limit: in the per-process store, and in an async cache stand-in whose calls each cross a macrotask, where the stored count also equals the burst size.
    • incrementFixedWindow hands a concurrent burst the counts 1 to N, each exactly once.
    • A key parked inside its read does not hold up another key.
    • OTP: the cooldown grants exactly one of a burst to one number; the hourly cap grants exactly maxPerHour; the cache path holds the cooldown too.
    • serializePerKey: a failing update releases the next and its error reaches only its caller; no key is left behind once a burst drains.
  • src/kernel-cache.test.ts (new, 10 cases), against real kernels, never a hand-built context.
    • ObjectKernel context: a factory-registered cache (and the synchronous accessor cannot answer it), a value, none, the kernel's degraded stand-in, a value that is not a counter store. LiteKernel context: a value.
    • Composed stack (SQL driver, ObjectQL, AuthPlugin) with a cache registered by factory: the limiter's counter lands in it; a concurrent burst against one key admits exactly the limit while counting there; the OTP budget and the invitation preview resolve the same cache.
    • Composed stack with no cache plugin: the kernel's stand-in is present (the control), and the counters keep the bounded per-process store, still admitting exactly the limit.
  • Forced test edits: src/auth-plugin.test.ts. The bootWith helper and the host-secondaryStorage case stubbed getServiceAsync on the context itself, a member no kernel hands a plugin, which is why the suite passed while production never reached the cache. They now stub it on the kernel getKernel() returns. With the old accessor put back (ablation 3 below), the two cases that count in a cache registered after init go red.

Acceptance notes

  • Across nodes, unchanged. ICacheService (packages/spec/src/contracts/cache-service.ts) has no atomic increment, so two nodes sharing one cache still read-modify-write against each other, as the docblocks state. Adding one is a contract change in packages/spec, outside this lane; reported to the seat.
  • What the rule costs. Updates of one key wait for each other. Against the per-process store that is a microtask each; against a networked cache each update waits for the previous one's read and write. Different keys are unaffected.
  • The stand-in rule is a choice, recorded as an open question in the report. The alternative counts in any registered cache, the kernel's degraded stand-in included; that would move every cache-less ObjectKernel deployment's counters off the bounded fallback for no cross-node gain.
  • One serialiser per bundle. The package ships its root and its ./rate-limit-storage subpath as self-contained bundles (splitting: false), so each carries its own copy of the per-key map. Callers of one key are ordered when they reach the counter through the same entry. Today the two entries count disjoint keys (the auth counters through the root, the SMS day quota through the subpath).
  • Not touched: auth-plugin.ts's invitationPlacement resolver also calls the context's absent getServiceAsync first. It works through its synchronous getService fallback, because plugin-security registers that service as a value. Dead first leg, noted here only.

Verification

Final head 7f52bb00 (origin/main merged at 0d6d4061 in 259b269f; the last commit rewords one docblock in src/kernel-cache.ts and changes no code). Every reading below names the head it was taken on.

Tests and typecheck
  • On 259b269f: pnpm --filter '@objectstack/plugin-auth...' build under the verify lock, exit 0; pnpm --filter @objectstack/plugin-auth test, Test Files 210 passed (210), Tests 3629 passed, 10 skipped (3639); pnpm --filter @objectstack/spec check:generated, all 14 generated artifacts up to date (spec moved on main's side only).
  • On 7f52bb00: the six new and touched test files (rate-limit-concurrency, kernel-cache, auth-plugin, rate-limit-storage, rate-limit-storage-isolation, otp-send-guard), 6 files, 175 tests passed; pnpm --filter @objectstack/plugin-auth typecheck, exit 0 (check:test-typecheck OK, debt ledger held).
Ablations (each through scripts/ablation-replace.mjs, restored to the HEAD blob, git diff HEAD empty)

The suites read the mutated modules from src/ by relative import, so no build sits between mutation and run.

  1. incrementFixedWindow without the serialiser: 5 cases red. The burst cases in the per-process store, in the async cache and on both composed stacks admit the whole burst where they expect the limit; the counts case reads 1 for every increment.
  2. OtpSendGuard.checkAndRecord without the serialiser: the 3 OTP cases red (every send of the burst granted).
  3. resolveKernelCache's ObjectKernel leg put back to the context's optional getServiceAsync: 7 cases red. Factory and value resolution answer undefined, the composed stack's counter never reaches the registered cache, and the two auth-plugin.test.ts cases that count in a cache registered after init go red.
  4. The LiteKernel leg removed: the LiteKernel case red.
  5. The degraded skip removed: 2 cases red (the stand-in unit case, and the composed no-cache stack now counting in the stand-in).
Dispatch gates and lint
  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack on 7f52bb00 derived 65 commands (9 paths, under the 3000-line threshold), the same list as on 259b269f. All 65 run with stdin from /dev/null and the exit captured before any pipe: 65 exit 0.
  • On 259b269f, two of them first answered exit 3 (prerequisite, nothing measured) and read exit 0 once it was met: check-plugin-teardown-shape --self-test after fetching its pinned fixture commit, and check:dual-build-cjs-loads after turbo run build --filter='./packages/**' (115 require entry points across 70 packages load). Both were in place for the final-head run.
  • Reconciliation on 7f52bb00, dispatch-gates --ran: 65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN (a derived zero).
  • eslint --no-inline-config packages/plugins/plugin-auth (the pnpm lint command scoped to the touched package) on 7f52bb00: exit 0, no findings. The repo-wide run is CI's.
  • test:repo is not run locally (no file of that project changed); it is CI's Test Core.
  • CI check-runs on this PR: not read yet (in progress at the time of writing).

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic. (No schema touched.)
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes. (eslint over the touched package; the repo-wide run is CI's.)
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: its generated artifacts are regenerated and committed. (Not touched.)
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English. (The 维护者速读 section is Chinese by the repo's maintainer rule.)
  • The Before Submitting list in CONTRIBUTING.md is done.

Generated by Claude Code

…concurrent burst in one process, and the counters count in a registered cache service

Two rules for the auth counters (ADR-0069 D2):

- Inside one process, updates of one counter key are serialised, so a
  fixed-window limit of max admits exactly max of a concurrent burst
  against one key, in the per-process store and in the kernel cache alike.
  The OTP send history follows the same rule per number.
- When a cache service is registered, as a value or as a factory, the
  counters count in it. It is resolved through the kernel the plugin
  context hands out (getKernel().getServiceAsync), with the synchronous
  getService for a KernelBase-shaped host; the context itself carries no
  getServiceAsync member.

Across nodes the cache contract still has no atomic increment, so the
documented slight over-admit between nodes stands.

Claude-Session: https://claude.ai/code/session_01ADHkmPtGEJ1jqz6awM6fkJ
Co-authored-by: Claude <noreply@anthropic.com>
…n the only cache is the kernel's own degraded stand-in; changeset

An ObjectKernel with no cache plugin injects an in-memory cache that
self-describes as degraded (ADR-0076 D12). It is process-local like the
package's own fallback but expires keys only on read, so counting in it
would trade the fallback's bound for nothing. resolveKernelCache answers
undefined for it, and the no-shared-cache warning stays true.

Tests run against real kernels: ObjectKernel value, factory, none, the
degraded stand-in, LiteKernel; and the composed stack with a factory cache
and with none.

Claude-Session: https://claude.ai/code/session_01ADHkmPtGEJ1jqz6awM6fkJ
Co-authored-by: Claude <noreply@anthropic.com>
…keep

Comment-only.

Claude-Session: https://claude.ai/code/session_01ADHkmPtGEJ1jqz6awM6fkJ
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Nov 2, 2026
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 11 documentable anchor(s).

15 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/deployment/self-hosting.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/deployment/tenancy-modes.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/kernel/architecture.mdx (via getService (literal, a string literal in resolveKernelCache))
  • content/docs/kernel/cluster.mdx (via getKernel (literal, a string literal in resolveKernelCache), getService (literal, a string literal in resolveKernelCache))
  • content/docs/kernel/runtime-services/audit-service.mdx (via getService (literal, a string literal in resolveKernelCache))
  • content/docs/kernel/services-checklist.mdx (via AuthPlugin (symbol, a top-level class), getService (literal, a string literal in resolveKernelCache))
  • content/docs/permissions/authentication.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/permissions/record-view-auditing.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/plugins/anatomy.mdx (via getKernel (literal, a string literal in resolveKernelCache), getService (literal, a string literal in resolveKernelCache))
  • content/docs/plugins/development.mdx (via getService (literal, a string literal in resolveKernelCache))
  • content/docs/plugins/index.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/plugins/packages.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/protocol/kernel/index.mdx (via getService (literal, a string literal in resolveKernelCache))
  • content/docs/upgrading.mdx (via AuthPlugin (symbol, a top-level class))

⛔ 7 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-3.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/releases/v17/17-5.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/releases/v17/17-6.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/releases/v18/18-0.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/releases/v18/18-1.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/releases/v18/18-2.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/releases/v18/index.mdx (via AuthPlugin (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 101 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 14208f2c504ff6f1529be8a28323a91df53d0562 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 1b33be20b4b12a922604732e80232335d208fdc4 — the merge of head 7f52bb00c750ef7c6231ffa5f2bfc9adc213c6b3 into base 14208f2c504ff6f1529be8a28323a91df53d0562, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 1b33be20b4b12a922604732e80232335d208fdc4 && git checkout 1b33be20b4b12a922604732e80232335d208fdc4
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 14208f2c504ff6f1529be8a28323a91df53d0562 7f52bb00c750ef7c6231ffa5f2bfc9adc213c6b3 && git checkout -B drift-repro 14208f2c504ff6f1529be8a28323a91df53d0562 && git merge --no-ff 7f52bb00c750ef7c6231ffa5f2bfc9adc213c6b3

node scripts/docs-audit/affected-docs.mjs --json 14208f2c504ff6f1529be8a28323a91df53d0562

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 14208f2c504ff6f1529be8a28323a91df53d0562 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#5 · os-steve · session_01ADHkmQKwKFCp1m6Pb6Q8c1

维护者速读

  • 改了什么:plugin-auth 的限流计数和短信验证码发送记录在同一进程内按键串行,限额是多少就只放行多少;计数器改用插件真正拿得到的内核访问器取 cache,注册了共享 cache 就计在 cache 里。
  • 为什么改:维护者裁决「A+C 同意」:限流在并发下必须守住限额,并计在已注册的共享 cache 里(ADR-0069 D2);原来两条都不成立。
  • 风险与回滚:同一个键的更新排队,不同键互不影响;没有 cache 插件时仍走有界的进程内兜底。跨节点仍无原子自增(契约在 spec,本 PR 不动)。回滚即撤销本 PR。
  • 先看:per-key-serial.ts(串行规则)、kernel-cache.ts(cache 解析与 degraded 跳过)、rate-limit-storage.ts 的 incrementFixedWindow。
  • 两分钟验证:pnpm --filter @objectstack/plugin-auth exec vitest run src/rate-limit-concurrency.test.ts src/kernel-cache.test.ts(2 个文件 19 例全绿)。
  • 需要你做的:无,席位落地。

席位意见:与 diff 相符。开放问题两项都取 dev 的方案:内核自带的 degraded 内存 cache 不当共享库(保留有界兜底);otp-send-guard.ts 同类就地修,认领文件面已修订。PR 只写规则,不含复现。

席位核对

ACCEPT · domain:server seat 5 · os-steve · session_01ADHkmQKwKFCp1m6Pb6Q8c1 · 2026-11-02T09:43Z · head 7f52bb00 · #29053

  1. Shape: draft against main, first line Fixes #29053, no other card number beside a closing keyword; patch changeset for @objectstack/plugin-auth; Clause-②: no, Metadata-protocol: none. Not governed, +564 / -51, 9 files, all under packages/plugins/plugin-auth/ plus the changeset. No PluginContext member added, no packages/core or packages/spec file changed (Assumption A held).
  2. Against the order: per-key serialisation (serializePerKey, internal, not re-exported) wraps incrementFixedWindow and OtpSendGuard.checkAndRecord; cache resolution goes through ctx.getKernel().getServiceAsync('cache') with a getService leg for hosts without it, and skips a degraded stand-in. Read in the diff. Rule-only wording in the PR body and changeset: no live-door burst recipe.
  3. Accepted deviations: the in-place otp-send-guard.ts fix (same defect class, a reader the card names, same test family), with the claim's file surface revised in the same round. Forced test edit named: auth-plugin.test.ts stubbed getServiceAsync on the context, a member no kernel hands a plugin; it now stubs the kernel.
  4. Gates: dev reports 65/65 derived gates exit 0 on 7f52bb00 (0 NOT-MEASURED), plugin-auth full suite 3629 passed on 259b269f and the six touched files 175/175 plus typecheck on 7f52bb00; five ablations, each red then restored. Check-runs on this head were still running at ACCEPT; the seat readies it only after Lint & Repo Gates and TypeScript Type Check read success and no check is red.
  5. Out of scope: cross-node atomic increment (ICacheService has none; a packages/spec contract) → Acceptance notes; the invitationPlacement resolver's dead first leg → Acceptance notes. Further findings were handed to the seat privately under the maintainer's ruling C.
  6. Landing: pr_ready + automerge_enable through the relay once CI is green.

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 Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

1 participant