notifications (3/5): notification:read and notification:manage permissions

[illegalprime]

Summary

This PR makes the Notifications surface (channels, alert rules, maintenance windows, delivery history) permission-gated. It introduces a new notification authorization resource with two permissions — notification:read (view) and notification:manage (mutate) — wires every Notifications RPC to the permission it requires, and seeds the permission rows while granting them to existing ADMIN roles. No Notifications handlers are mounted yet (that lands in the next stack PR); this PR establishes the authorization policy those handlers will enforce.

It is PR 3 of a 5-PR stack: #452 (proto + generated) → #453 (history-listing DB) → #454 (this PR, authz) → #455 (service + handler mounts + wiring) → #456 (client UI).

How it works

There are two independent paths: the request-time enforcement policy (in code) and the permission data in the database (rows + role grants).

Request-time enforcement. Every authenticated request flows through an auth interceptor that resolves the caller's EffectivePermissions (the union of permissions granted by the caller's roles) and stashes it on the request context. A handler gates itself by calling RequirePermission(key, scope), which fails closed: missing context value → Internal; no session → Unauthenticated; caller lacks the key → PermissionDenied. The ProcedurePermissions map is the central policy table that says which catalog key each gated procedure needs. This PR adds the Notifications entries to that map: all list/read procedures → notification:read; every mutation → notification:manage. TestChannel is classified as a mutation (:manage) because it triggers a real outbound delivery, not just a read.

Permission data reaching the database. Two mechanisms populate the permission and role_permission tables:

1. Boot reconciler (authz.Reconcile, runs once per server start after migrations): upserts every catalog permission row (refreshing description text), and seeds built-in roles. Crucially, the reconciler seeds ADMIN/FIELD_TECH role grants only when the role row is first created — it never re-asserts the seed set onto an already-existing role, so operators' revocations are preserved. That means new catalog keys do NOT auto-propagate onto already-seeded ADMIN roles.
2. Seed migration (000088_seed_notification_permissions): does the one-time job the reconciler deliberately won't do — backfill the two new permissions onto existing ADMIN roles. It also inserts the permission rows itself (ON CONFLICT makes this redundant-but-safe alongside the reconciler), scopes the role grant to builtin_key = 'ADMIN' so custom roles are untouched, and uses ON CONFLICT DO NOTHING so it is replay-safe.

The migration is numbered 000088 so it sorts above the keyset-index migration 000087 from #453 (which merges first), keeping golang-migrate's numeric order aligned with merge order.

Diagrams

Request authorization flow:

flowchart TD
    client["Client RPC call"] --> interceptor["Auth interceptor"]
    interceptor -->|"resolves caller roles -> EffectivePermissions"| ctx["Request context (stashed permissions)"]
    ctx --> handler["Notifications handler"]
    handler -->|"RequirePermission(key, scope)"| gate{"Caller has key?"}
    policy["ProcedurePermissions map (read -> notification:read, mutations -> notification:manage)"] -.->|"supplies required key"| gate
    gate -->|"yes"| allow["Handler executes (proxies to Grafana)"]
    gate -->|"no"| deny["PermissionDenied"]
    gate -->|"no session"| unauth["Unauthenticated"]
    gate -->|"missing ctx value"| internal["Internal (fail-closed)"]

Loading

Two ways the permissions reach the database:

flowchart LR
    catalog["catalog.go (in-code permission definitions)"]
    subgraph boot["Every server boot"]
        reconcile["authz.Reconcile"]
    end
    subgraph migrate["One-time migration 000088"]
        seed["seed_notification_permissions.up.sql"]
    end
    permTable[("permission rows")]
    rpTable[("role_permission grants")]

    catalog --> reconcile
    reconcile -->|"upsert all catalog rows"| permTable
    reconcile -->|"seed grants ONLY on first role creation"| rpTable
    seed -->|"insert permission rows (ON CONFLICT safe)"| permTable
    seed -->|"backfill grants onto EXISTING ADMIN roles"| rpTable

Loading

Lifecycle of a notification:* permission on an existing-DB upgrade:

