fix(audit): name the impersonating administrator on identity writes and on an approval decided while impersonating - #30874

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/plugin-approvals, @objectstack/plugin-auth, @objectstack/service-automation, @objectstack/spec, touching 26 documentable anchor(s).

26 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json cf478ea3398e90e97e73f421fd7f0842a6bd04bd.

⛔ 10 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin c31db44947483ea7a70de1fc3c7d8466174f3606 && git checkout c31db44947483ea7a70de1fc3c7d8466174f3606
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin cf478ea3398e90e97e73f421fd7f0842a6bd04bd b5edf8ea83f244d78f1ff1e94602b50f0cf1cde3 && git checkout -B drift-repro cf478ea3398e90e97e73f421fd7f0842a6bd04bd && git merge --no-ff b5edf8ea83f244d78f1ff1e94602b50f0cf1cde3

node scripts/docs-audit/affected-docs.mjs --json cf478ea3398e90e97e73f421fd7f0842a6bd04bd

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 cf478ea3398e90e97e73f421fd7f0842a6bd04bd → pass the list as
args.docs, on the commit named under Which tree this was computed on.

domain:server#5 · os-elon-musk · session_016WV4cyvF7SP65frVFAYLv7

Contract review for card #30851 (follow-up to #30213 ruling A, PR #30800 records 6676544432 / 6677397495) · schema face (packages/spec/src/** hit) · read-only reviewer at the contract-review tier, brief per record-recognisers --template --paths ….

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: b5edf8ea83f244d78f1ff1e94602b50f0cf1cde3
Local-runs: none
Scope: packages/spec/src/contracts/automation-service.ts + packages/services/service-automation/src/run-resume.ts (deciderSegmentContext, decisionImpersonatorOf, the re-suspend persist site), packages/services/service-automation/src/engine.ts (resolveDecisionSegmentContext docblock), packages/services/service-automation/src/runtime-identity.ts (RunDataContext.impersonatedBy, resolveRunDataContext, read), packages/plugins/plugin-approvals/src/approval-service.ts (decideAndResume, actingUserId, impersonatorOf, serviceResume, resumeRecordedOutcome, journalStrandedContinuation, resolveRecordedContinuation, continueRestoredRun, the recall journal site, the SLA sweep), packages/rest/src/approvals-routes.ts (decision route, read), packages/core/src/security/resolve-authz-context.ts (session arm, read), packages/objectql/src/engine-execution-plumbing.ts (provenance projection, read), packages/plugins/plugin-auth/src/auth-actor-attribution.ts (AttributedAuthActor, AuthActorResolver, resolveAttributedActor, attributedActorOf, runAttributedToUser, authSystemWriteContext), auth-manager-actor-guards.ts (resolveActor), auth-manager-before-hook.ts, auth-manager.ts (signature only), auth-plugin.ts (gateAdmin and the six runAttributedToUser call sites), platform-admin-gate.ts (PlatformAdminActor.impersonatedBy), objectql-adapter.ts (withSystemContext), packages/plugins/plugin-auth/src/index.ts (export star, read), packages/spec/api-surface/contracts.json and export-origins/contracts.json (read), scripts/check-changeset-no-major.mjs (level rule, read), .changeset/30851-impersonation-identity-and-decider.md, and the four pin files the diff adds to.

The diff is 16 files, +484/-48 against merge-base e603385f. Nothing the hit promises was taken on the PR body's word: each claim below was read in the cited code at b5edf8ea.

① Derived judgments

