Skip to content

Commit 0148071

Browse files
fix(plugin-security)!: under a controlled_by_parent declaration whose one master reference is readonly or system, every write that sets it is refused (ADR-0055 §1) (#25256)
<!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuTXH2TuPG1JBf3xdmP114Fh)_ Fixes #25095 Clause-②: no (narrowing) Metadata-protocol: none ⛔ Disclosure ruling `5323310017` (#9387) applies: this PR states the rule and its prescription only. No probe, posture, caller, request or served value is recorded here or in any comment; the measurements are held privately. > **Route.** Ruling `6345869077` on #25095 chose route B: refuse at the write gate. This PR implements that ruling; it supersedes the narrower insert-only route this draft first carried. ## The rule ADR-0055 §1 derives a `controlled_by_parent` detail's access from exactly one required `master_detail` that every write of a detail supplies. Since #25092 the parse refuses that master when it is declared `readonly: true` or `system: true` (`controlledByParentMasterRefusal`, shape `master-not-writable`). An object with that declaration still reaches the registry through a door that does not parse: - a raw `registerObject` or `registerApp`; - `defineStack(…, { strict: false })`; - a stored object the boot registers under its `[metadata_spec_invalid]` diagnostic (the registry registers off-spec rows for availability). On such an object the write gate let writes that set the master reference through, and a detail could end up outside the master its access derives from: readable by nobody, while the write reported success. **Now** the write gate refuses every write that sets that reference, `422 INVALID_METADATA`, before any master is asked and before anything is written: - every insert, single or batch, whether or not its body names a master; - every update whose body carries the reference key as an own property, whatever the value, `null` included (an upsert reaches the gate as one of the two). The message is the parse's own refusal of the declaration, prefixed with the write it refused, so it carries the parse's prescription: remove the flag and declare the master reference as an ordinary required field, or give the object its own `sharingModel`. Both flags follow one rule, and every caller the gate judges gets the same answer. **Premise checked first** (ruling item 7): on all three doors above, the object registers, `resolveCbpRelation` resolves its relation, and the flag is readable on the field def the relation came from. ## What changed - `packages/plugins/plugin-security/src/security-plugin.ts` - `CbpRelation.masterNotWritable`: resolved in `resolveCbpRelation` from the same field def the relation came from, judged by `controlledByParentMasterRefusal` on that one def, so the flags and the sentence are spelled once, in the parse. The relation still resolves; the read leg, the cache and `reportCbpMasterUnresolved` are untouched. No second registry reader, and `omissionRefusedByValidation` is not widened. - `assertControlledByParentWrite`: right after the missing-relation leg, before any master is asked, `writesMasterReference` (insert always; update when the body carries the key) throws `MasterReferenceNotWritableError`. A delete, and an update that leaves the key alone, take the path they took before. - The two stale freeze-note paragraphs at the empty-master branch are rewritten to the current state (both ramp legs have landed, the parse refuses all three shapes, and no insert of theirs reaches that branch). The freeze itself stands. - `packages/plugins/plugin-security/src/errors.ts`: `MasterReferenceNotWritableError` beside `MasterDetailRelationMissingError` (`INVALID_METADATA`, 422; the error-code ledger already has this row for plugin-security). Not exported. The `MasterReferenceMissingError` docblock is corrected. - `.changeset/25095-cbp-unwritable-master-write.md`: `patch`, BREAKING, `Clause-②: no (narrowing)`, `Metadata-protocol: none`, ADR-0087 `not-required (already-registered cbp-master-relation-tiers-retired)`, as `pnpm check:adr-0087-registration` judges it. - `content/docs/api/error-catalog.mdx` and `content/docs/protocol/kernel/error-handling.mdx`: corrected, because this PR makes them false. They said a write that leaves out a `readonly` or `system` master reference answers `422 MISSING_REQUIRED_FIELD`; such a write now answers `422 INVALID_METADATA` (the two-item list above), and the `422 MISSING_REQUIRED_FIELD` exception is narrowed to a by-id update or delete of a detail whose stored master reference is null. The flat example body moves to that case. ## Unchanged, on purpose - **Reads.** The relation still resolves, so rows already stored keep the master their reads derive from. - **A delete, and an update whose body leaves the key out.** - System contexts (the bypass is unchanged), objects with an ordinary required master, and every object that does not declare `controlled_by_parent`. - No `packages/spec`, `packages/objectql` or lint change, and no read-side change. - The served member (`checkControlledByParentWrite`) asks a by-id update with no body, so it never reaches the new refusal, and its closed outcome vocabulary is untouched. ## Tests `packages/plugins/plugin-security/src/controlled-by-parent-unwritable-master-write.test.ts` (renamed from `…-insert.test.ts`, 83 cases). Real stack: `ObjectQL` over better-sqlite3, the real `SecurityPlugin`, the real sharing service and middleware, a member context from `resolveAuthzContext` + `assembleExecutionContext`, a platform admin's carried posture, and the engine's security source wired as a kernel wires it, so a detail read composes the engine's `ReadableIds` leaf over its master. The flagged declarations are registered raw through `registerApp`, for `readonly`, `system` and both, on an ordinary object and on a `sys_`-named one. For a member and for a platform admin: - a single create naming a master, a single create naming none, and a batch create each answer `INVALID_METADATA` / 422 with exactly the parse's message, and nothing is stored; the master walk is never entered; - a by-id update naming a master, and one setting it to `null`, answer the same, and the row keeps its master; - controls: a stored row is still read, an update that leaves the key out lands, a delete lands; - a detail declared as the three in-repo `controlled_by_parent` objects declare theirs (`Field.masterDetail`, `required: true`, through the parse) takes single and batch creates, a re-pointing update, a note update and a delete as before. A system create lands on every flagged declaration, and each flagged declaration is one the parse refuses at `fields` as `master-not-writable`. Moved pins, by the ruling: `controlled-by-parent-sharing.test.ts`'s `[#8688]` flagged-insert cases now expect `INVALID_METADATA` with the parse's message (and its stale "AUTHORABLE" wording is corrected); `controlled-by-parent-every-row-master.test.ts`'s cases on its `system`-flagged detail now expect `INVALID_METADATA` for every batch create and every emptying update, and its "the stored master is asked first" ordering pin moves to the validated detail, where it still holds. Ablation and the gate list are reported in the card's `os-dev-report`. ## Notes - **Branch.** `claude/project-thread-fi0g2a` is the harness-pinned branch for this session and names no issue, so the Duplicate Fix Guard's branch-name hint will not match. The claim on #25095 (`6345497639`) is the identity record. - **Serial.** PR #25198 (#24932) landed first; this branch merged `main` after it. The read scope it moved to the engine's `ReadableIds` leaf is not this gate, and the new suite fills the engine's security-source slot the way #25198's rigs do. Split card #25223 waits on this card. - `packages/spec/src/conversions/refused-shapes.ts` still says a stored `master-not-writable` object "loses the save door" only; a pointer is left for the `domain:spec` seat (#18549), since spec is not this lane's. - A `CONTRACT_REVIEW_TIER` review of record on the narrowing face is owed before the queue. --- _Generated by [Claude Code](https://claude.ai/code/session_01LQG7z3CxDLtiMaHLbTFSeB)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LQG7z3CxDLtiMaHLbTFSeB --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8dfd126 commit 0148071

