diff --git a/bots/rhodibot/src/canon.rs b/bots/rhodibot/src/canon.rs index 31266387..f11d24e6 100644 --- a/bots/rhodibot/src/canon.rs +++ b/bots/rhodibot/src/canon.rs @@ -39,7 +39,9 @@ //! The failure mode to avoid is the quiet one: a rule set that silently shrinks //! and reports every repository as compliant. +pub mod local; pub mod profile; +pub mod report; pub mod requirement; pub mod verdict; diff --git a/bots/rhodibot/src/canon/local.rs b/bots/rhodibot/src/canon/local.rs new file mode 100644 index 00000000..d1c88f1f --- /dev/null +++ b/bots/rhodibot/src/canon/local.rs @@ -0,0 +1,256 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! Reading a repository that is already on disk. +//! +//! The same checks run against a local checkout as against the API, which is +//! useful for three things: a repository too large for GitHub to list in one +//! response, a private repository with no token to hand, and checking before +//! pushing rather than after. +//! +//! What "the repository's files" means here is the **tracked** set, taken from +//! `git ls-files` when the directory is a work tree. A walk of the working +//! directory would count build output and editor scratch files, and one of +//! those can easily satisfy a criterion by accident -- a `CODE_OF_CONDUCT.md` +//! inside `target/` is not the repository's code of conduct. Where there is no +//! git to ask, the walk is the fallback, and it skips `.git` because those are +//! not files anybody commits. + +use std::path::Path; +use std::process::Command; + +use anyhow::{Context, Result, ensure}; + +/// The two paths the canon's baseline allows for a profile, in preference +/// order: both are acceptable, and the first is the estate's. +const PROFILE_PATHS: [&str; 2] = [ + ".machine_readable/rsr-profile.a2ml", + "machine-readable/rsr-profile.a2ml", +]; + +/// A local checkout, read. +#[derive(Debug)] +pub struct LocalSource { + /// The repository's tracked files, relative to its root. + pub files: Vec, + /// The text of its profile, when it has one. + pub profile: Option, + /// Whether the file list came from git or from a walk. + from_git: bool, +} + +impl LocalSource { + /// Whether the file list came from git rather than a walk. + /// + /// Worth reporting: a walked list includes untracked files, so a criterion + /// can be satisfied by something the repository does not actually carry. + pub fn from_git(&self) -> bool { + self.from_git + } + + /// Kept private so a `LocalSource` can only come from `read`, which is the + /// only thing that knows whether the list is the tracked set. + fn new(files: Vec, profile: Option, from_git: bool) -> Self { + Self { + files, + profile, + from_git, + } + } +} + +/// Read a checkout: its files, and its profile if it has one. +pub fn read(root: &Path) -> Result { + ensure!( + root.is_dir(), + "{} is not a directory; --path expects a checkout", + root.display() + ); + + let git_files = tracked_files(root); + let from_git = git_files.is_some(); + let files = match git_files { + Some(files) => files, + None => { + let mut files = Vec::new(); + walk(root, root, &mut files).with_context(|| format!("walking {}", root.display()))?; + files.sort(); + files + } + }; + + Ok(LocalSource::new(files, profile(root)?, from_git)) +} + +/// The repository's tracked files, when git can be asked. +fn tracked_files(root: &Path) -> Option> { + let output = Command::new("git") + .arg("-C") + .arg(root) + .arg("ls-files") + .output() + .ok()?; + + if !output.status.success() { + return None; + } + + let listing = String::from_utf8(output.stdout).ok()?; + let mut files: Vec = listing + .lines() + .map(str::trim) + .filter(|line| !line.is_empty()) + .map(str::to_string) + .collect(); + files.sort(); + files.dedup(); + Some(files) +} + +/// Every file under `directory`, as a path relative to `root`. +fn walk(root: &Path, directory: &Path, files: &mut Vec) -> Result<()> { + for entry in + std::fs::read_dir(directory).with_context(|| format!("reading {}", directory.display()))? + { + let entry = entry?; + let path = entry.path(); + let name = entry.file_name(); + let name = name.to_string_lossy(); + + if path.is_dir() { + if name == ".git" { + continue; + } + walk(root, &path, files)?; + continue; + } + + let relative = path + .strip_prefix(root) + .with_context(|| format!("{} is not under {}", path.display(), root.display()))?; + files.push(relative.to_string_lossy().replace('\\', "/")); + } + Ok(()) +} + +/// The repository's profile text, if it has one. +fn profile(root: &Path) -> Result> { + for candidate in PROFILE_PATHS { + let path = root.join(candidate); + match std::fs::read_to_string(&path) { + Ok(text) => return Ok(Some(text)), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, + Err(error) => { + // Present but unreadable is not absent: reading it as "no + // profile" would silently shrink the check to the universal + // criteria and report a cleaner scorecard than the truth. + return Err(error).with_context(|| format!("reading {}", path.display())); + } + } + } + Ok(None) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A scratch directory with a unique name, removed on drop. + struct Scratch(std::path::PathBuf); + + impl Scratch { + fn new(label: &str) -> Self { + let path = + std::env::temp_dir().join(format!("rhodibot-local-{}-{label}", std::process::id())); + let _ = std::fs::remove_dir_all(&path); + std::fs::create_dir_all(&path).expect("the scratch directory is creatable"); + Self(path) + } + + fn write(&self, relative: &str, contents: &str) -> &Self { + let path = self.0.join(relative); + std::fs::create_dir_all(path.parent().expect("has a parent")) + .expect("parent directories are creatable"); + std::fs::write(&path, contents).expect("the file is writable"); + self + } + + fn path(&self) -> &Path { + &self.0 + } + } + + impl Drop for Scratch { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + #[test] + fn a_walk_skips_dot_git_and_finds_the_rest() { + let scratch = Scratch::new("walk"); + scratch.write("README.adoc", "hi"); + scratch.write(".machine_readable/descriptiles/STATE.a2ml", "state"); + // A file inside .git is not a file the repository carries. + scratch.write(".git/objects/deadbeef", "not yours"); + + let files = file_list_for(&scratch); + assert_eq!( + files, + vec![ + ".machine_readable/descriptiles/STATE.a2ml".to_string(), + "README.adoc".to_string() + ] + ); + } + + #[test] + fn a_profile_is_read_from_either_allowed_path() { + let estate = Scratch::new("profile-estate"); + estate.write( + ".machine_readable/rsr-profile.a2ml", + "[rsr-profile]\ncapabilities = [\"bash\"]\n", + ); + let source = read(estate.path()).expect("reads"); + assert!( + source + .profile + .as_deref() + .expect("a profile") + .contains("capabilities") + ); + + let alternate = Scratch::new("profile-alt"); + alternate.write( + "machine-readable/rsr-profile.a2ml", + "[rsr-profile]\ncapabilities = [\"bash\"]\n", + ); + assert!(read(alternate.path()).expect("reads").profile.is_some()); + } + + #[test] + fn no_profile_is_none_rather_than_an_error() { + let scratch = Scratch::new("no-profile"); + scratch.write("README.adoc", "hi"); + let source = read(scratch.path()).expect("reads"); + assert_eq!(source.profile, None, "a repository may declare nothing"); + } + + #[test] + fn a_path_that_is_not_a_directory_is_refused() { + let error = read(Path::new("/nonexistent-rhodibot-check")).expect_err("not a directory"); + assert!( + format!("{error:#}").contains("not a directory"), + "{error:#}" + ); + } + + /// The walk's own answer, bypassing git: the scratch directory is not a + /// work tree, so this is what `read` produces, but asserting on it directly + /// keeps the test meaningful on a machine where /tmp is inside a repository. + fn file_list_for(scratch: &Scratch) -> Vec { + let mut files = Vec::new(); + walk(scratch.path(), scratch.path(), &mut files).expect("the walk succeeds"); + files.sort(); + files + } +} diff --git a/bots/rhodibot/src/canon/report.rs b/bots/rhodibot/src/canon/report.rs new file mode 100644 index 00000000..2ab9ae89 --- /dev/null +++ b/bots/rhodibot/src/canon/report.rs @@ -0,0 +1,668 @@ +// SPDX-License-Identifier: MPL-2.0 + +//! A repository's canon conformance, assembled from the three steps before it. +//! +//! Step one reads what a criterion asks for from its own description, step two +//! decides whether the criterion applies to this repository at all, and step +//! three says what was found. This turns those into one report: what was asked, +//! what applies, what was found, and what to do about it. +//! +//! The report is deliberately *quiet about the unapplicable*. A gated criterion +//! a repository does not declare is counted as `na` and left out of the +//! findings, because reporting it would be reporting a requirement that does +//! not exist -- the same mistake as reading `template_ref` as a requirement, +//! one level up. + +use std::collections::BTreeMap; + +use anyhow::Result; +use serde::Serialize; + +use super::Canon; +use super::profile::{GateTable, Profile}; +use super::verdict::{Deprecation, GroupVerdict, Severity, Verdict}; + +/// What a repository would be told, if it asked. +#[derive(Debug, Serialize)] +pub struct CanonReport { + /// What was checked: `owner/repo`, or a path. + pub subject: String, + /// The criteria the canon holds, before any filtering. + pub criteria: usize, + /// Criteria scored against this repository: applicable, and asking about files. + pub scored: usize, + /// Gated criteria the repository does not declare a capability for. `na`, + /// and excluded from the denominator rather than counted against it. + pub not_applicable: usize, + /// Criteria that apply but ask a content question rather than naming files. + /// A human answers these, or hypatia's rules do; this report cannot. + pub not_file_questions: usize, + /// How the scored criteria came out. + pub counts: Counts, + /// Everything not satisfied, worst first. Empty is the good outcome. + pub findings: Vec, + /// The locations the canon has retired, so a report can explain a + /// "deprecated location" verdict without the reader having to find it. + pub retired_locations: Vec, + /// Capabilities the repository declared, if it has a profile. + pub declared_capabilities: Vec, +} + +#[derive(Debug, Default, Serialize, PartialEq, Eq)] +pub struct Counts { + pub satisfied: usize, + pub relocated: usize, + pub deprecated: usize, + pub missing: usize, +} + +impl Counts { + fn record(&mut self, severity: Severity) { + match severity { + Severity::Satisfied => self.satisfied += 1, + Severity::Relocated => self.relocated += 1, + Severity::Deprecated => self.deprecated += 1, + Severity::Missing => self.missing += 1, + } + } +} + +#[derive(Debug, Serialize)] +pub struct RetiredLocation { + pub location: String, + pub since: String, + pub stated_by: String, +} + +/// One criterion the repository does not satisfy, and why. +#[derive(Debug, Serialize)] +pub struct Finding { + pub id: String, + pub name: String, + pub tier: String, + pub severity: Severity, + /// Where the canon's own template satisfies it, when that differs from + /// what was found. A note, never a fault. + pub canon_keeps_it_at: Option, + /// What the description asks for. + pub expected: Vec, + /// What the repository has instead, by that name. + pub found: Vec, + /// Copies of the same file under a retired location. + pub deprecated_copies: Vec, + /// One line per unsatisfied group, ready to print. + pub details: Vec, +} + +/// How bad a finding has to be before the command fails. +/// +/// Advisory by default. The canon designates hypatia's `rsr-conformance` as the +/// single normative checker, so a gate here would be a second opinion claiming +/// authority it does not have -- but a repository that *wants* the exit code can +/// ask for one. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FailOn { + Nothing, + Relocated, + Deprecated, + Missing, +} + +impl FailOn { + /// The severity that fails, if any. + pub fn threshold(self) -> Option { + match self { + Self::Nothing => None, + Self::Relocated => Some(Severity::Relocated), + Self::Deprecated => Some(Severity::Deprecated), + Self::Missing => Some(Severity::Missing), + } + } + + pub fn parse(value: &str) -> Result { + match value { + "none" | "nothing" => Ok(Self::Nothing), + "relocated" => Ok(Self::Relocated), + "deprecated" => Ok(Self::Deprecated), + "missing" => Ok(Self::Missing), + other => anyhow::bail!( + "unknown --fail-on value {other:?}; expected one of none, relocated, deprecated, \ + missing" + ), + } + } + + pub fn as_str(self) -> &'static str { + match self { + Self::Nothing => "none", + Self::Relocated => "relocated", + Self::Deprecated => "deprecated", + Self::Missing => "missing", + } + } + + /// Does this report contain a finding at or above the threshold? + pub fn breached_by(self, report: &CanonReport) -> bool { + let Some(threshold) = self.threshold() else { + return false; + }; + report + .findings + .iter() + .any(|finding| finding.severity >= threshold) + } +} + +impl CanonReport { + /// Check one repository's file list against the canon. + /// + /// `profile_source` is the text of the repository's + /// `.machine_readable/rsr-profile.a2ml`, or `None` when it has none -- which + /// means it declares no capabilities, not that it is non-compliant. + pub fn build( + subject: &str, + canon: &Canon, + gates: &GateTable, + files: &[String], + profile_source: Option<&str>, + ) -> Result { + let profile = match profile_source { + Some(source) => Profile::parse(source, gates)?, + None => Profile::default(), + }; + let deprecations = Deprecation::from_canon(canon)?; + + let mut report = Self { + subject: subject.to_string(), + criteria: canon.criterion_count(), + scored: 0, + not_applicable: 0, + not_file_questions: 0, + counts: Counts::default(), + findings: Vec::new(), + retired_locations: deprecations + .iter() + .map(|deprecation| RetiredLocation { + location: deprecation.location.clone(), + since: deprecation.since.clone(), + stated_by: deprecation.stated_by.clone(), + }) + .collect(), + declared_capabilities: profile.declared().cloned().collect(), + }; + + for criterion in canon.criteria() { + if !profile.is_applicable(criterion) { + report.not_applicable += 1; + continue; + } + + let Some(verdict) = Verdict::of(criterion, files, &deprecations) else { + report.not_file_questions += 1; + continue; + }; + + report.scored += 1; + let severity = verdict.severity(); + report.counts.record(severity); + + if severity.is_finding() { + report.findings.push(Finding::of(criterion, &verdict)); + } + } + + // Worst first, and stable, so equal severities stay in canon order -- + // which is category order, and the order a reader expects. + report + .findings + .sort_by_key(|finding| std::cmp::Reverse(finding.severity)); + + Ok(report) + } + + /// The report as a person reads it. + pub fn render(&self) -> String { + let mut out = String::new(); + + out.push_str(&format!("Canon conformance — {}\n", self.subject)); + for location in &self.retired_locations { + out.push_str(&format!( + " retired: {} (since {}, per criterion {})\n", + location.location, location.since, location.stated_by + )); + } + out.push('\n'); + + out.push_str(&format!( + " scored {} of {} criteria ({} na — capability not declared, {} ask content \ + questions, not files)\n", + self.scored, self.criteria, self.not_applicable, self.not_file_questions + )); + out.push_str(&format!( + " at path {} elsewhere {} deprecated location {} absent {}\n", + self.counts.satisfied, + self.counts.relocated, + self.counts.deprecated, + self.counts.missing + )); + + if !self.declared_capabilities.is_empty() { + out.push_str(&format!( + " declared: {}\n", + self.declared_capabilities.join(", ") + )); + } + + out.push('\n'); + if self.findings.is_empty() { + out.push_str(" nothing to act on.\n"); + return out; + } + + for finding in &self.findings { + out.push_str(&format!( + " {:<7} {:<6} {:<19} {}\n", + finding.id, + finding.tier, + finding.severity.as_str(), + finding.name + )); + for detail in &finding.details { + out.push_str(&format!(" {detail}\n")); + } + if let Some(note) = &finding.canon_keeps_it_at { + out.push_str(&format!(" canon keeps it at {note}\n")); + } + if !finding.deprecated_copies.is_empty() { + out.push_str(&format!( + " stale copy: {}\n", + finding.deprecated_copies.join(", ") + )); + } + } + + out + } + + /// How many findings there are, by severity. + pub fn findings_by_severity(&self) -> BTreeMap<&'static str, usize> { + let mut counts = BTreeMap::new(); + for finding in &self.findings { + *counts.entry(finding.severity.as_str()).or_insert(0) += 1; + } + counts + } +} + +impl Finding { + fn of(criterion: &super::Criterion, verdict: &Verdict) -> Self { + let mut expected = Vec::new(); + let mut found = Vec::new(); + let mut details = Vec::new(); + let mut canon_keeps_it_at = None; + + for group in &verdict.groups { + match group { + GroupVerdict::AtPath { .. } => {} + GroupVerdict::Deprecated { + found: path, + location, + since, + } => { + found.push(path.clone()); + details.push(format!( + "present, but {path} is under {location}, retired {since}" + )); + } + GroupVerdict::Elsewhere { + expected: alternatives, + found: elsewhere, + deprecated_copies, + } => { + expected.extend(alternatives.iter().cloned()); + found.extend(elsewhere.iter().cloned()); + if let Some(note) = + super::verdict::canon_location_note(&verdict.template_ref, &elsewhere[0]) + { + canon_keeps_it_at = Some(note); + } + let mut line = format!( + "present at {} — not {}", + elsewhere.join(", "), + alternatives.join(" or ") + ); + if !deprecated_copies.is_empty() { + line.push_str(&format!( + "; also a stale copy at {}", + deprecated_copies.join(", ") + )); + } + details.push(line); + } + GroupVerdict::Absent { + expected: alternatives, + near_misses, + } => { + expected.extend(alternatives.iter().cloned()); + let mut line = format!("absent — expected {}", alternatives.join(" or ")); + if !near_misses.is_empty() { + line.push_str(&format!( + "; there is a {} where the canon asks for one ending in {}", + near_misses.join(", "), + extension_of(&alternatives[0]) + )); + } + details.push(line); + } + } + } + + Self { + id: criterion.id.clone(), + name: criterion.name.clone(), + tier: criterion.tier.to_string(), + severity: verdict.severity(), + canon_keeps_it_at, + expected, + found, + deprecated_copies: verdict.deprecated_copies().into_iter().cloned().collect(), + details, + } + } +} + +/// The extension a criterion asked for, for a line about near misses. +fn extension_of(path: &str) -> String { + match path.rsplit_once('.') { + Some((_, extension)) => format!(".{extension}"), + None => path.to_string(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::canon::verdict::Severity; + + fn canon() -> Canon { + Canon::vendored().expect("the canon parses") + } + + fn gates() -> GateTable { + GateTable::vendored().expect("the gate table parses") + } + + fn files(paths: &[&str]) -> Vec { + paths.iter().map(|path| path.to_string()).collect() + } + + /// Everything the canon asks for in one of the named locations. + const COMPLIANT: &[&str] = &[ + "Justfile", + ".editorconfig", + ".pre-commit-config.yaml", + ".tool-versions", + "README.adoc", + "LICENSE", + "LICENSES/MPL-2.0.txt", + "SECURITY.md", + "CODE_OF_CONDUCT.md", + "CONTRIBUTING.md", + ".gitignore", + ".gitattributes", + ".well-known/security.txt", + "0-AI-MANIFEST.a2ml", + ".machine_readable/rsr-profile.a2ml", + ".machine_readable/descriptiles/STATE.a2ml", + ".machine_readable/descriptiles/META.a2ml", + ".machine_readable/descriptiles/ECOSYSTEM.a2ml", + ".machine_readable/descriptiles/AGENTIC.a2ml", + ".machine_readable/descriptiles/NEUROSYM.a2ml", + ".machine_readable/descriptiles/PLAYBOOK.a2ml", + ".machine_readable/descriptiles/anchors/ANCHOR.a2ml", + ]; + + #[test] + fn a_compliant_repository_has_nothing_to_act_on() { + let report = + CanonReport::build("acme/widgets", &canon(), &gates(), &files(COMPLIANT), None) + .expect("the report builds"); + + assert_eq!(report.findings.len(), 0, "{:#?}", report.findings); + assert_eq!(report.counts.missing, 0); + assert_eq!(report.counts.satisfied, report.scored); + assert!(report.render().contains("nothing to act on.")); + } + + #[test] + fn the_unapplicable_are_counted_and_not_reported() { + let report = + CanonReport::build("acme/widgets", &canon(), &gates(), &files(COMPLIANT), None) + .expect("the report builds"); + + // The vendored canon: 74 criteria, 26 gated on a capability. + assert_eq!(report.criteria, 74); + assert_eq!(report.not_applicable, 26); + assert_eq!(report.scored, 22); + assert_eq!(report.not_file_questions, 26); + assert_eq!( + report.findings.len() + report.counts.satisfied, + report.scored + ); + assert!( + report.findings.iter().all(|finding| ![ + // governance-tier, docs-site and web-ui criteria the pilot's + // repositories did not declare + "2.1.7", "2.1.8", "2.1.9", "3.1.9", "9.1.4", "11.1.2" + ] + .contains(&finding.id.as_str())), + "a criterion the repository cannot satisfy from lack of a capability must not be \ + reported at all" + ); + } + + #[test] + fn declaring_a_capability_brings_its_criteria_into_scope() { + let profile = "[rsr-profile]\nversion = \"1.0.0\"\ncapabilities = [\"governance-tier\"]\n"; + + let without = + CanonReport::build("acme/widgets", &canon(), &gates(), &files(COMPLIANT), None) + .expect("builds"); + let with = CanonReport::build( + "acme/widgets", + &canon(), + &gates(), + &files(COMPLIANT), + Some(profile), + ) + .expect("builds"); + + // Ten criteria gate on governance-tier; six of them ask about files + // and four ask content questions, so declaring the capability moves ten + // out of `na` and adds six to `scored`. + assert_eq!(without.not_applicable - with.not_applicable, 10); + assert_eq!(without.scored + 6, with.scored); + assert_eq!(without.not_file_questions + 4, with.not_file_questions); + assert_eq!( + with.declared_capabilities, + vec!["governance-tier".to_string()] + ); + assert!( + with.findings.iter().any(|finding| finding.id == "2.1.7"), + "MAINTAINERS.adoc is now asked for, and this repository does not have it" + ); + } + + #[test] + fn a_retired_leftover_is_reported_with_the_deprecation_that_retired_it() { + let report = CanonReport::build( + "acme/widgets", + &canon(), + &gates(), + &files(&[".machine_readable/6a2/STATE.a2ml"]), + None, + ) + .expect("builds"); + + let finding = report + .findings + .iter() + .find(|finding| finding.id == "3.1.2") + .expect("3.1.2 asks for STATE.a2ml"); + + assert_eq!(finding.severity, Severity::Deprecated); + assert!( + finding.details[0].contains(".machine_readable/6a2/") + && finding.details[0].contains("2026-06-30"), + "{:?}", + finding.details + ); + assert_eq!( + report.retired_locations.len(), + 1, + "the report can explain the word it just used" + ); + assert!(report.render().contains("retired: .machine_readable/6a2/")); + } + + #[test] + fn findings_are_worst_first_and_stable_within_a_severity() { + let report = CanonReport::build( + "acme/widgets", + &canon(), + &gates(), + &files(&[".machine_readable/6a2/STATE.a2ml"]), + None, + ) + .expect("builds"); + + let severities: Vec = report.findings.iter().map(|f| f.severity).collect(); + let mut sorted = severities.clone(); + sorted.sort_by_key(|severity| std::cmp::Reverse(*severity)); + assert_eq!(severities, sorted, "findings must read worst first"); + } + + #[test] + fn a_near_miss_is_described_by_what_the_canon_asked_for() { + let report = CanonReport::build( + "acme/widgets", + &canon(), + &gates(), + &files(&["CODE_OF_CONDUCT.adoc"]), + None, + ) + .expect("builds"); + + let finding = report + .findings + .iter() + .find(|finding| finding.id == "2.1.4") + .expect("2.1.4 asks for CODE_OF_CONDUCT.md"); + + assert_eq!(finding.severity, Severity::Missing); + assert!( + finding.details[0].contains("CODE_OF_CONDUCT.adoc") + && finding.details[0].contains(".md"), + "{:?}", + finding.details + ); + } + + #[test] + fn fail_on_is_advisory_until_asked_otherwise() { + let report = CanonReport::build( + "acme/widgets", + &canon(), + &gates(), + &files(&[".machine_readable/6a2/STATE.a2ml"]), + None, + ) + .expect("builds"); + assert!(report.counts.deprecated > 0 && report.counts.missing > 0); + + // The default: report, do not fail. + assert!(!FailOn::Nothing.breached_by(&report)); + // Opt-in thresholds, in order of how much they tolerate. + assert!(FailOn::Relocated.breached_by(&report)); + assert!(FailOn::Deprecated.breached_by(&report)); + assert!(FailOn::Missing.breached_by(&report)); + + let satisfiable = + CanonReport::build("acme/widgets", &canon(), &gates(), &files(COMPLIANT), None) + .expect("builds"); + assert!(!FailOn::Relocated.breached_by(&satisfiable)); + assert!(!FailOn::Missing.breached_by(&satisfiable)); + } + + #[test] + fn the_severity_of_a_finding_is_the_worst_group_in_it() { + // 2.1.10 wants .gitignore and .gitattributes. A repository with a stale + // .gitignore and no .gitattributes at all is missing, not deprecated: + // the missing group is the thing to fix first. + let report = CanonReport::build( + "acme/widgets", + &canon(), + &gates(), + &files(&[".machine_readable/6a2/.gitignore"]), + None, + ) + .expect("builds"); + + let finding = report + .findings + .iter() + .find(|finding| finding.id == "2.1.10") + .expect("2.1.10 asks for two files"); + assert_eq!(finding.severity, Severity::Missing); + assert_eq!(finding.details.len(), 2, "{:?}", finding.details); + } + + #[test] + fn the_report_serialises_to_json() { + let report = CanonReport::build( + "acme/widgets", + &canon(), + &gates(), + &files(&[".machine_readable/6a2/STATE.a2ml"]), + None, + ) + .expect("builds"); + + let value = serde_json::to_value(&report).expect("serialises"); + assert_eq!(value["subject"], "acme/widgets"); + assert_eq!(value["criteria"], 74); + assert!(value["counts"]["deprecated"].as_u64().unwrap() > 0); + assert!( + value["findings"] + .as_array() + .unwrap() + .iter() + .any(|finding| { finding["severity"] == "deprecated" && finding["id"] == "3.1.2" }) + ); + assert_eq!(value["retired_locations"][0]["since"], "2026-06-30"); + } + + #[test] + fn an_unparseable_profile_is_an_error_not_an_empty_one() { + // A repository with a broken profile declares nothing, which would + // silently shrink the report to the universal criteria -- a clean + // scorecard for a repository whose declaration could not be read. + let error = CanonReport::build( + "acme/widgets", + &canon(), + &gates(), + &files(COMPLIANT), + Some("[rsr-profile]\ncapabilites = [\"rust\"]\n"), + ) + .expect_err("a misspelt key must not pass as a profile"); + assert!(format!("{error:#}").contains("capabilites"), "{error:#}"); + } + + #[test] + fn fail_on_rejects_a_value_it_does_not_know() { + assert_eq!(FailOn::parse("missing").expect("known"), FailOn::Missing); + assert_eq!(FailOn::parse("none").expect("known"), FailOn::Nothing); + let error = FailOn::parse("critical").expect_err("not a severity"); + assert!(format!("{error:#}").contains("critical"), "{error:#}"); + } +} diff --git a/bots/rhodibot/src/canon/verdict.rs b/bots/rhodibot/src/canon/verdict.rs index 14d33b24..57e61a7e 100644 --- a/bots/rhodibot/src/canon/verdict.rs +++ b/bots/rhodibot/src/canon/verdict.rs @@ -51,13 +51,14 @@ //! every retired path as ordinary. use anyhow::{Result, bail, ensure}; +use serde::Serialize; use super::Canon; use super::Criterion; use super::requirement::requirement_from; /// A location the canon has retired, and the sentence that says so. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct Deprecation { /// The retired location, resolved to a full path: `.machine_readable/6a2/`. pub location: String, @@ -219,7 +220,8 @@ impl GroupVerdict { /// Ordered by how far the repository is from what the canon asks: a file that /// is missing is worse than one in a retired place, which is worse than one /// somewhere the canon did not name. -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize)] +#[serde(rename_all = "kebab-case")] pub enum Severity { /// Every group is present where the canon records it. Satisfied, @@ -525,7 +527,7 @@ fn deprecated_copies_for( } /// Where the canon's template keeps this file, when that is somewhere else. -fn canon_location_note(template_ref: &str, found: &str) -> Option { +pub fn canon_location_note(template_ref: &str, found: &str) -> Option { if template_ref == "-" || template_ref.is_empty() || template_ref.ends_with('/') { return None; } diff --git a/bots/rhodibot/src/github.rs b/bots/rhodibot/src/github.rs index faccabef..8a1df8e5 100644 --- a/bots/rhodibot/src/github.rs +++ b/bots/rhodibot/src/github.rs @@ -181,6 +181,106 @@ impl GitHubClient { Ok(response.json().await?) } + /// Every file path in a repository's default branch. + /// + /// One request, not one per path: the canon's file-presence criteria ask + /// about a few dozen paths and a repository holds thousands of files, so + /// asking path by path would spend the whole rate limit on one repository. + /// + /// A truncated response is an error, not a short answer. GitHub truncates + /// the tree at 100,000 entries; a truncated list is missing files, and a + /// missing file reads as an absent one, which would manufacture findings + /// against a repository that has the file. Refusing is the honest option -- + /// there is no way to tell "absent" from "not fetched" in a partial tree. + pub async fn tree_paths(&self, owner: &str, repo: &str) -> Result> { + let repository = self.get_repository(owner, repo).await?; + self.tree_paths_at(owner, repo, &repository.default_branch) + .await + } + + /// Every file path at one ref. + pub async fn tree_paths_at( + &self, + owner: &str, + repo: &str, + reference: &str, + ) -> Result> { + sanitize::validate_file_path(reference)?; + let url = format!( + "{}/repos/{}/{}/git/trees/{}?recursive=1", + self.base_url, owner, repo, reference + ); + let request = self.authorize(self.client.get(&url), owner, repo).await?; + + let response = request + .header("Accept", "application/vnd.github+json") + .header("User-Agent", "rhodibot") + .send() + .await?; + + if !response.status().is_success() { + bail!( + "could not read the tree of {owner}/{repo} at {reference}: {}", + response.status() + ); + } + + let tree: TreeResponse = response.json().await?; + if tree.truncated { + bail!( + "{owner}/{repo} at {reference} is too large for GitHub to list in one response, \ + so the file list is incomplete. A partial list cannot tell an absent file from \ + an unfetched one, and reporting the difference as a finding would be wrong. \ + Check this repository with `--path` against a local clone instead." + ); + } + + Ok(tree + .tree + .into_iter() + .filter(|entry| entry.entry_type == "blob") + .map(|entry| entry.path) + .collect()) + } + + /// Read a file, treating "not found" as `None` rather than an error. + /// + /// The distinction matters for optional files: an absent + /// `.machine_readable/rsr-profile.a2ml` means a repository declares no + /// capabilities, which is an answer. Only a 404 is absence; anything else + /// is a real failure and is reported as one. + pub async fn get_file_content_if_present( + &self, + owner: &str, + repo: &str, + path: &str, + ) -> Result> { + sanitize::validate_file_path(path)?; + let url = format!( + "{}/repos/{}/{}/contents/{}", + self.base_url, owner, repo, path + ); + let request = self.authorize(self.client.get(&url), owner, repo).await?; + + let response = request + .header("Accept", "application/vnd.github.raw+json") + .header("User-Agent", "rhodibot") + .send() + .await?; + + if response.status() == reqwest::StatusCode::NOT_FOUND { + return Ok(None); + } + if !response.status().is_success() { + bail!( + "could not read {path} from {owner}/{repo}: {}", + response.status() + ); + } + + Ok(Some(response.text().await?)) + } + /// Check if a file exists /// /// The `path` parameter is validated against path traversal before use. @@ -323,6 +423,22 @@ pub struct License { } /// Content item from the GitHub contents API (directory listings). +/// A tree listing, as the git trees API returns it. +#[derive(Debug, Deserialize)] +struct TreeResponse { + tree: Vec, + /// Absent from the payload when false. + #[serde(default)] + truncated: bool, +} + +#[derive(Debug, Deserialize)] +struct TreeEntry { + path: String, + #[serde(rename = "type")] + entry_type: String, +} + #[derive(Debug, Deserialize)] pub struct ContentItem { pub name: String, @@ -513,6 +629,130 @@ mod tests { ); } + /// A repository whose default branch is `main`, as `/repos/{o}/{r}` reports. + async fn mount_repository(server: &MockServer) { + Mock::given(method("GET")) + .and(path("/repos/acme/widgets")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "id": 1, + "name": "widgets", + "full_name": "acme/widgets", + "description": null, + "default_branch": "main", + "language": null, + "topics": [], + "license": null + }))) + .mount(server) + .await; + } + + #[tokio::test] + async fn the_tree_listing_returns_files_and_not_directories() { + let server = MockServer::start().await; + mount_repository(&server).await; + Mock::given(method("GET")) + .and(path("/repos/acme/widgets/git/trees/main")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "truncated": false, + "tree": [ + { "path": "src", "type": "tree" }, + { "path": "src/main.rs", "type": "blob" }, + { "path": "README.adoc", "type": "blob" }, + { "path": ".machine_readable/descriptiles/STATE.a2ml", "type": "blob" } + ] + }))) + .mount(&server) + .await; + + let client = GitHubClient::new(&config_for(&server)); + let paths = client + .tree_paths("acme", "widgets") + .await + .expect("the listing parses"); + + assert_eq!( + paths, + vec![ + "src/main.rs", + "README.adoc", + ".machine_readable/descriptiles/STATE.a2ml" + ], + "directories are not files: counting `src` would not tell a check anything, and a \ + criterion naming a directory is satisfied by what is inside it" + ); + } + + #[tokio::test] + async fn a_truncated_tree_is_an_error_rather_than_a_short_list() { + let server = MockServer::start().await; + mount_repository(&server).await; + Mock::given(method("GET")) + .and(path("/repos/acme/widgets/git/trees/main")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "truncated": true, + "tree": [{ "path": "README.adoc", "type": "blob" }] + }))) + .mount(&server) + .await; + + let client = GitHubClient::new(&config_for(&server)); + let error = client + .tree_paths("acme", "widgets") + .await + .expect_err("a partial tree must not be reported as a complete one"); + + let message = format!("{error:#}"); + assert!( + message.contains("incomplete") && message.contains("--path"), + "the refusal must say why it matters and what to do instead: {message}" + ); + } + + #[tokio::test] + async fn an_absent_optional_file_is_none_and_a_failure_is_an_error() { + let server = MockServer::start().await; + + Mock::given(method("GET")) + .and(path( + "/repos/acme/widgets/contents/.machine_readable/rsr-profile.a2ml", + )) + .respond_with(ResponseTemplate::new(404).set_body_json(serde_json::json!({ + "message": "Not Found" + }))) + .mount(&server) + .await; + + Mock::given(method("GET")) + .and(path( + "/repos/acme/widgets/contents/.machine_readable/STATE.a2ml", + )) + .respond_with(ResponseTemplate::new(500).set_body_string("boom")) + .mount(&server) + .await; + + let client = GitHubClient::new(&config_for(&server)); + + assert_eq!( + client + .get_file_content_if_present( + "acme", + "widgets", + ".machine_readable/rsr-profile.a2ml" + ) + .await + .expect("a 404 is an answer, not a failure"), + None, + "a repository with no profile declares no capabilities" + ); + + let error = client + .get_file_content_if_present("acme", "widgets", ".machine_readable/STATE.a2ml") + .await + .expect_err("a server error must not read as \"the file is absent\""); + assert!(format!("{error:#}").contains("500"), "{error:#}"); + } + #[tokio::test] async fn a_static_token_is_still_used_for_repository_requests() { let server = MockServer::start().await; diff --git a/bots/rhodibot/src/main.rs b/bots/rhodibot/src/main.rs index 0f5dfe1c..e091d7e2 100644 --- a/bots/rhodibot/src/main.rs +++ b/bots/rhodibot/src/main.rs @@ -20,6 +20,10 @@ use tokio::net::TcpListener; use tower_http::trace::TraceLayer; use tracing::{info, warn}; +use rhodibot::canon::Canon; +use rhodibot::canon::local; +use rhodibot::canon::profile::GateTable; +use rhodibot::canon::report::{CanonReport, FailOn}; use rhodibot::config; use rhodibot::github::GitHubClient; use rhodibot::rsr; @@ -64,6 +68,43 @@ struct Cli { #[derive(Subcommand, Debug)] enum Command { + /// Check a repository against the RSR canon's file-presence criteria. + /// + /// Reads the canon vendored beside the rules (never a hardcoded list), + /// skips the criteria the repository's `.machine_readable/rsr-profile.a2ml` + /// does not declare a capability for, and reports what is present where the + /// canon records it, present elsewhere, present under a location the canon + /// has retired, or absent. + /// + /// **Advisory by default**: exits 0 whatever it finds. The canon names + /// hypatia's `rsr-conformance` the single normative checker, so a second + /// opinion should not claim authority it does not have. Pass `--fail-on` + /// to make it gate CI anyway. + /// + /// Works unauthenticated on public repositories; set `GITHUB_TOKEN` to + /// check private ones or lift the rate limit. + Canon { + /// Repository owner (user or org), e.g. `hyperpolymath` + #[arg(long)] + owner: Option, + + /// Repository name, e.g. `ubicity` + #[arg(long)] + repo: Option, + + /// Check a local checkout instead of a GitHub repository + #[arg(long, conflicts_with_all = ["owner", "repo"])] + path: Option, + + /// Output format + #[arg(long, default_value = "pretty", value_parser = ["pretty", "json"])] + format: String, + + /// Exit non-zero when a finding at or above this severity is present + #[arg(long, default_value = "none", value_parser = ["none", "relocated", "deprecated", "missing"])] + fail_on: String, + }, + /// Run a one-shot RSR compliance check against a remote repository and exit. /// /// Uses the GitHub REST API (honours the `GITHUB_TOKEN` env var for rate @@ -127,6 +168,19 @@ async fn main() -> Result<()> { return run_check(&config, owner, repo, format).await; } + // The canon check: the same rule set the estate is scored against, read + // from the canon itself rather than from a list in this file. + if let Some(Command::Canon { + owner, + repo, + path, + format, + fail_on, + }) = &cli.command + { + return run_canon(&config, owner, repo, path, format, fail_on).await; + } + info!("Starting Rhodibot v{}", env!("CARGO_PKG_VERSION")); // Refuse to start with credentials that cannot work. A half-configured App @@ -162,6 +216,92 @@ async fn main() -> Result<()> { /// One-shot RSR compliance check for CI. Prints a report and exits non-zero /// when required checks fail (so the caller's job fails too). +/// The file paths a repository's profile may live at, in preference order. +const PROFILE_PATHS: [&str; 2] = [ + ".machine_readable/rsr-profile.a2ml", + "machine-readable/rsr-profile.a2ml", +]; + +/// Check one repository against the canon. +/// +/// The rules come from the vendored canon; the applicability comes from the +/// repository's own profile; the classification comes from the verdict module. +/// Nothing about the rule set is decided here. +async fn run_canon( + config: &Config, + owner: &Option, + repo: &Option, + path: &Option, + format: &str, + fail_on: &str, +) -> Result<()> { + let fail_on = FailOn::parse(fail_on)?; + + let (subject, files, profile_source, from_git) = match (owner, repo, path) { + (Some(owner), Some(repo), None) => { + rhodibot::sanitize::validate_owner_repo(owner, repo)?; + let client = GitHubClient::new(config); + let files = client.tree_paths(owner, repo).await?; + + // The profile decides which criteria apply, so a failure to read it + // must not pass as "declares nothing". + let mut profile = None; + for candidate in PROFILE_PATHS { + if let Some(text) = client + .get_file_content_if_present(owner, repo, candidate) + .await? + { + profile = Some(text); + break; + } + } + + (format!("{owner}/{repo}"), files, profile, true) + } + (None, None, Some(path)) => { + let source = local::read(std::path::Path::new(path))?; + let from_git = source.from_git(); + (path.clone(), source.files, source.profile, from_git) + } + _ => anyhow::bail!("give either --owner and --repo, or --path: one repository, one way"), + }; + + let canon = Canon::vendored()?; + let gates = GateTable::vendored()?; + let report = CanonReport::build(&subject, &canon, &gates, &files, profile_source.as_deref())?; + + if format == "json" { + println!("{}", serde_json::to_string_pretty(&report)?); + } else { + if !from_git && path.is_some() { + // A walked list includes untracked files, which can satisfy a + // criterion by accident. Say so rather than let the reader assume + // the tracked set. + println!( + "warning: git was not available, so this is a walk of the working directory -- \ + untracked and build files are included." + ); + } + print!("{}", report.render()); + } + + if fail_on.breached_by(&report) { + let counts = report.findings_by_severity(); + println!( + "\n--fail-on {}: {}", + fail_on.as_str(), + counts + .iter() + .map(|(severity, count)| format!("{count} {severity}")) + .collect::>() + .join(", ") + ); + std::process::exit(1); + } + + Ok(()) +} + async fn run_check(config: &Config, owner: &str, repo: &str, format: &str) -> Result<()> { rhodibot::sanitize::validate_owner_repo(owner, repo)?;