diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 16a05b0..bf3ff87 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -2009,6 +2009,45 @@ history and in a CI log. It stays in the environment and is deliberately not a rule field: a bypass written into the policy file would be committed, reviewed once, and permanent. +### Recording what a run decided + +```sh +UPHOLD_VERDICTS=/path/run.jsonl uphold guard --stage pre-push +``` + +With the variable set, each rule the stage evaluated appends one JSON line to +the file it names. Unset, nothing is written. + +```json +{"schema_version":1,"seam":"guard","stage":"pre-push","rule":"prevent-public-push","set":"unowned-push","outcome":"pass"} +{"schema_version":1,"seam":"guard","stage":"pre-push","rule":"no-local-merge","set":"unreviewed-history","outcome":"bypass","by":"UPHOLD_ALLOW"} +``` + +| field | holds | +|---|---| +| `schema_version` | `1`. Bumped when a field changes meaning or goes away, never when one is added | +| `seam` | `guard` | +| `stage` | the stage the run was told it is at | +| `rule` | the rule's id | +| `set` | the bundled set the rule came from; absent for a rule the policy declares itself | +| `outcome` | `pass`, `refuse`, `bypass` or `unknown` | +| `by` | `UPHOLD_ALLOW`, on a bypass and only there | + +- **No subject.** A line never carries the message, the diff or the refs the + rule read, so the file never holds the text a refusal was protecting. +- **Appended, never replaced.** The file belongs to the caller; two runs + sharing it interleave whole lines. +- **`unknown` is the last line.** A rule that could not look ends the run with + exit `2`, and its line says which rule that was. +- **A file that cannot be written is exit `2`**, checked before any rule runs. + So is a variable set to nothing: a caller who asked for verdicts and got + none must not read the absence as a clean run. + +uphold records nothing itself. Signing the lines, storing them beside the +commit and publishing them belong to whatever ran the gate. A commit pushed +with `--no-verify` ran no hook and so has no lines, which a verifier +expecting a record on every pushed commit sees as the record's absence. + ## `uphold shim` — the shims A pull-request body is typed into a CLI and goes straight to a public API diff --git a/src/guard/mod.rs b/src/guard/mod.rs index a4a44d5..1b7c4f8 100644 --- a/src/guard/mod.rs +++ b/src/guard/mod.rs @@ -28,6 +28,7 @@ use std::path::Path; use crate::config::{CheckKind, Policy, Rule}; use crate::error::{Exit, Fatal, Result}; +use crate::verdicts::Outcome; /// Every built-in check this binary carries, and the only list of them. /// @@ -647,8 +648,12 @@ pub(crate) fn parameters(builtin: &str) -> &'static [&'static str] { /// Run every guard the policy declares that has something to say at `stage`. pub(crate) fn run(root: &Path, policy: &Policy, request: &RunRequest<'_>) -> Result { + // Opened before anything is decided, so a file that cannot be written stops + // the run while it has decided nothing. See `verdicts`. + let mut verdicts = crate::verdicts::Sink::from_env()?; + let stage = request.stage.as_str(); // Every rule the CONFIG puts at this hook, and nothing else decides it. - let at_hook: Vec<&Rule> = policy.at_hook(request.stage.as_str()).collect(); + let at_hook: Vec<&Rule> = policy.at_hook(stage).collect(); if policy.at_hook_any().next().is_none() { println!("no rule declares a git hook"); return Ok(Exit::Clean); @@ -658,11 +663,13 @@ pub(crate) fn run(root: &Path, policy: &Policy, request: &RunRequest<'_>) -> Res let mut ran = 0usize; let mut bypassed_here = 0usize; for rule in at_hook { + let set = policy.set_of(&rule.id); // Counted where the work happens, not where the loop starts. A bypassed // rule did not run, and folding it into the same number reported a // deliberately disabled check as one that passed. if bypassed(rule.id.as_str()) { eprintln!("uphold guard: {} bypassed by UPHOLD_ALLOW", rule.id); + verdicts.record(stage, &rule.id, set, Outcome::Bypass)?; bypassed_here += 1; continue; } @@ -678,8 +685,23 @@ pub(crate) fn run(root: &Path, policy: &Policy, request: &RunRequest<'_>) -> Res remote_name: request.remote_name, remote_url: request.remote_url, }; - if let Some(refusal) = evaluate(&one)? { - refusals.push(refusal); + match evaluate(&one) { + Ok(Some(refusal)) => { + verdicts.record(stage, &rule.id, set, Outcome::Refuse)?; + refusals.push(refusal); + } + Ok(None) => verdicts.record(stage, &rule.id, set, Outcome::Pass)?, + // The rule could not look, and the run ends here with exit 2. The + // line says which rule that was; the error the reader acts on is + // still the rule's own, with the file's appended if both failed. + Err(error) => { + return Err( + match verdicts.record(stage, &rule.id, set, Outcome::Unknown) { + Ok(()) => error, + Err(also) => Fatal::new(format!("{error}\n\n{also}")), + }, + ); + } } } @@ -701,12 +723,9 @@ pub(crate) fn run(root: &Path, policy: &Policy, request: &RunRequest<'_>) -> Res } if bypassed_here > 0 { - println!( - "{ran} guard(s) passed at {}, {bypassed_here} bypassed by UPHOLD_ALLOW", - request.stage.as_str() - ); + println!("{ran} guard(s) passed at {stage}, {bypassed_here} bypassed by UPHOLD_ALLOW"); } else { - println!("{ran} guard(s) passed at {}", request.stage.as_str()); + println!("{ran} guard(s) passed at {stage}"); } Ok(Exit::Clean) } diff --git a/src/main.rs b/src/main.rs index dc31399..981413f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -104,6 +104,7 @@ mod shim; mod sources; mod supply; mod text; +mod verdicts; use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; @@ -167,6 +168,7 @@ exit codes: 2 the check could not be made (bad policy, missing source, unreadable file) UPHOLD_ALLOW=, bypasses named guards for one invocation. +UPHOLD_VERDICTS=FILE appends one JSON line per guard a `guard` run evaluated. "; /// The policy file, newest name first. diff --git a/src/verdicts.rs b/src/verdicts.rs new file mode 100644 index 0000000..d47f094 --- /dev/null +++ b/src/verdicts.rs @@ -0,0 +1,177 @@ +//! `UPHOLD_VERDICTS` -- what a guard run decided, for whoever ran it. +//! +//! uphold decides and does not record. Signing a pass, storing it beside the +//! commit and publishing it are a recorder's, and a recorder already knows +//! where its record goes; what it cannot know without parsing prose is which +//! rules ran and what each one answered. So the caller names a file, and each +//! rule the run evaluated appends one JSON line to it. Unset, nothing is +//! written and the run is the run it always was. +//! +//! A line carries the decision and never the subject: no message, no body, no +//! argv. A record of a refusal that quoted what was refused would hold exactly +//! the text the rule existed to keep out of somewhere. +//! +//! A file that cannot be written is exit 2. A caller who asked for verdicts and +//! received none must not read the absence as a run that found nothing. + +use std::fs::{File, OpenOptions}; +use std::io::Write; +use std::path::PathBuf; + +use serde::Serialize; + +use crate::error::{Fatal, Result}; + +/// The variable naming the file. A path, never a descriptor: a hook runner +/// owns the descriptors a hook inherits, and a path survives every one of them. +pub(crate) const VARIABLE: &str = "UPHOLD_VERDICTS"; + +/// Bumped when a field changes meaning or goes away, never when one is added. +/// The same contract `uphold check --json` keeps. +const SCHEMA_VERSION: u32 = 1; + +/// The four answers a rule can give, and no fifth: they are the exit codes and +/// the bypass, named. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Outcome { + /// Ran, and found nothing to refuse. + Pass, + /// Ran, and refused. + Refuse, + /// Did not run, because `UPHOLD_ALLOW` named it for this invocation. + Bypass, + /// Could not look. The run stops here with exit 2, so a line with this + /// outcome is the last one the run writes. + Unknown, +} + +impl Outcome { + const fn name(self) -> &'static str { + match self { + Self::Pass => "pass", + Self::Refuse => "refuse", + Self::Bypass => "bypass", + Self::Unknown => "unknown", + } + } +} + +/// One line, in the order a reader scans it. +#[derive(Serialize)] +struct Line<'a> { + schema_version: u32, + seam: &'a str, + stage: &'a str, + rule: &'a str, + #[serde(skip_serializing_if = "Option::is_none")] + set: Option<&'a str>, + outcome: &'a str, + /// What switched the rule off, present on a bypass and only there. + #[serde(skip_serializing_if = "Option::is_none")] + by: Option<&'a str>, +} + +/// Where this run's verdicts go, if anywhere. +pub(crate) struct Sink { + open: Option<(PathBuf, File)>, +} + +impl Sink { + /// The file the caller named, opened for appending before any rule runs, + /// so a path that cannot be written stops the run before it decides + /// anything rather than after it decided everything. + pub(crate) fn from_env() -> Result { + let Some(path) = std::env::var_os(VARIABLE) else { + return Ok(Self { open: None }); + }; + // Refused rather than read as unset. A caller who exported the + // variable and blanked it asked for verdicts somewhere; running on as + // if they had not is the silent absence this file exists to rule out. + if path.is_empty() { + return Err(Fatal::new(format!( + "{VARIABLE} is set and names no file. Unset it to write no verdicts" + ))); + } + let path = PathBuf::from(path); + let file = OpenOptions::new() + .create(true) + .append(true) + .open(&path) + .map_err(|error| Fatal::at(&path, format!("{VARIABLE}: {error}")))?; + Ok(Self { + open: Some((path, file)), + }) + } + + /// Append one rule's answer. One `write_all` per line on a file opened for + /// appending, so two runs sharing a file interleave whole lines. + pub(crate) fn record( + &mut self, + stage: &str, + rule: &str, + set: Option<&str>, + outcome: Outcome, + ) -> Result<()> { + let Some((path, file)) = &mut self.open else { + return Ok(()); + }; + let line = Line { + schema_version: SCHEMA_VERSION, + seam: "guard", + stage, + rule, + set, + outcome: outcome.name(), + by: (outcome == Outcome::Bypass).then_some("UPHOLD_ALLOW"), + }; + let mut bytes = serde_json::to_vec(&line) + .map_err(|error| Fatal::at(path, format!("{VARIABLE}: {error}")))?; + bytes.push(b'\n'); + file.write_all(&bytes) + .map_err(|error| Fatal::at(path, format!("{VARIABLE}: {error}"))) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn written(outcome: Outcome, set: Option<&str>) -> serde_json::Value { + let directory = crate::fixture::scratch("verdicts"); + std::fs::create_dir_all(&directory).unwrap(); + let path = directory.join("run.jsonl"); + let file = OpenOptions::new() + .create(true) + .append(true) + .open(&path) + .unwrap(); + let mut sink = Sink { + open: Some((path.clone(), file)), + }; + sink.record("pre-push", "a-rule", set, outcome).unwrap(); + let text = std::fs::read_to_string(&path).unwrap(); + assert!(text.ends_with('\n') && text.lines().count() == 1, "{text}"); + serde_json::from_str(&text).unwrap() + } + + #[test] + fn a_bypass_names_what_switched_the_rule_off_and_a_pass_does_not() { + let bypass = written(Outcome::Bypass, Some("a-set")); + assert_eq!(bypass["outcome"], "bypass"); + assert_eq!(bypass["by"], "UPHOLD_ALLOW"); + assert_eq!(bypass["set"], "a-set"); + let pass = written(Outcome::Pass, None); + assert_eq!(pass["outcome"], "pass"); + assert!(pass.get("by").is_none(), "{pass}"); + assert!(pass.get("set").is_none(), "{pass}"); + assert_eq!(pass["schema_version"], 1); + assert_eq!(pass["seam"], "guard"); + } + + #[test] + fn a_sink_nobody_asked_for_writes_nothing() { + let mut sink = Sink { open: None }; + sink.record("pre-push", "a-rule", None, Outcome::Refuse) + .unwrap(); + } +} diff --git a/tests/guard_cli.rs b/tests/guard_cli.rs index e1e9120..1b7ea95 100644 --- a/tests/guard_cli.rs +++ b/tests/guard_cli.rs @@ -142,6 +142,118 @@ fn a_bypass_for_one_guard_does_not_release_another() { assert_eq!(code(&output), 1, "{}", stderr(&output)); } +const AI_AUTHOR_AND_ASCII_SUBJECT: &str = r#" +[rule.prevent-ai-author] +builtin = "prevent-ai-author" + +[rule.prevent-ai-author.git] +hooks = ["commit-msg"] + +[rule.ascii-only-commit-subject] +builtin = "ascii-only-commit-subject" + +[rule.ascii-only-commit-subject.git] +hooks = ["commit-msg"] +"#; + +fn guard_with_verdicts(root: &Path, verdicts: &Path, allow: Option<&str>) -> Output { + let mut command = Command::new(env!("CARGO_BIN_EXE_uphold")); + command + .args(["guard", "--stage", "commit-msg", "--message", "msg.txt"]) + .current_dir(root) + .env("UPHOLD_VERDICTS", verdicts) + .env_remove("UPHOLD_ALLOW"); + if let Some(allow) = allow { + command.env("UPHOLD_ALLOW", allow); + } + command.output().unwrap() +} + +fn verdict_lines(path: &Path) -> Vec { + std::fs::read_to_string(path) + .unwrap() + .lines() + .map(|line| serde_json::from_str(line).unwrap()) + .collect() +} + +fn outcome_of<'a>(lines: &'a [serde_json::Value], rule: &str) -> Vec<&'a str> { + lines + .iter() + .filter(|line| line["rule"] == rule) + .map(|line| line["outcome"].as_str().unwrap()) + .collect() +} + +#[test] +fn verdicts_name_each_rules_answer_and_never_the_subject() { + let root = repository(AI_AUTHOR_AND_ASCII_SUBJECT); + write(&root, "msg.txt", "x\n\nGenerated with Claude Code\n"); + let verdicts = root.join("verdicts.jsonl"); + + let output = guard_with_verdicts(&root, &verdicts, None); + assert_eq!(code(&output), 1, "{}", stderr(&output)); + // A second run appends: the file is the caller's, and what it already + // holds is not this run's to replace. + let output = guard_with_verdicts(&root, &verdicts, Some("prevent-ai-author")); + assert_eq!(code(&output), 0, "{}", stderr(&output)); + + let lines = verdict_lines(&verdicts); + assert_eq!(lines.len(), 4, "{lines:?}"); + assert_eq!( + outcome_of(&lines, "prevent-ai-author"), + ["refuse", "bypass"] + ); + assert_eq!( + outcome_of(&lines, "ascii-only-commit-subject"), + ["pass", "pass"] + ); + for line in &lines { + assert_eq!(line["schema_version"], 1, "{line}"); + assert_eq!(line["seam"], "guard", "{line}"); + assert_eq!(line["stage"], "commit-msg", "{line}"); + assert_eq!( + line.get("by").is_some(), + line["outcome"] == "bypass", + "{line}" + ); + } + let text = std::fs::read_to_string(&verdicts).unwrap(); + assert!(!text.contains("Generated"), "{text}"); +} + +#[test] +fn a_verdicts_file_that_cannot_be_opened_stops_the_run_before_it_decides() { + let root = repository(AI_AUTHOR); + write(&root, "msg.txt", "x\n\nGenerated with Claude Code\n"); + // A directory where the file should be: nothing can append to it. + let output = guard_with_verdicts(&root, &root, None); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains("UPHOLD_VERDICTS"), + "{}", + stderr(&output) + ); + assert!( + !stderr(&output).contains("guard refused"), + "{}", + stderr(&output) + ); +} + +#[test] +fn an_empty_verdicts_variable_is_refused_and_not_read_as_unset() { + let root = repository(AI_AUTHOR); + write(&root, "msg.txt", "Fix the thing\n"); + let output = guard_with_verdicts(&root, Path::new(""), None); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains("names no file"), + "{}", + stderr(&output) + ); +} + #[test] fn a_guard_is_only_asked_at_a_stage_it_can_observe() { // Asked at a stage it cannot see, a guard has not passed -- it has not run.