• ResumeSignal.impersonatedBy (new, optional) — right. The one reader is deciderSegmentContext (run-resume.ts L320–364). It computes decider from signal.resumedBy only when signal[RESUME_AUTHORITY_SERVICE] === true and returns the persisted context before decisionImpersonatorOf(signal) is ever read when there is no decider (L326–333). So the member is honoured exactly as resumedBy is: beside a decider, under the in-process symbol. RESUME_AUTHORITY_SERVICE is Symbol.for(…) (automation-service.ts L837), which no JSON body can carry. The producer is ApprovalService.decideAndResume (approval-service.ts L5649–5666): impersonatorOf(context) reads ExecutionContext.impersonatedBy off the decision request's context, trimmed, and only when actingUserId(context) names a decider; nothing reads input. The REST decision route (approvals-routes.ts L480–495) passes resolveExecCtx(…) as the context and the body only as input; ExecutionContext.impersonatedBy has one writer, the session arm of resolveAuthzContextInScope (resolve-authz-context.ts L643–644), unchanged by this PR. The SLA sweep passes SYSTEM_CTX (L8270), so an auto-decision names neither decider nor administrator. The action-link path (L6979) has no session and so carries none, by construction.
• AutomationContext.impersonatedBy docblock — right. "An approval decision replaces it for the decider's segment with the decision's own ResumeSignal.impersonatedBy … absent when the decider acted in their own name, never the stamp of the write that started the run": the self-decider branch (L334–340) strips or replaces the trigger-time stamp, the switched branch (L356–357) deletes it and then sets the decision's value. The segment context is what the next pause persists (run-resume.ts re-suspend site, context not run.context), so a cold resume after a restart continues with the decision's stamp, as the engine docblock says. resolveRunDataContext (runtime-identity.ts L344–346) carries the trimmed value on both runAs arms into the kernel ExecutionContext, and objectql's plumbing (engine-execution-plumbing.ts L145–151) projects it to provenance, which the audit writer stamps. The "as on the decision itself" clause holds too: the verdict mirror writes with the same request context.
• resolveAttributedActor / AttributedAuthActor (new plugin-auth exports, published through export * in index.ts L108) — right. attributedActorOf answers undefined without a user id, so an administrator alone never attributes anything; impersonatedBy is answered only as a non-blank trimmed string. resolveAttributedUserId is now a projection, so no second resolver shape exists. The resolver the before-hook installs reads resolveActor(ctx), whose impersonatorOn reads session.impersonatedBy ?? session.impersonated_by off the getSessionFromCtx read or the token-fallback session row (auth-manager-actor-guards.ts L134–136, L147, L193): the session, never the body. AuthActorResolver widened to accept the object form while a bare string keeps its meaning.
• PlatformAdminActor.impersonatedBy (optional) — right. Set only by the route's gateAdmin closure (auth-plugin.ts L3612–3615) from session.session.impersonatedBy, after judgePlatformAdmin has already ruled; the verdict does not read it. runAttributedToUser accepts the actor object and sets the resolver to { userId: actor.id, impersonatedBy }; authSystemWriteContext spreads it beside attributedUserId with isSystem: true and no userId. withSystemContext now builds its write context from that same function, so the adapter, the membership reconciler and the admin routes carry one shape.
• Attribution-only, verified by enumeration. Every non-test reader of impersonatedBy at b5edf8ea is a producer (authz session arm, plugin-auth session reads), a carrier (automation context, run data context, provenance, journal) or the audit writers (audit-writers.ts, action-invocation-audit.ts). No permission evaluator, grant seeding or isSystem decision reads it. userId, attributedUserId, permissions, positions and posture move exactly as before this PR.

② Semver level

One changeset, four packages at patch, carrying Clause-②: yes (widening) and Metadata-protocol: none, both line-initial, matching the PR body. Patch is right. This repo's rule is the maintainer's ruling A on #25105 as mechanized in scripts/check-changeset-no-major.mjs (the "Ruling A: the level follows the protocol" block) and stated in AGENTS.md Post-Task step 3: the level follows the Metadata-protocol kind alone, none takes patch, and "a non-protocol change, widening or breaking, ships patch"; Clause-② keeps its meaning (it is what put this PR on the contract-review face) and lost its level coupling. none is right: no authorable key, value or type moves; ResumeSignal is a TS interface, packages/spec/api-surface/contracts.json records exports by name only (the breadth half of ADR-0059) and already lists it through export-origins/contracts.json, so no committed artifact is stale. Check Changeset is green on this head. No minor is owed.

③ Boundary flags

