fix(plugin-sharing): business_unit recipient expands exactly one unit, not the subtree (#7807) (#8097)

The two business-unit recipient kinds were declared as two widths and enforced
as one. `SharingRuleService.expandRecipient` routed both through the identical
`BusinessUnitGraphService.expandUsers` call, whose first act is a BFS over
`parent_business_unit_id`, so the branches differed only in their comments.

A rule authored `recipient_type: 'business_unit'` — declared by the spec, the
lint red-line table and ADR-0057 D5 as "exactly one business unit's members (no
subtree)" — reached that unit plus every descendant unit's members. On a
three-level tree a division-anchored rule silently granted to every department
and office beneath it, and `unit_and_subordinates`, documented as the strictly
wider grant of the pair, was not wider at all.

Maintainer ruling 2026-08-12, direction 1: narrow the runtime to the
declaration. `business_unit` now resolves through a new
`expandUnitMembers()` — no descent — while `unit_and_subordinates` keeps the
subtree walk unchanged. Both kinds survive; neither is merged or retired.
`expandUsers()` keeps its contract meaning for the `bu:` approver prefix and
org rollups. The narrow width caches separately, since one map keyed by unit id
would let whichever width ran first answer for the other.

Both widths are pinned as a pair on one three-level fixture, at the graph, the
service and the REST level, because a change narrowing BOTH kinds would satisfy
the headline while destroying the distinction. Verified by two ablations: with
the narrowing reverted exactly the 8 narrow assertions go red and every wide one
stays green; with both branches narrowed exactly the wide assertions go red.


Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-zhuang

.changeset/business-unit-recipient-exact-width.md

