diff --git a/hive-forge/src/verbs/credential_helper.rs b/hive-forge/src/verbs/credential_helper.rs index cbda9841..9ce5dd94 100644 --- a/hive-forge/src/verbs/credential_helper.rs +++ b/hive-forge/src/verbs/credential_helper.rs @@ -62,14 +62,26 @@ pub fn run(args: Args, forge_label: Option<&str>) -> Result<()> { let (base, token) = resolve_credentials(forge_label)?; if let Some(requested) = input.lines().find_map(|l| l.strip_prefix("host=")) { + let requested = requested.trim(); + // Fail closed, not open: a configured forge URL that won't parse must + // refuse the same as a genuine mismatch would, not skip the check. + // `expected`'s previous `Option` collapsed "doesn't parse" and + // "parses but has no host" into the same `None` that `is_some_and` + // then read as "nothing to check" — exactly backwards for a check + // that exists to close a reported credential leak. let expected = url::Url::parse(&base) .ok() .and_then(|u| u.host_str().map(str::to_owned)); - if expected.is_some_and(|e| e != requested.trim()) { - bail!( + match expected { + Some(e) if e == requested => {} + Some(e) => bail!( "hive-forge credential-helper: refusing to hand the forge token to \ - host {requested:?} (configured forge is {base:?})" - ); + host {requested:?} (configured forge is {e:?})" + ), + None => bail!( + "hive-forge credential-helper: refusing — configured forge URL {base:?} \ + does not resolve to a host, cannot verify requested host {requested:?}" + ), } } diff --git a/hive-forge/tests/credential_helper.rs b/hive-forge/tests/credential_helper.rs index 77936bb6..6c79de09 100644 --- a/hive-forge/tests/credential_helper.rs +++ b/hive-forge/tests/credential_helper.rs @@ -91,6 +91,29 @@ fn credential_helper_get_refuses_a_mismatched_host_and_withholds_the_token() { let _ = std::fs::remove_dir_all(&dir); } +/// A configured forge URL that doesn't parse must refuse like a genuine +/// mismatch, not silently skip the check because there was nothing to +/// compare against — the previous shape failed open exactly where this +/// check exists to close a reported credential leak. +#[test] +fn credential_helper_get_refuses_when_the_configured_forge_url_does_not_parse() { + let dir = scratch_state_dir("unparseable"); + let (ok, stdout, stderr) = run_get( + "this is not a url", + &dir, + "protocol=https\nhost=not-the-forge.example\n", + ); + assert!( + !ok, + "an unparseable configured URL must fail, stderr={stderr}" + ); + assert!( + !stdout.contains(TOKEN), + "the token must never reach stdout when the forge URL won't parse: 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.