Skip to content

Commit 698454e

Browse files
os-zhuangclaude
andauthored
fix(plugin-security): 约束 self-delegation position anchor 防横向可见性越权(#830 follow-up) (#2945)
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 路径重新成立。 Claude-Session: https://claude.ai/code/session_019QRUvVfpvSycAHMMF2xTxs Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2e947e1 commit 698454e

3 files changed

Lines changed: 247 additions & 10 deletions

File tree

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
Security fix: constrain self-delegation (D3) position anchor to prevent lateral
6+
visibility escalation (cloud#830 follow-up).
7+
8+
cloud#830 (C1 position-anchor) made `sys_user_position.business_unit_id`
9+
visibility **load-bearing** — it is the readScope depth anchor, so a
10+
`unit`/`unit_and_below` holder sees the owner set rooted at that BU (and, for
11+
`unit_and_below`, its whole subtree). The delegated-admin gate's self-service
12+
delegation path (`assertSelfDelegation`) stamped this anchor with **no
13+
subtree/source constraint**: a holder of a delegatable, non-admin-scope position
14+
anchored at a LOW business unit could delegate it to a co-conspirator with an
15+
**ancestor / arbitrary-high** anchor, leaking that BU's whole subtree of member
16+
records — visibility beyond the delegator's own range. Mutual delegation could
17+
grant it both ways.
18+
19+
The gate now requires a self-delegated `business_unit_id` to fall inside the
20+
delegator's **own effective anchor** for that position (the subtree of their own
21+
direct holding's anchor, or of their member BU when the holding is unanchored) —
22+
the same "assignments must target your subtree" spirit as the D12
23+
delegated-admin boundary. Fail-closed: an anchor that cannot be proven inside the
24+
delegator's range is rejected. Unanchored delegation rows keep prior behavior
25+
(the delegate resolves to their own member BU — not a widening). The
26+
"anchoring only narrows, never widens" invariant now holds on the D3 path too.

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

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,3 +474,136 @@ describe('DelegatedAdminGate — self-service delegation of duty (ADR-0091 D3)',
474474
.rejects.toThrow(/audience anchor/);
475475
});
476476
});
477+
478+
// ── [cloud#830 follow-up] Self-delegation anchor containment ────────────────
479+
//
480+
// cloud#830 (C1 position-anchor) made sys_user_position.business_unit_id
481+
// visibility LOAD-BEARING: it is the readScope depth anchor, so a
482+
// `unit`/`unit_and_below` holder sees the owner set rooted at that BU (and, for
483+
// unit_and_below, its whole subtree). The self-delegation (D3) path stamped the
484+
// anchor without any subtree/source constraint, so a holder of a delegatable
485+
// non-admin position anchored at a LOW BU could delegate it to a co-conspirator
486+
// with an ANCESTOR/arbitrary-high anchor — leaking that BU's whole subtree,
487+
// beyond the delegator's own range (lateral visibility escalation). The fix
488+
// requires the delegated anchor to fall inside the delegator's OWN effective
489+
// anchor for the position (same spirit as the D12 delegated-admin subtree
490+
// check), fail-closed.
491+
//
492+
// Topology:
493+
// hq (bu_hq)
494+
// ├── east (bu_east) ← u_boss's own approver anchor
495+
// │ └── east_sales (bu_es)
496+
// └── west (bu_west)
497+
function makeAnchoredDelegationHarness(nowMs = T0) {
498+
const tables: Record<string, any[]> = {
499+
sys_business_unit: [
500+
{ id: 'bu_hq', name: 'hq', parent_business_unit_id: null },
501+
{ id: 'bu_east', name: 'east', parent_business_unit_id: 'bu_hq' },
502+
{ id: 'bu_es', name: 'east_sales', parent_business_unit_id: 'bu_east' },
503+
{ id: 'bu_west', name: 'west', parent_business_unit_id: 'bu_hq' },
504+
],
505+
sys_position: [{ id: 'p_appr', name: 'approver', delegatable: true }],
506+
sys_permission_set: [{ id: 's_appr', name: 'approve_set' }],
507+
sys_position_permission_set: [{ id: 'b_appr', position_id: 'p_appr', permission_set_id: 's_appr' }],
508+
// u_boss holds approver DIRECTLY, anchored at east.
509+
sys_user_position: [{ id: 'ha', user_id: 'u_boss', position: 'approver', business_unit_id: 'bu_east' }],
510+
sys_business_unit_member: [{ id: 'm_boss', user_id: 'u_boss', business_unit_id: 'bu_es' }],
511+
sys_user: [{ id: 'u_boss' }, { id: 'u_deleg' }],
512+
};
513+
const matches = (row: any, where: any): boolean =>
514+
Object.entries(where ?? {}).every(([k, v]) => {
515+
if (v && typeof v === 'object' && Array.isArray((v as any).$in)) return (v as any).$in.includes(row[k]);
516+
return row[k] === v;
517+
});
518+
const ql = {
519+
tables,
520+
async find(object: string, opts: any) {
521+
const rows = (tables[object] ?? []).filter((r) => matches(r, opts?.where));
522+
return typeof opts?.limit === 'number' ? rows.slice(0, opts.limit) : rows;
523+
},
524+
async findOne(object: string, opts: any) {
525+
return (tables[object] ?? []).filter((r) => matches(r, opts?.where))[0] ?? null;
526+
},
527+
} as any;
528+
const gate = new DelegatedAdminGate({
529+
ql,
530+
resolveSets: async () => [{ name: 'member_default', objects: {} }],
531+
now: () => nowMs,
532+
});
533+
const delegate = (userId: string, row: any) =>
534+
gate.assert({ object: 'sys_user_position', operation: 'insert', data: row, context: { userId, positions: [] } });
535+
const base = (extra: any) => ({
536+
user_id: 'u_deleg', position: 'approver', delegated_from: 'u_boss',
537+
valid_until: iso(nowMs + 5 * DAY), reason: 'coverage', ...extra,
538+
});
539+
return { gate, ql, tables, delegate, base };
540+
}
541+
542+
describe('DelegatedAdminGate — self-delegation anchor containment (cloud#830 follow-up)', () => {
543+
it('anchor equal to the delegator\'s own anchor (east) is allowed', async () => {
544+
const d = makeAnchoredDelegationHarness();
545+
await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_east' }))).resolves.toBeUndefined();
546+
});
547+
548+
it('anchor inside the delegator\'s own subtree (east_sales) is allowed — narrowing', async () => {
549+
const d = makeAnchoredDelegationHarness();
550+
await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_es' }))).resolves.toBeUndefined();
551+
});
552+
553+
it('anchor at an ANCESTOR BU (hq) is DENIED — a delegation may not widen visibility', async () => {
554+
const d = makeAnchoredDelegationHarness();
555+
await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_hq' })))
556+
.rejects.toThrow(/only narrows|never widen/);
557+
});
558+
559+
it('anchor at an UNRELATED sibling BU (west) is DENIED — outside your own effective anchor', async () => {
560+
const d = makeAnchoredDelegationHarness();
561+
await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_west' })))
562+
.rejects.toThrow(/outside your own effective anchor/);
563+
});
564+
565+
it('an unanchored delegation row keeps prior behavior (no anchor to widen)', async () => {
566+
const d = makeAnchoredDelegationHarness();
567+
await expect(d.delegate('u_boss', d.base({}))).resolves.toBeUndefined();
568+
});
569+
570+
it('mutual delegation cannot cross ranges: neither direction may hand out the other\'s BU', async () => {
571+
// u_boss anchored east may not delegate a west anchor…
572+
const d = makeAnchoredDelegationHarness();
573+
await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_west' })))
574+
.rejects.toThrow(/outside your own effective anchor/);
575+
// …and a would-be west holder cannot reach east either (no east holding to source it).
576+
d.tables.sys_user_position.push({ id: 'hb', user_id: 'u_deleg', position: 'approver', business_unit_id: 'bu_west' });
577+
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' }))
578+
.rejects.toThrow(/outside your own effective anchor/);
579+
});
580+
581+
it('when the delegator holds the position UNANCHORED, the anchor is bounded by their MEMBER BU', async () => {
582+
const d = makeAnchoredDelegationHarness();
583+
// Drop the anchor on u_boss's own holding; boss is a member of east_sales.
584+
d.tables.sys_user_position.find((r: any) => r.id === 'ha').business_unit_id = null;
585+
// Member-BU (east_sales) or below is allowed…
586+
await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_es' }))).resolves.toBeUndefined();
587+
// …but the parent (east) — above the member BU — is not.
588+
await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_east' })))
589+
.rejects.toThrow(/outside your own effective anchor/);
590+
});
591+
592+
it('fail-closed: an anchor that cannot be validated (delegator has no resolvable range) is refused', async () => {
593+
const d = makeAnchoredDelegationHarness();
594+
// Boss holds approver unanchored AND has no BU membership → no provable range.
595+
d.tables.sys_user_position.find((r: any) => r.id === 'ha').business_unit_id = null;
596+
d.tables.sys_business_unit_member.length = 0;
597+
await expect(d.delegate('u_boss', d.base({ business_unit_id: 'bu_es' })))
598+
.rejects.toThrow(/cannot be validated/);
599+
});
600+
601+
it('a holding acquired VIA delegation cannot source an anchored re-delegation (chains stay cut)', async () => {
602+
const d = makeAnchoredDelegationHarness();
603+
// u_relay holds approver only via delegation, anchored east.
604+
d.tables.sys_user.push({ id: 'u_relay' });
605+
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) });
606+
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' }))
607+
.rejects.toThrow(/only via delegation/);
608+
});
609+
});

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

