feat(rhodibot): GitHub App authentication and a GraphQL client - #538
Merged
Merged
Conversation
rhodibot could not act as a GitHub App: the only credential it understood was a single-repository GITHUB_TOKEN. This adds the RS256 JWT signing, the installation-token exchange and cache, and a GraphQL client for the check runs and issues the bot writes. ring and base64 are taken at versions already in Cargo.lock, so the dependency graph gains no new packages - which matters because CI builds with --locked. Verified locally with rustc 1.98.1: fmt clean, clippy -D warnings clean, 95 tests pass, including 12 new wiremock tests covering the handshake, token caching and refresh, and the GraphQL error 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 ignored due to path filters (1)
📒 Files selected for processing (6)
✨ 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 |
The rust.yml matrix was [robot-repo-automaton, shared-context, dashboard]. rhodibot - the crate implementing the GitHub App, and the one this branch changes - was not in it, so it had never been compiled in CI. Adding it means the gated commands (build --locked --all-targets, test --locked, clippy --all-targets -- -D warnings) actually cover it.
The job's working directory is ${{ matrix.crate }}, so naming the entry
'rhodibot' sent it looking for a top-level 'rhodibot/' directory. It does not
exist; the crate is at bots/rhodibot. The job failed in 'Ensure clippy +
rustfmt components' before compiling a single line, with
'No such file or directory'.
Split the matrix into a display label (module) and a real path (dir), so the
job name stays 'build - test - clippy (rhodibot)' while the working directory
is correct.
hyperpolymath
added a commit
that referenced
this pull request
Sep 19, 2026
App authentication landed in #538 but nothing called it: the REST client still sent a single `GITHUB_TOKEN` for every repository. **Credentials are resolved once, in the client constructor** | configuration | behaviour | | --- | --- | | `app_id` + private key | GitHub App; each request scoped to its repository | | `GITHUB_TOKEN` | static token, unchanged | | neither | anonymous, unchanged | **A half-configured App is refused, not downgraded.** `app_id` without a key (or the reverse) leaves the client `misconfigured`: `readiness()` fails so `main.rs` exits at start-up, and `authorize()` refuses to issue the request. Falling back to anonymous silently would turn a config mistake into a permissions puzzle at the first webhook. `/health` now 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.
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
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.
Implements the blocker for deploying rhodibot: it had no way to act as a GitHub
App.
config.rscarriedTODO: Implement GitHub App JWT authenticationand theonly credential it understood was a single-repository
GITHUB_TOKEN.app_auth— the two-step App handshakeiss= App ID).five minutes early.
Both PEM forms GitHub hands out are accepted: PKCS#1 (
BEGIN RSA PRIVATE KEY)and PKCS#8 (
BEGIN PRIVATE KEY).ringonly parses PKCS#8, so the PKCS#1envelope is wrapped here — and the output is byte-identical to
openssl pkcs8 -topk8 -nocrypt, asserted by a test.graphql— the writesCheck runs and issues are GraphQL mutations, so the GraphQL passes are the
natural choice: REST has no repository node ID at all (its
idis a different,numeric identifier), and a query costs ~1 point however many repositories it
aliases. Errors are read from the
errorsarray rather than inferred from anHTTP status, because GitHub answers 200 with errors and no data — treating that
as success is how a bot silently reports nothing.
Dependencies: no new packages
ringandbase64are taken at versions already inCargo.lockviarustls/reqwest, so the dependency graph gains nothing new and the lock edit is
two lines. That matters because CI builds with
--locked.Verification
Every CI gate was run locally before pushing, with a real toolchain
(rustc 1.98.1):
cargo fmt --check— cleancargo clippy --locked --all-targets -- -D warnings— cleancargo test --locked— 95 tests pass (45 unit, 50 integration)Twelve of those tests are new and exercise real flows against a mock GitHub via
wiremock: the JWT is well-formed and carries the right issuer; the tokenexchange, caching, refresh-on-expiry, and per-installation isolation all behave;
a rejected request errors without echoing a credential; a check run carries the
FAILUREenum rather than the Rust variant name; a repository node ID isresolved once and cached; and a 200-with-errors response is an error.
Three defects were found by running the gates rather than assuming:
hmac = "0.13.0"in Cargo.toml,ring::signature::KeyPairimported unnecessarily (public()is inherent),00pad byte, so it was 257 byteswhere ring requires 256.
Not in this change
Nothing wires
AppAuthinto the webhook handlers yet, and nothing is deployed —the Cloudflare Tunnel host, the App registration, and the pilot installation are
the next increment. The rule-set-from-
standardswork is also still to come.