8 files changed

Lines changed: 727 additions & 151 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
fix(plugin-security)!: under a `controlled_by_parent` declaration whose one master reference is `readonly` or `system`, every write that sets that reference is refused (ADR-0055 §1)
6+
7+
Clause-②: no (narrowing)
8+
9+
Metadata-protocol: none
10+
11+
<!-- adr-0087: not-required (already-registered cbp-master-relation-tiers-retired) registered before this change, and it covers it: that entry's stored-row listing (`os migrate meta --stored`) already reports this declaration as `master-not-writable`, with the prescription below. Nothing new moves: no spec key, authorable spelling, object definition, stored row or stored shape is added, removed, renamed or re-shaped, and no exported declaration is removed or narrowed. What changes is the answer the security plugin's write gate gives on an object that reached the registry with a declaration the parse already refuses. -->
12+
13+
**BREAKING** for a caller that relied on such a write landing. A non-protocol change, shipped as `patch`.
14+
15+
ADR-0055 §1 derives a `controlled_by_parent` detail's access from exactly one required `master_detail` field that every write of a detail supplies. The parse refuses that master when it is declared `readonly: true` or `system: true`, so an object with it reaches the runtime only through a door that does not parse: a raw `registerObject` or `registerApp`, a `defineStack` call with `strict: false`, or a stored object the boot registers under its `[metadata_spec_invalid]` diagnostic. On such an object the security plugin's write gate let writes that set the master reference through, and a detail could end up outside the master its access derives from, readable by nobody, while the write reported success.
16+
17+
- **Before.** An insert under that declaration was written when it named a master. An update could set the reference.
18+
- **Now.** Every write that sets the reference answers `INVALID_METADATA` (422) and writes nothing, before any master is asked: every insert, single or batch, whether or not it names a master, and every update whose body carries the reference key, whatever the value, `null` included. The message is the parse's own refusal of the declaration, prefixed with the write it refused, so it names the object, the field, each flag and the fix. It is the same for every caller the gate judges.
19+
- **Unchanged.**
20+
- The rows already stored: the security plugin still derives their read access from that field, so they keep their master and are read as before.
21+
- A delete, and an update whose body leaves the reference key out.
22+
- System contexts, every object declared with an ordinary required master, and every object that does not declare `controlled_by_parent`.
23+
24+
**What to do.** Fix the declaration as the parse says: remove `readonly: true` and `system: true` from the master reference and declare it as an ordinary required field. If the rows must outlive their parent, the object is not a detail: give it its own `sharingModel` (`private`, `public_read` or `public_read_write`).

