diff --git a/flake.nix b/flake.nix index d32a796a..8eef657c 100644 --- a/flake.nix +++ b/flake.nix @@ -149,11 +149,34 @@ nixosConfigurations = let + # Values the agent modules require but that only a real + # deployment can know. Real containers are built from the + # generated meta flake, where hive-c0re renders these per + # agent from the host's `HIVE_FORGE_URL` (see meta.rs's + # `SERVICE_URL_OPTIONS`) — they never evaluate through + # `self.nixosConfigurations`, so nothing here can reach a + # running agent. These two configs exist only to typecheck + # the modules and to pre-build the container closure + # (`system.extraDependencies`, see hive-c0re/default.nix). + # + # Deliberately a `.invalid` host (RFC 2606: guaranteed not to + # resolve) rather than something plausible like a loopback + # port. If this value ever *did* escape into a runtime path, + # it must fail loudly at DNS instead of quietly connecting to + # whatever happens to be listening — which is the entire + # point of removing the `http://localhost:3000` default this + # replaces. + evalOnlyPlaceholders = { + hyperhive.forge.url = "http://forge.invalid"; + }; mkContainer = module: nixpkgs.lib.nixosSystem { system = "x86_64-linux"; - modules = [ module ]; + modules = [ + module + evalOnlyPlaceholders + ]; }; in { diff --git a/hive-c0re/src/forge/mod.rs b/hive-c0re/src/forge/mod.rs index 3b343810..84ff854a 100644 --- a/hive-c0re/src/forge/mod.rs +++ b/hive-c0re/src/forge/mod.rs @@ -37,15 +37,25 @@ use users::{ const FORGE_CONTAINER: &str = "hive-forge"; -/// Base HTTP URL for the local Forgejo instance. Reads `HIVE_FORGE_URL` -/// from the environment (set unconditionally by `hive-c0re.nix` to -/// `http://`) so the forge port is never hardcoded. -/// Falls back to `http://localhost:3000` for bare runs outside the -/// NixOS module (tests, manual invocation). +/// Base HTTP URL for the local Forgejo instance, from `HIVE_FORGE_URL` +/// (set unconditionally by `hive-c0re.nix` to `http://`). +/// +/// # Panics +/// +/// When `HIVE_FORGE_URL` is unset. That is deliberate: this daemon only +/// runs under the NixOS module, which always sets it, so an unset var +/// means the deployment is broken. There is no loopback fallback, +/// because a guess is wrong in exactly the cases that matter — the +/// forge may live on a different host from the daemon, and a fallback +/// turns "misconfigured" into "silently talking to the wrong machine" +/// or, worse, "connection refused" surfacing far from its cause. pub(crate) fn forge_http_base() -> &'static str { static BASE: OnceLock = OnceLock::new(); BASE.get_or_init(|| { - std::env::var("HIVE_FORGE_URL").unwrap_or_else(|_| "http://localhost:3000".to_string()) + std::env::var("HIVE_FORGE_URL").expect( + "HIVE_FORGE_URL is unset — hive-c0re.nix sets it unconditionally, \ + so this process was started outside the NixOS module", + ) }) } @@ -53,15 +63,24 @@ pub(crate) fn forge_http_base() -> &'static str { /// `core:` credentials between the scheme and authority of /// [`forge_http_base()`] — the form git accepts for inline auth. pub(crate) fn forge_git_url(token: &str, repo: &str) -> String { - let base = forge_http_base(); - // Split on "://" to isolate scheme + authority. The base URL always - // contains "://" (validated fallback + `HIVE_FORGE_URL` is - // operator-set and expected to be well-formed). - if let Some((scheme, host)) = base.split_once("://") { - format!("{scheme}://core:{token}@{host}/{repo}.git") - } else { - format!("http://core:{token}@localhost:3000/{repo}.git") - } + git_url_with_base(forge_http_base(), token, repo) +} + +/// The credential-insertion half of [`forge_git_url`], split out so it +/// can be tested without a process-wide env var (which would race every +/// other test in this binary). +/// +/// # Panics +/// +/// When `base` has no `://`. Previously this fell back to +/// `http://core:@localhost:3000` — a guess that would have sent +/// a *credentialed* push at whatever answers on the local port. A +/// malformed base is a broken deployment; failing on it is the point. +fn git_url_with_base(base: &str, token: &str, repo: &str) -> String { + let (scheme, host) = base + .split_once("://") + .unwrap_or_else(|| panic!("HIVE_FORGE_URL is not a URL (no \"://\"): {base}")); + format!("{scheme}://core:{token}@{host}/{repo}.git") } /// Forgejo org grouping every agent's config repo. Core is a site admin diff --git a/hive-c0re/src/forge/pr_merge.rs b/hive-c0re/src/forge/pr_merge.rs index 6b685736..758ff514 100644 --- a/hive-c0re/src/forge/pr_merge.rs +++ b/hive-c0re/src/forge/pr_merge.rs @@ -253,7 +253,7 @@ pub async fn post_pr_comment(repo: &str, pr: u64, body: &str) -> Result<(), Forg #[cfg(test)] mod tests { use super::repo_agent_name; - use crate::forge::forge_git_url; + use crate::forge::git_url_with_base; #[test] fn repo_agent_name_takes_trailing_segment() { @@ -264,13 +264,23 @@ mod tests { #[test] fn forge_git_url_shape() { - // Credentials are inserted between scheme and authority; fallback - // base is `http://localhost:3000` when HIVE_FORGE_URL is unset. - let url = forge_git_url("tok", "agent-configs/iris"); - assert!(url.contains("core:tok@"), "must embed credentials: {url}"); + // Tests the pure half: credentials go between scheme and + // authority. Deliberately not via `forge_git_url`, which reads + // HIVE_FORGE_URL — setting that here would race every other + // test in this binary, and there is no fallback to lean on any + // more (a guessed base is the bug this issue removes). + let url = git_url_with_base("http://forge.example.test", "tok", "a/iris"); + assert_eq!(url, "http://core:tok@forge.example.test/a/iris.git"); + } + + #[test] + fn forge_git_url_preserves_https() { + // The scheme is carried through rather than assumed: a swarm + // whose forge is behind TLS must not be downgraded to http. + let url = git_url_with_base("https://forge.example.test", "tok", "a/iris"); assert!( - url.ends_with("/agent-configs/iris.git"), - "must end with repo path: {url}" + url.starts_with("https://core:tok@"), + "https must survive: {url}" ); } } diff --git a/nix/agent-modules/forge.nix b/nix/agent-modules/forge.nix index 038cded1..0e4eb8e9 100644 --- a/nix/agent-modules/forge.nix +++ b/nix/agent-modules/forge.nix @@ -21,7 +21,6 @@ in { options.hyperhive.forge.url = lib.mkOption { type = lib.types.str; - default = "http://localhost:3000"; example = "http://forge.internal:3000"; description = '' Base URL of the hyperhive-managed Forgejo. Used at container @@ -31,16 +30,27 @@ in shell out to `tea` without an extra auth dance. No-op when the forge-token file is missing (i.e. hive-forge isn't running on the host). + + **Required, deliberately undefaulted.** hive-c0re renders it into + every agent's config from the host's `HIVE_FORGE_URL`, which + `hive-c0re.nix` sets unconditionally --- the forge is mandatory. + A loopback default would be a guess: the forge may run on a + different host from the agents, and inside an agent's network + namespace `localhost` reaches the agent, not the forge. An + unevaluatable config is better than one that builds and then + talks to the wrong machine. ''; }; config = { assertions = [ - # hyperhive.forge.url must look like an HTTP URL when non-default. + # The empty string is the one value the type permits that cannot + # be a URL, and it is what a caller supplies when they have + # nothing --- exactly the case the removed loopback default used + # to paper over. Reject it here so the failure names the option. { assertion = - config.hyperhive.forge.url == "" - || lib.hasPrefix "http://" config.hyperhive.forge.url + lib.hasPrefix "http://" config.hyperhive.forge.url || lib.hasPrefix "https://" config.hyperhive.forge.url; message = "hyperhive.forge.url must be an http:// or https:// URL (got: \"${config.hyperhive.forge.url}\")"; }