Repository navigation
fix(plugin-approvals,plugin-sharing): the approver a pending step waits on reads the record its request is about - #26397
Conversation
…is about (red first) Claude-Session: https://claude.ai/code/session_016NZtJpJqVWVJA1XDU9dbnz Co-authored-by: Claude <noreply@anthropic.com>
…ts on reads the record its request is about While an approval request is pending, each person on its current step's slate and the override holder may read the one record it is about: read only, ending when the step resolves, never across organizations. The approvals service derives the record ids from the slate it already resolves; the sharing read filter ORs them in beside the record shares and fails closed; explain names the grant via the approval request. Claude-Session: https://claude.ai/code/session_016NZtJpJqVWVJA1XDU9dbnz Co-authored-by: Claude <noreply@anthropic.com>
…he memo-retirement pin Claude-Session: https://claude.ai/code/session_016NZtJpJqVWVJA1XDU9dbnz Co-authored-by: Claude <noreply@anthropic.com>
…rom approver-address.ts, and classify the site Claude-Session: https://claude.ai/code/session_016NZtJpJqVWVJA1XDU9dbnz Co-authored-by: Claude <noreply@anthropic.com>
…ending approver's memo A rejected read leaves nothing behind, with no cleanup step; the system context needs no branch of its own (the sharing filter bypasses it). Claude-Session: https://claude.ai/code/session_016NZtJpJqVWVJA1XDU9dbnz Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 3 package(s): 18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 21 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin ede3cf2accc1d81540ccc0b685257742d599b206 && git checkout ede3cf2accc1d81540ccc0b685257742d599b206
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin abb9046057b6713b78712dfe53a329deefa534ba e8fb0d3bb12fbb202af1ba9789227e8585cb055c && git checkout -B drift-repro abb9046057b6713b78712dfe53a329deefa534ba && git merge --no-ff e8fb0d3bb12fbb202af1ba9789227e8585cb055c
node scripts/docs-audit/affected-docs.mjs --json abb9046057b6713b78712dfe53a329deefa534ba
|
席位核对 · ACCEPT ·
|
| item | reading |
|---|---|
| Shape | Draft, base main, Fixes #26180, patch changesets for @objectstack/plugin-approvals, @objectstack/plugin-sharing and @objectstack/plugin-security; Clause-②: no, Metadata-protocol: none. Not governed (0 of 12 paths), 1,051 changed lines. Security card: no advisory, no new issue. |
| Fix | Ruling 6429330267 letter A. A resolved person on a pending step's slate (user id or their own e-mail; position:* literals grant nobody), and the override holder inside their own organization, may read the one record the request is about. The grant enters only through SharingService.buildReadFilter as id $in the pending requests' record ids on that object. No write door reads it (PATCH 403 pinned). It ends when the step resolves (syncApproverIndex plus a status: 'pending' re-read, with the memo keyed by engine.writeEpoch). It is fenced to the request's organization, and explain names "via approval request N". It fails closed with one warn, and costs one bounded probe per context and epoch, never one per row. |
| Evidence | 36 check runs succeeded and 11 were expected skips on this head, 0 failed; mergeable clean. Integration and dogfood tests cover the ruling's seven cases: slate member reads, stranger does not, revoked on approve, reject or advance, no write, explain, override holder, and the org wall at the service. Red first on main. Derived gates: 73 of 74 exit 0. dual-build-cjs-loads was not measured locally (dist missing for untouched packages); CI's build answers it. |
| Carried | Reviewer notes, no action owed: explain reuses kind record_share (a dedicated kind needs a spec enum and a ui label, outside this card). A unanimous or quorum voter who has voted loses the read while the step stays pending for others, which is literal to the ruling. Overlap: #26337 (not this lane's) has adjacent inserts in sharing-service.ts, sharing-plugin.ts and explain-engine.ts; whichever lands second keeps both arms. Merge-tree with #26389 (#26181, same plugin) is clean. The record was taken while CI ran; the seat re-read the conclusions on this head before arming. |
| Landing | 「按照 pm-dispatch 规则复核过的都授权」 (Leehom, 2026-10-25T11:35Z): ready and auto-merge through the relay, once every check on this head is green or an expected skip. |
Generated by Claude Code
Main's pending-approver read grant (#26397) and this branch's ownership queue both extend the sharing read filter, the sharing plugin's service options and the explain sharing layer. Both are kept: the read filter ORs the owner match, the queue-sitting match and the granted ids (record shares plus pending-approval reads); explain reports the approval read grants, then the queue membership and claim rows. Claude-Session: https://claude.ai/code/session_01GTntg542RUmmRRbMY6wMZD Co-authored-by: Claude <noreply@anthropic.com>
Requested by Leehom · project thread
Fixes #26180
Clause-②: no
Metadata-protocol: none
维护者速读
plugin-approvals/src/pending-approver-read-grant.ts(谁有读权、组织围栏、按请求缓存)、plugin-sharing/src/sharing-service.ts的buildReadFilter并入处与 fail-closed 分支、plugin-security/src/explain-engine.ts的 approval request 归因。pnpm --filter @objectstack/dogfood exec vitest run test/approval-pending-approver-record-read.dogfood.test.ts,预期 1 passed:审批人 200、陌生人 404、PATCH 403、批准/驳回后 404。What changes
While an approval request is pending, each person on its current step's slate and the override holder may read the one record the request is about. The grant is derived from state plugin-approvals already keeps (the pending-approver index
sys_approval_approver), so nothing is declared: no metadata key, no app setting (ruling 6429330267, letter A).@objectstack/plugin-approvals: newPendingApproverReadGrantSource(pending-approver-read-grant.ts), exposed asApprovalService.pendingApproverReadGrants(object, context), which answersrecord id → pending request ids.actingAddresseswith no positions (the two addressesslateNamesAPersoncalls a person). Aposition:/team:literal names no person and grants nothing.isOverrideActorasked with the caller's active organization: every pending request of that organization.status: 'pending', and the index only holds rows while pending, so a step that is approved, rejected or advanced stops granting at the next read.permissionSetMemoprecedent). A person's first read is onesys_userread and one indexedsys_approval_approverread; a caller on no slate stops there for every object. A caller on a slate, or the override holder, pays onesys_approval_requestread per object. Never a read per row. Only settled answers are memoized; a failed read rejects and is asked again next time.@objectstack/plugin-sharing:SharingService.buildReadFilterORs those record ids into theid $inarm beside the caller's record shares, so every read door composing the sharing filter serves them. The probe is late-bound to theapprovalsservice and fails closed: no approvals service, no member, a lookup or read that throws, or an on-behalf-of principal (ADR-0090 D10) all grant nothing, with one warn line when a read fails. Write gates readsys_record_shareaccess levels only, so the member cannot write. NewlistApprovalReadGrants(object, recordId, context)for explain.@objectstack/plugin-security:explainattributes the grant on the sharing layer as arecord_sharerule withvia: "approval request N":admitson read,neutralon writes. A failing lookup attributes nothing.Tests
packages/qa/dogfood/test/approval-pending-approver-record-read.dogfood.test.ts(new, realbootStack, red first on main): a private object, a two-step flow with approvers by email. The first approver gets GET 200 and the record in the list; a stranger gets 404; the approver's PATCH is 403 with nothing landed;POST /security/explainnames "approval request N"; the override holder (organization_admin_no_bypass) reads; approving advances the step (first approver 404, second approver 200); the final approval revokes (second approver and overrider 404); a rejection revokes.packages/plugins/plugin-approvals/src/pending-approver-read-grant.integration.test.ts(new, ObjectQL + SqlDriver on better-sqlite3): person by id and by email, position literal grants nobody, stranger/submitter get nothing; override holder; decided requests (approve, reject) grant nothing, also on the same context; the org wall holds, for the platform admin and for an org-less caller too; the cost pin (five reads of one object plus one of another =sys_user,sys_approval_approver, twosys_approval_requestreads; a stranger costs two reads; a write retires the memo); a rejected read is rethrown and not memoized.packages/plugins/plugin-sharing/src/sharing-service.test.ts: seven cases on the OR-in, the join with record shares, no write gate, not asked for null/deny, delegated principal gets none, the fail-closed variants, andlistApprovalReadGrants.packages/plugins/plugin-security/src/explain-engine.test.ts: attribution on read (admits) and update (neutral), and a failing lookup attributes nothing.approver-address-readers.test.ts: the new slot comparison site is classified as a reader.Acceptance notes
record_sharewithvia: "approval request N", because a dedicated kind is a spec enum edit plus a ui label, outside this card.sharing-service.ts(options, field, constructor, thebuildReadFiltertail),sharing-plugin.tsandexplain-engine.ts.git merge-treereports content conflicts in those three files, all adjacent inserts. Whichever lands second keeps both: the queue disjunct and the approval ids in theid $inarm. No other open PR touches these files.Verification
All on head e8fb0d3 unless noted.
Red on main
The five source files reverted to base 6390fe5 (trap restoring from HEAD), the three plugins rebuilt,
ablation-dist-preflight.mjs --absentconfirmed the dist: the dogfood test failed at line 183,expected 404 to be 200(the first approver's read). Restore proved by an emptygit diff HEAD, then the dist was rebuilt and the preflight presence check passed.Suites
--no-inline-configon the three plugin packages and the dogfood file: exit 0, no findings.Derived gates
Derived with
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack(74 commands), each run withtimeout 900, exit captured before any pipe.pnpm check:dual-build-cjs-loads: exit 3, PREREQUISITE NOT MET (nodist/for packages this diff does not touch: studio, client-react, embedder-openai, knowledge-memory and others). NOT MEASURED locally; CI builds them.--ranreconciliation: "74 derived famil(ies) accounted for, 73 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3)", exit 0.Generated by Claude Code