diff --git a/Cargo.lock b/Cargo.lock index abcdcced..d95ae5b5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1673,7 +1673,6 @@ dependencies = [ "clap-markdown", "clap_complete", "forgejo-api", - "futures-util", "hive-agent-sock", "hive-core-agent-sock", "hive-host-sock", diff --git a/docs/knowledge.md b/docs/knowledge.md index 604ad8be..5f2b00d1 100644 --- a/docs/knowledge.md +++ b/docs/knowledge.md @@ -38,23 +38,18 @@ create a new document. hive-c0re maintains the local clone at `/var/lib/hyperhive/knowledge` via two paths: -1. **Swarm event** — the swarm controller holds the single push hook on - `internal/knowledge` (see `docs/swarm/README.md` § Swarm-wide forge - webhooks). On any push to main, including merge commits, it sends an - event to every hive over the swarm queue and each hive runs `git - pull`, so agents see the new content on their next turn. +1. **Forgejo push webhook** — `ensure_webhook` registers a push + hook on `internal/knowledge` at startup pointing at + `https:///webhook/knowledge` (routed through the + gateway, avoiding the Forgejo SSRF guard that blocks loopback + delivery). On any push to main (including merge commits) hive-c0re + runs `git pull` so agents see the new content on their next turn. + The endpoint is protected by an auto-generated HMAC secret that + hive-c0re verifies on every delivery. - A hive that is offline when the event is sent does not get it on - reconnect — the periodic pull below is what closes that gap. So one - hive briefly showing older `/knowledge` content than another is - expected, and resolves by itself within the fallback interval. - - **Do not add a per-hive hook.** A webhook has exactly one target - URL, so a second registration against the same repo does not add a - recipient — it takes delivery away from whoever registered first. - Earlier versions had each hive register its own; hive-c0re now - removes its own leftover at startup, so no operator step is needed - to migrate. + On a swarm, a **second** hook on the same repo points at the + swarm controller — see `docs/swarm/README.md` § Swarm-wide forge + webhooks. Both are expected; neither should be deleted. 2. **Periodic pull** — a background task in `hive-c0re::main` pulls on a fixed cadence as a fallback (webhook missed, c0re diff --git a/docs/swarm/README.md b/docs/swarm/README.md index 853a44cc..2338b0fd 100644 --- a/docs/swarm/README.md +++ b/docs/swarm/README.md @@ -356,23 +356,18 @@ itself — a `push` hook on `internal/knowledge` and a `pull_request` hook on the `agent-configs` org, both under `https:///webhook/forge/`. +**Two hooks exist per swarm-wide repo: each hive's own, plus the +controller's.** Both are expected — **do not delete either.** Removing a +hive's hook stops that hive acting on knowledge pushes and config PRs; +removing the controller's just gets recreated on its next start. + The controller **interprets** a delivery and sends hives a specific message — *the knowledge repo changed*, *deploy agent `foo` at rev `abc123`* — rather than forwarding forge payloads for each hive to re-derive. Approval happens once, at the swarm level: a hive receives a -decision, not an event to adjudicate. - -**`internal/knowledge` is on that path.** The controller's is the only -hook on it: hives no longer register their own, and each removes its -leftover at startup. A webhook has exactly one target URL, so per-hive -registration never added a recipient — it took delivery away from -whichever hive registered before it. - -**The `agent-configs` org is not yet.** Each hive still registers its own -`pull_request` hook there, so that repo has two — the hive's and the -controller's — and **both are expected; do not delete either.** Removing -a hive's stops it acting on config PRs; removing the controller's just -gets recreated on its next start. +decision, not an event to adjudicate. Today the controller logs each +verified delivery and sends nothing, because the swarm→hive channel does +not exist yet; the hive-side hooks are what act in the meantime. Nothing to configure. The hooks are registered only when this host also serves the swarm UI vhost — that is what publishes the endpoint, and a diff --git a/hive-c0re/Cargo.toml b/hive-c0re/Cargo.toml index 68440cc2..6ffc9549 100644 --- a/hive-c0re/Cargo.toml +++ b/hive-c0re/Cargo.toml @@ -8,9 +8,6 @@ readme = "README.md" workspace = true [dependencies] -# For `StreamExt::next` on the swarm-event subscription in `swarm_status`. -# Workspace-level, same version swarm-controller already uses — not a second copy. -futures-util.workspace = true anyhow.workspace = true # Named directly only for the client type the swarm status publisher passes # around; the connect itself lives in `swarm-queue-client` below. diff --git a/hive-c0re/src/dashboard/mod.rs b/hive-c0re/src/dashboard/mod.rs index 1a820d13..9fc491b2 100644 --- a/hive-c0re/src/dashboard/mod.rs +++ b/hive-c0re/src/dashboard/mod.rs @@ -184,6 +184,7 @@ pub async fn serve( .routes(routes!(schedules::post_schedule_resume)) .routes(routes!(schedules::post_schedule_fire_now)) .routes(routes!(schedules::post_rebuild_queue_cancel)) + .routes(routes!(webhook::post_webhook_knowledge)) .routes(routes!(webhook::post_webhook_config_pr)) .routes(routes!(approvals::post_approve)) .routes(routes!(approvals::post_deny)) @@ -429,6 +430,7 @@ mod router_build_probe { .routes(routes!(schedules::post_schedule_resume)) .routes(routes!(schedules::post_schedule_fire_now)) .routes(routes!(schedules::post_rebuild_queue_cancel)) + .routes(routes!(webhook::post_webhook_knowledge)) .routes(routes!(webhook::post_webhook_config_pr)) .routes(routes!(approvals::post_approve)) .routes(routes!(approvals::post_deny)) diff --git a/hive-c0re/src/dashboard/webhook.rs b/hive-c0re/src/dashboard/webhook.rs index ce993841..248c6c03 100644 --- a/hive-c0re/src/dashboard/webhook.rs +++ b/hive-c0re/src/dashboard/webhook.rs @@ -1,18 +1,12 @@ //! Forgejo webhook endpoints. //! +//! - **`/webhook/knowledge`** — push events on `internal/knowledge` trigger a +//! `git pull` on the local clone so agents see up-to-date docs. //! - **`/webhook/config-pr`** — `pull_request` events on any `agent-configs/*` //! repo queue a [`hive_sh4re::approvals::ApprovalKind::MergeConfigPr`] approval row //! so the operator can review + approve the merge from the dashboard. //! -//! There was a second endpoint here, `/webhook/knowledge`, which pulled the -//! local `internal/knowledge` clone on push. It is gone along with the -//! per-hive registration that fed it: a webhook has exactly one target URL, -//! so every hive registering one against the shared repository was -//! last-writer-wins. The swarm controller now holds the single registration -//! and addresses an event to each hive over the queue, which -//! [`crate::workers::knowledge`] documents. -//! -//! The endpoint is reached via the gateway (HTTPS, public domain URL) so +//! Both endpoints are reached via the gateway (HTTPS, public domain URL) so //! Forgejo's SSRF guard does not block delivery. Each delivery is verified //! against the `X-Hub-Signature-256` HMAC header Forgejo attaches; the //! shared secret is auto-generated at startup and persisted to @@ -48,6 +42,104 @@ fn verify_hmac(state: &AppState, headers: &HeaderMap, body: &Bytes) -> Result<() crate::webhook_secret::verify_signature(secret, body, sig).map_err(|e| e.to_string()) } +// ── knowledge webhook ────────────────────────────────────────────────────────── + +/// Minimal Forgejo push-webhook payload — only the fields we care about. +#[derive(Deserialize)] +pub(super) struct PushWebhookPayload { + #[serde(rename = "ref")] + git_ref: Option, + repository: Option, +} + +#[derive(Deserialize)] +pub(super) struct PushWebhookRepo { + full_name: Option, +} + +/// POST `/webhook/knowledge` — Forgejo push webhook for +/// `internal/knowledge`. +/// +/// Runs `git pull` on the local clone so agents see up-to-date documents +/// on their next turn. +/// +/// Expected Forgejo webhook configuration: +/// - URL: `https:///webhook/knowledge` +/// - Content type: `application/json` +/// - Event: "Push" (fires on merge commits to main as well) +/// - Secret: auto-generated HMAC key (see [`crate::webhook_secret`]) +/// +/// The gateway routes `/webhook/` → hive-c0re; the HMAC secret protects +/// the endpoint from unauthenticated callers. +#[utoipa::path( + post, + path = "/webhook/knowledge", + request_body( + content = String, + content_type = "application/json", + description = "Forgejo push-webhook payload, taken as raw bytes \ + (not a typed extractor) so HMAC verification runs \ + over the exact wire bytes before any JSON parsing" + ), + responses( + (status = 200, description = "processed (pull triggered or ignored)", body = String), + (status = 400, description = "invalid JSON payload"), + (status = 401, description = "bad or missing HMAC signature"), + (status = 503, description = "HMAC secret unavailable at startup"), + ), + tag = "webhook" +)] +pub(super) async fn post_webhook_knowledge( + State(state): State, + headers: HeaderMap, + body: Bytes, +) -> Response { + if let Err(e) = verify_hmac(&state, &headers, &body) { + tracing::warn!("webhook/knowledge: HMAC verification failed: {e}"); + let status = if e.contains("unavailable") { + StatusCode::SERVICE_UNAVAILABLE + } else { + StatusCode::UNAUTHORIZED + }; + return (status, e).into_response(); + } + + let payload = match serde_json::from_slice::(&body) { + Ok(p) => p, + Err(e) => { + tracing::warn!("webhook/knowledge: JSON parse error: {e}"); + return (StatusCode::BAD_REQUEST, "invalid JSON").into_response(); + } + }; + + let expected_repo = format!("{}/{}", crate::knowledge::ORG, crate::knowledge::REPO); + let full_name = payload + .repository + .as_ref() + .and_then(|r| r.full_name.as_deref()) + .unwrap_or(""); + if full_name != expected_repo { + tracing::debug!( + full_name, + "webhook/knowledge: ignoring push from unexpected repo" + ); + return (StatusCode::OK, "ignored").into_response(); + } + let git_ref = payload.git_ref.as_deref().unwrap_or(""); + if git_ref != "refs/heads/main" { + tracing::debug!(git_ref, "webhook/knowledge: ignoring non-main push"); + return (StatusCode::OK, "ignored").into_response(); + } + tracing::info!("webhook/knowledge: pull triggered by push to {expected_repo}"); + let coord = state.coord.clone(); + tokio::spawn(async move { + if let Err(e) = crate::knowledge::pull(&coord).await { + tracing::warn!(error = ?e, "webhook/knowledge: pull failed"); + } + }); + (StatusCode::OK, "ok").into_response() +} + // ── config-PR webhook ────────────────────────────────────────────────────────── /// Minimal Forgejo `pull_request`-webhook payload. diff --git a/hive-c0re/src/forge/mod.rs b/hive-c0re/src/forge/mod.rs index 4da942b1..5d5bb182 100644 --- a/hive-c0re/src/forge/mod.rs +++ b/hive-c0re/src/forge/mod.rs @@ -483,7 +483,8 @@ pub async fn ensure_all() { /// existing hooks first and skips creation when one is already targeting the /// correct URL. /// -/// `hive_domain` is the public domain name of the hive; the webhook URL is +/// `hive_domain` is the public domain name of the hive (e.g. +/// `pr1ma.darkest.space`); the webhook URL is /// `https:///webhook/config-pr` (routed through the gateway, /// avoiding the Forgejo SSRF guard that blocks loopback delivery). /// @@ -494,9 +495,8 @@ pub async fn ensure_all() { /// An org-level hook covers every repo in `agent-configs` automatically, /// so no per-repo setup is needed as new agents are provisioned. /// -/// Called at startup beside `knowledge::remove_webhook`, its opposite: that -/// repo's one hook is the controller's now, this one has not moved yet. No-op -/// when the core token is absent (forge not yet provisioned). +/// Called at startup alongside `knowledge::ensure_webhook`. No-op when the +/// core token is absent (forge not yet provisioned). /// /// # Errors /// diff --git a/hive-c0re/src/job_queue/exec.rs b/hive-c0re/src/job_queue/exec.rs index 5d55b9cf..51578c0e 100644 --- a/hive-c0re/src/job_queue/exec.rs +++ b/hive-c0re/src/job_queue/exec.rs @@ -179,18 +179,10 @@ async fn run_matrix_sweep() -> Result<()> { } } -/// Boot-time Forgejo webhook management as a DAG node — see +/// Boot-time Forgejo webhook registration as a DAG node — see /// [`NodeKind::WebhookRegister`]. Mirrors the guard chain the /// `tokio::spawn` block it replaced used: no-op (not an error) when the /// HMAC secret, core token, or hive domain aren't available yet. -/// -/// The node now does one of each: it still registers the config-PR hook, -/// and it *removes* the knowledge one. A knowledge push is delivered to -/// the swarm controller, which addresses an event to each hive over the -/// queue — so a hive holding its own registration is holding a shared -/// resource only one party can own. The removal runs every boot rather -/// than behind a marker because it is already idempotent: it is a no-op -/// the moment the hook is gone. async fn run_webhook_register() -> Result<()> { let Ok(webhook_secret) = crate::webhook_secret::load_or_generate() else { tracing::debug!("webhook secret unavailable; skipping hook registration"); @@ -206,8 +198,10 @@ async fn run_webhook_register() -> Result<()> { tracing::debug!("HYPERHIVE_HIVE_DOMAIN unset; skipping webhook registration"); return Ok(()); }; - if let Err(e) = crate::workers::knowledge::remove_webhook(&token, &domain).await { - tracing::warn!(error = ?e, "knowledge: remove_webhook failed"); + if let Err(e) = + crate::workers::knowledge::ensure_webhook(&token, &domain, &webhook_secret).await + { + tracing::warn!(error = ?e, "knowledge: ensure_webhook failed"); } if let Err(e) = crate::forge::ensure_config_pr_webhook(&token, &domain, &webhook_secret).await { tracing::warn!(error = ?e, "forge: ensure_config_pr_webhook failed"); diff --git a/hive-c0re/src/main.rs b/hive-c0re/src/main.rs index 5e5d6f06..9efe1d51 100644 --- a/hive-c0re/src/main.rs +++ b/hive-c0re/src/main.rs @@ -483,7 +483,7 @@ async fn cmd_serve( // A no-op on a standalone hive (no queue env, logged once) — see // swarm_status, which owns the whole task including its own decision // not to start. - swarm_status::spawn(std::sync::Arc::clone(&coord), coord.shutdown_rx()); + swarm_status::spawn(coord.shutdown_rx()); // Per-agent events.sqlite + bash-tasks file cleanup now runs // agent-side in the harness (`hive_agent::vacuum`): the files are // agent-owned, so host-side deletes hit PermissionDenied / readonly-db diff --git a/hive-c0re/src/swarm_status.rs b/hive-c0re/src/swarm_status.rs index d4310b91..822b8564 100644 --- a/hive-c0re/src/swarm_status.rs +++ b/hive-c0re/src/swarm_status.rs @@ -31,9 +31,6 @@ use std::time::Duration; use anyhow::{Context, Result}; -// `Subscriber` is a `Stream`, so reading the next event needs the extension -// trait — there is no inherent `next()` on it. -use futures_util::StreamExt as _; use crate::stats::sweep_health::{self, SweepHealth}; @@ -65,10 +62,7 @@ const FAILURES_BEFORE_BANNER: u32 = 3; /// makes it a hard error; it is bannered here rather than swallowed, /// because the failure it otherwise produces is a hive that looks fine /// and silently never reports. -pub fn spawn( - coord: std::sync::Arc, - mut shutdown: tokio::sync::watch::Receiver, -) { +pub fn spawn(mut shutdown: tokio::sync::watch::Receiver) { let cfg = match swarm_queue_client::QueueConfig::from_env(ENV_PREFIX) { Ok(Some(cfg)) => cfg, Ok(None) => { @@ -129,14 +123,6 @@ pub fn spawn( } }; - // The hive's ONE queue connection, now serving both directions: - // status goes up, swarm events come down. A second `connect` would - // double the auth-callout traffic against authelia and give the two - // paths independent reconnect state, so one could be serving while - // the other was still down. `async_nats::Client` is a handle, so the - // clone is cheap. - tokio::spawn(drain_swarm_events(client.clone(), coord, shutdown.clone())); - let mut health = SweepHealth::new("swarm_status_publish", "warn", FAILURES_BEFORE_BANNER); loop { match publish(&client, &hive).await { @@ -174,84 +160,6 @@ pub fn spawn( }); } -/// Listen on the swarm's knowledge-event subject and act on what arrives. -/// -/// The controller decides *what a forge delivery means* and addresses the -/// result here; this end does not know a forge exists. Today the one event is -/// **the knowledge repository changed**, and the response is the pull this -/// daemon already runs at boot. -/// -/// # There is no payload, and that is deliberate -/// -/// The event carries nothing. The webhook handler this replaces read two -/// fields from Forgejo and used neither — both were filters — then ran -/// `git pull`, which re-derives everything from the repository. So it is an -/// edge trigger, and reading a body here would invent a contract nobody owes. -/// -/// # What a missed message costs -/// -/// Core NATS, so delivery is at-most-once: a hive that is down when the -/// controller publishes never hears it, and its knowledge stays as of its last -/// pull until it next boots. **That is not a regression** — a webhook delivery -/// to a hive that is down is lost identically, and this daemon pulls at startup -/// regardless. `JetStream` would require this end to *publish* to -/// `$JS.API.CONSUMER.CREATE.`, which the callout policy does not grant, -/// so durability would cost a grant on both sides to remove a failure the boot -/// pull already covers. -/// -/// ⚠️ **A refused subscription is indistinguishable from a quiet one.** NATS -/// reports an authorization violation asynchronously on the connection, not as -/// an error from `subscribe`, so this task cannot tell "no events published" -/// from "not allowed to hear them". If a hive stops picking up knowledge -/// changes, the server log is the thing that knows why — nothing here will say. -async fn drain_swarm_events( - client: async_nats::Client, - coord: std::sync::Arc, - mut shutdown: tokio::sync::watch::Receiver, -) { - // One subject for the whole swarm, so this hive's own name never enters - // it: the controller publishes once and core NATS fans out to whoever is - // subscribed. - let subject = swarm_queue_client::KNOWLEDGE_SUBJECT; - let mut sub = match client.subscribe(subject).await { - Ok(sub) => sub, - Err(e) => { - // Warn rather than a boot banner: the hive is fully functional - // without this, it just falls back to learning about knowledge - // changes at its next boot. - tracing::warn!( - %subject, error = %e, - "swarm events: subscribe failed; this hive will not hear knowledge changes" - ); - return; - } - }; - tracing::info!(%subject, "swarm events: listening"); - - loop { - tokio::select! { - msg = sub.next() => { - if msg.is_none() { - // The subscription ended — the connection went away for - // good. Returning is right: `async-nats` reconnects - // underneath a live subscription, so a closed stream is - // not a blip this should spin on. - tracing::warn!(%subject, "swarm events: subscription closed"); - return; - } - tracing::info!(%subject, "swarm events: knowledge change announced, pulling"); - if let Err(e) = crate::workers::knowledge::pull(&coord).await { - tracing::warn!(error = ?e, "swarm events: knowledge pull failed"); - } - } - _ = shutdown.changed() => { - tracing::info!("swarm events: shutdown signal received"); - return; - } - } - } -} - /// Offer one snapshot: this hive's current readiness, under its own key. async fn publish(client: &async_nats::Client, hive: &str) -> Result<()> { // An unconnected client does not fail a JetStream request, it hangs diff --git a/hive-c0re/src/workers/knowledge.rs b/hive-c0re/src/workers/knowledge.rs index c2e36079..dbd249a9 100644 --- a/hive-c0re/src/workers/knowledge.rs +++ b/hive-c0re/src/workers/knowledge.rs @@ -6,18 +6,15 @@ //! contribute by forking the repo and opening PRs — they never write //! to the bind-mounted path inside the container. //! -//! hive-c0re maintains the local clone. It learns that the repository -//! moved from the **swarm controller**, which owns the one Forgejo -//! webhook and addresses an event to each hive over the queue; a +//! hive-c0re maintains the local clone. A Forgejo webhook notifies it +//! on push to main so agents always see an up-to-date snapshot. The +//! webhook is auto-created by [`ensure_webhook`] at startup. A //! periodic pull in `main.rs` provides a fallback cadence. -//! -//! A hive used to register that webhook itself, pointing at its own -//! `/webhook/knowledge`. A webhook has exactly one target URL, so with -//! more than one hive that was last-writer-wins rather than idempotent — -//! every hive but the most recent silently stopped receiving deliveries. -//! [`remove_webhook`] is the migration off it. + +use std::collections::BTreeMap; use anyhow::{Context, Result}; +use forgejo_api::structs::{CreateHookOption, CreateHookOptionConfig, CreateHookOptionType}; use crate::coordinator::Coordinator; use crate::forge::{core_auth_header, forge_git_url}; @@ -145,71 +142,103 @@ async fn seed_readme(core_token: &str) -> Result<()> { } } -/// Delete this hive's own `internal/knowledge` push webhook if it is -/// still registered, so the swarm controller is the only party holding -/// one. +/// Ensure a Forgejo push webhook for `internal/knowledge` exists and +/// points at hive-c0re's `/webhook/knowledge` endpoint. Idempotent — +/// lists existing hooks first and skips creation when one is already +/// targeting the correct URL. /// -/// # Why this is a migration and not just a deletion +/// `hive_domain` is the public domain name of the hive; the webhook URL is +/// `https:///webhook/knowledge` (routed through the gateway, +/// avoiding the Forgejo SSRF guard that blocks loopback delivery). /// -/// Not registering any more fixes nothing on a hive that has already -/// run: the hook it created persists on the forge, so the contention -/// this removes would survive on exactly the deployments that have it -/// while fresh installs looked fixed. The hive that created a hook is -/// the one that removes it. +/// `webhook_secret` is the HMAC secret Forgejo will attach as +/// `X-Hub-Signature-256` on each delivery; hive-c0re verifies this header +/// in the dashboard webhook handler (`post_webhook_knowledge`). /// -/// # It removes only its OWN hook, never a neighbour's -/// -/// The match is the full URL, not the `/webhook/knowledge` suffix. A -/// hook with that suffix and a different base belongs to *another hive* — -/// one that may not have been upgraded yet — and deleting it would break -/// its knowledge sync until it was. Reaping a neighbour's registration is -/// the very behaviour this issue is about; doing it in the name of fixing -/// it would just invert the direction. -/// -/// (The predecessor did reap by suffix, to clear loopback hooks left by -/// an older single-hive layout. That was safe when a hive was alone on -/// its forge and is not safe now.) -/// -/// A listing failure is an error rather than a silent skip: there is no -/// create attempt left to fall through to, so swallowing it would leave -/// the hook in place with nothing said. The caller logs and continues — -/// boot does not depend on this. -pub async fn remove_webhook(core_token: &str, hive_domain: &str) -> Result<()> { +/// Called at startup alongside [`ensure_local_clone`]. No-op when the +/// core token is absent (forge not yet provisioned). +pub async fn ensure_webhook( + core_token: &str, + hive_domain: &str, + webhook_secret: &str, +) -> Result<()> { // The typed client carries no per-request timeout, so each call is // wrapped in one: this runs as a detached startup task, and a forge - // that accepts connections but never answers would otherwise hang it - // forever. + // that accepts connections but never answers would otherwise hang + // it forever (and the hourly pull fallback masks the missing hook). const HTTP_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); - let own_url = format!("https://{hive_domain}/webhook/knowledge"); + let target_url = format!("https://{hive_domain}/webhook/knowledge"); let client = crate::forge::api(core_token)?; - let hooks = tokio::time::timeout(HTTP_TIMEOUT, client.repo_list_hooks(ORG, REPO).all()) + // List existing hooks — skip creation if ours is already there. + // Best-effort like the raw-HTTP predecessor: a listing failure + // falls through to the create attempt. + let listed = tokio::time::timeout(HTTP_TIMEOUT, client.repo_list_hooks(ORG, REPO).all()) + .await + .map_err(anyhow::Error::from) + .and_then(|r| r.map_err(anyhow::Error::from)); + match listed { + Ok(hooks) => { + let already_exists = hooks.iter().any(|h| { + h.config + .as_ref() + .and_then(|c| c.get("url")) + .map(String::as_str) + == Some(target_url.as_str()) + }); + if already_exists { + tracing::debug!(%target_url, "knowledge: push webhook already configured"); + return Ok(()); + } + // Delete stale hooks that point at our path but a different base + // (e.g. old loopback hooks from before the SSRF-bypass migration). + for h in &hooks { + let hook_url = h + .config + .as_ref() + .and_then(|c| c.get("url")) + .map_or("", String::as_str); + if hook_url.ends_with("/webhook/knowledge") + && hook_url != target_url + && let Some(id) = h.id + { + tracing::info!(hook_url, "knowledge: deleting stale webhook (wrong base)"); + let _ = tokio::time::timeout( + HTTP_TIMEOUT, + client.repo_delete_hook(ORG, REPO, id).send(), + ) + .await; + } + } + } + Err(e) => { + tracing::debug!(error = %e, "knowledge: listing hooks failed; attempting create"); + } + } + + // Create the webhook. + let mut additional = BTreeMap::new(); + additional.insert("secret".to_owned(), webhook_secret.to_owned()); + + let hook = CreateHookOption { + active: Some(true), + authorization_header: None, + branch_filter: None, + config: CreateHookOptionConfig { + content_type: "json".to_owned(), + url: url::Url::parse(&target_url).context("parse webhook target url")?, + additional, + }, + events: Some(vec!["push".to_owned()]), + r#type: CreateHookOptionType::Forgejo, + }; + tokio::time::timeout(HTTP_TIMEOUT, client.repo_create_hook(ORG, REPO, hook)) .await .map_err(anyhow::Error::from) .and_then(|r| r.map_err(anyhow::Error::from)) - .with_context(|| format!("list webhooks for {ORG}/{REPO}"))?; - - for h in &hooks { - let hook_url = h - .config - .as_ref() - .and_then(|c| c.get("url")) - .map_or("", String::as_str); - if hook_url == own_url - && let Some(id) = h.id - { - tokio::time::timeout(HTTP_TIMEOUT, client.repo_delete_hook(ORG, REPO, id).send()) - .await - .map_err(anyhow::Error::from) - .and_then(|r| r.map_err(anyhow::Error::from)) - .with_context(|| format!("delete webhook {id} for {ORG}/{REPO}"))?; - tracing::info!( - %own_url, - "knowledge: removed this hive's push webhook — the swarm controller owns it now" - ); - } - } + .with_context(|| format!("create webhook for {ORG}/{REPO}"))?; + tracing::info!(%target_url, "knowledge: push webhook created"); Ok(()) } diff --git a/swarm-controller/src/status.rs b/swarm-controller/src/status.rs index 5090cd7a..eaff6038 100644 --- a/swarm-controller/src/status.rs +++ b/swarm-controller/src/status.rs @@ -133,23 +133,6 @@ impl StatusReader { } } - /// A handle on the queue connection this reader holds. - /// - /// The controller has exactly **one** connection to the swarm queue and - /// more than one thing to do with it: status is read out of a KV bucket, - /// swarm events are published on a subject. Handing out a clone is cheap — - /// `async_nats::Client` is a handle, not a socket — and is strictly better - /// than opening a second connection, which would double the auth-callout - /// traffic and give the two paths independent reconnect state, so one could - /// be serving while the other was still down. - /// - /// That this lives on the *status* reader is an accident of who constructed - /// the connection first, not a claim that events are a kind of status. - #[must_use] - pub fn queue_client(&self) -> async_nats::Client { - self.client.clone() - } - /// Reads [`STALE_AFTER_ENV`], falling back to /// [`DEFAULT_STALE_AFTER`]. A zero or unparseable value takes the /// default rather than failing startup — same rule as `load_hives`: diff --git a/swarm-controller/src/webhook.rs b/swarm-controller/src/webhook.rs index ce4f9098..5be60268 100644 --- a/swarm-controller/src/webhook.rs +++ b/swarm-controller/src/webhook.rs @@ -170,14 +170,13 @@ pub(super) enum DeliveryKind { /// The route prefix a registered `target_url` must point at. /// /// ⚠️ Deliberately **not** `/webhook/knowledge` or `/webhook/config-pr`, the -/// paths the per-hive receivers use. A hive-side registrar deletes any hook -/// whose URL ends with *its* path but has a different base — see -/// `hive-c0re`'s `forge::ensure_config_pr_webhook`. A swarm-level hook under -/// such a path would therefore be deleted by every hive on every boot, and -/// the symptom is a hook that silently stops existing. -/// `webhook_urls_survive_the_hive_side_reapers` pins that, and its own doc -/// records why `/webhook/knowledge` stays in the check even though the -/// knowledge registrar no longer reaps. +/// paths the per-hive receivers use. Both hive-side registrars delete any +/// hook whose URL ends with *their* path but has a different base — see +/// `hive-c0re`'s `forge::ensure_config_pr_webhook` and +/// `workers::knowledge::ensure_webhook`. A swarm-level hook under those +/// paths would therefore be deleted by every hive on every boot, and the +/// symptom is a hook that silently stops existing. `webhook_urls_survive_the_hive_side_reapers` +/// pins that. const ROUTE_PREFIX: &str = "/webhook/forge/"; impl DeliveryKind { @@ -366,75 +365,9 @@ pub(super) async fn post_webhook_forge( bytes = body.len(), "webhook: verified delivery" ); - - // Only the knowledge delivery is acted on. Deploy coordination is a - // separate concern with its own issue — a hive does not want to hear that a - // config PR was opened, it wants to be told when to rebuild from main, and - // that is a decision the controller makes after a merge rather than a relay - // of this delivery. Written as a condition rather than a match arm holding - // an empty body, which would claim this is where that path is handled. - if kind == DeliveryKind::Knowledge { - announce_knowledge_change(&state).await; - } - (StatusCode::OK, "ok").into_response() } -/// Tell every hive in the swarm that the knowledge repository changed. -/// -/// The event carries **no payload**, because there is nothing to carry: the -/// hive-side handler this replaces read two fields from Forgejo's webhook and -/// used neither — both were filters — and then ran `git pull`, which re-derives -/// everything from the repository itself. So what crosses the queue is an edge -/// trigger, and adding fields to it would invent a contract nobody reads. -/// -/// One publish to one shared subject, not one per hive: every subscriber gets -/// the same empty event, so the roster is never consulted and this daemon does -/// not need to know who the hives are in order to tell them. -/// -/// # Failure -/// -/// Returns nothing and fails soft. A missed announcement costs a hive stale -/// knowledge until its next boot pull — the same cost as a webhook delivery to -/// a hive that happened to be down, which is what this replaces. -/// -/// ⚠️ A **permission** failure cannot be observed here. `publish` hands the -/// message to the connection's buffer, and a NATS authorization violation is -/// reported asynchronously on the connection rather than as an error on this -/// call — it reaches a client as a timeout, or as nothing at all. The `flush` -/// below therefore proves the bytes left this process, and nothing more; if -/// hives stop hearing events, the server log is the place that knows why. -async fn announce_knowledge_change(state: &AppState) { - let Some(status) = state.status.as_ref() else { - // Verified, accepted, and dropped. Worth a warning rather than - // silence: the forge will report a 200 and nobody would otherwise - // learn that the delivery reached a controller with nowhere to put it. - tracing::warn!( - "webhook: knowledge delivery verified but no swarm queue is \ - configured; no hive will be told" - ); - return; - }; - let client = status.queue_client(); - let subject = swarm_queue_client::KNOWLEDGE_SUBJECT; - - // One publish, not one per hive: every subscriber gets the same empty - // event, so the roster is not consulted at all. The controller does not - // need to know who the hives are in order to say the repository moved. - if let Err(e) = client.publish(subject, Vec::new().into()).await { - tracing::warn!(%subject, error = %e, "webhook: publishing the knowledge event failed"); - return; - } - // Logged after the flush rather than after the publish: `publish` only - // hands the message to the client's write buffer, so a line printed there - // would claim delivery this end cannot yet know about. - if let Err(e) = client.flush().await { - tracing::warn!(%subject, error = %e, "webhook: flushing the knowledge event failed"); - return; - } - tracing::info!(%subject, "webhook: knowledge event published"); -} - #[cfg(test)] mod tests { use super::{DeliveryKind, Refusal, load_or_generate_at, secret_path_from, verify}; @@ -634,22 +567,17 @@ mod tests { } } - /// A cross-daemon invariant with nothing else to enforce it: a per-hive - /// registrar in `hive-c0re` **deletes** hooks whose URL ends with its - /// own path but carries a different base. A controller URL matching such - /// a suffix would be deleted by every hive on every boot — the swarm hook - /// would simply cease to exist, with the cause in a different daemon's - /// startup sweep. Serving these under `/webhook/forge/` is what avoids - /// it, and this is the only place that says so in a form that fails. + /// A cross-daemon invariant with nothing else to enforce it: both + /// per-hive registrars in `hive-c0re` **delete** hooks whose URL ends + /// with their own path but carries a different base — see + /// `forge::ensure_config_pr_webhook` and `knowledge::ensure_webhook`. /// - /// `forge::ensure_config_pr_webhook` still reaps that way. The knowledge - /// registrar no longer does — it was replaced by a removal that matches - /// the full URL, so it cannot touch another hive's hook. **The - /// `/webhook/knowledge` arm is kept anyway**, because the hazard is not - /// this repository's current code: it is whatever is *deployed*, and a - /// hive still running the previous version reaps by suffix until it is - /// upgraded. Drop that arm once no such hive can exist, not when the - /// source stops mentioning it. + /// While the swarm-level hooks live alongside the per-hive ones, a + /// controller URL matching either suffix would be deleted by every hive + /// on every boot: the swarm hook would simply cease to exist, with the + /// cause in a different daemon's startup sweep. Serving these under + /// `/webhook/forge/` is what avoids it, and this is the only place that + /// says so in a form that fails. #[test] fn webhook_urls_survive_the_hive_side_reapers() { for suffix in ["/webhook/knowledge", "/webhook/config-pr"] { diff --git a/swarm-nats-auth/src/policy.rs b/swarm-nats-auth/src/policy.rs index 1eb3e083..4f3a76b2 100644 --- a/swarm-nats-auth/src/policy.rs +++ b/swarm-nats-auth/src/policy.rs @@ -234,19 +234,6 @@ impl Policy { // stays for a named/durable consumer. format!("$JS.API.CONSUMER.CREATE.{stream}"), format!("$JS.API.CONSUMER.CREATE.{stream}.>"), - // The knowledge event. One writer, many readers: the controller is - // the only publisher and every hive subscribes, so this is one - // literal subject rather than a per-hive family — named from the - // crate the publisher and the subscriber also name, so a rename - // cannot leave the grant pointing at a subject nobody uses. - // - // This is the reader's only non-JetStream subject, and without it - // the controller cannot emit the event at all. Worth stating because - // the symptom is unhelpful: a refused publish reaches the client as - // a **timeout**, so the visible failure is a hive that never hears - // about a change, with nothing in the controller's log to say a - // permission was the reason. - swarm_queue_client::KNOWLEDGE_SUBJECT.to_owned(), ]); subjects } @@ -308,40 +295,6 @@ mod tests { assert!(!p.publish.iter().any(|s| s.contains("beta"))); } - #[test] - fn a_reader_may_publish_the_knowledge_event() { - let p = policy().permissions("swarm-controller").expect("a reader"); - assert!( - p.publish - .contains(&swarm_queue_client::KNOWLEDGE_SUBJECT.to_owned()), - "the controller is the only publisher of this event; without the \ - grant its publish is refused, and a refusal arrives as a timeout" - ); - } - - #[test] - fn a_hive_may_not_publish_the_knowledge_event_to_anyone_including_itself() { - // The controller *interprets* what a delivery means; a hive receives - // that verdict. A hive able to publish here could tell every other hive - // in the swarm — or itself — that the knowledge repo changed when it did - // not, which is an unauthenticated write into someone else's control - // path wearing an event's shape. - // - // One writer and many readers makes this arm matter MORE, not less: with - // a single shared subject a forged event reaches the whole swarm, where - // a per-hive subject would have reached one. - let p = policy() - .permissions("hive-alpha") - .expect("a hive is admitted"); - assert!( - !p.publish - .iter() - .any(|s| s == swarm_queue_client::KNOWLEDGE_SUBJECT), - "a hive must not publish the knowledge event: {:?}", - p.publish - ); - } - #[test] fn a_hive_grant_never_includes_the_jetstream_wildcard() { // `$JS.API.>` also covers `$JS.API.STREAM.DELETE.KV_hive-status`, with diff --git a/swarm-queue-client/src/lib.rs b/swarm-queue-client/src/lib.rs index c48f981b..1c03a317 100644 --- a/swarm-queue-client/src/lib.rs +++ b/swarm-queue-client/src/lib.rs @@ -156,15 +156,6 @@ pub fn chain(error: &dyn std::error::Error) -> String { /// which is the disagreement this module exists to prevent. pub mod status; -/// The subject the swarm controller publishes on when the hive-wide knowledge -/// repository has changed. One writer, many readers — every hive subscribes. -/// -/// Here rather than in a module of its own for the same reason as the bucket -/// name above: three crates must agree on the string, and the one that agrees -/// hardest — the auth-callout responder, which decides whether a publish is -/// permitted at all — speaks neither `jetstream` nor `kv`. -pub const KNOWLEDGE_SUBJECT: &str = "$SWARM.knowledge"; - /// Only the fields this needs; authelia returns several. #[derive(serde::Deserialize)] struct TokenResponse {