From f50cd5f104da2ec9f27158d39d76997196d37cac Mon Sep 17 00:00:00 2001 From: Tong Date: Wed, 12 Aug 2026 21:22:11 +0900 Subject: [PATCH 01/10] Name the seam a rule runs at, where an empty hook list could not `rules --effective --json` emitted `git_hooks` and nothing else, so a rule that fires at no git hook was indistinguishable from any other rule that fires at no git hook -- and there are two unrelated kinds. `files.*` is the scan's; `command.before` is a checker standing in front of a command, which runs when the shim is on PATH ahead of the real one. A reader with only the hooks has to guess between them. `uphold_check.py` guessed the scan. Its `elif scan` branch credited every hookless rule to `uphold scan`, so a claim on a shim-only rule reconciled green, exit 0, in a repository that pins `uphold-scan` and nothing else -- over a rule the scan never touches. This repository declares two such rules of its own, `no-published-host-identity` and `no-published-markers`. The drift test between the two readers did not catch it and could not have: it compared `git_hooks`, and both readers agreed on the empty list. They were wrong in the same direction, which is the failure mode a comparison of two implementations has and a comparison against the answer does not. So the loader says it. `Rule::seams` returns `scan` / `guard` / `shim` from the same conditions the three seams use to select rules, the JSON carries it, and the human form prints it where it used to print "no git hook" of both kinds. `_rule_stages` reads `command.before` to match, and returns a `Where` carrying both fields, so the drift test compares the seam as well. A shim-only rule is reported as a seam this script cannot establish -- no runner configuration here says whether the shim is on PATH -- rather than as one the scan supplies. That is could-not-look, and `inventory_local` remains where a repository asserts a seam this script cannot observe. --- docs/REFERENCE.md | 17 +++++-- src/config.rs | 35 ++++++++++++++ src/main.rs | 19 +++++++- tests/scan_cli.rs | 70 ++++++++++++++++++++++++++-- tests/test_uphold_check.py | 68 +++++++++++++++++++++++++++- uphold_check.py | 93 +++++++++++++++++++++++++++++++------- 6 files changed, 275 insertions(+), 27 deletions(-) diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 3a6464d..f071cd8 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -137,11 +137,18 @@ uphold rules --effective # every resolved rule, and where it fires uphold rules --effective --json # the same, for a program ``` -The JSON is one array of `{"id": ..., "git_hooks": [...]}`, in the order the -engine resolved them. It exists so that nothing has to re-implement the loader -to find out what runs — a second reader of these fields is a reader free to -disagree with the engine, and it will disagree exactly where somebody used a -field it does not know about. +The JSON is one array of `{"id": ..., "git_hooks": [...], "seams": [...]}`, in +the order the engine resolved them. It exists so that nothing has to +re-implement the loader to find out what runs — a second reader of these fields +is a reader free to disagree with the engine, and it will disagree exactly where +somebody used a field it does not know about. + +`seams` is `scan`, `guard`, `shim`, or more than one, and it is the half +`git_hooks` cannot express. An empty hook list is true of a content rule and of +a checker standing in front of a command alike, so a reader with only the hooks +has to guess between two unrelated places — and the reconciler guessed `scan`, +which credited a shim-only rule to a seam that never touches it. An empty +`seams` means nothing runs the rule at all, which the loader refuses. The two requests this shape exists to make writable: diff --git a/src/config.rs b/src/config.rs index 99b6068..7ab14bf 100644 --- a/src/config.rs +++ b/src/config.rs @@ -648,6 +648,41 @@ impl Rule { self.hooks().iter().any(|name| name == hook) } + /// Which seams run this rule: `scan`, `guard`, `shim`, in that order. + /// + /// The question `git.hooks` alone cannot answer. A rule with no hooks is + /// either a content rule the scan owns or a checker standing in front of a + /// command, and the two are not the same place -- but an empty hook list + /// looked identical for both, so every reader downstream had to guess, and + /// the reconciler guessed `scan`. A claim on a shim-only rule then + /// reconciled green in a repository where the scan never touches it. + /// + /// Answered here rather than derived by a caller, for the reason + /// `effective_rules_command` gives: every second reader of these fields is + /// a reader free to disagree with the engine about which rules run. + /// + /// Empty is a real answer and not a gap: it means nothing runs the rule. + /// `validate` refuses that at load, so it should not be reachable -- and it + /// is spelled out rather than folded into one of the three, because a rule + /// nobody runs must not read as a rule the scan runs. + pub(crate) fn seams(&self) -> Vec<&'static str> { + let mut seams = Vec::new(); + // The scan's own filter, in `scan::Scan::run`: a built-in is the scan's + // only when it reads files, and every other check that reads files is. + if self.reads_files() && (self.check() != Some(Check::Builtin) || self.hooks().is_empty()) { + seams.push("scan"); + } + if !self.hooks().is_empty() { + seams.push("guard"); + } + // `shim::run` consults `Check::Exec` rules only, and `validate` refuses + // `command.before` on anything else. + if self.command.is_some() { + seams.push("shim"); + } + seams + } + /// Whether this rule stands in front of `command` invoked as `argv`. pub(crate) fn stands_before(&self, command: &str, argv: &[String]) -> bool { self.command diff --git a/src/main.rs b/src/main.rs index 77aa2d0..e3e3d9a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -545,7 +545,13 @@ fn effective_rules_command(as_json: bool) -> Result { for rule in &policy.rules { let hooks = rule.hooks(); let at = if hooks.is_empty() { - String::from("no git hook") + // Which seam, where there is no hook to name. "no git hook" was + // true of a content rule and of a checker standing in front of + // `gh` alike, and those are not the same place. + match rule.seams().as_slice() { + [] => String::from("nothing runs it"), + seams => seams.join(", "), + } } else { hooks.join(", ") }; @@ -568,6 +574,17 @@ fn effective_rules_command(as_json: bool) -> Result { } json_string(hook, &mut document); } + // `git_hooks` alone cannot answer where a hookless rule runs, and a + // caller that has to guess guesses the scan -- which is how a claim on + // a rule whose only place is `command.before` reconciled green in a + // repository where nothing runs it. The loader knows; it says so here. + document.push_str("], \"seams\": ["); + for (position, seam) in rule.seams().iter().enumerate() { + if position > 0 { + document.push_str(", "); + } + json_string(seam, &mut document); + } document.push_str("]}"); } if !policy.rules.is_empty() { diff --git a/tests/scan_cli.rs b/tests/scan_cli.rs index 674e074..5b9c209 100644 --- a/tests/scan_cli.rs +++ b/tests/scan_cli.rs @@ -1154,14 +1154,18 @@ fn the_effective_rules_are_what_inheritance_resolved_to() { // the seam it fires at is installed. assert!( text.contains( - "{\"id\": \"no-local-merge\", \"git_hooks\": [\"pre-merge-commit\", \"manual\"]}" + "{\"id\": \"no-local-merge\", \"git_hooks\": [\"pre-merge-commit\", \"manual\"], \ + \"seams\": [\"guard\"]}" ), "{text}" ); // A content rule fires at no git hook, and says so rather than being - // reported under whichever stage happened to be installed. + // reported under whichever stage happened to be installed -- and names the + // seam, because an empty hook list is true of a content rule and of a + // checker standing in front of a command alike, and those are not the same + // place. A reader that has to guess between them guesses the scan. assert!( - text.contains("{\"id\": \"of-its-own\", \"git_hooks\": []}"), + text.contains("{\"id\": \"of-its-own\", \"git_hooks\": [], \"seams\": [\"scan\"]}"), "{text}" ); } @@ -1241,3 +1245,63 @@ fn a_guard_built_in_that_no_hook_runs_is_still_refused() { stderr(&output) ); } + +#[test] +fn a_rule_that_only_stands_in_front_of_a_command_names_the_shim_seam() { + // The seam `git_hooks` cannot express. A checker with `command.before` and + // no hooks looked identical to a content rule -- an empty list for both -- + // so every reader downstream had to guess, and the reconciler guessed the + // scan. A claim on such a rule then reconciled green in a repository where + // the scan never touches it. + let root = workspace(); + write( + &root, + "policy/principles.toml", + r#" + [[shim]] + command = "gh" + match = ["pr:create"] + text_flags = ["-b", "--body"] + + [rule.stands-in-front] + message = "do not publish that" + exec = "uphold guard --text -" + command.before = ["gh"] + + [rule.searches-the-tree] + message = "no TODO" + regexp = 'TODO' + files.include = ["."] +"#, + ); + + let output = Command::new(env!("CARGO_BIN_EXE_uphold")) + .args(["rules", "--effective", "--json"]) + .current_dir(&root) + .output() + .unwrap(); + assert_eq!(code(&output), 0, "{}", stderr(&output)); + let text = stdout(&output); + + assert!( + text.contains("{\"id\": \"stands-in-front\", \"git_hooks\": [], \"seams\": [\"shim\"]}"), + "{text}" + ); + assert!( + text.contains("{\"id\": \"searches-the-tree\", \"git_hooks\": [], \"seams\": [\"scan\"]}"), + "{text}" + ); + + // And the human form says it too, where it used to say "no git hook" of + // both. + let human = Command::new(env!("CARGO_BIN_EXE_uphold")) + .args(["rules", "--effective"]) + .current_dir(&root) + .output() + .unwrap(); + assert!( + stdout(&human).contains("stands-in-front (shim)"), + "{}", + stdout(&human) + ); +} diff --git a/tests/test_uphold_check.py b/tests/test_uphold_check.py index a8f8de2..cd4787b 100644 --- a/tests/test_uphold_check.py +++ b/tests/test_uphold_check.py @@ -435,7 +435,9 @@ def test_the_two_readers_of_the_policy_agree(self): ) self.assertEqual(answered.returncode, 0, answered.stderr) engine = { - entry["id"]: set(entry["git_hooks"]) + entry["id"]: uphold_check.Where( + hooks=set(entry["git_hooks"]), seams=set(entry["seams"]) + ) for entry in json.loads(answered.stdout) } declared, disabled, _sets, _paths = uphold_check.content_policy_rules(ROOT) @@ -444,6 +446,70 @@ def test_the_two_readers_of_the_policy_agree(self): } self.assertEqual(here, engine) + def test_a_rule_that_only_stands_in_front_of_a_command_is_not_the_scans(self): + """The seam `git.hooks` cannot name, and the reason it is compared. + + A rule whose only declared place is `command.before` fires when the shim + is on PATH ahead of the real command. It has no hooks and reads no + files, and while this reader knew only about hooks, an empty hook list + meant "the file scan's" -- so the rule was credited to `uphold scan` and + a claim on it reconciled green in a repository where the scan never + touches it. This repository has two such rules of its own. + + Asserted here rather than left to the agreement test above, because that + test compares the two readers and would stay green if BOTH were wrong in + the same direction, which is what they were. + """ + declared, _disabled, _sets, _paths = uphold_check.content_policy_rules(ROOT) + for rule_id in ("no-published-host-identity", "no-published-markers"): + where = declared[rule_id] + self.assertEqual(where.seams, {"shim"}, rule_id) + self.assertEqual(where.hooks, set(), rule_id) + + def test_a_claim_on_a_shim_only_rule_is_refused_and_not_credited_to_the_scan(self): + """The reconcile end of the same bug. + + The repository pins `uphold-scan` and nothing else, and its policy holds + one rule whose only declared place is `command.before`. Nothing here + establishes that the shim is on PATH in front of `gh`, so the claim is + not supplied -- but while an empty hook list meant "the file scan's", + the pinned `uphold-scan` was read as supplying it and the claim + reconciled green, exit 0, over a rule the scan never touches. + """ + with tempfile.TemporaryDirectory() as tmp: + build( + Path(tmp), + """ + [[enforce]] + principle = "complete-mediation" + rule = "no-published-markers" + """, + **{ + ".pre-commit-config.yaml": LOCAL_CONTENT_POLICY, + # The `[[shim]]` is not decoration: the engine refuses a + # `command.before` naming a command no shim declares, so a + # fixture without one is a policy that would never load. + "policy__principles.toml": ( + "[[shim]]\n" + 'command = "gh"\n' + 'match = ["pr:create"]\n' + 'text_flags = ["-b", "--body"]\n' + "\n" + "[rule.no-published-markers]\n" + 'message = "do not publish that"\n' + 'exec = "uphold guard --text -"\n' + 'command.before = ["gh"]\n' + ), + }, + ) + result = run(Path(tmp)) + coverage = run(Path(tmp), "--coverage") + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("no-published-markers", result.stderr) + # And the coverage report says which seam went unestablished, rather + # than listing the rule as one the scan runs. + self.assertIn("stands in front of a command", coverage.stdout) + def test_inherit_paths_naming_a_file_that_is_not_there_is_two_not_one(self): with tempfile.TemporaryDirectory() as tmp: build( diff --git a/uphold_check.py b/uphold_check.py index 17077f0..264aab6 100755 --- a/uphold_check.py +++ b/uphold_check.py @@ -556,8 +556,37 @@ def lefthook_commands(root: Path) -> set[str]: return names -def _rule_stages(policy: dict) -> dict[str, set[str]]: - """Every rule id in one policy document, mapped to the git stages it fires at. +class Where: + """The seams one rule runs at, and the git hooks if `guard` is one of them. + + Two fields rather than one, because `git.hooks` alone cannot answer the + question. An empty hook list was read as "the file scan's rule", and that is + true of a content rule and false of a checker standing in front of `gh` -- + which is how a claim on a rule whose only place is `command.before` was + credited to `uphold scan` and reconciled green in a repository where the + scan never touches it. + + `seams` holds the same three names the engine prints in + `uphold rules --effective --json`, and the drift test compares them. + """ + + __slots__ = ("hooks", "seams") + + def __init__(self, hooks: set[str], seams: set[str]) -> None: + self.hooks = hooks + self.seams = seams + + def __eq__(self, other: object) -> bool: + if not isinstance(other, Where): + return NotImplemented + return self.hooks == other.hooks and self.seams == other.seams + + def __repr__(self) -> str: + return f"Where(hooks={sorted(self.hooks)}, seams={sorted(self.seams)})" + + +def _rule_stages(policy: dict) -> dict[str, Where]: + """Every rule id in one policy document, mapped to where it runs. ONE table name, which is the whole point. This function used to walk a hardcoded list of six array-of-tables names against an engine that had @@ -569,24 +598,40 @@ def _rule_stages(policy: dict) -> dict[str, set[str]]: The id is the section header -- `[rule.]` -- so the ids are the keys of one table, and a duplicate cannot even parse. - `git.hooks` is carried out rather than discarded because it is the field - that says WHERE a rule runs, and a caller that has only the ids cannot tell - a file rule from a pre-push guard. An empty set means no git hook runs it, - which is the file scan's rule and not a rule that runs nowhere. + The seam is carried out beside the hooks, and it is the field that was + missing. `git.hooks` says which git stages a guard fires at; it says nothing + about a rule that fires at none, and there are two unrelated kinds of those. + `files.*` is the scan's, `command.before` is the shim's, and reading the + second as the first is the whole of the bug this record exists to close. """ rules = policy.get("rule", {}) if not isinstance(rules, dict): raise CouldNotLook("policy: [rule] must be a table of [rule.] sections") - stages: dict[str, set[str]] = {} + stages: dict[str, Where] = {} for rule_id, body in rules.items(): - git = body.get("git", {}) if isinstance(body, dict) else {} - hooks = git.get("hooks", []) if isinstance(git, dict) else [] + body = body if isinstance(body, dict) else {} + git = body.get("git", {}) if isinstance(body.get("git"), dict) else {} + hooks = git.get("hooks", []) if not isinstance(hooks, list): raise CouldNotLook( f"policy: [rule.{rule_id}] git.hooks must be an array of git hook names" ) - stages[rule_id] = {value for value in hooks if isinstance(value, str)} + hooks = {value for value in hooks if isinstance(value, str)} + + seams: set[str] = set() + # The engine's own filter, in `Rule::seams`: a built-in reaches the scan + # only when it reads files and fires at no hook; every other rule that + # declares `files.*` is the scan's. + if isinstance(body.get("files"), dict) and not ( + isinstance(body.get("builtin"), str) and hooks + ): + seams.add("scan") + if hooks: + seams.add("guard") + if isinstance(body.get("command"), dict): + seams.add("shim") + stages[rule_id] = Where(hooks=hooks, seams=seams) return stages @@ -861,18 +906,32 @@ def inventory_principles(root: Path) -> Inventory: # demonstrably running. rules: set[str] = set() uninstalled: list[str] = [] - for rule_id, hooks in sorted(declared.items()): + for rule_id, where in sorted(declared.items()): if rule_id in disabled: continue - if hooks: - if hooks & stages: + if "guard" in where.seams: + if where.hooks & stages: + rules.add(rule_id) + else: + uninstalled.append(f"{rule_id} ({', '.join(sorted(where.hooks))})") + elif "scan" in where.seams: + if scan: rules.add(rule_id) else: - uninstalled.append(f"{rule_id} ({', '.join(sorted(hooks))})") - elif scan: - rules.add(rule_id) + uninstalled.append(f"{rule_id} (file scan)") + elif "shim" in where.seams: + # The shim seam, which this branch used to fall through to `scan`. + # A checker standing in front of `gh` runs when the shim is on PATH + # ahead of the real command, and no runner configuration in this + # repository says whether it is -- so the honest answer is that it + # was not established here, not that the file scan supplies it. + # `inventory_local` is where a repository asserts a seam this script + # cannot observe. + uninstalled.append(f"{rule_id} (stands in front of a command)") else: - uninstalled.append(f"{rule_id} (file scan)") + # `validate` refuses a rule with no declared place, so reaching this + # means the policy said something the engine would not have loaded. + uninstalled.append(f"{rule_id} (nothing declares where it runs)") if uninstalled: notes.append( "declared, but no runner configuration here installs the seam it " From b93623eee1a830d59fba1249009a554774c3ff33 Mon Sep 17 00:00:00 2001 From: Tong Date: Wed, 12 Aug 2026 21:22:49 +0900 Subject: [PATCH 02/10] Keep a principle in the review when the claim on it enforces nothing `--review` builds `claimed` from every entry in the declaration and `active` from the entries whose rule a seam actually supplies. Two lines apart, one filtered and one not, and the unfiltered one decided what a human is asked about. `review.route` drops an `automatable = "yes"` record when a rule claims it -- "a rule enforces it; a reviewer repeating it is noise" -- which is right when a rule does enforce it. A claim naming a rule no seam here supplies enforces nothing, so it is not that case: the record left the review document, and the rule was absent from the "already active here" list the same document prints, because that list IS filtered. Enforced by nothing, reviewed by nobody, and the page showed no trace of either. The reconcile refuses such a claim outright. This mode has to survive one, because it runs over a declaration somebody is still writing -- so it filters rather than refuses, and the record goes back to the reviewer it was taken from. --- tests/test_review.py | 64 ++++++++++++++++++++++++++++++++++++++++++++ uphold_check.py | 13 ++++++++- 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/tests/test_review.py b/tests/test_review.py index 77be8ac..167d833 100644 --- a/tests/test_review.py +++ b/tests/test_review.py @@ -147,6 +147,70 @@ def test_no_field_beyond_those_three_crosses_over(self): self.assertNotIn("the rationale", document) +class ClaimsThatEnforceNothing(unittest.TestCase): + """A claim naming a rule no seam supplies must not silence the review. + + `route` drops an `automatable = "yes"` record from the document when a rule + claims it -- "a rule enforces it; a reviewer repeating it is noise". A claim + whose rule nothing here supplies enforces nothing, so it is not that case, + and passing it in unfiltered removed the record from the human tier while + the same document's "already active here" list -- which IS filtered by + suppliers -- left the rule out. Enforced by nothing and reviewed by nobody, + with the page showing no trace of either. + """ + + def setUp(self): + self._directory = tempfile.TemporaryDirectory() + self.tmp = Path(self._directory.name) + self.addCleanup(self._directory.cleanup) + (self.tmp / "policy").mkdir() + + def review(self, declaration: str, policy: str) -> subprocess.CompletedProcess: + (self.tmp / "policy" / "upheld.toml").write_text( + textwrap.dedent(declaration), encoding="utf-8" + ) + (self.tmp / "policy" / "principles.toml").write_text( + textwrap.dedent(policy), encoding="utf-8" + ) + (self.tmp / ".pre-commit-config.yaml").write_text( + "repos:\n" + " - repo: https://github.com/HackingGate/uphold\n" + " rev: v2.0.0\n" + " hooks:\n" + " - id: uphold-scan\n", + encoding="utf-8", + ) + return subprocess.run( + [sys.executable, str(SCRIPT), "--review"], + cwd=self.tmp, + capture_output=True, + text=True, + check=False, + ) + + def test_a_claim_no_seam_supplies_does_not_remove_its_principle_from_review(self): + # `fail-safe-defaults` is `automatable = "yes"` in the shipped + # catalog, and the policy here supplies no rule by the claimed name at + # all -- so the claim enforces nothing and the record still needs an + # answer from somebody. + result = self.review( + """ + [[enforce]] + principle = "fail-safe-defaults" + rule = "a-rule-that-does-not-exist" + """, + """ + [rule.no-todo] + message = "no TODO" + regexp = 'TODO' + files.include = ["."] + """, + ) + self.assertEqual(result.returncode, 1, result.stdout) + self.assertIn("fail-safe-defaults", result.stderr) + self.assertIn("no rule here claims it", result.stderr) + + class Settings(unittest.TestCase): """`[review]` is configuration, so a field of the wrong type is exit 2. diff --git a/uphold_check.py b/uphold_check.py index 264aab6..982c246 100755 --- a/uphold_check.py +++ b/uphold_check.py @@ -1411,7 +1411,18 @@ def run_review(argv: list[str]) -> int: return 2 records = load_records() - claimed = {principle for principle, _ in claims} + # 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} for_review, errors, stale = review_mod.route( records, claimed, settings["exempt"], settings["include_domains"] ) From 8411a963712c01ceb7e0df0753bae21bd323adc7 Mon Sep 17 00:00:00 2001 From: Tong Date: Wed, 12 Aug 2026 21:56:01 +0900 Subject: [PATCH 03/10] Let the loader answer which rules run, and close the second reader `uphold_check.py` re-implemented `config::load` to reconcile a declaration: bundled sets, `inherit.paths`, `inherit.disabled_rules`, and a repository's own rule shadowing an inherited id. Five interacting fields, read twice, by two programs free to disagree -- and `effective_rules_command` has said since it was written that this is what would end it: "every second reader of them is a reader free to disagree with the engine about which rules run. The reconciler in `uphold_check.py` is that second reader today." They disagreed about the seam a hookless rule runs at. `files.*` is the scan's and `command.before` is a checker standing in front of a command; both come back with no git hooks, and the reconciler read the second as the first. A claim on a shim-only rule reconciled green, exit 0, in a repository that pins `uphold-scan` and nothing else -- over a rule the scan never touches. `Rule::seams` answers it now, from the same conditions the three seams use to select rules, and it is on `rules --effective --json` for anything else that has to ask. The reconcile and the coverage report are `uphold check` and `uphold check --coverage`. What stayed in Python is what never reads the policy -- `--explain`, `--list`, `--review`, `--oscal`, `--init` -- because a mode that cannot read the policy cannot disagree with the loader about which rules run. `--oscal` gates on the reconcile, so it asks the binary and treats an unreachable one as could-not-look; `--review` asks too but survives a refusal, because it runs over a declaration somebody is still writing. `uphold-check` becomes `language: rust` like every other id in the manifest. It was the only `language: script` one, and pre-commit and prek key an environment on (repo, language, version) -- so it now shares the environment the other seven ids already build, and costs no second compile. Three things the port fixes rather than carries: The runner configs are PARSED. The script line-scanned them because it could take no dependency, which is how `configs:` -- the key README.md tells every lefthook consumer to write under `remotes:` -- was read as a command name. `package.repository` is read at compile time through `CARGO_PKG_REPOSITORY` rather than off disk, so the slug cannot drift from the crate and works outside a checkout of this repository. The coverage denominator counts what a seam SUPPLIES. `records: N of M` was computed from the claims, so a declaration whose only claim named a rule nothing runs reported one record as claimed two lines under the line saying that rule is supplied by nothing. Verified against the fleet: `uphold check` and the reader it replaces return the same exit code and the same claim count in 68 of 71 repositories. The three that differ have a policy `config::load` refuses outright -- `command.before` on a `builtin` -- which the Python never validated and which `uphold scan` already fails on today. 35 behaviour tests move from tests/test_uphold_check.py to tests/check_cli.rs rather than going away, and two caught real regressions in the port: a lefthook remote given as a filesystem path, which is what scripts/consumer_check.sh writes, and the rule that a remote is only ours when ONE entry names this repository and takes its config. --- .pre-commit-hooks.yaml | 17 +- src/catalog.rs | 223 +++++++ src/check.rs | 748 +++++++++++++++++++++++ src/main.rs | 31 + tests/check_cli.rs | 689 ++++++++++++++++++++++ tests/test_review.py | 8 + tests/test_uphold_check.py | 968 +----------------------------- uphold_check.py | 1145 +++++------------------------------- 8 files changed, 1861 insertions(+), 1968 deletions(-) create mode 100644 src/catalog.rs create mode 100644 src/check.rs create mode 100644 tests/check_cli.rs diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index 04c8be9..49c9640 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -19,14 +19,25 @@ # claim into a false one. It does not run on every commit, and it carries no # principle text into any runtime: what it prints is the rule that went missing # and the file that says so. +# +# `language: rust` like every other id here, and it is the same environment. +# This was `language: script` running `uphold_check.py`, which had to +# re-implement `config::load` to know which rules resolve -- a second reader, +# free to disagree with the engine, which it did. The loader answers now. The +# script kept the catalog modes, which read no policy and so cannot disagree. +# +# `.cmd-shims/checks.enabled` left the trigger list with the tier: whether a +# shim is on PATH ahead of the real command is not written in any file a +# repository can be asked, so the reconcile reports that seam as one it could +# not establish rather than reading a file that does not settle it. - id: uphold-check name: uphold description: reconcile policy/upheld.toml against the rules this repo actually runs - entry: uphold_check.py - language: script + entry: uphold check + language: rust stages: [pre-commit, manual] pass_filenames: false - files: '^(policy/upheld\.toml|policy/principles\.toml|\.pre-commit-config\.yaml|lefthook\.yml|\.cmd-shims/checks\.enabled)$' + files: '^(policy/upheld\.toml|policy/principles\.toml|\.pre-commit-config\.yaml|lefthook\.yml)$' # ── the content policy ─────────────────────────────────────────────── # diff --git a/src/catalog.rs b/src/catalog.rs new file mode 100644 index 0000000..433f13f --- /dev/null +++ b/src/catalog.rs @@ -0,0 +1,223 @@ +//! The principle catalog, as the reconcile needs it. +//! +//! Compiled in rather than read off disk, for the reason the bundled rule sets +//! are: this binary runs in a CONSUMER's repository, which has a policy file +//! and a declaration and no copy of this catalog. A reader that went looking +//! for `principles/` beside the executable would find it only in the one +//! repository that does not need it. +//! +//! Four fields, and deliberately not the record. A reconcile asks whether a +//! claimed principle exists, whether it is deprecated, and whether its own +//! record says a machine can enforce it -- three questions with a yes or no +//! each. Everything else in a record is prose for a person building a rule, and +//! prose a tool holds is prose it has no condition on which to emit; see the +//! `enforcement-needs-a-trigger` record, which is in this catalog. `--explain` +//! reads the files themselves and stays where the prose is. + +use std::collections::BTreeMap; +use std::sync::OnceLock; + +use serde::Deserialize; + +use crate::error::{Fatal, Result}; + +/// Every record, as `include_str!` pairs of id and TOML source. +/// +/// Generated by no build script on purpose: a list a reader can see is a list a +/// reader can check against `ls principles/`, and `tests/test_catalog.py` +/// asserts the two agree, so a record added to the directory and not to this +/// list fails rather than going quietly missing from every reconcile. +const RECORDS: &[(&str, &str)] = &[ + ( + "backpressure", + include_str!("../principles/backpressure.toml"), + ), + ( + "complete-mediation", + include_str!("../principles/complete-mediation.toml"), + ), + ( + "defense-in-depth", + include_str!("../principles/defense-in-depth.toml"), + ), + ( + "end-to-end-principle", + include_str!("../principles/end-to-end-principle.toml"), + ), + ( + "enforcement-needs-a-trigger", + include_str!("../principles/enforcement-needs-a-trigger.toml"), + ), + ( + "explicit-unknown", + include_str!("../principles/explicit-unknown.toml"), + ), + ("fail-fast", include_str!("../principles/fail-fast.toml")), + ( + "fail-safe-defaults", + include_str!("../principles/fail-safe-defaults.toml"), + ), + ( + "graceful-degradation", + include_str!("../principles/graceful-degradation.toml"), + ), + ( + "high-cohesion-low-coupling", + include_str!("../principles/high-cohesion-low-coupling.toml"), + ), + ( + "idempotency", + include_str!("../principles/idempotency.toml"), + ), + ( + "information-hiding", + include_str!("../principles/information-hiding.toml"), + ), + ( + "informed-consent", + include_str!("../principles/informed-consent.toml"), + ), + ( + "least-astonishment", + include_str!("../principles/least-astonishment.toml"), + ), + ( + "least-privilege", + include_str!("../principles/least-privilege.toml"), + ), + ( + "make-illegal-states-unrepresentable", + include_str!("../principles/make-illegal-states-unrepresentable.toml"), + ), + ( + "mechanism-policy-separation", + include_str!("../principles/mechanism-policy-separation.toml"), + ), + ( + "observability", + include_str!("../principles/observability.toml"), + ), + ( + "parameterize-do-not-enumerate", + include_str!("../principles/parameterize-do-not-enumerate.toml"), + ), + ( + "psychological-acceptability", + include_str!("../principles/psychological-acceptability.toml"), + ), + ( + "reversible-decisions", + include_str!("../principles/reversible-decisions.toml"), + ), + ( + "separation-of-concerns", + include_str!("../principles/separation-of-concerns.toml"), + ), + ( + "single-authoritative-source", + include_str!("../principles/single-authoritative-source.toml"), + ), + ( + "unix-composability", + include_str!("../principles/unix-composability.toml"), + ), +]; + +/// What a reconcile asks of one record. +#[derive(Debug, Clone, Deserialize)] +pub(crate) struct Record { + pub id: String, + #[serde(default)] + pub status: String, + #[serde(default)] + pub enforcement: Enforcement, +} + +#[derive(Debug, Clone, Default, Deserialize)] +pub(crate) struct Enforcement { + /// `"yes"`, `"partial"`, or `"no"`. Absent is not `"no"`: a record that says + /// nothing has not refused, and treating silence as a refusal would fail a + /// claim on grounds nobody wrote. + #[serde(default)] + pub automatable: Option, +} + +impl Record { + pub(crate) fn deprecated(&self) -> bool { + self.status == "deprecated" + } + + /// Whether the record itself says no rule can enforce it. + pub(crate) fn refuses_automation(&self) -> bool { + self.automatable() == Some("no") + } + + pub(crate) fn automatable(&self) -> Option<&str> { + self.enforcement.automatable.as_deref() + } +} + +/// The catalog, parsed once. +/// +/// A record that does not parse is a defect in this binary and not in the tree +/// being checked -- the file is compiled in, and `validate` refuses a malformed +/// one at commit time. It is still exit 2 rather than a crash, because the +/// contract a caller reads is the exit code: a checker that dies is a checker +/// whose answer nobody has, and the state for that is could-not-look. See the +/// `explicit-unknown` record. +fn catalog() -> Result<&'static BTreeMap> { + static CATALOG: OnceLock, String>> = + OnceLock::new(); + CATALOG + .get_or_init(|| { + let mut catalog = BTreeMap::new(); + for (id, source) in RECORDS { + let record: Record = toml::from_str(source) + .map_err(|error| format!("bundled principle {id}: {error}"))?; + if &record.id != id { + return Err(format!( + "bundled principle {id} carries the id {:?}", + record.id + )); + } + catalog.insert(record.id.clone(), record); + } + Ok(catalog) + }) + .as_ref() + .map_err(|error| Fatal::new(error.clone())) +} + +pub(crate) fn get(id: &str) -> Result> { + Ok(catalog()?.get(id)) +} + +/// Every id, for a caller reporting one that is not there. +pub(crate) fn ids() -> Result> { + Ok(catalog()?.keys().map(String::as_str).collect()) +} + +/// The records a claim is ALLOWED to name -- the coverage denominator. +/// +/// Not the `automatable = "yes"` ones. Those are the records a reviewer is +/// chased about in `--review`; this is the wider set a declaration may draw +/// from, which is everything the reconcile would not refuse: not deprecated, +/// and not `automatable = "no"`. Measuring against the narrower set understated +/// the denominator and, worse, dropped from the numerator every principle a +/// rule really does enforce whose record is `partial`. +pub(crate) fn claimable_ids() -> Result> { + Ok(catalog()? + .values() + .filter(|record| !record.deprecated() && !record.refuses_automation()) + .map(|record| record.id.as_str()) + .collect()) +} + +/// Refuse a claim on an id the catalog does not define, in the words a person +/// can act on. +pub(crate) fn unknown(id: &str, known: usize) -> String { + format!( + "unknown principle id {id:?}; the catalog defines {known} of them, and \ + `uphold_check.py --list` prints every one" + ) +} diff --git a/src/check.rs b/src/check.rs new file mode 100644 index 0000000..fb9f23d --- /dev/null +++ b/src/check.rs @@ -0,0 +1,748 @@ +//! Reconcile `policy/upheld.toml` against the rules this repository runs. +//! +//! A claim is that a named rule is what enforces a named principle HERE: +//! +//! ```toml +//! [[enforce]] +//! principle = "explicit-unknown" +//! rule = "catalog-tests" +//! ``` +//! +//! It is falsifiable from this repository's own configuration -- the rule is +//! resolved and its seam is installed, or it is not -- and that is the only +//! thing checked. When it is false, the principle stopped being enforced while +//! the declaration went on saying it was. +//! +//! This lived in `uphold_check.py`, where answering it meant re-implementing +//! `config::load`: the bundled sets, `inherit.paths`, `inherit.disabled_rules`, +//! and a repository's own rule shadowing an inherited id. Five interacting +//! fields, read twice, by two programs free to disagree -- and they did, about +//! the seam a hookless rule runs at, which credited a checker standing in front +//! of `gh` to the file scan and reconciled a claim on it green in a repository +//! where nothing ran it. The loader answers now, and this asks. +//! +//! What stayed in Python is what never reads the policy: `--explain`, +//! `--list`, `--review`. Those read the catalog and render prose, so they +//! cannot disagree with the engine about which rules run. + +use std::collections::{BTreeMap, BTreeSet}; +use std::path::Path; + +use serde::Deserialize; + +use crate::catalog; +use crate::config::Policy; +use crate::error::{Exit, Fatal, Result}; + +const DECLARATION: &str = "policy/upheld.toml"; + +/// `owner/name` -- the form a consumer's runner configuration writes down. +/// +/// From `package.repository` at COMPILE time. The Python read the manifest off +/// disk beside itself, which works only in a checkout of this repository and is +/// a second copy of a value cargo already holds. This one cannot drift from the +/// crate it is compiled into. +fn upstream_slug() -> Option<(&'static str, &'static str)> { + let url = env!("CARGO_PKG_REPOSITORY") + .trim_end_matches('/') + .trim_end_matches(".git"); + let (rest, name) = url.rsplit_once('/')?; + let owner = rest.rsplit('/').next()?; + (!owner.is_empty() && !name.is_empty()).then_some((owner, name)) +} + +/// Does this url name THIS repository? +/// +/// `owner/name`, or the bare `name`. lefthook takes any git url and most carry +/// no owner: `scripts/consumer_check.sh` points its consumer at a clone by +/// FILESYSTEM PATH, and requiring the slug reported that consumer as running no +/// seam at all -- so the one CI job that drives a real lefthook consumer refused +/// a clean commit. +/// +/// The last path segment has to match exactly. A url ending `my-uphold-fork` is +/// not this repository, and a substring test said it was. +fn names_this_repository(url: &str) -> bool { + let trimmed = url.trim().trim_end_matches('/').trim_end_matches(".git"); + let Some((owner, name)) = upstream_slug() else { + return false; + }; + let Some((before, last)) = trimmed.rsplit_once('/') else { + // A bare name and nothing else. + return trimmed == name; + }; + // Exactly the last segment, never a substring: a url ending + // `my-uphold-fork` is not this repository, and a `contains` said it was. + if last != name { + return false; + } + // A url that names a HOST names an owner too, and a fork under another + // owner publishes the same name while being a different repository. A + // filesystem path names no owner at all, which is what + // `scripts/consumer_check.sh` writes -- so the segment before the name is + // asked to match only where there is a host for it to belong to. + let remote = trimmed.contains("://") || trimmed.contains('@'); + if !remote { + return true; + } + before.rsplit('/').next() == Some(owner) +} + +/// One `[[enforce]]` entry, as written. +#[derive(Debug, Deserialize)] +struct Claim { + principle: Option, + rule: Option, + /// Refused rather than ignored. `tier` said which namespace `rule` resolved + /// in, back when the seams were separate repositories. Ignoring a leftover + /// one silently reinterprets the claim; failing it as a false claim sends + /// the author looking for a rule that is present. + #[serde(default)] + tier: Option, +} + +#[derive(Debug, Deserialize)] +struct Declaration { + #[serde(default)] + enforce: Vec, +} + +/// Which seams of `uphold` this repository installs. +/// +/// Two answers and not one. A repository-wide "uphold runs here" let every rule +/// in the policy resolve against it, so one that pinned `uphold-scan` and no +/// guard id reconciled a claim on a `pre-push` guard whose stage nothing +/// installed. What is returned is what was installed: the file scan, and the +/// set of git stages some pinned id actually runs. +/// +/// Both runners are read and unioned rather than the first one winning. A +/// repository may drive the fast stages from pre-commit and the slow ones from +/// lefthook, and either file alone understates it. +#[derive(Debug, Default)] +pub(crate) struct Installed { + pub scan: bool, + pub stages: BTreeSet, + /// How it was established, so a reconcile that passes does not pass for a + /// reason the reader cannot see. + pub how: Vec, + /// Seams that could not be read. Never counted as absent: a rule missing + /// from what could be read is not a missing rule, which is exit 2 and not + /// exit 1. See the `explicit-unknown` record. + pub unreadable: Vec, + /// The `local` tier: every hook id installed here from any repository, plus + /// every lefthook command name. A claim may name a formatter, a linter, or + /// a hook this repository wrote, and those are rules that fire here. + pub local: BTreeSet, +} + +impl Installed { + fn nothing(&self) -> bool { + !self.scan && self.stages.is_empty() + } +} + +/// The published ids, and which stage each one installs. +/// +/// Read off this binary's own manifest at compile time rather than listed here. +/// A list here would be a literal describing a constant there, which is the +/// shape of the defect `_rule_stages` carries a paragraph about: the list was +/// short by one id and a claim on the rule it named reported enforcing nothing. +const MANIFEST: &str = include_str!("../.pre-commit-hooks.yaml"); + +#[derive(Debug, Deserialize)] +struct PublishedHook { + id: String, + #[serde(default)] + entry: String, + #[serde(default)] + stages: Vec, +} + +/// `(ids that run the scan, stage -> the id that installs it)`. +fn published() -> Result<(BTreeSet, BTreeMap)> { + let hooks: Vec = serde_yaml_ng::from_str(MANIFEST) + .map_err(|error| Fatal::new(format!(".pre-commit-hooks.yaml: {error}")))?; + + let mut scans = BTreeSet::new(); + let mut guards = BTreeMap::new(); + for hook in hooks { + let entry = hook.entry.split_whitespace().collect::>(); + match entry.as_slice() { + // `uphold scan` over the tree. `--text` reads a message on stdin + // and establishes nothing about the tree, so it is not the scan + // seam a content rule runs at. + [_, "scan", rest @ ..] if !rest.contains(&"--text") => { + scans.insert(hook.id); + } + [_, "guard", ..] => { + for stage in hook.stages { + guards.entry(stage).or_insert_with(|| hook.id.clone()); + } + } + _ => {} + } + } + Ok((scans, guards)) +} + +#[derive(Debug, Deserialize)] +struct PreCommitConfig { + #[serde(default)] + repos: Option>, +} + +#[derive(Debug, Deserialize)] +struct PreCommitRepo { + #[serde(default)] + repo: String, + #[serde(default)] + hooks: Vec, +} + +#[derive(Debug, Deserialize)] +struct PinnedHook { + #[serde(default)] + id: String, +} + +/// Hook ids from a pre-commit config: `(this repository's, every one).` +/// +/// Two sets, because they answer two questions. Which of THIS binary's seams +/// are installed is evidence only an id of this binary's can give -- a consumer +/// pinning some other repository's `uphold-scan` establishes nothing here. But +/// a claim may also name a rule that is not this binary's at all: a local hook, +/// or a formatter from a third-party repository, is a rule that fires here and +/// can be claimed as one. That is the `local` tier, and it is every id. +fn pinned_ids(root: &Path) -> Result, BTreeSet)>> { + let path = root.join(".pre-commit-config.yaml"); + if !path.is_file() { + return Ok(None); + } + let text = std::fs::read_to_string(&path).map_err(|error| Fatal::at(&path, error))?; + let config: PreCommitConfig = + serde_yaml_ng::from_str(&text).map_err(|error| Fatal::at(&path, error))?; + let Some(repos) = config.repos else { + return Err(Fatal::at( + &path, + "has no top-level `repos:` key, so which hooks it installs cannot be read. \ + Reporting no hooks would be an empty answer where the honest one is \ + could-not-look", + )); + }; + let mut ours = BTreeSet::new(); + let mut every = BTreeSet::new(); + for entry in repos { + let mine = names_this_repository(&entry.repo); + for hook in entry.hooks { + if mine { + ours.insert(hook.id.clone()); + } + every.insert(hook.id); + } + } + Ok(Some((ours, every))) +} + +#[derive(Debug, Deserialize)] +struct LefthookConfig { + #[serde(default)] + remotes: Vec, + #[serde(flatten)] + stages: BTreeMap, +} + +#[derive(Debug, Deserialize)] +struct LefthookRemote { + #[serde(default)] + git_url: String, + #[serde(default)] + configs: Vec, +} + +/// The git stages a lefthook config drives the binary at, directly. +/// +/// Parsed rather than line-scanned. The Python read this with a regex over +/// indentation and a stack of enclosing keys, which accepted `configs:` -- the +/// key under `remotes:` that README.md tells every consumer to write -- as a +/// command name, so a claim naming a rule called `configs` reconciled green +/// against a file defining no such thing. A parser cannot make that mistake. +fn lefthook_seams(root: &Path, guards: &BTreeMap) -> Result { + let mut found = Installed::default(); + let path = root.join("lefthook.yml"); + if !path.is_file() { + return Ok(found); + } + let text = std::fs::read_to_string(&path).map_err(|error| Fatal::at(&path, error))?; + let config: LefthookConfig = + serde_yaml_ng::from_str(&text).map_err(|error| Fatal::at(&path, error))?; + + let mut direct = false; + for (stage, body) in &config.stages { + // Only a name git knows is a stage; `remotes`, `colors` and the rest of + // lefthook's top-level keys are not. + if !guards.contains_key(stage.as_str()) { + continue; + } + for run in runs_in(body) { + let words: Vec<&str> = run.split_whitespace().collect(); + // The subcommand, and the word before it. Matched on the + // SUBCOMMAND and not the executable: this repository runs its own + // binary out of the tree with `cargo run -- scan`, a consumer runs + // `uphold scan` from PATH, and a third by absolute path. All three + // are the same seam, and a pattern anchored on the program name + // recognised only the middle one. + let Some((before, subcommand)) = words.windows(2).find_map(|pair| match pair { + [before, word @ ("scan" | "guard")] => Some((*before, *word)), + _ => None, + }) else { + continue; + }; + if !(before.ends_with("uphold") || before == "--") { + continue; + } + direct = true; + if subcommand == "scan" && !words.contains(&"--text") { + found.scan = true; + } else if subcommand == "guard" { + found.stages.insert(stage.clone()); + } + } + } + if direct { + found.how.push(String::from("lefthook.yml runs the binary")); + } + + let commands = lefthook_commands(&config, guards); + if !commands.is_empty() { + found.how.push(format!( + "lefthook.yml defines {} command(s)", + commands.len() + )); + } + found.local = commands; + + // BOTH halves, in the SAME entry. Checked separately, either alone was + // enough: a remote whose url merely resembled this repository, or one + // pulling a file that happens to be called `hooks/lefthook.yml` out of + // somebody else's. Either match granted every stage this manifest + // publishes, because the branch it feeds assumes the remote IS this + // repository's config -- so a fork, a mirror, or an unrelated project + // following the same conventional filename was credited with running every + // guard here. + if config.remotes.iter().any(|remote| { + names_this_repository(&remote.git_url) + && remote + .configs + .iter() + .any(|named| named.trim() == "hooks/lefthook.yml") + }) { + // The remote config is this repository's `hooks/lefthook.yml`, which + // wires every stage the manifest publishes. Including it is the one + // form that needs no per-stage reading. + found.scan = true; + found.stages.extend(guards.keys().cloned()); + found.how.push(String::from( + "lefthook.yml includes this repository as a remote", + )); + } + Ok(found) +} + +/// The command names a lefthook config defines, and nothing else. +/// +/// A command is a key under a stage's `commands:` mapping. The Python matched +/// on indentation with a stack of enclosing keys, which accepted `configs:` -- +/// the key under `remotes:` that README.md tells every consumer to write +/// verbatim -- as a command named `configs`, so a claim naming that rule +/// reconciled green against a file that defines no such thing. Reading the +/// mapping cannot make that mistake, because `configs` is not under a stage. +fn lefthook_commands( + config: &LefthookConfig, + guards: &BTreeMap, +) -> BTreeSet { + let mut names = BTreeSet::new(); + for (stage, body) in &config.stages { + if !guards.contains_key(stage.as_str()) { + continue; + } + let Some(commands) = body.get("commands").and_then(|value| value.as_mapping()) else { + continue; + }; + for key in commands.keys() { + if let Some(name) = key.as_str() { + names.insert(name.to_owned()); + } + } + } + names +} + +/// Every `run:` string under a lefthook stage, at any nesting. +fn runs_in(value: &serde_yaml_ng::Value) -> Vec { + let mut found = Vec::new(); + match value { + serde_yaml_ng::Value::Mapping(mapping) => { + for (key, nested) in mapping { + if key.as_str() == Some("run") { + if let Some(text) = nested.as_str() { + found.push(text.to_owned()); + } + } else { + found.extend(runs_in(nested)); + } + } + } + serde_yaml_ng::Value::Sequence(items) => { + for item in items { + found.extend(runs_in(item)); + } + } + _ => {} + } + found +} + +pub(crate) fn installed(root: &Path) -> Result { + let (scans, guards) = published()?; + let mut found = Installed::default(); + + match pinned_ids(root) { + Ok(Some((ours, every))) => { + found.scan = ours.iter().any(|id| scans.contains(id)); + for (stage, hook) in &guards { + if ours.contains(hook) { + found.stages.insert(stage.clone()); + } + } + found.local.extend(every); + let mut named: Vec<&str> = ours + .iter() + .filter(|id| scans.contains(*id) || guards.values().any(|hook| hook == *id)) + .map(String::as_str) + .collect(); + named.sort_unstable(); + if !named.is_empty() { + found + .how + .push(format!(".pre-commit-config.yaml pins {}", named.join(", "))); + } + } + Ok(None) => {} + // Present and unreadable is not absent. A config this cannot parse is + // exactly where the missing seam might be. + Err(error) => found.unreadable.push(error.to_string()), + } + + match lefthook_seams(root, &guards) { + Ok(lefthook) => { + found.scan = found.scan || lefthook.scan; + found.stages.extend(lefthook.stages); + found.how.extend(lefthook.how); + found.local.extend(lefthook.local); + } + Err(error) => found.unreadable.push(error.to_string()), + } + + if found.nothing() && found.how.is_empty() { + found.how.push(String::from( + "no runner configuration here runs `uphold scan` or `uphold guard`", + )); + } + Ok(found) +} + +/// Which seams supply each resolved rule, and what could not be established. +/// +/// `Rule::seams` is the loader's answer to where a rule runs; this asks whether +/// that place is installed here. A `shim` rule is the one seam no runner +/// configuration can settle -- whether the shim is on PATH ahead of the real +/// command is not written in any file this reads -- so it is reported as +/// unestablished rather than credited to whichever seam happens to be on. +pub(crate) fn suppliers(policy: &Policy, installed: &Installed) -> Supply { + let mut supplied: BTreeMap> = BTreeMap::new(); + let mut unestablished: Vec = Vec::new(); + + for name in &installed.local { + supplied + .entry(name.clone()) + .or_default() + .push(String::from("a hook installed here")); + } + + for rule in &policy.rules { + let mut by: Vec = Vec::new(); + for seam in rule.seams() { + match seam { + "scan" if installed.scan => by.push(String::from("uphold scan")), + "scan" => unestablished.push(format!("{} (file scan)", rule.id)), + "guard" => { + let live: Vec<&str> = rule + .hooks() + .iter() + .filter(|hook| installed.stages.contains(*hook)) + .map(String::as_str) + .collect(); + if live.is_empty() { + unestablished.push(format!("{} ({})", rule.id, rule.hooks().join(", "))); + } else { + by.push(format!("uphold guard at {}", live.join(", "))); + } + } + "shim" => { + unestablished.push(format!("{} (stands in front of a command)", rule.id)); + } + _ => {} + } + } + if !by.is_empty() { + supplied.entry(rule.id.clone()).or_default().extend(by); + } + } + Supply { + supplied, + unestablished, + } +} + +pub(crate) struct Supply { + pub supplied: BTreeMap>, + pub unestablished: Vec, +} + +/// `uphold check`, and `uphold check --coverage`. +pub(crate) fn run(root: &Path, policy: &Policy, coverage: bool) -> Result { + let path = root.join(DECLARATION); + if !path.is_file() { + return Err(Fatal::new(format!( + "{DECLARATION} not found under {}. Create one with: \ + uphold_check.py --init > {DECLARATION}", + root.display() + ))); + } + 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 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"), + )); + } + + let Some(record) = catalog::get(principle)? else { + failures.push(format!( + "{at}: {}", + catalog::unknown(principle, catalog::ids()?.len()) + )); + continue; + }; + if record.deprecated() { + failures.push(format!( + "{at}: {principle:?} is deprecated; the catalog keeps it for redirects only" + )); + continue; + } + if record.refuses_automation() { + failures.push(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!( + "{at}: no rule {rule:?} in what could be read, and {} \ + could not be read; cannot tell whether the claim holds", + installed.unreadable.join("; ") + ))); + } + + failures.push(format!( + "{at}: {principle:?} claims {rule:?}, which no seam here supplies" + )); + } + + if !failures.is_empty() { + eprintln!("enforcement claims refused ({DECLARATION}):"); + for failure in &failures { + eprintln!("- {failure}"); + } + return Ok(Exit::Violations); + } + + println!("reconciled {} enforcement claims:", evidence.len()); + for line in &evidence { + println!(" {line}"); + } + for note in &installed.how { + println!(" note {note}"); + } + Ok(Exit::Clean) +} + +/// The denominator the reconcile cannot see: rules that run and claim nothing. +/// +/// It reports and does not refuse. A mode that failed a build over an unclaimed +/// rule would be paid for in claims written to silence it, and deciding which +/// principle a rule serves is a judgment. Exit 2 only where something could not +/// be read, because a coverage number computed over a seam nobody could inspect +/// means less than it looks like it means. +fn report_coverage( + policy: &Policy, + declaration: &Declaration, + installed: &Installed, + supply: &Supply, +) -> Result { + let mut claims: BTreeMap<&str, Vec<&str>> = BTreeMap::new(); + for claim in &declaration.enforce { + if let (Some(principle), Some(rule)) = (claim.principle.as_deref(), claim.rule.as_deref()) { + claims.entry(rule).or_default().push(principle); + } + } + + let mut lines: Vec = Vec::new(); + + // The engine's own rules, and how many of them a claim names. + let mut carrying = 0_usize; + let mut unclaimed: Vec<&str> = Vec::new(); + for rule in &policy.rules { + if !supply.supplied.contains_key(&rule.id) { + continue; + } + match claims.get(rule.id.as_str()) { + Some(principles) => { + carrying += 1; + for principle in principles { + lines.push(format!(" {} -> {principle}", rule.id)); + } + } + None => unclaimed.push(&rule.id), + } + } + let supplied = carrying + unclaimed.len(); + println!("uphold: {carrying} of {supplied} rules carry a principle"); + for line in &lines { + println!("{line}"); + } + for rule in &unclaimed { + println!(" unclaimed {rule}"); + } + + // The local tier: hooks and commands this repository installs, which a + // claim may name and the engine does not own. `?` and not 0 where it could + // not be read -- a hole reported as zero reads as coverage nobody measured. + if installed.unreadable.is_empty() { + let claimed_local = installed + .local + .iter() + .filter(|name| claims.contains_key(name.as_str())) + .count(); + println!( + "local: {claimed_local} of {} rules carry a principle", + installed.local.len() + ); + for name in &installed.local { + if !claims.contains_key(name.as_str()) { + println!(" unclaimed {name}"); + } + } + } else { + println!("local: 0 of ? rules carry a principle"); + for note in &installed.unreadable { + println!(" could not look {note}"); + } + } + + for note in &installed.how { + println!(" note {note}"); + } + + // Claims naming a rule nothing here supplies. Reported, not refused -- + // `uphold check` is where that is a failure. + let orphans: Vec<&str> = claims + .keys() + .filter(|rule| !supply.supplied.contains_key(**rule)) + .copied() + .collect(); + if !orphans.is_empty() { + println!( + " claimed but supplied by nothing here: {}", + orphans.join(", ") + ); + } + if !supply.unestablished.is_empty() { + println!( + " declared, but no runner configuration here installs the seam it fires at: {}", + supply.unestablished.join(", ") + ); + } + + // The one number a reader takes away, computed from what a seam SUPPLIES + // and not from what the declaration says. Counting the claims instead + // reported a record as claimed by a rule here two lines under the line + // saying that rule is supplied by nothing. + let claimable = catalog::claimable_ids()?; + let held: BTreeSet<&str> = declaration + .enforce + .iter() + .filter(|claim| { + claim + .rule + .as_deref() + .is_some_and(|rule| supply.supplied.contains_key(rule)) + }) + .filter_map(|claim| claim.principle.as_deref()) + .collect(); + let counted = claimable.iter().filter(|id| held.contains(**id)).count(); + println!( + "records: {counted} of {} claimable records are claimed by a rule here", + claimable.len() + ); + + if installed.unreadable.is_empty() { + Ok(Exit::Clean) + } else { + Ok(Exit::Broken) + } +} diff --git a/src/main.rs b/src/main.rs index e3e3d9a..2ce8b3b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,6 +22,8 @@ )] mod audit; +mod catalog; +mod check; mod config; mod engine; mod error; @@ -47,6 +49,8 @@ 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 --coverage which rules run here and carry no principle uphold audit --for-publication what a private->public flip would republish uphold rules --set NAME what a bundled rule set refuses, rule by rule uphold rules --effective [--json] every rule this repository resolves to, and @@ -206,6 +210,14 @@ fn run() -> Result { }; match text_of(first)? { + // The url `package.repository` holds, for the OSCAL export's property + // namespace. Asked of the binary because the binary is where that value + // is compiled in; a second copy read off `Cargo.toml` is a copy that + // drifts, and it can only be read in a checkout of this repository. + "--upstream" => { + println!("{}", env!("CARGO_PKG_REPOSITORY")); + Ok(Exit::Clean) + } "--version" | "-V" => { println!("uphold {}", env!("CARGO_PKG_VERSION")); Ok(Exit::Clean) @@ -217,6 +229,13 @@ fn run() -> Result { "scan" => scan_command(rest), "guard" => guard_command(rest), "audit" => audit_command(rest), + "check" => match rest { + [] => check_command(false), + [flag] if flag == "--coverage" => check_command(true), + _ => Err(Fatal::new(format!( + "usage: uphold check [--coverage]\n\n{USAGE}" + ))), + }, "rules" => match rest { [flag, name] if flag == "--set" => rules_command(text_of(name)?), [flag] if flag == "--effective" => effective_rules_command(false), @@ -239,6 +258,18 @@ fn run() -> Result { } } +/// Reconcile the declaration against the rules this repository resolves to. +/// +/// 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 { + 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) +} + fn scan_command(arguments: &[OsString]) -> Result { let mut explicit_policy: Option = None; let mut text_source: Option = None; diff --git a/tests/check_cli.rs b/tests/check_cli.rs new file mode 100644 index 0000000..dee54d6 --- /dev/null +++ b/tests/check_cli.rs @@ -0,0 +1,689 @@ +//! CLI-level tests for `uphold check`. +//! +//! Ported from `tests/test_uphold_check.py`, where the reconcile lived until +//! the loader took it over. At the CLI and not the function boundary, for the +//! reason the exit-code contract IS the interface: 0 clean, 1 refused, 2 could +//! not look. A test reaching inside would pass on a change to all three. + +#![expect( + clippy::let_underscore_must_use, + clippy::tests_outside_test_module, + clippy::unwrap_used, + reason = "A CLI test asserts on the outcome; a panic in the harness that builds the fixture IS the failure report, and there is no caller to hand a Result to" +)] + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; +use std::sync::atomic::{AtomicUsize, Ordering}; + +fn workspace() -> PathBuf { + static NEXT: AtomicUsize = AtomicUsize::new(0); + let path = std::env::temp_dir().join(format!( + "uphold-check-{}-{}", + std::process::id(), + NEXT.fetch_add(1, Ordering::Relaxed) + )); + let _ = std::fs::remove_dir_all(&path); + std::fs::create_dir_all(path.join("policy")).unwrap(); + path +} + +fn write(root: &Path, relative: &str, contents: &str) { + let path = root.join(relative); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(path, contents).unwrap(); +} + +fn check(root: &Path, args: &[&str]) -> Output { + Command::new(env!("CARGO_BIN_EXE_uphold")) + .arg("check") + .args(args) + .current_dir(root) + .output() + .unwrap() +} + +fn code(output: &Output) -> i32 { + output.status.code().unwrap() +} + +fn stderr(output: &Output) -> String { + String::from_utf8_lossy(&output.stderr).into_owned() +} + +fn stdout(output: &Output) -> String { + String::from_utf8_lossy(&output.stdout).into_owned() +} + +/// A consumer pinning the ids the fixtures below rely on. One id installs +/// exactly one git stage, so pinning `uphold-scan` and `uphold-guard-push` is +/// evidence about the file scan and about pre-push and about nothing else. +const PRE_COMMIT: &str = "\ +repos: + - repo: https://github.com/HackingGate/uphold + rev: v2.0.0 + hooks: + - id: uphold-scan + - id: uphold-guard-commit-msg + - id: uphold-guard-push + - repo: local + hooks: + - id: my-own-check + name: my own check +"; + +const SCAN_ONLY: &str = "\ +repos: + - repo: https://github.com/HackingGate/uphold + rev: v2.0.0 + hooks: + - id: uphold-scan +"; + +const GUARD_POLICY: &str = "\ +[rule.prevent-public-push] +builtin = \"prevent-public-push\" +git.hooks = [\"pre-push\"] +"; + +// ── the exit-code contract ─────────────────────────────────────────── + +#[test] +fn a_missing_declaration_is_two_not_zero() { + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + let output = check(&root, &[]); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains("upheld.toml"), + "{}", + stderr(&output) + ); +} + +#[test] +fn an_unreadable_declaration_is_two_not_one() { + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write( + &root, + "policy/upheld.toml", + "[[enforce]] this is not toml\n", + ); + assert_eq!(code(&check(&root, &[])), 2); +} + +#[test] +fn an_empty_declaration_is_zero() { + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write(&root, "policy/upheld.toml", "# nothing claimed yet\n"); + write(&root, ".pre-commit-config.yaml", PRE_COMMIT); + let output = check(&root, &[]); + assert_eq!(code(&output), 0, "{}", stderr(&output)); +} + +#[test] +fn a_leftover_tier_field_is_two_not_one() { + // A declaration written for the old schema is unreadable, not false. `tier` + // said which namespace `rule` resolved in; ignoring one silently + // reinterprets the claim, and failing it sends the author looking for a + // rule that is present. + 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\"\ntier = \"local\"\nrule = \"my-own-check\"\n", + ); + let output = check(&root, &[]); + assert_eq!(code(&output), 2, "{}", stderr(&output)); + assert!( + stderr(&output).contains("The field is gone"), + "{}", + stderr(&output) + ); +} + +#[test] +fn a_declaration_that_is_not_utf8_is_two_and_not_a_crash() { + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + std::fs::write( + root.join("policy/upheld.toml"), + b"[[enforce]]\nprinciple = \"\xff\"\n", + ) + .unwrap(); + assert_eq!(code(&check(&root, &[])), 2); +} + +#[test] +fn a_policy_file_that_is_not_utf8_is_two_and_not_a_crash() { + let root = workspace(); + std::fs::write( + root.join("policy/principles.toml"), + b"[rule.prevent-public-push]\nmessage = \"\xff\"\n", + ) + .unwrap(); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"prevent-public-push\"\n", + ); + assert_eq!(code(&check(&root, &[])), 2); +} + +// ── reconciling one claim ──────────────────────────────────────────── + +#[test] +fn an_installed_guard_reconciles() { + 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", + ); + let output = check(&root, &[]); + assert_eq!(code(&output), 0, "{}", stderr(&output)); + assert!( + stdout(&output).contains("fail-safe-defaults <- prevent-public-push"), + "{}", + stdout(&output) + ); +} + +#[test] +fn a_guard_claim_fails_when_the_stage_it_fires_at_is_not_installed() { + // The whole reason the answer is per-stage and not per-repository. This + // pins the file scan and no guard id at all, so the pre-push guard runs + // nowhere -- and a repository-wide "uphold runs here" reconciled it green. + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write(&root, ".pre-commit-config.yaml", SCAN_ONLY); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"prevent-public-push\"\n", + ); + let output = check(&root, &[]); + assert_eq!(code(&output), 1, "{}", stdout(&output)); + assert!( + stderr(&output).contains("no seam here supplies"), + "{}", + stderr(&output) + ); +} + +#[test] +fn an_unknown_principle_is_refused() { + 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 = \"no-such-principle\"\nrule = \"prevent-public-push\"\n", + ); + let output = check(&root, &[]); + assert_eq!(code(&output), 1, "{}", stdout(&output)); + assert!( + stderr(&output).contains("unknown principle id"), + "{}", + stderr(&output) + ); +} + +#[test] +fn a_rule_the_policy_disables_is_refused() { + // `inherit.disabled_rules` is one of the five fields that decide what runs, + // and the reason the reconcile cannot read `[rule.*]` tables alone. + let root = workspace(); + write( + &root, + "policy/principles.toml", + "[inherit]\nsets = [\"process-residue\"]\ndisabled_rules = [\"no-task-tracker-references\"]\n", + ); + write(&root, ".pre-commit-config.yaml", PRE_COMMIT); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"single-authoritative-source\"\nrule = \"no-task-tracker-references\"\n", + ); + let output = check(&root, &[]); + assert_eq!(code(&output), 1, "{}", stdout(&output)); +} + +#[test] +fn a_rule_from_a_bundled_base_set_is_supplied() { + let root = workspace(); + write( + &root, + "policy/principles.toml", + "[inherit]\nsets = [\"process-residue\"]\n", + ); + write(&root, ".pre-commit-config.yaml", PRE_COMMIT); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"single-authoritative-source\"\nrule = \"no-merge-conflict-markers\"\n", + ); + let output = check(&root, &[]); + assert_eq!(code(&output), 0, "{}", stderr(&output)); +} + +#[test] +fn a_rule_inherited_through_inherit_paths_is_supplied() { + let root = workspace(); + write( + &root, + "policy/principles.toml", + "[inherit]\npaths = [\"policy/extra.toml\"]\n", + ); + write( + &root, + "policy/extra.toml", + "[rule.from-a-path]\nmessage = \"no\"\nregexp = 'nothing-matches-this'\nfiles.include = [\".\"]\n", + ); + write(&root, ".pre-commit-config.yaml", PRE_COMMIT); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"single-authoritative-source\"\nrule = \"from-a-path\"\n", + ); + let output = check(&root, &[]); + assert_eq!(code(&output), 0, "{}", stderr(&output)); +} + +#[test] +fn inherit_paths_naming_a_file_that_is_not_there_is_two_not_one() { + let root = workspace(); + write( + &root, + "policy/principles.toml", + "[inherit]\npaths = [\"policy/gone.toml\"]\n", + ); + write(&root, ".pre-commit-config.yaml", PRE_COMMIT); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"single-authoritative-source\"\nrule = \"no-merge-conflict-markers\"\n", + ); + assert_eq!(code(&check(&root, &[])), 2); +} + +#[test] +fn a_rule_enforced_at_two_seams_reports_both() { + // A rule that both searches the tree and fires at a hook is the ordinary + // case, not an ambiguity, and every seam that supplies it is reported. + let root = workspace(); + write( + &root, + "policy/principles.toml", + "[rule.prevent-unusual-unicode-in-files]\nbuiltin = \"prevent-unusual-unicode-in-files\"\ngit.hooks = [\"pre-push\"]\n", + ); + write(&root, ".pre-commit-config.yaml", PRE_COMMIT); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"complete-mediation\"\nrule = \"prevent-unusual-unicode-in-files\"\n", + ); + let output = check(&root, &[]); + assert_eq!(code(&output), 0, "{}", stderr(&output)); + assert!(stdout(&output).contains("pre-push"), "{}", stdout(&output)); +} + +#[test] +fn a_claim_on_a_shim_only_rule_is_refused_and_not_credited_to_the_scan() { + // The seam an empty hook list could not express. `command.before` runs when + // the shim is on PATH ahead of the real command, which no runner + // configuration settles -- so it is not the file scan, and reading it as + // one reconciled this claim green over a rule nothing runs. + let root = workspace(); + write( + &root, + "policy/principles.toml", + "[[shim]]\ncommand = \"gh\"\nmatch = [\"pr:create\"]\ntext_flags = [\"-b\", \"--body\"]\n\n\ + [rule.no-published-markers]\nmessage = \"do not publish that\"\n\ + exec = \"uphold guard --text -\"\ncommand.before = [\"gh\"]\n", + ); + write(&root, ".pre-commit-config.yaml", SCAN_ONLY); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"complete-mediation\"\nrule = \"no-published-markers\"\n", + ); + let output = check(&root, &[]); + assert_eq!(code(&output), 1, "{}{}", stdout(&output), stderr(&output)); + + let coverage = check(&root, &["--coverage"]); + assert!( + stdout(&coverage).contains("stands in front of a command"), + "{}", + stdout(&coverage) + ); +} + +#[test] +fn the_seam_is_found_by_a_published_id_not_by_one_repositorys_name() { + // A consumer pinning some OTHER repository's `uphold-scan` establishes + // nothing about this binary's seams. + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write( + &root, + ".pre-commit-config.yaml", + "repos:\n - repo: https://github.com/somebody-else/uphold\n rev: v1.0.0\n hooks:\n - id: uphold-guard-push\n", + ); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"prevent-public-push\"\n", + ); + assert_eq!(code(&check(&root, &[])), 1); +} + +#[test] +fn a_lefthook_consumer_reconciles_with_no_pre_commit_config() { + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write( + &root, + "lefthook.yml", + "remotes:\n - git_url: https://github.com/HackingGate/uphold\n ref: v1.0.0\n configs:\n - hooks/lefthook.yml\n", + ); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"prevent-public-push\"\n", + ); + let output = check(&root, &[]); + assert_eq!(code(&output), 0, "{}", stderr(&output)); +} + +#[test] +fn a_lefthook_remote_by_filesystem_path_is_still_ours() { + // lefthook takes any git url and most carry no `owner/name`. + // `scripts/consumer_check.sh` points its consumer at a clone by PATH, and + // requiring the slug reported that consumer as running no seam at all. + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write( + &root, + "lefthook.yml", + "remotes:\n - git_url: /srv/example/uphold\n ref: v1.0.0\n configs:\n - hooks/lefthook.yml\n", + ); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"prevent-public-push\"\n", + ); + let output = check(&root, &[]); + assert_eq!(code(&output), 0, "{}", stderr(&output)); +} + +#[test] +fn a_remote_is_only_ours_when_one_entry_says_both() { + // Either half alone granted every stage this manifest publishes, because + // the branch it feeds assumes the remote IS this repository's config. A + // fork, a mirror, or an unrelated project using the same conventional + // filename was credited with running every guard here. + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write( + &root, + "lefthook.yml", + "remotes:\n - git_url: https://github.com/somebody-else/thing\n configs:\n - hooks/lefthook.yml\n \ + - git_url: https://github.com/HackingGate/uphold\n configs:\n - some/other.yml\n", + ); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"prevent-public-push\"\n", + ); + assert_eq!(code(&check(&root, &[])), 1); +} + +#[test] +fn a_lefthook_key_that_is_not_a_command_is_not_a_rule() { + // `configs:` is the key README.md tells every lefthook consumer to write + // under `remotes:`, at exactly the indent a command name sits at. A scan + // keyed on indentation read it as a command, so a claim naming a rule + // called `configs` reconciled green against a file defining no such thing. + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write( + &root, + "lefthook.yml", + "remotes:\n - git_url: https://github.com/HackingGate/uphold\n configs:\n - hooks/lefthook.yml\n", + ); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"explicit-unknown\"\nrule = \"configs\"\n", + ); + assert_eq!(code(&check(&root, &[])), 1); +} + +#[test] +fn a_lefthook_command_is_a_rule_a_claim_may_name() { + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write( + &root, + "lefthook.yml", + "pre-commit:\n commands:\n my-own-check:\n run: ./check.sh\n", + ); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"explicit-unknown\"\nrule = \"my-own-check\"\n", + ); + let output = check(&root, &[]); + assert_eq!(code(&output), 0, "{}", stderr(&output)); +} + +#[test] +fn an_uninstalled_hook_is_refused() { + 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 = \"a-hook-nobody-installed\"\n", + ); + assert_eq!(code(&check(&root, &[])), 1); +} + +// ── coverage ───────────────────────────────────────────────────────── + +const COVERAGE_POLICY: &str = "\ +[rule.prevent-public-push] +builtin = \"prevent-public-push\" +git.hooks = [\"pre-push\"] + +[rule.prevent-ai-author] +builtin = \"prevent-ai-author\" +git.hooks = [\"commit-msg\"] +"; + +fn coverage_fixture() -> PathBuf { + let root = workspace(); + write(&root, "policy/principles.toml", COVERAGE_POLICY); + write(&root, ".pre-commit-config.yaml", PRE_COMMIT); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"prevent-public-push\"\n", + ); + root +} + +#[test] +fn a_claimed_rule_is_reported_against_its_principle() { + let root = coverage_fixture(); + let output = check(&root, &["--coverage"]); + assert_eq!(code(&output), 0, "{}", stderr(&output)); + assert!( + stdout(&output).contains("prevent-public-push -> fail-safe-defaults"), + "{}", + stdout(&output) + ); + assert!( + stdout(&output).contains("uphold: 1 of 2"), + "{}", + stdout(&output) + ); +} + +#[test] +fn a_rule_no_claim_names_is_reported_as_unclaimed() { + let root = coverage_fixture(); + let output = check(&root, &["--coverage"]); + assert!( + stdout(&output).contains("unclaimed prevent-ai-author"), + "{}", + stdout(&output) + ); + assert!( + stdout(&output).contains("my-own-check"), + "{}", + stdout(&output) + ); +} + +#[test] +fn a_false_claim_is_reported_rather_than_refused() { + let root = workspace(); + write(&root, "policy/principles.toml", COVERAGE_POLICY); + write(&root, ".pre-commit-config.yaml", PRE_COMMIT); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"no-such-hook\"\n", + ); + assert_eq!(code(&check(&root, &[])), 1); + + let coverage = check(&root, &["--coverage"]); + assert_eq!(code(&coverage), 0, "{}", stderr(&coverage)); + assert!( + stdout(&coverage).contains("claimed but supplied by nothing here: no-such-hook"), + "{}", + stdout(&coverage) + ); +} + +#[test] +fn an_orphan_claim_is_not_counted_in_the_number_it_was_reported_under() { + // `records: N of M` is the one number a reader takes away, and it was + // computed from the claims rather than from what a seam supplies -- so a + // declaration whose only claim names a rule nothing runs reported one + // record as claimed, two lines under the line saying it is supplied by + // nothing. + let root = workspace(); + write(&root, "policy/principles.toml", COVERAGE_POLICY); + write(&root, ".pre-commit-config.yaml", PRE_COMMIT); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"no-such-hook\"\n", + ); + let output = check(&root, &["--coverage"]); + assert!( + stdout(&output).contains("records: 0 of"), + "{}", + stdout(&output) + ); +} + +#[test] +fn a_seam_that_could_not_be_read_is_not_a_seam_running_nothing() { + // The count is `?`, not 0, and the exit is 2: a hole in the denominator + // reported as zero reads as coverage that was never measured. + let root = workspace(); + write(&root, "policy/principles.toml", COVERAGE_POLICY); + write( + &root, + ".pre-commit-config.yaml", + "this: is not a repos list\n", + ); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"prevent-public-push\"\n", + ); + let output = check(&root, &["--coverage"]); + assert_eq!(code(&output), 2, "{}", stdout(&output)); + assert!( + stdout(&output).contains("local: 0 of ?"), + "{}", + stdout(&output) + ); +} + +#[test] +fn it_counts_records_against_what_can_be_claimed() { + let root = coverage_fixture(); + let output = check(&root, &["--coverage"]); + assert!( + stdout(&output).contains("claimable records are claimed by a rule here"), + "{}", + stdout(&output) + ); +} + +// ── this repository ────────────────────────────────────────────────── + +#[test] +fn this_repository_reconciles() { + let root = Path::new(env!("CARGO_MANIFEST_DIR")); + let output = check(root, &[]); + assert_eq!(code(&output), 0, "{}{}", stdout(&output), stderr(&output)); +} + +#[test] +fn the_starter_declaration_is_valid_and_enforces_nothing() { + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write(&root, ".pre-commit-config.yaml", PRE_COMMIT); + let starter = Command::new("python3") + .args([ + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("uphold_check.py") + .to_str() + .unwrap(), + "--init", + ]) + .output() + .unwrap(); + assert_eq!(starter.status.code().unwrap(), 0); + write(&root, "policy/upheld.toml", &stdout(&starter)); + let output = check(&root, &[]); + assert_eq!(code(&output), 0, "{}{}", stdout(&output), stderr(&output)); +} + +#[test] +fn the_output_carries_no_record_prose() { + // A tool holding prose has no condition on which to emit it; see the + // `enforcement-needs-a-trigger` record. What this prints is the rule that + // went missing and the file that says so. + let root = Path::new(env!("CARGO_MANIFEST_DIR")); + let run = check(root, &[]); + let mut text = stdout(&run); + text.push_str(&stderr(&run)); + for prose in [ + "Any verification, evaluation, or measurement", + "review_questions", + "does not mean", + ] { + assert!( + !text.contains(prose), + "record prose reached the output: {prose}" + ); + } +} diff --git a/tests/test_review.py b/tests/test_review.py index 167d833..184c07e 100644 --- a/tests/test_review.py +++ b/tests/test_review.py @@ -224,6 +224,14 @@ def review(self, body: str, *args: str) -> subprocess.CompletedProcess: policy = Path(self.tmp) / "policy" policy.mkdir(exist_ok=True) (policy / "upheld.toml").write_text(textwrap.dedent(body), encoding="utf-8") + # `--review` asks the binary which claims are live, and the binary needs + # a policy to answer. A repository with no policy is could-not-look here + # for the same reason it is for the reconcile. + (policy / "principles.toml").write_text( + '[rule.prevent-public-push]\nbuiltin = "prevent-public-push"\n' + 'git.hooks = ["pre-push"]\n', + encoding="utf-8", + ) return subprocess.run( [sys.executable, str(SCRIPT), "--review", *args], cwd=self.tmp, diff --git a/tests/test_uphold_check.py b/tests/test_uphold_check.py index cd4787b..41becde 100644 --- a/tests/test_uphold_check.py +++ b/tests/test_uphold_check.py @@ -12,7 +12,6 @@ import sys import tempfile import textwrap -import tomllib import unittest from pathlib import Path @@ -118,645 +117,14 @@ def build(directory: Path, declaration: str, **files: str) -> None: path.write_text(body, encoding="utf-8") -class ExitCodeContract(unittest.TestCase): - """`explicit-unknown`: could-not-look must not be reported as clean.""" - - def test_missing_declaration_is_two_not_zero(self): - with tempfile.TemporaryDirectory() as tmp: - result = run(Path(tmp)) - self.assertEqual(result.returncode, 2, result.stderr) - self.assertIn("could not look", result.stderr) - - def test_unreadable_declaration_is_two_not_one(self): - with tempfile.TemporaryDirectory() as tmp: - build(Path(tmp), "[[enforce]] this is not toml\n") - result = run(Path(tmp)) - self.assertEqual(result.returncode, 2, result.stderr) - - def test_a_leftover_tier_field_is_two_not_one(self): - """A declaration written for the old schema is unreadable, not false. - - `tier` said which namespace `rule` resolved in. Ignoring a leftover one - would silently reinterpret the claim, and refusing it as a false claim - would send the author looking for a rule that is present. - """ - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "explicit-unknown" - tier = "local" - rule = "my-own-check" - """, - **{ - ".pre-commit-config.yaml": PRE_COMMIT_WITH_PRINCIPLES, - "policy__principles.toml": "", - }, - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 2, result.stderr) - self.assertIn("The field is gone", result.stderr) - - def test_absent_tier_config_is_two_not_one(self): - """No .pre-commit-config.yaml means the claim is unverifiable, not false.""" - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "fail-safe-defaults" - rule = "prevent-public-push" - """, - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 2, result.stderr) - self.assertIn("could not look", result.stderr) - - def test_a_declaration_that_is_not_utf8_is_two_not_a_traceback(self): - """One byte that is not UTF-8 is a file this tool could not read. - - `UnicodeDecodeError` derives from `ValueError` and not from `OSError`, - so it escaped the handler that catches an unreadable file and left the - process on a traceback and exit 1 -- and exit 1 in this tool means a - claim is false. A repository whose declaration is mis-encoded was - reported as a repository that lies about what it enforces. - """ - with tempfile.TemporaryDirectory() as tmp: - build(Path(tmp), "# placeholder\n") - (Path(tmp) / "policy" / "upheld.toml").write_bytes( - b'[[enforce]]\nprinciple = "\xff"\n' - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 2, result.stdout) - self.assertIn("could not look", result.stderr) - - def test_a_policy_file_that_is_not_utf8_is_two_not_a_traceback(self): - """The same byte in the file the claim is reconciled against.""" - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "fail-safe-defaults" - rule = "prevent-public-push" - """, - **{".pre-commit-config.yaml": PRE_COMMIT_WITH_PRINCIPLES}, - ) - (Path(tmp) / "policy" / "principles.toml").write_bytes( - b'[rule.prevent-public-push]\nmessage = "\xff"\n' - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 2, result.stdout) - self.assertIn("could not look", result.stderr) - - def test_empty_declaration_is_zero(self): - with tempfile.TemporaryDirectory() as tmp: - build(Path(tmp), "# nothing enforced yet\n") - result = run(Path(tmp)) - self.assertEqual(result.returncode, 0, result.stderr) - - -class Reconciliation(unittest.TestCase): - def test_installed_hook_reconciles(self): - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "fail-safe-defaults" - rule = "prevent-public-push" - """, - **{ - ".pre-commit-config.yaml": PRE_COMMIT_WITH_PRINCIPLES, - "policy__principles.toml": GUARD_POLICY, - }, - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 0, result.stderr) - self.assertIn("enforced by uphold", result.stdout) - - def test_a_lefthook_consumer_reconciles_with_no_pre_commit_config(self): - """A repository that runs lefthook has no .pre-commit-config.yaml. - - That absence used to be could-not-look, so every lefthook consumer - exited 2 on a declaration their own config could answer -- and the - answer was not "unknown", it was in the file the script declined to - look for. A remote naming this repository IS the installation. - """ - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "fail-safe-defaults" - rule = "prevent-public-push" - """, - **{ - "lefthook.yml": LEFTHOOK_WITH_PRINCIPLES, - "policy__principles.toml": GUARD_POLICY, - }, - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 0, result.stderr) - self.assertIn("enforced by uphold", result.stdout) - - def test_the_seam_is_found_by_a_published_id_not_by_one_repositorys_name(self): - """Every published guard id has to make its own stage visible. - - The predicate was a single literal hook name that only this repository - used, so a consumer pinning the ids this repository publishes was told - the seam supplying every guard was absent. The manifest is the list of - ids now, so a new id cannot be added there and forgotten here -- and the - stage is read from the same manifest, so the pair - `pre-push -> uphold-guard-push` cannot drift either. - """ - scans, guards = uphold_check.published_seams() - self.assertIn("uphold-scan", scans) - self.assertEqual( - guards.get("pre-push"), - "uphold-guard-push", - f"the published guard ids are {guards}", - ) - for stage, hook_id in sorted(guards.items()): - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "fail-safe-defaults" - rule = "prevent-public-push" - """, - **{ - ".pre-commit-config.yaml": ( - "repos:\n" - " - repo: https://github.com/HackingGate/uphold\n" - " rev: v2.0.0\n" - " hooks:\n" - f" - id: {hook_id}\n" - ), - "policy__principles.toml": ( - "[rule.prevent-public-push]\n" - 'builtin = "prevent-public-push"\n' - f'git.hooks = ["{stage}"]\n' - ), - }, - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 0, f"{hook_id}: {result.stderr}") - self.assertIn("enforced by uphold", result.stdout, hook_id) - - def test_the_reconciler_s_own_id_is_not_evidence_that_a_rule_runs(self): - """`uphold-check` runs this script, which enforces nothing. - - While every id in the manifest counted as evidence, a repository that - pinned the reconciler and nothing else was accepted as proof that every - content rule and every guard fires here -- the reconciler certifying - itself, and printing "reconciled 1 enforcement claims" over a repository - running no rule at all. - """ - self.assertNotIn("uphold-check", uphold_check.published_hook_ids()) - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "fail-safe-defaults" - rule = "prevent-public-push" - """, - **{ - ".pre-commit-config.yaml": ( - "repos:\n" - " - repo: https://github.com/HackingGate/uphold\n" - " rev: v2.0.0\n" - " hooks:\n" - " - id: uphold-check\n" - ), - "policy__principles.toml": GUARD_POLICY, - }, - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 1, result.stdout) - self.assertIn("no seam here supplies", result.stderr) - - def test_a_guard_claim_fails_when_the_stage_it_fires_at_is_not_installed(self): - """A guard id installs one stage, and `uphold-scan` installs none. - - The seam was one repository-wide yes/no, so a repository that pinned - `uphold-scan` -- the file scan, which runs no guard -- reconciled a - claim on a rule declaring `git.hooks = ["pre-push"]`. The rule ran - nowhere: what installs it is `uphold-guard-push`, which nothing here - pinned. - """ - for hooks, expected in (("uphold-scan", 1), ("uphold-guard-push", 0)): - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "fail-safe-defaults" - rule = "prevent-public-push" - """, - **{ - ".pre-commit-config.yaml": ( - "repos:\n" - " - repo: https://github.com/HackingGate/uphold\n" - " rev: v2.0.0\n" - " hooks:\n" - f" - id: {hooks}\n" - ), - "policy__principles.toml": GUARD_POLICY, - }, - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, expected, f"{hooks}: {result.stderr}") - - def test_a_rule_inherited_through_inherit_paths_is_supplied(self): - """`[inherit]` has three fields and the reader used to see one. - - `inherit.paths` names the repository's own extra policy files, which - `config::load` merges exactly as it merges the bundled sets. Reading - only `inherit.sets` made every rule arriving that way invisible, so a - claim on one was refused as supplied by nothing while the engine was - running it -- and the action a person takes on that answer is to delete - a claim that was true. - """ - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "single-authoritative-source" - rule = "no-merge-conflict-markers" - """, - **{ - ".pre-commit-config.yaml": LOCAL_CONTENT_POLICY, - "policy__extra.toml": HYGIENE_BASE, - "policy__principles.toml": ( - '[inherit]\npaths = ["policy/extra.toml"]\n' - ), - }, - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 0, result.stderr) - self.assertIn("enforced by uphold", result.stdout) - - def test_the_two_readers_of_the_policy_agree(self): - """This script and the engine must resolve the same rules. - - `content_policy_rules` re-implements part of `config::load`: the bundled - sets, `inherit.paths`, `inherit.disabled_rules`, and a repository's own - rule shadowing an inherited id. The engine can now be asked directly -- - `uphold rules --effective --json` -- and the reason this script does not - simply call it is written on that function: it is the hook other - repositories install, and two of the three runners keep the binary - inside their own environment directory rather than on PATH. - - So the duplication stays, and this is what keeps it honest. Every field - the two readers disagree about is a rule reported to run where it does - not, or the other way round, and the answer a person acts on is to - delete a claim that was true. Asked of THIS repository's policy, which - is the one tree that exercises inheritance, disabling and shadowing at - once. - - Skipped where the binary has not been built, because a test that needs - a `cargo build` to be meaningful must not report a red suite to somebody - who has not run one. - """ - binary = ROOT / "target" / "debug" / "uphold" - if not binary.is_file(): - self.skipTest(f"{binary} is not built; `cargo build` first") - answered = subprocess.run( - [str(binary), "rules", "--effective", "--json"], - cwd=ROOT, - capture_output=True, - text=True, - check=False, - ) - self.assertEqual(answered.returncode, 0, answered.stderr) - engine = { - entry["id"]: uphold_check.Where( - hooks=set(entry["git_hooks"]), seams=set(entry["seams"]) - ) - for entry in json.loads(answered.stdout) - } - declared, disabled, _sets, _paths = uphold_check.content_policy_rules(ROOT) - here = { - rule: stages for rule, stages in declared.items() if rule not in disabled - } - self.assertEqual(here, engine) - - def test_a_rule_that_only_stands_in_front_of_a_command_is_not_the_scans(self): - """The seam `git.hooks` cannot name, and the reason it is compared. - - A rule whose only declared place is `command.before` fires when the shim - is on PATH ahead of the real command. It has no hooks and reads no - files, and while this reader knew only about hooks, an empty hook list - meant "the file scan's" -- so the rule was credited to `uphold scan` and - a claim on it reconciled green in a repository where the scan never - touches it. This repository has two such rules of its own. - - Asserted here rather than left to the agreement test above, because that - test compares the two readers and would stay green if BOTH were wrong in - the same direction, which is what they were. - """ - declared, _disabled, _sets, _paths = uphold_check.content_policy_rules(ROOT) - for rule_id in ("no-published-host-identity", "no-published-markers"): - where = declared[rule_id] - self.assertEqual(where.seams, {"shim"}, rule_id) - self.assertEqual(where.hooks, set(), rule_id) - - def test_a_claim_on_a_shim_only_rule_is_refused_and_not_credited_to_the_scan(self): - """The reconcile end of the same bug. - - The repository pins `uphold-scan` and nothing else, and its policy holds - one rule whose only declared place is `command.before`. Nothing here - establishes that the shim is on PATH in front of `gh`, so the claim is - not supplied -- but while an empty hook list meant "the file scan's", - the pinned `uphold-scan` was read as supplying it and the claim - reconciled green, exit 0, over a rule the scan never touches. - """ - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "complete-mediation" - rule = "no-published-markers" - """, - **{ - ".pre-commit-config.yaml": LOCAL_CONTENT_POLICY, - # The `[[shim]]` is not decoration: the engine refuses a - # `command.before` naming a command no shim declares, so a - # fixture without one is a policy that would never load. - "policy__principles.toml": ( - "[[shim]]\n" - 'command = "gh"\n' - 'match = ["pr:create"]\n' - 'text_flags = ["-b", "--body"]\n' - "\n" - "[rule.no-published-markers]\n" - 'message = "do not publish that"\n' - 'exec = "uphold guard --text -"\n' - 'command.before = ["gh"]\n' - ), - }, - ) - result = run(Path(tmp)) - coverage = run(Path(tmp), "--coverage") - self.assertEqual(result.returncode, 1, result.stdout + result.stderr) - self.assertIn("no-published-markers", result.stderr) - # And the coverage report says which seam went unestablished, rather - # than listing the rule as one the scan runs. - self.assertIn("stands in front of a command", coverage.stdout) - - def test_inherit_paths_naming_a_file_that_is_not_there_is_two_not_one(self): - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "single-authoritative-source" - rule = "no-merge-conflict-markers" - """, - **{ - ".pre-commit-config.yaml": LOCAL_CONTENT_POLICY, - "policy__principles.toml": ( - '[inherit]\npaths = ["policy/gone.toml"]\n' - ), - }, - ) - result = run(Path(tmp)) - coverage = run(Path(tmp), "--coverage") - # A policy file the engine merges and this reader cannot open is a seam - # that could not be read, which is exit 2 at both ends -- and the - # coverage report is where the file that could not be opened is named. - self.assertEqual(result.returncode, 2, result.stdout) - self.assertIn("could not look", result.stderr) - self.assertEqual(coverage.returncode, 2, coverage.stdout) - self.assertIn("inherit.paths", coverage.stdout) - - def test_a_lefthook_key_that_is_not_a_command_is_not_a_rule(self): - """`configs:` under `remotes:` is not a rule called `configs`. - - README.md tells every lefthook consumer to write that key, at exactly - the indent a command name sits at, so a scan keyed on indentation - accepted a claim on it -- a green reconcile over a rule that exists - nowhere. What makes a command name a command name is `commands:` above - it, which is what the scan reads now. - """ - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "fail-safe-defaults" - rule = "configs" - """, - **{ - "lefthook.yml": LEFTHOOK_WITH_PRINCIPLES, - "policy__principles.toml": GUARD_POLICY, - }, - ) - refused = run(Path(tmp)) - - build( - Path(tmp), - """ - [[enforce]] - principle = "fail-safe-defaults" - rule = "my-own-check" - """, - ) - command = run(Path(tmp)) - self.assertEqual(refused.returncode, 1, refused.stdout) - self.assertIn("no seam here supplies", refused.stderr) - # The command in the same file, which is a rule, still resolves -- the - # fix is a narrower scan and not a disabled one. - self.assertEqual(command.returncode, 0, command.stderr) - self.assertIn("enforced by local", command.stdout) - - def test_a_consumer_inheriting_a_bundled_base_set_can_be_read(self): - """`inherit.sets` names a set that ships HERE, not in the consumer. - - The engine compiles the base sets into the binary with `include_str!`, - so a consuming repository inherits rules whose file it does not have - and never will. Resolving the name against their tree turns every such - repository into exit 2 on a declaration that is fine. - """ - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "fail-safe-defaults" - rule = "prevent-public-push" - """, - **{ - ".pre-commit-config.yaml": PRE_COMMIT_WITH_PRINCIPLES, - "policy__principles.toml": ( - '[inherit]\nsets = ["process-residue"]\n' + GUARD_POLICY - ), - }, - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 0, result.stderr) - self.assertIn("enforced by uphold", result.stdout) - - def test_uninstalled_hook_is_refused(self): - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "fail-safe-defaults" - rule = "no-merge-commit" - """, - **{ - ".pre-commit-config.yaml": PRE_COMMIT_WITH_PRINCIPLES, - "policy__principles.toml": "", - }, - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 1, result.stderr) - self.assertIn("no seam here supplies", result.stderr) - - def test_disabled_content_policy_rule_is_refused(self): - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "single-authoritative-source" - rule = "no-merge-conflict-markers" - """, - **{ - ".pre-commit-config.yaml": LOCAL_CONTENT_POLICY, - "policy__base__process-residue.toml": HYGIENE_BASE, - "policy__principles.toml": ( - "[inherit]\n" - 'sets = ["process-residue"]\n' - 'disabled_rules = ["no-merge-conflict-markers"]\n' - ), - }, - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 1, result.stderr) - self.assertIn("no seam here supplies", result.stderr) - - def test_a_language_rule_claim_resolves(self): - """The drift this schema deleted, pinned so it cannot come back. - - The reconciler walked a hardcoded list of six array-of-tables names - against an engine that had seven. `language_rule` was the missing one, - so a claim naming a language rule was reported as enforcing nothing - while it was in fact enforced -- and neither repository could catch it, - because the list was a literal in one describing a constant in the - other. - """ - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "least-astonishment" - rule = "latin-only-docs" - """, - **{ - ".pre-commit-config.yaml": LOCAL_CONTENT_POLICY, - "policy__principles.toml": ( - "[rule.latin-only-docs]\n" - 'allowed_scripts = ["Latin"]\n' - 'files.glob = ["*.md"]\n' - ), - }, - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 0, result.stderr + result.stdout) - - def test_cmd_shims_check_must_be_enabled(self): - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "complete-mediation" - rule = "prevent-ai-author" - """, - **{ - ".pre-commit-config.yaml": PRE_COMMIT_WITH_PRINCIPLES, - "policy__principles.toml": "", - ".cmd-shims__checks.enabled": "# only this one\nno-os-identity\n", - }, - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 1, result.stderr) - self.assertIn("no seam here supplies", result.stderr) - - def test_a_rule_enforced_by_two_seams_reports_both(self): - """A rule enforced by more than one tool is the ordinary case. - - `prevent-ai-author` is a commit-msg hook in git-guards and a checker in - cmd-shims, because a commit message and a pull-request body are two - paths to the same public place. A structure holding one supplier per - rule id would have had to call one of them a duplicate, which inverts - what the two entries mean -- and `complete-mediation` is the record - saying a control is only as wide as the paths it mediates. - """ - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "complete-mediation" - rule = "prevent-ai-author" - """, - **{ - ".pre-commit-config.yaml": PRE_COMMIT_WITH_PRINCIPLES, - "policy__principles.toml": ( - "[rule.prevent-ai-author]\n" - 'builtin = "prevent-ai-author"\n' - 'git.hooks = ["commit-msg"]\n' - ), - ".cmd-shims__checks.enabled": "prevent-ai-author\n", - }, - ) - result = run(Path(tmp)) - coverage = run(Path(tmp), "--coverage") - self.assertEqual(result.returncode, 0, result.stderr) - self.assertIn("enforced by uphold, cmd-shims", result.stdout) - # And it is claimed under BOTH seams in the coverage report, rather than - # counting once and leaving the other seam's copy looking unclaimed. - self.assertEqual( - coverage.stdout.count("claimed prevent-ai-author -> complete-mediation"), - 2, - coverage.stdout, - ) - - def test_unknown_principle_is_refused(self): - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "no-such-principle" - rule = "my-own-check" - """, - **{ - ".pre-commit-config.yaml": PRE_COMMIT_WITH_PRINCIPLES, - "policy__principles.toml": "", - }, - ) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 1, result.stderr) - self.assertIn("unknown principle id", result.stderr) - - class NoProseInRuntime(unittest.TestCase): """`enforcement-needs-a-trigger`: the tool must not carry principle text.""" - def test_output_contains_no_record_prose(self): - result = run(ROOT) + def test_the_catalog_modes_carry_no_record_prose_into_a_report(self): + # `--list` is the mode a runtime is most likely to pipe somewhere. The + # reconcile's half of this invariant moved with the reconcile and is + # asserted in tests/check_cli.rs. + result = run(ROOT, "--list") self.assertEqual(result.returncode, 0, result.stderr) records = uphold_check.load_records() for record in records.values(): @@ -799,7 +167,7 @@ def test_the_review_tier_may_not_exist_without_its_ceiling(self): """, **{ ".pre-commit-config.yaml": PRE_COMMIT_WITH_PRINCIPLES, - "policy__principles.toml": "", + "policy__principles.toml": GUARD_POLICY, }, ) result = run(Path(tmp), "--review") @@ -881,327 +249,3 @@ def test_a_refused_declaration_exports_nothing(self): result = run(Path(tmp), "--oscal") self.assertEqual(result.returncode, 1, result.stdout) self.assertEqual(result.stdout.strip(), "") - - -class Coverage(unittest.TestCase): - """The denominator the reconcile cannot see: rules running under no claim. - - `--coverage` counts the direction the declaration does not: it walks what the - four tiers actually run here and reports which of those rules any claim - names. It reports and does not refuse -- a mode that failed a build over an - unclaimed rule would be paid for in claims written to silence it. - """ - - DECLARATION = """ - [[enforce]] - principle = "fail-safe-defaults" - rule = "prevent-public-push" - """ - - # Both guard stages the policy below declares are pinned, because a rule - # whose stage nothing installs is a rule that runs nowhere and does not - # belong in this denominator. - PRE_COMMIT = """\ -repos: - - repo: https://github.com/HackingGate/uphold - rev: v2.0.0 - hooks: - - id: uphold-scan - - id: uphold-guard - - id: uphold-guard-push - - repo: local - hooks: - - id: my-own-check - name: my own check -""" - - POLICY = """\ -[rule.prevent-public-push] -builtin = "prevent-public-push" -git.hooks = ["pre-push"] - -[rule.no-merge-commit] -builtin = "no-merge-commit" -git.hooks = ["pre-commit"] -""" - - def coverage(self, tmp: str) -> subprocess.CompletedProcess: - build( - Path(tmp), - self.DECLARATION, - **{ - ".pre-commit-config.yaml": self.PRE_COMMIT, - "policy__principles.toml": self.POLICY, - }, - ) - return run(Path(tmp), "--coverage") - - def test_a_claimed_rule_is_reported_against_its_principle(self): - with tempfile.TemporaryDirectory() as tmp: - result = self.coverage(tmp) - self.assertEqual(result.returncode, 0, result.stderr) - self.assertIn("prevent-public-push -> fail-safe-defaults", result.stdout) - self.assertIn("uphold: 1 of 2", result.stdout) - - def test_a_rule_no_claim_names_is_reported_as_unclaimed(self): - with tempfile.TemporaryDirectory() as tmp: - result = self.coverage(tmp) - self.assertIn("unclaimed no-merge-commit", result.stdout) - self.assertIn("my-own-check", result.stdout) - - def test_a_seam_not_in_use_counts_zero_and_exits_zero(self): - # A repository with no `.cmd-shims/checks.enabled` is not running - # cmd-shims. That is a real zero, and it has to be told apart from the - # hole below -- while a claim named its seam the two could be conflated - # harmlessly, and once a claim resolves against every seam, calling - # "not in use" unreadable makes a reconcile that can never say `false`. - with tempfile.TemporaryDirectory() as tmp: - result = self.coverage(tmp) - self.assertEqual(result.returncode, 0, result.stdout) - self.assertIn("cmd-shims: 0 of 0 rules", result.stdout) - - def test_a_seam_that_could_not_be_read_is_not_a_seam_running_nothing(self): - # The count is `?`, not 0, and the exit code is 2: an unreadable seam is - # a hole in the denominator, and a hole reported as zero reads as - # coverage that was never measured. - with tempfile.TemporaryDirectory() as tmp: - build(Path(tmp), self.DECLARATION) # no .pre-commit-config.yaml - result = run(Path(tmp), "--coverage") - self.assertEqual(result.returncode, 2, result.stdout) - self.assertIn("local: 0 of ? rules", result.stdout) - - def test_a_false_claim_is_reported_rather_than_refused(self): - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "fail-safe-defaults" - rule = "no-such-hook" - """, - **{ - ".pre-commit-config.yaml": self.PRE_COMMIT, - ".cmd-shims__checks.enabled": "no-os-identity\n", - "policy__principles.toml": "", - }, - ) - result = run(Path(tmp), "--coverage") - reconcile = run(Path(tmp)) - self.assertEqual(reconcile.returncode, 1, reconcile.stderr) - self.assertEqual(result.returncode, 0, result.stderr) - self.assertIn( - "claimed but supplied by nothing here: no-such-hook", result.stdout - ) - - def test_an_orphan_claim_is_not_counted_in_the_number_it_was_reported_under(self): - """The numerator counted the orphans the same report had just named. - - `records: N of M claimable records are claimed by a rule here` is the - one number a reader takes away, and it was computed from the claims - rather than from what any seam supplies -- so a declaration whose only - claim names a rule nothing runs reported one record as claimed by a rule - here, two lines under the line saying that rule is supplied by nothing. - """ - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - """ - [[enforce]] - principle = "fail-safe-defaults" - rule = "no-such-hook" - """, - **{ - ".pre-commit-config.yaml": self.PRE_COMMIT, - "policy__principles.toml": "", - }, - ) - result = run(Path(tmp), "--coverage") - self.assertEqual(result.returncode, 0, result.stderr) - self.assertIn( - "claimed but supplied by nothing here: no-such-hook", result.stdout - ) - self.assertIn("records: 0 of ", result.stdout) - - def test_it_counts_records_against_what_can_be_claimed(self): - result = run(ROOT, "--coverage") - self.assertEqual(result.returncode, 0, result.stderr) - self.assertIn("claimable records are claimed by a rule here", result.stdout) - - def test_a_bundled_base_set_is_counted_and_named(self): - """The one hole in this report, now closed. - - While the base set lived in another repository at a pinned rev its rules - ran here and could not be enumerated from here, so the tier was reported - as locally declared rules plus a note naming what could not be seen. - The sets ship in this repository now, so they are counted -- and still - named, because a reader has to know which ones went into the number. - """ - with tempfile.TemporaryDirectory() as tmp: - build( - Path(tmp), - "# nothing enforced yet\n", - **{ - ".pre-commit-config.yaml": LOCAL_CONTENT_POLICY, - "policy__base__process-residue.toml": HYGIENE_BASE, - "policy__principles.toml": ( - '[inherit]\nsets = ["process-residue"]\n' - ), - ".cmd-shims__checks.enabled": "no-os-identity\n", - }, - ) - result = run(Path(tmp), "--coverage") - self.assertEqual(result.returncode, 0, result.stderr) - self.assertIn("bundled base set", result.stdout) - self.assertIn("no-merge-conflict-markers", result.stdout) - - def test_coverage_carries_no_record_prose(self): - result = run(ROOT, "--coverage") - for record in uphold_check.load_records().values(): - self.assertNotIn(record["claim"], result.stdout) - - -class SelfApplication(unittest.TestCase): - def test_this_repository_reconciles(self): - result = run(ROOT) - self.assertEqual(result.returncode, 0, result.stderr) - self.assertIn("explicit-unknown <- catalog-tests", result.stdout) - - def test_starter_declaration_is_valid_and_enforces_nothing(self): - with tempfile.TemporaryDirectory() as tmp: - starter = run(ROOT, "--init") - self.assertEqual(starter.returncode, 0, starter.stderr) - build(Path(tmp), starter.stdout) - result = run(Path(tmp)) - self.assertEqual(result.returncode, 0, result.stderr) - - -class UpstreamIdentity(unittest.TestCase): - """Who this repository is, is stated once and read everywhere else. - - The reconciler has to recognise a lefthook consumer that names this - repository under `remotes:`, and the OSCAL export has to stamp a namespace. - Both used to spell `owner/name` out as a literal beside a `repository` field - in Cargo.toml that already said it -- two copies of one fact, which is what - `single-authoritative-source` refuses. The drift is the silent direction: a - rename that lands in the manifest and not in the literal leaves the seam - unrecognised, which reads as "no runner configuration here" rather than as a - stale pattern, so nothing anywhere says the check stopped matching. - """ - - def test_the_slug_is_derived_from_the_cargo_manifest(self) -> None: - cargo = tomllib.loads((ROOT / "Cargo.toml").read_text(encoding="utf-8")) - url = cargo["package"]["repository"] - - self.assertEqual(uphold_check.upstream_url(), url) - self.assertEqual(uphold_check.upstream_slug(), "/".join(url.split("/")[-2:])) - # Asserted through the reader rather than against its pattern: the - # pattern is gone, and what the drift would break is the recognition. - slug = uphold_check.upstream_slug() - self.assertTrue( - uphold_check.includes_our_lefthook_remote( - f"remotes:\n" - f" - git_url: https://github.com/{slug}\n" - f" ref: v1.0.0\n" - f" configs:\n" - f" - hooks/lefthook.yml\n" - ) - ) - - def test_a_remote_naming_this_repository_without_an_owner_is_still_ours( - self, - ) -> None: - """lefthook takes any git url, and most of them carry no `owner/name`. - - Requiring the slug rejected a clone by filesystem path, which is exactly - what `scripts/consumer_check.sh` writes: the parity harness points the - consumer at the checkout under test. A consumer wired the way the - documentation describes was reported as running no seam at all, and the - one CI job that drives a real lefthook consumer refused a clean commit. - """ - name = uphold_check.upstream_slug().rsplit("/", 1)[-1] - # Neutral placeholders, because this repository's own - # `no-running-os-identity-metadata` rule reads the running home path and - # searches the tracked files for it. A realistic workspace path written - # here passes on a developer's machine and refuses the scan on a CI - # runner, whose home directory it happens to name -- and a comment - # quoting that path to explain the trap falls into it exactly as the - # test data did. `scripts/consumer_check.sh` avoids the same edge by - # cloning to a neutral path, and says so. - for url in ( - f"/srv/example/work/{name}", - f"git@github.com:HackingGate/{name}.git", - "/srv/example/neutral-clone-name", - ): - with self.subTest(url=url): - self.assertTrue( - uphold_check.includes_our_lefthook_remote( - f"remotes:\n" - f" - git_url: {url}\n" - f" ref: v1.0.0\n" - f" configs:\n" - f" - hooks/lefthook.yml\n" - ) - ) - - def test_a_remote_is_only_ours_when_one_entry_says_both(self) -> None: - """Neither half alone, because the branch it feeds grants every stage. - - This read as an alternation, so a fork of this repository pinning its own - config, or an unrelated project whose config happens to carry the - conventional filename, was credited with running every guard published - here. - """ - slug = uphold_check.upstream_slug() - ours_but_another_config = ( - f"remotes:\n" - f" - git_url: https://github.com/{slug}\n" - f" configs:\n" - f" - hooks/something-else.yml\n" - ) - our_filename_from_elsewhere = ( - "remotes:\n" - " - git_url: https://github.com/someone/unrelated\n" - " configs:\n" - " - hooks/lefthook.yml\n" - ) - split_across_two_entries = ( - f"remotes:\n" - f" - git_url: https://github.com/{slug}\n" - f" configs:\n" - f" - hooks/something-else.yml\n" - f" - git_url: https://github.com/someone/unrelated\n" - f" configs:\n" - f" - hooks/lefthook.yml\n" - ) - - self.assertFalse( - uphold_check.includes_our_lefthook_remote(ours_but_another_config) - ) - self.assertFalse( - uphold_check.includes_our_lefthook_remote(our_filename_from_elsewhere) - ) - self.assertFalse( - uphold_check.includes_our_lefthook_remote(split_across_two_entries) - ) - - def test_the_python_manifest_names_the_same_repository(self) -> None: - """The one copy that survives, because no backend derives it. - - maturin reads Cargo.toml for the version and the binary, but - `[project.urls]` is pyproject's own and nothing reconciles the two. So - the copy is allowed and the drift is not -- this is the check that makes - the difference. - """ - cargo = tomllib.loads((ROOT / "Cargo.toml").read_text(encoding="utf-8")) - pyproject = tomllib.loads((ROOT / "pyproject.toml").read_text(encoding="utf-8")) - - self.assertEqual( - pyproject["project"]["urls"]["Repository"], - cargo["package"]["repository"], - "pyproject.toml and Cargo.toml name different repositories; rename " - "both in the commit that renames either", - ) - - -if __name__ == "__main__": - unittest.main() diff --git a/uphold_check.py b/uphold_check.py index 982c246..5949140 100755 --- a/uphold_check.py +++ b/uphold_check.py @@ -1,70 +1,52 @@ #!/usr/bin/env python3 -"""Reconcile a repository's enforcement claims against the principle catalog. +"""The principle catalog, for a person building a rule. The catalog says what a principle claims and what a machine could observe about it. Turning one into a rule that fires is work a person or an agent does once, -in the seam that can observe the property: `uphold` over this repository's -own files and over what git is about to do, cmd-shims over what a command -publishes, or a local hook. This script is the reconcile step afterwards. - -A declaration entry is a claim that a named rule is what enforces a named -principle *here*:: - - # policy/upheld.toml - [[enforce]] - principle = "explicit-unknown" - rule = "catalog-tests" - -The claim is falsifiable from this repository's own configuration: the rule is -installed and enabled somewhere, or it is not. When it is false the principle -stopped being enforced while the declaration went on saying it was, and that is -the only thing this checks. - -A rule id resolves against every seam at once. It named a tier as well until -the seams stopped being separate repositories, and a rule enforced by more than -one tool is the ordinary case rather than an ambiguity: `prevent-ai-author` -guards a commit message in git-guards and a pull-request body in cmd-shims, -because those are two paths to the same public place. The reconcile reports -every seam that supplies the rule. - -It carries no prose into any runtime. A principle's text is design input for -whoever builds the rule -- read it with `--explain` -- not a value a tool -injects, because a tool holding prose has no condition on which to emit it. See -the `enforcement-needs-a-trigger` record. +in the seam that can observe the property. What is here is everything around +that work which is PROSE: the record itself, the alias index a reader arrives +through, the review tier for principles no rule can decide, and the OSCAL +export. + +The reconcile is not here. Asking whether a claim holds means knowing which +rules this repository resolves to -- the bundled sets, `inherit.paths`, +`inherit.disabled_rules`, and a repository's own rule shadowing an inherited id +-- and answering that here meant re-implementing `config::load` in a second +program free to disagree with the first. It disagreed about the seam a hookless +rule runs at, and credited a checker standing in front of `gh` to the file scan. + + uphold check # the claims still hold + uphold check --coverage # which rules here carry a principle + +This script reads the catalog and never the policy, which is why it can stay a +script: a mode that cannot read the policy cannot disagree with the loader about +which rules run. Usage:: - uphold_check.py # reconcile the declaration (hook mode) uphold_check.py --explain ID # print one catalog record (an alias works too) uphold_check.py --list # list catalog ids - uphold_check.py --coverage # which rules here carry a principle uphold_check.py --review # what routes to a rule and what to a reviewer uphold_check.py --review --emit # write the compiled review document uphold_check.py --init # print a starter declaration uphold_check.py --oscal # emit the claims as OSCAL component-definition uphold_check.py --self-check # validate the bundled catalog itself -Exit codes -- the same three the other HackingGate tiers use, so a caller never -has to learn a second convention: +Exit codes -- the same three every seam here uses, so a caller never has to +learn a second convention: - 0 every claim reconciles - 1 a claim is false: the rule is absent, disabled, or the principle is not one - 2 could not look (missing declaration, unparseable config, unreadable seam) + 0 the mode did what it was asked + 1 a refusal: the review document is over its ceiling, a claim is false + 2 could not look (missing declaration, unreadable catalog, no binary to ask) Exit 2 is deliberately not exit 0. A configuration this tool could not read is not a repository that complies; see the `explicit-unknown` record. - -`--coverage` is the one mode that reports rather than refuses: 0, or 2 where a -tier's configuration could not be read. A rule running under no claim is not a -failure -- deciding which principle a rule serves is a judgment, and a mode that -exited 1 over a missing one would be paid in claims nobody believes. """ from __future__ import annotations import functools import json -import re import subprocess import sys import tomllib @@ -79,236 +61,26 @@ HERE = Path(__file__).resolve().parent DECLARATION_RELPATH = Path("policy") / "upheld.toml" -PRE_COMMIT_CONFIG = Path(".pre-commit-config.yaml") -LEFTHOOK_CONFIG = Path("lefthook.yml") -CONTENT_POLICY = Path("policy") / "principles.toml" -CONTENT_POLICY_BASE = Path("policy") / "base" -CMD_SHIMS_CHECKS = Path(".cmd-shims") / "checks.enabled" - -# `uphold` is one seam now, not two. The content rules and the guards live -# in one policy file under one id namespace and are run by one binary, so -# splitting them here would put back the boundary the merge deleted -- and -# git-guards no longer supplies anything: its eleven ids are guard rules. -TIERS = ("uphold", "cmd-shims", "local") - -# `- repo: ` / `- id: ` in a pre-commit or prek configuration. A -# line scan rather than a YAML parse: this script installs as `language: script` -# and has no dependencies. The scan sees the block form every HackingGate repo -# uses; a flow-style file yields zero repos, which is reported as could-not-look -# rather than as an absent hook. -REPO_LINE = re.compile(r"^\s*-\s*repo:\s*(\S+)") -HOOK_ID_LINE = re.compile(r"^\s*-\s*id:\s*(\S+)") - -# A valueless mapping key: `commands:` itself, a command name under it, or the -# `configs:` key a lefthook consumer writes under `remotes:`. WHICH of those it -# is cannot be read off the line, so the scan below tracks the key that encloses -# it instead of matching on indentation. Matching on indentation alone is what -# made `configs` a command name -- README.md tells every lefthook consumer to -# write that key verbatim at exactly the indent a command name sits at, so a -# claim on a rule called `configs` reconciled green in every repository that -# followed the documentation. -LEFTHOOK_KEY = re.compile(r"^(\s*)([A-Za-z0-9._-]+):\s*(?:#.*)?$") - -# The ids THIS repository publishes, read from the manifest that publishes them. -# -# Written out here as a literal it would be an enumeration describing a constant -# in another file -- the exact shape of the bug `_rule_stages` was rewritten to -# delete, where a hardcoded list of six table names sat opposite an engine that -# had seven and silently under-reported. The manifest is the list; this reads it. -PUBLISHED_HOOKS = Path(".pre-commit-hooks.yaml") - -# What an id RUNS, which is the part of the manifest that says whether pinning -# it is evidence of anything. `entry:` is the command the runner executes, so -# `uphold scan` is the file scan, `uphold guard --stage ` is the guard at -# exactly one git stage, and `uphold_check.py` is this script -- the reconciler -# itself, which runs no rule and enforces nothing. -# -# Reading the stage out of the entry rather than writing the five pairs down -# here is the same decision as reading the ids: the mapping pre-commit -> -# uphold-guard, commit-msg -> uphold-guard-commit-msg, pre-merge-commit -> -# uphold-guard-merge, pre-push -> uphold-guard-push and manual -> -# uphold-guard-manual is a fact the manifest already states, and a copy of it -# here is the copy that goes stale when a sixth stage is published. -ENTRY_LINE = re.compile(r"^\s+entry:\s*(.+?)\s*$") -ENTRY_GUARD = re.compile(r"(?:\buphold\b|--)\s+guard\b.*?--stage\s+([A-Za-z0-9-]+)") -ENTRY_SCAN = re.compile(r"(?:\buphold\b|--)\s+scan\b") - -# A lefthook consumer pins nothing by id. It names this repository under -# `remotes:` and lefthook merges the commands in, so the consumer's own file -# contains neither an id nor a command name -- only the repository name and, -# for anyone wiring it by hand, the command line itself. -# -# The subcommand is what is matched, not the executable. This repository runs -# its own binary out of the working tree with `cargo run -- scan`, a consumer -# runs `uphold scan` from PATH, and a third might run it by absolute path; -# all three are the same seam, and a pattern anchored on the program name would -# have recognised only the middle one. -LEFTHOOK_RUN = re.compile(r"^\s*run:\s*.*(?:\buphold|--)\s+(?:scan|guard)\b") -# Which seam that hand-wired command line is, asked of the same line. A config -# that runs the guards and never runs the scan installs no file rules, and one -# that names `--stage pre-commit` says nothing about pre-push -- the same -# distinction the published ids make, arriving as an argument instead of an id. -LEFTHOOK_RUN_SCAN = re.compile(r"^\s*run:\s*.*(?:\buphold|--)\s+scan\b") -LEFTHOOK_RUN_STAGE = re.compile(r"--stage\s+([A-Za-z0-9-]+)") -# Either the repository name or the config path it publishes. The path is the -# more reliable of the two: `git_url` may be a mirror, an SSH form, or a local -# clone, and none of those has to contain the repository's name -- but a remote -# that includes `hooks/lefthook.yml` is including THIS repository's config, and -# that string is what the consumer wrote down. -# -# The name carries the owner because a bare repository name is not this -# repository's to claim: a remote for someone else's `uphold-policies` would -# otherwise read as this one. -# -# Owner and name are READ rather than written down here, for the same reason -# PUBLISHED_HOOKS is read: a literal would be a second statement of a fact -# `Cargo.toml` already owns, and the copy is the one that goes stale. A rename -# that lands in the manifest and not in this line fails silently and late -- the -# seam simply stops being recognised in a lefthook consumer, which reads as "no -# runner configuration here" rather than as a stale pattern. -# -# Read from HERE rather than from the tree under check, because this names the -# UPSTREAM. The repository being reconciled is a consumer, and its own manifest -# names something else entirely. -CARGO_MANIFEST = HERE / "Cargo.toml" - - -@functools.cache -def upstream_url() -> str: - """The `https://host/owner/name` this repository publishes itself as.""" - try: - manifest = tomllib.loads(CARGO_MANIFEST.read_text(encoding="utf-8")) - url = manifest["package"]["repository"] - except (OSError, tomllib.TOMLDecodeError, KeyError, TypeError) as error: - raise CouldNotLook( - f"cannot read `package.repository` from {CARGO_MANIFEST}, so the " - f"upstream this repository is cannot be named: {error}" - ) from error - if not isinstance(url, str) or not url.strip(): - raise CouldNotLook(f"`package.repository` in {CARGO_MANIFEST} is empty") - return url.strip() - - -@functools.cache -def upstream_slug() -> str: - """`owner/name` -- the form a consumer's runner configuration writes down.""" - parts = upstream_url().rstrip("/").removesuffix(".git").split("/") - if len(parts) < 2 or not all(parts[-2:]): - raise CouldNotLook( - f"`package.repository` in {CARGO_MANIFEST} is not an owner/name URL: " - f"{upstream_url()}" - ) - return "/".join(parts[-2:]) - +# The declaration is the only file this still reads out of a consumer's tree, +# and `--oscal` is the only mode that reads it. Everything that used to be here +# -- the pre-commit and lefthook scanners, the published-id table, the content +# policy reader -- answered "which rules run here", and `uphold check` answers +# that now, out of the loader that decides it. -def includes_our_lefthook_remote(text: str) -> bool: - """Does one `remotes:` entry name THIS repository and take its config? - Both halves, in the SAME entry. This was one regex alternating between the - two, so either alone was enough: a remote whose url merely contained the - slug, or a remote pulling a file that happens to be called - `hooks/lefthook.yml` out of somebody else's repository. Either match granted - every stage this manifest publishes, because the branch it feeds assumes the - remote IS this repository's config -- so a fork, a mirror, or an unrelated - project following the same conventional filename was credited with running - every guard here. +class Refused(Exception): + """The reconcile said a claim is false. Exit 1, not could-not-look. - Read by indentation rather than by pattern, because a `remotes:` item spells - its url and its config on separate lines, and the question is which lines - belong to the same item. + Kept apart from `CouldNotLook` because the two are different answers about + the world and the export owes an outside reader the difference: a claim this + tool refused is a claim it DID evaluate. """ - slug = upstream_slug() - entries: list[list[str]] = [] - current: list[str] | None = None - marker = -1 - for line in text.splitlines(): - if not line.strip() or line.lstrip().startswith("#"): - continue - indent = len(line) - len(line.lstrip()) - stripped = line.lstrip() - if stripped.startswith("- "): - # A less-indented item ends the one before it and is not part of it. - if current is not None and indent <= marker: - entries.append(current) - current = None - if current is None: - current, marker = [], indent - elif current is not None and indent <= marker: - entries.append(current) - current = None - if current is not None: - current.append(stripped) - if current is not None: - entries.append(current) - # The test is not "does this url name us". It is "does this url name someone - # ELSE", because most git urls cannot answer the first question at all. - # - # lefthook takes any git url. A consumer may clone this repository from a - # filesystem path, from a mirror, or from a bare directory whose name says - # nothing -- `scripts/consumer_check.sh` does exactly that, cloning to a - # neutral `$WORK/hooks` on purpose, so the url the consumer writes carries - # neither the owner nor the repository name. Demanding the slug there is - # demanding evidence the format does not carry, and answering "no seam here - # supplies it" is answering exit 1 -- the claim is false -- about a - # repository whose only fault is cloning from a path. - # - # So a remote is rejected only when it is identifiably somebody else's: it - # spells a forge `owner/name` and that pair is not ours. Anything without a - # host is a path, and a path is unidentifiable rather than foreign. - # - # The load-bearing half of the previous fix is untouched: both the remote and - # `hooks/lefthook.yml` must appear in the SAME entry, so a fork pinning its - # own config, or an unrelated project pulling a file that happens to share - # the conventional name, is still not credited with running every guard here. - forge_url = re.compile( - r"(?:https?://|ssh://|git://|[\w.-]+@)[\w.-]+[/:](?P[\w.\-/]+)" - ) - - def could_be_this_repository(line: str) -> bool: - _, _, value = line.partition(":") - for word in value.split(): - found = forge_url.search(word) - if not found: - # No host, so no owner to disagree with: a path or a bare name. - continue - spelled = found.group("slug").rstrip("/").removesuffix(".git") - if "/" in spelled and not spelled.endswith(slug): - return False - return True - - return any( - any(could_be_this_repository(line) for line in entry if "git_url" in line) - and any("hooks/lefthook.yml" in line for line in entry) - for entry in entries - ) class CouldNotLook(Exception): """Raised where the tool cannot inspect what it claims to check (exit 2).""" -def _string_list(value: object, field: str) -> list[str]: - """Every entry, or a refusal naming the one that is not a string. - - The alternative -- keeping the strings and dropping the rest -- answers a - question nobody asked, because the engine reading the same file will not - silently agree. Whatever this list is short by is a rule the engine runs and - this reconcile has never heard of. - """ - if not isinstance(value, list): - raise CouldNotLook( - f"{CONTENT_POLICY}: {field} must be a list, not {type(value).__name__}" - ) - for index, entry in enumerate(value): - if not isinstance(entry, str): - raise CouldNotLook( - f"{CONTENT_POLICY}: {field}[{index}] is {type(entry).__name__}, not a string. " - f"Which rules it would have inherited cannot be resolved, so what this " - f"repository runs is unknown" - ) - return list(value) - - def discover_root() -> Path: """Walk up from cwd until the declaration is found.""" candidate = Path.cwd().resolve() @@ -359,657 +131,72 @@ def read_text(path: Path) -> str: # --------------------------------------------------------------------------- -# Reading the tiers' own configuration -# --------------------------------------------------------------------------- - - -def installed_hooks(root: Path) -> dict[str, list[str]]: - """Map hook id -> the repo urls that supply it, from .pre-commit-config.yaml.""" - path = root / PRE_COMMIT_CONFIG - if not path.is_file(): - # A repository that runs lefthook has no .pre-commit-config.yaml and is - # not a repository this script cannot read. Treating the absent file as - # could-not-look made every lefthook consumer exit 2 on a declaration - # that was in fact reconcilable from the file they do have. - if (root / LEFTHOOK_CONFIG).is_file(): - return {} - raise CouldNotLook( - f"neither {PRE_COMMIT_CONFIG} nor {LEFTHOOK_CONFIG} found; " - f"cannot tell whether a hook is installed" - ) - - hooks: dict[str, list[str]] = {} - current = "" - saw_repo = False - for line in read_text(path).splitlines(): - repo = REPO_LINE.match(line) - if repo: - current = repo.group(1) - saw_repo = True - continue - hook = HOOK_ID_LINE.match(line) - if hook and saw_repo: - hooks.setdefault(hook.group(1), []).append(current) - - if not saw_repo: - raise CouldNotLook( - f"{PRE_COMMIT_CONFIG} declares no `- repo:` entries this scan can read" - ) - return hooks - - -def published_seams() -> tuple[set[str], dict[str, str]]: - """(the ids that run `uphold scan`, stage -> the id that runs the guard there). - - Read from the manifest's `entry:` lines rather than from its `- id:` lines, - because an id is not evidence that anything runs. `uphold-check` is this - very script: a repository that pins it and nothing else runs the reconcile - and no rule at all, and while every published id counted as evidence that - pin was accepted as proof that every scan rule and every guard fires here -- - the reconciler certifying itself, and printing "reconciled N enforcement - claims" over a repository enforcing nothing. - - The stage is read for the same reason it is asked for: a guard id installs - exactly one git stage, so `uphold-guard-push` says nothing about what fires - at commit-msg. - """ - path = HERE / PUBLISHED_HOOKS - if not path.is_file(): - raise CouldNotLook( - f"{PUBLISHED_HOOKS} not found beside this script; " - f"cannot tell which hook ids run `uphold`" - ) - - scans: set[str] = set() - guards: dict[str, str] = {} - current = "" - for line in read_text(path).splitlines(): - hook = HOOK_ID_LINE.match(line) - if hook: - current = hook.group(1) - continue - entry = ENTRY_LINE.match(line) - if not entry or not current: - continue - command = entry.group(1) - guard = ENTRY_GUARD.search(command) - if guard: - guards.setdefault(guard.group(1), current) - elif ENTRY_SCAN.search(command): - scans.add(current) - current = "" - - if not scans or not guards: - raise CouldNotLook( - f"{PUBLISHED_HOOKS} publishes no id whose `entry:` runs `uphold scan` " - f"or `uphold guard --stage`; cannot tell what pinning an id would run" - ) - return scans, guards - - -def published_hook_ids() -> set[str]: - """The published ids that run a seam -- the evidence set, not the id list. - - Deliberately NOT every id in the manifest: see `published_seams`. - """ - scans, guards = published_seams() - return scans | set(guards.values()) - - -def runs_principles(root: Path) -> tuple[bool, set[str], str]: - """Which seams of `uphold` run here -- the file scan, and which git stages. - - The question used to be asked as "is there a hook called `content-policy`", - which is the name of a command in THIS repository's own lefthook.yml. No - consumer has that name: a pre-commit consumer pins `uphold-scan`, and a - lefthook consumer pins nothing at all and names the repository under - `remotes:`. So the seam that supplies every guard and every content rule - reported itself absent in every repository except this one, and every claim - against it was refused as enforced by nothing. - - Three ways in, one per runner, and the answer says which was taken -- a - reconcile that passes for a reason the reader cannot see is one they cannot - check. - - Two answers rather than one, and that is the second half of the same fix. A - single repository-wide yes/no said "uphold runs here" and let every rule in - the policy file resolve against it, so a repository that pinned `uphold-scan` - and no guard id at all reconciled a claim on a `pre-push` guard: the stage - that guard fires at is installed by `uphold-guard-push`, which nothing here - pinned, and the rule ran nowhere. What is returned is what was installed -- - the file scan, and the set of git stages some pinned id actually runs. - - Both runners are read and the answers unioned rather than the first one - winning: a repository may install pre-commit for the fast stages and drive - the slow ones from lefthook, and either file alone understates it. - """ - scans, guards = published_seams() - installed = set(installed_hooks(root)) - - scan = bool(scans & installed) - stages = {stage for stage, hook in guards.items() if hook in installed} - how: list[str] = [] - pinned = sorted((scans | set(guards.values())) & installed) - if pinned: - how.append(f"{PRE_COMMIT_CONFIG} pins {', '.join(pinned)}") - - path = root / LEFTHOOK_CONFIG - if path.is_file(): - text = read_text(path) - direct = [line for line in text.splitlines() if LEFTHOOK_RUN.match(line)] - if direct: - scan = scan or any(LEFTHOOK_RUN_SCAN.match(line) for line in direct) - ran = { - match.group(1) - for line in direct - if (match := LEFTHOOK_RUN_STAGE.search(line)) - } - stages |= ran - how.append(f"{LEFTHOOK_CONFIG} runs the binary directly") - if includes_our_lefthook_remote(text): - # The remote config is this repository's `hooks/lefthook.yml`, which - # wires every stage the manifest publishes. A consumer that includes - # it has them all, which is why including it is the one form that - # needs no per-stage reading. - scan = True - stages |= set(guards) - how.append(f"{LEFTHOOK_CONFIG} includes this repository as a remote") - - if not scan and not stages: - return ( - False, - set(), - "no runner configuration here runs `uphold scan` or `uphold guard`", - ) - return scan, stages, "; ".join(how) - - -def lefthook_commands(root: Path) -> set[str]: - """The command names a lefthook config defines, and nothing else. - - A command name is a valueless mapping key nested under `commands:`, and the - nesting is the whole of what distinguishes it. Matching indentation alone - accepted `configs:` -- the key under `remotes:` that README.md tells every - lefthook consumer to write verbatim -- as a rule named `configs`, so a claim - naming that rule reconciled green against a file that defines no such thing. - - The enclosing key is tracked with a stack of the valueless keys seen so far, - popped back to the current indent. A key that carries a value cannot enclose - anything, so it never joins the stack. - """ - path = root / LEFTHOOK_CONFIG - if not path.is_file(): - return set() - - names: set[str] = set() - enclosing: list[tuple[int, str]] = [] - for line in read_text(path).splitlines(): - match = LEFTHOOK_KEY.match(line) - if not match: - continue - indent, key = len(match.group(1)), match.group(2) - while enclosing and enclosing[-1][0] >= indent: - enclosing.pop() - if enclosing and enclosing[-1][1] == "commands": - names.add(key) - enclosing.append((indent, key)) - return names - - -class Where: - """The seams one rule runs at, and the git hooks if `guard` is one of them. - - Two fields rather than one, because `git.hooks` alone cannot answer the - question. An empty hook list was read as "the file scan's rule", and that is - true of a content rule and false of a checker standing in front of `gh` -- - which is how a claim on a rule whose only place is `command.before` was - credited to `uphold scan` and reconciled green in a repository where the - scan never touches it. - - `seams` holds the same three names the engine prints in - `uphold rules --effective --json`, and the drift test compares them. - """ - - __slots__ = ("hooks", "seams") - - def __init__(self, hooks: set[str], seams: set[str]) -> None: - self.hooks = hooks - self.seams = seams - - def __eq__(self, other: object) -> bool: - if not isinstance(other, Where): - return NotImplemented - return self.hooks == other.hooks and self.seams == other.seams - - def __repr__(self) -> str: - return f"Where(hooks={sorted(self.hooks)}, seams={sorted(self.seams)})" - - -def _rule_stages(policy: dict) -> dict[str, Where]: - """Every rule id in one policy document, mapped to where it runs. - - ONE table name, which is the whole point. This function used to walk a - hardcoded list of six array-of-tables names against an engine that had - seven: `language_rule` was missing, so a claim naming a language rule was - reported as enforcing nothing while it was in fact enforced. Nothing in - either repository could catch that -- the list was a literal here describing - a constant there. There is no list to be short now. - - The id is the section header -- `[rule.]` -- so the ids are the keys of - one table, and a duplicate cannot even parse. - - The seam is carried out beside the hooks, and it is the field that was - missing. `git.hooks` says which git stages a guard fires at; it says nothing - about a rule that fires at none, and there are two unrelated kinds of those. - `files.*` is the scan's, `command.before` is the shim's, and reading the - second as the first is the whole of the bug this record exists to close. - """ - rules = policy.get("rule", {}) - if not isinstance(rules, dict): - raise CouldNotLook("policy: [rule] must be a table of [rule.] sections") - - stages: dict[str, Where] = {} - for rule_id, body in rules.items(): - body = body if isinstance(body, dict) else {} - git = body.get("git", {}) if isinstance(body.get("git"), dict) else {} - hooks = git.get("hooks", []) - if not isinstance(hooks, list): - raise CouldNotLook( - f"policy: [rule.{rule_id}] git.hooks must be an array of git hook names" - ) - hooks = {value for value in hooks if isinstance(value, str)} - - seams: set[str] = set() - # The engine's own filter, in `Rule::seams`: a built-in reaches the scan - # only when it reads files and fires at no hook; every other rule that - # declares `files.*` is the scan's. - if isinstance(body.get("files"), dict) and not ( - isinstance(body.get("builtin"), str) and hooks - ): - seams.add("scan") - if hooks: - seams.add("guard") - if isinstance(body.get("command"), dict): - seams.add("shim") - stages[rule_id] = Where(hooks=hooks, seams=seams) - return stages - - -def content_policy_rules( - root: Path, -) -> tuple[dict[str, set[str]], set[str], list[str], list[str]]: - """Return (rule id -> git stages, disabled ids, inherited sets, inherited paths). - - Declared INCLUDES what `[inherit]` pulls in, because those rules run here. - While the base set lived in another repository at a pinned rev its rules ran - here and could not be enumerated from here, so the count was reported as - locally declared rules plus a note naming the hole. - - A bundled set resolves beside THIS SCRIPT, not beside the consumer's policy - file. The engine embeds the bundled sets with `include_str!`, so - `sets = ["process-residue"]` in a consuming repository resolves to a file - that repository does not have and never will -- and resolving it against - their tree made every consumer that inherits a base set exit 2 on a - declaration that was in fact fine. `HERE` is the clone a runner made of - this repository, which is where the sets the engine compiled in are. - - `inherit.paths` resolves the other way, against the tree under check, which - is where `config::load` resolves it: those are the consumer's own extra - policy files. Reading only `inherit.sets` made every rule arriving that way - invisible, so a claim on one was refused as supplied by nothing while the - engine was running it -- a false negative in the direction that costs the - most, because the answer a person acts on is "delete the claim". - - It is still a SECOND, partial reader of what `config::load` already - resolves, and every field the two disagree about is a rule reported to run - where it does not or the other way round. The one loader can now be asked - directly -- `uphold rules --effective --json` prints the resolved rule ids - with their `git.hooks` -- and that is what this function should eventually - read instead of the policy file. - - It does not read it yet, and the reason is where this script runs. It is the - hook OTHER repositories install, and two of the three runners build the - binary inside their own environment directory rather than putting it on - PATH; a consumer whose `uphold` is not reachable would go from a reconcile - that works to exit 2 on every commit. Shelling out with a fallback to this - reader would keep both readers AND add a third behaviour, so until the - binary's location is something this script can rely on, there is one reader - here and a test -- `Reconciliation.test_the_two_readers_of_the_policy_agree` - -- that fails when it drifts from the engine on this repository's own - policy. That test is the thing that makes the duplication survivable, and - deleting it is what would make it dangerous. - """ - bundled = HERE / CONTENT_POLICY_BASE - path = root / CONTENT_POLICY - if not path.is_file(): - raise CouldNotLook( - f"{CONTENT_POLICY} not found; cannot tell which rules this repo runs" - ) - policy = read_toml(path) - inherit = policy.get("inherit", {}) - if not isinstance(inherit, dict): - raise CouldNotLook(f"{CONTENT_POLICY}: 'inherit' must be a table") - - # Refused, not filtered. These two comprehensions dropped a non-string entry - # in silence, which turns a malformed declaration into a SHORTER list of - # inherited rules than the engine resolves -- and a claim on one of the rules - # that went missing then fails as "no seam here supplies it", exit 1, which - # in this tool means the claim is false. It is not false; nobody looked. The - # honest answer is exit 2, the same one a missing inherited file gets a few - # lines below. - names = _string_list(inherit.get("sets", []), "inherit.sets") - relatives = _string_list(inherit.get("paths", []), "inherit.paths") - - # Merged in the order the engine merges them -- bundled sets, then the - # named paths, then the repository's own rules -- so a rule the repository - # redefines is read with the stages the repository gave it. - declared: dict[str, set[str]] = {} - for name in names: - base_path = bundled / f"{name}.toml" - if not base_path.is_file(): - raise CouldNotLook( - f"{CONTENT_POLICY}: inherit.sets names {name!r}, " - f"which is not a bundled base set ({base_path} does not exist)" - ) - declared |= _rule_stages(read_toml(base_path)) - for relative in relatives: - extra = root / relative - if not extra.is_file(): - raise CouldNotLook( - f"{CONTENT_POLICY}: inherit.paths names {relative!r}, " - f"which this repository does not have ({extra} does not exist)" - ) - declared |= _rule_stages(read_toml(extra)) - declared |= _rule_stages(policy) - - # The third list in the same table, and it was left filtering in silence when - # the other two stopped. It is the one where dropping an entry is worst: the - # engine refuses a `disabled_rules` id that names nothing inherited, so a - # malformed entry that vanishes here is a load failure over there -- this - # tool reporting on a policy the binary will not even accept. - disabled = set( - _string_list(inherit.get("disabled_rules", []), "inherit.disabled_rules") - ) - return declared, disabled, names, relatives - - -def cmd_shims_checks(root: Path) -> set[str]: - """Which cmd-shims checks are enabled, or none where the seam is not in use. - - An ABSENT file is not an unreadable one, and the difference started - mattering when `tier` went away. While a claim named its seam, a missing - file meant a claim pointing at cmd-shims could not be judged -- fair, the - author had said that was where to look. A claim naming only a rule is judged - against every seam, so treating "this repository does not use cmd-shims" as - could-not-look made every false claim in every repository without the file - exit 2, and a reconcile that can never say `false` is not a reconcile. - - A file that exists and cannot be read still raises: that is a seam declared - and then unreadable, which is the case `explicit-unknown` is about. - """ - path = root / CMD_SHIMS_CHECKS - if not path.is_file(): - return set() - names = set() - for line in read_text(path).splitlines(): - stripped = line.split("#", 1)[0].strip() - if stripped: - names.add(stripped) - return names - - -# --------------------------------------------------------------------------- -# Reconciling one claim -# --------------------------------------------------------------------------- - - -def reconcile( - root: Path, declaration: dict, records: dict[str, dict] -) -> tuple[list[str], list[str]]: - """Return (failures, evidence). Raises CouldNotLook for unreadable config.""" - entries = declaration.get("enforce", []) - if not isinstance(entries, list): - raise CouldNotLook("`enforce` must be an array of tables") - - failures: list[str] = [] - evidence: list[str] = [] - suppliers, unreadable = rule_suppliers(root) - - for index, entry in enumerate(entries): - where = f"enforce[{index}]" - if not isinstance(entry, dict): - raise CouldNotLook(f"{where} must be a table") - - if "tier" in entry: - raise CouldNotLook( - f"{where} carries a `tier`. The field is gone: a rule id resolves " - f"across every seam at once, so a claim naming one no longer has " - f"to say which. Drop the line." - ) - - principle = entry.get("principle") - rule = entry.get("rule") - for field, value in (("principle", principle), ("rule", rule)): - if not isinstance(value, str) or not value.strip(): - raise CouldNotLook(f"{where}.{field} is required and must be a string") - - record = records.get(principle) - if record is None: - failures.append(f"{where}: unknown principle id {principle!r}") - continue - if record.get("status") == "deprecated": - failures.append( - f"{where}: {principle!r} is deprecated; the catalog keeps it for redirects only" - ) - continue - if record.get("enforcement", {}).get("automatable") == "no": - failures.append( - f'{where}: the {principle!r} record says enforcement.automatable = "no"; ' - f"no rule can be claimed to enforce it" - ) - continue - - held = suppliers.get(rule, []) - if held: - evidence.append(f"{principle} <- {rule} enforced by {', '.join(held)}") - continue - - if unreadable: - # A rule absent from what could be read is not an absent rule. The - # tiers that could not be inspected are exactly where it might be, - # so this is could-not-look and not a false claim -- see the - # `explicit-unknown` record. - raise CouldNotLook( - f"{where}: no rule {rule!r} in what could be read, and " - f"{', '.join(unreadable)} could not be read; cannot tell whether " - f"the claim holds" - ) - - failures.append( - f"{where}: {principle!r} claims {rule!r}, which no seam here supplies" - ) - - return failures, evidence - - -# --------------------------------------------------------------------------- -# Coverage: the denominator the reconcile cannot see +# Asking the engine # --------------------------------------------------------------------------- # -# The reconcile walks the declaration, so its entire universe is what somebody -# already claimed. A rule that fires under no claim is invisible to it, and so is -# a record nothing here enforces -- while "reconciled 7 enforcement claims" reads -# like coverage and is not. This mode counts the other direction: every rule the -# four tiers actually run in this repository, against the claims that name one. +# The reconcile moved into the binary as `uphold check`. What is left here reads +# the CATALOG and never the policy, which is the whole of why it is still +# Python: a mode that cannot read the policy cannot disagree with the loader +# about which rules run, and disagreeing about exactly that is what the two +# readers did. # -# It reports and does not refuse. An unclaimed rule is not a defect -- the -# mapping from a rule to the principle behind it is a human judgment, and a mode -# that exited 1 over a missing one would buy its own green by pushing people to -# write claims they do not believe, which is the declaration becoming decoration. -# Exit 2 stays, because a tier whose configuration could not be read is a hole in -# the denominator rather than a tier running nothing; see `explicit-unknown`. - +# `--oscal` is the exception, because a component definition is an assertion to +# an outside reader and exporting an unreconciled one would publish a claim +# nobody confirmed. So it asks the binary rather than deriving the answer, and a +# binary it cannot reach is could-not-look -- exit 2, not a smaller export. -class Inventory: - """What one tier actually runs here, and what could not be seen of it.""" - - __slots__ = ("notes", "rules", "unreadable") - - def __init__( - self, - rules: set[str] | None = None, - notes: list[str] | None = None, - unreadable: bool = False, - ) -> None: - self.rules = rules if rules is not None else set() - self.notes = notes if notes is not None else [] - self.unreadable = unreadable - - -def inventory_principles(root: Path) -> Inventory: - notes: list[str] = [] - try: - scan, stages, how = runs_principles(root) - except CouldNotLook as error: - return Inventory(notes=[str(error)], unreadable=True) - if not scan and not stages: - return Inventory(notes=[how]) - notes.append(how) +def engine(root: Path, *args: str) -> subprocess.CompletedProcess: + """Run the binary, wherever this checkout keeps it.""" + candidates = [ + HERE / "target" / "release" / "uphold", + HERE / "target" / "debug" / "uphold", + ] + binary = next((path for path in candidates if path.is_file()), None) + found = str(binary) if binary else "uphold" try: - declared, disabled, sets, paths = content_policy_rules(root) - except CouldNotLook as error: - return Inventory(notes=[str(error)], unreadable=True) - if sets: - # This tier used to be the one hole in the coverage report: the base set - # lived in another repository at a pinned rev, its rules ran here, and - # they could not be enumerated from here. They ship in this repository - # now, so the count is whole and says which sets it counted. - notes.append(f"includes the bundled base set(s): {', '.join(sets)}") - if paths: - notes.append( - f"includes the policy file(s) inherit.paths names: {', '.join(paths)}" - ) - if disabled: - notes.append(f"extend.disabled_rules turns off {', '.join(sorted(disabled))}") - - # A rule is supplied where the seam that runs it is installed, which is a - # question per rule and not per repository. A rule with no `git.hooks` is - # the file scan's; a rule with them fires at those git stages and nowhere - # else, so a policy declaring a pre-push guard in a repository that pinned - # only `uphold-scan` declares a rule that runs nowhere -- and reporting it - # as supplied is how a claim on it reconciled green. - # - # ANY of a rule's stages is enough. `no-stale-hook-pins` fires at pre-push - # and manual and the manual stage is reached by a scheduled run rather than - # by a pinned id, so requiring every stage would refuse a rule that is - # demonstrably running. - rules: set[str] = set() - uninstalled: list[str] = [] - for rule_id, where in sorted(declared.items()): - if rule_id in disabled: - continue - if "guard" in where.seams: - if where.hooks & stages: - rules.add(rule_id) - else: - uninstalled.append(f"{rule_id} ({', '.join(sorted(where.hooks))})") - elif "scan" in where.seams: - if scan: - rules.add(rule_id) - else: - uninstalled.append(f"{rule_id} (file scan)") - elif "shim" in where.seams: - # The shim seam, which this branch used to fall through to `scan`. - # A checker standing in front of `gh` runs when the shim is on PATH - # ahead of the real command, and no runner configuration in this - # repository says whether it is -- so the honest answer is that it - # was not established here, not that the file scan supplies it. - # `inventory_local` is where a repository asserts a seam this script - # cannot observe. - uninstalled.append(f"{rule_id} (stands in front of a command)") - else: - # `validate` refuses a rule with no declared place, so reaching this - # means the policy said something the engine would not have loaded. - uninstalled.append(f"{rule_id} (nothing declares where it runs)") - if uninstalled: - notes.append( - "declared, but no runner configuration here installs the seam it " - f"fires at: {', '.join(uninstalled)}" + return subprocess.run( + [found, *args], + cwd=root, + capture_output=True, + text=True, + check=False, ) - return Inventory(rules=rules, notes=notes) - - -def inventory_cmd_shims(root: Path) -> Inventory: - try: - return Inventory(rules=cmd_shims_checks(root)) - except CouldNotLook as error: - return Inventory(notes=[str(error)], unreadable=True) + except OSError as error: + raise CouldNotLook( + f"`uphold {' '.join(args)}` could not be run ({error}), so what this " + f"repository enforces is unknown. Build it with `cargo build --release`, " + f"or install it on PATH." + ) from error -def inventory_local(root: Path) -> Inventory: - """Everything `tier = "local"` could name: this repo's hooks and commands. +@functools.cache +def upstream_url() -> str: + """The `https://host/owner/name` this repository publishes itself as. - Wider than `- repo: local`, because a formatter or a linter from a - third-party repository is a rule that fires here and can be claimed as one. + Asked of the binary, which holds it as `package.repository` compiled in, so + the namespace an OSCAL property is stamped with cannot drift from the crate + that produced it. Reading `Cargo.toml` off disk beside this script worked + only in a checkout of this repository, and this mode is meant to be run + from a consumer's. """ - notes: list[str] = [] - unreadable = False - rules: set[str] = set() - try: - hooks = installed_hooks(root) - except CouldNotLook as error: - notes.append(str(error)) - unreadable = True - else: - rules |= set(hooks) - commands = lefthook_commands(root) - if commands: - notes.append(f"{LEFTHOOK_CONFIG} defines {len(commands)} command(s)") - return Inventory(rules=rules | commands, notes=notes, unreadable=unreadable) - - -INVENTORIES = { - "uphold": inventory_principles, - "cmd-shims": inventory_cmd_shims, - "local": inventory_local, -} - - -def rule_suppliers(root: Path) -> tuple[dict[str, list[str]], list[str]]: - """Every rule id this repository runs, and every seam that supplies it. + answered = engine(Path.cwd(), "--upstream") + url = answered.stdout.strip() + if answered.returncode != 0 or not url: + raise CouldNotLook( + "the upstream this repository publishes itself as could not be read " + f"from the binary: {answered.stderr.strip() or 'no answer'}" + ) + return url - A MULTIMAP, and that is the whole design. One rule enforced by more than one - tool is the normal case, not a collision to refuse: `prevent-ai-author` is a - commit-msg hook in git-guards AND a checker in cmd-shims, because a commit - message and a pull-request body are two paths to the same public place and - each needs its own mediation. `complete-mediation` is the record that says - so. A structure holding one supplier per id would have had to pick one of - them and call the other a duplicate, which is the opposite of what the two - entries mean. - This is also what retired `tier` from a claim. The field existed because - `rule` resolved in a different namespace per tier and something had to say - which; a claim resolves against every seam at once now, and a rule with two - suppliers reports both rather than making the author choose. +def declared_claims(declaration: dict) -> list[tuple[str, str]]: + """Return (principle, rule) per entry, without judging either of them. - Returns the map and the seams that could not be read -- named rather than - folded in, because a rule absent from what could be read is not an absent - rule. + Judging them is `uphold check`. This reads the pairs so `--review` knows + which principles a rule is claimed for, and asks the binary separately + whether any seam supplies that rule. """ - suppliers: dict[str, list[str]] = {} - unreadable: list[str] = [] - for tier in TIERS: - inventory = INVENTORIES[tier](root) - if inventory.unreadable: - unreadable.append(tier) - for rule in sorted(inventory.rules): - suppliers.setdefault(rule, []).append(tier) - return suppliers, unreadable - - -def declared_claims(declaration: dict) -> list[tuple[str, str]]: - """Return (principle, rule) per entry, without judging either of them.""" entries = declaration.get("enforce", []) if not isinstance(entries, list): raise CouldNotLook("`enforce` must be an array of tables") @@ -1024,82 +211,45 @@ def declared_claims(declaration: dict) -> list[tuple[str, str]]: return claims -def format_coverage( - root: Path, declaration: dict, records: dict[str, dict] -) -> tuple[list[str], int]: - claims = declared_claims(declaration) - lines = [f"coverage in {root}"] - status = 0 - - # rule -> every principle claiming it. A list rather than a single value: - # two principles may rest on one rule, and a dict keyed by rule would have - # kept whichever claim was written last and silently dropped the other. - claimed: dict[str, list[str]] = {} - for principle, rule in claims: - claimed.setdefault(rule, []).append(principle) - - supplied: set[str] = set() - for tier in TIERS: - inventory = INVENTORIES[tier](root) - supplied |= inventory.rules - held = sorted(rule for rule in inventory.rules if rule in claimed) - unclaimed = sorted(inventory.rules - set(claimed)) - - seen = "?" if inventory.unreadable else str(len(inventory.rules)) - lines.append("") - lines.append(f"{tier}: {len(held)} of {seen} rules carry a principle") - for rule in held: - lines.append(f" claimed {rule} -> {', '.join(claimed[rule])}") - if unclaimed: - lines.append(f" unclaimed {', '.join(unclaimed)}") - for note in inventory.notes: - lines.append(f" note {note}") - if inventory.unreadable: - status = 2 - - # A claim naming a rule no seam supplies is a repository-level fact, not a - # per-seam one: without `tier` there is no seam it was pointing at to be - # missing from. Reported once, and reported here because a claim pointing at - # nothing inflates the numerator of any coverage read off this. - orphans = sorted(rule for rule in claimed if rule not in supplied) - if orphans: - lines.append("") - for rule in orphans: - lines.append( - f"claimed but supplied by nothing here: {rule} -> " - f"{', '.join(claimed[rule])}" - ) +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. - claimable = { - record_id: record - for record_id, record in records.items() - if record.get("status") != "deprecated" - and record.get("enforcement", {}).get("automatable") != "no" - } - # Intersected with what is SUPPLIED, not merely with what was claimed. The - # orphans printed immediately above are claims naming a rule no seam here - # runs, and counting them here put them back into the numerator of the one - # number a reader takes away -- a record counted as claimed by a rule that - # this very report has just said does not exist. - enforced = {principle for principle, rule in claims if rule in supplied} & set( - claimable - ) - unclaimable = len(records) - len(claimable) - lines.append("") - lines.append( - f"records: {len(enforced)} of {len(claimable)} claimable records are " - f"claimed by a rule here" - ) - if unclaimable: - lines.append( - f" {unclaimable} record(s) are deprecated or declare " - f'enforcement.automatable = "no" and can never be claimed' + `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. + """ + answered = engine(root, "check") + if answered.returncode == 2: + raise CouldNotLook(answered.stderr.strip() or "uphold check could not look") + if answered.returncode != 0 and strict: + raise Refused( + "the enforcement claims do not reconcile, so there is nothing " + f"honest to export:\n{answered.stderr.strip()}" ) - lines.append( - " an unclaimed record is not a gap to close by writing a claim: a claim " - "without a rule behind it is the failure `enforcement-needs-a-trigger` names" - ) - return lines, status + suppliers: dict[str, list[str]] = {} + for line in answered.stdout.splitlines(): + if " <- " not in line or "enforced by" not in line: + continue + _, 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) + return suppliers # --------------------------------------------------------------------------- @@ -1211,7 +361,7 @@ def build_oscal(root: Path, declaration: dict, records: dict[str, dict]) -> dict # two seams becomes an implemented-requirement under both -- which is what # an outside reader of this export needs to know, and what a single `tier` # on the claim could never have said. - suppliers, _ = rule_suppliers(root) + suppliers = engine_suppliers(root) by_tier: dict[str, list[dict]] = {} for entry in entries: for tier in suppliers.get(entry["rule"], []): @@ -1405,7 +555,7 @@ def run_review(argv: list[str]) -> int: # document exists to write. targets = [(name, emit_target(root, name)) for name in settings["emit"]] claims = declared_claims(declaration) - suppliers, _ = rule_suppliers(root) + suppliers = engine_suppliers(root, strict=False) except CouldNotLook as error: print(f"uphold review could not look: {error}", file=sys.stderr) return 2 @@ -1527,16 +677,21 @@ def main(argv: list[str]) -> int: return validate.main() - oscal_mode = argv[:1] == ["--oscal"] if argv[:1] == ["--review"]: return run_review(argv[1:]) - coverage_mode = argv[:1] == ["--coverage"] - if argv and not (oscal_mode or coverage_mode): + if argv[:1] != ["--oscal"]: + # The reconcile and the coverage report are `uphold check` and + # `uphold check --coverage`. They were here while this script + # re-implemented `config::load` to answer them; the loader answers now. print( "usage: uphold_check.py " - "[--explain ID|NAME | --list | --init | --oscal | --coverage " - "| --self-check]", + "[--explain ID|NAME | --list | --init | --oscal | --review " + "| --self-check]\n" + "\n" + "To reconcile this repository's enforcement claims:\n" + " uphold check # the claims still hold\n" + " uphold check --coverage # which rules here carry a principle", file=sys.stderr, ) return 2 @@ -1554,34 +709,18 @@ def main(argv: list[str]) -> int: try: declaration = read_toml(declaration_path) - # Before the reconcile, not after: coverage is a report over the - # configuration, and a declaration with one false claim in it is exactly - # when a reader wants to see what else is there. - if coverage_mode: - lines, status = format_coverage(root, declaration, records) - print("\n".join(lines)) - return status - failures, evidence = reconcile(root, declaration, records) + # The export gates on the reconcile and only emits what held. A + # component definition is a claim to an outside reader; exporting an + # unreconciled one would publish an assertion nobody had confirmed. + document = build_oscal(root, declaration, records) + except Refused as error: + print(f"uphold check refused: {error}", file=sys.stderr) + return 1 except CouldNotLook as error: print(f"uphold check could not look: {error}", file=sys.stderr) return 2 - if failures: - print(f"enforcement claims refused ({declaration_path}):", file=sys.stderr) - for failure in failures: - print(f"- {failure}", file=sys.stderr) - return 1 - - # The export runs the reconcile first and only emits what held. A component - # definition is a claim to an outside reader; exporting an unreconciled one - # would publish an assertion this tool had just been unable to confirm. - if oscal_mode: - print(json.dumps(build_oscal(root, declaration, records), indent=2)) - return 0 - - print(f"reconciled {len(evidence)} enforcement claims:") - for line in evidence: - print(f" {line}") + print(json.dumps(document, indent=2)) return 0 From a884fd2722892b064a5a2c4786fba90c9a97396d Mon Sep 17 00:00:00 2001 From: Tong Date: Wed, 12 Aug 2026 21:56:48 +0900 Subject: [PATCH 04/10] Point the docs at the command that answers each question `uphold check` and `uphold check --coverage` where the reconcile used to be, and a paragraph in the README on where the split falls: a mode that decides whether a check passed reads the policy, and the loader that resolves the policy is the binary. What is left in the script renders prose for a person and cannot disagree with the engine about anything. --- README.md | 9 ++++++++- docs/REFERENCE.md | 6 +++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index e236f89..cb789dd 100644 --- a/README.md +++ b/README.md @@ -86,6 +86,12 @@ could-not-look, never as a false claim. A principle with no rule yet does not belong in this file. Build the rule first. +The split is which question the mode asks. Anything that decides whether a check +passed reads the policy, and the loader that resolves the policy is the binary, +so it lives there — one answer, not two programs entitled to disagree. What is +left in the script reads the catalog and renders prose for a person, and cannot +disagree with the engine about anything. + Exit codes, everywhere: `0` clean, `1` a claim is false / a violation, `2` could not look — see [`explicit-unknown`](principles/explicit-unknown.toml). @@ -94,6 +100,8 @@ not look — see [`explicit-unknown`](principles/explicit-unknown.toml). ```sh uphold scan # content rules over the tree uphold scan --text - # a commit message, release note, PR body +uphold check # the claims in policy/upheld.toml still hold +uphold check --coverage # which rules here carry a principle uphold rules --effective # every rule inheritance resolved to, and where each runs uphold guard --stage pre-push # the guards for that git hook uphold shim gh pr create ... # stand in front of a command, then exec @@ -101,7 +109,6 @@ uphold audit --for-publication # before flipping private -> public uphold_check.py --explain ID # one record in full; also accepts a name uphold_check.py --list # every id in the catalog -uphold_check.py --coverage # which rules here carry a principle uphold_check.py --init # a starter declaration uphold_check.py --oscal # OSCAL component-definition JSON uphold_check.py --review # what routes to the review tier diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index f071cd8..4561d49 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -9,7 +9,7 @@ namespace — that is what lets a claim in - [`uphold guard` — the guards](#uphold-guard--the-guards) - [`uphold shim` — the shims](#uphold-shim--the-shims) - [`uphold audit --for-publication`](#uphold-audit---for-publication) -- [`--coverage` and `--oscal`](#--coverage-and---oscal) +- [`uphold check --coverage` and `--oscal`](#uphold-check---coverage-and---oscal) - [The review tier](#the-review-tier) ## Rule shape @@ -603,10 +603,10 @@ found, `2` where a surface this run tried to read could not be read, `0` when every surface a flip would republish was read and was clean — subject to the standing caveats, which the clean line says. -## `--coverage` and `--oscal` +## `uphold check --coverage` and `--oscal` ```sh -uphold_check.py --coverage # every rule the four tiers run, vs the claims +uphold check --coverage # every rule this repository runs, vs the claims uphold_check.py --oscal > component-definition.json ``` From 987b4b60acb637481489f03bcef590302add91be Mon Sep 17 00:00:00 2001 From: Tong Date: Wed, 12 Aug 2026 22:04:17 +0900 Subject: [PATCH 05/10] Reach the binary from a hook that has not built one CI found the constraint `content_policy_rules` documented before this port started: the modes that ask the engine run in a pre-commit environment, and the binary is not on PATH there. `--review` is a `language: system` hook in this repository's own config, so it ran on a machine holding cargo, a checkout, and every ingredient except the one command nobody had run -- and reported could-not-look on a tree that was fine. Three attempts now, in order: a built binary under `target/`, then PATH, then `cargo run --manifest-path`. The last is not a convenience. Neither caller leaves this repository -- `--review` is in no consumer's manifest and `--oscal` is run by hand -- so the fallback costs a consumer nothing and is the difference between a hook that works in a fresh checkout and one that needs a build first. A binary none of the three can produce is still could-not-look, and still exit 2. It is not a smaller answer or an older one. --- uphold_check.py | 62 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/uphold_check.py b/uphold_check.py index 5949140..2c66b70 100755 --- a/uphold_check.py +++ b/uphold_check.py @@ -147,27 +147,53 @@ def read_text(path: Path) -> str: def engine(root: Path, *args: str) -> subprocess.CompletedProcess: - """Run the binary, wherever this checkout keeps it.""" - candidates = [ + """Run the binary, wherever this checkout keeps it. + + Built binary first, then PATH, then `cargo run`. The last one matters and is + not a convenience: the modes that call this are the ones that never leave + this repository -- `--review` is in no consumer's manifest, and `--oscal` is + run by hand -- and the environment they run in is a pre-commit hook that has + not built anything. Without it the review hook fails in CI on a machine that + has cargo, a checkout, and every ingredient except the one command nobody + ran. + + A binary that cannot be produced by any of the three is could-not-look. It + is not a smaller answer or an older one; see the `explicit-unknown` record. + """ + attempts: list[list[str]] = [] + for built in ( HERE / "target" / "release" / "uphold", HERE / "target" / "debug" / "uphold", - ] - binary = next((path for path in candidates if path.is_file()), None) - found = str(binary) if binary else "uphold" - try: - return subprocess.run( - [found, *args], - cwd=root, - capture_output=True, - text=True, - check=False, + ): + if built.is_file(): + attempts.append([str(built), *args]) + attempts.append(["uphold", *args]) + if (HERE / "Cargo.toml").is_file(): + attempts.append( + [ + "cargo", + "run", + "--quiet", + "--manifest-path", + str(HERE / "Cargo.toml"), + "--", + *args, + ] ) - except OSError as error: - raise CouldNotLook( - f"`uphold {' '.join(args)}` could not be run ({error}), so what this " - f"repository enforces is unknown. Build it with `cargo build --release`, " - f"or install it on PATH." - ) from error + + reasons: list[str] = [] + for attempt in attempts: + try: + return subprocess.run( + attempt, cwd=root, capture_output=True, text=True, check=False + ) + except OSError as error: + reasons.append(f"{attempt[0]}: {error}") + raise CouldNotLook( + f"`uphold {' '.join(args)}` could not be run, so what this repository " + f"enforces is unknown ({'; '.join(reasons)}). Build it with " + f"`cargo build --release`, or install it on PATH." + ) @functools.cache From 7b7685dbfe30577f63df6bd344dea3c775b0a062 Mon Sep 17 00:00:00 2001 From: Tong Date: Wed, 12 Aug 2026 22:12:57 +0900 Subject: [PATCH 06/10] Run this repository's own reconcile through the binary that does it Three call sites still invoked `uphold_check.py` for a mode it no longer has: the `uphold-check-here` pre-commit hook, the `uphold-check` lefthook command, and the trigger list on both, which still named `.cmd-shims/checks.enabled`. They run `cargo run --quiet -- check` now, beside the `content-policy` and `guards` commands that already did. The Python tests that reach the engine skip where neither a built binary nor cargo can answer, which is the precedent `test_the_two_readers_of_the_policy_agree` already set: the catalog job runs on an image with no Rust toolchain by design, and a test needing a `cargo build` to be meaningful must not report a red suite to somebody who has not run one. What is skipped there is asserted in tests/check_cli.rs, which runs where a toolchain exists -- so the behaviour is covered, and it is covered in the language that can reach it. Verified by running the suite with `target/` moved aside and a PATH holding python3 and git and nothing else: 22 skip, the rest pass. With a toolchain, all 73 run. --- .pre-commit-config.yaml | 4 ++-- lefthook.yml | 2 +- tests/test_review.py | 23 +++++++++++++++++++++++ tests/test_uphold_check.py | 19 +++++++++++++++++++ 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 54410be..4d0026f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -226,8 +226,8 @@ repos: # `enforcement-needs-a-trigger` record refuses. - id: uphold-check-here name: this repo's own enforcement claims - entry: ./uphold_check.py + entry: cargo run --quiet -- check language: system pass_filenames: false stages: [pre-commit, manual] - files: '^(policy/upheld\.toml|policy/principles\.toml|\.pre-commit-config\.yaml|lefthook\.yml|\.cmd-shims/checks\.enabled)$' + files: '^(policy/upheld\.toml|policy/principles\.toml|\.pre-commit-config\.yaml|lefthook\.yml)$' diff --git a/lefthook.yml b/lefthook.yml index 1d22fa3..5f8c938 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -17,7 +17,7 @@ pre-commit: catalog-tests: run: python3 -m unittest discover -s tests uphold-check: - run: python3 uphold_check.py + run: cargo run --quiet -- check content-policy: run: cargo run --quiet -- scan guards: diff --git a/tests/test_review.py b/tests/test_review.py index 184c07e..89e9c7e 100644 --- a/tests/test_review.py +++ b/tests/test_review.py @@ -22,6 +22,26 @@ import review as review_mod # noqa: E402 +sys.path.insert(0, str(ROOT)) + +import uphold_check # noqa: E402 + + +def needs_the_engine(test): + """Skip where neither a built binary nor cargo can answer. + + The same reason `test_the_two_readers_of_the_policy_agree` gives: a test + that needs a `cargo build` to be meaningful must not report a red suite to + somebody who has not run one, and the catalog job runs on an image with no + Rust toolchain by design. What is skipped here is asserted in + tests/check_cli.rs, which runs where a toolchain exists. + """ + try: + uphold_check.engine(ROOT, "--version") + except uphold_check.CouldNotLook as error: + return unittest.skip(str(error))(test) + return test + def record(record_id: str, automatable: str, **extra: object) -> dict: base = { @@ -147,6 +167,7 @@ def test_no_field_beyond_those_three_crosses_over(self): self.assertNotIn("the rationale", document) +@needs_the_engine class ClaimsThatEnforceNothing(unittest.TestCase): """A claim naming a rule no seam supplies must not silence the review. @@ -211,6 +232,7 @@ def test_a_claim_no_seam_supplies_does_not_remove_its_principle_from_review(self self.assertIn("no rule here claims it", result.stderr) +@needs_the_engine class Settings(unittest.TestCase): """`[review]` is configuration, so a field of the wrong type is exit 2. @@ -347,6 +369,7 @@ def test_an_emit_name_inside_the_repository_is_written(self): self.assertTrue((Path(self.tmp) / "REVIEW.md").is_file()) +@needs_the_engine class SelfApplication(unittest.TestCase): def test_this_repository_routes_cleanly(self): result = subprocess.run( diff --git a/tests/test_uphold_check.py b/tests/test_uphold_check.py index 41becde..6bd1445 100644 --- a/tests/test_uphold_check.py +++ b/tests/test_uphold_check.py @@ -22,6 +22,23 @@ import uphold_check # noqa: E402 + +def needs_the_engine(test): + """Skip where neither a built binary nor cargo can answer. + + The same reason `test_the_two_readers_of_the_policy_agree` gives: a test + that needs a `cargo build` to be meaningful must not report a red suite to + somebody who has not run one, and the catalog job runs on an image with no + Rust toolchain by design. What is skipped here is asserted in + tests/check_cli.rs, which runs where a toolchain exists. + """ + try: + uphold_check.engine(ROOT, "--version") + except uphold_check.CouldNotLook as error: + return unittest.skip(str(error))(test) + return test + + # The guards are this repository's own rules now, so the seam that supplies one # is `uphold`, and what installs it is a PUBLISHED hook id -- what a # consumer actually writes. The fixture used to name a local hook called @@ -117,6 +134,7 @@ def build(directory: Path, declaration: str, **files: str) -> None: path.write_text(body, encoding="utf-8") +@needs_the_engine class NoProseInRuntime(unittest.TestCase): """`enforcement-needs-a-trigger`: the tool must not carry principle text.""" @@ -175,6 +193,7 @@ def test_the_review_tier_may_not_exist_without_its_ceiling(self): self.assertIn("Do not raise the ceiling", result.stderr) +@needs_the_engine class OscalExport(unittest.TestCase): """The mapping crosses to OSCAL; the records deliberately do not.""" From c76d78b8dd802cc21c49cf6e99a21adb9fa2d02b Mon Sep 17 00:00:00 2001 From: Tong Date: Wed, 12 Aug 2026 22:20:14 +0900 Subject: [PATCH 07/10] Recognise a seam by the id that installs it, not by a repository's name The port scoped hook ids by the `repo:` url they were pinned under, on the reasoning that another repository's `uphold-scan` establishes nothing here. The reasoning is fine and the predicate is wrong, which the consumer harness caught: `scripts/consumer_check.sh` clones this repository into a temporary directory and pins it by PATH, so the last segment of its `repo:` is `hooks`. Every guard the consumer pinned was read as belonging to somebody else, and a true claim was refused in the one job that drives a real consumer end to end. The id is specific enough on its own -- `uphold-guard-push` is this binary's name for this binary's stage -- and the manifest is where the list comes from, so an id cannot be published there and forgotten here. This is what the test named `the_seam_is_found_by_a_published_id_not_by_one_repositorys_name` has been saying since before the port; I ported the name and inverted the behaviour. It now asserts what it says, over every published guard id and against a `repo:` url naming no owner at all. `names_this_repository` stays for the lefthook `remotes:` block, where the url is the only thing there is to match on and both halves of one entry have to agree. --- src/check.rs | 48 ++++++++++++++++++++++------------------- tests/check_cli.rs | 53 +++++++++++++++++++++++++++++++++------------- 2 files changed, 64 insertions(+), 37 deletions(-) diff --git a/src/check.rs b/src/check.rs index fb9f23d..bb33234 100644 --- a/src/check.rs +++ b/src/check.rs @@ -192,8 +192,10 @@ struct PreCommitConfig { #[derive(Debug, Deserialize)] struct PreCommitRepo { - #[serde(default)] - repo: String, + // No `repo:` field. Which repository an id came from is deliberately not + // consulted -- see `pinned_ids`. `repo: local` entries are read the same + // way as any other, because a local hook is a rule that fires here and a + // claim may name it. #[serde(default)] hooks: Vec, } @@ -204,15 +206,22 @@ struct PinnedHook { id: String, } -/// Hook ids from a pre-commit config: `(this repository's, every one).` +/// Every hook id a pre-commit config installs, from any repository. +/// +/// By ID and not by repository url, which is the whole of what +/// `published_seams` is for. The predicate this replaced was a repository name, +/// and a consumer does not necessarily write one a matcher would recognise: +/// `scripts/consumer_check.sh` clones this repository to a temporary directory +/// and pins it by PATH, so the last segment of its `repo:` is `hooks`. Scoping +/// on the url told that consumer the seam supplying every guard was absent. /// -/// Two sets, because they answer two questions. Which of THIS binary's seams -/// are installed is evidence only an id of this binary's can give -- a consumer -/// pinning some other repository's `uphold-scan` establishes nothing here. But -/// a claim may also name a rule that is not this binary's at all: a local hook, -/// or a formatter from a third-party repository, is a rule that fires here and -/// can be claimed as one. That is the `local` tier, and it is every id. -fn pinned_ids(root: &Path) -> Result, BTreeSet)>> { +/// An id is specific enough on its own. `uphold-guard-push` is this binary's +/// name for this binary's stage, and the manifest is where the list comes from +/// so a new id cannot be published there and forgotten here. +/// +/// The same set answers the `local` tier: a claim may name a formatter, a +/// linter, or a hook this repository wrote, and those are rules that fire here. +fn pinned_ids(root: &Path) -> Result>> { let path = root.join(".pre-commit-config.yaml"); if !path.is_file() { return Ok(None); @@ -228,18 +237,13 @@ fn pinned_ids(root: &Path) -> Result, BTreeSet) could-not-look", )); }; - let mut ours = BTreeSet::new(); let mut every = BTreeSet::new(); for entry in repos { - let mine = names_this_repository(&entry.repo); for hook in entry.hooks { - if mine { - ours.insert(hook.id.clone()); - } every.insert(hook.id); } } - Ok(Some((ours, every))) + Ok(Some(every)) } #[derive(Debug, Deserialize)] @@ -406,20 +410,20 @@ pub(crate) fn installed(root: &Path) -> Result { let mut found = Installed::default(); match pinned_ids(root) { - Ok(Some((ours, every))) => { - found.scan = ours.iter().any(|id| scans.contains(id)); + Ok(Some(ids)) => { + found.scan = ids.iter().any(|id| scans.contains(id)); for (stage, hook) in &guards { - if ours.contains(hook) { + if ids.contains(hook) { found.stages.insert(stage.clone()); } } - found.local.extend(every); - let mut named: Vec<&str> = ours + let mut named: Vec = ids .iter() .filter(|id| scans.contains(*id) || guards.values().any(|hook| hook == *id)) - .map(String::as_str) + .cloned() .collect(); named.sort_unstable(); + found.local.extend(ids); if !named.is_empty() { found .how diff --git a/tests/check_cli.rs b/tests/check_cli.rs index dee54d6..364a941 100644 --- a/tests/check_cli.rs +++ b/tests/check_cli.rs @@ -370,21 +370,44 @@ fn a_claim_on_a_shim_only_rule_is_refused_and_not_credited_to_the_scan() { #[test] fn the_seam_is_found_by_a_published_id_not_by_one_repositorys_name() { - // A consumer pinning some OTHER repository's `uphold-scan` establishes - // nothing about this binary's seams. - let root = workspace(); - write(&root, "policy/principles.toml", GUARD_POLICY); - write( - &root, - ".pre-commit-config.yaml", - "repos:\n - repo: https://github.com/somebody-else/uphold\n rev: v1.0.0\n hooks:\n - id: uphold-guard-push\n", - ); - write( - &root, - "policy/upheld.toml", - "[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"prevent-public-push\"\n", - ); - assert_eq!(code(&check(&root, &[])), 1); + // Every published guard id has to make its own stage visible, and the id is + // the whole of what is matched. The predicate this replaced was a + // repository NAME, and a consumer does not necessarily write one a matcher + // would recognise: `scripts/consumer_check.sh` clones this repository to a + // temporary directory and pins it by path, so the last segment of its + // `repo:` is `hooks`. Scoping on the url told that consumer the seam + // supplying every guard was absent. + for (stage, id) in [ + ("pre-commit", "uphold-guard"), + ("commit-msg", "uphold-guard-commit-msg"), + ("pre-merge-commit", "uphold-guard-merge"), + ("pre-push", "uphold-guard-push"), + ] { + let root = workspace(); + write( + &root, + "policy/principles.toml", + &format!( + "[rule.prevent-unusual-unicode-in-files]\n\ + builtin = \"prevent-unusual-unicode-in-files\"\n\ + git.hooks = [\"{stage}\"]\n" + ), + ); + write( + &root, + ".pre-commit-config.yaml", + &format!( + "repos:\n - repo: /tmp/some-checkout/hooks\n rev: v1.0.0\n hooks:\n - id: {id}\n" + ), + ); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"complete-mediation\"\nrule = \"prevent-unusual-unicode-in-files\"\n", + ); + let output = check(&root, &[]); + assert_eq!(code(&output), 0, "{stage}/{id}: {}", stderr(&output)); + } } #[test] From 8200b04909b0078e4361fb50e6e94a9837e41783 Mon Sep 17 00:00:00 2001 From: Tong Date: Wed, 12 Aug 2026 22:21:16 +0900 Subject: [PATCH 08/10] Give the consumer integration job the binary it now checks with The job proved the reconcile by running `uphold_check.py` from a directory that is not this repository, which is the right shape and the wrong entry point: the reconcile is `uphold check`. It runs on a toolchain-free image, so it builds one, and builds rather than pinning a release because what is under test is this commit's reconcile against this commit's manifest. The consumer also gets a `policy/principles.toml`. It had none, and did not need one while the reader tolerated a repository with no policy; the loader does not, and a repository with nothing to resolve has nothing to reconcile against. Both steps verified locally against the built binary: the starter declaration reconciles at exit 0 in a tree with no runner configuration, and a directory with no declaration at all is still exit 2. --- .github/workflows/test.yml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ba78641..33ee33f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -310,6 +310,12 @@ jobs: steps: - uses: actions/checkout@v7 + # The reconcile is the binary's now, so this job needs one. It builds + # rather than pinning a release because what is under test is this + # commit's reconcile against this commit's manifest. + - uses: dtolnay/rust-toolchain@stable + - run: cargo build --release + # A hook is only proven by the path a consumer takes: a repository that # is not this one, declaring a profile, running the released entry point. - name: A consuming repository's declaration @@ -317,7 +323,8 @@ jobs: set -euo pipefail mkdir -p /tmp/consumer/policy ./uphold_check.py --init > /tmp/consumer/policy/upheld.toml - cd /tmp/consumer && "$GITHUB_WORKSPACE/uphold_check.py" + cp -r policy/principles.toml /tmp/consumer/policy/principles.toml + cd /tmp/consumer && "$GITHUB_WORKSPACE/target/release/uphold" check - name: A declaration that cannot be read is not a pass run: | @@ -325,7 +332,7 @@ jobs: mkdir -p /tmp/unreadable cd /tmp/unreadable set +e - "$GITHUB_WORKSPACE/uphold_check.py" + "$GITHUB_WORKSPACE/target/release/uphold" check status=$? set -e test "$status" -eq 2 || { echo "expected exit 2, got $status"; exit 1; } From 7c2244b81ef81efb204a455c4f7ba2aac9e2e32f Mon Sep 17 00:00:00 2001 From: Tong Date: Wed, 12 Aug 2026 22:28:13 +0900 Subject: [PATCH 09/10] Put the declaration check on PATH, where the other commands already are `hooks/lefthook.yml` -- the config a lefthook consumer inherits -- still ran `uphold_check.py` through `.lefthook/pre-commit/uphold-check`, so a clean commit in the lefthook consumer was refused by a usage message. That wrapper existed for one reason, written at the top of it: "uphold_check.py is not in the binary, so it has to be reached IN THIS REPOSITORY -- and lefthook's `scripts` is the one mechanism that resolves against the remote clone rather than against the consumer's own tree." The reconcile is `uphold check` now and PATH reaches it, so the job is a plain `run:` beside `uphold scan` and `uphold guard`, and the wrapper is deleted. It stays a `jobs:` entry rather than a command, for the reason already recorded: `glob` is a job key, and a check with no firing condition would load the catalog in front of every one-line fix while `.pre-commit-hooks.yaml` publishes the opposite as the design. The shell-lint globs in lefthook.yml go back to `scripts/*.sh`. The brace pattern was there to catch an extensionless wrapper that no longer exists. --- .lefthook/pre-commit/uphold-check | 15 ------------- .pre-commit-config.yaml | 11 ++++----- hooks/lefthook.yml | 37 +++++++++++++------------------ lefthook.yml | 11 ++++----- 4 files changed, 27 insertions(+), 47 deletions(-) delete mode 100755 .lefthook/pre-commit/uphold-check diff --git a/.lefthook/pre-commit/uphold-check b/.lefthook/pre-commit/uphold-check deleted file mode 100755 index 887c710..0000000 --- a/.lefthook/pre-commit/uphold-check +++ /dev/null @@ -1,15 +0,0 @@ -#!/bin/sh -# The declaration check, reachable from a lefthook consumer. -# -# Everything else this repository publishes is the `uphold` binary, which a -# consumer puts on PATH. uphold_check.py is not in the binary, so it has to -# be reached IN THIS REPOSITORY -- and lefthook's `scripts` is the one mechanism -# that resolves against the remote clone rather than against the consumer's own -# tree. Hence a wrapper: lefthook can only run a file it finds under source_dir, -# and the checker is two levels up from here. -# -# The working directory is the CONSUMING repository, which is what the checker -# reads. Only the script's own location comes from the clone. -set -eu -here=$(CDPATH='' cd -- "$(dirname -- "$0")" && pwd) -exec python3 "$here/../../uphold_check.py" "$@" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4d0026f..8fbf888 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -56,11 +56,12 @@ repos: hooks: - id: actionlint - # The shell this repository ships is the part a consumer runs before anything - # else: the lefthook wrapper it publishes under `scripts:`, and the consumer - # harness that decides whether the three runners agree. Both were unlinted, - # and the first thing this hook found was a `CDPATH=` in the published wrapper - # that reads as an assignment and is a command prefix. + # The shell this repository ships is the consumer harness that decides whether + # the three runners agree, which is the part a consumer's experience rests on. + # It was unlinted, alongside a published lefthook wrapper this hook found a + # `CDPATH=` in -- reading as an assignment where it is a command prefix. The + # wrapper is gone with the Python reconciler it existed to reach; the harness + # is not. - repo: https://github.com/shellcheck-py/shellcheck-py rev: v0.11.0.1 hooks: diff --git a/hooks/lefthook.yml b/hooks/lefthook.yml index c07953e..97adf7a 100644 --- a/hooks/lefthook.yml +++ b/hooks/lefthook.yml @@ -34,32 +34,25 @@ pre-commit: run: uphold scan uphold-guard: run: uphold guard --stage pre-commit - # The declaration check is the one part of this repository that is not the - # binary, so PATH cannot reach it. lefthook resolves a script against the - # remote clone rather than the consumer's tree, which is the only mechanism - # here that can reach a file in THIS repository -- so the checker arrives as a - # script and .lefthook/pre-commit/uphold-check is a shim onto it. + # The declaration check fires on a CONDITION rather than on every commit, which + # is why it is a job and not another command: `glob` is a job key. # - # It is a `jobs:` entry rather than a `scripts:` one for a single reason: - # `glob` is a job key and a script has no equivalent. A script with no firing - # condition runs on EVERY commit, which loads and validates the whole catalog - # in front of a one-line fix -- and .pre-commit-hooks.yaml publishes the - # opposite as the design ("It does not run on every commit") and holds itself - # to it with a `files:` regex. Two distribution paths that fire on different - # occasions are two products wearing one version number, and the one that - # fires more is the one a consumer switches off. + # It arrived as a `script:` onto `.lefthook/pre-commit/uphold-check` while the + # checker was a Python file in this repository that PATH could not reach -- + # lefthook's `scripts` is the one mechanism that resolves against the remote + # clone rather than the consumer's tree. The reconcile is `uphold check` now, + # so it is on PATH like every other command here and the shim is gone. # - # The list is the same list as that regex, file for file: the declaration - # itself, plus every file a claim is reconciled against. Those are exactly the - # edits that can turn a true enforcement claim into a false one -- a rule - # deleted from policy/principles.toml, a hook id dropped from a runner's - # config, a shim check disabled. Nothing else can, which is why nothing else - # is worth a catalog load. + # .pre-commit-hooks.yaml publishes the same condition as a `files:` regex, and + # the two lists are the same list file for file: the declaration itself, plus + # every file a claim is reconciled against. Those are exactly the edits that + # can turn a true enforcement claim into a false one. Two distribution paths + # that fire on different occasions are two products wearing one version + # number, and the one that fires more is the one a consumer switches off. jobs: - name: uphold-check - script: "uphold-check" - runner: sh - glob: "{policy/upheld.toml,policy/principles.toml,.pre-commit-config.yaml,lefthook.yml,.cmd-shims/checks.enabled}" + run: uphold check + glob: "{policy/upheld.toml,policy/principles.toml,.pre-commit-config.yaml,lefthook.yml}" commit-msg: commands: diff --git a/lefthook.yml b/lefthook.yml index 5f8c938..0516494 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -45,14 +45,15 @@ pre-commit: glob: ".github/workflows/*.{yml,yaml}" run: actionlint {staged_files} # The two shell files this repository ships: the consumer harness and the - # wrapper a lefthook consumer runs out of the clone. The brace pattern is - # what keeps this hook and the pre-commit one asking the same question -- - # `*.sh` alone would miss the wrapper, which has no extension. + # extensionless wrapper a lefthook consumer ran out of the clone, which + # `*.sh` alone would have missed. That wrapper existed to reach a Python + # reconciler PATH could not; the reconcile is `uphold check` now, the + # directory is gone, and the pattern is the plain one again. shellcheck: - glob: "{scripts/*.sh,.lefthook/pre-commit/*}" + glob: "scripts/*.sh" run: shellcheck {staged_files} bashate: - glob: "{scripts/*.sh,.lefthook/pre-commit/*}" + glob: "scripts/*.sh" run: bashate --ignore E006 {staged_files} # Not git hooks. lefthook has no equivalent of pre-commit's manual stage, so the From 5745367c7593b415144424c0e92da3c125c8a00b Mon Sep 17 00:00:00 2001 From: Tong Date: Wed, 12 Aug 2026 22:37:53 +0900 Subject: [PATCH 10/10] Reject a remote that is identifiably somebody else, not one that is a path The port asked "does this url name us", and most git urls cannot answer it. lefthook takes any of them, and `scripts/consumer_check.sh` clones this repository to a neutral `$WORK/hooks` on purpose -- so the url its consumer writes carries neither the owner nor the repository name, every guard went unrecognised, and a true claim was refused. Answering exit 1 there says the claim is FALSE about a repository whose only fault is cloning from a path. The question is the other way round, as it was before the port: a remote is rejected only when it spells a forge `owner/name` and that pair is not ours. Anything without a host is a path, and a path is unidentifiable rather than foreign. Both spellings git accepts for a host are read. The load-bearing half is untouched: the remote and `hooks/lefthook.yml` must appear in the SAME entry, so a fork pinning its own config is still not credited with running every guard here. --- src/check.rs | 58 +++++++++++++++++++++++++-------------------- tests/check_cli.rs | 59 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 25 deletions(-) diff --git a/src/check.rs b/src/check.rs index bb33234..43318ff 100644 --- a/src/check.rs +++ b/src/check.rs @@ -51,40 +51,48 @@ fn upstream_slug() -> Option<(&'static str, &'static str)> { (!owner.is_empty() && !name.is_empty()).then_some((owner, name)) } -/// Does this url name THIS repository? +/// Is this url NOT identifiably somebody else's? /// -/// `owner/name`, or the bare `name`. lefthook takes any git url and most carry -/// no owner: `scripts/consumer_check.sh` points its consumer at a clone by -/// FILESYSTEM PATH, and requiring the slug reported that consumer as running no -/// seam at all -- so the one CI job that drives a real lefthook consumer refused -/// a clean commit. +/// The question is deliberately that way round. lefthook takes any git url, and +/// most cannot answer "does this name us" at all: a consumer may clone from a +/// filesystem path, a mirror, or a bare directory whose name says nothing. +/// `scripts/consumer_check.sh` clones to a neutral `$WORK/hooks` on purpose, so +/// the url it writes carries neither the owner nor the repository name. +/// Demanding the slug there demands evidence the format does not carry, and +/// answering "no seam here supplies it" is answering exit 1 -- the claim is +/// FALSE -- about a repository whose only fault is cloning from a path. /// -/// The last path segment has to match exactly. A url ending `my-uphold-fork` is -/// not this repository, and a substring test said it was. +/// So a remote is rejected only when it spells a forge `owner/name` and that +/// pair is not ours. Anything without a host is a path, and a path is +/// unidentifiable rather than foreign. +/// +/// The load-bearing half is elsewhere and untouched: the remote and +/// `hooks/lefthook.yml` must appear in the SAME entry, so a fork pinning its own +/// config, or an unrelated project pulling a file that happens to share the +/// conventional name, is still not credited with running every guard here. fn names_this_repository(url: &str) -> bool { let trimmed = url.trim().trim_end_matches('/').trim_end_matches(".git"); let Some((owner, name)) = upstream_slug() else { return false; }; - let Some((before, last)) = trimmed.rsplit_once('/') else { - // A bare name and nothing else. - return trimmed == name; + // A host, in either spelling git accepts: `scheme://host/owner/name` and + // `user@host:owner/name`. Without one there is no owner to compare. + let after_host = if let Some((_, rest)) = trimmed.split_once("://") { + rest.split_once('/').map(|(_, path)| path) + } else if let Some((_, rest)) = trimmed.split_once('@') { + rest.split_once(':').map(|(_, path)| path) + } else { + return true; }; - // Exactly the last segment, never a substring: a url ending - // `my-uphold-fork` is not this repository, and a `contains` said it was. - if last != name { - return false; - } - // A url that names a HOST names an owner too, and a fork under another - // owner publishes the same name while being a different repository. A - // filesystem path names no owner at all, which is what - // `scripts/consumer_check.sh` writes -- so the segment before the name is - // asked to match only where there is a host for it to belong to. - let remote = trimmed.contains("://") || trimmed.contains('@'); - if !remote { + let Some(path) = after_host else { return true; - } - before.rsplit('/').next() == Some(owner) + }; + let mut segments = path.rsplit('/'); + let (Some(last), Some(before)) = (segments.next(), segments.next()) else { + // A host and one segment names no owner, so it identifies nobody. + return true; + }; + before == owner && last == name } /// One `[[enforce]]` entry, as written. diff --git a/tests/check_cli.rs b/tests/check_cli.rs index 364a941..cfe0932 100644 --- a/tests/check_cli.rs +++ b/tests/check_cli.rs @@ -710,3 +710,62 @@ fn the_output_carries_no_record_prose() { ); } } + +#[test] +fn a_remote_cloned_to_a_neutral_path_is_not_somebody_elses() { + // The question is "is this identifiably somebody else's", not "does this + // name us", because most git urls cannot answer the second. + // `scripts/consumer_check.sh` clones to a neutral `$WORK/hooks` on purpose, + // so the url a lefthook consumer writes carries neither the owner nor the + // repository name. Demanding the slug there demanded evidence the format + // does not carry, and answered exit 1 -- the claim is FALSE -- about a + // repository whose only fault was cloning from a path. + for url in [ + "/tmp/tmp.Lwu7lqsxpk/hooks", + "/srv/example/some-checkout", + "../a-sibling-clone", + ] { + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write( + &root, + "lefthook.yml", + &format!( + "remotes:\n - git_url: {url}\n ref: v1.0.0\n configs:\n - hooks/lefthook.yml\n" + ), + ); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"prevent-public-push\"\n", + ); + let output = check(&root, &[]); + assert_eq!(code(&output), 0, "{url}: {}", stderr(&output)); + } +} + +#[test] +fn a_forge_url_under_another_owner_is_somebody_elses() { + // The one case a url CAN answer: it spells `owner/name` and the pair is not + // ours, in either spelling git accepts. + for url in [ + "https://github.com/somebody-else/uphold", + "git@github.com:somebody-else/uphold.git", + ] { + let root = workspace(); + write(&root, "policy/principles.toml", GUARD_POLICY); + write( + &root, + "lefthook.yml", + &format!( + "remotes:\n - git_url: {url}\n ref: v1.0.0\n configs:\n - hooks/lefthook.yml\n" + ), + ); + write( + &root, + "policy/upheld.toml", + "[[enforce]]\nprinciple = \"fail-safe-defaults\"\nrule = \"prevent-public-push\"\n", + ); + assert_eq!(code(&check(&root, &[])), 1, "{url}"); + } +}