Repository navigation
perf(core,plugin-auth): an authenticated request resolves its caller's grants once, not twice - #22441
Conversation
…sion principal's grants once The session read inside resolveAuthzContext runs plugin-auth's customSession hook, which resolves the principal's grants for the payload's positions[]; the resolver then resolved the same grants again with the same arguments. A request-scoped memo (AsyncLocalStorage, closed when the call settles) now serves the second resolution the first one's envelope, keyed by user, tenant and seeds, guarded by the engine write epoch and the next validity boundary. The hook passes the session email as its seed so both calls ask for the same resolution. Claude-Session: https://claude.ai/code/session_01WVbr5J6u8BHh8EyFtcWciH Co-authored-by: Claude <noreply@anthropic.com>
…count, isolation and freshness Claude-Session: https://claude.ai/code/session_01WVbr5J6u8BHh8EyFtcWciH Co-authored-by: Claude <noreply@anthropic.com>
…t resolve grants once per warm request Claude-Session: https://claude.ai/code/session_01WVbr5J6u8BHh8EyFtcWciH Co-authored-by: Claude <noreply@anthropic.com>
…rants memo Claude-Session: https://claude.ai/code/session_01WVbr5J6u8BHh8EyFtcWciH Co-authored-by: Claude <noreply@anthropic.com>
…yped user Claude-Session: https://claude.ai/code/session_01WVbr5J6u8BHh8EyFtcWciH Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 17 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 10 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 34 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 76575da92df0c1f35ccc87c6535072aed23e7ba7 && git checkout 76575da92df0c1f35ccc87c6535072aed23e7ba7
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 440bed63e731117bc194166fea3eec3d923ad2c6 da29c616426e02fb511a44573acc46dffe5987b1 && git checkout -B drift-repro 440bed63e731117bc194166fea3eec3d923ad2c6 && git merge --no-ff da29c616426e02fb511a44573acc46dffe5987b1
node scripts/docs-audit/affected-docs.mjs --json 440bed63e731117bc194166fea3eec3d923ad2c6
|
Contract reviewServed-tier: Isolated review at the contract-review tier, owed by the cross-lane rule (the claiming seat ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: FAIL |
…flight since the first resolution opened The engine bumps its write epoch when a write STARTS, so an epoch-only guard served an envelope read while an already-bumped revocation was in flight, after that revocation landed. The memo now also registers a write observer on the engine (a middleware counting writes into the chain and, in a finally around next(), out of it) and serves only when the epoch and both counters read what they read at the open with nothing in flight. The scope that registers an engine's observer stores nothing for it. Pins: bump-then-deferred-landing, in flight at step 2, two engines, a nested resolveAuthzContext, an API-key caller class, the positive clock window. Claude-Session: https://claude.ai/code/session_01WVbr5J6u8BHh8EyFtcWciH Co-authored-by: Claude <noreply@anthropic.com>
…e that carries the write epoch Claude-Session: https://claude.ai/code/session_01WVbr5J6u8BHh8EyFtcWciH Co-authored-by: Claude <noreply@anthropic.com>
… the read count Claude-Session: https://claude.ai/code/session_01WVbr5J6u8BHh8EyFtcWciH Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Round 2 of the isolated review at the contract-review tier, owed by the cross-lane rule (the claiming seat ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS |
…commit visibility; state the transactional residual Comment and changeset text only. A write inside engine.transaction() is executed through the observer but becomes visible at the driver COMMIT, outside every middleware chain: on driver-sql, a request whose step 2 falls in that one commit round trip is authorised as of its first resolution and the next request reads fresh, the same answer as an out-of-process write. Claude-Session: https://claude.ai/code/session_01WVbr5J6u8BHh8EyFtcWciH Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Round 3 of the isolated review at the contract-review tier: a delta check of ① Derived judgments
② Semver levelUnchanged: ③ Boundary flags
Implemented-by: VERDICT: PASS |
|
CI note from the seat driving this PR ( Failing check: The failing test: Not this PR's, for three reasons:
What I'm doing:
Generated by Claude Code |
Refs objectstack-ai/cloud#2634 (item 2: the pre-handler lever)
Clause-②: no
What this changes
An authenticated request that resolves identity through
resolveAuthzContextwith a better-auth session read resolved the caller's grants twice, with the same arguments and no write in between:resolveAuthzContextcalls the transport'sgetSession; against plugin-auth that is better-auth'sgetSession, whosecustomSessionhook resolves the grants for the payload'spositions[]/isPlatformAdmin(packages/plugins/plugin-auth/src/auth-manager.ts:4079on main).resolveAuthzContextthen callsresolveUserAuthzGrantsagain for the request envelope (packages/core/src/security/resolve-authz-context.ts:428on main).Both call sites were confirmed on an instrumented request (async stacks below), not only by reading. Each resolution is 8 tenant-DB reads for a caller in an organization:
sys_user,sys_member(own and peers),sys_user_position,sys_user_permission_set,sys_position,sys_position_permission_setandsys_permission_set.The fix is a request-scoped grants memo (
packages/core/src/security/request-grants-memo.ts):resolveAuthzContextruns its whole body, thegetSessioncall included, inside anAsyncLocalStoragescope. The scope is closed when the call settles. No envelope survives the request, and a continuation the request started reads afresh once it has settled.resolveUserAuthzGrantsconsults the scope first. A completed resolution with the same arguments (user, tenant, seed email, seed permissions, spelled as the cross-request cache spells its key, with the engine as the outer key) is served as a clone. It is served only while a fresh read would agree with it:No write has started, been executed at the driver, or is in flight on the engine since that resolution opened. Two signals are read at the open (before its first read) and again at the lookup:
finallyaroundnext(), each write whose driver step has settled. The driver step runs inside thatnext(), so a write cannot be executed at the driver without moving the counters.An entry is served only when the epoch and both counters read what they read at the open and nothing is inside the observer. The observer sees statement execution, not commit visibility: a write inside an
engine.transaction()becomes visible at the driver COMMIT, outside every chain (see Acceptance notes).The reading clock lies in
[resolvedAt, nextValidityBoundary).registerMiddleware, abypassGrantsCachecaller, a resolution that threw (never stored), and any call outside aresolveAuthzContextscope. The scope that registers an engine's observer stores nothing for that engine, so that request reads twice. An engine without the epoch seam gets no observer at all.seedEmail, spelled exactly asresolveAuthzContextspells it for the same session, so the two calls ask for the same resolution. That seed reaches only the envelope'semail, which the hook does not read. The hook'spositions[]/isPlatformAdminare unchanged, and platform-admin standing still compares the storedsys_user.email, never a seed (core §6b-config).Why a memo-side observer, not a second engine-side bump.
writeagain in afinallyafterexecutor(). That changes the pinned one-bump-per-write seam:objectql/src/write-epoch.test.tspins "insert, update and delete each advance it exactly once". It would also double the clusterauthz.invalidatedhints, becauseauthz-invalidation-bridge.tspublishes every non-remote bump. And it would still serve an entry while a write had committed at the driver but not yet settled.started === completedcheck covers that last window. Every otherwriteEpochreader is untouched.Where this landed, and why here. The card's suggested file surface was the downstream agent route. Measurement put both resolutions in this repo: the dispatcher's
resolveExecutionContext→ coreresolveAuthzContext→ plugin-auth's hook, all before any route handler runs. Any downstream-side change would have been a workaround over a framework duplicate. So the fix sits at the producer, and the downstream repo gets it with its next framework pin bump.Other doors. Every caller of
resolveAuthzContextwith a better-authgetSessiongets the same saving through the same function. That covers the runtime dispatcher (agent chat, Ask, data routes, metadata, everything behindresolveRequestScope), the REST server, the settings, storage, datasource-admin, sharing and marketplace-install routes, and the downstream env-settings routes.Where the saving does not apply (the request reads twice, as before; always the safe direction):
enforceSessionControlsstampssys_session.last_activity_atabout once a minute per session;resolveAuthzContextcall on an engine.Equivalence: the same decision, per caller class
The security floor: the grant set a request is authorised with must be the same decision as before, the dedupe is scoped to one request, and no check is skipped or loosened.
resolve-authz-context.request-grants-memo.test.ts,CALLER_CLASSESat :285): platform administrator via the unscopedadmin_full_accessgrant (single posture) and via the declared administrator email (isolated posture), organization owner, admin and member, a non-member whose claimed organization is dropped (walled) or stands (single), an API-key principal with scopes and a stamped organization (x-api-key), and an anonymous request. For each class, the envelope a request resolves with the memo serving step 2 is deep-equal to the envelope step 2 resolves on its own, and the read multiset equals one resolution's. Each class also asserts its own expected posture, positions or scopes, so two equally wrong envelopes cannot pass.session-grants-resolved-once.test.ts): a real better-authgetSessionover the shared memory engine double, which the test drives through the write epoch and a middleware chain the way the engine runs them. Org member, owner, platform operator, removed member with a stale claim (dropped exactly as before), and anonymous each resolve to an envelope deep-equal to the same request with the memo declined (epoch seam removed, which is the pre-memo path), with one resolution's grant reads instead of two.resolveAuthzContextserves nothing to the outer step 2.Measurement: round trips before the handler, hosted composition
Rig: the downstream hosted HTTP composition (artifact kernel factory with the hosted forced requires, kernel manager, cloud kernel resolver, the objectos host slate, REST and dispatcher over Hono). The tenant driver is a
TursoDriveron the remote face, over a@libsql/clientwrapped so that everyexecute/batch/transactionop counts as one round trip. The handler entry is a wrapper on the env kernel's agent-chat route; the model is a memory adapter. The framework is at the downstream pin56bf27af, unpatched for before, with this PR's code files at870b4297applied for after; the downstream checkout isb7d13034. Every request isPOST /api/v1/ai/agents/build/chat, stream on, status 200.sys_job_queue, background)sys_user3,sys_member4, and 2 each for the five grant-only tables.sys_user2,sys_member2, and 1 each for the five grant-only tables.ai_messages1,sys_session2,sys_jwks2,sys_setting1.ObjectQLengine took the observer throughregisterMiddleware, and the warm path met no concurrent write.sys_user_positionwas read twice. The first read came fromtryFind←resolveUserAuthzGrants←auth-manager.ts:4079← better-authcustom-session←resolve-execution-context.ts:160(getSession) ←resolve-authz-context.ts:401(resolveAuthzContext). The second came fromtryFind←resolveUserAuthzGrants←resolve-authz-context.ts:428, with the same dispatcher frames below (http-dispatcher.ts:1008/:619/:2640).56bf27af.Tests
Final head
da29c616(round 2 changed comment and changeset text only; the code and tests are those of870b4297).@objectstack/coretypecheckexit 0; the test layer holds its debt at 4 files / 4 errors, unchanged;test:repo: 3 files, 48 passed.@objectstack/plugin-authtypecheckexit 0; debt 10 files / 94 errors, unchanged;resolveAuthzContext(at606c3340, whose code equals the head's;870b4297reorders assertions in one core test only):@objectstack/runtimefull suite: 346 files, 5,579 passed, 19 skipped;resolveAuthzContext/resolveExecutionContext: plugin-security 2 files, 126 passed; plugin-sharing 1, 28; service-datasource 1, 29; service-storage 1, 26; rest 7 files, 216 passed.scripts/ablation-replace.mjsin wrap mode against the committed memo file (HEAD blob9e08f71d). Every leg's anchor went 1 → 0, the blob changed, the restore blob equals the HEAD blob, andgit diff HEADwas empty. Over the 26 memo pins:0b07e024: 2 red / 24 green. :589 reds withexpected [ 'org_member', 'auditor', … ] to not include 'auditor': the request is authorised with the revoked position, which is the review's interleaving. :622 reds withexpected 8 to be 16. With the guard: green.node scripts/pm/dispatch-gates.mjs --commandsderived 68 families at870b4297. All 68 exited 0, recorded per command with its exit code and reconciled with--ran:68 run, 0 NOT-MEASURED (a DERIVED zero).check:dual-build-cjs-loadsfirst exited 3 (nodist/yet in this worktree). It exited 0 aftercheck:type-check-debt's re-measure had built the packages.origin/mainda159f74, withci.yml,partition-test-shards.mjsandsdui-manifest.record.jsonchanged there. No upstream commit touchescore,plugin-authorobjectql.da29c616.870b4297touches onlyrequest-grants-memo.ts(+17/−2, every added and removed line inside the module's JSDoc block) and the changeset (1 line). The two versions of the.tsfile produce byte-identical comment-strippedtranspileModuleoutput (sha256 prefix9c90b60a9eba1fddboth).@objectstack/coretypecheck exit 0; the memo pin, 26 passed.check:nul-bytes,check-comment-mask-adoption(and its--self-test),check-comment-mask-corpus(8,517 files, 0 disagree),check-changeset-no-major,check-empty-changeset,check-adr-0087-registration,check:doc-authoringandcheck:issue-citations.Acceptance notes
What the observer cannot see, stated in the module doc. The engine snapshots its middleware list when a write starts, so a write that began before the observer was registered never passes it. The registering request stores nothing, which leaves one case open: a write that began before an engine's first
resolveAuthzContext, still in flight when a later request's resolution opens, landing before that request's step 2. Writes from another process are read as of the first resolution, a few milliseconds earlier than the second used to read them. Closing the first case needs the engine to report in-flight writes, which is a public-surface change toWriteEpochand out of this patch.Transactional COMMIT, stated in the module doc. A grant-table write executed on an
engine.transaction()passes the observer per statement, so the counters move and balance. Its rows become visible to other connections only at the driver COMMIT, which runs outside every middleware chain. If that COMMIT lands between the first resolution and step 2 (one commit round trip after the transaction's last statement), step 2 is served the pre-commit envelope./batch.transactionsUnsupportedand opens no transaction.Other duplicates. A few session reads still resolve grants outside
resolveAuthzContext, so this memo does not reach them; this is a code reading, not measured on the agent-chat path:HttpDispatcher.enforceAuthGatere-reads the session when an auth-gate feature is active;enforceProjectMembershipre-reads it when membership enforcement is on;plugin-hono-server/src/current-user-endpoints.ts:430) reads a session and then resolves grants with different seeds.None of them ran on the measured hosted request. No owner.
Read twice in the handler. The localization setting is still read twice per AI request, once by the execution context and once by the agent route's turn time zone. That is inside the handler and already on the downstream card. No owner here.
Generated by Claude Code