Skip to content

Commit 669a8aa

Browse files
fix(plugin-security)!: a delegated business-unit walk never starts at a unit outside the caller's organization (#31442)
Clause-②: no (narrowing) — the delegated-administration gate holds the root of every delegated business-unit walk to the same organization as its descendants, so the range a self-delegation's anchor is judged against holds units of the caller's organization only. Nothing is widened: no export, error code or status is added. Metadata-protocol: none Skills-docs: none — a runtime authorization bound inside `@objectstack/plugin-security`; no authorable key, value or metadata type changes, and no `skills/**` or docs page teaches the self-delegation anchor range's organization bound. ## 维护者速读 - 改了什么:委派管理按业务单元圈定的范围,其起点(根单元)与下级单元同一条规则:只能是调用者所在组织的单元。 - 为什么改:ADR-0105 D6 红线 2 —— 一切业务单元机制只在一个组织内运作,没有跨组织的树;委派范围里的每个单元(含根)都属于调用者的组织。 - 风险与回滚:只动 plugin-security 的委派管理闸门;组织内范围里的锚点、不带锚点的职务代理照旧通过,拒绝的答复与范围外锚点一致。按先例 c1e08fe 声明为收窄(BREAKING,patch 级):锚在本组织之外单元的自我委派从此被拒。回滚 = revert 本 PR。 - 先看:`packages/plugins/plugin-security/src/delegated-admin-gate.ts` 的 `resolveSubtreeById`(根判定那一段);再看两个测试文件末尾新增的 describe。 - 两分钟验证:`pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2 src/delegated-admin-own-name-set.posture.test.ts -t "never starts at a unit outside"`,预期 `Tests 15 passed`。 - 需要你做的:无,席位落地。 ## What changes **Rule** (ADR-0105 D6, red line 2: *"Every BU mechanism — `unit_and_subordinates` sharing, `adminScope` delegation, depth scopes — operates within one organization. There is no cross-org tree."*): a delegated business-unit walk never starts at a unit outside the caller's organization. - `DelegatedAdminGate.resolveSubtreeById` reads the walk's root under the bound organization's own scoped context and applies to it the same row predicate the walk already applies to every descendant. A root that is not a unit of that organization covers nothing. - Every subtree a self-delegation's anchor range is built from (ADR-0091 D3, rule 4b) therefore holds units of the caller's organization only. An anchor outside the range is refused with `403 PERMISSION_DENIED`, the same answer whichever organization the unit belongs to. - `resolveSubtree` (a scope's by-name anchor) keeps its answers: its root is already resolved inside the organization, so the new predicate holds for it by construction. It gains one by-id read per resolved scope. - No refusal text, error code, schema or export changes. ## Tests - `delegated-admin-own-name-set.posture.test.ts`, new describe *a delegated business-unit walk never starts at a unit outside the caller's organization*, on a real `ObjectQL` engine and `SqlDriver` with the real `SecurityPlugin`, in `single`, `group` and `isolated` (5 tests per posture, 15 in all): - an unanchored holding's range is the delegator's member units of its own organization; an anchor at a unit of another organization gets the same answer as an anchor outside the range; - a holding anchored at a unit of another organization makes no unit an admitted anchor; - CONTROL: a holding anchored at a unit of the caller's organization keeps its subtree as the range; - the data door stores no delegation anchored at a unit of another organization, and stores one inside the range; - CONTROLS throughout: an in-organization anchor inside the range is admitted, and so is an unanchored delegation. - `delegated-admin-gate.test.ts`, new describe on a double that scopes nothing, so the row predicate is the only thing standing: 3 tests, one of them a CONTROL, including an organization-less unit. - The posture rig's `boot` takes optional extra catalog entries, so the new describe's delegatable position does not change what the other describes enumerate. ## Acceptance notes - **Not added:** a second anchor check in the self-delegation rule. Once every root is held to the organization predicate it is redundant, and a refusal reason of its own would answer ids that are units of the caller's organization differently from other ids. - **Untouched, as ordered:** the by-name root for a context that names no organization, with its docblocks and pins. The existing pins on the subtree staying inside one organization, with or without an organization on the context, hold unchanged. ## Verification Readings at `3de01b6fb` unless stated. The final head `b731e114b` changes only the changeset (the narrowing declaration, its BREAKING paragraph and the ADR-0087 marker). - Dogfood battery at `b731e114b`: lock `VERDICT command-exit 0`, `Test Files 438 passed | 3 skipped (441)`, `Tests 3622 passed | 41 skipped (3663)`, no failures. <details><summary>Package suite and typecheck</summary> - `pnpm --filter @objectstack/plugin-security test`: `Test Files 249 passed (249)`, `Tests 5886 passed | 45 skipped (5931)`, lock `VERDICT command-exit 0`. - `pnpm --filter @objectstack/plugin-security typecheck`: exit 0 (`tsc` src and scripts, plus `check:test-typecheck` OK). - These two ran at `cf5c2456c`. The commit after it changes only the changeset's declaration line. </details> <details><summary>Ablation (one-time, nothing left in the tree)</summary> - Mutation: the root predicate is removed on disk through `scripts/ablation-replace.mjs` (anchor hits 1 → 0, blob `4cd4659e28e4` → `e0c1deb04f78`). The subject resolves through relative `src` imports, so no `dist` is in the path. - With it removed, the two files report `Tests 11 failed | 242 passed (253)`: the 9 posture legs (3 per posture) and the 2 fake-engine pins that are not controls. Every CONTROL stayed green. - Restored: the blob equals the one at `HEAD` (`4cd4659e28e4`) and `git diff HEAD` is empty. The same files then report `Tests 253 passed (253)`. </details> <details><summary>Derived gates</summary> - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` at `3de01b6fb`: 66 commands. Each ran from the worktree, with its exit code captured before any pipe: 66 of 66 exit 0. - `--ran` reconciliation: `66 derived, 66 run, 0 NOT-MEASURED, 0 UNRUN` (a derived zero: every line records its exit code). - At `cf5c2456c`, the first sweep had `check:dual-build-cjs-loads` and `check:i18n` refuse their prerequisite (exit 3, missing `dist`). After building the closure that `check:i18n` names, both measure green on the head re-run: 122 require entry points across 72 packages load, and 10 packages' bundles are in sync. - Changeset gates on the final changeset: `check-adr-0087-registration`, `check-changeset-no-major` and `check-empty-changeset` (all `--base origin/main`) exit 0, and so does `check:nul-bytes`. - Every family CI runs is read from this PR's check-runs. None of them is re-run locally. </details> <!-- signed: security#1 · sunhaolin · session_017xRBcT1w3zpekijraHJE4f --> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a40eb02 commit 669a8aa

4 files changed

Lines changed: 261 additions & 6 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
fix(plugin-security)!: a delegated business-unit walk never starts at a unit outside the caller's organization
6+
7+
Clause-②: no (narrowing) — the delegated-administration gate holds the root of every delegated business-unit walk to the same organization as its descendants, so the range a self-delegation's anchor is judged against holds units of the caller's organization only. Nothing is widened: no export, error code or status is added.
8+
Metadata-protocol: none
9+
10+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export of the package entry changes: a delegation row keeps its shape and meaning, and the range its anchor is judged against is bounded to the caller's organization. There is nothing for a migration to rewrite and no tombstone. -->
11+
12+
**BREAKING** accept-set narrowing in the delegated-administration gate (`@objectstack/plugin-security`), only for a self-delegation that is anchored, or whose holding is anchored, at a unit outside the caller's organization.
13+
14+
- **Rule.** ADR-0105 D6 (red line 2): every business-unit mechanism operates within one organization. A delegated business-unit walk, its root included, covers only units of the caller's organization. A self-delegation (ADR-0091 D3) anchored at a unit outside the caller's organization, or whose holding's anchor is outside it, is refused `403 PERMISSION_DENIED` with the same answer as an anchor outside the delegator's range, and nothing is stored.
15+
- **Unchanged.** An anchor inside the delegator's range, and an unanchored delegation, are admitted as before. A scope's by-name anchor keeps every answer it gave.
16+
- **Who moves.** Nobody whose delegations and holdings anchor at units of their own organization.

‎packages/plugins/plugin-security/src/delegated-admin-gate.test.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,73 @@ describe('DelegatedAdminGate — self-delegation anchor containment (cloud#830 f
741741
});
742742
});
743743

744+
// ── [ADR-0105 D6] The walk's root is one organization's, on a driver that scopes NOTHING ─
745+
//
746+
// Red line 2: every BU mechanism operates within one organization. The
747+
// harness `ql` ignores `context`, so the root read hands back a unit of any
748+
// organization and the row-level predicate is the only thing standing — the
749+
// same predicate the walk applies to every descendant. The posture rig
750+
// (`delegated-admin-own-name-set.posture.test.ts`) pins the rule through the
751+
// doors on a real engine; this half also pins an organization-less unit, which
752+
// a real table cannot hold (its tenant column is NOT NULL, ADR-0131 D1).
753+
describe("[ADR-0105 D6] DelegatedAdminGate — a delegated business-unit walk never starts at a unit outside the caller's organization", () => {
754+
/** A unit of another organization with a unit below it, and an organization-less unit. */
755+
function withUnitsOutsideTheOrganization(d: ReturnType<typeof makeAnchoredDelegationHarness>) {
756+
d.tables.sys_business_unit.push(
757+
{ id: 'bu_x', name: 'x_desk', parent_business_unit_id: null, organization_id: 'org_other' },
758+
{ id: 'bu_x_field', name: 'x_field', parent_business_unit_id: 'bu_x', organization_id: 'org_other' },
759+
{ id: 'bu_none', name: 'none', parent_business_unit_id: null, organization_id: null },
760+
);
761+
return d;
762+
}
763+
const OUTSIDE = ['bu_x', 'bu_x_field', 'bu_none'];
764+
/** The refusal of `u_boss`'s delegation anchored at `unit`: its envelope, and its message with the anchor spelled out. */
765+
const answerAt = async (d: ReturnType<typeof makeAnchoredDelegationHarness>, unit: string) => {
766+
const err = await refusalOf(() => d.delegate('u_boss', d.base({ business_unit_id: unit })));
767+
return [err.code, err.status, String(err.message).split(`'${unit}'`).join("'ANCHOR'")];
768+
};
769+
770+
it("an unanchored holding's range holds no root outside the caller's organization: a member unit there is refused as any unit outside the range", async () => {
771+
const d = withUnitsOutsideTheOrganization(makeAnchoredDelegationHarness());
772+
d.tables.sys_user_position.find((r: any) => r.id === 'ha').business_unit_id = null;
773+
d.tables.sys_business_unit_member.push(
774+
{ id: 'm_boss_x', user_id: 'u_boss', business_unit_id: 'bu_x', organization_id: 'org_other' },
775+
{ id: 'm_boss_none', user_id: 'u_boss', business_unit_id: 'bu_none', organization_id: DELEGATION_ORG },
776+
);
777+
const outside = await answerAt(d, 'bu_hq');
778+
expect(outside.slice(0, 2), String(outside[2])).toEqual(['PERMISSION_DENIED', 403]);
779+
for (const unit of OUTSIDE) expect(await answerAt(d, unit), unit).toEqual(outside);
780+
// CONTROL: the member unit of the caller's organization still roots the range.
781+
await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_es' }))).resolves.toBeUndefined();
782+
});
783+
784+
it("a holding anchored at a unit outside the caller's organization roots no range: no anchor is admitted through it", async () => {
785+
// The answer of a holding whose anchor names no unit at all: no range.
786+
const noRange = withUnitsOutsideTheOrganization(makeAnchoredDelegationHarness());
787+
noRange.tables.sys_user_position.find((r: any) => r.id === 'ha').business_unit_id = 'bu_no_such_unit';
788+
const refused = await answerAt(noRange, 'bu_es');
789+
expect(refused.slice(0, 2), String(refused[2])).toEqual(['PERMISSION_DENIED', 403]);
790+
for (const anchor of ['bu_x', 'bu_none']) {
791+
const d = withUnitsOutsideTheOrganization(makeAnchoredDelegationHarness());
792+
d.tables.sys_user_position.find((r: any) => r.id === 'ha').business_unit_id = anchor;
793+
for (const unit of [...OUTSIDE, 'bu_es']) expect(await answerAt(d, unit), `${anchor} · ${unit}`).toEqual(refused);
794+
// CONTROL: unanchored, the delegation is admitted, as before.
795+
await expect(d.delegate('u_boss', d.base({}))).resolves.toBeUndefined();
796+
}
797+
});
798+
799+
it("CONTROL: a holding anchored at a unit of the caller's organization keeps its subtree, and a unit of another organization under it stays outside", async () => {
800+
const d = withUnitsOutsideTheOrganization(makeAnchoredDelegationHarness());
801+
d.tables.sys_business_unit.push({ id: 'bu_x_linked', name: 'x_linked', parent_business_unit_id: 'bu_east', organization_id: 'org_other' });
802+
for (const unit of ['bu_east', 'bu_es']) {
803+
await expect(d.delegate('u_boss', d.base({ business_unit_id: unit })), unit).resolves.toBeUndefined();
804+
}
805+
const outside = await answerAt(d, 'bu_hq');
806+
expect(outside.slice(0, 2), String(outside[2])).toEqual(['PERMISSION_DENIED', 403]);
807+
for (const unit of ['bu_x_linked', ...OUTSIDE]) expect(await answerAt(d, unit), unit).toEqual(outside);
808+
});
809+
});
810+
744811
// ── [ADR-0090 D12 / ADR-0105 D8] describeDelegableScope — the READ half ──────
745812
//
746813
// A picker narrows with this; the gate still decides. So the property that

