Skip to content

Commit bf2ca34

Browse files
committed
wip: refuse an unresolvable reassign target
Claude-Session: https://claude.ai/code/session_01HopxxRtFE6z3z91wUD54tp Co-authored-by: Claude <noreply@anthropic.com>
1 parent ba9e0c3 commit bf2ca34

2 files changed

Lines changed: 82 additions & 0 deletions

File tree

‎packages/plugins/plugin-approvals/src/approval-service.ts‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ import {
8686
// decision methods' slot test and `can_act` alike (`approver-address.ts`).
8787
import {
8888
actingAddresses,
89+
addressedPosition,
8990
equivalentApproverAddresses,
9091
heldSlot,
9192
positionAddresses,
@@ -5513,6 +5514,10 @@ export class ApprovalService implements IApprovalService {
55135514
} else {
55145515
throw new Error(`FORBIDDEN: '${from}' is not a pending approver on this request`);
55155516
}
5517+
// #25485 — the new slot must name someone who can take it. Asked only
5518+
// AFTER the authorization branches above, so a caller who may not move
5519+
// this slot learns nothing about the directory from the answer.
5520+
await this.assertReassignTargetResolves(to, raw.organization_id ?? null);
55165521
const now = this.clock.now().toISOString();
55175522
// Audit first, then mutate — mirrors decideNode(), so a failed audit
55185523
// write can never leave a moved slot without a trail.
@@ -5570,6 +5575,69 @@ export class ApprovalService implements IApprovalService {
55705575
return { request: fresh };
55715576
}
55725577

5578+
/**
5579+
* Refuse a reassign target that resolves to no one (#25485): a slot nobody
5580+
* can take strands the request, waiting on no one until an override actor
5581+
* moves it again. `to` is a slot address in the spellings a slate holds, and
5582+
* it resolves when:
5583+
*
5584+
* - a user id or an email — a `sys_user` row carries it (`id`, or `email`
5585+
* as stored: the spelling a default actor takes the slot under,
5586+
* {@link actingAddresses}), and that user is not PROVABLY a member of
5587+
* other organizations only. "Provably" is the posture of the
5588+
* manager and team screens ({@link managerIsProvablyOutsideOrg},
5589+
* {@link dropMembersProvablyOutsideOrg}), on the same `sys_member`
5590+
* column: membership rows that name other organizations and not this one
5591+
* refuse; no membership rows at all, or a request with no organization,
5592+
* leave the target as it is;
5593+
* - `position:<p>` — someone holds `p` in the request's organization
5594+
* ({@link expandPositionUsers}, the routing read a position approver
5595+
* opens on);
5596+
* - any other `type:value` literal — never: no actor takes a `team:` /
5597+
* `department:` / `org_membership_level:` / stored `role:` address
5598+
* (`approver-address.ts`), so such a slot is the stranded slate itself.
5599+
*
5600+
* One refusal text for a missing user and a user of another organization,
5601+
* so the answer is not a probe of who exists in other tenants. A failed
5602+
* `sys_user` read propagates — the caller is told, and nothing moved — while
5603+
* an unreadable `sys_member` is an absent tenancy fact, as in the sibling
5604+
* screens.
5605+
*/
5606+
private async assertReassignTargetResolves(to: string, organizationId: string | null): Promise<void> {
5607+
const refuse = (why: string): never => {
5608+
throw new Error(
5609+
`VALIDATION_FAILED: cannot reassign to '${to}' — ${why} The request and its approvers are unchanged. `
5610+
+ `Reassign to the user id or email of a person in this request's organization, or to `
5611+
+ `'position:<name>' for a position someone in it holds.`,
5612+
);
5613+
};
5614+
const position = addressedPosition(to);
5615+
if (position !== undefined) {
5616+
if ((await this.expandPositionUsers(position, organizationId)).length) return;
5617+
refuse(`no one holds position '${position}' in this request's organization.`);
5618+
}
5619+
if (to.includes(':')) {
5620+
refuse('it is a slot literal no approver acts under, so no one could ever decide the request.');
5621+
}
5622+
const rows = await this.engine.find('sys_user', {
5623+
where: to.includes('@') ? { email: to } : { id: to },
5624+
fields: ['id'], limit: 1, context: SYSTEM_CTX,
5625+
});
5626+
const userId = Array.isArray(rows) && rows[0]?.id != null ? String(rows[0].id) : '';
5627+
if (!userId) refuse('it names no user in this request\'s organization.');
5628+
if (!organizationId) return;
5629+
let memberships: any[] = [];
5630+
try {
5631+
memberships = await this.engine.find('sys_member', {
5632+
where: { user_id: userId }, fields: ['organization_id'], limit: 1000, context: SYSTEM_CTX,
5633+
});
5634+
} catch { return; } // membership unreadable ⇒ no tenancy fact, as in the sibling screens
5635+
const orgs = (memberships ?? []).map((r: any) => String(r?.organization_id ?? '')).filter(Boolean);
5636+
if (orgs.length && !orgs.includes(String(organizationId))) {
5637+
refuse('it names no user in this request\'s organization.');
5638+
}
5639+
}
5640+
55735641
/**
55745642
* Submitter nudge — notify every pending approver. Throttled to one
55755643
* reminder per {@link REMIND_COOLDOWN_MS} per request.

‎packages/plugins/plugin-approvals/src/approver-address.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,20 @@ export function equivalentApproverAddresses(address: string): string[] {
8787
return [address];
8888
}
8989

90+
/**
91+
* The position `address` names — `p` for `position:<p>` — or `undefined` when
92+
* it names none (a user id, an email, any other `type:value` literal, a stored
93+
* `role:` slot). The inverse of {@link positionAddresses}, read where a slot
94+
* address is RESOLVED to the people who could take it (`reassign`'s target
95+
* check), never compared with a caller.
96+
*/
97+
export function addressedPosition(address: string): string | undefined {
98+
for (const prefix of POSITION_ADDRESS_PREFIXES) {
99+
if (address.startsWith(prefix)) return address.slice(prefix.length);
100+
}
101+
return undefined;
102+
}
103+
90104
/**
91105
* The caller's identity as the SERVER resolved it — never a client-supplied
92106
* value: a body or query parameter that names an address is an ASK, judged

0 commit comments

Comments
 (0)