From b981b6fea0325e6c091fb7896ef827ef62f47c9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Balatoni?= Date: Sun, 25 Oct 2026 00:44:33 +0200 Subject: [PATCH] feat(petri): scope git credentials to the configured host --- lib/apps/fabro-server/src/run_manifest.rs | 10 +- lib/components/fabro-github/src/lib.rs | 31 +++- .../fabro-github/tests/live_access.rs | 2 +- .../fabro-petri/src/stage_credentials.rs | 168 +++++++++++++++--- lib/foundation/fabro-static/src/env_vars.rs | 4 + .../fabro-static/src/secret_registry.rs | 2 + 6 files changed, 186 insertions(+), 31 deletions(-) diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index 66cec5868..22c9dc08d 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -1247,7 +1247,7 @@ async fn probe_github_repository( host: &fabro_types::GitHubHost, ) -> std::result::Result<(), String> { let url = slug.https_url(host); - probe_with_replication_retry(token.snapshot, || run_probe_ls_remote(&url, &token)).await + probe_with_replication_retry(token.snapshot, || run_probe_ls_remote(host, &url, &token)).await } /// Retry auth-shaped failures with the SAME token: replication of a given @@ -1370,9 +1370,13 @@ where } } -async fn run_probe_ls_remote(url: &str, token: &ResolvedToken) -> std::result::Result<(), String> { +async fn run_probe_ls_remote( + host: &fabro_types::GitHubHost, + url: &str, + token: &ResolvedToken, +) -> std::result::Result<(), String> { let mut command = Command::new("git"); - fabro_github::apply_probe_git_env(&mut command, token.token.expose()); + fabro_github::apply_probe_git_env(&mut command, host, token.token.expose()); command.args(["ls-remote", url, "HEAD"]); run_ls_remote(command).await } diff --git a/lib/components/fabro-github/src/lib.rs b/lib/components/fabro-github/src/lib.rs index 120b33a0e..8f45ea432 100644 --- a/lib/components/fabro-github/src/lib.rs +++ b/lib/components/fabro-github/src/lib.rs @@ -25,10 +25,6 @@ pub(crate) mod tests_mock; pub use access::GitHubRepositoryAccess; pub use identity::GitHubAccountIdentity; -/// Git config key that routes github.com HTTPS credentials through -/// [`GITHUB_CREDENTIAL_HELPER`]. -pub const GITHUB_CREDENTIAL_HELPER_KEY: &str = "credential.https://github.com.helper"; - /// Secret-free git credential helper: reads `$GITHUB_TOKEN` from the /// invoking git process's environment at invocation time, so the token never /// lands in git configuration, argv, or rendered errors. Non-`get` @@ -37,18 +33,18 @@ pub const GITHUB_CREDENTIAL_HELPER_KEY: &str = "credential.https://github.com.he /// test, so the probes always exercise exactly what the bridge configures. pub const GITHUB_CREDENTIAL_HELPER: &str = r#"!f() { if [ "$1" = get ]; then echo username=x-access-token; echo "password=$GITHUB_TOKEN"; fi; }; f"#; -/// Configure a `git` invocation to authenticate to github.com through +/// Configure a `git` invocation to authenticate to `host` through /// [`GITHUB_CREDENTIAL_HELPER`] with `token`, isolated from user/system git /// configuration and terminal prompts. The token is passed only through the /// child process environment. -pub fn apply_probe_git_env(command: &mut Command, token: &str) { +pub fn apply_probe_git_env(command: &mut Command, host: &GitHubHost, token: &str) { command .env("GIT_TERMINAL_PROMPT", "0") .env("GIT_CONFIG_NOSYSTEM", "1") .env("GIT_CONFIG_GLOBAL", "/dev/null") .env(EnvVars::GITHUB_TOKEN, token) .env("GIT_CONFIG_COUNT", "1") - .env("GIT_CONFIG_KEY_0", GITHUB_CREDENTIAL_HELPER_KEY) + .env("GIT_CONFIG_KEY_0", host.credential_helper_key()) .env("GIT_CONFIG_VALUE_0", GITHUB_CREDENTIAL_HELPER); } @@ -1710,6 +1706,27 @@ mod tests { } } + #[test] + fn probe_git_env_keys_the_helper_to_the_host() { + let key_for = |host: &GitHubHost| { + let mut command = Command::new("git"); + apply_probe_git_env(&mut command, host, "token"); + command + .as_std() + .get_envs() + .find(|(name, _)| *name == "GIT_CONFIG_KEY_0") + .and_then(|(_, value)| value.map(|value| value.to_string_lossy().into_owned())) + }; + assert_eq!( + key_for(&GitHubHost::default()).as_deref(), + Some("credential.https://github.com.helper") + ); + assert_eq!( + key_for(&ghes()).as_deref(), + Some("credential.https://ghe.example.com.helper") + ); + } + #[test] fn installation_url_uses_the_web_origin() { let creds = app_with_slug(Some("fabro")); diff --git a/lib/components/fabro-github/tests/live_access.rs b/lib/components/fabro-github/tests/live_access.rs index 7dfc038b0..5a2e36bc4 100644 --- a/lib/components/fabro-github/tests/live_access.rs +++ b/lib/components/fabro-github/tests/live_access.rs @@ -36,7 +36,7 @@ fn env_var(name: &str) -> String { async fn ls_remote_with_token(slug: &GitHubRepositorySlug, token: &str) -> bool { let url = slug.https_url(&GitHubHost::default()); let mut command = Command::new("git"); - fabro_github::apply_probe_git_env(&mut command, token); + fabro_github::apply_probe_git_env(&mut command, &GitHubHost::default(), token); let output = command .args(["ls-remote", &url, "HEAD"]) .stdout(Stdio::null()) diff --git a/lib/components/fabro-petri/src/stage_credentials.rs b/lib/components/fabro-petri/src/stage_credentials.rs index 25dc85b0b..6bdbbdab4 100644 --- a/lib/components/fabro-petri/src/stage_credentials.rs +++ b/lib/components/fabro-petri/src/stage_credentials.rs @@ -12,7 +12,7 @@ use std::time::Duration; use async_trait::async_trait; use fabro_github::token_source::{InstallationTokenSource, ResolvedToken}; -use fabro_github::{GITHUB_CREDENTIAL_HELPER_KEY, GitHubCredentials, GitHubRepositoryAccess}; +use fabro_github::{GitHubCredentials, GitHubRepositoryAccess}; use fabro_static::EnvVars; use fabro_types::settings::run::{RunIntegrationsGithubSettings, RunMode}; use fabro_types::{GitHubHost, GitHubRepositorySlug, RunSpec, RunTarget}; @@ -38,6 +38,7 @@ pub struct StageCredentials { git_tokens: Arc, api_tokens: Option>, repositories: Vec, + host: GitHubHost, } impl StageCredentials { @@ -98,6 +99,7 @@ impl StageCredentials { git_tokens, api_tokens, repositories, + host: host.clone(), })) } @@ -263,14 +265,15 @@ impl CredentialEnv { } let password = utf8_percent_encode(token.token.expose(), NON_ALPHANUMERIC).to_string(); self.masker.register_explicit(&password); + let authority = self.credentials.host.host(); let store = self .credentials .repositories .iter() .flat_map(|repo| { [ - format!("https://x-access-token:{password}@github.com/{repo}\n"), - format!("https://x-access-token:{password}@github.com/{repo}.git\n"), + format!("https://x-access-token:{password}@{authority}/{repo}\n"), + format!("https://x-access-token:{password}@{authority}/{repo}.git\n"), ] }) .collect::(); @@ -309,24 +312,18 @@ impl CredentialEnv { })?; // Preserve fetch/publish headers and workflow configuration. The store // returns a credential only for an allowed repository path. + let host = &self.credentials.host; + let helper_key = host.credential_helper_key(); + let rewrite_key = format!("url.{}/.insteadOf", host.web_url()); let entries = [ ( - "credential.https://github.com.useHttpPath", + format!("credential.{}.useHttpPath", host.web_url()), "true".to_string(), ), - (GITHUB_CREDENTIAL_HELPER_KEY, String::new()), - ( - GITHUB_CREDENTIAL_HELPER_KEY, - format!("store --file={}", self.store_path()), - ), - ( - "url.https://github.com/.insteadOf", - "git@github.com:".to_string(), - ), - ( - "url.https://github.com/.insteadOf", - "ssh://git@github.com/".to_string(), - ), + (helper_key.clone(), String::new()), + (helper_key, format!("store --file={}", self.store_path())), + (rewrite_key.clone(), format!("git@{}:", host.host())), + (rewrite_key, format!("ssh://git@{}/", host.host())), ]; for index in 0..count { for prefix in ["GIT_CONFIG_KEY_", "GIT_CONFIG_VALUE_"] { @@ -341,7 +338,7 @@ impl CredentialEnv { for (offset, (key, value)) in entries.iter().enumerate() { env.insert( format!("GIT_CONFIG_KEY_{}", count + offset).into(), - (*key).into(), + key.as_str().into(), ); env.insert( format!("GIT_CONFIG_VALUE_{}", count + offset).into(), @@ -371,6 +368,15 @@ impl SpawnEnv for CredentialEnv { if let Some(tokens) = &self.credentials.api_tokens { let resolved = self.resolve(tokens, "resolve").await?; env.insert(EnvVars::GITHUB_TOKEN.into(), resolved.token.expose().into()); + // The `gh` CLI reads GITHUB_TOKEN for github.com only; any other + // host needs GH_HOST and GH_ENTERPRISE_TOKEN. + if !self.credentials.host.is_github_com() { + env.insert(EnvVars::GH_HOST.into(), self.credentials.host.host().into()); + env.insert( + EnvVars::GH_ENTERPRISE_TOKEN.into(), + resolved.token.expose().into(), + ); + } } // The store and the helper configuration that names it live in the // scope's sandbox; a one-shot container cannot read either. @@ -467,17 +473,139 @@ mod tests { } fn credentials(api: bool) -> StageCredentials { + credentials_on(api, GitHubHost::default()) + } + + fn credentials_on(api: bool, host: GitHubHost) -> StageCredentials { let tokens = test_support::installation_token_source( "acme/private", Arc::new(RotatingMinter(AtomicUsize::new(0))), ); StageCredentials { - git_tokens: tokens.clone(), - api_tokens: api.then_some(tokens), + git_tokens: tokens.clone(), + api_tokens: api.then_some(tokens), repositories: vec![GitHubRepositorySlug::try_new("acme/private").expect("slug")], + host, } } + #[expect( + clippy::disallowed_methods, + reason = "sync read of a test fixture's credential store; not on a Tokio path" + )] + /// The environment a process spawn in a scope receives under `host`, and + /// the credential store written for it. + async fn process_env_on(host: GitHubHost) -> (BTreeMap, String) { + let dir = tempfile::tempdir().expect("directory"); + let runtime = providers::standard_runtime(&SandboxProviderConfig::default()); + let router = runtime.sandbox_router_for(dir.path()).expect("router"); + let handle = router + .acquire( + &ScopeSpec::new(ScopeId::new(0), "host-credentials"), + &AcquireContext::bare(), + ) + .await + .expect("acquire"); + let store = dir.path().join("store-directory"); + fs::create_dir(&store).expect("store directory"); + let env = CredentialEnv { + inner: handle.exec(), + credentials: credentials_on(true, host), + masker: MapSecrets::empty().masker(), + directory: store.display().to_string(), + written: Mutex::new(None), + }; + let mut process = BTreeMap::new(); + env.apply(SpawnTarget::Process, &mut process) + .await + .expect("apply"); + let written = fs::read_to_string(store.join("store")).expect("store"); + assert!( + router + .release(handle, ScopeOutcome::Succeeded) + .await + .is_clean() + ); + (process, written) + } + + /// The git config entries of `env`, in order, as `(key, value)`. + fn git_config(env: &BTreeMap) -> Vec<(String, String)> { + let count: usize = env["GIT_CONFIG_COUNT"].parse().expect("count"); + (0..count) + .map(|index| { + ( + env[format!("GIT_CONFIG_KEY_{index}").as_str()].to_string(), + env[format!("GIT_CONFIG_VALUE_{index}").as_str()].to_string(), + ) + }) + .collect() + } + + #[tokio::test] + async fn github_com_scopes_keep_the_default_git_config_and_no_gh_host() { + let (env, store) = process_env_on(GitHubHost::default()).await; + let config = git_config(&env); + let keys: Vec<_> = config.iter().map(|(key, _)| key.as_str()).collect(); + assert_eq!(keys, [ + "credential.https://github.com.useHttpPath", + "credential.https://github.com.helper", + "credential.https://github.com.helper", + "url.https://github.com/.insteadOf", + "url.https://github.com/.insteadOf", + ]); + assert_eq!(config[3].1, "git@github.com:"); + assert_eq!(config[4].1, "ssh://git@github.com/"); + assert!(store.contains("@github.com/acme/private\n")); + assert!(store.contains("@github.com/acme/private.git\n")); + assert!(!env.contains_key("GH_HOST")); + assert!(!env.contains_key("GH_ENTERPRISE_TOKEN")); + } + + #[tokio::test] + async fn a_ghes_scope_scopes_git_and_gh_credentials_to_its_host() { + let host = GitHubHost::new("https://ghe.example.com", None).expect("host"); + let (env, store) = process_env_on(host).await; + let config = git_config(&env); + assert_eq!( + config[0], + ( + "credential.https://ghe.example.com.useHttpPath".to_string(), + "true".to_string() + ) + ); + assert_eq!( + config[1], + ( + "credential.https://ghe.example.com.helper".to_string(), + String::new() + ) + ); + assert_eq!(config[2].0, "credential.https://ghe.example.com.helper"); + assert!(config[2].1.starts_with("store --file=")); + assert_eq!( + config[3], + ( + "url.https://ghe.example.com/.insteadOf".to_string(), + "git@ghe.example.com:".to_string() + ) + ); + assert_eq!( + config[4], + ( + "url.https://ghe.example.com/.insteadOf".to_string(), + "ssh://git@ghe.example.com/".to_string() + ) + ); + assert!(config.iter().all(|(key, _)| !key.contains("github.com"))); + assert!(store.contains("@ghe.example.com/acme/private\n")); + assert!(store.contains("@ghe.example.com/acme/private.git\n")); + assert!(!store.contains("github.com")); + assert_eq!(env["GH_HOST"], "ghe.example.com"); + assert_eq!(env["GH_ENTERPRISE_TOKEN"], env["GITHUB_TOKEN"]); + assert!(env["GITHUB_TOKEN"].starts_with("scripted-token-generation-")); + } + #[test] fn default_origin_access_does_not_request_an_api_token() { let mut spec = types_support::test_run_spec(); diff --git a/lib/foundation/fabro-static/src/env_vars.rs b/lib/foundation/fabro-static/src/env_vars.rs index 6536f2e32..330c6f0a2 100644 --- a/lib/foundation/fabro-static/src/env_vars.rs +++ b/lib/foundation/fabro-static/src/env_vars.rs @@ -118,6 +118,8 @@ impl EnvVars { pub const ZAI_API_KEY: &'static str = "ZAI_API_KEY"; // GitHub, OAuth, and Slack + pub const GH_ENTERPRISE_TOKEN: &'static str = "GH_ENTERPRISE_TOKEN"; + pub const GH_HOST: &'static str = "GH_HOST"; pub const GH_TOKEN: &'static str = "GH_TOKEN"; pub const GITHUB_APP_CLIENT_SECRET: &'static str = "GITHUB_APP_CLIENT_SECRET"; pub const GITHUB_APP_PRIVATE_KEY: &'static str = "GITHUB_APP_PRIVATE_KEY"; @@ -280,6 +282,8 @@ mod tests { EnvVars::OPENROUTER_API_KEY, EnvVars::POOLSIDE_API_KEY, EnvVars::ZAI_API_KEY, + EnvVars::GH_ENTERPRISE_TOKEN, + EnvVars::GH_HOST, EnvVars::GH_TOKEN, EnvVars::GITHUB_APP_CLIENT_SECRET, EnvVars::GITHUB_APP_PRIVATE_KEY, diff --git a/lib/foundation/fabro-static/src/secret_registry.rs b/lib/foundation/fabro-static/src/secret_registry.rs index c6759e2b0..127bc77ad 100644 --- a/lib/foundation/fabro-static/src/secret_registry.rs +++ b/lib/foundation/fabro-static/src/secret_registry.rs @@ -121,6 +121,8 @@ mod tests { #[test] fn leaves_legacy_aliases_and_non_secret_config_unclassified() { for name in [ + EnvVars::GH_ENTERPRISE_TOKEN, + EnvVars::GH_HOST, EnvVars::GH_TOKEN, EnvVars::GITHUB_BASE_URL, EnvVars::GITHUB_WEB_URL,