feat(rhodibot): authenticate the REST client as a GitHub App - #539
Merged
Merged
Conversation
App authentication existed but nothing used it: GitHubClient still read a single GITHUB_TOKEN, so every repository request went out as one identity. Credentials are now resolved once, in the client constructor: - app_id + private key -> GitHub App. Each request is scoped to the repository it concerns, because installation tokens do not carry across repositories. - otherwise GITHUB_TOKEN, otherwise anonymous (unchanged behaviour). A half-configured App is refused rather than downgraded. Setting app_id with no key (or a key with no app_id) leaves the client "misconfigured": readiness() fails, so main.rs exits at start-up instead of serving traffic, and authorize() refuses to send the request at all. Quietly falling back to anonymous access would turn a config mistake into a permissions puzzle at the first webhook. /health now names the credential mode so an operator can see what was picked up. Two defects that only bite at fleet scale are fixed in AppAuth while wiring it: the token cache held one installation, so a sweep across owners re-minted a token on nearly every request; and the installation lookup - an API call billed to the App's own much smaller rate-limit budget - was repeated per request. Both are now keyed caches. file_exists reports a boolean by contract, so an authentication failure is logged and reported as absent rather than silently mass-reporting non-compliance. Tests: 5 new unit tests (100 total, 50 unit + 50 integration), including the handshake end to end against a mock server that answers only to the installation token, reuse across requests, and the two misconfiguration paths.
Contributor
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
hyperpolymath
added a commit
that referenced
this pull request
Sep 19, 2026
) **Depends on #539** — this branch is built on it, because the deployment assumes the start-up credential gate that #539 adds. Merge that first; if it squashes, this branch will need a rebase onto `main` (the deploy files are all new, so it is mechanical). The tunnel is the public origin; the host stays unreachable. `cloudflared` dials out to Cloudflare's edge and forwards back down that connection to a loopback origin — no inbound port, no certificate on this host. **Loopback by default.** The origin previously bound `0.0.0.0` unconditionally. A tunnel-fronted service that also listens on every interface is published twice, and the second publication has no webhook secret in front of it. `--bind 0.0.0.0` / `BIND_ADDR=0.0.0.0` restores the old behaviour. **Ingress publishes exactly two routes:** `POST /webhook` and `GET /health`. Everything else is `http_status:404`, including any route added to the router later without a matching rule — the default is denial. `GET /api/check/{owner}/{repo}` is deliberately *not* published: it spends GitHub API quota and reports on repositories. | file | purpose | | --- | --- | | `deploy/cloudflared/rhodibot.yml` | tunnel ingress, with the two placeholders to replace | | `deploy/systemd/rhodibot.service` | the bot: loopback, no writable paths, `ProtectHome`, read-only `/etc` | | `deploy/systemd/cloudflared-rhodibot.service` | the tunnel; `Requires=rhodibot.service` so a tunnel is never up without an origin behind it | | `deploy/rhodibot.env.example` | App credentials as environment variables | | `deploy/GITHUB-APP-REGISTRATION.adoc` | the one manual step, written as a form | | `deploy/RHODIBOT-DEPLOYMENT.adoc` | runbook, in the order the steps have to happen | | `deploy/verify-rhodibot.sh` | proof of life, outside in | **Verification, proven not asserted.** `deploy/verify-rhodibot.sh` checks the origin's health and credential mode, that the process listens on loopback only, that the tunnel reaches it, that an unsigned delivery is refused with 401, and that the private endpoints 404. Run here against a live server, both directions: ``` PASS GET /health answers PASS /health reports credentials: anonymous (public repositories only) PASS rhodibot (pid 20547) listens on loopback only: 127.0.0.1:3000 ``` ``` FAIL rhodibot (pid 20604) is listening on a non-loopback address: 0.0.0.0:3001 ``` The second run is a server deliberately started with `--bind 0.0.0.0`; a check that cannot fail is not a check. The first draft of this script *did* fail vacuously — it asked "who is listening on port 3000?" and flagged an unrelated process that happened to share the port number. It now asks what it means: where is rhodibot listening? The unsigned-POST check is the one that matters most: with no webhook secret configured the handler accepts anything, so a 401 proves both that the tunnel reaches the origin and that the secret is set. **Start-up gate, observed:** `GITHUB_APP_ID=123 rhodibot` (no key) exits 1 with `Error: GITHUB_APP_ID is set but no private key was provided`, rather than serving unauthenticated traffic. What remains human: the App registration in the browser (GitHub has no API for it), the tunnel UUID, the zone hostname, and the `GITHUB_APP_ID` in `/etc/fleet/rhodibot.env`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
App authentication landed in #538 but nothing called it: the REST client still
sent a single
GITHUB_TOKENfor every repository.Credentials are resolved once, in the client constructor
app_id+ private keyGITHUB_TOKENA half-configured App is refused, not downgraded.
app_idwithout a key (orthe reverse) leaves the client
misconfigured:readiness()fails somain.rsexits at start-up, and
authorize()refuses to issue the request. Falling backto anonymous silently would turn a config mistake into a permissions puzzle at
the first webhook.
/healthnow names the credential mode.Two defects that only appear at fleet scale, fixed while wiring: the token
cache held a single installation (a sweep across owners re-minted per request),
and the installation lookup — billed to the App's own smaller rate-limit budget
— repeated per request. Both are keyed caches now.
Tests: 5 new unit tests; 100 total (50 unit + 50 integration). The handshake
is exercised end to end against a mock server that answers only to the
installation token, so a leaked app JWT fails the test rather than passing
quietly. Misconfiguration and reuse paths are covered too.