Skip to content

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

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-xv0lyn
Nov 6, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/project-thread-xv0lyn

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Elon · project thread

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 写管理员;② 审批决定后,流程里决定人那一段只带「这次决定」时的代登录管理员,本人决定则不带。
  • 为什么改:[Decision] Actions taken while impersonating a user are recorded only under that user #30213(PR feat(audit): record the administrator on writes made while impersonating a user #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 feat(audit): record the administrator on writes made while impersonating a user #30800). The key is the one auth-session-audit.ts and feat(audit): record the administrator on writes made while impersonating a user #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.
Ablations (each red, each restored; scripts/ablation-replace.mjs WRAP mode, git diff HEAD empty after each)
# 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.

Acceptance notes

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.

Builds, typecheck, package tests
  • 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.
Gates (dispatch-gates derivation, 88 families + 3 named)
  • 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).

Generated by Claude Code

…and on an approval decider's segment

plugin-auth's identity-table writes (the better-auth adapter under
withSystemContext, authSystemWriteContext, and the platform-admin routes)
now carry the attribution-only impersonatedBy read off the session beside
attributedUserId. An approval decision's resume signal carries the
decision request's impersonatedBy (ResumeSignal.impersonatedBy, honoured
only with the service marker beside resumedBy), and deciderSegmentContext
stamps exactly that value on the decider's segment, dropping the
trigger-time stamp, the self-decider path included.

Claude-Session: https://claude.ai/code/session_016WV4czA9H21XfNGbug1dac
Co-authored-by: Claude <noreply@anthropic.com>
…e approval decider's segment

Claude-Session: https://claude.ai/code/session_016WV4czA9H21XfNGbug1dac
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Nov 6, 2026
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

📓 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.

What this run could not see
  • 1 cross-cutting symbol(s) contributed no route anchor: userId (5 routes)
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 63 of 222 client-bound route-ledger rows — the other 159 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 159: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 104 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 175 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json cf478ea3398e90e97e73f421fd7f0842a6bd04bd → packageMentionDocs.

Which tree this was computed on

This run read content/docs from c31db44947483ea7a70de1fc3c7d8466174f3606 — the merge of head b5edf8ea83f244d78f1ff1e94602b50f0cf1cde3 into base cf478ea3398e90e97e73f421fd7f0842a6bd04bd, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

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.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

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.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 6, 2026 08:36
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 6, 2026
Merged via the queue into main with commit 9038187 Nov 6, 2026
65 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-xv0lyn branch November 6, 2026 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Impersonation attribution follow-ups after #30213: plugin-auth identity writes and the approval resume decider segment

1 participant