diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 1e6aa7f..0896a98 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -169,6 +169,14 @@ component, here are the controls it implements. Its ecosystem Compliance-to-Policy) already turns that into policy for several engines and normalizes their results back, which a private format would forgo. +Rust evaluates reconciliation once into a report; the default human renderer +and the versioned `uphold check --json` renderer consume that same result. +Python review and OSCAL read the JSON contract, including explicit component +identities, rather than interpreting a human evidence line. A malformed report, +unknown schema version, unavailable evidence, or unexpected process status is +could-not-look. Review can still use the partial evidence from an evaluated +refusal; an export requires the whole reconciliation to hold. + The export reconciles first and emits only what held: a component definition is an assertion to an outside reader, and exporting an unreconciled one would publish a claim this tool had just failed to confirm. diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 4acc372..419f750 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -3780,6 +3780,56 @@ PATH. Hook trust in Codex and user-level settings are outside this declaration check. A settings file that cannot be parsed in its JSON or TOML format is exit `2`, including when another settings file declares a valid hook. +`uphold check --json` renders the same evaluated reconciliation as the default +human report. It emits one JSON object on stdout, including partial evidence +when the result is refused or unavailable: + +```json +{ + "schema_version": 1, + "verdict": "clean", + "claims": [ + { + "principle": "explicit-unknown", + "rule": "catalog-tests", + "status": "reconciled", + "suppliers": [{"component": "local", "seam": "a hook installed here"}] + } + ], + "violations": [], + "unavailable": [], + "notes": [] +} +``` + +`schema_version` is an integer. `verdict` is `clean`, `violations`, or +`could_not_look`, corresponding to process status `0`, `1`, or `2`. `claims` +contains every evaluated declaration entry, in declaration order. Its `status` +is `reconciled`, `refused`, or `unavailable`; only reconciled claims have +suppliers. Each supplier names its `component` (`local` or `uphold`) explicitly +and carries the supplying `seam` as a description. Consumers route by component +and treat seam descriptions, notes and diagnostics as text for a person. +`violations`, `unavailable` and `notes` are arrays of diagnostic strings. +Additional fields may be added within a schema version. + +A missing or invalid policy or declaration, discovery failure, or other error +before evaluation produces no JSON: stdout is empty, the diagnostic is on +stderr, and the process exits `2`. `--coverage` and `--json` are separate modes +and cannot be combined. Exit precedence remains the same as the human mode: +a claim whose missing rule may be in unread configuration exits `2`; otherwise +a known violation exits `1` even if other configuration is unreadable. The +`unavailable` array records unread evidence regardless of the exit code. + +A range-only hook configured exclusively at stages without a push range does +not supply a claim. A valid declaration of the same id at another stage or +through another runner still supplies it independently. + +The Python review and OSCAL consumers read this JSON contract. They reject +unknown schema versions, malformed or inconsistent reports, unexpected process +statuses, and any unavailable evidence. Review accepts an evaluated refusal +and retains the claims that reconciled. OSCAL requires a clean report. Neither +consumer parses the human report's wording. + `--coverage` counts the direction the reconcile cannot — a rule firing under no claim is invisible to a reconcile. It reports and does not refuse: `0`, or `2` where a tier's configuration could not be read, with a count of `?` rather than diff --git a/src/check.rs b/src/check.rs index aea8fdf..b350487 100644 --- a/src/check.rs +++ b/src/check.rs @@ -28,7 +28,7 @@ use std::collections::{BTreeMap, BTreeSet}; use std::path::Path; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use crate::catalog; use crate::config::Policy; @@ -340,6 +340,18 @@ fn pinned_ids(root: &Path) -> Result, Vec)>> { let mut every = BTreeSet::new(); for entry in repos { for hook in entry.hooks { + // A range-only hook at no push stage cannot supply a claim. Filter + // this declaration only: another declaration may install the same + // id at a valid stage, or another runner may supply it independently. + if RANGE_IDS.contains(&hook.id.as_str()) + && hook.stages.as_ref().is_some_and(|stages| { + !stages + .iter() + .any(|stage| RANGE_STAGES.contains(&stage.as_str())) + }) + { + continue; + } every.insert(hook.id); } } @@ -711,8 +723,66 @@ pub(crate) struct Supply { pub unestablished: Vec, } -/// `uphold check`, and `uphold check --coverage`. -pub(crate) fn run(root: &Path, policy: &Policy, coverage: bool) -> Result { +/// The versioned machine contract, independent of the human report's wording. +#[derive(Debug, Serialize)] +pub(crate) struct Report { + schema_version: u8, + pub(crate) verdict: Verdict, + pub(crate) claims: Vec, + violations: Vec, + unavailable: Vec, + notes: Vec, + #[serde(skip)] + misplaced_count: usize, + #[serde(skip)] + fatal: Option, +} + +/// The machine report names the same outcome that determines the process status. +#[derive(Debug, Clone, Copy, Serialize)] +#[serde(rename_all = "snake_case")] +pub(crate) enum Verdict { + Clean, + Violations, + CouldNotLook, +} + +impl Verdict { + const fn exit(self) -> Exit { + match self { + Self::Clean => Exit::Clean, + Self::Violations => Exit::Violations, + Self::CouldNotLook => Exit::Broken, + } + } +} + +/// A declaration entry retains its individual result and supplying seams, so a +/// refused or incomplete report can still identify the claims that held. +#[derive(Debug, Serialize)] +pub(crate) struct EvaluatedClaim { + pub(crate) principle: String, + pub(crate) rule: String, + status: ClaimStatus, + suppliers: Vec, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +enum ClaimStatus { + Reconciled, + Refused, + Unavailable, +} + +#[derive(Debug, Serialize)] +struct Supplier { + component: &'static str, + seam: String, +} + +/// `uphold check`, its JSON form, and the separate coverage report. +pub(crate) fn run(root: &Path, policy: &Policy, coverage: bool, json: bool) -> Result { let path = root.join(DECLARATION); if !path.is_file() { return Err(Fatal::new(format!( @@ -724,125 +794,222 @@ pub(crate) fn run(root: &Path, policy: &Policy, coverage: bool) -> Result let text = std::fs::read_to_string(&path).map_err(|error| Fatal::at(&path, error))?; let declaration: Declaration = toml::from_str(&text).map_err(|error| Fatal::at(&path, error))?; - let installed = installed(root)?; let supply = suppliers(policy, &installed); - if coverage { return report_coverage(policy, &declaration, &installed, &supply); } + let report = evaluate(&path, &declaration, &installed, &supply)?; + if json { + println!( + "{}", + serde_json::to_string(&report).map_err(|error| Fatal::new(error.to_string()))? + ); + Ok(report.verdict.exit()) + } else { + report_human(&report) + } +} - let mut failures: Vec = Vec::new(); - let mut evidence: Vec = Vec::new(); - - for (index, claim) in declaration.enforce.iter().enumerate() { - let at = format!("enforce[{index}]"); - if claim.tier.is_some() { - return Err(Fatal::at( - &path, - format!( - "{at} carries a `tier`. The field is gone: a rule id resolves across \ - every seam at once, so a claim naming one no longer has to say which. \ - Drop the line." - ), - )); - } - let (Some(principle), Some(rule)) = (claim.principle.as_deref(), claim.rule.as_deref()) - else { - return Err(Fatal::at( - &path, - format!("{at}: `principle` and `rule` are both required"), - )); - }; - if principle.trim().is_empty() || rule.trim().is_empty() { - return Err(Fatal::at( - &path, - format!("{at}: `principle` and `rule` must not be blank"), - )); - } +fn validate_claim<'a>(path: &Path, index: usize, claim: &'a Claim) -> Result<(&'a str, &'a str)> { + let at = format!("enforce[{index}]"); + if claim.tier.is_some() { + return Err(Fatal::at( + path, + format!( + "{at} carries a `tier`. The field is gone: a rule id resolves across \ + every seam at once, so a claim naming one no longer has to say which. \ + Drop the line." + ), + )); + } + let (Some(principle), Some(rule)) = (claim.principle.as_deref(), claim.rule.as_deref()) else { + return Err(Fatal::at( + path, + format!("{at}: `principle` and `rule` are both required"), + )); + }; + if principle.trim().is_empty() || rule.trim().is_empty() { + return Err(Fatal::at( + path, + format!("{at}: `principle` and `rule` must not be blank"), + )); + } + Ok((principle, rule)) +} - let Some(record) = catalog::get(principle)? else { - failures.push(format!( +fn assess_claim( + index: usize, + principle: &str, + rule: &str, + supplied: bool, + unreadable: &[String], +) -> Result<(ClaimStatus, Option)> { + let at = format!("enforce[{index}]"); + let Some(record) = catalog::get(principle)? else { + return Ok(( + ClaimStatus::Refused, + Some(format!( "{at}: {}", catalog::unknown(principle, catalog::ids()?.len()) - )); - continue; - }; - if record.deprecated() { - failures.push(format!( + )), + )); + }; + if record.deprecated() { + return Ok(( + ClaimStatus::Refused, + Some(format!( "{at}: {principle:?} is deprecated; the catalog keeps it for redirects only" - )); - continue; - } - if record.refuses_automation() { - failures.push(format!( + )), + )); + } + if record.refuses_automation() { + return Ok(( + ClaimStatus::Refused, + Some(format!( "{at}: the {principle:?} record says enforcement.automatable = \"no\"; \ - no rule can be claimed to enforce it" - )); - continue; - } - - if let Some(by) = supply.supplied.get(rule) { - evidence.push(format!( - "{principle} <- {rule} enforced by {}", - by.join(", ") - )); - continue; - } - - if !installed.unreadable.is_empty() { - // A rule absent from what could be read is not an absent rule. The - // configuration that could not be inspected is exactly where it - // might be, so this is could-not-look and not a false claim. - return Err(Fatal::new(format!( + no rule can be claimed to enforce it" + )), + )); + } + if supplied { + return Ok((ClaimStatus::Reconciled, None)); + } + // A rule absent from an unreadable configuration is unknown, not false. + if !unreadable.is_empty() { + return Ok(( + ClaimStatus::Unavailable, + Some(format!( "{at}: no rule {rule:?} in what could be read, and {} \ - could not be read; cannot tell whether the claim holds", - installed.unreadable.join("; ") - ))); - } + could not be read; cannot tell whether the claim holds", + unreadable.join("; ") + )), + )); + } + let mut failure = format!("{at}: {principle:?} claims {rule:?}, which no seam here supplies"); + if let Some((_, went)) = REMOVED.iter().find(|(id, _)| *id == rule) { + failure.push_str("; "); + failure.push_str(went); + } + Ok((ClaimStatus::Refused, Some(failure))) +} - let mut failure = - format!("{at}: {principle:?} claims {rule:?}, which no seam here supplies"); - if let Some((_, went)) = REMOVED.iter().find(|(id, _)| *id == rule) { - failure.push_str("; "); - failure.push_str(went); +fn evaluate( + path: &Path, + declaration: &Declaration, + installed: &Installed, + supply: &Supply, +) -> Result { + // Invalid declarations are pre-evaluation failures: neither renderer emits a report. + let claims = declaration + .enforce + .iter() + .enumerate() + .map(|(index, claim)| validate_claim(path, index, claim)) + .collect::>>()?; + let mut report = Report { + schema_version: 1, + verdict: Verdict::Clean, + claims: Vec::new(), + violations: installed.misplaced.clone(), + unavailable: installed.unreadable.clone(), + notes: installed.how.clone(), + misplaced_count: installed.misplaced.len(), + fatal: None, + }; + for (index, (principle, rule)) in claims.into_iter().enumerate() { + let by = supply.supplied.get(rule); + let (status, diagnostic) = + assess_claim(index, principle, rule, by.is_some(), &installed.unreadable)?; + if let Some(diagnostic) = diagnostic { + if status == ClaimStatus::Unavailable { + if report.fatal.is_none() { + report.fatal = Some(diagnostic.clone()); + } + report.unavailable.push(diagnostic); + } else { + report.violations.push(diagnostic); + } } - failures.push(failure); + report.claims.push(EvaluatedClaim { + principle: principle.to_owned(), + rule: rule.to_owned(), + status, + suppliers: if status == ClaimStatus::Reconciled { + by.into_iter() + .flatten() + .map(|seam| Supplier { + component: if seam == "a hook installed here" { + "local" + } else { + "uphold" + }, + seam: seam.clone(), + }) + .collect() + } else { + Vec::new() + }, + }); } + // Preserve the reconcile's precedence: an indeterminate claim stops the run; + // otherwise a found violation outranks an unread runner configuration. + let exit = if report.fatal.is_some() { + Exit::Broken + } else { + verdict(report.violations.len(), report.unavailable.len()) + }; + report.verdict = match exit { + Exit::Clean => Verdict::Clean, + Exit::Violations => Verdict::Violations, + Exit::Broken => Verdict::CouldNotLook, + }; + Ok(report) +} - // A hook that can only fail where it is pinned is a defect in the file this - // reconcile reads, whatever it is claimed for, so it is refused beside the - // claims rather than inside them. - if !installed.misplaced.is_empty() { +fn report_human(report: &Report) -> Result { + if let Some(fatal) = &report.fatal { + return Err(Fatal::new(fatal.clone())); + } + if report.misplaced_count > 0 { eprintln!("hook configuration refused:"); - for misplaced in &installed.misplaced { + for misplaced in report.violations.iter().take(report.misplaced_count) { eprintln!("- {misplaced}"); } } - if !failures.is_empty() { + if report.violations.len() > report.misplaced_count { eprintln!("enforcement claims refused ({DECLARATION}):"); - for failure in &failures { + for failure in report.violations.iter().skip(report.misplaced_count) { eprintln!("- {failure}"); } } - if !failures.is_empty() || !installed.misplaced.is_empty() { + if !report.violations.is_empty() { return Ok(Exit::Violations); } - if !installed.unreadable.is_empty() { + if !report.unavailable.is_empty() { return Err(Fatal::new(format!( "runner configuration could not be read: {}", - installed.unreadable.join("; ") + report.unavailable.join("; ") ))); } - - println!("reconciled {} enforcement claims:", evidence.len()); - for line in &evidence { - println!(" {line}"); + println!("reconciled {} enforcement claims:", report.claims.len()); + for claim in &report.claims { + println!( + " {} <- {} enforced by {}", + claim.principle, + claim.rule, + claim + .suppliers + .iter() + .map(|supplier| supplier.seam.as_str()) + .collect::>() + .join(", ") + ); } - for note in &installed.how { + for note in &report.notes { println!(" note {note}"); } - Ok(Exit::Clean) + Ok(report.verdict.exit()) } /// The denominator the reconcile cannot see: rules that run and claim nothing. diff --git a/src/main.rs b/src/main.rs index 68947a0..21c5588 100644 --- a/src/main.rs +++ b/src/main.rs @@ -114,7 +114,7 @@ usage: uphold scan --text [FILE|-] run the host-identity rules over text uphold guard --stage STAGE run the guards that fire at STAGE uphold guard --text [FILE|-] run the text-capable guards over text - uphold check reconcile policy/upheld.toml against what runs + uphold check [--json] reconcile policy/upheld.toml against what runs uphold check --coverage which rules run here and carry no principle uphold audit --for-publication what a private->public flip would republish uphold init --owner OWNER write a first policy, claims and hook config @@ -361,10 +361,11 @@ fn run() -> Result { "guard" => guard_command(rest), "audit" => audit_command(rest), "check" => match rest { - [] => check_command(false), - [flag] if flag == "--coverage" => check_command(true), + [] => check_command(false, false), + [flag] if flag == "--coverage" => check_command(true, false), + [flag] if flag == "--json" => check_command(false, true), _ => Err(Fatal::new(format!( - "usage: uphold check [--coverage]\n\n{USAGE}" + "usage: uphold check [--coverage | --json]\n\n{USAGE}" ))), }, "hooks" => match rest { @@ -579,11 +580,11 @@ fn run() -> Result { /// The loader runs first and its answer is what the reconcile reads, which is /// the whole point of moving this in: `uphold_check.py` re-implemented /// `config::load` to answer the same question and was free to disagree with it. -fn check_command(coverage: bool) -> Result { +fn check_command(coverage: bool, json: bool) -> Result { let working = std::env::current_dir()?; let (root, policy_path) = discover(&working).ok_or_else(|| no_policy_here(&working))?; let policy = config::load(&root, &policy_path)?; - check::run(&root, &policy, coverage) + check::run(&root, &policy, coverage, json) } fn scan_command(arguments: &[OsString]) -> Result { diff --git a/tests/check_cli.rs b/tests/check_cli.rs index bad0b7b..9e3a272 100644 --- a/tests/check_cli.rs +++ b/tests/check_cli.rs @@ -1190,3 +1190,247 @@ fn a_claim_on_a_text_only_rule_is_credited_to_scan_text() { stdout(&present) ); } + +#[test] +fn json_and_human_output_share_the_evaluated_claims() { + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write(&root, ".pre-commit-config.yaml", PRE_COMMIT); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"prevent-public-push\"\n\n[[enforce]]\nprinciple = \"explicit-unknown\"\nrule = \"my-own-check\"\n", + ); + let human = check(&root, &[]); + let machine = check(&root, &["--json"]); + assert_eq!(code(&human), 0, "{}", stderr(&human)); + assert_eq!(code(&machine), 0, "{}", stderr(&machine)); + assert!(machine.stderr.is_empty(), "{}", stderr(&machine)); + let report: serde_json::Value = serde_json::from_slice(&machine.stdout).unwrap(); + assert_eq!(report.pointer("/schema_version").unwrap(), 1); + assert_eq!(report.pointer("/verdict").unwrap(), "clean"); + let claims = report.pointer("/claims").unwrap().as_array().unwrap(); + assert_eq!(claims.len(), 2); + for claim in claims { + assert_eq!(claim.pointer("/status").unwrap(), "reconciled"); + let principle = claim.pointer("/principle").unwrap().as_str().unwrap(); + let rule = claim.pointer("/rule").unwrap().as_str().unwrap(); + let suppliers = claim.pointer("/suppliers").unwrap().as_array().unwrap(); + let seams: Vec<&str> = suppliers + .iter() + .map(|by| by.pointer("/seam").unwrap().as_str().unwrap()) + .collect(); + assert!(stdout(&human).contains(&format!( + "{principle} <- {rule} enforced by {}", + seams.join(", ") + ))); + } + assert_eq!( + claims + .first() + .unwrap() + .pointer("/suppliers/0/component") + .unwrap(), + "uphold" + ); + assert_eq!( + claims + .last() + .unwrap() + .pointer("/suppliers/0/component") + .unwrap(), + "local" + ); + assert_eq!( + report.pointer("/violations").unwrap(), + &serde_json::json!([]) + ); + assert_eq!( + report.pointer("/unavailable").unwrap(), + &serde_json::json!([]) + ); + for note in report.pointer("/notes").unwrap().as_array().unwrap() { + assert!(stdout(&human).contains(note.as_str().unwrap())); + } +} + +#[test] +fn json_refusal_keeps_the_claims_that_reconciled() { + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write(&root, ".pre-commit-config.yaml", PRE_COMMIT); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"explicit-unknown\"\nrule = \"my-own-check\"\n\n[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"missing-check\"\n", + ); + let human = check(&root, &[]); + let machine = check(&root, &["--json"]); + assert_eq!(code(&human), 1); + assert_eq!(human.stdout, Vec::::new()); + assert_eq!(code(&machine), 1, "{}", stderr(&machine)); + let report: serde_json::Value = serde_json::from_slice(&machine.stdout).unwrap(); + assert_eq!(report.pointer("/verdict").unwrap(), "violations"); + assert_eq!(report.pointer("/claims/0/status").unwrap(), "reconciled"); + assert_eq!(report.pointer("/claims/1/status").unwrap(), "refused"); + assert_eq!( + report.pointer("/claims/1/suppliers").unwrap(), + &serde_json::json!([]) + ); + let failure = report.pointer("/violations/0").unwrap().as_str().unwrap(); + assert!(failure.contains("missing-check")); + assert!(stderr(&human).contains(failure)); +} + +#[test] +fn json_unavailable_evidence_preserves_partial_claims_and_status_precedence() { + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write(&root, ".pre-commit-config.yaml", PRE_COMMIT); + write(&root, ".codex/hooks.json", "{ not json"); + let held = "[[enforce]]\nprinciple = \"explicit-unknown\"\nrule = \"my-own-check\"\n"; + // A supplied claim remains visible even when another configuration cannot be read. + for (extra, expected, status) in [ + ("", 2, "reconciled"), + ( + "\n[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"missing-check\"\n", + 2, + "unavailable", + ), + ( + "\n[[enforce]]\nprinciple = \"not-a-principle\"\nrule = \"my-own-check\"\n", + 1, + "refused", + ), + ] { + write(&root, "policy/upheld.toml", &format!("{held}{extra}")); + let machine = check(&root, &["--json"]); + assert_eq!(code(&machine), expected, "{}", stderr(&machine)); + assert_eq!(code(&check(&root, &[])), expected); + let report: serde_json::Value = serde_json::from_slice(&machine.stdout).unwrap(); + assert_eq!(report.pointer("/claims/0/status").unwrap(), "reconciled"); + assert_eq!( + report + .pointer("/claims") + .unwrap() + .as_array() + .unwrap() + .last() + .unwrap() + .get("status") + .unwrap(), + status + ); + assert!( + report + .pointer("/unavailable") + .unwrap() + .as_array() + .unwrap() + .iter() + .any(|note| note.as_str().unwrap().contains(".codex/hooks.json")) + ); + assert_eq!( + report.pointer("/verdict").unwrap(), + if expected == 1 { + "violations" + } else { + "could_not_look" + } + ); + } +} + +#[test] +fn json_pre_evaluation_failures_have_no_report() { + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + for declaration in [ + None, + Some("[[enforce]] this is not TOML"), + Some("[[enforce]]\nprinciple = \"explicit-unknown\"\n"), + ] { + if let Some(contents) = declaration { + write(&root, "policy/upheld.toml", contents); + } + let output = check(&root, &["--json"]); + assert_eq!(code(&output), 2); + assert_eq!(output.stdout, Vec::::new()); + assert_ne!(output.stderr, Vec::::new()); + } + write(&root, "policy/principles.toml", "[rule"); + let output = check(&root, &["--json"]); + assert_eq!(code(&output), 2); + assert_eq!(output.stdout, Vec::::new()); +} + +#[test] +fn json_is_a_reconciliation_format_and_not_a_coverage_format() { + let root = workspace(); + let output = check(&root, &["--coverage", "--json"]); + assert_eq!(code(&output), 2); + assert_eq!(output.stdout, Vec::::new()); + assert!(stderr(&output).contains("usage: uphold check")); +} + +#[test] +fn misplaced_range_hooks_do_not_supply_partial_claim_evidence() { + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = 'explicit-unknown'\nrule = 'uphold-supply-chain'\n", + ); + for (stages, other_hook, lefthook, expected_status, expected_exit) in [ + ("[manual]", "", "", "refused", 1), + ("[pre-commit]", "", "", "refused", 1), + ("[]", "", "", "refused", 1), + ("[pre-push, manual]", "", "", "reconciled", 1), + ( + "[manual]", + " - id: uphold-supply-chain\n stages: [pre-push]\n", + "", + "reconciled", + 1, + ), + ( + "[manual]", + "", + "pre-push:\n commands:\n uphold-supply-chain:\n run: ./check.sh\n", + "reconciled", + 1, + ), + ("[pre-push]", "", "", "reconciled", 0), + ] { + let config = format!( + "repos:\n - repo: https://github.com/HackingGate/uphold\n rev: v2.0.0\n hooks:\n - id: uphold-supply-chain\n stages: {stages}\n{other_hook}" + ); + write(&root, ".pre-commit-config.yaml", &config); + write(&root, "lefthook.yml", lefthook); + let output = check(&root, &["--json"]); + assert_eq!( + code(&output), + expected_exit, + "{}{}", + stdout(&output), + stderr(&output) + ); + let report: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!( + report.pointer("/claims/0/status").unwrap(), + expected_status, + "{report}" + ); + let suppliers = report + .pointer("/claims/0/suppliers") + .unwrap() + .as_array() + .unwrap(); + assert_eq!( + suppliers.is_empty(), + expected_status == "refused", + "{report}" + ); + } +} diff --git a/tests/test_uphold_check.py b/tests/test_uphold_check.py index f7d9045..3dbe530 100644 --- a/tests/test_uphold_check.py +++ b/tests/test_uphold_check.py @@ -289,15 +289,7 @@ def test_a_refused_declaration_exports_nothing(self): @needs_the_engine class TheTwoReadersOfOneReport(unittest.TestCase): - """The binary decides which rules run; the export must carry that answer whole. - - Since the reconcile moved into the loader, this script no longer derives a - rule set of its own -- it PARSES the binary's report. That closes the old - disagreement and opens a quieter one: a parser that stops recognising the - report it reads produces an empty answer rather than an error, and an empty - answer here is a component definition with no components, published at exit - 0 over a repository whose reconcile had just succeeded on every claim. - """ + """The binary decides which rules run; its machine contract carries that answer.""" def setUp(self): self._directory = tempfile.TemporaryDirectory() @@ -341,50 +333,248 @@ def test_the_binary_and_the_export_name_the_same_rules(self): } self.assertEqual(rules, {"prevent-public-push", "no-merge-conflict-markers"}) - def test_a_report_this_reader_cannot_parse_is_could_not_look(self): - """One word of the report changes, and the reader recovers nothing. + def test_misplaced_range_only_hook_does_not_silence_review(self): + exemptions = [ + f'{key} = "No subject in this scratch repository."' + for key, record in uphold_check.load_records().items() + if record["enforcement"]["automatable"] == "yes" + and key != "explicit-unknown" + ] + build( + self.tmp, + "\n".join( + [ + "[[enforce]]", + 'principle = "explicit-unknown"', + 'rule = "uphold-supply-chain"', + "[review]", + "max_lines = 10000", + "[review.no_subject_here]", + *exemptions, + ] + ), + **{ + ".pre-commit-config.yaml": "repos:\n - repo: https://github.com/HackingGate/uphold\n rev: v2.0.0\n hooks:\n - id: uphold-supply-chain\n stages: [manual]\n", + }, + ) + self.assertEqual(uphold_check.engine_suppliers(self.tmp, strict=False), {}) + result = run(self.tmp, "--review") + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn("explicit-unknown", result.stderr) + self.assertIn("no rule here claims it", result.stderr) - Before the count check this returned `{}`, which is indistinguishable - from a repository where no claim is supplied by anything. - """ + def test_human_wording_is_not_part_of_the_consumer_contract(self): real = uphold_check.engine - def drifted(root, *args): - answered = real(root, *args) - if args[:1] == ("check",): - answered.stdout = answered.stdout.replace("enforced by", "supplied by") - return answered + def human_wording(root, *args): + self.assertEqual(args, ("check", "--json")) + return real(root, *args) - with ( - mock.patch.object(uphold_check, "engine", drifted), - self.assertRaises(uphold_check.CouldNotLook) as raised, - ): - uphold_check.engine_suppliers(self.tmp) - self.assertIn("2 reconciled claim(s)", str(raised.exception)) - self.assertIn("0 evidence line(s)", str(raised.exception)) - - def test_a_drifted_report_publishes_no_component_definition(self): - """Could not look is exit 2 and no document, not exit 0 and an empty one.""" - real = uphold_check.engine - - def drifted(root, *args): - answered = real(root, *args) - if args[:1] == ("check",): - answered.stdout = answered.stdout.replace("enforced by", "supplied by") - return answered + with mock.patch.object(uphold_check, "engine", human_wording): + self.assertEqual( + uphold_check.engine_suppliers(self.tmp), + { + "prevent-public-push": ["uphold"], + "no-merge-conflict-markers": ["uphold"], + }, + ) + def test_malformed_json_publishes_no_component_definition(self): + failed = subprocess.CompletedProcess( + ["uphold", "check", "--json"], 0, "not JSON", "" + ) stdout = io.StringIO() with ( contextlib.chdir(self.tmp), contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(io.StringIO()), - mock.patch.object(uphold_check, "engine", drifted), + mock.patch.object(uphold_check, "engine", return_value=failed), ): code = uphold_check.main(["--oscal"]) self.assertEqual(code, 2) self.assertEqual(stdout.getvalue().strip(), "") +class ReconciliationContract(unittest.TestCase): + def clean_report(self): + return { + "schema_version": 1, + "verdict": "clean", + "claims": [ + { + "principle": "explicit-unknown", + "rule": "my-check", + "status": "reconciled", + "suppliers": [{"component": "local", "seam": "wording can change"}], + } + ], + "violations": [], + "unavailable": [], + "notes": [], + } + + def suppliers(self, report, *, status=0, strict=True): + answered = subprocess.CompletedProcess( + ["uphold", "check", "--json"], status, json.dumps(report), "" + ) + with mock.patch.object(uphold_check, "engine", return_value=answered) as engine: + result = uphold_check.engine_suppliers(ROOT, strict=strict) + engine.assert_called_once_with(ROOT, "check", "--json") + return result + + def test_component_is_explicit_and_seam_wording_does_not_route_it(self): + report = self.clean_report() + self.assertEqual(self.suppliers(report), {"my-check": ["local"]}) + report["claims"][0]["suppliers"][0]["seam"] = "a hook installed here" + report["claims"][0]["suppliers"][0]["component"] = "uphold" + self.assertEqual(self.suppliers(report), {"my-check": ["uphold"]}) + + def test_empty_evaluated_claims_are_a_valid_clean_report(self): + report = self.clean_report() + report["claims"] = [] + self.assertEqual(self.suppliers(report), {}) + + def test_unknown_versions_including_boolean_and_float_are_rejected(self): + for version in (None, True, 1.0, 0, 2, "1"): + with ( + self.subTest(version=version), + self.assertRaises(uphold_check.CouldNotLook), + ): + report = self.clean_report() + report["schema_version"] = version + self.suppliers(report) + + def test_malformed_and_inconsistent_reports_fail_closed(self): + mutations = [ + lambda r: r.pop("claims"), + lambda r: r.update(claims={}), + lambda r: r.update(verdict="unknown"), + lambda r: r.update(verdict="violations", violations=["refused"]), + lambda r: r.update(unavailable=["missing config"]), + lambda r: r.update(violations=["refused"]), + lambda r: r.update(notes=[3]), + lambda r: r["claims"].append(None), + lambda r: r["claims"][0].update(rule=""), + lambda r: r["claims"][0].update(status="unknown"), + lambda r: r["claims"][0].update(status="refused", suppliers=[]), + lambda r: r["claims"][0].update(status="unavailable", suppliers=[]), + lambda r: r["claims"][0].update(suppliers=[]), + lambda r: r["claims"][0].update(suppliers=[None]), + lambda r: r["claims"][0]["suppliers"][0].update(component="other"), + lambda r: r["claims"][0]["suppliers"][0].update(seam=""), + ] + for mutate in mutations: + with ( + self.subTest(mutate=mutate), + self.assertRaises(uphold_check.CouldNotLook), + ): + report = self.clean_report() + mutate(report) + self.suppliers(report, strict=False) + for report in (None, [], True): + with ( + self.subTest(report=report), + self.assertRaises(uphold_check.CouldNotLook), + ): + self.suppliers(report) + + def test_malformed_json_and_duplicate_fields_fail_closed(self): + for payload in ("", "not JSON", '{"schema_version": 1, "schema_version": 1}'): + answered = subprocess.CompletedProcess(["uphold", "check"], 0, payload, "") + with ( + self.subTest(payload=payload), + mock.patch.object(uphold_check, "engine", return_value=answered), + self.assertRaises(uphold_check.CouldNotLook), + ): + uphold_check.engine_suppliers(ROOT, strict=False) + + def test_review_retains_partial_evidence_from_an_evaluated_refusal(self): + report = self.clean_report() + report.update(verdict="violations", violations=["a false claim"]) + report["claims"].append( + { + "principle": "explicit-unknown", + "rule": "missing", + "status": "refused", + "suppliers": [], + } + ) + self.assertEqual( + self.suppliers(report, status=1, strict=False), {"my-check": ["local"]} + ) + with self.assertRaises(uphold_check.Refused): + self.suppliers(report, status=1) + + def test_unavailable_evidence_never_becomes_an_ordinary_review_refusal(self): + for verdict, status, violations in ( + ("could_not_look", 2, []), + ("violations", 1, ["bad claim"]), + ): + report = self.clean_report() + report.update( + verdict=verdict, violations=violations, unavailable=["unread runner"] + ) + for strict in (False, True): + with ( + self.subTest(verdict=verdict, strict=strict), + self.assertRaises(uphold_check.CouldNotLook), + ): + self.suppliers(report, status=status, strict=strict) + + def test_a_refused_claim_cannot_borrow_a_reconciled_claims_rule(self): + report = self.clean_report() + report["claims"][0]["rule"] = "my-check" + report["claims"].append( + { + "principle": "conways-law", + "rule": "my-check", + "status": "refused", + "suppliers": [], + } + ) + report.update( + verdict="violations", violations=["conways-law cannot be automated"] + ) + refused = subprocess.CompletedProcess( + ["uphold", "check"], 1, json.dumps(report), "" + ) + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + exemptions = [ + f'{key} = "No subject in this scratch repository."' + for key, record in uphold_check.load_records().items() + if record["enforcement"]["automatable"] == "yes" + and key != "explicit-unknown" + ] + build( + root, + "\n".join( + [ + "[[enforce]]", + 'principle = "explicit-unknown"', + 'rule = "my-check"', + "[[enforce]]", + 'principle = "conways-law"', + 'rule = "my-check"', + "[review]", + "max_lines = 10000", + "[review.no_subject_here]", + *exemptions, + ] + ), + ) + output = io.StringIO() + with ( + contextlib.chdir(root), + contextlib.redirect_stdout(output), + contextlib.redirect_stderr(io.StringIO()), + mock.patch.object(uphold_check, "engine", return_value=refused), + ): + code = uphold_check.main(["--review"]) + self.assertEqual(code, 0) + self.assertIn("1 static rule(s)", output.getvalue()) + + class AnUnavailableEngine(unittest.TestCase): def test_unavailable_engine_prevents_review_emission_and_oscal(self): with tempfile.TemporaryDirectory() as directory: @@ -423,7 +613,21 @@ def test_unavailable_engine_prevents_review_emission_and_oscal(self): self.assertFalse((root / "REVIEW.md").exists()) def test_evaluated_refusal_remains_available_to_review_only(self): - refused = subprocess.CompletedProcess(["uphold", "check"], 1, "", "bad claim") + refused = subprocess.CompletedProcess( + ["uphold", "check", "--json"], + 1, + json.dumps( + { + "schema_version": 1, + "verdict": "violations", + "claims": [], + "violations": ["bad claim"], + "unavailable": [], + "notes": [], + } + ), + "", + ) with mock.patch.object(uphold_check, "engine", return_value=refused): self.assertEqual(uphold_check.engine_suppliers(ROOT, strict=False), {}) with self.assertRaises(uphold_check.Refused): diff --git a/uphold_check.py b/uphold_check.py index bb0be62..219bc9a 100755 --- a/uphold_check.py +++ b/uphold_check.py @@ -238,95 +238,122 @@ def declared_claims(declaration: dict) -> list[tuple[str, str]]: return claims -# The report `uphold check` prints, as the two fragments this reader keys on. -# Written down here rather than inline because the count check below is the only -# thing that notices when the binary's format and this reader drift apart, and a -# reader that has to be read alongside the format it parses should say what it -# expects in one place. -RECONCILED_PREFIX = "reconciled " -RECONCILED_SUFFIX = " enforcement claims:" -ENFORCED_BY = " enforced by " - +def report_object(pairs: list[tuple[str, object]]) -> dict: + """JSON objects cannot silently overwrite evidence through duplicate keys.""" + result = {} + for key, value in pairs: + if key in result: + raise ValueError(f"duplicate JSON field {key!r}") + result[key] = value + return result + + +def reconciliation_report(answered: subprocess.CompletedProcess) -> dict: + """Validate the versioned engine contract before any evidence is consumed.""" + try: + report = json.loads(answered.stdout, object_pairs_hook=report_object) + except (ValueError, TypeError) as error: + raise CouldNotLook( + f"uphold check --json returned malformed JSON: {error}" + ) from error + if not isinstance(report, dict) or type(report.get("schema_version")) is not int: + raise CouldNotLook("uphold check --json has no integer schema_version") + if report["schema_version"] != 1: + raise CouldNotLook( + f"unsupported uphold check schema version {report['schema_version']!r}" + ) + verdicts = {"clean": 0, "violations": 1, "could_not_look": 2} + verdict = report.get("verdict") + if not isinstance(verdict, str) or verdicts.get(verdict) != answered.returncode: + raise CouldNotLook( + "uphold check --json verdict disagrees with its process status" + ) + for field in ("violations", "unavailable", "notes"): + values = report.get(field) + if not isinstance(values, list) or not all( + isinstance(value, str) and value.strip() for value in values + ): + raise CouldNotLook(f"uphold check --json has malformed {field}") + claims = report.get("claims") + if not isinstance(claims, list): + raise CouldNotLook("uphold check --json has no claims array") + for claim in claims: + if not isinstance(claim, dict) or not all( + isinstance(claim.get(field), str) and claim[field].strip() + for field in ("principle", "rule") + ): + raise CouldNotLook("uphold check --json has a malformed claim") + status = claim.get("status") + if status not in ("reconciled", "refused", "unavailable"): + raise CouldNotLook("uphold check --json has an unknown claim status") + suppliers = claim.get("suppliers") + if not isinstance(suppliers, list) or bool(suppliers) != ( + status == "reconciled" + ): + raise CouldNotLook("uphold check --json has inconsistent claim suppliers") + for supplier in suppliers: + if ( + not isinstance(supplier, dict) + or supplier.get("component") not in ("local", "uphold") + or not isinstance(supplier.get("seam"), str) + or not supplier["seam"].strip() + ): + raise CouldNotLook("uphold check --json has a malformed supplier") + if status == "refused" and not report["violations"]: + raise CouldNotLook("uphold check --json omits the claim's violation") + if status == "unavailable" and not report["unavailable"]: + raise CouldNotLook( + "uphold check --json omits the claim's unavailable evidence" + ) + if ( + (verdict == "clean" and (report["violations"] or report["unavailable"])) + or (verdict == "violations" and not report["violations"]) + or (verdict == "could_not_look" and not report["unavailable"]) + ): + raise CouldNotLook("uphold check --json has inconsistent verdict evidence") + return report -def reconciled_count(line: str) -> int | None: - """The N of `reconciled N enforcement claims:`, or None for any other line.""" - line = line.strip() - if not (line.startswith(RECONCILED_PREFIX) and line.endswith(RECONCILED_SUFFIX)): - return None - count = line[len(RECONCILED_PREFIX) : -len(RECONCILED_SUFFIX)] - return int(count) if count.isdigit() else None +def engine_report(root: Path, *, strict: bool = True) -> dict: + """Read Rust's evaluated evidence through its versioned JSON contract. -def engine_suppliers(root: Path, *, strict: bool = True) -> dict[str, list[str]]: - """Which seams supply each rule, as the reconcile in the binary sees it. - - `strict` is the difference between the two callers. `--oscal` publishes an - assertion to an outside reader, so a declaration that does not reconcile has - nothing honest to export and the refusal travels. `--review` runs over a - declaration somebody is still writing: a claim naming a rule nothing - supplies is exactly the state it exists to help with, and refusing there - would take the review document away at the moment it is most wanted. The - evidence lines for the claims that DID hold are on stdout either way. - - The evidence lines are counted against the binary's own header, because this - is a PARSER of another program's report and a parser that stops recognising - it fails silently: every line skipped by the `continue` below, an empty - mapping returned, and `--oscal` publishing a component definition with no - components at all -- exit 0, to an outside reader, over a repository whose - reconcile had just succeeded on every claim. Nothing in the pipeline could - tell that apart from a repository that enforces nothing. A report this - reader could not read is could-not-look; see the `explicit-unknown` record. + OSCAL requires a clean reconciliation. Review also accepts an evaluated + refusal and retains evidence for claims that held. Neither can use a report + with unavailable evidence, even when a violation determines the exit code. """ - answered = engine(root, "check") - if answered.returncode == 2: - raise CouldNotLook(answered.stderr.strip() or "uphold check could not look") - if answered.returncode not in (0, 1): + answered = engine(root, "check", "--json") + if answered.returncode not in (0, 1, 2): raise CouldNotLook( f"uphold check exited unexpectedly with status {answered.returncode}: " f"{answered.stderr.strip() or 'no diagnostic was reported'}" ) - if answered.returncode != 0 and strict: + if answered.returncode == 2 and not answered.stdout.strip(): + # Loading/discovery/declaration failures happen before a report exists. + raise CouldNotLook(answered.stderr.strip() or "uphold check could not look") + report = reconciliation_report(answered) + if report["unavailable"]: + raise CouldNotLook( + "uphold check could not look: " + "; ".join(report["unavailable"]) + ) + if answered.returncode == 1 and strict: raise Refused( "the enforcement claims do not reconcile, so there is nothing " - f"honest to export:\n{answered.stderr.strip()}" + "honest to export:\n" + "\n".join(report["violations"]) ) + return report + + +def engine_suppliers(root: Path, *, strict: bool = True) -> dict[str, list[str]]: + """Fold reconciled claims into the components used by the OSCAL export.""" + report = engine_report(root, strict=strict) suppliers: dict[str, list[str]] = {} - reconciled: int | None = None - read = 0 - for line in answered.stdout.splitlines(): - if reconciled is None: - reconciled = reconciled_count(line) - if " <- " not in line or ENFORCED_BY not in line: + for claim in report["claims"]: + if claim["status"] != "reconciled": continue - read += 1 - _, rest = line.split(" <- ", 1) - rule, by = rest.split(ENFORCED_BY, 1) - # Folded back to the SEAM, because an OSCAL component is a thing that - # implements a control and the seam is that thing. `uphold check` names - # the evidence -- which stage, which scan -- and a component per stage - # would split one implementation across five. - seams = [] - for part in by.split(","): - part = part.strip() - if not part: - continue - seams.append("local" if part.startswith("a hook") else "uphold") - for seam in seams: - if seam not in suppliers.setdefault(rule.strip(), []): - suppliers[rule.strip()].append(seam) - - # Only where the binary said the reconcile HELD. A refused one prints its - # failures on stderr and nothing on stdout, and `--review` is meant to keep - # going over exactly that; there is no count there to check against, and - # requiring one would turn the state this mode exists for into exit 2. - if answered.returncode == 0 and read != reconciled: - raise CouldNotLook( - f"`uphold check` reported {reconciled if reconciled is not None else 'no'} " - f"reconciled claim(s) and this reader recovered {read} evidence line(s) " - "from the same report, so which seam supplies which rule is unknown. " - "The binary's report and this reader have drifted apart; the binary " - "is the authority, so fix the reader." - ) + for supplier in claim["suppliers"]: + seams = suppliers.setdefault(claim["rule"], []) + if supplier["component"] not in seams: + seams.append(supplier["component"]) return suppliers @@ -730,28 +757,28 @@ def run_review(argv: list[str]) -> int: (name, emit_target(root, name)) for name in settings["emit_controls"] ] claims = declared_claims(declaration) - suppliers = engine_suppliers(root, strict=False) + report = engine_report(root, strict=False) + reconciled = { + (claim["principle"], claim["rule"]) + for claim in report["claims"] + if claim["status"] == "reconciled" + } except CouldNotLook as error: print(f"uphold review could not look: {error}", file=sys.stderr) return 2 records = load_records() - # Filtered by `suppliers`, exactly as `active` is on the line below. A - # principle leaves the review document because a rule enforces it -- that is - # the `continue` in `review.route`, "a rule enforces it; a reviewer - # repeating it is noise" -- and a claim naming a rule no seam here supplies - # enforces nothing. Unfiltered, such a claim removed the principle from the - # document AND was absent from the "already active here" list the same - # document prints, so an `automatable = "yes"` record could be enforced by - # nothing and reviewed by nobody, with the page showing no trace of either. - # - # The reconcile refuses that claim outright; this mode has to survive it, - # because `--review` runs over a declaration a person is still writing. - claimed = {principle for principle, rule in claims if rule in suppliers} + # Partial reports identify the exact claims that held. A refused claim may + # name the same rule as a reconciled one, so the rule id alone is insufficient. + claimed = { + principle for principle, rule in claims if (principle, rule) in reconciled + } for_review, errors, stale = review_mod.route( records, claimed, settings["exempt"], settings["include_domains"] ) - active = sorted({rule for _, rule in claims if rule in suppliers}) + active = sorted( + {rule for principle, rule in claims if (principle, rule) in reconciled} + ) document = review_mod.render(for_review, active) control_errors, uncontrolled = review_mod.audit_controls(