Skip to content

Commit 7c7741d

Browse files
fix(service-storage,plugin-audit): the attachment and comment parent gates give the answer a by-id update of the parent gets (#24641)
Fixes #24549 Clause-②: no (narrowing) ## The invariant The `sys_attachment` parent gate (`@objectstack/service-storage`) and the `sys_comment` moderation gate (`@objectstack/plugin-audit`) now hold one invariant: **each gate gives the same answer as a by-id update of the parent record.** Every limb that asks whether the caller may edit the parent record asks, after record sharing and the master-detail check, the security service's judgement of a by-id `update` of that parent: `ISecurityService.explain` with `{ object, operation: 'update', recordId }` under the caller's envelope (the operation-private keys removed, as for the sharing call), admitting only when `allowed` and `record.visible` are both `true`. That verdict is the write path's own composition (the parent object's grant, its row-level security, its record sharing and its master), so the gates keep no second copy of it. The limbs: the attach rule, the uploader-or-parent-editor rule on update and delete, the attach rule on a re-point, `FileAttachmentService.assertMayAttach` (it asks the same `mayEditAttachmentParent`), and the comment kit's author-or-parent-editor rule on update and delete. - The step runs last and only for a caller the earlier steps admit, so it adds refusals and never admits one. Each limb keeps its envelope: `ATTACHMENT_PARENT_ACCESS`, `ATTACHMENT_DELETE_DENIED`, `RECORD_NOT_ACCESSIBLE` (all 403), and the not-visible `PERMISSION_DENIED` / 403 for a caller who cannot read the parent. - Record sharing's `deny`, the master check's refusals and a store fault keep their answers. The owner-only edit depth the sharing step is asked at stays, as the 2026-08-10 ruling recorded on `ISharingService` ("Write DEPTH is an input the CALLER supplies") requires. - A kernel with no security service asks nothing new (the parent's own update meets no such judgement there). A security service that serves no `explain` is not asked it, the same feature detection `assertMayAttach` already applies to its own `explain` question. - Posting a comment is unchanged: it stays a read rule on the parent. - `@objectstack/plugin-security` is not changed. Measured on a real stack, its `explain` update verdict already agrees with the by-id update on every row this card's invariant needs (one restrictive exception, below). ## Files - `packages/services/service-storage/src/attachment-access-hooks.ts`: `mayEditAttachmentParent` is two steps, record sharing and the master check, then `parentUpdateAdmits`. `AttachmentSecurityLike` adds an optional `explain`. - `packages/plugins/plugin-audit/src/comment-access-hooks.ts`: `canEditParent` takes the same two steps. `CommentSecurityLike` adds an optional `explain`. - `packages/services/service-storage/src/attachment-access-hooks.test.ts`: the parity rows, on the file's own pinned engine double. - `packages/plugins/plugin-audit/src/comment-parent-update-parity.test.ts`: the parity rows for the comment gate. - `.changeset/24549-parent-gate-parity.md`: `minor` for both packages, as a declared narrowing (`Clause-②: no (narrowing)`) in the launch window, with its ADR-0087 disposition `not-required (no-migration-prescription)`. ## Evidence The reproduction stays off GitHub under the disclosure ruling (#9387). What is cited here is counts and verdict lines only. - **Pins first, red on the unfixed gates.** Each row names the shape of the answers the gate is handed (sharing verdict, master outcome, the update judgement) and asserts the gate's answer, with `code` and `status` on every refusal. Before the fix, at commit `04ed98cf8`: attachment rows 38 failed / 29 passed, comment rows 20 failed / 12 passed. With the fix, all of them pass. - **Ablation**, on the final head `f4f4615e0`. With the fix committed, `scripts/ablation-replace.mjs` put the old decision back (record sharing and the master check alone) in each gate. The pins went red again on the same rows: attachment file 38 failed / 104 passed (142), comment pins 20 failed / 12 passed. The restore was proven: blob equal to `HEAD`, `git diff HEAD` empty, marker count 0. - **Real stack, differential.** ObjectQL with the SQL driver, `SecurityPlugin`, `SharingServicePlugin`, `StorageServicePlugin` and `AuditPlugin`, one process. For 8 caller shapes and 14 parent records, each gate operation (attach, delete another user's file, delete another user's comment) was compared with a by-id update of the parent by the same caller and with `security.explain` for the same pair. Cells where the gate admitted and the parent's update refused: 38 before, 0 after. The parent update and `explain` agree on every row but one. - **Package suites and typecheck** on the final head `f4f4615e0`: `@objectstack/service-storage` 61 files / 1118 tests passed; `@objectstack/plugin-audit` 57 files / 930 tests passed; `typecheck` for both, test layer included (`check:test-typecheck: OK`, 0 errors). - **Derived gates** (`node scripts/pm/dispatch-gates.mjs --commands`) on the final head `7deb8468f`: 99 commands (the docs page added 30 families), every command exit 0. `--ran` reconciliation: 99 derived, 99 run, 0 NOT-MEASURED, a zero derived from the recorded exit codes. The attachment parity rows live in `attachment-access-hooks.test.ts` so that they drive that file's ledgered engine double (`check:engine-double-contract`, `check:objectql-double-limit`). ## Acceptance notes - **Restrictive disagreements that remain**, where the gate refuses and the parent's update admits. Both were refusals before this change and stay refusals: 1. A caller whose edit depth on the parent object is wider than `own`. The sharing step is asked at `own` by the 2026-08-10 ruling recorded on `ISharingService`; this PR does not reopen it. `explain` agrees with the update here, so on these rows the gates and `explain` disagree. Raised to the seat as an open question. 2. One row where `explain`'s update verdict itself disagrees with the by-id update (`explain` refuses, the update admits). Reported to the seat as a finding of the explain-versus-enforce family; not fixed here. - **Changed answer:** an attach naming a parent object the registry does not hold now answers `404 OBJECT_NOT_FOUND`, what a by-id update of that object answers, in place of `403 FILES_DISABLED`. - **Real-stack pins inside the gate packages** would need `@objectstack/plugin-security` and `@objectstack/plugin-sharing` as devDependencies of both packages (two `package.json` files and the lockfile), outside this claim's file surface, so they are not added. The real-stack half (the `explain` update verdict equals the by-id update) is held by `plugin-security`'s explain parity suites. - **Docs:** `content/docs/permissions/attachments-access.mdx` states this invariant (commit `e1fdd691e`). The seat amended the claim’s file surface to cover the page at the review of record. --- _Generated by [Claude Code](https://claude.ai/code/session_01LR1cD9JqgkeqY7k9rrZH8s)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7879391 commit 7c7741d

6 files changed

Lines changed: 792 additions & 26 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/service-storage': minor
3+
'@objectstack/plugin-audit': minor
4+
---
5+
6+
The `sys_attachment` parent gate and the `sys_comment` moderation gate give the answer a by-id update of the parent record gets
7+
8+
Clause-②: no (narrowing)
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling or stored shape is removed, renamed or re-shaped, and no stored row is read, rewritten, converted or dropped, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is a runtime accept set: the attachment parent gate and the comment moderation gate admit only where a by-id update of the parent record would be admitted. The remedy for a refused caller is an access decision on the parent record, not a rewrite. The exported port types AttachmentSecurityLike and CommentSecurityLike gain one optional member, which no caller has to supply. The other categories are closed on facts: both bumped packages publish (not unpublished); no ADR-0087 id covers these paths and this diff adds none (not registered, not already-registered); the port types stay concretely typed (not type-surface-only). -->
11+
12+
- Every limb that asks whether the caller may edit the parent record (attaching a file, editing or removing another user's attachment or comment, moving an attachment to another record, and `assertMayAttach`) now admits only where a by-id update of that parent admits. After record sharing and the master-detail check, the gate asks the security service's judgement of that update: its record-grained `explain` verdict for `update` on the parent, `allowed` and `record.visible` both.
13+
- The step only adds refusals, with each limb's existing envelope (`ATTACHMENT_PARENT_ACCESS`, `ATTACHMENT_DELETE_DENIED`, `RECORD_NOT_ACCESSIBLE`, or the not-visible `PERMISSION_DENIED` for a caller who cannot read the parent). A store fault keeps its own status, and an attach naming a parent object the registry does not hold answers what a by-id update of it answers (`404 OBJECT_NOT_FOUND`).
14+
- Posting a comment is unchanged: it stays a read rule on the parent.

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

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ separates *file storage* from *where a file is attached*:
1515

1616
The governing principle: **an attachment has no access model of its own — it
1717
inherits its parent record's.** A caller who can read a record can read its
18-
attachments; a caller who can edit a record can attach to and detach from it.
19-
Enforcement is layered, and every gate is fail-closed.
18+
attachments; a caller who may edit a record — a by-id update of it by that
19+
caller would be admitted — can attach to it, detach from it and re-point an
20+
attachment to it. Enforcement is layered, and every gate is fail-closed.
2021

2122
`Field.file` / `Field.image` are a **separate** path — those store an opaque
2223
`sys_file` id in the record's own column (ADR-0104 D3; the
@@ -32,24 +33,45 @@ Attachments panel renders only for such objects. Any other target is rejected:
3233

3334
| Code | Status | When |
3435
| --- | --- | --- |
35-
| `FILES_DISABLED` | 403 | Creating a `sys_attachment` whose `parent_object` does not declare `enable.files: true` |
36+
| `FILES_DISABLED` | 403 | Creating a `sys_attachment` whose `parent_object` is a registered object that does not declare `enable.files: true` |
3637

3738
This is enforced by a `beforeInsert` hook on `sys_attachment` and is
3839
independent of the record-level checks below.
3940

4041
## Create — parent-edit access & provenance
4142

42-
Creating a `sys_attachment` requires that the caller can **edit** the parent
43-
record — Salesforce edit-on-parent parity — verified with the sharing service's
44-
`canEdit`, so RLS / OWD / sharing of the parent object apply (public-model
45-
parents are editable by any member by design). When no sharing service is
46-
wired, the gate degrades to caller-scoped read visibility (a `findOne`).
47-
`uploaded_by` is **server-stamped** from the session; a client-supplied value
48-
is ignored.
43+
Creating a `sys_attachment` requires that the caller may **edit** the parent
44+
record — Salesforce edit-on-parent parity — and "may edit" has one meaning
45+
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).
55+
56+
The same rule governs every check that asks about the parent: attaching,
57+
detaching or editing another user's attachment, re-pointing an attachment to
58+
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").
63+
64+
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.
4970

5071
| Code | Status | When |
5172
| --- | --- | --- |
52-
| `ATTACHMENT_PARENT_ACCESS` | 403 | The caller cannot access the parent record they are attaching to |
73+
| `ATTACHMENT_PARENT_ACCESS` | 403 | The parent record does not exist, or a by-id update of it by the caller would not be admitted (on a re-point, the new parent) |
74+
| `OBJECT_NOT_FOUND` | 404 | The `parent_object` names an object the registry does not hold — the answer a by-id update of it gets |
5375

5476
## Read / list — inherited parent visibility
5577

@@ -65,8 +87,9 @@ resolve the filter fails closed (deny-all).
6587
## Delete — uploader or parent editor
6688

6789
Deleting a `sys_attachment` is allowed when the caller is **the uploader** OR
68-
**can edit the parent record** (`sharing.canEdit` — public-model parents are
69-
editable by design). A multi-delete (`multi: true`) matches only attachments
90+
**may edit the parent record** — the parent-edit rule above: a by-id update of
91+
the parent by that caller would be admitted. The uploader keeps their own rows
92+
whatever the parent answers. A multi-delete (`multi: true`) matches only attachments
7093
the caller can read: an attachment whose parent is out of the caller's sight is
7194
not matched, not removed and not counted, so a predicate that reaches only such
7295
attachments answers exactly what a predicate that matches nothing answers. Every
@@ -81,7 +104,15 @@ matched row must then pass.
81104
An update of another user's attachment follows the same rule — the uploader or
82105
a parent editor — and a by-id update of an attachment the caller cannot read
83106
gets the same not-found answer. A multi-update matches only attachments the
84-
caller can read, as a multi-delete does.
107+
caller can read, as a multi-delete does. Re-pointing an attachment
108+
(`parent_object` / `parent_id`) to another record is an attach there: the new
109+
parent must pass the parent-edit rule too (`ATTACHMENT_PARENT_ACCESS`, 403).
110+
111+
The comment thread's moderation gate (`sys_comment`, `@objectstack/plugin-audit`)
112+
asks the same parent-edit rule: editing or removing another person's comment
113+
requires that a by-id update of the thread's parent record would be admitted,
114+
and the author keeps their own comments. Posting a comment stays a read rule on
115+
the parent.
85116

86117
The platform baseline also ships a parent-blind row-level delete floor
87118
(`owner_only_deletes`: you may delete only the rows you created, for members
@@ -181,7 +212,7 @@ can be shared across records). Reclamation is handled by the platform LifecycleS
181212
| Operation | Requirement | Deny code |
182213
| --- | --- | --- |
183214
| Attach (create) | parent object opts in (`enable.files`) | `FILES_DISABLED` (403) |
184-
| Attach (create) | can edit the parent record | `ATTACHMENT_PARENT_ACCESS` (403) |
215+
| Attach (create), re-point | a by-id update of the parent record by the caller would be admitted | `ATTACHMENT_PARENT_ACCESS` (403); `OBJECT_NOT_FOUND` (404) when the parent object is not registered |
185216
| List / read | inherits parent read visibility | *(filtered out)* |
186217
| Delete | uploader or parent editor (+ RBAC delete grant) | `ATTACHMENT_DELETE_DENIED` (403); `PERMISSION_DENIED` (403) when the parent is not readable or no delete grant is held |
187218
| Download | session + owner-or-parent-read (attachments scope); session only for a file with no parent; none for `acl: 'public_read'` | `AUTH_REQUIRED` (401) / `ATTACHMENT_DOWNLOAD_DENIED` (403) |

‎packages/plugins/plugin-audit/src/comment-access-hooks.ts‎

Lines changed: 71 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,22 @@
6464
* by-id update gets), whose `allow` and `not_applicable` admit and whose `deny`
6565
* and `unresolvable` refuse; a rejection propagates unchanged. A kernel whose
6666
* security service does not serve that member composes no master check, so the
67-
* abstention admits there, as the parent's own update would. ⛔ Never `canEdit`:
67+
* abstention admits there, as the parent's own update would.
68+
*
69+
* [#24549] Whatever those steps admit (the degraded read included), the
70+
* parent's own by-id UPDATE must admit too, and it decides last: the security
71+
* service's judgement of that update, `ISecurityService.explain`'s
72+
* record-grained verdict for `update` on the parent — `allowed` and
73+
* `record.visible` both. That verdict is the write path's own composition (the
74+
* parent object's grant, its row-level security, its record sharing and its
75+
* master), so the moderation limb gives the answer a by-id update of the
76+
* parent gets, and the gate keeps no copy of that judgement — the same step
77+
* the attachment kit takes. Asked only for a caller the steps above admit, it
78+
* adds refusals and never admits one; record sharing's `deny`, the master
79+
* check's refusals, the owner-only edit DEPTH (the #7144 ruling below) and a
80+
* rejection keep their answers. A kernel with no security service meets no
81+
* such judgement on the parent's update either, and a security service that
82+
* serves no `explain` is not asked it. ⛔ Never `canEdit`:
6883
* it folds `abstain` into `true`, and `effectiveSharingModel` maps every
6984
* `controlled_by_parent` parent to `public`, so that fold let any member holding
7085
* the `sys_comment` delete bit remove other people's comments from every
@@ -177,11 +192,16 @@ export interface CommentReadMiddlewareCtx {
177192
export type CommentSharingLike = Pick<ISharingService, 'checkEdit'>;
178193

179194
/**
180-
* The single member of the security service's contract this gate consults: the
181-
* ADR-0055 master-detail write check, for a `controlled_by_parent` parent.
182-
* OPTIONAL on the contract, so the gate handles its absence by construction.
195+
* The two members of the security service's contract this gate consults: the
196+
* ADR-0055 master-detail write check, for a `controlled_by_parent` parent
197+
* (OPTIONAL on the contract, so the gate handles its absence by construction),
198+
* and `explain`, whose record-grained `update` verdict is the parent's own
199+
* by-id update judgement (see "Parent EDIT" above). `explain` is
200+
* feature-detected like every member here, so it is optional on this view of
201+
* the contract.
183202
*/
184-
export type CommentSecurityLike = Pick<ISecurityService, 'checkControlledByParentWrite'>;
203+
export type CommentSecurityLike = Pick<ISecurityService, 'checkControlledByParentWrite'> &
204+
Partial<Pick<ISecurityService, 'explain'>>;
185205

186206
export interface CommentAccessLogger {
187207
info(msg: string, meta?: unknown): void;
@@ -682,9 +702,53 @@ export function installCommentAccessHooks(
682702
getSecurity?: () => CommentSecurityLike | null | undefined,
683703
): void {
684704
/** May the caller EDIT the parent record behind `target`? The composition
685-
* "Parent EDIT" in the module header sets out; caller-scoped parent read
686-
* visibility when no sharing service is present (degraded mode). */
705+
* "Parent EDIT" in the module header sets out, in two steps: record sharing
706+
* (or, with no sharing service, caller-scoped parent read visibility — the
707+
* degraded mode) and the master check; then, only when that admits, the
708+
* parent's own by-id update judgement. */
687709
const canEditParent = async (ctx: any, target: CommentThreadTarget, verb: string): Promise<boolean> => {
710+
if (!(await sharingAndMasterAdmit(ctx, target, verb))) return false;
711+
return parentUpdateAdmits(target, callerContext(ctx), verb);
712+
};
713+
714+
/** [#24549] The parent's own by-id update judgement: the security service's
715+
* record-grained `explain` verdict for `update` on the parent, which admits
716+
* only when `allowed` and `record.visible` are both `true` — a decision that
717+
* carries no record verdict did not judge the record, and refuses. See
718+
* "Parent EDIT" in the module header. A rejection propagates unchanged. */
719+
const parentUpdateAdmits = async (
720+
target: CommentThreadTarget,
721+
callerCtx: ExecutionContext,
722+
verb: string,
723+
): Promise<boolean> => {
724+
let security: CommentSecurityLike | null | undefined;
725+
try {
726+
security = getSecurity?.();
727+
} catch {
728+
security = undefined;
729+
}
730+
if (!security || typeof security.explain !== 'function') {
731+
logger.debug?.(
732+
`[audit] comment access: the security service judges no by-id update of ${target.object} here — ${verb} ` +
733+
'decided by record sharing and the master-detail check alone',
734+
);
735+
return true;
736+
}
737+
const decision = await security.explain(
738+
{ object: target.object, operation: 'update', recordId: target.recordId },
739+
callerCtx,
740+
);
741+
if (decision?.allowed === true && decision.record?.visible === true) return true;
742+
logger.debug?.(
743+
`[audit] comment access: ${verb} refused — a by-id update of ${target.object}/${target.recordId} is refused ` +
744+
'to this caller',
745+
);
746+
return false;
747+
};
748+
749+
/** The first step of {@link canEditParent}: record sharing (or the degraded
750+
* read), then the master check. */
751+
const sharingAndMasterAdmit = async (ctx: any, target: CommentThreadTarget, verb: string): Promise<boolean> => {
688752
const sharing = getSharing();
689753
if (!sharing || typeof sharing.checkEdit !== 'function') {
690754
logger.debug?.(

0 commit comments

Comments
 (0)