@@ -0,0 +1,67 @@
+---
+"@objectstack/plugin-sharing": patch
+---
+
+fix(plugin-sharing): the `business_unit` sharing-rule recipient expands exactly one unit, not its whole subtree (#7807)
+
+⚠️ **This is an intentional over-grant fix, and it REDUCES visible rows for any
+deployment that authored `business_unit` sharing rules.** Read the migration note
+below before upgrading if you use that recipient kind.
+
+## What was wrong
+
+The two business-unit recipient kinds were **declared** as two different widths
+and **enforced** as the same width. `SharingRuleService.expandRecipient` routed
+both through the identical `BusinessUnitGraphService.expandUsers` call, whose
+first act is a BFS over `parent_business_unit_id` — so the two branches differed
+only in their comments.
+
+A rule authored as `recipient_type: 'business_unit'`, which the authoring spec
+(`ShareRecipientType`), the org-axis lint red-line table and ADR-0057 D5 all
+describe as *"exactly one business unit's members (no subtree)"*, in fact reached
+that unit **plus every descendant unit's members**. On a three-level tree a rule
+anchored at a division silently granted to every department and office beneath
+it.
+
+Two consequences, and the second is why this is filed as security rather than
+tidiness: the narrow spelling over-granted **silently**, which is the worst
+failure shape for generated security metadata (an agent that asks for the narrow
+grant should get the narrow grant); and `unit_and_subordinates`, documented as
+the *strictly wider* grant of the pair, was not wider at all, leaving the
+distinction the lint red-line draws unenforceable in practice.
+
+## What changed
+
+`business_unit` now resolves through a new
+`BusinessUnitGraphService.expandUnitMembers()` — members whose
+`business_unit_id` equals the named unit, with no descent. It keeps every other
+guarantee the subtree walk had: an inactive or out-of-tenant anchor unit
+contributes nobody, and an unreadable unit fails closed rather than granting.
+
+`unit_and_subordinates` is **unchanged** and keeps the subtree walk — it is the
+kind whose declared semantics *is* the hierarchy widening (ADR-0057 D5). The two
+kinds remain two kinds; neither is merged into the other or retired.
+`expandUsers()` also keeps its meaning for the `bu:` approver prefix and org
+rollups, which are subtree consumers by contract.
+
+Grant recomputation on business-unit graph writes (#7729) still covers both
+recipient kinds, because a unit-only expansion still reads `sys_business_unit`
+for its anchor's `active` flag and tenant scope and `sys_business_unit_member`
+for its members. What changed there is blast radius, not coverage: re-parenting a
+unit no longer moves a `business_unit` rule's recipients, while deactivating the
+anchor or editing its membership still does.
+
+## Migration
+
+**In-tree cost is zero** — no shipped example app or seeded rule authors
+`business_unit` (the showcase and CRM apps use `position` and
+`unit_and_subordinates`), so nothing in this repository changes behaviour.
+
+**Out-of-tree deployments:** if you authored a `business_unit` rule and were
+relying — knowingly or not — on it reaching descendant units, those descendant
+members **lose the grants that rule materialised**. Grants are reconciled on the
+next evaluation pass, so the reduction lands without any action on your part.
+
+If the subtree reach was what you actually wanted, change the rule's recipient to
+`unit_and_subordinates`, which has always meant exactly that and is unaffected by
+this release. If you wanted the narrow grant, you now have it.

packages/plugins/plugin-sharing/src/bu-tree-recompute.test.ts

@@ -319,7 +319,14 @@ describe('#7729 business-unit graph writes recompute BU-tree sharing rules', ()
     expect(grantsFor('priya')).toBe(0);
   });
 
-  it('covers `business_unit` recipients too — today they walk the same subtree resolver', async () => {
+  // [#7807] narrowed `business_unit` to exactly one unit's members. This case
+  // still belongs here, and deliberately: a unit-only expansion still reads
+  // `sys_business_unit` (its own `active` flag, its tenant scope) and
+  // `sys_business_unit_member`, so a write to either table can still move who
+  // the rule reaches. The anchor below is the unit priya is a DIRECT member
+  // of, which is what makes this a test of RECOMPUTE COVERAGE rather than of
+  // the subtree walk.
+  it('covers `business_unit` recipients too — a unit-only expansion still reads both BU tables', async () => {
     engine._tables.sys_sharing_rule[0].recipient_type = 'business_unit';
     engine._tables.sys_sharing_rule[0].recipient_id = 'bu_west';
     await rules.evaluateRule(RULE, SYS);

packages/plugins/plugin-sharing/src/bu-tree-recompute.ts

@@ -30,19 +30,22 @@
  * |-------------------------|---------------------------------------------|---|
  * | `user`                  | the literal id                              | no |
  * | `team`                  | `TeamGraphService` (`sys_team_member`, `sys_member`, `sys_user`) | no |
- * | `business_unit`         | `BusinessUnitGraphService.expandUsers`      | YES |
+ * | `business_unit`         | `BusinessUnitGraphService.expandUnitMembers` | YES |
  * | `position`              | `PositionGraphService` (`sys_user_position`, `sys_member`) | no |
  * | `unit_and_subordinates` | `BusinessUnitGraphService.expandUsers`      | YES |
  * | `queue`                 | returns `[]` (no `sys_queue` yet)           | no |
  *
- * `business_unit` is in that set on the strength of what the code does today:
- * `expandRecipient` routes it through the SAME `expandUsers` call as
- * `unit_and_subordinates`, so it walks `descendants()` and is just as exposed
- * to a re-parent. (The spec declares it as "exactly one business unit's
- * members (no subtree)" — that divergence is a separate defect and is filed
- * separately; covering the kind here is correct under either reading, since a
- * unit-only expansion still reads `sys_business_unit` for its own `active`
- * flag and `sys_business_unit_member` for its members.)
+ * `business_unit` stays in that set after #7807 narrowed it to exactly one
+ * unit's members. The divergence this file originally noted — `expandRecipient`
+ * routing it through the SAME subtree `expandUsers` call as
+ * `unit_and_subordinates`, against a spec declaring it as "exactly one business
+ * unit's members (no subtree)" — was resolved in favour of the declaration, and
+ * membership here was correct under either reading for the reason that fix
+ * confirmed: a unit-only expansion still reads `sys_business_unit` for its own
+ * `active` flag and tenant scope, and `sys_business_unit_member` for its
+ * members. What changed is the blast radius, not the coverage — a re-parent no
+ * longer moves a `business_unit` rule's recipients (its anchor's own membership
+ * is what moves them), while a deactivation or a membership edit still does.
  *
  * Everything else is deliberately NOT recomputed. That exclusion is a
  * requirement, not an optimisation: a fix that recomputed every rule on every

packages/plugins/plugin-sharing/src/business-unit-graph.test.ts

@@ -113,6 +113,83 @@ describe('BusinessUnitGraphService — subtree expansion', () => {
   });
 });
 
+/**
+ * [#7807] The two widths, pinned as a PAIR on one three-level tree.
+ *
+ * A division ⊃ department ⊃ office tree is the floor for this: on a two-level
+ * fixture "exactly one unit" and "unit plus its children" can agree by
+ * accident, so a two-level pin cannot tell the fixed behaviour from the
+ * defect. Each assertion below names the width it guards, because a change
+ * that narrowed BOTH kinds would satisfy the `business_unit` half while
+ * destroying the distinction the spec draws between them.
+ */
+const DIV_UNITS: UnitRow[] = [
+  { id: 'bu_div', organization_id: null, active: true },
+  { id: 'bu_dept', parent_business_unit_id: 'bu_div', organization_id: null, active: true },
+  { id: 'bu_office', parent_business_unit_id: 'bu_dept', organization_id: null, active: true },
+];
+const DIV_MEMBERS: MemberRow[] = [
+  { business_unit_id: 'bu_div', user_id: 'u_div' },
+  { business_unit_id: 'bu_dept', user_id: 'u_dept' },
+  { business_unit_id: 'bu_office', user_id: 'u_office' },
+];
+
+describe('BusinessUnitGraphService — the two widths are actually two widths (#7807)', () => {
+  const graph = () => new BusinessUnitGraphService({ engine: makeEngine(DIV_UNITS, DIV_MEMBERS) });
+
+  it('NARROW — expandUnitMembers returns only the named unit, three levels notwithstanding', async () => {
+    expect(await graph().expandUnitMembers('bu_div')).toEqual(['u_div']);
+  });
+
+  it('WIDE — expandUsers still returns the whole subtree (the control)', async () => {
+    expect((await graph().expandUsers('bu_div')).sort()).toEqual(['u_dept', 'u_div', 'u_office']);
+  });
+
+  it('the narrow width skips even a DIRECT child, not merely the grandchild', async () => {
+    const users = await graph().expandUnitMembers('bu_div');
+    expect(users).not.toContain('u_dept');
+    expect(users).not.toContain('u_office');
+  });
+
+  it('a mid-tree unit expands to its own members only', async () => {
+    expect(await graph().expandUnitMembers('bu_dept')).toEqual(['u_dept']);
+  });
+
+  it('an inactive unit contributes nobody to the narrow width either', async () => {
+    const units = DIV_UNITS.map((u) => (u.id === 'bu_div' ? { ...u, active: false } : u));
+    const g = new BusinessUnitGraphService({ engine: makeEngine(units, DIV_MEMBERS) });
+    expect(await g.expandUnitMembers('bu_div')).toEqual([]);
+  });
+
+  it('an unknown unit expands to nobody rather than to everybody', async () => {
+    expect(await graph().expandUnitMembers('bu_nope')).toEqual([]);
+    expect(await graph().expandUnitMembers('')).toEqual([]);
+  });
+
+  it('the narrow width is org-predicated exactly like the wide one', async () => {
+    const g = new BusinessUnitGraphService({
+      engine: makeEngine(DIV_UNITS, DIV_MEMBERS),
+      organizationId: 'org_a',
+    });
+    // Seeded (null-org) units are not visible to an org-scoped rule — the
+    // same `[divergence]` posture the wide width holds below.
+    expect(await g.expandUnitMembers('bu_div')).toEqual([]);
+  });
+
+  it('the two widths do NOT share a cache entry for the same unit id', async () => {
+    // Both maps are keyed by BU id. One shared map would let whichever width
+    // ran first answer for the other — the over-grant returning through the
+    // cache door.
+    const g = graph();
+    expect(await g.expandUnitMembers('bu_div')).toEqual(['u_div']);
+    expect((await g.expandUsers('bu_div')).sort()).toEqual(['u_dept', 'u_div', 'u_office']);
+    // …and in the opposite order, on a fresh instance.
+    const g2 = graph();
+    expect((await g2.expandUsers('bu_div')).sort()).toEqual(['u_dept', 'u_div', 'u_office']);
+    expect(await g2.expandUnitMembers('bu_div')).toEqual(['u_div']);
+  });
+});
+
 describe('BusinessUnitGraphService — org scoping (#3807)', () => {
   it('an org-less rule (today’s materialized shape) expands seeded units fine', async () => {
     // `expandRecipient` passes `rule.organization_id ?? null`, and every

packages/plugins/plugin-sharing/src/business-unit-graph.ts

@@ -9,6 +9,15 @@ const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const;
 type DeptCache = {
   descendants?: Map<string, string[]>;
   expandUsers?: Map<string, string[]>;
+  /**
+   * [#7807] Members of exactly one unit — a SEPARATE map from
+   * {@link DeptCache.expandUsers} on purpose. Both are keyed by business-unit
+   * id but answer different questions, so sharing one map would let a narrow
+   * `business_unit` expansion be served a cached subtree answer (re-opening
+   * the over-grant this issue closed) or vice versa, depending only on which
+   * recipient kind happened to be evaluated first in the pass.
+   */
+  unitMembers?: Map<string, string[]>;
   head?: Map<string, string | null>;
 };
 
@@ -34,6 +43,14 @@ export interface BusinessUnitGraphOptions {
  * `active` flag as a hard filter (inactive departments contribute no
  * members and stop BFS descent into their subtrees).
  *
+ * Two DIFFERENT widths live here, and keeping them distinct is the point
+ * (#7807):
+ *   - {@link BusinessUnitGraphService.expandUsers} — the unit PLUS every
+ *     descendant unit (the `IBusinessUnitGraphService` contract; drives the
+ *     `unit_and_subordinates` recipient).
+ *   - {@link BusinessUnitGraphService.expandUnitMembers} — exactly that one
+ *     unit's members (drives the `business_unit` recipient).
+ *
  * Reuses {@link TeamGraphService.managerOf} for user-level manager
  * lookup so callers can use this single service in approval / sharing
  * pipelines.
@@ -50,6 +67,7 @@ export class BusinessUnitGraphService implements IBusinessUnitGraphService {
     this.cache = opts.cache ?? {};
     this.cache.descendants ??= new Map();
     this.cache.expandUsers ??= new Map();
+    this.cache.unitMembers ??= new Map();
     this.cache.head ??= new Map();
     this.teamGraph = opts.teamGraph;
   }
@@ -60,20 +78,7 @@ export class BusinessUnitGraphService implements IBusinessUnitGraphService {
     if (cached) return cached;
 
     // Verify seed itself is active + within tenant scope.
-    let seedActive = true;
-    try {
-      const seedRows = await this.engine.find('sys_business_unit', {
-        where: this.orgScope({ id: businessUnitId }),
-        fields: ['id', 'active'],
-        limit: 1,
-        context: SYSTEM_CTX,
-      });
-      const seedRow: any = Array.isArray(seedRows) ? seedRows[0] : null;
-      if (!seedRow) seedActive = false;
-      else if (seedRow.active === false) seedActive = false;
-    } catch {
-      seedActive = false;
-    }
+    const seedActive = await this.seedIsUsable(businessUnitId);
     if (!seedActive) {
       this.cache.descendants!.set(businessUnitId, []);
       return [];
@@ -107,6 +112,79 @@ export class BusinessUnitGraphService implements IBusinessUnitGraphService {
     return out;
   }
 
+  /**
+   * Is the seed unit itself active and inside the tenant scope?
+   *
+   * Shared by both widths so they agree on what an unusable seed is: a unit
+   * that does not exist, sits in another organization, or carries
+   * `active: false` contributes NOBODY — it is never merely "expanded without
+   * its descendants". A read failure answers `false` (fail closed: an
+   * unreadable unit must not grant).
+   */
+  private async seedIsUsable(businessUnitId: string): Promise<boolean> {
+    try {
+      const seedRows = await this.engine.find('sys_business_unit', {
+        where: this.orgScope({ id: businessUnitId }),
+        fields: ['id', 'active'],
+        limit: 1,
+        context: SYSTEM_CTX,
+      });
+      const seedRow: any = Array.isArray(seedRows) ? seedRows[0] : null;
+      if (!seedRow) return false;
+      if (seedRow.active === false) return false;
+      return true;
+    } catch {
+      return false;
+    }
+  }
+
+  /**
+   * [#7807] Members of EXACTLY ONE business unit — no subtree descent.
+   *
+   * This is the enforcement of the `business_unit` sharing-rule recipient,
+   * which the spec (`ShareRecipientType`), the lint red-line table and
+   * ADR-0057 D5 all declare as "exactly one business unit's members (no
+   * subtree)". Until #7807 the runtime routed it through
+   * {@link BusinessUnitGraphService.expandUsers} instead, so a rule anchored
+   * at a division silently reached every department and office beneath it —
+   * an over-grant, and one that made the strictly-wider `unit_and_subordinates`
+   * kind not wider at all.
+   *
+   * Deliberately NOT a variant of `expandUsers`: that method is the
+   * `IBusinessUnitGraphService` contract's SUBTREE expansion ("all user ids in
+   * `businessUnitId` or any descendant business unit") and keeps that meaning
+   * for `unit_and_subordinates`, the `bu:` approver prefix and org rollups.
+   * The two widths are now two methods rather than one method and two
+   * comments.
+   */
+  async expandUnitMembers(businessUnitId: string): Promise<string[]> {
+    if (!businessUnitId) return [];
+    const cached = this.cache.unitMembers!.get(businessUnitId);
+    if (cached) return cached;
+
+    if (!(await this.seedIsUsable(businessUnitId))) {
+      this.cache.unitMembers!.set(businessUnitId, []);
+      return [];
+    }
+
+    let rows: any[] = [];
+    try {
+      rows = await this.engine.find('sys_business_unit_member', {
+        where: { business_unit_id: businessUnitId },
+        fields: ['user_id'],
+        limit: 10000,
+        context: SYSTEM_CTX,
+      });
+    } catch {
+      rows = [];
+    }
+    const users = Array.from(
+      new Set((rows ?? []).map((r: any) => String(r.user_id ?? '')).filter(Boolean)),
+    );
+    this.cache.unitMembers!.set(businessUnitId, users);
+    return users;
+  }
+
   async expandUsers(businessUnitId: string): Promise<string[]> {
     if (!businessUnitId) return [];
     const cached = this.cache.expandUsers!.get(businessUnitId);