Skip to content

approvals: the team approver expansion is not organization-screened either — a cross-org team resolves into a request's approver slate #10230

Description

@os-warren

Found while verifying the warrant of #10153 ("every sibling approver expansion is org-scoped"). Filed separately rather than widened into that card, per its scope fence.

What was measured

#10153's body lists team among the org-scoped expansions:

  • teamexpandTeamUsers(value, directoryOrg)

That is not what main does. packages/plugins/plugin-approvals/src/approval-service.ts calls it with one argument at both call sites (the resolveApproverSpec branch and the expression / resolveAs branch), and the method takes no organization parameter at all:

privateasyncexpandTeamUsers(teamId: string): Promise<string[]>{
...
rows=awaitthis.engine.find('sys_team_member',{where: {team_id: teamId},fields: ['user_id'],limit: 10000,context: SYSTEM_CTX,}asany);

team is also the only graph-shaped type not wrapped in the bounded(...) helper that applies filterApproversWhoCanRead on a cross-org slate — department, position and org_membership_level all are.

Measured on a fake-engine harness at 04096f17e, with a request in org_a routed to a team stamped organization_id: 'org_b':

[PROBE W] org_a request, org_b team -> pending_approvers = ["u_team_b"]

For contrast, on the same tree and the same run, the screened sibling resolves to the dead literal:

[PROBE B] org_a request, only an org_b `cfo` holder -> pending_approvers = ["position:cfo"]
[PROBE B2] same-org `cfo` holder present -> pending_approvers = ["u_pos_a"]

Why it is not obviously the same bug as #10153

sys_team carries an organization_id column (packages/platform-objects/src/identity/sys-team.object.ts), so a team_id transitively names exactly one organization — unlike sys_user.manager_id, which names a row in a global identity table with no tenancy fact on it. An author who picks a team has, in principle, already picked an organization.

What is unscreened is the case where the authored team value points at another organization's team: nothing re-verifies that the team belongs to the request's organization, so the slate silently crosses the boundary.

packages/spec/src/automation/approval.zod.ts also declares both types unscoped in the same table:

team: false,manager: false,

That flag answers "does the ADR-0105 D9 organization: declaration apply to this type" (retargetability), which is not the same question as "is the expansion screened to the request's organization" — but it does mean the two unscreened branches sit together deliberately, and a fix for one should say what it implies for the other.

Related

Filed unassigned for triage.


Generated by Claude Code

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions