diff --git a/hive-c0re/src/webhook_secret.rs b/hive-c0re/src/webhook_secret.rs index ed70e932..8674abd1 100644 --- a/hive-c0re/src/webhook_secret.rs +++ b/hive-c0re/src/webhook_secret.rs @@ -97,3 +97,41 @@ fn hex_decode(s: &str) -> Option> { } Some(out) } + +#[cfg(test)] +mod tests { + use super::{hex_encode, verify_signature}; + + /// Compute the `sha256=` header Forgejo would send for `secret` + + /// `body`, so the "matches" test below isn't asserting against a + /// hand-picked string that happens to look like a signature. + fn sign(secret: &str, body: &[u8]) -> String { + use hmac::{Hmac, KeyInit, Mac}; + use sha2::Sha256; + let mut mac = Hmac::::new_from_slice(secret.as_bytes()).unwrap(); + mac.update(body); + format!("sha256={}", hex_encode(&mac.finalize().into_bytes())) + } + + /// `verify_hmac` (`dashboard/webhook.rs`) delegates the actual HMAC + /// comparison here — this is the only gate on an inbound Forgejo + /// webhook reachable via the public gateway. A signature computed + /// with the right secret over the right body must verify. + #[test] + fn verify_signature_accepts_the_correct_hmac() { + let header = sign("s3cr3t", b"payload"); + assert!(verify_signature("s3cr3t", b"payload", &header).is_ok()); + } + + /// If `mac.verify_slice`'s result were ever inverted (accept on + /// mismatch), an unauthenticated actor could queue an operator + /// approval through the public webhook endpoint. Cover both ways a + /// signature can stop matching: wrong secret, and a body that no + /// longer matches the one that was signed. + #[test] + fn verify_signature_rejects_a_mismatched_hmac() { + let header = sign("s3cr3t", b"payload"); + assert!(verify_signature("different-secret", b"payload", &header).is_err()); + assert!(verify_signature("s3cr3t", b"tampered-payload", &header).is_err()); + } +} diff --git a/hive-forge/tests/credential_helper.rs b/hive-forge/tests/credential_helper.rs new file mode 100644 index 00000000..e15947a4 --- /dev/null +++ b/hive-forge/tests/credential_helper.rs @@ -0,0 +1,113 @@ +//! Integration coverage for `hive-forge credential-helper get` — the +//! `host=` check in `verbs::credential_helper::run` exists to close a +//! reported credential leak, and git invokes the helper fresh over +//! stdin/env on every fetch/push. The check is inlined in `run()` +//! (real stdin, real env, real stdout), so it can only be driven the way +//! git actually drives it: run the compiled binary. + +use std::io::Write as _; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; + +const TOKEN: &str = "test-token-value"; + +/// A unique scratch `HYPERHIVE_STATE_DIR`, seeded with a `forge-token` +/// file. Matches the manual-tmpdir pattern `hive-c0re`'s +/// `dashboard::state_files` tests use — this crate carries no `tempfile` +/// dependency to reach for instead. +fn scratch_state_dir(tag: &str) -> PathBuf { + let ts = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |d| d.as_nanos()); + let dir = std::env::temp_dir().join(format!("hive-forge-cred-test-{tag}-{ts}")); + std::fs::create_dir_all(&dir).expect("create scratch state dir"); + std::fs::write(dir.join("forge-token"), format!("{TOKEN}\n")).expect("seed forge-token"); + dir +} + +/// Run `hive-forge credential-helper get` against `base_url` + +/// `state_dir`, feeding `request` as the git credential-protocol +/// request body on stdin. Returns `(exit success, stdout, stderr)`. +fn run_get(base_url: &str, state_dir: &Path, request: &str) -> (bool, String, String) { + let mut child = Command::new(env!("CARGO_BIN_EXE_hive-forge")) + .arg("credential-helper") + .arg("get") + .env("HIVE_FORGE_URL", base_url) + .env("HYPERHIVE_STATE_DIR", state_dir) + .env_remove("HIVE_LABEL") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn hive-forge credential-helper get"); + child + .stdin + .take() + .expect("piped stdin") + .write_all(request.as_bytes()) + .expect("write credential request to stdin"); + let out = child.wait_with_output().expect("wait for hive-forge"); + ( + out.status.success(), + String::from_utf8_lossy(&out.stdout).into_owned(), + String::from_utf8_lossy(&out.stderr).into_owned(), + ) +} + +/// This is the check the module doc says exists to close a reported +/// credential leak: refusing to hand the forge token to a host git +/// didn't ask this invocation to serve. If the host comparison were ever +/// inverted, a mismatched host would get the token instead of being +/// refused. +#[test] +fn credential_helper_get_refuses_a_mismatched_host_and_withholds_the_token() { + let dir = scratch_state_dir("mismatch"); + let (ok, stdout, stderr) = run_get( + "http://forge.internal.example", + &dir, + "protocol=https\nhost=not-the-forge.example\n", + ); + assert!(!ok, "a mismatched host= must fail, stderr={stderr}"); + assert!( + !stdout.contains(TOKEN), + "the token must never reach stdout on a host mismatch: stdout={stdout}" + ); + let _ = std::fs::remove_dir_all(&dir); +} + +/// The other side of the same check: a `host=` that matches the +/// configured forge must still print the credentials, or every real git +/// fetch/push would break. +#[test] +fn credential_helper_get_answers_when_host_matches() { + let dir = scratch_state_dir("match"); + let (ok, stdout, stderr) = run_get( + "http://forge.internal.example", + &dir, + "protocol=https\nhost=forge.internal.example\n", + ); + assert!(ok, "a matching host= must succeed, stderr={stderr}"); + assert!(stdout.contains("username="), "stdout={stdout}"); + assert!( + stdout.contains(&format!("password={TOKEN}")), + "stdout={stdout}" + ); + let _ = std::fs::remove_dir_all(&dir); +} + +/// Backwards compatibility: the request shape git sent before this check +/// existed — no `host=` line at all — must still work. +#[test] +fn credential_helper_get_still_works_with_no_host_line() { + let dir = scratch_state_dir("nohost"); + let (ok, stdout, stderr) = run_get("http://forge.internal.example", &dir, "protocol=https\n"); + assert!( + ok, + "a request with no host= line must not be treated as a failure, stderr={stderr}" + ); + assert!( + stdout.contains(&format!("password={TOKEN}")), + "stdout={stdout}" + ); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs index 4831b5d2..b3c6ee54 100644 --- a/hive-priv/src/main.rs +++ b/hive-priv/src/main.rs @@ -3114,9 +3114,10 @@ mod tests { use super::{ AgentTmpfilesEntry, BindMount, OwnedFd, PAUSED_MARKER_FILE, PrivRequest, agent_tmpfiles_content, check_fd_agreement, clear_runner_credentials, - contains_secret_shaped_run, git_overlay_flags, limits_dropin_body, matrix_token_filename, - partial_name, redact_secret_line, remove_marker_in, single_output_path, toplevel_attr, - validate_account_name, write_agent_dir_file, write_state_file_nofollow, + contains_secret_shaped_run, ensure_plain_filename, git_overlay_flags, limits_dropin_body, + matrix_token_filename, partial_name, redact_secret_line, remove_marker_in, + single_output_path, toplevel_attr, validate_account_name, validate_credential_name, + validate_snapshot_name, write_agent_dir_file, write_state_file_nofollow, }; use std::path::PathBuf; use std::sync::atomic::{AtomicU32, Ordering}; @@ -3735,4 +3736,44 @@ mod tests { ); std::fs::remove_dir_all(&dir).ok(); } + + /// The only gate on the root-privileged socket for a `--load-credential` + /// name. The doc comment is explicit that `.` is excluded on purpose + /// ("this name gets interpolated into filesystem paths") — a future + /// "let's allow dots for version numbers" loosening must fail this, + /// not just the `/` case below. + #[test] + fn validate_credential_name_rejects_dot_slash_and_empty_but_allows_the_charset() { + assert!( + validate_credential_name("").is_err(), + "empty must be rejected" + ); + assert!(validate_credential_name("valid-name_123").is_ok()); + assert!( + validate_credential_name("bad.name").is_err(), + "dot must be rejected — see the fn's doc comment" + ); + assert!(validate_credential_name("bad/name").is_err()); + } + + /// A snapshot label must additionally carry the `hive-` prefix (it + /// doubles as the allow-list gating the btrfs snapshot/delete + /// shellouts) on top of [`validate_credential_name`]'s charset rule. + #[test] + fn validate_snapshot_name_requires_hive_prefix() { + assert!(validate_snapshot_name("not-hive-prefixed").is_err()); + assert!(validate_snapshot_name("hive-valid").is_ok()); + } + + /// `ensure_plain_filename` is the gate between an agent-writable + /// directory and a root-privileged file write. `.`/`..` would resolve + /// to the directory itself or its parent; any `/` climbs into a path + /// component the caller never named. + #[test] + fn ensure_plain_filename_rejects_dot_dotdot_and_any_slash() { + assert!(ensure_plain_filename("test", ".").is_err()); + assert!(ensure_plain_filename("test", "..").is_err()); + assert!(ensure_plain_filename("test", "sub/dir").is_err()); + assert!(ensure_plain_filename("test", "matrix-token").is_ok()); + } }