feat(auth): 把 better-auth 的真实操作人接到身份表写入上,作为归因 (#4586) - #4656
Merged
Conversation
…r attribution (#4586) better-auth owns every write to the identity tables and its ObjectQL adapter runs them `isSystem: true` on purpose — the route already authorized the action under better-auth's own ACL. The human who clicked *make admin* was known exactly once, in the hook layer, then discarded, so every `trackHistory` transition on `sys_member` recorded "system" as its actor. W1 — a general seam, not a `sys_member` special case: a request-scoped attribution store opened at `AuthManager.handleRequest`, filled lazily from better-auth's global before-hook, surfaced as `ExecutionContext.attributedUserId` → `HookContext.provenance.attributedUserId` and read by the audit writer. W2 — `auto-org-admin-grant` stamps the attributed human into the `granted_by` column it always wrote null into, plus a machine-provenance `reason` naming the writer and the triggering `sys_member` row. W3 — covered at the real routes (invite-accept, update-member-role, the reconciler bind, demotion) in a dogfood test over the live HTTP stack. ATTRIBUTION ONLY: the threaded actor never becomes the authorization subject. It rides `provenance`, which no security middleware reads; `isSystem` stays the unconditional authorization half. Re-authorizing as the human would open the second adjudication track ADR-0095 D3 closed — pinned by tests at the engine seam, the adapter, and the live route. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 5 package(s): 116 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…r attribution (#4586) better-auth owns every write to the identity tables and its ObjectQL adapter runs them `isSystem: true` on purpose — the route already authorized the action under better-auth's own ACL. The human who clicked *make admin* was known exactly once, in the hook layer, then discarded, so every `trackHistory` transition on `sys_member` recorded "system" as its actor. W1 — a general seam, not a `sys_member` special case: a request-scoped attribution store opened at `AuthManager.handleRequest`, filled lazily from better-auth's global before-hook, surfaced as `ExecutionContext.attributedUserId` → `HookContext.provenance.attributedUserId` and read by the audit writer. W2 — `auto-org-admin-grant` stamps the attributed human into the `granted_by` column it always wrote null into, plus a machine-provenance `reason` naming the writer and the triggering `sys_member` row. W3 — covered at the real routes (invite-accept, update-member-role, the reconciler bind, demotion) in a dogfood test over the live HTTP stack. ATTRIBUTION ONLY: the threaded actor never becomes the authorization subject. It rides `provenance`, which no security middleware reads; `isSystem` stays the unconditional authorization half. Re-authorizing as the human would open the second adjudication track ADR-0095 D3 closed — pinned by tests at the engine seam, the adapter, and the live route. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5
…not its position (#4586) The Dogfood Regression Gate failed on this file's own new assertion: expected '{"role":"admin"}' to contain 'member' Not a defect in what `trackHistory` stores — the audit writer's `update` leg writes a CHANGED-FIELDS DIFF, `old_value` the before-state and `new_value` the after-state (`diff()` in `audit-writers.ts` fills both halves), so a demotion really is recorded as `{"role":"admin"}` → `{"role":"member"}`. The audit question "who changed X from member to admin" was already answerable; #4586 adds WHO to a row that already knew WHAT. The defect was the TEST's row SELECTION. It took `rows[rows.length - 1]` as "the newest row", which is wrong twice over: 1. `find` without an explicit sort is unordered, so the last element is not the newest anything; 2. the audit row lands ASYNCHRONOUSLY after the endpoint returns, so at the moment the demotion test polled, the only `update` row present was the PROMOTION from the earlier test — and it asserted the demotion's expectation against it. Locally green, red in CI, because `packages/qa/dogfood` sits outside the `--filter` scope the change was verified under. `waitForHistoryMatching` now waits for the row that says the thing under test, so both hazards are gone. The demotion case additionally pins the pair (`old_value` admin → `new_value` member) and asserts the promotion survives as a DISTINCT row keeping its own actor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5
The Check Changeset gate was red: a user-visible behaviour change (identity-table writes now carry the true actor; `granted_by` / `reason` stop being null) with no `.changeset/*.md`. Minor across the five published packages whose surface moves — `spec` gains an authorable `ExecutionContext.attributedUserId` and the `HookContext.provenance` envelope, so this is additive API, not a pure fix. The body states the hard constraint the reviewer must be able to find later: the threaded actor is attribution ONLY and never the authorization subject. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5
The predecessor session rebased this branch onto a newer `main` locally, which left the pushed head unreachable. Force-pushing is banned (AGENTS.md Prime Directive #3), so the two are joined by a merge instead: the change itself is identical on both sides, only the base differs. # Conflicts: # packages/qa/dogfood/test/membership-actor-attribution.dogfood.test.ts
…or-attribution-seam
os-zhuang
marked this pull request as ready for review
August 2, 2026 14:49
This was referenced Aug 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4586
问题
sys_member本来就是trackHistory: true,角色变更一直有记录 —— 缺的只是「是谁改的」。原因是身份表的每一个写入方都是 better-auth 路径,适配器故意以isSystem: true执行(路由已经在 better-auth 自己的 ACL 下授权过了,而 ADR-0092 D2 干脆拒绝以用户上下文写入这些表)。点下 make admin 的那个人,只在 better-auth 的 hook 层被知道过一次 —— 会话在那里存在 —— 然后就被丢掉了。于是每一条 grade 变更的历史行都写着 "system"。第二跳同理:
auto-org-admin-grant.ts往一个已经存在的granted_by列里无条件写null,所以「X 为什么是 tenant admin」查一行就断了。硬约束(整个改动挂在这上面)
被线程化的 actor 是纯归因,永远不是授权主体。
它走
provenance这条通道 —— 没有任何安全中间件读它;isSystem仍然是无条件的授权那一半。把这个人提升成写入的授权主体,等于在 better-auth 已经裁决过的地方再裁决一次,也就是在 ADR-0095 D3 刚关掉的边界上重新开一条并行裁决轨。属性和授权是两个独立字段,这是刻意的。这条约束在三个层面都有 pin 测试守着:引擎接缝(
engine.test.ts—— 归因只落在provenance,session上不出现,只带归因的上下文按 ADR-0118 D2「absence is never system」等价于匿名)、适配器(auth-actor-attribution.test.ts—— 写入里userId必须是 undefined),以及真实路由(dogfood —— 普通成员依旧无法把自己提成 admin)。实现
W1(正文推荐的通用形状,不是
sys_member特例) ——packages/plugins/plugin-auth/src/auth-actor-attribution.ts:一个请求作用域的AsyncLocalStorage,在AuthManager.handleRequest打开,由 better-auth 的全局hooks.before(matcher: () => true,覆盖包括 organization 插件在内的每一个端点)惰性填入一个 resolver —— 存起来,不执行。只有真正需要归因的写入才会去 await,且每请求最多解析一次并记忆化。所以只读的 auth 请求一分钱不花,写请求只付一次会话解析。链路:
ExecutionContext.attributedUserId→HookContext.provenance.attributedUserId→ 审计写入器落到sys_audit_log.user_id。会话主体存在时仍然优先 —— 归因不覆盖真正的行为人。/auth/admin/create-user和/auth/admin/import-users走服务端调用、不经过handleRequest,用runAttributedToUser以已在手的 actor 显式开作用域。W2 —— 自动授权行现在记下完整来源:
granted_by落归因到的人(没有人在场时是null,不是哨兵字符串 —— ADR-0118 D1 要求这个sys_userlookup 列只能是真 id 或 null),机器标记和触发它的那条sys_member行落在reason(自由文本该待的地方)。W3 ——
packages/qa/dogfood/test/membership-actor-attribution.dogfood.test.ts驱动真实 HTTP 栈上的真实端点:invite-member→accept-invitation、update-member-role一升一降,然后读平台实际写下的行。这是 #3106 的教训:一个孤立可用的函数,不能证明有任何路由真的经过它。无作用域(boot sync、迁移、定时任务)解析为
undefined,记为null—— 缺席永远不会被升格成一个调用者。边界
正文「Deliberately NOT doing」一节照办:
sys_member上的 ADR-0091 有效期窗口、临时成员资格,一律没碰;resolve-authz-context.ts的 projection union 也没动。顺带发现(未在本 PR 修)
见 dogfood 文件里降级用例上的 NOTE:
auto-org-admin-grant唯一的 delete 通道调用签名与引擎不符,导致每次 revoke 都抛进一个吞异常的 catch(它的单测桩实现了错误的签名,所以套件一直没看见)。那是既有缺陷、属于安全行为变更,值得独立评审和 changeset,不该搭归因 PR 的便车。已单独立为 #4640。CI 修复(接手上一轮容器中断后)
1. Dogfood Regression Gate 红 —— 是测试选错了行,不是实现半成品
失败断言:
先回答该列到底记什么,因为这决定了该改哪一边:
sys_audit_log的update分支存的是变更字段的 diff,前后两半都在(audit-writers.ts的diff():某字段前后不等时,oldOut[k]收旧值、newOut[k]收新值)。所以一次降级真实记为old_value = {"role":"admin"}→new_value = {"role":"member"}。(create腿:old_value = null,new_value是整条新记录;delete腿反之。)也就是说,「谁把 X 从 member 改成了 admin」这个审计问题,WHAT 那一半本来就是完整可答的 —— #4586 补的是 WHO。实现不欠账,不需要再去补 before/after 对。
真正的缺陷在测试的取行方式:它用
rows[rows.length - 1]当「最新一行」,这里错了两次 ——find不带显式 sort 是无序的,最后一个元素不是最新的任何东西;update行是上一个用例留下的升级行 —— 于是拿降级的期望去断言了升级的行。修法是按行自己说了什么来认行(
waitForHistoryMatching),两个隐患一起消失;顺带把这对前后值(old_valueadmin →new_valuemember)显式 pin 住,并断言升级行作为另一行存活、保留它自己的 actor。2. Check Changeset 红
补
.changeset/actor-attribution-seam.md。5 个发布包minor——spec新增了可编写的ExecutionContext.attributedUserId与HookContext.provenance,属新增 API,不是纯修复;不用major(全仓 fixed 组,check-changeset-no-major.mjs已本地跑过)。changeset 正文把硬约束(归因永不成为授权主体)写进了会随 npm 包发出去的 CHANGELOG,以免下一个人只看 diff 就把它拆了。3. 分支与 main 的对账
上一轮会话在本地把分支 rebase 到了更新的
main,导致已推送的 head 不可达。force-push 是禁止的(AGENTS.md Prime Directive #3),所以用 merge 把两边接上(改动内容两边一致,只有 base 不同),再 merge 当前origin/main。git diff origin/main...HEAD仍然只有本 PR 的 24 个文件。Generated by Claude Code