From b3bfa62b9b15552fc600cc29f2ac95842927f35b Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Thu, 12 Nov 2026 22:57:56 +0000 Subject: [PATCH 01/18] fix(finishingbot,glambot): accept estate canon, stop reporting retired or tooling-only findings False positives seen on minted repos (chancel, canonry), each checked against the rule code first. finishingbot - SCM-001 retired: the six .machine_readable/*.scm documents are clauses of _chora.deed. Existing .scm files are still validated (SCM-002..005). - V1-DOC-ROADMAP accepts docs/status/ROADMAP.adoc (or the root). - V1-COM-CONTRIBUTING.md / V1-COM-CODE_OF_CONDUCT.md accept .adoc and .github/. - V1-SEC-001 and REL-004 share one SECURITY policy lookup that also accepts .github/SECURITY.md and .adoc. - TOOL-001 accepts mise.toml (or .mise.toml) in place of .tool-versions. - REL-008 matches key material (PEM headers incl. ENCRYPTED/PGP, AWS key ids, vendor token shapes, long key literals), not the words 'private key' / 'aws secret' such as the label trust.no-private-keys = "probe-deferred". - TEST-RU-003 also accepts tests/fuzz/. - TEST-RU-005/006 are exempt for a crate with no code yet, defined exactly: no .rs file outside target/, vendor/, node_modules/ and hidden dirs has a line that is neither blank nor a comment. - CLM-006 resolves a workflow named in prose (estate-rules.yml, or workflows/estate-rules.yml) under .github/workflows/ first, so a same-named file elsewhere no longer makes it ambiguous. - PH-100/PH-nnn honour the repository's own .hypatia-ignore (the file the RSR template ships) with this bot's module name, finishing_bot/PH:, instead of hard-coding any repository. glambot - MACH-007 retired; MACH-006 now only asks for the .machine_readable/ directory and its auto-fix no longer generates the retired .scm templates. - MACH-001 parses .devcontainer/*.json and .vscode/*.json as JSONC (estate JSON policy carve-out). - GS-001 (git-seo not installed) is a property of the runner, not the repository: logged, not reported. Same change as the report-back-scope PR. - both bots log to stderr so stdout stays the JSON report (same hunk as the report-back-scope PR). Each rule change has a unit test with a positive and a negative case. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/finishingbot/README.adoc | 13 + bots/finishingbot/src/analyzers/mod.rs | 19 ++ .../finishingbot/src/analyzers/placeholder.rs | 95 +++++++ bots/finishingbot/src/analyzers/references.rs | 42 +++ bots/finishingbot/src/analyzers/release.rs | 173 ++++++++---- bots/finishingbot/src/analyzers/scm_files.rs | 58 +++- bots/finishingbot/src/analyzers/testing.rs | 129 ++++++++- bots/finishingbot/src/analyzers/tooling.rs | 65 ++++- .../src/analyzers/v1_readiness.rs | 150 ++++++++-- bots/finishingbot/src/ignore.rs | 131 +++++++++ bots/finishingbot/src/lib.rs | 1 + bots/finishingbot/src/main.rs | 4 + bots/finishingbot/tests/analyzer_tests.rs | 17 +- bots/glambot/README.adoc | 8 +- .../src/analyzers/git_seo_integration.rs | 86 +++--- bots/glambot/src/analyzers/machine.rs | 261 +++++++++++------- bots/glambot/src/main.rs | 3 + bots/glambot/tests/integration.rs | 18 ++ 18 files changed, 1032 insertions(+), 241 deletions(-) create mode 100644 bots/finishingbot/src/ignore.rs diff --git a/bots/finishingbot/README.adoc b/bots/finishingbot/README.adoc index ee74fefd..b59da872 100644 --- a/bots/finishingbot/README.adoc +++ b/bots/finishingbot/README.adoc @@ -171,6 +171,19 @@ finishing-bot release finishing-bot security --panic-attacker-path /path/to/panic-attacker ---- +=== Estate canon and exemptions + +The analyzers accept the estate's canonical locations, not only one spelling: + +* `ROADMAP.adoc` at `docs/status/ROADMAP.adoc` (or the root); `SECURITY`, `CONTRIBUTING` and `CODE_OF_CONDUCT` as `.md` or `.adoc`, at the root or in `.github/` (V1-DOC, V1-SEC-001, V1-COM, REL-004). +* `mise.toml` in place of `.tool-versions` (TOOL-001). +* `tests/fuzz/` as well as `fuzz/` and `.clusterfuzzlite/` (TEST-RU-003); a crate with no code yet (no `.rs` line that is not blank or a comment) is exempt from TEST-RU-005 and TEST-RU-006. +* A workflow named by file name in prose resolves under `.github/workflows/` (CLM-006). +* SCM-001 is retired: STATE, META, ECOSYSTEM, PLAYBOOK, AGENTIC and NEUROSYM are clauses of `_chora.deed`, not files. +* REL-008 matches key material (PEM headers, AWS key ids, vendor token shapes, long key literals), not the words "private key". + +A repository can exempt a path from a rule in its `.hypatia-ignore` (the file the RSR template ships), using this bot's module name: `finishing_bot/PH:build/just/` exempts the placeholder family under `build/just/`; `finishing_bot/*:` exempts every finishing-bot rule. Entries for other modules are not read. Currently only the placeholder analyzer (PH-nnn, hence PH-100) honours it. + === Security Analysis finishing-bot integrates with `panic-attacker` for security validation: diff --git a/bots/finishingbot/src/analyzers/mod.rs b/bots/finishingbot/src/analyzers/mod.rs index bb2a4622..df04008e 100644 --- a/bots/finishingbot/src/analyzers/mod.rs +++ b/bots/finishingbot/src/analyzers/mod.rs @@ -17,6 +17,25 @@ use crate::error::Result; use serde::{Deserialize, Serialize}; use std::path::{Path, PathBuf}; +/// First of `candidates` (repo-relative, in preference order) that exists +/// under `root`. Estate canon allows a document in more than one place +/// (`.adoc` or `.md`, repo root or `.github/`), and a rule that names exactly +/// one spelling reports a false positive for the others. +pub(crate) fn first_existing(root: &Path, candidates: &[&str]) -> Option { + candidates + .iter() + .map(|c| root.join(c)) + .find(|p| p.exists()) +} + +/// Accepted spellings and locations of a security policy. +pub(crate) const SECURITY_POLICY: &[&str] = &[ + "SECURITY.md", + "SECURITY.adoc", + ".github/SECURITY.md", + ".github/SECURITY.adoc", +]; + /// Severity levels for findings #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] diff --git a/bots/finishingbot/src/analyzers/placeholder.rs b/bots/finishingbot/src/analyzers/placeholder.rs index 313659d5..16e85865 100644 --- a/bots/finishingbot/src/analyzers/placeholder.rs +++ b/bots/finishingbot/src/analyzers/placeholder.rs @@ -5,6 +5,7 @@ use super::{AnalysisResult, Analyzer, Finding, Severity}; use crate::config::{Config, PlaceholderAction}; use crate::error::Result; +use crate::ignore::IgnoreFile; use regex::{Regex, RegexSet}; use std::path::Path; use tracing::debug; @@ -39,6 +40,9 @@ impl Analyzer for PlaceholderAnalyzer { // Add custom regex patterns patterns.extend(config.placeholders.custom_patterns.clone()); + // Exemptions from the repository's own .hypatia-ignore (finishing_bot/PH:...). + let ignore = IgnoreFile::load(path); + let pattern_set = RegexSet::new(&patterns)?; let patterns_with_context: Vec = patterns .iter() @@ -75,6 +79,8 @@ impl Analyzer for PlaceholderAnalyzer { &pattern_set, &patterns_with_context, config, + &ignore, + path, &mut result, ); } @@ -140,6 +146,7 @@ impl PlaceholderAnalyzer { } /// Scan file content for placeholders + #[allow(clippy::too_many_arguments)] fn scan_file_content( &self, path: &Path, @@ -147,6 +154,8 @@ impl PlaceholderAnalyzer { pattern_set: &RegexSet, patterns: &[Regex], config: &Config, + ignore: &IgnoreFile, + root: &Path, result: &mut AnalysisResult, ) { let pattern_names = &config.placeholders.patterns; @@ -157,6 +166,12 @@ impl PlaceholderAnalyzer { if let Some(pattern) = patterns.get(match_idx) { if let Some(m) = pattern.find(line) { let pattern_name = pattern_names.get(match_idx).map(|s| s.as_str()).unwrap_or("custom"); + let rule_id = format!("PH-{:03}", match_idx + 1); + let rel = path.strip_prefix(root).unwrap_or(path).to_string_lossy(); + if ignore.suppresses(&rule_id, &rel) { + debug!(file = %rel, rule = %rule_id, "Exempted by .hypatia-ignore"); + continue; + } debug!( file = %path.display(), @@ -318,3 +333,83 @@ impl PlaceholderAnalyzer { Ok(fixed) } } + +#[cfg(test)] +mod ignore_tests { + use super::*; + use tempfile::TempDir; + + fn write(dir: &Path, rel: &str, body: &str) { + let p = dir.join(rel); + std::fs::create_dir_all(p.parent().unwrap()).unwrap(); + std::fs::write(p, body).unwrap(); + } + + /// A template-shaped repo: its own scanners legitimately name the tokens. + fn repo(ignore: Option<&str>) -> TempDir { + let d = TempDir::new().unwrap(); + write(d.path(), "build/just/scan.just", "grep -E 'TODO|FIXME' .\n"); + write(d.path(), "src/main.rs", "// TODO: real placeholder\n"); + if let Some(text) = ignore { + write(d.path(), ".hypatia-ignore", text); + } + d + } + + fn files(d: &TempDir) -> Vec { + PlaceholderAnalyzer + .analyze(d.path(), &Config::default()) + .unwrap() + .findings + .into_iter() + .filter(|f| f.id.starts_with("PH-0")) + .map(|f| { + f.file + .unwrap() + .strip_prefix(d.path()) + .unwrap() + .to_string_lossy() + .into_owned() + }) + .collect() + } + + #[test] + fn without_an_ignore_file_every_placeholder_is_reported() { + let d = repo(None); + let got = files(&d); + assert!(got.contains(&"build/just/scan.just".to_string()), "{got:?}"); + assert!(got.contains(&"src/main.rs".to_string()), "{got:?}"); + } + + #[test] + fn hypatia_ignore_exempts_the_named_path_only() { + let d = repo(Some("finishing_bot/PH:build/just/\n")); + let got = files(&d); + assert!(!got.contains(&"build/just/scan.just".to_string()), "{got:?}"); + // A real placeholder elsewhere still counts. + assert!(got.contains(&"src/main.rs".to_string()), "{got:?}"); + } + + #[test] + fn another_modules_entry_exempts_nothing_here() { + let d = repo(Some("structural_drift/SD022:build/just/\n")); + assert!(files(&d).contains(&"build/just/scan.just".to_string())); + } + + #[test] + fn exempted_scanner_files_no_longer_inflate_ph_100() { + let mut cfg = Config::default(); + cfg.placeholders.max_allowed = 2; // 2 hits in the scanner file, none elsewhere + let d = TempDir::new().unwrap(); + write(d.path(), "build/just/scan.just", "grep -E 'TODO|FIXME' .\n"); + let over = PlaceholderAnalyzer.analyze(d.path(), &cfg).unwrap(); + assert!(!over.findings.iter().any(|f| f.id == "PH-100"), "at the limit"); + cfg.placeholders.max_allowed = 1; + let over = PlaceholderAnalyzer.analyze(d.path(), &cfg).unwrap(); + assert!(over.findings.iter().any(|f| f.id == "PH-100"), "over the limit"); + write(d.path(), ".hypatia-ignore", "finishing_bot/PH:build/just/\n"); + let ok = PlaceholderAnalyzer.analyze(d.path(), &cfg).unwrap(); + assert!(ok.findings.is_empty(), "{:?}", ok.findings); + } +} diff --git a/bots/finishingbot/src/analyzers/references.rs b/bots/finishingbot/src/analyzers/references.rs index 501dd0d4..525b4018 100644 --- a/bots/finishingbot/src/analyzers/references.rs +++ b/bots/finishingbot/src/analyzers/references.rs @@ -23,6 +23,19 @@ fn local_file(root: &Path, candidate: &Path) -> Option { (resolved.starts_with(root) && resolved.is_file()).then_some(resolved) } +/// `Some(file name)` when `path` is a bare `*.yml`/`*.yaml` name or +/// `workflows/`, the two spellings prose uses for a GitHub workflow. +fn workflow_file_name(path: &Path) -> Option<&std::ffi::OsStr> { + let name = path.file_name()?; + let is_yaml = path + .extension() + .is_some_and(|e| e == "yml" || e == "yaml"); + let parts: Vec<_> = path.components().collect(); + let qualified_ok = parts.len() == 1 + || (parts.len() == 2 && parts[0].as_os_str() == "workflows"); + (is_yaml && qualified_ok).then_some(name) +} + /// Prose can name a document-relative path, a repository-relative path, or an /// unanchored module basename. The last form is accepted only when unique. /// This resolves a file's identity; it does not verify its contents or claims. @@ -44,6 +57,15 @@ pub(super) fn resolve_local_reference( if let Some(found) = local_file(&root, &root.join(path)) { return Some(found); } + // A workflow is named by its file name in prose (`estate-rules.yml`, or + // `workflows/estate-rules.yml`) but lives under .github/workflows/. Look + // there first, so the reference resolves even when the same basename also + // exists elsewhere (which would make the basename search ambiguous). + if let Some(name) = workflow_file_name(path) { + if let Some(found) = local_file(&root, &root.join(".github/workflows").join(name)) { + return Some(found); + } + } // A qualified missing path must not be "repaired" by a same-named file // elsewhere. Nor may generated output stand in for a source module. if path.components().count() != 1 { @@ -127,6 +149,26 @@ mod tests { assert!(!s.resolves("docs/note.adoc", "Echo.agda")); } + #[test] + fn workflow_names_resolve_under_dot_github_workflows() { + let s = Scratch::new(); + s.file(".github/workflows/estate-rules.yml"); + assert!(s.resolves("docs/note.adoc", "estate-rules.yml")); + assert!(s.resolves("docs/note.adoc", "workflows/estate-rules.yml")); + assert!(s.resolves("docs/note.adoc", ".github/workflows/estate-rules.yml")); + // Ambiguous basenames elsewhere do not defeat the workflow directory. + s.file("tools/estate-rules.yml"); + assert!(s.resolves("docs/note.adoc", "estate-rules.yml")); + } + + #[test] + fn a_workflow_name_with_no_such_workflow_is_still_unresolved() { + let s = Scratch::new(); + s.file(".github/workflows/other.yml"); + assert!(!s.resolves("docs/note.adoc", "estate-rules.yml")); + assert!(!s.resolves("docs/note.adoc", "workflows/estate-rules.yml")); + } + #[test] fn a_missing_qualified_path_is_not_satisfied_by_another_basename() { let s = Scratch::new(); diff --git a/bots/finishingbot/src/analyzers/release.rs b/bots/finishingbot/src/analyzers/release.rs index a3056d4a..ceecdf0d 100644 --- a/bots/finishingbot/src/analyzers/release.rs +++ b/bots/finishingbot/src/analyzers/release.rs @@ -2,13 +2,12 @@ // SPDX-FileCopyrightText: 2025 Jonathan D.A. Jewell //! Release readiness analyzer -use super::{AnalysisResult, Analyzer, Finding, Severity}; +use super::{first_existing, AnalysisResult, Analyzer, Finding, Severity, SECURITY_POLICY}; use crate::config::Config; use crate::error::Result; use regex::Regex; use sha2::{Digest, Sha256, Sha512}; use std::path::Path; -use tracing::debug; use walkdir::WalkDir; /// Release readiness analyzer @@ -244,32 +243,19 @@ impl ReleaseAnalyzer { /// Check for required security files fn check_security_files(&self, path: &Path, result: &mut AnalysisResult) { - let security_files = [ - ("SECURITY.md", "Security policy", Severity::Warning), - ("SECURITY.adoc", "Security policy", Severity::Warning), - ]; - - let has_security = security_files - .iter() - .any(|(name, _, _)| path.join(name).exists()); - - if !has_security { + // SECURITY.md or .adoc, at the root or in .github/ (GitHub reads both). + if first_existing(path, SECURITY_POLICY).is_none() { result.add( Finding::new( "REL-004", "Missing Security Policy", Severity::Warning, - "No SECURITY.md file found", + "No SECURITY.md found (checked the root and .github/)", ) + .with_file(path.join(SECURITY_POLICY[0])) .with_suggestion("Add a SECURITY.md describing how to report vulnerabilities"), ); } - - // Check for .github/SECURITY.md - let github_security = path.join(".github/SECURITY.md"); - if !github_security.exists() && !has_security { - debug!("No security policy found in .github either"); - } } /// Verify release artifact hashes @@ -386,23 +372,6 @@ impl ReleaseAnalyzer { /// Check for sensitive data that shouldn't be released fn check_sensitive_data(&self, path: &Path, config: &Config, result: &mut AnalysisResult) { - let sensitive_patterns = [ - (r#"(?i)api[_-]?key\s*[:=]\s*['"]?[a-zA-Z0-9]{20,}"#, "API Key"), - (r#"(?i)secret[_-]?key\s*[:=]\s*['"]?[a-zA-Z0-9]{20,}"#, "Secret Key"), - (r#"(?i)password\s*[:=]\s*['"]?[^\s'"]{8,}"#, "Password"), - (r"(?i)private[_-]?key", "Private Key Reference"), - (r"-----BEGIN (RSA |DSA |EC |OPENSSH )?PRIVATE KEY-----", "Private Key"), - (r"(?i)aws[_-]?access[_-]?key", "AWS Access Key"), - (r"(?i)aws[_-]?secret", "AWS Secret"), - (r"ghp_[a-zA-Z0-9]{36}", "GitHub Personal Access Token"), - (r"gho_[a-zA-Z0-9]{36}", "GitHub OAuth Token"), - ]; - - let compiled: Vec<(Regex, &str)> = sensitive_patterns - .iter() - .filter_map(|(p, name)| Regex::new(p).ok().map(|r| (r, *name))) - .collect(); - for entry in WalkDir::new(path) .follow_links(false) .max_depth(5) @@ -438,20 +407,17 @@ impl ReleaseAnalyzer { } if let Ok(content) = std::fs::read_to_string(entry_path) { - for (pattern, name) in &compiled { - if pattern.is_match(&content) { - result.add( - Finding::new( - "REL-008", - "Sensitive Data", - Severity::Error, - &format!("Potential {} found in source file", name), - ) - .with_file(entry_path.to_path_buf()) - .with_suggestion("Remove sensitive data and rotate any exposed credentials"), - ); - break; // One warning per file - } + if let Some(name) = sensitive_kind(&content) { + result.add( + Finding::new( + "REL-008", + "Sensitive Data", + Severity::Error, + &format!("Potential {} found in source file", name), + ) + .with_file(entry_path.to_path_buf()) + .with_suggestion("Remove sensitive data and rotate any exposed credentials"), + ); } } } @@ -494,3 +460,110 @@ impl ReleaseAnalyzer { } } } + +/// Key material patterns for REL-008. These match credentials themselves +/// (PEM headers, assignments of a long literal, vendor token shapes), never +/// the WORDS "private key" or "aws secret": a config label such as +/// `trust.no-private-keys = "probe-deferred"` or a document describing key +/// hygiene is not a leaked key. +static SENSITIVE_PATTERNS: std::sync::LazyLock> = + std::sync::LazyLock::new(|| { + [ + (r#"(?i)api[_-]?key\s*[:=]\s*['"]?[a-zA-Z0-9]{20,}"#, "API Key"), + (r#"(?i)secret[_-]?key\s*[:=]\s*['"]?[a-zA-Z0-9]{20,}"#, "Secret Key"), + (r#"(?i)password\s*[:=]\s*['"]?[^\s'"]{8,}"#, "Password"), + (r"-----BEGIN ([A-Z0-9]+ )*PRIVATE KEY( BLOCK)?-----", "Private Key"), + (r"\b(AKIA|ASIA)[0-9A-Z]{16}\b", "AWS Access Key ID"), + (r"ghp_[a-zA-Z0-9]{36}", "GitHub Personal Access Token"), + (r"gho_[a-zA-Z0-9]{36}", "GitHub OAuth Token"), + ] + .iter() + .map(|(p, name)| { + ( + Regex::new(p).expect("REL-008 sensitive pattern is a valid constant regex"), + *name, + ) + }) + .collect() + }); + +/// The kind of key material `content` contains, if any (first match wins). +fn sensitive_kind(content: &str) -> Option<&'static str> { + SENSITIVE_PATTERNS + .iter() + .find(|(re, _)| re.is_match(content)) + .map(|(_, name)| *name) +} + +#[cfg(test)] +mod sensitive_tests { + use super::*; + + #[test] + fn words_about_keys_are_not_key_material() { + for text in [ + r#"trust.no-private-keys = "probe-deferred""#, + "Never commit a private key or an AWS secret.", + "aws_access_key and aws-secret are documented here", + "private_key_path = \"/etc/ssl/key\"", + ] { + assert_eq!(sensitive_kind(text), None, "{text}"); + } + } + + #[test] + fn pem_headers_are_key_material() { + for kind in ["RSA PRIVATE KEY", "PRIVATE KEY", "EC PRIVATE KEY", "OPENSSH PRIVATE KEY", "ENCRYPTED PRIVATE KEY"] { + let text = format!("{}{}{}\nMIIB\n", "-----BEGIN ", kind, "-----"); + assert_eq!(sensitive_kind(&text), Some("Private Key"), "{kind}"); + } + let pgp = format!("{}{}{}", "-----BEGIN ", "PGP PRIVATE KEY BLOCK", "-----"); + assert_eq!(sensitive_kind(&pgp), Some("Private Key")); + } + + #[test] + fn key_literals_are_key_material() { + let aws = format!("id = {}{}", "AKIA", "IOSFODNN7EXAMPLE"); + assert_eq!(sensitive_kind(&aws), Some("AWS Access Key ID")); + let ghp = format!("t = {}{}", "ghp_", "a".repeat(36)); + assert_eq!(sensitive_kind(&ghp), Some("GitHub Personal Access Token")); + assert_eq!(sensitive_kind("api_key = \"abcdefghij0123456789xyz\""), Some("API Key")); + } + + #[test] + fn rel_008_ignores_a_label_but_reports_a_key_file() { + let dir = tempfile::TempDir::new().unwrap(); + std::fs::write( + dir.path().join("trust.toml"), + "trust.no-private-keys = \"probe-deferred\"\n", + ) + .unwrap(); + let mut result = AnalysisResult::new(); + ReleaseAnalyzer.check_sensitive_data(dir.path(), &Config::default(), &mut result); + assert!(result.findings.is_empty(), "{:?}", result.findings); + + std::fs::write( + dir.path().join("leak.toml"), + format!("k = \"\"\"\n-----BEGIN {} KEY-----\nAAAA\n\"\"\"\n", "PRIVATE"), + ) + .unwrap(); + ReleaseAnalyzer.check_sensitive_data(dir.path(), &Config::default(), &mut result); + assert_eq!(result.findings.len(), 1); + assert_eq!(result.findings[0].id, "REL-008"); + } + + #[test] + fn security_policy_is_found_in_dot_github() { + let dir = tempfile::TempDir::new().unwrap(); + let mut result = AnalysisResult::new(); + ReleaseAnalyzer.check_security_files(dir.path(), &mut result); + assert_eq!(result.findings.len(), 1); + assert_eq!(result.findings[0].id, "REL-004"); + + std::fs::create_dir_all(dir.path().join(".github")).unwrap(); + std::fs::write(dir.path().join(".github/SECURITY.md"), "x").unwrap(); + let mut again = AnalysisResult::new(); + ReleaseAnalyzer.check_security_files(dir.path(), &mut again); + assert!(again.findings.is_empty()); + } +} diff --git a/bots/finishingbot/src/analyzers/scm_files.rs b/bots/finishingbot/src/analyzers/scm_files.rs index a7e89fe2..d882b820 100644 --- a/bots/finishingbot/src/analyzers/scm_files.rs +++ b/bots/finishingbot/src/analyzers/scm_files.rs @@ -38,7 +38,7 @@ impl Analyzer for ScmFilesAnalyzer { let machine_readable_dir = path.join(".machine_readable"); - for (filename, description) in &required_scm_files { + for (filename, _description) in &required_scm_files { let correct_path = machine_readable_dir.join(filename); let root_path = path.join(filename); @@ -69,18 +69,12 @@ impl Analyzer for ScmFilesAnalyzer { )), ); } else { - result.add( - Finding::new( - &format!("SCM-001-{}", filename), - &format!("Missing {}", filename), - Severity::Error, - &format!("Required SCM file {} not found ({}).", filename, description), - ) - .with_suggestion(&format!( - "Create .machine_readable/{} with proper metadata and content. See hyperpolymath SCM file specs.", - filename - )), - ); + // SCM-001 (a required .scm file is missing) is retired. The six + // descriptive documents (STATE, META, ECOSYSTEM, PLAYBOOK, + // AGENTIC, NEUROSYM) are clauses of the repository's + // `_chora.deed`, not files, so absence is the canonical + // state. Files that DO exist are still validated above. + debug!("{} not present (retired: now a clause of the chora deed)", filename); } } @@ -184,3 +178,41 @@ impl ScmFilesAnalyzer { } } } + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + fn ids(dir: &Path) -> Vec { + ScmFilesAnalyzer + .analyze(dir, &Config::default()) + .unwrap() + .findings + .into_iter() + .map(|f| f.id) + .collect() + } + + #[test] + fn absent_scm_files_are_not_a_finding() { + // The six descriptive documents are clauses of _chora.deed now. + let d = TempDir::new().unwrap(); + std::fs::write(d.path().join("x_chora.deed"), "(deed)").unwrap(); + let got = ids(d.path()); + assert!( + got.iter().all(|i| !i.starts_with("SCM-001")), + "SCM-001 is retired, got {:?}", + got + ); + assert!(got.is_empty(), "got {:?}", got); + } + + #[test] + fn a_misplaced_scm_file_is_still_reported() { + // Negative control: the analyzer still runs for files that exist. + let d = TempDir::new().unwrap(); + std::fs::write(d.path().join("STATE.scm"), ";; x\n").unwrap(); + assert!(ids(d.path()).contains(&"SCM-005-STATE.scm".to_string())); + } +} diff --git a/bots/finishingbot/src/analyzers/testing.rs b/bots/finishingbot/src/analyzers/testing.rs index 6b7f56b4..a5bde3a9 100644 --- a/bots/finishingbot/src/analyzers/testing.rs +++ b/bots/finishingbot/src/analyzers/testing.rs @@ -185,15 +185,16 @@ impl TestingAnalyzer { // Check for fuzzing (cargo-fuzz or ClusterFuzzLite) let fuzz_dir = path.join("fuzz"); + let tests_fuzz_dir = path.join("tests/fuzz"); let clusterfuzz_dir = path.join(".clusterfuzzlite"); - if !fuzz_dir.exists() && !clusterfuzz_dir.exists() { + if !fuzz_dir.exists() && !tests_fuzz_dir.exists() && !clusterfuzz_dir.exists() { result.add( Finding::new( "TEST-RU-003", "Missing fuzzing infrastructure", Severity::Error, - "Rust project missing fuzz/ or .clusterfuzzlite/ for fuzzing.", + "Rust project missing fuzz/, tests/fuzz/ or .clusterfuzzlite/ for fuzzing.", ) .with_suggestion("Add ClusterFuzzLite configuration for automated fuzzing"), ); @@ -219,9 +220,13 @@ impl TestingAnalyzer { } } + // A crate with no code yet has nothing to stress or benchmark, so + // TEST-RU-005 and TEST-RU-006 wait until there is some (see `rust_has_code`). + let has_code = rust_has_code(path); + // Check for stress test workflow let stress_workflow = path.join(".github/workflows/stress-test.yml"); - if !stress_workflow.exists() { + if !stress_workflow.exists() && has_code { result.add( Finding::new( "TEST-RU-005", @@ -237,7 +242,7 @@ impl TestingAnalyzer { // Check Cargo.toml for criterion dependency if let Ok(content) = std::fs::read_to_string(path.join("Cargo.toml")) { - if !content.contains("criterion") { + if has_code && !content.contains("criterion") { result.add( Finding::new( "TEST-RU-006", @@ -422,3 +427,119 @@ mod agda_layout_tests { .any(|finding| finding.id == "TEST-001")); } } + +/// Whether the Rust project at `root` has any code yet. +/// +/// "No code" is exact: no `.rs` file outside build output (`target/`), +/// vendored trees, `node_modules/` and hidden directories contains a line that +/// is neither blank nor a comment. An empty `src/lib.rs`, one holding only +/// `//!` docs or an SPDX header, and a crate with no `.rs` files at all, are +/// all "no code"; `fn main() {}` is code. Used to exempt benchmark and stress +/// requirements from a freshly scaffolded crate. +pub(crate) fn rust_has_code(root: &Path) -> bool { + static BLOCK_COMMENT: std::sync::LazyLock = std::sync::LazyLock::new(|| { + regex::Regex::new(r"(?s)/\*.*?\*/").expect("block-comment regex is a valid constant pattern") + }); + walkdir::WalkDir::new(root) + .follow_links(false) + .into_iter() + .filter_entry(|e| { + if e.depth() == 0 || !e.file_type().is_dir() { + return true; + } + let name = e.file_name().to_string_lossy(); + !(name.starts_with('.') || matches!(name.as_ref(), "target" | "vendor" | "node_modules")) + }) + .filter_map(|e| e.ok()) + .filter(|e| e.file_type().is_file() && e.path().extension().is_some_and(|x| x == "rs")) + .any(|e| { + std::fs::read_to_string(e.path()).is_ok_and(|src| { + BLOCK_COMMENT + .replace_all(&src, "") + .lines() + .map(str::trim) + .any(|l| !l.is_empty() && !l.starts_with("//")) + }) + }) +} + +#[cfg(test)] +mod no_code_tests { + use super::*; + use tempfile::TempDir; + + fn write(dir: &Path, rel: &str, body: &str) { + let p = dir.join(rel); + std::fs::create_dir_all(p.parent().unwrap()).unwrap(); + std::fs::write(p, body).unwrap(); + } + + fn rust_repo(lib: Option<&str>) -> TempDir { + let d = TempDir::new().unwrap(); + write(d.path(), "Cargo.toml", "[package]\nname = \"x\"\nversion = \"0.1.0\"\n"); + if let Some(body) = lib { + write(d.path(), "src/lib.rs", body); + } + d + } + + fn ids(dir: &Path) -> Vec { + TestingAnalyzer + .analyze(dir, &Config::default()) + .unwrap() + .findings + .into_iter() + .map(|f| f.id) + .collect() + } + + #[test] + fn no_code_means_no_non_comment_line_in_any_rs_file() { + for body in [ + None, + Some(""), + Some("// SPDX-License-Identifier: MPL-2.0\n\n//! docs only\n"), + Some("/* block\n comment */\n// c\n"), + ] { + let d = rust_repo(body); + assert!(!rust_has_code(d.path()), "{body:?}"); + } + for body in ["fn main() {}\n", "// c\npub fn f() {}\n", "/* c */ pub struct S;\n"] { + let d = rust_repo(Some(body)); + assert!(rust_has_code(d.path()), "{body:?}"); + } + } + + #[test] + fn code_in_build_output_vendor_or_hidden_dirs_does_not_count() { + let d = rust_repo(Some("// nothing\n")); + write(d.path(), "target/debug/build/x.rs", "fn a() {}\n"); + write(d.path(), "vendor/dep/lib.rs", "fn b() {}\n"); + write(d.path(), ".claude/worktrees/w/src/lib.rs", "fn c() {}\n"); + assert!(!rust_has_code(d.path())); + } + + #[test] + fn stress_and_criterion_are_exempt_for_a_crate_with_no_code() { + let d = rust_repo(Some("// SPDX only\n")); + let got = ids(d.path()); + assert!(!got.contains(&"TEST-RU-005".to_string()), "{got:?}"); + assert!(!got.contains(&"TEST-RU-006".to_string()), "{got:?}"); + } + + #[test] + fn stress_and_criterion_still_required_once_there_is_code() { + let d = rust_repo(Some("pub fn f() {}\n")); + let got = ids(d.path()); + assert!(got.contains(&"TEST-RU-005".to_string()), "{got:?}"); + assert!(got.contains(&"TEST-RU-006".to_string()), "{got:?}"); + } + + #[test] + fn fuzz_under_tests_satisfies_ru_003() { + let d = rust_repo(Some("pub fn f() {}\n")); + assert!(ids(d.path()).contains(&"TEST-RU-003".to_string())); + write(d.path(), "tests/fuzz/fuzz_targets/t.rs", "// target\n"); + assert!(!ids(d.path()).contains(&"TEST-RU-003".to_string())); + } +} diff --git a/bots/finishingbot/src/analyzers/tooling.rs b/bots/finishingbot/src/analyzers/tooling.rs index 87b79ea9..93c7c3f2 100644 --- a/bots/finishingbot/src/analyzers/tooling.rs +++ b/bots/finishingbot/src/analyzers/tooling.rs @@ -2,12 +2,15 @@ // SPDX-FileCopyrightText: 2025 Jonathan D.A. Jewell //! Tooling and version management analyzer -use super::{AnalysisResult, Analyzer, Finding, Severity}; +use super::{first_existing, AnalysisResult, Analyzer, Finding, Severity}; use crate::config::Config; use crate::error::Result; use std::path::Path; use tracing::debug; +/// mise manifests that pin tool versions (accepted in place of `.tool-versions`). +const MISE_MANIFESTS: &[&str] = &["mise.toml", ".mise.toml"]; + /// Tooling and version management analyzer pub struct ToolingAnalyzer; @@ -26,17 +29,22 @@ impl Analyzer for ToolingAnalyzer { let mut result = AnalysisResult::new(); let start = std::time::Instant::now(); - // Check for .tool-versions (asdf/mise) + // Check for a tool-version pin: .tool-versions (asdf/mise) or mise.toml + // (mise's own manifest, which the estate uses). Either satisfies TOOL-001. let tool_versions = path.join(".tool-versions"); - if !tool_versions.exists() { + let mise_manifest = first_existing(path, MISE_MANIFESTS); + if !tool_versions.exists() && mise_manifest.is_some() { + debug!("Found mise manifest instead of .tool-versions"); + } else if !tool_versions.exists() { result.add( Finding::new( "TOOL-001", - "Missing .tool-versions", + "Missing tool-version pin", Severity::Error, - ".tool-versions file not found for asdf/mise version management.", + "Neither .tool-versions nor mise.toml found for asdf/mise version management.", ) - .with_suggestion("Create .tool-versions file with required tool versions (rust, just, etc.)"), + .with_file(path.join("mise.toml")) + .with_suggestion("Create mise.toml (or .tool-versions) with required tool versions (rust, just, etc.)"), ); } else { debug!("Found .tool-versions"); @@ -170,3 +178,48 @@ impl ToolingAnalyzer { } } } + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + fn ids(dir: &Path) -> Vec { + ToolingAnalyzer + .analyze(dir, &Config::default()) + .unwrap() + .findings + .into_iter() + .map(|f| f.id) + .collect() + } + + #[test] + fn tool_001_fires_when_neither_pin_exists_and_names_mise_toml() { + let d = TempDir::new().unwrap(); + let f = ToolingAnalyzer + .analyze(d.path(), &Config::default()) + .unwrap() + .findings + .into_iter() + .find(|f| f.id == "TOOL-001") + .expect("TOOL-001 on a repo with no pin"); + assert_eq!(f.file.as_deref(), Some(d.path().join("mise.toml").as_path())); + } + + #[test] + fn mise_toml_satisfies_tool_001() { + for name in ["mise.toml", ".mise.toml"] { + let d = TempDir::new().unwrap(); + std::fs::write(d.path().join(name), "[tools]\nrust = \"1.90\"\n").unwrap(); + assert!(!ids(d.path()).contains(&"TOOL-001".to_string()), "{name}"); + } + } + + #[test] + fn tool_versions_still_satisfies_tool_001() { + let d = TempDir::new().unwrap(); + std::fs::write(d.path().join(".tool-versions"), "rust 1.90.0\n").unwrap(); + assert!(!ids(d.path()).contains(&"TOOL-001".to_string())); + } +} diff --git a/bots/finishingbot/src/analyzers/v1_readiness.rs b/bots/finishingbot/src/analyzers/v1_readiness.rs index 7bfecd32..ac237b60 100644 --- a/bots/finishingbot/src/analyzers/v1_readiness.rs +++ b/bots/finishingbot/src/analyzers/v1_readiness.rs @@ -2,7 +2,7 @@ // SPDX-FileCopyrightText: 2025 Jonathan D.A. Jewell //! V1 release readiness analyzer -use super::{AnalysisResult, Analyzer, Finding, Severity}; +use super::{first_existing, AnalysisResult, Analyzer, Finding, Severity, SECURITY_POLICY}; use crate::config::Config; use crate::error::Result; use std::path::Path; @@ -53,15 +53,22 @@ impl Analyzer for V1ReadinessAnalyzer { impl V1ReadinessAnalyzer { /// Check for core documentation files fn check_core_docs(&self, path: &Path, result: &mut AnalysisResult) { - let required_docs = [ - ("README.adoc", "Main project documentation"), - ("ROADMAP.adoc", "Project roadmap"), - ("LICENSE", "License file"), + // (name, description, accepted locations in preference order). The + // first location is the canonical one and is the path reported when + // the document is missing. Estate canon keeps the roadmap at + // docs/status/ROADMAP.adoc (the STATE journal). + let required_docs: [(&str, &str, &[&str]); 3] = [ + ("README.adoc", "Main project documentation", &["README.adoc"]), + ( + "ROADMAP.adoc", + "Project roadmap", + &["docs/status/ROADMAP.adoc", "ROADMAP.adoc"], + ), + ("LICENSE", "License file", &["LICENSE"]), ]; - for (filename, description) in &required_docs { - let file_path = path.join(filename); - if !file_path.exists() { + for (filename, description, locations) in &required_docs { + if first_existing(path, locations).is_none() { result.add( Finding::new( &format!("V1-DOC-{}", filename), @@ -69,7 +76,8 @@ impl V1ReadinessAnalyzer { Severity::Error, &format!("Required file {} not found ({}).", filename, description), ) - .with_suggestion(&format!("Create {} file", filename)), + .with_file(path.join(locations[0])) + .with_suggestion(&format!("Create {} file", locations[0])), ); } else { debug!("Found {}", filename); @@ -79,19 +87,19 @@ impl V1ReadinessAnalyzer { /// Check for security files fn check_security_files(&self, path: &Path, result: &mut AnalysisResult) { - let security_file = path.join("SECURITY.md"); - if !security_file.exists() { + if first_existing(path, SECURITY_POLICY).is_none() { result.add( Finding::new( "V1-SEC-001", "Missing SECURITY.md", Severity::Warning, - "SECURITY.md not found for responsible disclosure.", + "No security policy (SECURITY.md or .github/SECURITY.md) found for responsible disclosure.", ) + .with_file(path.join(SECURITY_POLICY[0])) .with_suggestion("Add SECURITY.md with security policy and contact information"), ); } else { - debug!("Found SECURITY.md"); + debug!("Found security policy"); } } @@ -158,14 +166,33 @@ impl V1ReadinessAnalyzer { /// Check for community files fn check_community_files(&self, path: &Path, result: &mut AnalysisResult) { - let community_files = [ - ("CODE_OF_CONDUCT.md", "Code of conduct"), - ("CONTRIBUTING.md", "Contribution guidelines"), + // The rule id keeps the historical .md spelling; the estate writes + // these as .adoc, and GitHub reads them from the root or .github/. + let community_files: [(&str, &str, &[&str]); 2] = [ + ( + "CODE_OF_CONDUCT.md", + "Code of conduct", + &[ + "CODE_OF_CONDUCT.md", + "CODE_OF_CONDUCT.adoc", + ".github/CODE_OF_CONDUCT.md", + ".github/CODE_OF_CONDUCT.adoc", + ], + ), + ( + "CONTRIBUTING.md", + "Contribution guidelines", + &[ + "CONTRIBUTING.md", + "CONTRIBUTING.adoc", + ".github/CONTRIBUTING.md", + ".github/CONTRIBUTING.adoc", + ], + ), ]; - for (filename, description) in &community_files { - let file_path = path.join(filename); - if !file_path.exists() { + for (filename, description, locations) in &community_files { + if first_existing(path, locations).is_none() { result.add( Finding::new( &format!("V1-COM-{}", filename), @@ -173,6 +200,7 @@ impl V1ReadinessAnalyzer { Severity::Warning, &format!("{} file not found ({}).", filename, description), ) + .with_file(path.join(locations[0])) .with_suggestion(&format!("Add {} for community health", filename)), ); } else { @@ -181,3 +209,87 @@ impl V1ReadinessAnalyzer { } } } + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + fn run(dir: &Path) -> Vec { + V1ReadinessAnalyzer + .analyze(dir, &Config::default()) + .unwrap() + .findings + } + + fn ids(dir: &Path) -> Vec { + run(dir).into_iter().map(|f| f.id).collect() + } + + fn touch(dir: &Path, rel: &str) { + let p = dir.join(rel); + std::fs::create_dir_all(p.parent().unwrap()).unwrap(); + std::fs::write(p, "x").unwrap(); + } + + #[test] + fn roadmap_is_missing_when_absent_and_names_the_canonical_path() { + let d = TempDir::new().unwrap(); + let f = run(d.path()) + .into_iter() + .find(|f| f.id == "V1-DOC-ROADMAP.adoc") + .expect("missing roadmap is reported"); + assert_eq!( + f.file.as_deref(), + Some(d.path().join("docs/status/ROADMAP.adoc").as_path()) + ); + } + + #[test] + fn roadmap_accepts_docs_status_and_root() { + for rel in ["docs/status/ROADMAP.adoc", "ROADMAP.adoc"] { + let d = TempDir::new().unwrap(); + touch(d.path(), rel); + assert!(!ids(d.path()).contains(&"V1-DOC-ROADMAP.adoc".to_string()), "{rel}"); + } + } + + #[test] + fn community_files_accept_adoc_and_dot_github() { + let none = TempDir::new().unwrap(); + let got = ids(none.path()); + assert!(got.contains(&"V1-COM-CONTRIBUTING.md".to_string())); + assert!(got.contains(&"V1-COM-CODE_OF_CONDUCT.md".to_string())); + for rel in [ + "CONTRIBUTING.md", + "CONTRIBUTING.adoc", + ".github/CONTRIBUTING.md", + ".github/CONTRIBUTING.adoc", + ] { + let d = TempDir::new().unwrap(); + touch(d.path(), rel); + assert!(!ids(d.path()).contains(&"V1-COM-CONTRIBUTING.md".to_string()), "{rel}"); + } + for rel in ["CODE_OF_CONDUCT.adoc", ".github/CODE_OF_CONDUCT.md"] { + let d = TempDir::new().unwrap(); + touch(d.path(), rel); + assert!( + !ids(d.path()).contains(&"V1-COM-CODE_OF_CONDUCT.md".to_string()), + "{rel}" + ); + } + // Accepting one document does not excuse the other. + let d = TempDir::new().unwrap(); + touch(d.path(), "CONTRIBUTING.adoc"); + assert!(ids(d.path()).contains(&"V1-COM-CODE_OF_CONDUCT.md".to_string())); + } + + #[test] + fn security_policy_accepts_dot_github() { + let none = TempDir::new().unwrap(); + assert!(ids(none.path()).contains(&"V1-SEC-001".to_string())); + let d = TempDir::new().unwrap(); + touch(d.path(), ".github/SECURITY.md"); + assert!(!ids(d.path()).contains(&"V1-SEC-001".to_string())); + } +} diff --git a/bots/finishingbot/src/ignore.rs b/bots/finishingbot/src/ignore.rs new file mode 100644 index 00000000..eae56211 --- /dev/null +++ b/bots/finishingbot/src/ignore.rs @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: 2026 Jonathan D.A. Jewell +//! Scoped exemptions from the repository's `.hypatia-ignore`. +//! +//! The estate already has one documented, reviewed place for "this path is +//! exempt from that rule, and here is why": `.hypatia-ignore`, which the RSR +//! template ships and scaffolded repositories inherit. finishing-bot honours +//! the same file with the same grammar rather than growing a second ignore +//! mechanism, and rather than special-casing any repository by name. +//! +//! Grammar (one entry per line; `#` starts a comment): +//! +//! ```text +//! /: rule-scoped, e.g. finishing_bot/PH:build/just/ +//! /*: any rule of that module +//! any rule, any module +//! ``` +//! +//! A path fragment is a substring of the repo-relative path. This bot's module +//! is `finishing_bot`; entries for other modules (`structural_drift/...`) are +//! not ours and are ignored. A `` is an exact finding id (`PH-002`) or a +//! family prefix (`PH` matches `PH-001`, `PH-002`, ...). + +use std::path::Path; + +/// The module name under which finishing-bot's rules are addressed. +pub const MODULE: &str = "finishing_bot"; + +#[derive(Debug, Default, Clone)] +pub struct IgnoreFile { + entries: Vec, +} + +#[derive(Debug, Clone)] +struct Entry { + /// `None` = any rule. + rule: Option, + fragment: String, +} + +impl IgnoreFile { + /// Load `/.hypatia-ignore`; an absent or unreadable file exempts nothing. + pub fn load(root: &Path) -> Self { + std::fs::read_to_string(root.join(".hypatia-ignore")) + .map(|t| Self::parse(&t)) + .unwrap_or_default() + } + + pub fn parse(text: &str) -> Self { + let mut entries = Vec::new(); + for line in text.lines() { + let line = line.trim(); + if line.is_empty() || line.starts_with('#') { + continue; + } + let (scope, fragment) = match line.split_once(':') { + Some((scope, fragment)) if scope.contains('/') && !scope.contains(char::is_whitespace) => { + (Some(scope), fragment) + } + _ => (None, line), + }; + let fragment = fragment.trim(); + if fragment.is_empty() { + continue; + } + let rule = match scope { + None => None, + Some(scope) => { + let (module, rule) = scope.split_once('/').unwrap_or((scope, "*")); + if module != MODULE { + continue; // another module's entry + } + if rule == "*" { None } else { Some(rule.to_string()) } + } + }; + entries.push(Entry { rule, fragment: fragment.to_string() }); + } + Self { entries } + } + + /// Does an entry exempt finding `rule_id` at repo-relative path `rel`? + pub fn suppresses(&self, rule_id: &str, rel: &str) -> bool { + let rel = rel.strip_prefix("./").unwrap_or(rel); + self.entries.iter().any(|e| { + rel.contains(&e.fragment) + && e.rule.as_deref().is_none_or(|r| { + rule_id == r || rule_id.strip_prefix(r).is_some_and(|rest| rest.starts_with('-')) + }) + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const FILE: &str = "\ +# comment +structural_drift/SD022:build/docs-seed/ +finishing_bot/PH:build/just/ +finishing_bot/PH-002:Justfile +finishing_bot/*:scripts/check- +generated/ +"; + + #[test] + fn rule_family_and_exact_id_and_wildcard_and_bare_entries() { + let f = IgnoreFile::parse(FILE); + assert!(f.suppresses("PH-001", "build/just/a.just")); // family PH + assert!(f.suppresses("PH-003", "./build/just/a.just")); // ./ tolerated + assert!(f.suppresses("PH-002", "Justfile")); // exact id + assert!(f.suppresses("REL-003", "scripts/check-x.sh")); // module wildcard + assert!(f.suppresses("CLM-006", "generated/out.md")); // bare: any rule + } + + #[test] + fn non_matching_rule_path_or_module_is_not_suppressed() { + let f = IgnoreFile::parse(FILE); + assert!(!f.suppresses("PH-001", "src/main.rs")); // other path + assert!(!f.suppresses("PH-003", "Justfile")); // exact id is PH-002 only + assert!(!f.suppresses("REL-003", "build/just/a.just")); // family is PH only + assert!(!f.suppresses("SD022", "build/docs-seed/x")); // another module's entry + assert!(!f.suppresses("PHX-001", "build/just/a.just")); // prefix needs '-' boundary + } + + #[test] + fn absent_file_exempts_nothing() { + let d = tempfile::TempDir::new().unwrap(); + assert!(!IgnoreFile::load(d.path()).suppresses("PH-001", "build/just/a.just")); + } +} diff --git a/bots/finishingbot/src/lib.rs b/bots/finishingbot/src/lib.rs index 5900c002..12b717b6 100644 --- a/bots/finishingbot/src/lib.rs +++ b/bots/finishingbot/src/lib.rs @@ -16,6 +16,7 @@ pub mod analyzers; pub mod config; pub mod error; +pub mod ignore; pub use analyzers::{ claims::ClaimsAnalyzer, license::LicenseAnalyzer, placeholder::PlaceholderAnalyzer, diff --git a/bots/finishingbot/src/main.rs b/bots/finishingbot/src/main.rs index 111ab1cf..012d63e3 100644 --- a/bots/finishingbot/src/main.rs +++ b/bots/finishingbot/src/main.rs @@ -5,6 +5,7 @@ mod analyzers; mod config; mod error; +mod ignore; #[cfg(feature = "fleet")] mod fleet; @@ -262,7 +263,10 @@ fn handle_fleet_mode( fn init_logging(level: &str) { let filter = EnvFilter::try_new(level).unwrap_or_else(|_| EnvFilter::new("info")); + // Logs go to stderr: stdout carries the JSON report that the fleet driver + // parses, and a log line there makes the report unreadable. let layer = fmt::layer() + .with_writer(std::io::stderr) .with_target(true) .with_span_events(FmtSpan::CLOSE) .with_ansi(true); diff --git a/bots/finishingbot/tests/analyzer_tests.rs b/bots/finishingbot/tests/analyzer_tests.rs index 995274f7..a17fbe77 100644 --- a/bots/finishingbot/tests/analyzer_tests.rs +++ b/bots/finishingbot/tests/analyzer_tests.rs @@ -629,24 +629,19 @@ mod scm_files_analyzer { use super::*; #[test] - fn test_missing_state_scm_error() { + fn test_missing_state_scm_is_retired() { let (_temp, path) = setup_test_repo(); - // No .machine_readable/ directory at all + // No .machine_readable/ directory at all: the descriptive documents are + // clauses of the chora deed now, so absence is not a finding. let analyzer = ScmFilesAnalyzer; let config = default_config(); let result = analyzer.analyze(&path, &config).unwrap(); assert!( - result.findings.iter().any(|f| f.id == "SCM-001-STATE.scm"), - "Should report missing STATE.scm" - ); - assert!( - result - .findings - .iter() - .any(|f| f.id == "SCM-001-STATE.scm" && f.severity == Severity::Error), - "Missing STATE.scm should be Error severity" + result.findings.iter().all(|f| !f.id.starts_with("SCM-001")), + "SCM-001 is retired; got {:?}", + result.findings.iter().map(|f| &f.id).collect::>() ); } diff --git a/bots/glambot/README.adoc b/bots/glambot/README.adoc index 553f8af1..1f4389c1 100644 --- a/bots/glambot/README.adoc +++ b/bots/glambot/README.adoc @@ -96,16 +96,16 @@ Validates JSON/YAML file syntax, structured data presence, robots.txt, and `.mac |=== | Finding ID | Description | Severity -| MACH-001 | Invalid JSON | Error +| MACH-001 | Invalid JSON | Error (`.devcontainer/*.json` and `.vscode/*.json` are tool-demanded JSONC and are parsed as JSONC) | MACH-002 | Invalid YAML | Error | MACH-003 | Missing structured data (HTML) | Warning | MACH-004 | Invalid JSON-LD | Error | MACH-005 | Missing robots.txt | Info | MACH-006 | Missing .machine_readable/ directory | Warning -| MACH-007 | Missing SCM file (STATE/META/ECOSYSTEM) | Info +| MACH-007 | _Retired_: the STATE/META/ECOSYSTEM `.scm` files are clauses of the repository's `_chora.deed`, so their absence is not reported | n/a |=== -**Auto-fixes:** Creates `.machine_readable/` directory with STATE.scm, META.scm, ECOSYSTEM.scm templates (MACH-006), adds schema.org JSON-LD template (MACH-003), creates robots.txt (MACH-005). +**Auto-fixes:** Creates the `.machine_readable/` directory (MACH-006), adds schema.org JSON-LD template (MACH-003), creates robots.txt (MACH-005). === Git-SEO Integration (`GitSeoAnalyzer`) @@ -115,7 +115,7 @@ Shells out to the `git-seo` CLI tool for comprehensive forge-based SEO scoring ( |=== | Finding ID | Description | Severity -| GS-001 | git-seo not installed | Info +| GS-001 | _Retired as a finding_: git-seo not being installed is a property of the machine running glambot, not of the repository, so it is logged (`warn`) and the analyzer reports nothing | n/a | GS-002 | git-seo analysis failed | Warning | GS-003 | No git remote found | Info | GS-100 | Overall SEO score | Varies diff --git a/bots/glambot/src/analyzers/git_seo_integration.rs b/bots/glambot/src/analyzers/git_seo_integration.rs index dfbb1810..ed66117c 100644 --- a/bots/glambot/src/analyzers/git_seo_integration.rs +++ b/bots/glambot/src/analyzers/git_seo_integration.rs @@ -20,26 +20,24 @@ impl Default for GitSeoAnalyzer { } } -impl Analyzer for GitSeoAnalyzer { - fn name(&self) -> &str { - "Git-SEO" - } - - fn analyze(&self, path: &Path, config: &Config) -> Result { +impl GitSeoAnalyzer { + /// Analyze with an explicit git-seo availability (testable seam). + fn analyze_with( + &self, + path: &Path, + config: &Config, + git_seo_installed: bool, + ) -> Result { let mut result = AnalysisResult::new(); let start = std::time::Instant::now(); - // Check if git-seo is installed - if !is_git_seo_installed() { - result.add( - Finding::new( - "GS-001", - "git-seo not installed", - Severity::Info, - "Install git-seo for comprehensive SEO analysis: https://github.com/hyperpolymath/git-seo", - ) - .with_suggestion("Install: git clone https://github.com/hyperpolymath/git-seo && cd git-seo && julia --project=. -e 'using Pkg; Pkg.instantiate()'"), - ); + // git-seo missing is a property of the machine running glambot, not of + // the repository under review, so it must never become a finding: a + // finding is graded as the change (an info finding turns the lane + // "minor" on every pull request). The analyzer did not run, which is + // logged loudly instead; install hint: https://github.com/hyperpolymath/git-seo + if !git_seo_installed { + warn!("git-seo is not installed; the Git-SEO analyzer did not run (no findings reported, not a clean result)"); result.duration_ms = start.elapsed().as_millis() as u64; return Ok(result); } @@ -75,6 +73,7 @@ impl Analyzer for GitSeoAnalyzer { Severity::Info, "Repository has no git remote. git-seo requires a forge URL (GitHub/GitLab/Bitbucket).", ) + .with_file(path.join(".git/config")) .with_suggestion("Add remote: git remote add origin "), ); } @@ -82,6 +81,16 @@ impl Analyzer for GitSeoAnalyzer { result.duration_ms = start.elapsed().as_millis() as u64; Ok(result) } +} + +impl Analyzer for GitSeoAnalyzer { + fn name(&self) -> &str { + "Git-SEO" + } + + fn analyze(&self, path: &Path, config: &Config) -> Result { + self.analyze_with(path, config, is_git_seo_installed()) + } fn fix(&self, path: &Path, config: &Config, findings: &[Finding]) -> Result> { let mut fixes = Vec::new(); @@ -321,30 +330,41 @@ mod tests { Config::default() } - #[test] - fn test_repo_without_git_remote() { + fn git_repo_without_remote() -> TempDir { let dir = TempDir::new().unwrap(); - // Initialize a git repo without a remote std::process::Command::new("git") .arg("init") .arg(dir.path()) .output() .unwrap(); + dir + } - let analyzer = GitSeoAnalyzer::default(); - let config = default_config(); - let result = analyzer.analyze(dir.path(), &config).unwrap(); - - // Should find no remote (GS-003) or git-seo not installed (GS-001) - let ids: Vec<&str> = result - .findings - .iter() - .map(|f| f.id.as_str()) - .collect(); + #[test] + fn test_repo_without_git_remote() { + let dir = git_repo_without_remote(); + let result = GitSeoAnalyzer::default() + .analyze_with(dir.path(), &default_config(), true) + .unwrap(); + let gs003: Vec<_> = result.findings.iter().filter(|f| f.id == "GS-003").collect(); + assert_eq!(gs003.len(), 1, "a repo with no remote reports GS-003"); + assert_eq!( + gs003[0].file.as_deref(), + Some(dir.path().join(".git/config").as_path()), + "GS-003 names the file that would fix it so the grid can scope it" + ); + } + + #[test] + fn missing_git_seo_is_not_a_finding() { + let dir = git_repo_without_remote(); + let result = GitSeoAnalyzer::default() + .analyze_with(dir.path(), &default_config(), false) + .unwrap(); assert!( - ids.contains(&"GS-001") || ids.contains(&"GS-003"), - "Should report GS-001 (no git-seo) or GS-003 (no remote), got: {:?}", - ids + result.findings.is_empty(), + "tool-availability advice is not a repo finding, got: {:?}", + result.findings.iter().map(|f| f.id.as_str()).collect::>() ); } diff --git a/bots/glambot/src/analyzers/machine.rs b/bots/glambot/src/analyzers/machine.rs index ba066a10..b2d59ad5 100644 --- a/bots/glambot/src/analyzers/machine.rs +++ b/bots/glambot/src/analyzers/machine.rs @@ -59,6 +59,8 @@ impl Analyzer for MachineAnalyzer { match finding.id.as_str() { "MACH-006" => { // Create missing .machine_readable/ directory structure + // Directory only: the STATE/META/ECOSYSTEM .scm templates this + // used to generate are retired (clauses of the chora deed now). let mr_dir = path.join(".machine_readable"); if !mr_dir.exists() { std::fs::create_dir_all(&mr_dir)?; @@ -67,76 +69,6 @@ impl Analyzer for MachineAnalyzer { mr_dir.display() )); } - - // Generate template SCM files - let repo_name = path - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("project"); - - let state_path = mr_dir.join("STATE.scm"); - if !state_path.exists() { - let state_content = format!( - ";; SPDX-License-Identifier: MPL-2.0\n\ - ;; STATE.scm - Project state for {name}\n\n\ - (state\n\ - \x20 (metadata\n\ - \x20 (version \"0.1.0\")\n\ - \x20 (schema-version \"1.0\")\n\ - \x20 (created \"TODO\")\n\ - \x20 (updated \"TODO\")\n\ - \x20 (project \"{name}\")\n\ - \x20 (repo \"hyperpolymath/{name}\"))\n\n\ - \x20 (project-context\n\ - \x20 (name \"{name}\")\n\ - \x20 (tagline \"TODO: Add tagline\")\n\ - \x20 (tech-stack (\"TODO\"))))\n", - name = repo_name - ); - std::fs::write(&state_path, state_content)?; - fixes.push(format!( - "Generated STATE.scm template at {}", - state_path.display() - )); - } - - let meta_path = mr_dir.join("META.scm"); - if !meta_path.exists() { - let meta_content = format!( - ";; SPDX-License-Identifier: MPL-2.0\n\ - ;; META.scm - Meta information for {name}\n\n\ - (meta\n\ - \x20 (architecture-decisions ())\n\ - \x20 (development-practices ())\n\ - \x20 (design-rationale ()))\n", - name = repo_name - ); - std::fs::write(&meta_path, meta_content)?; - fixes.push(format!( - "Generated META.scm template at {}", - meta_path.display() - )); - } - - let ecosystem_path = mr_dir.join("ECOSYSTEM.scm"); - if !ecosystem_path.exists() { - let eco_content = format!( - ";; SPDX-License-Identifier: MPL-2.0\n\ - ;; ECOSYSTEM.scm - Ecosystem context for {name}\n\n\ - (ecosystem\n\ - \x20 (version \"1.0\")\n\ - \x20 (name \"{name}\")\n\ - \x20 (type \"application\")\n\ - \x20 (purpose \"TODO\")\n\ - \x20 (related-projects ()))\n", - name = repo_name - ); - std::fs::write(&ecosystem_path, eco_content)?; - fixes.push(format!( - "Generated ECOSYSTEM.scm template at {}", - ecosystem_path.display() - )); - } } "MACH-003" => { // Add structured data (schema.org JSON-LD) template @@ -202,7 +134,15 @@ impl MachineAnalyzer { result.files_checked += 1; if let Ok(content) = std::fs::read_to_string(entry.path()) { - if let Err(e) = serde_json::from_str::(&content) { + // .devcontainer/*.json and .vscode/*.json are JSONC (comments and + // trailing commas) because the tools that read them demand it; the + // estate JSON policy carves them out. Parse them as JSONC. + let parsed = if is_tool_demanded_jsonc(entry.path()) { + serde_json::from_str::(&strip_jsonc(&content)) + } else { + serde_json::from_str::(&content) + }; + if let Err(e) = parsed { result.add( Finding::new( "MACH-001", @@ -341,35 +281,101 @@ impl MachineAnalyzer { "MACH-006", "Missing .machine_readable/ directory", Severity::Warning, - "No .machine_readable/ directory found. SCM files (STATE.scm, META.scm, ECOSYSTEM.scm) should be placed here.", + "No .machine_readable/ directory found (contractiles and other machine-readable estate metadata live here).", ) .fixable() - .with_suggestion("Create .machine_readable/ directory with STATE.scm, META.scm, ECOSYSTEM.scm"), + .with_file(mr_dir.clone()) + .with_suggestion("Create the .machine_readable/ directory"), ); return; } result.files_checked += 1; - // Check for required SCM files - let required_scm = ["STATE.scm", "META.scm", "ECOSYSTEM.scm"]; - for scm_file in &required_scm { - // Check both direct path and 6scm subdirectory - let direct_path = mr_dir.join(scm_file); - let sub_path = mr_dir.join("6scm").join(scm_file); - if !direct_path.exists() && !sub_path.exists() { - result.add( - Finding::new( - "MACH-007", - &format!("Missing {}", scm_file), - Severity::Info, - &format!("{} not found in .machine_readable/ directory.", scm_file), - ) - .with_suggestion(&format!("Add {} to .machine_readable/", scm_file)), - ); + // MACH-007 (missing STATE/META/ECOSYSTEM .scm) is retired: those + // documents are clauses of the repository's `_chora.deed` now, + // not files under .machine_readable/, so their absence is canonical. + } +} + +/// True for JSON files whose consumer demands JSONC: anything under a +/// `.devcontainer/` or `.vscode/` directory. +fn is_tool_demanded_jsonc(path: &Path) -> bool { + path.parent().is_some_and(|dir| { + dir.components() + .any(|c| c.as_os_str() == ".devcontainer" || c.as_os_str() == ".vscode") + }) +} + +/// Reduce JSONC to JSON: drop a BOM, `//` and `/* */` comments and trailing +/// commas, leaving string contents untouched. +fn strip_jsonc(src: &str) -> String { + let src = src.strip_prefix('\u{feff}').unwrap_or(src); + let chars: Vec = src.chars().collect(); + let mut out = String::with_capacity(src.len()); + let mut i = 0; + while i < chars.len() { + let c = chars[i]; + if c == '"' { + out.push(c); + i += 1; + while i < chars.len() { + out.push(chars[i]); + if chars[i] == '\\' && i + 1 < chars.len() { + out.push(chars[i + 1]); + i += 2; + continue; + } + if chars[i] == '"' { + i += 1; + break; + } + i += 1; + } + } else if c == '/' && chars.get(i + 1) == Some(&'/') { + while i < chars.len() && chars[i] != '\n' { + i += 1; } + } else if c == '/' && chars.get(i + 1) == Some(&'*') { + i += 2; + while i + 1 < chars.len() && !(chars[i] == '*' && chars[i + 1] == '/') { + i += 1; + } + i = (i + 2).min(chars.len()); + } else { + out.push(c); + i += 1; + } + } + // Trailing commas: a comma whose next non-space char closes a container. + let b: Vec = out.chars().collect(); + let mut res = String::with_capacity(b.len()); + let mut in_str = false; + let mut j = 0; + while j < b.len() { + let c = b[j]; + if in_str { + res.push(c); + if c == '\\' && j + 1 < b.len() { + res.push(b[j + 1]); + j += 1; + } else if c == '"' { + in_str = false; + } + } else if c == '"' { + in_str = true; + res.push(c); + } else if c == ',' { + let next = b[j + 1..].iter().find(|x| !x.is_whitespace()); + if !matches!(next, Some('}') | Some(']')) { + res.push(c); + } + } else { + res.push(c); } + j += 1; } + res } #[cfg(test)] @@ -482,17 +488,70 @@ mod tests { dir.path().join(".machine_readable").exists(), ".machine_readable/ should exist after fix" ); - assert!( - dir.path().join(".machine_readable").join("STATE.scm").exists(), - "STATE.scm should exist after fix" - ); - assert!( - dir.path().join(".machine_readable").join("META.scm").exists(), - "META.scm should exist after fix" - ); - assert!( - dir.path().join(".machine_readable").join("ECOSYSTEM.scm").exists(), - "ECOSYSTEM.scm should exist after fix" - ); + // The retired .scm descriptive files are not regenerated. + for scm in ["STATE.scm", "META.scm", "ECOSYSTEM.scm"] { + assert!( + !dir.path().join(".machine_readable").join(scm).exists(), + "{scm} is retired and must not be generated" + ); + } + } + + fn ids(dir: &Path) -> Vec { + MachineAnalyzer::default() + .analyze(dir, &default_config()) + .unwrap() + .findings + .into_iter() + .map(|f| f.id) + .collect() + } + + #[test] + fn mach_007_is_retired() { + let dir = TempDir::new().unwrap(); + std::fs::create_dir_all(dir.path().join(".machine_readable")).unwrap(); + let got = ids(dir.path()); + assert!(!got.contains(&"MACH-007".to_string()), "{got:?}"); + assert!(!got.contains(&"MACH-006".to_string()), "{got:?}"); + // The directory itself is still expected. + let empty = TempDir::new().unwrap(); + assert!(ids(empty.path()).contains(&"MACH-006".to_string())); + } + + const JSONC: &str = "{\n // a comment\n \"url\": \"http://x//y\", /* inline */\n \"list\": [1, 2,],\n}\n"; + + #[test] + fn devcontainer_and_vscode_json_are_parsed_as_jsonc() { + for rel in [".devcontainer/devcontainer.json", ".vscode/settings.json", ".devcontainer/sub/devcontainer.json"] { + let dir = TempDir::new().unwrap(); + let p = dir.path().join(rel); + std::fs::create_dir_all(p.parent().unwrap()).unwrap(); + std::fs::write(&p, JSONC).unwrap(); + assert!(!ids(dir.path()).contains(&"MACH-001".to_string()), "{rel}"); + } + } + + #[test] + fn the_same_text_elsewhere_is_still_invalid_json() { + let dir = TempDir::new().unwrap(); + std::fs::write(dir.path().join("config.json"), JSONC).unwrap(); + assert!(ids(dir.path()).contains(&"MACH-001".to_string())); + } + + #[test] + fn genuinely_broken_jsonc_is_still_reported() { + let dir = TempDir::new().unwrap(); + let p = dir.path().join(".vscode/settings.json"); + std::fs::create_dir_all(p.parent().unwrap()).unwrap(); + std::fs::write(&p, "{ \"a\": }").unwrap(); + assert!(ids(dir.path()).contains(&"MACH-001".to_string())); + } + + #[test] + fn strip_jsonc_leaves_string_contents_alone() { + let v: serde_json::Value = serde_json::from_str(&strip_jsonc(JSONC)).unwrap(); + assert_eq!(v["url"], "http://x//y"); + assert_eq!(v["list"], serde_json::json!([1, 2])); } } diff --git a/bots/glambot/src/main.rs b/bots/glambot/src/main.rs index d826e986..50b71e1b 100644 --- a/bots/glambot/src/main.rs +++ b/bots/glambot/src/main.rs @@ -134,8 +134,11 @@ fn main() -> ExitCode { fn init_logging(level: &str) { let filter = EnvFilter::try_new(level).unwrap_or_else(|_| EnvFilter::new("info")); + // Logs go to stderr: stdout carries the JSON report that the fleet driver + // parses, and a log line there makes the report unreadable. fmt() .with_target(true) + .with_writer(std::io::stderr) .with_ansi(true) .with_env_filter(filter) .init(); diff --git a/bots/glambot/tests/integration.rs b/bots/glambot/tests/integration.rs index 19326966..b5390e3b 100644 --- a/bots/glambot/tests/integration.rs +++ b/bots/glambot/tests/integration.rs @@ -223,3 +223,21 @@ fn test_analysis_result_methods() { assert!(result.has_errors(), "has_errors() should return true"); assert!(result.files_checked > 0, "Should have checked some files"); } + +/// The fleet driver parses stdout as the JSON report, so nothing else may be +/// written there: log lines belong on stderr. +#[test] +fn json_audit_stdout_is_only_the_report() { + let dir = TempDir::new().unwrap(); + // Exactly the invocation scripts/fleet-report-back.sh uses. + let out = std::process::Command::new(env!("CARGO_BIN_EXE_glambot")) + .arg("--path") + .arg(dir.path()) + .args(["--format", "json", "audit"]) + .output() + .unwrap(); + let text = String::from_utf8(out.stdout).unwrap(); + let parsed: serde_json::Value = + serde_json::from_str(&text).unwrap_or_else(|e| panic!("stdout is not pure JSON ({e}): {text}")); + assert!(parsed.is_object()); +} From ff2ebc3d697ecb4216ade2ef45048873999da718 Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 11:33:19 +0000 Subject: [PATCH 02/18] fix(finishingbot,glambot): resolve Codex review findings on #669 - REL-008: detect aws_secret_access_key with a 40-char value (incl. / + =); prose still ignored - MACH-001: unterminated JSONC block comment no longer parses - MACH-006: no longer auto-fixable (git cannot track an empty dir; a marker would be invented content) - TOOL-001: mise.toml / .mise.toml must parse and pin at least one tool in [tools] - rust_has_code: nesting-aware block-comment stripping - .hypatia-ignore: strip inline `# reason` - PH: normalise relative paths to `/` before ignore matching Touched files were also rustfmt-formatted (base was not fmt-clean). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- .../finishingbot/src/analyzers/placeholder.rs | 66 +++++++--- bots/finishingbot/src/analyzers/release.rs | 123 ++++++++++++++---- bots/finishingbot/src/analyzers/testing.rs | 51 ++++++-- bots/finishingbot/src/analyzers/tooling.rs | 61 +++++++-- bots/finishingbot/src/ignore.rs | 43 +++++- bots/glambot/src/analyzers/machine.rs | 110 +++++++--------- 6 files changed, 328 insertions(+), 126 deletions(-) diff --git a/bots/finishingbot/src/analyzers/placeholder.rs b/bots/finishingbot/src/analyzers/placeholder.rs index 16e85865..8286fb8d 100644 --- a/bots/finishingbot/src/analyzers/placeholder.rs +++ b/bots/finishingbot/src/analyzers/placeholder.rs @@ -47,7 +47,8 @@ impl Analyzer for PlaceholderAnalyzer { let patterns_with_context: Vec = patterns .iter() .map(|p| Regex::new(p)) - .collect::, regex::Error>>()?; + .collect::, regex::Error>>( + )?; // Scan files for entry in WalkDir::new(path) @@ -100,7 +101,10 @@ impl Analyzer for PlaceholderAnalyzer { config.placeholders.max_allowed ), ) - .with_suggestion(&format!("Remove at least {} placeholders before release", excess)), + .with_suggestion(&format!( + "Remove at least {} placeholders before release", + excess + )), ); } @@ -131,12 +135,10 @@ impl PlaceholderAnalyzer { /// Check if a file is likely binary fn is_binary_file(&self, path: &Path) -> bool { let binary_extensions = [ - "exe", "dll", "so", "dylib", "bin", "obj", "o", "a", "lib", - "png", "jpg", "jpeg", "gif", "bmp", "ico", "webp", "svg", - "mp3", "mp4", "wav", "ogg", "flac", "avi", "mkv", "mov", - "zip", "tar", "gz", "bz2", "xz", "7z", "rar", - "pdf", "doc", "docx", "xls", "xlsx", "ppt", "pptx", - "wasm", "pyc", "class", "lock", + "exe", "dll", "so", "dylib", "bin", "obj", "o", "a", "lib", "png", "jpg", "jpeg", + "gif", "bmp", "ico", "webp", "svg", "mp3", "mp4", "wav", "ogg", "flac", "avi", "mkv", + "mov", "zip", "tar", "gz", "bz2", "xz", "7z", "rar", "pdf", "doc", "docx", "xls", + "xlsx", "ppt", "pptx", "wasm", "pyc", "class", "lock", ]; path.extension() @@ -165,9 +167,12 @@ impl PlaceholderAnalyzer { for match_idx in matches { if let Some(pattern) = patterns.get(match_idx) { if let Some(m) = pattern.find(line) { - let pattern_name = pattern_names.get(match_idx).map(|s| s.as_str()).unwrap_or("custom"); + let pattern_name = pattern_names + .get(match_idx) + .map(|s| s.as_str()) + .unwrap_or("custom"); let rule_id = format!("PH-{:03}", match_idx + 1); - let rel = path.strip_prefix(root).unwrap_or(path).to_string_lossy(); + let rel = normalise_rel(path.strip_prefix(root).unwrap_or(path)); if ignore.suppresses(&rule_id, &rel) { debug!(file = %rel, rule = %rule_id, "Exempted by .hypatia-ignore"); continue; @@ -247,10 +252,8 @@ impl PlaceholderAnalyzer { let lines: Vec<&str> = content.lines().collect(); // Get line numbers to remove (1-indexed in findings) - let lines_to_remove: std::collections::HashSet = file_findings - .iter() - .filter_map(|f| f.line) - .collect(); + let lines_to_remove: std::collections::HashSet = + file_findings.iter().filter_map(|f| f.line).collect(); // Filter out lines with placeholders (convert to 0-indexed) let new_lines: Vec<&str> = lines @@ -334,8 +337,22 @@ impl PlaceholderAnalyzer { } } +/// Repo-relative path with `/` separators, so `.hypatia-ignore` fragments +/// (always written with `/`) match on Windows too. +fn normalise_rel(p: &Path) -> String { + p.to_string_lossy().replace('\\', "/") +} + #[cfg(test)] mod ignore_tests { + #[test] + fn backslash_paths_are_normalised_before_matching() { + let rel = normalise_rel(Path::new(r"build\just\scan.just")); + assert_eq!(rel, "build/just/scan.just"); + let f = crate::ignore::IgnoreFile::parse("finishing_bot/PH:build/just/\n"); + assert!(f.suppresses("PH-001", &rel)); + } + use super::*; use tempfile::TempDir; @@ -386,7 +403,10 @@ mod ignore_tests { fn hypatia_ignore_exempts_the_named_path_only() { let d = repo(Some("finishing_bot/PH:build/just/\n")); let got = files(&d); - assert!(!got.contains(&"build/just/scan.just".to_string()), "{got:?}"); + assert!( + !got.contains(&"build/just/scan.just".to_string()), + "{got:?}" + ); // A real placeholder elsewhere still counts. assert!(got.contains(&"src/main.rs".to_string()), "{got:?}"); } @@ -404,11 +424,21 @@ mod ignore_tests { let d = TempDir::new().unwrap(); write(d.path(), "build/just/scan.just", "grep -E 'TODO|FIXME' .\n"); let over = PlaceholderAnalyzer.analyze(d.path(), &cfg).unwrap(); - assert!(!over.findings.iter().any(|f| f.id == "PH-100"), "at the limit"); + assert!( + !over.findings.iter().any(|f| f.id == "PH-100"), + "at the limit" + ); cfg.placeholders.max_allowed = 1; let over = PlaceholderAnalyzer.analyze(d.path(), &cfg).unwrap(); - assert!(over.findings.iter().any(|f| f.id == "PH-100"), "over the limit"); - write(d.path(), ".hypatia-ignore", "finishing_bot/PH:build/just/\n"); + assert!( + over.findings.iter().any(|f| f.id == "PH-100"), + "over the limit" + ); + write( + d.path(), + ".hypatia-ignore", + "finishing_bot/PH:build/just/\n", + ); let ok = PlaceholderAnalyzer.analyze(d.path(), &cfg).unwrap(); assert!(ok.findings.is_empty(), "{:?}", ok.findings); } diff --git a/bots/finishingbot/src/analyzers/release.rs b/bots/finishingbot/src/analyzers/release.rs index ceecdf0d..64d22951 100644 --- a/bots/finishingbot/src/analyzers/release.rs +++ b/bots/finishingbot/src/analyzers/release.rs @@ -120,7 +120,10 @@ impl ReleaseAnalyzer { "REL-001", "Version Inconsistency", Severity::Error, - &format!("Version numbers are inconsistent:\n {}", version_list.join("\n ")), + &format!( + "Version numbers are inconsistent:\n {}", + version_list.join("\n ") + ), ) .with_suggestion("Ensure all version files have the same version number"), ); @@ -148,16 +151,11 @@ impl ReleaseAnalyzer { "import pdb", "pdb.set_trace", "dbg!", - "println!", // In Rust release builds + "println!", // In Rust release builds "#[cfg(debug_assertions)]", ]; - let debug_files = [ - ".env.local", - ".env.development", - "debug.log", - "*.log", - ]; + let debug_files = [".env.local", ".env.development", "debug.log", "*.log"]; // Check for debug files for pattern in &debug_files { @@ -229,7 +227,10 @@ impl ReleaseAnalyzer { "REL-003", "Debug Statement", Severity::Info, - &format!("Found {} occurrence(s) of '{}' in source", count, pattern), + &format!( + "Found {} occurrence(s) of '{}' in source", + count, pattern + ), ) .with_file(entry_path.to_path_buf()) .with_suggestion("Review and remove debug statements for release"), @@ -344,7 +345,10 @@ impl ReleaseAnalyzer { Err(_) => return, }; - let actual_hash = if config.release.hash_algorithms.contains(&"SHA512".to_string()) + let actual_hash = if config + .release + .hash_algorithms + .contains(&"SHA512".to_string()) && hash_path.to_string_lossy().contains("512") { let mut hasher = Sha512::new(); @@ -362,7 +366,10 @@ impl ReleaseAnalyzer { "REL-007", "Hash Mismatch", Severity::Error, - &format!("Hash mismatch for {}: expected {}, got {}", filename, expected_hash, actual_hash), + &format!( + "Hash mismatch for {}: expected {}, got {}", + filename, expected_hash, actual_hash + ), ) .with_file(hash_path.to_path_buf()) .with_suggestion("Regenerate hash file or investigate file corruption"), @@ -395,9 +402,29 @@ impl ReleaseAnalyzer { .map(|ext| { matches!( ext, - "rs" | "js" | "ts" | "py" | "rb" | "go" | "java" | "kt" | "swift" - | "c" | "cpp" | "h" | "hpp" | "yml" | "yaml" | "json" | "toml" - | "xml" | "env" | "conf" | "cfg" | "ini" | "sh" | "bash" + "rs" | "js" + | "ts" + | "py" + | "rb" + | "go" + | "java" + | "kt" + | "swift" + | "c" + | "cpp" + | "h" + | "hpp" + | "yml" + | "yaml" + | "json" + | "toml" + | "xml" + | "env" + | "conf" + | "cfg" + | "ini" + | "sh" + | "bash" ) }) .unwrap_or(false); @@ -416,7 +443,9 @@ impl ReleaseAnalyzer { &format!("Potential {} found in source file", name), ) .with_file(entry_path.to_path_buf()) - .with_suggestion("Remove sensitive data and rotate any exposed credentials"), + .with_suggestion( + "Remove sensitive data and rotate any exposed credentials", + ), ); } } @@ -438,7 +467,9 @@ impl ReleaseAnalyzer { "No [profile.release] configuration in Cargo.toml", ) .with_file(cargo_toml) - .with_suggestion("Consider adding release optimizations (lto, codegen-units)"), + .with_suggestion( + "Consider adding release optimizations (lto, codegen-units)", + ), ); } } @@ -455,7 +486,9 @@ impl ReleaseAnalyzer { Severity::Info, "No GitHub release workflow found", ) - .with_suggestion("Consider adding .github/workflows/release.yml for automated releases"), + .with_suggestion( + "Consider adding .github/workflows/release.yml for automated releases", + ), ); } } @@ -469,10 +502,23 @@ impl ReleaseAnalyzer { static SENSITIVE_PATTERNS: std::sync::LazyLock> = std::sync::LazyLock::new(|| { [ - (r#"(?i)api[_-]?key\s*[:=]\s*['"]?[a-zA-Z0-9]{20,}"#, "API Key"), - (r#"(?i)secret[_-]?key\s*[:=]\s*['"]?[a-zA-Z0-9]{20,}"#, "Secret Key"), + ( + r#"(?i)api[_-]?key\s*[:=]\s*['"]?[a-zA-Z0-9]{20,}"#, + "API Key", + ), + ( + r#"(?i)secret[_-]?key\s*[:=]\s*['"]?[a-zA-Z0-9]{20,}"#, + "Secret Key", + ), + ( + r#"(?i)aws[_-]?secret[_-]?(access[_-]?)?key\s*[:=]\s*['"]?[A-Za-z0-9/+=]{40}\b"#, + "AWS Secret Access Key", + ), (r#"(?i)password\s*[:=]\s*['"]?[^\s'"]{8,}"#, "Password"), - (r"-----BEGIN ([A-Z0-9]+ )*PRIVATE KEY( BLOCK)?-----", "Private Key"), + ( + r"-----BEGIN ([A-Z0-9]+ )*PRIVATE KEY( BLOCK)?-----", + "Private Key", + ), (r"\b(AKIA|ASIA)[0-9A-Z]{16}\b", "AWS Access Key ID"), (r"ghp_[a-zA-Z0-9]{36}", "GitHub Personal Access Token"), (r"gho_[a-zA-Z0-9]{36}", "GitHub OAuth Token"), @@ -511,9 +557,34 @@ mod sensitive_tests { } } + #[test] + fn aws_secret_access_key_value_is_key_material() { + for text in [ + r#"aws_secret_access_key = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY""#, + "AWS_SECRET_ACCESS_KEY=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + ] { + assert_eq!( + sensitive_kind(text), + Some("AWS Secret Access Key"), + "{text}" + ); + } + assert_eq!(sensitive_kind("aws_secret_access_key = \"$SECRET\""), None); + assert_eq!( + sensitive_kind("The aws_secret_access_key is rotated monthly"), + None + ); + } + #[test] fn pem_headers_are_key_material() { - for kind in ["RSA PRIVATE KEY", "PRIVATE KEY", "EC PRIVATE KEY", "OPENSSH PRIVATE KEY", "ENCRYPTED PRIVATE KEY"] { + for kind in [ + "RSA PRIVATE KEY", + "PRIVATE KEY", + "EC PRIVATE KEY", + "OPENSSH PRIVATE KEY", + "ENCRYPTED PRIVATE KEY", + ] { let text = format!("{}{}{}\nMIIB\n", "-----BEGIN ", kind, "-----"); assert_eq!(sensitive_kind(&text), Some("Private Key"), "{kind}"); } @@ -527,7 +598,10 @@ mod sensitive_tests { assert_eq!(sensitive_kind(&aws), Some("AWS Access Key ID")); let ghp = format!("t = {}{}", "ghp_", "a".repeat(36)); assert_eq!(sensitive_kind(&ghp), Some("GitHub Personal Access Token")); - assert_eq!(sensitive_kind("api_key = \"abcdefghij0123456789xyz\""), Some("API Key")); + assert_eq!( + sensitive_kind("api_key = \"abcdefghij0123456789xyz\""), + Some("API Key") + ); } #[test] @@ -544,7 +618,10 @@ mod sensitive_tests { std::fs::write( dir.path().join("leak.toml"), - format!("k = \"\"\"\n-----BEGIN {} KEY-----\nAAAA\n\"\"\"\n", "PRIVATE"), + format!( + "k = \"\"\"\n-----BEGIN {} KEY-----\nAAAA\n\"\"\"\n", + "PRIVATE" + ), ) .unwrap(); ReleaseAnalyzer.check_sensitive_data(dir.path(), &Config::default(), &mut result); diff --git a/bots/finishingbot/src/analyzers/testing.rs b/bots/finishingbot/src/analyzers/testing.rs index a5bde3a9..bda5f847 100644 --- a/bots/finishingbot/src/analyzers/testing.rs +++ b/bots/finishingbot/src/analyzers/testing.rs @@ -428,6 +428,28 @@ mod agda_layout_tests { } } +/// Remove Rust block comments, which nest (`/* a /* b */ c */`). +fn strip_block_comments(src: &str) -> String { + let b = src.as_bytes(); + let mut out = Vec::with_capacity(b.len()); + let (mut i, mut depth) = (0, 0usize); + while i < b.len() { + if b[i] == b'/' && b.get(i + 1) == Some(&b'*') { + depth += 1; + i += 2; + } else if depth > 0 && b[i] == b'*' && b.get(i + 1) == Some(&b'/') { + depth -= 1; + i += 2; + } else { + if depth == 0 || b[i] == b'\n' { + out.push(b[i]); + } + i += 1; + } + } + String::from_utf8_lossy(&out).into_owned() +} + /// Whether the Rust project at `root` has any code yet. /// /// "No code" is exact: no `.rs` file outside build output (`target/`), @@ -437,9 +459,6 @@ mod agda_layout_tests { /// all "no code"; `fn main() {}` is code. Used to exempt benchmark and stress /// requirements from a freshly scaffolded crate. pub(crate) fn rust_has_code(root: &Path) -> bool { - static BLOCK_COMMENT: std::sync::LazyLock = std::sync::LazyLock::new(|| { - regex::Regex::new(r"(?s)/\*.*?\*/").expect("block-comment regex is a valid constant pattern") - }); walkdir::WalkDir::new(root) .follow_links(false) .into_iter() @@ -448,14 +467,14 @@ pub(crate) fn rust_has_code(root: &Path) -> bool { return true; } let name = e.file_name().to_string_lossy(); - !(name.starts_with('.') || matches!(name.as_ref(), "target" | "vendor" | "node_modules")) + !(name.starts_with('.') + || matches!(name.as_ref(), "target" | "vendor" | "node_modules")) }) .filter_map(|e| e.ok()) .filter(|e| e.file_type().is_file() && e.path().extension().is_some_and(|x| x == "rs")) .any(|e| { std::fs::read_to_string(e.path()).is_ok_and(|src| { - BLOCK_COMMENT - .replace_all(&src, "") + strip_block_comments(&src) .lines() .map(str::trim) .any(|l| !l.is_empty() && !l.starts_with("//")) @@ -476,7 +495,11 @@ mod no_code_tests { fn rust_repo(lib: Option<&str>) -> TempDir { let d = TempDir::new().unwrap(); - write(d.path(), "Cargo.toml", "[package]\nname = \"x\"\nversion = \"0.1.0\"\n"); + write( + d.path(), + "Cargo.toml", + "[package]\nname = \"x\"\nversion = \"0.1.0\"\n", + ); if let Some(body) = lib { write(d.path(), "src/lib.rs", body); } @@ -504,12 +527,24 @@ mod no_code_tests { let d = rust_repo(body); assert!(!rust_has_code(d.path()), "{body:?}"); } - for body in ["fn main() {}\n", "// c\npub fn f() {}\n", "/* c */ pub struct S;\n"] { + for body in [ + "fn main() {}\n", + "// c\npub fn f() {}\n", + "/* c */ pub struct S;\n", + ] { let d = rust_repo(Some(body)); assert!(rust_has_code(d.path()), "{body:?}"); } } + #[test] + fn nested_block_comments_are_stripped() { + let d = rust_repo(Some("/* a /* b */ c */\n// x\n")); + assert!(!rust_has_code(d.path())); + let d = rust_repo(Some("/* a /* b */ c */ fn f() {}\n")); + assert!(rust_has_code(d.path())); + } + #[test] fn code_in_build_output_vendor_or_hidden_dirs_does_not_count() { let d = rust_repo(Some("// nothing\n")); diff --git a/bots/finishingbot/src/analyzers/tooling.rs b/bots/finishingbot/src/analyzers/tooling.rs index 93c7c3f2..b79deb80 100644 --- a/bots/finishingbot/src/analyzers/tooling.rs +++ b/bots/finishingbot/src/analyzers/tooling.rs @@ -2,7 +2,7 @@ // SPDX-FileCopyrightText: 2025 Jonathan D.A. Jewell //! Tooling and version management analyzer -use super::{first_existing, AnalysisResult, Analyzer, Finding, Severity}; +use super::{AnalysisResult, Analyzer, Finding, Severity}; use crate::config::Config; use crate::error::Result; use std::path::Path; @@ -11,6 +11,19 @@ use tracing::debug; /// mise manifests that pin tool versions (accepted in place of `.tool-versions`). const MISE_MANIFESTS: &[&str] = &["mise.toml", ".mise.toml"]; +/// Whether `p` is a parseable mise manifest with at least one entry in `[tools]`. +fn mise_pins_tools(p: &Path) -> bool { + std::fs::read_to_string(p) + .ok() + .and_then(|s| s.parse::().ok()) + .and_then(|t| { + t.get("tools") + .and_then(|v| v.as_table()) + .map(|t| !t.is_empty()) + }) + .unwrap_or(false) +} + /// Tooling and version management analyzer pub struct ToolingAnalyzer; @@ -32,7 +45,10 @@ impl Analyzer for ToolingAnalyzer { // Check for a tool-version pin: .tool-versions (asdf/mise) or mise.toml // (mise's own manifest, which the estate uses). Either satisfies TOOL-001. let tool_versions = path.join(".tool-versions"); - let mise_manifest = first_existing(path, MISE_MANIFESTS); + let mise_manifest = MISE_MANIFESTS + .iter() + .map(|n| path.join(n)) + .find(|p| mise_pins_tools(p)); if !tool_versions.exists() && mise_manifest.is_some() { debug!("Found mise manifest instead of .tool-versions"); } else if !tool_versions.exists() { @@ -41,7 +57,7 @@ impl Analyzer for ToolingAnalyzer { "TOOL-001", "Missing tool-version pin", Severity::Error, - "Neither .tool-versions nor mise.toml found for asdf/mise version management.", + "Neither .tool-versions nor a mise.toml with a non-empty [tools] table found for asdf/mise version management.", ) .with_file(path.join("mise.toml")) .with_suggestion("Create mise.toml (or .tool-versions) with required tool versions (rust, just, etc.)"), @@ -90,7 +106,11 @@ impl Analyzer for ToolingAnalyzer { if let Ok(entries) = std::fs::read_dir(&workflows_dir) { let workflow_count = entries .filter_map(|e| e.ok()) - .filter(|e| e.path().extension().is_some_and(|ext| ext == "yml" || ext == "yaml")) + .filter(|e| { + e.path() + .extension() + .is_some_and(|ext| ext == "yml" || ext == "yaml") + }) .count(); if workflow_count < 10 { @@ -124,12 +144,7 @@ impl Analyzer for ToolingAnalyzer { impl ToolingAnalyzer { /// Validate .tool-versions content - fn validate_tool_versions( - &self, - content: &str, - file_path: &Path, - result: &mut AnalysisResult, - ) { + fn validate_tool_versions(&self, content: &str, file_path: &Path, result: &mut AnalysisResult) { let is_rust = file_path .parent() .map(|p| p.join("Cargo.toml").exists()) @@ -194,6 +209,27 @@ mod tests { .collect() } + #[test] + fn tool_001_requires_mise_manifest_to_pin_tools() { + for (body, ok) in [ + ("", false), + ("not [valid toml", false), + ("[env]\nA = \"1\"\n", false), + ("[tools]\n", false), + ("[tools]\nrust = \"1.83\"\n", true), + ] { + for name in ["mise.toml", ".mise.toml"] { + let d = TempDir::new().unwrap(); + std::fs::write(d.path().join(name), body).unwrap(); + assert_eq!( + !ids(d.path()).contains(&"TOOL-001".to_string()), + ok, + "{name}: {body:?}" + ); + } + } + } + #[test] fn tool_001_fires_when_neither_pin_exists_and_names_mise_toml() { let d = TempDir::new().unwrap(); @@ -204,7 +240,10 @@ mod tests { .into_iter() .find(|f| f.id == "TOOL-001") .expect("TOOL-001 on a repo with no pin"); - assert_eq!(f.file.as_deref(), Some(d.path().join("mise.toml").as_path())); + assert_eq!( + f.file.as_deref(), + Some(d.path().join("mise.toml").as_path()) + ); } #[test] diff --git a/bots/finishingbot/src/ignore.rs b/bots/finishingbot/src/ignore.rs index eae56211..d74a6342 100644 --- a/bots/finishingbot/src/ignore.rs +++ b/bots/finishingbot/src/ignore.rs @@ -38,6 +38,18 @@ struct Entry { fragment: String, } +/// Drop a trailing `# reason`: a `#` that starts the line or follows whitespace. +fn strip_inline_comment(line: &str) -> &str { + let mut prev_ws = true; + for (i, c) in line.char_indices() { + if c == '#' && prev_ws { + return &line[..i]; + } + prev_ws = c.is_whitespace(); + } + line +} + impl IgnoreFile { /// Load `/.hypatia-ignore`; an absent or unreadable file exempts nothing. pub fn load(root: &Path) -> Self { @@ -49,12 +61,14 @@ impl IgnoreFile { pub fn parse(text: &str) -> Self { let mut entries = Vec::new(); for line in text.lines() { - let line = line.trim(); + let line = strip_inline_comment(line).trim(); if line.is_empty() || line.starts_with('#') { continue; } let (scope, fragment) = match line.split_once(':') { - Some((scope, fragment)) if scope.contains('/') && !scope.contains(char::is_whitespace) => { + Some((scope, fragment)) + if scope.contains('/') && !scope.contains(char::is_whitespace) => + { (Some(scope), fragment) } _ => (None, line), @@ -70,10 +84,17 @@ impl IgnoreFile { if module != MODULE { continue; // another module's entry } - if rule == "*" { None } else { Some(rule.to_string()) } + if rule == "*" { + None + } else { + Some(rule.to_string()) + } } }; - entries.push(Entry { rule, fragment: fragment.to_string() }); + entries.push(Entry { + rule, + fragment: fragment.to_string(), + }); } Self { entries } } @@ -84,7 +105,10 @@ impl IgnoreFile { self.entries.iter().any(|e| { rel.contains(&e.fragment) && e.rule.as_deref().is_none_or(|r| { - rule_id == r || rule_id.strip_prefix(r).is_some_and(|rest| rest.starts_with('-')) + rule_id == r + || rule_id + .strip_prefix(r) + .is_some_and(|rest| rest.starts_with('-')) }) }) } @@ -123,6 +147,15 @@ generated/ assert!(!f.suppresses("PHX-001", "build/just/a.just")); // prefix needs '-' boundary } + #[test] + fn inline_reason_comment_is_not_part_of_the_path() { + let f = IgnoreFile::parse("finishing_bot/PH:build/just/ # scanner names tokens\ngenerated/ # reason\nkeep#hash/\n"); + assert!(f.suppresses("PH-001", "build/just/a.just")); + assert!(f.suppresses("CLM-001", "generated/x")); + assert!(f.suppresses("CLM-001", "keep#hash/x")); + assert!(!f.suppresses("PH-001", "reason")); + } + #[test] fn absent_file_exempts_nothing() { let d = tempfile::TempDir::new().unwrap(); diff --git a/bots/glambot/src/analyzers/machine.rs b/bots/glambot/src/analyzers/machine.rs index b2d59ad5..eca99da0 100644 --- a/bots/glambot/src/analyzers/machine.rs +++ b/bots/glambot/src/analyzers/machine.rs @@ -57,19 +57,6 @@ impl Analyzer for MachineAnalyzer { for finding in findings { match finding.id.as_str() { - "MACH-006" => { - // Create missing .machine_readable/ directory structure - // Directory only: the STATE/META/ECOSYSTEM .scm templates this - // used to generate are retired (clauses of the chora deed now). - let mr_dir = path.join(".machine_readable"); - if !mr_dir.exists() { - std::fs::create_dir_all(&mr_dir)?; - fixes.push(format!( - "Created .machine_readable/ directory at {}", - mr_dir.display() - )); - } - } "MACH-003" => { // Add structured data (schema.org JSON-LD) template if let Some(ref file) = finding.file { @@ -102,10 +89,7 @@ impl Analyzer for MachineAnalyzer { let content = "# robots.txt for repository documentation\nUser-agent: *\nAllow: /\n\nSitemap: TODO\n"; std::fs::write(&robots_path, content)?; - fixes.push(format!( - "Created robots.txt at {}", - robots_path.display() - )); + fixes.push(format!("Created robots.txt at {}", robots_path.display())); } } _ => {} @@ -208,7 +192,8 @@ impl MachineAnalyzer { if let Ok(content) = std::fs::read_to_string(entry.path()) { let has_json_ld = content.contains("application/ld+json"); - let has_microdata = content.contains("itemscope") || content.contains("itemprop"); + let has_microdata = + content.contains("itemscope") || content.contains("itemprop"); let has_schema_org = content.contains("schema.org"); if !has_json_ld && !has_microdata && config.machine.require_schema_org { @@ -220,15 +205,20 @@ impl MachineAnalyzer { "HTML file has no structured data (JSON-LD or microdata).", ) .with_file(entry.path().to_path_buf()) - .with_suggestion("Add schema.org structured data for better machine readability"), + .with_suggestion( + "Add schema.org structured data for better machine readability", + ), ); } if has_json_ld { // Try to validate JSON-LD - if let Some(json_ld_start) = content.find("") { - let json_ld = &content[json_ld_start + 36..json_ld_start + json_ld_end]; + let json_ld = + &content[json_ld_start + 36..json_ld_start + json_ld_end]; if serde_json::from_str::(json_ld).is_err() { result.add( Finding::new( @@ -283,8 +273,10 @@ impl MachineAnalyzer { Severity::Warning, "No .machine_readable/ directory found (contractiles and other machine-readable estate metadata live here).", ) - .fixable() .with_file(mr_dir.clone()) + // Not auto-fixable: git cannot track an empty directory, and a + // placeholder file would be invented content. What belongs here + // (contractiles, the chora deed) is owner-authored. .with_suggestion("Create the .machine_readable/ directory"), ); return; @@ -337,11 +329,18 @@ fn strip_jsonc(src: &str) -> String { i += 1; } } else if c == '/' && chars.get(i + 1) == Some(&'*') { + let start = i; i += 2; while i + 1 < chars.len() && !(chars[i] == '*' && chars[i + 1] == '/') { i += 1; } - i = (i + 2).min(chars.len()); + if i + 1 < chars.len() { + i += 2; + } else { + // Unterminated block comment: keep the text so parsing fails. + out.extend(&chars[start..]); + i = chars.len(); + } } else { out.push(c); i += 1; @@ -433,11 +432,7 @@ mod tests { let result = analyzer.analyze(dir.path(), &config).unwrap(); assert!(result.has_errors(), "Invalid JSON should produce errors"); - let error_ids: Vec<&str> = result - .errors() - .iter() - .map(|f| f.id.as_str()) - .collect(); + let error_ids: Vec<&str> = result.errors().iter().map(|f| f.id.as_str()).collect(); assert!( error_ids.contains(&"MACH-001"), "Should report MACH-001 for invalid JSON" @@ -453,11 +448,7 @@ mod tests { let config = default_config(); let result = analyzer.analyze(dir.path(), &config).unwrap(); - let ids: Vec<&str> = result - .findings - .iter() - .map(|f| f.id.as_str()) - .collect(); + let ids: Vec<&str> = result.findings.iter().map(|f| f.id.as_str()).collect(); assert!( ids.contains(&"MACH-006"), "Should report MACH-006 for missing .machine_readable/" @@ -465,36 +456,17 @@ mod tests { } #[test] - fn test_fix_creates_machine_readable_structure() { + fn mach_006_is_not_auto_fixable_and_fix_creates_nothing() { let dir = TempDir::new().unwrap(); - let analyzer = MachineAnalyzer::default(); let config = default_config(); - - let findings = vec![Finding::new( - "MACH-006", - "Missing .machine_readable/", - Severity::Warning, - "No .machine_readable/ directory found", - )]; - + let result = analyzer.analyze(dir.path(), &config).unwrap(); + let f = result.findings.iter().find(|f| f.id == "MACH-006").unwrap(); + assert!(!f.fixable, "an empty dir is untracked by git; not fixable"); + let findings = vec![f.clone()]; let fixes = analyzer.fix(dir.path(), &config, &findings).unwrap(); - - assert!( - fixes.iter().any(|f| f.contains(".machine_readable")), - "Should create .machine_readable directory" - ); - assert!( - dir.path().join(".machine_readable").exists(), - ".machine_readable/ should exist after fix" - ); - // The retired .scm descriptive files are not regenerated. - for scm in ["STATE.scm", "META.scm", "ECOSYSTEM.scm"] { - assert!( - !dir.path().join(".machine_readable").join(scm).exists(), - "{scm} is retired and must not be generated" - ); - } + assert_eq!(fixes, vec!["No machine-readability fixes applicable".to_string()]); + assert!(!dir.path().join(".machine_readable").exists()); } fn ids(dir: &Path) -> Vec { @@ -519,11 +491,16 @@ mod tests { assert!(ids(empty.path()).contains(&"MACH-006".to_string())); } - const JSONC: &str = "{\n // a comment\n \"url\": \"http://x//y\", /* inline */\n \"list\": [1, 2,],\n}\n"; + const JSONC: &str = + "{\n // a comment\n \"url\": \"http://x//y\", /* inline */\n \"list\": [1, 2,],\n}\n"; #[test] fn devcontainer_and_vscode_json_are_parsed_as_jsonc() { - for rel in [".devcontainer/devcontainer.json", ".vscode/settings.json", ".devcontainer/sub/devcontainer.json"] { + for rel in [ + ".devcontainer/devcontainer.json", + ".vscode/settings.json", + ".devcontainer/sub/devcontainer.json", + ] { let dir = TempDir::new().unwrap(); let p = dir.path().join(rel); std::fs::create_dir_all(p.parent().unwrap()).unwrap(); @@ -548,6 +525,17 @@ mod tests { assert!(ids(dir.path()).contains(&"MACH-001".to_string())); } + #[test] + fn unterminated_block_comment_is_invalid() { + let dir = TempDir::new().unwrap(); + let p = dir.path().join(".vscode/settings.json"); + std::fs::create_dir_all(p.parent().unwrap()).unwrap(); + std::fs::write(&p, "{\"a\": 1} /*").unwrap(); + assert!(ids(dir.path()).contains(&"MACH-001".to_string())); + std::fs::write(&p, "{\"a\": 1} /* ok */").unwrap(); + assert!(!ids(dir.path()).contains(&"MACH-001".to_string())); + } + #[test] fn strip_jsonc_leaves_string_contents_alone() { let v: serde_json::Value = serde_json::from_str(&strip_jsonc(JSONC)).unwrap(); From ab3fb5c3a0d96fb1bf0b6ad01c2185e8d1587215 Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 12:02:58 +0000 Subject: [PATCH 03/18] fix(glambot): a stripped JSONC block comment leaves whitespace, not nothing strip_jsonc removed /*...*/ without a separator, so {"value": 1/*c*/2} became the valid number 12. Replace the comment with a space and keep its newlines. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/glambot/src/analyzers/machine.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/bots/glambot/src/analyzers/machine.rs b/bots/glambot/src/analyzers/machine.rs index eca99da0..71e3fbc2 100644 --- a/bots/glambot/src/analyzers/machine.rs +++ b/bots/glambot/src/analyzers/machine.rs @@ -335,6 +335,10 @@ fn strip_jsonc(src: &str) -> String { i += 1; } if i + 1 < chars.len() { + // A comment is whitespace, not nothing: `1/*c*/2` must not + // fuse into `12`. Newlines inside it are kept (line numbers). + out.push(' '); + out.extend(chars[start..i].iter().filter(|c| **c == '\n')); i += 2; } else { // Unterminated block comment: keep the text so parsing fails. @@ -542,4 +546,23 @@ mod tests { assert_eq!(v["url"], "http://x//y"); assert_eq!(v["list"], serde_json::json!([1, 2])); } + + #[test] + fn a_block_comment_is_whitespace_so_it_cannot_fuse_tokens() { + // `1/*c*/2` must not become the valid number 12. + let stripped = strip_jsonc(r#"{"value": 1/*c*/2}"#); + assert!( + serde_json::from_str::(&stripped).is_err(), + "fused into valid JSON: {stripped}" + ); + // A comment between tokens that need no separator still parses. + let ok = strip_jsonc("{\"a\": 1, /* c */ \"b\": 2}"); + assert!(serde_json::from_str::(&ok).is_ok()); + } + + #[test] + fn a_multiline_block_comment_keeps_its_newlines() { + let stripped = strip_jsonc("{\"a\":\n/* one\ntwo\nthree */ 1}"); + assert_eq!(stripped.matches('\n').count(), 3, "{stripped:?}"); + } } From ee5464d258c6f9e126906091b68939be70dbc095 Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 12:24:26 +0000 Subject: [PATCH 04/18] fix(finishingbot): AWS secret pattern terminates on any non-key char or end of input The trailing \b cannot follow a final / + or =, so a 40-char key ending in one of those escaped detection. Use (?:$|[^A-Za-z0-9/+=]) instead. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/finishingbot/src/analyzers/release.rs | 28 +++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/bots/finishingbot/src/analyzers/release.rs b/bots/finishingbot/src/analyzers/release.rs index 64d22951..3acdd0f0 100644 --- a/bots/finishingbot/src/analyzers/release.rs +++ b/bots/finishingbot/src/analyzers/release.rs @@ -511,7 +511,7 @@ static SENSITIVE_PATTERNS: std::sync::LazyLock> = "Secret Key", ), ( - r#"(?i)aws[_-]?secret[_-]?(access[_-]?)?key\s*[:=]\s*['"]?[A-Za-z0-9/+=]{40}\b"#, + r#"(?i)aws[_-]?secret[_-]?(access[_-]?)?key\s*[:=]\s*['"]?[A-Za-z0-9/+=]{40}(?:$|[^A-Za-z0-9/+=])"#, "AWS Secret Access Key", ), (r#"(?i)password\s*[:=]\s*['"]?[^\s'"]{8,}"#, "Password"), @@ -576,6 +576,32 @@ mod sensitive_tests { ); } + #[test] + fn aws_secret_ending_in_base64_punctuation_is_key_material() { + // The 40th character may be `/`, `+` or `=`: `\b` cannot terminate those. + for last in ['/', '+', '=', 'A', '9'] { + let key = format!("{}{}", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKE", last); + assert_eq!(key.len(), 40); + for text in [ + format!("aws_secret_access_key = \"{key}\""), + format!("AWS_SECRET_ACCESS_KEY={key}"), + format!("AWS_SECRET_ACCESS_KEY={key}\n"), + format!("aws_secret_access_key: {key} # note"), + ] { + assert_eq!( + sensitive_kind(&text), + Some("AWS Secret Access Key"), + "{text}" + ); + } + } + // A run longer than 40 key characters is not a 40-character key. + let long = format!("AWS_SECRET_ACCESS_KEY={}", "A".repeat(41)); + assert_eq!(sensitive_kind(&long), None); + let long_slash = format!("AWS_SECRET_ACCESS_KEY={}/", "A".repeat(40)); + assert_eq!(sensitive_kind(&long_slash), None); + } + #[test] fn pem_headers_are_key_material() { for kind in [ From 383dab9a4ca79ba7270c8a5f6f1f4776e42f091c Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 12:29:29 +0000 Subject: [PATCH 05/18] fix(finishingbot): a mise manifest must pin the project's tools, like .tool-versions A manifest pinning only unrelated tools (e.g. node) passed TOOL-001 silently for a Rust repo. The mise branch now runs the same project-relevant checks as the .tool-versions branch (TOOL-005 rust, TOOL-006 just), sharing one function. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/finishingbot/src/analyzers/tooling.rs | 141 ++++++++++++++++----- 1 file changed, 107 insertions(+), 34 deletions(-) diff --git a/bots/finishingbot/src/analyzers/tooling.rs b/bots/finishingbot/src/analyzers/tooling.rs index b79deb80..4078a323 100644 --- a/bots/finishingbot/src/analyzers/tooling.rs +++ b/bots/finishingbot/src/analyzers/tooling.rs @@ -11,17 +11,24 @@ use tracing::debug; /// mise manifests that pin tool versions (accepted in place of `.tool-versions`). const MISE_MANIFESTS: &[&str] = &["mise.toml", ".mise.toml"]; -/// Whether `p` is a parseable mise manifest with at least one entry in `[tools]`. -fn mise_pins_tools(p: &Path) -> bool { - std::fs::read_to_string(p) - .ok() - .and_then(|s| s.parse::().ok()) - .and_then(|t| { - t.get("tools") - .and_then(|v| v.as_table()) - .map(|t| !t.is_empty()) - }) - .unwrap_or(false) +/// The tool names a parseable mise manifest pins in a non-empty `[tools]` +/// table (`None` if it is unparseable or pins nothing). A backend prefix +/// (`asdf:just`, `core:rust`) is dropped, so the name is the tool itself. +fn mise_tool_names(p: &Path) -> Option> { + let table = std::fs::read_to_string(p) + .ok()? + .parse::() + .ok()?; + let tools = table.get("tools")?.as_table()?; + if tools.is_empty() { + return None; + } + Some( + tools + .keys() + .map(|k| k.rsplit(':').next().unwrap_or(k).to_string()) + .collect(), + ) } /// Tooling and version management analyzer @@ -48,9 +55,19 @@ impl Analyzer for ToolingAnalyzer { let mise_manifest = MISE_MANIFESTS .iter() .map(|n| path.join(n)) - .find(|p| mise_pins_tools(p)); + .find_map(|p| mise_tool_names(&p).map(|names| (p, names))); if !tool_versions.exists() && mise_manifest.is_some() { debug!("Found mise manifest instead of .tool-versions"); + // Same project-relevant tools as the .tool-versions branch: a + // manifest pinning only unrelated tools is not a clean pass. + if let Some((manifest, names)) = &mise_manifest { + self.check_project_tools( + |t| names.iter().any(|n| n == t), + manifest, + "the mise manifest", + &mut result, + ); + } } else if !tool_versions.exists() { result.add( Finding::new( @@ -145,50 +162,69 @@ impl Analyzer for ToolingAnalyzer { impl ToolingAnalyzer { /// Validate .tool-versions content fn validate_tool_versions(&self, content: &str, file_path: &Path, result: &mut AnalysisResult) { + self.check_project_tools( + |t| content.contains(&format!("{t} ")), + file_path, + ".tool-versions", + result, + ); + + // Check for empty or very short file + if content.trim().is_empty() { + result.add( + Finding::new( + "TOOL-007", + "Empty .tool-versions", + Severity::Error, + ".tool-versions file is empty.", + ) + .with_file(file_path.to_path_buf()) + .with_suggestion("Add tool versions (e.g., 'rust 1.83.0')"), + ); + } + } + + /// The project-relevant tools a pin file (`.tool-versions` or a mise + /// manifest) must carry: rust (TOOL-005) and just (TOOL-006) for a Rust + /// project. `pins(t)` says whether the pin file pins tool `t`; `label` + /// names the pin file in the finding. + fn check_project_tools( + &self, + pins: impl Fn(&str) -> bool, + file_path: &Path, + label: &str, + result: &mut AnalysisResult, + ) { let is_rust = file_path .parent() .map(|p| p.join("Cargo.toml").exists()) .unwrap_or(false); // Check for rust version if Rust project - if is_rust && !content.contains("rust ") { + if is_rust && !pins("rust") { result.add( Finding::new( "TOOL-005", - "Missing rust version in .tool-versions", + &format!("Missing rust version in {label}"), Severity::Warning, - "Rust project should specify rust version in .tool-versions.", + &format!("Rust project should specify rust version in {label}."), ) .with_file(file_path.to_path_buf()) - .with_suggestion("Add 'rust ' to .tool-versions"), + .with_suggestion(&format!("Add rust to {label}")), ); } // Check for just (task runner) - if is_rust && !content.contains("just ") { + if is_rust && !pins("just") { result.add( Finding::new( "TOOL-006", - "Missing just version in .tool-versions", + &format!("Missing just version in {label}"), Severity::Info, - "Consider adding 'just' task runner to .tool-versions.", + &format!("Consider adding the 'just' task runner to {label}."), ) .with_file(file_path.to_path_buf()) - .with_suggestion("Add 'just ' to .tool-versions if using justfile"), - ); - } - - // Check for empty or very short file - if content.trim().is_empty() { - result.add( - Finding::new( - "TOOL-007", - "Empty .tool-versions", - Severity::Error, - ".tool-versions file is empty.", - ) - .with_file(file_path.to_path_buf()) - .with_suggestion("Add tool versions (e.g., 'rust 1.83.0')"), + .with_suggestion(&format!("Add just to {label} if using justfile")), ); } } @@ -230,6 +266,43 @@ mod tests { } } + #[test] + fn a_mise_manifest_must_pin_the_tools_the_project_needs() { + let write = |d: &TempDir, mise: &str| { + std::fs::write(d.path().join("Cargo.toml"), "[package]\nname = \"x\"\n").unwrap(); + std::fs::write(d.path().join("mise.toml"), mise).unwrap(); + }; + // Unrelated tools only: a Rust repo gets the same findings as a + // `.tool-versions` that lacks rust and just. + let d = TempDir::new().unwrap(); + write(&d, "[tools]\nnode = \"22\"\n"); + let got = ids(d.path()); + assert!(got.contains(&"TOOL-005".to_string()), "{got:?}"); + assert!(got.contains(&"TOOL-006".to_string()), "{got:?}"); + // rust without just: only the just note. + let d = TempDir::new().unwrap(); + write(&d, "[tools]\nrust = \"1.90\"\n"); + let got = ids(d.path()); + assert!(!got.contains(&"TOOL-005".to_string()), "{got:?}"); + assert!(got.contains(&"TOOL-006".to_string()), "{got:?}"); + // Both pinned (also behind a backend prefix): clean. + let d = TempDir::new().unwrap(); + write(&d, "[tools]\nrust = \"1.90\"\n\"asdf:just\" = \"1.40\"\n"); + let got = ids(d.path()); + assert!( + !got.iter().any(|i| i == "TOOL-005" || i == "TOOL-006"), + "{got:?}" + ); + // A non-Rust repo is not asked for rust or just. + let d = TempDir::new().unwrap(); + std::fs::write(d.path().join("mise.toml"), "[tools]\nnode = \"22\"\n").unwrap(); + let got = ids(d.path()); + assert!( + !got.iter().any(|i| i == "TOOL-005" || i == "TOOL-006"), + "{got:?}" + ); + } + #[test] fn tool_001_fires_when_neither_pin_exists_and_names_mise_toml() { let d = TempDir::new().unwrap(); From efa407d0a0718dcfa9d035b6a0be8ff7a60c72f0 Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 12:31:06 +0000 Subject: [PATCH 06/18] fix(finishingbot): a /* inside a // comment or string does not open a block comment strip_block_comments treated any /* as a block opener, so '// ... /*' hid the code below it and the crate read as having no code. Line comments, string, raw-string and char literals are now skipped before an opener is recognised. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/finishingbot/src/analyzers/testing.rs | 129 +++++++++++++++++++-- 1 file changed, 119 insertions(+), 10 deletions(-) diff --git a/bots/finishingbot/src/analyzers/testing.rs b/bots/finishingbot/src/analyzers/testing.rs index bda5f847..e41f4bd8 100644 --- a/bots/finishingbot/src/analyzers/testing.rs +++ b/bots/finishingbot/src/analyzers/testing.rs @@ -429,27 +429,106 @@ mod agda_layout_tests { } /// Remove Rust block comments, which nest (`/* a /* b */ c */`). +/// +/// `/*` only opens a comment in code: not inside a `//` line comment (to the +/// end of the line), a string literal (`"..."` with escapes, `r#"..."#`) or a +/// char literal. Line comments and strings are kept verbatim; inside a block +/// comment only newlines are kept, so line numbers survive. fn strip_block_comments(src: &str) -> String { let b = src.as_bytes(); let mut out = Vec::with_capacity(b.len()); let (mut i, mut depth) = (0, 0usize); while i < b.len() { - if b[i] == b'/' && b.get(i + 1) == Some(&b'*') { - depth += 1; - i += 2; - } else if depth > 0 && b[i] == b'*' && b.get(i + 1) == Some(&b'/') { - depth -= 1; - i += 2; - } else { - if depth == 0 || b[i] == b'\n' { - out.push(b[i]); + if depth > 0 { + if b[i] == b'/' && b.get(i + 1) == Some(&b'*') { + depth += 1; + i += 2; + } else if b[i] == b'*' && b.get(i + 1) == Some(&b'/') { + depth -= 1; + i += 2; + } else { + if b[i] == b'\n' { + out.push(b'\n'); + } + i += 1; + } + continue; + } + match b[i] { + b'/' if b.get(i + 1) == Some(&b'*') => { + depth = 1; + i += 2; + } + b'/' if b.get(i + 1) == Some(&b'/') => { + // Line comment: copy to (not including) the newline. + while i < b.len() && b[i] != b'\n' { + out.push(b[i]); + i += 1; + } + } + b'r' if raw_string_hashes(b, i).is_some() => { + let hashes = raw_string_hashes(b, i).unwrap_or(0); + // r, hashes, opening quote. + let open = 1 + hashes + 1; + out.extend_from_slice(&b[i..i + open]); + i += open; + while i < b.len() { + let closes = b[i] == b'"' && (1..=hashes).all(|k| b.get(i + k) == Some(&b'#')); + out.push(b[i]); + i += 1; + if closes { + out.extend_from_slice(&b[i..i + hashes]); + i += hashes; + break; + } + } + } + b'"' => { + out.push(b'"'); + i += 1; + while i < b.len() { + let c = b[i]; + out.push(c); + i += 1; + if c == b'\\' && i < b.len() { + out.push(b[i]); + i += 1; + } else if c == b'"' { + break; + } + } + } + b'\'' => { + // A char literal ('x', '\n', '"'); a lifetime ('a) has no + // closing quote two or three bytes on and is left alone. + let end = match (b.get(i + 1), b.get(i + 2), b.get(i + 3)) { + (Some(b'\\'), _, Some(b'\'')) => Some(i + 4), + (Some(c), Some(b'\''), _) if *c != b'\\' => Some(i + 3), + _ => None, + }; + let end = end.unwrap_or(i + 1); + out.extend_from_slice(&b[i..end]); + i = end; + } + c => { + out.push(c); + i += 1; } - i += 1; } } String::from_utf8_lossy(&out).into_owned() } +/// If a raw string literal (`r"`, `r#"`, ...) starts at `b[i]` (an `r` that is +/// not the tail of an identifier), the number of `#`s in its delimiter. +fn raw_string_hashes(b: &[u8], i: usize) -> Option { + if i > 0 && (b[i - 1].is_ascii_alphanumeric() || b[i - 1] == b'_') { + return None; + } + let hashes = b[i + 1..].iter().take_while(|&&c| c == b'#').count(); + (b.get(i + 1 + hashes) == Some(&b'"')).then_some(hashes) +} + /// Whether the Rust project at `root` has any code yet. /// /// "No code" is exact: no `.rs` file outside build output (`target/`), @@ -537,6 +616,36 @@ mod no_code_tests { } } + #[test] + fn a_block_comment_opener_inside_a_line_comment_or_string_is_not_a_comment() { + // `/*` after `//` is line-comment text, not the start of block mode. + for body in [ + "// block comments start with /*\npub fn f() {}\n", + "/// docs mention /* here\npub fn f() {}\n", + "pub fn f() {} // trailing /* note\n", + "const S: &str = \"/*\";\n", + "const R: &str = r#\"/* \"#;\n", + "const C: char = '\"';\nconst S: &str = \"/*\";\n", + ] { + let d = rust_repo(Some(body)); + assert!(rust_has_code(d.path()), "{body:?}"); + } + // Real block comments still hide their contents, including a `//` + // inside them, and `//` inside a string does not hide a real opener. + for body in [ + "/* // not a line comment */\n// c\n", + "/* a\n// b\nc */\n", + "// only /* a comment\n// and another\n", + ] { + let d = rust_repo(Some(body)); + assert!(!rust_has_code(d.path()), "{body:?}"); + } + let d = rust_repo(Some("let u = \"http://x\"; /* real\ncomment */\n")); + assert!(rust_has_code(d.path()), "code before the opener"); + let d = rust_repo(Some("/* c */ // x /* y\n")); + assert!(!rust_has_code(d.path())); + } + #[test] fn nested_block_comments_are_stripped() { let d = rust_repo(Some("/* a /* b */ c */\n// x\n")); From 86e5853ec3ad07b7189a541af54d7927f9370e4b Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 12:33:11 +0000 Subject: [PATCH 07/18] docs(glambot): MACH-006 is not auto-fixed; guard it with a test The README listed creating .machine_readable/ among the auto-fixes, but the analyzer deliberately leaves MACH-006 unfixable (an empty directory is untracked by git and a placeholder would be invented content). Drop it from the list and add a test that fix() does not create the directory. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/glambot/README.adoc | 2 +- bots/glambot/src/analyzers/machine.rs | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/bots/glambot/README.adoc b/bots/glambot/README.adoc index 1f4389c1..b9614f5f 100644 --- a/bots/glambot/README.adoc +++ b/bots/glambot/README.adoc @@ -105,7 +105,7 @@ Validates JSON/YAML file syntax, structured data presence, robots.txt, and `.mac | MACH-007 | _Retired_: the STATE/META/ECOSYSTEM `.scm` files are clauses of the repository's `_chora.deed`, so their absence is not reported | n/a |=== -**Auto-fixes:** Creates the `.machine_readable/` directory (MACH-006), adds schema.org JSON-LD template (MACH-003), creates robots.txt (MACH-005). +**Auto-fixes:** Adds schema.org JSON-LD template (MACH-003), creates robots.txt (MACH-005). === Git-SEO Integration (`GitSeoAnalyzer`) diff --git a/bots/glambot/src/analyzers/machine.rs b/bots/glambot/src/analyzers/machine.rs index 71e3fbc2..f964f5ac 100644 --- a/bots/glambot/src/analyzers/machine.rs +++ b/bots/glambot/src/analyzers/machine.rs @@ -495,6 +495,17 @@ mod tests { assert!(ids(empty.path()).contains(&"MACH-006".to_string())); } + #[test] + fn mach_006_is_not_auto_fixable() { + // The README documents the auto-fix list; MACH-006 must stay off it. + let dir = TempDir::new().unwrap(); + let a = MachineAnalyzer::default(); + let findings = a.analyze(dir.path(), &default_config()).unwrap().findings; + assert!(findings.iter().any(|f| f.id == "MACH-006")); + a.fix(dir.path(), &default_config(), &findings).unwrap(); + assert!(!dir.path().join(".machine_readable").exists()); + } + const JSONC: &str = "{\n // a comment\n \"url\": \"http://x//y\", /* inline */\n \"list\": [1, 2,],\n}\n"; From 7c17322dbcee98fbf1ccdc0876926417b23d3e1e Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 13:02:31 +0000 Subject: [PATCH 08/18] fix(finishingbot): REL-008 detects unarmored private keys and JSON-quoted names - value-aware private_key assignment (long hex or base64), excluding paths and prose - optional closing quote between name and separator (JSON form) - test fixtures built from concat! fragments; self-scan test guards release.rs Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/finishingbot/src/analyzers/release.rs | 85 +++++++++++++++++++--- 1 file changed, 76 insertions(+), 9 deletions(-) diff --git a/bots/finishingbot/src/analyzers/release.rs b/bots/finishingbot/src/analyzers/release.rs index 3acdd0f0..84264627 100644 --- a/bots/finishingbot/src/analyzers/release.rs +++ b/bots/finishingbot/src/analyzers/release.rs @@ -503,22 +503,29 @@ static SENSITIVE_PATTERNS: std::sync::LazyLock> = std::sync::LazyLock::new(|| { [ ( - r#"(?i)api[_-]?key\s*[:=]\s*['"]?[a-zA-Z0-9]{20,}"#, + r#"(?i)api[_-]?key["']?\s*[:=]\s*['"]?[a-zA-Z0-9]{20,}"#, "API Key", ), ( - r#"(?i)secret[_-]?key\s*[:=]\s*['"]?[a-zA-Z0-9]{20,}"#, + r#"(?i)secret[_-]?key["']?\s*[:=]\s*['"]?[a-zA-Z0-9]{20,}"#, "Secret Key", ), ( - r#"(?i)aws[_-]?secret[_-]?(access[_-]?)?key\s*[:=]\s*['"]?[A-Za-z0-9/+=]{40}(?:$|[^A-Za-z0-9/+=])"#, + r#"(?i)aws[_-]?secret[_-]?(access[_-]?)?key["']?\s*[:=]\s*['"]?[A-Za-z0-9/+=]{40}(?:$|[^A-Za-z0-9/+=])"#, "AWS Secret Access Key", ), - (r#"(?i)password\s*[:=]\s*['"]?[^\s'"]{8,}"#, "Password"), + (r#"(?i)password["']?\s*[:=]\s*['"]?[^\s'"]{8,}"#, "Password"), ( r"-----BEGIN ([A-Z0-9]+ )*PRIVATE KEY( BLOCK)?-----", "Private Key", ), + // Unarmored key material: the key NAME, then a long hex or base64 value. + // `private_key_path = "/etc/..."` and prose never match: the name must be + // followed directly by the separator, and the value must start alphanumeric. + ( + r#"(?im)private[_-]?key["']?\s*[:=]\s*['"]?(?:[0-9a-f]{64,}|[a-z0-9][a-z0-9+/]{42,}={0,2})(?:["'\s,;}\]]|$)"#, + "Private Key", + ), (r"\b(AKIA|ASIA)[0-9A-Z]{16}\b", "AWS Access Key ID"), (r"ghp_[a-zA-Z0-9]{36}", "GitHub Personal Access Token"), (r"gho_[a-zA-Z0-9]{36}", "GitHub OAuth Token"), @@ -559,12 +566,16 @@ mod sensitive_tests { #[test] fn aws_secret_access_key_value_is_key_material() { + let key = concat!("wJalrXUtnF", "EMI/K7MDEN", "G/bPxRfiCY", "EXAMPLEKEY"); for text in [ - r#"aws_secret_access_key = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY""#, - "AWS_SECRET_ACCESS_KEY=wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", + format!("aws_secret_access_key = \"{key}\""), + format!("AWS_SECRET_ACCESS_KEY={key}"), + // JSON: a closing quote sits between the name and the separator. + format!("{{\"aws_secret_access_key\": \"{key}\"}}"), + format!("\"AWS_SECRET_ACCESS_KEY\":\"{key}\""), ] { assert_eq!( - sensitive_kind(text), + sensitive_kind(&text), Some("AWS Secret Access Key"), "{text}" ); @@ -580,7 +591,11 @@ mod sensitive_tests { fn aws_secret_ending_in_base64_punctuation_is_key_material() { // The 40th character may be `/`, `+` or `=`: `\b` cannot terminate those. for last in ['/', '+', '=', 'A', '9'] { - let key = format!("{}{}", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKE", last); + let key = format!( + "{}{}", + concat!("wJalrXUtnF", "EMI/K7MDEN", "G/bPxRfiCY", "EXAMPLEKE"), + last + ); assert_eq!(key.len(), 40); for text in [ format!("aws_secret_access_key = \"{key}\""), @@ -602,6 +617,58 @@ mod sensitive_tests { assert_eq!(sensitive_kind(&long_slash), None); } + #[test] + fn json_quoted_names_are_key_material() { + assert_eq!( + sensitive_kind(concat!("{\"api_key\": \"abcdefghij", "0123456789xyz\"}")), + Some("API Key") + ); + assert_eq!( + sensitive_kind(concat!("{\"secret_key\": \"abcdefghij", "0123456789xyz\"}")), + Some("Secret Key") + ); + assert_eq!( + sensitive_kind(concat!("{\"password\": \"hunter", "2hunter2\"}")), + Some("Password") + ); + } + + #[test] + fn unarmored_private_key_values_are_key_material() { + let hex = "0123456789abcdef".repeat(4); + let b64 = concat!("QUJDREVGR0", "hJSktMTU5P", "UFFSU1RVVl", "dYWVowMTIz", "NDU2Nzg5"); + for text in [ + format!("private_key = \"{hex}\""), + format!("PRIVATE-KEY: {hex}"), + format!("{{\"private_key\": \"{hex}\"}}"), + format!("private_key = \"{b64}=\""), + format!("privateKey: {b64}"), + ] { + assert_eq!(sensitive_kind(&text), Some("Private Key"), "{text}"); + } + } + + #[test] + fn private_key_paths_prose_and_short_values_are_not_key_material() { + let hex = "0123456789abcdef".repeat(4); + for text in [ + "private_key_path = \"/etc/ssl/private/server.key\"".to_string(), + format!("private_key_file = \"{hex}\""), + "the private key is stored in the vault, never in the repo".to_string(), + "private_key = \"/etc/ssl/private/a/very/long/path/to/the/server/key\"".to_string(), + "private_key = \"deadbeef\"".to_string(), + "private_key = \"$PRIVATE_KEY\"".to_string(), + ] { + assert_eq!(sensitive_kind(&text), None, "{text}"); + } + } + + /// REL-008 must not flag its own source: fixtures are built from fragments. + #[test] + fn this_file_is_not_itself_key_material() { + assert_eq!(sensitive_kind(include_str!("release.rs")), None); + } + #[test] fn pem_headers_are_key_material() { for kind in [ @@ -625,7 +692,7 @@ mod sensitive_tests { let ghp = format!("t = {}{}", "ghp_", "a".repeat(36)); assert_eq!(sensitive_kind(&ghp), Some("GitHub Personal Access Token")); assert_eq!( - sensitive_kind("api_key = \"abcdefghij0123456789xyz\""), + sensitive_kind(concat!("api_key = \"abcdefghij", "0123456789xyz\"")), Some("API Key") ); } From 07d6582fd5e771290340e6ab3aac3a3f917f2320 Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 13:04:52 +0000 Subject: [PATCH 09/18] fix(glambot): JSONC lone commas are invalid; detect JSONC from repo-relative path only - {,} [,] and ,, are rejected; a comma is dropped only after a completed member/element - is_tool_demanded_jsonc takes the repo root and ignores ancestors above it Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/glambot/src/analyzers/machine.rs | 49 ++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 5 deletions(-) diff --git a/bots/glambot/src/analyzers/machine.rs b/bots/glambot/src/analyzers/machine.rs index f964f5ac..9b6ee704 100644 --- a/bots/glambot/src/analyzers/machine.rs +++ b/bots/glambot/src/analyzers/machine.rs @@ -121,7 +121,7 @@ impl MachineAnalyzer { // .devcontainer/*.json and .vscode/*.json are JSONC (comments and // trailing commas) because the tools that read them demand it; the // estate JSON policy carves them out. Parse them as JSONC. - let parsed = if is_tool_demanded_jsonc(entry.path()) { + let parsed = if is_tool_demanded_jsonc(path, entry.path()) { serde_json::from_str::(&strip_jsonc(&content)) } else { serde_json::from_str::(&content) @@ -291,9 +291,14 @@ impl MachineAnalyzer { } /// True for JSON files whose consumer demands JSONC: anything under a -/// `.devcontainer/` or `.vscode/` directory. -fn is_tool_demanded_jsonc(path: &Path) -> bool { - path.parent().is_some_and(|dir| { +/// `.devcontainer/` or `.vscode/` directory of the repository. Only the +/// repo-relative components count: a checkout that itself lives under a +/// `.vscode/` or `.devcontainer/` ancestor does not make every JSON JSONC. +fn is_tool_demanded_jsonc(root: &Path, path: &Path) -> bool { + let Ok(rel) = path.strip_prefix(root) else { + return false; + }; + rel.parent().is_some_and(|dir| { dir.components() .any(|c| c.as_os_str() == ".devcontainer" || c.as_os_str() == ".vscode") }) @@ -370,7 +375,11 @@ fn strip_jsonc(src: &str) -> String { res.push(c); } else if c == ',' { let next = b[j + 1..].iter().find(|x| !x.is_whitespace()); - if !matches!(next, Some('}') | Some(']')) { + // Only a comma after a completed member or element is trailing; + // `{,}`, `[,]` and `,,` stay so the parse fails. + let prev = res.chars().rev().find(|x| !x.is_whitespace()); + let after_value = !matches!(prev, None | Some('{' | '[' | ',' | ':')); + if !(after_value && matches!(next, Some('}') | Some(']'))) { res.push(c); } } else { @@ -551,6 +560,36 @@ mod tests { assert!(!ids(dir.path()).contains(&"MACH-001".to_string())); } + #[test] + fn empty_containers_with_a_lone_comma_are_invalid() { + for body in ["{,}", "[,]", "{ , }", "[1,,]", "{\"a\":1,,}", "{\"a\":,}"] { + let dir = TempDir::new().unwrap(); + let p = dir.path().join(".vscode/settings.json"); + std::fs::create_dir_all(p.parent().unwrap()).unwrap(); + std::fs::write(&p, body).unwrap(); + assert!(ids(dir.path()).contains(&"MACH-001".to_string()), "{body}"); + } + } + + #[test] + fn valid_trailing_commas_still_work() { + for body in ["{\"a\":1,}", "[1,2,]", "{\"a\":[1,],}", "{\"a\":1 , \n}", "[\"x\", /* c */ ]"] { + let v: std::result::Result = serde_json::from_str(&strip_jsonc(body)); + assert!(v.is_ok(), "{body}"); + } + } + + #[test] + fn jsonc_detection_ignores_ancestors_above_the_repo_root() { + let dir = TempDir::new().unwrap(); + let root = dir.path().join(".vscode/checkout"); + std::fs::create_dir_all(&root).unwrap(); + std::fs::write(root.join("config.json"), JSONC).unwrap(); + assert!(ids(&root).contains(&"MACH-001".to_string())); + assert!(!is_tool_demanded_jsonc(&root, &root.join("config.json"))); + assert!(is_tool_demanded_jsonc(&root, &root.join(".vscode/settings.json"))); + } + #[test] fn strip_jsonc_leaves_string_contents_alone() { let v: serde_json::Value = serde_json::from_str(&strip_jsonc(JSONC)).unwrap(); From 4e3cf60402dd658e63c8b6d2ac6db9aab2815695 Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 13:07:08 +0000 Subject: [PATCH 10/18] test(finishingbot): assemble REL-008 fixtures from short fragments No high-entropy run appears whole in the source (GitGuardian). Runtime fixture strings are unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/finishingbot/src/analyzers/release.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bots/finishingbot/src/analyzers/release.rs b/bots/finishingbot/src/analyzers/release.rs index 84264627..920aee7e 100644 --- a/bots/finishingbot/src/analyzers/release.rs +++ b/bots/finishingbot/src/analyzers/release.rs @@ -566,7 +566,7 @@ mod sensitive_tests { #[test] fn aws_secret_access_key_value_is_key_material() { - let key = concat!("wJalrXUtnF", "EMI/K7MDEN", "G/bPxRfiCY", "EXAMPLEKEY"); + let key = concat!("wJalrXUt", "nFEMI/K7", "MDENG/bP", "xRfiCYEX", "AMPLEKEY"); for text in [ format!("aws_secret_access_key = \"{key}\""), format!("AWS_SECRET_ACCESS_KEY={key}"), @@ -593,7 +593,7 @@ mod sensitive_tests { for last in ['/', '+', '=', 'A', '9'] { let key = format!( "{}{}", - concat!("wJalrXUtnF", "EMI/K7MDEN", "G/bPxRfiCY", "EXAMPLEKE"), + concat!("wJalrXUt", "nFEMI/K7", "MDENG/bP", "xRfiCYEX", "AMPLEKE"), last ); assert_eq!(key.len(), 40); @@ -636,7 +636,7 @@ mod sensitive_tests { #[test] fn unarmored_private_key_values_are_key_material() { let hex = "0123456789abcdef".repeat(4); - let b64 = concat!("QUJDREVGR0", "hJSktMTU5P", "UFFSU1RVVl", "dYWVowMTIz", "NDU2Nzg5"); + let b64 = concat!("QUJDREVG", "R0hJSktM", "TU5PUFFS", "U1RVVldY", "WVowMTIz", "NDU2Nzg5"); for text in [ format!("private_key = \"{hex}\""), format!("PRIVATE-KEY: {hex}"), From 8339d211dc1b3261dcce18dbfc2ba2e7c60bf342 Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 13:08:32 +0000 Subject: [PATCH 11/18] fix(finishingbot): workflow fallback only for workflow-qualified references A bare .yml name resolved against .github/workflows/ and gave false passes. The fallback now needs a workflows/ prefix or workflow prose on the line, and the basename search skips .github/workflows otherwise. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/finishingbot/src/analyzers/claims.rs | 16 +++++- bots/finishingbot/src/analyzers/references.rs | 53 +++++++++++++++---- 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/bots/finishingbot/src/analyzers/claims.rs b/bots/finishingbot/src/analyzers/claims.rs index efbc7010..6f49dfdc 100644 --- a/bots/finishingbot/src/analyzers/claims.rs +++ b/bots/finishingbot/src/analyzers/claims.rs @@ -23,6 +23,13 @@ static PATH_RE: LazyLock = LazyLock::new(|| { .expect("Cargo workspace member-path regex is a valid constant pattern") }); +/// The line of `content` containing byte offset `at` (prose context for a reference). +fn line_of(content: &str, at: usize) -> &str { + let start = content[..at].rfind('\n').map_or(0, |i| i + 1); + let end = content[at..].find('\n').map_or(content.len(), |i| at + i); + &content[start..end] +} + /// Claim verification analyzer pub struct ClaimsAnalyzer; @@ -295,7 +302,14 @@ impl ClaimsAnalyzer { for pattern in &file_ref_patterns { for caps in pattern.captures_iter(content) { let file_ref = &caps[1]; - if resolve_local_reference(repo_root, doc_path, file_ref).is_none() { + if resolve_local_reference( + repo_root, + doc_path, + file_ref, + line_of(content, caps.get(0).map_or(0, |m| m.start())), + ) + .is_none() + { debug!(doc = %doc_path.display(), reference = file_ref, "Broken file reference"); result.add( Finding::new( diff --git a/bots/finishingbot/src/analyzers/references.rs b/bots/finishingbot/src/analyzers/references.rs index 525b4018..32088178 100644 --- a/bots/finishingbot/src/analyzers/references.rs +++ b/bots/finishingbot/src/analyzers/references.rs @@ -23,15 +23,15 @@ fn local_file(root: &Path, candidate: &Path) -> Option { (resolved.starts_with(root) && resolved.is_file()).then_some(resolved) } -/// `Some(file name)` when `path` is a bare `*.yml`/`*.yaml` name or -/// `workflows/`, the two spellings prose uses for a GitHub workflow. -fn workflow_file_name(path: &Path) -> Option<&std::ffi::OsStr> { +/// `Some(file name)` when `path` is a `workflows/.yml` reference, or a +/// bare `*.yml`/`*.yaml` name written in workflow prose (`in_workflow_context`). +/// A bare YAML name with no workflow context gets no `.github/workflows/` +/// fallback: it could be any config file, and must resolve normally. +fn workflow_file_name(path: &Path, in_workflow_context: bool) -> Option<&std::ffi::OsStr> { let name = path.file_name()?; - let is_yaml = path - .extension() - .is_some_and(|e| e == "yml" || e == "yaml"); + let is_yaml = path.extension().is_some_and(|e| e == "yml" || e == "yaml"); let parts: Vec<_> = path.components().collect(); - let qualified_ok = parts.len() == 1 + let qualified_ok = (parts.len() == 1 && in_workflow_context) || (parts.len() == 2 && parts[0].as_os_str() == "workflows"); (is_yaml && qualified_ok).then_some(name) } @@ -39,10 +39,14 @@ fn workflow_file_name(path: &Path) -> Option<&std::ffi::OsStr> { /// Prose can name a document-relative path, a repository-relative path, or an /// unanchored module basename. The last form is accepted only when unique. /// This resolves a file's identity; it does not verify its contents or claims. +/// +/// `context` is the prose line the reference sits on; it only decides whether a +/// bare YAML name is a workflow reference. pub(super) fn resolve_local_reference( repo: &Path, document: &Path, reference: &str, + context: &str, ) -> Option { let root = repo.canonicalize().ok()?; let path = Path::new(reference); @@ -61,7 +65,8 @@ pub(super) fn resolve_local_reference( // `workflows/estate-rules.yml`) but lives under .github/workflows/. Look // there first, so the reference resolves even when the same basename also // exists elsewhere (which would make the basename search ambiguous). - if let Some(name) = workflow_file_name(path) { + let workflow_prose = context.to_ascii_lowercase().contains("workflow"); + if let Some(name) = workflow_file_name(path, workflow_prose) { if let Some(found) = local_file(&root, &root.join(".github/workflows").join(name)) { return Some(found); } @@ -72,6 +77,10 @@ pub(super) fn resolve_local_reference( return None; } let name = path.file_name()?; + // A bare YAML name that is not workflow prose is not a workflow reference, + // so the basename search must not be satisfied by a workflow file either. + let skip_workflows = workflow_file_name(path, true).is_some() && !workflow_prose; + let workflows_dir = root.join(".github/workflows"); let mut pending = vec![root.clone()]; let mut found = None; while let Some(directory) = pending.pop() { @@ -82,6 +91,7 @@ pub(super) fn resolve_local_reference( if !GENERATED_DIRS .iter() .any(|excluded| entry.file_name() == *excluded) + && !(skip_workflows && entry.path() == workflows_dir) { pending.push(entry.path()); } @@ -121,7 +131,10 @@ mod tests { path } fn resolves(&self, document: &str, reference: &str) -> bool { - resolve_local_reference(&self.0, &self.0.join(document), reference).is_some() + self.resolves_in(document, reference, "") + } + fn resolves_in(&self, document: &str, reference: &str, context: &str) -> bool { + resolve_local_reference(&self.0, &self.0.join(document), reference, context).is_some() } } impl Drop for Scratch { @@ -153,12 +166,30 @@ mod tests { fn workflow_names_resolve_under_dot_github_workflows() { let s = Scratch::new(); s.file(".github/workflows/estate-rules.yml"); - assert!(s.resolves("docs/note.adoc", "estate-rules.yml")); + let ctx = "the estate-rules.yml workflow runs the gate"; + assert!(s.resolves_in("docs/note.adoc", "estate-rules.yml", ctx)); assert!(s.resolves("docs/note.adoc", "workflows/estate-rules.yml")); assert!(s.resolves("docs/note.adoc", ".github/workflows/estate-rules.yml")); // Ambiguous basenames elsewhere do not defeat the workflow directory. s.file("tools/estate-rules.yml"); - assert!(s.resolves("docs/note.adoc", "estate-rules.yml")); + assert!(s.resolves_in("docs/note.adoc", "estate-rules.yml", ctx)); + } + + #[test] + fn a_bare_yml_name_without_workflow_context_does_not_fall_back_to_workflows() { + let s = Scratch::new(); + s.file(".github/workflows/config.yml"); + // Not a workflow reference: a missing config file is not satisfied by an + // unrelated workflow of the same name. + assert!(!s.resolves("docs/note.adoc", "config.yml")); + assert!(!s.resolves_in( + "docs/note.adoc", + "config.yml", + "see config.yml for settings" + )); + // Normal resolution still works for a real, unique file. + s.file("conf/real.yml"); + assert!(s.resolves("docs/note.adoc", "real.yml")); } #[test] From f1bfab436b21a83cc3ab31352c9069ae68fa4045 Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 13:09:02 +0000 Subject: [PATCH 12/18] fix(finishingbot): TEST-RU-003 requires a recognisable fuzz harness A placeholder under fuzz/ or tests/fuzz/ no longer suppresses the finding: it needs a Cargo.toml with [[bin]] targets or fuzz_targets/*.rs using fuzz_target!. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/finishingbot/src/analyzers/testing.rs | 54 +++++++++++++++++++++- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/bots/finishingbot/src/analyzers/testing.rs b/bots/finishingbot/src/analyzers/testing.rs index e41f4bd8..f1a3a27c 100644 --- a/bots/finishingbot/src/analyzers/testing.rs +++ b/bots/finishingbot/src/analyzers/testing.rs @@ -188,13 +188,16 @@ impl TestingAnalyzer { let tests_fuzz_dir = path.join("tests/fuzz"); let clusterfuzz_dir = path.join(".clusterfuzzlite"); - if !fuzz_dir.exists() && !tests_fuzz_dir.exists() && !clusterfuzz_dir.exists() { + if !has_fuzz_harness(&fuzz_dir) + && !has_fuzz_harness(&tests_fuzz_dir) + && !clusterfuzz_dir.exists() + { result.add( Finding::new( "TEST-RU-003", "Missing fuzzing infrastructure", Severity::Error, - "Rust project missing fuzz/, tests/fuzz/ or .clusterfuzzlite/ for fuzzing.", + "Rust project has no fuzz harness: expected fuzz/ or tests/fuzz/ with a Cargo.toml declaring [[bin]] targets or fuzz_targets/*.rs using fuzz_target!, or .clusterfuzzlite/.", ) .with_suggestion("Add ClusterFuzzLite configuration for automated fuzzing"), ); @@ -561,6 +564,26 @@ pub(crate) fn rust_has_code(root: &Path) -> bool { }) } +/// True when `dir` (a `fuzz/` or `tests/fuzz/` directory) holds a recognisable +/// harness: a `Cargo.toml` declaring `[[bin]]` targets (cargo-fuzz layout), or a +/// `fuzz_targets/*.rs` file that invokes `fuzz_target!`. An empty directory or a +/// placeholder file is not fuzzing infrastructure. +fn has_fuzz_harness(dir: &Path) -> bool { + if let Ok(manifest) = std::fs::read_to_string(dir.join("Cargo.toml")) { + if manifest.contains("[[bin]]") { + return true; + } + } + let Ok(entries) = std::fs::read_dir(dir.join("fuzz_targets")) else { + return false; + }; + entries.filter_map(|e| e.ok()).any(|e| { + let p = e.path(); + p.extension().is_some_and(|x| x == "rs") + && std::fs::read_to_string(&p).is_ok_and(|s| s.contains("fuzz_target!")) + }) +} + #[cfg(test)] mod no_code_tests { use super::*; @@ -683,7 +706,34 @@ mod no_code_tests { fn fuzz_under_tests_satisfies_ru_003() { let d = rust_repo(Some("pub fn f() {}\n")); assert!(ids(d.path()).contains(&"TEST-RU-003".to_string())); + // A placeholder is not a harness. + write(d.path(), "tests/fuzz/README", "todo\n"); write(d.path(), "tests/fuzz/fuzz_targets/t.rs", "// target\n"); + assert!(ids(d.path()).contains(&"TEST-RU-003".to_string())); + write( + d.path(), + "tests/fuzz/fuzz_targets/t.rs", + "#![no_main]\nlibfuzzer_sys::fuzz_target!(|data: &[u8]| { let _ = data; });\n", + ); + assert!(!ids(d.path()).contains(&"TEST-RU-003".to_string())); + } + + #[test] + fn fuzz_cargo_manifest_with_bin_targets_satisfies_ru_003() { + let d = rust_repo(Some("pub fn f() {}\n")); + write(d.path(), "fuzz/.gitkeep", ""); + assert!(ids(d.path()).contains(&"TEST-RU-003".to_string())); + write( + d.path(), + "fuzz/Cargo.toml", + "[package]\nname = \"x-fuzz\"\n", + ); + assert!(ids(d.path()).contains(&"TEST-RU-003".to_string())); + write( + d.path(), + "fuzz/Cargo.toml", + "[package]\nname = \"x-fuzz\"\n\n[[bin]]\nname = \"t\"\npath = \"fuzz_targets/t.rs\"\n", + ); assert!(!ids(d.path()).contains(&"TEST-RU-003".to_string())); } } From fb96576765faf3d692face5ffb7fcba72a4d3dd9 Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 13:10:48 +0000 Subject: [PATCH 13/18] fix(glambot): report an unavailable git-seo as skipped, not as a clean scan AnalysisResult gains an optional skipped reason (serialized only when set). The git-seo analyzer and the audit error path set it, and fleet-report-back.sh copies it into the ran envelope's reason and the driver log line. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- .../src/analyzers/git_seo_integration.rs | 18 ++++++++++++++++ bots/glambot/src/analyzers/mod.rs | 5 +++++ bots/glambot/src/main.rs | 8 +++++-- docs/REPORT-BACK.adoc | 6 +++++- scripts/fleet-report-back.sh | 20 +++++++++++++----- tests/fleet-report-back-test.sh | 21 +++++++++++++++++++ 6 files changed, 70 insertions(+), 8 deletions(-) diff --git a/bots/glambot/src/analyzers/git_seo_integration.rs b/bots/glambot/src/analyzers/git_seo_integration.rs index ed66117c..897386e1 100644 --- a/bots/glambot/src/analyzers/git_seo_integration.rs +++ b/bots/glambot/src/analyzers/git_seo_integration.rs @@ -38,6 +38,7 @@ impl GitSeoAnalyzer { // logged loudly instead; install hint: https://github.com/hyperpolymath/git-seo if !git_seo_installed { warn!("git-seo is not installed; the Git-SEO analyzer did not run (no findings reported, not a clean result)"); + result.skipped = Some("git-seo is not installed".to_string()); result.duration_ms = start.elapsed().as_millis() as u64; return Ok(result); } @@ -366,6 +367,23 @@ mod tests { "tool-availability advice is not a repo finding, got: {:?}", result.findings.iter().map(|f| f.id.as_str()).collect::>() ); + assert_eq!( + result.skipped.as_deref(), + Some("git-seo is not installed"), + "an unavailable git-seo is reported as skipped, not as a clean scan" + ); + let json = serde_json::to_value(&result).unwrap(); + assert_eq!(json["skipped"], "git-seo is not installed"); + } + + #[test] + fn a_run_is_not_a_skip() { + let dir = git_repo_without_remote(); + let result = GitSeoAnalyzer + .analyze_with(dir.path(), &default_config(), true) + .unwrap(); + assert!(result.skipped.is_none()); + assert!(serde_json::to_value(&result).unwrap().get("skipped").is_none()); } #[test] diff --git a/bots/glambot/src/analyzers/mod.rs b/bots/glambot/src/analyzers/mod.rs index 718d606a..b9d1f91e 100644 --- a/bots/glambot/src/analyzers/mod.rs +++ b/bots/glambot/src/analyzers/mod.rs @@ -107,6 +107,11 @@ pub struct AnalysisResult { pub files_checked: usize, /// Duration in milliseconds pub duration_ms: u64, + /// Why the analyzer did not run (tool missing or failed). `None` means it ran. + /// A skipped analyzer has no findings, which is NOT a clean scan: consumers + /// (scripts/fleet-report-back.sh) must read this before trusting an empty list. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub skipped: Option, } impl AnalysisResult { diff --git a/bots/glambot/src/main.rs b/bots/glambot/src/main.rs index 50b71e1b..cb9aa440 100644 --- a/bots/glambot/src/main.rs +++ b/bots/glambot/src/main.rs @@ -247,8 +247,12 @@ fn handle_audit(path: &PathBuf, config: &config::Config, strict: bool, format: & Ok(r) => r, Err(e) => { eprintln!("Git-SEO analysis error: {}", e); - // Don't fail if git-seo is unavailable, just skip - Default::default() + // Don't fail the audit, but say it was skipped: an empty result + // here must not read as a clean scan. + analyzers::AnalysisResult { + skipped: Some(format!("git-seo analysis error: {e}")), + ..Default::default() + } } }; } diff --git a/docs/REPORT-BACK.adoc b/docs/REPORT-BACK.adoc index 450fde15..e9c7ebf5 100644 --- a/docs/REPORT-BACK.adoc +++ b/docs/REPORT-BACK.adoc @@ -126,7 +126,11 @@ grading can match them against the diff. | glambot | `glambot --path --format json audit` | Visual, accessibility, SEO, machine-readability and git-SEO analyzers, - flattened. + flattened. An analyzer that could not run (git-seo not installed) + reports a `skipped` reason in its own object; the driver keeps `status` `ran` + but copies the reason into the envelope `reason`, so an empty result is not + mistaken for a clean scan. A first-class `skipped` field in the envelope + would need a change to the verdict-grid spec in standards. | accessibilitybot | `accessibilitybot fleet ` diff --git a/scripts/fleet-report-back.sh b/scripts/fleet-report-back.sh index 60290a26..d4f21d8c 100755 --- a/scripts/fleet-report-back.sh +++ b/scripts/fleet-report-back.sh @@ -505,13 +505,18 @@ cleanup_adapter() { rm -f "$ADAPTER_OUT" "$ADAPTER_ERR"; } # status 124 always produces a timeout error; invalid output produces an error # with a stderr diagnostic. Bind $repo for relativize_filter, write the member's # envelope, and remove the capture files. Envelope I/O failures are not caught. -translate_or_error() { # member jq-program - local m="$1" prog="$2" findings +translate_or_error() { # member jq-program [ran-note-jq-program] + local m="$1" prog="$2" findings note="" if [ "$ADAPTER_RC" -eq 124 ]; then write_envelope "$m" error "adapter exceeded the ${timeout_s}s timeout" '[]' elif findings=$(jq -c --arg repo "$repo" "$(relativize_filter) $prog" "$ADAPTER_OUT" 2> "$ADAPTER_ERR.jq") \ && [ -n "$findings" ] && jq -e 'type == "array"' <<<"$findings" >/dev/null 2>&1; then - write_envelope "$m" ran "" "$findings" + # An analyzer that did not run has no findings, which is not a clean scan: + # the optional program reads the adapter output for such skips and the note + # becomes the envelope reason (ran + reason = ran, with caveats). + if [ -n "${3:-}" ]; then note=$(jq -r "$3" "$ADAPTER_OUT" 2>/dev/null) || note=""; fi + [ -z "$note" ] || echo "fleet-report-back: $m: $note" >&2 + write_envelope "$m" ran "$note" "$findings" elif grep -q 'unexpected output shape' "$ADAPTER_ERR.jq" 2>/dev/null; then # The shape guard fired: valid JSON, but not what this adapter documents. write_envelope "$m" error "adapter output had an $(grep -o 'unexpected output shape: [^(]*' "$ADAPTER_ERR.jq" | head -n1 | sed 's/ *$//')" '[]' @@ -603,7 +608,12 @@ run_member() { translate_or_error "$m" "$SHAPE_ANALYZERS [ .[] | .findings[] | $FINDING ]" ;; glambot) run_adapter "$bin" --path "$repo" --format json audit - translate_or_error "$m" "$SHAPE_ANALYZERS [ .[] | .findings[] | $FINDING ]" ;; + # An analyzer that could not run (git-seo missing) reports `skipped`; say so + # in the envelope reason rather than leaving an empty list that reads clean. + translate_or_error "$m" "$SHAPE_ANALYZERS [ .[] | .findings[] | $FINDING ]" \ + '[ (if type == "object" then to_entries[] else {key: "analyzer", value: .[]} end) + | select((.value.skipped // null) | type == "string") + | "\(.key) analyzer skipped: \(.value.skipped); its empty result is not a clean scan" ] | join("; ")' ;; accessibilitybot) run_adapter "$bin" fleet "$repo" translate_or_error "$m" "$SHAPE_OBJ_FINDINGS [ .findings[] | $FINDING ]" ;; @@ -617,7 +627,7 @@ if [ "${#members[@]}" -eq 0 ]; then fi for m in "${members[@]+"${members[@]}"}"; do run_member "$m" - echo "fleet-report-back: $m -> $(jq -r '.status + (if .status == "ran" then " (\(.findings | length) finding(s))" else ": " + (.reason // "") end)' "$out/$m.json")" + echo "fleet-report-back: $m -> $(jq -r '.status + (if .status == "ran" then " (\(.findings | length) finding(s))" + (if .reason then " [" + .reason + "]" else "" end) else ": " + (.reason // "") end)' "$out/$m.json")" done # ── summary ────────────────────────────────────────────────────────────────── diff --git a/tests/fleet-report-back-test.sh b/tests/fleet-report-back-test.sh index 6c035fb1..aff601c5 100755 --- a/tests/fleet-report-back-test.sh +++ b/tests/fleet-report-back-test.sh @@ -169,6 +169,27 @@ check "a missing binary is error" error "$(field "$TMP/o4/glambot.json" .status) check "...naming the binary and the crate" yes \ "$(field "$TMP/o4/glambot.json" .reason | grep -q "glambot.*bots/glambot" && echo yes || echo no)" + +# A skipped analyzer (git-seo not installed) has no findings, which is not a +# clean scan: the envelope still says `ran`, but its reason says what did not. +stub glambot '[ "$1 $3 $4 $5" = "--path --format json audit" ] || { echo "bad args: $*" >&2; exit 9; } +cat <&2; exit 9; } +cat <&2; exit 9; } +cat < Date: Fri, 13 Nov 2026 13:12:55 +0000 Subject: [PATCH 14/18] fix(finishingbot): read .hypatia-baseline.json, not .hypatia-ignore, for PH exemptions The estate's documented exemption mechanism is the per-repo .hypatia-baseline.json (EXEMPTION-MECHANISMS.adoc Layer 2), which rejects .hypatia-ignore. ignore.rs is replaced by baseline.rs: exact severity/rule_module/type match, file or file_pattern, expires_at honoured, severity_override not applied. The placeholder test helper now uses normalise_rel(), and the well-tested fixture uses a real fuzz harness. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/finishingbot/README.adoc | 2 +- .../finishingbot/src/analyzers/placeholder.rs | 111 ++++---- bots/finishingbot/src/baseline.rs | 250 ++++++++++++++++++ bots/finishingbot/src/ignore.rs | 164 ------------ bots/finishingbot/src/lib.rs | 2 +- bots/finishingbot/src/main.rs | 2 +- bots/finishingbot/tests/analyzer_tests.rs | 14 +- 7 files changed, 330 insertions(+), 215 deletions(-) create mode 100644 bots/finishingbot/src/baseline.rs delete mode 100644 bots/finishingbot/src/ignore.rs diff --git a/bots/finishingbot/README.adoc b/bots/finishingbot/README.adoc index b59da872..714d73e3 100644 --- a/bots/finishingbot/README.adoc +++ b/bots/finishingbot/README.adoc @@ -182,7 +182,7 @@ The analyzers accept the estate's canonical locations, not only one spelling: * SCM-001 is retired: STATE, META, ECOSYSTEM, PLAYBOOK, AGENTIC and NEUROSYM are clauses of `_chora.deed`, not files. * REL-008 matches key material (PEM headers, AWS key ids, vendor token shapes, long key literals), not the words "private key". -A repository can exempt a path from a rule in its `.hypatia-ignore` (the file the RSR template ships), using this bot's module name: `finishing_bot/PH:build/just/` exempts the placeholder family under `build/just/`; `finishing_bot/*:` exempts every finishing-bot rule. Entries for other modules are not read. Currently only the placeholder analyzer (PH-nnn, hence PH-100) honours it. +A repository can acknowledge a finding in its `.hypatia-baseline.json`, the estate's documented exemption mechanism (`docs/upstream-proposals/standards--hypatia-baseline-consumer/docs/EXEMPTION-MECHANISMS.adoc`, "Layer 2"). `.hypatia-ignore` is not read: that document rejects it. An entry uses this bot's `rule_module`, `finishing_bot`, and the finding id as `type`, with the estate severity word (error = `high`, warning = `medium`, info = `info`, suggestion = `low`) and exactly one of `file` or `file_pattern`: `{"severity": "medium", "rule_module": "finishing_bot", "type": "PH-001", "file_pattern": "build/just/**", "note": "scanner names the tokens"}`. Matching is exact on severity, module and type, so a rule family needs one entry per id. Entries past `expires_at`, or carrying `severity_override` (this bot does not downgrade), acknowledge nothing; other modules' entries (`cicd_rules`, ...) are Hypatia's and are not read. Currently only the placeholder analyzer (PH-nnn, hence PH-100) honours it. See `src/baseline.rs`. === Security Analysis diff --git a/bots/finishingbot/src/analyzers/placeholder.rs b/bots/finishingbot/src/analyzers/placeholder.rs index 8286fb8d..644663e9 100644 --- a/bots/finishingbot/src/analyzers/placeholder.rs +++ b/bots/finishingbot/src/analyzers/placeholder.rs @@ -3,9 +3,9 @@ //! Placeholder detection analyzer use super::{AnalysisResult, Analyzer, Finding, Severity}; +use crate::baseline::Baseline; use crate::config::{Config, PlaceholderAction}; use crate::error::Result; -use crate::ignore::IgnoreFile; use regex::{Regex, RegexSet}; use std::path::Path; use tracing::debug; @@ -40,8 +40,9 @@ impl Analyzer for PlaceholderAnalyzer { // Add custom regex patterns patterns.extend(config.placeholders.custom_patterns.clone()); - // Exemptions from the repository's own .hypatia-ignore (finishing_bot/PH:...). - let ignore = IgnoreFile::load(path); + // Findings the repository has acknowledged in its .hypatia-baseline.json + // (rule_module "finishing_bot", type "PH-nnn"). + let baseline = Baseline::load(path); let pattern_set = RegexSet::new(&patterns)?; let patterns_with_context: Vec = patterns @@ -80,7 +81,7 @@ impl Analyzer for PlaceholderAnalyzer { &pattern_set, &patterns_with_context, config, - &ignore, + &baseline, path, &mut result, ); @@ -156,7 +157,7 @@ impl PlaceholderAnalyzer { pattern_set: &RegexSet, patterns: &[Regex], config: &Config, - ignore: &IgnoreFile, + baseline: &Baseline, root: &Path, result: &mut AnalysisResult, ) { @@ -173,8 +174,13 @@ impl PlaceholderAnalyzer { .unwrap_or("custom"); let rule_id = format!("PH-{:03}", match_idx + 1); let rel = normalise_rel(path.strip_prefix(root).unwrap_or(path)); - if ignore.suppresses(&rule_id, &rel) { - debug!(file = %rel, rule = %rule_id, "Exempted by .hypatia-ignore"); + let severity = match config.placeholders.action { + PlaceholderAction::Remove => Severity::Error, + PlaceholderAction::Comment => Severity::Warning, + PlaceholderAction::Flag => Severity::Warning, + }; + if baseline.acknowledges(&rule_id, severity, &rel) { + debug!(file = %rel, rule = %rule_id, "Acknowledged in .hypatia-baseline.json"); continue; } @@ -185,12 +191,6 @@ impl PlaceholderAnalyzer { "Found placeholder" ); - let severity = match config.placeholders.action { - PlaceholderAction::Remove => Severity::Error, - PlaceholderAction::Comment => Severity::Warning, - PlaceholderAction::Flag => Severity::Warning, - }; - result.add( Finding::new( &format!("PH-{:03}", match_idx + 1), @@ -337,22 +337,14 @@ impl PlaceholderAnalyzer { } } -/// Repo-relative path with `/` separators, so `.hypatia-ignore` fragments -/// (always written with `/`) match on Windows too. +/// Repo-relative path with `/` separators, so `.hypatia-baseline.json` paths +/// and globs (always written with `/`) match on Windows too. fn normalise_rel(p: &Path) -> String { p.to_string_lossy().replace('\\', "/") } #[cfg(test)] -mod ignore_tests { - #[test] - fn backslash_paths_are_normalised_before_matching() { - let rel = normalise_rel(Path::new(r"build\just\scan.just")); - assert_eq!(rel, "build/just/scan.just"); - let f = crate::ignore::IgnoreFile::parse("finishing_bot/PH:build/just/\n"); - assert!(f.suppresses("PH-001", &rel)); - } - +mod baseline_tests { use super::*; use tempfile::TempDir; @@ -363,16 +355,17 @@ mod ignore_tests { } /// A template-shaped repo: its own scanners legitimately name the tokens. - fn repo(ignore: Option<&str>) -> TempDir { + fn repo(baseline: Option<&str>) -> TempDir { let d = TempDir::new().unwrap(); write(d.path(), "build/just/scan.just", "grep -E 'TODO|FIXME' .\n"); write(d.path(), "src/main.rs", "// TODO: real placeholder\n"); - if let Some(text) = ignore { - write(d.path(), ".hypatia-ignore", text); + if let Some(text) = baseline { + write(d.path(), ".hypatia-baseline.json", text); } d } + /// Repo-relative, `/`-separated paths of every PH-0nn finding. fn files(d: &TempDir) -> Vec { PlaceholderAnalyzer .analyze(d.path(), &Config::default()) @@ -380,19 +373,30 @@ mod ignore_tests { .findings .into_iter() .filter(|f| f.id.starts_with("PH-0")) - .map(|f| { - f.file - .unwrap() - .strip_prefix(d.path()) - .unwrap() - .to_string_lossy() - .into_owned() - }) + .map(|f| normalise_rel(f.file.unwrap().strip_prefix(d.path()).unwrap())) .collect() } + /// The default config flags placeholders as warnings (estate word `medium`); + /// PH-001 is TODO and PH-002 is FIXME. + const BASELINE_JUST: &str = r#"[ + {"severity":"medium","rule_module":"finishing_bot","type":"PH-001","file_pattern":"build/just/**","note":"scanner names the tokens"}, + {"severity":"medium","rule_module":"finishing_bot","type":"PH-002","file_pattern":"build/just/**","note":"scanner names the tokens"} + ]"#; + #[test] - fn without_an_ignore_file_every_placeholder_is_reported() { + fn backslash_paths_are_normalised_before_matching() { + let rel = normalise_rel(Path::new(r"build\just\scan.just")); + assert_eq!(rel, "build/just/scan.just"); + let b = Baseline::parse( + BASELINE_JUST, + chrono::NaiveDate::from_ymd_opt(2026, 11, 13).unwrap(), + ); + assert!(b.acknowledges("PH-001", Severity::Warning, &rel)); + } + + #[test] + fn without_a_baseline_every_placeholder_is_reported() { let d = repo(None); let got = files(&d); assert!(got.contains(&"build/just/scan.just".to_string()), "{got:?}"); @@ -400,8 +404,8 @@ mod ignore_tests { } #[test] - fn hypatia_ignore_exempts_the_named_path_only() { - let d = repo(Some("finishing_bot/PH:build/just/\n")); + fn hypatia_baseline_acknowledges_the_named_path_only() { + let d = repo(Some(BASELINE_JUST)); let got = files(&d); assert!( !got.contains(&"build/just/scan.just".to_string()), @@ -412,13 +416,32 @@ mod ignore_tests { } #[test] - fn another_modules_entry_exempts_nothing_here() { - let d = repo(Some("structural_drift/SD022:build/just/\n")); + fn hypatia_ignore_is_not_read() { + let d = repo(None); + write( + d.path(), + ".hypatia-ignore", + "finishing_bot/PH:build/just/\n", + ); assert!(files(&d).contains(&"build/just/scan.just".to_string())); } #[test] - fn exempted_scanner_files_no_longer_inflate_ph_100() { + fn another_modules_or_severitys_entry_acknowledges_nothing_here() { + let hypatia = r#"[{"severity":"high","rule_module":"cicd_rules","type":"PH-001","file_pattern":"build/just/**"}]"#; + assert!(files(&repo(Some(hypatia))).contains(&"build/just/scan.just".to_string())); + let wrong_severity = BASELINE_JUST.replace("medium", "high"); + assert!(files(&repo(Some(&wrong_severity))).contains(&"build/just/scan.just".to_string())); + } + + #[test] + fn an_expired_baseline_entry_lets_the_finding_reappear() { + let expired = BASELINE_JUST.replace("\"note\"", "\"expires_at\":\"2020-01-01\",\"note\""); + assert!(files(&repo(Some(&expired))).contains(&"build/just/scan.just".to_string())); + } + + #[test] + fn acknowledged_scanner_files_no_longer_inflate_ph_100() { let mut cfg = Config::default(); cfg.placeholders.max_allowed = 2; // 2 hits in the scanner file, none elsewhere let d = TempDir::new().unwrap(); @@ -434,11 +457,7 @@ mod ignore_tests { over.findings.iter().any(|f| f.id == "PH-100"), "over the limit" ); - write( - d.path(), - ".hypatia-ignore", - "finishing_bot/PH:build/just/\n", - ); + write(d.path(), ".hypatia-baseline.json", BASELINE_JUST); let ok = PlaceholderAnalyzer.analyze(d.path(), &cfg).unwrap(); assert!(ok.findings.is_empty(), "{:?}", ok.findings); } diff --git a/bots/finishingbot/src/baseline.rs b/bots/finishingbot/src/baseline.rs new file mode 100644 index 00000000..85997dfe --- /dev/null +++ b/bots/finishingbot/src/baseline.rs @@ -0,0 +1,250 @@ +// SPDX-License-Identifier: MPL-2.0 +// SPDX-FileCopyrightText: 2026 Jonathan D.A. Jewell +//! Acknowledged findings from the repository's `.hypatia-baseline.json`. +//! +//! The estate's documented exemption mechanism is the per-repo +//! `.hypatia-baseline.json` (`docs/upstream-proposals/standards--hypatia-baseline-consumer/`, +//! `docs/EXEMPTION-MECHANISMS.adoc`, "Layer 2"). `.hypatia-ignore` is an +//! anti-pattern there ("never read by anything"), so finishing-bot does not read +//! it. Format (`docs/HYPATIA-BASELINE-FORMAT.adoc`): a JSON array of entries. +//! +//! ```json +//! { "severity": "high", "rule_module": "finishing_bot", "type": "PH-001", +//! "file_pattern": "build/just/**", "note": "scanner names the tokens", +//! "tracking_issue": "owner/repo#1", "expires_at": "2027-06-30" } +//! ``` +//! +//! An entry acknowledges a finding iff `severity`, `rule_module` and `type` match +//! EXACTLY and `file` equals (or `file_pattern` globs) the finding's repo-relative +//! path. This bot's `rule_module` is `finishing_bot`; its `type` is the finding id +//! (`PH-001`); entries of other modules (`cicd_rules`, ...) belong to Hypatia and +//! are not read here. `severity` uses the estate words, mapped from this bot's own +//! as the report-back driver does: error = `high`, warning = `medium`, +//! info = `info`, suggestion = `low`. +//! +//! Deliberate limits, so that nothing is silently half-honoured: +//! - matching is exact, so a rule family needs one entry per finding id (and per +//! severity the id can take); there is no `PH` prefix or `*` wildcard; +//! - an entry past its `expires_at` (or with an unparseable one) acknowledges +//! nothing, which makes the finding reappear; +//! - an entry with `severity_override` is not applied: this bot does not downgrade +//! findings, so the finding stays visible at its own severity; +//! - a baseline that is not a JSON array, or an entry that is malformed, exempts +//! nothing and is logged. + +use crate::analyzers::Severity; +use chrono::NaiveDate; +use serde_json::Value; +use std::path::Path; +use tracing::warn; + +/// The `rule_module` under which finishing-bot's rules are addressed. +pub const MODULE: &str = "finishing_bot"; + +/// The baseline file name, at the repository root. +pub const FILE_NAME: &str = ".hypatia-baseline.json"; + +#[derive(Debug, Default, Clone)] +pub struct Baseline { + entries: Vec, +} + +#[derive(Debug, Clone)] +struct Entry { + severity: String, + rule: String, + target: Target, +} + +#[derive(Debug, Clone)] +enum Target { + File(String), + Pattern(glob::Pattern), +} + +/// The estate severity word for one of this bot's severities. +fn estate_severity(s: Severity) -> &'static str { + match s { + Severity::Error => "high", + Severity::Warning => "medium", + Severity::Info => "info", + Severity::Suggestion => "low", + } +} + +fn text_of<'a>(entry: &'a Value, key: &str) -> Option<&'a str> { + entry.get(key).and_then(Value::as_str) +} + +impl Baseline { + /// Load `/.hypatia-baseline.json`; an absent or unreadable file + /// acknowledges nothing. + pub fn load(root: &Path) -> Self { + match std::fs::read_to_string(root.join(FILE_NAME)) { + Ok(t) => Self::parse(&t, chrono::Utc::now().date_naive()), + Err(_) => Self::default(), + } + } + + /// Parse baseline text as of `today` (entries expire after their date). + pub fn parse(text: &str, today: NaiveDate) -> Self { + let value: Value = match serde_json::from_str(text) { + Ok(v) => v, + Err(e) => { + warn!("{FILE_NAME} is not valid JSON ({e}); acknowledging nothing"); + return Self::default(); + } + }; + let Some(array) = value.as_array() else { + warn!("{FILE_NAME} is not a JSON array; acknowledging nothing"); + return Self::default(); + }; + let mut entries = Vec::new(); + for entry in array { + if text_of(entry, "rule_module") != Some(MODULE) { + continue; // Hypatia's own entry (or another module's), not ours + } + if entry.get("severity_override").is_some() { + warn!("{FILE_NAME}: severity_override is not supported by finishing-bot; entry not applied"); + continue; + } + if let Some(expires) = entry.get("expires_at") { + match expires + .as_str() + .and_then(|d| NaiveDate::parse_from_str(d, "%Y-%m-%d").ok()) + { + Some(date) if date >= today => {} + Some(_) => continue, // expired: the finding reappears + None => { + warn!("{FILE_NAME}: unparseable expires_at; entry not applied"); + continue; + } + } + } + let (Some(severity), Some(rule)) = (text_of(entry, "severity"), text_of(entry, "type")) + else { + warn!("{FILE_NAME}: entry lacks severity or type; not applied"); + continue; + }; + let target = match (text_of(entry, "file"), text_of(entry, "file_pattern")) { + (Some(file), None) => Target::File(file.to_string()), + (None, Some(pattern)) => match glob::Pattern::new(pattern) { + Ok(p) => Target::Pattern(p), + Err(e) => { + warn!("{FILE_NAME}: bad file_pattern {pattern:?} ({e}); not applied"); + continue; + } + }, + _ => { + warn!( + "{FILE_NAME}: entry needs exactly one of file or file_pattern; not applied" + ); + continue; + } + }; + entries.push(Entry { + severity: severity.to_string(), + rule: rule.to_string(), + target, + }); + } + Self { entries } + } + + /// Does an entry acknowledge finding `rule_id` of `severity` at repo-relative + /// path `rel`? + pub fn acknowledges(&self, rule_id: &str, severity: Severity, rel: &str) -> bool { + let rel = rel.strip_prefix("./").unwrap_or(rel); + let severity = estate_severity(severity); + let options = glob::MatchOptions { + case_sensitive: true, + require_literal_separator: true, + require_literal_leading_dot: false, + }; + self.entries.iter().any(|e| { + e.severity == severity + && e.rule == rule_id + && match &e.target { + Target::File(f) => f == rel, + Target::Pattern(p) => p.matches_with(rel, options), + } + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn day(s: &str) -> NaiveDate { + NaiveDate::parse_from_str(s, "%Y-%m-%d").unwrap() + } + + const FILE: &str = r#"[ + {"severity":"critical","rule_module":"cicd_rules","type":"banned_language_file","file":"a.py"}, + {"severity":"high","rule_module":"finishing_bot","type":"PH-001","file_pattern":"build/just/**"}, + {"severity":"medium","rule_module":"finishing_bot","type":"PH-002","file":"Justfile","note":"why"}, + {"severity":"high","rule_module":"finishing_bot","type":"REL-003","file_pattern":"scripts/check-*.sh"} + ]"#; + + #[test] + fn exact_rule_severity_and_path_or_pattern_are_acknowledged() { + let b = Baseline::parse(FILE, day("2026-11-13")); + assert!(b.acknowledges("PH-001", Severity::Error, "build/just/a.just")); + assert!(b.acknowledges("PH-001", Severity::Error, "./build/just/deep/a.just")); + assert!(b.acknowledges("PH-002", Severity::Warning, "Justfile")); + assert!(b.acknowledges("REL-003", Severity::Error, "scripts/check-x.sh")); + } + + #[test] + fn nothing_but_an_exact_match_is_acknowledged() { + let b = Baseline::parse(FILE, day("2026-11-13")); + assert!(!b.acknowledges("PH-001", Severity::Error, "src/main.rs")); // other path + assert!(!b.acknowledges("PH-001", Severity::Warning, "build/just/a.just")); // severity + assert!(!b.acknowledges("PH-003", Severity::Error, "build/just/a.just")); // exact id + assert!(!b.acknowledges("PH", Severity::Error, "build/just/a.just")); // no prefix form + assert!(!b.acknowledges("PH-002", Severity::Warning, "sub/Justfile")); // exact file + assert!(!b.acknowledges("REL-003", Severity::Error, "scripts/sub/check-x.sh")); // * stops at / + assert!(!b.acknowledges("banned_language_file", Severity::Error, "a.py")); + // Hypatia's entry + } + + #[test] + fn expired_and_unsupported_entries_acknowledge_nothing() { + let text = r#"[ + {"severity":"high","rule_module":"finishing_bot","type":"PH-001","file":"a","expires_at":"2026-11-12"}, + {"severity":"high","rule_module":"finishing_bot","type":"PH-002","file":"a","expires_at":"2026-11-13"}, + {"severity":"high","rule_module":"finishing_bot","type":"PH-003","file":"a","expires_at":"soon"}, + {"severity":"high","rule_module":"finishing_bot","type":"PH-004","file":"a","severity_override":"advisory"}, + {"severity":"high","rule_module":"finishing_bot","type":"PH-005"}, + {"severity":"high","rule_module":"finishing_bot","type":"PH-006","file":"a","file_pattern":"*"} + ]"#; + let b = Baseline::parse(text, day("2026-11-13")); + assert!(!b.acknowledges("PH-001", Severity::Error, "a"), "expired"); + assert!( + b.acknowledges("PH-002", Severity::Error, "a"), + "last valid day" + ); + for id in ["PH-003", "PH-004", "PH-005", "PH-006"] { + assert!(!b.acknowledges(id, Severity::Error, "a"), "{id}"); + } + } + + #[test] + fn malformed_or_absent_baseline_acknowledges_nothing() { + for text in ["", "{", "{}", "\"x\"", "[1, null]"] { + let b = Baseline::parse(text, day("2026-11-13")); + assert!(!b.acknowledges("PH-001", Severity::Error, "a"), "{text}"); + } + let d = tempfile::TempDir::new().unwrap(); + assert!(!Baseline::load(d.path()).acknowledges("PH-001", Severity::Error, "a")); + } + + #[test] + fn estate_severity_words_follow_the_report_back_mapping() { + assert_eq!(estate_severity(Severity::Error), "high"); + assert_eq!(estate_severity(Severity::Warning), "medium"); + assert_eq!(estate_severity(Severity::Info), "info"); + assert_eq!(estate_severity(Severity::Suggestion), "low"); + } +} diff --git a/bots/finishingbot/src/ignore.rs b/bots/finishingbot/src/ignore.rs deleted file mode 100644 index d74a6342..00000000 --- a/bots/finishingbot/src/ignore.rs +++ /dev/null @@ -1,164 +0,0 @@ -// SPDX-License-Identifier: MPL-2.0 -// SPDX-FileCopyrightText: 2026 Jonathan D.A. Jewell -//! Scoped exemptions from the repository's `.hypatia-ignore`. -//! -//! The estate already has one documented, reviewed place for "this path is -//! exempt from that rule, and here is why": `.hypatia-ignore`, which the RSR -//! template ships and scaffolded repositories inherit. finishing-bot honours -//! the same file with the same grammar rather than growing a second ignore -//! mechanism, and rather than special-casing any repository by name. -//! -//! Grammar (one entry per line; `#` starts a comment): -//! -//! ```text -//! /: rule-scoped, e.g. finishing_bot/PH:build/just/ -//! /*: any rule of that module -//! any rule, any module -//! ``` -//! -//! A path fragment is a substring of the repo-relative path. This bot's module -//! is `finishing_bot`; entries for other modules (`structural_drift/...`) are -//! not ours and are ignored. A `` is an exact finding id (`PH-002`) or a -//! family prefix (`PH` matches `PH-001`, `PH-002`, ...). - -use std::path::Path; - -/// The module name under which finishing-bot's rules are addressed. -pub const MODULE: &str = "finishing_bot"; - -#[derive(Debug, Default, Clone)] -pub struct IgnoreFile { - entries: Vec, -} - -#[derive(Debug, Clone)] -struct Entry { - /// `None` = any rule. - rule: Option, - fragment: String, -} - -/// Drop a trailing `# reason`: a `#` that starts the line or follows whitespace. -fn strip_inline_comment(line: &str) -> &str { - let mut prev_ws = true; - for (i, c) in line.char_indices() { - if c == '#' && prev_ws { - return &line[..i]; - } - prev_ws = c.is_whitespace(); - } - line -} - -impl IgnoreFile { - /// Load `/.hypatia-ignore`; an absent or unreadable file exempts nothing. - pub fn load(root: &Path) -> Self { - std::fs::read_to_string(root.join(".hypatia-ignore")) - .map(|t| Self::parse(&t)) - .unwrap_or_default() - } - - pub fn parse(text: &str) -> Self { - let mut entries = Vec::new(); - for line in text.lines() { - let line = strip_inline_comment(line).trim(); - if line.is_empty() || line.starts_with('#') { - continue; - } - let (scope, fragment) = match line.split_once(':') { - Some((scope, fragment)) - if scope.contains('/') && !scope.contains(char::is_whitespace) => - { - (Some(scope), fragment) - } - _ => (None, line), - }; - let fragment = fragment.trim(); - if fragment.is_empty() { - continue; - } - let rule = match scope { - None => None, - Some(scope) => { - let (module, rule) = scope.split_once('/').unwrap_or((scope, "*")); - if module != MODULE { - continue; // another module's entry - } - if rule == "*" { - None - } else { - Some(rule.to_string()) - } - } - }; - entries.push(Entry { - rule, - fragment: fragment.to_string(), - }); - } - Self { entries } - } - - /// Does an entry exempt finding `rule_id` at repo-relative path `rel`? - pub fn suppresses(&self, rule_id: &str, rel: &str) -> bool { - let rel = rel.strip_prefix("./").unwrap_or(rel); - self.entries.iter().any(|e| { - rel.contains(&e.fragment) - && e.rule.as_deref().is_none_or(|r| { - rule_id == r - || rule_id - .strip_prefix(r) - .is_some_and(|rest| rest.starts_with('-')) - }) - }) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - const FILE: &str = "\ -# comment -structural_drift/SD022:build/docs-seed/ -finishing_bot/PH:build/just/ -finishing_bot/PH-002:Justfile -finishing_bot/*:scripts/check- -generated/ -"; - - #[test] - fn rule_family_and_exact_id_and_wildcard_and_bare_entries() { - let f = IgnoreFile::parse(FILE); - assert!(f.suppresses("PH-001", "build/just/a.just")); // family PH - assert!(f.suppresses("PH-003", "./build/just/a.just")); // ./ tolerated - assert!(f.suppresses("PH-002", "Justfile")); // exact id - assert!(f.suppresses("REL-003", "scripts/check-x.sh")); // module wildcard - assert!(f.suppresses("CLM-006", "generated/out.md")); // bare: any rule - } - - #[test] - fn non_matching_rule_path_or_module_is_not_suppressed() { - let f = IgnoreFile::parse(FILE); - assert!(!f.suppresses("PH-001", "src/main.rs")); // other path - assert!(!f.suppresses("PH-003", "Justfile")); // exact id is PH-002 only - assert!(!f.suppresses("REL-003", "build/just/a.just")); // family is PH only - assert!(!f.suppresses("SD022", "build/docs-seed/x")); // another module's entry - assert!(!f.suppresses("PHX-001", "build/just/a.just")); // prefix needs '-' boundary - } - - #[test] - fn inline_reason_comment_is_not_part_of_the_path() { - let f = IgnoreFile::parse("finishing_bot/PH:build/just/ # scanner names tokens\ngenerated/ # reason\nkeep#hash/\n"); - assert!(f.suppresses("PH-001", "build/just/a.just")); - assert!(f.suppresses("CLM-001", "generated/x")); - assert!(f.suppresses("CLM-001", "keep#hash/x")); - assert!(!f.suppresses("PH-001", "reason")); - } - - #[test] - fn absent_file_exempts_nothing() { - let d = tempfile::TempDir::new().unwrap(); - assert!(!IgnoreFile::load(d.path()).suppresses("PH-001", "build/just/a.just")); - } -} diff --git a/bots/finishingbot/src/lib.rs b/bots/finishingbot/src/lib.rs index 12b717b6..34436efe 100644 --- a/bots/finishingbot/src/lib.rs +++ b/bots/finishingbot/src/lib.rs @@ -16,7 +16,7 @@ pub mod analyzers; pub mod config; pub mod error; -pub mod ignore; +pub mod baseline; pub use analyzers::{ claims::ClaimsAnalyzer, license::LicenseAnalyzer, placeholder::PlaceholderAnalyzer, diff --git a/bots/finishingbot/src/main.rs b/bots/finishingbot/src/main.rs index 012d63e3..bb797f50 100644 --- a/bots/finishingbot/src/main.rs +++ b/bots/finishingbot/src/main.rs @@ -5,7 +5,7 @@ mod analyzers; mod config; mod error; -mod ignore; +mod baseline; #[cfg(feature = "fleet")] mod fleet; diff --git a/bots/finishingbot/tests/analyzer_tests.rs b/bots/finishingbot/tests/analyzer_tests.rs index a17fbe77..ce4186e5 100644 --- a/bots/finishingbot/tests/analyzer_tests.rs +++ b/bots/finishingbot/tests/analyzer_tests.rs @@ -824,8 +824,18 @@ mod testing_analyzer { .unwrap(); fs::create_dir_all(path.join("benches")).unwrap(); fs::write(path.join("benches/bench.rs"), "fn main() {}\n").unwrap(); - fs::create_dir_all(path.join("fuzz")).unwrap(); - fs::write(path.join("fuzz/fuzz_target.rs"), "fn main() {}\n").unwrap(); + // A recognisable cargo-fuzz harness: a placeholder file no longer counts. + fs::create_dir_all(path.join("fuzz/fuzz_targets")).unwrap(); + fs::write( + path.join("fuzz/Cargo.toml"), + "[package]\nname = \"test-fuzz\"\n\n[[bin]]\nname = \"t\"\npath = \"fuzz_targets/t.rs\"\n", + ) + .unwrap(); + fs::write( + path.join("fuzz/fuzz_targets/t.rs"), + "libfuzzer_sys::fuzz_target!(|data: &[u8]| { let _ = data; });\n", + ) + .unwrap(); fs::create_dir_all(path.join(".github/workflows")).unwrap(); fs::write( path.join(".github/workflows/stress-test.yml"), From 1ae0bd566960014d612762afc6e5bb120efb5758 Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 13:58:04 +0000 Subject: [PATCH 15/18] fix(finishingbot): baseline file_pattern follows gitignore anchoring A pattern with no slash (e.g. *.rs) now matches at any depth, a leading slash anchors to the root, a trailing slash names a directory and everything beneath it, and any other pattern containing a slash stays anchored. Previously require_literal_separator limited basename patterns to the repo root. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/finishingbot/src/baseline.rs | 55 ++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/bots/finishingbot/src/baseline.rs b/bots/finishingbot/src/baseline.rs index 85997dfe..931ed9a9 100644 --- a/bots/finishingbot/src/baseline.rs +++ b/bots/finishingbot/src/baseline.rs @@ -76,6 +76,32 @@ fn text_of<'a>(entry: &'a Value, key: &str) -> Option<&'a str> { entry.get(key).and_then(Value::as_str) } +/// Rewrite a gitignore-style `file_pattern` into an anchored glob over +/// repo-relative paths: a leading `/` anchors to the root (and is dropped); a +/// pattern with no `/` other than a trailing one matches at any depth +/// (`*.rs` -> `**/*.rs`); a trailing `/` names a directory, so everything +/// beneath it matches. Any other pattern containing `/` stays anchored. +fn gitignore_glob(pattern: &str) -> String { + let (rooted, rest) = match pattern.strip_prefix('/') { + Some(r) => (true, r), + None => (false, pattern), + }; + let (dir_only, body) = match rest.strip_suffix('/') { + Some(b) => (true, b), + None => (false, rest), + }; + let anchored = rooted || body.contains('/'); + let mut out = String::new(); + if !anchored && !body.starts_with("**/") { + out.push_str("**/"); + } + out.push_str(body); + if dir_only { + out.push_str("/**"); + } + out +} + impl Baseline { /// Load `/.hypatia-baseline.json`; an absent or unreadable file /// acknowledges nothing. @@ -128,7 +154,7 @@ impl Baseline { }; let target = match (text_of(entry, "file"), text_of(entry, "file_pattern")) { (Some(file), None) => Target::File(file.to_string()), - (None, Some(pattern)) => match glob::Pattern::new(pattern) { + (None, Some(pattern)) => match glob::Pattern::new(&gitignore_glob(pattern)) { Ok(p) => Target::Pattern(p), Err(e) => { warn!("{FILE_NAME}: bad file_pattern {pattern:?} ({e}); not applied"); @@ -209,6 +235,33 @@ mod tests { // Hypatia's entry } + #[test] + fn file_pattern_follows_gitignore_anchoring() { + let text = r#"[ + {"severity":"high","rule_module":"finishing_bot","type":"PH-001","file_pattern":"*.rs"}, + {"severity":"high","rule_module":"finishing_bot","type":"PH-002","file_pattern":"/*.rs"}, + {"severity":"high","rule_module":"finishing_bot","type":"PH-003","file_pattern":"build/"}, + {"severity":"high","rule_module":"finishing_bot","type":"PH-004","file_pattern":"src/*.rs"} + ]"#; + let b = Baseline::parse(text, day("2026-11-13")); + let e = Severity::Error; + // no slash: any depth, including the root + assert!(b.acknowledges("PH-001", e, "main.rs")); + assert!(b.acknowledges("PH-001", e, "src/main.rs")); + assert!(b.acknowledges("PH-001", e, "a/b/c/x.rs")); + assert!(!b.acknowledges("PH-001", e, "src/main.py")); + // leading slash: root only + assert!(b.acknowledges("PH-002", e, "main.rs")); + assert!(!b.acknowledges("PH-002", e, "src/main.rs")); + // trailing slash: a directory at any depth, everything beneath it + assert!(b.acknowledges("PH-003", e, "build/x")); + assert!(b.acknowledges("PH-003", e, "a/build/x/y")); + assert!(!b.acknowledges("PH-003", e, "rebuild/x")); + // a slash inside the pattern keeps it anchored + assert!(b.acknowledges("PH-004", e, "src/main.rs")); + assert!(!b.acknowledges("PH-004", e, "x/src/main.rs")); + } + #[test] fn expired_and_unsupported_entries_acknowledge_nothing() { let text = r#"[ From e3d1c09ac51562fee729ac516e10c99d9c9b9a1e Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 13:58:04 +0000 Subject: [PATCH 16/18] fix(finishingbot): TEST-RU-003 needs real fuzz harness evidence, not comments fuzz_target! must be invoked outside // and /* */ comments (followed by ( { or |); a [[bin]] table must be a non-comment header whose path, or default target file, exists. A comment-only fuzz_targets/t.rs or a commented [[bin]] no longer satisfies the check. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/finishingbot/src/analyzers/testing.rs | 114 ++++++++++++++++++++- 1 file changed, 109 insertions(+), 5 deletions(-) diff --git a/bots/finishingbot/src/analyzers/testing.rs b/bots/finishingbot/src/analyzers/testing.rs index f1a3a27c..5622f0e0 100644 --- a/bots/finishingbot/src/analyzers/testing.rs +++ b/bots/finishingbot/src/analyzers/testing.rs @@ -565,12 +565,13 @@ pub(crate) fn rust_has_code(root: &Path) -> bool { } /// True when `dir` (a `fuzz/` or `tests/fuzz/` directory) holds a recognisable -/// harness: a `Cargo.toml` declaring `[[bin]]` targets (cargo-fuzz layout), or a -/// `fuzz_targets/*.rs` file that invokes `fuzz_target!`. An empty directory or a -/// placeholder file is not fuzzing infrastructure. +/// harness: a `Cargo.toml` with a real `[[bin]]` table whose target file exists +/// (cargo-fuzz layout), or a `fuzz_targets/*.rs` file that invokes +/// `fuzz_target!` outside comments. An empty directory, a placeholder file or a +/// commented-out declaration is not fuzzing infrastructure. fn has_fuzz_harness(dir: &Path) -> bool { if let Ok(manifest) = std::fs::read_to_string(dir.join("Cargo.toml")) { - if manifest.contains("[[bin]]") { + if manifest_has_existing_bin(dir, &manifest) { return true; } } @@ -580,7 +581,59 @@ fn has_fuzz_harness(dir: &Path) -> bool { entries.filter_map(|e| e.ok()).any(|e| { let p = e.path(); p.extension().is_some_and(|x| x == "rs") - && std::fs::read_to_string(&p).is_ok_and(|s| s.contains("fuzz_target!")) + && std::fs::read_to_string(&p).is_ok_and(|s| invokes_fuzz_target(&s)) + }) +} + +/// True when `src` invokes the `fuzz_target!` macro in code: not in a `//` or +/// `/* */` comment, and followed (after optional whitespace) by `(`, `{` or `|`. +fn invokes_fuzz_target(src: &str) -> bool { + const NAME: &str = "fuzz_target!"; + strip_block_comments(src).lines().any(|line| { + let code = line.split("//").next().unwrap_or(""); + code.match_indices(NAME).any(|(at, _)| { + code[at + NAME.len()..] + .trim_start() + .starts_with(['(', '{', '|']) + }) + }) +} + +/// True when the manifest has a `[[bin]]` table header on a non-comment line +/// whose target file exists under `dir`: its `path` if given, else the cargo +/// default for the named target (`src/bin/.rs`, `src/bin//main.rs`, +/// or `src/main.rs`). +fn manifest_has_existing_bin(dir: &Path, manifest: &str) -> bool { + let mut tables: Vec<(Option, Option)> = Vec::new(); + let mut in_bin = false; + for raw in manifest.lines() { + let line = raw.split('#').next().unwrap_or("").trim(); + if line.starts_with('[') { + in_bin = line == "[[bin]]"; + if in_bin { + tables.push((None, None)); + } + } else if in_bin { + if let Some((key, value)) = line.split_once('=') { + let value = value.trim().trim_matches(['"', '\'']).to_string(); + if let Some(t) = tables.last_mut() { + match key.trim() { + "path" => t.0 = Some(value), + "name" => t.1 = Some(value), + _ => {} + } + } + } + } + } + tables.iter().any(|(path, name)| match (path, name) { + (Some(p), _) => dir.join(p).is_file(), + (None, Some(n)) => { + dir.join(format!("src/bin/{n}.rs")).is_file() + || dir.join(format!("src/bin/{n}/main.rs")).is_file() + || dir.join("src/main.rs").is_file() + } + (None, None) => dir.join("src/main.rs").is_file(), }) } @@ -734,6 +787,57 @@ mod no_code_tests { "fuzz/Cargo.toml", "[package]\nname = \"x-fuzz\"\n\n[[bin]]\nname = \"t\"\npath = \"fuzz_targets/t.rs\"\n", ); + write(d.path(), "fuzz/fuzz_targets/t.rs", "fn main() {}\n"); + assert!(!ids(d.path()).contains(&"TEST-RU-003".to_string())); + } + + #[test] + fn comment_only_fuzz_target_mention_is_not_a_harness() { + let d = rust_repo(Some("pub fn f() {}\n")); + for body in [ + "// TODO: add fuzz_target!\n", + "/* fuzz_target!(|d: &[u8]| {}); */\n", + "// fuzz_target!(|d: &[u8]| {});\nfn main() {}\n", + "fn main() {} // fuzz_target!(|d: &[u8]| {})\n", + "fn fuzz_target_name() {}\n", + ] { + write(d.path(), "fuzz/fuzz_targets/t.rs", body); + assert!( + ids(d.path()).contains(&"TEST-RU-003".to_string()), + "{body:?}" + ); + } + for body in [ + "fuzz_target!(|d: &[u8]| {});\n", + "libfuzzer_sys::fuzz_target! { |d: &[u8]| { let _ = d; } }\n", + "/* c */ fuzz_target! (|d: &[u8]| {});\n", + ] { + write(d.path(), "fuzz/fuzz_targets/t.rs", body); + assert!( + !ids(d.path()).contains(&"TEST-RU-003".to_string()), + "{body:?}" + ); + } + } + + #[test] + fn commented_or_dangling_bin_table_is_not_a_harness() { + let d = rust_repo(Some("pub fn f() {}\n")); + write(d.path(), "fuzz/fuzz_targets/t.rs", "// nothing\n"); + for manifest in [ + "[package]\nname = \"x\"\n# [[bin]]\n# name = \"t\"\n", + "[package]\nname = \"x\"\n\n[[bin]] # real header, missing file\nname = \"t\"\npath = \"fuzz_targets/missing.rs\"\n", + "[package]\nname = \"x\"\n\n[[bin]]\nname = \"t\"\n", + ] { + write(d.path(), "fuzz/Cargo.toml", manifest); + assert!(ids(d.path()).contains(&"TEST-RU-003".to_string()), "{manifest:?}"); + } + write(d.path(), "fuzz/src/bin/t.rs", "fn main() {}\n"); + write( + d.path(), + "fuzz/Cargo.toml", + "[package]\nname = \"x\"\n\n[[bin]]\nname = \"t\"\n", + ); assert!(!ids(d.path()).contains(&"TEST-RU-003".to_string())); } } From c56bc435e5f1c485541d240b143c41fcd65f1b68 Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 13:58:04 +0000 Subject: [PATCH 17/18] docs(finishingbot,glambot): SCM files are validated only if present Their absence is canonical now that the descriptiles are clauses of the chora deed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/finishingbot/README.adoc | 2 +- bots/glambot/README.adoc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bots/finishingbot/README.adoc b/bots/finishingbot/README.adoc index 714d73e3..8d4e7849 100644 --- a/bots/finishingbot/README.adoc +++ b/bots/finishingbot/README.adoc @@ -49,7 +49,7 @@ the gitbot-fleet shared context layer and makes the final release-readiness dete |No |**SCM Files** -|All 6 `.machine_readable/` files (STATE.scm, META.scm, ECOSYSTEM.scm, PLAYBOOK.scm, AGENTIC.scm, NEUROSYM.scm), SPDX headers, required sections, content completeness +|Legacy `.machine_readable/` SCM files (STATE.scm, META.scm, ECOSYSTEM.scm, PLAYBOOK.scm, AGENTIC.scm, NEUROSYM.scm) are validated only if present (SPDX headers, required sections, content completeness); their absence is canonical, since those descriptiles are now clauses of `_chora.deed` |No |**Testing** diff --git a/bots/glambot/README.adoc b/bots/glambot/README.adoc index b9614f5f..bf289555 100644 --- a/bots/glambot/README.adoc +++ b/bots/glambot/README.adoc @@ -90,7 +90,7 @@ Evaluates repository discoverability through README descriptions, HTML meta tags === Machine-Readability (`MachineAnalyzer`) -Validates JSON/YAML file syntax, structured data presence, robots.txt, and `.machine_readable/` directory with SCM files. +Validates JSON/YAML file syntax, structured data presence, robots.txt, and the `.machine_readable/` directory. Legacy SCM files there are validated only if present; their absence is canonical (the descriptiles are clauses of `_chora.deed`). [cols="1,2,1"] |=== From a724b6c768a6180b415cf76493cf272466cfc6ba Mon Sep 17 00:00:00 2001 From: CLAUDE <355856132+HoTT-stuff@users.noreply.github.com> Date: Fri, 13 Nov 2026 13:58:04 +0000 Subject: [PATCH 18/18] style(finishingbot,glambot): cargo fmt Formatting only; the base did not pass cargo fmt --check. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0136P7ZkERqsQWMogHRpM2z8 --- bots/finishingbot/src/analyzers/license.rs | 27 +++-- bots/finishingbot/src/analyzers/mod.rs | 5 +- bots/finishingbot/src/analyzers/scm_files.rs | 23 +++- .../src/analyzers/v1_readiness.rs | 26 +++- bots/finishingbot/src/config.rs | 10 +- bots/finishingbot/src/fleet.rs | 44 ++++--- bots/finishingbot/src/lib.rs | 12 +- bots/finishingbot/src/main.rs | 54 ++++----- bots/finishingbot/tests/analyzer_tests.rs | 111 +++++++----------- bots/glambot/src/analyzers/accessibility.rs | 54 ++++----- .../src/analyzers/git_seo_integration.rs | 35 +++--- bots/glambot/src/analyzers/machine.rs | 21 +++- bots/glambot/src/analyzers/mod.rs | 6 +- bots/glambot/src/analyzers/seo.rs | 28 ++--- bots/glambot/src/analyzers/visual.rs | 61 ++++++---- bots/glambot/src/config/mod.rs | 4 +- bots/glambot/src/lib.rs | 4 +- bots/glambot/src/main.rs | 13 +- bots/glambot/tests/integration.rs | 15 +-- 19 files changed, 286 insertions(+), 267 deletions(-) diff --git a/bots/finishingbot/src/analyzers/license.rs b/bots/finishingbot/src/analyzers/license.rs index 65cb6c7e..5c75c927 100644 --- a/bots/finishingbot/src/analyzers/license.rs +++ b/bots/finishingbot/src/analyzers/license.rs @@ -7,13 +7,15 @@ use crate::config::Config; use crate::error::Result; use regex::Regex; use std::path::Path; +use std::sync::LazyLock; use tracing::debug; use walkdir::WalkDir; -use std::sync::LazyLock; // Compiled once. These were rebuilt on every call (hypatia expect_in_hot_path). -static SPDX_PATTERN: LazyLock = - LazyLock::new(|| Regex::new(r"(?i)SPDX-License-Identifier:\s*[A-Za-z0-9.-]+").expect("SPDX header regex is a valid constant pattern")); +static SPDX_PATTERN: LazyLock = LazyLock::new(|| { + Regex::new(r"(?i)SPDX-License-Identifier:\s*[A-Za-z0-9.-]+") + .expect("SPDX header regex is a valid constant pattern") +}); /// Repositories that deliberately use AGPL-3.0-or-later instead of PMPL. /// These are co-developed projects with the user's son and use AGPL by deliberate choice. @@ -100,9 +102,7 @@ impl LicenseAnalyzer { "COPYING.md", ]; - let found_license = license_files - .iter() - .find(|name| path.join(name).exists()); + let found_license = license_files.iter().find(|name| path.join(name).exists()); match found_license { None => { @@ -206,7 +206,9 @@ impl LicenseAnalyzer { return Some("GPL-3.0-or-later".to_string()); } - if content_lower.contains("mit license") || content_lower.contains("permission is hereby granted, free of charge") { + if content_lower.contains("mit license") + || content_lower.contains("permission is hereby granted, free of charge") + { return Some("MIT".to_string()); } @@ -214,7 +216,9 @@ impl LicenseAnalyzer { return Some("Apache-2.0".to_string()); } - if content_lower.contains("bsd 3-clause") || content_lower.contains("redistribution and use in source and binary forms") { + if content_lower.contains("bsd 3-clause") + || content_lower.contains("redistribution and use in source and binary forms") + { return Some("BSD-3-Clause".to_string()); } @@ -330,7 +334,12 @@ impl LicenseAnalyzer { let new_content = if content.starts_with("#!") { let mut lines = content.lines(); let shebang = lines.next().unwrap_or(""); - format!("{}\n{}{}", shebang, header, lines.collect::>().join("\n")) + format!( + "{}\n{}{}", + shebang, + header, + lines.collect::>().join("\n") + ) } else { format!("{}{}", header, content) }; diff --git a/bots/finishingbot/src/analyzers/mod.rs b/bots/finishingbot/src/analyzers/mod.rs index df04008e..b0a17ca5 100644 --- a/bots/finishingbot/src/analyzers/mod.rs +++ b/bots/finishingbot/src/analyzers/mod.rs @@ -22,10 +22,7 @@ use std::path::{Path, PathBuf}; /// (`.adoc` or `.md`, repo root or `.github/`), and a rule that names exactly /// one spelling reports a false positive for the others. pub(crate) fn first_existing(root: &Path, candidates: &[&str]) -> Option { - candidates - .iter() - .map(|c| root.join(c)) - .find(|p| p.exists()) + candidates.iter().map(|c| root.join(c)).find(|p| p.exists()) } /// Accepted spellings and locations of a security policy. diff --git a/bots/finishingbot/src/analyzers/scm_files.rs b/bots/finishingbot/src/analyzers/scm_files.rs index d882b820..692c4b2d 100644 --- a/bots/finishingbot/src/analyzers/scm_files.rs +++ b/bots/finishingbot/src/analyzers/scm_files.rs @@ -29,7 +29,10 @@ impl Analyzer for ScmFilesAnalyzer { // Required SCM files for v1.0 let required_scm_files = [ ("STATE.scm", "Project state and progress tracking"), - ("META.scm", "Meta-level information and architectural decisions"), + ( + "META.scm", + "Meta-level information and architectural decisions", + ), ("ECOSYSTEM.scm", "Project's place in the ecosystem"), ("PLAYBOOK.scm", "Operational playbooks and procedures"), ("AGENTIC.scm", "Agentic AI integration documentation"), @@ -65,7 +68,8 @@ impl Analyzer for ScmFilesAnalyzer { ) .with_file(root_path) .with_suggestion(&format!( - "Move {} to .machine_readable/{}", filename, filename + "Move {} to .machine_readable/{}", + filename, filename )), ); } else { @@ -74,7 +78,10 @@ impl Analyzer for ScmFilesAnalyzer { // AGENTIC, NEUROSYM) are clauses of the repository's // `_chora.deed`, not files, so absence is the canonical // state. Files that DO exist are still validated above. - debug!("{} not present (retired: now a clause of the chora deed)", filename); + debug!( + "{} not present (retired: now a clause of the chora deed)", + filename + ); } } @@ -121,7 +128,11 @@ impl ScmFilesAnalyzer { &format!("SCM-003-{}", filename), &format!("{} appears incomplete", filename), Severity::Warning, - &format!("{} is very short ({} bytes). May be a placeholder.", filename, content.len()), + &format!( + "{} is very short ({} bytes). May be a placeholder.", + filename, + content.len() + ), ) .with_file(file_path.to_path_buf()) .with_suggestion("Ensure file has complete documentation content"), @@ -141,7 +152,9 @@ impl ScmFilesAnalyzer { "STATE.scm should contain 'metadata' and 'current-position' sections.", ) .with_file(file_path.to_path_buf()) - .with_suggestion("Add required sections following hyperpolymath STATE.scm format"), + .with_suggestion( + "Add required sections following hyperpolymath STATE.scm format", + ), ); } "META.scm" diff --git a/bots/finishingbot/src/analyzers/v1_readiness.rs b/bots/finishingbot/src/analyzers/v1_readiness.rs index ac237b60..35ca9a3b 100644 --- a/bots/finishingbot/src/analyzers/v1_readiness.rs +++ b/bots/finishingbot/src/analyzers/v1_readiness.rs @@ -58,7 +58,11 @@ impl V1ReadinessAnalyzer { // the document is missing. Estate canon keeps the roadmap at // docs/status/ROADMAP.adoc (the STATE journal). let required_docs: [(&str, &str, &[&str]); 3] = [ - ("README.adoc", "Main project documentation", &["README.adoc"]), + ( + "README.adoc", + "Main project documentation", + &["README.adoc"], + ), ( "ROADMAP.adoc", "Project roadmap", @@ -110,7 +114,10 @@ impl V1ReadinessAnalyzer { // Check for banned language files let banned_patterns = [ ("*.ts", "TypeScript files found (use AffineScript instead)"), - ("*.py", "Python files found (use Julia/Rust/AffineScript instead)"), + ( + "*.py", + "Python files found (use Julia/Rust/AffineScript instead)", + ), ("*.go", "Go files found (use Rust instead)"), ]; @@ -143,7 +150,10 @@ impl V1ReadinessAnalyzer { &format!("{}. Found {} file(s).", message, found_files.len()), ) .with_file(found_files[0].clone()) - .with_suggestion(&format!("Remove or convert {} files to allowed languages", pattern)), + .with_suggestion(&format!( + "Remove or convert {} files to allowed languages", + pattern + )), ); } } @@ -250,7 +260,10 @@ mod tests { for rel in ["docs/status/ROADMAP.adoc", "ROADMAP.adoc"] { let d = TempDir::new().unwrap(); touch(d.path(), rel); - assert!(!ids(d.path()).contains(&"V1-DOC-ROADMAP.adoc".to_string()), "{rel}"); + assert!( + !ids(d.path()).contains(&"V1-DOC-ROADMAP.adoc".to_string()), + "{rel}" + ); } } @@ -268,7 +281,10 @@ mod tests { ] { let d = TempDir::new().unwrap(); touch(d.path(), rel); - assert!(!ids(d.path()).contains(&"V1-COM-CONTRIBUTING.md".to_string()), "{rel}"); + assert!( + !ids(d.path()).contains(&"V1-COM-CONTRIBUTING.md".to_string()), + "{rel}" + ); } for rel in ["CODE_OF_CONDUCT.adoc", ".github/CODE_OF_CONDUCT.md"] { let d = TempDir::new().unwrap(); diff --git a/bots/finishingbot/src/config.rs b/bots/finishingbot/src/config.rs index 9a1c8087..9aca0bf2 100644 --- a/bots/finishingbot/src/config.rs +++ b/bots/finishingbot/src/config.rs @@ -308,7 +308,10 @@ fn default_log_format() -> String { /// Load configuration from a path pub fn load_config(path: &Path) -> Result { if !path.exists() { - debug!("Config file not found at {}, using defaults", path.display()); + debug!( + "Config file not found at {}, using defaults", + path.display() + ); return Ok(Config::default()); } @@ -362,9 +365,8 @@ mod tests { #[test] fn test_placeholder_action_serde() { let yaml = "action: remove"; - let config: PlaceholderConfig = serde_yaml::from_str(&format!( - "patterns: [TODO]\n{}", yaml - )).unwrap(); + let config: PlaceholderConfig = + serde_yaml::from_str(&format!("patterns: [TODO]\n{}", yaml)).unwrap(); assert_eq!(config.action, PlaceholderAction::Remove); } } diff --git a/bots/finishingbot/src/fleet.rs b/bots/finishingbot/src/fleet.rs index 1c75b48a..6375a964 100644 --- a/bots/finishingbot/src/fleet.rs +++ b/bots/finishingbot/src/fleet.rs @@ -40,11 +40,20 @@ pub mod cicd_hyper_a { #[serde(tag = "type", rename_all = "kebab-case")] pub enum ReleaseCheck { /// License validation - License { spdx_id: Option, require_header: bool }, + License { + spdx_id: Option, + require_header: bool, + }, /// Placeholder detection - Placeholder { patterns: Vec, allow_in_tests: bool }, + Placeholder { + patterns: Vec, + allow_in_tests: bool, + }, /// Claim verification - Claim { claim_type: String, verify_method: String }, + Claim { + claim_type: String, + verify_method: String, + }, /// Release artifact check Artifact { required_files: Vec }, /// Version consistency @@ -129,17 +138,11 @@ pub mod cicd_hyper_a { } /// Check if release should be blocked based on cicd-hyper-a rules - pub fn should_block_release( - findings: &[super::Finding], - rules: &[ReleaseRule], - ) -> bool { + pub fn should_block_release(findings: &[super::Finding], rules: &[ReleaseRule]) -> bool { for finding in findings { for rule in rules { if finding.id == rule.id && rule.blocks_release { - if matches!( - finding.severity, - super::Severity::Error - ) { + if matches!(finding.severity, super::Severity::Error) { return true; } } @@ -366,8 +369,12 @@ pub fn run_in_context(ctx: &mut Context, config: &Config) -> AuditResult { } // Complete bot execution - if let Err(e) = ctx.complete_bot(BotId::Finishbot, findings_count, errors_count, files_checked) - { + if let Err(e) = ctx.complete_bot( + BotId::Finishbot, + findings_count, + errors_count, + files_checked, + ) { error!("Failed to complete finishing-bot in context: {}", e); } @@ -448,9 +455,14 @@ mod tests { #[test] fn test_finding_conversion() { - let finding = Finding::new("LIC-001", "Missing License", Severity::Error, "No LICENSE file") - .with_file(PathBuf::from("LICENSE")) - .with_suggestion("Add a LICENSE file"); + let finding = Finding::new( + "LIC-001", + "Missing License", + Severity::Error, + "No LICENSE file", + ) + .with_file(PathBuf::from("LICENSE")) + .with_suggestion("Add a LICENSE file"); let fleet_finding = to_fleet_finding(&finding, "license"); diff --git a/bots/finishingbot/src/lib.rs b/bots/finishingbot/src/lib.rs index 34436efe..b5e43f83 100644 --- a/bots/finishingbot/src/lib.rs +++ b/bots/finishingbot/src/lib.rs @@ -14,9 +14,9 @@ #![forbid(unsafe_code)] pub mod analyzers; +pub mod baseline; pub mod config; pub mod error; -pub mod baseline; pub use analyzers::{ claims::ClaimsAnalyzer, license::LicenseAnalyzer, placeholder::PlaceholderAnalyzer, @@ -191,7 +191,10 @@ pub fn print_summary(result: &AuditResult) { } fn print_findings_section(category: &str, findings: &[Finding]) { - println!("┌─ {} ─────────────────────────────────────────────────────", category); + println!( + "┌─ {} ─────────────────────────────────────────────────────", + category + ); for finding in findings { let severity_icon = match finding.severity { @@ -202,10 +205,7 @@ fn print_findings_section(category: &str, findings: &[Finding]) { }; println!("│"); - println!( - "│ {} [{}] {}", - severity_icon, finding.id, finding.name - ); + println!("│ {} [{}] {}", severity_icon, finding.id, finding.name); println!("│ {}", finding.message); if let Some(ref file) = finding.file { diff --git a/bots/finishingbot/src/main.rs b/bots/finishingbot/src/main.rs index bb797f50..d3d40d76 100644 --- a/bots/finishingbot/src/main.rs +++ b/bots/finishingbot/src/main.rs @@ -3,9 +3,9 @@ //! finishing-bot CLI - Release Readiness Validator mod analyzers; +mod baseline; mod config; mod error; -mod baseline; #[cfg(feature = "fleet")] mod fleet; @@ -13,8 +13,8 @@ use clap::{Parser, Subcommand}; use std::path::{Path, PathBuf}; use std::process::ExitCode; use tracing_subscriber::fmt::format::FmtSpan; -use tracing_subscriber::{fmt, EnvFilter}; use tracing_subscriber::prelude::*; +use tracing_subscriber::{fmt, EnvFilter}; /// finishing-bot: Release Readiness Validator /// @@ -146,39 +146,21 @@ fn main() -> ExitCode { // Handle commands match cli.command { - Some(Command::Init { format }) => { - handle_init(&config_path, &format) - } - Some(Command::Show) => { - handle_show(&config) - } - Some(Command::Audit { strict }) => { - handle_audit(&cli.path, &config, strict, &cli.format) - } - Some(Command::Fix { only }) => { - handle_fix(&cli.path, &config, only.as_deref()) - } - Some(Command::License { list_allowed }) => { - handle_license(&cli.path, &config, list_allowed) - } - Some(Command::Placeholders { action }) => { - handle_placeholders(&cli.path, &config, &action) - } - Some(Command::Claims) => { - handle_claims(&cli.path, &config) - } - Some(Command::Release) => { - handle_release(&cli.path, &config) - } + Some(Command::Init { format }) => handle_init(&config_path, &format), + Some(Command::Show) => handle_show(&config), + Some(Command::Audit { strict }) => handle_audit(&cli.path, &config, strict, &cli.format), + Some(Command::Fix { only }) => handle_fix(&cli.path, &config, only.as_deref()), + Some(Command::License { list_allowed }) => handle_license(&cli.path, &config, list_allowed), + Some(Command::Placeholders { action }) => handle_placeholders(&cli.path, &config, &action), + Some(Command::Claims) => handle_claims(&cli.path, &config), + Some(Command::Release) => handle_release(&cli.path, &config), #[cfg(feature = "fleet")] Some(Command::Fleet { repo, context, session, output, - }) => { - handle_fleet_mode(repo, context, session, output, config) - } + }) => handle_fleet_mode(repo, context, session, output, config), None => { // Default to audit handle_audit(&cli.path, &config, false, &cli.format) @@ -302,7 +284,10 @@ fn handle_show(config: &config::Config) -> ExitCode { println!("Licenses:"); println!(" Allowed: {}", config.licenses.allowed.join(", ")); println!(" Strict: {}", config.licenses.strict); - println!(" Require SPDX headers: {}", config.licenses.require_spdx_headers); + println!( + " Require SPDX headers: {}", + config.licenses.require_spdx_headers + ); println!(); println!("Placeholders:"); @@ -320,7 +305,10 @@ fn handle_show(config: &config::Config) -> ExitCode { println!("Release:"); println!(" Require hashes: {}", config.release.require_hashes); - println!(" Require signatures: {}", config.release.require_signatures); + println!( + " Require signatures: {}", + config.release.require_signatures + ); println!(); println!("General:"); @@ -435,9 +423,7 @@ fn handle_audit(path: &Path, config: &config::Config, strict: bool, format: &str } fn handle_fix(path: &Path, config: &config::Config, only: Option<&str>) -> ExitCode { - use crate::analyzers::{ - license::LicenseAnalyzer, placeholder::PlaceholderAnalyzer, Analyzer, - }; + use crate::analyzers::{license::LicenseAnalyzer, placeholder::PlaceholderAnalyzer, Analyzer}; if only.is_none() || only == Some("license") { let license = LicenseAnalyzer; diff --git a/bots/finishingbot/tests/analyzer_tests.rs b/bots/finishingbot/tests/analyzer_tests.rs index ce4186e5..85a20070 100644 --- a/bots/finishingbot/tests/analyzer_tests.rs +++ b/bots/finishingbot/tests/analyzer_tests.rs @@ -137,10 +137,7 @@ mod license_analyzer { .iter() .filter(|f| f.id.starts_with("LIC-002")) .collect(); - assert!( - !spdx_findings.is_empty(), - "Should find missing SPDX header" - ); + assert!(!spdx_findings.is_empty(), "Should find missing SPDX header"); assert_eq!( spdx_findings[0].severity, Severity::Warning, @@ -228,11 +225,7 @@ mod placeholder_analyzer { let file_findings: Vec<_> = result .findings .iter() - .filter(|f| { - f.file - .as_ref() - .is_some_and(|p| p.ends_with("clean.rs")) - }) + .filter(|f| f.file.as_ref().is_some_and(|p| p.ends_with("clean.rs"))) .collect(); assert!( file_findings.is_empty(), @@ -255,21 +248,18 @@ mod placeholder_analyzer { let config = default_config(); let result = analyzer.analyze(&path, &config).unwrap(); - let rs_findings = result.findings.iter().filter(|f| { - f.file - .as_ref() - .is_some_and(|p| p.ends_with("code.rs")) - }); - let md_findings = result.findings.iter().filter(|f| { - f.file - .as_ref() - .is_some_and(|p| p.ends_with("notes.md")) - }); - let toml_findings = result.findings.iter().filter(|f| { - f.file - .as_ref() - .is_some_and(|p| p.ends_with("Cargo.toml")) - }); + let rs_findings = result + .findings + .iter() + .filter(|f| f.file.as_ref().is_some_and(|p| p.ends_with("code.rs"))); + let md_findings = result + .findings + .iter() + .filter(|f| f.file.as_ref().is_some_and(|p| p.ends_with("notes.md"))); + let toml_findings = result + .findings + .iter() + .filter(|f| f.file.as_ref().is_some_and(|p| p.ends_with("Cargo.toml"))); assert!(rs_findings.count() > 0, "Should detect in .rs files"); assert!(md_findings.count() > 0, "Should detect in .md files"); @@ -302,7 +292,11 @@ mod placeholder_analyzer { #[test] fn test_max_allowed_placeholders_no_error() { let (_temp, path) = setup_test_repo(); - fs::write(path.join("code.rs"), "// TODO: one\n// TODO: two\nfn main() {}\n").unwrap(); + fs::write( + path.join("code.rs"), + "// TODO: one\n// TODO: two\nfn main() {}\n", + ) + .unwrap(); let mut config = default_config(); config.placeholders.max_allowed = 5; @@ -456,7 +450,11 @@ mod claims_analyzer { ) .unwrap(); fs::create_dir_all(path.join("tests")).unwrap(); - fs::write(path.join("tests/test_main.rs"), "#[test]\nfn it_works() { assert!(true); }\n").unwrap(); + fs::write( + path.join("tests/test_main.rs"), + "#[test]\nfn it_works() { assert!(true); }\n", + ) + .unwrap(); let mut config = default_config(); config.claims.require_readme = true; @@ -664,10 +662,7 @@ mod scm_files_analyzer { let result = analyzer.analyze(&path, &config).unwrap(); assert!( - result - .findings - .iter() - .any(|f| f.id == "SCM-005-STATE.scm"), + result.findings.iter().any(|f| f.id == "SCM-005-STATE.scm"), "Should warn about STATE.scm in root instead of .machine_readable/" ); assert!( @@ -742,10 +737,7 @@ mod scm_files_analyzer { let result = analyzer.analyze(&path, &config).unwrap(); assert!( - result - .findings - .iter() - .any(|f| f.id == "SCM-002-STATE.scm"), + result.findings.iter().any(|f| f.id == "SCM-002-STATE.scm"), "Should warn about missing SPDX header in STATE.scm" ); } @@ -853,18 +845,13 @@ mod testing_analyzer { .iter() .filter(|f| { f.severity == Severity::Error - && (f.id == "TEST-RU-001" - || f.id == "TEST-RU-003" - || f.id == "TEST-RU-005") + && (f.id == "TEST-RU-001" || f.id == "TEST-RU-003" || f.id == "TEST-RU-005") }) .collect(); assert!( testing_errors.is_empty(), "Well-tested repo should have no testing infrastructure errors, got: {:?}", - testing_errors - .iter() - .map(|f| &f.id) - .collect::>() + testing_errors.iter().map(|f| &f.id).collect::>() ); } } @@ -1102,11 +1089,7 @@ mod integration { let (_temp, path) = setup_test_repo(); // Create a fairly complete repo - fs::write( - path.join("LICENSE"), - "Palimpsest License\nMPL-2.0\n", - ) - .unwrap(); + fs::write(path.join("LICENSE"), "Palimpsest License\nMPL-2.0\n").unwrap(); let readme = "# My Project\n\n\ Description of the project with enough words.\n\n\ ## Installation\n\nRun the install command.\n\n\ @@ -1132,18 +1115,10 @@ mod integration { let config = default_config(); - let license = LicenseAnalyzer - .analyze(&path, &config) - .unwrap(); - let placeholder = PlaceholderAnalyzer - .analyze(&path, &config) - .unwrap(); - let claims = ClaimsAnalyzer - .analyze(&path, &config) - .unwrap(); - let release = ReleaseAnalyzer - .analyze(&path, &config) - .unwrap(); + let license = LicenseAnalyzer.analyze(&path, &config).unwrap(); + let placeholder = PlaceholderAnalyzer.analyze(&path, &config).unwrap(); + let claims = ClaimsAnalyzer.analyze(&path, &config).unwrap(); + let release = ReleaseAnalyzer.analyze(&path, &config).unwrap(); let result = AuditResult { license, @@ -1174,10 +1149,7 @@ mod integration { // Verify Finding serializes to JSON correctly let json = serde_json::to_string(&finding).expect("Finding should serialize to JSON"); assert!(json.contains("LIC-001"), "JSON should contain finding ID"); - assert!( - json.contains("error"), - "JSON should contain severity level" - ); + assert!(json.contains("error"), "JSON should contain severity level"); // Verify round-trip let deserialized: finishing_bot::Finding = @@ -1197,12 +1169,15 @@ mod integration { Severity::Error, "No LICENSE file found", )); - result.placeholder.findings.push(finishing_bot::Finding::new( - "PH-001", - "Placeholder: TODO", - Severity::Warning, - "Found TODO placeholder", - )); + result + .placeholder + .findings + .push(finishing_bot::Finding::new( + "PH-001", + "Placeholder: TODO", + Severity::Warning, + "Found TODO placeholder", + )); assert_eq!(result.total_findings(), 2); assert_eq!(result.total_errors(), 1); diff --git a/bots/glambot/src/analyzers/accessibility.rs b/bots/glambot/src/analyzers/accessibility.rs index 29c1e1f6..c39c9de5 100644 --- a/bots/glambot/src/analyzers/accessibility.rs +++ b/bots/glambot/src/analyzers/accessibility.rs @@ -5,18 +5,21 @@ use super::{AnalysisResult, Analyzer, Finding, Severity}; use crate::config::Config; use crate::error::Result; +use regex::Regex; use std::path::Path; -use walkdir::WalkDir; use std::sync::LazyLock; -use regex::Regex; +use walkdir::WalkDir; // Compiled once. These were rebuilt on every call (hypatia expect_in_hot_path). -static MD_IMAGE_RE: LazyLock = - LazyLock::new(|| Regex::new(r"!\[(.*?)\]\((.*?)\)").expect("static markdown image regex literal is valid")); -static ADOC_IMAGE_RE: LazyLock = - LazyLock::new(|| Regex::new(r"image::(.*?)\[(.*?)\]").expect("static asciidoc image regex literal is valid")); -static MD_LINK_RE: LazyLock = - LazyLock::new(|| Regex::new(r"\[(.*?)\]\((.*?)\)").expect("static markdown link regex literal is valid")); +static MD_IMAGE_RE: LazyLock = LazyLock::new(|| { + Regex::new(r"!\[(.*?)\]\((.*?)\)").expect("static markdown image regex literal is valid") +}); +static ADOC_IMAGE_RE: LazyLock = LazyLock::new(|| { + Regex::new(r"image::(.*?)\[(.*?)\]").expect("static asciidoc image regex literal is valid") +}); +static MD_LINK_RE: LazyLock = LazyLock::new(|| { + Regex::new(r"\[(.*?)\]\((.*?)\)").expect("static markdown link regex literal is valid") +}); /// Accessibility analyzer pub struct AccessibilityAnalyzer; @@ -64,8 +67,7 @@ impl Analyzer for AccessibilityAnalyzer { // Add alt text placeholders to images in markdown files if let Some(ref file) = finding.file { if let Ok(content) = std::fs::read_to_string(file) { - let updated = content - .replace("![](", "![TODO: Add alt text]("); + let updated = content.replace("![](", "![TODO: Add alt text]("); if updated != content { std::fs::write(file, &updated)?; fixes.push(format!( @@ -131,7 +133,6 @@ impl Analyzer for AccessibilityAnalyzer { impl AccessibilityAnalyzer { fn check_image_alt_text(&self, path: &Path, config: &Config, result: &mut AnalysisResult) { - let md_image_re = &*MD_IMAGE_RE; let adoc_image_re = &*ADOC_IMAGE_RE; @@ -162,7 +163,9 @@ impl AccessibilityAnalyzer { ) .with_file(entry.path().to_path_buf()) .with_line(line_num + 1) - .with_suggestion("Add descriptive alt text: ![Description](image.png)"), + .with_suggestion( + "Add descriptive alt text: ![Description](image.png)", + ), ); } else if alt_text.len() < 3 { result.add( @@ -192,7 +195,9 @@ impl AccessibilityAnalyzer { ) .with_file(entry.path().to_path_buf()) .with_line(line_num + 1) - .with_suggestion("Add descriptive alt text: image::file[Description]"), + .with_suggestion( + "Add descriptive alt text: image::file[Description]", + ), ); } } @@ -246,7 +251,6 @@ impl AccessibilityAnalyzer { } fn check_link_text(&self, path: &Path, config: &Config, result: &mut AnalysisResult) { - let md_link_re = &*MD_LINK_RE; let non_descriptive = ["click here", "here", "read more", "link", "more"]; @@ -327,11 +331,7 @@ mod tests { let result = analyzer.analyze(dir.path(), &config).unwrap(); assert!(result.has_errors(), "Missing alt text should be an error"); - let error_ids: Vec<&str> = result - .errors() - .iter() - .map(|f| f.id.as_str()) - .collect(); + let error_ids: Vec<&str> = result.errors().iter().map(|f| f.id.as_str()).collect(); assert!( error_ids.contains(&"ACC-001"), "Should report ACC-001 for missing alt text" @@ -349,11 +349,7 @@ mod tests { let config = default_config(); let result = analyzer.analyze(dir.path(), &config).unwrap(); - let warning_ids: Vec<&str> = result - .warnings() - .iter() - .map(|f| f.id.as_str()) - .collect(); + let warning_ids: Vec<&str> = result.warnings().iter().map(|f| f.id.as_str()).collect(); assert!( warning_ids.contains(&"ACC-003"), "Should report ACC-003 for skipped heading level" @@ -370,11 +366,7 @@ mod tests { let config = default_config(); let result = analyzer.analyze(dir.path(), &config).unwrap(); - let warning_ids: Vec<&str> = result - .warnings() - .iter() - .map(|f| f.id.as_str()) - .collect(); + let warning_ids: Vec<&str> = result.warnings().iter().map(|f| f.id.as_str()).collect(); assert!( warning_ids.contains(&"ACC-004"), "Should report ACC-004 for non-descriptive link text" @@ -398,7 +390,9 @@ mod tests { let fixes = analyzer.fix(dir.path(), &config, &findings).unwrap(); assert!( - fixes.iter().any(|f| f.contains("WCAG compliance checklist")), + fixes + .iter() + .any(|f| f.contains("WCAG compliance checklist")), "Should generate WCAG checklist" ); assert!( diff --git a/bots/glambot/src/analyzers/git_seo_integration.rs b/bots/glambot/src/analyzers/git_seo_integration.rs index 897386e1..c59f93b2 100644 --- a/bots/glambot/src/analyzers/git_seo_integration.rs +++ b/bots/glambot/src/analyzers/git_seo_integration.rs @@ -143,10 +143,7 @@ impl Analyzer for GitSeoAnalyzer { if !funding_path.exists() { let content = "# SPDX-License-Identifier: MPL-2.0\n# Funding information\n# github: hyperpolymath\n"; std::fs::write(&funding_path, content)?; - fixes.push(format!( - "Created FUNDING.yml at {}", - funding_path.display() - )); + fixes.push(format!("Created FUNDING.yml at {}", funding_path.display())); } // If git-seo is installed, also run the tool @@ -246,12 +243,8 @@ fn run_git_seo_apply(repo_url: &str) -> Result { /// Process git-seo JSON report into glambot findings fn process_seo_report(report: &serde_json::Value, result: &mut AnalysisResult) { // Extract overall score - let score = report["scores"]["total"] - .as_f64() - .unwrap_or(0.0); - let percentage = report["scores"]["percentage"] - .as_f64() - .unwrap_or(0.0); + let score = report["scores"]["total"].as_f64().unwrap_or(0.0); + let percentage = report["scores"]["percentage"].as_f64().unwrap_or(0.0); // Create finding based on score let severity = if percentage >= 80.0 { @@ -347,7 +340,11 @@ mod tests { let result = GitSeoAnalyzer::default() .analyze_with(dir.path(), &default_config(), true) .unwrap(); - let gs003: Vec<_> = result.findings.iter().filter(|f| f.id == "GS-003").collect(); + let gs003: Vec<_> = result + .findings + .iter() + .filter(|f| f.id == "GS-003") + .collect(); assert_eq!(gs003.len(), 1, "a repo with no remote reports GS-003"); assert_eq!( gs003[0].file.as_deref(), @@ -365,7 +362,11 @@ mod tests { assert!( result.findings.is_empty(), "tool-availability advice is not a repo finding, got: {:?}", - result.findings.iter().map(|f| f.id.as_str()).collect::>() + result + .findings + .iter() + .map(|f| f.id.as_str()) + .collect::>() ); assert_eq!( result.skipped.as_deref(), @@ -383,7 +384,10 @@ mod tests { .analyze_with(dir.path(), &default_config(), true) .unwrap(); assert!(result.skipped.is_none()); - assert!(serde_json::to_value(&result).unwrap().get("skipped").is_none()); + assert!(serde_json::to_value(&result) + .unwrap() + .get("skipped") + .is_none()); } #[test] @@ -401,10 +405,7 @@ mod tests { process_seo_report(&report, &mut result); // High score should produce Info severity, not Error - assert!( - !result.has_errors(), - "High score should not produce errors" - ); + assert!(!result.has_errors(), "High score should not produce errors"); let finding = &result.findings[0]; assert_eq!(finding.severity, Severity::Info); } diff --git a/bots/glambot/src/analyzers/machine.rs b/bots/glambot/src/analyzers/machine.rs index 9b6ee704..f5efaef5 100644 --- a/bots/glambot/src/analyzers/machine.rs +++ b/bots/glambot/src/analyzers/machine.rs @@ -478,7 +478,10 @@ mod tests { assert!(!f.fixable, "an empty dir is untracked by git; not fixable"); let findings = vec![f.clone()]; let fixes = analyzer.fix(dir.path(), &config, &findings).unwrap(); - assert_eq!(fixes, vec!["No machine-readability fixes applicable".to_string()]); + assert_eq!( + fixes, + vec!["No machine-readability fixes applicable".to_string()] + ); assert!(!dir.path().join(".machine_readable").exists()); } @@ -573,8 +576,15 @@ mod tests { #[test] fn valid_trailing_commas_still_work() { - for body in ["{\"a\":1,}", "[1,2,]", "{\"a\":[1,],}", "{\"a\":1 , \n}", "[\"x\", /* c */ ]"] { - let v: std::result::Result = serde_json::from_str(&strip_jsonc(body)); + for body in [ + "{\"a\":1,}", + "[1,2,]", + "{\"a\":[1,],}", + "{\"a\":1 , \n}", + "[\"x\", /* c */ ]", + ] { + let v: std::result::Result = + serde_json::from_str(&strip_jsonc(body)); assert!(v.is_ok(), "{body}"); } } @@ -587,7 +597,10 @@ mod tests { std::fs::write(root.join("config.json"), JSONC).unwrap(); assert!(ids(&root).contains(&"MACH-001".to_string())); assert!(!is_tool_demanded_jsonc(&root, &root.join("config.json"))); - assert!(is_tool_demanded_jsonc(&root, &root.join(".vscode/settings.json"))); + assert!(is_tool_demanded_jsonc( + &root, + &root.join(".vscode/settings.json") + )); } #[test] diff --git a/bots/glambot/src/analyzers/mod.rs b/bots/glambot/src/analyzers/mod.rs index b9d1f91e..a864d091 100644 --- a/bots/glambot/src/analyzers/mod.rs +++ b/bots/glambot/src/analyzers/mod.rs @@ -2,11 +2,11 @@ // SPDX-FileCopyrightText: 2025 Jonathan D.A. Jewell //! Analyzers for presentation quality validation -pub mod visual; pub mod accessibility; -pub mod seo; -pub mod machine; pub mod git_seo_integration; +pub mod machine; +pub mod seo; +pub mod visual; use crate::config::Config; use crate::error::Result; diff --git a/bots/glambot/src/analyzers/seo.rs b/bots/glambot/src/analyzers/seo.rs index cf96ed92..9faffa9b 100644 --- a/bots/glambot/src/analyzers/seo.rs +++ b/bots/glambot/src/analyzers/seo.rs @@ -149,9 +149,7 @@ impl Analyzer for SeoAnalyzer { impl SeoAnalyzer { fn check_repo_description(&self, path: &Path, result: &mut AnalysisResult) { let readme_files = ["README.md", "README.adoc"]; - let found = readme_files - .iter() - .find(|name| path.join(name).exists()); + let found = readme_files.iter().find(|name| path.join(name).exists()); if let Some(readme) = found { let readme_path = path.join(readme); @@ -202,7 +200,9 @@ impl SeoAnalyzer { "HTML file missing meta description tag.", ) .with_file(entry.path().to_path_buf()) - .with_suggestion("Add in "), + .with_suggestion( + "Add in ", + ), ); } @@ -215,7 +215,9 @@ impl SeoAnalyzer { "HTML file missing meta keywords tag.", ) .with_file(entry.path().to_path_buf()) - .with_suggestion("Add in "), + .with_suggestion( + "Add in ", + ), ); } } @@ -270,9 +272,7 @@ impl SeoAnalyzer { fn check_keywords(&self, path: &Path, result: &mut AnalysisResult) { let readme_files = ["README.md", "README.adoc"]; - let found = readme_files - .iter() - .find(|name| path.join(name).exists()); + let found = readme_files.iter().find(|name| path.join(name).exists()); if let Some(readme) = found { let readme_path = path.join(readme); @@ -358,11 +358,7 @@ mod tests { let result = analyzer.analyze(dir.path(), &config).unwrap(); // Should find missing description (SEO-001) - let ids: Vec<&str> = result - .findings - .iter() - .map(|f| f.id.as_str()) - .collect(); + let ids: Vec<&str> = result.findings.iter().map(|f| f.id.as_str()).collect(); assert!( ids.contains(&"SEO-001"), "Should report SEO-001 for short description" @@ -381,11 +377,7 @@ mod tests { let analyzer = SeoAnalyzer::default(); let result = analyzer.analyze(dir.path(), &config).unwrap(); - let ids: Vec<&str> = result - .findings - .iter() - .map(|f| f.id.as_str()) - .collect(); + let ids: Vec<&str> = result.findings.iter().map(|f| f.id.as_str()).collect(); assert!( ids.contains(&"SEO-002"), "Should report SEO-002 for missing meta description" diff --git a/bots/glambot/src/analyzers/visual.rs b/bots/glambot/src/analyzers/visual.rs index e2ee6da5..3590b7ea 100644 --- a/bots/glambot/src/analyzers/visual.rs +++ b/bots/glambot/src/analyzers/visual.rs @@ -67,7 +67,10 @@ impl Analyzer for VisualAnalyzer { repo_name ); std::fs::write(&readme_path, template)?; - fixes.push(format!("Created README.adoc template at {}", readme_path.display())); + fixes.push(format!( + "Created README.adoc template at {}", + readme_path.display() + )); } } "VIS-004" => { @@ -85,12 +88,20 @@ impl Analyzer for VisualAnalyzer { }; // Insert badges after the first heading let updated = if let Some(pos) = content.find('\n') { - format!("{}{}{}", &content[..pos + 1], badge_section, &content[pos + 1..]) + format!( + "{}{}{}", + &content[..pos + 1], + badge_section, + &content[pos + 1..] + ) } else { format!("{}{}", content, badge_section) }; std::fs::write(&readme_path, updated)?; - fixes.push(format!("Added badges section to {}", readme_path.display())); + fixes.push(format!( + "Added badges section to {}", + readme_path.display() + )); } } break; @@ -107,7 +118,10 @@ impl Analyzer for VisualAnalyzer { if !placeholder_path.exists() { let content = "# Screenshots\n\nTODO: Add screenshots or visual documentation here.\n\n## UI Preview\n\n\n"; std::fs::write(&placeholder_path, content)?; - fixes.push(format!("Created screenshot placeholder at {}", placeholder_path.display())); + fixes.push(format!( + "Created screenshot placeholder at {}", + placeholder_path.display() + )); } } _ => {} @@ -125,9 +139,7 @@ impl Analyzer for VisualAnalyzer { impl VisualAnalyzer { fn check_readme(&self, path: &Path, _config: &Config, result: &mut AnalysisResult) { let readme_files = ["README.md", "README.adoc", "README.rst", "README.txt"]; - let found = readme_files - .iter() - .find(|name| path.join(name).exists()); + let found = readme_files.iter().find(|name| path.join(name).exists()); match found { None => { @@ -154,17 +166,22 @@ impl VisualAnalyzer { "VIS-002", "README too short", Severity::Warning, - &format!("README is only {} characters. Should be more descriptive.", content.len()), + &format!( + "README is only {} characters. Should be more descriptive.", + content.len() + ), ) .with_file(readme_path.clone()) - .with_suggestion("Expand README with project description, usage, installation"), + .with_suggestion( + "Expand README with project description, usage, installation", + ), ); } // Check for title/heading - let has_title = content.lines().any(|line| { - line.starts_with("# ") || line.starts_with("= ") - }); + let has_title = content + .lines() + .any(|line| line.starts_with("# ") || line.starts_with("= ")); if !has_title { result.add( @@ -185,9 +202,7 @@ impl VisualAnalyzer { fn check_badges(&self, path: &Path, result: &mut AnalysisResult) { let readme_files = ["README.md", "README.adoc"]; - let found = readme_files - .iter() - .find(|name| path.join(name).exists()); + let found = readme_files.iter().find(|name| path.join(name).exists()); if let Some(readme) = found { let readme_path = path.join(readme); @@ -203,7 +218,9 @@ impl VisualAnalyzer { "README does not contain any badges (build status, coverage, etc.).", ) .with_file(readme_path) - .with_suggestion("Add badges for build status, test coverage, license, etc."), + .with_suggestion( + "Add badges for build status, test coverage, license, etc.", + ), ); } } @@ -323,11 +340,7 @@ mod tests { // Should find missing README (VIS-001) which is an error assert!(result.has_errors(), "Bare repo should have errors"); - let error_ids: Vec<&str> = result - .errors() - .iter() - .map(|f| f.id.as_str()) - .collect(); + let error_ids: Vec<&str> = result.errors().iter().map(|f| f.id.as_str()).collect(); assert!( error_ids.contains(&"VIS-001"), "Should report missing README" @@ -344,11 +357,7 @@ mod tests { let config = default_config(); let result = analyzer.analyze(dir.path(), &config).unwrap(); - let warning_ids: Vec<&str> = result - .warnings() - .iter() - .map(|f| f.id.as_str()) - .collect(); + let warning_ids: Vec<&str> = result.warnings().iter().map(|f| f.id.as_str()).collect(); assert!( warning_ids.contains(&"VIS-002"), "Short README should trigger VIS-002 warning" diff --git a/bots/glambot/src/config/mod.rs b/bots/glambot/src/config/mod.rs index 4b90c21f..2dd55bfc 100644 --- a/bots/glambot/src/config/mod.rs +++ b/bots/glambot/src/config/mod.rs @@ -109,8 +109,8 @@ impl Default for SeoConfig { require_opengraph: false, check_titles: true, check_descriptions: true, - enable_git_seo: Some(true), // Enable by default - enable_auto_apply: Some(false), // Safety: manual by default + enable_git_seo: Some(true), // Enable by default + enable_auto_apply: Some(false), // Safety: manual by default } } } diff --git a/bots/glambot/src/lib.rs b/bots/glambot/src/lib.rs index 0100e162..e599600d 100644 --- a/bots/glambot/src/lib.rs +++ b/bots/glambot/src/lib.rs @@ -21,8 +21,8 @@ pub mod fleet; pub use analyzers::{ accessibility::AccessibilityAnalyzer, git_seo_integration::GitSeoAnalyzer, - machine::MachineAnalyzer, seo::SeoAnalyzer, visual::VisualAnalyzer, AnalysisResult, - Analyzer, AuditResult, Finding, Severity, + machine::MachineAnalyzer, seo::SeoAnalyzer, visual::VisualAnalyzer, AnalysisResult, Analyzer, + AuditResult, Finding, Severity, }; pub use config::Config; pub use error::{GlambotError, Result}; diff --git a/bots/glambot/src/main.rs b/bots/glambot/src/main.rs index cb9aa440..c350fc4e 100644 --- a/bots/glambot/src/main.rs +++ b/bots/glambot/src/main.rs @@ -174,7 +174,10 @@ fn handle_show(config: &config::Config) -> ExitCode { println!("Accessibility:"); println!(" WCAG level: {:?}", config.accessibility.wcag_level); - println!(" Require alt text: {}", config.accessibility.require_alt_text); + println!( + " Require alt text: {}", + config.accessibility.require_alt_text + ); println!(); println!("SEO:"); @@ -184,7 +187,10 @@ fn handle_show(config: &config::Config) -> ExitCode { println!("Machine:"); println!(" Validate JSON: {}", config.machine.validate_json); - println!(" Require schema.org: {}", config.machine.require_schema_org); + println!( + " Require schema.org: {}", + config.machine.require_schema_org + ); println!(); println!("General:"); @@ -197,8 +203,7 @@ fn handle_show(config: &config::Config) -> ExitCode { fn handle_audit(path: &PathBuf, config: &config::Config, strict: bool, format: &str) -> ExitCode { use crate::analyzers::{ accessibility::AccessibilityAnalyzer, git_seo_integration::GitSeoAnalyzer, - machine::MachineAnalyzer, seo::SeoAnalyzer, visual::VisualAnalyzer, Analyzer, - AuditResult, + machine::MachineAnalyzer, seo::SeoAnalyzer, visual::VisualAnalyzer, Analyzer, AuditResult, }; let mut result = AuditResult::default(); diff --git a/bots/glambot/tests/integration.rs b/bots/glambot/tests/integration.rs index b5390e3b..258b8ea8 100644 --- a/bots/glambot/tests/integration.rs +++ b/bots/glambot/tests/integration.rs @@ -139,9 +139,8 @@ fn test_all_analyzers_deficient_repo() { ); // Verify correct total count - let total = visual_result.findings.len() - + acc_result.findings.len() - + machine_result.findings.len(); + let total = + visual_result.findings.len() + acc_result.findings.len() + machine_result.findings.len(); assert!( total >= 4, "Should have at least 4 total findings across analyzers, got {}", @@ -191,11 +190,7 @@ fn test_finding_severity_levels() { let result = machine.analyze(dir.path(), &config).unwrap(); // Verify we get different severity levels - let severities: Vec = result - .findings - .iter() - .map(|f| f.severity) - .collect(); + let severities: Vec = result.findings.iter().map(|f| f.severity).collect(); // Invalid JSON should be Error assert!( @@ -237,7 +232,7 @@ fn json_audit_stdout_is_only_the_report() { .output() .unwrap(); let text = String::from_utf8(out.stdout).unwrap(); - let parsed: serde_json::Value = - serde_json::from_str(&text).unwrap_or_else(|e| panic!("stdout is not pure JSON ({e}): {text}")); + let parsed: serde_json::Value = serde_json::from_str(&text) + .unwrap_or_else(|e| panic!("stdout is not pure JSON ({e}): {text}")); assert!(parsed.is_object()); }