From 44572d1e1a78e05e6346e89c846c24e25c50ad7c Mon Sep 17 00:00:00 2001 From: atlas Date: Sun, 2 Aug 2026 13:21:42 +0200 Subject: [PATCH] fix(#2911): keep the forge token out of argv MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `forge_git_url` spliced `core:@` between scheme and authority, and that URL is a process argument. `/proc//cmdline` is mode 0444 — world-readable — so the core admin token, which provisions every agent's forge account, was published to any local user for the lifetime of each git child. Seven call sites built such a URL. The credential now travels in the environment instead: `git_command_authed` sets `http.extraHeader` via `GIT_CONFIG_*`, which git reads exactly like a config file, and `/proc//environ` is 0400 — owner-only. Same credential, materially smaller audience. The remote is a plain `http://forge//.git`, and `forge_git_url` no longer takes a token, so the old shape cannot be rebuilt by accident. `knowledge`'s clone was the one place a credentialed URL was stored as a named remote — git persists the clone URL into `.git/config`, so the token sat on disk and every later `pull` authenticated from there. That is the case `forge::repos::push_config` documents as forbidden ("the tokenised URL ... deliberately never stored as a named remote"). `pull` now rewrites `origin` to the plain URL first, which also scrubs the persisted token from existing deployments, and authenticates from the environment when a token is available. The repo is public, so the pull still works without one. Three call sites also stopped spawning `Command::new("git")` directly, so they honour the `HYPERHIVE_GIT` path the NixOS module bakes in and the `kill_on_drop` every other git spawn gets. The two URL-shape tests now assert the *absence* of a credential, and a new one decodes the header back to `core:` — without that, a malformed header would leave every forge operation silently anonymous with the other assertions still green. --- hive-c0re/src/forge/mod.rs | 44 +++++++++++++++----------- hive-c0re/src/forge/pr_merge.rs | 51 ++++++++++++++++++++---------- hive-c0re/src/forge/reconcile.rs | 7 ++-- hive-c0re/src/forge/repos.rs | 17 +++++----- hive-c0re/src/lifecycle/git.rs | 36 +++++++++++++++++++-- hive-c0re/src/lifecycle/mod.rs | 5 +-- hive-c0re/src/workers/knowledge.rs | 30 ++++++++++++++---- 7 files changed, 134 insertions(+), 56 deletions(-) diff --git a/hive-c0re/src/forge/mod.rs b/hive-c0re/src/forge/mod.rs index b077b979..468d3904 100644 --- a/hive-c0re/src/forge/mod.rs +++ b/hive-c0re/src/forge/mod.rs @@ -59,28 +59,34 @@ pub(crate) fn forge_http_base() -> &'static str { }) } -/// Token-in-URL git remote for `repo` (e.g. `"core/meta"`). Inserts -/// `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 { - git_url_with_base(forge_http_base(), token, repo) +/// Git remote for `repo` (e.g. `"core/meta"`) — **credential-free**. +/// +/// The token does not go here. A URL is a process argument, and `argv` is +/// world-readable through `/proc//cmdline` for as long as the git child +/// lives, so a credentialed remote publishes the core admin token to every +/// local user on the host. Credentials travel in the environment instead, via +/// [`core_auth_header`] and [`crate::lifecycle::git_command_authed`] — +/// `/proc//environ` is owner-only. +pub(crate) fn forge_git_url(repo: &str) -> String { + git_url_with_base(forge_http_base(), 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). +/// The pure 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). +fn git_url_with_base(base: &str, repo: &str) -> String { + format!("{base}/{repo}.git") +} + +/// The `http.extraHeader` value authenticating as the forge core user. /// -/// # 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") +/// Basic auth over a header rather than userinfo in the URL, so the secret +/// reaches git through the environment (see [`forge_git_url`]). Pair with +/// [`crate::lifecycle::git_command_authed`], which is the only thing that +/// should ever hold the result. +pub(crate) fn core_auth_header(token: &str) -> String { + use base64::Engine as _; + let basic = base64::engine::general_purpose::STANDARD.encode(format!("core:{token}")); + format!("Authorization: Basic {basic}") } /// 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 758ff514..62972722 100644 --- a/hive-c0re/src/forge/pr_merge.rs +++ b/hive-c0re/src/forge/pr_merge.rs @@ -6,7 +6,7 @@ use anyhow::Context; use forgejo_api::structs::{MergePullRequestOption, MergePullRequestOptionDo, StateType}; -use super::{CONFIG_ORG, api, core_token, forge_git_url}; +use super::{CONFIG_ORG, api, core_auth_header, core_token, forge_git_url}; // --------------------------------------------------------------------------- // PR-based config-flow merge primitives (part of the @@ -74,9 +74,9 @@ fn repo_agent_name(repo: &str) -> &str { pub async fn pr_head_sha(repo: &str, pr: u64) -> Result { let token = core_token() .ok_or_else(|| ForgeMergeError::Other(anyhow::anyhow!("forge core token absent")))?; - let url = forge_git_url(&token, repo); + let url = forge_git_url(repo); let refspec = format!("refs/pull/{pr}/head"); - let out = crate::lifecycle::git_command() + let out = crate::lifecycle::git_command_authed(&core_auth_header(&token)) .args(["ls-remote", &url, &refspec]) .output() .await @@ -142,10 +142,10 @@ pub fn config_repo(agent: &str) -> String { pub async fn fetch_pr_head_into_applied(repo: &str, pr: u64) -> Result<(), ForgeMergeError> { let token = core_token() .ok_or_else(|| ForgeMergeError::Other(anyhow::anyhow!("forge core token absent")))?; - let url = forge_git_url(&token, repo); + let url = forge_git_url(repo); let applied = crate::paths::applied_dir(repo_agent_name(repo)); let refspec = format!("refs/pull/{pr}/head"); - let out = crate::lifecycle::git_command() + let out = crate::lifecycle::git_command_authed(&core_auth_header(&token)) .current_dir(&applied) .args(["fetch", "--no-tags", &url, &refspec]) .output() @@ -262,25 +262,44 @@ mod tests { assert_eq!(repo_agent_name("a/b/c"), "c"); } + /// The remote carries **no credential**. `argv` is world-readable through + /// `/proc//cmdline`, so a token spliced in here would be published to + /// every local user for the life of the git child. + /// + /// Deliberately not via `forge_git_url`, which reads `HIVE_FORGE_URL` — + /// setting that here would race every other test in this binary. #[test] - fn forge_git_url_shape() { - // 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"); + fn forge_git_url_carries_no_credential() { + let url = git_url_with_base("http://forge.example.test", "a/iris"); + assert_eq!(url, "http://forge.example.test/a/iris.git"); + assert!(!url.contains('@'), "no userinfo: {url}"); } #[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"); + let url = git_url_with_base("https://forge.example.test", "a/iris"); + assert!(url.starts_with("https://"), "https must survive: {url}"); + } + + /// The credential goes in an `Authorization` header instead — decodable + /// back to `core:`, so the swap is genuinely equivalent auth and + /// not a silent downgrade to anonymous. + #[test] + fn core_auth_header_is_basic_core_token() { + use base64::Engine as _; + let header = crate::forge::core_auth_header("s3cret"); + let b64 = header + .strip_prefix("Authorization: Basic ") + .expect("basic auth header"); + let decoded = base64::engine::general_purpose::STANDARD + .decode(b64) + .expect("valid base64"); + assert_eq!(String::from_utf8_lossy(&decoded), "core:s3cret"); assert!( - url.starts_with("https://core:tok@"), - "https must survive: {url}" + !header.contains("s3cret"), + "token not in the clear: {header}" ); } } diff --git a/hive-c0re/src/forge/reconcile.rs b/hive-c0re/src/forge/reconcile.rs index 23ca5b96..d38ff882 100644 --- a/hive-c0re/src/forge/reconcile.rs +++ b/hive-c0re/src/forge/reconcile.rs @@ -17,7 +17,7 @@ use anyhow::{Context, Result}; use hive_host_sock::{HostResponse, ReconcileDirection}; -use super::{CONFIG_ORG, core_token, forge_git_url, is_present}; +use super::{CONFIG_ORG, core_auth_header, core_token, forge_git_url, is_present}; /// Scratch ref the forge `main` is fetched into — outside the normal /// branch/tag namespace so it never collides with real refs. @@ -36,10 +36,11 @@ async fn fetch_forge_main(agent: &str) -> Result { if !dir.join(".git").exists() { anyhow::bail!("agent `{agent}` has no applied config checkout"); } - let url = forge_git_url(&token, &format!("{CONFIG_ORG}/{agent}")); - crate::lifecycle::git( + let url = forge_git_url(&format!("{CONFIG_ORG}/{agent}")); + crate::lifecycle::git_authed( &dir, &["fetch", "--force", &url, &format!("main:{FORGE_MAIN_REF}")], + &core_auth_header(&token), ) .await .context("fetch forge config main (does forge main exist yet?)")?; diff --git a/hive-c0re/src/forge/repos.rs b/hive-c0re/src/forge/repos.rs index 5a6957a8..a65f1bc0 100644 --- a/hive-c0re/src/forge/repos.rs +++ b/hive-c0re/src/forge/repos.rs @@ -16,13 +16,12 @@ use forgejo_api::structs::{ }; use forgejo_api::{ApiErrorKind, ForgejoError}; use reqwest::StatusCode; -use tokio::process::Command; use crate::coordinator::Coordinator; use super::{ AGENTS_ORG, CONFIG_ORG, KNOWLEDGE_REPO, OPERATORS_TEAM, SHARED_DOCS_REPO, SHARED_ORG, api, - core_token, forge_git_url, forge_http_base, is_present, + core_auth_header, core_token, forge_git_url, forge_http_base, is_present, }; /// Creation options for an empty repo defaulting to `main`. @@ -239,8 +238,8 @@ pub async fn push_meta(dir: &Path) -> Result<()> { // raise a dashboard warning and leave the remote intact rather than // silently erasing its history — consistent with `push_config`'s // non-fast-forward handling. - let url = forge_git_url(&token, "core/meta"); - let out = Command::new("git") + let url = forge_git_url("core/meta"); + let out = crate::lifecycle::git_command_authed(&core_auth_header(&token)) .current_dir(dir) .args(["push", &url, "HEAD:main"]) .output() @@ -482,9 +481,10 @@ pub async fn push_config(name: &str) -> Result<()> { if !dir.join(".git").exists() { return Ok(()); } - let url = forge_git_url(&token, &format!("{CONFIG_ORG}/{name}")); + let url = forge_git_url(&format!("{CONFIG_ORG}/{name}")); + let auth = core_auth_header(&token); // Tags first, in their own push, so they land regardless of main's fate. - let out = run_config_push(&dir, &url, "refs/tags/*:refs/tags/*").await?; + let out = run_config_push(&dir, &url, &auth, "refs/tags/*:refs/tags/*").await?; if !out.status.success() { anyhow::bail!( "git push tags {CONFIG_ORG}/{name} failed ({}): {}", @@ -495,7 +495,7 @@ pub async fn push_config(name: &str) -> Result<()> { // Then main on its own — a protected-branch / non-ff reject of an // established main is expected (pr_merge owns main); only the initial // empty-repo seed actually advances it here. - let out = run_config_push(&dir, &url, "refs/heads/main:refs/heads/main").await?; + let out = run_config_push(&dir, &url, &auth, "refs/heads/main:refs/heads/main").await?; if !out.status.success() { let stderr = String::from_utf8_lossy(&out.stderr); if stderr.contains("non-fast-forward") @@ -525,9 +525,10 @@ pub async fn push_config(name: &str) -> Result<()> { async fn run_config_push( dir: &std::path::Path, url: &str, + auth: &str, refspec: &str, ) -> Result { - crate::lifecycle::git_command() + crate::lifecycle::git_command_authed(auth) .current_dir(dir) .args(["push", url, refspec]) .output() diff --git a/hive-c0re/src/lifecycle/git.rs b/hive-c0re/src/lifecycle/git.rs index 9a7b190d..cc5712e0 100644 --- a/hive-c0re/src/lifecycle/git.rs +++ b/hive-c0re/src/lifecycle/git.rs @@ -60,8 +60,29 @@ pub fn git_command() -> Command { cmd } -pub async fn git(dir: &Path, args: &[&str]) -> Result<()> { - let out = git_command() +/// [`git_command`] carrying HTTP credentials **in its environment**. +/// +/// `auth_header` is an `Authorization:` line (see +/// [`crate::forge::core_auth_header`]) handed to git as `http.extraHeader` +/// through `GIT_CONFIG_*`, which git reads exactly like a config file. The +/// alternative — userinfo in the remote URL — puts the secret in `argv`, and +/// `/proc//cmdline` is world-readable while `/proc//environ` is +/// owner-only. Same credential, materially smaller audience. +/// +/// `GIT_CONFIG_COUNT` needs git >= 2.31; `HYPERHIVE_GIT` points at a pinned +/// nixpkgs git well past that. +#[must_use] +pub fn git_command_authed(auth_header: &str) -> Command { + let mut cmd = git_command(); + cmd.env("GIT_CONFIG_COUNT", "1") + .env("GIT_CONFIG_KEY_0", "http.extraHeader") + .env("GIT_CONFIG_VALUE_0", auth_header); + cmd +} + +/// Run `cmd` as `git ` in `dir`, erroring on a non-zero exit. +async fn run(mut cmd: Command, dir: &Path, args: &[&str]) -> Result<()> { + let out = cmd .current_dir(dir) .args(args) .output() @@ -78,6 +99,17 @@ pub async fn git(dir: &Path, args: &[&str]) -> Result<()> { Ok(()) } +pub async fn git(dir: &Path, args: &[&str]) -> Result<()> { + run(git_command(), dir, args).await +} + +/// [`git`] against an authenticated remote — for the paths that talk to the +/// forge. The credential rides the environment, so `args` (and therefore the +/// error above) stay free of it. +pub async fn git_authed(dir: &Path, args: &[&str], auth_header: &str) -> Result<()> { + run(git_command_authed(auth_header), dir, args).await +} + /// Resolve `refname` (a tag, branch, or sha) in `dir` to its full sha. pub async fn git_rev_parse(dir: &Path, refname: &str) -> Result { let out = git_command() diff --git a/hive-c0re/src/lifecycle/mod.rs b/hive-c0re/src/lifecycle/mod.rs index 52d5aa41..7ff2b32f 100644 --- a/hive-c0re/src/lifecycle/mod.rs +++ b/hive-c0re/src/lifecycle/mod.rs @@ -7,8 +7,9 @@ mod setup; mod tests; pub use git::{ - git, git_command, git_delete_ref, git_is_ancestor, git_read_tree_reset, git_rev_parse, git_tag, - git_tag_annotated, git_update_ref, git_update_ref_cas, + git, git_authed, git_command, git_command_authed, git_delete_ref, git_is_ancestor, + git_read_tree_reset, git_rev_parse, git_tag, git_tag_annotated, git_update_ref, + git_update_ref_cas, }; pub use host_config::write_dropins; pub use setup::{ diff --git a/hive-c0re/src/workers/knowledge.rs b/hive-c0re/src/workers/knowledge.rs index 553b3268..180183c6 100644 --- a/hive-c0re/src/workers/knowledge.rs +++ b/hive-c0re/src/workers/knowledge.rs @@ -17,7 +17,7 @@ use anyhow::{Context, Result}; use forgejo_api::structs::{CreateHookOption, CreateHookOptionConfig, CreateHookOptionType}; use crate::coordinator::Coordinator; -use crate::forge::forge_git_url; +use crate::forge::{core_auth_header, forge_git_url}; pub const ORG: &str = "internal"; pub const REPO: &str = "knowledge"; @@ -73,8 +73,8 @@ pub async fn ensure_local_clone(core_token: &str) -> Result<()> { return Ok(()); } std::fs::create_dir_all(LOCAL_DIR).context("create knowledge local dir")?; - let url = forge_git_url(core_token, &format!("{ORG}/{REPO}")); - let out = tokio::process::Command::new("git") + let url = forge_git_url(&format!("{ORG}/{REPO}")); + let out = crate::lifecycle::git_command_authed(&core_auth_header(core_token)) .args(["clone", &url, LOCAL_DIR]) .output() .await @@ -127,8 +127,8 @@ async fn seed_readme(core_token: &str) -> Result<()> { anyhow::bail!("git {args:?} failed: {stderr}"); } } - let url = forge_git_url(core_token, &format!("{ORG}/{REPO}")); - let out = tokio::process::Command::new("git") + let url = forge_git_url(&format!("{ORG}/{REPO}")); + let out = crate::lifecycle::git_command_authed(&core_auth_header(core_token)) .args(["-C", LOCAL_DIR, "push", &url, "HEAD:main"]) .output() .await @@ -273,8 +273,26 @@ pub async fn pull(coord: &Coordinator) -> Result<()> { if !git_dir.exists() { anyhow::bail!("knowledge: {LOCAL_DIR}/.git not found — clone first"); } + // Rewrite any credentialed `origin` left by an older clone, which spliced + // `core:@` into the URL and persisted it in `.git/config` — the one + // place this repo stored a token in a named remote, against the rule + // `forge::repos::push_config` states. Harmless to repeat once the URL is + // already clean. + let plain = crate::forge::forge_git_url(&format!("{ORG}/{REPO}")); + let _ = crate::lifecycle::git_command() + .args(["-C", LOCAL_DIR, "remote", "set-url", "origin", &plain]) + .output() + .await; let before = head_sha().await; - let out = tokio::process::Command::new("git") + // The repo is public (`ensure_knowledge_repo` makes it so), so an + // unauthenticated pull is enough — but authenticate when a token is around, + // which keeps this working if the repo is ever made private again. + let auth = crate::forge::core_token().map(|t| core_auth_header(&t)); + let mut cmd = match &auth { + Some(header) => crate::lifecycle::git_command_authed(header), + None => crate::lifecycle::git_command(), + }; + let out = cmd .args(["-C", LOCAL_DIR, "pull", "--ff-only"]) .output() .await