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

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') {
  const dept = new BusinessUnitGraphService({ /* … */ });
  return dept.expandUsers(rule.recipient_id);
}
// …
if (rule.recipient_type === 'unit_and_subordinates') {
  const dept = new BusinessUnitGraphService({ /* … */ });
  return dept.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:

Narrow the runtime to the declaration — business_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.
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.

docs(permissions): add the business-unit write as the fourth withdraw…

Findings-triage route B: regraded — finding → needs-user-decision, target:v17 stamped (security + domain:identity kept).

Why decision box, not queue: the card's own bottom half is right — the fix direction is a semantics ruling. Direction 1 (narrow runtime to declaration) silently reduces access for any deployment that authored business_unit rules relying on today's subtree walk; direction 2 (widen declaration to runtime) makes the two recipient kinds one kind with two names and raises an ADR-0049 enforce-or-remove retirement. Neither is a lane call.
Why target:v17 (binary test, class ② with a class-① edge): the shipped surface grants wider than the public contract declares — a sharing rule reaching a whole subtree its author scoped to one unit is an over-grant users hit today, and the spec + lint red-line table both assert the distinction the runtime does not enforce. Shipping the RC with a declared≠enforced security boundary is the release-note-apology class. The label rides the decision card: whichever direction is ruled, the ruling's execution sub-card inherits it (target follows the work).
Premise verified as filed: measured on origin/main @ 098b629, still the tip this round; both expandRecipient branches call the identical expandUsers, whose first line BFSes descendants.
Pre-decision measurement worth attaching (from the card): whether any shipped example app / seeded rule authors business_unit — decides direction 1's real-world blast radius.

Four-lens block (filing requirement for the decision box):
① Platform long-term coherence — direction 1 keeps two genuinely distinct vocabulary widths (shrinks a lie); direction 2 collapses to one kind and should then retire a name (ADR-0049); status quo keeps a lint red-line that is unenforceable.
② Measured business pull — subtree-vs-exact is a real org-modeling distinction (ADR-0057 D5 added the wide form deliberately); pull for exact-unit semantics should be confirmed via the shipped-rule scan the card names.
③ AI-agent error-resistance — direction 1 is strictly safer for AI-authored metadata: an agent choosing business_unit gets the narrow grant it asked for; today the narrow spelling silently over-grants, the worst failure shape for generated security rules.
④ Startup scope discipline — direction 2 is a capability removal (of the distinction) and cheaper to maintain; but keeping a declared key whose behaviour contradicts its declaration is the one option with permanent cost. Recommendation from triage: direction 1, with a release note; sequence after #7729's timing fix (PR #7806) lands, since that PR deliberately covers both kinds either way.

本评论来自分诊座位 Routine（#5474 试点），不构成认领。

Generated by Claude Code

Maintainer ruling — 2026-08-12

裁定:方向 1 —— 收窄运行时以匹配声明。 business_unit 收件人只展开该单位本身的成员;子树展开是 unit_and_subordinates 独有的语义(ADR-0057 D5)。

要点:

spec、lint 红线表、ADR-0057 三处声明一致,错的是 expandRecipient 的实现;两个 kind 合并成一个才是需要避免的结局(那会再欠一次 ADR-0049 退役)。
实测(2026-08-12):树内已发货规则只授权 position 与 unit_and_subordinates(showcase sharing-rules.ts),没有任何在册规则使用 business_unit —— 树内迁移成本为零。
需要 changeset + release note:树外部署若依赖 business_unit 的子树行为,本次收窄会减少其可见范围,这是有意的 over-grant 修复。
修法:business_unit 分支不再走 descendants BFS,只展开 business_unit_id 等于该单位的成员;两个分支从「只差注释」变成真正两种宽度,并各自加 pin。

裁定人:维护者 huangyiirene(2026-08-12,接受 PM 综合分析后批准);由 PM 会话 session_01GZKbx4xyF7U5WXj6ch49BM 代笔落卡。转 pm:queue。

Generated by Claude Code

Claim: PM loop round 2 (domain:identity seat #6022)
Session: session_01PEVB6w7D7uCszR9Mw1BL73
Branch: claude/issue-7807-business-unit-exact-width
Worktree: objectstack-issue-7807
Domain: domain:identity
File surface: packages/plugins/plugin-sharing/src/sharing-rule-service.ts — the business_unit branch of expandRecipient (:711; the unit_and_subordinates branch at :730 is the control and ⛔ must not change) + its tests + a changeset carrying the release note. ⚠️ packages/spec/** is out of surface: the spec already declares this correctly, and the ruling is that the runtime is wrong.
Container & model: M, mode:subagent, model: opus — a security narrowing with a deliberate blast radius, not a mechanical edit.
Serial constraints cleared: plugin-sharing is now free. PR #7974 (#7861, share-link-service.ts) MERGED — verified by content on origin/main, both refusal codes wired into the service and the spec contract text untouched. ⚠️ #7858 and #7795 also land in this package and are held behind this card.

Prerequisite from the ruling — verified, satisfied

The triage note sequenced this after #7729's timing fix (PR #7806) lands, because that PR deliberately covers both recipient kinds either way. Verified on origin/main: bu-tree-recompute.ts is present with its test file. The prerequisite is met, so this dispatches now rather than waiting.

Premise re-verified on the merged `origin/main` (⛔ not on the ref the card cites)

The card measured at 098b629; main has moved a long way since, and this lane just landed a PR in this very file. Re-measured:

expandRecipient still routes both kinds through the identical call — business_unit at :711, unit_and_subordinates at :730.
The over-grant therefore still stands, and fix(sharing): enforce the declared publicSharing.eligibility predicate at link creation #7974's landing did not incidentally fix it.

The ruling this executes (维护者 2026-08-12) — quoted verbatim, untranslated

裁定:方向 1 —— 收窄运行时以匹配声明。 business_unit 收件人只展开该单位本身的成员;子树展开是 unit_and_subordinates 独有的语义(ADR-0057 D5)。

spec、lint 红线表、ADR-0057 三处声明一致,错的是 expandRecipient 的实现;两个 kind 合并成一个才是需要避免的结局(那会再欠一次 ADR-0049 退役)。

实测(2026-08-12):树内已发货规则只授权 position 与 unit_and_subordinates(showcase sharing-rules.ts),没有任何在册规则使用 business_unit —— 树内迁移成本为零。

需要 changeset + release note:树外部署若依赖 business_unit 的子树行为,本次收窄会减少其可见范围,这是有意的 over-grant 修复。

修法:business_unit 分支不再走 descendants BFS,只展开 business_unit_id 等于该单位的成员;两个分支从「只差注释」变成真正两种宽度,并各自加 pin。

⚠️ The ruling contains one measurement with a shelf life. 「没有任何在册规则使用 business_unit」 was measured 2026-08-12; the dev re-checks it, because it is the entire basis for "in-tree migration cost is zero". If a shipped rule now authors business_unit, that does not reverse the ruling — it changes what the release note has to say, and it gets reported rather than absorbed.

Generated by Claude Code

{
  "issue": 7807,
  "status": "done",
  "branch": "claude/issue-7807-business-unit-exact-width",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/8097",
  "premise_still_valid": true,
  "summary": "Premise re-verified on the merged origin/main before any edit: expandRecipient still routed BOTH recipient kinds through the identical dept.expandUsers call (business_unit at :711, unit_and_subordinates at :730 — the PM's offsets were exact). Implemented the 2026-08-12 ruling, direction 1: business_unit now resolves through a new BusinessUnitGraphService.expandUnitMembers() (members whose business_unit_id equals the named unit, no descendants BFS), preserving the seed-active/tenant-scope gate and the fail-closed-on-read-error posture; unit_and_subordinates keeps expandUsers and the subtree walk unchanged as the control. Neither kind merged or retired. packages/spec untouched. One design point beyond the ruling's letter: the two widths get SEPARATE cache maps — both are keyed by BU id, so a shared map would let whichever width ran first answer for the other, re-opening the over-grant through the cache door; that is pinned by its own test. Changeset included stating the reduction and the migration path. NOTE for the PM (surprise, not a blocker): the ruling's shelf-life measurement HOLDS for shipped rules, but two in-tree TEST fixtures did author business_unit and pinned the subtree behaviour — see out_of_scope_findings and the fixture-disposition note below.",
  "tests": "① packages/plugins/plugin-sharing — `npx vitest run --passWithNoTests`: 'Test Files 19 passed (19) / Tests 490 passed (490)'; `pnpm --filter @objectstack/plugin-sharing typecheck` (tsc --noEmit) clean. ② packages/qa/dogfood — `npx vitest run test/showcase-bu-hierarchy-sharing.dogfood.test.ts`: 'Test Files 1 passed (1) / Tests 7 passed (7)' (real booted stack, real REST), and test/authz-conformance.test.ts: 15 passed; `typecheck` clean after building the closure (`pnpm --filter '@objectstack/dogfood^...' build` — the first run failed with TS2307 'Cannot find module @objectstack/verify' purely from the unbuilt closure, not from my edits). ③ PREDICT-THEN-MUTATE ABLATION, prediction written to disk BEFORE running either. Ablation A (revert only the narrowing — the defect shape): predicted exactly the narrow assertions red, every wide one green. Measured 8 failed / 482 passed, and all 8 are narrow-width — e.g. \"expected [ 'u_dept', 'u_div', 'u_office' ] to deeply equal [ 'u_div' ]\" and \"expected 3 to be greater than 3\" on the strictly-wider pin; the WIDE control ('grants the anchor unit AND every descendant unit, three levels down') and the entire business-unit-graph suite stayed GREEN, as predicted, since expandUnitMembers still existed but was unwired. Ablation B (narrow BOTH branches — the failure mode the PM named): predicted the wide assertions red, narrow green. Measured 8 failed / 482 passed, all 8 wide-width — \"expected [ 'u_div' ] to deeply equal [ 'u_dept', 'u_div', 'u_office' ]\", \"expected 1 to be greater than 1\" — PLUS four in the existing #7729 bu-tree-recompute suite, which turns out to be a second independent control for the wide width. Both ablations reverted via a saved file copy + `git checkout`, never `git stash`. The symmetry is the evidence that the wide pin is a real control and not a tautology. ④ Gates re-derived for the FINAL surface with `node scripts/pm/dispatch-gates.mjs` (the card's predicted `check:cross-package-test-inputs` did NOT match; reporting that rather than the predicted list). Run green: check:nul-bytes (plus a targeted `grep -naP` control-byte self-scan over my diff, clean), check:engine-double-contract (my new fake in recipient-width.test.ts classifies as 'pinned [delete]' AND 'pinned [update]' — not debt; it opens both verbs with assertEngineDeleteDispatch/assertEngineUpdateDispatch), check:test-source-alias, check:docs-audit-scope, check:changeset-gate-self-tests, check:objectui-changeset, check-changeset-no-major. CI gate status at report time: in_progress (reporting at draft-PR time per the 2026-08-10 ruling; not idle-polling). Rebased onto origin/main (7 commits ahead at push time) with no overlap on my files; full suite re-run green post-rebase.",
  "open_questions": [],
  "out_of_scope_findings": [
    "filed as #8098: two contract docstrings (packages/spec/src/contracts/sharing-service.ts:565 and packages/platform-objects/src/identity/sys-business-unit.object.ts:17) still attribute the BU SUBTREE expansion to the `business_unit` recipient. Both sit outside the surface the ruling allowed (spec explicitly ⛔), so recorded rather than fixed. Observation class: `finding` label, no `pm:queue`, unassigned.",
    "NOT filed — handled in-PR as fixture triage, reported because the ruling's zero-cost measurement is what it qualifies: the ruling's 「没有任何在册规则使用 business_unit」 re-measured TRUE for shipped/seeded rules (showcase authors `position` + `unit_and_subordinates`; CRM authors `position`), but THREE in-tree test fixtures authored `business_unit`, two of them pinning the subtree as if it were the contract. Dispositioned individually, not batch re-spelled: (a) sharing-rule.test.ts's test literally named 'expands via the BU graph (BFS)' asserted the over-grant — REPLACED WHOLESALE by the pair (narrow + a new unit_and_subordinates sibling); (b) showcase-bu-hierarchy-sharing.dogfood.test.ts authored `business_unit` to demonstrate hierarchy widening, which is unit_and_subordinates' semantics — RE-SPELLED (both its authz-conformance rows preserved) and the narrow half added on the same booted stack; (c) bu-tree-recompute.test.ts's `business_unit` case was already anchored at the unit its member belongs to DIRECTLY, so it survives the narrowing unchanged — only its now-false title ('today they walk the same subtree resolver') was corrected. Consequence for the release note: none — the migration cost stays zero for deployments, since no shipped rule authored the kind. The in-tree cost was three test fixtures, not zero, and that is worth the PM knowing."
  ]
}

Generated by Claude Code

PM review: ACCEPT — PR #8097. Seat #6022, session session_01PEVB6w7D7uCszR9Mw1BL73. CI convergence is mine; the verdict does not wait on it.

Verified against GitHub, not against the report

check	result
Path face — `docs/adr/` / `.claude/skills/` / `skills/**`	✅ zero hits
The ruling's fenced surface — `packages/spec/**`	✅ untouched, confirmed by diff
`unit_and_subordinates` still walks the subtree	✅ its branch still calls `expandUsers`; only the `business_unit` branch moved to `expandUnitMembers`
Changed files vs declared surface	✅ 10 files, all inside `plugin-sharing` + `qa/dogfood` + the changeset
Changeset with the release note	✅ present
First line `Fixes #7807`	✅

The double ablation is the best verification this lane has seen

Most ablations prove the fix does something. Yours proves the pair, which is what the ruling actually protects:

A (revert only the narrowing) → 8 failures, all narrow-width, wide control green.
B (narrow both branches — the failure mode I named in the dispatch) → 8 failures, all wide-width, narrow green.

⚠️ B is the one that matters and it is the one nobody asks for. A fix that narrowed both branches would have satisfied this card's headline, passed a single-direction ablation, and silently destroyed the very distinction the ruling exists to preserve — collapsing the two kinds into one, which is the outcome the maintainer explicitly named as the thing to avoid. The symmetry is what proves the wide pin is a control rather than a tautology. Also noted: B surfaced that the existing #7729 recompute suite is a second independent control for the wide width, which nobody knew.

The cache-key finding is beyond the ruling's letter and belongs in the fix

both are keyed by BU id, so a shared map would let whichever width ran first answer for the other

That is the over-grant returning through the cache door — same defect, different mechanism, and invisible to every test that exercises one width per process. Separate maps plus a pin for it is exactly right, and it is the kind of thing a dev implementing the ruling's literal text would not have looked for.

The docs question — I checked, and the answer is "no change needed"

The drift check flagged sharing-service.mdx / permissions-matrix.mdx, and a narrowing that leaves user-facing prose promising the old width just relocates the lie. So I read it rather than assuming. content/docs/permissions/sharing-rules.mdx:145 already says:

| business_unit | Everyone in exactly that business unit (no subtree) |

The prose was never wrong — spec, lint red-line table, ADR-0057 and the docs all described the narrow width, and only the runtime disagreed. Nothing to update, and that is now a verified reading rather than an open question.

Your qualification of the ruling's measurement is the right kind of correction

The ruling's 「没有任何在册规则使用 business_unit」 → zero migration cost. You re-measured it and it holds for shipped rules (showcase authors position + unit_and_subordinates, CRM authors position) — but three in-tree test fixtures authored it, two pinning the subtree as if it were the contract.

⚠️ That distinction is worth stating precisely, because it would be easy to report it as "the ruling was wrong": it was not. Deployment migration cost is still zero, so the release note is unchanged. What was non-zero was the in-tree fixture cost, which is a different quantity the ruling never claimed. And you dispositioned all three individually rather than batch re-spelling — including recognising that bu-tree-recompute.test.ts's case was already anchored directly at the member's own unit and survives untouched, needing only its now-false title corrected. A batch re-spell would have quietly rewritten a passing test's meaning.

A test named "expands via the BU graph (BFS)" asserting the over-grant as the contract is the clearest possible illustration of why this card existed at all.

#8098 — filed correctly, and routing is not mine

Two contract docstrings (packages/spec/src/contracts/sharing-service.ts and platform-objects/.../sys-business-unit.object.ts) still attribute the subtree expansion to business_unit. Both sit outside the surface the ruling allowed — packages/spec explicitly so — and recording rather than fixing them was the correct call. ⚠️ Note for whoever picks it up: after this PR those docstrings are stale in the dangerous direction (they describe a width the runtime no longer grants). packages/spec has one owner regardless of who needs it, so that card belongs to the spec seat; grading and routing are the triage seat's single channel and I am not pre-empting either.

Next steps (mine)

Gates were still in_progress at review time (Build Core ✅, Dogfood Verify CLI ✅, Check Changeset ✅, with Test Core ×3, Dogfood Regression ×3, ESLint and TypeScript Type Check running). I will read the per-job conclusions, then flip ready and arm — in that order, since auto-merge does not survive a draft conversion.

Generated by Claude Code

Armed — PR #8097, 15:39Z. Seat #6022, closing the loop the ACCEPT left open.

All gates read per job, ⛔ never the aggregate: Test Core (1/3) ✅ 15:22Z · (2/3) ✅ 15:27Z · (3/3) ✅ 15:22Z · TypeScript Type Check ✅ · ESLint ✅ 15:22Z · Dogfood Regression Gate (1–3/3) + rollup ✅ · Temporal Conformance (live PG + MySQL) ✅ · Build Core · Check Changeset · Spec property liveness ✅. Zero failures.

TypeScript Type Check was the last one running and was fetched by check-run id rather than by re-reading all 26 — same authoritative per-job conclusion, a fraction of the cost.

Before arming: head sha re-confirmed as 51b3d205, the exact commit CI ran on (no post-CI push slipped in), and the path face re-verified at that head — zero hits on docs/adr/** / .claude/skills/** / skills/**, so this takes the normal terminal path.

Then draft: false → enable_pr_auto_merge, in that order, since auto-merge does not survive a draft conversion.

Held until MERGED. The landing check will verify content on origin/main, and specifically the negative space this card turns on: that business_unit resolves through expandUnitMembers (narrow) and unit_and_subordinates still resolves through expandUsers (wide), with the two widths still on separate cache maps. A landing check that only confirmed the narrowing would miss precisely the regression your ablation B was designed to catch.

Generated by Claude Code

MERGED and landing-verified — PR #8097 landed as 9b519815c. Seat #6022. Card closed out.

Verified by content on origin/main — the queue squashes, so ancestry proves nothing.

Positive: the business_unit branch of expandRecipient now returns dept.expandUnitMembers(rule.recipient_id), and expandUnitMembers exists at business-unit-graph.ts:160.

Negative space — the half that carries the security claim:

must still be true	reading on merged main
`unit_and_subordinates` still walks the subtree	✅ its branch is present and still calls `expandUsers`, with the comment "unchanged — `expandUsers` is the contract's descendant expansion"
the two widths cache separately	✅ `DeptCache` carries distinct `descendants` / `expandUsers` / `unitMembers` maps, with the comment naming the hazard: a shared map would let a `business_unit` expansion "be served a cached subtree answer (re-opening…)"

⚠️ A process note on how this check nearly went wrong, because it is the more useful half of this comment. My first pass ran the negative-space greps through an -A3 context pipeline, and both came back with zero hits — which, read literally, would have said the unit_and_subordinates branch had been deleted, i.e. that the two kinds had been collapsed into one, the exact outcome the ruling forbids.

That reading would have been wrong. The zero was my pipeline dropping lines, not the branch disappearing. The rule that caught it: a zero hit is not a reading until a known-present neighbouring term proves the search works. The control — unit_and_subordinates appears 3× in that file — is what made the difference between "verified" and "raised a false alarm on a correct merge".

Had I trusted the first zero, I would have opened an incident against a PR that is in fact exactly right.

plugin-sharing is now free. #7858 is being claimed next, with its file surface re-verified on this merged ref first.

Generated by Claude Code

fix(plugin-sharing): recompute unit_and_subordinates grants on busine…

fix(plugin-sharing): business_unit recipient expands exactly one unit…

docs: fix BU subtree attribution in two docstrings (objectstack-ai#8098…

Navigation Menu

Repository navigation

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

Symptom

Declared

Enforced

Which way to fix is a decision, not a detail

Source

Maintainer ruling — 2026-08-12

Prerequisite from the ruling — verified, satisfied

Premise re-verified on the merged `origin/main` (⛔ not on the ref the card cites)

The ruling this executes (维护者 2026-08-12) — quoted verbatim, untranslated

Verified against GitHub, not against the report

The double ablation is the best verification this lane has seen

The cache-key finding is beyond the ruling's letter and belongs in the fix

The docs question — I checked, and the answer is "no change needed"

Your qualification of the ruling's measurement is the right kind of correction

#8098 — filed correctly, and routing is not mine

Next steps (mine)

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Symptom

Declared

Enforced

Which way to fix is a decision, not a detail

Source

Activity

os-zhuang commented on Aug 11, 2026

huangyiirene commented on Aug 12, 2026

Maintainer ruling — 2026-08-12

os-zhuang commented on Aug 12, 2026

Prerequisite from the ruling — verified, satisfied

Premise re-verified on the merged origin/main (⛔ not on the ref the card cites)

The ruling this executes (维护者 2026-08-12) — quoted verbatim, untranslated

os-zhuang commented on Aug 12, 2026

os-zhuang commented on Aug 12, 2026

Verified against GitHub, not against the report

The double ablation is the best verification this lane has seen

The cache-key finding is beyond the ruling's letter and belongs in the fix

The docs question — I checked, and the answer is "no change needed"

Your qualification of the ruling's measurement is the right kind of correction

#8098 — filed correctly, and routing is not mine

Next steps (mine)

os-zhuang commented on Aug 12, 2026

os-zhuang commented on Aug 12, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

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

Premise re-verified on the merged `origin/main` (⛔ not on the ref the card cites)