Skip to content

Commit 3782b2d

Browse files
fix(plugin-security)!: a batch create of a controlled_by_parent detail asks for edit access on every row's master, and an update no longer empties the master reference (ADR-0055 §1, §3) (#25090)
Fixes #25049 Fixes #25068 Clause-②: no (narrowing) ## What changed ADR-0055 §3 requires edit access to the master for every write to a `controlled_by_parent` detail, and it reads a create's master from the master id in the incoming body. ADR-0055 §1 derives a detail's access from exactly one required master. The master-detail write gate (`assertControlledByParentWrite`, step 2.8) held neither on two write shapes. It now holds both. - **A batch create** (#25049). The engine runs the middleware chain once for an array of rows, and the gate read a create's master only from a body that is a single record. So a batch create asked about no master at all. The gate now asks about every master the rows name. It uses the same chain walk a single create runs (`assertControlledByParentChainEditable`, the one walk; no second copy), once per master, in row order, before anything is written. The first refusal is the answer, in the by-id leg's own envelope (`403 PERMISSION_DENIED`, naming the caller's detail and operation, leg recorded), and nothing is written. After the walk, the gate's empty-master branch answers the batch exactly as it did before. - **An update that empties the master reference** (#25068). On an update, a body that sets the reference to null names no master and empties the one the row's access derives from. The gate now answers it with its empty-master rule, the one an insert already gets. Where required-field validation provably judges the same field, the gate hands it to validation (the unchanged `400 VALIDATION_FAILED`). Otherwise it refuses with `422 MISSING_REQUIRED_FIELD` and nothing is written. It is asked after the stored masters' question, so a stored master the caller may not edit is still the answer. This covers all three update shapes: addressed by `where`, addressed in the body, and by predicate. - **The `[#25044]` docblock sentence**, "on every shape it asks EVERY master the write involves", is now true of the batch create too. The gate's docblock, the `#7474` list, the `#8688` paragraph, the freeze note's hand-over paragraph and the chain walk's docblock each gain the matching sentence. - No master scope is composed into any write. `computeControlledByParentFilter`, `packages/objectql/**` and `packages/spec/**` are untouched. Only `assertControlledByParentWrite` and its docblocks change. Files: - `packages/plugins/plugin-security/src/security-plugin.ts`: the gate only. - `packages/plugins/plugin-security/src/controlled-by-parent-every-row-master.test.ts` (new): 18 cases on a real stack. That is `ObjectQL` over better-sqlite3, the real `SecurityPlugin`, the real sharing service and middleware, the real protocol (`createManyData`, `insertManyData`), and contexts built by `resolveAuthzContext` and `assembleExecutionContext`, the agent's with the `mcp_agent_data_write` OAuth ceiling. - `.changeset/25049-cbp-every-row-master.md`: `@objectstack/plugin-security` `minor`. It is BREAKING under the launch-window convention for an accept-set narrowing. It carries the `Clause-②` line and the ADR-0087 disposition `not-required (no-migration-prescription)`. ## The door table (ADR-0055 §4's proof, public copy) Triage R555 asked for every read and write door to a `controlled_by_parent` detail. The table below is enumerated from the code. Its verdicts were measured on a real `ObjectQL` + `SqlDriver` + `SecurityPlugin`, for a member, an organization admin and an agent acting for a member, with throwaway probes that are not committed. Under the disclosure ruling recorded on #9387, no shape is named for a gap this PR leaves open. The full table is held privately by the seat. | # | door | reached from | master read | master edit, before | master edit, this PR | |---|---|---|---|---|---| | 1 | `find` / `findOne` / `count` / `aggregate` | REST list, get and export; the analytics read scope | yes, the read derivation | n/a | unchanged | | 2 | create, one record | REST create; the batch route's create and upsert rows; clone; import's per-row fallback | through the edit check | yes, the body's master | unchanged | | 3 | create, many records (array insert, `insertMany`) | REST `createMany`; import | **no**; now through the edit check | **no** | **yes**: every row's master, once each, in row order | | 4 | update by id, body leaves the reference out | REST update; the batch route; `updateMany` (one id at a time) | yes, the addressed row's read | yes, the stored master | unchanged | | 5 | update by id or by predicate, body names a master | the same doors | yes | yes, the stored master and the body's (PR #25066) | unchanged | | 6 | update by id or by predicate, body empties the reference | the same doors | yes | the stored master only; the emptied reference was not judged where validation does not judge it | **refused `422`** where validation does not judge it; validation's `400` where it does | | 7 | update by predicate, body leaves the reference out | engine callers | yes, the readable narrowing | yes, every selected row's master (PR #25042) | unchanged | | 8 | delete by id or by predicate | REST delete; the batch route; `deleteMany` (one id at a time) | yes | yes | unchanged | | 9 | record-grained explanation of update, delete, transfer, restore and purge; the attachment and comment parent gates | `security/explain`; attachments; comments | n/a | yes, the by-id update check | unchanged | | 10 | record-grained explanation of create | `security/explain` | n/a | not asked, by design: an explanation carries no body | unchanged | | 11 | system context (seeds, platform writes) | n/a | bypassed by design | bypassed by design | unchanged | - Row 3 is #25049's door, and row 6 is #25068's. - `transfer`, `restore` and `purge` never reach the engine's middleware as operations (the vocabulary pin in `packages/objectql`), so they are not doors here. - One further defect turned up during the enumeration. It lies outside the write gate and is not changed here. It goes to the seat privately, with options, for filing under the same ruling. ## Reach Reach was measured before this change on `origin/main` `2b1382a5b3`, with PR #25066's gate diff applied transiently and restored, and again on this head. Row 3 landed for all three callers through the engine and through the protocol's `createManyData` and `insertManyData`. Row 6 landed for all three callers on all three update shapes. On this head both are refused and nothing is written. The import door degrades a refused batch to one create per row. There, the rows under masters the caller may edit land, and the other row is refused on its own (measured). ## Verification Every run below ran under the shared verify lock. - **Build.** The dependency closure was built with `pnpm exec turbo run build --filter='@objectstack/dogfood^...' --concurrency=2`. That is a superset of `@objectstack/plugin-security^...` and includes plugin-security itself. Its tasks were 66 successful of 66, and it exited 0. - **The new pins** (at `1b27f979b4`). `controlled-by-parent-every-row-master.test.ts` passes 18/18. The package run below includes it, beside PR #25042's and PR #25066's suites. - **The package** (at `1b27f979b4`). `pnpm --filter @objectstack/plugin-security test`: Test Files 205 passed (205), Tests 4195 passed and 45 skipped (4240). `pnpm --filter @objectstack/plugin-security typecheck` exited 0. That covers `tsc` on both programs and `check:test-typecheck`. The new test file is in the `tsconfig.test.json` program (`--listFiles`, 1 hit). - **Dogfood battery** (`pnpm --filter @objectstack/dogfood test`, no shard and no path, owed by the `!` title). It ran on `d8dc243c15`, this head. The lock's verdict was command-exit 0. Test Files: 286 passed, 3 skipped (289). Tests: 2387 passed, 18 skipped (2405). - **Derived gate families.** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derives 69 families, the same list at `1b27f979b4` and at `d8dc243c15`. All 69 ran on `d8dc243c15`, after the final commit, each exit code captured before any pipe; all 69 exited 0. `--ran` reports: 69 derived, 69 run, 0 NOT-MEASURED, 0 UNRUN, a derived zero (every line carries its exit code). The same 69 also ran on `1b27f979b4`, where `check:dual-build-cjs-loads` answered PREREQUISITE NOT MET (eight unrelated packages had no `dist/`); it was rerun after building them, and passed. - **Ablation**, one leg per pinned behaviour. The fix was committed first. Each leg is a mutation made through `scripts/ablation-replace.mjs`, where the anchor must hit and the blob must change. Each restore is `git checkout HEAD --`, proven by blob equality with `HEAD` and an empty `git diff HEAD`. The subject is imported as `./security-plugin.js` from source, so no `dist/` sits on its path. The legs ran at `1b27f979b4`: | leg | mutation | went red | |---|---|---| | L1 | skip the walk for a batch row | the batch refusal pins (5), the count pin, and the refusal pin under a reference validation does not judge | | L2 | a blanket refusal in place of the walk | the lands-as-it-did pins (5), and 8 more: the refusal envelopes, the count pin, the no-master validation pin and the unjudged-reference pin | | L3 | drop the once-per-master skip | the count pin | | L4 | walk a row that names no master | the validation pin for a row naming no master | | L5 | let a batch stand down in the empty-master branch | the still-refused-whole pin | | L6 | drop the refusal of an emptied reference | the emptied-reference pins (3 callers) | | L7 | drop the hand-over to validation | the validation pin for an emptied reference | | L8 | judge the emptied reference before the stored master | the stored-master-first pin | The first attempts at L1, L2 and L8 were refused by the tool: each replacement contained its anchor, so the anchor count did not move. They measured nothing, and they were rerun with non-overlapping anchors (the table above). The unmutated control passed 18/18. The final tree is byte-identical to `HEAD`. - **Base.** The branch is `main` at `f46ef4d70e` (PR #25066 landed) plus this change, merged with `origin/main` `a39389eb51` (PR #25076, service-messaging, disjoint from this diff). ## Acceptance notes - **The partial-success `insertMany` now refuses the whole call** when one row names a master the caller may not edit, rather than reporting that row. The middleware chain runs once for the batch, so a per-row answer there would need the engine, which this PR does not touch. The import door's fallback to one create per row keeps import's per-row reporting. - **A batch row naming a master id with no row behind it** now meets the walk's `403` first, as a single create already does, where validation used to answer `400`. - **Under a reference required-field validation does not judge**, a batch create is still refused whole, exactly as before. It is now refused on the master-edit question first, when a row names a master the caller may not edit. Whether such a batch should instead be answered row by row, as a single create is, is left open. It goes to the seat as a decision, because it would admit writes this gate refuses today. - **On a write made on a user's behalf**, the agent's own pass runs first and whole. For the batch in the previous note, the agent's pass can therefore meet the empty-master answer before the member's pass asks the master. The write is refused either way, and nothing is written. This is pinned. - **The emptied-reference rule reads the body before the engine's field strips run**, as PR #25066's question does. On a reference the engine would strip from the update, a body that empties it is now refused, where before the key was dropped. That fails closed. Mirroring the engine's strip rules in the gate would make a second copy of them. A value of `undefined` is read as empty, which is how validation reads it on a judged reference. --- _Generated by [Claude Code](https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8f59304 commit 3782b2d

