From 9753e50386a7bda352914baaad25a06d863e29e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 06:13:30 +0000 Subject: [PATCH] feat(lint): gate a sharing rule anchored where sharing has nothing to widen (#9698) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `validateSharingRuleEnforceability` gains its second arm. It already judged a sharing rule's `condition` against the compiler that lowers it; it now judges the rule's `object` against the verdict that decides whether the grant can exist at all — `SharingService.inertGrantReason` (ADR-0111 D7), mirrored rather than modelled, over the two of its arms that are decidable from authored metadata before anything boots. Two ids, because the two are not the same failure. Measured against a real `SharingService`: a `public` OWD refuses the grant and leaves `buildReadFilter` returning null, so the audience already reads everything and the declaration advertises a restriction that does not exist; a `controlled_by_parent` anchor refuses with a different reason and leaves the recipient seeing only what the MASTER grants, so the author believes a grant exists that does not. Blast radius measured through `objectstack build` before choosing `error`: 5 sharing rules exist in this repo, 3 fire, all in examples/app-crm, all on `public_read_write` objects, all failing their boot backfill since they were written. Removed under ADR-0049 enforce-or-remove, as #9237 did for the two equivalent showcase rules. The showcase's remaining 2 stay silent. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja --- .changeset/sharing-rule-inert-anchor-gate.md | 56 ++++ content/docs/permissions/index.mdx | 24 +- examples/app-crm/objectstack.config.ts | 23 +- examples/app-crm/src/security/index.ts | 6 - .../app-crm/src/security/sharing-rules.ts | 65 ----- examples/app-crm/test/smoke.test.ts | 33 ++- packages/lint/src/authoring-rules.ts | 45 ++-- packages/lint/src/index.ts | 12 +- ...lidate-sharing-rule-enforceability.test.ts | 155 +++++++++++ .../validate-sharing-rule-enforceability.ts | 249 +++++++++++++++++- 10 files changed, 554 insertions(+), 114 deletions(-) create mode 100644 .changeset/sharing-rule-inert-anchor-gate.md delete mode 100644 examples/app-crm/src/security/sharing-rules.ts diff --git a/.changeset/sharing-rule-inert-anchor-gate.md b/.changeset/sharing-rule-inert-anchor-gate.md new file mode 100644 index 0000000000..d0225a1c4d --- /dev/null +++ b/.changeset/sharing-rule-inert-anchor-gate.md @@ -0,0 +1,56 @@ +--- +"@objectstack/lint": minor +--- + +feat(lint): a sharing rule anchored where sharing has nothing to widen is now an authoring-time error (#9698) + +`validateSharingRuleEnforceability` gains its second arm. It already judged a +sharing rule's `condition` against the compiler that lowers it; it now judges +the rule's `object` against the verdict that decides whether the grant can +exist at all. + +Two new `error` ids, both decidable from authored metadata before anything +boots, and both mirroring `SharingService.inertGrantReason` (ADR-0111 D7) +rather than modelling it: + +- **`sharing-rule-object-not-shareable`** — the anchor object's effective + sharing model is `public` (an explicit `sharingModel: 'public_read_write'`, + or no `sharingModel` on a system object, which ADR-0090 D1 resolves to + public). Sharing only ever WIDENS an OWD baseline, so on the widest baseline + there is nothing to widen. +- **`sharing-rule-object-controlled-by-parent`** — the anchor is a + master-detail detail, whose visibility is derived from its master + (ADR-0055). It gets its own id and its own fix-it ("share the master + record instead"), because `effectiveSharingModel` collapses it onto the same + `public` verdict while the correct repair is completely different. + +Both were previously accepted by `SharingRuleSchema`, accepted by `defineRule`, +seeded into `sys_sharing_rule`, and only then refused — once per boot, as a +WARN line inside the boot diagnostics block. That WARN is not a sufficient +diagnostic, and the reason is measured rather than argued: a rule whose criteria +match no seeded row never reaches `grant`, so it never throws and warns nothing +while being exactly as dead. The WARN is a function of the DATA; the defect is a +property of the DECLARATION. + +**Blast radius, measured through `objectstack build` before deciding the +severity:** 5 sharing rules are declared in this repo. 3 fire, all of them in +`examples/app-crm` — `share_high_value_opps_with_managers`, +`share_active_leads_with_manager` and `share_won_deal_activities`, anchored on +`crm_opportunity`, `crm_lead` and `crm_activity`, every one of them +`sharingModel: 'public_read_write'`. They have been failing their boot backfill +on every boot of that app since they were written, and they are removed here +under ADR-0049 enforce-or-remove — the same call #9237 made for the two +equivalent rules in `app-showcase`. The other 2 (app-showcase's, both on +`private` objects) stay silent, which is the direction that had to be proven +rather than hoped for. + +The CRM's smoke test used to assert that these rules existed and were of the +enforced `criteria` type. Both assertions passed while all three rules enforced +nothing, so the assertion is replaced by the property their greenness hid: no +declared rule may be anchored where sharing has nothing to widen. + +Deliberately NOT judged, because they are not decidable from authored metadata: +the `owner_id` arm (`owner_id` is injected by the schema registry, so asserting +it would fail every object that correctly does not declare it by hand), the +`bypassObjects` arm (plugin configuration, not stack metadata), and the +federated phantom-anchor arm (a provenance test over that same injected column). diff --git a/content/docs/permissions/index.mdx b/content/docs/permissions/index.mdx index 3d3f3db02b..48cbd90a38 100644 --- a/content/docs/permissions/index.mdx +++ b/content/docs/permissions/index.mdx @@ -25,22 +25,28 @@ is reserved (D3) — if you knew the v1 model, start at [Positions](/docs/permissions/positions). Access rules are metadata like everything else — this is a real sharing rule from -the CRM example app: +the showcase example app: ```typescript -export const HighValueOpportunitySharingRule = defineSharingRule({ +export const KeyAccountQualifiedContactRule = defineSharingRule({ type: 'criteria', - name: 'share_high_value_opps_with_managers', - label: 'High-Value Deals → Sales Managers', - description: 'Automatically share opportunities over $100,000 with all Sales Managers.', - object: 'crm_opportunity', - condition: 'record.amount > 100000', - accessLevel: 'edit', - sharedWith: { type: 'position', value: 'sales_manager' }, + name: 'share_key_account_qualified_contacts_with_managers', + label: 'Key-Account Qualified Contacts → Managers', + description: 'Share qualified contacts at the key account with managers.', + object: 'showcase_contact', + condition: "record.stage == 'qualified' && record.company == 'Northwind'", + accessLevel: 'read', + sharedWith: { type: 'position', value: 'manager' }, active: true, }); ``` +Note the object it is anchored on. `showcase_contact` is OWD `private`, which is +what makes this grant one a read gate actually consults — sharing only ever +WIDENS a baseline, so a rule on an object that is already `public_read_write` +grants nothing and is refused at boot. That is an authoring-time build error +(`sharing-rule-object-not-shareable`), not a runtime surprise. + Because AI agents act through the same permission-aware surface, these rules bound agent access exactly as they bound users ([Actions as Tools](/docs/ai/actions-as-tools)). diff --git a/examples/app-crm/objectstack.config.ts b/examples/app-crm/objectstack.config.ts index fe3e5d1bec..55825762d6 100644 --- a/examples/app-crm/objectstack.config.ts +++ b/examples/app-crm/objectstack.config.ts @@ -17,9 +17,6 @@ import { FinanceApproverPosition, SalesUserPermissionSet, GuestPortalProfile, - HighValueOpportunitySharingRule, - RepLeadSharingRule, - WonDealActivitySharingRule, } from './src/security/index.js'; import { registerCrmPositionBindings } from './src/security/bind-position-sets.js'; import { CrmSeedData } from './src/data/index.js'; @@ -102,11 +99,21 @@ export default defineStack({ // Security positions: [SalesRepPosition, SalesManagerPosition, FinanceApproverPosition], permissions: [SalesUserPermissionSet, GuestPortalProfile], - sharingRules: [ - HighValueOpportunitySharingRule, - RepLeadSharingRule, - WonDealActivitySharingRule, - ], + // No `sharingRules`. The three this app used to declare + // (`share_high_value_opps_with_managers`, `share_active_leads_with_manager`, + // `share_won_deal_activities`) were anchored on `crm_opportunity`, + // `crm_lead` and `crm_activity` — all three `sharingModel: + // 'public_read_write'`. Sharing only ever WIDENS an OWD baseline, so on the + // widest baseline there is nothing to widen: `assertNotInertGrant` refused + // every grant with SHARING_NOT_ENABLED and the boot backfill failed for each + // rule, on every boot, since they were written. They granted nothing and + // were removed under ADR-0049 enforce-or-remove (#9698), the same call + // #9237 made for app-showcase's two. + // + // ⛔ Do not re-add one on a public object — `sharing-rule-object-not-shareable` + // now fails the build, and its message states the two honest fixes. Giving + // this app a LIVE sharing demonstration means giving it a `private` object + // first; that is an access-matrix change (ADR-0090 D6 review), not a rider. // Seed data data: CrmSeedData, diff --git a/examples/app-crm/src/security/index.ts b/examples/app-crm/src/security/index.ts index 99f94e182b..b14d6a05cc 100644 --- a/examples/app-crm/src/security/index.ts +++ b/examples/app-crm/src/security/index.ts @@ -8,9 +8,3 @@ export { GuestPortalProfile, } from './sales-positions.js'; -export { - HighValueOpportunitySharingRule, - RepLeadSharingRule, - WonDealActivitySharingRule, -} from './sharing-rules.js'; - diff --git a/examples/app-crm/src/security/sharing-rules.ts b/examples/app-crm/src/security/sharing-rules.ts deleted file mode 100644 index 876ae54490..0000000000 --- a/examples/app-crm/src/security/sharing-rules.ts +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. - -import { defineSharingRule } from '@objectstack/spec/security'; - -/** - * Criteria-based sharing: share high-value opportunities (amount > 100000) - * with the Sales Manager role so managers always have read access to big deals. - */ -export const HighValueOpportunitySharingRule = defineSharingRule({ - type: 'criteria', - name: 'share_high_value_opps_with_managers', - label: 'High-Value Deals → Sales Managers', - description: 'Automatically share opportunities over $100,000 with all Sales Managers.', - object: 'crm_opportunity', - condition: 'record.amount > 100000', - accessLevel: 'edit', - sharedWith: { - type: 'position', - value: 'sales_manager', - }, - active: true, -}); - -/** - * Criteria-based sharing: in-flight leads (not yet converted or disqualified) - * are shared read-only with Sales Managers for coaching visibility. - * - * Replaces the retired owner-based `share_rep_leads_with_manager` rule: - * `type: 'owner'` (`ownedBy`) no longer parses — it depended on live position - * membership and was silently skipped at seed time (ADR-0078). The enforced - * criteria form scopes the same coaching set by pipeline state instead. - */ -export const RepLeadSharingRule = defineSharingRule({ - type: 'criteria', - name: 'share_active_leads_with_manager', - label: 'Active Leads → Manager (read-only)', - description: 'Share in-flight (not converted/disqualified) leads with Sales Managers for coaching visibility.', - object: 'crm_lead', - condition: "record.status != 'converted' && record.status != 'disqualified'", - accessLevel: 'read', - sharedWith: { - type: 'position', - value: 'sales_manager', - }, - active: true, -}); - -/** - * Criteria-based: share activities linked to won deals with the whole - * Sales team so everyone can learn from successful engagement patterns. - */ -export const WonDealActivitySharingRule = defineSharingRule({ - type: 'criteria', - name: 'share_won_deal_activities', - label: 'Won-Deal Activities → All Sales', - description: 'Share activities attached to closed-won opportunities across the sales team.', - object: 'crm_activity', - condition: "record.status == 'completed'", - accessLevel: 'read', - sharedWith: { - type: 'position', - value: 'sales_rep', - }, - active: true, -}); diff --git a/examples/app-crm/test/smoke.test.ts b/examples/app-crm/test/smoke.test.ts index 1fb560b54c..8e59310bec 100644 --- a/examples/app-crm/test/smoke.test.ts +++ b/examples/app-crm/test/smoke.test.ts @@ -93,11 +93,34 @@ describe('app-crm minimal metadata bundle', () => { expect(stack.i18n!.supportedLocales).toContain('zh-CN'); }); - it('has criteria sharing rules (the enforced form — owner-type was retired)', () => { - const rules = stack.sharingRules ?? []; - expect(rules.length).toBeGreaterThanOrEqual(2); - // `type: 'owner'` no longer parses (never enforced; ADR-0078): every - // declared rule is the enforced criteria form. + // #9698 — this used to assert `rules.length >= 2` and `every(type === + // 'criteria')`. Both passed, and neither was the property that mattered: + // all three declared rules were anchored on `public_read_write` objects, so + // `assertNotInertGrant` refused every grant and the boot backfill failed for + // each one, on every boot. A test can assert the enforced FORM and stay + // green while the rule enforces nothing. The three were removed (ADR-0049 + // enforce-or-remove); what replaces the assertion is the property their + // greenness hid, so re-adding one on a public object goes red HERE as well + // as at `objectstack build`. + it('no declared sharing rule is anchored where sharing has nothing to widen (#9698)', () => { + const rules = (stack.sharingRules ?? []) as Array<{ name?: string; object?: string; type?: string }>; + const owdOf = new Map( + ((stack.objects ?? []) as Array<{ name?: string; sharingModel?: string }>) + .map((o) => [String(o.name), o.sharingModel]), + ); + // `effectiveSharingModel` maps BOTH to 'public'; `controlled_by_parent` + // earns its own runtime refusal ("share the master record instead"). + const INERT_OWD = new Set(['public_read_write', 'controlled_by_parent']); + const offenders = rules + .filter((r) => INERT_OWD.has(String(owdOf.get(String(r.object))))) + .map((r) => `${r.name} → ${r.object} (sharingModel '${owdOf.get(String(r.object))}')`); + expect( + offenders, + `sharing rule(s) whose grant no gate would consult — the boot backfill refuses these with ` + + `SHARING_NOT_ENABLED: ${offenders.join(', ')}`, + ).toEqual([]); + // `type: 'owner'` no longer parses (never enforced; ADR-0078): whatever is + // declared is the enforced criteria form. expect(rules.every((r) => r.type === 'criteria')).toBe(true); }); diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index d0058b07e6..20a309e224 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -1306,16 +1306,26 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ surfaceReason: RUNTIME_NEEDS_FULL_SNAPSHOT, run: (stack) => validateOrgAxisRedLines(stack), }, - // #4698 — the "declared but never read" gate, for the one surface where the - // predicate is EXACT rather than inferred. A sharing rule's `condition` has a - // single runtime consumer (`bootstrapDeclaredSharingRules`) whose only use of - // the key is `compileCelToFilter(condition, { variables: {} })`; a condition - // that does not lower means the rule is SKIPPED at boot, so the grant is - // declared and does not exist. The lint calls that same compiler, from the - // same package, with the same options — the verdict cannot drift from the - // consumer's. Gating for the ADR-0078 reason `SharingRuleSchema`'s own - // docblock states: the whole authorable surface is enforced, and this was the - // one field where that sentence was not yet true. + // #4698 / #9698 — the "declared but never read" gate, for the two fields of a + // sharing rule where the predicate is EXACT rather than inferred. + // + // - `condition` has a single runtime consumer + // (`bootstrapDeclaredSharingRules`) whose only use of the key is + // `compileCelToFilter(condition, { variables: {} })`; a condition that + // does not lower means the rule is SKIPPED at boot. + // - `object` decides whether the grant is refused outright: reconcile hands + // each row to `SharingService.grant`, whose ADR-0111 D7 pre-flight THROWS + // `SHARING_NOT_ENABLED` when the anchor's effective sharing model is + // `public` or it is a `controlled_by_parent` detail. Both are decidable + // from authored metadata; the other arms of that verdict (`owner_id`, the + // bypass set, federated anchors) are not, and are excluded by name. + // + // Either way the grant is declared and does not exist. The lint calls the + // same compiler and mirrors the same verdict function, from the same inputs + // — the verdict cannot drift from the consumers'. Gating for the ADR-0078 + // reason `SharingRuleSchema`'s own docblock states: the whole authorable + // surface is enforced, and these were the fields where that sentence was not + // yet true. { name: 'validateSharingRuleEnforceability', tier: 'gating', @@ -1324,11 +1334,16 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ source: 'packages/lint/src/validate-sharing-rule-enforceability.ts', surfaces: CLI_ONLY, surfaceReason: - 'P2 (#4463): a sharing rule is not a `flow`, and P1 gates `flow` alone. The rule itself is ' - + 'snapshot-safe — it reads ONLY `stack.sharingRules[].condition` and needs no other collection — ' - + 'so widening it here is a `runtimeTypes: [\'sharing_rule\']` edit once the gate accepts that type, ' - + 'not new wiring. Recorded as pending rather than done, because a rule that has never run at a ' - + 'door should not claim it.', + 'P2 (#4463): a sharing rule is not a `flow`, and P1 gates `flow` alone. This entry used to add ' + + 'that the rule reads ONLY `stack.sharingRules[].condition` and needs no other collection, so ' + + 'crossing was a lone `runtimeTypes` edit. #9698 FALSIFIED that: the anchor arm resolves ' + + '`sharingRules[].object` against `stack.objects` to read the anchor\'s OWD, so the rule is now ' + + 'cross-collection. `objects` IS carried by the per-write snapshot (`CONTEXT_STACK_KEYS`, #8309), ' + + 'so the remaining gap is unchanged in SHAPE — the gate must accept a `sharing_rule` type and the ' + + 'snapshot must carry `sharingRules`, which it does not — but it is now TWO collections, not one. ' + + 'Crossing with `sharingRules` uncarried would enforce this id for zero of its inputs while the ' + + 'entry claimed the door (#7220). Recorded as pending rather than done, because a rule that has ' + + 'never run at a door should not claim it.', run: (stack) => validateSharingRuleEnforceability(stack), }, // #4983 — the sibling surface of the rule above, and ADR-0056 D4's gate, diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index e79e0c6c13..5540222eb5 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -261,14 +261,18 @@ export { } from './validate-org-axis-red-lines.js'; export type { OrgAxisFinding, OrgAxisSeverity } from './validate-org-axis-red-lines.js'; -// #4698 — "a key that nothing reads should not validate clean", for the one -// surface where "is it read?" is decidable: a sharing rule's `condition` is -// read ONLY through `compileCelToFilter`, so the lint calls that same compiler -// rather than modelling the consumer. +// #4698 / #9698 — "a key that nothing reads should not validate clean", for the +// two fields of a sharing rule where "is it read?" is decidable. The +// `condition` is read ONLY through `compileCelToFilter`, and the `object` +// decides whether `assertNotInertGrant` would refuse the grant outright — so +// the lint calls the same compiler and mirrors the same verdict function, +// rather than modelling either consumer. export { validateSharingRuleEnforceability, SHARING_RULE_UNLOWERABLE_CONDITION, SHARING_RULE_RUNTIME_VARIABLE_CONDITION, + SHARING_RULE_OBJECT_NOT_SHAREABLE, + SHARING_RULE_OBJECT_CONTROLLED_BY_PARENT, } from './validate-sharing-rule-enforceability.js'; export type { SharingRuleEnforceabilityFinding, diff --git a/packages/lint/src/validate-sharing-rule-enforceability.test.ts b/packages/lint/src/validate-sharing-rule-enforceability.test.ts index 386a28acbc..bc673a4297 100644 --- a/packages/lint/src/validate-sharing-rule-enforceability.test.ts +++ b/packages/lint/src/validate-sharing-rule-enforceability.test.ts @@ -7,6 +7,8 @@ import { validateSharingRuleEnforceability, SHARING_RULE_UNLOWERABLE_CONDITION, SHARING_RULE_RUNTIME_VARIABLE_CONDITION, + SHARING_RULE_OBJECT_NOT_SHAREABLE, + SHARING_RULE_OBJECT_CONTROLLED_BY_PARENT, } from './validate-sharing-rule-enforceability.js'; import { AUTHORING_RULES, runAuthoringRules } from './authoring-rules.js'; @@ -238,3 +240,156 @@ describe('validateSharingRuleEnforceability — the verdict IS the seeder\'s ver } }); }); + +// ── The ANCHOR arm: would a share row on this object ever be consulted? ── +// +// The `condition` arm above judges one field of the rule; this one judges the +// other field that can decide the same question. Every fixture below carries a +// LOWERABLE condition, so any finding here is the anchor arm's alone. + +/** A stack with one object at `owd` and one rule anchored on it. */ +const anchoredOn = (owd: unknown, extra: Record = {}) => ({ + objects: [ + { + name: 'crm_opportunity', + label: 'Opportunity', + ...(owd === undefined ? {} : { sharingModel: owd }), + fields: { name: { type: 'text', label: 'Name' }, amount: { type: 'number', label: 'Amount' } }, + ...extra, + }, + ], + sharingRules: [ + { + name: 'high_value_opps', + type: 'criteria', + object: 'crm_opportunity', + accessLevel: 'read', + sharedWith: { type: 'position', value: 'sales_manager' }, + condition: 'record.amount > 100000', + }, + ], +}); + +describe('validateSharingRuleEnforceability — an anchor no gate would consult goes RED', () => { + it('flags `public_read_write` — nothing left to widen', () => { + const findings = validateSharingRuleEnforceability(anchoredOn('public_read_write')); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + severity: 'error', + rule: SHARING_RULE_OBJECT_NOT_SHAREABLE, + // The defect is on `object`, not on `condition` — a different field with + // a different fix, so it gets its own path. + path: 'sharingRules[0].object', + where: 'sharing rule "high_value_opps" on object "crm_opportunity"', + }); + // An author must be able to act on this: it has to name the object, the + // rule, and WHY the rule cannot take effect. + expect(findings[0].message).toMatch(/high_value_opps/); + expect(findings[0].message).toMatch(/crm_opportunity/); + expect(findings[0].message).toMatch(/sharingModel 'public_read_write'/); + expect(findings[0].message).toMatch(/SHARING_NOT_ENABLED/); + // …and both honest fixes, because which one is right is the author's call. + expect(findings[0].hint).toMatch(/delete it/); + expect(findings[0].hint).toMatch(/sharingModel: 'private'/); + }); + + it('flags `controlled_by_parent` with its OWN reason and its OWN id', () => { + const findings = validateSharingRuleEnforceability( + anchoredOn('controlled_by_parent', { + fields: { + name: { type: 'text', label: 'Name' }, + opportunity: { type: 'master_detail', reference: 'crm_account', required: true }, + }, + }), + ); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + severity: 'error', + rule: SHARING_RULE_OBJECT_CONTROLLED_BY_PARENT, + path: 'sharingRules[0].object', + }); + // `effectiveSharingModel` maps this value to 'public' too, so a single-id + // implementation would hand the author the WRONG fix. The runtime tests + // `controlled_by_parent` first and answers "share the master record + // instead"; this arm mirrors that order, and names the master it resolved. + expect(findings[0].message).toMatch(/derived from its master/i); + expect(findings[0].message).toMatch(/share the master record instead/); + expect(findings[0].hint).toMatch(/crm_account/); + }); + + it('flags a SYSTEM object with no OWD — absent resolves to public there (ADR-0090 D1)', () => { + // The one arm that is not readable off `sharingModel` alone: for `sys_*` / + // `isSystem` the ABSENCE of an OWD is the public fall-through, and + // `security-owd-unset` deliberately exempts system objects, so nothing + // else reports it either. + const stack = anchoredOn(undefined); + (stack.objects[0] as Record).name = 'sys_audit_entry'; + (stack.sharingRules[0] as Record).object = 'sys_audit_entry'; + const findings = validateSharingRuleEnforceability(stack); + expect(findings.map((f) => f.rule)).toEqual([SHARING_RULE_OBJECT_NOT_SHAREABLE]); + expect(findings[0].message).toMatch(/declares no sharingModel and is a system object/); + }); + + it('reports the anchor and the condition INDEPENDENTLY — two fields, two fixes', () => { + const stack = anchoredOn('public_read_write'); + (stack.sharingRules[0] as Record).condition = 'has(record.amount)'; + const findings = validateSharingRuleEnforceability(stack); + expect(findings.map((f) => f.rule).sort()).toEqual( + [SHARING_RULE_OBJECT_NOT_SHAREABLE, SHARING_RULE_UNLOWERABLE_CONDITION].sort(), + ); + // Fixing the condition would not make this rule grant anything, and fixing + // the anchor would not make the condition lower. Suppressing either would + // hide a defect the author still has to fix. + expect(new Set(findings.map((f) => f.path))).toEqual( + new Set(['sharingRules[0].object', 'sharingRules[0].condition']), + ); + }); +}); + +// ── The direction that matters more: it must NOT fire on correct metadata ── + +describe('validateSharingRuleEnforceability — an anchor a gate WOULD consult stays SILENT', () => { + it.each([ + ['private — the posture sharing exists for', 'private'], + ['public_read — owner writes, so a share row still widens WRITE', 'public_read'], + ])('is silent on %s', (_label, owd) => { + expect(validateSharingRuleEnforceability(anchoredOn(owd))).toEqual([]); + }); + + it('is silent on a CUSTOM object with no OWD — absence fails CLOSED to private', () => { + // The asymmetry that makes the system-object case above a real arm and + // this one a false positive if the mirror were sloppy: ADR-0090 D1 sends + // an unset custom OWD to `private`, where sharing IS enforced. + expect(validateSharingRuleEnforceability(anchoredOn(undefined))).toEqual([]); + }); + + it('is silent on a retired OWD alias — the runtime fails CLOSED, so the rule is LIVE', () => { + // `sharingModel: 'read'` is not canonical (ADR-0090 D4). The runtime's + // fall-through sends an unrecognised value to `private`, NOT to public, so + // reporting inertness here would be a false positive on top of the + // `security-owd-alias` error the value already earns. + expect(validateSharingRuleEnforceability(anchoredOn('read'))).toEqual([]); + }); + + it('is silent when the anchor object is not declared by this stack', () => { + // Absence of a schema is absence of EVIDENCE of inertness, not evidence of + // liveness — the object may come from a plugin or an upstream stack. The + // runtime draws the same line: `assertSharingEnforced` keeps existence a + // SEPARATE verdict from inertness. + const stack = anchoredOn('public_read_write'); + (stack.sharingRules[0] as Record).object = 'not_in_this_stack'; + expect(validateSharingRuleEnforceability(stack)).toEqual([]); + }); + + it('is silent on an owner-less object — `owner_id` is REGISTRY-INJECTED, not authored', () => { + // `inertGrantReason`'s third arm refuses a grant on an object with no + // owner field, and it is deliberately NOT mirrored: `owner_id` is injected + // by the schema registry, so it is absent from authored metadata and + // present on the runtime schema. Judging it here would fail every object + // that correctly does not declare it by hand — which is every object in + // the fixture above, none of which declares `owner_id`. + const stack = anchoredOn('private'); + expect(Object.keys((stack.objects[0] as { fields: object }).fields)).not.toContain('owner_id'); + expect(validateSharingRuleEnforceability(stack)).toEqual([]); + }); +}); diff --git a/packages/lint/src/validate-sharing-rule-enforceability.ts b/packages/lint/src/validate-sharing-rule-enforceability.ts index 70d17e2299..ff3f239c40 100644 --- a/packages/lint/src/validate-sharing-rule-enforceability.ts +++ b/packages/lint/src/validate-sharing-rule-enforceability.ts @@ -30,6 +30,24 @@ * verdict is bit-identical to the seeder's by construction — there is no * heuristic to drift and no false positive that is not also a real skip. * + * ## The second consumer: the rule's ANCHOR object (#9698) + * + * `condition` is not the only field of a declared sharing rule with an exact + * predicate. `object` has one too, and a different consumer: + * `SharingRuleService.reconcile` hands each resolved row to + * `SharingService.grant`, whose `assertNotInertGrant` pre-flight (ADR-0111 D7) + * REFUSES a grant whose `sys_record_share` row no gate could ever consult. Two + * of `inertGrantReason`'s arms are decidable before anything boots — the + * anchor object's effective sharing model being `public`, and its being a + * `controlled_by_parent` detail — and {@link anchorFindings} reports exactly + * those two, mirroring the runtime's own function rather than modelling it. + * + * The two halves of this file are therefore the two halves of one question, + * "will this declared grant ever exist?", answered over the two fields that + * can decide it. They are reported independently: an inert anchor and an + * unlowerable condition are different defects on different fields with + * different fixes, and fixing one does not reveal the other any earlier. + * * ## Why `error` * * `SharingRuleSchema`'s own docblock makes the claim this rule enforces: "The @@ -46,6 +64,17 @@ * declared anywhere in this repo (examples, platform permission sets) lowers * cleanly, so the gate turns nothing red that works today. * + * The anchor arm's severity is the same verdict on a stronger runtime fact — + * `assertNotInertGrant` does not skip and log, it THROWS — but its blast + * radius was NOT zero, and that is recorded rather than smoothed over. + * Measured across every sharing rule declared in this repo at the time it + * landed: 5 declarations, of which 3 fire, ALL of them in `examples/app-crm`, + * whose three rules are anchored on `public_read_write` objects and have + * therefore been failing their boot backfill on every boot of that app. Those + * three are repaired in the same change (see the changeset). The remaining 2 + * — `examples/app-showcase`'s, both on `private` objects — stay silent, which + * is the direction that had to be proven and not merely hoped for. + * * ## The two ids, and why not one * * `compileCelToFilter` fails for three reasons; two of them are authoring @@ -105,6 +134,10 @@ import { compileCelToFilter } from '@objectstack/formula'; export const SHARING_RULE_UNLOWERABLE_CONDITION = 'sharing-rule-unlowerable-condition'; /** A `condition` reading `current_user.*` — unresolvable when grants are materialized. */ export const SHARING_RULE_RUNTIME_VARIABLE_CONDITION = 'sharing-rule-runtime-variable-condition'; +/** The anchor object's OWD is already the widest — sharing has nothing to widen. */ +export const SHARING_RULE_OBJECT_NOT_SHAREABLE = 'sharing-rule-object-not-shareable'; +/** The anchor object is a master-detail DETAIL — its shares belong to its master. */ +export const SHARING_RULE_OBJECT_CONTROLLED_BY_PARENT = 'sharing-rule-object-controlled-by-parent'; export type SharingRuleEnforceabilitySeverity = 'error' | 'warning'; @@ -173,8 +206,212 @@ const PUSHDOWN_SUBSET = 'paths (ADR-0058 D2).'; /** - * Gate stack-declared sharing rules on the ONE thing their runtime consumer - * does with `condition`: lower it to a `criteria_json` filter. + * The object's effective sharing model, as `SharingService` computes it. + * + * A point-for-point mirror of `effectiveSharingModel` in + * `packages/plugins/plugin-sharing/src/sharing-service.ts` — same four + * recognised values, same `null` fall-through, same fail-CLOSED default. It is + * copied rather than imported for the reason stated at the head of this file: + * `@objectstack/lint` never depends on a runtime. The same discipline (and the + * same justification) already governs `resolveCbpRelation` in + * `validate-security-posture.ts`, which mirrors plugin-security's copy. + * + * ## Why the mirror cannot drift into a false positive + * + * Every input this function reads is AUTHORED metadata, and none of it is + * resolved, defaulted or injected between the authoring tier and the runtime: + * + * - `sharingModel` is `z.enum([...]).optional()` on `ObjectSchema` with **no + * `.default()`**, so "authored `private`" and "absent" stay distinguishable + * after parsing. (This is exactly the distinction a `.default()` erases — + * the trap that made a neighbouring one-line tightening reject 96 + * declarations instead of 1.) + * - The runtime's `schema?.security?.sharingModel` fallback is unreachable + * for any stack an author can ship: `ObjectSchema` is strict and declares no + * `security` key, so a stack nesting the OWD there is REFUSED, not stripped. + * `owdOf` in `validate-security-posture.ts` records the same finding. + * - `isSystem` / the `sys_` name prefix are authored too. `isSystem` carries + * `.default(false)`, which is why this reads `=== true` exactly as the + * runtime does — the default and the explicit `false` are the same verdict. + * + * So the linter and the service answer from the same bytes. What this function + * deliberately does NOT model is the part of the runtime verdict that is not in + * the metadata — see the `## What this rule deliberately does NOT do` section. + */ +function effectiveSharingModelOf(obj: AnyRec): 'private' | 'read' | 'public' { + const m = obj.sharingModel; + if (m === 'private') return 'private'; + if (m === 'public_read') return 'read'; + if (m === 'public_read_write' || m === 'controlled_by_parent') return 'public'; + if (m == null) { + const isSystem = obj.isSystem === true || str(obj.name).startsWith('sys_'); + return isSystem ? 'public' : 'private'; + } + // Fails CLOSED, like the runtime: an unrecognised value (a retired ADR-0090 + // D4 alias such as `read`/`full`) resolves to `private`, which means sharing + // IS enforced there and the rule is live. Reporting it would be a false + // positive — and the value itself is already `security-owd-alias`' finding. + return 'private'; +} + +/** The master a `controlled_by_parent` detail derives its access from, if named. */ +function masterOf(obj: AnyRec): string | undefined { + for (const f of asArray(obj.fields)) { + if (f.type === 'master_detail') { + const ref = f.reference; + if (typeof ref === 'string' && ref) return ref; + } + } + return undefined; +} + +/** + * The rule's ANCHOR arm: would a `sys_record_share` row on `rule.object` ever + * be consulted? + * + * This is the second runtime consumer of a declared sharing rule, and it + * refuses for reasons the `condition` arm above cannot see. + * `SharingRuleService.reconcile` calls `SharingService.grant` per resolved + * row; `grant` runs `assertNotInertGrant`, which THROWS + * `SHARING_NOT_ENABLED` when `inertGrantReason` names one (ADR-0111 D7). Two + * of that function's arms are decidable from authored metadata alone, and they + * are the two this arm reports. + * + * ## Why the boot WARN is not the diagnostic + * + * The refusal surfaces as one WARN per rule inside the boot diagnostics block + * — but only for a rule whose criteria matched at least one seeded row. + * Measured on the stock showcase: THREE rules were in this state and only TWO + * warned. The third's compound condition matched nothing, so `reconcile` built + * an empty desired set, never reached `grant`, and never threw. It was exactly + * as dead as the other two and produced no diagnostic at all. The WARN is a + * function of the DATA; the defect is a property of the DECLARATION, which is + * why it belongs here. + * + * ## Two ids, because the two arms are not the same failure + * + * Measured against a real `SharingService` over an in-memory engine (grant + + * `buildReadFilter`, all three postures), the arms differ in the direction + * that decides the wording: + * + * - **`public` OWD** → grant refused, `buildReadFilter` returns `null`, zero + * share rows. Nothing is filtered, so the intended audience already reads + * every row — and so does everyone else. Nobody UNDER-sees; the harm is + * that the declaration advertises a restriction that does not exist. + * - **`controlled_by_parent`** → grant refused with a DIFFERENT reason + * ("share the master record instead"), and the detail's visibility comes + * from its master's path (ADR-0055), not from this rule. Here the author + * genuinely believes a grant exists when it does not, and the intended + * recipient may see nothing. + * + * Different cause, different fix, different thing to tell the author — so two + * ids, the same reasoning that split the two `condition` ids above. + * + * ## What this arm deliberately does NOT do + * + * - **It does not judge the `owner_id` arm.** `inertGrantReason` also refuses + * an object with no owner field, but `owner_id` is INJECTED by the schema + * registry (`packages/objectql/src/registry.ts`) — absent from authored + * metadata by design, present on the runtime schema. Asserting it here + * would fail every object that correctly does not declare it by hand. The + * same exclusion, for the same reason, is written into the showcase's own + * guard. + * - **It does not judge the `bypassObjects` or federated-anchor arms.** The + * bypass set is plugin CONFIGURATION (`SharingPluginOptions.bypassObjects` + * plus a built-in list), not stack metadata, so it is not in this door's + * input at all; the federated phantom-anchor arm is a provenance test over + * an injected column, i.e. the `owner_id` exclusion one layer in. + * - **It does not report an unresolvable `rule.object`.** A name this stack + * does not declare is absence of EVIDENCE, not evidence of inertness — the + * object may be contributed by a plugin or an upstream stack. The runtime + * draws the same line: `assertSharingEnforced` treats existence as a + * SEPARATE verdict from inertness, and deliberately does not hard-fail the + * rule evaluator's system-context pass on an unregistered name. + */ +function anchorFindings( + rule: AnyRec, + index: number, + objectsByName: Map, +): SharingRuleEnforceabilityFinding[] { + const object = str(rule.object); + if (!object) return []; + const target = objectsByName.get(object); + if (!target) return []; + + const name = str(rule.name) || String(index); + const where = `sharing rule "${name}" on object "${object}"`; + const path = `sharingRules[${index}].object`; + const owd = target.sharingModel; + + // Mirrors `inertGrantReason`'s own order: the `controlled_by_parent` test + // runs FIRST and returns its own reason, before the `effectiveSharingModel` + // test that also maps that value to `public`. Same order here, so the author + // gets the specific fix-it rather than the generic one. + if (owd === 'controlled_by_parent') { + const master = masterOf(target); + return [{ + severity: 'error', + rule: SHARING_RULE_OBJECT_CONTROLLED_BY_PARENT, + where, + path, + message: + `Sharing rule "${name}" is anchored on object "${object}", which declares ` + + `sharingModel 'controlled_by_parent'. A detail record has no record-level access of its own — ` + + `its visibility is DERIVED from its master (ADR-0055), so it holds no shares to widen. ` + + `\`SharingService.assertNotInertGrant\` refuses the grant with ` + + `SHARING_NOT_ENABLED ("'${object}' is controlled by its parent (master-detail); share the ` + + `master record instead"), so the rule's boot backfill fails, no \`sys_record_share\` row is ` + + `ever written, and the recipients this rule names get whatever the MASTER grants them — ` + + `which may be nothing. The grant is declared and does not exist.`, + hint: + `Move the rule onto the MASTER object` + + (master ? ` — "${object}" derives from "${master}" through its master_detail field, so share ` + + `"${master}" and the detail rows follow` : `, and share that instead; the detail rows follow`) + + `. If "${object}" is meant to carry a record-level baseline of its own, that is a different ` + + `decision: change its sharingModel to 'private' (owner + shares) or 'public_read', and this ` + + `rule becomes enforceable where it stands.`, + }]; + } + + if (effectiveSharingModelOf(target) !== 'public') return []; + + // The remaining way to reach `public`: an explicit `public_read_write`, or an + // absent OWD on a SYSTEM object (ADR-0090 D1 keeps the pre-existing public + // fall-through for `isSystem` / `sys_*`; a CUSTOM object with no OWD fails + // closed to `private`, so it is NOT reported here). + const declared = + owd === 'public_read_write' + ? `declares sharingModel 'public_read_write'` + : `declares no sharingModel and is a system object (\`isSystem: true\` or a \`sys_\` name), ` + + `which ADR-0090 D1 resolves to public`; + + return [{ + severity: 'error', + rule: SHARING_RULE_OBJECT_NOT_SHAREABLE, + where, + path, + message: + `Sharing rule "${name}" is anchored on object "${object}", which ${declared}. Its effective ` + + `sharing model is therefore \`public\`, and sharing only ever WIDENS an OWD baseline — on the ` + + `widest baseline there is nothing left to widen. \`SharingService.assertNotInertGrant\` refuses ` + + `the grant with SHARING_NOT_ENABLED ("'${object}' is not under record-sharing enforcement"), so ` + + `the rule's boot backfill fails and no \`sys_record_share\` row is ever written. Measured: ` + + `\`buildReadFilter\` returns \`null\` for this object, i.e. NO record-level filter at all — every ` + + `principal already reads every row, so this rule advertises a restriction that does not exist.`, + hint: + `Decide which half is wrong. If the ACCESS is right — everyone should read and write these ` + + `records — the rule is dead metadata: delete it (ADR-0049 enforce-or-remove). If the RULE is ` + + `right — only the named audience should reach these records — then "${object}"'s OWD is the ` + + `defect: set sharingModel: 'private' (owner + shares) or 'public_read', and this rule starts ` + + `enforcing. Do NOT re-home the rule onto another public object; that moves the inertness ` + + `instead of removing it.`, + }]; +} + +/** + * Gate stack-declared sharing rules on the two things their runtime consumers + * do with them: lower the `condition` to a `criteria_json` filter, and write a + * `sys_record_share` row on the `object` the rule is anchored to. * * Pure `(stack) => Finding[]`; tolerates the normalized and the parsed tier. */ @@ -182,7 +419,15 @@ export function validateSharingRuleEnforceability(stack: unknown): SharingRuleEn const findings: SharingRuleEnforceabilityFinding[] = []; const cfg = (stack ?? {}) as AnyRec; + const objectsByName = new Map(); + for (const obj of asArray(cfg.objects)) { + const name = str(obj.name); + if (name) objectsByName.set(name, obj); + } + asArray(cfg.sharingRules).forEach((rule, index) => { + anchorFindings(rule, index, objectsByName).forEach((f) => findings.push(f)); + const input = toCompilerInput(rule.condition); if (input === null) return;