Skip to content

Commit 1f949da

Browse files
feat(service-storage, plugin-audit): the parent gates ask the parent's by-id update alone; explain composes the on-behalf-of write (#24745)
Fixes #24646 Clause-②: yes The `sys_attachment` parent gate and the `sys_comment` moderation gate now ask one question about the parent record: the security service's record-grained judgement of a by-id `update` of it. Ruling item 3 made a measurement the precondition. It found that `security.explain`'s verdict for an on-behalf-of (OAuth agent) caller did not match that caller's by-id PATCH, so this PR first makes explain compose the delegated write path, as item 3's fork arm requires, and only then drops the two steps. ## The ruling (comment 6279880675 on this card), items 1–3 > 1. The `sys_attachment` parent gate (`packages/services/service-storage/src/attachment-access-hooks.ts`) and the `sys_comment` moderation gate (`packages/plugins/plugin-audit/src/comment-access-hooks.ts`) drop their own-depth record-sharing step and their master-detail step; 「may edit the parent」 is the security service's record-grained by-id `update` judgement of the parent alone (`explain`, admitting only `allowed === true && record.visible === true`), the question PR #24641 already asks third. > 2. The governing paragraph 「Write DEPTH is an input the CALLER supplies — absent means `own`」 in `packages/spec/src/contracts/sharing-service.ts` is rewritten: the parent-record gates no longer ask this service; the 2026-08-10 ruling on #7144 (comment 5236141537, Option 1: the gates STAY at `own`) is superseded for the parent gates by this ruling, and #7144 carries a pointer. > 3. Premise first (三件套 ②): before the two steps are removed, the dev measures on a real stack, in both postures, that for an on-behalf-of caller `security.explain({ operation: 'update', recordId })` on the parent answers the same as the CRUD by-id update of that parent. ③ If it does not, the dev reports the fork and fixes `explain`'s delegated path first, on this card — ⛔ never drops the two steps anyway, ⛔ never falls back to A in silence. ## Item 3: the on-behalf-of measurement, and the fork it found **Rig.** The real `SecurityPlugin`, a real `ObjectQL` over `better-sqlite3`, the real `SharingService` and its middleware, and real platform identity objects. Each caller's context is assembled by the real `resolveAuthzContext` and `assembleExecutionContext`. The on-behalf-of caller is an OAuth agent with the `mcp_agent_data_write` ceiling acting for the user. There are three users: an `org`-write-depth delegator, an `own`-write-depth delegator, and a third user who owns the rest. There are three objects: a `private` parent with owner and creator columns, a `private` master, and a `controlled_by_parent` detail. The rig crosses 13 (user, record) pairs with two principals (the user itself, or the agent on their behalf) and two postures (`isolated`, which is walled, and `single`, which is wall-less), for 52 cells. Each cell compares `security.explain({ object, operation: 'update', recordId })` against the by-id PATCH of the same record. | tree | cells | parity | fork | |:--|--:|--:|--:| | base `ec2694dc19`, before the explain change | 52 | 48 | 4 | | this branch, after the explain change | 52 | 52 | 0 | All four forks have the same caller, an agent acting for the `org`-write-depth delegator, with two per posture. The fork ran in both directions: - **explain refused a row the PATCH admits.** The row is owned by someone else and was created by the delegator. Explain asked record sharing's per-record gate once, with no depth stamped, at `own`. The write asks it twice: once as the agent at its agent-leg depth, then as the delegator at the delegator's own depth. - **explain admitted a row the PATCH refuses.** The row is owned by the delegator and was created by someone else. Explain left out the delegator's own row-level write filter. The write's pre-image check ANDs that filter beside the agent's. The human (non-delegated) cells were at parity before and after, in both postures. That includes the `org`-depth writer on someone else's row: the PATCH and explain both admit it, and the pre-B gates refused it. That refusal is the narrower duplicate this ruling removes. Under ③, explain's delegated path is changed first, on this card, in commit `7da68682b1`. Item 3 was then measured again (the table's second row), and only after that were the two steps dropped (`ba6fb4912c`). ## What changes 1. **`@objectstack/plugin-security`: explain composes the on-behalf-of by-id write** (`security-plugin.ts` explain binding, `explain-engine.ts` record verdict). - For an on-behalf-of caller, record sharing's per-record `update` / `delete` gate is asked the way the sharing middleware asks it. The first call is as the agent, with step 2.6's agent-leg `__writeScope` (`intersectDelegatedScope`) and `__delegatorWriteScope`. If that admits, the second call is as the delegator at the delegator's depth. A delegation link that names no user refuses. - The authored-row-write probe receives the same stamped agent context. - The record verdict of an `update` / `delete` ANDs the delegator's own write filter (`computeRlsFilter` over the delegator's sets and context, the same call step 2.7 makes) beside the principal's Layer 1. A rejection is reported fail-closed, on `rls`. - Ordinary callers and the delegated READ verdict are unchanged. 2. **The two gates ask one question** (`attachment-access-hooks.ts`, `comment-access-hooks.ts`). Wherever the security service serves `explain`, "may edit the parent" is its record-grained `update` verdict for the parent alone (`allowed === true && record.visible === true`). Record sharing's `checkEdit` and the master-detail check are not asked there. A store fault propagates unchanged. Uploaders keep their own attachments and authors their own comments. The not-visible refusal for a caller who cannot read the parent is unchanged, and so is the read rule for posting a comment. 3. **Degraded kernels keep today's composition exactly** (the seat's safety constraint). - A security service that serves no `explain`, or no security service at all, judges no by-id update. There the gate keeps record sharing at `own`, then the master-detail check on an abstention. With no sharing service, the caller-scoped parent read stands alone. - With no sharing service but `explain` served, the parent read is still asked first and the judgement second, as today. - Dropping the steps on a kernel that serves no `explain` would admit every caller. Both module headers say so, and a degraded-kernel suite in each gate's tests holds it. 4. **`packages/spec/src/contracts/sharing-service.ts`, prose only.** - The paragraph "Write DEPTH is an input the CALLER supplies" now records that the parent-record gates ask the by-id update judgement instead of this service, and that the #7144 ruling is superseded for them. - It keeps the `resolveWriteScope` fail-open as the reason the gates never resolve a depth themselves. - The `canEdit` sentence that pointed at it is rewritten to match. - No type, member or export moves. `check:generated` reports all 14 artifacts up to date: the prose feeds none of them. 5. **`content/docs/permissions/attachments-access.mdx`.** The "Create — parent-edit access" section described the three-question composition, and it would have read false after this change. Not in the claim's file surface, but this change is what made it false. 6. **Changesets.** - `.changeset/24646-parent-gates-one-question.md`: `minor` for `@objectstack/service-storage`, `@objectstack/plugin-audit` and `@objectstack/spec`, `Clause-②: yes (widening)`, with ruling item 5's sentence. - `.changeset/24646-explain-on-behalf-of-write-verdict.md`: `patch` for `@objectstack/plugin-security`. **The behaviour, in item 5's words:** a caller who can update the parent record by id can also attach files to it and moderate its comments; a caller who cannot, still cannot. The gates only stop refusing what the CRUD path already admits. ## Pins - **Item 3, kept as rows** (`explain-enforce-parity.test.ts`, rig 4, `#24646 item 3`). There are 15 rows per posture, under `isolated` and `single`, covering update and delete. Each row checks explain against the by-id PATCH / DELETE for an on-behalf-of caller. The delegators' own requests are the controls. - **The pin #24647 pre-registered, flipped and kept load-bearing.** The case is "the parent gates' one question, the by-id update verdict, admits the row the by-id update admits". It asserts four things: - record sharing alone still answers `deny`; - explain's record verdict is `visible: true`; - the PATCH is admitted; - the **real** `sys_comment` moderation gate (plugin-audit source, wired to the rig's own security and sharing services) admits the moderation. On the row the policy does not admit, the PATCH, explain and the gate all refuse, the gate with `RECORD_NOT_ACCESSIBLE` / 403. - **The real gate on both postures** (7 cells × 2). The moderation gate admits exactly where the parent's PATCH does: - the `org` writer on someone else's row (the widening); - a `controlled_by_parent` detail judged through its master, admitted under an editable master and refused `RECORD_NOT_ACCESSIBLE` / 403 under a master the caller reads but may not edit; - an unreadable parent refused `PERMISSION_DENIED` / 403; - both delegated fork shapes. - **Gate units** (`attachment-access-hooks.test.ts`, `comment-parent-update-parity.test.ts`). Each gate has 10 shapes on every limb. Where `explain` is served, the gate answers exactly what the judgement answers, and `checkEdit` and the master check are never called. Two rows show the change itself: - record sharing at owner-only depth refuses and the judgement admits: the gate admits; - the master check alone would admit and the judgement refuses: the gate refuses. Refusals assert the ADR-0112 envelope (`code` and `status`). A degraded-kernel suite holds today's answers where `explain` is not served. ## Ablations Every ablation was run from committed HEAD, through `scripts/ablation-replace.mjs` in wrap mode with a trap restore. In every case the anchor moved 1 → 0, the restored blob equals the HEAD blob, and `git diff HEAD` is empty. The subjects resolve to source (the vitest alias for `@objectstack/plugin-audit`), so there is no `dist` leg. | # | forbidden behaviour put back | suite | result | |:--|:--|:--|:--| | A1 | explain's record verdict without the delegator's write filter | parity | 8 red / 150: the delegated "own row someone else created" update and delete, "row owned and created by someone else", the real-gate agent cell, both postures | | A2 | explain's per-record gate asked once, unstamped, for a delegated caller | parity | 6 red / 150: the delegated "row owned by someone else that it created" update and delete, the real-gate agent cell, both postures | | G1 | attachment: the two steps back in front of the judgement | gate | 18 red / 141 | | G2 | attachment: the judgement's refusal ignored | gate | 32 red / 141 | | G3 | attachment: a kernel serving no `explain` admits | gate | 52 red / 141 | | G4 | attachment: the old steps decide instead of the judgement | gate | 39 red / 141 | | C1 | comment: the two steps back | gate | 12 red / 36 | | C2 | comment: the judgement's refusal ignored | gate | 14 red / 36 | | C3 | comment: a kernel serving no `explain` admits | gate | 4 red / 36 | | C4 | comment: the old steps decide | gate | 21 red / 36 | | C5 | comment: the two steps back, on the real stack | parity | 5 red / 150: the flipped pin and the real-gate `org`-writer and agent cells, both postures | ## Tests Package suites ran at `79c3d98b98`, the merge of `origin/main` `9b2953d62c`. The only later commit, `d35ec05d55`, is a type annotation in one test file. Both commands for that file were re-run at `d35ec05d55`. - `pnpm --filter @objectstack/service-storage test`: 62 files, 1129 passed. `typecheck`: exit 0. - `pnpm --filter @objectstack/plugin-audit test`: 58 files, 950 passed. `typecheck`: exit 0. - `pnpm --filter @objectstack/plugin-security test`: 189 files, 3839 passed, 45 skipped. - `typecheck`: exit 0 at `d35ec05d55`. It had found 2 errors at `79c3d98b98`, in the new cells' expected-answer type, and `d35ec05d55` fixes them. - `explain-enforce-parity.test.ts`: 150 passed at `d35ec05d55`. - `pnpm --filter @objectstack/spec test`: 702 files, 21352 passed, 1 todo. `typecheck`: exit 0. - `pnpm --filter @objectstack/spec build && pnpm --filter @objectstack/spec check:generated`: all 14 generated artifacts up to date. ## Gates `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derives 114 families from this diff (merge-base `origin/main` `9b2953d62c`). I ran all 114 at `d35ec05d55`, each exit code captured before any pipe, and all 114 exited 0. The first run, at `79c3d98b98`, had three families refuse with `PREREQUISITE NOT MET` (exit 3), and none of the three measured anything: `check:skill-examples` (client SDK dists), `check:dual-build-cjs-loads` (package dists) and `check:i18n` (the CLI build closure). Their prerequisites were built (`turbo run build` of that closure), and then the whole set was run again on the final head. `--ran` reconciliation: "114 derived, 114 run, 0 NOT-MEASURED, 0 UNRUN" (exit 0). The local-only `pnpm check:dev-prereqs` also exits 0, since this diff touches `packages/spec`. CI runs the full battery. ## Acceptance notes - **The degraded-kernel choice is mine.** The ruling does not speak to degraded kernels, so the gates drop the two steps only where `explain` is served (point 3 above). The dev report puts this in `open_questions`. - **Not measured.** I did not measure explain's delegated READ verdict. The binding still says the delegated read depths are not modelled there, and B does not ask it. - **The door, outside this card.** The rig also showed one row shape where an on-behalf-of by-id update and the delegator's own by-id update disagree under an app-authored row-level update policy. Explain matches the door there, so the parent gates match it too, and this PR neither changes nor widens that answer. It is reported to the seat as a security-class finding, and its reproduction is kept off public surfaces. - **The pointer on #7144** is the seat's write after landing. - **Related:** #7144 (the superseded ruling), #24549 and PR #24641 (the gates' update judgement), #24647 (explain's authored-row-write deferral, which this PR's flipped pin builds on). --- _Generated by [Claude Code](https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 580fd4e commit 1f949da

