diff --git a/.changeset/showcase-sharing-rules-enforce-or-remove.md b/.changeset/showcase-sharing-rules-enforce-or-remove.md new file mode 100644 index 0000000000..41bf92d64c --- /dev/null +++ b/.changeset/showcase-sharing-rules-enforce-or-remove.md @@ -0,0 +1,31 @@ +--- +"@objectstack/example-showcase": patch +--- + +Retire the showcase sharing rules no gate could consult; re-home the position/compound demo (#9237) + +Booting `examples/app-showcase` logged two WARNs per boot — `SharingServicePlugin: boot +rule backfill failed for rule` for `share_open_tasks_with_manager` and +`share_red_projects_with_execs`. Both sat on objects declaring +`sharingModel: 'public_read_write'`, where sharing has nothing left to widen, so +`assertNotInertGrant` (ADR-0111 D7) refused every grant they reconciled. A third rule, +`share_high_value_red_projects_with_managers`, was in exactly the same state and produced +no diagnostic at all: its compound condition matched no seeded row, so `reconcile` never +reached `grant` and never threw. + +`showcase_project` and `showcase_task` are `public_read_write` by deliberate ADR-0090 D1 +declaration and that OWD is load-bearing beyond the security demo, so no rule can ever take +effect there. ADR-0049 enforce-or-remove leaves one honest move, and all three are removed +rather than re-homed onto another public object — the shape the previous repair took, which +moved the inertness instead of removing it. + +The two capabilities they carried are kept: a `position` recipient and a compound CEL +condition (ADR-0058 D3) now live on `share_key_account_qualified_contacts_with_managers`, +targeting `showcase_contact` (OWD `private`, and the `showcase_manager` set grants it +`allowRead` — the object-level bit a share row still needs). The seeded contacts +demonstrate the AND in both directions: rows satisfying either clause alone are not +shared. + +`inert-wirings.test.ts` gains the guard that fails the build on the next such declaration, +in both of its shapes — a rule anchored where the OWD leaves nothing to widen, and a rule +whose audience holds no `allowRead` on the object it shares. diff --git a/docs/qa/platform-checklist/areas/access-security.json b/docs/qa/platform-checklist/areas/access-security.json index aa9dd9774d..4e1196e1a2 100644 --- a/docs/qa/platform-checklist/areas/access-security.json +++ b/docs/qa/platform-checklist/areas/access-security.json @@ -1005,7 +1005,7 @@ "title": "Per-record manual shares grant, scope, and revoke access on a private-OWD record; rule evaluate reconciles the audience", "since": "v16", "status": "active", - "revision": 2, + "revision": 3, "priority": "P1", "surface": "api", "personas": [ @@ -1019,7 +1019,8 @@ "requires": [ "showcase_private_note (sharingModel: private — member_default grants create/read/edit, so a non-owner sees nothing without a share)", "three fresh sign-ups (A/B/C) whose sys_user ids are resolved via a system-context read", - "the seeded criteria sharing rule share_red_projects_with_execs on showcase_project + at least one red-health showcase_project (examples/app-showcase/src/security/sharing-rules.ts) for the evaluate clause" + "the seeded criteria sharing rule share_key_account_qualified_contacts_with_managers on showcase_contact (OWD private) + at least one qualified contact at company Northwind — seeded as Nora West (examples/app-showcase/src/security/sharing-rules.ts) — for the evaluate clause", + "a holder of the manager position, so the rule's recipient expansion is non-empty: without one the reconcile answers grantsCreated 0 and materializes nothing, which is a fixture gap, not a defect" ] }, "steps": [ @@ -1033,7 +1034,7 @@ "as B (a non-manager on A's note): POST /api/v1/data/showcase_private_note//shares granting themselves — must be refused (creating a share is not a reader's power, ADR-0111 D5)", "as A: DELETE /api/v1/data/showcase_private_note//shares/ → 204; then as B: GET the note by id again — refused once more", "revoke-scope probe: attempt DELETE of the same shareId through a DIFFERENT record's path (/data/showcase_private_note//shares/) — must be refused (the URL's object/id is the revoke scope, ADR-0111 D4)", - "as admin: POST /api/v1/sharing/rules/share_red_projects_with_execs/evaluate — capture the reconcile result; read sys_record_share for the matched red project" + "as admin: assign the manager position to one persona (Setup -> Access Control), then POST /api/v1/sharing/rules/share_key_account_qualified_contacts_with_managers/evaluate — capture the reconcile result; read sys_record_share for the matched contact" ], "acceptance": [ { @@ -1073,9 +1074,9 @@ "evidence": "the mis-scoped DELETE trace + survival read" }, { - "clause": "rule evaluate reconciles the audience: POST /sharing/rules/share_red_projects_with_execs/evaluate returns {ruleId, matchedRecords>=1, grantsCreated/grantsUpdated} and a sys_record_share row exists for the matched red project with source 'rule' and source_id = the rule's ROW ID (sys_sharing_rule.id, e.g. srule_…), NOT the rule name", + "clause": "rule evaluate reconciles the audience: POST /sharing/rules/share_key_account_qualified_contacts_with_managers/evaluate returns {ruleId, matchedRecords>=1, grantsCreated/grantsUpdated} and a sys_record_share row exists for the matched contact with source 'rule' and source_id = the rule's ROW ID (sys_sharing_rule.id, e.g. srule_…), NOT the rule name", "oracle": "api", - "verify": "the SharingRuleEvaluationResult body (packages/plugins/plugin-sharing/src/sharing-rule-service.ts evaluateRule) + the materialized rule-sourced share row: assert share.source_id === the sys_sharing_rule row's id, not 'share_red_projects_with_execs'. The row id is the stable FK the reconcile path relies on — purgeRuleGrants(ruleId) deletes on where {source:'rule', source_id: ruleId} with ruleId = rule.id (sharing-rule-service.ts), and sys-sharing-rule.object.ts documents 'source_id={rule.id}' — so a run asserting the NAME here would be asserting a value the implementation never writes", + "verify": "the SharingRuleEvaluationResult body (packages/plugins/plugin-sharing/src/sharing-rule-service.ts evaluateRule) + the materialized rule-sourced share row: assert share.source_id === the sys_sharing_rule row's id, not 'share_key_account_qualified_contacts_with_managers'. The row id is the stable FK the reconcile path relies on — purgeRuleGrants(ruleId) deletes on where {source:'rule', source_id: ruleId} with ruleId = rule.id (sharing-rule-service.ts), and sys-sharing-rule.object.ts documents 'source_id={rule.id}' — so a run asserting the NAME here would be asserting a value the implementation never writes", "evidence": "evaluate response + the sys_record_share read (both the row's source_id and the sys_sharing_rule id it must equal)" } ], @@ -1091,7 +1092,7 @@ "packages/rest/src/rest-server.ts (registerRecordShareEndpoints ~L7246-7331; registerSharingRuleEndpoints evaluate ~L7477-7493)", "packages/plugins/plugin-sharing/src/objects/sys-record-share.object.ts (recipient_type/recipient_id/access_level/source fields)", "packages/plugins/plugin-sharing/src/sharing-rule-service.ts (evaluateRule → SharingRuleEvaluationResult)", - "examples/app-showcase/src/security/sharing-rules.ts (share_red_projects_with_execs seeded criteria rule)", + "examples/app-showcase/src/security/sharing-rules.ts (share_key_account_qualified_contacts_with_managers seeded criteria rule)", "ADR-0111 D1/D4/D5" ], "history": [ @@ -1106,6 +1107,12 @@ "date": "2026-08-11", "change": "clause 6 text corrected against the implementation (run #7637): a rule-materialized share carries source_id = the sys_sharing_rule ROW ID, not the rule name — the stable FK purgeRuleGrants reconciles on. Semantic intent was already right; the asserted value was not", "ref": "#7687" + }, + { + "revision": 3, + "date": "2026-08-18", + "change": "the evaluate clause was UNRUNNABLE as written and had been since it was authored: it drove share_red_projects_with_execs, a rule on showcase_project, whose public_read_write OWD leaves sharing nothing to widen — so evaluate\u0027s reconcile hit assertNotInertGrant and the call was refused (SHARING_NOT_ENABLED, ADR-0111 D7) rather than returning a reconcile result. That rule is retired; the clause now drives share_key_account_qualified_contacts_with_managers on showcase_contact (OWD private), and a step assigns the manager position so the recipient expansion is non-empty", + "ref": "#9237" } ] }, diff --git a/examples/app-showcase/src/security/bind-position-sets.ts b/examples/app-showcase/src/security/bind-position-sets.ts index 6188fda71e..32210c631e 100644 --- a/examples/app-showcase/src/security/bind-position-sets.ts +++ b/examples/app-showcase/src/security/bind-position-sets.ts @@ -29,7 +29,14 @@ * framework cannot infer. */ -const BINDINGS: ReadonlyArray = [ +/** + * The persona bindings, exported because they are the only place that answers + * "which permission sets does a position actually hold?" — the question a + * sharing rule with a `position` recipient has to answer before its grant can + * mean anything (a share row for a principal with no object-level `allowRead` + * is never consulted). `inert-wirings.test.ts` §6 reads this list. + */ +export const POSITION_PERMISSION_SET_BINDINGS: ReadonlyArray = [ ['contributor', 'showcase_contributor'], ['manager', 'showcase_manager'], ['exec', 'showcase_executive'], @@ -72,7 +79,7 @@ async function findOneByName(ctx: BindHostContext, object: string, name: string) export function registerShowcasePositionBindings(ctx: BindHostContext): void { const run = async (): Promise => { let created = 0; - for (const [positionName, setName] of BINDINGS) { + for (const [positionName, setName] of POSITION_PERMISSION_SET_BINDINGS) { const position = await findOneByName(ctx, 'sys_position', positionName); const set = await findOneByName(ctx, 'sys_permission_set', setName); if (!position?.id || !set?.id) { @@ -100,7 +107,7 @@ export function registerShowcasePositionBindings(ctx: BindHostContext): void { }); } } - ctx.logger?.info?.('[showcase] position bindings ensured', { created, total: BINDINGS.length }); + ctx.logger?.info?.('[showcase] position bindings ensured', { created, total: POSITION_PERMISSION_SET_BINDINGS.length }); }; // Bind on `kernel:bootstrapped` — the anchor that fires only after every diff --git a/examples/app-showcase/src/security/index.ts b/examples/app-showcase/src/security/index.ts index d524252b2a..c4acc081d1 100644 --- a/examples/app-showcase/src/security/index.ts +++ b/examples/app-showcase/src/security/index.ts @@ -51,9 +51,7 @@ export { } from './capabilities.js'; export { - RedProjectSharingRule, - HighValueRedProjectRule, NewInquiryFieldOpsRule, - ContributorTaskSharingRule, + KeyAccountQualifiedContactRule, allSharingRules, } from './sharing-rules.js'; diff --git a/examples/app-showcase/src/security/sharing-rules.ts b/examples/app-showcase/src/security/sharing-rules.ts index 701e434c30..4053da1935 100644 --- a/examples/app-showcase/src/security/sharing-rules.ts +++ b/examples/app-showcase/src/security/sharing-rules.ts @@ -11,43 +11,51 @@ * `position` (flat holder expansion) and `unit_and_subordinates` * (business-unit SUBTREE expansion — the unit named by `value` plus every * descendant unit's members, ADR-0057 D5 / ADR-0090 D3). + * + * ## Where a rule may live (ADR-0111 D7 / ADR-0049), and why two were retired + * + * "Sharing only ever WIDENS" has a consequence that is easy to author past: + * on an object whose OWD is already the widest one, there is nothing left to + * widen, so a `sys_record_share` row on it is never consulted by any gate. + * `SharingService.inertGrantReason` states that as a verdict and + * `assertNotInertGrant` REFUSES the write — which is why a rule anchored on + * such an object does not merely under-deliver, it fails its boot backfill: + * + * WARN SharingServicePlugin: boot rule backfill failed for rule + * {"rule":"…","error":"SHARING_NOT_ENABLED: '…' is not under + * record-sharing enforcement (public sharing model or no 'owner_id' + * field); a share row on it would never be consulted"} + * + * `showcase_project` and `showcase_task` are `sharingModel: + * 'public_read_write'` by DELIBERATE declaration — each carries an explicit + * ADR-0090 D1 grandfather stamp, and that OWD is load-bearing well beyond the + * security demo (it is what lets a `showcase_contributor` PATCH a project row + * it did not create, the write floor pinned by + * `owd-public-read-write-write-floor.dogfood.test.ts`). So no sharing rule can + * ever take effect there, and ADR-0049's enforce-or-remove leaves exactly one + * honest move: remove. + * + * Retired here, therefore, and NOT re-homed onto another public object: + * + * • `share_red_projects_with_execs` — criteria rule on `showcase_project`. + * • `share_open_tasks_with_manager` — criteria rule on `showcase_task`. + * This one was ITSELF a repair: it replaced the owner-based + * `share_contributor_tasks_with_manager`, which `type: 'owner'` made + * silently skipped at seed time (ADR-0078: nothing on the authoring + * surface may be silently inert). That repair moved the inertness instead + * of removing it — the replacement validated, seeded, and then failed its + * backfill on every boot. The lesson is written down rather than repeated: + * a sharing-rule demonstration belongs on an object under record-sharing + * ENFORCEMENT, and `inert-wirings.test.ts` §6 now fails the build if a + * future rule lands on a public one again. + * + * The capabilities those two carried — a `position` recipient, and a COMPOUND + * CEL condition (ADR-0058 D3) — are not dropped: both live on + * `KeyAccountQualifiedContactRule` below, on an object where the grant is real. */ import { defineSharingRule } from '@objectstack/spec/security'; -/** criteria-based: red-health projects are shared up to executives. */ -export const RedProjectSharingRule = defineSharingRule({ - type: 'criteria', - name: 'share_red_projects_with_execs', - label: 'Red Projects → Executives', - description: 'Automatically share at-risk (red health) projects with executives.', - object: 'showcase_project', - condition: "record.health == 'red'", - accessLevel: 'read', - sharedWith: { type: 'position', value: 'exec' }, - active: true, -}); - -/** - * [ADR-0058 D3 / closes #1887] criteria-based with a COMPOUND CEL condition. - * Before #1887 a multi-clause `&&` condition was silently skipped (the sharing - * rule was decorative metadata); now it compiles to a compound `criteria_json` - * and enforces. Shares only projects that are BOTH at-risk (red) AND high-budget - * with managers — the AND matters: a red but low-budget project is NOT shared. - */ -export const HighValueRedProjectRule = defineSharingRule({ - type: 'criteria', - name: 'share_high_value_red_projects_with_managers', - label: 'High-Value Red Projects → Managers', - description: - 'Share at-risk (red health) projects over the budget threshold with managers (compound condition, ADR-0058 D3).', - object: 'showcase_project', - condition: "record.health == 'red' && record.budget > 100000", - accessLevel: 'read', - sharedWith: { type: 'position', value: 'manager' }, - active: true, -}); - /** * Business-unit SUBTREE recipient (`unit_and_subordinates`): new inquiries are * shared for triage with everyone in the Field Operations unit — AND every @@ -72,31 +80,40 @@ export const NewInquiryFieldOpsRule = defineSharingRule({ }); /** - * criteria-based: open (not-done) tasks are shared read-only with managers - * for oversight. + * [ADR-0058 D3 / closes #1887] criteria-based with a COMPOUND CEL condition, + * and the showcase's `position`-recipient demonstration. * - * This replaces the retired owner-based `share_contributor_tasks_with_manager` - * demonstration rule: `type: 'owner'` (`ownedBy`) no longer parses — it - * depended on live position membership, which the static materialiser cannot - * track, so it validated but was silently skipped at seed time (ADR-0078: - * nothing on the authoring surface may be silently inert). The enforced - * equivalent is a criteria rule scoping the rows managers need. + * Before #1887 a multi-clause `&&` condition was silently skipped (the sharing + * rule was decorative metadata); now it compiles to a compound `criteria_json` + * and enforces. The AND matters, and the seed data demonstrates it in BOTH + * directions rather than one: of the seeded contacts, three are `qualified` + * (only one of them at Northwind) and many are at Northwind (none of those + * others `qualified`) — so a row satisfying either clause alone is NOT shared, + * and exactly the row satisfying both is. + * + * ## Why this object and this recipient + * + * `showcase_contact` is OWD `private`, so the grant is one the read gate + * actually consults (ADR-0111 D7), and the `showcase_manager` permission set + * grants `showcase_contact.allowRead` — the object-level bit a share row still + * needs before any record-level widening can be observed. Both halves are + * pinned by `inert-wirings.test.ts` §6, which is the guard that would have + * caught the two rules retired above at authoring time. */ -export const ContributorTaskSharingRule = defineSharingRule({ +export const KeyAccountQualifiedContactRule = defineSharingRule({ type: 'criteria', - name: 'share_open_tasks_with_manager', - label: 'Open Tasks → Manager', - description: 'Share open (not-done) tasks with managers for oversight.', - object: 'showcase_task', - condition: 'record.done == false', + name: 'share_key_account_qualified_contacts_with_managers', + label: 'Key-Account Qualified Contacts → Managers', + description: + 'Share qualified contacts at the key account with managers (compound condition, ADR-0058 D3).', + object: 'showcase_contact', + condition: "record.stage == 'qualified' && record.company == 'Northwind'", accessLevel: 'read', sharedWith: { type: 'position', value: 'manager' }, active: true, }); export const allSharingRules = [ - RedProjectSharingRule, - HighValueRedProjectRule, NewInquiryFieldOpsRule, - ContributorTaskSharingRule, + KeyAccountQualifiedContactRule, ]; diff --git a/examples/app-showcase/test/inert-wirings.test.ts b/examples/app-showcase/test/inert-wirings.test.ts index b7aa966235..01752d75e0 100644 --- a/examples/app-showcase/test/inert-wirings.test.ts +++ b/examples/app-showcase/test/inert-wirings.test.ts @@ -6,6 +6,7 @@ import stack from '../objectstack.config.js'; import { PLATFORM_CAPABILITY_NAMES } from '@objectstack/spec/security'; import { FILE_REFERENCE_TYPES, valueSchemaFor } from '@objectstack/spec/data'; import { healthFor, sweepProjectHealth, bindShowcaseJobRuntime } from '../src/automation/jobs/index.js'; +import { POSITION_PERMISSION_SET_BINDINGS } from '../src/security/bind-position-sets.js'; import { ADMIN_EMAIL, PHONE_DEMO_USER, @@ -532,3 +533,120 @@ describe('seeded notify recipients resolve to a real user (#7746)', () => { ); }); }); + +// ─────────────────────────────────────────────────────────────────────────── +// 6. Sharing rules — the grant must be one a gate would CONSULT (#9237) +// ─────────────────────────────────────────────────────────────────────────── +/** + * The same bug class as §1-§5, on the security surface: a declaration the + * runtime accepts at authoring time and then refuses at boot, announced only + * by a line in the boot warning block. + * + * Sharing only ever WIDENS an object's OWD baseline, so on an object whose OWD + * is already the widest there is nothing to widen and a `sys_record_share` row + * would never be consulted. `SharingService.inertGrantReason` states that as a + * verdict and `assertNotInertGrant` REFUSES the write, so the rule's boot + * backfill fails per rule: + * + * WARN SharingServicePlugin: boot rule backfill failed for rule + * {"rule":"share_open_tasks_with_manager","error":"SHARING_NOT_ENABLED: + * 'showcase_task' is not under record-sharing enforcement …"} + * + * Measured on the stock showcase before #9237: TWO such WARNs per boot, and a + * THIRD rule in the same state that produced no diagnostic at all — its + * compound condition matched zero seeded rows, so `reconcile` never reached + * `grant` and never threw. The silent one is the reason this guard reads the + * DECLARATION rather than the boot log: a rule can be just as dead without a + * warning to notice. + * + * ⛔ The runtime's other inertness arm — "no `owner_id` field" — is + * deliberately NOT reproduced here. `owner_id` is INJECTED by the schema + * registry for ordinary business objects, so it is absent from the authored + * metadata this guard reads and present on the schema the runtime judges; + * asserting it here would fail every object that (correctly) does not declare + * it by hand. + */ +describe('sharing rules target an object under record-sharing enforcement (#9237)', () => { + interface AuthoredRule { + name: string; + object: string; + sharedWith?: { type?: string; value?: string }; + } + interface AuthoredSet { + name: string; + isDefault?: boolean; + objects?: Record; + } + + const rules = ((stack as { sharingRules?: unknown[] }).sharingRules ?? []) as AuthoredRule[]; + const objects = ((stack as { objects?: unknown[] }).objects ?? []) as Array<{ + name: string; + sharingModel?: string; + }>; + const sets = ((stack as { permissions?: unknown[] }).permissions ?? []) as AuthoredSet[]; + + /** The permission sets a holder of `position` effectively carries. */ + function setsHeldBy(position: string): AuthoredSet[] { + const held = new Set( + POSITION_PERMISSION_SET_BINDINGS.filter(([p]) => p === position).map(([, s]) => s), + ); + // Every authenticated member also holds the `everyone` baseline (the + // `isDefault` set, ADR-0090 D5) IN ADDITION to their explicit grants. + return sets.filter((s) => held.has(s.name) || s.isDefault === true); + } + + it('the stack declares sharing rules (guard is not vacuous)', () => { + expect(rules.length).toBeGreaterThan(0); + }); + + it('no rule is anchored on an object whose OWD leaves nothing to widen', () => { + // `effectiveSharingModel` maps BOTH of these to 'public'; the + // `controlled_by_parent` case has its own refusal ("share the master + // record instead"). Either way the grant is refused, so either way the + // declaration is inert. + const INERT_OWD = new Set(['public_read_write', 'controlled_by_parent']); + const offenders: string[] = []; + for (const rule of rules) { + const target = objects.find((o) => o.name === rule.object); + if (!target) { + offenders.push(`${rule.name} → '${rule.object}' (no such object)`); + continue; + } + if (INERT_OWD.has(String(target.sharingModel))) { + offenders.push(`${rule.name} → ${rule.object} (sharingModel '${target.sharingModel}')`); + } + } + expect( + offenders, + `sharing rule(s) on an object not under record-sharing enforcement — the boot backfill ` + + `refuses these with SHARING_NOT_ENABLED: ${offenders.join(', ')}`, + ).toEqual([]); + }); + + it('every rule names an audience that can READ the object it shares', () => { + // The second way a grant goes unconsulted: object-level CRUD is decided + // BEFORE record-level visibility, so a share row minted for a principal + // holding no `allowRead` on that object widens nothing. This arm is what + // stops the retired project/task rules being "fixed" by re-homing them + // onto a private object whose recipient cannot read it either. + const offenders: string[] = []; + for (const rule of rules) { + const recipient = rule.sharedWith ?? {}; + // Non-`position` recipients expand to ordinary members, whose grant is + // the `everyone` baseline — represented by the isDefault set. + const candidates = + recipient.type === 'position' + ? setsHeldBy(String(recipient.value)) + : sets.filter((s) => s.isDefault === true); + const canRead = candidates.some((s) => s.objects?.[rule.object]?.allowRead === true); + if (!canRead) { + offenders.push(`${rule.name} → ${recipient.type}:${recipient.value} on ${rule.object}`); + } + } + expect( + offenders, + `sharing rule(s) whose audience holds no allowRead on the shared object — the share row ` + + `is never reached: ${offenders.join(', ')}`, + ).toEqual([]); + }); +}); diff --git a/packages/qa/dogfood/test/org-scoped-sharing-rule-listing.dogfood.test.ts b/packages/qa/dogfood/test/org-scoped-sharing-rule-listing.dogfood.test.ts index 66af2541a4..cb37856855 100644 --- a/packages/qa/dogfood/test/org-scoped-sharing-rule-listing.dogfood.test.ts +++ b/packages/qa/dogfood/test/org-scoped-sharing-rule-listing.dogfood.test.ts @@ -44,12 +44,18 @@ import { bootStack, type VerifyStack } from '@objectstack/verify'; const RULES = '/sharing/rules'; const SYS = { isSystem: true } as const; -/** The four rules `examples/app-showcase` declares, all seeded org-less. */ +/** + * The rules `examples/app-showcase` declares, all seeded org-less. + * + * [#9237] Two of the former four were retired rather than re-homed: they sat + * on `showcase_project` / `showcase_task`, whose `public_read_write` OWD + * leaves sharing nothing to widen, so their boot backfill was refused + * (`SHARING_NOT_ENABLED`) on every boot. What this file measures — an org-bound + * admin's READ over org-less rows — is unchanged by their number. + */ const SEEDED_RULE_NAMES = [ - 'share_red_projects_with_execs', - 'share_high_value_red_projects_with_managers', 'share_new_inquiries_with_field_ops', - 'share_open_tasks_with_manager', + 'share_key_account_qualified_contacts_with_managers', ]; interface RuleRow { diff --git a/packages/qa/dogfood/test/showcase-d3-d4-capabilities.dogfood.test.ts b/packages/qa/dogfood/test/showcase-d3-d4-capabilities.dogfood.test.ts index 68c29b7d24..f3d6427303 100644 --- a/packages/qa/dogfood/test/showcase-d3-d4-capabilities.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-d3-d4-capabilities.dogfood.test.ts @@ -4,6 +4,16 @@ // • ADR-0058 D3 / #1887 — a COMPOUND sharing `condition` (`&&`) compiles to a // compound criteria_json and enforces (the AND matters; before #1887 it was // silently skipped). +// +// [#9237] The compound rule moved from `showcase_project` to +// `showcase_contact`, and the D3 proof got its missing half with it. On +// `showcase_project` (OWD `public_read_write`) sharing has nothing to +// widen, so every grant the rule reconciled was REFUSED +// (`SHARING_NOT_ENABLED`, ADR-0111 D7) — this file could assert the +// criteria compiled and matched, and never that anything was ENFORCED, +// while its own header claimed both. `showcase_contact` is OWD `private`, +// so the grant lands and the enforcement half is now measured rather than +// implied. // • ADR-0058 D4 — an RLS `check` clause validates the write POST-IMAGE: a // contributor cannot reassign an invoice they own to a different owner. // @@ -16,6 +26,8 @@ import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plug import { PermissionSetSchema } from '@objectstack/spec/security'; const MEMBER = 'd34-member@verify.test'; +/** The showcase's compound (ADR-0058 D3) sharing rule — re-homed by #9237. */ +const RULE = 'share_key_account_qualified_contacts_with_managers'; const SYS = { context: { isSystem: true } } as const; // Member set: invoice CRUD + an OWNER read/pre-image policy AND the D4 CHECK @@ -25,7 +37,7 @@ const memberSet = PermissionSetSchema.parse({ name: 'showcase_d34_member', label: 'D3/D4 Member', objects: { - showcase_project: { allowRead: true }, + showcase_contact: { allowRead: true }, showcase_invoice: { allowRead: true, allowCreate: true, allowEdit: true }, }, rowLevelSecurity: [ @@ -55,10 +67,12 @@ describe('showcase: D3 compound sharing (#1887) + D4 RLS check', () => { const idOf = (r: any) => r?.id ?? r?.record?.id ?? r; const acct = idOf(await ql.insert('showcase_account', { name: 'D34 Co', status: 'prospect' }, SYS)); - // Three projects: only the at-risk (red) AND high-budget (>100k) one matches. - await ql.insert('showcase_project', { id: 'pj_red_hi', name: 'Red-Hi', account: acct, status: 'planned', health: 'red', budget: 250000 }, SYS); - await ql.insert('showcase_project', { id: 'pj_red_lo', name: 'Red-Lo', account: acct, status: 'planned', health: 'red', budget: 40000 }, SYS); - await ql.insert('showcase_project', { id: 'pj_grn_hi', name: 'Grn-Hi', account: acct, status: 'planned', health: 'green', budget: 250000 }, SYS); + // Three contacts: only the `qualified` AND key-account ('Northwind') one + // matches. The other two each satisfy exactly ONE clause, so the AND is + // measured in both directions rather than one. + await ql.insert('showcase_contact', { id: 'ct_q_key', name: 'Q-Key', email: 'q.key@d34.example', company: 'Northwind', stage: 'qualified' }, SYS); + await ql.insert('showcase_contact', { id: 'ct_q_other', name: 'Q-Other', email: 'q.other@d34.example', company: 'Contoso', stage: 'qualified' }, SYS); + await ql.insert('showcase_contact', { id: 'ct_new_key', name: 'New-Key', email: 'new.key@d34.example', company: 'Northwind', stage: 'new' }, SYS); // An invoice owned by the member (system insert sidesteps authoring rules). invId = idOf(await ql.insert('showcase_invoice', { name: 'INV-D34', account: acct, owner: MEMBER, status: 'draft' }, SYS)); @@ -68,26 +82,54 @@ describe('showcase: D3 compound sharing (#1887) + D4 RLS check', () => { // ── ADR-0058 D3 / #1887 ──────────────────────────────────────────────────── it('compound sharing condition is SEEDED as a compound criteria_json (not skipped)', async () => { - const rule = await ql.findOne('sys_sharing_rule', { where: { name: 'share_high_value_red_projects_with_managers' }, context: SYS.context }); + const rule = await ql.findOne('sys_sharing_rule', { where: { name: RULE }, context: SYS.context }); expect(rule, 'compound rule was seeded').toBeTruthy(); expect(JSON.parse(rule.criteria_json)).toEqual({ - $and: [{ health: 'red' }, { budget: { $gt: 100000 } }], + $and: [{ stage: 'qualified' }, { company: 'Northwind' }], }); }); - it('the compound criteria_json matches ONLY the project satisfying BOTH clauses', async () => { - const rule = await ql.findOne('sys_sharing_rule', { where: { name: 'share_high_value_red_projects_with_managers' }, context: SYS.context }); + it('the compound criteria_json matches ONLY the contact satisfying BOTH clauses', async () => { + const rule = await ql.findOne('sys_sharing_rule', { where: { name: RULE }, context: SYS.context }); const criteria = JSON.parse(rule.criteria_json); - // Apply the SEEDED compound criteria to our three projects: only red-AND-high passes. - const hit = await ql.find('showcase_project', { - where: { $and: [criteria, { id: { $in: ['pj_red_hi', 'pj_red_lo', 'pj_grn_hi'] } }] }, + // Apply the SEEDED compound criteria to our three contacts: only qualified-AND-key passes. + const hit = await ql.find('showcase_contact', { + where: { $and: [criteria, { id: { $in: ['ct_q_key', 'ct_q_other', 'ct_new_key'] } }] }, fields: ['id'], context: SYS.context, }); - expect((hit ?? []).map((r: any) => r.id).sort()).toEqual(['pj_red_hi']); + expect((hit ?? []).map((r: any) => r.id).sort()).toEqual(['ct_q_key']); // The rule also evaluates end-to-end (matched count includes seed data). const rules: any = stack.kernel.getService('sharingRules'); - const res = await rules.evaluateRule('share_high_value_red_projects_with_managers', SYS.context); - expect(res.matchedRecords, 'rule is evaluable + at least Red-Hi matches').toBeGreaterThanOrEqual(1); + const res = await rules.evaluateRule(RULE, SYS.context); + expect(res.matchedRecords, 'rule is evaluable + at least Q-Key matches').toBeGreaterThanOrEqual(1); + }); + + it('[#9237] the grant the rule reconciles is one the gates CONSULT — not refused as inert', async () => { + // The half this proof could never carry on `showcase_project`. The verdict + // is caller-independent (ADR-0111 D7) and computed from the OWD alone, so + // it is asserted directly rather than inferred from a reconcile count that + // an empty recipient expansion would leave silently at zero — which is + // exactly how the sibling rule on `showcase_project` produced no boot WARN + // while being just as dead. + const sharing: any = await stack.kernel.getServiceAsync('sharing'); + const granted = await sharing.grant( + { object: 'showcase_contact', recordId: 'ct_q_key', recipientType: 'user', recipientId: 'u_d34_probe', accessLevel: 'read' }, + { isSystem: true }, + ); + expect(granted, 'a share row on a private-OWD object is accepted').toBeTruthy(); + + // The contrast that names the retired rules' defect, on the object they + // used to sit on: same call, same caller, refused for the OWD alone. The + // record id deliberately names no row — the D7 verdict is computed from + // the OBJECT before any record is read, which is precisely why it is + // caller- and record-independent. Both the code and the object are + // asserted, so a refusal arriving for some other reason cannot pass. + await expect( + sharing.grant( + { object: 'showcase_project', recordId: 'pj_absent', recipientType: 'user', recipientId: 'u_d34_probe', accessLevel: 'read' }, + { isSystem: true }, + ), + ).rejects.toThrow(/SHARING_NOT_ENABLED: 'showcase_project' is not under record-sharing enforcement/); }); // ── ADR-0058 D4 ──────────────────────────────────────────────────────────── diff --git a/packages/qa/dogfood/test/showcase-declarative-rbac-seeding.dogfood.test.ts b/packages/qa/dogfood/test/showcase-declarative-rbac-seeding.dogfood.test.ts index 350778d655..f50a21936e 100644 --- a/packages/qa/dogfood/test/showcase-declarative-rbac-seeding.dogfood.test.ts +++ b/packages/qa/dogfood/test/showcase-declarative-rbac-seeding.dogfood.test.ts @@ -35,31 +35,47 @@ describe('showcase: declarative RBAC seeding (ADR-0057 D6 / #2077)', () => { it('declared criteria sharing rule lands in sys_sharing_rule, CEL→criteria_json translated', async () => { const rules = await ql.find('sys_sharing_rule', { where: {}, context: { isSystem: true } }); - const red = (rules ?? []).find((r: any) => r.name === 'share_red_projects_with_execs'); - expect(red, 'criteria rule seeded (was count = 0)').toBeTruthy(); - expect(red.object_name).toBe('showcase_project'); - expect(red.recipient_type).toBe('position'); - expect(red.recipient_id).toBe('exec'); - // condition "record.health == 'red'" → JSON FilterCondition { health: 'red' } - const criteria = JSON.parse(red.criteria_json); - expect(criteria).toEqual({ health: 'red' }); + const inquiry = (rules ?? []).find((r: any) => r.name === 'share_new_inquiries_with_field_ops'); + expect(inquiry, 'criteria rule seeded (was count = 0)').toBeTruthy(); + expect(inquiry.object_name).toBe('showcase_inquiry'); + expect(inquiry.recipient_type).toBe('unit_and_subordinates'); + expect(inquiry.recipient_id).toBe('bu_field_ops'); + // condition "record.status == 'new'" → JSON FilterCondition { status: 'new' } + const criteria = JSON.parse(inquiry.criteria_json); + expect(criteria).toEqual({ status: 'new' }); }); - it('retired owner-based rule is gone; its criteria replacement seeds and enforces', async () => { + it('every retired demonstration rule stays gone; the position demo seeds where it enforces', async () => { const rules = await ql.find('sys_sharing_rule', { where: {}, context: { isSystem: true } }); - // `type: 'owner'` was removed from the authoring spec (never enforced — - // ADR-0078): the old demonstration rule can no longer exist. - const owner = (rules ?? []).find((r: any) => r.name === 'share_contributor_tasks_with_manager'); - expect(owner, 'retired owner-based rule must not reappear').toBeFalsy(); - // Its replacement is a real criteria rule — seeded WITH translated - // criteria_json (not skipped, not match-all). - const open = (rules ?? []).find((r: any) => r.name === 'share_open_tasks_with_manager'); - expect(open, 'criteria replacement seeded').toBeTruthy(); - expect(open.object_name).toBe('showcase_task'); - expect(open.recipient_type).toBe('position'); - expect(open.recipient_id).toBe('manager'); - const criteria = JSON.parse(open.criteria_json); - expect(criteria).toEqual({ done: false }); + // Three generations of the same ADR-0078 lesson, each retired for being + // inert in a different way, none of which may reappear: + // • `type: 'owner'` never parsed — silently skipped at seed time; + // • its criteria replacement on `showcase_task`, and the two rules on + // `showcase_project`, seeded fine and then had every grant REFUSED + // (`SHARING_NOT_ENABLED`) because those objects are `public_read_write` + // — sharing has nothing to widen there (#9237). + for (const retired of [ + 'share_contributor_tasks_with_manager', + 'share_open_tasks_with_manager', + 'share_red_projects_with_execs', + 'share_high_value_red_projects_with_managers', + ]) { + expect( + (rules ?? []).find((r: any) => r.name === retired), + `retired rule must not reappear: ${retired}`, + ).toBeFalsy(); + } + // The surviving `position`-recipient demonstration lives on an object under + // record-sharing enforcement, so its grant is one the gates consult. + const contacts = (rules ?? []).find( + (r: any) => r.name === 'share_key_account_qualified_contacts_with_managers', + ); + expect(contacts, 'position-recipient rule seeded').toBeTruthy(); + expect(contacts.object_name).toBe('showcase_contact'); + expect(contacts.recipient_type).toBe('position'); + expect(contacts.recipient_id).toBe('manager'); + const criteria = JSON.parse(contacts.criteria_json); + expect(criteria).toEqual({ $and: [{ stage: 'qualified' }, { company: 'Northwind' }] }); }); it('re-seed is idempotent (no duplicate rows on a second boot)', async () => {