stateDiagram-v2
    [*] --> CatalogDefined: "added to catalog.go"
    CatalogDefined --> RowUpserted: "reconciler upserts permission row on boot"
    RowUpserted --> GrantedToAdmin: "migration 000088 backfills existing ADMIN roles"
    CatalogDefined --> GrantedToAdmin: "migration also inserts rows if reconciler has not run"
    GrantedToAdmin --> [*]: "ADMIN callers now pass RequirePermission"
    note right of RowUpserted
      Reconciler will NOT grant new keys
      onto already-existing roles, hence
      the migration does the backfill.
    end note

Loading

Areas of the code involved

Area / package / file	What changed	Why it matters for review
server/internal/domain/authz/catalog.go	New notification resource constant; notification:read / notification:manage permission constants; two CatalogEntry rows.	Source of truth for the permission catalog. Confirm the resource is placed in declaration order (the order is contract-tested) and the descriptions match the migration text.
server/internal/handlers/middleware/rpc_permissions.go	Imports notificationsv1connect; adds 13 ProcedurePermissions entries (Channel/Rule/MaintenanceWindow/History services) mapping reads → :read, mutations → :manage.	The actual enforcement policy. Verify each mutation is :manage, each read is :read, and that TestChannel is :manage (outbound delivery).
server/migrations/000088_seed_notification_permissions.up.sql	New. Inserts the two permission rows (ON CONFLICT upsert) and backfills them onto existing ADMIN roles (ON CONFLICT DO NOTHING).	The one-time grant onto existing ADMIN roles. Verify scoping to builtin_key = 'ADMIN', deleted_at IS NULL, and replay-safety.
server/migrations/000088_seed_notification_permissions.down.sql	New. Deletes the role grants then the permission rows.	Dev-only rollback. Confirm ordering (grants before rows, FK safety).
server/internal/domain/authz/catalog_test.go	Adds the two new permissions and the notification resource to the completeness / grouping / ordering contract fixtures.	Generated/derived expectations — quick check that fixtures match catalog.go, not deep review.

No DB table for permissions-on-channels exists or is added: channels, rules, and maintenance windows live in Grafana, not in this database.

Key technical decisions & trade-offs