11 files changed

Lines changed: 987 additions & 349 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
fix(plugin-security): `security.explain` answers an on-behalf-of caller's by-id update and delete the way the write itself does
6+
7+
For a principal acting on behalf of a user (an OAuth agent), explain's record-grained `update` / `delete` verdict now composes what the by-id PATCH / DELETE composes (ADR-0090 D10):
8+
9+
- record sharing's per-record gate is asked twice, as the write asks it: as the delegated principal at its agent-leg write depth, then as the delegator at the delegator's own write depth;
10+
- the delegator's own row-level write filter is ANDed beside the principal's, as the write's pre-image check ANDs it.
11+
12+
Before, explain asked the gate once at owner-only depth and left the delegator's write filter out, so it could refuse a row the write admits and admit a row the write refuses. Ordinary (non-delegated) callers are unaffected, and so is the delegated read verdict.
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@objectstack/service-storage': minor
3+
'@objectstack/plugin-audit': minor
4+
'@objectstack/spec': minor
5+
---
6+
7+
feat(service-storage, plugin-audit): the attachment and comment gates judge "may edit the parent record" by the parent's own by-id update alone
8+
9+
Clause-②: yes (widening)
10+
11+
A caller who can update the parent record by id can also attach files to it and moderate its comments; a caller who cannot, still cannot. The gates only stop refusing what the CRUD path already admits.
12+
13+
- **What widens.** The `sys_attachment` parent gate (`@objectstack/service-storage`: attaching, detaching or editing another user's attachment, re-pointing one, and `assertMayAttach`) and the `sys_comment` moderation gate (`@objectstack/plugin-audit`: editing or removing another person's comment) now ask one question about the parent record: the security service's record-grained judgement of a by-id `update` of it (`security.explain` for `update` on the parent, admitting only `allowed === true` and `record.visible === true`). They no longer ask record sharing at owner-only depth first, nor the master-detail check: both are part of that judgement already. So a caller whose write depth on the parent object is `unit`, `unit_and_below` or `org` may now attach and moderate wherever that depth lets them update the parent, and so may a caller an app-authored row-level update policy admits.
14+
- **What does not move.** A caller the parent's by-id update refuses is refused, with the same codes as before (`ATTACHMENT_PARENT_ACCESS`, `ATTACHMENT_DELETE_DENIED`, `RECORD_NOT_ACCESSIBLE`, all 403), and a caller who cannot read the parent still gets the not-visible refusal. A `controlled_by_parent` parent is still judged through its master. Uploaders keep their own attachments and authors their own comments. Posting a comment stays a read rule on the parent. A store fault keeps its own status.
15+
- **Kernels that judge no by-id update.** Where the security service serves no `explain` (or there is none), both gates keep exactly the composition they had: record sharing's tri-state edit verdict at owner-only depth, then the master-detail check. With no sharing service, a caller-scoped read of the parent is still asked first.
16+
- **`@objectstack/spec`.** The `ISharingService` contract prose ("Write DEPTH is an input the CALLER supplies") now records that the parent-record gates ask the security service's by-id update judgement instead of this service, and why they never resolve the parent's depth themselves. No type, member or export changes.

