feat: git-over-HTTPS credentials via App tokens

[chaodu-agent]

Summary

MCP now covers issues/PRs, but git push speaks the Git protocol and needs a real credential at push time — today that means a long-lived PAT or user OAuth session in the agent container. This PR closes that gap: GET /git-credential exchanges an agent's X-Ghpool-Key for a short-lived, single-repo GitHub App installation token, and ghp git-credential plugs it into git as a standard credential helper.

git push → ghp git-credential (GHPOOL_KEY)
         → GET /git-credential?repo=owner/name (X-Ghpool-Key)
         → key auth → repo allowlist → installation routing
         → fail-closed audit (phase: git_credential)
         → single-repo token (~1h, GitHub-enforced)
         → push authenticated as <app>[bot]

Design

• Opt-in via [mcp].enable_git_credentials, hard-gated at startup exactly like enable_writes: authenticated agents + App backend + [mcp.audit].
• Single-repo mints — the tightest scope GitHub can enforce; wildcard allowlist entries still resolve to one concrete repo per credential.
• Deny-by-default — repo-less agents (PAT-riding), off-allowlist repos, malformed repo params, and owners without an installation are refused before any mint or audit write.
• Fail-closed audit — no persisted issuance record, no credential (503). Record carries agent, installation (github-app:<owner>), repo, and expiry — never the token value.
• Helper ergonomics — store/erase no-ops; quiet decline (exit 1) for non-github hosts, missing GHPOOL_KEY, or missing path so git falls through to other helpers.

Tested

ghpool: clippy -D warnings clean; 105 passed / 0 failed — 6 new: disabled→404, missing/bad key→401, single-repo mint + audit record (token absent from log), owner routing across installations, policy denial matrix (no mint/no audit on deny), audit fail-closed→503. ghp: clippy clean; 14 passed / 0 failed — credential-protocol parsing and owner/repo path extraction (.git suffix, extra segments, malformed).

Not run: a live push against GitHub (needs Contents: R&W granted to an installed App); will validate during deployment.

Review Contract

• Scope: new opt-in endpoint + shim subcommand; no changes to MCP proxying, REST/GraphQL, caching, or PAT pooling.
• Acceptance: (1) endpoint absent (404) unless explicitly enabled; (2) issuance impossible without agent auth + repo allowlist + installation coverage + persisted audit; (3) tokens single-repo and short-lived; (4) token values never logged/audited; (5) both crates clippy clean, tests green.
• Deployment follow-up (separate change): grant the App Contents: R&W, enable the flag, configure B0's git credential helper, purge the OAuth hosts.yml from the home tarball.

[chaodu-agent]

Important

CHANGES REQUESTED ⚠️ — the server-side issuance path is solid, but the client helper's host matching can silently hand a recognized GitHub push to a broader credential helper, defeating the feature's core guarantee.

What This PR Does

Adds Git-over-HTTPS credentials backed by GitHub App installation tokens: an authenticated, repository-scoped agent calls GET /git-credential?repo=owner/name and receives a short-lived, single-repo, Contents-only token; ghp git-credential plugs that into git's standard credential-helper protocol. Closes the last long-lived-credential gap (git push no longer needs a PAT/OAuth token in the agent container).

How It Works

Policy stack, all fail-closed: key auth → strict repo parse → agent repo allowlist → installation routing (multi-App by owner, or singular App with required owner) → durable audit preflight → installation account verified against GitHub (GET /app/installations/{id}) → Contents-only single-repo mint in a git-specific cache namespace → audit result → token. Helper side, recognized github.com requests that can't be served emit quit=true so git stops the helper cascade.

Findings

