From 89daeb07f1fd106e3d0e0d555fa63b0ac58902c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 06:06:31 +0000 Subject: [PATCH 1/5] feat(plugin-sharing): expand the `field` sharing recipient per matched record The services half of the #14103 ruling (B): `expandRecipient` becomes per-record for the `field` member only. The per-record pass reads the named user-typed column off the matched row (one query for match and recipients), honours `multiple: true`, and fails closed on an empty column. The whole-rule pass (`evaluateRule`, behind the background re-grant, the boot backfill and the REST evaluate endpoint) derives per-record pairs instead of a matched-records x recipients product; the recipient-axis revoke declines `field` rules; the rule-wide switch refuses the member loudly. The bootstrap seeds `field` rules, the `sys_sharing_rule` select stores them, `defineRule` holds the recipient to the field-name grammar, and the "until the executor lands" doc sentences are retired. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .changeset/plugin-sharing-field-recipient.md | 38 + .../docs/permissions/permissions-matrix.mdx | 2 +- content/docs/permissions/sharing-rules.mdx | 2 +- content/docs/protocol/objectql/security.mdx | 4 +- .../src/bootstrap-declared-sharing-rules.ts | 6 +- .../plugin-sharing/src/bu-tree-recompute.ts | 5 +- .../src/field-recipient.test.ts | 810 ++++++++++++++++++ .../src/objects/sys-sharing-rule.object.ts | 19 +- .../src/sharing-rule-service.ts | 336 +++++++- 9 files changed, 1183 insertions(+), 39 deletions(-) create mode 100644 .changeset/plugin-sharing-field-recipient.md create mode 100644 packages/plugins/plugin-sharing/src/field-recipient.test.ts diff --git a/.changeset/plugin-sharing-field-recipient.md b/.changeset/plugin-sharing-field-recipient.md new file mode 100644 index 0000000000..89177fd28b --- /dev/null +++ b/.changeset/plugin-sharing-field-recipient.md @@ -0,0 +1,38 @@ +--- +"@objectstack/plugin-sharing": minor +--- + +feat(plugin-sharing): the `field` sharing recipient is enforced — expanded once per matched record + +`ShareRecipientType` gained `field` on the spec side (#14103, maintainer ruling +B): `sharedWith: { type: 'field', value: '' }` shares each +record the rule's criteria match with the user or users named by that column +on the record. This is the executor half (#15072): + +- `SharingRuleService` reads the named user-typed column on each matched + record. A `multiple: true` column shares with every user it names; a single- + user column with the one it names. **Fail-closed on empty**: a null or empty + column materialises no grant — never a match-all principal, never a fallback + to the record owner. `field` is the only recipient resolved per record; every + other kind (`user`, `team`, `position`, `business_unit`, + `unit_and_subordinates`) still expands once per rule. +- The grants re-materialise on the record's own write: the existing + `afterUpdate` hook has no changed-field gating, so an update that touches only + the recipient column re-runs the per-record reconcile, which revokes the + stale grant and materialises the new one. No second trigger was added. +- The whole-rule pass (`evaluateRule` — the background re-grant after an + unbounded bulk write, the `kernel:bootstrapped` backfill and the REST evaluate + endpoint) derives per-record (record, user) pairs for a `field` rule instead + of a matched-records × recipients product, so the rule is as correct after a + bulk write and a restart as it is inline. The recipient-axis revoke + (`revokeRuleGrantsForRetiredRecipients`) declines `field` rules — they have no + rule-wide recipient set to retire against. +- The declared-rule bootstrap seeds `field` rules (previously skipped with a + warning), the `sys_sharing_rule.recipient_type` select accepts `field`, and + `defineRule` refuses a `field` recipient whose `recipientId` is not a field + name (the same grammar the spec applies at parse). +- An active `field` rule whose column the object does not declare as user-typed + grants nobody and says so once per rule. + +There is no `manager` recipient: "the owner's manager" is a user field the +application stores on the record, named by a `field` recipient. diff --git a/content/docs/permissions/permissions-matrix.mdx b/content/docs/permissions/permissions-matrix.mdx index c4188af9b6..62698d3d32 100644 --- a/content/docs/permissions/permissions-matrix.mdx +++ b/content/docs/permissions/permissions-matrix.mdx @@ -152,7 +152,7 @@ Sharing rules extend access beyond ownership and the depth axis. The declarative | **Criteria-Based** | `criteria` | Share records matching a CEL predicate over field values | All opportunities where `record.amount > 100000` are shared with "VP Sales" | -**Enforcement status:** every authorable rule type is enforced, and so is every recipient type but the newest: the `field` recipient (#14103) parses, and until its per-record executor (#15072) lands the declared-rule bootstrap skips such a rule with a logged warning — never silently, never as a wider grant. v17 reconciled the surface with the runtime (#1878): `owner`-type rules (`type: 'owner'`, `ownedBy`) and `group` / `guest` recipients — previously declared but skipped at seed time — **no longer parse**; `group` became the enforced `team` and `business_unit` joined the enum. That is a stronger statement than "declared but not enforced": a skipped rule is still authorable and is ignored, whereas a removed one is rejected by `SharingRuleSchema`, so a stale definition fails loudly at authoring time instead of silently doing nothing (ADR-0078). See [Sharing Rules](/docs/permissions/sharing-rules#recipient-types). +**Enforcement status:** every authorable rule type is enforced, and so is every recipient type — including the `field` recipient (#14103), expanded once per matched record by its executor (#15072). v17 reconciled the surface with the runtime (#1878): `owner`-type rules (`type: 'owner'`, `ownedBy`) and `group` / `guest` recipients — previously declared but skipped at seed time — **no longer parse**; `group` became the enforced `team` and `business_unit` joined the enum. That is a stronger statement than "declared but not enforced": a skipped rule is still authorable and is ignored, whereas a removed one is rejected by `SharingRuleSchema`, so a stale definition fails loudly at authoring time instead of silently doing nothing (ADR-0078). See [Sharing Rules](/docs/permissions/sharing-rules#recipient-types). diff --git a/content/docs/permissions/sharing-rules.mdx b/content/docs/permissions/sharing-rules.mdx index b80a822191..453dcec347 100644 --- a/content/docs/permissions/sharing-rules.mdx +++ b/content/docs/permissions/sharing-rules.mdx @@ -143,7 +143,7 @@ for the first five, once per **matched record** for `field`: | `position` | Everyone assigned that position (flat expansion — positions have no tree) | | `unit_and_subordinates` | Everyone in that **business unit and every unit beneath it** (the BU tree is the one hierarchy — ADR-0090 D3) | | `business_unit` | Everyone in exactly that business unit (no subtree) | -| `field` | The user or users named by a **user-typed field on each matched record** — `value` is that field's name (`assignees`). A `multiple: true` field shares with every user it holds; an empty column shares with nobody. There is deliberately no `manager` recipient: "the owner's manager" is a user field the application stores on the record, named here (maintainer ruling 2026-09-02, objectstack#14103). The per-record executor is objectstack#15072 — until it lands, a `field` rule is skipped with a logged warning at seed, never seeded wider | +| `field` | The user or users named by a **user-typed field on each matched record** — `value` is that field's name (`assignees`). A `multiple: true` field shares with every user it holds; an empty column shares with nobody. There is deliberately no `manager` recipient: "the owner's manager" is a user field the application stores on the record, named here (maintainer ruling 2026-09-02, objectstack#14103). Expanded once per matched record by the sharing-rule executor (objectstack#15072), and re-materialised when the record's own write changes that field | A criteria `condition` must be compilable by the CEL → filter pushdown compiler. A condition the compiler cannot lower is **skipped and logged — diff --git a/content/docs/protocol/objectql/security.mdx b/content/docs/protocol/objectql/security.mdx index e83cb601d3..4a3ff36cd2 100644 --- a/content/docs/protocol/objectql/security.mdx +++ b/content/docs/protocol/objectql/security.mdx @@ -383,7 +383,7 @@ sharedWith: `unit_and_subordinates` expands a **business-unit subtree**: the unit named by `value` plus every descendant unit's members (ADR-0057 D5 / ADR-0090 D3 — the former position-tree walk was re-homed onto the `sys_business_unit` tree). -`field` is the **record-relative** recipient (#14103, maintainer ruling 2026-09-02): `value` names a user-typed field on the object, and each matched record is shared with the user or users that column holds on it (`multiple: true` shares with every user it names; an empty column shares with nobody). It expands once per matched record, not once per rule. There is no `manager` recipient — "the owner's manager" is a user field the application stores on the record, named by a `field` recipient. The per-record executor is #15072. +`field` is the **record-relative** recipient (#14103, maintainer ruling 2026-09-02): `value` names a user-typed field on the object, and each matched record is shared with the user or users that column holds on it (`multiple: true` shares with every user it names; an empty column shares with nobody). It expands once per matched record, not once per rule. There is no `manager` recipient — "the owner's manager" is a user field the application stores on the record, named by a `field` recipient. The executor (#15072) derives the rule's grants from each matched record's column, on the record's own write and on every whole-rule pass alike. ### Owner-Based Sharing — removed in v17 @@ -419,7 +419,7 @@ sharedWith: value: west_region_managers ``` -> **Enforcement status.** Criteria rules with `user` / `team` / `position` / `unit_and_subordinates` / `business_unit` recipients compile and enforce (the CEL condition lowers to a runtime filter that materializes `sys_record_share` grants, ADR-0058 D3). The `field` recipient is the contract half of a two-part landing (#14103): it parses, and until its per-record executor (#15072) lands the declared-rule bootstrap skips such a rule with a logged warning — never silently, never as a wider grant. Owner-type rules and the `group` / `guest` recipients are **not** `[experimental — not enforced]` and are no longer skipped at seed time — v17 removed them from the schema, so they do not parse at all (see above). What is still skipped-and-logged is a `condition` the compiler cannot lower (functions, cross-object traversal): it is never seeded as a permissive match-all (ADR-0049). +> **Enforcement status.** Criteria rules with `user` / `team` / `position` / `unit_and_subordinates` / `business_unit` / `field` recipients compile and enforce (the CEL condition lowers to a runtime filter that materializes `sys_record_share` grants, ADR-0058 D3); the `field` recipient (#14103) is expanded once per matched record rather than once per rule (#15072). Owner-type rules and the `group` / `guest` recipients are **not** `[experimental — not enforced]` and are no longer skipped at seed time — v17 removed them from the schema, so they do not parse at all (see above). What is still skipped-and-logged is a `condition` the compiler cannot lower (functions, cross-object traversal): it is never seeded as a permissive match-all (ADR-0049). > `accessLevel` is one of `read` or `edit`. Sharing widens **which rows** a principal reaches, never **which verbs** they may use — an `edit` share opens *update*, not *delete*: delete comes from ownership, the ADR-0057 DEPTH scopes, or the `modifyAllRecords` bypass, enforced by the sharing layer's own `canDelete` gate (distinct from the `canEdit` update gate) on top of the object-level CRUD gate (ADR-0111 D3). A third level `full` ("Full Access — transfer/share/delete") was authorable through protocol 16 but never granted any of those verbs: both enforcement sites matched `edit`/`full` alike, so it was equivalent to `edit` while telling admins otherwise, and it was removed (#3865, ADR-0078). Stacks still authoring it are rewritten to `edit` at load by the `sharing-rule-access-level-full-to-edit` conversion. diff --git a/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts b/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts index d93189485f..ea2cfbe7d0 100644 --- a/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts +++ b/packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts @@ -9,7 +9,7 @@ * (`criteria_json` JSON filter + `recipient_type`/`recipient_id`). ADR-0057 D6 * makes the RUNTIME shape canonical and translates the authorable fields. * Every currently-authorable recipient (`user` / `team` / `position` / - * `unit_and_subordinates` / `business_unit`) maps 1:1 and ENFORCES — the + * `unit_and_subordinates` / `business_unit` / `field`) maps 1:1 and ENFORCES — the * retired `group`/`guest` recipients and `owner`-type rules no longer parse * at the spec (ADR-0078; `group` was renamed → `team`). What the runtime * still cannot enforce is SKIPPED (logged) rather than seeded as a match-all @@ -108,6 +108,10 @@ function mapRecipientType(t: unknown): SharingRuleRecipientType | null { // ADR-0057 D5: business-unit subtree recipient. case 'business_unit': return 'business_unit' as SharingRuleRecipientType; case 'unit_and_subordinates': return 'unit_and_subordinates' as SharingRuleRecipientType; + // [#15072] The RECORD-RELATIVE recipient (#14103, maintainer ruling B): + // `value` is a field NAME, held to the `FieldSchema.name` grammar at parse, + // and `SharingRuleService` reads that column on each matched record. + case 'field': return 'field'; // Defensive only: the authoring enum matches the cases above 1:1, but a // stale pre-built package could still register a retired shape — skip, // never seed match-all. diff --git a/packages/plugins/plugin-sharing/src/bu-tree-recompute.ts b/packages/plugins/plugin-sharing/src/bu-tree-recompute.ts index 361513e60e..0902b0efb3 100644 --- a/packages/plugins/plugin-sharing/src/bu-tree-recompute.ts +++ b/packages/plugins/plugin-sharing/src/bu-tree-recompute.ts @@ -24,7 +24,7 @@ * ## Which recipient kinds this covers, measured rather than assumed * * `SharingRuleService.expandRecipient` is the one switch that decides, and it - * reads the BU tree for TWO of the six kinds: + * reads the BU tree for TWO of the seven kinds: * * | recipient_type | resolver | reads the BU tree | * |-------------------------|---------------------------------------------|---| @@ -34,6 +34,7 @@ * | `position` | `PositionGraphService` (`sys_user_position`, `sys_member`) | no | * | `unit_and_subordinates` | `BusinessUnitGraphService.expandUsers` | YES | * | `queue` | returns `[]` (no `sys_queue` yet) | no | + * | `field` | the matched record's own user column, per record (#15072; `expandRecipientForRecord`, never rule-wide) | no | * * `business_unit` stays in that set after #7807 narrowed it to exactly one * unit's members. The divergence this file originally noted — `expandRecipient` @@ -110,7 +111,7 @@ export const BU_TREE_RECOMPUTE_PACKAGE = 'plugin-sharing:bu-tree-recompute'; * The recipient kinds whose expansion reads the business-unit graph. * * This set is the non-regression guarantee. A rule recipient that never reads - * the BU tree (`user` / `team` / `position` / `queue`) is not recomputed by a + * the BU tree (`user` / `team` / `position` / `queue` / `field`) is not recomputed by a * BU write at all — not more cheaply, not at all — so a deployment whose rules * are all `user`-recipient pays one `sys_sharing_rule` read per BU write and * nothing else. diff --git a/packages/plugins/plugin-sharing/src/field-recipient.test.ts b/packages/plugins/plugin-sharing/src/field-recipient.test.ts new file mode 100644 index 0000000000..69ebab2570 --- /dev/null +++ b/packages/plugins/plugin-sharing/src/field-recipient.test.ts @@ -0,0 +1,810 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15072] The `field` sharing recipient — the services half of the #14103 + * ruling (maintainer, 2026-09-02, B): `sharedWith: { type: 'field', value: + * '' }` shares each matched record with the user or users + * named by that column ON THE RECORD, honouring `multiple: true`; + * `expandRecipient` becomes per-record for that member ONLY; ⛔ no `manager` + * member. + * + * ## What this file pins, and why each pin exists + * + * The executor contract (issue #15072, comment 5532175167) has seven points; + * the discharged precondition (comment 5535095827) adds two rule-wide call + * sites of `expandRecipient` that the contract's per-record reading does not + * cover. Every `describe` below names which of those it guards: + * + * 1. **Per record** — `evaluateAllForRecord`, the write-hook pass: single + * column, multi column, FAIL-CLOSED on empty (never an owner fallback), + * and the record's own users never reaching another record. + * 2. **Re-materialisation on the record's own write** — driven through the + * real `bindRuleHooks` binding: a write touching ONLY the recipient column + * revokes the stale grant and materialises the new one, with NO second + * trigger (the bound hook set is unchanged). + * 3. **Site 2, `evaluateRule`** — the whole-rule pass behind the background + * re-grant, the boot backfill and the REST evaluate endpoint: per-record + * pairs, never a cartesian product, and the unbounded-bulk-write path + * (`revokeRuleGrantsForObject` → `evaluateAllRulesForObject`) restores + * exactly the per-record grants. + * 4. **Site 3, `revokeRuleGrantsForRetiredRecipients`** — declines a `field` + * rule at the door (0, no grant touched), beside a control that still + * retires a rule-wide recipient, and the BU-graph recompute that is its one + * caller never hands a `field` rule in. + * 5. **The ruling's explicit pin** — a `position` recipient still expands + * RULE-WIDE: one holder read per pass however many records match, while + * the `field` rule's expansion reads no principal table at all. + * 6. **The column must hold users** — when the object's schema can say: + * a `user` field or a `sys_user` lookup is read, anything else grants + * nobody and warns once; a failing read fails closed and warns once. + * 7. **Authoring seams** — the declared-rule bootstrap seeds a `field` rule + * (it used to skip it), `defineRule` holds a `field` `recipientId` to the + * field-name grammar, and the `sys_sharing_rule.recipient_type` select can + * store every authorable `ShareRecipientType` member. + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/objectql'; +import { ShareRecipientType } from '@objectstack/spec/security'; +import { SharingService } from './sharing-service.js'; +import { SharingRuleService } from './sharing-rule-service.js'; +import { bindRuleHooks, unbindAllRuleHooks, SHARING_RULE_HOOK_PACKAGE } from './rule-hooks.js'; +import { bindBusinessUnitTreeRecompute } from './bu-tree-recompute.js'; +import { bootstrapDeclaredSharingRules } from './bootstrap-declared-sharing-rules.js'; +import { SysSharingRule } from './objects/sys-sharing-rule.object.js'; + +interface Row { [k: string]: any } + +const SYS = { isSystem: true, positions: [], permissions: [] } as any; +/** What an interactive admin's write carries. */ +const ADMIN_SESSION = { isSystem: false, userId: 'admin' }; + +type HookEntry = { event: string; handler: (ctx: any) => any; options: Row }; + +function matches(row: Row, f: any): boolean { + if (!f || typeof f !== 'object') return true; + // A combinator is CONJOINED with its sibling field keys, never a + // short-circuit that returns before they are read (#7676) — `listRules` + // composes `{object_name, active, $or:[…org scope…]}`. + if (Array.isArray(f.$or) && !f.$or.some((x: any) => matches(row, x))) return false; + if (Array.isArray(f.$and) && !f.$and.every((x: any) => matches(row, x))) return false; + for (const [k, v] of Object.entries(f)) { + if (k === '$or' || k === '$and') continue; + const rv = row[k]; + if (v != null && typeof v === 'object' && !Array.isArray(v)) { + const op: any = v; + if ('$in' in op) { if (!op.$in.includes(rv)) return false; continue; } + if ('$ne' in op) { if (rv === op.$ne) return false; continue; } + if ('$gte' in op) { if (!(rv >= op.$gte)) return false; continue; } + } + if (rv !== v) return false; + } + return true; +} + +/** + * A fake ObjectQL engine: tables, an optional schema per object, a hook + * registry the real `bindRuleHooks` can bind to, and a `find` census. Both + * write verbs open with the PRODUCER's own dispatch predicate (#4550 / #5480) + * so a double looser than `ObjectQL` cannot green a call production refuses. + */ +function makeEngine() { + const tables: Record = {}; + const schemas: Record = {}; + const hooks: HookEntry[] = []; + const finds: Array<{ object: string; fields?: string[] }> = []; + const ensure = (n: string) => (tables[n] ??= []); + let seq = 0; + + const engine = { + _tables: tables, + _schemas: schemas, + _finds: finds, + /** A projected field name whose read the engine refuses (`INVALID_FIELD`). */ + _refuseField: '' as string, + getSchema(name: string) { return schemas[name]; }, + seed(object: string, rows: Row[]) { ensure(object).push(...rows.map((r) => ({ ...r }))); }, + async find(o: string, opts?: any) { + finds.push({ object: o, fields: opts?.fields }); + if (engine._refuseField && Array.isArray(opts?.fields) && opts.fields.includes(engine._refuseField)) { + const err: any = new Error(`Unknown field '${engine._refuseField}' on ${o}`); + err.code = 'INVALID_FIELD'; + throw err; + } + const f = opts?.filter ?? opts?.where; + return ensure(o).filter((r) => matches(r, f)).slice(0, opts?.limit ?? 10000); + }, + async insert(o: string, data: any) { + const row = { id: data.id ?? `${o}_${++seq}`, ...data }; + ensure(o).push(row); + return row; + }, + async update(o: string, data: any, options?: any) { + const verdict = assertEngineUpdateDispatch(data, options); + const t = ensure(o); + const targets = verdict.kind === 'by-id' + ? t.filter((r) => r.id === verdict.id) + : t.filter((r) => matches(r, options?.where)); + for (const r of targets) Object.assign(r, data); + return verdict.kind === 'by-id' ? (targets[0] ?? null) : targets.length; + }, + async delete(o: string, opts?: any) { + assertEngineDeleteDispatch(opts); + const t = ensure(o); + const where = opts?.where ?? (opts?.id != null ? { id: opts.id } : {}); + for (let i = t.length - 1; i >= 0; i--) if (matches(t[i], where)) t.splice(i, 1); + return { ok: true }; + }, + registerHook(event: string, handler: (ctx: any) => any, options: Row = {}) { + hooks.push({ event, handler, options }); + }, + unregisterHooksByPackage(packageId: string) { + let removed = 0; + for (let i = hooks.length - 1; i >= 0; i--) { + if (hooks[i].options.packageId === packageId) { hooks.splice(i, 1); removed++; } + } + return removed; + }, + boundFor(packageId: string, object?: string) { + return hooks.filter((h) => h.options.packageId === packageId && (!object || h.options.object === object)); + }, + async fire(event: string, object: string, ctx: any) { + for (const h of [...hooks]) { + if (h.event === event && h.options.object === object) await h.handler(ctx); + } + }, + /** One row insert, fired the way the engine fires `afterInsert`. */ + async simulateInsert(object: string, row: Row, session: any = ADMIN_SESSION) { + ensure(object).push({ ...row }); + await engine.fire('afterInsert', object, { + object, event: 'afterInsert', input: { data: row }, result: row, session, + }); + }, + /** + * A SINGLE-ID update — the shape the `before` stash resolves without + * querying (`resolveAffectedRows` step 1), so `afterUpdate` takes its + * bounded per-row branch. + */ + async simulateUpdate(object: string, id: string, data: Row, session: any = ADMIN_SESSION) { + const ctx: any = { + object, event: 'beforeUpdate', + input: { id, data: { ...data, id }, options: {} }, + session, + }; + await engine.fire('beforeUpdate', object, ctx); + const t = ensure(object); + const i = t.findIndex((r) => r.id === id); + if (i >= 0) t[i] = { ...t[i], ...data }; + ctx.event = 'afterUpdate'; + await engine.fire('afterUpdate', object, ctx); + }, + }; + return engine; +} + +type Engine = ReturnType; + +const OBJECT = 'request'; +const RULE = 'approved_to_assignees'; + +/** The `field` rule under test: approved requests → the users the row's `assignees` column names. */ +const fieldRule = (over: Row = {}): Row => ({ + id: 'srule_assignees', organization_id: null, name: RULE, + label: 'Approved requests → assignees', object_name: OBJECT, + criteria_json: JSON.stringify({ status: 'approved' }), + recipient_type: 'field', recipient_id: 'assignees', + access_level: 'read', active: true, managed_by: 'package', + ...over, +}); + +function harness() { + const engine = makeEngine(); + const warn = vi.fn(); + const sharing = new SharingService({ engine: engine as any }); + const rules = new SharingRuleService({ engine: engine as any, sharing, logger: { warn } as any }); + /** Who currently holds a rule-materialised grant on `recordId`, sorted. */ + const granteesOf = (recordId: string, ruleId = 'srule_assignees'): string[] => + (engine._tables.sys_record_share ?? []) + .filter((r) => r.record_id === recordId && r.source === 'rule' && r.source_id === ruleId) + .map((r) => String(r.recipient_id)) + .sort(); + const ruleShares = (ruleId = 'srule_assignees') => + (engine._tables.sys_record_share ?? []).filter((r) => r.source === 'rule' && r.source_id === ruleId); + /** A grant the rule materialised at some earlier point — the stale row a reconcile must retire. */ + const seedStaleGrant = (recordId: string, userId: string, ruleId = 'srule_assignees') => + engine.seed('sys_record_share', [{ + id: `shr_stale_${recordId}_${userId}`, object_name: OBJECT, record_id: recordId, + recipient_type: 'user', recipient_id: userId, access_level: 'read', + source: 'rule', source_id: ruleId, + }]); + const fieldWarns = () => warn.mock.calls.filter((c) => String(c[0]).includes('field-recipient rule grants NOBODY')); + return { engine, rules, sharing, warn, granteesOf, ruleShares, seedStaleGrant, fieldWarns }; +} + +// ───────────────────────────────────────────────────────────────────────── +describe('#15072 per record — `evaluateAllForRecord`, the write-hook pass', () => { + let h: ReturnType; + + beforeEach(() => { + h = harness(); + h.engine.seed('sys_sharing_rule', [fieldRule()]); + }); + + it('a single-user column shares the record with the one user it names', async () => { + h.engine.seed('sys_sharing_rule', [fieldRule({ id: 'srule_mgr', name: 'approved_to_manager', recipient_id: 'owner_manager' })]); + h.engine.seed(OBJECT, [{ id: 'req_1', status: 'approved', owner_id: 'boss', owner_manager: 'u_mgr', assignees: null }]); + + const results = await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + + expect(h.granteesOf('req_1', 'srule_mgr')).toEqual(['u_mgr']); + const mine = results.find((r) => r.ruleId === 'srule_mgr')!; + expect(mine).toMatchObject({ matchedRecords: 1, expandedUsers: 1, grantsCreated: 1, grantsRevoked: 0 }); + }); + + it('a multi-user column (`multiple: true`) shares with EVERY user it names, once each', async () => { + h.engine.seed(OBJECT, [{ id: 'req_1', status: 'approved', owner_id: 'boss', assignees: ['u_a', 'u_b', 'u_a'] }]); + + const [result] = await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + + expect(h.granteesOf('req_1')).toEqual(['u_a', 'u_b']); + expect(result).toMatchObject({ matchedRecords: 1, expandedUsers: 2, grantsCreated: 2 }); + }); + + describe('FAIL-CLOSED on empty (executor-contract point 3)', () => { + it.each([ + ['null', null], + ['undefined (column absent from the row)', undefined], + ['an empty string', ''], + ['an empty array', []], + ['whitespace', ' '], + ])('%s materialises no grant', async (_label, value) => { + const row: Row = { id: 'req_1', status: 'approved', owner_id: 'boss' }; + if (value !== undefined) row.assignees = value; + h.engine.seed(OBJECT, [row]); + + const [result] = await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + + expect(h.granteesOf('req_1')).toEqual([]); + expect(result).toMatchObject({ matchedRecords: 1, expandedUsers: 0, grantsCreated: 0 }); + // Not a misconfiguration: an unassigned record shares with nobody by design. + expect(h.fieldWarns()).toHaveLength(0); + }); + + it('never falls back to the record owner: an empty column does not share with `owner_id`', async () => { + h.engine.seed(OBJECT, [{ id: 'req_1', status: 'approved', owner_id: 'boss', assignees: [] }]); + await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + expect(h.granteesOf('req_1')).not.toContain('boss'); + expect(h.ruleShares()).toEqual([]); + }); + + it('a value that is not a user id — an object — names nobody', async () => { + h.engine.seed(OBJECT, [{ id: 'req_1', status: 'approved', owner_id: 'boss', assignees: { id: 'u_a' } }]); + await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + expect(h.granteesOf('req_1')).toEqual([]); + }); + }); + + it('a stale grant is REVOKED when the column no longer names its user', async () => { + h.engine.seed(OBJECT, [{ id: 'req_1', status: 'approved', owner_id: 'boss', assignees: ['u_new'] }]); + h.seedStaleGrant('req_1', 'u_old'); + + const [result] = await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + + expect(h.granteesOf('req_1')).toEqual(['u_new']); + expect(result).toMatchObject({ grantsCreated: 1, grantsRevoked: 1 }); + }); + + it('an emptied column revokes the grant it once earned (fail-closed in the revoke direction too)', async () => { + h.engine.seed(OBJECT, [{ id: 'req_1', status: 'approved', owner_id: 'boss', assignees: null }]); + h.seedStaleGrant('req_1', 'u_old'); + + const [result] = await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + + expect(h.granteesOf('req_1')).toEqual([]); + expect(result).toMatchObject({ matchedRecords: 1, grantsCreated: 0, grantsRevoked: 1 }); + }); + + it('the criteria still bite: a filled column on a record OUTSIDE the criteria grants nobody', async () => { + h.engine.seed(OBJECT, [{ id: 'req_draft', status: 'draft', owner_id: 'boss', assignees: ['u_a'] }]); + const [result] = await h.rules.evaluateAllForRecord(OBJECT, 'req_draft', SYS); + expect(h.granteesOf('req_draft')).toEqual([]); + expect(result).toMatchObject({ matchedRecords: 0, expandedUsers: 0 }); + }); + + it('the users are read from THE matched record — one record\'s users never reach another', async () => { + h.engine.seed(OBJECT, [ + { id: 'req_1', status: 'approved', owner_id: 'boss', assignees: ['u_a'] }, + { id: 'req_2', status: 'approved', owner_id: 'boss', assignees: ['u_b'] }, + ]); + await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + await h.rules.evaluateAllForRecord(OBJECT, 'req_2', SYS); + expect(h.granteesOf('req_1')).toEqual(['u_a']); + expect(h.granteesOf('req_2')).toEqual(['u_b']); + }); + + it('an INACTIVE field rule desires nothing and purges what it held', async () => { + h.engine._tables.sys_sharing_rule = [fieldRule({ active: false })]; + h.engine.seed(OBJECT, [{ id: 'req_1', status: 'approved', owner_id: 'boss', assignees: ['u_a'] }]); + h.seedStaleGrant('req_1', 'u_a'); + const [result] = await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + expect(h.granteesOf('req_1')).toEqual([]); + expect(result).toMatchObject({ matchedRecords: 0, grantsRevoked: 1 }); + }); +}); + +// ───────────────────────────────────────────────────────────────────────── +describe('#15072 re-materialisation on the record\'s OWN write (executor-contract point 4)', () => { + let h: ReturnType; + const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; + + /** Bind the real hooks against whatever is in `sys_sharing_rule`. */ + const bind = async () => { + const ruleRows = await h.rules.listRules({ activeOnly: true }, SYS); + unbindAllRuleHooks(h.engine as any); + bindRuleHooks(h.engine as any, h.rules, ruleRows, logger); + }; + + beforeEach(async () => { + h = harness(); + h.engine.seed('sys_sharing_rule', [fieldRule()]); + await bind(); + }); + + it('an insert naming a user materialises the grant through `afterInsert`', async () => { + await h.engine.simulateInsert(OBJECT, { id: 'req_1', status: 'approved', owner_id: 'boss', assignees: ['u_a'] }); + expect(h.granteesOf('req_1')).toEqual(['u_a']); + }); + + it('a write that changes ONLY the recipient column revokes the stale grant and materialises the new one', async () => { + await h.engine.simulateInsert(OBJECT, { id: 'req_1', status: 'approved', owner_id: 'boss', assignees: ['u_a'] }); + expect(h.granteesOf('req_1')).toEqual(['u_a']); + + // The patch carries the recipient column and nothing else — the criteria + // column is untouched, so this is the write the precondition asked about: + // `afterUpdate` has no changed-field gating and re-runs the per-record + // reconcile regardless of which columns moved. + await h.engine.simulateUpdate(OBJECT, 'req_1', { assignees: ['u_b'] }); + + expect(h.granteesOf('req_1')).toEqual(['u_b']); + // The old grant is gone, not merely joined by a new one. + expect(h.ruleShares().map((r) => r.recipient_id)).toEqual(['u_b']); + }); + + it('widening the column (one user → two) adds the second grant and keeps the first', async () => { + await h.engine.simulateInsert(OBJECT, { id: 'req_1', status: 'approved', owner_id: 'boss', assignees: ['u_a'] }); + await h.engine.simulateUpdate(OBJECT, 'req_1', { assignees: ['u_a', 'u_b'] }); + expect(h.granteesOf('req_1')).toEqual(['u_a', 'u_b']); + }); + + it('clearing the column withdraws every grant the record held (fail-closed on the write path)', async () => { + await h.engine.simulateInsert(OBJECT, { id: 'req_1', status: 'approved', owner_id: 'boss', assignees: ['u_a', 'u_b'] }); + await h.engine.simulateUpdate(OBJECT, 'req_1', { assignees: [] }); + expect(h.granteesOf('req_1')).toEqual([]); + }); + + it('a write moving the record OUT of the criteria withdraws the grant, whatever the column says', async () => { + await h.engine.simulateInsert(OBJECT, { id: 'req_1', status: 'approved', owner_id: 'boss', assignees: ['u_a'] }); + await h.engine.simulateUpdate(OBJECT, 'req_1', { status: 'rejected' }); + expect(h.granteesOf('req_1')).toEqual([]); + }); + + it('NO second materialisation trigger: the bound hook set for the object is the existing five', () => { + // The precondition was CONFIRMED on the tree (comment 5535095827): the + // existing `afterUpdate` covers a recipient-column write. So the binding a + // `field` rule receives must be byte-for-byte the binding every rule + // receives — nothing registered for the recipient column specifically. + const events = h.engine.boundFor(SHARING_RULE_HOOK_PACKAGE, OBJECT).map((x) => x.event).sort(); + expect(events).toEqual(['afterDelete', 'afterInsert', 'afterUpdate', 'beforeDelete', 'beforeUpdate']); + }); +}); + +// ───────────────────────────────────────────────────────────────────────── +describe('#15072 site 2 — `evaluateRule`, the whole-rule pass (background re-grant, boot backfill, REST evaluate)', () => { + let h: ReturnType; + + beforeEach(() => { + h = harness(); + h.engine.seed('sys_sharing_rule', [fieldRule()]); + h.engine.seed(OBJECT, [ + { id: 'req_1', status: 'approved', owner_id: 'boss', assignees: ['u_a'] }, + { id: 'req_2', status: 'approved', owner_id: 'boss', assignees: ['u_b', 'u_c'] }, + { id: 'req_3', status: 'approved', owner_id: 'boss', assignees: [] }, + { id: 'req_draft', status: 'draft', owner_id: 'boss', assignees: ['u_a'] }, + ]); + }); + + it('derives per-record (record, user) pairs — never matched records × one recipient list', async () => { + const result = await h.rules.evaluateRule(RULE, SYS); + + expect(h.granteesOf('req_1')).toEqual(['u_a']); + expect(h.granteesOf('req_2')).toEqual(['u_b', 'u_c']); + expect(h.granteesOf('req_3')).toEqual([]); + expect(h.granteesOf('req_draft')).toEqual([]); + // The cartesian product would have put u_a on req_2 and u_b/u_c on req_1. + expect(h.granteesOf('req_2')).not.toContain('u_a'); + expect(h.granteesOf('req_1')).not.toContain('u_b'); + expect(result).toMatchObject({ + matchedRecords: 3, // req_1, req_2, req_3 — the empty one still matched + expandedUsers: 3, // DISTINCT users across the matched records + grantsCreated: 3, + grantsUpdated: 0, + grantsRevoked: 0, + }); + }); + + it('revokes a cross-product grant a rule-wide materialisation would have left behind', async () => { + h.seedStaleGrant('req_2', 'u_a'); // u_a is named on req_1 only + h.seedStaleGrant('req_3', 'u_b'); // req_3 names nobody + const result = await h.rules.evaluateRule(RULE, SYS); + expect(h.granteesOf('req_2')).toEqual(['u_b', 'u_c']); + expect(h.granteesOf('req_3')).toEqual([]); + expect(result.grantsRevoked).toBe(2); + }); + + it('is idempotent: a second pass changes nothing', async () => { + await h.rules.evaluateRule(RULE, SYS); + const again = await h.rules.evaluateRule(RULE, SYS); + expect(again).toMatchObject({ matchedRecords: 3, expandedUsers: 3, grantsCreated: 0, grantsUpdated: 0, grantsRevoked: 0 }); + }); + + it('the unbounded-bulk-write path end to end: revoke the object set-based, then the background re-grant restores exactly the per-record grants', async () => { + // `rule-hooks.ts` `revokeThenQueueRegrant`: `revokeRuleGrantsForObject` + // synchronously, `evaluateAllRulesForObject` on the queue. Driven here + // without the queue so the assertion is about the two primitives. + await h.rules.evaluateRule(RULE, SYS); + const before = h.ruleShares().map((r) => `${r.record_id}::${r.recipient_id}`).sort(); + expect(before).toEqual(['req_1::u_a', 'req_2::u_b', 'req_2::u_c']); + + await h.rules.revokeRuleGrantsForObject(OBJECT); + expect(h.ruleShares()).toEqual([]); + + const reconciled = await h.rules.evaluateAllRulesForObject(OBJECT); + + expect(reconciled).toBe(1); + expect(h.ruleShares().map((r) => `${r.record_id}::${r.recipient_id}`).sort()).toEqual(before); + }); + + it('the rule-wide expansion is never asked for a field rule on this path (no "object reconcile failed")', async () => { + await h.rules.evaluateAllRulesForObject(OBJECT); + const failed = h.warn.mock.calls.filter((c) => String(c[0]).includes('object reconcile failed')); + expect(failed).toEqual([]); + expect(h.granteesOf('req_2')).toEqual(['u_b', 'u_c']); + }); + + it('the pass reads the recipient column ALONGSIDE the id on the matched rows — one criteria read, no per-record round trip', async () => { + await h.rules.evaluateRule(RULE, SYS); + const criteriaReads = h.engine._finds.filter((f) => f.object === OBJECT); + expect(criteriaReads).toHaveLength(1); + expect(criteriaReads[0].fields).toEqual(['id', 'assignees']); + }); + + it('an INACTIVE field rule purges its grants on the whole-rule pass (#4433 holds for the new kind)', async () => { + await h.rules.evaluateRule(RULE, SYS); + expect(h.ruleShares()).toHaveLength(3); + await h.engine.update('sys_sharing_rule', { id: 'srule_assignees', active: false }); + const result = await h.rules.evaluateRule(RULE, SYS); + expect(result.grantsRevoked).toBe(3); + expect(h.ruleShares()).toEqual([]); + }); +}); + +// ───────────────────────────────────────────────────────────────────────── +describe('#15072 site 3 — `revokeRuleGrantsForRetiredRecipients`, the recipient-axis revoke', () => { + let h: ReturnType; + + beforeEach(async () => { + h = harness(); + h.engine.seed('sys_sharing_rule', [fieldRule()]); + h.engine.seed(OBJECT, [ + { id: 'req_1', status: 'approved', owner_id: 'boss', assignees: ['u_a'] }, + { id: 'req_2', status: 'approved', owner_id: 'boss', assignees: ['u_b', 'u_c'] }, + ]); + await h.rules.evaluateRule(RULE, SYS); + expect(h.ruleShares()).toHaveLength(3); + }); + + it('declines a field rule at the door: returns 0 and touches no grant', async () => { + const rule = (await h.rules.getRule('srule_assignees', SYS))!; + expect(rule.recipient_type).toBe('field'); + const grantReadsBefore = h.engine._finds.filter((f) => f.object === 'sys_record_share').length; + + const retired = await h.rules.revokeRuleGrantsForRetiredRecipients(rule); + + expect(retired).toBe(0); + // Every grant is still correct and still standing — a rule-wide "desired" + // set for this kind would have been empty, and the diff would have + // retired all three. + expect(h.ruleShares().map((r) => `${r.record_id}::${r.recipient_id}`).sort()) + .toEqual(['req_1::u_a', 'req_2::u_b', 'req_2::u_c']); + // Declined at the door: not even the grant table was read. + expect(h.engine._finds.filter((f) => f.object === 'sys_record_share').length).toBe(grantReadsBefore); + }); + + it('CONTROL: a rule-wide recipient still retires the recipients it no longer reaches', async () => { + h.engine.seed('sys_sharing_rule', [fieldRule({ + id: 'srule_alice', name: 'approved_to_alice', recipient_type: 'user', recipient_id: 'alice', + })]); + await h.rules.evaluateRule('approved_to_alice', SYS); + h.seedStaleGrant('req_1', 'bob', 'srule_alice'); // bob is nobody the rule expands to + const rule = (await h.rules.getRule('srule_alice', SYS))!; + + const retired = await h.rules.revokeRuleGrantsForRetiredRecipients(rule); + + expect(retired).toBe(1); + expect(h.granteesOf('req_1', 'srule_alice')).toEqual(['alice']); + }); + + it('the BU-graph recompute — its one production caller — never hands a field rule in', async () => { + // A business-unit rule beside the field rule, so the filter is exercised + // in both directions on one write. + h.engine.seed('sys_sharing_rule', [fieldRule({ + id: 'srule_bu', name: 'approved_to_unit', recipient_type: 'business_unit', recipient_id: 'bu_ops', + })]); + const spy = vi.spyOn(h.rules, 'revokeRuleGrantsForRetiredRecipients'); + bindBusinessUnitTreeRecompute(h.engine as any, h.rules, { warn: vi.fn() } as any); + + await h.engine.fire('afterInsert', 'sys_business_unit_member', { + object: 'sys_business_unit_member', event: 'afterInsert', + input: { data: { id: 'bum_1', business_unit_id: 'bu_ops', user_id: 'u_z' } }, + }); + + const handed = spy.mock.calls.map((c) => c[0].recipient_type); + expect(handed).toEqual(['business_unit']); + expect(handed).not.toContain('field'); + expect(h.ruleShares()).toHaveLength(3); + }); +}); + +// ───────────────────────────────────────────────────────────────────────── +describe('#15072 the ruling\'s explicit pin — a `position` recipient still expands RULE-WIDE', () => { + let h: ReturnType; + const POSITION_RULE = 'approved_to_sales_reps'; + + beforeEach(() => { + h = harness(); + h.engine.seed('sys_sharing_rule', [ + fieldRule(), + fieldRule({ id: 'srule_pos', name: POSITION_RULE, recipient_type: 'position', recipient_id: 'sales_rep' }), + ]); + h.engine.seed('sys_user_position', [ + { id: 'up_1', position: 'sales_rep', user_id: 'u_p1' }, + { id: 'up_2', position: 'sales_rep', user_id: 'u_p2' }, + ]); + h.engine.seed(OBJECT, [ + { id: 'req_1', status: 'approved', owner_id: 'boss', assignees: ['u_a'] }, + { id: 'req_2', status: 'approved', owner_id: 'boss', assignees: ['u_b'] }, + { id: 'req_3', status: 'approved', owner_id: 'boss', assignees: [] }, + ]); + }); + + const holderReads = () => h.engine._finds.filter((f) => f.object === 'sys_user_position').length; + + it('every matched record gets the SAME holder set, resolved ONCE per pass however many records match', async () => { + const before = holderReads(); + const result = await h.rules.evaluateRule(POSITION_RULE, SYS); + + for (const id of ['req_1', 'req_2', 'req_3']) expect(h.granteesOf(id, 'srule_pos')).toEqual(['u_p1', 'u_p2']); + expect(result).toMatchObject({ matchedRecords: 3, expandedUsers: 2, grantsCreated: 6 }); + // One holder read for three matched records: the expansion is per RULE. + expect(holderReads() - before).toBe(1); + }); + + it('…while the field rule on the same object reads NO principal table at all', async () => { + const before = h.engine._finds.length; + await h.rules.evaluateRule(RULE, SYS); + const objects = new Set(h.engine._finds.slice(before).map((f) => f.object)); + for (const principalTable of ['sys_user_position', 'sys_position', 'sys_member', 'sys_team_member', 'sys_business_unit_member', 'sys_business_unit']) { + expect(objects.has(principalTable)).toBe(false); + } + // The two kinds coexist on one object without either widening the other. + expect(h.granteesOf('req_1')).toEqual(['u_a']); + expect(h.granteesOf('req_1', 'srule_pos')).toEqual([]); + }); + + it('on the per-record pass too: the position rule and the field rule each answer for themselves', async () => { + await h.rules.evaluateAllForRecord(OBJECT, 'req_2', SYS); + expect(h.granteesOf('req_2', 'srule_pos')).toEqual(['u_p1', 'u_p2']); + expect(h.granteesOf('req_2')).toEqual(['u_b']); + }); +}); + +// ───────────────────────────────────────────────────────────────────────── +describe('#15072 the recipient column must EXIST and HOLD USERS — judged from the object\'s schema when it can be', () => { + let h: ReturnType; + + beforeEach(() => { + h = harness(); + h.engine._schemas[OBJECT] = { + name: OBJECT, + fields: { + id: { type: 'text' }, + status: { type: 'select', options: ['draft', 'approved'] }, + owner_id: { type: 'lookup', reference: 'sys_user' }, + assignees: { type: 'user', multiple: true }, + reviewer: { type: 'lookup', reference: 'sys_user' }, + account: { type: 'lookup', reference: 'account' }, + notes: { type: 'text' }, + }, + }; + h.engine.seed(OBJECT, [{ + id: 'req_1', status: 'approved', owner_id: 'boss', + assignees: ['u_a'], reviewer: 'u_rev', account: 'acc_1', notes: 'u_smuggled', + }]); + }); + + const withRecipient = (recipient_id: string) => + h.engine.seed('sys_sharing_rule', [fieldRule({ recipient_id })]); + + it('a `user` field is read', async () => { + withRecipient('assignees'); + await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + expect(h.granteesOf('req_1')).toEqual(['u_a']); + expect(h.fieldWarns()).toHaveLength(0); + }); + + it('a lookup to `sys_user` is read', async () => { + withRecipient('reviewer'); + await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + expect(h.granteesOf('req_1')).toEqual(['u_rev']); + expect(h.fieldWarns()).toHaveLength(0); + }); + + it('a column of another type grants NOBODY — a text column holding a user id is not read as one — and warns once per rule', async () => { + withRecipient('notes'); + await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + await h.rules.evaluateRule(RULE, SYS); + + expect(h.granteesOf('req_1')).toEqual([]); + const warns = h.fieldWarns(); + expect(warns).toHaveLength(1); + expect(warns[0][1]).toMatchObject({ rule: RULE, object: OBJECT, field: 'notes', cause: 'not-user-typed' }); + }); + + it('a lookup to another object is not a user column either', async () => { + withRecipient('account'); + await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + expect(h.granteesOf('req_1')).toEqual([]); + expect(h.fieldWarns()[0][1]).toMatchObject({ field: 'account', cause: 'not-user-typed' }); + }); + + it('a column the object does not declare grants nobody and warns (cause: no-such-field)', async () => { + withRecipient('ghost'); + await h.rules.evaluateRule(RULE, SYS); + expect(h.ruleShares()).toEqual([]); + expect(h.fieldWarns()).toHaveLength(1); + expect(h.fieldWarns()[0][1]).toMatchObject({ field: 'ghost', cause: 'no-such-field' }); + }); + + it('an unusable column still REVOKES the grants the rule once held (fail-closed, not frozen)', async () => { + withRecipient('notes'); + h.seedStaleGrant('req_1', 'u_smuggled'); + const [result] = await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + expect(h.granteesOf('req_1')).toEqual([]); + expect(result.grantsRevoked).toBe(1); + }); + + it('NO schema to consult: the column is read on its declared semantics', async () => { + delete h.engine._schemas[OBJECT]; + withRecipient('assignees'); + await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + expect(h.granteesOf('req_1')).toEqual(['u_a']); + }); + + it('a read the engine REFUSES (`INVALID_FIELD`) fails closed and warns once (cause: read-failed)', async () => { + delete h.engine._schemas[OBJECT]; // nothing to pre-judge — the engine is the one that refuses + h.engine._refuseField = 'ghost'; + withRecipient('ghost'); + h.seedStaleGrant('req_1', 'u_old'); + + const [result] = await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + await h.rules.evaluateAllForRecord(OBJECT, 'req_1', SYS); + + expect(result).toMatchObject({ matchedRecords: 0, grantsCreated: 0, grantsRevoked: 1 }); + expect(h.granteesOf('req_1')).toEqual([]); + const warns = h.fieldWarns(); + expect(warns).toHaveLength(1); + expect(warns[0][1]).toMatchObject({ field: 'ghost', cause: 'read-failed', error: expect.stringContaining('ghost') }); + }); +}); + +// ───────────────────────────────────────────────────────────────────────── +describe('#15072 authoring seams', () => { + describe('the declared-rule bootstrap seeds a `field` rule (it used to skip it as unmappable)', () => { + function seedHarness(declared: any[]) { + const engine = { _registry: { listItems: (type: string) => (type === 'sharing_rule' ? declared : []) } }; + const defineRule = vi.fn(async (input: any) => ({ id: `id_${input.name}` })); + const warns: Array<{ msg: string; meta: any }> = []; + const logger = { warn: (msg: string, meta?: any) => { warns.push({ msg, meta }); }, info: () => {} }; + return { engine, ruleService: { defineRule } as any, logger, warns, defineRule }; + } + + it('maps `sharedWith.type: field` onto `recipient_type: field` with the field NAME as the recipient', async () => { + const { engine, ruleService, logger, warns, defineRule } = seedHarness([{ + name: 'approved_to_assignees', object: OBJECT, + condition: "record.status == 'approved'", + sharedWith: { type: 'field', value: 'assignees' }, + accessLevel: 'edit', + }]); + + const res = await bootstrapDeclaredSharingRules(ruleService, null, engine, logger); + + expect(res).toEqual({ seeded: 1, skipped: 0 }); + expect(warns).toEqual([]); + expect(defineRule).toHaveBeenCalledTimes(1); + expect(defineRule.mock.calls[0][0]).toMatchObject({ + name: 'approved_to_assignees', + object: OBJECT, + criteria: { status: 'approved' }, + recipientType: 'field', + recipientId: 'assignees', + accessLevel: 'edit', + managedBy: 'package', + }); + }); + + it('CONTROL: an unmapped recipient kind is still skipped, never seeded wider', async () => { + const { engine, ruleService, logger, warns, defineRule } = seedHarness([{ + name: 'r_manager', object: OBJECT, + condition: "record.status == 'approved'", + sharedWith: { type: 'manager', value: 'owner_id' }, // ⛔ no `manager` member — the ruling + }]); + const res = await bootstrapDeclaredSharingRules(ruleService, null, engine, logger); + expect(res).toEqual({ seeded: 0, skipped: 1 }); + expect(defineRule).not.toHaveBeenCalled(); + expect(warns.some((w) => w.msg.includes('unmappable recipient'))).toBe(true); + }); + }); + + describe('`defineRule` holds a `field` recipientId to the field-name grammar (the seam the spec parse never sees)', () => { + let h: ReturnType; + const base = { + label: 'Approved → field', object: OBJECT, criteria: { status: 'approved' }, + accessLevel: 'read' as const, + }; + + beforeEach(() => { h = harness(); }); + + it('a snake_case field name is accepted and stored as the recipient', async () => { + const row = await h.rules.defineRule({ ...base, name: 'ok_field', recipientType: 'field', recipientId: 'assignees' } as any, SYS); + expect(row).toMatchObject({ recipient_type: 'field', recipient_id: 'assignees' }); + }); + + it.each([ + ['a dotted path (a graph walk spelled as a value)', 'owner.manager_id'], + ['a principal-shaped id', 'usr_01HZX'], + ['a name starting with a digit', '1st_owner'], + ['upper case', 'Assignees'], + ])('refuses %s with VALIDATION_FAILED', async (_label, recipientId) => { + await expect( + h.rules.defineRule({ ...base, name: 'bad_field', recipientType: 'field', recipientId } as any, SYS), + ).rejects.toThrow(/^VALIDATION_FAILED: recipientId must name a user-typed field/); + expect(h.engine._tables.sys_sharing_rule ?? []).toEqual([]); + }); + + it('is scoped to `field`: a `user` recipient keeps its opaque id, dots and case included', async () => { + const row = await h.rules.defineRule({ ...base, name: 'user_rule', recipientType: 'user', recipientId: 'Usr.01HZX' } as any, SYS); + expect(row).toMatchObject({ recipient_type: 'user', recipient_id: 'Usr.01HZX' }); + }); + }); + + describe('the `sys_sharing_rule.recipient_type` select can store every authorable recipient', () => { + const selectValues = (): string[] => + ((SysSharingRule as any).fields.recipient_type.options as Array<{ value: string }>).map((o) => o.value); + + it('lists `field` (executor-contract point 5 — a stored field row is no longer refused at the select)', () => { + expect(selectValues()).toContain('field'); + }); + + it('is a superset of `ShareRecipientType` — the class, not just the newest member', () => { + for (const member of ShareRecipientType.options) { + expect(selectValues(), `authorable member '${member}' must be storable`).toContain(member); + } + }); + + it('the recipient help text tells an admin what to put there for a field recipient', () => { + const help = String((SysSharingRule as any).fields.recipient_id.description); + expect(help).toMatch(/field/i); + }); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.object.ts b/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.object.ts index c7a85768b3..64e48d1aac 100644 --- a/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.object.ts +++ b/packages/plugins/plugin-sharing/src/objects/sys-sharing-rule.object.ts @@ -164,15 +164,19 @@ export const SysSharingRule = ObjectSchema.create({ recipient_type: Field.select( // `queue` was removed: it is declared-but-unenforced (the evaluator returns // no users for it), so offering it would author a silently-inert rule - // (ADR-0078). The five values below are the ones the evaluator expands. - ['user', 'team', 'business_unit', 'position', 'unit_and_subordinates'], + // (ADR-0078). The six values below are the ones the evaluator expands — + // `field` (#15072, the record-relative recipient ruled on #14103) per + // matched record, the other five once per rule. The list is held equal to + // the authorable `ShareRecipientType` by `field-recipient.test.ts`, so a + // stored row of any authorable kind is never refused at this select. + ['user', 'team', 'business_unit', 'position', 'unit_and_subordinates', 'field'], { label: 'Recipient Type', required: true, defaultValue: 'business_unit', // The engine detail this used to spell out (which tree is walked, which // expansion is flat, the ADRs behind each) lives in the class doc above. - description: 'Who receives access. Picking a team, business unit or position gives access to everyone in it. "Business unit and subordinates" also covers every unit below the one you pick.', + description: 'Who receives access. Picking a team, business unit or position gives access to everyone in it. "Business unit and subordinates" also covers every unit below the one you pick. "Field" shares each record with the user or users held in one of its own fields — name that field as the recipient.', group: 'Recipient', }, ), @@ -184,11 +188,14 @@ export const SysSharingRule = ObjectSchema.create({ // Rendered as a record picker whose target object follows recipient_type // (dependsOn: recipient_type): sys_user / sys_team / sys_business_unit / // sys_position. Stores the value the evaluator matches on — a record id - // for user/team/business_unit, the position NAME for `position`. Falls - // back to a text input when the widget is unavailable. + // for user/team/business_unit, the position NAME for `position`, and for + // `field` (#15072) the NAME of a user-typed field of the shared object + // (the picker has no mapping for that kind and degrades to its text + // input, which is the right input for a field name). Falls back to a + // text input when the widget is unavailable. widget: 'recipient-picker', dependsOn: ['recipient_type'], - description: 'The specific user, team, business unit or position that receives access.', + description: 'The specific user, team, business unit or position that receives access — or, for the "Field" recipient type, the name of the record field that holds the user or users to share with.', group: 'Recipient', }), diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts index d65eeaa8fe..fc5321eefe 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts @@ -40,6 +40,56 @@ import { BusinessUnitGraphService } from './business-unit-graph.js'; */ const SYSTEM_CTX: ExecutionContext = { isSystem: true, positions: [], permissions: [] }; +/** + * [#15072] The `FieldSchema.name` machine-name grammar, as `packages/spec` + * spells it inline (`data/field.zod.ts`; `shared/identifiers.zod.ts` records + * that object and field names are bound there and by no exported schema). A + * `field` recipient's `recipient_id` is a field NAME and is held to it at the + * authoring seams that never run the spec parse — see + * {@link SharingRuleService.defineRule}. + */ +const FIELD_NAME_GRAMMAR = /^[a-z_][a-z0-9_]*$/; + +/** + * [#15072] The user ids a record's user-typed column names. + * + * Exactly the stored shapes a user-valued field has: one id (a scalar) or, + * with `multiple: true`, an array of ids — the drivers hydrate a multi-value + * column back to an array before a row reaches an `engine.find` caller, so a + * string here is one id, never JSON to be parsed (a driver handing back an + * unhydrated column would be the driver's defect, and a parse here would + * fossilise it into a second contract — AGENTS.md PD #12). Everything else — + * `null`, an empty string, an empty array, an object — names NOBODY: fail- + * closed, executor-contract point 3 (never a match-all principal, never a + * fallback to the owner). Duplicates collapse: a column naming one user twice + * grants once. + */ +function usersNamedBy(raw: unknown): string[] { + const out = new Set(); + const add = (v: unknown): void => { + if (typeof v !== 'string' && typeof v !== 'number') return; + const id = String(v).trim(); + if (id) out.add(id); + }; + if (Array.isArray(raw)) for (const v of raw) add(v); + else add(raw); + return [...out]; +} + +/** + * [#15072] Does this declared field hold USERS? The two spellings the spec + * has for a user-valued column: the `user` type (its target is fixed to + * `sys_user` by the type — `Field.user()` takes no target), and a `lookup` / + * `master_detail` whose `reference` is `sys_user` (the same pair + * `defaultValue: 'current_user'` is accepted on, `data/field.zod.ts`). + */ +function fieldHoldsUsers(def: unknown): boolean { + if (!def || typeof def !== 'object') return false; + const d = def as { type?: unknown; reference?: unknown }; + if (d.type === 'user') return true; + return (d.type === 'lookup' || d.type === 'master_detail') && d.reference === 'sys_user'; +} + function uid(prefix: string): string { const g: any = globalThis as any; if (g.crypto?.randomUUID) return `${prefix}_${g.crypto.randomUUID()}`; @@ -113,6 +163,25 @@ type RuleEvaluationPass = { positionActive?: Map; }; +/** [#15072] One grant a rule DESIRES on a whole-rule pass: a (record, user) pair. */ +interface DesiredGrant { + record_id: string; + recipient_id: string; +} + +/** + * [#15072] Everything a whole-rule pass wants to exist, keyed + * `${record_id}::${recipient_id}`, plus the two counts the evaluation result + * reports. Built per recipient kind by {@link SharingRuleService.desiredGrantsForRule} + * so that `reconcile` diffs pairs without knowing whether the recipient was + * resolved once per rule or once per record. + */ +interface DesiredGrantSet { + desired: Map; + matchedRecords: number; + expandedUsers: number; +} + /** * Default {@link ISharingRuleService} implementation. * @@ -142,6 +211,13 @@ export class SharingRuleService implements ISharingRuleService { * deployment's log. */ private readonly emptyUnitExpansionSeen = new Set(); + /** + * [#15072] `field` rules already warned about this process, keyed by cause + * — the same once-per-rule dedup {@link inertRuleSeen} and + * {@link emptyUnitExpansionSeen} carry, for the same reason: the per-record + * path runs on every write of the shared object. + */ + private readonly fieldRuleWarned = new Set(); constructor(opts: SharingRuleServiceOptions) { this.engine = opts.engine; @@ -407,6 +483,21 @@ export class SharingRuleService implements ISharingRuleService { if (!input.object) throw new Error('VALIDATION_FAILED: object is required'); if (!input.recipientType) throw new Error('VALIDATION_FAILED: recipientType is required'); if (!input.recipientId) throw new Error('VALIDATION_FAILED: recipientId is required'); + // [#15072] A `field` recipient's `recipientId` is a FIELD NAME, not a + // principal id, and is held to the grammar `sharedWith.value` meets at + // parse (`ShareRecipientType` `field`, `sharing.zod.ts`): the + // `FieldSchema.name` machine-name shape. Refused HERE as well because this + // entry — the REST `POST /sharing/rules` body and programmatic authoring — + // never runs the spec schema, so without it a dotted path + // (`owner.manager_id`: a graph walk spelled as a value, the shape the + // ruling's ⛔ on a `manager` member refuses) would reach the executor and + // share with nobody in silence. + if (input.recipientType === 'field' && !FIELD_NAME_GRAMMAR.test(String(input.recipientId))) { + throw new Error( + "VALIDATION_FAILED: recipientId must name a user-typed field on the object when recipientType is 'field' " + + `(got ${JSON.stringify(input.recipientId)}) — a snake_case field name, not a principal id and not a dotted path`, + ); + } // [#3896] `criteria` is as required as the fields above — and for a // sharper reason. Omitting `recipientId` yields a rule that shares with // nobody; omitting `criteria` used to yield one that shares EVERYTHING @@ -672,9 +763,14 @@ export class SharingRuleService implements ISharingRuleService { const revoked = await this.purgeRuleGrants(rule.id); return { ruleId: rule.id, matchedRecords: 0, expandedUsers: 0, grantsCreated: 0, grantsUpdated: 0, grantsRevoked: revoked }; } - const matches = await this.findMatchingRecords(rule); - const users = await this.expandRecipient(rule); - return this.reconcile(rule, matches, users); + // [#15072] One whole-rule pass whatever the recipient kind: the desired + // set is derived per kind ({@link desiredGrantsForRule}) and the diff is + // shared. This is the primitive behind the background re-grant + // (`rule-hooks.ts` `revokeThenQueueRegrant` → `evaluateAllRulesForObject`), + // the `kernel:bootstrapped` backfill and the REST evaluate endpoint — so a + // `field` rule is either right here or wrong after every unbounded bulk + // write and every restart. + return this.reconcile(rule, await this.desiredGrantsForRule(rule)); } /** @@ -767,9 +863,11 @@ export class SharingRuleService implements ISharingRuleService { const pass: RuleEvaluationPass = {}; for (const rule of rules) { // An inactive rule desires nothing; skip the criteria query entirely. - const match = rule.active ? await this.recordMatches(rule, recordId) : false; - const users = match ? await this.expandRecipient(rule, pass) : []; - results.push(await this.reconcileForRecord(rule, recordId, match, users)); + // [#15072] The match read answers with the ROW, not a boolean: for a + // `field` rule the recipients are on it, read by the per-record switch. + const matched = rule.active ? await this.matchRecord(rule, recordId) : null; + const users = matched ? await this.expandRecipientForRecord(rule, matched, pass) : []; + results.push(await this.reconcileForRecord(rule, recordId, matched !== null, users)); } return results; } @@ -911,6 +1009,17 @@ export class SharingRuleService implements ISharingRuleService { */ async revokeRuleGrantsForRetiredRecipients(rule: SharingRuleRow): Promise { if (!rule?.id) return 0; + // [#15072] A `field` rule has NO rule-wide recipient set to diff against: + // its recipients live on each matched record and are re-derived by that + // record's own write ({@link evaluateAllForRecord}) and by the whole-rule + // pass ({@link evaluateRule} → {@link desiredGrantsForRule}). Expanding it + // "rule-wide" would answer nobody, and this method would then delete every + // grant the rule still deserves — a mass revocation, on the recipient + // axis, of grants that are all still correct. Its one production caller + // (`bu-tree-recompute.ts`) filters on `BU_TREE_RECIPIENT_TYPES` and never + // hands a `field` rule in; this guard is what makes that a contract rather + // than a coincidence, and `field-recipient.test.ts` pins both halves. + if (rule.recipient_type === 'field') return 0; const existing = await this.engine.find('sys_record_share', { where: { source: 'rule', source_id: rule.id }, fields: ['id', 'recipient_id'], @@ -1038,30 +1147,51 @@ export class SharingRuleService implements ISharingRuleService { } private async findMatchingRecords(rule: SharingRuleRow): Promise { + const rows = await this.findMatchingRows(rule, ['id']); + return rows.map((r: any) => String(r.id)).filter(Boolean); + } + + /** + * [#15072] The rule's matched ROWS with `fields` projected — `['id']` for + * every rule-wide recipient, `['id', ]` for a `field` + * rule, whose recipients are read off these very rows + * ({@link desiredGrantsForRule}). + */ + private async findMatchingRows(rule: SharingRuleRow, fields: string[]): Promise[]> { if (this.isInertMatchAll(rule)) return []; const filter = (rule.criteria ?? {}) as any; try { const rows = await this.engine.find(rule.object_name, { filter, - fields: ['id'], + fields, limit: 5000, // [#10119] The rule's own organization, when it has one. context: this.criteriaContext(rule), }); - return Array.isArray(rows) ? rows.map((r: any) => String(r.id)).filter(Boolean) : []; + return Array.isArray(rows) ? rows : []; } catch (err: any) { this.logger?.warn?.('[sharing-rule] criteria query failed', { rule: rule.name, error: err?.message }); return []; } } - private async recordMatches(rule: SharingRuleRow, recordId: string): Promise { - if (this.isInertMatchAll(rule)) return false; + /** + * The per-record half of the criteria read: the matched ROW, or `null`. + * + * [#15072] Was `recordMatches(): boolean`. It answers with the row because + * a `field` rule's recipients are ON it — the match read projects that + * column beside `id`, so one query answers "does this record match" and + * "whom does it name" together and the two can never be read from two + * versions of the row. Every other kind still projects `id` alone. + */ + private async matchRecord(rule: SharingRuleRow, recordId: string): Promise | null> { + if (this.isInertMatchAll(rule)) return null; const filter = { ...((rule.criteria ?? {}) as any), id: recordId }; + const fields = rule.recipient_type === 'field' ? ['id', rule.recipient_id] : ['id']; try { const rows = await this.engine.find(rule.object_name, { filter, - fields: ['id'], + fields, limit: 1, // [#10119] Same scope as the whole-rule sweep — see // {@link criteriaContext}. A per-record hook pass that stayed @@ -1069,13 +1199,162 @@ export class SharingRuleService implements ISharingRuleService { // rows the sweep no longer creates. context: this.criteriaContext(rule), }); - return Array.isArray(rows) && rows.length > 0; - } catch { - return false; + return Array.isArray(rows) && rows[0] ? (rows[0] as Record) : null; + } catch (err: any) { + // [#15072] For a `field` rule the read carries the recipient column, so + // a failure here is most likely the rule naming a column the object + // does not have (`INVALID_FIELD`) — an authoring defect that must be + // loud (ADR-0078), once per rule. The verdict stays fail-closed: no + // match, no grant, and the record's stale grants are revoked. + if (rule.recipient_type === 'field') this.warnFieldRuleOnce(rule, 'read-failed', err?.message); + return null; } } + /** + * [#15072] The recipients of ONE matched record — the per-record switch. + * + * `field` is the only member expanded here (maintainer ruling 2026-09-02 on + * #14103, B: "`expandRecipient` becomes per-record for that member only"): + * the user or users the row's named column holds. Every other member is the + * rule-wide expansion, memoised across the pass exactly as before + * ({@link expandRecipient}). + */ + private async expandRecipientForRecord( + rule: SharingRuleRow, + row: Record, + pass: RuleEvaluationPass, + ): Promise { + if (rule.recipient_type !== 'field') return this.expandRecipient(rule, pass); + return this.recipientsNamedOnRecord(rule, row); + } + + /** + * [#15072] Whom `row`'s recipient column names, fail-closed + * ({@link usersNamedBy}) — once the column has been judged usable + * ({@link fieldRecipientColumnIsUsable}). + */ + private recipientsNamedOnRecord(rule: SharingRuleRow, row: Record): string[] { + if (!this.fieldRecipientColumnIsUsable(rule)) return []; + return usersNamedBy(row[rule.recipient_id]); + } + + /** + * [#15072] Does the object DECLARE the column a `field` rule names as one + * that holds users? Judged from the registry on every call — a metadata + * reload may add the column, and a verdict cached at boot would outlive it + * (AGENTS.md "Startup registry reads") — and warned about once per rule. + * + * Three answers: + * - the engine cannot say (no `getSchema`, or an object it does not + * resolve) → the column is READ. Absence of a schema is absence of + * evidence, and the declared semantics are the column's value. + * - declared, and user-typed ({@link fieldHoldsUsers}) → read. + * - declared but absent, or of another type → NOBODY, and one warn naming + * the rule and the column. A rule that grants nobody in silence is the + * authoring trap ADR-0078 names, and a `text` column read as user ids + * would grant to whatever strings the records happen to hold. + */ + private fieldRecipientColumnIsUsable(rule: SharingRuleRow): boolean { + if (typeof this.engine.getSchema !== 'function') return true; + const schema = this.engine.getSchema(rule.object_name); + const fields = schema?.fields; + if (!fields || typeof fields !== 'object') return true; + const def = (fields as Record)[rule.recipient_id]; + if (fieldHoldsUsers(def)) return true; + this.warnFieldRuleOnce(rule, def ? 'not-user-typed' : 'no-such-field'); + return false; + } + + /** + * [#15072] An active `field` rule whose recipient column cannot be read as + * users says so — once per rule per cause ({@link fieldRuleWarned}). An + * EMPTY column is deliberately not a cause: an unassigned record sharing + * with nobody is the declared semantics, not a misconfiguration. + */ + private warnFieldRuleOnce( + rule: SharingRuleRow, + cause: 'no-such-field' | 'not-user-typed' | 'read-failed', + detail?: string, + ): void { + const key = `${String(rule.id ?? rule.name)}::${cause}`; + if (this.fieldRuleWarned.has(key)) return; + this.fieldRuleWarned.add(key); + this.logger?.warn?.( + '[sharing-rule] active field-recipient rule grants NOBODY — its recipient column cannot be read as ' + + 'users (logged once per rule per process). The rule must name a field of the shared object that ' + + 'exists and holds users (a user field, or a lookup to sys_user); fix the field name or set active: false', + { + rule: rule.name ?? rule.id, + object: rule.object_name, + recipientType: rule.recipient_type, + field: rule.recipient_id, + cause, + ...(detail ? { error: detail } : {}), + }, + ); + } + + /** + * [#15072] Every grant a whole-rule pass wants to exist, derived per + * recipient kind — the one place the rule-wide and the per-record members + * meet, so {@link reconcile} never has to know which it is diffing. + * + * Rule-wide members: the matched ids × ONE recipient expansion — the + * cartesian product `evaluateRule` always materialised, and a `position` + * recipient still expands exactly once per rule here (the ruling's explicit + * pin). `field`: the matched ROWS, each contributing a (record, user) pair + * for every user its own column names — no product, because there is no + * rule-wide recipient list to multiply by. `expandedUsers` for that kind is + * the number of DISTINCT users across the matched records. + */ + private async desiredGrantsForRule(rule: SharingRuleRow): Promise { + const desired = new Map(); + if (rule.recipient_type === 'field') { + const rows = (await this.findMatchingRows(rule, ['id', rule.recipient_id])) + .filter((r) => r.id != null && String(r.id) !== ''); + const users = new Set(); + for (const row of rows) { + const recordId = String(row.id); + for (const userId of this.recipientsNamedOnRecord(rule, row)) { + desired.set(`${recordId}::${userId}`, { record_id: recordId, recipient_id: userId }); + users.add(userId); + } + } + return { desired, matchedRecords: rows.length, expandedUsers: users.size }; + } + const matchedIds = await this.findMatchingRecords(rule); + const users = await this.expandRecipient(rule); + for (const recordId of matchedIds) { + for (const userId of users) desired.set(`${recordId}::${userId}`, { record_id: recordId, recipient_id: userId }); + } + return { desired, matchedRecords: matchedIds.length, expandedUsers: users.length }; + } + + /** + * The RULE-WIDE recipient expansion — "the one switch that decides" + * (`bu-tree-recompute.ts`), answered once per rule per pass. + * + * [#15072] `field` is deliberately NOT a case here, and asking for it is an + * error rather than an empty answer. A `field` recipient (maintainer ruling + * 2026-09-02 on #14103, B) is the user or users a column on EACH MATCHED + * RECORD names, so it has no rule-wide expansion at all — and answering `[]` + * (what the `queue` fallthrough below would have done) is precisely how a + * rule-wide caller turns "per record" into "nobody": `reconcile` would then + * revoke every grant, and `revokeRuleGrantsForRetiredRecipients` would + * retire every recipient. The three callers each decide explicitly: a + * caller with a record in hand goes through {@link expandRecipientForRecord}; + * the whole-rule pass derives per-record pairs in {@link desiredGrantsForRule}; + * the recipient-axis revoke declines `field` at its door. Reaching this + * throw is a programming error, and it is loud so that it stays one. + */ private async expandRecipient(rule: SharingRuleRow, pass: RuleEvaluationPass = {}): Promise { + if (rule.recipient_type === 'field') { + throw new Error( + `[sharing-rule] rule '${rule.name ?? rule.id}' has a field recipient, which expands per RECORD — ` + + 'there is no rule-wide expansion to answer (#15072)', + ); + } const team = new TeamGraphService({ engine: this.engine, organizationId: rule.organization_id ?? null, @@ -1166,7 +1445,9 @@ export class SharingRuleService implements ISharingRuleService { * reasons for an empty set that this issue did not measure. ⛔ Do not widen * this into "warn whenever any recipient expands to zero" without measuring * those — `queue` expands to `[]` by construction today, so a blanket warn - * would fire on every pass of every queue rule. + * would fire on every pass of every queue rule, and a `field` rule's empty + * column is a legitimate state (an unassigned record shares with nobody, + * #15072), not a misconfiguration. * * Named completely: the rule, the object, the recipient kind, the unit, the * organization, and the two causes worth checking first. Once per rule per @@ -1299,21 +1580,24 @@ export class SharingRuleService implements ISharingRuleService { } } - private async reconcile( - rule: SharingRuleRow, - matchedIds: string[], - users: string[], - ): Promise { + /** + * Diff a whole-rule pass's desired pairs against the rule's materialised + * grants: upsert what is wanted, revoke the remainder. + * + * [#15072] Takes the pairs ({@link DesiredGrantSet}) rather than + * `(matchedIds, users)`: the product of those two is what a rule-wide + * recipient wants, and it is built in {@link desiredGrantsForRule} — where + * a `field` rule builds its per-record pairs instead. The diff below is + * unchanged. + */ + private async reconcile(rule: SharingRuleRow, want: DesiredGrantSet): Promise { const existing = await this.engine.find('sys_record_share', { where: { source: 'rule', source_id: rule.id }, fields: ['id', 'record_id', 'recipient_id', 'access_level'], limit: 100000, context: SYSTEM_CTX, }); - const desired = new Map(); - for (const rid of matchedIds) { - for (const uId of users) desired.set(`${rid}::${uId}`, { record_id: rid, recipient_id: uId }); - } + const desired = want.desired; const existingMap = new Map(); for (const row of (existing ?? [])) existingMap.set(`${row.record_id}::${row.recipient_id}`, row); @@ -1367,8 +1651,8 @@ export class SharingRuleService implements ISharingRuleService { return { ruleId: rule.id, - matchedRecords: matchedIds.length, - expandedUsers: users.length, + matchedRecords: want.matchedRecords, + expandedUsers: want.expandedUsers, grantsCreated: created, grantsUpdated: updated, grantsRevoked: revoked, From 6bb483176bedf210ae20d885120978b1ab207166 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 06:19:23 +0000 Subject: [PATCH 2/5] test(plugin-sharing): pin the field-recipient doubles in the ledger; re-anchor the shifted system-context citations The new suite's fake engine is recorded in scripts/engine-double-contract.pinned.json (the gate's own --write), and the two elevation-read anchors the system-context page carries into sharing-rule-service.ts moved with the inserted module helpers (the census gate's own --fix; a shift, not a population change). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- content/docs/permissions/system-context.mdx | 2 +- .../plugins/plugin-sharing/src/field-recipient.test.ts | 2 -- scripts/engine-double-contract.pinned.json | 10 ++++++++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index d65bc425f4..0440dc8356 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -137,7 +137,7 @@ The largest single consumer — **17 of the 106 sites**. | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1088` | | 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `plugin-sharing/src/share-link-service.ts:469`, `:523`, `:527`, `:600`, `:630` | | 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | -| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:165`, `:390` | +| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:241`, `:466` | ### 4. Approvals, reports, attachments, comments, knowledge diff --git a/packages/plugins/plugin-sharing/src/field-recipient.test.ts b/packages/plugins/plugin-sharing/src/field-recipient.test.ts index 69ebab2570..7b0509de6f 100644 --- a/packages/plugins/plugin-sharing/src/field-recipient.test.ts +++ b/packages/plugins/plugin-sharing/src/field-recipient.test.ts @@ -182,8 +182,6 @@ function makeEngine() { return engine; } -type Engine = ReturnType; - const OBJECT = 'request'; const RULE = 'approved_to_assignees'; diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index ae302177dd..38de08d54c 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -2781,6 +2781,16 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/plugins/plugin-sharing/src/field-recipient.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/plugins/plugin-sharing/src/field-recipient.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-sharing/src/recipient-width.test.ts", "verb": "delete", From fc1a4cf56691ff36f5476aac42328c0d3a32a50d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 06:37:09 +0000 Subject: [PATCH 3/5] i18n(plugin-sharing): translate the field recipient option and help text; keep the tracker id out of the runtime string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The regenerated bundles carry the new select option and the two changed help strings; the zh-CN / ja-JP / es-ES leaves are translated rather than English fills, so no source-hash provenance row is left behind. The rule-wide refusal message drops its issue reference (check:doc-authoring) — the id lives in the adjacent comment. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../plugins/plugin-sharing/src/sharing-rule-service.ts | 4 +++- .../src/translations/en.objects.generated.ts | 7 ++++--- .../src/translations/es-ES.objects.generated.ts | 7 ++++--- .../src/translations/ja-JP.objects.generated.ts | 7 ++++--- .../src/translations/zh-CN.objects.generated.ts | 7 ++++--- 5 files changed, 19 insertions(+), 13 deletions(-) diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts index fc5321eefe..b9fda78835 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts @@ -1350,9 +1350,11 @@ export class SharingRuleService implements ISharingRuleService { */ private async expandRecipient(rule: SharingRuleRow, pass: RuleEvaluationPass = {}): Promise { if (rule.recipient_type === 'field') { + // The card: #15072. Kept out of the string — a runtime message reaches + // readers who cannot resolve a tracker id (check:doc-authoring). throw new Error( `[sharing-rule] rule '${rule.name ?? rule.id}' has a field recipient, which expands per RECORD — ` + - 'there is no rule-wide expansion to answer (#15072)', + 'there is no rule-wide expansion to answer', ); } const team = new TeamGraphService({ diff --git a/packages/plugins/plugin-sharing/src/translations/en.objects.generated.ts b/packages/plugins/plugin-sharing/src/translations/en.objects.generated.ts index 2da79799dd..94b4d9dcb8 100644 --- a/packages/plugins/plugin-sharing/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-sharing/src/translations/en.objects.generated.ts @@ -136,18 +136,19 @@ export const enObjects: NonNullable = { }, recipient_type: { label: "Recipient Type", - help: "Who receives access. Picking a team, business unit or position gives access to everyone in it. \"Business unit and subordinates\" also covers every unit below the one you pick.", + help: "Who receives access. Picking a team, business unit or position gives access to everyone in it. \"Business unit and subordinates\" also covers every unit below the one you pick. \"Field\" shares each record with the user or users held in one of its own fields — name that field as the recipient.", options: { user: "user", team: "team", business_unit: "business_unit", position: "position", - unit_and_subordinates: "unit_and_subordinates" + unit_and_subordinates: "unit_and_subordinates", + field: "field" } }, recipient_id: { label: "Recipient", - help: "The specific user, team, business unit or position that receives access." + help: "The specific user, team, business unit or position that receives access — or, for the \"Field\" recipient type, the name of the record field that holds the user or users to share with." }, access_level: { label: "Access Level", diff --git a/packages/plugins/plugin-sharing/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-sharing/src/translations/es-ES.objects.generated.ts index 01390fa9fd..9b93bcb751 100644 --- a/packages/plugins/plugin-sharing/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-sharing/src/translations/es-ES.objects.generated.ts @@ -136,18 +136,19 @@ export const esESObjects: NonNullable = { }, recipient_type: { label: "Tipo de destinatario", - help: "Quién recibe el acceso. Al elegir un equipo, una unidad de negocio o un puesto, lo reciben todas las personas que lo integran. «Unidad de negocio y subordinadas» abarca además todas las unidades por debajo de la elegida.", + help: "Quién recibe el acceso. Al elegir un equipo, una unidad de negocio o un puesto, lo reciben todas las personas que lo integran. «Unidad de negocio y subordinadas» abarca además todas las unidades por debajo de la elegida. «Campo» comparte cada registro con el usuario o los usuarios guardados en uno de sus propios campos: indique ese campo como destinatario.", options: { user: "Usuario", team: "Equipo", business_unit: "Unidad de negocio", position: "Puesto", - unit_and_subordinates: "Unidad de negocio y subordinados" + unit_and_subordinates: "Unidad de negocio y subordinados", + field: "Campo" } }, recipient_id: { label: "Destinatario", - help: "El usuario, equipo, unidad de negocio o puesto concreto que recibe el acceso." + help: "El usuario, equipo, unidad de negocio o puesto concreto que recibe el acceso o, para el tipo de destinatario «Campo», el nombre del campo del registro que guarda el usuario o los usuarios con quienes compartir." }, access_level: { label: "Nivel de acceso", diff --git a/packages/plugins/plugin-sharing/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-sharing/src/translations/ja-JP.objects.generated.ts index 8ac2b7d9b0..d5611312f0 100644 --- a/packages/plugins/plugin-sharing/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-sharing/src/translations/ja-JP.objects.generated.ts @@ -136,18 +136,19 @@ export const jaJPObjects: NonNullable = { }, recipient_type: { label: "受信者タイプ", - help: "誰にアクセスを与えるか。チーム・事業単位・役職を選ぶと、そこに属する全員がアクセスできます。「事業単位と下位組織」を選ぶと、その配下のすべての単位も対象になります。", + help: "誰にアクセスを与えるか。チーム・事業単位・役職を選ぶと、そこに属する全員がアクセスできます。「事業単位と下位組織」を選ぶと、その配下のすべての単位も対象になります。「フィールド」を選ぶと、各レコードはそのレコード自身のフィールドに保持されているユーザーに共有されます。そのフィールドを受信者として指定してください。", options: { user: "ユーザー", team: "チーム", business_unit: "ビジネスユニット", position: "ポジション", - unit_and_subordinates: "ビジネスユニットと下位階層" + unit_and_subordinates: "ビジネスユニットと下位階層", + field: "フィールド" } }, recipient_id: { label: "受信者", - help: "アクセスを与える具体的なユーザー・チーム・事業単位・役職。" + help: "アクセスを与える具体的なユーザー・チーム・事業単位・役職。受信者種別が「フィールド」の場合は、共有先のユーザーを保持しているレコードのフィールド名。" }, access_level: { label: "アクセスレベル", diff --git a/packages/plugins/plugin-sharing/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-sharing/src/translations/zh-CN.objects.generated.ts index ebf859204c..7e53a72d10 100644 --- a/packages/plugins/plugin-sharing/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-sharing/src/translations/zh-CN.objects.generated.ts @@ -136,18 +136,19 @@ export const zhCNObjects: NonNullable = { }, recipient_type: { label: "接收方类型", - help: "把访问权限给谁。选择团队、业务单元或岗位时，其中的每个人都会获得访问权限；选择「业务单元及下级」还会包含该单元下属的所有单元。", + help: "把访问权限给谁。选择团队、业务单元或岗位时，其中的每个人都会获得访问权限；选择「业务单元及下级」还会包含该单元下属的所有单元；选择「字段」时，每条记录会共享给该记录自身某个字段里保存的用户——把那个字段填作接收方即可。", options: { user: "用户", team: "团队", business_unit: "业务单元", position: "岗位", - unit_and_subordinates: "业务单元及下级" + unit_and_subordinates: "业务单元及下级", + field: "字段" } }, recipient_id: { label: "接收方", - help: "具体把访问权限给哪一个用户、团队、业务单元或岗位。" + help: "具体把访问权限给哪一个用户、团队、业务单元或岗位；接收方类型为「字段」时，填写保存共享对象用户的那个记录字段的名称。" }, access_level: { label: "访问级别", From 78cca29ea86d639105a793fe512f5b2e9f0331dc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 06:57:07 +0000 Subject: [PATCH 4/5] docs(permissions): re-derive the system-context census anchors on the merged tree; re-sync the engine-double ledger The os-regen merge driver does not text-merge system-context.mdx; regenerated with pnpm gen:system-context-census on the committed merge as its own commit. The engine-double-contract ledger is re-written by the gate's own --write on the same tree. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- content/docs/permissions/system-context.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content/docs/permissions/system-context.mdx b/content/docs/permissions/system-context.mdx index 0440dc8356..6568db9856 100644 --- a/content/docs/permissions/system-context.mdx +++ b/content/docs/permissions/system-context.mdx @@ -137,7 +137,7 @@ The largest single consumer — **17 of the 106 sites**. | 36 | `sys_record_share` reads are **not** self-scoped | Get: tenant-wide share listing without `manage_sharing` | `sharing-plugin.ts:1088` | | 37 | Share-link policy `enabled` check bypassed; system callers re-enter under a system context | Get: link **creation** while the policy is off — resolution is **not** bypassed since #14033 (`publicSharing.enabled` is a standing policy held at every redemption): a link minted this way does not resolve until the block is enabled | `plugin-sharing/src/share-link-service.ts:469`, `:523`, `:527`, `:600`, `:630` | | 38 | Sharing-rule provenance stamp skipped | Lose: the row is not marked as an admin customization — seeder / `defineRule` / boot reconcilers are "the package door" | `sharing-rule-provenance.ts:47` | -| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:241`, `:466` | +| 39 | Sharing-rule service write + delete paths return early | Lose: the manage-rules gate on the service surface, and the platform-global-rule delete guard | `sharing-rule-service.ts:278`, `:503` | ### 4. Approvals, reports, attachments, comments, knowledge From fe9281910a564c069763d5116331d544f0bc5aea Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 07:05:36 +0000 Subject: [PATCH 5/5] test(plugin-sharing): pin that the per-grant catch and the field recipient compose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After merging #14930 into this branch the two changes share reconcile: the whole-rule pass diffs a DesiredGrantSet and attempts each grant individually. Pinned in both directions on the field kind — a refused grant is counted and the pass (with its stale-row revocations) continues on both reconcile paths; the catch stays narrow on an unrelated engine error; the rule-wide switch's refusal of a field rule carries no engine code and is never reached by a production pass. Also tidies the blank line the merge left before reconcile. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --- .../src/field-recipient.test.ts | 108 ++++++++++++++++++ .../src/sharing-rule-service.ts | 1 - 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/packages/plugins/plugin-sharing/src/field-recipient.test.ts b/packages/plugins/plugin-sharing/src/field-recipient.test.ts index 7b0509de6f..6853192c4a 100644 --- a/packages/plugins/plugin-sharing/src/field-recipient.test.ts +++ b/packages/plugins/plugin-sharing/src/field-recipient.test.ts @@ -114,7 +114,19 @@ function makeEngine() { const f = opts?.filter ?? opts?.where; return ensure(o).filter((r) => matches(r, f)).slice(0, opts?.limit ?? 10000); }, + /** + * [#14754] A recipient whose `sys_record_share` INSERT the engine refuses, + * with the organization-rule code the per-grant catch absorbs (or another + * code, to prove the catch stays narrow). Empty = nothing refused. + */ + _refuseGrantFor: '' as string, + _refuseGrantCode: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED' as string, async insert(o: string, data: any) { + if (o === 'sys_record_share' && engine._refuseGrantFor && data?.recipient_id === engine._refuseGrantFor) { + const err: any = new Error(`Insert on '${o}' was refused: the write resolves no organization`); + err.code = engine._refuseGrantCode; + throw err; + } const row = { id: data.id ?? `${o}_${++seq}`, ...data }; ensure(o).push(row); return row; @@ -806,3 +818,99 @@ describe('#15072 authoring seams', () => { }); }); }); + +// ───────────────────────────────────────────────────────────────────────── +/** + * [#15072 × #14754] The per-grant catch (#14930, landed while this card was in + * flight) and the per-record recipient kind compose — pinned in both + * directions, because they meet in the same `reconcile`: + * + * - a refused grant inside a FIELD rule's pass is counted and the pass, with + * its stale-row revocations, continues (the #14754 contract holds for the + * new kind on both reconcile paths); + * - the catch is NARROW and does not absorb this card's deliberate refusal: + * the rule-wide `expandRecipient` THROWS for a field rule (never `[]`), that + * error carries no engine code, and no production pass ever reaches it. + */ +describe('#15072 × #14754 — the per-grant catch and the per-record kind compose', () => { + let h: ReturnType; + + /** The organization-refusal lines the pass logged. */ + const refusalWarns = () => + h.warn.mock.calls.filter((c) => String(c[0]).includes('refused by the engine organization rule')); + + beforeEach(() => { + h = harness(); + h.engine.seed('sys_sharing_rule', [fieldRule()]); + h.engine.seed(OBJECT, [ + { id: 'req_1', status: 'approved', owner_id: 'boss', assignees: ['u_a'] }, + { id: 'req_2', status: 'approved', owner_id: 'boss', assignees: ['u_refused', 'u_c'] }, + ]); + }); + + it('whole-rule pass: ONE refused pair is counted, the other pairs land, and the stale row is still revoked', async () => { + h.seedStaleGrant('req_1', 'u_old'); + h.engine._refuseGrantFor = 'u_refused'; + + const result = await h.rules.evaluateRule(RULE, SYS); + + expect(h.granteesOf('req_1')).toEqual(['u_a']); // u_old revoked — the security half + expect(h.granteesOf('req_2')).toEqual(['u_c']); // the pair AFTER the refused one landed + expect(result).toMatchObject({ matchedRecords: 2, expandedUsers: 3, grantsCreated: 2, grantsRevoked: 1, grantsRefused: 1 }); + const warns = refusalWarns(); + expect(warns).toHaveLength(1); + expect(warns[0][1]).toMatchObject({ rule: RULE, object: OBJECT, record: 'req_2', recipient: 'u_refused', code: 'ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED' }); + }); + + it('per-record pass: the refused user does not stop that record\'s other user, nor its own stale revocation', async () => { + h.seedStaleGrant('req_2', 'u_old'); + h.engine._refuseGrantFor = 'u_refused'; + + const [result] = await h.rules.evaluateAllForRecord(OBJECT, 'req_2', SYS); + + expect(h.granteesOf('req_2')).toEqual(['u_c']); + expect(result).toMatchObject({ matchedRecords: 1, expandedUsers: 2, grantsCreated: 1, grantsRevoked: 1, grantsRefused: 1 }); + }); + + it('the catch stays NARROW on the new kind too: an unrelated engine error still aborts the pass', async () => { + h.engine._refuseGrantFor = 'u_refused'; + h.engine._refuseGrantCode = 'SOME_OTHER_ENGINE_ERROR'; + await expect(h.rules.evaluateRule(RULE, SYS)).rejects.toMatchObject({ code: 'SOME_OTHER_ENGINE_ERROR' }); + expect(refusalWarns()).toEqual([]); + }); + + it('the rule-wide switch REFUSES a field rule with a code-less error — nothing the per-grant catch could absorb', async () => { + const rule = (await h.rules.getRule('srule_assignees', SYS))!; + let thrown: any; + try { + await (h.rules as any).expandRecipient(rule); + } catch (err) { + thrown = err; + } + expect(thrown).toBeInstanceOf(Error); + expect(String(thrown.message)).toMatch(/field recipient, which expands per RECORD/); + // No `code` at all: `grantOrAbsorbOrganizationRefusal` compares `err.code` + // against the ONE engine code it absorbs, and this error can never equal it. + expect(thrown.code).toBeUndefined(); + expect(thrown.code).not.toBe('ERR_SYSTEM_WRITE_ORGANIZATION_REQUIRED'); + }); + + it('no production pass reaches that refusal for a field rule — and the rule-wide switch still serves the other kinds', async () => { + h.engine.seed('sys_sharing_rule', [fieldRule({ + id: 'srule_alice', name: 'approved_to_alice', recipient_type: 'user', recipient_id: 'alice', + })]); + const spy = vi.spyOn(h.rules as any, 'expandRecipient'); + + await h.rules.evaluateRule(RULE, SYS); + await h.rules.evaluateAllRulesForObject(OBJECT); + await h.rules.evaluateAllForRecord(OBJECT, 'req_2', SYS); + + const kinds = spy.mock.calls.map((c: any[]) => (c[0] as Row).recipient_type); + expect(kinds).not.toContain('field'); + expect(kinds.filter((k) => k === 'user').length).toBeGreaterThan(0); // the control kind went through it + // …and every pass completed with the field grants materialised. + expect(h.granteesOf('req_2')).toEqual(['u_c', 'u_refused']); + expect(h.granteesOf('req_1', 'srule_alice')).toEqual(['alice']); + spy.mockRestore(); + }); +}); diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts index 4e71a4c2a5..dafa6fddf4 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts @@ -1695,7 +1695,6 @@ export class SharingRuleService implements ISharingRuleService { } } - /** * Diff a whole-rule pass's desired pairs against the rule's materialised * grants: upsert what is wanted, revoke the remainder.