‎content/docs/permissions/attachments-access.mdx‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -43,30 +43,30 @@ independent of the record-level checks below.
4343
Creating a `sys_attachment` requires that the caller may **edit** the parent
4444
record — Salesforce edit-on-parent parity — and "may edit" has one meaning
4545
here: **a by-id update of that parent record, by the same caller, would be
46-
admitted.** The gate asks three questions in order and admits only when all
47-
three do:
48-
49-
1. record sharing (`ISharingService.checkEdit`);
50-
2. the master-detail check, for a `controlled_by_parent` parent
51-
(`ISecurityService.checkControlledByParentWrite`);
52-
3. the security service's judgement of that update — the record-grained verdict
53-
`security.explain` reports for `update` on the parent record (`allowed` and
54-
`record.visible` both).
46+
admitted.** The gate asks exactly that question, of the security service: the
47+
record-grained verdict `security.explain` reports for `update` on the parent
48+
record (`allowed` and `record.visible` both). That verdict is the write path's
49+
own composition — the parent object's grant, its row-level security, its record
50+
sharing at the caller's write depth, the master of a `controlled_by_parent`
51+
parent, and both legs of an on-behalf-of (agent) request — so a caller who can
52+
update the parent record by id can also attach files to it, and a caller who
53+
cannot, still cannot. A caller whose write depth on the parent object is
54+
`unit`, `unit_and_below` or `org` attaches wherever that depth lets them update.
5555

