Skip to content

[finding] Three spellings of the ADR-0068 platform-admin read now live in plugin-auth, and one of them skips the system read context #10348

Description

@os-warren

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_setadmin_full_access / organization_id = null lookup — was never consolidated, and now has three spellings in one package:

#SiteRead path
1auth-manager.ts — the isPlatformAdmin() closure inside customSessiondataEngine.find(...)directly
2auth-manager.tsisOrgOrPlatformAdmin()'s first halfwithSystemReadContext(engine).find(...)
3auth-manager.tsisPlatformAdminUserId() (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 withoutwithSystemReadContext, 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.

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions