Skip to content

sharing rules: the business_unit recipient expands the whole SUBTREE, but the spec declares it as exactly one unit (no subtree) #7807

Description

@os-help

Symptom

⚠️Potentially security-relevant (over-grant). The two business-unit recipient kinds are declared as different widths and enforced as the same width. A sharing rule authored with recipient_type: 'business_unit', expecting to reach exactly one unit's members, in fact reaches that unit plus every descendant unit's members.

Declared

packages/spec/src/security/sharing.zod.ts, on ShareRecipientType:

  • unit_and_subordinates — a business unit plus every descendant unit's members (ADR-0057 D5 subtree widening).
  • business_unit — exactly one business unit's members (no subtree).

packages/lint/src/validate-org-axis-red-lines.ts carries the same table and calls unit_and_subordinates the "strictly WIDER grant" of the pair.

Enforced

SharingRuleService.expandRecipient routes both kinds through the identical call. On origin/main at 098b629, packages/plugins/plugin-sharing/src/sharing-rule-service.ts:

if(rule.recipient_type==='business_unit'){constdept=newBusinessUnitGraphService({/* … */});returndept.expandUsers(rule.recipient_id);}// …if(rule.recipient_type==='unit_and_subordinates'){constdept=newBusinessUnitGraphService({/* … */});returndept.expandUsers(rule.recipient_id);}

BusinessUnitGraphService.expandUsers opens with const units = await this.descendants(businessUnitId) — a BFS over parent_business_unit_id — and then expands members over business_unit_id: { $in: units }. So business_unit walks the subtree. The two branches differ only in their comments.

Consequence: on a three-level tree, a business_unit rule anchored at a division silently grants to every department and office beneath it. The wider of the two declared kinds is not wider at all, which also makes the distinction the lint red-line draws between them unenforceable in practice.

Which way to fix is a decision, not a detail

Both directions are defensible and they are not equivalent:

  1. Narrow the runtime to the declarationbusiness_unit expands members of that unit only. Matches the spec, the lint table and ADR-0057 D5's framing of unit_and_subordinates as the widening. Reduces access for any deployment that already authored business_unit rules and is relying, knowingly or not, on the subtree behaviour, so it needs a release note at minimum.
  2. Change the declaration to match the runtime — and then business_unit and unit_and_subordinates are one kind with two names, which raises whether one should be retired under ADR-0049 enforce-or-remove.

Worth checking before either: whether any shipped example app or seeded rule authors business_unit, since that decides how much real pull direction 1's narrowing has to overcome.

Source

Found while implementing #7729 (lazy revocation on BU-tree changes, PR #7806). Out of scope there and deliberately not fixed in that PR: #7729 is a timing defect and its fix must cover whatever expandRecipient reads today, which is why that PR covers both kinds. Covering both stays correct whichever way this issue is decided.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions