From 4f45d846510ab3b9a9cef6c642629b2a29ddd619 Mon Sep 17 00:00:00 2001 From: HackingGate Date: Sun, 18 Oct 2026 17:03:46 +0900 Subject: [PATCH] Write JUnit evidence from the shim, the harness hook and guard --text The text seams answered in refusals only, so #388's evidence file could not be written from them. The text dispatch now returns one answer per rule it consulted: pass, refuse, unknown, bypass, or out of scope at this destination, a fifth state a stage never has. The seam's exit code and stderr are derived from that list, so the human report and the file cannot disagree, and the kani proofs in `text` are extended to it. Each seam writes one document per invocation: `uphold shim `, `uphold hook ` and `uphold guard --text`, `package="uphold"`. Out of scope is a skipped testcase with `scope=destination`, because a rule stood down at this destination is a run-time decision a reader of a bypass log asks about, where a rule a stage never declared is a fact about the policy. A `subject` property carries the subject kind and never its content; no destination, owner, URL, branch, argv or body text enters the file, since the destination is what prevent-unowned-target protects. Consultations write the rules they ran and not themselves. UPHOLD_JUNIT or --junit naming an existing directory writes one `-.xml` per invocation, named by a v8 UUID built from the clock, the process id and a counter. The shim and hook accept only the directory form, a file there being exit 2 before the command runs; the hook and `guard --text` take --junit, the shim has no flag. UPHOLD_ALLOW=all at the shim writes a suite of zero tests with the suite property `allowed=all`. `guard --text` writes a document instead of refusing; the refusal's test and the proof helpers the old answer shape needed go with it, each replaced by the per-answer form or the shadow-array form: clean_means_every_consulted_kind_looked_and_found_nothing, every_consulted_kind_is_asked_once_and_every_finding_is_kept, looked_and_found_nothing, the_text_seam_refuses_to_run_with_the_variable_set, the_verdict_is_a_function_of_the_consulted_answers. The proofs model the list cheaply: a four-letter answer alphabet, a unit ledger with a refusal counter for the two verdict proofs, and a fixed shadow array of byte codes for the two list proofs. The first shape, a vector of outcomes read back symbolically, took minutes and gigabytes per harness and put the CI job past its runner's memory. That first shape's proof a_consulted_kind_that_could_not_look_is_broken_and_ends_the_list is folded into a_consulted_kind_that_could_not_look and the_list_is_a_function_of_the_consulted_answers, and its helper outcome, which decoded a ledger entry into an Outcome, is gone with the entries it decoded. Closes #389. Claude-Session: https://claude.ai/code/session_01BxiYVoqKiSHbbi4BMzPyba --- Cargo.lock | 1 + Cargo.toml | 7 + docs/DESIGN.md | 20 +- docs/REFERENCE.md | 120 +++++-- src/guard/mod.rs | 228 +++++++++---- src/hook.rs | 30 +- src/junit.rs | 505 +++++++++++++++++++++++++-- src/main.rs | 144 +++++--- src/prose.rs | 39 ++- src/shim.rs | 226 +++++++++--- src/shim/checker.rs | 6 + src/text.rs | 759 ++++++++++++++++++++++++++++++++++------- tests/guard_cli.rs | 58 ++-- tests/hook_cli.rs | 153 +++++++++ tests/shim_cli.rs | 335 ++++++++++++++++++ tests/support/junit.rs | 86 +++++ tests/support/mod.rs | 1 + tests/text_cli.rs | 110 ++++++ 18 files changed, 2456 insertions(+), 372 deletions(-) create mode 100644 tests/support/junit.rs diff --git a/Cargo.lock b/Cargo.lock index 242df5e..3d2d24e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1011,6 +1011,7 @@ dependencies = [ "unicode-security", "unicode_names2", "ureq", + "uuid", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 6c8fef2..e66f355 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -132,6 +132,13 @@ quick-junit = "0.8" # `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"] } +# Already in the tree under quick-junit. Names the one file each invocation +# writes into a directory `UPHOLD_JUNIT` names, so two `gh` calls in one act +# never pick the same name. `v8` and not `v4` or `v7`: those draw on a random +# source and bring `getrandom` with them, while this one is built from the +# clock, the process id and a counter, which is unique by construction on one +# machine and adds no crate. +uuid = { version = "1", default-features = false, features = ["v8"] } [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 1059176..c29b6b9 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -263,8 +263,8 @@ be committed, reviewed once, and permanent, which is not a bypass. 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 +the directory into a report. The testcase states are the answers a +guard already gives, and one more for a rule stood down at a destination, 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 @@ -280,6 +280,22 @@ 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. +A seam that runs many times in one act cannot share one file that is replaced +each time: two `gh` calls would leave the second call's record and lose the +first. So the same request may name a directory, and each invocation writes its +own file there. That is the results-directory convention JUnit tooling already +has, and a publisher that reads every XML under `.test-evidence/` needs no +change to read them. The shim and the hook accept only the directory form, +because for them the single file is a way to lose evidence without a word. + +A rule stood down at a destination is recorded as a skipped testcase, while a +rule a stage never declared is not recorded at all. The second is a fact about +the policy: nothing aimed the rule there, and a reader can look that up. The +first is a decision made at run time about one invocation, and the question a +reader of a bypass log asks is whether the check ran on that body or not. The +file answers it without a second source. Leaving it out would make absence mean +"set aside, passed or never ran", the ambiguity the file exists to remove. + `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 diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 4b2f061..88a5264 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -2009,11 +2009,12 @@ 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 stage decided +### Recording what a run decided ```sh uphold guard --stage manual --junit .test-evidence/check-guards/uphold.xml UPHOLD_JUNIT=/abs/path/pre-push.xml git push +UPHOLD_JUNIT=/abs/dir gh pr create --body-file body.md ``` Two spellings of one request. `--junit FILE` is the conventional one, as @@ -2024,25 +2025,62 @@ 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. +Three seams evaluate rules, and each writes the same document. The suite name +says which one ran: + +| the seam | the suite | the flag | +|---|---|---| +| a stage | `uphold guard ` | `--junit` | +| the shim | `uphold shim ` | none | +| a harness hook | `uphold hook ` | `--junit` | +| `uphold guard --text` | `uphold guard --text` | `--junit` | + +The shim has no flag because its argv is the command it stands in front of, and +`--junit` cannot be typed into a `gh` call the shim does not own. It reads the +variable alone. + 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 +`` with `package="uphold"`, and one `` per rule the seam +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 | +those are for a machine reader and not the report. Allure, nextest and pytest +read the same format, and an Allure publisher reading `.test-evidence/` +consumes the file unchanged. + +### A file or a directory + +`UPHOLD_JUNIT` and `--junit` name either. + +- **A file** is created or replaced, never appended, because a JUnit document is + one tree. That is why a file names one stage and not one act. +- **An existing directory** gets one file per invocation, named + `-.xml`, where `` is unique to the invocation. This is JUnit's + own results-directory convention, as nextest and pytest-split write. Two `gh` + calls in one act leave two files, and a publisher that reads every XML under a + directory reads both. +- **The shim and the hook accept only a directory.** Each runs many times in one + act, and a file replaced on every call would keep the last and lose the rest. + A file there is exit `2` naming the value. + +| the outcome | the testcase | exit | |---|---|---| | pass | `` with no child | `0` | | refuse | `` | `1` | | could not look | `` | `2` | | bypass | `` | `0` | +| out of scope at this destination | `` with property `scope=destination` | `0` | + +The fifth outcome is a rule that has a scope of its own, such as a private +destination, and whose scope did not hold for this invocation. It neither ran +nor was bypassed. It is written rather than left out: a rule a stage never +declared is absent because the policy never aimed it there, a fact a reader can +look up, but a rule stood down here was set aside at run time for this +invocation, and the file answers "did that check run on this body" without a +second source. ```xml - + @@ -2061,25 +2099,49 @@ publisher reading `.test-evidence/` consumes the file unchanged. + + + + + + + + + ``` 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. +(the dispatch name, since a repository may call a rule anything; a prose or +literal rule has none), `set`, `stage` where the seam is a 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. A shim or hook testcase carries +`subject`, the kind of thing it collected, such as `pr-body`, `issue-title` or +`release-note`. It holds the kind and never the content. - **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 + word; the report stays on stderr. No commit message, path, body or argv enters + the file, and neither does a hostname or a user name. A timestamp is the only + run metadata kept. +- **No destination.** The file holds no owner, repository, URL or branch name. + The repository a shim invocation publishes to is what `prevent-unowned-target` + protects, and a private name in a file a publisher renders is the leak that + rule exists to stop. +- **Consultations write their inner rules, not themselves.** `text-guards` and + `text-literals` are dispatch, not judgment. Each inner rule they ran gets its + own testcase and the consultation gets none, so one refusal is counted once. +- **Opened first.** The file is opened before the first rule runs, so a path + that cannot be written is exit `2` with nothing judged. A seam that declares no rules writes a suite with zero tests, which tells "ran, nothing declared" from "never ran". +- **The hook writes on every invocation.** The harness calls it only for the + tool names its own matcher selects, so the volume is that of publishing calls, + and one small file per call is the cost of a record that never decides whether + a call was interesting. Absence would otherwise mean "passed or never ran". + Retention is the caller's. - **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 @@ -2094,15 +2156,17 @@ was named in `UPHOLD_ALLOW`, `all` when `all` matched. 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_ALLOW=all` at the shim** runs the command unchecked before the + policy is read, so it knows no rules. It writes a suite with zero tests and the + suite property `allowed=all`, so "ran unchecked" is a document and not an + absence. Allure reads no suite properties; the machine reader does. +- **`uphold guard --text` writes a document**, the same one a stage does, with + one testcase per rule consulted. + +stdout and stderr are unchanged; the file is additive, and the exit code and the +report on stderr are computed from the same list of outcomes the file is, so +they cannot disagree. uphold decides and records nothing beyond this 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 72f8cc7..efd2e55 100644 --- a/src/guard/mod.rs +++ b/src/guard/mod.rs @@ -29,6 +29,7 @@ use std::path::Path; use crate::config::{CheckKind, Policy, Rule}; use crate::error::{Exit, Fatal, Result}; use crate::junit::Outcome; +use crate::text::{Answer, Judgment, Verdict, asked}; /// Every built-in check this binary carries, and the only list of them. /// @@ -193,7 +194,7 @@ pub(crate) struct Request<'a> { /// What a guard concluded. A refusal carries the whole report, because a guard /// that says only "refused" makes the reader go and find out what it saw. -#[derive(Debug)] +#[derive(Debug, Clone)] pub(crate) struct Refusal { pub id: String, pub report: String, @@ -320,14 +321,21 @@ pub(crate) const TARGET_GUARDS: &[&str] = &["prevent-unowned-target"]; /// -- is this destination one this workspace owns -- and [`push::unowned`] is /// the single place it is made. /// -/// `None` where the rule judges no destination, or is bypassed, or found -/// nothing. `Err` where the rule is in scope and there was no destination to -/// look at, which is the third answer and never a pass. +/// The answer goes into `ledger`, as [`text_refusal`]'s does: nothing where +/// the rule judges no destination, a bypass, a pass, a refusal, or -- where the +/// rule is in scope and there was no destination to look at -- an `Unknown` +/// and an `Err`, which is the third answer and never a pass. No subject is +/// recorded: what this reads is where the text goes, and the record never +/// says where that is. /// /// `forge` is which forge the invocation publishes to, as the shim already /// knows it from the command's own name, and `asked` is the memo for the /// question that reaches it -- both passed straight through to [`push::unowned`], /// which decides whether either is consulted. +#[expect( + clippy::too_many_arguments, + reason = "the destination guard's own inputs and the list it answers into, from the one caller" +)] pub(crate) fn target_refusal( root: &Path, policy: &Policy, @@ -335,14 +343,51 @@ pub(crate) fn target_refusal( target: Option<&str>, forge: Option, hostname: Option<&str>, - asked: &mut BTreeMap, -) -> Result> { + memo: &mut BTreeMap, + ledger: &mut Vec, +) -> Result<()> { let Some(builtin) = rule.builtin() else { - return Ok(None); + return Ok(()); }; - if !TARGET_GUARDS.contains(&builtin) || bypassed(&rule.id) { - return Ok(None); + if !TARGET_GUARDS.contains(&builtin) { + return Ok(()); + } + if let Some(allowed) = allowed(&rule.id) { + ledger.push(Judgment::of(rule, None, Answer::Bypass(allowed))); + return Ok(()); } + let found = asked( + ledger, + rule, + None, + unowned_target(root, policy, rule, builtin, target, forge, hostname, memo), + )?; + ledger.push(Judgment::of( + rule, + None, + found.map_or(Answer::Pass, |refusal| { + Answer::Refused(Verdict::Guard(refusal)) + }), + )); + Ok(()) +} + +/// The body of [`target_refusal`] once the rule is known to judge a +/// destination and to be switched on. +#[expect( + clippy::too_many_arguments, + reason = "the destination guard's own inputs, passed straight through from the one caller" +)] +fn unowned_target( + root: &Path, + policy: &Policy, + rule: &Rule, + builtin: &str, + target: Option<&str>, + forge: Option, + hostname: Option<&str>, + asked: &mut BTreeMap, +) -> Result> { // In scope, and nothing to judge. Exit 2 rather than a pass, and rather // than a refusal: nothing was found to be wrong, the subject could not be // looked at -- `explicit-unknown`, the same reading the editor pass and the @@ -406,13 +451,16 @@ pub(crate) type ScopeAsk<'a> = &'a mut dyn FnMut(&Rule) -> Result; /// One piece of published text, as a text guard is handed it. /// /// `label` is what the report calls it -- the shim's subject kind, the tool -/// name at the hook, the source at `--text`. `headline` says the text is a +/// name at the hook, the source at `--text`. `kind` is what a record calls it, +/// which is the same but at `--text`, where the label is a file name and a +/// record carries no path. `headline` says the text is a /// SUBJECT rather than prose: a title the shim collected, which a forge-side /// squash merge makes a commit subject on the default branch with no local /// hook in the way. It is set from the shim's subject kind and never read off /// the label. pub(crate) struct Published<'a> { pub label: &'a str, + pub kind: &'a str, pub text: &'a str, /// The part of `text` an edit adds to a body the forge already stores, /// where the shim knows it. `prevent-unusual-unicode` reads this instead @@ -433,39 +481,51 @@ pub(crate) struct Published<'a> { /// /// `destination` is the repository the text is published to, where the caller /// resolved one -- passed through to [`text_refusal`]. +/// +/// Each text guard's answer goes into `ledger`, and a guard whose own scope +/// did not hold goes in as out of scope rather than being left out: it was +/// seen and set aside, and a reader of the record asking whether it ran on +/// this body is owed that answer. A built-in that judges no text is not asked +/// and not recorded; neither are the consultations, which this is the loop of. pub(crate) fn over_text( root: &Path, policy: &Policy, destination: Option<&str>, published: &Published<'_>, in_scope: ScopeAsk<'_>, -) -> Result> { - let mut refusals = Vec::new(); + ledger: &mut Vec, +) -> Result<()> { for rule in policy.of_check(CheckKind::Builtin) { - if rule - .builtin() - .is_some_and(|builtin| META_TEXT_GUARDS.contains(&builtin)) - { + if rule.builtin().is_none_or(|builtin| { + META_TEXT_GUARDS.contains(&builtin) || !TEXT_GUARDS.contains(&builtin) + }) { continue; } - if !in_scope(rule)? { + if !asked(ledger, rule, Some(published.kind), in_scope(rule))? { + ledger.push(Judgment::of(rule, Some(published.kind), Answer::OutOfScope)); continue; } - if let Some(refusal) = text_refusal(root, policy, rule, destination, published, in_scope)? { - refusals.push(refusal); - } + text_refusal(root, policy, rule, destination, published, in_scope, ledger)?; } - Ok(refusals) + Ok(()) } -/// One text-capable guard's verdict over one piece of text. +/// One text-capable guard's verdict over one piece of text, into `ledger`. +/// +/// Nothing where the rule is not a text guard, since it was not asked. A +/// bypass, a pass or a refusal under the rule's own id where it is one, and +/// an `Unknown` before the error where it could not look -- the three reasons +/// a caller used to receive as one `None`, told apart because a record of +/// what ran cannot be written without them. Extracted so the shim seam +/// consults exactly the same dispatch `uphold guard --text` does: a text guard +/// that judged a commit message one way and a PR body another would be two +/// rules under one id. /// -/// `None` where the rule is not a text guard, or is bypassed, or found nothing -/// -- three different reasons a caller does not have to tell apart, because a -/// guard with nothing to say about a pull-request body is not a guard that -/// passed it. Extracted so the shim seam consults exactly the same dispatch -/// `uphold guard --text` does: a text guard that judged a commit message one -/// way and a PR body another would be two rules under one id. +/// A consultation records the rules it ran, each under its own id, and +/// nothing under its own: it is dispatch rather than judgment, and an entry +/// for both would count one refusal twice. It is recorded under its own id +/// only where it did not run -- bypassed -- since then there is nothing else +/// to say so. What the caller reports for it is [`told`]. /// /// `destination` is the repository the text is published to, where the caller /// resolved one. Only the shim has one; every other seam passes `None` and is @@ -477,42 +537,54 @@ pub(crate) fn text_refusal( destination: Option<&str>, published: &Published<'_>, in_scope: ScopeAsk<'_>, -) -> Result> { + ledger: &mut Vec, +) -> Result<()> { let &Published { label, + kind, text, added, headline, } = published; let Some(builtin) = rule.builtin() else { - return Ok(None); + return Ok(()); }; - if !TEXT_GUARDS.contains(&builtin) || bypassed(&rule.id) { - return Ok(None); + if !TEXT_GUARDS.contains(&builtin) { + return Ok(()); } - Ok(match builtin { - "prevent-ai-author" => message::ai_author_in(rule, label, text), + if let Some(allowed) = allowed(&rule.id) { + ledger.push(Judgment::of(rule, Some(kind), Answer::Bypass(allowed))); + return Ok(()); + } + let mut single = |found: Result>| -> Result<()> { + let found = asked(ledger, rule, Some(kind), found)?; + ledger.push(Judgment::of( + rule, + Some(kind), + found.map_or(Answer::Pass, |refusal| { + Answer::Refused(Verdict::Guard(refusal)) + }), + )); + Ok(()) + }; + match builtin { + "prevent-ai-author" => single(Ok(message::ai_author_in(rule, label, text))), // The one guard whose question is what this invocation types: an // edit that resubmits a stored body is judged on the lines it adds, // so a character typed into the body long ago does not hold every // later edit hostage. - "prevent-unusual-unicode" => { - message::unusual_unicode_in(rule, label, added.unwrap_or(text), headline)? - } - "no-private-repo-names" => names::in_text(root, policy, rule, destination, label, text)?, - // The consultations. Each folds what it ran into one refusal under this - // rule's id, naming each inner rule, so the reader is told which check - // refused and not only that the consultation did. `over_text` skips the - // meta guards, so the recursion is one level deep by construction. - "text-guards" => { - let inner = over_text(root, policy, destination, published, in_scope)?; - fold( - rule, - inner - .into_iter() - .map(|refusal| format!("{}: {}", refused_by(policy, &refusal), refusal.report)), - ) + "prevent-unusual-unicode" => single(message::unusual_unicode_in( + rule, + label, + added.unwrap_or(text), + headline, + )), + "no-private-repo-names" => { + single(names::in_text(root, policy, rule, destination, label, text)) } + // The consultations. `over_text` skips the meta guards, so the + // recursion is one level deep by construction. + "text-guards" => over_text(root, policy, destination, published, in_scope, ledger), // No scope question fans out here. The literal rules are not a kind the // shim ever dispatches directly -- `Seam::Command` does not consult // `Judged::Literals`, it reaches them only through this built-in -- so @@ -521,25 +593,48 @@ pub(crate) fn text_refusal( // not a policy rule at all. Gating them on the table's scope would only // stand the identity check down for private destinations, which is the // arrangement `command.scope = "always"` on this rule exists to undo. - "text-literals" => { - let failures = crate::text::failures_in(root, policy, text)?; - fold( - rule, - failures.into_iter().map(|failure| { + "text-literals" => crate::text::failures_in(root, policy, text, Some(kind), ledger), + _ => Ok(()), + } +} + +/// What a caller reports for `rule`, read off the entries one call to +/// [`text_refusal`] or [`target_refusal`] put in the list. +/// +/// A single check's report is its own refusal. A consultation's is one +/// refusal under its own id carrying every inner one, naming each inner rule, +/// so the reader is told which check refused and not only that the +/// consultation did -- and one rather than many, because the caller +/// attributes what it gets back to the rule it asked about. +pub(crate) fn told(policy: &Policy, rule: &Rule, answered: &[Judgment]) -> Option { + let mut refused = answered.iter().filter_map(Judgment::refused); + if rule + .builtin() + .is_some_and(|builtin| META_TEXT_GUARDS.contains(&builtin)) + { + return fold( + rule, + refused.map(|verdict| match verdict { + Verdict::Guard(refusal) => { + format!("{}: {}", refused_by(policy, refusal), refusal.report) + } + Verdict::Rule(failure) => { format!("{}: {}\n{}", failure.label, failure.message, failure.body) - }), - ) - } - _ => None, + } + }), + ); + } + refused.next().map(|verdict| match verdict { + Verdict::Guard(refusal) => refusal.clone(), + Verdict::Rule(failure) => Refusal { + id: failure.label.clone(), + report: failure.report(), + }, }) } /// What a consultation answers with: one refusal carrying every inner one. -/// -/// One rather than many, because the caller attributes what it gets back to -/// the rule it asked about -- a consultation returning three refusals under -/// three other ids would be three reports from rules the caller never -/// consulted. `None` for none, exactly as a single check answers. +/// `None` for none, exactly as a single check answers. fn fold(rule: &Rule, reports: impl Iterator) -> Option { let joined = reports.collect::>().join("\n"); if joined.is_empty() { @@ -685,7 +780,9 @@ pub(crate) fn run(root: &Path, policy: &Policy, request: RunRequest<'_>) -> Resu // 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)); + let mut suite = evidence.map(|sink| { + crate::junit::Suite::new(sink, crate::junit::Run::Stage(stage_name.to_owned())) + }); // Every rule the CONFIG puts at this hook, and nothing else decides it. let at_hook: Vec<&Rule> = policy.at_hook(stage_name).collect(); if policy.at_hook_any().next().is_none() { @@ -702,6 +799,7 @@ pub(crate) fn run(root: &Path, policy: &Policy, request: RunRequest<'_>) -> Resu id: &rule.id, builtin: rule.builtin(), set: policy.set_of(&rule.id), + subjects: &[], }; let mut record = |outcome| { if let Some(suite) = suite.as_mut() { diff --git a/src/hook.rs b/src/hook.rs index 51a12b8..8c7a0c1 100644 --- a/src/hook.rs +++ b/src/hook.rs @@ -52,6 +52,7 @@ use serde_json::Value; use crate::error::{Exit, Fatal, Result}; use crate::guard; use crate::invocation::{Invocation, Observed}; +use crate::junit::{Place, Record, Run}; use crate::shim::{Walked, walk_strings}; use crate::text::{self, Verdict}; @@ -137,7 +138,18 @@ fn refusal_document(shape: &Shape, report: &str) -> Result { /// without one is the host-identity half, which carries its own fallback for /// exactly this case; the guards do not, and their absence is reported rather /// than counted as a pass. -pub(crate) fn run(harness: &str, found: Option<&(PathBuf, PathBuf)>) -> Result { +/// +/// `evidence` is the directory a `JUnit` document goes into, one per call, +/// where one was asked for. Written on every call that got as far as judging, +/// refused or not: a hook that wrote only on a refusal would make an absent +/// file mean "passed or never ran", which is the ambiguity the record exists +/// to remove. Which calls arrive is the harness's matcher's decision, so the +/// volume is the volume of publishing calls. +pub(crate) fn run( + harness: &str, + found: Option<&(PathBuf, PathBuf)>, + evidence: Option, +) -> Result { let shape = SHAPES .iter() .find(|shape| shape.names.contains(&harness)) @@ -232,6 +244,13 @@ pub(crate) fn run(harness: &str, found: Option<&(PathBuf, PathBuf)>) -> Result) -> Result verdicts, + Err(error) => return record.finish(Err(error)), + }; + record.write(false)?; + for verdict in verdicts { match verdict { // The message as well as the finding. At the shim and at `--text` // the message is what tells the author what to do instead; here the @@ -275,7 +299,7 @@ pub(crate) fn run(harness: &str, found: Option<&(PathBuf, PathBuf)>) -> Result` per rule the stage evaluated. Unset, nothing is //! written and the run is the run it always was. //! +//! A stage is one of four places a rule is evaluated. The shim in front of +//! `gh` and `git push`, `uphold hook ` and `uphold guard --text` judge +//! text that never becomes a file, and each writes the same document under its +//! own suite name. The shim runs many times in one act and nobody can type a +//! flag into the argv it stands in front of, so the variable may name a +//! DIRECTORY instead: one file per invocation inside it, named for the suite +//! and a fresh id, which is the results-directory convention every `JUnit` +//! publisher already reads. The shim and the hook take only that form, since +//! two `gh` calls replacing one file would lose the first. +//! //! `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 | `` | +//! | guard | testcase | +//! | ------------ | --------------------------------- | +//! | pass | no child | +//! | refuse | `` | +//! | unknown | `` | +//! | bypass | ``, property `allowed` | +//! | out of scope | ``, property `scope` | +//! +//! The fifth is the seams' alone: a rule whose own scope did not hold at this +//! destination neither ran nor was bypassed. It is a skipped testcase rather +//! than an absence because it is a decision made about THIS invocation, and a +//! reader of a bypass log asking whether the private-name check ran on that +//! body should get the answer from the file and not from a second source. //! //! "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 @@ -27,18 +44,24 @@ //! 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. +//! report a reader acts on is still the one on stderr. What a seam records of +//! its subject is the KIND -- `text`, `title`, a tool's name -- never what was +//! in it. Nor where it was going: the repository a shim invocation publishes +//! to is what `prevent-unowned-target` protects, so no owner, repository, URL +//! or branch. 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::fs::{File, OpenOptions}; use std::io::Write; use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU32, Ordering}; use quick_junit::{NonSuccessKind, Property, Report, TestCase, TestCaseStatus, TestSuite}; -use crate::error::{Fatal, Result}; +use crate::error::{Exit, Fatal, Result}; use crate::guard::Allowed; +use crate::text::{Answered, Judgment}; /// 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. @@ -48,7 +71,22 @@ pub(crate) const VARIABLE: &str = "UPHOLD_JUNIT"; /// 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. +/// Where the caller asked for the document: one file, or a directory to put +/// one file per invocation in. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum Place { + /// Created or replaced, as `guard --stage` always has. + File(PathBuf), + /// An existing directory, which gets a new file on every invocation. + Directory(PathBuf), +} + +/// The place the caller asked for, if they asked. +/// +/// A directory that already exists is the directory form, and anything else +/// is a file to create or replace. Read off the filesystem rather than off a +/// trailing slash, because the caller who exported the variable once for a +/// whole act did not necessarily spell it with one. /// /// 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 @@ -59,7 +97,35 @@ pub(crate) const FLAG: &str = "--junit"; /// /// 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> { +pub(crate) fn requested(flag: Option<&OsStr>) -> Result> { + Ok(named(flag)?.map(|path| { + if path.is_dir() { + Place::Directory(path) + } else { + Place::File(path) + } + })) +} + +/// [`requested`], for a seam that runs many times in one act and so takes +/// only the directory form: a file value is exit 2 naming it, before anything +/// else the run does. +pub(crate) fn directory_requested(flag: Option<&OsStr>, seam: &str) -> Result> { + match requested(flag)? { + None => Ok(None), + Some(Place::Directory(path)) => Ok(Some(path)), + Some(Place::File(path)) => Err(Fatal::new(format!( + "{}={} names no existing directory. `{seam}` runs many times in one act, and \ + each run replacing one file would leave only the last; name an existing \ + directory and each run writes its own file in it", + if flag.is_some() { FLAG } else { VARIABLE }, + path.display() + ))), + } +} + +/// The path the caller named, before it is told file from directory. +fn named(flag: Option<&OsStr>) -> Result> { if let Some(given) = flag { if given.is_empty() { return Err(Fatal::new(format!("{FLAG} names no file"))); @@ -134,6 +200,50 @@ impl Sink<'static> { File::create(path).map_err(|error| Fatal::at(path, format!("{VARIABLE}: {error}")))?; Ok(Self::new(file, path)) } + + /// Open the document one run writes at `place`: the file itself, or a new + /// file in the directory, named for the suite. Created new and never + /// opened over an existing one, so a name that somehow repeated is an + /// error and not a lost record. + pub(crate) fn open(place: &Place, run: &Run) -> Result { + match place { + Place::File(path) => Self::create(path), + Place::Directory(directory) => { + let path = directory.join(format!("{}-{}.xml", run.slug(), fresh_id())); + let file = OpenOptions::new() + .write(true) + .create_new(true) + .open(&path) + .map_err(|error| Fatal::at(&path, format!("{VARIABLE}: {error}")))?; + Ok(Self::new(file, &path)) + } + } + } +} + +/// A name no other invocation on this machine is writing. +/// +/// The clock to the millisecond and the microsecond under it, the process id +/// and a counter for a process that writes twice -- which is unique on one +/// machine by construction, where a random id is unique by probability and +/// needs a random source this crate does not otherwise carry. Laid out as a +/// version 8 UUID so the name has the shape a reader expects, and so names +/// sort by when they were written. +fn fresh_id() -> uuid::Uuid { + static WRITTEN: AtomicU32 = AtomicU32::new(0); + let since = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default(); + let millis = u64::try_from(since.as_millis()).unwrap_or(u64::MAX); + let micros = since.subsec_micros() % 1000; + let mut bytes = [0_u8; 16]; + bytes[..6].copy_from_slice(&millis.to_be_bytes()[2..]); + // Twelve bits under the version nibble, and a thousand needs ten. + bytes[6..8].copy_from_slice(&u16::try_from(micros).unwrap_or(0).to_be_bytes()); + // Thirty bits under the variant's two, and a process id needs fewer. + bytes[8..12].copy_from_slice(&std::process::id().to_be_bytes()); + bytes[12..].copy_from_slice(&WRITTEN.fetch_add(1, Ordering::Relaxed).to_be_bytes()); + uuid::Uuid::new_v8(bytes) } impl<'a> Sink<'a> { @@ -147,7 +257,8 @@ impl<'a> Sink<'a> { } } -/// One rule's answer, and no fifth: the exit codes and the bypass, named. +/// One rule's answer, and no sixth: the exit codes, the bypass and the scope, +/// named. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum Outcome { /// Ran, and found nothing to refuse. @@ -160,6 +271,15 @@ pub(crate) enum Outcome { Unknown, /// Did not run, because `UPHOLD_ALLOW` named it or said `all`. Bypass(Allowed), + /// Did not run, because its own scope did not hold at this destination. + /// Only a seam with a destination answers this; a stage never does. + OutOfScope, +} + +impl Answered for Outcome { + fn outcome(&self) -> Outcome { + *self + } } /// What a rule is, as far as its testcase says. @@ -171,19 +291,81 @@ pub(crate) struct Case<'r> { 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 kinds of subject the rule was asked about at a seam -- `text`, + /// `title`, a tool's name -- and never what any of them said. + pub subjects: &'r [String], } -/// The suite under construction for one stage. +/// What wrote a suite, which is its name. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum Run { + /// `uphold guard --stage `. + Stage(String), + /// The shim, standing in front of the named command. + Shim(String), + /// `uphold hook `. + Hook(String), + /// `uphold guard --text`. + Text, +} + +impl Run { + /// The suite's name. + fn name(&self) -> String { + match self { + Self::Stage(stage) => format!("uphold guard {stage}"), + Self::Shim(command) => format!("uphold shim {command}"), + Self::Hook(harness) => format!("uphold hook {harness}"), + Self::Text => String::from("uphold guard --text"), + } + } + + /// The document's name, which is the command that wrote it. + const fn report(&self) -> &'static str { + match self { + Self::Stage(_) | Self::Text => "uphold guard", + Self::Shim(_) => "uphold shim", + Self::Hook(_) => "uphold hook", + } + } + + /// The property every testcase carries to say where it was decided: the + /// stage a hook ran at, or which seam judged the text. + const fn property(&self) -> (&'static str, &str) { + match self { + Self::Stage(stage) => ("stage", stage.as_str()), + Self::Shim(_) => ("seam", crate::text::Seam::Command.name()), + Self::Hook(_) => ("seam", crate::text::Seam::Hook.name()), + Self::Text => ("seam", crate::text::Seam::Guard.name()), + } + } + + /// The suite's name as a file name: what is not a letter or a digit + /// becomes one dash, so `uphold guard --text` is `uphold-guard-text`. + fn slug(&self) -> String { + let mut slug = String::new(); + for character in self.name().chars() { + if character.is_ascii_alphanumeric() { + slug.push(character.to_ascii_lowercase()); + } else if !slug.is_empty() && !slug.ends_with('-') { + slug.push('-'); + } + } + slug.trim_end_matches('-').to_owned() + } +} + +/// The suite under construction for one run. #[derive(Debug)] pub(crate) struct Suite<'a> { sink: Sink<'a>, - stage: String, + run: Run, 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}")); + pub(crate) fn new(sink: Sink<'a>, run: Run) -> Self { + let mut suite = TestSuite::new(run.name()); // 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 @@ -194,22 +376,26 @@ impl<'a> Suite<'a> { )); Self { sink, - stage: stage.to_owned(), + run, built: suite, } } + /// Say on the suite that `UPHOLD_ALLOW=all` ran the command unchecked + /// before any rule was read. Allure reads no suite property; the machine + /// reader does, and "ran unchecked" is then a document and not an absence. + pub(crate) fn allowed_all(&mut self) { + self.built.add_property(Property::new("allowed", "all")); + } + /// 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 - } + Outcome::Bypass(_) => skipped("bypassed by UPHOLD_ALLOW"), + Outcome::OutOfScope => skipped("out of scope at this destination"), }; let mut testcase = TestCase::new(case.id, status); // `local` rather than no classname: Allure files a testcase by its @@ -222,7 +408,11 @@ impl<'a> Suite<'a> { if let Some(set) = case.set { testcase.add_property(Property::new("set", set)); } - testcase.add_property(Property::new("stage", self.stage.as_str())); + for subject in case.subjects { + testcase.add_property(Property::new("subject", subject.as_str())); + } + let (where_, decided) = self.run.property(); + testcase.add_property(Property::new(where_, decided)); // 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"))); @@ -235,6 +425,24 @@ impl<'a> Suite<'a> { }, )); } + if outcome == Outcome::OutOfScope { + testcase.add_property(Property::new("scope", "destination")); + } + self.built.add_test_case(testcase); + } + + /// A testcase for a run that could not look at something no one rule + /// owns. Named for the tool rather than a rule, because no rule refused + /// and no rule was skipped. + fn could_not_look(&mut self) { + let mut testcase = TestCase::new( + "uphold", + non_success(NonSuccessKind::Error, "uphold", "could not look"), + ); + testcase.set_classname("uphold"); + let (where_, decided) = self.run.property(); + testcase.add_property(Property::new(where_, decided)); + testcase.add_property(Property::new("uphold", env!("CARGO_PKG_VERSION"))); self.built.add_test_case(testcase); } @@ -242,9 +450,11 @@ impl<'a> Suite<'a> { /// that got as far as opening the sink. pub(crate) fn write(self) -> Result<()> { let Self { - mut sink, built, .. + mut sink, + run, + built, } = self; - let mut report = Report::new("uphold guard"); + let mut report = Report::new(run.report()); report.add_test_suite(built); let failed = |error: &dyn std::fmt::Display| Fatal::at(&sink.place, format!("{VARIABLE}: {error}")); @@ -255,6 +465,175 @@ impl<'a> Suite<'a> { } } +/// What one seam invocation decided, held until it is written. +/// +/// A stage records into its suite as each rule answers. A seam keeps the +/// answers as a list instead, because that list is also what its report and +/// its exit are computed from -- the report a reader acts on and the file a +/// publisher renders are two readings of one list, so they cannot disagree -- +/// and the suite is built from it when the run is done. +/// +/// Written once, on every way out: before the shim hands the process to the +/// real command, since nothing runs after that, and otherwise when the seam +/// returns. +#[derive(Debug)] +pub(crate) struct Record { + place: Option, + run: Run, + sink: Option>, + allowed_all: bool, + written: bool, + /// Every rule's answer, in the order asked. A rule asked about several + /// subjects appears once per subject here and once in the file. + pub ledger: Vec, +} + +impl Record { + /// A record of `run`, written to `place` where one was asked for. + pub(crate) const fn new(place: Option, run: Run) -> Self { + Self { + place, + run, + sink: None, + allowed_all: false, + written: false, + ledger: Vec::new(), + } + } + + /// A record nobody asked for: it keeps the list a seam reports from, and + /// writes nothing. + pub(crate) const fn unrequested() -> Self { + Self::new(None, Run::Text) + } + + /// Open the file now rather than when it is written, so a place that + /// cannot be written stops the run while it has decided nothing. + pub(crate) fn open(&mut self) -> Result<()> { + if let (Some(place), None) = (&self.place, &self.sink) { + self.sink = Some(Sink::open(place, &self.run)?); + } + Ok(()) + } + + /// See [`Suite::allowed_all`]. + pub(crate) const fn allowed_all(&mut self) { + self.allowed_all = true; + } + + /// Write the document, if one was asked for and it is not written yet. + /// + /// `failed` is a run that ended at exit 2. Where its last answer is not + /// already a rule that could not look -- the destination could not be + /// read, the subject was not text -- the suite says so in a testcase of + /// its own, because a document of passes beside an exit 2 reads to a + /// publisher as a run that passed. + pub(crate) fn write(&mut self, failed: bool) -> Result<()> { + if std::mem::replace(&mut self.written, true) { + return Ok(()); + } + let Some(place) = &self.place else { + return Ok(()); + }; + let sink = match self.sink.take() { + Some(sink) => sink, + None => Sink::open(place, &self.run)?, + }; + let mut suite = Suite::new(sink, self.run.clone()); + if self.allowed_all { + suite.allowed_all(); + } + for merged in merged(&self.ledger) { + suite.record( + Case { + id: &merged.id, + builtin: merged.builtin.as_deref(), + set: merged.set.as_deref(), + subjects: &merged.subjects, + }, + merged.outcome, + ); + } + if failed + && self + .ledger + .last() + .is_none_or(|last| last.outcome() != Outcome::Unknown) + { + suite.could_not_look(); + } + suite.write() + } + + /// Write after the run ended in `result`, and hand `result` back: the + /// run's own error is still the one a reader acts on, with the file's + /// appended if both failed. + pub(crate) fn finish(&mut self, result: Result) -> Result { + let failed = matches!(result, Err(_) | Ok(Exit::Broken)); + match (self.write(failed), result) { + (Ok(()), result) => result, + (Err(also), Err(error)) => Err(Fatal::new(format!("{error}\n\n{also}"))), + (Err(also), Ok(_)) => Err(also), + } + } +} + +/// One testcase's worth of a seam's list: a rule asked about a title and then +/// a body is one rule, and two testcases would count one refusal twice. +struct Merged { + id: String, + builtin: Option, + set: Option, + subjects: Vec, + outcome: Outcome, +} + +/// The list folded to one entry per rule, in the order each was first asked. +/// The answer kept is the one a reader most needs: could not look over +/// refused over passed over did not run. +fn merged(ledger: &[Judgment]) -> Vec { + const fn weight(outcome: Outcome) -> u8 { + match outcome { + Outcome::Unknown => 4, + Outcome::Refuse => 3, + Outcome::Pass => 2, + Outcome::Bypass(_) => 1, + Outcome::OutOfScope => 0, + } + } + let mut merged: Vec = Vec::new(); + for judgment in ledger { + let outcome = judgment.outcome(); + if !merged.iter().any(|entry| entry.id == judgment.id) { + merged.push(Merged { + id: judgment.id.clone(), + builtin: judgment.builtin.clone(), + set: judgment.set.clone(), + subjects: Vec::new(), + outcome, + }); + } + let Some(entry) = merged.iter_mut().find(|entry| entry.id == judgment.id) else { + continue; + }; + if weight(outcome) > weight(entry.outcome) { + entry.outcome = outcome; + } + if let Some(subject) = &judgment.subject + && !entry.subjects.contains(subject) + { + entry.subjects.push(subject.clone()); + } + } + merged +} + +fn skipped(message: &str) -> TestCaseStatus { + let mut status = TestCaseStatus::skipped(); + status.set_message(message); + status +} + /// 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 { @@ -269,7 +648,10 @@ mod tests { 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"); + let mut suite = Suite::new( + Sink::new(&mut buffer, Path::new("memory")), + Run::Stage(String::from("pre-push")), + ); for (case, outcome) in cases { suite.record(*case, *outcome); } @@ -288,11 +670,13 @@ mod tests { id: "a-rule", builtin: Some("a-builtin"), set: Some("a-set"), + subjects: &[], }; const LOCAL: Case<'static> = Case { id: "b-rule", builtin: Some("b-builtin"), set: None, + subjects: &[], }; #[test] @@ -370,4 +754,71 @@ mod tests { let suite = &report.test_suites[0]; assert_eq!(suite.tests, 0); } + + /// The fifth state is a skip of its own, told from a bypass by its + /// message and its property. + #[test] + fn out_of_scope_is_skipped_and_says_the_destination_stood_it_down() { + let report = written(&[(LOCAL, Outcome::OutOfScope)]); + let suite = &report.test_suites[0]; + assert_eq!((suite.tests, suite.skipped), (1, 1)); + let case = &suite.test_cases[0]; + assert!(matches!( + &case.status, + TestCaseStatus::Skipped { message: Some(message), .. } + if message.as_str() == "out of scope at this destination" + )); + assert_eq!(property(case, "scope"), Some("destination")); + assert_eq!(property(case, "allowed"), None); + } + + #[test] + fn a_suite_name_becomes_a_file_name_of_dashes_and_lowercase() { + assert_eq!(Run::Text.slug(), "uphold-guard-text"); + assert_eq!(Run::Shim(String::from("gh")).slug(), "uphold-shim-gh"); + assert_eq!( + Run::Hook(String::from("claude-code")).slug(), + "uphold-hook-claude-code" + ); + } + + #[test] + fn two_ids_from_one_process_differ() { + assert_ne!(fresh_id(), fresh_id()); + } + + fn judged(id: &str, subject: &str, answer: crate::text::Answer) -> Judgment { + Judgment { + id: id.to_owned(), + builtin: None, + set: None, + subject: Some(subject.to_owned()), + answer, + } + } + + /// One rule asked about a title and a body is one testcase, carrying both + /// kinds and the answer that refused. + #[test] + fn a_rule_asked_about_two_subjects_is_one_testcase() { + use crate::text::Answer; + let refusal = crate::guard::Refusal { + id: String::from("a"), + report: String::from("no"), + }; + let merged = merged(&[ + judged("a", "title", Answer::Pass), + judged("b", "title", Answer::Pass), + judged( + "a", + "text", + Answer::Refused(crate::text::Verdict::Guard(refusal)), + ), + ]); + assert_eq!(merged.len(), 2); + assert_eq!(merged[0].id, "a"); + assert_eq!(merged[0].outcome, Outcome::Refuse); + assert_eq!(merged[0].subjects, ["title", "text"]); + assert_eq!(merged[1].outcome, Outcome::Pass); + } } diff --git a/src/main.rs b/src/main.rs index 156d86e..2637907 100644 --- a/src/main.rs +++ b/src/main.rs @@ -118,6 +118,7 @@ usage: uphold guard --stage STAGE [--junit FILE] run the guards that fire at STAGE uphold guard --text [FILE|-] run the text-capable guards over text + [--junit FILE|DIR] uphold check [--json] [--probe] reconcile claims; optionally measure hook behavior uphold check --coverage which rules run here and carry no principle uphold audit --for-publication what a private->public flip would republish @@ -140,6 +141,7 @@ usage: uphold rules --effective [--json] every rule this repository resolves to, and the git hooks each one fires at uphold hook claude-code|codex judge a pending agent tool call, read on stdin + [--junit DIR] uphold shim [args...] check what a command would publish, then run it uphold shim --install [COMMAND...] link this binary under each command's name uphold shim --status what is linked, and whether PATH reaches it @@ -171,6 +173,8 @@ exit codes: UPHOLD_ALLOW=, bypasses named guards for one invocation. `--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. +An existing directory instead gets one file per invocation. `guard --text` writes +the same document; the shim and `uphold hook` take only a directory. "; /// The policy file, newest name first. @@ -562,8 +566,14 @@ fn run() -> Result { uphold rules --effective [--json]\n\n{USAGE}" ))), }, - "hook" => match rest { - [harness] => { + "hook" => match hook_arguments(rest)? { + (Some(harness), junit_flag) => { + // Before the harness is looked up or the event read, as every + // seam judges it: a value that cannot be honoured is exit 2 while + // nothing has been decided. A directory only, since a harness + // calls this once per publishing call and one file would keep + // only the last. + let evidence = junit::directory_requested(junit_flag, "uphold hook")?; let working = std::env::current_dir()?; // Not `ok_or_else(no_policy_here)`, which is what every other // seam here does. A tool call is made from wherever the session @@ -574,10 +584,10 @@ fn run() -> Result { // everywhere and get the hook switched off within a day. What // still runs without a policy, and what does not, is // `hook::run`'s to do and to report. - hook::run(text_of(harness)?, discover(&working).as_ref()) + hook::run(text_of(harness)?, discover(&working).as_ref(), evidence) } - _ => Err(Fatal::new(format!( - "usage: uphold hook {}\n\n{USAGE}", + (None, _) => Err(Fatal::new(format!( + "usage: uphold hook {} [--junit DIR]\n\n{USAGE}", hook::known() ))), }, @@ -704,6 +714,31 @@ fn scan_command(arguments: &[OsString]) -> Result { Ok(exit) } +/// The harness `uphold hook` was named, and the `--junit` value beside it. +/// +/// The flag is accepted here and not at the shim because this command line is +/// the harness's settings file, which the operator writes. `None` for the +/// harness where there is not exactly one, which the caller answers with usage. +fn hook_arguments(rest: &[OsString]) -> Result<(Option<&OsString>, Option<&OsStr>)> { + let mut harness = None; + let mut junit_flag = None; + let mut extra = false; + let mut words = rest.iter(); + while let Some(word) = words.next() { + if word == junit::FLAG { + let value = words + .next() + .ok_or_else(|| Fatal::new(format!("{} needs a value", junit::FLAG)))?; + junit_flag = Some(value.as_os_str()); + } else if harness.is_none() { + harness = Some(word); + } else { + extra = true; + } + } + Ok((harness.filter(|_| !extra), junit_flag)) +} + fn guard_command(arguments: &[OsString]) -> Result { let mut stage: Option = None; let mut message: Option = None; @@ -767,22 +802,6 @@ fn guard_command(arguments: &[OsString]) -> Result { // 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))?; @@ -809,10 +828,13 @@ fn guard_command(arguments: &[OsString]) -> Result { // and a seam that consulted one and not the other would be the same // rule with two answers. // - // Both arms become a refusal, because that is the word this command - // reports in: a finding reached through `--text` and one reached - // through a guard are the same verdict on the same text. - let refusals: Vec = text::judged( + // Every rule's answer goes into the record, and the report below and + // the exit are read back off it, so the file a caller asked for and + // the refusals on stderr cannot disagree. Opened before the first rule + // is asked, as a stage's is. + let mut record = junit::Record::new(evidence, junit::Run::Text); + record.open()?; + let judged = text::judged( &invocation::Invocation::text( text::Seam::Guard, &source, @@ -821,21 +843,31 @@ fn guard_command(arguments: &[OsString]) -> Result { ), &root, &policy, - )? - .into_iter() - .map(|verdict| match verdict { - text::Verdict::Guard(refusal) => refusal, - text::Verdict::Rule(failure) => guard::Refusal { - report: failure.report(), - id: failure.label, - }, - }) - .collect(); + &mut record.ledger, + ); + let verdicts = match judged { + Ok(verdicts) => verdicts, + Err(error) => return record.finish(Err(error)), + }; + record.write(false)?; + // Both arms become a refusal, because that is the word this command + // reports in: a finding reached through `--text` and one reached + // through a guard are the same verdict on the same text. + let refusals: Vec = verdicts + .into_iter() + .map(|verdict| match verdict { + text::Verdict::Guard(refusal) => refusal, + text::Verdict::Rule(failure) => guard::Refusal { + report: failure.report(), + id: failure.label, + }, + }) + .collect(); for refusal in &refusals { eprintln!("guard refused: {}", guard::refused_by(&policy, refusal)); eprintln!("{}", refusal.report.trim_end()); } - let exit = verdict(refusals.len(), 0); + let exit = verdict(text::refusals(&record.ledger), 0); if exit == Exit::Clean { println!("text checks passed"); } @@ -865,7 +897,10 @@ fn guard_command(arguments: &[OsString]) -> Result { // 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()?; + let evidence = evidence + .as_ref() + .map(|place| junit::Sink::open(place, &junit::Run::Stage(stage.as_str().to_owned()))) + .transpose()?; guard::run( &root, @@ -1285,6 +1320,28 @@ fn shim_command(name: &str, argv: &[OsString], invoked: shim::Invoked) -> Result { return Ok(exit); } + // Judged next, and before anything this invocation decides or runs: a + // value the shim cannot honour is exit 2 with the wrapped command not run, + // as every other exit 2 here is. The variable and not a flag, because the + // argv is the wrapped command's and the shim owns none of it. + let place = junit::directory_requested(None, "uphold shim")?; + let mut record = junit::Record::new( + place.map(junit::Place::Directory), + junit::Run::Shim(name.to_owned()), + ); + // Written on every way out: by the hand-off, before the process becomes + // the real command, and here for every run that returns instead. + let judged = shim_judged(name, argv, invoked, &mut record); + record.finish(judged) +} + +/// [`shim_command`] once the evidence it writes is settled. +fn shim_judged( + name: &str, + argv: &[OsString], + invoked: shim::Invoked, + record: &mut junit::Record, +) -> Result { let working = std::env::current_dir()?; // No policy where the command was typed means no repository here declares // anything to stand in front of it. Run as the command, that is almost @@ -1311,7 +1368,7 @@ fn shim_command(name: &str, argv: &[OsString], invoked: shim::Invoked) -> Result // command. let Some((root, policy_path)) = discover(&working) else { return match invoked { - shim::Invoked::AsTheCommand => shim::run_without_policy(&working, name, argv), + shim::Invoked::AsTheCommand => shim::run_without_policy(&working, name, argv, record), shim::Invoked::ByName => Err(no_policy_here(&working)), // Re-entered as an editor with no policy to read. The no-policy // reading of `gh` installs this editor itself, for a body bound for @@ -1331,7 +1388,7 @@ fn shim_command(name: &str, argv: &[OsString], invoked: shim::Invoked) -> Result working.display() ))) }, - |policy| shim::run(&working, &policy, name, argv, invoked), + |policy| shim::run(&working, &policy, name, argv, invoked, record), ), }; }; @@ -1350,13 +1407,16 @@ fn shim_command(name: &str, argv: &[OsString], invoked: shim::Invoked) -> Result // // It is still not a pass. Nothing is checked here and nothing pretends to // be: the line below says so on stderr, every time, so a bypass that became - // habit is visible in a shell history and in a CI log. + // habit is visible in a shell history and in a CI log -- and the record + // says so too, as a suite of no rules that carries `allowed=all`, so "ran + // unchecked" is a document and not an absence. if guard::bypassed_entirely() { eprintln!( "uphold shim: {name} ran unchecked, by UPHOLD_ALLOW=all. Nothing here looked at \ what it publishes." ); - return shim::exec_through(name, argv); + record.allowed_all(); + return shim::exec_through(name, argv, record); } // A policy that exists and cannot be read is fatal, and the refusal names // the way out. Without that line the reader is holding a broken declaration @@ -1377,7 +1437,7 @@ fn shim_command(name: &str, argv: &[OsString], invoked: shim::Invoked) -> Result // command that runs is the command that was typed. Where the shim has // something to CHECK, it refuses the untranslatable argument itself, in the // words of what it could not read. - shim::run(&root, &policy, name, argv, invoked) + shim::run(&root, &policy, name, argv, invoked, record) } fn main() { diff --git a/src/prose.rs b/src/prose.rs index 1db1c34..dbac229 100644 --- a/src/prose.rs +++ b/src/prose.rs @@ -28,7 +28,7 @@ use crate::comments::{self, Language}; use crate::config::{CheckKind, Policy}; use crate::error::{Fatal, Result}; use crate::report::Failure; -use crate::text::Seam; +use crate::text::{Answer, Judgment, Seam, Verdict, asked}; /// One run of prose, unwrapped. #[derive(Debug, Clone, PartialEq, Eq)] @@ -401,8 +401,16 @@ pub(crate) fn compile(pattern: &str, id: &str) -> Result { /// And only the rules whose `seams`, where given, names `seam`: a rule written /// to refuse a command through the shim and the hook is not asked about a /// commit message that merely mentions the command. -pub(crate) fn over_text(policy: &Policy, seam: Seam, text: &str) -> Result> { - let mut failures = Vec::new(); +/// +/// Each rule asked answers into `ledger`, under `recorded`, the kind of +/// subject the seam calls this text. +pub(crate) fn over_text( + policy: &Policy, + seam: Seam, + text: &str, + recorded: &str, + ledger: &mut Vec, +) -> Result<()> { for rule in policy.of_check(CheckKind::ProseRegexp) { if rule .command @@ -418,11 +426,17 @@ pub(crate) fn over_text(policy: &Policy, seam: Seam, text: &str) -> Result Result>() .join("\n"); - if body.is_empty() { - continue; - } - failures.push(Failure::new(&rule.id, rule.message(), body)); + ledger.push(Judgment::of( + rule, + Some(recorded), + if body.is_empty() { + Answer::Pass + } else { + Answer::Refused(Verdict::Rule(Failure::new(&rule.id, rule.message(), body))) + }, + )); } - Ok(failures) + Ok(()) } #[cfg(test)] diff --git a/src/shim.rs b/src/shim.rs index d4f1eca..e833c34 100644 --- a/src/shim.rs +++ b/src/shim.rs @@ -44,6 +44,8 @@ use crate::config::{Policy, Rule}; use crate::error::{Exit, Fatal, Result}; use crate::git; use crate::invocation::Invocation; +use crate::junit::Record; +use crate::text::{Answer, Judgment, Verdict}; mod checker; mod destination; @@ -542,6 +544,7 @@ impl Subject { pub(crate) fn published(&self) -> crate::guard::Published<'_> { crate::guard::Published { label: self.kind, + kind: self.kind, text: &self.value, added: self.added.as_deref(), headline: self.kind == Self::HEADLINE, @@ -5301,8 +5304,10 @@ fn identity_of_token(token: &str) -> Option { /// loop is named and nothing runs, because a probe that got that far is not /// answering a question. pub(crate) fn inner_passthrough(name: &str, argv: &[OsString]) -> Result> { + // uphold's own probe, or its own hand-off coming back: not the operator's + // invocation, so it writes no record of its own. if handed_back(name).is_some() { - return exec_through(name, argv).map(Some); + return exec_through(name, argv, &mut Record::unrequested()).map(Some); } let Some(depth) = inner_depth() else { return Ok(None); @@ -5320,7 +5325,7 @@ pub(crate) fn inner_passthrough(name: &str, argv: &[OsString]) -> Result Result Result { +/// +/// `record` is written before the hand-off, since nothing runs after it: a +/// run that judged nothing writes a suite of no rules, which says it ran. +pub(crate) fn exec_through(name: &str, argv: &[OsString], record: &mut Record) -> Result { let own = std::env::current_exe().ok(); let Some(real) = real_command(name, own.as_deref()) else { return Err(Fatal::new(format!( @@ -5344,7 +5352,7 @@ pub(crate) fn exec_through(name: &str, argv: &[OsString]) -> Result { }; let mut command = handing_off(&real, name); command.args(argv); - hand_off(&mut command, name, None) + hand_off(&mut command, name, None, record) } /// What the policy built for a command typed where none is in reach is called @@ -5407,9 +5415,14 @@ pub(crate) fn without_policy(root: &Path, name: &str) -> Result> /// exists to avoid. What the table names goes through [`run`], scope and all, so a /// private destination stands down there exactly as it does in a repository /// that declared the same table. -pub(crate) fn run_without_policy(root: &Path, name: &str, argv: &[OsString]) -> Result { +pub(crate) fn run_without_policy( + root: &Path, + name: &str, + argv: &[OsString], + record: &mut Record, +) -> Result { let Some(policy) = without_policy(root, name)? else { - return exec_through(name, argv); + return exec_through(name, argv, record); }; let words: Vec = argv .iter() @@ -5422,9 +5435,9 @@ pub(crate) fn run_without_policy(root: &Path, name: &str, argv: &[OsString]) -> && shim.carries_a_body(&words) }); if !publishes { - return exec_through(name, argv); + return exec_through(name, argv, record); } - run(root, &policy, name, argv, Invoked::AsTheCommand) + run(root, &policy, name, argv, Invoked::AsTheCommand, record) } /// The real command, found by walking PATH past ourselves. @@ -5691,7 +5704,13 @@ fn install_editor( /// Refusing here is what makes it a checkpoint rather than a report: `gh` and /// `glab` abandon what they were doing when their editor exits non-zero, /// exactly as git abandons a commit when `commit-msg` does. -fn edit_and_check(root: &Path, policy: &Policy, name: &str, argv: &[String]) -> Result { +fn edit_and_check( + root: &Path, + policy: &Policy, + name: &str, + argv: &[String], + record: &mut Record, +) -> Result { // The command appends the file it wants written to the editor command line, // so the last word is that path however this process was routed back here. let Some(file) = argv.last() else { @@ -5792,8 +5811,18 @@ fn edit_and_check(root: &Path, policy: &Policy, name: &str, argv: &[String]) -> let shim = *shim; let mut kept: Vec<&Rule> = Vec::new(); for rule in &named { - if scopes.holds(effective_scope(rule, shim), policy)? { + let holds = crate::text::asked( + &mut record.ledger, + rule, + Some(subject.kind), + scopes.holds(effective_scope(rule, shim), policy), + )?; + if holds { kept.push(*rule); + } else { + record + .ledger + .push(Judgment::of(rule, Some(subject.kind), Answer::OutOfScope)); } } if kept.is_empty() { @@ -5817,7 +5846,12 @@ fn edit_and_check(root: &Path, policy: &Policy, name: &str, argv: &[String]) -> let invocation = Invocation::command(name, std::slice::from_ref(&subject)); let mut refusals: Vec = Vec::new(); for rule in checkers { - if crate::guard::bypassed(&rule.id) { + if let Some(allowed) = crate::guard::allowed(&rule.id) { + record.ledger.push(Judgment::of( + rule, + Some(subject.kind), + Answer::Bypass(allowed), + )); continue; } if !rule.selects_subject(subject.kind) { @@ -5834,11 +5868,15 @@ fn edit_and_check(root: &Path, policy: &Policy, name: &str, argv: &[String]) -> if !rule.judged_at(invocation.seam) { continue; } + let start = record.ledger.len(); match kind { crate::text::Judged::Prose | crate::text::Judged::Patterns => { - if let Some(refusal) = pattern_refusal(rule, &subject)? { - refusals.push(refusal); - } + answered( + &mut record.ledger, + rule, + subject.kind, + pattern_refusal(rule, &subject), + )?; } crate::text::Judged::Guards => { // As in `run`: a rule this consultation reaches keeps its own @@ -5853,29 +5891,37 @@ fn edit_and_check(root: &Path, policy: &Policy, name: &str, argv: &[String]) -> // scope to judge under and nothing to stand down for. None => Ok(true), }; - if let Some(refusal) = crate::guard::text_refusal( + crate::guard::text_refusal( root, policy, rule, destination.as_deref(), &subject.published(), &mut in_scope, - )? { - refusals.push(refusal.report); - } + &mut record.ledger, + )?; } crate::text::Judged::Consultation => { - if let Some(refusal) = consult(root, rule, &subject)? { - refusals.push(format!("{refusal}\n{}", rule.message())); - } + answered( + &mut record.ledger, + rule, + subject.kind, + consult(root, rule, &subject) + .map(|found| found.map(|refusal| format!("{refusal}\n{}", rule.message()))), + )?; } // Not consulted here, and skipped above: a policy reaches the // literal rules from this seam through the `text-literals` // built-in, which arrives as `Guards`. crate::text::Judged::Literals => {} } + if let Some(refusal) = + crate::guard::told(policy, rule, record.ledger.get(start..).unwrap_or_default()) + { + refusals.push(refusal.report); + } } - if refusals.is_empty() { + if crate::text::refusals(&record.ledger) == 0 { return Ok(Exit::Clean); } for refusal in &refusals { @@ -5888,6 +5934,30 @@ fn edit_and_check(root: &Path, policy: &Policy, name: &str, argv: &[String]) -> Ok(Exit::Violations) } +/// One rule's answer about one subject, put in the list: a pass, or a +/// refusal carrying the report the shim prints for it. The kinds the shim +/// renders itself -- a pattern, a program it consults -- arrive here already +/// worded; a guard's answer goes in through `guard::text_refusal`. +fn answered( + ledger: &mut Vec, + rule: &Rule, + subject: &str, + found: Result>, +) -> Result<()> { + let found = crate::text::asked(ledger, rule, Some(subject), found)?; + ledger.push(Judgment::of( + rule, + Some(subject), + found.map_or(Answer::Pass, |report| { + Answer::Refused(Verdict::Guard(crate::guard::Refusal { + id: rule.id.clone(), + report, + })) + }), + )); + Ok(()) +} + /// Hand the process over to the real command. /// /// A real `exec`, not a spawn and a wait. The shim is not a supervisor: exec @@ -5897,9 +5967,18 @@ fn edit_and_check(root: &Path, policy: &Policy, name: &str, argv: &[String]) -> /// `exit(status.code().unwrap_or(1))` loses all of it -- `code()` is `None` for /// every death by a signal, so a command killed by SIGINT reported a plain exit /// 1, which in this tool's own vocabulary is a policy violation. +/// +/// The record is written first, because after `exec` there is no process left +/// to write it. #[cfg(unix)] -fn hand_off(command: &mut Command, name: &str, stdin: Option<&[u8]>) -> Result { +fn hand_off( + command: &mut Command, + name: &str, + stdin: Option<&[u8]>, + record: &mut Record, +) -> Result { use std::os::unix::process::CommandExt; + record.write(false)?; // A file rather than a pipe, and only here. After `exec` there is no process // left to feed a pipe, so the bytes have to be somewhere the kernel can hand // over on its own -- and the file is unlinked while still open, which leaves @@ -5914,7 +5993,15 @@ fn hand_off(command: &mut Command, name: &str, stdin: Option<&[u8]>) -> Result) -> Result { +fn hand_off( + command: &mut Command, + name: &str, + stdin: Option<&[u8]>, + record: &mut Record, +) -> Result { + // Before the spawn, for the reason the Unix branch writes before `exec`: + // this branch leaves by `process::exit`, and nothing after it runs. + record.write(false)?; // A pipe rather than a file, and for a reason that is not symmetry: this // branch does not exec, it spawns and waits, so there IS a process left to // write the bytes -- and unlink-on-open is a Unix property. Windows refuses @@ -5960,12 +6047,17 @@ fn hand_off(command: &mut Command, name: &str, stdin: Option<&[u8]>) -> Result Result { let words: Vec = argv .iter() @@ -5976,7 +6068,7 @@ pub(crate) fn run( // else: in this pass argv is an editor's argv -- one file path -- and none // of the flag reading below applies to it. if invoked == Invoked::AsEditor { - return edit_and_check(root, policy, name, &words); + return edit_and_check(root, policy, name, &words, record); } let shims: BTreeMap<&str, &Shim> = policy @@ -6004,7 +6096,7 @@ pub(crate) fn run( // front of anything, and the caller is entitled to hear that rather // than watch a typo run. return match invoked { - Invoked::AsTheCommand => exec_through(name, argv), + Invoked::AsTheCommand => exec_through(name, argv, record), Invoked::ByName => Err(Fatal::new(format!( "no shim declares the command {name:?}; this policy declares {}", if shims.is_empty() { @@ -6106,9 +6198,29 @@ pub(crate) fn run( let mut scopes = CommandContext::new(shim, root, &collected, &words); let in_scope = scopes.holds(&shim.scope, policy)?; for rule in &checkers { - if scopes.holds(effective_scope(rule, shim), policy)? { + let holds = crate::text::asked( + &mut record.ledger, + rule, + None, + scopes.holds(effective_scope(rule, shim), policy), + )?; + if holds { any_applies = true; } + // Recorded here, once per rule, for the two reasons a rule below + // is passed over without being asked: switched off, or stood down + // at this destination. Neither is a pass and neither is an + // absence -- the rule was seen and set aside, and a reader asking + // whether the private-name check ran on this body is owed that. + if let Some(allowed) = crate::guard::allowed(&rule.id) { + record + .ledger + .push(Judgment::of(rule, None, Answer::Bypass(allowed))); + } else if !holds { + record + .ledger + .push(Judgment::of(rule, None, Answer::OutOfScope)); + } } // A `[[shim]]` that named this invocation and a policy with no rule // standing in front of it: the shim collects the body, consults nobody, @@ -6182,7 +6294,8 @@ pub(crate) fn run( if !scopes.holds(effective_scope(rule, shim), policy)? { continue; } - if let Some(refusal) = crate::guard::target_refusal( + let start = record.ledger.len(); + crate::guard::target_refusal( root, policy, rule, @@ -6190,7 +6303,11 @@ pub(crate) fn run( forge, hostname.as_deref(), &mut asked, - )? { + &mut record.ledger, + )?; + if let Some(refusal) = + crate::guard::told(policy, rule, record.ledger.get(start..).unwrap_or_default()) + { destination_refusals += 1; // Named the way a hook names a refusal, set and all. This // rule ships in `published-text`, so the reader who greps @@ -6264,11 +6381,15 @@ pub(crate) fn run( if !rule.judged_at(invocation.seam) { continue; } + let start = record.ledger.len(); match kind { crate::text::Judged::Prose | crate::text::Judged::Patterns => { - if let Some(refusal) = pattern_refusal(rule, subject)? { - refusals.push(refusal); - } + answered( + &mut record.ledger, + rule, + subject.kind, + pattern_refusal(rule, subject), + )?; } // A subject that was named and left empty is still a // published subject, and a pattern rule judges it above @@ -6300,32 +6421,45 @@ pub(crate) fn run( } scopes.holds(effective_scope(inner, shim), policy) }; - if let Some(refusal) = crate::guard::text_refusal( + crate::guard::text_refusal( root, policy, rule, destination.as_deref(), &subject.published(), &mut inner_in_scope, - )? { - refusals.push(refusal.report); - } + &mut record.ledger, + )?; } crate::text::Judged::Consultation => { - if let Some(refusal) = consult(root, rule, subject)? { - refusals.push(format!("{refusal}\n{}", rule.message())); - } + answered( + &mut record.ledger, + rule, + subject.kind, + consult(root, rule, subject).map(|found| { + found.map(|refusal| format!("{refusal}\n{}", rule.message())) + }), + )?; } // See the editor pass: reached from here through the // `text-literals` built-in, which arrives as `Guards`. crate::text::Judged::Literals => {} } + // The report is read back off what this rule just answered, + // so it cannot say anything the record does not. + if let Some(refusal) = crate::guard::told( + policy, + rule, + record.ledger.get(start..).unwrap_or_default(), + ) { + refusals.push(refusal.report); + } } } } } - if !refusals.is_empty() { + if crate::text::refusals(&record.ledger) > 0 { for refusal in &refusals { eprintln!("{name}: {refusal}"); } @@ -6386,7 +6520,7 @@ pub(crate) fn run( EditorPath::Closed => {} } } - hand_off(&mut command, name, collected.stdin.as_deref()) + hand_off(&mut command, name, collected.stdin.as_deref(), record) } #[cfg(test)] @@ -7735,9 +7869,13 @@ mod tests { // The transparent path still has to be a path. A link left behind // after the real command was uninstalled has nothing to exec, and // exiting 0 there is a shim reporting a command ran when none did. - let report = exec_through("uphold-no-such-command-4b1f", &[]) - .unwrap_err() - .to_string(); + let report = exec_through( + "uphold-no-such-command-4b1f", + &[], + &mut Record::unrequested(), + ) + .unwrap_err() + .to_string(); assert!( report.contains("nothing here stands in front of"), "{report}" diff --git a/src/shim/checker.rs b/src/shim/checker.rs index adb6b08..b2c270b 100644 --- a/src/shim/checker.rs +++ b/src/shim/checker.rs @@ -111,6 +111,12 @@ pub(super) fn consult(root: &Path, rule: &Rule, subject: &Subject) -> Result, + /// The bundled set the rule came from. + pub set: Option, + pub subject: Option, + pub answer: Answer, +} + +impl Judgment { + pub(crate) fn of(rule: &Rule, subject: Option<&str>, answer: Answer) -> Self { + Self { + id: rule.id.clone(), + builtin: rule.builtin().map(str::to_owned), + set: rule.origin.set().map(str::to_owned), + subject: subject.map(str::to_owned), + answer, + } + } + + /// The refusal this entry carries, if it refused. + pub(crate) const fn refused(&self) -> Option<&Verdict> { + match &self.answer { + Answer::Refused(verdict) => Some(verdict), + _ => None, + } + } +} + +/// Anything that is one rule's answer, as the record names it. A trait so the +/// count every seam settles its exit on is one function over the real list +/// and over the model of it the proofs below reason about. +pub(crate) trait Answered { + fn outcome(&self) -> Outcome; +} + +impl Answered for Judgment { + fn outcome(&self) -> Outcome { + match &self.answer { + Answer::Pass => Outcome::Pass, + Answer::Refused(_) => Outcome::Refuse, + Answer::Unknown => Outcome::Unknown, + Answer::Bypass(allowed) => Outcome::Bypass(*allowed), + Answer::OutOfScope => Outcome::OutOfScope, + } + } +} + +/// How many answers refused: the one number a seam's exit is read from. +pub(crate) fn refusals(answers: &[V]) -> usize { + answers + .iter() + .filter(|answer| answer.outcome() == Outcome::Refuse) + .count() +} + +/// Ask `rule` something, and where it could not answer put that in the list +/// before the error goes up: the run stops here, and the record of it must say +/// which rule it stopped at. +pub(crate) fn asked( + ledger: &mut Vec, + rule: &Rule, + subject: Option<&str>, + asking: Result, +) -> Result { + if asking.is_err() { + ledger.push(Judgment::of(rule, subject, Answer::Unknown)); + } + asking +} + +/// The findings a seam reports, read off its list in the order they were +/// asked. A guard asked about a body and again about each headline is one +/// refusal per rule -- see [`merged_by_rule`] -- placed where its first one +/// was. +pub(crate) fn verdicts(ledger: &[Judgment]) -> Vec { + let mut found = Vec::new(); + let mut guards = Vec::new(); + let mut guards_at = None; + for verdict in ledger.iter().filter_map(Judgment::refused) { + match verdict { + Verdict::Rule(failure) => found.push(Verdict::Rule(failure.clone())), + Verdict::Guard(refusal) => { + guards_at.get_or_insert(found.len()); + guards.push(refusal.clone()); + } + } + } + if let Some(at) = guards_at { + found.splice( + at..at, + merged_by_rule(guards).into_iter().map(Verdict::Guard), + ); + } + found +} + +/// The list of one seam, given how each kind of rule it consults answered. /// /// Every input arrives through `answer`, so nothing here reads a file, runs a /// command or looks at a policy. That is what lets `#[cfg(kani)] mod proofs` /// state the fail-closed property over every combination of answers rather /// than over the rules a test can write. [`judged`] is the caller that knows /// what each kind reads. +/// +/// The list is the caller's and not a return value, because a kind that +/// could not look stops the seam and what was answered before it -- the +/// `Unknown` among it -- is still the record of the run. pub(crate) fn over_kinds( seam: Seam, - mut answer: impl FnMut(Judged) -> Result>, -) -> Result> { - let mut verdicts: Vec = Vec::new(); + ledger: &mut Vec, + mut answer: impl FnMut(Judged, &mut Vec) -> Result<()>, +) -> Result<()> { for kind in EVERY_JUDGED { if seam.consults(*kind) { - verdicts.extend(answer(*kind)?); + answer(*kind, ledger)?; } } - Ok(verdicts) + Ok(()) } /// One headline a seam was handed beside its prose: a title, or a commit @@ -306,16 +444,30 @@ pub(crate) struct Headline { /// leaves the tool observable, while unread required prose stops judgment. /// Checkers read prose and headlines joined; guards receive each headline /// separately, preserving the same headline treatment as command collection. +/// +/// Every rule's answer goes into `ledger`, and what the seam reports is read +/// back off it. The subject each is recorded under is the tool's name at the +/// hook and `text` elsewhere: a `--text` label is a file name, and a record +/// carries no path. pub(crate) fn judged( invocation: &Invocation<'_>, root: &std::path::Path, policy: &Policy, + ledger: &mut Vec, ) -> Result> { let input = invocation.text_input()?; let prose = input.prose.read()?.copied(); let headlines = input.headlines; let seam = invocation.seam; let label = invocation.label; + let recorded = |named: &'_ str| -> String { + if seam == Seam::Hook { + named.to_owned() + } else { + String::from("text") + } + }; + let subject = recorded(label); let has_text = prose.is_some() || !headlines.is_empty(); let text = prose.unwrap_or_default(); let joined; @@ -329,15 +481,12 @@ pub(crate) fn judged( .join("\n"); joined.as_str() }; - over_kinds(seam, |kind| { + over_kinds(seam, ledger, |kind, ledger| { if !has_text && kind != Judged::Patterns { - return Ok(Vec::new()); + return Ok(()); } - Ok(match kind { - Judged::Literals => failures_in(root, policy, whole)? - .into_iter() - .map(Verdict::Rule) - .collect(), + match kind { + Judged::Literals => failures_in(root, policy, whole, Some(&subject), ledger), // Every inner rule is in scope here: the three seams that reach // this line -- `scan --text`, `guard --text`, the harness hook -- // are handed text and no destination, so there is nothing to ask a @@ -348,46 +497,44 @@ pub(crate) fn judged( // body, or a tool call's strings joined, and neither says which // part of it is a subject. The headlines are, each on its own, // because a subject is judged line by line as one. - let mut verdicts = Vec::new(); if !text.is_empty() || headlines.is_empty() { let published = crate::guard::Published { label, + kind: &subject, text, added: None, headline: false, }; - verdicts.extend(crate::guard::over_text( + crate::guard::over_text( root, policy, None, &published, &mut |_| Ok(true), - )?); + ledger, + )?; } for headline in headlines { + let headline_kind = recorded(&headline.label); let published = crate::guard::Published { label: &headline.label, + kind: &headline_kind, text: &headline.text, added: None, headline: true, }; - verdicts.extend(crate::guard::over_text( + crate::guard::over_text( root, policy, None, &published, &mut |_| Ok(true), - )?); + ledger, + )?; } - merged_by_rule(verdicts) - .into_iter() - .map(Verdict::Guard) - .collect() + Ok(()) } - Judged::Prose => crate::prose::over_text(policy, seam, whole)? - .into_iter() - .map(Verdict::Rule) - .collect(), + Judged::Prose => crate::prose::over_text(policy, seam, whole, &subject, ledger), // Only the hook consults these here, and only the rules whose // `seams` names it -- see `Seam::runs_by_default`. Judged::Patterns => patterns_over( @@ -396,16 +543,16 @@ pub(crate) fn judged( invocation.tool(), has_text.then_some(whole), headlines, - )? - .into_iter() - .map(Verdict::Rule) - .collect(), + &subject, + ledger, + ), // The shim's, and the shim does not arrive here: it holds a subject // and a per-rule scope this function is not given, so it dispatches // rule by rule through `Judged::of` instead. - Judged::Consultation => Vec::new(), - }) - }) + Judged::Consultation => Ok(()), + } + })?; + Ok(verdicts(ledger)) } /// One refusal per rule, from the several parts of a call a rule was run over. @@ -482,12 +629,14 @@ fn patterns_over( tool: Option<&str>, text: Option<&str>, headlines: &[Headline], -) -> Result> { + recorded: &str, + ledger: &mut Vec, +) -> Result<()> { // The shim runs its pattern rules itself, over the subjects it collected, // and never through this function; were it ever handed here, running its // rules over one pathless blob is the guess this module exists to refuse. if seam != Seam::Hook { - return Ok(Vec::new()); + return Ok(()); } // Each with the label a report names it by: the kind alone for the call's // own two, the key for a headline, so a finding says which field it was in. @@ -522,7 +671,6 @@ fn patterns_over( }, ) })); - let mut failures = Vec::new(); for rule in &policy.rules { if !matches!(rule.kind(), CheckKind::Regexp | CheckKind::RequireRegexp) || rule.command.is_none() @@ -530,8 +678,9 @@ fn patterns_over( { continue; } - if crate::guard::bypassed(&rule.id) { + if let Some(allowed) = crate::guard::allowed(&rule.id) { eprintln!("uphold: {} bypassed by UPHOLD_ALLOW", rule.id); + ledger.push(Judgment::of(rule, Some(recorded), Answer::Bypass(allowed))); continue; } let mut found = Vec::new(); @@ -539,7 +688,13 @@ fn patterns_over( .iter() .filter(|(_, subject)| rule.selects_subject(subject.kind)) { - if let Some(finding) = crate::shim::pattern_finding(rule, subject)? { + let finding = asked( + ledger, + rule, + Some(recorded), + crate::shim::pattern_finding(rule, subject), + )?; + if let Some(finding) = finding { let at = named.map_or_else(String::new, |name| format!(" (in {name})")); found.push(if policy.redact_matches { format!( @@ -551,18 +706,30 @@ fn patterns_over( }); } } - if !found.is_empty() { - failures.push(Failure::new(&rule.id, rule.message(), found.join("\n"))); - } + ledger.push(Judgment::of( + rule, + Some(recorded), + if found.is_empty() { + Answer::Pass + } else { + Answer::Refused(Verdict::Rule(Failure::new( + &rule.id, + rule.message(), + found.join("\n"), + ))) + }, + )); } - Ok(failures) + Ok(()) } pub(crate) fn check(found: Option<&(PathBuf, PathBuf)>, source: &str) -> Result { let text = read(source)?; let (root, policy) = load_for(found)?; let invocation = Invocation::text(Seam::Scan, source, Observed::Present(&text), &[]); - let verdicts = judged(&invocation, &root, &policy)?; + // `scan --text` writes no record; the list is what it reports from. + let mut ledger = Vec::new(); + let verdicts = judged(&invocation, &root, &policy, &mut ledger)?; for verdict in &verdicts { match verdict { @@ -577,7 +744,7 @@ pub(crate) fn check(found: Option<&(PathBuf, PathBuf)>, source: &str) -> Result< } } } - let exit = verdict(verdicts.len(), 0); + let exit = verdict(refusals(&ledger), 0); if exit == Exit::Clean { println!("policy checks passed (text)"); } @@ -598,16 +765,20 @@ pub(crate) fn load_for(found: Option<&(PathBuf, PathBuf)>) -> Result<(PathBuf, P } } -/// The same rules over an already-loaded policy. +/// The literal rules, and the running-host fallback under them, over an +/// already-loaded policy, each answering into `ledger` once per needle found +/// or once to pass. /// -/// Split from [`failures`] for the `text-literals` built-in: a shim consulting +/// Over the loaded policy for the `text-literals` built-in: a shim consulting /// it already holds the policy that named the rule, and loading it again from /// disk would be a second reading free to disagree with the first. pub(crate) fn failures_in( root: &std::path::Path, policy: &Policy, text: &str, -) -> Result> { + recorded: Option<&str>, + ledger: &mut Vec, +) -> Result<()> { // The test is for the identity rule itself, not for the CHECK KIND it // happens to use. Asking whether any `forbidden_literals` rule existed made // an unrelated one -- a repository's own list of literals, a command @@ -628,25 +799,34 @@ pub(crate) fn failures_in( owned.push(fallback_rule()); } - let mut failures: Vec = Vec::new(); for rule in &owned { - let needles = sources::resolve( - // `forbidden_literals_from` IS the command source; v2 said it twice. - if rule.forbidden_literals_from().is_some() { - "command" - } else { - rule.forbidden_literals().unwrap_or_default() - }, - rule.forbidden_literals_from(), - root, - rule.files().word, - &rule.id, - rule.ignore_literals(), + let needles = asked( + ledger, + rule, + recorded, + sources::resolve( + // `forbidden_literals_from` IS the command source; v2 said it twice. + if rule.forbidden_literals_from().is_some() { + "command" + } else { + rule.forbidden_literals().unwrap_or_default() + }, + rule.forbidden_literals_from(), + root, + rule.files().word, + &rule.id, + rule.ignore_literals(), + ), )?; + let mut refused = false; for needle in needles { let label = format!("{} ({})", rule.id, needle.label); - let hits = - engine::search_text(text, &Query::literal(&needle.value, needle.word), &label)?; + let hits = asked( + ledger, + rule, + recorded, + engine::search_text(text, &Query::literal(&needle.value, needle.word), &label), + )?; if hits.is_empty() { continue; } @@ -662,27 +842,38 @@ pub(crate) fn failures_in( }) .collect::>() .join("\n"); - failures.push(Failure::new(label, rule.message(), body)); + refused = true; + ledger.push(Judgment::of( + rule, + recorded, + Answer::Refused(Verdict::Rule(Failure::new(label, rule.message(), body))), + )); + } + if !refused { + ledger.push(Judgment::of(rule, recorded, Answer::Pass)); } } - Ok(failures) + Ok(()) } /// The fail-closed property of the published-text seams, over every answer. /// /// `cargo kani`, and see CONTRIBUTING for what it costs. What a kind of rule -/// answers is left to the model checker: could not look, found nothing, or -/// found one thing. The rule bodies themselves -- a regex, a literal search, a -/// command source, a guard reading the repository -- are the part no harness -/// here reaches. What is proven is that no combination of their answers -/// reaches clean except the one where every kind the seam asks looked and -/// found nothing. +/// answers is left to the model checker: could not look, or one answer of the +/// five a rule gives -- passed, refused, bypassed by id or by `all`, out of +/// scope -- or none at all. The rule bodies themselves -- a regex, a literal +/// search, a command source, a guard reading the repository -- are the part no +/// harness here reaches. What is proven is that no combination of their +/// answers reaches clean except the one where every kind the seam asks looked +/// and refused nothing, and that a rule which did not run -- bypassed, or out +/// of scope -- moves the verdict neither way. /// -/// One finding is the bound. `over_kinds` never looks inside what a kind -/// found: it passes the list on or stops at an error, so an empty list and a -/// non-empty one reach every branch it has, and five kinds at one each still -/// make a concatenation longer than any single list. +/// One answer per kind, and an `Unknown` after it where the kind could not +/// look, is the bound. `over_kinds` never looks inside what a kind answered: it +/// hands the list on or stops at an error, so an empty answer and a non-empty +/// one reach every branch it has, and five kinds at two each still make a list +/// longer than any one kind's. #[cfg(kani)] mod proofs { use super::{EVERY_JUDGED, Judged, Seam, load_for, over_kinds}; @@ -699,110 +890,239 @@ mod proofs { } } + /// What one kind answered, as far as the seam can tell them apart. A rule + /// that was bypassed, by its id or by `all`, and a rule stood down at this + /// destination are one letter here: `over_kinds`, `refusals` and `verdict` + /// read none of them, and the proofs say what the seam does with an answer + /// it does not read. An enum the checker picks from directly, and not a + /// byte reduced by arithmetic, because arithmetic on a symbolic value is a + /// circuit. + #[derive(Clone, Copy, PartialEq, Eq, kani::Arbitrary)] + enum Said { + Pass, + Refuse, + /// Bypassed, or out of scope: did not run, and is not a refusal. + NotRun, + /// The kind had no rule to ask. + Nothing, + } + #[derive(Clone, Copy, kani::Arbitrary)] - struct Answer { + struct Given { could_not_look: bool, - found_one: bool, + said: Said, } - /// What one kind answered, as a list of `finding`. The harnesses that only - /// ask whether a list is empty pass `()`, whose list never allocates: - /// `over_kinds` cannot look inside a finding, so its type changes the cost - /// of the proof and nothing about what it proves. - fn answer(answers: &[Answer; 5], kind: Judged, finding: V) -> Result> { - let given = answers[kind as usize]; - if given.could_not_look { - Err(Fatal::new(String::new())) - } else { - Ok(vec![finding; usize::from(given.found_one)]) + impl Given { + /// The byte `answer` records for this answer, or none where there is + /// no rule. The checker compares bytes; an `Outcome` compared on a + /// symbolic value is a case split per variant per entry, and that was + /// what put these proofs past a runner's memory. + fn code(self) -> Option { + match self.said { + Said::Pass => Some(0), + Said::Refuse => Some(1), + Said::NotRun => Some(2), + Said::Nothing => None, + } + } + } + + /// The byte for a kind that could not look. + const UNKNOWN: u8 = 3; + + /// One entry per kind asked, and one `UNKNOWN` after the kind that could + /// not look, which is the last one asked -- so the list never grows past + /// this and the checker never has to model it growing. + const LONGEST: usize = 6; + + /// What one kind answered, into a list the harness cannot look inside, + /// counting the refusals beside it. The list is of `()`, which never + /// allocates: `over_kinds` cannot look inside an entry, so the entry's + /// type changes the cost of the proof and nothing about what it proves. + /// A kind that could not look stops the seam, as every rule body does + /// through `asked`. + fn answer( + given: &[Given; 5], + kind: Judged, + ledger: &mut Vec<()>, + refused: &mut usize, + ) -> Result<()> { + let one = given[kind as usize]; + if let Some(code) = one.code() { + ledger.push(()); + if code == 1 { + *refused += 1; + } + } + if one.could_not_look { + ledger.push(()); + return Err(Fatal::new(String::new())); } + Ok(()) } - fn looked_and_found_nothing(answers: &[Answer; 5], seam: Seam) -> bool { + /// What the list would hold, written beside it into an array the checker + /// can read back cheaply: the byte `kind << 2 | code` per entry. The list + /// itself stays one of `()`, pushed once per entry so its length is the + /// array's; reading a heap vector back symbolically is what the two + /// harnesses below could not afford. + struct Shadow { + codes: [u8; LONGEST], + len: usize, + } + + impl Shadow { + const fn new() -> Self { + Self { + codes: [0; LONGEST], + len: 0, + } + } + + fn push(&mut self, ledger: &mut Vec<()>, code: u8) { + self.codes[self.len] = code; + self.len += 1; + ledger.push(()); + } + } + + /// The same answer as `answer`, with its bytes recorded in the shadow. + fn tagged( + given: &[Given; 5], + kind: Judged, + ledger: &mut Vec<()>, + shadow: &mut Shadow, + ) -> Result<()> { + let one = given[kind as usize]; + let tag = (kind as u8) << 2; + if let Some(code) = one.code() { + shadow.push(ledger, tag | code); + } + if one.could_not_look { + shadow.push(ledger, tag | UNKNOWN); + return Err(Fatal::new(String::new())); + } + Ok(()) + } + + fn looked_and_refused_nothing(given: &[Given; 5], seam: Seam) -> bool { EVERY_JUDGED.iter().all(|kind| { - let given = answers[*kind as usize]; - !seam.consults(*kind) || (!given.could_not_look && !given.found_one) + let one = given[*kind as usize]; + !seam.consults(*kind) || (!one.could_not_look && one.said != Said::Refuse) }) } - /// `clean <-> every consulted kind looked and found nothing`, through the + /// `clean <-> every consulted kind looked and refused nothing`, through the /// same `verdict` and `Exit::of` the callers settle on. Clean is a claim - /// that every consulted kind was evaluated, not the absence of a finding. + /// that every consulted kind was evaluated, not the absence of a finding + /// -- and a rule that did not run is neither a finding nor a failure to + /// look. #[kani::proof] #[kani::unwind(7)] - fn clean_means_every_consulted_kind_looked_and_found_nothing() { + fn clean_means_every_consulted_kind_looked_and_refused_nothing() { let seam = any_seam(); - let answers: [Answer; 5] = kani::any(); - let run = over_kinds(seam, |kind| answer(&answers, kind, ())) - .map(|verdicts| verdict(verdicts.len(), 0)); + let given: [Given; 5] = kani::any(); + let mut ledger = Vec::new(); + let mut refused = 0; + let run = over_kinds(seam, &mut ledger, |kind, ledger| { + answer(&given, kind, ledger, &mut refused) + }) + .map(|()| verdict(refused, 0)); let clean = Exit::of(&run) as i32 == Exit::Clean as i32; - assert!(clean == looked_and_found_nothing(&answers, seam)); + assert!(clean == looked_and_refused_nothing(&given, seam)); } - /// `a consulted kind could not look -> exit 2`, whatever the others found. - /// A finding does not outrank it here, unlike in `verdict`: the seam - /// stops at the first kind that could not look, so what it would have + /// `a consulted kind could not look -> exit 2`, whatever the others + /// answered. A refusal does not outrank it here, unlike in `verdict`: the + /// seam stops at the first kind that could not look, so what it would have /// reported is not known. #[kani::proof] #[kani::unwind(7)] fn a_consulted_kind_that_could_not_look_is_broken() { let seam = any_seam(); - let answers: [Answer; 5] = kani::any(); + let given: [Given; 5] = kani::any(); let broken_kind = EVERY_JUDGED .iter() - .any(|kind| seam.consults(*kind) && answers[*kind as usize].could_not_look); - let run = over_kinds(seam, |kind| answer(&answers, kind, ())) - .map(|verdicts| verdict(verdicts.len(), 0)); + .any(|kind| seam.consults(*kind) && given[*kind as usize].could_not_look); + let mut ledger = Vec::new(); + let mut refused = 0; + let run = over_kinds(seam, &mut ledger, |kind, ledger| { + answer(&given, kind, ledger, &mut refused) + }) + .map(|()| verdict(refused, 0)); assert!(broken_kind == (Exit::of(&run) as i32 == Exit::Broken as i32)); } /// Every consulted kind is asked exactly once when none could not look, /// no kind the seam does not consult is ever asked, and nothing a kind - /// found is dropped or duplicated on the way to the verdict. + /// answered is dropped or duplicated on the way to the list the record is + /// written from. #[kani::proof] #[kani::unwind(7)] - fn every_consulted_kind_is_asked_once_and_every_finding_is_kept() { + fn every_consulted_kind_is_asked_once_and_every_answer_is_kept() { let seam = any_seam(); - let answers: [Answer; 5] = kani::any(); + let given: [Given; 5] = kani::any(); let mut asked = [0_u8; 5]; - let run = over_kinds(seam, |kind| { + let mut ledger = Vec::new(); + let mut shadow = Shadow::new(); + let run = over_kinds(seam, &mut ledger, |kind, ledger| { asked[kind as usize] += 1; - answer(&answers, kind, kind as u8) + tagged(&given, kind, ledger, &mut shadow) }); + assert!(ledger.len() == shadow.len); for kind in EVERY_JUDGED { let slot = *kind as usize; if !seam.consults(*kind) { assert!(asked[slot] == 0); - } else if let Ok(verdicts) = &run { + } else if run.is_ok() { assert!(asked[slot] == 1); - let kept = verdicts - .iter() - .filter(|found| **found == *kind as u8) - .count(); - assert!(kept == usize::from(answers[slot].found_one)); + let mut kept = 0_u8; + let mut found = false; + for at in 0..LONGEST { + if at < shadow.len && shadow.codes[at] >> 2 == *kind as u8 { + kept += 1; + found |= Some(shadow.codes[at] & 3) == given[slot].code(); + } + } + match given[slot].code() { + Some(_) => assert!(kept == 1 && found), + None => assert!(kept == 0), + } } } } - /// The verdict is a function of the seam and the answers, and an answer - /// from a kind the seam does not consult cannot move it. + /// The list is a function of the seam and the answers: every entry in it + /// is tagged with a kind the seam consults, and says what that kind was + /// given to say -- or that it could not look, and then it is the last + /// entry. An answer from a kind the seam does not consult therefore has + /// nowhere to enter it. #[kani::proof] #[kani::unwind(7)] - fn the_verdict_is_a_function_of_the_consulted_answers() { + fn the_list_is_a_function_of_the_consulted_answers() { let seam = any_seam(); - let answers: [Answer; 5] = kani::any(); - let mut elsewhere: [Answer; 5] = kani::any(); - for kind in EVERY_JUDGED { - if seam.consults(*kind) { - elsewhere[*kind as usize] = answers[*kind as usize]; + let given: [Given; 5] = kani::any(); + let mut ledger = Vec::new(); + let mut shadow = Shadow::new(); + let run = over_kinds(seam, &mut ledger, |kind, ledger| { + tagged(&given, kind, ledger, &mut shadow) + }); + assert!(ledger.len() == shadow.len && shadow.len <= LONGEST); + for at in 0..LONGEST { + if at < shadow.len { + let entry = shadow.codes[at]; + let slot = usize::from(entry >> 2); + assert!(seam.consults(EVERY_JUDGED[slot])); + if entry & 3 == UNKNOWN { + assert!(given[slot].could_not_look); + } else { + assert!(given[slot].code() == Some(entry & 3)); + } } } - let first = over_kinds(seam, |kind| answer(&answers, kind, kind as u8)); - let second = over_kinds(seam, |kind| answer(&elsewhere, kind, kind as u8)); - match (first, second) { - (Ok(first), Ok(second)) => assert!(first == second), - (Err(_), Err(_)) => {} - _ => panic!("one input, two verdicts"), + if run.is_err() { + assert!(shadow.len > 0 && shadow.codes[shadow.len - 1] & 3 == UNKNOWN); } } @@ -895,4 +1215,187 @@ mod tests { let push = Rule::synthetic("push", named_builtin("prevent-public-push")); assert_eq!(Judged::of(&push), None); } + + /// Three text guards and one that judges no text, with a consultation + /// standing in front of a command. + const ANSWERING: &str = "\ +[rule.consult] +builtin = \"text-guards\" +command.before = [\"faux\"] + +[rule.no-merge-commit] +builtin = \"no-merge-commit\" +git.hooks = [\"pre-commit\"] + +[rule.prevent-ai-author] +builtin = \"prevent-ai-author\" +git.hooks = [\"commit-msg\"] + +[rule.prevent-unusual-unicode] +builtin = \"prevent-unusual-unicode\" +git.hooks = [\"commit-msg\"] + +[[shim]] +command = \"faux\" +match = [\"pr:create\"] +text_flags = [\"-b\"] +scope = \"always\" +"; + + fn answering() -> Policy { + config::load_text( + std::path::Path::new("."), + std::path::Path::new("ANSWERING"), + ANSWERING, + ) + .unwrap() + } + + fn published(text: &str) -> crate::guard::Published<'_> { + crate::guard::Published { + label: "body.md", + kind: "text", + text, + added: None, + headline: false, + } + } + + fn answer_of(ledger: &[Judgment], id: &str) -> Vec { + ledger + .iter() + .filter(|judgment| judgment.id == id) + .map(Answered::outcome) + .collect() + } + + /// Every text guard asked answers, one of them out of scope; a built-in + /// that judges no text is not asked and so is not in the list; and the + /// exit is read off the same list. + #[test] + fn each_text_guard_answers_and_a_guard_out_of_scope_says_so() { + let policy = answering(); + let mut ledger = Vec::new(); + crate::guard::over_text( + std::path::Path::new("."), + &policy, + None, + &published("Generated with Claude Code"), + &mut |rule| Ok(rule.id != "prevent-unusual-unicode"), + &mut ledger, + ) + .unwrap(); + assert_eq!(answer_of(&ledger, "prevent-ai-author"), [Outcome::Refuse]); + assert_eq!( + answer_of(&ledger, "prevent-unusual-unicode"), + [Outcome::OutOfScope] + ); + assert_eq!(answer_of(&ledger, "no-merge-commit"), Vec::new()); + assert_eq!(answer_of(&ledger, "consult"), Vec::new()); + assert_eq!(refusals(&ledger), 1); + assert!( + ledger + .iter() + .all(|judgment| judgment.subject.as_deref() == Some("text")) + ); + } + + /// A rule whose scope could not be told is `Unknown`, last, and the + /// error goes up: the record ends where the run did. + #[test] + fn a_scope_that_could_not_be_told_is_the_last_answer() { + let policy = answering(); + let mut ledger = Vec::new(); + let run = crate::guard::over_text( + std::path::Path::new("."), + &policy, + None, + &published("plain"), + &mut |rule| { + if rule.id == "prevent-ai-author" { + Err(Fatal::new("no destination")) + } else { + Ok(true) + } + }, + &mut ledger, + ); + assert!(run.is_err()); + let last = ledger.last().unwrap(); + assert_eq!(last.id, "prevent-ai-author"); + assert_eq!(last.outcome(), Outcome::Unknown); + } + + /// A consultation answers through the rules it ran, each under its own + /// id, and the report the caller prints for it is folded from those. + #[test] + fn a_consultation_answers_through_its_rules_and_is_reported_as_one() { + let policy = answering(); + let consult = policy + .rules + .iter() + .find(|rule| rule.id == "consult") + .unwrap(); + let mut ledger = Vec::new(); + crate::guard::text_refusal( + std::path::Path::new("."), + &policy, + consult, + None, + &published("Generated with Claude Code"), + &mut |_| Ok(true), + &mut ledger, + ) + .unwrap(); + assert_eq!(answer_of(&ledger, "consult"), Vec::new()); + assert_eq!(answer_of(&ledger, "prevent-ai-author"), [Outcome::Refuse]); + assert_eq!( + answer_of(&ledger, "prevent-unusual-unicode"), + [Outcome::Pass] + ); + let told = crate::guard::told(&policy, consult, &ledger).unwrap(); + assert_eq!(told.id, "consult"); + assert!(told.report.starts_with("prevent-ai-author: "), "{told:?}"); + } + + /// A guard asked about a body and again about a headline is one finding + /// in the report, where its first one was, between the findings of the + /// kinds around it. + #[test] + fn a_guard_refused_twice_is_reported_once_in_its_place() { + let rule = |id: &str| Judgment { + id: id.to_owned(), + builtin: None, + set: None, + subject: None, + answer: Answer::Pass, + }; + let refused = |id: &str, verdict: Verdict| Judgment { + answer: Answer::Refused(verdict), + ..rule(id) + }; + let guard = |report: &str| { + Verdict::Guard(crate::guard::Refusal { + id: String::from("g"), + report: report.to_owned(), + }) + }; + let ledger = [ + refused("lit", Verdict::Rule(Failure::new("lit", "m", "b"))), + refused("g", guard("in the body\n\nadvice\n")), + rule("other"), + refused("g", guard("in the title\n\nadvice\n")), + refused("prose", Verdict::Rule(Failure::new("prose", "m", "b"))), + ]; + let found = verdicts(&ledger); + let names: Vec<&str> = found + .iter() + .map(|verdict| match verdict { + Verdict::Rule(failure) => failure.label.as_str(), + Verdict::Guard(refusal) => refusal.id.as_str(), + }) + .collect(); + assert_eq!(names, ["lit", "g", "prose"]); + assert_eq!(refusals(&ledger), 4); + } } diff --git a/tests/guard_cli.rs b/tests/guard_cli.rs index 998b8d7..1d9d7cc 100644 --- a/tests/guard_cli.rs +++ b/tests/guard_cli.rs @@ -388,9 +388,13 @@ fn a_rule_that_could_not_look_is_an_error_and_the_rest_are_absent() { 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 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()] { + // A path under a file, and a path under a directory that does not exist: + // neither can be created. An existing directory is not this case -- it is + // the directory form, and gets a file of its own inside it. + for junit in [ + root.join("msg.txt/commit-msg.xml"), + root.join("missing/commit-msg.xml"), + ] { let output = guard_with_junit(&root, junit.as_os_str(), None); assert_eq!(code(&output), 2, "{}", stderr(&output)); assert!( @@ -527,27 +531,35 @@ fn a_run_that_stops_before_its_first_rule_leaves_no_file() { 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. +/// A directory is one file per run inside it, named for the suite, so two +/// runs of one stage leave two documents and neither replaces the other. #[test] -fn the_text_seam_refuses_to_run_with_the_variable_set() { - let root = repository(AI_AUTHOR); - 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("does not write evidence yet"), - "{}", - stderr(&output) - ); - assert!(!junit.exists()); +fn a_directory_gets_one_file_per_run() { + let root = repository(AI_AUTHOR_AND_ASCII_SUBJECT); + write(&root, "msg.txt", "x\n\nGenerated with Claude Code\n"); + let evidence = root.join("evidence"); + std::fs::create_dir_all(&evidence).unwrap(); + for _ in 0..2 { + let output = guard_with_junit(&root, evidence.as_os_str(), None); + assert_eq!(code(&output), 1, "{}", stderr(&output)); + } + let mut written: Vec = std::fs::read_dir(&evidence) + .unwrap() + .map(|entry| entry.unwrap().path()) + .collect(); + written.sort(); + assert_eq!(written.len(), 2, "{written:?}"); + for path in &written { + let name = path.file_name().unwrap().to_str().unwrap(); + assert!( + name.starts_with("uphold-guard-commit-msg-") + && path.extension().is_some_and(|extension| extension == "xml"), + "{name}" + ); + let suite = the_suite(path); + assert_eq!(suite.name.as_str(), "uphold guard commit-msg"); + assert_eq!((suite.tests, suite.failures), (2, 1)); + } } /// The variable set on a real `git push` reaches a real pre-push hook, which diff --git a/tests/hook_cli.rs b/tests/hook_cli.rs index aaad268..be6cba2 100644 --- a/tests/hook_cli.rs +++ b/tests/hook_cli.rs @@ -926,3 +926,156 @@ fn a_message_key_is_prose_so_a_lookalike_passes_and_an_invisible_character_does_ assert!(reason.contains("prevent-unusual-unicode"), "{reason}"); assert!(reason.contains("U+200B"), "{reason}"); } + +// -- UPHOLD_JUNIT: what a call decided, as one JUnit document --------------- + +/// [`hook`], with `args` after `hook` and `UPHOLD_JUNIT` set to `junit` when +/// given. +fn hook_recorded(root: &Path, args: &[&str], stdin: &str, junit: Option<&Path>) -> Output { + let mut command = Command::new(env!("CARGO_BIN_EXE_uphold")); + command + .arg("hook") + .args(args) + .current_dir(root) + .env_remove("UPHOLD_ALLOW") + .env_remove("UPHOLD_JUNIT") + .env("HOME", "/home/example-user") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + if let Some(junit) = junit { + command.env("UPHOLD_JUNIT", junit); + } + let mut child = command.spawn().unwrap(); + if let Err(error) = child.stdin.as_mut().unwrap().write_all(stdin.as_bytes()) { + assert_eq!(error.kind(), std::io::ErrorKind::BrokenPipe, "{error}"); + } + child.wait_with_output().unwrap() +} + +/// Every call writes its own document into the directory, refused or not: +/// a seam that wrote only on a refusal would make an absent file mean "passed +/// or never ran". Each rule the call consulted is a testcase under the +/// `uphold hook ` suite, its subject is the tool's name, and the body +/// it judged is in no file. +#[test] +fn every_call_writes_one_document_naming_each_rule_and_never_the_body() { + let root = workspace( + "hook-junit", + Some(&format!("{POLICY}\n{PROSE_POLICY}{SHIMS}")), + ); + let evidence = root.join("evidence"); + std::fs::create_dir_all(&evidence).unwrap(); + let tool = "mcp__github__create_pull_request"; + + let refused = hook_recorded( + &root, + &["claude-code"], + &event(tool, "Arguably fine.\n\nGenerated with Claude Code"), + Some(&evidence), + ); + assert_eq!(code(&refused), 0, "{}", stderr(&refused)); + assert!(stdout(&refused).contains("deny"), "{}", stdout(&refused)); + let clean = hook_recorded( + &root, + &["claude-code"], + &event(tool, "An ordinary change."), + Some(&evidence), + ); + assert_eq!(code(&clean), 0, "{}", stderr(&clean)); + assert!(stdout(&clean).is_empty(), "{}", stdout(&clean)); + + let files = support::junit::files_in(&evidence); + assert_eq!(files.len(), 2, "{files:?}"); + for path in &files { + let name = path.file_name().unwrap().to_str().unwrap(); + assert!(name.starts_with("uphold-hook-claude-code-"), "{name}"); + } + let (first, text) = support::junit::suite(files.first().unwrap()); + assert_eq!(first.name.as_str(), "uphold hook claude-code"); + assert_eq!(support::junit::package(&first), Some("uphold")); + let ai = support::junit::case(&first, "prevent-ai-author"); + assert_eq!( + support::junit::state(ai), + ("failure", Some("prevent-ai-author refused")) + ); + assert_eq!( + support::junit::property(ai, "builtin"), + Some("prevent-ai-author") + ); + let hedge = support::junit::case(&first, "no-empty-hedge-here"); + assert_eq!( + support::junit::state(hedge), + ("failure", Some("no-empty-hedge-here refused")) + ); + // A prose rule names no built-in. + assert_eq!(support::junit::property(hedge, "builtin"), None); + let identity = support::junit::case(&first, "no-running-os-identity-metadata"); + assert_eq!(support::junit::state(identity), ("pass", None)); + for case in &first.test_cases { + assert_eq!(support::junit::properties(case, "subject"), [tool]); + assert_eq!(support::junit::property(case, "seam"), Some("hook")); + } + assert!(!text.contains("Arguably"), "{text}"); + assert!(!text.contains("Generated"), "{text}"); + + let (second, _) = support::junit::suite(files.last().unwrap()); + assert_eq!((second.tests, second.failures), (3, 0), "{second:?}"); +} + +/// The hook runs once per publishing call, so one file would keep only the +/// last; a value naming anything but an existing directory is exit 2, named, +/// before the call is judged. +#[test] +fn a_value_that_is_not_a_directory_is_refused_naming_it() { + let root = workspace("hook-junit-file", Some(POLICY)); + let file = root.join("hook.xml"); + let output = hook_recorded( + &root, + &["claude-code"], + &event("publish", "Generated with Claude Code"), + Some(&file), + ); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains(&format!("UPHOLD_JUNIT={}", file.display())), + "{}", + stderr(&output) + ); + assert!(stdout(&output).is_empty(), "{}", stdout(&output)); + assert!(!file.exists()); + + let flagged = hook_recorded( + &root, + &["claude-code", "--junit", "hook.xml"], + &event("publish", "Generated with Claude Code"), + None, + ); + assert_eq!(code(&flagged), 2, "{}", stderr(&flagged)); + assert!( + stderr(&flagged).contains("--junit="), + "{}", + stderr(&flagged) + ); + assert!(!file.exists()); +} + +/// `--junit DIR` is the spelling for the harness's settings file, before or +/// after the harness name, and a relative value is the directory the hook +/// runs in. +#[test] +fn the_flag_names_the_directory_on_either_side_of_the_harness() { + let root = workspace("hook-junit-flag", Some(POLICY)); + std::fs::create_dir_all(root.join("evidence")).unwrap(); + for args in [ + ["--junit", "evidence", "codex"], + ["codex", "--junit", "evidence"], + ] { + let output = hook_recorded(&root, &args, &event("publish", "plain"), None); + assert_eq!(code(&output), 0, "{args:?}: {}", stderr(&output)); + } + let files = support::junit::files_in(&root.join("evidence")); + assert_eq!(files.len(), 2, "{files:?}"); + let (suite, _) = support::junit::suite(files.first().unwrap()); + assert_eq!(suite.name.as_str(), "uphold hook codex"); +} diff --git a/tests/shim_cli.rs b/tests/shim_cli.rs index 52e33c4..9e38cd7 100644 --- a/tests/shim_cli.rs +++ b/tests/shim_cli.rs @@ -90,6 +90,7 @@ fn shim_with(root: &Path, args: &[&str], variables: &[(&str, &str)]) -> Output { .current_dir(root) .env("PATH", path) .env_remove("UPHOLD_ALLOW") + .env_remove("UPHOLD_JUNIT") .env_remove("UPHOLD_SHIM_INNER") .env_remove("GH_REPO") .env_remove("GH_HOST") @@ -8202,3 +8203,337 @@ fn glab_work_items_group_environment_requires_a_project_override() { assert!(stdout(&output).contains("glab ran:")); } } + +// ── UPHOLD_JUNIT: what one invocation decided, as one JUnit document ── + +/// [`shim_with`], with `UPHOLD_JUNIT` naming `junit`. +fn shim_recorded(root: &Path, args: &[&str], junit: &Path, variables: &[(&str, &str)]) -> Output { + let mut all = vec![("UPHOLD_JUNIT", junit.to_str().unwrap())]; + all.extend_from_slice(variables); + shim_with(root, args, &all) +} + +/// The evidence directory of a fixture, made. +fn evidence(root: &Path) -> PathBuf { + let directory = root.join("evidence"); + std::fs::create_dir_all(&directory).unwrap(); + directory +} + +/// The shim runs many times in one act, so a directory gets one file per +/// invocation -- two `faux` calls leave two documents, and neither replaced +/// the other. Each is the `uphold shim ` suite, one testcase per rule +/// with the kind of subject it read, and the answer the stderr report gave. +#[test] +fn each_invocation_writes_its_own_document_and_two_leave_two() { + let root = workspace(BUILTIN_CHECKER); + let directory = evidence(&root); + let refused = shim_recorded( + &root, + &[ + "faux", + "pr", + "create", + "-b", + "this fixes acme-private/thing", + ], + &directory, + &[], + ); + assert_eq!(code(&refused), 1, "{}", stderr(&refused)); + let clean = shim_recorded( + &root, + &["faux", "pr", "create", "-b", "an ordinary change"], + &directory, + &[], + ); + assert_eq!(code(&clean), 0, "{}", stderr(&clean)); + assert!(stdout(&clean).contains("faux ran"), "{}", stdout(&clean)); + + let files = support::junit::files_in(&directory); + assert_eq!(files.len(), 2, "{files:?}"); + let mut states = Vec::new(); + for path in &files { + let name = path.file_name().unwrap().to_str().unwrap(); + assert!( + name.starts_with("uphold-shim-faux-") + && path.extension().is_some_and(|extension| extension == "xml"), + "{name}" + ); + let (suite, text) = support::junit::suite(path); + assert_eq!(suite.name.as_str(), "uphold shim faux"); + assert_eq!(support::junit::package(&suite), Some("uphold")); + assert_eq!(suite.tests, 1, "{suite:?}"); + let case = support::junit::case(&suite, "no-private-repo-names"); + assert_eq!( + support::junit::property(case, "builtin"), + Some("no-private-repo-names") + ); + assert_eq!(support::junit::properties(case, "subject"), ["text"]); + assert_eq!(support::junit::property(case, "seam"), Some("shim")); + assert!(!text.contains("acme-private"), "{text}"); + assert!(!text.contains("ordinary"), "{text}"); + states.push(support::junit::state(case).0); + } + states.sort_unstable(); + assert_eq!(states, ["failure", "pass"]); +} + +/// A program the shim consults is the shim's own dispatch, and its answer is +/// a testcase under the rule that named it. The program here is `uphold guard +/// --text`, which would write a document of its own about the same body if +/// it inherited the variable -- one refusal, counted twice. +#[test] +fn a_consulted_checker_writes_no_document_of_its_own() { + let root = workspace(POLICY); + let directory = evidence(&root); + let output = shim_recorded( + &root, + &["faux", "pr", "create", "-t", "Generated with Claude Code"], + &directory, + &[], + ); + assert_eq!(code(&output), 1, "{}", stderr(&output)); + let files = support::junit::files_in(&directory); + assert_eq!(files.len(), 1, "{files:?}"); + let (suite, _) = support::junit::suite(files.first().unwrap()); + assert_eq!(suite.name.as_str(), "uphold shim faux"); + let case = support::junit::case(&suite, "no-published-markers"); + assert_eq!( + support::junit::state(case), + ("failure", Some("no-published-markers refused")) + ); + assert_eq!(support::junit::property(case, "builtin"), None); +} + +/// An exit 2 no one rule owns -- here, a body nothing stands in front of -- +/// still leaves a document, and the document does not read as a pass: it +/// carries an error testcase for the run itself. +#[test] +fn an_exit_two_no_rule_owns_is_an_error_in_the_document() { + let root = workspace(NARROWED_CHECKER); + let directory = evidence(&root); + let output = shim_recorded( + &root, + &["faux", "issue", "create", "-t", "An ordinary title"], + &directory, + &[], + ); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + let files = support::junit::files_in(&directory); + assert_eq!(files.len(), 1, "{files:?}"); + let (suite, _) = support::junit::suite(files.first().unwrap()); + assert_eq!((suite.tests, suite.errors), (1, 1), "{suite:?}"); + assert_eq!( + support::junit::state(support::junit::case(&suite, "uphold")), + ("error", Some("uphold could not look")) + ); +} + +/// A file value at the shim would be replaced by every `gh` call in one act, +/// so it is exit 2 naming the value -- refused before the wrapped command +/// runs, as every other exit 2 here is, and nothing is written. +#[test] +fn a_file_value_is_exit_two_and_the_command_never_runs() { + let root = workspace(BUILTIN_CHECKER); + let file = root.join("shim.xml"); + let output = shim_recorded( + &root, + &["faux", "pr", "create", "-b", "an ordinary change"], + &file, + &[], + ); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains(&format!("UPHOLD_JUNIT={}", file.display())), + "{}", + stderr(&output) + ); + assert!(!stdout(&output).contains("faux ran"), "{}", stdout(&output)); + assert!(!file.exists()); +} + +/// `UPHOLD_ALLOW=all` runs the command before a rule is read, so there is no +/// rule to give a testcase. The suite still says what happened: no tests, and +/// a suite property `allowed=all`, so "ran unchecked" is a document rather +/// than an absence. +#[test] +fn allow_all_writes_a_suite_of_no_rules_that_says_so() { + let root = workspace(BUILTIN_CHECKER); + let directory = evidence(&root); + let output = shim_recorded( + &root, + &[ + "faux", + "pr", + "create", + "-b", + "this fixes acme-private/thing", + ], + &directory, + &[("UPHOLD_ALLOW", "all")], + ); + assert_eq!(code(&output), 0, "{}", stderr(&output)); + assert!(stdout(&output).contains("faux ran"), "{}", stdout(&output)); + let files = support::junit::files_in(&directory); + assert_eq!(files.len(), 1, "{files:?}"); + let (suite, _) = support::junit::suite(files.first().unwrap()); + assert_eq!(suite.name.as_str(), "uphold shim faux"); + assert_eq!(suite.tests, 0, "{suite:?}"); + let allowed: Vec<(&str, &str)> = suite + .properties + .iter() + .map(|property| (property.name.as_str(), property.value.as_str())) + .collect(); + assert_eq!(allowed, [("allowed", "all")]); +} + +/// A rule whose own scope did not hold here neither ran nor was bypassed: it +/// is a skipped testcase saying the destination stood it down, so a reader +/// asking whether it ran on this body is answered by the file. +#[test] +fn a_rule_stood_down_at_this_destination_is_skipped_with_its_scope() { + let root = workspace(NARROWER_RULE_POLICY); + std::fs::remove_file(root.join("bin/uphold")).unwrap(); + let directory = evidence(&root); + let output = shim_recorded( + &root, + &["faux", "pr", "create", "-t", "Generated with Claude Code"], + &directory, + &[], + ); + assert_eq!(code(&output), 0, "{}", stderr(&output)); + let files = support::junit::files_in(&directory); + assert_eq!(files.len(), 1, "{files:?}"); + let (suite, _) = support::junit::suite(files.first().unwrap()); + assert_eq!((suite.tests, suite.skipped), (1, 1), "{suite:?}"); + let case = support::junit::case(&suite, "no-published-markers"); + assert_eq!( + support::junit::state(case), + ("skipped", Some("out of scope at this destination")) + ); + assert_eq!(support::junit::property(case, "scope"), Some("destination")); + assert_eq!(support::junit::property(case, "allowed"), None); +} + +/// A consultation is dispatch, not judgment: the rules it ran each get a +/// testcase, and it gets none, since both would count one refusal twice. +#[test] +fn a_consultation_records_the_rules_it_ran_and_not_itself() { + let root = workspace(WIDER_RULE_POLICY); + std::fs::remove_file(root.join("bin/uphold")).unwrap(); + let directory = evidence(&root); + let output = shim_recorded( + &root, + &["faux", "pr", "create", "-t", "Generated with Claude Code"], + &directory, + &[], + ); + assert_eq!(code(&output), 1, "{}", stderr(&output)); + let files = support::junit::files_in(&directory); + let (suite, _) = support::junit::suite(files.first().unwrap()); + assert_eq!((suite.tests, suite.failures), (1, 1), "{suite:?}"); + assert_eq!( + support::junit::state(support::junit::case(&suite, "prevent-ai-author")), + ("failure", Some("prevent-ai-author refused")) + ); +} + +/// A pull-request body refused on its way to a private repository. +const PRIVATE_DESTINATION_POLICY: &str = r#" +[[shim]] +command = "faux" +match = ["pr:create"] +title_flags = ["-t", "--title"] +text_flags = ["-b", "--body"] +target_flags = ["-R", "--repo"] +target = "forge-repo" +scope = "always" + +[rule.prevent-ai-author] +builtin = "prevent-ai-author" +command.before = ["faux"] + +[rule.no-private-repo-names] +builtin = "no-private-repo-names" +visibility = "public" +command.before = ["faux"] + +[rule.no-running-os-identity-metadata] +message = "Do not publish host identity." +forbidden_literals = "running-os-identity" +files.include = ["evidence"] +files.word = true +"#; + +/// The repository a shim invocation publishes to is what a private-names rule +/// protects, so the record of that invocation names none of it: no owner, no +/// repository, no URL, no title, no body -- only rule ids and the kinds of +/// subject they read. And it is held to the rule the fleet holds a published +/// file to: the identity scan reads this machine's user name, home path and +/// hostname and finds none of them in it. +#[test] +fn a_refused_body_to_a_private_destination_leaves_no_trace_of_it_in_the_file() { + let root = checkout_under(PRIVATE_DESTINATION_POLICY); + let directory = evidence(&root); + let output = shim_recorded( + &root, + &[ + "faux", + "pr", + "create", + "--repo", + "example-org/private-repo", + "-t", + "Fix the parser", + "-b", + "Refs example-org/private-repo#1\n\nGenerated with Claude Code", + ], + &directory, + &[], + ); + assert_eq!(code(&output), 1, "{}", stderr(&output)); + assert!( + stderr(&output).contains("attribution"), + "{}", + stderr(&output) + ); + + let files = support::junit::files_in(&directory); + assert_eq!(files.len(), 1, "{files:?}"); + let (suite, text) = support::junit::suite(files.first().unwrap()); + let ai = support::junit::case(&suite, "prevent-ai-author"); + assert_eq!(support::junit::state(ai).0, "failure"); + assert_eq!( + support::junit::properties(ai, "subject"), + ["title", "text"], + "{suite:?}" + ); + let names = support::junit::case(&suite, "no-private-repo-names"); + assert_eq!(support::junit::state(names), ("pass", None)); + for absent in [ + // The repository half alone is a substring of the rule's own id, so + // the owner and the whole name are what is looked for. + "example-org", + "example-org/private-repo", + "github.com", + "https://", + "Fix the parser", + "Refs", + "Generated", + "main", + "faux pr", + ] { + assert!(!text.contains(absent), "{absent:?} in {text}"); + } + + support::git(&root, &["add", "-A"]); + let scan = Command::new(env!("CARGO_BIN_EXE_uphold")) + .arg("scan") + .current_dir(&root) + .env_remove("UPHOLD_ALLOW") + .env_remove("UPHOLD_JUNIT") + .output() + .unwrap(); + assert_eq!(code(&scan), 0, "{}", stderr(&scan)); +} diff --git a/tests/support/junit.rs b/tests/support/junit.rs new file mode 100644 index 0000000..8e088e0 --- /dev/null +++ b/tests/support/junit.rs @@ -0,0 +1,86 @@ +//! Reading back the `JUnit` documents a seam writes, with the library that +//! wrote them. +//! +//! Shared because three seams write the same document -- the shim, the harness +//! hook and `guard --text` -- and each suite asks the same questions of it: +//! which file, which suite, which testcase, and what state it is in. + +#![expect( + clippy::expect_used, + reason = "a test's reader reports by panicking; there is no caller to hand a Result to" +)] + +use std::path::{Path, PathBuf}; + +use quick_junit::{NonSuccessKind, Report, TestCase, TestCaseStatus, TestSuite, XmlString}; + +/// The file read back, holding exactly one suite, and the text it was read +/// from -- for the assertions about what is NOT in it. +pub fn suite(path: &Path) -> (TestSuite, String) { + let text = std::fs::read_to_string(path).expect("the document a run wrote"); + let mut report = Report::deserialize_from_str(&text).expect("a JUnit document"); + assert_eq!(report.test_suites.len(), 1, "{text}"); + (report.test_suites.remove(0), text) +} + +/// Every file in `directory`, in name order, which is the order they were +/// written in. +pub fn files_in(directory: &Path) -> Vec { + let mut files: Vec = std::fs::read_dir(directory) + .expect("the evidence directory") + .map(|entry| entry.expect("a directory entry").path()) + .collect(); + files.sort(); + files +} + +/// The one testcase named `id`. +pub fn case<'s>(suite: &'s TestSuite, id: &str) -> &'s TestCase { + let mut found = suite + .test_cases + .iter() + .filter(|case| case.name.as_str() == id); + let case = found.next(); + assert!(case.is_some(), "no {id} in {suite:?}"); + assert!(found.next().is_none(), "{id} twice in {suite:?}"); + case.expect("asserted above") +} + +/// Every value of the property `name`, in order. +pub fn properties<'c>(case: &'c TestCase, name: &str) -> Vec<&'c str> { + case.properties + .iter() + .filter(|property| property.name.as_str() == name) + .map(|property| property.value.as_str()) + .collect() +} + +/// The first value of the property `name`. +pub fn property<'c>(case: &'c TestCase, name: &str) -> Option<&'c str> { + properties(case, name).first().copied() +} + +/// Which of the testcase states this is, and the message it carries. +pub 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)) + } + } +} + +/// The suite's own `package` attribute. +pub fn package(suite: &TestSuite) -> Option<&str> { + suite.extra.get("package").map(XmlString::as_str) +} diff --git a/tests/support/mod.rs b/tests/support/mod.rs index ed3740a..2f75a48 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -34,6 +34,7 @@ reason = "a fixture reports by panicking; there is no caller to hand a Result to" )] +pub mod junit; pub mod syntax; use std::path::{Path, PathBuf}; diff --git a/tests/text_cli.rs b/tests/text_cli.rs index 9c7a0a4..aa412e8 100644 --- a/tests/text_cli.rs +++ b/tests/text_cli.rs @@ -42,6 +42,7 @@ fn scan_text(root: &Path, stdin: &[u8], home: &str) -> Output { .args(["scan", "--text", "-"]) .current_dir(root) .env_remove("UPHOLD_ALLOW") + .env_remove("UPHOLD_JUNIT") .env("HOME", home) .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -271,6 +272,7 @@ fn guard_text(root: &Path, stdin: &[u8], home: &str, allow: Option<&str>) -> Out command .args(["guard", "--text", "-"]) .current_dir(root) + .env_remove("UPHOLD_JUNIT") .env("HOME", home) .env( "PATH", @@ -768,3 +770,111 @@ fn an_invisible_on_the_allow_list_is_refused_at_load() { assert!(report.contains("U+200B"), "{report}"); assert!(report.contains("draws nothing"), "{report}"); } + +// ── what `guard --text` decided, as one JUnit document ─────────────── + +/// [`PROSE_POLICY`] with a text guard beside the prose rule, and a built-in +/// that judges no text, which is not asked and so not recorded. +const RECORDED_POLICY: &str = r#" +[rule.prevent-ai-author] +builtin = "prevent-ai-author" +git.hooks = ["commit-msg"] + +[rule.no-merge-commit] +builtin = "no-merge-commit" +git.hooks = ["pre-commit"] +"#; + +/// `uphold guard --text -` with `extra` after it and `UPHOLD_JUNIT` set to +/// `junit` when given. +fn guard_text_recorded(root: &Path, extra: &[&str], stdin: &[u8], junit: Option<&Path>) -> Output { + let mut command = Command::new(env!("CARGO_BIN_EXE_uphold")); + command + .args(["guard", "--text", "-"]) + .args(extra) + .current_dir(root) + .env_remove("UPHOLD_ALLOW") + .env_remove("UPHOLD_JUNIT") + .env("HOME", EXAMPLE_HOME) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + if let Some(junit) = junit { + command.env("UPHOLD_JUNIT", junit); + } + let mut child = command.spawn().unwrap(); + if let Err(error) = child.stdin.as_mut().unwrap().write_all(stdin) { + assert_eq!(error.kind(), std::io::ErrorKind::BrokenPipe, "{error}"); + } + child.wait_with_output().unwrap() +} + +/// `--text` with `--junit FILE` writes the document a stage writes: one +/// testcase per rule it consulted, the report on stderr unchanged, and none of +/// the text it judged in the file. It used to refuse to run, because it could +/// name what refused and not what passed; it can name both now. +#[test] +fn guard_text_with_a_file_writes_one_testcase_per_rule_consulted() { + let root = workspace(&format!("{RECORDED_POLICY}{PROSE_POLICY}")); + let body = b"Arguably the count is right.\n\nGenerated with Claude Code\n"; + let output = guard_text_recorded(&root, &["--junit", "text.xml"], body, None); + assert_eq!(code(&output), 1, "{}", stderr(&output)); + assert!( + stderr(&output).contains("guard refused: prevent-ai-author"), + "{}", + stderr(&output) + ); + assert!( + stderr(&output).contains("guard refused: no-empty-hedge"), + "{}", + stderr(&output) + ); + + let (suite, text) = support::junit::suite(&root.join("text.xml")); + assert_eq!(suite.name.as_str(), "uphold guard --text"); + assert_eq!(support::junit::package(&suite), Some("uphold")); + assert_eq!((suite.tests, suite.failures), (2, 2), "{suite:?}"); + let ai = support::junit::case(&suite, "prevent-ai-author"); + assert_eq!( + support::junit::state(ai), + ("failure", Some("prevent-ai-author refused")) + ); + assert_eq!( + support::junit::property(ai, "builtin"), + Some("prevent-ai-author") + ); + let hedge = support::junit::case(&suite, "no-empty-hedge"); + assert_eq!(support::junit::property(hedge, "builtin"), None); + for case in &suite.test_cases { + assert_eq!(support::junit::properties(case, "subject"), ["text"]); + assert_eq!(support::junit::property(case, "seam"), Some("text")); + assert_eq!(support::junit::property(case, "stage"), None); + } + assert!(!text.contains("Arguably"), "{text}"); + assert!(!text.contains("Generated"), "{text}"); + assert!(!text.contains("no-merge-commit"), "{text}"); + + // A file is replaced, as a stage's is: a clean run leaves its own suite. + let again = guard_text_recorded(&root, &["--junit", "text.xml"], b"Plain.\n", None); + assert_eq!(code(&again), 0, "{}", stderr(&again)); + let (replaced, _) = support::junit::suite(&root.join("text.xml")); + assert_eq!((replaced.tests, replaced.failures), (2, 0), "{replaced:?}"); +} + +/// The variable naming a directory is one file per run inside it. +#[test] +fn guard_text_with_a_directory_writes_a_file_per_run() { + let root = workspace(RECORDED_POLICY); + let evidence = root.join("evidence"); + std::fs::create_dir_all(&evidence).unwrap(); + for _ in 0..2 { + let output = guard_text_recorded(&root, &[], b"Plain.\n", Some(&evidence)); + assert_eq!(code(&output), 0, "{}", stderr(&output)); + } + let files = support::junit::files_in(&evidence); + assert_eq!(files.len(), 2, "{files:?}"); + for path in &files { + let name = path.file_name().unwrap().to_str().unwrap(); + assert!(name.starts_with("uphold-guard-text-"), "{name}"); + } +}