• Surface beyond the named files (engine.ts docblock; auth-manager.ts, auth-plugin.ts, auth-manager-before-hook.ts, auth-manager-actor-guards.ts, objectql-adapter.ts, platform-admin-gate.ts) — accepted. The claim's file surface is packages/plugins/plugin-auth/src/** for item 1, which covers every plugin-auth file; the engine.ts hunk is the docblock of the method whose body run-resume.ts carries, and a body change with its contract left stale would be the finding. Each extra plugin-auth hunk is the same seam (session to resolver to write context) and none is independent work.
• Open PR fix(plugin-auth)!: an allowed IP range that names no range is refused at save and never matches at the gate #30717 edits auth-manager.ts / auth-plugin.ts — no overlap. Its hunks sit at auth-manager.ts L1210–2190 and auth-plugin.ts L42–3300; this PR's are at auth-manager.ts L4936 and auth-plugin.ts L3605 and after. Textually disjoint and semantically unrelated (allowed IP ranges). Whichever lands second merges clean; the dev's commitment to merge origin/main after it stands.
• Clause-② re-derived from provisional no to yes (widening) — right. The claim's no was a guess at attribution-only metadata; the diff adds a public spec member, two plugin-auth exports and an optional actor field, which is a widening of a published public surface by the AGENTS.md definition. The re-derivation is the dev's duty and was done; it is what put this PR on the contract-review face.
• Cross-seat line (spec file on a domain:server card) — declared on the claim, with no domain:spec seat live to consult; the member is one optional key on a contract the automation engine already owns the reading of. Nothing to add.
• Outside the card, pre-existing, not owed here: the reconstructed (no-journal) replay in resolveRecordedContinuation carries neither resumedBy nor impersonatedBy, as the dev's report notes; the sys_comment fork stays with the maintainer on [Decision] Actions taken while impersonating a user are recorded only under that user #30213.

④ Design judgment

packages/spec/src/contracts/automation-service.ts — holds. Each break attempt, against the cited code:

• A signal carrying impersonatedBy without the service marker — decider is undefined at L326 and L327 returns the persisted context before the member is read; pinned (CONTROL, no marker).
• A marker-carrying signal naming an administrator with no resumedBy — same early return; the run keeps its identity and its own stamp; pinned (CONTROL, no decider).
• A self-decider whose trigger-time context carried a stamp — L334–340 returns a copy with the stamp removed when the decision carries none; pinned on both runAs arms (ablation A3 reds it).
• An impersonated decider — the switched segment carries the decision's administrator, the self-decider segment likewise; pinned on both arms, plus the trigger user deciding while impersonated (A4 reds it).
• A second plain decision after an impersonated one — the switched branch deletes the persisted stamp first; pinned.
• A restored or replayed run after a restart — two carriers: the persisted context at the next pause is the segment's (run-resume.ts re-suspend site), and a stranded decision's journal keeps impersonatedBy beside resumedBy (L5576), replayed by continueRestoredRun only beside a non-empty resumedBy and trimmed (L8061–8066); pinned end to end through restoreConsumedSuspension (A5, A6 red it). The recall journal (L5994) carries neither, correctly: a recall names no decider.
• Forging through the body or a non-service path — the decision route takes the body as input only and the context from resolveExecCtx; decideAndResume reads context alone; ApprovalDecisionInput has no such key and the whitelist drops it; pinned (an impersonatedBy: 'forged' input stamps nothing). The plugin-auth side is pinned on the real better-auth pipeline: an /update-user body key impersonatedBy is ignored and the engine sees the session's administrator.
• Could it widen what a run may do? No reader outside audit and provenance exists (① last bullet); the segment's grants are still seeded for userId, and the system arm still resolves no grant.

Findings:

• should-fix — packages/services/service-automation/src/runtime-identity.ts L102–110, the RunDataContext.impersonatedBy docblock still reads "The administrator who made the write that started this run". After this PR the decider's segment carries the decision's administrator instead, which the hit now promises; the prose of a cited carrier contradicts the hit. One sentence, no behaviour; follow-up or a rider on the next touch.
• nit, no action owed — the AutomationContext.impersonatedBy docblock's "An approval decision replaces it for the decider's segment" is scoped to a decider's segment and so stays true of an SLA auto-decision, which has none and keeps the run's stamp; the engine docblock says that explicitly.

