diff --git a/CHANGELOG.md b/CHANGELOG.md index 90975bd..adc2634 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to polygo. Versions follow [SemVer](https://semver.org); dat ## Unreleased +- `polygo-bot` runs on Cloud Run, where its always-free tier covers roughly 18,000 pull requests a month: `bot/deploy-cloud-run.sh` takes a project and the App's key and does the rest (APIs, Secret Manager, Cloud Build, deploy), then prints the webhook URL to paste into the App. `POLYGO_BOT_SYNC=1` reviews before answering GitHub, since Cloud Run stops a container's CPU once the response goes out and a background thread would be frozen mid-review. +- The bot refuses what an App anyone can install will eventually be pointed at: a repository over `POLYGO_BOT_MAX_REPO_MB` (default 500, asked of the API before anything is fetched) and an installation asking for more than `POLYGO_BOT_MAX_REVIEWS` reviews in ten minutes (default 20). + - The GitHub Action's manifest could not be parsed at all: one input description held an unquoted `(default: …)`, which YAML reads as a mapping, so every workflow using `Na5co/polygo/action@v0` failed at "Set up job" with a YAML error before polygo ran. Found by using the Action on a real pull request. Quoted, plus two guards: a test that lints every plain scalar in the manifest for the same trap, and a CI job that uses the Action from this checkout on a fixture and asserts it finds the planted error. ## 0.1.8 — 2026-09-22 diff --git a/bot/README.md b/bot/README.md index a3a5b24..b44864d 100644 --- a/bot/README.md +++ b/bot/README.md @@ -30,7 +30,31 @@ rather not add a workflow to, or for offering the review to other people's repos It never blocks a pull request: the review is a `COMMENT`, and the repository's own CI decides whether anything fails. -## Run it +## Deploy it to Cloud Run (free at this size) + +Cloud Run's always-free tier is 2M requests, 180,000 vCPU-seconds and 360,000 GiB-seconds a +month — about **18,000 pull requests**, at the ~10 seconds a review takes. From a Google +Cloud project with billing enabled and `gcloud` installed: + +```sh +PROJECT=your-project ./bot/deploy-cloud-run.sh ~/Downloads/your-app.private-key.pem +``` + +It enables the APIs, puts the App's key and webhook secret in Secret Manager, builds the +image with Cloud Build (your laptop is arm64, Cloud Run is amd64), deploys, and prints the +webhook URL and secret to paste into the App's settings. Run it again to ship a new build. + +The flags that matter, and why: + +- `--concurrency 1` — a review peaks around 150 MB; two at once would not fit in 512 MiB, + so Cloud Run runs another instance instead of thrashing one. +- `--max-instances 3` — the ceiling on what a busy day can cost. +- `POLYGO_BOT_SYNC=1` — Cloud Run stops a container's CPU when the response goes out, so + the review happens *before* the answer. A check that takes longer than GitHub's ten + seconds then shows in the delivery log as timed out; the review is still posted, because + the platform lets the handler finish after GitHub hangs up. + +## Run it anywhere else ```sh docker build -f bot/Dockerfile -t polygo-bot . @@ -47,6 +71,9 @@ docker run -p 8080:8080 \ | `POLYGO_BOT_PRIVATE_KEY` | the App's private key (PEM), or `POLYGO_BOT_PRIVATE_KEY_FILE` to read it from a file — a mounted secret | | `POLYGO_BOT_WEBHOOK_SECRET` | the webhook secret you gave GitHub | | `PORT` | default 8080 | +| `POLYGO_BOT_SYNC` | `1` to review before answering — for platforms that stop the CPU after a response (Cloud Run, and serverless generally). Leave unset on a normal server. | +| `POLYGO_BOT_MAX_REPO_MB` | skip repositories bigger than this (default 500) | +| `POLYGO_BOT_MAX_REVIEWS` | reviews one installation may ask for per 10 minutes (default 20) | `GET /health` answers `polygo-bot ok`; the webhook is `POST /webhook` (or `/`). One process, one container, no state: run it anywhere that can run a container, and scale it by @@ -69,6 +96,13 @@ string file gets a review. ## Cost and limits Each event is one shallow fetch and one check: seconds of CPU and a few MB of disk, freed -when it finishes. A review holds at most 40 comments; the rest are counted in its summary. -The App's API calls are per installation, well inside GitHub's rate limits for anything -short of a very busy organization. +when it finishes. Measured on the largest real project polygo has been pointed at +(open-webui: 3,484 keys × 61 locales, 140,961 translations) a review peaks at **146 MB** and +takes about two seconds of CPU, which is why 512 MiB is the size to run it at. + +An App anyone can install is an App anyone can point anywhere, so the bot says no to two +things: a repository over `POLYGO_BOT_MAX_REPO_MB` (asked of the API before anything is +fetched) and an installation asking for more than `POLYGO_BOT_MAX_REVIEWS` in ten minutes. +A review holds at most 40 comments; the rest are counted in its summary. The App's API +calls are per installation, well inside GitHub's rate limits for anything short of a very +busy organization. diff --git a/bot/cloudbuild.yaml b/bot/cloudbuild.yaml new file mode 100644 index 0000000..c30ede8 --- /dev/null +++ b/bot/cloudbuild.yaml @@ -0,0 +1,9 @@ +# Cloud Build builds the image, because bot/Dockerfile expects the repository root as its +# context and because a Mac builds arm64 while Cloud Run runs amd64. +# gcloud builds submit --config bot/cloudbuild.yaml --substitutions=_IMAGE= . +steps: + - name: gcr.io/cloud-builders/docker + args: ["build", "-f", "bot/Dockerfile", "-t", "${_IMAGE}", "."] +images: ["${_IMAGE}"] +options: + logging: CLOUD_LOGGING_ONLY diff --git a/bot/deploy-cloud-run.sh b/bot/deploy-cloud-run.sh new file mode 100755 index 0000000..87e371d --- /dev/null +++ b/bot/deploy-cloud-run.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash +# Deploy polygo-bot to Cloud Run, from nothing to a webhook URL. +# +# PROJECT=my-project ./bot/deploy-cloud-run.sh path/to/app-private-key.pem +# +# Run it from the repository root. It is idempotent: run it again to deploy a new build, +# and again after changing a secret. Everything it creates is free at this size — Cloud +# Run's always-free tier covers roughly 18,000 pull requests a month. +# +# PROJECT the Google Cloud project (required) +# REGION default us-central1, which is inside the free tier +# SERVICE default polygo-bot +# APP_ID the GitHub App's id; asked for if unset +# SECRET the webhook secret; a fresh random one is made if unset +set -euo pipefail + +key="${1:-}" +[ -n "$key" ] && [ -f "$key" ] || { echo "usage: PROJECT=… $0 " >&2; exit 1; } +: "${PROJECT:?set PROJECT to your Google Cloud project id}" +REGION="${REGION:-us-central1}" +SERVICE="${SERVICE:-polygo-bot}" +IMAGE="${REGION}-docker.pkg.dev/${PROJECT}/polygo/${SERVICE}:$(date +%Y%m%d-%H%M%S)" +command -v gcloud >/dev/null || { echo "gcloud is not installed: https://cloud.google.com/sdk/docs/install" >&2; exit 1; } +[ -f bot/Dockerfile ] || { echo "run this from the repository root" >&2; exit 1; } + +if [ -z "${APP_ID:-}" ]; then + read -rp "GitHub App id: " APP_ID +fi +SECRET="${SECRET:-$(openssl rand -hex 24)}" + +echo "▸ enabling the services this needs (once per project)" +gcloud services enable run.googleapis.com cloudbuild.googleapis.com \ + artifactregistry.googleapis.com secretmanager.googleapis.com --project "$PROJECT" --quiet + +echo "▸ a place to keep the image" +gcloud artifacts repositories describe polygo --location "$REGION" --project "$PROJECT" >/dev/null 2>&1 || + gcloud artifacts repositories create polygo --repository-format=docker \ + --location "$REGION" --project "$PROJECT" --description "polygo images" --quiet + +echo "▸ the App's key and webhook secret, in Secret Manager (never in the image)" +put_secret() { # name, file + gcloud secrets describe "$1" --project "$PROJECT" >/dev/null 2>&1 || + gcloud secrets create "$1" --replication-policy=automatic --project "$PROJECT" --quiet + gcloud secrets versions add "$1" --data-file="$2" --project "$PROJECT" --quiet >/dev/null +} +put_secret polygo-bot-key "$key" +printf '%s' "$SECRET" | put_secret polygo-bot-webhook-secret /dev/stdin + +# Cloud Run's runtime identity has to be allowed to read them. +SA="$(gcloud projects describe "$PROJECT" --format='value(projectNumber)')-compute@developer.gserviceaccount.com" +for s in polygo-bot-key polygo-bot-webhook-secret; do + gcloud secrets add-iam-policy-binding "$s" --member "serviceAccount:$SA" \ + --role roles/secretmanager.secretAccessor --project "$PROJECT" --quiet >/dev/null +done + +echo "▸ building $IMAGE" +gcloud builds submit --config bot/cloudbuild.yaml --substitutions="_IMAGE=${IMAGE}" \ + --project "$PROJECT" --quiet . + +echo "▸ deploying" +# --concurrency=1 because a review needs ~150 MB and two at once would not fit in 512; +# Cloud Run runs more instances instead. --max-instances caps what a busy day can cost. +# --timeout is generous: GitHub stops waiting after 10s but the platform lets the handler +# finish, and the review still gets posted. +gcloud run deploy "$SERVICE" \ + --image "$IMAGE" \ + --region "$REGION" \ + --project "$PROJECT" \ + --platform managed \ + --allow-unauthenticated \ + --memory 512Mi \ + --cpu 1 \ + --concurrency 1 \ + --max-instances 3 \ + --timeout 300 \ + --set-env-vars "POLYGO_BOT_APP_ID=${APP_ID},POLYGO_BOT_SYNC=1" \ + --set-secrets "POLYGO_BOT_PRIVATE_KEY=polygo-bot-key:latest,POLYGO_BOT_WEBHOOK_SECRET=polygo-bot-webhook-secret:latest" \ + --quiet + +URL=$(gcloud run services describe "$SERVICE" --region "$REGION" --project "$PROJECT" --format='value(status.url)') +echo +echo "deployed: $URL" +curl -fsS "$URL/health" && echo +echo +echo "Now, in the GitHub App's settings:" +echo " Webhook URL $URL/webhook" +echo " Webhook secret $SECRET" +echo +echo "Logs: gcloud run services logs tail $SERVICE --region $REGION --project $PROJECT" diff --git a/bot/src/github.rs b/bot/src/github.rs index 7f31c81..5bb6215 100644 --- a/bot/src/github.rs +++ b/bot/src/github.rs @@ -148,6 +148,15 @@ pub fn diff(repo: &str, number: u64, token: &str) -> Result { Ok(s) } +/// The repository's size as GitHub reports it, in kilobytes. Asked before anything is +/// fetched, so a repository too big to be worth cloning costs one API call. +pub fn repo_size_kb(repo: &str, token: &str) -> Result { + let v = get(&format!("{API}/repos/{repo}"), token)?; + v["size"] + .as_u64() + .ok_or_else(|| anyhow!("no size in the repository response")) +} + /// `path:line` of every review comment the bot already left, so it does not say a thing /// twice when the branch is pushed again. pub fn said_already(repo: &str, number: u64, token: &str) -> Result> { diff --git a/bot/src/handle.rs b/bot/src/handle.rs index c417697..5e09962 100644 --- a/bot/src/handle.rs +++ b/bot/src/handle.rs @@ -3,6 +3,7 @@ //! pull request is what `polygo check` says on a laptop. use crate::github::{self, App}; +use crate::limits::{Limits, Rate}; use crate::webhook::Event; use anyhow::{Context, Result}; use polygo::config::Config; @@ -10,7 +11,15 @@ use serde_json::Value; use std::path::Path; /// What happened, for the log. -pub fn review(app: &App, ev: &Event) -> Result { +pub fn review(app: &App, ev: &Event, limits: &Limits, rate: &Rate) -> Result { + // An installation that asks for more than its share waits; whatever is looping will + // stop before it costs anything. + if !rate.allow(ev.installation, limits) { + return Ok(format!( + "{}#{}: installation {} is over its rate limit, skipped", + ev.repo, ev.number, ev.installation + )); + } let token = app.installation_token(ev.installation)?; let diff = github::diff(&ev.repo, ev.number, &token)?; if !touches_strings(&diff) { @@ -19,6 +28,15 @@ pub fn review(app: &App, ev: &Event) -> Result { ev.repo, ev.number )); } + // Ask how big it is before fetching it: the bot runs on a small box, and a repository + // the size of a distribution would take it down for everyone else. + let size_mb = github::repo_size_kb(&ev.repo, &token)? / 1024; + if size_mb > limits.max_repo_mb { + return Ok(format!( + "{}#{}: {size_mb} MB is over the {} MB limit, skipped", + ev.repo, ev.number, limits.max_repo_mb + )); + } let dir = Scratch::new(&format!("{}-{}", ev.repo.replace('/', "-"), ev.number))?; github::fetch_head(&ev.repo, ev.number, &token, dir.path())?; diff --git a/bot/src/limits.rs b/bot/src/limits.rs new file mode 100644 index 0000000..877b62d --- /dev/null +++ b/bot/src/limits.rs @@ -0,0 +1,124 @@ +//! What the bot refuses to do. An App anyone can install is an App anyone can point at a +//! repository the size of a distribution, or at a branch that is pushed every few seconds; +//! neither should cost more than it is worth, and neither should take the bot down for the +//! installations behaving normally. + +use std::collections::HashMap; +use std::sync::Mutex; +use std::time::{Duration, Instant}; + +/// Sizes and rates, all overridable from the environment so a bigger box can say yes to +/// more without a new build. +pub struct Limits { + /// Skip a repository larger than this (GitHub reports the size; the clone is shallow, + /// so this is a generous upper bound on what lands on disk). + pub max_repo_mb: u64, + /// Reviews one installation may ask for within `window`. + pub per_installation: usize, + pub window: Duration, +} + +impl Default for Limits { + fn default() -> Self { + // 500 MB covers every repository polygo has been pointed at (open-webui, the + // largest, is 66 MB checked out) with room to spare; 20 reviews in 10 minutes is + // more pull-request activity than a person can produce. + Limits { + max_repo_mb: 500, + per_installation: 20, + window: Duration::from_secs(600), + } + } +} + +impl Limits { + pub fn from_env() -> Limits { + let d = Limits::default(); + Limits { + max_repo_mb: env("POLYGO_BOT_MAX_REPO_MB").unwrap_or(d.max_repo_mb), + per_installation: env("POLYGO_BOT_MAX_REVIEWS").unwrap_or(d.per_installation), + window: d.window, + } + } +} + +fn env(name: &str) -> Option { + std::env::var(name).ok()?.parse().ok() +} + +/// How many reviews each installation has asked for lately. In memory on purpose: the bot +/// keeps no state between restarts, and a limiter that forgets on restart still stops the +/// runaway it exists for. +#[derive(Default)] +pub struct Rate { + seen: Mutex>>, +} + +impl Rate { + /// Record an event for `installation` and say whether it is within the limit. + pub fn allow(&self, installation: u64, limits: &Limits) -> bool { + let now = Instant::now(); + let mut seen = match self.seen.lock() { + Ok(s) => s, + // A poisoned lock means another thread panicked mid-review; letting the event + // through is friendlier than refusing every one from then on. + Err(e) => e.into_inner(), + }; + // Installations nobody has heard from in a while are forgotten entirely. + seen.retain(|_, times| { + times.retain(|t| now.duration_since(*t) < limits.window); + !times.is_empty() + }); + let times = seen.entry(installation).or_default(); + if times.len() >= limits.per_installation { + return false; + } + times.push(now); + true + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn an_installation_gets_its_share_and_no_more() { + let limits = Limits { + per_installation: 3, + window: Duration::from_secs(600), + ..Limits::default() + }; + let rate = Rate::default(); + assert!(rate.allow(1, &limits)); + assert!(rate.allow(1, &limits)); + assert!(rate.allow(1, &limits)); + assert!(!rate.allow(1, &limits), "the fourth is over the limit"); + // One noisy installation does not spend another's share. + assert!(rate.allow(2, &limits)); + } + + #[test] + fn the_window_passes_and_the_share_comes_back() { + let limits = Limits { + per_installation: 1, + window: Duration::from_millis(50), + ..Limits::default() + }; + let rate = Rate::default(); + assert!(rate.allow(7, &limits)); + assert!(!rate.allow(7, &limits)); + std::thread::sleep(Duration::from_millis(60)); + assert!(rate.allow(7, &limits), "the window has passed"); + } + + #[test] + fn the_environment_can_raise_them() { + let d = Limits::default(); + assert_eq!(d.max_repo_mb, 500); + assert_eq!(d.per_installation, 20); + // Unset variables leave the defaults alone. + let e = Limits::from_env(); + assert_eq!(e.max_repo_mb, d.max_repo_mb); + } +} diff --git a/bot/src/main.rs b/bot/src/main.rs index 8ce1654..eef9e74 100644 --- a/bot/src/main.rs +++ b/bot/src/main.rs @@ -9,9 +9,19 @@ //! POLYGO_BOT_PRIVATE_KEY_FILE a file holding it //! POLYGO_BOT_WEBHOOK_SECRET the webhook secret, the same string GitHub signs with //! PORT default 8080 +//! POLYGO_BOT_SYNC=1 review before answering GitHub, instead of on a thread +//! POLYGO_BOT_MAX_REPO_MB skip repositories bigger than this (default 500) +//! POLYGO_BOT_MAX_REVIEWS reviews one installation may ask for per 10 min (default 20) +//! +//! `POLYGO_BOT_SYNC=1` is for platforms that stop the CPU as soon as a request is answered +//! (Cloud Run's default, and anything else serverless): a background thread would be +//! frozen mid-review there. The cost is that a check taking longer than GitHub's ten +//! seconds makes the delivery show as timed out — the review is still posted, since the +//! platform lets the handler finish after GitHub hangs up. mod github; mod handle; +mod limits; mod webhook; use anyhow::{Context, Result, bail}; @@ -27,6 +37,9 @@ fn main() -> Result<()> { _ => env("POLYGO_BOT_PRIVATE_KEY")?, }; let app = Arc::new(github::App::new(&app_id, &key)?); + let limits = Arc::new(limits::Limits::from_env()); + let rate = Arc::new(limits::Rate::default()); + let sync = std::env::var("POLYGO_BOT_SYNC").is_ok_and(|v| v == "1"); let port: u16 = std::env::var("PORT") .ok() .and_then(|p| p.parse().ok()) @@ -36,8 +49,9 @@ fn main() -> Result<()> { .map_err(|e| anyhow::anyhow!("listening on port {port}: {e}"))?; // The bot has no version of its own; the one that matters is the check's. log(&format!( - "polygo-bot (polygo {}) listening on :{port} as app {app_id}", - polygo::VERSION + "polygo-bot (polygo {}) listening on :{port} as app {app_id}{}", + polygo::VERSION, + if sync { " (sync)" } else { "" } )); for mut req in server.incoming_requests() { let path = req.url().split('?').next().unwrap_or("/").to_string(); @@ -75,20 +89,40 @@ fn main() -> Result<()> { continue; } }; + let Some(ev) = event else { + let _ = req.respond(text(202, "accepted")); + continue; + }; + if sync { + // The platform stops this container's CPU when the response goes out, so the + // review has to happen first. + let what = run(&app, &ev, &limits, &rate); + log(&what); + let _ = req.respond(text(202, what)); + continue; + } // Answer now and work after: GitHub expects a delivery to be acknowledged within // ten seconds, and a check on a large catalog takes longer than that. let _ = req.respond(text(202, "accepted")); - if let Some(ev) = event { - let app = Arc::clone(&app); - std::thread::spawn(move || match handle::review(&app, &ev) { - Ok(what) => log(&what), - Err(e) => log(&format!("{}#{}: {e:#}", ev.repo, ev.number)), - }); - } + let (app, limits, rate) = (Arc::clone(&app), Arc::clone(&limits), Arc::clone(&rate)); + std::thread::spawn(move || log(&run(&app, &ev, &limits, &rate))); } Ok(()) } +/// One review, and one line about it either way. +fn run( + app: &github::App, + ev: &webhook::Event, + limits: &limits::Limits, + rate: &limits::Rate, +) -> String { + match handle::review(app, ev, limits, rate) { + Ok(what) => what, + Err(e) => format!("{}#{}: {e:#}", ev.repo, ev.number), + } +} + fn header(req: &tiny_http::Request, name: &'static str) -> Option { req.headers() .iter() @@ -103,8 +137,8 @@ fn env(name: &str) -> Result { } } -fn text(status: u16, body: &str) -> tiny_http::Response>> { - tiny_http::Response::from_string(body).with_status_code(status) +fn text(status: u16, body: impl AsRef) -> tiny_http::Response>> { + tiny_http::Response::from_string(body.as_ref()).with_status_code(status) } /// One line per event, to stdout: whatever runs the container collects it. diff --git a/bot/tests/server.rs b/bot/tests/server.rs index d471cf2..04b9bd6 100644 --- a/bot/tests/server.rs +++ b/bot/tests/server.rs @@ -117,6 +117,51 @@ fn the_app_answers_health_and_refuses_an_unsigned_delivery() { drop(bot); } +/// The mode serverless platforms need: the review happens before the answer, so nothing is +/// left running on a CPU the platform has already stopped. +#[test] +fn sync_mode_answers_after_doing_the_work() { + let dir = tempfile::tempdir().unwrap(); + let key = dir.path().join("app.pem"); + if !test_key(&key) { + eprintln!("openssl not available; skipping"); + return; + } + let port = free_port(); + let bot = Bot(Command::new(env!("CARGO_BIN_EXE_polygo-bot")) + .env("POLYGO_BOT_APP_ID", "12345") + .env("POLYGO_BOT_PRIVATE_KEY_FILE", &key) + .env("POLYGO_BOT_WEBHOOK_SECRET", SECRET) + .env("POLYGO_BOT_SYNC", "1") + .env("PORT", port.to_string()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .unwrap()); + assert!(wait_for(port), "polygo-bot did not start"); + let body = r#"{"zen":"hi"}"#; + let head = format!( + "POST /webhook HTTP/1.1\r\nHost: localhost:{port}\r\nConnection: close\r\nContent-Type: application/json\r\nX-GitHub-Event: ping\r\n" + ); + let signed = format!("{head}X-Hub-Signature-256: {}\r\n", sign(body.as_bytes())); + let res = request(port, &signed, body); + assert!(res.contains("202"), "{res}"); + // A pull_request event with an installation nobody can mint a token for: the answer + // still comes back, carrying what went wrong rather than a bare "accepted". + let pr = r#"{"action":"opened","installation":{"id":1},"repository":{"full_name":"acme/app"},"pull_request":{"number":7,"draft":false,"head":{"sha":"abc"}}}"#; + let head = format!( + "POST /webhook HTTP/1.1\r\nHost: localhost:{port}\r\nConnection: close\r\nContent-Type: application/json\r\nX-GitHub-Event: pull_request\r\n" + ); + let signed = format!("{head}X-Hub-Signature-256: {}\r\n", sign(pr.as_bytes())); + let res = request(port, &signed, pr); + assert!(res.contains("202"), "{res}"); + assert!( + res.contains("acme/app#7"), + "the body should carry the outcome of the review: {res}" + ); + drop(bot); +} + #[test] fn the_app_refuses_to_start_without_its_secrets() { let out = Command::new(env!("CARGO_BIN_EXE_polygo-bot"))