diff --git a/Cargo.lock b/Cargo.lock index 041b08a9..edbb6a33 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4591,6 +4591,7 @@ dependencies = [ "hive-jobq", "hive-jobq-wire", "hive-types", + "problem_details", "reqwest 0.13.1", "serde", "serde_json", diff --git a/docs/conventions.md b/docs/conventions.md index bbc97f2f..c19fedfe 100644 --- a/docs/conventions.md +++ b/docs/conventions.md @@ -318,6 +318,28 @@ refactor's concern. The dashboard frontend parses via `util.js::epochSec` wherever it needs arithmetic and feeds the string straight to `new Date(s)` for display. +### HTTP error bodies + +Every HTTP API in this repo answers failures with **RFC 9457 +`application/problem+json`** (`{ type, title, status, detail }`), with the +human-readable cause in `detail`. An endpoint of ours returning a bare string +or a bespoke error shape is a **bug to file against the backend**, not +something for the caller to work around. + +Use the `problem_details` crate (`features = ["axum"]`), which the daemons +already depend on: type a handler `Result<_, ProblemDetails>` and hand +`ProblemDetails::from_status_code(...).with_detail(...)` to `Err`. + +The reason is the consumer, not tidiness. The UIs show errors through one +shared component with a copy button, so a caller has to know **which part of +the body is the message**. A bare string forces it to treat the whole payload +as prose, which is the difference between offering "copy the cause" and +dumping a response — and the cause is frequently the entire diagnosis (a +JetStream permission refusal, a TLS chain failure) rather than a summary. + +Not in scope: the `hivectl` host-admin and in-agent unix sockets. Those are a +JSON-line protocol with their own result types; RFC 9457 is an HTTP format. + ## Tool groups The MCP tool surface an agent receives is derived from a set of named diff --git a/docs/setup.md b/docs/setup.md index 9af3819f..d5ec447c 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -21,16 +21,19 @@ operator has to place, and where. ### 1 · Forge ```bash -# Provision (or refresh) ruth's own forge account — do this first +# Provision (or refresh) ruth's own forge account — do this first. Ruth's +# bootstrap bypasses the normal spawn-approval flow (see step 6), so unlike +# every other agent it does not get its forge account auto-provisioned — +# this manual step is still load-bearing. hivectl forge create-user ruth -# Create a human operator account (prints the token to stdout) -hivectl forge create-user mara --password hunter2 - -# Provision forge accounts for any sub-agents spawned later -hivectl forge create-user +# Sub-agents spawned later (via the approval flow in step 6) get their +# forge accounts auto-provisioned — nothing to run here for them. ``` +The human operator's own forge account is created via swarm SSO instead of +a manual `hivectl` step — see step 3 (`swarmctl user add`). + ### 2 · Gateway (HTTP Basic auth) ```bash @@ -89,20 +92,21 @@ control: [`swarm/ui.md`](swarm/ui.md). # 5a. Ensure the hive-internal admin account exists first hivectl matrix sync-admin -# 5b. Provision ruth's own matrix account +# 5b. Provision ruth's own matrix account — same bootstrap-bypass reasoning +# as forge above, still a required manual step. hivectl matrix create-user ruth -# 5c. Create a human matrix account -hivectl matrix create-user mara --password hunter2 - -# 5d. Invite the operator to the hive Space (and optionally to rooms) +# 5c. Invite the operator to the hive Space (and optionally to rooms) hivectl matrix invite mara hivectl matrix invite @mara:yourserver --room '#hive-chat:yourserver' -# 5e. Promote the operator to homeserver admin if needed +# 5d. Promote the operator to homeserver admin if needed hivectl matrix promote-user mara ``` +The human operator's own matrix account is created via swarm SSO instead of +a manual `hivectl` step — see step 3 (`swarmctl user add`). + ### 6 · Spawn sub-agents Sub-agent creation goes through the approval queue — ruth proposes, the diff --git a/frontend/packages/dashboard/src/credentials.js b/frontend/packages/dashboard/src/credentials.js index 5d2420c3..4ad106ec 100644 --- a/frontend/packages/dashboard/src/credentials.js +++ b/frontend/packages/dashboard/src/credentials.js @@ -23,6 +23,7 @@ import { $, esc, fmtAgeSecs, renderServerWarnings } from './common.js'; import { el } from '@hive/shared/dom.js'; import '@hive/shared/hive-tab-strip.js'; import { themedConfirm, themedToast } from '@hive/shared/modal.js'; +import { readApiError, problemMessage } from '@hive/shared/api-error.js'; let agents = []; // agent name → container running (bool), from /api/state. Cross-referenced by @@ -65,28 +66,13 @@ function renderAgentPicker() { for (const a of agents) sel.append(el('option', { value: a }, a)); } -// ─── shape-agnostic error-body parsing (shared by both tabs' submit -// handlers) ─────────────────────────────────────────────────────────────── -// The BE error-body shape is in transition: today hive-c0re's -// error_response sends a bare plain-text body; the RFC 9457 rework moves it -// to application/problem+json ({ type, title, detail, … }). Read shape- -// agnostically: pull the body once as text, and if it parses as JSON -// surface `detail` (problem+json) → `error`/`title` fallback, else use the -// raw text. A bare HTTP code is the last resort. -async function readErrorBody(resp) { - try { - const raw = (await resp.text()).trim(); - if (raw && (raw[0] === '{' || raw[0] === '[')) { - try { - const body = JSON.parse(raw); - return body.detail || body.error || body.title || raw; - } catch { /* not JSON after all — keep the raw text */ } - } - return raw; - } catch { - return ''; - } -} +// Shape-agnostic error-body parsing (shared by both tabs' submit handlers) +// lives in `@hive/shared/api-error.js` now — `readApiError` + +// `problemMessage` (this page only needs the one-line message, not the +// full `ApiErrorPanel`; its result lines are single-line `aria-live` +// regions, not a swap-in-a-panel context). Was a local function here +// originally; promoted so swarm-ui shares the same +// shape-agnostic reader instead of each side maintaining its own copy. // ─── MATRIX tab ──────────────────────────────────────────────────────────── // Live status dot — the daemon heartbeats every ~30s (advances as_of_unix), @@ -253,7 +239,7 @@ async function submitLogin(e) { clearSecrets(formEl); } } else { - const msg = await readErrorBody(resp); + const msg = problemMessage(await readApiError(resp)); out.className = 'ma-result err'; out.textContent = '✗ ' + (msg || ('login failed (HTTP ' + resp.status + ')')); clearSecrets(formEl); @@ -337,7 +323,7 @@ async function submitGithub(e) { clearSecrets(formEl); } } else { - const msg = await readErrorBody(resp); + const msg = problemMessage(await readApiError(resp)); out.className = 'ma-result err'; out.textContent = '✗ ' + (msg || ('store failed (HTTP ' + resp.status + ')')); clearSecrets(formEl); @@ -419,7 +405,7 @@ async function onForgeRemoveClick(agent, forge, btn) { loadForgeAccounts(agent); return; } - const msg = await readErrorBody(resp); + const msg = problemMessage(await readApiError(resp)); btn.textContent = orig; btn.disabled = false; themedToast('✗ ' + (msg || ('remove failed (HTTP ' + resp.status + ')')), { type: 'error' }); @@ -474,7 +460,7 @@ async function submitForgeAccount(e) { clearSecrets(formEl); } } else { - const msg = await readErrorBody(resp); + const msg = problemMessage(await readApiError(resp)); out.className = 'ma-result err'; out.textContent = '✗ ' + (msg || ('store failed (HTTP ' + resp.status + ')')); clearSecrets(formEl); diff --git a/frontend/packages/shared/package.json b/frontend/packages/shared/package.json index ba195859..02bde7a9 100644 --- a/frontend/packages/shared/package.json +++ b/frontend/packages/shared/package.json @@ -31,7 +31,12 @@ "./jobq-graph.js": "./src/jobq-graph/JobqGraph.tsx", "./jobq-graph.css": "./src/jobq-graph/jobq-graph.css", "./jobq-rollup.js": "./src/jobq-rollup/JobqRollup.tsx", - "./jobq-rollup.css": "./src/jobq-rollup/jobq-rollup.css" + "./jobq-rollup.css": "./src/jobq-rollup/jobq-rollup.css", + "./api-error.js": "./src/api-error.ts", + "./api-error-panel.js": "./src/api-error-panel/ApiErrorPanel.tsx", + "./api-error-panel.css": "./src/api-error-panel/api-error-panel.css", + "./warn-banner.js": "./src/warn-banner/WarnBanner.tsx", + "./warn-banner.css": "./src/warn-banner/WarnBanner.css" }, "files": [ "src/" diff --git a/frontend/packages/shared/src/api-error-panel/ApiErrorPanel.tsx b/frontend/packages/shared/src/api-error-panel/ApiErrorPanel.tsx new file mode 100644 index 00000000..f0ebb3ce --- /dev/null +++ b/frontend/packages/shared/src/api-error-panel/ApiErrorPanel.tsx @@ -0,0 +1,75 @@ +// ApiErrorPanel.tsx — , the shared "show the operator why +// an API call failed" component (mara, on the issue that asked for this: +// "the error display component is for showing ProblemDetails in a nicer +// way with copy button [...] wherever we want to show an error, this +// component should be used"). Renders a `ProblemDetails` (./api-error.ts) +// — heading + the full `detail` text, unclamped (it can be a raw NATS/ +// JetStream error, and on the incident that prompted this component +// *that string was the entire diagnosis* — truncating it defeats the +// point) — plus a copy button for pasting straight into a bug report. +// +// Composed from `WarnBanner` (../warn-banner/WarnBanner.js) rather than +// owning the border/colour/pulse styling itself — mara, on review: "i +// want each component to import its own css file itself[;] if you need +// a bunch of extra css externally, its not a proper component". This +// file's own `.css` only lays out what's specific to it (heading, copy +// button, detail text); the banner "shape" belongs to `WarnBanner` and +// stays there so any future error/warning surface reuses it too, rather +// than every caller growing its own copy of the same rules. +// +// Same shared-component shape as `JobqRollup`/`JobqGraph`: +// `render(h(ApiErrorPanel, { problem }), container)` from vanilla JS, or +// `` from swarm-ui's JSX. +import { useState } from 'preact/hooks'; +import { WarnBanner } from '../warn-banner/WarnBanner.js'; +import type { ProblemDetails } from '../api-error.js'; +import './api-error-panel.css'; + +export interface ApiErrorPanelProps { + problem: ProblemDetails; + // Optional short prefix naming what failed ("failed to load the hive + // roster") — the panel is otherwise just the server's own words, which + // don't always say what the caller was trying to do. + context?: string; +} + +function formatForCopy(p: ProblemDetails): string { + const lines: string[] = []; + if (p.status !== undefined) lines.push(`status: ${p.status}`); + if (p.title) lines.push(`title: ${p.title}`); + if (p.type) lines.push(`type: ${p.type}`); + if (p.detail) lines.push(`detail: ${p.detail}`); + return lines.length ? lines.join('\n') : 'request failed'; +} + +export function ApiErrorPanel({ problem, context }: ApiErrorPanelProps) { + const [copied, setCopied] = useState(false); + const heading = problem.title || (problem.status !== undefined ? `http ${problem.status}` : 'request failed'); + + async function copy() { + try { + await navigator.clipboard.writeText(formatForCopy(problem)); + setCopied(true); + setTimeout(() => setCopied(false), 1500); + } catch { + // clipboard permission denied / unavailable (e.g. non-secure + // context) — the text is still fully visible to select by hand, + // so this is a silent no-op rather than a second error to show. + } + } + + return ( + +
+ + {context ? `${context}: ` : ''} + {heading} + + +
+ {problem.detail ?

{problem.detail}

: null} + + ); +} diff --git a/frontend/packages/shared/src/api-error-panel/api-error-panel.css b/frontend/packages/shared/src/api-error-panel/api-error-panel.css new file mode 100644 index 00000000..df4c853c --- /dev/null +++ b/frontend/packages/shared/src/api-error-panel/api-error-panel.css @@ -0,0 +1,40 @@ +/* api-error-panel.css — layout for ``'s content, plus the + one spacing concern that's genuinely this component's own (the top + margin — `WarnBanner` only sets `margin-bottom`, tuned for a banner + that's the first thing in a page, not a panel that usually follows a + form or a table). Colour/border/pulse belong to `WarnBanner` + (../warn-banner/WarnBanner.css) — not duplicated here. */ + +.api-error-panel { + margin-top: 1em; +} +.api-error-heading { + display: flex; + align-items: baseline; + justify-content: space-between; + gap: 0.8em; +} +.api-error-title { + font-weight: 600; +} +.api-error-copy { + flex: none; + background: transparent; + color: inherit; + border: 1px solid currentColor; + border-radius: 0.3em; + padding: 0.1em 0.6em; + font: inherit; + font-size: 0.85em; + cursor: pointer; +} +.api-error-copy:hover { + background: color-mix(in srgb, currentColor 12%, transparent); +} +.api-error-detail { + margin: 0.5em 0 0; + white-space: pre-wrap; + word-break: break-word; + font-size: 0.9em; + opacity: 0.9; +} diff --git a/frontend/packages/shared/src/api-error.ts b/frontend/packages/shared/src/api-error.ts new file mode 100644 index 00000000..ff69de49 --- /dev/null +++ b/frontend/packages/shared/src/api-error.ts @@ -0,0 +1,63 @@ +// api-error.ts — reads a failed fetch `Response` into a `ProblemDetails` +// (RFC 9457, `application/problem+json`) object, shape-agnostically. +// +// Promoted from `dashboard/src/credentials.js`'s original `readErrorBody`, +// which returned a flat message string. This returns the structured object +// instead so a caller — chiefly `ApiErrorPanel` (./api-error-panel/) — can +// render title/status/detail separately and build a useful copy-button +// payload, rather than re-parsing a pre-squashed string. +// +// RFC 9457 is this hive's committed error-body contract for first-party +// APIs (mara: "any api of our own responding with error that is not rfc +// shaped is a bug to be filed against backend") — every first-party route +// emits it as of this writing, so the non-JSON fallback below is NOT +// compatibility with an in-flight backend transition (that transition is +// finished). It's for bodies we did not author: a caller can be pointed +// at something that isn't ours, and an nginx-synthesised 502/504 never +// reaches our handlers at all, so it's plain HTML, not problem+json. A +// first-party route landing here anyway is a bug to file, not something +// this reader should quietly paper over. + +export interface ProblemDetails { + type?: string; + title?: string; + status?: number; + detail?: string; +} + +export async function readApiError(resp: Response): Promise { + const status = resp.status; + let raw: string; + try { + raw = (await resp.text()).trim(); + } catch { + return { status }; + } + if (raw && (raw[0] === '{' || raw[0] === '[')) { + try { + const body = JSON.parse(raw) as Record; + return { + type: typeof body.type === 'string' ? body.type : undefined, + title: typeof body.title === 'string' ? body.title : undefined, + status: typeof body.status === 'number' ? body.status : status, + detail: + typeof body.detail === 'string' + ? body.detail + : typeof body.error === 'string' + ? body.error + : undefined, + }; + } catch { + // not JSON after all — fall through to the raw-text path below + } + } + return { status, detail: raw || undefined }; +} + +// One-line message for a context that only has room for a single string +// (a form's inline result line, a toast) rather than the full panel — +// same fallback order `readErrorBody` used: detail (or its `error` alias, +// already folded in above) → title → a bare status line. +export function problemMessage(p: ProblemDetails): string { + return p.detail || p.title || (p.status !== undefined ? `http ${p.status}` : 'request failed'); +} diff --git a/frontend/packages/shared/src/warn-banner/WarnBanner.css b/frontend/packages/shared/src/warn-banner/WarnBanner.css new file mode 100644 index 00000000..1f16ea22 --- /dev/null +++ b/frontend/packages/shared/src/warn-banner/WarnBanner.css @@ -0,0 +1,36 @@ +/* WarnBanner.css — three levels, not an open-ended severity+modifier + combination (same rule `hive-warn.css` states, copied faithfully since + this is that component's visual language, just reached through a + class instead of a `:host([level])` attribute selector). Colours reuse + theme.css's own documented semantics: --cyan = "info accents", --amber + = "warnings", --red = "errors, fail state". */ + +.warn-banner { + display: block; + border: 1px solid var(--amber); + border-radius: 4px; + padding: 0.5em 0.8em; + margin-bottom: 0.6em; + color: var(--amber); + background: color-mix(in srgb, var(--amber) 8%, transparent); +} +.warn-banner-info { + border-color: var(--cyan); + color: var(--cyan); + background: color-mix(in srgb, var(--cyan) 8%, transparent); +} +.warn-banner-error { + border-color: var(--red); + color: var(--red); + background: color-mix(in srgb, var(--red) 8%, transparent); + /* Only `error` pulses — an active incident, not a standing caveat. */ + text-shadow: 0 0 6px color-mix(in srgb, currentColor 40%, transparent); + animation: warn-banner-pulse 2.4s ease-in-out infinite; +} +@keyframes warn-banner-pulse { + 0%, 100% { box-shadow: 0 0 12px -4px color-mix(in srgb, currentColor 55%, transparent); } + 50% { box-shadow: 0 0 22px -2px color-mix(in srgb, currentColor 95%, transparent); } +} +.warn-banner strong { + color: inherit; +} diff --git a/frontend/packages/shared/src/warn-banner/WarnBanner.tsx b/frontend/packages/shared/src/warn-banner/WarnBanner.tsx new file mode 100644 index 00000000..f036d465 --- /dev/null +++ b/frontend/packages/shared/src/warn-banner/WarnBanner.tsx @@ -0,0 +1,38 @@ +// WarnBanner.tsx — , the Preact-component successor to the +// shadow-DOM custom element (../hive-warn/hive-warn.js). +// Same three-tier visual language (mara, on `hive-warn`'s own review: +// "there should be distinction between info, warning, error — all +// warnings should be styled identically") — built as a plain Preact +// component instead so it composes cleanly wherever this package's other +// shared components do (JobqRollup, JobqGraph, ApiErrorPanel), rather +// than needing a shadow root + CSS-as-text build step that only resolves +// under one of the two consuming packages' esbuild configs. `ApiErrorPanel` +// is the first consumer — compose from this instead of duplicating the +// banner's own border/colour/pulse rules locally. +// +// `level` defaults to 'warning', matching `hive-warn.js`'s own default. +// Only `error` pulses — an active incident, not a standing caveat, same +// rule `hive-warn.css`'s own comment states. +import type { ComponentChildren } from 'preact'; +import './WarnBanner.css'; + +export type WarnLevel = 'info' | 'warning' | 'error'; + +export interface WarnBannerProps { + level?: WarnLevel; + // Passed straight through to the root element's `class` list — + // for a caller-specific concern the banner itself has no opinion on + // (e.g. `ApiErrorPanel` adding a top margin for the context it usually + // sits in), not for re-styling the banner's own look. + class?: string; + children?: ComponentChildren; +} + +export function WarnBanner({ level = 'warning', class: extraClass, children }: WarnBannerProps) { + const classes = ['warn-banner', `warn-banner-${level}`, extraClass].filter(Boolean).join(' '); + return ( +
+ {children} +
+ ); +} diff --git a/frontend/packages/swarm-ui/src/pages/CreateAgentPage.css b/frontend/packages/swarm-ui/src/pages/CreateAgentPage.css index eeabde39..61f8f368 100644 --- a/frontend/packages/swarm-ui/src/pages/CreateAgentPage.css +++ b/frontend/packages/swarm-ui/src/pages/CreateAgentPage.css @@ -49,6 +49,3 @@ .create-agent-result-ok { color: var(--green); } -.create-agent-result-error { - color: var(--red); -} diff --git a/frontend/packages/swarm-ui/src/pages/CreateAgentPage.tsx b/frontend/packages/swarm-ui/src/pages/CreateAgentPage.tsx index c06ae2eb..ed500fe8 100644 --- a/frontend/packages/swarm-ui/src/pages/CreateAgentPage.tsx +++ b/frontend/packages/swarm-ui/src/pages/CreateAgentPage.tsx @@ -23,6 +23,8 @@ // comment states. import { useState } from 'preact/hooks'; import { Link } from 'wouter-preact'; +import { ApiErrorPanel } from '@hive/shared/api-error-panel.js'; +import { readApiError, type ProblemDetails } from '@hive/shared/api-error.js'; import { Panel } from '../ui/panel/Panel.js'; import './CreateAgentPage.css'; @@ -34,13 +36,22 @@ type SubmitState = | { status: 'idle' } | { status: 'submitting' } | { status: 'done'; nodeId: number } - | { status: 'error'; message: string }; + | { status: 'error'; problem: ProblemDetails }; // Client-side mirror of `hive_types::Ident` (1-63 chars of `[a-z0-9-]`) — // a `pattern` hint only, not a substitute for the server's own // validation; a name this rejects would fail server-side anyway, so // catching it before a round-trip is a pure UX win, not a new gate. -const NAME_PATTERN = '[a-z0-9-]{1,63}'; +// +// Hyphen escaped (`\-`) rather than left trailing in the class: modern +// browsers validate the `pattern` attribute's regex in Unicode-set (`v`) +// mode, which is far stricter about hyphen placement than classic mode — +// `[a-z0-9-]` throws "Invalid character class" under `v` mode even though +// it's valid classic-mode regex (a trailing `-` is unambiguous there). +// Reproduced directly: `new RegExp('[a-z0-9-]', 'v')` throws, +// `new RegExp('[a-z0-9\\-]', 'v')` doesn't — confirmed via mara's console +// exception report on this page. +const NAME_PATTERN = '[a-z0-9\\-]{1,63}'; export function CreateAgentPage() { const [name, setName] = useState(''); @@ -55,12 +66,15 @@ export function CreateAgentPage() { headers: { 'content-type': 'application/json' }, body: JSON.stringify({ name }), }); - if (!r.ok) throw new Error((await r.text()) || `http ${r.status}`); + if (!r.ok) { + setResult({ status: 'error', problem: await readApiError(r) }); + return; + } const data = (await r.json()) as CreateAgentResponse; setResult({ status: 'done', nodeId: data.node_id }); setName(''); } catch (err) { - setResult({ status: 'error', message: String(err) }); + setResult({ status: 'error', problem: { detail: String(err) } }); } } @@ -96,9 +110,7 @@ export function CreateAgentPage() {

)} {result.status === 'error' && ( -

- failed to queue: {result.message} -

+ )} ); diff --git a/frontend/packages/swarm-ui/src/pages/OverviewPage.tsx b/frontend/packages/swarm-ui/src/pages/OverviewPage.tsx index b78f8f12..a7451303 100644 --- a/frontend/packages/swarm-ui/src/pages/OverviewPage.tsx +++ b/frontend/packages/swarm-ui/src/pages/OverviewPage.tsx @@ -8,6 +8,8 @@ // than living in `App.tsx`, matching `JobsPage`'s shape: `App.tsx` is // routing, a page owns its own fetch + render. import { useEffect, useState } from 'preact/hooks'; +import { ApiErrorPanel } from '@hive/shared/api-error-panel.js'; +import { readApiError, type ProblemDetails } from '@hive/shared/api-error.js'; import { Panel } from '../ui/panel/Panel.js'; import { StatusChip, type ChipTone } from '../ui/status-chip/StatusChip.js'; import { Table, type TableColumn } from '../ui/table/Table.js'; @@ -60,21 +62,22 @@ const COLUMNS: TableColumn[] = [ export function OverviewPage() { const [hives, setHives] = useState(null); - const [error, setError] = useState(null); + const [error, setError] = useState(null); useEffect(() => { - fetch('/api/hives/status') - .then((r) => { - if (!r.ok) throw new Error(`http ${r.status}`); - return r.json() as Promise; - }) - .then(setHives) - .catch((e: unknown) => setError(String(e))); + (async () => { + const r = await fetch('/api/hives/status'); + if (!r.ok) { + setError(await readApiError(r)); + return; + } + setHives((await r.json()) as HiveStatus[]); + })().catch((e: unknown) => setError({ detail: String(e) })); }, []); return ( - {error ?

