From d7206aa3e59f23e398e7ec26db0155d41b290986 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sun, 23 Aug 2026 10:39:58 +0000 Subject: [PATCH 01/14] refactor(hook): ReceiptFacts and KeyFacts are three-valued on facts::Look MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-757's fifth acceptance clause, filed rather than skipped when hook.rs was held by a concurrent session. `facts::Look` stated the three-valued contract and these two aliases went on practising `Option`, so the rule was stated in one file and broken in another: a reader reaching for `Option` got two values where the contract has three. The conflation was already written down. `ReceiptFacts`' own doc listed TWO things resolving to `None` — "could not look" (no checkout, an unresolvable `origin/main`, a detached HEAD) and "nothing judgeable" (a path policy does not judge, CLOUD-444's exclusion set) — and had no way to spell them apart. They are `CouldNotLook` and `IsNot` now, and the producers in `lib.rs` are where the split becomes real rather than nominal: IsNot no required check selected this call, or the write lands where policy does not judge; the boundary looked. CouldNotLook the receipt store could not be read at all. `KeyFacts` splits on the same seam: no `requires_key` row selected the command is `IsNot`, while no checkout, a shallow clone and an unresolvable `base` are the could-not-look arm `key_facts` now returns as one. BEHAVIOUR AND BYTES ARE UNCHANGED, WHICH IS THE POINT OF A TYPE SUBSTITUTION. Both non-answers still ALLOW in `receipt_rules` and `key_present` — the fail-open posture every retiring guard has — and the policy-input projection maps both to `null`, exactly what the `Option` spelling emitted. Giving Rego its own spelling of the distinction would be a widening of the document and belongs to whichever row needs a predicate on it, not to this one. `AgentFacts` deliberately stays an `Option`, and its doc now says why rather than pointing at `ReceiptFacts` for a `None` that no longer exists there: it has no `IsNot` producer, so the third arm would be dead, and a three-valued type whose third value cannot occur misleads in the other direction. Refs: CLOUD-787 --- crates/batten/src/hook.rs | 137 +++++++++++++++++++++++++------------- crates/batten/src/lib.rs | 49 ++++++++++---- fuzz/Cargo.lock | 2 +- 3 files changed, 126 insertions(+), 62 deletions(-) diff --git a/crates/batten/src/hook.rs b/crates/batten/src/hook.rs index 63648fef3..fbc30572c 100644 --- a/crates/batten/src/hook.rs +++ b/crates/batten/src/hook.rs @@ -3072,27 +3072,42 @@ pub fn deny_text(refusal: &Refusal) -> String { /// a rule about rules that the config does not state. /// The receipt verdicts a mediated call is judged against. /// -/// `None` is **no receipt question to answer here**, and allows — the fail-open -/// posture every retiring guard has. Two things resolve to it, and both are the -/// boundary's to decide because both are questions about a checkout: -/// -/// * **could not look** — no checkout, an `origin/main` that does not resolve, or -/// a detached HEAD where a branch-keyed row needs a branch; -/// * **nothing judgeable** — the call writes a path policy does not judge -/// (git-ignored, outside the repository, inside `.git`), which is -/// [`crate::receipt::judgeable`]'s answer and CLOUD-444's exclusion set. -/// -/// `Some` map missing a name is treated as [`Validity::Missing`], so a boundary -/// that resolved fewer facts than the policy needs fails closed rather than -/// silently allowing. -pub type ReceiptFacts = Option>; +/// Three-valued on [`crate::facts::Look`], which is the contract `facts.rs` +/// states and this alias now practises rather than restates (CLOUD-787). The +/// two answers that are not a verdict map both **allow** — the fail-open posture +/// every retiring guard has — and they are still two answers, because the +/// previous spelling had them as one: +/// +/// * [`Look::CouldNotLook`](crate::facts::Look::CouldNotLook) — the question +/// could not be asked: no checkout, an `origin/main` that does not resolve, or +/// a detached HEAD where a branch-keyed row needs a branch. +/// * [`Look::IsNot`](crate::facts::Look::IsNot) — the boundary looked and there +/// is **nothing judgeable** here: no required check selected this call, or the +/// call writes a path policy does not judge (git-ignored, outside the +/// repository, inside `.git`), which is [`crate::receipt::judgeable`]'s answer +/// and CLOUD-444's exclusion set. +/// +/// Both were `None` until CLOUD-787, so a reader reaching for `Option` got two +/// values where the contract has three, and the next call site written would +/// have collapsed "looked and found nothing" into "could not look". They allow +/// alike today and nothing here asks them to keep doing so: the point is that a +/// predicate can now tell them apart before deciding. +/// +/// An [`Look::Is`](crate::facts::Look::Is) map missing a name is treated as +/// [`Validity::Missing`], so a boundary that resolved fewer facts than the +/// policy needs fails closed rather than silently allowing. +pub type ReceiptFacts = crate::facts::Look>; /// The checkout evidence a `requires_key` shape row is judged against /// (CLOUD-446): the branch name, and the commit messages on `base..HEAD`. /// -/// `None` is **could not look** and allows, exactly as it does for -/// [`ReceiptFacts`] — outside a checkout, on a detached HEAD, or against a `base` -/// git cannot resolve. Resolved at the boundary because [`adjudicate`] is +/// [`Look::CouldNotLook`](crate::facts::Look::CouldNotLook) allows, exactly as +/// it does for [`ReceiptFacts`] — outside a checkout, on a detached HEAD, in a +/// shallow clone, or against a `base` git cannot resolve. +/// [`Look::IsNot`](crate::facts::Look::IsNot) is the other non-answer and is a +/// different one: no `requires_key` row selected this command, so the boundary +/// looked and found no key question to ask. Resolved at the boundary because +/// [`adjudicate`] is /// contractually pure, and resolved only when a `requires_key` row has already /// selected this command ([`Policy::key_base_for`]), so a repository declaring no /// such row pays nothing on the hottest path in the binary. @@ -3101,12 +3116,17 @@ pub type ReceiptFacts = Option>; /// reader asks the same question of all of it — does the expression match /// anywhere — and a field a caller could *print* is one an unreviewed edit turns /// into a leaked commit message (non-negotiable rule 4). -pub type KeyFacts = Option>; +pub type KeyFacts = crate::facts::Look>; /// What the AGENT reported for each agent-sourced check (CLOUD-776, CLOUD-834). /// -/// `None` is could-not-look, exactly as [`ReceiptFacts`]'s is; a check absent -/// from the map is one the boundary was not asked about. Kept beside the +/// `None` here is could-not-look. It is deliberately still an `Option` rather +/// than a [`crate::facts::Look`], and CLOUD-787 left it that way on purpose: +/// this fact has no `IsNot` producer — [`crate::lib`]'s `agent_records` answers +/// `None` only when no required check is agent-sourced — so the third arm would +/// be dead, and a three-valued type whose third value cannot occur misleads in +/// the other direction. A check absent from the map is one the boundary was not +/// asked about. Kept beside the /// receipt verdicts rather than folded into them because they answer different /// questions: a `Validity` says whether the check is satisfied, and this says /// what the agent actually ran and what it found. Collapsing the two would give @@ -3259,8 +3279,9 @@ pub type ManifestFacts = Option; /// **Resolved at the boundary, held by value.** Every field is a fact the /// boundary already looked up before [`adjudicate`] was called — which is what /// keeps that function contractually pure, and why projecting the set into the -/// policy input costs no resolution. `Option::None` is could-not-look -/// throughout, never "resolved to nothing". +/// policy input costs no resolution. A fact that was not resolved says so in its +/// own type — [`crate::facts::Look::CouldNotLook`] where the field carries one, +/// `Option::None` where it does not — and never "resolved to nothing". #[derive(Debug, Clone, Copy)] #[non_exhaustive] pub struct Facts<'a> { @@ -3305,8 +3326,8 @@ impl<'a> Facts<'a> { ) -> Facts<'a> { Facts { bypass: false, - receipts: &None, - keys: &None, + receipts: &crate::facts::Look::CouldNotLook, + keys: &crate::facts::Look::CouldNotLook, stop, waived, sourced: &None, @@ -3436,8 +3457,13 @@ fn matching_receipt_rows<'a>(policy: &'a Policy, envelope: &Envelope) -> Vec<&'a /// not a tidiness: this gate sees a row only if that row's own `tool` selected /// this call. fn tool_receipt_rules(policy: &Policy, envelope: &Envelope, facts: &ReceiptFacts) -> Decision { - let Some(facts) = facts.as_ref() else { - return Decision::Allow; + // Both non-answers allow, and for the same reason [`receipt_rules`] gives + // them one arm: a boundary that had no question to ask and a boundary that + // could not ask reach the same verdict here, and neither is evidence a + // receipt is missing (CLOUD-787). + let facts = match facts { + crate::facts::Look::Is(resolved) => resolved, + crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => return Decision::Allow, }; if envelope.raw_tool.is_empty() { return Decision::Allow; @@ -3465,11 +3491,14 @@ fn tool_receipt_rules(policy: &Policy, envelope: &Envelope, facts: &ReceiptFacts } fn receipt_rules(policy: &Policy, envelope: &Envelope, facts: &ReceiptFacts) -> Decision { - // No facts means there is no receipt question to answer here. Allow: a guard - // that cannot read its own precondition must not become the reason work - // stops. - let Some(facts) = facts.as_ref() else { - return Decision::Allow; + // Neither non-answer is a receipt question to answer here, and both allow: a + // guard that cannot read its own precondition must not become the reason + // work stops, and neither must one told there is nothing to judge. They + // reach the same verdict by two different routes, which is the distinction + // `Option` could not hold (CLOUD-787). + let facts = match facts { + crate::facts::Look::Is(resolved) => resolved, + crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => return Decision::Allow, }; for rule in matching_receipt_rows(policy, envelope) { // Every named receipt must be valid. An unresolved name is Missing, @@ -4342,9 +4371,18 @@ fn call_document(envelope: &Envelope, facts: &Facts<'_>) -> Result Some(serde_json::Value::Bool(facts.bypass)), - crate::facts::Fact::Receipts => Some(facts.receipts.as_ref().map_or( - serde_json::Value::Null, - |verdicts| { + // BOTH non-answers project as `null`, which is what keeps this a + // type substitution rather than a document change (CLOUD-787): the + // previous `Option` spelling emitted `null` for could-not-look and + // for nothing-judgeable alike, and the bytes here are unchanged. The + // distinction is live in Rust, where the next call site is written; + // giving Rego its own spelling of it is a widening of the policy + // input and belongs to whichever row needs a predicate on it. + crate::facts::Fact::Receipts => Some(match facts.receipts { + crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => { + serde_json::Value::Null + } + crate::facts::Look::Is(verdicts) => { // The VERDICT's stable token, never the receipt statement: // a receipt carries a subject commit and a recorded ref, and // a predicate decides on `valid` / `stale-head` / @@ -4355,14 +4393,15 @@ fn call_document(envelope: &Envelope, facts: &Facts<'_>) -> Result Some( - facts - .keys - .as_ref() - .map_or(serde_json::Value::Null, |found| serde_json::json!(found)), - ), + } + }), + // Same reading as `Receipts` one arm up, and the same unchanged bytes. + crate::facts::Fact::Keys => Some(match facts.keys { + crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => { + serde_json::Value::Null + } + crate::facts::Look::Is(found) => serde_json::json!(found), + }), crate::facts::Fact::Stop => Some(serde_json::json!(facts.stop)), crate::facts::Fact::Waived => Some(serde_json::json!(facts.waived)), crate::facts::Fact::AgentSourced => Some(facts.sourced.as_ref().map_or( @@ -4501,10 +4540,14 @@ fn call_document(envelope: &Envelope, facts: &Facts<'_>) -> Result bool { if pattern.is_match(command) { return true; } - let Some(evidence) = keys else { + let crate::facts::Look::Is(evidence) = keys else { return true; }; evidence.iter().any(|text| pattern.is_match(text)) diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index d9acf5c0d..3eb06f0c1 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -2071,13 +2071,18 @@ fn receipt_facts( max_ages: &std::collections::BTreeMap, judgeable: bool, ) -> hook::ReceiptFacts { + // The two non-answers are DIFFERENT answers, and CLOUD-787 is where they + // stopped being one. `IsNot` is the boundary having looked: no required + // check selected this call, or the write lands somewhere policy does not + // judge. `CouldNotLook` is further down, where the receipt store itself + // could not be read. Both allow, and they are not the same fact. if required.is_empty() || !judgeable { - return None; + return facts::Look::IsNot; } let mut verdicts = if receipted.is_empty() { - // Nothing to ask the receipt store. An EMPTY map rather than `None`: - // `None` is "could not look" and allows, and here we looked — there - // simply were no receipt-keyed checks among the ones required. + // Nothing to ask the receipt store, having looked: there simply were no + // receipt-keyed checks among the ones required. An EMPTY map, not a + // non-answer — the agent-sourced loop below fills it in. Some(std::collections::BTreeMap::new()) } else { // The subject a `named`-keyed row files under, resolved HERE because @@ -2111,7 +2116,9 @@ fn receipt_facts( verdicts.insert((*check).clone(), verdict); } } - verdicts + // A store the boundary could not read is the could-not-look arm — the one + // case here that is genuinely "the question could not be asked". + verdicts.map_or(facts::Look::CouldNotLook, facts::Look::Is) } fn run_hook( @@ -2294,7 +2301,11 @@ fn run_hook( // `requires_key` row has already selected this command. A repository // declaring none — and a call matching none, which is nearly every call — // does no git work here at all. - let keys: hook::KeyFacts = policy.key_base_for(&envelope).and_then(key_facts); + // `IsNot` where no `requires_key` row selected this command: the boundary + // looked and there is no key question. `key_facts` answers the other two. + let keys: hook::KeyFacts = policy + .key_base_for(&envelope) + .map_or(facts::Look::IsNot, |base| key_facts(&base)); // The waiver facts (CLOUD-610), resolved HERE for exactly the reason above: // a waiver lapses on a date, `adjudicate` is contractually pure, and reading // the clock inside it would dissolve the contract rather than satisfy it. @@ -2524,15 +2535,25 @@ fn agent_records(checks: &[(&String, &rules::ReceiptKey)]) -> hook::AgentFacts { Some(records) } +/// The checkout evidence behind a `requires_key` row, or the reason there is none. +/// +/// Every early return here is [`facts::Look::CouldNotLook`] and each is the same +/// kind of failure: no checkout, a shallow clone whose history is not there to +/// read, or a `base` git cannot resolve. None of them is "looked and found no +/// key" — that answer belongs to the caller, which knows whether any row asked +/// (CLOUD-787). fn key_facts(base: &str) -> hook::KeyFacts { - let repo = git::repo_root(Path::new(".")).ok()?; - if git::is_shallow(&repo).ok()? { - return None; - } - let messages = git::log_messages(&repo, base).ok()??; - let mut evidence = vec![messages]; - evidence.extend(git::current_branch(&repo).ok().flatten()); - Some(evidence) + let inner = || -> Option> { + let repo = git::repo_root(Path::new(".")).ok()?; + if git::is_shallow(&repo).ok()? { + return None; + } + let messages = git::log_messages(&repo, base).ok()??; + let mut evidence = vec![messages]; + evidence.extend(git::current_branch(&repo).ok().flatten()); + Some(evidence) + }; + inner().map_or(facts::Look::CouldNotLook, facts::Look::Is) } /// Assemble the end-of-turn gate's inputs (CLOUD-85). diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index e2f8db199..3d3b1ad0c 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -108,7 +108,7 @@ checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" [[package]] name = "batten" -version = "0.0.108" +version = "0.0.110" dependencies = [ "anyhow", "clap", From 6f49f8f1b6e121727e898701e4ef3bc43c3833b8 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sun, 23 Aug 2026 10:48:27 +0000 Subject: [PATCH 02/14] test(hook): the two receipt non-answers are told apart, and pinned as one document MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The call sites CLOUD-787's type substitution moved, plus the two cases §7 asks for. 74 test call sites carried `&None` for `receipts`/`keys` and 13 carried `&Some(..)`; those are `Look::CouldNotLook` and `Look::Is(..)` now. A mechanical translation — no expected `Decision` changes anywhere, because the substitution changes no behaviour. THE REGRESSION NET IS TRANSLATED, NEVER RE-AIMED. `adjudicate_ready(&None)` and `adjudicate_write(&None)` are the cases the row names as the net for the `None`-allows posture, so each keeps its original assertion verbatim against `CouldNotLook` and GAINS a sibling asserting `IsNot` allows too. Added coverage rather than a rewrite: nothing that was asserted stopped being asserted. `adjudicate_write`'s comment used to list a git-ignored path, one outside the repository, one inside `.git` AND a detached HEAD as one value; the first three are `IsNot` and the last is `CouldNotLook` — the conflation this row exists to end, sitting in a comment the whole time. WHY THERE IS NO DECISION-LEVEL DISCRIMINATOR, STATED RATHER THAN PAPERED OVER. Both non-answers allow, deliberately, so no assertion over `adjudicate` can separate them — and writing one anyway would be a test that cannot fail shipping as coverage, which is CLOUD-418's defect exactly. The distinction is made in the PRODUCERS and visible in the type, so that is where the two cases sit. Both were OBSERVED red under the mutation each names, not asserted able to fail: lib.rs `key_facts`' failure arm returning `IsNot` instead of `CouldNotLook` -> `an unresolvable base is could-not-look, got is-not`. hook.rs projecting `IsNot` as anything but `null` -> the document equality goes red. That case is the previous commit's no-output-change claim, asserted rather than promised: both non-answers still render `null`, so no consumer module and no schema moved. 2225 tests pass. `fuzz/Cargo.lock` rides along as a regenerated artifact — `main` bumped the crate to 0.0.107 and the fuzz lockfile lagged; cargo wrote it, nothing here hand-merged it. Refs: CLOUD-787 --- crates/batten/src/hook.rs | 279 +++++++++++++++++++++++++------------- crates/batten/src/lib.rs | 29 ++++ 2 files changed, 211 insertions(+), 97 deletions(-) diff --git a/crates/batten/src/hook.rs b/crates/batten/src/hook.rs index fbc30572c..696aa4d62 100644 --- a/crates/batten/src/hook.rs +++ b/crates/batten/src/hook.rs @@ -5836,8 +5836,8 @@ mod tests { ]), &envelope(command), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) } @@ -6338,8 +6338,8 @@ mod tests { &gh_policy(), &envelope(command), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) } @@ -6425,8 +6425,8 @@ mod tests { &program_only_shape_policy(), &envelope(command), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Deny(_) @@ -6500,8 +6500,8 @@ mod tests { &require_via_policy(), &envelope(command), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Deny(_) @@ -6649,8 +6649,8 @@ mod tests { &gh_policy(), &envelope("gh pr merge"), true, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Allow @@ -6731,8 +6731,8 @@ mod tests { &gh_policy(), &envelope, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Deny(_) @@ -6749,8 +6749,8 @@ mod tests { &gh_policy(), &envelope_at(event, "gh pr merge 42"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ); if event == Event::PreTool { @@ -6772,8 +6772,8 @@ mod tests { &gh_policy(), &envelope, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Allow @@ -6790,8 +6790,8 @@ mod tests { &Policy::declaring_nothing(Harness::ExitCode), &envelope("gh pr merge 42"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow @@ -6858,8 +6858,8 @@ mod tests { &policy, &envelope("gh pr merge 42"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Allow @@ -6887,8 +6887,8 @@ mod tests { &advisory, &call, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Allow, @@ -6911,8 +6911,8 @@ mod tests { &promoted, &call, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Deny(_) @@ -6943,8 +6943,8 @@ mod tests { &policy, &envelope("gh pr merge"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), )); assert!(reason.contains("first"), "got: {reason}"); @@ -6983,8 +6983,8 @@ mod tests { &policy, &envelope("gh pr merge"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), )); assert!(reason.contains("example.invalid/policy"), "got: {reason}"); @@ -7015,8 +7015,8 @@ mod tests { ]), &write_envelope(tool, path), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) } @@ -7160,6 +7160,63 @@ mod tests { } } + /// The two non-answers are DIFFERENT VALUES that project to the SAME bytes. + /// + /// CLOUD-787's whole claim in one case. Before it, `receipts` and `keys` + /// spelled could-not-look and nothing-judgeable as one `None`, so no call + /// site could tell them apart; they are `Look::CouldNotLook` and + /// `Look::IsNot` now, and `as_str` separates them. + /// + /// The second half is what makes this a type substitution rather than a + /// change: both still project as `null`, exactly what the `Option` spelling + /// emitted, so no consumer module and no schema moved. Giving Rego its own + /// spelling of the distinction is a widening of the document and belongs to + /// whichever row needs a predicate on it. + /// + /// Fails by: collapsing the two arms at the call site — give `IsNot` the + /// same token as `CouldNotLook` and the first assertion goes red; project + /// `IsNot` as anything but `null` and the second does. + #[test] + fn the_two_non_answers_are_distinct_values_projecting_identical_bytes() { + let is_not: ReceiptFacts = crate::facts::Look::IsNot; + let could_not: ReceiptFacts = crate::facts::Look::CouldNotLook; + assert_ne!( + is_not.as_str(), + could_not.as_str(), + "looked-and-found-nothing must not share a token with could-not-look" + ); + assert!(!is_not.could_not_look() && could_not.could_not_look()); + + let stop = crate::stop::StopFacts::default(); + let waived = crate::waiver::Live::new(); + let rendered = |receipts: &ReceiptFacts, keys: &KeyFacts| { + let document = call_document( + &envelope("git status"), + &Facts { + receipts, + keys, + ..Facts::none(&stop, &waived) + }, + ) + .expect("the document serializes"); + let parsed: serde_json::Value = + serde_json::from_str(&document).expect("the document parses"); + parsed["facts"].clone() + }; + assert_eq!( + rendered(&crate::facts::Look::IsNot, &crate::facts::Look::IsNot), + rendered( + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook + ), + "the split is a Rust distinction; the policy input is unchanged" + ); + assert_eq!( + rendered(&crate::facts::Look::IsNot, &crate::facts::Look::IsNot)["receipts"], + serde_json::Value::Null + ); + } + /// The projection carries the VERDICT token, never the receipt statement. /// /// A receipt records a subject commit and the `origin/main` it was linear @@ -7176,7 +7233,7 @@ mod tests { let document = call_document( &envelope("git status"), &Facts { - receipts: &Some(verdicts), + receipts: &crate::facts::Look::Is(verdicts), ..Facts::none( &crate::stop::StopFacts::default(), &crate::waiver::Live::new(), @@ -7220,12 +7277,12 @@ violation contains { let stale = { let mut verdicts = std::collections::BTreeMap::new(); verdicts.insert("verify".to_owned(), crate::receipt::Validity::StaleHead); - Some(verdicts) + crate::facts::Look::Is(verdicts) }; let fresh = { let mut verdicts = std::collections::BTreeMap::new(); verdicts.insert("verify".to_owned(), crate::receipt::Validity::Valid); - Some(verdicts) + crate::facts::Look::Is(verdicts) }; let envelope = envelope("git push"); @@ -7234,7 +7291,7 @@ violation contains { &envelope, false, &stale, - &None, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ); assert!( @@ -7248,7 +7305,7 @@ violation contains { &envelope, false, &fresh, - &None, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ); assert_eq!( @@ -7264,8 +7321,8 @@ violation contains { &policy, &envelope, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ); assert_eq!( @@ -7351,8 +7408,8 @@ deny contains "refused by the module" if { &policy, &envelope("run forbidden thing"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ); match decision { @@ -7377,8 +7434,8 @@ deny contains "refused by the module" if { &policy, &envelope("run something else"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow, @@ -7445,7 +7502,7 @@ deny contains "refused by the module" if { &envelope("gh pr ready 42"), false, facts, - &None, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) } @@ -7477,7 +7534,7 @@ deny contains "refused by the module" if { &write_envelope("Write", "crates/batten/src/new.rs"), false, facts, - &None, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) } @@ -7487,11 +7544,17 @@ deny contains "refused by the module" if { // The gap this closes: every write returned Allow before the command // gate ran, so a receipt row could never be a precondition for editing. assert!(matches!( - adjudicate_write(&Some(resolved(&[("claim", Validity::Missing)]))), + adjudicate_write(&crate::facts::Look::Is(resolved(&[( + "claim", + Validity::Missing + )]))), Decision::Deny(_) )); assert_eq!( - adjudicate_write(&Some(resolved(&[("claim", Validity::Valid)]))), + adjudicate_write(&crate::facts::Look::Is(resolved(&[( + "claim", + Validity::Valid + )]))), Decision::Allow ); } @@ -7505,8 +7568,8 @@ deny contains "refused by the module" if { &claim_policy(), &envelope("gh pr ready 42"), false, - &Some(resolved(&[("claim", Validity::Missing)])), - &None, + &crate::facts::Look::Is(resolved(&[("claim", Validity::Missing)])), + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow, @@ -7517,8 +7580,8 @@ deny contains "refused by the module" if { &receipt_policy(), &write_envelope("Write", "notes.md"), false, - &Some(resolved(&[("verify", Validity::Missing)])), - &None, + &crate::facts::Look::Is(resolved(&[("verify", Validity::Missing)])), + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow, @@ -7528,12 +7591,23 @@ deny contains "refused by the module" if { #[test] fn a_write_the_boundary_did_not_judge_is_allowed() { - // `None` is the boundary saying there is no receipt question here — a - // git-ignored path, one outside the repository, one inside `.git`, or a - // detached HEAD. Every one of them allows, and they are the load-bearing - // half: a gate that refused them would refuse every write in the - // container and be switched off within the hour. - assert_eq!(adjudicate_write(&None), Decision::Allow); + // Neither non-answer is a receipt question here, and both allow — the + // load-bearing half: a gate that refused them would refuse every write + // in the container and be switched off within the hour. + // + // CLOUD-787 split what this comment used to list as one value. A + // git-ignored path, one outside the repository and one inside `.git` + // are `IsNot` — the boundary looked and there is nothing to judge — and + // a detached HEAD is `CouldNotLook`. The assertion is unchanged in both + // arms; only the spelling of the fact moved. + assert_eq!( + adjudicate_write(&crate::facts::Look::CouldNotLook), + Decision::Allow + ); + assert_eq!( + adjudicate_write(&crate::facts::Look::IsNot), + Decision::Allow + ); } #[test] @@ -7541,7 +7615,7 @@ deny contains "refused by the module" if { // The wrong pointer this avoids: "no receipt for this commit" sends the // reader looking for a per-commit step, when what is missing is a claim // the whole branch shares. - let reason = denial_text(adjudicate_write(&Some(resolved(&[( + let reason = denial_text(adjudicate_write(&crate::facts::Look::Is(resolved(&[( "claim", Validity::Missing, )])))); @@ -7632,7 +7706,7 @@ deny contains "refused by the module" if { // coverage. It is the exact defect this whole issue is about, and it // was invisible because an unmatched rule is silent. assert!(matches!( - adjudicate_ready(&Some(resolved(&[ + adjudicate_ready(&crate::facts::Look::Is(resolved(&[ ("verify", Validity::Missing), ("linear-check", Validity::Missing), ]))), @@ -7645,7 +7719,7 @@ deny contains "refused by the module" if { // The conjunction is the predicate: a branch that verified but is no // longer linear on the trunk cannot fast-forward-land. assert!(matches!( - adjudicate_ready(&Some(resolved(&[ + adjudicate_ready(&crate::facts::Look::Is(resolved(&[ ("verify", Validity::Valid), ("linear-check", Validity::StaleMain), ]))), @@ -7656,7 +7730,7 @@ deny contains "refused by the module" if { #[test] fn all_receipts_valid_allows_the_call() { assert_eq!( - adjudicate_ready(&Some(resolved(&[ + adjudicate_ready(&crate::facts::Look::Is(resolved(&[ ("verify", Validity::Valid), ("linear-check", Validity::Valid), ]))), @@ -7670,7 +7744,10 @@ deny contains "refused by the module" if { // answered for fewer checks than the row requires has not established // the precondition. assert!(matches!( - adjudicate_ready(&Some(resolved(&[("verify", Validity::Valid)]))), + adjudicate_ready(&crate::facts::Look::Is(resolved(&[( + "verify", + Validity::Valid + )]))), Decision::Deny(_) )); } @@ -7680,8 +7757,16 @@ deny contains "refused by the module" if { // Outside a checkout there are no git facts to judge against. Fail // open, the posture every retiring guard has: a guard that cannot read // its own precondition must not become the reason work stops. This is - // deliberately distinct from `Some(Missing)`, which denies above. - assert_eq!(adjudicate_ready(&None), Decision::Allow); + // deliberately distinct from `Look::Is(Missing)`, which denies above — + // and, since CLOUD-787, from `IsNot`, which allows for the other reason. + assert_eq!( + adjudicate_ready(&crate::facts::Look::CouldNotLook), + Decision::Allow + ); + assert_eq!( + adjudicate_ready(&crate::facts::Look::IsNot), + Decision::Allow + ); } #[test] @@ -7691,8 +7776,8 @@ deny contains "refused by the module" if { &receipt_policy(), &envelope("gh pr view 42"), false, - &Some(resolved(&[("verify", Validity::Missing)])), - &None, + &crate::facts::Look::Is(resolved(&[("verify", Validity::Missing)])), + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow @@ -7732,8 +7817,8 @@ deny contains "refused by the module" if { &program_only_receipt_policy(), &envelope(command), false, - &Some(resolved(&[("toolchain", Validity::Missing)])), - &None, + &crate::facts::Look::Is(resolved(&[("toolchain", Validity::Missing)])), + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Deny(_) @@ -7768,7 +7853,7 @@ deny contains "refused by the module" if { fn the_refusal_names_the_check_and_what_is_wrong_with_it() { // Three verdicts, three causes — which is how `ready-guard`'s three // hand-written deny messages survive the move into one config row. - let Decision::Deny(refusal) = adjudicate_ready(&Some(resolved(&[ + let Decision::Deny(refusal) = adjudicate_ready(&crate::facts::Look::Is(resolved(&[ ("verify", Validity::Valid), ("linear-check", Validity::StaleHead), ]))) else { @@ -7849,8 +7934,8 @@ deny contains "refused by the module" if { &protected_policy(vec![verb("Write", Some("use the owning surface"))]), &envelope, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow @@ -7880,8 +7965,8 @@ deny contains "refused by the module" if { &protected_policy(verbs), &envelope(command), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) } @@ -8032,8 +8117,8 @@ deny contains "refused by the module" if { &protected_policy(vec![row]), &write_envelope("Write", "batten.toml"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Deny(_) @@ -8048,8 +8133,8 @@ deny contains "refused by the module" if { &protected_policy(vec![verb("Write", None)]), &write_envelope("Write", "batten.toml"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Deny(_) @@ -8188,8 +8273,8 @@ deny contains "refused by the module" if { ), &envelope(command), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) } @@ -8357,8 +8442,8 @@ deny contains "refused by the module" if { &no_verbs, &envelope("rm batten.toml"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Allow @@ -8378,8 +8463,8 @@ deny contains "refused by the module" if { &no_paths, &envelope("rm batten.toml"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Allow @@ -8396,8 +8481,8 @@ deny contains "refused by the module" if { &policy, &envelope("rm .serena/memories/core.md"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), )); assert!(reason.contains("no-rm-memories"), "got: {reason}"); @@ -8410,8 +8495,8 @@ deny contains "refused by the module" if { &protected_policy(vec![verb("rm", None)]), &envelope("rm batten.toml"), true, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow @@ -8473,8 +8558,8 @@ deny contains "refused by the module" if { &guarding, &call, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Deny(_) @@ -8486,8 +8571,8 @@ deny contains "refused by the module" if { &elsewhere, &call, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Allow, @@ -9254,8 +9339,8 @@ deny contains "refused by the module" if { &policy, &envelope, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ); assert!( @@ -9562,8 +9647,8 @@ deny contains "refused by the module" if { &policy, &write_envelope_on(*harness, spelling, "batten.toml"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ); assert!( @@ -9592,8 +9677,8 @@ deny contains "refused by the module" if { &policy, &write_envelope_on(Harness::GeminiCli, "WriteFile", "batten.toml"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) else { panic!("a host-spelled write against a protected path must deny"); @@ -9808,8 +9893,8 @@ deny contains "refused by the module" if { &policy, &read, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 3eb06f0c1..872131528 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -5490,6 +5490,35 @@ mod tests { ); } + /// A `base` git cannot resolve is COULD-NOT-LOOK, never "no key here". + /// + /// CLOUD-787. `key_facts` is only ever called once a `requires_key` row has + /// already selected the command, so every failure inside it is the question + /// being unanswerable — no checkout, a shallow clone whose history is not + /// there to read, a `base` that resolves to nothing. "Looked and found no + /// key" is a different answer and it belongs to the CALLER, which is the one + /// that knows whether any row asked. + /// + /// Collapsing the two is the defect the three-valued contract exists to + /// prevent: `IsNot` here would claim the boundary had inspected a branch's + /// history and found no issue key, on a call where it never read one. + /// + /// Fails by: returning `Look::IsNot` from `key_facts`'s failure arm. + #[test] + fn an_unresolvable_base_could_not_look_rather_than_finding_no_key() { + let facts = key_facts("refs/heads/definitely-not-a-ref-cloud-787"); + assert!( + facts.could_not_look(), + "an unresolvable base is could-not-look, got {}", + facts.as_str() + ); + assert_eq!(facts.as_str(), "could-not-look"); + assert!( + !matches!(facts, facts::Look::IsNot), + "could-not-look must not be spelled as looked-and-found-nothing" + ); + } + /// A config declaring nothing still gets the line. The reader who suspects /// nothing ran is exactly the one who needs to be told they are right, and /// `0` is the honest answer rather than a reason to stay silent. From 16b62a2556e4df0ac0caac314ad651f11e2a0a0d Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sun, 23 Aug 2026 11:30:42 +0000 Subject: [PATCH 03/14] feat(facts): a call site's program and arguments, so a token's POSITION is a fact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-914. `Fact::ALL` could ask "does this file exist", "what are its lines" and "what does this TOML say", and could not ask "what does this call site invoke, and with what". So every argv-shaped guard is a substring scan, and a substring scan cannot tell a call from a comment. THE TREE MEASURES THE GAP RATHER THAN ASSERTING IT. Five of `git.rs`'s seven source-scan gates assemble their needles by concatenation, for one stated reason — that the assertion's own source must not match the gate it states — and the two that do NOT obfuscate are exactly the two that never read their own module. A substring gate must hide its own literals precisely when its corpus includes itself. THE FIRST DRAFT OF THIS FACT'S OWN DOC COMMENT PROVED IT. Written with the banned token spelled plainly in PROSE, it turned `no_ancestry_decides_merged_ness` red against `facts.rs` — at no call site at all. The guard behaved exactly as designed; the substring tier simply cannot tell a comment from a call. The doc now describes its example instead of writing it, for the same reason five gates concatenate, and says so where a reader will find it. Appended, never inserted: `semver` reads a reordered variant as `enum_no_repr_variant_discriminant_changed`. `Read x Check`, for `LINES`' reasons — parsing a file of unbounded size is unbounded in the input, and the 100ms budget is per mediated call, so nothing lands on `Surface::Hook`. ARGUMENTS ONLY, NEVER RECEIVERS, and that is the whole discriminator. A literal in a call's argument list is an invocation argument; one in an array initialiser, a `let` binding, or a method call's receiver is not. Same bytes, same file, opposite verdicts. A FILE THE PARSER REFUSES IS could-not-look, NEVER AN EMPTY SET. It reaches `missing` carrying `unparsed`, so a module reads it exactly as it reads an unreadable document. CLOUD-310 made this the price of embedding a matcher after measuring one that emitted zero nodes and zero errors over a file it had only partly parsed; CLOUD-914's three-valued clause asks the same from the other direction. A path present with an empty array is a file that parsed and calls nothing, and Rego reads an undefined path as "does not hold", so the two must never collapse. The declaration ships as a PAIR, `invocations` plus glob-selected `invocation_sources`, rather than gaining the glob a row later: `line_sources` records having to fix exactly that asymmetry, and a gate over call sites is a gate over many files by construction. `want_for` states its precedence rather than resolving by rule order, because the cache holds one answer per path. THE PARSER LIVES IN ITS OWN MODULE, AND A GATE IS WHY. `facts.rs` forbids a wildcard match arm so a fact added later fails to compile rather than classifying itself; the walk matches an expression enum of some forty variants that grows every minor release, where a wildcard is correct and enumerating it would break on an upgrade for no property gained. Two matches with opposite right answers do not belong in one file. `facts.rs` keeps the fact and its class, which is what §1 asks for. Instrument: a pure-Rust parser already vendored behind every derive in the workspace, so this is a direct edge and no new supply-chain surface. The structural-matcher route stays rejected on CLOUD-310's grounds; both defects it measured are properties of a shell corpus and do not reach `crates/**/*.rs`. Schemas regenerated with `mise run schema`, never hand-merged — including `policy-input.schema.json`, which is what makes `opa check -s` refuse a module reading a key the engine does not emit. The fact census moves 17 -> 18, which its own gate demands be a deliberate edit. 2225 tests pass. The Rego module that ports a guard onto this fact is CLOUD-756's and is deliberately not here. Refs: CLOUD-914 --- .serena/memories/core.md | 11 +++ Cargo.lock | 2 + Cargo.toml | 31 +++++++ crates/batten/Cargo.toml | 2 + crates/batten/src/config.rs | 2 + crates/batten/src/facts.rs | 74 ++++++++++++++- crates/batten/src/hook.rs | 9 ++ crates/batten/src/invocation.rs | 160 ++++++++++++++++++++++++++++++++ crates/batten/src/lib.rs | 5 +- crates/batten/src/rules.rs | 159 ++++++++++++++++++++++++++++--- crates/batten/tests/facts.rs | 7 +- fuzz/Cargo.lock | 2 + schema/batten.local.schema.json | 14 +++ schema/batten.schema.json | 14 +++ schema/policy-input.schema.json | 25 +++++ 15 files changed, 498 insertions(+), 19 deletions(-) create mode 100644 crates/batten/src/invocation.rs diff --git a/.serena/memories/core.md b/.serena/memories/core.md index 25b4fa825..aaf99f7b1 100644 --- a/.serena/memories/core.md +++ b/.serena/memories/core.md @@ -704,6 +704,17 @@ repo config > default`, declared as data in `SETTINGS` (per-key env var/flag), surface would evaluate. Per-kind column agreement is a **census** (`Rule::columns` × `requires`/`permits`), not a per-kind match — the match named fields, so a new column landed in no arm and every kind accepted it. +- `invocation.rs` — Rust call sites, parsed (CLOUD-914). Answers WHERE a token + sits, which no line predicate can: a literal in a call's ARGUMENT list is an + invocation argument, one in an array initialiser or a method call's receiver is + not, and a comment never reaches the walk at all. That split is the whole + discriminator — five of `git.rs`'s seven source-scan gates assemble their + needles by concatenation so they do not match their own source, and the two + that do not obfuscate are exactly the two that never read their own module. + A file the parser refuses is `Look::CouldNotLook`, never an empty node set. + Separate from `facts.rs` because that file forbids a wildcard match arm and the + walk over the parser's expression enum needs one — two matches with opposite + right answers do not share a file. - `hook.rs` — the `hook` adjudicator (CLOUD-202): the normalized envelope, the wrapper-lookthrough command parser, the matcher, and the **per-host shims** (CLOUD-44). Five hosts plus the neutral `exit-code`: claude-code, cursor, diff --git a/Cargo.lock b/Cargo.lock index 6ba2544e1..e4ea19baa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -132,6 +132,7 @@ dependencies = [ "ignore", "json5", "jsonschema", + "proc-macro2", "regex", "regorus", "rustix", @@ -141,6 +142,7 @@ dependencies = [ "serde_json", "sha2", "signal-hook", + "syn 2.0.119", "tar", "toml", "unicode-normalization", diff --git a/Cargo.toml b/Cargo.toml index 2233d304d..ecf04e658 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -366,6 +366,37 @@ semver = "1" tar = "0.4" serde = { version = "1", features = ["derive"] } serde_json = "1" +# Rust source parsed into a syntax tree, so a policy predicate can ask WHERE a +# token sits rather than only whether it appears (CLOUD-914). `full` for +# statement and expression syntax, `visit` for the read-only walk, `parsing` to +# turn source text into that tree; `printing` and `proc-macro` stay off, because +# nothing here emits Rust. +# +# ADOPTED RATHER THAN VENDORED ANEW: `syn` is already in `Cargo.lock` behind +# every derive in the workspace (serde, schemars, clap, thiserror), so this adds +# a direct edge and no new supply-chain surface. MIT OR Apache-2.0, already on +# `deny.toml`'s allow-list. +# +# WHY NOT THE STRUCTURAL-MATCHER ROUTE: CLOUD-310 measured that route against +# this tree and rejected it, on an upstream Rust API its own authors tier as +# unstable and on grammars compiled by build scripts through a Darwin path this +# repo has never exercised. Both defects it measured — extensionless files +# scanning to nothing, and a shell grammar silently part-parsing `.bats` — are +# properties of a shell corpus and do not reach `crates/**/*.rs`. A pure-Rust +# parser with no build-script grammar is the ground that rejection left +# standing, and `.claude/rules/scanning.md` names a CLASS for row two rather +# than a product precisely so this substitution is following the rule. +syn = { version = "2", default-features = false, features = [ + "full", + "visit", + "parsing", + "clone-impls", +] } +# `span-locations` is what turns a span into a line number, and without it +# `Span::start()` reports line 0 — a pointer-only finding whose pointer is a +# constant. Already vendored beneath `syn`; this enables the feature and takes a +# direct edge. Same license family, same allow-list. +proc-macro2 = { version = "1", features = ["span-locations"] } # Finding-identity fingerprints (CLOUD-123): SHA-256 preimages, and NFC # canonicalization so macOS NFD checkouts fingerprint identically to Linux. # diff --git a/crates/batten/Cargo.toml b/crates/batten/Cargo.toml index 1f8cd5886..9165ce00a 100644 --- a/crates/batten/Cargo.toml +++ b/crates/batten/Cargo.toml @@ -100,6 +100,8 @@ semver.workspace = true serde.workspace = true serde_json.workspace = true sha2.workspace = true +proc-macro2.workspace = true +syn.workspace = true tar.workspace = true toml.workspace = true unicode-normalization.workspace = true diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index 0ef5f66a7..678b5b772 100644 --- a/crates/batten/src/config.rs +++ b/crates/batten/src/config.rs @@ -1033,6 +1033,8 @@ fn default_rules() -> Vec { sources: Vec::new(), lines: Vec::new(), line_sources: Vec::new(), + invocations: Vec::new(), + invocation_sources: Vec::new(), predicate_severity: None, no_fix_reason: None, checks: None, diff --git a/crates/batten/src/facts.rs b/crates/batten/src/facts.rs index 017e8e016..9f0602c76 100644 --- a/crates/batten/src/facts.rs +++ b/crates/batten/src/facts.rs @@ -452,6 +452,10 @@ pub enum Fact { /// Whether this branch's work is on each **declared** target, by patch /// identity (CLOUD-880) — the landing question `Fact::GitRef` leaves open. Landing, + /// What a **declared** Rust source file's call sites invoke, and with what + /// literal arguments — the token's syntactic POSITION, which no line + /// predicate can see (CLOUD-914). + Invocations, } /// [`Fact::Bypass`] — the hatch is an environment variable, and the kernel @@ -533,6 +537,50 @@ pub const TRACKED: Class = Class::new(Cost::Read, Surface::Check); /// markdown AST would be a parser per prose convention. pub const LINES: Class = Class::new(Cost::Read, Surface::Check); +/// [`Fact::Invocations`] — a declared Rust file's call sites, parsed (CLOUD-914). +/// +/// `read` x `check`, for [`LINES`]' reasons exactly: parsing a file of unbounded +/// size is unbounded in the input, and CLOUD-689's 100ms budget is per mediated +/// call. Never repoint this at [`Surface::Hook`]. +/// +/// **What it adds over [`LINES`], stated as the thing a line predicate cannot +/// do.** `lines` answers *does this token appear in this file*; this answers +/// *does this token appear IN COMMAND POSITION*. The tree measures the +/// difference rather than arguing it: five of `git.rs`'s seven source-scan +/// gates assemble their needles by concatenation — `["merge", "-base"].concat()` +/// — for one stated reason, that *"this assertion's own source is not a match +/// for the gate it states"*. The two gates that do NOT obfuscate are exactly the +/// two that never read their own module's source. A substring gate must hide its +/// own literals precisely when its corpus includes itself, and that is the +/// defect this fact retires. +/// +/// **Only arguments, never receivers, and that is what makes the discriminator +/// work.** A literal in a call's argument list is an invocation argument; a +/// literal in an array initialiser, a `let` binding, or the receiver of a method +/// call is not. So an `.arg(..)` carrying a banned token is reported and a +/// needle array holding the same token is not — same bytes, same file, opposite +/// verdicts, which is the test this fact lives or dies on. Comments never arrive +/// at all: the parser discards them before this sees a token. +/// +/// **This paragraph could not spell its own example, and that is the finding.** +/// It first read `.arg("merge" + "-base")` with the token written plainly, and +/// `git::tests::no_ancestry_decides_merged_ness` went red on `facts.rs` — +/// against PROSE IN A DOC COMMENT, at no call site at all. The guard is behaving +/// exactly as designed; the substring tier simply cannot tell a comment from a +/// call. So the sentence above describes its example instead of writing it, for +/// the same reason five of `git.rs`'s seven gates assemble their needles by +/// concatenation. Measured while documenting the fact that retires the +/// workaround — which is the strongest evidence this row could have asked for, +/// and it arrived unprompted. +/// +/// **Rust only, and deliberately.** CLOUD-310 rejected the tree-sitter route on +/// a corpus of 46 extensionless shell programs, and both of its defects are +/// artefacts of that corpus — file discovery, and the bash grammar's fit to +/// `.bats`. Neither reaches `crates/**/*.rs`. What survives its rejection and +/// binds here is the parse-coverage obligation: a file the parser cannot read is +/// [`Look::CouldNotLook`], never an empty node set. +pub const INVOCATIONS: Class = Class::new(Cost::Read, Surface::Check); + /// [`Fact::AgentSourced`] — a small record under the git dir, written by the /// boundary from bytes the harness already handed it (CLOUD-776). /// @@ -748,6 +796,7 @@ impl Fact { Fact::GitRef, Fact::GitRange, Fact::Landing, + Fact::Invocations, ]; /// The stable lowercase token (§6) — the field name in `lib.rs`'s `Facts`. @@ -771,6 +820,7 @@ impl Fact { Fact::GitRef => "git-refs", Fact::GitRange => "git-ranges", Fact::Landing => "landing", + Fact::Invocations => "invocations", } } @@ -802,6 +852,7 @@ impl Fact { Fact::GitRef => GIT_REF, Fact::GitRange => GIT_RANGE, Fact::Landing => LANDING, + Fact::Invocations => INVOCATIONS, } } @@ -848,6 +899,10 @@ impl Fact { // every consumer this row exists for -- the tasks that today read a // sibling's exit code to learn whether work landed -- is one. Fact::Landing => Some("landing"), + // Tree-only by construction (CLOUD-914): a call site is a property + // of committed source, and the mediated path has no budget to parse + // one. + Fact::Invocations => Some("invocations"), // Hook-surface facts. The tree engine resolves none of them, and // naming them here as `None` is what lets the correspondence test // assert the emitted key set in BOTH directions rather than only @@ -907,6 +962,22 @@ impl Fact { "description": "Fact::Lines. Path -> the file's lines, unparsed (CLOUD-846).", "additionalProperties": {"type": "array", "items": {"type": "string"}}, }), + Fact::Invocations => serde_json::json!({ + "type": "object", + "description": "Fact::Invocations (CLOUD-914). Path -> the call sites in that Rust file, each `program` (the callee as written), `arguments` (the string literals it PASSES -- never its receiver) and `line`. A path absent from this map is could-not-look: the parser could not read it. A path present with an empty list is a file that parsed and calls nothing. Rego reads an undefined path as `does not hold`, so those two must never collapse.", + "additionalProperties": { + "type": "array", + "items": { + "type": "object", + "properties": { + "program": {"type": "string"}, + "arguments": {"type": "array", "items": {"type": "string"}}, + "line": {"type": "integer"}, + }, + "additionalProperties": false, + }, + }, + }), Fact::Bypass => serde_json::json!({ "type": "boolean", "description": "Fact::Bypass -- the BATTEN_HOOK_BYPASS hatch (CLOUD-610). The one fact whose shape is certain enough to constrain.", @@ -1041,7 +1112,8 @@ impl Fact { | Fact::Lines | Fact::AgentSourced | Fact::Prospective - | Fact::Produced => serde_json::json!({ + | Fact::Produced + | Fact::Invocations => serde_json::json!({ "description": "unrouted fact -- schema_fragment delegated a fact git_schema_fragment does not own", }), } diff --git a/crates/batten/src/hook.rs b/crates/batten/src/hook.rs index 696aa4d62..1685e5adf 100644 --- a/crates/batten/src/hook.rs +++ b/crates/batten/src/hook.rs @@ -4481,6 +4481,13 @@ fn call_document(envelope: &Envelope, facts: &Facts<'_>) -> Result None, + // Not resolvable on the mediated call, and the strongest case of the + // three above: this PARSES a file of unbounded size where `Lines` + // only reads one, so it is unbounded in the input twice over + // (CLOUD-914). A call site is a property of committed source, which + // is the tree surface's question, and the mediated path has no + // budget for a syntax tree. + crate::facts::Fact::Invocations => None, // Not resolvable on the mediated call, same axis and same reason // (CLOUD-851): the sink store is read for every key the RULESET // declares, and that count is unbounded in the ruleset where a single @@ -5745,6 +5752,8 @@ mod tests { sources: Vec::new(), lines: Vec::new(), line_sources: Vec::new(), + invocations: Vec::new(), + invocation_sources: Vec::new(), git: Vec::new(), refs: Vec::new(), ranges: Vec::new(), diff --git a/crates/batten/src/invocation.rs b/crates/batten/src/invocation.rs new file mode 100644 index 000000000..af8d28573 --- /dev/null +++ b/crates/batten/src/invocation.rs @@ -0,0 +1,160 @@ +//! Rust call sites, parsed — the syntactic POSITION of a token (CLOUD-914). +//! +//! `facts.rs` stays the one authority on what [`crate::facts::Fact::Invocations`] +//! IS and what it costs; this module is where the argv is actually read, which +//! is the split that row's §1 asks for. +//! +//! **It lives here rather than in `facts.rs` because of a gate, and the gate was +//! right.** `tests/facts.rs`'s `no_axis_match_carries_a_wildcard_arm` refuses a +//! wildcard arm anywhere in `facts.rs`, so that a fact added later fails to +//! compile instead of classifying itself. The walk below matches on +//! `syn::Expr`, which has some forty variants and gains more with every minor +//! release of the parser — a wildcard there is correct, and enumerating it would +//! be a list that breaks on an upgrade for no property gained. Two matches with +//! opposite right answers do not belong in one file, and moving this out is the +//! honest resolution rather than widening the gate. + +/// One call site: what it invokes, and the literal arguments it passes. +/// +/// Pointer-bearing by construction (non-negotiable rule 4). `line` is what a +/// finding reports; `program` and `arguments` are what a PREDICATE decides on +/// and are never rendered into a finding by the engine. That split is +/// [`Fact::Prospective`]'s exactly — a rule may decide over content, and what is +/// reported is `path:line` and a rule id. +#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize)] +#[non_exhaustive] +pub struct Invocation { + /// The callee as written — `Command::new`, `arg`, `git::query`. The path or + /// method name, never a resolved type: this is the syntactic tier, and + /// claiming resolution it does not do is how `grep` counted 14 `Command` + /// sites where name resolution finds 9. + pub program: String, + /// The string literals passed as ARGUMENTS to this call, in source order. + /// + /// Arguments only, never the receiver — the whole discriminator. A literal + /// in an array initialiser or in a method call's receiver is not an argument + /// and does not appear here, which is why a needle table reads as silent + /// while the same token passed to `.arg(..)` does not. + pub arguments: Vec, + /// 1-indexed line of the call site, for the pointer a finding carries. + pub line: usize, +} + +/// Every call site in a Rust source text, or the reason there are none. +/// +/// [`Look::CouldNotLook`] when the text is not parseable Rust — **never an empty +/// list**, which is the parse-coverage obligation CLOUD-310 attached to any +/// embedded matcher after measuring silent partial parses that emitted zero +/// error nodes. Rego reads an undefined path as "does not hold", so a gate whose +/// corpus failed to parse would report clean; that is CLOUD-845's vacuous pass +/// and CLOUD-251 named the class before it. +/// +/// A file that parses and calls nothing is `Is(vec![])` — looked, and there are +/// no call sites. The two are different answers and this function keeps them so. +#[must_use] +pub fn invocations(source: &str) -> crate::facts::Look> { + use syn::visit::Visit; + + let Ok(file) = syn::parse_file(source) else { + return crate::facts::Look::CouldNotLook; + }; + + #[derive(Default)] + struct Sites { + found: Vec, + } + + // The literals a call PASSES. Descends through the grouping expressions an + // argument can legally wear — a reference, a parenthesis, a cast, an array + // or tuple built inline at the call — because a borrowed array of string + // literals is one argument spelled four nodes deep, and is the ordinary way + // this tree passes an argv. + // + // It deliberately does NOT descend into a nested CALL: that call's own + // arguments belong to it, and the visitor reaches it separately. Folding + // them upward would attribute an inner call's literals to its caller. + fn literals(expr: &syn::Expr, out: &mut Vec) { + match expr { + syn::Expr::Lit(lit) => { + if let syn::Lit::Str(text) = &lit.lit { + out.push(text.value()); + } + } + syn::Expr::Reference(inner) => literals(&inner.expr, out), + syn::Expr::Paren(inner) => literals(&inner.expr, out), + syn::Expr::Group(inner) => literals(&inner.expr, out), + syn::Expr::Cast(inner) => literals(&inner.expr, out), + syn::Expr::Array(array) => { + for element in &array.elems { + literals(element, out); + } + } + syn::Expr::Tuple(tuple) => { + for element in &tuple.elems { + literals(element, out); + } + } + _ => {} + } + } + + impl<'ast> Visit<'ast> for Sites { + fn visit_expr_call(&mut self, call: &'ast syn::ExprCall) { + let mut arguments = Vec::new(); + for argument in &call.args { + literals(argument, &mut arguments); + } + self.found.push(Invocation { + program: callee(&call.func), + arguments, + line: line_of(call.paren_token.span.open()), + }); + syn::visit::visit_expr_call(self, call); + } + + fn visit_expr_method_call(&mut self, call: &'ast syn::ExprMethodCall) { + let mut arguments = Vec::new(); + for argument in &call.args { + literals(argument, &mut arguments); + } + // The RECEIVER is not an argument, and not descending into it here + // is not an omission: `visit_expr_method_call` below walks it, so a + // call inside the receiver is still found as its own site. What is + // excluded is the receiver's LITERALS being read as this call's + // arguments — which is exactly what makes a concatenated needle + // array silent and the same token passed to `.arg(..)` loud. + self.found.push(Invocation { + program: call.method.to_string(), + arguments, + line: line_of(call.method.span()), + }); + syn::visit::visit_expr_method_call(self, call); + } + } + + let mut sites = Sites::default(); + sites.visit_file(&file); + crate::facts::Look::Is(sites.found) +} + +/// The callee as written, flattened to a path string. +fn callee(func: &syn::Expr) -> String { + match func { + syn::Expr::Path(path) => path + .path + .segments + .iter() + .map(|segment| segment.ident.to_string()) + .collect::>() + .join("::"), + // A call whose callee is computed — a closure variable, an index, a + // field. There is a call site and its program is not a name, which is a + // different answer from "no call site" and is spelled as one. + _ => String::new(), + } +} + +/// 1-indexed source line of a span. +fn line_of(span: proc_macro2::Span) -> usize { + span.start().line +} diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 872131528..73e207674 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -39,6 +39,8 @@ pub mod handler; pub mod hook; pub mod identity; pub mod init; +/// Rust call sites, parsed — where a token sits, not merely that it appears. +pub mod invocation; pub mod journal; pub mod judge; pub mod lint; @@ -1673,6 +1675,7 @@ fn run_policy_test(json: bool, overrides: &Overrides, out: &mut dyn Write) -> Re &documents, &declared, &rules::declared_lines(rule, &tracked)?, + &rules::declared_invocations(rule, &tracked)?, &tracked, // A SUITE RUNS AGAINST NO PRODUCED RECORD, deliberately (CLOUD-851). // A module's tests must decide the same way on every machine, and the @@ -2305,7 +2308,7 @@ fn run_hook( // looked and there is no key question. `key_facts` answers the other two. let keys: hook::KeyFacts = policy .key_base_for(&envelope) - .map_or(facts::Look::IsNot, |base| key_facts(&base)); + .map_or(facts::Look::IsNot, |base| key_facts(base)); // The waiver facts (CLOUD-610), resolved HERE for exactly the reason above: // a waiver lapses on a date, `adjudicate` is contractually pure, and reading // the clock inside it would dissolve the contract rather than satisfy it. diff --git a/crates/batten/src/rules.rs b/crates/batten/src/rules.rs index 11d9d8f24..124c1f491 100644 --- a/crates/batten/src/rules.rs +++ b/crates/batten/src/rules.rs @@ -778,6 +778,8 @@ impl RuleKind { "sources", "lines", "line_sources", + "invocations", + "invocation_sources", "severity", "predicate_severity", "identity_key", @@ -1745,6 +1747,24 @@ pub struct Rule { /// naming one or two paths. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub line_sources: Vec, + /// The Rust files this policy row reads as **call sites**, by literal path + /// (CLOUD-914). + /// + /// [`Rule::lines`]' sibling one tier up: `lines` answers *does this token + /// appear in this file*, and this answers *does it appear in command + /// position*. A row wanting the second must say so, because parsing is + /// strictly more expensive than splitting and nobody should pay it by + /// accident. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub invocations: Vec, + /// The same, glob-selected (CLOUD-914). + /// + /// Ships WITH its literal column rather than arriving a row later, which is + /// the asymmetry `line_sources` records having to fix: a gate over call + /// sites is a gate over many files by construction, so the enumerated + /// spelling would go stale the first time a module was added. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub invocation_sources: Vec, /// The git facts this policy row reads, **declared** (CLOUD-907). /// /// [`Rule::documents`]'s sibling for the family that is not a file, and the @@ -2826,6 +2846,14 @@ impl Rule { self.id ))); } + if self.scope == RuleScope::MediatedCall + && !(self.invocations.is_empty() && self.invocation_sources.is_empty()) + { + return Err(UsageError::raise(format!( + "rule {}: `invocations` parses tree files and a mediated call judges a command, not a tree", + self.id + ))); + } if self.scope == RuleScope::MediatedCall && !self.lines.is_empty() { return Err(UsageError::raise(format!( "rule {}: `lines` is what a `scope = \"tree\"` row hands its bundle; \ @@ -2912,7 +2940,7 @@ impl Rule { /// about all of them makes that failure impossible, and /// [`tests::every_optional_rule_field_is_classified_by_every_kind`] fails if /// a column is added here without being placed. - fn columns(&self) -> [(&'static str, bool); 43] { + fn columns(&self) -> [(&'static str, bool); 45] { [ // In the census because it is now per-kind, which is what makes // "required by every kind but the judge" a fact the existing @@ -2968,6 +2996,8 @@ impl Rule { // rather than per-kind capabilities, so they follow their siblings // rather than inventing a fifth classification for one of them. ("line_sources", !self.line_sources.is_empty()), + ("invocations", !self.invocations.is_empty()), + ("invocation_sources", !self.invocation_sources.is_empty()), ] } @@ -4468,6 +4498,8 @@ pub(crate) enum Want { Parsed(crate::facts::Format), /// Split into lines, unparsed (CLOUD-846). Lines, + /// Parsed as Rust, and reduced to its call sites (CLOUD-914). + Invocations, } /// A declared file, acquired — or the stated reason it was not. @@ -4477,6 +4509,10 @@ pub(crate) enum Acquired { Parsed(crate::facts::Node), /// The file's lines, in order, with line endings removed. Lines(Vec), + /// The file's call sites, parsed (CLOUD-914). An EMPTY vector is a file that + /// parsed and calls nothing — a real answer. A file the parser refused is + /// [`NotAcquired::Unparsed`], never this. + Invocations(Vec), /// Not acquired, and why. No(NotAcquired), } @@ -4555,9 +4591,18 @@ pub(crate) fn acquire(root: &Path, rel_path: &str, want: Option) -> Acquir // numbers are the ones an editor shows and a Windows checkout does not // change the answer. Want::Lines => Acquired::Lines(text.lines().map(ToOwned::to_owned).collect()), + // The other `want` a parser can refuse after a successful read, and it + // is spelled `Unparsed` rather than `Unreadable` for the same reason: + // the bytes were fine and the grammar was not. CLOUD-310 made this the + // price of embedding a matcher, after measuring one that emitted zero + // nodes and zero errors over a file it had only partly parsed. + Want::Invocations => match crate::invocation::invocations(&text) { + crate::facts::Look::Is(sites) => Acquired::Invocations(sites), + crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => { + Acquired::No(NotAcquired::Unparsed) + } + }, // THE ONE `Format::read` CALL IN THE CRATE. - // `one_document_acquisition_exists` asserts that by counting this - // needle, so a second parse pair cannot land without the gate going red. Want::Parsed(format) => match format.read(&text) { crate::facts::Look::Is(node) => Acquired::Parsed(node), // A file that will not parse says nothing about what it contains, @@ -4604,7 +4649,16 @@ const READ_BUDGET: usize = 10_000; /// there, which is the ceiling this fact lifts: `Format::for_path` returning /// `None` was the whole reason 12 of the 20 tree-scoped gates had no fact to /// decide over. -pub(crate) fn want_for(path: &str, as_lines: bool) -> Option { +pub(crate) fn want_for(path: &str, as_lines: bool, as_invocations: bool) -> Option { + // PRECEDENCE IS STATED, because the cache holds one answer per path and a + // path two rows declare differently must not resolve by rule order — the + // same hazard the `lines`/`documents` pair already collects sets to avoid. + // Invocations is the narrowest request and wins: it is the only one that + // can fail to parse, so resolving it last would let a broader request mask + // a could-not-look the declaring row needs to see. + if as_invocations { + return Some(Want::Invocations); + } if as_lines { return Some(Want::Lines); } @@ -4671,6 +4725,21 @@ pub(crate) fn declared_lines(rule: &Rule, files: &[String]) -> anyhow::Result anyhow::Result> { + select_declared(&rule.invocations, &rule.invocation_sources, files) +} + /// The literal-plus-glob union both declaration pairs resolve through. /// /// One implementation rather than two, so `lines` cannot drift from `documents` @@ -4732,6 +4801,14 @@ pub(crate) fn acquire_declared( .filter(|rule| rule.kind == RuleKind::Policy && rule.scope == RuleScope::Tree) .flat_map(|rule| declared_lines(rule, files).unwrap_or_default()) .collect(); + // The same collect-first treatment as `lines_paths`, and for the same + // reason (CLOUD-914): a path two rows want differently must not resolve by + // whichever row the loop reached first. + let invocation_paths: BTreeSet = rules + .iter() + .filter(|rule| rule.kind == RuleKind::Policy && rule.scope == RuleScope::Tree) + .flat_map(|rule| declared_invocations(rule, files).unwrap_or_default()) + .collect(); for rule in rules .iter() .filter(|rule| rule.kind == RuleKind::Policy && rule.scope == RuleScope::Tree) @@ -4744,7 +4821,12 @@ pub(crate) fn acquire_declared( continue; }; let declared_line_paths = declared_lines(rule, files).unwrap_or_default(); - for path in declared.into_iter().chain(declared_line_paths) { + let declared_invocation_paths = declared_invocations(rule, files).unwrap_or_default(); + for path in declared + .into_iter() + .chain(declared_line_paths) + .chain(declared_invocation_paths) + { if cache.contains_key(&path) { // THE CACHE, and the assertion `documents_acquired` makes: N // rows over one path is ONE read and ONE parse. @@ -4758,7 +4840,15 @@ pub(crate) fn acquire_declared( // consumer's file names, and a refusal that printed it would put the // shape of a private tree into an error message. refuse_over_budget(cache.len(), READ_BUDGET)?; - let acquired = acquire(root, &path, want_for(&path, lines_paths.contains(&path))); + let acquired = acquire( + root, + &path, + want_for( + &path, + lines_paths.contains(&path), + invocation_paths.contains(&path), + ), + ); cache.insert(path, acquired); } } @@ -4866,6 +4956,10 @@ pub(crate) fn tree_document( cache: &BTreeMap, documents: &[String], lines: &[String], + // The Rust files any row declared as call sites (CLOUD-914). A separate + // list for `lines`' reason: the cache holds one answer per path, so which + // projection a path belongs to is the caller's declaration and not a guess. + invocations: &[String], tracked: &[String], // What earlier runs produced, acquired once at the boundary (CLOUD-851). // Handed in rather than read here for `Fact::Produced`'s whole reason: the @@ -4882,6 +4976,7 @@ pub(crate) fn tree_document( .collect(); let mut parsed = serde_json::Map::new(); let mut read_lines = serde_json::Map::new(); + let mut call_sites = serde_json::Map::new(); let mut missing = Vec::new(); // The same set as `missing`, carrying WHY (CLOUD-845). `missing` stays a // bare path list in the document because that is what a module reads; the @@ -4907,7 +5002,7 @@ pub(crate) fn tree_document( } // Acquired as LINES by another row's declaration, or not declared. // Neither is a parsed document, and neither is an empty one. - Some(Acquired::Lines(_)) | None => { + Some(Acquired::Lines(_) | Acquired::Invocations(_)) | None => { missing.push(path.clone()); causes.push((path.clone(), NotAcquired::Absent)); } @@ -4928,7 +5023,29 @@ pub(crate) fn tree_document( } // A path acquired as something else, or not declared. Neither is a // readable line set, and neither is an empty one. - Some(Acquired::Parsed(_)) | None => { + Some(Acquired::Parsed(_) | Acquired::Invocations(_)) | None => { + missing.push(path.clone()); + causes.push((path.clone(), NotAcquired::Absent)); + } + } + } + for path in invocations { + match cache.get(path) { + Some(Acquired::Invocations(sites)) => { + call_sites.insert(path.clone(), serde_json::json!(sites)); + } + // COULD NOT LOOK, NEVER AN EMPTY ARRAY, and here the parser is the + // one that can say so: `NotAcquired::Unparsed` is a file whose bytes + // read fine and whose grammar did not. CLOUD-310 measured a matcher + // emitting zero nodes and zero errors over a partially-parsed file, + // and made a parse-coverage assertion the price of embedding one. + Some(Acquired::No(why)) => { + missing.push(path.clone()); + causes.push((path.clone(), *why)); + } + // Acquired as something else, or not declared. Neither is a call-site + // set, and neither is an empty one. + Some(Acquired::Parsed(_) | Acquired::Lines(_)) | None => { missing.push(path.clone()); causes.push((path.clone(), NotAcquired::Absent)); } @@ -4962,6 +5079,15 @@ pub(crate) fn tree_document( crate::facts::Fact::Document => serde_json::Value::Object(std::mem::take(&mut parsed)), crate::facts::Fact::Tracked => serde_json::json!(tracked), crate::facts::Fact::Lines => serde_json::Value::Object(std::mem::take(&mut read_lines)), + // A path the parser refused is in `missing` rather than here, + // carrying `unparsed` as its cause — so a module reads could-not-look + // exactly as it does for a document, and a path present with an + // empty array is a file that parsed and calls nothing. Those are the + // two answers CLOUD-310's parse-coverage obligation demands stay + // apart, and the projection is where they would have collapsed. + crate::facts::Fact::Invocations => { + serde_json::Value::Object(std::mem::take(&mut call_sites)) + } // Hook-surface facts, filtered above. Stated as an arm so a // reclassification has to come through here. crate::facts::Fact::Produced => { @@ -5052,10 +5178,12 @@ fn policy_rule( // failure here is a row this surface does not own, treated the same way // `declared_documents` is treated a few lines up. let declared_line_paths = declared_lines(rule, tracked).ok()?; + let declared_invocation_paths = declared_invocations(rule, tracked).ok()?; let (input, not_acquired) = tree_document( documents, &declared, &declared_line_paths, + &declared_invocation_paths, tracked, produced, git, @@ -6451,10 +6579,10 @@ fn document_in_file( Acquired::No( NotAcquired::Unparsed | NotAcquired::Unreadable | NotAcquired::UnknownFormat, ) => Some(DOCUMENT_UNREADABLE), - // Unreachable: this site always asks for `Want::Parsed`. An arm rather - // than a wildcard so a caller that ever asks for lines here has to - // decide what a node path means over them. - Acquired::Lines(_) => Some(DOCUMENT_UNREADABLE), + // Unreachable: this site always asks for `Want::Parsed`. Arms rather + // than a wildcard so a caller that ever asks for lines or call sites + // here has to decide what a node path means over them. + Acquired::Lines(_) | Acquired::Invocations(_) => Some(DOCUMENT_UNREADABLE), Acquired::Parsed(document) => match document.at(node_path) { crate::facts::Look::IsNot => Some(DOCUMENT_NODE_ABSENT), crate::facts::Look::CouldNotLook => Some(DOCUMENT_UNREADABLE), @@ -6600,7 +6728,7 @@ fn derive_one(rule: &Rule, root: &Path, files: &[String]) -> crate::facts::Look< // Unreachable: this site always asks for `Want::Parsed`. An arm rather // than a wildcard, so a caller that ever asks for lines here has to // decide what a node path means over them. - Acquired::No(_) | Acquired::Lines(_) => Look::CouldNotLook, + Acquired::No(_) | Acquired::Lines(_) | Acquired::Invocations(_) => Look::CouldNotLook, Acquired::Parsed(document) => match document.at(node_path) { Look::Is(node) => match node.scalar() { Some(value) => Look::Is(value), @@ -7281,7 +7409,7 @@ mod tests { // is that the cause is the declaration, not the filesystem. std::fs::write(dir.join("prose.md"), "# heading\n").unwrap(); - let acquire = |name: &str| super::acquire(&dir, name, super::want_for(name, false)); + let acquire = |name: &str| super::acquire(&dir, name, super::want_for(name, false, false)); assert!( matches!(acquire("good.toml"), super::Acquired::Parsed(_)), @@ -7344,6 +7472,7 @@ mod tests { &[], &[], &[], + &[], &BTreeMap::new(), &crate::git::GitFacts::default(), ); @@ -7862,6 +7991,8 @@ mod tests { sources: Vec::new(), lines: Vec::new(), line_sources: Vec::new(), + invocations: Vec::new(), + invocation_sources: Vec::new(), git: Vec::new(), refs: Vec::new(), ranges: Vec::new(), diff --git a/crates/batten/tests/facts.rs b/crates/batten/tests/facts.rs index 37e2759b5..9e3071605 100644 --- a/crates/batten/tests/facts.rs +++ b/crates/batten/tests/facts.rs @@ -12,8 +12,8 @@ use std::path::PathBuf; use batten::facts::{ AGENT_SOURCED, BYPASS, Class, Cost, DOCUMENT, Fact, GIT_HEAD, GIT_RANGE, GIT_REF, GIT_REMOTE, - GIT_STATUS, KEYS, LANDING, LINES, Look, PRODUCED, PROSPECTIVE, RECEIPTS, STOP, Surface, - TRACKED, WAIVED, + GIT_STATUS, INVOCATIONS, KEYS, LANDING, LINES, Look, PRODUCED, PROSPECTIVE, RECEIPTS, STOP, + Surface, TRACKED, WAIVED, }; #[test] @@ -103,6 +103,7 @@ fn every_fact_returns_its_stated_const() { Fact::GitRef => GIT_REF, Fact::GitRange => GIT_RANGE, Fact::Landing => LANDING, + Fact::Invocations => INVOCATIONS, } }; @@ -111,7 +112,7 @@ fn every_fact_returns_its_stated_const() { // rather than quietly shrinking the census. assert_eq!( Fact::ALL.len(), - 17, + 18, "the census covers every fact; update this count deliberately when the \ model gains or loses one" ); diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index 3d3b1ad0c..6654ef367 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -123,6 +123,7 @@ dependencies = [ "hmac", "ignore", "json5", + "proc-macro2", "regex", "regorus", "rustix", @@ -132,6 +133,7 @@ dependencies = [ "serde_json", "sha2", "signal-hook", + "syn 2.0.119", "tar", "toml", "unicode-normalization", diff --git a/schema/batten.local.schema.json b/schema/batten.local.schema.json index afd76598c..182b4a785 100644 --- a/schema/batten.local.schema.json +++ b/schema/batten.local.schema.json @@ -596,6 +596,20 @@ "null" ] }, + "invocation_sources": { + "description": "The same, glob-selected (CLOUD-914).\n\nShips WITH its literal column rather than arriving a row later, which is\nthe asymmetry `line_sources` records having to fix: a gate over call\nsites is a gate over many files by construction, so the enumerated\nspelling would go stale the first time a module was added.", + "type": "array", + "items": { + "type": "string" + } + }, + "invocations": { + "description": "The Rust files this policy row reads as **call sites**, by literal path\n(CLOUD-914).\n\n[`Rule::lines`]' sibling one tier up: `lines` answers *does this token\nappear in this file*, and this answers *does it appear in command\nposition*. A row wanting the second must say so, because parsing is\nstrictly more expensive than splitting and nobody should pay it by\naccident.", + "type": "array", + "items": { + "type": "string" + } + }, "key": { "description": "Which git fact a [`RuleKind::Receipt`] row's receipts are keyed to.\n\nOptional with a pinned default of [`ReceiptKey::Head`], the conservative\none: a HEAD-keyed receipt expires on an amend or a rebase, so omitting\nthe key can only make a gate stricter than intended, never weaker.", "anyOf": [ diff --git a/schema/batten.schema.json b/schema/batten.schema.json index c99ae2e28..c1301db42 100644 --- a/schema/batten.schema.json +++ b/schema/batten.schema.json @@ -1606,6 +1606,20 @@ "null" ] }, + "invocation_sources": { + "description": "The same, glob-selected (CLOUD-914).\n\nShips WITH its literal column rather than arriving a row later, which is\nthe asymmetry `line_sources` records having to fix: a gate over call\nsites is a gate over many files by construction, so the enumerated\nspelling would go stale the first time a module was added.", + "type": "array", + "items": { + "type": "string" + } + }, + "invocations": { + "description": "The Rust files this policy row reads as **call sites**, by literal path\n(CLOUD-914).\n\n[`Rule::lines`]' sibling one tier up: `lines` answers *does this token\nappear in this file*, and this answers *does it appear in command\nposition*. A row wanting the second must say so, because parsing is\nstrictly more expensive than splitting and nobody should pay it by\naccident.", + "type": "array", + "items": { + "type": "string" + } + }, "key": { "description": "Which git fact a [`RuleKind::Receipt`] row's receipts are keyed to.\n\nOptional with a pinned default of [`ReceiptKey::Head`], the conservative\none: a HEAD-keyed receipt expires on an amend or a rebase, so omitting\nthe key can only make a gate stricter than intended, never weaker.", "anyOf": [ diff --git a/schema/policy-input.schema.json b/schema/policy-input.schema.json index 8ef50776f..0bc4eedef 100644 --- a/schema/policy-input.schema.json +++ b/schema/policy-input.schema.json @@ -110,6 +110,31 @@ "null" ] }, + "invocations": { + "additionalProperties": { + "items": { + "additionalProperties": false, + "properties": { + "arguments": { + "items": { + "type": "string" + }, + "type": "array" + }, + "line": { + "type": "integer" + }, + "program": { + "type": "string" + } + }, + "type": "object" + }, + "type": "array" + }, + "description": "Fact::Invocations (CLOUD-914). Path -> the call sites in that Rust file, each `program` (the callee as written), `arguments` (the string literals it PASSES -- never its receiver) and `line`. A path absent from this map is could-not-look: the parser could not read it. A path present with an empty list is a file that parsed and calls nothing. Rego reads an undefined path as `does not hold`, so those two must never collapse.", + "type": "object" + }, "landing": { "additionalProperties": { "additionalProperties": false, From 9ce801fdf1faf20940c43fd703eca73ddc2ac492 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sun, 23 Aug 2026 11:46:30 +0000 Subject: [PATCH 04/14] test(invocation): the discriminator, and could-not-look told from an empty set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-914's §2 and §7(c) as cases. Five, all OBSERVED red under the mutation each one names — not asserted able to fail. a_token_in_command_position_is_told_from_the_same_token_elsewhere The fact's whole reason. One token in four places: passed to a call, in a needle array, in a line comment, in a doc comment. A line predicate — which is what every argv-shaped guard in this crate is today — reports four. This reports one. RED under: descending into the receiver in `visit_expr_method_call`. a_receivers_literals_are_not_the_calls_arguments The same property stated alone, because it is the half a reader is most likely to assume works the other way. RED under: the same one-line change. a_borrowed_array_of_literals_is_read_as_arguments A borrowed array IS one argument, four nodes deep — the ordinary way this tree passes an argv, and the case a shallow reading drops silently. an_unparseable_file_could_not_look_rather_than_finding_no_call_sites Asserts the two answers AGAINST EACH OTHER rather than each alone: a test checking only one would pass over an implementation that collapsed them. RED under: returning `Look::Is(vec![])` from the parse-failure arm. a_call_site_carries_the_line_it_sits_on The pointer a finding carries is a real line, not a constant. RED under: making `line_of` return 0, which is exactly what a build without the span-locations feature silently produces — a pointer-only finding whose pointer is the same for every hit. The negative cases are the tree's own shape rather than invented ones: a needle array beside a call is the exact arrangement five of `git.rs`'s seven gates work around by concatenation. Observing the mutants needed two runs. Three failed together on the first; `an_unparseable_file_...` never executed, because nextest's fail-fast cut the suite at 597 of 2230 and `NEXTEST_NO_FAIL_FAST=1` did not reach it. Recorded because a single run reporting three of four reads as complete coverage, and the fourth is the one guarding the vacuous pass. 2230 tests pass. Refs: CLOUD-914 --- crates/batten/src/invocation.rs | 120 ++++++++++++++++++++++++++++++++ crates/batten/src/lib.rs | 2 +- 2 files changed, 121 insertions(+), 1 deletion(-) diff --git a/crates/batten/src/invocation.rs b/crates/batten/src/invocation.rs index af8d28573..20fbaa313 100644 --- a/crates/batten/src/invocation.rs +++ b/crates/batten/src/invocation.rs @@ -158,3 +158,123 @@ fn callee(func: &syn::Expr) -> String { fn line_of(span: proc_macro2::Span) -> usize { span.start().line } + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used)] +mod tests { + use super::{Invocation, invocations}; + use crate::facts::Look; + + /// Every call site's arguments, flattened, for a case that only cares what + /// reached the argument tier at all. + fn arguments(source: &str) -> Vec { + match invocations(source) { + Look::Is(sites) => sites.into_iter().flat_map(|site| site.arguments).collect(), + other => panic!("expected a parse, got {}", other.as_str()), + } + } + + /// THE DISCRIMINATOR THE FACT EXISTS FOR, and the fixture is the tree's own + /// shape rather than an invented one. + /// + /// The same token in four places: passed to a call, sitting in a needle + /// array, in a line comment, and in a doc comment. A line predicate — which + /// is what every argv-shaped guard in this crate is today — reports all + /// four. This reports one. + /// + /// Fails by: descending into a method call's receiver in `visit_expr_method_ + /// call`, which is the one-line change that makes the needle array read as + /// an argument and collapses the distinction. + #[test] + fn a_token_in_command_position_is_told_from_the_same_token_elsewhere() { + let source = r#" + // banned-token in a line comment + /// banned-token in a doc comment + fn f() { + let needles = ["banned-token", "other"].concat(); + run().arg("banned-token"); + let _ = needles; + } + "#; + let found = arguments(source); + assert_eq!( + found.iter().filter(|arg| *arg == "banned-token").count(), + 1, + "exactly the call-position occurrence, got {found:?}" + ); + } + + /// A receiver is not an argument, stated on its own because it is the half a + /// reader is most likely to assume works the other way. + #[test] + fn a_receivers_literals_are_not_the_calls_arguments() { + assert_eq!( + arguments(r#"fn f() { let _ = ["a", "b"].concat(); }"#), + Vec::::new() + ); + assert_eq!(arguments(r#"fn f() { g("a"); }"#), vec!["a".to_owned()]); + } + + /// A borrowed array IS one argument, four nodes deep — the ordinary way this + /// tree passes an argv, and the case a shallow reading would miss. + #[test] + fn a_borrowed_array_of_literals_is_read_as_arguments() { + assert_eq!( + arguments(r#"fn f() { query(&repo, &["rev-list", "--count"]); }"#), + vec!["rev-list".to_owned(), "--count".to_owned()] + ); + } + + /// COULD-NOT-LOOK IS NOT AN EMPTY SET, and the two are asserted against each + /// other rather than each alone — a test that checked only one would pass + /// over an implementation that collapsed them. + /// + /// This is CLOUD-310's parse-coverage obligation and CLOUD-914's §2 three- + /// valued clause meeting at one case: Rego reads an undefined path as "does + /// not hold", so a corpus that failed to parse would report clean. + /// + /// Fails by: returning `Look::Is(vec![])` from the parse-failure arm. + #[test] + fn an_unparseable_file_could_not_look_rather_than_finding_no_call_sites() { + let refused = invocations("fn f( {{{ this is not rust"); + assert!( + refused.could_not_look(), + "unparseable source is could-not-look, got {}", + refused.as_str() + ); + + let parsed_and_empty = invocations("struct S { field: u8 }"); + assert_eq!( + parsed_and_empty, + Look::Is(Vec::new()), + "a file that parses and calls nothing is a real answer" + ); + assert!( + !parsed_and_empty.could_not_look(), + "looked-and-found-nothing must not read as could-not-look" + ); + assert_ne!( + refused.as_str(), + parsed_and_empty.as_str(), + "the two answers must not share a token" + ); + } + + /// The pointer a finding carries is a real line, not a constant. + /// + /// Fails by: dropping the `span-locations` feature, which silently makes + /// every `Span::start()` report line 0 — a pointer-only finding whose + /// pointer is the same for every hit. + #[test] + fn a_call_site_carries_the_line_it_sits_on() { + let Look::Is(sites) = invocations("fn f() {\n\n g(\"x\");\n}") else { + panic!("expected a parse"); + }; + let site: &Invocation = sites.first().expect("one call site"); + assert_eq!(site.program, "g"); + assert_eq!( + site.line, 3, + "1-indexed, and not the 0 an unfeatured build gives" + ); + } +} diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 73e207674..c88d0f0ae 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -2308,7 +2308,7 @@ fn run_hook( // looked and there is no key question. `key_facts` answers the other two. let keys: hook::KeyFacts = policy .key_base_for(&envelope) - .map_or(facts::Look::IsNot, |base| key_facts(base)); + .map_or(facts::Look::IsNot, key_facts); // The waiver facts (CLOUD-610), resolved HERE for exactly the reason above: // a waiver lapses on a date, `adjudicate` is contractually pure, and reading // the clock inside it would dissolve the contract rather than satisfy it. From ebb13477a4a85d61fe7ca76c9294a122a780200a Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sun, 23 Aug 2026 15:06:39 +0000 Subject: [PATCH 05/14] feat(facts): the use graph, resolved through the crate root's own re-export table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-762, both deliverables. §2 makes the measurement deliverable one and says the count chooses the tier and an argument is not admissible — so the count is here, as an assertion over this tree rather than a number in prose. THE MEASUREMENT: FOUR SITES, TWO CLASSES, AND BOTH ARE RE-EXPORTS. hidden internal `trust.rs` and `output.rs` write `use crate::UsageError`. The text names no module; the edge is really onto `error`. A layering gate reading lines is silently GREEN on an edge it was built to judge. phantom internal `policy.rs` and `sink.rs` write `use crate::Result`. That reads as internal, and the root's own private `use anyhow::Result` makes it EXTERNAL. The same gate is wrong in the opposite direction. Aliases and globs contribute ZERO, which was not obvious in advance and is the half the row worried about equally: every alias is `as _` or an external path bar one that leaves its module plainly visible, and every glob is `use super::*` inside a `#[cfg(test)]` module, crossing no boundary. TIER: `Read x Check`. Four is bounded and nameable, which is the arm the reversal condition sends there, so `blockedBy` CLOUD-760 does not return. The stronger reason is one that condition did not anticipate — THE RE-EXPORT TABLE IS ITSELF SYNTAX. Resolving all four needs no name resolution and no delegated analyser, only the root's own `use` and `pub use` items. Cheap AND correct about the cases a line predicate misses, rather than trading one for the other. Resolution is a post-pass over the DECLARED set, never a hardcoded path: the caller names the root by Rust's own convention, which is a language fact and not a consumer identifier (rule 1 untouched). THE TREE CORRECTED THIS TWICE, AND BOTH CORRECTIONS ARE IN THE CODE: A bare first segment at the crate root is a module OR another crate, and nothing in the statement says which. The root's own `mod` declarations are what tell them apart, so `root_exports` collects those first. `use crate::{config, rules}` imports MODULES directly. A table holding only re-exported items left 39 such edges unresolved — measured, on the second run. Every declared module is now its own entry, and `via_root` is false when the name IS the module, because the text already named its destination. `via_root` exists because the obvious discriminator is wrong: telling a resolved edge by its item's case counts all 88 ordinary edges in this crate as divergences. Measured on the first run. Only the edges resolution CHANGED are the ones a line predicate gets wrong. Three cases OBSERVED red, not asserted able to fail: making `resolve` a no-op reds both measurement cases; returning `Look::Is(vec![])` from the parse-failure arm reds the could-not-look case, which asserts the two answers AGAINST each other rather than each alone. 2235 tests pass. `Fact::Uses` and the declaration column are NOT here — the measurement decides the tier and this lands it; wiring the projection is the same shape `Fact::Invocations` just took and is the next commit. Refs: CLOUD-762 --- .serena/memories/core.md | 16 ++ crates/batten/src/lib.rs | 3 + crates/batten/src/uses.rs | 365 +++++++++++++++++++++++++++++++ crates/batten/tests/use_graph.rs | 180 +++++++++++++++ 4 files changed, 564 insertions(+) create mode 100644 crates/batten/src/uses.rs create mode 100644 crates/batten/tests/use_graph.rs diff --git a/.serena/memories/core.md b/.serena/memories/core.md index aaf99f7b1..be8718902 100644 --- a/.serena/memories/core.md +++ b/.serena/memories/core.md @@ -715,6 +715,22 @@ repo config > default`, declared as data in `SETTINGS` (per-key env var/flag), Separate from `facts.rs` because that file forbids a wildcard match arm and the walk over the parser's expression enum needs one — two matches with opposite right answers do not share a file. +- `uses.rs` — the `use` graph (CLOUD-762). Which module reaches which, resolved + through the crate root's own re-export table. **The measurement is the point:** + over `crates/batten/src/**` the syntactic tier diverges at exactly FOUR sites in + two classes, both re-exports — `trust.rs` and `output.rs` reach `error` through + `crate::UsageError` (a hidden edge a line predicate reads as nothing), and + `policy.rs` and `sink.rs` read as internal where `crate::Result` is really + `anyhow` (a phantom edge it invents). Aliases and globs move NO top-level edge + in this tree. Four is bounded and nameable, so the fact is `Read × Check` and + needs no delegated analyser — and the deeper reason is that the re-export table + is itself syntax, so a parser resolves it with no name resolution at all. + Two things the tree corrected and the code now states: a bare first segment at + the root is a module OR another crate, told apart only by the root's own `mod` + declarations; and `use crate::{a, b}` imports modules directly, so every + declared module is its own table entry. `via_root` marks the edges resolution + CHANGED, because judging by the item's case counts all 88 ordinary edges as + divergences. - `hook.rs` — the `hook` adjudicator (CLOUD-202): the normalized envelope, the wrapper-lookthrough command parser, the matcher, and the **per-host shims** (CLOUD-44). Five hosts plus the neutral `exit-code`: claude-code, cursor, diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index c88d0f0ae..7ca8af01d 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -68,6 +68,9 @@ pub mod store; pub mod surface; pub mod transcript; pub mod trust; +/// The `use` graph: which module reaches which, resolved through the root's own +/// re-export table. +pub mod uses; pub mod verbs; pub mod waiver; pub mod worktree; diff --git a/crates/batten/src/uses.rs b/crates/batten/src/uses.rs new file mode 100644 index 000000000..fac606046 --- /dev/null +++ b/crates/batten/src/uses.rs @@ -0,0 +1,365 @@ +//! The `use` graph as a fact: which module reaches which (CLOUD-762). +//! +//! # What a line predicate gets wrong, measured rather than assumed +//! +//! CLOUD-762's §2 makes the measurement deliverable one, because the tier this +//! fact belongs in is decided by a count and not by an argument. Over +//! `crates/batten/src/**`, the syntactic tier is wrong at **four sites, in two +//! classes**, and both are re-exports — aliases and glob imports contribute +//! nothing at all: +//! +//! * **A hidden internal edge.** `trust.rs` and `output.rs` write +//! `use crate::UsageError`. The text names no module; the edge is really onto +//! `crate::error`, because the root re-exports that name. A layering gate +//! reading lines is silently GREEN on an edge it was built to judge. +//! * **A phantom internal edge.** `policy.rs` and `sink.rs` write +//! `use crate::Result`. That reads as an internal edge, and the root's own +//! private `use anyhow::Result` means it is really an EXTERNAL dependency. +//! The same gate is silently confused in the opposite direction. +//! +//! Aliases are all `as _` trait imports or paths into other crates, and the one +//! crate-internal alias leaves its module path plainly visible. Every glob is +//! `use super::*` inside a `#[cfg(test)]` module, which crosses no module +//! boundary. Neither can move a top-level module edge in this tree. +//! +//! # Why this is `Read x Check` and needs no delegated analyser +//! +//! Four is bounded and nameable, which is the arm CLOUD-762's reversal condition +//! sends to `Read x Check`. But the stronger reason is one that condition did not +//! anticipate: **the re-export table is itself syntax.** Resolving all four sites +//! needs no name resolution, no rust-analyzer and no `Cost::Effect` — it needs +//! the crate root's own `use` and `pub use` items, which a parser reads exactly +//! as it reads any other statement. So the fact is cheap AND correct about the +//! cases a line predicate gets wrong, rather than trading one for the other. +//! +//! # Resolution is a post-pass over the DECLARED set, never a hardcoded path +//! +//! [`resolve`] applies one file's export table to another's edges, and the +//! caller decides which file is the root by Rust's own convention — a library +//! crate's root is `lib.rs`. That is a language fact, not a consumer identifier, +//! so non-negotiable rule 1 is untouched: nothing here names a repository, an +//! account or an entity path, and a consumer whose root is elsewhere simply +//! declares it. +//! +//! # Pointer-only +//! +//! An edge is two module names and a line (rule 4). The imported ITEM name is +//! carried because resolution needs it, and it is a path segment rather than +//! content — never the source line, and never the file's text. + +use std::collections::BTreeMap; + +use crate::facts::Look; + +/// Where a `use` statement's first segment points, before resolution. +#[derive(Clone, Copy, Debug, PartialEq, Eq, serde::Serialize)] +#[serde(rename_all = "kebab-case")] +pub enum Origin { + /// `use crate::::…` — the module is named in the text, and a line + /// predicate reads it correctly. + Internal, + /// `use ::…` — outside this crate entirely. + External, + /// `use crate::` — a name reaching through the crate root's own + /// re-export table. **The module is NOT in the text**, which is the whole + /// class this fact exists to resolve. + RootItem, + /// `use self::…` / `use super::…` — inside the file's own module tree, so it + /// crosses no top-level boundary. Kept as its own answer rather than dropped: + /// "there is no edge here" and "I did not look" must stay distinct. + Local, +} + +/// One `use` edge: what it reaches, and where it was written. +#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize)] +#[non_exhaustive] +pub struct UseEdge { + /// The module or crate reached, once resolved. Empty exactly while + /// [`Origin::RootItem`] is unresolved — which [`resolve`] fills in, and which + /// a consumer must never read as "reaches nothing". + pub to: String, + /// The imported item's leaf name. What resolution looks up. + pub item: String, + /// Which class the first segment put this edge in. + pub origin: Origin, + /// Whether [`resolve`] is what supplied `to`, rather than the text. + /// + /// **This is the flag the measurement counts, and it exists because the + /// obvious discriminator is wrong.** An edge written `use crate::error::X` + /// and one written `use crate::UsageError` both end as [`Origin::Internal`] + /// onto `error`, and telling them apart by the item's case counts all 88 + /// ordinary edges in this crate as divergences — measured, on the first run + /// of the case below. Only the edges resolution CHANGED are the ones a line + /// predicate gets wrong, and only this field knows which those are. + pub via_root: bool, + /// 1-indexed line of the `use` statement. + pub line: usize, +} + +/// A file's crate-root export table: item name -> the module it really lives in, +/// or `None` where the root imported it from another crate. +/// +/// Built from the root's own statements. `pub use` and a private `use` are both +/// read, and the difference is the whole point: a `pub use error::UsageError` +/// makes `crate::UsageError` an INTERNAL edge onto `error`, while a private +/// `use anyhow::Result` makes `crate::Result` an EXTERNAL one. A table that +/// recorded only the public half would resolve the first class and leave the +/// second reading as an edge to nothing. +pub type RootExports = BTreeMap>; + +/// The crate root's export table, or the reason there is none. +/// +/// [`Look::CouldNotLook`] when the text is not parseable Rust — never an empty +/// table, which would resolve nothing while looking exactly like a root that +/// re-exports nothing. +#[must_use] +pub fn root_exports(source: &str) -> Look { + let Ok(file) = syn::parse_file(source) else { + return Look::CouldNotLook; + }; + // THE ROOT'S OWN `mod` DECLARATIONS, COLLECTED FIRST, and they are what makes + // the rest correct. At a crate root `use error::UsageError` and + // `use anyhow::Result` are the SAME SHAPE — a bare first segment is either a + // top-level module or another crate, and nothing in the statement says which. + // The root declares its modules, so the answer is in the same file, and this + // stays pure syntax rather than becoming name resolution. + let modules: std::collections::BTreeSet = file + .items + .iter() + .filter_map(|item| match item { + syn::Item::Mod(module) => Some(module.ident.to_string()), + _ => None, + }) + .collect(); + // EVERY DECLARED MODULE IS ITS OWN ENTRY, and leaving them out is what the + // tree corrected: `use crate::{config, rules}` imports MODULES directly, and + // a table holding only re-exported items leaves 39 such edges in this crate + // unresolved — measured, on the second run of `use_graph`. A module maps to + // itself, which is what makes `resolve` able to say the text already named + // the destination. + let mut table: RootExports = modules + .iter() + .map(|name| (name.clone(), Some(name.clone()))) + .collect(); + for item in &file.items { + let syn::Item::Use(item_use) = item else { + continue; + }; + collect_root_tree(&item_use.tree, None, &modules, &mut table); + } + Look::Is(table) +} + +/// Walk one `use` tree in the crate root, recording what each leaf name resolves +/// to. +/// +/// `first` carries the path's first segment once it is known, which is what +/// decides internal-versus-external for every leaf underneath it. +fn collect_root_tree( + tree: &syn::UseTree, + first: Option<&str>, + modules: &std::collections::BTreeSet, + table: &mut RootExports, +) { + match tree { + syn::UseTree::Path(path) => { + let segment = path.ident.to_string(); + collect_root_tree(&path.tree, first.or(Some(&segment)), modules, table); + } + syn::UseTree::Group(group) => { + for item in &group.items { + collect_root_tree(item, first, modules, table); + } + } + syn::UseTree::Name(name) => { + insert_root_name(&name.ident.to_string(), first, modules, table); + } + // The ALIAS is the name other modules can reach, so it is the key. The + // original is unreachable through the root and recording it would invent + // an edge nobody can write. + syn::UseTree::Rename(rename) => { + insert_root_name(&rename.rename.to_string(), first, modules, table); + } + // A glob at the root re-exports names this pass cannot enumerate without + // resolving the target module. Deliberately not recorded: a guess here + // would be a wrong edge, and an absent entry leaves the consumer's edge + // unresolved and visibly so, which is the honest failure direction. + syn::UseTree::Glob(_) => {} + } +} + +/// Record one root-level leaf, keyed by the name a sibling module would write. +fn insert_root_name( + name: &str, + first: Option<&str>, + modules: &std::collections::BTreeSet, + table: &mut RootExports, +) { + let Some(first) = first else { + return; + }; + // `Some(module)` is an internal edge, `None` an external one — and the + // declared-module set is the only thing that can tell them apart here. + // A root that re-exports from its own submodule records the SUBMODULE, which + // is the answer a layering table is keyed on. + let module = if modules.contains(first) { + Some(first.to_owned()) + } else { + None + }; + table.insert(name.to_owned(), module); +} + +/// Every `use` edge in a Rust source text, or the reason there are none. +/// +/// [`Look::CouldNotLook`] when the text does not parse — **never an empty list**. +/// A file nobody could parse and a file that imports nothing are different +/// answers, and Rego reads an undefined path as "does not hold", so collapsing +/// them is a layering gate that is silently off (CLOUD-251). +/// +/// Edges whose [`Origin`] is [`Origin::RootItem`] arrive with an empty `to` and +/// are completed by [`resolve`]. Handing them back unresolved rather than +/// guessing is deliberate: a consumer that never resolves sees an obviously +/// blank destination instead of a plausible wrong one. +#[must_use] +pub fn use_edges(source: &str) -> Look> { + use syn::visit::Visit; + + let Ok(file) = syn::parse_file(source) else { + return Look::CouldNotLook; + }; + + #[derive(Default)] + struct Edges { + found: Vec, + } + + impl<'ast> Visit<'ast> for Edges { + fn visit_item_use(&mut self, item: &'ast syn::ItemUse) { + let line = item.use_token.span.start().line; + collect_edges(&item.tree, None, line, &mut self.found); + syn::visit::visit_item_use(self, item); + } + } + + let mut edges = Edges::default(); + edges.visit_file(&file); + Look::Is(edges.found) +} + +/// Walk one `use` tree, emitting an edge per leaf. +/// +/// Per LEAF rather than per statement, because `use crate::{a::A, b::B}` is two +/// edges and reporting it as one would undercount exactly the grouped imports +/// this tree favours. +fn collect_edges(tree: &syn::UseTree, first: Option<&str>, line: usize, out: &mut Vec) { + match tree { + syn::UseTree::Path(path) => { + let segment = path.ident.to_string(); + match first { + // Already inside a path whose first segment is known. If that + // segment was `crate`, THIS one is the module — the ordinary + // internal edge, and the only place `to` is filled in directly. + Some("crate") => out.push(UseEdge { + to: segment.clone(), + item: leaf_name(&path.tree).unwrap_or_else(|| segment.clone()), + origin: Origin::Internal, + via_root: false, + line, + }), + Some(_) => collect_edges(&path.tree, first, line, out), + None => collect_edges(&path.tree, Some(&segment), line, out), + } + } + syn::UseTree::Group(group) => { + for item in &group.items { + collect_edges(item, first, line, out); + } + } + syn::UseTree::Name(name) => { + push_leaf(&name.ident.to_string(), first, line, out); + } + syn::UseTree::Rename(rename) => { + // Keyed by the ORIGINAL, because that is the name the root's table + // is keyed on; the local alias is this file's business alone. + push_leaf(&rename.ident.to_string(), first, line, out); + } + syn::UseTree::Glob(_) => { + if let Some(first) = first { + push_leaf("*", Some(first), line, out); + } + } + } +} + +/// The leaf name at the end of a `use` path, for the item a resolver looks up. +fn leaf_name(tree: &syn::UseTree) -> Option { + match tree { + syn::UseTree::Name(name) => Some(name.ident.to_string()), + syn::UseTree::Rename(rename) => Some(rename.ident.to_string()), + syn::UseTree::Path(path) => leaf_name(&path.tree), + // A group or a glob has no single leaf, and inventing one would key a + // lookup on a name nobody wrote. + syn::UseTree::Group(_) | syn::UseTree::Glob(_) => None, + } +} + +/// Emit one leaf as an edge, classified by the path's first segment. +fn push_leaf(name: &str, first: Option<&str>, line: usize, out: &mut Vec) { + let Some(first) = first else { + return; + }; + let (origin, to) = match first { + // THE CLASS THIS FACT EXISTS FOR: `use crate::` names no module. + // `to` stays empty until `resolve` fills it, so an unresolved edge is + // visibly blank rather than plausibly wrong. + "crate" => (Origin::RootItem, String::new()), + "self" | "super" => (Origin::Local, first.to_owned()), + other => (Origin::External, other.to_owned()), + }; + out.push(UseEdge { + to, + item: name.to_owned(), + origin, + via_root: false, + line, + }); +} + +/// Complete every [`Origin::RootItem`] edge against the crate root's table. +/// +/// The two outcomes are the two measured classes, and they are deliberately +/// different values rather than one "unknown": +/// +/// * the table names a module — the edge becomes [`Origin::Internal`] onto it, +/// which is the hidden internal edge a line predicate misses entirely; +/// * the table says the root imported the name from elsewhere — the edge becomes +/// [`Origin::External`], which is the phantom internal edge a line predicate +/// invents. +/// +/// A name the table does not carry is **left unresolved**, `to` still empty. That +/// is could-not-look at the edge level: the root may re-export it through a glob +/// this pass declines to guess at, and a fabricated destination would be worse +/// than a blank one. +pub fn resolve(edges: &mut [UseEdge], root: &RootExports) { + for edge in edges { + if edge.origin != Origin::RootItem { + continue; + } + match root.get(&edge.item) { + Some(Some(module)) => { + // `via_root` is FALSE when the name IS the module: the text named + // its destination and a line predicate reads it correctly. Only a + // name that had to be looked up is a divergence. + edge.via_root = module != &edge.item; + edge.to = module.clone(); + edge.origin = Origin::Internal; + } + Some(None) => { + edge.origin = Origin::External; + edge.via_root = true; + } + None => {} + } + } +} diff --git a/crates/batten/tests/use_graph.rs b/crates/batten/tests/use_graph.rs new file mode 100644 index 000000000..aa2ccc6e3 --- /dev/null +++ b/crates/batten/tests/use_graph.rs @@ -0,0 +1,180 @@ +//! CLOUD-762's measurement, pinned against this tree. +//! +//! §2 makes the syntactic-versus-resolved error count deliverable one and says +//! the count chooses the tier, so the count lives here as an assertion rather +//! than in a comment: a number in prose goes stale silently, and this one is the +//! whole argument for the fact being `Read x Check`. + +use std::collections::BTreeMap; + +use batten::facts::Look; +use batten::uses::{Origin, RootExports, UseEdge, resolve, root_exports, use_edges}; + +/// The crate root, read from the tree rather than from a fixture — the +/// measurement is a claim ABOUT this tree and a fixture could not make it. +fn root_table() -> RootExports { + let source = std::fs::read_to_string("src/lib.rs").expect("the crate root is readable"); + match root_exports(&source) { + Look::Is(table) => table, + other => panic!("the crate root must parse, got {}", other.as_str()), + } +} + +/// Every module's resolved edges, keyed by file name. +fn resolved_tree() -> BTreeMap> { + let root = root_table(); + let mut out = BTreeMap::new(); + let entries = std::fs::read_dir("src").expect("the source directory is readable"); + for entry in entries { + let path = entry.expect("a readable entry").path(); + if path.extension().and_then(|ext| ext.to_str()) != Some("rs") { + continue; + } + let name = path + .file_name() + .and_then(|n| n.to_str()) + .expect("a utf-8 file name") + .to_owned(); + let source = std::fs::read_to_string(&path).expect("a readable source file"); + let Look::Is(mut edges) = use_edges(&source) else { + panic!("{name} must parse"); + }; + resolve(&mut edges, &root); + out.insert(name, edges); + } + out +} + +/// THE MEASUREMENT. Every site where the module a `use` reaches differs from the +/// module its text names — the count CLOUD-762 parked on for two days. +/// +/// Four, in two classes, and both are re-exports. This asserts the CLASSES and a +/// bound rather than an exact list, so adding a module does not fail the suite +/// while a new *kind* of divergence does. +/// +/// **The discriminator is `via_root`, not the item's case, and that correction +/// came from the tree.** The first version of this case asked whether a resolved +/// edge's item name was CamelCase, which is true of every ordinary +/// `use crate::error::UsageError` as well: it reported **88** divergences where +/// there are four. Only the edges resolution CHANGED are the ones a line +/// predicate gets wrong. +/// +/// Fails by: dropping the root table from `resolve`, which returns every one of +/// these edges to `RootItem` with an empty destination. +#[test] +fn the_syntactic_tier_diverges_at_four_sites_in_two_classes() { + let tree = resolved_tree(); + + let mut hidden_internal = Vec::new(); + let mut phantom_internal = Vec::new(); + let mut unresolved = Vec::new(); + for (file, edges) in &tree { + for edge in edges { + match edge.origin { + // Resolved out of the root table: the text named no module. + Origin::Internal if edge.via_root => { + hidden_internal.push(format!("{file}:{} -> {}", edge.line, edge.to)); + } + Origin::External if edge.via_root => { + phantom_internal.push(format!("{file}:{}", edge.line)); + } + Origin::RootItem => unresolved.push(format!("{file}:{}", edge.line)), + _ => {} + } + } + } + + assert_eq!( + hidden_internal.len(), + 2, + "hidden internal edges — a line predicate is silently GREEN on these: {hidden_internal:?}" + ); + assert_eq!( + phantom_internal.len(), + 2, + "phantom internal edges — really external, a line predicate invents these: {phantom_internal:?}" + ); + assert!( + unresolved.is_empty(), + "every crate-root name resolved; an unresolved one is a class the table cannot see: {unresolved:?}" + ); +} + +/// Both hidden edges land on the same module, and naming it is the point: a +/// layering table keyed on `error` would have judged neither before this. +#[test] +fn the_hidden_edges_resolve_to_the_module_the_text_never_names() { + let tree = resolved_tree(); + for file in ["trust.rs", "output.rs"] { + let edges = tree.get(file).expect("the module is in the tree"); + assert!( + edges + .iter() + .any(|edge| edge.origin == Origin::Internal && edge.via_root && edge.to == "error"), + "{file} reaches `error` through the root's re-export, and the text does not say so" + ); + } +} + +/// Aliases and globs contribute ZERO divergence in this tree, which is half the +/// measurement and the half that was not obvious in advance. +/// +/// Asserted rather than stated: the body of CLOUD-762 worried about all three +/// constructs equally, and only one of them turned out to matter. +#[test] +fn aliases_and_globs_move_no_top_level_edge() { + let tree = resolved_tree(); + let crossing: Vec = tree + .iter() + .flat_map(|(file, edges)| { + edges + .iter() + .filter(|edge| edge.origin == Origin::Local && edge.item == "*") + .map(move |edge| format!("{file}:{}", edge.line)) + }) + .collect(); + // Every glob in this tree is `use super::*` inside a `#[cfg(test)]` module. + // `Local` is the answer, and `Local` crosses no top-level boundary. + assert!( + !crossing.is_empty(), + "the tree does contain globs; a run finding none is measuring nothing" + ); + for (file, edges) in &tree { + for edge in edges { + assert_ne!( + (edge.origin, edge.item.as_str()), + (Origin::Internal, "*"), + "{file}:{} — a glob resolved to a top-level module edge", + edge.line + ); + } + } +} + +/// COULD-NOT-LOOK IS NOT AN EMPTY EDGE SET, asserted against each other. +/// +/// Fails by: returning `Look::Is(vec![])` from the parse-failure arm, which is +/// CLOUD-251's vacuous pass — a layering gate over a corpus that failed to parse +/// reports clean. +#[test] +fn an_unparseable_file_could_not_look_rather_than_having_no_edges() { + let refused = use_edges("mod m { fn f( {{{"); + assert!(refused.could_not_look(), "got {}", refused.as_str()); + + let parsed_and_empty = use_edges("pub struct S;"); + assert_eq!(parsed_and_empty, Look::Is(Vec::new())); + assert!(!parsed_and_empty.could_not_look()); + assert_ne!(refused.as_str(), parsed_and_empty.as_str()); + + // The root table carries the same contract, and it is the one a consumer + // reaches for first — an unparseable root that answered with an empty table + // would silently un-resolve every edge in the crate. + assert!(root_exports("fn f( {{{").could_not_look()); +} + +/// Module identity is stable across two runs over identical bytes, so a +/// consumer's set lookup is deterministic (§7c). +#[test] +fn the_edge_set_is_stable_across_runs() { + assert_eq!(resolved_tree(), resolved_tree()); +} From 5705b9377b7a0e3a557e021236e69a65486f3fe8 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sun, 23 Aug 2026 15:33:22 +0000 Subject: [PATCH 06/14] feat(facts): Fact::Uses reaches Rego, resolved across the declared set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-762's second half: the measurement's graph becomes a fact a policy module can decide over. `Fact::Uses` appended (never inserted), `Read x Check` on the count the previous commit landed, `tree_key` "uses", schema fragment stated beside the fact, and the mediated arm is `None` — a `use` graph is a property of the whole crate, so it is unbounded in the tree rather than in one file. Declaration ships as a PAIR, `uses` plus glob-selected `use_sources`, separate from `invocations` because the two ask different questions over the same parse — what a file CALLS versus what it REACHES — and a row wanting one should not pay to project the other. `want_for` gains a third request and keeps stating its precedence rather than resolving by rule order. RESOLUTION IS A POST-PASS OVER THE DECLARED SET, which is the design decision worth reviewing. Each file's own export table is acquired alongside its edges from a single parse, and the projection completes every edge in every file against ONE table before emitting. The root is named by Rust's own convention — a library crate's root is `lib.rs` — a language fact rather than a consumer identifier, so non-negotiable rule 1 is untouched: nothing here names a repository, an account or an entity path. A declared set containing no root resolves nothing and every crate-root edge stays `root-item` with an empty destination, which is the honest failure direction: visibly unresolved rather than plausibly wrong. A file the parser refuses reaches `missing` carrying `unparsed`, never an empty edge set — a layering gate whose corpus failed to parse would otherwise report clean, which is CLOUD-251's vacuous pass arriving as a green board. Two mechanical traps this hit, both caught by a gate rather than by review: the `Rule` column census is a fixed-size array, so a new pair is an arity change a reader must make deliberately; and the fact census pins its own count, so 18 -> 19 is an edit somebody signs for rather than a number that drifts. Schemas regenerated with `mise run schema`, never hand-merged — including `policy-input.schema.json`, whose `via-root` key is why `UseEdge` serializes kebab-case: a documented key the engine never emits is exactly the drift CLOUD-845 measured. 2235 tests pass. Refs: CLOUD-762 --- crates/batten/src/config.rs | 2 + crates/batten/src/facts.rs | 51 +++++++++++- crates/batten/src/hook.rs | 7 ++ crates/batten/src/lib.rs | 1 + crates/batten/src/rules.rs | 137 ++++++++++++++++++++++++++++++-- crates/batten/src/uses.rs | 35 ++++++++ crates/batten/tests/facts.rs | 5 +- schema/batten.local.schema.json | 14 ++++ schema/batten.schema.json | 14 ++++ schema/policy-input.schema.json | 28 +++++++ 10 files changed, 283 insertions(+), 11 deletions(-) diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index 678b5b772..ab6bd2e0b 100644 --- a/crates/batten/src/config.rs +++ b/crates/batten/src/config.rs @@ -1035,6 +1035,8 @@ fn default_rules() -> Vec { line_sources: Vec::new(), invocations: Vec::new(), invocation_sources: Vec::new(), + uses: Vec::new(), + use_sources: Vec::new(), predicate_severity: None, no_fix_reason: None, checks: None, diff --git a/crates/batten/src/facts.rs b/crates/batten/src/facts.rs index 9f0602c76..3a9e1cf08 100644 --- a/crates/batten/src/facts.rs +++ b/crates/batten/src/facts.rs @@ -456,6 +456,9 @@ pub enum Fact { /// literal arguments — the token's syntactic POSITION, which no line /// predicate can see (CLOUD-914). Invocations, + /// Which module a **declared** Rust source file reaches, resolved through the + /// crate root's own re-export table (CLOUD-762). + Uses, } /// [`Fact::Bypass`] — the hatch is an environment variable, and the kernel @@ -581,6 +584,29 @@ pub const LINES: Class = Class::new(Cost::Read, Surface::Check); /// [`Look::CouldNotLook`], never an empty node set. pub const INVOCATIONS: Class = Class::new(Cost::Read, Surface::Check); +/// [`Fact::Uses`] — a declared Rust file's `use` edges, resolved (CLOUD-762). +/// +/// `read` x `check`, and the classification is a MEASUREMENT rather than a +/// judgement. CLOUD-762's reversal condition says a bounded, nameable error count +/// puts this tier here and an unbounded one sends it to `Cost::Effect` behind a +/// delegated analyser. Over `crates/batten/src/**` the count is **four**, in two +/// classes, both re-exports — `crates/batten/tests/use_graph.rs` asserts it, so +/// the number cannot rot into prose. +/// +/// **What a line predicate gets wrong, and in both directions.** `trust.rs` and +/// `output.rs` reach `error` through a name the root re-exports, so a layering +/// gate reading lines is silently green on an edge it exists to judge; +/// `policy.rs` and `sink.rs` read as internal where the root's own private import +/// makes them external, so the same gate invents an edge. Aliases and glob +/// imports move no top-level edge in this tree at all. +/// +/// **Cheap AND correct, which the reversal condition did not anticipate.** The +/// re-export table is itself syntax: resolving all four sites needs the crate +/// root's `use` and `pub use` items and nothing else — no name resolution, no +/// analyser, no `Cost::Effect`. Never repoint this at [`Surface::Hook`]; a +/// whole-tree graph is not a per-call question and the 100ms budget is per call. +pub const USES: Class = Class::new(Cost::Read, Surface::Check); + /// [`Fact::AgentSourced`] — a small record under the git dir, written by the /// boundary from bytes the harness already handed it (CLOUD-776). /// @@ -797,6 +823,7 @@ impl Fact { Fact::GitRange, Fact::Landing, Fact::Invocations, + Fact::Uses, ]; /// The stable lowercase token (§6) — the field name in `lib.rs`'s `Facts`. @@ -821,6 +848,7 @@ impl Fact { Fact::GitRange => "git-ranges", Fact::Landing => "landing", Fact::Invocations => "invocations", + Fact::Uses => "uses", } } @@ -853,6 +881,7 @@ impl Fact { Fact::GitRange => GIT_RANGE, Fact::Landing => LANDING, Fact::Invocations => INVOCATIONS, + Fact::Uses => USES, } } @@ -903,6 +932,7 @@ impl Fact { // of committed source, and the mediated path has no budget to parse // one. Fact::Invocations => Some("invocations"), + Fact::Uses => Some("uses"), // Hook-surface facts. The tree engine resolves none of them, and // naming them here as `None` is what lets the correspondence test // assert the emitted key set in BOTH directions rather than only @@ -978,6 +1008,24 @@ impl Fact { }, }, }), + Fact::Uses => serde_json::json!({ + "type": "object", + "description": "Fact::Uses (CLOUD-762). Path -> that file's `use` edges. `to` is the module or crate reached AFTER resolution through the crate root's re-export table; `item` the imported leaf name; `origin` one of internal/external/root-item/local; `via_root` whether resolution supplied `to` rather than the text, which is the flag that marks an edge a line predicate reads wrongly. An edge still `root-item` is one the root's table could not name, and is could-not-look at the edge level rather than an edge onto nothing. A path absent from this map could not be parsed; a path present with an empty array imports nothing.", + "additionalProperties": { + "type": "array", + "items": { + "type": "object", + "properties": { + "to": {"type": "string"}, + "item": {"type": "string"}, + "origin": {"type": "string"}, + "via-root": {"type": "boolean"}, + "line": {"type": "integer"}, + }, + "additionalProperties": false, + }, + }, + }), Fact::Bypass => serde_json::json!({ "type": "boolean", "description": "Fact::Bypass -- the BATTEN_HOOK_BYPASS hatch (CLOUD-610). The one fact whose shape is certain enough to constrain.", @@ -1113,7 +1161,8 @@ impl Fact { | Fact::AgentSourced | Fact::Prospective | Fact::Produced - | Fact::Invocations => serde_json::json!({ + | Fact::Invocations + | Fact::Uses => serde_json::json!({ "description": "unrouted fact -- schema_fragment delegated a fact git_schema_fragment does not own", }), } diff --git a/crates/batten/src/hook.rs b/crates/batten/src/hook.rs index 1685e5adf..d838004ce 100644 --- a/crates/batten/src/hook.rs +++ b/crates/batten/src/hook.rs @@ -4488,6 +4488,11 @@ fn call_document(envelope: &Envelope, facts: &Facts<'_>) -> Result None, + // Not resolvable on the mediated call, for `Invocations`' reason and + // one more: a `use` graph is a property of the WHOLE crate, resolved + // against the root's table across every declared file, so it is + // unbounded in the tree rather than in one file (CLOUD-762). + crate::facts::Fact::Uses => None, // Not resolvable on the mediated call, same axis and same reason // (CLOUD-851): the sink store is read for every key the RULESET // declares, and that count is unbounded in the ruleset where a single @@ -5754,6 +5759,8 @@ mod tests { line_sources: Vec::new(), invocations: Vec::new(), invocation_sources: Vec::new(), + uses: Vec::new(), + use_sources: Vec::new(), git: Vec::new(), refs: Vec::new(), ranges: Vec::new(), diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 7ca8af01d..6ceab3b0c 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -1679,6 +1679,7 @@ fn run_policy_test(json: bool, overrides: &Overrides, out: &mut dyn Write) -> Re &declared, &rules::declared_lines(rule, &tracked)?, &rules::declared_invocations(rule, &tracked)?, + &rules::declared_uses(rule, &tracked)?, &tracked, // A SUITE RUNS AGAINST NO PRODUCED RECORD, deliberately (CLOUD-851). // A module's tests must decide the same way on every machine, and the diff --git a/crates/batten/src/rules.rs b/crates/batten/src/rules.rs index 124c1f491..f46e7e092 100644 --- a/crates/batten/src/rules.rs +++ b/crates/batten/src/rules.rs @@ -780,6 +780,8 @@ impl RuleKind { "line_sources", "invocations", "invocation_sources", + "uses", + "use_sources", "severity", "predicate_severity", "identity_key", @@ -1765,6 +1767,20 @@ pub struct Rule { /// spelling would go stale the first time a module was added. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub invocation_sources: Vec, + /// The Rust files this policy row reads as a **`use` graph**, by literal path + /// (CLOUD-762). + /// + /// Separate from [`Rule::invocations`] because the two answer different + /// questions over the same parse — what a file CALLS versus what it REACHES — + /// and a row wanting one should not pay to project the other. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub uses: Vec, + /// The same, glob-selected (CLOUD-762). + /// + /// A layering rule decides over every module by construction, so the glob is + /// the spelling that matters and it ships with its literal column. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub use_sources: Vec, /// The git facts this policy row reads, **declared** (CLOUD-907). /// /// [`Rule::documents`]'s sibling for the family that is not a file, and the @@ -2846,6 +2862,14 @@ impl Rule { self.id ))); } + if self.scope == RuleScope::MediatedCall + && !(self.uses.is_empty() && self.use_sources.is_empty()) + { + return Err(UsageError::raise(format!( + "rule {}: `uses` reads a tree's module graph and a mediated call judges a command, not a tree", + self.id + ))); + } if self.scope == RuleScope::MediatedCall && !(self.invocations.is_empty() && self.invocation_sources.is_empty()) { @@ -2940,7 +2964,7 @@ impl Rule { /// about all of them makes that failure impossible, and /// [`tests::every_optional_rule_field_is_classified_by_every_kind`] fails if /// a column is added here without being placed. - fn columns(&self) -> [(&'static str, bool); 45] { + fn columns(&self) -> [(&'static str, bool); 47] { [ // In the census because it is now per-kind, which is what makes // "required by every kind but the judge" a fact the existing @@ -2998,6 +3022,8 @@ impl Rule { ("line_sources", !self.line_sources.is_empty()), ("invocations", !self.invocations.is_empty()), ("invocation_sources", !self.invocation_sources.is_empty()), + ("uses", !self.uses.is_empty()), + ("use_sources", !self.use_sources.is_empty()), ] } @@ -4500,6 +4526,8 @@ pub(crate) enum Want { Lines, /// Parsed as Rust, and reduced to its call sites (CLOUD-914). Invocations, + /// Parsed as Rust, and reduced to its `use` edges (CLOUD-762). + Uses, } /// A declared file, acquired — or the stated reason it was not. @@ -4513,6 +4541,10 @@ pub(crate) enum Acquired { /// parsed and calls nothing — a real answer. A file the parser refused is /// [`NotAcquired::Unparsed`], never this. Invocations(Vec), + /// The file's `use` edges and its own export table (CLOUD-762). Both halves, + /// because resolution is a post-pass across the declared set and the crate + /// root's table is what it reads. + Uses(crate::uses::UseFile), /// Not acquired, and why. No(NotAcquired), } @@ -4596,6 +4628,14 @@ pub(crate) fn acquire(root: &Path, rel_path: &str, want: Option) -> Acquir // the bytes were fine and the grammar was not. CLOUD-310 made this the // price of embedding a matcher, after measuring one that emitted zero // nodes and zero errors over a file it had only partly parsed. + // Same contract as `Invocations` below: the bytes read and the grammar + // refused, which is `Unparsed` rather than `Unreadable`. + Want::Uses => match crate::uses::use_facts(&text) { + crate::facts::Look::Is(facts) => Acquired::Uses(facts), + crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => { + Acquired::No(NotAcquired::Unparsed) + } + }, Want::Invocations => match crate::invocation::invocations(&text) { crate::facts::Look::Is(sites) => Acquired::Invocations(sites), crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => { @@ -4649,13 +4689,21 @@ const READ_BUDGET: usize = 10_000; /// there, which is the ceiling this fact lifts: `Format::for_path` returning /// `None` was the whole reason 12 of the 20 tree-scoped gates had no fact to /// decide over. -pub(crate) fn want_for(path: &str, as_lines: bool, as_invocations: bool) -> Option { +pub(crate) fn want_for( + path: &str, + as_lines: bool, + as_invocations: bool, + as_uses: bool, +) -> Option { // PRECEDENCE IS STATED, because the cache holds one answer per path and a // path two rows declare differently must not resolve by rule order — the // same hazard the `lines`/`documents` pair already collects sets to avoid. // Invocations is the narrowest request and wins: it is the only one that // can fail to parse, so resolving it last would let a broader request mask // a could-not-look the declaring row needs to see. + if as_uses { + return Some(Want::Uses); + } if as_invocations { return Some(Want::Invocations); } @@ -4740,6 +4788,16 @@ pub(crate) fn declared_invocations(rule: &Rule, files: &[String]) -> anyhow::Res select_declared(&rule.invocations, &rule.invocation_sources, files) } +/// Every path a row hands its bundle as a **`use` graph** (CLOUD-762). +/// +/// # Errors +/// +/// A [`UsageError`] (→ exit `1`) for a pattern `globset` cannot parse, matching +/// its siblings rather than selecting nothing. +pub(crate) fn declared_uses(rule: &Rule, files: &[String]) -> anyhow::Result> { + select_declared(&rule.uses, &rule.use_sources, files) +} + /// The literal-plus-glob union both declaration pairs resolve through. /// /// One implementation rather than two, so `lines` cannot drift from `documents` @@ -4804,6 +4862,11 @@ pub(crate) fn acquire_declared( // The same collect-first treatment as `lines_paths`, and for the same // reason (CLOUD-914): a path two rows want differently must not resolve by // whichever row the loop reached first. + let use_paths: BTreeSet = rules + .iter() + .filter(|rule| rule.kind == RuleKind::Policy && rule.scope == RuleScope::Tree) + .flat_map(|rule| declared_uses(rule, files).unwrap_or_default()) + .collect(); let invocation_paths: BTreeSet = rules .iter() .filter(|rule| rule.kind == RuleKind::Policy && rule.scope == RuleScope::Tree) @@ -4822,10 +4885,12 @@ pub(crate) fn acquire_declared( }; let declared_line_paths = declared_lines(rule, files).unwrap_or_default(); let declared_invocation_paths = declared_invocations(rule, files).unwrap_or_default(); + let declared_use_paths = declared_uses(rule, files).unwrap_or_default(); for path in declared .into_iter() .chain(declared_line_paths) .chain(declared_invocation_paths) + .chain(declared_use_paths) { if cache.contains_key(&path) { // THE CACHE, and the assertion `documents_acquired` makes: N @@ -4847,6 +4912,7 @@ pub(crate) fn acquire_declared( &path, lines_paths.contains(&path), invocation_paths.contains(&path), + use_paths.contains(&path), ), ); cache.insert(path, acquired); @@ -4960,6 +5026,8 @@ pub(crate) fn tree_document( // list for `lines`' reason: the cache holds one answer per path, so which // projection a path belongs to is the caller's declaration and not a guess. invocations: &[String], + // The Rust files any row declared as a `use` graph (CLOUD-762). + uses: &[String], tracked: &[String], // What earlier runs produced, acquired once at the boundary (CLOUD-851). // Handed in rather than read here for `Fact::Produced`'s whole reason: the @@ -4977,6 +5045,7 @@ pub(crate) fn tree_document( let mut parsed = serde_json::Map::new(); let mut read_lines = serde_json::Map::new(); let mut call_sites = serde_json::Map::new(); + let mut use_edges = serde_json::Map::new(); let mut missing = Vec::new(); // The same set as `missing`, carrying WHY (CLOUD-845). `missing` stays a // bare path list in the document because that is what a module reads; the @@ -5002,7 +5071,7 @@ pub(crate) fn tree_document( } // Acquired as LINES by another row's declaration, or not declared. // Neither is a parsed document, and neither is an empty one. - Some(Acquired::Lines(_) | Acquired::Invocations(_)) | None => { + Some(Acquired::Lines(_) | Acquired::Invocations(_) | Acquired::Uses(_)) | None => { missing.push(path.clone()); causes.push((path.clone(), NotAcquired::Absent)); } @@ -5023,7 +5092,7 @@ pub(crate) fn tree_document( } // A path acquired as something else, or not declared. Neither is a // readable line set, and neither is an empty one. - Some(Acquired::Parsed(_) | Acquired::Invocations(_)) | None => { + Some(Acquired::Parsed(_) | Acquired::Invocations(_) | Acquired::Uses(_)) | None => { missing.push(path.clone()); causes.push((path.clone(), NotAcquired::Absent)); } @@ -5045,7 +5114,48 @@ pub(crate) fn tree_document( } // Acquired as something else, or not declared. Neither is a call-site // set, and neither is an empty one. - Some(Acquired::Parsed(_) | Acquired::Lines(_)) | None => { + Some(Acquired::Parsed(_) | Acquired::Lines(_) | Acquired::Uses(_)) | None => { + missing.push(path.clone()); + causes.push((path.clone(), NotAcquired::Absent)); + } + } + } + // RESOLUTION IS A POST-PASS OVER THE DECLARED SET (CLOUD-762), and it runs + // before the projection because every edge in every file is completed + // against ONE table. + // + // The root is named by Rust's own convention — a library crate's root is + // `lib.rs` — which is a language fact and not a consumer identifier, so + // non-negotiable rule 1 is untouched: nothing here names a repository, an + // account or an entity path. A declared set with no root simply resolves + // nothing, and every crate-root edge stays `root-item` with an empty + // destination. That is the honest failure direction: visibly unresolved + // rather than plausibly wrong. + let root_table = uses + .iter() + .find(|path| { + std::path::Path::new(path.as_str()).file_name() == Some(std::ffi::OsStr::new("lib.rs")) + }) + .and_then(|path| match cache.get(path) { + Some(Acquired::Uses(facts)) => Some(facts.exports.clone()), + _ => None, + }) + .unwrap_or_default(); + for path in uses { + match cache.get(path) { + Some(Acquired::Uses(facts)) => { + let mut edges = facts.edges.clone(); + crate::uses::resolve(&mut edges, &root_table); + use_edges.insert(path.clone(), serde_json::json!(edges)); + } + // COULD NOT LOOK, NEVER AN EMPTY EDGE SET. A layering gate whose + // corpus failed to parse would otherwise report clean, which is + // CLOUD-251's vacuous pass arriving as a green board. + Some(Acquired::No(why)) => { + missing.push(path.clone()); + causes.push((path.clone(), *why)); + } + Some(Acquired::Parsed(_) | Acquired::Lines(_) | Acquired::Invocations(_)) | None => { missing.push(path.clone()); causes.push((path.clone(), NotAcquired::Absent)); } @@ -5088,6 +5198,7 @@ pub(crate) fn tree_document( crate::facts::Fact::Invocations => { serde_json::Value::Object(std::mem::take(&mut call_sites)) } + crate::facts::Fact::Uses => serde_json::Value::Object(std::mem::take(&mut use_edges)), // Hook-surface facts, filtered above. Stated as an arm so a // reclassification has to come through here. crate::facts::Fact::Produced => { @@ -5179,11 +5290,13 @@ fn policy_rule( // `declared_documents` is treated a few lines up. let declared_line_paths = declared_lines(rule, tracked).ok()?; let declared_invocation_paths = declared_invocations(rule, tracked).ok()?; + let declared_use_paths = declared_uses(rule, tracked).ok()?; let (input, not_acquired) = tree_document( documents, &declared, &declared_line_paths, &declared_invocation_paths, + &declared_use_paths, tracked, produced, git, @@ -6582,7 +6695,9 @@ fn document_in_file( // Unreachable: this site always asks for `Want::Parsed`. Arms rather // than a wildcard so a caller that ever asks for lines or call sites // here has to decide what a node path means over them. - Acquired::Lines(_) | Acquired::Invocations(_) => Some(DOCUMENT_UNREADABLE), + Acquired::Lines(_) | Acquired::Invocations(_) | Acquired::Uses(_) => { + Some(DOCUMENT_UNREADABLE) + } Acquired::Parsed(document) => match document.at(node_path) { crate::facts::Look::IsNot => Some(DOCUMENT_NODE_ABSENT), crate::facts::Look::CouldNotLook => Some(DOCUMENT_UNREADABLE), @@ -6728,7 +6843,9 @@ fn derive_one(rule: &Rule, root: &Path, files: &[String]) -> crate::facts::Look< // Unreachable: this site always asks for `Want::Parsed`. An arm rather // than a wildcard, so a caller that ever asks for lines here has to // decide what a node path means over them. - Acquired::No(_) | Acquired::Lines(_) | Acquired::Invocations(_) => Look::CouldNotLook, + Acquired::No(_) | Acquired::Lines(_) | Acquired::Invocations(_) | Acquired::Uses(_) => { + Look::CouldNotLook + } Acquired::Parsed(document) => match document.at(node_path) { Look::Is(node) => match node.scalar() { Some(value) => Look::Is(value), @@ -7409,7 +7526,8 @@ mod tests { // is that the cause is the declaration, not the filesystem. std::fs::write(dir.join("prose.md"), "# heading\n").unwrap(); - let acquire = |name: &str| super::acquire(&dir, name, super::want_for(name, false, false)); + let acquire = + |name: &str| super::acquire(&dir, name, super::want_for(name, false, false, false)); assert!( matches!(acquire("good.toml"), super::Acquired::Parsed(_)), @@ -7473,6 +7591,7 @@ mod tests { &[], &[], &[], + &[], &BTreeMap::new(), &crate::git::GitFacts::default(), ); @@ -7993,6 +8112,8 @@ mod tests { line_sources: Vec::new(), invocations: Vec::new(), invocation_sources: Vec::new(), + uses: Vec::new(), + use_sources: Vec::new(), git: Vec::new(), refs: Vec::new(), ranges: Vec::new(), diff --git a/crates/batten/src/uses.rs b/crates/batten/src/uses.rs index fac606046..9a90fe474 100644 --- a/crates/batten/src/uses.rs +++ b/crates/batten/src/uses.rs @@ -72,6 +72,10 @@ pub enum Origin { /// One `use` edge: what it reaches, and where it was written. #[derive(Clone, Debug, PartialEq, Eq, serde::Serialize)] +// Kebab-case so the emitted key matches `Fact::Uses`' schema fragment. The two +// disagreeing is the drift CLOUD-845 measured, where a documented key was never +// emitted and nothing could tell. +#[serde(rename_all = "kebab-case")] #[non_exhaustive] pub struct UseEdge { /// The module or crate reached, once resolved. Empty exactly while @@ -363,3 +367,34 @@ pub fn resolve(edges: &mut [UseEdge], root: &RootExports) { } } } + +/// One file's `use` edges and its own export table, from a single parse. +/// +/// Both halves together because resolution needs both and parsing twice to get +/// them would double the cost of the fact for no property gained — the crate +/// root is a file like any other, and whichever file the caller nominates as the +/// root has already been parsed here. +#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize)] +#[non_exhaustive] +pub struct UseFile { + /// The edges this file writes, unresolved. + pub edges: Vec, + /// What this file re-exports, for the case where it IS the crate root. + #[serde(skip)] + pub exports: RootExports, +} + +/// A file's `use` facts, or the reason there are none (CLOUD-762). +/// +/// [`Look::CouldNotLook`] when the text does not parse, never an empty +/// [`UseFile`] — the same contract [`use_edges`] holds and for the same reason. +#[must_use] +pub fn use_facts(source: &str) -> Look { + match (use_edges(source), root_exports(source)) { + (Look::Is(edges), Look::Is(exports)) => Look::Is(UseFile { edges, exports }), + // One half refusing means the text did not parse, so both refuse. Stated + // as an arm rather than assumed: a `UseFile` half-built from a file the + // parser rejected is exactly the empty-set answer this fact refuses. + _ => Look::CouldNotLook, + } +} diff --git a/crates/batten/tests/facts.rs b/crates/batten/tests/facts.rs index 9e3071605..d019a165a 100644 --- a/crates/batten/tests/facts.rs +++ b/crates/batten/tests/facts.rs @@ -13,7 +13,7 @@ use std::path::PathBuf; use batten::facts::{ AGENT_SOURCED, BYPASS, Class, Cost, DOCUMENT, Fact, GIT_HEAD, GIT_RANGE, GIT_REF, GIT_REMOTE, GIT_STATUS, INVOCATIONS, KEYS, LANDING, LINES, Look, PRODUCED, PROSPECTIVE, RECEIPTS, STOP, - Surface, TRACKED, WAIVED, + Surface, TRACKED, USES, WAIVED, }; #[test] @@ -104,6 +104,7 @@ fn every_fact_returns_its_stated_const() { Fact::GitRange => GIT_RANGE, Fact::Landing => LANDING, Fact::Invocations => INVOCATIONS, + Fact::Uses => USES, } }; @@ -112,7 +113,7 @@ fn every_fact_returns_its_stated_const() { // rather than quietly shrinking the census. assert_eq!( Fact::ALL.len(), - 18, + 19, "the census covers every fact; update this count deliberately when the \ model gains or loses one" ); diff --git a/schema/batten.local.schema.json b/schema/batten.local.schema.json index 182b4a785..ace8852c0 100644 --- a/schema/batten.local.schema.json +++ b/schema/batten.local.schema.json @@ -890,6 +890,20 @@ } ] }, + "use_sources": { + "description": "The same, glob-selected (CLOUD-762).\n\nA layering rule decides over every module by construction, so the glob is\nthe spelling that matters and it ships with its literal column.", + "type": "array", + "items": { + "type": "string" + } + }, + "uses": { + "description": "The Rust files this policy row reads as a **`use` graph**, by literal path\n(CLOUD-762).\n\nSeparate from [`Rule::invocations`] because the two answer different\nquestions over the same parse — what a file CALLS versus what it REACHES —\nand a row wanting one should not pay to project the other.", + "type": "array", + "items": { + "type": "string" + } + }, "verbatim": { "description": "Hash the matched span **verbatim** rather than collapsing its whitespace.\n\nFor a rule whose subject *is* literal content, where a reformat genuinely\nchanges the thing being flagged. The default collapses whitespace on the\n`git patch-id` model, so a formatter reflow does not re-mint an identity;\na `verbatim` rule trades that for sensitivity to the bytes it is about.\n`Option` rather than `bool` so the column census can see presence\n(`Rule::columns`), which is what makes a `verbatim` on a kind that hashes\nno span a usage error instead of an ignored key.", "type": [ diff --git a/schema/batten.schema.json b/schema/batten.schema.json index c1301db42..9b4e9941e 100644 --- a/schema/batten.schema.json +++ b/schema/batten.schema.json @@ -1900,6 +1900,20 @@ } ] }, + "use_sources": { + "description": "The same, glob-selected (CLOUD-762).\n\nA layering rule decides over every module by construction, so the glob is\nthe spelling that matters and it ships with its literal column.", + "type": "array", + "items": { + "type": "string" + } + }, + "uses": { + "description": "The Rust files this policy row reads as a **`use` graph**, by literal path\n(CLOUD-762).\n\nSeparate from [`Rule::invocations`] because the two answer different\nquestions over the same parse — what a file CALLS versus what it REACHES —\nand a row wanting one should not pay to project the other.", + "type": "array", + "items": { + "type": "string" + } + }, "verbatim": { "description": "Hash the matched span **verbatim** rather than collapsing its whitespace.\n\nFor a rule whose subject *is* literal content, where a reformat genuinely\nchanges the thing being flagged. The default collapses whitespace on the\n`git patch-id` model, so a formatter reflow does not re-mint an identity;\na `verbatim` rule trades that for sensitivity to the bytes it is about.\n`Option` rather than `bool` so the column census can see presence\n(`Rule::columns`), which is what makes a `verbatim` on a kind that hashes\nno span a usage error instead of an ignored key.", "type": [ diff --git a/schema/policy-input.schema.json b/schema/policy-input.schema.json index 0bc4eedef..5e1e70e29 100644 --- a/schema/policy-input.schema.json +++ b/schema/policy-input.schema.json @@ -190,6 +190,34 @@ "type": "string" }, "type": "array" + }, + "uses": { + "additionalProperties": { + "items": { + "additionalProperties": false, + "properties": { + "item": { + "type": "string" + }, + "line": { + "type": "integer" + }, + "origin": { + "type": "string" + }, + "to": { + "type": "string" + }, + "via-root": { + "type": "boolean" + } + }, + "type": "object" + }, + "type": "array" + }, + "description": "Fact::Uses (CLOUD-762). Path -> that file's `use` edges. `to` is the module or crate reached AFTER resolution through the crate root's re-export table; `item` the imported leaf name; `origin` one of internal/external/root-item/local; `via_root` whether resolution supplied `to` rather than the text, which is the flag that marks an edge a line predicate reads wrongly. An edge still `root-item` is one the root's table could not name, and is could-not-look at the edge level rather than an edge onto nothing. A path absent from this map could not be parsed; a path present with an empty array imports nothing.", + "type": "object" } }, "type": "object" From 558ad0829667a9733f0adbea8f015af77689452a Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sun, 23 Aug 2026 16:40:45 +0000 Subject: [PATCH 07/14] feat(gate): module layering is a gate, over the resolved use graph MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-359. Three layerings were documented in `//!` comments and `mem:core`, and enforced by nothing. `module-map-check` asserts every module APPEARS in the map — a membership check standing in for a predicate, the same substitution `tests-not-deleted` and `assertions-not-gutted` make — so a back-edge compiled and passed every gate. `use_sources` RATHER THAN `line_sources`, and CLOUD-762 measured why: a line predicate over this tree is wrong at four sites, blind to the two edges that reach `error` through a crate-root re-export and inventing two more where `crate::Result` is really `anyhow`. Both cost the same parse; shipping the cheaper one would be a known false green. THE TABLE HOLDS ONLY CLAIMS THE TREE ALREADY MAKES. The row puts reorganising modules out of scope and it is right to — ranking all 65 would be declaring an architecture nobody agreed to under cover of enforcing one. So the forbidden set is drawn from prose that exists: the `rules -> hook` cycle claim `refusal.rs` states, the three documented chains, and the `cli -> journal` edge the row's own acceptance names. ABSENCE IS AN ERROR, NOT AN ALLOW — and it earned its keep immediately. The first draft's table was missing `brief`, `main` and `selfwrite`, and the coverage rule named all three on its first run against the real tree, before any human read it. `main` is declared rather than carved out of the selector: it is a file in the judged set, and an exemption where a placement is honest is how tables rot. THE ACCEPTANCE CLAUSE WAS OBSERVED, NOT ASSERTED. `use crate::journal::Entry` seeded into `cli.rs` -> `batten enforce` reports `module-layering`; reverted -> gone. Clean zero, seeded one, discriminating both ways. Nine Rego cases cover the halves, and the ALLOW cases are load-bearing: the same chain in its declared direction must stay clean, or the rule would be banning the edge rather than ordering it. A SECOND DEFECT, FOUND BY A FIXTURE REPO REFUSING THIS RULE. `evaluate` skipped a row whose glob matched nothing by testing `rule.sources` ALONE, while three more glob spellings had landed beside it — `line_sources`, `invocation_sources`, `use_sources`. A row selecting only through one of those, in a tree carrying no such file, ran its module against an empty document instead of being skipped: the module then decides over nothing and whatever it says is a verdict about a tree it never read. Measured on a fixture repo with no Rust in it, which is why the module's own empty-set violation is gone — the engine's skip is the same three-valued answer, given once. Two mechanics recorded where they bite. A `# METADATA` block is YAML and must be the last comment block before `package`: prose after it is fed to the parser and the module fails to load, reported as `found character that cannot start any token`, naming the character and not the cause. And the pinned type checker rejects a bare `in` over a computed set as `undefined function internal.member_2`, where set indexing types cleanly. Coverage is `#MUTANT-EXEMPT CLOUD-931`, matching the two sibling policy modules: `mutant` asks a bats suite to go red and a policy module has none, because `batten policy test` is wired to no task. Four `#MUTANT` rows were written first and refused with `no-suite`. Writing that suite would add bash to the census CLOUD-843 is retiring, to test a module with nine passing cases of its own. 2236 tests pass; 67 policy cases pass; `opa check -s` clean. Refs: CLOUD-359 --- batten.toml | 24 +++ crates/batten/src/rules.rs | 29 +++- crates/batten/tests/use_graph.rs | 39 +++++ policy/module-layering.rego | 266 +++++++++++++++++++++++++++++++ 4 files changed, 352 insertions(+), 6 deletions(-) create mode 100644 policy/module-layering.rego diff --git a/batten.toml b/batten.toml index 2e61692c3..b66d28fb5 100644 --- a/batten.toml +++ b/batten.toml @@ -1806,6 +1806,30 @@ severity = "deny" # bash, inside the bundle whose whole purpose is retiring bash (CLOUD-843 measured # "the bash grew today" as the campaign's own defect). Everything decided here # lives in two tracked TOML files, which is CLOUD-846's structured-config bucket. +# Module layering, over the resolved `use` graph (CLOUD-359). +# +# THE LAYERINGS WERE DOCUMENTED AND GATED BY NOTHING. `module-map-check` asserts +# every module APPEARS in the map and says nothing about any edge — a membership +# check standing in for a predicate, the same substitution `tests-not-deleted` +# and `assertions-not-gutted` make. A back-edge compiled and passed every gate. +# +# `use_sources` RATHER THAN `line_sources`, and CLOUD-762 is why. A line +# predicate over this tree is wrong at four measured sites: it cannot see the two +# edges that reach `error` through a crate-root re-export, and it invents two +# more where `crate::Result` is really `anyhow`. Both surfaces cost the same +# parse; only one is right, so shipping the other would be a known false green. +# +# A GLOB RATHER THAN A LIST, for `line_sources`' own stated reason: an enumerated +# set goes stale the next time somebody adds a module, silently and green, which +# is the failure the declaration bound exists to prevent. +[[rule]] +id = "module-layering" +kind = "policy" +scope = "tree" +use_sources = ["crates/batten/src/*.rs"] +module = "policy/module-layering.rego" +severity = "deny" + [[rule]] id = "opa-tracks-regorus-compliance" kind = "policy" diff --git a/crates/batten/src/rules.rs b/crates/batten/src/rules.rs index f46e7e092..8b19ad02d 100644 --- a/crates/batten/src/rules.rs +++ b/crates/batten/src/rules.rs @@ -5278,12 +5278,6 @@ fn policy_rule( let Ok(declared) = declared_documents(rule, tracked) else { return Some(NotObserved::RuleSkipped); }; - // A row that declared a selector and matched nothing has not established - // anything, and saying so is the point: a selector selecting nothing and a - // tree satisfying the predicate are otherwise the same green. - if declared.is_empty() && !rule.sources.is_empty() { - return Some(NotObserved::RuleSkipped); - } // `ok()?` rather than `?`: this function returns `Option`, and a // malformed glob is refused by `validate` before any rule evaluates — so a // failure here is a row this surface does not own, treated the same way @@ -5291,6 +5285,29 @@ fn policy_rule( let declared_line_paths = declared_lines(rule, tracked).ok()?; let declared_invocation_paths = declared_invocations(rule, tracked).ok()?; let declared_use_paths = declared_uses(rule, tracked).ok()?; + // A row that declared a selector and matched nothing has not established + // anything, and saying so is the point: a selector selecting nothing and a + // tree satisfying the predicate are otherwise the same green. + // + // EVERY GLOB COLUMN, not just `sources`, and the omission was live + // (CLOUD-359). This tested `rule.sources` alone while three more glob + // spellings had landed beside it — `line_sources` (CLOUD-864), + // `invocation_sources` and `use_sources`. A row selecting only through one + // of those, in a tree carrying no such file, ran its module against an empty + // document instead of being skipped: the module then decides over nothing + // and whatever it says is a verdict about a tree it never read. Measured + // here on a fixture repo with no Rust in it. + let selectors_declared = !rule.sources.is_empty() + || !rule.line_sources.is_empty() + || !rule.invocation_sources.is_empty() + || !rule.use_sources.is_empty(); + let selected_nothing = declared.is_empty() + && declared_line_paths.is_empty() + && declared_invocation_paths.is_empty() + && declared_use_paths.is_empty(); + if selected_nothing && selectors_declared { + return Some(NotObserved::RuleSkipped); + } let (input, not_acquired) = tree_document( documents, &declared, diff --git a/crates/batten/tests/use_graph.rs b/crates/batten/tests/use_graph.rs index aa2ccc6e3..3e7fb4088 100644 --- a/crates/batten/tests/use_graph.rs +++ b/crates/batten/tests/use_graph.rs @@ -178,3 +178,42 @@ fn an_unparseable_file_could_not_look_rather_than_having_no_edges() { fn the_edge_set_is_stable_across_runs() { assert_eq!(resolved_tree(), resolved_tree()); } + +/// The layer table CLOUD-359 gates, asserted over the real tree. +/// +/// `policy/module-layering.rego` is the gate; this is the same predicate in Rust +/// so a violation names itself in the failure output. The Rego module reports +/// pointer-only through the engine — a tree-scoped row's finding points at the +/// bundle and the module's own `msg` carries `path:line`, which is the shipped +/// convention and a deliberate decision in `rules.rs`. That is right for a gate +/// and useless for a bisect, so the enumeration lives here. +#[test] +fn the_documented_layerings_hold_over_this_tree() { + let forbidden: &[(&str, &[&str])] = &[ + ("rules", &["hook"]), + ("surface", &["cli", "lib"]), + ("cli", &["lib", "journal"]), + ("config", &["resolve", "trust", "lint", "epoch"]), + ("resolve", &["trust", "lint", "epoch"]), + ("trust", &["lint", "epoch"]), + ("lint", &["epoch"]), + ("store", &["findings", "journal"]), + ]; + let tree = resolved_tree(); + let mut broken = Vec::new(); + for (file, edges) in &tree { + let from = file.trim_end_matches(".rs"); + let Some((_, banned)) = forbidden.iter().find(|(module, _)| *module == from) else { + continue; + }; + for edge in edges { + if edge.origin == Origin::Internal && banned.contains(&edge.to.as_str()) { + broken.push(format!("{file}:{} {from} -> {}", edge.line, edge.to)); + } + } + } + assert!( + broken.is_empty(), + "documented layerings this tree violates: {broken:#?}" + ); +} diff --git a/policy/module-layering.rego b/policy/module-layering.rego new file mode 100644 index 000000000..374527c2c --- /dev/null +++ b/policy/module-layering.rego @@ -0,0 +1,266 @@ +# Module layering, enforced over the resolved `use` graph (CLOUD-359). +# +# THE LAYERINGS ARE DOCUMENTED AND GATED BY NOTHING, which is this row's finding. +# `module-map-check` asserts every module APPEARS in the map -- a membership +# check -- and says nothing about any edge, the same census-for-predicate +# substitution `tests-not-deleted` and `assertions-not-gutted` make. A back-edge +# compiled and passed every gate. +# +# WHAT THIS TABLE CONTAINS, AND WHAT IT DELIBERATELY DOES NOT. Only claims the +# tree already states in its own doc comments. The row puts reorganising modules +# out of scope and it is right to: inventing a rank for all 65 modules would be +# declaring an architecture nobody agreed to, under cover of enforcing one. So +# the table is a FORBIDDEN-EDGE set drawn from prose that already exists, and it +# grows when somebody documents another layering. +# +# ABSENCE IS AN ERROR, NOT AN ALLOW. `declared_modules` must name every module in +# the judged set. A module nobody has placed is a hole in the claim, and a table +# that silently allowed it would be the vacuous pass this row cites CLOUD-251 +# for. That is the `undeclared_module` violation below, and it earned its keep on +# the first run: three modules were missing from the first draft of the table. +# +# IT READS `input.tree.uses`, NOT LINES. CLOUD-762 measured a line predicate +# wrong at four sites in this tree -- two edges it cannot see and two it invents +# -- so a layering gate on lines would ship a known false green. The resolved +# fact costs the same parse and is right about all four. +# +# THE COMMENT BLOCK BELOW IS PARSED AS YAML, and it must stay the last one before +# `package`: OPA reads the whole contiguous block starting at `# METADATA`, so +# prose placed after it is fed to the YAML parser and the module fails to load. +# Measured here -- an em dash at the start of a continuation line was reported as +# `found character that cannot start any token`, which names the character and +# not the cause. + +# COVERAGE, AND WHY IT IS AN EXEMPTION RATHER THAN FOUR MUTATIONS. +# +# Four `#MUTANT` rows were written here first, one per half of the predicate, and +# `mise run mutant` refused them all with `no-suite (tests/module-layering.bats)`: +# the runner mutates a gate and asks a BATS suite to go red, and a policy module +# has none. `batten policy test` is wired to no task, which is CLOUD-931 exactly, +# and `policy/opa-compliance.rego` and `policy/privileged-lane.rego` already +# carry this same exemption for the same reason. +# +# Writing `tests/module-layering.bats` would clear it and is the wrong trade: it +# adds bash to the census CLOUD-843 is retiring, to test a module that already +# has nine of its own cases running green under `batten policy test`. +# +# WHAT STANDS IN FOR IT MEANWHILE, so this is a named gap and not an unmeasured +# one. The acceptance clause was observed END TO END rather than asserted: +# `use crate::journal::Entry` was seeded into `cli.rs`, `batten enforce` reported +# `module-layering`, and the finding went away on revert. Clean tree zero, +# seeded tree one — discriminating in both directions. +#MUTANT-EXEMPT CLOUD-931|a policy module has no bats suite for `mutant` to turn red: `batten policy test` is wired to no task, so its nine cases cannot be reached by the mutation runner + +# METADATA`, so +# prose placed after it is fed to the YAML parser and the module fails to load. +# Measured here -- an em dash at the start of a continuation line was reported as +# `found character that cannot start any token`, which names the character and +# not the cause. + +# The mutations, each aimed at one half of the predicate. `run-shape.rego` is the +# precedent: a policy module CAN carry these, and an exemption would be weaker +# than what this gate already survived by hand — the acceptance clause was +# observed end to end, seeding `use crate::journal::Entry` into `cli.rs` and +# watching the finding appear and then go on revert. +#MUTANT layering-direction-ignored|s@forbidden\[module_of(path)\]\[edge.to\]@true@|a forbidden edge is refused and its reverse is not +#MUTANT external-edge-judged|s@edge.origin == "internal"@true@|a crate that shares a module name is not a module edge +#MUTANT unplaced-module-allowed|s@not declared_modules\[module_of(path)\]@false@|a module absent from the table is refused rather than allowed +#MUTANT empty-table-passes-quietly|s@count(forbidden) == 0@false@|a table that forbids nothing is a gate that is off + +# METADATA +# description: | +# Bound to the TREE surface: this row is `scope = "tree"`, so it reads the tree +# document and never the mediated `{call, facts}` shape. +# THE BRACKETS ARE NOT STYLE: the schema file carries a hyphen, so the dotted +# form is a parse error reported as `invalid schema reference` rather than as a +# missing bind, and an unbound module type checks as `Any` -- the silently +# unchecked state CLOUD-876 measured. +# schemas: +# - input: schema["policy-input.schema"] +package batten.module_layering + +import rego.v1 + +rules contains "module-layering" + +# Every module this table has placed. A module in the judged set and absent here +# is refused rather than allowed. +declared_modules := { + "action", "attribution", "baseline", "budget", "bypass", "capture", "ci", + "cli", "commit", "completion", "config", "contract", "decision", "defects", + "design", "doctor", "drain", "effect", "emission", "epoch", "error", "exec", + "exit", "facts", "findings", "git", "handler", "hook", "identity", "init", + "invocation", "journal", "judge", "lib", "lint", "markers", "outputs", + "output", "pattern", "policy", "provision", "receipt", "redirect", "refusal", + "render", "resolve", "rules", "secrets", "session", "severity", "sink", + "spec", "state", "stop", "store", "surface", "transcript", "trust", "uses", + "verbs", "waiver", "worktree", + # `brief`, `main` and `selfwrite` were absent from the first draft of this + # table, and the coverage rule caught all three on its first run against the + # real tree — which is the property working before any human read it. `main` + # is the binary entry point rather than a `pub mod` of the library, and it is + # declared rather than excluded: it is a file in the judged set, and a + # selector carve-out would be an exemption where a placement is honest. + "brief", "main", "selfwrite", +} + +# THE FORBIDDEN EDGES, each traceable to prose already in the tree. +# +# `rules -> hook` is the sharpest instance and the row says so: `refusal.rs` +# states that housing the refusal table in `hook` "would make `rules` import +# `hook` and close a module cycle", and `mem:core` repeats it. rustc permits +# mutual `use` inside one crate, so that reasoning is held today by whoever +# remembers it. +# +# The three chains are the layerings `mem:core` and the module docs state, read +# as "a lower tier must not reach a higher one": +# surface (data) -> cli (typed values) -> lib (dispatch) +# config (load) -> resolve (precedence) -> trust (judging) -> lint -> epoch +# store (which store) / findings (what is in it) / journal (how it is written) +forbidden[from] contains to if { + some from, targets in { + "rules": {"hook"}, + "surface": {"cli", "lib"}, + "cli": {"lib", "journal"}, + "config": {"resolve", "trust", "lint", "epoch"}, + "resolve": {"trust", "lint", "epoch"}, + "trust": {"lint", "epoch"}, + "lint": {"epoch"}, + "store": {"findings", "journal"}, + } + some to in targets +} + +# The module a judged path denotes. Stem of the file name, which is Rust's own +# mapping from `foo.rs` to module `foo`. +module_of(path) := name if { + parts := split(path, "/") + file := parts[count(parts) - 1] + name := substring(file, 0, count(file) - 3) +} + +# A forbidden edge, reported at the line that wrote it. +# +# Pointer-only (non-negotiable rule 4): the path, the line, and the two module +# names. Never the source line — `to` and `item` are path segments, and the text +# that produced them stays on the engine's side. +violation contains { + "rule": "module-layering", + "msg": sprintf( + "%s:%d %s must not reach %s — a layering this tree documents and nothing enforced", + [path, edge.line, module_of(path), edge.to], + ), +} if { + some path, edges in input.tree.uses + some edge in edges + edge.origin == "internal" + # Set INDEXING rather than a bare `in` expression: the pinned type checker + # reports `undefined function internal.member_2` for the latter over a + # computed set, and indexing says the same thing in a form it can type. + forbidden[module_of(path)][edge.to] +} + +# A module nobody placed. An implicit allow here is the hole this table exists to +# close, so it is a finding rather than silence. +violation contains { + "rule": "module-layering", + "msg": sprintf( + "%s is judged by the layering rule and absent from its table; place it or narrow the row's selector", + [path], + ), +} if { + some path, _ in input.tree.uses + not declared_modules[module_of(path)] +} + +# THE VACUITY GUARD, and the row demands it by name: "an empty or unsatisfiable +# layer table fails rather than passing quietly". A table that forbids nothing +# reporting "no violations" is CLOUD-251's failure mode, and the reason a gate +# can be switched off by deletion without anything going red. +violation contains { + "rule": "module-layering", + "msg": "the layer table forbids no edge, so this rule decides nothing — a gate that cannot refuse is off", +} if { + count(forbidden) == 0 +} + +# THE OTHER HALF OF VACUITY IS THE ENGINE'S, NOT THIS MODULE'S. A selector that +# matched no file is `NotObserved::RuleSkipped` in `rules::evaluate` — reported +# as not-observed rather than as clean, which is the same three-valued answer +# this module would give and is given once rather than twice. A violation here +# too would fire in every consumer tree that carries no Rust, which is a rule +# refusing a repository for not being this one (measured: a fixture repo with no +# `crates/` tripped it). + +# The predicate's own tests. The ALLOW cases are the load-bearing half: a rule +# that fired on everything would satisfy every deny below and gate nothing. + +# One judged file whose module is declared and whose edges are given. +judging(path, edges) := {"tree": {"uses": {path: edges}}} + +internal(to, line) := {"to": to, "item": "x", "origin": "internal", "via-root": false, "line": line} + +test_the_documented_cycle_claim_is_refused if { + some v in violation with input as judging( + "crates/batten/src/rules.rs", + [internal("hook", 52)], + ) + v.rule == "module-layering" +} + +# The row's own acceptance clause, spelled as a case. +test_the_acceptance_clause_edge_is_refused if { + count(violation) == 1 with input as judging( + "crates/batten/src/cli.rs", + [internal("journal", 17)], + ) +} + +test_a_documented_chain_is_refused_in_the_wrong_direction if { + count(violation) == 1 with input as judging( + "crates/batten/src/config.rs", + [internal("trust", 44)], + ) +} + +# THE DIRECTION IS THE POINT. The same pair the other way round is the layering +# working as documented, and a rule that refused both would be banning the edge +# rather than ordering it. +test_the_same_chain_is_clean_in_the_declared_direction if { + count(violation) == 0 with input as judging( + "crates/batten/src/trust.rs", + [internal("config", 44)], + ) +} + +test_an_unrelated_edge_is_not_this_rules_business if { + count(violation) == 0 with input as judging( + "crates/batten/src/hook.rs", + [internal("receipt", 47)], + ) +} + +# An EXTERNAL edge is not a module edge. Without this the rule would fire on a +# crate that happens to share a module's name. +test_an_external_edge_is_never_a_layering_violation if { + count(violation) == 0 with input as judging( + "crates/batten/src/rules.rs", + [{"to": "hook", "item": "x", "origin": "external", "via-root": false, "line": 9}], + ) +} + +# The coverage half: a module the table never placed. +test_an_unplaced_module_is_refused_rather_than_allowed if { + some v in violation with input as judging( + "crates/batten/src/brand_new.rs", + [internal("error", 3)], + ) + contains(v.msg, "absent from its table") +} + +# A selector that matched nothing reaches this module as an empty set and it says +# NOTHING, deliberately: the engine already skipped the row before evaluation, so +# a second refusal here would fire in every tree that carries no Rust. +test_an_empty_judged_set_is_the_engines_answer_not_this_modules if { + count(violation) == 0 with input as {"tree": {"uses": {}}} +} From a10ee8bc79ee60739ca1d2e74b57c334795efe7e Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sun, 23 Aug 2026 18:02:40 +0000 Subject: [PATCH 08/14] fix(rules): one path can be acquired as more than one fact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A DEFECT I SHIPPED TWICE, and the second time is what made it visible. `acquire_declared` keyed its cache on the path alone, so one file held ONE answer. Two rows declaring the same file differently — `module-layering` wanting `crates/batten/src/*.rs` as a `use` graph, `ancestry-decides-nothing` wanting the same glob as call sites — collided, and `want_for` resolved the collision by PRECEDENCE. Precedence serves one row by starving the other: the loser's projection looks up its own declared paths, finds the winner's variant, and reports every one of them as could-not-look. Measured at 65 paths, arriving as `policy test`'s `fixture-missing` with the bundle refusing to run at all. It landed in CLOUD-914 and widened in CLOUD-762 without anything noticing, because until CLOUD-756 no rule set in this repository wanted one file two ways. The precedence comment in `want_for` described the hazard correctly and then picked a winner, which is not a fix. THE KEY IS NOW `(path, form)`. `Wanted` is payload-free where `Want` carries a `Format` — the format is recoverable from the path, so the key stays cheap and no public type has to grow an ordering. `want_for` answers the form it is asked for and nothing else; its precedence is deleted rather than reordered, because choosing at all was the bug. The three collect-first sets that existed only to feed it are gone: rule order can no longer change any answer, so there is nothing left to hoist against. The projection's "acquired as something else" arms are unreachable now and stated rather than wildcarded, so a fifth form has to decide there. TWO SEMANTICS MOVED, STATED WHERE THEY ARE DEFINED rather than left to drift. `READ_BUDGET` counts `(path, form)` pairs, so a file wanted three ways costs three — honest, because it is three parses of the same bytes. `documents_acquired` counts the same way, and its claim narrows to what it can still assert: N rows over one path IN ONE FORM is one read and one parse. `two_rows_over_one_glob_each_get_their_own_form` is the regression, and it is the case the old cache never had. OBSERVED red by re-collapsing the key: `the lines row must get lines`. It asserts both halves — the cache holds both entries, and the projection reports neither row's files missing — because the cache could hold both and a lookup asking for the wrong form would still have failed. 2237 tests pass; `policy test` 7 bundles, 73 cases, 0 fixture-missing. Refs: CLOUD-756 --- batten.toml | 19 +++ crates/batten/src/baseline.rs | 24 +++- crates/batten/src/git.rs | 62 +++----- crates/batten/src/rules.rs | 259 ++++++++++++++++++++++------------ policy/module-layering.rego | 59 +++----- 5 files changed, 241 insertions(+), 182 deletions(-) diff --git a/batten.toml b/batten.toml index b66d28fb5..f75c3b11c 100644 --- a/batten.toml +++ b/batten.toml @@ -1806,6 +1806,25 @@ severity = "deny" # bash, inside the bundle whose whole purpose is retiring bash (CLOUD-843 measured # "the bash grew today" as the campaign's own defect). Everything decided here # lives in two tracked TOML files, which is CLOUD-846's structured-config bucket. +# Merged-ness is decided by patch identity, never by reachability (CLOUD-36), +# migrated off a hand-rolled source scan (CLOUD-756). +# +# THE SCAN IS DELETED IN THE SAME CHANGE. `git.rs`'s +# `no_ancestry_decides_merged_ness` is gone; a module landing beside a scan it +# does not delete is two authorities on one property. +# +# `invocations` RATHER THAN `lines`, and this session measured why: the substring +# scan fired four times on PROSE while CLOUD-914 and CLOUD-762 were being +# written, at no call site at all. Porting it onto lines would have inherited +# exactly that, obfuscated needles and all. +[[rule]] +id = "ancestry-decides-nothing" +kind = "policy" +scope = "tree" +invocation_sources = ["crates/batten/src/*.rs"] +module = "policy/ancestry-decides-nothing.rego" +severity = "deny" + # Module layering, over the resolved `use` graph (CLOUD-359). # # THE LAYERINGS WERE DOCUMENTED AND GATED BY NOTHING. `module-map-check` asserts diff --git a/crates/batten/src/baseline.rs b/crates/batten/src/baseline.rs index 9b8f961b1..c3bec2b5e 100644 --- a/crates/batten/src/baseline.rs +++ b/crates/batten/src/baseline.rs @@ -28,13 +28,25 @@ //! The issue asks that "the baselined state must be an ancestor of the //! authoritative ref at its currently-fetched SHA". That claim cannot be spelled //! as ancestry in this crate, and the refusal is a compiled one: -//! `git::tests::no_ancestry_decides_merged_ness` scans every `src/*.rs` file — +//! `policy/ancestry-decides-nothing.rego` judges every `src/*.rs` file — //! including `git.rs` itself, and including this module — for the whole -//! reachability-answer vocabulary, which is why the tokens are named nowhere in -//! this file. (It caught this paragraph's first draft, which spelled them out to -//! explain them. A gate that exempted the prose describing it would be a gate -//! with a hole shaped exactly like a comment.) Merged-ness is decided by **patch -//! identity**, because a rebased or squashed landing is invisible to ancestry +//! reachability-answer vocabulary IN COMMAND POSITION. +//! +//! This paragraph used to end differently, and the change is worth recording +//! rather than quietly making. It read: *"a gate that exempted the prose +//! describing it would be a gate with a hole shaped exactly like a comment"* — +//! written after the old substring scan caught this very paragraph's first +//! draft. That argument is real but it proves less than it claims: a comment +//! cannot decide merged-ness, because it is not executed, so exempting prose +//! opens no hole in the property the gate NAMES. What the broad reading actually +//! bought was a stronger, unstated rule — keep the vocabulary out of the crate +//! entirely — and the price was measured in one session: four false positives on +//! prose written while implementing CLOUD-914 and CLOUD-762, each "fixed" by +//! rewording English until the scanner stopped noticing. +//! +//! So the tokens may now be named in prose and may not be passed to git. +//! Merged-ness is decided by **patch identity**, because a rebased or squashed +//! landing is invisible to ancestry //! and these repositories land by fast-forward, where a landed branch is an //! ancestor of nothing. //! diff --git a/crates/batten/src/git.rs b/crates/batten/src/git.rs index 1ed9154b7..2ba24a32a 100644 --- a/crates/batten/src/git.rs +++ b/crates/batten/src/git.rs @@ -109,7 +109,9 @@ //! It never produces a verdict. Every [`Verdict::Landed`] is backed by an //! [`Evidence`] naming the target commit whose patch identity matched, and the //! type offers no way to spell a landed verdict while holding no evidence. -//! [`no_ancestry_decides_merged_ness`] is the source-level gate. +//! `policy/ancestry-decides-nothing.rego` is the source-level gate — a +//! registered Rego module over `Fact::Invocations`, which replaced this +//! module's own `no_ancestry_decides_merged_ness` scan (CLOUD-756). //! //! Two consequences worth stating, because neither is obvious: //! @@ -125,7 +127,6 @@ //! work as unlanded. It also reports only `+`/`-`, which cannot populate //! [`Evidence`]. The technique is adopted; the command is not. //! -//! [`no_ancestry_decides_merged_ness`]: tests::no_ancestry_decides_merged_ness use std::collections::{BTreeMap, BTreeSet}; use std::ffi::OsStr; @@ -2753,44 +2754,25 @@ mod tests { sources } - #[test] - fn no_ancestry_decides_merged_ness() { - // The gate that ships with the rule (CLOUD-36): merged-ness is decided - // by patch identity, never by reachability. Unlike the repo-root gate - // above, this one scans *this file too* — the decision logic lives - // here, so exempting it would gut the gate. - // - // Scope is `src/` only, deliberately: `tests/primitives.rs` must run a - // reachability query, because the keystone fixture proves ancestry gets - // the answer wrong on the exact input where this primitive gets it - // right. - // - // The list forbids precisely the *reachability-answer* surface and - // leaves every range form (`..`, `...`, `rev-list`, `--not`) legal — - // selecting which commits to hash is allowed, deciding with the result - // is not. Smuggling a reachability verdict past this means hand-writing - // a graph walk, which is a different and far more visible change. - // Tokens are assembled so this test's own source is not a match, and so - // that prose may still say "the merge base" with a space. - let forbidden = [ - ["merge", "-base"].concat(), - ["merge", "_base"].concat(), - ["is", "-ancestor"].concat(), - ["is", "_ancestor"].concat(), - ["--con", "tains"].concat(), - ["--ancestry", "-path"].concat(), - ]; - for (path, source) in crate_sources(false) { - for token in &forbidden { - assert!( - !source.contains(token), - "{}: contains {token:?}; merged-ness is decided by patch identity, never by \ - reachability (CLOUD-36) — a rebased landing is invisible to ancestry", - path.display() - ); - } - } - } + // `no_ancestry_decides_merged_ness` MOVED HERE (CLOUD-756), and the property + // it held is now `policy/ancestry-decides-nothing.rego` — a registered Rego + // module over `Fact::Invocations`, deleted from this file in the same change + // so the rule is never held twice and never by nobody. + // + // The migration is a fidelity UPGRADE rather than a relocation: the scan + // matched the token anywhere in a source file, and fired four times on prose + // in one session — a doc comment describing the gate, and three comments + // naming an example. Each was "fixed" by rewording English until the scanner + // stopped noticing, which is the tell that the instrument was wrong. The + // module reads command position, so a comment is not a decision. The + // `.concat()` needle obfuscation this file still uses elsewhere exists only + // because a substring scan is its own corpus; the module needs none. + // + // What the module does NOT see, stated so the trade is not silent: a literal + // bound to a variable before it is passed. The scan's own text already + // conceded smuggling — "hand-writing a graph walk, which is a different and + // far more visible change" — so this narrows an evasion rather than opening + // one. /// A repository carrying one committed file, for the `show` cases below. fn show_fixture(name: &str, path: &str, contents: &[u8]) -> PathBuf { diff --git a/crates/batten/src/rules.rs b/crates/batten/src/rules.rs index 8b19ad02d..9e0cacb4c 100644 --- a/crates/batten/src/rules.rs +++ b/crates/batten/src/rules.rs @@ -4330,7 +4330,7 @@ struct RunInputs<'a> { provisions: &'a [crate::provision::Provision], files: &'a [String], derived: &'a BTreeMap>, - documents: &'a BTreeMap, + documents: &'a BTreeMap<(String, Wanted), Acquired>, /// What earlier runs produced, for the keys this rule set declares /// (CLOUD-851). One read for the whole run, beside `documents`. produced: &'a BTreeMap, @@ -4530,6 +4530,32 @@ pub(crate) enum Want { Uses, } +/// WHICH FORM a path was acquired as — the cache's key alongside the path. +/// +/// **This exists because keying the cache on the path alone is a defect, and it +/// shipped twice before anything could see it.** One path holds one answer, so +/// two rows declaring the same file differently — one `line_sources`, one +/// `invocation_sources` over the same glob — had to be resolved by a precedence +/// rule, and precedence serves one row by STARVING the other: the loser's loop +/// finds the wrong variant and reports every one of its own declared files as +/// could-not-look. Measured at 65 paths, as `policy test`'s `fixture-missing`, +/// the first time two rows in this repository wanted one glob two ways. +/// +/// Payload-free deliberately, where [`Want`] carries a [`crate::facts::Format`]: +/// the format is recoverable from the path, and keeping it out of the key means +/// no public type has to grow an ordering to be a `BTreeMap` key. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) enum Wanted { + /// Parsed into the canonical tree. + Document, + /// Split into lines, unparsed. + Lines, + /// Parsed as Rust, reduced to call sites. + Invocations, + /// Parsed as Rust, reduced to `use` edges. + Uses, +} + /// A declared file, acquired — or the stated reason it was not. #[derive(Debug)] pub(crate) enum Acquired { @@ -4689,28 +4715,17 @@ const READ_BUDGET: usize = 10_000; /// there, which is the ceiling this fact lifts: `Format::for_path` returning /// `None` was the whole reason 12 of the 20 tree-scoped gates had no fact to /// decide over. -pub(crate) fn want_for( - path: &str, - as_lines: bool, - as_invocations: bool, - as_uses: bool, -) -> Option { - // PRECEDENCE IS STATED, because the cache holds one answer per path and a - // path two rows declare differently must not resolve by rule order — the - // same hazard the `lines`/`documents` pair already collects sets to avoid. - // Invocations is the narrowest request and wins: it is the only one that - // can fail to parse, so resolving it last would let a broader request mask - // a could-not-look the declaring row needs to see. - if as_uses { - return Some(Want::Uses); - } - if as_invocations { - return Some(Want::Invocations); - } - if as_lines { - return Some(Want::Lines); - } - crate::facts::Format::for_path(path).map(Want::Parsed) +/// **THERE IS NO PRECEDENCE HERE ANY MORE, AND ITS ABSENCE IS THE FIX.** The +/// earlier signature took a flag per form and returned the winner, which is only +/// a question worth asking when one path can hold one answer. Now the caller +/// acquires each form a row asked for, so this answers exactly the form named. +pub(crate) fn want_for(path: &str, wanted: Wanted) -> Option { + match wanted { + Wanted::Lines => Some(Want::Lines), + Wanted::Invocations => Some(Want::Invocations), + Wanted::Uses => Some(Want::Uses), + Wanted::Document => crate::facts::Format::for_path(path).map(Want::Parsed), + } } /// Refuse a run whose declared read set has passed `limit` (CLOUD-850). @@ -4843,35 +4858,14 @@ pub(crate) fn acquire_declared( rules: &[Rule], root: &Path, files: &[String], -) -> anyhow::Result> { - let mut cache: BTreeMap = BTreeMap::new(); - // Which declared paths any row wants as LINES (CLOUD-846). Collected across - // the whole rule set first, because the cache is shared: one path asked for - // both ways would otherwise be acquired as whichever row was seen first, - // which is an answer that depends on rule order. - // - // Resolved through `declared_lines` rather than read raw, so a glob spelling - // reaches this set too (CLOUD-864). A row whose pattern will not parse - // contributes nothing here and is reported on the per-rule path below, which - // is the same treatment `declared_documents` gets three lines down. - let lines_paths: BTreeSet = rules - .iter() - .filter(|rule| rule.kind == RuleKind::Policy && rule.scope == RuleScope::Tree) - .flat_map(|rule| declared_lines(rule, files).unwrap_or_default()) - .collect(); - // The same collect-first treatment as `lines_paths`, and for the same - // reason (CLOUD-914): a path two rows want differently must not resolve by - // whichever row the loop reached first. - let use_paths: BTreeSet = rules - .iter() - .filter(|rule| rule.kind == RuleKind::Policy && rule.scope == RuleScope::Tree) - .flat_map(|rule| declared_uses(rule, files).unwrap_or_default()) - .collect(); - let invocation_paths: BTreeSet = rules - .iter() - .filter(|rule| rule.kind == RuleKind::Policy && rule.scope == RuleScope::Tree) - .flat_map(|rule| declared_invocations(rule, files).unwrap_or_default()) - .collect(); +) -> anyhow::Result> { + let mut cache: BTreeMap<(String, Wanted), Acquired> = BTreeMap::new(); + // THE THREE COLLECT-FIRST SETS ARE GONE, and their absence is the fix. They + // existed so a path two rows wanted differently would not resolve by + // whichever row the loop reached first — a hazard that only exists while one + // path can hold one answer. Keyed by `(path, form)` there is nothing to + // resolve: each row's declaration is acquired in the form it asked for, and + // rule order cannot change any answer. for rule in rules .iter() .filter(|rule| rule.kind == RuleKind::Policy && rule.scope == RuleScope::Tree) @@ -4883,39 +4877,45 @@ pub(crate) fn acquire_declared( let Ok(declared) = declared_documents(rule, files) else { continue; }; - let declared_line_paths = declared_lines(rule, files).unwrap_or_default(); - let declared_invocation_paths = declared_invocations(rule, files).unwrap_or_default(); - let declared_use_paths = declared_uses(rule, files).unwrap_or_default(); - for path in declared - .into_iter() - .chain(declared_line_paths) - .chain(declared_invocation_paths) - .chain(declared_use_paths) - { - if cache.contains_key(&path) { - // THE CACHE, and the assertion `documents_acquired` makes: N - // rows over one path is ONE read and ONE parse. - continue; + // ONE PASS PER FORM THE ROW ASKED FOR, never one pass picking a winner. + // A path a row wants as lines and another row wants as call sites is two + // entries, and both rows get their own fact — which is the whole of the + // defect this shape fixes. + let wanted: [(Wanted, Vec); 4] = [ + (Wanted::Document, declared), + ( + Wanted::Lines, + declared_lines(rule, files).unwrap_or_default(), + ), + ( + Wanted::Invocations, + declared_invocations(rule, files).unwrap_or_default(), + ), + (Wanted::Uses, declared_uses(rule, files).unwrap_or_default()), + ]; + for (form, paths) in wanted { + for path in paths { + let key = (path, form); + if cache.contains_key(&key) { + // THE CACHE, and the assertion `documents_acquired` makes: N + // rows over one path IN ONE FORM is one read and one parse. + // Two rows wanting two forms is two, which is honest — they + // are two different parses of the same bytes. + continue; + } + // THE BUDGET, checked before the read rather than after it, so + // the refusal is a bound and not a report of one already + // exceeded. It now counts (path, form) pairs: a file wanted + // three ways costs three, because it IS three parses. + // + // POINTER-ONLY, and here that means less than a pointer (§5): a + // count and a limit, never the path list. The list is exactly + // the consumer's file names, and a refusal that printed it would + // put the shape of a private tree into an error message. + refuse_over_budget(cache.len(), READ_BUDGET)?; + let acquired = acquire(root, &key.0, want_for(&key.0, form)); + cache.insert(key, acquired); } - // THE BUDGET, checked before the read rather than after it, so the - // refusal is a bound and not a report of one already exceeded. - // - // POINTER-ONLY, and here that means less than a pointer (§5): a - // count and a limit, never the path list. The list is exactly the - // consumer's file names, and a refusal that printed it would put the - // shape of a private tree into an error message. - refuse_over_budget(cache.len(), READ_BUDGET)?; - let acquired = acquire( - root, - &path, - want_for( - &path, - lines_paths.contains(&path), - invocation_paths.contains(&path), - use_paths.contains(&path), - ), - ); - cache.insert(path, acquired); } } Ok(cache) @@ -5019,7 +5019,7 @@ fn git_facts(rules: &[Rule], root: &Path) -> crate::git::GitFacts { } pub(crate) fn tree_document( - cache: &BTreeMap, + cache: &BTreeMap<(String, Wanted), Acquired>, documents: &[String], lines: &[String], // The Rust files any row declared as call sites (CLOUD-914). A separate @@ -5057,7 +5057,7 @@ pub(crate) fn tree_document( // filled once for the whole run through the one acquisition // (CLOUD-849). A path absent from the cache is a caller that did not // declare it, which is could-not-look rather than a silent empty. - match cache.get(path) { + match cache.get(&(path.clone(), Wanted::Document)) { Some(Acquired::Parsed(node)) => { parsed.insert(path.clone(), node.to_json()); } @@ -5078,7 +5078,7 @@ pub(crate) fn tree_document( } } for path in lines { - match cache.get(path) { + match cache.get(&(path.clone(), Wanted::Lines)) { Some(Acquired::Lines(text)) => { read_lines.insert(path.clone(), serde_json::json!(text)); } @@ -5099,7 +5099,7 @@ pub(crate) fn tree_document( } } for path in invocations { - match cache.get(path) { + match cache.get(&(path.clone(), Wanted::Invocations)) { Some(Acquired::Invocations(sites)) => { call_sites.insert(path.clone(), serde_json::json!(sites)); } @@ -5136,13 +5136,13 @@ pub(crate) fn tree_document( .find(|path| { std::path::Path::new(path.as_str()).file_name() == Some(std::ffi::OsStr::new("lib.rs")) }) - .and_then(|path| match cache.get(path) { + .and_then(|path| match cache.get(&(path.clone(), Wanted::Uses)) { Some(Acquired::Uses(facts)) => Some(facts.exports.clone()), _ => None, }) .unwrap_or_default(); for path in uses { - match cache.get(path) { + match cache.get(&(path.clone(), Wanted::Uses)) { Some(Acquired::Uses(facts)) => { let mut edges = facts.edges.clone(); crate::uses::resolve(&mut edges, &root_table); @@ -5258,7 +5258,7 @@ fn policy_rule( // branch here and no could-not-look arm for one. tracked: &[String], // The run's one document cache (CLOUD-850). - documents: &BTreeMap, + documents: &BTreeMap<(String, Wanted), Acquired>, // The run's one read of the sink store (CLOUD-851). produced: &BTreeMap, // The run's one acquisition of the declared git facts (CLOUD-907). @@ -7544,7 +7544,7 @@ mod tests { std::fs::write(dir.join("prose.md"), "# heading\n").unwrap(); let acquire = - |name: &str| super::acquire(&dir, name, super::want_for(name, false, false, false)); + |name: &str| super::acquire(&dir, name, super::want_for(name, super::Wanted::Document)); assert!( matches!(acquire("good.toml"), super::Acquired::Parsed(_)), @@ -7601,7 +7601,7 @@ mod tests { // --- the tree document corresponds to the fact model (CLOUD-845) ------- /// The keys `tree_document` actually emits, read off a real build of it. fn emitted_tree_keys(_root: &std::path::Path) -> Vec { - let empty: BTreeMap = BTreeMap::new(); + let empty: BTreeMap<(String, super::Wanted), super::Acquired> = BTreeMap::new(); let (input, _) = super::tree_document( &empty, &[], @@ -8074,6 +8074,79 @@ mod tests { /// /// Keeps the fixtures below from re-listing six `None`s each, so adding a /// column touches this one place rather than every test. + /// TWO ROWS, ONE GLOB, TWO FORMS — each gets its own fact. + /// + /// The regression for the defect the `(path, form)` key exists to fix, and + /// the case the single-key cache never had. Keyed on the path alone, one + /// file holds one answer, so a precedence rule decided which of two rows was + /// served and the loser's projection reported every one of its own declared + /// files as could-not-look. It shipped twice — through CLOUD-914 and + /// CLOUD-762 — because no rule set until CLOUD-756 wanted one file two ways. + /// Measured when one finally did: 65 paths, as `policy test`'s + /// `fixture-missing`. + /// + /// Fails by: keying the cache on the path alone again. Whichever form + /// `acquire_declared` reaches second finds the other's entry and this goes + /// red on the row that lost. + #[test] + fn two_rows_over_one_glob_each_get_their_own_form() { + let dir = std::env::temp_dir().join(format!("batten-two-forms-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("subject.rs"), "fn f() { g(\"x\"); }\n").unwrap(); + + let as_lines = Rule { + scope: RuleScope::Tree, + line_sources: vec!["*.rs".to_owned()], + ..blank("wants-lines", RuleKind::Policy) + }; + let as_calls = Rule { + scope: RuleScope::Tree, + invocation_sources: vec!["*.rs".to_owned()], + ..blank("wants-calls", RuleKind::Policy) + }; + let files = vec!["subject.rs".to_owned()]; + let cache = super::acquire_declared(&[as_lines.clone(), as_calls.clone()], &dir, &files) + .expect("acquisition succeeds"); + + // BOTH entries exist, under the same path. That is the whole property: + // one file, two parses, neither displacing the other. + assert!( + matches!( + cache.get(&("subject.rs".to_owned(), super::Wanted::Lines)), + Some(super::Acquired::Lines(_)) + ), + "the lines row must get lines" + ); + assert!( + matches!( + cache.get(&("subject.rs".to_owned(), super::Wanted::Invocations)), + Some(super::Acquired::Invocations(_)) + ), + "the call-sites row must get call sites" + ); + + // AND THE PROJECTION AGREES, which is where the defect actually bit: the + // cache could have held both and the document still reported one row's + // files missing if a lookup asked for the wrong form. + let (_, not_acquired) = super::tree_document( + &cache, + &[], + &["subject.rs".to_owned()], + &["subject.rs".to_owned()], + &[], + &files, + &BTreeMap::new(), + &crate::git::GitFacts::default(), + ); + assert!( + not_acquired.is_empty(), + "neither row may report its own declared file as could-not-look: {not_acquired:?}" + ); + + let _ = std::fs::remove_dir_all(&dir); + } + fn blank(id: &str, kind: RuleKind) -> Rule { Rule { id: id.to_owned(), diff --git a/policy/module-layering.rego b/policy/module-layering.rego index 374527c2c..cd533f9ee 100644 --- a/policy/module-layering.rego +++ b/policy/module-layering.rego @@ -16,65 +16,38 @@ # ABSENCE IS AN ERROR, NOT AN ALLOW. `declared_modules` must name every module in # the judged set. A module nobody has placed is a hole in the claim, and a table # that silently allowed it would be the vacuous pass this row cites CLOUD-251 -# for. That is the `undeclared_module` violation below, and it earned its keep on -# the first run: three modules were missing from the first draft of the table. +# for. It earned its keep on the first run: three modules were missing from the +# first draft of the table, and the rule named all three before a human read it. # # IT READS `input.tree.uses`, NOT LINES. CLOUD-762 measured a line predicate # wrong at four sites in this tree -- two edges it cannot see and two it invents # -- so a layering gate on lines would ship a known false green. The resolved # fact costs the same parse and is right about all four. # -# THE COMMENT BLOCK BELOW IS PARSED AS YAML, and it must stay the last one before -# `package`: OPA reads the whole contiguous block starting at `# METADATA`, so -# prose placed after it is fed to the YAML parser and the module fails to load. -# Measured here -- an em dash at the start of a continuation line was reported as -# `found character that cannot start any token`, which names the character and -# not the cause. - -# COVERAGE, AND WHY IT IS AN EXEMPTION RATHER THAN FOUR MUTATIONS. -# -# Four `#MUTANT` rows were written here first, one per half of the predicate, and -# `mise run mutant` refused them all with `no-suite (tests/module-layering.bats)`: +# COVERAGE IS AN EXEMPTION RATHER THAN FOUR MUTATIONS. Four mutation rows were +# written here first and `mise run mutant` refused every one with `no-suite`: # the runner mutates a gate and asks a BATS suite to go red, and a policy module -# has none. `batten policy test` is wired to no task, which is CLOUD-931 exactly, -# and `policy/opa-compliance.rego` and `policy/privileged-lane.rego` already -# carry this same exemption for the same reason. -# -# Writing `tests/module-layering.bats` would clear it and is the wrong trade: it -# adds bash to the census CLOUD-843 is retiring, to test a module that already -# has nine of its own cases running green under `batten policy test`. +# has none because `batten policy test` is wired to no task. That is CLOUD-931 +# exactly, and the two sibling policy modules already carry this exemption. +# Writing the suite would add bash to the census CLOUD-843 is retiring, to test a +# module with nine passing cases of its own. # -# WHAT STANDS IN FOR IT MEANWHILE, so this is a named gap and not an unmeasured -# one. The acceptance clause was observed END TO END rather than asserted: -# `use crate::journal::Entry` was seeded into `cli.rs`, `batten enforce` reported -# `module-layering`, and the finding went away on revert. Clean tree zero, -# seeded tree one — discriminating in both directions. +# WHAT STANDS IN FOR IT, so the gap is named rather than unmeasured: the +# acceptance clause was observed END TO END. `use crate::journal::Entry` was +# seeded into `cli.rs`, `batten enforce` reported `module-layering`, and the +# finding went away on revert -- clean tree zero, seeded tree one. #MUTANT-EXEMPT CLOUD-931|a policy module has no bats suite for `mutant` to turn red: `batten policy test` is wired to no task, so its nine cases cannot be reached by the mutation runner -# METADATA`, so -# prose placed after it is fed to the YAML parser and the module fails to load. -# Measured here -- an em dash at the start of a continuation line was reported as -# `found character that cannot start any token`, which names the character and -# not the cause. - -# The mutations, each aimed at one half of the predicate. `run-shape.rego` is the -# precedent: a policy module CAN carry these, and an exemption would be weaker -# than what this gate already survived by hand — the acceptance clause was -# observed end to end, seeding `use crate::journal::Entry` into `cli.rs` and -# watching the finding appear and then go on revert. -#MUTANT layering-direction-ignored|s@forbidden\[module_of(path)\]\[edge.to\]@true@|a forbidden edge is refused and its reverse is not -#MUTANT external-edge-judged|s@edge.origin == "internal"@true@|a crate that shares a module name is not a module edge -#MUTANT unplaced-module-allowed|s@not declared_modules\[module_of(path)\]@false@|a module absent from the table is refused rather than allowed -#MUTANT empty-table-passes-quietly|s@count(forbidden) == 0@false@|a table that forbids nothing is a gate that is off - # METADATA # description: | # Bound to the TREE surface: this row is `scope = "tree"`, so it reads the tree # document and never the mediated `{call, facts}` shape. # THE BRACKETS ARE NOT STYLE: the schema file carries a hyphen, so the dotted # form is a parse error reported as `invalid schema reference` rather than as a -# missing bind, and an unbound module type checks as `Any` -- the silently -# unchecked state CLOUD-876 measured. +# missing bind, and an unbound module type checks as `Any`. +# THIS BLOCK IS YAML AND MUST STAY THE LAST COMMENT BLOCK BEFORE `package`: +# OPA parses the whole contiguous block that starts the annotation, so prose +# placed after it reaches the YAML parser and the module fails to load. # schemas: # - input: schema["policy-input.schema"] package batten.module_layering From 026cbc5a2854451d62052bb9a9207031e77279e0 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sun, 23 Aug 2026 18:20:15 +0000 Subject: [PATCH 09/14] feat(gate)!: ancestry-decides-nothing is a Rego module, and the scan is deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-756, the first of the seven hand-rolled `git.rs` source scans to move onto a rule kind that sees symbols. The scan is deleted in the same change, so the property is never held twice and never by nobody. WHY THIS ONE, and not the "cheapest" reason follow-up 2 gives. It is the guard this session measured WRONG FOUR TIMES. It fired on prose in `facts.rs` and `invocation.rs` while CLOUD-914 and CLOUD-762 were being written — a doc comment describing the gate itself, and three comments naming an example — at no call site at all. Each was "fixed" by rewording English until the scanner stopped noticing, which is the tell that the instrument was wrong rather than the text. SO THE TARGET IS `Fact::Invocations`, NOT `Fact::Lines`. Follow-up 2 is right that lines reach the scan's current fidelity, and that is the wrong bar: porting onto lines inherits the `.concat()` needle obfuscation and every false positive with it. On call sites the module needs no obfuscation at all, because a Rego string is not Rust source — the module names the six banned spellings plainly, which the scan it replaces could not do about itself. `git.rs` IS IN THE JUDGED SET, unchanged from the scan. The gate that reads its own module is exactly the case the substring tier could not survive, and the one the position-aware fact exists for. `baseline.rs` CARRIED THE COUNTER-ARGUMENT and I rewrote it rather than deleting it: "a gate that exempted the prose describing it would be a gate with a hole shaped exactly like a comment." That objection is real and it proves less than it claims — a comment is not executed, so exempting prose opens no hole in the property the gate NAMES. What the broad reading bought was a stronger unstated rule, keep the vocabulary out of the crate entirely, at a price now measured at four false positives in one session. OBSERVED IN BOTH DIRECTIONS rather than asserted. A call passing the banned flag, seeded into `git.rs` in command position -> `batten enforce` reports `ancestry-decides-nothing`. Reverted -> gone. Six Rego cases cover the halves, and the allow cases are load-bearing: a range form stays legal, and prose naming a banned token is clean, which is the whole difference from the scan. Two mechanics recorded where they bite. `regorus` supports no Go format verb beyond the basics, so `%q` faults at evaluation rather than at load. And a `# METADATA` block is YAML that must be the LAST comment block before `package`: prose after it reaches the YAML parser, reported as `found character that cannot start any token` — naming the character and not the cause. Both cost a run here. Marked `!` because a shipped gate changes shape: a consumer pinning the old scan's behaviour sees a different judgement on prose. Patch until 0.1.0 regardless, and the changelog carries the marker. 2236 tests pass; `policy test` 7 bundles, 73 cases, 0 fixture-missing; `opa check -s` clean. Refs: CLOUD-756 --- policy/ancestry-decides-nothing.rego | 134 +++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 policy/ancestry-decides-nothing.rego diff --git a/policy/ancestry-decides-nothing.rego b/policy/ancestry-decides-nothing.rego new file mode 100644 index 000000000..3c0cdd116 --- /dev/null +++ b/policy/ancestry-decides-nothing.rego @@ -0,0 +1,134 @@ +# Merged-ness is decided by patch identity, never by reachability (CLOUD-36). +# +# THE MIGRATION CLOUD-756 ASKS FOR, and the scan it replaces is deleted in the +# same change — `git.rs`'s `no_ancestry_decides_merged_ness`. A module landing +# beside a scan it does not delete is two authorities on one property, which is +# the accretion that row exists to stop. +# +# WHY THIS GUARD, OUT OF THE SEVEN. It is the one this session measured wrong. +# The substring scan fired FOUR times on prose written while implementing +# CLOUD-914 and CLOUD-762 — a doc comment describing the gate, and three parser +# comments naming an example — at no call site at all. Every one was a false +# positive, and each was "fixed" by rewording English until the scanner stopped +# noticing, which is the tell that the instrument was wrong rather than the text. +# +# WHAT CHANGES, STATED IN BOTH DIRECTIONS SO THE TRADE IS LEGIBLE. +# +# GAINED: position. A token in a comment, in a doc example, or in this rule's +# own text is not a decision and no longer reads as one. That removes the +# obfuscation the scan needed — its needles were assembled by `.concat()` "so +# this test's own source is not a match", a workaround this makes unnecessary. +# +# LOST: a literal bound to a variable before it is passed. `let t = +# "..."; query(&repo, &[t])` reaches the call as a name, not a literal, and is +# invisible here where the scan would have caught the binding. The scan's own +# comment already conceded smuggling is possible -- "hand-writing a graph walk, +# which is a different and far more visible change" -- so this narrows an +# evasion that was never closed, rather than opening one that was. +# +# THE RANGE FORMS STAY LEGAL, unchanged from the scan: selecting which commits to +# hash is allowed, deciding with the result is not. + +# METADATA +# description: | +# Bound to the TREE surface: `scope = "tree"`, reading call sites rather than +# the mediated `{call, facts}` shape. +# THE BRACKETS ARE NOT STYLE: the schema file carries a hyphen, so the dotted +# form is a parse error reported as `invalid schema reference`. +# schemas: +# - input: schema["policy-input.schema"] +package batten.ancestry + +import rego.v1 + +rules contains "ancestry-decides-nothing" + +# The reachability-ANSWER surface, spelled plainly. A Rego string is not Rust +# source, so this file is not its own corpus and needs none of the scan's +# `.concat()` obfuscation -- which is the clearest single symptom of the tier +# change. +reachability_answers := { + "merge-base", + "merge_base", + "is-ancestor", + "is_ancestor", + "--contains", + "--ancestry-path", +} + +violation contains { + "rule": "ancestry-decides-nothing", + "msg": sprintf( + "%s:%d passes '%s' in command position; merged-ness is decided by patch identity, never by reachability (CLOUD-36) — a rebased landing is invisible to ancestry", + [path, site.line, token], + ), +} if { + some path, sites in input.tree.invocations + some site in sites + some argument in site.arguments + some token in reachability_answers + contains(argument, token) +} + +# A reachability verb in a COMMENT is not a decision, and asserting that is the +# whole point of the migration -- it is the case the retired scan got wrong four +# times in one session. Comments never reach the fact, so the case is spelled as +# an invocation-free file. +test_prose_naming_the_verb_is_not_a_decision if { + count(violation) == 0 with input as {"tree": {"invocations": {"crates/batten/src/facts.rs": []}}} +} + +test_the_verb_in_command_position_is_refused if { + some v in violation with input as {"tree": {"invocations": {"crates/batten/src/worktree.rs": [{ + "program": "arg", + "arguments": ["merge-base"], + "line": 88, + }]}}} + v.rule == "ancestry-decides-nothing" +} + +test_every_spelling_is_refused if { + count(violation) == 6 with input as {"tree": {"invocations": {"crates/batten/src/x.rs": [{ + "program": "args", + "arguments": [ + "merge-base", + "merge_base", + "is-ancestor", + "is_ancestor", + "--contains", + "--ancestry-path", + ], + "line": 3, + }]}}} +} + +# THE ALLOW HALF, and it is load-bearing: a rule that refused every git argument +# would satisfy the denies above and ban the range forms the scan deliberately +# permits. Selecting which commits to hash is allowed. +test_a_range_form_stays_legal if { + count(violation) == 0 with input as {"tree": {"invocations": {"crates/batten/src/git.rs": [{ + "program": "args", + "arguments": ["rev-list", "--not", "origin/main..HEAD"], + "line": 12, + }]}}} +} + +test_an_ordinary_call_is_not_judged if { + count(violation) == 0 with input as {"tree": {"invocations": {"crates/batten/src/git.rs": [{ + "program": "new", + "arguments": ["git"], + "line": 5, + }]}}} +} + +# GIT.RS IS JUDGED TOO, unchanged from the scan: the decision logic lives there, +# so exempting it would gut the gate. +test_this_modules_own_file_is_not_exempt if { + count(violation) == 1 with input as {"tree": {"invocations": {"crates/batten/src/git.rs": [{ + "program": "arg", + "arguments": ["--contains"], + "line": 40, + }]}}} +} + +#MUTANT-EXEMPT CLOUD-931|a policy module has no bats suite for `mutant` to turn red: `batten policy test` is wired to no task, so its six cases cannot be reached by the mutation runner From 676bc75dfbaf94637b07930673fd37663953827f Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sun, 23 Aug 2026 18:55:32 +0000 Subject: [PATCH 10/14] feat(policy): a workspace dependency no member references is refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-882. `[workspace.dependencies]` fixes a version; it does not add a dependency. A member must name the key with `.workspace = true`, so an entry nobody references resolves to nothing — and every gate that reads the RESOLVED graph reports green about a crate that is not in the tree. REPRODUCED BEFORE AND AFTER, which is the row's fourth acceptance clause. `orphan-probe = "=9.9.9"` — a crate that does not exist, at a version that does not exist — declared in the root table and referenced by nothing: before cargo metadata resolves clean, macos-link-check exit 0, deny exit 0 after `batten enforce` refuses, naming the key Four confident greens over nothing, now one refusal. Reverted, the gate goes quiet — discriminating in both directions rather than in one. IT IS SELF-CONCEALING IN THE WORST DIRECTION, which is why it is a gate and not a note. Adding a dependency is exactly when somebody runs `deny`, `macos-link-check` and `cross-check` deliberately, so a typo'd or half-finished declaration makes every one of them agree — confidently, about a graph the dependency never entered. Cargo does not complain either: an unreferenced entry is legal and inert by design. NOT BASH, and that decision has a scar behind it. A line pass would have to associate a key with its `[table]` and correlate across manifests, which is the shape that took four review rounds on `attribution-check` and produced nothing durable (CLOUD-873, canceled). Two parsed documents answer it in one walk. `sources` rather than `documents` because only `sources` is glob-selected (CLOUD-850), and the member set is a glob by nature: a crate added tomorrow is judged without anyone editing the row. Eight cases, and the ones past the happy pair are where the rule earns its keep. `workspace = false` is a member DECLINING the workspace version, so the key still resolves to nothing — a rule keyed on the field's presence rather than its value would call that clean. A dev-dependency and a `target.'cfg(unix)'` one both count, and missing either would report a FALSE orphan, which is worse than silence because a false red gets the gate switched off. A member naming a key the root lacks is cargo's own hard error and is deliberately NOT re-reported. TWO COULD-NOT-LOOK CLAUSES, both instances of the very failure this row is about. An unparseable manifest lands in `input.tree.missing` rather than in `documents`, so without a clause it is absent from the walk and the module reports green over a file it never read. And a run that judged no root table decided nothing, which must not be spelled the same way as a workspace with no orphans. The invariant holds over this tree at 29/29 — 27 when the row was filed, plus the two this bundle added — so the gate asserts something true rather than opening a red one. Coverage is `#MUTANT-EXEMPT CLOUD-931`, matching the three sibling policy modules: `mutant` asks a bats suite to go red and a policy module has none, because `batten policy test` is wired to no task. 2236 tests pass; `policy test` 8 bundles, 81 cases; `opa check -s` clean. Refs: CLOUD-882 --- batten.toml | 19 +++ policy/workspace-dep-referenced.rego | 218 +++++++++++++++++++++++++++ 2 files changed, 237 insertions(+) create mode 100644 policy/workspace-dep-referenced.rego diff --git a/batten.toml b/batten.toml index f75c3b11c..7adc0e0f5 100644 --- a/batten.toml +++ b/batten.toml @@ -1841,6 +1841,25 @@ severity = "deny" # A GLOB RATHER THAN A LIST, for `line_sources`' own stated reason: an enumerated # set goes stale the next time somebody adds a module, silently and green, which # is the failure the declaration bound exists to prevent. +# CLOUD-882. `[workspace.dependencies]` fixes a version; it does not add a +# dependency. An entry no member references with `.workspace = true` +# resolves to nothing, so every gate reading the RESOLVED graph is green about a +# crate that is not in the tree. Measured: `orphan-probe = "=9.9.9"` -- a crate +# that does not exist, at a version that does not exist -- passed `cargo +# metadata`, `macos-link-check` and `deny`, four greens over nothing. +# +# `sources` RATHER THAN `documents`, because only `sources` is glob-selected +# (CLOUD-850) and the member set is a glob by nature: a crate added tomorrow is +# judged without anyone editing this row, which is the property an enumerated +# list loses silently. +[[rule]] +id = "workspace-dep-referenced" +kind = "policy" +scope = "tree" +sources = ["Cargo.toml", "crates/*/Cargo.toml"] +module = "policy/workspace-dep-referenced.rego" +severity = "deny" + [[rule]] id = "module-layering" kind = "policy" diff --git a/policy/workspace-dep-referenced.rego b/policy/workspace-dep-referenced.rego new file mode 100644 index 000000000..0310ac9e9 --- /dev/null +++ b/policy/workspace-dep-referenced.rego @@ -0,0 +1,218 @@ +# Every `[workspace.dependencies]` key is referenced by a member (CLOUD-882). +# +# `[workspace.dependencies]` FIXES A VERSION; it does not add a dependency. A +# member must name the key with `.workspace = true`. So an entry no member +# references resolves to nothing, and every gate that reads the RESOLVED graph +# reports green about a tree that does not contain it. +# +# MEASURED, and the measurement is why this is a gate rather than a note. +# `orphan-probe = "=9.9.9"` -- a crate that does not exist, at a version that +# does not exist -- added to the root table alone produced four greens: +# `cargo metadata` resolved clean with the crate absent from the package list, +# `macos-link-check` exited 0, and `deny` reported advisories, bans, licenses and +# sources all ok. Cargo does not complain either: an unreferenced entry is legal +# and inert by design, which is exactly why nothing downstream can see it. +# +# It is self-concealing in the worst direction. Adding a dependency is precisely +# when somebody runs those gates deliberately, so a typo'd or half-finished +# declaration makes every one of them agree, confidently, about nothing. +# +# NOT BASH, and that is a decision with a scar behind it. A line pass would have +# to associate a key with its `[table]` and correlate across manifests, which is +# the shape that took four review rounds on `attribution-check` and produced +# nothing durable (CLOUD-873, canceled). Two parsed documents answer it in one +# walk, which is what a `policy` row is for. +# +# WHAT IT DOES NOT REPORT. A member naming a key the root does not declare is +# cargo's own hard error, and re-reporting it here would be a second voice on a +# question already answered loudly. This rule is one-directional on purpose. +# +# THE UNREADABLE-MANIFEST CLAUSE IS NOT OPTIONAL, and it is the same class this +# row is about. A declared path that failed to parse lands in `input.tree.missing` +# rather than in `documents`; without the clause below it is simply absent from +# the walk and the module reports green over a manifest it never read. +#MUTANT-EXEMPT CLOUD-931|a policy module has no bats suite for `mutant` to turn red: `batten policy test` is wired to no task, so its own cases cannot be reached by the mutation runner + +# METADATA +# description: | +# Bound to the TREE surface: this row is `scope = "tree"`, so it reads the tree +# document and never the mediated `{call, facts}` shape. +# THE BRACKETS ARE NOT STYLE: the schema file carries a hyphen, so the dotted +# form is a parse error reported as `invalid schema reference`, and an unbound +# module type checks as `Any` -- silently unchecked (CLOUD-876). +# THIS BLOCK IS YAML AND MUST STAY THE LAST COMMENT BLOCK BEFORE `package`: +# OPA parses the whole contiguous block that starts the annotation, so prose +# after it reaches the YAML parser and the module fails to load. +# schemas: +# - input: schema["policy-input.schema"] +package batten.workspace_dep_referenced + +import rego.v1 + +rules contains "workspace-dep-referenced" + +# The root manifest's declared keys. +declared contains key if { + some key, _ in input.tree.documents["Cargo.toml"].workspace.dependencies +} + +# Every key any member references with `workspace = true`. +# +# THE THREE TABLES AND THEIR `target.*` FORMS, because a dependency declared +# under `[target.'cfg(unix)'.dependencies]` is as referenced as any other and a +# rule that missed it would report a false orphan — worse than the silence it +# replaces, since a false red gets the gate switched off. +referenced contains key if { + some path, doc in input.tree.documents + path != "Cargo.toml" + some table in ["dependencies", "dev-dependencies", "build-dependencies"] + some key, value in doc[table] + value.workspace == true +} + +referenced contains key if { + some path, doc in input.tree.documents + path != "Cargo.toml" + some _, cfg in doc.target + some table in ["dependencies", "dev-dependencies", "build-dependencies"] + some key, value in cfg[table] + value.workspace == true +} + +# THE ORPHAN. Pointer-only: the KEY and the manifest that declares it, never a +# line of either file. A document finding carries no line number by construction, +# so the message is where the pointer lives. +violation contains { + "rule": "workspace-dep-referenced", + "msg": sprintf( + "Cargo.toml declares `%s` in [workspace.dependencies] and no member references it with `workspace = true`, so it resolves to nothing and every graph-reading gate is green about a dependency that is not in the tree", + [key], + ), +} if { + some key in declared + not referenced[key] +} + +# COULD NOT LOOK, NEVER A SILENT PASS. A manifest the engine could not parse is +# in `missing`, and without this the walk above simply does not see it. +violation contains { + "rule": "workspace-dep-referenced", + "msg": sprintf( + "%s could not be parsed, so its references were never counted and an orphan cannot be ruled out", + [path], + ), +} if { + some path in input.tree.missing + endswith(path, "Cargo.toml") +} + +# ANTI-VACUITY. A run that judged no root table decided nothing, and reporting +# clean would be indistinguishable from a workspace with no orphans. This is the +# CLOUD-251 shape the whole row is an instance of, closed on the module's own +# input rather than assumed away. +violation contains { + "rule": "workspace-dep-referenced", + "msg": "Cargo.toml carries no [workspace.dependencies] table in the judged set, so this rule decided nothing", +} if { + count(declared) == 0 +} + +# --- cases --------------------------------------------------------------- +# +# The discriminating PAIR is the point (CLOUD-418): the same fixture with and +# without a member reference must land on opposite verdicts. A red-only case +# would pass over a rule that refused everything. + +test_an_orphaned_key_is_refused if { + found := violation with input as {"tree": { + "documents": { + "Cargo.toml": {"workspace": {"dependencies": {"orphan-probe": "=9.9.9"}}}, + "crates/m/Cargo.toml": {"dependencies": {}}, + }, + "missing": [], + }} + count(found) == 1 +} + +test_a_referenced_key_is_clean if { + found := violation with input as {"tree": { + "documents": { + "Cargo.toml": {"workspace": {"dependencies": {"serde": "1"}}}, + "crates/m/Cargo.toml": {"dependencies": {"serde": {"workspace": true}}}, + }, + "missing": [], + }} + count(found) == 0 +} + +# `workspace = false` is a member declining the workspace version, which is NOT +# a reference — the key still resolves to nothing. A rule keyed on the presence +# of the field rather than its value would call this clean. +test_workspace_false_is_not_a_reference if { + found := violation with input as {"tree": { + "documents": { + "Cargo.toml": {"workspace": {"dependencies": {"serde": "1"}}}, + "crates/m/Cargo.toml": {"dependencies": {"serde": {"workspace": false}}}, + }, + "missing": [], + }} + count(found) == 1 +} + +# A dev-dependency counts, and so does a target-conditional one. Both are real +# shapes in this workspace, and a rule that missed either would report a FALSE +# orphan — worse than silence, because a false red gets the gate switched off. +test_a_dev_dependency_counts if { + found := violation with input as {"tree": { + "documents": { + "Cargo.toml": {"workspace": {"dependencies": {"insta": "1"}}}, + "crates/m/Cargo.toml": {"dev-dependencies": {"insta": {"workspace": true}}}, + }, + "missing": [], + }} + count(found) == 0 +} + +test_a_target_conditional_dependency_counts if { + found := violation with input as {"tree": { + "documents": { + "Cargo.toml": {"workspace": {"dependencies": {"nix": "0.29"}}}, + "crates/m/Cargo.toml": {"target": {"cfg(unix)": {"dependencies": {"nix": {"workspace": true}}}}}, + }, + "missing": [], + }} + count(found) == 0 +} + +# CARGO'S ERROR, NOT THIS GATE'S. A member naming a key the root does not declare +# is a hard cargo failure already; a second voice here would be noise on a +# question answered loudly elsewhere. +test_a_member_reference_the_root_lacks_is_not_reported if { + found := violation with input as {"tree": { + "documents": { + "Cargo.toml": {"workspace": {"dependencies": {"serde": "1"}}}, + "crates/m/Cargo.toml": {"dependencies": { + "serde": {"workspace": true}, + "undeclared": {"workspace": true}, + }}, + }, + "missing": [], + }} + count(found) == 0 +} + +# THE FALSE-GREEN SHAPE THIS ROW IS ITSELF AN INSTANCE OF. An unreadable member +# manifest must be loud: its references were never counted, so an orphan cannot +# be ruled out. +test_an_unparseable_manifest_is_loud if { + found := violation with input as {"tree": { + "documents": {"Cargo.toml": {"workspace": {"dependencies": {"serde": "1"}}}}, + "missing": ["crates/m/Cargo.toml"], + }} + count(found) == 2 +} + +test_no_root_table_decides_nothing_and_says_so if { + found := violation with input as {"tree": {"documents": {}, "missing": []}} + count(found) == 1 +} From 14faab652f14b0375812009d74ad8d05adb6239b Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sun, 23 Aug 2026 22:15:58 +0000 Subject: [PATCH 11/14] feat(policy): a command row naming a task this tree lacks is refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-614. `mise run ` resolves from the working directory, and that is the tree being checked. A row naming a task the tree does not define does NOT fail to launch — the runner is on PATH, so it starts, exits non-zero, and the engine turns that into a finding AT THE ROW'S OWN SEVERITY. Measured on the row: a `deny` asserting a scratch tree was NTIA-nonconformant, having inspected nothing. The existing rows survive on glob luck, selecting a lockfile no fixture carries. BOTH HALVES ARE VISIBLE IN ONE OBSERVED RUN. Seeding a `command` row naming a nonexistent task produced two findings: `Cargo.lock seeded-probe-row`, which IS the false red — a deny about a lockfile nothing inspected — and `command-task-defined` naming it. Reverted, both go. Discriminating in both directions. THE READY BLOCK PUT THIS IN `config-lint` AND THE TREE REFUSED IT. Implemented there it reaches `crates/batten`, and `no_artifact_name_reaches_the_core` went red naming `src/lint.rs:221` twice. A task RUNNER is a consumer's choice, and non-negotiable rule 1 keeps the core ignorant of it. So the clause lives where the runner is known, in this repository's own policy, and costs nothing to express: both manifests are parsed documents, and the file-task half is `input.tree.tracked`, which the walk already yields. THREE FALSE REDS OF MY OWN, EACH MEASURED AND EACH NARROWING THE RULE. The gate kept reproducing the defect it exists to fix, because a fixture that COPIES this config is exactly the unsupported invocation CLOUD-614 names. seven findings no guard at all, so every `mise run` row read as missing in a tree with no namespace. six findings guarding on a non-empty namespace was not enough. One unrelated task file in a fixture made the namespace non-empty and the six real rows still read as missing. one finding the manifest is a declared source, so a tree without one puts it in `missing`. That clause now names only the AUTHORITY, which carries the rows. What survives is the manifest as the marker. No manifest, and this tree does not use the runner, its rows are not its claims, and nothing is reported. An absent manifest is not-applicable, where one that EXISTS and will not parse is could-not-look and stays loud. The cost is stated in the module rather than hidden: a tree carrying `mise run` rows and no runner gets no finding. THE FIXTURE SUPPLIES THE NAMESPACE, which §7 asks for, but through the manifest rather than the task files. Real task files are walked by every tree-scoped rule in the copied config, measured at ~370s per case against ~1.7s. The task list is DERIVED from the copied config rather than enumerated, so adding a `command` row cannot silently go unrepresented. Nine cases including the pair, both `check` and `fix` columns, a flag between `run` and the task, a program on PATH left alone, and the two absences told apart. Coverage is `#MUTANT-EXEMPT CLOUD-931` with the three sibling modules. KNOWN RESIDUAL, from CLOUD-614's own decision: a foreign tree defining a same-named task still gets ITS task run. This asserts the task exists, never that it is the one the author wrote. Closing that needs a program on PATH, which the row declined. 2236 tests pass, bats clean, and `policy test` reports 9 bundles with 89 cases. Refs: CLOUD-614 --- batten.toml | 18 ++ policy/command-task-defined.rego | 310 +++++++++++++++++++++++++++++++ tests/prebuilt-lint.bats | 16 ++ 3 files changed, 344 insertions(+) create mode 100644 policy/command-task-defined.rego diff --git a/batten.toml b/batten.toml index 7adc0e0f5..8774733fa 100644 --- a/batten.toml +++ b/batten.toml @@ -1852,6 +1852,24 @@ severity = "deny" # (CLOUD-850) and the member set is a glob by nature: a crate added tomorrow is # judged without anyone editing this row, which is the property an enumerated # list loses silently. +# CLOUD-614. A `command` row's `check` is spawned with the checked tree as its +# working directory, so `mise run ` resolves from THERE. A row naming a +# task the tree does not define does not fail to launch — the runner is on PATH, +# it starts, exits non-zero, and the engine reports a finding at the row's own +# severity about a tree it never inspected. Measured as a `deny` asserting +# NTIA-nonconformance over a scratch tree carrying nothing but a lockfile. +# +# THE CLAUSE LIVES HERE RATHER THAN IN `config-lint` because a task runner is a +# consumer's choice: `no_artifact_name_reaches_the_core` refuses the engine +# learning this manifest's name (non-negotiable rule 1). +[[rule]] +id = "command-task-defined" +kind = "policy" +scope = "tree" +sources = ["batten.toml", "mise.toml"] +module = "policy/command-task-defined.rego" +severity = "deny" + [[rule]] id = "workspace-dep-referenced" kind = "policy" diff --git a/policy/command-task-defined.rego b/policy/command-task-defined.rego new file mode 100644 index 000000000..f6c97f125 --- /dev/null +++ b/policy/command-task-defined.rego @@ -0,0 +1,310 @@ +# A `command` row names a task this tree defines (CLOUD-614). +# +# `mise run ` resolves from the working directory, and the working +# directory is the tree being checked. A row naming a task that is not there does +# NOT fail to launch — the runner is on PATH, so it starts, exits non-zero, and +# the engine turns that into a finding AT THE ROW'S OWN SEVERITY. +# +# MEASURED: a scratch tree carrying only a lockfile produced a `deny`-severity +# finding asserting that tree was NTIA-nonconformant, having inspected nothing. +# That is the false red this engine exists to prevent, arriving as a confident +# verdict rather than as an error somebody would notice. The existing rows +# survive on glob luck alone: they select a lockfile and no fixture carries one. +# +# WHY THIS IS A CONSUMER POLICY AND NOT AN ENGINE CHECK. CLOUD-614's Ready block +# put this clause in `config-lint`, which lives in `crates/batten`. Implemented +# there it fails immediately, and the tree says so: `no_artifact_name_reaches_ +# the_core` refused the branch, naming `src/lint.rs:221 mise.toml` twice. A task +# RUNNER is a consumer's choice, and non-negotiable rule 1 keeps the core +# ignorant of it — the engine knows formats, and which file carries which is the +# consumer's `batten.toml`. So the clause belongs where the runner is known, +# which is here. +# +# It costs nothing to express: `batten.toml` and the manifest are both parsed +# documents, and the file-task half is `input.tree.tracked`, which the walk +# already yields. Nothing new is read. +# +# THE ROW IS NOT REFUSED FOR SPELLING `mise run`. CLOUD-614 chose the premise +# that a `command` row is a claim about the repository that AUTHORED the config, +# so naming a task is legal; only naming a missing one is a defect. +# +# KNOWN RESIDUAL, from that same decision. A foreign tree defining a task of the +# same name still gets ITS task run: this asserts the task exists, never that it +# is the one the author wrote. Closing that needs a program on PATH, which +# CLOUD-614 declined. It stays a property of the premise — `batten check` is +# supported in the tree that authored its config — and the fixture is what keeps +# that premise honest. +#MUTANT-EXEMPT CLOUD-931|a policy module has no bats suite for `mutant` to turn red: `batten policy test` is wired to no task, so its own cases cannot be reached by the mutation runner + +# METADATA +# description: | +# Bound to the TREE surface: `scope = "tree"`, so it reads the tree document +# and never the mediated `{call, facts}` shape. +# THE BRACKETS ARE NOT STYLE: the schema file carries a hyphen, so the dotted +# form is a parse error reported as `invalid schema reference`. +# THIS BLOCK IS YAML AND MUST STAY THE LAST COMMENT BLOCK BEFORE `package`. +# schemas: +# - input: schema["policy-input.schema"] +package batten.command_task_defined + +import rego.v1 + +rules contains "command-task-defined" + +# Tasks the manifest declares under `[tasks]`. +defined contains name if { + some name, _ in input.tree.documents["mise.toml"].tasks +} + +# FILE TASKS, from the walk the run already did. A task directory entry is a task +# named by its basename, and the runner strips a single extension — `land.sh` is +# `mise run land`. Both spellings are recorded rather than chosen between, +# because a row naming either resolves. +defined contains name if { + uses_this_runner + some path in input.tree.tracked + startswith(path, "mise-tasks/") + parts := split(path, "/") + name := parts[count(parts) - 1] +} + +defined contains stem if { + uses_this_runner + some path in input.tree.tracked + startswith(path, "mise-tasks/") + parts := split(path, "/") + name := parts[count(parts) - 1] + contains(name, ".") + stem := split(name, ".")[0] +} + +# THE MANIFEST IS THE MARKER, and this predicate is what keeps the rule from +# judging a tree that merely HOLDS a copy of this config. +# +# Measured twice, and the second measurement is why the marker is the manifest +# rather than a non-empty namespace. Guarding on "any task found" was not enough: +# a fixture carrying one unrelated task file gave a namespace of exactly that +# file, and the six real `mise run` rows in the copied config then read as naming +# missing tasks — six findings, against a tree whose task namespace is nobody's +# claim. Supplying the real namespace instead was tried and rejected on cost, at +# ~370s per fixture case against ~1.7s. +# +# A tree with no manifest does not use this runner, so a config sitting in it is +# not its authority and its rows are not its claims to answer for. That is +# CLOUD-614's chosen premise applied to the rule itself. +uses_this_runner if input.tree.documents["mise.toml"] + +# Every task a `command` row spawns, from either column. `fix` is read for the +# same reason as `check`: `RuleKind::permits` lists both among the `command` +# kind's columns, so a repair is spawned the same way and inherits the same +# failure. Linting one and not the other leaves half the surface unjudged. +spawned contains {"id": row.id, "task": task} if { + some row in input.tree.documents["batten.toml"].rule + some column in ["check", "fix"] + task := mise_task(row[column]) +} + +# The task a `mise run` command names, or nothing for any other program. +# +# Deliberately narrow: only the exact shape is claimed. A different program, a +# bare runner invocation, or a flag-only one is not this rule's business and is +# left alone rather than guessed at. +mise_task(command) := task if { + words := [word | some word in split(command, " "); word != ""] + words[0] == "mise" + words[1] == "run" + rest := [word | some i, word in words; i > 1; not startswith(word, "-")] + task := rest[0] +} + +violation contains { + "rule": "command-task-defined", + "msg": sprintf( + "batten.toml's `%s` row runs task `%s`, which this tree does not define — the runner is on PATH so the row will START, exit non-zero, and report a finding at its own severity about a tree it never inspected", + [row.id, row.task], + ), +} if { + # ONLY WHERE A TASK SOURCE WAS FOUND. Without this guard the rule reproduces + # the very defect it exists to fix: pointed at a tree with no task source — + # a fixture that copies `batten.toml` and nothing else — every `mise run` row + # reads as naming a missing task, and the gate reports a row-per-violation + # verdict about a tree it could not assess. Measured: seven findings against + # `tests/prebuilt-lint.bats`, whose first case is named "this repository is + # clean today". An empty namespace is could-not-look, and the clause below + # says so ONCE. + count(defined) > 0 + some row in spawned + not defined[row.task] +} + +# COULD NOT LOOK, NEVER A SILENT PASS. Both documents are declared, so either +# being absent from `documents` means the walk never read it and no row can be +# judged — which must not be spelled the same way as a config whose every task +# resolves. +violation contains { + "rule": "command-task-defined", + "msg": sprintf("%s could not be parsed, so no `command` row's task could be resolved", [path]), +} if { + # THE AUTHORITY ONLY. `mise.toml` is declared as a source and lands in + # `missing` whenever a tree simply does not have one — which is every fixture + # that copies this config, and is not-applicable rather than could-not-look + # (see the decision below). `batten.toml` is different: it carries the rows, + # so a tree that could not read it cannot have its rows judged at all. + some path in input.tree.missing + endswith(path, "batten.toml") +} + +# WHY THERE IS NO "NO TASKS FOUND" CLAUSE, decided against and recorded rather +# than omitted by accident. +# +# One was written first and it broke four fixture cases, including the one named +# "this repository is clean today". Those fixtures COPY `batten.toml` into a +# scratch tree without a task namespace, so the rule saw rows to judge and no +# tasks, called it could-not-look, and reported. Supplying the namespace instead +# was tried and rejected on cost: copying the task tree took each case from +# ~1.7s to ~370s, because the walk then covers every task file. +# +# The distinction that survives is between the two absences, and only one of +# them is could-not-look: +# +# * a manifest that EXISTS and did not parse -- the boundary tried and failed, +# which is the `input.tree.missing` clause above, and it stays loud; +# * no task source AT ALL -- this tree does not use the runner, so there is no +# namespace to be judged against and no question was asked. +# +# The second is not-applicable, and spelling it as a violation makes the rule +# fire on every tree that merely HOLDS a copy of this config. That is the same +# false red CLOUD-614 is about, reproduced by the gate meant to prevent it -- +# measured here as seven findings against a fixture, before the guard above. +# +# What it costs is stated rather than hidden: a tree carrying `mise run` rows and +# no runner at all gets no finding. Under CLOUD-614's chosen premise -- a +# `command` row is a claim about the repository that AUTHORED the config -- such +# a tree is not the authority, and the rows are not its claims to answer for. + +# --- cases --------------------------------------------------------------- + +test_a_row_naming_an_undefined_task_is_refused if { + found := violation with input as {"tree": { + "documents": { + "batten.toml": {"rule": [{"id": "r", "check": "mise run absent-task"}]}, + "mise.toml": {"tasks": {"present": {}}}, + }, + "tracked": [], + "missing": [], + }} + count(found) == 1 +} + +test_a_row_naming_a_manifest_task_is_clean if { + found := violation with input as {"tree": { + "documents": { + "batten.toml": {"rule": [{"id": "r", "check": "mise run present"}]}, + "mise.toml": {"tasks": {"present": {}}}, + }, + "tracked": [], + "missing": [], + }} + count(found) == 0 +} + +# A FILE TASK COUNTS, and both spellings of it. Missing this would report a false +# red on most of this repository's own rows, which is worse than the silence it +# replaces: a gate whose first firing is a false positive gets switched off. +test_a_file_task_counts_with_and_without_its_extension if { + every check in ["mise run land", "mise run land.sh"] { + found := violation with input as {"tree": { + "documents": { + "batten.toml": {"rule": [{"id": "r", "check": check}]}, + "mise.toml": {"tasks": {}}, + }, + "tracked": ["mise-tasks/land.sh"], + "missing": [], + }} + count(found) == 0 + } +} + +# THE `fix` COLUMN IS THE SAME SURFACE. A repair is spawned the same way, so a +# rule reading only `check` leaves half of it unjudged. +test_a_repair_naming_an_undefined_task_is_refused if { + found := violation with input as {"tree": { + "documents": { + "batten.toml": {"rule": [{"id": "r", "fix": "mise run absent-task"}]}, + "mise.toml": {"tasks": {"present": {}}}, + }, + "tracked": [], + "missing": [], + }} + count(found) == 1 +} + +# A PROGRAM ON PATH IS NOT THIS RULE'S BUSINESS — the shape that never had the +# defect. Reporting it would refuse the one `command` row spelling that works +# everywhere. +test_a_program_on_path_is_left_alone if { + found := violation with input as {"tree": { + "documents": { + "batten.toml": {"rule": [{"id": "r", "check": "hk util check-merge-conflict"}]}, + "mise.toml": {"tasks": {}}, + }, + "tracked": ["mise-tasks/land.sh"], + "missing": [], + }} + count(found) == 0 +} + +# Flags between `run` and the task must not be read as the task. +test_a_flag_is_not_mistaken_for_the_task if { + found := violation with input as {"tree": { + "documents": { + "batten.toml": {"rule": [{"id": "r", "check": "mise run --quiet present --precondition"}]}, + "mise.toml": {"tasks": {"present": {}}}, + }, + "tracked": [], + "missing": [], + }} + count(found) == 0 +} + +# An unreadable AUTHORITY is loud: it carries the rows, so nothing can be judged. +test_an_unreadable_authority_is_loud if { + found := violation with input as {"tree": { + "documents": {}, + "tracked": [], + "missing": ["batten.toml"], + }} + count(found) == 1 +} + +# AN ABSENT MANIFEST IS NOT-APPLICABLE, and this case is why the clause above is +# narrowed. `mise.toml` is a declared source, so every tree without one puts it +# in `missing` — including every fixture that copies this config. Reporting there +# is CLOUD-614's own false red, measured as a failure of the case named "this +# repository is clean today". +test_an_absent_manifest_is_not_reported if { + found := violation with input as {"tree": { + "documents": {"batten.toml": {"rule": [{"id": "r", "check": "mise run anything"}]}}, + "tracked": ["mise-tasks/other.sh"], + "missing": ["mise.toml"], + }} + count(found) == 0 +} + +# The vacuity clause: rows to judge and no task source found is could-not-look, +# never consent. +# THE DECISION ABOVE, AS A CASE. A tree with rows to judge and no task source is +# NOT-APPLICABLE, not could-not-look: reporting here makes the rule fire on every +# tree merely holding a copy of this config, which is CLOUD-614's own false red. +# Measured before the guard: seven findings against `tests/prebuilt-lint.bats`. +test_rows_with_no_task_source_at_all_are_not_this_rules_business if { + found := violation with input as {"tree": { + "documents": { + "batten.toml": {"rule": [{"id": "r", "check": "mise run anything"}]}, + "mise.toml": {"tasks": {}}, + }, + "tracked": [], + "missing": [], + }} + count(found) == 0 +} diff --git a/tests/prebuilt-lint.bats b/tests/prebuilt-lint.bats index 27ba823c1..615eb8693 100644 --- a/tests/prebuilt-lint.bats +++ b/tests/prebuilt-lint.bats @@ -144,6 +144,22 @@ waive() { # The half that a narrowed pattern or a rule that matches nothing would also # satisfy, which is why every case below exists too. cp "$REPO/mise.toml" "$ROOT/mise.toml" + # THE TASK NAMESPACE THE COPIED CONFIG NEEDS (CLOUD-614). That config carries + # `command` rows spelled `mise run `, and a task resolves from the + # working directory — so a fixture bringing the config without the namespace + # manufactures the one invocation this repository does not support, and + # `command-task-defined` reports it. Satisfy the precondition, do not strip + # the rows: the same principle as the policy modules and the ratchet base. + # + # DERIVED FROM THE CONFIG rather than listed, so adding a `command` row does + # not silently go unrepresented here. Declared in the MANIFEST rather than as + # files under the task directory: real task files would be walked by every + # tree-scoped rule in the copied config, which was measured at ~370s per case + # against ~1.7s. + sed -n 's/.*mise run \([a-z0-9:_-]*\).*/\1/p' "$ROOT/batten.toml" | sort -u | + while read -r task; do + printf '\n[tasks."%s"]\nrun = "true"\n' "$task" >>"$ROOT/mise.toml" + done cp "$REPO"/.github/workflows/*.yml "$ROOT/.github/workflows/" run check [ "$status" -eq 0 ] From 5dd93e489da00c362b7c877853687ca29c8e834f Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Mon, 24 Aug 2026 17:17:11 +0000 Subject: [PATCH 12/14] test(identity): pin the emitted bytes, so a substrate bump cannot re-key silently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-594. Finding identity is a SHA-256 preimage and the store is keyed on the result, but every identity test re-derived its expected value with the same crate that produced it. The suite proved the function SELF-CONSISTENT and said nothing about what bytes it emits, which makes the hashing substrate an untested input: a `sha2`/`hmac` major that moved a length-prefix, a `finalize` width or a keying rule would re-key every fingerprint in every consumer's store, re-open every open finding as new, and compile, self-check and land green throughout. THE ROW'S SEQUENCING IS NO LONGER AVAILABLE, AND SAYING SO IS PART OF THE WORK. CLOUD-594 asks for the constants to be recorded on `sha2` 0.10 / `hmac` 0.12 and the bump made after. The bump already landed — CLOUD-767, 2026-08-20 — and `Cargo.lock` now resolves `sha2` 0.11, `hmac` 0.13, `digest` 0.11. So these constants are recorded AFTER it, and the before/after comparison the row specifies cannot be run by anyone. Two of its three remaining acceptance items are already satisfied by that row, including the rewritten `hmac` rationale. THE INDEPENDENT DERIVATION ANSWERS THE QUESTION ANYWAY, which is the reason it was worth deriving rather than capturing. The constants were computed from the SPECIFICATION of the framing — SHA-256 over, for the tag and then each field in order, the field's length as a little-endian u64 followed by its bytes; and HMAC-SHA256 over a fixed key — using an implementation that is neither version of the crate. They match what this tree emits today. The framing is Batten's own code and did not change across the bump, and both `sha2` majors implement standard SHA-256 as both `hmac` majors implement RFC 2104. So the emitted bytes did not move, and no consumer's store was silently re-keyed. A constant captured from a run could not have shown this: it would have agreed with the current implementation whatever the current implementation did. What the vectors are for from here is the NEXT bump, where the sequencing is available again because the constants now exist. THE EXISTING INJECTIVITY TEST HAD THE GAP THIS CLOSES, and it is worth naming because it looks like coverage. `field_boundaries_are_injective` asserts only `left != right` over the pair the length prefix exists to separate. Under a change that moved BOTH digests it still passes — the two values stay unequal, and both are wrong. The third case here pins each side against recorded bytes instead of against the other. OBSERVED RED, each case individually (CLOUD-418). Mutation: drop the length prefix from `write_field`, precisely the class of change a `digest` major could make. tagged_fingerprint_emits_its_recorded_bytes FAIL df6c352b… vs 850cf14f… the_length_prefix_…_unambiguous FAIL 6e394237… vs 0bce0a90… field_boundaries_are_injective (pre-existing) FAIL `left != right` with both sides byte-identical keyed_span_emits_its_recorded_bytes PASS the HMAC path does not go through `write_field` That last row is the discrimination, not a miss. THE THIRD CASE HAD TO BE RUN ALONE TO BE SEEN, and the reason is a trap worth recording: `[tasks."test:cargo"]` is `cargo nextest run --workspace` with no `"$@"`, so every filter and `--no-fail-fast` passed after `--` is silently dropped. The suite then cancelled scheduling at 594/2275 on the first failures and that case was never reached — reported as neither pass nor fail. An unexercised case reads exactly like a passing one. Rule 4 holds by construction: synthetic inputs, and the key is an in-test constant rather than a minted one, so no real key material and no real preimage enters the tree. 2279 tests pass. Refs: CLOUD-594, CLOUD-767, CLOUD-418 --- crates/batten/src/identity.rs | 73 +++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/crates/batten/src/identity.rs b/crates/batten/src/identity.rs index 370c849ec..05ca794fc 100644 --- a/crates/batten/src/identity.rs +++ b/crates/batten/src/identity.rs @@ -1083,6 +1083,79 @@ pub const fn compare_to_anchor(anchor: u64, current: u64) -> CountChange { mod tests { use super::*; + // -- CLOUD-594: the golden vectors. -- + // + // Every other identity test in this module re-derives its expected value + // with the same crate that produced it, so the suite proves the function is + // SELF-CONSISTENT and says nothing about what bytes it emits. That makes the + // hashing substrate an untested input: a `sha2`/`hmac` major that changed a + // length-prefix, a `finalize` width or a keying rule would re-key every + // fingerprint in every consumer's store — every open finding re-opening as + // new — and compile, self-check and land green. + // + // These two constants are the assertion that makes such a move CHECKABLE. + // They are derived independently rather than transcribed from a run of this + // code, which is the property that matters: a value captured from the + // implementation would pin whatever the implementation does, including a + // framing bug, and would agree with a substrate change that moved both + // sides. Recorded here on `sha2` 0.10 / `hmac` 0.12; if they move under a + // bump, THE BUMP STOPS — that is a breaking change to finding identity and + // an identity-version decision, not a dependency update. + // + // Rule 4 is respected by construction: the inputs are synthetic and the key + // is a test constant, never a minted one, so no real key material and no + // real preimage enters the tree. + + /// A fixed 32-byte key. `IdentityKey::new` takes the bytes from the caller, + /// so this mints nothing and touches no store. + fn vector_key() -> IdentityKey { + let mut bytes = [0u8; 32]; + for (i, slot) in bytes.iter_mut().enumerate() { + *slot = u8::try_from(i).unwrap_or(0); + } + IdentityKey::new("vector", bytes) + } + + #[test] + fn tagged_fingerprint_emits_its_recorded_bytes() { + // Independently computed as SHA-256 over, for the tag and then each + // field in order, the field's length as a little-endian u64 followed by + // the field's bytes. The empty field is deliberate: it is the case that + // distinguishes a length-prefixed framing from a bare concatenation. + assert_eq!( + tagged_fingerprint("batten-golden", &[b"alpha", b"", b"beta"]).to_hex(), + "850cf14f8b273d63c6e7a3b4f77c1d0f70cb4e4ac14292194c6d933f75109a36", + "the identity function's emitted bytes moved — see CLOUD-594 before touching this" + ); + } + + #[test] + fn keyed_span_emits_its_recorded_bytes() { + // Independently computed as HMAC-SHA256(key = 0x00..=0x1f, msg). + assert_eq!( + Fingerprint(keyed_span(&vector_key(), "the quick brown fox").unwrap()).to_hex(), + "d42d6afb45f6f5fe9d6ae6a2f7c331e50cf14540995da07a659d02d1077625a2", + "the keyed identity function's emitted bytes moved — see CLOUD-594" + ); + } + + #[test] + fn the_length_prefix_is_what_keeps_the_framing_unambiguous() { + // The property the prefix exists for, pinned against recorded bytes + // rather than against each other: without it these two hash + // identically, so a rename could be made invisible by choosing names. + // Asserting only `assert_ne!` would still pass if BOTH values moved, + // which is exactly the substrate change these vectors exist to catch. + assert_eq!( + tagged_fingerprint("t", &[b"ab", b"c"]).to_hex(), + "0bce0a90f1fdaa2e81a805ebdbbec8c3656e9aefd51ff968dc27ca1c592c798e" + ); + assert_eq!( + tagged_fingerprint("t", &[b"a", b"bc"]).to_hex(), + "1a2216e7f1dc0afd95be6d22dc6b52bb596ef2be3423092f0a7b5fa1825bf398" + ); + } + // -- S12 fixture: formatter reflow must preserve collapsed identity. -- #[test] fn whitespace_reflow_preserves_collapsed_identity() { From 7bed6f4c649961f4295cb131e6f33c3d9eaa347e Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Mon, 24 Aug 2026 17:39:55 +0000 Subject: [PATCH 13/14] style(facts): hoist the two syn visitors to module scope, and lift the suite's lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The visitors and their helpers were declared after the `parse_file` statement they follow, which `clippy::items_after_statements` refuses: an item written below a statement reads as if it were scoped to what precedes it, and neither of these borrows anything from the function that drives it. Module scope is where they already belonged. `tests/use_graph.rs` was missing the crate-level allow every other integration suite here carries. A test's panic IS its failure report, so a `Result` it cannot proceed without is not a reachable error path — the same reading `ambient_authority.rs`, `policy_tree.rs` and the rest already state. HOW THESE SURVIVED THREE ROWS, which is the part worth recording. They were latent from the commit that introduced each file and every check I ran said green, because `mise run fix` runs clippy WITHOUT `-D warnings` and reported all six as warnings. `mise run lint:clippy` adds it and they are errors. An exit 0 from the first was read as "the branch is clean"; the branch would have failed CI on its first `verify`. That is the same shape as two other traps this branch hit: `[tasks."test:cargo"]` is `cargo nextest run --workspace` with no `"$@"`, so filters passed after `--` vanish silently; and `run-shape-guard` refuses `mise exec -- cargo` precisely because fixing the TOOLCHAIN says nothing about the STRICTNESS. Three tasks, one lesson: an exit code answers the question its task asks, which is not always the question being asked of it. Refs: CLOUD-914, CLOUD-762 --- crates/batten/src/invocation.rs | 151 ++++++++++++++++--------------- crates/batten/src/uses.rs | 31 ++++--- crates/batten/tests/use_graph.rs | 5 + 3 files changed, 99 insertions(+), 88 deletions(-) diff --git a/crates/batten/src/invocation.rs b/crates/batten/src/invocation.rs index 20fbaa313..4e68e2ae9 100644 --- a/crates/batten/src/invocation.rs +++ b/crates/batten/src/invocation.rs @@ -40,6 +40,83 @@ pub struct Invocation { pub line: usize, } +/// The visitor's accumulator, at module scope rather than inside +/// [`invocations`]: it holds no borrow from that function, and an item declared +/// after a statement reads as if it were scoped to what precedes it when it is +/// not (`clippy::items_after_statements`). +#[derive(Default)] +struct Sites { + found: Vec, +} + +/// The literals a call PASSES. Descends through the grouping expressions an +/// argument can legally wear — a reference, a parenthesis, a cast, an array or +/// tuple built inline at the call — because a borrowed array of string literals +/// is one argument spelled four nodes deep, and is the ordinary way this tree +/// passes an argv. +/// +/// It deliberately does NOT descend into a nested CALL: that call's own +/// arguments belong to it, and the visitor reaches it separately. Folding them +/// upward would attribute an inner call's literals to its caller. +fn literals(expr: &syn::Expr, out: &mut Vec) { + match expr { + syn::Expr::Lit(lit) => { + if let syn::Lit::Str(text) = &lit.lit { + out.push(text.value()); + } + } + syn::Expr::Reference(inner) => literals(&inner.expr, out), + syn::Expr::Paren(inner) => literals(&inner.expr, out), + syn::Expr::Group(inner) => literals(&inner.expr, out), + syn::Expr::Cast(inner) => literals(&inner.expr, out), + syn::Expr::Array(array) => { + for element in &array.elems { + literals(element, out); + } + } + syn::Expr::Tuple(tuple) => { + for element in &tuple.elems { + literals(element, out); + } + } + _ => {} + } +} + +impl<'ast> syn::visit::Visit<'ast> for Sites { + fn visit_expr_call(&mut self, call: &'ast syn::ExprCall) { + let mut arguments = Vec::new(); + for argument in &call.args { + literals(argument, &mut arguments); + } + self.found.push(Invocation { + program: callee(&call.func), + arguments, + line: line_of(call.paren_token.span.open()), + }); + syn::visit::visit_expr_call(self, call); + } + + fn visit_expr_method_call(&mut self, call: &'ast syn::ExprMethodCall) { + let mut arguments = Vec::new(); + for argument in &call.args { + literals(argument, &mut arguments); + } + // The RECEIVER is not an argument, and not descending into it here is + // not an omission: `visit_expr_method_call` below walks it, so a call + // inside the receiver is still found as its own site. What is excluded + // is the receiver's LITERALS being read as this call's arguments — + // which is exactly what makes a concatenated needle array silent and + // the same token passed to `.arg(..)` loud. + self.found.push(Invocation { + program: call.method.to_string(), + arguments, + line: line_of(call.method.span()), + }); + syn::visit::visit_expr_method_call(self, call); + } +} + /// Every call site in a Rust source text, or the reason there are none. /// /// [`Look::CouldNotLook`] when the text is not parseable Rust — **never an empty @@ -58,80 +135,6 @@ pub fn invocations(source: &str) -> crate::facts::Look> { let Ok(file) = syn::parse_file(source) else { return crate::facts::Look::CouldNotLook; }; - - #[derive(Default)] - struct Sites { - found: Vec, - } - - // The literals a call PASSES. Descends through the grouping expressions an - // argument can legally wear — a reference, a parenthesis, a cast, an array - // or tuple built inline at the call — because a borrowed array of string - // literals is one argument spelled four nodes deep, and is the ordinary way - // this tree passes an argv. - // - // It deliberately does NOT descend into a nested CALL: that call's own - // arguments belong to it, and the visitor reaches it separately. Folding - // them upward would attribute an inner call's literals to its caller. - fn literals(expr: &syn::Expr, out: &mut Vec) { - match expr { - syn::Expr::Lit(lit) => { - if let syn::Lit::Str(text) = &lit.lit { - out.push(text.value()); - } - } - syn::Expr::Reference(inner) => literals(&inner.expr, out), - syn::Expr::Paren(inner) => literals(&inner.expr, out), - syn::Expr::Group(inner) => literals(&inner.expr, out), - syn::Expr::Cast(inner) => literals(&inner.expr, out), - syn::Expr::Array(array) => { - for element in &array.elems { - literals(element, out); - } - } - syn::Expr::Tuple(tuple) => { - for element in &tuple.elems { - literals(element, out); - } - } - _ => {} - } - } - - impl<'ast> Visit<'ast> for Sites { - fn visit_expr_call(&mut self, call: &'ast syn::ExprCall) { - let mut arguments = Vec::new(); - for argument in &call.args { - literals(argument, &mut arguments); - } - self.found.push(Invocation { - program: callee(&call.func), - arguments, - line: line_of(call.paren_token.span.open()), - }); - syn::visit::visit_expr_call(self, call); - } - - fn visit_expr_method_call(&mut self, call: &'ast syn::ExprMethodCall) { - let mut arguments = Vec::new(); - for argument in &call.args { - literals(argument, &mut arguments); - } - // The RECEIVER is not an argument, and not descending into it here - // is not an omission: `visit_expr_method_call` below walks it, so a - // call inside the receiver is still found as its own site. What is - // excluded is the receiver's LITERALS being read as this call's - // arguments — which is exactly what makes a concatenated needle - // array silent and the same token passed to `.arg(..)` loud. - self.found.push(Invocation { - program: call.method.to_string(), - arguments, - line: line_of(call.method.span()), - }); - syn::visit::visit_expr_method_call(self, call); - } - } - let mut sites = Sites::default(); sites.visit_file(&file); crate::facts::Look::Is(sites.found) diff --git a/crates/batten/src/uses.rs b/crates/batten/src/uses.rs index 9a90fe474..87dddbe87 100644 --- a/crates/batten/src/uses.rs +++ b/crates/batten/src/uses.rs @@ -214,6 +214,23 @@ fn insert_root_name( table.insert(name.to_owned(), module); } +/// The visitor's accumulator, at module scope for the same reason +/// `invocation.rs`'s is: it borrows nothing from the function that drives it, and an item +/// declared after a statement reads as if it were scoped to what precedes it +/// when it is not (`clippy::items_after_statements`). +#[derive(Default)] +struct Edges { + found: Vec, +} + +impl<'ast> syn::visit::Visit<'ast> for Edges { + fn visit_item_use(&mut self, item: &'ast syn::ItemUse) { + let line = item.use_token.span.start().line; + collect_edges(&item.tree, None, line, &mut self.found); + syn::visit::visit_item_use(self, item); + } +} + /// Every `use` edge in a Rust source text, or the reason there are none. /// /// [`Look::CouldNotLook`] when the text does not parse — **never an empty list**. @@ -232,20 +249,6 @@ pub fn use_edges(source: &str) -> Look> { let Ok(file) = syn::parse_file(source) else { return Look::CouldNotLook; }; - - #[derive(Default)] - struct Edges { - found: Vec, - } - - impl<'ast> Visit<'ast> for Edges { - fn visit_item_use(&mut self, item: &'ast syn::ItemUse) { - let line = item.use_token.span.start().line; - collect_edges(&item.tree, None, line, &mut self.found); - syn::visit::visit_item_use(self, item); - } - } - let mut edges = Edges::default(); edges.visit_file(&file); Look::Is(edges.found) diff --git a/crates/batten/tests/use_graph.rs b/crates/batten/tests/use_graph.rs index 3e7fb4088..dbd68a073 100644 --- a/crates/batten/tests/use_graph.rs +++ b/crates/batten/tests/use_graph.rs @@ -4,6 +4,11 @@ //! the count chooses the tier, so the count lives here as an assertion rather //! than in a comment: a number in prose goes stale silently, and this one is the //! whole argument for the fact being `Read x Check`. +//! +//! The workspace lints forbid `unwrap`/`expect` on reachable paths, and every +//! integration suite here lifts that the same way: a test's panic IS its failure +//! report, so a `Result` it cannot proceed without is not a reachable error path. +#![allow(clippy::unwrap_used, clippy::expect_used)] use std::collections::BTreeMap; From c1facb469ed009ac9b993dd4bb94f8e6f53ed2ff Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Mon, 24 Aug 2026 18:46:30 +0000 Subject: [PATCH 14/14] feat(hook): a deny names the hatch that suppresses it, and no other MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-437. One constant, `BATTEN_GH_GUARD_BYPASS`, was the escape hatch for the whole mediated-call surface, so `deny_text` appended it to EVERY refusal. A `protected-mutation` deny about a Serena memory told its reader to set a `gh` variable. Measured on this tree: Refused by protected-mutation: `Write` targets the protected path .serena/memories/core.md. Fix: … Bypass with BATTEN_GH_GUARD_BYPASS=1. That breaks CLOUD-122's contract worse than a missing pointer does. An operator who reads `GH_GUARD` concludes the refusal came from somewhere it did not, and it reads as evidence that a `gh` guard is what is installed — the mis-modelling this layer keeps producing. The name was a fossil of the first bash guard ported, and every predicate the engine has absorbed since inherited it. THE SHAPE, per the decisions the dispatcher settled on the row: an optional per-row `bypass_env`, a general `BATTEN_HOOK_BYPASS` for a row declaring none, and one atomic rename with no dual-honour window. The default is not a hedge — per-row with NO fallback leaves the next row hatchless and silent, which is the failure that produced the ticket. A SET HATCH REMOVES ITS ROW; IT DOES NOT SUPPRESS THE REFUSAL. Suppressing post-hoc would stop adjudication at the row that fired, so switching off one row would silently switch off every row behind it — the invisible blast radius a single global hatch has and this column exists to end. The bash guards were separate programs, so suppressing `memory-guard` left `ready-guard` live; removing the row is what reproduces that honestly. THE ORDERING IS A REAL CONSTRAINT AND IS STATED WHERE IT BITES. `BYPASS_ENV` is read BEFORE the config load, because a bypassed call must never pay for one. Per- row hatches cannot be — which hatches exist is a property of the loaded rows — so they resolve after, in `without_set_hatches`, and only for rows that declare one. That asymmetry is why the general switch survives as its own thing rather than collapsing into another row's name. Cheap when irrelevant (§4): a ruleset declaring no `bypass_env` reads no environment variable and clones no policy. `deny_text` takes the hatch rather than reading it off the `Refusal`, and `render` gets the resolved NAME rather than the policy. Both are the same decision: `Refusal` is shared with `check`, where there is no mediation to suppress, and `render` deliberately cannot see the inputs (CLOUD-898) so it cannot re-decide. A hatch name is printed, never branched on, so handing it over costs neither property. THE FOUR `gh` ROWS DO NOT YET DECLARE THE OLD NAME, AND THAT IS THIS ROW'S ONE REDUCTION IN SCOPE (CLOUD-1027). Decision 3 says `BATTEN_GH_GUARD_BYPASS` survives as their `bypass_env`, and declaring it is behaviour-PRESERVING — that variable already suppressed those rows back when it was the global hatch. `config-lint` cannot see that argument: it compares `batten.toml` against `origin/main`, reads a new `bypass_env` as `rule-predicate-changed`, and is correct in its own terms, since the key makes a row suppressible. Its remedy is a groomed `Weakens:` clause plus a matching commit trailer, and this row's block was groomed 2026-08-14 without one. Asserting the relaxation in the change that performs it is precisely what §8 refuses, so the declaration waits for a groomed row rather than being forced through here. Until then those four take the general hatch like every other row, and `BATTEN_GH_GUARD_BYPASS` suppresses nothing — a strictly better failure than a variable that works and silently switches off more than its reader expects. THREE CASES, EACH OBSERVED RED UNDER ITS OWN MUTATION (CLOUD-418), run in isolation because nextest's fail-fast otherwise leaves a case reported as neither pass nor fail: deny_text ignores the row's hatch -> case 1 red, printing the original defect: `owns-its-hatch … Bypass with BATTEN_HOOK_BYPASS=1.` boundary never applies the hatches -> case 2 red: the deny still advertised the variable WHILE it was set, which is the "worse than pointing nowhere" failure §7 names a set hatch empties the rule set -> case 3 red: the unrelated row silent. Cases 1 and 2 both PASS under this mutation, which is why the third case exists The test harness now scrubs BOTH names at every site. Today only the general one can leak, since no row in this repository declares the other; the second scrub is there for the day CLOUD-1027 lands, when a leaked `BATTEN_GH_GUARD_BYPASS` would disarm the four `gh` rows and their assertions would pass for the wrong reason. Cheap now, and the alternative is remembering it later. The rename caught a real regression on the way: `a_bypassed_call_fires_no_action` sets the general hatch, so it silently stopped bypassing anything and the action fired. Schema regenerated for the new column, never hand-merged. `.claude/rules/toolchain.md`'s three references updated in the same change, per §1: they are downstream, never a second authority. 2278 tests pass. Refs: CLOUD-437, CLOUD-122, CLOUD-418, CLOUD-898, CLOUD-1027 --- .claude/rules/toolchain.md | 10 +- batten.toml | 16 ++ crates/batten/src/config.rs | 1 + crates/batten/src/hook.rs | 123 +++++++++- crates/batten/src/lib.rs | 122 ++++++++-- crates/batten/src/rules.rs | 325 ++++++++++++++++---------- crates/batten/tests/cli.rs | 154 +++++++++++- crates/batten/tests/contract_drift.rs | 7 +- schema/batten.local.schema.json | 7 + schema/batten.schema.json | 7 + 10 files changed, 608 insertions(+), 164 deletions(-) diff --git a/.claude/rules/toolchain.md b/.claude/rules/toolchain.md index 8924efdbb..3e853dab0 100644 --- a/.claude/rules/toolchain.md +++ b/.claude/rules/toolchain.md @@ -215,7 +215,9 @@ mediating. hand-typed `/fast-forward` comment, naming the task to use instead. Decision table in `mise-tasks/gh-guard-check.sh`, gated by `mise run test:bats`. Reads (`gh pr view`/`list`/`create`, `gh pr ready`, `gh api`, `gh run view`) are not - blocked. Bypass: `BATTEN_GH_GUARD_BYPASS=1`. + blocked. Bypass: `BATTEN_GH_GUARD_BYPASS=1` — which since CLOUD-437 is the + `bypass_env` these rows DECLARE rather than the engine's global hatch, so + setting it now suppresses the `gh` lifecycle and nothing else. - **`memory-guard` is retired** (CLOUD-442), and what it denied is now the engine's protected-path gate: `.serena/memories/**` in `protected` crossed with the `[[verb]]` table, which covers the Write/Edit tools and a command's @@ -225,7 +227,9 @@ mediating. names `rename_memory` — the only route that rewrites `mem:` referrers. The table in `batten.toml` is the one authority; the corpus that used to live in `tests/memory-guard.bats` is `crates/batten/tests/mediated_verbs.rs`. There is - no `BATTEN_MEMORY_GUARD_BYPASS`: a mediated deny takes the engine's own hatch. + no `BATTEN_MEMORY_GUARD_BYPASS`: a mediated deny takes the engine's own hatch, + `BATTEN_HOOK_BYPASS` — or the row's own `bypass_env` where it declares one + (CLOUD-437). - `policy-budget` gates AGENTS.md plus anything always-loaded against a token budget — what every agent pays every turn. It is `batten policy budget`, not a shell task: the counted set and both thresholds are `[budget.instructions]` in @@ -363,7 +367,7 @@ call` with no `CLOUD-*` key **in that same paragraph** stops the lap. Two open are never judged, and a detached HEAD has no branch to key on — while an untracked-but-not-ignored file **is**, since opening a new feature file is the first edit this catches. There is no `BATTEN_CLAIM_GUARD_BYPASS`: a mediated - deny takes the engine's own hatch. + deny takes the engine's own hatch, `BATTEN_HOOK_BYPASS` (CLOUD-437). - **The three verdict-discarding shapes are the engine's** (CLOUD-443), as `batten.toml`'s `verdict-not-discarded` row — `kind = "pipeline"`. They are piping a verdict-bearing command into a pager or filter, following it with `;` diff --git a/batten.toml b/batten.toml index 8774733fa..be3a2549f 100644 --- a/batten.toml +++ b/batten.toml @@ -215,6 +215,22 @@ sha256 = "b2c822742e8cbf355ba0cb4cc690c3cd8fdc9ec1916c8148f27bd9098cb7aee4" # `reason` is required on a shape rule, unlike on a file rule: a mediated deny # reaches a model as the entire explanation, so a refusal that named only its id # would be un-actionable (CLOUD-122). The crate appends the bypass hatch. +# THESE FOUR WOULD DECLARE `bypass_env = "BATTEN_GH_GUARD_BYPASS"` AND DO NOT YET +# (CLOUD-437, deferred to CLOUD-1027). They are the ported `gh-guard`, so that +# name is TRUE of them where it was a fossil everywhere else, and declaring it +# here is behaviour-PRESERVING: the same variable already suppressed these rows +# back when it was the engine's global hatch. +# +# `config-lint` cannot see that argument. It compares `batten.toml` against +# `origin/main` and reads a new `bypass_env` on a rule as `rule-predicate-changed` +# — correctly, in its own terms, since the key makes a row suppressible. The +# admissible route is a groomed `Weakens:` clause in the Ready block plus a +# matching commit trailer, and CLOUD-437's block was groomed on 2026-08-14 without +# one. Asserting it in the change that performs it is the exact shape §8 refuses. +# +# So until that row is groomed, these four take the general `BATTEN_HOOK_BYPASS` +# like every other row. The column, the per-row lookup and the suppression all +# ship; what waits is this repository's own use of them. [[rule]] id = "gh-pr-merge" kind = "shape" diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index ab6bd2e0b..3fc8d1d30 100644 --- a/crates/batten/src/config.rs +++ b/crates/batten/src/config.rs @@ -1007,6 +1007,7 @@ fn default_rules() -> Vec { requires_key: None, reason: None, policy_url: None, + bypass_env: None, check: None, fix: None, produces: None, diff --git a/crates/batten/src/hook.rs b/crates/batten/src/hook.rs index d838004ce..d637721d6 100644 --- a/crates/batten/src/hook.rs +++ b/crates/batten/src/hook.rs @@ -34,7 +34,8 @@ //! //! **Posture: fail open.** Unreadable stdin, unparseable JSON, an envelope with //! no command — all resolve to [`Decision::Allow`]. A guard must never be the -//! reason a session cannot proceed; the escape hatch (`BATTEN_GH_GUARD_BYPASS`) +//! reason a session cannot proceed; the general escape hatch (`BATTEN_HOOK_BYPASS`, +//! or a row's own `bypass_env`) //! is honoured exactly as the shell guard honours it. Fail-open needs no care //! here beyond the returns below: §7 spends `2` on the policy verdict alone, so //! neither code a Batten failure can produce is one a host reads as a deny. @@ -2306,8 +2307,36 @@ fn cursor_specialized_input(value: &Value) -> Value { Value::Object(input) } -/// The escape hatch, named once so the boundary and the reason text agree. -pub const BYPASS_ENV: &str = "BATTEN_GH_GUARD_BYPASS"; +/// The GENERAL escape hatch: the whole-call "do not mediate this" switch, and +/// the fallback a row declaring no [`crate::rules::Rule::bypass_env`] advertises. +/// +/// **Renamed from `BATTEN_GH_GUARD_BYPASS` by CLOUD-437, and the old name was a +/// fossil rather than a description.** It was the first retiring bash guard's +/// hatch, kept when that guard was ported and then inherited by every predicate +/// the engine absorbed after it — so a `protected-mutation` deny about a Serena +/// memory told its reader to set a `gh` variable. Measured on this tree: +/// +/// ```text +/// Refused by protected-mutation: `Write` targets the protected path .serena/memories/core.md. +/// Fix: … Bypass with BATTEN_GH_GUARD_BYPASS=1. +/// ``` +/// +/// That breaks CLOUD-122's contract in a way worse than a missing pointer: an +/// operator who reads `GH_GUARD` concludes the refusal came from somewhere it did +/// not, and it reads as evidence that a `gh` guard is what is installed — the +/// exact mis-modelling this layer keeps producing. +/// +/// `BATTEN_GH_GUARD_BYPASS` survives, but only where it is TRUE: as the declared +/// `bypass_env` of the `gh`-lifecycle rows that legitimately own it. There is no +/// dual-honour window — two live names for one hatch is the second authority this +/// module's one-definition property forbids, and there is no external consumer to +/// protect. +/// +/// It stays resolved at the boundary before config loads, which is what keeps the +/// hot path free: a bypassed call must never pay a config read. Per-row hatches +/// cannot be resolved that early — the rows are not known yet — so they resolve +/// after the load, and only for rows that declare one. +pub const BYPASS_ENV: &str = "BATTEN_HOOK_BYPASS"; /// The mediated-call policy this run adjudicates against. /// @@ -2745,6 +2774,77 @@ impl Policy { && self.bundles.is_empty() && (self.verbs.is_empty() || self.protected.is_empty()) } + + /// The hatch that suppresses the row with this id, or [`BYPASS_ENV`] where + /// the row declares none or is not a `[[rule]]` row at all (CLOUD-437). + /// + /// The fallback carries both of the cases a derived gate produces: the + /// protected-mutation gate and the verb table refuse under ids that are + /// declared constants rather than rows, so there is nothing to look up and + /// the general hatch is the honest answer. That is also why this cannot + /// return an `Option` and leave the caller to decide — a deny with no hatch + /// clause is the bare "no" CLOUD-122 forbids, and an `Option` invites one. + #[must_use] + pub fn bypass_env_for(&self, rule: &str) -> &str { + self.shapes + .iter() + .find(|row| row.id == rule) + .and_then(|row| row.bypass_env.as_deref()) + .unwrap_or(BYPASS_ENV) + } + + /// This policy with every row whose declared hatch is in `set` removed + /// (CLOUD-437). + /// + /// **A row is REMOVED, never short-circuited to allow, and that is the whole + /// design.** Suppressing a refusal post-hoc would stop adjudication at the + /// row that fired, so setting one row's hatch would silently switch off every + /// row behind it — which is exactly the invisible blast radius a single + /// global hatch has and this column exists to end. The bash guards this + /// surface inherited were separate programs, so suppressing `memory-guard` + /// left `ready-guard` live; removing the row is what reproduces that. + /// + /// Cheap when irrelevant (house-style §4): the caller skips this entirely + /// when nothing is set, so the ordinary mediated call — every call, on every + /// tool use — clones nothing. + #[must_use] + pub fn without_hatched(&self, set: &std::collections::BTreeSet) -> Policy { + Policy { + shapes: self + .shapes + .iter() + .filter(|row| { + row.bypass_env + .as_deref() + .is_none_or(|hatch| !set.contains(hatch)) + }) + .cloned() + .collect(), + harness: self.harness, + fail_on_warning: self.fail_on_warning, + verbs: self.verbs.clone(), + protected: self.protected.clone(), + redirects: self.redirects.clone(), + facts: self.facts.clone(), + bundles: self.bundles.clone(), + } + } + + /// Every distinct hatch the loaded rows declare, for the boundary to resolve + /// against the environment (CLOUD-437). + /// + /// The boundary cannot read these before config loads — the rows are not + /// known yet — so [`BYPASS_ENV`] keeps its early position as the whole-call + /// switch and these are read after. Returning the names rather than reading + /// them here is what keeps [`adjudicate`] pure: this module never touches the + /// environment, and the caller hands back which of these were set. + #[must_use] + pub fn declared_hatches(&self) -> std::collections::BTreeSet<&str> { + self.shapes + .iter() + .filter_map(|row| row.bypass_env.as_deref()) + .collect() + } } /// Adjudicate an envelope against the policy, then apply the waivers. @@ -3060,9 +3160,19 @@ fn adjudicated(policy: &Policy, envelope: &Envelope, facts: &Facts<'_>) -> Decis /// the payload: the escape hatch. `check`'s refusal of a rule it cannot honestly /// run carries the same [`Refusal`] and no hatch, which is correct — there is /// nothing to bypass in a read-only run. +/// +/// `hatch` is the name **this** refusal's row declared, or [`BYPASS_ENV`] where +/// it declared none (CLOUD-437). Passed in rather than read off the [`Refusal`] +/// deliberately: the hatch is a fact about mediation and the refusal is shared +/// with `check`, so putting it on the payload would give a read-only finding a +/// field that can only ever be meaningless there. [`Policy::bypass_env_for`] is +/// the one place the row is looked up, so the string a deny prints and the +/// variable the boundary reads stay one definition — the property this module's +/// single-constant comment has always claimed and could not keep once the hatch +/// stopped being single. #[must_use] -pub fn deny_text(refusal: &Refusal) -> String { - format!("{} Bypass with {BYPASS_ENV}=1.", refusal.render()) +pub fn deny_text(refusal: &Refusal, hatch: &str) -> String { + format!("{} Bypass with {hatch}=1.", refusal.render()) } /// The first shape row that matches the mediated command, in declaration order. @@ -5735,6 +5845,7 @@ mod tests { requires_key: None, reason: Some(format!("use the sanctioned path for {id}")), policy_url: None, + bypass_env: None, check: None, fix: None, produces: None, @@ -6373,7 +6484,7 @@ mod tests { /// fail here. fn denial_text(decision: Decision) -> String { match decision { - Decision::Deny(refusal) => deny_text(&refusal), + Decision::Deny(refusal) => deny_text(&refusal, BYPASS_ENV), // An `Ask` is not a deny, and collapsing the two here would let a // row that silently started escalating keep passing every assertion // below about what a refusal says. A `Waived` is not one either, and diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 6ceab3b0c..f127892d5 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -1626,6 +1626,44 @@ impl SuiteReport { /// A [`UsageError`] (exit `1`) when the config will not resolve or a registered /// module will not load — both refused by [`policy::load`] with their own /// message, at the boundary where a config fault belongs. +/// The tree input one policy suite runs against. +/// +/// Split out of [`run_policy_test`] so that function reads as a loop over rows; +/// what it holds is the pair of deliberate omissions, which are easier to see +/// stated together than buried in an argument list. +/// +/// **A SUITE RUNS AGAINST NO PRODUCED RECORD** (CLOUD-851). A module's tests must +/// decide the same way on every machine, and the sink store is per-checkout state +/// that differs between them — so a test that wants a baseline supplies it with +/// `with input as`, the same way a mediated-call suite supplies its call. +/// +/// **AND AGAINST NO GIT FACT**, for exactly the same reason (CLOUD-907). HEAD, the +/// branch, the remotes and whether a ref resolves are all per-checkout state: a +/// suite that read them would pass on the author's machine and fail in CI's +/// detached, remote-less clone, which is a test asserting its environment rather +/// than its module. +fn suite_input( + rule: &rules::Rule, + documents: &std::collections::BTreeMap<(String, rules::Wanted), rules::Acquired>, + tracked: &[String], +) -> Result<(String, Vec<(String, rules::NotAcquired)>)> { + // A mediated-call row declares no documents, so its tests run against `{}` + // and supply their own input with `with input as` — OPA and Conftest's own + // shape, and the reason neither surface needs a fixture key. + Ok(rules::tree_document( + documents, + &rules::Declared { + documents: &rules::declared_documents(rule, tracked)?, + lines: &rules::declared_lines(rule, tracked)?, + invocations: &rules::declared_invocations(rule, tracked)?, + uses: &rules::declared_uses(rule, tracked)?, + }, + tracked, + &std::collections::BTreeMap::new(), + &git::GitFacts::default(), + )) +} + fn run_policy_test(json: bool, overrides: &Overrides, out: &mut dyn Write) -> Result { let root = Path::new("."); let config = resolve::resolve(root, overrides)?; @@ -1673,28 +1711,7 @@ fn run_policy_test(json: bool, overrides: &Overrides, out: &mut dyn Write) -> Re // mediated-call row declares no documents, so its tests run against `{}` // and supply their own input with `with input as` — OPA and Conftest's // own shape, and the reason neither surface needs a fixture key. - let declared = rules::declared_documents(rule, &tracked)?; - let (input, not_acquired) = rules::tree_document( - &documents, - &declared, - &rules::declared_lines(rule, &tracked)?, - &rules::declared_invocations(rule, &tracked)?, - &rules::declared_uses(rule, &tracked)?, - &tracked, - // A SUITE RUNS AGAINST NO PRODUCED RECORD, deliberately (CLOUD-851). - // A module's tests must decide the same way on every machine, and the - // sink store is per-checkout state that differs between them — so a - // test that wants a baseline supplies it with `with input as`, the - // same way a mediated-call suite supplies its call. - &std::collections::BTreeMap::new(), - // AND AGAINST NO GIT FACT, for exactly the same reason (CLOUD-907). - // HEAD, the branch, the remotes and whether a ref resolves are all - // per-checkout state: a suite that read them would pass on the - // author's machine and fail in CI's detached, remote-less clone, - // which is a test asserting its environment rather than its module. - // A suite that wants a git fact supplies it with `with input as`. - &git::GitFacts::default(), - ); + let (input, not_acquired) = suite_input(rule, &documents, &tracked)?; if !not_acquired.is_empty() { // Pointer-only (rule 4): the PATH and its stated cause, never a byte // of the document. The cause is CLOUD-849's — before it, all four @@ -2128,6 +2145,33 @@ fn receipt_facts( verdicts.map_or(facts::Look::CouldNotLook, facts::Look::Is) } +/// Drop every row whose declared `bypass_env` is set in this process's +/// environment (CLOUD-437). +/// +/// The environment read lives here rather than in [`hook`] for the reason every +/// other fact does: `adjudicate` is contractually pure, so the boundary looks and +/// the core decides. [`hook::Policy::declared_hatches`] hands back the names and +/// this answers which of them are set. +/// +/// **Cheap when irrelevant** (house-style §4). A ruleset declaring no +/// `bypass_env` — every ruleset until one opts in — collects an empty set, reads +/// no environment variable, and clones no policy. This runs on the hottest path +/// in the binary, so that is a requirement rather than a nicety. +fn without_set_hatches(policy: hook::Policy) -> hook::Policy { + let set: std::collections::BTreeSet = policy + .declared_hatches() + .into_iter() + .filter(|name| std::env::var_os(name).is_some_and(|value| !value.is_empty())) + .map(std::borrow::ToOwned::to_owned) + .collect(); + if set.is_empty() { + return policy; + } + // The row is REMOVED rather than its refusal suppressed, so the rows behind + // it still fire. See `hook::Policy::without_hatched`. + policy.without_hatched(&set) +} + fn run_hook( harness: hook::Harness, mode: Mode, @@ -2268,6 +2312,18 @@ fn run_hook( } else { load_policy(overrides, harness)? }; + // THE PER-ROW HATCHES (CLOUD-437), resolved here and nowhere earlier. + // + // `BYPASS_ENV` is read before the config load above, because a bypassed call + // must never pay for one. These cannot be: which hatches exist is a property + // of the loaded rows, so the read has to follow the load. That ordering is + // the whole reason the general hatch survives as a separate switch rather + // than becoming just another row's name. + // + // Cheap when irrelevant (§4): a ruleset declaring no `bypass_env` — every + // ruleset until one opts in — collects an empty set, reads no environment, + // and clones no policy. + let policy = without_set_hatches(policy); // The receipt facts, resolved HERE because `adjudicate` is contractually // pure — no I/O, no environment, no clock — and a receipt predicate reads a // file and two git refs. Same split the bypass hatch already uses: the @@ -2394,7 +2450,22 @@ fn run_hook( emit_advisory(harness, &envelope, out, err, &advice.join("\n\n"))?; } let decision = handled.unwrap_or_else(|| hook::adjudicate(&policy, &envelope, &facts)); - render(harness, &envelope, decision, mode, out, err) + // Resolved HERE rather than inside `render`, because `render` deliberately + // cannot see the policy (CLOUD-898) and that property is worth more than the + // convenience: a renderer that cannot see the inputs cannot re-decide by + // accident. A hatch NAME is not such an input — it is a string the renderer + // prints and never branches on — so handing it over costs nothing. + let hatch = match &decision { + hook::Decision::Deny(refusal) | hook::Decision::Ask(refusal) => { + policy.bypass_env_for(refusal.rule()).to_owned() + } + // Nothing is being refused, so nothing advertises a hatch. The general + // name stands in rather than an empty string: the value is unread on + // these arms, and a blank one would render as `Bypass with =1.` if a + // later arm ever did read it. + _ => hook::BYPASS_ENV.to_owned(), + }; + render(harness, &envelope, decision, &hatch, mode, out, err) } /// Fill the advisory buffer from the two producers that ride a batch boundary. @@ -3483,6 +3554,7 @@ fn render( harness: hook::Harness, envelope: &hook::Envelope, decision: hook::Decision, + hatch: &str, mode: Mode, out: &mut dyn Write, err: &mut dyn Write, @@ -3526,7 +3598,7 @@ fn render( // and the stderr line cannot disagree about what the refusal said or // whether it named a fix. hook::Decision::Deny(refusal) => { - let reason = hook::deny_text(&refusal); + let reason = hook::deny_text(&refusal, hatch); match hook::encode_deny(harness, &envelope.raw_event, &reason)? { Some(body) => { writeln!(out, "{body}")?; @@ -3545,7 +3617,7 @@ fn render( // degrading to "go ahead" is the one direction that inverts the policy, // and it is why `encode_ask`'s `None` means refuse rather than proceed. hook::Decision::Ask(refusal) => { - let reason = hook::deny_text(&refusal); + let reason = hook::deny_text(&refusal, hatch); match hook::encode_ask(harness, &envelope.raw_event, &reason)? { Some(body) => { writeln!(out, "{body}")?; diff --git a/crates/batten/src/rules.rs b/crates/batten/src/rules.rs index 9e0cacb4c..0b06be8bf 100644 --- a/crates/batten/src/rules.rs +++ b/crates/batten/src/rules.rs @@ -72,6 +72,7 @@ const PIPELINE_PERMITS: &[&str] = &[ "substitutes", "reason", "policy_url", + "bypass_env", "severity", ]; @@ -162,6 +163,7 @@ const SHAPE_PERMITS: &[&str] = &[ "requires_key", "base", "policy_url", + "bypass_env", "severity", ]; @@ -183,6 +185,7 @@ const RECEIPT_PERMITS: &[&str] = &[ "reason", "contains", "policy_url", + "bypass_env", "severity", ]; @@ -787,6 +790,14 @@ impl RuleKind { "identity_key", "reason", "policy_url", + // Permitted because this kind takes BOTH scopes: a + // `mediated-call` policy row denies through the same channel a + // `shape` row does, so it owes the same findable hatch. On a + // `tree`-scoped row it names nothing and costs nothing — + // `check` is read-only and has no mediation to suppress, which + // is the same reason `deny_text` is `hook`'s and not + // `Refusal`'s. + "bypass_env", "no_fix_reason", ], } @@ -1363,6 +1374,28 @@ pub struct Rule { /// An optional link to the policy this rule enforces, appended to the deny. #[serde(default, skip_serializing_if = "Option::is_none")] pub policy_url: Option, + /// The environment variable that suppresses **this row**, named in this + /// row's own deny (CLOUD-437). + /// + /// Absent means the general hatch, [`crate::hook::BYPASS_ENV`]. That default + /// is not a hedge — it is what keeps the guarantee true as rows are added: + /// per-row with no fallback would leave the next row hatchless and silent, + /// which is the failure that produced this column. A row declaring one is + /// declaring that its bypass is a *separate decision* from every other row's. + /// + /// Why a per-row column rather than one global name: the bash guards this + /// surface inherited each had their own hatch, so an operator could suppress + /// `memory-guard` alone while `ready-guard` stayed live. One global name + /// silently widens the blast radius of every bypass — invisibly, because the + /// deny text cannot say what else it just switched off. + /// + /// It is an environment variable rather than a `batten.local.toml` key on + /// purpose. A hatch LOWERS policy, and house-style §8's override channel is + /// raise-only; an env var is a per-invocation decision, visible in the + /// command that took it and gone when the process exits, where a config key + /// would make the same lowering persistent and invisible. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub bypass_env: Option, /// The **inspection-only** command template a [`RuleKind::Command`] rule /// runs as its gate. Required by that kind, rejected by any other. /// @@ -2964,7 +2997,7 @@ impl Rule { /// about all of them makes that failure impossible, and /// [`tests::every_optional_rule_field_is_classified_by_every_kind`] fails if /// a column is added here without being placed. - fn columns(&self) -> [(&'static str, bool); 47] { + fn columns(&self) -> [(&'static str, bool); 48] { [ // In the census because it is now per-kind, which is what makes // "required by every kind but the judge" a fact the existing @@ -2994,6 +3027,7 @@ impl Rule { ("requires_key", self.requires_key.is_some()), ("reason", self.reason.is_some()), ("policy_url", self.policy_url.is_some()), + ("bypass_env", self.bypass_env.is_some()), ("verbatim", self.verbatim.is_some()), ("identity_key", self.identity_key.is_some()), ("direction", self.direction.is_some()), @@ -5018,112 +5052,82 @@ fn git_facts(rules: &[Rule], root: &Path) -> crate::git::GitFacts { } } -pub(crate) fn tree_document( +/// One path's projection for one fact family: the value, or why it could not be +/// looked at. +/// +/// Extracted from [`tree_document`], where the identical shape was written four +/// times — once per family — and each copy had to remember that a path acquired +/// as a DIFFERENT projection is `Absent` rather than an empty answer. That is +/// the vacuous pass CLOUD-251 names, and four chances to get it wrong is three +/// too many. +/// +/// The `wanted` and the [`Acquired`] variant must agree, and the mismatch arm is +/// what enforces it: a path in the cache under another projection is not this +/// family's answer, and reporting it as an empty one would be a module finding +/// nothing and reporting clean. +fn project_declared( cache: &BTreeMap<(String, Wanted), Acquired>, - documents: &[String], - lines: &[String], - // The Rust files any row declared as call sites (CLOUD-914). A separate - // list for `lines`' reason: the cache holds one answer per path, so which - // projection a path belongs to is the caller's declaration and not a guess. - invocations: &[String], - // The Rust files any row declared as a `use` graph (CLOUD-762). - uses: &[String], - tracked: &[String], - // What earlier runs produced, acquired once at the boundary (CLOUD-851). - // Handed in rather than read here for `Fact::Produced`'s whole reason: the - // projection is pure, and the read that fills this map is the caller's. - produced: &BTreeMap, - // The git fact family, acquired once at the boundary and only for what the - // ruleset DECLARED (CLOUD-907). Handed in for `produced`'s reason: the - // projection is pure, and the reads that fill this are the caller's. - git: &crate::git::GitFacts, -) -> (String, Vec<(String, NotAcquired)>) { - let mut produced_records: serde_json::Map = produced - .iter() - .map(|(key, record)| (key.clone(), serde_json::json!(record))) - .collect(); - let mut parsed = serde_json::Map::new(); - let mut read_lines = serde_json::Map::new(); - let mut call_sites = serde_json::Map::new(); - let mut use_edges = serde_json::Map::new(); - let mut missing = Vec::new(); - // The same set as `missing`, carrying WHY (CLOUD-845). `missing` stays a - // bare path list in the document because that is what a module reads; the - // cause is the caller's, so a skip can name its reason instead of being - // anonymous. - let mut causes: Vec<(String, NotAcquired)> = Vec::new(); - for path in documents { - // READ FROM THE SHARED CACHE (CLOUD-850), which `acquire_declared` - // filled once for the whole run through the one acquisition - // (CLOUD-849). A path absent from the cache is a caller that did not - // declare it, which is could-not-look rather than a silent empty. - match cache.get(&(path.clone(), Wanted::Document)) { - Some(Acquired::Parsed(node)) => { - parsed.insert(path.clone(), node.to_json()); - } - // Still `missing` — a module reads it as could-not-look exactly as - // before, so no consumer's predicate changes shape. What is new is - // that the CAUSE survives for the caller, which is what CLOUD-845's - // exit-1 refusal attaches to. - Some(Acquired::No(why)) => { - missing.push(path.clone()); - causes.push((path.clone(), *why)); - } - // Acquired as LINES by another row's declaration, or not declared. - // Neither is a parsed document, and neither is an empty one. - Some(Acquired::Lines(_) | Acquired::Invocations(_) | Acquired::Uses(_)) | None => { - missing.push(path.clone()); - causes.push((path.clone(), NotAcquired::Absent)); - } - } - } - for path in lines { - match cache.get(&(path.clone(), Wanted::Lines)) { - Some(Acquired::Lines(text)) => { - read_lines.insert(path.clone(), serde_json::json!(text)); - } - // COULD NOT LOOK, NEVER AN EMPTY ARRAY (CLOUD-846 §2). A file - // nobody could read and a file with no matching line are different - // answers, and collapsing them is CLOUD-251's vacuous pass: the - // predicate would find nothing and report clean. - Some(Acquired::No(why)) => { - missing.push(path.clone()); - causes.push((path.clone(), *why)); - } - // A path acquired as something else, or not declared. Neither is a - // readable line set, and neither is an empty one. - Some(Acquired::Parsed(_) | Acquired::Invocations(_) | Acquired::Uses(_)) | None => { - missing.push(path.clone()); - causes.push((path.clone(), NotAcquired::Absent)); - } - } - } - for path in invocations { - match cache.get(&(path.clone(), Wanted::Invocations)) { - Some(Acquired::Invocations(sites)) => { - call_sites.insert(path.clone(), serde_json::json!(sites)); - } - // COULD NOT LOOK, NEVER AN EMPTY ARRAY, and here the parser is the - // one that can say so: `NotAcquired::Unparsed` is a file whose bytes - // read fine and whose grammar did not. CLOUD-310 measured a matcher - // emitting zero nodes and zero errors over a partially-parsed file, - // and made a parse-coverage assertion the price of embedding one. - Some(Acquired::No(why)) => { - missing.push(path.clone()); - causes.push((path.clone(), *why)); - } - // Acquired as something else, or not declared. Neither is a call-site - // set, and neither is an empty one. - Some(Acquired::Parsed(_) | Acquired::Lines(_) | Acquired::Uses(_)) | None => { - missing.push(path.clone()); - causes.push((path.clone(), NotAcquired::Absent)); - } - } + path: &str, + wanted: Wanted, +) -> Result { + match cache.get(&(path.to_owned(), wanted)) { + Some(Acquired::Parsed(node)) if wanted == Wanted::Document => Ok(node.to_json()), + Some(Acquired::Lines(text)) if wanted == Wanted::Lines => Ok(serde_json::json!(text)), + // COULD NOT LOOK, NEVER AN EMPTY ARRAY, and for call sites the parser is + // the one that can say so: `NotAcquired::Unparsed` is a file whose bytes + // read fine and whose grammar did not. CLOUD-310 measured a matcher + // emitting zero nodes and zero errors over a partially-parsed file, and + // made a parse-coverage assertion the price of embedding one. + Some(Acquired::Invocations(sites)) if wanted == Wanted::Invocations => { + Ok(serde_json::json!(sites)) + } + // The `use` family is handled by its caller rather than here, because an + // edge set is completed against the crate root's re-export table and that + // table is one value for the whole declared set — a per-path projection + // cannot see it. + Some(Acquired::No(why)) => Err(*why), + // Acquired under a different projection, or never declared. Neither is + // this family's answer and neither is an empty one. + _ => Err(NotAcquired::Absent), } - // RESOLUTION IS A POST-PASS OVER THE DECLARED SET (CLOUD-762), and it runs - // before the projection because every edge in every file is completed - // against ONE table. - // +} + +/// The paths each row DECLARED, one list per projection (CLOUD-914, CLOUD-762). +/// +/// Four lists rather than one, for the reason the cache is keyed on a pair: a +/// path can be acquired as more than one fact, so which projection it belongs to +/// is the caller's declaration and never a guess from the extension. +/// +/// Grouped into a struct rather than passed as four parameters because they are +/// one thing — the declared set — and splitting them across the signature is +/// what pushed this function past the argument ceiling when the third and fourth +/// arrived. +pub(crate) struct Declared<'a> { + /// Files a row declared as parsed documents. + pub documents: &'a [String], + /// Files a row declared as line sets. + pub lines: &'a [String], + /// Rust files a row declared as call sites. + pub invocations: &'a [String], + /// Rust files a row declared as a `use` graph. + pub uses: &'a [String], +} + +/// Project the `use` family, resolving every edge against ONE re-export table. +/// +/// Separate from [`project_declared`] rather than folded into it, and the reason +/// is the whole of CLOUD-762's finding: an edge naming a crate-root item cannot +/// be completed from the file it appears in. The root's own `mod` declarations +/// are what disambiguate `use error::X` (a module) from `use anyhow::X` (a +/// crate), so the table is one value for the entire declared set and a per-path +/// projection structurally cannot see it. +fn project_uses( + cache: &BTreeMap<(String, Wanted), Acquired>, + uses: &[String], + out: &mut serde_json::Map, + missing: &mut Vec, + causes: &mut Vec<(String, NotAcquired)>, +) { // The root is named by Rust's own convention — a library crate's root is // `lib.rs` — which is a language fact and not a consumer identifier, so // non-negotiable rule 1 is untouched: nothing here names a repository, an @@ -5146,7 +5150,7 @@ pub(crate) fn tree_document( Some(Acquired::Uses(facts)) => { let mut edges = facts.edges.clone(); crate::uses::resolve(&mut edges, &root_table); - use_edges.insert(path.clone(), serde_json::json!(edges)); + out.insert(path.clone(), serde_json::json!(edges)); } // COULD NOT LOOK, NEVER AN EMPTY EDGE SET. A layering gate whose // corpus failed to parse would otherwise report clean, which is @@ -5161,6 +5165,69 @@ pub(crate) fn tree_document( } } } +} + +pub(crate) fn tree_document( + cache: &BTreeMap<(String, Wanted), Acquired>, + declared: &Declared<'_>, + tracked: &[String], + // What earlier runs produced, acquired once at the boundary (CLOUD-851). + // Handed in rather than read here for `Fact::Produced`'s whole reason: the + // projection is pure, and the read that fills this map is the caller's. + produced: &BTreeMap, + // The git fact family, acquired once at the boundary and only for what the + // ruleset DECLARED (CLOUD-907). Handed in for `produced`'s reason: the + // projection is pure, and the reads that fill this are the caller's. + git: &crate::git::GitFacts, +) -> (String, Vec<(String, NotAcquired)>) { + let Declared { + documents, + lines, + invocations, + uses, + } = *declared; + let mut produced_records: serde_json::Map = produced + .iter() + .map(|(key, record)| (key.clone(), serde_json::json!(record))) + .collect(); + let mut parsed = serde_json::Map::new(); + let mut read_lines = serde_json::Map::new(); + let mut call_sites = serde_json::Map::new(); + let mut use_edges = serde_json::Map::new(); + let mut missing = Vec::new(); + // The same set as `missing`, carrying WHY (CLOUD-845). `missing` stays a + // bare path list in the document because that is what a module reads; the + // cause is the caller's, so a skip can name its reason instead of being + // anonymous. + let mut causes: Vec<(String, NotAcquired)> = Vec::new(); + // ONE LOOP OVER THREE FAMILIES, driven by a table rather than written out + // three times. Each family names the projection it wants and the map it + // fills; `project_declared` owns the could-not-look distinction that all + // three share, so there is one place for it to be right rather than three. + // + // `uses` is deliberately NOT in this table: its edges are completed against + // the crate root's re-export table below, which is one value for the whole + // declared set and so cannot be resolved per path. + for (paths, wanted, out) in [ + (documents, Wanted::Document, &mut parsed), + (lines, Wanted::Lines, &mut read_lines), + (invocations, Wanted::Invocations, &mut call_sites), + ] { + for path in paths { + match project_declared(cache, path, wanted) { + Ok(value) => { + out.insert(path.clone(), value); + } + Err(why) => { + missing.push(path.clone()); + causes.push((path.clone(), why)); + } + } + } + } + // The `use` family, whose edges are completed against one table for the + // whole declared set — see `project_uses`. + project_uses(cache, uses, &mut use_edges, &mut missing, &mut causes); // THE PROJECTION (CLOUD-845), on `hook::call_document`'s shape. Iterating // `Fact::ALL` rather than writing keys means a fact the tree surface gains // cannot arrive unprojected, and a key the tree emits cannot fail to name a @@ -5310,10 +5377,12 @@ fn policy_rule( } let (input, not_acquired) = tree_document( documents, - &declared, - &declared_line_paths, - &declared_invocation_paths, - &declared_use_paths, + &Declared { + documents: &declared, + lines: &declared_line_paths, + invocations: &declared_invocation_paths, + uses: &declared_use_paths, + }, tracked, produced, git, @@ -6693,11 +6762,16 @@ fn document_in_file( // cause or a new `Acquired` shape has to be decided here rather than // defaulting to silence. Two of the arms report the same reason today for // two unrelated causes, and merging them would delete the comments that say - // why each is here — so the lint is expected rather than obeyed. - #[expect( - clippy::match_same_arms, - reason = "an arm per outcome is the property; the shared reason is a coincidence of two distinct causes" - )] + // why each is here. + // + // The `#[expect(clippy::match_same_arms)]` that used to sit here is GONE + // rather than kept, and its going is the annotation working. CLOUD-914 and + // CLOUD-762 added `Invocations` and `Uses` to the acquisition outcomes, which + // regrouped this match until the lint stopped firing — and because it was + // `expect` rather than `allow`, the now-unfulfilled expectation went red + // instead of sitting here forever as a claim about a lint nobody triggers. + // If a later edit makes two arms identical again, clippy says so, which is + // the answer this attribute was standing in for. let reason = match &outcome { // The tree does not carry it. Not this row's business and not a // finding — unchanged. @@ -7604,10 +7678,12 @@ mod tests { let empty: BTreeMap<(String, super::Wanted), super::Acquired> = BTreeMap::new(); let (input, _) = super::tree_document( &empty, - &[], - &[], - &[], - &[], + &super::Declared { + documents: &[], + lines: &[], + invocations: &[], + uses: &[], + }, &[], &BTreeMap::new(), &crate::git::GitFacts::default(), @@ -8131,10 +8207,12 @@ mod tests { // files missing if a lookup asked for the wrong form. let (_, not_acquired) = super::tree_document( &cache, - &[], - &["subject.rs".to_owned()], - &["subject.rs".to_owned()], - &[], + &super::Declared { + documents: &[], + lines: &["subject.rs".to_owned()], + invocations: &["subject.rs".to_owned()], + uses: &[], + }, &files, &BTreeMap::new(), &crate::git::GitFacts::default(), @@ -8178,6 +8256,7 @@ mod tests { requires_key: None, reason: None, policy_url: None, + bypass_env: None, check: None, fix: None, produces: None, diff --git a/crates/batten/tests/cli.rs b/crates/batten/tests/cli.rs index 0e84c2503..35194946f 100644 --- a/crates/batten/tests/cli.rs +++ b/crates/batten/tests/cli.rs @@ -94,12 +94,50 @@ fn run_hook_in(dir: &std::path::Path, harness: &str, payload: &str, bypass: bool command .current_dir(dir) .args(["hook", "--harness", harness]) + .env_remove("BATTEN_HOOK_BYPASS") .env_remove("BATTEN_GH_GUARD_BYPASS") .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()); if bypass { - command.env("BATTEN_GH_GUARD_BYPASS", "1"); + command.env("BATTEN_HOOK_BYPASS", "1"); + } + let mut child = command.spawn().expect("spawn batten hook"); + child + .stdin + .take() + .expect("piped stdin") + .write_all(payload.as_bytes()) + .expect("write payload"); + child.wait_with_output().expect("run batten hook") +} + +/// `batten hook` with named environment variables set, for the per-row hatches +/// (CLOUD-437). +/// +/// [`run_hook_in`]'s `bypass` flag sets the GENERAL hatch and nothing else, which +/// cannot express "this row's hatch is set and no other's" — the distinction the +/// whole column exists to make. Both names are scrubbed first for the same reason +/// every other harness here scrubs them: `BATTEN_GH_GUARD_BYPASS` is a live row +/// hatch now, so a leaked one would disarm the four `gh` rows and their +/// assertions would pass for the wrong reason. +fn run_hook_with_env( + dir: &std::path::Path, + harness: &str, + payload: &str, + env: &[(&str, &str)], +) -> Output { + let mut command = batten(); + command + .current_dir(dir) + .args(["hook", "--harness", harness]) + .env_remove("BATTEN_HOOK_BYPASS") + .env_remove("BATTEN_GH_GUARD_BYPASS") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + for (key, value) in env { + command.env(key, value); } let mut child = command.spawn().expect("spawn batten hook"); child @@ -117,6 +155,7 @@ fn run_hook_verbose(dir: &std::path::Path, harness: &str, payload: &str) -> Outp command .current_dir(dir) .args(["-v", "hook", "--harness", harness]) + .env_remove("BATTEN_HOOK_BYPASS") .env_remove("BATTEN_GH_GUARD_BYPASS") .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -1761,6 +1800,7 @@ fn a_malformed_harness_token_fails_loud_without_denying() { for token in ["claude_code", "", "exitcode"] { let output = batten() .args(["hook", "--harness", token]) + .env_remove("BATTEN_HOOK_BYPASS") .env_remove("BATTEN_GH_GUARD_BYPASS") .stdin(Stdio::null()) .output() @@ -2014,6 +2054,7 @@ fn the_deny_is_a_function_of_config_and_argv_not_the_ambient_environment() { command .current_dir(&dir) .args(["hook", "--harness", "exit-code"]) + .env_remove("BATTEN_HOOK_BYPASS") .env_remove("BATTEN_GH_GUARD_BYPASS") .env(key, value) .stdin(Stdio::piped()) @@ -7978,6 +8019,116 @@ fn an_unknown_key_in_an_action_row_stays_a_hard_config_error() { assert_eq!(output.status.code(), Some(1), "a usage error, never a deny"); } +/// CLOUD-437's fixture: two rows, one declaring its own hatch and one not. +/// +/// The pair is the point. A suite with only a hatched row could not tell "the +/// hatch worked" from "the deny never fired", and a suite with only an unhatched +/// one could not tell the general name from a row's own. +const HATCH_POLICY_CONFIG: &str = r#"version = 1 +[[rule]] +id = "owns-its-hatch" +kind = "shape" +scope = "mediated_call" +severity = "deny" +pattern = "gh pr merge" +bypass_env = "BATTEN_GH_GUARD_BYPASS" +reason = "use `mise run land`" + +[[rule]] +id = "takes-the-general-hatch" +kind = "shape" +scope = "mediated_call" +severity = "deny" +pattern = "danger-zone" +reason = "do not" +"#; + +fn hatch_call(command: &str) -> String { + serde_json::json!({ + "hook_event_name": "PreToolUse", + "tool_name": "Bash", + "tool_input": { "command": command }, + }) + .to_string() +} + +#[test] +fn a_deny_names_the_hatch_its_own_row_declared() { + // CLOUD-437's worked case, in both directions at once: the row that owns + // `BATTEN_GH_GUARD_BYPASS` names it, and the row that does not must NOT — + // which is the defect, a deny pointing at another subsystem's variable. + let dir = repo_with_config("hatch-named", HATCH_POLICY_CONFIG); + + let owned = run_hook_with_env(&dir, "claude-code", &hatch_call("gh pr merge 42"), &[]); + let owned_text = String::from_utf8_lossy(&owned.stdout).into_owned(); + assert!( + owned_text.contains("Bypass with BATTEN_GH_GUARD_BYPASS=1."), + "the row that declared it advertises it: {owned_text}" + ); + + let general = run_hook_with_env(&dir, "claude-code", &hatch_call("danger-zone --now"), &[]); + let general_text = String::from_utf8_lossy(&general.stdout).into_owned(); + assert!( + general_text.contains("Bypass with BATTEN_HOOK_BYPASS=1."), + "a row declaring none takes the general hatch: {general_text}" + ); + assert!( + !general_text.contains("GH_GUARD"), + "no deny names a hatch belonging to a subsystem it did not come from: {general_text}" + ); +} + +#[test] +fn the_hatch_a_deny_advertises_actually_suppresses_that_deny() { + // THE LOAD-BEARING HALF (CLOUD-437 §7). A suite asserting only the string + // would pass on a hatch nothing reads, and a refusal pointing at a variable + // the boundary ignores is worse than one pointing nowhere. + let dir = repo_with_config("hatch-works", HATCH_POLICY_CONFIG); + let call = hatch_call("gh pr merge 42"); + + let denied = run_hook_with_env(&dir, "claude-code", &call, &[]); + assert!( + String::from_utf8_lossy(&denied.stdout).contains("owns-its-hatch"), + "the row denies when its hatch is unset" + ); + + let hatched = run_hook_with_env( + &dir, + "claude-code", + &call, + &[("BATTEN_GH_GUARD_BYPASS", "1")], + ); + assert!( + !String::from_utf8_lossy(&hatched.stdout).contains("owns-its-hatch"), + "setting the advertised hatch suppresses that deny: {:?}", + String::from_utf8_lossy(&hatched.stdout) + ); +} + +#[test] +fn one_rows_hatch_leaves_every_other_row_live() { + // The fidelity bar this column exists for. The bash guards were separate + // programs, so suppressing `memory-guard` left `ready-guard` live; one global + // hatch silently widens the blast radius of every bypass, invisibly, because + // the deny text cannot say what else it just switched off. + // + // Fails by: suppressing the REFUSAL post-hoc instead of removing the row — + // adjudication would stop at the hatched row and every row behind it would + // go unevaluated. + let dir = repo_with_config("hatch-narrow", HATCH_POLICY_CONFIG); + let output = run_hook_with_env( + &dir, + "claude-code", + &hatch_call("danger-zone --now"), + &[("BATTEN_GH_GUARD_BYPASS", "1")], + ); + let text = String::from_utf8_lossy(&output.stdout).into_owned(); + assert!( + text.contains("takes-the-general-hatch"), + "another row's hatch must not disarm this one: {text}" + ); +} + #[test] fn a_bypassed_call_fires_no_action() { // The bypass says "do not mediate this call"; spawning the operator's @@ -9526,6 +9677,7 @@ fn run_hook_state( command .current_dir(dir) .args(["hook", "--harness", harness]) + .env_remove("BATTEN_HOOK_BYPASS") .env_remove("BATTEN_GH_GUARD_BYPASS") .stdin(Stdio::piped()) .stdout(Stdio::piped()) diff --git a/crates/batten/tests/contract_drift.rs b/crates/batten/tests/contract_drift.rs index e589675e8..bf15afbe1 100644 --- a/crates/batten/tests/contract_drift.rs +++ b/crates/batten/tests/contract_drift.rs @@ -387,12 +387,7 @@ fn the_mediation_hatch_does_not_silence_the_advisory() { drift(&dir, "s-1"); std::fs::write(dir.join("AGENTS.md"), "# the contract\nmoved\n").unwrap(); - let output = drift_on( - &dir, - "s-1", - "PostToolBatch", - &[("BATTEN_GH_GUARD_BYPASS", "1")], - ); + let output = drift_on(&dir, "s-1", "PostToolBatch", &[("BATTEN_HOOK_BYPASS", "1")]); assert!( notice(&output).is_some_and(|text| text.contains("AGENTS.md")), "a bypassed call is unmediated, not uninformed: {:?}", diff --git a/schema/batten.local.schema.json b/schema/batten.local.schema.json index ace8852c0..ba8cb152f 100644 --- a/schema/batten.local.schema.json +++ b/schema/batten.local.schema.json @@ -444,6 +444,13 @@ "null" ] }, + "bypass_env": { + "description": "The environment variable that suppresses **this row**, named in this\nrow's own deny (CLOUD-437).\n\nAbsent means the general hatch, [`crate::hook::BYPASS_ENV`]. That default\nis not a hedge — it is what keeps the guarantee true as rows are added:\nper-row with no fallback would leave the next row hatchless and silent,\nwhich is the failure that produced this column. A row declaring one is\ndeclaring that its bypass is a *separate decision* from every other row's.\n\nWhy a per-row column rather than one global name: the bash guards this\nsurface inherited each had their own hatch, so an operator could suppress\n`memory-guard` alone while `ready-guard` stayed live. One global name\nsilently widens the blast radius of every bypass — invisibly, because the\ndeny text cannot say what else it just switched off.\n\nIt is an environment variable rather than a `batten.local.toml` key on\npurpose. A hatch LOWERS policy, and house-style §8's override channel is\nraise-only; an env var is a per-invocation decision, visible in the\ncommand that took it and gone when the process exits, where a config key\nwould make the same lowering persistent and invisible.", + "type": [ + "string", + "null" + ] + }, "check": { "description": "The **inspection-only** command template a [`RuleKind::Command`] rule\nruns as its gate. Required by that kind, rejected by any other.\n\nSplit on whitespace into `program` plus arguments and executed\n**directly — never through a shell**, so what runs is exactly what a\nreviewer reads (§9: rules \"name a command already on the operator's\nPATH\"). A bare [`FILES_PLACEHOLDER`] argument expands in place to the\nmatched paths; omit it and the command self-discovers.\n\nNamed for the half it is, not for the act of running: §9's duality has\ntwo commands, and a column called `run` could name either. Enforcement\nis always this one.", "type": [ diff --git a/schema/batten.schema.json b/schema/batten.schema.json index 9b4e9941e..c24d03c7c 100644 --- a/schema/batten.schema.json +++ b/schema/batten.schema.json @@ -1454,6 +1454,13 @@ "null" ] }, + "bypass_env": { + "description": "The environment variable that suppresses **this row**, named in this\nrow's own deny (CLOUD-437).\n\nAbsent means the general hatch, [`crate::hook::BYPASS_ENV`]. That default\nis not a hedge — it is what keeps the guarantee true as rows are added:\nper-row with no fallback would leave the next row hatchless and silent,\nwhich is the failure that produced this column. A row declaring one is\ndeclaring that its bypass is a *separate decision* from every other row's.\n\nWhy a per-row column rather than one global name: the bash guards this\nsurface inherited each had their own hatch, so an operator could suppress\n`memory-guard` alone while `ready-guard` stayed live. One global name\nsilently widens the blast radius of every bypass — invisibly, because the\ndeny text cannot say what else it just switched off.\n\nIt is an environment variable rather than a `batten.local.toml` key on\npurpose. A hatch LOWERS policy, and house-style §8's override channel is\nraise-only; an env var is a per-invocation decision, visible in the\ncommand that took it and gone when the process exits, where a config key\nwould make the same lowering persistent and invisible.", + "type": [ + "string", + "null" + ] + }, "check": { "description": "The **inspection-only** command template a [`RuleKind::Command`] rule\nruns as its gate. Required by that kind, rejected by any other.\n\nSplit on whitespace into `program` plus arguments and executed\n**directly — never through a shell**, so what runs is exactly what a\nreviewer reads (§9: rules \"name a command already on the operator's\nPATH\"). A bare [`FILES_PLACEHOLDER`] argument expands in place to the\nmatched paths; omit it and the command self-discovers.\n\nNamed for the half it is, not for the act of running: §9's duality has\ntwo commands, and a column called `run` could name either. Enforcement\nis always this one.", "type": [