diff --git a/.changeset/30905-cbp-optional-master.md b/.changeset/30905-cbp-optional-master.md new file mode 100644 index 00000000000..5b6a89d149a --- /dev/null +++ b/.changeset/30905-cbp-optional-master.md @@ -0,0 +1,26 @@ +--- +'@objectstack/spec': minor +'@objectstack/lint': minor +'@objectstack/plugin-security': minor +'@objectstack/objectql': minor +--- + +feat(spec, security): a `controlled_by_parent` object may declare an optional master, and a detail row with no master is read by its owner + +Clause-②: yes (widening) +Metadata-protocol: added +Skills-docs: follow-up owed — #30907 (card 3/3 of the ruling: the docs and skills that teach `controlled_by_parent` still say the master is always required); this diff teaches the optional master in ADR-0055 §1 and §2 as amended, in `os explain relationship/master-detail-required` and `os explain security-controlled-by-parent-no-relation`, and in the semantic migration entry `cbp-optional-master-admitted` + +A rule like "a rep sees the contacts of the accounts they own; a contact need not have an account" now has a declarative form. Declare the detail's only `master_detail` with `required: false`: + +```ts +fields: { + account: Field.masterDetail('crm_account', { label: 'Account', required: false }), +}, +sharingModel: 'controlled_by_parent', +``` + +- **`@objectstack/spec`: the optional master.** A `controlled_by_parent` object whose only `master_detail` is declared `required: false` passes the parse and `ObjectSchema.create()` with that value kept. `controlledByParentMasterOf(fields)` is exported from `@objectstack/spec/data` beside `controlledByParentMasterRefusal`: it answers the one master's field and whether it is optional. `isControlledByParentMasterField` still means the required master. Every other refusal stands: two masters, a required `lookup` only, and a `readonly` or `system` master. `ObjectSchema.create()` still forces an omitted `required` to `true`, and still refuses `required: false` on a `master_detail` beside another one. A stored row the v18 upgrade listed because its master declared `required: false` is valid now and no longer listed (`os migrate meta`); the semantic entry `cbp-optional-master-admitted` records it. The raw parse reads an omitted `required` as `false` and so admits it as the optional master; `os lint` reports the omission (below). +- **`@objectstack/plugin-security`: the null-master read arm.** For an optional master, the controlled-by-parent read filter is the unchanged master arm OR a second arm for rows whose master reference is empty: every such row for a holder of View All on the detail, else the caller's own (`owner_id`), else none (an object without an `owner_id` column). A row with no owner (a queue row) is View All's alone. The two arms never share a row, and View All on the detail does not open the master arm. Every read door composes the one filter, so the data API, analytics, search, export, the parent gates of attachments, comments, activity and approvals, chains and delegated reads follow. A required master's filter is unchanged. `security/explain` names the null-master arm on a record read. Writes are unchanged: an insert or update that leaves the master reference empty is still refused with 422. +- **`@objectstack/lint`.** `relationship/master-detail-required` passes the optional master, still reports `readonly` / `system` on it, and reports an omitted `required` on the only `master_detail` at `error`, naming both spellings (`required: true`, or `required: false` for an optional master). `security-controlled-by-parent-no-relation` and `-ambiguous-relation` resolve the optional master as the parse does. +- **`@objectstack/objectql`.** The boot refusal of a master the engine cannot compose a read over now judges an optional master's detail too. diff --git a/docs/adr/0055-master-detail-controlled-by-parent.md b/docs/adr/0055-master-detail-controlled-by-parent.md index c619458dd65..fe869c322d7 100644 --- a/docs/adr/0055-master-detail-controlled-by-parent.md +++ b/docs/adr/0055-master-detail-controlled-by-parent.md @@ -5,6 +5,7 @@ · **Amended** (2026-10-22, #24932, ruling B `6303721609` and Q1 C / Q2 A `6329027469`) — **§2's chosen mechanism (b) is retired for `controlled_by_parent`, and the set-size limit with it**: the read derivation (`packages/plugins/plugin-security/src/security-plugin.ts#computeControlledByParentFilter`) answers ADR-0139's `ReadableIds` leaf over the master, the engine composes it from this plugin's own `canReadObject` / `getReadFilter` of the master, and the driver compiles one statement with the master inside a subquery — the seventh consumer of that leaf, in the end-state form. No master id set is pre-resolved any more. The 2026-10-10 line above reads as of its date. What still stands: §1 (the one required `master_detail`), §3 (the write gate), the cycle guard and the depth bound (now the engine's own cut, judged first over the registry), and the RLS compiler's `rlsMembership` IN-form, which serves `current_user.` memberships and never served this derivation. The limit that replaces the set-size ceiling is ADR-0139 D5's two master shapes, which the boot refuses (#25144). · **Amended** (2026-10-26, #25760, ruling `6398566164`) — **§3 gains one per-object opt-in**: `sharing.controlledByParent.ownerMayEdit: true` lets the OWNER of a detail row (its `owner_id`) pass the write gate on a by-id `update` of that row with **read** access to the master; every other caller, and every insert, delete, predicate update and on-behalf-of write, keeps the master-**edit** rule. The default is unchanged. See **"Amendment (2026-10-26, #25760): the owner of a detail row may update it with read on the master, by opt-in"** at the end. · **Amended** (2026-10-27, #26591 / PR #26658) — **the 2026-10-26 amendment's Lint bullet is narrowed in place**: the lint prescribes the opt-in only for an `owner_id`-keyed `update` / `all` policy on a `controlled_by_parent` detail that carries `owner_id` and has not opted in; any other caller-keyed update policy is flagged in both states with the `owner_id` remedy, never the opt-in. The gate, the spec and the lint are unchanged by this note. See the note under **Lint** in "What the opt-in does, exactly". +· **Amended** (2026-11-06, #30905, maintainer ruling 「A」) — **§1 admits an optional master and §2 gains a null-master arm**: a `controlled_by_parent` object whose only `master_detail` is declared `required: false` derives an OPTIONAL master. A detail row that names its master is read through it, unchanged; a row whose master reference is empty is read by its owner (`owner_id`) and by View All on the detail, through a second arm OR-ed with the master arm inside the one read chokepoint, `computeControlledByParentFilter`. §3 (the write gate) is unchanged by this note: a write that leaves the reference empty is still refused. See **"Amendment (2026-11-06, #30905): an optional master, and the null-master row its owner reads"** at the end. **Deciders**: ObjectStack Protocol Architects **Builds on**: [ADR-0049](./0049-no-unenforced-security-properties.md) (enforce-or-remove), [ADR-0054](./0054-runtime-proof-for-authorable-surface.md) (prove-it-runs) **Surfaced by**: an audit of master-detail permission semantics — `OWDModel.controlled_by_parent` is **declared but unenforced** (zero runtime consumers; not reachable through the object's `sharingModel` enum; the RLS compiler is relationship-blind). This is *false compliance* (ADR-0049) and *unproven liveness* (ADR-0054). @@ -36,6 +37,7 @@ A **master-detail detail** record today is access-controlled entirely on its own - Add `controlled_by_parent` to the **authorable** `object.sharingModel` enum (converging it with `OWDModel`; resolves the inconsistency). - An object with `sharingModel: 'controlled_by_parent'` **must declare exactly one required `master_detail` field**; its `reference` identifies the master object and its field key is the master FK. Validation error otherwise (fail closed — an unsatisfiable "controlled by parent" must not silently fall open). + ⚠️ **Amended 2026-11-06 (#30905, ruling A):** or, with no required one, its **only** `master_detail` declared `required: false` — an **optional** master, whose rows may name none. Every other refusal stands (two masters, a required `lookup` only, a `readonly` / `system` master). See the amendment at the end. - The author writes **no RLS policy** for this — "controlled by parent" is *derived automatically* from the relationship (Salesforce-like OWD), which is the whole point. ### 2. Read mechanism — pre-resolved accessible-master-id set (chosen) @@ -47,6 +49,8 @@ For a `controlled_by_parent` object, the security layer, per request: This composes with the detail's own tenant/owner RLS (all AND-ed) and flows to analytics via `getReadFilter` unchanged. +⚠️ **Amended 2026-11-06 (#30905, ruling A):** for an **optional** master (§1 as amended), the derivation is one `$or` of two arms that never share a row — the master arm (now ADR-0139's leaf, per the 2026-10-22 amendment) and a **null-master** arm for rows whose master reference is empty, read by the row's owner and by View All on the detail. The `$or` is the cbp layer's own answer, so it is still AND-ed under the tenant wall and beside the detail's own layers. A required master's derivation is unchanged. See the amendment at the end. + **Why this mechanism (trade-offs):** | Option | Verdict | Why | @@ -190,3 +194,39 @@ The ruling names **update**. So the leg does not reach: an **insert** (no row to ### What is unchanged §1 (one required `master_detail`, no author-written derivation), §2 (read derived from the master), the chain walk, the cycle guard and the depth bound all stand. Every detail that does not declare the opt-in behaves byte-for-byte as before this amendment. + +--- + +## Amendment (2026-11-06, #30905): an optional master, and the null-master row its owner reads + +**Ruling.** objectstack-ai/objectstack#30905, maintainer ruling 「A」 (2026-11-06): `controlled_by_parent` admits an optional master, and a detail row with no master is visible only to its owner — the Salesforce reading of a contact with no account, a private contact. Rejected with it: a membership key collecting its object's own id, one-hop RLS traversal, and no platform change. The ruling lands in three serial changes; this amendment records the first (spec, lint, the read). The write gate (§3) and the docs follow in their own changes, and this record is amended again for §3 then. + +### The gap this closes + +"A rep sees the contacts of the accounts they own; a contact need not have an account" had no declarative form. §1 required the master reference on every row, the parse and `ObjectSchema.create()` refused a `master_detail` declared `required: false`, a `lookup` derives no access, and RLS cannot read the master's owner. The one working route was a copied owner field kept in step by hooks. + +### What changes, exactly + +- **Spec (§1).** The object's **only** `master_detail`, declared `required: false`, is its master when it declares no required one — an **optional** master. The parse's one-master rule (`packages/spec/src/data/controlled-by-parent-master.ts#controlledByParentMasterRefusal`) judges the master through `controlledByParentMasterOf`, exported beside it, which answers the field and whether it is optional; `isControlledByParentMasterField` keeps meaning the REQUIRED master. `ObjectSchema.create()` keeps an explicit `required: false` on the only `master_detail` as written; beside another `master_detail` it still refuses it, since that field is a second parent, not the master. Every other refusal stands: two masters, a required `lookup` only, and a `readonly` / `system` master (required or optional). +- **An omitted `required` keeps meaning required where the omission is visible.** The builder forces it to `true` and the stored-row conversion `cbp-master-detail-required-defaulted` converts it to `true`, both unchanged. The **parse** cannot see it: `required` defaults to `false` on the parsed field map, so a raw parse (and `defineStack` over a plain object literal) admits an omitted `required` on the only `master_detail` as the optional master. `os lint`'s `relationship/master-detail-required` reports the omission at `error`, naming both spellings. Whether the parse should tell the two spellings apart was ruled by the maintainer on #30905 (record `6687830530`, 2026-11-06): option A, the parse does not tell them apart, and this bullet implements that ruling; option B (refusing the omitted spelling at every door through a new raw-input check) and option C (a separate explicit key) were not chosen. +- **Compatibility.** No accepted stored object changes meaning. A stored row the v18 upgrade listed under `cbp-master-relation-tiers-retired` as `master-not-required` because it declared `required: false` is a valid optional-master detail now and is no longer listed; the semantic migration entry `cbp-optional-master-admitted` records it. +- **Read (§2).** `packages/plugins/plugin-security/src/security-plugin-cbp-read-filter.ts#computeControlledByParentFilter` answers an optional master with `{ $or: [ { fk: { $in: ReadableIds.of(master) } }, nullMasterArm ] }`. The null-master arm is `{ fk: null }` for a holder of View All on the detail (`viewAllRecords` or `modifyAllRecords`, the super-user read bypass on the detail), else `{ $and: [ { fk: null }, { owner_id: } ] }` when the detail carries the `owner_id` column, else nothing. The function is the one chokepoint: the data middleware, `getReadFilter` (analytics, and the engine's `ReadableIds` composer behind the parent gates of attachments, comments, activity and approvals, and behind a chain's next level) and the delegated read compose it, each with the permission sets of the context it is asked for, so every read door follows. A required master's leaf is byte-identical to before. +- **Explain.** `security/explain` narrates the null-master arm on a record read: which arm returned the row, or, for a row the read door withholds, that it names no master and who reads it instead (`packages/plugins/plugin-security/src/explain-engine.ts`). + +### The rules the read keeps + +- A row that names a master is never read through the null-master arm, and a row that names none never through the master arm (`$in` never matches an empty reference). +- View All on the detail admits the null-master rows and nothing more: it does not open the master arm, which stays the master's own read answer. +- A null-master row with no owner (an ADR-0142 queue row), or on an object with no `owner_id` column (`ownership` other than `'user'`), is read by View All alone until it is claimed. +- In a chain, an intermediate row whose own master is empty is read by its owner, and the next level's master arm sees exactly that: its leaf is composed with the intermediate level's own read scope, the `$or` included. +- A caller who reads another master does not read the null-master rows they do not own. + +### Fail-closed boundaries + +- An omitted `required` on an object that reached the registry without the parse resolves no master (the runtime reads `required === false`, not its absence), so every read is denied, as before. +- A cycle or a depth refusal of the chain still denies the whole object, the null-master rows included: the declared chain is the defect. +- The write gate is unchanged: an insert or update that leaves the master reference empty is refused (`MasterReferenceMissingError`, 422), so no null-master row can be written through the platform until §3 is amended. + +### What is unchanged + +§3, the chain walk, the cycle guard and the depth bound; every `controlled_by_parent` object with a required master, byte for byte; the owner opt-in of the 2026-10-26 amendment. diff --git a/packages/lint/src/data-model-rules.master-detail-required.test.ts b/packages/lint/src/data-model-rules.master-detail-required.test.ts index 86f32ed75c9..4413761a5ff 100644 --- a/packages/lint/src/data-model-rules.master-detail-required.test.ts +++ b/packages/lint/src/data-model-rules.master-detail-required.test.ts @@ -6,7 +6,9 @@ * On a `sharingModel: 'controlled_by_parent'` object the master reference is * what the object's record access is derived through, and three declarable * shapes leave the security gate as the only thing refusing a detail record - * saved without its master: `required` absent (or `false`), `required: true` + + * saved without its master: `required` absent (or `false` beside another + * `master_detail` — the only one declared `false` is the optional master since + * #30905 ruling A, and passes), `required: true` + * `readonly: true`, and `required: true` + `system: true` (record validation * never checks a non-required field and skips readonly/system fields before its * required check). All three are refused here at `error` — the v18 narrowing of @@ -62,9 +64,13 @@ const masterRef = (shape: Record) => ({ const UNSAFE_SHAPES: Array<[label: string, shape: Record, path: string]> = [ ['`required` absent', {}, 'objects[1].fields.order.required'], - ['`required: false`', { required: false }, 'objects[1].fields.order.required'], + ['`required` not a boolean', { required: 'false' }, 'objects[1].fields.order.required'], ['`required: true` + `readonly: true`', { required: true, readonly: true }, 'objects[1].fields.order.readonly'], ['`required: true` + `system: true`', { required: true, system: true }, 'objects[1].fields.order.system'], + // [#30905 ruling A] The optional master is a master too: the flags still + // leave its reference unwritable. + ['`required: false` + `readonly: true`', { required: false, readonly: true }, 'objects[1].fields.order.readonly'], + ['`required: false` + `system: true`', { required: false, system: true }, 'objects[1].fields.order.system'], ]; describe('R2 on a controlled_by_parent object — the three unsafe shapes are refused at error', () => { @@ -80,6 +86,33 @@ describe('R2 on a controlled_by_parent object — the three unsafe shapes are re expect(r2(stack('controlled_by_parent', masterRef({ required: true })))).toEqual([]); }); + // [#30905 ruling A · ADR-0055 §1 as amended] The object's ONLY + // master_detail, declared `required: false`, is its optional master. + it('the optional master — the only master_detail, an explicit `required: false` — passes', () => { + expect(r2(stack('controlled_by_parent', masterRef({ required: false })))).toEqual([]); + expect( + r2(stack('controlled_by_parent', masterRef({ required: false, readonly: false, system: false }))), + ).toEqual([]); + }); + + it('an explicit `required: false` beside another master_detail is still reported — it is not the master', () => { + const issues = r2( + stack('controlled_by_parent', { + order: { type: 'master_detail', reference: 'work_order', required: true }, + other: { type: 'master_detail', reference: 'work_order', required: false }, + }), + ); + expect(issues.map((issue) => [issue.severity, issue.path, issue.fix])).toEqual([ + ['error', 'objects[1].fields.other.required', 'required: true'], + ]); + }); + + it('an omitted `required` on the only master_detail names both spellings, and fixes to `required: true`', () => { + const [issue] = r2(stack('controlled_by_parent', masterRef({}))); + expect(issue).toMatchObject({ severity: 'error', fix: 'required: true' }); + expect(issue!.message).toContain('declares no required: write true (every detail names its master) or false (an optional master)'); + }); + it('CONTROL: an explicit `readonly: false` / `system: false` is clean — the flag, not the key, is the defect', () => { expect( r2(stack('controlled_by_parent', masterRef({ required: true, readonly: false, system: false }))), @@ -150,7 +183,8 @@ describe('R2 outside controlled_by_parent — the verdict is unchanged', () => { // What reaches the rule from the authoring builder. Under // `controlled_by_parent`, `ObjectSchema.create()` forces an omitted `required` -// to `true` and refuses an explicit `false`; the parse it ends in refuses a one +// to `true` and keeps an explicit `false` on the only `master_detail` (the +// optional master, #30905 ruling A; beside another it refuses it); the parse it ends in refuses a one // master marked `readonly` or `system` (spec shape `master-not-writable`), so // the two flagged shapes no longer leave the builder, and this rule reports // them on a stack that never went through it. @@ -187,6 +221,12 @@ describe('R2 over objects built by ObjectSchema.create()', () => { const detail = build(Field.masterDetail('work_order', { label: 'Work Order' })); expect(r2([{ name: 'work_order', sharingModel: 'private', fields: {} }, detail])).toEqual([]); }); + + it('CONTROL: the builder keeps an explicit `required: false` (the optional master), and it lints clean', () => { + const detail = build(Field.masterDetail('work_order', { label: 'Work Order', required: false })); + expect((detail.fields as Record).order!.required).toBe(false); + expect(r2([{ name: 'work_order', sharingModel: 'private', fields: {} }, detail])).toEqual([]); + }); }); describe('[#22161] one-line verdicts — relationship/master-detail-required, relationship/delete-behavior', () => { @@ -195,14 +235,30 @@ describe('[#22161] one-line verdicts — relationship/master-detail-required, re expect(RELATIONSHIP_DELETE_BEHAVIOR).toBe('relationship/delete-behavior'); }); - it('each controlled_by_parent shape reads as one sentence ending in the one shared consequence', () => { + it('each controlled_by_parent shape reads as one sentence: the shared consequence, or the two spellings', () => { const verdict = (shape: Record) => r2(stack('controlled_by_parent', masterRef(shape)))[0]!.message; const consequence = 'a controlled_by_parent detail saved without its master is readable by nobody'; - expect(verdict({})).toBe(`master_detail "work_order_item.order" → work_order must be required: ${consequence}`); + // [#30905] The sole master_detail's omitted `required` names its two spellings. + expect(verdict({})).toBe( + 'master_detail "work_order_item.order" → work_order declares no required: write true (every detail ' + + 'names its master) or false (an optional master)', + ); expect(verdict({ readonly: true, system: true })).toBe( - 'master_detail "work_order_item.order" → work_order must be required and not marked readonly or ' + - `system: ${consequence}`, + 'master_detail "work_order_item.order" → work_order declares no required and is marked readonly or ' + + 'system: write required: true or false, and drop the flag', + ); + // Beside another master_detail, `false` is not the master: the required consequence stands. + const beside = r2( + stack('controlled_by_parent', { + lead: { type: 'master_detail', reference: 'work_order', required: true }, + order: { type: 'master_detail', reference: 'work_order', required: false }, + }), + )[0]!.message; + expect(beside).toBe(`master_detail "work_order_item.order" → work_order must be required: ${consequence}`); + expect(verdict({ required: false, readonly: true })).toBe( + 'master_detail "work_order_item.order" → work_order is the optional master and is marked readonly, ' + + 'which no detail write supplies: the master reference must be writable', ); expect(verdict({ required: true, readonly: true })).toBe( 'master_detail "work_order_item.order" → work_order is marked readonly, which skips its required ' + @@ -246,6 +302,10 @@ describe('[#22161] one-line verdicts — relationship/master-detail-required, re '`assertControlledByParentWrite`', '`ObjectSchema.create()`', 'never a publish verdict', + // [#30905 ruling A] The optional master and the two spellings. + 'optional master (ADR-0055 §1 as amended)', + 'read by its owner (`owner_id`) and by a holder of View All on the detail', + 'Write `required: true` (every detail names its master) or `required: false`', ], [RELATIONSHIP_DELETE_BEHAVIOR]: [ 'an unset one included', diff --git a/packages/lint/src/data-model-rules.ts b/packages/lint/src/data-model-rules.ts index 064313c38fc..63d9e3df2a3 100644 --- a/packages/lint/src/data-model-rules.ts +++ b/packages/lint/src/data-model-rules.ts @@ -336,6 +336,19 @@ const CBP_ORPHAN_CONSEQUENCE = * 2. `required: true` + `readonly: true`; * 3. `required: true` + `system: true`. * + * ⚠️ Amended by ruling A of #30905 (ADR-0055 §1 as amended): the object's + * ONLY `master_detail`, declared `required: false`, is its optional master — a + * detail row may name none, and such a row is read by its owner and by View + * All on the detail — so that spelling passes, and only `readonly` / `system` + * on it is reported (the parse refuses it too, shape `master-not-writable`). + * Shape 1 now reads: `required` absent (or not a boolean) on any + * `master_detail`, or `false` on one beside another `master_detail`. On the + * sole `master_detail` an absent `required` is reported with both honest + * spellings, because the parse cannot see the omission (it reads the parsed + * `false`, so it admits the optional master) while `ObjectSchema.create()` + * forces it to `true`: one source, two meanings, until the author writes one. + * The fix stays `required: true`, the reading the builder already gives it. + * * Why these three. A `controlled_by_parent` detail derives ALL of its record * access from the master its `master_detail` reference names (ADR-0055: such an * object "must declare exactly one required `master_detail` field"). Record @@ -394,21 +407,41 @@ function cbpMasterReferenceFinding( fieldPath: string, parent: string, def: any, + masterDetailCount: number, ): LintIssue | undefined { - const missingRequired = def.required !== true; + // [ADR-0055 §1 as amended · #30905 ruling A] The object's ONLY + // `master_detail`, declared `required: false`, is its optional master — the + // spelling passes. Beside another `master_detail` an explicit `false` is + // judged as before; an omitted `required` is reported on every field. + const soleMasterDetail = masterDetailCount === 1; + const optionalMaster = soleMasterDetail && def.required === false; + const missingRequired = def.required !== true && !optionalMaster; + // On the sole `master_detail`, an omitted (or non-boolean) `required` has + // two honest spellings; elsewhere only `required: true` is one. + const eitherSpelling = missingRequired && soleMasterDetail; const flags = (['readonly', 'system'] as const).filter((flag) => def[flag] === true); if (!missingRequired && flags.length === 0) return undefined; const subject = `master_detail "${objectName}.${fieldName}" → ${parent}`; // [#22161] One verdict per shape, ending in the one shared consequence; the // derivation (why the record is unreadable, why the flags skip the check) - // is `os explain` text. - const message = missingRequired + // is `os explain` text. [#30905] The sole master_detail's omitted + // `required` names its two spellings instead: the consequence is the + // required one's only, and which one is meant is the author's to write. + const message = eitherSpelling + ? flags.length === 0 + ? `${subject} declares no required: write true (every detail names its master) or false (an optional master)` + : `${subject} declares no required and is marked ${flags.join(' or ')}: write required: true or false, ` + + 'and drop the flag' + : missingRequired ? flags.length === 0 ? `${subject} must be required: ${CBP_ORPHAN_CONSEQUENCE}` : `${subject} must be required and not marked ${flags.join(' or ')}: ${CBP_ORPHAN_CONSEQUENCE}` - : `${subject} is marked ${flags.join(' and ')}, which skips its required check: ` + - CBP_ORPHAN_CONSEQUENCE; + : optionalMaster + ? `${subject} is the optional master and is marked ${flags.join(' and ')}, which no detail ` + + 'write supplies: the master reference must be writable' + : `${subject} is marked ${flags.join(' and ')}, which skips its required check: ` + + CBP_ORPHAN_CONSEQUENCE; return { severity: 'error', @@ -1016,7 +1049,10 @@ export function lintDataModel(objects: any[]): LintIssue[] { // `master_detail` is a likely-wrong choice and stays a `warning`, // byte-for-byte as before. if (obj.sharingModel === 'controlled_by_parent') { - const finding = cbpMasterReferenceFinding(obj.name, fieldName, fieldPath, parent, def); + const masterDetailCount = fields.filter(({ def: f }) => f?.type === 'master_detail').length; + const finding = cbpMasterReferenceFinding( + obj.name, fieldName, fieldPath, parent, def, masterDetailCount, + ); if (finding) issues.push(finding); } else if (def.required !== true) { issues.push({ diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 794d736b209..77f338af03f 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -719,8 +719,10 @@ const SECURITY_CBP_NO_RELATION_EXPLANATION: RuleExplanation = { paragraphs: [ 'A `controlled_by_parent` object says its access comes from its master. ADR-0055 §1 names that ' + 'master by exactly one `master_detail` field declared `required: true`, with a `reference` to the ' + - 'master object, and by nothing else: a `master_detail` without `required: true` and a required ' + - '`lookup` derive no access.', + 'master object, or, as amended, with no such field, by the object\'s ONLY `master_detail` declared ' + + '`required: false`, an optional master (a detail row with an empty master reference is read by its ' + + 'owner and by View All on the detail). Nothing else names it: a `master_detail` that declares no ' + + '`required`, one declared `required: false` beside another, and a required `lookup` derive no access.', 'Both runtime halves refuse an object without one: every read is DENIED, and every write is refused ' + 'with 422 INVALID_METADATA — as a metadata defect rather than a permission denial — so the object ' + 'is unusable rather than merely locked down. System objects are judged too, because the runtime ' + @@ -3309,16 +3311,26 @@ const RELATIONSHIP_MASTER_DETAIL_REQUIRED_EXPLANATION: RuleExplanation = { '`masterFK IN (accessible master ids)` — never matches an empty master, so a detail saved ' + 'without one is readable by nobody and refused on every later write. Record validation never ' + 'checks a field that is not `required`, and skips `readonly` and `system` fields before its ' + - 'required check, so on the three shapes reported — `required` absent or false, ' + + 'required check, so on the three shapes reported — `required` absent, or false beside another ' + + '`master_detail`, ' + '`required: true` with `readonly: true`, and `required: true` with `system: true` — the ' + 'security gate (`assertControlledByParentWrite`) is the only thing refusing such an insert.', + 'One spelling passes: the object\'s ONLY `master_detail`, declared `required: false`, is an ' + + 'optional master (ADR-0055 §1 as amended). A detail row with an empty master reference is read ' + + 'by its owner (`owner_id`) and by a holder of View All on the detail, by nobody else; a row ' + + 'that names a master is read through it as before. (The security gate still refuses a write ' + + 'that leaves the reference empty.) `readonly` or `system` on it is ' + + 'still reported. An omitted `required` is reported on every `master_detail`: the spec parse ' + + 'reads the parsed `false` and admits it as the optional master, while `ObjectSchema.create()` ' + + 'forces it to `true`, so one source would mean two things. Write `required: true` (every detail ' + + 'names its master) or `required: false` (a detail may name none).', 'Every `master_detail` field of such an object is judged, not only the one the runtime resolves ' + - 'as the master: that is the scope `ObjectSchema.create()` enforces when it forces ' + - '`required: true` and refuses an explicit `required: false`. The spec parse also ' + - 'refuses every shape that leaves the object without exactly one writable required master, so ' + - '`os validate`, `os build` and the save door stop there; a shape on a second, non-required ' + - '`master_detail` beside the master is this rule\'s report alone. One finding per field, located ' + - 'at its first defect.', + 'as the master: that is the scope `ObjectSchema.create()` enforces when it forces an omitted ' + + '`required` to `true` and refuses an explicit `required: false` beside another `master_detail`. ' + + 'The spec parse also refuses every shape that leaves the object without exactly one writable ' + + 'master, so `os validate`, `os build` and the save door stop there; a shape on a second, ' + + 'non-required `master_detail` beside the master is this rule\'s report alone. One finding per ' + + 'field, located at its first defect.', DATA_MODEL_SWEEP_REACH, ], }; diff --git a/packages/lint/src/validate-security-posture-extension-merge.test.ts b/packages/lint/src/validate-security-posture-extension-merge.test.ts index b8368d94026..8b959688577 100644 --- a/packages/lint/src/validate-security-posture-extension-merge.test.ts +++ b/packages/lint/src/validate-security-posture-extension-merge.test.ts @@ -77,7 +77,6 @@ describe('[#23210] validateSecurityPosture judges a controlled_by_parent object it('an extension that demotes the one master is no-relation at the extension, in the words of the declared control', () => { for (const demoted of [ - { type: 'master_detail', reference: 'cbpx_header', required: false }, { type: 'text' }, { type: 'lookup', reference: 'cbpx_header', required: true }, ]) { @@ -91,6 +90,18 @@ describe('[#23210] validateSecurityPosture judges a controlled_by_parent object } }); + // [#30905 ruling A · ADR-0055 §1 as amended] Declared `required: false`, + // the only master_detail is the OPTIONAL master: the merged object resolves, + // as the same object declared directly does. Beside a second master_detail + // with no `required: true`, it names no master. + it('an extension that declares the one master required: false makes it the optional master — silent, as its declared twin', () => { + const optional = { type: 'master_detail', reference: 'cbpx_header', required: false }; + expect(cbpFindings(extending({ header: optional }))).toEqual([]); + expect(cbpFindings({ objects: OBJECTS({ header: optional }) })).toEqual([]); + const findings = cbpFindings(extending({ header: optional, other: { ...OTHER, required: false } })); + expect(findings.map((f) => [f.rule, f.path])).toEqual([[SECURITY_CBP_NO_RELATION, 'objectExtensions[0].fields']]); + }); + it('an extension whose target the stack does not declare is unjudged; the visible twin is reported', () => { expect(cbpFindings(extending({ a: OTHER, b: HEADER }, 'crm_line'))).toEqual([]); expect(cbpFindings(extending({ other: OTHER }, 'cbpx__cbpx_line'))).toEqual([]); @@ -137,7 +148,9 @@ describe('[#23210] the parse and the lint agree on the merged shape', () => { it.each<[string, Record, string[]]>([ ['the card\'s stack', CARD_STACK, ['objectExtensions.0.fields']], ['a non-master field', extending({ other: { type: 'text' } }), []], - ['a demoted master', extending({ header: { type: 'master_detail', reference: 'cbpx_header', required: false } }), ['objectExtensions.0.fields']], + // [#30905 ruling A] Declared `required: false`, the one master is the optional master. + ['a master made optional', extending({ header: { type: 'master_detail', reference: 'cbpx_header', required: false } }), []], + ['a demoted master', extending({ header: { type: 'text' } }), ['objectExtensions.0.fields']], ['an unseen target', extending({ other: OTHER }, 'crm_line'), []], ['a non-controlled target', { ...CARD_STACK, objects: OBJECTS({ header: HEADER }, 'private') }, []], [ diff --git a/packages/lint/src/validate-security-posture.test.ts b/packages/lint/src/validate-security-posture.test.ts index 714303395f0..bf8876fde80 100644 --- a/packages/lint/src/validate-security-posture.test.ts +++ b/packages/lint/src/validate-security-posture.test.ts @@ -1101,6 +1101,39 @@ describe('validateSecurityPosture · controlled_by_parent with no relation (#750 expect(authoring[0]).toMatchObject({ severity: 'error', path: 'objects[1].fields.order.required' }); }); + // [#30905 ruling A · ADR-0055 §1 as amended] The only master_detail, + // declared `required: false`, is the optional master: it resolves. + it('stays silent on the OPTIONAL master — the only master_detail, declared required: false', () => { + expect( + cbpOnly(cbpStack({ order: { name: 'order', type: 'master_detail', reference: 'work_order', required: false } })), + ).toEqual([]); + expect( + cbpOnly(cbpStack([{ name: 'order', type: 'master_detail', reference: 'work_order', required: false }])), + ).toEqual([]); + }); + + it('errors on two master_details declared required: false — neither is the one master', () => { + expect( + cbpOnly( + cbpStack({ + a: { name: 'a', type: 'master_detail', reference: 'work_order', required: false }, + b: { name: 'b', type: 'master_detail', reference: 'work_order', required: false }, + }), + ), + ).toHaveLength(1); + }); + + it('a required master beside a master_detail declared required: false: the required one is the master', () => { + expect( + cbpOnly( + cbpStack({ + order: { name: 'order', type: 'master_detail', reference: 'work_order', required: true }, + other: { name: 'other', type: 'master_detail', reference: 'work_order', required: false }, + }), + ), + ).toEqual([]); + }); + // Step 3 used to be pinned silent here; it is HotCRM's // `crm_campaign_member` / `crm_event_attendee` / `crm_article_feedback` shape. it('errors on a REQUIRED lookup (the retired third tier)', () => { @@ -1169,7 +1202,16 @@ describe('validateSecurityPosture · controlled_by_parent with no relation (#750 // ── ONE rule, read by the parse and by this linter — held equal ───────── it.each([ ['one required master_detail', { order: { type: 'master_detail', label: 'O', reference: 'work_order', required: true } }, []], - ['a non-required master_detail', { order: { type: 'master_detail', label: 'O', reference: 'work_order' } }, ['sharingModel']], + // [#30905 ruling A] The optional master: accepted by both. + ['the only master_detail, required: false', { order: { type: 'master_detail', label: 'O', reference: 'work_order', required: false } }, []], + [ + 'two master_details, neither required', + { + a: { type: 'master_detail', label: 'A', reference: 'work_order', required: false }, + b: { type: 'master_detail', label: 'B', reference: 'work_order' }, + }, + ['sharingModel'], + ], ['a required lookup', { order: { type: 'lookup', label: 'O', reference: 'work_order', required: true } }, ['sharingModel']], ['no relation', { note: { type: 'text', label: 'N' } }, ['sharingModel']], [ @@ -1193,6 +1235,33 @@ describe('validateSecurityPosture · controlled_by_parent with no relation (#750 }); }); +// [#30905 ruling A] The one recorded divergence between the parse and this +// linter. The parse reads the PARSED map, where an omitted `required` is +// `false`, so it admits the only master_detail without one as the optional +// master; this linter reads the authored shape, sees the omission, and +// resolves no master (the runtime's unparsed read agrees); the data-model rule +// reports it at `error` with both spellings. `ObjectSchema.create()` reads the +// omission as `required: true`. +describe('validateSecurityPosture · an omitted required on the only master_detail (#30905 ruling A)', () => { + it('the parse admits it as optional; this linter and the data-model rule refuse it', () => { + const fields = { order: { type: 'master_detail', label: 'O', reference: 'work_order' } }; + const child = { name: 'work_order_item', label: 'Work Order Item', sharingModel: 'controlled_by_parent', fields }; + expect(cbpParseIssuePaths(child)).toEqual([]); + const stack = { + objects: [ + { name: 'work_order', label: 'Work Order', sharingModel: 'private', fields: { name: { name: 'name', label: 'Name' } } }, + child, + ], + }; + expect(validateSecurityPosture(stack).filter((f) => f.rule === SECURITY_CBP_NO_RELATION)).toHaveLength(1); + const authoring = lintDataModel(stack.objects as any[]).filter( + (issue) => issue.rule === 'relationship/master-detail-required', + ); + expect(authoring).toHaveLength(1); + expect(authoring[0]).toMatchObject({ severity: 'error', path: 'objects[1].fields.order.required' }); + }); +}); + // ── Rule: security-controlled-by-parent-ambiguous-relation (#14747 / #23100) ── // // The mirror image of the rule above: not ZERO required master_detail fields, @@ -2021,6 +2090,9 @@ describe('validateSecurityPosture — reads only keys the spec declares (meta-te // #30303: the shared extension fold's own collections (`object-graph.ts`, scanned with this rule) — // JS Map / Set / Array methods, and `other.carrier`, the fold's own record of a package's entry. 'byTarget', 'kept', 'other', 'list', 'lastWriter', 'tiedKeys', 'tied', + // #30905: `cbpMasterCandidates`' required-master list (`.length`, a JS property) and the spec + // resolver's answer (`controlledByParentMasterOf`: `.field` / `.optional`, fields of its own return value). + 'requiredMasters', 'master', ]); expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]); }); diff --git a/packages/lint/src/validate-security-posture.ts b/packages/lint/src/validate-security-posture.ts index d87715223f0..311921436c2 100644 --- a/packages/lint/src/validate-security-posture.ts +++ b/packages/lint/src/validate-security-posture.ts @@ -122,7 +122,12 @@ */ import { parseCelToAst, sqlPredicateToCel, type CelAstNode } from '@objectstack/formula'; -import { controlledByParentOwnerMayEdit, isControlledByParentMasterField, referenceCarrierOf } from '@objectstack/spec/data'; +import { + controlledByParentMasterOf, + controlledByParentOwnerMayEdit, + isControlledByParentMasterField, + referenceCarrierOf, +} from '@objectstack/spec/data'; import { describeAnchorForbiddenBits, type AnchorBindingContext } from '@objectstack/spec/security'; import { indexObjectGraph, @@ -375,6 +380,12 @@ interface CbpRelation { * `security-controlled-by-parent-no-relation`, two or more is * `security-controlled-by-parent-ambiguous-relation`. * + * ⚠️ Amended by ruling A of #30905 (ADR-0055 §1 as amended): with none + * declared `required: true`, the object's ONLY `master_detail`, declared + * `required: false` (read `=== false`, so an omitted `required` is not it), is + * the one candidate, an optional master. The spec's `controlledByParentMasterOf` + * picks it, as the parse does. + * * This used to be a three-tier table (`CBP_TIERS`, #7503 / #14747) mirroring a * three-tier fallback in the runtime: a required `master_detail`, else ANY * `master_detail`, else a required `lookup`. ADR-0055 §1 never recorded the @@ -400,9 +411,21 @@ interface CbpRelation { * names the real defect (the alias key) rather than this rule guessing past it. */ function cbpMasterCandidates(obj: AnyRec): CbpRelation[] { - return recordsOf(obj.fields) - .filter((f) => isControlledByParentMasterField(f) && refOf(f)) - .map((f) => ({ field: String(f.name ?? '?'), type: 'master_detail', master: refOf(f) as string })); + const fields = recordsOf(obj.fields); + const relationOf = (f: AnyRec): CbpRelation => ({ + field: String(f.name ?? '?'), + type: 'master_detail', + master: refOf(f) as string, + }); + const requiredMasters = fields.filter((f) => isControlledByParentMasterField(f) && refOf(f)).map(relationOf); + if (requiredMasters.length > 0) return requiredMasters; + // [ADR-0055 §1 as amended · #30905 ruling A] With no required one, the + // object's only `master_detail` declared `required: false` is its optional + // master — the spec's resolver says which, so the parse and this linter + // still read one sentence. + const master = controlledByParentMasterOf(Object.fromEntries(fields.map((f) => [String(f.name ?? '?'), f]))); + const optional = master?.optional ? fields.find((f) => String(f.name ?? '?') === master.field) : undefined; + return optional && refOf(optional) ? [relationOf(optional)] : []; } /** @@ -636,7 +659,7 @@ function pushCbpNoRelation(findings: SecurityFinding[], objName: string, path: s // the object is unusable rather than locked down are this id's // `os explain` entry. message: - `declares sharingModel 'controlled_by_parent' but no required master_detail field to derive access ` + + `declares sharingModel 'controlled_by_parent' but no master_detail field it can derive access ` + `from, so every read is denied and every write refused (422 INVALID_METADATA)`, // [#22161] One remedy line; that an object with no master decides its // own baseline, and what each value means, is this id's `os explain` diff --git a/packages/metadata-protocol/src/protocol.stored-migration.test.ts b/packages/metadata-protocol/src/protocol.stored-migration.test.ts index a86e6e095b3..034c8cc6891 100644 --- a/packages/metadata-protocol/src/protocol.stored-migration.test.ts +++ b/packages/metadata-protocol/src/protocol.stored-migration.test.ts @@ -18,6 +18,11 @@ * it declines to touch says so instead of being silently counted clean. */ import { describe, expect, it } from 'vitest'; +// The protocol's diagnostics load `@objectstack/lint` with a dynamic +// `import()` (`metadata-diagnostics.ts`). Paid here, at collection, so no +// case's `testTimeout` window pays that first load +// (`scripts/check-test-source-alias.mjs`, "The source reach"). +import '@objectstack/lint'; // [#5619] The producer's OWN write-verb dispatch decisions (#4550 delete / // #5480 update), so the fake engine below cannot accept a call ObjectQL // refuses. Imported from `@objectstack/metadata-core` and not from @@ -1120,6 +1125,10 @@ describe('migrateStoredMetadata — the v18 stored shapes refused with no lossle }; it('#23270 — each refused controlled_by_parent shape is listed by object; a conforming object is counted canonical', async () => { + // [ADR-0055 §1 as amended · #30905 ruling A] `crm_false` — its only + // `master_detail` declared `required: false` — is an optional master + // now, so it is counted canonical beside `crm_ok`, no longer listed. + // Its twin `crm_two` (two masters) still is. const { engine, tables } = makeStubEngine([ cbpObject('crm_two', { quote: md('crm_quote', { required: true }), order: md('crm_order', { required: true }) }), cbpObject('crm_none', {}), @@ -1132,18 +1141,17 @@ describe('migrateStoredMetadata — the v18 stored shapes refused with no lossle const report = await protocol.migrateStoredMetadata(); - expect(report.canonical).toBe(1); + expect(report.canonical).toBe(2); expect(report.rows.map((r) => [r.name, r.outcome, r.todos.map((t) => [t.conversionId, t.path])])).toEqual([ ['crm_two', 'skipped', [['cbp-master-relation-tiers-retired', 'objects[0].fields']]], ['crm_none', 'skipped', [['cbp-master-relation-tiers-retired', 'objects[0].sharingModel']]], ['crm_lookup', 'skipped', [['cbp-master-relation-tiers-retired', 'objects[0].sharingModel']]], - ['crm_false', 'skipped', [['cbp-master-relation-tiers-retired', 'objects[0].sharingModel']]], ]); const text = formatStoredMigrationReport(report).join('\n'); expect(text).toContain('object/crm_two [env-wide]'); expect(text).toContain('TODO cbp-master-relation-tiers-retired:'); expect(text).toMatch(/which of them is the master is the owner's choice: the upgrade never picks one/); - expect(text).toMatch(/☐ TODO: 4 site\(s\) in 4 row\(s\) are left as stored/); + expect(text).toMatch(/☐ TODO: 3 site\(s\) in 3 row\(s\) are left as stored/); // Listed, never written — on --apply too — and no verdict flips over it. expect(storedMigrationClean(report)).toBe(true); const applied = await protocol.migrateStoredMetadata({ apply: true }); diff --git a/packages/objectql/src/engine-readable-ids-permission-formula.ts b/packages/objectql/src/engine-readable-ids-permission-formula.ts index 5d67025ab7b..34d88be6d08 100644 --- a/packages/objectql/src/engine-readable-ids-permission-formula.ts +++ b/packages/objectql/src/engine-readable-ids-permission-formula.ts @@ -64,7 +64,7 @@ * stays on the class, the contract, and its function keeps the placeholder. */ -import { ReadableIds, RLS_DENY_FILTER, isControlledByParentMasterField, markFilterSubtreeProvenance } from '@objectstack/spec/data'; +import { ReadableIds, RLS_DENY_FILTER, controlledByParentMasterOf, markFilterSubtreeProvenance } from '@objectstack/spec/data'; import type { FilterCondition } from '@objectstack/spec/data'; import type { ExecutionContext } from '@objectstack/spec/kernel'; import { toEvalPermissions, type EvalError, type EvalPermissions } from '@objectstack/formula'; @@ -215,10 +215,13 @@ export function findUnservableControlledByParentMasters( const found: UnservableControlledByParentMaster[] = []; for (const object of this._registry.getAllObjects() as Array<{ name?: string; sharingModel?: unknown; fields?: Record }>) { if (object?.sharingModel !== 'controlled_by_parent' || typeof object.name !== 'string') continue; - const masters = Object.entries(object.fields ?? {}).filter(([, f]) => isControlledByParentMasterField(f)); - if (masters.length !== 1) continue; - const [field, def] = masters[0]; - const master = (def as { reference?: unknown }).reference; + // [ADR-0055 §1 as amended · #30905 ruling A] The one master, required or + // optional: an optional master's rows that name one are read through the + // same leaf over it. + const relation = controlledByParentMasterOf(object.fields); + if (!relation) continue; + const field = relation.field; + const master = (object.fields?.[field] as { reference?: unknown } | undefined)?.reference; if (typeof master !== 'string' || !this._registry.getObject(master)) continue; const uncomposable = this.uncomposableReadableIdsParent(object.name, master); if (uncomposable) found.push({ detail: object.name, field, master, ...uncomposable }); diff --git a/packages/objectql/src/engine-top-level.ts b/packages/objectql/src/engine-top-level.ts index e86fd97bcf7..4e806729419 100644 --- a/packages/objectql/src/engine-top-level.ts +++ b/packages/objectql/src/engine-top-level.ts @@ -4928,7 +4928,10 @@ export interface ReadableIdsChain { export type UnservableControlledByParentMaster = { /** The `controlled_by_parent` detail. */ readonly detail: string; - /** Its one required `master_detail` field. */ + /** + * Its master `master_detail` field: the one required, or (ADR-0055 §1 as + * amended) its only `master_detail`, declared `required: false`. + */ readonly field: string; /** The object that field names. */ readonly master: string; diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 0a08f253a67..83d2b6a400f 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -1650,14 +1650,15 @@ export class ObjectQL implements IObjectQLEngine { * master an object-scoped read middleware narrows, or one bound to another * datasource than the detail — judged by the read path's own predicate * ({@link uncomposableReadableIdsParent}). A detail's access derives from its - * one required `master_detail` (ADR-0055 §1), so such a declaration is one + * one master `master_detail` (ADR-0055 §1: the required one, or as amended + * its only one declared `required: false`), so such a declaration is one * the runtime cannot honour: `ObjectQLPlugin` refuses the boot on any entry. * * Both facts are deployment facts (the datasource mapping, a package's * default datasource, lifecycle routing, the middleware registrations), so * the answer is final only once every plugin has registered its middleware — * which several do inside their own `kernel:ready` handler. Asked earlier, it - * is short. A detail with no single required master, or whose master the + * is short. A detail with no single master, or whose master the * registry does not hold, is no entry: other rules own those. */ findUnservableControlledByParentMasters(): UnservableControlledByParentMaster[] { diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-optional-master.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-optional-master.test.ts new file mode 100644 index 00000000000..03ec3637a30 --- /dev/null +++ b/packages/plugins/plugin-security/src/controlled-by-parent-optional-master.test.ts @@ -0,0 +1,465 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [ADR-0055 §1 and §2 as amended · #30905, maintainer ruling A] A +// `controlled_by_parent` object whose only `master_detail` is declared +// `required: false` has an OPTIONAL master. A detail row that names its +// master is read through it, exactly as before; a row whose master reference +// is empty belongs to no master and is read through a second arm, OR-ed with +// the master arm inside `computeControlledByParentFilter` — the one +// chokepoint every read door composes — by its owner (`owner_id`) and by a +// holder of View All on the detail, and by nobody else. +// +// The rules the card states, one case each: +// - a row that names a master is never read through the owner arm, and a +// row that names none never through the master arm; +// - View All on the detail admits the null-master rows only: it does not +// open the master arm; +// - a null-master row with no owner (an ADR-0142 queue row), or on an +// object with no `owner_id` column, is read by View All alone; +// - a chain's intermediate level whose own master is empty is read by its +// owner, and its children's master arm sees exactly that; +// - a caller who reads another master does not see the null-master rows +// they do not own; +// - a required master's leaf is unchanged, and the write gate is unchanged +// (a null-master insert is still refused, 422). +// +// The fixture mirrors `controlled-by-parent-sharing.test.ts`: the REAL +// security plugin and the REAL sharing service over an in-memory read +// double, the leaf composed the way the engine composes it +// (`readable-ids-in-memory.testkit.ts`), every filter applied by the same +// evaluator the plugin uses. Two read faces are run on every rule: the data +// middleware's injected `where` and `getReadFilter` (the analytics read and +// the engine's `ReadableIds` composer for parent gates). + +import { describe, it, expect, vi } from 'vitest'; +import { SecurityPlugin } from './security-plugin.js'; +import { SharingService, type SharingEngine } from '@objectstack/plugin-sharing'; +import { matchesFilterCondition } from '@objectstack/formula'; +import { assertEngineFindOnePredicate } from '@objectstack/objectql'; +import { ObjectSchema, isComposedReadableIds, isReadableIds } from '@objectstack/spec/data'; +import { PermissionSetSchema, type PermissionSet } from '@objectstack/spec/security'; +import { composeReadableIdsInMemory } from './readable-ids-in-memory.testkit.js'; +import { PermissionEvaluator } from './permission-evaluator.js'; +import { explainAccessForAsker, type ExplainEngineDeps } from './explain-engine.js'; + +const REP = 'usr_rep'; +const OTHER = 'usr_other'; + +/** The MASTER — owner-scoped by OWD (`private`), with no authored RLS. */ +const ACCOUNT_SCHEMA = { + name: 'crm_account', + sharingModel: 'private', + fields: { + id: { name: 'id', type: 'text' }, + name: { name: 'name', type: 'text' }, + owner_id: { name: 'owner_id', type: 'lookup', reference: 'sys_user' }, + }, +}; + +/** The DETAIL, its master OPTIONAL — the shape ruling A admits. */ +const CONTACT_OPTIONAL = { + name: 'crm_contact', + sharingModel: 'controlled_by_parent', + fields: { + id: { name: 'id', type: 'text' }, + name: { name: 'name', type: 'text' }, + account: { name: 'account', type: 'master_detail', required: false, reference: 'crm_account' }, + owner_id: { name: 'owner_id', type: 'lookup', reference: 'sys_user' }, + }, +}; + +/** The same detail with a REQUIRED master — the leaf it answers must not move. */ +const CONTACT_REQUIRED = { + ...CONTACT_OPTIONAL, + fields: { + ...CONTACT_OPTIONAL.fields, + account: { name: 'account', type: 'master_detail', required: true, reference: 'crm_account' }, + }, +}; + +/** The optional-master detail on an object that carries no `owner_id` column (`ownership` other than `'user'`). */ +const CONTACT_OPTIONAL_NO_OWNER_COLUMN = { + name: 'crm_contact', + sharingModel: 'controlled_by_parent', + fields: { + id: { name: 'id', type: 'text' }, + name: { name: 'name', type: 'text' }, + account: { name: 'account', type: 'master_detail', required: false, reference: 'crm_account' }, + }, +}; + +/** + * A detail of the contact — a chain whose intermediate level (the contact) + * has an optional master. Its own master is required. + */ +const NOTE_SCHEMA = { + name: 'crm_contact_note', + sharingModel: 'controlled_by_parent', + fields: { + id: { name: 'id', type: 'text' }, + contact: { name: 'contact', type: 'master_detail', required: true, reference: 'crm_contact' }, + }, +}; + +const DETAILS = { + optional: CONTACT_OPTIONAL, + required: CONTACT_REQUIRED, + 'optional-no-owner-column': CONTACT_OPTIONAL_NO_OWNER_COLUMN, +} as const; + +const SHARE_SCHEMA = { + name: 'sys_record_share', + isSystem: true, + fields: { + id: { name: 'id', type: 'text' }, + object_name: { name: 'object_name', type: 'text' }, + record_id: { name: 'record_id', type: 'text' }, + recipient_type: { name: 'recipient_type', type: 'text' }, + recipient_id: { name: 'recipient_id', type: 'text' }, + access_level: { name: 'access_level', type: 'text' }, + owner_id: { name: 'owner_id', type: 'lookup', reference: 'sys_user' }, + }, +}; + +const CRUD = { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }; + +/** The rep: full CRUD, no View All, no RLS — record access is OWD, sharing and the derivation. */ +const REP_SET = { + name: 'crm_rep', + label: 'CRM Rep', + objects: { crm_account: CRUD, crm_contact: CRUD, crm_contact_note: CRUD }, +} as unknown as PermissionSet; + +/** The same, plus View All on the DETAIL (not on the master). */ +const VIEW_ALL_CONTACTS_SET = { + name: 'crm_rep', + label: 'CRM Rep with View All contacts', + objects: { + crm_account: CRUD, + crm_contact: { ...CRUD, viewAllRecords: true }, + crm_contact_note: CRUD, + }, +} as unknown as PermissionSet; + +type Row = Record; + +/** + * The rows, identical for every case. + * + * - `acct_us` is OTHER's and shared to the rep (read); `acct_eu` is OTHER's and + * NOT shared; `acct_own` is the rep's. + * - `ct_eu_mine` names `acct_eu` and is OWNED BY THE REP: a row that names a + * master the rep cannot read — the owner arm must not reach it. + * - `ct_orphan_mine` / `ct_orphan_other` / `ct_queue` name no master: the + * rep's, OTHER's, and nobody's (an ADR-0142 queue row). + */ +function fixtureRows(): Record { + return { + crm_account: [ + { id: 'acct_us', name: 'US Corp', owner_id: OTHER }, + { id: 'acct_eu', name: 'EU Corp', owner_id: OTHER }, + { id: 'acct_own', name: 'Own Corp', owner_id: REP }, + ], + crm_contact: [ + { id: 'ct_us', account: 'acct_us', owner_id: OTHER }, + { id: 'ct_eu_mine', account: 'acct_eu', owner_id: REP }, + { id: 'ct_own', account: 'acct_own', owner_id: OTHER }, + { id: 'ct_orphan_mine', account: null, owner_id: REP }, + { id: 'ct_orphan_other', account: null, owner_id: OTHER }, + { id: 'ct_queue', account: null, owner_id: null, queue: 'support' }, + ], + crm_contact_note: [ + { id: 'nt_us', contact: 'ct_us' }, + { id: 'nt_orphan_mine', contact: 'ct_orphan_mine' }, + { id: 'nt_orphan_other', contact: 'ct_orphan_other' }, + { id: 'nt_eu_mine', contact: 'ct_eu_mine' }, + ], + sys_record_share: [ + { + id: 'shr_1', + object_name: 'crm_account', + record_id: 'acct_us', + recipient_type: 'user', + recipient_id: REP, + access_level: 'read', + }, + ], + }; +} + +async function boot(options: { detail?: keyof typeof DETAILS; sets?: PermissionSet[] } = {}) { + const rows = fixtureRows(); + const schemas: Record = { + crm_account: ACCOUNT_SCHEMA, + crm_contact: DETAILS[options.detail ?? 'optional'], + crm_contact_note: NOTE_SCHEMA, + sys_record_share: SHARE_SCHEMA, + }; + const store = { + rows, + getSchema: (object: string) => schemas[object], + find: vi.fn(async (object: string, opts: any = {}) => + (rows[object] ?? []).filter((r) => matchesFilterCondition(r, opts?.where ?? null))), + findOne: vi.fn(async (object: string, opts: any = {}) => { + assertEngineFindOnePredicate(object, opts); + return (rows[object] ?? []).find((r) => matchesFilterCondition(r, opts?.where ?? null)) ?? null; + }), + }; + const sets = options.sets ?? [REP_SET]; + + let middleware: any; + const ql = { + registerMiddleware: (mw: any) => { + if (!middleware) middleware = mw; + }, + getSchema: store.getSchema, + find: store.find, + findOne: store.findOne, + }; + const services: Record = { + manifest: { register: vi.fn() }, + objectql: ql, + metadata: { get: async (n: string) => store.getSchema(n), list: async () => sets }, + sharing: new SharingService({ engine: store as unknown as SharingEngine }), + }; + const ctx: any = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + registerService: vi.fn(), + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin({ defaultPermissionSets: sets, fallbackPermissionSet: 'crm_rep' }); + await plugin.init(ctx); + await plugin.start(ctx); + + const repContext = () => ({ userId: REP, tenantId: 'org-1', positions: [], permissions: [] }); + const compose = (where: unknown, context: any) => + composeReadableIdsInMemory(where, context, { + rows: (object) => store.rows[object], + readAdmission: (object, c) => plugin.canReadObject(object, c), + readScope: (object, c) => plugin.getReadFilter(object, c), + }); + + /** The `where` the data middleware injects for a find of `object`, uncomposed. */ + const injected = async (object: string): Promise => { + const opCtx: any = { object, operation: 'find', ast: {}, options: {}, context: repContext() }; + await middleware(opCtx, async () => {}); + return opCtx.ast.where ?? null; + }; + const apply = (object: string, where: unknown) => + (store.rows[object] ?? []).filter((r) => matchesFilterCondition(r, where as any)).map((r) => String(r.id)).sort(); + + /** The data middleware's read face. */ + const visible = async (object: string) => apply(object, await compose(await injected(object), repContext())); + /** The `getReadFilter` face: analytics, and the engine's composer for parent gates and chains. */ + const readFilterVisible = async (object: string) => + apply(object, await compose((await plugin.getReadFilter(object, repContext())) ?? null, repContext())); + + /** The INSERT face: whether the executor was reached; a refusal throws. */ + const insertContact = async (data: Row): Promise => { + const opCtx: any = { object: 'crm_contact', operation: 'insert', data, options: {}, context: repContext() }; + let reached = false; + await middleware(opCtx, async () => { + reached = true; + }); + return reached; + }; + + return { plugin, ctx, injected, visible, readFilterVisible, insertContact }; +} + +/** The node at `path` inside `where`. */ +function leafAt(where: unknown, path: (string | number)[]): unknown { + return path.reduce((node, key) => (node as any)?.[key], where); +} + +describe('[#30905 ruling A] the cbp read leaf of an OPTIONAL master — one $or, two arms that never share a row', () => { + it('the leaf the middleware injects: the master arm unchanged, OR-ed with `fk IS NULL AND owner_id = caller`', async () => { + const h = await boot(); + const where = (await h.injected('crm_contact')) as Record; + // Everything the middleware ANDs in is one `$and` of layers or the cbp + // layer itself; find the cbp layer as the node carrying the `$or`. + const layers: any[] = Array.isArray(where?.$and) ? where.$and : [where]; + const cbp = layers.find((l) => l && Array.isArray(l.$or)); + expect(cbp, JSON.stringify(where)).toBeDefined(); + expect(cbp.$or).toHaveLength(2); + const [masterArm, nullArm] = cbp.$or; + expect(Object.keys(masterArm)).toEqual(['account']); + expect(isReadableIds(leafAt(masterArm, ['account', '$in']))).toBe(true); + expect(isComposedReadableIds(leafAt(masterArm, ['account', '$in']))).toBe(false); + expect(nullArm).toEqual({ $and: [{ account: null }, { owner_id: REP }] }); + }); + + it('a REQUIRED master answers the leaf it always did: no $or, no owner arm', async () => { + const h = await boot({ detail: 'required' }); + const where = (await h.injected('crm_contact')) as Record; + const layers: any[] = Array.isArray(where?.$and) ? where.$and : [where]; + expect(layers.some((l) => l && '$or' in l)).toBe(false); + const cbp = layers.find((l) => l && 'account' in l); + expect(Object.keys(cbp)).toEqual(['account']); + expect(isReadableIds(cbp.account.$in)).toBe(true); + // And the rows: the null-master row the rep owns is NOT read through a required master. + expect(await h.visible('crm_contact')).toEqual(['ct_own', 'ct_us']); + }); + + for (const face of ['visible', 'readFilterVisible'] as const) { + describe(`read face: ${face === 'visible' ? 'the data middleware' : 'getReadFilter (analytics, ReadableIds composer)'}`, () => { + it('the rep reads the rows of masters they read, and the null-master rows they own — nothing else', async () => { + const h = await boot(); + expect(await h[face]('crm_contact')).toEqual(['ct_orphan_mine', 'ct_own', 'ct_us']); + }); + + it('a row that NAMES a master is never read through the owner arm (the rep owns ct_eu_mine, cannot read acct_eu)', async () => { + const h = await boot(); + expect(await h[face]('crm_contact')).not.toContain('ct_eu_mine'); + }); + + it('a caller who reads another master does not see the null-master rows they do not own', async () => { + const h = await boot(); + const rows = await h[face]('crm_contact'); + // The rep reads acct_us (OTHER's, shared) and still not OTHER's orphan. + expect(rows).toContain('ct_us'); + expect(rows).not.toContain('ct_orphan_other'); + }); + + it('a null-master row with no owner (an ADR-0142 queue row) is not read through the owner arm', async () => { + const h = await boot(); + expect(await h[face]('crm_contact')).not.toContain('ct_queue'); + }); + + it('View All on the detail reads every null-master row, the queue row included — and does NOT open the master arm', async () => { + const h = await boot({ sets: [VIEW_ALL_CONTACTS_SET] }); + expect(await h[face]('crm_contact')).toEqual(['ct_orphan_mine', 'ct_orphan_other', 'ct_own', 'ct_queue', 'ct_us']); + }); + + it('on an object with no owner_id column, a null-master row is View All\'s alone', async () => { + const plain = await boot({ detail: 'optional-no-owner-column' }); + expect(await plain[face]('crm_contact')).toEqual(['ct_own', 'ct_us']); + const viewAll = await boot({ detail: 'optional-no-owner-column', sets: [VIEW_ALL_CONTACTS_SET] }); + expect(await viewAll[face]('crm_contact')).toEqual(['ct_orphan_mine', 'ct_orphan_other', 'ct_own', 'ct_queue', 'ct_us']); + }); + + it('chain: the children of an intermediate row with no master are read by that row\'s owner, and exactly those', async () => { + const h = await boot(); + expect(await h[face]('crm_contact_note')).toEqual(['nt_orphan_mine', 'nt_us']); + }); + }); + } + + it('the answer is the disjoint union of the two arms', async () => { + // The master arm reads the rows naming a master the rep reads; the + // null-master arm the rep's own rows naming none. No row is in both. + const h = await boot(); + const rows = await h.visible('crm_contact'); + const masterArmRows = ['ct_own', 'ct_us']; + const nullArmRows = ['ct_orphan_mine']; + expect(rows).toEqual([...masterArmRows, ...nullArmRows].sort()); + }); +}); + +describe('[#30905 ruling A] the write gate is unchanged (card 2/3 moves it)', () => { + it('an insert of an optional-master detail with an empty master reference is still refused, 422', async () => { + const h = await boot(); + let err: any; + try { + await h.insertContact({ id: 'ct_new', account: null, owner_id: REP }); + } catch (e) { + err = e; + } + expect(err?.code).toBe('MISSING_REQUIRED_FIELD'); + expect(err?.statusCode).toBe(422); + }); + + it('an insert naming a master the rep owns reaches the executor, as before', async () => { + const h = await boot(); + expect(await h.insertContact({ id: 'ct_new', account: 'acct_own', owner_id: REP })).toBe(true); + }); +}); + +describe('[#30905 ruling A] the relation resolver reads the optional master — and an omitted required stays unresolved', () => { + it('the optional master resolves: no "resolves no master relation" report', async () => { + const h = await boot(); + await h.visible('crm_contact'); + const reports = (h.ctx.logger.warn as ReturnType).mock.calls + .map((c) => String(c[0])) + .filter((m) => m.includes('resolves no master relation')); + expect(reports).toEqual([]); + }); + + it('the parse accepts the optional detail whole, so a parsed registry agrees with the raw one', () => { + const parsed = ObjectSchema.safeParse({ ...CONTACT_OPTIONAL, fields: { account: CONTACT_OPTIONAL.fields.account } }); + expect(parsed.success).toBe(true); + }); +}); + +// ── security/explain narrates the null-master arm ─────────────────────────── + +describe('[#30905 ruling A] security/explain narrates the null-master arm on a record read', () => { + const OBJECT = 'crm_contact'; + const EDITOR = PermissionSetSchema.parse({ name: 'crm_rep', objects: { [OBJECT]: CRUD } }); + const EXPLAINED = { userId: REP, tenantId: 'org1', positions: [], permissions: ['crm_rep'] }; + + const deps = (opts: { record: Row; absent: boolean; schema?: unknown; sets?: PermissionSet[] }): ExplainEngineDeps => ({ + ql: { getSchema: () => opts.schema ?? CONTACT_OPTIONAL }, + resolveSets: async () => opts.sets ?? [EDITOR], + evaluator: new PermissionEvaluator(), + getObjectSecurityMeta: async () => ({ + isPrivate: false, + requiredPermissions: { all: [], read: [], create: [], update: [], delete: [] }, + fieldRequiredPermissions: {}, + }), + requiredCaps: () => [], + computeRlsFilter: async () => null, + getFieldMask: () => ({}), + getPartialMaskRules: async () => ({}), + baselinePermissionSets: [], + tenancyPosture: 'single', + computeLayeredRlsFilter: async () => ({ layer0: { organization_id: 'org1' }, layer1: null }), + fetchRecord: async () => ({ organization_id: 'org1', ...opts.record }), + sharingReadFilter: async () => null, + listRecordShares: async () => [], + canEditRecord: async () => true, + canDeleteRecord: async () => true, + // The read door withholds the record from the EXPLAINED principal (or not); the asker always reads it. + recordAbsentToCaller: vi.fn(async (_o: string, _r: string, context: unknown) => (context === EXPLAINED ? opts.absent : false)), + }); + const sharingDetail = (d: any): string => + String(d.layers.find((l: any) => l.layer === 'sharing')?.record?.detail ?? ''); + // Asked by an administrator who reads the record, as `security/explain` is + // for another principal: a principal explaining itself a record the door + // withholds is told what an id no row carries gets. + const ASKER = { userId: 'usr_admin', tenantId: 'org1', positions: [], permissions: [] }; + const explain = (d: ExplainEngineDeps) => + explainAccessForAsker(d, { object: OBJECT, operation: 'read', context: EXPLAINED, recordId: 'r1' }, { + context: ASKER, + cannotRead: (o, id) => d.recordAbsentToCaller!(o, id, ASKER), + }); + + it('a null-master row the read door returns: names the empty reference and the owner arm', async () => { + const d = await explain(deps({ record: { id: 'r1', account: null, owner_id: REP }, absent: false })); + expect(d.record).toMatchObject({ recordId: 'r1', visible: true }); + expect(sharingDetail(d)).toContain("its master reference 'account' is empty"); + expect(sharingDetail(d)).toContain('through the null-master arm — the principal owns it'); + }); + + it('a null-master row the read door withholds: who reads it instead, and a queue row is View All\'s alone', async () => { + const owned = await explain(deps({ record: { id: 'r1', account: null, owner_id: OTHER }, absent: true })); + expect(owned.record).toEqual({ recordId: 'r1', visible: false, decidedBy: 'sharing' }); + expect(sharingDetail(owned)).toContain("its master reference 'account' is empty, so it is read only by its owner and by a holder of View All on the object"); + expect(sharingDetail(owned)).not.toContain('queue row'); + + const queue = await explain(deps({ record: { id: 'r1', account: null, owner_id: null }, absent: true })); + expect(sharingDetail(queue)).toContain('it has no owner (a queue row), so View All alone reads it'); + }); + + it('a row that names its master, and any row of a required master, keep the master-scope narration', async () => { + const named = await explain(deps({ record: { id: 'r1', account: 'acct_eu', owner_id: REP }, absent: true })); + expect(sharingDetail(named)).toContain("so its master is outside the principal's read scope"); + expect(sharingDetail(named)).not.toContain('null-master'); + + const required = await explain(deps({ record: { id: 'r1', account: null, owner_id: REP }, absent: true, schema: CONTACT_REQUIRED })); + expect(sharingDetail(required)).toContain("so its master is outside the principal's read scope"); + }); +}); diff --git a/packages/plugins/plugin-security/src/controlled-by-parent-sharing.test.ts b/packages/plugins/plugin-security/src/controlled-by-parent-sharing.test.ts index bbc322a15a8..b4301ec058b 100644 --- a/packages/plugins/plugin-security/src/controlled-by-parent-sharing.test.ts +++ b/packages/plugins/plugin-security/src/controlled-by-parent-sharing.test.ts @@ -1100,10 +1100,16 @@ describe('[#23100] the retired master tiers derive NO master — fail-closed on }); it.each([ - ['a master_detail without `required` (the retired second tier)', 'master-not-required' as const, CONTACT_SCHEMA_MASTER_NOT_REQUIRED, 'sharingModel', /no required master_detail field; master_detail without required: true: account/], - ['a required lookup (the retired third tier — the HotCRM shape)', 'master-required-lookup' as const, CONTACT_SCHEMA_MASTER_REQUIRED_LOOKUP, 'sharingModel', /no required master_detail field; required lookup: account/], - ['two required master_details (no longer resolved by field order)', 'two-masters' as const, CONTACT_SCHEMA_TWO_MASTERS, 'fields', /2 required master_detail fields \(account, billing_account\)/], - ])('%s: every read DENIED, every write 422 INVALID_METADATA, reported once', async (_label, detail, schema, parsePath, why) => { + // [#30905 ruling A] The parse reads the PARSED field map, where an omitted + // `required` is `false`, so it admits this object as an OPTIONAL master; + // this gate reads the unparsed registry, where the omission is visible, + // and still derives no master from it (`required === false` is the + // optional master, an absent key is not). The recorded divergence: a + // registry row in this shape fails closed, as before. + ['a master_detail without `required` (the retired second tier)', 'master-not-required' as const, CONTACT_SCHEMA_MASTER_NOT_REQUIRED, [] as string[], /no required master_detail field; master_detail without required: true: account \(add required: true, or required: false for an optional master/], + ['a required lookup (the retired third tier — the HotCRM shape)', 'master-required-lookup' as const, CONTACT_SCHEMA_MASTER_REQUIRED_LOOKUP, ['sharingModel'], /no required master_detail field; required lookup: account/], + ['two required master_details (no longer resolved by field order)', 'two-masters' as const, CONTACT_SCHEMA_TWO_MASTERS, ['fields'], /2 required master_detail fields \(account, billing_account\)/], + ])('%s: every read DENIED, every write 422 INVALID_METADATA, reported once', async (_label, detail, schema, parsePaths, why) => { const h = await boot({ shareLevel: 'edit', detail }); // READ: the master the old tier would have picked holds readable rows @@ -1126,8 +1132,9 @@ describe('[#23100] the retired master tiers derive NO master — fail-closed on expect(reports).toHaveLength(1); expect(String(reports[0][0])).toMatch(why); - // …and the parse refuses the same object, at the path the lint shares. - expect(cbpParseIssuePaths(schema)).toEqual([parsePath]); + // …and the parse refuses the same object, at the path the lint shares + // (bar the one divergence above). + expect(cbpParseIssuePaths(schema)).toEqual(parsePaths); }); }); diff --git a/packages/plugins/plugin-security/src/explain-engine.ts b/packages/plugins/plugin-security/src/explain-engine.ts index 71abb07e5b1..4b8a50b3927 100644 --- a/packages/plugins/plugin-security/src/explain-engine.ts +++ b/packages/plugins/plugin-security/src/explain-engine.ts @@ -44,6 +44,7 @@ import type { ControlledByParentWriteUnresolvedReason, } from '@objectstack/spec/contracts'; import type { FieldMaskingRule } from '@objectstack/spec/data'; +import { controlledByParentMasterOf } from '@objectstack/spec/data'; import type { PermissionSet } from '@objectstack/spec/security'; import type { AuthzPosture, @@ -1135,7 +1136,8 @@ const MASTER_DENIAL_LEG_DETAIL: Record const MASTER_UNRESOLVED_DETAIL: Record = { master_detail_relation_missing: { condition: - 'the object declares controlled_by_parent without exactly one required master_detail field to derive access from', + 'the object declares controlled_by_parent without one master_detail field to derive access from (exactly ' + + 'one declared required: true, or, with none, its only master_detail declared required: false)', answer: '422 INVALID_METADATA', }, record_not_found: { condition: 'the record does not exist', answer: '404 RECORD_NOT_FOUND' }, @@ -1145,6 +1147,28 @@ const MASTER_UNRESOLVED_DETAIL: Record, + object: string, + record: Record | null | undefined, +): string | undefined { + if (record == null) return undefined; + let schema: any; + try { schema = deps.ql?.getSchema?.(object) ?? null; } catch { schema = null; } + const master = controlledByParentMasterOf(schema?.fields); + if (!master?.optional) return undefined; + return record[master.field] == null ? master.field : undefined; +} + /** * [ADR-0055] The record-grained writes whose verdict on a `controlled_by_parent` * record the master-detail write check decides: every write step 2.8 of the @@ -2008,18 +2032,52 @@ async function applyRecordAttribution( ? await settle(deps.recordAbsentToCaller(object, recordId, context)) : undefined; const masterReadFault = masterReadOrFault === DEPENDENCY_FAULT; + // [ADR-0055 §2 as amended · #30905 ruling A] A record of an OPTIONAL master + // whose master reference is empty names no master: the read door reads it + // through the null-master arm (its owner, or View All on the object), never + // through a master. Read off the registry by the spec's own resolver, the + // one the door's relation resolver picks an optional master with, so the + // narration names the arm the door answered on; the verdict is still the + // door's own read above. + const nullMasterFk = masterReadOrFault === true || masterReadOrFault === false + ? nullMasterReferenceOf(deps, object, record as Record) + : undefined; + const delegatorComposed = context?.onBehalfOf?.userId != null + ? " (on this on-behalf-of read, the delegator's read scope is composed in as well)" + : ''; const masterReadRefusal: { outcome: ExplainRecordAttribution['outcome']; detail: string } | undefined = masterReadOrFault === true ? { outcome: 'excluded', - detail: `${MASTER_READ_DERIVED}: the read door, asked by this principal, does not return the record, though ` + - "every row-level layer above admits it, so its master is outside the principal's read scope" + - (context?.onBehalfOf?.userId != null - ? " (on this on-behalf-of read, the delegator's read scope is composed in as well)" - : '') + - '. The record is reported NOT visible, as the read door answers.', + detail: nullMasterFk !== undefined + ? `${MASTER_READ_DERIVED} when it names one, and it names none: its master reference '${nullMasterFk}' ` + + 'is empty, so it is read only by its owner and by a holder of View All on the object. The read door, ' + + 'asked by this principal, does not return it, though every row-level layer above admits it' + + (record != null && (record as Record)[OWNER_FIELD] == null + ? ': it has no owner (a queue row), so View All alone reads it' + : '') + + delegatorComposed + + '. The record is reported NOT visible, as the read door answers.' + : `${MASTER_READ_DERIVED}: the read door, asked by this principal, does not return the record, though ` + + "every row-level layer above admits it, so its master is outside the principal's read scope" + + delegatorComposed + + '. The record is reported NOT visible, as the read door answers.', } : undefined; + // The same arm, when the door DOES return such a record: which half of the + // null-master arm admitted it. Narration only; the layer's outcome is the + // one the rest of this pass computes. + const masterReadNullArmAdmits: string | undefined = masterReadOrFault === false && nullMasterFk !== undefined + ? `${MASTER_READ_DERIVED} when it names one, and it names none: its master reference '${nullMasterFk}' is ` + + 'empty, and the read door returns it to this principal through the null-master arm — ' + + (vamaEffective + ? `View All on the object [${vamaSets.join(', ')}]` + : ownerIsMe + ? 'the principal owns it' + : 'its owner or View All on the object') + + delegatorComposed + + '. No master is consulted for it.' + : undefined; // One master leg per verb: the write check for a by-id write, the read // door's master scope for a read. At most one of the two is ever asked. const masterLegRefusal = masterRefusal ?? masterReadRefusal; @@ -2096,7 +2154,7 @@ async function applyRecordAttribution( rules: shareRules, // A fault is only ever recorded against an existing record, and the // master leg [ADR-0055] is only ever asked of one. - detail: sharingFaultDetail ?? masterLegRefusal?.detail ?? (!recordExists + detail: sharingFaultDetail ?? masterLegRefusal?.detail ?? masterReadNullArmAdmits ?? (!recordExists ? 'Record not found; sharing not evaluated.' : owd.effect !== 'private' ? 'Baseline is not private — sharing adds nothing beyond it for this record.' diff --git a/packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts b/packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts index f3a6b3d705a..5213674d411 100644 --- a/packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts +++ b/packages/plugins/plugin-security/src/security-plugin-authz-middleware.ts @@ -2358,7 +2358,7 @@ export function registerAuthzMiddleware( // (ADR-0139 D6), which the engine composes at its executor boundary // from this very service's `canReadObject` / `getReadFilter` of the // master. Nothing for a write verb's predicate (ADR-0139 D1). - const cbpFilter = this.computeControlledByParentFilter(opCtx.object, opCtx.operation, opCtx.context); + const cbpFilter = this.computeControlledByParentFilter(opCtx.object, opCtx.operation, opCtx.context, permissionSets); if (cbpFilter) extra.push(cbpFilter); // [ADR-0090 D10] AND the delegator's read RLS (and CBP) into the same // where — the delegated principal sees only rows BOTH may see. Computed @@ -2377,7 +2377,7 @@ export function registerAuthzMiddleware( // scope `getReadFilter` answers as the ADR-0090 D10 intersection // (ADR-0139 D5.1 as amended), so both halves carry one subquery // twice — correct rows, measured, and left as it is. - const delCbp = this.computeControlledByParentFilter(opCtx.object, opCtx.operation, delegatorContext); + const delCbp = this.computeControlledByParentFilter(opCtx.object, opCtx.operation, delegatorContext, delegatorSets); if (delCbp) extra.push(delCbp); } // [#21829] The write doors' rule on the PREDICATE door: a row the diff --git a/packages/plugins/plugin-security/src/security-plugin-cbp-read-filter.ts b/packages/plugins/plugin-security/src/security-plugin-cbp-read-filter.ts index 3699d62d702..2ba0466291d 100644 --- a/packages/plugins/plugin-security/src/security-plugin-cbp-read-filter.ts +++ b/packages/plugins/plugin-security/src/security-plugin-cbp-read-filter.ts @@ -45,13 +45,31 @@ * (below). * * Nothing re-exports this module, so the published surface does not move. + * + * ## Since the move + * + * [ADR-0055 §1 and §2 as amended · #30905 ruling A] The leaf of an OPTIONAL + * master grew its null-master arm here, the one chokepoint every read door + * composes: `computeControlledByParentFilter` takes the caller's resolved + * permission sets (for View All on the detail) and the host hands it + * `permissionEvaluator`. A required master's leaf is unchanged. */ import type { SecurityPlugin } from './security-plugin.js'; import { CBP_MAX_CHAIN_DEPTH } from './security-plugin.js'; import { ReadableIds } from '@objectstack/spec/data'; +import type { PermissionSet } from '@objectstack/spec/security'; import { RLS_DENY_FILTER } from './rls-compiler.js'; +/** + * [ADR-0055 §2 as amended · #30905 ruling A] The column an optional master's + * null-master arm compares the caller against: the registry's injected + * per-record owner, the column `CbpRelation.ownerColumn` reports present — + * the one the relation resolver's `OWNER_COLUMN` and the sharing service's + * `OWNER_FIELD` name. + */ +const CBP_OWNER_COLUMN = 'owner_id'; + /** * [ADR-0139 D1] The engine verbs whose `where` may carry a `ReadableIds` * leaf — the engine's own `READABLE_IDS_READ_VERBS`. The read derivation @@ -81,6 +99,7 @@ type CbpRelation = NonNullable> */ export interface CbpReadFilterHost { readonly logger: SecurityPlugin['logger']; + readonly permissionEvaluator: SecurityPlugin['permissionEvaluator']; readonly ql: SecurityPlugin['ql']; controlledByParentChainRefusal: SecurityPlugin['controlledByParentChainRefusal']; resolveCbpRelation: SecurityPlugin['resolveCbpRelation']; @@ -88,11 +107,17 @@ export interface CbpReadFilterHost { /** The body of `SecurityPlugin#computeControlledByParentFilter`; the method's docblock there is its contract. */ export function computeControlledByParentFilter( - this: Pick, + this: Pick, object: string, /** The engine verb the answer is composed onto; a write verb answers `null` (ADR-0139 D1). */ operation: string, context: any, + /** + * The permission sets resolved for `context`'s principal — read for View + * All on the detail, by an optional master's null-master arm and nothing + * else (ADR-0055 §2 as amended). + */ + permissionSets: PermissionSet[], ): Record | null { if (!this.ql || !context?.userId) return null; const schema = typeof this.ql.getSchema === 'function' ? this.ql.getSchema(object) : null; @@ -128,7 +153,35 @@ export function computeControlledByParentFilter( ); return { [rel.fk]: { $in: [] } }; } - return { [rel.fk]: { $in: ReadableIds.of(rel.master) } }; + const masterArm = { [rel.fk]: { $in: ReadableIds.of(rel.master) } }; + // A required master: its rows are read through it and nothing else, the + // leaf byte-identical to before ruling A of #30905. + if (!rel.optional) return masterArm; + // [ADR-0055 §2 as amended · #30905 ruling A] An OPTIONAL master: a row whose + // reference is empty belongs to no master, so the master arm never matches + // it (`$in` never matches an empty value) and a second arm, OR-ed beside the + // master arm and confined to such rows, reads it. The `$or` is this one + // layer's answer, ANDed by every caller with the other layers as before — so + // it is still under the Layer-0 tenant wall, the detail's own RLS and its + // sharing layer. Neither arm reaches the other's rows: the master arm + // matches only a non-empty reference, the null-master arm only an empty one. + // - View All on the DETAIL (`viewAllRecords` / `modifyAllRecords`, the + // super-user read bypass on this object) reads every such row. It is + // confined to them: it does not open the master arm, which stays the + // master's own read answer (unchanged). + // - Otherwise the row's owner reads it: `owner_id` equal to the caller. + // A row with no owner (an ADR-0142 queue row) matches nobody, and an + // object that carries no `owner_id` column (`ownership` other than + // `'user'`) has no owner arm, so its such rows are View All's alone. + // `{ fk: null }` is the has-no-value predicate (`$eq: null`), the spelling + // the sharing service's queue arm and the readable-ids conformance table use. + const nullMaster = { [rel.fk]: null }; + const nullMasterArm = this.permissionEvaluator.hasSuperuserReadBypass(object, permissionSets) + ? nullMaster + : rel.ownerColumn + ? { $and: [nullMaster, { [CBP_OWNER_COLUMN]: context.userId }] } + : null; + return nullMasterArm ? { $or: [masterArm, nullMasterArm] } : masterArm; } /** The body of `SecurityPlugin#controlledByParentChainRefusal`; the method's docblock there is its contract. */ diff --git a/packages/plugins/plugin-security/src/security-plugin-cbp-relation.ts b/packages/plugins/plugin-security/src/security-plugin-cbp-relation.ts index 8bf71860481..fe589f94a2c 100644 --- a/packages/plugins/plugin-security/src/security-plugin-cbp-relation.ts +++ b/packages/plugins/plugin-security/src/security-plugin-cbp-relation.ts @@ -43,7 +43,7 @@ import type { SecurityPlugin } from './security-plugin.js'; // [ADR-0055 §1] The parse's own one-master judgement, asked of an object that // reached the registry without the parse (see `resolveCbpRelation`). -import { controlledByParentMasterRefusal, controlledByParentOwnerMayEdit } from '@objectstack/spec/data'; +import { controlledByParentMasterOf, controlledByParentMasterRefusal, controlledByParentOwnerMayEdit } from '@objectstack/spec/data'; import { masterGovernsRowWrites, OWD_DELEGATING_ROW_WRITES } from './platform-ownership-policies.js'; /** @@ -162,7 +162,17 @@ export function resolveCbpRelation( // naming a reference — see the docblock. `required === true` is the spec's // own type for the key and the lint's reading of it. const candidates = entries.filter(([, f]) => f?.type === 'master_detail' && f?.required === true && ref(f)); - const found = candidates.length === 1 ? candidates[0] : undefined; + // [ADR-0055 §1 as amended · #30905 ruling A] With none, the object's ONLY + // `master_detail`, declared `required: false`, is its optional master. Which + // field that is, is the spec's own resolver's answer (the parse judges the + // object with it); the reference is read here as tolerantly as above. + const judged = candidates.length === 0 + ? controlledByParentMasterOf(Object.fromEntries(entries.map(([k, f]) => [String(k), f]))) + : undefined; + const optionalEntry = judged?.optional ? entries.find(([k]) => String(k) === judged.field) : undefined; + const found = candidates.length === 1 + ? candidates[0] + : optionalEntry && ref(optionalEntry[1]) ? optionalEntry : undefined; // [ADR-0055 §3 amendment] Resolved HERE, on the same schema the relation // came from, so the opt-in and the relation can never describe two objects. // Judged only for an object whose relation resolves: an opt-in on an object @@ -196,9 +206,12 @@ export function resolveCbpRelation( // reports `true` for is a shape validation provably refuses. // // `master_detail` + `required` are the resolver's own predicate above - // (since #23100 nothing else resolves), so they always hold here; they - // stay spelled out so this licence reads as the validator's test it - // mirrors and cannot widen if the resolver ever changes. + // for a required master; they stay spelled out so this licence reads as + // the validator's test it mirrors and cannot widen if the resolver + // changes — and it did: an OPTIONAL master (#30905 ruling A) resolves + // with `required: false`, so this is `false` there and the master gate + // keeps answering an insert that leaves the reference empty (the write + // gate is unchanged by that ruling: such an insert is still refused). omissionRefusedByValidation: def?.type === 'master_detail' && !!def?.required && !def?.readonly && !def?.system, // [#25095] From the same def, asked of the parse's own predicate. @@ -207,6 +220,10 @@ export function resolveCbpRelation( // the spec's one reader, and only where the schema carries the column // it names — see {@link CbpRelation.ownerMayEdit}. ownerMayEdit, + // [ADR-0055 §1 and §2 as amended · #30905 ruling A] See + // {@link CbpRelation.optional} and {@link CbpRelation.ownerColumn}. + optional: candidates.length === 0, + ownerColumn: hasOwnerColumn, }; } this.cbpRelCache.set(object, rel); @@ -220,6 +237,11 @@ export function reportCbpMasterUnresolved( entries: ReadonlyArray, requiredMasters: readonly string[], ): void { + // Every `master_detail` not declared `required: true`. The log key keeps + // its name; since ruling A of #30905 an OPTIONAL master proper (the only + // `master_detail`, declared `required: false`) resolves, so it is never + // listed here: what is listed is one that declares no `required`, or one of + // several. const optionalMasters = entries .filter(([, f]) => f?.type === 'master_detail' && f?.required !== true) .map(([k]) => String(k)); @@ -232,9 +254,17 @@ export function reportCbpMasterUnresolved( 'and derives its master from exactly one' : [ 'it declares no required master_detail field', - ...(optionalMasters.length > 0 - ? [`master_detail without required: true: ${optionalMasters.join(', ')} (add required: true)`] - : []), + ...(optionalMasters.length > 1 + ? [ + `master_detail without required: true: ${optionalMasters.join(', ')} (add required: true to ` + + 'the master and declare every other parent as a lookup)', + ] + : optionalMasters.length === 1 + ? [ + `master_detail without required: true: ${optionalMasters[0]} (add required: true, or ` + + 'required: false for an optional master whose rows may name none)', + ] + : []), ...(requiredLookups.length > 0 ? [ `required lookup: ${requiredLookups.join(', ')} (a lookup derives no access — declare the ` + @@ -246,8 +276,9 @@ export function reportCbpMasterUnresolved( `[security/controlled-by-parent] object "${object}" declares sharingModel 'controlled_by_parent' but ` + `resolves no master relation — ${why}. Every read of it is DENIED and every write refused ` + '(422 INVALID_METADATA): ADR-0055 §1 derives the master from exactly one master_detail field declared ' + - 'required: true, and from nothing else. A non-required master_detail and a required lookup no longer ' + - 'derive access.', + 'required: true, or, with none, from the only master_detail declared required: false, and from nothing ' + + 'else. A master_detail that declares no required, one of several without required: true, and a ' + + 'required lookup derive no access.', { object, requiredMasters, optionalMasters, requiredLookups }, ); } diff --git a/packages/plugins/plugin-security/src/security-plugin-read-filter.ts b/packages/plugins/plugin-security/src/security-plugin-read-filter.ts index e6eaca6cda0..5bc6b34d8ba 100644 --- a/packages/plugins/plugin-security/src/security-plugin-read-filter.ts +++ b/packages/plugins/plugin-security/src/security-plugin-read-filter.ts @@ -156,7 +156,7 @@ export async function getReadFilter( // sentinel for an unresolvable relation, `$in: []` for a chain the // registry walk refuses); a THROW propagates to the catch below, which // denies — the same posture as the surrounding layers. - const cbpFilter = this.computeControlledByParentFilter(object, 'find', context); + const cbpFilter = this.computeControlledByParentFilter(object, 'find', context, permissionSets); // [#4467] RLS AND controlled-by-parent AND sharing — the same // AND-composition the middlewares achieve by all writing into `ast.where`. // Any layer may be absent; `andComposeLayers` returns the others, or null @@ -231,9 +231,9 @@ export async function computeDelegatedReadFilter( // Layers 1 to 4 — the security middleware's own four, in its order. const filter = await this.computeRlsFilter(permissionSets, object, 'find', context); - const cbpFilter = this.computeControlledByParentFilter(object, 'find', context); + const cbpFilter = this.computeControlledByParentFilter(object, 'find', context, permissionSets); const delFilter = await this.computeRlsFilter(delegatorSets, object, 'find', delegatorContext); - const delCbpFilter = this.computeControlledByParentFilter(object, 'find', delegatorContext); + const delCbpFilter = this.computeControlledByParentFilter(object, 'find', delegatorContext, delegatorSets); const securityLayers = andComposeLayers(andComposeLayers(filter, cbpFilter), andComposeLayers(delFilter, delCbpFilter)); const sharingLayers = andComposeLayers(sharingFilter, delegatorSharingFilter); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index a96cc14b46a..56ebc891604 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -767,6 +767,27 @@ interface CbpRelation { * master-edit rule whatever this says. */ ownerMayEdit: boolean; + /** + * [ADR-0055 §1 and §2 as amended · #30905 ruling A] The master is OPTIONAL: + * the object declares no required `master_detail`, and its only one is + * declared `required: false` (read `=== false`, as the spec's + * `controlledByParentMasterOf` reads it — an omitted `required` on an object + * that reached the registry without the parse is not it). A detail row whose + * {@link fk} is empty then belongs to no master, and the read leaf + * ({@link SecurityPlugin.computeControlledByParentFilter}) admits it through + * the owner arm alone. `false` for a required master: its rows are read + * through the master and nothing else, exactly as before. + */ + optional: boolean; + /** + * The registry's schema carries the `owner_id` column — the injected + * per-record owner under `ownership: 'user'` (the default), or an author's + * own field of that name. The owner arm of an {@link optional} master + * compares the caller against it; without it a row with an empty master is + * read through View All on the detail alone. Resolved from the same schema + * as {@link ownerMayEdit}, which reads it too. + */ + ownerColumn: boolean; } /** @@ -4600,6 +4621,14 @@ export class SecurityPlugin implements Plugin { * tolerance below is why — and ⛔ it must not grow a fallback tier back: a * relation the parse refuses must not be one this gate derives access from. * + * ⚠️ Amended by ruling A of #30905 (ADR-0055 §1 as amended): with no + * required one, the object's ONLY `master_detail`, declared + * `required: false`, resolves too — an OPTIONAL master + * ({@link CbpRelation.optional}), picked by the spec's own resolver + * (`controlledByParentMasterOf`, which the parse judges with). An omitted + * `required` is not `false` on this unparsed read, so such an object still + * resolves `null`, as before. + * * A `controlled_by_parent` object that resolves no relation is REPORTED, once * per object per cache generation ({@link SecurityPlugin.reportCbpMasterUnresolved}), * so a stored or raw-registered object that relied on a retired tier is not @@ -4815,6 +4844,19 @@ export class SecurityPlugin implements Plugin { * object matches no row). Returns null when the object is not * controlled_by_parent, or when the caller is the system. * + * [ADR-0055 §1 and §2 as amended · #30905 ruling A] An OPTIONAL master + * ({@link CbpRelation.optional}) answers one `$or` of two arms that never + * share a row: the master arm above, unchanged, and a null-master arm for + * the rows whose reference is empty — every such row for a holder of View + * All on the detail (`permissionSets`; it opens nothing of the master arm), + * else the caller's own (`owner_id` equal to the caller), else none (an + * object without an `owner_id` column, {@link CbpRelation.ownerColumn}). + * A queue row (ADR-0142, no owner) is therefore View All's alone. The `$or` + * is this layer's answer, so every caller still ANDs it under the tenant + * wall and beside the detail's own layers. A required master's leaf is + * byte-identical to before. A cycle or a depth refusal still denies the + * whole object, null-master rows too: the declared shape is the defect. + * * [ADR-0055 amendment] The derivation COMPOSES ACROSS A CHAIN — a master * that is itself `controlled_by_parent` is scoped by ITS master, at every * level — and two guards bound the chain, both fail-CLOSED (`$in: []`, the @@ -4838,17 +4880,25 @@ export class SecurityPlugin implements Plugin { /** The engine verb the answer is composed onto; a write verb answers `null` (ADR-0139 D1). */ operation: string, context: any, + /** + * The permission sets resolved for `context`'s principal: read for View + * All on the detail by an optional master's null-master arm, and by + * nothing else (ADR-0055 §2 as amended). + */ + permissionSets: PermissionSet[], ): Record | null { // [#25223 cut 4] The body moved to `security-plugin-cbp-read-filter.ts` (see `CbpReadFilterHost`). const readLogger = () => this.logger; + const readPermissionEvaluator = () => this.permissionEvaluator; const readQl = () => this.ql; - const host: Pick = { + const host: Pick = { controlledByParentChainRefusal: (...a) => this.controlledByParentChainRefusal(...a), get logger() { return readLogger(); }, + get permissionEvaluator() { return readPermissionEvaluator(); }, get ql() { return readQl(); }, resolveCbpRelation: (...a) => this.resolveCbpRelation(...a), }; - return computeControlledByParentFilter.call(host, object, operation, context); + return computeControlledByParentFilter.call(host, object, operation, context, permissionSets); } /** diff --git a/packages/spec/api-surface/data.json b/packages/spec/api-surface/data.json index 7c659c35594..ee93f9a40f0 100644 --- a/packages/spec/api-surface/data.json +++ b/packages/spec/api-surface/data.json @@ -127,6 +127,7 @@ "ContextTokenPlaceholder (type)", "ContextTokenPlaceholderSchema (const)", "ContextTokenSchema (const)", + "ControlledByParentMaster (interface)", "ControlledByParentMasterRefusal (interface)", "ControlledByParentMasterShape (type)", "CrossFieldColumnVerdict (type)", @@ -882,6 +883,7 @@ "classifyDottedFilterHead (function)", "classifyFilterToken (function)", "containsUnresolvedPlaceholder (function)", + "controlledByParentMasterOf (function)", "controlledByParentMasterRefusal (function)", "controlledByParentOwnerMayEdit (function)", "countAuthorableFields (function)", diff --git a/packages/spec/export-origins/data.json b/packages/spec/export-origins/data.json index b2cdd04edb5..ceef70e5966 100644 --- a/packages/spec/export-origins/data.json +++ b/packages/spec/export-origins/data.json @@ -124,6 +124,7 @@ "ContextTokenPlaceholder": "src/data/context-tokens.zod.ts#ContextTokenPlaceholder (type)", "ContextTokenPlaceholderSchema": "src/data/context-tokens.zod.ts#ContextTokenPlaceholderSchema (const)", "ContextTokenSchema": "src/data/context-tokens.zod.ts#ContextTokenSchema (const)", + "ControlledByParentMaster": "src/data/controlled-by-parent-master.ts#ControlledByParentMaster (interface)", "ControlledByParentMasterRefusal": "src/data/controlled-by-parent-master.ts#ControlledByParentMasterRefusal (interface)", "ControlledByParentMasterShape": "src/data/controlled-by-parent-master.ts#ControlledByParentMasterShape (type)", "CrossFieldColumnVerdict": "src/data/filter-cross-field-comparison-class.ts#CrossFieldColumnVerdict (type)", @@ -868,6 +869,7 @@ "classifyDottedFilterHead": "src/data/filter-dotted-head.ts#classifyDottedFilterHead (function)", "classifyFilterToken": "src/data/context-tokens.zod.ts#classifyFilterToken (function)", "containsUnresolvedPlaceholder": "src/data/driver/common.zod.ts#containsUnresolvedPlaceholder (function)", + "controlledByParentMasterOf": "src/data/controlled-by-parent-master.ts#controlledByParentMasterOf (function)", "controlledByParentMasterRefusal": "src/data/controlled-by-parent-master.ts#controlledByParentMasterRefusal (function)", "controlledByParentOwnerMayEdit": "src/data/object.zod.ts#controlledByParentOwnerMayEdit (function)", "countAuthorableFields": "src/data/record-surface.ts#countAuthorableFields (function)", diff --git a/packages/spec/src/conversions/cbp-master-detail-required-defaulted.test.ts b/packages/spec/src/conversions/cbp-master-detail-required-defaulted.test.ts index 0ee223452a6..f56a03a1bce 100644 --- a/packages/spec/src/conversions/cbp-master-detail-required-defaulted.test.ts +++ b/packages/spec/src/conversions/cbp-master-detail-required-defaulted.test.ts @@ -18,9 +18,12 @@ import type { ConversionNotice } from './types.js'; * with the Required toggle off), which converts to `required: true` and then * parses; * - on the shapes it must leave alone, each of which the parse goes on - * refusing; - * - on the authoring funnel, which does not replay it, so a live author is - * still refused and taught. + * refusing (and the explicit `required: false`, which it leaves alone and + * the parse now accepts as the optional master, #30905 ruling A); + * - on the authoring funnel, which does not replay it. ⚠️ Since ruling A of + * #30905 the parse reads an omitted `required` as the parsed `false`, the + * optional master, so the funnel admits the omission rather than refusing + * it; `os lint` reports it at `error` (`relationship/master-detail-required`). */ const ID = 'cbp-master-detail-required-defaulted'; @@ -58,7 +61,11 @@ describe('cbp-master-detail-required-defaulted (ADR-0087 D2)', () => { it('a stored Studio-shape row (one master_detail, `required` omitted) converts to required:true, then parses whole', () => { const row = cbpRow({ opportunity: STUDIO_MASTER, name: { type: 'text', label: 'Name' } }); - expect(cbpIssuePaths(row), 'refused before the replay').toEqual(['sharingModel']); + // [#30905 ruling A] Before the replay the parse admits it as the optional + // master (the parsed map reads the omitted `required` as `false`); the + // conversion gives it the reading the row was stored with, the required + // master, exactly as before the ruling. + expect(cbpIssuePaths(row), 'admitted as optional before the replay').toEqual([]); const { item, notices } = convertStored(row); expect(notices).toHaveLength(1); @@ -73,8 +80,15 @@ describe('cbp-master-detail-required-defaulted (ADR-0087 D2)', () => { expect(convertStored(item).notices).toEqual([]); }); + it('leaves an explicit required:false untouched, and the parse accepts it as the optional master (#30905 ruling A)', () => { + const row = cbpRow({ m: { ...STUDIO_MASTER, required: false } }); + const { item, notices } = convertStored(row); + expect(notices).toEqual([]); + expect(item).toEqual(row); + expect(cbpIssuePaths(item)).toEqual([]); + }); + it.each([ - ['an explicit required:false (the builder refuses it too)', { m: { ...STUDIO_MASTER, required: false } }, 'sharingModel'], [ 'two master_details (no pick)', { a: STUDIO_MASTER, b: { type: 'master_detail', label: 'Quote', reference: 'crm_quote' } }, @@ -100,15 +114,18 @@ describe('cbp-master-detail-required-defaulted (ADR-0087 D2)', () => { expect(item.fields.m).toEqual(STUDIO_MASTER); }); - it('the authoring funnel does NOT replay it — defineStack still refuses the same shape and names the fix', () => { - expect(() => - defineStack({ - manifest: { id: 'com.example.cbp-defaulted', namespace: 'cbpd', version: '0.0.1', type: 'app', name: 'CBP' }, - objects: [ - { name: 'cbpd_header', label: 'Header', fields: { name: { type: 'text', label: 'Name' } } }, - { name: 'cbpd_line', label: 'Line', sharingModel: 'controlled_by_parent', fields: { header: { type: 'master_detail', label: 'H', reference: 'cbpd_header' } } }, - ], - } as never), - ).toThrow(/`header` is a `master_detail` without `required: true` — add it/); + it('the authoring funnel does NOT replay it — defineStack keeps the omission as the parse reads it (#30905 ruling A)', () => { + // Reversed, not deleted: this pinned defineStack refusing the omission and + // naming the fix. Since ruling A the parse admits the parsed `false` as the + // optional master; that the funnel does not replay the conversion is still + // the pin: the field comes out `required: false`, never forced to `true`. + const stack = defineStack({ + manifest: { id: 'com.example.cbp-defaulted', namespace: 'cbpd', version: '0.0.1', type: 'app', name: 'CBP' }, + objects: [ + { name: 'cbpd_header', label: 'Header', fields: { name: { type: 'text', label: 'Name' } } }, + { name: 'cbpd_line', label: 'Line', sharingModel: 'controlled_by_parent', fields: { header: { type: 'master_detail', label: 'H', reference: 'cbpd_header' } } }, + ], + } as never) as unknown as { objects: Array<{ fields: Record }> }; + expect(stack.objects[1]!.fields.header!.required).toBe(false); }); }); diff --git a/packages/spec/src/conversions/refused-shapes.test.ts b/packages/spec/src/conversions/refused-shapes.test.ts index e081eb9f7f1..fec9d2e5e50 100644 --- a/packages/spec/src/conversions/refused-shapes.test.ts +++ b/packages/spec/src/conversions/refused-shapes.test.ts @@ -127,12 +127,6 @@ describe('#23270 — a stored controlled_by_parent object without exactly one re 'objects[0].sharingModel', /declares 2 `master_detail` fields \(`quote`, `order`\), and which of them is the master is the owner's choice/, ], - [ - 'an explicit required: false', - { quote: md('crm_quote', { required: false }) }, - 'objects[0].sharingModel', - /`quote`, declares `required: false`, which the builder refuses rather than forcing/, - ], [ 'a required lookup only', { quote: { type: 'lookup', label: 'Quote', reference: 'crm_quote', required: true } }, @@ -187,6 +181,32 @@ describe('#23270 — a stored controlled_by_parent object without exactly one re expect(cbpIssue(item)?.path).toEqual(['fields']); }); + // [#30905 ruling A] The `master-not-required` arm retired for an explicit + // `required: false`: the only `master_detail` so declared is the optional + // master, a valid object, so a stored row in that shape is not listed, not + // rewritten, and parses. + it('the optional master — one master_detail, `required: false` — is NOT listed, not rewritten, and parses', () => { + const row = cbpRow('crm_line', { quote: md('crm_quote', { required: false }) }); + const { item, todos, notices } = storedTodos('object', row); + expect(notices.filter((n) => n.conversionId === 'cbp-master-detail-required-defaulted')).toEqual([]); + expect(item).toEqual(row); + expect(todos).toEqual([]); + expect(cbpIssue(row)).toBeUndefined(); + }); + + it('an optional master marked readonly: listed under the same entry, naming it optional', () => { + const row = cbpRow('crm_line', { quote: md('crm_quote', { required: false, readonly: true }) }); + const { item, todos } = storedTodos('object', row); + expect(item).toEqual(row); + expect(todos).toHaveLength(1); + const todo = todos[0]!; + expect(todo.conversionId).toBe('cbp-master-relation-tiers-retired'); + expect(todo.path).toBe('objects[0].fields'); + expect(todo.reason).toMatch(/Its one optional `master_detail`, `quote`, is marked `readonly` or `system`\./); + expect(todo.reason).toContain('declare the master reference as an ordinary field'); + expect(cbpIssue(item)?.path).toEqual(['fields']); + }); + it('the Studio shape — one master_detail, `required` omitted — converts and is NOT listed', () => { const row = cbpRow('crm_line', { opportunity: md('crm_opportunity') }); const { item, todos, notices } = storedTodos('object', row); diff --git a/packages/spec/src/conversions/refused-shapes.ts b/packages/spec/src/conversions/refused-shapes.ts index e4d01e4abf7..9b0bd7a78fd 100644 --- a/packages/spec/src/conversions/refused-shapes.ts +++ b/packages/spec/src/conversions/refused-shapes.ts @@ -66,8 +66,8 @@ import { type FlowNodeValueTemplateRefusal, } from '../automation/flow-value-slot-template.js'; import { + controlledByParentMasterOf, controlledByParentMasterRefusal, - isControlledByParentMasterField, type ControlledByParentMasterRefusal, } from '../data/controlled-by-parent-master.js'; import { duplicateActionNames } from '../data/duplicate-action-names.js'; @@ -146,12 +146,17 @@ function controlledByParentFate(refusal: ControlledByParentMasterRefusal, fields + 'the upgrade never picks one. Keep `master_detail` with `required: true` on the master and ' + `declare each other parent as a \`lookup\`, ${ownSharingModel}.`; case 'master-not-required': { + // [#30905 ruling A] An explicit `required: false` is the optional master + // and is no longer listed: this arm retired for it. What is left is a + // `required` that is neither boolean, which the field parse refuses on + // its own (an omitted one is converted by the chain before this reads). const name = refusal.masterDetails[0]!; const def = fields[name]; const written = isDict(def) && 'required' in def ? JSON.stringify(def.required) : 'nothing'; - return `Its one \`master_detail\`, \`${name}\`, declares \`required: ${written}\`, which the builder ` - + 'refuses rather than forcing, so the upgrade does not overrule it. Declare `required: true` on it, ' - + `${ownSharingModel}.`; + return `Its one \`master_detail\`, \`${name}\`, declares \`required: ${written}\`, which is neither ` + + 'the required master nor the optional one, so the upgrade does not guess. Declare `required: true` ' + + '(every row names its master) or `required: false` (a row may name none, and is then readable only ' + + `by its owner and by View All on the object) on it, ${ownSharingModel}.`; } case 'required-lookup-only': return `It declares no \`master_detail\`, and ${fieldList(refusal.requiredLookups)} ` @@ -165,11 +170,11 @@ function controlledByParentFate(refusal: ControlledByParentMasterRefusal, fields + 'and nothing to convert. Add a `master_detail` field with `required: true` referencing the master ' + `object, ${ownSharingModel}.`; case 'master-not-writable': { - const master = refusal.masterDetails.find((n) => isControlledByParentMasterField(fields[n]))!; - return `Its one required \`master_detail\`, \`${master}\`, ` + const master = controlledByParentMasterOf(fields)!; + return `Its one ${master.optional ? 'optional' : 'required'} \`master_detail\`, \`${master.field}\`, ` + 'is marked `readonly` or `system`. Dropping the flag changes who may write the reference, so the ' - + 'upgrade does not write it. Remove the flag and declare the master reference as an ordinary required ' - + `field, ${ownSharingModel}.`; + + 'upgrade does not write it. Remove the flag and declare the master reference as an ordinary ' + + `${master.optional ? '' : 'required '}field, ${ownSharingModel}.`; } } } @@ -206,7 +211,8 @@ const controlledByParentWithoutOneMaster: RefusedStoredShape = { path: `${path}.${refusal.path}`, from: JSON.stringify({ sharingModel: obj.sharingModel, fields: relation }), reason: 'No lossless rewrite: protocol 18 derives a controlled-by-parent object\'s master from exactly ' - + 'one writable `master_detail` declared `required: true`, and this object does not declare exactly one. ' + + 'one writable `master_detail` declared `required: true`, or, with none, from its only `master_detail` ' + + 'declared `required: false`, and this object does not declare exactly one. ' + `${controlledByParentLeftAsStored(refusal)} ` + `${controlledByParentFate(refusal, fields)} ${refusal.message}`, }); diff --git a/packages/spec/src/data/controlled-by-parent-master.test.ts b/packages/spec/src/data/controlled-by-parent-master.test.ts index b2c9d7d765b..af0cfeada85 100644 --- a/packages/spec/src/data/controlled-by-parent-master.test.ts +++ b/packages/spec/src/data/controlled-by-parent-master.test.ts @@ -6,6 +6,7 @@ import { ZodError } from 'zod'; // Read through the `@objectstack/spec/data` entry, not the module: the pin is // on the PUBLISHED predicate a producer imports (#23272). import { + controlledByParentMasterOf, controlledByParentMasterRefusal, isControlledByParentMasterField, type ControlledByParentMasterShape, @@ -55,8 +56,26 @@ const SHAPES: Array<[string, Record, ControlledByParentMasterSh 'several-required-masters', ], ['two master_details, none required', { a: md('work_order'), b: md('account') }, 'several-masters'], - ['one master_detail, required: false', { order: md('work_order', { required: false }) }, 'master-not-required'], - ['one master_detail, required omitted', { order: md('work_order') }, 'master-not-required'], + // [#30905 ruling A · ADR-0055 §1 as amended] The only master_detail, + // declared `required: false`, is the optional master: accepted. + ['one master_detail, required: false', { order: md('work_order', { required: false }) }, undefined], + [ + 'one master_detail, required: false, beside a required lookup', + { order: md('work_order', { required: false }), owner_ref: lookup('account', { required: true }) }, + undefined, + ], + [ + 'one required master_detail beside a master_detail declared required: false', + { order: md('work_order', { required: true }), other: md('account', { required: false }) }, + undefined, + ], + [ + 'two master_details, both required: false', + { a: md('work_order', { required: false }), b: md('account', { required: false }) }, + 'several-masters', + ], + ['one master_detail, required: false, readonly: true', { order: md('work_order', { required: false, readonly: true }) }, 'master-not-writable'], + ['one master_detail, required: false, system: true', { order: md('work_order', { required: false, system: true }) }, 'master-not-writable'], ['a required lookup only', { order: lookup('work_order', { required: true }) }, 'required-lookup-only'], ['no relation', { note: { type: 'text', label: 'Note' } }, 'no-master'], // [#25092] The one master is writable: `readonly` / `system` on it are refused. @@ -96,6 +115,27 @@ describe('controlledByParentMasterRefusal — the export and ObjectSchema agree } }); + // [#30905 ruling A] The ONE deliberate divergence. The predicate reads the + // raw `required`, and an omitted one is neither master; the parse reads the + // parsed map, where an omitted `required` is `false`, so it admits the + // object as the optional master. The builder sees the omission and forces + // `required: true` (the required master); `os lint` reports it at `error`. + it('an omitted required on the only master_detail: the predicate refuses it, the parse admits it as optional', () => { + const obj = { name: 'work_order_item', label: 'Work Order Item', sharingModel: 'controlled_by_parent', fields: { order: md('work_order') } }; + const refusal = controlledByParentMasterRefusal(obj.name, obj.sharingModel, obj.fields); + expect(refusal?.shape).toBe('master-not-required'); + expect(refusal?.message).toMatch(/`order` is a `master_detail` that declares no `required` — declare `required: true` \(every row names its master\) or `required: false` \(a row may name none\)/); + expect(ObjectSchema.safeParse(obj).success).toBe(true); + // Read on the parsed map, the predicate agrees with the parse. + const parsed = ObjectSchema.parse(obj); + expect(controlledByParentMasterRefusal(parsed.name, parsed.sharingModel, parsed.fields)).toBeUndefined(); + expect(controlledByParentMasterOf(parsed.fields)).toEqual({ field: 'order', optional: true }); + // The builder sees the omission and reads it as the required master. + const built = ObjectSchema.create(obj as Parameters[0]); + expect(built.fields.order!.required).toBe(true); + expect(controlledByParentMasterOf(built.fields)).toEqual({ field: 'order', optional: false }); + }); + it('answers undefined for any other sharing model, as the parse accepts it', () => { for (const sharingModel of ['private', 'public_read', 'public_read_write']) { const obj = { name: 'pair', label: 'Pair', sharingModel, fields: { a: md('x', { required: true }), b: md('y', { required: true }) } }; @@ -123,16 +163,40 @@ describe('isControlledByParentMasterField — the field half the refusal counts expect(isControlledByParentMasterField(field)).toBe(expected); }); - it('is the count the refusal makes: exactly one counting field, and a writable one, satisfies the rule', () => { + it('is the count the refusal makes: one master (the required one, else the only optional one), writable, satisfies the rule', () => { for (const [, fields] of SHAPES) { const counted = Object.values(fields).filter(isControlledByParentMasterField) as Array>; - const writable = counted.length === 1 && counted[0]!.readonly !== true && counted[0]!.system !== true; + const master = controlledByParentMasterOf(fields); + // [#30905 ruling A] One required field is the master; with none, the + // resolver's optional one is. + expect(master?.optional === false).toBe(counted.length === 1); + const def = master === undefined ? undefined : (fields[master.field] as Record); + const writable = def !== undefined && def.readonly !== true && def.system !== true; const refusal = controlledByParentMasterRefusal('o', 'controlled_by_parent', fields); expect(refusal === undefined).toBe(writable); } }); }); +describe('controlledByParentMasterOf — the one master, required or optional (#30905 ruling A)', () => { + it.each([ + ['one required master_detail', { order: md('w', { required: true }) }, { field: 'order', optional: false }], + ['one master_detail, required: false', { order: md('w', { required: false }) }, { field: 'order', optional: true }], + ['a required one beside one declared false', { a: md('w', { required: true }), b: md('x', { required: false }) }, { field: 'a', optional: false }], + // The flags are the refusal's to judge: the field is still the master. + ['an optional one marked readonly', { order: md('w', { required: false, readonly: true }) }, { field: 'order', optional: true }], + ['one master_detail, required omitted (read raw: neither)', { order: md('w') }, undefined], + ['one master_detail, required "false" (not a boolean)', { order: md('w', { required: 'false' }) }, undefined], + ['two required', { a: md('w', { required: true }), b: md('x', { required: true }) }, undefined], + ['two declared false', { a: md('w', { required: false }), b: md('x', { required: false }) }, undefined], + ['a required lookup only', { order: lookup('w', { required: true }) }, undefined], + ['no field map', undefined, undefined], + ['an array', [md('w', { required: false })], undefined], + ])('%s', (_label, fields, expected) => { + expect(controlledByParentMasterOf(fields)).toEqual(expected); + }); +}); + describe('#25092 a one master marked readonly or system is refused at authoring, through the one rule', () => { it('names the object, the field and every flag, and prescribes dropping them', () => { const refusal = controlledByParentMasterRefusal('work_order_item', 'controlled_by_parent', { diff --git a/packages/spec/src/data/controlled-by-parent-master.ts b/packages/spec/src/data/controlled-by-parent-master.ts index 98db836bcf7..7ce29c82047 100644 --- a/packages/spec/src/data/controlled-by-parent-master.ts +++ b/packages/spec/src/data/controlled-by-parent-master.ts @@ -1,9 +1,13 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [ADR-0055 §1 · #23100, maintainer ruling B] The ONE predicate behind "a - * `controlled_by_parent` object derives its master from exactly one - * `master_detail` field declared `required: true`". + * [ADR-0055 §1 · #23100, maintainer ruling B, as amended by ruling A of + * #30905] The ONE predicate behind "a `controlled_by_parent` object derives + * its master from exactly one `master_detail` field": the one declared + * `required: true`, or, when the object declares no required one, its only + * `master_detail` declared `required: false` (an OPTIONAL master: a detail row + * may name none, and such a row is readable by its owner and by View All on + * the detail, never through a master). * * Two readers, one judgement: * @@ -26,15 +30,33 @@ * yes. A producer choosing a default asks it with `'controlled_by_parent'` * as the sharing model it is about to write; * - {@link isControlledByParentMasterField} answers the field-level half, - * "does this field count as a master?", which the refusal counts. A reader - * that lists the candidates itself (`@objectstack/lint`'s - * `cbpMasterCandidates`, which names each candidate's `reference` in its - * finding) reads this, so what counts as a master is spelled here once. + * "does this field count as a REQUIRED master?", which the refusal counts: + * two of them are two masters whatever else the object declares; + * - {@link controlledByParentMasterOf} answers "which field is the one + * master, and is it optional?" on a field map. The optional master is an + * object-level judgement, not a field-level one: a `master_detail` declared + * `required: false` is the master only where no required one stands beside + * it, so a reader that resolves the master (the security runtime's + * `resolveCbpRelation`, the engine's boot refusal of an unservable master, + * `@objectstack/lint`'s relation rules) reads this, and what counts as the + * master is spelled here once. * * Read on either the PARSED field map or a stored body: `required` is * `z.boolean().default(false)`, so on the parsed map an omitted `required` is - * `false`, and on a stored body it is `undefined`. The predicate counts a - * master only where `required === true`, so the two give the same verdict. + * `false`, and on a stored body it is `undefined`. The required master counts + * only where `required === true` and the optional master only where + * `required === false`, so a body that writes no `required` at all on its one + * `master_detail` derives no master here (shape `master-not-required`). That + * is the one place the two readings differ, and it is deliberate: the spec + * names an OMITTED `required` a required master everywhere it can see the + * omission — `ObjectSchema.create()` forces it to `true` + * (`forceCbpMasterDetailRequired`), the stored-row seams convert it to `true` + * (the D2 conversion `cbp-master-detail-required-defaulted`), and + * `@objectstack/lint`'s `relationship/master-detail-required` reports it at + * `error` — and an object registered without the parse keeps failing closed + * on it. A raw parse cannot see the omission (the field default has already + * written `false` when the object refinement runs), so there an omitted + * `required` reads as the optional master. * `readonly` (`z.boolean().default(false)`) and `system` (optional) are read * the same way: only `=== true` marks the one master unwritable. */ @@ -47,16 +69,19 @@ * `required: true`; choosing the master is the owner's call; * - `several-masters` — two or more `master_detail` fields and none declared * `required: true`; again a pick; - * - `master-not-required` — exactly one `master_detail`, not declared - * `required: true`. An OMITTED `required` is converted by the D2 entry - * `cbp-master-detail-required-defaulted` before this is read, so on a stored - * row that has been through the chain this is an explicit `required: false` - * (or a non-boolean the field parse refuses on its own); + * - `master-not-required` — exactly one `master_detail`, declaring neither + * `required: true` nor `required: false`: an OMITTED `required` on a body no + * parse has defaulted (or a non-boolean the field parse refuses on its own). + * The D2 entry `cbp-master-detail-required-defaulted` converts the omitted + * key before a stored row is read, so a stored row that has been through the + * chain never carries it, and a parsed field map never does either. An + * explicit `required: false` is not this shape: it is the optional master + * (ADR-0055 §1 as amended, #30905); * - `required-lookup-only` — no `master_detail`, one or more required `lookup` * fields (the retired third tier); * - `no-master` — no `master_detail` and no required `lookup`; - * - `master-not-writable` — exactly one required `master_detail`, declared - * `readonly: true` or `system: true`. ADR-0055 §1 derives the master from a + * - `master-not-writable` — the one master (required, or optional), + * declared `readonly: true` or `system: true`. ADR-0055 §1 derives the master from a * reference every write of a detail supplies, and those two flags mark a * field an author's write does not supply (one the platform fills, or one a * write may not set), so it cannot be the one master. Dropping the flag is @@ -87,12 +112,13 @@ export interface ControlledByParentMasterRefusal { } /** - * Does this field count as a `controlled_by_parent` master under ADR-0055 §1? - * A `master_detail` field declared `required: true`, and nothing else: a - * non-required `master_detail` and a required `lookup` are the two tiers - * ruling B retired (#23100). An object satisfies the rule when EXACTLY ONE of - * its fields counts, and that one is writable (neither `readonly: true` nor - * `system: true`) — {@link controlledByParentMasterRefusal} is that judgement. + * Does this field count as a REQUIRED `controlled_by_parent` master under + * ADR-0055 §1? A `master_detail` field declared `required: true`, and nothing + * else: a required `lookup` is the tier ruling B retired (#23100), and a + * `master_detail` declared `required: false` is the optional master only when + * no required one stands beside it, which a field alone cannot say — + * {@link controlledByParentMasterOf} judges that on the field map. An object + * with two counting fields has two masters, whatever else it declares. * The flags are judged on the one master, not here: a flagged required * `master_detail` is still the field the runtime resolves as the master, so a * second one beside it is still two masters. @@ -106,6 +132,53 @@ export function isControlledByParentMasterField(field: unknown): boolean { return f.type === 'master_detail' && f.required === true; } +/** The one master a `controlled_by_parent` object derives its access from — see {@link controlledByParentMasterOf}. */ +export interface ControlledByParentMaster { + /** The master reference field's key: the detail's master FK column. */ + readonly field: string; + /** + * `true` when the master is declared `required: false` (ADR-0055 §1 as + * amended, #30905): a detail row may name no master, and such a row is read + * by its owner and by View All on the detail, never through a master. `false` + * for the required master. + */ + readonly optional: boolean; +} + +/** + * Which field of this field map is the `controlled_by_parent` master, and is + * it optional? (ADR-0055 §1 as amended, #30905.) + * + * - exactly one `master_detail` declared `required: true` + * ({@link isControlledByParentMasterField}) is the master, whatever else the + * map declares — a second `master_detail` declared `required: false` beside + * it is an ordinary parent, as it was before the optional master existed; + * - otherwise, when the map declares exactly ONE `master_detail` and it is + * declared `required: false`, that field is the optional master; + * - anything else (two required masters, two `master_detail` fields none of + * them required, one whose `required` is not written, a required `lookup` + * only, nothing) names no master: `undefined`. + * + * Judges the master only. Whether the one master is writable (`readonly` / + * `system`) is {@link controlledByParentMasterRefusal}'s question, and the + * sharing model is the caller's: this reads any field map. Reads `required` + * as written (see the module header), and accepts any value: a non-object + * map names no master. + */ +export function controlledByParentMasterOf(fields: unknown): ControlledByParentMaster | undefined { + if (fields === null || typeof fields !== 'object' || Array.isArray(fields)) return undefined; + const entries = Object.entries(fields as Record); + const required = entries.filter(([, f]) => isControlledByParentMasterField(f)); + if (required.length === 1) return { field: required[0]![0], optional: false }; + if (required.length > 1) return undefined; + const masterDetails = entries.filter( + ([, f]) => f !== null && typeof f === 'object' && (f as { type?: unknown }).type === 'master_detail', + ); + if (masterDetails.length !== 1) return undefined; + const [field, def] = masterDetails[0]!; + return (def as { required?: unknown }).required === false ? { field, optional: true } : undefined; +} + /** * The writability flags a `controlled_by_parent` master may not carry, in the * order a refusal names them. See the `master-not-writable` shape. @@ -113,9 +186,11 @@ export function isControlledByParentMasterField(field: unknown): boolean { const UNWRITABLE_MASTER_FLAGS = ['readonly', 'system'] as const; /** - * Judge one object against ADR-0055 §1's one-required-master rule. Returns - * `undefined` when the rule does not apply (another sharing model, no field - * map) or the object satisfies it; otherwise the refusal. + * Judge one object against ADR-0055 §1's one-master rule (as amended, + * #30905): exactly one required `master_detail`, or, with none, exactly one + * `master_detail` declared `required: false`; and that one master writable. + * Returns `undefined` when the rule does not apply (another sharing model, no + * field map) or the object satisfies it; otherwise the refusal. */ export function controlledByParentMasterRefusal( objectName: unknown, @@ -131,24 +206,25 @@ export function controlledByParentMasterRefusal( .filter(([, f]) => f?.type === 'lookup' && f.required === true) .map(([key]) => key); const masters = entries.filter(([, f]) => isControlledByParentMasterField(f)).map(([key]) => key); - if (masters.length === 1) { - const master = masters[0]!; - const def = (fields as Record | undefined>)[master]; + const master = controlledByParentMasterOf(fields); + if (master !== undefined) { + const def = (fields as Record | undefined>)[master.field]; const flags = UNWRITABLE_MASTER_FLAGS.filter((flag) => def?.[flag] === true); if (flags.length === 0) return undefined; + const which = master.optional ? 'one optional `master_detail` (declared `required: false`)' : 'one required `master_detail`'; return { path: 'fields', shape: 'master-not-writable', masterDetails, requiredLookups, message: - `Object '${name}' declares sharingModel 'controlled_by_parent' with its one required \`master_detail\`, ` - + `\`${master}\`, marked ${flags.map((f) => `\`${f}: true\``).join(' and ')}. ADR-0055 §1: since ` - + '@objectstack/spec 18, a controlled-by-parent object derives its master from exactly one required ' + `Object '${name}' declares sharingModel 'controlled_by_parent' with its ${which}, ` + + `\`${master.field}\`, marked ${flags.map((f) => `\`${f}: true\``).join(' and ')}. ADR-0055 §1: since ` + + '@objectstack/spec 18, a controlled-by-parent object derives its master from exactly one ' + '`master_detail` field that every write of a detail supplies, and a field marked `readonly` or ' + '`system` is not one an author\'s write supplies, so this reference cannot be that master. ' - + `Remove ${flags.map((f) => `\`${f}: true\``).join(' and ')} from \`${master}\` and declare the ` - + 'master reference as an ordinary required field; if the rows must outlive their parent, this object ' + + `Remove ${flags.map((f) => `\`${f}: true\``).join(' and ')} from \`${master.field}\` and declare the ` + + `master reference as an ordinary ${master.optional ? '' : 'required '}field; if the rows must outlive their parent, this object ` + "is not a detail: give it its own sharingModel ('private', 'public_read' or 'public_read_write').", }; } @@ -168,7 +244,16 @@ export function controlledByParentMasterRefusal( }; } const found = [ - ...masterDetails.map((m) => `\`${m}\` is a \`master_detail\` without \`required: true\` — add it`), + ...(masterDetails.length > 1 + ? [ + `${masterDetails.map((m) => `\`${m}\``).join(', ')} are \`master_detail\` fields and none declares ` + + '`required: true`, so none of them is the master — keep `master_detail` on the one master and ' + + 'declare every other parent as a `lookup`', + ] + : masterDetails.map( + (m) => `\`${m}\` is a \`master_detail\` that declares no \`required\` — declare \`required: true\` ` + + '(every row names its master) or `required: false` (a row may name none)', + )), ...requiredLookups.map( (l) => `\`${l}\` is a required \`lookup\`, and a lookup derives no access — declare it ` + "`type: 'master_detail'` (a detail is deleted with its master; declare `deleteBehavior: 'restrict'` " @@ -189,10 +274,12 @@ export function controlledByParentMasterRefusal( masterDetails, requiredLookups, message: - `Object '${name}' declares sharingModel 'controlled_by_parent' but no required \`master_detail\` field. ` + `Object '${name}' declares sharingModel 'controlled_by_parent' but no required \`master_detail\` field, ` + + 'nor a single `master_detail` declared `required: false`. ' + 'ADR-0055 §1: since @objectstack/spec 18, a controlled-by-parent object derives its master from exactly ' - + 'one `master_detail` field declared `required: true`, and from nothing else, so as declared the ' - + 'platform derives no master ' + + 'one `master_detail` field declared `required: true`, or, with none, from its only `master_detail` ' + + 'declared `required: false` (an optional master: a row that names none is readable only by its owner ' + + 'and by View All on the object), and from nothing else, so as declared the platform derives no master ' + '(every read is denied, every write refused). ' + (found.length > 0 ? `${found.join('; ')}. ` : '') + 'If the rows must outlive their parent, this object is not a detail: give it its own sharingModel ' diff --git a/packages/spec/src/data/object.test.ts b/packages/spec/src/data/object.test.ts index cc34dd89cc7..ee42ba8bb54 100644 --- a/packages/spec/src/data/object.test.ts +++ b/packages/spec/src/data/object.test.ts @@ -1510,27 +1510,43 @@ describe('ObjectSchema.create() forces a required master_detail under controlled expect((obj.fields as Record).parent.required).toBe(true); }); - it('REFUSES an explicit required: false, loudly, naming object + field + the fix', () => { + it('KEEPS an explicit required: false on the only master_detail — the optional master (#30905 ruling A)', () => { + // Reversed, not deleted: this pinned the builder REFUSING the explicit + // `false`. ADR-0055 §1 as amended makes it the optional master, so the + // builder passes it to the parse as written, and the parse accepts it. + const obj = ObjectSchema.create({ + name: 'cbp_optional', + sharingModel: 'controlled_by_parent', + fields: { + parent: { type: 'master_detail', reference: 'cbp_header', required: false }, + note: { type: 'text' }, + }, + }); + expect((obj.fields as Record).parent.required).toBe(false); + }); + + it('still REFUSES an explicit required: false beside another master_detail, loudly, naming object + both fields + the fix', () => { let message = ''; try { ObjectSchema.create({ name: 'cbp_bad', sharingModel: 'controlled_by_parent', fields: { - parent: { type: 'master_detail', reference: 'cbp_header', required: false }, + header: { type: 'master_detail', reference: 'cbp_header', required: true }, + parent: { type: 'master_detail', reference: 'cbp_other', required: false }, }, }); - throw new Error('expected ObjectSchema.create to refuse required: false under controlled_by_parent'); + throw new Error('expected ObjectSchema.create to refuse required: false beside another master_detail'); } catch (e) { message = (e as Error).message; } expect(message).not.toContain('expected ObjectSchema.create to refuse'); expect(message).toContain("ObjectSchema.create('cbp_bad')"); - expect(message).toContain('`parent`'); - expect(message).toContain('required: false'); + expect(message).toContain('field `parent` declares `required: false`'); + expect(message).toContain('beside another `master_detail` (`header`)'); expect(message).toContain('controlled_by_parent'); // The message carries the prescription, not just the verdict. - expect(message).toContain('Remove `required: false`'); + expect(message).toContain('Declare every parent other than the master as a `lookup`'); expect(message).toContain('change its `sharingModel`'); }); @@ -1566,14 +1582,12 @@ describe('ObjectSchema.create() forces a required master_detail under controlled expect((explicit.fields as Record).parent.required).toBe(false); }); - it('raw .parse()/.safeParse() do NOT force the shape — they refuse it (#23100 ruling B)', () => { - // Reversed, not deleted: this pinned the raw parse TOLERATING a - // non-required master reference "so metadata at rest keeps loading". The - // runtime's tolerance it leaned on (any `master_detail`, then a required - // `lookup`) retired with ruling B, so the parse now refuses the shape where - // an author is present to act; stored rows still register under their - // `[metadata_spec_invalid]` diagnostic. And still UNREWRITTEN: the issue - // names the object, it does not repair it. + it('raw .parse()/.safeParse() do NOT force the shape — they judge the one master (#23100 ruling B, #30905 ruling A)', () => { + // Reversed twice, not deleted. First this pinned the raw parse TOLERATING + // a non-required master reference; ruling B (#23100) made it refuse one. + // Ruling A (#30905) makes the only `master_detail` declared + // `required: false` the optional master, so the stored row is accepted — + // and still UNREWRITTEN: the parse forces nothing. const atRest = { name: 'cbp_stored', sharingModel: 'controlled_by_parent', @@ -1582,11 +1596,24 @@ describe('ObjectSchema.create() forces a required master_detail under controlled }, }; const result = ObjectSchema.safeParse(atRest); - expect(result.success).toBe(false); + expect(result.success).toBe(true); expect(atRest.fields.parent.required).toBe(false); - if (!result.success) { - expect(result.error.issues.map((i) => i.path.join('.'))).toEqual(['sharingModel']); - expect(result.error.issues[0].message).toMatch(/^Object 'cbp_stored' declares sharingModel 'controlled_by_parent' but no required `master_detail` field\./); + if (result.success) expect((result.data.fields as Record).parent.required).toBe(false); + // Two `master_detail` fields with none required name no master: refused, + // unrewritten, at the shared `sharingModel` path. + const two = { + name: 'cbp_stored_two', + sharingModel: 'controlled_by_parent', + fields: { + a: { type: 'master_detail', reference: 'cbp_header', required: false }, + b: { type: 'master_detail', reference: 'cbp_other', required: false }, + }, + }; + const refused = ObjectSchema.safeParse(two); + expect(refused.success).toBe(false); + if (!refused.success) { + expect(refused.error.issues.map((i) => i.path.join('.'))).toEqual(['sharingModel']); + expect(refused.error.issues[0].message).toMatch(/^Object 'cbp_stored_two' declares sharingModel 'controlled_by_parent' but no required `master_detail` field, nor a single `master_detail` declared `required: false`\./); } }); }); @@ -1621,7 +1648,7 @@ describe('ObjectSchema refuses a controlled_by_parent object without exactly one const issues = issuesOf(cbp({ parent: { type: 'lookup', reference: 'cbp_header', required: true } })); expect(issues).toHaveLength(1); expect(issues[0].path).toBe('sharingModel'); - expect(issues[0].message).toMatch(/^Object 'cbp_line' declares sharingModel 'controlled_by_parent' but no required `master_detail` field\. ADR-0055 §1:/); + expect(issues[0].message).toMatch(/^Object 'cbp_line' declares sharingModel 'controlled_by_parent' but no required `master_detail` field, nor a single `master_detail` declared `required: false`\. ADR-0055 §1:/); expect(issues[0].message).toContain("`parent` is a required `lookup`"); // The builder path refuses it too — the HotCRM shape. expect(() => ObjectSchema.create(cbp({ parent: Field.lookup('cbp_header', { label: 'Header', required: true }) }))).toThrow( @@ -1629,15 +1656,27 @@ describe('ObjectSchema refuses a controlled_by_parent object without exactly one ); }); - it('REFUSES a non-required master_detail as the only relation (the retired second tier) — omitted or false', () => { - for (const parent of [ - { type: 'master_detail', reference: 'cbp_header' }, - { type: 'master_detail', reference: 'cbp_header', required: false }, - ]) { - const issues = issuesOf(cbp({ parent })); - expect(issues.map((i) => i.path)).toEqual(['sharingModel']); - expect(issues[0].message).toContain('`parent` is a `master_detail` without `required: true`'); - } + it('ACCEPTS the only master_detail declared required: false — the optional master (#30905 ruling A)', () => { + // Reversed, not deleted: this pinned the retired second tier — a + // non-required `master_detail` as the only relation — refused, omitted or + // false. Ruling A admits the explicit `false` as the optional master. On + // the parsed map an omitted `required` IS `false` (the default), so the + // raw parse admits that spelling too: the recorded divergence, which the + // builder (it forces `true`) and `os lint` (it reports the omission at + // `error`) see past. + expect(issuesOf(cbp({ parent: { type: 'master_detail', reference: 'cbp_header', required: false } }))).toEqual([]); + expect(issuesOf(cbp({ parent: { type: 'master_detail', reference: 'cbp_header' } }))).toEqual([]); + // Two of them name no master: still refused, naming both and the fix. + const two = issuesOf(cbp({ + a: { type: 'master_detail', reference: 'master_a', required: false }, + b: { type: 'master_detail', reference: 'master_b' }, + })); + expect(two.map((i) => i.path)).toEqual(['sharingModel']); + expect(two[0].message).toContain('`a`, `b` are `master_detail` fields and none declares `required: true`'); + // `readonly` / `system` on the optional master: still refused. + const flagged = issuesOf(cbp({ parent: { type: 'master_detail', reference: 'cbp_header', required: false, readonly: true } })); + expect(flagged.map((i) => i.path)).toEqual(['fields']); + expect(flagged[0].message).toContain('with its one optional `master_detail` (declared `required: false`), `parent`, marked `readonly: true`'); }); it('REFUSES no relation at all, and two required master_details, at the two lint-shared paths', () => { @@ -1670,7 +1709,14 @@ describe('ObjectSchema refuses a controlled_by_parent object without exactly one ], }); expect(() => defineStack(stack({ header: { type: 'master_detail', label: 'H', reference: 'cbpo_header', required: true } }) as never)).not.toThrow(); - expect(() => defineStack(stack({ header: { type: 'master_detail', label: 'H', reference: 'cbpo_header' } }) as never)).toThrow( + // [#30905 ruling A] The optional master reaches defineStack accepted, and + // so does an omitted `required` (the parsed map reads it `false`). + expect(() => defineStack(stack({ header: { type: 'master_detail', label: 'H', reference: 'cbpo_header', required: false } }) as never)).not.toThrow(); + expect(() => defineStack(stack({ header: { type: 'master_detail', label: 'H', reference: 'cbpo_header' } }) as never)).not.toThrow(); + expect(() => defineStack(stack({ + header: { type: 'master_detail', label: 'H', reference: 'cbpo_header' }, + other: { type: 'master_detail', label: 'O', reference: 'cbpo_header', required: false }, + }) as never)).toThrow( /objects\.1\.sharingModel: Object 'cbpo_line' declares sharingModel 'controlled_by_parent' but no required `master_detail` field/, ); }); diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index 0eca424ae49..895abe65dbe 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -33,10 +33,11 @@ import { SINGLE_OPTION_TYPES } from './field-value.zod'; // `password` exemption rather than restating it. No runtime cycle: that module's // only imports are type-only. import { isMaskedOnReadFieldType } from './masked-field-types'; -// The one-required-master predicate, shared with the v18 stored-row listing +// The one-master predicate, shared with the v18 stored-row listing // (`conversions/refused-shapes.ts`) so the parse and the upgrade's preview // refuse one set. Public: re-exported from `@objectstack/spec/data`, with the -// field predicate `isControlledByParentMasterField` it counts. +// field predicate `isControlledByParentMasterField` it counts and the +// resolver `controlledByParentMasterOf` it judges the one master with. import { controlledByParentMasterRefusal } from './controlled-by-parent-master'; // The ADR-0015 §4.2 binding predicate, shared with `defineStack`'s // cross-reference pass and the v18 stored-row listing. Internal to the spec. @@ -2760,6 +2761,17 @@ function refuseFieldsInRelatedListGroups(fieldGroups: unknown, fields: unknown, * derives its master from EXACTLY ONE `master_detail` field declared * `required: true`. The parse refuses the object otherwise — zero such fields, * or two or more — naming the object and the rule, at + * + * ⚠️ Amended by ruling A of #30905 (ADR-0055 §1 as amended): with no required + * one, the object's ONLY `master_detail`, declared `required: false`, is its + * master too, an OPTIONAL one — a detail row may name no master, and such a + * row is read by its owner and by View All on the detail. On the parsed map an + * omitted `required` is `false` (see below), so this refinement admits it as + * the optional master as well: the parse cannot tell the two spellings apart. + * The omission is caught where it is visible — `ObjectSchema.create()` forces + * it to `true`, the stored-row seams convert it to `true`, and `os lint`'s + * `relationship/master-detail-required` reports it at `error`. The rest of + * this docblock records the rule as first landed and reads with that change: * `['sharingModel']` (nothing to derive from) or `['fields']` (more than one * candidate), the same two locations `os lint` reports * `security-controlled-by-parent-no-relation` / @@ -2790,7 +2802,9 @@ function refuseFieldsInRelatedListGroups(fieldGroups: unknown, fields: unknown, * refusal every door that parses meets. * * Read on the PARSED field map: `required` is `z.boolean().default(false)`, so - * an omitted `required` here is `false` — and is refused. That is the builder's + * an omitted `required` here is `false` — and is refused. ⚠️ Since ruling A of + * #30905, `false` on the object's only `master_detail` is the optional master, + * so an omitted one is admitted as that (see above). That is the builder's * own contract seen from the other side: `ObjectSchema.create()` forces an * omitted `required` to `true` on every `master_detail` under this sharing * model ({@link forceCbpMasterDetailRequired}), so an object authored through @@ -2806,7 +2820,9 @@ function refuseFieldsInRelatedListGroups(fieldGroups: unknown, fields: unknown, * diagnostic carrying this message (ADR-0087's 2026-07-31 addendum, "reads * diagnose, never drop"), and the save door refuses it with 422. The other * shapes are not converted: - * - an explicit `required: false` — the builder refuses it, too; + * - an explicit `required: false` — the builder refuses it, too (⚠️ until + * ruling A of #30905; on the object's only `master_detail` it is the + * optional master now, accepted as stored); * - two masters — choosing one is the pick §1 refuses; * - a required `lookup` — turning it into a `master_detail` changes what * deleting the master does to every row, and whether the object should @@ -3719,7 +3735,10 @@ const ObjectSchemaBase = strictObject( * master-detail relationship: its access is *derived* from the master record * — a user sees/edits a detail only if they can see/edit its master. The * object must declare exactly one required `master_detail` field identifying - * the master; the security layer auto-injects `masterFK IN (accessible master + * the master — or, with none, exactly one `master_detail` declared + * `required: false`, an optional master: a detail row that names no master is + * read by its owner (`owner_id`) and by View All on the detail (ADR-0055 §1 + * and §2 as amended); the security layer auto-injects `masterFK IN (accessible master * ids)` on reads and requires master edit-access on by-id writes. No RLS policy * is authored — the inheritance is derived from the relationship. */ @@ -4083,9 +4102,10 @@ const ObjectSchemaBase = strictObject( // [#27069] A field group carrying `relatedList` is a body table: no field // joins it. See the helper's docblock. refuseFieldsInRelatedListGroups(object.fieldGroups, object.fields, ctx); - // [ADR-0055 §1 · #23100 ruling B] Exactly one required `master_detail` - // under `controlled_by_parent` — the one master relation the runtime derives - // access from. See the helper's docblock. + // [ADR-0055 §1 · #23100 ruling B, as amended by #30905 ruling A] Exactly one + // master `master_detail` under `controlled_by_parent` (the required one, or + // the only one declared `required: false`) — the one master relation the + // runtime derives access from. See the helper's docblock. refuseControlledByParentWithoutOneMaster(object.name, object.sharingModel, object.fields, ctx); // [ADR-0055 §3 amendment · ruling 6398566164] `sharing.controlledByParent.ownerMayEdit` // is read only on a `controlled_by_parent` detail. See the helper's docblock. @@ -4425,8 +4445,17 @@ function assertReferenceViaSiblingDeclared(objectName: unknown, fields: unknown) /** * [#9138 — commit 75b7c240a, maintainer ruling Direction 2 / ADR-0055] Under * `sharingModel: 'controlled_by_parent'` the builder FORCES `required: true` - * on every `master_detail` reference, and REFUSES an explicit - * `required: false` there, loudly. + * on every `master_detail` reference that OMITS `required`, and keeps an + * explicit `required: true` as written. + * + * ⚠️ Amended by ruling A of #30905 (ADR-0055 §1 as amended): an explicit + * `required: false` on the object's ONLY `master_detail` is no longer refused + * here (one beside another still is). It declares the OPTIONAL master + * — a detail row may name none, and such a row is read by its owner and by + * View All on the detail ({@link refuseControlledByParentWithoutOneMaster} + * judges whether it is the object's one master). The text below, down to the + * second bullet, records the rule as first landed; the force itself is + * unchanged: an omitted `required` still means required. * * Why: a `controlled_by_parent` detail's access is *derived* from its master * through that reference (the `sharingModel` docblock above already states @@ -4448,7 +4477,10 @@ function assertReferenceViaSiblingDeclared(objectName: unknown, fields: unknown) * `relationship/master-detail-required` already computes exactly this fix); * - explicit `required: false` → refused with a located error — the author * wrote a contradiction, and silently flipping an explicitly authored value - * would hide it (ADR-0032 "no silent failure"). + * would hide it (ADR-0032 "no silent failure"). ⚠️ Retired by ruling A of + * #30905 on the object's ONLY `master_detail`: it is the optional master + * now, passed to the parse as written. Beside another `master_detail` it + * is still refused here, with the message naming the other. * * `readonly` / `system` are not read here: the builder forces nothing about * them, and the parse it ends in refuses a one master carrying either @@ -4481,21 +4513,30 @@ function forceCbpMasterDetailRequired( if (sharingModel !== 'controlled_by_parent') return fields; if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return fields; let out: Record | undefined; - for (const [fieldName, def] of Object.entries(fields as Record)) { - if (!def || typeof def !== 'object' || Array.isArray(def)) continue; + const masterDetails = Object.entries(fields as Record).filter( + ([, def]) => + !!def && typeof def === 'object' && !Array.isArray(def) + && (def as Record).type === 'master_detail', + ); + for (const [fieldName, def] of masterDetails) { const field = def as Record; - if (field.type !== 'master_detail') continue; if (field.required === true) continue; + // [ADR-0055 §1 as amended · #30905 ruling A] An explicit `required: false` + // on the object's ONLY `master_detail` is the optional master, kept as + // written. Beside another `master_detail` it is still the contradiction + // this builder refused before the ruling: that field is not the master + // (§1 keeps one), so it is a second parent, which §1 declares a `lookup`. if (field.required === false) { + if (masterDetails.length === 1) continue; const name = typeof objectName === 'string' && objectName.length > 0 ? objectName : ''; + const others = masterDetails.filter(([other]) => other !== fieldName).map(([other]) => `\`${other}\``); throw new Error( `ObjectSchema.create('${name}'): field \`${fieldName}\` declares \`required: false\` on a ` - + "`master_detail` reference under `sharingModel: 'controlled_by_parent'` — a contradiction " - + 'with no honest reading. A controlled-by-parent detail derives ALL of its record ' - + 'access from the master this field names (ADR-0055); a row allowed to omit the master FK ' - + 'is unreadable by everyone (the derived filter `masterFK IN (accessible master ids)` ' - + 'never matches null) and unwritable thereafter. Remove `required: false` (the builder ' - + 'declares the reference required by construction), or if this object is not a ' + + "`master_detail` reference under `sharingModel: 'controlled_by_parent'`, beside another " + + `\`master_detail\` (${others.join(', ')}). Only an object's ONLY \`master_detail\` may be ` + + 'declared `required: false`, an optional master (ADR-0055 §1): beside another it names a ' + + 'second parent, and a controlled-by-parent detail derives its access from one master. ' + + 'Declare every parent other than the master as a `lookup`, or if this object is not a ' + 'master-detail child, change its `sharingModel`.', ); } @@ -4630,11 +4671,10 @@ export const ObjectSchema = lazySchema(() => { // sibling assertions, rather than one error per seeded row at load time. assertReferenceViaSiblingDeclared(cfg.name, cfg.fields); // [#9138 — commit 75b7c240a, ruling Direction 2] A `controlled_by_parent` object's - // `master_detail` reference is forced `required: true` (an explicit - // `required: false` throws, loudly) so the unsafe shape cannot be newly - // declared. Raw `.parse()`/`.safeParse()` do not force it — they refuse a - // `controlled_by_parent` object without exactly one required - // `master_detail` instead (#23100 ruling B) — see the function's docblock. + // `master_detail` reference that omits `required` is forced `required: true`; + // an explicit `required: false` on its only `master_detail` is kept, the + // optional master (ruling A of #30905), and refused beside another. Raw `.parse()`/`.safeParse()` do not force it — they judge the + // object's one master instead (#23100 ruling B) — see the function's docblock. const forcedFields = forceCbpMasterDetailRequired(cfg.name, cfg.sharingModel, cfg.fields); const withDefaults = { ...cfg, diff --git a/packages/spec/src/data/readable-ids-conformance.ts b/packages/spec/src/data/readable-ids-conformance.ts index 96e01dc306e..f9ba111371e 100644 --- a/packages/spec/src/data/readable-ids-conformance.ts +++ b/packages/spec/src/data/readable-ids-conformance.ts @@ -270,6 +270,35 @@ export const READABLE_IDS_CASES: readonly ReadableIdsCase[] = [ + 'the parents u1 owns, so the rows are the narrowing case\'s. A driver that resolves only the outer leaf ' + 'hands the inner one to its compiler unresolved and refuses it, or evaluates it as no restriction.', }, + { + name: 'an optional master\'s read: a leaf whose scope ORs a nested leaf, OR-ed with an empty-reference arm', + where: () => ({ + $or: [ + { + record_id: { + $in: composed({ + $or: [ + { id: { $in: composed({ owner: 'u2' }) } }, + { $and: [{ owner: 'u1' }, { id: 'p3' }] }, + ], + }), + }, + }, + { $and: [{ record_id: null }, { label: 'keep' }] }, + ], + }), + verdict: 'matches', + holdsComposedLeaf: true, + expected: ['c2', 'c3', 'c5'], + note: 'ADR-0055 §2 as amended (an optional master): a detail is read through ' + + '`fk IN (readable masters) OR (fk IS NULL AND owner_id = caller)`, and an intermediate level of a chain ' + + 'whose own master is optional answers its scope with the same `$or`, so the nested leaf sits in one arm ' + + 'of an `$or` inside a composed leaf\'s scope. Here `label` stands in for the owner column and the inner ' + + '`$and` for an intermediate row read by its owner: the parents are p2 (the nested leaf) and p3, so the ' + + 'children are c2 and c3, plus c5, which names no parent and passes the empty-reference arm. A driver ' + + 'that evaluates the nested leaf as no restriction returns c1, c6 and c7 too; one that drops the ' + + 'empty-reference arm loses c5.', + }, { name: 'an uncomposed leaf is refused, never evaluated as no restriction', where: () => ({ record_id: { $in: ReadableIds.of(P) } }), diff --git a/packages/spec/src/migrations/entries/semantic/18.cbp-optional-master-admitted.ts b/packages/spec/src/migrations/entries/semantic/18.cbp-optional-master-admitted.ts new file mode 100644 index 00000000000..749282bf3b9 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.cbp-optional-master-admitted.ts @@ -0,0 +1,51 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// ADR-0055 §1 and §2 as amended (maintainer ruling A on the optional master): +// a controlled_by_parent object whose only master_detail is declared +// `required: false` derives an OPTIONAL master. A widening beside +// cbp-master-relation-tiers-retired, which refused the shape: an explicit +// `required: false` is accepted at every door, the builder keeps it as written, +// and a stored row in it stops being listed and denied. A detail row that names +// no master is read by its owner and by View All on the detail; a row that +// names one is read through its master, as before. An OMITTED `required` keeps +// its meaning where the spec sees the omission (the builder force and the D2 +// conversion cbp-master-detail-required-defaulted, both unchanged). No D2 +// conversion: nothing is rewritten. +export const entry: SemanticMigration = { + id: 'cbp-optional-master-admitted', + surface: 'object.sharingModel = \'controlled_by_parent\' together with object.fields — a `master_detail` ' + + 'field declared `required: false` as the object\'s one master', + replacement: 'Nothing to replace: a `controlled_by_parent` object may now declare its only `master_detail` ' + + 'field with `required: false`, an optional master. A detail row that names its master is read exactly as ' + + 'before, by whoever can read that master. A row whose master reference is empty is read by its owner ' + + '(`owner_id`) and by a holder of View All on the detail, and by nobody else: a row with no owner, or on ' + + 'an object that carries no `owner_id` column, is read through View All alone. Writes are unchanged by ' + + 'this entry: an insert or update that leaves the master reference empty is still refused with 422. ' + + 'Declare `required: true` when every row must name its master; an omitted `required` keeps meaning ' + + 'required through `ObjectSchema.create()` and on stored rows, and `os lint` reports it at `error` so the ' + + 'author writes one of the two.', + reason: + 'The rule "a rep sees the contacts of the accounts they own; a contact need not have an account" had no ' + + 'declarative form: `controlled_by_parent` refused a `master_detail` that is not required at the builder ' + + 'and at the parse, a lookup derives no access, and the only route left was a copied owner field kept in ' + + 'step by hooks. The maintainer ruling (letter A) admits an optional master and gives a row without one ' + + 'to its owner, the Salesforce reading of a private contact. ADR-0055 §1 and §2 are amended in the same ' + + 'change. The read is one more arm inside the derived controlled-by-parent filter, OR-ed with the master ' + + 'arm and ANDed under the tenant wall like it, so every read door that composes that filter (the data ' + + 'middleware, the analytics read filter, the parent gates of attachments, comments, activity and ' + + 'approvals, delegated reads) follows it. A row that names a master is never read through the owner ' + + 'arm, and a row that names none never through a master. For an explicit `required: false` this entry ' + + 'supersedes what `cbp-master-relation-tiers-retired` and `cbp-master-detail-required-forced` say of it ' + + '(refused at the builder and at the parse, listed and denied at rest); everything else they say stands.', + acceptanceCriteria: + 'An object declaring `sharingModel: \'controlled_by_parent\'` whose only `master_detail` field is ' + + 'declared `required: false` passes `ObjectSchema.safeParse` and `ObjectSchema.create()` with that value ' + + 'kept, and `os migrate meta --stored` no longer lists a stored row in that shape under ' + + '`cbp-master-relation-tiers-retired`. At runtime the security plugin resolves its master relation (no ' + + '"resolves no master relation" line), a member who can read a master reads that master\'s rows, the ' + + 'owner of a row with an empty master reads that row, a holder of View All on the object reads every ' + + 'row with an empty master, and nobody else reads one. An object whose `master_detail` omits `required` ' + + 'is still forced to `required: true` by the builder and converted to it on stored rows.', +}; diff --git a/packages/spec/src/migrations/registry.ts.template b/packages/spec/src/migrations/registry.ts.template index 5600bdd93ca..74699288e8b 100644 --- a/packages/spec/src/migrations/registry.ts.template +++ b/packages/spec/src/migrations/registry.ts.template @@ -1318,6 +1318,18 @@ const STEP18_RATIONALE: readonly RationaleFragment[] = [ + '`required: false` or a second master. Its D3 record is the semantic entry ' + '`cbp-master-relation-tiers-retired`.', }, + { + id: 'cbp-optional-master-admitted', + order: 119, + text: + 'It then admits an optional master under `controlled_by_parent` (ADR-0055 §1 and §2 as amended, ' + + 'maintainer ruling A): an object whose only `master_detail` is declared `required: false` derives its ' + + 'master from it, and a detail row that names no master is read by its owner and by View All on the ' + + 'detail, never through a master, while a row that names one is read through it as before. The builder ' + + 'keeps an explicit `required: false` as written and still forces an omitted one to `true`, and the ' + + 'stored-row listing stops naming the shape. Writes are unchanged: an empty master reference is still ' + + 'refused with 422. Its D3 record is the semantic entry `cbp-optional-master-admitted`.', + }, { id: 'chart-config-aria-retired', order: 31, diff --git a/packages/spec/src/shared/retired-key-removal-version.pin.test.ts b/packages/spec/src/shared/retired-key-removal-version.pin.test.ts index e457e363b35..c6b69a906fb 100644 --- a/packages/spec/src/shared/retired-key-removal-version.pin.test.ts +++ b/packages/spec/src/shared/retired-key-removal-version.pin.test.ts @@ -341,14 +341,24 @@ const JOIN: Readonly> = { refusal("action:group member's patch", () => ActionGroupPropsSchema.safeParse({ actions: [{ label: 'Close', patch: { status: 'done' } }] }), 'actions'), refusal("action:menu member's patch", () => ActionMenuPropsSchema.safeParse({ actions: [{ label: 'Close', patch: { status: 'done' } }] }), 'actions'), ], + // [ADR-0055 §1 as amended · #30905 ruling A] The parse reads an omitted + // `required` on an object's ONLY `master_detail` as `false` (the parsed field + // map defaults it), so that shape is an optional master at the parse and no + // longer refused there; the builder still forces it, this conversion still + // converts it at rest, and `os lint` reports it. The refusal the parse still + // answers on this surface is the shape the conversion leaves as stored: two + // `master_detail` fields, neither declaring `required`. 'cbp-master-detail-required-defaulted': [ refusal( - 'a controlled_by_parent master_detail without `required`', + 'a controlled_by_parent object whose two master_detail fields declare no `required`', () => ObjectSchema.safeParse({ name: 'cbp_line', label: 'Line', sharingModel: 'controlled_by_parent', - fields: { order: { type: 'master_detail', label: 'Order', reference: 'cbp_order' } }, + fields: { + order: { type: 'master_detail', label: 'Order', reference: 'cbp_order' }, + batch: { type: 'master_detail', label: 'Batch', reference: 'cbp_batch' }, + }, }), 'sharingModel', ), diff --git a/packages/spec/src/stack-cbp-extension-master.test.ts b/packages/spec/src/stack-cbp-extension-master.test.ts index 096c56c02c8..79f6bd9e9a6 100644 --- a/packages/spec/src/stack-cbp-extension-master.test.ts +++ b/packages/spec/src/stack-cbp-extension-master.test.ts @@ -110,9 +110,23 @@ describe('[#23210] defineStack judges a controlled_by_parent object on its merge expect(cbpIssues(withField(OTHER)).map(([p]) => p)).toEqual(['objectExtensions.0.fields']); }); + it('an extension that makes the one master optional passes, as its declared control does; beside a second master it is refused', () => { + // [ADR-0055 §1 as amended · #30905 ruling A] The object's only `master_detail` + // declared `required: false` is an optional master — accepted on the merged + // field set exactly as on the object declared directly. + const optional = { type: 'master_detail', reference: 'cbpx_header', required: false }; + const stack = { manifest: MANIFEST, objects: OBJECTS(), objectExtensions: [{ extend: 'cbpx_line', fields: { header: optional } }] }; + expect(cbpIssues(stack)).toEqual([]); + expect(ObjectStackDefinitionSchema.safeParse(stack).success).toBe(true); + expect(cbpIssues({ manifest: MANIFEST, objects: OBJECTS({ header: optional }) })).toEqual([]); + // The twin: the same optional master beside a second `master_detail` the + // extension adds is no one master, and is refused at the extension. + const twin = { manifest: MANIFEST, objects: OBJECTS({ header: optional }), objectExtensions: [{ extend: 'cbpx_line', fields: { other: { ...OTHER, required: false } } }] }; + expect(cbpIssues(twin).map(([p]) => p)).toEqual(['objectExtensions.0.fields']); + }); + it('an extension that demotes the one master is refused, in the words of the declared control', () => { for (const demoted of [ - { type: 'master_detail', reference: 'cbpx_header', required: false }, { type: 'text' }, { type: 'lookup', reference: 'cbpx_header', required: true }, ]) { diff --git a/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin-cbp-read-filter.ts.json b/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin-cbp-read-filter.ts.json index 35fb1ed30fe..d7dbfdb50dc 100644 --- a/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin-cbp-read-filter.ts.json +++ b/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin-cbp-read-filter.ts.json @@ -3,5 +3,5 @@ "adrs": [ "ADR-0055" ], - "invariant": "ADR-0055 §1 — `resolveCbpRelation` derives a `controlled_by_parent` object's master from exactly one `master_detail` field declared `required: true` that names a reference, and from nothing else: zero such fields or two or more resolve `null`, which both halves treat as fail-closed (reads answer RLS_DENY_FILTER, writes throw MasterDetailRelationMissingError, 422 INVALID_METADATA), reported once per object per cache generation. Do NOT restore the retired fallback tiers (any `master_detail`, then a required `lookup`) or a first-by-field-order pick among two: they were never part of ADR-0055 §1, `ObjectSchema` refuses those shapes at parse, and a gate that derives access from a relation the parse refuses is the disagreement the step-18 semantic migration `cbp-master-relation-tiers-retired` closed. The reference-SPELLING tolerance in the same reader is a different, deliberate decision and stays." + "invariant": "ADR-0055 §1 (as amended 2026-11-06, #30905, ruling record 6687830530) — `resolveCbpRelation` derives a `controlled_by_parent` object's master from exactly one `master_detail` field declared `required: true` that names a reference, or, with none, from the object's ONLY `master_detail`, declared `required: false` and naming a reference: an OPTIONAL master (`CbpRelation.optional`), picked by the spec's own `controlledByParentMasterOf` so the parse and this reader cannot name different fields. A detail row that names its master is read through it, and a required master's read leaf is byte-identical to before; a row whose master reference is empty is read through the null-master arm of `computeControlledByParentFilter` alone, by the row's owner (`owner_id`, only where the detail carries that column) and by View All on the detail; the write gate still refuses a write that leaves the reference empty. Anything else resolves `null`, which both halves treat as fail-closed (reads answer RLS_DENY_FILTER, writes throw MasterDetailRelationMissingError, 422 INVALID_METADATA), reported once per object per cache generation: no `master_detail`, two or more required ones, two or more with none required, and one whose `required` is not written at all (this reader tests `required === false`, never its absence, so an object that reached the registry without the parse resolves no master). Do NOT restore the retired fallback tiers (any `master_detail`, then a required `lookup`) or a first-by-field-order pick among two: they were never part of ADR-0055 §1, `ObjectSchema` refuses those shapes at parse, and a gate that derives access from a relation the parse refuses is the disagreement the step-18 semantic migration `cbp-master-relation-tiers-retired` closed. Do NOT widen the null-master arm past rows whose master reference is empty, or let View All on the detail open the master arm. The reference-SPELLING tolerance in the same reader is a different, deliberate decision and stays." } diff --git a/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin-cbp-relation.ts.json b/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin-cbp-relation.ts.json index 01d6f32c2e1..79294d888d6 100644 --- a/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin-cbp-relation.ts.json +++ b/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin-cbp-relation.ts.json @@ -3,5 +3,5 @@ "adrs": [ "ADR-0055" ], - "invariant": "ADR-0055 §1 — `resolveCbpRelation` derives a `controlled_by_parent` object's master from exactly one `master_detail` field declared `required: true` that names a reference, and from nothing else: zero such fields or two or more resolve `null`, which both halves treat as fail-closed (reads answer RLS_DENY_FILTER, writes throw MasterDetailRelationMissingError, 422 INVALID_METADATA), reported once per object per cache generation. Do NOT restore the retired fallback tiers (any `master_detail`, then a required `lookup`) or a first-by-field-order pick among two: they were never part of ADR-0055 §1, `ObjectSchema` refuses those shapes at parse, and a gate that derives access from a relation the parse refuses is the disagreement the step-18 semantic migration `cbp-master-relation-tiers-retired` closed. The reference-SPELLING tolerance in the same reader is a different, deliberate decision and stays." + "invariant": "ADR-0055 §1 (as amended 2026-11-06, #30905, ruling record 6687830530) — `resolveCbpRelation` derives a `controlled_by_parent` object's master from exactly one `master_detail` field declared `required: true` that names a reference, or, with none, from the object's ONLY `master_detail`, declared `required: false` and naming a reference: an OPTIONAL master (`CbpRelation.optional`), picked by the spec's own `controlledByParentMasterOf` so the parse and this reader cannot name different fields. A detail row that names its master is read through it, and a required master's read leaf is byte-identical to before; a row whose master reference is empty is read through the null-master arm of `computeControlledByParentFilter` alone, by the row's owner (`owner_id`, only where the detail carries that column) and by View All on the detail; the write gate still refuses a write that leaves the reference empty. Anything else resolves `null`, which both halves treat as fail-closed (reads answer RLS_DENY_FILTER, writes throw MasterDetailRelationMissingError, 422 INVALID_METADATA), reported once per object per cache generation: no `master_detail`, two or more required ones, two or more with none required, and one whose `required` is not written at all (this reader tests `required === false`, never its absence, so an object that reached the registry without the parse resolves no master). Do NOT restore the retired fallback tiers (any `master_detail`, then a required `lookup`) or a first-by-field-order pick among two: they were never part of ADR-0055 §1, `ObjectSchema` refuses those shapes at parse, and a gate that derives access from a relation the parse refuses is the disagreement the step-18 semantic migration `cbp-master-relation-tiers-retired` closed. Do NOT widen the null-master arm past rows whose master reference is empty, or let View All on the detail open the master arm. The reference-SPELLING tolerance in the same reader is a different, deliberate decision and stays." } diff --git a/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin-cbp-write-gate.ts.json b/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin-cbp-write-gate.ts.json index 6e3ccf15253..40967ce54c7 100644 --- a/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin-cbp-write-gate.ts.json +++ b/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin-cbp-write-gate.ts.json @@ -3,5 +3,5 @@ "adrs": [ "ADR-0055" ], - "invariant": "ADR-0055 §1 — `resolveCbpRelation` derives a `controlled_by_parent` object's master from exactly one `master_detail` field declared `required: true` that names a reference, and from nothing else: zero such fields or two or more resolve `null`, which both halves treat as fail-closed (reads answer RLS_DENY_FILTER, writes throw MasterDetailRelationMissingError, 422 INVALID_METADATA), reported once per object per cache generation. Do NOT restore the retired fallback tiers (any `master_detail`, then a required `lookup`) or a first-by-field-order pick among two: they were never part of ADR-0055 §1, `ObjectSchema` refuses those shapes at parse, and a gate that derives access from a relation the parse refuses is the disagreement the step-18 semantic migration `cbp-master-relation-tiers-retired` closed. The reference-SPELLING tolerance in the same reader is a different, deliberate decision and stays." + "invariant": "ADR-0055 §1 (as amended 2026-11-06, #30905, ruling record 6687830530) — `resolveCbpRelation` derives a `controlled_by_parent` object's master from exactly one `master_detail` field declared `required: true` that names a reference, or, with none, from the object's ONLY `master_detail`, declared `required: false` and naming a reference: an OPTIONAL master (`CbpRelation.optional`), picked by the spec's own `controlledByParentMasterOf` so the parse and this reader cannot name different fields. A detail row that names its master is read through it, and a required master's read leaf is byte-identical to before; a row whose master reference is empty is read through the null-master arm of `computeControlledByParentFilter` alone, by the row's owner (`owner_id`, only where the detail carries that column) and by View All on the detail; the write gate still refuses a write that leaves the reference empty. Anything else resolves `null`, which both halves treat as fail-closed (reads answer RLS_DENY_FILTER, writes throw MasterDetailRelationMissingError, 422 INVALID_METADATA), reported once per object per cache generation: no `master_detail`, two or more required ones, two or more with none required, and one whose `required` is not written at all (this reader tests `required === false`, never its absence, so an object that reached the registry without the parse resolves no master). Do NOT restore the retired fallback tiers (any `master_detail`, then a required `lookup`) or a first-by-field-order pick among two: they were never part of ADR-0055 §1, `ObjectSchema` refuses those shapes at parse, and a gate that derives access from a relation the parse refuses is the disagreement the step-18 semantic migration `cbp-master-relation-tiers-retired` closed. Do NOT widen the null-master arm past rows whose master reference is empty, or let View All on the detail open the master arm. The reference-SPELLING tolerance in the same reader is a different, deliberate decision and stays." } diff --git a/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin.ts.json b/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin.ts.json index 75565e3c22e..4f5a05b4a03 100644 --- a/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin.ts.json +++ b/scripts/adr-anchors/packages__plugins__plugin-security__src__security-plugin.ts.json @@ -6,5 +6,5 @@ "ADR-0123", "ADR-0138" ], - "invariant": "ADR-0055 §1 — `resolveCbpRelation` derives a `controlled_by_parent` object's master from exactly one `master_detail` field declared `required: true` that names a reference, and from nothing else: zero such fields or two or more resolve `null`, which both halves treat as fail-closed (reads answer RLS_DENY_FILTER, writes throw MasterDetailRelationMissingError, 422 INVALID_METADATA), reported once per object per cache generation. Do NOT restore the retired fallback tiers (any `master_detail`, then a required `lookup`) or a first-by-field-order pick among two: they were never part of ADR-0055 §1, `ObjectSchema` refuses those shapes at parse, and a gate that derives access from a relation the parse refuses is the disagreement the step-18 semantic migration `cbp-master-relation-tiers-retired` closed. The reference-SPELLING tolerance in the same reader is a different, deliberate decision and stays.\n\nADR-0106 D7 — getMetadataReadableFields differs from getReadableFields in exactly one place, and the asymmetry is the decision. On the DATA plane a caller resolving to zero permission sets falls OPEN, mirroring the engine middleware, because reporting a narrowing the data path would not enforce is its own drift. On the METADATA plane the same caller resolves the configured fallback permission set (the two-step /auth/me/permissions performs), so a guest-facing deployment's schema exposure is a deliberate permission-set decision rather than an accidental everything-default. Converging the two methods in either direction reverses this.\n\nADR-0123 D2 — an authenticated session with NO active organization is a legal state with fail-closed semantics, and the two sides are deliberately ASYMMETRIC: tenant-scoped reads resolve to nothing (Layer 0's deny sentinel, silent, HTTP 200) while tenant-scoped insert/update are REFUSED with a 403 whose message names the missing active organization. Step 3.7's no-active-organization refusal is that write half. Deleting it, or 'simplifying' it into the sibling forge guard, restores the state the ruling closed: the forge guard validates SUPPLIED organization_id values only, so a payload naming no tenant written by a caller having no tenant walks past it, lands with organization_id NULL, and is then hidden by the read wall from every reader including its own author. `delete` is absent on purpose (it places no row; the Layer 0 row wall already resolves its target to nothing), and the refusal must stay ordered AHEAD of the forge guard so a caller with no organization scope is told that, not that the value they supplied is foreign.\n\nADR-0138 D1 — for the guest envelope the owner stays unset, and only the scenario's own writers (a hook, a flow, an assignment rule) set it; so the seed-ownership claim this file runs on every `app:seeded` (`claimSeedOwnershipOnSettle`) and at the first promotion re-owns a NULL-owner row only when the seed reported landing on it (`readSeededRecordIds`, the `seed-settlement` contract's `seededRecordIds`), and widening that arm back to every unowned row hands guest-written and public-form rows to the platform admin on the next boot." + "invariant": "ADR-0055 §1 (as amended 2026-11-06, #30905, ruling record 6687830530) — `resolveCbpRelation` derives a `controlled_by_parent` object's master from exactly one `master_detail` field declared `required: true` that names a reference, or, with none, from the object's ONLY `master_detail`, declared `required: false` and naming a reference: an OPTIONAL master (`CbpRelation.optional`), picked by the spec's own `controlledByParentMasterOf` so the parse and this reader cannot name different fields. A detail row that names its master is read through it, and a required master's read leaf is byte-identical to before; a row whose master reference is empty is read through the null-master arm of `computeControlledByParentFilter` alone, by the row's owner (`owner_id`, only where the detail carries that column) and by View All on the detail; the write gate still refuses a write that leaves the reference empty. Anything else resolves `null`, which both halves treat as fail-closed (reads answer RLS_DENY_FILTER, writes throw MasterDetailRelationMissingError, 422 INVALID_METADATA), reported once per object per cache generation: no `master_detail`, two or more required ones, two or more with none required, and one whose `required` is not written at all (this reader tests `required === false`, never its absence, so an object that reached the registry without the parse resolves no master). Do NOT restore the retired fallback tiers (any `master_detail`, then a required `lookup`) or a first-by-field-order pick among two: they were never part of ADR-0055 §1, `ObjectSchema` refuses those shapes at parse, and a gate that derives access from a relation the parse refuses is the disagreement the step-18 semantic migration `cbp-master-relation-tiers-retired` closed. Do NOT widen the null-master arm past rows whose master reference is empty, or let View All on the detail open the master arm. The reference-SPELLING tolerance in the same reader is a different, deliberate decision and stays.\n\nADR-0106 D7 — getMetadataReadableFields differs from getReadableFields in exactly one place, and the asymmetry is the decision. On the DATA plane a caller resolving to zero permission sets falls OPEN, mirroring the engine middleware, because reporting a narrowing the data path would not enforce is its own drift. On the METADATA plane the same caller resolves the configured fallback permission set (the two-step /auth/me/permissions performs), so a guest-facing deployment's schema exposure is a deliberate permission-set decision rather than an accidental everything-default. Converging the two methods in either direction reverses this.\n\nADR-0123 D2 — an authenticated session with NO active organization is a legal state with fail-closed semantics, and the two sides are deliberately ASYMMETRIC: tenant-scoped reads resolve to nothing (Layer 0's deny sentinel, silent, HTTP 200) while tenant-scoped insert/update are REFUSED with a 403 whose message names the missing active organization. Step 3.7's no-active-organization refusal is that write half. Deleting it, or 'simplifying' it into the sibling forge guard, restores the state the ruling closed: the forge guard validates SUPPLIED organization_id values only, so a payload naming no tenant written by a caller having no tenant walks past it, lands with organization_id NULL, and is then hidden by the read wall from every reader including its own author. `delete` is absent on purpose (it places no row; the Layer 0 row wall already resolves its target to nothing), and the refusal must stay ordered AHEAD of the forge guard so a caller with no organization scope is told that, not that the value they supplied is foreign.\n\nADR-0138 D1 — for the guest envelope the owner stays unset, and only the scenario's own writers (a hook, a flow, an assignment rule) set it; so the seed-ownership claim this file runs on every `app:seeded` (`claimSeedOwnershipOnSettle`) and at the first promotion re-owns a NULL-owner row only when the seed reported landing on it (`readSeededRecordIds`, the `seed-settlement` contract's `seededRecordIds`), and widening that arm back to every unowned row hands guest-written and public-form rows to the platform admin on the next boot." } diff --git a/scripts/adr-anchors/packages__spec__src__data__object.zod.ts.json b/scripts/adr-anchors/packages__spec__src__data__object.zod.ts.json index df3a912578a..930b2b3e71b 100644 --- a/scripts/adr-anchors/packages__spec__src__data__object.zod.ts.json +++ b/scripts/adr-anchors/packages__spec__src__data__object.zod.ts.json @@ -5,5 +5,5 @@ "ADR-0120", "ADR-0144" ], - "invariant": "ADR-0055 §1 — a `controlled_by_parent` object derives its master from EXACTLY ONE `master_detail` field declared `required: true`, and the parse refuses anything else (`refuseControlledByParentWithoutOneMaster`, in `ObjectSchemaBase`'s superRefine): no such field (a non-required `master_detail`, a required `lookup`, nothing) at `sharingModel`, two or more at `fields`. It runs on the PARSE, not only in `ObjectSchema.create()`, because the shapes it refuses were reachable through every door that skips the builder and the security runtime derives no master from them — so an object the parse accepted would be denied every read and refused every write. Do NOT move it back to `create()` only, and do NOT accept a second tier here (any `master_detail`, a required `lookup`): the runtime's two fallback tiers were retired under ADR-0049 with the step-18 semantic migration `cbp-master-relation-tiers-retired`, and a parse wider than the runtime is the three-seam disagreement that retirement closed.\n\n`IndexSchema.unique` scope contract (ADR-0120 D1, amending #3696): `'global'` = verbatim columns (no organization column injected); `'organization'` = the driver prepends the NULL-safe organization key part (COALESCE(organization_id, '__global__')) at registration; bare `true` = REFUSED since protocol 18 (#5082, ADR-0120 D7) with a prescription naming both words — it was the positional spelling of `'global'`; stored and built metadata converts it to `'global'` through the ADR-0087 conversion `declared-index-unique-scope` (byte-identical physical index), retired from the authoring funnel so a live author is refused, never converted silently. Field-level `unique: true` stays valid (D1) — never narrow `UniqueScopeSchema` in step with this one. Do NOT re-broaden the describe() back to 'true and global are synonyms, list the tenant column yourself' — that is the #4986 trap, and the hand-written composite is not NULL-safe (#5030).\n\nADR-0144 D2 — a `record`-mode amount's `currencyConfig.currencyField` must name ANOTHER declared field of the same object whose type may carry a value domain (`VALUE_DOMAIN_FIELD_TYPES`, `text` today) and which declares `valueDomain: 'iso_4217_currency'` (`refuseDishonestRecordCurrencyField`, in `ObjectSchemaBase`'s superRefine beside `refuseNonPictureImageField`). It runs on the parse because only the object holds the sibling field map, and it covers a `formula` with `returnType: 'currency'` in the same shape. Do NOT accept a pointer into nothing, at a non-code field, or at a text field without the domain: each lets a write store an amount whose currency is any string or none. It is deliberately not run on `ObjectExtensionSchema`, whose pointer may name a field only the owner declares.\n\nADR-0144 D5 — a `record`-mode amount's `currencyConfig.homeAmount` is judged on the parse (`refuseDishonestHomeAmount`, beside `refuseDishonestRecordCurrencyField`): the amount itself is a stored `currency` field (never a `formula`); `field` names ANOTHER declared `currency` field in `dynamic` mode (D8 rule 5: metadata never names the home currency) that no other amount already stores into; `rateDate` a declared `date` / `datetime` field; `rateField` a declared `number` field, shared only by amounts with the same `currencyField` and `rateDate`. The home and rate fields are platform-written and say so with the EXISTING mark, `readonly: true`, and carry no `expression`. Do NOT invent a new platform-computed key for them, and do NOT relax `readonly`: it is what drops a client's write of an engine-stored value." + "invariant": "ADR-0055 §1 (as amended 2026-11-06, #30905, ruling record 6687830530) — a `controlled_by_parent` object derives its master from exactly one `master_detail` field declared `required: true`, or, with none, from its ONLY `master_detail` declared `required: false`: an optional master, whose rows may name none and are then read by the row's owner and by View All on the detail (`controlledByParentMasterOf` in `controlled-by-parent-master.ts` answers which field is the master, for this parse, the lint and the security runtime alike). The parse refuses anything else (`refuseControlledByParentWithoutOneMaster`, in `ObjectSchemaBase`'s superRefine): no master (nothing, a required `lookup` only, two or more `master_detail` fields with none required) at `sharingModel`; two or more required ones, or a `readonly` / `system` master (required or optional), at `fields`. The parse judges `required` after its default, so an omitted `required` on the only `master_detail` parses as the optional master — option A of that ruling; the builder still forces the omission to `true`, the stored-row conversion `cbp-master-detail-required-defaulted` converts it to `true`, and `os lint` reports it at `error`. It runs on the PARSE, not only in `ObjectSchema.create()`, because the shapes it refuses were reachable through every door that skips the builder and the security runtime derives no master from them — so an object the parse accepted would be denied every read and refused every write. Do NOT move it back to `create()` only, and do NOT accept a retired tier here (a `master_detail` that is not the one master by the rule above, a required `lookup`, a first-by-field-order pick among two): the runtime's two fallback tiers were retired under ADR-0049 with the step-18 semantic migration `cbp-master-relation-tiers-retired`, and a parse wider than the runtime is the three-seam disagreement that retirement closed.\n\n`IndexSchema.unique` scope contract (ADR-0120 D1, amending #3696): `'global'` = verbatim columns (no organization column injected); `'organization'` = the driver prepends the NULL-safe organization key part (COALESCE(organization_id, '__global__')) at registration; bare `true` = REFUSED since protocol 18 (#5082, ADR-0120 D7) with a prescription naming both words — it was the positional spelling of `'global'`; stored and built metadata converts it to `'global'` through the ADR-0087 conversion `declared-index-unique-scope` (byte-identical physical index), retired from the authoring funnel so a live author is refused, never converted silently. Field-level `unique: true` stays valid (D1) — never narrow `UniqueScopeSchema` in step with this one. Do NOT re-broaden the describe() back to 'true and global are synonyms, list the tenant column yourself' — that is the #4986 trap, and the hand-written composite is not NULL-safe (#5030).\n\nADR-0144 D2 — a `record`-mode amount's `currencyConfig.currencyField` must name ANOTHER declared field of the same object whose type may carry a value domain (`VALUE_DOMAIN_FIELD_TYPES`, `text` today) and which declares `valueDomain: 'iso_4217_currency'` (`refuseDishonestRecordCurrencyField`, in `ObjectSchemaBase`'s superRefine beside `refuseNonPictureImageField`). It runs on the parse because only the object holds the sibling field map, and it covers a `formula` with `returnType: 'currency'` in the same shape. Do NOT accept a pointer into nothing, at a non-code field, or at a text field without the domain: each lets a write store an amount whose currency is any string or none. It is deliberately not run on `ObjectExtensionSchema`, whose pointer may name a field only the owner declares.\n\nADR-0144 D5 — a `record`-mode amount's `currencyConfig.homeAmount` is judged on the parse (`refuseDishonestHomeAmount`, beside `refuseDishonestRecordCurrencyField`): the amount itself is a stored `currency` field (never a `formula`); `field` names ANOTHER declared `currency` field in `dynamic` mode (D8 rule 5: metadata never names the home currency) that no other amount already stores into; `rateDate` a declared `date` / `datetime` field; `rateField` a declared `number` field, shared only by amounts with the same `currencyField` and `rateDate`. The home and rate fields are platform-written and say so with the EXISTING mark, `readonly: true`, and carry no `expression`. Do NOT invent a new platform-computed key for them, and do NOT relax `readonly`: it is what drops a client's write of an engine-stored value." } diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 7a7fbd3e301..a1e73e4ed05 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -3981,6 +3981,11 @@ "verb": "findOne", "pinned": 1 }, + { + "file": "packages/plugins/plugin-security/src/controlled-by-parent-optional-master.test.ts", + "verb": "findOne", + "pinned": 1 + }, { "file": "packages/plugins/plugin-security/src/controlled-by-parent-sharing.test.ts", "verb": "findOne", diff --git a/ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.cbpRequiredMaster-12326.test.tsx b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.cbpRequiredMaster-12326.test.tsx index 05baffb80fc..df1498f05ad 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.cbpRequiredMaster-12326.test.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.cbpRequiredMaster-12326.test.tsx @@ -7,8 +7,12 @@ * * Since `@objectstack/spec` 18 (objectstack#23100, ADR-0055 §1) such an object * derives its master from exactly one `master_detail` declared - * `required: true`; the save door refuses any other value. The installed - * spec's `ObjectSchema` is the oracle for the object each edit leaves behind. + * `required: true`. Since ADR-0055 §1 as amended (objectstack#30905 ruling A) + * the spec also reads an object's only `master_detail` that is not required as + * an OPTIONAL master, whose rows may name none; the designer keeps writing + * `required: true`, so the master it shows is one every row names. The + * installed spec's `ObjectSchema` is the oracle for the object each edit + * leaves behind. */ import '@testing-library/jest-dom/vitest'; @@ -18,6 +22,12 @@ import { cleanup, fireEvent, render, screen, within } from '@testing-library/rea import userEvent from '@testing-library/user-event'; import { ObjectSchema } from '@objectstack/spec/data'; import { t } from '../i18n'; +// The inspector's object picker and CEL authoring load `@objectstack/lint` and +// `@objectstack/formula` with function-body `import()`s. Warmed here so their +// first load is paid at collection, not inside a case's bounded wait. Same +// specifiers, so the source's own dynamic imports resolve from the cache. +import '@objectstack/lint'; +import '@objectstack/formula'; type Row = Record; @@ -104,13 +114,19 @@ describe('ObjectFieldInspector — a controlled_by_parent master is required, lo }, 'account', ); - expect(ObjectSchema.safeParse(object()).success).toBe(false); + // The control leg: as stored, the spec reads this master as optional — + // it parses, as `required: false`. + const before = ObjectSchema.safeParse(object()); + expect(before.success).toBe(true); + expect((before.data?.fields as Record).account.required).toBe(false); expect(requiredBox()).not.toBeChecked(); expect(requiredBox()).toBeEnabled(); fireEvent.click(requiredBox()); expect(field().required).toBe(true); expect(requiredBox()).toBeDisabled(); - expect(ObjectSchema.safeParse(object()).success).toBe(true); + const after = ObjectSchema.safeParse(object()); + expect(after.success).toBe(true); + expect((after.data?.fields as Record).account.required).toBe(true); }); it('outside controlled_by_parent the box stays free and no reason is shown', () => { diff --git a/ui/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.test.ts b/ui/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.test.ts index 5d923a10cd6..5252519d131 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.test.ts +++ b/ui/packages/app-shell/src/views/metadata-admin/previews/object-fields-io.test.ts @@ -392,8 +392,11 @@ describe('toFieldNameLoose (prefix-stable — for live keystroke input)', () => /** * objectui#12326 — a master-detail field starts required. Added to a * `controlled_by_parent` object, it is the one required `master_detail` the - * installed spec derives the master from (ADR-0055 §1); without `required` the - * same object is refused. The spec's `ObjectSchema` is the oracle. + * installed spec derives the master from (ADR-0055 §1). Without `required` the + * spec reads that field as an OPTIONAL master (ADR-0055 §1 as amended, + * objectstack#30905 ruling A: a row may name none), so the `required: true` + * this writes is what keeps every row naming its master. The spec's + * `ObjectSchema` is the oracle. */ describe('newField — a master-detail field starts required (objectui#12326)', () => { const cbpObject = (def: Record) => ({ @@ -406,9 +409,12 @@ describe('newField — a master-detail field starts required (objectui#12326)', const { def } = newField('account', 'master_detail'); expect(def.required).toBe(true); expect(ObjectSchema.safeParse(cbpObject(def)).success).toBe(true); - // The control leg: the same field without `required` is refused. + // The control leg: the same field without `required` is read as an + // optional master — it parses, as `required: false`. const { required: _omitted, ...withoutRequired } = def; - expect(ObjectSchema.safeParse(cbpObject(withoutRequired)).success).toBe(false); + const control = ObjectSchema.safeParse(cbpObject(withoutRequired)); + expect(control.success).toBe(true); + expect((control.data?.fields as Record).account.required).toBe(false); }); it('leaves every other type without required', () => { diff --git a/ui/packages/app-shell/src/views/studio-design/ObjectSettingsPanel.cbpRequiredMaster-12326.test.tsx b/ui/packages/app-shell/src/views/studio-design/ObjectSettingsPanel.cbpRequiredMaster-12326.test.tsx index 3f04b8da964..0099f34f275 100644 --- a/ui/packages/app-shell/src/views/studio-design/ObjectSettingsPanel.cbpRequiredMaster-12326.test.tsx +++ b/ui/packages/app-shell/src/views/studio-design/ObjectSettingsPanel.cbpRequiredMaster-12326.test.tsx @@ -6,11 +6,13 @@ * * Since `@objectstack/spec` 18 (objectstack#23100, ADR-0055 §1) a * `controlled_by_parent` object derives its master from exactly one - * `master_detail` field declared `required: true`, and the save door refuses - * any other shape. The designer forces the value: the pick writes - * `required: true` on the master in the same patch. With two or more - * `master_detail` fields it picks none for the author, and shows the spec's own - * refusal, which names the fix. + * `master_detail` field declared `required: true`. Since ADR-0055 §1 as + * amended (objectstack#30905 ruling A) the spec also reads an object's only + * `master_detail` that is not required as an OPTIONAL master, whose rows may + * name none. The designer forces the value: the pick writes `required: true` + * on the master in the same patch, so every row names its master. With two or + * more `master_detail` fields it picks none for the author, and shows the + * spec's own refusal, which names the fix. * * The installed spec's `ObjectSchema` is the oracle: the object the patch * leaves behind is parsed as the save door parses it. @@ -54,8 +56,11 @@ const TWO_MASTERS = { describe('ObjectSettingsPanel — controlled_by_parent forces the master required (objectui#12326)', () => { it('choosing it with one non-required master writes required: true, and the object parses', async () => { const draft = { name: 'crm_case', sharingModel: 'private', fields: ONE_MASTER }; - // The control leg: as the draft stands, switching only the model is refused. - expect(ObjectSchema.safeParse({ ...draft, sharingModel: 'controlled_by_parent' }).success).toBe(false); + // The control leg: as the draft stands, switching only the model leaves an + // optional master — it parses, as `required: false`. + const control = ObjectSchema.safeParse({ ...draft, sharingModel: 'controlled_by_parent' }); + expect(control.success).toBe(true); + expect((control.data?.fields as Record).account.required).toBe(false); const { onPatch } = renderPanel(draft); await pickSharing(CBP_LABEL); @@ -69,7 +74,9 @@ describe('ObjectSettingsPanel — controlled_by_parent forces the master require account: { type: 'master_detail', label: 'Account', reference: 'crm_account', required: true }, }, }); - expect(ObjectSchema.safeParse({ ...draft, ...patch }).success).toBe(true); + const parsed = ObjectSchema.safeParse({ ...draft, ...patch }); + expect(parsed.success).toBe(true); + expect((parsed.data?.fields as Record).account.required).toBe(true); }); it('writes only the model when the master is already required', async () => { diff --git a/ui/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.cbpRequiredMaster-12326.test.tsx b/ui/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.cbpRequiredMaster-12326.test.tsx index 938eaeb6192..4fff4c8fb9b 100644 --- a/ui/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.cbpRequiredMaster-12326.test.tsx +++ b/ui/packages/app-shell/src/views/studio-design/PackageOwdOverviewPanel.cbpRequiredMaster-12326.test.tsx @@ -10,6 +10,14 @@ * `controlled_by_parent` sets `required: true` on its `master_detail` fields in * the saved draft, as the per-object Settings tab does. The installed spec's * `ObjectSchema` is the oracle for the saved body. + * + * ADR-0055 §1 as amended (objectstack#30905 ruling A) adds an OPTIONAL master: + * an object's only `master_detail` declared `required: false`. The parse cannot + * tell an omitted `required` from `false`, so it admits the omitted shape too. + * This overview still names none for an omitted `required`, as the runtime + * does for an object that reaches it unparsed (it reads `required: false` + * literally, never its absence), and that row records the parse's answer + * apart from the master shown. */ import '@testing-library/jest-dom/vitest'; @@ -54,9 +62,16 @@ afterEach(cleanup); const cbp = (fields: Body): Body => ({ name: 'crm_case', label: 'Case', sharingModel: 'controlled_by_parent', fields }); describe('PackageOwdOverviewPanel — the master shown is the one the runtime derives (objectui#12326)', () => { - const SHAPES: Array<[string, Body, string | undefined]> = [ + // [label, fields, the master shown, whether the spec parse accepts it when + // that differs from "a master is shown"] + const SHAPES: Array<[string, Body, string | undefined, boolean?]> = [ ['one required master', { account: { type: 'master_detail', reference: 'crm_account', required: true } }, 'crm_account'], - ['one master without required', { account: { type: 'master_detail', reference: 'crm_account' } }, undefined], + [ + 'one master without required (the parse reads it as an optional master; no master is shown)', + { account: { type: 'master_detail', reference: 'crm_account' } }, + undefined, + true, + ], [ 'two required masters', { @@ -67,9 +82,10 @@ describe('PackageOwdOverviewPanel — the master shown is the one the runtime de ], ]; - it.each(SHAPES)('%s', async (_label, fields, master) => { - // The oracle: the spec accepts exactly the shapes a master is shown for. - expect(ObjectSchema.safeParse(cbp(fields)).success).toBe(master !== undefined); + it.each(SHAPES)('%s', async (_label, fields, master, parses) => { + // The oracle: the spec accepts the shapes a master is shown for, and the + // one row that says otherwise. + expect(ObjectSchema.safeParse(cbp(fields)).success).toBe(parses ?? master !== undefined); renderPanel({ crm_case: cbp(fields) }); const cell = await screen.findByTestId('owd-master-crm_case'); expect(cell).toHaveTextContent( @@ -86,8 +102,11 @@ describe('PackageOwdOverviewPanel — switching a row to controlled_by_parent (o sharingModel: 'private', fields: { account: { type: 'master_detail', reference: 'crm_account' } }, }; - // The control leg: switching only the model is refused. - expect(ObjectSchema.safeParse({ ...body, sharingModel: 'controlled_by_parent' }).success).toBe(false); + // The control leg: switching only the model leaves an optional master — + // it parses, as `required: false`. + const control = ObjectSchema.safeParse({ ...body, sharingModel: 'controlled_by_parent' }); + expect(control.success).toBe(true); + expect((control.data?.fields as Record).account.required).toBe(false); const { saved } = renderPanel({ crm_case: body }); await screen.findByTestId('owd-row-crm_case'); @@ -103,7 +122,9 @@ describe('PackageOwdOverviewPanel — switching a row to controlled_by_parent (o await waitFor(() => expect(saved.length).toBe(1)); expect(saved[0].body.sharingModel).toBe('controlled_by_parent'); expect((saved[0].body.fields as Record).account.required).toBe(true); - expect(ObjectSchema.safeParse(saved[0].body).success).toBe(true); + const parsed = ObjectSchema.safeParse(saved[0].body); + expect(parsed.success).toBe(true); + expect((parsed.data?.fields as Record).account.required).toBe(true); }); it('a row with two master-detail fields shows no master before the save', async () => { diff --git a/ui/packages/app-shell/src/views/studio-design/owd-sharing.test.ts b/ui/packages/app-shell/src/views/studio-design/owd-sharing.test.ts index 4950511fd54..97d09097411 100644 --- a/ui/packages/app-shell/src/views/studio-design/owd-sharing.test.ts +++ b/ui/packages/app-shell/src/views/studio-design/owd-sharing.test.ts @@ -46,15 +46,29 @@ describe('owd-sharing — deriveMasterObject', () => { * the master, and any other shape names none. The installed spec's * `ObjectSchema` is the oracle for which shapes the platform accepts: every * shape this reader names a master for parses as a `controlled_by_parent` - * object, and every shape it names none for is refused. + * object, and every shape it names none for is refused, with one exception. + * + * ADR-0055 §1 as amended (objectstack#30905 ruling A) adds an OPTIONAL master: + * an object's only `master_detail` declared `required: false`. The parse cannot + * tell an omitted `required` from `false`, so it admits the omitted shape too, + * while this reader names none for it, as the runtime does for an object that + * reaches it unparsed (it reads `required: false` literally, never its + * absence). That row records the parse's answer apart from the master named. */ describe('owd-sharing — deriveMasterObject agrees with the runtime (objectui#12326)', () => { const asCbp = (fields: Record) => ObjectSchema.safeParse({ name: 'crm_case', sharingModel: 'controlled_by_parent', fields }).success; - const SHAPES: Array<[string, Record, string | undefined]> = [ + // [label, fields, the master named, whether the spec parse accepts it when + // that differs from "a master is named"] + const SHAPES: Array<[string, Record, string | undefined, boolean?]> = [ ['one required master', { account: { type: 'master_detail', reference: 'crm_account', required: true } }, 'crm_account'], - ['one master without required', { account: { type: 'master_detail', reference: 'crm_account' } }, undefined], + [ + 'one master without required (the parse reads it as an optional master; this reader names none)', + { account: { type: 'master_detail', reference: 'crm_account' } }, + undefined, + true, + ], [ 'two required masters', { @@ -75,9 +89,9 @@ describe('owd-sharing — deriveMasterObject agrees with the runtime (objectui#1 ['no relationship field', { subject: { type: 'text' } }, undefined], ]; - it.each(SHAPES)('%s', (_label, fields, master) => { + it.each(SHAPES)('%s', (_label, fields, master, parses) => { expect(deriveMasterObject(fields)).toBe(master); - expect(asCbp(fields)).toBe(master !== undefined); + expect(asCbp(fields)).toBe(parses ?? master !== undefined); }); });