fix(plugin-security): the delegated-admin gate resolves a scope's business-unit anchor inside the caller's own organization

[huangyiirene]

Fixes #19775

Clause-②: no

The delegated-administration gate resolved a scope's business-unit anchor by NAME across organizations. In a single-database multi-org posture (ADR-0105 D1 group / isolated; ADR-0132 「single-database organization isolation ships open」) a name collision therefore crossed an organization boundary, in both directions at once.

The defect, verified at source before the repair

Verified on origin/main at the branch point 2cf9db7c43, not taken from the card:

• delegated-admin-gate.ts:44 — const SYSTEM_CTX = { isSystem: true } as const; carries no tenant.
• delegated-admin-gate.ts:912 — the anchor lookup: ql.find('sys_business_unit', { where: { name: businessUnitName }, limit: 1, context: SYSTEM_CTX }). That is the by-name call. The descendant walk at :933-937 runs under the same context with no organization predicate.
• packages/objectql/src/engine.ts:4295 — hasTenant is execCtx?.tenantId !== undefined && …, and :4316 sets opts.tenantId only under it. It is independent of isSystem, which is what makes the repair possible at all.
• packages/drivers/driver-sql/src/sql-driver.ts:13771 — applyTenantScope returns the builder untouched when tenantId is undefined, null or empty.
• packages/platform-objects/src/identity/sys-business-unit.object.ts:260-263 — name carries no uniqueness; the only unique index is ['code', 'organization_id'].

So nothing scoped that read: limit: 1 plus the SQL driver's ORDER BY id ASC answered "whichever id sorts first", across every organization.

The repair — fail closed, and narrower at every choice

The anchor read, the descendant walk, and the two catalog reads behind describeDelegableScope now carry the caller's own organization, and the candidates that come back are reduced to the caller's own rows.

• The organization is organizationId ?? tenantId — the same spelling SecurityPlugin.callerOrganizationId (security-plugin.ts:989) already resolves a caller's permission sets with. The adminScope this gate reads arrives on a set loaded out of THAT organization's catalog, so anchoring its business unit anywhere else pairs an authority minted in one tenant with a tree owned by another.
• Two arms, both load-bearing, each measured (ablation below): the context carries the organization so applyTenantScope composes a predicate, AND resolveOwnOrganizationRow (reused from per-organization-catalog.ts, no new symbol) picks the caller's own row out of what came back. The driver's compatibility arm deliberately also returns organization-less rows, and a driver with no tenant scoping at all returns every organization's.
• Narrower where there was a choice. Under the group posture the anchor resolves in the caller's ACTIVE organization, not their whole membership set. Under a walled posture an organization-less business unit no longer answers a delegation boundary — that posture already declares such a row invalid state (per-organization-catalog.ts). A tenant admin's picker is unconstrained inside its organization and never across organizations.
• Unchanged where there is no boundary to cross: a caller carrying no organization (the single posture) keeps the by-name answer, pinned by its own test.
• No new exported symbol, no new key on any published payload. describeDelegableScope and scopesCoverUser take the caller's context as a new OPTIONAL argument; DelegableScopeReport is byte-identical. ISecurityService.describeDelegableScope(callerContext) in packages/spec already declared that parameter, so this lane touches packages/spec in zero lines.
• No new error code, no new refusal message. The crossing lands on the refusals that already existed: "outside the delegated subtree" for a write, and the empty fail-closed subtree a misconfigured scope already produced.

Evidence — the crossing itself, failing before and passing after

Two suites, deliberately complementary. delegated-admin-gate-cross-organization.test.ts is a REAL ObjectQL over a REAL SqlDriver on in-memory SQLite: two organizations each holding a unit named sales with one child, ids ordered so the OTHER organization's sort first (bu_0_* before bu_a_*). delegated-admin-gate.test.ts gains the companion block on a fake ql that ignores context entirely — a driver with no tenant scoping at all.

BEFORE — the two source files reverted to the branch point 2cf9db7c43 (proved on disk by blob hash, 07ee0a9ff2… for the gate), the new test kept:

$ pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2 \
    src/delegated-admin-gate-cross-organization.test.ts

 × the org A delegate KEEPS its own subtree — a write inside org A `sales` is approved
     AssertionError: promise rejected "PermissionDeniedError: [Security] Access …" instead of resolving
 × the org A delegate cannot reach org B — a write anchored in the other organization is REFUSED
     AssertionError: promise resolved "undefined" instead of rejecting
 × describeDelegableScope names the caller's own units and NOTHING of the other organization
     AssertionError: expected [ 'bu_0_sales', 'bu_0_sales_east' ] to deeply equal [ 'bu_a_sales', 'bu_a_sales_east' ]
 × a tenant admin is unconstrained INSIDE its organization, never across organizations
     AssertionError: expected [ 'bu_0_sales', …(3) ] to deeply equal [ 'bu_a_sales', 'bu_a_sales_east' ]

 Test Files  1 failed (1)
      Tests  4 failed | 4 passed (8)

Both measured directions of the defect are in those four lines: the org A delegate lost its own subtree (rejected where it should resolve), and the gate approved a delegated write anchored in org B (resolved where it should reject). The four that passed are the controls — ground truth, the single-organization dark control, the firing control, and the organization-less caller.

AFTER — same command, same tree, the repair restored (git checkout HEAD -- …, restore proved by git diff HEAD empty and a matching git hash-object):

 Test Files  1 passed (1)
      Tests  8 passed (8)

ABLATION — each arm proved independently load-bearing. Removing only the row-level arm (resolveOwnOrganizationRow(...).own replaced by rows[0] ?? null; the injected text confirmed present on disk and the deleted text confirmed absent, under an EXIT INT TERM trap that restores from HEAD):

$ pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2 \
    src/delegated-admin-gate.test.ts src/delegated-admin-gate-cross-organization.test.ts

 × resolves the caller's own `sales`, not the first row the driver handed over
 × approves the same write inside the caller's own organization
 × an anchor that exists ONLY in another organization approves nothing (fail closed)

 Test Files  1 failed | 1 passed (2)
      Tests  3 failed | 69 passed (72)

The unscoping-driver half goes red while the real-engine half stays green — exactly the split the code comment claims, so neither arm is decoration. The tree was restored byte-exact afterwards (git hash-object = 732cc64311b9a36af7a2e338e6fc86c8ff80f698, the HEAD blob).

No existing test asserted the defect. The package's 118 files / 2261 tests pass unchanged; nothing had to be corrected, and no admission decision was widened to make anything green.

Verification

what	command	result
package tests	pnpm --filter @objectstack/plugin-security test	118 files / 2261 passed
package typecheck	pnpm --filter @objectstack/plugin-security typecheck	exit 0, test layer 0 errors
dependency closure	pnpm --filter '@objectstack/plugin-security^...' build	exit 0
derived gate families	node scripts/pm/dispatch-gates.mjs --commands then --ran	62 derived, 62 run, 0 NOT-MEASURED, 0 UNRUN
repo lint (whole tree, not narrowed)	pnpm lint (eslint . --no-inline-config)	exit 0

Three of the 62 first answered PREREQUISITE NOT MET (exit 3, not a finding) because they read built output; all three are green after pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*' (72/72 successful): check:dual-build-cjs-loads, check:i18n, check:type-check-debt. check:where-matcher found one real defect in my own new fixture — a $-prefixed key read as a field name — fixed in da621549c0 and green. All readings are taken at da621549c0, the final commit.

Not run here and left to CI: the five path-scheduled CI jobs, the type-check lanes over the whole workspace, and the artifact-roster families the derivation scores silent for every card.

Acceptance notes

Observed while reading the fix face, deliberately NOT changed here — handed back to the PM rather than filed, and rather than widened into this PR:

• The sibling by-name reads of sys_position in the same file are the same defect class, unrepaired. positionIsDelegatable (:586 pre-fix) and setsBoundToPosition (:1005 pre-fix) both do ql.find('sys_position', { where: { name: positionName }, limit: 1, context: SYSTEM_CTX }), and sys_position is a per-organization catalog row upserted by (name, organization_id). They decide whether a position may be self-delegated and which permission sets it distributes, so a cross-organization row answering either one is an authority decision. The card listed them under "Not measured"; they need their own fixture and their own witness, which is a new verification surface this card's scope does not carry.
• businessUnitsOfUser and assignmentAnchorsOfPosition read sys_business_unit_member / sys_user_position under the same bare system context. Both are keyed on a globally unique id rather than a name, so the collision this card is about does not reach them — noted as boundary, not claimed as a defect.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security, touching 16 documentable anchor(s).

