docs(plans): accept granular RBAC plan#282
Open
mcharles-square wants to merge 3 commits into
Open
Conversation
Adds the technical design for moving Proto Fleet from the current SUPER_ADMIN/ADMIN/RequireAdmin gate to a granular permission model with scoped role assignments, editable built-in roles, and per-org custom roles. Key decisions baked in: - Three seeded roles: SUPER_ADMIN (immutable), ADMIN and FIELD_TECH (editable by SUPER_ADMIN, not deletable) - Permission catalog plus read-pairing rule (action perms require matching read perms; enforced at role save) - Site-scope narrowing semantics (site-scoped assignment overrides org-scoped grant at that site; org grant still applies elsewhere) - Filter-don't-reject list semantics for site-scoped readers - Route-guard parity: every primary-nav entry has a permission predicate; nav hides AND route redirects when false - Additive-only startup reconciliation for ADMIN/FIELD_TECH so operator edits survive upgrades; SUPER_ADMIN fully reconciled - Two error codes: BUILTIN_ROLE_IMMUTABLE (SUPER_ADMIN only) and BUILTIN_ROLE_NON_DELETABLE (ADMIN/FIELD_TECH delete attempts)
github-actions
Bot
added
the
documentation
🔐 Codex Security Review
Review SummaryOverall Risk: HIGH Findings[HIGH] Assignment Schema Does Not Enforce Org Ownership for Roles or Targets
[HIGH] Resolver “First Match” Algorithm Can Bypass Site-Scoped Narrowing
[HIGH] Planned U5 Migration Breaks the Staged Rollout
[MEDIUM] Built-In Role UI Contradicts Editable ADMIN/FIELD_TECH Design
NotesThis diff only adds an accepted planning document; no runtime code, migrations, generated protobufs, or frontend implementation changed yet. I did not find hardcoded pool, wallet, payout, or stratum addresses in the reviewed diff. Generated by Codex Security Review | |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an accepted technical design plan for evolving Proto Fleet authorization from the current coarse SUPER_ADMIN/ADMIN gate (RequireAdmin) to a granular, scoped permission model with role assignments, seeded built-in roles, and custom roles.
Changes:
- Introduces a comprehensive RBAC design document covering schema, resolver/middleware flow, permission catalog, and rollout sequencing.
- Specifies key security behaviors (fail-closed enforcement, cross-org IDOR checks, privilege-parity rules) and test strategy (unit + contract + E2E).
Comment on lines
+218
to
+219
| | `fleet:read` | Required for any list/dashboard view (miner list, telemetry). Implicit floor — a role with any action permission must also hold `fleet:read`; enforced at role save (see U8). | | ||
| | `miner:read` | Per-row detail view, status snapshot, error history. Required alongside any `miner:*` action permission. | |
Comment on lines
+241
to
+245
| **Read pairing rule (enforced at role save).** Every action permission has | ||
| a corresponding read permission that must be present in the same role: | ||
| - any `miner:*` action requires `miner:read` and `fleet:read` | ||
| - `rack:manage` requires `rack:read` and `fleet:read` | ||
| - `site:manage` requires `site:read` and `fleet:read` |
Comment on lines
+1331
to
+1334
| - **Built-in roles read-only:** the modal still opens (admins want | ||
| to inspect the permission set), but the form is disabled and a | ||
| full-width info banner at the top reads "Built-in role — permissions | ||
| are managed by the application." Save button replaced with Close. |
7 tasks
The original plan made built-in roles globally unique and editable. Codex security review correctly flagged this as a cross-tenant authorization bleed: a SUPER_ADMIN editing ADMIN in org A would affect users in org B that hold ADMIN. Fixed by making EVERY role per-org. Updates: - Summary: roles are now per-org; editing one org's ADMIN cannot leak into another org's ADMIN. - Tech design: role table carries organization_id; uq_role_name (from 000002) is dropped and replaced with two partial unique indexes — built-ins unique per (org, builtin_key), customs unique per (org, name). chk_role_builtin_key_matches_flag enforces the is_builtin ↔ builtin_key invariant at the DB layer. - U2 schema example: full DDL updated to show the per-org schema and the partial unique indexes for both role and user_organization_role (replacing the broken NULL-distinct UNIQUE constraint). - U3 sqlc: every role query is org-aware; UpsertBuiltinRoleForOrg targets the partial index. SoftDeleteCustomRole gates on is_builtin=FALSE. - U4 reconciliation: rewritten to loop over active orgs and call per-org SeedOrgBuiltins. Onboarding wiring documented — CreateAdminUserWithOrganization seeds per-org built-ins inside its existing tx so a new org's founding user can be assigned a per-org SUPER_ADMIN immediately, no boot required. - Per-org test scenarios: explicit isolation test (editing org A's ADMIN does not affect org B's ADMIN); onboarding seeds per-org built-ins. - Built-in editability section reframed: ADMIN and FIELD_TECH are editable per-org by a SUPER_ADMIN of that org; SUPER_ADMIN remains immutable per-org (full reconcile every boot).
Codex security review (round 2) correctly flagged that the original "additive-only" reconciliation behavior was a bug: re-asserting the seed set on every boot silently restored permissions an operator had deliberately revoked (e.g., miner:update_pools, miner:firmware_update). Updated the plan to reflect the corrected semantics: - ADMIN and FIELD_TECH per-org rows are seeded on first creation only. After that, reconciliation never touches role_permission — the operator owns the role. - Catalog growth does NOT auto-propagate to existing orgs' ADMIN or FIELD_TECH. SUPER_ADMIN remains fully reconciled. - Test scenarios updated to assert the corrected behavior: sensitive permissions stay revoked across restarts; new catalog keys never silently appear in existing built-in rows. - Risks section updated to note that startup reconciliation is no longer the safety net for ADMIN/FIELD_TECH; the safety net is the privilege-parity rule plus the always-available SUPER_ADMIN re-edit path. - Decisions section: built-ins reconciliation reframed from "additive-only" (misleading) to "seed-on-create" (accurate).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the technical design for moving Proto Fleet from the current
SUPER_ADMIN/ADMIN/RequireAdmingate to a granular permission model with scoped role assignments, editable built-in roles, and per-org custom roles. Status:accepted— implementation will follow in subsequent PRs per the plan's own PR sequencing.Key decisions baked in
SUPER_ADMINis immutable;ADMINandFIELD_TECHare editable by SUPER_ADMIN but cannot be deleted. Two distinct error codes (BUILTIN_ROLE_IMMUTABLEvsBUILTIN_ROLE_NON_DELETABLE) so the UI can render the right copy.miner:reboot,rack:manage,fleet:read, etc.) with a read-pairing rule enforced at role save — any action permission requires its matching read partner.fleet:readgets a filtered list, not a 403.{user:manage, role:manage, site:manage, apikey:manage, serverlog:read}defines Settings visibility. A<NoAccess>terminal page handles users with no available primary nav.ADMIN/FIELD_TECHso operator edits survive upgrades;SUPER_ADMINis fully reconciled toAllPermissions()on every boot.rack:read/rack:manageso field techs can organize the physical layout at their assigned sites.Out of scope (deferred follow-ups)
Implementation PR sequence
The plan calls out four PRs: foundation (schema + sqlc + proto + seed + backfill), security-critical (resolver + middleware + handler swap), role-management surface (RPC + admin UI), and E2E + contract tests. Each is independently shippable — the existing
RequireAdminkeeps working until the security-critical PR lands the swap.Test plan
lefthookpre-commit hooks (markdown only).