feat(desktop): relay admin console for the /api/admin/v1 operator surface - #4768
feat(desktop): relay admin console for the /api/admin/v1 operator surface#4768wpfleger96 wants to merge 3 commits into
Conversation
42e67e3 to
9bb1666
Compare
…orcement states, feedback status, staffing tab Implement Plan v4 Phase 3 for the desktop admin console panel (#4768). ## Probe - AdminProbeResult::Nip98Authorized now carries optional role and source fields (Rust enum variant updated to struct variant). - AdminConsoleSettingsCard propagates role/source from probe result to AdminConsolePanel; AdminConsolePanel renders a role+source badge strip when role is present. ## Report actions (frozen v4 matrix) - Event reports: delete/kick/ban/timeout/dismiss/escalate - Pubkey reports: ban/timeout/dismiss/escalate - Blob reports: dismiss/escalate - ResolveReportForm generates a client UUID request_id per submission attempt (v4 §6a amendment 2); 409/processing errors preserve the request_id for retry idempotency. - Timeout action shows a duration (expiration_secs) input; submit is disabled until a value is provided. ## Enforcement states - processing reports are disabled (non-actionable) in the list with a spinner. - EnforcementStateBlock renders pending/enforcing/succeeded/failed states. - Failed actions surface Retry (reuses same request_id) and Cancel (dismiss with fresh request_id; server-rejected cancel treated as authoritative). ## Feedback status - FeedbackStatusControl: new/reviewed/archived PATCH buttons with optimistic local-state sync; server error surfaces inline. - FeedbackTab list shows non-new status as a badge. ## Staffing tab - Operator-only (gated by role === 'operator' from probe). - SourceBadge distinguishes config/owner_fallback (immutable) from db. - Config-backed operator rows have disabled remove buttons with title explaining why. - PUT 409 (config-backed add conflict) and DELETE 409 surfaced clearly. ## File structure AdminConsolePanel.tsx split into four files to satisfy the 1000-line ratchet (all new files under the limit): - AdminConsolePanelHelpers.tsx: AsyncState, useAsyncLoad, formatTimestamp, DetailRow, LoadingSpinner, ErrorMessage, AttachmentMeta, parseImetaAttachments - AdminConsoleFeedbackTab.tsx: FeedbackTab, FeedbackDetail, and related sub-components - AdminConsoleStaffingTab.tsx: StaffingTab, SourceBadge - AdminConsolePanel.tsx: ReportsTab, ReportDetail, report action components, TabBar, AdminConsolePanel root src-tauri/src/commands/admin/helpers.rs extracted from mod.rs to keep mod.rs under 1000 lines. ## Tests - 7 new tests: probe-role-source-badge, probe-moderator-role, probe-operator-role, probe-no-role, processing-report-not-actionable, action-matrix-types, plus reportButton.disabled assertion. - All 4511 TS tests pass; all 12 jsdom tests pass; Rust compiles clean; desktop-check, desktop-tauri-check, desktop-tauri-test all green. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
fa29ae1 to
99527b1
Compare
99527b1 to
6bd27a2
Compare
6bd27a2 to
fbb8b6c
Compare
…y-scoped nav gate Close the desktop half of Thufir's #4768 pass-1 findings that need no relay change. The relay-contract consumption (canonical action DTO, real cancel route) waits on #3777. Processing report rows were disabled in the list, but the enforcement progress/retry/cancel UI lives only inside the detail view — so the row was locked exactly when an operator needs to inspect a pending or failed action. Keep processing rows navigable; the detail view already suppresses the resolve form for any non-open report. Feedback triage `status` was optional on the wire types and silently defaulted to "new" when absent, misreporting a reviewed/archived entry as new after reload. Make `status` required on both feedback DTOs and read it directly, and type PATCH's actual `{status}` echo instead of claiming a full summary record. The Moderation nav resolver keyed its 60s cache on pubkey alone, but NIP-11 discovery is relay-dependent — a workspace switch could serve the previous relay's verdict. Key the resolver on the connected relay origin (and gate its `enabled` on a resolved origin), and defer the `?section=moderation` invalid-section redirect while the resolver is unresolved so a direct link is not bounced before the probe can authorize. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested
-
[P1] Do not auto-probe an unrestricted relay-advertised origin —
desktop/src-tauri/src/commands/admin/mod.rs:938-949admin_apiis explicitly untrusted NIP-11 data, but discovery applies the same permissive policy as manual operator input: HTTP loopback is accepted and every HTTPS host is accepted. Both the nav resolver and settings card then probe that value automatically without confirmation (hooks.ts:45-56,AdminConsoleSettingsCard.tsx:303-316). A malicious connected relay can therefore make the native client contacthttp://localhost:<port>(the client pinslocalhostto127.0.0.1) or HTTPS private/link-local targets. If the target returns401 WWW-Authenticate: Nostr, the app sends it a fresh app-key signature, adding a signing oracle and pubkey-ownership disclosure to the SSRF. Separate advertised-origin validation from manual configuration: do not auto-probe loopback/private/link-local advertised destinations, and account for DNS resolution/rebinding, or require explicit operator confirmation before any cross-origin probe. Keep the loopback HTTP carve-out only for manual origins. -
[P2] Actually make disabled-auth mode read-only —
desktop/src/features/admin-console/AdminConsoleSettingsCard.tsx:397-480disabledcurrently renders the sameAdminConsolePanelas an authorized principal, with only the staffing tab hidden. The report resolve/reopen/cancel controls and feedback status controls remain enabled, although the related relay contract intentionally rejects every mutation in disabled mode with 403. This contradicts the PR’s stated read-only behavior and presents controls guaranteed to fail. Pass a read-only capability into the panel and suppress/disable all mutation affordances when the probe result isdisabled.
The command registration/API seams, identity and async fencing, report lifecycle/idempotency paths, attachment cleanup, navigation, staffing gating, and legacy-surface removal were otherwise coherent in this read-only diff review. GitHub currently exposes only a passing DCO check for this head; PR code was not executed locally under the automation trust policy.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Two security/resource-boundary blockers remain on this head:
-
Do not auto-probe a relay-advertised admin origin under the manual-origin trust policy. The NIP-11
admin_apivalue is explicitly untrusted, butadmin_origin_from_nip11passes it through the sameAdminOrigin::parsepolicy used for operator-entered values. That policy accepts loopback HTTP and unrestricted HTTPS hosts. Both the navigation resolver and the settings card then automatically probe the advertised origin, and the probe performs a native GET before sending a fresh NIP-98 signature when the target replies with401 WWW-Authenticate: Nostr. A malicious connected relay can therefore advertisehttp://127.0.0.1:<port>/localhost(the client explicitly pinslocalhostto127.0.0.1) or an HTTPS private/link-local target and trigger zero-click native SSRF; a cooperating target also gets an attacker-triggered signing oracle and proof of the app pubkey. This is not hypothetical policy drift:mod_tests.rscurrently asserts that a loopback origin from NIP-11 is accepted and returned for automatic probing. Please separate advertised-origin validation from explicit manual configuration: never auto-probe loopback/private/link-local advertised destinations, and account for DNS rebinding, or require an explicit operator confirmation before any cross-origin probe. Keep loopback HTTP only for a manually entered origin. -
Bound attachment preview work across the whole feedback item, not only per response.
parseImetaAttachmentsaccepts and returns every validimetatag,FeedbackDetailmounts anAttachmentViewerfor every result, and eachimage/*viewer immediately starts its own native fetch. The Rust command caps each response at 10 MiB, but there is no attachment-count, aggregate-byte, or concurrency bound. The relay’s feedback ingest bounds serialized tags to 64 KiB but does not cap the number ofimetaentries, so an admitted feedback event can fan out many simultaneous image reads and retain their blob URLs until navigation. The existing test explicitly blesses multiple tags but covers no aggregate/concurrency ceiling. Please impose a small count plus aggregate-byte budget before rendering/fetching, avoid unbounded parallel auto-load, and add a regression showing excess attachments are not requested.
The closed route construction, exact-body NIP-98 binding, no-redirect client, mutation idempotency/recovery fences, and identity/origin React state fences otherwise held in this read-only diff review. GitHub currently exposes only the passing DCO check for this head; I did not execute PR code under the automation policy.
Accidental duplicate from a concurrent automated recovery. The earlier marked review 5032499207 is authoritative.
…NIP-11 discovery (#3777) Adds authenticated, role-based moderation to the relay admin API. On `main` the admin API is read-only and gated only by `Host`/`Origin` matching; this branch adds NIP-98 authentication, a two-tier Operator/Moderator principal model, mutation and staffing endpoints, and NIP-11 auto-discovery so clients never type the admin URL by hand. ## Authentication (`BUZZ_ADMIN_AUTH`) `BUZZ_ADMIN_AUTH` accepts `nip98` or `disabled`. Leaving it unset defaults to `nip98` (fail-secure). Configuration fails closed: any other value aborts startup, while a lingering `BUZZ_ADMIN_TOKEN` is ignored with a startup warning — token (bearer) authentication is not supported. `Host`/`Origin` matching is retained in every mode as defense-in-depth. - **`nip98`** (default) — per-request signed NIP-98 (kind 27235) events, resolved to an Operator or Moderator principal with per-person attribution and individual revocability. Read-write per resolved principal. - **`disabled`** — no credential; relies entirely on network-layer controls (reverse proxy, VPN, firewall) and logs a `WARN` on every boot. Always read-only: `authorize()` resolves no principal, so mutation and staffing routes always `403`. ## Roles Buzz has two independent authority axes after this change. **Relay-level** roles (new here) are deployment-global: they act across every community on the relay, through the admin API. **Community-level** roles (pre-existing, unchanged by this PR) are tenant-scoped: they act inside one community, through signed Nostr moderation commands. ### Relay level (new) | Role | Description | |---|---| | **Operator** | Full control of the deployment's moderation surface: read all reports, feedback, and attachments across every community; resolve reports with enforcement (`delete`/`kick`/`ban`/`timeout`) or decisions (`dismiss`/`escalate`); reopen and cancel; update feedback status; and manage the Operator/Moderator roster via the staffing endpoints. | | **Moderator** | Day-to-day triage: everything an Operator can do except staffing — cannot view or change the roster. | How a pubkey acquires a relay role (resolution order; config always outranks DB): 1. Listed in `RELAY_OPERATOR_PUBKEYS` → **Operator** (source `config`) 2. Equals `RELAY_OWNER_PUBKEY` while `RELAY_OPERATOR_PUBKEYS` is empty → **Operator** (source `owner_fallback`, a break-glass grant for self-hosters that deactivates once any operator is configured) 3. Row in the `relay_operators` table → **Operator** or **Moderator** (source `db`, managed via the staffing endpoints) 4. No match → `403` ### Community level (pre-existing, unchanged) | Role | Description | |---|---| | **Owner** (community) | Full authority within their community: every moderation action (delete, kick, ban/unban, timeout/untimeout, resolve reports, view queue) plus member, role, and invite management. No guard rails. | | **Admin** (community) | Same community-wide moderation capabilities as owner, except an admin cannot ban or time out the owner or a fellow admin — only the owner may action an admin. Manages members and invites; only the owner grants the admin role. | | **Member** (community) | Standard participant; no moderation capability. | | **Owner / Admin** (channel) | Channel-local authority only: delete messages and kick users within their own channel. | | **Member / Guest / Bot** (channel) | No moderation authority. | There is no community-level Moderator tier in v1; relay-level Moderator is the only role by that name. ## Escalation scoping The operator report queue is an escalation backstop, not the community's day-to-day triage surface (per `VISION_MODERATION`, the severe class is the platform's to review rather than the community's). Two rules enforce that: - **Escalated-by-default listing.** `GET /reports` with no `status` parameter returns only `escalated` reports. An explicit `status=<open|resolved|dismissed|escalated>` filter is always honored as given, and full visibility across every status stays available for platform-safety and legal review via `scope=all` (which lists reports regardless of status). `scope` accepts only `all` and is ignored when an explicit `status` is present. - **Auto-escalated `illegal` reports.** Member reports whose category is `illegal` are ingested with `status=escalated` rather than `open`, so the severe class reaches the operator backstop without waiting for a community admin to forward it. Every other category still lands `open`. Auto-escalation only sets the queue status — it records no moderator decision and stamps no resolver, so an auto-escalated report is indistinguishable downstream from an admin-escalated one: the reopen route returns it to `open` on the same terms, keyed only on status, never on how the report became escalated. ## Principal resolution and NIP-98 admission `resolve_admin_principal()` returns `AdminPrincipal { pubkey, role, source }` per the resolution order above; `None` never falls through as a role. Admission is ordered so the replay guard is a privilege, not a public surface: signature/URL/method/payload-hash verification first, roster check second, and only then is the deployment-scoped replay id atomically consumed — a validly-signing but unrostered key never allocates a replay slot. Redis failure fails closed. ## Report resolution, recovery, and enforcement provenance `POST /reports/{id}/resolve` is a crash-safe enforcement state machine: decision-only outcomes (`dismiss`/`escalate`) are a single CAS-plus-audit transaction; enforcement (`delete`/`kick`/`ban`/`timeout`) claims the report (`open`→`processing`), runs the durable mutation, then finalizes — a re-drive resumes at the step marker and converges to exactly-one enforcement, fenced by a lease and an outbox claim token. Person-directed enforcement on an `event`-kind report derives its target from the stored event's author (server-owned truth, never the reporter's `p` tag) via a single `derive_enforcement_target` shared by the HTTP driver and the recovery worker. If the reported event was purged before its author could be read, person-directed actions are rejected pre-claim and the report stays `open`; `delete` needs only the event id and is exempt. `GET /reports/{id}` and the resolve response carry an `activeAction` field surfacing the enforcement that actually executed — a report dismissed after a reopen still reports the ban that ran. `POST /reports/{id}/reopen` returns a terminal report to `open` (idempotent on `requestId`). `POST /reports/{id}/cancel` is the sole recovery path for a pre-mutation `failed` action, attributed via `relay_admin_actions.cancelled_by`. ## Feedback `GET /feedback` and `/feedback/{id}` survive a tenant purge: provenance columns are severed to `NULL` rather than cascade-deleted, and the attachment path fails closed to `404` on a severed row. `PATCH /feedback/{id}` updates lifecycle `status` (`new`/`reviewed`/`archived`). ## Staffing and probe `GET/PUT/DELETE /operators/{pubkey}` are Operator-only; mutating a config-backed pubkey returns `409 Conflict`. `GET /operators` returns the union of config and DB principals with per-entry `source`. `GET /probe` reports auth mode, role, source, `canAct`, and `canStaff` for the desktop console. ## NIP-11 auto-discovery The NIP-11 relay-information document gains an optional `admin_api` field carrying the canonical admin origin (`scheme://host[:port]`, no path), present iff `BUZZ_ADMIN_HOST` is set and omitted otherwise. The scheme follows the same loopback rule as NIP-98 `u`-tag verification via a shared `scheme_for_host` helper, so the advertised origin and the origin the relay verifies against can never diverge. ## Operator API origin decoupling `RELAY_OPERATOR_API_ORIGIN` is no longer required at boot when `RELAY_OPERATOR_PUBKEYS` is set — it is used only by the community-provisioning endpoints, which fail closed at request time (with a boot-time `WARN`) until it is set. The admin console needs no origin. ## Admin-web adaptation The standalone `admin-web` dashboard signs each request as a NIP-98 event via a NIP-07 browser extension, discovers the auth mode with a single unauthenticated probe (`200` → `disabled`, anything else → `nip98`, fail-secure), and carries no token entry surface. Playwright coverage exercises the NIP-98 and CSP paths. ## Security hardening Three findings from security review are folded in: - **Append-only roster audit.** `PUT`/`DELETE /operators/{pubkey}` mutate the deployment-wide root of trust, but the upsert overwrites `role`/`added_by` in place and the delete removes the only row — so a grant→revoke sequence left no trace of who was ever granted or by whom. Each mutation now writes an `relay_operator_audit` row (actor, target, `grant`/`revoke`, pre-image `prev_role`, `new_role`, timestamp) inside the same transaction as the mutation. A per-target transaction-scoped advisory lock serializes concurrent mutations of the same pubkey before the pre-image read, so the recorded `prev_role` is always the true predecessor even under a concurrent-grant race. Chronology is keyed on a `BIGINT GENERATED ALWAYS AS IDENTITY` `seq` column, not the wall clock: the serializing lock guarantees insertion order and `seq` captures it, so ordered reads (`ORDER BY seq`) follow the true privilege chain even across a backward NTP step that a `clock_timestamp()` ordering would invert. `created_at` (`clock_timestamp()`) is retained as informational occurrence time only. Append-only by construction — no `UPDATE`/`DELETE` path and no API surface. A no-op delete writes nothing. - **`expirationSecs` overflow.** The timeout path built `Utc::now() + Duration::seconds(secs as i64)` from an attacker-controlled `u64`: `i64::MAX` panicked the handler, and a wrapped-negative magnitude minted a *past* expiry that still passed validation. `compute_timeout_until` now rejects zero, rejects magnitudes above a documented `MAX_TIMEOUT_SECS` (365 days), and uses checked `try_seconds`/`checked_add_signed` so no input can panic or produce a past expiry — over-cap, zero, `i64::MAX`, and wrapping-negative inputs all return a clean `4xx`. - **Uppercase-hex config-backed bypass.** Config pubkeys are lowercased at parse, but the `409` immutability check raw-string-compared the path param while `decode_hex_pubkey` accepted uppercase — so `PUT /operators/{UPPERCASE}` skipped the guard and wrote a shadow row for the same 32 bytes. The validated param is now canonicalized (lowercased) before the `409` check, DB write, `DELETE`, and response body. ## Migrations - `0035_relay_operators.sql` — `relay_operators` roster table (deployment-global), `actor_authority` on `moderation_actions`, `processing` status plus `active_action_id` on `moderation_reports`, `status` on `product_feedback`. - `0036_relay_admin_actions.sql` — enforcement-action table with a `request_id` idempotency key, a `step_marker` for crash recovery, and a `cancelled_by` attribution column. - `0037_relay_admin_action_lease.sql` — lease fencing for the action worker. - `0038_relay_admin_outbox_claim_token.sql` — fenced claim token on the outbox worker. - `0039_relay_operator_audit.sql` — append-only `relay_operator_audit` trail for roster mutations (see Security hardening). `docs/admin/README.md` documents the full principal model, NIP-98 event requirements, capabilities by role, the startup error matrix, and the discovery field. ## Production blast radius A relay without `BUZZ_ADMIN_HOST` is completely unaffected — the admin surface stays disabled and `BUZZ_ADMIN_AUTH` is ignored; a lingering `BUZZ_ADMIN_TOKEN` logs a startup warning and must be removed. Where `BUZZ_ADMIN_HOST` **is** set, unset `BUZZ_ADMIN_AUTH` defaults to `nip98` (per-person signed auth); `BUZZ_ADMIN_AUTH=disabled` reproduces `main`'s prior `Host`/`Origin`-only gating but is read-only (mutation routes `403`). The five migrations add tables and columns without touching existing data. --- Related: [#4768](#4768) (desktop admin console consuming the `admin_api` field), [squareup/bb-public#339](squareup/bb-public#339) (Phase 4 rollout config) --------- Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> Co-authored-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…y-scoped nav gate Close the desktop half of Thufir's #4768 pass-1 findings that need no relay change. The relay-contract consumption (canonical action DTO, real cancel route) waits on #3777. Processing report rows were disabled in the list, but the enforcement progress/retry/cancel UI lives only inside the detail view — so the row was locked exactly when an operator needs to inspect a pending or failed action. Keep processing rows navigable; the detail view already suppresses the resolve form for any non-open report. Feedback triage `status` was optional on the wire types and silently defaulted to "new" when absent, misreporting a reviewed/archived entry as new after reload. Make `status` required on both feedback DTOs and read it directly, and type PATCH's actual `{status}` echo instead of claiming a full summary record. The Moderation nav resolver keyed its 60s cache on pubkey alone, but NIP-11 discovery is relay-dependent — a workspace switch could serve the previous relay's verdict. Key the resolver on the connected relay origin (and gate its `enabled` on a resolved origin), and defer the `?section=moderation` invalid-section redirect while the resolver is unresolved so a direct link is not bounced before the probe can authorize. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
ff769d3 to
587d23d
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested
-
[P1] Do not automatically probe an unrestricted relay-advertised origin —
desktop/src-tauri/src/commands/admin/mod.rs:938-950admin_apiis untrusted NIP-11 data, but discovery runs it through the manual-entry policy, which accepts loopback HTTP and every HTTPS host (origin.rs:64-90). With no saved origin, both the settings surface and navigation resolver automatically probe the advertised value (AdminConsoleSettingsCard.tsx:284-316,hooks.ts:41-56). The native probe first GETs that attacker-selected destination and, if it answers401 WWW-Authenticate: Nostr, sends a fresh NIP-98 signature to it (mod.rs:160-215). A malicious or compromised connected relay can therefore target localhost, private/link-local addresses, or an attacker hostname that resolves there; no-redirect handling does not constrain this initial destination. Separate advertised-origin trust from explicit manual configuration: require an explicit confirmation or prove/bind the advertised endpoint to the connected relay, and reject resolved loopback/private/link-local/reserved targets with rebinding-safe connection handling. Add advertised-origin tests for loopback, RFC1918/link-local, and a public hostname resolving privately. The existing discovery test currently blesses automatic loopback probing (mod_tests.rs:872-916). -
[P1] Bound attachment fan-out across the feedback item —
desktop/src/features/admin-console/AdminConsolePanelHelpers.tsx:243-272parseImetaAttachmentsreturns every validimetatag,FeedbackDetailmounts every result (AdminConsoleFeedbackTab.tsx:420-465), and eachimage/*viewer immediately starts its own native fetch on mount (AdminConsoleFeedbackTab.tsx:215-224). The 10 MiB native cap is per response, with no attachment-count, aggregate-byte, or concurrency ceiling. A single admitted feedback event can therefore trigger many simultaneous native reads and retain all resulting blobs until navigation. Impose a small item-wide count/aggregate budget and bounded fetch concurrency (or explicit loading), then add a regression proving excess attachments are not requested. -
[P2] Keep disabled-auth mode read-only as documented —
desktop/src/features/admin-console/AdminConsoleSettingsCard.tsx:470-480A coherent
disabledprobe mounts the same panel as an authorized principal without passing a read-only capability. Report resolve/reopen/cancel and feedback status controls therefore remain live, although the relay rejects mutations in this mode. This contradicts the documented contract that disabled mode renders feedback status read-only (docs/admin/README.md:267-276) and the PR description. Pass the auth capability into the panel and hide or disable every mutation affordance while preserving reads. -
[P2] Expose the selected feedback status programmatically —
desktop/src/features/admin-console/AdminConsoleFeedbackTab.tsx:352-375The current status is communicated only through button styling (
variantand ring classes). The three ordinary buttons have noaria-pressed, radio semantics, or textual current-value output, so a screen-reader user cannot determine the active status before changing it. Add a programmatic selected state and assert that semantic contract rather than only the visual class.
The closed route construction, exact URL/body NIP-98 binding, no-redirect client, response caps, identity/origin async fences, report lifecycle and request-ID recovery contract, role-gated staffing, and legacy moderation-surface removal were otherwise coherent in this read-only exact-head review. Desktop, Rust, build, and desktop integration checks are green; PR code was not executed locally under the automation trust policy.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested at exact head 63db828a09b270239d5e56bf82c3f5e8468cea43 and exact base a3730784fc851bb1125b40cca9b0a30788a293c1.
P1: require confirmation before removing an operator
The Staffing trash button calls handleRemove directly, which immediately invokes deleteAdminOperator (desktop/src/features/admin-console/AdminConsoleStaffingTab.tsx:105-121,203-224). One stray click therefore deletes a DB-backed operator, potentially the acting operator, without a confirmation, target/role recap, or undo. That can remove administrative access and may require another operator or out-of-band configuration to recover.
Put this destructive authorization change behind an explicit confirmation dialog. Make self-removal especially clear, and add cancel/confirm behavior tests. The current event suite only checks whether staffing controls are rendered; it does not exercise removal confirmation.
P2: recover when the active Staffing tab becomes unauthorized
activeTab persists independently of role (AdminConsolePanel.tsx:832-843). If the same mounted panel changes from operator to moderator/unknown while Staffing is selected, the tab button disappears and the content is suppressed (AdminConsolePanel.tsx:859-887), leaving an empty panel with no selected visible tab. Reset an invalid active tab to Reports when role/capabilities change and cover that prop transition.
The previous advertised-origin SSRF, attachment fan-out, disabled-mode mutation, and feedback-status accessibility blockers are otherwise closed at this head. Relay discovery is now prefill-only until an operator explicitly saves the origin; item-wide attachment limits, read-only mutation gating, and aria-pressed are present. Hosted substantive checks are green. This was a read-only review of immutable GitHub source/diffs; no PR code was checked out or executed.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested: four P2 desktop integration defects.
Reviewed head 662db64518d72ba96a3e1726765baae71d203644 against exact base a3730784fc851bb1125b40cca9b0a30788a293c1.
The product boundary is a relay-operator/moderator console, distinct from community-owner administration. Discovery must remain untrusted until the operator confirms the origin; the relay remains authoritative for roles, report transitions and public enforcement notices. The four inline findings concern unreachable manual setup, non-escalated report access, undisclosed public-reason semantics, and retries that claim the wrong action succeeded. They do not require changing the relay's intentional escalated-only default or weakening discovery trust.
Coverage: Settings navigation/discovery and identity/origin transitions; native origin, route/auth, bounded-body/error plumbing; report list/detail/resolve/reopen/cancel and idempotency; feedback attachments/status; staffing confirmation/role changes. Compared client routes and closed action/status/role sets to exact-head relay producers. The previous staffing-confirmation, hidden-active-tab and unconditional request-ID-clearing defects are fixed in this head and are not repeated. Community signed-command enforcement and mobile/web UI are not changed by this desktop PR; they were not independently reviewed as whole features.
Validation was source/diff and existing-test inspection only under the read-only automation policy. Key source blobs were checked against the pinned Git tree. No checkout, build, test execution or live relay/app run; this is not a runtime pass.
a2fdea7 to
b522404
Compare
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Changes requested at head 0b972699c51d010ebdf349498fbe6cbc40468d16, base 4952f5810290d00a1dd9413235b39e9a7154acf6.
P2: Do not let “Add operator” silently demote an existing operator
AdminConsoleStaffingTab.tsx:97–105 sends an immediate role upsert, while the form says “Add operator” / “Add” and defaults to moderator (lines 81, 195, 229). With DB operators A and B, paste an existing operator key into this form and leave the default role: clicking Add changes that account to moderator without a role-change confirmation. If A submits their own key, A loses roster-management access and cannot reverse the change themselves. The new Remove confirmation and self-removal warning protect DELETE only, not this PUT path.
This is confirmed by the actual producer: crates/buzz-relay/src/api/admin/mod.rs:1037–1055 forwards the role to crates/buzz-db/src/store/relay_operators.rs:134–145, whose conflict branch updates the existing role. The last-operator guard at lines 169–174 deliberately permits the demotion when another operator exists. This is accidental individual access loss, not a claim that the deployment can lose its last operator.
Exit criterion: distinguish adding a new principal from modifying one. Reject an already-present key in Add, or explicitly present the existing→requested role change and confirm demotions, including a self-access warning. Keep relay authority and last-operator protection unchanged. Add a real form/IPC regression with two operator rows proving that an existing-key submission does not silently demote, while adding a new moderator still works. The existing event tests cover DELETE confirmation and synthetic role-prop changes, not this PUT submission. This was missed in earlier review rounds; it is not a regression introduced by the four latest fixes.
P2: Accept the relay’s supported .localhost manual origin
desktop/src-tauri/src/commands/admin/origin.rs:123–130 recognizes only bare localhost and loopback IPs. Consequently AdminOrigin::parse("http://admin.localhost:3000") takes the non-loopback HTTP rejection at lines 64–74, so an operator cannot save the repository’s local admin origin. This is an existing producer contract, not a proposed new deployment: Justfile:498 defaults BUZZ_ADMIN_HOST to admin.localhost:3000, and relay api/admin/auth.rs:109–155,534–538 deliberately advertises and verifies HTTP for every .localhost name. Substituting 127.0.0.1 is not equivalent: admin routing checks the configured Host exactly (auth.rs:99–106).
Exit criterion: support explicit manual entry of the relay’s loopback-name contract, preserving the configured hostname for Host/NIP-98 signing and ensuring it connects as loopback. Add a regression for the real http://admin.localhost:3000 value. Do not relax the separate prohibition on automatically probing untrusted advertised internal origins. This is a missed compatibility path, not a regression in the four latest fixes.
Non-blocking P3: Preserve status in read-only feedback detail
AdminConsoleFeedbackTab.tsx:497–507 hides the only detail status representation when canMutate=false; FeedbackFields (lines 340–358) has no passive status row. A disabled-auth reader can see a reviewed/archived badge in the list, then loses that information when opening detail. A passive status badge would preserve the read-only contract without adding mutation controls. This is not an additional merge gate.
Verified scope and limits
All four findings from review 5070332066 are closed: manual setup remains reachable without discovery; scope=all reaches the native query and relay while retaining the relay’s escalated-only default; reason-audience copy matches the actual notices; resolve retries freeze the command and derive success copy from the relay response. DELETE confirmation and hidden-Staffing reset remain present.
The integrated review traced Desktop Settings/Reports/Feedback/Staffing through native admin transport and exact-head relay/DB producers, including authorization, report/action closed sets, mutation retries, capability transitions, and attachment budgets/lifetimes. Community moderation, mobile/web operator UIs, and deployment configuration are outside this Desktop delivery. Reopen replay preserves the original audit and cannot reopen a later resolution again; its editable optional reason is not being promoted into an enforcement blocker. Raw Staffing error presentation remains the declared follow-up. The proposed blanket private-address prohibition on signed requests is not a blocker: the agreed boundary permits manually trusted internal origins after explicit Save, and discovery does not probe its advertised destination.
Source-only validation: immutable GitHub source/diffs and existing regression tests were inspected; key evidence blobs were verified against the pinned Git tree. No PR checkout, build, test execution, or live app/relay run. This is not a runtime pass.
Refreshed all 14 screenshots with the new Relay admin nav label and ServerCog icon. Shot 01 renamed from 01-moderation-nav to 01-relay-admin-nav to match the updated surface name. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Review clear: prior blockers resolved
Reviewed head 21f34f0138a4e00782f914d28a90346b994b650c against base 42aeb1571cc1f1ed4d73f05b420474be970021ab, including the corrections since review 5073088157. No remaining blocking finding in the reviewed paths and agreed delivery contract.
- Staffing: Add now requires a loaded roster and rejects normalized existing keys before PUT. The actual form/IPC regression checks zero PUTs for an existing operator and a successful new-moderator add with roster refresh. This closes the demonstrated accidental-demotion path; the API remains an upsert, so this is snapshot-based duplicate prevention, not atomic server-side create-only behavior.
- Manual localhost:
http://admin.localhost:<port>is accepted and pinned to loopback without rewriting the hostname used for Host or NIP-98 signing. The added production-client/probe test inspects the signing URL and both wire Host headers. Discovery still requires explicit Save before contacting an advertised admin origin. - Integration: Read-only feedback detail now shows its status. The “Relay admin” rename preserves the old
moderationURL token in route validation and AppShell. Report scope, frozen resolve retries, cancel/reopen semantics, and operator/moderator authorization remain consistent with the previously reviewed contract.
Non-blocking follow-ups: the staffing regression does not explicitly exercise uppercase/whitespace, self-key, or pending/failed roster loading. Discovery’s reserved-domain check omits .localhost, so failed system DNS can allow a loopback prefill; explicit Save still prevents automatic access. These do not reopen the agreed fixes.
Validation: immutable source/diff and existing-test inspection only, with independent native and staffing/feedback reviews integrated. No checkout, builds, tests, or PR-code execution; no runtime-pass claim. Separate community moderation, mobile/web operator UX, deployment configuration, and unrelated merged-main changes were outside this corrective review. This is a COMMENTED review, not approval.
…face Add a NIP-98 client for the /api/admin/v1 relay API, surfaced as a "Relay admin" section in Settings. Relay operators view deployment-wide moderation reports and product feedback, resolve/dismiss reports, update feedback status, and manage the operator/moderator staffing roster — no browser extension or bearer token required. The console auto-discovers the relay's admin origin from its NIP-11 document (admin_api field): on mount it fetches the connected relay's relay-information document, validates the advertised origin through AdminOrigin::parse, and auto-probes. The manual origin field remains as a pre-filled fallback for relays that do not advertise. Rust: - AdminOrigin value object: validates scheme+host+optional-port, rejects credentials/path/query/fragment; http:// only for loopback hosts - AdminRoute closed enum: no IPC surface accepts arbitrary URLs or paths; the signed URL is byte-identical to the fetched URL - Dedicated no-redirect reqwest client (SSRF guard: relay 3xx surfaced as error, NIP-98 header never forwarded across origins) - admin_probe: typed state enum (Nip98Authorized/Denied, TokenMode, Disabled, NotAdminApi, NetworkOrIntercepted); Nip98Authorized only on authenticated 2xx - NIP-11 admin-origin discovery command; advertised value treated as untrusted input and revalidated before use - NIP-98 signing via AppState::signing_keys(); one retry on 401 with a fresh event - Response bounds enforced by Content-Length preflight and streaming byte counter; per-pubkey origin storage (atomic write, 0o600) TypeScript: - admin-console API wrappers for all Tauri commands; attachments return a Blob URL from caller-supplied MIME - AdminConsoleSettingsCard: auto-discovery + probe flow, per-pubkey state, honest copy for every probe state - AdminConsolePanel with Reports / Feedback / Staffing tabs - Settings panels split under the file-size ratchet - Legacy ?section=moderation URL token aliased to relay-admin - Deleted the unreachable community moderation queue card Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
21f34f0 to
efd15bb
Compare
Items addressed:
1. Render without Save: auto-discover → auto-save → auto-probe on first
open. Panel renders immediately; Save is only needed for manual
origin changes. Falls back to pre-fill if set_admin_origin rejects.
2. Remove auto-detect tooltip sentence from SettingsSectionHeader
description. Rename section title 'Relay admin' → 'Admin'.
3. Layout reorder: probe status badge + AdminConsolePanel moved ABOVE
the Advanced <details> disclosure. Origin input goes to the bottom.
4. Badge cleanup: the role (OPERATOR) and provenance (CONFIG/DB) pills
above the tab row were confusing users into thinking they were nav.
Remove both from above the tabs. Fold the role into the connection
status line as plain text ('Connected as operator'). Move provenance
into the Advanced card as small muted text ('Origin resolved from
relay config'). Zero badge-shaped elements remain above the tabs.
5. Nav rename: SettingsPanels.tsx label 'Relay admin' -> 'Admin'.
Updated matching comments in nav.ts and hooks.ts.
6. Staffing parity with Invites: display names via useUsersBatchQuery,
npub cross-fade on hover (HoverStaffingIdentity), in-place role
change via <select> calling putAdminOperator, read-only badge for
config-managed entries, error copy updated to mention edit control.
mountPanel test harness now wraps AdminConsolePanel in a
QueryClientProvider so useUsersBatchQuery resolves cleanly.
7. Timeout expiry rendering: useEffect in useTimeoutState clears store
when derived state is INACTIVE but store still active; tick interval
now starts for null-expiry entries so the blocking overlay does not
persist after TTL. useMembersSidebarModeration gets a 1s nowMs
interval so moderationStateByPubkey re-runs reactively on expiry.
8. Report failure UX: reportErrorMessage() strips HTTP status prefixes
and surfaces the relay's own error reason in the toast. Self-report
blocking UI removed per Hayt's verdict (relay has no self-report
check; failure was client-side error surfacing only).
Tests: updated discovery-success test; added discovery-save-fails-falls-
back test; new timeoutStore.test.mjs; new reportMessageDialog.test.mjs.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…pe timeout ticker Pass-2 fixes for Thufir review findings: IMPORTANT: replace bespoke useQuery/getUsersBatch in StaffingTab with the shared useUsersBatchQuery hook. CommunitiesProvider wraps the entire Settings tree (main.tsx), so the hook is always in context; the prior false comment is removed. This picks up normalized delta-fetch, per-pubkey caching, persisted-label seeding, focus-recovery retry, and profile-cache seeding that the bespoke path bypassed. Also removes the now-unnecessary check-pubkey-truncation allowlist entry for the deleted .slice().sort() queryKey. MINOR: scope the useTimeoutState interval to known-expiry timeouts (expiresAtMs !== null). The null-expiry case ticked every second without state-change potential (isTimeoutActive(null) is permanently true) — a pure no-op re-render loop. Fix the doc comment to match; known-TTL expiry path is unchanged. Tests: add CommunitiesProvider to the mountPanel and staffing-tab-reset-on-role-downgrade harnesses so useUsersBatchQuery resolves in jsdom. Four new behavioral tests: - staffing-display-name: resolved profile name renders (not raw pubkey) - staffing-npub-hover: npub span present and encodes to npub1... - staffing-role-change-success: PUT fired with correct pubkey+role, list refreshes - staffing-role-change-409: 409 rejection surfaces config-backed copy, not raw error Biome gate: remove unused container destructure (adminConsolePanel.test.mjs:1391), string concat → template literals (inboxReopenNavigation.test.mjs), computed keys (channelMutesStorage, channelStarsStorage). Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
2697185 to
b0b5d64
Compare
Adds a relay admin console under Settings — the single surface for relay-level operator work — replacing the previously unreachable moderation section. Operators authenticate with their own Nostr key over NIP-98 — no browser extension, no bearer token — to view deployment-wide moderation reports and product feedback, resolve/dismiss/escalate/reopen reports, update feedback status, and manage the operator/moderator staffing roster.
Nav reachability
The old
moderationsection id was defined insettingsSectionsbut never wired into any group insettingsNavGroups, so the sidebar never rendered it — unreachable since #1617. This PR replaces it with arelay-adminsection wired into the Communities nav group, pointing at the relay admin console, and drops the separateadmin-consolesection id so a single surface owns relay-level trust & safety.Relay admin console
The console is gated behind the relay
OPERATOR/MODERATORroles from #3777; every/api/admin/v1/*route returns403to anyone else. Community self-administration (community-members/ Invites) and in-channel enforcement (the 9040–9043 signed commands) are a separate axis and are untouched. The previously-duplicate community report queue (ModerationQueueCard,moderationQueue.ts, and three dead hooks) was unreachable dead code and is deleted in its own commit;useBanMemberMutationand the rest offeatures/moderation/hooks.tsremain in use by the members sidebar and message menu.The Relay admin nav entry is hidden when
originSource === "none"(i.e. NIP-11 discovery has not yet run or returned noadmin_apifield and no manual origin has been saved). Once a valid origin is discovered or manually saved, the entry becomes visible. Auth still gates the panel itself; the nav entry is a door, not a credential.Discovery and connection
On mount the console reads the connected relay's NIP-11 document for the optional
admin_apifield. When a valid origin is discovered it is automatically saved and probed — the panel renders immediately with no Save step required. Saving is only required for manual origin changes. The advertised origin is untrusted input: revalidated throughAdminOrigin::parse, falling back to manual entry when absent or invalid. A saved manual origin always takes precedence over discovery.Auth and transport (Rust,
src-tauri/src/commands/admin/)AdminOriginvalue object: validates scheme + host + optional port, rejects credentials/path/query/fragment;http://only for loopback.AdminRouteclosed enum: no IPC surface accepts arbitrary URLs, so the NIP-98-signed URL is byte-identical to the fetched URL.reqwestclient as an SSRF guard: a relay3xxis an error and the NIP-98 header never crosses origins.admin_probesignsGET /probeand returns a typed state (nip98Authorized/nip98Denied/tokenMode/disabled/notAdminApi/networkOrIntercepted). The response body is validated against the full relay contract —nip98Authorizedonly when the complete NIP-98 invariant holds,disabledonly on a coherent disabled-mode body; any incoherent or unrelated JSON classifies asnotAdminApi, so the console never fails open at the network boundary. Unknown fields are tolerated for forward compatibility.AppState::signing_keys(), one retry on401with a fresh signed event; response size bounded by aContent-Lengthpreflight and a streaming byte counter; per-pubkey origin storage with atomic0o600writes.Console UI (TypeScript,
src/features/admin-console/)AdminConsoleSettingsCard: discovery + probe flow, honest copy for every probe state, manual origin under anAdvanceddisclosure at the bottom. The role (operator/moderator) is rendered in the connection status line as plain text ("Connected as operator") — no badge-shaped elements above the tab row. The origin provenance (relay config / database) appears inside the Advanced card as small muted text. Section title changed toAdmin.AdminConsolePanelwith Reports / Feedback / Staffing tabs (Staffing gated onrole === "operator"); reports and feedback grouped by community for cross-community triage.scope=allso the existing resolve/cancel/reopen controls can reach every status (open,processing,resolved,dismissed,escalated). The relay's omitted-scope default isescalated-only (the platform-safety backstop);scope=allis explicit and scoped to this console. Full action matrix pertarget_kind(event → delete/kick/ban/timeout/dismiss/escalate; pubkey → ban/timeout/dismiss/escalate; blob → dismiss/escalate); kick is suppressed when the report carries nochannelId. Aprocessingrow stays navigable — enforcement state lives in the detail view. Afailedaction (always pre-mutation) offers a single Cancel & reopen viaPOST /reports/{id}/cancelwith{actionId}fencing; there is no client-side retry.pending/enforcingactions belong to the relay recovery worker and offer no button; a409reloads detail. Lists refetch on back-navigation after a mutation.reportErrorMessage()strips HTTP status prefixes and surfaces the relay's own error reason in the toast (e.g. relay returns"400: you cannot report this content"→ toast shows"you cannot report this content"). Self-report blocking UI removed — the relay has no self-report gate, and the failure was purely client-side error surfacing.processing— a report enforced then reopened isopenyet still shows itssucceededaction as executed history alongside the resolve form; a reopen does not un-happen a ban. The resolve form gates onopenstatus alone.resolved/dismissed/escalated) can be returned toopenfor re-triage viaPOST /reports/{id}/reopen. Reopen never reverses applied enforcement, and the copy says so.new/reviewed/archived) with a generation-fenced attachment viewer. A purged source community severs provenance tonullrather than deleting the row; severed rows bucket under a "source community removed" heading.useUsersBatchQueryand npub cross-fade on hover (HoverStaffingIdentity). An entered key that already appears in the authoritative roster (config, owner-fallback, or DB) is rejected locally with an inline duplicate-key message and no request is issued; a config-backed principal targeted by remove returns the relay's409 Conflict. Roster withconfig/owner_fallback/dbsource labels; config-backed entries disable remove client-side. In-place role change via<select>callingputAdminOperator; read-only badge for config-managed entries.useTimeoutStateclears the store when derived state isINACTIVEbut the store is still active; the 1s tick interval is scoped to entries with a knownexpiresAtMsso that the countdown and automatic clearing work reactively without a permanent rerender loop for null-expiry entries.useMembersSidebarModerationgets a 1snowMsinterval somoderationStateByPubkeyre-runs reactively on expiry.sonnertoasts; failures surface the relay's parsed errormessage, not raw JSON.tokenModeanddisabledauth modes render read-only.Mutation idempotency
Resolve and reopen carry a per-attempt
requestId. On first submit the whole command ({requestId, action, reason, expirationSecs}) is frozen; after an ambiguous failure (409, 5xx, transport error, truncated response) the snapshot is retained, action/reason/duration controls are locked, and the retry sends the exact same payload byte-for-byte. A definitive pre-commit rejection (non-409 4xx withbodyComplete: true) discards the snapshot, unlocks controls, and the corrected submit uses a fresh payload. On success the toast derives from the authoritativeAdminReportResolution(activeAction.actionwhen present; otherwisestatusfor decision-only outcomes) — never from the mutable form selection.The native commands reject with a typed
AdminMutationErrorcarrying the relay's HTTP status (relayStatus,nullwhen no verdict exists) and abodyCompleteflag that istrueonly when the full response body was read.Deferred
AdminConsoleStaffingTab.tsxrenders non-409staffing mutation errors via rawe.messagerather thanadminErrorMessage(e)(the409branches and the Reports/Feedback handlers already use friendly copy). Pre-existing, error-presentation only — no effect on auth or authorization.Related: #3777 (relay
OPERATOR/MODERATORrole model + NIP-98 auth +admin_apiNIP-11 advertisement — provides the runtime and the discovery field this console consumes)Related: bb-public#339 (Phase 4 rollout config)
Screenshots
Captured headless at
197f0ba0cagainst the mock bridge (1280×720).Setup
Origin setup — Advanced disclosure open, idle state

Authorized console — all three tabs

Reports
Reports queue — open, processing, resolved across two relay groups

Open report detail — reporter, target, message snapshot, resolve form

Processing report — PROCESSING/HARASSMENT, ban enforcing state

Resolve form — Delete selected, audience disclosure copy

Resolved report — enforcement history block

Feedback
Feedback list — community grouping with status badges

Feedback detail — mutable status control (operator mode)

Feedback detail — passive status badge (read-only/disabled mode)

Staffing
Staffing roster — config/owner_fallback/db source labels

Add form — new pubkey filled, ready to submit

Duplicate rejection — inline error for existing principal