5656
The same rule governs every check that asks about the parent: attaching,
5757
detaching or editing another user's attachment, re-pointing an attachment to
5858
another record (judged on the new parent), and the file-attachments service's
59-
caller-scoped `assertMayAttach`. It never admits more than the parent's update
60-
does, and it can refuse more: record sharing is asked at the narrowest edit
61-
depth for these gates (the ruling recorded on `ISharingService`, "Write DEPTH
62-
is an input the CALLER supplies").
59+
caller-scoped `assertMayAttach`.
6360

6461
When no sharing service is wired, a caller-scoped read of the parent (a
65-
`findOne`) stands in for step 1. A kernel with no security service asks no
66-
update judgement, as the parent's own update meets none there. A store fault in
67-
step 2 or 3 keeps its own status (a `503` stays a `503`); it is never reported
68-
as a refusal. `uploaded_by` is **server-stamped** from the session; a
69-
client-supplied value is ignored.
62+
`findOne`) is asked first. A security service that judges no by-id update (it
63+
serves no `explain`, or there is none) leaves the gate on the composition it
64+
had before: record sharing (`ISharingService.checkEdit`) at the narrowest edit
65+
depth, then, for a `controlled_by_parent` parent, the master-detail check
66+
(`ISecurityService.checkControlledByParentWrite`). A store fault keeps its own
67+
status (a `503` stays a `503`); it is never reported as a refusal.
68+
`uploaded_by` is **server-stamped** from the session; a client-supplied value
69+
is ignored.
7070

7171
| Code | Status | When |
7272
| --- | --- | --- |

0 commit comments

Comments
 (0)