diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ad27252..453fd35 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,8 @@ on: - "src/**" - "Cargo.toml" - "Cargo.lock" + - "ghp/**" + - ".github/workflows/ci.yml" env: CARGO_TERM_COLOR: always @@ -19,9 +21,21 @@ jobs: with: components: clippy - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 # v2 (2026-03-12) + with: + workspaces: | + . + ghp - name: cargo check run: cargo check - name: cargo clippy run: cargo clippy -- -D warnings - name: cargo test run: cargo test + # ghp is a separate crate (not a workspace member): the git credential + # helper's fail-closed contract must be enforced in CI too. + - name: ghp clippy + run: cargo clippy --all-targets -- -D warnings + working-directory: ghp + - name: ghp test + run: cargo test + working-directory: ghp diff --git a/README.md b/README.md index 2c9e438..cb9d162 100644 --- a/README.md +++ b/README.md @@ -459,6 +459,74 @@ Trade-off vs. one key per org: a leaked key reaches the allowlisted repos of **all** configured installations. Prefer separate agents/keys when you want per-org blast-radius isolation. +#### Git over HTTPS via App tokens (`/git-credential`) + +MCP covers issues and PRs, but `git push` speaks the Git protocol — it needs +a real credential at push time. `enable_git_credentials` lets a +repository-scoped agent exchange its ghpool key for a **short-lived, +single-repo** App installation token, eliminating the last long-lived GitHub +credential in the agent container: + +```toml +[mcp] +enable_git_credentials = true # same hard gate as writes: + # agents + App backend + [mcp.audit] +``` + +The App needs **Contents: Read & write** on the target repositories. With +the singular `[mcp.github_app]` form, `owner` is **required** when git +credentials are enabled — the explicit `installation_id` is verified against +the installation's actual account before any token is issued. + +Agent side, `ghp` doubles as a standard git credential helper. Register it +as the **only** helper for `github.com` — `--replace-all` with an empty +first entry clears any inherited helpers (osxkeychain, GCM, `gh auth +git-credential`) that would otherwise supply broader credentials: + +```sh +git config --global --replace-all credential."https://github.com".helper "" +git config --global --add credential."https://github.com".helper "!ghp git-credential" +git config --global credential."https://github.com".useHttpPath true +``` + +Every `git push` then flows: + +``` +git push → ghp git-credential (GHPOOL_KEY from env) + → GET /git-credential?repo=owner/name (X-Ghpool-Key) + → key auth → repo allowlist → installation routing + → durable audit preflight (phase: git_credential_request) + → installation owner verified against GitHub + → Contents-only single-repo token (~1h, GitHub-enforced scope) + → audit result (phase: git_credential_result) + → push authenticated as [bot] +``` + +Properties: + +- **Single-repo, Contents-only tokens** — each credential is minted with + exactly the repo being pushed and `contents: write` only, never the App's + full permission set; GitHub itself enforces both boundaries. Git tokens + are cached separately from MCP tokens. +- **Owner binding** — the configured owner label is verified against the + actual installation account (`GET /app/installations/{id}`) before + issuance and re-verified hourly (accounts can rename); a mislabeled + installation ID is refused. +- **Fail-closed audit** — a request record is persisted *before* any mint or + cache lookup, and a result record before the token is returned; if either + write fails, no credential (503). The token value is never written to the + audit log. +- **Deny-by-default** — repo-less agents, off-allowlist repos, and owners + without an installation are refused before any mint. +- **Fail-closed helper** — once a request is recognized as `github.com` + HTTPS, any failure (missing `GHPOOL_KEY`, missing path, policy denial, + network error) makes `ghp` emit `quit=true`, telling git to stop the + helper cascade instead of falling through to broader stored credentials + or prompting. Non-GitHub hosts are declined quietly so other helpers can + serve them. `gist.github.com` is not supported. +- **Nothing to store** — `store`/`erase` are no-ops; tokens expire on their + own. `cache-control: no-store` on the response. + Deployment notes: - Requires egress to `api.githubcopilot.com` (the only additional external dependency). diff --git a/config.example.toml b/config.example.toml index b85d45e..134ee0e 100644 --- a/config.example.toml +++ b/config.example.toml @@ -47,6 +47,18 @@ enabled = false # PATs, and are always fail-closed audited. When set, the default upstream # switches to the full write-capable surface. # enable_writes = false +# Enable GET /git-credential: repository-scoped agents exchange their +# X-Ghpool-Key for a short-lived GitHub App installation token scoped to +# EXACTLY ONE repository with Contents-only permissions, usable as a +# git-over-HTTPS credential (username `x-access-token`). Closes the +# long-lived-PAT gap for `git push`. Same hard gate as writes: agents + +# App backend + audit (durable preflight before mint, result before the +# token is returned). The App needs Contents: read/write for pushes. With +# the singular [mcp.github_app] form, `owner` is REQUIRED — the explicit +# installation_id is verified against the installation's actual account +# before issuance. Client side: `ghp git-credential` is a drop-in git +# credential helper (see README for the fail-closed helper setup). +# enable_git_credentials = false # Max concurrent write calls per agent (0 = unlimited). # max_inflight_writes = 4 # Upstream MCP endpoint. Default: hosted read-only variant, or the full diff --git a/ghp/src/main.rs b/ghp/src/main.rs index c669f9a..fab51e7 100644 --- a/ghp/src/main.rs +++ b/ghp/src/main.rs @@ -14,6 +14,16 @@ fn main() { exit(0); } + // git credential helper protocol: `ghp git-credential ` + // Configure as the ONLY helper for github.com (the empty --replace-all + // entry clears inherited helpers so nothing broader can be consulted): + // git config --global --replace-all credential."https://github.com".helper "" + // git config --global --add credential."https://github.com".helper "!ghp git-credential" + // git config --global credential."https://github.com".useHttpPath true + if args.first().map(|s| s.as_str()) == Some("git-credential") { + exit(git_credential(args.get(1).map(|s| s.as_str()), &ghpool_url)); + } + // Try to handle as a pooled read via ghpool REST if let Some(code) = try_pooled(&args, &ghpool_url) { exit(code); @@ -209,6 +219,163 @@ fn http_get(url: &str) -> Option { reqwest::blocking::get(url).ok()?.text().ok() } +/// Git credential helper backed by ghpool's /git-credential endpoint: +/// exchanges GHPOOL_KEY for a short-lived, single-repo GitHub App +/// installation token. Only the `get` operation does anything; `store` +/// and `erase` are no-ops (tokens are ephemeral, nothing to persist). +/// +/// Exit codes: 0 with output = credential provided; 1 = decline quietly so +/// git falls through to the next configured helper (if any). +fn git_credential(op: Option<&str>, base: &str) -> i32 { + match op { + Some("get") => {} + Some("store") | Some("erase") => return 0, + _ => { + eprintln!("usage: ghp git-credential "); + return 1; + } + } + let mut input = String::new(); + use std::io::Read as _; + if std::io::stdin().read_to_string(&mut input).is_err() { + return 1; + } + match credential_plan(&input, env::var("GHPOOL_KEY").ok()) { + CredentialPlan::Decline => 1, + CredentialPlan::Quit => credential_quit(), + CredentialPlan::Fetch { repo, key } => match fetch_git_credential(base, &repo, &key) { + Some((user, pass)) => { + println!("username={}", user); + println!("password={}", pass); + 0 + } + None => credential_quit(), + }, + } +} + +/// Decision for a `git credential get` request, separated from I/O so the +/// fail-closed contract is testable. +#[derive(Debug, PartialEq)] +enum CredentialPlan { + /// Not a recognized github.com HTTPS repository request: decline quietly + /// (exit 1) so git may consult other helpers for other hosts. Gists use + /// a different path model and installation authorization surface, so + /// gist.github.com is deliberately not recognized. + Decline, + /// Recognized github.com request that cannot be served. The caller emits + /// `quit=true` so git stops the helper cascade instead of falling + /// through to osxkeychain/GCM/gh auth — that would bypass ghpool repo + /// policy and audit. + Quit, + /// Recognized and serviceable: exchange GHPOOL_KEY for a short-lived + /// single-repo token via ghpool. + Fetch { repo: String, key: String }, +} + +fn credential_plan(input: &str, key: Option) -> CredentialPlan { + let attrs = parse_credential_input(input); + if attrs.get("protocol").map(String::as_str) != Some("https") { + return CredentialPlan::Decline; + } + // Only github.com repository URLs are supported. Hostnames are + // case-insensitive and git passes an explicit port through in `host` + // (e.g. `GitHub.com`, `github.com:443`) — normalize before matching so + // no github.com variant can slip past to a broader helper. + let host = attrs + .get("host") + .map(|h| h.to_ascii_lowercase()) + .unwrap_or_default(); + let (hostname, port) = match host.split_once(':') { + Some((h, p)) => (h, Some(p)), + None => (host.as_str(), None), + }; + if hostname != "github.com" { + return CredentialPlan::Decline; + } + // From here the request is a recognized GitHub credential request and + // MUST NOT fall through on any failure. + if !matches!(port, None | Some("443")) { + return CredentialPlan::Quit; // github.com on a non-HTTPS port + } + let Some(key) = key.filter(|k| !k.is_empty()) else { + return CredentialPlan::Quit; + }; + let Some(repo) = attrs.get("path").and_then(|p| repo_from_path(p)) else { + return CredentialPlan::Quit; // useHttpPath missing/malformed + }; + CredentialPlan::Fetch { repo, key } +} + +/// Exchange the agent key for a single-repo token. Any failure — network, +/// non-2xx, malformed body — returns None; the caller fails closed. +fn fetch_git_credential(base: &str, repo: &str, key: &str) -> Option<(String, String)> { + let url = format!("{}/git-credential?repo={}", base, repo); + let client = reqwest::blocking::Client::new(); + let resp = client + .get(&url) + .header("X-Ghpool-Key", key) + .timeout(std::time::Duration::from_secs(15)) + .send() + .ok()?; + if !resp.status().is_success() { + return None; + } + let v = resp.json::().ok()?; + match (v["username"].as_str(), v["password"].as_str()) { + (Some(u), Some(p)) => Some((u.to_string(), p.to_string())), + _ => None, + } +} + +/// Fail closed for a recognized github.com credential request. Git's +/// credential protocol interprets `quit=true` as "stop trying helpers and do +/// not prompt", preventing fallback to broader stored credentials. +fn credential_quit() -> i32 { + println!("quit=true"); + 0 +} + +/// Parse `key=value` lines of the git credential helper protocol. +fn parse_credential_input(input: &str) -> std::collections::HashMap { + input + .lines() + .take_while(|l| !l.is_empty()) + .filter_map(|l| { + l.split_once('=') + .map(|(k, v)| (k.to_string(), v.to_string())) + }) + .collect() +} + +/// `owner/repo` from a git request path: strips a trailing `.git` and any +/// extra segments (`owner/repo/info/refs` → `owner/repo`). Both components +/// must match GitHub's identifier charset — anything else (percent-encoding, +/// whitespace, control chars, unicode) is refused so the value is provably +/// safe to interpolate into the ghpool query string. +fn repo_from_path(path: &str) -> Option { + let mut parts = path.trim_start_matches('/').splitn(3, '/'); + let owner = parts.next().filter(|s| !s.is_empty())?; + let repo = parts.next().filter(|s| !s.is_empty())?; + let repo = repo.strip_suffix(".git").unwrap_or(repo); + if repo.is_empty() { + return None; + } + if !owner + .bytes() + .all(|b| b.is_ascii_alphanumeric() || b == b'-') + { + return None; + } + if !repo + .bytes() + .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'-' | b'_' | b'.')) + { + return None; + } + Some(format!("{}/{}", owner, repo)) +} + fn flag_val(args: &[String], flag: &str) -> Option { args.iter().position(|a| a == flag).and_then(|i| args.get(i + 1).cloned()) } @@ -355,3 +522,188 @@ mod tests { assert_eq!(try_api(&a, "http://fake:8080"), None); } } + +#[cfg(test)] +mod git_credential_tests { + use super::*; + + #[test] + fn test_parse_credential_input() { + let attrs = parse_credential_input( + "protocol=https\nhost=github.com\npath=openabdev/openab.git\n\nignored=after-blank\n", + ); + assert_eq!(attrs.get("protocol").unwrap(), "https"); + assert_eq!(attrs.get("host").unwrap(), "github.com"); + assert_eq!(attrs.get("path").unwrap(), "openabdev/openab.git"); + assert!(!attrs.contains_key("ignored"), "parsing stops at blank line"); + } + + #[test] + fn test_repo_from_path() { + assert_eq!(repo_from_path("openabdev/openab.git").as_deref(), Some("openabdev/openab")); + assert_eq!(repo_from_path("openabdev/openab").as_deref(), Some("openabdev/openab")); + assert_eq!(repo_from_path("/oablab/chi.git").as_deref(), Some("oablab/chi")); + assert_eq!( + repo_from_path("openabdev/openab/info/refs").as_deref(), + Some("openabdev/openab") + ); + assert_eq!(repo_from_path("justowner"), None); + assert_eq!(repo_from_path("owner/.git"), None); + assert_eq!(repo_from_path(""), None); + } + + const GH_INPUT: &str = "protocol=https\nhost=github.com\npath=openabdev/openab.git\n"; + + #[test] + fn test_plan_declines_unrecognized_hosts() { + // Non-GitHub hosts decline quietly (exit 1) so other helpers can + // serve them — ghp claims no authority there. + for input in [ + "protocol=https\nhost=gitlab.com\npath=o/r.git\n", + "protocol=https\nhost=gist.github.com\npath=abc123.git\n", + "protocol=https\nhost=evil-github.com\npath=o/r.git\n", + "protocol=http\nhost=github.com\npath=o/r.git\n", // not https + "host=github.com\npath=o/r.git\n", // no protocol + ] { + assert_eq!( + credential_plan(input, Some("k".into())), + CredentialPlan::Decline, + "input: {:?}", + input + ); + } + } + + #[test] + fn test_plan_recognizes_github_host_variants() { + // Hostnames are case-insensitive and git passes explicit ports + // through in `host` — every github.com variant must be recognized, + // never handed to a broader helper. + for host in ["github.com", "GitHub.com", "GITHUB.COM", "github.com:443"] { + let input = format!("protocol=https\nhost={}\npath=o/r.git\n", host); + assert_eq!( + credential_plan(&input, Some("k".into())), + CredentialPlan::Fetch { repo: "o/r".into(), key: "k".into() }, + "host: {}", + host + ); + // …and still fail closed (never decline) when unservable + assert_eq!( + credential_plan(&input, None), + CredentialPlan::Quit, + "host {} without key must quit, not decline", + host + ); + } + // github.com on a non-HTTPS port: recognized but unsupported → quit + assert_eq!( + credential_plan( + "protocol=https\nhost=github.com:8443\npath=o/r.git\n", + Some("k".into()) + ), + CredentialPlan::Quit + ); + } + + #[test] + fn test_plan_quits_for_recognized_github_failures() { + // A recognized github.com request must NEVER fall through to broader + // helpers (osxkeychain/GCM/gh auth) — every unservable case quits. + assert_eq!(credential_plan(GH_INPUT, None), CredentialPlan::Quit, "missing key"); + assert_eq!( + credential_plan(GH_INPUT, Some(String::new())), + CredentialPlan::Quit, + "empty key" + ); + assert_eq!( + credential_plan("protocol=https\nhost=github.com\n", Some("k".into())), + CredentialPlan::Quit, + "missing path (useHttpPath not set)" + ); + for bad_path in [ + "justowner", + "owner/re%2Fpo", // percent-encoding + "owner/re po", // whitespace + "owner/r\u{00e9}po", // non-ASCII + "own~er/repo", // invalid owner charset + ] { + assert_eq!( + credential_plan( + &format!("protocol=https\nhost=github.com\npath={}\n", bad_path), + Some("k".into()) + ), + CredentialPlan::Quit, + "path: {:?}", + bad_path + ); + } + } + + #[test] + fn test_plan_fetches_for_serviceable_request() { + assert_eq!( + credential_plan(GH_INPUT, Some("secret-key".into())), + CredentialPlan::Fetch { + repo: "openabdev/openab".into(), + key: "secret-key".into() + } + ); + } + + /// One-shot raw HTTP mock: serves a canned response and closes. + fn mock_server(response: String) -> String { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + std::thread::spawn(move || { + if let Ok((mut stream, _)) = listener.accept() { + use std::io::{Read, Write}; + let mut buf = [0u8; 4096]; + let _ = stream.read(&mut buf); + let _ = stream.write_all(response.as_bytes()); + } + }); + format!("http://{}", addr) + } + + fn http_response(status: &str, body: &str) -> String { + format!( + "HTTP/1.1 {}\r\ncontent-type: application/json\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{}", + status, + body.len(), + body + ) + } + + #[test] + fn test_fetch_fails_closed_on_server_errors() { + // 403 from ghpool (policy denial) → None → caller emits quit=true + let base = mock_server(http_response("403 Forbidden", "")); + assert_eq!(fetch_git_credential(&base, "o/r", "k"), None); + + // 200 with a malformed body → None + let base = mock_server(http_response("200 OK", "notjson")); + assert_eq!(fetch_git_credential(&base, "o/r", "k"), None); + + // 200 with JSON missing the password → None + let base = mock_server(http_response("200 OK", r#"{"username":"x-access-token"}"#)); + assert_eq!(fetch_git_credential(&base, "o/r", "k"), None); + + // network failure (nothing listening) → None + assert_eq!( + fetch_git_credential("http://127.0.0.1:1", "o/r", "k"), + None + ); + } + + #[test] + fn test_fetch_returns_credential_on_success() { + let base = mock_server(http_response( + "200 OK", + r#"{"username":"x-access-token","password":"ghs_short","expires_at":1}"#, + )); + assert_eq!( + fetch_git_credential(&base, "openabdev/openab", "k"), + Some(("x-access-token".into(), "ghs_short".into())) + ); + } +} diff --git a/ghp/tests/git_credential_cli.rs b/ghp/tests/git_credential_cli.rs new file mode 100644 index 0000000..af52a1d --- /dev/null +++ b/ghp/tests/git_credential_cli.rs @@ -0,0 +1,88 @@ +//! End-to-end tests of the `ghp git-credential` helper contract against the +//! real binary: a recognized github.com request must emit `quit=true` (exit +//! 0) on every failure — never fall through to another credential helper — +//! while non-GitHub hosts decline quietly (exit 1, no output). + +use std::io::Write; +use std::process::{Command, Stdio}; + +/// Run `ghp git-credential get` with the given stdin and optional +/// GHPOOL_KEY. GHPOOL_URL points at a closed port so any fetch attempt +/// fails without touching the network. +fn run_get(input: &str, key: Option<&str>) -> (String, i32) { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_ghp")); + cmd.args(["git-credential", "get"]) + .env_remove("GHPOOL_KEY") + .env("GHPOOL_URL", "http://127.0.0.1:1") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()); + if let Some(k) = key { + cmd.env("GHPOOL_KEY", k); + } + let mut child = cmd.spawn().expect("spawn ghp"); + child + .stdin + .as_mut() + .unwrap() + .write_all(input.as_bytes()) + .unwrap(); + let out = child.wait_with_output().expect("wait ghp"); + ( + String::from_utf8_lossy(&out.stdout).into_owned(), + out.status.code().expect("exit code"), + ) +} + +#[test] +fn recognized_github_missing_key_quits() { + let (stdout, code) = run_get("protocol=https\nhost=github.com\npath=o/r.git\n\n", None); + assert_eq!(stdout, "quit=true\n"); + assert_eq!(code, 0); +} + +#[test] +fn recognized_github_host_variants_quit_on_failure() { + // case-insensitive host + explicit :443 port are still recognized; + // with the server unreachable the helper must quit, not fall through + for host in ["GitHub.com", "github.com:443"] { + let input = format!("protocol=https\nhost={}\npath=o/r.git\n\n", host); + let (stdout, code) = run_get(&input, Some("some-key")); + assert_eq!(stdout, "quit=true\n", "host: {}", host); + assert_eq!(code, 0, "host: {}", host); + } +} + +#[test] +fn recognized_github_missing_path_quits() { + let (stdout, code) = run_get("protocol=https\nhost=github.com\n\n", Some("some-key")); + assert_eq!(stdout, "quit=true\n"); + assert_eq!(code, 0); +} + +#[test] +fn non_github_host_declines_quietly() { + let (stdout, code) = run_get( + "protocol=https\nhost=gitlab.com\npath=o/r.git\n\n", + Some("some-key"), + ); + assert_eq!(stdout, ""); + assert_eq!(code, 1); +} + +#[test] +fn store_and_erase_are_noops() { + for op in ["store", "erase"] { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_ghp")); + let out = cmd + .args(["git-credential", op]) + .env_remove("GHPOOL_KEY") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .output() + .unwrap(); + assert_eq!(out.status.code(), Some(0), "op: {}", op); + assert!(out.stdout.is_empty(), "op: {}", op); + } +} diff --git a/src/app_token.rs b/src/app_token.rs index 64b1d8b..1bf078b 100644 --- a/src/app_token.rs +++ b/src/app_token.rs @@ -23,6 +23,16 @@ use serde::Deserialize; /// handed to an in-flight request stays valid for the request's lifetime. const REFRESH_MARGIN: Duration = Duration::from_secs(300); +/// Owner verification is re-checked this often. GitHub accounts can rename +/// (and freed names can be re-claimed), so a verified owner→installation +/// binding must not live for the whole process lifetime. +const VERIFY_TTL: Duration = Duration::from_secs(3600); + +/// Hard ceiling on any single GitHub App API call (JWT-authenticated mint, +/// installation resolution/verification). Without it a stalled connection +/// would hold a singleflight waiter queue indefinitely. +const MINT_TIMEOUT: Duration = Duration::from_secs(30); + /// A minted installation token plus its expiry (unix seconds). #[derive(Clone, Debug)] pub struct AppToken { @@ -38,9 +48,21 @@ pub struct AppTokenProvider { owner: Option, api_base: String, http: reqwest::Client, - /// Cache keyed by scope envelope ("" = installation-wide; otherwise the - /// sorted repo list). One credential per policy envelope. + /// Cache keyed by purpose + scope envelope. MCP and git credentials are + /// intentionally isolated: a repo-scoped MCP token may carry broader App + /// permissions and must never satisfy a git credential request. cached: Mutex>, + /// Per-key singleflight locks: concurrent misses for the SAME cache key + /// wait for one mint; distinct keys (different repos or purposes) mint + /// in parallel so one slow mint cannot stall unrelated issuance. + /// Entries are evicted when the mint completes, so the map is bounded + /// by in-flight mints — request-supplied repo names (e.g. failed mints + /// under a wildcard allowlist) cannot accumulate. + mint_locks: Mutex>>>, + /// Owner verified from GitHub's installation API for an explicit ID, + /// with the verification time — re-checked after VERIFY_TTL. The + /// configured owner label is not trusted by itself. + verified_owner: Mutex>, } #[derive(Deserialize)] @@ -52,6 +74,12 @@ struct TokenResponse { #[derive(Deserialize)] struct Installation { id: u64, + account: Option, +} + +#[derive(Deserialize)] +struct InstallationAccount { + login: String, } impl AppTokenProvider { @@ -75,8 +103,13 @@ impl AppTokenProvider { installation_id: Mutex::new(installation_id), owner, api_base, - http: reqwest::Client::new(), + http: reqwest::Client::builder() + .timeout(MINT_TIMEOUT) + .build() + .map_err(|e| format!("http client build failed: {}", e))?, cached: Mutex::new(HashMap::new()), + mint_locks: Mutex::new(HashMap::new()), + verified_owner: Mutex::new(None), }) } @@ -92,9 +125,31 @@ impl AppTokenProvider { /// enforces the boundary. Empty slice = installation-wide. Tokens are /// cached per envelope and refreshed near expiry. pub async fn token_scoped(&self, repositories: &[String]) -> Result { + self.token_for("mcp", repositories, None).await + } + + /// Git-over-HTTPS credential: a distinct cache namespace and a + /// least-privilege permission envelope. `contents:write` is sufficient + /// for fetch/push and prevents the returned token from bypassing MCP tool + /// policy for issues, PRs, Actions, administration, etc. + pub async fn token_git(&self, repository: &str) -> Result { + self.token_for( + "git:contents=write", + &[repository.to_string()], + Some(serde_json::json!({ "contents": "write" })), + ) + .await + } + + async fn token_for( + &self, + purpose: &str, + repositories: &[String], + permissions: Option, + ) -> Result { let mut envelope: Vec = repositories.to_vec(); envelope.sort(); - let key = envelope.join(","); + let key = format!("{}:{}", purpose, envelope.join(",")); let now = unix_now(); if let Some(t) = self.cached.lock().unwrap().get(&key) { @@ -102,12 +157,59 @@ impl AppTokenProvider { return Ok(t.clone()); } } - let fresh = self.mint(&envelope).await?; - self.cached.lock().unwrap().insert(key, fresh.clone()); - Ok(fresh) + // Key-scoped singleflight: concurrent misses for the SAME key wait + // for one mint and then re-check the cache; distinct keys proceed in + // parallel so a slow mint never stalls unrelated issuance. + let key_lock = self + .mint_locks + .lock() + .unwrap() + .entry(key.clone()) + .or_insert_with(|| std::sync::Arc::new(tokio::sync::Mutex::new(()))) + .clone(); + let _mint_guard = key_lock.lock().await; + if let Some(t) = self.cached.lock().unwrap().get(&key) { + if t.expires_at > unix_now() + REFRESH_MARGIN.as_secs() { + // fulfilled by the leader while we waited — evict our own + // generation too in case the leader's eviction raced our + // map insert + self.evict_mint_lock(&key, &key_lock); + return Ok(t.clone()); + } + } + let result = self.mint(&envelope, permissions.as_ref()).await; + if let Ok(fresh) = &result { + self.cached.lock().unwrap().insert(key.clone(), fresh.clone()); + } + // Evict the singleflight entry whether the mint succeeded or failed: + // waiters already holding this Arc still serialize behind it and + // re-check the cache; the next fresh miss creates a new lock. This + // bounds the map to in-flight mints instead of every key ever seen. + self.evict_mint_lock(&key, &key_lock); + result + } + + /// Remove a singleflight entry only if it is still OUR generation. + /// A stale waiter from an evicted generation must never remove a newer + /// generation's in-flight lock — that would let a third caller mint + /// concurrently and unboundedly repeat the race. With the ptr_eq guard, + /// each generation removes at most itself, so duplicate mints are + /// bounded to one per evicted generation. + fn evict_mint_lock(&self, key: &str, ours: &std::sync::Arc>) { + let mut locks = self.mint_locks.lock().unwrap(); + if locks + .get(key) + .is_some_and(|current| std::sync::Arc::ptr_eq(current, ours)) + { + locks.remove(key); + } } - async fn mint(&self, repositories: &[String]) -> Result { + async fn mint( + &self, + repositories: &[String], + permissions: Option<&serde_json::Value>, + ) -> Result { let jwt = self.sign_jwt()?; let installation_id = self.resolve_installation(&jwt).await?; @@ -115,16 +217,23 @@ impl AppTokenProvider { "{}/app/installations/{}/access_tokens", self.api_base, installation_id ); + let mut body = serde_json::Map::new(); + if !repositories.is_empty() { + // Scope the token to explicit repositories (names within the + // VERIFIED installation owner). GitHub rejects unknown repos. + body.insert("repositories".into(), serde_json::json!(repositories)); + } + if let Some(p) = permissions { + body.insert("permissions".into(), p.clone()); + } let mut req = self .http .post(&url) .header("Authorization", format!("Bearer {}", jwt)) .header("Accept", "application/vnd.github+json") .header("User-Agent", concat!("ghpool/", env!("CARGO_PKG_VERSION"))); - if !repositories.is_empty() { - // Scope the token to explicit repositories (names within the - // installation's owner). GitHub rejects unknown repos at mint. - req = req.json(&serde_json::json!({ "repositories": repositories })); + if !body.is_empty() { + req = req.json(&body); } let resp = req .send() @@ -156,6 +265,59 @@ impl AppTokenProvider { Ok(AppToken { token: tr.token, expires_at }) } + /// Verify that this provider's installation belongs to `expected_owner`. + /// This is mandatory before credential issuance: an explicit + /// installation ID plus an operator-supplied owner label is not an + /// identity binding. Without this API check, same-named repositories in + /// different accounts could receive a token under a misbound route. + pub async fn verify_owner(&self, expected_owner: &str) -> Result<(), String> { + let expected = expected_owner.to_lowercase(); + if self + .verified_owner + .lock() + .unwrap() + .as_ref() + .is_some_and(|(o, at)| *o == expected && at + VERIFY_TTL.as_secs() > unix_now()) + { + return Ok(()); + } + let jwt = self.sign_jwt()?; + let installation_id = self.resolve_installation(&jwt).await?; + let url = format!("{}/app/installations/{}", self.api_base, installation_id); + let resp = self + .http + .get(&url) + .header("Authorization", format!("Bearer {}", jwt)) + .header("Accept", "application/vnd.github+json") + .header("User-Agent", concat!("ghpool/", env!("CARGO_PKG_VERSION"))) + .send() + .await + .map_err(|e| format!("installation owner verification failed: {}", e))?; + if !resp.status().is_success() { + return Err(format!( + "installation owner verification failed: GitHub returned {}", + resp.status() + )); + } + let installation: Installation = resp + .json() + .await + .map_err(|e| format!("installation owner response parse failed: {}", e))?; + let actual = installation + .account + .ok_or_else(|| "installation owner response missing account".to_string())? + .login + .to_lowercase(); + if actual != expected { + return Err(format!( + "installation {} belongs to owner '{}', not configured owner '{}'", + installation_id, actual, expected_owner + )); + } + *self.verified_owner.lock().unwrap() = Some((actual, unix_now())); + Ok(()) + } + async fn resolve_installation(&self, jwt: &str) -> Result { if let Some(id) = *self.installation_id.lock().unwrap() { return Ok(id); @@ -371,7 +533,7 @@ pub(crate) mod tests { // Force near-expiry → refresh mints again p.cached.lock().unwrap().insert( - String::new(), + "mcp:".to_string(), AppToken { token: "ghs_mock_1".into(), expires_at: unix_now() + 10 }, ); let t3 = p.token().await.unwrap(); @@ -390,4 +552,298 @@ pub(crate) mod tests { assert_eq!(s3.token, "ghs_mock_4"); assert_eq!(MINTS.load(Ordering::SeqCst), 4); } + + #[tokio::test] + async fn test_git_token_downscopes_permissions_and_isolates_cache() { + use axum::{extract::State, routing::post, Json, Router}; + use std::sync::Arc; + + type Bodies = Arc>>; + async fn mint( + State(bodies): State, + Json(body): Json, + ) -> Json { + bodies.lock().unwrap().push(body); + let n = bodies.lock().unwrap().len(); + let exp = time::OffsetDateTime::from_unix_timestamp((unix_now() + 3600) as i64) + .unwrap() + .format(&time::format_description::well_known::Rfc3339) + .unwrap(); + Json(serde_json::json!({ + "token": format!("ghs_scope_{}", n), + "expires_at": exp, + })) + } + + let bodies: Bodies = Arc::new(Mutex::new(Vec::new())); + let app = Router::new() + .route("/app/installations/42/access_tokens", post(mint)) + .with_state(bodies.clone()); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + + let p = AppTokenProvider::new( + "123".into(), + TEST_RSA_PEM, + Some(42), + None, + format!("http://{}", addr), + ) + .unwrap(); + + // MCP token first: same repo, default App permissions. + let mcp = p.token_scoped(&["openab".into()]).await.unwrap(); + // Git token MUST mint separately despite identical repo envelope. + let git = p.token_git("openab").await.unwrap(); + assert_ne!(mcp.token, git.token); + // Repeated git lookup hits its own cache. + assert_eq!(p.token_git("openab").await.unwrap().token, git.token); + + let seen = bodies.lock().unwrap(); + assert_eq!(seen.len(), 2, "MCP and git cache namespaces must not overlap"); + assert_eq!(seen[0]["repositories"], serde_json::json!(["openab"])); + assert!(seen[0].get("permissions").is_none()); + assert_eq!(seen[1]["repositories"], serde_json::json!(["openab"])); + assert_eq!( + seen[1]["permissions"], + serde_json::json!({"contents": "write"}) + ); + } + + #[tokio::test] + async fn test_verify_owner_binds_explicit_installation_id() { + use axum::{routing::get, Router}; + use std::sync::atomic::{AtomicU32, Ordering}; + + static HITS: AtomicU32 = AtomicU32::new(0); + let app = Router::new().route( + "/app/installations/42", + get(|| async { + HITS.fetch_add(1, Ordering::SeqCst); + axum::Json(serde_json::json!({ + "id": 42, + "account": {"login": "openabdev"} + })) + }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + let p = AppTokenProvider::new( + "123".into(), + TEST_RSA_PEM, + Some(42), + Some("openabdev".into()), + format!("http://{}", addr), + ) + .unwrap(); + + p.verify_owner("OpenABdev").await.unwrap(); + assert_eq!(HITS.load(Ordering::SeqCst), 1); + // fresh verification is cached — no second API call + p.verify_owner("openabdev").await.unwrap(); + assert_eq!(HITS.load(Ordering::SeqCst), 1); + // a stale verification (past VERIFY_TTL) is re-checked with GitHub: + // orgs can rename, so the binding must not live forever + *p.verified_owner.lock().unwrap() = + Some(("openabdev".into(), unix_now() - VERIFY_TTL.as_secs() - 1)); + p.verify_owner("openabdev").await.unwrap(); + assert_eq!(HITS.load(Ordering::SeqCst), 2); + + let err = p.verify_owner("oablab").await.unwrap_err(); + assert!(err.contains("belongs to owner 'openabdev'")); + assert!(err.contains("not configured owner 'oablab'")); + } + + #[tokio::test] + async fn test_concurrent_misses_mint_once() { + use axum::{routing::post, Router}; + use std::sync::atomic::{AtomicU32, Ordering}; + use std::sync::Arc; + + static MINTS2: AtomicU32 = AtomicU32::new(0); + let app = Router::new().route( + "/app/installations/7/access_tokens", + post(|| async { + MINTS2.fetch_add(1, Ordering::SeqCst); + // slow mint so concurrent callers overlap the await + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + let exp = time::OffsetDateTime::from_unix_timestamp((unix_now() + 3600) as i64) + .unwrap() + .format(&time::format_description::well_known::Rfc3339) + .unwrap(); + axum::Json(serde_json::json!({"token": "ghs_single", "expires_at": exp})) + }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + + let p = Arc::new( + AppTokenProvider::new( + "123".into(), + TEST_RSA_PEM, + Some(7), + None, + format!("http://{}", addr), + ) + .unwrap(), + ); + let (a, b, c) = tokio::join!( + p.token_git("openab"), + p.token_git("openab"), + p.token_git("openab"), + ); + assert_eq!(a.unwrap().token, "ghs_single"); + assert_eq!(b.unwrap().token, "ghs_single"); + assert_eq!(c.unwrap().token, "ghs_single"); + assert_eq!( + MINTS2.load(Ordering::SeqCst), + 1, + "singleflight: concurrent misses must mint exactly once" + ); + } + + #[tokio::test] + async fn test_distinct_keys_mint_in_parallel() { + use axum::{routing::post, Json, Router}; + use std::sync::Arc; + use std::time::Instant; + + // The "slowrepo" mint stalls; "fastrepo" answers immediately. A + // slow mint for one key must not delay issuance for another key. + async fn mint(Json(body): Json) -> Json { + let repos = body["repositories"].as_array().cloned().unwrap_or_default(); + if repos.iter().any(|r| r == "slowrepo") { + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + } + let exp = time::OffsetDateTime::from_unix_timestamp((unix_now() + 3600) as i64) + .unwrap() + .format(&time::format_description::well_known::Rfc3339) + .unwrap(); + Json(serde_json::json!({ + "token": format!("ghs_{}", repos[0].as_str().unwrap()), + "expires_at": exp, + })) + } + let app = Router::new().route("/app/installations/8/access_tokens", post(mint)); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + + let p = Arc::new( + AppTokenProvider::new( + "123".into(), + TEST_RSA_PEM, + Some(8), + None, + format!("http://{}", addr), + ) + .unwrap(), + ); + let start = Instant::now(); + let slow = { + let p = p.clone(); + tokio::spawn(async move { p.token_git("slowrepo").await }) + }; + // give the slow mint a head start so its lock is held + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + let fast = p.token_git("fastrepo").await.unwrap(); + let fast_elapsed = start.elapsed(); + assert_eq!(fast.token, "ghs_fastrepo"); + assert!( + fast_elapsed < std::time::Duration::from_millis(400), + "distinct key must not wait behind a slow mint (took {:?})", + fast_elapsed + ); + let slow = slow.await.unwrap().unwrap(); + assert_eq!(slow.token, "ghs_slowrepo"); + assert!(start.elapsed() >= std::time::Duration::from_millis(500)); + // singleflight entries are evicted once mints complete — the map + // must not grow with every key ever requested + assert!(p.mint_locks.lock().unwrap().is_empty()); + } + + #[tokio::test] + async fn test_mint_locks_evicted_after_success_and_failure() { + use axum::{routing::post, Json, Router}; + + // "badrepo*" fails to mint; "goodrepo" succeeds. + async fn mint(Json(body): Json) -> axum::response::Response { + let repos = body["repositories"].as_array().cloned().unwrap_or_default(); + if repos.iter().any(|r| r.as_str().unwrap().starts_with("badrepo")) { + return axum::response::Response::builder() + .status(422) + .body(axum::body::Body::from("{\"message\":\"not found\"}")) + .unwrap(); + } + let exp = time::OffsetDateTime::from_unix_timestamp((unix_now() + 3600) as i64) + .unwrap() + .format(&time::format_description::well_known::Rfc3339) + .unwrap(); + axum::response::Response::builder() + .status(200) + .header("content-type", "application/json") + .body(axum::body::Body::from( + serde_json::json!({"token": "ghs_good", "expires_at": exp}).to_string(), + )) + .unwrap() + } + let app = Router::new().route("/app/installations/9/access_tokens", post(mint)); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + + let p = AppTokenProvider::new( + "123".into(), + TEST_RSA_PEM, + Some(9), + None, + format!("http://{}", addr), + ) + .unwrap(); + + p.token_git("goodrepo").await.unwrap(); + assert!(p.mint_locks.lock().unwrap().is_empty(), "evicted on success"); + + // A wildcard-allowlisted agent can request arbitrary names; failed + // mints must not leave lock entries behind (unbounded growth). + for i in 0..5 { + p.token_git(&format!("badrepo{}", i)).await.unwrap_err(); + } + assert!(p.mint_locks.lock().unwrap().is_empty(), "evicted on failure"); + } + + #[test] + fn test_evict_mint_lock_is_generation_guarded() { + let p = AppTokenProvider::new( + "123".into(), + TEST_RSA_PEM, + Some(1), + None, + "http://x".into(), + ) + .unwrap(); + let gen_a = std::sync::Arc::new(tokio::sync::Mutex::new(())); + let gen_b = std::sync::Arc::new(tokio::sync::Mutex::new(())); + + // Map holds generation B (a newer in-flight mint); a stale waiter + // from evicted generation A must NOT remove it. + p.mint_locks + .lock() + .unwrap() + .insert("k".into(), gen_b.clone()); + p.evict_mint_lock("k", &gen_a); + assert!( + p.mint_locks.lock().unwrap().contains_key("k"), + "a stale generation must never evict a newer generation's lock" + ); + // The owning generation removes itself. + p.evict_mint_lock("k", &gen_b); + assert!(p.mint_locks.lock().unwrap().is_empty()); + // Evicting an absent key is a no-op. + p.evict_mint_lock("k", &gen_b); + assert!(p.mint_locks.lock().unwrap().is_empty()); + } } diff --git a/src/audit.rs b/src/audit.rs index 921419c..f4b9541 100644 --- a/src/audit.rs +++ b/src/audit.rs @@ -98,6 +98,46 @@ impl AuditSink { })) } + /// Pre-flight git credential issuance record. This MUST be persisted + /// before owner verification or token mint/cache lookup. An unaudited + /// token mint is forbidden even when the token is never returned. + pub fn record_git_credential_request( + &self, + agent: &str, + credential: &str, + repo: &str, + ) -> Result<(), String> { + self.append(serde_json::json!({ + "ts": unix_now_ms(), + "phase": "git_credential_request", + "agent": agent, + "cred": credential, + "repo": repo, + "decision": "allow", + })) + } + + /// Result record after owner verification + mint/cache lookup. A failure + /// to persist a successful result rejects the credential response. + pub fn record_git_credential_result( + &self, + agent: &str, + credential: &str, + repo: &str, + success: bool, + expires_at: Option, + ) -> Result<(), String> { + self.append(serde_json::json!({ + "ts": unix_now_ms(), + "phase": "git_credential_result", + "agent": agent, + "cred": credential, + "repo": repo, + "success": success, + "expires_at": expires_at, + })) + } + /// Append one JSONL record and fsync. Small blocking write on the async /// path — acceptable: write calls are rare and records are <1 KB. fn append(&self, record: serde_json::Value) -> Result<(), String> { diff --git a/src/config.rs b/src/config.rs index 22ba145..d68a390 100644 --- a/src/config.rs +++ b/src/config.rs @@ -70,6 +70,14 @@ pub struct McpConfig { /// [mcp.audit] configured (writes are fail-closed audited). #[serde(default)] pub enable_writes: bool, + /// Enable the /git-credential endpoint: repository-scoped agents can + /// exchange their X-Ghpool-Key for a short-lived, single-repo GitHub App + /// installation token usable as a git-over-HTTPS credential + /// (username `x-access-token`). Hard requirements mirror enable_writes: + /// [[mcp.agents]], an App backend, and [mcp.audit] (every issuance is + /// fail-closed audited). The App needs Contents: read/write for pushes. + #[serde(default)] + pub enable_git_credentials: bool, /// Upstream MCP endpoint. Defaults to GitHub's hosted read-only variant, /// or the full write-capable surface when enable_writes is set. #[serde(default)] @@ -185,6 +193,7 @@ impl Default for McpConfig { Self { enabled: false, enable_writes: false, + enable_git_credentials: false, upstream: None, toolsets: Vec::new(), session_ttl_secs: default_mcp_session_ttl(), @@ -242,6 +251,27 @@ impl McpConfig { } } } + // Git credential issuance shares the write gate's hard requirements: + // authenticated agents, App-backed tokens (never PATs), and a + // fail-closed audit trail for every issuance. + if self.enable_git_credentials { + if self.agents.is_empty() { + return Err("enable_git_credentials requires [[mcp.agents]] — credentials are only issued to authenticated agents".into()); + } + if self.github_app.is_none() && self.github_apps.is_empty() { + return Err("enable_git_credentials requires [mcp.github_app] or [[mcp.github_apps]] — git credentials are App installation tokens, never PATs".into()); + } + if self.audit.is_none() { + return Err("enable_git_credentials requires [mcp.audit] — issuance is fail-closed audited".into()); + } + if self + .github_app + .as_ref() + .is_some_and(|app| app.owner.as_deref().is_none_or(|o| o.trim().is_empty())) + { + return Err("enable_git_credentials with [mcp.github_app] requires `owner` — explicit installation IDs are verified against this owner before issuance".into()); + } + } // Mutual exclusion: singular and plural forms cannot coexist if self.github_app.is_some() && !self.github_apps.is_empty() { return Err("[mcp.github_app] and [[mcp.github_apps]] are mutually exclusive — use one or the other".into()); @@ -676,6 +706,7 @@ mod tests { wa.tools = vec!["issue_read".into(), "create_issue".into()]; let m = McpConfig { enable_writes: true, + enable_git_credentials: false, github_apps: vec![entry("openabdev")], agents: vec![wa], audit: Some(AuditConfig { path: "/tmp/a.jsonl".into(), max_result_bytes: 1024 }), @@ -725,6 +756,7 @@ mod tests { let m = McpConfig { enabled: true, enable_writes: true, + enable_git_credentials: false, github_apps: vec![entry("openabdev"), entry("oablab")], agents: vec![multi_agent(&["openabdev/openab", "oablab/chi"])], audit: Some(AuditConfig { path: "/tmp/a.jsonl".into(), max_result_bytes: 1024 }), @@ -732,4 +764,91 @@ mod tests { }; assert!(m.validate().is_ok()); } + + #[test] + fn test_mcp_validate_git_credentials_gate() { + fn agent() -> McpAgentConfig { + McpAgentConfig { + id: "b0".into(), + key: None, + keys: vec!["k".into()], + tools: vec![], + repos: vec!["openabdev/openab".into()], + } + } + fn audit() -> Option { + Some(AuditConfig { path: "/tmp/a.jsonl".into(), max_result_bytes: 1024 }) + } + fn single(owner: Option<&str>) -> Option { + Some(GithubAppConfig { + app_id: "1".into(), + private_key: "pem".into(), + installation_id: Some(1), + owner: owner.map(|s| s.to_string()), + }) + } + + // agents required + let m = McpConfig { enable_git_credentials: true, ..Default::default() }; + assert!(m.validate().unwrap_err().contains("[[mcp.agents]]")); + + // App backend required — never PATs + let m = McpConfig { + enable_git_credentials: true, + agents: vec![agent()], + ..Default::default() + }; + assert!(m.validate().unwrap_err().contains("never PATs")); + + // audit required + let m = McpConfig { + enable_git_credentials: true, + agents: vec![agent()], + github_app: single(Some("openabdev")), + ..Default::default() + }; + assert!(m.validate().unwrap_err().contains("[mcp.audit]")); + + // singular App without owner: the explicit installation ID cannot be + // verified against an account, so startup must fail + for owner in [None, Some(""), Some(" ")] { + let m = McpConfig { + enable_git_credentials: true, + agents: vec![agent()], + github_app: single(owner), + audit: audit(), + ..Default::default() + }; + assert!( + m.validate().unwrap_err().contains("requires `owner`"), + "owner {:?} must be rejected", + owner + ); + } + + // singular App with owner set: valid + let m = McpConfig { + enable_git_credentials: true, + agents: vec![agent()], + github_app: single(Some("openabdev")), + audit: audit(), + ..Default::default() + }; + assert!(m.validate().is_ok()); + + // multi-App form (owners inherent to entries): valid + let m = McpConfig { + enable_git_credentials: true, + agents: vec![agent()], + github_apps: vec![GithubAppsEntry { + app_id: "1".into(), + private_key: "pem".into(), + installation_id: Some(1), + owner: "openabdev".into(), + }], + audit: audit(), + ..Default::default() + }; + assert!(m.validate().is_ok()); + } } diff --git a/src/git_credential.rs b/src/git_credential.rs new file mode 100644 index 0000000..6f86188 --- /dev/null +++ b/src/git_credential.rs @@ -0,0 +1,575 @@ +//! Git-over-HTTPS credential issuance (`/git-credential`). +//! +//! Repository-scoped agents exchange their `X-Ghpool-Key` for a short-lived +//! GitHub App installation token scoped to EXACTLY ONE repository, usable as +//! a git HTTPS credential (`x-access-token:`). This closes the last +//! long-lived-credential gap for agents: pushes authenticate as the App +//! (`[bot]`), expire within the hour, and every issuance is fail-closed +//! audited. +//! +//! Request: GET /git-credential?repo=/ (X-Ghpool-Key header) +//! Response: {"username":"x-access-token","password":"…","expires_at":…} +//! +//! Policy stack (all fail-closed): +//! key auth → repo-scoped agent → repo allowlist → installation coverage → +//! audited issuance → single-repo token mint (GitHub enforces the boundary). + +use axum::{ + extract::{Query, State}, + http::{HeaderMap, StatusCode}, + response::Response, +}; +use std::collections::HashMap; +use std::sync::Arc; + +use crate::mcp::{authenticate, rpc_error}; +use crate::AppState; + +pub async fn git_credential( + State(state): State>, + Query(params): Query>, + headers: HeaderMap, +) -> Response { + if !state.config.mcp.enable_git_credentials { + return rpc_error(StatusCode::NOT_FOUND, "git credentials are not enabled"); + } + // Authenticated agents only. Startup validation guarantees agents exist + // when the endpoint is enabled, so network-trust mode (None) is denied. + let agent = match authenticate(&state, &headers) { + Ok(Some(a)) => a, + Ok(None) => { + return rpc_error(StatusCode::UNAUTHORIZED, "agent authentication required") + } + Err(resp) => return *resp, + }; + + // Exactly one repository per credential: owner/name, strict shape AND + // strict charset (GitHub logins: alphanumeric + hyphen; repo names: + // alphanumeric + `-_.`). Percent-encoded or exotic input is rejected + // here — before the allowlist, audit preflight, or any mint attempt. + let Some((owner, name)) = params + .get("repo") + .and_then(|r| r.split_once('/')) + .filter(|(o, n)| { + !o.is_empty() + && !n.is_empty() + && o.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'-') + && n.bytes() + .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'-' | b'_' | b'.')) + }) + else { + return rpc_error(StatusCode::BAD_REQUEST, "repo=/ query required"); + }; + + // Repository-scoped agents only — a repo-less agent has no installation + // envelope, and git credentials are never PAT-backed. + if agent.repos.is_empty() { + tracing::warn!( + "git-credential DENIED (repo-less agent) [agent={}]", + agent.id + ); + return rpc_error( + StatusCode::FORBIDDEN, + "git credentials require a repository-scoped agent", + ); + } + if !crate::policy::repo_allowed(&agent.repos, owner, name) { + tracing::warn!( + "git-credential DENIED (repo {}/{} not allowlisted) [agent={}]", + owner, name, agent.id + ); + return rpc_error( + StatusCode::FORBIDDEN, + "repository not permitted by agent policy", + ); + } + + // Resolve the installation: multi-app routes by owner; single-app must + // match the configured owner when one is set. + let owner_key = owner.to_lowercase(); + let provider = if let Some(multi) = &state.multi_app_tokens { + match multi.get(&owner_key) { + Some(p) => p, + None => { + tracing::warn!( + "git-credential DENIED (no installation for owner {}) [agent={}]", + owner, agent.id + ); + return rpc_error( + StatusCode::FORBIDDEN, + "no GitHub App installation configured for repository owner", + ); + } + } + } else if let Some(single) = &state.app_tokens { + let Some(configured) = state + .config + .mcp + .github_app + .as_ref() + .and_then(|a| a.owner.as_deref()) + .filter(|o| !o.trim().is_empty()) + else { + // Startup validation rejects this; defense-in-depth for manually + // constructed state/tests. + return rpc_error( + StatusCode::FORBIDDEN, + "single-App git credentials require a configured owner", + ); + }; + if !configured.eq_ignore_ascii_case(owner) { + return rpc_error( + StatusCode::FORBIDDEN, + "no GitHub App installation configured for repository owner", + ); + } + single + } else { + // Unreachable: validation requires an App backend. + return rpc_error(StatusCode::BAD_GATEWAY, "no GitHub App backend configured"); + }; + + // Durable preflight BEFORE any owner verification, mint, or cache lookup. + // This ensures even a token minted but never returned has an audit trail. + let Some(sink) = &state.audit else { + return rpc_error(StatusCode::SERVICE_UNAVAILABLE, "audit backend unavailable"); + }; + let cred_label = format!("github-app:{}", owner_key); + let repo_label = format!("{}/{}", owner, name); + if let Err(e) = sink.record_git_credential_request(&agent.id, &cred_label, &repo_label) { + tracing::error!( + "audit unavailable — rejecting git-credential before mint (fail-closed): {}", + e + ); + return rpc_error( + StatusCode::SERVICE_UNAVAILABLE, + "audit backend unavailable — credential rejected", + ); + } + + // Bind the configured route label / explicit installation ID to the + // actual installation account returned by GitHub. Never trust config + // labels alone: same-named repos can exist under another owner. + if let Err(e) = provider.verify_owner(owner).await { + tracing::error!( + "git-credential owner verification failed for {}/{}: {}", + owner, name, e + ); + if let Err(audit_err) = sink.record_git_credential_result( + &agent.id, + &cred_label, + &repo_label, + false, + None, + ) { + tracing::error!("git-credential failure result audit failed: {}", audit_err); + } + return rpc_error(StatusCode::FORBIDDEN, "installation owner verification failed"); + } + + // Git-specific token: exactly one repository + contents:write only, + // with a cache namespace separate from MCP tokens. + let token = match provider.token_git(name).await { + Ok(t) => t, + Err(e) => { + tracing::error!( + "git-credential mint failed for {}/{}: {}", + owner, name, e + ); + if let Err(audit_err) = sink.record_git_credential_result( + &agent.id, + &cred_label, + &repo_label, + false, + None, + ) { + tracing::error!("git-credential failure result audit failed: {}", audit_err); + } + return rpc_error(StatusCode::BAD_GATEWAY, "credential mint failed"); + } + }; + + // Result record: if this cannot be persisted, do not return the token. + if let Err(e) = sink.record_git_credential_result( + &agent.id, + &cred_label, + &repo_label, + true, + Some(token.expires_at), + ) { + tracing::error!( + "audit result unavailable — rejecting git-credential response: {}", + e + ); + return rpc_error( + StatusCode::SERVICE_UNAVAILABLE, + "audit backend unavailable — credential rejected", + ); + } + + tracing::info!( + "git-credential issued for {}/{} [agent={} via {}] (expires_at={})", + owner, name, agent.id, cred_label, token.expires_at + ); + let body = serde_json::json!({ + "username": "x-access-token", + "password": token.token, + "expires_at": token.expires_at, + }); + Response::builder() + .status(StatusCode::OK) + .header("content-type", "application/json") + .header("cache-control", "no-store") + .body(axum::body::Body::from(body.to_string())) + .unwrap_or_else(|_| rpc_error(StatusCode::INTERNAL_SERVER_ERROR, "response build failed")) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{cache, config, pool}; + use axum::body::Body; + use axum::http::Request; + use tower::ServiceExt; + + type MintLog = Arc>>; + + async fn spawn_mock_github() -> (String, MintLog) { + use axum::extract::Path; + + async fn mint( + State(log): State, + Path(id): Path, + axum::Json(body): axum::Json, + ) -> axum::Json { + log.lock().unwrap().push((id, body)); + let exp = time::OffsetDateTime::from_unix_timestamp( + (std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() + + 3600) as i64, + ) + .unwrap() + .format(&time::format_description::well_known::Rfc3339) + .unwrap(); + axum::Json(serde_json::json!({ + "token": if id == 41 { "ghs_git_openabdev" } else { "ghs_git_oablab" }, + "expires_at": exp + })) + } + async fn installation(Path(id): Path) -> axum::Json { + axum::Json(serde_json::json!({ + "id": id, + "account": {"login": if id == 41 { "openabdev" } else { "oablab" }} + })) + } + + let log: MintLog = Arc::new(std::sync::Mutex::new(Vec::new())); + let app = axum::Router::new() + .route( + "/app/installations/{id}/access_tokens", + axum::routing::post(mint), + ) + .route( + "/app/installations/{id}", + axum::routing::get(installation), + ) + .with_state(log.clone()); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { axum::serve(listener, app).await.unwrap() }); + (format!("http://{}", addr), log) + } + + fn agent(id: &str, key: &str, repos: &[&str]) -> config::McpAgentConfig { + config::McpAgentConfig { + id: id.into(), + key: None, + keys: vec![key.into()], + tools: vec![], + repos: repos.iter().map(|s| s.to_string()).collect(), + } + } + + async fn test_state( + enabled: bool, + sink: Option, + ) -> (Arc, MintLog) { + let (gh, mint_log) = spawn_mock_github().await; + let entries = vec![ + config::GithubAppsEntry { + app_id: "111".into(), + private_key: crate::app_token::tests::TEST_RSA_PEM.into(), + installation_id: Some(41), + owner: "openabdev".into(), + }, + config::GithubAppsEntry { + app_id: "222".into(), + private_key: crate::app_token::tests::TEST_RSA_PEM.into(), + installation_id: Some(42), + owner: "oablab".into(), + }, + // Mislabeled on purpose: installation 43 actually belongs to + // "oablab" per the mock — owner verification must catch this. + config::GithubAppsEntry { + app_id: "333".into(), + private_key: crate::app_token::tests::TEST_RSA_PEM.into(), + installation_id: Some(43), + owner: "mislabeled".into(), + }, + ]; + let multi = crate::app_token::MultiAppTokenProvider::new(&entries, gh).unwrap(); + let cache_config = config::CacheConfig::default(); + ( + Arc::new(AppState { + pool: pool::PatPool::new(&[]), + cache: cache::Cache::new(&cache_config), + config: config::Config { + port: 8080, + identities: vec![], + allowed_owners: vec![], + cache: cache_config, + mcp: config::McpConfig { + enabled: true, + enable_writes: false, + enable_git_credentials: enabled, + upstream: None, + toolsets: vec![], + session_ttl_secs: 3600, + max_inflight_writes: 4, + agents: vec![ + agent( + "b0", + "key-b0", + &["openabdev/openab", "oablab/chi", "mislabeled/repo"], + ), + agent("norepo", "key-norepo", &[]), + agent("other", "key-other", &["otherorg/thing"]), + ], + github_app: None, + github_apps: entries, + audit: None, + }, + }, + token_users: moka::future::Cache::builder().max_capacity(10).build(), + http: reqwest::Client::new(), + mcp_sessions: moka::future::Cache::builder().max_capacity(10).build(), + app_tokens: None, + multi_app_tokens: Some(multi), + audit: sink, + write_inflight: Arc::new(std::sync::Mutex::new(std::collections::HashMap::new())), + }), + mint_log, + ) + } + + fn app(state: Arc) -> axum::Router { + axum::Router::new() + .route("/git-credential", axum::routing::get(git_credential)) + .with_state(state) + } + + fn req(repo: &str, key: Option<&str>) -> Request { + let mut b = Request::builder() + .method("GET") + .uri(format!("/git-credential?repo={}", repo)); + if let Some(k) = key { + b = b.header("x-ghpool-key", k); + } + b.body(Body::empty()).unwrap() + } + + fn audit_tmp(name: &str) -> String { + std::env::temp_dir() + .join(format!("ghpool-gitcred-{}-{}.jsonl", name, std::process::id())) + .to_str() + .unwrap() + .to_string() + } + + #[tokio::test] + async fn test_disabled_is_404() { + let (state, _) = test_state(false, None).await; + let resp = app(state) + .oneshot(req("openabdev/openab", Some("key-b0"))) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + + #[tokio::test] + async fn test_missing_or_bad_key_is_401() { + let path = audit_tmp("auth"); + let sink = crate::audit::AuditSink::open(&path).unwrap(); + let (state, mint_log) = test_state(true, Some(sink)).await; + for key in [None, Some("wrong")] { + let resp = app(state.clone()) + .oneshot(req("openabdev/openab", key)) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + } + assert!(mint_log.lock().unwrap().is_empty()); + std::fs::remove_file(&path).ok(); + } + + #[tokio::test] + async fn test_issues_single_repo_token_and_audits() { + let path = audit_tmp("ok"); + let sink = crate::audit::AuditSink::open(&path).unwrap(); + let (state, mint_log) = test_state(true, Some(sink)).await; + let resp = app(state) + .oneshot(req("openabdev/openab", Some("key-b0"))) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + assert_eq!(resp.headers().get("cache-control").unwrap(), "no-store"); + let body = axum::body::to_bytes(resp.into_body(), usize::MAX).await.unwrap(); + let v: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(v["username"], "x-access-token"); + assert_eq!(v["password"], "ghs_git_openabdev"); + assert!(v["expires_at"].as_u64().unwrap() > 0); + + // Mint envelope: routed to the openabdev installation, EXACTLY one + // repository, contents:write only — never the App's full permissions. + { + let minted = mint_log.lock().unwrap(); + assert_eq!(minted.len(), 1); + assert_eq!(minted[0].0, 41); + assert_eq!(minted[0].1["repositories"], serde_json::json!(["openab"])); + assert_eq!( + minted[0].1["permissions"], + serde_json::json!({"contents": "write"}) + ); + } + + // Two-phase audit: durable preflight, then a success result. + let records: Vec = std::fs::read_to_string(&path) + .unwrap() + .lines() + .map(|l| serde_json::from_str(l).unwrap()) + .collect(); + assert_eq!(records.len(), 2); + assert_eq!(records[0]["phase"], "git_credential_request"); + assert_eq!(records[0]["decision"], "allow"); + assert_eq!(records[1]["phase"], "git_credential_result"); + assert_eq!(records[1]["success"], true); + assert!(records[1]["expires_at"].as_u64().unwrap() > 0); + for r in &records { + assert_eq!(r["agent"], "b0"); + assert_eq!(r["cred"], "github-app:openabdev"); + assert_eq!(r["repo"], "openabdev/openab"); + // the token value itself is never audited + assert!(!r.to_string().contains("ghs_git_openabdev")); + } + std::fs::remove_file(&path).ok(); + } + + #[tokio::test] + async fn test_routes_by_owner() { + let path = audit_tmp("route"); + let sink = crate::audit::AuditSink::open(&path).unwrap(); + let (state, mint_log) = test_state(true, Some(sink)).await; + let resp = app(state) + .oneshot(req("oablab/chi", Some("key-b0"))) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + let body = axum::body::to_bytes(resp.into_body(), usize::MAX).await.unwrap(); + let v: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(v["password"], "ghs_git_oablab"); + let minted = mint_log.lock().unwrap(); + assert_eq!(minted.len(), 1); + assert_eq!(minted[0].0, 42, "must route to the oablab installation"); + assert_eq!(minted[0].1["repositories"], serde_json::json!(["chi"])); + std::fs::remove_file(&path).ok(); + } + + #[tokio::test] + async fn test_policy_denials() { + let path = audit_tmp("deny"); + let sink = crate::audit::AuditSink::open(&path).unwrap(); + let (state, mint_log) = test_state(true, Some(sink)).await; + // off-allowlist repo + let resp = app(state.clone()) + .oneshot(req("openabdev/secret-repo", Some("key-b0"))) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::FORBIDDEN); + // repo-less agent + let resp = app(state.clone()) + .oneshot(req("openabdev/openab", Some("key-norepo"))) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::FORBIDDEN); + // malformed repo params, including encoded and exotic-charset forms + // (%2F decodes back to '/'; charset validation rejects the rest) + for bad in [ + "justanowner", + "openabdev%2Fopenab%2Fx", + "openabdev/", + "/openab", + "openabdev/open%20ab", // decodes to a space + "openabdev/open+ab", // '+' decodes to a space + "open~abdev/openab", // invalid owner charset + ] { + let resp = app(state.clone()) + .oneshot(req(bad, Some("key-b0"))) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST, "repo={}", bad); + } + // allowlisted repo whose owner has no App installation + let resp = app(state.clone()) + .oneshot(req("otherorg/thing", Some("key-other"))) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::FORBIDDEN); + // denials never mint, never audit + assert!(mint_log.lock().unwrap().is_empty()); + assert!(std::fs::read_to_string(&path).unwrap().is_empty()); + std::fs::remove_file(&path).ok(); + } + + #[tokio::test] + async fn test_owner_mismatch_fails_closed() { + let path = audit_tmp("mismatch"); + let sink = crate::audit::AuditSink::open(&path).unwrap(); + let (state, mint_log) = test_state(true, Some(sink)).await; + // Config labels installation 43 as "mislabeled" but GitHub says the + // installation account is "oablab" — the label must not be trusted. + let resp = app(state) + .oneshot(req("mislabeled/repo", Some("key-b0"))) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::FORBIDDEN); + // verification failure must never reach the mint endpoint + assert!(mint_log.lock().unwrap().is_empty()); + // audited: preflight + failed result + let records: Vec = std::fs::read_to_string(&path) + .unwrap() + .lines() + .map(|l| serde_json::from_str(l).unwrap()) + .collect(); + assert_eq!(records.len(), 2); + assert_eq!(records[0]["phase"], "git_credential_request"); + assert_eq!(records[1]["phase"], "git_credential_result"); + assert_eq!(records[1]["success"], false); + assert!(records[1]["expires_at"].is_null()); + std::fs::remove_file(&path).ok(); + } + + #[tokio::test] + async fn test_audit_fail_closed() { + let sink = crate::audit::AuditSink::failing_for_tests(); + let (state, mint_log) = test_state(true, Some(sink)).await; + let resp = app(state) + .oneshot(req("openabdev/openab", Some("key-b0"))) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE); + // a failed audit preflight must never reach the mint endpoint + assert!(mint_log.lock().unwrap().is_empty()); + } +} diff --git a/src/main.rs b/src/main.rs index dad926e..742f285 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2,6 +2,7 @@ mod app_token; mod audit; mod cache; mod config; +mod git_credential; mod mcp; mod policy; mod pool; @@ -103,12 +104,12 @@ async fn main() { write_inflight: Arc::new(std::sync::Mutex::new(std::collections::HashMap::new())), }); - let mut app = Router::new() - .route("/healthz", get(healthz)) - .route("/stats", get(stats)) - .route("/graphql", post(graphql_proxy)) - .route("/raw/{*path}", get(proxy_raw)) - .route("/{*path}", get(proxy)); + let mut app = base_router(); + + if config.mcp.enable_git_credentials { + config.mcp.validate().expect("invalid [mcp] config"); + tracing::info!("git credential issuance enabled → /git-credential"); + } if config.mcp.enabled { config.mcp.validate().expect("invalid [mcp] config"); @@ -133,6 +134,20 @@ async fn main() { axum::serve(listener, app).await.unwrap(); } +/// The base route table shared by production and tests. `/git-credential` is +/// always registered as an exact route so it takes precedence over the +/// `/{*path}` GitHub REST catch-all: when issuance is disabled the handler +/// returns its explicit 404 and the request is never proxied upstream. +fn base_router() -> Router> { + Router::new() + .route("/healthz", get(healthz)) + .route("/stats", get(stats)) + .route("/graphql", post(graphql_proxy)) + .route("/git-credential", get(git_credential::git_credential)) + .route("/raw/{*path}", get(proxy_raw)) + .route("/{*path}", get(proxy)) +} + async fn healthz() -> &'static str { "ok" } @@ -456,12 +471,8 @@ mod tests { } fn app(state: Arc) -> axum::Router { - axum::Router::new() - .route("/healthz", axum::routing::get(healthz)) - .route("/stats", axum::routing::get(stats)) - .route("/raw/{*path}", axum::routing::get(proxy_raw)) - .route("/{*path}", axum::routing::get(proxy)) - .with_state(state) + // the production route table — ordering/precedence is under test + base_router().with_state(state) } #[tokio::test] @@ -516,4 +527,29 @@ mod tests { assert!(is_allowed_path("/rate_limit", &owners)); assert!(is_allowed_path("/user", &owners)); } + + #[tokio::test] + async fn test_git_credential_disabled_never_reaches_github_proxy() { + // Default McpConfig has enable_git_credentials = false. The exact + // /git-credential route must win over the /{*path} catch-all so the + // request is answered locally (404) and never proxied to GitHub. + let state = test_state(vec!["openabdev"]); + let resp = app(state) + .oneshot( + Request::builder() + .uri("/git-credential?repo=openabdev/openab") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + let body = axum::body::to_bytes(resp.into_body(), usize::MAX).await.unwrap(); + let text = String::from_utf8_lossy(&body); + assert!( + text.contains("not enabled"), + "expected the local handler's message, got: {}", + text + ); + } } diff --git a/src/mcp.rs b/src/mcp.rs index 42900e2..6fe4cb8 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -1245,7 +1245,7 @@ impl Drop for InFlightGuard { /// - No [[mcp.agents]] configured → Phase 1 network-trust mode: Ok(None). /// - Agents configured → every request must present a valid X-Ghpool-Key; /// missing or unknown keys get 401 with a JSON-RPC error body. -fn authenticate<'a>( +pub(crate) fn authenticate<'a>( state: &'a AppState, headers: &HeaderMap, ) -> Result, Box> { @@ -1284,7 +1284,7 @@ fn audit_via(agent: Option<&crate::config::McpAgentConfig>, identity_id: &str) - /// Minimal JSON-RPC error body for proxy-level failures, so MCP clients that /// only speak JSON-RPC degrade gracefully. -fn rpc_error(status: StatusCode, message: &str) -> Response { +pub(crate) fn rpc_error(status: StatusCode, message: &str) -> Response { let body = serde_json::json!({ "jsonrpc": "2.0", "id": null, @@ -1451,6 +1451,7 @@ mod tests { mcp: config::McpConfig { enabled: true, enable_writes: false, + enable_git_credentials: false, upstream: Some(upstream.to_string()), toolsets: toolsets.iter().map(|s| s.to_string()).collect(), session_ttl_secs: 3600, @@ -2333,6 +2334,7 @@ mod tests { mcp: config::McpConfig { enabled: true, enable_writes: false, + enable_git_credentials: false, upstream: Some(upstream.to_string()), toolsets: vec![], session_ttl_secs: 3600, @@ -2462,6 +2464,7 @@ mod tests { mcp: config::McpConfig { enabled: true, enable_writes: false, + enable_git_credentials: false, upstream: Some(upstream.to_string()), toolsets: vec![], session_ttl_secs: 3600, @@ -2652,6 +2655,7 @@ mod tests { mcp: config::McpConfig { enabled: true, enable_writes: true, + enable_git_credentials: false, upstream: Some(upstream.to_string()), toolsets: vec![], session_ttl_secs: 3600, @@ -2946,6 +2950,7 @@ mod tests { mcp: config::McpConfig { enabled: true, enable_writes, + enable_git_credentials: false, upstream: Some(upstream.to_string()), toolsets: vec![], session_ttl_secs: 3600,