fix(plugin-approvals): every door that acts on an approval request answers a request the caller may not read as the read door does - #28916

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-approvals, touching 13 documentable anchor(s).

6 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/api/client-sdk.mdx (via approvals.getRequest (sdk, the route ledger binds it to GET /api/v1/approvals/requests/:id, selected by route anchor /approvals/requests/:id), getRequest (sdk, the bare tail of client method approvals.getRequest, bound to GET /api/v1/approvals/requests/:id))
• content/docs/api/plugin-endpoints.mdx (via /approvals/requests/:id (route, bridged from symbol requestInfo — its route source's handler names it; bridged from symbol sendBack — its route source's handler names it))
• content/docs/automation/approvals.mdx (via getRequest (sdk, the bare tail of client method approvals.getRequest, bound to GET /api/v1/approvals/requests/:id), /approvals/requests/:id (route, bridged from symbol requestInfo — its route source's handler names it; bridged from symbol sendBack — its route source's handler names it))
• content/docs/automation/flows.mdx (via ApprovalService (symbol, a top-level class), /approvals/requests/:id (route, bridged from symbol requestInfo — its route source's handler names it; bridged from symbol sendBack — its route source's handler names it))
• content/docs/kernel/services-checklist.mdx (via /approvals/requests/:id (route, bridged from symbol requestInfo — its route source's handler names it; bridged from symbol sendBack — its route source's handler names it))
• content/docs/permissions/system-context.mdx (via requestInfo (symbol, a method of class ApprovalService), sendBack (symbol, a method of class ApprovalService))

⛔ 4 release-owned page(s) also name something this change touched. These are read-only:

• content/docs/releases/v16.mdx (via getRequest (sdk, the bare tail of client method approvals.getRequest, bound to GET /api/v1/approvals/requests/:id))
• content/docs/releases/v17/17-0.mdx (via getRequest (sdk, the bare tail of client method approvals.getRequest, bound to GET /api/v1/approvals/requests/:id))
• content/docs/releases/v17/17-6.mdx (via ApprovalService (symbol, a top-level class))
• content/docs/releases/v18/18-1.mdx (via ApprovalService (symbol, a top-level class), /approvals/requests/:id (route, bridged from symbol requestInfo — its route source's handler names it; bridged from symbol sendBack — its route source's handler names it))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 8ac57677c73d74af56cd79a2f96bd6eea124d34e && git checkout 8ac57677c73d74af56cd79a2f96bd6eea124d34e
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9a9823d921a85a523529c25978daeb8229fa4ff9 7f95ac12a480f23750ba3a57394e8c79f3bc3ef3 && git checkout -B drift-repro 9a9823d921a85a523529c25978daeb8229fa4ff9 && git merge --no-ff 7f95ac12a480f23750ba3a57394e8c79f3bc3ef3