#	Severity	Finding	Location
1	🔴	Helper matches host by exact string github.com; host=GitHub.com or host=github.com:443 (git passes URL case and explicit ports through) falls into the Decline path — git then consults broader helpers (osxkeychain/GCM/gh auth) for what is actually a GitHub push, bypassing ghpool policy and audit	ghp/src/main.rs:282
2	🟡	verify_owner caches the verified owner for process lifetime; GitHub orgs can rename and the freed name can be re-claimed, leaving a stale owner→installation binding until restart	src/app_token.rs:47,252
3	🟡	Server validates only shape (owner/name, no empty, no extra slash, trim); no charset validation — percent-encoded garbage or non-GitHub-charset names pass the deny-before-mint gate and reach audit preflight + mint attempt. Helper also interpolates repo into the query string without URL encoding	src/git_credential.rs:47-53, ghp/src/main.rs (fetch_git_credential)
4	🟡	CI paths filter only covers src/**, Cargo.toml, Cargo.lock, and the root crate is not a workspace — ghp/** changes are never built, linted, or tested in CI. The helper's fail-closed contract (the whole point of finding 1) has no CI enforcement; there is also no end-to-end test asserting the binary actually prints quit=true	.github/workflows/ci.yml:4-7
5	🟡	token_for does check-mint-insert without singleflight: concurrent cache misses for the same key mint redundant tokens. Each is audited and short-lived, so impact is waste + audit noise, not a security hole	src/app_token.rs (token_for)
6	🟢	Two-phase audit (durable preflight before any mint/cache lookup, result before token return), owner verification against the installation's actual account, permission downscoping to contents:write, and cache-namespace isolation are all correctly implemented and well-tested — the endpoint tests assert mint bodies, zero-mint on preflight failure, and token absence from audit records	src/git_credential.rs, src/app_token.rs

Finding Details

🔴 F1: host matching misses legitimate github.com variants

Git's credential protocol passes the host from the remote URL: a remote configured as https://GitHub.com/owner/repo.git or https://github.com:443/owner/repo.git produces host=GitHub.com / host=github.com:443. credential_plan compares against the exact string "github.com", so these fall into Decline (exit 1) — git then tries the next helper or prompts. Any broader stored credential (keychain OAuth, GCM) would be used for the push, silently bypassing repo policy and audit. This is exactly the fallback the quit=true design exists to prevent.

Fix: lowercase the host, split off the port; hostname != "github.com" → Decline, github.com with port other than 443 → Quit (recognized but unsupported), otherwise proceed. Add regression tests for GitHub.com, github.com:443, and github.com:8443.

🟡 F2: verify_owner result cached forever

The verified-owner cache never expires. GitHub organizations can rename, and the released name can be claimed by another account; a long-running ghpool process would keep issuing tokens under a stale binding. Add a TTL (re-verify on the order of the token lifetime, e.g. hourly) so the binding is re-checked against GitHub periodically.

🟡 F3: no charset validation on repo owner/name

GitHub owners match [A-Za-z0-9-]+ and repo names [A-Za-z0-9._-]+. Enforcing that at the parse step (400 before allowlist/preflight) keeps garbage out of the audit log and off the mint path, and makes the unencoded query interpolation in the helper provably safe. Apply the same charset check in ghp (Quit on violation).

🟡 F4: ghp crate invisible to CI

ci.yml triggers only on src/**/Cargo.toml/Cargo.lock and runs cargo at the root; ghp is a separate crate, not a workspace member. A regression in the helper — including in the fail-closed logic — would merge green. Add ghp/** to the paths filter and a step that runs clippy + tests in ghp/. Add an integration test that spawns the real binary (CARGO_BIN_EXE) and asserts quit=true on stdout for the recognized-failure case and exit 1 for non-GitHub hosts.

🟡 F5: redundant concurrent mints

Two simultaneous cache misses for the same namespace key both mint. Cheap fix: double-checked locking around the mint (async mutex per provider). Not a security issue — every mint is audited and expires — but it adds avoidable GitHub API calls and audit noise.

Baseline Check

• origin/main has no git-credential code — the feature is entirely net-new value.
• Head 15d5dc3 already incorporates the round-1 security review (owner binding, permission downscoping, two-phase audit, fail-closed helper, route precedence); this round reviews the incremental changes and found the above. F1 is NEW EVIDENCE (host variant behavior of git's credential protocol); F2–F5 are hardening/regression-coverage items on the new code.

Verified

At head 15d5dc36db6a29510bdfe7797f0eafda78b1dd06 in a pristine clone (arm64 macOS):

• root: cargo clippy --all-targets -- -D warnings clean; cargo test → 110 passed, 0 failed
• ghp: cargo clippy --all-targets -- -D warnings clean; cargo test → 19 passed, 0 failed
• GitHub CI check: passing (but see F4 — it never compiled ghp)

5️⃣ Three Reasons We Might Not Need This PR

1. Keep the OAuth token — B0 already pushes via gh auth git-credential. But that token is long-lived, all-repo, all-scope, and unaudited: the exact anti-pattern the MCP work eliminated everywhere else.
2. Deploy keys per repo — SSH deploy keys are single-repo too. But they're still long-lived static secrets needing per-repo lifecycle management, and they bypass the agent-key policy/audit plane entirely.
3. Push via MCP file APIs — the Contents API can commit without git. But it can't do real merges, force-with-lease, or multi-commit branches; agents need actual git push.

The approach stands. Fix F1 (and ideally F2–F5) before merge.

[chaodu-agent]

Fixes for review round 2 (commit 08c98c7)

All findings from the review addressed:

🔴 F1: helper matches host by exact string github.com

✅ Fixed in 08c98c7. credential_plan lowercases the host and splits the port: any github.com hostname is recognized — GitHub.com / GITHUB.COM / github.com:443 proceed to Fetch (or Quit when unservable), and github.com on a non-443 port Quits (recognized but unsupported). Only genuinely different hostnames decline. Regression tests: test_plan_recognizes_github_host_variants (unit, includes the never-decline assertion per variant) and recognized_github_host_variants_quit_on_failure (integration, real binary).

🟡 F2: verify_owner caches the verified owner for process lifetime

✅ Fixed. Verification is cached with a timestamp and re-checked against GET /app/installations/{id} after a 1-hour TTL (VERIFY_TTL). Test extends test_verify_owner_binds_explicit_installation_id: counts API hits — cached verify makes no call, a backdated-stale entry triggers re-verification.

🟡 F3: no charset validation on repo owner/name; helper interpolates repo un-encoded

✅ Fixed. Server: owner must match [A-Za-z0-9-]+, name [A-Za-z0-9._-]+ — 400 before the allowlist, audit preflight, or any mint (test_policy_denials now covers %20, +-as-space, and invalid-charset owners, asserting zero mints and an empty audit log). Helper: the same charset enforced in repo_from_path, so the value interpolated into the query string is provably URL-safe; violations Quit (test_plan_quits_for_recognized_github_failures covers percent-encoding, whitespace, non-ASCII).

🟡 F4: CI excludes ghp/**; no end-to-end quit=true assertion

✅ Fixed. ci.yml paths now include ghp/** (and the workflow itself); new steps run clippy -D warnings + tests in ghp/ with rust-cache covering both crate roots. New ghp/tests/git_credential_cli.rs spawns the built binary: asserts stdout is exactly quit=true\n + exit 0 for missing-key / host-variant / missing-path failures, empty stdout + exit 1 for non-GitHub hosts, and no-op store/erase.

🟡 F5: token_for check-mint-insert without singleflight

✅ Fixed. Double-checked locking around the mint via a per-provider async mutex. test_concurrent_misses_mint_once races three token_git calls against a slow mock mint and asserts exactly one mint.

Validation (macmini, arm64)

• ghpool: clippy -D warnings clean, 111 tests passed
• ghp: clippy -D warnings clean, 20 unit + 5 integration tests passed

[chaodu-agent]

Fix for review round 3 (commit 508dcc3)

Round-3 review verified all round-2 fixes (F1–F5) and found one regression introduced by the F5 singleflight:

MAJOR: one async mint mutex per provider, held across mint network I/O — distinct repository keys and MCP/git purpose namespaces on the same provider serialize; reqwest client has no timeout, so one stalled mint can block unrelated credential/MCP token issuance for that owner.

✅ Fixed in 508dcc3.

• Singleflight is now key-scoped: a map of Arc<tokio::sync::Mutex<()>> per cache key. Same-key concurrent misses still mint exactly once (test_concurrent_misses_mint_once retained); distinct keys mint in parallel.
• The App API client now has a 30s hard timeout (MINT_TIMEOUT) covering token mints, installation resolution, and owner verification — a stalled connection can no longer hold a waiter queue indefinitely.
• New regression test test_distinct_keys_mint_in_parallel: a mint stalled 500ms for one key must not delay another key's issuance (asserted <400ms while the slow mint is in flight).

Validation (macmini, arm64)

• ghpool: clippy -D warnings clean, 112 tests passed
• ghp: clippy -D warnings clean, 20 unit + 5 integration tests passed