From 6875a4f0f8e92c856bd5ba34e1f9ba4ef0f6f363 Mon Sep 17 00:00:00 2001 From: iris Date: Wed, 11 Nov 2026 18:29:29 +0100 Subject: [PATCH] swarm-ui: set the review classifier's keys from the classifier settings The classifier's Anthropic and ACP provider keys were written by hand in the secret store's UI. The controller now stores them: GET /api/review-classifier/keys says whether each is set and when it changed, and PUT /api/review-classifier/keys/{runtime} writes one. No route answers a key. The controller's store policy gains create and update on the two leaves with no read of their values, and a metadata read for the set state. The swarm UI's classifier settings card gets a key form beside the settings, saved apart because it goes to the secret store. --- docs/2-handbook/swarm/services.md | 7 +- docs/2-handbook/swarm/ui.md | 2 +- docs/4-implementation/swarm/credentials.md | 3 +- .../swarm/review-classifier.md | 32 +- docs/4-implementation/web-ui/swarm-ui.md | 11 +- .../pages/review-settings/ClassifierKey.tsx | 127 +++++++ .../review-settings/ClassifierSettings.tsx | 11 +- .../src/pages/review-settings/KeyTypes.ts | 45 +++ .../swarm-ui/test/classifier-key.test.mjs | 47 +++ nix/host-modules/deploy.nix | 2 +- nix/host-modules/swarm-bao.nix | 21 +- nix/module-eval/bao-grants.nix | 11 +- swarm-controller/src/review_classifier.rs | 1 + .../src/review_classifier/keys.rs | 323 ++++++++++++++++++ swarm-controller/src/startup.rs | 2 + swarm-secret-client/src/review_classifier.rs | 3 +- 16 files changed, 628 insertions(+), 20 deletions(-) create mode 100644 frontend/packages/swarm-ui/src/pages/review-settings/ClassifierKey.tsx create mode 100644 frontend/packages/swarm-ui/src/pages/review-settings/KeyTypes.ts create mode 100644 frontend/packages/swarm-ui/test/classifier-key.test.mjs create mode 100644 swarm-controller/src/review_classifier/keys.rs diff --git a/docs/2-handbook/swarm/services.md b/docs/2-handbook/swarm/services.md index b19ec8ebe..b74e8a92e 100644 --- a/docs/2-handbook/swarm/services.md +++ b/docs/2-handbook/swarm/services.md @@ -350,8 +350,11 @@ identity: the host that runs the store has it, and a services host with the store elsewhere gets the classifier only once it holds a copy of the pair (see below). Set `deploy.review-classifier.enable` directly to override either way. -The operator writes the key of the runtime the swarm settings choose by hand, -in the secret store's UI under the OIDC login, field `api_key`: +The operator sets the key of the runtime the swarm settings choose in the +swarm UI: the **classifier settings** card on `/jobs` takes a key for the +chosen runtime and shows whether one exists and when it changed, never the +key. You can also write the same two keys in the secret store's UI under the +OIDC login, field `api_key`: - the API key runtime's Anthropic key at `secret/swarm/services/review-classifier/anthropic-api-key` diff --git a/docs/2-handbook/swarm/ui.md b/docs/2-handbook/swarm/ui.md index 10dfa8f69..e51da3da5 100644 --- a/docs/2-handbook/swarm/ui.md +++ b/docs/2-handbook/swarm/ui.md @@ -18,7 +18,7 @@ place, and every one reads from [`swarm-controller`](../../../swarm-controller/R | `/agents//terminal` | one agent's live terminal | | `/agents//subagents//terminal` | one of that agent's subagents' live terminal | | `/reports` | three folded cards, each read only once opened: the **alerts** the swarm's Grafana holds now, the firing ones, with a button for the pending, no-data and error ones and a link to Grafana's own alert list; read through the controller with a Viewer service-account token, and a cross-repo **issues** list, and a cross-repo **pull requests** list | -| `/jobs` | the controller's job graph, where agent creation and credential mints show progress, and under it the swarm builder's recent builds (agent config PRs, and agents built from `internal/meta` before a deploy), with what nix printed when one failed, and the review classifier's recent runs, a try-out on a real PR, and its settings | +| `/jobs` | the controller's job graph, where agent creation and credential mints show progress, and under it the swarm builder's recent builds (agent config PRs, and agents built from `internal/meta` before a deploy), with what nix printed when one failed, and the review classifier's recent runs, a try-out on a real PR, and its settings and keys | | `/messages` | the swarm-wide message feed: every message between agents, and to them from the operator, reminders and compaction, with a history window and a filter on sender or recipient; a folded **broadcast** card above the feed sends one message to more than one agent at once, and a folded **schedules** card holds the scheduled prompts: add, edit, pause, resume and remove | An agent created here or with `swarmctl agent create` starts `paused`; set it diff --git a/docs/4-implementation/swarm/credentials.md b/docs/4-implementation/swarm/credentials.md index bed4654af..0857b09c1 100644 --- a/docs/4-implementation/swarm/credentials.md +++ b/docs/4-implementation/swarm/credentials.md @@ -233,7 +233,8 @@ leaves under `secret/data/swarm/services/attic/` (`push-token`, `pull-token`, `url`, `cache` and `public-key`). That shows the key tree and each key's versions and timestamps, and no value: the store refuses every `secret/data/` read. The writes are the leaves the operator fills in by hand: the review -classifier's keys, and the cache's tokens and facts, which configure an external +classifier's keys (which `swarm-controller` also writes for the swarm UI, with +`create` and `update` and no `read` of the value), and the cache's tokens and facts, which configure an external cache or replace the in-swarm cache's own. The cache's host units create those leaves only while the store holds none, so a saved value survives their next run and every deploy. The cache's signing key, `attic/jwt-secret`, isn't writable here. diff --git a/docs/4-implementation/swarm/review-classifier.md b/docs/4-implementation/swarm/review-classifier.md index ee18bd3d3..aecf014c8 100644 --- a/docs/4-implementation/swarm/review-classifier.md +++ b/docs/4-implementation/swarm/review-classifier.md @@ -284,8 +284,33 @@ does. Every error is problem+json with the reason in `detail`: - **500** when the controller can't read or write the settings When the controller can't read the settings, a try-out answers the same 503 -or 500, and a review posts a no-verdict comment. The operator writes each -runtime's key by hand in the secret store +or 500, and a review posts a no-verdict comment. The keys aren't in this +document; see [The keys](#the-keys). + +### The keys + +`GET /api/review-classifier/keys` answers whether each runtime has a key: + +```json +{ + "api_key": { "set": true, "updated": "2026-11-11T10:00:00Z" }, + "acp": { "set": false, "updated": null } +} +``` + +`PUT /api/review-classifier/keys/{runtime}` (`api-key` or `acp`) takes +`{"api_key": "…"}`, stores it at that runtime's leaf and answers the same +document. No route answers a key. The controller's store policy holds `create` +and `update` on the two leaves with no `read` of their values, and `read` on +their metadata, which is where `set` and `updated` come from. The route trims a +key and refuses one that's empty, over 4096 bytes or holds whitespace. The +PUT needs the `Remote-User` header, and logs the operator and the runtime, never +the key. Every error is problem+json: **400** for a body that isn't this shape, +a runtime that isn't one of the two or a refused key, **401** for no +`Remote-User`, **503** when the store is unreachable or refuses. + +The classifier reads the leaf on every run, so a new key applies from the next +run. The operator can still write the leaves in the secret store's UI ([`services.md`](../../2-handbook/swarm/services.md#review-classifier)). The per-agent equivalent is [`inference.md`](inference.md), where the @@ -479,7 +504,8 @@ ACP provider, so each runtime has a model field of its own. To switch to ACP, an admin: -1. writes the provider key in the secret store's UI under the OIDC login, at +1. stores the provider key, in the swarm UI's classifier settings or in the + secret store's UI under the OIDC login, at `secret/swarm/services/review-classifier/acp-provider`, field `api_key` 2. PUTs the runtime and the provider: diff --git a/docs/4-implementation/web-ui/swarm-ui.md b/docs/4-implementation/web-ui/swarm-ui.md index 900e3332c..01c376c0c 100644 --- a/docs/4-implementation/web-ui/swarm-ui.md +++ b/docs/4-implementation/web-ui/swarm-ui.md @@ -327,8 +327,15 @@ limit gets a no-verdict review, and a try-out with more gets a 400. the runtime, model, effort and limits, and the provider too while the runtime is ACP, so switching back to API key keeps the stored provider for next time. If the controller refuses it, the page shows the reason and leaves the form as you -typed it. A change takes effect from the next run, with no restart. The page -has no API key field: admins set each runtime's key in the bao UI. +typed it. A change takes effect from the next run, with no restart. + +Under the settings form sits a **key** form for the runtime the form chooses. +Its label says whether that runtime has a key and when it last changed +(`GET /api/review-classifier/keys`, which never answers a key). The field is a +password input; **save key** sends it alone with `PUT +/api/review-classifier/keys/{runtime}`, apart from the settings, and clears the +field once the controller has stored it. Switching the runtime drops a key you +typed for the other one. ### Scheduled prompts diff --git a/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierKey.tsx b/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierKey.tsx new file mode 100644 index 000000000..7707da604 --- /dev/null +++ b/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierKey.tsx @@ -0,0 +1,127 @@ +// — the key the chosen runtime runs on, set from here instead +// of in the bao UI (`GET /api/review-classifier/keys`, `PUT .../keys/{runtime}`). +// The page can say whether a key is stored and when it changed, never what it +// is: the field takes a new key and is cleared once it's saved. A key is +// saved apart from the settings, because it goes to the secret store and not +// into the settings document. +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 { ActionRow } from "../../ui/action-row/ActionRow.js"; +import { Button } from "../../ui/button/Button.js"; +import { Hint } from "../../ui/hint/Hint.js"; +import { TextField } from "../../ui/text-field/TextField.js"; +import { keyOf, keyProblem, keyStatus, type KeysBody } from "./KeyTypes.js"; +import type { Runtime } from "./SettingsTypes.js"; + +const KEYS = "/api/review-classifier/keys"; + +export function ClassifierKey({ runtime }: { runtime: Runtime }) { + const [keys, setKeys] = useState(null); + const [loadError, setLoadError] = useState(null); + const [saveError, setSaveError] = useState(null); + // The key as typed. Never kept after a save. + const [text, setText] = useState(""); + const [saving, setSaving] = useState(false); + const [justSaved, setJustSaved] = useState(false); + + useEffect(() => { + let cancelled = false; + void (async () => { + try { + const res = await fetch(KEYS); + if (cancelled) return; + if (!res.ok) { + setLoadError(await readApiError(res)); + return; + } + const body = (await res.json()) as KeysBody; + if (!cancelled) setKeys(body); + } catch (e: unknown) { + if (!cancelled) setLoadError({ detail: String(e) }); + } + })(); + return () => { + cancelled = true; + }; + }, []); + + // A key typed for one runtime must not be saved to the other. + useEffect(() => { + setText(""); + setSaveError(null); + setJustSaved(false); + }, [runtime]); + + const problem = keyProblem(text); + + async function save(e: Event) { + e.preventDefault(); + if (problem !== null) return; + setSaving(true); + setSaveError(null); + setJustSaved(false); + try { + const res = await fetch(`${KEYS}/${runtime}`, { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ api_key: text.trim() }), + }); + if (!res.ok) { + setSaveError(await readApiError(res)); + return; + } + setKeys((await res.json()) as KeysBody); + setText(""); + setJustSaved(true); + } catch (err: unknown) { + setSaveError({ detail: String(err) }); + } finally { + setSaving(false); + } + } + + const label = runtime === "acp" ? "provider key" : "Anthropic API key"; + return ( +
void save(e)}> + {loadError ? ( + + ) : null} + { + setText(t); + setJustSaved(false); + }} + /> + {saveError ? ( + + ) : null} + + + {justSaved ? ( + + Saved. The next run uses it. + + ) : null} + + + ); +} diff --git a/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.tsx b/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.tsx index 3b95934b1..cdb6e55db 100644 --- a/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.tsx +++ b/frontend/packages/swarm-ui/src/pages/review-settings/ClassifierSettings.tsx @@ -5,8 +5,8 @@ // fields it carries: the form sends the runtime, model, effort and limit, and // the ACP provider only while the runtime is ACP, so the stored provider // survives a switch back. A refused save shows the route's own reason and -// leaves the form as typed. There is no API key field: admins set each -// runtime's key in the bao UI. +// leaves the form as typed. The runtime's key has a form of its own below +// (), saved apart because it goes to the secret store. 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"; @@ -18,6 +18,7 @@ import { Disclosure } from "../../ui/disclosure/Disclosure.js"; import { CheckboxField } from "../../ui/checkbox-field/CheckboxField.js"; import { SelectField } from "../../ui/select-field/SelectField.js"; import { TextField } from "../../ui/text-field/TextField.js"; +import { ClassifierKey } from "./ClassifierKey.js"; import { EFFORTS, MAX_REVIEWS, @@ -137,8 +138,9 @@ export function ClassifierSettings() { many requested reviews run at once. The API key runtime runs claude on the Anthropic key; the ACP runtime runs opencode on the provider given here. A change takes effect from the next run, with no restart. Try-outs - don't count toward the limit. Each runtime's key is set in the bao UI, - not here. + don't count toward the limit. The key of the chosen runtime is saved + below, apart from the settings. The page shows whether a key is stored, + never the key.