node scripts/docs-audit/affected-docs.mjs --json 9a9823d921a85a523529c25978daeb8229fa4ff9

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

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 7f95ac12a480f23750ba3a57394e8c79f3bc3ef3
Local-runs: none
Scope: packages/plugins/plugin-approvals/src/approval-service.ts, .changeset/harden-approval-act-doors-read-visibility.md, content/docs/automation/approvals.mdx, the 9 fixture suites under packages/plugins/plugin-approvals/src, packages/qa/dogfood/test/approval-act-doors-read-visibility.dogfood.test.ts, packages/qa/dogfood/test/position-address-readers.dogfood.test.ts + packages/rest/src/approvals-routes.ts (the nine act routes and handleApprovalError), packages/runtime/src/action-execution.ts (forwardApiAction / apiActionForwardRefusal, the action-door and MCP run_action forward), packages/plugins/plugin-approvals/src/pending-approver-read-grant.ts, request-read-gate.ts, approvals-plugin.ts (boot rebuildApproverIndex), packages/spec/src/contracts/approval-service.ts, packages/spec/src/api/error-code-ledger.zod.ts. reviewFace returns no governed hit on this file list; the review is owed at tier by the security-boundary clause of ruling 5985885287 (#21771), and the dispatching seat named the tier. Read-only: diff, PR text, the ruling, the head's check-runs (at 2026-11-02T04:20Z: core/lint/type-check shards still in progress, none concluded red; queue guard, PR-body guards, changeset-registration, dogfood regression gate green).

① Derived judgments

Door census (own enumeration, head tree). Service doors that act on one request: decideNode (reached by decide, so by the approve / reject routes, redeemActionToken and the SLA sweep's decideAndResume), recall, resubmit, and via loadPendingRow: sendBack (revise route), reassign, remind, requestInfo (request-info), comment. REST: nine POST /approvals/requests/:id/... routes, all through one handleApprovalError. Other reaches: the action-door type: 'api' forward (REST /actions and MCP run_action, one forwardApiAction) hands the approvals route the caller's own credentials and relays the target's status and body whole; the /approvals/act page and the plugin's raw mount decide through redeemActionToken; no other in-package caller of an act door exists (approval-node.ts, approval-revise-node.ts, lifecycle-hooks.ts, approvals-plugin.ts call none). continueRestoredRun and issueActionTokens are no caller door; the latter now reads as the system in place. Census matches the PR's list — correct.

1. Refusal placement (every side effect). In all eight service doors the read-rule call sits on the line after the row is found and before the status check, the slate test, the submitter test, the resume pre-flight, every sys_approval_action insert, every sys_approval_request update, every notify. What precedes it on each door is resolveActor (reads only) and input validation, both independent of the row and answered identically for a missing id. Nothing is written or emitted for a refused caller — correct; pinned at the service (row and action count unchanged, pending and decided) and at the HTTP door (row and history unchanged).

2. Same bytes as a missing id, per door. Both branches throw REQUEST_NOT_FOUND: plus the id the caller sent; the REST arm maps both through the same mapping row to 404 {code, error} with the code prefix stripped, same status path, no header differs. Across the forward, apiActionForwardRefusal relays the target's status and {code, error} for both ids alike — the forward's own doc already treats a filtered and a missing subject as one answer to the target, so no oracle opens there. Note only, outside the ruling's per-door claim: the read door's own 404 sentence is worded differently from the act doors' — each door is self-consistent, which is what the ruling asks.

3. Single definition. requestRowForCaller is the moved body of loadRequest (tenancy wall via findRequestRow, then visibleRequestIds with the row's own anchor); loadRequest and refuseUnreadableRequest both call it; no second predicate was written. The override exemption reuses isOverrideActor, judged against the request's organization as each door already judges it (documented in place). The generic data-door gate keeps calling visibleRequestIds as before. Correct.

4. Principal classes, accept set before and after:

   • pending approver by user id, by account email, by a held position: address: a reader through the approver index (sys_approval_approver, kept by syncApproverIndex on open, reassign and escalation; boot rebuildApproverIndex for legacy rows) — door's own rule, unchanged. The pending-approver READ GRANT (pending-approver-read-grant.ts) grants the target record, not the request; no approver reads the request only through it, so none is newly refused.
   • OOO delegate: the slot is rewritten to the delegate at open, so the delegate is on the index — unchanged. Reassigned approver: on the index after syncApproverIndex(next) — unchanged.
   • submitter: reader via submitter_id — unchanged. Past actor: reader via the actor_id / acted_as probe — unchanged.
   • record-reader tier (opt-in): reader, held to the door's own refusal — unchanged, pinned (this is also the proof the doors follow the rule rather than a copy).
   • tenant admin: exempt in their own organization and on an org-less request (isOverrideActor with a null request org) — unchanged. Another organization's admin: no override there and behind the wall — refused as a missing id, could not act before either.
   • platform admin across organizations: exempt — unchanged, pinned (decision lands, via_override, echo refuses as before).
   • action-link / email token: {...SYSTEM_CTX, userId} is a system context — exempt, unchanged (hono conformance 4/4 reported). SLA sweep: SYSTEM_CTX — exempt. Flow-engine resume: no door reaches an act door.
   • the one accepted change: an org-scoped caller on an org-less request. The read door already answers them as a missing id; the act doors now do the same before any write, where before the write landed and the echo failed. The PR names the pending approver; by construction the same holds for an org-scoped SUBMITTER on an org-less request on recall, remind, resubmit, comment. Same mechanism, same ruling clause, not pinned and not named — should-fix below.
     No class is newly admitted anywhere: the new call can only refuse.

5. Doors not touched, on purpose and correctly: GET /approvals/requests/:id and /actions (already one answer), continueRestoredRun, the action-link redemption.

6. Fixture triage across the 9 files. Every replaced refusal is one of: (a) a non-participant now asserted as a missing id with the state asserted unchanged (admin-exemption, both name-authority suites — their viaName vs viaPlain comparisons still measure the minted name against the plain member, later-step submitter, the override block, reassign and comment outsiders, the slot-reader block); (b) the door's own refusal re-pointed to a reader so that refusal is still what fires: the submitter holding another position (READING_BYSTANDER, READING_HOLDER), the approver put on the slate in the localized-recall suite (with the account row a user approver needs), the override actor for the submitter-only resubmit guard. (c) Two engine doubles learn $or as the real engine composes it — the doubles were wrong, not the probe; a hand-inserted legacy row gets the index the boot backfill gives it. Nothing those tests proved is weakened. The dogfood fix in 7f95ac1 is a correct re-point: that file asserts the bystander's detail read is 404 two lines above, so the door answer must match it; the holder's 200 and the submitter's 403 stay.

7. Pins non-vacuous. The ablation in the body replaces the one read-rule call with an unconditional return: dogfood pin 1/1 red, service block 1/4 red (the every-door case; the reader, tier and override cases do not depend on that call by design), restore proven by blob and clean tree. Consistent with the code: that call is the only thing the pin can detect.

8. Disclosure. PR body, changeset, commit messages and test titles name classes, positions and contracts; no earlier answer is stated as a probe recipe. The accepted-change paragraph names a legitimate caller's prior failure, not a probe.

9. Out-of-scope finding (submitter-less recall / remind admit any reader): the raw.submitter_id && guard on both doors is untouched by this diff and present on main; the new call only narrows ahead of it. Pre-existing, not widened; queued separately.

10. Client census (the ruling's first step): Console's inbox humanizeError already maps 404 to "request no longer exists" and 403 to "not allowed"; no shipped client reads a by-id act's 403 as "exists but no access"; the app-shell action panels show buttons only when viewer.can_act holds. The SDK's 403 note at client/src/index.ts is about the AI pending-actions queue, not approvals.

② Semver level

patch on @objectstack/plugin-approvals, Clause-②: no, Metadata-protocol: none — consistent. No published type changes (IApprovalService unchanged; requestRowForCaller / refuseUnreadableRequest private). The accept set of every reader is unchanged; the one refusal that moves earlier replaced a declared server-side failure (READ_BACK_FAILED, "the write IS recorded") for a caller the read door already hid the row from, and the maintainer accepted it. no stands; not (narrowing).

③ Boundary flags

• Author objectstack-fleet[bot] — no login to name.
• The maintainer's acceptance of the org-scoped-caller change is quoted in the body and relayed by the dispatching seat; there is no card and no GitHub comment carrying it. The dispatching seat holds that acceptance; it should also cover the submitter arm named in ① 4 (should-fix 1).
• dev open questions: none declared in the body.

④ Design judgment

Against ruling 5985885287 (letter A): "on the write doors, a row the caller cannot read is a row that does not exist", "for every principal class", "a caller who can read the row but may not write it keeps 403". The change implements exactly that on the nine approval act doors, reusing the read door's own visibility (the ruling's ④: "复用读接口已有的可见性判断,不加新门"), with the override and system exemptions the ruling's "every principal class" sentence does not forbid (an override actor is served the row by the read door inside their organization; the cross-organization platform admin is the one exemption the read door does not serve, and that exemption is documented, pinned and acts exactly as before). The docs change in approvals.mdx states the rule and the error table correctly. Holds.

Must-fix: none.

Should-fix:

1. Name the submitter arm of the accepted change (org-scoped submitter, org-less request: recall, remind, resubmit, comment) in the acceptance note, and pin it beside the approver arm in the org-filtered read-back block — same mechanism, currently unpinned.
2. approval-act-doors-read-visibility.dogfood.test.ts header names src/act-doors-read-visibility.test.ts as the service-side pin; that file does not exist — the block lives in approval-service.test.ts.
3. The read door's own 404 sentence and the act doors' differ in wording; each door is self-consistent, so nothing is owed by the ruling, but one sentence for all ten doors would cost nothing.

Implemented-by: claude/harden-approval-decision-visibility
Reviewed-by: session_017xRBcVy4VXmkQjn2MGeaWk

VERDICT: PASS

domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk

ACCEPT → head 7f95ac12a480f23750ba3a57394e8c79f3bc3ef3 · 2026-11-02T10:35Z

维护者速读

• 改了什么：审批请求的所有操作入口(批准、驳回、撤回、退回、重新提交、转办、催办、要求补充信息、评论)拿到请求行后，先按"查看"入口的可见性规则判断。规则只有一份，即 requestRowForCaller,和 getRequest 共用。看不到这条请求的调用方会得到 REQUEST_NOT_FOUND,和请求不存在时完全一样，系统不写入任何内容。override actor 和系统上下文不受影响；能看到请求的人，原来该拿 403/409 的照旧。
• 为什么改：QA 第三轮发现的安全加固(细节按披露裁决不公开)。
• 风险与回滚：只有一类正常调用方的行为会变，维护者已经接受：组织范围内的审批人或提交人，操作一条不属于任何组织的请求时，原来是先写入、再报 500,现在直接回 404,什么都不写。回滚即 revert 本 PR。
• 先看：packages/plugins/plugin-approvals/src/approval-service.ts,重点是 requestRowForCaller 和 refuseUnreadableRequest。
• 两分钟验证：pnpm --filter @objectstack/dogfood exec vitest run test/approval-act-doors-read-visibility.dogfood.test.ts,预期 1/1 通过。
• 需要你做的：无，席位落地。

席位意见：收。本 PR 落实 W3-RT 的私下安全报告。维护者在 PM 会话中批准了推送(「我同意推送，你应该使用pm-dispatch 技能审核和合并pr」),并分别接受了审批人一侧(「一个连带的行为变化，我接受」)和提交人一侧(「同意」)的行为变化。

席位核对

1. 形态：draft,目标 main。按披露裁决不建卡，正文不写关闭关键词。正文首行是签名；接受行为变化的原文已写进验收说明。
2. 前提实测：用 stock showcase 做了一次性探针(未提交):修复前，9 个操作入口对"看不到"和"不存在"给出的回答不同；修复后，12 次调用全部是同一个 404。ablation:撤掉修复后，pin 1/1 变红，服务端那组 4 个里有 1 个变红；恢复后 blob 与 HEAD 一致，dist 标记已清除。
3. 门禁：本地派生 93 项全部 exit 0。plugin-approvals 全套 1380 个测试通过。完整 dogfood 只有 1 处陈旧期望没过，已在 7f95ac12a 改为指向新的 404。本 head CI 共 54 项：40 success、14 skipped、0 失败。
4. 契约面：没有碰契约四面，Clause-②: no。按裁决 5985885287 做了存在性边界的达档复核 6568970094,结论 PASS:各入口都已覆盖，拒绝发生在任何副作用之前，回答逐字节一致，规则只有一份定义；逐类核对调用方，没有哪一类被新放行。
5. 范围外去向：should-fix 1 的 pin(提交人一侧)和 should-fix 2(dogfood 文件头的引用路径)并入后续 ④ 一起做(同一块代码)。无提交人请求的撤回/催办守卫也是 ④,按披露裁决私下交给车道。
6. 落地：不属于受管面，795 行，低于 3000 行上限 → pr_ready + automerge_enable。

Generated by Claude Code