From 41df57b8a01098c53232d5bb36c16ff441be684e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Balatoni?= Date: Sat, 24 Oct 2026 23:09:04 +0200 Subject: [PATCH 01/13] feat(config): add GitHub host settings --- docs/public/api-reference/fabro-api.yaml | 10 + .../fabro-cli/src/commands/run/publish.rs | 12 +- lib/apps/fabro-server/src/diagnostics.rs | 4 +- lib/apps/fabro-server/src/serve.rs | 2 +- lib/apps/fabro-server/src/server.rs | 49 +-- .../src/server/handler/pull_requests.rs | 2 +- .../fabro-server/src/server/handler/system.rs | 2 +- lib/apps/fabro-server/src/server/tests.rs | 10 +- lib/apps/fabro-server/src/spawn_env.rs | 5 + lib/apps/fabro-server/src/test_support.rs | 2 +- lib/components/fabro-github/src/lib.rs | 59 ++- .../fabro-config/src/layers/server.rs | 4 + .../fabro-config/src/resolve/server.rs | 43 +- .../fabro-config/src/tests/resolve_server.rs | 70 +++- lib/foundation/fabro-static/src/env_vars.rs | 2 + .../fabro-static/src/secret_registry.rs | 1 + lib/foundation/fabro-types/src/github_host.rs | 368 ++++++++++++++++++ lib/foundation/fabro-types/src/lib.rs | 2 + .../fabro-types/src/settings/server.rs | 5 +- .../src/models/github-integration-settings.ts | 8 + 20 files changed, 605 insertions(+), 55 deletions(-) create mode 100644 lib/foundation/fabro-types/src/github_host.rs diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index a734c4ddc..669b683e0 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -15672,7 +15672,17 @@ components: - client_id - slug - webhooks + - web_url + - api_url properties: + web_url: + type: string + description: Web origin of the GitHub deployment, without a trailing slash. + example: https://github.com + api_url: + type: string + description: REST API base URL of the GitHub deployment, without a trailing slash. + example: https://api.github.com enabled: type: boolean strategy: diff --git a/lib/apps/fabro-cli/src/commands/run/publish.rs b/lib/apps/fabro-cli/src/commands/run/publish.rs index 8555f3161..b4850eec4 100644 --- a/lib/apps/fabro-cli/src/commands/run/publish.rs +++ b/lib/apps/fabro-cli/src/commands/run/publish.rs @@ -39,7 +39,7 @@ use fabro_static::EnvVars; use fabro_store::platform_records::{PlatformRecord, PullRequestCreatedRecord}; use fabro_types::settings::run::{PullRequestSettings, RunMode}; use fabro_types::settings::server::GithubIntegrationStrategy; -use fabro_types::{GitHubRepositorySlug, RunId, RunSpec, RunTarget}; +use fabro_types::{GitHubHost, GitHubRepositorySlug, RunId, RunSpec, RunTarget}; use fabro_vault::Vault; use fabro_workflow::pull_request::{self, AutoMergeOptions, OpenPullRequestRequest}; use tokio::time; @@ -52,6 +52,12 @@ const PUSH_TIMEOUT: Duration = Duration::from_mins(5); const PUSH_ATTEMPTS: u32 = 3; const PUSH_RETRY_DELAY: Duration = Duration::from_secs(2); +/// The GitHub deployment the server is configured for. +pub(super) fn github_host() -> Result { + let settings = ServerSettingsBuilder::load_default().context("loading the server settings")?; + fabro_github::github_host(&settings.server.integrations.github) +} + /// The server's GitHub credentials as the worker reaches them, or `None` /// when none are configured. pub(super) fn github_credentials(vault: &Vault) -> Result> { @@ -313,8 +319,8 @@ impl RunPublisher for GitHubPublisher { .credentials .as_ref() .ok_or_else(|| "GitHub credentials are unavailable".to_string())?; - let base_url = fabro_github::github_api_base_url(); - let context = GitHubContext::new(credentials, &base_url); + let host = github_host().map_err(|err| format!("{err:#}"))?; + let context = GitHubContext::new(credentials, host.api_url()); Box::pin(self.open_pull_request(context, settings, publication)).await } } diff --git a/lib/apps/fabro-server/src/diagnostics.rs b/lib/apps/fabro-server/src/diagnostics.rs index d9f5111a2..5d43949e0 100644 --- a/lib/apps/fabro-server/src/diagnostics.rs +++ b/lib/apps/fabro-server/src/diagnostics.rs @@ -399,7 +399,7 @@ async fn check_github_app(state: &AppState) -> CheckResult { }; let probe = timeout( EXTERNAL_SERVICE_PROBE_TIMEOUT, - http.get(format!("{}/user", fabro_github::github_api_base_url())) + http.get(format!("{}/user", state.github_host.api_url())) .header("Authorization", format!("Bearer {token}")) .header("Accept", "application/vnd.github+json") .header("User-Agent", "fabro-server") @@ -549,7 +549,7 @@ async fn check_github_app(state: &AppState) -> CheckResult { }; let auth_result = timeout( EXTERNAL_SERVICE_PROBE_TIMEOUT, - fabro_github::get_authenticated_app(&http, &jwt, &fabro_github::github_api_base_url()), + fabro_github::get_authenticated_app(&http, &jwt, state.github_host.api_url()), ) .await; match auth_result { diff --git a/lib/apps/fabro-server/src/serve.rs b/lib/apps/fabro-server/src/serve.rs index deedd584f..06e8cc56d 100644 --- a/lib/apps/fabro-server/src/serve.rs +++ b/lib/apps/fabro-server/src/serve.rs @@ -737,7 +737,7 @@ where preloaded_vault: startup_vault.into_vault(), server_secrets, env_lookup, - github_api_base_url: None, + github_host: None, active_config_path, http_client: None, sandbox_inventory: None, diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index 413c8c608..91f7bfa30 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -1152,28 +1152,28 @@ pub struct AppState { control_request_locks: KeyedMutex, parent_link_lock: AsyncMutex<()>, - pub(super) server_secrets: ServerSecrets, - pub(crate) llm_source: Arc, + pub(super) server_secrets: ServerSecrets, + pub(crate) llm_source: Arc, /// The database pool the stores share, for the Petri run store a /// server-process run writes its records through. - pub(crate) db_pool: DbPool, - manifest_run_defaults: RwLock>, - manifest_run_settings: RwLock>, + pub(crate) db_pool: DbPool, + manifest_run_defaults: RwLock>, + manifest_run_settings: RwLock>, pub(crate) server_settings: RwLock>, - effective_web_url: RwLock, - catalog: RwLock>, - pub(crate) env_lookup: EnvLookup, - pub(crate) github_api_base_url: String, - active_config_path: PathBuf, - http_client: Option, - sandbox_inventory: SandboxInventory, - shutdown: CancellationToken, - shutting_down: AtomicBool, + effective_web_url: RwLock, + catalog: RwLock>, + pub(crate) env_lookup: EnvLookup, + pub(crate) github_host: fabro_types::GitHubHost, + active_config_path: PathBuf, + http_client: Option, + sandbox_inventory: SandboxInventory, + shutdown: CancellationToken, + shutting_down: AtomicBool, /// Test switch: execute runs in this process instead of a worker. - execute_in_process: bool, - slack_service: Option>, - slack_started: AtomicBool, - github_webhook_secret: Option, + execute_in_process: bool, + slack_service: Option>, + slack_started: AtomicBool, + github_webhook_secret: Option, } pub(crate) struct AppStores { @@ -1277,7 +1277,7 @@ impl AppState { .map_err(|source| RunMaterializeError::LoadCredentials { source })?; ProductionAutomationRunMaterializer::new( credentials, - self.github_api_base_url.clone(), + self.github_host.api_url().to_string(), self.http_client.clone(), Arc::clone(&self.automation_repo_cache), fabro_workflow_version::WorkflowVersionStore::new(self.store_ref().blobs()), @@ -1352,7 +1352,7 @@ pub(crate) struct AppStateConfig { pub(crate) preloaded_vault: Vault, pub(crate) server_secrets: ServerSecrets, pub(crate) env_lookup: EnvLookup, - pub(crate) github_api_base_url: Option, + pub(crate) github_host: Option, pub(crate) active_config_path: PathBuf, pub(crate) http_client: Option, pub(crate) sandbox_inventory: Option, @@ -2519,7 +2519,7 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result anyhow::Result host, + None => fabro_github::github_host(¤t_server_settings.server.integrations.github)?, + }; let storage_root = PathBuf::from(¤t_server_settings.server.storage.root); let automation_repo_cache = Arc::new(GitRepoCache::new( Storage::new(&storage_root) @@ -2721,7 +2724,7 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result( })?; Ok(fabro_github::GitHubContext::with_http_client( creds, - state.github_api_base_url.as_str(), + state.github_host.api_url(), http_client, )) } diff --git a/lib/apps/fabro-server/src/server/handler/system.rs b/lib/apps/fabro-server/src/server/handler/system.rs index 34420eaa6..917fff590 100644 --- a/lib/apps/fabro-server/src/server/handler/system.rs +++ b/lib/apps/fabro-server/src/server/handler/system.rs @@ -455,7 +455,7 @@ async fn get_github_repo( } let settings = state.server_settings(); let github_settings = &settings.server.integrations.github; - let base_url = fabro_github::github_api_base_url(); + let base_url = state.github_host.api_url(); let (token, client) = match github_settings.strategy { GithubIntegrationStrategy::App => { if github_settings.app_id.is_none() { diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 02b3d006c..f8a8e785c 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -2114,7 +2114,7 @@ fn slack_app_state_with_settings_and_secret_sources( preloaded_vault: vault, server_secrets: load_test_server_secrets(server_env_path, server_secret_env), env_lookup: default_env_lookup(), - github_api_base_url: None, + github_host: None, active_config_path: tempfile::tempdir().unwrap().path().join("settings.toml"), http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")), sandbox_inventory: None, @@ -5102,7 +5102,9 @@ fn create_github_token_app_state_with_env_lookup_and_llm_catalog_settings( preloaded_vault, server_secrets: load_test_server_secrets(server_env_path, HashMap::new()), env_lookup: Arc::new(env_lookup), - github_api_base_url, + github_host: github_api_base_url.map(|url| { + fabro_types::GitHubHost::new(&url, Some(&url)).expect("mock GitHub URL should be valid") + }), active_config_path, http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")), sandbox_inventory: None, @@ -10808,7 +10810,7 @@ methods = ["dev-token"] preloaded_vault, server_secrets: ServerSecrets::load(server_env_path, HashMap::new()).unwrap(), env_lookup: default_env_lookup(), - github_api_base_url: None, + github_host: None, active_config_path: tempfile::tempdir().unwrap().path().join("settings.toml"), http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")), sandbox_inventory: None, @@ -10866,7 +10868,7 @@ fn slack_service_respects_disabled_server_config_even_with_vault_tokens() { HashMap::new(), ), env_lookup: default_env_lookup(), - github_api_base_url: None, + github_host: None, active_config_path: tempfile::tempdir().unwrap().path().join("settings.toml"), http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")), sandbox_inventory: None, diff --git a/lib/apps/fabro-server/src/spawn_env.rs b/lib/apps/fabro-server/src/spawn_env.rs index 333a48723..02b433093 100644 --- a/lib/apps/fabro-server/src/spawn_env.rs +++ b/lib/apps/fabro-server/src/spawn_env.rs @@ -68,6 +68,11 @@ const WORKER_ENV_ALLOWLIST: &[&str] = &[ // the vault and supplies it explicitly to the in-process provider. EnvVars::DAYTONA_API_URL, EnvVars::DAYTONA_ORGANIZATION_ID, + // The GitHub deployment's non-secret selection. The worker's GitHub + // client reads the web origin and API base overrides; the settings + // reach it through FABRO_HOME. + EnvVars::GITHUB_WEB_URL, + EnvVars::GITHUB_BASE_URL, // A test's checkpoint gates: the worker's hooks hold at a named point // until the test releases them, so a crash can be placed there. EnvVars::FABRO_TEST_CHECKPOINT_GATES, diff --git a/lib/apps/fabro-server/src/test_support.rs b/lib/apps/fabro-server/src/test_support.rs index bd61d3009..93a14f6a2 100644 --- a/lib/apps/fabro-server/src/test_support.rs +++ b/lib/apps/fabro-server/src/test_support.rs @@ -302,7 +302,7 @@ impl TestAppStateBuilder { preloaded_vault, server_secrets: load_test_server_secrets(server_env_path, self.server_secret_env), env_lookup: self.env_lookup, - github_api_base_url: None, + github_host: None, active_config_path, http_client: Some( fabro_http::test_http_client().expect("test HTTP client should build"), diff --git a/lib/components/fabro-github/src/lib.rs b/lib/components/fabro-github/src/lib.rs index 9505b4e7c..c4a9f3fdb 100644 --- a/lib/components/fabro-github/src/lib.rs +++ b/lib/components/fabro-github/src/lib.rs @@ -4,8 +4,9 @@ use base64::engine::general_purpose::STANDARD; use chrono::{DateTime, Utc}; use fabro_redact::DisplaySafeUrl; use fabro_static::EnvVars; -use fabro_types::PullRequestGithubDetail; use fabro_types::settings::run::MergeStrategy; +use fabro_types::settings::server::GithubIntegrationSettings; +use fabro_types::{GitHubHost, PullRequestGithubDetail}; use serde::Deserialize; use tokio::process::Command; @@ -23,8 +24,6 @@ pub(crate) mod tests_mock; pub use access::GitHubRepositoryAccess; pub use identity::GitHubAccountIdentity; -pub const GITHUB_API_BASE_URL: &str = "https://api.github.com"; - /// 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"; @@ -52,14 +51,44 @@ pub fn apply_probe_git_env(command: &mut Command, token: &str) { .env("GIT_CONFIG_VALUE_0", GITHUB_CREDENTIAL_HELPER); } -/// Returns the GitHub API base URL, allowing override via `GITHUB_BASE_URL` env -/// var. +/// REST API base from `GITHUB_BASE_URL` or github.com, for call sites that +/// have no settings to resolve a [`GitHubHost`] from. #[expect( clippy::disallowed_methods, reason = "GitHub API client exposes a documented process-env base URL override." )] pub fn github_api_base_url() -> String { - std::env::var(EnvVars::GITHUB_BASE_URL).unwrap_or_else(|_| GITHUB_API_BASE_URL.to_string()) + std::env::var(EnvVars::GITHUB_BASE_URL) + .unwrap_or_else(|_| GitHubHost::default().api_url().to_string()) +} + +/// Resolves the GitHub deployment from settings, letting `GITHUB_WEB_URL` +/// override the web origin and `GITHUB_BASE_URL` override the REST API base. +#[expect( + clippy::disallowed_methods, + reason = "GitHub API client exposes documented process-env web origin and API base overrides." +)] +pub fn github_host(settings: &GithubIntegrationSettings) -> anyhow::Result { + host_with_overrides( + &settings.host, + std::env::var(EnvVars::GITHUB_WEB_URL).ok().as_deref(), + std::env::var(EnvVars::GITHUB_BASE_URL).ok().as_deref(), + ) +} + +/// A `web_url` override re-derives the API URL, since the settings' API URL +/// belonged to the old origin; an `api_url` override always wins. +fn host_with_overrides( + configured: &GitHubHost, + web_url: Option<&str>, + api_url: Option<&str>, +) -> anyhow::Result { + let host = match (web_url, api_url) { + (None, None) => return Ok(configured.clone()), + (None, Some(api)) => GitHubHost::new(configured.web_url(), Some(api)), + (Some(web), api) => GitHubHost::new(web, api), + }; + host.context("invalid GITHUB_WEB_URL or GITHUB_BASE_URL override") } /// Bundle of GitHub credentials and the API base URL, threaded through every @@ -1642,6 +1671,24 @@ mod tests { use super::*; + #[test] + fn host_overrides_apply_over_settings() { + let configured = GitHubHost::new("https://ghe.example.com", None).unwrap(); + assert_eq!( + host_with_overrides(&configured, None, None).unwrap(), + configured + ); + + let api_only = host_with_overrides(&configured, None, Some("http://127.0.0.1:9")).unwrap(); + assert_eq!(api_only.web_url(), "https://ghe.example.com"); + assert_eq!(api_only.api_url(), "http://127.0.0.1:9"); + + let web = host_with_overrides(&configured, Some("https://octocorp.ghe.com"), None).unwrap(); + assert_eq!(web.api_url(), "https://api.octocorp.ghe.com"); + + assert!(host_with_overrides(&configured, Some("http://ghe.example.com"), None).is_err()); + } + #[test] fn decode_pem_env_accepts_raw_pem() { let pem = "-----BEGIN TEST KEY-----\nabc\n-----END TEST KEY-----"; diff --git a/lib/foundation/fabro-config/src/layers/server.rs b/lib/foundation/fabro-config/src/layers/server.rs index e15892707..8cba449db 100644 --- a/lib/foundation/fabro-config/src/layers/server.rs +++ b/lib/foundation/fabro-config/src/layers/server.rs @@ -233,6 +233,10 @@ pub struct GithubIntegrationLayer { pub slug: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub webhooks: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub web_url: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub api_url: Option, } /// `[server.integrations.slack]` — Slack workspace credentials and defaults. diff --git a/lib/foundation/fabro-config/src/resolve/server.rs b/lib/foundation/fabro-config/src/resolve/server.rs index 8ce62c516..66d75ad9b 100644 --- a/lib/foundation/fabro-config/src/resolve/server.rs +++ b/lib/foundation/fabro-config/src/resolve/server.rs @@ -1,7 +1,6 @@ use std::collections::BTreeMap; use std::path::Path; -use fabro_types::SandboxProviderKind; use fabro_types::settings::server::{ GithubIntegrationSettings, GithubIntegrationStrategy, IntegrationWebhooksSettings, ObjectStoreProvider, ObjectStoreSettings, SandboxPluginSettings, ServerApiSettings, @@ -11,6 +10,7 @@ use fabro_types::settings::server::{ ServerSchedulerSettings, ServerStorageSettings, ServerWebSettings, SlackIntegrationSettings, WebhookStrategy, }; +use fabro_types::{GitHubHost, SandboxProviderKind}; use fabro_util::Home; use super::{ @@ -19,9 +19,10 @@ use super::{ }; use crate::user::default_storage_dir; use crate::{ - IntegrationWebhooksLayer, ObjectStoreLocalLayer, ObjectStoreS3Layer, ServerApiLayer, - ServerArtifactsLayer, ServerAuthLayer, ServerIntegrationsLayer, ServerLayer, ServerListenLayer, - ServerSandboxLayer, ServerSandboxProviderLayer, ServerStorageLayer, ServerWebLayer, + GithubIntegrationLayer, IntegrationWebhooksLayer, ObjectStoreLocalLayer, ObjectStoreS3Layer, + ServerApiLayer, ServerArtifactsLayer, ServerAuthLayer, ServerIntegrationsLayer, ServerLayer, + ServerListenLayer, ServerSandboxLayer, ServerSandboxProviderLayer, ServerStorageLayer, + ServerWebLayer, }; pub fn resolve_server(layer: &ServerLayer, errors: &mut Vec) -> ServerNamespace { @@ -29,7 +30,7 @@ pub fn resolve_server(layer: &ServerLayer, errors: &mut Vec) -> Se let listen = resolve_listen(layer.listen.as_ref(), errors); let web = resolve_web(layer.web.as_ref()); let auth = resolve_auth(layer.auth.as_ref(), errors); - let integrations = resolve_integrations(layer.integrations.as_ref()); + let integrations = resolve_integrations(layer.integrations.as_ref(), errors); validate_github_webhook_strategy(&integrations, layer.api.as_ref(), errors); let api_url = layer.api.as_ref().and_then(|api| api.url.clone()); @@ -330,7 +331,10 @@ fn resolve_object_store( } } -fn resolve_integrations(layer: Option<&ServerIntegrationsLayer>) -> ServerIntegrationsSettings { +fn resolve_integrations( + layer: Option<&ServerIntegrationsLayer>, + errors: &mut Vec, +) -> ServerIntegrationsSettings { ServerIntegrationsSettings { github: layer .and_then(|integrations| integrations.github.as_ref()) @@ -344,13 +348,15 @@ fn resolve_integrations(layer: Option<&ServerIntegrationsLayer>) -> ServerIntegr github.client_id.as_deref(), ); warn_if_demoted_template("server.integrations.github.slug", github.slug.as_deref()); + let host = resolve_github_host(github, errors); GithubIntegrationSettings { - enabled: github.enabled.unwrap_or(true), - strategy: github.strategy.unwrap_or_default(), - app_id: github.app_id.clone(), + host, + enabled: github.enabled.unwrap_or(true), + strategy: github.strategy.unwrap_or_default(), + app_id: github.app_id.clone(), client_id: github.client_id.clone(), - slug: github.slug.clone(), - webhooks: github.webhooks.as_ref().map(resolve_github_webhooks), + slug: github.slug.clone(), + webhooks: github.webhooks.as_ref().map(resolve_github_webhooks), } }) .unwrap_or_default(), @@ -375,6 +381,21 @@ fn resolve_integrations(layer: Option<&ServerIntegrationsLayer>) -> ServerIntegr } } +fn resolve_github_host( + layer: &GithubIntegrationLayer, + errors: &mut Vec, +) -> GitHubHost { + let default = GitHubHost::default(); + let web_url = layer.web_url.as_deref().unwrap_or(default.web_url()); + GitHubHost::new(web_url, layer.api_url.as_deref()).unwrap_or_else(|err| { + errors.push(ResolveError::Invalid { + path: format!("server.integrations.github.{}", err.field()), + reason: err.to_string(), + }); + default + }) +} + fn resolve_github_webhooks(layer: &IntegrationWebhooksLayer) -> IntegrationWebhooksSettings { IntegrationWebhooksSettings { strategy: layer.strategy, diff --git a/lib/foundation/fabro-config/src/tests/resolve_server.rs b/lib/foundation/fabro-config/src/tests/resolve_server.rs index 33995be1f..6518afe94 100644 --- a/lib/foundation/fabro-config/src/tests/resolve_server.rs +++ b/lib/foundation/fabro-config/src/tests/resolve_server.rs @@ -3,11 +3,11 @@ reason = "sync test fixture setup and raw template source assertions; not on a Tokio path" )] -use fabro_types::SandboxProviderKind; use fabro_types::settings::server::{ GithubIntegrationStrategy, LogDestination, ObjectStoreSettings, ServerAuthMethod, ServerListenSettings, ServerNamespace, }; +use fabro_types::{GitHubHost, SandboxProviderKind}; use fabro_util::Home; use temp_env::with_var; @@ -108,6 +108,8 @@ fn resolved_server_integrations_disable_slack_when_config_is_absent() { "client_id": null, "slug": null, "webhooks": null, + "web_url": "https://github.com", + "api_url": "https://api.github.com", }, "slack": { "enabled": false, @@ -527,6 +529,72 @@ enabled = true ); } +#[test] +fn github_host_defaults_to_github_com() { + let settings = resolve_server(&parse("_version = 1\n")); + assert_eq!(settings.integrations.github.host, GitHubHost::default()); + + let settings = resolve_server(&parse( + "_version = 1\n\n[server.integrations.github]\nenabled = true\n", + )); + assert_eq!(settings.integrations.github.host, GitHubHost::default()); +} + +#[test] +fn github_host_derives_api_url_from_web_url() { + let settings = resolve_server(&parse( + r#" +_version = 1 + +[server.integrations.github] +web_url = "https://ghe.example.com" +"#, + )); + let host = &settings.integrations.github.host; + assert_eq!(host.web_url(), "https://ghe.example.com"); + assert_eq!(host.api_url(), "https://ghe.example.com/api/v3"); +} + +#[test] +fn github_host_honours_explicit_api_url() { + let settings = resolve_server(&parse( + r#" +_version = 1 + +[server.integrations.github] +web_url = "https://ghe.example.com" +api_url = "https://api.ghe.example.com" +"#, + )); + assert_eq!( + settings.integrations.github.host.api_url(), + "https://api.ghe.example.com" + ); +} + +#[test] +fn github_host_rejects_invalid_web_url() { + let file = parse( + r#" +_version = 1 + +[server.integrations.github] +web_url = "http://ghe.example.com" +"#, + ); + let error = + ServerSettingsBuilder::from_layer(&file).expect_err("plain http should be rejected"); + let errors = resolve_errors(error); + assert!( + errors.iter().any(|err| { + err.to_string() + .contains("server.integrations.github.web_url") + && err.to_string().contains("must use https") + }), + "unexpected errors: {errors:?}" + ); +} + #[test] fn parsing_rejects_removed_server_ip_allowlist() { let err = r#" diff --git a/lib/foundation/fabro-static/src/env_vars.rs b/lib/foundation/fabro-static/src/env_vars.rs index 53a45f158..6536f2e32 100644 --- a/lib/foundation/fabro-static/src/env_vars.rs +++ b/lib/foundation/fabro-static/src/env_vars.rs @@ -124,6 +124,7 @@ impl EnvVars { pub const GITHUB_APP_WEBHOOK_SECRET: &'static str = "GITHUB_APP_WEBHOOK_SECRET"; pub const GITHUB_BASE_URL: &'static str = "GITHUB_BASE_URL"; pub const GITHUB_TOKEN: &'static str = "GITHUB_TOKEN"; + pub const GITHUB_WEB_URL: &'static str = "GITHUB_WEB_URL"; pub const OAUTH_CALLBACK_PATH: &'static str = "OAUTH_CALLBACK_PATH"; pub const OAUTH_CLIENT_ID: &'static str = "OAUTH_CLIENT_ID"; pub const OAUTH_ISSUER: &'static str = "OAUTH_ISSUER"; @@ -285,6 +286,7 @@ mod tests { EnvVars::GITHUB_APP_WEBHOOK_SECRET, EnvVars::GITHUB_BASE_URL, EnvVars::GITHUB_TOKEN, + EnvVars::GITHUB_WEB_URL, EnvVars::OAUTH_CALLBACK_PATH, EnvVars::OAUTH_CLIENT_ID, EnvVars::OAUTH_ISSUER, diff --git a/lib/foundation/fabro-static/src/secret_registry.rs b/lib/foundation/fabro-static/src/secret_registry.rs index 4fb715b6f..c6759e2b0 100644 --- a/lib/foundation/fabro-static/src/secret_registry.rs +++ b/lib/foundation/fabro-static/src/secret_registry.rs @@ -123,6 +123,7 @@ mod tests { for name in [ EnvVars::GH_TOKEN, EnvVars::GITHUB_BASE_URL, + EnvVars::GITHUB_WEB_URL, EnvVars::SLACK_BASE_URL, EnvVars::DAYTONA_API_URL, EnvVars::DAYTONA_ORGANIZATION_ID, diff --git a/lib/foundation/fabro-types/src/github_host.rs b/lib/foundation/fabro-types/src/github_host.rs new file mode 100644 index 000000000..bc928e90c --- /dev/null +++ b/lib/foundation/fabro-types/src/github_host.rs @@ -0,0 +1,368 @@ +use serde::{Deserialize, Serialize}; + +const DEFAULT_WEB_URL: &str = "https://github.com"; +const DEFAULT_API_URL: &str = "https://api.github.com"; + +/// Rejected GitHub host setting, naming the offending field. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +#[error("{field} {problem}")] +pub struct GitHubHostError { + field: &'static str, + problem: String, +} + +impl GitHubHostError { + /// Name of the rejected setting: `web_url` or `api_url`. + #[must_use] + pub fn field(&self) -> &'static str { + self.field + } + + fn new(field: &'static str, problem: impl Into) -> Self { + Self { + field, + problem: problem.into(), + } + } +} + +/// The GitHub deployment Fabro talks to: github.com (the default), a GitHub +/// Enterprise Cloud tenant with data residency (`*.ghe.com`) or a GitHub +/// Enterprise Server instance. The web origin is the one source of truth; +/// every other URL is derived from it unless the API URL is given explicitly. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(try_from = "GitHubHostWire")] +pub struct GitHubHost { + web_url: String, + api_url: String, +} + +#[derive(Deserialize)] +struct GitHubHostWire { + #[serde(default)] + web_url: Option, + #[serde(default)] + api_url: Option, +} + +impl TryFrom for GitHubHost { + type Error = GitHubHostError; + + fn try_from(wire: GitHubHostWire) -> Result { + Self::new( + wire.web_url.as_deref().unwrap_or(DEFAULT_WEB_URL), + wire.api_url.as_deref(), + ) + } +} + +impl Default for GitHubHost { + fn default() -> Self { + Self { + web_url: DEFAULT_WEB_URL.to_string(), + api_url: DEFAULT_API_URL.to_string(), + } + } +} + +#[expect( + clippy::disallowed_types, + reason = "GitHub host settings are public service origins; validation errors never echo credentials." +)] +fn parse_origin(field: &'static str, raw: &str) -> Result { + let raw = raw.trim().trim_end_matches('/'); + let parsed = url::Url::parse(raw).map_err(|_| { + GitHubHostError::new( + field, + format!("must be an https URL such as https://github.com (got `{raw}`)"), + ) + })?; + let host = parsed + .host_str() + .ok_or_else(|| GitHubHostError::new(field, "must include a host"))?; + let loopback = matches!(host, "localhost" | "127.0.0.1" | "[::1]"); + match parsed.scheme() { + "https" => {} + "http" if loopback => {} + _ => { + return Err(GitHubHostError::new( + field, + format!("must use https (got `{raw}`); http is only allowed for localhost"), + )); + } + } + if !parsed.username().is_empty() || parsed.password().is_some() { + return Err(GitHubHostError::new(field, "must not contain credentials")); + } + if parsed.query().is_some() || parsed.fragment().is_some() { + return Err(GitHubHostError::new( + field, + "must not contain a query or fragment", + )); + } + Ok(parsed) +} + +impl GitHubHost { + /// Builds a host from the web origin and an optional explicit API URL. + /// Without `api_url`, the API URL is derived from the web origin. + pub fn new(web_url: &str, api_url: Option<&str>) -> Result { + let web = parse_origin("web_url", web_url)?; + if !matches!(web.path(), "" | "/") { + return Err(GitHubHostError::new( + "web_url", + "must not contain a path; GitHub is never served under a path prefix", + )); + } + let web_origin = web.origin().ascii_serialization(); + let api_url = match api_url { + Some(api) => { + parse_origin("api_url", api)?; + api.trim().trim_end_matches('/').to_string() + } + None => derive_api_url(&web_origin, web.host_str().unwrap_or_default()), + }; + Ok(Self { + web_url: web_origin, + api_url, + }) + } + + /// Web origin without a trailing slash, e.g. `https://ghe.example.com`. + #[must_use] + pub fn web_url(&self) -> &str { + &self.web_url + } + + /// REST API base without a trailing slash, e.g. + /// `https://ghe.example.com/api/v3`. + #[must_use] + pub fn api_url(&self) -> &str { + &self.api_url + } + + /// Authority of the web origin, e.g. `ghe.example.com` or + /// `ghe.example.com:8443`. + #[must_use] + pub fn host(&self) -> &str { + self.web_url + .split_once("://") + .map_or(self.web_url.as_str(), |(_, authority)| authority) + } + + #[must_use] + pub fn is_github_com(&self) -> bool { + self.host() == "github.com" + } + + /// GraphQL endpoint derived from the REST API URL. + #[must_use] + pub fn graphql_url(&self) -> String { + match self.api_url.strip_suffix("/api/v3") { + Some(base) => format!("{base}/api/graphql"), + None => format!("{}/graphql", self.api_url), + } + } + + #[must_use] + pub fn repository_url(&self, owner: &str, repo: &str) -> String { + format!("{}/{owner}/{repo}", self.web_url) + } + + #[must_use] + pub fn pull_request_url(&self, owner: &str, repo: &str, number: u64) -> String { + format!("{}/pull/{number}", self.repository_url(owner, repo)) + } + + /// Domain of GitHub's private noreply commit emails. + #[must_use] + pub fn noreply_email_domain(&self) -> String { + format!("users.noreply.{}", self.host()) + } + + /// Git config key that routes this host's HTTPS credentials to a helper. + #[must_use] + pub fn credential_helper_key(&self) -> String { + format!("credential.{}.helper", self.web_url) + } + + /// Whether `origin` is a git remote URL on this host (HTTPS, HTTP, scp-like + /// SSH or `ssh://`). + #[must_use] + pub fn is_origin(&self, origin: &str) -> bool { + let host = self.host(); + origin.starts_with(&format!("{}/", self.web_url)) + || origin.starts_with(&format!("http://{host}/")) + || origin.starts_with(&format!("git@{host}:")) + || origin.starts_with(&format!("ssh://git@{host}/")) + } +} + +fn derive_api_url(web_origin: &str, hostname: &str) -> String { + let under = |domain: &str| hostname == domain || hostname.ends_with(&format!(".{domain}")); + if under("github.com") { + DEFAULT_API_URL.to_string() + } else if under("ghe.com") { + web_origin.replacen("://", "://api.", 1) + } else { + format!("{web_origin}/api/v3") + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn host(web: &str) -> GitHubHost { + GitHubHost::new(web, None).unwrap() + } + + #[test] + fn default_matches_github_com_constants() { + let default = GitHubHost::default(); + assert_eq!(default.web_url(), "https://github.com"); + assert_eq!(default.api_url(), "https://api.github.com"); + assert_eq!(default, host("https://github.com")); + assert!(default.is_github_com()); + assert_eq!(default.graphql_url(), "https://api.github.com/graphql"); + assert_eq!(default.repository_url("o", "r"), "https://github.com/o/r"); + assert_eq!( + default.pull_request_url("o", "r", 7), + "https://github.com/o/r/pull/7" + ); + assert_eq!(default.noreply_email_domain(), "users.noreply.github.com"); + assert_eq!( + default.credential_helper_key(), + "credential.https://github.com.helper" + ); + assert!(default.is_origin("git@github.com:o/r.git")); + assert!(default.is_origin("https://github.com/o/r")); + assert!(default.is_origin("http://github.com/o/r")); + assert!(default.is_origin("ssh://git@github.com/o/r")); + assert!(!default.is_origin("https://gitlab.com/o/r")); + } + + #[test] + fn data_residency_host_derives_urls() { + let ghec = host("https://octocorp.ghe.com"); + assert_eq!(ghec.api_url(), "https://api.octocorp.ghe.com"); + assert_eq!(ghec.graphql_url(), "https://api.octocorp.ghe.com/graphql"); + assert_eq!(ghec.host(), "octocorp.ghe.com"); + assert!(!ghec.is_github_com()); + assert_eq!( + ghec.repository_url("o", "r"), + "https://octocorp.ghe.com/o/r" + ); + assert_eq!( + ghec.pull_request_url("o", "r", 7), + "https://octocorp.ghe.com/o/r/pull/7" + ); + assert_eq!( + ghec.noreply_email_domain(), + "users.noreply.octocorp.ghe.com" + ); + assert_eq!( + ghec.credential_helper_key(), + "credential.https://octocorp.ghe.com.helper" + ); + assert!(ghec.is_origin("git@octocorp.ghe.com:o/r.git")); + assert!(ghec.is_origin("ssh://git@octocorp.ghe.com/o/r")); + assert!(!ghec.is_origin("https://github.com/o/r")); + } + + #[test] + fn server_host_derives_urls() { + let ghes = host("https://ghe.example.com/"); + assert_eq!(ghes.web_url(), "https://ghe.example.com"); + assert_eq!(ghes.api_url(), "https://ghe.example.com/api/v3"); + assert_eq!(ghes.graphql_url(), "https://ghe.example.com/api/graphql"); + assert_eq!(ghes.host(), "ghe.example.com"); + assert_eq!(ghes.repository_url("o", "r"), "https://ghe.example.com/o/r"); + assert_eq!( + ghes.pull_request_url("o", "r", 7), + "https://ghe.example.com/o/r/pull/7" + ); + assert_eq!(ghes.noreply_email_domain(), "users.noreply.ghe.example.com"); + assert_eq!( + ghes.credential_helper_key(), + "credential.https://ghe.example.com.helper" + ); + assert!(ghes.is_origin("git@ghe.example.com:o/r.git")); + assert!(ghes.is_origin("https://ghe.example.com/o/r")); + } + + #[test] + fn host_keeps_port_and_lowercases() { + let ghes = host("https://GHE.Example.com:8443"); + assert_eq!(ghes.web_url(), "https://ghe.example.com:8443"); + assert_eq!(ghes.host(), "ghe.example.com:8443"); + } + + #[test] + fn explicit_api_url_wins() { + let ghes = GitHubHost::new( + "https://ghe.example.com", + Some("https://api.ghe.example.com/"), + ) + .unwrap(); + assert_eq!(ghes.api_url(), "https://api.ghe.example.com"); + assert_eq!(ghes.graphql_url(), "https://api.ghe.example.com/graphql"); + } + + #[test] + fn loopback_http_is_accepted() { + let local = host("http://127.0.0.1:1234"); + assert_eq!(local.web_url(), "http://127.0.0.1:1234"); + assert_eq!(local.api_url(), "http://127.0.0.1:1234/api/v3"); + let mock = GitHubHost::new("http://127.0.0.1:1234", Some("http://127.0.0.1:1234")).unwrap(); + assert_eq!(mock.graphql_url(), "http://127.0.0.1:1234/graphql"); + } + + #[test] + fn invalid_values_name_the_problem() { + let plain_http = GitHubHost::new("http://ghe.example.com", None).unwrap_err(); + assert!( + plain_http.to_string().starts_with("web_url must use https"), + "{plain_http}" + ); + let bare = GitHubHost::new("ghe.example.com", None).unwrap_err(); + assert!( + bare.to_string().starts_with("web_url must be an https URL"), + "{bare}" + ); + let path = GitHubHost::new("https://ghe.example.com/github", None).unwrap_err(); + assert!( + path.to_string().contains("must not contain a path"), + "{path}" + ); + let api = GitHubHost::new( + "https://ghe.example.com", + Some("http://ghe.example.com/api/v3"), + ) + .unwrap_err(); + assert!( + api.to_string().starts_with("api_url must use https"), + "{api}" + ); + } + + #[test] + fn serde_round_trips_and_validates() { + let ghes = host("https://ghe.example.com"); + let json = serde_json::to_value(&ghes).unwrap(); + assert_eq!( + json, + serde_json::json!({ + "web_url": "https://ghe.example.com", + "api_url": "https://ghe.example.com/api/v3", + }) + ); + assert_eq!(serde_json::from_value::(json).unwrap(), ghes); + assert!( + serde_json::from_value::( + serde_json::json!({"web_url": "http://x.example.com", "api_url": "https://x.example.com"}) + ) + .is_err() + ); + } +} diff --git a/lib/foundation/fabro-types/src/lib.rs b/lib/foundation/fabro-types/src/lib.rs index e08f61bfa..cc7684d6e 100644 --- a/lib/foundation/fabro-types/src/lib.rs +++ b/lib/foundation/fabro-types/src/lib.rs @@ -15,6 +15,7 @@ pub mod diff; pub mod engine; pub mod failure_signature; pub mod git_identity; +pub mod github_host; mod id; mod input_scalar; pub mod interview; @@ -86,6 +87,7 @@ pub use diff::{DiffStats, DiffSummary, RunDiff}; pub use engine::{PetriAdmission, PetriGraphRef}; pub use failure_signature::FailureSignature; pub use git_identity::{GitIdentity, GitIdentitySource}; +pub use github_host::{GitHubHost, GitHubHostError}; pub use input_scalar::{ JsonScalarToTomlError, TomlScalarToJsonError, json_scalar_to_toml_value, toml_scalar_to_json_value, diff --git a/lib/foundation/fabro-types/src/settings/server.rs b/lib/foundation/fabro-types/src/settings/server.rs index d66a83154..d121d208a 100644 --- a/lib/foundation/fabro-types/src/settings/server.rs +++ b/lib/foundation/fabro-types/src/settings/server.rs @@ -11,7 +11,7 @@ use std::net::SocketAddr; use serde::de::Error as _; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use crate::SandboxProviderKind; +use crate::{GitHubHost, SandboxProviderKind}; /// A structurally resolved `[server]` view for consumers. /// @@ -297,6 +297,9 @@ pub struct GithubIntegrationSettings { pub client_id: Option, pub slug: Option, pub webhooks: Option, + /// The GitHub deployment (`web_url`/`api_url` on the wire). + #[serde(flatten)] + pub host: GitHubHost, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] diff --git a/lib/packages/fabro-api-client/src/models/github-integration-settings.ts b/lib/packages/fabro-api-client/src/models/github-integration-settings.ts index fa6bdfcac..7263955b8 100644 --- a/lib/packages/fabro-api-client/src/models/github-integration-settings.ts +++ b/lib/packages/fabro-api-client/src/models/github-integration-settings.ts @@ -27,4 +27,12 @@ export interface GithubIntegrationSettings { 'client_id': string | null; 'slug': string | null; 'webhooks': IntegrationWebhooksSettings | null; + /** + * Web origin of the GitHub deployment, without a trailing slash. + */ + 'web_url': string; + /** + * REST API base URL of the GitHub deployment, without a trailing slash. + */ + 'api_url': string; } From 21be241a763527554e2de060ae2feeeb3ba88e42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Balatoni?= Date: Sat, 24 Oct 2026 23:44:32 +0200 Subject: [PATCH 02/13] feat(github): derive GitHub URLs from the configured host --- lib/apps/fabro-cli/src/commands/install.rs | 6 +- .../src/commands/run/petri_worker.rs | 13 +- .../fabro-cli/src/commands/run/publish.rs | 18 +- .../src/automation_materializer.rs | 14 +- lib/apps/fabro-server/src/git_checkout.rs | 8 +- lib/apps/fabro-server/src/github_webhooks.rs | 6 +- lib/apps/fabro-server/src/run_manifest.rs | 46 ++- lib/apps/fabro-server/src/serve.rs | 13 +- lib/apps/fabro-server/src/server.rs | 2 +- .../src/server/handler/pull_requests.rs | 2 +- .../fabro-server/src/server/handler/system.rs | 10 +- lib/components/fabro-github/src/identity.rs | 33 +- lib/components/fabro-github/src/lib.rs | 327 ++++++++++++------ .../fabro-github/src/token_source.rs | 34 +- .../fabro-github/tests/integration.rs | 30 +- .../fabro-github/tests/live_access.rs | 15 +- .../fabro-petri/src/stage_credentials.rs | 23 +- lib/components/fabro-tracker/src/github.rs | 16 +- .../fabro-workflow/src/pull_request.rs | 16 +- 19 files changed, 424 insertions(+), 208 deletions(-) diff --git a/lib/apps/fabro-cli/src/commands/install.rs b/lib/apps/fabro-cli/src/commands/install.rs index 1c61c032c..f5edbcdc0 100644 --- a/lib/apps/fabro-cli/src/commands/install.rs +++ b/lib/apps/fabro-cli/src/commands/install.rs @@ -518,7 +518,7 @@ impl InstallInputSource for InteractiveInstallInputSource { match strategy { 0 => { let token = if gh_available { - fabro_github::gh_auth_token() + fabro_github::gh_auth_token(&fabro_github::GitHubHost::default()) .await .context("Run `gh auth login` and rerun `fabro install`.")? } else { @@ -696,7 +696,7 @@ impl InstallInputSource for NonInteractiveInstallInputSource { ) -> Result { match self.args.github_strategy { Some(InstallGitHubStrategyArg::Token) => { - let token = fabro_github::gh_auth_token() + let token = fabro_github::gh_auth_token(&fabro_github::GitHubHost::default()) .await .context("Run `gh auth login` and rerun `fabro install`.")?; fabro_github::validate_static_github_token(&token)?; @@ -782,7 +782,7 @@ async fn choose_install_github_selection( match github_args.strategy { Some(InstallGitHubStrategyArg::Token) => { - let token = fabro_github::gh_auth_token() + let token = fabro_github::gh_auth_token(&fabro_github::GitHubHost::default()) .await .context("Run `gh auth login` and rerun `fabro install github`.")?; fabro_github::validate_static_github_token(&token)?; diff --git a/lib/apps/fabro-cli/src/commands/run/petri_worker.rs b/lib/apps/fabro-cli/src/commands/run/petri_worker.rs index 267c18070..39652fc87 100644 --- a/lib/apps/fabro-cli/src/commands/run/petri_worker.rs +++ b/lib/apps/fabro-cli/src/commands/run/petri_worker.rs @@ -221,9 +221,15 @@ pub(super) async fn execute(worker: PetriWorker<'_>) -> Result<()> { None } }; - let read_tokens = publish::read_token_source(&worker.run_state.spec, github.as_ref()); - runtime.stage_credentials = - StageCredentials::for_run(&worker.run_state.spec, github.as_ref(), read_tokens.clone())?; + let github_host = publish::github_host()?; + let read_tokens = + publish::read_token_source(&worker.run_state.spec, github.as_ref(), &github_host); + runtime.stage_credentials = StageCredentials::for_run( + &worker.run_state.spec, + github.as_ref(), + &github_host, + read_tokens.clone(), + )?; let mut source = RunSource::for_run( worker.run_state.spec.target.as_ref(), &worker.run_state.spec.settings.run, @@ -236,6 +242,7 @@ pub(super) async fn execute(worker: PetriWorker<'_>) -> Result<()> { run_id, &worker.run_state.spec, github, + github_host, Arc::new(VaultCredentialSource::new(Arc::clone(&vault))), Arc::new(catalog), Arc::clone(&records), diff --git a/lib/apps/fabro-cli/src/commands/run/publish.rs b/lib/apps/fabro-cli/src/commands/run/publish.rs index b4850eec4..68976306c 100644 --- a/lib/apps/fabro-cli/src/commands/run/publish.rs +++ b/lib/apps/fabro-cli/src/commands/run/publish.rs @@ -95,12 +95,14 @@ fn repository(spec: &RunSpec) -> Option { fn token_source( spec: &RunSpec, credentials: Option<&GitHubCredentials>, + host: &GitHubHost, permissions: serde_json::Value, ) -> Option> { let repository = repository(spec)?; let credentials = credentials?; match InstallationTokenSource::for_repository( credentials, + host, repository.owner().to_string(), repository.repo().to_string(), permissions, @@ -120,8 +122,14 @@ fn token_source( pub(super) fn read_token_source( spec: &RunSpec, credentials: Option<&GitHubCredentials>, + host: &GitHubHost, ) -> Option> { - token_source(spec, credentials, serde_json::json!({ "contents": "read" })) + token_source( + spec, + credentials, + host, + serde_json::json!({ "contents": "read" }), + ) } /// Fetch credentials over the run's read-only token source. @@ -166,6 +174,7 @@ pub(super) struct GitHubPublisher { /// The run's model, when its settings name one. model: Option, credentials: Option, + host: GitHubHost, /// The run's `contents: write` token source; `None` without credentials. push_tokens: Option>, pull_request: Option, @@ -182,6 +191,7 @@ impl GitHubPublisher { run_id: RunId, spec: &RunSpec, credentials: Option, + host: GitHubHost, llm_source: Arc, catalog: Arc, records: Arc, @@ -202,6 +212,7 @@ impl GitHubPublisher { let push_tokens = token_source( spec, credentials.as_ref(), + &host, serde_json::json!({ "contents": "write" }), ); Some(Self { @@ -211,6 +222,7 @@ impl GitHubPublisher { goal: spec.graph.goal.clone(), model: settings.model.name.clone(), credentials, + host, push_tokens, pull_request: settings .pull_request @@ -319,8 +331,7 @@ impl RunPublisher for GitHubPublisher { .credentials .as_ref() .ok_or_else(|| "GitHub credentials are unavailable".to_string())?; - let host = github_host().map_err(|err| format!("{err:#}"))?; - let context = GitHubContext::new(credentials, host.api_url()); + let context = GitHubContext::new(credentials, &self.host); Box::pin(self.open_pull_request(context, settings, publication)).await } } @@ -405,6 +416,7 @@ mod tests { RunId::new(), spec, None, + GitHubHost::default(), Arc::new(NoCredentials), Arc::new(test_support::test_catalog()), Arc::new(MemoryPlatformRecords::new()), diff --git a/lib/apps/fabro-server/src/automation_materializer.rs b/lib/apps/fabro-server/src/automation_materializer.rs index c3feace37..ac32323f1 100644 --- a/lib/apps/fabro-server/src/automation_materializer.rs +++ b/lib/apps/fabro-server/src/automation_materializer.rs @@ -143,9 +143,9 @@ trait AutomationGitRemoteResolver: Send + Sync { } struct ServerGitHubRemoteResolver { - credentials: Option, - api_base_url: String, - http_client: Option, + credentials: Option, + host: fabro_types::GitHubHost, + http_client: Option, } #[async_trait] @@ -154,7 +154,7 @@ impl AutomationGitRemoteResolver for ServerGitHubRemoteResolver { let auth = git_checkout::resolve_git_read_auth_config( self.credentials.as_ref(), repo, - &self.api_base_url, + &self.host, self.http_client.clone(), ) .await?; @@ -168,7 +168,7 @@ impl AutomationGitRemoteResolver for ServerGitHubRemoteResolver { impl ProductionAutomationRunMaterializer { pub(crate) fn new( github_credentials: Option, - github_api_base_url: String, + github_host: fabro_types::GitHubHost, http_client: Option, repo_cache: Arc, version_store: WorkflowVersionStore, @@ -176,7 +176,7 @@ impl ProductionAutomationRunMaterializer { Self { remote_resolver: Arc::new(ServerGitHubRemoteResolver { credentials: github_credentials, - api_base_url: github_api_base_url, + host: github_host, http_client, }), repo_cache, @@ -772,7 +772,7 @@ mod tests { ) -> ProductionAutomationRunMaterializer { ProductionAutomationRunMaterializer::new( None, - "https://api.github.com".to_string(), + fabro_types::GitHubHost::default(), None, Arc::new(GitRepoCache::new(root.join("cache"))), store, diff --git a/lib/apps/fabro-server/src/git_checkout.rs b/lib/apps/fabro-server/src/git_checkout.rs index d2856be42..7ad32dc8a 100644 --- a/lib/apps/fabro-server/src/git_checkout.rs +++ b/lib/apps/fabro-server/src/git_checkout.rs @@ -313,17 +313,15 @@ impl GitAuthConfig { pub(crate) async fn resolve_git_read_auth_config( credentials: Option<&fabro_github::GitHubCredentials>, repo: &GitHubRepositorySlug, - github_api_base_url: &str, + host: &fabro_types::GitHubHost, http_client: Option, ) -> anyhow::Result> { let Some(credentials) = credentials else { return Ok(None); }; let context = match http_client { - Some(client) => { - fabro_github::GitHubContext::with_http_client(credentials, github_api_base_url, client) - } - None => fabro_github::GitHubContext::new(credentials, github_api_base_url), + Some(client) => fabro_github::GitHubContext::with_http_client(credentials, host, client), + None => fabro_github::GitHubContext::new(credentials, host), }; let credentials = fabro_github::resolve_read_only_clone_credentials(&context, repo.owner(), repo.repo()) diff --git a/lib/apps/fabro-server/src/github_webhooks.rs b/lib/apps/fabro-server/src/github_webhooks.rs index dfc74491c..9da8337bf 100644 --- a/lib/apps/fabro-server/src/github_webhooks.rs +++ b/lib/apps/fabro-server/src/github_webhooks.rs @@ -1,4 +1,5 @@ use fabro_static::EnvVars; +use fabro_types::GitHubHost; use hmac::{Hmac, Mac}; use sha2::Sha256; use tokio::process::Command; @@ -56,6 +57,7 @@ pub struct TailscaleFunnelManager { impl TailscaleFunnelManager { pub async fn start( main_server_port: u16, + host: &GitHubHost, app_id: &str, private_key_pem: &str, ) -> anyhow::Result { @@ -63,7 +65,9 @@ impl TailscaleFunnelManager { info!(port = main_server_port, url = %funnel_url, "Tailscale funnel enabled"); let webhook_url = format!("{funnel_url}{WEBHOOK_ROUTE}"); - match fabro_github::update_app_webhook_config(app_id, private_key_pem, &webhook_url).await { + match fabro_github::update_app_webhook_config(host, app_id, private_key_pem, &webhook_url) + .await + { Ok(()) => { info!(url = %webhook_url, "GitHub App webhook URL updated"); } diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index 59b5c7097..3652fab09 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -513,10 +513,17 @@ async fn build_preflight_report( prepared, &resolved_run, github_app.clone(), + &state.github_host, + ) + .await; + let github_token_ok = run_github_token_check( + &mut checks, + prepared, + &resolved_run, + github_app, + &state.github_host, ) .await; - let github_token_ok = - run_github_token_check(&mut checks, prepared, &resolved_run, github_app).await; let checks_ok = sandbox_ok && repository_access_ok && github_token_ok; @@ -699,6 +706,7 @@ async fn run_repository_access_check( prepared: &PreparedManifest, resolved_run: &RunNamespace, github_app: Option, + host: &fabro_types::GitHubHost, ) -> bool { run_repository_access_check_with( checks, @@ -706,7 +714,7 @@ async fn run_repository_access_check( prepared, resolved_run, github_app, - check_git_remote_ref, + |request, github_app| check_git_remote_ref(request, github_app, host), ) .await } @@ -780,11 +788,12 @@ where async fn check_git_remote_ref( request: GitRemoteRefCheck, github_app: Option, + host: &fabro_types::GitHubHost, ) -> Result<(), String> { let auth_url = match github_app.as_ref() { Some(creds) => Some( fabro_github::resolve_authenticated_url( - &fabro_github::GitHubContext::new(creds, &fabro_github::github_api_base_url()), + &fabro_github::GitHubContext::new(creds, host), &request.origin_url, ) .await @@ -940,13 +949,15 @@ async fn run_github_token_check( prepared: &PreparedManifest, resolved_run: &RunNamespace, github_app: Option, + host: &fabro_types::GitHubHost, ) -> bool { run_github_token_check_with( checks, prepared, resolved_run, github_app, - mint_scoped_github_token, + host, + |access, creds| mint_scoped_github_token(access, creds, host), probe_github_repository, ) .await @@ -957,6 +968,7 @@ async fn run_github_token_check_with( prepared: &PreparedManifest, resolved_run: &RunNamespace, github_app: Option, + host: &fabro_types::GitHubHost, mint_scoped_token: M, probe_repository: P, ) -> bool @@ -998,6 +1010,7 @@ where checks, prepared, github_app, + host, &integration.permissions, perm_details, ) @@ -1163,11 +1176,12 @@ async fn check_primary_only_github_token( checks: &mut Vec, prepared: &PreparedManifest, github_app: Option, + host: &fabro_types::GitHubHost, permissions: &HashMap, perm_details: Vec, ) -> bool { if let (Some(creds), Some(git)) = (&github_app, prepared.git.as_ref()) { - match mint_github_token(creds, &git.origin_url, permissions).await { + match mint_github_token(creds, host, &git.origin_url, permissions).await { Ok(_) => { checks.push(CheckResult { name: "GitHub Token".into(), @@ -1207,9 +1221,10 @@ async fn check_primary_only_github_token( async fn mint_scoped_github_token( access: fabro_github::GitHubRepositoryAccess, creds: fabro_github::GitHubCredentials, + host: &fabro_types::GitHubHost, ) -> std::result::Result { - let source = - InstallationTokenSource::for_access(&creds, &access).map_err(|err| format!("{err:#}"))?; + let source = InstallationTokenSource::for_access(&creds, host, &access) + .map_err(|err| format!("{err:#}"))?; source.resolve().await.map_err(|err| format!("{err:#}")) } @@ -1356,6 +1371,7 @@ async fn run_probe_ls_remote(url: &str, token: &ResolvedToken) -> std::result::R async fn mint_github_token( creds: &fabro_github::GitHubCredentials, + host: &fabro_types::GitHubHost, origin_url: &str, permissions: &HashMap, ) -> Result { @@ -1364,13 +1380,7 @@ async fn mint_github_token( let client = fabro_http::http_client()?; let perms_json = serde_json::to_value(permissions)?; creds - .resolve_bearer_token( - &client, - &owner, - &repo, - &fabro_github::github_api_base_url(), - perms_json, - ) + .resolve_bearer_token(&client, &owner, &repo, host, perms_json) .await } @@ -3155,6 +3165,7 @@ dockerfile = { path = "Dockerfile" } &prepared, &resolved, Some(pat_creds()), + &fabro_types::GitHubHost::default(), move |access, _creds| { minted_for_seam.lock().unwrap().push(access); async { Ok(static_token("scoped-token")) } @@ -3212,6 +3223,7 @@ dockerfile = { path = "Dockerfile" } &prepared, &resolved, Some(pat_creds()), + &fabro_types::GitHubHost::default(), |_access, _creds| async { Err( "the GitHub App installation cannot see repository acme/keystone; add \ @@ -3250,6 +3262,7 @@ dockerfile = { path = "Dockerfile" } &prepared, &resolved, Some(pat_creds()), + &fabro_types::GitHubHost::default(), |_access, _creds| async { Ok(static_token("scoped-token")) }, |slug, _token| async move { if slug.repo() == "keystone" { @@ -3296,6 +3309,7 @@ dockerfile = { path = "Dockerfile" } &prepared, &resolved, Some(pat_creds()), + &fabro_types::GitHubHost::default(), |_access, _creds| async { Ok(static_token("scoped-token")) }, |_slug, _token| async { Ok(()) }, ) @@ -3324,6 +3338,7 @@ dockerfile = { path = "Dockerfile" } &prepared, &resolved, Some(pat_creds()), + &fabro_types::GitHubHost::default(), |_access, _creds| async { Ok(static_token("scoped-token")) }, |_slug, _token| async { Ok(()) }, ) @@ -3354,6 +3369,7 @@ dockerfile = { path = "Dockerfile" } &prepared, &resolved, None, + &fabro_types::GitHubHost::default(), |_access, _creds| async { Ok(static_token("scoped-token")) }, |_slug, _token| async { Ok(()) }, ) diff --git a/lib/apps/fabro-server/src/serve.rs b/lib/apps/fabro-server/src/serve.rs index 06e8cc56d..ab4a0ed09 100644 --- a/lib/apps/fabro-server/src/serve.rs +++ b/lib/apps/fabro-server/src/serve.rs @@ -334,7 +334,9 @@ async fn start_webhook_strategy( ); return Ok(None); }; - match TailscaleFunnelManager::start(port, &app_id, &private_key_pem).await { + match TailscaleFunnelManager::start(port, &state.github_host, &app_id, &private_key_pem) + .await + { Ok(manager) => Ok(Some(manager)), Err(err) => { error!( @@ -356,8 +358,13 @@ async fn start_webhook_strategy( ) })?; let webhook_url = format!("{}{WEBHOOK_ROUTE}", server_api_url.trim_end_matches('/')); - match fabro_github::update_app_webhook_config(&app_id, &private_key_pem, &webhook_url) - .await + match fabro_github::update_app_webhook_config( + &state.github_host, + &app_id, + &private_key_pem, + &webhook_url, + ) + .await { Ok(()) => info!(url = %webhook_url, "GitHub App webhook URL updated"), Err(err) => warn!( diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index 91f7bfa30..58db78bed 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -1277,7 +1277,7 @@ impl AppState { .map_err(|source| RunMaterializeError::LoadCredentials { source })?; ProductionAutomationRunMaterializer::new( credentials, - self.github_host.api_url().to_string(), + self.github_host.clone(), self.http_client.clone(), Arc::clone(&self.automation_repo_cache), fabro_workflow_version::WorkflowVersionStore::new(self.store_ref().blobs()), diff --git a/lib/apps/fabro-server/src/server/handler/pull_requests.rs b/lib/apps/fabro-server/src/server/handler/pull_requests.rs index 10e826b30..e665964c5 100644 --- a/lib/apps/fabro-server/src/server/handler/pull_requests.rs +++ b/lib/apps/fabro-server/src/server/handler/pull_requests.rs @@ -123,7 +123,7 @@ pub(in crate::server) fn server_github_context<'a>( })?; Ok(fabro_github::GitHubContext::with_http_client( creds, - state.github_host.api_url(), + &state.github_host, http_client, )) } diff --git a/lib/apps/fabro-server/src/server/handler/system.rs b/lib/apps/fabro-server/src/server/handler/system.rs index 917fff590..c950df09d 100644 --- a/lib/apps/fabro-server/src/server/handler/system.rs +++ b/lib/apps/fabro-server/src/server/handler/system.rs @@ -493,9 +493,11 @@ async fn get_github_repo( .into_response(); } }; - let install_url = creds.installation_url(&owner).unwrap_or_else(|| { - format!("https://github.com/organizations/{owner}/settings/installations") - }); + let install_url = creds + .installation_url(&state.github_host, &owner) + .unwrap_or_else(|| { + format!("https://github.com/organizations/{owner}/settings/installations") + }); let client = match state.http_client() { Ok(http) => http, @@ -537,7 +539,7 @@ async fn get_github_repo( &jwt, &owner, &name, - &base_url, + &state.github_host, serde_json::json!({ "contents": "write", "pull_requests": "write" }), Some(&install_url), ) diff --git a/lib/components/fabro-github/src/identity.rs b/lib/components/fabro-github/src/identity.rs index ac53116b6..b1c4e36d4 100644 --- a/lib/components/fabro-github/src/identity.rs +++ b/lib/components/fabro-github/src/identity.rs @@ -2,11 +2,12 @@ //! //! A run's commits carry the identity of the credential the run uses: //! the App's bot account (`[bot]`) or the authenticated user of a -//! personal access token. Both resolve to a stable GitHub.com noreply -//! address, `{id}+{login}@users.noreply.github.com`, so GitHub attributes +//! personal access token. Both resolve to a stable noreply +//! address, `{id}+{login}@users.noreply.`, so GitHub attributes //! the commits to that account without a private email. use anyhow::{Context as _, bail}; +use fabro_types::GitHubHost; use serde::Deserialize; use crate::{GitHubAppCredentials, HttpClient, HttpMethod, github_headers, sign_app_jwt}; @@ -21,10 +22,10 @@ pub struct GitHubAccountIdentity { } impl GitHubAccountIdentity { - /// The GitHub.com noreply address GitHub attributes to this account. + /// The noreply address `host` attributes to this account. #[must_use] - pub fn noreply_email(&self) -> String { - format!("{}+{}@users.noreply.github.com", self.id, self.login) + pub fn noreply_email(&self, host: &GitHubHost) -> String { + format!("{}+{}@{}", self.id, self.login, host.noreply_email_domain()) } } @@ -153,6 +154,24 @@ mod tests { } } + #[test] + fn noreply_email_uses_the_hosts_domain() { + let identity = GitHubAccountIdentity { + login: "octo".to_string(), + id: 7, + }; + let ghes = GitHubHost::new("https://ghe.example.com", None).unwrap(); + assert_eq!( + identity.noreply_email(&ghes), + "7+octo@users.noreply.ghe.example.com" + ); + let ghec = GitHubHost::new("https://octocorp.ghe.com", None).unwrap(); + assert_eq!( + identity.noreply_email(&ghec), + "7+octo@users.noreply.octocorp.ghe.com" + ); + } + #[test] fn noreply_email_uses_id_plus_login() { let identity = GitHubAccountIdentity { @@ -160,7 +179,7 @@ mod tests { id: 281_434_857, }; assert_eq!( - identity.noreply_email(), + identity.noreply_email(&GitHubHost::default()), "281434857+fabro-sh[bot]@users.noreply.github.com" ); } @@ -251,7 +270,7 @@ mod tests { assert_eq!(identity.login, "my-app[bot]"); assert_eq!(identity.id, 4242); assert_eq!( - identity.noreply_email(), + identity.noreply_email(&GitHubHost::default()), "4242+my-app[bot]@users.noreply.github.com" ); // The configured slug skips the `/app` lookup entirely. diff --git a/lib/components/fabro-github/src/lib.rs b/lib/components/fabro-github/src/lib.rs index c4a9f3fdb..83f116945 100644 --- a/lib/components/fabro-github/src/lib.rs +++ b/lib/components/fabro-github/src/lib.rs @@ -4,9 +4,10 @@ use base64::engine::general_purpose::STANDARD; use chrono::{DateTime, Utc}; use fabro_redact::DisplaySafeUrl; use fabro_static::EnvVars; +pub use fabro_types::GitHubHost; +use fabro_types::PullRequestGithubDetail; use fabro_types::settings::run::MergeStrategy; use fabro_types::settings::server::GithubIntegrationSettings; -use fabro_types::{GitHubHost, PullRequestGithubDetail}; use serde::Deserialize; use tokio::process::Command; @@ -51,17 +52,6 @@ pub fn apply_probe_git_env(command: &mut Command, token: &str) { .env("GIT_CONFIG_VALUE_0", GITHUB_CREDENTIAL_HELPER); } -/// REST API base from `GITHUB_BASE_URL` or github.com, for call sites that -/// have no settings to resolve a [`GitHubHost`] from. -#[expect( - clippy::disallowed_methods, - reason = "GitHub API client exposes a documented process-env base URL override." -)] -pub fn github_api_base_url() -> String { - std::env::var(EnvVars::GITHUB_BASE_URL) - .unwrap_or_else(|_| GitHubHost::default().api_url().to_string()) -} - /// Resolves the GitHub deployment from settings, letting `GITHUB_WEB_URL` /// override the web origin and `GITHUB_BASE_URL` override the REST API base. #[expect( @@ -91,33 +81,33 @@ fn host_with_overrides( host.context("invalid GITHUB_WEB_URL or GITHUB_BASE_URL override") } -/// Bundle of GitHub credentials and the API base URL, threaded through every +/// Bundle of GitHub credentials and the GitHub host, threaded through every /// authenticated GitHub call. Lets call sites pass one parameter instead of /// two, and keeps the auth/endpoint pair from drifting apart. #[derive(Debug, Clone)] pub struct GitHubContext<'a> { creds: &'a GitHubCredentials, - base_url: &'a str, + host: &'a GitHubHost, http_client: Option, } impl<'a> GitHubContext<'a> { - pub fn new(creds: &'a GitHubCredentials, base_url: &'a str) -> Self { + pub fn new(creds: &'a GitHubCredentials, host: &'a GitHubHost) -> Self { Self { creds, - base_url, + host, http_client: None, } } pub fn with_http_client( creds: &'a GitHubCredentials, - base_url: &'a str, + host: &'a GitHubHost, http_client: fabro_http::HttpClient, ) -> Self { Self { creds, - base_url, + host, http_client: Some(http_client), } } @@ -202,9 +192,12 @@ impl GitHubAppCredentials { })) } - pub fn installation_url(&self, owner: &str) -> Option { + pub fn installation_url(&self, host: &GitHubHost, owner: &str) -> Option { self.slug.as_ref().map(|slug| { - format!("https://github.com/organizations/{owner}/settings/apps/{slug}/installations") + format!( + "{}/organizations/{owner}/settings/apps/{slug}/installations", + host.web_url() + ) }) } @@ -213,7 +206,7 @@ impl GitHubAppCredentials { client: &impl HttpClient, owner: &str, repo: &str, - base_url: &str, + host: &GitHubHost, permissions: serde_json::Value, install_url: Option<&str>, ) -> anyhow::Result { @@ -221,7 +214,7 @@ impl GitHubAppCredentials { client, owner, &[repo.to_string()], - base_url, + host, permissions, install_url, ) @@ -236,19 +229,19 @@ impl GitHubAppCredentials { client: &impl HttpClient, owner: &str, repository_names: &[String], - base_url: &str, + host: &GitHubHost, permissions: serde_json::Value, install_url: Option<&str>, ) -> anyhow::Result { let jwt = sign_app_jwt(&self.app_id, &self.private_key_pem)?; - let default_install_url = self.installation_url(owner); + let default_install_url = self.installation_url(host, owner); let install_url = install_url.or(default_install_url.as_deref()); mint_installation_token_with_jwt( client, &jwt, owner, repository_names, - base_url, + host, permissions, install_url, ) @@ -309,18 +302,18 @@ impl GitHubCredentials { client: &impl HttpClient, owner: &str, repo: &str, - base_url: &str, + host: &GitHubHost, permissions: serde_json::Value, ) -> anyhow::Result { match self { Self::App(creds) => { - let install_url = creds.installation_url(owner); + let install_url = creds.installation_url(host, owner); creds .mint_installation_token( client, owner, repo, - base_url, + host, permissions, install_url.as_deref(), ) @@ -343,9 +336,9 @@ pub fn validate_static_github_token(token: &str) -> anyhow::Result<()> { Ok(()) } -pub async fn gh_auth_token() -> anyhow::Result { +pub async fn gh_auth_token(host: &GitHubHost) -> anyhow::Result { let output = Command::new("gh") - .args(["auth", "token"]) + .args(["auth", "token", "--hostname", host.host()]) .output() .await .context("Failed to run `gh auth token`")?; @@ -539,7 +532,7 @@ pub async fn create_installation_access_token_with_permissions( jwt: &str, owner: &str, repo: &str, - base_url: &str, + host: &GitHubHost, permissions: serde_json::Value, ) -> anyhow::Result { create_installation_access_token_with_permissions_and_install_url( @@ -547,7 +540,7 @@ pub async fn create_installation_access_token_with_permissions( jwt, owner, repo, - base_url, + host, permissions, None, ) @@ -559,7 +552,7 @@ pub async fn create_installation_access_token_with_permissions_and_install_url( jwt: &str, owner: &str, repo: &str, - base_url: &str, + host: &GitHubHost, permissions: serde_json::Value, install_url: Option<&str>, ) -> anyhow::Result { @@ -568,7 +561,7 @@ pub async fn create_installation_access_token_with_permissions_and_install_url( jwt, owner, &[repo.to_string()], - base_url, + host, permissions, install_url, ) @@ -627,10 +620,11 @@ async fn mint_installation_token_with_jwt( jwt: &str, owner: &str, repos: &[String], - base_url: &str, + host: &GitHubHost, permissions: serde_json::Value, install_url: Option<&str>, ) -> anyhow::Result { + let base_url = host.api_url(); let Some(primary_repo) = repos.first() else { bail!("installation token mint requires at least one repository"); }; @@ -646,7 +640,12 @@ async fn mint_installation_token_with_jwt( InstallationLookup::Found(id) => id, InstallationLookup::NotFound => { let install_url = install_url.map_or_else( - || format!("https://github.com/organizations/{owner}/settings/installations"), + || { + format!( + "{}/organizations/{owner}/settings/installations", + host.web_url() + ) + }, str::to_string, ); bail!( @@ -754,14 +753,14 @@ pub async fn create_installation_access_token( jwt: &str, owner: &str, repo: &str, - base_url: &str, + host: &GitHubHost, ) -> anyhow::Result { create_installation_access_token_with_permissions( client, jwt, owner, repo, - base_url, + host, serde_json::json!({ "contents": "write" }), ) .await @@ -774,14 +773,14 @@ pub async fn create_installation_access_token_for_pr( jwt: &str, owner: &str, repo: &str, - base_url: &str, + host: &GitHubHost, ) -> anyhow::Result { create_installation_access_token_with_permissions( client, jwt, owner, repo, - base_url, + host, serde_json::json!({ "contents": "write", "pull_requests": "write" }), ) .await @@ -846,12 +845,15 @@ pub async fn find_open_pull_request_with_client( client, owner, repo, - ctx.base_url, + ctx.host, serde_json::json!({ "contents": "write", "pull_requests": "write" }), ) .await?; - let mut url = DisplaySafeUrl::parse(&format!("{}/repos/{owner}/{repo}/pulls", ctx.base_url)) - .context("Failed to build pull request reconciliation URL")?; + let mut url = DisplaySafeUrl::parse(&format!( + "{}/repos/{owner}/{repo}/pulls", + ctx.host.api_url() + )) + .context("Failed to build pull request reconciliation URL")?; url.query_pairs_mut() .append_pair("state", "open") .append_pair("base", base) @@ -938,7 +940,7 @@ pub async fn create_pull_request_with_client( client, owner, repo, - ctx.base_url, + ctx.host, serde_json::json!({ "contents": "write", "pull_requests": "write" }), ) .await?; @@ -953,7 +955,7 @@ pub async fn create_pull_request_with_client( "draft": draft, }); - let url = format!("{}/repos/{owner}/{repo}/pulls", ctx.base_url); + let url = format!("{}/repos/{owner}/{repo}/pulls", ctx.host.api_url()); let auth = format!("Bearer {token}"); let resp = HttpClient::request( client, @@ -1034,7 +1036,7 @@ pub async fn enable_auto_merge_with_client( client, owner, repo, - ctx.base_url, + ctx.host, serde_json::json!({ "contents": "write", "pull_requests": "write" }), ) .await?; @@ -1059,7 +1061,7 @@ pub async fn enable_auto_merge_with_client( "Enabling auto-merge" ); - let graphql_url = format!("{}/graphql", ctx.base_url); + let graphql_url = ctx.host.graphql_url(); let auth = format!("Bearer {token}"); let graphql_body = serde_json::json!({ "query": query }); let resp = HttpClient::request( @@ -1177,12 +1179,15 @@ async fn branch_head_sha_with_client( client, owner, repo, - ctx.base_url, + ctx.host, serde_json::json!({ "contents": "read" }), ) .await?; - let url = format!("{}/repos/{owner}/{repo}/branches/{branch}", ctx.base_url); + let url = format!( + "{}/repos/{owner}/{repo}/branches/{branch}", + ctx.host.api_url() + ); let auth = format!("Bearer {token}"); let resp = client .request(HttpMethod::Get, &url, &github_headers(&auth), None) @@ -1269,13 +1274,14 @@ pub async fn get_authenticated_app( /// /// Signs an App JWT and sets the webhook endpoint and content type. pub async fn update_app_webhook_config( + host: &GitHubHost, app_id: &str, private_key_pem: &str, webhook_url: &str, ) -> anyhow::Result<()> { let jwt = sign_app_jwt(app_id, private_key_pem)?; let client = http_client()?; - let url = format!("{}/app/hook/config", github_api_base_url()); + let url = format!("{}/app/hook/config", host.api_url()); let auth = format!("Bearer {jwt}"); let body = serde_json::json!({ "url": webhook_url, @@ -1411,7 +1417,7 @@ async fn mint_git_token( permissions: serde_json::Value, ) -> anyhow::Result { ctx.creds - .resolve_bearer_token(client, owner, repo, ctx.base_url, permissions) + .resolve_bearer_token(client, owner, repo, ctx.host, permissions) .await } @@ -1470,12 +1476,12 @@ pub async fn get_pull_request_with_client( client, owner, repo, - ctx.base_url, + ctx.host, serde_json::json!({ "contents": "write", "pull_requests": "write" }), ) .await?; - let url = format!("{}/repos/{owner}/{repo}/pulls/{number}", ctx.base_url); + let url = format!("{}/repos/{owner}/{repo}/pulls/{number}", ctx.host.api_url()); let auth = format!("Bearer {token}"); let resp = client .request(HttpMethod::Get, &url, &github_headers(&auth), None) @@ -1540,12 +1546,15 @@ pub async fn merge_pull_request_with_client( client, owner, repo, - ctx.base_url, + ctx.host, serde_json::json!({ "contents": "write", "pull_requests": "write" }), ) .await?; - let url = format!("{}/repos/{owner}/{repo}/pulls/{number}/merge", ctx.base_url); + let url = format!( + "{}/repos/{owner}/{repo}/pulls/{number}/merge", + ctx.host.api_url() + ); let body = serde_json::json!({ "merge_method": method }); let auth = format!("Bearer {token}"); @@ -1604,12 +1613,12 @@ pub async fn close_pull_request_with_client( client, owner, repo, - ctx.base_url, + ctx.host, serde_json::json!({ "contents": "write", "pull_requests": "write" }), ) .await?; - let url = format!("{}/repos/{owner}/{repo}/pulls/{number}", ctx.base_url); + let url = format!("{}/repos/{owner}/{repo}/pulls/{number}", ctx.host.api_url()); let body = serde_json::json!({ "state": "closed" }); let auth = format!("Bearer {token}"); @@ -1645,14 +1654,14 @@ pub async fn create_installation_access_token_for_projects( jwt: &str, owner: &str, repo: &str, - base_url: &str, + host: &GitHubHost, ) -> anyhow::Result { create_installation_access_token_with_permissions( client, jwt, owner, repo, - base_url, + host, serde_json::json!({ "issues": "write", "organization_projects": "write" }), ) .await @@ -1671,6 +1680,75 @@ mod tests { use super::*; + fn ghes() -> GitHubHost { + GitHubHost::new("https://ghe.example.com", None).unwrap() + } + + fn app_with_slug(slug: Option<&str>) -> GitHubAppCredentials { + GitHubAppCredentials { + app_id: "test".to_string(), + private_key_pem: test_rsa_key().to_string(), + slug: slug.map(str::to_string), + } + } + + #[test] + fn installation_url_uses_the_web_origin() { + let creds = app_with_slug(Some("fabro")); + assert_eq!( + creds + .installation_url(&GitHubHost::default(), "acme") + .unwrap(), + "https://github.com/organizations/acme/settings/apps/fabro/installations" + ); + assert_eq!( + creds.installation_url(&ghes(), "acme").unwrap(), + "https://ghe.example.com/organizations/acme/settings/apps/fabro/installations" + ); + } + + #[tokio::test] + async fn missing_installation_guidance_names_the_configured_host() { + let mock = + MockHttpClient::new().on(HttpMethod::Get, "/repos/acme/widgets/installation", 404, ""); + let error = app_with_slug(None) + .mint_installation_token( + &mock, + "acme", + "widgets", + &ghes(), + serde_json::json!({}), + None, + ) + .await + .unwrap_err(); + assert!( + error + .to_string() + .contains("https://ghe.example.com/organizations/acme/settings/installations"), + "{error}" + ); + } + + #[tokio::test] + async fn auto_merge_posts_to_the_hosts_graphql_endpoint() { + let mock = + MockHttpClient::new().on(HttpMethod::Post, "/api/graphql", 200, r#"{"data":{}}"#); + let creds = GitHubCredentials::Pat("ghp_test".to_string()); + let host = ghes(); + enable_auto_merge_with_client( + &mock, + &GitHubContext::new(&creds, &host), + "acme", + "widgets", + "PR_node", + MergeStrategy::Squash, + ) + .await + .unwrap(); + assert_eq!(mock.request_count(), 1); + } + #[test] fn host_overrides_apply_over_settings() { let configured = GitHubHost::new("https://ghe.example.com", None).unwrap(); @@ -2041,7 +2119,7 @@ mod tests { &mock, "owner", "repo", - "", + &GitHubHost::default(), serde_json::json!({ "contents": "write" }), None, ) @@ -2103,7 +2181,7 @@ mod tests { &mock, access.owner(), &access.repository_names(), - "", + &GitHubHost::default(), access.permissions_json().unwrap(), None, ) @@ -2132,9 +2210,15 @@ mod tests { .with_req_header("Authorization", "Bearer test-jwt") .with_req_body(r#"{"permissions":{"contents":"write"},"repositories":["repo"]}"#); - let token = create_installation_access_token(&mock, "test-jwt", "owner", "repo", "") - .await - .unwrap(); + let token = create_installation_access_token( + &mock, + "test-jwt", + "owner", + "repo", + &GitHubHost::default(), + ) + .await + .unwrap(); assert_eq!(token, "ghs_xxx"); } @@ -2147,10 +2231,11 @@ mod tests { let mock = MockHttpClient::new().on(HttpMethod::Get, "/repos/owner/repo/installation", 404, ""); - let err = create_installation_access_token(&mock, "jwt", "owner", "repo", "") - .await - .unwrap_err() - .to_string(); + let err = + create_installation_access_token(&mock, "jwt", "owner", "repo", &GitHubHost::default()) + .await + .unwrap_err() + .to_string(); assert!(err.contains("not installed"), "got: {err}"); assert!(err.contains("owner"), "got: {err}"); } @@ -2167,7 +2252,7 @@ mod tests { "jwt", "owner", "repo", - "", + &GitHubHost::default(), serde_json::json!({ "contents": "write" }), Some(install_url), ) @@ -2188,10 +2273,11 @@ mod tests { let mock = MockHttpClient::new().on(HttpMethod::Get, "/repos/owner/repo/installation", 403, ""); - let err = create_installation_access_token(&mock, "jwt", "owner", "repo", "") - .await - .unwrap_err() - .to_string(); + let err = + create_installation_access_token(&mock, "jwt", "owner", "repo", &GitHubHost::default()) + .await + .unwrap_err() + .to_string(); assert!(err.contains("suspended"), "got: {err}"); } @@ -2211,10 +2297,11 @@ mod tests { "", ); - let err = create_installation_access_token(&mock, "jwt", "owner", "repo", "") - .await - .unwrap_err() - .to_string(); + let err = + create_installation_access_token(&mock, "jwt", "owner", "repo", &GitHubHost::default()) + .await + .unwrap_err() + .to_string(); assert!(err.contains("does not have access"), "got: {err}"); assert!(err.contains("repo"), "got: {err}"); } @@ -2245,7 +2332,8 @@ mod tests { ) .with_req_header("Authorization", "Bearer ghu_test"); let creds = GitHubCredentials::Pat("ghu_test".to_string()); - let ctx = GitHubContext::new(&creds, "https://api.test"); + let host = GitHubHost::default(); + let ctx = GitHubContext::new(&creds, &host); let found = find_open_pull_request_with_client( &mock, @@ -2270,10 +2358,11 @@ mod tests { let mock = MockHttpClient::new().on(HttpMethod::Get, "/repos/owner/repo/installation", 401, ""); - let err = create_installation_access_token(&mock, "jwt", "owner", "repo", "") - .await - .unwrap_err() - .to_string(); + let err = + create_installation_access_token(&mock, "jwt", "owner", "repo", &GitHubHost::default()) + .await + .unwrap_err() + .to_string(); assert!(err.contains("authentication failed"), "got: {err}"); } @@ -2302,9 +2391,15 @@ mod tests { r#"{"permissions":{"contents":"write","pull_requests":"write"},"repositories":["repo"]}"#, ); - let token = create_installation_access_token_for_pr(&mock, "test-jwt", "owner", "repo", "") - .await - .unwrap(); + let token = create_installation_access_token_for_pr( + &mock, + "test-jwt", + "owner", + "repo", + &GitHubHost::default(), + ) + .await + .unwrap(); assert_eq!(token, "ghs_pr_token"); } @@ -2350,7 +2445,7 @@ mod tests { let creds = app_creds(); let result = branch_head_sha_with_client( &mock, - &GitHubContext::new(&creds, ""), + &GitHubContext::new(&creds, &GitHubHost::default()), "owner", "repo", "my-branch", @@ -2372,7 +2467,7 @@ mod tests { let creds = app_creds(); let result = branch_head_sha_with_client( &mock, - &GitHubContext::new(&creds, ""), + &GitHubContext::new(&creds, &GitHubHost::default()), "owner", "repo", "no-such-branch", @@ -2394,7 +2489,7 @@ mod tests { let creds = app_creds(); let result = branch_head_sha_with_client( &mock, - &GitHubContext::new(&creds, ""), + &GitHubContext::new(&creds, &GitHubHost::default()), "owner", "repo", "broken", @@ -2418,7 +2513,7 @@ mod tests { let creds = GitHubCredentials::Pat("ghu_test".to_string()); let result = branch_head_sha_with_client( &mock, - &GitHubContext::new(&creds, ""), + &GitHubContext::new(&creds, &GitHubHost::default()), "owner", "repo", "my-branch", @@ -2563,7 +2658,7 @@ mod tests { }); let detail = get_pull_request_with_client( &mock, - &GitHubContext::new(&creds, ""), + &GitHubContext::new(&creds, &GitHubHost::default()), "owner", "repo", 42, @@ -2609,7 +2704,7 @@ mod tests { }); let err = get_pull_request_with_client( &mock, - &GitHubContext::new(&creds, ""), + &GitHubContext::new(&creds, &GitHubHost::default()), "owner", "repo", 999, @@ -2643,7 +2738,7 @@ mod tests { let creds = GitHubCredentials::Pat("ghu_test".to_string()); let detail = get_pull_request_with_client( &mock, - &GitHubContext::new(&creds, ""), + &GitHubContext::new(&creds, &GitHubHost::default()), "owner", "repo", 42, @@ -2658,10 +2753,13 @@ mod tests { async fn resolve_clone_credentials_returns_token_for_token_credentials() { let creds = GitHubCredentials::Pat("ghu_test".to_string()); - let credentials = - resolve_clone_credentials(&GitHubContext::new(&creds, ""), "owner", "repo") - .await - .unwrap(); + let credentials = resolve_clone_credentials( + &GitHubContext::new(&creds, &GitHubHost::default()), + "owner", + "repo", + ) + .await + .unwrap(); assert_eq!(credentials.username(), "x-access-token"); assert_eq!(credentials.password(), "ghu_test"); @@ -2672,10 +2770,13 @@ mod tests { async fn resolve_read_only_clone_credentials_returns_static_token_unchanged() { let creds = GitHubCredentials::Pat("ghu_test".to_string()); - let credentials = - resolve_read_only_clone_credentials(&GitHubContext::new(&creds, ""), "owner", "repo") - .await - .unwrap(); + let credentials = resolve_read_only_clone_credentials( + &GitHubContext::new(&creds, &GitHubHost::default()), + "owner", + "repo", + ) + .await + .unwrap(); assert_eq!(credentials.username(), "x-access-token"); assert_eq!(credentials.password(), "ghu_test"); @@ -2702,7 +2803,8 @@ mod tests { private_key_pem: test_rsa_key().to_string(), slug: None, }); - let context = GitHubContext::new(&credentials, ""); + let host = GitHubHost::default(); + let context = GitHubContext::new(&credentials, &host); let token = mint_git_token( &mock, &context, @@ -2737,7 +2839,8 @@ mod tests { private_key_pem: test_rsa_key().to_string(), slug: None, }); - let context = GitHubContext::new(&credentials, ""); + let host = GitHubHost::default(); + let context = GitHubContext::new(&credentials, &host); let token = mint_git_token( &mock, &context, @@ -2831,7 +2934,7 @@ mod tests { }); merge_pull_request_with_client( &mock, - &GitHubContext::new(&creds, ""), + &GitHubContext::new(&creds, &GitHubHost::default()), "owner", "repo", 42, @@ -2866,7 +2969,7 @@ mod tests { }); let err = merge_pull_request_with_client( &mock, - &GitHubContext::new(&creds, ""), + &GitHubContext::new(&creds, &GitHubHost::default()), "owner", "repo", 42, @@ -2902,7 +3005,7 @@ mod tests { }); let err = merge_pull_request_with_client( &mock, - &GitHubContext::new(&creds, ""), + &GitHubContext::new(&creds, &GitHubHost::default()), "owner", "repo", 42, @@ -2945,9 +3048,15 @@ mod tests { private_key_pem: pem.to_string(), slug: None, }); - close_pull_request_with_client(&mock, &GitHubContext::new(&creds, ""), "owner", "repo", 42) - .await - .unwrap(); + close_pull_request_with_client( + &mock, + &GitHubContext::new(&creds, &GitHubHost::default()), + "owner", + "repo", + 42, + ) + .await + .unwrap(); } #[tokio::test] @@ -2975,7 +3084,7 @@ mod tests { }); let err = close_pull_request_with_client( &mock, - &GitHubContext::new(&creds, ""), + &GitHubContext::new(&creds, &GitHubHost::default()), "owner", "repo", 999, diff --git a/lib/components/fabro-github/src/token_source.rs b/lib/components/fabro-github/src/token_source.rs index 6e67f3423..a4765568b 100644 --- a/lib/components/fabro-github/src/token_source.rs +++ b/lib/components/fabro-github/src/token_source.rs @@ -17,6 +17,7 @@ use std::time::Duration; use anyhow::{Context as _, bail}; use chrono::{DateTime, Utc}; +use fabro_types::GitHubHost; use tokio::sync::Mutex; use crate::{GitHubAppCredentials, GitHubCredentials, InstallationToken}; @@ -162,10 +163,10 @@ enum AppTokenScope { /// Real minter backed by GitHub App credentials. struct AppTokenMinter { - creds: GitHubAppCredentials, - http: fabro_http::HttpClient, - base_url: String, - scope: AppTokenScope, + creds: GitHubAppCredentials, + http: fabro_http::HttpClient, + host: GitHubHost, + scope: AppTokenScope, } #[async_trait::async_trait] @@ -182,7 +183,7 @@ impl InstallationTokenMinter for AppTokenMinter { &self.http, owner, repo, - &self.base_url, + &self.host, permissions.clone(), None, ) @@ -190,7 +191,7 @@ impl InstallationTokenMinter for AppTokenMinter { } AppTokenScope::Access(access) => { access - .mint_installation_token(&self.creds, &self.http, &self.base_url) + .mint_installation_token(&self.creds, &self.http, self.host.api_url()) .await } } @@ -263,24 +264,26 @@ impl InstallationTokenSource { /// pass through and ignore it. pub fn for_origin( creds: &GitHubCredentials, + host: &GitHubHost, origin_url: &str, permissions: serde_json::Value, ) -> anyhow::Result> { let normalized = crate::normalize_repo_origin_url(origin_url); let (owner, repo) = crate::parse_github_owner_repo(&normalized) .context("parsing GitHub origin for token source")?; - Self::for_repository(creds, owner, repo, permissions) + Self::for_repository(creds, host, owner, repo, permissions) } /// Build a source for an already parsed GitHub repository. pub fn for_repository( creds: &GitHubCredentials, + host: &GitHubHost, owner: String, repo: String, permissions: serde_json::Value, ) -> anyhow::Result> { let repo_display = format!("{owner}/{repo}"); - Self::with_app_scope(creds, repo_display, AppTokenScope::Repository { + Self::with_app_scope(creds, host, repo_display, AppTokenScope::Repository { owner, repo, permissions, @@ -294,15 +297,22 @@ impl InstallationTokenSource { /// single-flight behavior are identical to the single-repository source. pub fn for_access( creds: &GitHubCredentials, + host: &GitHubHost, access: &crate::GitHubRepositoryAccess, ) -> anyhow::Result> { let repository_names = access.repository_names(); let repo_display = repository_set_display(access.owner(), &repository_names)?; - Self::with_app_scope(creds, repo_display, AppTokenScope::Access(access.clone())) + Self::with_app_scope( + creds, + host, + repo_display, + AppTokenScope::Access(access.clone()), + ) } fn with_app_scope( creds: &GitHubCredentials, + host: &GitHubHost, repo_display: String, scope: AppTokenScope, ) -> anyhow::Result> { @@ -317,7 +327,7 @@ impl InstallationTokenSource { minter: Box::new(AppTokenMinter { creds: app.clone(), http, - base_url: crate::github_api_base_url(), + host: host.clone(), scope, }), cache: Mutex::new(None), @@ -550,6 +560,7 @@ mod tests { async fn pat_resolves_as_static_generation_zero() { let source = InstallationTokenSource::for_origin( &GitHubCredentials::Pat("ghp_pat".to_string()), + &GitHubHost::default(), "https://github.com/owner/repo.git", serde_json::json!({ "contents": "write" }), ) @@ -578,6 +589,7 @@ mod tests { let source = InstallationTokenSource::for_access( &GitHubCredentials::Pat("ghp_pat".to_string()), + &GitHubHost::default(), &access, ) .unwrap(); @@ -594,6 +606,7 @@ mod tests { token: "ghs_static".to_string(), expires_at: Utc::now() + chrono::Duration::minutes(30), }), + &GitHubHost::default(), "https://github.com/owner/repo.git", serde_json::json!({}), ) @@ -607,6 +620,7 @@ mod tests { token: "ghs_expired".to_string(), expires_at: Utc::now() - chrono::Duration::seconds(1), }), + &GitHubHost::default(), "https://github.com/owner/repo.git", serde_json::json!({}), ) diff --git a/lib/components/fabro-github/tests/integration.rs b/lib/components/fabro-github/tests/integration.rs index 5dc0ec20a..6801a44b0 100644 --- a/lib/components/fabro-github/tests/integration.rs +++ b/lib/components/fabro-github/tests/integration.rs @@ -4,10 +4,15 @@ use fabro_github::{ get_pull_request, merge_pull_request, resolve_authenticated_url, sign_app_jwt, }; use fabro_test::{GitHubAppOptions, GitHubAppState, TwinGitHub}; +use fabro_types::GitHubHost; use fabro_types::settings::run::MergeStrategy; const TEST_RSA_KEY: &str = include_str!("../src/testdata/rsa_private.pem"); +fn twin_host(twin: &TwinGitHub) -> GitHubHost { + GitHubHost::new(&twin.base_url, Some(&twin.base_url)).expect("twin URL should be a valid host") +} + fn github_credentials() -> GitHubCredentials { GitHubCredentials::App(GitHubAppCredentials { app_id: "42".to_string(), @@ -40,7 +45,8 @@ fn standard_app_state() -> GitHubAppState { async fn create_and_get_pull_request() { let twin = TwinGitHub::start(standard_app_state()).await; let creds = github_credentials(); - let ctx = &GitHubContext::new(&creds, &twin.base_url); + let host = twin_host(&twin); + let ctx = &GitHubContext::new(&creds, &host); let created = create_pull_request( ctx, @@ -71,7 +77,8 @@ async fn create_and_get_pull_request() { async fn create_merge_and_verify_state() { let twin = TwinGitHub::start(standard_app_state()).await; let creds = github_credentials(); - let ctx = &GitHubContext::new(&creds, &twin.base_url); + let host = twin_host(&twin); + let ctx = &GitHubContext::new(&creds, &host); let created = create_pull_request( ctx, "acme", "widgets", "main", "feature", "Merge me", "PR body", false, @@ -104,7 +111,8 @@ async fn create_close_and_verify_state() { let twin = TwinGitHub::start(standard_app_state()).await; let creds = github_credentials(); - let ctx = &GitHubContext::new(&creds, &twin.base_url); + let host = twin_host(&twin); + let ctx = &GitHubContext::new(&creds, &host); let created = create_pull_request( ctx, "acme", "widgets", "main", "feature", "Close me", "PR body", false, @@ -130,7 +138,8 @@ async fn enable_auto_merge_persists() { let twin = TwinGitHub::start(standard_app_state()).await; let creds = github_credentials(); - let ctx = &GitHubContext::new(&creds, &twin.base_url); + let host = twin_host(&twin); + let ctx = &GitHubContext::new(&creds, &host); let created = create_pull_request( ctx, @@ -160,10 +169,9 @@ async fn enable_auto_merge_persists() { }; let jwt = sign_app_jwt(&app_creds.app_id, &app_creds.private_key_pem).unwrap(); let client = fabro_test::test_http_client(); - let token = - create_installation_access_token_for_pr(&client, &jwt, "acme", "widgets", &twin.base_url) - .await - .unwrap(); + let token = create_installation_access_token_for_pr(&client, &jwt, "acme", "widgets", &host) + .await + .unwrap(); let detail: serde_json::Value = fabro_test::test_http_client() .get(format!( @@ -192,9 +200,10 @@ async fn enable_auto_merge_persists() { async fn resolve_authenticated_url_embeds_token() { let twin = TwinGitHub::start(standard_app_state()).await; let creds = github_credentials(); + let host = twin_host(&twin); let url = resolve_authenticated_url( - &GitHubContext::new(&creds, &twin.base_url), + &GitHubContext::new(&creds, &host), "https://github.com/acme/widgets.git", ) .await @@ -214,9 +223,10 @@ async fn resolve_authenticated_url_embeds_token() { async fn resolve_authenticated_url_errors_on_non_github_url() { let twin = TwinGitHub::start(standard_app_state()).await; let creds = github_credentials(); + let host = twin_host(&twin); let error = resolve_authenticated_url( - &GitHubContext::new(&creds, &twin.base_url), + &GitHubContext::new(&creds, &host), "https://gitlab.com/foo/bar", ) .await diff --git a/lib/components/fabro-github/tests/live_access.rs b/lib/components/fabro-github/tests/live_access.rs index 0b13a582c..62c539135 100644 --- a/lib/components/fabro-github/tests/live_access.rs +++ b/lib/components/fabro-github/tests/live_access.rs @@ -21,7 +21,7 @@ use std::time::Duration; use fabro_github::token_source::InstallationTokenSource; use fabro_github::{GitHubAppCredentials, GitHubCredentials, GitHubRepositoryAccess}; -use fabro_types::GitHubRepositorySlug; +use fabro_types::{GitHubHost, GitHubRepositorySlug}; use tokio::process::Command; use tokio::time::sleep; @@ -75,8 +75,8 @@ async fn scoped_token_reaches_the_declared_additional_repository() { // The production choreography: every target resolves to one shared // installation, then one mint scoped to the whole effective set. let creds = GitHubCredentials::App(app.clone()); - let source = - InstallationTokenSource::for_access(&creds, &access).expect("token source should build"); + let source = InstallationTokenSource::for_access(&creds, &GitHubHost::default(), &access) + .expect("token source should build"); let resolved = source.resolve().await.expect("scoped mint should succeed"); let token = resolved.token.expose(); @@ -108,9 +108,12 @@ async fn scoped_token_reaches_the_declared_additional_repository() { ) .expect("primary-only access should validate") .expect("origin should produce an access value"); - let narrow_source = - InstallationTokenSource::for_access(&GitHubCredentials::App(app), &primary_only) - .expect("primary-only token source should build"); + let narrow_source = InstallationTokenSource::for_access( + &GitHubCredentials::App(app), + &GitHubHost::default(), + &primary_only, + ) + .expect("primary-only token source should build"); let narrow = narrow_source .resolve() .await diff --git a/lib/components/fabro-petri/src/stage_credentials.rs b/lib/components/fabro-petri/src/stage_credentials.rs index 547d529ae..ab0bacfdb 100644 --- a/lib/components/fabro-petri/src/stage_credentials.rs +++ b/lib/components/fabro-petri/src/stage_credentials.rs @@ -15,7 +15,7 @@ use fabro_github::token_source::{InstallationTokenSource, ResolvedToken}; use fabro_github::{GITHUB_CREDENTIAL_HELPER_KEY, GitHubCredentials, GitHubRepositoryAccess}; use fabro_static::EnvVars; use fabro_types::settings::run::{RunIntegrationsGithubSettings, RunMode}; -use fabro_types::{GitHubRepositorySlug, RunSpec, RunTarget}; +use fabro_types::{GitHubHost, GitHubRepositorySlug, RunSpec, RunTarget}; use fabro_util::shell; use percent_encoding::{NON_ALPHANUMERIC, utf8_percent_encode}; use petri_runtime::executor::{ @@ -48,6 +48,7 @@ impl StageCredentials { pub fn for_run( spec: &RunSpec, credentials: Option<&GitHubCredentials>, + host: &GitHubHost, read_tokens: Option>, ) -> anyhow::Result> { if spec.settings.run.execution.mode == RunMode::DryRun { @@ -76,7 +77,7 @@ impl StageCredentials { }; let api_tokens = integration .is_token_requested() - .then(|| InstallationTokenSource::for_access(credentials, &access)) + .then(|| InstallationTokenSource::for_access(credentials, host, &access)) .transpose()?; let contents_declared = integration .permissions @@ -487,9 +488,14 @@ mod tests { ); let pat = GitHubCredentials::Pat("scripted-personal-access-token".to_string()); let read = InstallationTokenSource::pat("scripted-personal-access-token".to_string()); - let default = StageCredentials::for_run(&spec, Some(&pat), Some(Arc::clone(&read))) - .expect("policy") - .expect("credentials"); + let default = StageCredentials::for_run( + &spec, + Some(&pat), + &GitHubHost::default(), + Some(Arc::clone(&read)), + ) + .expect("policy") + .expect("credentials"); assert!(default.api_tokens.is_none()); assert_eq!(default.repositories.len(), 1); spec.settings @@ -504,9 +510,10 @@ mod tests { .github .additional_repositories .insert(GitHubRepositorySlug::try_new("acme/another").expect("slug")); - let declared = StageCredentials::for_run(&spec, Some(&pat), Some(read)) - .expect("policy") - .expect("credentials"); + let declared = + StageCredentials::for_run(&spec, Some(&pat), &GitHubHost::default(), Some(read)) + .expect("policy") + .expect("credentials"); assert!(declared.api_tokens.is_some()); assert_eq!(declared.repositories.len(), 2); } diff --git a/lib/components/fabro-tracker/src/github.rs b/lib/components/fabro-tracker/src/github.rs index c0ced8015..8bbffe178 100644 --- a/lib/components/fabro-tracker/src/github.rs +++ b/lib/components/fabro-tracker/src/github.rs @@ -1,7 +1,7 @@ use anyhow::{Context as _, anyhow}; use async_trait::async_trait; use fabro_github::{ - GitHubCredentials, create_installation_access_token_for_projects, sign_app_jwt, + GitHubCredentials, GitHubHost, create_installation_access_token_for_projects, sign_app_jwt, }; use tokio::sync::OnceCell; @@ -35,7 +35,7 @@ pub struct GitHubTracker { owner: String, repo: String, project_number: u64, - base_url: String, + host: GitHubHost, project_node_id: OnceCell, } @@ -46,7 +46,7 @@ impl GitHubTracker { owner: String, repo: String, project_number: u64, - base_url: String, + host: GitHubHost, ) -> Self { Self { creds, @@ -54,13 +54,13 @@ impl GitHubTracker { owner, repo, project_number, - base_url, + host, project_node_id: OnceCell::new(), } } fn graphql_url(&self) -> String { - format!("{}/graphql", self.base_url) + self.host.graphql_url() } async fn fresh_token(&self) -> anyhow::Result { @@ -73,7 +73,7 @@ impl GitHubTracker { &jwt, &self.owner, &self.repo, - &self.base_url, + &self.host, ) .await .context("creating GitHub App installation token") @@ -645,7 +645,7 @@ mod tests { "owner".to_string(), "repo".to_string(), 1, - server_url.to_string(), + GitHubHost::new(server_url, Some(server_url)).unwrap(), ) } @@ -1225,7 +1225,7 @@ mod tests { #[tokio::test] async fn github_tracker_fetch_issues_by_ids_empty() { let pem = test_rsa_key(); - let tracker = mock_github_tracker("http://unused", pem); + let tracker = mock_github_tracker("http://127.0.0.1:1", pem); // Empty input → no HTTP calls at all let issues = tracker.fetch_issues_by_ids(&[]).await.unwrap(); diff --git a/lib/components/fabro-workflow/src/pull_request.rs b/lib/components/fabro-workflow/src/pull_request.rs index 130abe3e4..8cd6f88d1 100644 --- a/lib/components/fabro-workflow/src/pull_request.rs +++ b/lib/components/fabro-workflow/src/pull_request.rs @@ -1272,9 +1272,11 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr let payload = pr_content_json("Fix bug", "Narrative."); let harness = setup_fallback_test_harness_with_branch_sha(&payload, "stale-sha").await; let github_base_url = harness.github_server.url(""); + let github_host = + fabro_github::GitHubHost::new(&github_base_url, Some(&github_base_url)).unwrap(); let error = open_pull_request(OpenPullRequestRequest { run_id: RunId::new(), - github: fabro_github::GitHubContext::new(&harness.creds, &github_base_url), + github: fabro_github::GitHubContext::new(&harness.creds, &github_host), origin_url: "https://github.com/owner/repo.git", base_branch: "main", head_branch: "fabro/run/123", @@ -1553,7 +1555,9 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr .await; let github_base_url = harness.github_server.url(""); - let github = github_app::GitHubContext::new(&harness.creds, &github_base_url); + let github_host = + fabro_github::GitHubHost::new(&github_base_url, Some(&github_base_url)).unwrap(); + let github = github_app::GitHubContext::new(&harness.creds, &github_host); let result = open_pull_request(OpenPullRequestRequest { run_id: RunId::new(), @@ -1601,7 +1605,9 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr let harness = setup_fallback_test_harness(&payload).await; let github_base_url = harness.github_server.url(""); - let github = github_app::GitHubContext::new(&harness.creds, &github_base_url); + let github_host = + fabro_github::GitHubHost::new(&github_base_url, Some(&github_base_url)).unwrap(); + let github = github_app::GitHubContext::new(&harness.creds, &github_host); let result = open_pull_request(OpenPullRequestRequest { run_id: RunId::new(), @@ -1635,7 +1641,9 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr let harness = setup_fallback_test_harness(&payload).await; let github_base_url = harness.github_server.url(""); - let github = github_app::GitHubContext::new(&harness.creds, &github_base_url); + let github_host = + fabro_github::GitHubHost::new(&github_base_url, Some(&github_base_url)).unwrap(); + let github = github_app::GitHubContext::new(&harness.creds, &github_host); // Single ~200-char line, no `Plan:` / heading prefix, no newlines. let goal = "x".repeat(200); From e42fd3c41d2c95738ba38d6129b7c42f5adde087 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Balatoni?= Date: Sun, 25 Oct 2026 00:35:07 +0200 Subject: [PATCH 03/13] feat(github): parse repository and pull request URLs on the configured host --- lib/apps/fabro-cli/src/commands/graph.rs | 1 + lib/apps/fabro-cli/src/commands/preflight.rs | 1 + lib/apps/fabro-cli/src/commands/repo/init.rs | 5 +- lib/apps/fabro-cli/src/commands/run/create.rs | 5 +- .../src/commands/run/petri_worker.rs | 3 +- .../fabro-cli/src/commands/run/publish.rs | 29 ++-- .../src/commands/run/remote_workflow.rs | 20 +-- .../fabro-cli/src/commands/run/resolution.rs | 6 +- lib/apps/fabro-cli/src/commands/validate.rs | 1 + lib/apps/fabro-cli/src/user_config.rs | 10 +- .../src/automation_materializer.rs | 10 +- lib/apps/fabro-server/src/demo/mod.rs | 1 + lib/apps/fabro-server/src/git_checkout.rs | 25 +++- lib/apps/fabro-server/src/run_manifest.rs | 26 +++- lib/apps/fabro-server/src/server.rs | 16 +- .../src/server/handler/pull_requests.rs | 121 +++++++++++++-- .../fabro-server/src/server/handler/runs.rs | 2 +- .../fabro-server/src/server/petri_runs.rs | 1 + .../2026071101_file_definitions_to_sqlite.rs | 3 +- lib/components/fabro-automation/src/model.rs | 6 +- lib/components/fabro-github/src/access.rs | 39 +++-- lib/components/fabro-github/src/lib.rs | 139 +++++++++++++++--- .../fabro-github/src/token_source.rs | 3 +- .../fabro-github/tests/live_access.rs | 4 +- lib/components/fabro-manifest/src/lib.rs | 117 +++++++++++---- .../fabro-petri/src/projection/platform.rs | 7 +- .../fabro-petri/src/projector/mod.rs | 22 ++- lib/components/fabro-petri/src/source.rs | 7 +- .../fabro-petri/src/stage_credentials.rs | 5 +- .../fabro-store/src/platform_records.rs | 44 ++++-- lib/components/fabro-store/src/run_summary.rs | 5 +- .../fabro-store/src/run_summary_store.rs | 13 +- lib/components/fabro-tool/src/create.rs | 2 +- .../fabro-workflow/src/pull_request.rs | 20 +-- .../fabro-workflow/src/run_lookup.rs | 1 + .../fabro-api/tests/run_summary_round_trip.rs | 7 +- lib/foundation/fabro-config/src/builders.rs | 19 ++- .../fabro-config/src/resolve/mod.rs | 1 + .../fabro-config/src/resolve/server.rs | 2 +- lib/foundation/fabro-types/src/github_host.rs | 16 ++ .../fabro-types/src/pull_request.rs | 103 +++++++++---- lib/foundation/fabro-types/src/repository.rs | 71 +++++++-- lib/foundation/fabro-types/src/run_intent.rs | 37 ++++- .../fabro-types/tests/run_intent.rs | 31 +++- 44 files changed, 776 insertions(+), 231 deletions(-) diff --git a/lib/apps/fabro-cli/src/commands/graph.rs b/lib/apps/fabro-cli/src/commands/graph.rs index 285c4add3..be3b505ea 100644 --- a/lib/apps/fabro-cli/src/commands/graph.rs +++ b/lib/apps/fabro-cli/src/commands/graph.rs @@ -39,6 +39,7 @@ pub(crate) async fn run( cwd: ctx.cwd().to_path_buf(), environment_defaults: fabro_environment::seeded_catalog_layer(), user_settings_path: Some(active_settings_path(None)), + github_host: crate::user_config::github_host()?, ..Default::default() })?; let client = ctx.server().await?; diff --git a/lib/apps/fabro-cli/src/commands/preflight.rs b/lib/apps/fabro-cli/src/commands/preflight.rs index 2bf94cb08..d122c3022 100644 --- a/lib/apps/fabro-cli/src/commands/preflight.rs +++ b/lib/apps/fabro-cli/src/commands/preflight.rs @@ -32,6 +32,7 @@ pub(crate) async fn execute( args: preflight_manifest_args(&args), environment_defaults: fabro_environment::seeded_catalog_layer(), user_settings_path: Some(active_settings_path(None)), + github_host: crate::user_config::github_host()?, })?; let spinner = (!ctx.json_output()).then(|| cyan_spinner("Running checks...")); diff --git a/lib/apps/fabro-cli/src/commands/repo/init.rs b/lib/apps/fabro-cli/src/commands/repo/init.rs index 71c644b72..430dd16a8 100644 --- a/lib/apps/fabro-cli/src/commands/repo/init.rs +++ b/lib/apps/fabro-cli/src/commands/repo/init.rs @@ -193,8 +193,11 @@ async fn check_github_app_installation(target: &ServerTargetArgs, base_ctx: &Com }; // Convert SSH URL to HTTPS and parse owner/repo + let Ok(host) = crate::user_config::github_host() else { + return; // Unreadable server settings — skip silently + }; let https_url = fabro_github::ssh_url_to_https(&remote_url); - let Ok((owner, repo)) = fabro_github::parse_github_owner_repo(&https_url) else { + let Ok((owner, repo)) = fabro_github::parse_github_owner_repo(&host, &https_url) else { return; // Not a GitHub repo — skip silently }; diff --git a/lib/apps/fabro-cli/src/commands/run/create.rs b/lib/apps/fabro-cli/src/commands/run/create.rs index 6e07c0a65..bcf4c9eb7 100644 --- a/lib/apps/fabro-cli/src/commands/run/create.rs +++ b/lib/apps/fabro-cli/src/commands/run/create.rs @@ -104,7 +104,10 @@ pub(crate) async fn create_run( && args.target_repo.is_none() && args.target_repo_selector.is_none() => { - fabro_manifest::configured_repo_origin_url_for_location(package.workflow_location())? + fabro_manifest::configured_repo_origin_url_for_location( + &crate::user_config::github_host()?, + package.workflow_location(), + )? } _ => None, }; diff --git a/lib/apps/fabro-cli/src/commands/run/petri_worker.rs b/lib/apps/fabro-cli/src/commands/run/petri_worker.rs index 39652fc87..c17478ab6 100644 --- a/lib/apps/fabro-cli/src/commands/run/petri_worker.rs +++ b/lib/apps/fabro-cli/src/commands/run/petri_worker.rs @@ -221,7 +221,7 @@ pub(super) async fn execute(worker: PetriWorker<'_>) -> Result<()> { None } }; - let github_host = publish::github_host()?; + let github_host = crate::user_config::github_host()?; let read_tokens = publish::read_token_source(&worker.run_state.spec, github.as_ref(), &github_host); runtime.stage_credentials = StageCredentials::for_run( @@ -231,6 +231,7 @@ pub(super) async fn execute(worker: PetriWorker<'_>) -> Result<()> { read_tokens.clone(), )?; let mut source = RunSource::for_run( + &github_host, worker.run_state.spec.target.as_ref(), &worker.run_state.spec.settings.run, None, diff --git a/lib/apps/fabro-cli/src/commands/run/publish.rs b/lib/apps/fabro-cli/src/commands/run/publish.rs index 68976306c..8ea028c7c 100644 --- a/lib/apps/fabro-cli/src/commands/run/publish.rs +++ b/lib/apps/fabro-cli/src/commands/run/publish.rs @@ -52,12 +52,6 @@ const PUSH_TIMEOUT: Duration = Duration::from_mins(5); const PUSH_ATTEMPTS: u32 = 3; const PUSH_RETRY_DELAY: Duration = Duration::from_secs(2); -/// The GitHub deployment the server is configured for. -pub(super) fn github_host() -> Result { - let settings = ServerSettingsBuilder::load_default().context("loading the server settings")?; - fabro_github::github_host(&settings.server.integrations.github) -} - /// The server's GitHub credentials as the worker reaches them, or `None` /// when none are configured. pub(super) fn github_credentials(vault: &Vault) -> Result> { @@ -83,11 +77,11 @@ pub(super) fn github_credentials(vault: &Vault) -> Result Option { +fn repository(spec: &RunSpec, host: &GitHubHost) -> Option { let Some(RunTarget::Git(target)) = spec.target.as_ref() else { return None; }; - Some(target.clone().validate().ok()?.repository().clone()) + Some(target.clone().validate(host).ok()?.repository().clone()) } /// A cached token source for the run's repository with `permissions`, or @@ -98,7 +92,7 @@ fn token_source( host: &GitHubHost, permissions: serde_json::Value, ) -> Option> { - let repository = repository(spec)?; + let repository = repository(spec, host)?; let credentials = credentials?; match InstallationTokenSource::for_repository( credentials, @@ -208,7 +202,7 @@ impl GitHubPublisher { let Some(RunTarget::Git(target)) = spec.target.as_ref() else { return None; }; - let repository = repository(spec)?; + let repository = repository(spec, &host)?; let push_tokens = token_source( spec, credentials.as_ref(), @@ -261,7 +255,7 @@ impl GitHubPublisher { .ok_or_else(|| "no LLM model is available to write the pull request".to_string())?; // The run so far, for the pull request's details: best effort. let run_state = self.client.get_run_state(&self.run_id).await.ok(); - let origin_url = self.repository.https_url(); + let origin_url = self.repository.https_url(&self.host); let created = pull_request::open_pull_request(OpenPullRequestRequest { run_id: self.run_id, github: context, @@ -311,7 +305,15 @@ impl RunPublisher for GitHubPublisher { .resolve() .await .map_err(|err| format!("no push credential for {}: {err:#}", self.repository))?; - push(&self.repository, &resolved.token, site, branch, sha).await + push( + &self.repository, + &self.host, + &resolved.token, + site, + branch, + sha, + ) + .await } async fn publish(&self, publication: &Publication) -> Result<(), String> { @@ -342,12 +344,13 @@ impl RunPublisher for GitHubPublisher { /// repository alone and never appears in the error. async fn push( repository: &GitHubRepositorySlug, + host: &GitHubHost, token: &SecretString, site: &Site, branch: &str, sha: &str, ) -> Result<(), String> { - let mut url = repository.https_url(); + let mut url = repository.https_url(host); url.push_str(".git"); let credential = basic(token); let env = credential diff --git a/lib/apps/fabro-cli/src/commands/run/remote_workflow.rs b/lib/apps/fabro-cli/src/commands/run/remote_workflow.rs index 8bec413d7..61369fd7a 100644 --- a/lib/apps/fabro-cli/src/commands/run/remote_workflow.rs +++ b/lib/apps/fabro-cli/src/commands/run/remote_workflow.rs @@ -7,7 +7,7 @@ use std::time::Duration; use anyhow::{Context as _, bail}; use fabro_manifest::CollectedWorkflowClosure; use fabro_proc::ProcessError; -use fabro_types::{GitHubRepositorySlug, GitRunTarget, repository}; +use fabro_types::{GitHubHost, GitHubRepositorySlug, GitRunTarget, repository}; use tokio::process::Command; use tokio::{fs, signal as tokio_signal, task}; use tokio_util::sync::CancellationToken; @@ -69,17 +69,19 @@ pub(super) enum RemoteWorkflowError { /// the same Git configuration: the user's global and system config applies, /// repository-local config from wherever the CLI was invoked does not. pub(super) struct NativeGit { + host: GitHubHost, timeout: Duration, #[cfg(test)] pub(super) environment: Vec<(String, String)>, } impl NativeGit { - pub(super) fn new() -> Self { + pub(super) fn new(host: GitHubHost) -> Self { Self { - timeout: Duration::from_mins(2), + host, + timeout: Duration::from_mins(2), #[cfg(test)] - environment: Vec::new(), + environment: Vec::new(), } } @@ -157,7 +159,7 @@ impl NativeGit { patterns: &[String], cancel: &CancellationToken, ) -> anyhow::Result { - let url = repository.https_url(); + let url = repository.https_url(&self.host); let mut args = vec!["ls-remote", "--symref", &url]; args.extend(patterns.iter().map(String::as_str)); let bytes = self.command("metadata lookup", root, &args, cancel).await?; @@ -193,7 +195,7 @@ impl NativeGit { tag: None, sha: Some(sha), }; - Ok(target.validate()?.into_target()) + Ok(target.validate(&self.host)?.into_target()) } /// Resolve `revision` to a commit SHA using metadata lookups run from @@ -298,7 +300,7 @@ impl NativeGit { "--depth=1", "--no-tags", "--no-recurse-submodules", - &repository.https_url(), + &repository.https_url(&self.host), sha, ], cancel, @@ -570,7 +572,7 @@ mod tests { ), ) .unwrap(); - let mut git = NativeGit::new(); + let mut git = NativeGit::new(GitHubHost::default()); git.environment = vec![ ("GIT_CONFIG_GLOBAL".into(), config.to_str().unwrap().into()), ("GIT_CONFIG_NOSYSTEM".into(), "1".into()), @@ -1081,7 +1083,7 @@ mod tests { let path = root.path().join("git"); std::fs::write(&path, format!("#!/bin/sh\n{script}\n")).unwrap(); std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap(); - let mut git = NativeGit::new(); + let mut git = NativeGit::new(GitHubHost::default()); git.environment .push(("PATH".into(), root.path().to_str().unwrap().into())); (root, git) diff --git a/lib/apps/fabro-cli/src/commands/run/resolution.rs b/lib/apps/fabro-cli/src/commands/run/resolution.rs index 085030c2c..f953f6178 100644 --- a/lib/apps/fabro-cli/src/commands/run/resolution.rs +++ b/lib/apps/fabro-cli/src/commands/run/resolution.rs @@ -55,7 +55,7 @@ pub(super) async fn workflow( selector, revision, } => { - let git = NativeGit::new(); + let git = NativeGit::new(crate::user_config::github_host()?); let (repository, selector, revision) = (repository.clone(), selector.clone(), revision.clone()); let closure = interruption @@ -84,7 +84,7 @@ pub(super) async fn target( if !provider.clones_workspace() { bail!("Git targets require a clone-enabled environment"); } - let git = NativeGit::new(); + let git = NativeGit::new(crate::user_config::github_host()?); let (repository, branch) = (repository.clone(), branch.clone()); let target = interruption .owned(move |cancel| async move { @@ -101,9 +101,11 @@ pub(super) async fn target( // The existing observer can push/query Git synchronously. Preserve its // behavior without blocking a Tokio worker or promising a new timeout. let provider = provider.clone(); + let host = crate::user_config::github_host()?; let configured_repo_origin_url = configured_repo_origin_url.map(str::to_owned); let derived = task::spawn_blocking(move || { fabro_manifest::derive_run_target_for_provider( + &host, &provider, &path, configured_repo_origin_url.as_deref(), diff --git a/lib/apps/fabro-cli/src/commands/validate.rs b/lib/apps/fabro-cli/src/commands/validate.rs index cec9f7672..16e3e7c1a 100644 --- a/lib/apps/fabro-cli/src/commands/validate.rs +++ b/lib/apps/fabro-cli/src/commands/validate.rs @@ -22,6 +22,7 @@ pub(crate) fn run( cwd: base_ctx.cwd().to_path_buf(), environment_defaults: fabro_environment::seeded_catalog_layer(), user_settings_path: Some(active_settings_path(None)), + github_host: crate::user_config::github_host()?, ..Default::default() })?; let response = manifest_validation::validate_manifest(&RunLayer::default(), &built.manifest)?; diff --git a/lib/apps/fabro-cli/src/user_config.rs b/lib/apps/fabro-cli/src/user_config.rs index e842a389e..ca90712f3 100644 --- a/lib/apps/fabro-cli/src/user_config.rs +++ b/lib/apps/fabro-cli/src/user_config.rs @@ -14,7 +14,7 @@ use fabro_static::EnvVars; use fabro_types::settings::RunNamespace; use fabro_types::settings::cli::CliTargetSettings; use fabro_types::settings::server::LogDestination; -use fabro_types::{ServerSettings, UserSettings}; +use fabro_types::{GitHubHost, ServerSettings, UserSettings}; use fabro_util::error::SharedError; use fabro_util::version::FABRO_VERSION; use tokio::fs; @@ -23,6 +23,14 @@ use tracing::debug; use crate::args::ServerTargetArgs; +/// The GitHub deployment configured in the local server settings, with the +/// `GITHUB_WEB_URL` / `GITHUB_BASE_URL` overrides applied. +pub(crate) fn github_host() -> Result { + let configured = ServerSettingsBuilder::load_default_github_host() + .context("loading the GitHub host from the server settings")?; + fabro_github::with_env_overrides(&configured) +} + #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub(crate) struct RunSettingsKeyPresence { pub(crate) run: bool, diff --git a/lib/apps/fabro-server/src/automation_materializer.rs b/lib/apps/fabro-server/src/automation_materializer.rs index ac32323f1..e5d8e8f79 100644 --- a/lib/apps/fabro-server/src/automation_materializer.rs +++ b/lib/apps/fabro-server/src/automation_materializer.rs @@ -128,6 +128,7 @@ pub(crate) struct ProductionAutomationRunMaterializer { remote_resolver: Arc, repo_cache: Arc, version_store: WorkflowVersionStore, + host: fabro_types::GitHubHost, } /// Where to fetch a repository from and how to authenticate. @@ -159,7 +160,7 @@ impl AutomationGitRemoteResolver for ServerGitHubRemoteResolver { ) .await?; Ok(GitRemote { - clone_url: git_checkout::github_clone_url(repo), + clone_url: git_checkout::github_clone_url(repo, &self.host), auth, }) } @@ -176,11 +177,12 @@ impl ProductionAutomationRunMaterializer { Self { remote_resolver: Arc::new(ServerGitHubRemoteResolver { credentials: github_credentials, - host: github_host, + host: github_host.clone(), http_client, }), repo_cache, version_store, + host: github_host, } } @@ -232,13 +234,13 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer { ) -> Result { let validated_target = input .target - .validate() + .validate(&self.host) .map_err(|source| RunMaterializeError::InvalidTarget { source })?; let target_repo = validated_target.repository().clone(); let mut exact_target = validated_target.into_target(); let workflow_source = input .workflow_source - .map(GitRunTarget::validate) + .map(|source| source.validate(&self.host)) .transpose() .map_err(|source| RunMaterializeError::InvalidWorkflowSource { source })?; // A workflow source naming the target's exact coordinate shares its diff --git a/lib/apps/fabro-server/src/demo/mod.rs b/lib/apps/fabro-server/src/demo/mod.rs index 74db61c57..b445dbf2a 100644 --- a/lib/apps/fabro-server/src/demo/mod.rs +++ b/lib/apps/fabro-server/src/demo/mod.rs @@ -1189,6 +1189,7 @@ mod runs { }, automation: None, repository: Some(RepositoryRef::from_origin_and_source( + &fabro_types::GitHubHost::default(), repo_origin_url, source_directory.as_deref(), )), diff --git a/lib/apps/fabro-server/src/git_checkout.rs b/lib/apps/fabro-server/src/git_checkout.rs index 7ad32dc8a..31c4d9b6a 100644 --- a/lib/apps/fabro-server/src/git_checkout.rs +++ b/lib/apps/fabro-server/src/git_checkout.rs @@ -264,8 +264,11 @@ async fn bare_clone_may_be_corrupt(bare_dir: &Path) -> bool { } } -pub(crate) fn github_clone_url(repo: &GitHubRepositorySlug) -> String { - let mut url = repo.https_url(); +pub(crate) fn github_clone_url( + repo: &GitHubRepositorySlug, + host: &fabro_types::GitHubHost, +) -> String { + let mut url = repo.https_url(host); url.push_str(".git"); url } @@ -740,11 +743,14 @@ mod tests { assert_eq!(repo.owner(), "fabro-sh"); assert_eq!(repo.repo(), "fabro"); assert_eq!( - github_clone_url(&repo), + github_clone_url(&repo, &fabro_types::GitHubHost::default()), "https://github.com/fabro-sh/fabro.git" ); - assert_eq!(repo.https_url(), "https://github.com/fabro-sh/fabro"); - assert!(!github_clone_url(&repo).contains('@')); + assert_eq!( + repo.https_url(&fabro_types::GitHubHost::default()), + "https://github.com/fabro-sh/fabro" + ); + assert!(!github_clone_url(&repo, &fabro_types::GitHubHost::default()).contains('@')); } #[test] @@ -753,13 +759,16 @@ mod tests { assert_eq!(repo.owner(), "owner"); assert_eq!(repo.repo(), ".github"); - assert_eq!(repo.https_url(), "https://github.com/owner/.github"); + assert_eq!( + repo.https_url(&fabro_types::GitHubHost::default()), + "https://github.com/owner/.github" + ); } #[test] fn bare_cache_command_plans_use_argv_prompt_disable_and_timeouts() { let repo = repository_slug("fabro-sh/fabro"); - let clone_url = github_clone_url(&repo); + let clone_url = github_clone_url(&repo, &fabro_types::GitHubHost::default()); let temp = TempDir::new().unwrap(); let bare_dir = temp.path().join("fabro-sh/fabro.git"); let worktree_dir = temp.path().join("worktree/repo"); @@ -847,7 +856,7 @@ mod tests { #[test] fn credential_config_env_keeps_clone_url_uncredentialed() { let repo = repository_slug("fabro-sh/fabro"); - let clone_url = github_clone_url(&repo); + let clone_url = github_clone_url(&repo, &fabro_types::GitHubHost::default()); let auth = GitAuthConfig::from_parts("x-access-token", "ghu_secret"); let plan = build_bare_clone_plan(&clone_url, Path::new("/tmp/fabro-checkout"), Some(&auth)); diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index 3652fab09..66cec5868 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -442,7 +442,7 @@ async fn build_preflight_report( check: &ManifestCheck, shape: &WorkflowShape, ) -> Result<(CheckReport, bool)> { - let mut checks = base_preflight_checks(prepared, shape); + let mut checks = base_preflight_checks(prepared, shape, &state.github_host); if check.has_errors() { return Ok(( CheckReport { @@ -539,12 +539,16 @@ async fn build_preflight_report( )) } -fn base_preflight_checks(prepared: &PreparedManifest, shape: &WorkflowShape) -> Vec { +fn base_preflight_checks( + prepared: &PreparedManifest, + shape: &WorkflowShape, + host: &fabro_types::GitHubHost, +) -> Vec { let repo_summary = prepared.git.as_ref().map_or_else( || "unknown".to_string(), |git| { let https = fabro_github::ssh_url_to_https(&git.origin_url); - fabro_github::parse_github_owner_repo(&https).map_or_else( + fabro_github::parse_github_owner_repo(host, &https).map_or_else( |_| git.origin_url.clone(), |(owner, repo)| format!("{owner}/{repo}"), ) @@ -714,6 +718,7 @@ async fn run_repository_access_check( prepared, resolved_run, github_app, + host, |request, github_app| check_git_remote_ref(request, github_app, host), ) .await @@ -725,6 +730,7 @@ async fn run_repository_access_check_with( prepared: &PreparedManifest, resolved_run: &RunNamespace, github_app: Option, + host: &fabro_types::GitHubHost, check_remote_ref: F, ) -> bool where @@ -742,7 +748,7 @@ where }; let origin_url = fabro_github::normalize_repo_origin_url(&git.origin_url); - if let Err(err) = fabro_github::parse_github_owner_repo(&origin_url) { + if let Err(err) = fabro_github::parse_github_owner_repo(host, &origin_url) { checks.push(CheckResult { name: "Repository Access".into(), status: CheckStatus::Error, @@ -958,7 +964,7 @@ async fn run_github_token_check( github_app, host, |access, creds| mint_scoped_github_token(access, creds, host), - probe_github_repository, + |slug, token| probe_github_repository(slug, token, host), ) .await } @@ -1063,6 +1069,7 @@ where // The same validated access value runtime initialization constructs, so // preflight and runtime cannot disagree about the effective set. let access = match fabro_github::GitHubRepositoryAccess::new( + host, Some(origin_url), &integration.additional_repositories, integration.permissions.clone(), @@ -1237,8 +1244,9 @@ async fn mint_scoped_github_token( async fn probe_github_repository( slug: fabro_types::GitHubRepositorySlug, token: ResolvedToken, + host: &fabro_types::GitHubHost, ) -> std::result::Result<(), String> { - let url = slug.https_url(); + let url = slug.https_url(host); probe_with_replication_retry(token.snapshot, || run_probe_ls_remote(&url, &token)).await } @@ -1376,7 +1384,7 @@ async fn mint_github_token( permissions: &HashMap, ) -> Result { let https_url = fabro_github::ssh_url_to_https(origin_url); - let (owner, repo) = fabro_github::parse_github_owner_repo(&https_url)?; + let (owner, repo) = fabro_github::parse_github_owner_repo(host, &https_url)?; let client = fabro_http::http_client()?; let perms_json = serde_json::to_value(permissions)?; creds @@ -1899,6 +1907,7 @@ provider = "local" &prepared, &resolved, None, + &fabro_types::GitHubHost::default(), move |request, _github_app| { calls_for_check.lock().unwrap().push(request); async { Ok(()) } @@ -1928,6 +1937,7 @@ provider = "local" &prepared, &resolved, None, + &fabro_types::GitHubHost::default(), move |request, _github_app| { calls_for_check.lock().unwrap().push(request); async { Ok(()) } @@ -1969,6 +1979,7 @@ provider = "local" &prepared, &resolved, None, + &fabro_types::GitHubHost::default(), move |request, _github_app| { calls_for_check.lock().unwrap().push(request); async { Ok(()) } @@ -2001,6 +2012,7 @@ provider = "local" &prepared, &resolved, None, + &fabro_types::GitHubHost::default(), |_request, _github_app| async { Err("remote branch not found".to_string()) }, ) .await; diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index 58db78bed..88bf7098a 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -2574,7 +2574,17 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result host, + None => fabro_github::github_host( + &resolved_settings.server_settings.server.integrations.github, + )?, + }; + let petri_projector = Projector::with_github_host( + db_pool.clone(), + store.run_summary_store().pool(), + github_host.clone(), + ); { let projector = Arc::clone(&petri_projector); store.set_platform_record_hook(Arc::new(move |run_id| projector.signal(run_id))); @@ -2643,10 +2653,6 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result host, - None => fabro_github::github_host(¤t_server_settings.server.integrations.github)?, - }; let storage_root = PathBuf::from(¤t_server_settings.server.storage.root); let automation_repo_cache = Arc::new(GitRepoCache::new( Storage::new(&storage_root) diff --git a/lib/apps/fabro-server/src/server/handler/pull_requests.rs b/lib/apps/fabro-server/src/server/handler/pull_requests.rs index e665964c5..3be488f1f 100644 --- a/lib/apps/fabro-server/src/server/handler/pull_requests.rs +++ b/lib/apps/fabro-server/src/server/handler/pull_requests.rs @@ -6,6 +6,7 @@ use axum::http::{HeaderValue, header}; use fabro_store::platform_records::{ PlatformRecord, PullRequestLinkedRecord, PullRequestRequestedRecord, }; +use fabro_types::GitHubHost; use super::super::{ ApiError, AppState, CloseRunPullRequestResponse, CreateRunPullRequestRequest, IntoResponse, @@ -47,15 +48,22 @@ const PULL_REQUEST_CREATION_RETRY_AFTER: Duration = Duration::from_secs(1); clippy::disallowed_types, reason = "Pull-request API validates public github.com URLs; these raw URLs are not credential-bearing log output." )] -fn parse_github_owner_repo_from_url(url: &str, kind: &str) -> Result<(String, String), ApiError> { +fn parse_github_owner_repo_from_url( + host: &GitHubHost, + url: &str, + kind: &str, +) -> Result<(String, String), ApiError> { let parsed = fabro_http::Url::parse(url) .map_err(|err| ApiError::bad_request(format!("Invalid {kind}: {err}")))?; match parsed.host_str() { - Some("github.com") => {} - Some(host) => { + Some(got) if got == host.hostname() => {} + Some(got) => { return Err(ApiError::with_code( StatusCode::BAD_REQUEST, - format!("Pull request operations support github.com only (got {host})."), + format!( + "Pull request operations support {} only (got {got}).", + host.host() + ), "unsupported_host", )); } @@ -66,19 +74,40 @@ fn parse_github_owner_repo_from_url(url: &str, kind: &str) -> Result<(String, St } } - fabro_github::parse_github_owner_repo(url).map_err(|err| ApiError::bad_request(err.to_string())) + fabro_github::parse_github_owner_repo(host, url) + .map_err(|err| ApiError::bad_request(err.to_string())) } +#[expect( + clippy::disallowed_types, + reason = "Pull-request links are public GitHub URLs; the raw URL is only compared with the configured host." +)] fn pull_request_record_from_link_request( + host: &GitHubHost, body: &LinkRunPullRequestRequest, ) -> Result { - PullRequestLink::from_github_url(body.html_url.trim()).map_err(|err| { + let reject = |err: String| { let code = if err.contains("GitHub pull request URL") { "unsupported_pull_request_provider" } else { "invalid_pull_request_url" }; ApiError::with_code(StatusCode::BAD_REQUEST, err, code) + }; + let raw_url = body.html_url.trim(); + let off_host = fabro_http::Url::parse(raw_url) + .is_ok_and(|url| url.origin().ascii_serialization() != host.web_url()); + if off_host { + return Err(reject(format!( + "Pull request link must be a GitHub pull request URL like {}/owner/repo/pull/123.", + host.web_url() + ))); + } + let link = PullRequestLink::from_github_url(raw_url).map_err(reject)?; + // Store the canonical URL, not the caller's spelling of it. + Ok(PullRequestLink { + html_url: host.pull_request_url(&link.owner, &link.repo, link.number), + ..link }) } @@ -272,7 +301,11 @@ impl<'a> RunPrInputs<'a> { )); } let normalized_origin = fabro_github::normalize_repo_origin_url(origin_url); - parse_github_owner_repo_from_url(&normalized_origin, "repo origin URL")?; + parse_github_owner_repo_from_url( + &state.github_host, + &normalized_origin, + "repo origin URL", + )?; Ok(Self { goal: run_spec.graph.goal(), base_branch, @@ -456,7 +489,7 @@ async fn link_run_pull_request( Json(body): Json, ) -> Response { let _create_guard = state.pull_request_create_locks.lock(id).await; - let pull_request = match pull_request_record_from_link_request(&body) { + let pull_request = match pull_request_record_from_link_request(&state.github_host, &body) { Ok(record) => record, Err(err) => return err.into_response(), }; @@ -612,9 +645,12 @@ async fn close_run_pull_request( #[cfg(test)] mod tests { use axum::http::StatusCode; - use fabro_types::{DirtyStatus, GitContext, StartRecord, blob_ref}; + use fabro_api::types::LinkRunPullRequestRequest; + use fabro_types::{DirtyStatus, GitContext, GitHubHost, StartRecord, blob_ref}; - use super::RunPrInputs; + use super::{ + RunPrInputs, parse_github_owner_repo_from_url, pull_request_record_from_link_request, + }; use crate::test_support::{test_app_state, test_concluded_run_projection}; /// A finished run with everything a pull request needs and `patch` as @@ -679,4 +715,69 @@ mod tests { assert_eq!(error.status(), StatusCode::INTERNAL_SERVER_ERROR); assert_eq!(error.detail(), "The saved final patch is missing."); } + + fn ghes() -> GitHubHost { + GitHubHost::new("https://ghe.example.com", None).unwrap() + } + + fn link_request(html_url: &str) -> LinkRunPullRequestRequest { + serde_json::from_value(serde_json::json!({ "html_url": html_url })).unwrap() + } + + #[test] + fn link_request_accepts_the_configured_host_and_stores_the_canonical_url() { + let link = pull_request_record_from_link_request( + &ghes(), + &link_request("https://GHE.example.com/acme/widgets/pull/7/"), + ) + .unwrap(); + assert_eq!(link.html_url, "https://ghe.example.com/acme/widgets/pull/7"); + assert_eq!( + (link.owner.as_str(), link.repo.as_str(), link.number), + ("acme", "widgets", 7) + ); + + let default = pull_request_record_from_link_request( + &GitHubHost::default(), + &link_request("https://github.com/acme/widgets/pull/7"), + ) + .unwrap(); + assert_eq!(default.html_url, "https://github.com/acme/widgets/pull/7"); + } + + #[test] + fn link_request_rejects_other_hosts_naming_the_configured_one() { + let error = pull_request_record_from_link_request( + &ghes(), + &link_request("https://github.com/acme/widgets/pull/7"), + ) + .unwrap_err(); + assert_eq!(error.status(), StatusCode::BAD_REQUEST); + assert!( + error + .detail() + .contains("https://ghe.example.com/owner/repo/pull/123"), + "{}", + error.detail() + ); + } + + #[test] + fn repo_origin_must_be_on_the_configured_host() { + let (owner, repo) = parse_github_owner_repo_from_url( + &ghes(), + "https://ghe.example.com/acme/widgets", + "origin", + ) + .unwrap(); + assert_eq!((owner.as_str(), repo.as_str()), ("acme", "widgets")); + + let error = + parse_github_owner_repo_from_url(&ghes(), "https://github.com/acme/widgets", "origin") + .unwrap_err(); + assert_eq!( + error.detail(), + "Pull request operations support ghe.example.com only (got github.com)." + ); + } } diff --git a/lib/apps/fabro-server/src/server/handler/runs.rs b/lib/apps/fabro-server/src/server/handler/runs.rs index 696c8af1e..ecae95f84 100644 --- a/lib/apps/fabro-server/src/server/handler/runs.rs +++ b/lib/apps/fabro-server/src/server/handler/runs.rs @@ -556,7 +556,7 @@ pub(crate) async fn create_run_from_intent( let explicit_title_supplied = intent.title.is_some(); // Validate the pure, in-memory request facts before paying for // blob-store reads and closure lowering. - let ValidatedRunTarget { target, git } = match intent.target.validate() { + let ValidatedRunTarget { target, git } = match intent.target.validate(&state.github_host) { Ok(validated) => validated, Err(error) => return run_intent_admission_error(error.into()), }; diff --git a/lib/apps/fabro-server/src/server/petri_runs.rs b/lib/apps/fabro-server/src/server/petri_runs.rs index 7a09f4743..c5a036baf 100644 --- a/lib/apps/fabro-server/src/server/petri_runs.rs +++ b/lib/apps/fabro-server/src/server/petri_runs.rs @@ -474,6 +474,7 @@ pub(crate) async fn execute(state: Arc, run_id: RunId) { // The in-process path serves the server's tests: a Git target is // fetched without a credential, and nothing is published. let source = RunSource::for_run( + &state.github_host, run_state.spec.target.as_ref(), &run_state.spec.settings.run, None, diff --git a/lib/components/fabro-automation/migrations/2026071101_file_definitions_to_sqlite.rs b/lib/components/fabro-automation/migrations/2026071101_file_definitions_to_sqlite.rs index 224506fd1..40d2597bf 100644 --- a/lib/components/fabro-automation/migrations/2026071101_file_definitions_to_sqlite.rs +++ b/lib/components/fabro-automation/migrations/2026071101_file_definitions_to_sqlite.rs @@ -142,8 +142,7 @@ fn legacy_target( tag, sha, }) - .validate() - .map(|validated| validated.target) + .canonical() .map_err(|source| AutomationStoreError::LegacyTarget { path: path.to_path_buf(), source, diff --git a/lib/components/fabro-automation/src/model.rs b/lib/components/fabro-automation/src/model.rs index 6df1e8e0c..2f67588d2 100644 --- a/lib/components/fabro-automation/src/model.rs +++ b/lib/components/fabro-automation/src/model.rs @@ -166,8 +166,7 @@ pub fn validate_workflow_source( source: AutomationGitWorkflowSource, ) -> Result { source - .validate() - .map(fabro_types::ValidatedGitRunTarget::into_target) + .canonical() .map_err(|source| AutomationValidationError::InvalidWorkflowSource { source }) } @@ -404,8 +403,7 @@ fn validate_target(target: RunTarget) -> Result, additional_repositories: &BTreeSet, permissions: HashMap, @@ -64,7 +65,7 @@ impl GitHubRepositoryAccess { }; let normalized = crate::normalize_repo_origin_url(origin_url); - let (owner, repo) = crate::parse_github_owner_repo(&normalized) + let (owner, repo) = crate::parse_github_owner_repo(host, &normalized) .context("parsing GitHub origin for repository access")?; let Some(primary) = GitHubRepositorySlug::try_new(&format!("{owner}/{repo}")) else { bail!("run origin does not name a valid GitHub `owner/repository`: {owner}/{repo}"); @@ -257,7 +258,12 @@ mod tests { additional: &[&str], permissions: HashMap, ) -> anyhow::Result> { - GitHubRepositoryAccess::new(Some(origin), &slugs(additional), permissions) + GitHubRepositoryAccess::new( + &GitHubHost::default(), + Some(origin), + &slugs(additional), + permissions, + ) } #[test] @@ -278,19 +284,34 @@ mod tests { #[test] fn no_origin_and_no_additional_repositories_is_none() { - let access = GitHubRepositoryAccess::new(None, &BTreeSet::new(), HashMap::new()).unwrap(); + let access = GitHubRepositoryAccess::new( + &GitHubHost::default(), + None, + &BTreeSet::new(), + HashMap::new(), + ) + .unwrap(); assert!(access.is_none()); - let blank = - GitHubRepositoryAccess::new(Some(" "), &BTreeSet::new(), HashMap::new()).unwrap(); + let blank = GitHubRepositoryAccess::new( + &GitHubHost::default(), + Some(" "), + &BTreeSet::new(), + HashMap::new(), + ) + .unwrap(); assert!(blank.is_none()); } #[test] fn additional_repositories_require_an_origin() { - let err = - GitHubRepositoryAccess::new(None, &slugs(&["fabro-sh/keystone"]), contents_read()) - .unwrap_err(); + let err = GitHubRepositoryAccess::new( + &GitHubHost::default(), + None, + &slugs(&["fabro-sh/keystone"]), + contents_read(), + ) + .unwrap_err(); assert!( err.to_string().contains("requires a GitHub run origin"), "{err:#}" diff --git a/lib/components/fabro-github/src/lib.rs b/lib/components/fabro-github/src/lib.rs index 83f116945..120b33a0e 100644 --- a/lib/components/fabro-github/src/lib.rs +++ b/lib/components/fabro-github/src/lib.rs @@ -54,13 +54,18 @@ pub fn apply_probe_git_env(command: &mut Command, token: &str) { /// Resolves the GitHub deployment from settings, letting `GITHUB_WEB_URL` /// override the web origin and `GITHUB_BASE_URL` override the REST API base. +pub fn github_host(settings: &GithubIntegrationSettings) -> anyhow::Result { + with_env_overrides(&settings.host) +} + +/// [`github_host`] for a caller that resolved only the configured host. #[expect( clippy::disallowed_methods, reason = "GitHub API client exposes documented process-env web origin and API base overrides." )] -pub fn github_host(settings: &GithubIntegrationSettings) -> anyhow::Result { +pub fn with_env_overrides(configured: &GitHubHost) -> anyhow::Result { host_with_overrides( - &settings.host, + configured, std::env::var(EnvVars::GITHUB_WEB_URL).ok().as_deref(), std::env::var(EnvVars::GITHUB_BASE_URL).ok().as_deref(), ) @@ -112,6 +117,12 @@ impl<'a> GitHubContext<'a> { } } + /// The GitHub deployment this context talks to. + #[must_use] + pub fn host(&self) -> &'a GitHubHost { + self.host + } + fn http_client(&self) -> anyhow::Result { self.http_client.clone().map_or_else(http_client, Ok) } @@ -442,14 +453,14 @@ impl HttpClient for fabro_http::HttpClient { } } -/// Parse `owner` and `repo` from a GitHub HTTPS URL. +/// Parse `owner` and `repo` from an HTTPS URL on `host`. /// -/// Accepts URLs like: +/// Accepts URLs like (shown for github.com): /// - `https://github.com/owner/repo.git` /// - `https://github.com/owner/repo` /// - `https://github.com/owner/repo/` /// - `https://x-access-token:TOKEN@github.com/owner/repo.git` -pub fn parse_github_owner_repo(url: &str) -> anyhow::Result<(String, String)> { +pub fn parse_github_owner_repo(host: &GitHubHost, url: &str) -> anyhow::Result<(String, String)> { // Strip credentials from URLs like https://x-access-token:TOKEN@github.com/... let stripped = url.strip_prefix("https://").and_then(|rest| { rest.split_once('@') @@ -458,8 +469,15 @@ pub fn parse_github_owner_repo(url: &str) -> anyhow::Result<(String, String)> { let url = stripped.as_deref().unwrap_or(url); let display_url = redacted_url_for_error(url); let path = url - .strip_prefix("https://github.com/") - .ok_or_else(|| anyhow!("Not a GitHub HTTPS URL: {display_url}"))?; + .strip_prefix(host.web_url()) + .and_then(|rest| rest.strip_prefix('/')) + .ok_or_else(|| { + if host.is_github_com() { + anyhow!("Not a GitHub HTTPS URL: {display_url}") + } else { + anyhow!("Not a GitHub HTTPS URL for {}: {display_url}", host.host()) + } + })?; let path = path.trim_end_matches('/'); let path = path.strip_suffix(".git").unwrap_or(path); @@ -1445,7 +1463,7 @@ pub async fn resolve_authenticated_url( ctx: &GitHubContext<'_>, url: &str, ) -> anyhow::Result { - let (owner, repo) = parse_github_owner_repo(url)?; + let (owner, repo) = parse_github_owner_repo(ctx.host, url)?; let credentials = resolve_clone_credentials(ctx, &owner, &repo).await?; embed_token_in_url(url, credentials.password()) } @@ -1818,28 +1836,38 @@ mod tests { #[test] fn parse_https_with_git_suffix() { - let (owner, repo) = parse_github_owner_repo("https://github.com/owner/repo.git").unwrap(); + let (owner, repo) = + parse_github_owner_repo(&GitHubHost::default(), "https://github.com/owner/repo.git") + .unwrap(); assert_eq!(owner, "owner"); assert_eq!(repo, "repo"); } #[test] fn parse_https_without_git_suffix() { - let (owner, repo) = parse_github_owner_repo("https://github.com/owner/repo").unwrap(); + let (owner, repo) = + parse_github_owner_repo(&GitHubHost::default(), "https://github.com/owner/repo") + .unwrap(); assert_eq!(owner, "owner"); assert_eq!(repo, "repo"); } #[test] fn parse_https_with_trailing_slash() { - let (owner, repo) = parse_github_owner_repo("https://github.com/owner/repo/").unwrap(); + let (owner, repo) = + parse_github_owner_repo(&GitHubHost::default(), "https://github.com/owner/repo/") + .unwrap(); assert_eq!(owner, "owner"); assert_eq!(repo, "repo"); } #[test] fn parse_rejects_extra_path_components() { - let error = parse_github_owner_repo("https://github.com/owner/repo/issues").unwrap_err(); + let error = parse_github_owner_repo( + &GitHubHost::default(), + "https://github.com/owner/repo/issues", + ) + .unwrap_err(); assert!( error.to_string().contains("must identify one repository"), @@ -1899,9 +1927,76 @@ mod tests { ); } + #[test] + fn parse_owner_repo_accepts_https_and_ssh_on_github_com() { + let host = GitHubHost::default(); + for url in [ + "https://github.com/acme/widgets", + "https://github.com/acme/widgets.git", + "git@github.com:acme/widgets.git", + "ssh://git@github.com/acme/widgets.git", + ] { + let normalized = normalize_repo_origin_url(url); + assert_eq!( + parse_github_owner_repo(&host, &normalized).unwrap(), + ("acme".to_string(), "widgets".to_string()), + "{url}" + ); + } + } + + #[test] + fn parse_owner_repo_accepts_https_and_ssh_on_enterprise_hosts() { + for web_url in ["https://ghe.example.com", "https://octocorp.ghe.com"] { + let host = GitHubHost::new(web_url, None).unwrap(); + let name = host.host(); + for url in [ + format!("{web_url}/acme/widgets"), + format!("{web_url}/acme/widgets.git"), + format!("https://x-access-token:ghs_abc@{name}/acme/widgets.git"), + format!("git@{name}:acme/widgets.git"), + format!("ssh://git@{name}/acme/widgets.git"), + ] { + let normalized = normalize_repo_origin_url(&url); + assert_eq!( + parse_github_owner_repo(&host, &normalized).unwrap(), + ("acme".to_string(), "widgets".to_string()), + "{url}" + ); + } + } + } + + #[test] + fn parse_owner_repo_rejects_foreign_hosts_naming_the_expected_one() { + let ghes = ghes(); + let error = parse_github_owner_repo(&ghes, "https://github.com/acme/widgets").unwrap_err(); + assert_eq!( + error.to_string(), + "Not a GitHub HTTPS URL for ghe.example.com: https://github.com/acme/widgets" + ); + let error = parse_github_owner_repo(&ghes, "https://gitlab.com/acme/widgets").unwrap_err(); + assert!( + error + .to_string() + .starts_with("Not a GitHub HTTPS URL for ghe.example.com: "), + "{error}" + ); + let error = parse_github_owner_repo( + &GitHubHost::default(), + "https://ghe.example.com/acme/widgets", + ) + .unwrap_err(); + assert_eq!( + error.to_string(), + "Not a GitHub HTTPS URL: https://ghe.example.com/acme/widgets" + ); + } + #[test] fn parse_github_url_with_credentials() { let (owner, repo) = parse_github_owner_repo( + &GitHubHost::default(), "https://x-access-token:ghs_abc123@github.com/acme/widgets.git", ) .unwrap(); @@ -1911,15 +2006,21 @@ mod tests { #[test] fn parse_github_url_with_credentials_no_password() { - let (owner, repo) = - parse_github_owner_repo("https://token@github.com/acme/widgets.git").unwrap(); + let (owner, repo) = parse_github_owner_repo( + &GitHubHost::default(), + "https://token@github.com/acme/widgets.git", + ) + .unwrap(); assert_eq!(owner, "acme"); assert_eq!(repo, "widgets"); } #[test] fn parse_credentials_non_github_still_errors() { - let result = parse_github_owner_repo("https://user:pass@gitlab.com/owner/repo"); + let result = parse_github_owner_repo( + &GitHubHost::default(), + "https://user:pass@gitlab.com/owner/repo", + ); assert!(result.is_err()); assert!( result @@ -2003,7 +2104,8 @@ mod tests { #[test] fn parse_non_github_url_errors() { - let result = parse_github_owner_repo("https://gitlab.com/owner/repo"); + let result = + parse_github_owner_repo(&GitHubHost::default(), "https://gitlab.com/owner/repo"); assert!(result.is_err()); assert!( result @@ -2015,14 +2117,14 @@ mod tests { #[test] fn parse_missing_repo_errors() { - let result = parse_github_owner_repo("https://github.com/owner"); + let result = parse_github_owner_repo(&GitHubHost::default(), "https://github.com/owner"); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("Missing repo")); } #[test] fn parse_empty_string_errors() { - let result = parse_github_owner_repo(""); + let result = parse_github_owner_repo(&GitHubHost::default(), ""); assert!(result.is_err()); } @@ -2141,6 +2243,7 @@ mod tests { #[tokio::test] async fn multi_repository_mint_lists_every_repository_name_once() { let access = GitHubRepositoryAccess::new( + &GitHubHost::default(), Some("git@github.com:owner/repo.git"), &[ "owner/keystone".parse().unwrap(), diff --git a/lib/components/fabro-github/src/token_source.rs b/lib/components/fabro-github/src/token_source.rs index a4765568b..81ef1bd86 100644 --- a/lib/components/fabro-github/src/token_source.rs +++ b/lib/components/fabro-github/src/token_source.rs @@ -269,7 +269,7 @@ impl InstallationTokenSource { permissions: serde_json::Value, ) -> anyhow::Result> { let normalized = crate::normalize_repo_origin_url(origin_url); - let (owner, repo) = crate::parse_github_owner_repo(&normalized) + let (owner, repo) = crate::parse_github_owner_repo(host, &normalized) .context("parsing GitHub origin for token source")?; Self::for_repository(creds, host, owner, repo, permissions) } @@ -580,6 +580,7 @@ mod tests { #[tokio::test] async fn for_access_source_resolves_like_the_single_repository_source() { let access = crate::GitHubRepositoryAccess::new( + &GitHubHost::default(), Some("https://github.com/owner/repo.git"), &["owner/keystone".parse().unwrap()].into_iter().collect(), std::collections::HashMap::from([("contents".to_string(), "read".to_string())]), diff --git a/lib/components/fabro-github/tests/live_access.rs b/lib/components/fabro-github/tests/live_access.rs index 62c539135..7dfc038b0 100644 --- a/lib/components/fabro-github/tests/live_access.rs +++ b/lib/components/fabro-github/tests/live_access.rs @@ -34,7 +34,7 @@ fn env_var(name: &str) -> String { } async fn ls_remote_with_token(slug: &GitHubRepositorySlug, token: &str) -> bool { - let url = slug.https_url(); + let url = slug.https_url(&GitHubHost::default()); let mut command = Command::new("git"); fabro_github::apply_probe_git_env(&mut command, token); let output = command @@ -65,6 +65,7 @@ async fn scoped_token_reaches_the_declared_additional_repository() { let additional_set: BTreeSet = [additional.clone()].into_iter().collect(); let access = GitHubRepositoryAccess::new( + &GitHubHost::default(), Some(&origin), &additional_set, std::collections::HashMap::from([("contents".to_string(), "read".to_string())]), @@ -102,6 +103,7 @@ async fn scoped_token_reaches_the_declared_additional_repository() { // read the additional repository (proves server-side scoping, not just // possession of a token). let primary_only = GitHubRepositoryAccess::new( + &GitHubHost::default(), Some(&origin), &BTreeSet::new(), std::collections::HashMap::from([("contents".to_string(), "read".to_string())]), diff --git a/lib/components/fabro-manifest/src/lib.rs b/lib/components/fabro-manifest/src/lib.rs index 343d87275..be4f12c3f 100644 --- a/lib/components/fabro-manifest/src/lib.rs +++ b/lib/components/fabro-manifest/src/lib.rs @@ -31,8 +31,8 @@ use fabro_template::validate_static_reference; use fabro_types::settings::interp::InterpString; use fabro_types::settings::run::{ApprovalMode, ResolvedGoalSource, ResolvedRunGoal, RunMode}; use fabro_types::{ - DirtyStatus, GitContext, GitHubRepositorySlug, GitRunTarget, ManifestPath, ReferenceKind, - RunTarget, SandboxProviderKind, WorkflowSettings, + DirtyStatus, GitContext, GitHubHost, GitHubRepositorySlug, GitRunTarget, ManifestPath, + ReferenceKind, RunTarget, SandboxProviderKind, WorkflowSettings, }; use fabro_workflow::git::{self, GitSyncStatus}; pub use fabro_workflow_version::CollectedWorkflowClosure; @@ -60,6 +60,9 @@ pub struct ManifestBuildInput { /// Path to the user settings file (for inclusion in /// `RunManifest.configs`). `None` skips the user config entry. pub user_settings_path: Option, + /// The GitHub deployment `run.scm` repositories and git origins are + /// resolved against; github.com unless the caller configures another. + pub github_host: GitHubHost, } #[derive(Debug)] @@ -223,7 +226,8 @@ pub fn build_run_manifest(input: ManifestBuildInput) -> Result { &working_directory, )?; - let configured_repo_origin_url = configured_repo_origin_url(&workflow_settings); + let configured_repo_origin_url = + configured_repo_origin_url(&input.github_host, &workflow_settings); let git = build_legacy_git_context(&working_directory, configured_repo_origin_url.as_deref()); let args = input.args.filter(|args| !manifest_args_is_empty(args)); @@ -357,13 +361,17 @@ pub enum ExactCommitStatus { /// branch so clone-based execution can resolve the observed commit. #[must_use] pub fn observe_git_run_target( + host: &GitHubHost, repo_path: &Path, configured_repo_origin_url: Option<&str>, ) -> Option { let local = inspect_local_git(repo_path, configured_repo_origin_url)?; let legacy_git_context = local.legacy_git_context; - let mut run_target = - github_run_target(&legacy_git_context.origin_url, &legacy_git_context.branch); + let mut run_target = github_run_target( + host, + &legacy_git_context.origin_url, + &legacy_git_context.branch, + ); let mut exact_commit = ExactCommitStatus::Unpublished; if let Some(target) = run_target.as_mut() { let publish_status = publish_manifest_branch_best_effort( @@ -459,6 +467,7 @@ pub enum RunTargetDerivationError { /// Returns the reason a clone-based target could not be derived; every /// variant's message is written for the CLI caller. pub fn derive_run_target_for_provider( + host: &GitHubHost, provider: &SandboxProviderKind, canonical_cwd: &Path, configured_repo_origin_url: Option<&str>, @@ -476,7 +485,7 @@ pub fn derive_run_target_for_provider( dirty_worktree: false, }); } - let Some(observation) = observe_git_run_target(canonical_cwd, configured_repo_origin_url) + let Some(observation) = observe_git_run_target(host, canonical_cwd, configured_repo_origin_url) else { return Ok(DerivedRunTarget { target: none_target_for_unversioned_directory(canonical_cwd)?, @@ -546,6 +555,7 @@ fn none_target_for_unversioned_directory( /// /// Returns an error when either config exists but cannot be read or parsed. pub fn configured_repo_origin_url_for_location( + host: &GitHubHost, location: &WorkflowLocation, ) -> Result> { let workflow = location @@ -565,7 +575,7 @@ pub fn configured_repo_origin_url_for_location( repository: workflow.repository.or(project.repository), github: workflow.github.or(project.github), }; - Ok(configured_repo_origin_url_from_scm_layer(&scm)) + Ok(configured_repo_origin_url_from_scm_layer(host, &scm)) } struct LocalGitObservation { @@ -623,8 +633,8 @@ fn build_legacy_git_context( Some(local.legacy_git_context) } -fn github_run_target(origin_url: &str, branch: &str) -> Option { - let (owner, repository) = fabro_github::parse_github_owner_repo(origin_url).ok()?; +fn github_run_target(host: &GitHubHost, origin_url: &str, branch: &str) -> Option { + let (owner, repository) = fabro_github::parse_github_owner_repo(host, origin_url).ok()?; let slug = GitHubRepositorySlug::try_new(&format!("{owner}/{repository}"))?; let validated = RunTarget::Git(GitRunTarget { repo: slug.to_string(), @@ -632,7 +642,7 @@ fn github_run_target(origin_url: &str, branch: &str) -> Option { tag: None, sha: None, }) - .validate() + .validate(host) .ok()?; let RunTarget::Git(target) = validated.target else { unreachable!("a validated Git target must remain a Git target") @@ -640,9 +650,10 @@ fn github_run_target(origin_url: &str, branch: &str) -> Option { Some(target) } -fn configured_repo_origin_url(settings: &WorkflowSettings) -> Option { +fn configured_repo_origin_url(host: &GitHubHost, settings: &WorkflowSettings) -> Option { let scm = &settings.run.scm; configured_repo_origin_url_from_scm( + host, scm.provider.as_deref(), scm.owner.as_deref(), scm.repository.as_deref(), @@ -650,6 +661,7 @@ fn configured_repo_origin_url(settings: &WorkflowSettings) -> Option { } fn configured_repo_origin_url_from_scm( + host: &GitHubHost, provider: Option<&str>, owner: Option<&str>, repository: Option<&str>, @@ -662,13 +674,17 @@ fn configured_repo_origin_url_from_scm( if owner.trim().is_empty() || repository.trim().is_empty() { return None; } - let origin = format!("https://github.com/{owner}/{repository}"); + let origin = host.repository_url(owner, repository); let normalized = fabro_github::normalize_repo_origin_url(&origin); (!normalized.is_empty()).then_some(normalized) } -fn configured_repo_origin_url_from_scm_layer(scm: &RunScmLayer) -> Option { +fn configured_repo_origin_url_from_scm_layer( + host: &GitHubHost, + scm: &RunScmLayer, +) -> Option { configured_repo_origin_url_from_scm( + host, scm.provider.as_deref(), scm.owner.as_deref(), scm.repository.as_deref(), @@ -865,6 +881,36 @@ pub(crate) mod test_fixtures { #[cfg(test)] mod tests { + #[test] + fn scm_origin_and_run_target_follow_the_configured_host() { + let ghes = GitHubHost::new("https://ghe.example.com", None).unwrap(); + assert_eq!( + configured_repo_origin_url_from_scm( + &ghes, + Some("github"), + Some("acme"), + Some("widgets") + ) + .as_deref(), + Some("https://ghe.example.com/acme/widgets") + ); + assert_eq!( + configured_repo_origin_url_from_scm( + &GitHubHost::default(), + None, + Some("acme"), + Some("widgets") + ) + .as_deref(), + Some("https://github.com/acme/widgets") + ); + + let target = + github_run_target(&ghes, "https://ghe.example.com/acme/widgets", "main").unwrap(); + assert_eq!(target.repo, "acme/widgets"); + assert!(github_run_target(&ghes, "https://github.com/acme/widgets", "main").is_none()); + } + use fabro_workflow::git::head_sha; use super::*; @@ -2050,8 +2096,12 @@ exit 1 ]); run_git(&workspace, &["push", "origin", "feature"]); - let observation = - observe_git_run_target(&workspace, Some("https://github.com/acme/widgets")).unwrap(); + let observation = observe_git_run_target( + &GitHubHost::default(), + &workspace, + Some("https://github.com/acme/widgets"), + ) + .unwrap(); let target = observation.run_target.as_ref().unwrap(); let legacy = &observation.legacy_git_context; @@ -2077,8 +2127,12 @@ exit 1 "https://github.com/acme/widgets", ]); - let observation = - observe_git_run_target(&workspace, Some("https://github.com/acme/widgets")).unwrap(); + let observation = observe_git_run_target( + &GitHubHost::default(), + &workspace, + Some("https://github.com/acme/widgets"), + ) + .unwrap(); let target = observation.run_target.as_ref().unwrap(); assert_eq!(target.sha, observation.legacy_git_context.sha); @@ -2112,8 +2166,12 @@ exit 1 ]); mark_origin_branch_synced(&workspace, "feature"); - let observation = - observe_git_run_target(&workspace, Some("https://github.com/acme/widgets")).unwrap(); + let observation = observe_git_run_target( + &GitHubHost::default(), + &workspace, + Some("https://github.com/acme/widgets"), + ) + .unwrap(); let target = observation.run_target.as_ref().unwrap(); assert_eq!(target.sha, None); @@ -2138,7 +2196,8 @@ exit 1 std::fs::create_dir_all(&workspace).unwrap(); init_git_repo(&workspace, branch, "https://github.com/acme/widgets"); - let observation = observe_git_run_target(&workspace, None).unwrap(); + let observation = + observe_git_run_target(&GitHubHost::default(), &workspace, None).unwrap(); assert_eq!(observation.run_target, None, "branch {branch}"); assert_eq!(observation.legacy_git_context.branch, branch); @@ -2162,9 +2221,12 @@ exit 1 "https://github.com/acme/widgets", ]); - let failed = - observe_git_run_target(&failed_workspace, Some("https://github.com/acme/widgets")) - .unwrap(); + let failed = observe_git_run_target( + &GitHubHost::default(), + &failed_workspace, + Some("https://github.com/acme/widgets"), + ) + .unwrap(); assert_eq!(failed.run_target.as_ref().unwrap().sha, None); assert!(failed.legacy_git_context.sha.is_some()); assert_eq!(failed.legacy_git_context.dirty, DirtyStatus::Clean); @@ -2179,6 +2241,7 @@ exit 1 ); let mismatched = observe_git_run_target( + &GitHubHost::default(), &mismatched_workspace, Some("https://github.com/acme/configured"), ) @@ -2198,7 +2261,7 @@ exit 1 init_git_repo(&workspace, "feature", bare_origin.to_str().unwrap()); std::fs::write(workspace.join("dirty.txt"), "dirty").unwrap(); - let unsupported = observe_git_run_target(&workspace, None).unwrap(); + let unsupported = observe_git_run_target(&GitHubHost::default(), &workspace, None).unwrap(); assert!(unsupported.run_target.is_none()); assert_eq!(unsupported.legacy_git_context.dirty, DirtyStatus::Dirty); assert_eq!( @@ -2208,15 +2271,15 @@ exit 1 let not_repo = temp.path().join("not-repo"); std::fs::create_dir_all(¬_repo).unwrap(); - assert!(observe_git_run_target(¬_repo, None).is_none()); + assert!(observe_git_run_target(&GitHubHost::default(), ¬_repo, None).is_none()); let unborn = temp.path().join("unborn"); std::fs::create_dir_all(&unborn).unwrap(); run_git(&unborn, &["init", "--quiet"]); - assert!(observe_git_run_target(&unborn, None).is_none()); + assert!(observe_git_run_target(&GitHubHost::default(), &unborn, None).is_none()); run_git(&workspace, &["checkout", "--detach", "--quiet"]); - assert!(observe_git_run_target(&workspace, None).is_none()); + assert!(observe_git_run_target(&GitHubHost::default(), &workspace, None).is_none()); } fn init_git_repo(path: &Path, branch: &str, origin_url: &str) { diff --git a/lib/components/fabro-petri/src/projection/platform.rs b/lib/components/fabro-petri/src/projection/platform.rs index eb0519d8a..a6c2d0fe5 100644 --- a/lib/components/fabro-petri/src/projection/platform.rs +++ b/lib/components/fabro-petri/src/projection/platform.rs @@ -148,9 +148,10 @@ impl RunView { } PlatformRecord::PullRequestCreated(record) => { let link = PullRequestLink { - owner: record.owner.clone(), - repo: record.repo.clone(), - number: record.number, + owner: record.owner.clone(), + repo: record.repo.clone(), + number: record.number, + html_url: record.html_url.clone(), }; projection.pull_request = Some(link.clone()); if let Some(creation) = projection diff --git a/lib/components/fabro-petri/src/projector/mod.rs b/lib/components/fabro-petri/src/projector/mod.rs index cada3395c..de1f949a9 100644 --- a/lib/components/fabro-petri/src/projector/mod.rs +++ b/lib/components/fabro-petri/src/projector/mod.rs @@ -67,7 +67,7 @@ use std::time::Duration; use fabro_db::DbPool; use fabro_store::platform_records::{PlatformRecordStore, now_ms}; use fabro_store::{RunProjection, RunSummaryStore}; -use fabro_types::{RunId, RunStreamItem}; +use fabro_types::{GitHubHost, RunId, RunStreamItem}; use fabro_util::error::collect_chain; use fabro_util::sync; use petri_execution::events::{EventId, EventSource, RunEvent}; @@ -236,6 +236,8 @@ pub struct Projector { /// stream grew. A wake-up for the stream's readers, never a source of /// facts; a reader that lags re-reads from its cursor. committed: broadcast::Sender, + /// The GitHub deployment run origins are classified against. + github_host: GitHubHost, } impl std::fmt::Debug for Projector { @@ -250,7 +252,14 @@ impl Projector { /// passes its one pool twice. #[must_use] pub fn new(records: DbPool, views: DbPool) -> Arc { + Self::with_github_host(records, views, GitHubHost::default()) + } + + /// [`Projector::new`] classifying run origins against `github_host`. + #[must_use] + pub fn with_github_host(records: DbPool, views: DbPool, github_host: GitHubHost) -> Arc { Arc::new(Self { + github_host, store: SqliteRunStore::new(records.clone()), platform: PlatformRecordStore::new(views.clone()), records, @@ -725,9 +734,14 @@ impl Projector { .map_err(ProjectError::Database)?; } if let Some(projection) = view.projection.as_ref() { - RunSummaryStore::write_petri_run_row_on_connection(&mut tx, &run_id, projection) - .await - .map_err(ProjectError::Store)?; + RunSummaryStore::write_petri_run_row_on_connection( + &mut tx, + &self.github_host, + &run_id, + projection, + ) + .await + .map_err(ProjectError::Store)?; } tx.commit().await.map_err(ProjectError::Database)?; Ok(true) diff --git a/lib/components/fabro-petri/src/source.rs b/lib/components/fabro-petri/src/source.rs index 14cb48831..3220925e4 100644 --- a/lib/components/fabro-petri/src/source.rs +++ b/lib/components/fabro-petri/src/source.rs @@ -19,7 +19,7 @@ use std::fmt; use std::sync::Arc; use fabro_types::settings::run::{RunCloneSettings, RunMode, RunNamespace}; -use fabro_types::{GitRunTarget, RunTarget}; +use fabro_types::{GitHubHost, GitRunTarget, RunTarget}; /// The revision a run starts from, in the order the target names it: an /// exact commit wins over a tag, and a tag over the branch head. @@ -170,6 +170,7 @@ impl RunSource { /// repository checks nothing out. #[must_use] pub fn for_run( + host: &GitHubHost, target: Option<&RunTarget>, settings: &RunNamespace, credentials: Option>, @@ -180,8 +181,8 @@ impl RunSource { if settings.execution.mode == RunMode::DryRun { return None; } - let validated = target.clone().validate().ok()?; - let origin = validated.repository().https_url(); + let validated = target.clone().validate(host).ok()?; + let origin = validated.repository().https_url(host); Self::for_target(validated.target(), origin, &settings.clone, credentials) } diff --git a/lib/components/fabro-petri/src/stage_credentials.rs b/lib/components/fabro-petri/src/stage_credentials.rs index ab0bacfdb..25dc85b0b 100644 --- a/lib/components/fabro-petri/src/stage_credentials.rs +++ b/lib/components/fabro-petri/src/stage_credentials.rs @@ -57,7 +57,7 @@ impl StageCredentials { let Some(RunTarget::Git(target)) = &spec.target else { return Ok(None); }; - let repository = target.clone().validate()?.repository().clone(); + let repository = target.clone().validate(host)?.repository().clone(); let integration = spec .settings .run @@ -65,7 +65,8 @@ impl StageCredentials { .github .resolve_integration()?; let Some(access) = GitHubRepositoryAccess::new( - Some(&repository.https_url()), + host, + Some(&repository.https_url(host)), &integration.additional_repositories, integration.permissions.clone(), )? diff --git a/lib/components/fabro-store/src/platform_records.rs b/lib/components/fabro-store/src/platform_records.rs index 2424740c9..0901ea670 100644 --- a/lib/components/fabro-store/src/platform_records.rs +++ b/lib/components/fabro-store/src/platform_records.rs @@ -27,7 +27,7 @@ use std::sync::Arc; use fabro_types::{ - ArtifactSource, BlobHash, DiffSummary, GitIdentity, PairId, PairTarget, Principal, + ArtifactSource, BlobHash, DiffSummary, GitHubHost, GitIdentity, PairId, PairTarget, Principal, PullRequestCreationId, PullRequestLink, RunControlAction, RunId, RunNoticeLevel, RunSpec, RunStatus, }; @@ -557,27 +557,37 @@ pub struct PullRequestFailedRecord { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct PullRequestLinkedRecord { - pub owner: String, - pub repo: String, - pub number: u64, + pub owner: String, + pub repo: String, + pub number: u64, + /// The pull request's URL when it is not on github.com; github.com links + /// are derived, so their records are unchanged. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub html_url: Option, } impl PullRequestLinkedRecord { #[must_use] pub fn link(&self) -> PullRequestLink { PullRequestLink { - owner: self.owner.clone(), - repo: self.repo.clone(), - number: self.number, + owner: self.owner.clone(), + repo: self.repo.clone(), + number: self.number, + html_url: self.html_url.clone().unwrap_or_else(|| { + GitHubHost::default().pull_request_url(&self.owner, &self.repo, self.number) + }), } } #[must_use] pub fn from_link(link: &PullRequestLink) -> Self { Self { - owner: link.owner.clone(), - repo: link.repo.clone(), - number: link.number, + owner: link.owner.clone(), + repo: link.repo.clone(), + number: link.number, + html_url: (link.html_url + != GitHubHost::default().pull_request_url(&link.owner, &link.repo, link.number)) + .then(|| link.html_url.clone()), } } } @@ -941,16 +951,18 @@ mod tests { } PlatformRecordKind::PullRequestLinked => { PlatformRecord::PullRequestLinked(PullRequestLinkedRecord { - owner: "fabro-sh".to_string(), - repo: "fabro".to_string(), - number: 7, + owner: "fabro-sh".to_string(), + repo: "fabro".to_string(), + number: 7, + html_url: None, }) } PlatformRecordKind::PullRequestUnlinked => { PlatformRecord::PullRequestUnlinked(PullRequestLinkedRecord { - owner: "fabro-sh".to_string(), - repo: "fabro".to_string(), - number: 7, + owner: "fabro-sh".to_string(), + repo: "fabro".to_string(), + number: 7, + html_url: None, }) } PlatformRecordKind::NotificationSent => { diff --git a/lib/components/fabro-store/src/run_summary.rs b/lib/components/fabro-store/src/run_summary.rs index 7fbcd389e..7c92570d0 100644 --- a/lib/components/fabro-store/src/run_summary.rs +++ b/lib/components/fabro-store/src/run_summary.rs @@ -2,7 +2,7 @@ //! the board and the scheduler read, derived from the projection. use fabro_types::{ - AskFabro, RepositoryRef, Run, RunId, RunLifecycle, RunLinks, RunModel, RunOrigin, + AskFabro, GitHubHost, RepositoryRef, Run, RunId, RunLifecycle, RunLinks, RunModel, RunOrigin, RunProjection, RunSize, RunTimestamps, WorkflowRef, sum_usage, }; use lithos_llm::types::Usage; @@ -10,7 +10,7 @@ use lithos_llm::types::Usage; /// The run summary (`Run`) a projection stands for: what the run list, the /// board and the scheduler read. #[must_use] -pub fn build_summary(state: &RunProjection, run_id: &RunId) -> Run { +pub fn build_summary(host: &GitHubHost, state: &RunProjection, run_id: &RunId) -> Run { let goal = state.spec.graph.goal().to_string(); let diff_summary = state .conclusion @@ -65,6 +65,7 @@ pub fn build_summary(state: &RunProjection, run_id: &RunId) -> Run { }, automation: state.spec.automation.clone(), repository: Some(RepositoryRef::from_origin_and_source( + host, repo_origin_url, source_directory.as_deref(), )), diff --git a/lib/components/fabro-store/src/run_summary_store.rs b/lib/components/fabro-store/src/run_summary_store.rs index 79b128031..faa87de23 100644 --- a/lib/components/fabro-store/src/run_summary_store.rs +++ b/lib/components/fabro-store/src/run_summary_store.rs @@ -6,7 +6,7 @@ use std::fmt::Write as _; use std::sync::{Arc, LazyLock, PoisonError, RwLock}; use chrono::{DateTime, Utc}; -use fabro_types::{Run, RunId, RunSize, RunStatusKind, RunTiming, timing}; +use fabro_types::{GitHubHost, Run, RunId, RunSize, RunStatusKind, RunTiming, timing}; use sqlx::query::Query; use sqlx::sqlite::{SqliteArguments, SqliteConnection, SqliteRow}; use sqlx::{QueryBuilder, Row as _, Sqlite, SqlitePool}; @@ -202,13 +202,15 @@ impl RunSummaryStore { /// Write the `runs` row of a run from its projection, on a connection /// the caller holds a transaction on: the columns the list views and - /// the scheduler read, and the summary JSON. + /// the scheduler read, and the summary JSON. `host` is the GitHub + /// deployment the run's origin is classified against. pub async fn write_petri_run_row_on_connection( connection: &mut SqliteConnection, + host: &GitHubHost, run_id: &RunId, projection: &RunProjection, ) -> Result<()> { - let record = PreparedRunSummary::from_projection(run_id, projection); + let record = PreparedRunSummary::from_projection(host, run_id, projection); bind_run_columns( sqlx::query(UPSERT_PETRI_RUN_SQL).bind(run_id.to_string()), &record, @@ -408,8 +410,8 @@ struct PreparedRunSummary { } impl PreparedRunSummary { - fn from_projection(run_id: &RunId, projection: &RunProjection) -> Self { - let mut run = build_summary(projection, run_id); + fn from_projection(host: &GitHubHost, run_id: &RunId, projection: &RunProjection) -> Self { + let mut run = build_summary(host, projection, run_id); if run.timing.is_none() { let at = run .timestamps @@ -706,6 +708,7 @@ mod tests { let mut transaction = store.pool.begin().await.unwrap(); RunSummaryStore::write_petri_run_row_on_connection( &mut transaction, + &fabro_types::GitHubHost::default(), &projection.spec.run_id, projection, ) diff --git a/lib/components/fabro-tool/src/create.rs b/lib/components/fabro-tool/src/create.rs index d3b23d30b..b3a713892 100644 --- a/lib/components/fabro-tool/src/create.rs +++ b/lib/components/fabro-tool/src/create.rs @@ -71,7 +71,7 @@ impl FabroRunCreateParams { if let Some(target) = &spec.target { target .clone() - .validate() + .canonical() .map_err(|error| ToolError::from_anyhow(&error.into()))?; } for (key, value) in &spec.args.inputs { diff --git a/lib/components/fabro-workflow/src/pull_request.rs b/lib/components/fabro-workflow/src/pull_request.rs index 8cd6f88d1..fbeee875b 100644 --- a/lib/components/fabro-workflow/src/pull_request.rs +++ b/lib/components/fabro-workflow/src/pull_request.rs @@ -465,9 +465,10 @@ async fn reconcile_existing_pull_request( .await; Ok(Some(CreatedPullRequest { link: PullRequestLink { - owner: owner.to_string(), - repo: repo.to_string(), - number: existing.number, + owner: owner.to_string(), + repo: repo.to_string(), + number: existing.number, + html_url: existing.html_url.clone(), }, title: existing.title, base_branch: req.base_branch.to_string(), @@ -553,8 +554,8 @@ pub async fn open_pull_request( req: OpenPullRequestRequest<'_>, ) -> Result { let https_url = ssh_url_to_https(req.origin_url); - let (owner, repo) = - github_app::parse_github_owner_repo(&https_url).map_err(|err| format!("{err:#}"))?; + let (owner, repo) = github_app::parse_github_owner_repo(req.github.host(), &https_url) + .map_err(|err| format!("{err:#}"))?; // Verify before generating content: this is the cheap check, and a stale // branch would otherwise cost a full LLM call before failing. @@ -625,6 +626,7 @@ pub async fn open_pull_request( owner, repo, number: created.number, + html_url: created.html_url.clone(), }; Ok(CreatedPullRequest { @@ -1273,7 +1275,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr let harness = setup_fallback_test_harness_with_branch_sha(&payload, "stale-sha").await; let github_base_url = harness.github_server.url(""); let github_host = - fabro_github::GitHubHost::new(&github_base_url, Some(&github_base_url)).unwrap(); + fabro_github::GitHubHost::new("https://github.com", Some(&github_base_url)).unwrap(); let error = open_pull_request(OpenPullRequestRequest { run_id: RunId::new(), github: fabro_github::GitHubContext::new(&harness.creds, &github_host), @@ -1556,7 +1558,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr let github_base_url = harness.github_server.url(""); let github_host = - fabro_github::GitHubHost::new(&github_base_url, Some(&github_base_url)).unwrap(); + fabro_github::GitHubHost::new("https://github.com", Some(&github_base_url)).unwrap(); let github = github_app::GitHubContext::new(&harness.creds, &github_host); let result = open_pull_request(OpenPullRequestRequest { @@ -1606,7 +1608,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr let github_base_url = harness.github_server.url(""); let github_host = - fabro_github::GitHubHost::new(&github_base_url, Some(&github_base_url)).unwrap(); + fabro_github::GitHubHost::new("https://github.com", Some(&github_base_url)).unwrap(); let github = github_app::GitHubContext::new(&harness.creds, &github_host); let result = open_pull_request(OpenPullRequestRequest { @@ -1642,7 +1644,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr let github_base_url = harness.github_server.url(""); let github_host = - fabro_github::GitHubHost::new(&github_base_url, Some(&github_base_url)).unwrap(); + fabro_github::GitHubHost::new("https://github.com", Some(&github_base_url)).unwrap(); let github = github_app::GitHubContext::new(&harness.creds, &github_host); // Single ~200-char line, no `Plan:` / heading prefix, no newlines. diff --git a/lib/components/fabro-workflow/src/run_lookup.rs b/lib/components/fabro-workflow/src/run_lookup.rs index a1f3c1b47..0fdab732e 100644 --- a/lib/components/fabro-workflow/src/run_lookup.rs +++ b/lib/components/fabro-workflow/src/run_lookup.rs @@ -485,6 +485,7 @@ mod tests { let mut transaction = summaries.pool().begin().await.unwrap(); RunSummaryStore::write_petri_run_row_on_connection( &mut transaction, + &fabro_types::GitHubHost::default(), &fixtures::RUN_1, &projection, ) diff --git a/lib/foundation/fabro-api/tests/run_summary_round_trip.rs b/lib/foundation/fabro-api/tests/run_summary_round_trip.rs index dc01c8590..50715f9f6 100644 --- a/lib/foundation/fabro-api/tests/run_summary_round_trip.rs +++ b/lib/foundation/fabro-api/tests/run_summary_round_trip.rs @@ -143,9 +143,10 @@ fn run_summary_json_matches_openapi_shape() { deletions: 4, }), pull_request: Some(PullRequestLink { - owner: "fabro-sh".to_string(), - repo: "fabro".to_string(), - number: 123, + owner: "fabro-sh".to_string(), + repo: "fabro".to_string(), + number: 123, + html_url: "https://github.com/fabro-sh/fabro/pull/123".to_string(), }), current_question: None, superseded_by: None, diff --git a/lib/foundation/fabro-config/src/builders.rs b/lib/foundation/fabro-config/src/builders.rs index d6bdf4608..479d4438e 100644 --- a/lib/foundation/fabro-config/src/builders.rs +++ b/lib/foundation/fabro-config/src/builders.rs @@ -4,14 +4,15 @@ use std::path::Path; use fabro_types::settings::run::McpServerSettings; use fabro_types::settings::{RunNamespace, WorkflowNamespace}; -use fabro_types::{ServerSettings, UserSettings, WorkflowSettings}; +use fabro_types::{GitHubHost, ServerSettings, UserSettings, WorkflowSettings}; use fabro_util::error::SharedError; use crate::defaults::DEFAULTS_LAYER; use crate::load::load_settings_path; use crate::parse::{SettingsSource, validate_settings_source}; use crate::resolve::{ - ResolveError, resolve_cli, resolve_project, resolve_run, resolve_server, resolve_workflow, + ResolveError, resolve_cli, resolve_github_host, resolve_project, resolve_run, resolve_server, + resolve_workflow, }; use crate::user::load_settings_config; use crate::{ @@ -86,6 +87,20 @@ impl ServerSettingsBuilder { Self::from_layer(&layer) } + /// The configured GitHub host alone, for callers that have no complete + /// server configuration to resolve (the CLI without a local server). + pub fn load_default_github_host() -> Result { + let layer = load_settings_config(None)?; + let github = layer + .server + .and_then(|server| server.integrations) + .and_then(|integrations| integrations.github) + .unwrap_or_default(); + let mut errors = Vec::new(); + let host = resolve_github_host(&github, &mut errors); + finish_result(host, "failed to resolve the GitHub host", errors) + } + pub fn from_toml(source: &str) -> Result { let layer = parse_settings_toml(source, SettingsSource::ActiveSettings)?; Self::from_layer(&layer) diff --git a/lib/foundation/fabro-config/src/resolve/mod.rs b/lib/foundation/fabro-config/src/resolve/mod.rs index 487f55aa9..5639ef660 100644 --- a/lib/foundation/fabro-config/src/resolve/mod.rs +++ b/lib/foundation/fabro-config/src/resolve/mod.rs @@ -13,6 +13,7 @@ pub use error::ResolveError; use fabro_types::settings::InterpString; pub use project::resolve_project; pub use run::resolve_run; +pub(crate) use server::resolve_github_host; pub use server::resolve_server; pub use workflow::resolve_workflow; diff --git a/lib/foundation/fabro-config/src/resolve/server.rs b/lib/foundation/fabro-config/src/resolve/server.rs index 66d75ad9b..d5458532b 100644 --- a/lib/foundation/fabro-config/src/resolve/server.rs +++ b/lib/foundation/fabro-config/src/resolve/server.rs @@ -381,7 +381,7 @@ fn resolve_integrations( } } -fn resolve_github_host( +pub(crate) fn resolve_github_host( layer: &GithubIntegrationLayer, errors: &mut Vec, ) -> GitHubHost { diff --git a/lib/foundation/fabro-types/src/github_host.rs b/lib/foundation/fabro-types/src/github_host.rs index bc928e90c..0033c38fd 100644 --- a/lib/foundation/fabro-types/src/github_host.rs +++ b/lib/foundation/fabro-types/src/github_host.rs @@ -150,6 +150,21 @@ impl GitHubHost { .map_or(self.web_url.as_str(), |(_, authority)| authority) } + /// The host name of the web origin, without any port. + #[must_use] + pub fn hostname(&self) -> &str { + let authority = self.host(); + authority + .rsplit_once(':') + .map_or(authority, |(name, port)| { + if port.bytes().all(|byte| byte.is_ascii_digit()) { + name + } else { + authority + } + }) + } + #[must_use] pub fn is_github_com(&self) -> bool { self.host() == "github.com" @@ -296,6 +311,7 @@ mod tests { let ghes = host("https://GHE.Example.com:8443"); assert_eq!(ghes.web_url(), "https://ghe.example.com:8443"); assert_eq!(ghes.host(), "ghe.example.com:8443"); + assert_eq!(ghes.hostname(), "ghe.example.com"); } #[test] diff --git a/lib/foundation/fabro-types/src/pull_request.rs b/lib/foundation/fabro-types/src/pull_request.rs index 4938083b5..19b263313 100644 --- a/lib/foundation/fabro-types/src/pull_request.rs +++ b/lib/foundation/fabro-types/src/pull_request.rs @@ -3,6 +3,7 @@ use serde::de::Error as DeError; use serde::ser::SerializeStruct; use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use crate::GitHubHost; use crate::id::ulid_id; ulid_id!(PullRequestCreationId); @@ -60,18 +61,18 @@ impl PullRequestCreation { /// Minimal GitHub pull request reference stored on a workflow run. #[derive(Debug, Clone, PartialEq, Eq)] pub struct PullRequestLink { - pub owner: String, - pub repo: String, - pub number: u64, + pub owner: String, + pub repo: String, + pub number: u64, + /// The URL GitHub serves the pull request at, on whichever host the + /// server is configured for. + pub html_url: String, } impl PullRequestLink { #[must_use] pub fn html_url(&self) -> String { - format!( - "https://github.com/{}/{}/pull/{}", - self.owner, self.repo, self.number - ) + self.html_url.clone() } pub fn from_github_url(url: &str) -> Result { @@ -88,7 +89,7 @@ impl Serialize for PullRequestLink { state.serialize_field("owner", &self.owner)?; state.serialize_field("repo", &self.repo)?; state.serialize_field("number", &self.number)?; - state.serialize_field("html_url", &self.html_url())?; + state.serialize_field("html_url", &self.html_url)?; state.end() } } @@ -115,34 +116,29 @@ impl<'de> Deserialize<'de> for PullRequestLink { let (Some(owner), Some(repo), Some(number)) = (wire.owner, wire.repo, wire.number) else { return Err(D::Error::custom("missing pull request owner/repo/number")); }; - let link = Self { - owner, - repo, - number, - }; - - if let Some(html_url) = wire.html_url { - let url_link = - github_pull_request_link_from_url(&html_url).map_err(D::Error::custom)?; - if url_link != link { - return Err(D::Error::custom( - "pull request html_url does not match owner/repo/number", - )); - } + // Records written before the link stored its URL are all github.com. + let html_url = wire + .html_url + .unwrap_or_else(|| GitHubHost::default().pull_request_url(&owner, &repo, number)); + let url_link = github_pull_request_link_from_url(&html_url).map_err(D::Error::custom)?; + if (&url_link.owner, &url_link.repo, url_link.number) != (&owner, &repo, number) { + return Err(D::Error::custom( + "pull request html_url does not match owner/repo/number", + )); } - Ok(link) + Ok(url_link) } } #[expect( clippy::disallowed_types, - reason = "Pull request links are public github.com URLs stored for display and coordinate inference." + reason = "Pull request links are public GitHub URLs stored for display and coordinate inference." )] fn github_pull_request_link_from_url(raw_url: &str) -> Result { let parsed = url::Url::parse(raw_url).map_err(|err| format!("Invalid pull request URL: {err}"))?; - if parsed.scheme() != "https" || parsed.host_str() != Some("github.com") { + if parsed.scheme() != "https" || parsed.host_str().is_none() { return Err( "Pull request link must be a GitHub pull request URL like https://github.com/owner/repo/pull/123." .to_string(), @@ -168,6 +164,7 @@ fn github_pull_request_link_from_url(raw_url: &str) -> Result(json!({ + "owner": "fabro-sh", + "repo": "fabro", + "number": 270 + })) + .unwrap(); + + assert_eq!(link.html_url, "https://github.com/fabro-sh/fabro/pull/270"); + } + + #[test] + fn pull_request_link_keeps_the_enterprise_url_it_was_given() { + let value = json!({ + "owner": "acme", + "repo": "widgets", + "number": 7, + "html_url": "https://ghe.example.com/acme/widgets/pull/7" + }); + let link = serde_json::from_value::(value.clone()).unwrap(); + + assert_eq!( + link.html_url(), + "https://ghe.example.com/acme/widgets/pull/7" + ); + assert_eq!(serde_json::to_value(&link).unwrap(), value); + assert_eq!( + PullRequestLink::from_github_url("https://octocorp.ghe.com/acme/widgets/pull/7") + .unwrap() + .owner, + "acme" + ); + } + + #[test] + fn pull_request_link_rejects_a_url_that_disagrees_with_its_coordinates() { + let result = serde_json::from_value::(json!({ + "owner": "acme", + "repo": "widgets", + "number": 8, + "html_url": "https://ghe.example.com/acme/widgets/pull/7" + })); + + assert!(result.is_err()); + } } diff --git a/lib/foundation/fabro-types/src/repository.rs b/lib/foundation/fabro-types/src/repository.rs index b0e69f3e5..64d2e5fdf 100644 --- a/lib/foundation/fabro-types/src/repository.rs +++ b/lib/foundation/fabro-types/src/repository.rs @@ -5,6 +5,8 @@ use std::str::FromStr; use serde::{Deserialize, Serialize}; +use crate::GitHubHost; + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct RepositoryRef { pub name: String, @@ -15,12 +17,13 @@ pub struct RepositoryRef { impl RepositoryRef { pub fn from_origin_and_source( + host: &GitHubHost, origin_url: Option, source_directory: Option<&str>, ) -> Self { Self { name: repository_name(origin_url.as_deref(), source_directory), - provider: repository_provider(origin_url.as_deref()), + provider: repository_provider(host, origin_url.as_deref()), origin_url, } } @@ -65,10 +68,10 @@ impl GitHubRepositorySlug { &self.repo } - /// Canonical credential-free HTTPS URL for this GitHub repository. + /// Canonical credential-free HTTPS URL for this repository on `host`. #[must_use] - pub fn https_url(&self) -> String { - format!("https://github.com/{self}") + pub fn https_url(&self, host: &GitHubHost) -> String { + host.repository_url(&self.owner, &self.repo) } /// Whether `other` names the same repository owner, ignoring ASCII case. @@ -264,24 +267,17 @@ pub enum RepositoryProvider { Unknown, } -fn repository_provider(origin_url: Option<&str>) -> RepositoryProvider { +fn repository_provider(host: &GitHubHost, origin_url: Option<&str>) -> RepositoryProvider { let Some(origin) = origin_url.filter(|origin| !origin.trim().is_empty()) else { return RepositoryProvider::Unknown; }; - if is_github_origin(origin) { + if host.is_origin(origin) { RepositoryProvider::Github } else { RepositoryProvider::Git } } -fn is_github_origin(origin: &str) -> bool { - origin.starts_with("git@github.com:") - || origin.starts_with("https://github.com/") - || origin.starts_with("http://github.com/") - || origin.starts_with("ssh://git@github.com/") -} - fn repository_name(origin_url: Option<&str>, source_directory: Option<&str>) -> String { origin_url .and_then(repository_name_from_origin) @@ -331,6 +327,48 @@ fn path_basename(path: &str) -> Option<&str> { mod tests { use super::*; + #[test] + fn repository_provider_is_github_only_for_origins_on_the_host() { + let ghes = GitHubHost::new("https://ghe.example.com", None).unwrap(); + let provider = |host: &GitHubHost, origin: &str| { + RepositoryRef::from_origin_and_source(host, Some(origin.to_string()), None).provider + }; + + assert_eq!( + provider(&GitHubHost::default(), "git@github.com:acme/widgets.git"), + RepositoryProvider::Github + ); + assert_eq!( + provider( + &GitHubHost::default(), + "https://ghe.example.com/acme/widgets" + ), + RepositoryProvider::Git + ); + assert_eq!( + provider(&ghes, "https://ghe.example.com/acme/widgets"), + RepositoryProvider::Github + ); + assert_eq!( + provider(&ghes, "ssh://git@ghe.example.com/acme/widgets"), + RepositoryProvider::Github + ); + assert_eq!( + provider(&ghes, "https://github.com/acme/widgets"), + RepositoryProvider::Git + ); + } + + #[test] + fn https_url_follows_the_host() { + let slug = GitHubRepositorySlug::try_new("acme/widgets").unwrap(); + let ghes = GitHubHost::new("https://octocorp.ghe.com", None).unwrap(); + assert_eq!( + slug.https_url(&ghes), + "https://octocorp.ghe.com/acme/widgets" + ); + } + #[test] fn valid_slugs_preserve_exact_parts() { let cases = [ @@ -404,8 +442,11 @@ mod tests { fn slug_https_url_preserves_spelling_and_has_no_credentials() { let slug: GitHubRepositorySlug = "Fabro-SH/Keystone".parse().unwrap(); - assert_eq!(slug.https_url(), "https://github.com/Fabro-SH/Keystone"); - assert!(!slug.https_url().contains('@')); + assert_eq!( + slug.https_url(&GitHubHost::default()), + "https://github.com/Fabro-SH/Keystone" + ); + assert!(!slug.https_url(&GitHubHost::default()).contains('@')); } #[test] diff --git a/lib/foundation/fabro-types/src/run_intent.rs b/lib/foundation/fabro-types/src/run_intent.rs index ba4c472f7..02f0b5732 100644 --- a/lib/foundation/fabro-types/src/run_intent.rs +++ b/lib/foundation/fabro-types/src/run_intent.rs @@ -3,7 +3,9 @@ use std::collections::HashMap; use serde::{Deserialize, Serialize}; use serde_json::Value; -use crate::{DirtyStatus, GitContext, GitHubRepositorySlug, RunId, WorkflowVersionId, repository}; +use crate::{ + DirtyStatus, GitContext, GitHubHost, GitHubRepositorySlug, RunId, WorkflowVersionId, repository, +}; /// A request to create a run from an immutable workflow version. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -73,6 +75,17 @@ pub struct GitRunTarget { } impl GitRunTarget { + /// Validates the coordinate and returns it in canonical form, for callers + /// that do not need the operational Git projection (and so no host). + /// + /// # Errors + /// + /// Same as [`GitRunTarget::validate`]. + pub fn canonical(self) -> Result { + self.validate(&GitHubHost::default()) + .map(ValidatedGitRunTarget::into_target) + } + /// Validates and canonicalizes this Git coordinate without resolving remote /// repository state. /// @@ -80,7 +93,10 @@ impl GitRunTarget { /// /// Returns an error when the repository slug, branch, tag, or exact commit /// does not use the canonical grammar accepted for Git-backed runs. - pub fn validate(self) -> Result { + pub fn validate( + self, + host: &GitHubHost, + ) -> Result { let Self { repo, branch, @@ -104,7 +120,7 @@ impl GitRunTarget { }) .transpose()?; let git = GitContext { - origin_url: repository.https_url(), + origin_url: repository.https_url(host), branch: branch.clone(), sha: sha.clone(), dirty: DirtyStatus::Clean, @@ -123,6 +139,17 @@ impl GitRunTarget { } impl RunTarget { + /// Validates the target and returns it in canonical form, for callers + /// that do not need the operational Git projection (and so no host). + /// + /// # Errors + /// + /// Same as [`RunTarget::validate`]. + pub fn canonical(self) -> Result { + self.validate(&GitHubHost::default()) + .map(|validated| validated.target) + } + /// The wire `kind` discriminator (`git`, `none`, or `folder`), for /// diagnostics. pub fn kind_name(&self) -> &'static str { @@ -139,10 +166,10 @@ impl RunTarget { /// /// Returns an error when a Git target's repository slug, branch, tag, or /// exact commit does not use the canonical grammar accepted for runs. - pub fn validate(self) -> Result { + pub fn validate(self, host: &GitHubHost) -> Result { match self { Self::Git(target) => { - let validated = target.validate().map_err(TargetValidationError::from)?; + let validated = target.validate(host).map_err(TargetValidationError::from)?; Ok(ValidatedRunTarget { target: Self::Git(validated.target), git: Some(validated.git), diff --git a/lib/foundation/fabro-types/tests/run_intent.rs b/lib/foundation/fabro-types/tests/run_intent.rs index a8c4b3b5e..c63cd042b 100644 --- a/lib/foundation/fabro-types/tests/run_intent.rs +++ b/lib/foundation/fabro-types/tests/run_intent.rs @@ -1,7 +1,8 @@ use std::collections::HashMap; use fabro_types::{ - GitRunTarget, RunIntent, RunIntentArgs, RunTarget, WorkflowVersionId, normalize_git_commit_sha, + GitHubHost, GitRunTarget, RunIntent, RunIntentArgs, RunTarget, WorkflowVersionId, + normalize_git_commit_sha, }; use serde_json::json; @@ -195,7 +196,7 @@ fn target_validation_normalizes_sha_without_network_resolution() { tag: Some("release/v1".to_string()), sha: Some("ABCDEF0123456789ABCDEF0123456789ABCDEF01".to_string()), }) - .validate() + .validate(&GitHubHost::default()) .unwrap(); let git = validated .git @@ -225,7 +226,7 @@ fn git_target_validation_carries_the_parsed_repository_proof() { tag: None, sha: Some("ABCDEF0123456789ABCDEF0123456789ABCDEF01".to_string()), } - .validate() + .validate(&GitHubHost::default()) .unwrap(); assert_eq!(validated.repository().owner(), "Fabro-Sh"); @@ -238,7 +239,7 @@ fn git_target_validation_carries_the_parsed_repository_proof() { #[test] fn run_intent_none_target_validates_without_a_git_projection() { - let validated = RunTarget::None {}.validate().unwrap(); + let validated = RunTarget::None {}.validate(&GitHubHost::default()).unwrap(); assert_eq!(validated.target, RunTarget::None {}); assert_eq!(validated.git, None); } @@ -249,7 +250,7 @@ fn run_intent_folder_target_is_preserved_for_provider_admission() { path: "/srv/fabro/workspaces/example".to_string(), }; - let validated = target.clone().validate().unwrap(); + let validated = target.clone().validate(&GitHubHost::default()).unwrap(); assert_eq!(validated.target, target); assert_eq!(validated.git, None); @@ -266,7 +267,7 @@ fn target_validation_rejects_invalid_grammar() { tag: tag.map(str::to_string), sha: sha.map(str::to_string), }) - .validate() + .validate(&GitHubHost::default()) }; assert_eq!( @@ -348,3 +349,21 @@ fn git_target_round_trips_all_branch_tag_sha_states() { ); } } + +#[test] +fn target_validation_derives_the_origin_from_the_configured_host() { + let host = GitHubHost::new("https://ghe.example.com", None).unwrap(); + let validated = RunTarget::Git(GitRunTarget { + repo: "acme/widgets".to_string(), + branch: "main".to_string(), + tag: None, + sha: None, + }) + .validate(&host) + .unwrap(); + + assert_eq!( + validated.git.unwrap().origin_url, + "https://ghe.example.com/acme/widgets" + ); +} 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 04/13] 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, From 8eaaa1f2c2e86a866808f70ad004c1c192dfa459 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1vid=20Balatoni?= Date: Sun, 25 Oct 2026 01:04:40 +0200 Subject: [PATCH 05/13] feat(server): derive GitHub OAuth, CSP and install URLs from the configured host --- lib/apps/fabro-cli/src/commands/server/mod.rs | 1 + .../fabro-server/src/auth/github_endpoints.rs | 40 +++++++-- lib/apps/fabro-server/src/csp.rs | 68 ++++++++++----- lib/apps/fabro-server/src/install.rs | 85 ++++++++++++++----- lib/apps/fabro-server/src/security_headers.rs | 16 ++-- lib/apps/fabro-server/src/server.rs | 12 ++- .../fabro-server/src/server/handler/system.rs | 5 +- lib/apps/fabro-server/src/server/tests.rs | 54 ++++++++++++ lib/apps/fabro-server/src/web_auth.rs | 14 +-- 9 files changed, 229 insertions(+), 66 deletions(-) diff --git a/lib/apps/fabro-cli/src/commands/server/mod.rs b/lib/apps/fabro-cli/src/commands/server/mod.rs index 2f3f3f8dc..851e82b47 100644 --- a/lib/apps/fabro-cli/src/commands/server/mod.rs +++ b/lib/apps/fabro-cli/src/commands/server/mod.rs @@ -224,6 +224,7 @@ async fn run_install_mode(bootstrap: InstallBootstrap, printer: Printer) -> Resu &bootstrap.storage_dir, &bootstrap.config_path, ) + .with_github_host(crate::user_config::github_host()?) .with_finish_hook(persist_install_dev_token_hook()); install::serve_install_command(bootstrap.bind_request, state, move |bind| { announce_install_mode(bind, &token, styles, printer); diff --git a/lib/apps/fabro-server/src/auth/github_endpoints.rs b/lib/apps/fabro-server/src/auth/github_endpoints.rs index 951c3af03..a7d59e680 100644 --- a/lib/apps/fabro-server/src/auth/github_endpoints.rs +++ b/lib/apps/fabro-server/src/auth/github_endpoints.rs @@ -4,6 +4,7 @@ )] use fabro_http::Url; +use fabro_types::GitHubHost; #[doc(hidden)] #[derive(Debug, Clone)] @@ -13,11 +14,12 @@ pub struct GithubEndpoints { } impl GithubEndpoints { - pub fn production_defaults() -> Self { + pub fn from_host(host: &GitHubHost) -> Self { Self { - oauth_base: Url::parse("https://github.com/").expect("github oauth base should parse"), - api_base: Url::parse("https://api.github.com/") - .expect("github api base should parse"), + oauth_base: Url::parse(&format!("{}/", host.web_url())) + .expect("GitHubHost web_url should parse as a URL"), + api_base: Url::parse(&format!("{}/", host.api_url())) + .expect("GitHubHost api_url should parse as a URL"), } } @@ -32,16 +34,42 @@ impl GithubEndpoints { #[cfg(test)] mod tests { + use fabro_types::GitHubHost; + use super::GithubEndpoints; #[test] - fn production_defaults_match_current_github_urls() { - let endpoints = GithubEndpoints::production_defaults(); + fn default_host_matches_current_github_urls() { + let endpoints = GithubEndpoints::from_host(&GitHubHost::default()); assert_eq!(endpoints.oauth_base.as_str(), "https://github.com/"); assert_eq!(endpoints.api_base.as_str(), "https://api.github.com/"); } + #[test] + fn enterprise_host_uses_its_web_origin_and_api_url() { + let host = GitHubHost::new("https://ghe.example.com", None).unwrap(); + let endpoints = GithubEndpoints::from_host(&host); + + assert_eq!(endpoints.oauth_base.as_str(), "https://ghe.example.com/"); + assert_eq!( + endpoints.api_base.as_str(), + "https://ghe.example.com/api/v3/" + ); + assert_eq!( + endpoints + .oauth_base + .join("login/oauth/authorize") + .unwrap() + .as_str(), + "https://ghe.example.com/login/oauth/authorize" + ); + assert_eq!( + endpoints.api_base.join("user").unwrap().as_str(), + "https://ghe.example.com/api/v3/user" + ); + } + #[test] fn with_bases_uses_custom_urls() { let endpoints = GithubEndpoints::with_bases( diff --git a/lib/apps/fabro-server/src/csp.rs b/lib/apps/fabro-server/src/csp.rs index efb16545c..6ba455045 100644 --- a/lib/apps/fabro-server/src/csp.rs +++ b/lib/apps/fabro-server/src/csp.rs @@ -4,24 +4,17 @@ //! `index.html` so any inline `