Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-sharing): a seeded business unit is a usable rule recipient, and its members are tenant-screened (#14547) - #14572
Conversation
…recipient, and its members are tenant-screened (#14547) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VR2khJ3Me96btawVsfG6jb
…er screen; ledger + census upkeep (#14547) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VR2khJ3Me96btawVsfG6jb
📓 Docs Drift CheckThis PR changes 1 package(s): ⛔ 1 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 8 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 86129a2ff2cec18eed68a7cab6b928b2e5238e7a && git checkout 86129a2ff2cec18eed68a7cab6b928b2e5238e7a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7286dd58e806ed321cbdfc23a1c455db8f80b1ad c079c35cea83cb342bccfeb96e8a567cebaf55f5 && git checkout -B drift-repro 7286dd58e806ed321cbdfc23a1c455db8f80b1ad && git merge --no-ff c079c35cea83cb342bccfeb96e8a567cebaf55f5
node scripts/docs-audit/affected-docs.mjs --json 7286dd58e806ed321cbdfc23a1c455db8f80b1ad
|
baozhoutao
commented
Sep 3, 2026
Closing this draft without merging, on the maintainer's instruction (2026-09-03): the kpi project only reports platform issues and does not fix them in the platform repository. The defect report stays open as #14547 (with #14570 for the residual gap); the branch is left as a reference for whoever picks the issue up. Nothing on Generated by Claude Code |
Part of #14547
A sharing rule with a business-unit recipient granted nobody — silently — whenever the unit came from seed data.
BusinessUnitGraphService.orgScopescreenedsys_business_unitwith a strictorganization_idequality, while the platform's own read-side tenant screen (SqlDriver.applyTenantScope) is null-inclusive:(organization_id = ? OR organization_id IS NULL). A rule always carries the caller's organization; a seeded unit carries none, because a seed cannot know the id the runtime mints at boot. The two never matched,seedIsUsableread the unit as "does not exist", both recipient widths returned zero users, and the rule stayedactive: truehaving materialised nosys_record_sharerow and logged nothing.This lands recommendation A from the triage — both halves in one PR, because the first alone is a leak.
1. The unit screen is now the platform's own null-inclusive one
orgScopecomposes$or: [{ organization_id }, { organization_id: null }], matchingapplyTenantScopeand the #2734 rationale, and matching the readingplugin-approvalsalready had for the same rows (#3807). The[divergence]pin that recorded the old posture as deliberate is flipped in place rather than deleted.Spelled as a predicate rather than as
context.tenantId, for two reasons: the graph's reads are elevated precisely so they can see rows no recipient could (elevation and tenant are separate axes inbuildDriverOptions), anddriver-memory/driver-mongodbimplement no tenant scoping at all, so a screen living only inside the SQL family would be no screen.2. The member reads are screened at all — they were not before
Both
expandUnitMembersandexpandUsersqueriedsys_business_unit_memberwith no organization predicate, under a system context that carries no tenant. That was invisible only because the strict unit screen kept an org-stamped rule from ever reaching the unscoped query. Widening the unit screen alone turns a silent under-grant into a silent cross-tenant over-grant: a seeded unit id exists identically in every tenant, so tenant A's rule would have expanded to tenant B's members.Both widths are screened, not only the narrow one the triage named:
unit_and_subordinatesis the recipient kind the reported reproduction used, and its member read carried the identical hole. The PR names it here rather than landing it quietly.3. An active business-unit rule that expands to nobody is loud
SharingRuleService.expandRecipientwarns once per rule per process — the same dedup the inert-criteria warn carries, for the same reason — naming the rule, the object, the recipient kind, the unit and the organization, and pointing at the two causes worth checking first. Scoped to the two business-unit recipient kinds deliberately:queueexpands to[]by construction today, so a blanket "any empty expansion" warn would fire on every pass of every queue rule.Prerequisite measurement: are membership rows organization-stamped on every write path?
No. This is what decides the shape of change 2, so it is stated in full.
organization_id?execCtx.tenantId, and the SQL driver'sinjectTenantOnInsertfills the injected columnseed-loader.tswithholds its single-orgfallbackOrgIdfrom everysys_/cloud_/ai_objectsys_business_unit_memberisunclassifiedinPLATFORM_OBJECT_TENANCY, soresolveSystemInsertOrganizationreturns earlydriver-memory/driver-mongodbSo the member screen is strict, not null-inclusive, and the asymmetry with the unit screen is the point. On a unit row a NULL organization is the documented platform/seeded class. On a membership row it is unknown tenancy — and admitting an identity of unknown tenancy into an org-stamped grant is the same cross-tenant over-grant arriving by the other door. A grant fails closed.
The declared cost: a rule carrying an organization whose unit AND memberships were both seeded still expands to nobody. That combination is exactly what change 3 makes loud, and the repair is to stamp the membership rows. The underlying gap —
sys_business_unit_memberbeing unadjudicated in the tenancy ledger — is filed separately as #14570 (unassigned, for the adjudication batch the ledger's header describes); it is not addressed here.The dominant path today is unmoved: with no organization on the rule there is nothing to screen against and both reads stay exactly as they were. That is what every materialised rule on a showcase stack looks like —
defineRulereads the caller's organization, and a bareisSystemcontext has none — which is why the dogfood BU-hierarchy fixture is byte-identically unaffected.Fixture triage
Three existing fixtures paired org-scoped units with org-less membership rows and passed anyway — because the member read had no predicate. Each was re-stamped rather than worked around, and each carries a comment saying which of the two facts it was always describing. One pin (
[divergence]) records a retired posture and is flipped; the head docblock that nominated this exact predicate as the future fix is rewritten to describe the pair of screens that landed.Verification
All commands run at
c079c35.pnpm --filter @objectstack/plugin-sharing test— 31 files / 731 tests passed.pnpm --filter @objectstack/plugin-sharing typecheck— green, includingcheck:test-typecheck, which does cover the new test file (the package'stsconfig.jsonexcludes*.test.ts, itstsconfig.test.jsondoes not; the gate named the new file's one type error before it was fixed, which is the proof it reads it).node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackfrom the real change set, re-run atc079c35after the last commit. All green exceptcheck-test-completeness, which exits 3 = NOT MEASURED by its own design when handed no savedturbo run testlog — not a red.check:system-context-censusneeded a--fix: inserting 7 lines intosharing-rule-service.tsrotted two anchors (:157to:164,:382to:389) incontent/docs/permissions/system-context.mdx. Pure line rot, arithmetic confirms it.check:engine-double-contractneeded a--write: the new test file pins engine doubles the ledger did not record.pnpm lint— the whole repo,eslint . --no-inline-config, green in 86s. No narrowing claimed.pnpm check:nul-bytesgreen, plus a direct control-character scan over every file in the diff: no hits.Reverse verification (two ablations)
No rebuild is involved: the tests import
./business-unit-graph.jsrelative, inside the package, so vitest resolves source and adist/cannot mask the mutation. Each leg proved the mutation on disk by grep counts of both the injected marker and the removed text (not by the editor's exit code), and by agit hash-objectdiffering from the HEAD blob; each restored withgit checkout HEAD -- ABSOLUTE_PATHunder anEXIT INT TERMtrap and was proved restored by an emptygit diff HEADplus a blob hash equal to HEAD's.orgScopeback to strict equality(a) an org-NULL unit is USABLE, the flipped divergence pin, and both end-to-end materialisation casesmemberScopecalls removed(b) members of ANOTHER organization are never expanded,(c) a NULL-org member row is NOT a member, and the zero-recipient warnNeither ablation is a subset of the other, which is what shows the two screens are two facts rather than one.
Not run locally
The dogfood suite (a booted stack) and the full cross-package test farm are CI's. The argument above — every materialised rule on that stack carries a null organization, so both screens are no-ops there — is reasoning, not a measurement, and the Dogfood Regression Gate is where it gets checked.
🤖 Generated with Claude Code
https://claude.ai/code/session_01VR2khJ3Me96btawVsfG6jb
Generated by Claude Code
Generated by Claude Code