Repository navigation
Commit ce3d0ad
fix(service-storage,plugin-audit,plugin-security)!: the attachment and comment parent gates judge a controlled_by_parent parent through its master (#22513)
Fixes #22455
Clause-②: no (narrowing)
The `sys_attachment` parent gate (`@objectstack/service-storage`) and
the `sys_comment` moderation gate (`@objectstack/plugin-audit`) now
judge a `controlled_by_parent` parent through its master: the answer the
parent's own by-id update gets. `@objectstack/plugin-security` serves
the master-detail write check that card #22464 declared
(`ISecurityService.checkControlledByParentWrite`) from the write path's
own composition, and both gates ask it when the sharing service
abstains. This executes seat 1's ruling A (`6079158667`) as carried by
triage (`6079448762`) and the claim `6083853580`.
## Reproduction (before)
Real stack: `bootStack`, org-bound, with `StorageServicePlugin` and
`AuditPlugin`. The fixture is
`packages/qa/dogfood/test/fixtures/cbp-parent-gates-fixture.ts`. A
member holds `org_member`, the fixture's read/create/edit baseline, and
a set granting `sys_attachment` and `sys_comment` create and delete. The
master `cpg_account` is `public_read`, so the member reads it, and it is
owned by the admin. `cpg_contract` is a `controlled_by_parent` child of
it. `cpg_vault_item` is a `controlled_by_parent` child of a private
admin-owned master. Measured with the new dogfood file on the unmodified
base `e148ca984` (test commit `bca810323`):
| Request by the member | Before |
|:--|:--|
| `PATCH /data/cpg_contract/ID` (precondition) | 403 `PERMISSION_DENIED`
|
| `POST /data/sys_attachment` on that `cpg_contract` | **201** (file
attached) |
| `POST /data/sys_attachment` on `cpg_vault_item`, which answers the
member 404 | **201** |
| `DELETE /data/sys_attachment/ID` of the admin's file on the
`cpg_contract` | **200** (row gone) |
| `DELETE /data/sys_comment/ID` of the admin's comment on the
`cpg_contract` | **200** (row gone) |
| `security.checkControlledByParentWrite` | not served (`undefined`) |
Controls, green before and after: the master owner attaches (201); the
member attaches under a master they own (201); a `public_read_write`
parent admits the member (201); an uploader deletes their own file
(200); an author deletes their own comment, and the master owner
moderates a member's comment (200).
## What changed
- **`plugin-security`**: serves `checkControlledByParentWrite(object,
recordId, context)` on the typed service literal, so the compiler holds
it to the contract.
- It runs `assertControlledByParentWrite` for the principal's sets, then
for the delegator's on an on-behalf-of context (step 2.8's two calls, in
order). The first refusal is the answer.
- Before that it runs the engine middleware's context prologue:
principal-less refusal, permission-set resolution failing closed, the
missing-delegator refusal and delegator resolution. To keep one
composition, that prologue moved out of the middleware into
`resolveOperationPrincipals`, which the middleware and the member both
call. Its refusals, their order and their wording are unchanged.
- Outcomes are read off what the check throws. Each master-edit refusal
is tagged with its leg in a `WeakMap` beside the error, never on it, so
the write path's envelope is byte for byte the same. The three
non-verdict errors map to the `unresolvable` reasons. Anything else (a
store fault, a refusal of the context) rejects unchanged.
- There is no second copy of the master-detail check: the member is the
check.
- **`service-storage`**: one composition, `mayEditParent`, answers
parent EDIT for all four limbs: attach, the row rule on update and
delete, and the re-point's attach rule. It reads the tri-state
`checkEdit`, not `canEdit`. The installer takes an optional trailing
security resolver, and `StorageServicePlugin` wires it.
- **`plugin-audit`**: `canEditParent` gets the same composition. It is
the one function behind the moderation limb, so it covers comment delete
(which reproduced) and comment update. Insert and re-point still ask
READ.
- **`AttachmentSharingLike` / `CommentSharingLike`** now `Pick`
`checkEdit`. New port types `AttachmentSecurityLike` and
`CommentSecurityLike` are `Pick`s of `ISecurityService`.
- **`content/docs/permissions/system-context.mdx`**: one census row
(9b), anchored at `security-plugin.ts#checkControlledByParentWrite`,
plus the census gate's own `--fix` counts (121 to 122) and nothing else
on the page. The reason: the served member answers `allow` for a system
context (the contract's first bullet), which is a new
`ExecutionContext.isSystem` read site, and `check:system-context-census`
requires every such site to carry a row. The seat authorized this one
row by amending the claim (`6083853580`), declared to `domain:devx`.
## Gate truth table (both gates, every limb)
| `checkEdit` | `checkControlledByParentWrite` | Gate |
|:--|:--|:--|
| `allow` | not asked | admit |
| `deny` | not asked | refuse, today's envelope |
| `abstain` | `allow` | admit |
| `abstain` | `not_applicable` | admit (the parent derives nothing from
a master: the `public_read_write` control) |
| `abstain` | `deny` (any leg) | refuse, today's envelope |
| `abstain` | `unresolvable` (broken declaration, missing record, null
master reference) | refuse, today's envelope (fails closed) |
| `abstain` | rejects (datasource fault, refused context) | propagates
unchanged; a fault keeps its `503` |
| `abstain` | member absent (no security service, or one that does not
serve it) | admit: today's answer, and the parent's own update meets no
master check either |
| no sharing service | n/a | degraded mode: caller-scoped parent read,
unchanged |
Envelopes: attach and re-point answer 403 `ATTACHMENT_PARENT_ACCESS`. A
delete of another user's file answers 403 `ATTACHMENT_DELETE_DENIED`,
and an update of it answers 403 `RECORD_NOT_ACCESSIBLE`. Comment delete
or update answers 403 `RECORD_NOT_ACCESSIBLE`. A caller who cannot read
the parent still gets the not-visible 403 `PERMISSION_DENIED`.
On `not_applicable`: the gates ask the member on every abstention. The
member is the one place that decides whether a parent is
`controlled_by_parent`, so the gates carry no second copy of that
predicate. `not_applicable` is therefore exactly the "any other abstain
admits" arm of the ruling, and it matches the contract's consumer rule
(proceed on `allow` and `not_applicable`). The claim's line first read
`not_applicable` as fail-closed. That reading only arises if the gate
decides "this parent is controlled_by_parent" itself, which this design
never does. The seat accepted this reading and corrected the claim line.
## Pins and ablations
Every ablation went through `scripts/ablation-replace.mjs`: the anchor
hit once, the blob changed, and on restore the blob equals HEAD with
`git diff HEAD` empty.
| Pin | Where | Ablation | Reading |
|:--|:--|:--|:--|
| Truth table (9 rows), member asked with the gate's envelope, 503
propagation, kernel without the member, delete / not-visible / update /
re-point limbs | `service-storage` `attachment-access-hooks.test.ts` |
`if (verdict === 'allow') return true;` becomes `if (verdict !== 'deny')
return true;` (the `canEdit` fold) | 12 failed / 63 passed; restored to
blob `c078a57acb10` = HEAD |
| The same for the moderation limb | `plugin-audit`
`comment-access-hooks.test.ts` | same mutation | 11 failed / 56 passed;
restored to blob `7718798ea3e2` = HEAD |
| Real stack: both attaches, the delete of another's file and the
comment delete refused; the controls admit; PATCH parity | `dogfood`
`cbp-parent-attachment-comment-gates.dogfood.test.ts` | both gates stop
consulting the master check, so an abstention admits; rebuilt;
`ablation-dist-preflight` found the marker in 2 built files of each
package | 4 failed / 6 passed (`expected 201 to be 403` twice, `expected
200 to be 403` twice). Restore leg: sources equal HEAD, rebuilt,
`--absent` passes with a clean tree, rerun 10 / 10 |
| Member parity table, served pin | `plugin-security`
`controlled-by-parent-write-member.test.ts`,
`registered-security-service-members.pin.test.ts` | A: member not
served. B: mirror step 2.8's `userId` guard. C: refusals carry no leg |
A 11 failed / 3 passed. B 2 failed (the guest pin and the principal-less
pin) / 12 passed. C 4 failed / 10 passed. Each restored to blob
`8b1ac155b333` = HEAD |
Three earlier dogfood ablation attempts were not measurements, and none
of them reached a test. Two mutations failed the declaration build (a
narrowing error, then an unused parameter), and one was refused by
`ablation-replace` because the replacement contained the anchor. Each
attempt restored the sources to HEAD.
## PATCH parity
- **Plugin level** (`controlled-by-parent-write-member.test.ts`): the
real engine middleware's by-id update and the served member run on one
store for one caller.
- `allow` for exactly the records the update admits: an owned master, a
master shared at `edit`, a master admitted by authored write RLS, and a
two-hop chain.
- Each `deny` leg (`object_permission`, `row_level_security`,
`record_sharing`) matches the reason step 2.8's refusal states. The
delegator leg is covered, and so is `master_chain` (an empty master
reference above the first hop).
- The three `unresolvable` reasons each meet a refused update. A system
context answers `allow`, and a non-`controlled_by_parent` object answers
`not_applicable`.
- A principal-less context and a missing delegator reject with the write
path's own refusal message.
- **Real stack**: the member's `PATCH` of the child answers 403, and its
message says `requires edit access to its master record (master
'cpg_account' not editable by this user (row-level security))`. The
member answers `{ outcome: 'deny', leg: 'row_level_security' }`.
- The leg is the master's write RLS, not record sharing: the platform's
`created_by` ownership floor binds `org_member` on the master, and
record sharing gives the member no basis to lift it. My first
expectation was `record_sharing`, and the measurement corrected it.
- The owner gets `allow` and a 200 `PATCH`. The member under their own
master gets `allow` and a 200 `PATCH`. The `public_read_write` board
gets `not_applicable`.
## The two serving notes (landing record `6083231669`)
1. **Step 2.8's `context.userId` guard is NOT mirrored.**
- Mirroring it would answer `allow` for a principal whose master nothing
measured. The contract's `allow` covers only a system context or every
leg passing, and this lane does not edit the spec TSDoc.
- Not mirroring it can only refuse: every non-`allow` outcome refuses.
The record's own update already refuses such a principal at the
object-level gate, because guest bindings refuse `allowEdit`, so the
overall parity holds.
- Pinned (a positions-only context answers `deny` on
`object_permission`) and ablated (mirroring the guard turns that pin
red). The spec's `allow` TSDoc needs no second clause.
2. **The registration log line** is now read off the registered object
(`Object.keys(registeredSecurityService)`, sorted), so it cannot fall
behind a served member again. The hand list named 16 of the 21 members
served before this PR. It omitted `describeDelegableScope`,
`describeDelegationNarrowing`, `getEffectiveObjectPermissions`,
`hasWriteBypass` and `resolveWriteScope`. It now prints all 22.
## Local verification (HEAD `15d05d2b6`)
- **Census:** `node scripts/check-system-context-census.mjs` at
`15d05d2b6` reads `check-system-context-census: OK — 122 elevation read
sites in 20 packages across 57 files, living in 104 symbol(s); the page
cites 117 symbol(s) against 117 required`. The gate's own `--fix` is a
no-op on the committed page (blob unchanged before and after).
- **Gates:** `dispatch-gates --commands` derived 105 families at
`15d05d2b6`. That is the 79 from the previous head plus 26 that the docs
path adds (doc frontmatter, route spelling, section names, landing
index, doc anchors, docs redirects, single H1, spec `check:docs`, and
others). All 105 were run with recorded exit codes, and every one exited
0. `--ran` reads: `105 derived famil(ies) accounted for — 105 run, 0
NOT-MEASURED`.
- **Workspace:** rebuilt in the recreated worktree (`turbo run build`,
73/73 tasks) before the gates ran.
- **Tests:** the code is unchanged since `bdce0149d`, so these readings
carry over. At `bdce0149d`, after rebuilding:
- the six cbp suites in `plugin-security`: 106 passed;
- the attachment and comment gate suites: 75 and 67 passed;
- the new dogfood file: 10 passed.
- **Full package suites** at `faea6141f` (main has moved since, through
merges that touch none of these packages):
- `plugin-security`: 191 files, 3969 passed / 45 skipped;
- `service-storage`: 47 files, 800 passed;
- `plugin-audit`: 42 files, 672 passed;
- `typecheck` for all three, test layers included: exit 0.
- **ESLint** over the 14 changed `.ts` files: 14 results, 0 errors, 0
warnings. No type-aware linting is enabled, so untouched files' verdicts
cannot move.
- **Main moved:** after the census commit, `main` moved by one commit,
`4e9fe9ff6` (the protocol version bump). It touches neither the census
page, these packages nor any `isSystem` read in code, so it was not
merged this round.
## Acceptance notes
- **Measured, other lane (class a):** `POST /api/v1/security/explain`
for the member, `{ object: 'cpg_contract', operation: 'update', recordId
}`, answers `allowed: true` with the OWD layer reading
"controlled_by_parent: rows are org-shared at this baseline". The same
member's `PATCH` of that record answers 403. Measured at `faea6141f`
with an untracked scratch probe on this PR's fixture, deleted after the
reading. The explain record write gate asks `sharing.canEdit`
(`canEditRecord`), whose abstention reads as permission. The member this
PR serves is the parity path. Not fixed here.
- Spec TSDoc drift: the `ISharingService.canEdit` TSDoc still names the
`sys_attachment` parent gate (and, under the write-depth paragraph, both
parent-record gates) as `canEdit` callers. Both now read `checkEdit`. No
runtime consumer; carrier: the spec lane's TSDoc follow-up, card #22497.
- The same caller list sits in `plugin-sharing`'s
`SharingService.canEdit` docblock. Doc only; carrier: none.
- The comment UPDATE limb moved with the delete limb (one function). For
org members the platform's edit floor answers that limb first, so the
visible change is on delete.
- Two changesets, not one. The served member is a non-breaking `minor`
for `plugin-security`, and the BREAKING banner rides only on the two
packages that narrow. The ADR-0087 disposition is `not-required
(no-migration-prescription)`. The remedy ("grant edit on the master") is
written as prose, because the gate refuses that category when a FROM-TO
block is detected and no other category applies to a runtime accept-set
narrowing.
- `scripts/engine-double-contract.pinned.json` gains one row, written by
`--write` for the new suite's pinned `findOne` double.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01WYYhVJ78u7PhwFViWo1EmQ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 215e662 commit ce3d0ad
18 files changed
Lines changed: 1683 additions & 181 deletions
File tree
- .changeset
- content/docs/permissions
- packages
- plugins
- plugin-audit/src
- plugin-security/src
- qa/dogfood/test
- fixtures
- services/service-storage/src
- scripts
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
10 | 10 | | |
11 | 11 | | |
12 | 12 | | |
13 | | - | |
| 13 | + | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| |||
106 | 106 | | |
107 | 107 | | |
108 | 108 | | |
| 109 | + | |
109 | 110 | | |
110 | 111 | | |
111 | 112 | | |
| |||
142 | 143 | | |
143 | 144 | | |
144 | 145 | | |
145 | | - | |
| 146 | + | |
146 | 147 | | |
147 | 148 | | |
148 | 149 | | |
| |||
285 | 286 | | |
286 | 287 | | |
287 | 288 | | |
288 | | - | |
| 289 | + | |
289 | 290 | | |
290 | 291 | | |
291 | 292 | | |
| |||
359 | 360 | | |
360 | 361 | | |
361 | 362 | | |
362 | | - | |
| 363 | + | |
363 | 364 | | |
364 | 365 | | |
365 | 366 | | |
366 | | - | |
367 | | - | |
| 367 | + | |
| 368 | + | |
368 | 369 | | |
369 | 370 | | |
370 | 371 | | |
371 | | - | |
| 372 | + | |
372 | 373 | | |
373 | 374 | | |
374 | 375 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
5 | | - | |
| 5 | + | |
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| |||
355 | 355 | | |
356 | 356 | | |
357 | 357 | | |
358 | | - | |
| 358 | + | |
359 | 359 | | |
360 | 360 | | |
361 | 361 | | |
| |||
372 | 372 | | |
373 | 373 | | |
374 | 374 | | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
375 | 386 | | |
376 | 387 | | |
377 | 388 | | |
| |||
0 commit comments