diff --git a/.claude/rules/toolchain.md b/.claude/rules/toolchain.md index 8924efdbb..3e853dab0 100644 --- a/.claude/rules/toolchain.md +++ b/.claude/rules/toolchain.md @@ -215,7 +215,9 @@ mediating. hand-typed `/fast-forward` comment, naming the task to use instead. Decision table in `mise-tasks/gh-guard-check.sh`, gated by `mise run test:bats`. Reads (`gh pr view`/`list`/`create`, `gh pr ready`, `gh api`, `gh run view`) are not - blocked. Bypass: `BATTEN_GH_GUARD_BYPASS=1`. + blocked. Bypass: `BATTEN_GH_GUARD_BYPASS=1` — which since CLOUD-437 is the + `bypass_env` these rows DECLARE rather than the engine's global hatch, so + setting it now suppresses the `gh` lifecycle and nothing else. - **`memory-guard` is retired** (CLOUD-442), and what it denied is now the engine's protected-path gate: `.serena/memories/**` in `protected` crossed with the `[[verb]]` table, which covers the Write/Edit tools and a command's @@ -225,7 +227,9 @@ mediating. names `rename_memory` — the only route that rewrites `mem:` referrers. The table in `batten.toml` is the one authority; the corpus that used to live in `tests/memory-guard.bats` is `crates/batten/tests/mediated_verbs.rs`. There is - no `BATTEN_MEMORY_GUARD_BYPASS`: a mediated deny takes the engine's own hatch. + no `BATTEN_MEMORY_GUARD_BYPASS`: a mediated deny takes the engine's own hatch, + `BATTEN_HOOK_BYPASS` — or the row's own `bypass_env` where it declares one + (CLOUD-437). - `policy-budget` gates AGENTS.md plus anything always-loaded against a token budget — what every agent pays every turn. It is `batten policy budget`, not a shell task: the counted set and both thresholds are `[budget.instructions]` in @@ -363,7 +367,7 @@ call` with no `CLOUD-*` key **in that same paragraph** stops the lap. Two open are never judged, and a detached HEAD has no branch to key on — while an untracked-but-not-ignored file **is**, since opening a new feature file is the first edit this catches. There is no `BATTEN_CLAIM_GUARD_BYPASS`: a mediated - deny takes the engine's own hatch. + deny takes the engine's own hatch, `BATTEN_HOOK_BYPASS` (CLOUD-437). - **The three verdict-discarding shapes are the engine's** (CLOUD-443), as `batten.toml`'s `verdict-not-discarded` row — `kind = "pipeline"`. They are piping a verdict-bearing command into a pager or filter, following it with `;` diff --git a/.serena/memories/core.md b/.serena/memories/core.md index 25b4fa825..be8718902 100644 --- a/.serena/memories/core.md +++ b/.serena/memories/core.md @@ -704,6 +704,33 @@ repo config > default`, declared as data in `SETTINGS` (per-key env var/flag), surface would evaluate. Per-kind column agreement is a **census** (`Rule::columns` × `requires`/`permits`), not a per-kind match — the match named fields, so a new column landed in no arm and every kind accepted it. +- `invocation.rs` — Rust call sites, parsed (CLOUD-914). Answers WHERE a token + sits, which no line predicate can: a literal in a call's ARGUMENT list is an + invocation argument, one in an array initialiser or a method call's receiver is + not, and a comment never reaches the walk at all. That split is the whole + discriminator — five of `git.rs`'s seven source-scan gates assemble their + needles by concatenation so they do not match their own source, and the two + that do not obfuscate are exactly the two that never read their own module. + A file the parser refuses is `Look::CouldNotLook`, never an empty node set. + Separate from `facts.rs` because that file forbids a wildcard match arm and the + walk over the parser's expression enum needs one — two matches with opposite + right answers do not share a file. +- `uses.rs` — the `use` graph (CLOUD-762). Which module reaches which, resolved + through the crate root's own re-export table. **The measurement is the point:** + over `crates/batten/src/**` the syntactic tier diverges at exactly FOUR sites in + two classes, both re-exports — `trust.rs` and `output.rs` reach `error` through + `crate::UsageError` (a hidden edge a line predicate reads as nothing), and + `policy.rs` and `sink.rs` read as internal where `crate::Result` is really + `anyhow` (a phantom edge it invents). Aliases and globs move NO top-level edge + in this tree. Four is bounded and nameable, so the fact is `Read × Check` and + needs no delegated analyser — and the deeper reason is that the re-export table + is itself syntax, so a parser resolves it with no name resolution at all. + Two things the tree corrected and the code now states: a bare first segment at + the root is a module OR another crate, told apart only by the root's own `mod` + declarations; and `use crate::{a, b}` imports modules directly, so every + declared module is its own table entry. `via_root` marks the edges resolution + CHANGED, because judging by the item's case counts all 88 ordinary edges as + divergences. - `hook.rs` — the `hook` adjudicator (CLOUD-202): the normalized envelope, the wrapper-lookthrough command parser, the matcher, and the **per-host shims** (CLOUD-44). Five hosts plus the neutral `exit-code`: claude-code, cursor, diff --git a/Cargo.lock b/Cargo.lock index 6ba2544e1..e4ea19baa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -132,6 +132,7 @@ dependencies = [ "ignore", "json5", "jsonschema", + "proc-macro2", "regex", "regorus", "rustix", @@ -141,6 +142,7 @@ dependencies = [ "serde_json", "sha2", "signal-hook", + "syn 2.0.119", "tar", "toml", "unicode-normalization", diff --git a/Cargo.toml b/Cargo.toml index 2233d304d..ecf04e658 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -366,6 +366,37 @@ semver = "1" tar = "0.4" serde = { version = "1", features = ["derive"] } serde_json = "1" +# Rust source parsed into a syntax tree, so a policy predicate can ask WHERE a +# token sits rather than only whether it appears (CLOUD-914). `full` for +# statement and expression syntax, `visit` for the read-only walk, `parsing` to +# turn source text into that tree; `printing` and `proc-macro` stay off, because +# nothing here emits Rust. +# +# ADOPTED RATHER THAN VENDORED ANEW: `syn` is already in `Cargo.lock` behind +# every derive in the workspace (serde, schemars, clap, thiserror), so this adds +# a direct edge and no new supply-chain surface. MIT OR Apache-2.0, already on +# `deny.toml`'s allow-list. +# +# WHY NOT THE STRUCTURAL-MATCHER ROUTE: CLOUD-310 measured that route against +# this tree and rejected it, on an upstream Rust API its own authors tier as +# unstable and on grammars compiled by build scripts through a Darwin path this +# repo has never exercised. Both defects it measured — extensionless files +# scanning to nothing, and a shell grammar silently part-parsing `.bats` — are +# properties of a shell corpus and do not reach `crates/**/*.rs`. A pure-Rust +# parser with no build-script grammar is the ground that rejection left +# standing, and `.claude/rules/scanning.md` names a CLASS for row two rather +# than a product precisely so this substitution is following the rule. +syn = { version = "2", default-features = false, features = [ + "full", + "visit", + "parsing", + "clone-impls", +] } +# `span-locations` is what turns a span into a line number, and without it +# `Span::start()` reports line 0 — a pointer-only finding whose pointer is a +# constant. Already vendored beneath `syn`; this enables the feature and takes a +# direct edge. Same license family, same allow-list. +proc-macro2 = { version = "1", features = ["span-locations"] } # Finding-identity fingerprints (CLOUD-123): SHA-256 preimages, and NFC # canonicalization so macOS NFD checkouts fingerprint identically to Linux. # diff --git a/batten.toml b/batten.toml index 2e61692c3..be3a2549f 100644 --- a/batten.toml +++ b/batten.toml @@ -215,6 +215,22 @@ sha256 = "b2c822742e8cbf355ba0cb4cc690c3cd8fdc9ec1916c8148f27bd9098cb7aee4" # `reason` is required on a shape rule, unlike on a file rule: a mediated deny # reaches a model as the entire explanation, so a refusal that named only its id # would be un-actionable (CLOUD-122). The crate appends the bypass hatch. +# THESE FOUR WOULD DECLARE `bypass_env = "BATTEN_GH_GUARD_BYPASS"` AND DO NOT YET +# (CLOUD-437, deferred to CLOUD-1027). They are the ported `gh-guard`, so that +# name is TRUE of them where it was a fossil everywhere else, and declaring it +# here is behaviour-PRESERVING: the same variable already suppressed these rows +# back when it was the engine's global hatch. +# +# `config-lint` cannot see that argument. It compares `batten.toml` against +# `origin/main` and reads a new `bypass_env` on a rule as `rule-predicate-changed` +# — correctly, in its own terms, since the key makes a row suppressible. The +# admissible route is a groomed `Weakens:` clause in the Ready block plus a +# matching commit trailer, and CLOUD-437's block was groomed on 2026-08-14 without +# one. Asserting it in the change that performs it is the exact shape §8 refuses. +# +# So until that row is groomed, these four take the general `BATTEN_HOOK_BYPASS` +# like every other row. The column, the per-row lookup and the suppression all +# ship; what waits is this repository's own use of them. [[rule]] id = "gh-pr-merge" kind = "shape" @@ -1806,6 +1822,86 @@ severity = "deny" # bash, inside the bundle whose whole purpose is retiring bash (CLOUD-843 measured # "the bash grew today" as the campaign's own defect). Everything decided here # lives in two tracked TOML files, which is CLOUD-846's structured-config bucket. +# Merged-ness is decided by patch identity, never by reachability (CLOUD-36), +# migrated off a hand-rolled source scan (CLOUD-756). +# +# THE SCAN IS DELETED IN THE SAME CHANGE. `git.rs`'s +# `no_ancestry_decides_merged_ness` is gone; a module landing beside a scan it +# does not delete is two authorities on one property. +# +# `invocations` RATHER THAN `lines`, and this session measured why: the substring +# scan fired four times on PROSE while CLOUD-914 and CLOUD-762 were being +# written, at no call site at all. Porting it onto lines would have inherited +# exactly that, obfuscated needles and all. +[[rule]] +id = "ancestry-decides-nothing" +kind = "policy" +scope = "tree" +invocation_sources = ["crates/batten/src/*.rs"] +module = "policy/ancestry-decides-nothing.rego" +severity = "deny" + +# Module layering, over the resolved `use` graph (CLOUD-359). +# +# THE LAYERINGS WERE DOCUMENTED AND GATED BY NOTHING. `module-map-check` asserts +# every module APPEARS in the map and says nothing about any edge — a membership +# check standing in for a predicate, the same substitution `tests-not-deleted` +# and `assertions-not-gutted` make. A back-edge compiled and passed every gate. +# +# `use_sources` RATHER THAN `line_sources`, and CLOUD-762 is why. A line +# predicate over this tree is wrong at four measured sites: it cannot see the two +# edges that reach `error` through a crate-root re-export, and it invents two +# more where `crate::Result` is really `anyhow`. Both surfaces cost the same +# parse; only one is right, so shipping the other would be a known false green. +# +# A GLOB RATHER THAN A LIST, for `line_sources`' own stated reason: an enumerated +# set goes stale the next time somebody adds a module, silently and green, which +# is the failure the declaration bound exists to prevent. +# CLOUD-882. `[workspace.dependencies]` fixes a version; it does not add a +# dependency. An entry no member references with `.workspace = true` +# resolves to nothing, so every gate reading the RESOLVED graph is green about a +# crate that is not in the tree. Measured: `orphan-probe = "=9.9.9"` -- a crate +# that does not exist, at a version that does not exist -- passed `cargo +# metadata`, `macos-link-check` and `deny`, four greens over nothing. +# +# `sources` RATHER THAN `documents`, because only `sources` is glob-selected +# (CLOUD-850) and the member set is a glob by nature: a crate added tomorrow is +# judged without anyone editing this row, which is the property an enumerated +# list loses silently. +# CLOUD-614. A `command` row's `check` is spawned with the checked tree as its +# working directory, so `mise run ` resolves from THERE. A row naming a +# task the tree does not define does not fail to launch — the runner is on PATH, +# it starts, exits non-zero, and the engine reports a finding at the row's own +# severity about a tree it never inspected. Measured as a `deny` asserting +# NTIA-nonconformance over a scratch tree carrying nothing but a lockfile. +# +# THE CLAUSE LIVES HERE RATHER THAN IN `config-lint` because a task runner is a +# consumer's choice: `no_artifact_name_reaches_the_core` refuses the engine +# learning this manifest's name (non-negotiable rule 1). +[[rule]] +id = "command-task-defined" +kind = "policy" +scope = "tree" +sources = ["batten.toml", "mise.toml"] +module = "policy/command-task-defined.rego" +severity = "deny" + +[[rule]] +id = "workspace-dep-referenced" +kind = "policy" +scope = "tree" +sources = ["Cargo.toml", "crates/*/Cargo.toml"] +module = "policy/workspace-dep-referenced.rego" +severity = "deny" + +[[rule]] +id = "module-layering" +kind = "policy" +scope = "tree" +use_sources = ["crates/batten/src/*.rs"] +module = "policy/module-layering.rego" +severity = "deny" + [[rule]] id = "opa-tracks-regorus-compliance" kind = "policy" diff --git a/crates/batten/Cargo.toml b/crates/batten/Cargo.toml index 1f8cd5886..9165ce00a 100644 --- a/crates/batten/Cargo.toml +++ b/crates/batten/Cargo.toml @@ -100,6 +100,8 @@ semver.workspace = true serde.workspace = true serde_json.workspace = true sha2.workspace = true +proc-macro2.workspace = true +syn.workspace = true tar.workspace = true toml.workspace = true unicode-normalization.workspace = true diff --git a/crates/batten/src/baseline.rs b/crates/batten/src/baseline.rs index 9b8f961b1..c3bec2b5e 100644 --- a/crates/batten/src/baseline.rs +++ b/crates/batten/src/baseline.rs @@ -28,13 +28,25 @@ //! The issue asks that "the baselined state must be an ancestor of the //! authoritative ref at its currently-fetched SHA". That claim cannot be spelled //! as ancestry in this crate, and the refusal is a compiled one: -//! `git::tests::no_ancestry_decides_merged_ness` scans every `src/*.rs` file — +//! `policy/ancestry-decides-nothing.rego` judges every `src/*.rs` file — //! including `git.rs` itself, and including this module — for the whole -//! reachability-answer vocabulary, which is why the tokens are named nowhere in -//! this file. (It caught this paragraph's first draft, which spelled them out to -//! explain them. A gate that exempted the prose describing it would be a gate -//! with a hole shaped exactly like a comment.) Merged-ness is decided by **patch -//! identity**, because a rebased or squashed landing is invisible to ancestry +//! reachability-answer vocabulary IN COMMAND POSITION. +//! +//! This paragraph used to end differently, and the change is worth recording +//! rather than quietly making. It read: *"a gate that exempted the prose +//! describing it would be a gate with a hole shaped exactly like a comment"* — +//! written after the old substring scan caught this very paragraph's first +//! draft. That argument is real but it proves less than it claims: a comment +//! cannot decide merged-ness, because it is not executed, so exempting prose +//! opens no hole in the property the gate NAMES. What the broad reading actually +//! bought was a stronger, unstated rule — keep the vocabulary out of the crate +//! entirely — and the price was measured in one session: four false positives on +//! prose written while implementing CLOUD-914 and CLOUD-762, each "fixed" by +//! rewording English until the scanner stopped noticing. +//! +//! So the tokens may now be named in prose and may not be passed to git. +//! Merged-ness is decided by **patch identity**, because a rebased or squashed +//! landing is invisible to ancestry //! and these repositories land by fast-forward, where a landed branch is an //! ancestor of nothing. //! diff --git a/crates/batten/src/config.rs b/crates/batten/src/config.rs index 0ef5f66a7..3fc8d1d30 100644 --- a/crates/batten/src/config.rs +++ b/crates/batten/src/config.rs @@ -1007,6 +1007,7 @@ fn default_rules() -> Vec { requires_key: None, reason: None, policy_url: None, + bypass_env: None, check: None, fix: None, produces: None, @@ -1033,6 +1034,10 @@ fn default_rules() -> Vec { sources: Vec::new(), lines: Vec::new(), line_sources: Vec::new(), + invocations: Vec::new(), + invocation_sources: Vec::new(), + uses: Vec::new(), + use_sources: Vec::new(), predicate_severity: None, no_fix_reason: None, checks: None, diff --git a/crates/batten/src/facts.rs b/crates/batten/src/facts.rs index 017e8e016..3a9e1cf08 100644 --- a/crates/batten/src/facts.rs +++ b/crates/batten/src/facts.rs @@ -452,6 +452,13 @@ pub enum Fact { /// Whether this branch's work is on each **declared** target, by patch /// identity (CLOUD-880) — the landing question `Fact::GitRef` leaves open. Landing, + /// What a **declared** Rust source file's call sites invoke, and with what + /// literal arguments — the token's syntactic POSITION, which no line + /// predicate can see (CLOUD-914). + Invocations, + /// Which module a **declared** Rust source file reaches, resolved through the + /// crate root's own re-export table (CLOUD-762). + Uses, } /// [`Fact::Bypass`] — the hatch is an environment variable, and the kernel @@ -533,6 +540,73 @@ pub const TRACKED: Class = Class::new(Cost::Read, Surface::Check); /// markdown AST would be a parser per prose convention. pub const LINES: Class = Class::new(Cost::Read, Surface::Check); +/// [`Fact::Invocations`] — a declared Rust file's call sites, parsed (CLOUD-914). +/// +/// `read` x `check`, for [`LINES`]' reasons exactly: parsing a file of unbounded +/// size is unbounded in the input, and CLOUD-689's 100ms budget is per mediated +/// call. Never repoint this at [`Surface::Hook`]. +/// +/// **What it adds over [`LINES`], stated as the thing a line predicate cannot +/// do.** `lines` answers *does this token appear in this file*; this answers +/// *does this token appear IN COMMAND POSITION*. The tree measures the +/// difference rather than arguing it: five of `git.rs`'s seven source-scan +/// gates assemble their needles by concatenation — `["merge", "-base"].concat()` +/// — for one stated reason, that *"this assertion's own source is not a match +/// for the gate it states"*. The two gates that do NOT obfuscate are exactly the +/// two that never read their own module's source. A substring gate must hide its +/// own literals precisely when its corpus includes itself, and that is the +/// defect this fact retires. +/// +/// **Only arguments, never receivers, and that is what makes the discriminator +/// work.** A literal in a call's argument list is an invocation argument; a +/// literal in an array initialiser, a `let` binding, or the receiver of a method +/// call is not. So an `.arg(..)` carrying a banned token is reported and a +/// needle array holding the same token is not — same bytes, same file, opposite +/// verdicts, which is the test this fact lives or dies on. Comments never arrive +/// at all: the parser discards them before this sees a token. +/// +/// **This paragraph could not spell its own example, and that is the finding.** +/// It first read `.arg("merge" + "-base")` with the token written plainly, and +/// `git::tests::no_ancestry_decides_merged_ness` went red on `facts.rs` — +/// against PROSE IN A DOC COMMENT, at no call site at all. The guard is behaving +/// exactly as designed; the substring tier simply cannot tell a comment from a +/// call. So the sentence above describes its example instead of writing it, for +/// the same reason five of `git.rs`'s seven gates assemble their needles by +/// concatenation. Measured while documenting the fact that retires the +/// workaround — which is the strongest evidence this row could have asked for, +/// and it arrived unprompted. +/// +/// **Rust only, and deliberately.** CLOUD-310 rejected the tree-sitter route on +/// a corpus of 46 extensionless shell programs, and both of its defects are +/// artefacts of that corpus — file discovery, and the bash grammar's fit to +/// `.bats`. Neither reaches `crates/**/*.rs`. What survives its rejection and +/// binds here is the parse-coverage obligation: a file the parser cannot read is +/// [`Look::CouldNotLook`], never an empty node set. +pub const INVOCATIONS: Class = Class::new(Cost::Read, Surface::Check); + +/// [`Fact::Uses`] — a declared Rust file's `use` edges, resolved (CLOUD-762). +/// +/// `read` x `check`, and the classification is a MEASUREMENT rather than a +/// judgement. CLOUD-762's reversal condition says a bounded, nameable error count +/// puts this tier here and an unbounded one sends it to `Cost::Effect` behind a +/// delegated analyser. Over `crates/batten/src/**` the count is **four**, in two +/// classes, both re-exports — `crates/batten/tests/use_graph.rs` asserts it, so +/// the number cannot rot into prose. +/// +/// **What a line predicate gets wrong, and in both directions.** `trust.rs` and +/// `output.rs` reach `error` through a name the root re-exports, so a layering +/// gate reading lines is silently green on an edge it exists to judge; +/// `policy.rs` and `sink.rs` read as internal where the root's own private import +/// makes them external, so the same gate invents an edge. Aliases and glob +/// imports move no top-level edge in this tree at all. +/// +/// **Cheap AND correct, which the reversal condition did not anticipate.** The +/// re-export table is itself syntax: resolving all four sites needs the crate +/// root's `use` and `pub use` items and nothing else — no name resolution, no +/// analyser, no `Cost::Effect`. Never repoint this at [`Surface::Hook`]; a +/// whole-tree graph is not a per-call question and the 100ms budget is per call. +pub const USES: Class = Class::new(Cost::Read, Surface::Check); + /// [`Fact::AgentSourced`] — a small record under the git dir, written by the /// boundary from bytes the harness already handed it (CLOUD-776). /// @@ -748,6 +822,8 @@ impl Fact { Fact::GitRef, Fact::GitRange, Fact::Landing, + Fact::Invocations, + Fact::Uses, ]; /// The stable lowercase token (§6) — the field name in `lib.rs`'s `Facts`. @@ -771,6 +847,8 @@ impl Fact { Fact::GitRef => "git-refs", Fact::GitRange => "git-ranges", Fact::Landing => "landing", + Fact::Invocations => "invocations", + Fact::Uses => "uses", } } @@ -802,6 +880,8 @@ impl Fact { Fact::GitRef => GIT_REF, Fact::GitRange => GIT_RANGE, Fact::Landing => LANDING, + Fact::Invocations => INVOCATIONS, + Fact::Uses => USES, } } @@ -848,6 +928,11 @@ impl Fact { // every consumer this row exists for -- the tasks that today read a // sibling's exit code to learn whether work landed -- is one. Fact::Landing => Some("landing"), + // Tree-only by construction (CLOUD-914): a call site is a property + // of committed source, and the mediated path has no budget to parse + // one. + Fact::Invocations => Some("invocations"), + Fact::Uses => Some("uses"), // Hook-surface facts. The tree engine resolves none of them, and // naming them here as `None` is what lets the correspondence test // assert the emitted key set in BOTH directions rather than only @@ -907,6 +992,40 @@ impl Fact { "description": "Fact::Lines. Path -> the file's lines, unparsed (CLOUD-846).", "additionalProperties": {"type": "array", "items": {"type": "string"}}, }), + Fact::Invocations => serde_json::json!({ + "type": "object", + "description": "Fact::Invocations (CLOUD-914). Path -> the call sites in that Rust file, each `program` (the callee as written), `arguments` (the string literals it PASSES -- never its receiver) and `line`. A path absent from this map is could-not-look: the parser could not read it. A path present with an empty list is a file that parsed and calls nothing. Rego reads an undefined path as `does not hold`, so those two must never collapse.", + "additionalProperties": { + "type": "array", + "items": { + "type": "object", + "properties": { + "program": {"type": "string"}, + "arguments": {"type": "array", "items": {"type": "string"}}, + "line": {"type": "integer"}, + }, + "additionalProperties": false, + }, + }, + }), + Fact::Uses => serde_json::json!({ + "type": "object", + "description": "Fact::Uses (CLOUD-762). Path -> that file's `use` edges. `to` is the module or crate reached AFTER resolution through the crate root's re-export table; `item` the imported leaf name; `origin` one of internal/external/root-item/local; `via_root` whether resolution supplied `to` rather than the text, which is the flag that marks an edge a line predicate reads wrongly. An edge still `root-item` is one the root's table could not name, and is could-not-look at the edge level rather than an edge onto nothing. A path absent from this map could not be parsed; a path present with an empty array imports nothing.", + "additionalProperties": { + "type": "array", + "items": { + "type": "object", + "properties": { + "to": {"type": "string"}, + "item": {"type": "string"}, + "origin": {"type": "string"}, + "via-root": {"type": "boolean"}, + "line": {"type": "integer"}, + }, + "additionalProperties": false, + }, + }, + }), Fact::Bypass => serde_json::json!({ "type": "boolean", "description": "Fact::Bypass -- the BATTEN_HOOK_BYPASS hatch (CLOUD-610). The one fact whose shape is certain enough to constrain.", @@ -1041,7 +1160,9 @@ impl Fact { | Fact::Lines | Fact::AgentSourced | Fact::Prospective - | Fact::Produced => serde_json::json!({ + | Fact::Produced + | Fact::Invocations + | Fact::Uses => serde_json::json!({ "description": "unrouted fact -- schema_fragment delegated a fact git_schema_fragment does not own", }), } diff --git a/crates/batten/src/git.rs b/crates/batten/src/git.rs index 1ed9154b7..2ba24a32a 100644 --- a/crates/batten/src/git.rs +++ b/crates/batten/src/git.rs @@ -109,7 +109,9 @@ //! It never produces a verdict. Every [`Verdict::Landed`] is backed by an //! [`Evidence`] naming the target commit whose patch identity matched, and the //! type offers no way to spell a landed verdict while holding no evidence. -//! [`no_ancestry_decides_merged_ness`] is the source-level gate. +//! `policy/ancestry-decides-nothing.rego` is the source-level gate — a +//! registered Rego module over `Fact::Invocations`, which replaced this +//! module's own `no_ancestry_decides_merged_ness` scan (CLOUD-756). //! //! Two consequences worth stating, because neither is obvious: //! @@ -125,7 +127,6 @@ //! work as unlanded. It also reports only `+`/`-`, which cannot populate //! [`Evidence`]. The technique is adopted; the command is not. //! -//! [`no_ancestry_decides_merged_ness`]: tests::no_ancestry_decides_merged_ness use std::collections::{BTreeMap, BTreeSet}; use std::ffi::OsStr; @@ -2753,44 +2754,25 @@ mod tests { sources } - #[test] - fn no_ancestry_decides_merged_ness() { - // The gate that ships with the rule (CLOUD-36): merged-ness is decided - // by patch identity, never by reachability. Unlike the repo-root gate - // above, this one scans *this file too* — the decision logic lives - // here, so exempting it would gut the gate. - // - // Scope is `src/` only, deliberately: `tests/primitives.rs` must run a - // reachability query, because the keystone fixture proves ancestry gets - // the answer wrong on the exact input where this primitive gets it - // right. - // - // The list forbids precisely the *reachability-answer* surface and - // leaves every range form (`..`, `...`, `rev-list`, `--not`) legal — - // selecting which commits to hash is allowed, deciding with the result - // is not. Smuggling a reachability verdict past this means hand-writing - // a graph walk, which is a different and far more visible change. - // Tokens are assembled so this test's own source is not a match, and so - // that prose may still say "the merge base" with a space. - let forbidden = [ - ["merge", "-base"].concat(), - ["merge", "_base"].concat(), - ["is", "-ancestor"].concat(), - ["is", "_ancestor"].concat(), - ["--con", "tains"].concat(), - ["--ancestry", "-path"].concat(), - ]; - for (path, source) in crate_sources(false) { - for token in &forbidden { - assert!( - !source.contains(token), - "{}: contains {token:?}; merged-ness is decided by patch identity, never by \ - reachability (CLOUD-36) — a rebased landing is invisible to ancestry", - path.display() - ); - } - } - } + // `no_ancestry_decides_merged_ness` MOVED HERE (CLOUD-756), and the property + // it held is now `policy/ancestry-decides-nothing.rego` — a registered Rego + // module over `Fact::Invocations`, deleted from this file in the same change + // so the rule is never held twice and never by nobody. + // + // The migration is a fidelity UPGRADE rather than a relocation: the scan + // matched the token anywhere in a source file, and fired four times on prose + // in one session — a doc comment describing the gate, and three comments + // naming an example. Each was "fixed" by rewording English until the scanner + // stopped noticing, which is the tell that the instrument was wrong. The + // module reads command position, so a comment is not a decision. The + // `.concat()` needle obfuscation this file still uses elsewhere exists only + // because a substring scan is its own corpus; the module needs none. + // + // What the module does NOT see, stated so the trade is not silent: a literal + // bound to a variable before it is passed. The scan's own text already + // conceded smuggling — "hand-writing a graph walk, which is a different and + // far more visible change" — so this narrows an evasion rather than opening + // one. /// A repository carrying one committed file, for the `show` cases below. fn show_fixture(name: &str, path: &str, contents: &[u8]) -> PathBuf { diff --git a/crates/batten/src/hook.rs b/crates/batten/src/hook.rs index 63648fef3..d637721d6 100644 --- a/crates/batten/src/hook.rs +++ b/crates/batten/src/hook.rs @@ -34,7 +34,8 @@ //! //! **Posture: fail open.** Unreadable stdin, unparseable JSON, an envelope with //! no command — all resolve to [`Decision::Allow`]. A guard must never be the -//! reason a session cannot proceed; the escape hatch (`BATTEN_GH_GUARD_BYPASS`) +//! reason a session cannot proceed; the general escape hatch (`BATTEN_HOOK_BYPASS`, +//! or a row's own `bypass_env`) //! is honoured exactly as the shell guard honours it. Fail-open needs no care //! here beyond the returns below: §7 spends `2` on the policy verdict alone, so //! neither code a Batten failure can produce is one a host reads as a deny. @@ -2306,8 +2307,36 @@ fn cursor_specialized_input(value: &Value) -> Value { Value::Object(input) } -/// The escape hatch, named once so the boundary and the reason text agree. -pub const BYPASS_ENV: &str = "BATTEN_GH_GUARD_BYPASS"; +/// The GENERAL escape hatch: the whole-call "do not mediate this" switch, and +/// the fallback a row declaring no [`crate::rules::Rule::bypass_env`] advertises. +/// +/// **Renamed from `BATTEN_GH_GUARD_BYPASS` by CLOUD-437, and the old name was a +/// fossil rather than a description.** It was the first retiring bash guard's +/// hatch, kept when that guard was ported and then inherited by every predicate +/// the engine absorbed after it — so a `protected-mutation` deny about a Serena +/// memory told its reader to set a `gh` variable. Measured on this tree: +/// +/// ```text +/// Refused by protected-mutation: `Write` targets the protected path .serena/memories/core.md. +/// Fix: … Bypass with BATTEN_GH_GUARD_BYPASS=1. +/// ``` +/// +/// That breaks CLOUD-122's contract in a way worse than a missing pointer: an +/// operator who reads `GH_GUARD` concludes the refusal came from somewhere it did +/// not, and it reads as evidence that a `gh` guard is what is installed — the +/// exact mis-modelling this layer keeps producing. +/// +/// `BATTEN_GH_GUARD_BYPASS` survives, but only where it is TRUE: as the declared +/// `bypass_env` of the `gh`-lifecycle rows that legitimately own it. There is no +/// dual-honour window — two live names for one hatch is the second authority this +/// module's one-definition property forbids, and there is no external consumer to +/// protect. +/// +/// It stays resolved at the boundary before config loads, which is what keeps the +/// hot path free: a bypassed call must never pay a config read. Per-row hatches +/// cannot be resolved that early — the rows are not known yet — so they resolve +/// after the load, and only for rows that declare one. +pub const BYPASS_ENV: &str = "BATTEN_HOOK_BYPASS"; /// The mediated-call policy this run adjudicates against. /// @@ -2745,6 +2774,77 @@ impl Policy { && self.bundles.is_empty() && (self.verbs.is_empty() || self.protected.is_empty()) } + + /// The hatch that suppresses the row with this id, or [`BYPASS_ENV`] where + /// the row declares none or is not a `[[rule]]` row at all (CLOUD-437). + /// + /// The fallback carries both of the cases a derived gate produces: the + /// protected-mutation gate and the verb table refuse under ids that are + /// declared constants rather than rows, so there is nothing to look up and + /// the general hatch is the honest answer. That is also why this cannot + /// return an `Option` and leave the caller to decide — a deny with no hatch + /// clause is the bare "no" CLOUD-122 forbids, and an `Option` invites one. + #[must_use] + pub fn bypass_env_for(&self, rule: &str) -> &str { + self.shapes + .iter() + .find(|row| row.id == rule) + .and_then(|row| row.bypass_env.as_deref()) + .unwrap_or(BYPASS_ENV) + } + + /// This policy with every row whose declared hatch is in `set` removed + /// (CLOUD-437). + /// + /// **A row is REMOVED, never short-circuited to allow, and that is the whole + /// design.** Suppressing a refusal post-hoc would stop adjudication at the + /// row that fired, so setting one row's hatch would silently switch off every + /// row behind it — which is exactly the invisible blast radius a single + /// global hatch has and this column exists to end. The bash guards this + /// surface inherited were separate programs, so suppressing `memory-guard` + /// left `ready-guard` live; removing the row is what reproduces that. + /// + /// Cheap when irrelevant (house-style §4): the caller skips this entirely + /// when nothing is set, so the ordinary mediated call — every call, on every + /// tool use — clones nothing. + #[must_use] + pub fn without_hatched(&self, set: &std::collections::BTreeSet) -> Policy { + Policy { + shapes: self + .shapes + .iter() + .filter(|row| { + row.bypass_env + .as_deref() + .is_none_or(|hatch| !set.contains(hatch)) + }) + .cloned() + .collect(), + harness: self.harness, + fail_on_warning: self.fail_on_warning, + verbs: self.verbs.clone(), + protected: self.protected.clone(), + redirects: self.redirects.clone(), + facts: self.facts.clone(), + bundles: self.bundles.clone(), + } + } + + /// Every distinct hatch the loaded rows declare, for the boundary to resolve + /// against the environment (CLOUD-437). + /// + /// The boundary cannot read these before config loads — the rows are not + /// known yet — so [`BYPASS_ENV`] keeps its early position as the whole-call + /// switch and these are read after. Returning the names rather than reading + /// them here is what keeps [`adjudicate`] pure: this module never touches the + /// environment, and the caller hands back which of these were set. + #[must_use] + pub fn declared_hatches(&self) -> std::collections::BTreeSet<&str> { + self.shapes + .iter() + .filter_map(|row| row.bypass_env.as_deref()) + .collect() + } } /// Adjudicate an envelope against the policy, then apply the waivers. @@ -3060,9 +3160,19 @@ fn adjudicated(policy: &Policy, envelope: &Envelope, facts: &Facts<'_>) -> Decis /// the payload: the escape hatch. `check`'s refusal of a rule it cannot honestly /// run carries the same [`Refusal`] and no hatch, which is correct — there is /// nothing to bypass in a read-only run. +/// +/// `hatch` is the name **this** refusal's row declared, or [`BYPASS_ENV`] where +/// it declared none (CLOUD-437). Passed in rather than read off the [`Refusal`] +/// deliberately: the hatch is a fact about mediation and the refusal is shared +/// with `check`, so putting it on the payload would give a read-only finding a +/// field that can only ever be meaningless there. [`Policy::bypass_env_for`] is +/// the one place the row is looked up, so the string a deny prints and the +/// variable the boundary reads stay one definition — the property this module's +/// single-constant comment has always claimed and could not keep once the hatch +/// stopped being single. #[must_use] -pub fn deny_text(refusal: &Refusal) -> String { - format!("{} Bypass with {BYPASS_ENV}=1.", refusal.render()) +pub fn deny_text(refusal: &Refusal, hatch: &str) -> String { + format!("{} Bypass with {hatch}=1.", refusal.render()) } /// The first shape row that matches the mediated command, in declaration order. @@ -3072,27 +3182,42 @@ pub fn deny_text(refusal: &Refusal) -> String { /// a rule about rules that the config does not state. /// The receipt verdicts a mediated call is judged against. /// -/// `None` is **no receipt question to answer here**, and allows — the fail-open -/// posture every retiring guard has. Two things resolve to it, and both are the -/// boundary's to decide because both are questions about a checkout: -/// -/// * **could not look** — no checkout, an `origin/main` that does not resolve, or -/// a detached HEAD where a branch-keyed row needs a branch; -/// * **nothing judgeable** — the call writes a path policy does not judge -/// (git-ignored, outside the repository, inside `.git`), which is -/// [`crate::receipt::judgeable`]'s answer and CLOUD-444's exclusion set. -/// -/// `Some` map missing a name is treated as [`Validity::Missing`], so a boundary -/// that resolved fewer facts than the policy needs fails closed rather than -/// silently allowing. -pub type ReceiptFacts = Option>; +/// Three-valued on [`crate::facts::Look`], which is the contract `facts.rs` +/// states and this alias now practises rather than restates (CLOUD-787). The +/// two answers that are not a verdict map both **allow** — the fail-open posture +/// every retiring guard has — and they are still two answers, because the +/// previous spelling had them as one: +/// +/// * [`Look::CouldNotLook`](crate::facts::Look::CouldNotLook) — the question +/// could not be asked: no checkout, an `origin/main` that does not resolve, or +/// a detached HEAD where a branch-keyed row needs a branch. +/// * [`Look::IsNot`](crate::facts::Look::IsNot) — the boundary looked and there +/// is **nothing judgeable** here: no required check selected this call, or the +/// call writes a path policy does not judge (git-ignored, outside the +/// repository, inside `.git`), which is [`crate::receipt::judgeable`]'s answer +/// and CLOUD-444's exclusion set. +/// +/// Both were `None` until CLOUD-787, so a reader reaching for `Option` got two +/// values where the contract has three, and the next call site written would +/// have collapsed "looked and found nothing" into "could not look". They allow +/// alike today and nothing here asks them to keep doing so: the point is that a +/// predicate can now tell them apart before deciding. +/// +/// An [`Look::Is`](crate::facts::Look::Is) map missing a name is treated as +/// [`Validity::Missing`], so a boundary that resolved fewer facts than the +/// policy needs fails closed rather than silently allowing. +pub type ReceiptFacts = crate::facts::Look>; /// The checkout evidence a `requires_key` shape row is judged against /// (CLOUD-446): the branch name, and the commit messages on `base..HEAD`. /// -/// `None` is **could not look** and allows, exactly as it does for -/// [`ReceiptFacts`] — outside a checkout, on a detached HEAD, or against a `base` -/// git cannot resolve. Resolved at the boundary because [`adjudicate`] is +/// [`Look::CouldNotLook`](crate::facts::Look::CouldNotLook) allows, exactly as +/// it does for [`ReceiptFacts`] — outside a checkout, on a detached HEAD, in a +/// shallow clone, or against a `base` git cannot resolve. +/// [`Look::IsNot`](crate::facts::Look::IsNot) is the other non-answer and is a +/// different one: no `requires_key` row selected this command, so the boundary +/// looked and found no key question to ask. Resolved at the boundary because +/// [`adjudicate`] is /// contractually pure, and resolved only when a `requires_key` row has already /// selected this command ([`Policy::key_base_for`]), so a repository declaring no /// such row pays nothing on the hottest path in the binary. @@ -3101,12 +3226,17 @@ pub type ReceiptFacts = Option>; /// reader asks the same question of all of it — does the expression match /// anywhere — and a field a caller could *print* is one an unreviewed edit turns /// into a leaked commit message (non-negotiable rule 4). -pub type KeyFacts = Option>; +pub type KeyFacts = crate::facts::Look>; /// What the AGENT reported for each agent-sourced check (CLOUD-776, CLOUD-834). /// -/// `None` is could-not-look, exactly as [`ReceiptFacts`]'s is; a check absent -/// from the map is one the boundary was not asked about. Kept beside the +/// `None` here is could-not-look. It is deliberately still an `Option` rather +/// than a [`crate::facts::Look`], and CLOUD-787 left it that way on purpose: +/// this fact has no `IsNot` producer — [`crate::lib`]'s `agent_records` answers +/// `None` only when no required check is agent-sourced — so the third arm would +/// be dead, and a three-valued type whose third value cannot occur misleads in +/// the other direction. A check absent from the map is one the boundary was not +/// asked about. Kept beside the /// receipt verdicts rather than folded into them because they answer different /// questions: a `Validity` says whether the check is satisfied, and this says /// what the agent actually ran and what it found. Collapsing the two would give @@ -3259,8 +3389,9 @@ pub type ManifestFacts = Option; /// **Resolved at the boundary, held by value.** Every field is a fact the /// boundary already looked up before [`adjudicate`] was called — which is what /// keeps that function contractually pure, and why projecting the set into the -/// policy input costs no resolution. `Option::None` is could-not-look -/// throughout, never "resolved to nothing". +/// policy input costs no resolution. A fact that was not resolved says so in its +/// own type — [`crate::facts::Look::CouldNotLook`] where the field carries one, +/// `Option::None` where it does not — and never "resolved to nothing". #[derive(Debug, Clone, Copy)] #[non_exhaustive] pub struct Facts<'a> { @@ -3305,8 +3436,8 @@ impl<'a> Facts<'a> { ) -> Facts<'a> { Facts { bypass: false, - receipts: &None, - keys: &None, + receipts: &crate::facts::Look::CouldNotLook, + keys: &crate::facts::Look::CouldNotLook, stop, waived, sourced: &None, @@ -3436,8 +3567,13 @@ fn matching_receipt_rows<'a>(policy: &'a Policy, envelope: &Envelope) -> Vec<&'a /// not a tidiness: this gate sees a row only if that row's own `tool` selected /// this call. fn tool_receipt_rules(policy: &Policy, envelope: &Envelope, facts: &ReceiptFacts) -> Decision { - let Some(facts) = facts.as_ref() else { - return Decision::Allow; + // Both non-answers allow, and for the same reason [`receipt_rules`] gives + // them one arm: a boundary that had no question to ask and a boundary that + // could not ask reach the same verdict here, and neither is evidence a + // receipt is missing (CLOUD-787). + let facts = match facts { + crate::facts::Look::Is(resolved) => resolved, + crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => return Decision::Allow, }; if envelope.raw_tool.is_empty() { return Decision::Allow; @@ -3465,11 +3601,14 @@ fn tool_receipt_rules(policy: &Policy, envelope: &Envelope, facts: &ReceiptFacts } fn receipt_rules(policy: &Policy, envelope: &Envelope, facts: &ReceiptFacts) -> Decision { - // No facts means there is no receipt question to answer here. Allow: a guard - // that cannot read its own precondition must not become the reason work - // stops. - let Some(facts) = facts.as_ref() else { - return Decision::Allow; + // Neither non-answer is a receipt question to answer here, and both allow: a + // guard that cannot read its own precondition must not become the reason + // work stops, and neither must one told there is nothing to judge. They + // reach the same verdict by two different routes, which is the distinction + // `Option` could not hold (CLOUD-787). + let facts = match facts { + crate::facts::Look::Is(resolved) => resolved, + crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => return Decision::Allow, }; for rule in matching_receipt_rows(policy, envelope) { // Every named receipt must be valid. An unresolved name is Missing, @@ -4342,9 +4481,18 @@ fn call_document(envelope: &Envelope, facts: &Facts<'_>) -> Result Some(serde_json::Value::Bool(facts.bypass)), - crate::facts::Fact::Receipts => Some(facts.receipts.as_ref().map_or( - serde_json::Value::Null, - |verdicts| { + // BOTH non-answers project as `null`, which is what keeps this a + // type substitution rather than a document change (CLOUD-787): the + // previous `Option` spelling emitted `null` for could-not-look and + // for nothing-judgeable alike, and the bytes here are unchanged. The + // distinction is live in Rust, where the next call site is written; + // giving Rego its own spelling of it is a widening of the policy + // input and belongs to whichever row needs a predicate on it. + crate::facts::Fact::Receipts => Some(match facts.receipts { + crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => { + serde_json::Value::Null + } + crate::facts::Look::Is(verdicts) => { // The VERDICT's stable token, never the receipt statement: // a receipt carries a subject commit and a recorded ref, and // a predicate decides on `valid` / `stale-head` / @@ -4355,14 +4503,15 @@ fn call_document(envelope: &Envelope, facts: &Facts<'_>) -> Result Some( - facts - .keys - .as_ref() - .map_or(serde_json::Value::Null, |found| serde_json::json!(found)), - ), + } + }), + // Same reading as `Receipts` one arm up, and the same unchanged bytes. + crate::facts::Fact::Keys => Some(match facts.keys { + crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => { + serde_json::Value::Null + } + crate::facts::Look::Is(found) => serde_json::json!(found), + }), crate::facts::Fact::Stop => Some(serde_json::json!(facts.stop)), crate::facts::Fact::Waived => Some(serde_json::json!(facts.waived)), crate::facts::Fact::AgentSourced => Some(facts.sourced.as_ref().map_or( @@ -4442,6 +4591,18 @@ fn call_document(envelope: &Envelope, facts: &Facts<'_>) -> Result None, + // Not resolvable on the mediated call, and the strongest case of the + // three above: this PARSES a file of unbounded size where `Lines` + // only reads one, so it is unbounded in the input twice over + // (CLOUD-914). A call site is a property of committed source, which + // is the tree surface's question, and the mediated path has no + // budget for a syntax tree. + crate::facts::Fact::Invocations => None, + // Not resolvable on the mediated call, for `Invocations`' reason and + // one more: a `use` graph is a property of the WHOLE crate, resolved + // against the root's table across every declared file, so it is + // unbounded in the tree rather than in one file (CLOUD-762). + crate::facts::Fact::Uses => None, // Not resolvable on the mediated call, same axis and same reason // (CLOUD-851): the sink store is read for every key the RULESET // declares, and that count is unbounded in the ruleset where a single @@ -4501,10 +4662,14 @@ fn call_document(envelope: &Envelope, facts: &Facts<'_>) -> Result bool { if pattern.is_match(command) { return true; } - let Some(evidence) = keys else { + let crate::facts::Look::Is(evidence) = keys else { return true; }; evidence.iter().any(|text| pattern.is_match(text)) @@ -5680,6 +5845,7 @@ mod tests { requires_key: None, reason: Some(format!("use the sanctioned path for {id}")), policy_url: None, + bypass_env: None, check: None, fix: None, produces: None, @@ -5702,6 +5868,10 @@ mod tests { sources: Vec::new(), lines: Vec::new(), line_sources: Vec::new(), + invocations: Vec::new(), + invocation_sources: Vec::new(), + uses: Vec::new(), + use_sources: Vec::new(), git: Vec::new(), refs: Vec::new(), ranges: Vec::new(), @@ -5793,8 +5963,8 @@ mod tests { ]), &envelope(command), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) } @@ -6295,8 +6465,8 @@ mod tests { &gh_policy(), &envelope(command), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) } @@ -6314,7 +6484,7 @@ mod tests { /// fail here. fn denial_text(decision: Decision) -> String { match decision { - Decision::Deny(refusal) => deny_text(&refusal), + Decision::Deny(refusal) => deny_text(&refusal, BYPASS_ENV), // An `Ask` is not a deny, and collapsing the two here would let a // row that silently started escalating keep passing every assertion // below about what a refusal says. A `Waived` is not one either, and @@ -6382,8 +6552,8 @@ mod tests { &program_only_shape_policy(), &envelope(command), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Deny(_) @@ -6457,8 +6627,8 @@ mod tests { &require_via_policy(), &envelope(command), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Deny(_) @@ -6606,8 +6776,8 @@ mod tests { &gh_policy(), &envelope("gh pr merge"), true, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Allow @@ -6688,8 +6858,8 @@ mod tests { &gh_policy(), &envelope, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Deny(_) @@ -6706,8 +6876,8 @@ mod tests { &gh_policy(), &envelope_at(event, "gh pr merge 42"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ); if event == Event::PreTool { @@ -6729,8 +6899,8 @@ mod tests { &gh_policy(), &envelope, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Allow @@ -6747,8 +6917,8 @@ mod tests { &Policy::declaring_nothing(Harness::ExitCode), &envelope("gh pr merge 42"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow @@ -6815,8 +6985,8 @@ mod tests { &policy, &envelope("gh pr merge 42"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Allow @@ -6844,8 +7014,8 @@ mod tests { &advisory, &call, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Allow, @@ -6868,8 +7038,8 @@ mod tests { &promoted, &call, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Deny(_) @@ -6900,8 +7070,8 @@ mod tests { &policy, &envelope("gh pr merge"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), )); assert!(reason.contains("first"), "got: {reason}"); @@ -6940,8 +7110,8 @@ mod tests { &policy, &envelope("gh pr merge"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), )); assert!(reason.contains("example.invalid/policy"), "got: {reason}"); @@ -6972,8 +7142,8 @@ mod tests { ]), &write_envelope(tool, path), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) } @@ -7117,6 +7287,63 @@ mod tests { } } + /// The two non-answers are DIFFERENT VALUES that project to the SAME bytes. + /// + /// CLOUD-787's whole claim in one case. Before it, `receipts` and `keys` + /// spelled could-not-look and nothing-judgeable as one `None`, so no call + /// site could tell them apart; they are `Look::CouldNotLook` and + /// `Look::IsNot` now, and `as_str` separates them. + /// + /// The second half is what makes this a type substitution rather than a + /// change: both still project as `null`, exactly what the `Option` spelling + /// emitted, so no consumer module and no schema moved. Giving Rego its own + /// spelling of the distinction is a widening of the document and belongs to + /// whichever row needs a predicate on it. + /// + /// Fails by: collapsing the two arms at the call site — give `IsNot` the + /// same token as `CouldNotLook` and the first assertion goes red; project + /// `IsNot` as anything but `null` and the second does. + #[test] + fn the_two_non_answers_are_distinct_values_projecting_identical_bytes() { + let is_not: ReceiptFacts = crate::facts::Look::IsNot; + let could_not: ReceiptFacts = crate::facts::Look::CouldNotLook; + assert_ne!( + is_not.as_str(), + could_not.as_str(), + "looked-and-found-nothing must not share a token with could-not-look" + ); + assert!(!is_not.could_not_look() && could_not.could_not_look()); + + let stop = crate::stop::StopFacts::default(); + let waived = crate::waiver::Live::new(); + let rendered = |receipts: &ReceiptFacts, keys: &KeyFacts| { + let document = call_document( + &envelope("git status"), + &Facts { + receipts, + keys, + ..Facts::none(&stop, &waived) + }, + ) + .expect("the document serializes"); + let parsed: serde_json::Value = + serde_json::from_str(&document).expect("the document parses"); + parsed["facts"].clone() + }; + assert_eq!( + rendered(&crate::facts::Look::IsNot, &crate::facts::Look::IsNot), + rendered( + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook + ), + "the split is a Rust distinction; the policy input is unchanged" + ); + assert_eq!( + rendered(&crate::facts::Look::IsNot, &crate::facts::Look::IsNot)["receipts"], + serde_json::Value::Null + ); + } + /// The projection carries the VERDICT token, never the receipt statement. /// /// A receipt records a subject commit and the `origin/main` it was linear @@ -7133,7 +7360,7 @@ mod tests { let document = call_document( &envelope("git status"), &Facts { - receipts: &Some(verdicts), + receipts: &crate::facts::Look::Is(verdicts), ..Facts::none( &crate::stop::StopFacts::default(), &crate::waiver::Live::new(), @@ -7177,12 +7404,12 @@ violation contains { let stale = { let mut verdicts = std::collections::BTreeMap::new(); verdicts.insert("verify".to_owned(), crate::receipt::Validity::StaleHead); - Some(verdicts) + crate::facts::Look::Is(verdicts) }; let fresh = { let mut verdicts = std::collections::BTreeMap::new(); verdicts.insert("verify".to_owned(), crate::receipt::Validity::Valid); - Some(verdicts) + crate::facts::Look::Is(verdicts) }; let envelope = envelope("git push"); @@ -7191,7 +7418,7 @@ violation contains { &envelope, false, &stale, - &None, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ); assert!( @@ -7205,7 +7432,7 @@ violation contains { &envelope, false, &fresh, - &None, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ); assert_eq!( @@ -7221,8 +7448,8 @@ violation contains { &policy, &envelope, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ); assert_eq!( @@ -7308,8 +7535,8 @@ deny contains "refused by the module" if { &policy, &envelope("run forbidden thing"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ); match decision { @@ -7334,8 +7561,8 @@ deny contains "refused by the module" if { &policy, &envelope("run something else"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow, @@ -7402,7 +7629,7 @@ deny contains "refused by the module" if { &envelope("gh pr ready 42"), false, facts, - &None, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) } @@ -7434,7 +7661,7 @@ deny contains "refused by the module" if { &write_envelope("Write", "crates/batten/src/new.rs"), false, facts, - &None, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) } @@ -7444,11 +7671,17 @@ deny contains "refused by the module" if { // The gap this closes: every write returned Allow before the command // gate ran, so a receipt row could never be a precondition for editing. assert!(matches!( - adjudicate_write(&Some(resolved(&[("claim", Validity::Missing)]))), + adjudicate_write(&crate::facts::Look::Is(resolved(&[( + "claim", + Validity::Missing + )]))), Decision::Deny(_) )); assert_eq!( - adjudicate_write(&Some(resolved(&[("claim", Validity::Valid)]))), + adjudicate_write(&crate::facts::Look::Is(resolved(&[( + "claim", + Validity::Valid + )]))), Decision::Allow ); } @@ -7462,8 +7695,8 @@ deny contains "refused by the module" if { &claim_policy(), &envelope("gh pr ready 42"), false, - &Some(resolved(&[("claim", Validity::Missing)])), - &None, + &crate::facts::Look::Is(resolved(&[("claim", Validity::Missing)])), + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow, @@ -7474,8 +7707,8 @@ deny contains "refused by the module" if { &receipt_policy(), &write_envelope("Write", "notes.md"), false, - &Some(resolved(&[("verify", Validity::Missing)])), - &None, + &crate::facts::Look::Is(resolved(&[("verify", Validity::Missing)])), + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow, @@ -7485,12 +7718,23 @@ deny contains "refused by the module" if { #[test] fn a_write_the_boundary_did_not_judge_is_allowed() { - // `None` is the boundary saying there is no receipt question here — a - // git-ignored path, one outside the repository, one inside `.git`, or a - // detached HEAD. Every one of them allows, and they are the load-bearing - // half: a gate that refused them would refuse every write in the - // container and be switched off within the hour. - assert_eq!(adjudicate_write(&None), Decision::Allow); + // Neither non-answer is a receipt question here, and both allow — the + // load-bearing half: a gate that refused them would refuse every write + // in the container and be switched off within the hour. + // + // CLOUD-787 split what this comment used to list as one value. A + // git-ignored path, one outside the repository and one inside `.git` + // are `IsNot` — the boundary looked and there is nothing to judge — and + // a detached HEAD is `CouldNotLook`. The assertion is unchanged in both + // arms; only the spelling of the fact moved. + assert_eq!( + adjudicate_write(&crate::facts::Look::CouldNotLook), + Decision::Allow + ); + assert_eq!( + adjudicate_write(&crate::facts::Look::IsNot), + Decision::Allow + ); } #[test] @@ -7498,7 +7742,7 @@ deny contains "refused by the module" if { // The wrong pointer this avoids: "no receipt for this commit" sends the // reader looking for a per-commit step, when what is missing is a claim // the whole branch shares. - let reason = denial_text(adjudicate_write(&Some(resolved(&[( + let reason = denial_text(adjudicate_write(&crate::facts::Look::Is(resolved(&[( "claim", Validity::Missing, )])))); @@ -7589,7 +7833,7 @@ deny contains "refused by the module" if { // coverage. It is the exact defect this whole issue is about, and it // was invisible because an unmatched rule is silent. assert!(matches!( - adjudicate_ready(&Some(resolved(&[ + adjudicate_ready(&crate::facts::Look::Is(resolved(&[ ("verify", Validity::Missing), ("linear-check", Validity::Missing), ]))), @@ -7602,7 +7846,7 @@ deny contains "refused by the module" if { // The conjunction is the predicate: a branch that verified but is no // longer linear on the trunk cannot fast-forward-land. assert!(matches!( - adjudicate_ready(&Some(resolved(&[ + adjudicate_ready(&crate::facts::Look::Is(resolved(&[ ("verify", Validity::Valid), ("linear-check", Validity::StaleMain), ]))), @@ -7613,7 +7857,7 @@ deny contains "refused by the module" if { #[test] fn all_receipts_valid_allows_the_call() { assert_eq!( - adjudicate_ready(&Some(resolved(&[ + adjudicate_ready(&crate::facts::Look::Is(resolved(&[ ("verify", Validity::Valid), ("linear-check", Validity::Valid), ]))), @@ -7627,7 +7871,10 @@ deny contains "refused by the module" if { // answered for fewer checks than the row requires has not established // the precondition. assert!(matches!( - adjudicate_ready(&Some(resolved(&[("verify", Validity::Valid)]))), + adjudicate_ready(&crate::facts::Look::Is(resolved(&[( + "verify", + Validity::Valid + )]))), Decision::Deny(_) )); } @@ -7637,8 +7884,16 @@ deny contains "refused by the module" if { // Outside a checkout there are no git facts to judge against. Fail // open, the posture every retiring guard has: a guard that cannot read // its own precondition must not become the reason work stops. This is - // deliberately distinct from `Some(Missing)`, which denies above. - assert_eq!(adjudicate_ready(&None), Decision::Allow); + // deliberately distinct from `Look::Is(Missing)`, which denies above — + // and, since CLOUD-787, from `IsNot`, which allows for the other reason. + assert_eq!( + adjudicate_ready(&crate::facts::Look::CouldNotLook), + Decision::Allow + ); + assert_eq!( + adjudicate_ready(&crate::facts::Look::IsNot), + Decision::Allow + ); } #[test] @@ -7648,8 +7903,8 @@ deny contains "refused by the module" if { &receipt_policy(), &envelope("gh pr view 42"), false, - &Some(resolved(&[("verify", Validity::Missing)])), - &None, + &crate::facts::Look::Is(resolved(&[("verify", Validity::Missing)])), + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow @@ -7689,8 +7944,8 @@ deny contains "refused by the module" if { &program_only_receipt_policy(), &envelope(command), false, - &Some(resolved(&[("toolchain", Validity::Missing)])), - &None, + &crate::facts::Look::Is(resolved(&[("toolchain", Validity::Missing)])), + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Deny(_) @@ -7725,7 +7980,7 @@ deny contains "refused by the module" if { fn the_refusal_names_the_check_and_what_is_wrong_with_it() { // Three verdicts, three causes — which is how `ready-guard`'s three // hand-written deny messages survive the move into one config row. - let Decision::Deny(refusal) = adjudicate_ready(&Some(resolved(&[ + let Decision::Deny(refusal) = adjudicate_ready(&crate::facts::Look::Is(resolved(&[ ("verify", Validity::Valid), ("linear-check", Validity::StaleHead), ]))) else { @@ -7806,8 +8061,8 @@ deny contains "refused by the module" if { &protected_policy(vec![verb("Write", Some("use the owning surface"))]), &envelope, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow @@ -7837,8 +8092,8 @@ deny contains "refused by the module" if { &protected_policy(verbs), &envelope(command), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) } @@ -7989,8 +8244,8 @@ deny contains "refused by the module" if { &protected_policy(vec![row]), &write_envelope("Write", "batten.toml"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Deny(_) @@ -8005,8 +8260,8 @@ deny contains "refused by the module" if { &protected_policy(vec![verb("Write", None)]), &write_envelope("Write", "batten.toml"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Deny(_) @@ -8145,8 +8400,8 @@ deny contains "refused by the module" if { ), &envelope(command), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) } @@ -8314,8 +8569,8 @@ deny contains "refused by the module" if { &no_verbs, &envelope("rm batten.toml"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Allow @@ -8335,8 +8590,8 @@ deny contains "refused by the module" if { &no_paths, &envelope("rm batten.toml"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Allow @@ -8353,8 +8608,8 @@ deny contains "refused by the module" if { &policy, &envelope("rm .serena/memories/core.md"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), )); assert!(reason.contains("no-rm-memories"), "got: {reason}"); @@ -8367,8 +8622,8 @@ deny contains "refused by the module" if { &protected_policy(vec![verb("rm", None)]), &envelope("rm batten.toml"), true, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow @@ -8430,8 +8685,8 @@ deny contains "refused by the module" if { &guarding, &call, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Deny(_) @@ -8443,8 +8698,8 @@ deny contains "refused by the module" if { &elsewhere, &call, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default() ), Decision::Allow, @@ -9211,8 +9466,8 @@ deny contains "refused by the module" if { &policy, &envelope, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ); assert!( @@ -9519,8 +9774,8 @@ deny contains "refused by the module" if { &policy, &write_envelope_on(*harness, spelling, "batten.toml"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ); assert!( @@ -9549,8 +9804,8 @@ deny contains "refused by the module" if { &policy, &write_envelope_on(Harness::GeminiCli, "WriteFile", "batten.toml"), false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ) else { panic!("a host-spelled write against a protected path must deny"); @@ -9765,8 +10020,8 @@ deny contains "refused by the module" if { &policy, &read, false, - &None, - &None, + &crate::facts::Look::CouldNotLook, + &crate::facts::Look::CouldNotLook, &crate::stop::StopFacts::default(), ), Decision::Allow diff --git a/crates/batten/src/identity.rs b/crates/batten/src/identity.rs index 370c849ec..05ca794fc 100644 --- a/crates/batten/src/identity.rs +++ b/crates/batten/src/identity.rs @@ -1083,6 +1083,79 @@ pub const fn compare_to_anchor(anchor: u64, current: u64) -> CountChange { mod tests { use super::*; + // -- CLOUD-594: the golden vectors. -- + // + // Every other identity test in this module re-derives its expected value + // with the same crate that produced it, so the suite proves the function is + // SELF-CONSISTENT and says nothing about what bytes it emits. That makes the + // hashing substrate an untested input: a `sha2`/`hmac` major that changed a + // length-prefix, a `finalize` width or a keying rule would re-key every + // fingerprint in every consumer's store — every open finding re-opening as + // new — and compile, self-check and land green. + // + // These two constants are the assertion that makes such a move CHECKABLE. + // They are derived independently rather than transcribed from a run of this + // code, which is the property that matters: a value captured from the + // implementation would pin whatever the implementation does, including a + // framing bug, and would agree with a substrate change that moved both + // sides. Recorded here on `sha2` 0.10 / `hmac` 0.12; if they move under a + // bump, THE BUMP STOPS — that is a breaking change to finding identity and + // an identity-version decision, not a dependency update. + // + // Rule 4 is respected by construction: the inputs are synthetic and the key + // is a test constant, never a minted one, so no real key material and no + // real preimage enters the tree. + + /// A fixed 32-byte key. `IdentityKey::new` takes the bytes from the caller, + /// so this mints nothing and touches no store. + fn vector_key() -> IdentityKey { + let mut bytes = [0u8; 32]; + for (i, slot) in bytes.iter_mut().enumerate() { + *slot = u8::try_from(i).unwrap_or(0); + } + IdentityKey::new("vector", bytes) + } + + #[test] + fn tagged_fingerprint_emits_its_recorded_bytes() { + // Independently computed as SHA-256 over, for the tag and then each + // field in order, the field's length as a little-endian u64 followed by + // the field's bytes. The empty field is deliberate: it is the case that + // distinguishes a length-prefixed framing from a bare concatenation. + assert_eq!( + tagged_fingerprint("batten-golden", &[b"alpha", b"", b"beta"]).to_hex(), + "850cf14f8b273d63c6e7a3b4f77c1d0f70cb4e4ac14292194c6d933f75109a36", + "the identity function's emitted bytes moved — see CLOUD-594 before touching this" + ); + } + + #[test] + fn keyed_span_emits_its_recorded_bytes() { + // Independently computed as HMAC-SHA256(key = 0x00..=0x1f, msg). + assert_eq!( + Fingerprint(keyed_span(&vector_key(), "the quick brown fox").unwrap()).to_hex(), + "d42d6afb45f6f5fe9d6ae6a2f7c331e50cf14540995da07a659d02d1077625a2", + "the keyed identity function's emitted bytes moved — see CLOUD-594" + ); + } + + #[test] + fn the_length_prefix_is_what_keeps_the_framing_unambiguous() { + // The property the prefix exists for, pinned against recorded bytes + // rather than against each other: without it these two hash + // identically, so a rename could be made invisible by choosing names. + // Asserting only `assert_ne!` would still pass if BOTH values moved, + // which is exactly the substrate change these vectors exist to catch. + assert_eq!( + tagged_fingerprint("t", &[b"ab", b"c"]).to_hex(), + "0bce0a90f1fdaa2e81a805ebdbbec8c3656e9aefd51ff968dc27ca1c592c798e" + ); + assert_eq!( + tagged_fingerprint("t", &[b"a", b"bc"]).to_hex(), + "1a2216e7f1dc0afd95be6d22dc6b52bb596ef2be3423092f0a7b5fa1825bf398" + ); + } + // -- S12 fixture: formatter reflow must preserve collapsed identity. -- #[test] fn whitespace_reflow_preserves_collapsed_identity() { diff --git a/crates/batten/src/invocation.rs b/crates/batten/src/invocation.rs new file mode 100644 index 000000000..4e68e2ae9 --- /dev/null +++ b/crates/batten/src/invocation.rs @@ -0,0 +1,283 @@ +//! Rust call sites, parsed — the syntactic POSITION of a token (CLOUD-914). +//! +//! `facts.rs` stays the one authority on what [`crate::facts::Fact::Invocations`] +//! IS and what it costs; this module is where the argv is actually read, which +//! is the split that row's §1 asks for. +//! +//! **It lives here rather than in `facts.rs` because of a gate, and the gate was +//! right.** `tests/facts.rs`'s `no_axis_match_carries_a_wildcard_arm` refuses a +//! wildcard arm anywhere in `facts.rs`, so that a fact added later fails to +//! compile instead of classifying itself. The walk below matches on +//! `syn::Expr`, which has some forty variants and gains more with every minor +//! release of the parser — a wildcard there is correct, and enumerating it would +//! be a list that breaks on an upgrade for no property gained. Two matches with +//! opposite right answers do not belong in one file, and moving this out is the +//! honest resolution rather than widening the gate. + +/// One call site: what it invokes, and the literal arguments it passes. +/// +/// Pointer-bearing by construction (non-negotiable rule 4). `line` is what a +/// finding reports; `program` and `arguments` are what a PREDICATE decides on +/// and are never rendered into a finding by the engine. That split is +/// [`Fact::Prospective`]'s exactly — a rule may decide over content, and what is +/// reported is `path:line` and a rule id. +#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize)] +#[non_exhaustive] +pub struct Invocation { + /// The callee as written — `Command::new`, `arg`, `git::query`. The path or + /// method name, never a resolved type: this is the syntactic tier, and + /// claiming resolution it does not do is how `grep` counted 14 `Command` + /// sites where name resolution finds 9. + pub program: String, + /// The string literals passed as ARGUMENTS to this call, in source order. + /// + /// Arguments only, never the receiver — the whole discriminator. A literal + /// in an array initialiser or in a method call's receiver is not an argument + /// and does not appear here, which is why a needle table reads as silent + /// while the same token passed to `.arg(..)` does not. + pub arguments: Vec, + /// 1-indexed line of the call site, for the pointer a finding carries. + pub line: usize, +} + +/// The visitor's accumulator, at module scope rather than inside +/// [`invocations`]: it holds no borrow from that function, and an item declared +/// after a statement reads as if it were scoped to what precedes it when it is +/// not (`clippy::items_after_statements`). +#[derive(Default)] +struct Sites { + found: Vec, +} + +/// The literals a call PASSES. Descends through the grouping expressions an +/// argument can legally wear — a reference, a parenthesis, a cast, an array or +/// tuple built inline at the call — because a borrowed array of string literals +/// is one argument spelled four nodes deep, and is the ordinary way this tree +/// passes an argv. +/// +/// It deliberately does NOT descend into a nested CALL: that call's own +/// arguments belong to it, and the visitor reaches it separately. Folding them +/// upward would attribute an inner call's literals to its caller. +fn literals(expr: &syn::Expr, out: &mut Vec) { + match expr { + syn::Expr::Lit(lit) => { + if let syn::Lit::Str(text) = &lit.lit { + out.push(text.value()); + } + } + syn::Expr::Reference(inner) => literals(&inner.expr, out), + syn::Expr::Paren(inner) => literals(&inner.expr, out), + syn::Expr::Group(inner) => literals(&inner.expr, out), + syn::Expr::Cast(inner) => literals(&inner.expr, out), + syn::Expr::Array(array) => { + for element in &array.elems { + literals(element, out); + } + } + syn::Expr::Tuple(tuple) => { + for element in &tuple.elems { + literals(element, out); + } + } + _ => {} + } +} + +impl<'ast> syn::visit::Visit<'ast> for Sites { + fn visit_expr_call(&mut self, call: &'ast syn::ExprCall) { + let mut arguments = Vec::new(); + for argument in &call.args { + literals(argument, &mut arguments); + } + self.found.push(Invocation { + program: callee(&call.func), + arguments, + line: line_of(call.paren_token.span.open()), + }); + syn::visit::visit_expr_call(self, call); + } + + fn visit_expr_method_call(&mut self, call: &'ast syn::ExprMethodCall) { + let mut arguments = Vec::new(); + for argument in &call.args { + literals(argument, &mut arguments); + } + // The RECEIVER is not an argument, and not descending into it here is + // not an omission: `visit_expr_method_call` below walks it, so a call + // inside the receiver is still found as its own site. What is excluded + // is the receiver's LITERALS being read as this call's arguments — + // which is exactly what makes a concatenated needle array silent and + // the same token passed to `.arg(..)` loud. + self.found.push(Invocation { + program: call.method.to_string(), + arguments, + line: line_of(call.method.span()), + }); + syn::visit::visit_expr_method_call(self, call); + } +} + +/// Every call site in a Rust source text, or the reason there are none. +/// +/// [`Look::CouldNotLook`] when the text is not parseable Rust — **never an empty +/// list**, which is the parse-coverage obligation CLOUD-310 attached to any +/// embedded matcher after measuring silent partial parses that emitted zero +/// error nodes. Rego reads an undefined path as "does not hold", so a gate whose +/// corpus failed to parse would report clean; that is CLOUD-845's vacuous pass +/// and CLOUD-251 named the class before it. +/// +/// A file that parses and calls nothing is `Is(vec![])` — looked, and there are +/// no call sites. The two are different answers and this function keeps them so. +#[must_use] +pub fn invocations(source: &str) -> crate::facts::Look> { + use syn::visit::Visit; + + let Ok(file) = syn::parse_file(source) else { + return crate::facts::Look::CouldNotLook; + }; + let mut sites = Sites::default(); + sites.visit_file(&file); + crate::facts::Look::Is(sites.found) +} + +/// The callee as written, flattened to a path string. +fn callee(func: &syn::Expr) -> String { + match func { + syn::Expr::Path(path) => path + .path + .segments + .iter() + .map(|segment| segment.ident.to_string()) + .collect::>() + .join("::"), + // A call whose callee is computed — a closure variable, an index, a + // field. There is a call site and its program is not a name, which is a + // different answer from "no call site" and is spelled as one. + _ => String::new(), + } +} + +/// 1-indexed source line of a span. +fn line_of(span: proc_macro2::Span) -> usize { + span.start().line +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used)] +mod tests { + use super::{Invocation, invocations}; + use crate::facts::Look; + + /// Every call site's arguments, flattened, for a case that only cares what + /// reached the argument tier at all. + fn arguments(source: &str) -> Vec { + match invocations(source) { + Look::Is(sites) => sites.into_iter().flat_map(|site| site.arguments).collect(), + other => panic!("expected a parse, got {}", other.as_str()), + } + } + + /// THE DISCRIMINATOR THE FACT EXISTS FOR, and the fixture is the tree's own + /// shape rather than an invented one. + /// + /// The same token in four places: passed to a call, sitting in a needle + /// array, in a line comment, and in a doc comment. A line predicate — which + /// is what every argv-shaped guard in this crate is today — reports all + /// four. This reports one. + /// + /// Fails by: descending into a method call's receiver in `visit_expr_method_ + /// call`, which is the one-line change that makes the needle array read as + /// an argument and collapses the distinction. + #[test] + fn a_token_in_command_position_is_told_from_the_same_token_elsewhere() { + let source = r#" + // banned-token in a line comment + /// banned-token in a doc comment + fn f() { + let needles = ["banned-token", "other"].concat(); + run().arg("banned-token"); + let _ = needles; + } + "#; + let found = arguments(source); + assert_eq!( + found.iter().filter(|arg| *arg == "banned-token").count(), + 1, + "exactly the call-position occurrence, got {found:?}" + ); + } + + /// A receiver is not an argument, stated on its own because it is the half a + /// reader is most likely to assume works the other way. + #[test] + fn a_receivers_literals_are_not_the_calls_arguments() { + assert_eq!( + arguments(r#"fn f() { let _ = ["a", "b"].concat(); }"#), + Vec::::new() + ); + assert_eq!(arguments(r#"fn f() { g("a"); }"#), vec!["a".to_owned()]); + } + + /// A borrowed array IS one argument, four nodes deep — the ordinary way this + /// tree passes an argv, and the case a shallow reading would miss. + #[test] + fn a_borrowed_array_of_literals_is_read_as_arguments() { + assert_eq!( + arguments(r#"fn f() { query(&repo, &["rev-list", "--count"]); }"#), + vec!["rev-list".to_owned(), "--count".to_owned()] + ); + } + + /// COULD-NOT-LOOK IS NOT AN EMPTY SET, and the two are asserted against each + /// other rather than each alone — a test that checked only one would pass + /// over an implementation that collapsed them. + /// + /// This is CLOUD-310's parse-coverage obligation and CLOUD-914's §2 three- + /// valued clause meeting at one case: Rego reads an undefined path as "does + /// not hold", so a corpus that failed to parse would report clean. + /// + /// Fails by: returning `Look::Is(vec![])` from the parse-failure arm. + #[test] + fn an_unparseable_file_could_not_look_rather_than_finding_no_call_sites() { + let refused = invocations("fn f( {{{ this is not rust"); + assert!( + refused.could_not_look(), + "unparseable source is could-not-look, got {}", + refused.as_str() + ); + + let parsed_and_empty = invocations("struct S { field: u8 }"); + assert_eq!( + parsed_and_empty, + Look::Is(Vec::new()), + "a file that parses and calls nothing is a real answer" + ); + assert!( + !parsed_and_empty.could_not_look(), + "looked-and-found-nothing must not read as could-not-look" + ); + assert_ne!( + refused.as_str(), + parsed_and_empty.as_str(), + "the two answers must not share a token" + ); + } + + /// The pointer a finding carries is a real line, not a constant. + /// + /// Fails by: dropping the `span-locations` feature, which silently makes + /// every `Span::start()` report line 0 — a pointer-only finding whose + /// pointer is the same for every hit. + #[test] + fn a_call_site_carries_the_line_it_sits_on() { + let Look::Is(sites) = invocations("fn f() {\n\n g(\"x\");\n}") else { + panic!("expected a parse"); + }; + let site: &Invocation = sites.first().expect("one call site"); + assert_eq!(site.program, "g"); + assert_eq!( + site.line, 3, + "1-indexed, and not the 0 an unfeatured build gives" + ); + } +} diff --git a/crates/batten/src/lib.rs b/crates/batten/src/lib.rs index d9acf5c0d..f127892d5 100644 --- a/crates/batten/src/lib.rs +++ b/crates/batten/src/lib.rs @@ -39,6 +39,8 @@ pub mod handler; pub mod hook; pub mod identity; pub mod init; +/// Rust call sites, parsed — where a token sits, not merely that it appears. +pub mod invocation; pub mod journal; pub mod judge; pub mod lint; @@ -66,6 +68,9 @@ pub mod store; pub mod surface; pub mod transcript; pub mod trust; +/// The `use` graph: which module reaches which, resolved through the root's own +/// re-export table. +pub mod uses; pub mod verbs; pub mod waiver; pub mod worktree; @@ -1621,6 +1626,44 @@ impl SuiteReport { /// A [`UsageError`] (exit `1`) when the config will not resolve or a registered /// module will not load — both refused by [`policy::load`] with their own /// message, at the boundary where a config fault belongs. +/// The tree input one policy suite runs against. +/// +/// Split out of [`run_policy_test`] so that function reads as a loop over rows; +/// what it holds is the pair of deliberate omissions, which are easier to see +/// stated together than buried in an argument list. +/// +/// **A SUITE RUNS AGAINST NO PRODUCED RECORD** (CLOUD-851). A module's tests must +/// decide the same way on every machine, and the sink store is per-checkout state +/// that differs between them — so a test that wants a baseline supplies it with +/// `with input as`, the same way a mediated-call suite supplies its call. +/// +/// **AND AGAINST NO GIT FACT**, for exactly the same reason (CLOUD-907). HEAD, the +/// branch, the remotes and whether a ref resolves are all per-checkout state: a +/// suite that read them would pass on the author's machine and fail in CI's +/// detached, remote-less clone, which is a test asserting its environment rather +/// than its module. +fn suite_input( + rule: &rules::Rule, + documents: &std::collections::BTreeMap<(String, rules::Wanted), rules::Acquired>, + tracked: &[String], +) -> Result<(String, Vec<(String, rules::NotAcquired)>)> { + // A mediated-call row declares no documents, so its tests run against `{}` + // and supply their own input with `with input as` — OPA and Conftest's own + // shape, and the reason neither surface needs a fixture key. + Ok(rules::tree_document( + documents, + &rules::Declared { + documents: &rules::declared_documents(rule, tracked)?, + lines: &rules::declared_lines(rule, tracked)?, + invocations: &rules::declared_invocations(rule, tracked)?, + uses: &rules::declared_uses(rule, tracked)?, + }, + tracked, + &std::collections::BTreeMap::new(), + &git::GitFacts::default(), + )) +} + fn run_policy_test(json: bool, overrides: &Overrides, out: &mut dyn Write) -> Result { let root = Path::new("."); let config = resolve::resolve(root, overrides)?; @@ -1668,26 +1711,7 @@ fn run_policy_test(json: bool, overrides: &Overrides, out: &mut dyn Write) -> Re // mediated-call row declares no documents, so its tests run against `{}` // and supply their own input with `with input as` — OPA and Conftest's // own shape, and the reason neither surface needs a fixture key. - let declared = rules::declared_documents(rule, &tracked)?; - let (input, not_acquired) = rules::tree_document( - &documents, - &declared, - &rules::declared_lines(rule, &tracked)?, - &tracked, - // A SUITE RUNS AGAINST NO PRODUCED RECORD, deliberately (CLOUD-851). - // A module's tests must decide the same way on every machine, and the - // sink store is per-checkout state that differs between them — so a - // test that wants a baseline supplies it with `with input as`, the - // same way a mediated-call suite supplies its call. - &std::collections::BTreeMap::new(), - // AND AGAINST NO GIT FACT, for exactly the same reason (CLOUD-907). - // HEAD, the branch, the remotes and whether a ref resolves are all - // per-checkout state: a suite that read them would pass on the - // author's machine and fail in CI's detached, remote-less clone, - // which is a test asserting its environment rather than its module. - // A suite that wants a git fact supplies it with `with input as`. - &git::GitFacts::default(), - ); + let (input, not_acquired) = suite_input(rule, &documents, &tracked)?; if !not_acquired.is_empty() { // Pointer-only (rule 4): the PATH and its stated cause, never a byte // of the document. The cause is CLOUD-849's — before it, all four @@ -2071,13 +2095,18 @@ fn receipt_facts( max_ages: &std::collections::BTreeMap, judgeable: bool, ) -> hook::ReceiptFacts { + // The two non-answers are DIFFERENT answers, and CLOUD-787 is where they + // stopped being one. `IsNot` is the boundary having looked: no required + // check selected this call, or the write lands somewhere policy does not + // judge. `CouldNotLook` is further down, where the receipt store itself + // could not be read. Both allow, and they are not the same fact. if required.is_empty() || !judgeable { - return None; + return facts::Look::IsNot; } let mut verdicts = if receipted.is_empty() { - // Nothing to ask the receipt store. An EMPTY map rather than `None`: - // `None` is "could not look" and allows, and here we looked — there - // simply were no receipt-keyed checks among the ones required. + // Nothing to ask the receipt store, having looked: there simply were no + // receipt-keyed checks among the ones required. An EMPTY map, not a + // non-answer — the agent-sourced loop below fills it in. Some(std::collections::BTreeMap::new()) } else { // The subject a `named`-keyed row files under, resolved HERE because @@ -2111,7 +2140,36 @@ fn receipt_facts( verdicts.insert((*check).clone(), verdict); } } - verdicts + // A store the boundary could not read is the could-not-look arm — the one + // case here that is genuinely "the question could not be asked". + verdicts.map_or(facts::Look::CouldNotLook, facts::Look::Is) +} + +/// Drop every row whose declared `bypass_env` is set in this process's +/// environment (CLOUD-437). +/// +/// The environment read lives here rather than in [`hook`] for the reason every +/// other fact does: `adjudicate` is contractually pure, so the boundary looks and +/// the core decides. [`hook::Policy::declared_hatches`] hands back the names and +/// this answers which of them are set. +/// +/// **Cheap when irrelevant** (house-style §4). A ruleset declaring no +/// `bypass_env` — every ruleset until one opts in — collects an empty set, reads +/// no environment variable, and clones no policy. This runs on the hottest path +/// in the binary, so that is a requirement rather than a nicety. +fn without_set_hatches(policy: hook::Policy) -> hook::Policy { + let set: std::collections::BTreeSet = policy + .declared_hatches() + .into_iter() + .filter(|name| std::env::var_os(name).is_some_and(|value| !value.is_empty())) + .map(std::borrow::ToOwned::to_owned) + .collect(); + if set.is_empty() { + return policy; + } + // The row is REMOVED rather than its refusal suppressed, so the rows behind + // it still fire. See `hook::Policy::without_hatched`. + policy.without_hatched(&set) } fn run_hook( @@ -2254,6 +2312,18 @@ fn run_hook( } else { load_policy(overrides, harness)? }; + // THE PER-ROW HATCHES (CLOUD-437), resolved here and nowhere earlier. + // + // `BYPASS_ENV` is read before the config load above, because a bypassed call + // must never pay for one. These cannot be: which hatches exist is a property + // of the loaded rows, so the read has to follow the load. That ordering is + // the whole reason the general hatch survives as a separate switch rather + // than becoming just another row's name. + // + // Cheap when irrelevant (§4): a ruleset declaring no `bypass_env` — every + // ruleset until one opts in — collects an empty set, reads no environment, + // and clones no policy. + let policy = without_set_hatches(policy); // The receipt facts, resolved HERE because `adjudicate` is contractually // pure — no I/O, no environment, no clock — and a receipt predicate reads a // file and two git refs. Same split the bypass hatch already uses: the @@ -2294,7 +2364,11 @@ fn run_hook( // `requires_key` row has already selected this command. A repository // declaring none — and a call matching none, which is nearly every call — // does no git work here at all. - let keys: hook::KeyFacts = policy.key_base_for(&envelope).and_then(key_facts); + // `IsNot` where no `requires_key` row selected this command: the boundary + // looked and there is no key question. `key_facts` answers the other two. + let keys: hook::KeyFacts = policy + .key_base_for(&envelope) + .map_or(facts::Look::IsNot, key_facts); // The waiver facts (CLOUD-610), resolved HERE for exactly the reason above: // a waiver lapses on a date, `adjudicate` is contractually pure, and reading // the clock inside it would dissolve the contract rather than satisfy it. @@ -2376,7 +2450,22 @@ fn run_hook( emit_advisory(harness, &envelope, out, err, &advice.join("\n\n"))?; } let decision = handled.unwrap_or_else(|| hook::adjudicate(&policy, &envelope, &facts)); - render(harness, &envelope, decision, mode, out, err) + // Resolved HERE rather than inside `render`, because `render` deliberately + // cannot see the policy (CLOUD-898) and that property is worth more than the + // convenience: a renderer that cannot see the inputs cannot re-decide by + // accident. A hatch NAME is not such an input — it is a string the renderer + // prints and never branches on — so handing it over costs nothing. + let hatch = match &decision { + hook::Decision::Deny(refusal) | hook::Decision::Ask(refusal) => { + policy.bypass_env_for(refusal.rule()).to_owned() + } + // Nothing is being refused, so nothing advertises a hatch. The general + // name stands in rather than an empty string: the value is unread on + // these arms, and a blank one would render as `Bypass with =1.` if a + // later arm ever did read it. + _ => hook::BYPASS_ENV.to_owned(), + }; + render(harness, &envelope, decision, &hatch, mode, out, err) } /// Fill the advisory buffer from the two producers that ride a batch boundary. @@ -2524,15 +2613,25 @@ fn agent_records(checks: &[(&String, &rules::ReceiptKey)]) -> hook::AgentFacts { Some(records) } +/// The checkout evidence behind a `requires_key` row, or the reason there is none. +/// +/// Every early return here is [`facts::Look::CouldNotLook`] and each is the same +/// kind of failure: no checkout, a shallow clone whose history is not there to +/// read, or a `base` git cannot resolve. None of them is "looked and found no +/// key" — that answer belongs to the caller, which knows whether any row asked +/// (CLOUD-787). fn key_facts(base: &str) -> hook::KeyFacts { - let repo = git::repo_root(Path::new(".")).ok()?; - if git::is_shallow(&repo).ok()? { - return None; - } - let messages = git::log_messages(&repo, base).ok()??; - let mut evidence = vec![messages]; - evidence.extend(git::current_branch(&repo).ok().flatten()); - Some(evidence) + let inner = || -> Option> { + let repo = git::repo_root(Path::new(".")).ok()?; + if git::is_shallow(&repo).ok()? { + return None; + } + let messages = git::log_messages(&repo, base).ok()??; + let mut evidence = vec![messages]; + evidence.extend(git::current_branch(&repo).ok().flatten()); + Some(evidence) + }; + inner().map_or(facts::Look::CouldNotLook, facts::Look::Is) } /// Assemble the end-of-turn gate's inputs (CLOUD-85). @@ -3455,6 +3554,7 @@ fn render( harness: hook::Harness, envelope: &hook::Envelope, decision: hook::Decision, + hatch: &str, mode: Mode, out: &mut dyn Write, err: &mut dyn Write, @@ -3498,7 +3598,7 @@ fn render( // and the stderr line cannot disagree about what the refusal said or // whether it named a fix. hook::Decision::Deny(refusal) => { - let reason = hook::deny_text(&refusal); + let reason = hook::deny_text(&refusal, hatch); match hook::encode_deny(harness, &envelope.raw_event, &reason)? { Some(body) => { writeln!(out, "{body}")?; @@ -3517,7 +3617,7 @@ fn render( // degrading to "go ahead" is the one direction that inverts the policy, // and it is why `encode_ask`'s `None` means refuse rather than proceed. hook::Decision::Ask(refusal) => { - let reason = hook::deny_text(&refusal); + let reason = hook::deny_text(&refusal, hatch); match hook::encode_ask(harness, &envelope.raw_event, &reason)? { Some(body) => { writeln!(out, "{body}")?; @@ -5469,6 +5569,35 @@ mod tests { ); } + /// A `base` git cannot resolve is COULD-NOT-LOOK, never "no key here". + /// + /// CLOUD-787. `key_facts` is only ever called once a `requires_key` row has + /// already selected the command, so every failure inside it is the question + /// being unanswerable — no checkout, a shallow clone whose history is not + /// there to read, a `base` that resolves to nothing. "Looked and found no + /// key" is a different answer and it belongs to the CALLER, which is the one + /// that knows whether any row asked. + /// + /// Collapsing the two is the defect the three-valued contract exists to + /// prevent: `IsNot` here would claim the boundary had inspected a branch's + /// history and found no issue key, on a call where it never read one. + /// + /// Fails by: returning `Look::IsNot` from `key_facts`'s failure arm. + #[test] + fn an_unresolvable_base_could_not_look_rather_than_finding_no_key() { + let facts = key_facts("refs/heads/definitely-not-a-ref-cloud-787"); + assert!( + facts.could_not_look(), + "an unresolvable base is could-not-look, got {}", + facts.as_str() + ); + assert_eq!(facts.as_str(), "could-not-look"); + assert!( + !matches!(facts, facts::Look::IsNot), + "could-not-look must not be spelled as looked-and-found-nothing" + ); + } + /// A config declaring nothing still gets the line. The reader who suspects /// nothing ran is exactly the one who needs to be told they are right, and /// `0` is the honest answer rather than a reason to stay silent. diff --git a/crates/batten/src/rules.rs b/crates/batten/src/rules.rs index 11d9d8f24..0b06be8bf 100644 --- a/crates/batten/src/rules.rs +++ b/crates/batten/src/rules.rs @@ -72,6 +72,7 @@ const PIPELINE_PERMITS: &[&str] = &[ "substitutes", "reason", "policy_url", + "bypass_env", "severity", ]; @@ -162,6 +163,7 @@ const SHAPE_PERMITS: &[&str] = &[ "requires_key", "base", "policy_url", + "bypass_env", "severity", ]; @@ -183,6 +185,7 @@ const RECEIPT_PERMITS: &[&str] = &[ "reason", "contains", "policy_url", + "bypass_env", "severity", ]; @@ -778,11 +781,23 @@ impl RuleKind { "sources", "lines", "line_sources", + "invocations", + "invocation_sources", + "uses", + "use_sources", "severity", "predicate_severity", "identity_key", "reason", "policy_url", + // Permitted because this kind takes BOTH scopes: a + // `mediated-call` policy row denies through the same channel a + // `shape` row does, so it owes the same findable hatch. On a + // `tree`-scoped row it names nothing and costs nothing — + // `check` is read-only and has no mediation to suppress, which + // is the same reason `deny_text` is `hook`'s and not + // `Refusal`'s. + "bypass_env", "no_fix_reason", ], } @@ -1359,6 +1374,28 @@ pub struct Rule { /// An optional link to the policy this rule enforces, appended to the deny. #[serde(default, skip_serializing_if = "Option::is_none")] pub policy_url: Option, + /// The environment variable that suppresses **this row**, named in this + /// row's own deny (CLOUD-437). + /// + /// Absent means the general hatch, [`crate::hook::BYPASS_ENV`]. That default + /// is not a hedge — it is what keeps the guarantee true as rows are added: + /// per-row with no fallback would leave the next row hatchless and silent, + /// which is the failure that produced this column. A row declaring one is + /// declaring that its bypass is a *separate decision* from every other row's. + /// + /// Why a per-row column rather than one global name: the bash guards this + /// surface inherited each had their own hatch, so an operator could suppress + /// `memory-guard` alone while `ready-guard` stayed live. One global name + /// silently widens the blast radius of every bypass — invisibly, because the + /// deny text cannot say what else it just switched off. + /// + /// It is an environment variable rather than a `batten.local.toml` key on + /// purpose. A hatch LOWERS policy, and house-style §8's override channel is + /// raise-only; an env var is a per-invocation decision, visible in the + /// command that took it and gone when the process exits, where a config key + /// would make the same lowering persistent and invisible. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub bypass_env: Option, /// The **inspection-only** command template a [`RuleKind::Command`] rule /// runs as its gate. Required by that kind, rejected by any other. /// @@ -1745,6 +1782,38 @@ pub struct Rule { /// naming one or two paths. #[serde(default, skip_serializing_if = "Vec::is_empty")] pub line_sources: Vec, + /// The Rust files this policy row reads as **call sites**, by literal path + /// (CLOUD-914). + /// + /// [`Rule::lines`]' sibling one tier up: `lines` answers *does this token + /// appear in this file*, and this answers *does it appear in command + /// position*. A row wanting the second must say so, because parsing is + /// strictly more expensive than splitting and nobody should pay it by + /// accident. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub invocations: Vec, + /// The same, glob-selected (CLOUD-914). + /// + /// Ships WITH its literal column rather than arriving a row later, which is + /// the asymmetry `line_sources` records having to fix: a gate over call + /// sites is a gate over many files by construction, so the enumerated + /// spelling would go stale the first time a module was added. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub invocation_sources: Vec, + /// The Rust files this policy row reads as a **`use` graph**, by literal path + /// (CLOUD-762). + /// + /// Separate from [`Rule::invocations`] because the two answer different + /// questions over the same parse — what a file CALLS versus what it REACHES — + /// and a row wanting one should not pay to project the other. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub uses: Vec, + /// The same, glob-selected (CLOUD-762). + /// + /// A layering rule decides over every module by construction, so the glob is + /// the spelling that matters and it ships with its literal column. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub use_sources: Vec, /// The git facts this policy row reads, **declared** (CLOUD-907). /// /// [`Rule::documents`]'s sibling for the family that is not a file, and the @@ -2826,6 +2895,22 @@ impl Rule { self.id ))); } + if self.scope == RuleScope::MediatedCall + && !(self.uses.is_empty() && self.use_sources.is_empty()) + { + return Err(UsageError::raise(format!( + "rule {}: `uses` reads a tree's module graph and a mediated call judges a command, not a tree", + self.id + ))); + } + if self.scope == RuleScope::MediatedCall + && !(self.invocations.is_empty() && self.invocation_sources.is_empty()) + { + return Err(UsageError::raise(format!( + "rule {}: `invocations` parses tree files and a mediated call judges a command, not a tree", + self.id + ))); + } if self.scope == RuleScope::MediatedCall && !self.lines.is_empty() { return Err(UsageError::raise(format!( "rule {}: `lines` is what a `scope = \"tree\"` row hands its bundle; \ @@ -2912,7 +2997,7 @@ impl Rule { /// about all of them makes that failure impossible, and /// [`tests::every_optional_rule_field_is_classified_by_every_kind`] fails if /// a column is added here without being placed. - fn columns(&self) -> [(&'static str, bool); 43] { + fn columns(&self) -> [(&'static str, bool); 48] { [ // In the census because it is now per-kind, which is what makes // "required by every kind but the judge" a fact the existing @@ -2942,6 +3027,7 @@ impl Rule { ("requires_key", self.requires_key.is_some()), ("reason", self.reason.is_some()), ("policy_url", self.policy_url.is_some()), + ("bypass_env", self.bypass_env.is_some()), ("verbatim", self.verbatim.is_some()), ("identity_key", self.identity_key.is_some()), ("direction", self.direction.is_some()), @@ -2968,6 +3054,10 @@ impl Rule { // rather than per-kind capabilities, so they follow their siblings // rather than inventing a fifth classification for one of them. ("line_sources", !self.line_sources.is_empty()), + ("invocations", !self.invocations.is_empty()), + ("invocation_sources", !self.invocation_sources.is_empty()), + ("uses", !self.uses.is_empty()), + ("use_sources", !self.use_sources.is_empty()), ] } @@ -4274,7 +4364,7 @@ struct RunInputs<'a> { provisions: &'a [crate::provision::Provision], files: &'a [String], derived: &'a BTreeMap>, - documents: &'a BTreeMap, + documents: &'a BTreeMap<(String, Wanted), Acquired>, /// What earlier runs produced, for the keys this rule set declares /// (CLOUD-851). One read for the whole run, beside `documents`. produced: &'a BTreeMap, @@ -4468,6 +4558,36 @@ pub(crate) enum Want { Parsed(crate::facts::Format), /// Split into lines, unparsed (CLOUD-846). Lines, + /// Parsed as Rust, and reduced to its call sites (CLOUD-914). + Invocations, + /// Parsed as Rust, and reduced to its `use` edges (CLOUD-762). + Uses, +} + +/// WHICH FORM a path was acquired as — the cache's key alongside the path. +/// +/// **This exists because keying the cache on the path alone is a defect, and it +/// shipped twice before anything could see it.** One path holds one answer, so +/// two rows declaring the same file differently — one `line_sources`, one +/// `invocation_sources` over the same glob — had to be resolved by a precedence +/// rule, and precedence serves one row by STARVING the other: the loser's loop +/// finds the wrong variant and reports every one of its own declared files as +/// could-not-look. Measured at 65 paths, as `policy test`'s `fixture-missing`, +/// the first time two rows in this repository wanted one glob two ways. +/// +/// Payload-free deliberately, where [`Want`] carries a [`crate::facts::Format`]: +/// the format is recoverable from the path, and keeping it out of the key means +/// no public type has to grow an ordering to be a `BTreeMap` key. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) enum Wanted { + /// Parsed into the canonical tree. + Document, + /// Split into lines, unparsed. + Lines, + /// Parsed as Rust, reduced to call sites. + Invocations, + /// Parsed as Rust, reduced to `use` edges. + Uses, } /// A declared file, acquired — or the stated reason it was not. @@ -4477,6 +4597,14 @@ pub(crate) enum Acquired { Parsed(crate::facts::Node), /// The file's lines, in order, with line endings removed. Lines(Vec), + /// The file's call sites, parsed (CLOUD-914). An EMPTY vector is a file that + /// parsed and calls nothing — a real answer. A file the parser refused is + /// [`NotAcquired::Unparsed`], never this. + Invocations(Vec), + /// The file's `use` edges and its own export table (CLOUD-762). Both halves, + /// because resolution is a post-pass across the declared set and the crate + /// root's table is what it reads. + Uses(crate::uses::UseFile), /// Not acquired, and why. No(NotAcquired), } @@ -4555,9 +4683,26 @@ pub(crate) fn acquire(root: &Path, rel_path: &str, want: Option) -> Acquir // numbers are the ones an editor shows and a Windows checkout does not // change the answer. Want::Lines => Acquired::Lines(text.lines().map(ToOwned::to_owned).collect()), + // The other `want` a parser can refuse after a successful read, and it + // is spelled `Unparsed` rather than `Unreadable` for the same reason: + // the bytes were fine and the grammar was not. CLOUD-310 made this the + // price of embedding a matcher, after measuring one that emitted zero + // nodes and zero errors over a file it had only partly parsed. + // Same contract as `Invocations` below: the bytes read and the grammar + // refused, which is `Unparsed` rather than `Unreadable`. + Want::Uses => match crate::uses::use_facts(&text) { + crate::facts::Look::Is(facts) => Acquired::Uses(facts), + crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => { + Acquired::No(NotAcquired::Unparsed) + } + }, + Want::Invocations => match crate::invocation::invocations(&text) { + crate::facts::Look::Is(sites) => Acquired::Invocations(sites), + crate::facts::Look::IsNot | crate::facts::Look::CouldNotLook => { + Acquired::No(NotAcquired::Unparsed) + } + }, // THE ONE `Format::read` CALL IN THE CRATE. - // `one_document_acquisition_exists` asserts that by counting this - // needle, so a second parse pair cannot land without the gate going red. Want::Parsed(format) => match format.read(&text) { crate::facts::Look::Is(node) => Acquired::Parsed(node), // A file that will not parse says nothing about what it contains, @@ -4604,11 +4749,17 @@ const READ_BUDGET: usize = 10_000; /// there, which is the ceiling this fact lifts: `Format::for_path` returning /// `None` was the whole reason 12 of the 20 tree-scoped gates had no fact to /// decide over. -pub(crate) fn want_for(path: &str, as_lines: bool) -> Option { - if as_lines { - return Some(Want::Lines); +/// **THERE IS NO PRECEDENCE HERE ANY MORE, AND ITS ABSENCE IS THE FIX.** The +/// earlier signature took a flag per form and returned the winner, which is only +/// a question worth asking when one path can hold one answer. Now the caller +/// acquires each form a row asked for, so this answers exactly the form named. +pub(crate) fn want_for(path: &str, wanted: Wanted) -> Option { + match wanted { + Wanted::Lines => Some(Want::Lines), + Wanted::Invocations => Some(Want::Invocations), + Wanted::Uses => Some(Want::Uses), + Wanted::Document => crate::facts::Format::for_path(path).map(Want::Parsed), } - crate::facts::Format::for_path(path).map(Want::Parsed) } /// Refuse a run whose declared read set has passed `limit` (CLOUD-850). @@ -4671,6 +4822,31 @@ pub(crate) fn declared_lines(rule: &Rule, files: &[String]) -> anyhow::Result anyhow::Result> { + select_declared(&rule.invocations, &rule.invocation_sources, files) +} + +/// Every path a row hands its bundle as a **`use` graph** (CLOUD-762). +/// +/// # Errors +/// +/// A [`UsageError`] (→ exit `1`) for a pattern `globset` cannot parse, matching +/// its siblings rather than selecting nothing. +pub(crate) fn declared_uses(rule: &Rule, files: &[String]) -> anyhow::Result> { + select_declared(&rule.uses, &rule.use_sources, files) +} + /// The literal-plus-glob union both declaration pairs resolve through. /// /// One implementation rather than two, so `lines` cannot drift from `documents` @@ -4716,22 +4892,14 @@ pub(crate) fn acquire_declared( rules: &[Rule], root: &Path, files: &[String], -) -> anyhow::Result> { - let mut cache: BTreeMap = BTreeMap::new(); - // Which declared paths any row wants as LINES (CLOUD-846). Collected across - // the whole rule set first, because the cache is shared: one path asked for - // both ways would otherwise be acquired as whichever row was seen first, - // which is an answer that depends on rule order. - // - // Resolved through `declared_lines` rather than read raw, so a glob spelling - // reaches this set too (CLOUD-864). A row whose pattern will not parse - // contributes nothing here and is reported on the per-rule path below, which - // is the same treatment `declared_documents` gets three lines down. - let lines_paths: BTreeSet = rules - .iter() - .filter(|rule| rule.kind == RuleKind::Policy && rule.scope == RuleScope::Tree) - .flat_map(|rule| declared_lines(rule, files).unwrap_or_default()) - .collect(); +) -> anyhow::Result> { + let mut cache: BTreeMap<(String, Wanted), Acquired> = BTreeMap::new(); + // THE THREE COLLECT-FIRST SETS ARE GONE, and their absence is the fix. They + // existed so a path two rows wanted differently would not resolve by + // whichever row the loop reached first — a hazard that only exists while one + // path can hold one answer. Keyed by `(path, form)` there is nothing to + // resolve: each row's declaration is acquired in the form it asked for, and + // rule order cannot change any answer. for rule in rules .iter() .filter(|rule| rule.kind == RuleKind::Policy && rule.scope == RuleScope::Tree) @@ -4743,23 +4911,45 @@ pub(crate) fn acquire_declared( let Ok(declared) = declared_documents(rule, files) else { continue; }; - let declared_line_paths = declared_lines(rule, files).unwrap_or_default(); - for path in declared.into_iter().chain(declared_line_paths) { - if cache.contains_key(&path) { - // THE CACHE, and the assertion `documents_acquired` makes: N - // rows over one path is ONE read and ONE parse. - continue; + // ONE PASS PER FORM THE ROW ASKED FOR, never one pass picking a winner. + // A path a row wants as lines and another row wants as call sites is two + // entries, and both rows get their own fact — which is the whole of the + // defect this shape fixes. + let wanted: [(Wanted, Vec); 4] = [ + (Wanted::Document, declared), + ( + Wanted::Lines, + declared_lines(rule, files).unwrap_or_default(), + ), + ( + Wanted::Invocations, + declared_invocations(rule, files).unwrap_or_default(), + ), + (Wanted::Uses, declared_uses(rule, files).unwrap_or_default()), + ]; + for (form, paths) in wanted { + for path in paths { + let key = (path, form); + if cache.contains_key(&key) { + // THE CACHE, and the assertion `documents_acquired` makes: N + // rows over one path IN ONE FORM is one read and one parse. + // Two rows wanting two forms is two, which is honest — they + // are two different parses of the same bytes. + continue; + } + // THE BUDGET, checked before the read rather than after it, so + // the refusal is a bound and not a report of one already + // exceeded. It now counts (path, form) pairs: a file wanted + // three ways costs three, because it IS three parses. + // + // POINTER-ONLY, and here that means less than a pointer (§5): a + // count and a limit, never the path list. The list is exactly + // the consumer's file names, and a refusal that printed it would + // put the shape of a private tree into an error message. + refuse_over_budget(cache.len(), READ_BUDGET)?; + let acquired = acquire(root, &key.0, want_for(&key.0, form)); + cache.insert(key, acquired); } - // THE BUDGET, checked before the read rather than after it, so the - // refusal is a bound and not a report of one already exceeded. - // - // POINTER-ONLY, and here that means less than a pointer (§5): a - // count and a limit, never the path list. The list is exactly the - // consumer's file names, and a refusal that printed it would put the - // shape of a private tree into an error message. - refuse_over_budget(cache.len(), READ_BUDGET)?; - let acquired = acquire(root, &path, want_for(&path, lines_paths.contains(&path))); - cache.insert(path, acquired); } } Ok(cache) @@ -4862,10 +5052,124 @@ fn git_facts(rules: &[Rule], root: &Path) -> crate::git::GitFacts { } } +/// One path's projection for one fact family: the value, or why it could not be +/// looked at. +/// +/// Extracted from [`tree_document`], where the identical shape was written four +/// times — once per family — and each copy had to remember that a path acquired +/// as a DIFFERENT projection is `Absent` rather than an empty answer. That is +/// the vacuous pass CLOUD-251 names, and four chances to get it wrong is three +/// too many. +/// +/// The `wanted` and the [`Acquired`] variant must agree, and the mismatch arm is +/// what enforces it: a path in the cache under another projection is not this +/// family's answer, and reporting it as an empty one would be a module finding +/// nothing and reporting clean. +fn project_declared( + cache: &BTreeMap<(String, Wanted), Acquired>, + path: &str, + wanted: Wanted, +) -> Result { + match cache.get(&(path.to_owned(), wanted)) { + Some(Acquired::Parsed(node)) if wanted == Wanted::Document => Ok(node.to_json()), + Some(Acquired::Lines(text)) if wanted == Wanted::Lines => Ok(serde_json::json!(text)), + // COULD NOT LOOK, NEVER AN EMPTY ARRAY, and for call sites the parser is + // the one that can say so: `NotAcquired::Unparsed` is a file whose bytes + // read fine and whose grammar did not. CLOUD-310 measured a matcher + // emitting zero nodes and zero errors over a partially-parsed file, and + // made a parse-coverage assertion the price of embedding one. + Some(Acquired::Invocations(sites)) if wanted == Wanted::Invocations => { + Ok(serde_json::json!(sites)) + } + // The `use` family is handled by its caller rather than here, because an + // edge set is completed against the crate root's re-export table and that + // table is one value for the whole declared set — a per-path projection + // cannot see it. + Some(Acquired::No(why)) => Err(*why), + // Acquired under a different projection, or never declared. Neither is + // this family's answer and neither is an empty one. + _ => Err(NotAcquired::Absent), + } +} + +/// The paths each row DECLARED, one list per projection (CLOUD-914, CLOUD-762). +/// +/// Four lists rather than one, for the reason the cache is keyed on a pair: a +/// path can be acquired as more than one fact, so which projection it belongs to +/// is the caller's declaration and never a guess from the extension. +/// +/// Grouped into a struct rather than passed as four parameters because they are +/// one thing — the declared set — and splitting them across the signature is +/// what pushed this function past the argument ceiling when the third and fourth +/// arrived. +pub(crate) struct Declared<'a> { + /// Files a row declared as parsed documents. + pub documents: &'a [String], + /// Files a row declared as line sets. + pub lines: &'a [String], + /// Rust files a row declared as call sites. + pub invocations: &'a [String], + /// Rust files a row declared as a `use` graph. + pub uses: &'a [String], +} + +/// Project the `use` family, resolving every edge against ONE re-export table. +/// +/// Separate from [`project_declared`] rather than folded into it, and the reason +/// is the whole of CLOUD-762's finding: an edge naming a crate-root item cannot +/// be completed from the file it appears in. The root's own `mod` declarations +/// are what disambiguate `use error::X` (a module) from `use anyhow::X` (a +/// crate), so the table is one value for the entire declared set and a per-path +/// projection structurally cannot see it. +fn project_uses( + cache: &BTreeMap<(String, Wanted), Acquired>, + uses: &[String], + out: &mut serde_json::Map, + missing: &mut Vec, + causes: &mut Vec<(String, NotAcquired)>, +) { + // The root is named by Rust's own convention — a library crate's root is + // `lib.rs` — which is a language fact and not a consumer identifier, so + // non-negotiable rule 1 is untouched: nothing here names a repository, an + // account or an entity path. A declared set with no root simply resolves + // nothing, and every crate-root edge stays `root-item` with an empty + // destination. That is the honest failure direction: visibly unresolved + // rather than plausibly wrong. + let root_table = uses + .iter() + .find(|path| { + std::path::Path::new(path.as_str()).file_name() == Some(std::ffi::OsStr::new("lib.rs")) + }) + .and_then(|path| match cache.get(&(path.clone(), Wanted::Uses)) { + Some(Acquired::Uses(facts)) => Some(facts.exports.clone()), + _ => None, + }) + .unwrap_or_default(); + for path in uses { + match cache.get(&(path.clone(), Wanted::Uses)) { + Some(Acquired::Uses(facts)) => { + let mut edges = facts.edges.clone(); + crate::uses::resolve(&mut edges, &root_table); + out.insert(path.clone(), serde_json::json!(edges)); + } + // COULD NOT LOOK, NEVER AN EMPTY EDGE SET. A layering gate whose + // corpus failed to parse would otherwise report clean, which is + // CLOUD-251's vacuous pass arriving as a green board. + Some(Acquired::No(why)) => { + missing.push(path.clone()); + causes.push((path.clone(), *why)); + } + Some(Acquired::Parsed(_) | Acquired::Lines(_) | Acquired::Invocations(_)) | None => { + missing.push(path.clone()); + causes.push((path.clone(), NotAcquired::Absent)); + } + } + } +} + pub(crate) fn tree_document( - cache: &BTreeMap, - documents: &[String], - lines: &[String], + cache: &BTreeMap<(String, Wanted), Acquired>, + declared: &Declared<'_>, tracked: &[String], // What earlier runs produced, acquired once at the boundary (CLOUD-851). // Handed in rather than read here for `Fact::Produced`'s whole reason: the @@ -4876,64 +5180,54 @@ pub(crate) fn tree_document( // projection is pure, and the reads that fill this are the caller's. git: &crate::git::GitFacts, ) -> (String, Vec<(String, NotAcquired)>) { + let Declared { + documents, + lines, + invocations, + uses, + } = *declared; let mut produced_records: serde_json::Map = produced .iter() .map(|(key, record)| (key.clone(), serde_json::json!(record))) .collect(); let mut parsed = serde_json::Map::new(); let mut read_lines = serde_json::Map::new(); + let mut call_sites = serde_json::Map::new(); + let mut use_edges = serde_json::Map::new(); let mut missing = Vec::new(); // The same set as `missing`, carrying WHY (CLOUD-845). `missing` stays a // bare path list in the document because that is what a module reads; the // cause is the caller's, so a skip can name its reason instead of being // anonymous. let mut causes: Vec<(String, NotAcquired)> = Vec::new(); - for path in documents { - // READ FROM THE SHARED CACHE (CLOUD-850), which `acquire_declared` - // filled once for the whole run through the one acquisition - // (CLOUD-849). A path absent from the cache is a caller that did not - // declare it, which is could-not-look rather than a silent empty. - match cache.get(path) { - Some(Acquired::Parsed(node)) => { - parsed.insert(path.clone(), node.to_json()); - } - // Still `missing` — a module reads it as could-not-look exactly as - // before, so no consumer's predicate changes shape. What is new is - // that the CAUSE survives for the caller, which is what CLOUD-845's - // exit-1 refusal attaches to. - Some(Acquired::No(why)) => { - missing.push(path.clone()); - causes.push((path.clone(), *why)); - } - // Acquired as LINES by another row's declaration, or not declared. - // Neither is a parsed document, and neither is an empty one. - Some(Acquired::Lines(_)) | None => { - missing.push(path.clone()); - causes.push((path.clone(), NotAcquired::Absent)); - } - } - } - for path in lines { - match cache.get(path) { - Some(Acquired::Lines(text)) => { - read_lines.insert(path.clone(), serde_json::json!(text)); - } - // COULD NOT LOOK, NEVER AN EMPTY ARRAY (CLOUD-846 §2). A file - // nobody could read and a file with no matching line are different - // answers, and collapsing them is CLOUD-251's vacuous pass: the - // predicate would find nothing and report clean. - Some(Acquired::No(why)) => { - missing.push(path.clone()); - causes.push((path.clone(), *why)); - } - // A path acquired as something else, or not declared. Neither is a - // readable line set, and neither is an empty one. - Some(Acquired::Parsed(_)) | None => { - missing.push(path.clone()); - causes.push((path.clone(), NotAcquired::Absent)); + // ONE LOOP OVER THREE FAMILIES, driven by a table rather than written out + // three times. Each family names the projection it wants and the map it + // fills; `project_declared` owns the could-not-look distinction that all + // three share, so there is one place for it to be right rather than three. + // + // `uses` is deliberately NOT in this table: its edges are completed against + // the crate root's re-export table below, which is one value for the whole + // declared set and so cannot be resolved per path. + for (paths, wanted, out) in [ + (documents, Wanted::Document, &mut parsed), + (lines, Wanted::Lines, &mut read_lines), + (invocations, Wanted::Invocations, &mut call_sites), + ] { + for path in paths { + match project_declared(cache, path, wanted) { + Ok(value) => { + out.insert(path.clone(), value); + } + Err(why) => { + missing.push(path.clone()); + causes.push((path.clone(), why)); + } } } } + // The `use` family, whose edges are completed against one table for the + // whole declared set — see `project_uses`. + project_uses(cache, uses, &mut use_edges, &mut missing, &mut causes); // THE PROJECTION (CLOUD-845), on `hook::call_document`'s shape. Iterating // `Fact::ALL` rather than writing keys means a fact the tree surface gains // cannot arrive unprojected, and a key the tree emits cannot fail to name a @@ -4962,6 +5256,16 @@ pub(crate) fn tree_document( crate::facts::Fact::Document => serde_json::Value::Object(std::mem::take(&mut parsed)), crate::facts::Fact::Tracked => serde_json::json!(tracked), crate::facts::Fact::Lines => serde_json::Value::Object(std::mem::take(&mut read_lines)), + // A path the parser refused is in `missing` rather than here, + // carrying `unparsed` as its cause — so a module reads could-not-look + // exactly as it does for a document, and a path present with an + // empty array is a file that parsed and calls nothing. Those are the + // two answers CLOUD-310's parse-coverage obligation demands stay + // apart, and the projection is where they would have collapsed. + crate::facts::Fact::Invocations => { + serde_json::Value::Object(std::mem::take(&mut call_sites)) + } + crate::facts::Fact::Uses => serde_json::Value::Object(std::mem::take(&mut use_edges)), // Hook-surface facts, filtered above. Stated as an arm so a // reclassification has to come through here. crate::facts::Fact::Produced => { @@ -5021,7 +5325,7 @@ fn policy_rule( // branch here and no could-not-look arm for one. tracked: &[String], // The run's one document cache (CLOUD-850). - documents: &BTreeMap, + documents: &BTreeMap<(String, Wanted), Acquired>, // The run's one read of the sink store (CLOUD-851). produced: &BTreeMap, // The run's one acquisition of the declared git facts (CLOUD-907). @@ -5041,21 +5345,44 @@ fn policy_rule( let Ok(declared) = declared_documents(rule, tracked) else { return Some(NotObserved::RuleSkipped); }; - // A row that declared a selector and matched nothing has not established - // anything, and saying so is the point: a selector selecting nothing and a - // tree satisfying the predicate are otherwise the same green. - if declared.is_empty() && !rule.sources.is_empty() { - return Some(NotObserved::RuleSkipped); - } // `ok()?` rather than `?`: this function returns `Option`, and a // malformed glob is refused by `validate` before any rule evaluates — so a // failure here is a row this surface does not own, treated the same way // `declared_documents` is treated a few lines up. let declared_line_paths = declared_lines(rule, tracked).ok()?; + let declared_invocation_paths = declared_invocations(rule, tracked).ok()?; + let declared_use_paths = declared_uses(rule, tracked).ok()?; + // A row that declared a selector and matched nothing has not established + // anything, and saying so is the point: a selector selecting nothing and a + // tree satisfying the predicate are otherwise the same green. + // + // EVERY GLOB COLUMN, not just `sources`, and the omission was live + // (CLOUD-359). This tested `rule.sources` alone while three more glob + // spellings had landed beside it — `line_sources` (CLOUD-864), + // `invocation_sources` and `use_sources`. A row selecting only through one + // of those, in a tree carrying no such file, ran its module against an empty + // document instead of being skipped: the module then decides over nothing + // and whatever it says is a verdict about a tree it never read. Measured + // here on a fixture repo with no Rust in it. + let selectors_declared = !rule.sources.is_empty() + || !rule.line_sources.is_empty() + || !rule.invocation_sources.is_empty() + || !rule.use_sources.is_empty(); + let selected_nothing = declared.is_empty() + && declared_line_paths.is_empty() + && declared_invocation_paths.is_empty() + && declared_use_paths.is_empty(); + if selected_nothing && selectors_declared { + return Some(NotObserved::RuleSkipped); + } let (input, not_acquired) = tree_document( documents, - &declared, - &declared_line_paths, + &Declared { + documents: &declared, + lines: &declared_line_paths, + invocations: &declared_invocation_paths, + uses: &declared_use_paths, + }, tracked, produced, git, @@ -6435,11 +6762,16 @@ fn document_in_file( // cause or a new `Acquired` shape has to be decided here rather than // defaulting to silence. Two of the arms report the same reason today for // two unrelated causes, and merging them would delete the comments that say - // why each is here — so the lint is expected rather than obeyed. - #[expect( - clippy::match_same_arms, - reason = "an arm per outcome is the property; the shared reason is a coincidence of two distinct causes" - )] + // why each is here. + // + // The `#[expect(clippy::match_same_arms)]` that used to sit here is GONE + // rather than kept, and its going is the annotation working. CLOUD-914 and + // CLOUD-762 added `Invocations` and `Uses` to the acquisition outcomes, which + // regrouped this match until the lint stopped firing — and because it was + // `expect` rather than `allow`, the now-unfulfilled expectation went red + // instead of sitting here forever as a claim about a lint nobody triggers. + // If a later edit makes two arms identical again, clippy says so, which is + // the answer this attribute was standing in for. let reason = match &outcome { // The tree does not carry it. Not this row's business and not a // finding — unchanged. @@ -6451,10 +6783,12 @@ fn document_in_file( Acquired::No( NotAcquired::Unparsed | NotAcquired::Unreadable | NotAcquired::UnknownFormat, ) => Some(DOCUMENT_UNREADABLE), - // Unreachable: this site always asks for `Want::Parsed`. An arm rather - // than a wildcard so a caller that ever asks for lines here has to - // decide what a node path means over them. - Acquired::Lines(_) => Some(DOCUMENT_UNREADABLE), + // Unreachable: this site always asks for `Want::Parsed`. Arms rather + // than a wildcard so a caller that ever asks for lines or call sites + // here has to decide what a node path means over them. + Acquired::Lines(_) | Acquired::Invocations(_) | Acquired::Uses(_) => { + Some(DOCUMENT_UNREADABLE) + } Acquired::Parsed(document) => match document.at(node_path) { crate::facts::Look::IsNot => Some(DOCUMENT_NODE_ABSENT), crate::facts::Look::CouldNotLook => Some(DOCUMENT_UNREADABLE), @@ -6600,7 +6934,9 @@ fn derive_one(rule: &Rule, root: &Path, files: &[String]) -> crate::facts::Look< // Unreachable: this site always asks for `Want::Parsed`. An arm rather // than a wildcard, so a caller that ever asks for lines here has to // decide what a node path means over them. - Acquired::No(_) | Acquired::Lines(_) => Look::CouldNotLook, + Acquired::No(_) | Acquired::Lines(_) | Acquired::Invocations(_) | Acquired::Uses(_) => { + Look::CouldNotLook + } Acquired::Parsed(document) => match document.at(node_path) { Look::Is(node) => match node.scalar() { Some(value) => Look::Is(value), @@ -7281,7 +7617,8 @@ mod tests { // is that the cause is the declaration, not the filesystem. std::fs::write(dir.join("prose.md"), "# heading\n").unwrap(); - let acquire = |name: &str| super::acquire(&dir, name, super::want_for(name, false)); + let acquire = + |name: &str| super::acquire(&dir, name, super::want_for(name, super::Wanted::Document)); assert!( matches!(acquire("good.toml"), super::Acquired::Parsed(_)), @@ -7338,11 +7675,15 @@ mod tests { // --- the tree document corresponds to the fact model (CLOUD-845) ------- /// The keys `tree_document` actually emits, read off a real build of it. fn emitted_tree_keys(_root: &std::path::Path) -> Vec { - let empty: BTreeMap = BTreeMap::new(); + let empty: BTreeMap<(String, super::Wanted), super::Acquired> = BTreeMap::new(); let (input, _) = super::tree_document( &empty, - &[], - &[], + &super::Declared { + documents: &[], + lines: &[], + invocations: &[], + uses: &[], + }, &[], &BTreeMap::new(), &crate::git::GitFacts::default(), @@ -7809,6 +8150,81 @@ mod tests { /// /// Keeps the fixtures below from re-listing six `None`s each, so adding a /// column touches this one place rather than every test. + /// TWO ROWS, ONE GLOB, TWO FORMS — each gets its own fact. + /// + /// The regression for the defect the `(path, form)` key exists to fix, and + /// the case the single-key cache never had. Keyed on the path alone, one + /// file holds one answer, so a precedence rule decided which of two rows was + /// served and the loser's projection reported every one of its own declared + /// files as could-not-look. It shipped twice — through CLOUD-914 and + /// CLOUD-762 — because no rule set until CLOUD-756 wanted one file two ways. + /// Measured when one finally did: 65 paths, as `policy test`'s + /// `fixture-missing`. + /// + /// Fails by: keying the cache on the path alone again. Whichever form + /// `acquire_declared` reaches second finds the other's entry and this goes + /// red on the row that lost. + #[test] + fn two_rows_over_one_glob_each_get_their_own_form() { + let dir = std::env::temp_dir().join(format!("batten-two-forms-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("subject.rs"), "fn f() { g(\"x\"); }\n").unwrap(); + + let as_lines = Rule { + scope: RuleScope::Tree, + line_sources: vec!["*.rs".to_owned()], + ..blank("wants-lines", RuleKind::Policy) + }; + let as_calls = Rule { + scope: RuleScope::Tree, + invocation_sources: vec!["*.rs".to_owned()], + ..blank("wants-calls", RuleKind::Policy) + }; + let files = vec!["subject.rs".to_owned()]; + let cache = super::acquire_declared(&[as_lines.clone(), as_calls.clone()], &dir, &files) + .expect("acquisition succeeds"); + + // BOTH entries exist, under the same path. That is the whole property: + // one file, two parses, neither displacing the other. + assert!( + matches!( + cache.get(&("subject.rs".to_owned(), super::Wanted::Lines)), + Some(super::Acquired::Lines(_)) + ), + "the lines row must get lines" + ); + assert!( + matches!( + cache.get(&("subject.rs".to_owned(), super::Wanted::Invocations)), + Some(super::Acquired::Invocations(_)) + ), + "the call-sites row must get call sites" + ); + + // AND THE PROJECTION AGREES, which is where the defect actually bit: the + // cache could have held both and the document still reported one row's + // files missing if a lookup asked for the wrong form. + let (_, not_acquired) = super::tree_document( + &cache, + &super::Declared { + documents: &[], + lines: &["subject.rs".to_owned()], + invocations: &["subject.rs".to_owned()], + uses: &[], + }, + &files, + &BTreeMap::new(), + &crate::git::GitFacts::default(), + ); + assert!( + not_acquired.is_empty(), + "neither row may report its own declared file as could-not-look: {not_acquired:?}" + ); + + let _ = std::fs::remove_dir_all(&dir); + } + fn blank(id: &str, kind: RuleKind) -> Rule { Rule { id: id.to_owned(), @@ -7840,6 +8256,7 @@ mod tests { requires_key: None, reason: None, policy_url: None, + bypass_env: None, check: None, fix: None, produces: None, @@ -7862,6 +8279,10 @@ mod tests { sources: Vec::new(), lines: Vec::new(), line_sources: Vec::new(), + invocations: Vec::new(), + invocation_sources: Vec::new(), + uses: Vec::new(), + use_sources: Vec::new(), git: Vec::new(), refs: Vec::new(), ranges: Vec::new(), diff --git a/crates/batten/src/uses.rs b/crates/batten/src/uses.rs new file mode 100644 index 000000000..87dddbe87 --- /dev/null +++ b/crates/batten/src/uses.rs @@ -0,0 +1,403 @@ +//! The `use` graph as a fact: which module reaches which (CLOUD-762). +//! +//! # What a line predicate gets wrong, measured rather than assumed +//! +//! CLOUD-762's §2 makes the measurement deliverable one, because the tier this +//! fact belongs in is decided by a count and not by an argument. Over +//! `crates/batten/src/**`, the syntactic tier is wrong at **four sites, in two +//! classes**, and both are re-exports — aliases and glob imports contribute +//! nothing at all: +//! +//! * **A hidden internal edge.** `trust.rs` and `output.rs` write +//! `use crate::UsageError`. The text names no module; the edge is really onto +//! `crate::error`, because the root re-exports that name. A layering gate +//! reading lines is silently GREEN on an edge it was built to judge. +//! * **A phantom internal edge.** `policy.rs` and `sink.rs` write +//! `use crate::Result`. That reads as an internal edge, and the root's own +//! private `use anyhow::Result` means it is really an EXTERNAL dependency. +//! The same gate is silently confused in the opposite direction. +//! +//! Aliases are all `as _` trait imports or paths into other crates, and the one +//! crate-internal alias leaves its module path plainly visible. Every glob is +//! `use super::*` inside a `#[cfg(test)]` module, which crosses no module +//! boundary. Neither can move a top-level module edge in this tree. +//! +//! # Why this is `Read x Check` and needs no delegated analyser +//! +//! Four is bounded and nameable, which is the arm CLOUD-762's reversal condition +//! sends to `Read x Check`. But the stronger reason is one that condition did not +//! anticipate: **the re-export table is itself syntax.** Resolving all four sites +//! needs no name resolution, no rust-analyzer and no `Cost::Effect` — it needs +//! the crate root's own `use` and `pub use` items, which a parser reads exactly +//! as it reads any other statement. So the fact is cheap AND correct about the +//! cases a line predicate gets wrong, rather than trading one for the other. +//! +//! # Resolution is a post-pass over the DECLARED set, never a hardcoded path +//! +//! [`resolve`] applies one file's export table to another's edges, and the +//! caller decides which file is the root by Rust's own convention — a library +//! crate's root is `lib.rs`. That is a language fact, not a consumer identifier, +//! so non-negotiable rule 1 is untouched: nothing here names a repository, an +//! account or an entity path, and a consumer whose root is elsewhere simply +//! declares it. +//! +//! # Pointer-only +//! +//! An edge is two module names and a line (rule 4). The imported ITEM name is +//! carried because resolution needs it, and it is a path segment rather than +//! content — never the source line, and never the file's text. + +use std::collections::BTreeMap; + +use crate::facts::Look; + +/// Where a `use` statement's first segment points, before resolution. +#[derive(Clone, Copy, Debug, PartialEq, Eq, serde::Serialize)] +#[serde(rename_all = "kebab-case")] +pub enum Origin { + /// `use crate::::…` — the module is named in the text, and a line + /// predicate reads it correctly. + Internal, + /// `use ::…` — outside this crate entirely. + External, + /// `use crate::` — a name reaching through the crate root's own + /// re-export table. **The module is NOT in the text**, which is the whole + /// class this fact exists to resolve. + RootItem, + /// `use self::…` / `use super::…` — inside the file's own module tree, so it + /// crosses no top-level boundary. Kept as its own answer rather than dropped: + /// "there is no edge here" and "I did not look" must stay distinct. + Local, +} + +/// One `use` edge: what it reaches, and where it was written. +#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize)] +// Kebab-case so the emitted key matches `Fact::Uses`' schema fragment. The two +// disagreeing is the drift CLOUD-845 measured, where a documented key was never +// emitted and nothing could tell. +#[serde(rename_all = "kebab-case")] +#[non_exhaustive] +pub struct UseEdge { + /// The module or crate reached, once resolved. Empty exactly while + /// [`Origin::RootItem`] is unresolved — which [`resolve`] fills in, and which + /// a consumer must never read as "reaches nothing". + pub to: String, + /// The imported item's leaf name. What resolution looks up. + pub item: String, + /// Which class the first segment put this edge in. + pub origin: Origin, + /// Whether [`resolve`] is what supplied `to`, rather than the text. + /// + /// **This is the flag the measurement counts, and it exists because the + /// obvious discriminator is wrong.** An edge written `use crate::error::X` + /// and one written `use crate::UsageError` both end as [`Origin::Internal`] + /// onto `error`, and telling them apart by the item's case counts all 88 + /// ordinary edges in this crate as divergences — measured, on the first run + /// of the case below. Only the edges resolution CHANGED are the ones a line + /// predicate gets wrong, and only this field knows which those are. + pub via_root: bool, + /// 1-indexed line of the `use` statement. + pub line: usize, +} + +/// A file's crate-root export table: item name -> the module it really lives in, +/// or `None` where the root imported it from another crate. +/// +/// Built from the root's own statements. `pub use` and a private `use` are both +/// read, and the difference is the whole point: a `pub use error::UsageError` +/// makes `crate::UsageError` an INTERNAL edge onto `error`, while a private +/// `use anyhow::Result` makes `crate::Result` an EXTERNAL one. A table that +/// recorded only the public half would resolve the first class and leave the +/// second reading as an edge to nothing. +pub type RootExports = BTreeMap>; + +/// The crate root's export table, or the reason there is none. +/// +/// [`Look::CouldNotLook`] when the text is not parseable Rust — never an empty +/// table, which would resolve nothing while looking exactly like a root that +/// re-exports nothing. +#[must_use] +pub fn root_exports(source: &str) -> Look { + let Ok(file) = syn::parse_file(source) else { + return Look::CouldNotLook; + }; + // THE ROOT'S OWN `mod` DECLARATIONS, COLLECTED FIRST, and they are what makes + // the rest correct. At a crate root `use error::UsageError` and + // `use anyhow::Result` are the SAME SHAPE — a bare first segment is either a + // top-level module or another crate, and nothing in the statement says which. + // The root declares its modules, so the answer is in the same file, and this + // stays pure syntax rather than becoming name resolution. + let modules: std::collections::BTreeSet = file + .items + .iter() + .filter_map(|item| match item { + syn::Item::Mod(module) => Some(module.ident.to_string()), + _ => None, + }) + .collect(); + // EVERY DECLARED MODULE IS ITS OWN ENTRY, and leaving them out is what the + // tree corrected: `use crate::{config, rules}` imports MODULES directly, and + // a table holding only re-exported items leaves 39 such edges in this crate + // unresolved — measured, on the second run of `use_graph`. A module maps to + // itself, which is what makes `resolve` able to say the text already named + // the destination. + let mut table: RootExports = modules + .iter() + .map(|name| (name.clone(), Some(name.clone()))) + .collect(); + for item in &file.items { + let syn::Item::Use(item_use) = item else { + continue; + }; + collect_root_tree(&item_use.tree, None, &modules, &mut table); + } + Look::Is(table) +} + +/// Walk one `use` tree in the crate root, recording what each leaf name resolves +/// to. +/// +/// `first` carries the path's first segment once it is known, which is what +/// decides internal-versus-external for every leaf underneath it. +fn collect_root_tree( + tree: &syn::UseTree, + first: Option<&str>, + modules: &std::collections::BTreeSet, + table: &mut RootExports, +) { + match tree { + syn::UseTree::Path(path) => { + let segment = path.ident.to_string(); + collect_root_tree(&path.tree, first.or(Some(&segment)), modules, table); + } + syn::UseTree::Group(group) => { + for item in &group.items { + collect_root_tree(item, first, modules, table); + } + } + syn::UseTree::Name(name) => { + insert_root_name(&name.ident.to_string(), first, modules, table); + } + // The ALIAS is the name other modules can reach, so it is the key. The + // original is unreachable through the root and recording it would invent + // an edge nobody can write. + syn::UseTree::Rename(rename) => { + insert_root_name(&rename.rename.to_string(), first, modules, table); + } + // A glob at the root re-exports names this pass cannot enumerate without + // resolving the target module. Deliberately not recorded: a guess here + // would be a wrong edge, and an absent entry leaves the consumer's edge + // unresolved and visibly so, which is the honest failure direction. + syn::UseTree::Glob(_) => {} + } +} + +/// Record one root-level leaf, keyed by the name a sibling module would write. +fn insert_root_name( + name: &str, + first: Option<&str>, + modules: &std::collections::BTreeSet, + table: &mut RootExports, +) { + let Some(first) = first else { + return; + }; + // `Some(module)` is an internal edge, `None` an external one — and the + // declared-module set is the only thing that can tell them apart here. + // A root that re-exports from its own submodule records the SUBMODULE, which + // is the answer a layering table is keyed on. + let module = if modules.contains(first) { + Some(first.to_owned()) + } else { + None + }; + table.insert(name.to_owned(), module); +} + +/// The visitor's accumulator, at module scope for the same reason +/// `invocation.rs`'s is: it borrows nothing from the function that drives it, and an item +/// declared after a statement reads as if it were scoped to what precedes it +/// when it is not (`clippy::items_after_statements`). +#[derive(Default)] +struct Edges { + found: Vec, +} + +impl<'ast> syn::visit::Visit<'ast> for Edges { + fn visit_item_use(&mut self, item: &'ast syn::ItemUse) { + let line = item.use_token.span.start().line; + collect_edges(&item.tree, None, line, &mut self.found); + syn::visit::visit_item_use(self, item); + } +} + +/// Every `use` edge in a Rust source text, or the reason there are none. +/// +/// [`Look::CouldNotLook`] when the text does not parse — **never an empty list**. +/// A file nobody could parse and a file that imports nothing are different +/// answers, and Rego reads an undefined path as "does not hold", so collapsing +/// them is a layering gate that is silently off (CLOUD-251). +/// +/// Edges whose [`Origin`] is [`Origin::RootItem`] arrive with an empty `to` and +/// are completed by [`resolve`]. Handing them back unresolved rather than +/// guessing is deliberate: a consumer that never resolves sees an obviously +/// blank destination instead of a plausible wrong one. +#[must_use] +pub fn use_edges(source: &str) -> Look> { + use syn::visit::Visit; + + let Ok(file) = syn::parse_file(source) else { + return Look::CouldNotLook; + }; + let mut edges = Edges::default(); + edges.visit_file(&file); + Look::Is(edges.found) +} + +/// Walk one `use` tree, emitting an edge per leaf. +/// +/// Per LEAF rather than per statement, because `use crate::{a::A, b::B}` is two +/// edges and reporting it as one would undercount exactly the grouped imports +/// this tree favours. +fn collect_edges(tree: &syn::UseTree, first: Option<&str>, line: usize, out: &mut Vec) { + match tree { + syn::UseTree::Path(path) => { + let segment = path.ident.to_string(); + match first { + // Already inside a path whose first segment is known. If that + // segment was `crate`, THIS one is the module — the ordinary + // internal edge, and the only place `to` is filled in directly. + Some("crate") => out.push(UseEdge { + to: segment.clone(), + item: leaf_name(&path.tree).unwrap_or_else(|| segment.clone()), + origin: Origin::Internal, + via_root: false, + line, + }), + Some(_) => collect_edges(&path.tree, first, line, out), + None => collect_edges(&path.tree, Some(&segment), line, out), + } + } + syn::UseTree::Group(group) => { + for item in &group.items { + collect_edges(item, first, line, out); + } + } + syn::UseTree::Name(name) => { + push_leaf(&name.ident.to_string(), first, line, out); + } + syn::UseTree::Rename(rename) => { + // Keyed by the ORIGINAL, because that is the name the root's table + // is keyed on; the local alias is this file's business alone. + push_leaf(&rename.ident.to_string(), first, line, out); + } + syn::UseTree::Glob(_) => { + if let Some(first) = first { + push_leaf("*", Some(first), line, out); + } + } + } +} + +/// The leaf name at the end of a `use` path, for the item a resolver looks up. +fn leaf_name(tree: &syn::UseTree) -> Option { + match tree { + syn::UseTree::Name(name) => Some(name.ident.to_string()), + syn::UseTree::Rename(rename) => Some(rename.ident.to_string()), + syn::UseTree::Path(path) => leaf_name(&path.tree), + // A group or a glob has no single leaf, and inventing one would key a + // lookup on a name nobody wrote. + syn::UseTree::Group(_) | syn::UseTree::Glob(_) => None, + } +} + +/// Emit one leaf as an edge, classified by the path's first segment. +fn push_leaf(name: &str, first: Option<&str>, line: usize, out: &mut Vec) { + let Some(first) = first else { + return; + }; + let (origin, to) = match first { + // THE CLASS THIS FACT EXISTS FOR: `use crate::` names no module. + // `to` stays empty until `resolve` fills it, so an unresolved edge is + // visibly blank rather than plausibly wrong. + "crate" => (Origin::RootItem, String::new()), + "self" | "super" => (Origin::Local, first.to_owned()), + other => (Origin::External, other.to_owned()), + }; + out.push(UseEdge { + to, + item: name.to_owned(), + origin, + via_root: false, + line, + }); +} + +/// Complete every [`Origin::RootItem`] edge against the crate root's table. +/// +/// The two outcomes are the two measured classes, and they are deliberately +/// different values rather than one "unknown": +/// +/// * the table names a module — the edge becomes [`Origin::Internal`] onto it, +/// which is the hidden internal edge a line predicate misses entirely; +/// * the table says the root imported the name from elsewhere — the edge becomes +/// [`Origin::External`], which is the phantom internal edge a line predicate +/// invents. +/// +/// A name the table does not carry is **left unresolved**, `to` still empty. That +/// is could-not-look at the edge level: the root may re-export it through a glob +/// this pass declines to guess at, and a fabricated destination would be worse +/// than a blank one. +pub fn resolve(edges: &mut [UseEdge], root: &RootExports) { + for edge in edges { + if edge.origin != Origin::RootItem { + continue; + } + match root.get(&edge.item) { + Some(Some(module)) => { + // `via_root` is FALSE when the name IS the module: the text named + // its destination and a line predicate reads it correctly. Only a + // name that had to be looked up is a divergence. + edge.via_root = module != &edge.item; + edge.to = module.clone(); + edge.origin = Origin::Internal; + } + Some(None) => { + edge.origin = Origin::External; + edge.via_root = true; + } + None => {} + } + } +} + +/// One file's `use` edges and its own export table, from a single parse. +/// +/// Both halves together because resolution needs both and parsing twice to get +/// them would double the cost of the fact for no property gained — the crate +/// root is a file like any other, and whichever file the caller nominates as the +/// root has already been parsed here. +#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize)] +#[non_exhaustive] +pub struct UseFile { + /// The edges this file writes, unresolved. + pub edges: Vec, + /// What this file re-exports, for the case where it IS the crate root. + #[serde(skip)] + pub exports: RootExports, +} + +/// A file's `use` facts, or the reason there are none (CLOUD-762). +/// +/// [`Look::CouldNotLook`] when the text does not parse, never an empty +/// [`UseFile`] — the same contract [`use_edges`] holds and for the same reason. +#[must_use] +pub fn use_facts(source: &str) -> Look { + match (use_edges(source), root_exports(source)) { + (Look::Is(edges), Look::Is(exports)) => Look::Is(UseFile { edges, exports }), + // One half refusing means the text did not parse, so both refuse. Stated + // as an arm rather than assumed: a `UseFile` half-built from a file the + // parser rejected is exactly the empty-set answer this fact refuses. + _ => Look::CouldNotLook, + } +} diff --git a/crates/batten/tests/cli.rs b/crates/batten/tests/cli.rs index 0e84c2503..35194946f 100644 --- a/crates/batten/tests/cli.rs +++ b/crates/batten/tests/cli.rs @@ -94,12 +94,50 @@ fn run_hook_in(dir: &std::path::Path, harness: &str, payload: &str, bypass: bool command .current_dir(dir) .args(["hook", "--harness", harness]) + .env_remove("BATTEN_HOOK_BYPASS") .env_remove("BATTEN_GH_GUARD_BYPASS") .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()); if bypass { - command.env("BATTEN_GH_GUARD_BYPASS", "1"); + command.env("BATTEN_HOOK_BYPASS", "1"); + } + let mut child = command.spawn().expect("spawn batten hook"); + child + .stdin + .take() + .expect("piped stdin") + .write_all(payload.as_bytes()) + .expect("write payload"); + child.wait_with_output().expect("run batten hook") +} + +/// `batten hook` with named environment variables set, for the per-row hatches +/// (CLOUD-437). +/// +/// [`run_hook_in`]'s `bypass` flag sets the GENERAL hatch and nothing else, which +/// cannot express "this row's hatch is set and no other's" — the distinction the +/// whole column exists to make. Both names are scrubbed first for the same reason +/// every other harness here scrubs them: `BATTEN_GH_GUARD_BYPASS` is a live row +/// hatch now, so a leaked one would disarm the four `gh` rows and their +/// assertions would pass for the wrong reason. +fn run_hook_with_env( + dir: &std::path::Path, + harness: &str, + payload: &str, + env: &[(&str, &str)], +) -> Output { + let mut command = batten(); + command + .current_dir(dir) + .args(["hook", "--harness", harness]) + .env_remove("BATTEN_HOOK_BYPASS") + .env_remove("BATTEN_GH_GUARD_BYPASS") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + for (key, value) in env { + command.env(key, value); } let mut child = command.spawn().expect("spawn batten hook"); child @@ -117,6 +155,7 @@ fn run_hook_verbose(dir: &std::path::Path, harness: &str, payload: &str) -> Outp command .current_dir(dir) .args(["-v", "hook", "--harness", harness]) + .env_remove("BATTEN_HOOK_BYPASS") .env_remove("BATTEN_GH_GUARD_BYPASS") .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -1761,6 +1800,7 @@ fn a_malformed_harness_token_fails_loud_without_denying() { for token in ["claude_code", "", "exitcode"] { let output = batten() .args(["hook", "--harness", token]) + .env_remove("BATTEN_HOOK_BYPASS") .env_remove("BATTEN_GH_GUARD_BYPASS") .stdin(Stdio::null()) .output() @@ -2014,6 +2054,7 @@ fn the_deny_is_a_function_of_config_and_argv_not_the_ambient_environment() { command .current_dir(&dir) .args(["hook", "--harness", "exit-code"]) + .env_remove("BATTEN_HOOK_BYPASS") .env_remove("BATTEN_GH_GUARD_BYPASS") .env(key, value) .stdin(Stdio::piped()) @@ -7978,6 +8019,116 @@ fn an_unknown_key_in_an_action_row_stays_a_hard_config_error() { assert_eq!(output.status.code(), Some(1), "a usage error, never a deny"); } +/// CLOUD-437's fixture: two rows, one declaring its own hatch and one not. +/// +/// The pair is the point. A suite with only a hatched row could not tell "the +/// hatch worked" from "the deny never fired", and a suite with only an unhatched +/// one could not tell the general name from a row's own. +const HATCH_POLICY_CONFIG: &str = r#"version = 1 +[[rule]] +id = "owns-its-hatch" +kind = "shape" +scope = "mediated_call" +severity = "deny" +pattern = "gh pr merge" +bypass_env = "BATTEN_GH_GUARD_BYPASS" +reason = "use `mise run land`" + +[[rule]] +id = "takes-the-general-hatch" +kind = "shape" +scope = "mediated_call" +severity = "deny" +pattern = "danger-zone" +reason = "do not" +"#; + +fn hatch_call(command: &str) -> String { + serde_json::json!({ + "hook_event_name": "PreToolUse", + "tool_name": "Bash", + "tool_input": { "command": command }, + }) + .to_string() +} + +#[test] +fn a_deny_names_the_hatch_its_own_row_declared() { + // CLOUD-437's worked case, in both directions at once: the row that owns + // `BATTEN_GH_GUARD_BYPASS` names it, and the row that does not must NOT — + // which is the defect, a deny pointing at another subsystem's variable. + let dir = repo_with_config("hatch-named", HATCH_POLICY_CONFIG); + + let owned = run_hook_with_env(&dir, "claude-code", &hatch_call("gh pr merge 42"), &[]); + let owned_text = String::from_utf8_lossy(&owned.stdout).into_owned(); + assert!( + owned_text.contains("Bypass with BATTEN_GH_GUARD_BYPASS=1."), + "the row that declared it advertises it: {owned_text}" + ); + + let general = run_hook_with_env(&dir, "claude-code", &hatch_call("danger-zone --now"), &[]); + let general_text = String::from_utf8_lossy(&general.stdout).into_owned(); + assert!( + general_text.contains("Bypass with BATTEN_HOOK_BYPASS=1."), + "a row declaring none takes the general hatch: {general_text}" + ); + assert!( + !general_text.contains("GH_GUARD"), + "no deny names a hatch belonging to a subsystem it did not come from: {general_text}" + ); +} + +#[test] +fn the_hatch_a_deny_advertises_actually_suppresses_that_deny() { + // THE LOAD-BEARING HALF (CLOUD-437 §7). A suite asserting only the string + // would pass on a hatch nothing reads, and a refusal pointing at a variable + // the boundary ignores is worse than one pointing nowhere. + let dir = repo_with_config("hatch-works", HATCH_POLICY_CONFIG); + let call = hatch_call("gh pr merge 42"); + + let denied = run_hook_with_env(&dir, "claude-code", &call, &[]); + assert!( + String::from_utf8_lossy(&denied.stdout).contains("owns-its-hatch"), + "the row denies when its hatch is unset" + ); + + let hatched = run_hook_with_env( + &dir, + "claude-code", + &call, + &[("BATTEN_GH_GUARD_BYPASS", "1")], + ); + assert!( + !String::from_utf8_lossy(&hatched.stdout).contains("owns-its-hatch"), + "setting the advertised hatch suppresses that deny: {:?}", + String::from_utf8_lossy(&hatched.stdout) + ); +} + +#[test] +fn one_rows_hatch_leaves_every_other_row_live() { + // The fidelity bar this column exists for. The bash guards were separate + // programs, so suppressing `memory-guard` left `ready-guard` live; one global + // hatch silently widens the blast radius of every bypass, invisibly, because + // the deny text cannot say what else it just switched off. + // + // Fails by: suppressing the REFUSAL post-hoc instead of removing the row — + // adjudication would stop at the hatched row and every row behind it would + // go unevaluated. + let dir = repo_with_config("hatch-narrow", HATCH_POLICY_CONFIG); + let output = run_hook_with_env( + &dir, + "claude-code", + &hatch_call("danger-zone --now"), + &[("BATTEN_GH_GUARD_BYPASS", "1")], + ); + let text = String::from_utf8_lossy(&output.stdout).into_owned(); + assert!( + text.contains("takes-the-general-hatch"), + "another row's hatch must not disarm this one: {text}" + ); +} + #[test] fn a_bypassed_call_fires_no_action() { // The bypass says "do not mediate this call"; spawning the operator's @@ -9526,6 +9677,7 @@ fn run_hook_state( command .current_dir(dir) .args(["hook", "--harness", harness]) + .env_remove("BATTEN_HOOK_BYPASS") .env_remove("BATTEN_GH_GUARD_BYPASS") .stdin(Stdio::piped()) .stdout(Stdio::piped()) diff --git a/crates/batten/tests/contract_drift.rs b/crates/batten/tests/contract_drift.rs index e589675e8..bf15afbe1 100644 --- a/crates/batten/tests/contract_drift.rs +++ b/crates/batten/tests/contract_drift.rs @@ -387,12 +387,7 @@ fn the_mediation_hatch_does_not_silence_the_advisory() { drift(&dir, "s-1"); std::fs::write(dir.join("AGENTS.md"), "# the contract\nmoved\n").unwrap(); - let output = drift_on( - &dir, - "s-1", - "PostToolBatch", - &[("BATTEN_GH_GUARD_BYPASS", "1")], - ); + let output = drift_on(&dir, "s-1", "PostToolBatch", &[("BATTEN_HOOK_BYPASS", "1")]); assert!( notice(&output).is_some_and(|text| text.contains("AGENTS.md")), "a bypassed call is unmediated, not uninformed: {:?}", diff --git a/crates/batten/tests/facts.rs b/crates/batten/tests/facts.rs index 37e2759b5..d019a165a 100644 --- a/crates/batten/tests/facts.rs +++ b/crates/batten/tests/facts.rs @@ -12,8 +12,8 @@ use std::path::PathBuf; use batten::facts::{ AGENT_SOURCED, BYPASS, Class, Cost, DOCUMENT, Fact, GIT_HEAD, GIT_RANGE, GIT_REF, GIT_REMOTE, - GIT_STATUS, KEYS, LANDING, LINES, Look, PRODUCED, PROSPECTIVE, RECEIPTS, STOP, Surface, - TRACKED, WAIVED, + GIT_STATUS, INVOCATIONS, KEYS, LANDING, LINES, Look, PRODUCED, PROSPECTIVE, RECEIPTS, STOP, + Surface, TRACKED, USES, WAIVED, }; #[test] @@ -103,6 +103,8 @@ fn every_fact_returns_its_stated_const() { Fact::GitRef => GIT_REF, Fact::GitRange => GIT_RANGE, Fact::Landing => LANDING, + Fact::Invocations => INVOCATIONS, + Fact::Uses => USES, } }; @@ -111,7 +113,7 @@ fn every_fact_returns_its_stated_const() { // rather than quietly shrinking the census. assert_eq!( Fact::ALL.len(), - 17, + 19, "the census covers every fact; update this count deliberately when the \ model gains or loses one" ); diff --git a/crates/batten/tests/use_graph.rs b/crates/batten/tests/use_graph.rs new file mode 100644 index 000000000..dbd68a073 --- /dev/null +++ b/crates/batten/tests/use_graph.rs @@ -0,0 +1,224 @@ +//! CLOUD-762's measurement, pinned against this tree. +//! +//! §2 makes the syntactic-versus-resolved error count deliverable one and says +//! the count chooses the tier, so the count lives here as an assertion rather +//! than in a comment: a number in prose goes stale silently, and this one is the +//! whole argument for the fact being `Read x Check`. +//! +//! The workspace lints forbid `unwrap`/`expect` on reachable paths, and every +//! integration suite here lifts that the same way: a test's panic IS its failure +//! report, so a `Result` it cannot proceed without is not a reachable error path. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use std::collections::BTreeMap; + +use batten::facts::Look; +use batten::uses::{Origin, RootExports, UseEdge, resolve, root_exports, use_edges}; + +/// The crate root, read from the tree rather than from a fixture — the +/// measurement is a claim ABOUT this tree and a fixture could not make it. +fn root_table() -> RootExports { + let source = std::fs::read_to_string("src/lib.rs").expect("the crate root is readable"); + match root_exports(&source) { + Look::Is(table) => table, + other => panic!("the crate root must parse, got {}", other.as_str()), + } +} + +/// Every module's resolved edges, keyed by file name. +fn resolved_tree() -> BTreeMap> { + let root = root_table(); + let mut out = BTreeMap::new(); + let entries = std::fs::read_dir("src").expect("the source directory is readable"); + for entry in entries { + let path = entry.expect("a readable entry").path(); + if path.extension().and_then(|ext| ext.to_str()) != Some("rs") { + continue; + } + let name = path + .file_name() + .and_then(|n| n.to_str()) + .expect("a utf-8 file name") + .to_owned(); + let source = std::fs::read_to_string(&path).expect("a readable source file"); + let Look::Is(mut edges) = use_edges(&source) else { + panic!("{name} must parse"); + }; + resolve(&mut edges, &root); + out.insert(name, edges); + } + out +} + +/// THE MEASUREMENT. Every site where the module a `use` reaches differs from the +/// module its text names — the count CLOUD-762 parked on for two days. +/// +/// Four, in two classes, and both are re-exports. This asserts the CLASSES and a +/// bound rather than an exact list, so adding a module does not fail the suite +/// while a new *kind* of divergence does. +/// +/// **The discriminator is `via_root`, not the item's case, and that correction +/// came from the tree.** The first version of this case asked whether a resolved +/// edge's item name was CamelCase, which is true of every ordinary +/// `use crate::error::UsageError` as well: it reported **88** divergences where +/// there are four. Only the edges resolution CHANGED are the ones a line +/// predicate gets wrong. +/// +/// Fails by: dropping the root table from `resolve`, which returns every one of +/// these edges to `RootItem` with an empty destination. +#[test] +fn the_syntactic_tier_diverges_at_four_sites_in_two_classes() { + let tree = resolved_tree(); + + let mut hidden_internal = Vec::new(); + let mut phantom_internal = Vec::new(); + let mut unresolved = Vec::new(); + for (file, edges) in &tree { + for edge in edges { + match edge.origin { + // Resolved out of the root table: the text named no module. + Origin::Internal if edge.via_root => { + hidden_internal.push(format!("{file}:{} -> {}", edge.line, edge.to)); + } + Origin::External if edge.via_root => { + phantom_internal.push(format!("{file}:{}", edge.line)); + } + Origin::RootItem => unresolved.push(format!("{file}:{}", edge.line)), + _ => {} + } + } + } + + assert_eq!( + hidden_internal.len(), + 2, + "hidden internal edges — a line predicate is silently GREEN on these: {hidden_internal:?}" + ); + assert_eq!( + phantom_internal.len(), + 2, + "phantom internal edges — really external, a line predicate invents these: {phantom_internal:?}" + ); + assert!( + unresolved.is_empty(), + "every crate-root name resolved; an unresolved one is a class the table cannot see: {unresolved:?}" + ); +} + +/// Both hidden edges land on the same module, and naming it is the point: a +/// layering table keyed on `error` would have judged neither before this. +#[test] +fn the_hidden_edges_resolve_to_the_module_the_text_never_names() { + let tree = resolved_tree(); + for file in ["trust.rs", "output.rs"] { + let edges = tree.get(file).expect("the module is in the tree"); + assert!( + edges + .iter() + .any(|edge| edge.origin == Origin::Internal && edge.via_root && edge.to == "error"), + "{file} reaches `error` through the root's re-export, and the text does not say so" + ); + } +} + +/// Aliases and globs contribute ZERO divergence in this tree, which is half the +/// measurement and the half that was not obvious in advance. +/// +/// Asserted rather than stated: the body of CLOUD-762 worried about all three +/// constructs equally, and only one of them turned out to matter. +#[test] +fn aliases_and_globs_move_no_top_level_edge() { + let tree = resolved_tree(); + let crossing: Vec = tree + .iter() + .flat_map(|(file, edges)| { + edges + .iter() + .filter(|edge| edge.origin == Origin::Local && edge.item == "*") + .map(move |edge| format!("{file}:{}", edge.line)) + }) + .collect(); + // Every glob in this tree is `use super::*` inside a `#[cfg(test)]` module. + // `Local` is the answer, and `Local` crosses no top-level boundary. + assert!( + !crossing.is_empty(), + "the tree does contain globs; a run finding none is measuring nothing" + ); + for (file, edges) in &tree { + for edge in edges { + assert_ne!( + (edge.origin, edge.item.as_str()), + (Origin::Internal, "*"), + "{file}:{} — a glob resolved to a top-level module edge", + edge.line + ); + } + } +} + +/// COULD-NOT-LOOK IS NOT AN EMPTY EDGE SET, asserted against each other. +/// +/// Fails by: returning `Look::Is(vec![])` from the parse-failure arm, which is +/// CLOUD-251's vacuous pass — a layering gate over a corpus that failed to parse +/// reports clean. +#[test] +fn an_unparseable_file_could_not_look_rather_than_having_no_edges() { + let refused = use_edges("mod m { fn f( {{{"); + assert!(refused.could_not_look(), "got {}", refused.as_str()); + + let parsed_and_empty = use_edges("pub struct S;"); + assert_eq!(parsed_and_empty, Look::Is(Vec::new())); + assert!(!parsed_and_empty.could_not_look()); + assert_ne!(refused.as_str(), parsed_and_empty.as_str()); + + // The root table carries the same contract, and it is the one a consumer + // reaches for first — an unparseable root that answered with an empty table + // would silently un-resolve every edge in the crate. + assert!(root_exports("fn f( {{{").could_not_look()); +} + +/// Module identity is stable across two runs over identical bytes, so a +/// consumer's set lookup is deterministic (§7c). +#[test] +fn the_edge_set_is_stable_across_runs() { + assert_eq!(resolved_tree(), resolved_tree()); +} + +/// The layer table CLOUD-359 gates, asserted over the real tree. +/// +/// `policy/module-layering.rego` is the gate; this is the same predicate in Rust +/// so a violation names itself in the failure output. The Rego module reports +/// pointer-only through the engine — a tree-scoped row's finding points at the +/// bundle and the module's own `msg` carries `path:line`, which is the shipped +/// convention and a deliberate decision in `rules.rs`. That is right for a gate +/// and useless for a bisect, so the enumeration lives here. +#[test] +fn the_documented_layerings_hold_over_this_tree() { + let forbidden: &[(&str, &[&str])] = &[ + ("rules", &["hook"]), + ("surface", &["cli", "lib"]), + ("cli", &["lib", "journal"]), + ("config", &["resolve", "trust", "lint", "epoch"]), + ("resolve", &["trust", "lint", "epoch"]), + ("trust", &["lint", "epoch"]), + ("lint", &["epoch"]), + ("store", &["findings", "journal"]), + ]; + let tree = resolved_tree(); + let mut broken = Vec::new(); + for (file, edges) in &tree { + let from = file.trim_end_matches(".rs"); + let Some((_, banned)) = forbidden.iter().find(|(module, _)| *module == from) else { + continue; + }; + for edge in edges { + if edge.origin == Origin::Internal && banned.contains(&edge.to.as_str()) { + broken.push(format!("{file}:{} {from} -> {}", edge.line, edge.to)); + } + } + } + assert!( + broken.is_empty(), + "documented layerings this tree violates: {broken:#?}" + ); +} diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index e2f8db199..6654ef367 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -108,7 +108,7 @@ checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" [[package]] name = "batten" -version = "0.0.108" +version = "0.0.110" dependencies = [ "anyhow", "clap", @@ -123,6 +123,7 @@ dependencies = [ "hmac", "ignore", "json5", + "proc-macro2", "regex", "regorus", "rustix", @@ -132,6 +133,7 @@ dependencies = [ "serde_json", "sha2", "signal-hook", + "syn 2.0.119", "tar", "toml", "unicode-normalization", diff --git a/policy/ancestry-decides-nothing.rego b/policy/ancestry-decides-nothing.rego new file mode 100644 index 000000000..3c0cdd116 --- /dev/null +++ b/policy/ancestry-decides-nothing.rego @@ -0,0 +1,134 @@ +# Merged-ness is decided by patch identity, never by reachability (CLOUD-36). +# +# THE MIGRATION CLOUD-756 ASKS FOR, and the scan it replaces is deleted in the +# same change — `git.rs`'s `no_ancestry_decides_merged_ness`. A module landing +# beside a scan it does not delete is two authorities on one property, which is +# the accretion that row exists to stop. +# +# WHY THIS GUARD, OUT OF THE SEVEN. It is the one this session measured wrong. +# The substring scan fired FOUR times on prose written while implementing +# CLOUD-914 and CLOUD-762 — a doc comment describing the gate, and three parser +# comments naming an example — at no call site at all. Every one was a false +# positive, and each was "fixed" by rewording English until the scanner stopped +# noticing, which is the tell that the instrument was wrong rather than the text. +# +# WHAT CHANGES, STATED IN BOTH DIRECTIONS SO THE TRADE IS LEGIBLE. +# +# GAINED: position. A token in a comment, in a doc example, or in this rule's +# own text is not a decision and no longer reads as one. That removes the +# obfuscation the scan needed — its needles were assembled by `.concat()` "so +# this test's own source is not a match", a workaround this makes unnecessary. +# +# LOST: a literal bound to a variable before it is passed. `let t = +# "..."; query(&repo, &[t])` reaches the call as a name, not a literal, and is +# invisible here where the scan would have caught the binding. The scan's own +# comment already conceded smuggling is possible -- "hand-writing a graph walk, +# which is a different and far more visible change" -- so this narrows an +# evasion that was never closed, rather than opening one that was. +# +# THE RANGE FORMS STAY LEGAL, unchanged from the scan: selecting which commits to +# hash is allowed, deciding with the result is not. + +# METADATA +# description: | +# Bound to the TREE surface: `scope = "tree"`, reading call sites rather than +# the mediated `{call, facts}` shape. +# THE BRACKETS ARE NOT STYLE: the schema file carries a hyphen, so the dotted +# form is a parse error reported as `invalid schema reference`. +# schemas: +# - input: schema["policy-input.schema"] +package batten.ancestry + +import rego.v1 + +rules contains "ancestry-decides-nothing" + +# The reachability-ANSWER surface, spelled plainly. A Rego string is not Rust +# source, so this file is not its own corpus and needs none of the scan's +# `.concat()` obfuscation -- which is the clearest single symptom of the tier +# change. +reachability_answers := { + "merge-base", + "merge_base", + "is-ancestor", + "is_ancestor", + "--contains", + "--ancestry-path", +} + +violation contains { + "rule": "ancestry-decides-nothing", + "msg": sprintf( + "%s:%d passes '%s' in command position; merged-ness is decided by patch identity, never by reachability (CLOUD-36) — a rebased landing is invisible to ancestry", + [path, site.line, token], + ), +} if { + some path, sites in input.tree.invocations + some site in sites + some argument in site.arguments + some token in reachability_answers + contains(argument, token) +} + +# A reachability verb in a COMMENT is not a decision, and asserting that is the +# whole point of the migration -- it is the case the retired scan got wrong four +# times in one session. Comments never reach the fact, so the case is spelled as +# an invocation-free file. +test_prose_naming_the_verb_is_not_a_decision if { + count(violation) == 0 with input as {"tree": {"invocations": {"crates/batten/src/facts.rs": []}}} +} + +test_the_verb_in_command_position_is_refused if { + some v in violation with input as {"tree": {"invocations": {"crates/batten/src/worktree.rs": [{ + "program": "arg", + "arguments": ["merge-base"], + "line": 88, + }]}}} + v.rule == "ancestry-decides-nothing" +} + +test_every_spelling_is_refused if { + count(violation) == 6 with input as {"tree": {"invocations": {"crates/batten/src/x.rs": [{ + "program": "args", + "arguments": [ + "merge-base", + "merge_base", + "is-ancestor", + "is_ancestor", + "--contains", + "--ancestry-path", + ], + "line": 3, + }]}}} +} + +# THE ALLOW HALF, and it is load-bearing: a rule that refused every git argument +# would satisfy the denies above and ban the range forms the scan deliberately +# permits. Selecting which commits to hash is allowed. +test_a_range_form_stays_legal if { + count(violation) == 0 with input as {"tree": {"invocations": {"crates/batten/src/git.rs": [{ + "program": "args", + "arguments": ["rev-list", "--not", "origin/main..HEAD"], + "line": 12, + }]}}} +} + +test_an_ordinary_call_is_not_judged if { + count(violation) == 0 with input as {"tree": {"invocations": {"crates/batten/src/git.rs": [{ + "program": "new", + "arguments": ["git"], + "line": 5, + }]}}} +} + +# GIT.RS IS JUDGED TOO, unchanged from the scan: the decision logic lives there, +# so exempting it would gut the gate. +test_this_modules_own_file_is_not_exempt if { + count(violation) == 1 with input as {"tree": {"invocations": {"crates/batten/src/git.rs": [{ + "program": "arg", + "arguments": ["--contains"], + "line": 40, + }]}}} +} + +#MUTANT-EXEMPT CLOUD-931|a policy module has no bats suite for `mutant` to turn red: `batten policy test` is wired to no task, so its six cases cannot be reached by the mutation runner diff --git a/policy/command-task-defined.rego b/policy/command-task-defined.rego new file mode 100644 index 000000000..f6c97f125 --- /dev/null +++ b/policy/command-task-defined.rego @@ -0,0 +1,310 @@ +# A `command` row names a task this tree defines (CLOUD-614). +# +# `mise run ` resolves from the working directory, and the working +# directory is the tree being checked. A row naming a task that is not there does +# NOT fail to launch — the runner is on PATH, so it starts, exits non-zero, and +# the engine turns that into a finding AT THE ROW'S OWN SEVERITY. +# +# MEASURED: a scratch tree carrying only a lockfile produced a `deny`-severity +# finding asserting that tree was NTIA-nonconformant, having inspected nothing. +# That is the false red this engine exists to prevent, arriving as a confident +# verdict rather than as an error somebody would notice. The existing rows +# survive on glob luck alone: they select a lockfile and no fixture carries one. +# +# WHY THIS IS A CONSUMER POLICY AND NOT AN ENGINE CHECK. CLOUD-614's Ready block +# put this clause in `config-lint`, which lives in `crates/batten`. Implemented +# there it fails immediately, and the tree says so: `no_artifact_name_reaches_ +# the_core` refused the branch, naming `src/lint.rs:221 mise.toml` twice. A task +# RUNNER is a consumer's choice, and non-negotiable rule 1 keeps the core +# ignorant of it — the engine knows formats, and which file carries which is the +# consumer's `batten.toml`. So the clause belongs where the runner is known, +# which is here. +# +# It costs nothing to express: `batten.toml` and the manifest are both parsed +# documents, and the file-task half is `input.tree.tracked`, which the walk +# already yields. Nothing new is read. +# +# THE ROW IS NOT REFUSED FOR SPELLING `mise run`. CLOUD-614 chose the premise +# that a `command` row is a claim about the repository that AUTHORED the config, +# so naming a task is legal; only naming a missing one is a defect. +# +# KNOWN RESIDUAL, from that same decision. A foreign tree defining a task of the +# same name still gets ITS task run: this asserts the task exists, never that it +# is the one the author wrote. Closing that needs a program on PATH, which +# CLOUD-614 declined. It stays a property of the premise — `batten check` is +# supported in the tree that authored its config — and the fixture is what keeps +# that premise honest. +#MUTANT-EXEMPT CLOUD-931|a policy module has no bats suite for `mutant` to turn red: `batten policy test` is wired to no task, so its own cases cannot be reached by the mutation runner + +# METADATA +# description: | +# Bound to the TREE surface: `scope = "tree"`, so it reads the tree document +# and never the mediated `{call, facts}` shape. +# THE BRACKETS ARE NOT STYLE: the schema file carries a hyphen, so the dotted +# form is a parse error reported as `invalid schema reference`. +# THIS BLOCK IS YAML AND MUST STAY THE LAST COMMENT BLOCK BEFORE `package`. +# schemas: +# - input: schema["policy-input.schema"] +package batten.command_task_defined + +import rego.v1 + +rules contains "command-task-defined" + +# Tasks the manifest declares under `[tasks]`. +defined contains name if { + some name, _ in input.tree.documents["mise.toml"].tasks +} + +# FILE TASKS, from the walk the run already did. A task directory entry is a task +# named by its basename, and the runner strips a single extension — `land.sh` is +# `mise run land`. Both spellings are recorded rather than chosen between, +# because a row naming either resolves. +defined contains name if { + uses_this_runner + some path in input.tree.tracked + startswith(path, "mise-tasks/") + parts := split(path, "/") + name := parts[count(parts) - 1] +} + +defined contains stem if { + uses_this_runner + some path in input.tree.tracked + startswith(path, "mise-tasks/") + parts := split(path, "/") + name := parts[count(parts) - 1] + contains(name, ".") + stem := split(name, ".")[0] +} + +# THE MANIFEST IS THE MARKER, and this predicate is what keeps the rule from +# judging a tree that merely HOLDS a copy of this config. +# +# Measured twice, and the second measurement is why the marker is the manifest +# rather than a non-empty namespace. Guarding on "any task found" was not enough: +# a fixture carrying one unrelated task file gave a namespace of exactly that +# file, and the six real `mise run` rows in the copied config then read as naming +# missing tasks — six findings, against a tree whose task namespace is nobody's +# claim. Supplying the real namespace instead was tried and rejected on cost, at +# ~370s per fixture case against ~1.7s. +# +# A tree with no manifest does not use this runner, so a config sitting in it is +# not its authority and its rows are not its claims to answer for. That is +# CLOUD-614's chosen premise applied to the rule itself. +uses_this_runner if input.tree.documents["mise.toml"] + +# Every task a `command` row spawns, from either column. `fix` is read for the +# same reason as `check`: `RuleKind::permits` lists both among the `command` +# kind's columns, so a repair is spawned the same way and inherits the same +# failure. Linting one and not the other leaves half the surface unjudged. +spawned contains {"id": row.id, "task": task} if { + some row in input.tree.documents["batten.toml"].rule + some column in ["check", "fix"] + task := mise_task(row[column]) +} + +# The task a `mise run` command names, or nothing for any other program. +# +# Deliberately narrow: only the exact shape is claimed. A different program, a +# bare runner invocation, or a flag-only one is not this rule's business and is +# left alone rather than guessed at. +mise_task(command) := task if { + words := [word | some word in split(command, " "); word != ""] + words[0] == "mise" + words[1] == "run" + rest := [word | some i, word in words; i > 1; not startswith(word, "-")] + task := rest[0] +} + +violation contains { + "rule": "command-task-defined", + "msg": sprintf( + "batten.toml's `%s` row runs task `%s`, which this tree does not define — the runner is on PATH so the row will START, exit non-zero, and report a finding at its own severity about a tree it never inspected", + [row.id, row.task], + ), +} if { + # ONLY WHERE A TASK SOURCE WAS FOUND. Without this guard the rule reproduces + # the very defect it exists to fix: pointed at a tree with no task source — + # a fixture that copies `batten.toml` and nothing else — every `mise run` row + # reads as naming a missing task, and the gate reports a row-per-violation + # verdict about a tree it could not assess. Measured: seven findings against + # `tests/prebuilt-lint.bats`, whose first case is named "this repository is + # clean today". An empty namespace is could-not-look, and the clause below + # says so ONCE. + count(defined) > 0 + some row in spawned + not defined[row.task] +} + +# COULD NOT LOOK, NEVER A SILENT PASS. Both documents are declared, so either +# being absent from `documents` means the walk never read it and no row can be +# judged — which must not be spelled the same way as a config whose every task +# resolves. +violation contains { + "rule": "command-task-defined", + "msg": sprintf("%s could not be parsed, so no `command` row's task could be resolved", [path]), +} if { + # THE AUTHORITY ONLY. `mise.toml` is declared as a source and lands in + # `missing` whenever a tree simply does not have one — which is every fixture + # that copies this config, and is not-applicable rather than could-not-look + # (see the decision below). `batten.toml` is different: it carries the rows, + # so a tree that could not read it cannot have its rows judged at all. + some path in input.tree.missing + endswith(path, "batten.toml") +} + +# WHY THERE IS NO "NO TASKS FOUND" CLAUSE, decided against and recorded rather +# than omitted by accident. +# +# One was written first and it broke four fixture cases, including the one named +# "this repository is clean today". Those fixtures COPY `batten.toml` into a +# scratch tree without a task namespace, so the rule saw rows to judge and no +# tasks, called it could-not-look, and reported. Supplying the namespace instead +# was tried and rejected on cost: copying the task tree took each case from +# ~1.7s to ~370s, because the walk then covers every task file. +# +# The distinction that survives is between the two absences, and only one of +# them is could-not-look: +# +# * a manifest that EXISTS and did not parse -- the boundary tried and failed, +# which is the `input.tree.missing` clause above, and it stays loud; +# * no task source AT ALL -- this tree does not use the runner, so there is no +# namespace to be judged against and no question was asked. +# +# The second is not-applicable, and spelling it as a violation makes the rule +# fire on every tree that merely HOLDS a copy of this config. That is the same +# false red CLOUD-614 is about, reproduced by the gate meant to prevent it -- +# measured here as seven findings against a fixture, before the guard above. +# +# What it costs is stated rather than hidden: a tree carrying `mise run` rows and +# no runner at all gets no finding. Under CLOUD-614's chosen premise -- a +# `command` row is a claim about the repository that AUTHORED the config -- such +# a tree is not the authority, and the rows are not its claims to answer for. + +# --- cases --------------------------------------------------------------- + +test_a_row_naming_an_undefined_task_is_refused if { + found := violation with input as {"tree": { + "documents": { + "batten.toml": {"rule": [{"id": "r", "check": "mise run absent-task"}]}, + "mise.toml": {"tasks": {"present": {}}}, + }, + "tracked": [], + "missing": [], + }} + count(found) == 1 +} + +test_a_row_naming_a_manifest_task_is_clean if { + found := violation with input as {"tree": { + "documents": { + "batten.toml": {"rule": [{"id": "r", "check": "mise run present"}]}, + "mise.toml": {"tasks": {"present": {}}}, + }, + "tracked": [], + "missing": [], + }} + count(found) == 0 +} + +# A FILE TASK COUNTS, and both spellings of it. Missing this would report a false +# red on most of this repository's own rows, which is worse than the silence it +# replaces: a gate whose first firing is a false positive gets switched off. +test_a_file_task_counts_with_and_without_its_extension if { + every check in ["mise run land", "mise run land.sh"] { + found := violation with input as {"tree": { + "documents": { + "batten.toml": {"rule": [{"id": "r", "check": check}]}, + "mise.toml": {"tasks": {}}, + }, + "tracked": ["mise-tasks/land.sh"], + "missing": [], + }} + count(found) == 0 + } +} + +# THE `fix` COLUMN IS THE SAME SURFACE. A repair is spawned the same way, so a +# rule reading only `check` leaves half of it unjudged. +test_a_repair_naming_an_undefined_task_is_refused if { + found := violation with input as {"tree": { + "documents": { + "batten.toml": {"rule": [{"id": "r", "fix": "mise run absent-task"}]}, + "mise.toml": {"tasks": {"present": {}}}, + }, + "tracked": [], + "missing": [], + }} + count(found) == 1 +} + +# A PROGRAM ON PATH IS NOT THIS RULE'S BUSINESS — the shape that never had the +# defect. Reporting it would refuse the one `command` row spelling that works +# everywhere. +test_a_program_on_path_is_left_alone if { + found := violation with input as {"tree": { + "documents": { + "batten.toml": {"rule": [{"id": "r", "check": "hk util check-merge-conflict"}]}, + "mise.toml": {"tasks": {}}, + }, + "tracked": ["mise-tasks/land.sh"], + "missing": [], + }} + count(found) == 0 +} + +# Flags between `run` and the task must not be read as the task. +test_a_flag_is_not_mistaken_for_the_task if { + found := violation with input as {"tree": { + "documents": { + "batten.toml": {"rule": [{"id": "r", "check": "mise run --quiet present --precondition"}]}, + "mise.toml": {"tasks": {"present": {}}}, + }, + "tracked": [], + "missing": [], + }} + count(found) == 0 +} + +# An unreadable AUTHORITY is loud: it carries the rows, so nothing can be judged. +test_an_unreadable_authority_is_loud if { + found := violation with input as {"tree": { + "documents": {}, + "tracked": [], + "missing": ["batten.toml"], + }} + count(found) == 1 +} + +# AN ABSENT MANIFEST IS NOT-APPLICABLE, and this case is why the clause above is +# narrowed. `mise.toml` is a declared source, so every tree without one puts it +# in `missing` — including every fixture that copies this config. Reporting there +# is CLOUD-614's own false red, measured as a failure of the case named "this +# repository is clean today". +test_an_absent_manifest_is_not_reported if { + found := violation with input as {"tree": { + "documents": {"batten.toml": {"rule": [{"id": "r", "check": "mise run anything"}]}}, + "tracked": ["mise-tasks/other.sh"], + "missing": ["mise.toml"], + }} + count(found) == 0 +} + +# The vacuity clause: rows to judge and no task source found is could-not-look, +# never consent. +# THE DECISION ABOVE, AS A CASE. A tree with rows to judge and no task source is +# NOT-APPLICABLE, not could-not-look: reporting here makes the rule fire on every +# tree merely holding a copy of this config, which is CLOUD-614's own false red. +# Measured before the guard: seven findings against `tests/prebuilt-lint.bats`. +test_rows_with_no_task_source_at_all_are_not_this_rules_business if { + found := violation with input as {"tree": { + "documents": { + "batten.toml": {"rule": [{"id": "r", "check": "mise run anything"}]}, + "mise.toml": {"tasks": {}}, + }, + "tracked": [], + "missing": [], + }} + count(found) == 0 +} diff --git a/policy/module-layering.rego b/policy/module-layering.rego new file mode 100644 index 000000000..cd533f9ee --- /dev/null +++ b/policy/module-layering.rego @@ -0,0 +1,239 @@ +# Module layering, enforced over the resolved `use` graph (CLOUD-359). +# +# THE LAYERINGS ARE DOCUMENTED AND GATED BY NOTHING, which is this row's finding. +# `module-map-check` asserts every module APPEARS in the map -- a membership +# check -- and says nothing about any edge, the same census-for-predicate +# substitution `tests-not-deleted` and `assertions-not-gutted` make. A back-edge +# compiled and passed every gate. +# +# WHAT THIS TABLE CONTAINS, AND WHAT IT DELIBERATELY DOES NOT. Only claims the +# tree already states in its own doc comments. The row puts reorganising modules +# out of scope and it is right to: inventing a rank for all 65 modules would be +# declaring an architecture nobody agreed to, under cover of enforcing one. So +# the table is a FORBIDDEN-EDGE set drawn from prose that already exists, and it +# grows when somebody documents another layering. +# +# ABSENCE IS AN ERROR, NOT AN ALLOW. `declared_modules` must name every module in +# the judged set. A module nobody has placed is a hole in the claim, and a table +# that silently allowed it would be the vacuous pass this row cites CLOUD-251 +# for. It earned its keep on the first run: three modules were missing from the +# first draft of the table, and the rule named all three before a human read it. +# +# IT READS `input.tree.uses`, NOT LINES. CLOUD-762 measured a line predicate +# wrong at four sites in this tree -- two edges it cannot see and two it invents +# -- so a layering gate on lines would ship a known false green. The resolved +# fact costs the same parse and is right about all four. +# +# COVERAGE IS AN EXEMPTION RATHER THAN FOUR MUTATIONS. Four mutation rows were +# written here first and `mise run mutant` refused every one with `no-suite`: +# the runner mutates a gate and asks a BATS suite to go red, and a policy module +# has none because `batten policy test` is wired to no task. That is CLOUD-931 +# exactly, and the two sibling policy modules already carry this exemption. +# Writing the suite would add bash to the census CLOUD-843 is retiring, to test a +# module with nine passing cases of its own. +# +# WHAT STANDS IN FOR IT, so the gap is named rather than unmeasured: the +# acceptance clause was observed END TO END. `use crate::journal::Entry` was +# seeded into `cli.rs`, `batten enforce` reported `module-layering`, and the +# finding went away on revert -- clean tree zero, seeded tree one. +#MUTANT-EXEMPT CLOUD-931|a policy module has no bats suite for `mutant` to turn red: `batten policy test` is wired to no task, so its nine cases cannot be reached by the mutation runner + +# METADATA +# description: | +# Bound to the TREE surface: this row is `scope = "tree"`, so it reads the tree +# document and never the mediated `{call, facts}` shape. +# THE BRACKETS ARE NOT STYLE: the schema file carries a hyphen, so the dotted +# form is a parse error reported as `invalid schema reference` rather than as a +# missing bind, and an unbound module type checks as `Any`. +# THIS BLOCK IS YAML AND MUST STAY THE LAST COMMENT BLOCK BEFORE `package`: +# OPA parses the whole contiguous block that starts the annotation, so prose +# placed after it reaches the YAML parser and the module fails to load. +# schemas: +# - input: schema["policy-input.schema"] +package batten.module_layering + +import rego.v1 + +rules contains "module-layering" + +# Every module this table has placed. A module in the judged set and absent here +# is refused rather than allowed. +declared_modules := { + "action", "attribution", "baseline", "budget", "bypass", "capture", "ci", + "cli", "commit", "completion", "config", "contract", "decision", "defects", + "design", "doctor", "drain", "effect", "emission", "epoch", "error", "exec", + "exit", "facts", "findings", "git", "handler", "hook", "identity", "init", + "invocation", "journal", "judge", "lib", "lint", "markers", "outputs", + "output", "pattern", "policy", "provision", "receipt", "redirect", "refusal", + "render", "resolve", "rules", "secrets", "session", "severity", "sink", + "spec", "state", "stop", "store", "surface", "transcript", "trust", "uses", + "verbs", "waiver", "worktree", + # `brief`, `main` and `selfwrite` were absent from the first draft of this + # table, and the coverage rule caught all three on its first run against the + # real tree — which is the property working before any human read it. `main` + # is the binary entry point rather than a `pub mod` of the library, and it is + # declared rather than excluded: it is a file in the judged set, and a + # selector carve-out would be an exemption where a placement is honest. + "brief", "main", "selfwrite", +} + +# THE FORBIDDEN EDGES, each traceable to prose already in the tree. +# +# `rules -> hook` is the sharpest instance and the row says so: `refusal.rs` +# states that housing the refusal table in `hook` "would make `rules` import +# `hook` and close a module cycle", and `mem:core` repeats it. rustc permits +# mutual `use` inside one crate, so that reasoning is held today by whoever +# remembers it. +# +# The three chains are the layerings `mem:core` and the module docs state, read +# as "a lower tier must not reach a higher one": +# surface (data) -> cli (typed values) -> lib (dispatch) +# config (load) -> resolve (precedence) -> trust (judging) -> lint -> epoch +# store (which store) / findings (what is in it) / journal (how it is written) +forbidden[from] contains to if { + some from, targets in { + "rules": {"hook"}, + "surface": {"cli", "lib"}, + "cli": {"lib", "journal"}, + "config": {"resolve", "trust", "lint", "epoch"}, + "resolve": {"trust", "lint", "epoch"}, + "trust": {"lint", "epoch"}, + "lint": {"epoch"}, + "store": {"findings", "journal"}, + } + some to in targets +} + +# The module a judged path denotes. Stem of the file name, which is Rust's own +# mapping from `foo.rs` to module `foo`. +module_of(path) := name if { + parts := split(path, "/") + file := parts[count(parts) - 1] + name := substring(file, 0, count(file) - 3) +} + +# A forbidden edge, reported at the line that wrote it. +# +# Pointer-only (non-negotiable rule 4): the path, the line, and the two module +# names. Never the source line — `to` and `item` are path segments, and the text +# that produced them stays on the engine's side. +violation contains { + "rule": "module-layering", + "msg": sprintf( + "%s:%d %s must not reach %s — a layering this tree documents and nothing enforced", + [path, edge.line, module_of(path), edge.to], + ), +} if { + some path, edges in input.tree.uses + some edge in edges + edge.origin == "internal" + # Set INDEXING rather than a bare `in` expression: the pinned type checker + # reports `undefined function internal.member_2` for the latter over a + # computed set, and indexing says the same thing in a form it can type. + forbidden[module_of(path)][edge.to] +} + +# A module nobody placed. An implicit allow here is the hole this table exists to +# close, so it is a finding rather than silence. +violation contains { + "rule": "module-layering", + "msg": sprintf( + "%s is judged by the layering rule and absent from its table; place it or narrow the row's selector", + [path], + ), +} if { + some path, _ in input.tree.uses + not declared_modules[module_of(path)] +} + +# THE VACUITY GUARD, and the row demands it by name: "an empty or unsatisfiable +# layer table fails rather than passing quietly". A table that forbids nothing +# reporting "no violations" is CLOUD-251's failure mode, and the reason a gate +# can be switched off by deletion without anything going red. +violation contains { + "rule": "module-layering", + "msg": "the layer table forbids no edge, so this rule decides nothing — a gate that cannot refuse is off", +} if { + count(forbidden) == 0 +} + +# THE OTHER HALF OF VACUITY IS THE ENGINE'S, NOT THIS MODULE'S. A selector that +# matched no file is `NotObserved::RuleSkipped` in `rules::evaluate` — reported +# as not-observed rather than as clean, which is the same three-valued answer +# this module would give and is given once rather than twice. A violation here +# too would fire in every consumer tree that carries no Rust, which is a rule +# refusing a repository for not being this one (measured: a fixture repo with no +# `crates/` tripped it). + +# The predicate's own tests. The ALLOW cases are the load-bearing half: a rule +# that fired on everything would satisfy every deny below and gate nothing. + +# One judged file whose module is declared and whose edges are given. +judging(path, edges) := {"tree": {"uses": {path: edges}}} + +internal(to, line) := {"to": to, "item": "x", "origin": "internal", "via-root": false, "line": line} + +test_the_documented_cycle_claim_is_refused if { + some v in violation with input as judging( + "crates/batten/src/rules.rs", + [internal("hook", 52)], + ) + v.rule == "module-layering" +} + +# The row's own acceptance clause, spelled as a case. +test_the_acceptance_clause_edge_is_refused if { + count(violation) == 1 with input as judging( + "crates/batten/src/cli.rs", + [internal("journal", 17)], + ) +} + +test_a_documented_chain_is_refused_in_the_wrong_direction if { + count(violation) == 1 with input as judging( + "crates/batten/src/config.rs", + [internal("trust", 44)], + ) +} + +# THE DIRECTION IS THE POINT. The same pair the other way round is the layering +# working as documented, and a rule that refused both would be banning the edge +# rather than ordering it. +test_the_same_chain_is_clean_in_the_declared_direction if { + count(violation) == 0 with input as judging( + "crates/batten/src/trust.rs", + [internal("config", 44)], + ) +} + +test_an_unrelated_edge_is_not_this_rules_business if { + count(violation) == 0 with input as judging( + "crates/batten/src/hook.rs", + [internal("receipt", 47)], + ) +} + +# An EXTERNAL edge is not a module edge. Without this the rule would fire on a +# crate that happens to share a module's name. +test_an_external_edge_is_never_a_layering_violation if { + count(violation) == 0 with input as judging( + "crates/batten/src/rules.rs", + [{"to": "hook", "item": "x", "origin": "external", "via-root": false, "line": 9}], + ) +} + +# The coverage half: a module the table never placed. +test_an_unplaced_module_is_refused_rather_than_allowed if { + some v in violation with input as judging( + "crates/batten/src/brand_new.rs", + [internal("error", 3)], + ) + contains(v.msg, "absent from its table") +} + +# A selector that matched nothing reaches this module as an empty set and it says +# NOTHING, deliberately: the engine already skipped the row before evaluation, so +# a second refusal here would fire in every tree that carries no Rust. +test_an_empty_judged_set_is_the_engines_answer_not_this_modules if { + count(violation) == 0 with input as {"tree": {"uses": {}}} +} diff --git a/policy/workspace-dep-referenced.rego b/policy/workspace-dep-referenced.rego new file mode 100644 index 000000000..0310ac9e9 --- /dev/null +++ b/policy/workspace-dep-referenced.rego @@ -0,0 +1,218 @@ +# Every `[workspace.dependencies]` key is referenced by a member (CLOUD-882). +# +# `[workspace.dependencies]` FIXES A VERSION; it does not add a dependency. A +# member must name the key with `.workspace = true`. So an entry no member +# references resolves to nothing, and every gate that reads the RESOLVED graph +# reports green about a tree that does not contain it. +# +# MEASURED, and the measurement is why this is a gate rather than a note. +# `orphan-probe = "=9.9.9"` -- a crate that does not exist, at a version that +# does not exist -- added to the root table alone produced four greens: +# `cargo metadata` resolved clean with the crate absent from the package list, +# `macos-link-check` exited 0, and `deny` reported advisories, bans, licenses and +# sources all ok. Cargo does not complain either: an unreferenced entry is legal +# and inert by design, which is exactly why nothing downstream can see it. +# +# It is self-concealing in the worst direction. Adding a dependency is precisely +# when somebody runs those gates deliberately, so a typo'd or half-finished +# declaration makes every one of them agree, confidently, about nothing. +# +# NOT BASH, and that is a decision with a scar behind it. A line pass would have +# to associate a key with its `[table]` and correlate across manifests, which is +# the shape that took four review rounds on `attribution-check` and produced +# nothing durable (CLOUD-873, canceled). Two parsed documents answer it in one +# walk, which is what a `policy` row is for. +# +# WHAT IT DOES NOT REPORT. A member naming a key the root does not declare is +# cargo's own hard error, and re-reporting it here would be a second voice on a +# question already answered loudly. This rule is one-directional on purpose. +# +# THE UNREADABLE-MANIFEST CLAUSE IS NOT OPTIONAL, and it is the same class this +# row is about. A declared path that failed to parse lands in `input.tree.missing` +# rather than in `documents`; without the clause below it is simply absent from +# the walk and the module reports green over a manifest it never read. +#MUTANT-EXEMPT CLOUD-931|a policy module has no bats suite for `mutant` to turn red: `batten policy test` is wired to no task, so its own cases cannot be reached by the mutation runner + +# METADATA +# description: | +# Bound to the TREE surface: this row is `scope = "tree"`, so it reads the tree +# document and never the mediated `{call, facts}` shape. +# THE BRACKETS ARE NOT STYLE: the schema file carries a hyphen, so the dotted +# form is a parse error reported as `invalid schema reference`, and an unbound +# module type checks as `Any` -- silently unchecked (CLOUD-876). +# THIS BLOCK IS YAML AND MUST STAY THE LAST COMMENT BLOCK BEFORE `package`: +# OPA parses the whole contiguous block that starts the annotation, so prose +# after it reaches the YAML parser and the module fails to load. +# schemas: +# - input: schema["policy-input.schema"] +package batten.workspace_dep_referenced + +import rego.v1 + +rules contains "workspace-dep-referenced" + +# The root manifest's declared keys. +declared contains key if { + some key, _ in input.tree.documents["Cargo.toml"].workspace.dependencies +} + +# Every key any member references with `workspace = true`. +# +# THE THREE TABLES AND THEIR `target.*` FORMS, because a dependency declared +# under `[target.'cfg(unix)'.dependencies]` is as referenced as any other and a +# rule that missed it would report a false orphan — worse than the silence it +# replaces, since a false red gets the gate switched off. +referenced contains key if { + some path, doc in input.tree.documents + path != "Cargo.toml" + some table in ["dependencies", "dev-dependencies", "build-dependencies"] + some key, value in doc[table] + value.workspace == true +} + +referenced contains key if { + some path, doc in input.tree.documents + path != "Cargo.toml" + some _, cfg in doc.target + some table in ["dependencies", "dev-dependencies", "build-dependencies"] + some key, value in cfg[table] + value.workspace == true +} + +# THE ORPHAN. Pointer-only: the KEY and the manifest that declares it, never a +# line of either file. A document finding carries no line number by construction, +# so the message is where the pointer lives. +violation contains { + "rule": "workspace-dep-referenced", + "msg": sprintf( + "Cargo.toml declares `%s` in [workspace.dependencies] and no member references it with `workspace = true`, so it resolves to nothing and every graph-reading gate is green about a dependency that is not in the tree", + [key], + ), +} if { + some key in declared + not referenced[key] +} + +# COULD NOT LOOK, NEVER A SILENT PASS. A manifest the engine could not parse is +# in `missing`, and without this the walk above simply does not see it. +violation contains { + "rule": "workspace-dep-referenced", + "msg": sprintf( + "%s could not be parsed, so its references were never counted and an orphan cannot be ruled out", + [path], + ), +} if { + some path in input.tree.missing + endswith(path, "Cargo.toml") +} + +# ANTI-VACUITY. A run that judged no root table decided nothing, and reporting +# clean would be indistinguishable from a workspace with no orphans. This is the +# CLOUD-251 shape the whole row is an instance of, closed on the module's own +# input rather than assumed away. +violation contains { + "rule": "workspace-dep-referenced", + "msg": "Cargo.toml carries no [workspace.dependencies] table in the judged set, so this rule decided nothing", +} if { + count(declared) == 0 +} + +# --- cases --------------------------------------------------------------- +# +# The discriminating PAIR is the point (CLOUD-418): the same fixture with and +# without a member reference must land on opposite verdicts. A red-only case +# would pass over a rule that refused everything. + +test_an_orphaned_key_is_refused if { + found := violation with input as {"tree": { + "documents": { + "Cargo.toml": {"workspace": {"dependencies": {"orphan-probe": "=9.9.9"}}}, + "crates/m/Cargo.toml": {"dependencies": {}}, + }, + "missing": [], + }} + count(found) == 1 +} + +test_a_referenced_key_is_clean if { + found := violation with input as {"tree": { + "documents": { + "Cargo.toml": {"workspace": {"dependencies": {"serde": "1"}}}, + "crates/m/Cargo.toml": {"dependencies": {"serde": {"workspace": true}}}, + }, + "missing": [], + }} + count(found) == 0 +} + +# `workspace = false` is a member declining the workspace version, which is NOT +# a reference — the key still resolves to nothing. A rule keyed on the presence +# of the field rather than its value would call this clean. +test_workspace_false_is_not_a_reference if { + found := violation with input as {"tree": { + "documents": { + "Cargo.toml": {"workspace": {"dependencies": {"serde": "1"}}}, + "crates/m/Cargo.toml": {"dependencies": {"serde": {"workspace": false}}}, + }, + "missing": [], + }} + count(found) == 1 +} + +# A dev-dependency counts, and so does a target-conditional one. Both are real +# shapes in this workspace, and a rule that missed either would report a FALSE +# orphan — worse than silence, because a false red gets the gate switched off. +test_a_dev_dependency_counts if { + found := violation with input as {"tree": { + "documents": { + "Cargo.toml": {"workspace": {"dependencies": {"insta": "1"}}}, + "crates/m/Cargo.toml": {"dev-dependencies": {"insta": {"workspace": true}}}, + }, + "missing": [], + }} + count(found) == 0 +} + +test_a_target_conditional_dependency_counts if { + found := violation with input as {"tree": { + "documents": { + "Cargo.toml": {"workspace": {"dependencies": {"nix": "0.29"}}}, + "crates/m/Cargo.toml": {"target": {"cfg(unix)": {"dependencies": {"nix": {"workspace": true}}}}}, + }, + "missing": [], + }} + count(found) == 0 +} + +# CARGO'S ERROR, NOT THIS GATE'S. A member naming a key the root does not declare +# is a hard cargo failure already; a second voice here would be noise on a +# question answered loudly elsewhere. +test_a_member_reference_the_root_lacks_is_not_reported if { + found := violation with input as {"tree": { + "documents": { + "Cargo.toml": {"workspace": {"dependencies": {"serde": "1"}}}, + "crates/m/Cargo.toml": {"dependencies": { + "serde": {"workspace": true}, + "undeclared": {"workspace": true}, + }}, + }, + "missing": [], + }} + count(found) == 0 +} + +# THE FALSE-GREEN SHAPE THIS ROW IS ITSELF AN INSTANCE OF. An unreadable member +# manifest must be loud: its references were never counted, so an orphan cannot +# be ruled out. +test_an_unparseable_manifest_is_loud if { + found := violation with input as {"tree": { + "documents": {"Cargo.toml": {"workspace": {"dependencies": {"serde": "1"}}}}, + "missing": ["crates/m/Cargo.toml"], + }} + count(found) == 2 +} + +test_no_root_table_decides_nothing_and_says_so if { + found := violation with input as {"tree": {"documents": {}, "missing": []}} + count(found) == 1 +} diff --git a/schema/batten.local.schema.json b/schema/batten.local.schema.json index afd76598c..ba8cb152f 100644 --- a/schema/batten.local.schema.json +++ b/schema/batten.local.schema.json @@ -444,6 +444,13 @@ "null" ] }, + "bypass_env": { + "description": "The environment variable that suppresses **this row**, named in this\nrow's own deny (CLOUD-437).\n\nAbsent means the general hatch, [`crate::hook::BYPASS_ENV`]. That default\nis not a hedge — it is what keeps the guarantee true as rows are added:\nper-row with no fallback would leave the next row hatchless and silent,\nwhich is the failure that produced this column. A row declaring one is\ndeclaring that its bypass is a *separate decision* from every other row's.\n\nWhy a per-row column rather than one global name: the bash guards this\nsurface inherited each had their own hatch, so an operator could suppress\n`memory-guard` alone while `ready-guard` stayed live. One global name\nsilently widens the blast radius of every bypass — invisibly, because the\ndeny text cannot say what else it just switched off.\n\nIt is an environment variable rather than a `batten.local.toml` key on\npurpose. A hatch LOWERS policy, and house-style §8's override channel is\nraise-only; an env var is a per-invocation decision, visible in the\ncommand that took it and gone when the process exits, where a config key\nwould make the same lowering persistent and invisible.", + "type": [ + "string", + "null" + ] + }, "check": { "description": "The **inspection-only** command template a [`RuleKind::Command`] rule\nruns as its gate. Required by that kind, rejected by any other.\n\nSplit on whitespace into `program` plus arguments and executed\n**directly — never through a shell**, so what runs is exactly what a\nreviewer reads (§9: rules \"name a command already on the operator's\nPATH\"). A bare [`FILES_PLACEHOLDER`] argument expands in place to the\nmatched paths; omit it and the command self-discovers.\n\nNamed for the half it is, not for the act of running: §9's duality has\ntwo commands, and a column called `run` could name either. Enforcement\nis always this one.", "type": [ @@ -596,6 +603,20 @@ "null" ] }, + "invocation_sources": { + "description": "The same, glob-selected (CLOUD-914).\n\nShips WITH its literal column rather than arriving a row later, which is\nthe asymmetry `line_sources` records having to fix: a gate over call\nsites is a gate over many files by construction, so the enumerated\nspelling would go stale the first time a module was added.", + "type": "array", + "items": { + "type": "string" + } + }, + "invocations": { + "description": "The Rust files this policy row reads as **call sites**, by literal path\n(CLOUD-914).\n\n[`Rule::lines`]' sibling one tier up: `lines` answers *does this token\nappear in this file*, and this answers *does it appear in command\nposition*. A row wanting the second must say so, because parsing is\nstrictly more expensive than splitting and nobody should pay it by\naccident.", + "type": "array", + "items": { + "type": "string" + } + }, "key": { "description": "Which git fact a [`RuleKind::Receipt`] row's receipts are keyed to.\n\nOptional with a pinned default of [`ReceiptKey::Head`], the conservative\none: a HEAD-keyed receipt expires on an amend or a rebase, so omitting\nthe key can only make a gate stricter than intended, never weaker.", "anyOf": [ @@ -876,6 +897,20 @@ } ] }, + "use_sources": { + "description": "The same, glob-selected (CLOUD-762).\n\nA layering rule decides over every module by construction, so the glob is\nthe spelling that matters and it ships with its literal column.", + "type": "array", + "items": { + "type": "string" + } + }, + "uses": { + "description": "The Rust files this policy row reads as a **`use` graph**, by literal path\n(CLOUD-762).\n\nSeparate from [`Rule::invocations`] because the two answer different\nquestions over the same parse — what a file CALLS versus what it REACHES —\nand a row wanting one should not pay to project the other.", + "type": "array", + "items": { + "type": "string" + } + }, "verbatim": { "description": "Hash the matched span **verbatim** rather than collapsing its whitespace.\n\nFor a rule whose subject *is* literal content, where a reformat genuinely\nchanges the thing being flagged. The default collapses whitespace on the\n`git patch-id` model, so a formatter reflow does not re-mint an identity;\na `verbatim` rule trades that for sensitivity to the bytes it is about.\n`Option` rather than `bool` so the column census can see presence\n(`Rule::columns`), which is what makes a `verbatim` on a kind that hashes\nno span a usage error instead of an ignored key.", "type": [ diff --git a/schema/batten.schema.json b/schema/batten.schema.json index c99ae2e28..c24d03c7c 100644 --- a/schema/batten.schema.json +++ b/schema/batten.schema.json @@ -1454,6 +1454,13 @@ "null" ] }, + "bypass_env": { + "description": "The environment variable that suppresses **this row**, named in this\nrow's own deny (CLOUD-437).\n\nAbsent means the general hatch, [`crate::hook::BYPASS_ENV`]. That default\nis not a hedge — it is what keeps the guarantee true as rows are added:\nper-row with no fallback would leave the next row hatchless and silent,\nwhich is the failure that produced this column. A row declaring one is\ndeclaring that its bypass is a *separate decision* from every other row's.\n\nWhy a per-row column rather than one global name: the bash guards this\nsurface inherited each had their own hatch, so an operator could suppress\n`memory-guard` alone while `ready-guard` stayed live. One global name\nsilently widens the blast radius of every bypass — invisibly, because the\ndeny text cannot say what else it just switched off.\n\nIt is an environment variable rather than a `batten.local.toml` key on\npurpose. A hatch LOWERS policy, and house-style §8's override channel is\nraise-only; an env var is a per-invocation decision, visible in the\ncommand that took it and gone when the process exits, where a config key\nwould make the same lowering persistent and invisible.", + "type": [ + "string", + "null" + ] + }, "check": { "description": "The **inspection-only** command template a [`RuleKind::Command`] rule\nruns as its gate. Required by that kind, rejected by any other.\n\nSplit on whitespace into `program` plus arguments and executed\n**directly — never through a shell**, so what runs is exactly what a\nreviewer reads (§9: rules \"name a command already on the operator's\nPATH\"). A bare [`FILES_PLACEHOLDER`] argument expands in place to the\nmatched paths; omit it and the command self-discovers.\n\nNamed for the half it is, not for the act of running: §9's duality has\ntwo commands, and a column called `run` could name either. Enforcement\nis always this one.", "type": [ @@ -1606,6 +1613,20 @@ "null" ] }, + "invocation_sources": { + "description": "The same, glob-selected (CLOUD-914).\n\nShips WITH its literal column rather than arriving a row later, which is\nthe asymmetry `line_sources` records having to fix: a gate over call\nsites is a gate over many files by construction, so the enumerated\nspelling would go stale the first time a module was added.", + "type": "array", + "items": { + "type": "string" + } + }, + "invocations": { + "description": "The Rust files this policy row reads as **call sites**, by literal path\n(CLOUD-914).\n\n[`Rule::lines`]' sibling one tier up: `lines` answers *does this token\nappear in this file*, and this answers *does it appear in command\nposition*. A row wanting the second must say so, because parsing is\nstrictly more expensive than splitting and nobody should pay it by\naccident.", + "type": "array", + "items": { + "type": "string" + } + }, "key": { "description": "Which git fact a [`RuleKind::Receipt`] row's receipts are keyed to.\n\nOptional with a pinned default of [`ReceiptKey::Head`], the conservative\none: a HEAD-keyed receipt expires on an amend or a rebase, so omitting\nthe key can only make a gate stricter than intended, never weaker.", "anyOf": [ @@ -1886,6 +1907,20 @@ } ] }, + "use_sources": { + "description": "The same, glob-selected (CLOUD-762).\n\nA layering rule decides over every module by construction, so the glob is\nthe spelling that matters and it ships with its literal column.", + "type": "array", + "items": { + "type": "string" + } + }, + "uses": { + "description": "The Rust files this policy row reads as a **`use` graph**, by literal path\n(CLOUD-762).\n\nSeparate from [`Rule::invocations`] because the two answer different\nquestions over the same parse — what a file CALLS versus what it REACHES —\nand a row wanting one should not pay to project the other.", + "type": "array", + "items": { + "type": "string" + } + }, "verbatim": { "description": "Hash the matched span **verbatim** rather than collapsing its whitespace.\n\nFor a rule whose subject *is* literal content, where a reformat genuinely\nchanges the thing being flagged. The default collapses whitespace on the\n`git patch-id` model, so a formatter reflow does not re-mint an identity;\na `verbatim` rule trades that for sensitivity to the bytes it is about.\n`Option` rather than `bool` so the column census can see presence\n(`Rule::columns`), which is what makes a `verbatim` on a kind that hashes\nno span a usage error instead of an ignored key.", "type": [ diff --git a/schema/policy-input.schema.json b/schema/policy-input.schema.json index 8ef50776f..5e1e70e29 100644 --- a/schema/policy-input.schema.json +++ b/schema/policy-input.schema.json @@ -110,6 +110,31 @@ "null" ] }, + "invocations": { + "additionalProperties": { + "items": { + "additionalProperties": false, + "properties": { + "arguments": { + "items": { + "type": "string" + }, + "type": "array" + }, + "line": { + "type": "integer" + }, + "program": { + "type": "string" + } + }, + "type": "object" + }, + "type": "array" + }, + "description": "Fact::Invocations (CLOUD-914). Path -> the call sites in that Rust file, each `program` (the callee as written), `arguments` (the string literals it PASSES -- never its receiver) and `line`. A path absent from this map is could-not-look: the parser could not read it. A path present with an empty list is a file that parsed and calls nothing. Rego reads an undefined path as `does not hold`, so those two must never collapse.", + "type": "object" + }, "landing": { "additionalProperties": { "additionalProperties": false, @@ -165,6 +190,34 @@ "type": "string" }, "type": "array" + }, + "uses": { + "additionalProperties": { + "items": { + "additionalProperties": false, + "properties": { + "item": { + "type": "string" + }, + "line": { + "type": "integer" + }, + "origin": { + "type": "string" + }, + "to": { + "type": "string" + }, + "via-root": { + "type": "boolean" + } + }, + "type": "object" + }, + "type": "array" + }, + "description": "Fact::Uses (CLOUD-762). Path -> that file's `use` edges. `to` is the module or crate reached AFTER resolution through the crate root's re-export table; `item` the imported leaf name; `origin` one of internal/external/root-item/local; `via_root` whether resolution supplied `to` rather than the text, which is the flag that marks an edge a line predicate reads wrongly. An edge still `root-item` is one the root's table could not name, and is could-not-look at the edge level rather than an edge onto nothing. A path absent from this map could not be parsed; a path present with an empty array imports nothing.", + "type": "object" } }, "type": "object" diff --git a/tests/prebuilt-lint.bats b/tests/prebuilt-lint.bats index 27ba823c1..615eb8693 100644 --- a/tests/prebuilt-lint.bats +++ b/tests/prebuilt-lint.bats @@ -144,6 +144,22 @@ waive() { # The half that a narrowed pattern or a rule that matches nothing would also # satisfy, which is why every case below exists too. cp "$REPO/mise.toml" "$ROOT/mise.toml" + # THE TASK NAMESPACE THE COPIED CONFIG NEEDS (CLOUD-614). That config carries + # `command` rows spelled `mise run `, and a task resolves from the + # working directory — so a fixture bringing the config without the namespace + # manufactures the one invocation this repository does not support, and + # `command-task-defined` reports it. Satisfy the precondition, do not strip + # the rows: the same principle as the policy modules and the ratchet base. + # + # DERIVED FROM THE CONFIG rather than listed, so adding a `command` row does + # not silently go unrepresented here. Declared in the MANIFEST rather than as + # files under the task directory: real task files would be walked by every + # tree-scoped rule in the copied config, which was measured at ~370s per case + # against ~1.7s. + sed -n 's/.*mise run \([a-z0-9:_-]*\).*/\1/p' "$ROOT/batten.toml" | sort -u | + while read -r task; do + printf '\n[tasks."%s"]\nrun = "true"\n' "$task" >>"$ROOT/mise.toml" + done cp "$REPO"/.github/workflows/*.yml "$ROOT/.github/workflows/" run check [ "$status" -eq 0 ]