From fd846549411f1dba79018e1a71b082506c039142 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 01:44:36 +0000 Subject: [PATCH 01/11] wip(spec): refuse a controlled_by_parent object without exactly one required master_detail Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H Co-authored-by: Claude --- packages/spec/src/data/object.zod.ts | 113 +++++++++++++++++++++++++-- 1 file changed, 107 insertions(+), 6 deletions(-) diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index 11f7a53b263..5e2d4b2bd21 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -1927,6 +1927,98 @@ function refuseAttachedBlockConflicts(attachedOnRead: unknown, fields: unknown, } } +/** + * [ADR-0055 §1 · #23100, maintainer ruling B] A `controlled_by_parent` object + * 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 + * `['sharingModel']` (nothing to derive from) or `['fields']` (more than one + * candidate), the same two locations `os lint`'s `security-cbp-no-relation` / + * `security-cbp-ambiguous-relation` report at. + * + * Why the parse and not only the builder: the shapes this refuses were + * reachable through every door that does not run `ObjectSchema.create()` — a + * plain object literal handed to `defineStack`, a raw `.parse()`, a stored + * `sys_metadata` row — and the security runtime used to derive a master from + * them anyway, through two fallbacks ADR-0055 §1 never recorded: ANY + * `master_detail` (required or not), then a required `lookup`. Both retired + * with this rule (ADR-0049 enforce-or-remove; the step-18 semantic migration + * `cbp-master-relation-tiers-retired`), so the runtime now derives a master + * from exactly the field this rule accepts and from nothing else, and an + * object this rule refuses is denied every read and refused every write by the + * runtime too. The parse says so where an author is present to act. + * + * 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 + * 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 + * the builder with one `master_detail` always passes, and one authored with two + * always has two required ones and is refused. A field's `reference` needs no + * test here: `FieldSchema` already refuses a `master_detail` without one. + * + * Metadata at rest still LOADS: the stored-row seams register an object that + * fails its parse with a `[metadata_spec_invalid]` diagnostic carrying this + * message (ADR-0087's 2026-07-31 addendum, "reads diagnose, never drop"), and + * the save door refuses it with 422. No conversion rewrites it: turning a + * `lookup` into a `master_detail` changes the delete behaviour of every row + * (a detail cascades with its master), and whether an object should rather + * leave `controlled_by_parent` is the author's decision, not a mapping. + */ +function refuseControlledByParentWithoutOneMaster( + objectName: unknown, + sharingModel: unknown, + fields: unknown, + ctx: z.RefinementCtx, +): void { + if (sharingModel !== 'controlled_by_parent') return; + if (fields === null || typeof fields !== 'object' || Array.isArray(fields)) return; + const name = typeof objectName === 'string' && objectName.length > 0 ? objectName : ''; + const entries = Object.entries(fields as Record); + const masters = entries + .filter(([, f]) => f?.type === 'master_detail' && f.required === true) + .map(([key]) => key); + if (masters.length === 1) return; + if (masters.length > 1) { + ctx.addIssue({ + code: 'custom', + path: ['fields'], + message: + `Object '${name}' declares sharingModel 'controlled_by_parent' with ${masters.length} required ` + + `\`master_detail\` fields (${masters.map((m) => `\`${m}\``).join(', ')}). ADR-0055 §1: a ` + + 'controlled-by-parent object derives its master from exactly one required `master_detail` field, so ' + + 'with two the declaration does not say which master governs its rows, and the platform derives none ' + + '(every read is denied, every write refused). Keep `master_detail` on the one master, and declare ' + + 'every other parent as a `lookup` field.', + }); + return; + } + const optionalMasters = entries.filter(([, f]) => f?.type === 'master_detail').map(([key]) => key); + const requiredLookups = entries + .filter(([, f]) => f?.type === 'lookup' && f.required === true) + .map(([key]) => key); + const found = [ + ...optionalMasters.map((m) => `\`${m}\` is a \`master_detail\` without \`required: true\` — add it`), + ...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'` " + + 'to refuse deleting a master that still has rows instead)', + ), + ]; + ctx.addIssue({ + code: 'custom', + path: ['sharingModel'], + message: + `Object '${name}' declares sharingModel 'controlled_by_parent' but no required \`master_detail\` field. ` + + 'ADR-0055 §1: 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 ' + + '(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 ' + + "('private', 'public_read' or 'public_read_write').", + }); +} + // ⚠️ ORDER IS LOAD-BEARING (#5593). This map used to live ~700 lines BELOW // `ObjectSchemaBase`, and the error map that reads it was built lazily // (`objectUnknownKeyErrorImpl ??= …`) purely to step around the temporal dead @@ -2843,6 +2935,10 @@ const ObjectSchemaBase = strictObject( // [#22211 ruling A] A read attachment names at least one leaf, and is not a // field: its block names may not repeat a declared field name. refuseAttachedBlockConflicts(object.attachedOnRead, 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. + refuseControlledByParentWithoutOneMaster(object.name, object.sharingModel, object.fields, ctx); }).overwrite((object) => { // [#23065] A `tree` that names no `reference` references this object, and the // parse writes that down after the refinements above judged what the author @@ -3171,10 +3267,14 @@ function assertReferenceViaSiblingDeclared(objectName: unknown, fields: unknown) * * Lives at `create()` — the authoring surface (ADR-0077) — beside * {@link assertSystemDataIsWritable}, and deliberately NOT in raw - * `.parse()`/`.safeParse()`: metadata already at rest must keep loading. - * Runtime tolerance is the other half of commit 75b7c240a's ruling — the security - * gate's fallbacks stay, and the lint rule stays `warning` until v18 — so - * publish-time refuses new declarations while runtime tolerates old ones. + * `.parse()`/`.safeParse()`: a parse must not rewrite what it reads. The force + * is the builder's convenience, not the rule: the RULE — exactly one required + * `master_detail` under this sharing model — is judged on the parse itself by + * {@link refuseControlledByParentWithoutOneMaster} (#23100, maintainer ruling + * B), so a raw parse of a `master_detail` without `required: true` is refused + * rather than forced, and the security gate's fallbacks this paragraph used to + * call "runtime tolerance" (any `master_detail`, then a required `lookup`) are + * retired. * * Alias spellings (`isRequired` / `mandatory` / `notNull`) need no handling * here: `FieldSchema` is a `strictObject`, so those keys are refused at parse @@ -3341,8 +3441,9 @@ export const ObjectSchema = lazySchema(() => { // [#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()` stay tolerant for metadata at - // rest — see the function's docblock. + // 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. const forcedFields = forceCbpMasterDetailRequired(cfg.name, cfg.sharingModel, cfg.fields); const withDefaults = { ...cfg, From 82c344d42c8a3c10d623c63d958f0dff0cbc0cb8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 01:52:04 +0000 Subject: [PATCH 02/11] wip: one required master_detail for controlled_by_parent across parse, lint and runtime Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H Co-authored-by: Claude --- packages/lint/src/rule-explanations.ts | 49 ++-- .../lint/src/validate-security-posture.ts | 221 +++++++----------- packages/objectql/src/master-detail.ts | 10 +- .../plugins/plugin-security/src/errors.ts | 11 +- .../plugin-security/src/explain-engine.ts | 3 +- .../plugin-security/src/security-plugin.ts | 116 +++++++-- packages/spec/src/data/object.test.ts | 135 ++++++++--- packages/spec/src/data/object.zod.ts | 5 +- .../18.cbp-master-detail-required-forced.ts | 14 +- ...8.cbp-master-detail-required-lint-error.ts | 21 +- .../18.cbp-master-relation-tiers-retired.ts | 49 ++++ 11 files changed, 414 insertions(+), 220 deletions(-) create mode 100644 packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 8af3928cea5..c4d6f3c4122 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -594,22 +594,19 @@ const DATASET_FILTER_FIELD_UNKNOWN_EXPLANATION: RuleExplanation = { const SECURITY_CBP_AMBIGUOUS_RELATION_EXPLANATION: RuleExplanation = { rule: 'security-controlled-by-parent-ambiguous-relation', - covers: 'how the master relation is chosen', + covers: 'why a controlled_by_parent object takes exactly one master', paragraphs: [ 'A `controlled_by_parent` object derives every row\'s record-level access from a master record. ' + - 'ADR-0055 resolves the master through three tiers, in order — a required master_detail, then any ' + - 'master_detail, then a required lookup, each of which must also name a `reference` target — and ' + - 'takes the FIRST match in the tier that wins.', - 'When two or more fields tie in the winning tier, which object this one derives its access from ' + - 'is decided by FIELD DECLARATION ORDER. Moving a field up or down the schema reads as a cosmetic ' + - 'edit in review, and it silently moves that security boundary to another object; the runtime ' + - 'reports nothing when it does, because it does not refuse, it picks. Author time is the only ' + - 'place this can surface, so the rule is an error. A tie in a lower tier is masked by a higher ' + - 'tier\'s single winner and is not reported.', - 'To fix it, leave exactly ONE candidate in the winning tier: promote the intended master into a ' + - 'higher tier (make it the object\'s only required master_detail), or demote the others (drop ' + - '`required`, or change the relation type). System objects are judged too: the ambiguity is a ' + - 'property of the document.', + 'ADR-0055 §1 names that master by exactly one `master_detail` field declared `required: true`, ' + + 'with a `reference` to the master object.', + 'With two or more such fields the declaration does not say which master governs the rows, and the ' + + 'platform derives none: every read is DENIED and every write is refused with 422 INVALID_METADATA, ' + + 'exactly as for an object with no master at all. Picking one by field declaration order — what ' + + 'the runtime once did — would let a cosmetic reordering of the schema move a security boundary to ' + + 'another object. System objects are judged too: the ambiguity is a property of the document.', + 'To fix it, keep `master_detail` on the ONE master this object\'s rows belong to, and declare every ' + + 'other parent as a `lookup` field. `ObjectSchema.create()` makes every `master_detail` on such an ' + + 'object required, so a second parent declared as `master_detail` is always a second master.', ], }; @@ -617,17 +614,19 @@ const SECURITY_CBP_NO_RELATION_EXPLANATION: RuleExplanation = { rule: 'security-controlled-by-parent-no-relation', covers: 'why the object is unusable without a master', paragraphs: [ - 'A `controlled_by_parent` object says its access comes from its master. ADR-0055 resolves the ' + - 'master through a required master_detail, then any master_detail, then a required lookup — each ' + - 'of which must also name a `reference` target — and an object that matches none of the three has ' + - 'nothing to derive access from.', - 'Both runtime halves refuse it: 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 ' + + '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.', + '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 ' + 'refusal does not exempt them.', - 'To fix it, add the master relation this object is derived from, a `master_detail` field with a ' + - '`reference` and `required: true`. An object with no master decides its own baseline: ' + - '\'private\' (the owner plus explicit shares), \'public_read\' or \'public_read_write\'.', + 'To fix it, declare the master relation this object is derived from: a `master_detail` field with a ' + + '`reference` and `required: true` (a lookup to the master becomes that field; a detail is deleted ' + + 'with its master unless it declares `deleteBehavior: \'restrict\'`). An object whose rows must ' + + 'outlive their parent is not a detail and decides its own baseline: \'private\' (the owner plus ' + + 'explicit shares), \'public_read\' or \'public_read_write\'.', ], }; @@ -663,7 +662,7 @@ const SECURITY_MASTER_DETAIL_UNGRANTED_EXPLANATION: RuleExplanation = { '(403) before the parent-derived access is ever consulted — the silent "can\'t submit the ' + 'subtable" trap.', 'A child is an object with a master_detail field, or a `controlled_by_parent` object that resolves ' + - 'its master the way the runtime does (a required lookup included). The rule is a warning, and it ' + + 'its master the way the runtime does (its one required master_detail). The rule is a warning, and it ' + 'stays silent when the stack authors no permission sets or a `\'*\'` wildcard grant covers every ' + 'object; one set granting the child while another forgets it is out of scope.', 'To fix it, grant the object in at least one permission set that already grants its master — ' + diff --git a/packages/lint/src/validate-security-posture.ts b/packages/lint/src/validate-security-posture.ts index ee3feebdbbe..ef3dfa69639 100644 --- a/packages/lint/src/validate-security-posture.ts +++ b/packages/lint/src/validate-security-posture.ts @@ -20,11 +20,12 @@ * | security-grant-expired-at-authoring(err)| ADR-0091 D2 resolution filtering| * | security-delegation-missing-reason(err) | ADR-0091 D3 dual audit | * | security-cbp-no-relation (error) | #7503 (runtime refusal #7474) | - * | security-cbp-ambiguous-relation(error) | #14747 order-dependent master | + * | security-cbp-ambiguous-relation(error) | #14747 order-dependent master; runtime refusal since #23100 | * * Per ADR-0049 discipline these are NOT advisory security: every `error` rule - * BUT ONE mirrors a runtime enforcement point (D1 fail-closed OWD default, D4 - * zod enum + fail-closed evaluator, D5/D9 anchor binding gate, D3 rename wave) + * mirrors a runtime enforcement point (D1 fail-closed OWD default, D4 + * zod enum + fail-closed evaluator, D5/D9 anchor binding gate, D3 rename wave, + * ADR-0055 §1's one required master_detail) * — the lint moves the failure from runtime-deny to author-time fix-it. * `security-anchor-high-privilege` mirrors that gate for the one suggestion a * package can actually declare (`isDefault: true` → the `everyone` anchor, @@ -35,14 +36,12 @@ * app-authored anchor set bound to `guest` (#16110). That binding is held by * the runtime's own `describeAnchorForbiddenBits(set, 'guest')` gate instead. * - * The exception INVERTS that argument rather than weakening it. - * `security-cbp-ambiguous-relation` has no runtime refusal to mirror precisely - * BECAUSE the runtime does not refuse: it silently PICKS a winner by field - * declaration order, so there is no later runtime verdict for the author to - * discover, and author time is the only place the ambiguity can surface at all - * (#14747). What it does satisfy is the admissibility bar the #7503 push site - * below states — a self-contained property of the object document, with no - * per-permission-set nuance to adjudicate and no legitimate reading. + * `security-cbp-ambiguous-relation` used to be the one exception: the runtime + * did not refuse two candidate masters, it silently PICKED one by field + * declaration order (#14747). Since #23100 (maintainer ruling B, ADR-0055 §1) + * the runtime derives a master from exactly one required `master_detail` and + * from nothing else, so two of them now meet the same runtime refusal as none + * — every read denied, every write refused — and both rules mirror it. * * The non-`error` rules are the ones with NO hard runtime refusal behind them * AND no such certainty: @@ -312,104 +311,70 @@ function firstMasterDetailField(obj: AnyRec): { name: string; parent?: string } interface CbpRelation { /** The field that answers. */ field: string; - /** `master_detail` or `lookup`. */ + /** Always `master_detail` — kept so the ungranted-detail rule's message names the type it reports. */ type: string; /** The object it references. */ master: string; } /** - * [#7503] The master-relation precedence, as ONE table both CBP rules read. + * [ADR-0055 §1 · #23100, maintainer ruling B] The master candidates of a + * `controlled_by_parent` object: every `master_detail` field declared + * `required: true` that names a `reference`, in declaration order. The object + * derives its master from EXACTLY ONE of them — zero is + * `security-controlled-by-parent-no-relation`, two or more is + * `security-controlled-by-parent-ambiguous-relation`. * - * A point-for-point mirror of `resolveCbpRelation` in - * `packages/plugins/plugin-security/src/security-plugin.ts` — the SAME - * three-step fallback, in the same order, with the same "must also carry a - * reference target" condition folded into each step (`pick` there requires - * `pred(f) && ref(f)`, so a `master_detail` naming no target resolves nothing): + * 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 + * second and third tiers, and ruling B retired both, so the table collapsed + * to its first row and is spelled as the one predicate it now is. The same + * sentence is read three times, each by the seam that owns its input shape — + * `ObjectSchema`'s parse (`refuseControlledByParentWithoutOneMaster`, + * `packages/spec/src/data/object.zod.ts`), this linter, and the runtime's + * `resolveCbpRelation` (`packages/plugins/plugin-security/src/security-plugin.ts`) + * — and the tests of this rule hold its verdict equal to the parse's on the + * shapes the ruling names. ⛔ Do not re-introduce a fallback tier here: a + * linter that accepts what the parse and the runtime refuse reads, to the next + * author, as a gate that is watching. * - * 1. a `required` `master_detail` with a reference, else - * 2. ANY `master_detail` with a reference, else - * 3. a `required` `lookup` with a reference. - * - * `required` is read for truthiness, not `=== true`, because the runtime does - * (`f?.required`) — mirroring the gate means mirroring its coercions too. - * - * The tiers are a table rather than a `??` chain spelled twice because the - * no-relation rule and the ambiguity rule (#14747) must not be able to disagree - * about which tier wins: one says "no tier matched", the other says "the tier - * that matched holds more than one candidate", and those are answers to the - * same question. The `label` is author-facing — it appears in the ambiguity - * message, so the tier the author is told about is the tier that was tested. - * - * The one DELIBERATE divergence from the runtime is the reference spelling. The - * runtime accepts `reference ?? reference_to ?? referenceTo`; `refOf` here - * accepts only `reference`, the sole spelling `FieldSchema` declares. The - * aliases do not parse (strict schema, #5017), so for any stack an author can - * ship the two agree; re-introducing the alias fallback here would restore - * precisely the inert branch #5017 removed, and on the pre-parse path the - * schema already names the real defect (the alias key) rather than this rule - * guessing past it. - */ -const CBP_TIERS: ReadonlyArray<{ label: string; pred: (f: AnyRec) => boolean }> = [ - { label: 'required master_detail', pred: (f) => f.type === 'master_detail' && !!f.required }, - { label: 'any master_detail', pred: (f) => f.type === 'master_detail' }, - { label: 'required lookup', pred: (f) => f.type === 'lookup' && !!f.required }, -]; - -/** - * [#7503 / #14747] The WINNING tier and EVERY candidate in it, in declaration - * order — `undefined` when no tier matches (nothing to derive access from). - * - * Only the winning tier is reported on, and that is not a simplification: the - * runtime's `??` chain stops at the first tier that resolves, so a tie in a - * lower tier is masked by a higher tier's single winner and is not a decision - * the platform ever makes. Reordering fields inside a masked tier changes - * nothing, and reporting it would be reporting a non-defect. + * `required` is read as `=== true`, the spec's own type for it; the runtime + * reads the same. The one deliberate divergence from the runtime is the + * reference spelling: the runtime accepts `reference ?? reference_to ?? + * referenceTo` (it reads the unparsed registry, ruling of 2026-08-30), while + * `refOf` here accepts only `reference`, the sole spelling `FieldSchema` + * declares. The aliases do not parse (strict schema, #5017), so for any stack + * an author can ship the two agree; on the pre-parse path the schema already + * names the real defect (the alias key) rather than this rule guessing past it. */ -function cbpMasterCandidates(obj: AnyRec): { tier: string; candidates: CbpRelation[] } | undefined { - const entries = recordsOf(obj.fields); - for (const { label, pred } of CBP_TIERS) { - const matched = entries.filter((f) => pred(f) && refOf(f)); - if (matched.length > 0) { - return { - tier: label, - candidates: matched.map((f) => ({ - field: String(f.name ?? '?'), - type: String(f.type), - master: refOf(f) as string, - })), - }; - } - } - return undefined; +function cbpMasterCandidates(obj: AnyRec): CbpRelation[] { + return recordsOf(obj.fields) + .filter((f) => f.type === 'master_detail' && f.required === true && refOf(f)) + .map((f) => ({ field: String(f.name ?? '?'), type: 'master_detail', master: refOf(f) as string })); } /** - * [#7503] The relation a `controlled_by_parent` object derives its access from, - * or `undefined` when the platform has nothing to derive from. - * - * The runtime's winner, exactly: `Array.prototype.find` over a tier is the - * first element `filter` over that same tier keeps, and the first tier with a - * candidate is the tier the `??` chain stops at — so `candidates[0]` of the - * winning tier IS `pick(...) ?? pick(...) ?? pick(...)`, with the ambiguity the - * runtime resolves by position now visible to the caller instead of discarded. + * [#7503 · ADR-0055 §1] The relation a `controlled_by_parent` object derives + * its access from — its one required `master_detail` — or `undefined` when it + * declares none, or more than one (the runtime derives no master from either). */ function resolveCbpRelation(obj: AnyRec): CbpRelation | undefined { - return cbpMasterCandidates(obj)?.candidates[0]; + const candidates = cbpMasterCandidates(obj); + return candidates.length === 1 ? candidates[0] : undefined; } /** * [#22212] The relation that makes an object a CHILD whose record-level access * is derived from a master — the subject of `security-master-detail-ungranted`. * - * A `controlled_by_parent` object is answered by {@link resolveCbpRelation}, - * the runtime's own precedence (required master_detail, then any master_detail, - * then a required LOOKUP): that is the field the platform derives the child's - * record access through, so a lookup-bound child is as much a child as a - * master-detail one. Keyed on `firstMasterDetailField` alone, the rule was - * silent on exactly that shape — a `controlled_by_parent` object bound through - * a required lookup and granted in no set — while object-level CRUD is never - * derived for it either, so the role-bound 403 is the same. + * A `controlled_by_parent` object is answered by {@link resolveCbpRelation}: + * its one required `master_detail` is the field the platform derives the + * child's record access through (ADR-0055 §1). Until #23100 that resolution + * also accepted a required LOOKUP, and #22212 widened this rule to it; ruling + * B retired that tier, so a lookup-bound `controlled_by_parent` object is now + * no child at all — the parse and `security-controlled-by-parent-no-relation` + * refuse it, and this rule has nothing to add. * * Every other object keeps the reading this rule always had: a master_detail * field makes it a detail whatever its sharingModel says. @@ -524,34 +489,38 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } } // ── [#7503] controlled_by_parent with nothing to derive access FROM ── - // The object says "my access comes from my master" and names no master. - // Both runtime halves of ADR-0055 already refuse this shape, and neither + // The object says "my access comes from my master" and names no master: + // no `master_detail` field declared `required: true` (ADR-0055 §1 — since + // #23100 a non-required `master_detail` and a required `lookup` no longer + // count). Both runtime halves of ADR-0055 refuse this shape, and neither // is a judgement call the linter has to second-guess: // - writes → 422 INVALID_METADATA (`MasterDetailRelationMissingError`, // the #7474 split — a metadata defect, explicitly NOT an access verdict) // - reads → `computeControlledByParentFilter` returns RLS_DENY_FILTER, - // so every read is denied too (defense-in-depth, whose own comment says - // "spec validation should prevent authoring it" — this rule is that). + // so every read is denied too. // So it is `error`, not advisory: it mirrors a hard runtime enforcement // point exactly (the ADR-0090 D7 / ADR-0049 criterion at the head of this // file), and the defect is a self-contained property of the object document - // — no per-permission-set nuance to adjudicate, no legitimate reading. + // — no per-permission-set nuance to adjudicate, no legitimate reading. The + // parse refuses the same shape (`ObjectSchema`); this rule is what an + // UNPARSED intake meets. // // NOT exempted for system objects, unlike the D1 unset-OWD rule above: the // runtime refusal does not exempt them either, and an object declaring a // derivation it cannot perform is broken on its own terms, independent of // who may author it. - if (owd === 'controlled_by_parent' && !resolveCbpRelation(obj)) { + const cbpCandidates = owd === 'controlled_by_parent' ? cbpMasterCandidates(obj) : undefined; + if (cbpCandidates && cbpCandidates.length === 0) { findings.push({ severity: 'error', rule: SECURITY_CBP_NO_RELATION, where: `object "${objName}"`, path: `${objPath}.sharingModel`, - // [#22161] One verdict sentence; the ADR-0055 precedence it tested and - // why the object is unusable rather than locked down are this id's + // [#22161] One verdict sentence; the ADR-0055 rule it tested and why + // the object is unusable rather than locked down are this id's // `os explain` entry. message: - `declares sharingModel 'controlled_by_parent' but has no master relation to derive access ` + + `declares sharingModel 'controlled_by_parent' but no required master_detail field to 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` @@ -562,48 +531,38 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } }); } - // ── [#14747] controlled_by_parent whose master is decided by ORDER ──── - // The opposite defect to the one above: not zero candidates, but TWO OR - // MORE in the tier that wins. `pick` is `find` here and in the runtime, so - // the master is whichever candidate the field map happens to list first — - // field declaration order, a property that carries no authored meaning. - // Moving a field up or down a schema file is a review-invisible edit that - // reads as cosmetic, and it silently repoints the record-level security of - // every row of the object. Nothing reports it at runtime: the platform does - // not refuse, it picks (#14747). - // - // `error`, not advisory, and for the INVERSE of the usual reason — there is - // no runtime refusal to mirror BECAUSE the runtime silently decides, which - // makes author time the only place this can ever surface. The - // admissibility bar is the one the rule above states and this shape meets - // identically: a self-contained property of the object document, no - // per-permission-set nuance to adjudicate, and no legitimate reading — two - // tied candidates is not an author saying which master they meant. See the - // ADR-0049 paragraph in the module header. + // ── [#14747 · #23100] controlled_by_parent with TWO OR MORE masters ─── + // The opposite defect to the one above: not zero candidates, but two or + // more required `master_detail` fields. Until #23100 the runtime did not + // refuse this — it PICKED the first by field declaration order, so moving + // a field up or down a schema file silently repointed the record-level + // security of every row (#14747), and this rule was the only place the + // tie could surface. Ruling B (ADR-0055 §1: exactly one) made the runtime + // derive NO master from two, so the shape now meets the same refusal as + // the rule above — every read denied, every write refused — and this rule + // mirrors it. Kept a separate id because the remedy differs: not "add a + // master" but "keep one". // // NOT exempted for system objects, for the same reason the rule above is // not: the ambiguity is a property of the document, independent of who may // author it. - const cbpTier = owd === 'controlled_by_parent' ? cbpMasterCandidates(obj) : undefined; - if (cbpTier && cbpTier.candidates.length > 1) { - const winner = cbpTier.candidates[0]; - const roster = cbpTier.candidates.map((cand) => `"${cand.field}" → ${cand.master}`).join(', '); + if (cbpCandidates && cbpCandidates.length > 1) { + const roster = cbpCandidates.map((cand) => `"${cand.field}" → ${cand.master}`).join(', '); findings.push({ severity: 'error', rule: SECURITY_CBP_AMBIGUOUS_RELATION, where: `object "${objName}"`, path: `${objPath}.fields`, - // [#22161] One verdict sentence; why a tie is decided by declaration - // order, and what reordering does to the security boundary, are this - // id's `os explain` entry. + // [#22161] One verdict sentence; why exactly one master is the rule is + // this id's `os explain` entry. message: - `the controlled_by_parent master is decided by field order: ${cbpTier.candidates.length} fields ` + - `tie in the ${cbpTier.tier} tier (${roster}) and "${winner.field}" wins today`, - // [#22161] One remedy line; the tier precedence, and why only a tie in - // the winning tier is ambiguous, are this id's `os explain` entry. + `declares sharingModel 'controlled_by_parent' with ${cbpCandidates.length} required master_detail ` + + `fields (${roster}), so no master is derived: every read is denied and every write refused ` + + `(422 INVALID_METADATA)`, + // [#22161] One remedy line. hint: - `Leave ONE candidate in the ${cbpTier.tier} tier: make the intended master the only required ` + - `master_detail, or drop required (or change the relation type) on the others`, + `Keep master_detail on the ONE master this object's rows belong to, and declare every other parent ` + + `as a lookup field`, }); } @@ -934,8 +893,8 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } // silent "can't fill in / can't submit the subtable" trap (framework#2700, // downstream os-tianshun-mtc#43). Statically detectable: a detail (has a // master_detail field, or — #22212 — is `controlled_by_parent` and resolves - // its master the way the runtime does, a required lookup included) that NO - // authored permission set grants. + // its master the way the runtime does: its one required master_detail, + // ADR-0055 §1) that NO authored permission set grants. // // Advisory `warning` — it does not gate the build. Two deliberate silences // keep the false-positive rate near zero: (a) if the package authors no diff --git a/packages/objectql/src/master-detail.ts b/packages/objectql/src/master-detail.ts index d5f26b09bc2..a6230e0aa24 100644 --- a/packages/objectql/src/master-detail.ts +++ b/packages/objectql/src/master-detail.ts @@ -13,10 +13,12 @@ * This module answers exactly one question — *which field points at the master, * and which object is it?* — so the write path can resolve the header row * without re-deriving the relationship at each call site. `plugin-security` - * keeps its own `controlled_by_parent` resolver: that one is deliberately more - * permissive (it falls back to a required `lookup`) because a CBP object with - * no derivable master must deny reads rather than open them, which is a - * different question from "what does `parent` mean in this predicate". + * keeps its own `controlled_by_parent` resolver because a CBP object with no + * derivable master must deny reads rather than open them, which is a different + * question from "what does `parent` mean in this predicate". (It used to be + * more permissive than this one — falling back to any `master_detail`, then a + * required `lookup`; since #23100 it resolves exactly one required + * `master_detail`, ADR-0055 §1, and nothing else.) * * ## Why `parent` is not guessed * diff --git a/packages/plugins/plugin-security/src/errors.ts b/packages/plugins/plugin-security/src/errors.ts index 70006b01035..a857d1c10d5 100644 --- a/packages/plugins/plugin-security/src/errors.ts +++ b/packages/plugins/plugin-security/src/errors.ts @@ -108,6 +108,11 @@ export class PermissionDeniedError extends Error { * `sharingModel: 'controlled_by_parent'` on an object with no `master_detail` * relation to derive access from → `422 INVALID_METADATA`. * + * [ADR-0055 §1 · #23100] "No relation" means: not exactly one `master_detail` + * field declared `required: true` — none, a non-required one, a required + * `lookup` only, or two or more required ones all land here, because the + * relation resolver derives a master from that one field and nothing else. + * * This is a precisely detectable AUTHORING defect, not an access verdict: the * object declares that its access is derived from a master and then gives the * platform no master to derive it from. Disguised as a routine permission @@ -128,8 +133,10 @@ export class MasterDetailRelationMissingError extends Error { constructor(object: string, operation: string) { super( `[Security] Configuration defect: ${operation} on '${object}' cannot be authorized because the object ` - + `declares sharingModel 'controlled_by_parent' but has no master_detail relation to derive access from. ` - + `Declare a required master_detail field pointing at the master object, or change sharingModel.`, + + `declares sharingModel 'controlled_by_parent' but not exactly one required master_detail field to derive ` + + `access from (a non-required master_detail or a required lookup derives none, and neither do two ` + + `required ones). Declare one master_detail field with required: true pointing at the master object, ` + + `or change sharingModel.`, ); this.name = 'MasterDetailRelationMissingError'; } diff --git a/packages/plugins/plugin-security/src/explain-engine.ts b/packages/plugins/plugin-security/src/explain-engine.ts index c22a124bc6d..a15469a1b3b 100644 --- a/packages/plugins/plugin-security/src/explain-engine.ts +++ b/packages/plugins/plugin-security/src/explain-engine.ts @@ -949,7 +949,8 @@ const MASTER_DENIAL_LEG_DETAIL: Record */ const MASTER_UNRESOLVED_DETAIL: Record = { master_detail_relation_missing: { - condition: 'the object declares controlled_by_parent with no master_detail relation to derive access from', + condition: + 'the object declares controlled_by_parent without exactly one required master_detail field to derive access from', answer: '422 INVALID_METADATA', }, record_not_found: { condition: 'the record does not exist', answer: '404 RECORD_NOT_FOUND' }, diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 0986060dcc5..b7a48a251f7 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -8580,8 +8580,31 @@ export class SecurityPlugin implements Plugin { /** * Resolve a controlled_by_parent object's master-detail relation (the FK field - * key + the master object name), or null. Prefers a required `master_detail` - * field; falls back to any `master_detail`, then a required `lookup`. Cached. + * key + the master object name), or null. Cached. + * + * [ADR-0055 §1 · #23100, maintainer ruling B] The master is the object's ONE + * `master_detail` field declared `required: true` that names a reference — and + * nothing else. Zero such fields, or two or more, resolve `null`, which both + * halves already treat as fail-closed: the read leg answers `RLS_DENY_FILTER` + * and the write leg throws `MasterDetailRelationMissingError` (422 + * INVALID_METADATA). This used to be a three-tier fallback — a required + * `master_detail`, else ANY `master_detail`, else a required `lookup`, the + * first match by field order within a tier — and ADR-0055 §1 recorded only + * the first tier. Ruling B retired the second and third (ADR-0049 + * enforce-or-remove; step-18 semantic migration + * `cbp-master-relation-tiers-retired`) and refused two candidates rather than + * picking one by declaration order. The same sentence is judged by + * `ObjectSchema`'s parse (`refuseControlledByParentWithoutOneMaster`) and by + * `os lint` (`cbpMasterCandidates` in + * `packages/lint/src/validate-security-posture.ts`); this reader keeps its own + * read of it because it reads the unparsed registry — the reference-spelling + * 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. + * + * 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 + * denied in silence. * * [#8688] Also carries {@link CbpRelation.omissionRefusedByValidation} — the * one bit the insert leg's conditional stand-down turns on. It is resolved @@ -8648,11 +8671,14 @@ export class SecurityPlugin implements Plugin { const k = refKey(f); return k === undefined ? undefined : f[k]; }; - const pick = (pred: (f: any) => boolean) => entries.find(([, f]) => pred(f) && ref(f)); - const found = - pick((f) => f?.type === 'master_detail' && f?.required) ?? - pick((f) => f?.type === 'master_detail') ?? - pick((f) => f?.type === 'lookup' && f?.required); + // [ADR-0055 §1 · #23100 ruling B] Exactly one required `master_detail` + // 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; + if (!found && masterGovernsRowWrites(schema)) { + this.reportCbpMasterUnresolved(object, entries, candidates.map(([k]) => String(k))); + } if (found) { const def = found[1]; // Resolved from the SAME field def the relation itself came from, and @@ -8674,10 +8700,10 @@ export class SecurityPlugin implements Plugin { // — it is the licence for the guard to stand down, and every shape it // reports `true` for is a shape validation provably refuses. // - // `master_detail` is required on top of the validator's own test: the - // relation resolver's third fallback accepts a required `lookup`, and - // the ruling covers `master_detail` only. Narrower than the validator - // is the safe direction — the guard keeps answering. + // `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. omissionRefusedByValidation: def?.type === 'master_detail' && !!def?.required && !def?.readonly && !def?.system, }; @@ -8686,6 +8712,63 @@ export class SecurityPlugin implements Plugin { return rel; } + /** + * [ADR-0055 §1 · #23100] Report that a `controlled_by_parent` object resolved NO + * master relation, so every read of it is denied and every write refused — and + * say why, naming the retired tier the object relied on when that is the cause. + * + * The parse refuses the same shape (`ObjectSchema`, with the same prescription), + * so an object reaching here unresolved came through a door that does not + * parse — a raw `registerObject`, or a stored row the boot registered anyway + * under its `[metadata_spec_invalid]` diagnostic. The read leg's denial is an + * EMPTY result, indistinguishable from "no rows" to its caller, which is why + * the reason is stated once here rather than left to be inferred. + * + * `warn`, not `error`, by the degradation-log-level question (AGENTS.md): + * nothing claimed persisted is lost — the object is visibly unusable, every + * write is refused to its caller with 422 INVALID_METADATA, and the denial is + * the fail-closed answer, not a fall-back. Same granularity and channel as + * {@link SecurityPlugin.reportRejectedReferenceAlias}: the only call site is + * `resolveCbpRelation`'s resolution body, which runs on a `cbpRelCache` miss + * alone, so it fires once per object and re-arms when metadata changes. + */ + private reportCbpMasterUnresolved( + object: string, + entries: ReadonlyArray, + requiredMasters: readonly string[], + ): void { + const optionalMasters = entries + .filter(([, f]) => f?.type === 'master_detail' && f?.required !== true) + .map(([k]) => String(k)); + const requiredLookups = entries + .filter(([, f]) => f?.type === 'lookup' && f?.required === true) + .map(([k]) => String(k)); + const why = + requiredMasters.length > 1 + ? `it declares ${requiredMasters.length} required master_detail fields (${requiredMasters.join(', ')}) ` + + '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)`] + : []), + ...(requiredLookups.length > 0 + ? [ + `required lookup: ${requiredLookups.join(', ')} (a lookup derives no access — declare the ` + + "master as type: 'master_detail', or give the object its own sharingModel)", + ] + : []), + ].join('; '); + this.logger.warn?.( + `[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.', + { object, requiredMasters, optionalMasters, requiredLookups }, + ); + } + /** * Report that a `controlled_by_parent` master relation resolved ONLY because * {@link SecurityPlugin.resolveCbpRelation} still accepts a spelling @@ -8784,9 +8867,9 @@ export class SecurityPlugin implements Plugin { * * An empty set yields `{ masterFK: { $in: [] } }`, which matches no rows (fail * closed), as does a failure to resolve the sharing half. A misconfigured - * object (no master_detail/lookup to derive from) denies all reads - * (defense-in-depth; spec validation should prevent authoring it). Returns null - * when the object is not controlled_by_parent. + * object (not exactly one required master_detail to derive from, ADR-0055 §1) + * denies all reads (defense-in-depth; the spec's parse refuses authoring it + * since #23100). Returns null when the object is not controlled_by_parent. * * [ADR-0055 amendment] The derivation COMPOSES ACROSS A CHAIN. It used to resolve the * master set from the two halves above and nothing else, which made a master @@ -9266,6 +9349,11 @@ export class SecurityPlugin implements Plugin { // PURPOSE so metadata already at rest keeps loading (its docblock // says so). Measured: `safeParse` of a `controlled_by_parent` object // takes both an omitted `required` and an explicit `required: false`. + // ⚠️ [#23100, ruling B] SUPERSEDED for this shape: the parse now + // refuses it, and `resolveCbpRelation` resolves only a required + // `master_detail`, so a non-required one never becomes `rel` and this + // branch is unreachable for it (the object meets the `!rel` 422 + // above instead). The freeze below still binds the other two shapes. // • `required` + `readonly` and `required` + `system` — STILL newly // authorable through `ObjectSchema.create()` itself: the force // returns early on `required === true` and never inspects diff --git a/packages/spec/src/data/object.test.ts b/packages/spec/src/data/object.test.ts index aaca3c8c865..7fc5089039b 100644 --- a/packages/spec/src/data/object.test.ts +++ b/packages/spec/src/data/object.test.ts @@ -11,6 +11,7 @@ import { resolveInjectedSystemColumns } from './injected-system-columns'; import { projectPublishedJsonSchema } from '../../scripts/lib/refinement-projection'; import { collectDroppedRefinements } from '../../scripts/lib/dropped-refinements'; import { Field } from './field.zod'; +import { defineStack } from '../stack.zod'; import { isMaskedOnReadFieldType } from './masked-field-types'; import type { StateMachineValidation } from './validation.zod'; @@ -1524,22 +1525,21 @@ describe('ObjectSchema.create() forces a required master_detail under controlled expect(message).toContain('change its `sharingModel`'); }); - it('forces EVERY master_detail reference under the object, not just the first', () => { - // The prose contract is "exactly one required master_detail", but nothing - // enforces the count today (#7474 owns the zero-reference case at publish); - // forcing each declared reference keeps every candidate safe rather than - // silently blessing only the first. - const obj = ObjectSchema.create({ - name: 'cbp_multi', - sharingModel: 'controlled_by_parent', - fields: { - a: { type: 'master_detail', reference: 'master_a' }, - b: { type: 'master_detail', reference: 'master_b' }, - }, - }); - const fields = obj.fields as Record; - expect(fields.a.required).toBe(true); - expect(fields.b.required).toBe(true); + it('forces EVERY master_detail reference — so two of them are two masters, and the parse refuses them', () => { + // [#23100 ruling B] Replaced, not deleted: this pinned that both references + // were forced while "nothing enforces the count today". The count is now + // the parse's rule (ADR-0055 §1: exactly one), so forcing both makes the + // object a two-master declaration, refused naming both fields. + expect(() => + ObjectSchema.create({ + name: 'cbp_multi', + sharingModel: 'controlled_by_parent', + fields: { + a: { type: 'master_detail', reference: 'master_a' }, + b: { type: 'master_detail', reference: 'master_b' }, + }, + }), + ).toThrow(/Object 'cbp_multi' declares sharingModel 'controlled_by_parent' with 2 required `master_detail` fields \(`a`, `b`\)/); }); it('leaves master_detail on a NON-controlled_by_parent object alone (scope pin)', () => { @@ -1557,12 +1557,14 @@ describe('ObjectSchema.create() forces a required master_detail under controlled expect((explicit.fields as Record).parent.required).toBe(false); }); - it('raw .parse()/.safeParse() stay TOLERANT of the old shape — metadata at rest keeps loading', () => { - // The other half of commit 75b7c240a's ruling: the narrowing is authoring-time - // only. Stored metadata rehydrated through the schema (never through the - // builder) must keep loading, UNREWRITTEN — runtime tolerance for existing - // installs stays with the security gate, and the lint rule stays `warning` - // until the v18 card (#9139) promotes it. + 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. const atRest = { name: 'cbp_stored', sharingModel: 'controlled_by_parent', @@ -1571,12 +1573,93 @@ describe('ObjectSchema.create() forces a required master_detail under controlled }, }; const result = ObjectSchema.safeParse(atRest); - expect(result.success).toBe(true); - if (result.success) { - const fields = (result.data as { fields: Record }).fields; - expect(fields.parent.required).toBe(false); + expect(result.success).toBe(false); + 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\./); + } + }); +}); + +// ============================================================================ +// controlled_by_parent derives its master from EXACTLY ONE required +// master_detail — ADR-0055 §1, enforced at the parse (#23100, maintainer +// ruling B). The runtime's two fallbacks (any master_detail; a required lookup) +// retired with it. +// ============================================================================ + +describe('ObjectSchema refuses a controlled_by_parent object without exactly one required master_detail', () => { + const cbp = (fields: Record) => ({ name: 'cbp_line', sharingModel: 'controlled_by_parent', fields }); + const issuesOf = (input: unknown) => { + const r = ObjectSchema.safeParse(input); + return r.success ? [] : r.error.issues.map((i) => ({ path: i.path.join('.'), message: i.message })); + }; + + it('ACCEPTS one required master_detail — through the raw parse and through the builder', () => { + expect(issuesOf(cbp({ parent: { type: 'master_detail', reference: 'cbp_header', required: true } }))).toEqual([]); + // The builder forces an omitted `required` on its one master_detail. + expect(() => ObjectSchema.create(cbp({ parent: Field.masterDetail('cbp_header', { label: 'Header' }) }))).not.toThrow(); + // One required master plus any number of lookups — a lookup is not a master. + expect(issuesOf(cbp({ + parent: { type: 'master_detail', reference: 'cbp_header', required: true }, + owner: { type: 'lookup', reference: 'sys_user', required: true }, + }))).toEqual([]); + }); + + it('REFUSES a required lookup as the only relation (the retired third tier), naming the object, the rule and the field', () => { + 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).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( + /declares sharingModel 'controlled_by_parent' but no required `master_detail` field/, + ); + }); + + 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('REFUSES no relation at all, and two required master_details, at the two lint-shared paths', () => { + expect(issuesOf(cbp({ title: { type: 'text' } })).map((i) => i.path)).toEqual(['sharingModel']); + const two = issuesOf(cbp({ + a: { type: 'master_detail', reference: 'master_a', required: true }, + b: { type: 'master_detail', reference: 'master_b', required: true }, + })); + expect(two.map((i) => i.path)).toEqual(['fields']); + expect(two[0].message).toMatch(/^Object 'cbp_line' declares sharingModel 'controlled_by_parent' with 2 required `master_detail` fields \(`a`, `b`\)\. ADR-0055 §1:/); + }); + + it('judges controlled_by_parent only — every other sharing model keeps its relations as authored', () => { + for (const sharingModel of ['private', 'public_read', 'public_read_write', undefined]) { + expect(issuesOf({ name: 'plain_line', sharingModel, fields: { parent: { type: 'lookup', reference: 'plain_header', required: true } } })).toEqual([]); + expect(issuesOf({ name: 'plain_line', sharingModel, fields: { title: { type: 'text' } } })).toEqual([]); + } + }); + + it('reaches defineStack — a plain object literal is judged like a raw parse, not like the builder', () => { + const stack = (fields: Record) => ({ + manifest: { id: 'com.example.cbp-one-master', namespace: 'cbpo', version: '0.0.1', type: 'app' as const, name: 'CBP' }, + objects: [ + { name: 'cbpo_header', label: 'Header', fields: { name: { type: 'text', label: 'Name' } } }, + { name: 'cbpo_line', label: 'Line', sharingModel: 'controlled_by_parent', fields }, + ], + }); + 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( + /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 5e2d4b2bd21..7f23b5191a7 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -1933,8 +1933,9 @@ function refuseAttachedBlockConflicts(attachedOnRead: unknown, fields: unknown, * `required: true`. The parse refuses the object otherwise — zero such fields, * or two or more — naming the object and the rule, at * `['sharingModel']` (nothing to derive from) or `['fields']` (more than one - * candidate), the same two locations `os lint`'s `security-cbp-no-relation` / - * `security-cbp-ambiguous-relation` report at. + * candidate), the same two locations `os lint` reports + * `security-controlled-by-parent-no-relation` / + * `security-controlled-by-parent-ambiguous-relation` at. * * Why the parse and not only the builder: the shapes this refuses were * reachable through every door that does not run `ObjectSchema.create()` — a diff --git a/packages/spec/src/migrations/entries/semantic/18.cbp-master-detail-required-forced.ts b/packages/spec/src/migrations/entries/semantic/18.cbp-master-detail-required-forced.ts index 6d6b7aaf447..97298407a62 100644 --- a/packages/spec/src/migrations/entries/semantic/18.cbp-master-detail-required-forced.ts +++ b/packages/spec/src/migrations/entries/semantic/18.cbp-master-detail-required-forced.ts @@ -9,10 +9,11 @@ export const entry: SemanticMigration = { replacement: '`required: true` on the master reference (or nothing at all — the builder now ' + 'forces `required: true` when the key is omitted). An explicit `required: false` on that ' + 'shape is refused at `ObjectSchema.create()` with a located error carrying this same ' - + 'prescription. Metadata at rest is untouched: raw `.parse()`/`.safeParse()` still accept ' - + 'the old shape, the security gate\'s derived enforcement stays, and the lint rule ' - + '`relationship/master-detail-required` stays `warning` until its own v18 promotion ' - + '(Direction 1 of the 2026-08-16 maintainer ruling whose Direction 2 this is)', + + 'prescription. The builder does not rewrite metadata at rest; a raw `.parse()`/`.safeParse()` of a ' + + '`controlled_by_parent` object whose only `master_detail` is not `required: true` is refused, and ' + + 'the security gate derives no master from it, by the later step-18 entry ' + + '`cbp-master-relation-tiers-retired`; the lint rule `relationship/master-detail-required` reports ' + + 'the shape at `error` under `cbp-master-detail-required-lint-error`', reason: 'A `controlled_by_parent` detail derives ALL of its record access from the master that its ' + '`master_detail` reference names (ADR-0055). With the reference not `required`, an insert ' @@ -32,6 +33,7 @@ export const entry: SemanticMigration = { + '`required: true`) or declares `required: true` explicitly; the authored app builds and ' + 'boots. An explicit `required: false` there fails the build with ' + '`ObjectSchema.create(...): field ... declares required: false on a master_detail ' - + 'reference under sharingModel: controlled_by_parent`. Stored metadata keeps loading ' - + 'byte-identically (`safeParse` green, `required` unrewritten).', + + 'reference under sharingModel: controlled_by_parent`. Stored metadata is not rewritten by ' + + 'the builder (`required` stays as stored); what a raw parse does with it is ' + + '`cbp-master-relation-tiers-retired`\'s acceptance criterion.', }; diff --git a/packages/spec/src/migrations/entries/semantic/18.cbp-master-detail-required-lint-error.ts b/packages/spec/src/migrations/entries/semantic/18.cbp-master-detail-required-lint-error.ts index 0149fa4ea64..6cc40799da9 100644 --- a/packages/spec/src/migrations/entries/semantic/18.cbp-master-detail-required-lint-error.ts +++ b/packages/spec/src/migrations/entries/semantic/18.cbp-master-detail-required-lint-error.ts @@ -30,20 +30,23 @@ export const entry: SemanticMigration = { + '`cbp-master-detail-required-forced` entry) forces `required: true` at ' + '`ObjectSchema.create` but never inspects `readonly` or `system`, so two of the three shapes ' + 'still pass the builder and meet their first authoring-time refusal here, and the third ' - + 'still reaches it from any object not authored through the builder. Runtime tolerance is ' - + 'unchanged on purpose: the security gate keeps refusing these inserts, and keeps resolving ' - + 'the master for metadata already at rest.', + + 'still reaches it from any object not authored through the builder. The security gate keeps ' + + 'refusing these inserts. (The later step-18 entry `cbp-master-relation-tiers-retired` goes ' + + 'further for the first shape: a `master_detail` without `required: true` is refused at parse, ' + + 'and the runtime no longer resolves a master from it.)', acceptanceCriteria: '`os lint` reports no `relationship/master-detail-required` finding at `error`: on every ' + 'object declaring `sharingModel: \'controlled_by_parent\'`, each `master_detail` reference ' + 'declares `required: true` (or omits it and is authored through `ObjectSchema.create`, ' + 'which emits `required: true`) and carries neither `readonly: true` nor `system: true`. ' - + '⚠️ WHICH DOOR: the refusal is `os lint`\'s and the metadata-generation rubric\'s only. The ' - + 'rule is not in the authoring-rule registry, so `os build`, `os validate` and the metadata ' - + 'save door do not run it, and a stack carrying the shape still builds and publishes — read ' - + '`os lint`\'s exit code, not a green build. Stored metadata is not rewritten and keeps ' - + 'loading, and the security gate still refuses an insert that omits the master FK on these ' - + 'shapes. Repo census at the time of the change: 129 authored objects across the example ' + + '⚠️ WHICH DOOR: this rule\'s refusal is `os lint`\'s and the metadata-generation rubric\'s ' + + 'only. The rule is not in the authoring-rule registry, so `os build`, `os validate` and the ' + + 'metadata save door do not run it, and a stack carrying the `readonly` or `system` shape still ' + + 'builds and publishes — read `os lint`\'s exit code, not a green build. (A missing `required` ' + + 'on an object\'s only `master_detail` is refused at parse by every one of those doors since ' + + '`cbp-master-relation-tiers-retired`.) Stored metadata is not rewritten and keeps loading, and ' + + 'the security gate still refuses an insert that omits the master FK on these shapes. Repo ' + + 'census at the time of the change: 129 authored objects across the example ' + 'apps, the platform and plugin objects and the CLI\'s golden eval corpus, 7 of them ' + '`controlled_by_parent`, 0 findings at `error`.', }; diff --git a/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts b/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts new file mode 100644 index 00000000000..db95387dea7 --- /dev/null +++ b/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts @@ -0,0 +1,49 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { SemanticMigration } from '../../types.js'; + +// ADR-0055 §1 enforced (maintainer ruling B): a `controlled_by_parent` object +// derives its master from exactly one required `master_detail`. The runtime's two +// unrecorded fallbacks retire under ADR-0049, and no D2 conversion exists — a +// `lookup` turned `master_detail` changes delete behaviour, and leaving +// `controlled_by_parent` is an author's call — so this entry is the whole record. +export const entry: SemanticMigration = { + id: 'cbp-master-relation-tiers-retired', + surface: 'object.sharingModel = \'controlled_by_parent\' together with object.fields — the master ' + + 'relation the object derives its record access from', + replacement: 'Exactly one `master_detail` field declared `required: true`, with a `reference` to the master ' + + 'object; every other parent of the object is a `lookup`. Nothing else derives a master any more: a ' + + '`master_detail` without `required: true` and a required `lookup` were the security runtime\'s second ' + + 'and third fallbacks and both retired, and two required `master_detail` fields are no longer resolved ' + + 'by field order. `ObjectSchema` refuses each of those shapes at parse, naming the object — so ' + + '`ObjectSchema.create()`, `defineStack`, `os validate` and the metadata save door refuse them — and ' + + 'the runtime derives no master from them: every read of such an object is denied and every write ' + + 'refused with 422 INVALID_METADATA. A required `lookup` to the master becomes `type: \'master_detail\'` ' + + '(its rows are then deleted with their master; declare `deleteBehavior: \'restrict\'` to refuse deleting ' + + 'a master that still has rows instead). An object whose rows must outlive their parent leaves ' + + '`controlled_by_parent` for its own sharing model (`private`, `public_read` or `public_read_write`).', + reason: + 'ADR-0055 §1 states that a `controlled_by_parent` object must declare exactly one required ' + + '`master_detail` field and that anything else is a validation error, failing closed. Neither the parse ' + + 'nor the runtime kept it: the parse accepted the object with no master at all, and the runtime derived ' + + 'a master through a three-tier fallback the record never stated — a required `master_detail`, then any ' + + '`master_detail`, then a required `lookup`, the first match by field order within a tier. So an author ' + + 'reading the record believed such an object was refused while it booted and derived its access from ' + + 'whatever the fallback picked, and the platform\'s own derived proof (the parent-derived write reach ' + + 'family) disagreed with its runtime about which objects the rule covers. The maintainer ruling (letter ' + + 'B) enforces the record: one tier, refused at parse, retired in the runtime. No conversion is offered ' + + 'because none is lossless — turning a `lookup` into a `master_detail` changes what deleting the master ' + + 'does to every row, and whether an object should instead stop being controlled by its parent is a ' + + 'product decision only its author can make.', + acceptanceCriteria: + 'Every object declaring `sharingModel: \'controlled_by_parent\'` declares exactly one `master_detail` ' + + 'field with `required: true` (through `ObjectSchema.create()`, an omitted `required` on its one ' + + '`master_detail` is emitted as `true`), and `ObjectSchema.safeParse` of it succeeds: no ' + + '"declares sharingModel \'controlled_by_parent\' but no required `master_detail` field" or ' + + '"with N required `master_detail` fields" issue. `os lint` reports neither ' + + '`security-controlled-by-parent-no-relation` nor `security-controlled-by-parent-ambiguous-relation`. At ' + + 'runtime the security plugin logs no `[security/controlled-by-parent]` "resolves no master relation" ' + + 'line for the object, and a member who can read its master reads its rows. Stored metadata is not ' + + 'rewritten: a stored row in a retired shape still registers (with its `[metadata_spec_invalid]` ' + + 'diagnostic) and is denied until it is corrected and saved.', +}; From 185569fafe8bdf368586697d28d522b663dc5f5b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 02:11:43 +0000 Subject: [PATCH 03/11] wip(lint): CBP rules read the one required master_detail; pins reversed Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H Co-authored-by: Claude --- packages/lint/src/rule-explanations.ts | 6 +- .../src/validate-security-posture.test.ts | 320 +++++++++--------- .../lint/src/validate-security-posture.ts | 5 +- 3 files changed, 168 insertions(+), 163 deletions(-) diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index c4d6f3c4122..770f82ddaa2 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -624,9 +624,9 @@ const SECURITY_CBP_NO_RELATION_EXPLANATION: RuleExplanation = { 'refusal does not exempt them.', 'To fix it, declare the master relation this object is derived from: a `master_detail` field with a ' + '`reference` and `required: true` (a lookup to the master becomes that field; a detail is deleted ' + - 'with its master unless it declares `deleteBehavior: \'restrict\'`). An object whose rows must ' + - 'outlive their parent is not a detail and decides its own baseline: \'private\' (the owner plus ' + - 'explicit shares), \'public_read\' or \'public_read_write\'.', + 'with its master unless it declares `deleteBehavior: \'restrict\'`). An object with no master decides ' + + 'its own baseline — and so does one whose rows must outlive their parent, which is not a detail: ' + + '\'private\' (the owner plus explicit shares), \'public_read\' or \'public_read_write\'.', ], }; diff --git a/packages/lint/src/validate-security-posture.test.ts b/packages/lint/src/validate-security-posture.test.ts index dfbffb2ce1c..b0856b648dc 100644 --- a/packages/lint/src/validate-security-posture.test.ts +++ b/packages/lint/src/validate-security-posture.test.ts @@ -806,11 +806,13 @@ describe('validateSecurityPosture · master-detail detail ungranted (framework#2 ).toEqual([]); }); - // [#22212] The child the RUNTIME derives access for, not only a master_detail - // one: a `controlled_by_parent` object resolves its master through a required - // lookup too (ADR-0055's third tier), and object-level CRUD is never derived - // for it either. The incident-shape control above stays the firing control. - describe('a controlled_by_parent child bound through a required lookup (#22212)', () => { + // [#22212 → #23100] The child the RUNTIME derives access for. #22212 widened + // this rule to a `controlled_by_parent` object bound through a required + // lookup, because the runtime resolved its master through one (the third + // tier). Ruling B retired that tier: such an object has NO master now, the + // parse and the no-relation error refuse it, and this advisory has nothing + // to add. The incident-shape control above stays the firing control. + describe('a controlled_by_parent child bound through a required lookup (#22212, tier retired by #23100)', () => { const lookupChild = (fieldOverrides: Record = {}, childGrant?: Record) => ({ objects: [ { name: 'crm_campaign', label: 'Campaign', sharingModel: 'private', fields: { name: { name: 'name', label: 'Name' } } }, @@ -834,17 +836,16 @@ describe('validateSecurityPosture · master-detail detail ungranted (framework#2 ], }); - it('warns when the lookup-bound child is granted by no permission set', () => { - const md = mdOnly(lookupChild()); - expect(md).toHaveLength(1); - expect(md[0]).toMatchObject({ severity: 'warning', where: 'object "crm_campaign_member"' }); - expect(md[0].path).toBe('objects[1].fields.campaign'); - expect(md[0].message).toContain('lookup "campaign" → "crm_campaign"'); - expect(md[0].hint).toContain('permissions[i].objects.crm_campaign_member'); - }); - - it('stays silent once the child is granted', () => { - expect(mdOnly(lookupChild({}, { allowRead: true }))).toEqual([]); + it('a REQUIRED lookup resolves no master any more — the no-relation error speaks, not this advisory', () => { + // Reversed, not deleted: this pinned the advisory FIRING on the + // lookup-bound child (the HotCRM `crm_campaign_member` shape). With the + // third tier retired the object is no child at all; what an author meets + // is the no-relation error, granted or not. + for (const grant of [undefined, { allowRead: true }]) { + const findings = validateSecurityPosture(lookupChild({}, grant)); + expect(findings.filter((f) => f.rule === SECURITY_MASTER_DETAIL_UNGRANTED)).toEqual([]); + expect(findings.map((f) => f.rule)).toContain('security-controlled-by-parent-no-relation'); + } }); it('an OPTIONAL lookup resolves no master — the no-relation error speaks, not this advisory', () => { @@ -895,16 +896,30 @@ describe('validateSecurityPosture · master-detail detail ungranted (framework#2 }); }); -// ── Rule: security-controlled-by-parent-no-relation (#7503 / #7474) ─── +// ── Rule: security-controlled-by-parent-no-relation (#7503 / #7474 / #23100) ── // -// The accept bar is the rule's own probe, and it is FOUR fixtures, not one: the -// defect shape must be reported, and each of the THREE shapes the runtime -// `resolveCbpRelation` actually resolves must be silent — one negative control -// per fallback step, because a rule that only mirrors step 1 would report two -// perfectly working objects. The three steps (security-plugin.ts): -// 1. a required `master_detail` 2. ANY `master_detail` 3. a required `lookup` -// each additionally requiring a `reference` target — mirrored by the fourth -// fixture pair below (a relation naming nothing resolves nothing). +// ADR-0055 §1, enforced by maintainer ruling B (#23100): a `controlled_by_parent` +// object derives its master from EXACTLY ONE `master_detail` field declared +// `required: true` that names a `reference`, and from nothing else. The rule +// used to mirror a three-step runtime fallback — 1. a required master_detail, +// 2. ANY master_detail, 3. a required lookup — with one silent negative control +// per step. Steps 2 and 3 retired, so their two controls are REVERSED below +// (each now errors) rather than deleted: a fallback put back into the rule, +// the parse or the runtime turns them red. +// +// Every verdict is also held equal to the PARSE's on the same object +// (`ObjectSchema` — `refuseControlledByParentWithoutOneMaster`), because the +// card behind the ruling was precisely three seams answering three different +// questions about one rule. +/** The `ObjectSchema` parse's controlled_by_parent issues on one object, by path. */ +function cbpParseIssuePaths(obj: Record): string[] { + const parsed = ObjectSchema.safeParse(obj); + if (parsed.success) return []; + return parsed.error.issues + .filter((i) => /declares sharingModel 'controlled_by_parent'/.test(i.message)) + .map((i) => i.path.join('.')); +} + describe('validateSecurityPosture · controlled_by_parent with no relation (#7503)', () => { const cbpOnly = (stack: Record) => validateSecurityPosture(stack).filter((f) => f.rule === SECURITY_CBP_NO_RELATION); @@ -937,25 +952,23 @@ describe('validateSecurityPosture · controlled_by_parent with no relation (#750 expect(cbpOnly(cbpStack(undefined))).toHaveLength(1); }); - // ── NEGATIVE CONTROLS — one per fallback step of `resolveCbpRelation` ── - it('stays silent on step 1: a REQUIRED master_detail', () => { + // ── NEGATIVE CONTROL — the one shape that resolves ───────────────────── + it('stays silent on a REQUIRED master_detail — the one tier ADR-0055 §1 records', () => { expect( cbpOnly(cbpStack({ order: { name: 'order', type: 'master_detail', reference: 'work_order', required: true } })), ).toEqual([]); }); - // Step 2 is a RUNTIME tolerance and nothing more. This pin used to read - // "ANY master_detail (not marked required)" as a supported shape; the v18 - // narrowing of the authoring contract (maintainer ruling of 2026-08-16, - // Direction 1) retires that reading. `resolveCbpRelation` keeps resolving a - // non-required master_detail — metadata already at rest keeps loading — so - // this rule, which mirrors the resolver, stays silent; and the SAME - // declaration is now refused at authoring time by - // `relationship/master-detail-required` at `error` (data-model-rules.ts). - // Both halves are asserted on one stack so neither can drift unseen. - it('stays silent on step 2: ANY master_detail (not marked required) resolves at runtime — and lint refuses that shape at error', () => { + // ── The two RETIRED tiers — reversed pins (#23100 ruling B) ───────────── + // + // Step 2 used to be pinned silent here ("ANY master_detail resolves at + // runtime", with `relationship/master-detail-required` refusing the shape at + // `error` beside it). The runtime no longer resolves it, so this rule now + // refuses it too; the data-model rule still speaks as well, and the pair is + // asserted on one stack so neither can drift unseen. + it('errors on a NON-required master_detail (the retired second tier) — and the data-model rule still refuses it', () => { const stack = cbpStack({ order: { name: 'order', type: 'master_detail', reference: 'work_order' } }); - expect(cbpOnly(stack)).toEqual([]); + expect(cbpOnly(stack)).toHaveLength(1); const authoring = lintDataModel(stack.objects as any[]).filter( (issue) => issue.rule === 'relationship/master-detail-required', @@ -964,22 +977,19 @@ describe('validateSecurityPosture · controlled_by_parent with no relation (#750 expect(authoring[0]).toMatchObject({ severity: 'error', path: 'objects[1].fields.order.required' }); }); - it('stays silent on step 3: a REQUIRED lookup', () => { + // 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)', () => { expect( cbpOnly(cbpStack({ order: { name: 'order', type: 'lookup', reference: 'work_order', required: true } })), - ).toEqual([]); + ).toHaveLength(1); }); - // The step-3 predicate is `lookup AND required` — an optional lookup is NOT a - // fourth resolution step at runtime, so it must still be reported. This is the - // "plausible thing for an agent to write" shape #7503 names: cbp next to a - // lookup someone forgot to mark required. - it('errors on an OPTIONAL lookup — the runtime does not resolve one', () => { + it('errors on an OPTIONAL lookup — it never resolved', () => { expect(cbpOnly(cbpStack({ order: { name: 'order', type: 'lookup', reference: 'work_order' } }))).toHaveLength(1); }); - // `pick` requires `pred(f) && ref(f)`: a relation naming no target resolves - // nothing, on every one of the three steps. + // A relation naming no target resolves nothing. it.each([ ['required master_detail', { name: 'order', type: 'master_detail', required: true }], ['bare master_detail', { name: 'order', type: 'master_detail' }], @@ -1031,22 +1041,51 @@ describe('validateSecurityPosture · controlled_by_parent with no relation (#750 }), ).toHaveLength(2); }); + + // ── 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']], + ['a required lookup', { order: { type: 'lookup', label: 'O', reference: 'work_order', required: true } }, ['sharingModel']], + ['no relation', { note: { type: 'text', label: 'N' } }, ['sharingModel']], + [ + 'two required master_details', + { + a: { type: 'master_detail', label: 'A', reference: 'work_order', required: true }, + b: { type: 'master_detail', label: 'B', reference: 'work_order', required: true }, + }, + ['fields'], + ], + ])('the parse and the lint agree on %s', (_label, fields, parsePaths) => { + const child = { name: 'work_order_item', label: 'Work Order Item', sharingModel: 'controlled_by_parent', fields }; + expect(cbpParseIssuePaths(child)).toEqual(parsePaths); + // The accepted shape parses WHOLE — so the refusals above are this rule's, + // not a refinement that never ran because something else failed first. + if (parsePaths.length === 0) expect(ObjectSchema.safeParse(child).success).toBe(true); + const lint = validateSecurityPosture(cbpStack(fields)) + .filter((f) => f.rule === SECURITY_CBP_NO_RELATION || f.rule === SECURITY_CBP_AMBIGUOUS_RELATION) + .map((f) => f.path.replace(/^objects\[1\]\./, '')); + expect(lint).toEqual(parsePaths); + }); }); -// ── Rule: security-controlled-by-parent-ambiguous-relation (#14747) ── +// ── Rule: security-controlled-by-parent-ambiguous-relation (#14747 / #23100) ── // -// The mirror image of the rule above: not ZERO candidates, but two or more in -// the tier that WINS. The runtime picks with `find`, so the master is decided -// by field declaration order and nothing says so. -// -// The accept bar has two halves, and the second is the one that makes this -// rule worth having. It must FIRE on a tie in each of the three tiers, and it -// must stay SILENT whenever the tie sits in a tier the `??` chain never -// reaches — a masked tie is not a decision the platform makes, and reporting it -// would send authors to edit fields that change nothing. +// The mirror image of the rule above: not ZERO required master_detail fields, +// but two or more. Until #23100 the runtime picked the first by field +// declaration order and nothing said so, in each of three tiers; this rule +// reported a tie in whichever tier won. Ruling B left one tier and made the +// runtime derive NO master from a tie, so the rule now reports one shape — +// two or more required `master_detail` fields — and the shapes that used to be +// ties in the retired tiers (two required lookups, two non-required +// master_details) are no-relation objects instead, pinned as such below. describe('validateSecurityPosture · controlled_by_parent with an AMBIGUOUS master (#14747)', () => { const ambiguousOnly = (stack: Record) => validateSecurityPosture(stack).filter((f) => f.rule === SECURITY_CBP_AMBIGUOUS_RELATION); + const cbpRules = (stack: Record) => + validateSecurityPosture(stack) + .map((f) => f.rule) + .filter((r) => r === SECURITY_CBP_NO_RELATION || r === SECURITY_CBP_AMBIGUOUS_RELATION); /** A cbp object carrying exactly the fields under test, plus two masters. */ const cbpStack = (fields: unknown): Record => ({ @@ -1057,16 +1096,14 @@ describe('validateSecurityPosture · controlled_by_parent with an AMBIGUOUS mast ], }); + const REQ_ACCOUNT_MD = { name: 'account', type: 'master_detail', reference: 'crm_account', required: true }; + const REQ_CONTACT_MD = { name: 'contact', type: 'master_detail', reference: 'crm_contact', required: true }; const REQ_ACCOUNT_LOOKUP = { name: 'account', type: 'lookup', reference: 'crm_account', required: true }; const REQ_CONTACT_LOOKUP = { name: 'contact', type: 'lookup', reference: 'crm_contact', required: true }; - // ── POSITIVE CONTROLS — one tie per tier ──────────────────────── - // - // The first is the shape measured on the card: HotCRM's `crm_contract`, two - // required lookups and no master_detail, where swapping the two field - // declarations moved the resolved master from `crm_account` to `crm_contact`. - it('errors on tier 3: two required lookups, naming both candidates and the winner', () => { - const findings = ambiguousOnly(cbpStack({ account: REQ_ACCOUNT_LOOKUP, contact: REQ_CONTACT_LOOKUP })); + // ── POSITIVE CONTROL ───────────────────────────────────────────────────── + it('errors on two REQUIRED master_detail fields, naming both candidates in declaration order', () => { + const findings = ambiguousOnly(cbpStack({ account: REQ_ACCOUNT_MD, contact: REQ_CONTACT_MD })); expect(findings).toHaveLength(1); expect(findings[0]).toMatchObject({ severity: 'error', @@ -1074,81 +1111,69 @@ describe('validateSecurityPosture · controlled_by_parent with an AMBIGUOUS mast where: 'object "crm_contract"', path: 'objects[2].fields', }); - // Every candidate is named — field and master, its type in the tier — in - // DECLARATION order, which is the order that decides the winner. A message - // naming only the winner would describe the pick, not the ambiguity. + expect(findings[0].message).toContain('2 required master_detail fields'); expect(findings[0].message).toContain('"account" → crm_account'); expect(findings[0].message).toContain('"contact" → crm_contact'); - expect(findings[0].message.indexOf('"account" →')).toBeLessThan( - findings[0].message.indexOf('"contact" →'), - ); - // The tier that was actually tested, and the winner it currently resolves. - expect(findings[0].message).toContain('required lookup tier'); - expect(findings[0].message).toContain('"account" wins today'); - expect(findings[0].hint).toContain('master_detail'); - }); - - it('names the OTHER field as the winner when the same two are declared the other way round', () => { - const findings = ambiguousOnly(cbpStack({ contact: REQ_CONTACT_LOOKUP, account: REQ_ACCOUNT_LOOKUP })); - expect(findings).toHaveLength(1); - expect(findings[0].message).toContain('"contact" wins today'); - expect(findings[0].message).toContain('"contact" → crm_contact'); + expect(findings[0].message).toContain('no controlled_by_parent master is derived'); + expect(findings[0].hint).toContain('lookup'); }); - it('errors on tier 1: two REQUIRED master_detail fields', () => { - const findings = ambiguousOnly( - cbpStack({ - a: { name: 'a', type: 'master_detail', reference: 'crm_account', required: true }, - b: { name: 'b', type: 'master_detail', reference: 'crm_contact', required: true }, - }), - ); - expect(findings).toHaveLength(1); - expect(findings[0].message).toContain('required master_detail tier'); + // Reversed, not deleted: this used to pin "names the OTHER field as the + // winner when the same two are declared the other way round". There is no + // winner any more — declaration order decides nothing, which is the point. + it('names no winner: the same two fields declared either way round are the same refusal', () => { + const ab = ambiguousOnly(cbpStack({ account: REQ_ACCOUNT_MD, contact: REQ_CONTACT_MD })); + const ba = ambiguousOnly(cbpStack({ contact: REQ_CONTACT_MD, account: REQ_ACCOUNT_MD })); + expect(ab).toHaveLength(1); + expect(ba).toHaveLength(1); + for (const f of [...ab, ...ba]) expect(f.message).not.toMatch(/wins/); }); - it('errors on tier 2: two master_detail fields, NEITHER required', () => { + it('names all three when three required master_detail fields are declared', () => { const findings = ambiguousOnly( cbpStack({ - a: { name: 'a', type: 'master_detail', reference: 'crm_account' }, - b: { name: 'b', type: 'master_detail', reference: 'crm_contact' }, + a: REQ_ACCOUNT_MD, + b: REQ_CONTACT_MD, + c: { name: 'c', type: 'master_detail', reference: 'crm_account', required: true }, }), ); expect(findings).toHaveLength(1); - expect(findings[0].message).toContain('any master_detail tier'); + expect(findings[0].message).toContain('3 required master_detail fields'); }); - it('names all three when three candidates tie', () => { - const findings = ambiguousOnly( - cbpStack({ - a: REQ_ACCOUNT_LOOKUP, - b: REQ_CONTACT_LOOKUP, - c: { name: 'c', type: 'lookup', reference: 'crm_account', required: true }, - }), - ); - expect(findings).toHaveLength(1); - expect(findings[0].message).toContain('3 fields tie'); + it('reports the array field form too', () => { + expect(ambiguousOnly(cbpStack([REQ_ACCOUNT_MD, REQ_CONTACT_MD]))).toHaveLength(1); }); - it('reports the array field form too', () => { - expect(ambiguousOnly(cbpStack([REQ_ACCOUNT_LOOKUP, REQ_CONTACT_LOOKUP]))).toHaveLength(1); + // ── The former tier-2 / tier-3 ties are NO-RELATION objects now ───────── + it('two required lookups are no tie any more: no candidate at all, so the no-relation rule speaks', () => { + // The tier-3 tie this rule was first written for (#14747). + expect(cbpRules(cbpStack({ account: REQ_ACCOUNT_LOOKUP, contact: REQ_CONTACT_LOOKUP }))).toEqual([ + SECURITY_CBP_NO_RELATION, + ]); }); - // ── NEGATIVE CONTROLS — a tie the runtime never reaches is NOT a defect ── - it('stays silent when a single required master_detail masks a tie in the lookup tier', () => { + it('two NON-required master_detail fields are no tie any more either', () => { expect( - ambiguousOnly( + cbpRules( cbpStack({ - parent: { name: 'parent', type: 'master_detail', reference: 'crm_account', required: true }, - account: REQ_ACCOUNT_LOOKUP, - contact: REQ_CONTACT_LOOKUP, + a: { name: 'a', type: 'master_detail', reference: 'crm_account' }, + b: { name: 'b', type: 'master_detail', reference: 'crm_contact' }, }), ), + ).toEqual([SECURITY_CBP_NO_RELATION]); + }); + + // ── NEGATIVE CONTROLS — one required master_detail is not a tie ───────── + it('stays silent when one required master_detail sits beside required lookups', () => { + expect( + cbpRules(cbpStack({ parent: { ...REQ_ACCOUNT_MD, name: 'parent' }, account: REQ_ACCOUNT_LOOKUP, contact: REQ_CONTACT_LOOKUP })), ).toEqual([]); }); - it('stays silent when tier 1 has ONE winner and tier 2 holds the rest', () => { + it('stays silent when one required master_detail sits beside a non-required one', () => { expect( - ambiguousOnly( + cbpRules( cbpStack({ parent: { name: 'parent', type: 'master_detail', reference: 'crm_account', required: true }, other: { name: 'other', type: 'master_detail', reference: 'crm_contact' }, @@ -1157,35 +1182,19 @@ describe('validateSecurityPosture · controlled_by_parent with an AMBIGUOUS mast ).toEqual([]); }); - it.each([ - ['one required master_detail', { order: { name: 'order', type: 'master_detail', reference: 'crm_account', required: true } }], - ['one bare master_detail', { order: { name: 'order', type: 'master_detail', reference: 'crm_account' } }], - ['one required lookup', { order: REQ_ACCOUNT_LOOKUP }], - ])('stays silent on %s — one candidate in the winning tier is not a tie', (_label, fields) => { - expect(ambiguousOnly(cbpStack(fields))).toEqual([]); - }); - - // `pick` is `pred(f) && ref(f)` in both halves: a relation naming no target - // is not a candidate, so it cannot make a tie — nor can an OPTIONAL lookup, - // which is no resolution step at all. - it('stays silent when the second "candidate" names no reference target', () => { - expect( - ambiguousOnly(cbpStack({ account: REQ_ACCOUNT_LOOKUP, contact: { name: 'contact', type: 'lookup', required: true } })), - ).toEqual([]); - }); - - it('stays silent when the second lookup is OPTIONAL', () => { + // A relation naming no target is not a candidate, so it cannot make a tie. + it('stays silent when the second required master_detail names no reference target', () => { expect( - ambiguousOnly(cbpStack({ account: REQ_ACCOUNT_LOOKUP, contact: { name: 'contact', type: 'lookup', reference: 'crm_contact' } })), + ambiguousOnly(cbpStack({ account: REQ_ACCOUNT_MD, contact: { name: 'contact', type: 'master_detail', required: true } })), ).toEqual([]); }); it.each(['private', 'public_read', 'public_read_write'])( - 'stays silent for a two-candidate object whose sharingModel is %s — it derives nothing', + 'stays silent for a two-master object whose sharingModel is %s — it derives nothing', (model) => { expect( ambiguousOnly({ - objects: [{ name: 'crm_contract', label: 'C', sharingModel: model, fields: { account: REQ_ACCOUNT_LOOKUP, contact: REQ_CONTACT_LOOKUP } }], + objects: [{ name: 'crm_contract', label: 'C', sharingModel: model, fields: { account: REQ_ACCOUNT_MD, contact: REQ_CONTACT_MD } }], }), ).toEqual([]); }, @@ -1193,40 +1202,36 @@ describe('validateSecurityPosture · controlled_by_parent with an AMBIGUOUS mast // ── The two CBP rules partition the space; they never both fire ────── it('is mutually exclusive with the no-relation rule', () => { - const cbpRules = (stack: Record) => - validateSecurityPosture(stack) - .map((f) => f.rule) - .filter((r) => r === SECURITY_CBP_NO_RELATION || r === SECURITY_CBP_AMBIGUOUS_RELATION); expect(cbpRules(cbpStack({ note: { name: 'note', type: 'text', label: 'Note' } }))).toEqual([ SECURITY_CBP_NO_RELATION, ]); - expect(cbpRules(cbpStack({ account: REQ_ACCOUNT_LOOKUP, contact: REQ_CONTACT_LOOKUP }))).toEqual([ + expect(cbpRules(cbpStack({ account: REQ_ACCOUNT_MD, contact: REQ_CONTACT_MD }))).toEqual([ SECURITY_CBP_AMBIGUOUS_RELATION, ]); }); - // Same reasoning as the no-relation rule: the runtime's pick does not exempt - // system objects, and an order-dependent security boundary is a property of - // the document, not of who authored it. + // Same reasoning as the no-relation rule: the runtime refusal does not + // exempt system objects, and the ambiguity is a property of the document, + // not of who authored it. it('reports system objects too', () => { expect( ambiguousOnly({ objects: [ - { name: 'sys_thing_link', sharingModel: 'controlled_by_parent', fields: { a: REQ_ACCOUNT_LOOKUP, b: REQ_CONTACT_LOOKUP } }, - { name: 'thing_internal', isSystem: true, sharingModel: 'controlled_by_parent', fields: { a: REQ_ACCOUNT_LOOKUP, b: REQ_CONTACT_LOOKUP } }, + { name: 'sys_thing_link', sharingModel: 'controlled_by_parent', fields: { a: REQ_ACCOUNT_MD, b: REQ_CONTACT_MD } }, + { name: 'thing_internal', isSystem: true, sharingModel: 'controlled_by_parent', fields: { a: REQ_ACCOUNT_MD, b: REQ_CONTACT_MD } }, ], }), ).toHaveLength(2); }); - // The mirror's ONE deliberate divergence (#5017), restated from this rule's - // side: `reference_to` is a rejected alias, so a field carrying it is not a - // candidate here and cannot create a tie. Re-introducing the alias fallback - // would restore the inert branch #5017 removed. + // The mirror's ONE deliberate divergence from the runtime (#5017), restated + // from this rule's side: `reference_to` is a rejected alias, so a field + // carrying it is not a candidate here and cannot create a tie. Re-introducing + // the alias fallback would restore the inert branch #5017 removed. it('does not count a `reference_to` field as a candidate', () => { expect( ambiguousOnly( - cbpStack({ account: REQ_ACCOUNT_LOOKUP, contact: { name: 'contact', type: 'lookup', reference_to: 'crm_contact', required: true } }), + cbpStack({ account: REQ_ACCOUNT_MD, contact: { name: 'contact', type: 'master_detail', reference_to: 'crm_contact', required: true } }), ), ).toEqual([]); }); @@ -1621,8 +1626,8 @@ describe('validateSecurityPosture — reads only keys the spec declares (meta-te 'flsGraph', // #16108: the shared object index — `.has` / `.get`, JS Map methods. 'flsField', // #16108: the key's remainder, a string — `.length`, a JS property. 'declared', // #16108: one object's sorted field-name list — `.length` / `.slice` / `.join`. - 'entries', // #7503: the rule's own field list — `.find`, a JS method. - 'matched', // #14747: one tier's candidate list — `.length` / `.map`, JS methods. + 'candidates', // #23100: `resolveCbpRelation`'s candidate list — `.length`, a JS property. + 'cbpCandidates', // #23100: one object's required master_detail list — `.length` / `.map`, JS methods. 'declaredCapabilities', // #18535: the stack's own capability list — `.length`, a JS property. ]); expect(receivers.filter((r) => !tabled.has(r) && !PLUMBING.has(r))).toEqual([]); @@ -1820,9 +1825,11 @@ const REACHABILITY_CORPUS: Array<{ label: string; stack: Record objects: [ objectFixture({ name: 'contract', sharingModel: 'controlled_by_parent', + // [#23100] Two required master_details — the one tie left after the + // retired tiers (two required lookups now reach the no-relation site). fields: { - account: { type: 'lookup', label: 'Account', reference: 'account', required: true }, - contact: { type: 'lookup', label: 'Contact', reference: 'contact', required: true }, + account: { type: 'master_detail', label: 'Account', reference: 'account', required: true }, + contact: { type: 'master_detail', label: 'Contact', reference: 'contact', required: true }, }, }), ], @@ -1880,7 +1887,7 @@ describe('[#22161] one-line verdicts — the rule ids this file shortened', () = // What each verdict stopped saying, which `os explain RULE_ID` now prints. const MOVED: Record = { - [SECURITY_CBP_AMBIGUOUS_RELATION]: ['ADR-0055', 'FIELD DECLARATION ORDER', 'security boundary', 'it does not refuse, it picks'], + [SECURITY_CBP_AMBIGUOUS_RELATION]: ['ADR-0055 §1', 'exactly one', 'INVALID_METADATA', 'security boundary'], [SECURITY_FLS_UNKNOWN_FIELD]: ['PermissionEvaluator.getFieldPermissions', 'stays as readable and as editable', 'field rename', 'security-fls-unqualified-key'], [SECURITY_CBP_NO_RELATION]: ['ADR-0055', 'INVALID_METADATA', 'unusable rather than merely locked down'], [SECURITY_MASTER_DETAIL_UNGRANTED]: ['`controlled_by_parent`', 'SEPARATE gate', "can't submit the", 'wildcard'], @@ -1948,8 +1955,7 @@ describe('[#22161] one-line fixes — the rule ids this file shortened', () => { const MOVED_FROM_FIX: Record = { [SECURITY_CBP_AMBIGUOUS_RELATION]: [ - 'a required master_detail, then any master_detail, then a required lookup', - "A tie in a lower tier is masked by a higher tier's single winner", + '`ObjectSchema.create()` makes every `master_detail` on such an object required', ], [SECURITY_MASTER_DETAIL_UNGRANTED]: ['named `sys_*` or set `isSystem: true`'], [SECURITY_CBP_NO_RELATION]: ["'private' (the owner plus explicit shares)", 'An object with no master decides its own baseline'], diff --git a/packages/lint/src/validate-security-posture.ts b/packages/lint/src/validate-security-posture.ts index ef3dfa69639..67a21b8d54e 100644 --- a/packages/lint/src/validate-security-posture.ts +++ b/packages/lint/src/validate-security-posture.ts @@ -556,9 +556,8 @@ export function validateSecurityPosture(stack: AnyRec, opts?: { nowMs?: number } // [#22161] One verdict sentence; why exactly one master is the rule is // this id's `os explain` entry. message: - `declares sharingModel 'controlled_by_parent' with ${cbpCandidates.length} required master_detail ` + - `fields (${roster}), so no master is derived: every read is denied and every write refused ` + - `(422 INVALID_METADATA)`, + `declares ${cbpCandidates.length} required master_detail fields (${roster}), so no ` + + `controlled_by_parent master is derived: every read denied, every write refused`, // [#22161] One remedy line. hint: `Keep master_detail on the ONE master this object's rows belong to, and declare every other parent ` + From 973eb8250a29e52d4ff48f016a972b5465063368 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 02:15:19 +0000 Subject: [PATCH 04/11] wip(lint): re-judge the expression-count fixture off controlled_by_parent Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H Co-authored-by: Claude --- packages/lint/src/validate-expressions.test.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/lint/src/validate-expressions.test.ts b/packages/lint/src/validate-expressions.test.ts index 806c046ac69..4d3ba40082c 100644 --- a/packages/lint/src/validate-expressions.test.ts +++ b/packages/lint/src/validate-expressions.test.ts @@ -3856,11 +3856,16 @@ describe('validateStackExpressions — every changed read is reachable from a sp // Exactly one master ⇒ `parent` binds ⇒ NO finding. That verdict depends on // `masterDetailCount` reading `reference`; if the read were broken the count // would be 0 and this would fire. + // + // [#23100] The sharing model is incidental to the count, so the fixtures + // declare `private`: `controlled_by_parent` is spec-valid only with exactly + // one REQUIRED master_detail (ADR-0055 §1, enforced at parse), which the + // two-master half below can never satisfy. expect( validateStackExpressions( specValid({ objects: [{ - name: 'line_item', label: 'Line', sharingModel: 'controlled_by_parent', + name: 'line_item', label: 'Line', sharingModel: 'private', fields: { invoice_ref: { type: 'master_detail', label: 'Invoice', reference: 'invoice' }, locked: { type: 'text', label: 'Locked', readonlyWhen: 'parent.posted == true' }, @@ -3874,7 +3879,7 @@ describe('validateStackExpressions — every changed read is reachable from a sp const issues = validateStackExpressions( specValid({ objects: [{ - name: 'line_item', label: 'Line', sharingModel: 'controlled_by_parent', + name: 'line_item', label: 'Line', sharingModel: 'private', fields: { invoice_ref: { type: 'master_detail', label: 'Invoice', reference: 'invoice' }, order_ref: { type: 'master_detail', label: 'Order', reference: 'sales_order' }, From 6ae38f1668b5a22e09641c1a0164ae6f68fb3212 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 02:19:28 +0000 Subject: [PATCH 05/11] wip: re-judge verify fixtures; error docs drop the retired tiers; runtime pins Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H Co-authored-by: Claude --- content/docs/api/error-catalog.mdx | 20 ++- .../docs/protocol/kernel/error-handling.mdx | 27 +-- .../src/controlled-by-parent-sharing.test.ts | 161 ++++++++++++++---- .../verify/src/parent-write-reach.test.ts | 39 +++-- 4 files changed, 175 insertions(+), 72 deletions(-) diff --git a/content/docs/api/error-catalog.mdx b/content/docs/api/error-catalog.mdx index 88ef13e1af5..f8fd37ef2c8 100644 --- a/content/docs/api/error-catalog.mdx +++ b/content/docs/api/error-catalog.mdx @@ -182,13 +182,16 @@ record, so the gate authorizing writes to it resolves that master *before* the e and the executor is where required-field validation runs. Which of the two refuses first depends on how the master reference is declared. A `master_detail` that is `required` and neither `readonly` nor `system` reaches validation and answers the documented -`400 VALIDATION_FAILED` with `fields`. The other four declarable shapes never reach it and +`400 VALIDATION_FAILED` with `fields`. The other two declarable shapes never reach it and answer `422 MISSING_REQUIRED_FIELD` with no `fields`: -- a `master_detail` with no `required` - a `master_detail` that is `required` + `readonly` - a `master_detail` that is `required` + `system` -- a `required` `lookup`, when the object declares no `master_detail` + +A `master_detail` with no `required`, and a `required` `lookup` on an object that declares no +`master_detail`, are not master references at all: the master is exactly one `master_detail` +declared `required: true` (ADR-0055 §1). `ObjectSchema` refuses such an object at parse, and +one that reaches the runtime unparsed answers every write `422 INVALID_METADATA`. The full matrix is on the protocol page: [`MISSING_REQUIRED_FIELD`](/docs/protocol/kernel/error-handling#missing_required_field). @@ -197,17 +200,16 @@ An update or delete **by id** whose *stored* master reference is null answers th `422`, whatever the declaration — the caller sent no such field, so nothing could be named in `fields` and no payload would fix it. -The refusal itself is correct and is not going to be relaxed: on those four shapes +The refusal itself is correct and is not going to be relaxed: on those two shapes required-field validation does not fire (it skips `system` and `readonly` fields before its -required check, and never fires on a field that is not `required`), so the gate is the only +required check), so the gate is the only thing standing between the request and a detail record with a null master reference — which the `controlled_by_parent` read filter (`fk IN (readable masters)`) can never match, leaving a record readable by nobody. -These shapes are **authorable today**: lint reports a `master_detail` without `required` as -a warning only, and does not report the `readonly`, `system`, or fallback-`lookup` shapes at -all. **Branch on `code`, treat `fields` as optional, and read the status off the response** -rather than deriving it from the table at the end of this page. +These shapes are **authorable today**: `os lint` reports both at error, but the publish gate +does not run that rule. **Branch on `code`, treat `fields` as optional, and read the status +off the response** rather than deriving it from the table at the end of this page. ### `INVALID_FORMAT` diff --git a/content/docs/protocol/kernel/error-handling.mdx b/content/docs/protocol/kernel/error-handling.mdx index 47d74ad38b0..7f148360c50 100644 --- a/content/docs/protocol/kernel/error-handling.mdx +++ b/content/docs/protocol/kernel/error-handling.mdx @@ -497,15 +497,21 @@ depends on how the reference is declared: | Master reference declared as | Refused by | Status | `code` | `fields` | |---|---|:---:|---|:---:| | `master_detail` + `required`, not `readonly`/`system` | required-field validation | 400 | `VALIDATION_FAILED` | yes | -| `master_detail` with no `required` | the master-access gate | **422** | `MISSING_REQUIRED_FIELD` | **absent** | | `master_detail` + `required` + `readonly` | the master-access gate | **422** | `MISSING_REQUIRED_FIELD` | **absent** | | `master_detail` + `required` + `system` | the master-access gate | **422** | `MISSING_REQUIRED_FIELD` | **absent** | -| a `required` `lookup`, when the object declares no `master_detail` | the master-access gate | **422** | `MISSING_REQUIRED_FIELD` | **absent** | -Only the first row is the documented 400 shape. On the other four, messages are prefixed +Only the first row is the documented 400 shape. On the other two, messages are prefixed `[Security] Missing master reference:` and name the object and the field, but no `fields` array rides along. +Two declarations are not rows of this table, because they name **no master at all**. A +`controlled_by_parent` object derives its master from exactly one `master_detail` field +declared `required: true` (ADR-0055 §1), so an object whose only relation is a `master_detail` +without `required`, or a `required` `lookup`, derives none — and neither does one with two +required `master_detail` fields. `ObjectSchema` refuses such an object at parse, naming it; +one that reaches the runtime unparsed has every read denied and every write refused with +`422 INVALID_METADATA`, whatever the request carries. + The same `422 MISSING_REQUIRED_FIELD` also answers an update or delete **by id** whose *stored* master reference is null, whatever the declaration. That is a different path: the caller supplied no such field, so there is no request field to name in `fields` and no @@ -513,19 +519,18 @@ payload that would fix it. **Why the gate refuses rather than handing over.** Required-field validation skips provenance-flagged fields before its required check is reached (`system` and `readonly` -fields are skipped outright) and never fires on a field that is not `required` at all. On -those four shapes the gate is the only thing refusing the write, and letting it through was +fields are skipped outright). On those two shapes the gate is the only thing refusing the +write, and letting it through was measured to create a detail record whose master reference is null — a record the `controlled_by_parent` read filter (`fk IN (readable masters)`) can never match, so it is readable by nobody and answers 422 on every later write by id. The refusal is correct; only its status departs from the rule above. -**These shapes are authorable today.** `os lint` refuses the three `master_detail` shapes at -*error* (`relationship/master-detail-required`), but that rule is not part of the publish -gate — `os build`, `os validate` and the metadata save door do not run it — and it does not -report the fallback-`lookup` shape at all. So a stack can publish clean and still reach the -422. Branch on `code`, and read the status off the response rather than deriving it from this -page. +**These shapes are authorable today.** `os lint` refuses both at *error* +(`relationship/master-detail-required`), but that rule is not part of the publish gate — +`os build`, `os validate` and the metadata save door do not run it. So a stack can publish +clean and still reach the 422. Branch on `code`, and read the status off the response rather +than deriving it from this page. #### `INVALID_FIELD` **HTTP Status:** 400 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 848ebab99d6..4cc1b3e2582 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 @@ -48,6 +48,7 @@ import { matchesFilterCondition } from '@objectstack/formula'; // to the source in the checkout rather than a prebuilt dist. import { validateRecord, assertEngineFindOnePredicate } from '@objectstack/objectql'; import type { PermissionSet } from '@objectstack/spec/security'; +import { ObjectSchema } from '@objectstack/spec/data'; const REP = 'usr_rep'; const OTHER = 'usr_other'; @@ -76,9 +77,9 @@ const CONTACT_SCHEMA = { /** * [#7474] The same detail, AUTHORED WRONG: `controlled_by_parent` with nothing - * to derive access from — no master_detail field, and no required lookup for - * `resolveCbpRelation` to fall back to. The gate's metadata-defect leg is the - * only thing between this object and a write that answers a false 403. + * to derive access from — no master_detail field at all. The gate's + * metadata-defect leg is the only thing between this object and a write that + * answers a false 403. */ const CONTACT_SCHEMA_NO_MASTER_DETAIL = { name: 'crm_contact', @@ -86,25 +87,29 @@ const CONTACT_SCHEMA_NO_MASTER_DETAIL = { fields: { id: { name: 'id', type: 'text' }, name: { name: 'name', type: 'text' }, - // A lookup, but OPTIONAL — the third fallback `resolveCbpRelation` tries - // requires `required: true`, so this object resolves to no relation at all. + // A lookup, and OPTIONAL — no relation at all, before #23100 and after it + // (a required lookup no longer resolves either; see the retired tiers). account: { name: 'account', type: 'lookup', reference: 'crm_account' }, }, }; /** - * [#8688] The three declarations required-field validation does NOT cover, and - * the reason the insert-leg stand-down is conditional rather than flat. + * [#8688] The declarations required-field validation does NOT cover, and the + * reason the insert-leg stand-down is conditional rather than flat. * * `validateRecord` (`@objectstack/objectql`) walks every declared field on an * insert but skips provenance-flagged ones outright — `if (def.system || * def.readonly) continue;` — before the required check, and never fires on a - * field that is not `required` at all. All three shapes below are AUTHORABLE: - * `resolveCbpRelation`'s second fallback accepts a `master_detail` with no - * `required` test, and nothing at runtime refuses `readonly`/`system` on one. - * On each of them this gate is the only refusal an absent master FK meets, so - * each must still answer 422 — the suite asserts that against the REAL - * validator rather than against a belief about it. + * field that is not `required` at all. The `readonly` / `system` shapes below + * are AUTHORABLE (nothing at runtime refuses either flag on a required + * `master_detail`), so on each of them this gate is the only refusal an absent + * master FK meets, and each must still answer 422 — the suite asserts that + * against the REAL validator rather than against a belief about it. + * + * [#23100] The first shape, a `master_detail` with no `required`, was the third + * member of this set while `resolveCbpRelation`'s second fallback resolved it. + * Ruling B retired that fallback, so it resolves NO relation now and meets the + * metadata-defect leg instead — pinned with the other retired tier below. */ const CONTACT_SCHEMA_MASTER_NOT_REQUIRED = { name: 'crm_contact', @@ -141,10 +146,10 @@ const CONTACT_SCHEMA_MASTER_SYSTEM = { }; /** - * [#8688] `controlled_by_parent` with no `master_detail` but a REQUIRED lookup - * — `resolveCbpRelation`'s third fallback, and the one shape where the gate's - * answer is narrower than validation strictly requires. See the case that pins - * it for why that is deliberate. + * [#8688 → #23100] `controlled_by_parent` with no `master_detail` but a + * REQUIRED lookup — `resolveCbpRelation`'s third fallback until ruling B + * retired it, and HotCRM's `crm_campaign_member` / `crm_event_attendee` / + * `crm_article_feedback` shape. It resolves no relation now. */ const CONTACT_SCHEMA_MASTER_REQUIRED_LOOKUP = { name: 'crm_contact', @@ -161,6 +166,22 @@ const CONTACT_SCHEMA_MASTER_REQUIRED_LOOKUP = { * selector. `valid` is the shape the ruling covers; the rest are the shapes it * deliberately does not. */ +/** + * [#23100] Two REQUIRED master_details: the one tie left after the retired + * tiers. The runtime used to take the first by field order; ADR-0055 §1 says + * exactly one, so it derives none. + */ +const CONTACT_SCHEMA_TWO_MASTERS = { + 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: true, reference: 'crm_account' }, + billing_account: { name: 'billing_account', type: 'master_detail', required: true, reference: 'crm_account' }, + }, +}; + const DETAIL_SCHEMAS = { 'valid': CONTACT_SCHEMA, 'no-master-detail': CONTACT_SCHEMA_NO_MASTER_DETAIL, @@ -168,6 +189,7 @@ const DETAIL_SCHEMAS = { 'master-readonly': CONTACT_SCHEMA_MASTER_READONLY, 'master-system': CONTACT_SCHEMA_MASTER_SYSTEM, 'master-required-lookup': CONTACT_SCHEMA_MASTER_REQUIRED_LOOKUP, + 'two-masters': CONTACT_SCHEMA_TWO_MASTERS, } as const; type DetailVariant = keyof typeof DETAIL_SCHEMAS; @@ -301,9 +323,11 @@ interface BootOptions { * DEFECT the metadata-defect leg exists to report: `controlled_by_parent` * declared on an object with no relation to derive access from. * - * [#8688] `'master-not-required'` / `'master-readonly'` / `'master-system'` - * swap in the three master-reference declarations required-field validation - * does not cover — see {@link DETAIL_SCHEMAS}. + * [#8688] `'master-readonly'` / `'master-system'` swap in the + * master-reference declarations required-field validation does not cover — + * see {@link DETAIL_SCHEMAS}. [#23100] `'master-not-required'`, + * `'master-required-lookup'` and `'two-masters'` are the shapes ruling B + * stopped resolving. */ detail?: DetailVariant; /** [#7474] Replace the detail rows — e.g. one whose master FK is null. */ @@ -900,7 +924,6 @@ describe('[#7474] the six refusal legs answer with six envelopes, not one', () = * refuses, AND the real validator raises nothing, which is why it must. */ it.each([ - ['a master_detail with no `required`', 'master-not-required' as const, CONTACT_SCHEMA_MASTER_NOT_REQUIRED], ['`required` + `readonly`', 'master-readonly' as const, CONTACT_SCHEMA_MASTER_READONLY], ['`required` + `system`', 'master-system' as const, CONTACT_SCHEMA_MASTER_SYSTEM], ])('[#8688] 422 MISSING_REQUIRED_FIELD survives on insert: %s', async (_label, detail, schema) => { @@ -919,21 +942,6 @@ describe('[#7474] the six refusal legs answer with six envelopes, not one', () = expect(err.message).not.toContain('requires edit access to its master record'); }); - it('[#8688] 422 survives on insert when the relation resolves through the required-LOOKUP fallback', async () => { - // A deliberate narrowing, recorded so it is not "simplified" away later. - // `validateRecord` WOULD refuse this omission (the field is required and - // carries no provenance flag), but the ruling covers `master_detail` only, - // and `resolveCbpRelation`'s third fallback is a different shape — an - // object that declares `controlled_by_parent` with no master_detail at all. - // Standing down there would be the implementer widening a ruling; keeping - // the gate is the fail-closed direction and costs one residual envelope. - const h = await boot({ shareLevel: 'edit', detail: 'master-required-lookup' }); - const err = await refusalOf(h.insertContact({ name: 'New contact' })); - expect(err.code).toBe('MISSING_REQUIRED_FIELD'); - expect(err.status).toBe(422); - expect(err.message).toContain("did not supply 'account'"); - }); - it('[#8688] the master-access verdict SURVIVES the stand-down: present but not writable is still 403', async () => { // The ruling requires this pinned as surviving. The stand-down is keyed on // the FK being ABSENT; a supplied master still runs every access leg, so @@ -986,6 +994,87 @@ describe('[#7474] the six refusal legs answer with six envelopes, not one', () = // --------------------------------------------------------------------------- +/** + * [ADR-0055 §1 · #23100, maintainer ruling B] The runtime derives a master from + * EXACTLY ONE required `master_detail` and from nothing else. Its second tier + * (any `master_detail`) and third tier (a required `lookup`) retired, and two + * required `master_detail` fields are no longer resolved by field order — so + * each of those three shapes now meets BOTH fail-closed halves the no-relation + * object always met: every read denied, every by-id write and every insert + * refused with `422 INVALID_METADATA`, and the object reported once. + * + * Each shape is also held against `ObjectSchema`'s parse, which refuses it with + * the same rule — the card behind the ruling was precisely the parse, the lint + * and this gate disagreeing about which objects the rule covers. + */ +describe('[#23100] the retired master tiers derive NO master — fail-closed on both faces', () => { + const refusalOf = async (run: Promise): Promise => { + try { + await run; + } catch (e) { + return e; + } + throw new Error('expected the write to be refused, but it resolved'); + }; + + /** The parse's controlled_by_parent issues on one schema, by path. */ + const cbpParseIssuePaths = (schema: Record) => { + const parsed = ObjectSchema.safeParse(schema); + if (parsed.success) return []; + return parsed.error.issues + .filter((i) => /declares sharingModel 'controlled_by_parent'/.test(i.message)) + .map((i) => i.path.join('.')); + }; + + const unresolvedReports = (h: Awaited>) => + (h.ctx.logger.warn as ReturnType).mock.calls.filter(([msg]) => + String(msg).startsWith('[security/controlled-by-parent] object "crm_contact"'), + ); + + // ── ACCEPTED CONTROL — the one tier ADR-0055 §1 records ────────────────── + it('CONTROL: one required master_detail derives — rows readable, writes reach the executor, nothing reported', async () => { + const h = await boot({ shareLevel: 'edit' }); + expect(await h.visibleContacts()).toEqual(['ct_us', 'ct_own']); + await expect(h.updateContact('ct_own')).resolves.toBeUndefined(); + await expect(h.insertContact({ name: 'New contact', account: 'acct_own' })).resolves.toBe(true); + expect(unresolvedReports(h)).toEqual([]); + expect(cbpParseIssuePaths(CONTACT_SCHEMA)).toEqual([]); + }); + + 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) => { + const h = await boot({ shareLevel: 'edit', detail }); + + // READ: the master the old tier would have picked holds readable rows + // (the control above reads two of them) — none survive now. + expect(await h.visibleContacts()).toEqual([]); + expect(await h.analyticsVisibleContacts()).toEqual([]); + + // WRITE: a by-id update under a master the caller CAN edit, and an insert + // naming one — the metadata-defect leg, not an authorization verdict, and + // not the null-FK leg the second tier used to reach. + for (const run of [h.updateContact('ct_own'), h.insertContact({ name: 'New contact', account: 'acct_own' })]) { + const err = await refusalOf(run); + expect(err.code).toBe('INVALID_METADATA'); + expect(err.status).toBe(422); + expect(err.message).toMatch(/^\[Security\] Configuration defect: (update|insert) on 'crm_contact' cannot be authorized because/); + } + + // REPORTED, once per object per cache generation — not once per request. + const reports = unresolvedReports(h); + 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]); + }); +}); + +// --------------------------------------------------------------------------- + /** * [#7505] The FIFTH condition the by-id gate can meet, and the one it used to * answer with somebody else's envelope: the store could not be read at all. diff --git a/packages/verify/src/parent-write-reach.test.ts b/packages/verify/src/parent-write-reach.test.ts index fbe87f73169..67799fa9301 100644 --- a/packages/verify/src/parent-write-reach.test.ts +++ b/packages/verify/src/parent-write-reach.test.ts @@ -67,7 +67,7 @@ const source = { label: 'Contract', sharingModel: 'controlled_by_parent', fields: { - account: { type: 'master_detail', label: 'Account', reference: 'pwr_account' }, + account: { type: 'master_detail', required: true, label: 'Account', reference: 'pwr_account' }, // A second required reference, to an object the personas hold no grant on. signer: { type: 'lookup', label: 'Signer', reference: 'pwr_person', required: true }, title: { type: 'text', label: 'Title' }, @@ -77,14 +77,14 @@ const source = { name: 'pwr_quote', label: 'Quote', sharingModel: 'controlled_by_parent', - fields: { account: { type: 'master_detail', label: 'Account', reference: 'pwr_account' }, title: { type: 'text', label: 'Title' } }, + fields: { account: { type: 'master_detail', required: true, label: 'Account', reference: 'pwr_account' }, title: { type: 'text', label: 'Title' } }, }, { // Two levels: a line's master is a quote, whose own master is an account. name: 'pwr_quote_line', label: 'Quote line', sharingModel: 'controlled_by_parent', - fields: { quote: { type: 'master_detail', label: 'Quote', reference: 'pwr_quote' }, title: { type: 'text', label: 'Title' } }, + fields: { quote: { type: 'master_detail', required: true, label: 'Quote', reference: 'pwr_quote' }, title: { type: 'text', label: 'Title' } }, }, ], permissions: [manager, rep], @@ -130,7 +130,7 @@ describe('deriveParentWriteReachCases — what the family reads from the metadat const deep = Array.from({ length: 10 }, (_, i) => ({ name: `pwr_deep_${i}`, sharingModel: 'controlled_by_parent', - fields: { up: { type: 'master_detail', reference: i === 9 ? 'pwr_account' : `pwr_deep_${i + 1}` } }, + fields: { up: { type: 'master_detail', required: true, reference: i === 9 ? 'pwr_account' : `pwr_deep_${i + 1}` } }, })); const config = { objects: [ @@ -140,17 +140,17 @@ describe('deriveParentWriteReachCases — what the family reads from the metadat name: 'pwr_two_masters', sharingModel: 'controlled_by_parent', fields: { - account: { type: 'master_detail', reference: 'pwr_account' }, - person: { type: 'master_detail', reference: 'pwr_person' }, + account: { type: 'master_detail', required: true, reference: 'pwr_account' }, + person: { type: 'master_detail', required: true, reference: 'pwr_person' }, }, }, { name: 'pwr_no_master', sharingModel: 'controlled_by_parent', fields: { account: { type: 'lookup', reference: 'pwr_account', required: true } } }, - { name: 'pwr_outside', sharingModel: 'controlled_by_parent', fields: { owner: { type: 'master_detail', reference: 'crm_elsewhere' } } }, - { name: 'pwr_loop_a', sharingModel: 'controlled_by_parent', fields: { b: { type: 'master_detail', reference: 'pwr_loop_b' } } }, - { name: 'pwr_loop_b', sharingModel: 'controlled_by_parent', fields: { a: { type: 'master_detail', reference: 'pwr_loop_a' } } }, + { name: 'pwr_outside', sharingModel: 'controlled_by_parent', fields: { owner: { type: 'master_detail', required: true, reference: 'crm_elsewhere' } } }, + { name: 'pwr_loop_a', sharingModel: 'controlled_by_parent', fields: { b: { type: 'master_detail', required: true, reference: 'pwr_loop_b' } } }, + { name: 'pwr_loop_b', sharingModel: 'controlled_by_parent', fields: { a: { type: 'master_detail', required: true, reference: 'pwr_loop_a' } } }, { name: 'pwr_badge', label: 'Badge', fields: { photo: { type: 'image', required: true } } }, - { name: 'pwr_badge_line', sharingModel: 'controlled_by_parent', fields: { badge: { type: 'master_detail', reference: 'pwr_badge' } } }, - { name: 'pwr_bad', sharingModel: 'controlled_by_parent', fields: { account: { type: 'master_detail', reference: 'pwr_account', bogus: true } } }, + { name: 'pwr_badge_line', sharingModel: 'controlled_by_parent', fields: { badge: { type: 'master_detail', required: true, reference: 'pwr_badge' } } }, + { name: 'pwr_bad', sharingModel: 'controlled_by_parent', fields: { account: { type: 'master_detail', required: true, reference: 'pwr_account', bogus: true } } }, ...deep, ], }; @@ -171,8 +171,15 @@ describe('deriveParentWriteReachCases — what the family reads from the metadat ], }); // Named instead of probed. - expect(byObject.get('pwr_two_masters')!.excluded).toMatch(/^pwr_two_masters declares 2 master_detail fields \(account, person\)/); - expect(byObject.get('pwr_no_master')!.excluded).toMatch(/^pwr_no_master declares no master_detail field/); + // [#23100] ADR-0055 §1 is the parse's rule now: two required masters and a + // master derived through a required lookup are refused by `ObjectSchema` + // itself, so the family names the parse's refusal rather than its own. + expect(byObject.get('pwr_two_masters')!.excluded).toMatch( + /^the object does not parse under the spec's ObjectSchema \(fields: Object 'pwr_two_masters' declares sharingModel 'controlled_by_parent' with 2 required `master_detail` fields/, + ); + expect(byObject.get('pwr_no_master')!.excluded).toMatch( + /^the object does not parse under the spec's ObjectSchema \(sharingModel: Object 'pwr_no_master' declares sharingModel 'controlled_by_parent' but no required `master_detail` field/, + ); expect(byObject.get('pwr_outside')!.excluded).toMatch(/^the master crm_elsewhere is not an object this app declares/); expect(byObject.get('pwr_loop_a')!.excluded).toMatch(/^the master chain re-enters pwr_loop_a \(a cycle\)/); expect(byObject.get('pwr_deep_0')!.excluded).toMatch(/^the master chain reaches 8 controlled_by_parent masters, the write gate's depth bound/); @@ -306,20 +313,20 @@ const controlSource = { name: 'pwc_signed', label: 'Signed', sharingModel: 'controlled_by_parent', - fields: { account: { type: 'master_detail', label: 'Account', reference: 'pwc_account' }, title: { type: 'text', label: 'Title' } }, + fields: { account: { type: 'master_detail', required: true, label: 'Account', reference: 'pwc_account' }, title: { type: 'text', label: 'Title' } }, }, { name: 'pwc_locked', label: 'Locked', sharingModel: 'controlled_by_parent', - fields: { account: { type: 'master_detail', label: 'Account', reference: 'pwc_account' }, title: { type: 'text', label: 'Title' } }, + fields: { account: { type: 'master_detail', required: true, label: 'Account', reference: 'pwc_account' }, title: { type: 'text', label: 'Title' } }, }, { name: 'pwc_frozen', label: 'Frozen', sharingModel: 'private', fields: { name: { type: 'text', label: 'Name', required: true } } }, { name: 'pwc_frozen_line', label: 'Frozen line', sharingModel: 'controlled_by_parent', - fields: { frozen: { type: 'master_detail', label: 'Frozen', reference: 'pwc_frozen' }, title: { type: 'text', label: 'Title' } }, + fields: { frozen: { type: 'master_detail', required: true, label: 'Frozen', reference: 'pwc_frozen' }, title: { type: 'text', label: 'Title' } }, }, ], hooks: [ From 810a155ddcee8ec603c6dd63705583486ace2134 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 02:25:13 +0000 Subject: [PATCH 06/11] wip: changeset; plugin-security 422 message pin follows the rule Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H Co-authored-by: Claude --- .changeset/23100-cbp-one-required-master.md | 37 +++++++++++++++++++ .../src/controlled-by-parent-sharing.test.ts | 6 ++- 2 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 .changeset/23100-cbp-one-required-master.md diff --git a/.changeset/23100-cbp-one-required-master.md b/.changeset/23100-cbp-one-required-master.md new file mode 100644 index 00000000000..3855284d410 --- /dev/null +++ b/.changeset/23100-cbp-one-required-master.md @@ -0,0 +1,37 @@ +--- +"@objectstack/spec": minor +"@objectstack/lint": minor +"@objectstack/plugin-security": minor +--- + +feat(spec)!: a `controlled_by_parent` object derives its master from exactly one required `master_detail` — the parse refuses anything else, and the runtime's two fallback tiers retire (#23100) + +Clause-②: no (narrowing) + +ADR-0055 §1 says a `controlled_by_parent` object "must declare exactly one required `master_detail` field … Validation error otherwise (fail closed)". Until now neither the parse nor the runtime kept it. `ObjectSchema` accepted such an object with no master at all. The security runtime derived a master through a three-tier fallback the record never stated: a required `master_detail`, then any `master_detail`, then a required `lookup`, taking the first match by field order within a tier. The maintainer ruling (letter B) enforces the record, under ADR-0049 (enforce-or-remove). + +**BREAKING — what moves for consumers.** + +- **`@objectstack/spec`.** `ObjectSchema` (raw `.parse()` / `.safeParse()`, `ObjectSchema.create()`, `defineStack`, `os validate` and the metadata save door) refuses an object declaring `sharingModel: 'controlled_by_parent'` unless exactly one of its fields is a `master_detail` declared `required: true`. With none, the issue sits at `sharingModel` and names each field that relied on a retired tier. With two or more, it sits at `fields` and names them. A plain object literal is judged like a raw parse: an omitted `required` on its `master_detail` is refused, while `ObjectSchema.create()` keeps forcing an omitted `required` to `true`. The step-18 semantic migration entry `cbp-master-relation-tiers-retired` carries the prescription for `os migrate meta`. +- **`@objectstack/plugin-security`.** The relation resolver derives a master from that one field and nothing else. An object that declares a non-required `master_detail` only, a required `lookup` only, or two required `master_detail` fields resolves no master. Every read of it is denied, and every write and insert is refused with `422 INVALID_METADATA`. The plugin reports such an object once per object (`[security/controlled-by-parent] … resolves no master relation`), naming the shape it relied on. The `422` message now says "not exactly one required master_detail field". +- **`@objectstack/lint`.** `security-controlled-by-parent-no-relation` now refuses the non-required `master_detail` and required `lookup` shapes as well. `security-controlled-by-parent-ambiguous-relation` reports two or more required `master_detail` fields as "no master is derived", since the runtime no longer picks one by field order. Two required `lookup` fields are now a no-relation object, not a tie. `security-master-detail-ungranted` no longer treats a lookup-bound `controlled_by_parent` object as a detail. + +**FROM → TO — HotCRM's three objects.** These are the only producers measured on the retired third tier: the read-only HotCRM clone at `0f84c9c`, 8 `controlled_by_parent` objects, 5 of them already on one required `master_detail`. HotCRM's pinned engine is unaffected until it bumps past the release that carries this change. + +| object | FROM | TO | +|---|---|---| +| `crm_campaign_member` | `crm_campaign: Field.lookup('crm_campaign', { required: true })` | `crm_campaign: Field.masterDetail('crm_campaign', { required: true, deleteBehavior: 'restrict' })` | +| `crm_event_attendee` | `crm_event: Field.lookup('crm_event', { required: true })` | `crm_event: Field.masterDetail('crm_event', { required: true, deleteBehavior: 'restrict' })` | +| `crm_article_feedback` | `crm_knowledge_article: Field.lookup('crm_knowledge_article', { required: true, deleteBehavior: 'cascade' })` | `crm_knowledge_article: Field.masterDetail('crm_knowledge_article', { required: true, deleteBehavior: 'cascade' })` | + +`deleteBehavior: 'restrict'` keeps what the two lookups do today: a campaign or an event with members or attendees cannot be deleted. Without it a `master_detail` cascades, and a detail's rows are deleted with their master. An object whose rows must outlive their parent is not a detail. It leaves `controlled_by_parent` for `private`, `public_read` or `public_read_write`. + +**Remedy, in one line.** On every object with `sharingModel: 'controlled_by_parent'`, declare exactly one `master_detail` field with `required: true`, pointing at the master. Every other parent is a `lookup`. + +**Unchanged.** + +- An object with one required `master_detail` resolves and derives exactly as before, chain walk included. So does a required `master_detail` beside other `lookup` fields, or beside a `master_detail` that is not required. Every in-tree `controlled_by_parent` object is in that set: `examples/app-crm` (1), `examples/app-showcase` (2), the CLI's eval corpus (4) and the dogfood fixtures. +- Stored metadata is not rewritten. A stored row in a retired shape still registers, under its `[metadata_spec_invalid]` diagnostic, and is denied until it is corrected and saved. No conversion is offered: turning a `lookup` into a `master_detail` changes what deleting the master does to every row, and leaving `controlled_by_parent` is the author's decision. +- No export, signature or error code is added or removed. + + 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 4cc1b3e2582..0466fda8c59 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 @@ -796,8 +796,10 @@ describe('[#7474] the six refusal legs answer with six envelopes, not one', () = expect(err.statusCode).toBe(422); // The remedy has to be IN the message: `details` is not a carrier the // client can rely on (#7450), so the sentence is the whole fix-it. - expect(err.message).toContain('no master_detail relation'); - expect(err.message).toContain('Declare a required master_detail field'); + // [#23100] Reworded with the rule: "no relation" is now "not exactly one + // required master_detail" (ADR-0055 §1), and the remedy says so. + expect(err.message).toContain('not exactly one required master_detail field'); + expect(err.message).toContain('Declare one master_detail field with required: true'); // …and it must NOT wear the 403's costume: that prefix is a MATCHER at both // transports, so borrowing it would re-flatten this to PERMISSION_DENIED. expect(err.message).not.toContain('[Security] Access denied'); From 21a57d26c62b8c99bc8f88da8adb00ef527cd47a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 02:39:11 +0000 Subject: [PATCH 07/11] wip(spec): type the test helper's sharingModel literal Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H Co-authored-by: Claude --- packages/spec/src/data/object.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/spec/src/data/object.test.ts b/packages/spec/src/data/object.test.ts index 7fc5089039b..f7e0753bb15 100644 --- a/packages/spec/src/data/object.test.ts +++ b/packages/spec/src/data/object.test.ts @@ -1590,7 +1590,8 @@ describe('ObjectSchema.create() forces a required master_detail under controlled // ============================================================================ describe('ObjectSchema refuses a controlled_by_parent object without exactly one required master_detail', () => { - const cbp = (fields: Record) => ({ name: 'cbp_line', sharingModel: 'controlled_by_parent', fields }); + const cbp = >(fields: F) => + ({ name: 'cbp_line', sharingModel: 'controlled_by_parent' as const, fields }); const issuesOf = (input: unknown) => { const r = ObjectSchema.safeParse(input); return r.success ? [] : r.error.issues.map((i) => ({ path: i.path.join('.'), message: i.message })); From 1f9bf4adeaa367a269a48d9853a12622b7b707c1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 02:40:36 +0000 Subject: [PATCH 08/11] =?UTF-8?q?wip:=20anchor=20ADR-0055=20=C2=A71=20on?= =?UTF-8?q?=20the=20parse=20and=20runtime=20seams?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H Co-authored-by: Claude --- ...ges__plugins__plugin-security__src__security-plugin.ts.json | 3 ++- .../adr-anchors/packages__spec__src__data__object.zod.ts.json | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) 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 2762c899a80..44884320ffc 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 @@ -1,8 +1,9 @@ { "file": "packages/plugins/plugin-security/src/security-plugin.ts", "adrs": [ + "ADR-0055", "ADR-0106", "ADR-0123" ], - "invariant": "ADR-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." + "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." } 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 1ba4c2ea3f5..5b081c48b6b 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 @@ -1,7 +1,8 @@ { "file": "packages/spec/src/data/object.zod.ts", "adrs": [ + "ADR-0055", "ADR-0120" ], - "invariant": "`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)." + "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)." } From 7ed362fd20a5af8dd8d2b428f8a68c0f55683145 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 04:50:41 +0000 Subject: [PATCH 09/11] test(verify): the badge-line case names the detail's own block once its master is required Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H Co-authored-by: Claude --- packages/verify/src/parent-write-reach.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/verify/src/parent-write-reach.test.ts b/packages/verify/src/parent-write-reach.test.ts index 231e635bdb1..7c28ff43813 100644 --- a/packages/verify/src/parent-write-reach.test.ts +++ b/packages/verify/src/parent-write-reach.test.ts @@ -187,7 +187,12 @@ describe('deriveParentWriteReachCases — what the family reads from the metadat expect(byObject.get('pwr_deep_2')!.excluded).toBeUndefined(); expect(byObject.get('pwr_deep_2')!.hops).toHaveLength(8); expect(byObject.get('pwr_deep_2')!.hops.at(-1)).toEqual({ object: 'pwr_deep_9', field: 'up', master: 'pwr_account' }); - expect(byObject.get('pwr_badge_line')!.notDerivable).toMatch(/^the master pwr_badge cannot be synthesized: required image field "photo"/); + // [#23100] With its master_detail now `required: true` (the one shape ADR-0055 §1 + // admits), the detail itself needs a synthesized master, so the family names the + // detail's own block — the same unsynthesizable master, reached one hop earlier. + expect(byObject.get('pwr_badge_line')!.notDerivable).toMatch( + /^the detail cannot be synthesized: required relational target "pwr_badge" is not synthesizable/, + ); expect(byObject.get('pwr_bad')!.excluded).toMatch(/^the object does not parse under the spec's ObjectSchema/); }); }); From dbd3338285e2cfb42993c9151e3913e6a0ca9b2a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 06:14:39 +0000 Subject: [PATCH 10/11] feat(spec): D2 conversion for the one lossless one-master shape; step-18 fragment; docs and fixture follow-ups Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H Co-authored-by: Claude --- .changeset/23100-cbp-one-required-master.md | 6 +- content/docs/permissions/authorization.mdx | 11 +- ...ne-unreadable-master-detail-header.test.ts | 9 +- ...p-master-detail-required-defaulted.test.ts | 114 +++++++++++++ packages/spec/src/conversions/registry.ts | 159 ++++++++++++++++++ packages/spec/src/data/object.zod.ts | 22 ++- .../18.cbp-master-relation-tiers-retired.ts | 31 ++-- .../spec/src/migrations/registry.ts.template | 19 +++ 8 files changed, 346 insertions(+), 25 deletions(-) create mode 100644 packages/spec/src/conversions/cbp-master-detail-required-defaulted.test.ts diff --git a/.changeset/23100-cbp-one-required-master.md b/.changeset/23100-cbp-one-required-master.md index 3855284d410..e139e4dc3b6 100644 --- a/.changeset/23100-cbp-one-required-master.md +++ b/.changeset/23100-cbp-one-required-master.md @@ -12,7 +12,7 @@ ADR-0055 §1 says a `controlled_by_parent` object "must declare exactly one requ **BREAKING — what moves for consumers.** -- **`@objectstack/spec`.** `ObjectSchema` (raw `.parse()` / `.safeParse()`, `ObjectSchema.create()`, `defineStack`, `os validate` and the metadata save door) refuses an object declaring `sharingModel: 'controlled_by_parent'` unless exactly one of its fields is a `master_detail` declared `required: true`. With none, the issue sits at `sharingModel` and names each field that relied on a retired tier. With two or more, it sits at `fields` and names them. A plain object literal is judged like a raw parse: an omitted `required` on its `master_detail` is refused, while `ObjectSchema.create()` keeps forcing an omitted `required` to `true`. The step-18 semantic migration entry `cbp-master-relation-tiers-retired` carries the prescription for `os migrate meta`. +- **`@objectstack/spec`.** `ObjectSchema` (raw `.parse()` / `.safeParse()`, `ObjectSchema.create()`, `defineStack`, `os validate` and the metadata save door) refuses an object declaring `sharingModel: 'controlled_by_parent'` unless exactly one of its fields is a `master_detail` declared `required: true`. With none, the issue sits at `sharingModel` and names each field that relied on a retired tier. With two or more, it sits at `fields` and names them. A plain object literal is judged like a raw parse: an omitted `required` on its `master_detail` is refused, while `ObjectSchema.create()` keeps forcing an omitted `required` to `true`. The step-18 semantic migration entry `cbp-master-relation-tiers-retired` carries the prescription for `os migrate meta`. The step-18 conversion `cbp-master-detail-required-defaulted` covers the one lossless shape: a `controlled_by_parent` object whose one `master_detail` omits `required`, which Studio's field designer writes. It gets `required: true`, exactly what `ObjectSchema.create()` does. It runs on stored rows, built artifacts and `os migrate meta --from 17`. A live authoring parse still refuses the shape and names the fix. - **`@objectstack/plugin-security`.** The relation resolver derives a master from that one field and nothing else. An object that declares a non-required `master_detail` only, a required `lookup` only, or two required `master_detail` fields resolves no master. Every read of it is denied, and every write and insert is refused with `422 INVALID_METADATA`. The plugin reports such an object once per object (`[security/controlled-by-parent] … resolves no master relation`), naming the shape it relied on. The `422` message now says "not exactly one required master_detail field". - **`@objectstack/lint`.** `security-controlled-by-parent-no-relation` now refuses the non-required `master_detail` and required `lookup` shapes as well. `security-controlled-by-parent-ambiguous-relation` reports two or more required `master_detail` fields as "no master is derived", since the runtime no longer picks one by field order. Two required `lookup` fields are now a no-relation object, not a tie. `security-master-detail-ungranted` no longer treats a lookup-bound `controlled_by_parent` object as a detail. @@ -31,7 +31,7 @@ ADR-0055 §1 says a `controlled_by_parent` object "must declare exactly one requ **Unchanged.** - An object with one required `master_detail` resolves and derives exactly as before, chain walk included. So does a required `master_detail` beside other `lookup` fields, or beside a `master_detail` that is not required. Every in-tree `controlled_by_parent` object is in that set: `examples/app-crm` (1), `examples/app-showcase` (2), the CLI's eval corpus (4) and the dogfood fixtures. -- Stored metadata is not rewritten. A stored row in a retired shape still registers, under its `[metadata_spec_invalid]` diagnostic, and is denied until it is corrected and saved. No conversion is offered: turning a `lookup` into a `master_detail` changes what deleting the master does to every row, and leaving `controlled_by_parent` is the author's decision. +- Stored metadata whose one `master_detail` merely omits `required` keeps serving, converted as above. A stored row in any other retired shape is not rewritten: it still registers, under its `[metadata_spec_invalid]` diagnostic, and is denied until it is corrected and saved. Those shapes are an explicit `required: false`, two masters, or a required `lookup`. None of them is converted: turning a `lookup` into a `master_detail` changes what deleting the master does to every row, choosing between two masters is the pick ADR-0055 §1 refuses, and leaving `controlled_by_parent` is the author's decision. - No export, signature or error code is added or removed. - + diff --git a/content/docs/permissions/authorization.mdx b/content/docs/permissions/authorization.mdx index bccf3c94995..86391cff932 100644 --- a/content/docs/permissions/authorization.mdx +++ b/content/docs/permissions/authorization.mdx @@ -517,10 +517,13 @@ Five mechanisms — four CI-time, one runtime — make the security posture a wider than internal (`security-external-wider-than-internal`), `'*'` wildcards carrying View/Modify All outside the platform admin set, high-privilege `isDefault` (everyone-suggested) sets, the reserved word - "role" in security identifiers, a `controlled_by_parent` object with no - relation the platform can derive access from (ADR-0055: no required - `master_detail`, no `master_detail` at all, and no required `lookup` — so - the runtime denies every read and refuses every write), and the ADR-0091 + "role" in security identifiers, a `controlled_by_parent` object without + exactly one required `master_detail` to derive access from (ADR-0055 §1: + `security-controlled-by-parent-no-relation` when it declares none — a + `master_detail` without `required: true` and a required `lookup` derive + nothing — and `security-controlled-by-parent-ambiguous-relation` when it + declares two or more; either way the runtime denies every read and refuses + every write), and the ADR-0091 grant-lifecycle rules (a seed grant already expired at authoring time; a delegation row missing its mandatory `reason`) — every error rule mirrors a runtime gate. diff --git a/packages/objectql/src/engine-unreadable-master-detail-header.test.ts b/packages/objectql/src/engine-unreadable-master-detail-header.test.ts index 7a1521445f9..4356f37379d 100644 --- a/packages/objectql/src/engine-unreadable-master-detail-header.test.ts +++ b/packages/objectql/src/engine-unreadable-master-detail-header.test.ts @@ -104,12 +104,19 @@ const RO_ITEM_OBJECT = { }, }; -/** The ADR-0055 §3 detail: the same rule, and the same answers. */ +/** + * The ADR-0055 §3 detail: the same rule, and the same answers. Its one + * `master_detail` is declared `required: true` — the only master a + * `controlled_by_parent` object may declare (ADR-0055 §1, refused at parse + * otherwise since #23100). The flag is set on THIS object only, so the plain, + * requiredWhen and readonlyWhen fixtures keep the reference exactly as before. + */ const CBP_LINE_OBJECT = { ...REQ_LINE_OBJECT, name: CBP_LINE, label: 'Controlled line', sharingModel: 'controlled_by_parent', + fields: { ...REQ_LINE_OBJECT.fields, invoice: { ...invoiceField, required: true } }, }; /** A plain `lookup` to the same object: its probe is not this rule's. */ 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 new file mode 100644 index 00000000000..0ee223452a6 --- /dev/null +++ b/packages/spec/src/conversions/cbp-master-detail-required-defaulted.test.ts @@ -0,0 +1,114 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, expect, it } from 'vitest'; + +import { ObjectSchema } from '../data/object.zod.js'; +import { defineStack } from '../stack.zod.js'; +import { ALL_CONVERSIONS } from './registry.js'; +import { applyConversionsToStoredItem } from './stored.js'; +import type { ConversionNotice } from './types.js'; + +/** + * [#23100] `cbp-master-detail-required-defaulted`: the one lossless shape inside + * ADR-0055 §1's one-master refusal, carried for data at rest. + * + * The fixture pair in `conversions.test.ts` proves before → after over the whole + * table. This file pins it where it matters: + * - on a stored row in the shape objectui's Studio writes (`{ type, label }`, + * 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. + */ + +const ID = 'cbp-master-detail-required-defaulted'; + +const cbpRow = (fields: Record) => ({ + name: 'crm_line', + label: 'Line', + sharingModel: 'controlled_by_parent', + fields, +}); +const STUDIO_MASTER = { type: 'master_detail', label: 'Opportunity', reference: 'crm_opportunity' }; + +function convertStored(row: Record) { + const notices: ConversionNotice[] = []; + const item = applyConversionsToStoredItem('object', row, { onNotice: (n) => notices.push(n) }); + return { item: item as Record, notices: notices.filter((n) => n.conversionId === ID) }; +} + +/** The parse's `controlled_by_parent` issues on one object, by path. */ +function cbpIssuePaths(row: unknown): string[] { + const r = ObjectSchema.safeParse(row); + if (r.success) return []; + return r.error.issues + .filter((i) => /declares sharingModel 'controlled_by_parent'/.test(i.message)) + .map((i) => i.path.join('.')); +} + +describe('cbp-master-detail-required-defaulted (ADR-0087 D2)', () => { + it('is registered for protocol 18 and retired from the authoring load path', () => { + const entry = ALL_CONVERSIONS.find((c) => c.id === ID); + expect(entry, 'the conversion is registered').toBeDefined(); + expect(entry!.toMajor).toBe(18); + expect(entry!.retiredFromLoadPath).toBe(true); + }); + + 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']); + + const { item, notices } = convertStored(row); + expect(notices).toHaveLength(1); + expect(notices[0].path).toBe('objects[0].fields.opportunity.required'); + expect(item.fields.opportunity).toEqual({ ...STUDIO_MASTER, required: true }); + expect(item.fields.name).toEqual({ type: 'text', label: 'Name' }); + expect(ObjectSchema.safeParse(item).success).toBe(true); + // Copy-on-write: the stored row itself is not mutated. + expect('required' in (row.fields.opportunity as object)).toBe(false); + + // Idempotent: a second pass finds nothing to convert. + expect(convertStored(item).notices).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' } }, + 'sharingModel', + ], + [ + 'a required lookup (the retired third tier)', + { m: { type: 'lookup', label: 'Campaign', reference: 'crm_campaign', required: true } }, + 'sharingModel', + ], + ])('leaves %s untouched, and the parse still refuses it', (_label, fields, path) => { + const row = cbpRow(fields); + const { item, notices } = convertStored(row); + expect(notices).toEqual([]); + expect(item).toEqual(row); + expect(cbpIssuePaths(item)).toEqual([path]); + }); + + it('touches no other sharing model', () => { + const row = { ...cbpRow({ m: STUDIO_MASTER }), sharingModel: 'private' }; + const { item, notices } = convertStored(row); + expect(notices).toEqual([]); + 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/); + }); +}); diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index 96d69e4b513..0c52c284bfc 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -13200,6 +13200,164 @@ const pageAssignedProfilesRemoved: MetadataConversion = { }, }; +/** + * The one `master_detail` of a `controlled_by_parent` object with `required` + * OMITTED → `required: true` (protocol 18, #23100 — ADR-0055 §1, maintainer + * ruling B; the data-at-rest half of the D3 semantic entry + * `cbp-master-relation-tiers-retired`). + * + * Ruling B made the parse refuse a `controlled_by_parent` object without + * exactly one `master_detail` declared `required: true`, and retired the + * security runtime's second tier, which derived the master from ANY + * `master_detail`. One shape inside that refusal has a producer at rest: + * objectui's Studio adds a field as `{ type, label }` and its Required toggle + * writes `required: true` or nothing, and the object's Settings tab offers + * `controlled_by_parent`. So a Studio-authored detail can be stored with its + * one master reference carrying no `required` key at all. Before protocol 18 + * that row loaded and derived its master through the second tier; after it, the + * row would register under `[metadata_spec_invalid]` and be denied every read. + * + * **Lossless — this is the builder's own force, applied to data at rest.** + * `ObjectSchema.create()` already turns an omitted `required` on every + * `master_detail` under this sharing model into `required: true` + * (`forceCbpMasterDetailRequired`, `data/object.zod.ts`), so the converted row + * is exactly the object the builder would have produced from the same source. + * The relation it resolves is the one the second tier resolved, and the write + * the row could not legally take (an insert without its master, an update + * nulling it) was refused before as well, by the security gate. + * + * **Deliberately narrow — exactly one shape, nothing else.** + * - An explicit `required: false` is NOT converted. The builder refuses it + * loudly rather than forcing it (an authored contradiction, ADR-0032), so the + * stored pass does not overrule an author the builder would have stopped. + * - An object with two or more `master_detail` fields is NOT converted. Choosing + * which reference becomes the master is exactly the pick ADR-0055 §1 refuses. + * - A required `lookup` (the retired third tier) is NOT converted. Turning it + * into a `master_detail` changes what deleting the master does to every row. + * - Any other sharing model is NOT touched. The force is this sharing model's + * rule only. + * + * **Retired from the load path, from day one.** The authoring funnel does not + * replay it, so a live author still meets the parse's refusal and its + * prescription. A stored row, a built artifact inside its declared-floor window + * and `os migrate meta --from 17` all replay it. Idempotent by construction: a + * converted field carries `required`, which this transform never touches again. + */ +const cbpMasterDetailRequiredDefaulted: MetadataConversion = { + id: 'cbp-master-detail-required-defaulted', + toMajor: 18, + retiredFromLoadPath: true, + retiredAfter: '17.7.0', + surface: "object.fields.MASTER.required, MASTER being the one master_detail of a sharingModel: 'controlled_by_parent' object", + summary: + "the one `master_detail` of a `controlled_by_parent` object with `required` omitted → `required: true` " + + '(ADR-0055 §1: the master is exactly one required master_detail; this is the force ' + + '`ObjectSchema.create()` already applies, so the row resolves the same master it always did; ' + + 'an explicit `required: false`, two masters and a required lookup are not converted)', + apply(stack, emit) { + return mapCollection(stack, 'objects', (obj, path) => { + if (obj.sharingModel !== 'controlled_by_parent') return obj; + const fields = obj.fields; + if (!isDict(fields)) return obj; + const masters = Object.entries(fields).filter(([, def]) => isDict(def) && def.type === 'master_detail'); + if (masters.length !== 1) return obj; + const [name, def] = masters[0] as [string, Dict]; + if ('required' in def) return obj; + emit({ from: '(omitted)', to: 'true', path: `${path}.fields.${name}.required` }); + return { ...obj, fields: { ...fields, [name]: { ...def, required: true } } }; + }); + }, + fixture: { + before: { + objects: [ + { + // The Studio-authored shape: converted. + name: 'cbpd_line', + label: 'Line', + sharingModel: 'controlled_by_parent', + fields: { + header: { type: 'master_detail', label: 'Header', reference: 'cbpd_header' }, + note: { type: 'text', label: 'Note' }, + }, + }, + { + // An explicit `required: false` — refused by the builder, not converted here. + name: 'cbpd_explicit', + label: 'Explicit', + sharingModel: 'controlled_by_parent', + fields: { header: { type: 'master_detail', label: 'Header', reference: 'cbpd_header', required: false } }, + }, + { + // Two masters — no pick. + name: 'cbpd_two', + label: 'Two', + sharingModel: 'controlled_by_parent', + fields: { + a: { type: 'master_detail', label: 'A', reference: 'cbpd_header' }, + b: { type: 'master_detail', label: 'B', reference: 'cbpd_other' }, + }, + }, + { + // The retired third tier — a required lookup — is not converted. + name: 'cbpd_member', + label: 'Member', + sharingModel: 'controlled_by_parent', + fields: { header: { type: 'lookup', label: 'Header', reference: 'cbpd_header', required: true } }, + }, + { + // Not controlled_by_parent: untouched. + name: 'cbpd_plain', + label: 'Plain', + sharingModel: 'private', + fields: { header: { type: 'master_detail', label: 'Header', reference: 'cbpd_header' } }, + }, + ], + }, + after: { + objects: [ + { + name: 'cbpd_line', + label: 'Line', + sharingModel: 'controlled_by_parent', + fields: { + header: { type: 'master_detail', label: 'Header', reference: 'cbpd_header', required: true }, + note: { type: 'text', label: 'Note' }, + }, + }, + { + name: 'cbpd_explicit', + label: 'Explicit', + sharingModel: 'controlled_by_parent', + fields: { header: { type: 'master_detail', label: 'Header', reference: 'cbpd_header', required: false } }, + }, + { + name: 'cbpd_two', + label: 'Two', + sharingModel: 'controlled_by_parent', + fields: { + a: { type: 'master_detail', label: 'A', reference: 'cbpd_header' }, + b: { type: 'master_detail', label: 'B', reference: 'cbpd_other' }, + }, + }, + { + name: 'cbpd_member', + label: 'Member', + sharingModel: 'controlled_by_parent', + fields: { header: { type: 'lookup', label: 'Header', reference: 'cbpd_header', required: true } }, + }, + { + name: 'cbpd_plain', + label: 'Plain', + sharingModel: 'private', + fields: { header: { type: 'master_detail', label: 'Header', reference: 'cbpd_header' } }, + }, + ], + }, + // One notice: `cbpd_line.header` only. + expectedNotices: 1, + }, +}; + /** * `chartConfig.aria` (protocol 18, ADR-0049 enforce-or-remove; maintainer * decision batch #118 item 2, 2026-09-12 — recommendation C, judge the protocol @@ -15834,6 +15992,7 @@ const MAJOR_18_CONVERSIONS: readonly OrderedConversion[] = [ { conversion: agentMemoryLongTermStoreRemoved, order: 56 }, { conversion: agentStructuredOutputRefusedMembersRemoved, order: 55 }, { conversion: apiEndpointCacheTtlToCacheTtlSeconds, order: 23 }, + { conversion: cbpMasterDetailRequiredDefaulted, order: 63 }, { conversion: chartConfigAriaRemoved, order: 32 }, { conversion: connectorConnectionTimeoutMsRemoved, order: 20 }, { conversion: connectorErrorMappingRemoved, order: 19 }, diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index 1f74c94d8fd..0a2b751a751 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -1958,13 +1958,21 @@ function refuseAttachedBlockConflicts(attachedOnRead: unknown, fields: unknown, * always has two required ones and is refused. A field's `reference` needs no * test here: `FieldSchema` already refuses a `master_detail` without one. * - * Metadata at rest still LOADS: the stored-row seams register an object that - * fails its parse with a `[metadata_spec_invalid]` diagnostic carrying this - * message (ADR-0087's 2026-07-31 addendum, "reads diagnose, never drop"), and - * the save door refuses it with 422. No conversion rewrites it: turning a - * `lookup` into a `master_detail` changes the delete behaviour of every row - * (a detail cascades with its master), and whether an object should rather - * leave `controlled_by_parent` is the author's decision, not a mapping. + * Metadata at rest still LOADS. The stored-row seams first replay the ADR-0087 + * D2 conversion `cbp-master-detail-required-defaulted`, which gives an OMITTED + * `required` on an object's one `master_detail` the builder's own `true`. That + * is the one lossless shape, and objectui's Studio writes it. An object that + * still fails this parse registers anyway, under a `[metadata_spec_invalid]` + * 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; + * - 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 + * rather leave `controlled_by_parent` is the author's decision. + * The conversion is retired from the authoring load path, so a live author + * still meets this refusal. */ function refuseControlledByParentWithoutOneMaster( objectName: unknown, diff --git a/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts b/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts index db95387dea7..3eeda476dc3 100644 --- a/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts +++ b/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts @@ -4,9 +4,12 @@ import type { SemanticMigration } from '../../types.js'; // ADR-0055 §1 enforced (maintainer ruling B): a `controlled_by_parent` object // derives its master from exactly one required `master_detail`. The runtime's two -// unrecorded fallbacks retire under ADR-0049, and no D2 conversion exists — a -// `lookup` turned `master_detail` changes delete behaviour, and leaving -// `controlled_by_parent` is an author's call — so this entry is the whole record. +// unrecorded fallbacks retire under ADR-0049. One shape converts losslessly at +// rest: an omitted `required` on the object's one `master_detail`, which gets the +// builder's own `true`. That is the D2 conversion `cbp-master-detail-required-defaulted`, +// and objectui's Studio is its producer. Every other shape is this entry's +// prescription: a `lookup` turned `master_detail` changes delete behaviour, and +// leaving `controlled_by_parent` is an author's call. export const entry: SemanticMigration = { id: 'cbp-master-relation-tiers-retired', surface: 'object.sharingModel = \'controlled_by_parent\' together with object.fields — the master ' @@ -18,7 +21,10 @@ export const entry: SemanticMigration = { + 'by field order. `ObjectSchema` refuses each of those shapes at parse, naming the object — so ' + '`ObjectSchema.create()`, `defineStack`, `os validate` and the metadata save door refuse them — and ' + 'the runtime derives no master from them: every read of such an object is denied and every write ' - + 'refused with 422 INVALID_METADATA. A required `lookup` to the master becomes `type: \'master_detail\'` ' + + 'refused with 422 INVALID_METADATA. Data at rest whose one `master_detail` merely omits `required` ' + + 'is converted, not refused: the D2 conversion `cbp-master-detail-required-defaulted` gives it the ' + + '`required: true` that `ObjectSchema.create()` would have given it. It runs on stored rows, built ' + + 'artifacts and `os migrate meta --from 17`, never on a live authoring parse. A required `lookup` to the master becomes `type: \'master_detail\'` ' + '(its rows are then deleted with their master; declare `deleteBehavior: \'restrict\'` to refuse deleting ' + 'a master that still has rows instead). An object whose rows must outlive their parent leaves ' + '`controlled_by_parent` for its own sharing model (`private`, `public_read` or `public_read_write`).', @@ -31,10 +37,14 @@ export const entry: SemanticMigration = { + 'reading the record believed such an object was refused while it booted and derived its access from ' + 'whatever the fallback picked, and the platform\'s own derived proof (the parent-derived write reach ' + 'family) disagreed with its runtime about which objects the rule covers. The maintainer ruling (letter ' - + 'B) enforces the record: one tier, refused at parse, retired in the runtime. No conversion is offered ' - + 'because none is lossless — turning a `lookup` into a `master_detail` changes what deleting the master ' - + 'does to every row, and whether an object should instead stop being controlled by its parent is a ' - + 'product decision only its author can make.', + + 'B) enforces the record: one tier, refused at parse, retired in the runtime. Exactly one shape converts ' + + 'losslessly: an OMITTED `required` on the object\'s one `master_detail` is the builder\'s own force. ' + + 'It was measured to have a producer (objectui\'s Studio adds a field as `{ type, label }`, its Required ' + + 'toggle writes `true` or nothing, and its Settings tab offers `controlled_by_parent`), so D2 carries ' + + 'it for data at rest. Nothing else converts. An explicit `required: false` is a contradiction the ' + + 'builder refuses. Two masters would need a pick. Turning a `lookup` into a `master_detail` changes ' + + 'what deleting the master does to every row, and whether an object should instead stop being ' + + 'controlled by its parent is a product decision only its author can make.', acceptanceCriteria: 'Every object declaring `sharingModel: \'controlled_by_parent\'` declares exactly one `master_detail` ' + 'field with `required: true` (through `ObjectSchema.create()`, an omitted `required` on its one ' @@ -43,7 +53,8 @@ export const entry: SemanticMigration = { + '"with N required `master_detail` fields" issue. `os lint` reports neither ' + '`security-controlled-by-parent-no-relation` nor `security-controlled-by-parent-ambiguous-relation`. At ' + 'runtime the security plugin logs no `[security/controlled-by-parent]` "resolves no master relation" ' - + 'line for the object, and a member who can read its master reads its rows. Stored metadata is not ' - + 'rewritten: a stored row in a retired shape still registers (with its `[metadata_spec_invalid]` ' + + 'line for the object, and a member who can read its master reads its rows. A stored row whose one ' + + '`master_detail` omitted `required` is served with `required: true` (`cbp-master-detail-required-defaulted`); ' + + 'a stored row in any other retired shape still registers (with its `[metadata_spec_invalid]` ' + 'diagnostic) and is denied until it is corrected and saved.', }; diff --git a/packages/spec/src/migrations/registry.ts.template b/packages/spec/src/migrations/registry.ts.template index 25737669141..9971b55eefc 100644 --- a/packages/spec/src/migrations/registry.ts.template +++ b/packages/spec/src/migrations/registry.ts.template @@ -1300,6 +1300,25 @@ const STEP18_RATIONALE: readonly RationaleFragment[] = [ + 'retired, deletes a retired `fallbackFormat` and drops `coerce_types` from the pipeline, ' + 'retired from the load path.', }, + { + id: 'cbp-master-relation-tiers-retired', + order: 95, + text: + 'It also enforces ADR-0055 §1 (ADR-0049 enforce-or-remove, maintainer ruling B): a ' + + '`controlled_by_parent` object derives its master from exactly one `master_detail` field ' + + 'declared `required: true`, and from nothing else. The parse now refuses the object with none or ' + + 'with two, naming it and the rule. The security runtime\'s two unrecorded fallback tiers retire: ' + + 'any `master_detail`, then a required `lookup`, picked by field order. The runtime derives no ' + + 'master from those shapes, denying every read and refusing every write with 422 ' + + 'INVALID_METADATA, and the linter\'s two rules read the same one tier. One shape converts ' + + 'losslessly: an omitted `required` on the object\'s one `master_detail`, which objectui\'s ' + + 'Studio writes. The D2 conversion `cbp-master-detail-required-defaulted` gives it the builder\'s ' + + 'own `required: true`, retired from the load path so a live author is refused at parse while ' + + 'stored rows and `os migrate meta` replay it. A required `lookup` is not converted, because ' + + 'turning it into a `master_detail` changes what deleting the master does. Neither is an explicit ' + + '`required: false` or a second master. Its D3 record is the semantic entry ' + + '`cbp-master-relation-tiers-retired`.', + }, { id: 'chart-config-aria-retired', order: 31, From 6582c1f0d8e30afbbb1386d731d524966075a46e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 13 Oct 2026 07:18:02 +0000 Subject: [PATCH 11/11] fix(spec): the one-master D3 entry names os migrate meta without the --from marker the house-sentence pin reserves Claude-Session: https://claude.ai/code/session_01BRQ9Mp1yH7R1az6MaUM18H Co-authored-by: Claude --- .../entries/semantic/18.cbp-master-relation-tiers-retired.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts b/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts index 3eeda476dc3..c4bf113ffb8 100644 --- a/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts +++ b/packages/spec/src/migrations/entries/semantic/18.cbp-master-relation-tiers-retired.ts @@ -24,7 +24,7 @@ export const entry: SemanticMigration = { + 'refused with 422 INVALID_METADATA. Data at rest whose one `master_detail` merely omits `required` ' + 'is converted, not refused: the D2 conversion `cbp-master-detail-required-defaulted` gives it the ' + '`required: true` that `ObjectSchema.create()` would have given it. It runs on stored rows, built ' - + 'artifacts and `os migrate meta --from 17`, never on a live authoring parse. A required `lookup` to the master becomes `type: \'master_detail\'` ' + + 'artifacts and `os migrate meta`, never on a live authoring parse. A required `lookup` to the master becomes `type: \'master_detail\'` ' + '(its rows are then deleted with their master; declare `deleteBehavior: \'restrict\'` to refuse deleting ' + 'a master that still has rows instead). An object whose rows must outlive their parent leaves ' + '`controlled_by_parent` for its own sharing model (`private`, `public_read` or `public_read_write`).',