Found while implementing #9968 (impersonation half). Filed unassigned; no fix attempted — consolidating a security predicate is not that card's scope.
platform-admin-gate.ts's own header states the principle this violates:
Before this module the gate existed as four near-identical inline copies … N copies of an authorization predicate is the shape that drifts: the next mount is written by copying whichever copy the author happened to open. One exported judge, called by every mount, is the fix.
That consolidation covered the session-shaped question (isPlatformAdminUser(sessionUser) / judgePlatformAdmin(session)). The id-shaped question — "is this user id a platform admin?", i.e. the sys_user_permission_set → admin_full_access / organization_id = null lookup — was never consolidated, and now has three spellings in one package:
| # |
Site |
Read path |
| 1 |
auth-manager.ts — the isPlatformAdmin() closure inside customSession |
dataEngine.find(...) directly |
| 2 |
auth-manager.ts — isOrgOrPlatformAdmin()'s first half |
withSystemReadContext(engine).find(...) |
| 3 |
auth-manager.ts — isPlatformAdminUserId() (added by #9968) |
withSystemReadContext(engine).find(...) |
All three do the same two reads with the same limit: 50 and the same admin_full_access name match, and all three fail closed.
The one substantive difference is #1. It queries the engine without withSystemReadContext, while #2 and #3 both wrap. isOrgOrPlatformAdmin's docblock says why the wrapper is there:
Reads through withSystemReadContext so the lookups are not themselves RLS-scoped to the acting (possibly non-privileged) user.
Whether that matters for #1 depends on whether customSession's callback already runs in a privileged context. This finding does not claim #1 is a bug — that is the measurement someone should take before acting, and it is exactly the kind of difference that is invisible while three copies exist. If the wrapper is unnecessary in #1, the two wrapped copies are carrying a cost for nothing; if it is necessary, #1 is a session-derivation path that can under-report platform admin for a caller whose RLS scope hides the join rows.
resolve-authz-context.ts is named as authoritative for the platform-admin half by isOrgOrPlatformAdmin's docblock, which suggests a fourth spelling may exist outside this package — worth counting as part of the same sweep.
Suggested shape: one exported id-shaped judge next to isPlatformAdminUser in platform-admin-gate.ts, taking the data engine, with the system-read-context decision made once and written down; the three call sites then differ only in what they do with the answer. Note that isOrgOrPlatformAdmin must keep its narrower/wider distinction — it deliberately also admits org owners/admins, which a platform-admin route must not.
Found while implementing #9968 (impersonation half). Filed unassigned; no fix attempted — consolidating a security predicate is not that card's scope.
platform-admin-gate.ts's own header states the principle this violates:That consolidation covered the session-shaped question (
isPlatformAdminUser(sessionUser)/judgePlatformAdmin(session)). The id-shaped question — "is this user id a platform admin?", i.e. thesys_user_permission_set→admin_full_access/organization_id = nulllookup — was never consolidated, and now has three spellings in one package:auth-manager.ts— theisPlatformAdmin()closure insidecustomSessiondataEngine.find(...)directlyauth-manager.ts—isOrgOrPlatformAdmin()'s first halfwithSystemReadContext(engine).find(...)auth-manager.ts—isPlatformAdminUserId()(added by #9968)withSystemReadContext(engine).find(...)All three do the same two reads with the same
limit: 50and the sameadmin_full_accessname match, and all three fail closed.The one substantive difference is #1. It queries the engine without
withSystemReadContext, while #2 and #3 both wrap.isOrgOrPlatformAdmin's docblock says why the wrapper is there:Whether that matters for #1 depends on whether
customSession's callback already runs in a privileged context. This finding does not claim #1 is a bug — that is the measurement someone should take before acting, and it is exactly the kind of difference that is invisible while three copies exist. If the wrapper is unnecessary in #1, the two wrapped copies are carrying a cost for nothing; if it is necessary, #1 is a session-derivation path that can under-report platform admin for a caller whose RLS scope hides the join rows.resolve-authz-context.tsis named as authoritative for the platform-admin half byisOrgOrPlatformAdmin's docblock, which suggests a fourth spelling may exist outside this package — worth counting as part of the same sweep.Suggested shape: one exported id-shaped judge next to
isPlatformAdminUserinplatform-admin-gate.ts, taking the data engine, with the system-read-context decision made once and written down; the three call sites then differ only in what they do with the answer. Note thatisOrgOrPlatformAdminmust keep its narrower/wider distinction — it deliberately also admits org owners/admins, which a platform-admin route must not.