‎content/docs/api/error-catalog.mdx‎

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -178,24 +178,35 @@ substitute. See the [Data API](/docs/api/data-api).
178178
### `MISSING_REQUIRED_FIELD`
179179
**Cause:** A required field was not provided in the request body.
180180
**Fix:** Include the missing field. Check `fields` for the field name — **except** on the
181-
`controlled_by_parent` master-reference paths below, where the status is `422` and no
182-
`fields` array is sent; read the field name out of the message there.
181+
`controlled_by_parent` stored-master path below, where the status is `422` and no `fields`
182+
array is sent; read the field name out of the message there.
183183
**Retry:** `no_retry`
184184

185185
<Callout type="warn">
186-
**`MISSING_REQUIRED_FIELD` is `400` with one exception: an absent `controlled_by_parent`
187-
master reference answers `422`, without `fields`.**
186+
**`MISSING_REQUIRED_FIELD` is `400` with one exception: a `controlled_by_parent` detail
187+
whose stored master reference is null answers `422`, without `fields`.**
188188

189189
An object whose `sharingModel` is `controlled_by_parent` derives its access from a master
190190
record, so the gate authorizing writes to it resolves that master *before* the executor —
191-
and the executor is where required-field validation runs. Which of the two refuses first
192-
depends on how the master reference is declared. A `master_detail` that is `required` and
193-
neither `readonly` nor `system` reaches validation and answers the documented
194-
`400 VALIDATION_FAILED` with `fields`. The other two declarable shapes never reach it and
195-
answer `422 MISSING_REQUIRED_FIELD` with no `fields`:
196-
197-
- a `master_detail` that is `required` + `readonly`
198-
- a `master_detail` that is `required` + `system`
191+
and the executor is where required-field validation runs. An update or delete **by id** of
192+
a detail record whose *stored* master reference is null answers
193+
`422 MISSING_REQUIRED_FIELD`, whatever the declaration: the caller sent no such field, so
194+
nothing could be named in `fields` and no payload would fix it.
195+
196+
A request that leaves the master reference out, or empties it, does not answer this code.
197+
On a `master_detail` that is `required` and neither `readonly` nor `system`, validation
198+
answers it with the documented `400 VALIDATION_FAILED` and `fields`. A master reference
199+
marked `readonly` or `system` is not one an author's write supplies, so it cannot be the
200+
master (ADR-0055 §1): `ObjectSchema` refuses that declaration at parse
201+
(`master-not-writable`), and on an object that reaches the runtime without the parse, every
202+
write that sets the reference answers `422 INVALID_METADATA`:
203+
204+
- every insert, single or batch, whether or not it names a master;
205+
- every update whose body carries the reference key, whatever the value, `null` included.
206+
207+
Its message starts `[Security] Configuration defect:` and carries the parse's own refusal,
208+
naming the object, the field, the flag and the fix. A delete, and an update that leaves the
209+
key out, are answered as on any detail.
199210

