From b51f26283d14caa8365193524868957b0bcc4558 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sat, 22 Aug 2026 04:24:23 +0000 Subject: [PATCH 1/7] feat(policy): give a module the matching vocabulary `forbid` already had MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLOUD-283 landed a `regex` column on `forbid` — "the escape for a predicate that genuinely is a shape". A `policy` module could not use one, because `regorus` is pinned `default-features = false` with only ast/std/arc/coverage. So the surface CLOUD-843 migrates 85 bash gates ONTO was strictly weaker at matching than the rule kind sitting beside it, while the language it retires is `grep -E`/`sed -E`/`awk` throughout. `policy/run-shape.rego` already paid that bill in ~90 lines of hand-rolled heredoc scanning and quote scrubbing, its own header naming the cause. Measured the way `macos-link-check`'s own exemption was — added as a real dependency, gates run, result recorded: regex + semver + time macos-link-check EXIT 1 core-foundation-sys v0.8.7: links an Apple framework regex + semver macos-link-check EXIT 0 evaluator-closure-check EXIT 0, 41 -> 46 activated, none of the nine IO-bearing crates present `glob` stays shut by construction rather than by preference: it is `dep:globset`, and `globset` is one of the nine names `evaluator-closure-check` decides on. A module needing path matching takes it from the row's `sources` glob, resolved by the engine outside the module. `time` is deferred with a predicate rather than attempted and abandoned. It is `dep:chrono` + `dep:chrono-tz`, and regorus declares both with `default-features = true` in every published version carrying the feature; chrono's default `clock` reaches `iana-time-zone` and thence `core-foundation-sys` on Apple targets. Cargo's feature unification is ADDITIVE, so a direct `chrono = { default-features = false }` here would not subtract what regorus requested — this is not fixable from our manifest. THE FEATURE IS LOAD-BEARING, NOT DECLARED (CLOUD-845's shape, applied to a Cargo feature). `run-shape.rego`'s short-cluster rule asked `contains` over the flag's tail, which cannot say "letters": `-x=mfoo` carries an `m`, so a commit that still blocks on $EDITOR was allowed through. `regex.match` over an anchored class is the predicate that rule's own comment already claimed. Its `#MUTANT` directive moves with it, and the new `test_` case is the discriminating one — a suite covering only `-m` and `-am` passes under both spellings. Refs: CLOUD-885 --- Cargo.toml | 23 +++++++++++++++++++++++ policy/run-shape.rego | 28 +++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 82d7a351e..f74a8612f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -241,6 +241,29 @@ gix = { version = "0.86", default-features = false, features = [ # code, in the harder place to review, to avoid a feature that costs a lockfile # edge. The first tree-scoped preset was written string-only and hit the wall on # the second rule, which is when this was reconsidered. +# +# `glob` IS REFUSED BY CONSTRUCTION, not by preference, and stating it here is +# what stops it being re-litigated as an oversight: `glob = ["dep:globset"]`, and +# `globset` is one of the nine names `evaluator-closure-check` decides on. The +# gate that defends the security claim already answers this. A module needing +# path matching takes it from the row's `sources` glob (CLOUD-850), resolved by +# the engine outside the module — which is the better shape anyway, since the +# engine bounds acquisition by declaration. +# +# `time` IS DEFERRED, AND THE BLOCKER IS UPSTREAM'S DECLARATION rather than this +# pin (CLOUD-885). `time = ["dep:chrono", "dep:chrono-tz"]` and regorus declares +# both with `default-features = true` — every published version carrying the +# feature does, checked against the index. chrono's default `clock` reaches +# `iana-time-zone`, which on Apple targets reaches `core-foundation-sys`, which +# `macos-link-check`'s `FRAMEWORK_CRATES` names; enabling it fails that gate at +# exit 1 on exactly that crate. Cargo's feature unification is ADDITIVE, so a +# direct `chrono = { default-features = false }` here would not subtract what +# regorus requested — this is not fixable from our manifest. CLOUD-885 carries +# the re-open predicate as a command. +# +# `semver` IS NOT HERE, on the same rule (CLOUD-885): no module calls it, and a +# feature nothing calls is `input.tree.tracked` documented and never built +# (CLOUD-845). It lands with its first consumer. regorus = { version = "0.11", default-features = false, features = [ "ast", "std", diff --git a/policy/run-shape.rego b/policy/run-shape.rego index 68fcfcba6..f5f577d5a 100644 --- a/policy/run-shape.rego +++ b/policy/run-shape.rego @@ -28,7 +28,7 @@ rules contains "commit-names-no-message-source" # the SCRUBBING and the SPLITTING rather than the flag table, which is where a # raw-string module goes quietly wrong. `@` delimits each sed script because the # rows themselves are `|`-separated. -#MUTANT message-flag-unchecked|s@some c in {"m", "F", "C", "c"}@some c in {"ZZZZ"}@|every form that CAN obtain a message stays allowed +#MUTANT message-flag-unchecked|s@\[mFCc\]@[Z]@|every form that CAN obtain a message stays allowed #MUTANT list-not-split|s@^elements :=.*@elements := [scrubbed]@|a compound list is judged per element #MUTANT heredoc-body-judged|s@ j < i@ j < -1@|a git commit inside a heredoc body is prose #MUTANT double-quoted-span-judged|s@^scrubbed := quoted_out(single_scrubbed.*@scrubbed := single_scrubbed@|a quoted span carrying a list separator is not a list @@ -191,12 +191,16 @@ names_a_message_source(stage) if { # A short cluster — `-m`, `-am`, `-F`, `-C`, `-c`: one `-`, then letters, at # least one of which selects a message source. +# +# `regex.match` RATHER THAN `contains`, and the difference is a verdict rather +# than a spelling (CLOUD-885). The predicate is "a cluster of LETTERS, one of +# which is a message flag", and `contains` over the tail cannot say "letters": +# `-x=mfoo` carries an `m` and read as naming a message source, so a commit that +# will still block on $EDITOR was allowed through. The anchored class is the +# predicate the comment above already claimed. names_a_message_source(stage) if { some t in tokens(stage) - startswith(t, "-") - not startswith(t, "--") - some c in {"m", "F", "C", "c"} - contains(substring(t, 1, -1), c) + regex.match(`^-[A-Za-z]*[mFCc]`, t) } # --------------------------------------------------------------------------- @@ -223,3 +227,17 @@ test_a_later_element_is_judged_too if { test_another_tool_is_not_judged if { count(violation) == 0 with input as {"call": {"command": "hg commit"}} } + +test_a_short_cluster_names_a_message_source if { + count(violation) == 0 with input as {"call": {"command": "git commit -am x"}} +} + +# THE DISCRIMINATING CASE for the `regex.match` above (CLOUD-885). `-x=mfoo` is +# not a flag cluster — it carries an `m`, which is all the previous `contains` +# over the tail could see, so a commit that still blocks on $EDITOR was allowed. +# A test that only covered `-m` and `-am` passes under both spellings and proves +# nothing about the change. +test_a_non_cluster_carrying_m_is_not_a_message_source if { + some v in violation with input as {"call": {"command": "git commit -x=mfoo"}} + v.rule == "commit-names-no-message-source" +} From 7450daa17b601318e856a55437b85feadb09e73b Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sat, 22 Aug 2026 04:30:30 +0000 Subject: [PATCH 2/7] test(run-shape): the negative control for the flag-cluster anchor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `batten policy test` is established as insufficient evidence (CLOUD-845): a `with input as` fabricates its own input, so a module can pass its own suite green over a shape the engine never produces. The control that counts drives the compiled binary through `batten hook` — the same door a mediated call comes through — and this case was missing for the predicate CLOUD-885 changed. THE DISCRIMINATING CASE, not another `-m`. The suite already covered `-m`, `-am`, `-F`, `-C`, `--message=`, `--amend --no-edit` and `--fixup`, and every one of them passes under BOTH the old `contains` spelling and the new anchored `regex.match`. None of them could have caught the defect. `-x=mfoo` is not a flag cluster at all — it carries an `m`, which was the whole of what `contains` over the tail could see — so the commit was allowed and would still have blocked on $EDITOR after spending the gate. Reproduced against the old spelling in a throwaway fixture before this landed, which is what the case exists to hold: git commit -x=mfoo old spelling -> allow git commit -x=mfoo new spelling -> DENY The second assertion proves the ANCHOR rather than the class: `-vm` reaches a message flag from the start of the cluster and must stay allowed, so a pattern that dropped `^` would go red here. Refs: CLOUD-885 --- tests/run-shape.bats | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/run-shape.bats b/tests/run-shape.bats index 09e74cf8c..13b6ea66f 100644 --- a/tests/run-shape.bats +++ b/tests/run-shape.bats @@ -99,6 +99,26 @@ allowed() { [[ "$1" != *'"deny"'* ]]; } done } +@test "THE MEASURED SHAPE: a token carrying an m is not a flag cluster" { + # CLOUD-885. The rule reads "one `-`, then LETTERS, at least one of which + # selects a message source". Before `regex.match` it was spelled as + # `contains` over the flag's tail, which cannot say "letters" — so `-x=mfoo` + # carried an `m`, read as naming a message source, and a commit that still + # blocks on $EDITOR went through. + # + # This is the discriminating case rather than another `-m`: the suite as it + # stood covered `-m`, `-am`, `-F`, `-C` and the long forms, and every one of + # them passes under BOTH spellings. Reproduced against the old spelling in a + # fixture before this landed, which is the evidence the case exists for. + run hook 'git commit -x=mfoo' + denied "$output" + + # The other direction, so the anchor is proven and not just the class: a + # message flag must be reached from the START of the cluster. `-vm` is one. + run hook 'git commit -vm "a message"' + allowed "$output" +} + # --- the list, which is where a raw-string module goes silent --------------- @test "a compound list is judged per element, not by its first word" { From 2e2fac7fdde7c0122ee15abce5878f4fa3d59cb9 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sat, 22 Aug 2026 04:43:31 +0000 Subject: [PATCH 3/7] fix(policy): drop the semver feature nothing calls, and the vacuous pass in run-shape's helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both found by the review bot on #649, both verified against the tree before acting, and the second is wider than it was reported. SEMVER WAS THIS ROW'S OWN RULE BROKEN BY THIS ROW. CLOUD-885's acceptance 4 says a feature must be load-bearing rather than declared, because a feature nothing calls is `input.tree.tracked` documented and never built (CLOUD-845): it reads as a capability, nothing checks it, and the first consumer discovers whether it works. `regex` earned its place by fixing a verdict in the same commit. `semver` was added beside it on the reasoning that version-comparison gates will want it, and zero modules in the tree call it. It lands with its first consumer instead. The measurement survives on CLOUD-885 either way, so re-adding it is a line rather than a re-derivation. Re-measured without it: the activated regorus closure is 45 packages, not the 46 the comment recorded, and that number is corrected here — a stale constant in the comment that states the pin is the exact defect `rules-drift` exists for. THE HELPERS WERE VACUOUS ON A CRASH, SUITE-WIDE. Reported against one line; it was every allow case in the file, and nothing in the suite referenced `$status` at all. `batten hook` prints NOTHING on an allow — the JSON is emitted only to deny, and both paths exit 0, because the contract is that the harness reads the decision and not the code. Measured on this branch: git commit -m x output: (empty) exit 0 git commit output: {...deny} exit 0 So `allowed() { [[ "$1" != *deny* ]]; }` over an empty string is TRUE, and a binary that died before judging anything would have taken every allow case green with it. That is CLOUD-251's vacuous pass wearing a test's clothes. Fixed in the helper rather than at the reported line, so all fourteen cases gain the assertion at once. A non-zero status is exactly and only the crash, which is what makes this the right discriminator rather than a belt-and-braces addition. Gates re-run after both changes: evaluator-closure-check EXIT 0 (45 packages, none of the nine), macos-link-check EXIT 0, perf-compare EXIT 0 (every measured path within 1.30x of the merge base), test:bats 2661/2661 under the stricter helpers. Refs: CLOUD-885 --- Cargo.toml | 10 ++++++++++ tests/run-shape.bats | 17 +++++++++++++++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f74a8612f..74d20b7b5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -242,6 +242,16 @@ gix = { version = "0.86", default-features = false, features = [ # edge. The first tree-scoped preset was written string-only and hit the wall on # the second rule, which is when this was reconsidered. # +# `semver` IS NOT HERE, AND THAT IS THIS ROW'S OWN RULE APPLIED TO ITSELF. It +# was added beside `regex` on the reasoning that version-comparison gates will +# want it — and no module in the tree calls it. A feature nothing calls is +# `input.tree.tracked` documented and never built (CLOUD-845): it reads as a +# capability, it is checked by nothing, and the first consumer discovers whether +# it actually works. So it lands with its first consumer rather than ahead of +# one. The measurement is recorded on CLOUD-885 either way — `semver` is +# `dep:semver` alone and cleared both gates — so re-adding it costs a line, not +# a re-derivation. +# # `glob` IS REFUSED BY CONSTRUCTION, not by preference, and stating it here is # what stops it being re-litigated as an oversight: `glob = ["dep:globset"]`, and # `globset` is one of the nine names `evaluator-closure-check` decides on. The diff --git a/tests/run-shape.bats b/tests/run-shape.bats index 13b6ea66f..e4c79fbb9 100644 --- a/tests/run-shape.bats +++ b/tests/run-shape.bats @@ -68,8 +68,21 @@ hook() { # hook (cd "$REPO" && printf '%s' "$envelope" | "$BIN" hook --harness claude-code) } -denied() { [[ "$1" == *'"permissionDecision":"deny"'* ]]; } -allowed() { [[ "$1" != *'"deny"'* ]]; } +# BOTH HELPERS ASSERT THE EXIT STATUS, and for `allowed` that is the whole +# assertion rather than a belt-and-braces addition. `batten hook` prints NOTHING +# on an allow — the JSON is emitted only to deny, and the exit status is 0 either +# way, because the contract is that the harness reads the decision and not the +# code. So `[[ "$1" != *deny* ]]` over an EMPTY string is true, and every allow +# case in this file went green on any output at all, including the output of a +# binary that died before it judged anything. Measured on this branch: allow and +# deny both exit 0, so a non-zero status is exactly and only the crash. +# +# That is CLOUD-251's vacuous pass wearing a test's clothes, and it was suite- +# wide rather than one case: nothing here referenced `$status` at all. Fixed in +# the helper so all fourteen cases gain it at once. `$status` is the global bats +# `run` sets, so this reads the status of the call the case just made. +denied() { [ "$status" -eq 0 ] && [[ "$1" == *'"permissionDecision":"deny"'* ]]; } +allowed() { [ "$status" -eq 0 ] && [[ "$1" != *'"deny"'* ]]; } # --- the predicate ---------------------------------------------------------- From 659c625b50d144c5a7fd789c6ef5fcbd5983f599 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sat, 22 Aug 2026 05:01:58 +0000 Subject: [PATCH 4/7] feat(policy): refuse a regex over the command line, so the capability ships with its bound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The commit that enabled `regex` shipped its own limit as PROSE — a paragraph in the manifest saying "this does not reach a syntax question". Non-negotiable rule 2 says prose is feedforward only: a new rule without a runnable gate is half a change. This is the other half. THE FAILURE THIS REFUSES IS THE ONE THE FEATURE MAKES AVAILABLE. 86 gate- described bash programs are queued to migrate onto Rego, their predicates are written in `grep -E`/`sed -E`/`awk`, and `regex.match` is now the closest-looking thing in the target language. The obvious translation of a shell predicate is a regex over the command line, and it is wrong for a reason no amount of care fixes: a shell command is not a regular language. Quoting nests, a heredoc binds to a later list element, `$(…)` recurses, and a token in command position is indistinguishable by pattern from the same token inside a comment or a string. Not hypothetical, and measured on this tree. CLOUD-310 ran one gate three ways: 40 findings by literal match, 13 after excluding comments, and 0 structurally. All 40 were false positives a regex reports with total confidence. THE LINE IS DECOMPOSE-THEN-MATCH. A regex over a leaf is the whole point and is what `forbid`'s own `regex` column has done since CLOUD-283; a regex over a scalar carrying nested structure is the barf. `policy/run-shape.rego` already reaches a regex only after `tokens(stage)` has split the command, and this refuses the shortcut that skips that step. regex.match(p, input.call.command) refused at load regex.match(p, word) loads, where word came from a split Mechanism, on the AST `describe` already reads for CLOUD-845's emittable-key check: `collect_regex_subjects` finds `Call` nodes whose `fcn` resolves to `regex.*` and collects every argument resolving to an `input.` path. EVERY argument rather than the subject position — `regex.match(pattern, value)` and `regex.replace(s, pattern, value)` disagree on ordering, and a per-builtin position table is a second thing to keep in step with upstream. Conservative in the right direction and free: the pattern argument is a literal in every real use, so it contributes no path. Refused at LOAD and in BOTH scopes: a config fault at exit 1 rather than a policy verdict at adjudication, which is house-style §8 and the placement of the sibling check it sits beside. The refusal names its remedy (CLOUD-437) and is pointer-only — the path and the module, never a byte of the body. SHOWN ABLE TO FAIL (CLOUD-418), both directions: - with `call.command` removed from the table, the module loads and the case panics: "a regex over the raw command line must not load" - the second half is the discriminator against an OVER-broad guard: a gate that refused every `regex.match` would satisfy the refusal assertion and be useless, so the same force-push predicate written correctly has to load WHAT THIS DOES NOT DO, stated so nobody reads it as more than it is. The table has one entry. It catches the specific mistake the migration will reach for; it cannot catch a regex applied to a parsed value to do something silly with it. There is no honest computable predicate for "is this pattern parsing something non-regular" — that is a model verdict, and non-negotiable rule 3 forbids it. The real answer is a parse tree on the mediated call, so a structural question has a CORRECT destination rather than only a forbidden one. Until that lands this guard says "not that way" without offering the way, which is a cost worth paying against a silent false positive. Refs: CLOUD-885 --- crates/batten/src/policy.rs | 133 ++++++++++++++++++++++++++ crates/batten/tests/policy_modules.rs | 65 +++++++++++++ 2 files changed, 198 insertions(+) diff --git a/crates/batten/src/policy.rs b/crates/batten/src/policy.rs index 1e0cce550..4dc33fd1f 100644 --- a/crates/batten/src/policy.rs +++ b/crates/batten/src/policy.rs @@ -616,6 +616,7 @@ pub fn load(root: &Path, rules: &[Rule], reference: Option<&str>) -> Result) -> Result Ok(()) } +/// The input paths a `regex.*` builtin may not be handed (CLOUD-885). +/// +/// **The reason this table exists is the reason `regex` was enabled at all.** +/// Opening the builtin gave every migrating gate the closest-looking thing to +/// the `grep -E`/`sed -E`/`awk` it is being translated from, and the obvious +/// translation of a shell predicate is a regex over the command line. That +/// answer is wrong for a reason no amount of care fixes: a shell command is not +/// a regular language. Quoting nests, heredocs bind to a later list element, +/// `$(…)` recurses, and a token in command position is indistinguishable by +/// pattern from the same token inside a comment or a string. +/// +/// It is not a hypothetical. CLOUD-310 measured one gate three ways over this +/// tree: a literal match found 40, comment exclusion cut it to 13, and the +/// structural answer was **0**. Every one of those 40 was a false positive that +/// a regex would have reported with total confidence. +/// +/// So the capability ships with its bound, per non-negotiable rule 2, and the +/// bound is the thing prose could not hold: `policy/run-shape.rego` reaches a +/// regex only after `tokens(stage)` has already decomposed the command, and +/// this refuses the shortcut that skips that step. +/// +/// **A leaf is fine and is the whole point.** `input.tree.lines[p][i]` is one +/// line, reached by iteration, and matching a shape against it is exactly what +/// `forbid`'s own `regex` column does (CLOUD-283). Only a scalar that carries +/// nested structure is named here. +/// +/// Stated as a table rather than derived, because there is no existing +/// authority for "which input fields carry structure" — `Fact::ALL` does not +/// cover `call.command`, which is an envelope field. Same shape as +/// `evaluator-closure-check`'s `IO_CRATES`: a short list that decides, written +/// once, with its reason beside it. `no_regex_over_structure` asserts the list +/// is non-empty and names the command, so it cannot be quietly emptied. +const NOT_REGULAR: &[(&str, &str)] = &[( + "call.command", + "a shell command line — quoting nests, a heredoc binds to a later list element, and a token in command position is indistinguishable by pattern from the same token inside a comment or a string. Decompose it first (`policy/run-shape.rego` does), then match the shape against a token", +)]; + +/// Refuse a module handing a `regex.*` builtin something that is not regular +/// (CLOUD-885). +/// +/// **Both scopes**, unlike [`check_tree_paths_are_emittable`]: the subject this +/// protects is `input.call.command`, which only a mediated-call row reads, and a +/// tree row that somehow reached it would be just as wrong. +/// +/// At load, so it is a config fault at exit `1` rather than a policy verdict — +/// the same placement and the same argument as the emittable-key check above. +/// +/// # Errors +/// +/// A [`UsageError`] (exit `1`) naming the path, the module and the remedy +/// (CLOUD-437). Pointer-only: never a line of the module body. +fn check_regex_subjects_are_regular(rule: &Rule, bundle: &Bundle, source: &str) -> Result<()> { + let Some(described) = describe(&bundle.engine) else { + // Could-not-look on the AST is not a refusal, for the reason stated on + // the sibling check: `load` has already compiled this module, so an + // unrecognised shape is this reader's limitation. + return Ok(()); + }; + for module in &described { + for rule_ast in &module.rules { + for subject in &rule_ast.regex_subjects { + let Some((path, why)) = NOT_REGULAR + .iter() + .find(|(path, _)| subject == path || subject.starts_with(&format!("{path}."))) + else { + continue; + }; + return Err(UsageError::raise(format!( + "rule `{}` registers `{source}`, whose module {} hands `input.{path}` to a `regex` builtin. That is {why}.", + rule.id, module.path, + ))); + } + } + } + Ok(()) +} + /// The keys `rules::tree_document` emits, derived from the fact model. /// /// Named once here so the refusal above and the engine agree by construction @@ -1318,6 +1397,14 @@ struct DescribedRule { /// literals beside it. Paths only: a reference is a NAME, never a value, so /// rule 4 has nothing to say about carrying it. input_paths: Vec, + /// Every `input.` handed to a `regex.*` builtin as an argument + /// (CLOUD-885). + /// + /// A subset of [`Self::input_paths`], kept separately because the question + /// is different: that field asks *can the engine produce this*, this one + /// asks *is this thing regular*. A path can be perfectly emittable and still + /// be the wrong subject for a regex. + regex_subjects: Vec, } /// Read every module's rule names, spans and literals off the compiled AST. @@ -1363,11 +1450,14 @@ fn describe(engine: ®orus::Engine) -> Option> { collect_literals(rule, &mut literals); let mut input_paths = Vec::new(); collect_input_paths(rule, &mut input_paths); + let mut regex_subjects = Vec::new(); + collect_regex_subjects(rule, &mut regex_subjects); rules.push(DescribedRule { name, head_line, literals, input_paths, + regex_subjects, }); } described.push(Described { @@ -1451,6 +1541,49 @@ fn collect_input_paths(value: &serde_json::Value, found: &mut Vec) { } } +/// Every `input.` handed to a `regex.*` builtin, without the +/// `input.` prefix (CLOUD-885). +/// +/// A call is `{"Call": {"fcn": , "params": [, …]}}`, and `fcn` +/// resolves through [`reference_path`] exactly as an input reference does — +/// `regex.match` is a `RefDot` over the var `regex`. +/// +/// **EVERY parameter, not the subject position.** The regex builtins do not +/// agree on argument order: `regex.match(pattern, value)` and +/// `regex.replace(s, pattern, value)` put the subject in different places, and +/// a table of per-builtin positions is a second thing to keep in step with +/// upstream. Reading them all is strictly conservative in the direction that +/// matters — the pattern argument is a literal in every real use, so it +/// contributes no input path and the widening costs nothing. +fn collect_regex_subjects(value: &serde_json::Value, found: &mut Vec) { + match value { + serde_json::Value::Object(object) => { + if let Some(call) = object.get("Call") + && let Some(name) = call.get("fcn").and_then(reference_path) + && name.starts_with("regex.") + && let Some(params) = call.get("params").and_then(serde_json::Value::as_array) + { + for param in params { + if let Some(path) = reference_path(param) + && let Some(rest) = path.strip_prefix("input.") + { + found.push(rest.to_owned()); + } + } + } + for child in object.values() { + collect_regex_subjects(child, found); + } + } + serde_json::Value::Array(items) => { + for item in items { + collect_regex_subjects(item, found); + } + } + _ => {} + } +} + /// Every string literal in a rule's AST subtree. fn collect_literals(value: &serde_json::Value, found: &mut Vec) { match value { diff --git a/crates/batten/tests/policy_modules.rs b/crates/batten/tests/policy_modules.rs index 8261bf01c..b69c77dc8 100644 --- a/crates/batten/tests/policy_modules.rs +++ b/crates/batten/tests/policy_modules.rs @@ -305,6 +305,71 @@ fn two_rows_registering_one_module_are_refused_at_load() { assert!(format!("{err}").contains("already registers")); } +/// A module reaching for a regex over the raw command line, which is the +/// mistake enabling `regex` makes available (CLOUD-885). +const REGEX_OVER_COMMAND: &str = r#" +package batten + +import rego.v1 + +deny contains "looks like a force push" if { + regex.match(`git\s+push\s+--force`, input.call.command) +} +"#; + +/// The same predicate written the way the bound requires: decompose first, +/// match the shape against a token. +const REGEX_OVER_A_TOKEN: &str = r#" +package batten + +import rego.v1 + +deny contains "looks like a force push" if { + some word in split(input.call.command, " ") + regex.match(`^--force`, word) +} +"#; + +#[test] +fn no_regex_over_structure() { + // THE CAPABILITY SHIPS WITH ITS BOUND (non-negotiable rule 2). Opening + // regorus's `regex` feature handed every migrating gate the closest-looking + // thing to the `grep -E`/`sed -E` it is translated from, and the obvious + // translation of a shell predicate is a regex over the command line. A shell + // command is not a regular language, and CLOUD-310 measured the cost on this + // very tree: one gate, 40 findings by literal match, 13 after excluding + // comments, and **0** structurally. All 40 were false positives a regex + // would have reported with total confidence. + // + // A comment saying so is feedforward only, which is what this replaces. + let root = scratch("regex-structure"); + let path = module_file(&root, "over-command.rego", REGEX_OVER_COMMAND); + let err = policy::load(&root, &[row("force-push", &path)], None) + .expect_err("a regex over the raw command line must not load"); + let rendered = format!("{err}"); + assert!( + rendered.contains("input.call.command"), + "the refusal names the path: {rendered}" + ); + assert!( + rendered.contains("Decompose it first"), + "the refusal names its remedy (CLOUD-437): {rendered}" + ); + assert!( + !rendered.contains("git\\s+push"), + "pointer-only: no byte of the module body reaches the refusal: {rendered}" + ); + + // THE DISCRIMINATING HALF. A gate that refused every `regex.match` would + // pass the assertion above and be useless — `regex` was enabled precisely so + // a module could match a shape against a leaf, which is what `forbid`'s own + // `regex` column does (CLOUD-283). The same predicate, decomposed first, + // has to load. + let ok = module_file(&root, "over-token.rego", REGEX_OVER_A_TOKEN); + policy::load(&root, &[row("force-push-token", &ok)], None) + .expect("a regex over a token, after the structure is decomposed, is the sanctioned form"); +} + #[test] fn a_module_holds_no_source_and_cannot_leak_one_through_debug() { // Rule 4 is structural here rather than careful: `Module` has no `source` From ed8063df06eab230eb3ed5f65c94455863717565 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sat, 22 Aug 2026 05:22:51 +0000 Subject: [PATCH 5/7] revert: NOT_REGULAR gates the rarer disease and blesses the common one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts b7b00d2. The guard refused a `regex.*` builtin over `input.call.command`, on the reasoning that a shell command is not a regular language and a model handed regex will reach for it anyway. Both halves of that are true. The guard is still wrong, and the measurement is why. WHAT THE TREE ACTUALLY SHOWS, measured on f3eb5cd with comments stripped: 82 of 140 shell programs use `grep -E`/`sed -E`/`awk`/`=~`, over 338 sites. But the dominant failure is not regex applied to something non-regular. It is ONE regular concept — a tracker key — carrying 19 distinct spellings across 17 programs. `CLOUD-[0-9]+` appears at 15 sites in 9 programs. A `CLOUD-*` GLOB spelling sits beside the regexes. `graph-check` holds two variants of its own pattern 52 lines apart, differing by a paren group: `((is|is` at :317 against `(is|is` at :369. Those are correct regexes over a genuinely regular thing, duplicated and drifting. This guard says nothing about them. It covers regex-parsing-shell, which in this tree is `run-shape-guard`, one program. AND THE STRUCTURED CASE IS ALREADY CURED, with no capability this branch added. `ci-local-parity` attacks `.github/workflows/*.yml` with 35 regex sites. `policy/privileged-lane.rego` — landed, tree-scoped, `sources` naming the same glob — asks the same class of question with `doc.jobs`, a field access. Same input, same job, 35 patterns against one field access. `Fact::Document` parses TOML/YAML/JSON/JSON5, so the whole structured bucket migrates onto a surface where regex is not the tempting answer because it is not the cheap one. SO THE GUARD BLESSES BY SILENCE. Everything it does not refuse now reads as sanctioned, and what it does not refuse is the failure that actually recurs. That is `.shellcheckrc`'s shape (mem:toolchain-and-hooks, CLOUD-295): a suppression looked like evidence a file was being read, and it was not. The gate worth having is over duplication — a regex literal appearing in two modules of one bundle — which is decidable off the AST `collect_literals` already walks, and fixable because CLOUD-837 made a bundle one engine so a helper defined once is callable from every module in it. "Is this pattern parsing something non-regular" is a model verdict, which non-negotiable rule 3 refuses. The `regex` feature stays. Matching a tracker key inside prose is regular and regex is the right tool for it. Refs: CLOUD-885 --- crates/batten/src/policy.rs | 133 -------------------------- crates/batten/tests/policy_modules.rs | 65 ------------- 2 files changed, 198 deletions(-) diff --git a/crates/batten/src/policy.rs b/crates/batten/src/policy.rs index 4dc33fd1f..1e0cce550 100644 --- a/crates/batten/src/policy.rs +++ b/crates/batten/src/policy.rs @@ -616,7 +616,6 @@ pub fn load(root: &Path, rules: &[Rule], reference: Option<&str>) -> Result) -> Result Ok(()) } -/// The input paths a `regex.*` builtin may not be handed (CLOUD-885). -/// -/// **The reason this table exists is the reason `regex` was enabled at all.** -/// Opening the builtin gave every migrating gate the closest-looking thing to -/// the `grep -E`/`sed -E`/`awk` it is being translated from, and the obvious -/// translation of a shell predicate is a regex over the command line. That -/// answer is wrong for a reason no amount of care fixes: a shell command is not -/// a regular language. Quoting nests, heredocs bind to a later list element, -/// `$(…)` recurses, and a token in command position is indistinguishable by -/// pattern from the same token inside a comment or a string. -/// -/// It is not a hypothetical. CLOUD-310 measured one gate three ways over this -/// tree: a literal match found 40, comment exclusion cut it to 13, and the -/// structural answer was **0**. Every one of those 40 was a false positive that -/// a regex would have reported with total confidence. -/// -/// So the capability ships with its bound, per non-negotiable rule 2, and the -/// bound is the thing prose could not hold: `policy/run-shape.rego` reaches a -/// regex only after `tokens(stage)` has already decomposed the command, and -/// this refuses the shortcut that skips that step. -/// -/// **A leaf is fine and is the whole point.** `input.tree.lines[p][i]` is one -/// line, reached by iteration, and matching a shape against it is exactly what -/// `forbid`'s own `regex` column does (CLOUD-283). Only a scalar that carries -/// nested structure is named here. -/// -/// Stated as a table rather than derived, because there is no existing -/// authority for "which input fields carry structure" — `Fact::ALL` does not -/// cover `call.command`, which is an envelope field. Same shape as -/// `evaluator-closure-check`'s `IO_CRATES`: a short list that decides, written -/// once, with its reason beside it. `no_regex_over_structure` asserts the list -/// is non-empty and names the command, so it cannot be quietly emptied. -const NOT_REGULAR: &[(&str, &str)] = &[( - "call.command", - "a shell command line — quoting nests, a heredoc binds to a later list element, and a token in command position is indistinguishable by pattern from the same token inside a comment or a string. Decompose it first (`policy/run-shape.rego` does), then match the shape against a token", -)]; - -/// Refuse a module handing a `regex.*` builtin something that is not regular -/// (CLOUD-885). -/// -/// **Both scopes**, unlike [`check_tree_paths_are_emittable`]: the subject this -/// protects is `input.call.command`, which only a mediated-call row reads, and a -/// tree row that somehow reached it would be just as wrong. -/// -/// At load, so it is a config fault at exit `1` rather than a policy verdict — -/// the same placement and the same argument as the emittable-key check above. -/// -/// # Errors -/// -/// A [`UsageError`] (exit `1`) naming the path, the module and the remedy -/// (CLOUD-437). Pointer-only: never a line of the module body. -fn check_regex_subjects_are_regular(rule: &Rule, bundle: &Bundle, source: &str) -> Result<()> { - let Some(described) = describe(&bundle.engine) else { - // Could-not-look on the AST is not a refusal, for the reason stated on - // the sibling check: `load` has already compiled this module, so an - // unrecognised shape is this reader's limitation. - return Ok(()); - }; - for module in &described { - for rule_ast in &module.rules { - for subject in &rule_ast.regex_subjects { - let Some((path, why)) = NOT_REGULAR - .iter() - .find(|(path, _)| subject == path || subject.starts_with(&format!("{path}."))) - else { - continue; - }; - return Err(UsageError::raise(format!( - "rule `{}` registers `{source}`, whose module {} hands `input.{path}` to a `regex` builtin. That is {why}.", - rule.id, module.path, - ))); - } - } - } - Ok(()) -} - /// The keys `rules::tree_document` emits, derived from the fact model. /// /// Named once here so the refusal above and the engine agree by construction @@ -1397,14 +1318,6 @@ struct DescribedRule { /// literals beside it. Paths only: a reference is a NAME, never a value, so /// rule 4 has nothing to say about carrying it. input_paths: Vec, - /// Every `input.` handed to a `regex.*` builtin as an argument - /// (CLOUD-885). - /// - /// A subset of [`Self::input_paths`], kept separately because the question - /// is different: that field asks *can the engine produce this*, this one - /// asks *is this thing regular*. A path can be perfectly emittable and still - /// be the wrong subject for a regex. - regex_subjects: Vec, } /// Read every module's rule names, spans and literals off the compiled AST. @@ -1450,14 +1363,11 @@ fn describe(engine: ®orus::Engine) -> Option> { collect_literals(rule, &mut literals); let mut input_paths = Vec::new(); collect_input_paths(rule, &mut input_paths); - let mut regex_subjects = Vec::new(); - collect_regex_subjects(rule, &mut regex_subjects); rules.push(DescribedRule { name, head_line, literals, input_paths, - regex_subjects, }); } described.push(Described { @@ -1541,49 +1451,6 @@ fn collect_input_paths(value: &serde_json::Value, found: &mut Vec) { } } -/// Every `input.` handed to a `regex.*` builtin, without the -/// `input.` prefix (CLOUD-885). -/// -/// A call is `{"Call": {"fcn": , "params": [, …]}}`, and `fcn` -/// resolves through [`reference_path`] exactly as an input reference does — -/// `regex.match` is a `RefDot` over the var `regex`. -/// -/// **EVERY parameter, not the subject position.** The regex builtins do not -/// agree on argument order: `regex.match(pattern, value)` and -/// `regex.replace(s, pattern, value)` put the subject in different places, and -/// a table of per-builtin positions is a second thing to keep in step with -/// upstream. Reading them all is strictly conservative in the direction that -/// matters — the pattern argument is a literal in every real use, so it -/// contributes no input path and the widening costs nothing. -fn collect_regex_subjects(value: &serde_json::Value, found: &mut Vec) { - match value { - serde_json::Value::Object(object) => { - if let Some(call) = object.get("Call") - && let Some(name) = call.get("fcn").and_then(reference_path) - && name.starts_with("regex.") - && let Some(params) = call.get("params").and_then(serde_json::Value::as_array) - { - for param in params { - if let Some(path) = reference_path(param) - && let Some(rest) = path.strip_prefix("input.") - { - found.push(rest.to_owned()); - } - } - } - for child in object.values() { - collect_regex_subjects(child, found); - } - } - serde_json::Value::Array(items) => { - for item in items { - collect_regex_subjects(item, found); - } - } - _ => {} - } -} - /// Every string literal in a rule's AST subtree. fn collect_literals(value: &serde_json::Value, found: &mut Vec) { match value { diff --git a/crates/batten/tests/policy_modules.rs b/crates/batten/tests/policy_modules.rs index b69c77dc8..8261bf01c 100644 --- a/crates/batten/tests/policy_modules.rs +++ b/crates/batten/tests/policy_modules.rs @@ -305,71 +305,6 @@ fn two_rows_registering_one_module_are_refused_at_load() { assert!(format!("{err}").contains("already registers")); } -/// A module reaching for a regex over the raw command line, which is the -/// mistake enabling `regex` makes available (CLOUD-885). -const REGEX_OVER_COMMAND: &str = r#" -package batten - -import rego.v1 - -deny contains "looks like a force push" if { - regex.match(`git\s+push\s+--force`, input.call.command) -} -"#; - -/// The same predicate written the way the bound requires: decompose first, -/// match the shape against a token. -const REGEX_OVER_A_TOKEN: &str = r#" -package batten - -import rego.v1 - -deny contains "looks like a force push" if { - some word in split(input.call.command, " ") - regex.match(`^--force`, word) -} -"#; - -#[test] -fn no_regex_over_structure() { - // THE CAPABILITY SHIPS WITH ITS BOUND (non-negotiable rule 2). Opening - // regorus's `regex` feature handed every migrating gate the closest-looking - // thing to the `grep -E`/`sed -E` it is translated from, and the obvious - // translation of a shell predicate is a regex over the command line. A shell - // command is not a regular language, and CLOUD-310 measured the cost on this - // very tree: one gate, 40 findings by literal match, 13 after excluding - // comments, and **0** structurally. All 40 were false positives a regex - // would have reported with total confidence. - // - // A comment saying so is feedforward only, which is what this replaces. - let root = scratch("regex-structure"); - let path = module_file(&root, "over-command.rego", REGEX_OVER_COMMAND); - let err = policy::load(&root, &[row("force-push", &path)], None) - .expect_err("a regex over the raw command line must not load"); - let rendered = format!("{err}"); - assert!( - rendered.contains("input.call.command"), - "the refusal names the path: {rendered}" - ); - assert!( - rendered.contains("Decompose it first"), - "the refusal names its remedy (CLOUD-437): {rendered}" - ); - assert!( - !rendered.contains("git\\s+push"), - "pointer-only: no byte of the module body reaches the refusal: {rendered}" - ); - - // THE DISCRIMINATING HALF. A gate that refused every `regex.match` would - // pass the assertion above and be useless — `regex` was enabled precisely so - // a module could match a shape against a leaf, which is what `forbid`'s own - // `regex` column does (CLOUD-283). The same predicate, decomposed first, - // has to load. - let ok = module_file(&root, "over-token.rego", REGEX_OVER_A_TOKEN); - policy::load(&root, &[row("force-push-token", &ok)], None) - .expect("a regex over a token, after the structure is decomposed, is the sanctioned form"); -} - #[test] fn a_module_holds_no_source_and_cannot_leak_one_through_debug() { // Rule 4 is structural here rather than careful: `Module` has no `source` From 84e989a5e609e387f4724000442438760bc906b1 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sat, 22 Aug 2026 06:03:24 +0000 Subject: [PATCH 6/7] feat(policy)!: a regex costs a declaration, so the cheap path is the correct one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit THE LEVER IS COST, NOT PROHIBITION, and the earlier attempt on this branch got that wrong twice. `NOT_REGULAR` was a denylist of subjects; it covered one program, said nothing about the failure that actually recurs, and blessed everything it did not name. Reverted already. This is the mechanism that replaces it. A `regex.*` builtin may not reach a string literal. Expressions are `[[pattern]]` rows in the one committed authority, projected into the evaluator's `data` document, referenced by id: regex.match(`CLOUD-[0-9]+`, w) refused at load regex.match(data.batten.patterns["tracker-key"], w) loads and decides The gradient is the point. A one-off pattern costs a config row and an id; a shared one is free after the first; and asking the same question of an already-parsed document costs a field access with no config edit at all. Effort now orders the same way correctness does, which is the only version of this that survives a translator that does not reason carefully — and 86 bash gates written in `grep -E` are queued to be translated. WHY THIS IS NOT FRICTION BOLTED ON. `Config::verbs` already carries the argument verbatim for the mutating-verb table: "consumer-specific by nature, so it lives here and never in the crate (non-negotiable rule 1)". A tracker-key expression is a consumer identifier by exactly that test. Two more things fall out rather than being designed in: duplication becomes unwritable instead of detectable — one declaration, one home — and the pattern inventory becomes reviewable data (§11), readable out of one file. Measured on the tree this engine gates, comments stripped: 82 of 140 shell programs use `grep -E`/`sed -E`/`awk`/`=~` over 338 sites, and ONE concept — a tracker key — carries 19 distinct spellings across 17 programs. `CLOUD-[0-9]+` alone sits at 15 sites in 9 of them, with a `CLOUD-*` glob spelling beside the regexes and one program holding two variants of its own pattern 52 lines apart. Correct regexes over a regular thing, duplicated until they drifted. CLOSED AT BOTH ENDS, and the second half is not optional. Refusing the inline form alone leaves the identical hole one step later: `data.batten.patterns["typo"]` resolves to UNDEFINED, Rego reads undefined as "this rule body does not hold", so the module loads clean, evaluates clean and gates nothing. So a reference no row declares is refused too — the same shape `check_tree_paths_are_emittable` uses against a `tree` key the engine never emits. The same failure arriving by deletion is `WeakeningKind::PatternRemoved`: a local override dropping a row silences every module referencing it, and one row can silence several predicates at once, because a declared pattern is shared by design. Reported with the id, never the expression. TWO BUGS ONLY RUNNING IT COULD HAVE FOUND, both from an AST probe rather than a reading: - a Rego backtick literal serialises as `RawString`, not `String`. Reading only `String` let every realistic inline pattern through, since backticks are how a regex carrying backslashes is written. - a REFERENCE contains a literal: `data.batten.patterns["x"]` is a `RefBrack` whose index is the string `"x"`, so a naive sweep read the sanctioned form as the refused one and no module could load at all. CONSUMER #1 PROVES IT. `policy/run-shape.rego`'s flag-cluster shape is declared in `batten.toml` and referenced by name; its `#MUTANT` directive now corrupts the REFERENCE, so the mutation exercises the silent-disarm path directly — the allow cases go red when the id stops resolving. Four completeness gates in this repo refused the new table until it was classified, and the third found a defect this change would otherwise have shipped: validation call site (`config.rs`), resolve provenance, trust verdict — which is where the silent-disarm consequence surfaced — and every-kind-exercised. Refs: CLOUD-885 BREAKING CHANGE: the pattern table has to reach the evaluator, so it is threaded beside `provisions` the way that table already is. `Config` gains `patterns`, `trust::WeakeningKind` gains `PatternRemoved`, and `rules::run_static`, `rules::run_recorded`, `rules::run_all`, `policy::load` and `policy::compile` each take one more argument. Caught by `mise run verify`'s semver gate, which named all three lint classes rather than letting a patch-compatible claim ship over an API that moved. --- .serena/memories/core.md | 18 ++ batten.toml | 17 ++ crates/batten/src/config.rs | 20 ++ crates/batten/src/hook.rs | 17 +- crates/batten/src/lib.rs | 35 ++- crates/batten/src/lint.rs | 2 +- crates/batten/src/pattern.rs | 135 ++++++++++ crates/batten/src/policy.rs | 273 ++++++++++++++++++++- crates/batten/src/resolve.rs | 6 + crates/batten/src/rules.rs | 22 +- crates/batten/src/trust.rs | 62 +++++ crates/batten/tests/document_read_count.rs | 5 +- crates/batten/tests/identity_churn.rs | 2 +- crates/batten/tests/policy_engine_count.rs | 3 +- crates/batten/tests/policy_modules.rs | 139 +++++++++-- crates/batten/tests/policy_presets.rs | 23 +- crates/batten/tests/policy_test_suite.rs | 10 +- crates/batten/tests/policy_tree.rs | 15 +- crates/batten/tests/policy_whole_set.rs | 5 + crates/batten/tests/primitives.rs | 21 +- hk.pkl | 1 + policy/run-shape.rego | 4 +- schema/batten.schema.json | 26 ++ tests/run-shape.bats | 8 + 24 files changed, 803 insertions(+), 66 deletions(-) create mode 100644 crates/batten/src/pattern.rs diff --git a/.serena/memories/core.md b/.serena/memories/core.md index c51cb4e71..d21ad1f2e 100644 --- a/.serena/memories/core.md +++ b/.serena/memories/core.md @@ -1403,6 +1403,24 @@ judge_fingerprint`, its own domain tag), so a caller can reference content it exactly what this surface could rebuild. `Module` holds no `source` field and hand-writes `Debug`, so a policy body has nowhere to live past compilation (rule 4). +- `pattern.rs` — the `[[pattern]]` table (CLOUD-885): named regular expressions a + policy module references by id, never writes inline. **The lever is cost, not + prohibition** — "do not regex things that are not regular" is a judgement and + rule 3 refuses a gate over one, but _where a pattern lives_ is decidable. So a + regex costs a config row and an id while the same question over an + already-parsed document costs a field access, and the cheap path becomes the + correct one without a translator reasoning about regularity. Consumer-owned for + `verbs`' stated reason (rule 1): a tracker-key expression is a consumer + identifier. Two consequences fall out rather than being designed in — + duplication becomes _unwritable_ (measured: one concept, 19 spellings across 17 + shell programs), and the inventory becomes reviewable data (§11). `policy.rs` + closes it at both ends: an inline literal is refused, and so is a reference no + row declares, because that resolves to UNDEFINED and Rego reads undefined as + "does not hold" — a silent gate. The same failure by deletion is + `trust::WeakeningKind::PatternRemoved`. Two defects here were found by an AST + probe rather than a reading: a backtick literal is a `RawString` node and not a + `String`, and a REFERENCE contains a literal (`patterns["x"]` is a `RefBrack` + indexed by a string), so a naive sweep refuses the sanctioned form. - `provision.rs` — the `[[provision]]` manifest (CLOUD-90): pinned tools fetched and cached out of tree. §9's check/fix pair — `provision status` (read) is freshness, `provision apply [-n]` (write) is the fix. **The provisioned binary diff --git a/batten.toml b/batten.toml index be9188df4..fac0bc1ab 100644 --- a/batten.toml +++ b/batten.toml @@ -1657,6 +1657,23 @@ no_fix_reason = "an IO crate reaching the evaluator is closed where it was enabl # # Mediated-call scoped, because the predicate is over a command line. The other # four families of that guard stay in bash and its header says which and why. +# The short flag cluster `git commit` accepts a message source in — `-m`, `-am`, +# `-F`, `-C`, `-c` — as a shape rather than a literal (CLOUD-885). +# +# DECLARED HERE RATHER THAN IN THE MODULE, and this row is the mechanism proving +# itself on consumer #1. An expression is a consumer fact: written into +# `policy/run-shape.rego` it would be a pattern with no home, invisible to +# `config show`, and the second module wanting the same shape would re-spell it. +# The engine refuses an inline regex at load, so this row is not a convention — +# it is the only way to write the predicate at all. +# +# The anchor is the predicate. `contains` over the flag's tail cannot say +# "letters", so `-x=mfoo` carried an `m` and read as naming a message source, +# letting through a commit that still blocks on $EDITOR. +[[pattern]] +id = "short-message-flag-cluster" +regex = "^-[A-Za-z]*[mFCc]" + [[rule]] id = "commit-message-obtainable" kind = "policy" diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index 79c1f35dc..f0c6a00c8 100644 --- a/crates/batten/src/config.rs +++ b/crates/batten/src/config.rs @@ -162,6 +162,18 @@ pub struct Config { /// lookup are [`crate::verbs`]. #[serde(default, rename = "verb", skip_serializing_if = "Vec::is_empty")] pub verbs: Vec, + /// The named-regex table (CLOUD-885): every expression a policy module may + /// apply, declared once and referenced by id. + /// + /// Consumer-specific by nature for exactly [`Config::verbs`]'s reason — a + /// tracker-key expression is a consumer identifier, so it lives here and + /// never in the crate (non-negotiable rule 1). A module reaches it at + /// `data.batten.patterns[""]`; writing one inline is refused at load, + /// which is what prices a one-off pattern against a field access over an + /// already-parsed document. The type and its validation are + /// [`crate::pattern`]. + #[serde(default, rename = "pattern", skip_serializing_if = "Vec::is_empty")] + pub patterns: Vec, /// The per-path-class redirect table (CLOUD-280): what to run instead, /// keyed by what is protected rather than by the verb reaching for it. /// @@ -616,6 +628,12 @@ fn parse_ungated(text: &str, source: &str) -> Result { // too: `batten.local.toml` may add verb rows, and a raise-only override that // adds an inert one has still written something that cannot mean anything. crate::verbs::validate(&config.verbs)?; + // The named-regex table, at parse for the identical reason (CLOUD-885): a + // malformed expression is a config fault, and refusing it here means + // `config lint` and `doctor` catch it rather than a mediated call + // discovering it at adjudication, which is the worst time and the wrong exit + // class (house style §8). + crate::pattern::validate(&config.patterns)?; crate::redirect::validate(&config.redirects)?; // And the marker table, for the identical reason in the identical shape // (CLOUD-253). Both tables arrived in one commit; CLOUD-242 wired one of @@ -766,6 +784,7 @@ impl Config { strictness: None, fail_on_warning: None, rules: Vec::new(), + patterns: Vec::new(), scope: Vec::new(), protected: Vec::new(), unlanded: Vec::new(), @@ -1029,6 +1048,7 @@ mod tests { /// [`parse_ungated`] that does it. Deleting a call fails the test below. const VALIDATED_AT_LOAD: &[(&str, &str)] = &[ ("verbs", "crate::verbs::validate("), + ("patterns", "crate::pattern::validate("), ("redirects", "crate::redirect::validate("), ("markers", "crate::markers::validate("), // The LOCATED form (CLOUD-773): the loaders hold the config text, so a diff --git a/crates/batten/src/hook.rs b/crates/batten/src/hook.rs index cc2c414fb..10f75396a 100644 --- a/crates/batten/src/hook.rs +++ b/crates/batten/src/hook.rs @@ -2037,6 +2037,19 @@ impl Policy { // still is at `batten check`/`enforce`, which is where a tree rule // is evaluated and where `verify` and CI both reach it — so the // module is still refused before it can matter, one surface over. + // + // AND THE MODULE CHECKS DO NOT RUN HERE EITHER (CLOUD-885), which is + // the same finding reached from the other side. Narrowing the rows + // above removes the tree modules; this removes the AST walk over the + // ones that remain. `check_no_inline_regex` and + // `check_tree_paths_are_emittable` read a module through + // `get_ast_as_json`, which serialises every rule of every module — + // and their answer is a property of the module TEXT, so it is fixed + // for the life of the load and identical on every surface. CI + // measured re-deriving it per call at `wired` 14.03 ms -> 22.98 ms + // (1.638x). Both are the same rule: the mediated call loads and + // decides; a config fault is reported where config faults are + // reported. bundles: crate::policy::load( root, &resolved @@ -2045,6 +2058,8 @@ impl Policy { .filter(|rule| rule.scope == RuleScope::MediatedCall) .cloned() .collect::>(), + &resolved.patterns, + crate::policy::ModuleChecks::SkipOnHotPath, reference, )?, }) @@ -5318,7 +5333,7 @@ mod tests { Policy { harness: Harness::ExitCode, facts: Vec::new(), - bundles: crate::policy::load(&dir, &[row], None).expect("load"), + bundles: crate::policy::load(&dir, &[row], &[], None).expect("load"), shapes: Vec::new(), fail_on_warning: false, verbs: Vec::new(), diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 60fdf232d..770e58d3d 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -43,6 +43,7 @@ pub mod lint; pub mod markers; pub mod output; pub mod outputs; +pub mod pattern; pub mod policy; pub mod provision; pub mod receipt; @@ -440,7 +441,7 @@ fn run_baseline( ) -> Result { let root = anchor(); let config = resolve::resolve(&root, overrides)?; - let scan = rules::run_static(&config.rules, &config.provisions, &root)?; + let scan = rules::run_static(&config.rules, &config.provisions, &config.patterns, &root)?; if prune { let Some(existing) = baseline::load(&root)? else { @@ -776,7 +777,12 @@ fn run_state_record(overrides: &Overrides, mode: Mode, err: &mut dyn Write) -> R // included (CLOUD-97 never once evaluated in this repository for exactly // that reason). Withholding is honest here because `record` below folds // `not_evaluated` into the store, where a withheld rule's findings HOLD. - let scan = rules::run_recorded(&config.rules, &config.provisions, Path::new("."))?; + let scan = rules::run_recorded( + &config.rules, + &config.provisions, + &config.patterns, + Path::new("."), + )?; if !scan.not_evaluated.is_empty() { // Never silent: a rule that did not look must say so, or a clean-looking // record is the false green. The COUNT carries that on the default rung @@ -1353,7 +1359,12 @@ impl SuiteReport { fn run_policy_test(json: bool, overrides: &Overrides, out: &mut dyn Write) -> Result { let root = Path::new("."); let config = resolve::resolve(root, overrides)?; - let bundles = policy::load(root, &config.rules, overrides.config_from.as_deref())?; + let bundles = policy::load( + root, + &config.rules, + &config.patterns, + overrides.config_from.as_deref(), + )?; // The same walk the tree engine hoists, so a suite's input carries the same // `tracked` a real `check` would hand the bundle (CLOUD-845). Resolved once // here rather than per row, for the reason `rules::run` gives. @@ -3767,12 +3778,26 @@ fn apply_baseline( Ok(kept) } +/// One of the two rule-running surfaces, as [`run_rules`] takes it. +/// +/// Named rather than written inline because the pattern table joined the +/// argument list (CLOUD-885) and a four-argument fn pointer is past what +/// `clippy::type_complexity` will read. The alias is also the clearer spelling: +/// the three tables and the root are what a runner needs, and saying so once +/// beats repeating it at both call sites. +type RuleRunner = fn( + &[rules::Rule], + &[provision::Provision], + &[pattern::NamedPattern], + &Path, +) -> Result; + fn run_rules( out: &mut dyn Write, err: &mut dyn Write, mode: Mode, overrides: &Overrides, - runner: fn(&[rules::Rule], &[provision::Provision], &Path) -> Result, + runner: RuleRunner, surface: Surface, json: bool, ) -> Result { @@ -3801,7 +3826,7 @@ fn run_rules( // store's resolve pass fail-closed (CLOUD-81), and the enforce surface now // journals (CLOUD-529), so dropping it here would let a rule that never // looked resolve every finding it covers. - let scan = runner(&config.rules, &config.provisions, &root)?; + let scan = runner(&config.rules, &config.provisions, &config.patterns, &root)?; let mut findings = scan.findings.clone(); // Declared budgets are gates, evaluated here rather than only under `policy diff --git a/crates/batten/src/lint.rs b/crates/batten/src/lint.rs index 3ca5106f6..a51fa19ef 100644 --- a/crates/batten/src/lint.rs +++ b/crates/batten/src/lint.rs @@ -462,7 +462,7 @@ pub fn run(dir: &Path, base_ref: Option<&str>, today: crate::waiver::Date) -> Re // reports nothing about a set it never saw. let bundles = config::parse(&text, &path.display().to_string()) .ok() - .map(|config| crate::policy::load(dir, &config.rules, base_ref)) + .map(|config| crate::policy::load(dir, &config.rules, &config.patterns, base_ref)) .and_then(std::result::Result::ok) .unwrap_or_default(); smells( diff --git a/crates/batten/src/pattern.rs b/crates/batten/src/pattern.rs new file mode 100644 index 000000000..69b86966d --- /dev/null +++ b/crates/batten/src/pattern.rs @@ -0,0 +1,135 @@ +//! Named regular expressions, declared in the one committed authority +//! (CLOUD-885). +//! +//! # Why a pattern may not be written inline in a module +//! +//! Enabling `regorus`'s `regex` builtin gave a policy module the matching +//! vocabulary [`crate::rules::RuleKind::Forbid`] has carried since CLOUD-283. +//! It also handed 86 queued bash migrations the closest-looking thing to the +//! `grep -E`/`sed -E`/`awk` they are being translated from, and a translator +//! reaches for whatever is cheapest to write. +//! +//! **The lever is cost, not prohibition.** A rule saying "do not regex things +//! that are not regular" cannot be a gate: "is this pattern parsing something +//! non-regular" is a judgement, and non-negotiable rule 3 says a gate resolves +//! to a command and an exit code over an object it decides. What *is* decidable +//! is where a pattern lives. So a regex costs a declaration — an id and a row in +//! `batten.toml` — while asking the same question of a parsed document or a +//! resolved invocation costs a field access. One-off patterns get priced; shared +//! ones stay cheap after the first. +//! +//! # Three things fall out, and none of them is friction for its own sake +//! +//! **Non-negotiable rule 1.** `CLOUD-[0-9]+` is a *consumer* identifier. Written +//! into a module under `crates/batten` it is a repo-agnosticism violation of +//! exactly the kind rule 1 names; written here it is where consumer facts +//! belong. [`crate::verbs::MutatingVerb`] already states this argument for the +//! mutating-verb table — "consumer-specific by nature, so it lives here and +//! never in the crate" — and this is the same table shape for the same reason. +//! +//! **Duplication becomes unwritable rather than detectable.** Measured on the +//! tree this engine gates, 2026-08-22 with comments stripped: 82 of 140 shell +//! programs use `grep -E`/`sed -E`/`awk`/`=~` over 338 sites, and one concept — +//! a tracker key — carries **19 distinct spellings across 17 programs**. +//! `CLOUD-[0-9]+` alone sits at 15 sites in 9 of them, with a `CLOUD-*` GLOB +//! spelling beside the regexes and one program holding two variants of its own +//! pattern 52 lines apart. Those are correct regexes over a genuinely regular +//! thing, duplicated until they drifted. A named declaration has one home, so +//! the second author finds the first one's work instead of re-deriving it. +//! +//! **The inventory becomes reviewable data** (house style §11). Every pattern +//! the policy set can apply is readable out of one file, which is not true of +//! any tree where they are written inline. +//! +//! # What a module sees +//! +//! The set is projected into the evaluator's `data` document, so a reference +//! reads `data.batten.patterns["tracker-key"]`. It is deliberately NOT on +//! `input`: `input` is the subject under adjudication and changes per call, +//! while this is configuration, fixed for the life of the load. + +use std::collections::BTreeSet; + +use serde::{Deserialize, Serialize}; + +use crate::error::UsageError; + +/// One declared regular expression. +/// +/// Mirrors [`crate::verbs::MutatingVerb`]'s shape deliberately: a consumer-owned +/// table in the one committed authority, keyed by an id the config author picks. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize, schemars::JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct NamedPattern { + /// The name a module references it by — the key under + /// `data.batten.patterns`. + /// + /// Unique across the table. A repeated id would make + /// `data.batten.patterns["x"]` ambiguous, and "which declaration answered + /// me" is not a question a reviewer should have to resolve — the same + /// reasoning [`crate::policy`] applies to two rows registering one module. + pub id: String, + /// The expression itself. + /// + /// Compiled at load by [`validate`], so a malformed pattern is a config + /// fault at exit `1` rather than an evaluation failure on the mediated path + /// — house style §8's placement, and the same one `forbid`'s `regex` column + /// already takes. + pub regex: String, +} + +/// Refuse a malformed pattern table, at load. +/// +/// Two rules, and both are about the table rather than about any predicate that +/// might use it: +/// +/// * an id is non-empty and unique; +/// * the expression compiles. +/// +/// # Errors +/// +/// A [`UsageError`] (exit `1`) naming the offending id. The expression itself is +/// a **declaration** — a literal the config author wrote, the class `config +/// show` exists to echo — so naming it is inside non-negotiable rule 4, and +/// without it a refusal over a malformed pattern could not be acted on. +pub fn validate(patterns: &[NamedPattern]) -> anyhow::Result<()> { + let mut seen: BTreeSet<&str> = BTreeSet::new(); + for pattern in patterns { + if pattern.id.trim().is_empty() { + return Err(UsageError::raise(String::from( + "pattern: `id` cannot be blank — it is the name a module references, \ + and an empty one names nothing", + ))); + } + if !seen.insert(pattern.id.as_str()) { + return Err(UsageError::raise(format!( + "pattern `{}` is declared twice; one concept, one spelling — \ + `data.batten.patterns[\"{}\"]` cannot resolve to two expressions", + pattern.id, pattern.id + ))); + } + regex::Regex::new(&pattern.regex).map_err(|err| { + UsageError::raise(format!( + "pattern `{}`: `{}` is not a valid expression: {err}", + pattern.id, pattern.regex + )) + })?; + } + Ok(()) +} + +/// The table as the evaluator's `data` document carries it. +/// +/// `{"batten": {"patterns": {"": ""}}}` — a flat map, because a +/// module's whole use of it is one lookup by name. +#[must_use] +pub fn data_document(patterns: &[NamedPattern]) -> serde_json::Value { + let mut map = serde_json::Map::new(); + for pattern in patterns { + map.insert( + pattern.id.clone(), + serde_json::Value::from(pattern.regex.as_str()), + ); + } + serde_json::json!({ "batten": { "patterns": serde_json::Value::Object(map) } }) +} diff --git a/crates/batten/src/policy.rs b/crates/batten/src/policy.rs index 1e0cce550..f1f6bc900 100644 --- a/crates/batten/src/policy.rs +++ b/crates/batten/src/policy.rs @@ -531,7 +531,18 @@ pub fn engines_constructed() -> usize { /// faults. Every one of those is a config error at load rather than a surprise /// at the gate, which is the whole reason this function drives a query it throws /// away. -pub fn load(root: &Path, rules: &[Rule], reference: Option<&str>) -> Result> { +pub fn load( + root: &Path, + rules: &[Rule], + patterns: &[crate::pattern::NamedPattern], + reference: Option<&str>, +) -> Result> { + // The table is validated at PARSE, beside `verbs` and `redirects` and for + // their reason (`config.rs`'s `VALIDATED_AT_LOAD` census asserts the call + // site exists). Validating again here would be a second authority for one + // question, which is the shape `rules-drift` exists to refuse. + let pattern_data = crate::pattern::data_document(patterns); + let declared_patterns: BTreeSet<&str> = patterns.iter().map(|p| p.id.as_str()).collect(); let mut bundles = Vec::new(); let mut seen: BTreeSet<&str> = BTreeSet::new(); // Every predicate id published so far, and the module that published it — @@ -607,7 +618,7 @@ pub fn load(root: &Path, rules: &[Rule], reference: Option<&str>) -> Result) -> Result) -> Result) -> Result) -> Result Result { +pub fn compile( + id: &str, + sources: &[(String, String)], + // The declared pattern table as a `data` document (CLOUD-885). Handed in + // rather than read here, so `compile` stays a pure function of what it is + // given and the one authority for the table remains `Config`. + data: &serde_json::Value, +) -> Result { // ONE ENGINE FOR THE WHOLE BUNDLE. `add_policy` once per file into it, so // the bundle compiles once and a helper defined in one module is callable // from another. This is how Conftest and OPA load a policy directory, and // constructing the engine outside the loop is the entire fix: it used to sit // inside it, which is why there was no composed rule set to speak of. let mut engine = new_engine(); + // BEFORE `add_policy`, so a module compiled against the table cannot be + // compiled against an empty one. An unusable data document is a config + // fault at exit 1, not a silent empty map — a module reading + // `data.batten.patterns["x"]` against an absent table gets UNDEFINED, and + // Rego reads undefined as "this rule body does not hold", which is + // CLOUD-251's vacuous pass with a regex in it. + engine + .add_data( + regorus::Value::from_json_str(&data.to_string()).map_err(|err| { + UsageError::raise(format!("the declared pattern table is not usable: {err}")) + })?, + ) + .map_err(|err| { + UsageError::raise(format!( + "the declared pattern table could not be loaded: {err}" + )) + })?; let mut modules = Vec::new(); for (path, source) in sources { engine @@ -1134,6 +1171,96 @@ fn check_tree_paths_are_emittable(rule: &Rule, bundle: &Bundle, source: &str) -> Ok(()) } +/// Refuse a regex written inline in a module (CLOUD-885). +/// +/// **The lever is cost, not prohibition**, and this is the half that applies it. +/// "Do not regex things that are not regular" cannot be a gate — that is a +/// judgement, and non-negotiable rule 3 says a gate resolves to a command and an +/// exit code over an object it decides. Where a pattern LIVES is decidable, so +/// that is what is decided: a regex costs an id and a row in `batten.toml`, +/// while the same question asked of a parsed document costs a field access. The +/// cheap path becomes the correct one without anyone having to reason about +/// regularity. +/// +/// Three things follow, and [`crate::pattern`] carries the argument for each: +/// rule 1 (a tracker-key expression is a consumer identifier and belongs in the +/// consumer's config), duplication becoming unwritable rather than merely +/// detectable, and the pattern inventory becoming reviewable data (§11). +/// +/// **`test_` rules are exempt**, and it is a real case rather than a hatch: a +/// test legitimately matches a declared pattern against a literal subject +/// (`regex.match(patterns.key, "CLOUD-1")`), which this check would otherwise +/// read as an inline pattern. The prefix is the one `policy test` already keys +/// on, so no second convention is introduced. +/// +/// # Errors +/// +/// A [`UsageError`] (exit `1`) naming the module and the remedy (CLOUD-437). +/// **The expression is named**, and that is inside rule 4 rather than an +/// exception to it: a pattern is a declaration the config author wrote — the +/// class `config show` exists to echo — not content read out of a subject file. +fn check_no_inline_regex( + rule: &Rule, + bundle: &Bundle, + declared: &BTreeSet<&str>, + source: &str, +) -> Result<()> { + let Some(described) = describe(&bundle.engine) else { + // Could-not-look on the AST is not a refusal, for the reason the sibling + // checks state: `load` has already compiled this module, so a shape this + // reader does not recognise is its own limitation, and failing the config + // over it would make a reader upgrade a breaking change. + return Ok(()); + }; + // A REFERENCE TO AN UNDECLARED ID IS THE SAME DEFECT ONE STEP LATER, and + // refusing the inline form without refusing this would leave the hole the + // mechanism exists to close: `data.batten.patterns["typo"]` resolves to + // UNDEFINED, Rego reads undefined as "this rule body does not hold", so the + // module loads clean, evaluates clean and gates nothing. That is CLOUD-251's + // vacuous pass, and it is exactly what `WeakeningKind::PatternRemoved` + // describes arriving by a different route — a typo rather than a deletion. + // Same shape as `check_tree_paths_are_emittable`: refuse a reference the + // engine cannot satisfy, at load, against the table rather than a list. + for module in &described { + for rule_ast in &module.rules { + for referenced in &rule_ast.pattern_refs { + if declared.contains(referenced.as_str()) { + continue; + } + let mut known: Vec<&str> = declared.iter().copied().collect(); + known.sort_unstable(); + return Err(UsageError::raise(format!( + "rule `{}` registers `{source}`, whose module {} references \ +`data.batten.patterns[\"{referenced}\"]`, which no `[[pattern]]` row declares — the \ +reference would be undefined and the predicate silent. Declared: {}", + rule.id, + module.path, + if known.is_empty() { + String::from("none") + } else { + known.join(", ") + }, + ))); + } + if rule_ast.name.starts_with("test_") { + continue; + } + let Some(pattern) = rule_ast.inline_regex.first() else { + continue; + }; + return Err(UsageError::raise(format!( + "rule `{}` registers `{source}`, whose module {} writes the regex `{pattern}` \ +inline in `{}`. Declare it once as a `[[pattern]]` row and reference it as \ +`data.batten.patterns[\"\"]`: an expression is a consumer fact, so it belongs in \ +the config rather than in a module (rule 1), and a named pattern has one home, which \ +is what stops one concept acquiring several spellings", + rule.id, module.path, rule_ast.name, + ))); + } + } + Ok(()) +} + /// The keys `rules::tree_document` emits, derived from the fact model. /// /// Named once here so the refusal above and the engine agree by construction @@ -1318,6 +1445,18 @@ struct DescribedRule { /// literals beside it. Paths only: a reference is a NAME, never a value, so /// rule 4 has nothing to say about carrying it. input_paths: Vec, + /// Every string literal this rule hands to a `regex.*` builtin (CLOUD-885). + /// + /// A subset of [`Self::literals`], separated because the question is + /// different: that field binds a predicate id to the rule raising it, this + /// one asks whether a pattern was written inline instead of declared. + inline_regex: Vec, + /// Every `data.batten.patterns[""]` this rule references (CLOUD-885). + /// + /// The ids only. A reference the config does not declare resolves to + /// undefined, which Rego reads as "this rule body does not hold" — so this + /// is what makes a typo a refusal rather than a silent disarm. + pattern_refs: Vec, } /// Read every module's rule names, spans and literals off the compiled AST. @@ -1363,11 +1502,17 @@ fn describe(engine: ®orus::Engine) -> Option> { collect_literals(rule, &mut literals); let mut input_paths = Vec::new(); collect_input_paths(rule, &mut input_paths); + let mut inline_regex = Vec::new(); + collect_inline_regex(rule, &mut inline_regex); + let mut pattern_refs = Vec::new(); + collect_pattern_refs(rule, &mut pattern_refs); rules.push(DescribedRule { name, head_line, literals, input_paths, + inline_regex, + pattern_refs, }); } described.push(Described { @@ -1451,6 +1596,126 @@ fn collect_input_paths(value: &serde_json::Value, found: &mut Vec) { } } +/// Every string literal a rule hands to a `regex.*` builtin (CLOUD-885). +/// +/// A call is `{"Call": {"fcn": , "params": [, …]}}`, and `fcn` +/// resolves through [`reference_path`] exactly as an input reference does — +/// `regex.match` is a `RefDot` over the var `regex`. +/// +/// **Every parameter, and the recursion into each is what closes the hole.** +/// The builtins disagree on argument order — `regex.match(pattern, value)` +/// against `regex.replace(s, pattern, value)` — so a per-builtin position table +/// would be a second thing to keep in step with upstream. Reading them all is +/// wider, and the width is load-bearing rather than lazy: recursing means +/// `regex.match(concat("", ["CLOUD", "-[0-9]+"]), s)` is caught too, which a +/// direct-parameter check would wave through. +/// +/// A reference to `data.batten.patterns["x"]` carries no literal, so the +/// sanctioned form passes by construction rather than by exemption. +fn collect_inline_regex(value: &serde_json::Value, found: &mut Vec) { + match value { + serde_json::Value::Object(object) => { + if let Some(call) = object.get("Call") + && let Some(name) = call.get("fcn").and_then(reference_path) + && name.starts_with("regex.") + && let Some(params) = call.get("params").and_then(serde_json::Value::as_array) + { + for param in params { + // A REFERENCE IS NOT A LITERAL, even though it contains one. + // `data.batten.patterns["x"]` is a `RefBrack` whose index is + // the string `"x"`, so a naive literal sweep reads the + // sanctioned form as the refused one and no module can load + // at all. Skipping the subtree is right rather than + // convenient: the id is a NAME, and the expression it names + // lives in the config, which is the property being enforced. + if reference_path(param) + .is_some_and(|path| path.starts_with("data.batten.patterns.")) + { + continue; + } + // BOTH SPELLINGS. A Rego backtick literal serialises as + // `RawString`, not `String`, and it is the spelling a regex + // is almost always written in — backticks are what let a + // pattern carry backslashes unescaped. Reading only + // `collect_literals` let every realistic inline pattern + // through, which is what the AST probe measured rather than + // what this reader assumed. + collect_string_values(param, "String", found); + collect_string_values(param, "RawString", found); + } + } + for child in object.values() { + collect_inline_regex(child, found); + } + } + serde_json::Value::Array(items) => { + for item in items { + collect_inline_regex(item, found); + } + } + _ => {} + } +} + +/// Every pattern id a rule reaches through `data.batten.patterns[…]` +/// (CLOUD-885). +/// +/// [`reference_path`] already resolves a string-literal index, so +/// `data.batten.patterns["x"]` arrives as the dotted path +/// `data.batten.patterns.x` and the id is its last segment. A VARIABLE index is +/// deliberately not resolved — that path is not statically knowable, and +/// answering `None` for it is could-not-look rather than a guess, which is the +/// same posture `reference_path` already takes. +fn collect_pattern_refs(value: &serde_json::Value, found: &mut Vec) { + const PREFIX: &str = "data.batten.patterns."; + match value { + serde_json::Value::Object(object) => { + if let Some(path) = reference_path(value) + && let Some(rest) = path.strip_prefix(PREFIX) + && !rest.is_empty() + { + found.push(rest.split('.').next().unwrap_or(rest).to_owned()); + } + for child in object.values() { + collect_pattern_refs(child, found); + } + } + serde_json::Value::Array(items) => { + for item in items { + collect_pattern_refs(item, found); + } + } + _ => {} + } +} + +/// Every `{"": {"value": "…"}}` node in a subtree, for one node kind. +/// +/// Rego has two literal spellings and regorus gives them different nodes: +/// `"x"` is a `String` and `` `x` `` is a `RawString`. A reader that knows only +/// one of them is blind to the other, and for a regex the backtick form is the +/// usual one, since it carries backslashes unescaped. +fn collect_string_values(value: &serde_json::Value, kind: &str, found: &mut Vec) { + match value { + serde_json::Value::Object(object) => { + if let Some(node) = object.get(kind) + && let Some(text) = node.get("value").and_then(serde_json::Value::as_str) + { + found.push(text.to_owned()); + } + for child in object.values() { + collect_string_values(child, kind, found); + } + } + serde_json::Value::Array(items) => { + for item in items { + collect_string_values(item, kind, found); + } + } + _ => {} + } +} + /// Every string literal in a rule's AST subtree. fn collect_literals(value: &serde_json::Value, found: &mut Vec) { match value { diff --git a/crates/batten/src/resolve.rs b/crates/batten/src/resolve.rs index eb8364868..14e91a839 100644 --- a/crates/batten/src/resolve.rs +++ b/crates/batten/src/resolve.rs @@ -294,6 +294,10 @@ pub struct Resolved { /// The mutating-verb table, consumer data the authority supplies. #[serde(rename = "verb")] pub verbs: Vec, + /// The named-regex table (CLOUD-885), consumer data the authority supplies — + /// carried for [`Resolved::verbs`]'s reason and layered the same way. + #[serde(rename = "pattern")] + pub patterns: Vec, /// The per-path-class redirect table (CLOUD-280), authority rows plus any a /// local file **added**. Local rows append after committed ones, and the /// lookup takes the first match, so an uncommitted file can add a class the @@ -1146,6 +1150,7 @@ fn assemble( unlanded: paths.unlanded, epoch: repo.epoch.clone(), verbs: repo.verbs.clone(), + patterns: repo.patterns.clone(), redirects: tables.redirects, facts: tables.facts, markers: repo.markers.clone(), @@ -1207,6 +1212,7 @@ fn attribution( ("unlanded", paths.unlanded_source.clone()), ("epoch", authority_set(repo.epoch.is_some())), ("verb", authority_set(!repo.verbs.is_empty())), + ("pattern", authority_set(!repo.patterns.is_empty())), ("marker", authority_set(!repo.markers.is_empty())), ( "exec_pattern", diff --git a/crates/batten/src/rules.rs b/crates/batten/src/rules.rs index da2946414..ecfb1575c 100644 --- a/crates/batten/src/rules.rs +++ b/crates/batten/src/rules.rs @@ -2790,6 +2790,9 @@ pub const SPAWNING_VERB: &str = "batten enforce"; pub fn run_static( rules: &[Rule], _provisions: &[crate::provision::Provision], + // The declared pattern table (CLOUD-885), riding beside `provisions` for the + // same reason it does: a second config table the rule set evaluates against. + patterns: &[crate::pattern::NamedPattern], root: &Path, ) -> anyhow::Result { // POLICY BUNDLES ARE LOADED HERE, on the read surface, and that is @@ -2801,7 +2804,7 @@ pub fn run_static( // process or reach the network, a property CLOUD-831 gates rather than // asserts. So admitting it here makes `check` MORE capable without making it // less honest, and the spawning refusal below is untouched. - let bundles = crate::policy::load(root, rules, None)?; + let bundles = crate::policy::load(root, rules, patterns, None)?; // Refuse before any work: the read-only surface must not even begin a run // it cannot complete honestly. for rule in rules { @@ -2862,13 +2865,14 @@ pub fn run_static( pub fn run_recorded( rules: &[Rule], provisions: &[crate::provision::Provision], + patterns: &[crate::pattern::NamedPattern], root: &Path, ) -> anyhow::Result { let (evaluable, withheld): (Vec<&Rule>, Vec<&Rule>) = rules .iter() .partition(|rule| !rule.kind.carries_ambient_authority()); let evaluable: Vec = evaluable.into_iter().cloned().collect(); - let bundles = crate::policy::load(root, &evaluable, None)?; + let bundles = crate::policy::load(root, &evaluable, patterns, None)?; let mut scan = run(&evaluable, provisions, root, &bundles)?; for rule in withheld { // `RuleSkipped`, not a variant of its own. The distinction between "the @@ -2898,6 +2902,7 @@ pub fn run_recorded( pub fn run_all( rules: &[Rule], provisions: &[crate::provision::Provision], + patterns: &[crate::pattern::NamedPattern], root: &Path, ) -> anyhow::Result { // Refuse before any work, the shape `run_static` above already uses: the @@ -2913,7 +2918,7 @@ pub fn run_all( ))); } } - let bundles = crate::policy::load(root, rules, None)?; + let bundles = crate::policy::load(root, rules, patterns, None)?; run(rules, provisions, root, &bundles) } @@ -5844,11 +5849,11 @@ mod tests { /// found; [`Scan::not_evaluated`] has its own tests, so shadowing keeps the /// suite reading as it did before that half existed. fn run_static(rules: &[Rule], root: &Path) -> anyhow::Result> { - super::run_static(rules, &[], root).map(|scan| scan.findings) + super::run_static(rules, &[], &[], root).map(|scan| scan.findings) } fn run_all(rules: &[Rule], root: &Path) -> anyhow::Result> { - super::run_all(rules, &[], root).map(|scan| scan.findings) + super::run_all(rules, &[], &[], root).map(|scan| scan.findings) } fn forbid(id: &str, glob: &str, pattern: &str) -> Rule { @@ -6022,7 +6027,8 @@ mod tests { let dir = temp_dir("scan-skipped"); write(&dir, "src/a.rs", "fine\n"); - let clean = super::run_static(&[forbid("looked", "**/*.rs", "TODO")], &[], &dir).unwrap(); + let clean = + super::run_static(&[forbid("looked", "**/*.rs", "TODO")], &[], &[], &dir).unwrap(); assert!(clean.findings.is_empty()); assert!( clean.not_evaluated.is_empty(), @@ -6030,7 +6036,8 @@ mod tests { ); let skipped = - super::run_static(&[forbid("never-looked", "**/*.md", "TODO")], &[], &dir).unwrap(); + super::run_static(&[forbid("never-looked", "**/*.md", "TODO")], &[], &[], &dir) + .unwrap(); assert!(skipped.findings.is_empty()); assert_eq!( skipped.not_evaluated.get("never-looked"), @@ -6046,6 +6053,7 @@ mod tests { ..forbid("switched-off", "**/*.rs", "TODO") }], &[], + &[], &dir, ) .unwrap(); diff --git a/crates/batten/src/trust.rs b/crates/batten/src/trust.rs index 1b90efe40..ceae9439a 100644 --- a/crates/batten/src/trust.rs +++ b/crates/batten/src/trust.rs @@ -540,6 +540,16 @@ pub enum WeakeningKind { /// A `[[verb]]` row is gone, so a mutating tool call is no longer mediated /// at the `PreToolUse` boundary (CLOUD-36). VerbRemoved, + /// A `[[pattern]]` row is gone, so every module referencing it resolves to + /// **undefined** (CLOUD-885). + /// + /// **This is a silent disarm rather than a load failure**, which is what + /// makes it belong on this table. Rego reads an undefined reference as "this + /// rule body does not hold", so a module whose pattern was deleted loads + /// clean, evaluates clean, and gates nothing — CLOUD-251's vacuous pass, + /// reachable from a local override. Removing one row can silence several + /// predicates at once, since a declared pattern is shared by design. + PatternRemoved, /// An agent-sourced fact's declared command changed (CLOUD-776). /// /// The one weakening on this table whose payoff is a FORGED FACT rather than @@ -637,6 +647,7 @@ impl WeakeningKind { WeakeningKind::MinVersionLowered, WeakeningKind::EpochPathRemoved, WeakeningKind::VerbRemoved, + WeakeningKind::PatternRemoved, WeakeningKind::FactCommandChanged, WeakeningKind::FactRemoved, WeakeningKind::MarkerRemoved, @@ -676,6 +687,7 @@ impl WeakeningKind { WeakeningKind::MinVersionLowered => "min-version-lowered", WeakeningKind::EpochPathRemoved => "epoch-path-removed", WeakeningKind::VerbRemoved => "verb-removed", + WeakeningKind::PatternRemoved => "pattern-removed", WeakeningKind::FactCommandChanged => "fact-command-changed", WeakeningKind::FactRemoved => "fact-removed", WeakeningKind::MarkerRemoved => "marker-removed", @@ -788,6 +800,10 @@ pub const CENSUS: &[FieldCoverage] = &[ field: "verbs", coverage: Coverage::Compared(&[WeakeningKind::VerbRemoved]), }, + FieldCoverage { + field: "patterns", + coverage: Coverage::Compared(&[WeakeningKind::PatternRemoved]), + }, FieldCoverage { field: "facts", coverage: Coverage::Compared(&[ @@ -1083,6 +1099,16 @@ fn entry_weakenings(base: &Config, working: &Config) -> Vec { "verb", )); + // The named-regex table (CLOUD-885): removing a row does not fail a load, it + // makes every reference to it undefined, and Rego reads undefined as "does + // not hold". So the predicates go quiet rather than red. + found.extend(removed_entries( + WeakeningKind::PatternRemoved, + &pattern_entries(base), + &pattern_entries(working), + "pattern", + )); + // The agent-sourced facts (CLOUD-776). Removal is reported and is a // tightening; a CHANGED command is the dangerous direction, because the same // string is both what the agent is told to run and what the record is checked @@ -1253,6 +1279,14 @@ fn verb_entries(config: &Config) -> Vec { .collect() } +/// The declared pattern ids, so [`removed_entries`] can compare them. +/// +/// The **id** and never the expression: a removal is identified by the name a +/// module references, which is also what keeps this a pointer (rule 4). +fn pattern_entries(config: &Config) -> Vec { + config.patterns.iter().map(|row| row.id.clone()).collect() +} + /// The ids of a table, collected so [`removed_entries`] can compare them. fn ids(entries: impl Iterator) -> Vec { entries.collect() @@ -2259,6 +2293,34 @@ mod tests { assert!(weakenings(&working, &base).is_empty()); } + #[test] + fn removing_a_declared_pattern_is_a_weakening() { + // NOT A LOAD FAILURE, which is the whole reason this is on the table. + // Every module referencing a deleted pattern resolves to UNDEFINED, and + // Rego reads undefined as "this rule body does not hold" — so the + // predicates go quiet rather than red, and one removed row can silence + // several at once, because a declared pattern is shared by design + // (CLOUD-885). That is CLOUD-251's vacuous pass reachable from a local + // override. + let base = config( + "\n[[pattern]]\nid = \"tracker-key\"\nregex = \"CLOUD-[0-9]+\"\n\n\ + [[pattern]]\nid = \"sha\"\nregex = \"[0-9a-f]{7,40}\"\n", + ); + let working = config("\n[[pattern]]\nid = \"tracker-key\"\nregex = \"CLOUD-[0-9]+\"\n"); + assert_eq!( + only(&base, &working), + Weakening::new( + WeakeningKind::PatternRemoved, + "pattern[sha]", + "present", + "absent", + ) + ); + // The other direction is a tightening: adding a pattern arms nothing on + // its own, since a module has to reference it. + assert!(weakenings(&working, &base).is_empty()); + } + fn verb_row(verb: &str, subcommand: Option<&str>) -> String { let qualifier = subcommand.map_or_else(String::new, |sub| format!("subcommand = \"{sub}\"\n")); diff --git a/crates/batten/tests/document_read_count.rs b/crates/batten/tests/document_read_count.rs index db0e6178c..1e71fe127 100644 --- a/crates/batten/tests/document_read_count.rs +++ b/crates/batten/tests/document_read_count.rs @@ -95,6 +95,7 @@ fn rows_declaring_one_path_read_it_once() { row("third", &["config.toml"]), ], &[], + &[], &root, ) .expect("the read surface runs the rows"); @@ -117,7 +118,7 @@ fn rows_declaring_one_path_read_it_once() { let before = rules::documents_acquired(); fs::write(root.join("other.toml"), "stray = true\n").expect("fixture"); write_bundles(&root, &["fourth"]); - let _ = rules::run_static(&[row("fourth", &["other.toml"])], &[], &root); + let _ = rules::run_static(&[row("fourth", &["other.toml"])], &[], &[], &root); assert!( rules::documents_acquired() > before, "the counter moves for a path not already cached, so the delta above \ @@ -148,7 +149,7 @@ fn a_glob_source_resolves_against_the_walk_rather_than_being_read_literally() { })) .expect("a row declaring a selector"); - let scan = rules::run_static(&[globbed], &[], &root).expect("the selector resolves"); + let scan = rules::run_static(&[globbed], &[], &[], &root).expect("the selector resolves"); assert_eq!( scan.findings.len(), 1, diff --git a/crates/batten/tests/identity_churn.rs b/crates/batten/tests/identity_churn.rs index 9d60919b4..e9a080c21 100644 --- a/crates/batten/tests/identity_churn.rs +++ b/crates/batten/tests/identity_churn.rs @@ -76,7 +76,7 @@ impl Scan { /// written against the whole matched line, and they still pass, so the /// engine demonstrably picks the same span the test used to. fn of(root: &Path, rules: &[Rule]) -> Self { - let findings = rules::run_static(rules, &[], root) + let findings = rules::run_static(rules, &[], &[], root) .expect("scan the tree") .findings; let identities = identity::count_occurrences( diff --git a/crates/batten/tests/policy_engine_count.rs b/crates/batten/tests/policy_engine_count.rs index 902911f2e..005187831 100644 --- a/crates/batten/tests/policy_engine_count.rs +++ b/crates/batten/tests/policy_engine_count.rs @@ -41,7 +41,8 @@ fn a_bundle_of_n_modules_constructs_exactly_one_engine() { .collect(); let before = policy::engines_constructed(); - let bundle = policy::compile("policy-many", &sources).expect("five modules, one bundle"); + let bundle = policy::compile("policy-many", &sources, &serde_json::json!({})) + .expect("five modules, one bundle"); let after = policy::engines_constructed(); assert_eq!( diff --git a/crates/batten/tests/policy_modules.rs b/crates/batten/tests/policy_modules.rs index 8261bf01c..a876b1fd6 100644 --- a/crates/batten/tests/policy_modules.rs +++ b/crates/batten/tests/policy_modules.rs @@ -223,7 +223,7 @@ fn scratch(name: &str) -> std::path::PathBuf { fn a_module_denies_on_a_fact_and_is_silent_otherwise() { let root = scratch("denies"); let path = module_file(&root, "writes.rego", DENIES_WRITES); - let bundles = policy::load(&root, &[row("policy-writes", &path)], None).expect("load"); + let bundles = policy::load(&root, &[row("policy-writes", &path)], &[], None).expect("load"); assert_eq!(bundles.len(), 1); let denied = policy::deny(&bundles[0], r#"{"call":{"operation":"write"}}"#); @@ -248,7 +248,7 @@ fn a_module_denies_on_a_fact_and_is_silent_otherwise() { fn an_unparseable_input_is_could_not_look_and_never_an_empty_deny_set() { let root = scratch("couldnotlook"); let path = module_file(&root, "writes.rego", DENIES_WRITES); - let bundles = policy::load(&root, &[row("policy-writes", &path)], None).expect("load"); + let bundles = policy::load(&root, &[row("policy-writes", &path)], &[], None).expect("load"); let answer = policy::deny(&bundles[0], "{not json"); assert!( @@ -265,7 +265,7 @@ fn a_cyclic_module_is_refused_at_load() { // in `load`, this module compiles clean here and faults at the gate. let root = scratch("cyclic"); let path = module_file(&root, "cyclic.rego", CYCLIC); - let err = policy::load(&root, &[row("policy-cyclic", &path)], None) + let err = policy::load(&root, &[row("policy-cyclic", &path)], &[], None) .expect_err("a cycle is a config error, not a runtime surprise"); let text = format!("{err}"); assert!( @@ -288,7 +288,7 @@ fn a_cyclic_module_is_refused_at_load() { #[test] fn a_module_that_cannot_be_read_is_refused_at_load() { let root = scratch("absent"); - let err = policy::load(&root, &[row("policy-absent", "nowhere.rego")], None) + let err = policy::load(&root, &[row("policy-absent", "nowhere.rego")], &[], None) .expect_err("a registration naming no file decides nothing and must not load"); assert!(format!("{err}").contains("nowhere.rego")); } @@ -300,11 +300,109 @@ fn two_rows_registering_one_module_are_refused_at_load() { // answer. Same reasoning as the duplicate derived-value name (CLOUD-773). let root = scratch("duplicate"); let path = module_file(&root, "writes.rego", DENIES_WRITES); - let err = policy::load(&root, &[row("first", &path), row("second", &path)], None) - .expect_err("one module, two registrations"); + let err = policy::load( + &root, + &[row("first", &path), row("second", &path)], + &[], + None, + ) + .expect_err("one module, two registrations"); assert!(format!("{err}").contains("already registers")); } +/// A pattern written inline, which is what a bash translation reaches for. +const INLINE_REGEX: &str = r#" +package batten + +import rego.v1 + +deny contains "names a tracker key" if { + regex.match(`CLOUD-[0-9]+`, input.call.command) +} +"#; + +/// The same predicate against a declared pattern — the sanctioned form. +const DECLARED_REGEX: &str = r#" +package batten + +import rego.v1 + +deny contains "names a tracker key" if { + regex.match(data.batten.patterns["tracker-key"], input.call.command) +} +"#; + +/// A literal smuggled past a direct-parameter check by building it up. +const SMUGGLED_REGEX: &str = r#" +package batten + +import rego.v1 + +deny contains "names a tracker key" if { + regex.match(concat("", ["CLOUD", "-[0-9]+"]), input.call.command) +} +"#; + +fn tracker_key() -> batten::pattern::NamedPattern { + batten::pattern::NamedPattern { + id: String::from("tracker-key"), + regex: String::from("CLOUD-[0-9]+"), + } +} + +#[test] +fn a_regex_written_inline_is_refused_and_a_declared_one_decides() { + // THE LEVER IS COST, NOT PROHIBITION. "Do not regex things that are not + // regular" cannot be a gate — that is a judgement, and non-negotiable rule 3 + // says a gate resolves to a command and an exit code over an object it + // decides. Where a pattern LIVES is decidable, so a regex costs an id and a + // config row while the same question over a parsed document costs a field + // access. The cheap path becomes the correct one without anyone reasoning + // about regularity. + let root = scratch("inline-regex"); + let inline = module_file(&root, "inline.rego", INLINE_REGEX); + let err = policy::load(&root, &[row("keys", &inline)], &[], None) + .expect_err("a regex written inline must not load"); + let rendered = format!("{err}"); + assert!( + rendered.contains("CLOUD-[0-9]+"), + "the refusal names the expression, a declaration and not content: {rendered}" + ); + assert!( + rendered.contains("[[pattern]]"), + "the refusal names its remedy (CLOUD-437): {rendered}" + ); + + // THE DISCRIMINATING HALF, and it is two claims rather than one. A check + // that refused every `regex.*` call would satisfy the assertions above and + // be useless, so the declared form must LOAD — and it must also DECIDE, + // because a module whose pattern resolved to undefined would load clean and + // gate nothing, which is CLOUD-251's vacuous pass with a regex in it. So + // this asserts the deny, not merely the load. + let declared = module_file(&root, "declared.rego", DECLARED_REGEX); + let bundles = policy::load(&root, &[row("keys", &declared)], &[tracker_key()], None) + .expect("a declared pattern is the sanctioned form"); + let doc = r#"{"call":{"command":"git commit -m CLOUD-885"}}"#; + let Look::Is(violations) = policy::deny(&bundles[0], doc) else { + panic!("the declared pattern must reach the module through `data`"); + }; + assert_eq!( + violations, + vec![unattributed("names a tracker key")], + "the projected table decides, rather than resolving to undefined" + ); + + // THE SMUGGLE. A check reading only direct parameters would wave this + // through; recursing into each parameter is what closes it. + let smuggled = module_file(&root, "smuggled.rego", SMUGGLED_REGEX); + let err = policy::load(&root, &[row("keys", &smuggled)], &[], None) + .expect_err("a literal assembled inside the call is still a literal"); + assert!( + format!("{err}").contains("[[pattern]]"), + "the smuggled form gets the same refusal: {err}" + ); +} + #[test] fn a_module_holds_no_source_and_cannot_leak_one_through_debug() { // Rule 4 is structural here rather than careful: `Module` has no `source` @@ -312,7 +410,7 @@ fn a_module_holds_no_source_and_cannot_leak_one_through_debug() { // re-derived it. This asserts the rendering, which is the reachable half. let root = scratch("pointer"); let path = module_file(&root, "writes.rego", DENIES_WRITES); - let bundles = policy::load(&root, &[row("policy-writes", &path)], None).expect("load"); + let bundles = policy::load(&root, &[row("policy-writes", &path)], &[], None).expect("load"); let rendered = format!("{:?}", bundles[0]); assert!(rendered.contains("policy-writes"), "the pointer is present"); assert!( @@ -369,7 +467,7 @@ fn no_evaluator_feature_admits_io() { // The control first. If this does not deny, nothing below discriminates. let included = module_file(&root, "included.rego", REACHES_AN_INCLUDED_BUILTIN); - let bundles = policy::load(&root, &[row("policy-included", &included)], None) + let bundles = policy::load(&root, &[row("policy-included", &included)], &[], None) .expect("a module over an in-closure builtin loads"); assert_eq!( policy::deny(&bundles[0], "{}"), @@ -385,7 +483,7 @@ fn no_evaluator_feature_admits_io() { // property the doc claims; both arms are accepted here and the assertion is // over the outcome that matters. let network = module_file(&root, "network.rego", REACHES_THE_NETWORK); - match policy::load(&root, &[row("policy-network", &network)], None) { + match policy::load(&root, &[row("policy-network", &network)], &[], None) { Err(refused) => { let text = format!("{refused}"); assert!( @@ -409,7 +507,7 @@ fn no_evaluator_feature_admits_io() { // manifest pins out. Same shape: a test covering one of the two would report // the pin held while half of it drifted. let schema = module_file(&root, "schema.rego", REACHES_JSONSCHEMA); - match policy::load(&root, &[row("policy-schema", &schema)], None) { + match policy::load(&root, &[row("policy-schema", &schema)], &[], None) { Err(refused) => { let text = format!("{refused}"); assert!( @@ -439,7 +537,7 @@ fn no_evaluator_feature_admits_io() { fn one_module_carries_two_predicates_that_deny_under_their_own_ids() { let root = scratch("two-predicates"); let path = module_file(&root, "two.rego", TWO_PREDICATES); - let bundles = policy::load(&root, &[row("policy-two", &path)], None).expect("load"); + let bundles = policy::load(&root, &[row("policy-two", &path)], &[], None).expect("load"); let denied = policy::deny(&bundles[0], r#"{"call":{"operation":"write"}}"#); let Look::Is(violations) = denied else { @@ -492,7 +590,7 @@ fn one_module_carries_two_predicates_that_deny_under_their_own_ids() { fn a_bare_string_deny_still_reports_under_the_registering_row() { let root = scratch("bare-string"); let path = module_file(&root, "writes.rego", DENIES_WRITES); - let bundles = policy::load(&root, &[row("policy-writes", &path)], None).expect("load"); + let bundles = policy::load(&root, &[row("policy-writes", &path)], &[], None).expect("load"); assert!( bundles[0].declared().is_empty(), @@ -523,7 +621,7 @@ fn a_bare_string_deny_still_reports_under_the_registering_row() { fn an_undeclared_violation_id_is_refused_at_load() { let root = scratch("undeclared"); let path = module_file(&root, "undeclared.rego", UNDECLARED_ID); - let err = policy::load(&root, &[row("policy-undeclared", &path)], None) + let err = policy::load(&root, &[row("policy-undeclared", &path)], &[], None) .expect_err("an id the module does not publish cannot be attributed"); let text = format!("{err}"); assert!( @@ -556,6 +654,7 @@ fn two_modules_declaring_one_id_are_refused_at_load() { let err = policy::load( &root, &[row("policy-a", &first), row("policy-b", &second)], + &[], None, ) .expect_err("one id, two publishers"); @@ -585,7 +684,7 @@ fn two_modules_declaring_one_id_are_refused_at_load() { fn a_waiver_over_one_predicate_does_not_suppress_its_sibling() { let root = scratch("waiver-sibling"); let path = module_file(&root, "two.rego", TWO_PREDICATES); - let bundles = policy::load(&root, &[row("policy-two", &path)], None).expect("load"); + let bundles = policy::load(&root, &[row("policy-two", &path)], &[], None).expect("load"); let Look::Is(violations) = policy::deny(&bundles[0], r#"{"call":{"operation":"write"}}"#) else { @@ -636,7 +735,7 @@ violation contains {"rule": "only-on-a-write", "msg": "reached later"} if { } "#; let path = module_file(&root, "late.rego", source); - let bundles = policy::load(&root, &[row("policy-late", &path)], None) + let bundles = policy::load(&root, &[row("policy-late", &path)], &[], None) .expect("load cannot reach this violation, so it loads"); let answer = policy::deny(&bundles[0], r#"{"call":{"operation":"write"}}"#); @@ -668,7 +767,7 @@ fn a_predicate_severity_naming_an_unpublished_id_is_refused_at_load() { .into_iter() .collect(), ); - let err = policy::load(&root, &[rule], None) + let err = policy::load(&root, &[rule], &[], None) .expect_err("a severity aimed at an id nothing publishes decides nothing"); let text = format!("{err}"); assert!(text.contains("no-such-predicate"), "names the key: {text}"); @@ -692,7 +791,7 @@ fn severity_resolves_per_predicate_and_falls_back_to_the_row() { .into_iter() .collect(), ); - let bundles = policy::load(&root, &[rule.clone()], None).expect("load"); + let bundles = policy::load(&root, &[rule.clone()], &[], None).expect("load"); assert_eq!( rule.severity_for(Some("no-stray-artifact")), @@ -747,8 +846,8 @@ violation contains {"rule": "no-force-push", "msg": "a force push at the trunk"} } "#; let path = module_file(&root, "git.rego", source); - let bundles = - policy::load(&root, &[row("policy-git", &path)], None).expect("a sub-package module loads"); + let bundles = policy::load(&root, &[row("policy-git", &path)], &[], None) + .expect("a sub-package module loads"); assert!( bundles[0].declared().contains("no-force-push"), @@ -812,6 +911,7 @@ violation contains {"rule": "no-protected-write", "msg": "a write under a protec ("shared.rego".to_owned(), helper.to_owned()), ("consumer.rego".to_owned(), consumer.to_owned()), ], + &serde_json::json!({}), ) .expect("two modules compose into one rule set"); @@ -876,6 +976,7 @@ fn a_bundle_of_n_modules_is_one_engine_by_construction() { "package batten.c\nimport rego.v1\nrules contains \"c\"\n".to_owned(), ), ], + &serde_json::json!({}), ) .expect("three modules, one bundle"); diff --git a/crates/batten/tests/policy_presets.rs b/crates/batten/tests/policy_presets.rs index fce2da8f8..eeaa045e4 100644 --- a/crates/batten/tests/policy_presets.rs +++ b/crates/batten/tests/policy_presets.rs @@ -54,7 +54,7 @@ fn scratch(name: &str) -> PathBuf { #[test] fn a_preset_predicate_denies_and_is_green_by_turns() { let root = scratch("denies"); - let bundles = policy::load(&root, &[preset_row("trunk", "trunk-based")], None) + let bundles = policy::load(&root, &[preset_row("trunk", "trunk-based")], &[], None) .expect("a vendored preset loads"); let Look::Is(violations) = policy::deny(&bundles[0], &call("git push --force origin topic")) @@ -92,7 +92,7 @@ fn a_preset_predicate_denies_and_is_green_by_turns() { #[test] fn the_commit_hygiene_preset_decides_both_ways() { let root = scratch("hygiene"); - let bundles = policy::load(&root, &[preset_row("hygiene", "commit-hygiene")], None) + let bundles = policy::load(&root, &[preset_row("hygiene", "commit-hygiene")], &[], None) .expect("the preset loads"); let Look::Is(violations) = policy::deny(&bundles[0], &call("git commit --allow-empty -m x")) @@ -114,7 +114,7 @@ fn the_commit_hygiene_preset_decides_both_ways() { #[test] fn an_unknown_preset_name_is_refused_at_load() { let root = scratch("unknown"); - let err = policy::load(&root, &[preset_row("typo", "trunk-basd")], None) + let err = policy::load(&root, &[preset_row("typo", "trunk-basd")], &[], None) .expect_err("a name this binary does not ship"); let text = format!("{err}"); assert!( @@ -131,7 +131,7 @@ fn an_unknown_preset_name_is_refused_at_load() { #[test] fn enabling_no_preset_yields_no_preset_predicates() { let root = scratch("opt-in"); - let bundles = policy::load(&root, &[], None).expect("no rows, no bundles"); + let bundles = policy::load(&root, &[], &[], None).expect("no rows, no bundles"); assert!( bundles.is_empty(), "a consumer who enables nothing gets nothing, which is what keeps this \ @@ -163,8 +163,13 @@ fn a_preset_id_colliding_with_an_in_repo_id_is_refused_at_load() { })) .expect("an in-repo row"); - let err = policy::load(&root, &[preset_row("trunk", "trunk-based"), mine], None) - .expect_err("one id, two publishers across the boundary"); + let err = policy::load( + &root, + &[preset_row("trunk", "trunk-based"), mine], + &[], + None, + ) + .expect_err("one id, two publishers across the boundary"); let text = format!("{err}"); assert!(text.contains("no-force-push"), "names the id: {text}"); assert!( @@ -184,7 +189,7 @@ fn every_advertised_preset_name_actually_loads() { let names = policy::preset_names(); assert!(!names.is_empty(), "the binary ships at least one preset"); for name in names { - policy::load(&root, &[preset_row("row", name)], None) + policy::load(&root, &[preset_row("row", name)], &[], None) .unwrap_or_else(|err| panic!("the advertised preset `{name}` does not load: {err}")); } } @@ -269,7 +274,7 @@ fn presets_are_inside_the_rule_one_glob() { fn every_shipped_preset_publishes_its_ids() { let root = scratch("published"); for name in policy::preset_names() { - let bundles = policy::load(&root, &[preset_row("row", name)], None).expect("loads"); + let bundles = policy::load(&root, &[preset_row("row", name)], &[], None).expect("loads"); assert!( !bundles[0].declared().is_empty(), "the preset `{name}` publishes no rule id, so nothing it denies could \ @@ -295,7 +300,7 @@ fn every_shipped_preset_publishes_its_ids() { fn every_shipped_preset_passes_its_own_suite() { let root = scratch("suites"); for name in policy::preset_names() { - let bundles = policy::load(&root, &[preset_row("row", name)], None).expect("loads"); + let bundles = policy::load(&root, &[preset_row("row", name)], &[], None).expect("loads"); let Look::Is(suite) = policy::test(&bundles[0], "{}").expect("the suite runs") else { panic!("the preset `{name}` has a suite that could not run at all"); }; diff --git a/crates/batten/tests/policy_test_suite.rs b/crates/batten/tests/policy_test_suite.rs index 926bef6e7..74b1f64a3 100644 --- a/crates/batten/tests/policy_test_suite.rs +++ b/crates/batten/tests/policy_test_suite.rs @@ -61,8 +61,12 @@ fn row(id: &str, module: &str) -> Rule { /// Compile `source` as a single-module bundle and run its suite. fn suite_of(source: &str) -> Suite { - let bundle = policy::compile("fixture", &[("fixture.rego".to_owned(), source.to_owned())]) - .expect("the fixture compiles"); + let bundle = policy::compile( + "fixture", + &[("fixture.rego".to_owned(), source.to_owned())], + &serde_json::json!({}), + ) + .expect("the fixture compiles"); match policy::test(&bundle, "{}").expect("the suite runs") { Look::Is(suite) => suite, Look::IsNot | Look::CouldNotLook => panic!("the suite did not run"), @@ -471,7 +475,7 @@ fn a_registered_module_with_tests_still_loads_and_denies() { // only surface at a mediated call. let dir = Fixture::new("policy-test-still-denies").build(); fs::write(dir.join("probe.rego"), CORRECT).expect("write module"); - let bundles = policy::load(Path::new(&dir), &[row("probe", "probe.rego")], None) + let bundles = policy::load(Path::new(&dir), &[row("probe", "probe.rego")], &[], None) .expect("the bundle loads"); let bundle = bundles.first().expect("one bundle"); let Look::Is(violations) = policy::deny(bundle, r#"{"call": {"command": "git push --force"}}"#) diff --git a/crates/batten/tests/policy_tree.rs b/crates/batten/tests/policy_tree.rs index 6333e238b..65337b747 100644 --- a/crates/batten/tests/policy_tree.rs +++ b/crates/batten/tests/policy_tree.rs @@ -67,7 +67,7 @@ fn write_bundle(root: &Path, source: &str) { } fn scan(root: &Path, rules: &[Rule]) -> rules::Scan { - rules::run_static(rules, &[], root).expect("the read surface runs a policy row") + rules::run_static(rules, &[], &[], root).expect("the read surface runs a policy row") } /// (a) A tree-scoped bundle denies on a fixture that violates. @@ -186,7 +186,7 @@ fn check_still_refuses_a_spawning_kind() { })) .expect("a command row"); - let err = rules::run_static(&[command], &[], &root) + let err = rules::run_static(&[command], &[], &[], &root) .expect_err("a read-effect verb will not run a configured command"); let text = format!("{err}"); assert!( @@ -242,6 +242,7 @@ fn an_empty_bundle_root_is_refused_at_load() { let err = rules::run_static( &[tree_row("repo-policy", "policy/", &["config.toml"])], &[], + &[], &root, ) .expect_err("a folder with no modules enables nothing"); @@ -381,7 +382,7 @@ violation contains {"rule": "reads-a-ghost", "msg": "x"} if { "#, ); - let err = batten::policy::load(&root, &[tree_row("repo-policy", "policy/", &[])], None) + let err = batten::policy::load(&root, &[tree_row("repo-policy", "policy/", &[])], &[], None) .expect_err("a module reading a key the engine cannot emit is refused at load"); let message = format!("{err}"); assert!( @@ -428,7 +429,7 @@ violation contains {"rule": "reads-real-keys", "msg": "z"} if { "#, ); - batten::policy::load(&root, &[tree_row("repo-policy", "policy/", &[])], None) + batten::policy::load(&root, &[tree_row("repo-policy", "policy/", &[])], &[], None) .expect("every key this module reads is one the engine emits"); } @@ -457,6 +458,7 @@ fn a_document_with_no_parser_is_refused_rather_than_skipped() { let err = rules::run_static( &[tree_row("repo-policy", "policy/", &["CLAUDE.md"])], &[], + &[], &root, ) .expect_err("a declared document this build cannot parse is a config fault"); @@ -513,6 +515,7 @@ fn an_absent_unsupported_document_is_still_a_parser_fault() { let err = rules::run_static( &[tree_row("repo-policy", "policy/", &["CLAUDE.md"])], &[], + &[], &root, ) .expect_err("the extension is decided before the tree is consulted"); @@ -555,7 +558,7 @@ violation contains {"rule": "reads-a-ghost", "msg": "x"} if { "#, ); - let err = batten::policy::load(&root, &[tree_row("repo-policy", "policy/", &[])], None) + let err = batten::policy::load(&root, &[tree_row("repo-policy", "policy/", &[])], &[], None) .expect_err("a bracket reference is a reference"); assert!( format!("{err}").contains("nonesuch"), @@ -585,7 +588,7 @@ violation contains {"rule": "reads-real-keys", "msg": "x"} if { "#, ); - batten::policy::load(&root, &[tree_row("repo-policy", "policy/", &[])], None) + batten::policy::load(&root, &[tree_row("repo-policy", "policy/", &[])], &[], None) .expect("`documents` is emitted, however it is spelled"); } diff --git a/crates/batten/tests/policy_whole_set.rs b/crates/batten/tests/policy_whole_set.rs index 93cd45cd1..fe597acaf 100644 --- a/crates/batten/tests/policy_whole_set.rs +++ b/crates/batten/tests/policy_whole_set.rs @@ -43,6 +43,7 @@ fn a_healthy_set_sweeps_clean_and_reaches_every_module() { "package batten.b\nimport rego.v1\nrules contains \"b\"\nviolation contains {\"rule\": \"b\", \"msg\": \"m\"} if { input.call.operation == \"read\" }\n", ), ], + &serde_json::json!({}), ) .expect("a healthy set compiles"); @@ -86,6 +87,7 @@ fn a_module_the_sweep_never_entered_is_reported() { "package elsewhere\nimport rego.v1\nnever_reached contains \"x\" if { true }\n", ), ], + &serde_json::json!({}), ) .expect("both compile — being unreachable is not a compile error"); @@ -121,6 +123,7 @@ fn two_contradicting_complete_rules_are_refused_by_the_sweep() { module("one", "package batten.c\nimport rego.v1\nverdict := 1\n"), module("two", "package batten.c\nimport rego.v1\nverdict := 2\n"), ], + &serde_json::json!({}), ); // The conflict may be caught by `compile`'s own smoke query — which is the // correct earliest place — or by the driven sweep. Either is a refusal at @@ -160,6 +163,7 @@ fn a_cyclic_set_is_refused_by_the_sweep() { "package batten.cyc\nimport rego.v1\nright contains x if { left[x] }\n", ), ], + &serde_json::json!({}), ); match bundle { Err(refused) => { @@ -193,6 +197,7 @@ fn the_analysis_carries_no_byte_of_any_policy_body() { "package batten.p\nimport rego.v1\nrules contains \"p\"\nviolation contains {{\"rule\": \"p\", \"msg\": \"{DISTINCTIVE}\"}} if {{ input.call.operation == \"write\" }}\n" ), )], + &serde_json::json!({}), ) .expect("compiles"); diff --git a/crates/batten/tests/primitives.rs b/crates/batten/tests/primitives.rs index c0e497228..6966f7e29 100644 --- a/crates/batten/tests/primitives.rs +++ b/crates/batten/tests/primitives.rs @@ -868,16 +868,27 @@ fn the_acceptance_runner_is_the_landed_rule_engine() { let repo = repo("acceptance-runner"); repo.write("a.txt", "content\n"); - let err = batten::rules::run_static(&config.rules, &config.provisions, &repo.dir).unwrap_err(); + let err = batten::rules::run_static( + &config.rules, + &config.provisions, + &config.patterns, + &repo.dir, + ) + .unwrap_err(); assert!( err.downcast_ref::().is_some(), "the read-effect surface refuses a spawning rule loudly, never skips it" ); assert!( - batten::rules::run_all(&config.rules, &config.provisions, &repo.dir) - .expect("the spawning surface runs it") - .findings - .is_empty(), + batten::rules::run_all( + &config.rules, + &config.provisions, + &config.patterns, + &repo.dir + ) + .expect("the spawning surface runs it") + .findings + .is_empty(), "an acceptance item that exits 0 produces no finding" ); } diff --git a/hk.pkl b/hk.pkl index 7c39f6a78..c65399718 100644 --- a/hk.pkl +++ b/hk.pkl @@ -411,6 +411,7 @@ local gate = new Mapping { "crates/batten/src/judge.rs", "crates/batten/src/markers.rs", "crates/batten/src/outputs.rs", + "crates/batten/src/pattern.rs", "crates/batten/src/provision.rs", "crates/batten/src/redirect.rs", "crates/batten/src/rules.rs", diff --git a/policy/run-shape.rego b/policy/run-shape.rego index f5f577d5a..a7906cb38 100644 --- a/policy/run-shape.rego +++ b/policy/run-shape.rego @@ -28,7 +28,7 @@ rules contains "commit-names-no-message-source" # the SCRUBBING and the SPLITTING rather than the flag table, which is where a # raw-string module goes quietly wrong. `@` delimits each sed script because the # rows themselves are `|`-separated. -#MUTANT message-flag-unchecked|s@\[mFCc\]@[Z]@|every form that CAN obtain a message stays allowed +#MUTANT message-flag-cluster-unreferenced|s@short-message-flag-cluster@zzz-no-such-pattern@|every form that CAN obtain a message stays allowed #MUTANT list-not-split|s@^elements :=.*@elements := [scrubbed]@|a compound list is judged per element #MUTANT heredoc-body-judged|s@ j < i@ j < -1@|a git commit inside a heredoc body is prose #MUTANT double-quoted-span-judged|s@^scrubbed := quoted_out(single_scrubbed.*@scrubbed := single_scrubbed@|a quoted span carrying a list separator is not a list @@ -200,7 +200,7 @@ names_a_message_source(stage) if { # predicate the comment above already claimed. names_a_message_source(stage) if { some t in tokens(stage) - regex.match(`^-[A-Za-z]*[mFCc]`, t) + regex.match(data.batten.patterns["short-message-flag-cluster"], t) } # --------------------------------------------------------------------------- diff --git a/schema/batten.schema.json b/schema/batten.schema.json index e3dc226b2..4c1794af7 100644 --- a/schema/batten.schema.json +++ b/schema/batten.schema.json @@ -167,6 +167,13 @@ "null" ] }, + "pattern": { + "description": "The named-regex table (CLOUD-885): every expression a policy module may\napply, declared once and referenced by id.\n\nConsumer-specific by nature for exactly [`Config::verbs`]'s reason — a\ntracker-key expression is a consumer identifier, so it lives here and\nnever in the crate (non-negotiable rule 1). A module reaches it at\n`data.batten.patterns[\"\"]`; writing one inline is refused at load,\nwhich is what prices a one-off pattern against a field access over an\nalready-parsed document. The type and its validation are\n[`crate::pattern`].", + "type": "array", + "items": { + "$ref": "#/$defs/NamedPattern" + } + }, "protected": { "description": "Paths whose modification is guarded. A plain include set — no `!`\nentries — evaluated independently of [`Config::scope`] and\n[`Config::unlanded`]. CLOUD-31's config-trust diff defends this set.", "type": "array", @@ -856,6 +863,25 @@ "effect" ] }, + "NamedPattern": { + "description": "One declared regular expression.\n\nMirrors [`crate::verbs::MutatingVerb`]'s shape deliberately: a consumer-owned\ntable in the one committed authority, keyed by an id the config author picks.", + "type": "object", + "properties": { + "id": { + "description": "The name a module references it by — the key under\n`data.batten.patterns`.\n\nUnique across the table. A repeated id would make\n`data.batten.patterns[\"x\"]` ambiguous, and \"which declaration answered\nme\" is not a question a reviewer should have to resolve — the same\nreasoning [`crate::policy`] applies to two rows registering one module.", + "type": "string" + }, + "regex": { + "description": "The expression itself.\n\nCompiled at load by [`validate`], so a malformed pattern is a config\nfault at exit `1` rather than an evaluation failure on the mediated path\n— house style §8's placement, and the same one `forbid`'s `regex` column\nalready takes.", + "type": "string" + } + }, + "additionalProperties": false, + "required": [ + "id", + "regex" + ] + }, "OperandScope": { "description": "Which operands of a matched invocation are write targets.\n\n[`OperandScope::All`] is the default, and that is load-bearing rather than\nincidental: it is the reading that fails toward *refusing*, and it is the one\na move needs — guarding only the source would miss the direction that\ndestroys the destination, which\n`hook::tests::every_operand_is_a_candidate_so_a_destination_is_guarded_too`\npins. [`OperandScope::Last`] is the narrowing a destination-only program\nneeds, where copying a guarded file *out* of the guarded set is a read and\nrefusing it is the false positive that gets a guard switched off.", "oneOf": [ diff --git a/tests/run-shape.bats b/tests/run-shape.bats index e4c79fbb9..0ff83f99c 100644 --- a/tests/run-shape.bats +++ b/tests/run-shape.bats @@ -53,6 +53,14 @@ setup() { echo 'scope = "mediated_call"' echo 'module = "policy/run-shape.rego"' echo 'severity = "deny"' + # The declared pattern the module references (CLOUD-885). An inline regex + # is refused at load, so this row is not fixture decoration — without it + # the module's reference is undefined and every allow case flips to a + # denial, which is the silent-disarm the engine now refuses outright. + echo + echo "[[pattern]]" + echo 'id = "short-message-flag-cluster"' + echo 'regex = "^-[A-Za-z]*[mFCc]"' } >"$REPO/batten.toml" # No global or system config: a contributor's own git settings must not be # able to change a verdict here (CLOUD-282). From ec32d2f94eae0c496d28747f9e5c6b764be1f6b5 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Sat, 22 Aug 2026 07:12:27 +0000 Subject: [PATCH 7/7] perf(policy): the module checks are config faults, so they leave the hot path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught a regression this branch introduced and local `verify` did not: wired: base p50=14.03ms -> head p50=22.98ms (1.638x, gate 1.30x) CAUSE, and it is a placement error rather than a slow function. `check_no_inline_regex` and `check_tree_paths_are_emittable` read a module's AST through `Engine::get_ast_as_json`, which serialises every rule of every module in the bundle. The tree-key check early-returns for a tree-scoped row, so it never paid this on the mediated path; the new one ran on both scopes. `hook` calls `load` once per mediated call, so every adjudication re-derived a constant — the answer is a property of the module TEXT, fixed for the life of the load and identical on every surface. So they move to where a config fault is REPORTED: `check`, `enforce`, `config lint`, `doctor`. That is house style §8's placement independently of the cost — the mediated call's job is to load and decide, not to re-validate config — and a module with an inline pattern is refused by this repository's own gate chain exactly as a `no-docs-tree` violation is. `ModuleChecks` names the two callers apart, and the enum's doc carries the measurement so the next person moving a check onto `load` sees the price first. wired: base p50=48.73ms -> head p50=46.52ms (0.95x) after WHY LOCAL `verify` WAS GREEN, which is the part worth keeping. The gate is a RATIO and this container's baseline is ~3.5x slower than CI's: 48.7ms against 14.0ms on the same path. A fixed ~9ms cost is 1.64x against CI's baseline and about 1.19x against this one — under the threshold, invisible. A slower machine therefore MASKS a fixed-cost regression, and the masking gets worse the slower the machine. That is not a flake and not a CI quirk: verify and CI measured the same change correctly and disagreed because a ratio over different baselines is a different question. Recorded on CLOUD-885. Refs: CLOUD-885 --- crates/batten/src/hook.rs | 3 +- crates/batten/src/lib.rs | 1 + crates/batten/src/lint.rs | 10 +- crates/batten/src/policy.rs | 56 +++++++- crates/batten/src/rules.rs | 24 +++- crates/batten/tests/policy_modules.rs | 167 +++++++++++++++++++---- crates/batten/tests/policy_presets.rs | 62 +++++++-- crates/batten/tests/policy_test_suite.rs | 10 +- crates/batten/tests/policy_tree.rs | 40 ++++-- fuzz/Cargo.lock | 2 +- 10 files changed, 316 insertions(+), 59 deletions(-) diff --git a/crates/batten/src/hook.rs b/crates/batten/src/hook.rs index 10f75396a..c863ec18c 100644 --- a/crates/batten/src/hook.rs +++ b/crates/batten/src/hook.rs @@ -5333,7 +5333,8 @@ mod tests { Policy { harness: Harness::ExitCode, facts: Vec::new(), - bundles: crate::policy::load(&dir, &[row], &[], None).expect("load"), + bundles: crate::policy::load(&dir, &[row], &[], crate::policy::ModuleChecks::Run, None) + .expect("load"), shapes: Vec::new(), fail_on_warning: false, verbs: Vec::new(), diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index 770e58d3d..8a5965e01 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -1363,6 +1363,7 @@ fn run_policy_test(json: bool, overrides: &Overrides, out: &mut dyn Write) -> Re root, &config.rules, &config.patterns, + policy::ModuleChecks::Run, overrides.config_from.as_deref(), )?; // The same walk the tree engine hoists, so a suite's input carries the same diff --git a/crates/batten/src/lint.rs b/crates/batten/src/lint.rs index a51fa19ef..d3cd07759 100644 --- a/crates/batten/src/lint.rs +++ b/crates/batten/src/lint.rs @@ -462,7 +462,15 @@ pub fn run(dir: &Path, base_ref: Option<&str>, today: crate::waiver::Date) -> Re // reports nothing about a set it never saw. let bundles = config::parse(&text, &path.display().to_string()) .ok() - .map(|config| crate::policy::load(dir, &config.rules, &config.patterns, base_ref)) + .map(|config| { + crate::policy::load( + dir, + &config.rules, + &config.patterns, + crate::policy::ModuleChecks::Run, + base_ref, + ) + }) .and_then(std::result::Result::ok) .unwrap_or_default(); smells( diff --git a/crates/batten/src/policy.rs b/crates/batten/src/policy.rs index f1f6bc900..98cb4c484 100644 --- a/crates/batten/src/policy.rs +++ b/crates/batten/src/policy.rs @@ -515,6 +515,31 @@ pub fn engines_constructed() -> usize { ENGINES_CONSTRUCTED.load(Ordering::Relaxed) } +/// Whether a `load` re-derives the AST-borne config checks. +/// +/// **A placement decision, and it was measured rather than reasoned.** The two +/// checks below read a module's AST through `Engine::get_ast_as_json`, which +/// serialises every rule of every module in the bundle. Their answer is a +/// property of the module TEXT, so it is identical on every surface and fixed +/// for the life of the load — but `hook` calls `load` once per mediated call, +/// so running them there re-derives a constant answer inside CLOUD-689's 100ms +/// budget. CI measured the cost as `wired` p50 14.03ms -> 22.98ms, a 1.638x +/// regression against a 1.30x gate, on a branch whose local `verify` was green. +/// +/// So they run where a config fault is REPORTED — `check`, `enforce`, `config +/// lint`, `doctor` — which is house style §8's placement independently of the +/// cost, and never on the adjudication path. A module with an inline pattern is +/// refused by this repository's own gate chain exactly as a `no-docs-tree` +/// violation is; what the mediated call must do is load and decide. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ModuleChecks { + /// Re-derive them: the caller is a surface that reports config faults. + Run, + /// Skip them: the caller is the mediated path, where the answer is already + /// known and the budget is per call. + SkipOnHotPath, +} + /// Load, compile and smoke-test every module the rule set registers. /// /// Boundary I/O, called once per process from the config resolution path — never @@ -535,6 +560,7 @@ pub fn load( root: &Path, rules: &[Rule], patterns: &[crate::pattern::NamedPattern], + checks: ModuleChecks, reference: Option<&str>, ) -> Result> { // The table is validated at PARSE, beside `verbs` and `redirects` and for @@ -626,8 +652,10 @@ pub fn load( // preset, so a preset reading an unemittable `input.tree` key would // have loaded as a dead gate — the exact failure the check exists to // refuse, arriving through the one source that bypasses it. - check_tree_paths_are_emittable(rule, &bundle, source_key)?; - check_no_inline_regex(rule, &bundle, &declared_patterns, source_key)?; + if checks == ModuleChecks::Run { + check_tree_paths_are_emittable(rule, &bundle, source_key)?; + check_no_inline_regex(rule, &bundle, &declared_patterns, source_key)?; + } claim_ids(&mut ids, &declared, source_key)?; bundles.push(bundle); continue; @@ -668,8 +696,10 @@ pub fn load( check_predicate_severity(rule, &declared, where_it_came_from)?; - check_tree_paths_are_emittable(rule, &bundle, where_it_came_from)?; - check_no_inline_regex(rule, &bundle, &declared_patterns, where_it_came_from)?; + if checks == ModuleChecks::Run { + check_tree_paths_are_emittable(rule, &bundle, where_it_came_from)?; + check_no_inline_regex(rule, &bundle, &declared_patterns, where_it_came_from)?; + } claim_ids(&mut ids, &declared, where_it_came_from)?; @@ -1221,6 +1251,24 @@ fn check_no_inline_regex( // describes arriving by a different route — a typo rather than a deletion. // Same shape as `check_tree_paths_are_emittable`: refuse a reference the // engine cannot satisfy, at load, against the table rather than a list. + // A VENDORED PRESET IS EXEMPT, and it is the rule's own scope rather than a + // hatch in it. The declaration requirement exists because a pattern written + // into a module smuggles a CONSUMER fact into a place consumer facts may not + // live (non-negotiable rule 1) — which is why `Config::verbs` states the + // same argument for its table. A preset ships INSIDE the crate, so rule 1 + // already forces its patterns to be repo-agnostic: `shell-hygiene`'s + // `\$\{?BASH_SOURCE|\$0` names no consumer and could not, or the preset + // itself would fail the rule. + // + // It is also unsatisfiable as a demand. A preset is compiled in; a consumer + // cannot add a `[[pattern]]` row on its behalf, and the preset cannot read + // one — so refusing it would make a vendored bundle unloadable with no fix + // available, which is the wrongly-refusing gate AGENTS.md calls a defect. + // Caught by `the_committed_delegating_rule_*` on the first run against a + // preset that uses one. + if rule.preset.is_some() { + return Ok(()); + } for module in &described { for rule_ast in &module.rules { for referenced in &rule_ast.pattern_refs { diff --git a/crates/batten/src/rules.rs b/crates/batten/src/rules.rs index ecfb1575c..205605ca8 100644 --- a/crates/batten/src/rules.rs +++ b/crates/batten/src/rules.rs @@ -2804,7 +2804,13 @@ pub fn run_static( // process or reach the network, a property CLOUD-831 gates rather than // asserts. So admitting it here makes `check` MORE capable without making it // less honest, and the spawning refusal below is untouched. - let bundles = crate::policy::load(root, rules, patterns, None)?; + let bundles = crate::policy::load( + root, + rules, + patterns, + crate::policy::ModuleChecks::Run, + None, + )?; // Refuse before any work: the read-only surface must not even begin a run // it cannot complete honestly. for rule in rules { @@ -2872,7 +2878,13 @@ pub fn run_recorded( .iter() .partition(|rule| !rule.kind.carries_ambient_authority()); let evaluable: Vec = evaluable.into_iter().cloned().collect(); - let bundles = crate::policy::load(root, &evaluable, patterns, None)?; + let bundles = crate::policy::load( + root, + &evaluable, + patterns, + crate::policy::ModuleChecks::Run, + None, + )?; let mut scan = run(&evaluable, provisions, root, &bundles)?; for rule in withheld { // `RuleSkipped`, not a variant of its own. The distinction between "the @@ -2918,7 +2930,13 @@ pub fn run_all( ))); } } - let bundles = crate::policy::load(root, rules, patterns, None)?; + let bundles = crate::policy::load( + root, + rules, + patterns, + crate::policy::ModuleChecks::Run, + None, + )?; run(rules, provisions, root, &bundles) } diff --git a/crates/batten/tests/policy_modules.rs b/crates/batten/tests/policy_modules.rs index a876b1fd6..b45e73768 100644 --- a/crates/batten/tests/policy_modules.rs +++ b/crates/batten/tests/policy_modules.rs @@ -223,7 +223,14 @@ fn scratch(name: &str) -> std::path::PathBuf { fn a_module_denies_on_a_fact_and_is_silent_otherwise() { let root = scratch("denies"); let path = module_file(&root, "writes.rego", DENIES_WRITES); - let bundles = policy::load(&root, &[row("policy-writes", &path)], &[], None).expect("load"); + let bundles = policy::load( + &root, + &[row("policy-writes", &path)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect("load"); assert_eq!(bundles.len(), 1); let denied = policy::deny(&bundles[0], r#"{"call":{"operation":"write"}}"#); @@ -248,7 +255,14 @@ fn a_module_denies_on_a_fact_and_is_silent_otherwise() { fn an_unparseable_input_is_could_not_look_and_never_an_empty_deny_set() { let root = scratch("couldnotlook"); let path = module_file(&root, "writes.rego", DENIES_WRITES); - let bundles = policy::load(&root, &[row("policy-writes", &path)], &[], None).expect("load"); + let bundles = policy::load( + &root, + &[row("policy-writes", &path)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect("load"); let answer = policy::deny(&bundles[0], "{not json"); assert!( @@ -265,8 +279,14 @@ fn a_cyclic_module_is_refused_at_load() { // in `load`, this module compiles clean here and faults at the gate. let root = scratch("cyclic"); let path = module_file(&root, "cyclic.rego", CYCLIC); - let err = policy::load(&root, &[row("policy-cyclic", &path)], &[], None) - .expect_err("a cycle is a config error, not a runtime surprise"); + let err = policy::load( + &root, + &[row("policy-cyclic", &path)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect_err("a cycle is a config error, not a runtime surprise"); let text = format!("{err}"); assert!( text.contains("cyclic.rego"), @@ -288,8 +308,14 @@ fn a_cyclic_module_is_refused_at_load() { #[test] fn a_module_that_cannot_be_read_is_refused_at_load() { let root = scratch("absent"); - let err = policy::load(&root, &[row("policy-absent", "nowhere.rego")], &[], None) - .expect_err("a registration naming no file decides nothing and must not load"); + let err = policy::load( + &root, + &[row("policy-absent", "nowhere.rego")], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect_err("a registration naming no file decides nothing and must not load"); assert!(format!("{err}").contains("nowhere.rego")); } @@ -304,6 +330,7 @@ fn two_rows_registering_one_module_are_refused_at_load() { &root, &[row("first", &path), row("second", &path)], &[], + policy::ModuleChecks::Run, None, ) .expect_err("one module, two registrations"); @@ -361,8 +388,14 @@ fn a_regex_written_inline_is_refused_and_a_declared_one_decides() { // about regularity. let root = scratch("inline-regex"); let inline = module_file(&root, "inline.rego", INLINE_REGEX); - let err = policy::load(&root, &[row("keys", &inline)], &[], None) - .expect_err("a regex written inline must not load"); + let err = policy::load( + &root, + &[row("keys", &inline)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect_err("a regex written inline must not load"); let rendered = format!("{err}"); assert!( rendered.contains("CLOUD-[0-9]+"), @@ -380,8 +413,14 @@ fn a_regex_written_inline_is_refused_and_a_declared_one_decides() { // gate nothing, which is CLOUD-251's vacuous pass with a regex in it. So // this asserts the deny, not merely the load. let declared = module_file(&root, "declared.rego", DECLARED_REGEX); - let bundles = policy::load(&root, &[row("keys", &declared)], &[tracker_key()], None) - .expect("a declared pattern is the sanctioned form"); + let bundles = policy::load( + &root, + &[row("keys", &declared)], + &[tracker_key()], + policy::ModuleChecks::Run, + None, + ) + .expect("a declared pattern is the sanctioned form"); let doc = r#"{"call":{"command":"git commit -m CLOUD-885"}}"#; let Look::Is(violations) = policy::deny(&bundles[0], doc) else { panic!("the declared pattern must reach the module through `data`"); @@ -395,8 +434,14 @@ fn a_regex_written_inline_is_refused_and_a_declared_one_decides() { // THE SMUGGLE. A check reading only direct parameters would wave this // through; recursing into each parameter is what closes it. let smuggled = module_file(&root, "smuggled.rego", SMUGGLED_REGEX); - let err = policy::load(&root, &[row("keys", &smuggled)], &[], None) - .expect_err("a literal assembled inside the call is still a literal"); + let err = policy::load( + &root, + &[row("keys", &smuggled)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect_err("a literal assembled inside the call is still a literal"); assert!( format!("{err}").contains("[[pattern]]"), "the smuggled form gets the same refusal: {err}" @@ -410,7 +455,14 @@ fn a_module_holds_no_source_and_cannot_leak_one_through_debug() { // re-derived it. This asserts the rendering, which is the reachable half. let root = scratch("pointer"); let path = module_file(&root, "writes.rego", DENIES_WRITES); - let bundles = policy::load(&root, &[row("policy-writes", &path)], &[], None).expect("load"); + let bundles = policy::load( + &root, + &[row("policy-writes", &path)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect("load"); let rendered = format!("{:?}", bundles[0]); assert!(rendered.contains("policy-writes"), "the pointer is present"); assert!( @@ -467,8 +519,14 @@ fn no_evaluator_feature_admits_io() { // The control first. If this does not deny, nothing below discriminates. let included = module_file(&root, "included.rego", REACHES_AN_INCLUDED_BUILTIN); - let bundles = policy::load(&root, &[row("policy-included", &included)], &[], None) - .expect("a module over an in-closure builtin loads"); + let bundles = policy::load( + &root, + &[row("policy-included", &included)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect("a module over an in-closure builtin loads"); assert_eq!( policy::deny(&bundles[0], "{}"), Look::Is(vec![unattributed( @@ -483,7 +541,13 @@ fn no_evaluator_feature_admits_io() { // property the doc claims; both arms are accepted here and the assertion is // over the outcome that matters. let network = module_file(&root, "network.rego", REACHES_THE_NETWORK); - match policy::load(&root, &[row("policy-network", &network)], &[], None) { + match policy::load( + &root, + &[row("policy-network", &network)], + &[], + policy::ModuleChecks::Run, + None, + ) { Err(refused) => { let text = format!("{refused}"); assert!( @@ -507,7 +571,13 @@ fn no_evaluator_feature_admits_io() { // manifest pins out. Same shape: a test covering one of the two would report // the pin held while half of it drifted. let schema = module_file(&root, "schema.rego", REACHES_JSONSCHEMA); - match policy::load(&root, &[row("policy-schema", &schema)], &[], None) { + match policy::load( + &root, + &[row("policy-schema", &schema)], + &[], + policy::ModuleChecks::Run, + None, + ) { Err(refused) => { let text = format!("{refused}"); assert!( @@ -537,7 +607,14 @@ fn no_evaluator_feature_admits_io() { fn one_module_carries_two_predicates_that_deny_under_their_own_ids() { let root = scratch("two-predicates"); let path = module_file(&root, "two.rego", TWO_PREDICATES); - let bundles = policy::load(&root, &[row("policy-two", &path)], &[], None).expect("load"); + let bundles = policy::load( + &root, + &[row("policy-two", &path)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect("load"); let denied = policy::deny(&bundles[0], r#"{"call":{"operation":"write"}}"#); let Look::Is(violations) = denied else { @@ -590,7 +667,14 @@ fn one_module_carries_two_predicates_that_deny_under_their_own_ids() { fn a_bare_string_deny_still_reports_under_the_registering_row() { let root = scratch("bare-string"); let path = module_file(&root, "writes.rego", DENIES_WRITES); - let bundles = policy::load(&root, &[row("policy-writes", &path)], &[], None).expect("load"); + let bundles = policy::load( + &root, + &[row("policy-writes", &path)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect("load"); assert!( bundles[0].declared().is_empty(), @@ -621,8 +705,14 @@ fn a_bare_string_deny_still_reports_under_the_registering_row() { fn an_undeclared_violation_id_is_refused_at_load() { let root = scratch("undeclared"); let path = module_file(&root, "undeclared.rego", UNDECLARED_ID); - let err = policy::load(&root, &[row("policy-undeclared", &path)], &[], None) - .expect_err("an id the module does not publish cannot be attributed"); + let err = policy::load( + &root, + &[row("policy-undeclared", &path)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect_err("an id the module does not publish cannot be attributed"); let text = format!("{err}"); assert!( text.contains("never-declared"), @@ -655,6 +745,7 @@ fn two_modules_declaring_one_id_are_refused_at_load() { &root, &[row("policy-a", &first), row("policy-b", &second)], &[], + policy::ModuleChecks::Run, None, ) .expect_err("one id, two publishers"); @@ -684,7 +775,14 @@ fn two_modules_declaring_one_id_are_refused_at_load() { fn a_waiver_over_one_predicate_does_not_suppress_its_sibling() { let root = scratch("waiver-sibling"); let path = module_file(&root, "two.rego", TWO_PREDICATES); - let bundles = policy::load(&root, &[row("policy-two", &path)], &[], None).expect("load"); + let bundles = policy::load( + &root, + &[row("policy-two", &path)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect("load"); let Look::Is(violations) = policy::deny(&bundles[0], r#"{"call":{"operation":"write"}}"#) else { @@ -735,8 +833,14 @@ violation contains {"rule": "only-on-a-write", "msg": "reached later"} if { } "#; let path = module_file(&root, "late.rego", source); - let bundles = policy::load(&root, &[row("policy-late", &path)], &[], None) - .expect("load cannot reach this violation, so it loads"); + let bundles = policy::load( + &root, + &[row("policy-late", &path)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect("load cannot reach this violation, so it loads"); let answer = policy::deny(&bundles[0], r#"{"call":{"operation":"write"}}"#); assert!( @@ -767,7 +871,7 @@ fn a_predicate_severity_naming_an_unpublished_id_is_refused_at_load() { .into_iter() .collect(), ); - let err = policy::load(&root, &[rule], &[], None) + let err = policy::load(&root, &[rule], &[], policy::ModuleChecks::Run, None) .expect_err("a severity aimed at an id nothing publishes decides nothing"); let text = format!("{err}"); assert!(text.contains("no-such-predicate"), "names the key: {text}"); @@ -791,7 +895,8 @@ fn severity_resolves_per_predicate_and_falls_back_to_the_row() { .into_iter() .collect(), ); - let bundles = policy::load(&root, &[rule.clone()], &[], None).expect("load"); + let bundles = + policy::load(&root, &[rule.clone()], &[], policy::ModuleChecks::Run, None).expect("load"); assert_eq!( rule.severity_for(Some("no-stray-artifact")), @@ -846,8 +951,14 @@ violation contains {"rule": "no-force-push", "msg": "a force push at the trunk"} } "#; let path = module_file(&root, "git.rego", source); - let bundles = policy::load(&root, &[row("policy-git", &path)], &[], None) - .expect("a sub-package module loads"); + let bundles = policy::load( + &root, + &[row("policy-git", &path)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect("a sub-package module loads"); assert!( bundles[0].declared().contains("no-force-push"), diff --git a/crates/batten/tests/policy_presets.rs b/crates/batten/tests/policy_presets.rs index eeaa045e4..bc4f897ed 100644 --- a/crates/batten/tests/policy_presets.rs +++ b/crates/batten/tests/policy_presets.rs @@ -54,8 +54,14 @@ fn scratch(name: &str) -> PathBuf { #[test] fn a_preset_predicate_denies_and_is_green_by_turns() { let root = scratch("denies"); - let bundles = policy::load(&root, &[preset_row("trunk", "trunk-based")], &[], None) - .expect("a vendored preset loads"); + let bundles = policy::load( + &root, + &[preset_row("trunk", "trunk-based")], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect("a vendored preset loads"); let Look::Is(violations) = policy::deny(&bundles[0], &call("git push --force origin topic")) else { @@ -92,8 +98,14 @@ fn a_preset_predicate_denies_and_is_green_by_turns() { #[test] fn the_commit_hygiene_preset_decides_both_ways() { let root = scratch("hygiene"); - let bundles = policy::load(&root, &[preset_row("hygiene", "commit-hygiene")], &[], None) - .expect("the preset loads"); + let bundles = policy::load( + &root, + &[preset_row("hygiene", "commit-hygiene")], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect("the preset loads"); let Look::Is(violations) = policy::deny(&bundles[0], &call("git commit --allow-empty -m x")) else { @@ -114,8 +126,14 @@ fn the_commit_hygiene_preset_decides_both_ways() { #[test] fn an_unknown_preset_name_is_refused_at_load() { let root = scratch("unknown"); - let err = policy::load(&root, &[preset_row("typo", "trunk-basd")], &[], None) - .expect_err("a name this binary does not ship"); + let err = policy::load( + &root, + &[preset_row("typo", "trunk-basd")], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect_err("a name this binary does not ship"); let text = format!("{err}"); assert!( text.contains("trunk-basd"), @@ -131,7 +149,8 @@ fn an_unknown_preset_name_is_refused_at_load() { #[test] fn enabling_no_preset_yields_no_preset_predicates() { let root = scratch("opt-in"); - let bundles = policy::load(&root, &[], &[], None).expect("no rows, no bundles"); + let bundles = policy::load(&root, &[], &[], policy::ModuleChecks::Run, None) + .expect("no rows, no bundles"); assert!( bundles.is_empty(), "a consumer who enables nothing gets nothing, which is what keeps this \ @@ -167,6 +186,7 @@ fn a_preset_id_colliding_with_an_in_repo_id_is_refused_at_load() { &root, &[preset_row("trunk", "trunk-based"), mine], &[], + policy::ModuleChecks::Run, None, ) .expect_err("one id, two publishers across the boundary"); @@ -189,8 +209,14 @@ fn every_advertised_preset_name_actually_loads() { let names = policy::preset_names(); assert!(!names.is_empty(), "the binary ships at least one preset"); for name in names { - policy::load(&root, &[preset_row("row", name)], &[], None) - .unwrap_or_else(|err| panic!("the advertised preset `{name}` does not load: {err}")); + policy::load( + &root, + &[preset_row("row", name)], + &[], + policy::ModuleChecks::Run, + None, + ) + .unwrap_or_else(|err| panic!("the advertised preset `{name}` does not load: {err}")); } } @@ -274,7 +300,14 @@ fn presets_are_inside_the_rule_one_glob() { fn every_shipped_preset_publishes_its_ids() { let root = scratch("published"); for name in policy::preset_names() { - let bundles = policy::load(&root, &[preset_row("row", name)], &[], None).expect("loads"); + let bundles = policy::load( + &root, + &[preset_row("row", name)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect("loads"); assert!( !bundles[0].declared().is_empty(), "the preset `{name}` publishes no rule id, so nothing it denies could \ @@ -300,7 +333,14 @@ fn every_shipped_preset_publishes_its_ids() { fn every_shipped_preset_passes_its_own_suite() { let root = scratch("suites"); for name in policy::preset_names() { - let bundles = policy::load(&root, &[preset_row("row", name)], &[], None).expect("loads"); + let bundles = policy::load( + &root, + &[preset_row("row", name)], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect("loads"); let Look::Is(suite) = policy::test(&bundles[0], "{}").expect("the suite runs") else { panic!("the preset `{name}` has a suite that could not run at all"); }; diff --git a/crates/batten/tests/policy_test_suite.rs b/crates/batten/tests/policy_test_suite.rs index 74b1f64a3..e39d51d08 100644 --- a/crates/batten/tests/policy_test_suite.rs +++ b/crates/batten/tests/policy_test_suite.rs @@ -475,8 +475,14 @@ fn a_registered_module_with_tests_still_loads_and_denies() { // only surface at a mediated call. let dir = Fixture::new("policy-test-still-denies").build(); fs::write(dir.join("probe.rego"), CORRECT).expect("write module"); - let bundles = policy::load(Path::new(&dir), &[row("probe", "probe.rego")], &[], None) - .expect("the bundle loads"); + let bundles = policy::load( + Path::new(&dir), + &[row("probe", "probe.rego")], + &[], + policy::ModuleChecks::Run, + None, + ) + .expect("the bundle loads"); let bundle = bundles.first().expect("one bundle"); let Look::Is(violations) = policy::deny(bundle, r#"{"call": {"command": "git push --force"}}"#) else { diff --git a/crates/batten/tests/policy_tree.rs b/crates/batten/tests/policy_tree.rs index 65337b747..801dc99ac 100644 --- a/crates/batten/tests/policy_tree.rs +++ b/crates/batten/tests/policy_tree.rs @@ -382,8 +382,14 @@ violation contains {"rule": "reads-a-ghost", "msg": "x"} if { "#, ); - let err = batten::policy::load(&root, &[tree_row("repo-policy", "policy/", &[])], &[], None) - .expect_err("a module reading a key the engine cannot emit is refused at load"); + let err = batten::policy::load( + &root, + &[tree_row("repo-policy", "policy/", &[])], + &[], + batten::policy::ModuleChecks::Run, + None, + ) + .expect_err("a module reading a key the engine cannot emit is refused at load"); let message = format!("{err}"); assert!( message.contains("nonesuch"), @@ -429,8 +435,14 @@ violation contains {"rule": "reads-real-keys", "msg": "z"} if { "#, ); - batten::policy::load(&root, &[tree_row("repo-policy", "policy/", &[])], &[], None) - .expect("every key this module reads is one the engine emits"); + batten::policy::load( + &root, + &[tree_row("repo-policy", "policy/", &[])], + &[], + batten::policy::ModuleChecks::Run, + None, + ) + .expect("every key this module reads is one the engine emits"); } /// The third of `missing`'s three causes, split out as a CONFIG FAULT @@ -558,8 +570,14 @@ violation contains {"rule": "reads-a-ghost", "msg": "x"} if { "#, ); - let err = batten::policy::load(&root, &[tree_row("repo-policy", "policy/", &[])], &[], None) - .expect_err("a bracket reference is a reference"); + let err = batten::policy::load( + &root, + &[tree_row("repo-policy", "policy/", &[])], + &[], + batten::policy::ModuleChecks::Run, + None, + ) + .expect_err("a bracket reference is a reference"); assert!( format!("{err}").contains("nonesuch"), "the refusal names the bracketed key: {err}" @@ -588,8 +606,14 @@ violation contains {"rule": "reads-real-keys", "msg": "x"} if { "#, ); - batten::policy::load(&root, &[tree_row("repo-policy", "policy/", &[])], &[], None) - .expect("`documents` is emitted, however it is spelled"); + batten::policy::load( + &root, + &[tree_row("repo-policy", "policy/", &[])], + &[], + batten::policy::ModuleChecks::Run, + None, + ) + .expect("`documents` is emitted, however it is spelled"); } // --- the lines fact (CLOUD-846) --------------------------------------------- diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index b6c92139c..0927c95f9 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -108,7 +108,7 @@ checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" [[package]] name = "batten" -version = "0.0.103" +version = "0.0.104" dependencies = [ "anyhow", "clap",