From d5bbc12261ec83bf084011f11f2656c9cfb11275 Mon Sep 17 00:00:00 2001 From: "brynary-fabro[bot]" <265161896+brynary-fabro[bot]@users.noreply.github.com> Date: Thu, 19 Mar 2026 12:29:55 -0400 Subject: [PATCH] Detect GitHub App visibility mismatch during `repo init` (#99) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR detects when a GitHub App's visibility will prevent installation on a cross-owner repository during `fabro repo init`. Previously, when the app wasn't installed, users saw only a generic "install at" URL with no indication of why the install link might not work—particularly confusing when the repo belongs to a different owner than the app and the app is private. Two new functions are added to `fabro-github`: `get_authenticated_app()` fetches the app's metadata (slug and owner) via the authenticated `GET /app` endpoint, and `is_app_public()` probes `GET /apps/{slug}` without authentication to determine visibility (public apps return 200, private ones return 404). In `init.rs`, when the app is not installed, we now compare the app owner against the repo owner and, if they differ and the app is private, display a targeted warning explaining that the app must be made public along with a direct link to the settings page. All new checks are best-effort—failures are silently ignored so the existing flow is unaffected. The PR also introduces a `GITHUB_API_BASE_URL` constant to replace hardcoded URL strings and adds five unit tests covering the new functions: successful app info retrieval, auth failure handling, public/private app detection, and verification that the visibility check sends no `Authorization` header. ### Fabro Details
Ran 10 stages in 18m 26s for $4.40 | Stage | Duration | Cost | Retries | |---|---|---|---| | start | 0s | – | 0 | | toolchain | 0s | – | 0 | | preflight_compile | 1m 13s | – | 0 | | preflight_lint | 13s | – | 0 | | implement | 3m 23s | $0.94 | 0 | | simplify_opus | 5m 17s | $1.44 | 0 | | simplify_gemini | 2m 52s | $0.92 | 0 | | simplify_gpt | 3m 24s | $1.10 | 0 | | verify | 1m 25s | – | 0 | | fmt | 1s | – | 0 | | **Total** | **18m 26s** | **$4.40** | **0** |
Ran ImplementAndSimplify.fabro (13 nodes and 16 edges) ```dot digraph ImplementAndSimplify { graph [ goal="Implement and simplify", model_stylesheet=" * { backend: api; model: claude-opus-4-6;} " ] rankdir=LR start [shape=Mdiamond, label="Start"] exit [shape=Msquare, label="Exit"] toolchain [label="Toolchain", shape=parallelogram, script="command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1", max_retries=0] preflight_compile [label="Preflight Compile", shape=parallelogram, script="cargo check -q --workspace 2>&1", max_retries=0] preflight_lint [label="Preflight Lint", shape=parallelogram, script="cargo clippy -q --workspace -- -D warnings 2>&1", max_retries=0] fix_lints [label="Fix Lints", prompt="The preflight lint step failed. Read the build output from context and fix all clippy lint warnings.", max_visits=3] implement [label="Implement", prompt="Read the plan file referenced in the goal and implement every step. Make all the code changes described in the plan. Use red/green TDD."] simplify_opus [label="Simplify (Opus)", prompt="@prompts/simplify.md"] simplify_gemini [label="Simplify (Gemini)", prompt="@prompts/simplify.md", model="gemini-3.1-pro-preview-customtools"] simplify_gpt [label="Simplify (GPT-54)", prompt="@prompts/simplify.md", model="gpt-54"] verify [label="Verify", shape=parallelogram, script="cargo clippy -q --workspace -- -D warnings 2>&1 && cargo nextest run --cargo-quiet --workspace --status-level fail 2>&1", goal_gate=true, retry_target="fixup"] fixup [label="Fixup", prompt="The verify step failed. Read the build output from context and fix all clippy lint warnings and test failures.", max_visits=3] fmt [label="Format", shape=parallelogram, script="cargo fmt --all 2>&1", max_retries=0] start -> toolchain toolchain -> preflight_compile [condition="outcome=success"] toolchain -> exit preflight_compile -> preflight_lint [condition="outcome=success"] preflight_compile -> exit preflight_lint -> implement [condition="outcome=success"] preflight_lint -> fix_lints fix_lints -> preflight_lint implement -> simplify_opus -> simplify_gemini -> simplify_gpt -> verify verify -> fmt [condition="outcome=success"] verify -> fixup fixup -> verify fmt -> exit } ```
⚒️ Generated with [Fabro](https://fabro.sh) --------- Co-authored-by: Fabro --- lib/crates/fabro-cli/src/init.rs | 48 +++++++- lib/crates/fabro-github/src/lib.rs | 175 +++++++++++++++++++++++++++++ 2 files changed, 220 insertions(+), 3 deletions(-) diff --git a/lib/crates/fabro-cli/src/init.rs b/lib/crates/fabro-cli/src/init.rs index 8b8fab529..552bffe60 100644 --- a/lib/crates/fabro-cli/src/init.rs +++ b/lib/crates/fabro-cli/src/init.rs @@ -237,8 +237,14 @@ async fn check_github_app_installation() { let client = reqwest::Client::new(); - match fabro_github::check_app_installed(&client, &jwt, &owner, &repo, "https://api.github.com") - .await + match fabro_github::check_app_installed( + &client, + &jwt, + &owner, + &repo, + fabro_github::GITHUB_API_BASE_URL, + ) + .await { Ok(true) => { let green = console::Style::new().green(); @@ -254,6 +260,42 @@ async fn check_github_app_installation() { }; let yellow = console::Style::new().yellow(); + + // Best-effort: warn if the app is private and the repo belongs to a different owner. + if let Ok(app_info) = fabro_github::get_authenticated_app( + &client, + &jwt, + fabro_github::GITHUB_API_BASE_URL, + ) + .await + { + let cross_owner = !app_info.owner.login.eq_ignore_ascii_case(&owner); + let is_private = cross_owner + && fabro_github::is_app_public( + &client, + &app_info.slug, + fabro_github::GITHUB_API_BASE_URL, + ) + .await + == Ok(false); + + if is_private { + eprintln!( + "\n {} GitHub App \"{}\" is private but this repo belongs to a different owner ({}).", + yellow.apply_to("!"), + app_info.slug, + owner + ); + eprintln!( + " The app must be made public before it can be installed outside {}.", + app_info.owner.login + ); + eprintln!( + " Update visibility at: https://github.com/settings/apps/{}", + app_info.slug + ); + } + } eprintln!( "\n {} GitHub App is not installed for {owner}/{repo}", yellow.apply_to("!") @@ -275,7 +317,7 @@ async fn check_github_app_installation() { &jwt, &owner, &repo, - "https://api.github.com", + fabro_github::GITHUB_API_BASE_URL, ) .await { diff --git a/lib/crates/fabro-github/src/lib.rs b/lib/crates/fabro-github/src/lib.rs index 355d6bd5f..9f0533ccc 100644 --- a/lib/crates/fabro-github/src/lib.rs +++ b/lib/crates/fabro-github/src/lib.rs @@ -37,6 +37,19 @@ pub struct PullRequestRef { pub ref_name: String, } +/// Owner information for a GitHub App. +#[derive(Debug, Clone, Deserialize)] +pub struct AppOwner { + pub login: String, +} + +/// Information about a GitHub App from the authenticated `/app` endpoint. +#[derive(Debug, Clone, Deserialize)] +pub struct AppInfo { + pub slug: String, + pub owner: AppOwner, +} + /// Credentials for authenticating as a GitHub App. #[derive(Clone, Debug)] pub struct GitHubAppCredentials { @@ -524,6 +537,70 @@ pub async fn check_app_installed( } } +/// Fetch information about the authenticated GitHub App. +/// +/// Uses the App JWT to call `GET /app` and returns the app's slug and owner. +pub async fn get_authenticated_app( + client: &reqwest::Client, + jwt: &str, + base_url: &str, +) -> Result { + let url = format!("{base_url}/app"); + let resp = client + .get(&url) + .header("Authorization", format!("Bearer {jwt}")) + .header("Accept", "application/vnd.github+json") + .header("User-Agent", "fabro") + .send() + .await + .map_err(|e| format!("Failed to fetch GitHub App info: {e}"))?; + + match resp.status().as_u16() { + 200 => {} + 401 => { + return Err("GitHub App authentication failed. \ + Check that app_id and GITHUB_APP_PRIVATE_KEY are correct." + .to_string()) + } + status => { + return Err(format!( + "Unexpected status {status} fetching GitHub App info" + )) + } + } + + resp.json::() + .await + .map_err(|e| format!("Failed to parse GitHub App info: {e}")) +} + +/// Check whether a GitHub App is publicly visible. +/// +/// Calls `GET /apps/{slug}` **without** authentication. Public apps return 200, +/// private apps return 404 to unauthenticated requests. +pub async fn is_app_public( + client: &reqwest::Client, + slug: &str, + base_url: &str, +) -> Result { + let url = format!("{base_url}/apps/{slug}"); + let resp = client + .get(&url) + .header("Accept", "application/vnd.github+json") + .header("User-Agent", "fabro") + .send() + .await + .map_err(|e| format!("Failed to check GitHub App visibility: {e}"))?; + + match resp.status().as_u16() { + 200 => Ok(true), + 404 => Ok(false), + status => Err(format!( + "Unexpected status {status} checking GitHub App visibility" + )), + } +} + /// Resolve git clone credentials for a GitHub repository. /// /// Returns `(username, password)` for authenticated cloning. @@ -1718,6 +1795,104 @@ mod tests { ); } + // ----------------------------------------------------------------------- + // get_authenticated_app + // ----------------------------------------------------------------------- + + #[tokio::test] + async fn get_authenticated_app_success() { + let mut server = mockito::Server::new_async().await; + + server + .mock("GET", "/app") + .match_header("Authorization", "Bearer test-jwt") + .with_status(200) + .with_body(r#"{"slug": "my-fabro-app", "owner": {"login": "my-org"}}"#) + .create_async() + .await; + + let client = reqwest::Client::new(); + let info = get_authenticated_app(&client, "test-jwt", &server.url()) + .await + .unwrap(); + assert_eq!(info.slug, "my-fabro-app"); + assert_eq!(info.owner.login, "my-org"); + } + + #[tokio::test] + async fn get_authenticated_app_auth_failure() { + let mut server = mockito::Server::new_async().await; + + server + .mock("GET", "/app") + .with_status(401) + .create_async() + .await; + + let client = reqwest::Client::new(); + let result = get_authenticated_app(&client, "bad-jwt", &server.url()).await; + assert!(result.is_err()); + assert!( + result.unwrap_err().contains("authentication failed"), + "expected auth error" + ); + } + + // ----------------------------------------------------------------------- + // is_app_public + // ----------------------------------------------------------------------- + + #[tokio::test] + async fn is_app_public_returns_true_on_200() { + let mut server = mockito::Server::new_async().await; + + server + .mock("GET", "/apps/my-fabro-app") + .with_status(200) + .with_body(r#"{"slug": "my-fabro-app"}"#) + .create_async() + .await; + + let client = reqwest::Client::new(); + let result = is_app_public(&client, "my-fabro-app", &server.url()).await; + assert_eq!(result.unwrap(), true); + } + + #[tokio::test] + async fn is_app_public_returns_false_on_404() { + let mut server = mockito::Server::new_async().await; + + server + .mock("GET", "/apps/my-private-app") + .with_status(404) + .create_async() + .await; + + let client = reqwest::Client::new(); + let result = is_app_public(&client, "my-private-app", &server.url()).await; + assert_eq!(result.unwrap(), false); + } + + #[tokio::test] + async fn is_app_public_no_auth_header() { + let mut server = mockito::Server::new_async().await; + + // Verify the request does NOT include an Authorization header + let mock = server + .mock("GET", "/apps/my-app") + .match_header("Authorization", mockito::Matcher::Missing) + .with_status(200) + .with_body(r#"{"slug": "my-app"}"#) + .create_async() + .await; + + let client = reqwest::Client::new(); + let result = is_app_public(&client, "my-app", &server.url()).await; + assert_eq!(result.unwrap(), true); + + mock.assert_async().await; + } + // ----------------------------------------------------------------------- // get_pull_request // -----------------------------------------------------------------------