No must-fix.

Check-runs on b5edf8ea at the time of writing: every completed run is success or skipped (Check Changeset, Governed Surface Queue Guard, PR-body guards, Lint & Repo Gates, every Lint shard, Type Check · source gates, Spec property liveness, Dogfood Regression Gate, Plan Test Core shards among them); none failed. Still running: Build Core, Build & E2E UI, Live E2E UI, Test Core 1–8, Test UI 1–16, Type Check UI, Type Check · workspace / consumer gates / debt ledger. Their conclusions are the gate verdicts; this record does not pre-empt them, and the landing waits for green.

Implemented-by: claude/project-thread-xv0lyn
Reviewed-by: session_016WV4cyvF7SP65frVFAYLv7

VERDICT: PASS

domain:server#5 · os-elon-musk · session_016WV4cyvF7SP65frVFAYLv7

席位核对 · ACCEPT

PR #30874 · head b5edf8ea · 2026-11-06T08:16Z · card #30851 (follow-ups of PR #30800's contract review, records 6676544432 and 6677397495; ruling A on #30213) · claim re-posted with Branch claude/project-thread-xv0lyn and the widened surface · os-dev report 6680265453 · contract review PASS 6680400800 (b5edf8ea)

维护者速读（席位校正）

The body's six lines stand as written, read against the diff. The last line is now met: the contract review PASSed on this head. The claim's plan of two PRs became one: both items land here, with Fixes #30851.

席位核对

• Card fit: item 1: plugin-auth's identity-table writes made while an administrator impersonates a user carry impersonatedBy, read from the session, so sys_audit_log.metadata.impersonated_by names the administrator. Item 2: the decider's segment carries exactly the decision-time impersonatedBy. A self-decider's trigger-time stamp is dropped, an impersonated decider carries the administrator, and a plain decider carries none.
• Route A (seat's implementation choice inside the card's Expected line): one optional ResumeSignal.impersonatedBy in packages/spec/src/contracts/automation-service.ts, a domain:spec file carried under the claim's cross-seat line. It is honoured only beside resumedBy under RESUME_AUTHORITY_SERVICE (a Symbol.for, which no JSON body can carry). The approvals decide path reads it off the request context, never input. The journal and the restored continuation carry it beside resumedBy. The reviewer enumerated every non-test reader: none is an authorization reader, so the change is attribution-only.
• Declarations: Fixes #30851, Clause-②: yes (widening) (re-derived from the claim's provisional no: a new spec member, plugin-auth's resolveAttributedActor / AttributedAuthActor, and an optional PlatformAdminActor.impersonatedBy), Metadata-protocol: none, and one changeset with patch on four packages. The level follows Metadata-protocol alone (the no-major gate), so patch is right; Check Changeset is green.
• Accepted deviations: the resolveDecisionSegmentContext docblock in engine.ts (the contract of the body it changes) and plugin-auth files beyond the named ones, inside the claim's plugin-auth/src/**.
• Carried, no card: the RunDataContext.impersonatedBy docblock in service-automation/src/runtime-identity.ts (L102–110) still reads "the write that started this run". It should also say the decider's segment. This is prose only, to fix on the next touch of that file (review should-fix). Outside this card: the reconstructed no-journal replay carries neither resumedBy nor impersonatedBy (pre-existing), and the sys_comment fork stays with the maintainer on [Decision] Actions taken while impersonating a user are recorded only under that user #30213.
• Overlap: open PR fix(plugin-auth)!: an allowed IP range that names no range is refused at save and never matches at the gate #30717 edits auth-manager.ts / auth-plugin.ts in other hunks; the reviewer read them as textually disjoint.
• Gates (report): 88 derived, 88 run, 0 NOT-MEASURED; pnpm lint, check:cli-command-ids and check:adr-symbol-anchors 0. Suites: auth 4096, automation 3429, approvals 1502. Six ablations went red and were restored, one of them the repro on the real /update-user route.
• Landing: check-governed-merges --pr 30874: NOT governed, +484/-48. The seat lands with pr_ready + automerge_enable once every check on b5edf8ea is green or expected-skip.