diff --git a/clippy.toml b/clippy.toml index bcebf47..4e8bcdd 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1 +1,5 @@ too-many-lines-threshold = 250 + +# Proper nouns clippy's doc lint would otherwise read as unbackticked code. +# `..` keeps its default list rather than replacing it. +doc-valid-idents = ["CircleCI", ".."] diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 74f718e..5a5f0ff 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -2137,9 +2137,11 @@ likely a different branch, and a green tick about the wrong tree is the failure the push guard refuses for the same reason. The changed set is filtered to `Cargo.toml`, `Cargo.lock`, `uv.lock`, -`pyproject.toml`, `package.json`, `package-lock.json` and -`.github/workflows/**`. A range holding none of them runs no scanner, prints -one line and exits `0`. Per scanner: osv-scanner is handed the changed +`pyproject.toml`, `package.json`, `package-lock.json`, `.github/workflows/**` +and the unscanned CI configuration below — that last group is in the set +*because* nothing reads it, so a push carrying only a `.circleci/config.yml` +is told the file went unscanned instead of being told there was nothing here. +A range holding none of them runs no scanner, prints one line and exits `0`. Per scanner: osv-scanner is handed the changed lockfiles by path; zizmor the changed workflow files; cargo-deny each crate root whose `Cargo.toml` or `Cargo.lock` moved; cargo-vet only where a `Cargo.lock` moved and a store exists; guarddog pypi each directory whose @@ -2156,6 +2158,48 @@ rather than a widening: there is no tree to widen into, and reporting a clean scan of manifests that are not on disk is the shape this crate exists to refuse. +### CI configuration nothing here scans + +zizmor parses GitHub Actions and nothing else, so a pipeline defined for any +other vendor is read by no scanner here. Where a run finds one it says so, by +name: + +```text +== zizmor -- workflow security + .circleci/config.yml is CI configuration no scanner here reads -- a declared gap, not a scanner that failed + 1 workflow directories +``` + +**The asymmetry is the tooling's, not a preference.** Actions is not the CI +system uphold favours; it is the one somebody wrote a scanner for, and a job +that mints a token, pulls an unpinned action or orb, or runs a command over +untrusted input is the same defect whichever vendor's file it lives in. The +rest are enumerated by name because their file names are what identifies them: +anything under a `.circleci/` directory at any depth, and `.gitlab-ci.yml`, +`.gitlab-ci.yaml`, `azure-pipelines.yml` or `Jenkinsfile`. A list rather than +`*.yml`, which at a repository root is a config file for anything. + +**A declaration, not a verdict.** Neither count moves and the exit code does +not change: nothing failed, and nothing was prevented from looking. What the +line refuses is the silence — five green sections over an unread pipeline read +like six. The paths are in the changed set for the same reason, so a push +carrying only a `.gitlab-ci.yml` is told the file went unscanned rather than +that there was nothing here. + +**Why declared rather than filled.** [checkov][checkov] is the one scanner +found that reads a CircleCI config, and it fails open: a YAML parse error is +logged at debug level, the parser returns no model, and the run exits `0` with +`"parsing_errors": 0`, so a config it could not read is indistinguishable from +a clean one — this command's third verdict imported as a silent pass. Its +CircleCI check set is thin besides (nine checks, one testing for a misspelled +`@volitile` orb tag since 2022), and no scanner surveyed works from a +normalised pipeline model, so there is no vendor-neutral tool to reach for +instead. It is named here, not recommended. Should a scanner appear that reads +one of these vendors the way zizmor reads Actions, the change is +`interesting()` and a sixth entry in the section array. + +[checkov]: https://github.com/bridgecrewio/checkov + ### guarddog, which cannot answer in its exit code at all `guarddog verify` **exits 0 whether it found three high-severity risks or diff --git a/src/supply.rs b/src/supply.rs index d6136fe..05d3a8e 100644 --- a/src/supply.rs +++ b/src/supply.rs @@ -50,6 +50,19 @@ //! that a question went unasked, which is this command's third verdict and the //! reason it exists. //! +//! WHAT NOTHING HERE READS IS SAID OUT LOUD TOO. zizmor parses GitHub Actions +//! and nothing else, so a pipeline defined for any other vendor is read by no +//! scanner here -- an asymmetry of tooling, not preference: Actions is the CI +//! system somebody wrote a scanner for, and the defects are the same file to +//! file. The run prints those files rather than leaving the gap implicit in a +//! section list nobody enumerates. +//! +//! IT IS DECLARED RATHER THAN FILLED because the only candidate fails open. +//! checkov, the one scanner found that reads a CircleCI config, logs a YAML +//! parse error at debug level and exits 0, so a config it could not read comes +//! back indistinguishable from a clean one -- this command's third verdict +//! imported as a silent pass. Named here, not run. +//! //! What the run looks at is a RANGE, not a tree. Every scanner here reaches the //! network, and a push that changes no lockfile, manifest or workflow was //! paying for all five: the whole-tree form is now `--all`, and the pre-push @@ -73,6 +86,22 @@ const ZIZMOR_DEFAULT: &str = include_str!("../policy/zizmor.default.yml"); /// backlog. const PRUNE: [&str; 5] = ["target", "node_modules", ".git", "vendor", "upstream"]; +/// CI configuration no scanner here reads, by directory -- the whole of one +/// is pipeline definition. Named so the run can say the files went unscanned; +/// there is no scanner to point at them. +const UNSCANNED_CI_DIRS: [&str; 1] = [".circleci"]; + +/// The same by file name, for the vendors that put one pipeline in one file. +/// +/// A list, not a pattern: `*.yml` at a repository root is a config file for +/// anything, and the declaration on files no CI runner reads is noise. +const UNSCANNED_CI_FILES: [&str; 4] = [ + ".gitlab-ci.yml", + ".gitlab-ci.yaml", + "azure-pipelines.yml", + "Jenkinsfile", +]; + /// What one section established. enum Section { /// Ran and found nothing to refuse. @@ -265,17 +294,22 @@ fn find_named(root: &Path, name: &str) -> Result> { Ok(found) } -/// Is this a path one of the five scanners would read? +/// Is this a path this run has anything to say about? /// -/// The two halves are the whole filter: a manifest or lock by name at any -/// depth, and any file under a `.github/workflows` directory. A path that is -/// neither changes nothing any scanner here would answer differently. +/// Two halves are what a scanner would read: a manifest or lock by name at any +/// depth, and any file under a `.github/workflows` directory. The third is the +/// opposite -- CI configuration nothing reads -- and it is here BECAUSE +/// nothing reads it: a push changing only a `.gitlab-ci.yml` would otherwise +/// leave an empty range, and be told there was nothing here a scanner reads +/// over the one file class whose being unread is worth saying out loud. +/// Nothing downstream is handed it: every section selects its own inputs by +/// name or by `is_workflow`. fn interesting(path: &Path) -> bool { let named = path .file_name() .and_then(|name| name.to_str()) .is_some_and(|name| MANIFEST_NAMES.contains(&name)); - named || is_workflow(path) + named || is_workflow(path) || is_unscanned_ci(path) } /// A file inside a `.github/workflows` directory, at any depth. @@ -286,6 +320,85 @@ fn is_workflow(path: &Path) -> bool { }) } +/// A CI configuration file no scanner in this set reads. +/// +/// Mirrors `is_workflow`: anything under a listed directory at any depth, and +/// a listed name at any depth. The directory itself is not one -- a directory +/// is not a file anybody could have scanned. +fn is_unscanned_ci(path: &Path) -> bool { + let parts: Vec<&std::ffi::OsStr> = path.iter().collect(); + let Some((last, ancestors)) = parts.split_last() else { + return false; + }; + let named = last + .to_str() + .is_some_and(|name| UNSCANNED_CI_FILES.contains(&name)); + let under = ancestors.iter().any(|part| { + part.to_str() + .is_some_and(|name| UNSCANNED_CI_DIRS.contains(&name)) + }); + named || under +} + +/// Every unscanned CI file in scope, as root-relative paths. +/// +/// The walk poisons on an unreadable directory like every other enumeration +/// here: a declaration naming two of three files understates the gap. +fn unscanned_ci(root: &Path, scope: &Scope) -> Result> { + if let Scope::Changed(paths) = scope { + return Ok(paths + .iter() + .filter(|path| is_unscanned_ci(path)) + .cloned() + .collect()); + } + let mut found = Vec::new(); + let walk = ignore::WalkBuilder::new(root) + .standard_filters(false) + .filter_entry(|entry| { + entry + .file_name() + .to_str() + .is_none_or(|file| !PRUNE.contains(&file)) + }) + .build(); + for entry in walk { + let entry = entry.map_err(|error| { + Fatal::new(format!( + "could not enumerate the tree looking for CI configuration: {error}. A run \ + that says which files went unscanned must not miss one" + )) + })?; + if !entry.file_type().is_some_and(|kind| kind.is_file()) { + continue; + } + let Ok(relative) = entry.path().strip_prefix(root) else { + continue; + }; + if is_unscanned_ci(relative) { + found.push(relative.to_path_buf()); + } + } + found.sort(); + Ok(found) +} + +/// Say which CI configuration nobody looked at, before zizmor says what it did. +/// +/// A statement, not a verdict: neither count in `run` moves, because nothing +/// failed and nothing was prevented from looking. What it refuses is the +/// silence -- five green sections over an unread pipeline read like six. +fn declare_unscanned_ci(root: &Path, scope: &Scope) -> Result<()> { + for path in unscanned_ci(root, scope)? { + println!( + " {} is CI configuration no scanner here reads -- a declared gap, \ + not a scanner that failed", + path.display() + ); + } + Ok(()) +} + /// The scope of one or more ranges, submodule pointers expanded. /// /// A range whose start is the all-zero id is a branch the remote does not have, @@ -531,6 +644,10 @@ fn osv(root: &Path, scope: &Scope) -> Result
{ } fn zizmor(root: &Path, scope: &Scope) -> Result
{ + // This section's edge is the gap: what zizmor parses bounds what the whole + // set covers. Before its early returns, so a missing zizmor and a tree + // with no Actions workflow are still told. + declare_unscanned_ci(root, scope)?; let (workflows, unit) = match scope { Scope::Whole => (find_workflow_dirs(root)?, "workflow directories"), // The changed files themselves, not their directory: zizmor reports per @@ -1138,7 +1255,7 @@ mod tempfile_guard { #[cfg(test)] mod tests { - use super::{find_named, find_workflow_dirs, PRUNE}; + use super::{find_named, find_workflow_dirs, interesting, unscanned_ci, Scope, PRUNE}; /// Paths under the fixture, as strings, so a failure names what was found. fn relative(root: &std::path::Path, found: &[std::path::PathBuf]) -> Vec { @@ -1198,13 +1315,72 @@ mod tests { ); } + /// CI configuration nothing scans is in scope, BECAUSE nothing scans it. + /// + /// Read as a list of what the five tools open, a pipeline definition + /// belongs out of it. It is in for what the run prints when the range is + /// empty: "nothing in this range that a scanner reads" is true of that + /// file in a way the sentence does not mean and the reader would not hear. + #[test] + fn ci_configuration_no_scanner_reads_is_in_scope_so_the_run_can_say_so() { + use std::path::Path; + + for path in [ + ".circleci/config.yml", + ".circleci/scripts/deploy.sh", + "sub/.circleci/config.yml", + ".gitlab-ci.yml", + "Jenkinsfile", + ] { + assert!(interesting(Path::new(path)), "{path}"); + } + // A config file at a root is a config file for anything. Calling every + // one of them unscanned CI would put the declaration on files no CI + // runner ever reads, and a declaration nobody believes is noise. + for path in ["deny.toml", "config.yml", "docs/config.yml", ".circleci"] { + assert!(!interesting(Path::new(path)), "{path}"); + } + } + + /// The declaration names every unscanned file, at any depth, and no + /// vendored one. + /// + /// A missed submodule is a pipeline the run said nothing about, which is + /// the silence the declaration exists to break; a vendored copy is a file + /// nobody in this tree could scan even if a scanner existed. + #[test] + fn unscanned_ci_configuration_is_found_at_any_depth_and_not_inside_a_pruned_tree() { + let root = crate::fixture::scratch("supply-unscanned-ci"); + for directory in [".circleci", "sub/.circleci", "vendor/.circleci"] { + std::fs::create_dir_all(root.join(directory)).unwrap(); + std::fs::write(root.join(directory).join("config.yml"), "jobs:\n").unwrap(); + } + std::fs::write(root.join(".gitlab-ci.yml"), "stages:\n").unwrap(); + std::fs::create_dir_all(root.join(".github/workflows")).unwrap(); + std::fs::write(root.join(".github/workflows/ci.yml"), "on: push\n").unwrap(); + + let found = unscanned_ci(&root, &Scope::Whole).unwrap(); + assert_eq!( + found + .iter() + .map(|path| path.display().to_string()) + .collect::>(), + [ + ".circleci/config.yml", + ".gitlab-ci.yml", + "sub/.circleci/config.yml" + ] + ); + } + /// A directory that cannot be read poisons the enumeration. /// /// The failure this refuses is the one the whole crate exists to refuse: an /// unreadable directory that merely SHRINKS the result gives a scan that /// looked at part of the tree and reported on all of it -- a clean run over /// manifests nobody read. It has to be an error, so the section is COULD - /// NOT LOOK and the run exits 2. + /// NOT LOOK and the run exits 2. The unscanned-CI walk is held to the same + /// rule: a declaration that missed a file understates the gap. #[cfg(unix)] #[test] fn an_unreadable_directory_fails_the_enumeration_rather_than_shrinking_it() { @@ -1223,6 +1399,7 @@ mod tests { let unreadable = std::fs::read_dir(&locked).is_err(); let named = find_named(&root, "Cargo.toml"); let workflows = find_workflow_dirs(&root); + let ci = unscanned_ci(&root, &Scope::Whole); std::fs::set_permissions(&locked, std::fs::Permissions::from_mode(0o755)).unwrap(); if !unreadable { @@ -1239,5 +1416,11 @@ mod tests { workflows.contains("could not enumerate workflow directories"), "{workflows}" ); + let ci = ci.unwrap_err().to_string(); + assert!( + ci.contains("could not enumerate the tree looking for CI configuration"), + "{ci}" + ); + assert!(ci.contains("must not miss one"), "{ci}"); } } diff --git a/tests/supply_chain_cli.rs b/tests/supply_chain_cli.rs index 427596e..366cca9 100644 --- a/tests/supply_chain_cli.rs +++ b/tests/supply_chain_cli.rs @@ -790,6 +790,71 @@ fn a_changed_workflow_is_handed_to_zizmor_by_file_and_its_neighbours_are_not() { ); } +/// A push carrying only a pipeline definition is told the file went unscanned. +/// +/// What this replaces is "nothing in this range that a scanner reads" -- +/// literally true, heard as "nothing here needed scanning". A job that mints a +/// token or pulls an unpinned orb is the surface zizmor exists for, in a file +/// zizmor cannot parse. +#[test] +fn a_range_holding_only_a_ci_config_says_the_file_is_unscanned_not_that_there_was_nothing() { + let root = tracked(); + let before = head(&root); + write(&root, ".circleci/config.yml", "version: 2.1\njobs: {}\n"); + let after = commit(&root, "a pipeline no scanner here reads"); + let tools = stubs(&[ + ("osv-scanner", &recording("exit 0")), + ("zizmor", &recording("exit 0")), + ("guarddog", &recording(GUARDDOG_CLEAN)), + ("cargo", &recording("exit 0")), + ]); + let output = pushed(&root, &tools, &before, &after); + // A declaration, not a verdict: neither count moves, so this exits clean. + assert_eq!(code(&output), 0, "{}", text(&output)); + assert!( + text(&output).contains(".circleci/config.yml is CI configuration no scanner here reads"), + "{}", + text(&output) + ); + assert!( + !text(&output).contains("nothing in this range"), + "{}", + text(&output) + ); + // And no scanner was handed it: the declaration says the file is unread. + assert!(journal(&root).is_empty(), "{}", journal(&root)); +} + +/// The whole-tree form declares it too, beside the workflows it did scan. +/// +/// `--all` is the sweep a reader trusts to have seen everything, so it is the +/// run where five green sections over an unscanned pipeline mislead most. +/// +/// A DIFFERENT VENDOR FROM THE TEST ABOVE, deliberately: the class is every CI +/// system no scanner reads, and two tests over one vendor would leave every +/// other name in the list resting on the unit test alone. +#[test] +fn a_whole_tree_sweep_declares_the_ci_configuration_it_did_not_scan() { + let root = tracked(); + write(&root, ".github/workflows/ci.yml", "on: push\n"); + write(&root, ".gitlab-ci.yml", "stages:\n - build\n"); + let _ = commit(&root, "two CI vendors, one scanner between them"); + let tools = stubs(&[ + ("osv-scanner", &recording("exit 0")), + ("zizmor", &recording("exit 0")), + ]); + let output = supply(&root, Some(&tools)); + assert_eq!(code(&output), 0, "{}", text(&output)); + assert!( + text(&output).contains(".gitlab-ci.yml is CI configuration no scanner here reads"), + "{}", + text(&output) + ); + let ran = journal(&root); + assert!(ran.contains("zizmor"), "{ran}"); + assert!(!ran.contains(".gitlab-ci.yml"), "{ran}"); +} + /// A member repository, cloned into the fixture as a real submodule. fn with_a_submodule(root: &Path) { let member = support::scratch("supply-chain-member");