‎packages/plugins/plugin-security/src/delegated-admin-gate.ts‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1752,11 +1752,31 @@ export class DelegatedAdminGate {
17521752
* state on a stamped store, `organization-scope.ts`, so met only on a store
17531753
* whose units carry no organization at all): organization-less descendants
17541754
* only, so a stamped unit is never reached from one, nor one from a stamped
1755-
* root. */
1755+
* root.
1756+
*
1757+
* [ADR-0105 D6, red line 2: BU trees remain org-internal] The ROOT is held
1758+
* to the same bound as its descendants: it is read under the bound's own
1759+
* context, and a root whose row is not a unit of `organizationId` (`null`:
1760+
* an organization-less unit) covers nothing — a walk never starts at a unit
1761+
* outside the organization it is bounded to, whichever caller names the
1762+
* id. */
17561763
private async resolveSubtreeById(rootId: string, organizationId: string | null): Promise<Set<string>> {
17571764
const ids = new Set<string>();
17581765
const ql = this.deps.ql;
17591766
if (!ql?.find || !rootId) return ids;
1767+
let roots: any[] = [];
1768+
try {
1769+
roots = await ql.find('sys_business_unit', {
1770+
where: { id: String(rootId) },
1771+
limit: 1, // `id` is the key: one unit or none
1772+
context: organizationScopedCtx(organizationId ?? undefined),
1773+
});
1774+
} catch { roots = []; }
1775+
// The descendants' predicate, applied to the root: whatever a driver
1776+
// returned, a unit outside the bound's organization roots no subtree.
1777+
const rootInOrganization = (Array.isArray(roots) ? roots : [])
1778+
.some((r: any) => String(r?.id ?? '') === String(rootId) && rowOrganizationId(r) === organizationId);
1779+
if (!rootInOrganization) return ids;
17601780
ids.add(String(rootId));
17611781
let frontier: string[] = [String(rootId)];
17621782
for (let depth = 0; depth < MAX_TREE_DEPTH && frontier.length > 0; depth++) {
@@ -1788,7 +1808,11 @@ export class DelegatedAdminGate {
17881808
* resolves the depth anchor to the holder's own membership). A self-delegated
17891809
* `business_unit_id` anchor must fall inside this set. Fail-closed:
17901810
* unresolvable holdings/memberships contribute nothing, so an anchor that
1791-
* can't be proven inside the delegator's range is refused upstream. */
1811+
* can't be proven inside the delegator's range is refused upstream.
1812+
*
1813+
* Every subtree in the union is one organization's, the caller's, its root
1814+
* included ({@link resolveSubtreeById}): a holding's anchor or a membership
1815+
* that is not a unit of the caller's organization contributes nothing. */
17921816
private async delegatorAnchorSubtree(
17931817
userId: string,
17941818
directHoldings: Array<{ businessUnitId: string | null }>,

0 commit comments

Comments
 (0)