Lines changed: 88 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,10 @@ export class DelegatedAdminGate {
259259
* re-delegatable (chains are cut);
260260
* 5. the position is `delegatable: true`;
261261
* 6. the position distributes NO `adminScope`-carrying set (administration
262-
* is never self-delegated — that would bypass the D12 containment gate).
262+
* is never self-delegated — that would bypass the D12 containment gate);
263+
* 7. [cloud#830 follow-up] if the delegation carries a `business_unit_id`
264+
* anchor, that anchor falls inside the delegator's OWN effective anchor
265+
* for the position — a delegation may NARROW visibility, never widen it.
263266
* The writer is stamped into `granted_by` (dual audit: `granted_by` = writer,
264267
* `delegated_from` = authority source).
265268
*/
@@ -321,6 +324,36 @@ export class DelegatedAdminGate {
321324
deny(`you do not currently hold '${positionName}' — only a current holder may delegate it`, { position: positionName });
322325
}
323326

327+
// 4b. [cloud#830 follow-up] Anchor containment. `business_unit_id` is
328+
// visibility LOAD-BEARING (cloud#830 made it the readScope depth
329+
// anchor: a `unit`/`unit_and_below` holder sees the owner set rooted
330+
// at this BU). "Anchoring only narrows, never widens" must therefore
331+
// hold on THIS path too — otherwise a holder anchored at a low BU
332+
// could hand a co-conspirator an ANCESTOR BU and leak that whole
333+
// subtree's records, exceeding the delegator's own range (lateral
334+
// escalation). So a delegated anchor must fall inside the delegator's
335+
// OWN effective anchor for this position — same spirit as the D12
336+
// delegated-admin subtree check (assertAssignmentWrite). Fail-closed:
337+
// an anchor we cannot prove is inside the delegator's range is
338+
// refused. An unanchored delegation row keeps the prior behavior (the
339+
// delegate resolves to their own member BU — not a widening).
340+
const rowAnchor =
341+
row?.business_unit_id != null && row.business_unit_id !== '' ? String(row.business_unit_id) : null;
342+
if (rowAnchor) {
343+
const allowed = await this.delegatorAnchorSubtree(
344+
String(ctx.userId),
345+
holdings.filter((hd) => hd.direct),
346+
);
347+
if (!allowed.has(rowAnchor)) {
348+
deny(
349+
allowed.size === 0
350+
? `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)`
351+
: `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)`,
352+
{ position: positionName, businessUnitId: rowAnchor },
353+
);
354+
}
355+
}
356+
324357
// 5. The position must opt in to delegation.
325358
if (!(await this.positionIsDelegatable(positionName))) {
326359
deny(`position '${positionName}' is not delegatable — set delegatable: true on the position to allow it`, { position: positionName });
@@ -341,12 +374,15 @@ export class DelegatedAdminGate {
341374

342375
/** The actor's holdings of `positionName` that are inside their validity
343376
* window, tagged `direct` when the holding itself did NOT arrive via
344-
* delegation (only a direct holding is re-delegatable). */
377+
* delegation (only a direct holding is re-delegatable) and carrying each
378+
* holding's own `businessUnitId` anchor (null = unanchored). The anchor of a
379+
* direct holding bounds what a self-delegation of that position may hand out
380+
* (cloud#830 — the anchor is visibility load-bearing). */
345381
private async activeHoldings(
346382
userId: string,
347383
positionName: string,
348384
now: number,
349-
): Promise<Array<{ direct: boolean }>> {
385+
): Promise<Array<{ direct: boolean; businessUnitId: string | null }>> {
350386
const ql = this.deps.ql;
351387
if (!ql?.find) return [];
352388
let rows: any[] = [];
@@ -361,7 +397,11 @@ export class DelegatedAdminGate {
361397
}
362398
return (Array.isArray(rows) ? rows : [])
363399
.filter((r) => isGrantActive(r, now))
364-
.map((r) => ({ direct: r?.delegated_from == null || r.delegated_from === '' }));
400+
.map((r) => ({
401+
direct: r?.delegated_from == null || r.delegated_from === '',
402+
businessUnitId:
403+
r?.business_unit_id != null && r.business_unit_id !== '' ? String(r.business_unit_id) : null,
404+
}));
365405
}
366406

367407
private async positionIsDelegatable(positionName: string): Promise<boolean> {
@@ -690,18 +730,28 @@ export class DelegatedAdminGate {
690730

691731
/** BU name → covered BU-id set (root + descendants when includeSubtree). */
692732
private async resolveSubtree(businessUnitName: string, includeSubtree: boolean): Promise<Set<string>> {
693-
const ids = new Set<string>();
694733
const ql = this.deps.ql;
695-
if (!ql?.find) return ids;
734+
if (!ql?.find) return new Set<string>();
696735
let root: any = null;
697736
try {
698737
const roots = await ql.find('sys_business_unit', { where: { name: businessUnitName }, limit: 1, context: SYSTEM_CTX });
699738
root = Array.isArray(roots) && roots[0] ? roots[0] : null;
700739
} catch { root = null; }
701-
if (!root?.id) return ids; // misconfigured scope → approves nothing (fail closed)
702-
ids.add(String(root.id));
703-
if (!includeSubtree) return ids;
704-
let frontier: string[] = [String(root.id)];
740+
if (!root?.id) return new Set<string>(); // misconfigured scope → approves nothing (fail closed)
741+
if (!includeSubtree) return new Set<string>([String(root.id)]);
742+
return this.resolveSubtreeById(String(root.id));
743+
}
744+
745+
/** BU id → covered BU-id set (root id + all descendants). Subtree is always
746+
* walked: the delegator's readScope depth is not known on the delegation
747+
* path, so the whole subtree is the containment bound — matching the D12
748+
* admin subtree check. Fail-closed on unresolvable ids (empty set). */
749+
private async resolveSubtreeById(rootId: string): Promise<Set<string>> {
750+
const ids = new Set<string>();
751+
const ql = this.deps.ql;
752+
if (!ql?.find || !rootId) return ids;
753+
ids.add(String(rootId));
754+
let frontier: string[] = [String(rootId)];
705755
for (let depth = 0; depth < MAX_TREE_DEPTH && frontier.length > 0; depth++) {
706756
let children: any[] = [];
707757
try {
@@ -721,6 +771,34 @@ export class DelegatedAdminGate {
721771
return ids;
722772
}
723773

774+
/** [cloud#830 follow-up] The union BU-subtree covered by the delegator's OWN
775+
* DIRECT holdings of a position: subtree(anchor) for each anchored holding,
776+
* plus subtree(member BU) for each unanchored holding (an unanchored holding
777+
* resolves the depth anchor to the holder's own membership). A self-delegated
778+
* `business_unit_id` anchor must fall inside this set. Fail-closed:
779+
* unresolvable holdings/memberships contribute nothing, so an anchor that
780+
* can't be proven inside the delegator's range is refused upstream. */
781+
private async delegatorAnchorSubtree(
782+
userId: string,
783+
directHoldings: Array<{ businessUnitId: string | null }>,
784+
): Promise<Set<string>> {
785+
const allowed = new Set<string>();
786+
let memberSubtreeResolved = false;
787+
for (const h of directHoldings) {
788+
if (h.businessUnitId) {
789+
for (const id of await this.resolveSubtreeById(h.businessUnitId)) allowed.add(id);
790+
} else if (!memberSubtreeResolved) {
791+
// An unanchored direct holding resolves to the delegator's own member
792+
// BU(s); resolve those once (they don't vary by holding).
793+
memberSubtreeResolved = true;
794+
for (const bu of await this.businessUnitsOfUser(userId)) {
795+
for (const id of await this.resolveSubtreeById(bu)) allowed.add(id);
796+
}
797+
}
798+
}
799+
return allowed;
800+
}
801+
724802
/** Rows targeted by this write: `{ next, prev }` per row (prev = pre-image on update/delete). */
725803
private async materializeTargets(opCtx: any, object: string): Promise<Array<{ next: any | null; prev: any | null }>> {
726804
const op = opCtx.operation;

0 commit comments

Comments
 (0)