Commit a61de55
authored
feat: git-over-HTTPS credentials via App tokens (#43)
* feat: git-over-HTTPS credentials via App tokens (/git-credential + ghp helper)
Closes the last long-lived GitHub credential gap for agents: git push
now authenticates with a short-lived, single-repo GitHub App installation
token instead of a PAT or user OAuth session.
Server (/git-credential, GET, opt-in via [mcp].enable_git_credentials):
- Same hard gate as MCP writes: [[mcp.agents]] + App backend + [mcp.audit],
validated at startup
- Policy stack, all fail-closed: X-Ghpool-Key auth → repository-scoped
agents only → repo allowlist → installation routing (multi-app by owner,
single-app owner match) → fail-closed audit record → single-repo token
mint (GitHub enforces the boundary)
- Response { username: x-access-token, password, expires_at } with
cache-control: no-store; token value never audited
Client (ghp git-credential):
- Standard git credential helper protocol (get/store/erase); get exchanges
GHPOOL_KEY for a token, store/erase are no-ops
- Declines quietly (exit 1) on missing key/path/non-github hosts so git
can fall through to other helpers; requires credential.useHttpPath
Verified: ghpool clippy -D warnings clean, 105 passed / 0 failed (6 new);
ghp clippy clean, 14 passed / 0 failed (2 new)
* fix(git-credential): bind installations to owners, downscope tokens, fail closed everywhere
Hardening from self-review of #43:
- BLOCKER: verify_owner() binds explicit installation IDs to the actual
installation account (GET /app/installations/{id}) before any issuance;
mislabeled config entries are refused (403) and audited
- MAJOR: git tokens mint with permissions={contents:write} and a
git-specific cache namespace — never the App's full permission set,
never shared with MCP token cache entries
- MAJOR: ghp helper emits quit=true for every unservable recognized
github.com request (missing key/path, policy denial, network failure)
so git never falls through to broader stored helpers; non-GitHub hosts
still decline quietly; gist.github.com unsupported
- MAJOR: two-phase audit — durable request preflight BEFORE owner
verification/mint/cache lookup, result record before the token is
returned; either write failing rejects the credential (503)
- singular [mcp.github_app] now requires owner when git credentials are
enabled (startup validation + defense-in-depth in the handler)
- /git-credential always registered before the /{*path} catch-all;
disabled mode answers 404 locally instead of proxying to GitHub
Tests: mint-envelope/cache-isolation, owner verification (unit +
endpoint mismatch), two-phase audit records, no-mint-on-preflight-
failure, production route precedence, helper plan/fetch fail-closed
matrix, config validation gates. README/config.example document the
--replace-all helper reset and quit=true semantics.
* fix(git-credential): normalize helper host matching, TTL owner re-verification, strict charset, CI for ghp
Round-2 review fixes:
- BLOCKER: helper host matching is now case-insensitive and
port-aware — host=GitHub.com and github.com:443 are recognized
github.com requests (Fetch/Quit), never declined to a broader
helper; github.com on a non-443 port quits (recognized but
unsupported)
- MAJOR: verify_owner caches with a 1h TTL and re-checks the
installation account with GitHub after expiry (orgs can rename)
- MAJOR: strict charset validation of repo owner/name on the server
(400 before allowlist/preflight/mint) and in the helper's
repo_from_path (quit) — percent-encoded/exotic input never reaches
the audit log, the mint path, or an unencoded query string
- MAJOR: CI now builds/lints/tests the ghp crate, plus an
integration test suite that spawns the real binary and asserts
quit=true on stdout for recognized-github failure modes and quiet
decline (exit 1) for other hosts
- MINOR: token mint is singleflight — concurrent cache misses for
the same key mint exactly once (asserted by test)
Tests: ghpool 111, ghp 20 unit + 5 integration, clippy clean both.
* fix(app-token): key-scoped singleflight and hard mint timeout
Round-3 review: the per-provider singleflight mutex was held across
mint network I/O, serializing distinct cache keys (different repos,
MCP vs git purposes) behind one another — and the reqwest client had
no timeout, so one stalled mint could block all issuance for an owner
indefinitely.
- Singleflight is now per cache key (Arc<tokio::sync::Mutex> map):
same-key misses still mint exactly once; distinct keys mint in
parallel
- App API client now has a 30s hard timeout (MINT_TIMEOUT) covering
mint, installation resolution, and owner verification calls
- New test: a stalled mint for one key must not delay another key
(test_distinct_keys_mint_in_parallel); same-key singleflight test
retained
Tests: ghpool 112, ghp 20 unit + 5 integration, clippy clean both.
* fix(app-token): evict singleflight lock entries when mints complete
Round-4 review: mint_locks entries were never removed, so unique
request-supplied repo names (e.g. failed mints under a wildcard
allowlist) accumulated lock entries for the process lifetime —
unbounded memory growth on an authenticated path.
Entries are now evicted after the mint completes (success or
failure): waiters already holding the Arc still serialize and
re-check the cache; the next fresh miss creates a new lock. The map
is bounded by in-flight mints.
Test: test_mint_locks_evicted_after_success_and_failure (success,
repeated failures, and the parallel test now asserts an empty map).
Tests: ghpool 113, ghp 20 unit + 5 integration, clippy clean.
* fix(app-token): generation-guard singleflight eviction
Round-5 review: unconditional remove(&key) let a stale waiter from an
evicted generation remove a NEWER generation's in-flight lock,
re-opening the duplicate-mint race repeatedly.
evict_mint_lock() now removes the entry only when the map still holds
the caller's own Arc (ptr_eq guard): each generation removes at most
itself, bounding duplicate mints to one per evicted generation while
keeping the map bounded by in-flight mints.
Test: test_evict_mint_lock_is_generation_guarded (stale generation
cannot evict, owner can, absent-key no-op).
Tests: ghpool 114, ghp 20 unit + 5 integration, clippy clean.
---------
Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>1 parent 072e16d commit a61de55
11 files changed
Lines changed: 1792 additions & 27 deletions
File tree
- .github/workflows
- ghp
- src
- tests
- src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
| 10 | + | |
9 | 11 | | |
10 | 12 | | |
11 | 13 | | |
| |||
19 | 21 | | |
20 | 22 | | |
21 | 23 | | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
22 | 28 | | |
23 | 29 | | |
24 | 30 | | |
25 | 31 | | |
26 | 32 | | |
27 | 33 | | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
459 | 459 | | |
460 | 460 | | |
461 | 461 | | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
462 | 530 | | |
463 | 531 | | |
464 | 532 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
50 | 62 | | |
51 | 63 | | |
52 | 64 | | |
| |||
0 commit comments