Observed while implementing the read-only crossing-manager_id audit for cloud#1369 (not fixed there — that card is explicitly read-only and files no writes). Recording it here because a fix would land in plugin-approvals, not in cloud.
What was measured
In packages/plugins/plugin-approvals/src/approval-service.ts, expandApprovers hands the directory organization to every graph-shaped approver expansion:
team → expandTeamUsers(value, directoryOrg)department → expandBusinessUnitUsers(value, directoryOrg)position → expandPositionUsers(value, directoryOrg)org_membership_level → expandMembershipTierUsers(value, directoryOrg)
The manager branch does not:
} else if (type === 'manager' && record) {
const subject = (record as any)[a.value] ?? (record as any).owner_id;
if (subject) {
const mgr = await this.lookupManager(String(subject));
if (mgr) return this.applyOooDelegation(mgr, now, organizationId, substitutions);
}
}and lookupManager (same file) reads the column directly under a system context, taking no organization argument at all:
private async lookupManager(userId: string): Promise<string | null> {
const rows = await this.engine.find('sys_user', {
where: { id: userId }, fields: ['id', 'manager_id'], limit: 1, context: SYSTEM_CTX,
} as any);
...
}Why it matters
sys_user is a global identity table with no organization_id column, so nothing else in this path supplies the tenancy fact. If a sys_user.manager_id crosses an organization boundary, an approval step with approverType: 'manager' routes the submission to an approver in another organization — an out-of-tenant person granted approval authority over the record.
The hierarchy consumer of the same column has been screened since cloud#1195 (HierarchyScopeResolver.screenToTenant drops users sys_member proves are outside the caller's org). Approvals is a different consumer of the same column and was not covered by that work, so the same row behaves in opposite directions: it silently narrows an own_and_reports owner set and widens approval routing.
Not asserting the fix shape
Two directions exist and they are not equivalent, so this is filed for triage rather than argued:
- screen
lookupManager the way the sibling expansions are screened (refuse / fall through when provablyDisjointOrgs holds), which makes the approval slot resolve to nobody and needs a story for that;
- treat it as an authoring-time invariant only, on the grounds that cloud#1195's write gate now refuses new crossing links — which leaves rows that predate the gate, the exact residual cloud#1369 exists to report.
Whether approver routing should imply tenancy at all overlaps with the open design question in #7497.
Related
Verified against 907c11d2cee5fcdd420e2eedf79dc6081d08f5c3 (the SHA cloud pins today).
Observed while implementing the read-only crossing-
manager_idaudit for cloud#1369 (not fixed there — that card is explicitly read-only and files no writes). Recording it here because a fix would land inplugin-approvals, not in cloud.What was measured
In
packages/plugins/plugin-approvals/src/approval-service.ts,expandApprovershands the directory organization to every graph-shaped approver expansion:team→expandTeamUsers(value, directoryOrg)department→expandBusinessUnitUsers(value, directoryOrg)position→expandPositionUsers(value, directoryOrg)org_membership_level→expandMembershipTierUsers(value, directoryOrg)The
managerbranch does not:and
lookupManager(same file) reads the column directly under a system context, taking no organization argument at all:Why it matters
sys_useris a global identity table with noorganization_idcolumn, so nothing else in this path supplies the tenancy fact. If asys_user.manager_idcrosses an organization boundary, an approval step withapproverType: 'manager'routes the submission to an approver in another organization — an out-of-tenant person granted approval authority over the record.The hierarchy consumer of the same column has been screened since cloud#1195 (
HierarchyScopeResolver.screenToTenantdrops userssys_memberproves are outside the caller's org). Approvals is a different consumer of the same column and was not covered by that work, so the same row behaves in opposite directions: it silently narrows anown_and_reportsowner set and widens approval routing.Not asserting the fix shape
Two directions exist and they are not equivalent, so this is filed for triage rather than argued:
lookupManagerthe way the sibling expansions are screened (refuse / fall through whenprovablyDisjointOrgsholds), which makes the approval slot resolve to nobody and needs a story for that;Whether approver routing should imply tenancy at all overlaps with the open design question in #7497.
Related
Verified against
907c11d2cee5fcdd420e2eedf79dc6081d08f5c3(the SHA cloud pins today).