From aa3b3394fe3006434048ef9e6dea2b8f52580606 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Jul 2026 03:43:31 +0000 Subject: [PATCH] =?UTF-8?q?fix(plugin-security):=20=E7=BA=A6=E6=9D=9F=20se?= =?UTF-8?q?lf-delegation=20position=20anchor=20=E9=98=B2=E6=A8=AA=E5=90=91?= =?UTF-8?q?=E5=8F=AF=E8=A7=81=E6=80=A7=E8=B6=8A=E6=9D=83=EF=BC=88#830=20fo?= =?UTF-8?q?llow-up=EF=BC=89?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cloud#830 (C1 position-anchor) 把 sys_user_position.business_unit_id 变成可见性 load-bearing（readScope 的深度锚定）。自助代理（D3）路径 assertSelfDelegation 对该 anchor 无任何 subtree/来源约束：持有一个 delegatable、非 admin-scope、锚定在低层 BU 的 position 的用户，可把它代理给同谋并把 business_unit_id 设成任意高层/祖先 BU，从而 泄露该 BU 整棵子树的成员记录可见性——超出委派人自身范围；互相代理可双向获得。 修复：self-delegation 的 business_unit_id 必须落在委派人自己对该 position 的有效 anchor 之内（自己直接持有行 anchor 的子树，或持有行无 anchor 时其成员 BU 的子树）—— 与 D12 委派管理员「assignments 必须落自己 subtree」同精神。fail-closed：无法证明落在 委派人范围内的 anchor 一律拒。无 anchor 的代理行保持原行为（代理人解析到自己的成员 BU，非放宽）。「锚定只收窄不放宽」不变量在 D3 路径重新成立。 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019QRUvVfpvSycAHMMF2xTxs --- .changeset/authz-secfix-self-deleg-anchor.md | 26 ++++ .../src/delegated-admin-gate.test.ts | 133 ++++++++++++++++++ .../src/delegated-admin-gate.ts | 98 +++++++++++-- 3 files changed, 247 insertions(+), 10 deletions(-) create mode 100644 .changeset/authz-secfix-self-deleg-anchor.md diff --git a/.changeset/authz-secfix-self-deleg-anchor.md b/.changeset/authz-secfix-self-deleg-anchor.md new file mode 100644 index 0000000000..98f008a062 --- /dev/null +++ b/.changeset/authz-secfix-self-deleg-anchor.md @@ -0,0 +1,26 @@ +--- +'@objectstack/plugin-security': patch +--- + +Security fix: constrain self-delegation (D3) position anchor to prevent lateral +visibility escalation (cloud#830 follow-up). + +cloud#830 (C1 position-anchor) made `sys_user_position.business_unit_id` +visibility **load-bearing** — it is the readScope depth anchor, so a +`unit`/`unit_and_below` holder sees the owner set rooted at that BU (and, for +`unit_and_below`, its whole subtree). The delegated-admin gate's self-service +delegation path (`assertSelfDelegation`) stamped this anchor with **no +subtree/source constraint**: a holder of a delegatable, non-admin-scope position +anchored at a LOW business unit could delegate it to a co-conspirator with an +**ancestor / arbitrary-high** anchor, leaking that BU's whole subtree of member +records — visibility beyond the delegator's own range. Mutual delegation could +grant it both ways. + +The gate now requires a self-delegated `business_unit_id` to fall inside the +delegator's **own effective anchor** for that position (the subtree of their own +direct holding's anchor, or of their member BU when the holding is unanchored) — +the same "assignments must target your subtree" spirit as the D12 +delegated-admin boundary. Fail-closed: an anchor that cannot be proven inside the +delegator's range is rejected. Unanchored delegation rows keep prior behavior +(the delegate resolves to their own member BU — not a widening). The +"anchoring only narrows, never widens" invariant now holds on the D3 path too. diff --git a/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts b/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts index d5b81a8ee8..b93124b0d6 100644 --- a/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts +++ b/packages/plugins/plugin-security/src/delegated-admin-gate.test.ts @@ -474,3 +474,136 @@ describe('DelegatedAdminGate — self-service delegation of duty (ADR-0091 D3)', .rejects.toThrow(/audience anchor/); }); }); + +// ── [cloud#830 follow-up] Self-delegation anchor containment ──────────────── +// +// cloud#830 (C1 position-anchor) made sys_user_position.business_unit_id +// visibility LOAD-BEARING: it is the readScope depth anchor, so a +// `unit`/`unit_and_below` holder sees the owner set rooted at that BU (and, for +// unit_and_below, its whole subtree). The self-delegation (D3) path stamped the +// anchor without any subtree/source constraint, so a holder of a delegatable +// non-admin position anchored at a LOW BU could delegate it to a co-conspirator +// with an ANCESTOR/arbitrary-high anchor — leaking that BU's whole subtree, +// beyond the delegator's own range (lateral visibility escalation). The fix +// requires the delegated anchor to fall inside the delegator's OWN effective +// anchor for the position (same spirit as the D12 delegated-admin subtree +// check), fail-closed. +// +// Topology: +// hq (bu_hq) +// ├── east (bu_east) ← u_boss's own approver anchor +// │ └── east_sales (bu_es) +// └── west (bu_west) +function makeAnchoredDelegationHarness(nowMs = T0) { + const tables: Record = { + sys_business_unit: [ + { id: 'bu_hq', name: 'hq', parent_business_unit_id: null }, + { id: 'bu_east', name: 'east', parent_business_unit_id: 'bu_hq' }, + { id: 'bu_es', name: 'east_sales', parent_business_unit_id: 'bu_east' }, + { id: 'bu_west', name: 'west', parent_business_unit_id: 'bu_hq' }, + ], + sys_position: [{ id: 'p_appr', name: 'approver', delegatable: true }], + sys_permission_set: [{ id: 's_appr', name: 'approve_set' }], + sys_position_permission_set: [{ id: 'b_appr', position_id: 'p_appr', permission_set_id: 's_appr' }], + // u_boss holds approver DIRECTLY, anchored at east. + sys_user_position: [{ id: 'ha', user_id: 'u_boss', position: 'approver', business_unit_id: 'bu_east' }], + sys_business_unit_member: [{ id: 'm_boss', user_id: 'u_boss', business_unit_id: 'bu_es' }], + sys_user: [{ id: 'u_boss' }, { id: 'u_deleg' }], + }; + const matches = (row: any, where: any): boolean => + Object.entries(where ?? {}).every(([k, v]) => { + if (v && typeof v === 'object' && Array.isArray((v as any).$in)) return (v as any).$in.includes(row[k]); + return row[k] === v; + }); + const ql = { + tables, + async find(object: string, opts: any) { + const rows = (tables[object] ?? []).filter((r) => matches(r, opts?.where)); + return typeof opts?.limit === 'number' ? rows.slice(0, opts.limit) : rows; + }, + async findOne(object: string, opts: any) { + return (tables[object] ?? []).filter((r) => matches(r, opts?.where))[0] ?? null; + }, + } as any; + const gate = new DelegatedAdminGate({ + ql, + resolveSets: async () => [{ name: 'member_default', objects: {} }], + now: () => nowMs, + }); + const delegate = (userId: string, row: any) => + gate.assert({ object: 'sys_user_position', operation: 'insert', data: row, context: { userId, positions: [] } }); + const base = (extra: any) => ({ + user_id: 'u_deleg', position: 'approver', delegated_from: 'u_boss', + valid_until: iso(nowMs + 5 * DAY), reason: 'coverage', ...extra, + }); + return { gate, ql, tables, delegate, base }; +} + +describe('DelegatedAdminGate — self-delegation anchor containment (cloud#830 follow-up)', () => { + it('anchor equal to the delegator\'s own anchor (east) is allowed', async () => { + const d = makeAnchoredDelegationHarness(); + await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_east' }))).resolves.toBeUndefined(); + }); + + it('anchor inside the delegator\'s own subtree (east_sales) is allowed — narrowing', async () => { + const d = makeAnchoredDelegationHarness(); + await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_es' }))).resolves.toBeUndefined(); + }); + + it('anchor at an ANCESTOR BU (hq) is DENIED — a delegation may not widen visibility', async () => { + const d = makeAnchoredDelegationHarness(); + await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_hq' }))) + .rejects.toThrow(/only narrows|never widen/); + }); + + it('anchor at an UNRELATED sibling BU (west) is DENIED — outside your own effective anchor', async () => { + const d = makeAnchoredDelegationHarness(); + await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_west' }))) + .rejects.toThrow(/outside your own effective anchor/); + }); + + it('an unanchored delegation row keeps prior behavior (no anchor to widen)', async () => { + const d = makeAnchoredDelegationHarness(); + await expect(d.delegate('u_boss', d.base({}))).resolves.toBeUndefined(); + }); + + it('mutual delegation cannot cross ranges: neither direction may hand out the other\'s BU', async () => { + // u_boss anchored east may not delegate a west anchor… + const d = makeAnchoredDelegationHarness(); + await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_west' }))) + .rejects.toThrow(/outside your own effective anchor/); + // …and a would-be west holder cannot reach east either (no east holding to source it). + d.tables.sys_user_position.push({ id: 'hb', user_id: 'u_deleg', position: 'approver', business_unit_id: 'bu_west' }); + await expect(d.delegate('u_deleg', { user_id: 'u_boss', position: 'approver', delegated_from: 'u_deleg', valid_until: iso(T0 + 5 * DAY), reason: 'x', business_unit_id: 'bu_east' })) + .rejects.toThrow(/outside your own effective anchor/); + }); + + it('when the delegator holds the position UNANCHORED, the anchor is bounded by their MEMBER BU', async () => { + const d = makeAnchoredDelegationHarness(); + // Drop the anchor on u_boss's own holding; boss is a member of east_sales. + d.tables.sys_user_position.find((r: any) => r.id === 'ha').business_unit_id = null; + // Member-BU (east_sales) or below is allowed… + await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_es' }))).resolves.toBeUndefined(); + // …but the parent (east) — above the member BU — is not. + await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_east' }))) + .rejects.toThrow(/outside your own effective anchor/); + }); + + it('fail-closed: an anchor that cannot be validated (delegator has no resolvable range) is refused', async () => { + const d = makeAnchoredDelegationHarness(); + // Boss holds approver unanchored AND has no BU membership → no provable range. + d.tables.sys_user_position.find((r: any) => r.id === 'ha').business_unit_id = null; + d.tables.sys_business_unit_member.length = 0; + await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_es' }))) + .rejects.toThrow(/cannot be validated/); + }); + + it('a holding acquired VIA delegation cannot source an anchored re-delegation (chains stay cut)', async () => { + const d = makeAnchoredDelegationHarness(); + // u_relay holds approver only via delegation, anchored east. + d.tables.sys_user.push({ id: 'u_relay' }); + d.tables.sys_user_position.push({ id: 'hd', user_id: 'u_relay', position: 'approver', business_unit_id: 'bu_east', delegated_from: 'u_boss', valid_until: iso(T0 + 20 * DAY) }); + await expect(d.delegate('u_relay', { user_id: 'u_deleg', position: 'approver', delegated_from: 'u_relay', valid_until: iso(T0 + 5 * DAY), reason: 'x', business_unit_id: 'bu_es' })) + .rejects.toThrow(/only via delegation/); + }); +}); diff --git a/packages/plugins/plugin-security/src/delegated-admin-gate.ts b/packages/plugins/plugin-security/src/delegated-admin-gate.ts index 4accbfea6b..2e005a8cc3 100644 --- a/packages/plugins/plugin-security/src/delegated-admin-gate.ts +++ b/packages/plugins/plugin-security/src/delegated-admin-gate.ts @@ -259,7 +259,10 @@ export class DelegatedAdminGate { * re-delegatable (chains are cut); * 5. the position is `delegatable: true`; * 6. the position distributes NO `adminScope`-carrying set (administration - * is never self-delegated — that would bypass the D12 containment gate). + * is never self-delegated — that would bypass the D12 containment gate); + * 7. [cloud#830 follow-up] if the delegation carries a `business_unit_id` + * anchor, that anchor falls inside the delegator's OWN effective anchor + * for the position — a delegation may NARROW visibility, never widen it. * The writer is stamped into `granted_by` (dual audit: `granted_by` = writer, * `delegated_from` = authority source). */ @@ -321,6 +324,36 @@ export class DelegatedAdminGate { deny(`you do not currently hold '${positionName}' — only a current holder may delegate it`, { position: positionName }); } + // 4b. [cloud#830 follow-up] Anchor containment. `business_unit_id` is + // visibility LOAD-BEARING (cloud#830 made it the readScope depth + // anchor: a `unit`/`unit_and_below` holder sees the owner set rooted + // at this BU). "Anchoring only narrows, never widens" must therefore + // hold on THIS path too — otherwise a holder anchored at a low BU + // could hand a co-conspirator an ANCESTOR BU and leak that whole + // subtree's records, exceeding the delegator's own range (lateral + // escalation). So a delegated anchor must fall inside the delegator's + // OWN effective anchor for this position — same spirit as the D12 + // delegated-admin subtree check (assertAssignmentWrite). Fail-closed: + // an anchor we cannot prove is inside the delegator's range is + // refused. An unanchored delegation row keeps the prior behavior (the + // delegate resolves to their own member BU — not a widening). + const rowAnchor = + row?.business_unit_id != null && row.business_unit_id !== '' ? String(row.business_unit_id) : null; + if (rowAnchor) { + const allowed = await this.delegatorAnchorSubtree( + String(ctx.userId), + holdings.filter((hd) => hd.direct), + ); + if (!allowed.has(rowAnchor)) { + deny( + allowed.size === 0 + ? `business unit anchor '${rowAnchor}' cannot be validated against your own '${positionName}' anchor — an anchor that cannot be proven within your own range is refused (cloud#830: anchoring only narrows)` + : `business unit anchor '${rowAnchor}' is outside your own effective anchor for '${positionName}' — a delegation may only narrow visibility, never widen it (cloud#830: anchoring only narrows)`, + { position: positionName, businessUnitId: rowAnchor }, + ); + } + } + // 5. The position must opt in to delegation. if (!(await this.positionIsDelegatable(positionName))) { deny(`position '${positionName}' is not delegatable — set delegatable: true on the position to allow it`, { position: positionName }); @@ -341,12 +374,15 @@ export class DelegatedAdminGate { /** The actor's holdings of `positionName` that are inside their validity * window, tagged `direct` when the holding itself did NOT arrive via - * delegation (only a direct holding is re-delegatable). */ + * delegation (only a direct holding is re-delegatable) and carrying each + * holding's own `businessUnitId` anchor (null = unanchored). The anchor of a + * direct holding bounds what a self-delegation of that position may hand out + * (cloud#830 — the anchor is visibility load-bearing). */ private async activeHoldings( userId: string, positionName: string, now: number, - ): Promise> { + ): Promise> { const ql = this.deps.ql; if (!ql?.find) return []; let rows: any[] = []; @@ -361,7 +397,11 @@ export class DelegatedAdminGate { } return (Array.isArray(rows) ? rows : []) .filter((r) => isGrantActive(r, now)) - .map((r) => ({ direct: r?.delegated_from == null || r.delegated_from === '' })); + .map((r) => ({ + direct: r?.delegated_from == null || r.delegated_from === '', + businessUnitId: + r?.business_unit_id != null && r.business_unit_id !== '' ? String(r.business_unit_id) : null, + })); } private async positionIsDelegatable(positionName: string): Promise { @@ -690,18 +730,28 @@ export class DelegatedAdminGate { /** BU name → covered BU-id set (root + descendants when includeSubtree). */ private async resolveSubtree(businessUnitName: string, includeSubtree: boolean): Promise> { - const ids = new Set(); const ql = this.deps.ql; - if (!ql?.find) return ids; + if (!ql?.find) return new Set(); let root: any = null; try { const roots = await ql.find('sys_business_unit', { where: { name: businessUnitName }, limit: 1, context: SYSTEM_CTX }); root = Array.isArray(roots) && roots[0] ? roots[0] : null; } catch { root = null; } - if (!root?.id) return ids; // misconfigured scope → approves nothing (fail closed) - ids.add(String(root.id)); - if (!includeSubtree) return ids; - let frontier: string[] = [String(root.id)]; + if (!root?.id) return new Set(); // misconfigured scope → approves nothing (fail closed) + if (!includeSubtree) return new Set([String(root.id)]); + return this.resolveSubtreeById(String(root.id)); + } + + /** BU id → covered BU-id set (root id + all descendants). Subtree is always + * walked: the delegator's readScope depth is not known on the delegation + * path, so the whole subtree is the containment bound — matching the D12 + * admin subtree check. Fail-closed on unresolvable ids (empty set). */ + private async resolveSubtreeById(rootId: string): Promise> { + const ids = new Set(); + const ql = this.deps.ql; + if (!ql?.find || !rootId) return ids; + ids.add(String(rootId)); + let frontier: string[] = [String(rootId)]; for (let depth = 0; depth < MAX_TREE_DEPTH && frontier.length > 0; depth++) { let children: any[] = []; try { @@ -721,6 +771,34 @@ export class DelegatedAdminGate { return ids; } + /** [cloud#830 follow-up] The union BU-subtree covered by the delegator's OWN + * DIRECT holdings of a position: subtree(anchor) for each anchored holding, + * plus subtree(member BU) for each unanchored holding (an unanchored holding + * resolves the depth anchor to the holder's own membership). A self-delegated + * `business_unit_id` anchor must fall inside this set. Fail-closed: + * unresolvable holdings/memberships contribute nothing, so an anchor that + * can't be proven inside the delegator's range is refused upstream. */ + private async delegatorAnchorSubtree( + userId: string, + directHoldings: Array<{ businessUnitId: string | null }>, + ): Promise> { + const allowed = new Set(); + let memberSubtreeResolved = false; + for (const h of directHoldings) { + if (h.businessUnitId) { + for (const id of await this.resolveSubtreeById(h.businessUnitId)) allowed.add(id); + } else if (!memberSubtreeResolved) { + // An unanchored direct holding resolves to the delegator's own member + // BU(s); resolve those once (they don't vary by holding). + memberSubtreeResolved = true; + for (const bu of await this.businessUnitsOfUser(userId)) { + for (const id of await this.resolveSubtreeById(bu)) allowed.add(id); + } + } + } + return allowed; + } + /** Rows targeted by this write: `{ next, prev }` per row (prev = pre-image on update/delete). */ private async materializeTargets(opCtx: any, object: string): Promise> { const op = opCtx.operation;