3 files changed

Lines changed: 616 additions & 8 deletions

File tree

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
'@objectstack/plugin-security': minor
3+
---
4+
5+
fix(plugin-security)!: a batch create of a `controlled_by_parent` detail asks for edit access on the master of every row it names, and an update no longer empties a detail's master reference (ADR-0055 §1, §3)
6+
7+
ADR-0055 §3 says creating, editing or deleting a detail requires edit access to its master, and it reads a create's master from the master id in the incoming body. A batch create carries one body per row. The master-detail write gate read a create's master only from a body that is a single record, so a batch create asked about no master at all. That covers the engine's array insert and `insertMany`, and the bulk-create and import doors that call them. The gate now asks about the master of every row the batch names, with the same check a single create runs, once per master and in row order. One master the caller may not edit refuses the whole batch, and nothing is written. On a write made on a user's behalf, both principals are asked, as on every other detail write.
8+
9+
ADR-0055 §1 derives a detail's access from exactly one required master. For a master reference that required-field validation does not judge, an update whose body set the reference to null landed. It stored a detail that no principal's read can match. The gate now refuses it as it already refuses a create that names no master.
10+
11+
Clause-②: no (narrowing)
12+
13+
<!-- adr-0087: not-required (no-migration-prescription) No metadata 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, so there is nothing for `objectstack migrate meta` to rewrite. What changes is the answer the security plugin's master-detail write gate gives for two shapes of write to a `controlled_by_parent` detail: a batch create now asks the master-edit question of the master each of its rows names, and refuses where that question refuses for any of them; and an update that empties a master reference required-field validation does not judge is refused. The remedy is a grant on the master, a batch kept within masters the caller may edit, or a master reference kept populated, never a rewrite of a package source or a stored row. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and nothing exported is removed or narrowed (not runtime-interface-only, type-surface-only or runtime-export-retired). -->
14+
15+
**BREAKING** for a caller that relied on either write landing. Shipped as `minor` under the launch-window convention for accept-set narrowings.
16+
17+
- **Before.**
18+
- A batch create of a detail landed whatever masters its rows named, including masters the caller may not edit.
19+
- An update that set a master reference, of a kind required-field validation does not judge, to null landed and stored the null.
20+
- **Now.**
21+
- That batch create answers the refusal a single create of the offending row answers: `PERMISSION_DENIED` (403), naming the detail and the master-edit reason. The partial-success `insertMany` refuses the whole call rather than reporting that row.
22+
- A batch whose rows' masters the caller may all edit lands as before and writes the same rows.
23+
- That update answers `MISSING_REQUIRED_FIELD` (422) and writes nothing. Where required-field validation does judge the reference, it still answers the update, as before.
24+
- **Unchanged.**
25+
- Single creates, and every update and delete that does not empty the master reference.
26+
- A row naming no master, which the same rules as before answer.
27+
- System contexts, the detail's read scope and the platform ownership floor.
28+
- Every object that does not declare `controlled_by_parent`.
29+
- The import door degrades a refused batch to one create per row. There, the rows under masters the caller may edit still land, and the other row is refused on its own.
30+
31+
**What to do.** Grant the caller edit access on every master a batch names, or keep each batch within masters the caller may edit. To move a detail to another master, write that master's id, never null.

0 commit comments

Comments
 (0)