diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index df2058c..74babdf 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -35,8 +35,10 @@ practice rather than asserted from habit — see Further reading. `helpers/`) tends toward the opposite, and a single feature change ends up touching files scattered across every layer. **Rule**: modules are named after what they do in the pipeline - (`checker`, `fetcher`, `verifier`, `builder`, `sanity`, `publisher`, - `state`), not generic buckets. A new pipeline stage gets a new module + (`release_source`, `fetcher`, `verifier`, `builder`, `sanity`, + `publisher`, `state`), not generic buckets. (`release_source` is a directory module: the + `ReleaseSource` trait and one file per host, re-exported from its + `mod.rs` so the rest of the crate never names a host's file.) A new pipeline stage gets a new module named after the stage, not a method bolted onto an existing one. **Anti-example to keep watching for**: a `utils.rs` grab-bag. `hash.rs` could look like one but isn't — it exists for exactly one piece of @@ -61,8 +63,9 @@ practice rather than asserted from habit — see Further reading. Same testability goal as #3, applied to the specific ways this program reaches outside itself. A consistent shape beats ad hoc mocking invented per call site. - **Already in force**: `GithubEndpoints` (checker/fetcher/verifier), - `repo_add_bin` and the pacman.conf path (publisher), `PKGWATCH_REPO_DIR` + **Already in force**: `GithubEndpoints`/`ForgejoEndpoints` (the + `ReleaseSource` implementations, whose API root is what fetcher/verifier + take), `repo_add_bin` and the pacman.conf path (publisher), `PKGWATCH_REPO_DIR` (main, for manual dry runs against a scratch repo instead of the real one). A new external call follows the same shape: production code calls a thin wrapper with the real default; tests call the parameterized @@ -106,8 +109,8 @@ practice rather than asserted from habit — see Further reading. directory, but the same information — why this way and not the obvious alternative — needs to live somewhere a future reader will actually see it: the doc comment on the thing itself. - **Example already here**: `checker.rs`'s doc comment on - `latest_github_release` explains why the newest Atom-feed entry isn't + **Example already here**: `release_source/github.rs`'s doc comment on + `GithubEndpoints::latest_release` explains why the newest Atom-feed entry isn't trusted outright (scaleway-cli's `-dbg1` tag has no real Release behind it) — the reasoning lives right next to the code it justifies, not in a commit message or a separate design doc no one will find later. diff --git a/docs/SPEC.md b/docs/SPEC.md index 3361cbe..dcb3b01 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -287,17 +287,33 @@ implements `repo`, `asset_pattern`, and `verification.method` matches the line by filename instead of assuming a single-hash file. - Separately, scaleway-cli's Atom feed lists a `vX.Y.Z-dbg1` tag newest, with no real Release object behind it (`releases/tags/` 404s) — - `checker::latest_github_release` now confirms each feed candidate + the GitHub source's `latest_release` now confirms each feed candidate against the releases API in feed order rather than trusting the first entry outright. `sanity_check` and `binary_name` are now real, implemented fields (see Builder/Sanity checker above) — added `packages.d/uv.toml`'s and `packages.d/scaleway-cli.toml`'s own `sanity_check` blocks, and -scaleway-cli's `binary_name = "scw"`. `source`, `check_method`, and -`check_interval` are still schema sketch, not yet read by the code — the -PoC only knows how to check GitHub-release sources, on a single one-shot -run rather than a scheduled loop. +scaleway-cli's `binary_name = "scw"`. `source` is implemented too, with +two values: `github-release` (the default when omitted, so existing +configs are unchanged) and `forgejo-release`, which also requires a +`base_url` (see Checker below). `check_method` and `check_interval` are +still schema sketch, not yet read by the code — checks are a fixed hourly +tick, not per-package. + +```toml +# A package released from a Forgejo instance instead of GitHub. Only +# `same-origin-sha256` is valid here: `github-attestation` needs GitHub. +[package.mytool] +source = "forgejo-release" +base_url = "https://code.austinschaefer.com" +repo = "schaefera/mytool" +asset_pattern = "mytool-linux-x86_64.tar.gz" + +[package.mytool.verification] +method = "same-origin-sha256" +checksum_asset_pattern = "SHA256SUMS" +``` Build/publish/review-queue (`makepkg`, `repo-add`, tier 4–6 human review) are now implemented too — see Builder/Sanity checker/Publisher/Reviewer @@ -357,15 +373,35 @@ Open questions on the schema: likely reuses `nvchecker`'s logic/sources conceptually for non-GitHub sources eventually. For GitHub sources, prefers the `github-atom` feed (see Scaling > Check method) over unconditional REST polling. - *(Implemented for GitHub only — `src/checker.rs` regex-matches the first - `releases/tag/` link in the feed rather than doing a full XML parse; - fine while the feed's newest-entry-first shape holds, revisit if that - ever changes. `check_interval`/per-package cadence not wired up yet — - the PoC is a single one-shot run, not a scheduled loop.)* + *(Implemented for GitHub and Forgejo. `src/release_source/` defines the + `ReleaseSource` trait (latest release + API root); each host implements + it in its own file (`github.rs`, `forgejo.rs`), and the module's + `for_package` picks one per package, so adding a host doesn't touch + existing ones. The rest of the crate imports the trait and hosts from + `crate::release_source`, which re-exports them. A trait + rather than an enum match because there are now two real hosts with + genuinely different logic. GitHub regex-matches the first + `releases/tag/` link in the feed rather than doing a full XML + parse; fine while the feed's newest-entry-first shape holds, revisit if + that ever changes. Forgejo is one call to + `/api/v1/repos//releases/latest`, which already returns + only the newest non-draft, non-prerelease release, so it needs none of + GitHub's confirm-each-tag step. `check_interval`/per-package cadence not + wired up yet — checks are a fixed hourly tick.)* + + The HTTP is hand-rolled on the `reqwest` already in the tree, not an + API-client crate: pkgwatch needs two `GET`s, GitHub's check deliberately + uses an Atom feed no API crate covers (to stay off the rate-limited REST + API), and the release-by-tag call is shared verbatim by both hosts. + `octocrab` is async against our blocking `reqwest` with a default tree + larger than pkgwatch's whole current one; `forgejo-api` is a generated + binding of the entire API for one endpoint. Revisit if pkgwatch needs + authenticated or write API calls. - **Fetcher**: downloads the artifact (and any checksum/signature/ attestation companion) for a resolved version. *(Implemented — - `src/fetcher.rs`, via the GitHub releases API; exact asset-name match, - not a glob.)* + `src/fetcher.rs`, via the GitHub or Forgejo releases API — same + `releases/tags/` endpoint and JSON shape on both; exact asset-name + match, not a glob.)* - **Verifier**: tier-specific verification implementations, dispatched via a `Verification` enum matched on `method` (an internally-tagged serde enum) rather than a trait — simpler while there are only two methods; @@ -490,8 +526,8 @@ Open questions on the schema: confirmed correct — no build-provenance attestations upstream. Required adding `{version}`-placeholder support to `asset_pattern`/ `checksum_asset_pattern`, filename-matched parsing of combined - multi-asset checksum files, and having `latest_github_release` - confirm each Atom-feed candidate against the releases API (this + multi-asset checksum files, and having the GitHub source's + `latest_release` confirm each Atom-feed candidate against the releases API (this repo's newest feed entry, a `-dbg1` tag, has no real Release behind it). Still just flags for human review, same as any tier 4-6 pass — not auto-installed; see the unchecked build/publish item below. @@ -517,7 +553,7 @@ Open questions on the schema: - [ ] Not yet implemented: `pkgwatch review --reject` (a pending review can only be approved or left pending, not dismissed), per-package `check_interval` (the timer is a fixed hourly tick), - non-GitHub sources, `minisign`/tier-1 + sources other than GitHub and Forgejo releases, `minisign`/tier-1 method, retention/pruning of old versions in the local repo (see Scaling > Local repo retention), staggering/auth for GitHub API rate limits at higher package counts. diff --git a/src/checker.rs b/src/checker.rs index 676120d..a32d2fb 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -1,45 +1,6 @@ -use crate::github::GithubEndpoints; -use anyhow::{Result, bail}; -use regex::Regex; - -/// Resolves the latest release tag for `repo` via its public Atom feed. -/// -/// Deliberately not a full XML parse: the feed lists entries newest-first, -/// and each `"/>` is matched -/// in document order. Revisit with a real XML parser if GitHub's feed -/// shape ever changes. -/// -/// The feed can list a tag newer than any tag with a real Release object -/// behind it — observed on scaleway/scaleway-cli, which pushes a -/// `vX.Y.Z-dbg1` tag (no corresponding Release; `releases/tags/` -/// 404s) right after each real release, and that tag sorts newest in the -/// feed. So each candidate is confirmed against the releases API in feed -/// order, returning the first that actually resolves. -pub fn latest_github_release( - client: &reqwest::blocking::Client, - endpoints: &GithubEndpoints, - repo: &str, -) -> Result { - let url = format!("{}/{repo}/releases.atom", endpoints.web); - let body = client.get(&url).send()?.error_for_status()?.text()?; - - let re = Regex::new(r#"releases/tag/([^"]+)""#)?; - let mut candidates = re - .captures_iter(&body) - .map(|caps| caps[1].to_string()) - .peekable(); - if candidates.peek().is_none() { - bail!("no release tag found in {url}"); - } - - for tag in candidates { - let release_url = format!("{}/repos/{repo}/releases/tags/{tag}", endpoints.api); - if client.get(&release_url).send()?.status().is_success() { - return Ok(tag); - } - } - bail!("no release tag in {url} resolved to a real release via the API") -} +//! Turns a release tag into a version string. What the latest tag *is* +//! comes from a `ReleaseSource` (see `release_source`); this is the one piece of +//! the check stage that isn't host-specific. /// Strips a leading `v` from a release tag, e.g. `v2.62.0` -> `2.62.0`. /// @@ -65,84 +26,4 @@ mod tests { fn version_from_tag_leaves_bare_version_unchanged() { assert_eq!(version_from_tag("0.12.15"), "0.12.15"); } - - fn atom_feed(tags: &[&str]) -> String { - let entries: String = tags - .iter() - .map(|t| { - format!(r#""#) - }) - .collect(); - format!("{entries}") - } - - #[test] - fn latest_github_release_skips_tags_with_no_real_release() { - let mut server = mockito::Server::new(); - let endpoints = GithubEndpoints { - web: server.url(), - api: server.url(), - }; - - // Mirrors the real scaleway-cli case: newest feed entry (a -dbg1 - // tag) has no Release object behind it and 404s. - let _feed = server - .mock("GET", "/o/r/releases.atom") - .with_status(200) - .with_body(atom_feed(&["v2.62.0-dbg1", "v2.62.0"])) - .create(); - let _missing = server - .mock("GET", "/repos/o/r/releases/tags/v2.62.0-dbg1") - .with_status(404) - .create(); - let _real = server - .mock("GET", "/repos/o/r/releases/tags/v2.62.0") - .with_status(200) - .with_body("{}") - .create(); - - let client = reqwest::blocking::Client::new(); - let tag = latest_github_release(&client, &endpoints, "o/r").unwrap(); - assert_eq!(tag, "v2.62.0"); - } - - #[test] - fn latest_github_release_errors_when_feed_has_no_tags() { - let mut server = mockito::Server::new(); - let endpoints = GithubEndpoints { - web: server.url(), - api: server.url(), - }; - let _feed = server - .mock("GET", "/o/r/releases.atom") - .with_status(200) - .with_body("") - .create(); - - let client = reqwest::blocking::Client::new(); - let err = latest_github_release(&client, &endpoints, "o/r").unwrap_err(); - assert!(err.to_string().contains("no release tag found")); - } - - #[test] - fn latest_github_release_errors_when_no_candidate_resolves() { - let mut server = mockito::Server::new(); - let endpoints = GithubEndpoints { - web: server.url(), - api: server.url(), - }; - let _feed = server - .mock("GET", "/o/r/releases.atom") - .with_status(200) - .with_body(atom_feed(&["v1.0.0-dbg1"])) - .create(); - let _missing = server - .mock("GET", "/repos/o/r/releases/tags/v1.0.0-dbg1") - .with_status(404) - .create(); - - let client = reqwest::blocking::Client::new(); - let err = latest_github_release(&client, &endpoints, "o/r").unwrap_err(); - assert!(err.to_string().contains("resolved to a real release")); - } } diff --git a/src/config.rs b/src/config.rs index 9c242b2..3119eaa 100644 --- a/src/config.rs +++ b/src/config.rs @@ -2,7 +2,7 @@ //! The only module that knows the TOML shape — everything downstream //! works with `Package`/`Verification`/`SanityCheck`, never raw TOML. -use anyhow::{Context, Result}; +use anyhow::{Context, Result, bail}; use serde::Deserialize; use std::collections::{BTreeMap, HashMap}; use std::path::Path; @@ -12,10 +12,29 @@ struct PackageFile { package: HashMap, } +/// Where a package's releases are published: the `source` key from +/// docs/SPEC.md > Config schema. Omitted means GitHub, so every existing +/// `packages.d/*.toml` keeps working. +#[derive(Debug, Deserialize, Clone, Copy, Default, PartialEq, Eq)] +#[serde(rename_all = "kebab-case")] +pub enum Source { + #[default] + GithubRelease, + /// A Forgejo (or Gitea) instance; needs `base_url` too. + ForgejoRelease, +} + #[derive(Debug, Deserialize, Clone)] pub struct Package { + /// `owner/name` on whichever `source` hosts it. pub repo: String, - /// Exact GitHub release asset name (still not a glob — see + #[serde(default)] + pub source: Source, + /// Web root of the Forgejo instance, e.g. `https://code.austinschaefer.com` + /// (the API lives under `/api/v1`). Required for, and only meaningful + /// with, `source = "forgejo-release"`. + pub base_url: Option, + /// Exact release asset name (still not a glob — see /// docs/SPEC.md > Architecture > Fetcher), optionally containing a /// `{version}` placeholder for projects whose asset names embed the /// version (e.g. `scaleway-cli_{version}_linux_amd64`). Substituted via @@ -60,6 +79,32 @@ pub struct Package { } impl Package { + /// Rejects combinations that can't work, once at load time rather than + /// as a confusing failure deep in a run (see docs/ARCHITECTURE.md > + /// "validate at the boundary, once"). + fn validate(&self, name: &str) -> Result<()> { + match (self.source, &self.base_url) { + (Source::GithubRelease, None) => {} + (Source::GithubRelease, Some(_)) => { + // Silently ignoring it would hide a mistyped `source`. + bail!("{name}: base_url only applies to source = \"forgejo-release\""); + } + (Source::ForgejoRelease, None) => { + bail!("{name}: source = \"forgejo-release\" needs a base_url"); + } + (Source::ForgejoRelease, Some(url)) => { + if !url.starts_with("https://") && !url.starts_with("http://") { + bail!("{name}: base_url '{url}' must start with http:// or https://"); + } + // `gh attestation verify` only speaks GitHub's attestation API. + if matches!(self.verification, Verification::GithubAttestation) { + bail!("{name}: github-attestation verification needs a GitHub source"); + } + } + } + Ok(()) + } + /// The name of the executable inside the built package: `binary_name` /// if the package declares one, else `pkg_name` itself. pub fn binary_name<'a>(&'a self, pkg_name: &'a str) -> &'a str { @@ -107,6 +152,10 @@ pub fn load_packages_dir(dir: &Path) -> Result> { .with_context(|| format!("reading {}", path.display()))?; let file: PackageFile = toml::from_str(&text).with_context(|| format!("parsing {}", path.display()))?; + for (name, pkg) in &file.package { + pkg.validate(name) + .with_context(|| format!("in {}", path.display()))?; + } out.extend(file.package); } Ok(out) @@ -258,4 +307,79 @@ mod tests { let dir = tempfile::tempdir().unwrap(); assert!(load_packages_dir(dir.path()).unwrap().is_empty()); } + + /// One `[package.p]` with the given extra top-level lines and + /// verification table, loaded through the real loader so validation + /// runs too. + fn load_one(extra: &str, verification: &str) -> Result { + let dir = tempfile::tempdir().unwrap(); + write( + dir.path(), + "p.toml", + &format!( + "[package.p]\nrepo = \"o/r\"\nasset_pattern = \"x\"\n{extra}\n\ + [package.p.verification]\n{verification}\n" + ), + ); + let mut loaded = load_packages_dir(dir.path())?; + Ok(loaded.remove(0).1) + } + + const SAME_ORIGIN: &str = "method = \"same-origin-sha256\"\nchecksum_asset_pattern = \"SUMS\""; + const ATTESTATION: &str = "method = \"github-attestation\""; + + const FORGEJO: &str = "source = \"forgejo-release\"\nbase_url = \"https://forge.example.com\""; + + #[test] + fn source_defaults_to_github_release() { + let pkg = load_one("", ATTESTATION).unwrap(); + assert_eq!(pkg.source, Source::GithubRelease); + assert_eq!(pkg.base_url, None); + } + + #[test] + fn loads_explicit_github_release_source() { + let pkg = load_one("source = \"github-release\"", ATTESTATION).unwrap(); + assert_eq!(pkg.source, Source::GithubRelease); + } + + #[test] + fn loads_forgejo_release_source() { + let pkg = load_one(FORGEJO, SAME_ORIGIN).unwrap(); + assert_eq!(pkg.source, Source::ForgejoRelease); + assert_eq!(pkg.base_url.as_deref(), Some("https://forge.example.com")); + } + + #[test] + fn rejects_forgejo_release_without_base_url() { + let err = load_one("source = \"forgejo-release\"", SAME_ORIGIN).unwrap_err(); + assert!(format!("{err:#}").contains("needs a base_url")); + } + + #[test] + fn rejects_base_url_on_a_github_source() { + let err = load_one("base_url = \"https://forge.example.com\"", SAME_ORIGIN).unwrap_err(); + assert!(format!("{err:#}").contains("only applies to source")); + } + + #[test] + fn rejects_forgejo_base_url_without_scheme() { + let err = load_one( + "source = \"forgejo-release\"\nbase_url = \"forge.example.com\"", + SAME_ORIGIN, + ) + .unwrap_err(); + assert!(format!("{err:#}").contains("must start with http")); + } + + #[test] + fn rejects_github_attestation_on_a_forgejo_source() { + let err = load_one(FORGEJO, ATTESTATION).unwrap_err(); + assert!(format!("{err:#}").contains("needs a GitHub source")); + } + + #[test] + fn rejects_unknown_source() { + assert!(load_one("source = \"gitlab-release\"", SAME_ORIGIN).is_err()); + } } diff --git a/src/fetcher.rs b/src/fetcher.rs index 2614258..314d394 100644 --- a/src/fetcher.rs +++ b/src/fetcher.rs @@ -1,8 +1,7 @@ -//! Downloads a named GitHub release asset to a local path. The only -//! module that talks to the releases API for asset bytes — `checker` only -//! resolves version tags, never downloads. +//! Downloads a named release asset (from GitHub or Forgejo) to a local +//! path. The only module that talks to the releases API for asset bytes — +//! `release_source` only resolves version tags, never downloads. -use crate::github::GithubEndpoints; use anyhow::{Context, Result}; use serde::Deserialize; use std::path::{Path, PathBuf}; @@ -29,16 +28,18 @@ pub struct DownloadedAsset { } /// Downloads the release asset named exactly `asset_name` for `repo`@`tag` -/// into `dest_dir`, returning the local path and its origin URL. +/// into `dest_dir`, returning the local path and its origin URL. `api` is +/// the releases API root (see `ReleaseSource::api`); GitHub and Forgejo +/// serve the same endpoint and JSON shape under it. pub fn download_asset( client: &reqwest::blocking::Client, - endpoints: &GithubEndpoints, + api: &str, repo: &str, tag: &str, asset_name: &str, dest_dir: &Path, ) -> Result { - let api_url = format!("{}/repos/{repo}/releases/tags/{tag}", endpoints.api); + let api_url = format!("{api}/repos/{repo}/releases/tags/{tag}"); let release: Release = client .get(&api_url) .send()? @@ -73,10 +74,7 @@ mod tests { #[test] fn download_asset_writes_matching_asset_to_dest_dir() { let mut server = mockito::Server::new(); - let endpoints = GithubEndpoints { - web: server.url(), - api: server.url(), - }; + let api = server.url(); let asset_url = format!("{}/download/thing.tar.gz", server.url()); let release_body = format!( r#"{{"assets": [{{"name": "thing.tar.gz", "browser_download_url": "{asset_url}"}}]}}"# @@ -96,7 +94,7 @@ mod tests { let dest_dir = tempfile::tempdir().unwrap(); let asset = download_asset( &client, - &endpoints, + &api, "o/r", "v1.0.0", "thing.tar.gz", @@ -112,10 +110,7 @@ mod tests { #[test] fn download_asset_errors_when_no_asset_matches() { let mut server = mockito::Server::new(); - let endpoints = GithubEndpoints { - web: server.url(), - api: server.url(), - }; + let api = server.url(); let _release = server .mock("GET", "/repos/o/r/releases/tags/v1.0.0") .with_status(200) @@ -126,7 +121,7 @@ mod tests { let dest_dir = tempfile::tempdir().unwrap(); let err = download_asset( &client, - &endpoints, + &api, "o/r", "v1.0.0", "thing.tar.gz", @@ -139,10 +134,7 @@ mod tests { #[test] fn download_asset_errors_when_release_not_found() { let mut server = mockito::Server::new(); - let endpoints = GithubEndpoints { - web: server.url(), - api: server.url(), - }; + let api = server.url(); let _release = server .mock("GET", "/repos/o/r/releases/tags/v1.0.0") .with_status(404) @@ -152,7 +144,7 @@ mod tests { let dest_dir = tempfile::tempdir().unwrap(); let err = download_asset( &client, - &endpoints, + &api, "o/r", "v1.0.0", "thing.tar.gz", diff --git a/src/github.rs b/src/github.rs deleted file mode 100644 index a511f16..0000000 --- a/src/github.rs +++ /dev/null @@ -1,29 +0,0 @@ -/// Base URLs for GitHub's public web host (Atom feeds, release pages) and -/// its REST API, factored out so tests can point both at a local mock -/// server instead of the real github.com/api.github.com. -#[derive(Debug, Clone)] -pub struct GithubEndpoints { - pub web: String, - pub api: String, -} - -impl Default for GithubEndpoints { - fn default() -> Self { - Self { - web: "https://github.com".to_string(), - api: "https://api.github.com".to_string(), - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn default_points_at_real_github() { - let endpoints = GithubEndpoints::default(); - assert_eq!(endpoints.web, "https://github.com"); - assert_eq!(endpoints.api, "https://api.github.com"); - } -} diff --git a/src/main.rs b/src/main.rs index e238779..5001beb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,12 +6,12 @@ mod builder; mod checker; mod config; mod fetcher; -mod github; mod hash; mod notifier; mod paths; mod pipeline; mod publisher; +mod release_source; mod sanity; mod state; #[cfg(test)] diff --git a/src/pipeline.rs b/src/pipeline.rs index 753698e..97bb471 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -9,10 +9,10 @@ use crate::builder; use crate::checker; use crate::config::{self, Package}; use crate::fetcher::{self, DownloadedAsset}; -use crate::github::GithubEndpoints; use crate::notifier::{self, Event}; use crate::paths::Paths; use crate::publisher; +use crate::release_source::{self, ReleaseSource}; use crate::sanity; use crate::state; use crate::verifier::{self, VerificationResult}; @@ -57,7 +57,6 @@ fn load_packages(packages_dir: &Path) -> Result> { pub fn run_check() -> Result<()> { let client = build_client()?; - let endpoints = GithubEndpoints::default(); let Paths { packages_dir, state_dir, @@ -73,7 +72,10 @@ pub fn run_check() -> Result<()> { let mut any_failed = false; for (name, pkg) in &packages { println!("== {name} ({}) ==", pkg.repo); - if let Err(err) = process_package(&client, &endpoints, &state_dir, &work_dir, name, pkg) { + let result = release_source::for_package(pkg).and_then(|host| { + process_package(&client, host.as_ref(), &state_dir, &work_dir, name, pkg) + }); + if let Err(err) = result { eprintln!(" error: {err:#}"); any_failed = true; } @@ -118,13 +120,13 @@ fn decide_tier_action(tier: u8, passed: bool, already_pending_this_version: bool fn process_package( client: &reqwest::blocking::Client, - endpoints: &GithubEndpoints, + host: &dyn ReleaseSource, state_dir: &Path, work_dir: &Path, name: &str, pkg: &Package, ) -> Result<()> { - let latest = checker::latest_github_release(client, endpoints, &pkg.repo)?; + let latest = host.latest_release(client, &pkg.repo)?; let last_seen = state::load_last_version(state_dir, name); if last_seen.as_deref() == Some(latest.as_str()) { println!(" up to date at {latest}"); @@ -132,7 +134,7 @@ fn process_package( } println!(" new version detected: {latest} (previously: {last_seen:?})"); - let fetched = fetch_and_verify(client, endpoints, work_dir, name, pkg, &latest)?; + let fetched = fetch_and_verify(client, host, work_dir, name, pkg, &latest)?; println!(" fetched {}", fetched.asset.path.display()); println!( " verification (tier {}): {} — {}", @@ -205,7 +207,7 @@ struct FetchVerifyResult { /// trusting a possibly-stale flag from an earlier run). fn fetch_and_verify( client: &reqwest::blocking::Client, - endpoints: &GithubEndpoints, + host: &dyn ReleaseSource, work_dir: &Path, name: &str, pkg: &Package, @@ -215,10 +217,11 @@ fn fetch_and_verify( let asset_name = pkg.asset_pattern.replace("{version}", &version); let dest_dir = work_dir.join(name).join(tag); - let asset = fetcher::download_asset(client, endpoints, &pkg.repo, tag, &asset_name, &dest_dir)?; + let api = host.api(); + let asset = fetcher::download_asset(client, api, &pkg.repo, tag, &asset_name, &dest_dir)?; let verification = verifier::verify( client, - endpoints, + api, &pkg.verification, &pkg.repo, tag, @@ -317,11 +320,11 @@ pub fn run_review(args: &[String]) -> Result<()> { fn approve(state_dir: &Path, work_dir: &Path, name: &str, pkg: &Package, tag: &str) -> Result<()> { let client = build_client()?; - let endpoints = GithubEndpoints::default(); + let host = release_source::for_package(pkg)?; // Re-verify rather than trusting the earlier flag: the artifact at // this tag could in principle have changed since it was queued. - let fetched = fetch_and_verify(&client, &endpoints, work_dir, name, pkg, tag)?; + let fetched = fetch_and_verify(&client, host.as_ref(), work_dir, name, pkg, tag)?; if !fetched.verification.passed { bail!( "re-verification failed on approve: {}", @@ -343,6 +346,8 @@ fn approve(state_dir: &Path, work_dir: &Path, name: &str, pkg: &Package, tag: &s #[cfg(test)] mod tests { use super::*; + use crate::release_source::{ForgejoEndpoints, GithubEndpoints}; + use crate::test_support::same_origin_package; #[test] fn decide_tier_action_failed_verification_overrides_everything() { @@ -374,6 +379,67 @@ mod tests { assert_eq!(decide_tier_action(4, true, true), TierAction::StillPending); } + /// Mocks the release-tag, asset, and checksum endpoints for `o/r@v1.0.0` + /// with a checksum that doesn't match the asset, so verification fails. + /// These are identical on GitHub and Forgejo (see `ReleaseSource::api`); only + /// how the latest tag is found differs per test. Returned mocks must + /// stay alive for the test's duration. + fn mock_release_with_bad_checksum(server: &mut mockito::ServerGuard) -> Vec { + let asset_url = format!("{}/download/thing.tar.gz", server.url()); + let sums_url = format!("{}/download/SHA256SUMS", server.url()); + let release_body = format!( + r#"{{"assets": [ + {{"name": "thing.tar.gz", "browser_download_url": "{asset_url}"}}, + {{"name": "SHA256SUMS", "browser_download_url": "{sums_url}"}} + ]}}"# + ); + vec![ + server + .mock("GET", "/repos/o/r/releases/tags/v1.0.0") + .with_status(200) + .with_body(release_body) + .create(), + server + .mock("GET", "/download/thing.tar.gz") + .with_status(200) + .with_body(b"artifact-bytes".as_slice()) + .create(), + // Wrong hash for "artifact-bytes" — forces a verification failure. + server + .mock("GET", "/download/SHA256SUMS") + .with_status(200) + .with_body( + "0000000000000000000000000000000000000000000000000000000000000000 thing.tar.gz\n", + ) + .create(), + ] + } + + /// Runs `process_package` for a package whose checksum is wrong and + /// asserts it errors with "verification failed" while leaving no trace + /// in state. + fn assert_verification_failure_is_an_error(host: &dyn ReleaseSource, pkg: &Package) { + let client = reqwest::blocking::Client::new(); + let state_dir = tempfile::tempdir().unwrap(); + let work_dir = tempfile::tempdir().unwrap(); + + let err = process_package( + &client, + host, + state_dir.path(), + work_dir.path(), + "thing", + pkg, + ) + .unwrap_err(); + + assert!(err.to_string().contains("verification failed")); + // Neither published nor queued for review — a failed verification + // shouldn't leave any trace in state. + assert_eq!(state::load_last_version(state_dir.path(), "thing"), None); + assert_eq!(state::load_pending_version(state_dir.path(), "thing"), None); + } + /// Regression test for the exit-code gap this PR fixes: a verification /// failure previously returned `Ok(())` from `process_package`, so /// `run_check` never counted it as a failure and the process exited 0 @@ -384,11 +450,10 @@ mod tests { #[test] fn process_package_returns_err_on_verification_failure() { let mut server = mockito::Server::new(); - let endpoints = GithubEndpoints { + let github = GithubEndpoints { web: server.url(), api: server.url(), }; - let feed = format!( r#""#, server.url() @@ -398,63 +463,27 @@ mod tests { .with_status(200) .with_body(feed) .create(); + let _release_mocks = mock_release_with_bad_checksum(&mut server); - let asset_url = format!("{}/download/thing.tar.gz", server.url()); - let sums_url = format!("{}/download/SHA256SUMS", server.url()); - let release_body = format!( - r#"{{"assets": [ - {{"name": "thing.tar.gz", "browser_download_url": "{asset_url}"}}, - {{"name": "SHA256SUMS", "browser_download_url": "{sums_url}"}} - ]}}"# - ); - let _release = server - .mock("GET", "/repos/o/r/releases/tags/v1.0.0") + assert_verification_failure_is_an_error(&github, &same_origin_package("")); + } + + /// Same as above but through a Forgejo source, proving the whole + /// check -> fetch -> verify path works there too: the error is the + /// verification failure, not a fetch or check failure on the way to it. + #[test] + fn process_package_returns_err_on_verification_failure_via_forgejo() { + let mut server = mockito::Server::new(); + let forgejo = ForgejoEndpoints { api: server.url() }; + let _latest = server + .mock("GET", "/repos/o/r/releases/latest") .with_status(200) - .with_body(release_body) - .create(); - let _asset = server - .mock("GET", "/download/thing.tar.gz") - .with_status(200) - .with_body(b"artifact-bytes".as_slice()) - .create(); - // Wrong hash for "artifact-bytes" — forces a verification failure. - let _sums = server - .mock("GET", "/download/SHA256SUMS") - .with_status(200) - .with_body( - "0000000000000000000000000000000000000000000000000000000000000000 thing.tar.gz\n", - ) + .with_body(r#"{"tag_name": "v1.0.0"}"#) .create(); + let _release_mocks = mock_release_with_bad_checksum(&mut server); - let pkg: Package = toml::from_str( - r#" - repo = "o/r" - asset_pattern = "thing.tar.gz" - [verification] - method = "same-origin-sha256" - checksum_asset_pattern = "SHA256SUMS" - "#, - ) - .unwrap(); - - let client = reqwest::blocking::Client::new(); - let state_dir = tempfile::tempdir().unwrap(); - let work_dir = tempfile::tempdir().unwrap(); - - let err = process_package( - &client, - &endpoints, - state_dir.path(), - work_dir.path(), - "thing", - &pkg, - ) - .unwrap_err(); - - assert!(err.to_string().contains("verification failed")); - // Neither published nor queued for review — a failed verification - // shouldn't leave any trace in state. - assert_eq!(state::load_last_version(state_dir.path(), "thing"), None); - assert_eq!(state::load_pending_version(state_dir.path(), "thing"), None); + // The package's own `source` is irrelevant here: `forgejo` is + // hand-built to point at the mock server, bypassing `for_package`. + assert_verification_failure_is_an_error(&forgejo, &same_origin_package("")); } } diff --git a/src/release_source/contract.rs b/src/release_source/contract.rs new file mode 100644 index 0000000..2cde371 --- /dev/null +++ b/src/release_source/contract.rs @@ -0,0 +1,20 @@ +//! The `ReleaseSource` trait: what the pipeline needs from a hosting +//! service a package's releases are published on. Each host implements it +//! in its own file next to this one, so per-host logic never accumulates +//! here. + +use anyhow::Result; + +/// `Debug` so a `Box` can sit in a `Result` that tests +/// unwrap. +pub trait ReleaseSource: std::fmt::Debug { + /// The latest release tag for `repo`. + fn latest_release(&self, client: &reqwest::blocking::Client, repo: &str) -> Result; + + /// The releases API root, which `fetcher` and `verifier` build their + /// own paths under. GitHub and Forgejo both serve + /// `/repos/{owner}/{repo}/releases/tags/{tag}` there with the same + /// `assets[].{name, browser_download_url}` shape, which is why those + /// stages need only this and not the source itself. + fn api(&self) -> &str; +} diff --git a/src/release_source/forgejo.rs b/src/release_source/forgejo.rs new file mode 100644 index 0000000..6ff070a --- /dev/null +++ b/src/release_source/forgejo.rs @@ -0,0 +1,112 @@ +//! A Forgejo (or Gitea) instance as a release source: its API root, and +//! how to find a repo's latest release there. + +use super::ReleaseSource; +use anyhow::{Context, Result, bail}; +use serde::Deserialize; + +#[derive(Debug, Clone)] +pub struct ForgejoEndpoints { + /// The instance's API root, i.e. `/api/v1`. + pub api: String, +} + +impl ForgejoEndpoints { + /// `base_url` is the instance's web root, e.g. + /// `https://code.austinschaefer.com`; a trailing slash is tolerated. + pub fn from_base_url(base_url: &str) -> Self { + Self { + api: format!("{}/api/v1", base_url.trim_end_matches('/')), + } + } +} + +impl ReleaseSource for ForgejoEndpoints { + /// One call, unlike GitHub's feed-then-confirm dance: Forgejo's + /// `releases/latest` already returns only the newest non-draft, + /// non-prerelease *release object*, so a stray tag with no release + /// behind it (GitHub's scaleway-cli `-dbg1` problem) can't be returned. + fn latest_release(&self, client: &reqwest::blocking::Client, repo: &str) -> Result { + #[derive(Deserialize)] + struct Latest { + tag_name: String, + } + + let url = format!("{}/repos/{repo}/releases/latest", self.api); + let response = client.get(&url).send()?; + // Forgejo answers 404 both for an unknown repo and for one with no + // releases yet — the common state for a project's very first + // release. + if response.status() == reqwest::StatusCode::NOT_FOUND { + bail!("no published release found at {url} (repo missing, or nothing released yet)"); + } + let latest: Latest = response + .error_for_status() + .with_context(|| format!("fetching latest release from {url}"))? + .json()?; + Ok(latest.tag_name) + } + + fn api(&self) -> &str { + &self.api + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn from_base_url_appends_api_v1() { + let endpoints = ForgejoEndpoints::from_base_url("https://code.example.com"); + assert_eq!(endpoints.api(), "https://code.example.com/api/v1"); + } + + #[test] + fn from_base_url_tolerates_trailing_slash() { + let endpoints = ForgejoEndpoints::from_base_url("https://code.example.com/"); + assert_eq!(endpoints.api(), "https://code.example.com/api/v1"); + } + + #[test] + fn latest_release_returns_tag_name() { + let mut server = mockito::Server::new(); + let _latest = server + .mock("GET", "/repos/o/r/releases/latest") + .with_status(200) + .with_body(r#"{"tag_name": "v0.1.0", "assets": []}"#) + .create(); + + let client = reqwest::blocking::Client::new(); + let endpoints = ForgejoEndpoints { api: server.url() }; + assert_eq!(endpoints.latest_release(&client, "o/r").unwrap(), "v0.1.0"); + } + + #[test] + fn latest_release_names_the_no_releases_case() { + let mut server = mockito::Server::new(); + let _latest = server + .mock("GET", "/repos/o/r/releases/latest") + .with_status(404) + .create(); + + let client = reqwest::blocking::Client::new(); + let endpoints = ForgejoEndpoints { api: server.url() }; + let err = endpoints.latest_release(&client, "o/r").unwrap_err(); + assert!(err.to_string().contains("no published release")); + } + + #[test] + fn latest_release_surfaces_server_errors() { + let mut server = mockito::Server::new(); + let _latest = server + .mock("GET", "/repos/o/r/releases/latest") + .with_status(500) + .create(); + + let client = reqwest::blocking::Client::new(); + let endpoints = ForgejoEndpoints { api: server.url() }; + let err = endpoints.latest_release(&client, "o/r").unwrap_err(); + assert!(err.to_string().contains("fetching latest release")); + } +} diff --git a/src/release_source/github.rs b/src/release_source/github.rs new file mode 100644 index 0000000..eac9803 --- /dev/null +++ b/src/release_source/github.rs @@ -0,0 +1,166 @@ +//! GitHub as a release source: its endpoints, and how to find a repo's +//! latest release there. + +use super::ReleaseSource; +use anyhow::{Result, bail}; +use regex::Regex; + +/// Base URLs for GitHub's public web host (Atom feeds, release pages) and +/// its REST API, factored out so tests can point both at a local mock +/// server instead of the real github.com/api.github.com. +#[derive(Debug, Clone)] +pub struct GithubEndpoints { + pub web: String, + pub api: String, +} + +impl Default for GithubEndpoints { + fn default() -> Self { + Self { + web: "https://github.com".to_string(), + api: "https://api.github.com".to_string(), + } + } +} + +impl ReleaseSource for GithubEndpoints { + /// Resolves the latest release tag for `repo` via its public Atom feed. + /// + /// Deliberately not a full XML parse: the feed lists entries + /// newest-first, and each `"/>` is matched in document order. Revisit + /// with a real XML parser if GitHub's feed shape ever changes. + /// + /// The feed can list a tag newer than any tag with a real Release + /// object behind it — observed on scaleway/scaleway-cli, which pushes a + /// `vX.Y.Z-dbg1` tag (no corresponding Release; `releases/tags/` + /// 404s) right after each real release, and that tag sorts newest in + /// the feed. So each candidate is confirmed against the releases API in + /// feed order, returning the first that actually resolves. + fn latest_release(&self, client: &reqwest::blocking::Client, repo: &str) -> Result { + let url = format!("{}/{repo}/releases.atom", self.web); + let body = client.get(&url).send()?.error_for_status()?.text()?; + + let re = Regex::new(r#"releases/tag/([^"]+)""#)?; + let mut candidates = re + .captures_iter(&body) + .map(|caps| caps[1].to_string()) + .peekable(); + if candidates.peek().is_none() { + bail!("no release tag found in {url}"); + } + + for tag in candidates { + let release_url = format!("{}/repos/{repo}/releases/tags/{tag}", self.api); + if client.get(&release_url).send()?.status().is_success() { + return Ok(tag); + } + } + bail!("no release tag in {url} resolved to a real release via the API") + } + + fn api(&self) -> &str { + &self.api + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn default_points_at_real_github() { + let endpoints = GithubEndpoints::default(); + assert_eq!(endpoints.web, "https://github.com"); + assert_eq!(endpoints.api, "https://api.github.com"); + } + + fn atom_feed(tags: &[&str]) -> String { + let entries: String = tags + .iter() + .map(|t| { + format!(r#""#) + }) + .collect(); + format!("{entries}") + } + + #[test] + fn latest_release_skips_tags_with_no_real_release() { + let mut server = mockito::Server::new(); + let endpoints = GithubEndpoints { + web: server.url(), + api: server.url(), + }; + + // Mirrors the real scaleway-cli case: newest feed entry (a -dbg1 + // tag) has no Release object behind it and 404s. + let _feed = server + .mock("GET", "/o/r/releases.atom") + .with_status(200) + .with_body(atom_feed(&["v2.62.0-dbg1", "v2.62.0"])) + .create(); + let _missing = server + .mock("GET", "/repos/o/r/releases/tags/v2.62.0-dbg1") + .with_status(404) + .create(); + let _real = server + .mock("GET", "/repos/o/r/releases/tags/v2.62.0") + .with_status(200) + .with_body("{}") + .create(); + + let client = reqwest::blocking::Client::new(); + let tag = endpoints.latest_release(&client, "o/r").unwrap(); + assert_eq!(tag, "v2.62.0"); + } + + #[test] + fn latest_release_errors_when_feed_has_no_tags() { + let mut server = mockito::Server::new(); + let endpoints = GithubEndpoints { + web: server.url(), + api: server.url(), + }; + let _feed = server + .mock("GET", "/o/r/releases.atom") + .with_status(200) + .with_body("") + .create(); + + let client = reqwest::blocking::Client::new(); + let err = endpoints.latest_release(&client, "o/r").unwrap_err(); + assert!(err.to_string().contains("no release tag found")); + } + + #[test] + fn latest_release_errors_when_no_candidate_resolves() { + let mut server = mockito::Server::new(); + let endpoints = GithubEndpoints { + web: server.url(), + api: server.url(), + }; + let _feed = server + .mock("GET", "/o/r/releases.atom") + .with_status(200) + .with_body(atom_feed(&["v1.0.0-dbg1"])) + .create(); + let _missing = server + .mock("GET", "/repos/o/r/releases/tags/v1.0.0-dbg1") + .with_status(404) + .create(); + + let client = reqwest::blocking::Client::new(); + let err = endpoints.latest_release(&client, "o/r").unwrap_err(); + assert!(err.to_string().contains("resolved to a real release")); + } + + #[test] + fn api_is_the_configured_api_root() { + let endpoints = GithubEndpoints { + web: "http://w".into(), + api: "http://a".into(), + }; + assert_eq!(endpoints.api(), "http://a"); + } +} diff --git a/src/release_source/mod.rs b/src/release_source/mod.rs new file mode 100644 index 0000000..2f75e02 --- /dev/null +++ b/src/release_source/mod.rs @@ -0,0 +1,57 @@ +//! Where a package's releases are published: the `ReleaseSource` trait, +//! one file per host implementing it, and `for_package`, which picks the +//! implementation for a package from its configured `source`. The +//! submodules are private and re-exported here, so the rest of the crate +//! imports everything from `crate::release_source` and never a host's +//! file. + +mod contract; +mod forgejo; +mod github; + +pub use contract::ReleaseSource; +pub use forgejo::ForgejoEndpoints; +pub use github::GithubEndpoints; + +use crate::config::{Package, Source}; +use anyhow::{Context, Result}; + +/// Defensive: errors only if a `forgejo-release` package has no +/// `base_url`, which `config::load_packages_dir` already guarantees. +pub fn for_package(pkg: &Package) -> Result> { + match pkg.source { + Source::GithubRelease => Ok(Box::new(GithubEndpoints::default())), + Source::ForgejoRelease => { + let base_url = pkg + .base_url + .as_deref() + .context("source = \"forgejo-release\" needs a base_url")?; + Ok(Box::new(ForgejoEndpoints::from_base_url(base_url))) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::test_support::same_origin_package as package; + + #[test] + fn github_source_uses_real_github() { + let source = for_package(&package("")).unwrap(); + assert_eq!(source.api(), "https://api.github.com"); + } + + #[test] + fn forgejo_source_uses_the_instances_api() { + let pkg = package("source = \"forgejo-release\"\nbase_url = \"https://code.example.com\""); + let source = for_package(&pkg).unwrap(); + assert_eq!(source.api(), "https://code.example.com/api/v1"); + } + + #[test] + fn forgejo_source_without_base_url_errors() { + let err = for_package(&package("source = \"forgejo-release\"")).unwrap_err(); + assert!(err.to_string().contains("needs a base_url")); + } +} diff --git a/src/test_support.rs b/src/test_support.rs index d53c80b..09d72f4 100644 --- a/src/test_support.rs +++ b/src/test_support.rs @@ -1,10 +1,11 @@ //! Test-only fixture helpers shared across modules' `#[cfg(test)]` code -//! (`publisher`, `sanity`, `notifier`) — not production code, and not built outside -//! `cargo test`. See docs/ARCHITECTURE.md > "organize by pipeline stage, not -//! by layer": this exists to remove one specific piece of duplication -//! (two near-identical copies of "write an executable shell script"), not -//! as a general test-utils dump. +//! — not production code, and not built outside `cargo test`. See +//! docs/ARCHITECTURE.md > "organize by pipeline stage, not by layer": this +//! exists to remove specific pieces of duplication (near-identical copies +//! of "write an executable shell script" and of "build a same-origin +//! `Package` from TOML"), not as a general test-utils dump. +use crate::config::Package; use std::path::{Path, PathBuf}; use std::process::Command; use std::time::{Duration, Instant}; @@ -52,3 +53,22 @@ fn wait_until_executable(path: &Path) { } } } + +/// A `same-origin-sha256` `Package` for repo `o/r`, asset `thing.tar.gz`, +/// checksum asset `SHA256SUMS`. `extra` is spliced in as top-level keys +/// before the `[verification]` table (e.g. `source`/`base_url`), and is +/// parsed directly rather than through `config::load_packages_dir`, so it +/// skips load-time validation. +pub(crate) fn same_origin_package(extra: &str) -> Package { + toml::from_str(&format!( + r#" + repo = "o/r" + asset_pattern = "thing.tar.gz" + {extra} + [verification] + method = "same-origin-sha256" + checksum_asset_pattern = "SHA256SUMS" + "# + )) + .unwrap() +} diff --git a/src/verifier.rs b/src/verifier.rs index 7ffa4d9..7b301d6 100644 --- a/src/verifier.rs +++ b/src/verifier.rs @@ -6,7 +6,6 @@ use crate::checker::version_from_tag; use crate::config::Verification; use crate::fetcher; -use crate::github::GithubEndpoints; use crate::hash; use anyhow::{Context, Result, bail}; use std::path::Path; @@ -23,7 +22,7 @@ pub struct VerificationResult { /// each tier does and does not prove. pub fn verify( client: &reqwest::blocking::Client, - endpoints: &GithubEndpoints, + api: &str, verification: &Verification, repo: &str, tag: &str, @@ -36,14 +35,8 @@ pub fn verify( } => { let checksum_asset_name = checksum_asset_pattern.replace("{version}", version_from_tag(tag)); - let checksum_asset = fetcher::download_asset( - client, - endpoints, - repo, - tag, - &checksum_asset_name, - dest_dir, - )?; + let checksum_asset = + fetcher::download_asset(client, api, repo, tag, &checksum_asset_name, dest_dir)?; let checksum_text = std::fs::read_to_string(&checksum_asset.path)?; let artifact_name = artifact_path .file_name() @@ -184,10 +177,7 @@ mod tests { #[test] fn verify_same_origin_sha256_passes_on_matching_checksum() { let mut server = mockito::Server::new(); - let endpoints = GithubEndpoints { - web: server.url(), - api: server.url(), - }; + let api = server.url(); let dest_dir = tempfile::tempdir().unwrap(); let artifact_path = dest_dir.path().join("thing.tar.gz"); std::fs::write(&artifact_path, b"hello world").unwrap(); @@ -214,7 +204,7 @@ mod tests { let client = reqwest::blocking::Client::new(); let result = verify( &client, - &endpoints, + &api, &verification, "o/r", "v1.0.0", @@ -230,10 +220,7 @@ mod tests { #[test] fn verify_same_origin_sha256_fails_on_mismatched_checksum() { let mut server = mockito::Server::new(); - let endpoints = GithubEndpoints { - web: server.url(), - api: server.url(), - }; + let api = server.url(); let dest_dir = tempfile::tempdir().unwrap(); let artifact_path = dest_dir.path().join("thing.tar.gz"); std::fs::write(&artifact_path, b"hello world").unwrap(); @@ -261,7 +248,7 @@ mod tests { let client = reqwest::blocking::Client::new(); let result = verify( &client, - &endpoints, + &api, &verification, "o/r", "v1.0.0",