From a40c0026cf14a01718edb3fc048d272914a03650 Mon Sep 17 00:00:00 2001 From: atlas Date: Fri, 2 Oct 2026 17:01:21 +0200 Subject: [PATCH] hive-c0re: test verify_hmac guards and webhook status mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `verify_hmac` is the only authentication gate on the public `/webhook/config-pr` endpoint, and neither of its guard clauses nor the handler's "unavailable" → 503 / otherwise → 401 mapping had a test. The tests build a real `AppState` over the tempdir `Coordinator` that the socket_server schedule tests already use. That helper is widened from `pub(in crate::socket_server)` to `pub(crate)` and re-exported from `socket_server` under `#[cfg(test)]`, so non-test code is unchanged. Removing the missing-secret guard (replacing it with `unwrap_or_default()`) fails verify_hmac_rejects_every_delivery_when_no_secret_is_loaded and config_pr_answers_503_when_no_secret_is_loaded. Deleting the missing-header guard fails verify_hmac_rejects_a_delivery_with_no_usable_signature_header. Breaking the "unavailable" match fails the 503 test. Without the missing-header guard, an unsigned delivery is still refused by `verify_signature` (no `sha256=` prefix). The test therefore pins the guard's own message rather than the bare rejection. Closes #4654 --- hive-c0re/src/dashboard/webhook.rs | 138 +++++++++++++++++++++++ hive-c0re/src/socket_server/mod.rs | 2 + hive-c0re/src/socket_server/schedules.rs | 2 +- 3 files changed, 141 insertions(+), 1 deletion(-) diff --git a/hive-c0re/src/dashboard/webhook.rs b/hive-c0re/src/dashboard/webhook.rs index ce993841..44842651 100644 --- a/hive-c0re/src/dashboard/webhook.rs +++ b/hive-c0re/src/dashboard/webhook.rs @@ -222,3 +222,141 @@ pub(super) async fn post_webhook_config_pr( (StatusCode::OK, "ok").into_response() } + +#[cfg(test)] +mod tests { + use axum::{ + body::Bytes, + extract::State, + http::{HeaderMap, HeaderValue, StatusCode}, + }; + + use super::{AppState, post_webhook_config_pr, verify_hmac}; + + const SECRET: &str = "s3cr3t"; + + /// The `TempDir` holds the coordinator's sqlite files; keep it alive as + /// long as the state. + fn state(webhook_secret: Option<&str>) -> (tempfile::TempDir, AppState) { + let (dir, coord) = crate::socket_server::coordinator(); + let state = AppState { + coord, + webhook_secret: webhook_secret.map(str::to_owned), + }; + (dir, state) + } + + /// The `X-Hub-Signature-256` header Forgejo would send for `secret` + `body`. + fn signed(secret: &str, body: &[u8]) -> HeaderMap { + use std::fmt::Write as _; + + use hmac::{Hmac, KeyInit, Mac}; + use sha2::Sha256; + let mut mac = Hmac::::new_from_slice(secret.as_bytes()).unwrap(); + mac.update(body); + let mut hex = String::new(); + for b in mac.finalize().into_bytes() { + write!(hex, "{b:02x}").unwrap(); + } + let mut headers = HeaderMap::new(); + headers.insert( + "x-hub-signature-256", + HeaderValue::from_str(&format!("sha256={hex}")).unwrap(), + ); + headers + } + + /// With no secret loaded, nothing verifies — including a delivery signed + /// with the empty key. The message must say "unavailable": the handler + /// maps on that word to 503. + #[test] + fn verify_hmac_rejects_every_delivery_when_no_secret_is_loaded() { + let (_dir, state) = state(None); + let body = Bytes::from_static(b"{}"); + for (label, headers) in [ + ("signed with the empty key", signed("", &body)), + ("signed with some key", signed(SECRET, &body)), + ("unsigned", HeaderMap::new()), + ] { + let err = verify_hmac(&state, &headers, &body).expect_err(label); + assert!(err.contains("unavailable"), "{label}: {err}"); + } + } + + /// An absent header, an empty one and one that is not valid UTF-8 all + /// read as "no signature", and are refused before any HMAC is computed. + #[test] + fn verify_hmac_rejects_a_delivery_with_no_usable_signature_header() { + let (_dir, state) = state(Some(SECRET)); + let body = Bytes::from_static(b"{}"); + let mut empty = HeaderMap::new(); + empty.insert("x-hub-signature-256", HeaderValue::from_static("")); + let mut not_utf8 = HeaderMap::new(); + not_utf8.insert( + "x-hub-signature-256", + HeaderValue::from_bytes(b"sha256=\xff").unwrap(), + ); + for (label, headers) in [ + ("absent", HeaderMap::new()), + ("empty", empty), + ("not UTF-8", not_utf8), + ] { + assert_eq!( + verify_hmac(&state, &headers, &body), + Err("missing X-Hub-Signature-256 header".to_owned()), + "{label}" + ); + } + } + + /// The other side of both guards: a loaded secret and a matching + /// signature pass. + #[test] + fn verify_hmac_accepts_a_correctly_signed_delivery() { + let (_dir, state) = state(Some(SECRET)); + let body = Bytes::from_static(b"{}"); + assert_eq!(verify_hmac(&state, &signed(SECRET, &body), &body), Ok(())); + } + + /// 503 means "this hive cannot verify any delivery", 401 means "this + /// delivery is not authentic". + #[tokio::test] + async fn config_pr_answers_503_when_no_secret_is_loaded() { + let (_dir, state) = state(None); + let body = Bytes::from_static(b"{}"); + let headers = signed(SECRET, &body); + let resp = post_webhook_config_pr(State(state), headers, body).await; + assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE); + } + + #[tokio::test] + async fn config_pr_answers_401_for_a_missing_or_wrong_signature() { + let body = Bytes::from_static(b"{}"); + for (label, headers) in [ + ("missing", HeaderMap::new()), + ("wrong secret", signed("different-secret", &body)), + ("other body", signed(SECRET, b"tampered")), + ] { + let (_dir, state) = state(Some(SECRET)); + let resp = post_webhook_config_pr(State(state), headers, body.clone()).await; + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED, "{label}"); + } + } + + /// A verified delivery reaches payload handling: an action the handler + /// ignores comes back 200, and an unparseable body 400, neither of which + /// is an auth status. + #[tokio::test] + async fn config_pr_lets_a_correctly_signed_delivery_through() { + for (body, expected) in [ + (&br#"{"action":"closed"}"#[..], StatusCode::OK), + (&b"not json"[..], StatusCode::BAD_REQUEST), + ] { + let (_dir, state) = state(Some(SECRET)); + let body = Bytes::from_static(body); + let headers = signed(SECRET, &body); + let resp = post_webhook_config_pr(State(state), headers, body).await; + assert_eq!(resp.status(), expected); + } + } +} diff --git a/hive-c0re/src/socket_server/mod.rs b/hive-c0re/src/socket_server/mod.rs index 20b5f456..06a39c2e 100644 --- a/hive-c0re/src/socket_server/mod.rs +++ b/hive-c0re/src/socket_server/mod.rs @@ -27,6 +27,8 @@ mod schedules; pub(crate) use config_approvals::submit_merge_config_pr; pub(crate) use schedules::filter_ghost_schedule_targets; pub use schedules::schedule_to_wire_public; +#[cfg(test)] +pub(crate) use schedules::tests::coordinator; use schedules::{ EditSchedulePatch, handle_cancel_schedule, handle_edit_schedule, handle_fire_schedule_now, diff --git a/hive-c0re/src/socket_server/schedules.rs b/hive-c0re/src/socket_server/schedules.rs index 64d018ad..cce2829c 100644 --- a/hive-c0re/src/socket_server/schedules.rs +++ b/hive-c0re/src/socket_server/schedules.rs @@ -380,7 +380,7 @@ pub(super) mod tests { /// A real `Coordinator` over a throwaway sqlite dir. The socket-server /// handlers take `&Arc`, so there is no lighter way in; /// `open` touches nothing outside the db path it is handed. - pub(in crate::socket_server) fn coordinator() -> (tempfile::TempDir, Arc) { + pub(crate) fn coordinator() -> (tempfile::TempDir, Arc) { let dir = tempfile::tempdir().expect("tempdir"); let coord = Coordinator::open( &dir.path().join("broker.sqlite"),