{loadError ? ( {problem} : null} ) : null} + {draft ? : null} ); } diff --git a/frontend/packages/swarm-ui/src/pages/review-settings/KeyTypes.ts b/frontend/packages/swarm-ui/src/pages/review-settings/KeyTypes.ts new file mode 100644 index 000000000..7e5c87f82 --- /dev/null +++ b/frontend/packages/swarm-ui/src/pages/review-settings/KeyTypes.ts @@ -0,0 +1,45 @@ +// Wire shape of the review classifier's keys +// (`GET /api/review-classifier/keys`, `PUT .../keys/{runtime}`), and the +// checks the key form makes before a save is worth sending. The routes never +// answer a key, only whether one is stored; the backend stays the real gate. + +import type { Runtime } from "./SettingsTypes.js"; + +/** Whether one runtime's key is stored. */ +export interface KeyState { + set: boolean; + /** When it last changed (RFC 3339), or `null` when none is stored. */ + updated: string | null; +} + +/** Both keys' state, as both routes answer it. */ +export interface KeysBody { + api_key: KeyState; + acp: KeyState; +} + +/** The most bytes a key may have, as the route takes it. */ +export const MAX_KEY_BYTES = 4096; + +/** The state of the key `runtime` runs on. */ +export function keyOf(keys: KeysBody, runtime: Runtime): KeyState { + return runtime === "acp" ? keys.acp : keys.api_key; +} + +/** Why `text` is not ready to save as a key, or null when it is. A pasted + * trailing newline is fine: the route drops it. */ +export function keyProblem(text: string): string | null { + const key = text.trim(); + if (key === "") return "paste the key"; + if (/\s/.test(key)) return "a key has no whitespace in it"; + if (new TextEncoder().encode(key).length > MAX_KEY_BYTES) { + return `a key is at most ${MAX_KEY_BYTES} bytes`; + } + return null; +} + +/** One line on whether the key is stored, for the key form. */ +export function keyStatus(state: KeyState): string { + if (!state.set) return "not set"; + return state.updated ? `set, last changed ${state.updated}` : "set"; +} diff --git a/frontend/packages/swarm-ui/test/classifier-key.test.mjs b/frontend/packages/swarm-ui/test/classifier-key.test.mjs new file mode 100644 index 000000000..399eb5a3a --- /dev/null +++ b/frontend/packages/swarm-ui/test/classifier-key.test.mjs @@ -0,0 +1,47 @@ +// The classifier key form's pure logic: which key a runtime means, the checks +// before a save, and the line saying whether a key is stored. Run by +// `npm test`, which Node runs with its built-in test runner and type +// stripping, so the `.ts` module is imported as it is. +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { + MAX_KEY_BYTES, + keyOf, + keyProblem, + keyStatus, +} from "../src/pages/review-settings/KeyTypes.ts"; + +const keys = { + api_key: { set: true, updated: "2026-11-11T10:00:00Z" }, + acp: { set: false, updated: null }, +}; + +test("a runtime means its own key", () => { + assert.equal(keyOf(keys, "api-key"), keys.api_key); + assert.equal(keyOf(keys, "acp"), keys.acp); +}); + +test("an empty key or one with whitespace is not ready to save", () => { + assert.notEqual(keyProblem(""), null); + assert.notEqual(keyProblem(" "), null); + assert.notEqual(keyProblem("two words"), null); + assert.equal(keyProblem("sk-ant-abc"), null); +}); + +test("a pasted trailing newline is fine", () => { + assert.equal(keyProblem("sk-ant-abc\n"), null); +}); + +test("a key over the limit is not ready to save", () => { + assert.equal(keyProblem("k".repeat(MAX_KEY_BYTES)), null); + assert.notEqual(keyProblem("k".repeat(MAX_KEY_BYTES + 1)), null); +}); + +test("the status says whether a key is stored and when it changed", () => { + assert.equal(keyStatus(keys.acp), "not set"); + assert.equal( + keyStatus(keys.api_key), + "set, last changed 2026-11-11T10:00:00Z", + ); + assert.equal(keyStatus({ set: true, updated: null }), "set"); +}); diff --git a/nix/host-modules/deploy.nix b/nix/host-modules/deploy.nix index f7ee82496..6c4eb02b2 100644 --- a/nix/host-modules/deploy.nix +++ b/nix/host-modules/deploy.nix @@ -638,7 +638,7 @@ in description = '' Run the review classifier in a `swarm-review-classifier` container on this host. It answers the swarm UI's review try-outs with `claude` on - the Anthropic API key the operator writes to the secret store at + the Anthropic API key the operator sets in the swarm UI, or writes to the secret store at `secret/swarm/services/review-classifier/anthropic-api-key`, field `api_key`. diff --git a/nix/host-modules/swarm-bao.nix b/nix/host-modules/swarm-bao.nix index c1a86c2ee..41b89091d 100644 --- a/nix/host-modules/swarm-bao.nix +++ b/nix/host-modules/swarm-bao.nix @@ -562,6 +562,22 @@ let capabilities = ["create", "read", "update"] } + path "${credentialMountPath}/data/${classifierApiKeyLeaf}" { + capabilities = ["create", "update"] + } + + path "${credentialMountPath}/metadata/${classifierApiKeyLeaf}" { + capabilities = ["read"] + } + + path "${credentialMountPath}/data/${classifierAcpKeyLeaf}" { + capabilities = ["create", "update"] + } + + path "${credentialMountPath}/metadata/${classifierAcpKeyLeaf}" { + capabilities = ["read"] + } + path "${credentialMountPath}/data/${builderForgeTokenLeaf}" { capabilities = ["create", "read", "update"] } @@ -775,7 +791,10 @@ let classifierForgeTokenLeaf = "swarm/services/review-classifier/forge/token"; # The review classifier's Anthropic API key. The operator writes it through - # the UI's OIDC login; the classifier reads it on every run. + # the UI's OIDC login, or the swarm UI's classifier settings, which the + # controller writes for them: `create` and `update` with no `read`, and a + # metadata `read` for whether one is set and when it changed. The classifier + # reads it on every run. classifierApiKeyLeaf = "swarm/services/review-classifier/anthropic-api-key"; # The swarm builder's forge token, the nix half of diff --git a/nix/module-eval/bao-grants.nix b/nix/module-eval/bao-grants.nix index 8e722d434..c137dc064 100644 --- a/nix/module-eval/bao-grants.nix +++ b/nix/module-eval/bao-grants.nix @@ -1934,15 +1934,18 @@ let # `forge::classifier_account` mints the review classifier's forge token # and reads it back to decide on a re-mint and to post reviews. Pinned # as the whole stanza, and spelled to the leaf, for the reason above. - name = "the controller's grant on the review classifier is its forge token alone"; + name = "the controller's grant on the review classifier is its forge token, and a write-only inference key each"; ok = let s = baoGrantHere.systemd.services.swarm-bao-controller-policy.script; + writeOnly = leaf: [ + "path \"secret/data/swarm/services/review-classifier/${leaf}\" {\n capabilities = [\"create\", \"update\"]\n}" + "path \"secret/metadata/swarm/services/review-classifier/${leaf}\" {\n capabilities = [\"read\"]\n}" + ]; in lib.hasInfix "path \"secret/data/swarm/services/review-classifier/forge/token\" {\n capabilities = [\"create\", \"read\", \"update\"]\n}" s - && !(lib.hasInfix "secret/data/swarm/services/review-classifier/*" s) - && !(lib.hasInfix "secret/data/swarm/services/review-classifier/anthropic-api-key" s) - && !(lib.hasInfix "secret/data/swarm/services/review-classifier/acp-provider" s); + && lib.all (p: lib.hasInfix p s) (writeOnly "anthropic-api-key" ++ writeOnly "acp-provider") + && !(lib.hasInfix "secret/data/swarm/services/review-classifier/*" s); } { # The runner's role is the token's only reader. Its policy is that leaf diff --git a/swarm-controller/src/review_classifier.rs b/swarm-controller/src/review_classifier.rs index 057436ac1..06cb88c6b 100644 --- a/swarm-controller/src/review_classifier.rs +++ b/swarm-controller/src/review_classifier.rs @@ -40,6 +40,7 @@ use self::settings::{Settings, SettingsSource}; use crate::forge::{self, DefaultBranchFiles, PullForReview}; use crate::routes::{AppState, error_problem}; +pub(crate) mod keys; pub(crate) mod metrics; pub(crate) mod open_pr; pub(crate) mod review; diff --git a/swarm-controller/src/review_classifier/keys.rs b/swarm-controller/src/review_classifier/keys.rs new file mode 100644 index 000000000..ea5c47423 --- /dev/null +++ b/swarm-controller/src/review_classifier/keys.rs @@ -0,0 +1,323 @@ +//! `GET /api/review-classifier/keys` and `PUT /api/review-classifier/keys/{runtime}`: +//! the keys the classifier's two runtimes run on, written from the swarm UI +//! instead of by hand in the secret store's UI. +//! +//! The controller only writes. Its store policy holds `create` and `update` +//! on the two leaves and no `read` of their values, so no route here answers a +//! key, and the one metadata read tells whether a key is set and when it last +//! changed. The classifier reads the key from the same leaf on every run, so +//! a new key applies from the next run with no restart. + +use std::future::Future; + +use axum::Json; +use axum::extract::rejection::JsonRejection; +use axum::extract::{Path, State}; +use axum::http::{HeaderMap, StatusCode}; +use problem_details::ProblemDetails; +use serde::{Deserialize, Serialize}; +use swarm_secret_client::{ + SecretStore, + acp::ProviderKey, + review_classifier::{ApiKey, acp_provider_key_path, api_key_path}, +}; +use utoipa::ToSchema; + +use crate::routes::{AppState, error_problem}; + +/// The most bytes a key may have. Provider keys run to a few hundred. +const MAX_KEY_BYTES: usize = 4096; + +/// Which runtime a key is for, as the path spells it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Which { + /// The Anthropic API key the `api-key` runtime runs `claude` with. + ApiKey, + /// The provider key the `acp` runtime runs its agent with. + Acp, +} + +impl Which { + fn parse(s: &str) -> Option { + match s { + "api-key" => Some(Self::ApiKey), + "acp" => Some(Self::Acp), + _ => None, + } + } + + fn path(self) -> Result { + match self { + Self::ApiKey => api_key_path(), + Self::Acp => acp_provider_key_path(), + } + } +} + +/// Whether one key is stored. Never the key. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, ToSchema)] +pub(crate) struct KeyState { + /// A key is stored. + set: bool, + /// When the key last changed, as the store spells it (RFC 3339), or + /// `null` when none is stored. + updated: Option, +} + +/// Both keys' state, what both routes answer. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, ToSchema)] +pub(crate) struct Keys { + /// The Anthropic API key of the `api-key` runtime. + api_key: KeyState, + /// The provider key of the `acp` runtime. + acp: KeyState, +} + +/// The body of a `PUT`: the key. +#[derive(Deserialize, ToSchema)] +#[serde(deny_unknown_fields)] +pub(crate) struct KeyBody { + /// The key. Surrounding whitespace is dropped, so a pasted trailing + /// newline is harmless. + api_key: String, +} + +/// The two store calls a route makes, so a test can stand in for the store. +trait KeyStore { + /// Store `key` at `path`, replacing what's there. + fn put(&self, which: Which, key: &str) -> impl Future> + Send; + + /// When the leaf last changed, or `None` when nothing is stored. + fn updated(&self, which: Which) -> impl Future, String>> + Send; +} + +impl KeyStore for SecretStore { + async fn put(&self, which: Which, key: &str) -> Result<(), String> { + let path = which.path().map_err(|e| e.to_string())?; + let result = match which { + Which::ApiKey => { + self.write( + &path, + &ApiKey { + api_key: key.to_owned(), + }, + ) + .await + } + Which::Acp => { + self.write( + &path, + &ProviderKey { + api_key: key.to_owned(), + }, + ) + .await + } + }; + result.map_err(|e| format!("writing {path}: {e}")) + } + + async fn updated(&self, which: Which) -> Result, String> { + let path = which.path().map_err(|e| e.to_string())?; + self.updated_time(&path) + .await + .map_err(|e| format!("reading {path}'s metadata: {e}")) + } +} + +/// A key to store, or why it's refused. +fn check(key: &str) -> Result<&str, String> { + let key = key.trim(); + if key.is_empty() { + return Err("the key is empty".to_owned()); + } + if key.len() > MAX_KEY_BYTES { + return Err(format!("the key is over {MAX_KEY_BYTES} bytes")); + } + if key.chars().any(|c| c.is_whitespace() || c.is_control()) { + return Err("the key holds whitespace or a control character".to_owned()); + } + Ok(key) +} + +async fn state_of(store: &impl KeyStore, which: Which) -> Result { + let updated = store.updated(which).await?; + Ok(KeyState { + set: updated.is_some(), + updated, + }) +} + +async fn both(store: &impl KeyStore) -> Result { + Ok(Keys { + api_key: state_of(store, Which::ApiKey).await?, + acp: state_of(store, Which::Acp).await?, + }) +} + +fn store_problem(why: &str) -> ProblemDetails { + tracing::warn!(error = %why, "review classifier keys: store failure"); + error_problem( + StatusCode::SERVICE_UNAVAILABLE, + &format!("the secret store refused: {why}"), + ) +} + +async fn connect() -> Result { + crate::store::connect() + .await + .map_err(|e| store_problem(&format!("logging in: {e}"))) +} + +/// Whether each runtime's key is stored. +#[utoipa::path( + get, + path = "/api/review-classifier/keys", + responses( + (status = 200, description = "whether each key is stored and when it last changed; \ + never a key", body = Keys), + (status = 503, description = "the secret store couldn't be reached, or refused \ + (problem+json)", body = String), + ), + tag = "review-classifier" +)] +pub(crate) async fn get_keys(State(_state): State) -> Result, ProblemDetails> { + let store = connect().await?; + both(&store).await.map(Json).map_err(|e| store_problem(&e)) +} + +/// Store a runtime's key. +#[utoipa::path( + put, + path = "/api/review-classifier/keys/{runtime}", + params(("runtime" = String, Path, description = "`api-key` or `acp`")), + request_body = KeyBody, + responses( + (status = 200, description = "both keys' state after the write; never a key", + body = Keys), + (status = 400, description = "not JSON or not this shape, a runtime other than \ + `api-key` or `acp`, or a key that is empty, over 4096 bytes or holds \ + whitespace (problem+json)", body = String), + (status = 401, description = "the request carries no `Remote-User`, so there is \ + no operator to name (problem+json)", body = String), + (status = 503, description = "the secret store couldn't be reached, or refused \ + (problem+json)", body = String), + ), + tag = "review-classifier" +)] +pub(crate) async fn put_key( + State(_state): State, + headers: HeaderMap, + Path(runtime): Path, + body: Result, JsonRejection>, +) -> Result, ProblemDetails> { + let operator = super::operator(&headers).ok_or_else(|| { + error_problem( + StatusCode::UNAUTHORIZED, + "no Remote-User header: the gateway names the operator, and this request did not \ + come through it", + ) + })?; + let which = Which::parse(&runtime).ok_or_else(|| { + error_problem( + StatusCode::BAD_REQUEST, + &format!("{runtime:?} is not a runtime with a key: use `api-key` or `acp`"), + ) + })?; + let Json(body) = body.map_err(|e| error_problem(StatusCode::BAD_REQUEST, &e.body_text()))?; + let key = check(&body.api_key).map_err(|why| error_problem(StatusCode::BAD_REQUEST, &why))?; + let store = connect().await?; + store.put(which, key).await.map_err(|e| store_problem(&e))?; + // The key itself is never logged. + tracing::info!(%operator, ?which, "set a review classifier key"); + both(&store).await.map(Json).map_err(|e| store_problem(&e)) +} + +#[cfg(test)] +mod tests { + use std::sync::Mutex; + + use super::*; + + /// A store holding what was put, with a leaf's time when it has a key. + #[derive(Default)] + struct Fake { + put: Mutex>, + refuse: bool, + } + + impl KeyStore for Fake { + async fn put(&self, which: Which, key: &str) -> Result<(), String> { + if self.refuse { + return Err("denied".to_owned()); + } + self.put.lock().unwrap().push((which, key.to_owned())); + Ok(()) + } + + async fn updated(&self, which: Which) -> Result, String> { + if self.refuse { + return Err("denied".to_owned()); + } + let put = self.put.lock().unwrap(); + Ok(put + .iter() + .any(|(w, _)| *w == which) + .then(|| "2026-11-11T10:00:00Z".to_owned())) + } + } + + #[test] + fn a_runtime_is_one_of_two_words() { + assert_eq!(Which::parse("api-key"), Some(Which::ApiKey)); + assert_eq!(Which::parse("acp"), Some(Which::Acp)); + assert_eq!(Which::parse("api_key"), None); + assert_eq!(Which::parse(""), None); + } + + #[test] + fn the_leaves_are_the_ones_the_policy_grants() { + assert_eq!( + Which::ApiKey.path().unwrap(), + "swarm/services/review-classifier/anthropic-api-key" + ); + assert_eq!( + Which::Acp.path().unwrap(), + "swarm/services/review-classifier/acp-provider" + ); + } + + #[test] + fn a_pasted_newline_is_dropped_and_a_bad_key_refused() { + assert_eq!(check("sk-ant-x\n"), Ok("sk-ant-x")); + assert!(check(" ").is_err()); + assert!(check("two words").is_err()); + assert!(check("a\u{0}b").is_err()); + assert!(check(&"k".repeat(MAX_KEY_BYTES + 1)).is_err()); + assert!(check(&"k".repeat(MAX_KEY_BYTES)).is_ok()); + } + + #[tokio::test] + async fn the_state_says_set_once_a_key_is_stored_and_never_holds_it() { + let store = Fake::default(); + let before = both(&store).await.unwrap(); + assert!(!before.api_key.set && !before.acp.set); + assert_eq!(before.api_key.updated, None); + + store.put(Which::ApiKey, "sk-secret").await.unwrap(); + let after = both(&store).await.unwrap(); + assert!(after.api_key.set && !after.acp.set); + assert!(after.api_key.updated.is_some()); + let json = serde_json::to_string(&after).unwrap(); + assert!(!json.contains("sk-secret"), "{json}"); + } + + #[tokio::test] + async fn a_refusing_store_is_an_error_not_unset() { + let store = Fake { + refuse: true, + ..Fake::default() + }; + assert!(both(&store).await.is_err()); + } +} diff --git a/swarm-controller/src/startup.rs b/swarm-controller/src/startup.rs index 39555dff5..3b7f94d47 100644 --- a/swarm-controller/src/startup.rs +++ b/swarm-controller/src/startup.rs @@ -702,6 +702,8 @@ pub(crate) fn build_app(state: AppState) -> axum::Router { review_classifier::settings::get_settings, review_classifier::settings::put_settings )) + .routes(routes!(review_classifier::keys::get_keys)) + .routes(routes!(review_classifier::keys::put_key)) .routes(routes!(notify_flows::get_flows, notify_flows::put_flows)) .routes(routes!(notify_router_api::get_deliveries)) .routes(routes!( diff --git a/swarm-secret-client/src/review_classifier.rs b/swarm-secret-client/src/review_classifier.rs index bcb5ac6d5..1aae44bc3 100644 --- a/swarm-secret-client/src/review_classifier.rs +++ b/swarm-secret-client/src/review_classifier.rs @@ -2,7 +2,8 @@ //! client secret and its forge token live in the store, and what each object //! holds. //! -//! The operator writes the keys by hand in the store's UI; the classifier +//! The operator writes the keys in the swarm UI, which `swarm-controller` +//! stores for them, or by hand in the store's UI; the classifier //! reads them and the client secret under its own certificate, a key once per //! run. Its read stanza in `swarm-bao.nix` names exactly these three paths. The forge token is `swarm-controller`'s alone: it mints it, and //! posts the classifier's reviews with it.