200211
A `master_detail` with no `required`, and a `required` `lookup` on an object that declares no
201212
`master_detail`, are not master references at all: the master is exactly one `master_detail`
@@ -205,20 +216,14 @@ one that reaches the runtime unparsed answers every write `422 INVALID_METADATA`
205216
The full matrix is on the protocol page:
206217
[`MISSING_REQUIRED_FIELD`](/docs/protocol/kernel/error-handling#missing_required_field).
207218

208-
An update or delete **by id** whose *stored* master reference is null answers the same
209-
`422`, whatever the declaration — the caller sent no such field, so nothing could be named
210-
in `fields` and no payload would fix it.
211-
212-
The refusal itself is correct and is not going to be relaxed: on those two shapes
213-
required-field validation does not fire (it skips `system` and `readonly` fields before its
214-
required check), so the gate is the only
215-
thing standing between the request and a detail record with a null master reference — which
216-
the `controlled_by_parent` read filter (`fk IN (readable masters)`) can never match, leaving
217-
a record readable by nobody.
219+
The `INVALID_METADATA` refusal is not going to be relaxed: required-field validation skips
220+
`system` and `readonly` fields before its required check, so on those two declarations the
221+
gate is the only thing standing between the request and a detail record outside its master
222+
— which the `controlled_by_parent` read filter (`fk IN (readable masters)`) can never match,
223+
leaving a record readable by nobody.
218224

219-
These shapes are **authorable today**: `os lint` reports both at error, but the publish gate
220-
does not run that rule. **Branch on `code`, treat `fields` as optional, and read the status
221-
off the response** rather than deriving it from the table at the end of this page.
225+
**Branch on `code`, treat `fields` as optional, and read the status off the response**
226+
rather than deriving it from the table at the end of this page.
222227
</Callout>
223228

224229
### `INVALID_FORMAT`
@@ -973,7 +978,7 @@ async function handleApiCall() {
973978
| 404 | `not_found` | `RECORD_NOT_FOUND`, `OBJECT_NOT_FOUND`, `ENDPOINT_NOT_FOUND` |
974979
| 409 | `conflict` | `CONCURRENT_MODIFICATION`, `UNIQUE_VIOLATION`, `DELETE_RESTRICTED` |
975980
| 413 | `validation` | `PAYLOAD_TOO_LARGE` — an upload over the storage settings' `max_upload_mb`, or an import over its row ceiling (see [above](#payload_too_large)) |
976-
| 422 | `validation` | `MISSING_REQUIRED_FIELD` on an absent `controlled_by_parent` master reference (see [above](#missing_required_field)) — this row is an exception to the 400 row, not a second home for the code |
981+
| 422 | `validation` | `MISSING_REQUIRED_FIELD` on a `controlled_by_parent` detail whose stored master reference is null (see [above](#missing_required_field)) — this row is an exception to the 400 row, not a second home for the code |
977982
| 429 | `rate_limit` | `RATE_LIMIT_EXCEEDED`, `QUOTA_EXCEEDED` |
978983
| 500 | `server` | `INTERNAL_ERROR`, `DATABASE_ERROR` |
979984
| 501 | `server` | `NOT_IMPLEMENTED` — the storage routes answer it when the adapter cannot issue presigned URLs |

‎content/docs/protocol/kernel/error-handling.mdx‎

Lines changed: 36 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ ObjectStack uses standard HTTP status codes:
281281
| **404** | Not Found | Resource doesn't exist |
282282
| **409** | Conflict | Resource already exists or version mismatch |
283283
| **413** | Content Too Large | An upload over the storage settings' `max_upload_mb`, or an import over its row ceiling — answered as [`PAYLOAD_TOO_LARGE`](/docs/api/error-catalog#payload_too_large) |
284-
| **422** | Unprocessable Entity | Semantic validation failed (e.g. metadata spec validation); also an absent `controlled_by_parent` master reference — see [`MISSING_REQUIRED_FIELD`](#missing_required_field) |
284+
| **422** | Unprocessable Entity | Semantic validation failed (e.g. metadata spec validation); also a `controlled_by_parent` detail whose stored master reference is null — see [`MISSING_REQUIRED_FIELD`](#missing_required_field) |
285285
| **429** | Too Many Requests | Rate limit exceeded |
286286
| **500** | Internal Server Error | Server-side error |
287287
| **503** | Service Unavailable | Server overloaded or maintenance |
@@ -534,7 +534,7 @@ code is the one the *master-access gate* and the package-publish route use.
534534
**Flat** (the 422 exception, on the `/data` door):
535535
```json
536536
{
537-
"error": "[Security] Missing master reference: insert on 'invoice_line' did not supply 'invoice_id'. A controlled_by_parent detail derives its access from its master, so 'invoice_id' must carry a master record id on every write.",
537+
"error": "[Security] Missing master reference: update on 'invoice_line' cannot be authorized because record 'line_42' has no value in 'invoice_id'. A controlled_by_parent detail derives its access from its master, so that reference must be populated.",
538538
"code": "MISSING_REQUIRED_FIELD",
539539
"object": "invoice_line"
540540
}
@@ -552,24 +552,38 @@ code is the one the *master-access gate* and the package-publish route use.
552552
```
553553
554554
⛔ Neither body carries a `details` bag. On the flat 422 there is no `fields` array either
555-
— see the table below for why.
555+
— see below for why.
556556
557-
**Exception — an absent `controlled_by_parent` master reference answers 422 with no `fields`.**
558-
An object whose `sharingModel` is `controlled_by_parent` derives its access from a master
559-
record, so the gate that authorizes writes to it must resolve that master *before* the
560-
executor runs — and the executor is where required-field validation lives. When the master
561-
reference is absent, whichever of the two refuses first decides the envelope, and that
562-
depends on how the reference is declared:
557+
**Exception — a `controlled_by_parent` detail whose stored master reference is null answers
558+
422 with no `fields`.** An object whose `sharingModel` is `controlled_by_parent` derives its
559+
access from a master record, so the gate that authorizes writes to it must resolve that
560+
master *before* the executor runs — and the executor is where required-field validation
561+
lives. An update or delete **by id** of a detail record whose *stored* master reference is
562+
null answers `422 MISSING_REQUIRED_FIELD`, whatever the declaration, with the message
563+
prefixed `[Security] Missing master reference:`. The caller supplied no such field, so there
564+
is no request field to name in `fields` and no payload that would fix it.
565+
566+
A request that leaves the master reference out, or empties it, is answered by whichever
567+
refuses first, and that depends on how the reference is declared:
563568
564569
| Master reference declared as | Refused by | Status | `code` | `fields` |
565570
|---|---|:---:|---|:---:|
566571
| `master_detail` + `required`, not `readonly`/`system` | required-field validation | 400 | `VALIDATION_FAILED` | yes |
567-
| `master_detail` + `required` + `readonly` | the master-access gate | **422** | `MISSING_REQUIRED_FIELD` | **absent** |
568-
| `master_detail` + `required` + `system` | the master-access gate | **422** | `MISSING_REQUIRED_FIELD` | **absent** |
569-
570-
Only the first row is the documented 400 shape. On the other two, messages are prefixed
571-
`[Security] Missing master reference:` and name the object and the field, but no `fields`
572-
array rides along.
572+
| `master_detail` + `required` + `readonly` | the master-access gate | **422** | `INVALID_METADATA` | **absent** |
573+
| `master_detail` + `required` + `system` | the master-access gate | **422** | `INVALID_METADATA` | **absent** |
574+
575+
Only the first row is a declaration the parse accepts. A master reference marked `readonly`
576+
or `system` is not one an author's write supplies, so it cannot be the master (ADR-0055 §1):
577+
`ObjectSchema` refuses both at parse (`master-not-writable`). An object reaches the runtime
578+
with one only through a door that does not parse — a raw `registerObject` / `registerApp`,
579+
`defineStack(…, { strict: false })`, or a stored object the boot registers under its
580+
`[metadata_spec_invalid]` diagnostic. There the gate refuses **every** write that sets the
581+
reference, not only one that leaves it out: every insert, single or batch, whether or not it
582+
names a master, and every update whose body carries the key, whatever the value, `null`
583+
included. The message is prefixed `[Security] Configuration defect:` and carries the parse's
584+
own refusal, naming the object, the field, the flag and the fix. A delete, and an update
585+
that leaves the key out, are answered as on any detail, so the rows already stored keep
586+
their master.
573587
574588
Two declarations are not rows of this table, because they name **no master at all**. A
575589
`controlled_by_parent` object derives its master from exactly one `master_detail` field
@@ -579,25 +593,16 @@ required `master_detail` fields. `ObjectSchema` refuses such an object at parse,
579593
one that reaches the runtime unparsed has every read denied and every write refused with
580594
`422 INVALID_METADATA`, whatever the request carries.
581595
582-
The same `422 MISSING_REQUIRED_FIELD` also answers an update or delete **by id** whose
583-
*stored* master reference is null, whatever the declaration. That is a different path: the
584-
caller supplied no such field, so there is no request field to name in `fields` and no
585-
payload that would fix it.
586-
587596
**Why the gate refuses rather than handing over.** Required-field validation skips
588597
provenance-flagged fields before its required check is reached (`system` and `readonly`
589598
fields are skipped outright). On those two shapes the gate is the only thing refusing the
590-
write, and letting it through was
591-
measured to create a detail record whose master reference is null — a record the
592-
`controlled_by_parent` read filter (`fk IN (readable masters)`) can never match, so it is
593-
readable by nobody and answers 422 on every later write by id. The refusal is correct; only
594-
its status departs from the rule above.
595-
596-
**These shapes are authorable today.** `os lint` refuses both at *error*
597-
(`relationship/master-detail-required`), but that rule is not part of the publish gate —
598-
`os build`, `os validate` and the metadata save door do not run it. So a stack can publish
599-
clean and still reach the 422. Branch on `code`, and read the status off the response rather
600-
than deriving it from this page.
599+
write, and letting it through was measured to store a detail record outside its master — a
600+
record the `controlled_by_parent` read filter (`fk IN (readable masters)`) can never match,
601+
so it is readable by nobody. No payload fixes the declaration; only its owner can, by
602+
dropping the flag.
603+
604+
Branch on `code`, and read the status off the response rather than deriving it from this
605+
page.
601606
602607
#### `INVALID_FIELD`
603608
**HTTP Status:** 400

0 commit comments

Comments
 (0)