Repository navigation
tech-debt(auth): platform_admin is a role literal that nothing mints; the live signal is a boolean #14938
Description
Found while landing the canonical role vocabulary in #14936. Preserved and documented there rather than fixed, because either resolution changes behaviour.
Finding
platform_admin is used as a role string at three production sites (now consolidated onto one CROSS_ORG_KB_ROLES constant in #14936):
llc/kb/write_guard.py— bypasses the ancestor-KB write guardapi/knowledge.py— the same bypass at the KB add-text pathapi/knowledge.py— permits targeting another org's KB config
No code path anywhere mints it. Searched the full tree: no JWT is issued with role="platform_admin", no migration seeds it, no service assigns it, no UI offers it, and no test fixture produces it outside the lint guard's own literal list.
The platform-level signal the codebase actually uses is a boolean, not a role: users.is_platform_admin (column) and TenantContext.is_platform_admin (request context), consumed by llc/deps.py and the tenancy guards.
So there are two spellings of one concept, and only one of them is ever set.
Why it was not resolved in #14936
Both directions are behaviour changes and neither is obviously right:
- Retire the literal — but if any deployment sets
role="platform_admin"through security config, removing it silently revokes that account's cross-org KB access. Removing code because no producer is visible in the tree is the failure mode this repo has been bitten by before. - Wire it — make the boolean and the role string agree, so
is_platform_adminalso satisfies the KB guards. That is an authorization widening and needs a security review; fix(auth): establish the canonical role vocabulary (#13854, #12786, #14024, #13957) #14936 deliberately shipped no widening.
Note the member set is a deliberate fork and must not be folded into ADMIN_ROLES:
ADMIN_ROLES : admin, superadmin
CROSS_ORG_KB_ROLES : platform_admin, superadmin
admin is absent on purpose — an org admin must not reach a parent org's KB namespace, which is the entire property assert_not_writing_to_ancestor_kb exists to enforce. Unioning the two sets would add admin and delete that property.
Ask
Decide which spelling is canonical:
- (a)
is_platform_adminboolean is the only platform signal; the role literal is retired after confirming no deployment sets it (needs a host check, not a code check). - (b)
platform_adminbecomes a real assignable role with an explicitROLE_PERMISSIONSentry, and the boolean is derived from it. - (c) Keep both, documented, with the guards reading one shared helper — the interim state fix(auth): establish the canonical role vocabulary (#13854, #12786, #14024, #13957) #14936 leaves in place.
Related
- fix(auth): establish the canonical role vocabulary (#13854, #12786, #14024, #13957) #14936 (found it, consolidated the three copies), tech-debt(auth): "superadmin" is used at 17 require_role call sites but is not a member of the canonical Role enum #12786 (asked whether
platform_adminis first-class), security(rbac): superadmin is administrative but is not a Role member and holds no permissions #13854 (the canonical vocabulary)
Activity
mrveiss commentedon Sep 5, 2026
Needs your decision. Both resolutions change behaviour, which is why #14936 preserved it rather than picking one. Options and a recommendation, so it can be settled rather than re-derived.
The state, restated
platform_adminis read as a role string at three sites (now oneCROSS_ORG_KB_ROLESconstant) and nothing mints it — no JWT, no migration, no service, no UI, no fixture outside the lint guard's own literal list. The signal the codebase actually uses is a boolean:users.is_platform_adminandTenantContext.is_platform_admin.So the three checks are dead branches today. That cuts two ways, and which way decides the fix.
Option A — retire the role literal, use the boolean (recommended)
Replace the three role-string checks with the
is_platform_adminsignal the rest of the tenancy layer already consumes.platform_adminin a JWT outside this repo — an external IdP mapping, a hand-issued token — those callers lose the KB bypass at the moment of the change. Nothing in the tree does this, but the tree cannot see an external issuer.Option B — keep the role, and mint it
Make something actually issue
platform_admin, so the checks become live.Option C — delete the checks entirely
Recommendation
Option A. The boolean is the populated signal, the role literal is not, and keeping a check that nothing can satisfy is indistinguishable from having no check — with the added cost that it reads like protection. The external-issuer risk is real but bounded and testable: a deployment relying on it would see the bypass stop working, which is loud rather than silent.
If you want A, the safe order is: assert the boolean grants everything the role string did, land that, then remove the literal in a follow-up — so the two are never both off at once.
Labelled
needs-decision. Nothing changed in the meantime.