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;