fix(approvals): an approval action is recorded against the authenticated caller (#3800)

[os-zhuang]

Closes#3800.

Public text kept neutral on purpose. This is a public repo with published
packages; the analysis and impact assessment live in a private draft advisory
under the repo's Security tab, per this repo's split-disclosure convention.
This description covers the requirement and the design only.

Requirement

On every ApprovalService entrypoint the acting identity must be resolved
server-side from the authenticated execution context. It was taken from the
request payload (actorId, forwarded verbatim by REST at all nine call sites)
and the service's own checks then read that same value — so the thing that
satisfied the check was supplied by the caller.

#3783 pinned the data-write identity to the context (actingUserId), scoped
itself there, and characterised the remaining payload use as tolerable. Too
generous: the same field is read by the authorization checks, so it needed the
same treatment.

The fix

ApprovalService.resolveActor pins the actor server-side on all nine
entrypoints.

The rule is deliberately not "actorId must equal context.userId." A slot
can legitimately be keyed by something else — resolveApproverSpec stores the
type:value literal when a graph lookup finds no holders, the showcase
documents { actorId: 'position:finance' }, and the Console picks from the
caller's own identity list (id, email, or role:<r>), gating its buttons on
that set client-side rather than on the server's viewer.can_act. A strict-uid
rule would 403 real approvers. So:

Caller	Outcome
System context	Keeps its explicit actor — SLA sentinel, ADR-0043 action-link token, both already server-derived. Unchanged.
No identity at all	FORBIDDEN.
No actorId, or one naming the caller	Resolves to the caller. The common path.
Anything else	Allowed only if the server can prove the caller holds it — position:/role: against the resolved authz context, or their own email (one lazy sys_user read, only when nothing cheaper matched). Else FORBIDDEN.

REST keeps forwarding the body value; it is now a hint the service validates,
which is what preserves the email and type:value slot cases. A comment at the
route says so, so nobody reads it as unchecked again.

Verification

A new suite covers the rule across all nine entrypoints — 16 cases, of which 12
were red before this change and all 16 pass now.

It deliberately keeps 4 of them as load-bearing negatives — the real approver
deciding their own slot, the real submitter recalling, the SLA sentinel under a
system context, and an admin override — so "reject every actorId that isn't
the caller" cannot pass by breaking the feature instead of securing it.

Note the pre-existing approval-service.test.ts:653 ("blocks a non-approver in
a non-system context") only ever exercised a non-approver, so it passed
while the case this PR closes went uncovered.

pnpm test: 132/132 turbo tasks green, dogfood included.

The 24 existing tests that failed against the fix

Worth reading as evidence rather than noise — every one acted with a context
that named nobody, or named someone other than the actor it claimed:

• approval-revise.test.ts used a single USER_CTX = { isSystem: false, positions: [], permissions: [] } — no userId — for all 32 calls,
  pinning the old permissive behaviour in place.
• approval-service.test.ts called as CTX (u1) while passing
  actorId: 'u9' in 11 places.

Each now presents the acting user's own context via an asUser(id) helper. Two
needed real thought rather than a mechanical fix:

• The 审批请求的可见性是「按租户」而非「按参与者」——getRequest 用 SYSTEM_CTX 读、只按 organization_id 收窄 #3590 echo-back test relied on a user-less non-system context, which is
  now unreachable. Split into the authenticated case (the approver's own write
  echoes back even though approving clears them from pending_approvers — a
  stronger guard for 审批请求的可见性是「按租户」而非「按参与者」——getRequest 用 SYSTEM_CTX 读、只按 organization_id 收窄 #3590's actual intent) and the service-to-service case
  under a system context.
• The Approvals status mirror drops the acting user, forcing every downstream record-change flow to runAs:'system' #3783 "never takes the identity from the caller-supplied actorId" test
  described a scenario that can no longer happen. It now asserts the call is
  refused and that the mirror identity did not follow the body, so neither
  line can regress silently behind the other.

Same shape as #3760, where tests had likewise pinned the prior behaviour.

Filed separately, not fixed here

#3801 — the generic run-resume route validates machine state only and needs an
authorization gate of its own, keyed on what the run is parked on. It is a
different shape (the route is load-bearing for screen flows) and belongs in its
own change.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
objectstack	Ignored		Jul 28, 2026 6:02am

[github-actions]

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-approvals, @objectstack/rest.

10 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

• content/docs/api/error-catalog.mdx(via @objectstack/rest)
• content/docs/api/error-handling-server.mdx(via @objectstack/rest)
• content/docs/api/index.mdx(via @objectstack/rest)
• content/docs/automation/approvals.mdx(via @objectstack/plugin-approvals)
• content/docs/plugins/index.mdx(via @objectstack/rest)
• content/docs/plugins/packages.mdx(via @objectstack/plugin-approvals, @objectstack/rest)
• content/docs/protocol/kernel/i18n-standard.mdx(via packages/rest)
• content/docs/releases/implementation-status.mdx(via @objectstack/plugin-approvals, @objectstack/rest)
• content/docs/releases/v12.mdx(via @objectstack/rest)
• content/docs/releases/v9.mdx(via @objectstack/plugin-approvals)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.