diff --git a/Cargo.lock b/Cargo.lock index ad8ee01a..f5c30ef2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4605,10 +4605,12 @@ dependencies = [ "hive-jobq", "hive-jobq-wire", "hive-types", + "hmac 0.13.0", "problem_details", "reqwest 0.13.1", "serde", "serde_json", + "sha2 0.11.0", "swarm-authelia-bridge-sock", "swarm-queue-client", "tokio", diff --git a/swarm-controller/Cargo.toml b/swarm-controller/Cargo.toml index 6ac33c60..cb7e0419 100644 --- a/swarm-controller/Cargo.toml +++ b/swarm-controller/Cargo.toml @@ -38,6 +38,12 @@ problem_details = { version = "0.9.0", features = ["axum"] } # same shape `hive-c0re/src/job_queue/scheduler.rs` uses over its own graph. hive-jobq.workspace = true hive-jobq-wire.workspace = true +# The forge webhook HMAC (`webhook.rs`). Kept in this crate rather than +# shared with hive-c0re's equivalent: c0re's copy is scheduled to be deleted +# with its webhook routes once registration moves here, so the second holder +# is departing, not arriving — see that module's docs. +hmac.workspace = true +sha2.workspace = true # Validates `POST /api/agents`' `name` before it becomes `agent`/`repo` # everywhere downstream — see `create_agent`'s doc comment for why this is # defense-in-depth, not the only gate (per an argus review finding). diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index f0a9841d..08ae0e25 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -35,6 +35,7 @@ use utoipa_axum::{router::OpenApiRouter, routes}; mod auth; mod forge; mod status; +mod webhook; /// Node payload for the swarm-level job graph. Named `Swarm*` rather than /// the bare `NodeKind`/`Resource` `hive-c0re::job_queue::model` already @@ -273,6 +274,7 @@ fn socket_path() -> PathBuf { (name = "links", description = "swarm service quick links"), (name = "jobq", description = "the swarm-level job graph"), (name = "agents", description = "creating agent identities at swarm level"), + (name = "webhook", description = "swarm-wide forge webhook receipt"), ) )] struct ApiDoc; @@ -329,6 +331,17 @@ struct AppState { /// claimed — same "queue now, fail per-job" shape as an unreachable /// swarm queue. jobq: Arc>>, + /// HMAC secret for swarm-wide forge webhooks, loaded once at startup. + /// `None` when it could not be read or created — the webhook endpoint + /// then refuses every delivery with 503 rather than admitting one it + /// cannot verify. Deliberately not fatal to startup: nothing is + /// registered against that endpoint yet, and the rest of this daemon's + /// surface is unaffected. See [`webhook::load_or_generate_secret`]. + /// + /// `Arc`, not `Arc`: the value is never mutated after + /// startup, and this way `as_deref()` yields the `&str` the verifier + /// takes without a second hop through `String`. + webhook_secret: Option>, } /// Env var the controller's NixOS module sets from @@ -756,11 +769,27 @@ async fn main() -> Result<()> { ))); spawn_jobq_worker(Arc::clone(&jobq), deps); + // Same "log and carry on" shape as the queue/bridge/forge wiring above. + // A controller that cannot hold a webhook secret still serves every + // other route; the webhook endpoint answers 503, which is the honest + // answer rather than a silent accept. + let webhook_secret = match webhook::load_or_generate_secret() { + Ok(secret) => Some(Arc::from(secret)), + Err(e) => { + tracing::warn!( + error = %format!("{e:#}"), + "webhook secret unavailable; swarm-wide forge webhooks are off" + ); + None + } + }; + let state = AppState { hives: Arc::new(load_hives()), links: Arc::new(load_links()), status, jobq, + webhook_secret, }; let (router, api) = OpenApiRouter::::with_openapi(ApiDoc::openapi()) @@ -771,6 +800,7 @@ async fn main() -> Result<()> { .routes(routes!(get_jobq_graph)) .routes(routes!(get_jobq_rollup)) .routes(routes!(create_agent)) + .routes(routes!(webhook::post_webhook_forge)) .split_for_parts(); // Just the JSON, not the UI — Swagger UI itself is nginx-hosted from // the nix store (see the module doc comment above). `api` is diff --git a/swarm-controller/src/webhook.rs b/swarm-controller/src/webhook.rs new file mode 100644 index 00000000..e8b5da48 --- /dev/null +++ b/swarm-controller/src/webhook.rs @@ -0,0 +1,512 @@ +//! Forgejo webhook receipt at the swarm level. +//! +//! A Forgejo webhook has exactly **one** `target_url`, so every hive +//! registering the same swarm-wide hooks is last-writer-wins rather than +//! idempotent, and every hive but the most recent silently stops receiving +//! deliveries. One owner is the only non-racing shape, and the only +//! swarm-wide thing in the deployment is this daemon. +//! +//! **This relays; it does not interpret.** The payload stays opaque bytes, +//! keyed by a [`DeliveryKind`] from the URL path. The hives already know +//! what a delivery *means* — which repo is the knowledge repo, which config +//! PR action queues an approval — so parsing it here would be a second place +//! deciding that, and the two would drift. +//! +//! **The HMAC code is not shared with `hive-c0re` because c0re's copy is +//! leaving, not staying.** Once registration moves here, hives stop +//! registering *and* receiving; c0re's routes, its secret and the gateway's +//! `/webhook/` route all go with it, and what survives there is driven by a +//! queue event, authenticated by the queue. A shared crate is right when a +//! second consumer *arrives* — here it is departing, so extracting would +//! build an abstraction in order to unwind it. The two verifiers meanwhile +//! check different hooks with different secrets and never need to agree. +//! +//! **Nothing is registered against these routes yet, on purpose.** The +//! replacement path is built and observable in full before anything takes +//! the old one away: delete a hive's registration first and the swarm's one +//! `target_url` points at a receiver that forwards nowhere — indistinguishable +//! from no activity, on both sides. + +use anyhow::{Context as _, Result}; +use axum::{ + body::Bytes, + extract::{Path, State}, + http::{HeaderMap, StatusCode}, + response::{IntoResponse as _, Response}, +}; +use hmac::{Hmac, KeyInit as _, Mac as _}; +use sha2::Sha256; + +use super::AppState; + +/// Where the HMAC secret is kept, relative to the daemon's state directory. +/// +/// The secret is generated on first start and handed to Forgejo when the +/// hook is registered, so it must survive a restart — a rotated secret +/// would make every subsequent delivery fail verification with a correctly +/// configured hook on both sides. +const SECRET_FILE: &str = "webhook-secret"; + +/// Fallback state directory, used only when `STATE_DIRECTORY` is unset (a +/// dev run outside systemd). Under the unit it is always set, because +/// `StateDirectory = "swarm-controller"` is declared there. +const DEFAULT_STATE_DIR: &str = "/var/lib/swarm-controller"; + +/// Path to the HMAC secret file. +/// +/// Reads systemd's `STATE_DIRECTORY` rather than introducing a +/// `SWARM_CONTROLLER_*` env var for it: the unit already declares +/// `StateDirectory=`, and a second env var would be a second declaration of +/// the same fact, free to drift from the first. +/// +/// ⚠️ `StateDirectory=` accepts a *list*, in which case systemd exports the +/// paths colon-separated. Only the first is taken — a bare `PathBuf::from` +/// of the whole variable would silently produce a path containing a colon +/// the day someone adds a second directory to the unit. +fn secret_path() -> std::path::PathBuf { + secret_path_from(std::env::var("STATE_DIRECTORY").ok().as_deref()) +} + +/// The pure half of [`secret_path`], split out so the colon-list handling is +/// testable **without touching the environment**. +/// +/// Not a stylistic split: `std::env::set_var` mutates process-global state, +/// and cargo runs a crate's tests on parallel threads in one process, so two +/// tests setting it race — which is not hypothetical here, it is how the +/// first version of this module's tests failed. +fn secret_path_from(raw: Option<&str>) -> std::path::PathBuf { + let dir = raw + .and_then(|raw| raw.split(':').next()) + .filter(|first| !first.is_empty()) + .unwrap_or(DEFAULT_STATE_DIR); + std::path::PathBuf::from(dir).join(SECRET_FILE) +} + +/// Load the swarm's webhook HMAC secret, generating and persisting it if the +/// file is absent or malformed. +/// +/// Returns a hex-encoded 32-byte secret (64 hex chars). +/// +/// # Errors +/// +/// Returns an error when the secret cannot be generated (no `/dev/urandom`) +/// or cannot be persisted. A malformed existing file is **not** an error: it +/// is regenerated, because a secret that cannot be parsed is +/// indistinguishable from one that was never written, and failing startup +/// over it would strand the daemon with no path forward. +pub(super) fn load_or_generate_secret() -> Result { + load_or_generate_at(&secret_path()) +} + +/// The path-taking half of [`load_or_generate_secret`], split out for the +/// same reason as [`secret_path_from`]: a test can point it at a scratch +/// file without mutating a process-global env var. +fn load_or_generate_at(path: &std::path::Path) -> Result { + if let Ok(raw) = std::fs::read_to_string(path) { + let trimmed = raw.trim().to_owned(); + if trimmed.len() == 64 && trimmed.chars().all(|c| c.is_ascii_hexdigit()) { + return Ok(trimmed); + } + tracing::warn!( + path = %path.display(), + "webhook-secret file malformed (wrong length/chars); regenerating" + ); + } + let secret = generate_hex_secret()?; + std::fs::create_dir_all(path.parent().unwrap_or(path)) + .with_context(|| format!("create dir for {}", path.display()))?; + std::fs::write(path, format!("{secret}\n")) + .with_context(|| format!("write webhook secret to {}", path.display()))?; + tracing::info!(path = %path.display(), "webhook secret generated and persisted"); + Ok(secret) +} + +/// Read 32 random bytes from `/dev/urandom` and hex-encode them. +fn generate_hex_secret() -> Result { + use std::io::Read as _; + + let mut buf = [0u8; 32]; + let mut f = + std::fs::File::open("/dev/urandom").context("open /dev/urandom for secret generation")?; + f.read_exact(&mut buf) + .context("read 32 bytes from /dev/urandom")?; + Ok(hex_encode(&buf)) +} + +/// Hex-encode `bytes` as a lowercase string. +fn hex_encode(bytes: &[u8]) -> String { + let mut out = String::with_capacity(bytes.len() * 2); + for b in bytes { + out.push(char::from_digit(u32::from(b >> 4), 16).unwrap_or('0')); + out.push(char::from_digit(u32::from(b & 0xf), 16).unwrap_or('0')); + } + out +} + +/// Decode a lowercase hex string into bytes; returns `None` on invalid input. +fn hex_decode(s: &str) -> Option> { + if !s.len().is_multiple_of(2) { + return None; + } + let mut out = Vec::with_capacity(s.len() / 2); + let mut chars = s.chars(); + while let (Some(hi), Some(lo)) = (chars.next(), chars.next()) { + let hi = u8::try_from(hi.to_digit(16)?).ok()?; + let lo = u8::try_from(lo.to_digit(16)?).ok()?; + out.push((hi << 4) | lo); + } + Some(out) +} + +/// Which swarm-wide hook a delivery arrived on. +/// +/// The kind comes from the URL path rather than from Forgejo's +/// `X-Forgejo-Event` header, so the routing key is decided by *the +/// registration we made* instead of by a field the sender chooses. Each +/// registered hook gets its own `target_url`, exactly as the per-hive hooks +/// do today. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) enum DeliveryKind { + /// Push events on the hive-wide knowledge repo. + Knowledge, + /// `pull_request` events on the agent-config repos. + ConfigPr, +} + +impl DeliveryKind { + /// Parse the `{kind}` path segment. Unknown values are rejected rather + /// than accepted-and-ignored: a typo in a registered `target_url` must + /// be *observable*, and a 200 for an unrecognised path is exactly the + /// silence this issue exists to remove. + fn parse(segment: &str) -> Option { + match segment { + "knowledge" => Some(Self::Knowledge), + "config-pr" => Some(Self::ConfigPr), + _ => None, + } + } + + /// Stable string form, used for logging and (once fan-out lands) as the + /// event's routing key. Deliberately the same spelling as the path + /// segment so a journal line can be matched against a registered URL. + fn as_str(self) -> &'static str { + match self { + Self::Knowledge => "knowledge", + Self::ConfigPr => "config-pr", + } + } +} + +/// Why a delivery was refused. +/// +/// A value rather than a string, so the status code is chosen by matching on +/// the reason instead of on message text — `hive-c0re`'s equivalent does +/// `e.contains("unavailable")`, which couples an HTTP response to the exact +/// wording of a log message. +#[derive(Debug, PartialEq, Eq)] +pub(super) enum Refusal { + /// No secret was loaded at startup: the endpoint cannot verify anything, + /// which is this daemon's fault and not the caller's. 503, so a retry is + /// meaningful. + SecretUnavailable, + /// The signature was missing, malformed, or did not match. 401. + BadSignature(String), +} + +impl Refusal { + fn status(&self) -> StatusCode { + match self { + Self::SecretUnavailable => StatusCode::SERVICE_UNAVAILABLE, + Self::BadSignature(_) => StatusCode::UNAUTHORIZED, + } + } + + fn message(&self) -> String { + match self { + Self::SecretUnavailable => { + "webhook HMAC secret unavailable; endpoint disabled".to_owned() + } + Self::BadSignature(detail) => detail.clone(), + } + } +} + +/// Verify the `X-Hub-Signature-256` header Forgejo attaches to a delivery +/// (`sha256=`). +/// +/// The body is the raw wire bytes, not a re-serialised structure: an HMAC is +/// over exactly what was sent, and any parse-then-reserialise step would +/// change whitespace or key order and fail every signature. +/// +/// The refusal messages are safe to log — they never contain the secret or +/// the expected digest, so a caller can surface *why* a delivery was refused +/// without handing an attacker the answer. +pub(super) fn verify( + secret: Option<&str>, + headers: &HeaderMap, + body: &Bytes, +) -> Result<(), Refusal> { + let secret = secret.ok_or(Refusal::SecretUnavailable)?; + + let sig = headers + .get("x-hub-signature-256") + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + if sig.is_empty() { + return Err(Refusal::BadSignature( + "missing X-Hub-Signature-256 header".to_owned(), + )); + } + let Some(sig_hex) = sig.strip_prefix("sha256=") else { + return Err(Refusal::BadSignature( + "X-Hub-Signature-256 missing 'sha256=' prefix".to_owned(), + )); + }; + let Some(expected) = hex_decode(sig_hex) else { + return Err(Refusal::BadSignature( + "X-Hub-Signature-256 contains non-hex chars".to_owned(), + )); + }; + + let mut mac = Hmac::::new_from_slice(secret.as_bytes()) + .map_err(|e| Refusal::BadSignature(format!("HMAC key error: {e}")))?; + mac.update(body); + // `verify_slice` is constant-time and also rejects a wrong-length digest, + // which is why the comparison is not written by hand here. + mac.verify_slice(&expected) + .map_err(|_| Refusal::BadSignature("X-Hub-Signature-256 mismatch".to_owned())) +} + +/// POST `/webhook/forge/{kind}` — a swarm-wide Forgejo delivery. +/// +/// Verifies the HMAC over the raw body, then (once fan-out lands) relays the +/// delivery to every hive over the swarm queue. The payload is never parsed +/// here — see the module docs. +/// +/// Returns 200 on an accepted delivery so Forgejo does not retry. A refused +/// one answers 401 (bad signature) or 503 (this daemon has no secret), and +/// an unknown `{kind}` answers 404 rather than a cheerful 200. +#[utoipa::path( + post, + path = "/webhook/forge/{kind}", + params( + ("kind" = String, Path, description = "which swarm-wide hook this delivery arrived on: \ + `knowledge` or `config-pr`") + ), + request_body( + content = String, + content_type = "application/json", + description = "Forgejo webhook payload, taken as raw bytes so HMAC \ + verification runs over the exact wire bytes and the \ + delivery can be relayed unmodified" + ), + responses( + (status = 200, description = "delivery accepted", body = String), + (status = 401, description = "bad or missing HMAC signature"), + (status = 404, description = "unknown hook kind in the path"), + (status = 503, description = "HMAC secret unavailable at startup"), + ), + tag = "webhook" +)] +pub(super) async fn post_webhook_forge( + State(state): State, + Path(kind): Path, + headers: HeaderMap, + body: Bytes, +) -> Response { + // Order matters: verify BEFORE looking at the path. An unauthenticated + // caller learning which `{kind}` values exist, from the difference + // between 404 and 401, is a small leak — but it is free to close. + if let Err(refusal) = verify(state.webhook_secret.as_deref(), &headers, &body) { + let message = refusal.message(); + tracing::warn!(%kind, %message, "webhook: refused delivery"); + return (refusal.status(), message).into_response(); + } + + let Some(kind) = DeliveryKind::parse(&kind) else { + tracing::warn!(%kind, "webhook: delivery on an unknown hook kind"); + return (StatusCode::NOT_FOUND, "unknown hook kind").into_response(); + }; + + tracing::info!( + kind = kind.as_str(), + bytes = body.len(), + "webhook: verified delivery" + ); + (StatusCode::OK, "ok").into_response() +} + +#[cfg(test)] +mod tests { + use super::{DeliveryKind, Refusal, load_or_generate_at, secret_path_from, verify}; + use axum::{body::Bytes, http::HeaderMap, http::StatusCode}; + use hmac::{Hmac, KeyInit as _, Mac as _}; + use sha2::Sha256; + use std::fmt::Write as _; + + /// Mint the header Forgejo would send. Written out rather than reusing + /// the verifier's internals so a test cannot pass by sharing a bug with + /// the code under test. + fn sign(secret: &str, body: &[u8]) -> String { + let mut mac = Hmac::::new_from_slice(secret.as_bytes()).expect("key"); + mac.update(body); + let mut hex = String::new(); + for byte in mac.finalize().into_bytes() { + write!(hex, "{byte:02x}").expect("writing to a String cannot fail"); + } + format!("sha256={hex}") + } + + fn headers_with(sig: &str) -> HeaderMap { + let mut h = HeaderMap::new(); + h.insert("x-hub-signature-256", sig.parse().expect("header value")); + h + } + + /// Each refusal reached separately, so a change that collapses them into + /// one always-refuse path is visible — and the accept case alongside + /// them, so a verifier that refuses *everything* cannot pass either. + #[test] + fn a_delivery_is_admitted_only_with_a_matching_signature() { + let body = Bytes::from_static(b"{\"action\":\"opened\"}"); + let good = sign("s3cret", &body); + + assert_eq!( + verify(Some("s3cret"), &headers_with(&good), &body), + Ok(()), + "a correctly signed delivery must be admitted" + ); + assert!( + matches!( + verify(Some("other"), &headers_with(&good), &body), + Err(Refusal::BadSignature(_)) + ), + "a signature minted with a different secret must be refused" + ); + assert!( + matches!( + verify( + Some("s3cret"), + &headers_with(&good), + &Bytes::from_static(b"tampered") + ), + Err(Refusal::BadSignature(_)) + ), + "a body that does not match the digest must be refused" + ); + assert!( + matches!( + verify( + Some("s3cret"), + &headers_with(good.trim_start_matches("sha256=")), + &body + ), + Err(Refusal::BadSignature(_)) + ), + "a bare hex digest with no 'sha256=' prefix is not what Forgejo sends" + ); + assert!( + matches!( + verify(Some("s3cret"), &HeaderMap::new(), &body), + Err(Refusal::BadSignature(_)) + ), + "a delivery with no signature header at all must be refused" + ); + assert_eq!( + verify(None, &headers_with(&good), &body), + Err(Refusal::SecretUnavailable), + "with no secret loaded the endpoint must refuse, not admit" + ); + } + + /// The status codes, asserted on the value rather than on message text. + #[test] + fn a_missing_secret_is_this_daemons_fault_and_a_bad_signature_is_the_callers() { + assert_eq!( + Refusal::SecretUnavailable.status(), + StatusCode::SERVICE_UNAVAILABLE, + "no secret is a server-side condition: a retry is meaningful" + ); + assert_eq!( + Refusal::BadSignature(String::new()).status(), + StatusCode::UNAUTHORIZED, + "a bad signature is the caller's, and a retry will not help" + ); + } + + /// An unknown segment must not parse. The whole point of the 404 is that + /// a mistyped `target_url` is observable rather than silently accepted. + #[test] + fn only_the_registered_hook_kinds_parse() { + assert_eq!( + DeliveryKind::parse("knowledge"), + Some(DeliveryKind::Knowledge) + ); + assert_eq!( + DeliveryKind::parse("config-pr"), + Some(DeliveryKind::ConfigPr) + ); + assert_eq!( + DeliveryKind::parse("config_pr"), + None, + "underscore is not the registered spelling; accepting both would make a journal line ambiguous" + ); + assert_eq!(DeliveryKind::parse(""), None); + assert_eq!(DeliveryKind::parse("../knowledge"), None); + } + + /// A malformed stored secret is replaced rather than fatal — and the + /// replacement is a *usable* secret that survives a reload, because a + /// version rotating on every call would pass a naive length check while + /// silently breaking every registration made with the previous value. + /// + #[test] + fn a_malformed_secret_file_is_regenerated_and_then_stable() { + let dir = std::env::temp_dir().join(format!("swarm-hook-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("mkdir"); + let path = dir.join("webhook-secret"); + std::fs::write(&path, "not-a-hex-secret\n").expect("seed"); + + let secret = load_or_generate_at(&path).expect("regenerates"); + let again = load_or_generate_at(&path).expect("second load"); + std::fs::remove_dir_all(&dir).ok(); + + assert_eq!(secret.len(), 64, "hex-encoded 32 bytes"); + assert!(secret.chars().all(|c| c.is_ascii_hexdigit())); + assert_eq!( + again, secret, + "a valid secret must be read back, not rotated" + ); + } + + /// `StateDirectory=` may name several directories, in which case systemd + /// exports them colon-separated — take the first, never the raw value. + /// + /// Asserted against the pure half, so this never touches the process + /// environment. The first version of these tests *did* use + /// `std::env::set_var`, and it failed: cargo runs a crate's tests on + /// parallel threads within **one** process, so the sibling env-setting + /// test removed the variable mid-run and the other silently fell back to + /// `/var/lib/swarm-controller`. + #[test] + fn the_secret_path_survives_a_multi_valued_state_directory() { + assert_eq!( + secret_path_from(Some("/var/lib/a:/var/lib/b")), + std::path::Path::new("/var/lib/a/webhook-secret") + ); + assert_eq!( + secret_path_from(Some("/var/lib/only")), + std::path::Path::new("/var/lib/only/webhook-secret") + ); + assert_eq!( + secret_path_from(None), + std::path::Path::new("/var/lib/swarm-controller/webhook-secret"), + "unset falls back to the directory the unit declares" + ); + assert_eq!( + secret_path_from(Some("")), + std::path::Path::new("/var/lib/swarm-controller/webhook-secret"), + "an empty value must not resolve to a relative path" + ); + } +}