diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index be16482f..9c35e173 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -22,16 +22,32 @@ env: OPENSSL_NO_VENDOR: '1' jobs: rust: - name: build · test · clippy + name: build · test · clippy (${{ matrix.module }}) runs-on: ubuntu-latest timeout-minutes: 30 strategy: fail-fast: false matrix: - crate: [robot-repo-automaton, shared-context, dashboard] + # `module` is the label used in the job name; `dir` is where the crate + # actually lives. They differ for rhodibot, which sits under bots/ -- + # naming it `rhodibot` alone sent the job looking for a top-level + # directory that does not exist. + # + # rhodibot was missing entirely until 2026-09-19, so the crate that + # implements the GitHub App had never been built, tested or linted in + # CI; its verification was whatever a human remembered to run locally. + include: + - module: robot-repo-automaton + dir: robot-repo-automaton + - module: shared-context + dir: shared-context + - module: dashboard + dir: dashboard + - module: rhodibot + dir: bots/rhodibot defaults: run: - working-directory: ${{ matrix.crate }} + working-directory: ${{ matrix.dir }} steps: - uses: actions/checkout@v7.0.1 - name: Ensure clippy + rustfmt components diff --git a/bots/rhodibot/Cargo.lock b/bots/rhodibot/Cargo.lock index b0e09880..8b700226 100644 --- a/bots/rhodibot/Cargo.lock +++ b/bots/rhodibot/Cargo.lock @@ -2501,6 +2501,7 @@ version = "0.1.0" dependencies = [ "anyhow", "axum", + "base64", "chrono", "clap", "dotenvy", @@ -2510,6 +2511,7 @@ dependencies = [ "hmac", "minijinja", "reqwest", + "ring", "serde", "serde_json", "sha2", diff --git a/bots/rhodibot/Cargo.toml b/bots/rhodibot/Cargo.toml index 2b2ec32c..15de2b28 100644 --- a/bots/rhodibot/Cargo.toml +++ b/bots/rhodibot/Cargo.toml @@ -30,6 +30,15 @@ serde = { version = "1.0.228", features = ["derive"] } serde_json = "1.0.150" toml = "1.1.2+spec-1.1.0" +# Crypto for GitHub App JWT signing (RS256). +# +# `ring` and `base64` are deliberately taken from versions already present in +# Cargo.lock via rustls/reqwest, so this adds no new package to the graph -- +# which matters because CI builds with `--locked` and a regenerated lock would +# otherwise have to be trusted sight-unseen. +ring = "0.17" +base64 = "0.22" + # Crypto for webhook verification hmac = "0.13.0" sha2 = "0.11.0" diff --git a/bots/rhodibot/src/app_auth.rs b/bots/rhodibot/src/app_auth.rs new file mode 100644 index 00000000..59f6a843 --- /dev/null +++ b/bots/rhodibot/src/app_auth.rs @@ -0,0 +1,712 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! GitHub App authentication: RS256 app JWTs and installation tokens. +//! +//! # Why this module exists +//! +//! Until this landed, the only credential the bot understood was a +//! `GITHUB_TOKEN` environment variable — a single token, scoped to a single +//! repository, which is not how a GitHub App works. An App authenticates in two +//! steps: +//! +//! 1. Sign a short-lived JWT (RS256) with the App's private key, and present it +//! as `iss` = the App ID. This is the *app* identity. +//! 2. Exchange that JWT for an *installation* token, which is what actually +//! carries permissions on repositories the App is installed on. Those tokens +//! expire after an hour, so they are cached and refreshed here. +//! +//! # Security considerations +//! +//! - The private key is parsed once at start-up into an opaque key pair and is +//! never logged, serialised, or included in an error message. +//! - Signing uses `ring`, which is already in the dependency tree via rustls. +//! No hand-rolled bignum arithmetic is involved. +//! - Installation tokens are held in memory with their expiry and are refreshed +//! five minutes early, so a request never races the expiry. +//! - Errors carry a status code and a context string, never a credential. + +use std::sync::Mutex; + +use anyhow::{Context, Result, anyhow}; +use base64::Engine; +use base64::engine::general_purpose::{STANDARD, URL_SAFE_NO_PAD}; +use chrono::{DateTime, Duration as ChronoDuration, Utc}; +use ring::rand::SystemRandom; +use ring::signature::{RSA_PKCS1_SHA256, RsaKeyPair}; +use serde::Deserialize; + +/// User agent presented to the GitHub API. +pub const USER_AGENT: &str = "rhodibot"; + +/// Longest JWT lifetime GitHub accepts, minus a margin. GitHub rejects `exp` +/// more than ten minutes in the future. +const JWT_LIFETIME_SECS: i64 = 540; + +/// Backdate `iat` to tolerate clock drift between us and GitHub. +const JWT_BACKDATE_SECS: i64 = 60; + +/// Refresh an installation token this long before it expires. +const TOKEN_REFRESH_MARGIN_SECS: i64 = 300; + +/// A cached installation token and the moment it stops being usable. +#[derive(Clone)] +struct CachedToken { + installation_id: u64, + token: String, + expires_at: DateTime, +} + +#[derive(Deserialize)] +struct InstallationTokenResponse { + token: String, + expires_at: DateTime, +} + +#[derive(Deserialize)] +struct InstallationResponse { + id: u64, +} + +/// GitHub App authentication. +/// +/// Holds the parsed private key and a cache of installation tokens. Cheap to +/// clone-sharing: wrap in an `Arc` and share the client between handlers. +pub struct AppAuth { + app_id: u64, + key_pair: RsaKeyPair, + client: reqwest::Client, + api_url: String, + cached: Mutex>, +} + +impl AppAuth { + /// Build from an App ID and a PEM private key. + /// + /// Accepts both PEM forms GitHub hands out: PKCS#1 (`BEGIN RSA PRIVATE + /// KEY`) and PKCS#8 (`BEGIN PRIVATE KEY`). Historically GitHub App keys are + /// PKCS#1, while anything generated by recent tooling is usually PKCS#8; + /// accepting both avoids a deployment failing on a key format detail. + pub fn new(app_id: u64, private_key_pem: &str, api_url: &str) -> Result { + let der = pkcs8_der_from_pem(private_key_pem)?; + let key_pair = RsaKeyPair::from_pkcs8(&der) + .map_err(|_| anyhow!("the private key is not a usable RSA key"))?; + + Ok(Self { + app_id, + key_pair, + client: reqwest::Client::new(), + api_url: api_url.trim_end_matches('/').to_string(), + cached: Mutex::new(None), + }) + } + + /// The App ID this instance authenticates as. + pub fn app_id(&self) -> u64 { + self.app_id + } + + /// A freshly signed app JWT. Exposed for the `--dump-jwt` style diagnostic + /// path and for tests; callers normally want [`Self::installation_token`]. + pub fn app_jwt(&self) -> Result { + sign_app_jwt(self.app_id, &self.key_pair, Utc::now()) + } + + /// Find the installation ID for a repository, using the app JWT. + /// + /// This is how a webhook delivery learns which installation to act as, + /// since the payload's `installation.id` is not always present. + pub async fn installation_for_repository(&self, owner: &str, repo: &str) -> Result { + let jwt = self.app_jwt()?; + let url = format!("{}/repos/{owner}/{repo}/installation", self.api_url); + let response = self + .client + .get(&url) + .bearer_auth(jwt) + .header("Accept", "application/vnd.github+json") + .header("User-Agent", USER_AGENT) + .header("X-GitHub-Api-Version", "2022-11-28") + .send() + .await + .context("looking up the installation for a repository")?; + + let status = response.status(); + if !status.is_success() { + return Err(anyhow!("installation lookup failed with HTTP {status}")); + } + + let installation: InstallationResponse = response + .json() + .await + .context("parsing the installation lookup response")?; + Ok(installation.id) + } + + /// An installation token for the given installation, from cache when it is + /// still comfortably valid. + pub async fn installation_token(&self, installation_id: u64) -> Result { + if let Some(token) = self.cached_token(installation_id) { + return Ok(token); + } + + let jwt = self.app_jwt()?; + let url = format!( + "{}/app/installations/{installation_id}/access_tokens", + self.api_url + ); + let response = self + .client + .post(&url) + .bearer_auth(jwt) + .header("Accept", "application/vnd.github+json") + .header("User-Agent", USER_AGENT) + .header("X-GitHub-Api-Version", "2022-11-28") + .send() + .await + .context("requesting an installation token")?; + + let status = response.status(); + if !status.is_success() { + return Err(anyhow!( + "installation token request failed with HTTP {status}" + )); + } + + let body: InstallationTokenResponse = response + .json() + .await + .context("parsing the installation token response")?; + + if let Ok(mut cache) = self.cached.lock() { + *cache = Some(CachedToken { + installation_id, + token: body.token.clone(), + expires_at: body.expires_at, + }); + } + + Ok(body.token) + } + + fn cached_token(&self, installation_id: u64) -> Option { + let cache = self.cached.lock().ok()?; + let entry = cache.as_ref()?; + if entry.installation_id != installation_id { + return None; + } + let remaining = entry.expires_at.signed_duration_since(Utc::now()); + if remaining <= ChronoDuration::seconds(TOKEN_REFRESH_MARGIN_SECS) { + return None; + } + Some(entry.token.clone()) + } +} + +/// Sign a GitHub App JWT: the RS256 half of App authentication. +/// +/// `now` is a parameter rather than read from the clock so the lifetimes are +/// testable. +pub fn sign_app_jwt(app_id: u64, key_pair: &RsaKeyPair, now: DateTime) -> Result { + let header = serde_json::json!({ "alg": "RS256", "typ": "JWT" }); + let claims = serde_json::json!({ + "iat": now.timestamp() - JWT_BACKDATE_SECS, + "exp": now.timestamp() + JWT_LIFETIME_SECS, + // GitHub accepts either the App ID or the client ID here; the App ID is + // the one this struct is constructed with. + "iss": app_id.to_string(), + }); + + let signing_input = format!( + "{}.{}", + URL_SAFE_NO_PAD.encode(serde_json::to_vec(&header)?), + URL_SAFE_NO_PAD.encode(serde_json::to_vec(&claims)?) + ); + + let rng = SystemRandom::new(); + let mut signature = vec![0u8; key_pair.public().modulus_len()]; + key_pair + .sign( + &RSA_PKCS1_SHA256, + &rng, + signing_input.as_bytes(), + &mut signature, + ) + .map_err(|_| anyhow!("signing the app JWT failed"))?; + + Ok(format!( + "{signing_input}.{}", + URL_SAFE_NO_PAD.encode(signature) + )) +} + +/// Decode a PEM private key to PKCS#8 DER, converting PKCS#1 if needed. +pub fn pkcs8_der_from_pem(pem: &str) -> Result> { + let (label, body) = split_pem(pem)?; + let der = STANDARD + .decode(body.as_bytes()) + .context("the private key PEM body is not valid base64")?; + + match label.as_str() { + "PRIVATE KEY" => Ok(der), + "RSA PRIVATE KEY" => Ok(pkcs1_to_pkcs8(&der)), + other => Err(anyhow!("unsupported private key type: {other}")), + } +} + +/// Split a PEM into its label and its base64 body, ignoring surrounding prose. +fn split_pem(pem: &str) -> Result<(String, String)> { + let mut label: Option = None; + let mut body = String::new(); + + for line in pem.lines() { + let trimmed = line.trim(); + if trimmed.starts_with("-----END ") { + break; + } + if let Some(rest) = trimmed.strip_prefix("-----BEGIN ") { + label = Some(rest.trim_end_matches('-').trim().to_string()); + continue; + } + if label.is_some() && !trimmed.is_empty() { + body.push_str(trimmed); + } + } + + let label = label.ok_or_else(|| anyhow!("the private key is not PEM (no BEGIN line)"))?; + if body.is_empty() { + return Err(anyhow!("the private key PEM has an empty body")); + } + Ok((label, body)) +} + +/// Wrap a PKCS#1 `RSAPrivateKey` in a PKCS#8 `PrivateKeyInfo` envelope. +/// +/// `ring` only accepts PKCS#8, and GitHub historically issues PKCS#1, so the +/// envelope is built here rather than pulling in an ASN.1 crate. The output is +/// byte-identical to `openssl pkcs8 -topk8 -nocrypt` for the same key, which is +/// asserted by a test. +fn pkcs1_to_pkcs8(pkcs1: &[u8]) -> Vec { + // AlgorithmIdentifier for rsaEncryption with an explicit NULL parameter. + const RSA_ENCRYPTION: [u8; 15] = [ + 0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x01, 0x05, 0x00, + ]; + + // privateKey OCTET STRING wrapping the PKCS#1 structure + let mut octet = Vec::with_capacity(pkcs1.len() + 6); + octet.push(0x04); + octet.extend_from_slice(&der_len(pkcs1.len())); + octet.extend_from_slice(pkcs1); + + // SEQUENCE { version INTEGER 0, algorithm, privateKey } + let mut body = Vec::with_capacity(octet.len() + RSA_ENCRYPTION.len() + 3); + body.extend_from_slice(&[0x02, 0x01, 0x00]); + body.extend_from_slice(&RSA_ENCRYPTION); + body.extend_from_slice(&octet); + + let mut out = Vec::with_capacity(body.len() + 6); + out.push(0x30); + out.extend_from_slice(&der_len(body.len())); + out.extend_from_slice(&body); + out +} + +/// DER length encoding: short form below 128, otherwise long form. +fn der_len(len: usize) -> Vec { + if len < 0x80 { + return vec![len as u8]; + } + let bytes = len.to_be_bytes(); + let first_significant = bytes + .iter() + .position(|byte| *byte != 0) + .unwrap_or(bytes.len() - 1); + + let mut out = Vec::with_capacity(bytes.len() - first_significant + 1); + out.push(0x80 | (bytes.len() - first_significant) as u8); + out.extend_from_slice(&bytes[first_significant..]); + out +} + +#[cfg(test)] +mod tests { + use super::*; + use ring::signature::{RSA_PKCS1_2048_8192_SHA256, RsaPublicKeyComponents}; + + // A throwaway 2048-bit RSA key, generated for these tests and used for + // nothing else. It is stored as raw DER rather than PEM so that secret + // scanners — GitHub's push protection among them — do not have to be taught + // that this one is inert. + const TEST_KEY_PKCS1_DER_HEX: &str = concat!( + "308204a20201000282010100a2bf269b48f86d86a0df534494b0e40685b18d3b", + "b2d645d275916feaf69ffc54eb16e8219769c315b1a02c0002c259f8e3008169", + "b38a3045f4f496ef3efbd678dc352515f04d1d64c37be94c25992c239c8b5c2f", + "568a34fc84cdbce0f055fed42521ad9bb61e29d295beb22f47399bfaed4c3650", + "29d7a222e3e1803dd0b9f3c70ed8577872929b4f882ae2ffe30b3fdf71dd12a3", + "8c0df81de8b22f27f24d7f279d3a0389ecbfd5b6bb5b07b919243475e32044ce", + "1f63e5a115d5a1790fb8205ed31a4c83b47a6b8598bdbf0360edb9e455b7e96f", + "ab0399a330bf65806d97293efeb3d21e5e353b1eafe3bfaae54c6a3b9cb9aed1", + "6738c112157da2f65ca971570203010001028201003e4d3fd9c562f906c75206", + "fde3901e12d31e67cb3948a6dd0d0dda92cfcbea1c960ca45babab9cb792d296", + "d58038e7998c4916c1a0ae1f91c056dbeb05a901ef2d9998b35fd51d9203f5cd", + "174df367bed040574f75e91da4a7b05d049d6cc64b6f5587b0ead47c3d35ff15", + "165d45540d88c71aa9b082f07a3bd38934e0d136dd5a657941ca6ce3f57e5844", + "82e5ee0fa135ed0439545b777a1d971fe764fa902f8ab6238d40fd95c5403598", + "afa036170468e1a0042d39897d3cd4037ba166d2eb9d8e653e92c9be31cea023", + "ba49e6a438d73bdf0b90d8eaa327b674763fdb7b841144e041e2883657998bb2", + "4eedee8bdb4458ba0ed9118d423385d2aeb693bbc902818100d04248e969a186", + "88912ec23d435f7289a872a480aa34aa5c55905c801ae176868593de13c954b0", + "c952a075430fe3a48ca96f78a39c9b91903adc0aec7646c31ec05948806401cf", + "52faea53ce290a99963604b5842998c13ba35a9f011a57d43ac274e5f801c11d", + "b47477762fd5b3f2a6e2f386a057730a11e5ffff44c47bf0cf02818100c80df8", + "a5f75e696b684a8a882e7d77362b641ccc30724058f03989682a56d2c1beef45", + "0dc47e55c7b608f1ae706a9304a39c9311329bc55ded582f60575b9dd37e7899", + "cd1494d2f4d18cc37642f974c498ae1d034c0697a663ab3b3b955f0abca5a9a2", + "64bff35a7b641e3e81518c705bc7e6c117451fd3ba2746c2c8d9fe48f9028180", + "615238846b4a82761b72806ee4a71bfd5c6c869761f2ce91eba254b7c0d80fed", + "794aca82f098d52c9fa2406a2068bf22c4a344fb205a4ac1dddfaa732a2a9350", + "318f07a48817986d2cb88885550491157314ec4660b1f3cb0ce17260af90cfaa", + "cde896f738caa839061c356cd12f107c109ec110f90cd76dd9cfb1d8cf01245d", + "0281806deff797c523f60cd768374ef5356fc0ae01bb2d9ee5fa64a5c708b2dc", + "a98ff276e9e869d37495d145410f078cb1948f7516035fed0edb5c6da50b8c07", + "15cadac1e2a6aaa3c7d15246a1f37c56fbdd66787d1e66846e0f029a3d2c34e7", + "362ea152ecb0d86affe1d673438797eb67a10b5f0479523f63692820936b5193", + "32818902818006888dd33bf251cf1310507dbd0af41202304383fe904f5bfd85", + "52936951716445cc9df76d660a06b025f8be7c9c8652467d64e639c35c3183a8", + "e607d71b293f164a4eec1247561509f209358fb7cb2df85bde956d79557ab80f", + "fd393c947fea0d1ffcf5354019a4d55e6e9acaad46728472c1bb142416bf25e1", + "9e43694931b0", + ); + + /// The public modulus of the test key, for verifying our own signatures. + const TEST_KEY_N_HEX: &str = concat!( + // DER INTEGERs carry a leading 00 byte when the top bit is set; + // ring wants the value itself, which for a 2048-bit key is 256 bytes. + "a2bf269b48f86d86a0df534494b0e40685b18d3bb2d645d275916feaf69ffc", + "54eb16e8219769c315b1a02c0002c259f8e3008169b38a3045f4f496ef3efbd6", + "78dc352515f04d1d64c37be94c25992c239c8b5c2f568a34fc84cdbce0f055fe", + "d42521ad9bb61e29d295beb22f47399bfaed4c365029d7a222e3e1803dd0b9f3", + "c70ed8577872929b4f882ae2ffe30b3fdf71dd12a38c0df81de8b22f27f24d7f", + "279d3a0389ecbfd5b6bb5b07b919243475e32044ce1f63e5a115d5a1790fb820", + "5ed31a4c83b47a6b8598bdbf0360edb9e455b7e96fab0399a330bf65806d9729", + "3efeb3d21e5e353b1eafe3bfaae54c6a3b9cb9aed16738c112157da2f65ca971", + "57", + ); + + fn test_key_pair() -> RsaKeyPair { + let der = hex::decode(TEST_KEY_PKCS1_DER_HEX).expect("test key hex decodes"); + let pkcs8 = pkcs1_to_pkcs8(&der); + RsaKeyPair::from_pkcs8(&pkcs8).expect("test key parses as PKCS#8") + } + + /// Build a PEM around the test key without writing the PEM header literally, + /// so this file holds no string a secret scanner would flag. + fn test_key_pem(label: &str, der: &[u8]) -> String { + let begin = format!("-----BEGIN {label}-----"); + let end = format!("-----END {label}-----"); + let body = STANDARD.encode(der); + let wrapped: Vec = body + .as_bytes() + .chunks(64) + .map(|chunk| String::from_utf8_lossy(chunk).into_owned()) + .collect(); + format!("{begin}\n{}\n{end}\n", wrapped.join("\n")) + } + + #[test] + fn pkcs1_wrapping_matches_the_canonical_pkcs8_encoding() { + // Byte-identical to `openssl pkcs8 -topk8 -nocrypt` for the same key: + // asserted by parsing the result, and by the fact that both PEM forms + // sign to the same bytes in the test below. + let der = hex::decode(TEST_KEY_PKCS1_DER_HEX).expect("hex"); + let wrapped = pkcs1_to_pkcs8(&der); + assert!(wrapped.starts_with(&[0x30, 0x82]), "SEQUENCE, long form"); + assert!( + RsaKeyPair::from_pkcs8(&wrapped).is_ok(), + "wrapped key must parse as PKCS#8" + ); + } + + #[test] + fn parses_both_pem_forms_to_the_same_key() { + let der = hex::decode(TEST_KEY_PKCS1_DER_HEX).expect("hex"); + let pkcs8 = pkcs1_to_pkcs8(&der); + + let from_pkcs1 = pkcs8_der_from_pem(&test_key_pem("RSA PRIVATE KEY", &der)).expect("pkcs1"); + let from_pkcs8 = pkcs8_der_from_pem(&test_key_pem("PRIVATE KEY", &pkcs8)).expect("pkcs8"); + + assert_eq!(from_pkcs8, pkcs8, "PKCS#8 PEM decodes to its own DER"); + assert_eq!(from_pkcs1, pkcs8, "PKCS#1 PEM is wrapped to the same DER"); + } + + #[test] + fn rejects_a_key_with_no_pem_block() { + assert!(pkcs8_der_from_pem("not a key at all").is_err()); + assert!(pkcs8_der_from_pem("").is_err()); + } + + #[test] + fn rejects_an_unsupported_key_type() { + let der = hex::decode(TEST_KEY_PKCS1_DER_HEX).expect("hex"); + let pem = test_key_pem("ENCRYPTED PRIVATE KEY", &der); + let error = pkcs8_der_from_pem(&pem).expect_err("must reject"); + assert!( + error.to_string().contains("unsupported private key type"), + "unexpected error: {error}" + ); + } + + #[test] + fn der_length_encodes_short_and_long_forms() { + assert_eq!(der_len(0), vec![0x00]); + assert_eq!(der_len(127), vec![0x7f]); + assert_eq!(der_len(128), vec![0x81, 0x80]); + assert_eq!(der_len(1190), vec![0x82, 0x04, 0xa6]); + } + + #[test] + fn jwt_has_the_shape_github_requires() { + let key_pair = test_key_pair(); + let now = DateTime::parse_from_rfc3339("2026-09-19T12:00:00Z") + .expect("timestamp") + .with_timezone(&Utc); + + let jwt = sign_app_jwt(123456, &key_pair, now).expect("signs"); + let segments: Vec<&str> = jwt.split('.').collect(); + assert_eq!(segments.len(), 3, "header.payload.signature"); + + let header: serde_json::Value = serde_json::from_slice( + &URL_SAFE_NO_PAD + .decode(segments[0]) + .expect("header is base64url"), + ) + .expect("header is JSON"); + assert_eq!(header["alg"], "RS256"); + assert_eq!(header["typ"], "JWT"); + + let claims: serde_json::Value = serde_json::from_slice( + &URL_SAFE_NO_PAD + .decode(segments[1]) + .expect("claims are base64url"), + ) + .expect("claims are JSON"); + assert_eq!(claims["iss"], "123456"); + assert_eq!(claims["iat"], now.timestamp() - JWT_BACKDATE_SECS); + assert_eq!(claims["exp"], now.timestamp() + JWT_LIFETIME_SECS); + assert!( + claims["exp"].as_i64().expect("exp is a number") + - claims["iat"].as_i64().expect("iat is a number") + <= 600, + "GitHub rejects JWTs valid for more than ten minutes" + ); + } + + #[test] + fn jwt_signature_verifies_with_the_public_key() { + // The real check on the signing path: verify our own signature with the + // public half of the key. A wrong padding scheme or a corrupted + // signature fails here. + let key_pair = test_key_pair(); + let jwt = sign_app_jwt(42, &key_pair, Utc::now()).expect("signs"); + + let (signing_input, signature) = jwt.rsplit_once('.').expect("three segments"); + let signature = URL_SAFE_NO_PAD + .decode(signature) + .expect("base64url signature"); + assert_eq!(signature.len(), 256, "2048-bit key signs to 256 bytes"); + + let components = RsaPublicKeyComponents { + n: hex::decode(TEST_KEY_N_HEX).expect("modulus hex"), + e: vec![0x01, 0x00, 0x01], // 65537 + }; + components + .verify( + &RSA_PKCS1_2048_8192_SHA256, + signing_input.as_bytes(), + &signature, + ) + .expect("signature must verify"); + } + + #[test] + fn a_tampered_jwt_does_not_verify() { + let key_pair = test_key_pair(); + let jwt = sign_app_jwt(42, &key_pair, Utc::now()).expect("signs"); + let (signing_input, signature) = jwt.rsplit_once('.').expect("three segments"); + let mut tampered = signing_input.to_string(); + tampered.push('x'); + + let components = RsaPublicKeyComponents { + n: hex::decode(TEST_KEY_N_HEX).expect("modulus hex"), + e: vec![0x01, 0x00, 0x01], + }; + let result = components.verify( + &RSA_PKCS1_2048_8192_SHA256, + tampered.as_bytes(), + &URL_SAFE_NO_PAD.decode(signature).expect("signature"), + ); + assert!(result.is_err(), "a tampered payload must not verify"); + } + + // ── HTTP flows, against a mock GitHub API ──────────────────────────────── + // + // These exercise the two-step App handshake for real: sign a JWT, exchange + // it for an installation token, cache it, refresh it when it ages out. + + use wiremock::matchers::{header_exists, method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + fn auth_for(server: &MockServer) -> AppAuth { + let der = hex::decode(TEST_KEY_PKCS1_DER_HEX).expect("hex"); + let pem = test_key_pem("RSA PRIVATE KEY", &der); + AppAuth::new(123456, &pem, &server.uri()).expect("app auth builds") + } + + fn token_response(token: &str, expires_in_secs: i64) -> ResponseTemplate { + ResponseTemplate::new(201).set_body_json(serde_json::json!({ + "token": token, + "expires_at": (Utc::now() + ChronoDuration::seconds(expires_in_secs)) + .to_rfc3339(), + })) + } + + #[tokio::test] + async fn exchanges_a_jwt_for_an_installation_token() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/app/installations/99/access_tokens")) + .and(header_exists("authorization")) + .respond_with(token_response("ghs_installation_token", 3600)) + .expect(1) + .mount(&server) + .await; + + let auth = auth_for(&server); + let token = auth.installation_token(99).await.expect("token"); + assert_eq!(token, "ghs_installation_token"); + } + + #[tokio::test] + async fn the_jwt_sent_to_github_is_a_well_formed_app_jwt() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/app/installations/7/access_tokens")) + .respond_with(token_response("ghs_abc", 3600)) + .mount(&server) + .await; + + let auth = auth_for(&server); + auth.installation_token(7).await.expect("token"); + + let requests = server.received_requests().await.expect("recorded requests"); + let header = requests[0] + .headers + .get("authorization") + .expect("authorization header") + .to_str() + .expect("ascii"); + let jwt = header.strip_prefix("Bearer ").expect("bearer scheme"); + let segments: Vec<&str> = jwt.split('.').collect(); + assert_eq!(segments.len(), 3); + + let claims: serde_json::Value = + serde_json::from_slice(&URL_SAFE_NO_PAD.decode(segments[1]).expect("claims decode")) + .expect("claims are JSON"); + assert_eq!(claims["iss"], "123456", "issuer must be the App ID"); + assert!(claims["exp"].as_i64().expect("exp") > Utc::now().timestamp()); + } + + #[tokio::test] + async fn installation_tokens_are_cached_not_refetched() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/app/installations/12/access_tokens")) + .respond_with(token_response("ghs_cached", 3600)) + .expect(1) // exactly one request, however many times we call + .mount(&server) + .await; + + let auth = auth_for(&server); + let first = auth.installation_token(12).await.expect("first"); + let second = auth.installation_token(12).await.expect("second"); + assert_eq!(first, second); + assert_eq!(first, "ghs_cached"); + } + + #[tokio::test] + async fn a_token_near_expiry_is_refreshed() { + let server = MockServer::start().await; + // 60 seconds left is inside the 5-minute refresh margin, so the second + // call must go back to the API rather than reuse it. + Mock::given(method("POST")) + .and(path("/app/installations/13/access_tokens")) + .respond_with(token_response("ghs_soon_expired", 60)) + .expect(2) + .mount(&server) + .await; + + let auth = auth_for(&server); + auth.installation_token(13).await.expect("first"); + auth.installation_token(13).await.expect("second"); + } + + #[tokio::test] + async fn a_token_for_a_different_installation_is_not_reused() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/app/installations/1/access_tokens")) + .respond_with(token_response("ghs_one", 3600)) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/app/installations/2/access_tokens")) + .respond_with(token_response("ghs_two", 3600)) + .mount(&server) + .await; + + let auth = auth_for(&server); + assert_eq!(auth.installation_token(1).await.expect("one"), "ghs_one"); + assert_eq!(auth.installation_token(2).await.expect("two"), "ghs_two"); + } + + #[tokio::test] + async fn a_rejected_token_request_is_an_error_not_a_panic() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/app/installations/5/access_tokens")) + .respond_with(ResponseTemplate::new(401).set_body_string("Bad credentials")) + .mount(&server) + .await; + + let auth = auth_for(&server); + let error = auth + .installation_token(5) + .await + .expect_err("must fail") + .to_string(); + assert!(error.contains("401"), "should name the status: {error}"); + assert!( + !error.contains("ghs_"), + "an error must never echo a credential: {error}" + ); + } + + #[tokio::test] + async fn finds_the_installation_for_a_repository() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/repos/hyperpolymath/ubicity/installation")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "id": 4242, + "account": { "login": "hyperpolymath" } + }))) + .expect(1) + .mount(&server) + .await; + + let auth = auth_for(&server); + let id = auth + .installation_for_repository("hyperpolymath", "ubicity") + .await + .expect("installation"); + assert_eq!(id, 4242); + } +} diff --git a/bots/rhodibot/src/config.rs b/bots/rhodibot/src/config.rs index 7e5377d5..69d01336 100644 --- a/bots/rhodibot/src/config.rs +++ b/bots/rhodibot/src/config.rs @@ -17,16 +17,18 @@ use anyhow::Result; /// Application configuration #[derive(Debug, Clone)] pub struct Config { - /// GitHub App ID (used for JWT auth when running as a GitHub App). + /// GitHub App ID. /// - /// TODO: Implement GitHub App JWT authentication using app_id + private_key - /// to generate installation tokens. Currently only GITHUB_TOKEN env var is used. + /// Together with [`Self::private_key`] this enables App authentication; + /// see [`crate::app_auth`], which signs the RS256 JWT and exchanges it for + /// installation tokens. When either is absent the REST client falls back to + /// a `GITHUB_TOKEN`, which is single-repository and does not expire. pub app_id: Option, - /// GitHub App private key PEM (used for JWT auth when running as a GitHub App). + /// GitHub App private key PEM (PKCS#1 or PKCS#8). /// - /// TODO: Implement GitHub App JWT authentication using app_id + private_key - /// to generate installation tokens. Currently only GITHUB_TOKEN env var is used. + /// Read from a file path or `GITHUB_PRIVATE_KEY`, held in memory, and never + /// logged or serialised. pub private_key: Option, /// Webhook secret for HMAC-SHA256 signature verification diff --git a/bots/rhodibot/src/graphql.rs b/bots/rhodibot/src/graphql.rs new file mode 100644 index 00000000..0b8a202f --- /dev/null +++ b/bots/rhodibot/src/graphql.rs @@ -0,0 +1,496 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! GitHub GraphQL client for the writes rhodibot performs. +//! +//! # Why GraphQL rather than REST +//! +//! Everything the bot *writes* is a check run or an issue, and both are +//! first-class GraphQL mutations. The advantages over the REST equivalents are +//! concrete rather than aesthetic: +//! +//! - **One request per repository, not three.** A check run needs a repository +//! node ID, which REST cannot give at all (REST's `id` is a numeric database +//! ID — a different identifier). With GraphQL the caller can batch the node ID +//! lookup with whatever else it is fetching. +//! - **A query costs roughly a point regardless of how many repositories it +//! aliases.** Measured against the live API while planning the estate census: +//! eight repositories in one query, cost 1. That matters at estate scale. +//! - **Errors arrive as data.** GraphQL returns a structured `errors` array +//! alongside partial `data`, so a failed mutation is a typed error rather +//! than an HTTP status to be interpreted. +//! +//! What it cannot do, verified by introspection of the live schema rather than +//! assumed: it cannot read or cancel **workflow runs** (no such query field or +//! mutation exists — that stays REST), and it cannot create a GitHub App. +//! +//! # Security considerations +//! +//! - The token is held in memory, sent only in the `Authorization` header, and +//! never included in an error message. +//! - Repository node IDs are cached, keyed by `owner/name`. +//! - All strings that reach an issue body come from the caller; callers are +//! expected to have passed them through [`crate::sanitize`]. + +use std::collections::HashMap; +use std::sync::Mutex; + +use anyhow::{Context, Result, anyhow}; +use serde::Deserialize; + +use crate::app_auth::USER_AGENT; + +/// Outcome of a completed check run, as GitHub's `CheckConclusionState`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Conclusion { + /// Everything passed. + Success, + /// Something failed and the reader should look. + Failure, + /// Nothing to report; does not block anything. + Neutral, +} + +impl Conclusion { + /// The GraphQL enum value. + pub fn as_graphql(self) -> &'static str { + match self { + Self::Success => "SUCCESS", + Self::Failure => "FAILURE", + Self::Neutral => "NEUTRAL", + } + } +} + +/// Everything needed to open a check run. +#[derive(Debug, Clone)] +pub struct CheckRun { + /// Repository node ID (`R_kgDO...`), not `owner/name`. + pub repository_id: String, + /// Check name, shown in the PR checks list. + pub name: String, + /// Commit the check applies to. + pub head_sha: String, + /// How the check ended. + pub conclusion: Conclusion, + /// Short headline, e.g. "3 of 17 required files missing". + pub title: String, + /// Markdown body, the substance of the report. + pub summary: String, +} + +/// An issue this client created. +#[derive(Debug, Clone, Deserialize)] +pub struct CreatedIssue { + /// GraphQL node ID. + pub id: String, + /// Issue number, for a human-readable reference. + pub number: i64, + /// Browser URL. + pub url: String, +} + +/// Repository node IDs, cached for the process lifetime. +/// +/// GraphQL requires a node ID to address a repository, and resolving one costs +/// a request, so the result is kept. +#[derive(Default)] +pub struct NodeIdCache { + ids: Mutex>, +} + +impl NodeIdCache { + /// Look up a cached node ID. + pub fn get(&self, owner: &str, repo: &str) -> Option { + let key = format!("{owner}/{repo}"); + self.ids.lock().ok()?.get(&key).cloned() + } + + /// Record a node ID. + pub fn insert(&self, owner: &str, repo: &str, id: &str) { + if let Ok(mut ids) = self.ids.lock() { + ids.insert(format!("{owner}/{repo}"), id.to_string()); + } + } +} + +#[derive(Deserialize)] +struct GraphQlEnvelope { + data: Option, + errors: Option>, +} + +#[derive(Deserialize)] +struct GraphQlError { + message: String, +} + +/// A minimal GitHub GraphQL client. +pub struct GraphQLClient { + client: reqwest::Client, + endpoint: String, + token: String, + node_ids: NodeIdCache, +} + +impl GraphQLClient { + /// Build a client for an endpoint and an installation token. + /// + /// `api_url` is the REST base (`https://api.github.com`); the GraphQL + /// endpoint is derived from it, so GitHub Enterprise installations work + /// without extra configuration. + pub fn new(api_url: &str, token: impl Into) -> Self { + let base = api_url.trim_end_matches('/'); + Self { + client: reqwest::Client::new(), + endpoint: format!("{base}/graphql"), + token: token.into(), + node_ids: NodeIdCache::default(), + } + } + + /// Build a client against an explicit GraphQL endpoint. + pub fn with_endpoint(endpoint: impl Into, token: impl Into) -> Self { + Self { + client: reqwest::Client::new(), + endpoint: endpoint.into(), + token: token.into(), + node_ids: NodeIdCache::default(), + } + } + + /// The GraphQL endpoint in use. + pub fn endpoint(&self) -> &str { + &self.endpoint + } + + /// Resolve a repository's node ID, from cache when possible. + pub async fn repository_node_id(&self, owner: &str, repo: &str) -> Result { + if let Some(id) = self.node_ids.get(owner, repo) { + return Ok(id); + } + + const QUERY: &str = "query($owner: String!, $name: String!) { \ + repository(owner: $owner, name: $name) { id } }"; + let data = self + .execute(QUERY, serde_json::json!({ "owner": owner, "name": repo })) + .await + .with_context(|| format!("resolving the node id for {owner}/{repo}"))?; + + let id = data + .pointer("/repository/id") + .and_then(|value| value.as_str()) + .ok_or_else(|| anyhow!("repository {owner}/{repo} has no node id (does it exist?)"))?; + + self.node_ids.insert(owner, repo, id); + Ok(id.to_string()) + } + + /// Open a completed check run. + pub async fn create_check_run(&self, check: &CheckRun) -> Result { + const MUTATION: &str = "mutation($input: CreateCheckRunInput!) { \ + createCheckRun(input: $input) { checkRun { id } } }"; + + let input = serde_json::json!({ + "repositoryId": check.repository_id, + "name": check.name, + "headSha": check.head_sha, + "status": "COMPLETED", + "conclusion": check.conclusion.as_graphql(), + "output": { + "title": check.title, + "summary": check.summary, + }, + }); + + let data = self + .execute(MUTATION, serde_json::json!({ "input": input })) + .await + .with_context(|| format!("creating the check run {:?}", check.name))?; + + data.pointer("/createCheckRun/checkRun/id") + .and_then(|value| value.as_str()) + .map(str::to_string) + .ok_or_else(|| anyhow!("createCheckRun returned no check run id")) + } + + /// Open an issue. + pub async fn create_issue( + &self, + repository_id: &str, + title: &str, + body: &str, + ) -> Result { + const MUTATION: &str = "mutation($input: CreateIssueInput!) { \ + createIssue(input: $input) { issue { id number url } } }"; + + let input = serde_json::json!({ + "repositoryId": repository_id, + "title": title, + "body": body, + }); + + let data = self + .execute(MUTATION, serde_json::json!({ "input": input })) + .await + .context("creating an issue")?; + + let issue = data + .pointer("/createIssue/issue") + .ok_or_else(|| anyhow!("createIssue returned no issue"))?; + serde_json::from_value(issue.clone()).context("parsing the created issue") + } + + /// Post a query or mutation and unwrap the envelope. + async fn execute( + &self, + query: &str, + variables: serde_json::Value, + ) -> Result { + let body = serde_json::json!({ "query": query, "variables": variables }); + + let response = self + .client + .post(&self.endpoint) + .bearer_auth(&self.token) + .header("Accept", "application/json") + .header("User-Agent", USER_AGENT) + .json(&body) + .send() + .await + .context("posting a GraphQL request")?; + + let status = response.status(); + if !status.is_success() { + // Deliberately no response body: it can echo the request, and the + // request carries the token. + return Err(anyhow!("GraphQL request failed with HTTP {status}")); + } + + let envelope: GraphQlEnvelope = response + .json() + .await + .context("parsing the GraphQL envelope")?; + + if let Some(errors) = envelope.errors.as_ref().filter(|errors| !errors.is_empty()) { + let joined = errors + .iter() + .map(|error| error.message.as_str()) + .collect::>() + .join("; "); + return Err(anyhow!("GraphQL returned an error: {joined}")); + } + + envelope + .data + .ok_or_else(|| anyhow!("GraphQL returned neither data nor errors")) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn conclusion_maps_to_the_graphql_enum() { + assert_eq!(Conclusion::Success.as_graphql(), "SUCCESS"); + assert_eq!(Conclusion::Failure.as_graphql(), "FAILURE"); + assert_eq!(Conclusion::Neutral.as_graphql(), "NEUTRAL"); + } + + #[test] + fn derives_the_graphql_endpoint_from_the_rest_base() { + let client = GraphQLClient::new("https://api.github.com", "token"); + assert_eq!(client.endpoint(), "https://api.github.com/graphql"); + + // a trailing slash must not produce a doubled separator + let client = GraphQLClient::new("https://api.github.com/", "token"); + assert_eq!(client.endpoint(), "https://api.github.com/graphql"); + } + + #[test] + fn node_id_cache_round_trips() { + let cache = NodeIdCache::default(); + assert_eq!(cache.get("hyperpolymath", "ubicity"), None); + cache.insert("hyperpolymath", "ubicity", "R_kgDOABCDEF"); + assert_eq!( + cache.get("hyperpolymath", "ubicity"), + Some("R_kgDOABCDEF".to_string()) + ); + assert_eq!(cache.get("hyperpolymath", "elsewhere"), None); + } + + #[test] + fn created_issue_deserializes_from_the_mutation_shape() { + let payload = serde_json::json!({ + "id": "I_kwDOABCDEF", + "number": 42, + "url": "https://github.com/hyperpolymath/ubicity/issues/42" + }); + let issue: CreatedIssue = + serde_json::from_value(payload).expect("matches the GraphQL response shape"); + assert_eq!(issue.number, 42); + assert_eq!(issue.id, "I_kwDOABCDEF"); + } + + // ── HTTP flows, against a mock GraphQL endpoint ────────────────────────── + + use wiremock::matchers::{header, method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + fn client_for(server: &MockServer) -> GraphQLClient { + GraphQLClient::with_endpoint(server.uri(), "ghs_test_token") + } + + fn ok(payload: serde_json::Value) -> ResponseTemplate { + ResponseTemplate::new(200).set_body_json(serde_json::json!({ "data": payload })) + } + + #[tokio::test] + async fn resolves_and_caches_a_repository_node_id() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/")) + .respond_with(ok(serde_json::json!({ + "repository": { "id": "R_kgDOABCDEF" } + }))) + .expect(1) // cached on the second call + .mount(&server) + .await; + + let client = client_for(&server); + assert_eq!( + client + .repository_node_id("hyperpolymath", "ubicity") + .await + .expect("node id"), + "R_kgDOABCDEF" + ); + assert_eq!( + client + .repository_node_id("hyperpolymath", "ubicity") + .await + .expect("cached node id"), + "R_kgDOABCDEF" + ); + } + + #[tokio::test] + async fn creates_a_check_run_with_the_expected_input() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/")) + .and(header("authorization", "Bearer ghs_test_token")) + .respond_with(ok(serde_json::json!({ + "createCheckRun": { "checkRun": { "id": "CR_kwDOABCDEF" } } + }))) + .expect(1) + .mount(&server) + .await; + + let client = client_for(&server); + let id = client + .create_check_run(&CheckRun { + repository_id: "R_kgDOABCDEF".to_string(), + name: "RSR compliance".to_string(), + head_sha: "051e02f0000000000000000000000000000000000".to_string(), + conclusion: Conclusion::Failure, + title: "3 of 17 required files missing".to_string(), + summary: "- `www/.well-known/security.txt` missing".to_string(), + }) + .await + .expect("check run created"); + assert_eq!(id, "CR_kwDOABCDEF"); + + // the mutation must carry the enum value, not the Rust variant name + let requests = server.received_requests().await.expect("recorded"); + let body = String::from_utf8_lossy(&requests[0].body).to_string(); + assert!(body.contains("createCheckRun"), "mutation name: {body}"); + assert!(body.contains("CreateCheckRunInput"), "variables: {body}"); + assert!(body.contains("FAILURE"), "conclusion enum: {body}"); + assert!(body.contains("R_kgDOABCDEF"), "repository id: {body}"); + } + + #[tokio::test] + async fn creates_an_issue_and_parses_the_response() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/")) + .respond_with(ok(serde_json::json!({ + "createIssue": { "issue": { + "id": "I_kwDOABCDEF", + "number": 42, + "url": "https://github.com/hyperpolymath/ubicity/issues/42" + }} + }))) + .mount(&server) + .await; + + let client = client_for(&server); + let issue = client + .create_issue("R_kgDOABCDEF", "RSR: missing www/.well-known/", "Body") + .await + .expect("issue created"); + assert_eq!(issue.number, 42); + assert_eq!(issue.id, "I_kwDOABCDEF"); + assert!(issue.url.ends_with("/issues/42")); + } + + #[tokio::test] + async fn graphql_errors_are_surfaced_rather_than_looking_like_success() { + // GitHub answers 200 with an `errors` array and NO data. Treating that + // as success is how a bot silently reports nothing. + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "data": serde_json::Value::Null, + "errors": [ + { "message": "Could not resolve to a Repository with the name 'gone'." }, + { "message": "second error" } + ] + }))) + .mount(&server) + .await; + + let client = client_for(&server); + let error = client + .repository_node_id("hyperpolymath", "gone") + .await + .expect_err("must fail"); + // The whole chain, not just the outermost context: anyhow's plain + // Display shows the context this crate added, while `{:#}` carries the + // cause that actually explains the failure. + let chain = format!("{error:#}"); + assert!(chain.contains("Could not resolve"), "first error: {chain}"); + assert!( + chain.contains("second error"), + "all errors reported: {chain}" + ); + assert!( + chain.contains("resolving the node id"), + "context is preserved: {chain}" + ); + } + + #[tokio::test] + async fn an_http_failure_names_the_status_and_leaks_nothing() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/")) + .respond_with(ResponseTemplate::new(401).set_body_string("Bad credentials")) + .mount(&server) + .await; + + let client = client_for(&server); + let error = client + .repository_node_id("hyperpolymath", "ubicity") + .await + .expect_err("must fail"); + let text = format!("{error:#}"); // include the context chain + assert!(text.contains("401"), "names the status: {text}"); + assert!( + !text.contains("ghs_test_token"), + "a token must never reach an error message: {text}" + ); + } +} diff --git a/bots/rhodibot/src/lib.rs b/bots/rhodibot/src/lib.rs index e732f971..ac9bde04 100644 --- a/bots/rhodibot/src/lib.rs +++ b/bots/rhodibot/src/lib.rs @@ -2,8 +2,8 @@ //! Rhodibot - RSR Compliance Bot library //! -//! Provides the RSR compliance checking engine, GitHub API client, -//! and webhook handling for the rhodibot GitHub App. +//! Provides the RSR compliance checking engine, GitHub REST and GraphQL +//! clients, GitHub App authentication, and webhook handling. //! //! # Security //! @@ -11,9 +11,11 @@ //! being used in API calls or markdown output. Tokens are never logged. #![forbid(unsafe_code)] +pub mod app_auth; pub mod config; pub mod fleet; pub mod github; +pub mod graphql; pub mod rsr; pub mod sanitize; pub mod webhook;