10 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/deployment/environment-variables.mdx (via sys_position (literal, a string literal in describeDelegableScope))
• content/docs/permissions/administrator-guide.mdx (via sys_business_unit (literal, a string literal in describeDelegableScope; a string literal in resolveSubtree))
• content/docs/permissions/authorization.mdx (via sys_business_unit (literal, a string literal in describeDelegableScope; a string literal in resolveSubtree), sys_position (literal, a string literal in describeDelegableScope))
• content/docs/permissions/delegated-administration.mdx (via DelegatedAdminGate (symbol, a top-level class), sys_business_unit (literal, a string literal in describeDelegableScope; a string literal in resolveSubtree), sys_position (literal, a string literal in describeDelegableScope), /api/v1/security/my-delegable-scope (route, the route ledger binds it to client method security.describeDelegableScope))
• content/docs/permissions/permissions-matrix.mdx (via sys_business_unit (literal, a string literal in describeDelegableScope; a string literal in resolveSubtree))
• content/docs/permissions/positions.mdx (via sys_business_unit (literal, a string literal in describeDelegableScope; a string literal in resolveSubtree), sys_position (literal, a string literal in describeDelegableScope))
• content/docs/permissions/sharing-rules.mdx (via sys_business_unit (literal, a string literal in describeDelegableScope; a string literal in resolveSubtree))
• content/docs/permissions/system-context.mdx (via explainAccessForCaller (symbol, a method of class SecurityPlugin))
• content/docs/protocol/backward-compatibility.mdx (via sys_position (literal, a string literal in describeDelegableScope))
• content/docs/protocol/objectql/security.mdx (via sys_business_unit (literal, a string literal in describeDelegableScope; a string literal in resolveSubtree))

⛔ 8 release-owned page(s) also name something this change touched. These are read-only:

• content/docs/releases/index.mdx (via sys_position (literal, a string literal in describeDelegableScope))
• content/docs/releases/v13.mdx (via DelegatedAdminGate (symbol, a top-level class), sys_business_unit (literal, a string literal in describeDelegableScope; a string literal in resolveSubtree), sys_position (literal, a string literal in describeDelegableScope))
• content/docs/releases/v14.mdx (via sys_position (literal, a string literal in describeDelegableScope))
• content/docs/releases/v15.mdx (via sys_position (literal, a string literal in describeDelegableScope))
• content/docs/releases/v16.mdx (via sys_business_unit (literal, a string literal in describeDelegableScope; a string literal in resolveSubtree))
• content/docs/releases/v17/17-1.mdx (via sys_position (literal, a string literal in describeDelegableScope))
• content/docs/releases/v17/17-2.mdx (via sys_position (literal, a string literal in describeDelegableScope))
• content/docs/releases/v17/17-4.mdx (via sys_business_unit (literal, a string literal in describeDelegableScope; a string literal in resolveSubtree))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

• 2 name(s) were too generic to anchor anything (single lowercase words)
• the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
• a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json fae870352ea59ddfb1c6dfd784bc6552cf158211 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 6efee75067c9d6dbeed8c0f9dedc18c836373cee — the merge of head da621549c08a44602c01b003a6b1e8d7b46a1eee into base fae870352ea59ddfb1c6dfd784bc6552cf158211, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 6efee75067c9d6dbeed8c0f9dedc18c836373cee && git checkout 6efee75067c9d6dbeed8c0f9dedc18c836373cee
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fae870352ea59ddfb1c6dfd784bc6552cf158211 da621549c08a44602c01b003a6b1e8d7b46a1eee && git checkout -B drift-repro fae870352ea59ddfb1c6dfd784bc6552cf158211 && git merge --no-ff da621549c08a44602c01b003a6b1e8d7b46a1eee

node scripts/docs-audit/affected-docs.mjs --json fae870352ea59ddfb1c6dfd784bc6552cf158211

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs fae870352ea59ddfb1c6dfd784bc6552cf158211 → pass the list as
args.docs, on the commit named under Which tree this was computed on.