You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
approvals: sys_team_member carries no tenancy fact — a same-org team still routes to a member who holds no membership in the request's organization #10547
#10230 screens the team: sys_team carries organization_id, so a team approver pointing at another organization's team no longer expands into the slate.
It deliberately does not screen the team's members. sys_team_member carries only team_id and user_id:
So once a team passes the organization screen, every listed user_id enters pending_approvers unconditionally. Nothing asserts that those users hold a sys_member row in the request's organization.
Why this may still be a hole
The reachable shape: team_a is stamped organization_id: org_a and passes the screen, but one of its sys_team_member rows names a user whose only sys_member row is in org_b (a member removed from the organization but never removed from the team, or a team row written directly rather than through better-auth's add-team-member). That user receives approval authority over an org_a record.
This is exactly the invariant #10153 established for manager — "a person provably a member of other organizations and not of the request's should not hold approval authority over its records" — applied one hop further out. #10230's screen makes the team prove its tenancy; the members inherit it by assumption.
Why it is genuinely a separate decision, not an oversight
Two reasons the #10230 lane did not simply extend its screen:
It is a different assertion, and a wider read. Screening members means a sys_member read per team (or an $in over the expanded set) rather than one row, and it asserts something about people rather than about the routed object. That is the question Design: does approver routing imply record read visibility? (#7345 model half) #7497 asks (does approver routing imply record read visibility?), which is open.
Fail-open posture would blunt it anyway. Both existing screens (managerIsProvablyOutsideOrg, teamIsProvablyOutsideOrg) treat an absent tenancy fact as "leave routing alone". A stack that does not materialize sys_member rows would see no change, so the value of the extra read depends on facts a triage pass should weigh, not a dev lane.
Not measured
⚠️ This is a code reading, not a probe. No fixture was built for the "member removed from org, left on the team" shape, and it is possible that a deployment invariant elsewhere (better-auth's remove-member cascading into sys_team_member, or RLS on the team-member table) already makes it unreachable. Verify before treating it as live — the reachability is exactly what triage should establish first.
Found while implementing #10230 (
teamapprover organization screen). Filed separately rather than widened into that card, per its scope fence.What #10230 closed, and what it did not
#10230 screens the team:
sys_teamcarriesorganization_id, so ateamapprover pointing at another organization's team no longer expands into the slate.It deliberately does not screen the team's members.
sys_team_membercarries onlyteam_idanduser_id:So once a team passes the organization screen, every listed
user_identerspending_approversunconditionally. Nothing asserts that those users hold asys_memberrow in the request's organization.Why this may still be a hole
The reachable shape:
team_ais stampedorganization_id: org_aand passes the screen, but one of itssys_team_memberrows names a user whose onlysys_memberrow is inorg_b(a member removed from the organization but never removed from the team, or a team row written directly rather than through better-auth'sadd-team-member). That user receives approval authority over anorg_arecord.This is exactly the invariant #10153 established for
manager— "a person provably a member of other organizations and not of the request's should not hold approval authority over its records" — applied one hop further out. #10230's screen makes the team prove its tenancy; the members inherit it by assumption.Why it is genuinely a separate decision, not an oversight
Two reasons the #10230 lane did not simply extend its screen:
sys_memberread per team (or an$inover the expanded set) rather than one row, and it asserts something about people rather than about the routed object. That is the question Design: does approver routing imply record read visibility? (#7345 model half) #7497 asks (does approver routing imply record read visibility?), which is open.managerIsProvablyOutsideOrg,teamIsProvablyOutsideOrg) treat an absent tenancy fact as "leave routing alone". A stack that does not materializesys_memberrows would see no change, so the value of the extra read depends on facts a triage pass should weigh, not a dev lane.Not measured
remove-membercascading intosys_team_member, or RLS on the team-member table) already makes it unreachable. Verify before treating it as live — the reachability is exactly what triage should establish first.Related
teamapprover expansion is not organization-screened either — a cross-org team resolves into a request's approver slate #10230 — theteamapprover organization screen (this issue is its explicitly-deferred half)managerapprover resolvessys_user.manager_idwith no organization screen, while every sibling approver expansion is org-scoped #10153 — the same invariant for themanagerapproverFiled unassigned for triage.
Generated by Claude Code