• TestChannel gated on :manage, not :read — chosen because it triggers an outbound delivery (a side-effecting action), over treating "test" as a read-only diagnostic.
• Migration backfills existing ADMIN roles, even though the reconciler upserts rows — chosen because the reconciler is intentionally additive only on first role creation and won't grant new keys to already-seeded roles; without the migration, existing orgs' admins would silently lack the new permissions.
• Permission rows inserted in BOTH the reconciler and the migration — chosen (with ON CONFLICT) over relying solely on the reconciler, so the migration is self-contained and the grant step has its FK target present regardless of boot ordering.
• Numbered 000088 (above notifications (2/5): notification_history keyset listing #453's 000087) — chosen to keep golang-migrate numeric order aligned with stack merge order, over a lower number that an already-upgraded DB would skip.
• Grant scoped to builtin_key = 'ADMIN' with DO NOTHING — chosen over a broad grant so custom/operator-tuned roles are untouched and re-runs are idempotent.
• Notifications services intentionally absent from the registeredServices RPC-contract fixture — those entries land in notifications (4/5): Grafana-backed service, handlers, and server wiring #455 alongside the real main.go handler mounts, because the contract test compares registeredServices against the live mux; the procedure→permission policy stays here.

Testing & validation

• Catalog contract tests (catalog_test.go): assert the new permissions and resource appear in the completeness set, are grouped under notification, and respect declaration order.
• RPC contract tests (rpc_permissions_test.go): every key referenced by ProcedurePermissions must exist in the catalog, and every registered procedure must be classified in exactly one bucket.
• Reconciler backfill tests (backfill_test.go) exercise the additive-on-first-creation policy that motivates the migration.

Explicitly NOT covered here:

• End-to-end handler enforcement — no Notifications handlers are mounted in main.go yet (that is notifications (4/5): Grafana-backed service, handlers, and server wiring #455), so there is no integration test exercising a real gated Notifications request in this PR.
• Migration apply/rollback against a live DB — relies on the standard migration harness; the down migration is dev-only and assumes no custom-role hand-grants.
• Per-scope authorization — these permissions are org-level; no site/resource-scoped checks are introduced.

🤖 Generated with Claude Code

[github-actions]

🔐 Codex Security Review

Note: This is an automated security-focused code review generated by Codex.
It should be used as a supplementary check alongside human review.
False positives are possible - use your judgment.

Scope summary

• Reviewed pull request diff only (6f51c39065b22e590ba4fc5effa6f3b0efcea698...9548ca8f40f713377078dac46c302446ac2a299d, exact PR three-dot diff)
• Model: gpt-5.5

💡 Click "edited" above to see previous reviews for this PR.

---

Review Summary

Overall Risk: MEDIUM

Findings

[MEDIUM] Notification permissions are hidden and can be stripped by the role editor

• Category: Frontend
• Location: server/internal/domain/authz/catalog.go:170
• Description: This diff adds notification as a new permission resource returned by the server catalog, but the client role editor groups permissions through a hardcoded RESOURCE_TO_GROUP / GROUP_ORDER list that does not include notification. buildPermissionGroups() therefore drops notification:read and notification:manage from the rendered catalog. CreateEditRoleModal also initializes editable permissions by filtering role permissions through visible keys, so any existing custom role with these permissions can lose them on an unrelated edit/save.
• Impact: Operators cannot grant the new notification permissions to custom roles through the UI, and existing notification grants can be silently removed from custom roles, causing unexpected authorization failures for notification features.
• Recommendation: Add notification to the client permission grouping/order, with tests that a catalog containing notification:read and notification:manage renders both. Consider failing closed or surfacing unknown resources rather than silently dropping server catalog entries.

Notes

No cryptostealing/pool hijack, command execution, network discovery, plugin, protobuf wire-format, or infrastructure changes were present in the scoped diff.

The notification Connect services are not mounted in cmd/fleetd/main.go at this commit, so the new ProcedurePermissions entries are not live RPC gates yet. When those services are mounted, the secret-bearing channel RPCs should also be added to the request-body redaction/sensitive logging policy.

---

_{Generated by Codex Security Review |
Triggered by: @illegalprime |
Review workflow run}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 507a4e593d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[illegalprime]

Codex finding [MEDIUM] services in RBAC contract but not mounted is a split artifact: this PR adds the permission map + registeredServices entries, and #455 mounts the four notificationsv1connect handlers in main.go. The contract and the mount land in adjacent PRs in the stack; nothing is missing once #455 merges. No change needed here.

[illegalprime]

Re the re-review:

• [MEDIUM] out-of-order migration won't backfill upgraded DBs — flagging as a merge-timing note, not a present bug. main's current max migration is 000086; this stack's 000087/000088 are the next sequential versions, so they apply cleanly on any main-based DB. Also, the permission rows are upserted at every startup by authz.Reconcile (main.go:204 → upsertCatalog), so they're never at risk. Migration 087's unique job is the one-time grant of notification:* onto existing ADMIN roles (the reconciler is additive by design and won't re-assert onto already-seeded roles). That backfill is only skipped if 087 ends up numbered below an already-applied version — i.e. only if other migrations land on main numbered ≥087 before this stack merges. Merge-time action: confirm 087/088 are still above main's max at merge; renumber if not. Renumbering now would be premature.
• [MEDIUM] services registered but not mounted — unchanged disposition: split artifact, notifications (4/5): Grafana-backed service, handlers, and server wiring #455 mounts the four handlers in main.go. No change here.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 66e55db8f5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[illegalprime]

codex-connector triage (both fixed):

• [P1] Renumber the seed after 000088 — fixed: seed is now 000088 (above the keyset index, now 000087 in notifications (2/5): notification_history keyset listing #453), so numeric order matches merge order and golang-migrate won't skip it.
• [P2] Drop unmounted services from the contract list — fixed: the registeredServices entries (which TestRPCContract_RegisteredServicesMatchMainMux compares against the main.go mux) moved to notifications (4/5): Grafana-backed service, handlers, and server wiring #455 where the handlers are mounted. The ProcedurePermissions policy map stays here. go test ./internal/handlers/middleware is green on both PRs.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d2685fc9dd

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[ankitgoswami]

approving based on the PR description