Repository navigation
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
Conversation
…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>
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>
📓 Docs Drift CheckThis PR changes 1 package(s): 15 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # 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
|
|
domain:server#5 · os-steve · session_01ADHkmQKwKFCp1m6Pb6Q8c1 维护者速读
席位意见:与 diff 相符。开放问题两项都取 dev 的方案:内核自带的 degraded 内存 cache 不当共享库(保留有界兜底); 席位核对ACCEPT ·
|
Requested by Steve · project thread
Fixes #29053
Clause-②: no
Metadata-protocol: none
维护者速读
cache服务(值注册、工厂注册都能拿到),注册了真正的 cache 就计在 cache 里。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 acacheservice is registered the counters count in it.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'srateLimit.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 privatecheckAndRecordOnce; the fail-opencatchstays 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.src/kernel-cache.ts, new, internal;src/auth-plugin.tscalls it):resolveKernelCache(ctx)asksctx.getKernel().getServiceAsync('cache'), which answers a value registration and a factory registration alike. On a host with nogetServiceAsync(LiteKernel, which has no service factories either) it asksctx.getService('cache'). NoPluginContextmember is added and nopackages/coreorpackages/specfile changes.ObjectKernelwith no cache plugin injects an in-memorycachethat self-describes asdegraded(ADR-0076 D12, read withreadServiceSelfInfo). 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.resolveKernelCacheanswersundefinedfor it, so the bounded per-process fallback and its one-shot "no shared cache" warning stay in charge. A value withoutget/setanswersundefinedtoo..changeset/29053-auth-counters-per-key.md,@objectstack/plugin-authpatch.Why
better-auth's own
consumecontract 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 agetServiceAsyncmember that the plugin context does not carry, so the optional call answeredundefinedon every host and the counters never reached a registered cache.Tests
src/rate-limit-concurrency.test.ts(new, 9 cases).incrementFixedWindowhands a concurrent burst the counts 1 to N, each exactly once.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.ObjectKernelcontext: a factory-registered cache (and the synchronous accessor cannot answer it), a value, none, the kernel'sdegradedstand-in, a value that is not a counter store.LiteKernelcontext: a value.AuthPlugin) with acacheregistered 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.src/auth-plugin.test.ts. ThebootWithhelper and the host-secondaryStoragecase stubbedgetServiceAsyncon 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 kernelgetKernel()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
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 inpackages/spec, outside this lane; reported to the seat.cache, the kernel'sdegradedstand-in included; that would move every cache-lessObjectKerneldeployment's counters off the bounded fallback for no cross-node gain../rate-limit-storagesubpath 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).auth-plugin.ts'sinvitationPlacementresolver also calls the context's absentgetServiceAsyncfirst. It works through its synchronousgetServicefallback, because plugin-security registers that service as a value. Dead first leg, noted here only.Verification
Final head
7f52bb00(origin/main merged at0d6d4061in259b269f; the last commit rewords one docblock insrc/kernel-cache.tsand changes no code). Every reading below names the head it was taken on.Tests and typecheck
259b269f:pnpm --filter '@objectstack/plugin-auth...' buildunder 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).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-typecheckOK, 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.incrementFixedWindowwithout 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.OtpSendGuard.checkAndRecordwithout the serialiser: the 3 OTP cases red (every send of the burst granted).resolveKernelCache'sObjectKernelleg put back to the context's optionalgetServiceAsync: 7 cases red. Factory and value resolution answerundefined, the composed stack's counter never reaches the registered cache, and the twoauth-plugin.test.tscases that count in a cache registered after init go red.LiteKernelleg removed: theLiteKernelcase red.degradedskip 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/objectstackon7f52bb00derived 65 commands (9 paths, under the 3000-line threshold), the same list as on259b269f. All 65 run with stdin from /dev/null and the exit captured before any pipe: 65 exit 0.259b269f, two of them first answered exit 3 (prerequisite, nothing measured) and read exit 0 once it was met:check-plugin-teardown-shape --self-testafter fetching its pinned fixture commit, andcheck:dual-build-cjs-loadsafterturbo run build --filter='./packages/**'(115 require entry points across 70 packages load). Both were in place for the final-head run.7f52bb00,dispatch-gates --ran: 65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN (a derived zero).eslint --no-inline-config packages/plugins/plugin-auth(thepnpm lintcommand scoped to the touched package) on7f52bb00: exit 0, no findings. The repo-wide run is CI's.test:repois not run locally (no file of that project changed); it is CI'sTest Core.Human Minimum
packages/specholds no runtime logic. (No schema touched.)pnpm lintpasses. (eslint over the touched package; the repo-wide run is CI's.)main, one pull request.packages/spectouched: its generated artifacts are regenerated and committed. (Not touched.)content/docs/references/,packages/*/CHANGELOG.mdorcontent/docs/releases/.Generated by Claude Code