Repository navigation
Commit 9038187
fix(audit): name the impersonating administrator on identity writes and on an approval decided while impersonating (#30874)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} -->
_Requested by **Elon** · [project
thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DASH6Wenq1awyqAanJTCYpU)_
Fixes #30851
Clause-②: yes (widening)
Metadata-protocol: none
domain:server#5 card thread · session `session_016WV4czA9H21XfNGbug1dac`
· branch `claude/project-thread-xv0lyn` (named in the seat's re-posted
claim on the card, with the widened file surface). Branched from `main`
at 677d542; `origin/main` merged at e603385. Item 2 adds one optional
member to a spec-owned contract (`ResumeSignal.impersonatedBy`): the dev
stopped on it as the dispatch required, and the seat chose route A and
one PR for both items.
## 维护者速读
- 改了什么:管理员代登录(模拟)某用户时,两类写入现在也记下真正操作的管理员:① plugin-auth 代 better-auth
写身份表(改名、改邮箱、成员变更、管理员路由如设置上级)时,上下文带上 `impersonatedBy`,审计行
`metadata.impersonated_by` 写管理员;②
审批决定后,流程里决定人那一段只带「这次决定」时的代登录管理员,本人决定则不带。
- 为什么改:#30213(PR
#30800)已给记录写入打上代登录标记,契约复核留下两处缺口:身份表写入审计行与用户本人操作无法区分;审批恢复时决定人那段要么沿用触发时的旧标记(本人决定也带),要么代登录决定时完全不带。
- 风险与回滚:只加归属信息,`user_id`、权限、鉴权路径一概不动;spec 只给 `ResumeSignal`
加一个可选成员,且只有带服务标记(HTTP 请求体伪造不了)时才生效;回滚即 revert,无迁移。
- 先看:`packages/services/service-automation/src/run-resume.ts` 的
`deciderSegmentContext`;`packages/plugins/plugin-auth/src/auth-actor-attribution.ts`
的 `authSystemWriteContext` /
`resolveAttributedActor`;`packages/plugins/plugin-approvals/src/approval-service.ts`
决定入口取 `impersonatorOf(context)` 那几行。
- 两分钟验证:`pnpm --filter @objectstack/service-automation exec vitest run
src/resume-decider-identity.integration.test.ts`,预期 32 条全绿,其中「the
decider's segment names exactly the DECISION's impersonating
administrator」一组覆盖本人决定、代登录决定、普通决定三种情况。
- 需要你做的:无,席位落地(本 PR 触 `packages/spec/src/**`,席位 ACCEPT 前需契约复核)。
## What changes
**1. plugin-auth identity-table writes name the impersonating
administrator.** An administrator who impersonates a user and renames
them (or makes any other write better-auth performs on the user's
behalf) produced a `sys_user` write credited to the user through
`attributedUserId` and nothing else, so its audit row read as the user's
own. Now:
- `resolveActor` (`auth-manager-actor-guards.ts`) also answers the
session's `impersonatedBy`, read from the session that
`getSessionFromCtx` resolves or from the session row on the token
fallback. Never from the request body.
- The global before-hook hands the attribution scope `{ userId,
impersonatedBy }`. `AuthActorResolver` may now answer that object (a
bare string still means the user alone). The new export
`resolveAttributedActor()` answers the user and the administrator
together, and `resolveAttributedUserId()` is now a projection of it.
- `authSystemWriteContext()` adds `impersonatedBy` beside
`attributedUserId` (attribution only; `isSystem: true` is untouched and
no `userId` appears). The adapter's `withSystemContext` now builds its
write context with that same function instead of a second copy of it, so
the adapter, the membership reconciler and `set-user-manager` all carry
the key.
- The platform-admin routes that drive better-auth programmatically
(`gateAdmin` → `runAttributedToUser`) pass the admitted actor, which now
carries the gated session's `impersonatedBy`
(`PlatformAdminActor.impersonatedBy`, optional, set by the route's gate
and never by `judgePlatformAdmin`).
- The engine's `buildProvenance` already copies
`ExecutionContext.impersonatedBy` onto `HookContext.provenance`, and the
audit writer already stamps `sys_audit_log.metadata.impersonated_by`
from it (PR #30800). The key is the one `auth-session-audit.ts` and
#30800 use.
**2. The approval decider's segment carries exactly the decision's
administrator.**
- `ResumeSignal.impersonatedBy` (new, optional,
`packages/spec/src/contracts/automation-service.ts`). It is honoured
only beside `resumedBy`, on a signal that carries
`RESUME_AUTHORITY_SERVICE`, exactly as `resumedBy` is. The
`AutomationContext.impersonatedBy` docblock now says what a decision
does to it.
- The decision door (`ApprovalService`'s decide path,
`approval-service.ts`) fills it from the decision request's
`ExecutionContext.impersonatedBy` (`impersonatorOf(context)`), only when
there is a decider. A stranded decision's journal
(`StrandedContinuationSignal`, `__strandedContinuation`) keeps it beside
`resumedBy`, and `continueRestoredRun` replays it.
- `deciderSegmentContext` (`run-resume.ts`) drops the trigger-time stamp
on every human decider's segment and sets the decision's value. That
covers (a) the `context.userId === decider` early return, which used to
keep the trigger-time stamp, and (b) an impersonated decider, who used
to get none. A signal with no decider (an SLA auto-decision) still keeps
the run's identity and its stamp. The engine's
`resolveDecisionSegmentContext` docblock says so.
One changeset, all four packages `patch`.
## Tests
- `service-automation/src/resume-decider-identity.integration.test.ts`:
new describe "the decider's segment names exactly the DECISION's
impersonating administrator, never the trigger's". It covers (a) a
trigger-time stamp with the trigger user deciding in their own name →
none, on both `runAs` arms; (b) an impersonated decider → the decision's
administrator, on both arms, and the trigger user deciding while
impersonated on an unstamped run; (c) a plain decider → none, plus a
second plain decision dropping the first decision's stamp. CONTROLs: a
signal without the service marker cannot name an administrator, and an
administrator with no decider changes nothing.
- `plugin-approvals/src/restored-approval-continuation.test.ts` (ruling
A describe): approve and reject while impersonated carry
`impersonatedBy: 'adm'` beside the decider; an administrator named in
the decision INPUT stamps nothing; a stranded impersonated decision
journals and replays it. The existing plain-decider cases now also
assert the key's absence (`toEqual`).
- `plugin-auth/src/auth-actor-attribution.test.ts`: the write context
carries the administrator; CONTROLs for a plain session, `null` and a
blank value; an administrator with no user attributes nothing;
`withSystemContext` stamps writes and not reads; `runAttributedToUser`
with the actor object.
- `plugin-auth/src/admin-impersonate-endpoint.test.ts` (the
impersonation harness): on the real better-auth pipeline, the
`/update-user` rename made from the impersonation session reaches the
engine with `{ isSystem: true, attributedUserId: target, impersonatedBy:
admin }`, and a body key `impersonatedBy` is ignored. CONTROL: the
administrator's own rename carries no key.
<details><summary>Ablations (each red, each restored;
<code>scripts/ablation-replace.mjs</code> WRAP mode, <code>git diff
HEAD</code> empty after each)</summary>
| # | Mutation | Test run | Red |
|:--|:--|:--|:--|
| A1 | before-hook resolver answers the user id only (`main`'s shape) |
`admin-impersonate-endpoint.test.ts` | 1 of 16: the impersonated rename
|
| A2 | `authSystemWriteContext` drops the `impersonatedBy` spread |
`auth-actor-attribution.test.ts` + `admin-impersonate-endpoint.test.ts`
| 4 of 34 |
| A3 | self-decider path returns the persisted context (`main`'s early
return) | `resume-decider-identity.integration.test.ts` | 3 of 32: (a)
on both arms, and the trigger user deciding while impersonated |
| A4 | the switched segment never sets the decision's value | same | 3
of 32: (b) on both arms, and the second-decision case |
| A5 | the decide door does not pass `impersonatedBy` |
`restored-approval-continuation.test.ts` | 3 of 24: approve, reject,
stranded replay |
| A6 | the journal drops `impersonatedBy` | same | 1 of 24: the stranded
replay |
Every ablated module is imported from `src/` by its test, so no dist
rebuild sits between the mutation and the run.
</details>
## Acceptance notes
- **Reproduction.** Ablation A1 is the reproduction. It turns the
before-hook resolver back to its `main` shape (user id only), and the
real-pipeline pin then fails: the impersonated rename reaches the engine
with no `impersonatedBy`.
- **Not done here:** the `sys_comment` fork (a separate maintainer
decision on #30213, options A/B/C). The ADR-0043 action-link decision
path authenticates by token and has no session, so it carries no
administrator, by construction.
- **Overlap:** open PR #30717 also edits `auth-manager.ts` and
`auth-plugin.ts`, in different hunks.
## Verification
All readings were taken on head `b5edf8ea`, or on `8e621657` for the
gate sweep. The one commit after `8e621657` edits only
`auth-actor-attribution.test.ts`, and `check:query-options-erasure` plus
eslint on that file were re-run on `b5edf8ea`.
<details><summary>Builds, typecheck, package tests</summary>
- Whole-tree `turbo run build --concurrency=2`: 119/119 tasks, exit 0.
- `pnpm --filter @objectstack/spec --filter @objectstack/plugin-auth
--filter @objectstack/plugin-approvals --filter
@objectstack/service-automation run typecheck`: exit 0.
- `pnpm --filter @objectstack/spec check:generated`: 13 of 14 artifacts
current. `check:test-typecheck` failed inside the wrapper with "tsc
failed without diagnostics" under the default heap. Run alone with
`--max-old-space-size=6144` it is OK (52 files, 240 errors, 131 pinned
signatures held). No artifact moved: `ResumeSignal` is a TS interface
and appears in no emitted snapshot.
- `pnpm test` per package: plugin-auth 228 files, 4096 passed, 10
skipped; service-automation 264 files, 3429 passed; plugin-approvals 115
files, 1502 passed.
</details>
<details><summary>Gates (dispatch-gates derivation, 88 families + 3
named)</summary>
- `node scripts/pm/dispatch-gates.mjs --ran`: 88 derived, 88 run, 0
NOT-MEASURED, 0 UNRUN.
- The one red in the sweep was `check:query-options-erasure`: the test
surface went 236 → 237 from an `as any` on a `find` call in the new
attribution pin. The cast is gone in `b5edf8ea`, and the gate now exits
0 at the 236 ceiling. Its "approval-service.ts erasure count fell 10 →
4" line is the same on `main` (both trees hold 52 `as any`), so it is
not this diff.
- `pnpm lint` exit 0 · `pnpm check:cli-command-ids` exit 0 · `pnpm
check:adr-symbol-anchors` exit 0.
- Exit-code file: `gates-ran-b5edf8ea.txt` (seat project folder).
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_016WV4czA9H21XfNGbug1dac)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 8b9cc71 commit 9038187
16 files changed
Lines changed: 484 additions & 48 deletions
File tree
- .changeset
- packages
- plugins
- plugin-approvals/src
- plugin-auth/src
- services/service-automation/src
- spec/src/contracts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
206 | 206 | | |
207 | 207 | | |
208 | 208 | | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
209 | 217 | | |
210 | 218 | | |
211 | 219 | | |
| |||
960 | 968 | | |
961 | 969 | | |
962 | 970 | | |
| 971 | + | |
| 972 | + | |
| 973 | + | |
| 974 | + | |
| 975 | + | |
| 976 | + | |
| 977 | + | |
963 | 978 | | |
964 | 979 | | |
965 | 980 | | |
| |||
1110 | 1125 | | |
1111 | 1126 | | |
1112 | 1127 | | |
| 1128 | + | |
| 1129 | + | |
| 1130 | + | |
| 1131 | + | |
| 1132 | + | |
| 1133 | + | |
| 1134 | + | |
| 1135 | + | |
| 1136 | + | |
| 1137 | + | |
| 1138 | + | |
| 1139 | + | |
1113 | 1140 | | |
1114 | 1141 | | |
1115 | 1142 | | |
| |||
5268 | 5295 | | |
5269 | 5296 | | |
5270 | 5297 | | |
5271 | | - | |
| 5298 | + | |
5272 | 5299 | | |
5273 | 5300 | | |
5274 | 5301 | | |
| |||
5485 | 5512 | | |
5486 | 5513 | | |
5487 | 5514 | | |
5488 | | - | |
| 5515 | + | |
5489 | 5516 | | |
5490 | 5517 | | |
5491 | 5518 | | |
| |||
5546 | 5573 | | |
5547 | 5574 | | |
5548 | 5575 | | |
| 5576 | + | |
5549 | 5577 | | |
5550 | 5578 | | |
5551 | 5579 | | |
| |||
5618 | 5646 | | |
5619 | 5647 | | |
5620 | 5648 | | |
| 5649 | + | |
| 5650 | + | |
| 5651 | + | |
| 5652 | + | |
| 5653 | + | |
5621 | 5654 | | |
5622 | 5655 | | |
5623 | 5656 | | |
| |||
5629 | 5662 | | |
5630 | 5663 | | |
5631 | 5664 | | |
| 5665 | + | |
5632 | 5666 | | |
5633 | 5667 | | |
5634 | 5668 | | |
| |||
8025 | 8059 | | |
8026 | 8060 | | |
8027 | 8061 | | |
| 8062 | + | |
| 8063 | + | |
| 8064 | + | |
| 8065 | + | |
| 8066 | + | |
8028 | 8067 | | |
8029 | 8068 | | |
8030 | 8069 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
948 | 948 | | |
949 | 949 | | |
950 | 950 | | |
951 | | - | |
| 951 | + | |
952 | 952 | | |
953 | 953 | | |
954 | 954 | | |
| |||
963 | 963 | | |
964 | 964 | | |
965 | 965 | | |
966 | | - | |
| 966 | + | |
| 967 | + | |
| 968 | + | |
| 969 | + | |
| 970 | + | |
| 971 | + | |
967 | 972 | | |
968 | 973 | | |
969 | 974 | | |
| |||
1034 | 1039 | | |
1035 | 1040 | | |
1036 | 1041 | | |
| 1042 | + | |
| 1043 | + | |
| 1044 | + | |
| 1045 | + | |
| 1046 | + | |
| 1047 | + | |
| 1048 | + | |
| 1049 | + | |
| 1050 | + | |
| 1051 | + | |
| 1052 | + | |
| 1053 | + | |
| 1054 | + | |
| 1055 | + | |
| 1056 | + | |
| 1057 | + | |
| 1058 | + | |
| 1059 | + | |
| 1060 | + | |
| 1061 | + | |
| 1062 | + | |
| 1063 | + | |
| 1064 | + | |
| 1065 | + | |
| 1066 | + | |
| 1067 | + | |
| 1068 | + | |
| 1069 | + | |
| 1070 | + | |
| 1071 | + | |
| 1072 | + | |
| 1073 | + | |
| 1074 | + | |
| 1075 | + | |
| 1076 | + | |
| 1077 | + | |
| 1078 | + | |
| 1079 | + | |
| 1080 | + | |
1037 | 1081 | | |
1038 | 1082 | | |
1039 | 1083 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
284 | 284 | | |
285 | 285 | | |
286 | 286 | | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
287 | 339 | | |
288 | 340 | | |
289 | 341 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
| 19 | + | |
19 | 20 | | |
20 | 21 | | |
| 22 | + | |
21 | 23 | | |
22 | 24 | | |
23 | 25 | | |
| |||
204 | 206 | | |
205 | 207 | | |
206 | 208 | | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
0 commit comments