failed to load the hive roster: {error}

: null} + {error ? : null} {!error && hives === null ?

loading…

: null} {hives ? h.name} /> : null} diff --git a/nix/host-modules/swarm-grafana.nix b/nix/host-modules/swarm-grafana.nix index 878f4b1f..79a99907 100644 --- a/nix/host-modules/swarm-grafana.nix +++ b/nix/host-modules/swarm-grafana.nix @@ -174,6 +174,30 @@ in ''; }; }; + + plugins = lib.mkOption { + type = lib.types.listOf lib.types.package; + default = [ ]; + example = lib.literalExpression "[ pkgs.grafanaPlugins.grafana-piechart-panel ]"; + description = '' + Grafana plugins to install, as packages. Declarative rather than + installed through the UI, which is the only shape that works here: + plugin management is **server-admin** scoped, and on an SSO hive + nobody holds that role — `users.auto_assign_org_role` grants an + *org* role, and the built-in local admin that does hold server + admin cannot log in because the login form is disabled whenever + SSO is configured. + + That is a deliberate pair of decisions rather than an oversight, + and this option is the way through it: plugins live in the store + and in git, so they survive a container rebuild and a state reset, + and the container needs no runtime egress to grafana.com. + + Empty by default, which leaves grafana's own plugin handling + untouched. Setting it takes over the plugin directory entirely — + anything installed by other means stops being visible. + ''; + }; }; config = lib.mkIf (hyperhiveCfg.enable && cfg.enable) { @@ -437,6 +461,25 @@ in enable = true; package = cfg.package; + # Passed through unconditionally, empty default included. + # Upstream distinguishes `null` from `[ ]`, and both differences + # favour always handing it a list: + # + # - `null` points the plugin path at grafana's mutable + # `/plugins`; any list points it at a store path. + # Switching on the day someone adds their first plugin would + # bury that change inside an unrelated one. + # - upstream defaults its plugin update-check to + # `declarativePlugins == null`, so a list also stops the + # container phoning grafana.com. That is the no-runtime-egress + # property this option exists for — it should not arrive only + # once a plugin happens to be listed. + # + # Nothing is taken over by claiming the directory on a hive with no + # plugins: manual installation is already impossible here (see the + # option's description), so there is nothing in it to lose. + declarativePlugins = cfg.plugins; + settings = { server = { # Already upstream's default (measured), but pinned rather diff --git a/swarm-controller/Cargo.toml b/swarm-controller/Cargo.toml index aad95ae4..6ac33c60 100644 --- a/swarm-controller/Cargo.toml +++ b/swarm-controller/Cargo.toml @@ -27,6 +27,10 @@ forgejo-api.workspace = true # raw bytes. base64.workspace = true futures-util.workspace = true +# RFC 9457 `application/problem+json` error bodies. Same version + `axum` +# feature as hive-c0re: the two daemons answer the same operator UIs, so a +# reader that handles one's failures has to handle the other's. +problem_details = { version = "0.9.0", features = ["axum"] } # The graph itself, held directly rather than behind a c0re-style wrapper # module — that layering (`hive-c0re::job_queue`) is partially legacy (predates # `hive-jobq`'s extraction into its own crate) and this daemon does not need it diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index 26aa08fd..f0a9841d 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -428,14 +428,29 @@ async fn get_links(State(state): State) -> Json> { /// body because a bare 503 on an operator-facing diagnostic is how a /// misconfiguration costs an afternoon; it is a queue/JetStream error /// string, and this surface is already behind the swarm's SSO. +/// +/// It travels in `detail` of an RFC 9457 `application/problem+json` body +/// rather than as a bare string, so the cause is an addressable field +/// instead of the whole payload — see `error_problem` below. struct StatusUnavailable(String); impl axum::response::IntoResponse for StatusUnavailable { fn into_response(self) -> axum::response::Response { - (axum::http::StatusCode::SERVICE_UNAVAILABLE, self.0).into_response() + error_problem(axum::http::StatusCode::SERVICE_UNAVAILABLE, &self.0).into_response() } } +/// Every error this daemon returns, in one shape. +/// +/// RFC 9457 `application/problem+json` is the hive-wide contract for HTTP +/// error bodies (`docs/conventions.md`), and the operator UIs read `detail` +/// for display. A bare string forces the reader to treat the entire body as +/// the message, which is the difference between a UI that can offer "copy the +/// cause" and one that can only dump a response. +fn error_problem(status: axum::http::StatusCode, detail: &str) -> problem_details::ProblemDetails { + problem_details::ProblemDetails::from_status_code(status).with_detail(detail) +} + /// What each hive last said about itself, read from the swarm queue at /// request time. /// @@ -522,17 +537,17 @@ struct CreateAgentResponse { request_body = CreateAgentRequest, responses( (status = 200, description = "job chain queued", body = CreateAgentResponse), - (status = 400, description = "`name` is not a valid identifier", body = String), - (status = 500, description = "the job chain could not be queued", body = String), + (status = 400, description = "`name` is not a valid identifier (problem+json)", body = String), + (status = 500, description = "the job chain could not be queued (problem+json)", body = String), ), tag = "agents" )] async fn create_agent( State(state): State, Json(req): Json, -) -> Result, (axum::http::StatusCode, String)> { +) -> Result, problem_details::ProblemDetails> { let agent = hive_types::Ident::parse(&req.name) - .map_err(|reason| (axum::http::StatusCode::BAD_REQUEST, reason.to_owned()))? + .map_err(|reason| error_problem(axum::http::StatusCode::BAD_REQUEST, reason))? .into_string(); let repo = agent.clone(); @@ -562,7 +577,12 @@ async fn create_agent( .after_ok(create_repo); vec![create_identity.guid()] }) - .map_err(|e| (axum::http::StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + error_problem( + axum::http::StatusCode::INTERNAL_SERVER_ERROR, + &e.to_string(), + ) + })?; let [id] = ids[..] else { unreachable!("exactly one handle was asked for"); }; @@ -770,11 +790,46 @@ async fn main() -> Result<()> { #[cfg(test)] mod tests { use super::{ - DEFAULT_SOCKET, HIVES_ENV, HiveEntry, LINKS_ENV, ServiceLink, SwarmNodeKind, WorkerDeps, - load_hives, load_links, run_swarm_node, + DEFAULT_SOCKET, HIVES_ENV, HiveEntry, LINKS_ENV, ServiceLink, StatusUnavailable, + SwarmNodeKind, WorkerDeps, load_hives, load_links, run_swarm_node, }; use std::path::Path; + /// The 503 this route returns is what the operator UIs' shared error + /// component renders, so assert the RENDERED response rather than the + /// `problem_details` crate: the contract a UI depends on is the content + /// type plus a `detail` it can address, and a handler that built the + /// value and returned it as a bare string would satisfy any test + /// written against the type alone. + #[tokio::test] + async fn status_unavailable_renders_problem_json_with_the_cause_in_detail() { + use axum::response::IntoResponse as _; + + let cause = "listing status bucket keys: timed out"; + let resp = StatusUnavailable(cause.to_owned()).into_response(); + + assert_eq!(resp.status(), axum::http::StatusCode::SERVICE_UNAVAILABLE); + let ct = resp + .headers() + .get(axum::http::header::CONTENT_TYPE) + .and_then(|v| v.to_str().ok()) + .unwrap_or_default() + .to_owned(); + assert!( + ct.starts_with("application/problem+json"), + "RFC 9457 media type, got {ct:?}" + ); + + let bytes = axum::body::to_bytes(resp.into_body(), 64 * 1024) + .await + .expect("body reads"); + let v: serde_json::Value = serde_json::from_slice(&bytes).expect("problem+json parses"); + assert_eq!(v["status"], 503); + // The cause is an addressable field, not the entire payload — that + // distinction is the point of the change, so it is what is asserted. + assert_eq!(v["detail"], cause); + } + /// Drives `SwarmNodeKind::CreateRepo` through the real /// `hive_jobq::scheduler::Scheduler` claim → run → complete path, /// rather than only through `create_agent`'s endpoint test (there diff --git a/swarmctl/src/users.rs b/swarmctl/src/users.rs index 400d89a3..a423d10d 100644 --- a/swarmctl/src/users.rs +++ b/swarmctl/src/users.rs @@ -122,6 +122,37 @@ fn quote(s: &str) -> String { out } +/// Domain for an address this crate invents. +/// +/// The same one `hive-c0re` already gives every agent's forge account +/// (`forge::users::agent_email`) and every hyperhive-authored git commit. A +/// deployment-derived domain was considered and rejected: it would have to be +/// passed in from config, and an operator who is already supplying a domain +/// may as well supply the whole address — while a *second* convention for +/// synthetic identities is a thing to keep in sync forever. +/// +/// Never routable, and that is correct rather than a compromise. Nothing +/// sends mail here; the address exists so that a relying party asking for an +/// `email` claim gets one. +const SYNTHETIC_EMAIL_DOMAIN: &str = "hyperhive.local"; + +/// The address a user with no explicit one is rendered as. +/// +/// Every user needs an email in the rendered file, because a relying party +/// that asks for the `email` claim and gets nothing does not degrade — it +/// fails. Grafana's OIDC login is the measured case: with no email claim it +/// falls through to `/emails`, a GitHub-ism authelia does not +/// implement, and the login dies with `InternalError` rather than anything +/// naming the missing field. +/// +/// Synthesised in the **renderer**, never written to the store: `users.json` +/// stays honest that no address was supplied, so an operator who later sets a +/// real one is not fighting a value swarmctl invented, and every existing user +/// is fixed by the next render with no migration step. +fn synthetic_email(username: &str) -> String { + format!("{username}@{SYNTHETIC_EMAIL_DOMAIN}") +} + /// Render the store as authelia's block-style YAML users database. /// /// Fallible because it re-runs validation over every value it is about to @@ -142,10 +173,15 @@ pub fn render_yaml(store: &UserStore) -> Result { writeln!(out, " {name}:")?; writeln!(out, " displayname: {}", quote(&user.displayname))?; writeln!(out, " password: {}", quote(&user.password))?; - if let Some(email) = &user.email { - reject_control_chars("email", email)?; - writeln!(out, " email: {}", quote(email))?; - } + // Unconditional: an absent email is the failure mode, not a valid + // rendering. The synthetic address is validated on the same path as + // a supplied one so neither can smuggle a control character. + let email = match &user.email { + Some(supplied) => supplied.clone(), + None => synthetic_email(name), + }; + reject_control_chars("email", &email)?; + writeln!(out, " email: {}", quote(&email))?; if !user.groups.is_empty() { writeln!(out, " groups:")?; for group in &user.groups { @@ -325,21 +361,61 @@ mod tests { } #[test] - fn optional_fields_are_omitted_rather_than_emitted_empty() { + fn an_empty_group_list_is_omitted_rather_than_emitted_empty() { let mut store = UserStore::default(); store.users.insert("mara".to_owned(), user("$argon2id$x")); let out = render_yaml(&store).expect("renders"); - assert!( - !out.contains("email"), - "absent email must not appear:\n{out}" - ); assert!( !out.contains("groups"), "an empty group list must not appear:\n{out}" ); } + /// Email is deliberately NOT in the test above any more. It used to + /// assert that an absent one is omitted, which pinned the behaviour that + /// broke grafana's login: authelia serves no `email` claim, and a relying + /// party that wants one fails rather than degrading. + #[test] + fn a_user_with_no_email_still_renders_one_from_the_shared_domain() { + let mut store = UserStore::default(); + store.users.insert("mara".to_owned(), user("$argon2id$x")); + + let out = render_yaml(&store).expect("renders"); + assert!( + out.contains(r#"email: "mara@hyperhive.local""#), + "a user with no email must still render one:\n{out}" + ); + } + + #[test] + fn a_supplied_email_is_never_replaced_by_the_synthetic_one() { + let mut u = user("$argon2id$x"); + u.email = Some("real@elsewhere.example".to_owned()); + let mut store = UserStore::default(); + store.users.insert("mara".to_owned(), u); + + let out = render_yaml(&store).expect("renders"); + assert!( + out.contains(r#"email: "real@elsewhere.example""#), + "the supplied address must win:\n{out}" + ); + assert!( + !out.contains("mara@hyperhive.local"), + "the synthetic address must not also appear:\n{out}" + ); + } + + /// The synthetic address goes through the same validation as a supplied + /// one. A username is already constrained to `[A-Za-z0-9._-]`, so this + /// cannot currently fail — which is exactly why it is worth pinning: the + /// day username rules loosen, the renderer must still refuse rather than + /// quietly emit whatever it built. + #[test] + fn the_synthetic_address_is_built_from_the_username_and_domain() { + assert_eq!(synthetic_email("mara"), "mara@hyperhive.local"); + } + #[test] fn groups_render_as_a_block_sequence() { let mut u = user("$argon2id$x");