feat(desktop): relay operator console as the single Moderation surface - #4768
feat(desktop): relay operator console as the single Moderation surface#4768wpfleger96 wants to merge 15 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>
Add a NIP-98 client for the /api/admin/v1 relay API, surfaced as an "Admin console" 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() (Err in recovery mode); 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 Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Wire "moderation" into settingsNavGroups under the Communities group. The entry was defined in settingsSections but never added to a nav group, so the sidebar never rendered it and nothing deep-linked to it — it has been unreachable since #1617. Repoint the "moderation" section to the relay admin console (AdminConsole SettingsCard), retitle it "Moderation", and drop the separate "admin-console" section id so a single surface owns relay-level trust & safety. Gate the nav entry behind origin + probe resolution: hidden with no origin, always visible for a saved manual origin (the Advanced control that fixes a bad URL lives inside the surface), and — for an advertised-only origin — visible on authorization or a transport flake but hidden on a definitive non-admin verdict. Group reports and feedback by community for cross-community triage, and collapse the manual origin field under an Advanced disclosure that keeps its controls mounted. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The community-mode moderation queue was reachable only through ModerationQueueCard, which the previous commit removed from the nav — the relay admin console is now the single moderation surface. Delete the card, its triage library (lib/moderationQueue.ts) and that library's tests, and the three hooks the card was the sole consumer of (useModerationReportsQuery, useModerationAuditQuery, useResolveReportMutation). The remaining moderation hooks — ban/timeout/unban and report submission — stay: the members sidebar and message menu still consume them. With the two report/audit read hooks gone, the write-invalidation now touches only the surviving restrictions read. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Add a reopen affordance to the report detail: terminal reports
(resolved | dismissed | escalated) show a form that moves the report
back to open for re-triage via POST /reports/{id}/reopen.
Reopen is re-triage only — it does not reverse any enforcement already
applied. The copy says so, and warns more emphatically when the report
carries an actionId (an enforcement action ran while it was resolved).
A per-attempt requestId is generated once and reused on a 409 retry so a
report that has moved to processing dedupes on the relay, mirroring the
resolve flow's idempotency.
Surface confirmation on the report and feedback actions with sonner
success toasts (resolve, retry, cancel, reopen, feedback status).
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…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>
Adopt the ratified relay->desktop action record JSON: add actorPubkey, actorRole, and errorMessage; replace the invented expirationSecs with the absolute expiresAt enforcement expiry; drop reportId (the parent report already carries provenance). Drop the now-absent expirationSecs from the retry body and rebuild the enforcement fixture from the wire shape. Type-level slice only; cancel-route wiring and feedback nullable-community shapes wait on the relay batch. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Wire the admin console to the relay's cancel route and align the report
detail to the ratified LATERAL semantics.
Cancel replaces the composed client-side retry. A failed enforcement is
always pre-mutation, so it is always cancellable; cancel returns the
report to `open` for a fresh resolution. Composing cancel + resolve would
imply an atomicity the relay does not provide, so the console offers a
single "Cancel & reopen" button on `failed` only — pending/enforcing are
owned by the relay recovery worker — POSTs `{actionId}` to
/reports/{id}/cancel, treats 409 as refresh-detail, and reloads rather
than consuming the response body.
Honest history: the detail LATERAL returns a succeeded action even on a
reopened (open) report — the ban actually ran, and a later reopen does
not un-happen it. The enforcement block now renders whenever an action is
present (live or history) and the resolve form gates on `isOpen` alone, so
a reopened-after-enforcement report shows both the executed-enforcement
history and the resolve form for re-triage. The old `isOpen && !activeAction`
gate wrongly stranded the operator on such a report.
Tighten the wire seam to match the relay's no-skip_serializing_if
serialization: reason/expiresAt/errorMessage are required-nullable, and
resolve/cancel return the `{status, activeAction}` envelope. Feedback
communityId/communityHost are nullable — a purged source community severs
provenance to NULL while retaining the row as operator evidence; severed
rows bucket under a "source community removed" heading and render em-dashes
in detail.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…errors The desktop admin console had four e2e defects. Probe discarded role/source: it hit /reports and hardcoded role: None behind a comment referencing X-Admin-Role headers the relay never sends, so the Staffing tab and role badges were unreachable for operators. Probe now targets the signed /probe endpoint and parses the ProbeResponse contract, carrying the relay-resolved role/source through. Reports and Feedback lists served stale rows after a detail-view mutation until a tab switch forced a remount. Each tab now bumps a list generation fence on mutation completion so back-nav refetches. Kick was offered on every event report, but the relay 400s it when the report carries no channel. It is now suppressed when channelId is null. Mutation errors rendered raw admin API error JSON inline; they now route through toast.error with the parsed relay message. The seed script inserted no channel_id on any report, so the failed-enforcement -> Cancel & reopen path was unreachable locally. It now seeds a channel and an event report carrying it. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The probe accepted every structurally deserializable 2xx as Nip98Authorized and forwarded whatever role/source strings arrived, without requiring status == "ok", authMode == "nip98", a recognized non-null role/source, canAct == true, or canStaff == (role == operator). A token-mode, error-status, or role/capability-mismatched body reaching the authenticated retry authorized the console and could expose Staffing against a relay that denied it — a fail-open network-boundary classification. The unauthenticated 200 branch matched authMode alone. Add typed ProbeRole/ProbeSource enums (closed vocabularies) and two validators on ProbeWire: authorized_principal returns the typed principal only when the complete nip98 invariant holds; is_coherent_disabled gates the disabled branch on the full disabled-mode shape. Both call sites fail closed to NotAdminApi otherwise. parse_probe stays pure structural deserialization with Serde's unknown-field tolerance for forward compatibility. Route FeedbackStatusControl mutation failures through toast.error(adminErrorMessage(e)) like the other three handlers, so a relay error envelope no longer renders as raw JSON inline. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The resolve/reopen retry paths preserved their idempotency requestId only when the error string contained "409"/"processing". The native layer serializes lost-response failures as "relay unreachable: …" and "admin response stream error", so every ambiguous transport/read failure cleared the UUID and the retry became a brand-new command — defeating idempotency on exactly the failures it exists to contain (two-operator interleave: A reopens, response lost, B resolves, A's retry with a fresh UUID clobbers B's later action). Replace the string match with a typed AdminMutationError carrying the relay's HTTP status. The UI preserves the requestId unless the relay definitively rejected the request pre-commit (a non-409 4xx); it preserves on null status (transport/pre-send), 409, 5xx, and a lost response body, so the relay dedupes the retry. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…ll body Two follow-ups to the idempotency fix from the kalvin-agent security review. The requestId-preservation tests exercised only admin_reopen_report, so the resolve call-site — the enforcement path — could regress to the stale-intent bug with no test going red (verified: unconditionally resetting the resolve requestId left the 35-test admin jsdom suite green). Add resolve lost-response and definitive-4xx tests, mutation-verified against the resolve catch block. preserveRequestIdOnError decided from HTTP status alone, so a truncated 4xx (status read, body lost mid-stream) was treated as a definitive pre-commit rejection and cleared the key even though no authoritative body arrived. Add a bodyComplete discriminator to AdminMutationError, set true only when the full response body is read (authoritative) and false for a redirect, over-cap, or mid-stream read failure (partial). The UI resets a non-409 4xx only when the body was complete; a truncated 4xx now preserves the key. Message strings are unchanged. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…sole The relay's bearer-token admin auth mode is being removed entirely (NIP-98 becomes the only authenticated mode), so the desktop console's TokenMode handling is now dead: the relay will never emit a WWW-Authenticate: Bearer challenge for the admin API. Drop AdminProbeResult::TokenMode and its Bearer-challenge classification. A Bearer 401 is no longer a recognized Buzz admin mode, so it now falls through to NotAdminApi alongside every other unrecognized 401 challenge. Remove the tokenMode UI state, its "use the web console" copy, and the tokenMode probe variant across api.ts, AdminConsoleSettingsCard.tsx, and nav.ts. The Bearer-401 test asserts the new NotAdminApi classification. 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.
P1-1 (SSRF/auto-probe): Relay-advertised admin_api now pre-fills the origin
field only — it is never auto-probed. Separate discovery.rs module validates
the advertised value with stricter rules than manual entry: rejects reserved
IP literals (loopback, RFC1918, link-local) and hostname targets that resolve
to private addresses (DNS-rebinding-safe). ModerationNavResolution hook and
AdminConsoleSettingsCard both adopt the pre-fill-only contract. Tests in
discovery.rs cover loopback, RFC1918, link-local, DNS-rebinding, accept-header,
and NIP-11 path contracts. NIP-11 code extracted to discovery.rs to keep
mod.rs and mod_tests.rs under the 1000-line gate.
P1-2 (attachment fan-out): applyAttachmentBudget enforces a 5-item count cap
and 50 MiB aggregate byte budget before rendering AttachmentViewers. Excess
attachments are dropped with a visible truncation notice. Exported function
has 4 regression tests proving count limit, byte limit, and empty-list cases.
P2-1 (disabled-auth read-only): AdminConsolePanel now accepts canMutate prop
(false when probe state is disabled). All mutation affordances — FeedbackStatus
Control, ResolveReportForm, ReopenReportForm, EnforcementStateBlock cancel
button — are gated on canMutate. AdminConsoleSettingsCard passes canMutate=true
only for authorized probes. Test asserts feedback-status-control is absent in
disabled mode and present in authorized mode.
P2-2 (aria-pressed): FeedbackStatusControl buttons carry aria-pressed={current
Status === s}. feedback-status-honest test extended to assert aria-pressed=true
on the active button and aria-pressed=false on inactive buttons.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…n-surface * origin/main: refactor(db): extract domain stores from database runtime (#6987) feat(desktop): add team sharing to community catalog (#3995) Refresh mobile utility surfaces and theme picker (#6944) fix(desktop): complete project empty and context states (#6980) Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
MinimalDocument does not route synthetic events through React 19's
container-level delegation, so feedbackTab.click() always throws
TypeError: feedbackTab.click is not a function.
Add an optional initialTab prop to AdminConsolePanel (test-only
escape hatch) so MinimalDocument tests can land on the feedback tab
without event dispatch. Update mountPanel() to thread the prop
through. Rewrite the two failing P2-1 regression tests:
- disabled-auth-read-only: uses mountPanel({ canMutate: false,
initialTab: 'feedback' }) and asserts feedback-status-control
is absent — fails if the {canMutate && <FeedbackStatusControl>}
guard is removed.
- authorized-auth-read-write: removes the pointless feedbackTab.click()
and simply asserts the panel renders in authorized mode; the full
FeedbackStatusControl presence check lives in the jsdom suite where
fireEvent drives React event delegation correctly.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Adds an in-app admin console for relay operators and makes it the single Moderation surface under Settings. 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 bug this fixes
The
moderationsettings section was defined insettingsSectionsbut never added to any group insettingsNavGroups, so the sidebar never rendered it — the surface has been unreachable since #1617. This wiresmoderationinto theCommunitiesnav group, repoints it at the relay admin console, and drops the separateadmin-consolesection id so a single surface owns relay-level trust & safety.Single Moderation surface
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 Moderation nav entry renders only when the console can plausibly authorize: hidden with no origin, always visible with a saved manual origin (the
Advancedcontrol that fixes a bad URL lives inside the surface), and for an advertised-only origin, visible unless the probe returns a definitive non-admin verdict (nip98Denied/tokenMode/notAdminApi). A probe network failure is never treated as unauthorized — the surface renders its error state rather than silently disappearing on a flake.Discovery and connection
On mount the console reads the connected relay's NIP-11 document for the optional
admin_apifield — zero manual input on the happy path. The advertised origin is untrusted input: revalidated throughAdminOrigin::parse, falling back to a manual field 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, role/source badges when authorized, honest copy for every probe state, manual origin under anAdvanceddisclosure.AdminConsolePanelwith Reports / Feedback / Staffing tabs (Staffing gated onrole === "operator"); reports and feedback grouped by community for cross-community triage.target_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, because composing cancel + resolve would imply an atomicity the relay does not provide.pending/enforcingactions belong to the relay recovery worker and offer no button; a409reloads detail. Lists refetch on back-navigation after a mutation.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.config/owner_fallback/dbsource badges; config-backed entries disable remove client-side and surface the relay's409as a human-readable message.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. The native commands reject with a typedAdminMutationErrorcarrying the relay's HTTP status (relayStatus,nullwhen no verdict exists) and abodyCompleteflag that istrueonly when the full response body was read. TherequestIdis preserved — so a retry reuses it and the relay dedupes — on409,5xx, any transport/pre-send failure, and a truncated response; it resets only on a definitive pre-commit rejection (non-4094xxwithbodyComplete: true). Status alone is insufficient — a truncated4xxlooks definitive but carries no authoritative body. This closes the two-operator interleave where a lost response on operator A's commit would otherwise let A's retry clobber operator B's later state.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)