diff --git a/Cargo.lock b/Cargo.lock index e20ff9a..242df5e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -63,6 +63,12 @@ dependencies = [ "serde_core", ] +[[package]] +name = "bumpalo" +version = "3.20.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72f5acc6cb2ba439de613abc23857ec3d78374d8ed5ac84e9d11336e87da8649" + [[package]] name = "bytes" version = "1.12.1" @@ -85,6 +91,15 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +[[package]] +name = "chrono" +version = "0.4.45" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1aa79e62e7697b8e29b513a68abacf485adcd1fe8284a4316c5ae868e6633327" +dependencies = [ + "num-traits", +] + [[package]] name = "core_detect" version = "1.0.0" @@ -161,6 +176,30 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d45db016d36b838f563236e9193d0ee6ce38f3f68b6c94e914b4929c96bbb890" +[[package]] +name = "futures-core" +version = "0.3.34" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92d699e522242e69e3003b94ecc1f960f3a5e015aa7c5d7486e65ad01dd94f5e" + +[[package]] +name = "futures-task" +version = "0.3.34" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd417de3d1d015fc3bfd2b1ea46dfc7bab72ef86f1cc7cc9c78e728b34a6d1fd" + +[[package]] +name = "futures-util" +version = "0.3.34" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0d50a92467f8ba5dd6e3ee5d4bd04d73ab2e4e1c44474a0674821dfce14b79bc" +dependencies = [ + "futures-core", + "futures-task", + "pin-project-lite", + "slab", +] + [[package]] name = "getrandom" version = "0.2.17" @@ -286,6 +325,17 @@ version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" +[[package]] +name = "js-sys" +version = "0.3.106" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7883d941dae510fb2d978fc3fe018c71c9e2892fd38854de3e8b92c2e5ad9cc5" +dependencies = [ + "cfg-if", + "futures-util", + "wasm-bindgen", +] + [[package]] name = "libc" version = "0.2.189" @@ -325,6 +375,15 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "743fb55ba31b18fb1ecef6bdc9aa2743314978ac084044301a7eee33fb99a20d" +[[package]] +name = "newtype-uuid" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0fa56da58097c7ae893c4097bb6d91bb5f7fa330053e07e033349db4d5f83ba8" +dependencies = [ + "uuid", +] + [[package]] name = "nohash-hasher" version = "0.2.0" @@ -390,6 +449,12 @@ dependencies = [ "siphasher", ] +[[package]] +name = "pin-project-lite" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" + [[package]] name = "ppv-lite86" version = "0.2.21" @@ -417,6 +482,30 @@ dependencies = [ "regex", ] +[[package]] +name = "quick-junit" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bae23b8e1a59a90f7bf98caa2f4c8c2eedf6df011c6c88ed633e85a87c84526e" +dependencies = [ + "chrono", + "indexmap", + "newtype-uuid", + "quick-xml", + "strip-ansi-escapes", + "thiserror", + "uuid", +] + +[[package]] +name = "quick-xml" +version = "0.41.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e660451e55124f798a69a5af3f49ccfbefbd41910eefd25caf2393e1f3473ec1" +dependencies = [ + "memchr", +] + [[package]] name = "quote" version = "1.0.47" @@ -662,6 +751,12 @@ version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ee5873ec9cce0195efcb7a4e9507a04cd49aec9c83d0389df45b1ef7ba2e649" +[[package]] +name = "slab" +version = "0.4.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c790de23124f9ab44544d7ac05d60440adc586479ce501c1d6d7da3cd8c9cf5" + [[package]] name = "smallvec" version = "1.16.1" @@ -674,6 +769,15 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2b2231b7c3057d5e4ad0156fb3dc807d900806020c5ffa3ee6ff2c8c76fb8520" +[[package]] +name = "strip-ansi-escapes" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a8f8038e7e7969abb3f1b7c2a811225e9296da208539e0f79c5251d6cac0025" +dependencies = [ + "vte", +] + [[package]] name = "subtle" version = "2.6.1" @@ -702,6 +806,26 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "thiserror" +version = "2.0.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09e52cb86a36cede5cb101bf8908837b3e4c6e5e59fe7fd85c23fb56200d189e" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "2.0.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe5197923287db20a58125f0bc85c062f7f2c892de97b18c356f9efb14b28524" +dependencies = [ + "proc-macro2", + "quote", + "syn 3.0.4", +] + [[package]] name = "tinyvec" version = "1.13.3" @@ -863,6 +987,7 @@ checksum = "8ecb6da28b8a351d773b68d5825ac39017e680750f980f3a1a85cd8dd28a47c1" name = "uphold" version = "1.32.0" dependencies = [ + "chrono", "encoding_rs", "globset", "grep-matcher", @@ -870,6 +995,7 @@ dependencies = [ "grep-searcher", "ignore", "pyreq", + "quick-junit", "regex", "rustix", "serde", @@ -921,6 +1047,25 @@ version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b8c0a043c9540bae7c578c88f91dda8bd82e59ae27c21baca69c8b191aaf5a6e" +[[package]] +name = "uuid" +version = "1.28.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7cc1186384beb7dd8eedea376413fd654937285ea6c9cfbb928dc3043ea4b606" +dependencies = [ + "js-sys", + "wasm-bindgen", +] + +[[package]] +name = "vte" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "231fdcd7ef3037e8330d8e17e61011a2c244126acc0a982f4040ac3f9f0bc077" +dependencies = [ + "memchr", +] + [[package]] name = "walkdir" version = "2.5.0" @@ -937,6 +1082,51 @@ version = "0.11.1+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ccf3ec651a847eb01de73ccad15eb7d99f80485de043efb2f370cd654f4ea44b" +[[package]] +name = "wasm-bindgen" +version = "0.2.129" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9bb54f33acc68fd454578d9820b0bde1a1a3d17aa17bb7b6595806d02886d409" +dependencies = [ + "cfg-if", + "once_cell", + "rustversion", + "wasm-bindgen-macro", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-macro" +version = "0.2.129" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e29d0c35b16e224a7eeb5cd2d25e3e1968fbd65604117b44d3b789d00ee8535" +dependencies = [ + "quote", + "wasm-bindgen-macro-support", +] + +[[package]] +name = "wasm-bindgen-macro-support" +version = "0.2.129" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6f501a8bc3719dba86ef8ae4728879c08001bea749eb1333ac5b91e040e2a6b7" +dependencies = [ + "bumpalo", + "proc-macro2", + "quote", + "syn 3.0.4", + "wasm-bindgen-shared", +] + +[[package]] +name = "wasm-bindgen-shared" +version = "0.2.129" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23f0c9c52aa7cd7d77769a4cfe2a9adb1b331f489a41d912ce14513d5ab995c6" +dependencies = [ + "unicode-ident", +] + [[package]] name = "webpki-roots" version = "1.0.9" diff --git a/Cargo.toml b/Cargo.toml index 2101ef3..6c8fef2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -117,6 +117,21 @@ shell-words = "1" ureq = { version = "3.4.2", default-features = false, features = ["rustls"] } # Parse PEP 508 requirements before accepting the closed optional-extra form. pyreq = "0.2.0" +# JUnit XML, for `UPHOLD_JUNIT`: the evidence format the gate tier already +# writes, publishes and links, so a guard run is read by the same publisher with +# nothing of uphold's own to parse. A library rather than a hand-rolled writer +# because the format's escaping and its counts on every suite are what a reader +# trusts first and a writer gets wrong last. This one is nextest's own, and it +# writes a `hostname` only when one is put in `extra`, which this crate never +# does -- `no-running-os-identity-metadata` refuses a machine name in a file a +# gate publishes. `junit-report` was the alternative and is not used: it writes +# `hostname="localhost"` on every suite, with no way to leave it out. +quick-junit = "0.8" +# Already in the tree under quick-junit. Named here only to turn the clock into +# the suite's `timestamp`, the one piece of run metadata the file keeps; +# `std` is the feature that converts a `SystemTime`, and the default features, +# the local time zone among them, stay off. +chrono = { version = "0.4", default-features = false, features = ["std"] } [target.'cfg(unix)'.dependencies] # Git discovers hooks with access(X_OK), which checks the caller's permission diff --git a/docs/DESIGN.md b/docs/DESIGN.md index ec04b99..1059176 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -257,6 +257,39 @@ in the environment and is deliberately not a rule field: a bypass belongs to one invocation by whoever is running it, and written into the policy file it would be committed, reviewed once, and permanent, which is not a bypass. +### uphold decides; it does not record + +`--junit FILE`, or `UPHOLD_JUNIT` for a run whose command line the caller does +not own, names a file and uphold writes one JUnit document to it. The +format is not one of uphold's own because the fleet already has one evidence +format: a gate writes JUnit XML under `.test-evidence/` and a publisher turns +the directory into a report. The four testcase states are the four answers a +guard already gives, so publishers and report tools read the file with no glue +and nothing downstream learns a second schema. + +The caller names the file, rather than uphold printing to stdout or keeping a +file in a state directory. Stdout is already the prose a person reads and a +hook runner may swallow. A default path in a state directory would be shared by +every repository and every act on the machine, and would put a write somewhere +the caller did not choose. A named file lets the caller put it where its +publisher already looks, outside the tree or inside an ignored directory. + +The flag is the conventional spelling and the one a gate task types. The +variable exists because a hook's command line is the hook definition's, pinned +by the repository, so a caller running `git push` has no argv to add a flag +to; the environment is the one channel that reaches through git and the runner +to the guard, which is also why `UPHOLD_ALLOW` is a variable. + +`git commit --no-verify` and `git push --no-verify` run no hook, so no file is +written. That needs no code here: a verifier that expects evidence on every +pushed commit sees the absence, held on the remote and out of reach of the +machine that skipped the hooks. + +"Evidence" here means what it means in `.test-evidence/`. Inside uphold the word +also names a provider's facts to a policy, described in the section "Evidence: +what a provider reports and what a policy may read", and the two meanings do +not meet. + ## What an external provider must establish A trustworthy provider claim has a verifiable rule name, a demonstrated diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index bf3ff87..4b2f061 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -2009,44 +2009,100 @@ 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 +### Recording what a stage decided ```sh -UPHOLD_VERDICTS=/path/run.jsonl uphold guard --stage pre-push +uphold guard --stage manual --junit .test-evidence/check-guards/uphold.xml +UPHOLD_JUNIT=/abs/path/pre-push.xml git 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"} +Two spellings of one request. `--junit FILE` is the conventional one, as +`--junitxml` is pytest's, for a caller who owns the command line; a relative +value means the caller's directory, as it does for any other command. The +variable is for the caller who does not own it: a hook's command line belongs +to the hook definition the repository pins, and the environment is the only +channel that reaches from `git push` through the runner to the guard. The flag +wins when both are given. + +Neither given, nothing is written. Given, the run writes one JUnit XML document: one +`` with `package="uphold"`, and one +`` per rule the stage evaluated. An Allure reader files a testcase by +its `classname` and the suite by its `package`; it reads no ``, so +those are for a machine reader and not the report. The file is created or replaced, never appended, because a +JUnit document is one tree. That is why either spelling names a file per stage and +not one per act. Allure, nextest and pytest read the same format, and an Allure +publisher reading `.test-evidence/` consumes the file unchanged. + +| the guard | the testcase | exit | +|---|---|---| +| pass | `` with no child | `0` | +| refuse | `` | `1` | +| could not look | `` | `2` | +| bypass | `` | `0` | + +```xml + + + + + + + + + + + + + + + + + + + + + + ``` -| 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. +The testcase `name` is the rule id and `classname` is the bundled set, or +`local` for a rule the policy declares itself. The properties are `builtin` +(the dispatch name, since a repository may call a rule anything), `set`, +`stage` and `uphold` (the binary's version, because a bundled set's contents +belong to the binary). A bypass also carries `allowed`: `id` when the rule's id +was named in `UPHOLD_ALLOW`, `all` when `all` matched. + +- **No subject text.** A `failure` or `error` message is the rule id and the + word; the report stays on stderr. No commit message, path or argv enters the + file, and neither does a hostname or a user name. A timestamp is the only run + metadata kept. +- **Opened first.** The file is opened before the first guard runs, so a path + that cannot be written is exit `2` with nothing judged. A stage that declares + no rules writes a suite with zero tests, which tells "ran, nothing declared" + from "never ran". +- **The variable's path is absolute.** Git runs a hook from the top of the + working tree, and a relative path would land where the caller did not look. + A relative value is exit `2` naming it. The flag has no such rule: it was + typed where the caller stands. +- **An empty value is exit `2`**, for the flag and the variable alike. For the + variable this is the opposite reading from `UPHOLD_ALLOW=`, on purpose: + there a blank value switched every guard off without a word, and here it + refuses one run loudly. +- **A rule that could not look ends the run.** It gets an `error`, the rules + after it get no testcase, the file is still written, and the exit is `2`. + Absence is not a claim about those rules. A failure before the first rule + runs, such as a push that was told nothing or a policy that will not load, + leaves the file absent and exits `2`. +- **`--stage manual` writes like any other stage.** +- **`uphold guard --text` with either given is exit `2`.** That seam + reaches rules through a path that reports refusals only, so it cannot + honestly write a pass. +- **The shim and `uphold hook` do not read the variable**, so one exported + around a whole act does not trip them. + +stdout and stderr are unchanged; the file is additive. uphold decides and +records nothing beyond this one file. Signing it, storing it and publishing it +belong to whatever runs the gate. ## `uphold shim` — the shims diff --git a/src/guard/mod.rs b/src/guard/mod.rs index 1b7c4f8..72f8cc7 100644 --- a/src/guard/mod.rs +++ b/src/guard/mod.rs @@ -28,7 +28,7 @@ use std::path::Path; use crate::config::{CheckKind, Policy, Rule}; use crate::error::{Exit, Fatal, Result}; -use crate::verdicts::Outcome; +use crate::junit::Outcome; /// Every built-in check this binary carries, and the only list of them. /// @@ -213,13 +213,36 @@ pub(crate) struct Refusal { /// into the policy file it would be committed, reviewed once, and permanent -- /// which is not a bypass, it is a rule that no longer applies. pub(crate) fn bypassed(id: &str) -> bool { - let Ok(value) = std::env::var("UPHOLD_ALLOW") else { - return false; - }; - value - .split(',') - .map(str::trim) - .any(|allowed| allowed == id || allowed == "all") + allowed(id).is_some() +} + +/// What in `UPHOLD_ALLOW` switched a rule off. +/// +/// The bypass is the same either way. What differs is what a reader of the +/// record may make of it: an id was a decision about this rule, and `all` was a +/// decision about the run that this rule fell under. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Allowed { + /// The rule's own id was in the list. + Named, + /// `all` was in the list. + All, +} + +/// Which entry of `UPHOLD_ALLOW` bypasses `id`, if any. +/// +/// The id wins over `all` when both are listed, because it is the narrower +/// statement and the one that names this rule. +pub(crate) fn allowed(id: &str) -> Option { + let value = std::env::var("UPHOLD_ALLOW").ok()?; + let entries = || value.split(',').map(str::trim); + if entries().any(|entry| entry == id) { + Some(Allowed::Named) + } else if entries().any(|entry| entry == "all") { + Some(Allowed::All) + } else { + None + } } /// Is EVERY rule bypassed for this invocation? @@ -647,14 +670,26 @@ 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(); +pub(crate) fn run(root: &Path, policy: &Policy, request: RunRequest<'_>) -> Result { + let RunRequest { + stage, + message_file, + push_refs, + push_source, + remote_name, + remote_url, + evidence, + } = request; + let stage_name = stage.as_str(); + // The sink was opened by the caller before this run decided anything; the + // suite is built here and written on every way out below, the early one + // included, so "ran, nothing declared" is a suite of zero tests and not + // the same absence as "never ran". + let mut suite = evidence.map(|sink| crate::junit::Suite::new(sink, stage_name)); // Every rule the CONFIG puts at this hook, and nothing else decides it. - let at_hook: Vec<&Rule> = policy.at_hook(stage).collect(); + let at_hook: Vec<&Rule> = policy.at_hook(stage_name).collect(); if policy.at_hook_any().next().is_none() { + written(suite)?; println!("no rule declares a git hook"); return Ok(Exit::Clean); } @@ -663,13 +698,22 @@ 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); + let case = crate::junit::Case { + id: &rule.id, + builtin: rule.builtin(), + set: policy.set_of(&rule.id), + }; + let mut record = |outcome| { + if let Some(suite) = suite.as_mut() { + suite.record(case, outcome); + } + }; // 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()) { + if let Some(allowed) = allowed(rule.id.as_str()) { eprintln!("uphold guard: {} bypassed by UPHOLD_ALLOW", rule.id); - verdicts.record(stage, &rule.id, set, Outcome::Bypass)?; + record(Outcome::Bypass(allowed)); bypassed_here += 1; continue; } @@ -678,32 +722,33 @@ pub(crate) fn run(root: &Path, policy: &Policy, request: &RunRequest<'_>) -> Res root, rule, policy, - stage: request.stage, - message_file: request.message_file, - push_refs: request.push_refs, - push_source: request.push_source, - remote_name: request.remote_name, - remote_url: request.remote_url, + stage, + message_file, + push_refs, + push_source, + remote_name, + remote_url, }; match evaluate(&one) { Ok(Some(refusal)) => { - verdicts.record(stage, &rule.id, set, Outcome::Refuse)?; + record(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. + Ok(None) => record(Outcome::Pass), + // The rule could not look, and the run ends here with exit 2. Its + // testcase is an error and the rules after it get none; 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}")), - }, - ); + record(Outcome::Unknown); + return Err(match written(suite) { + Ok(()) => error, + Err(also) => Fatal::new(format!("{error}\n\n{also}")), + }); } } } + written(suite)?; for refusal in &refusals { eprintln!("guard refused: {}", refused_by(policy, refusal)); @@ -723,13 +768,19 @@ pub(crate) fn run(root: &Path, policy: &Policy, request: &RunRequest<'_>) -> Res } if bypassed_here > 0 { - println!("{ran} guard(s) passed at {stage}, {bypassed_here} bypassed by UPHOLD_ALLOW"); + println!("{ran} guard(s) passed at {stage_name}, {bypassed_here} bypassed by UPHOLD_ALLOW"); } else { - println!("{ran} guard(s) passed at {stage}"); + println!("{ran} guard(s) passed at {stage_name}"); } Ok(Exit::Clean) } +/// Write the run's `JUnit` document, if one was asked for. +fn written(suite: Option>) -> Result<()> { + suite.map_or(Ok(()), crate::junit::Suite::write) +} + +#[derive(Debug)] pub(crate) struct RunRequest<'a> { pub stage: Stage, pub message_file: Option<&'a Path>, @@ -737,6 +788,10 @@ pub(crate) struct RunRequest<'a> { pub push_source: crate::runner::Source, pub remote_name: Option<&'a str>, pub remote_url: Option<&'a str>, + /// Where the run's `JUnit` document goes, opened by the caller before + /// anything is decided; `None` when nobody asked for one. A sink and not a + /// path, so a test can hand the run a buffer. See `junit`. + pub evidence: Option>, } #[cfg(test)] diff --git a/src/junit.rs b/src/junit.rs new file mode 100644 index 0000000..cbc1b6c --- /dev/null +++ b/src/junit.rs @@ -0,0 +1,373 @@ +//! `UPHOLD_JUNIT` -- what a guard run decided, as one `JUnit` XML document. +//! +//! uphold decides and does not record. Signing a pass, storing it beside the +//! commit and publishing it belong to whatever runs the gate, and that caller +//! 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 the run writes one document to it: one `` for the +//! stage, one `` per rule the stage evaluated. Unset, nothing is +//! written and the run is the run it always was. +//! +//! `JUnit` because the gate tier already speaks it: its evidence directory holds +//! `JUnit` files, its publisher turns them into one report, and its recorder links +//! that report. A format of uphold's own would be one more thing for each of +//! them to read. The four testcase states are the four answers a guard gives: +//! +//! | guard | testcase | +//! | ------- | --------------- | +//! | pass | no child | +//! | refuse | `` | +//! | unknown | `` | +//! | bypass | `` | +//! +//! "Evidence" in this module means what it means in the gate's directory, a +//! record of a run. It is not `crate::evidence`, a provider's facts handed to a +//! policy; the two do not meet. +//! +//! A testcase carries the decision and never the subject: no message, no body, +//! no path, 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, and the +//! report a reader acts on is still the one on stderr. No hostname and no user +//! name either; the timestamp is the only run metadata kept. + +use std::ffi::OsStr; +use std::fs::File; +use std::io::Write; +use std::path::{Path, PathBuf}; + +use quick_junit::{NonSuccessKind, Property, Report, TestCase, TestCaseStatus, TestSuite}; + +use crate::error::{Fatal, Result}; +use crate::guard::Allowed; + +/// 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_JUNIT"; + +/// The flag naming the file: the conventional spelling, as `--junitxml` is +/// pytest's, for a caller who owns the command line. +pub(crate) const FLAG: &str = "--junit"; + +/// The file the caller asked for, if they asked. +/// +/// Two spellings of one request. `--junit FILE` is the one a caller types who +/// owns the command line. The variable exists for the caller who does not: a +/// hook's argv is the hook definition's, pinned by the repository, and the +/// only channel that reaches from `git push` through the runner to the guard +/// is the environment. The flag wins when both are given, as a flag usually +/// does over the environment it was typed into. +/// +/// Read and judged before anything else the run does, so a value that names no +/// usable place is exit 2 before a policy is loaded or a rule is asked. +pub(crate) fn requested(flag: Option<&OsStr>) -> Result> { + if let Some(given) = flag { + if given.is_empty() { + return Err(Fatal::new(format!("{FLAG} names no file"))); + } + let path = PathBuf::from(given); + // Typed where the caller stands, so a relative path means what it + // means to any other command: this directory. The hook case below is + // the one where "this directory" is not the caller's. + return Ok(Some(if path.is_absolute() { + path + } else { + std::env::current_dir()?.join(path) + })); + } + let Some(value) = std::env::var_os(VARIABLE) else { + return Ok(None); + }; + // Refused rather than read as unset, the opposite reading from + // `UPHOLD_ALLOW=` on purpose. There, blank-means-set switched every guard + // off and said nothing, so blank is read as unset. Here, blank-means-unset + // would be the silent one: a caller who exported the variable and blanked + // it asked for evidence somewhere, and a run that wrote none and exited 0 + // reads to them as a run whose record went missing later. Refusing one run + // loudly is the cheaper of the two mistakes. + if value.is_empty() { + return Err(Fatal::new(format!( + "{VARIABLE}= is set and names no file. Unset it to write no evidence, \ + or name an absolute path" + ))); + } + let path = PathBuf::from(value); + // git runs a hook from the top of the working tree, not from where the + // caller stood, so a relative path would be written somewhere the caller + // did not look -- and the place they did look would hold nothing, which is + // the absence this file exists to rule out. + if !path.is_absolute() { + return Err(Fatal::new(format!( + "{VARIABLE}={} is a relative path. A hook runs from the top of the \ + working tree, so name an absolute one", + path.display() + ))); + } + Ok(Some(path)) +} + +/// Where one run's document goes, opened before any rule is asked. +pub(crate) struct Sink<'a> { + out: Box, + /// Named in an error, so a failed write says which file. + place: PathBuf, +} + +impl std::fmt::Debug for Sink<'_> { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter + .debug_struct("Sink") + .field("place", &self.place) + .finish_non_exhaustive() + } +} + +impl Sink<'static> { + /// Create or replace the file, never append to it. + /// + /// A `JUnit` document is one tree, and a second run appended to the first + /// is not a document at all. That is why the variable names a file per + /// stage and not one per act. Opened here, before the run, so a path that + /// cannot be written stops it while it has decided nothing rather than after + /// it decided everything. + pub(crate) fn create(path: &Path) -> Result { + let file = + File::create(path).map_err(|error| Fatal::at(path, format!("{VARIABLE}: {error}")))?; + Ok(Self::new(file, path)) + } +} + +impl<'a> Sink<'a> { + /// Any writer, named by `place` in an error. The file is one; a test's + /// buffer is another. + pub(crate) fn new(out: impl Write + 'a, place: &Path) -> Self { + Self { + out: Box::new(out), + place: place.to_path_buf(), + } + } +} + +/// One rule's answer, and no fifth: 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, + /// Could not look. The run stops here with exit 2, so this is the last + /// testcase the document holds; a rule the run never reached has none, + /// because absence is not a claim. + Unknown, + /// Did not run, because `UPHOLD_ALLOW` named it or said `all`. + Bypass(Allowed), +} + +/// What a rule is, as far as its testcase says. +#[derive(Clone, Copy, Debug)] +pub(crate) struct Case<'r> { + /// The rule's id: the testcase's `name`. + pub id: &'r str, + /// The dispatch name, since a repository may call a rule anything. + pub builtin: Option<&'r str>, + /// The bundled set the rule came from, or `None` for a rule written here. + pub set: Option<&'r str>, +} + +/// The suite under construction for one stage. +#[derive(Debug)] +pub(crate) struct Suite<'a> { + sink: Sink<'a>, + stage: String, + built: TestSuite, +} + +impl<'a> Suite<'a> { + pub(crate) fn new(sink: Sink<'a>, stage: &str) -> Self { + let mut suite = TestSuite::new(format!("uphold guard {stage}")); + // Allure's JUnit reader takes the `package` attribute as its package + // label, so the report groups these suites under the tool that wrote + // them. It reads no ``, which is why the grouping cannot + // ride on one. + suite.extra.insert("package".into(), "uphold".into()); + suite.set_timestamp(chrono::DateTime::::from( + std::time::SystemTime::now(), + )); + Self { + sink, + stage: stage.to_owned(), + built: suite, + } + } + + /// Add one rule's testcase. + pub(crate) fn record(&mut self, case: Case<'_>, outcome: Outcome) { + let status = match outcome { + Outcome::Pass => TestCaseStatus::success(), + Outcome::Refuse => non_success(NonSuccessKind::Failure, case.id, "refused"), + Outcome::Unknown => non_success(NonSuccessKind::Error, case.id, "could not look"), + Outcome::Bypass(_) => { + let mut skipped = TestCaseStatus::skipped(); + skipped.set_message("bypassed by UPHOLD_ALLOW"); + skipped + } + }; + let mut testcase = TestCase::new(case.id, status); + // `local` rather than no classname: Allure files a testcase by its + // class, and a rule written in this repository's own policy is as much + // a class as one a set brought in. + testcase.set_classname(case.set.unwrap_or("local")); + if let Some(builtin) = case.builtin { + testcase.add_property(Property::new("builtin", builtin)); + } + if let Some(set) = case.set { + testcase.add_property(Property::new("set", set)); + } + testcase.add_property(Property::new("stage", self.stage.as_str())); + // A bundled set's contents are a property of the binary, so the + // version is what says which rules `set` meant on this run. + testcase.add_property(Property::new("uphold", env!("CARGO_PKG_VERSION"))); + if let Outcome::Bypass(allowed) = outcome { + testcase.add_property(Property::new( + "allowed", + match allowed { + Allowed::Named => "id", + Allowed::All => "all", + }, + )); + } + self.built.add_test_case(testcase); + } + + /// Serialize the whole document. Called once, on every path out of the run + /// that got as far as opening the sink. + pub(crate) fn write(self) -> Result<()> { + let Self { + mut sink, built, .. + } = self; + let mut report = Report::new("uphold guard"); + report.add_test_suite(built); + let failed = + |error: &dyn std::fmt::Display| Fatal::at(&sink.place, format!("{VARIABLE}: {error}")); + report + .serialize(&mut sink.out) + .map_err(|error| failed(&error))?; + sink.out.flush().map_err(|error| failed(&error)) + } +} + +/// A failure or an error whose message is the rule and the word, and nothing +/// the rule read. +fn non_success(kind: NonSuccessKind, id: &str, word: &str) -> TestCaseStatus { + let mut status = TestCaseStatus::non_success(kind); + status.set_message(format!("{id} {word}")); + status +} + +#[cfg(test)] +mod tests { + use super::*; + + fn written(cases: &[(Case<'_>, Outcome)]) -> Report { + let mut buffer = Vec::new(); + let mut suite = Suite::new(Sink::new(&mut buffer, Path::new("memory")), "pre-push"); + for (case, outcome) in cases { + suite.record(*case, *outcome); + } + suite.write().unwrap(); + Report::deserialize_from_str(std::str::from_utf8(&buffer).unwrap()).unwrap() + } + + fn property<'c>(case: &'c TestCase, name: &str) -> Option<&'c str> { + case.properties + .iter() + .find(|property| property.name.as_str() == name) + .map(|property| property.value.as_str()) + } + + const BUNDLED: Case<'static> = Case { + id: "a-rule", + builtin: Some("a-builtin"), + set: Some("a-set"), + }; + const LOCAL: Case<'static> = Case { + id: "b-rule", + builtin: Some("b-builtin"), + set: None, + }; + + #[test] + fn each_answer_is_its_own_testcase_state() { + let report = written(&[ + (BUNDLED, Outcome::Pass), + (LOCAL, Outcome::Refuse), + (BUNDLED, Outcome::Bypass(Allowed::All)), + (LOCAL, Outcome::Unknown), + ]); + assert_eq!(report.test_suites.len(), 1, "{report:?}"); + let suite = &report.test_suites[0]; + assert_eq!(suite.name.as_str(), "uphold guard pre-push"); + assert_eq!( + suite + .extra + .get("package") + .map(quick_junit::XmlString::as_str), + Some("uphold") + ); + assert!(suite.timestamp.is_some()); + assert_eq!( + (suite.tests, suite.failures, suite.errors, suite.skipped), + (4, 1, 1, 1) + ); + assert_eq!(suite.test_cases.len(), 4, "{suite:?}"); + let (pass, refuse, bypass, unknown) = ( + &suite.test_cases[0], + &suite.test_cases[1], + &suite.test_cases[2], + &suite.test_cases[3], + ); + assert!(matches!(pass.status, TestCaseStatus::Success { .. })); + assert!(matches!( + &refuse.status, + TestCaseStatus::NonSuccess { kind: NonSuccessKind::Failure, message: Some(message), .. } + if message.as_str() == "b-rule refused" + )); + assert!(matches!( + &bypass.status, + TestCaseStatus::Skipped { message: Some(message), .. } + if message.as_str() == "bypassed by UPHOLD_ALLOW" + )); + assert!(matches!( + &unknown.status, + TestCaseStatus::NonSuccess { kind: NonSuccessKind::Error, message: Some(message), .. } + if message.as_str() == "b-rule could not look" + )); + assert_eq!(property(bypass, "allowed"), Some("all")); + assert_eq!(property(pass, "allowed"), None); + assert_eq!( + pass.classname.as_ref().map(quick_junit::XmlString::as_str), + Some("a-set") + ); + assert_eq!(property(pass, "set"), Some("a-set")); + assert_eq!( + refuse + .classname + .as_ref() + .map(quick_junit::XmlString::as_str), + Some("local") + ); + assert_eq!(property(refuse, "set"), None); + for case in &suite.test_cases { + assert_eq!(property(case, "stage"), Some("pre-push")); + assert_eq!(property(case, "uphold"), Some(env!("CARGO_PKG_VERSION"))); + assert!(property(case, "builtin").is_some()); + } + } + + #[test] + fn a_stage_with_nothing_to_ask_is_still_a_document() { + let report = written(&[]); + assert_eq!(report.test_suites.len(), 1, "{report:?}"); + let suite = &report.test_suites[0]; + assert_eq!(suite.tests, 0); + } +} diff --git a/src/main.rs b/src/main.rs index 981413f..156d86e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -90,6 +90,7 @@ mod hooks; mod init; mod install; mod invocation; +mod junit; mod out; mod pins; mod policy; @@ -104,7 +105,6 @@ mod shim; mod sources; mod supply; mod text; -mod verdicts; use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; @@ -115,7 +115,8 @@ const USAGE: &str = "\ usage: uphold scan [--policy PATH] run the content policy over this repository uphold scan --text [FILE|-] run the host-identity rules over text - uphold guard --stage STAGE run the guards that fire at STAGE + uphold guard --stage STAGE [--junit FILE] + run the guards that fire at STAGE uphold guard --text [FILE|-] run the text-capable guards over text uphold check [--json] [--probe] reconcile claims; optionally measure hook behavior uphold check --coverage which rules run here and carry no principle @@ -168,7 +169,8 @@ 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. +`--junit FILE` writes what a `guard --stage` run decided as JUnit XML. +UPHOLD_JUNIT=/abs/FILE is the same request from outside a hook's argv; the flag wins. "; /// The policy file, newest name first. @@ -708,6 +710,7 @@ fn guard_command(arguments: &[OsString]) -> Result { let mut remote_name: Option = None; let mut remote_url: Option = None; let mut text_source: Option = None; + let mut junit_flag: Option = None; let mut index = 0; while let Some(argument) = arguments.get(index) { // A stage name, a remote name, a remote URL and a text source are all @@ -747,11 +750,40 @@ fn guard_command(arguments: &[OsString]) -> Result { None => String::from("-"), }); } + // A path, like `--message`: it does not have to be text to be + // created. + "--junit" => { + index += 1; + let path = arguments + .get(index) + .ok_or_else(|| Fatal::new(format!("{} needs a value", junit::FLAG)))?; + junit_flag = Some(path.clone()); + } other => return Err(Fatal::new(format!("unknown option {other:?}\n\n{USAGE}"))), } index += 1; } + // Judged before a policy is read, so a value that names no usable file is + // exit 2 whatever else is wrong with the run. + let evidence = junit::requested(junit_flag.as_deref())?; + if evidence.is_some() && text_source.is_some() { + // `--text` reaches rules through `text::judged`, which returns refusals + // only. A document written from it could name what refused and could + // not honestly name what passed, so this seam writes none until it can. + return Err(Fatal::new(format!( + "{} was asked for, and `uphold guard --text` does not write evidence yet: \ + it learns only what refused, never what passed. Drop {} and unset {} for this run", + if junit_flag.is_some() { + junit::FLAG + } else { + junit::VARIABLE + }, + junit::FLAG, + junit::VARIABLE + ))); + } + let working = std::env::current_dir()?; let (root, policy_path) = discover(&working).ok_or_else(|| no_policy_here(&working))?; let policy = config::load(&root, &policy_path)?; @@ -829,16 +861,23 @@ fn guard_command(arguments: &[OsString]) -> Result { } }; + // Opened last before the first rule is asked: after the policy loaded and + // the push was read, because a run that stops before its loop has judged + // nothing and leaves no file, and before the loop, because a file that + // cannot be written must stop the run while it has judged nothing too. + let evidence = evidence.as_deref().map(junit::Sink::create).transpose()?; + guard::run( &root, &policy, - &guard::RunRequest { + guard::RunRequest { stage, message_file: message.as_deref(), push_refs: &push.refs, push_source: push.source, remote_name: push.remote_name.as_deref(), remote_url: push.remote_url.as_deref(), + evidence, }, ) } diff --git a/src/verdicts.rs b/src/verdicts.rs deleted file mode 100644 index d47f094..0000000 --- a/src/verdicts.rs +++ /dev/null @@ -1,177 +0,0 @@ -//! `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 1b7ea95..998b8d7 100644 --- a/tests/guard_cli.rs +++ b/tests/guard_cli.rs @@ -15,9 +15,12 @@ mod support; +use std::ffi::OsStr; use std::path::{Path, PathBuf}; use std::process::{Command, Output, Stdio}; +use quick_junit::{NonSuccessKind, Report, TestCase, TestCaseStatus, TestSuite, XmlString}; + fn repository(policy: &str) -> PathBuf { let root = support::scratch("guard"); let _ = std::fs::remove_dir_all(&root); @@ -156,12 +159,16 @@ builtin = "ascii-only-commit-subject" hooks = ["commit-msg"] "#; -fn guard_with_verdicts(root: &Path, verdicts: &Path, allow: Option<&str>) -> Output { +// --- UPHOLD_JUNIT: what a run decided, as one JUnit document ---------------- + +/// `uphold guard` at commit-msg over `msg.txt`, with `UPHOLD_JUNIT` set to +/// `junit` and `UPHOLD_ALLOW` to `allow` when given. +fn guard_with_junit(root: &Path, junit: &OsStr, 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("UPHOLD_JUNIT", junit) .env_remove("UPHOLD_ALLOW"); if let Some(allow) = allow { command.env("UPHOLD_ALLOW", allow); @@ -169,89 +176,430 @@ fn guard_with_verdicts(root: &Path, verdicts: &Path, allow: Option<&str>) -> Out 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() +/// The file read back by the library that wrote it, holding exactly one suite. +fn the_suite(path: &Path) -> TestSuite { + let text = std::fs::read_to_string(path).unwrap(); + let mut report = Report::deserialize_from_str(&text).unwrap(); + assert_eq!(report.test_suites.len(), 1, "{text}"); + report.test_suites.remove(0) } -fn outcome_of<'a>(lines: &'a [serde_json::Value], rule: &str) -> Vec<&'a str> { - lines +fn the_case<'s>(suite: &'s TestSuite, id: &str) -> &'s TestCase { + let mut found = suite + .test_cases .iter() - .filter(|line| line["rule"] == rule) - .map(|line| line["outcome"].as_str().unwrap()) - .collect() + .filter(|case| case.name.as_str() == id); + let case = found.next().unwrap(); + assert!(found.next().is_none(), "{id} twice in {suite:?}"); + case +} + +fn property<'c>(case: &'c TestCase, name: &str) -> Option<&'c str> { + case.properties + .iter() + .find(|property| property.name.as_str() == name) + .map(|property| property.value.as_str()) +} + +/// The message a `failure`, `error` or `skipped` carries, and which of the +/// four states the testcase is in. +fn state(case: &TestCase) -> (&'static str, Option<&str>) { + match &case.status { + TestCaseStatus::Success { .. } => ("pass", None), + TestCaseStatus::NonSuccess { + kind: NonSuccessKind::Failure, + message, + .. + } => ("failure", message.as_ref().map(XmlString::as_str)), + TestCaseStatus::NonSuccess { + kind: NonSuccessKind::Error, + message, + .. + } => ("error", message.as_ref().map(XmlString::as_str)), + TestCaseStatus::Skipped { message, .. } => { + ("skipped", message.as_ref().map(XmlString::as_str)) + } + } } #[test] -fn verdicts_name_each_rules_answer_and_never_the_subject() { +fn each_rules_answer_is_a_testcase_and_the_subject_is_not_in_the_file() { 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 junit = root.join("commit-msg.xml"); - let output = guard_with_verdicts(&root, &verdicts, None); + let output = guard_with_junit(&root, junit.as_os_str(), 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)); + // The report a reader acts on is still on stderr, unchanged. + assert!( + stderr(&output).contains("guard refused: prevent-ai-author"), + "{}", + stderr(&output) + ); - let lines = verdict_lines(&verdicts); - assert_eq!(lines.len(), 4, "{lines:?}"); + let suite = the_suite(&junit); + assert_eq!(suite.name.as_str(), "uphold guard commit-msg"); + assert_eq!( + suite.extra.get("package").map(XmlString::as_str), + Some("uphold") + ); assert_eq!( - outcome_of(&lines, "prevent-ai-author"), - ["refuse", "bypass"] + (suite.tests, suite.failures, suite.errors, suite.skipped), + (2, 1, 0, 0) ); + let refused = the_case(&suite, "prevent-ai-author"); assert_eq!( - outcome_of(&lines, "ascii-only-commit-subject"), - ["pass", "pass"] + state(refused), + ("failure", Some("prevent-ai-author refused")) ); - for line in &lines { - assert_eq!(line["schema_version"], 1, "{line}"); - assert_eq!(line["seam"], "guard", "{line}"); - assert_eq!(line["stage"], "commit-msg", "{line}"); + let passed = the_case(&suite, "ascii-only-commit-subject"); + assert_eq!(state(passed), ("pass", None)); + for case in &suite.test_cases { assert_eq!( - line.get("by").is_some(), - line["outcome"] == "bypass", - "{line}" + case.classname.as_ref().map(XmlString::as_str), + Some("local") ); + assert_eq!(property(case, "builtin"), Some(case.name.as_str())); + assert_eq!(property(case, "set"), None); + assert_eq!(property(case, "stage"), Some("commit-msg")); + assert_eq!(property(case, "uphold"), Some(env!("CARGO_PKG_VERSION"))); + // `allowed` is a bypass's and only a bypass's. + assert_eq!(property(case, "allowed"), None); } - let text = std::fs::read_to_string(&verdicts).unwrap(); + + // No subject text, and no machine or person: the fleet's + // `no-running-os-identity-metadata` refuses both in a published file. + let text = std::fs::read_to_string(&junit).unwrap(); assert!(!text.contains("Generated"), "{text}"); + assert!(!text.contains("msg.txt"), "{text}"); + assert!(!text.contains("hostname"), "{text}"); + assert!(suite.extra.get("hostname").is_none(), "{suite:?}"); + assert!(!text.contains("example.test"), "{text}"); +} + +/// The document a run writes, held to the rule the fleet holds a published +/// file to: the scan reads this machine's user name, home path, hostname and +/// default route and finds none of them in it. Every answer is in the file, so +/// every branch of the serializer is under the rule. +#[test] +fn the_document_carries_no_identity_of_the_machine_that_wrote_it() { + let root = repository(&format!( + "{AI_AUTHOR_AND_ASCII_SUBJECT}\n\ + [rule.no-running-os-identity-metadata]\n\ + message = \"Do not publish host identity.\"\n\ + forbidden_literals = \"running-os-identity\"\n\ + files.include = [\"evidence\"]\n\ + files.word = true\n" + )); + write(&root, "msg.txt", "x\n\nGenerated with Claude Code\n"); + std::fs::create_dir_all(root.join("evidence")).unwrap(); + for (name, allow) in [("refused.xml", None), ("bypassed.xml", Some("all"))] { + let junit = root.join("evidence").join(name); + let output = guard_with_junit(&root, junit.as_os_str(), allow); + assert_ne!(code(&output), 2, "{}", stderr(&output)); + assert!(junit.exists()); + } + support::git(&root, &["add", "-A"]); + let output = Command::new(env!("CARGO_BIN_EXE_uphold")) + .arg("scan") + .current_dir(&root) + .env_remove("UPHOLD_ALLOW") + .output() + .unwrap(); + assert_eq!(code(&output), 0, "{}", stderr(&output)); +} + +#[test] +fn a_bypass_is_skipped_and_says_whether_its_id_or_all_switched_it_off() { + let root = repository(AI_AUTHOR_AND_ASCII_SUBJECT); + write(&root, "msg.txt", "x\n\nGenerated with Claude Code\n"); + let junit = root.join("commit-msg.xml"); + + let output = guard_with_junit(&root, junit.as_os_str(), Some("prevent-ai-author")); + assert_eq!(code(&output), 0, "{}", stderr(&output)); + let suite = the_suite(&junit); + assert_eq!((suite.tests, suite.skipped), (2, 1)); + let bypassed = the_case(&suite, "prevent-ai-author"); + assert_eq!( + state(bypassed), + ("skipped", Some("bypassed by UPHOLD_ALLOW")) + ); + assert_eq!(property(bypassed, "allowed"), Some("id")); + let passed = the_case(&suite, "ascii-only-commit-subject"); + assert_eq!(state(passed), ("pass", None)); + assert_eq!(property(passed, "allowed"), None); + + // The same file, run again: replaced, never appended, so the second + // document is the second run's alone. + let output = guard_with_junit(&root, junit.as_os_str(), Some("all")); + assert_eq!(code(&output), 0, "{}", stderr(&output)); + let suite = the_suite(&junit); + assert_eq!((suite.tests, suite.skipped), (2, 2)); + for case in &suite.test_cases { + assert_eq!(property(case, "allowed"), Some("all"), "{case:?}"); + } } +/// A rule that could not look is an `error`, and the rules the run never +/// reached are not in the file at all: absence is not a claim. +/// +/// Rules run in id order, so the passing one comes first, the one with no +/// `origin` to ask about second, and the one after it is never asked. #[test] -fn a_verdicts_file_that_cannot_be_opened_stops_the_run_before_it_decides() { +fn a_rule_that_could_not_look_is_an_error_and_the_rest_are_absent() { + let root = repository( + "[rule.ascii-only-commit-subject]\nbuiltin = \"ascii-only-commit-subject\"\n\n\ + [rule.ascii-only-commit-subject.git]\nhooks = [\"commit-msg\"]\n\n\ + [rule.no-stale-visibility]\nbuiltin = \"no-stale-visibility\"\nvisibility = \"private\"\n\n\ + [rule.no-stale-visibility.git]\nhooks = [\"commit-msg\"]\n\n\ + [rule.prevent-ai-author]\nbuiltin = \"prevent-ai-author\"\n\n\ + [rule.prevent-ai-author.git]\nhooks = [\"commit-msg\"]\n", + ); + write(&root, "msg.txt", "Fix the thing\n"); + let junit = root.join("commit-msg.xml"); + + let output = guard_with_junit(&root, junit.as_os_str(), None); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains("has no `origin` remote"), + "{}", + stderr(&output) + ); + let suite = the_suite(&junit); + assert_eq!((suite.tests, suite.errors), (2, 1), "{suite:?}"); + assert_eq!( + state(the_case(&suite, "ascii-only-commit-subject")), + ("pass", None) + ); + assert_eq!( + state(the_case(&suite, "no-stale-visibility")), + ("error", Some("no-stale-visibility could not look")) + ); + assert!( + suite + .test_cases + .iter() + .all(|case| case.name.as_str() != "prevent-ai-author"), + "{suite:?}" + ); +} + +#[test] +fn a_file_that_cannot_be_written_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); + // A path under a file, and a directory where the file should be: neither + // can be created. + for junit in [root.join("msg.txt/commit-msg.xml"), root.clone()] { + let output = guard_with_junit(&root, junit.as_os_str(), None); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains("UPHOLD_JUNIT"), + "{}", + stderr(&output) + ); + assert!( + !stderr(&output).contains("guard refused"), + "{}", + stderr(&output) + ); + } +} + +#[test] +fn an_empty_or_relative_value_is_refused_naming_it() { + let root = repository(AI_AUTHOR); + write(&root, "msg.txt", "x\n\nGenerated with Claude Code\n"); + for (value, named) in [ + ("", "UPHOLD_JUNIT= is set and names no file"), + ( + "commit-msg.xml", + "UPHOLD_JUNIT=commit-msg.xml is a relative path", + ), + ] { + let output = guard_with_junit(&root, OsStr::new(value), None); + assert_eq!(code(&output), 2, "{value:?}: {}", stderr(&output)); + assert!(stderr(&output).contains(named), "{}", stderr(&output)); + assert!( + !stderr(&output).contains("guard refused"), + "{}", + stderr(&output) + ); + } + assert!(!root.join("commit-msg.xml").exists()); +} + +/// `--junit FILE` is the spelling for a caller who owns the command line. A +/// relative value means this directory, as it does for any other command, and +/// the flag wins over a variable that names somewhere else. +#[test] +fn the_flag_names_the_file_relative_to_the_caller_and_wins_over_the_variable() { + let root = repository(AI_AUTHOR); + write(&root, "msg.txt", "plain\n"); + std::fs::create_dir_all(root.join("sub")).unwrap(); + let elsewhere = root.join("elsewhere.xml"); + let output = Command::new(env!("CARGO_BIN_EXE_uphold")) + .args([ + "guard", + "--stage", + "commit-msg", + "--message", + "../msg.txt", + "--junit", + "commit-msg.xml", + ]) + .current_dir(root.join("sub")) + .env("UPHOLD_JUNIT", &elsewhere) + .env_remove("UPHOLD_ALLOW") + .output() + .unwrap(); + assert_eq!(code(&output), 0, "{}", stderr(&output)); + let suite = the_suite(&root.join("sub").join("commit-msg.xml")); + assert_eq!(suite.tests, 1, "{suite:?}"); + assert!(!elsewhere.exists(), "the variable's file was written too"); + + let output = Command::new(env!("CARGO_BIN_EXE_uphold")) + .args([ + "guard", + "--stage", + "commit-msg", + "--message", + "msg.txt", + "--junit", + "", + ]) + .current_dir(&root) + .env_remove("UPHOLD_JUNIT") + .env_remove("UPHOLD_ALLOW") + .output() + .unwrap(); assert_eq!(code(&output), 2, "{}", stderr(&output)); assert!( - stderr(&output).contains("UPHOLD_VERDICTS"), + stderr(&output).contains("--junit names no file"), "{}", stderr(&output) ); +} + +/// "Ran, nothing declared here" is a suite of zero tests, which is not the +/// same absence as "never ran" -- whether other stages declare rules or no +/// stage does. +#[test] +fn a_stage_no_rule_declares_writes_a_suite_with_zero_tests() { + for policy in [AI_AUTHOR, ""] { + let root = repository(policy); + let junit = root.join("pre-commit.xml"); + let output = Command::new(env!("CARGO_BIN_EXE_uphold")) + .args(["guard", "--stage", "pre-commit"]) + .current_dir(&root) + .env("UPHOLD_JUNIT", &junit) + .env_remove("UPHOLD_ALLOW") + .output() + .unwrap(); + assert_eq!(code(&output), 0, "{policy:?}: {}", stderr(&output)); + let suite = the_suite(&junit); + assert_eq!(suite.name.as_str(), "uphold guard pre-commit"); + assert_eq!(suite.tests, 0, "{policy:?}: {suite:?}"); + } +} + +/// A run that stops before its first rule has judged nothing, so it leaves no +/// file; a policy that will not load is one. +#[test] +fn a_run_that_stops_before_its_first_rule_leaves_no_file() { + let root = repository( + "[rule.invent-a-guard]\nbuiltin = \"invent-a-guard\"\n\n[rule.invent-a-guard.git]\nhooks = []\n", + ); + let junit = root.join("pre-commit.xml"); + let output = Command::new(env!("CARGO_BIN_EXE_uphold")) + .args(["guard", "--stage", "pre-commit"]) + .current_dir(&root) + .env("UPHOLD_JUNIT", &junit) + .env_remove("UPHOLD_ALLOW") + .output() + .unwrap(); + assert_eq!(code(&output), 2, "{}", stderr(&output)); assert!( - !stderr(&output).contains("guard refused"), + stderr(&output).contains("no built-in is called"), "{}", stderr(&output) ); + assert!(!junit.exists()); } +/// `--text` learns only what refused, so it cannot honestly write a pass, and +/// it writes nothing rather than a document that reads as complete. #[test] -fn an_empty_verdicts_variable_is_refused_and_not_read_as_unset() { +fn the_text_seam_refuses_to_run_with_the_variable_set() { let root = repository(AI_AUTHOR); - write(&root, "msg.txt", "Fix the thing\n"); - let output = guard_with_verdicts(&root, Path::new(""), None); + let junit = root.join("text.xml"); + let output = Command::new(env!("CARGO_BIN_EXE_uphold")) + .args(["guard", "--text"]) + .current_dir(&root) + .env("UPHOLD_JUNIT", &junit) + .env_remove("UPHOLD_ALLOW") + .stdin(Stdio::null()) + .output() + .unwrap(); assert_eq!(code(&output), 2, "{}", stderr(&output)); assert!( - stderr(&output).contains("names no file"), + stderr(&output).contains("does not write evidence yet"), "{}", stderr(&output) ); + assert!(!junit.exists()); +} + +/// The variable set on a real `git push` reaches a real pre-push hook, which +/// writes the `pre-push` suite: git passes its environment on to the hook. +#[cfg(unix)] +#[test] +fn a_real_push_writes_the_pre_push_suite() { + let root = repository(IN_FILES); + let remote = support::scratch("guard-remote"); + std::fs::create_dir_all(&remote).unwrap(); + support::git(&remote, &["init", "-q", "--bare"]); + support::git( + &root, + &["remote", "add", "origin", remote.to_str().unwrap()], + ); + write(&root, "a.txt", "one\n"); + support::git(&root, &["add", "-A"]); + support::git(&root, &["commit", "-qm", "one", "--no-verify"]); + write( + &root, + ".git/hooks/pre-push", + &format!( + "#!/bin/sh\nexec '{}' guard --stage pre-push --remote \"$1\" --remote-url \"$2\"\n", + env!("CARGO_BIN_EXE_uphold") + ), + ); + let hook = root.join(".git/hooks/pre-push"); + let mut mode = std::fs::metadata(&hook).unwrap().permissions(); + std::os::unix::fs::PermissionsExt::set_mode(&mut mode, 0o755); + std::fs::set_permissions(&hook, mode).unwrap(); + + let junit = root.join("pre-push.xml"); + let output = support::git_command(&root) + .args([ + "-c", + "core.hooksPath=.git/hooks", + "push", + "-q", + "origin", + "main", + ]) + .env("UPHOLD_JUNIT", &junit) + .env_remove("UPHOLD_ALLOW") + .output() + .unwrap(); + assert!(output.status.success(), "{}", stderr(&output)); + let suite = the_suite(&junit); + assert_eq!(suite.name.as_str(), "uphold guard pre-push"); + assert_eq!( + state(the_case(&suite, "prevent-unusual-unicode-in-files")), + ("pass", None) + ); } #[test]