From a611cc9563fa4ebdc5aa727eff56a2d0ae529652 Mon Sep 17 00:00:00 2001 From: HackingGate Date: Wed, 21 Oct 2026 15:53:18 +0900 Subject: [PATCH] Drop guarddog from uphold supply-chain uphold no longer runs guarddog. Its heuristic half (a malicious release published but not yet reported to OSV) is covered at install time by the package manager's release-age setting instead, and the scanner capped its own dependencies so that it would not install on a current Python. The first-party git-pin check that lived in the guarddog section stays, as a section of its own ("git pins") over the same manifests: uv export --frozen for each uv.lock, and each package.json's dependencies. Direct URLs, paths outside the tree and git sources under another owner are still refused by name. Dependencies it no longer needs are gone: the PyPI extras lookup, ureq and pyreq. GUARDDOG_NPM_INCLUDE_DEV_DEPENDENCIES is no longer read. A [[supply_chain.waive]] row naming guarddog used to be valid; it is still read and ignored, and the run names each one on stderr, so a policy written earlier keeps working until its owner deletes the row. It is a warning, not a failure. A row naming any other scanner is still refused at load, as before. Removed with the guarddog section (its report reader, waiver ledger and nested-policy waivers, the PyPI extras lookup, the unpack-directory handling) and their tests, each a function that only that section called: a_bin_prefix_removed_anywhere_but_the_start_is_still_a_finding accepts a_changed_python_lock_runs_guarddog_in_that_directory_and_not_the_others a_description_line_that_does_not_read_as_one_difference_is_still_a_finding a_first_party_git_source_whose_tag_resolves_is_not_sent_to_guarddog_and_passes a_first_party_npm_git_dependency_whose_tag_resolves_is_not_sent_to_guarddog_and_passes a_lock_out_of_step_with_its_manifest_is_exported_and_scanned_without_a_write a_manifest_loses_its_git_dependencies_to_the_lock_and_keeps_the_rest a_manifest_with_only_dev_dependencies_is_not_sent_to_guarddog_and_passes a_manifest_with_only_dev_dependencies_leaves_nothing_for_npm a_mismatch_that_is_only_npms_publish_rewrite_is_tolerated_and_named a_nested_policy_that_will_not_load_is_an_error_and_never_no_waivers a_nested_policy_that_will_not_load_is_not_checked_rather_than_no_waivers a_nested_repositorys_own_waiver_covers_its_manifests_and_names_its_policy a_nested_repositorys_policy_is_found_under_every_name_uphold_loads a_nested_repositorys_unmatched_waiver_is_reported_by_its_own_scan_alone a_nested_repositorys_waiver_never_covers_the_superprojects_or_a_siblings_manifests a_nested_repositorys_waivers_reach_its_own_tree_and_nothing_outside_it an_export_is_sorted_into_index_tree_git_and_refused an_export_with_nothing_from_an_index_does_not_ask_guarddog an_inner_repositorys_waiver_never_covers_the_repository_around_it an_npm_manifest_is_scanned_where_it_lives_and_a_clean_one_is_a_pass a_normalization_beside_any_other_difference_is_still_a_finding an_outer_repositorys_row_that_repeats_an_inner_one_is_named_as_a_removable_copy an_outer_repositorys_waiver_covers_a_manifest_in_a_repository_inside_it an_unmatched_pypi_metadata_mismatch_waiver_is_not_called_removable an_unparseable_manifest_is_still_sent_to_guarddog_and_is_not_checked a_package_lock_supplies_the_commit_and_a_registry_only_manifest_is_left_whole a_pushed_range_judges_a_moved_submodule_manifest_against_the_submodules_own_waiver a_python_lock_is_exported_for_guarddog_and_its_refusal_is_the_runs a_refusing_npm_scan_names_the_directory_it_refused_in a_root_row_that_repeats_a_nested_repositorys_own_is_named_as_a_removable_copy a_supply_chain_waiver_is_the_repositorys_own_and_is_written_once a_waived_guarddog_finding_is_printed_and_not_held_against_the_run a_waiver_for_another_version_does_not_apply_and_is_reported_stale a_waiver_without_a_reason_or_a_version_is_refused_at_load cache_of clean_dependencies confirms covers depending_on_left_pad describing dev_dependencies_are_sent_to_guarddog_where_it_is_told_to_read_them dev_dependencies_count_and_lose_their_git_sources_where_guarddog_reads_them differences every_enclosing_repository_with_a_policy_contributes_innermost_first every_occurrence_must_require_a_positive_extra evidence_must_name_the_scanned_release_and_every_difference fetch guarddog_coverage_normalises_pypi_names_and_ignores_local_requirements guarddog_coverage_uses_real_npm_alias_names guarddog_issues_without_risks_are_not_a_finding guarddog_must_report_every_submitted_npm_dependency guarddog_partial_pypi_reports_name_every_omitted_dependency guarddog_read guarddog_risks_survive_a_partial_report guarddog_rules_that_did_not_run_are_could_not_look_rather_than_a_clean_scan guarddog_that_could_not_scan_a_dependency_is_could_not_look guarddog_that_found_risks_and_exited_zero_is_a_finding_not_a_clean_scan guarddog_unpack_directory guarddog_unpacks_in_the_same_directory_whatever_tmpdir_says guarddog_unpacks_under guarddog_unpacks_under_the_cache_directory_and_never_under_tmpdir guarddog_with_an_incomplete_result_is_could_not_look in_order judged loader load_nested nested_repository nested_waivers npm_identity npm_tolerance_requires_the_result_and_risk_to_agree objected only_a_complete_bounded_json_response_can_be_evidence owners_of owning package_name parts pypi_extras_need_release_evidence_and_agreeing_reports pypi_metadata_mismatch_is_not_tolerated read_response refused_at release_identity_cannot_change_the_destination release_url report_unmatched response rules_that_did_not_run the_innermost_waiver_covers_first_and_every_outer_row_for_the_same_risk_repeats_it the_recorded_pandas_report_is_explained_by_its_release_metadata tolerated tolerated_metadata typosquat typosquat_waiver unknown_or_ambiguous_report_shapes_are_not_candidates verified_pypi_extras_leave_unrelated_risks_counted waiver waiving --- .pre-commit-hooks.yaml | 8 +- Cargo.lock | 228 +-- Cargo.toml | 5 - README.md | 4 +- deny.toml | 3 - docs/REFERENCE.md | 317 +--- src/config.rs | 144 +- src/main.rs | 33 +- src/supply.rs | 1629 +---------------- src/supply/pypi.rs | 293 --- src/supply/references.rs | 304 +-- .../guarddog/npm-react-dom-19.2.0.json | 1 - .../npm-swagger-ui-express-5.0.1.json | 1 - .../guarddog/pypi-pandas-3.0.3-metadata.json | 92 - .../fixtures/guarddog/pypi-pandas-3.0.3.json | 1 - tests/supply_chain_cli.rs | 1415 ++------------ 16 files changed, 391 insertions(+), 4087 deletions(-) delete mode 100644 src/supply/pypi.rs delete mode 100644 tests/fixtures/guarddog/npm-react-dom-19.2.0.json delete mode 100644 tests/fixtures/guarddog/npm-swagger-ui-express-5.0.1.json delete mode 100644 tests/fixtures/guarddog/pypi-pandas-3.0.3-metadata.json delete mode 100644 tests/fixtures/guarddog/pypi-pandas-3.0.3.json diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index dd49a19..35c5a4c 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -150,8 +150,8 @@ pass_filenames: false always_run: true -# Six external scanners, one verdict. The scanners are NOT built by this -# hook -- osv-scanner, zizmor, cargo-deny, cargo-vet, guarddog and gitleaks +# Five external scanners and a git-pin check, one verdict. The scanners are NOT built by this +# hook -- osv-scanner, zizmor, cargo-deny, cargo-vet and gitleaks # come from the host's own toolchain, and one that is missing is exit 2, never # a pass. gitleaks runs only where the policy inherits the `credentials` set, # and only at the version this uphold pins. @@ -169,7 +169,7 @@ # passes the files in the push, and the range is not one of them. - id: uphold-supply-chain name: uphold supply-chain - description: origin, advisories, typosquats, workflow security and committed secrets over the pushed range -- six scanners, one verdict + description: origin, advisories, workflow security, git pins and committed secrets over the pushed range -- five scanners and a git-pin check, one verdict entry: uphold supply-chain language: rust stages: [pre-push] @@ -189,7 +189,7 @@ # is the pair this was split into. - id: uphold-supply-chain-all name: uphold supply-chain --all - description: the same six scanners over every manifest in the tree and every commit, for CI and scheduled runs + description: the same checks over every manifest in the tree and every commit, for CI and scheduled runs entry: uphold supply-chain args: ["--all"] language: rust diff --git a/Cargo.lock b/Cargo.lock index c097399..876fc48 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -69,12 +69,6 @@ version = "3.20.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72f5acc6cb2ba439de613abc23857ec3d78374d8ed5ac84e9d11336e87da8649" -[[package]] -name = "bytes" -version = "1.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc652a48c352aef3ea3aed32080501cf3ef6ed5da78602a020c991775b0aff04" - [[package]] name = "cc" version = "1.4.4" @@ -167,7 +161,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys", ] [[package]] @@ -277,22 +271,6 @@ version = "0.17.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" -[[package]] -name = "http" -version = "1.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "918d3568bebf352712bc2ef3d46a8bcf1a75b373be6539de198e9105cbbf9ce0" -dependencies = [ - "bytes", - "itoa", -] - -[[package]] -name = "httparse" -version = "1.10.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6dbf3de79e51f3d586ab4cb9d5c3e2c14aa28ed23d180cf89b4df0454a69cc87" - [[package]] name = "ignore" version = "0.4.33" @@ -405,12 +383,6 @@ version = "1.21.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" -[[package]] -name = "percent-encoding" -version = "2.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" - [[package]] name = "phf" version = "0.11.3" @@ -473,15 +445,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "pyreq" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c6eb3ecbf06d7c20d3e767683ce199979ed7bfdb5ae421f0b6dcb9f391d3462f" -dependencies = [ - "regex", -] - [[package]] name = "quick-junit" version = "0.8.0" @@ -574,20 +537,6 @@ version = "0.8.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" -[[package]] -name = "ring" -version = "0.17.14" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4689e6c2294d81e88dc6261c768b63bc4fcdb852be6d1352498b114f61383b7" -dependencies = [ - "cc", - "cfg-if", - "getrandom", - "libc", - "untrusted", - "windows-sys 0.52.0", -] - [[package]] name = "rustix" version = "1.1.5" @@ -598,42 +547,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.61.2", -] - -[[package]] -name = "rustls" -version = "0.23.46" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "48e13bd8c0e9365c43cfa5c9e8f9ad49d3c8444926c9aac819e0e4dc503c8fdf" -dependencies = [ - "log", - "once_cell", - "ring", - "rustls-pki-types", - "rustls-webpki", - "subtle", - "zeroize", -] - -[[package]] -name = "rustls-pki-types" -version = "1.15.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f4925028c7eb5d1fcdaf196971378ed9d2c1c4efc7dc5d011256f76c99c0a96" -dependencies = [ - "zeroize", -] - -[[package]] -name = "rustls-webpki" -version = "0.103.16" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4443b1b6d39d7c663139942cc4558262562d2c84123bb6282ef177009ca8b281" -dependencies = [ - "ring", - "rustls-pki-types", - "untrusted", + "windows-sys", ] [[package]] @@ -778,12 +692,6 @@ dependencies = [ "vte", ] -[[package]] -name = "subtle" -version = "2.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" - [[package]] name = "syn" version = "2.0.119" @@ -977,12 +885,6 @@ dependencies = [ "rand", ] -[[package]] -name = "untrusted" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ecb6da28b8a351d773b68d5825ac39017e680750f980f3a1a85cd8dd28a47c1" - [[package]] name = "uphold" version = "1.34.3" @@ -994,7 +896,6 @@ dependencies = [ "grep-regex", "grep-searcher", "ignore", - "pyreq", "quick-junit", "regex", "rustix", @@ -1010,44 +911,9 @@ dependencies = [ "unicode-script", "unicode-security", "unicode_names2", - "ureq", "uuid", ] -[[package]] -name = "ureq" -version = "3.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a7ac20be9b7726e0bbdbf974c059676d9acb1cd414961f570a4e8231cacd7fc" -dependencies = [ - "base64", - "log", - "percent-encoding", - "rustls", - "rustls-pki-types", - "ureq-proto", - "utf8-zero", - "webpki-roots", -] - -[[package]] -name = "ureq-proto" -version = "0.6.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f86fd172ccca569e458f61b6bdd6220965a9ef36e672a6852953b51a0e1583be" -dependencies = [ - "base64", - "http", - "httparse", - "log", -] - -[[package]] -name = "utf8-zero" -version = "0.8.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b8c0a043c9540bae7c578c88f91dda8bd82e59ae27c21baca69c8b191aaf5a6e" - [[package]] name = "uuid" version = "1.28.0" @@ -1128,22 +994,13 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "webpki-roots" -version = "1.0.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1c1c8e20c8a38fc7d65abfe74eb7280e4aa667e8eec8bc84996d2874810748d9" -dependencies = [ - "rustls-pki-types", -] - [[package]] name = "winapi-util" version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.2", + "windows-sys", ] [[package]] @@ -1152,15 +1009,6 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" -[[package]] -name = "windows-sys" -version = "0.52.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" -dependencies = [ - "windows-targets", -] - [[package]] name = "windows-sys" version = "0.61.2" @@ -1170,70 +1018,6 @@ dependencies = [ "windows-link", ] -[[package]] -name = "windows-targets" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" -dependencies = [ - "windows_aarch64_gnullvm", - "windows_aarch64_msvc", - "windows_i686_gnu", - "windows_i686_gnullvm", - "windows_i686_msvc", - "windows_x86_64_gnu", - "windows_x86_64_gnullvm", - "windows_x86_64_msvc", -] - -[[package]] -name = "windows_aarch64_gnullvm" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" - -[[package]] -name = "windows_aarch64_msvc" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" - -[[package]] -name = "windows_i686_gnu" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" - -[[package]] -name = "windows_i686_gnullvm" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" - -[[package]] -name = "windows_i686_msvc" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" - -[[package]] -name = "windows_x86_64_gnu" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" - -[[package]] -name = "windows_x86_64_gnullvm" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" - -[[package]] -name = "windows_x86_64_msvc" -version = "0.52.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" - [[package]] name = "winnow" version = "1.0.4" @@ -1260,12 +1044,6 @@ dependencies = [ "syn 2.0.119", ] -[[package]] -name = "zeroize" -version = "1.9.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e13084392c5e4bc371903e2935a5eaeed24905a7511356b883835e18a78f6879" - [[package]] name = "zmij" version = "1.0.23" diff --git a/Cargo.toml b/Cargo.toml index a5a8aeb..fe54ccb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -112,11 +112,6 @@ tree-sitter-go = "0.25" # positional word after them. Where this reader and git's `split_cmdline` # disagree, `expand_alias` says which case and how it is closed. shell-words = "1" -# The exact-release PyPI metadata used to verify optional extras, with bounded -# HTTPS requests and no asynchronous runtime. -ureq = { version = "3.4.2", default-features = false, features = ["rustls"] } -# Parse PEP 508 requirements before accepting the closed optional-extra form. -pyreq = "0.2.0" # JUnit XML, for `UPHOLD_JUNIT`: the evidence format the gate tier already # writes, publishes and links, so a guard run is read by the same publisher with # nothing of uphold's own to parse. A library rather than a hand-rolled writer diff --git a/README.md b/README.md index 23d71db..ebfe8cd 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,7 @@ Three scanner ids are left out above because they need tools on the host: diff. Each scanner must be at least the release its output reader was measured -against ([floors](docs/REFERENCE.md#uphold-supply-chain--six-scanners-one-verdict)); +against ([floors](docs/REFERENCE.md#uphold-supply-chain--four-scanners-one-check-of-its-own-one-verdict)); an older one is exit 2 for its section, so an uphold upgrade that raises a floor can newly refuse a push on a host with an old scanner. @@ -248,7 +248,7 @@ uphold shim --status # what is linked, and whether PATH reaches it uphold hook claude-code # judge a pending agent tool call, read on stdin uphold hook codex # the same checks through Codex's PreToolUse hook uphold audit --for-publication # before flipping private -> public -uphold supply-chain # six scanners over the pushed range; a missing one is exit 2 +uphold supply-chain # scanners and the git-pin check over the pushed range; a missing one is exit 2 uphold supply-chain --all # the same over every manifest and every commit uphold hooks --identity ../a ../b # do these repositories declare the same hooks diff --git a/deny.toml b/deny.toml index 135dddf..a49c4f2 100644 --- a/deny.toml +++ b/deny.toml @@ -41,9 +41,6 @@ allow = [ "Apache-2.0 WITH LLVM-exception", "BSD-2-Clause", "BSD-3-Clause", - # TLS implementation and the Mozilla root certificate data carried by ureq. - "ISC", - "CDLA-Permissive-2.0", "MIT", "Unicode-3.0", "Unicode-DFS-2016", diff --git a/docs/REFERENCE.md b/docs/REFERENCE.md index 703ee29..4e3b7a5 100644 --- a/docs/REFERENCE.md +++ b/docs/REFERENCE.md @@ -12,7 +12,7 @@ namespace — that is what lets a claim in - [When the policy itself will not load](#when-the-policy-itself-will-not-load) - [`uphold hook` — the caller that spawns no process](#uphold-hook--the-caller-that-spawns-no-process) - [`uphold audit --for-publication`](#uphold-audit---for-publication) -- [`uphold supply-chain` — six scanners, one verdict](#uphold-supply-chain--six-scanners-one-verdict) +- [`uphold supply-chain` — six scanners, one verdict](#uphold-supply-chain--four-scanners-one-check-of-its-own-one-verdict) - [`uphold hooks --identity` — across repositories](#uphold-hooks---identity--across-repositories) - [`uphold hooks --install` — the hooks git actually runs](#uphold-hooks---install--the-hooks-git-actually-runs) - [`uphold probe` — can each hook refuse?](#uphold-probe--can-each-hook-refuse) @@ -3322,7 +3322,7 @@ found, `2` where a surface this run tried to read could not be read, `0` when every surface a flip would republish was read and was clean — subject to the standing caveats, which the clean line says. -## `uphold supply-chain` — six scanners, one verdict +## `uphold supply-chain` — four scanners, one check of its own, one verdict ```sh uphold supply-chain # what the pushed range changed @@ -3330,22 +3330,30 @@ uphold supply-chain --base REV # what REV..HEAD changed uphold supply-chain --all # every manifest in the tree ``` -Runs five external scanners: **osv-scanner** (known +Runs four external scanners: **osv-scanner** (known vulnerabilities and reported-malicious packages), **zizmor** (workflow -security), **cargo-deny** (origin, advisories, bans, licenses), **cargo-vet** -(has anyone looked at this dependency) and **guarddog** (publisher identity -and typosquats — the half OSV cannot reach, scoring an *unknown* package on -how closely its name shadows a popular one), plus gitleaks -([below](#gitleaks-which-owns-secret-shapes)). +security), **cargo-deny** (origin, advisories, bans, licenses) and +**cargo-vet** (has anyone looked at this dependency); one check of its own, +the **git pins** ([below](#git-pins-git-sources-urls-and-paths)); and +gitleaks ([below](#gitleaks-which-owns-secret-shapes)). + +**There is no heuristic malware scanner.** Earlier releases ran guarddog for +the window between a malicious release being published and OSV learning of it. +uphold no longer does. That window is closed at install time instead, by the +package manager's own release-age setting, which refuses a version younger than +a chosen number of days (bun's `minimumReleaseAge`, uv's `exclude-newer` with a +relative duration, Dependabot's `cooldown`). A `[[supply_chain.waive]]` row +naming guarddog is read and ignored: the run names each such row on stderr and +is not failed by it, so a policy written earlier keeps working until the row is +deleted. The scanners come from the host's own toolchain; none is built or fetched here. A tool that is not on PATH is **could not look**: reported by name, exit `2` through the same verdict ranking every other command here uses, and never a refusal or a pass. -Each of the five also has a **version floor**. Several sections read a -scanner's own output to learn that it did not look — guarddog's timed-out -rules, cargo-vet's 255 with an empty stdout, cargo-deny's exit 1 over an +Each of the four also has a **version floor**. Several sections read a +scanner's own output to learn that it did not look — cargo-vet's 255 with an empty stdout, cargo-deny's exit 1 over an unfetched database, zizmor skipping an unparseable workflow — and each reader was measured against one release. Before a scanner runs it is asked its version (`--version`; `cargo deny --version` and `cargo vet --version` for the @@ -3358,7 +3366,6 @@ compared with the floor: | zizmor | `1.30.0` | | cargo-deny | `0.20.2` | | cargo-vet | `0.10.2` | -| guarddog | `3.2.0` | A release below its floor is **could not look** for that section, exit `2`, naming the tool, the version found and the floor. So is a version command that @@ -3379,10 +3386,9 @@ are never descended into, because vendored and third-party manifests are out of scope. cargo-deny runs once per crate or workspace root against the root `deny.toml` (no `deny.toml`, and the section says so and stands down); cargo-vet runs only where a `supply-chain/` store exists, because a store -created automatically is a store nobody owns; guarddog reads each `uv.lock` -through `uv export`, less the direct references it cannot look up (see -[below](#what-guarddog-is-not-handed-git-sources-urls-and-paths)), and each -`package.json` directly, metadata rules only. A +created automatically is a store nobody owns; the git pins check reads each +`uv.lock` through `uv export` and each `package.json` directly (see +[below](#git-pins-git-sources-urls-and-paths)). A section with nothing to read says so — "no workflows here" and "checked and clean" must never look the same. @@ -3392,7 +3398,7 @@ missing or out of step with its manifest is the tool's refusal rather than a file resolved and written for the scan. `uv export` runs with `--frozen`: it reads a `uv.lock` as it is and writes nothing, and whether the lock is in step with its `pyproject.toml` is uv's question (`uv lock --check`), not the scan's, -which asks only whether the pinned packages are malicious. A crate root with no +which reads only the pinned set. A crate root with no `Cargo.lock` beside it is asked of the locks that are here before cargo-deny is pointed at it, by `cargo metadata --locked` of each crate that has one, nearest enclosing first: @@ -3424,8 +3430,8 @@ Each scanner's own exit code decides; findings are shown, never re-judged. The one filter applied is cargo-deny's headline lines, dropping the four classes that describe `deny.toml` rather than a dependency. -Three hook ids ship it in `.pre-commit-hooks.yaml`. The five dependency -scanners never run at `pre-commit`, because each reaches the network. +Three hook ids ship it in `.pre-commit-hooks.yaml`. The dependency +sections never run at `pre-commit`, because each reaches the network. `uphold-supply-chain` at `pre-push` scans the range; `uphold-supply-chain-all` at `manual` carries `--all`, since one entry cannot vary its arguments by stage. Pin the first for the push gate and the second for a schedule — only @@ -3454,8 +3460,7 @@ stage weekly, where a new advisory against an untouched dependency is found. "aqua:zizmorcore/zizmor" = "latest" "aqua:EmbarkStudios/cargo-deny" = "latest" "cargo:cargo-vet" = "latest" # only where a supply-chain/ store exists -"pipx:guarddog" = "latest" -"aqua:astral-sh/uv" = "latest" # guarddog reads `uv export`, not uv.lock +"aqua:astral-sh/uv" = "latest" # the git pins check reads `uv export`, not uv.lock "aqua:gitleaks/gitleaks" = "8.30.1" # only where the policy inherits credentials ``` @@ -3482,7 +3487,7 @@ jobs: extra-args: --all-files --hook-stage manual ``` -`latest` and not a version for the five dependency scanners: each floor above +`latest` and not a version for the four dependency scanners: each floor above is the oldest release whose output the readers here were measured against, and a newer one satisfies it. gitleaks is the exception, pinned for the reason given [below](#gitleaks-which-owns-secret-shapes). A lefthook consumer writes the same @@ -3542,7 +3547,7 @@ non-zero code is could-not-look (exit `2`). Findings print with `--redact`. for the `pre-commit` id `uphold-supply-chain-staged`. The range scan sees a secret at `pre-push`, after it is in local history and can only be rewritten out; the staged scan sees it while unstaging is the fix. gitleaks reads no -network, so the cost that keeps the other five scanners off the commit is not +network, so the cost that keeps the other sections off the commit is not paid here, and they are not run. The same gate applies: only where the policy inherits `credentials`, the same pin, the same config and exit codes, and a missing gitleaks is exit `2`. `--staged` with `--all` or `--base` is a usage error rather than one flag winning. A staged finding has no @@ -3566,15 +3571,15 @@ The changed set is filtered to `Cargo.toml`, `Cargo.lock`, `uv.lock`, 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 none of the first five scanners and says so +A range holding none of them runs none of the first five sections and says so in one line; where gitleaks is not asked for either, that is the whole run and it 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, and the crate whose lock pins a moved path dependency; cargo-vet only where a -`Cargo.lock` moved and a store exists; guarddog pypi each directory with a +`Cargo.lock` moved and a store exists; the git pins check each directory with a `uv.lock` whose `uv.lock` or `pyproject.toml` moved (one with no `uv.lock` is -named and left out, as the whole-tree walk leaves it out) and guarddog npm each changed +named and left out, as the whole-tree walk leaves it out) and each changed `package.json`; gitleaks each pushed range as `--log-opts=FROM..TO`, whatever files it touched. @@ -3628,76 +3633,19 @@ these vendors the way zizmor reads Actions would be added through [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 -none**, in both ecosystems, so its exit code cannot carry a verdict. - -`--exit-non-zero-on-finding` is not the remedy. It counts `issues`, which -includes capability matches: `six` reports `issues: 2` with `risks: []` and -guarddog's own label `no_risks_detected`, so the flag fails a package guarddog -itself calls clean. It trades a false negative for a false positive. - -So guarddog is run with `--output-format json` and its own `risks` list is -counted. This is the one scanner whose findings are read here. The count is -reported, never recomputed, with one named exception: a `metadata_mismatch` -risk made up only of known metadata normalizations is printed as -tolerated rather than counted -([below](#what-npm-writes-at-publish-is-tolerated-by-name)). - -The same report carries the could-not-look. A dependency guarddog could not -download populates `errors` and drops `results` while still exiting 0, and a -total network failure returns a bare `[]`; both are unread rather than clean. -A report also needs a `results` object and a `risks` array; missing or mistyped -fields are unread, never an empty risk list. npm tolerance requires the -underlying `results.metadata_mismatch` and the risk's `threat_description` to -agree exactly before their differences are accepted. -Each submitted registry dependency must appear in the report: npm aliases are -resolved to their package names and PyPI names are normalized. Missing names -are reported as unread even when other entries are clean. A reported risk -still refuses the run, with any missing coverage printed alongside it. -Its older admission, "Some rules failed to run while scanning \" at -exit 0 — the two email-domain rules time out routinely — is still read, and -still names the packages and how many rules. None of that is a finding: it is -the record that the question was asked and nobody answered. - -### Where guarddog unpacks - -guarddog is run with `TMPDIR` set to `$XDG_CACHE_HOME/uphold/guarddog`, else -`$HOME/.cache/uphold/guarddog`, created owner-only if missing. The caller's -`TMPDIR` is not passed on, because guarddog's `metadata_mismatch` verdict on -a PyPI wheel depends on the filesystem it unpacks on. - -guarddog 3.2.0 takes the first `os.listdir` entry under the unpacked release -whose name starts with the package's as the package root. A wheel such as -pandas 3.0.3 holds two such entries, `pandas/` (which carries a -`pyproject.toml`) and `pandas-3.0.3.dist-info/` (which carries none), and the -filesystem decides their order: tmpfs lists the newest first, btrfs in creation -order, ext4 by a hash seeded per filesystem. The order that reaches the -manifest reports a mismatch and the other reports nothing, so with `TMPDIR` on -tmpfs the same lock failed that passed with it on btrfs. - -Fixing the directory makes one host give one verdict. It does not make two -hosts agree when their cache directories are on different filesystems. That -needs guarddog to choose the package root deterministically, which nothing -outside its process can do. A relative or empty `XDG_CACHE_HOME` is ignored. -Where neither variable names an absolute directory, or the directory cannot -be created, guarddog inherits `TMPDIR` as before and the section prints a line -saying its verdict may differ with that filesystem. - -### What guarddog is not handed: git sources, URLs and paths - -`guarddog pypi verify` looks every requirement up on PyPI by name. A PEP 508 -direct reference -- `name @ git+https://...@`, which is how `uv export` -spells a uv git source, `name @ https://...`, or a path -- names nothing PyPI -holds, and guarddog answers it with a 404. So the export is sorted before -guarddog reads it: +### Git pins: git sources, URLs and paths + +A PEP 508 direct reference -- `name @ git+https://...@`, which is how +`uv export` spells a uv git source, `name @ https://...`, or a path -- names +nothing an index holds, so no registry and no scanner here can vouch for it. +This check reads each `uv.lock` through `uv export --frozen` and sorts the +export: | In the export | What happens | |---|---| -| `name==version` (an index) | handed to guarddog, as before | -| `-e ./member`, `./libs/x`, `name @ file://...` inside the repository | handed to guarddog, as before | -| `name @ git+@` on the first-party forge under the declared owner | asked of its remote with `git ls-remote`; not handed to guarddog | +| `name==version` (an index) | not this check's | +| `-e ./member`, `./libs/x`, `name @ file://...` inside the repository | the repository's own; nothing to check | +| `name @ git+@` on the first-party forge under the declared owner | asked of its remote with `git ls-remote` | | a git source under any other host or owner | refused by name, exit `1` | | `name @ https://...` or any other direct URL | refused by name, exit `1` | | a path that resolves outside the repository, through a link or not, or does not resolve | refused by name, exit `1` | @@ -3726,175 +3674,18 @@ variables removed; a remote it cannot answer for (no network, authentication refused, git not on PATH) is could-not-look, exit `2`, with git's own reason. An `owner_from` command that fails is could-not-look for the same reason. -An export with nothing left that an index resolves is not handed to guarddog -at all: guarddog handed an empty list answers `[]`, which this section reads as -a network failure. - -`guarddog npm verify` reads a `package.json`'s `dependencies` and asks npm for -each by name, so an npm git dependency is the same 404. Its git dependencies -(`git+#`, `git://...`, `github:`, `gitlab:` and `bitbucket:` -specifiers, and the `owner/repo` shorthand) are taken out and guarddog is -handed the manifest without them; a manifest with none is handed over as it -is. Each git dependency is held to the same first-party rule and the same -remote check. Its commit is the one `bun.lock` or `package-lock.json` records, -read from the manifest's directory and each one above it up to the -repository root. Its `#`, where it names one, must point at that commit -if it is a tag; if it is a branch, the commit must be what some ref points at, -as for a commit the lock names alone. A git dependency no lock records a -commit for, and whose `#` is not itself a commit, is refused by name: there is -no pin to check. Under `GUARDDOG_NPM_INCLUDE_DEV_DEPENDENCIES=true` -guarddog reads `devDependencies` as well, and they are sorted the same way. A -manifest with nothing npm resolves among what guarddog reads -- no -`dependencies`, only `devDependencies` without that setting, or only git -dependencies -- is not handed to guarddog, for the same `[]`; the section -says so and passes. A manifest that -does not parse is still handed over, for guarddog to say so. - -### What npm writes at publish is tolerated, by name - -guarddog's `metadata_mismatch` compares the registry's manifest for a release -with the `package.json` inside its tarball. `npm publish` normalizes the -manifest it uploads and leaves the tarball's file as written, so some -differences are written by npm, not by the package author. They recur on -every release, so a version-keyed waiver would need a new row each time. A -`metadata_mismatch` risk on an npm package whose every difference is on this -list is not counted: - -| Normalization | Registry | Tarball | npm's step | -|---|---|---|---| -| npm default start script | `scripts.start` = `node server.js` | no `scripts.start` | `serverjs`: a package with `server.js` at its root and no `start` | -| npm strips a leading node_modules/.bin/ | `scripts.` = `X` | `scripts.` = `./node_modules/.bin/X` or `node_modules/.bin/X` | `scriptpath`: one leading prefix removed | - -The list is `PUBLISH_NORMALIZATIONS` in `src/supply.rs`. A tolerated risk is -still printed, naming each normalization it accepted: - -``` - tolerated: metadata_mismatch on npm:react-dom@19.2.0 at web -- npm default start script (scripts.start = "node server.js") -``` - -The risk is counted as before, and a waiver still applies to it, when any of -these is true: - -- the description lists any other difference, such as a second script, a - `dependencies` or `devDependencies` entry, or `main`; -- a `start` has a value other than `node server.js`, or the tarball has its own - `start`; -- the `node_modules/.bin/` text appears anywhere but the start of the value; -- the description has a line that does not read as exactly one difference. - guarddog does not escape the values, so a line in which either the - `: Manifest("` or the `"), package.json("` marker appears more than once - is not read. - -`"install": "node-gyp rebuild"`, which npm adds where `binding.gyp` exists, -is not on the list. An install script runs at install time, so it is not the -same risk as `start`. - -guarddog's report does not list the tarball's files, so the `server.js` that -npm's step requires is not confirmed. guarddog also prints an absent value as -`None`, the same way it prints a literal `"None"`. - -### PyPI optional extras require release evidence - -guarddog compares every `requires_dist` name with the required dependencies -in `pyproject.toml`, including names declared only under optional extras. Its -report drops the markers, so uphold fetches the exact scanned release from -[PyPI's release JSON endpoint](https://docs.pypi.org/api/json/) only when the -entire difference consists of names present in PyPI metadata and absent from -`pyproject.toml`. It verifies the response's package and version, then parses -all `requires_dist` entries with `pyreq`. - -A name is tolerated only when every occurrence has a simple positive -`extra == "name"` marker (including the reversed equality). An occurrence -with no marker, a platform marker, a conjunction or an alternative keeps the -finding. This closed form covers the ordinary extras declarations without -evaluating conditions against the host. All reported names must qualify. Both copies of guarddog's -description must agree; malformed reports, unknown differences, missing -metadata or unparseable requirements retain the finding and remain eligible -for an exact-version waiver. - -The lookup uses HTTPS at `pypi.org`, refuses redirects, and is bounded to -15 seconds and 2 MiB per response. A failed lookup prints why the finding was -retained. A successful normalization prints `tolerated:` with the verified -names and release. No package-wide exemption or cross-run metadata cache is -created. This verifies registry metadata, not the artifact contents; PyPI -metadata can differ from the uploaded artifact. This does not repair guarddog's order-dependent choice of a wheel -root, so an unmatched PyPI metadata waiver is still not called removable. - -### Waiving a confirmed guarddog false positive - -Because the findings are read here, a finding somebody has looked at and judged -a false positive has to be recordable here too. Without that, one such finding -refuses every push that touches the lockfile, and the only levers are -`UPHOLD_ALLOW` per invocation or dropping guarddog. The waiver goes in the -repository's own policy: - -```toml -[[supply_chain.waive]] -scanner = "guarddog" -package = "pypi:pandas@3.0.6" # :@; pypi or npm -check = "metadata_mismatch" # the guarddog rule, as its report names it -reason = "compares optional extras against the required dependencies" -``` +The same rule applies to an npm `package.json`'s `dependencies`. Its git +dependencies (`git+#`, `git://...`, `github:`, `gitlab:` and +`bitbucket:` specifiers, and the `owner/repo` shorthand) are held to the same +first-party rule and the same remote check. Its commit is the one `bun.lock` or +`package-lock.json` records, read from the manifest's directory and each one +above it up to the repository root. Its `#`, where it names one, must +point at that commit if it is a tag; if it is a branch, the commit must be what +some ref points at, as for a commit the lock names alone. A git dependency no +lock records a commit for, and whose `#` is not itself a commit, is refused by +name: there is no pin to check. `devDependencies` are not read. -- **It is keyed on the version.** The next release is a different package with - its own reasons to be suspicious or not, so a bump is read fresh. For npm the - version is the one guarddog resolved, not the range in `package.json`. -- **A waived finding is still printed**, as `waived: metadata_mismatch on - pypi:pandas@3.0.6 at -- `. The verdict says what was not held - against the push; it does not hide it. -- **A waiver that matched nothing is reported**, where the run could have - matched it: the package appeared in a report, or `--all` read its ecosystem. - A range scan reads only the manifests that moved, so a waiver about one that - did not move is not called stale. A `pypi` waiver for `metadata_mismatch` is - reported without the advice to remove it, because whether that check reaches - the release's manifest depends on the host's filesystem - ([above](#where-guarddog-unpacks)); another host may still need it. -- **A nested repository's own waivers cover its own tree.** Scanned from a - superproject, a manifest inside a submodule, or inside any directory that is - a repository root with a policy, is judged against the root's waivers and the - `[[supply_chain.waive]]` rows of every enclosing repository with a policy, - between the root and the manifest. Each policy is the one `uphold` would - load if run in that directory: the same `policy/principles.toml` or - `policy/rg-policy.toml`, the same boundary at `.git`, the same loader with - its `[inherit]`. A repository's rows cover its whole tree, its submodules' - manifests included, as they do when it is scanned on its own; a repository - without a policy adds nothing. They never cover a manifest outside that - tree: not the superproject's own, not a sibling's, and not the repository's - around it. The line names the policy whose row matched: - - ``` - waived (leaf/policy/principles.toml): typosquatting on npm:left-pad@1.3.0 at -- - ``` - - The innermost repository's rows are consulted first, then each one outward, - then the root's. An outer row, nested or the root's, that would also have - covered the risk is a repeat of the row that matched. Over the whole tree, a - row that only ever repeated an inner one is named as a copy, rather than - called a match: - - ``` - waiver repeats leaf/policy/principles.toml: typosquatting on npm:left-pad@1.3.0 -- a repository nearer the manifest waives it in its own policy, so this copy can be removed - waiver in outer/policy/principles.toml repeats outer/leaf/policy/principles.toml: ... - ``` - - A range scan reads only the manifests that moved, so it cannot tell a copy - from a row that also covers a manifest it did not read, and says neither. - A nested repository's row that matched nothing is not reported by the - superproject. That repository's own scan reports it, and that repository is - the one that can remove it. An enclosing policy that does not load leaves - the manifests in its tree unchecked, which is could-not-look and exit `2` - naming the manifest, the repository and the loader's error. It is never - read as having no waivers. -- `scanner` is `guarddog` alone. The other scanners carry their own - suppression in their own configuration (`osv-scanner.toml`, `deny.toml`, - zizmor's config, cargo-vet's audits), and a second list here would be two - sets of exceptions free to disagree. -- Refused at load, as exit `2`: another scanner, a `package` without an - ecosystem or a version, an empty `check` or `reason`, the same waiver twice, - and `[supply_chain]` in a bundled set or an `inherit.paths` file, which have - no lockfile of their own. - -### How the other three report could-not-look +### How the scanners report could-not-look `tool_read()` hands each scanner's exit code, stdout and stderr to a reader that may name a could-not-look. A tool whose exit code already separates the @@ -3921,8 +3712,8 @@ audit exit `255` and name them, which is a finding. A run that could not happen at all — no store to read, a store that does not parse, a lockfile `--locked` refuses — also exits `255`. `tool()` answers by exit code, and every non-zero code is a verdict, so the could-not-look half currently reports as out of step. -This is the guarddog defect with the codes reversed: guarddog exits `0` on work -it did not do, cargo-vet exits `255` on work it could not start. +A scanner that exits `0` on work it did not do would be the same defect with the +codes reversed; cargo-vet exits `255` on work it could not start. The exit code is the only place the two are confused. They separate cleanly on the stream: a vetting failure prints `Vetting Failed!` to stdout and leaves diff --git a/src/config.rs b/src/config.rs index c7838bf..4926f54 100644 --- a/src/config.rs +++ b/src/config.rs @@ -614,130 +614,63 @@ impl Override { #[derive(Debug, Clone, Default, Deserialize)] #[serde(deny_unknown_fields)] pub(crate) struct SupplyChain { - /// `[[supply_chain.waive]]`: findings this repository has looked at and - /// holds not to be a reason to refuse a push. See [`Waiver`]. + /// `[[supply_chain.waive]]`: rows written for the guarddog section that + /// `uphold supply-chain` used to have. Still read, so that a policy written + /// before the section was removed keeps loading; every row is ignored, and + /// each is named as ignored when the command runs. See [`Waiver`]. #[serde(default)] pub waive: Vec, /// The forge host a first-party git dependency lives on, for the owner the /// policy declares: `github.com` where this is not written. A git source - /// under that host and owner is checked against its remote rather than - /// handed to guarddog, which looks names up on `PyPI`. + /// under that host and owner is checked against its remote; any other is + /// refused. #[serde(default)] pub forge_host: Option, } -/// The scanners a waiver may name. guarddog alone: it is the one scanner whose -/// findings `uphold supply-chain` reads out of its report, and the other four -/// each carry their own suppression in their own configuration -- an OSV -/// `osv-scanner.toml`, a cargo-deny `deny.toml`, a zizmor config, cargo-vet's -/// audits -- where a second one here would be two lists of exceptions free to -/// disagree. -const WAIVABLE_SCANNERS: [&str; 1] = ["guarddog"]; +/// The scanner a retired waiver may still name. uphold no longer runs it, so +/// the row is read and ignored; naming any other scanner is an error, as it +/// always was, because every scanner that is run carries its own suppression in +/// its own configuration. +const RETIRED_SCANNER: &str = "guarddog"; -/// The ecosystems a guarddog waiver may name, as its `package` spells them. -const WAIVABLE_ECOSYSTEMS: [&str; 2] = ["pypi", "npm"]; - -/// One confirmed false positive: a scanner's rule, on one package at one -/// version, with the reason it was judged one. +/// A `[[supply_chain.waive]]` row written when the guarddog section existed. /// -/// Keyed on the VERSION, so a bump is read fresh: what was looked at is that -/// release, and the next one is a different tarball with a different reason to -/// be suspicious or not. A waived finding is still printed, marked waived with -/// its reason, so the verdict says what was not held against the push rather -/// than hiding it, and a waiver that matched nothing in a run that read its -/// ecosystem is reported, so a stale one gets removed. +/// It waives nothing now. It is kept loadable so a policy that still carries +/// one is told so, by `uphold supply-chain`, rather than refused at load: the +/// row is a leftover to delete, not a policy error. #[derive(Debug, Clone, Deserialize)] #[serde(deny_unknown_fields)] pub(crate) struct Waiver { /// `guarddog`. pub scanner: String, - /// `:@` -- `pypi:pandas@3.0.6`, - /// `npm:@scope/pkg@1.2.0`. + /// `:@`. pub package: String, - /// The scanner's rule, as it names it: guarddog's `metadata_mismatch`. + /// The scanner's rule, as it named it. pub check: String, - /// Why this finding is not a reason to refuse. Required, because a waiver - /// nobody can re-judge is a switch nobody can turn back on. + /// Why the finding was judged a false positive. pub reason: String, } impl Waiver { - /// `(ecosystem, name, version)`, or `None` where `package` is not the - /// three-part spelling. Validated at load, so a loaded waiver always has - /// one. - pub(crate) fn parts(&self) -> Option<(&str, &str, &str)> { - let (ecosystem, rest) = self.package.split_once(':')?; - // `rsplit`, because an npm scope carries its own `@`. - let (name, version) = rest.rsplit_once('@')?; - (!name.is_empty() && !version.is_empty()).then_some((ecosystem, name, version)) - } - - /// Whether this waiver covers `check` reported on `name` at `version` in - /// `ecosystem`. - pub(crate) fn covers(&self, ecosystem: &str, name: &str, version: &str, check: &str) -> bool { - self.check == check - && self - .parts() - .is_some_and(|parts| parts == (ecosystem, name, version)) - } - fn validate(&self) -> Result<()> { - let at = format!("`[[supply_chain.waive]]` for {:?}", self.package); - if !WAIVABLE_SCANNERS.contains(&self.scanner.as_str()) { - return Err(Fatal::new(format!( - "{at} names the scanner {:?}. A waiver here is for {}; every other scanner \ - carries its own suppression in its own configuration, and a second list \ - here would be two sets of exceptions free to disagree", - self.scanner, - WAIVABLE_SCANNERS.join(", ") - ))); - } - match self.parts() { - Some((ecosystem, _, _)) if WAIVABLE_ECOSYSTEMS.contains(&ecosystem) => {} - _ => { - return Err(Fatal::new(format!( - "{at}: `package` is `:@`, with the ecosystem one \ - of {} -- `pypi:pandas@3.0.6`. A waiver with no version would outlive \ - the release it was judged on", - WAIVABLE_ECOSYSTEMS.join(", ") - ))); - } - } - if self.check.trim().is_empty() { - return Err(Fatal::new(format!( - "{at}: `check` is empty. Name the scanner's rule, as guarddog reports it \ - (`metadata_mismatch`): a waiver of every rule on a package is not a \ - finding somebody looked at" - ))); - } - if self.reason.trim().is_empty() { + if self.scanner != RETIRED_SCANNER { return Err(Fatal::new(format!( - "{at}: `reason` is empty. Say why the finding is not a reason to refuse, so \ - the next reader can judge whether it still holds" + "`[[supply_chain.waive]]` for {:?} names the scanner {:?}. No scanner takes a \ + waiver here any more (the one that did, {RETIRED_SCANNER}, was removed); every \ + scanner carries its own suppression in its own configuration, and a second \ + list here would be two sets of exceptions free to disagree", + self.package, self.scanner ))); } Ok(()) } } -/// Every waiver this policy declares, held to its shape, and none twice. +/// Every waiver this policy declares, held to its shape. fn validate_waivers(path: &Path, waivers: &[Waiver]) -> Result<()> { - for (index, waiver) in waivers.iter().enumerate() { + for waiver in waivers { waiver.validate().map_err(|error| Fatal::at(path, error))?; - if waivers.iter().take(index).any(|earlier| { - earlier.scanner == waiver.scanner - && earlier.package == waiver.package - && earlier.check == waiver.check - }) { - return Err(Fatal::at( - path, - format!( - "`[[supply_chain.waive]]` waives {} on {} twice. Keep one, with the \ - reason that holds", - waiver.check, waiver.package - ), - )); - } } Ok(()) } @@ -4320,10 +4253,13 @@ mod tests { assert_eq!(unicode.builtin(), Some("prevent-unusual-unicode-in-files")); } - /// A waiver belongs to the repository whose lockfile it is about: refused - /// in a file somebody else's `[inherit]` line reads, and refused twice. + /// A retired waiver belongs to the repository whose lockfile it was about: + /// refused in a file somebody else's `[inherit]` line reads. It still loads + /// in the repository's own file, so removing guarddog does not turn a + /// leftover row into a policy error, and a row naming any other scanner is + /// refused. #[test] - fn a_supply_chain_waiver_is_the_repositorys_own_and_is_written_once() { + fn a_retired_supply_chain_waiver_loads_and_is_the_repositorys_own() { let waiver = "[[supply_chain.waive]]\nscanner = \"guarddog\"\n\ package = \"pypi:pandas@3.0.6\"\ncheck = \"metadata_mismatch\"\n\ reason = \"extras read as required\"\n"; @@ -4335,19 +4271,13 @@ mod tests { let error = load(&dir, &path).unwrap_err().to_string(); assert!(error.contains("an inherited file has none"), "{error}"); - let twice = policy_from(&format!("{waiver}\n{waiver}")) - .unwrap_err() - .to_string(); - assert!(twice.contains("twice"), "{twice}"); - let once = policy_from(waiver).unwrap(); assert_eq!(once.supply_chain.waive.len(), 1); - assert!(once.supply_chain.waive.first().unwrap().covers( - "pypi", - "pandas", - "3.0.6", - "metadata_mismatch" - )); + + let other = policy_from(&waiver.replace("guarddog", "osv")) + .unwrap_err() + .to_string(); + assert!(other.contains("names the scanner"), "{other}"); } /// `require_any_link = false` on the `broken-links` rule, for a repository diff --git a/src/main.rs b/src/main.rs index 0c1c9e3..c91cad0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -127,9 +127,10 @@ usage: uphold hooks --identity DIR... do these repositories declare the same hooks uphold hooks --install write the hooks git runs, as tracked files [--adopt | --check] take over a hand-written copy; or only report - uphold supply-chain origin, advisories, typosquats, workflow - security and committed secrets over the - pushed range -- six scanners, one verdict + uphold supply-chain origin, advisories, workflow security, + git pins and committed secrets over the + pushed range -- five scanners and a + git-pin check, one verdict uphold supply-chain --all the same, over every manifest and commit uphold supply-chain --base REV the same, over what REV..HEAD changed uphold supply-chain --staged gitleaks alone, over the staged diff @@ -470,9 +471,9 @@ fn run() -> Result { // workflows, and a superproject that only tracks submodules is // still where its own workflows live. let (root, policy) = discover(&working).ok_or_else(|| no_policy_here(&working))?; - // Loaded for three facts: whether it inherits the set that asks for - // gitleaks, the guarddog findings it has waived, and who owns this - // repository, which decides the first-party git dependencies. A + // Loaded for two facts: whether it inherits the set that asks for + // gitleaks, and who owns this repository, which decides the + // first-party git dependencies. A // policy that will not load is exit 2 here as it is // everywhere else: which sections apply cannot be known without it. let loaded = config::load(&root, &policy)?; @@ -507,25 +508,7 @@ fn run() -> Result { } supply::scope_for_push(&root, &push.refs)? }; - // A repository nested in this tree -- a submodule, a vendored clone - // with its own `.git` -- keeps its own waivers for its own - // manifests, and they are found here the way `uphold` run inside it - // would find them: the same boundary, the same file names, the same - // loader with its `[inherit]`. Asked only of a directory that IS a - // repository root, because `discover` from anywhere else climbs, and - // the climb would land on this root's policy or another - // repository's. - let nested = |directory: &Path| -> Result> { - if !is_repository_root(directory) { - return Ok(None); - } - discover(directory) - .map(|(own, path)| { - config::load(&own, &path).map(|own_policy| (path, own_policy)) - }) - .transpose() - }; - supply::run(&root, &scope, secrets, &loaded, &nested) + supply::run(&root, &scope, secrets, &loaded) } "probe" => { let usage = || { diff --git a/src/supply.rs b/src/supply.rs index 45f40f8..1e31b69 100644 --- a/src/supply.rs +++ b/src/supply.rs @@ -1,12 +1,18 @@ -//! `uphold supply-chain` -- origin, advisories, typosquats, workflow security -//! and committed secrets, in one run. +//! `uphold supply-chain` -- origin, advisories, workflow security, first-party +//! git pins and committed secrets, in one run. //! -//! Five external scanners over dependencies and workflows, orchestrated: +//! Four external scanners over dependencies and workflows, orchestrated: //! osv-scanner (known vulnerabilities and reported-malicious packages), zizmor -//! (workflow security), cargo-deny (origin, advisories, bans, licenses), -//! cargo-vet (has anyone looked at this dependency) and guarddog (publisher -//! identity and typosquats -- the half OSV cannot reach, scoring an UNKNOWN -//! package on how closely its name shadows a popular one). +//! (workflow security), cargo-deny (origin, advisories, bans, licenses) and +//! cargo-vet (has anyone looked at this dependency). Beside them runs one check +//! of this crate's own: the git sources a Python lock or an npm manifest names, +//! which no registry holds and no scanner reads, checked against their own +//! remote (see `references`). +//! +//! Heuristic malware scanning of unreported packages is not done here. The +//! window it covered -- a malicious release published but not yet reported to +//! OSV -- is closed at install time instead, by the package manager's own +//! release-age setting, which refuses a version younger than the window. //! //! A sixth, gitleaks, reads commits rather than manifests, and owns secret //! shapes: the token formats, entropy thresholds and allowlists the @@ -34,33 +40,12 @@ //! config rather than a dependency. A wrapper that re-judged findings would be //! a second opinion nobody asked for, drifting from the tool it wraps. //! -//! ONE SCANNER'S FINDINGS ARE READ ANYWAY, and the exception is guarddog. -//! `guarddog verify` exits 0 whether it found three high-severity risks or -//! none, in both ecosystems, so for as long as this section answered by exit -//! code it called every guarddog finding clean -- a false negative on the one -//! scanner here that looks for malware. Its `--exit-non-zero-on-finding` flag -//! is not the remedy: it counts `issues`, which includes capability matches -//! guarddog itself scores 0.0 and labels `no_risks_detected`. So guarddog is -//! run with `--output-format json` and its own `risks` list is counted. The -//! count is reported, never recomputed; the choice is to parse it or to run it -//! for nothing. One named exception: a `metadata_mismatch` risk whose every -//! difference is a known npm rewrite or a verified `PyPI` optional extra is -//! printed as tolerated rather than counted. -//! -//! guarddog is also not handed what it cannot look up. A `uv export` spells a -//! git source, a direct URL and a path as a PEP 508 direct reference, and -//! guarddog asks `PyPI` for each by name and gets a 404. `references` sorts the -//! export first: a git source under the owner the policy declares is checked -//! against its own remote, and every other direct reference outside the tree -//! is refused by name rather than skipped. -//! //! What IS read out of a scanner's output is the opposite of a finding: its -//! own admission that it did not look. Four of the five need it, because four -//! of the five cannot say so in their exit code. guarddog reports rules that -//! timed out and still exits 0; cargo-vet answers 255 both for an unvetted -//! dependency and for a store it could not open; cargo-deny's exit 1 is a -//! matched advisory or a database it could not fetch; and zizmor, handed one -//! unparseable workflow among good ones, skips it and exits 0. None of those +//! own admission that it did not look. Three of the four need it, because they +//! cannot say so in their exit code. cargo-vet answers 255 both for an +//! unvetted dependency and for a store it could not open; cargo-deny's exit 1 +//! is a matched advisory or a database it could not fetch; and zizmor, handed +//! one unparseable workflow among good ones, skips it and exits 0. None of those //! is a judgement about a dependency being re-judged here. Each is the record //! that a question went unasked, which is this command's third verdict and the //! reason it exists. @@ -105,10 +90,9 @@ use std::collections::BTreeSet; use std::path::{Path, PathBuf}; use std::process::Command; -use crate::config::{Policy, Waiver}; +use crate::config::Policy; use crate::error::{Exit, Fatal, Result, verdict}; -mod pypi; mod references; /// The zizmor policy run where the repository has none of its own. @@ -154,8 +138,8 @@ enum Section { type SectionRun<'a> = (&'static str, &'a dyn Fn(&Path, &Scope) -> Result
); /// The file names a scanner here reads. Everything else in a diff -- source, -/// documentation, a test fixture -- changes nothing any of these five tools -/// would answer differently, so a range holding only those is a range with +/// documentation, a test fixture -- changes nothing any of these +/// tools would answer differently, so a range holding only those is a range with /// nothing to scan rather than a range nobody scanned. const MANIFEST_NAMES: [&str; 6] = [ "Cargo.toml", @@ -197,17 +181,11 @@ pub(crate) const SECRETS_SET: &str = "credentials"; /// `uphold supply-chain`. `secrets` is whether the policy inherits /// [`SECRETS_SET`]. /// -/// `policy` is read by the guarddog section alone: its `[[supply_chain.waive]]` -/// entries, and the declared owner that decides which git dependencies are -/// first party. `nested` is read by that section too, and for the waivers -/// alone: see [`NestedPolicy`]. -pub(crate) fn run( - root: &Path, - scope: &Scope, - secrets: bool, - policy: &Policy, - nested: NestedPolicy<'_>, -) -> Result { +/// `policy` is read for the declared owner, which decides which git +/// dependencies are first party, and for any `[[supply_chain.waive]]` row left +/// from the guarddog section this command no longer has: those are named as +/// ignored, and are not a failure. +pub(crate) fn run(root: &Path, scope: &Scope, secrets: bool, policy: &Policy) -> Result { let manifests_moved = !matches!(scope, Scope::Changed(paths, _) if paths.is_empty()); if !manifests_moved && !secrets { println!( @@ -216,9 +194,16 @@ pub(crate) fn run( ); return Ok(Exit::Clean); } + for waiver in &policy.supply_chain.waive { + eprintln!( + "supply chain: ignoring `[[supply_chain.waive]]` for {} ({}): scanner {:?} is no \ + longer run by uphold, so the row waives nothing. Remove it (its reason was: {})", + waiver.package, waiver.check, waiver.scanner, waiver.reason + ); + } let mut failed = 0_usize; let mut unread = 0_usize; - let guarded = |at: &Path, over: &Scope| guarddog(at, over, policy, nested); + let pinned = |at: &Path, over: &Scope| git_pins(at, over, policy); let mut sections: Vec> = Vec::new(); if manifests_moved { let dependencies: [SectionRun<'_>; 5] = [ @@ -229,7 +214,10 @@ pub(crate) fn run( ("zizmor -- workflow security", &zizmor), ("cargo-deny -- origin, advisories, bans, licenses", &deny), ("cargo-vet -- has anyone looked at this dependency", &vet), - ("guarddog -- publisher identity and typosquats", &guarded), + ( + "git pins -- first-party git sources against their own remote", + &pinned, + ), ]; sections.extend(dependencies); } else { @@ -1418,7 +1406,7 @@ fn vet_could_not_look(code: i32, stdout: &str, stderr: &str) -> Option { /// The first thing a tool said, so a could-not-look names something. /// /// "The scanner was inconclusive" with no subject is a red nobody can act on, -/// which is the same argument the guarddog reason is built on. +/// which is why every could-not-look names its subject. fn first_said(text: &str) -> &str { text.lines() .map(str::trim) @@ -1520,664 +1508,6 @@ fn vet(root: &Path, scope: &Scope) -> Result
{ tool_read(root, "cargo", &["vet", "--locked"], vet_could_not_look) } -/// Metadata rules only: the source-code rules download every release, which -/// is a different job for a different schedule. -const GUARDDOG_RULES: [&str; 10] = [ - "-r", - "typosquatting", - "-r", - "deceptive_author", - "-r", - "unclaimed_maintainer_email_domain", - "-r", - "potentially_compromised_email_domain", - "-r", - "metadata_mismatch", -]; - -/// The closed list of metadata normalizations supported by the report reader. -/// `OptionalExtras` additionally requires release evidence from [`pypi`]. -/// -/// The rule compares the registry's manifest for a release with the -/// `package.json` inside its tarball. `npm publish` normalizes the manifest it -/// uploads and leaves the tarball's file as written, so some differences are -/// npm's own and recur on every release of a package, where a version-keyed -/// waiver needs a new row each time. These are the normalize steps of -/// `@npmcli/package-json`, and only these two are accepted: -/// -/// - `DefaultStart`, npm's `serverjs` step: a package with `server.js` at its -/// root and no `scripts.start` is published with `"start": "node server.js"`. -/// guarddog prints the absent tarball side as `None`, which is also how a -/// literal `"None"` prints; either way the registry side is npm's default. -/// The report does not list the tarball's files, so `server.js` itself is -/// not confirmed. -/// - `BinPrefixStripped`, npm's `scriptpath` step: one leading -/// `./node_modules/.bin/` or `node_modules/.bin/` is removed from each -/// script. The same text anywhere else in the value is not removed by npm -/// and is not accepted here. -/// -/// A risk is tolerated only where every difference its description lists is -/// one of these. Any other field, any other script difference, or a line that -/// does not read as exactly one difference leaves the risk counted as before, -/// and a waiver still applies to it. Not on the list: `"install": "node-gyp -/// rebuild"`, which npm adds where `binding.gyp` exists; an install script runs -/// at install time, which `start` does not, so it joins only with a case and a -/// test. `PyPI` optional extras use separate release evidence in [`pypi`], -/// because the report itself drops the markers that identify an extra. -#[derive(Clone, Copy)] -enum Normalization { - DefaultStart, - BinPrefixStripped, - OptionalExtras, -} - -/// Every normalization tolerated, in the order a difference is tried against -/// them. -const PUBLISH_NORMALIZATIONS: [Normalization; 3] = [ - Normalization::DefaultStart, - Normalization::BinPrefixStripped, - Normalization::OptionalExtras, -]; - -impl Normalization { - /// What a `tolerated:` line calls it. - const fn name(self) -> &'static str { - match self { - Self::DefaultStart => "npm default start script", - Self::BinPrefixStripped => "npm strips a leading node_modules/.bin/", - Self::OptionalExtras => "PyPI optional extras verified against release requires_dist", - } - } - - /// Whether npm's publish turns the tarball's `scripts.` into the - /// registry's, both as guarddog printed them. - fn accepts(self, key: &str, registry: &str, tarball: &str) -> bool { - match self { - Self::DefaultStart => { - key == "start" && registry == "node server.js" && tarball == "None" - } - Self::BinPrefixStripped => ["./node_modules/.bin/", "node_modules/.bin/"] - .iter() - .any(|prefix| tarball.strip_prefix(prefix) == Some(registry)), - Self::OptionalExtras => false, - } - } -} - -/// The normalization behind each difference an npm `metadata_mismatch` -/// description lists, described, or `None` where any difference is not one of -/// [`PUBLISH_NORMALIZATIONS`] or any line cannot be read as one difference. -/// -/// guarddog 3.2.0's `describe_diff` prints a header, a `: ` line per -/// differing field, and under `scripts` one -/// ` : Manifest(""), package.json("") ` line per -/// differing key, in an order that is not stable. It escapes nothing, so a -/// line in which either marker appears more than once is ambiguous and is not -/// read, and a value spanning lines leaves a line that is not a difference. -/// Only `scripts` can hold a tolerated difference, so any other field refuses. -fn tolerated(description: &str) -> Option> { - const OPEN: &str = ": Manifest(\""; - const BETWEEN: &str = "\"), package.json(\""; - let mut lines = description.lines().map(str::trim_end); - if lines.next()? != "Difference between manifest and package.json found:" - || lines.next()? != "scripts:" - { - return None; - } - let mut accepted = Vec::new(); - for line in lines { - let entry = line.strip_prefix(" ")?; - if entry.matches(OPEN).count() != 1 || entry.matches(BETWEEN).count() != 1 { - return None; - } - let (key, rest) = entry.split_once(OPEN)?; - let (registry, tarball) = rest.strip_suffix("\")")?.split_once(BETWEEN)?; - let normalization = PUBLISH_NORMALIZATIONS - .into_iter() - .find(|normalization| normalization.accepts(key, registry, tarball))?; - accepted.push(format!( - "{} (scripts.{key} = \"{registry}\")", - normalization.name() - )); - } - (!accepted.is_empty()).then_some(accepted) -} - -/// A second source is consulted only for the exact `PyPI` report shape it can -/// explain. Unavailable or mismatched evidence leaves the scanner's finding -/// intact, so optional normalization never makes the scan depend on a pass. -fn tolerated_metadata( - ecosystem: &str, - name: &str, - version: &str, - description: &str, - metadata: &mut impl FnMut(&str, &str) -> std::result::Result, -) -> Option> { - match ecosystem { - "npm" => tolerated(description), - "pypi" => { - let differences = pypi::differences(description)?; - match metadata(name, version) { - Ok(release) => pypi::confirms(&release, name, version, &differences) - .map(|accepted| vec![accepted]), - Err(reason) => { - println!( - " retained: metadata_mismatch on pypi:{name}@{version} -- \ - could not verify optional extras: {reason}" - ); - None - } - } - } - _ => None, - } -} - -/// guarddog's floor: the "rules failed to run while scanning" text, the JSON -/// report's `errors` / `results` / `risks` keys and exit 0 on a finding were -/// all measured against guarddog 3.2.0. -const GUARDDOG_FLOOR: Floor = Floor { - tool: "guarddog", - program: "guarddog", - args: &["--version"], - oldest: Version(3, 2, 0), -}; - -/// The directory guarddog unpacks what it downloads under: -/// `$XDG_CACHE_HOME/uphold/guarddog`, else `$HOME/.cache/uphold/guarddog`, or -/// `None` where neither names an absolute directory. -/// -/// GUARDDOG'S `metadata_mismatch` VERDICT ON A PYPI WHEEL DEPENDS ON THE -/// FILESYSTEM IT UNPACKS ON. guarddog 3.2.0 extracts the release into a fresh -/// `tempfile.TemporaryDirectory()` and takes the first `os.listdir` entry that -/// starts with the package name as the package's root. A wheel such as pandas -/// 3.0.3 holds two, `pandas/` (which carries a `pyproject.toml`) and -/// `pandas-3.0.3.dist-info/` (which carries none), and the order the two are -/// listed in is the filesystem's: tmpfs lists the newest first, btrfs in -/// creation order, ext4 in the order of a hash seeded per filesystem. One -/// order reaches the manifest and reports a mismatch; the other reaches none -/// and reports nothing. Inheriting the caller's `TMPDIR` made that the -/// verdict's input, so one lock passed or failed with where `TMPDIR` pointed. -/// -/// The defect is guarddog's, and nothing outside its process reorders a -/// listing. What is fixed here is the input: guarddog is handed this directory -/// as `TMPDIR`, so the caller's `TMPDIR` no longer decides, and one host gives -/// one verdict. Two hosts whose cache directories sit on different filesystems -/// can still disagree, which is why the stale-waiver notice for a `pypi` -/// `metadata_mismatch` waiver does not advise removing it. -/// -/// A relative `XDG_CACHE_HOME` is ignored, as the specification says: guarddog -/// runs in the manifest's directory, so a relative one would unpack downloads -/// into the tree being checked. The environment is a parameter so a test can -/// hand in one of its own. -fn guarddog_unpacks_under(env: impl Fn(&str) -> Option) -> Option { - let absolute = |name: &str| env(name).filter(|path| path.is_absolute()); - absolute("XDG_CACHE_HOME") - .or_else(|| absolute("HOME").map(|home| home.join(".cache"))) - .map(|cache| cache.join("uphold").join("guarddog")) -} - -/// [`guarddog_unpacks_under`] on this machine, created; `None`, with the line -/// saying why, where there is none or it could not be made. guarddog then -/// inherits `TMPDIR` as before, and the run says its verdict may move with it. -fn guarddog_unpack_directory() -> Option { - let fallback = "guarddog unpacks under TMPDIR, so its metadata_mismatch verdict may \ - differ with the filesystem TMPDIR is on"; - let Some(directory) = guarddog_unpacks_under(|name| std::env::var_os(name).map(PathBuf::from)) - else { - println!(" {fallback}: neither XDG_CACHE_HOME nor HOME names an absolute directory"); - return None; - }; - let mut builder = std::fs::DirBuilder::new(); - builder.recursive(true); - #[cfg(unix)] - std::os::unix::fs::DirBuilderExt::mode(&mut builder, 0o700); - match builder.create(&directory) { - Ok(()) => Some(directory), - Err(error) => { - println!( - " {fallback}: {} could not be created: {error}", - directory.display() - ); - None - } - } -} - -/// guarddog's own report that a rule did not run. -/// -/// It prints "Some rules failed to run while scanning :" and a bullet -/// per rule, then EXITS 0 -- the two email-domain rules time out routinely, and -/// an orchestrator reading only the exit code files that under "clean". A rule -/// that timed out is a question nobody answered, which is the could-not-look -/// verdict and not the pass. The one line returned names the packages and how -/// many rules, because "guarddog was inconclusive" with no subject is a red -/// nobody can act on. -fn rules_that_did_not_run(output: &str) -> Option { - let mark = "failed to run rule"; - let scanning = "rules failed to run while scanning "; - let mut rules = 0_usize; - let mut packages: Vec = Vec::new(); - for line in output.lines() { - if let Some((_, tail)) = line.split_once(scanning) { - let package = tail.trim().trim_end_matches(':').trim(); - if !package.is_empty() && !packages.iter().any(|held| held == package) { - packages.push(package.to_owned()); - } - } - if line.contains(mark) { - rules += 1; - } - } - if rules == 0 { - return None; - } - let named = if packages.is_empty() { - String::from("a package it did not name") - } else if packages.len() > 3 { - format!( - "{} and {} more", - packages - .iter() - .take(3) - .cloned() - .collect::>() - .join(", "), - packages.len() - 3 - ) - } else { - packages.join(", ") - }; - Some(format!( - "guarddog left {rules} rule(s) unrun on {named}, and still exited 0 -- a rule that \ - did not run answered nothing" - )) -} - -/// What one `guarddog verify` run established. -enum Read { - /// guarddog objected to something, with any missing coverage described. - Risks(String, Option), - /// guarddog did not answer, described. - Unread(String), - /// guarddog ran and objected to nothing. - Clean, -} - -/// Read one `guarddog verify` run out of its report rather than its exit code. -/// -/// GUARDDOG CANNOT SAY "I FOUND SOMETHING" IN ITS EXIT CODE. `verify` answers -/// 0 whether it found three high-severity risks or none, in both ecosystems, -/// so a section reading the code called every finding clean. That is the one -/// direction this crate must never get wrong, and it was wrong here. -/// -/// `--exit-non-zero-on-finding` is not the fix. It keys off `issues`, which -/// counts capability matches -- `six` scores `issues: 2` with `risks: []` and -/// guarddog's own label `no_risks_detected` -- so the flag turns a package its -/// own report calls clean into a hard failure, trading a false negative for a -/// false positive. -/// -/// So this is the one scanner whose FINDINGS are read here, against the rule -/// the module header sets, and the reason is that the alternative is running -/// it for nothing. `risks` is guarddog's own list of what it objected to and -/// nothing is re-judged: the count is reported, not recomputed. -/// -/// A risk covered by [`tolerated_metadata`] is -/// printed as tolerated, naming each, and is not held against the run. -/// -/// A risk a waiver covers is printed as waived, with the waiver's reason, and -/// is not held against the run; the index of each root waiver that matched is -/// recorded in the ledger, with every package a report named, so the section -/// can report the waivers that did not. A nested repository's waiver is named -/// on its line by that repository's policy file, so whose decision let the -/// risk through is on the line that says it was let through. -#[expect( - clippy::too_many_arguments, - reason = "scan inputs and subprocess output meet at this reader" -)] -fn guarddog_read( - at: &str, - ecosystem: &str, - expected: &BTreeSet, - waivers: &Waivers<'_>, - ledger: &mut Ledger, - code: Option, - stdout: &str, - stderr: &str, - metadata: &mut impl FnMut(&str, &str) -> std::result::Result, -) -> Read { - // Its own admission first, because it survives whatever the report is. - if let Some(reason) = rules_that_did_not_run(stdout).or_else(|| rules_that_did_not_run(stderr)) - { - return Read::Unread(reason); - } - if code != Some(0) { - return Read::Unread(format!( - "guarddog gave no report at {at}: {}", - first_said(stderr) - )); - } - let Ok(serde_json::Value::Array(entries)) = serde_json::from_str::(stdout) - else { - return Read::Unread(format!( - "guarddog printed no report this could read at {at}" - )); - }; - // Where the input could not be read, even an empty expected set cannot - // establish coverage. Known inputs are reconciled below, including `[]`. - if entries.is_empty() && expected.is_empty() { - return Read::Unread(format!("guarddog reported on no dependency at all at {at}")); - } - let mut objected: Vec = Vec::new(); - let mut observed = BTreeSet::new(); - for entry in &entries { - let name = entry - .get("dependency") - .and_then(serde_json::Value::as_str) - .unwrap_or("a dependency it did not name"); - observed.insert(if ecosystem == "pypi" { - references::normalise(name) - } else { - name.to_owned() - }); - let Some(result) = entry.get("result") else { - return Read::Unread(format!("guarddog said nothing about {name} at {at}")); - }; - let scanned = result - .get("errors") - .and_then(serde_json::Value::as_object) - .is_some_and(serde_json::Map::is_empty); - if !scanned || result.get("results").is_none() { - return Read::Unread(format!( - "guarddog could not scan {name} at {at}: {}", - result.get("errors").map_or_else( - || String::from("it reported no result for it"), - ToString::to_string - ) - )); - } - let Some(results) = result.get("results").and_then(serde_json::Value::as_object) else { - return Read::Unread(format!( - "guarddog gave an incomplete report for {name} at {at}: `results` is not an object" - )); - }; - let Some(reported_risks) = result.get("risks").and_then(serde_json::Value::as_array) else { - return Read::Unread(format!( - "guarddog gave an incomplete report for {name} at {at}: `risks` is not an array" - )); - }; - // The version guarddog resolved, which for npm is the release a range - // in `package.json` picked and not the range itself. - let version = result - .get("package_version") - .or_else(|| entry.get("version")) - .and_then(serde_json::Value::as_str) - .unwrap_or(""); - ledger - .seen - .insert((ecosystem.to_owned(), (*name).to_owned())); - let mut risks = 0_usize; - for risk in reported_risks { - // The rule that raised it, as a waiver names it; `name` is the - // risk's own id, for a risk no single rule raised. - let rule = risk - .get("threat_rule") - .or_else(|| risk.get("capability_rule")) - .and_then(serde_json::Value::as_str) - .or_else(|| risk.get("name").and_then(serde_json::Value::as_str)) - .unwrap_or(""); - // Known metadata normalizations, named on the line that lets - // them through. The risk and the underlying result must describe - // the same difference; anything else keeps the risk counted. - if rule == "metadata_mismatch" - && let Some(description) = risk - .get("threat_description") - .and_then(serde_json::Value::as_str) - && results - .get("metadata_mismatch") - .and_then(serde_json::Value::as_str) - == Some(description) - && let Some(accepted) = - tolerated_metadata(ecosystem, name, version, description, metadata) - { - println!( - " tolerated: {rule} on {ecosystem}:{name}@{version} at {at} -- {}", - accepted.join("; ") - ); - continue; - } - // The innermost repository's rows first, then each one out, then - // the root's. The first to cover the risk lets it through and is - // named on the line; every later row that also covers it is a - // repeat of that one, recorded so the stale report can say so. - let mut covering: Option<(Option<&str>, &str)> = None; - for (source, rows) in waivers.in_order() { - let Some((index, row)) = rows - .iter() - .enumerate() - .find(|(_, row)| row.covers(ecosystem, name, version, rule)) - else { - continue; - }; - let key = (source.map(str::to_owned), index); - match covering { - None => { - ledger.matched.insert(key); - covering = Some((source, row.reason.as_str())); - } - Some((first, _)) => { - ledger.duplicated.entry(key).or_insert_with(|| Repeat { - of: first.unwrap_or_default().to_owned(), - check: row.check.clone(), - package: row.package.clone(), - }); - } - } - } - match covering { - Some((Some(source), reason)) => println!( - " waived ({source}): {rule} on {ecosystem}:{name}@{version} at {at} \ - -- {reason}" - ), - Some((None, reason)) => println!( - " waived: {rule} on {ecosystem}:{name}@{version} at {at} -- {reason}" - ), - None => risks += 1, - } - } - if risks > 0 { - let at_version = if version.is_empty() { - String::new() - } else { - format!("@{version}") - }; - objected.push(format!("{name}{at_version} ({risks} risk(s))")); - } - } - let missing: Vec<&str> = expected.difference(&observed).map(String::as_str).collect(); - let unread = (!missing.is_empty()).then(|| { - format!( - "guarddog omitted {} at {at}: no report for submitted {ecosystem} dependency(s)", - missing.join(", ") - ) - }); - if !objected.is_empty() { - Read::Risks( - format!("{at}: guarddog objected to {}", objected.join(", ")), - unread, - ) - } else if let Some(reason) = unread { - Read::Unread(reason) - } else { - Read::Clean - } -} - -/// How the guarddog section finds the policy of a repository nested inside the -/// scan root: handed a directory under the root, it answers that directory's -/// own policy file and the policy loaded from it where the directory is a -/// repository root carrying one, and `None` everywhere else. -/// -/// Handed in rather than written here because which file `uphold` loads at a -/// directory is the command's discovery, not this section's: a second walk -/// written here would be a second answer to "whose policy is this", free to -/// disagree with the one every other entry point gives -- over -/// `policy/principles.toml` against `policy/rg-policy.toml`, over where a -/// repository begins, and over `[inherit]`. A load that fails is an error and -/// never `None`: a policy that would not load has said nothing about which -/// findings its owner accepted, and reading it as "no waivers" would refuse on -/// a guess, or worse, if the run ever read it the other way, pass on one. -pub(crate) type NestedPolicy<'a> = &'a dyn Fn(&Path) -> Result>; - -/// A nested repository's own waivers, as the guarddog section reads them. -struct Nested { - /// Its policy file, relative to the scan root, which is how every line - /// that applies one of its waivers names whose decision it was. - source: String, - /// Its `[[supply_chain.waive]]` rows. - waive: Vec, -} - -/// The waivers one manifest is judged against. -/// -/// The scan root's rows cover every manifest in the tree, as they always have: -/// the root's policy is the one the run was started under. Each nested -/// repository's rows cover the manifests in that repository's tree and nothing -/// else, because they are that repository's decisions about the dependencies -/// it carries -- its own and its submodules', the same tree `uphold` run there -/// would judge. The superproject's manifests and a sibling's are somebody -/// else's, and a submodule able to waive them would be a repository excusing a -/// tree it does not own, under the superproject's name. -#[derive(Default)] -struct Waivers<'a> { - /// The scan root's own. - root: &'a [Waiver], - /// Every enclosing nested repository's that has a policy, innermost first. - nested: &'a [std::rc::Rc], -} - -impl Waivers<'_> { - /// Each owner's rows in the order a risk is put to them: the innermost - /// repository first, each one out after it, the root last, each named by - /// its policy file (`None` for the root's, whose lines have always carried - /// no name). - /// - /// Innermost first because the repository closest to a manifest is the - /// one whose dependency it is most directly; an outer row covering the - /// same risk is then a repeat of the inner one, which is what the stale - /// report needs to know to say a copy may go. - fn in_order(&self) -> impl Iterator, &[Waiver])> { - self.nested - .iter() - .map(|owner| (Some(owner.source.as_str()), owner.waive.as_slice())) - .chain(std::iter::once((None, self.root))) - } -} - -/// What the nested-repository walk found, by directory, so a repository with -/// many manifests is loaded once. A load that failed is kept as the loader's -/// error, because the same repository is the same refusal for each of its -/// manifests. -type NestedFound = - std::collections::BTreeMap>, String>>; - -/// The nested repositories whose waivers also cover a manifest in -/// `directory`, innermost first: every directory between it and `root` -- -/// `directory` itself included, `root` excluded -- that `nested` answers a -/// policy for. -/// -/// Every one, and not only the nearest, because each repository's policy -/// speaks for its whole tree: a repository scanned on its own judges its -/// submodules' manifests against its own rows, and a superproject that read -/// only the innermost repository's would refuse what the repository between -/// them accepts. A repository with no policy of its own adds nothing and is -/// passed over, for the reason `uphold` run there refuses to borrow one. -/// -/// The walk is `directory`'s own ancestors and nothing else, which is the -/// whole of the boundary: the root's own manifests have no nested repository -/// above them, a repository's rows never reach a manifest outside it, and a -/// sibling submodule is never an ancestor. Any enclosing policy that did not -/// load is an error for the manifest, whatever the others said: the rows it -/// would have contributed are unknown, so the findings they cover are too. -fn nested_waivers( - root: &Path, - directory: &Path, - nested: NestedPolicy<'_>, - found: &mut NestedFound, -) -> std::result::Result>, String> { - let mut owners = Vec::new(); - for ancestor in directory - .ancestors() - .take_while(|ancestor| *ancestor != root && ancestor.starts_with(root)) - { - let answer = found - .entry(ancestor.to_path_buf()) - .or_insert_with(|| load_nested(root, ancestor, nested)); - match answer { - Ok(Some(owner)) => owners.push(std::rc::Rc::clone(owner)), - Ok(None) => {} - Err(error) => { - return Err(format!( - "the guarddog findings at {} were not judged: {} is a repository with a \ - policy of its own, which decides waivers for the manifests in its tree, \ - and that policy did not load: {error}", - directory.display(), - ancestor.display() - )); - } - } - } - Ok(owners) -} - -/// One directory's answer for [`nested_waivers`]. -fn load_nested( - root: &Path, - ancestor: &Path, - nested: NestedPolicy<'_>, -) -> std::result::Result>, String> { - match nested(ancestor) { - Ok(Some((path, policy))) => Ok(Some(std::rc::Rc::new(Nested { - source: path - .strip_prefix(root) - .unwrap_or(&path) - .display() - .to_string(), - waive: policy.supply_chain.waive, - }))), - Ok(None) => Ok(None), - Err(error) => Err(error.to_string()), - } -} - -/// One waiver row: its policy file, as [`Waivers::in_order`] names it (`None` -/// for the scan root's), and its index there. -type Row = (Option, usize); - -/// An outer row that covered a risk an inner row had already covered. -struct Repeat { - /// The policy file whose row covered it first. - of: String, - /// The repeating row's `check` and `package`, as its line names them. - check: String, - package: String, -} - -/// What the guarddog reports of one run said about the waivers. -#[derive(Default)] -struct Ledger { - /// The rows that covered a risk first. - matched: BTreeSet, - /// The rows that would have covered a risk an inner repository's row - /// covered first: the copies superprojects made before nested waivers - /// were read, which the stale report names so they can go. - duplicated: std::collections::BTreeMap, - /// Every `(ecosystem, package)` a report named. - seen: BTreeSet<(String, String)>, - /// The ecosystems a report was read for at all. - read: BTreeSet<&'static str>, -} - /// Who is first party here, as the policy declares it. fn first_party_of<'a>(policy: &'a Policy, root: &Path) -> references::FirstParty<'a> { references::FirstParty { @@ -2204,22 +1534,25 @@ fn npm_locks(directory: &Path, root: &Path) -> Vec { .collect() } -fn guarddog( - root: &Path, - scope: &Scope, - policy: &Policy, - nested: NestedPolicy<'_>, -) -> Result
{ - let waivers = policy.supply_chain.waive.as_slice(); - let mut found = NestedFound::new(); +/// The first-party git pins section: every git source a Python lock or an npm +/// manifest names, checked against its own remote. +/// +/// Registries cannot vouch for a git source, and no scanner here reads one, so +/// this section does. A source under the owner the policy declares is asked of +/// its remote (`git ls-remote`): the locked commit must be what some ref there +/// points at, and a tag the lock names must point at that commit. Everything +/// else that no index holds -- a git source under anybody else, a direct URL, a +/// path outside the tree -- is refused by name, because skipping it would be a +/// pass nobody earned. +fn git_pins(root: &Path, scope: &Scope, policy: &Policy) -> Result
{ let (python, npm) = match scope { Scope::Whole => ( find_named(root, "uv.lock")?, find_named(root, "package.json")?, ), - // A directory, not a file: guarddog is run with the working directory - // set to the manifest's own, and `pyproject.toml` moving is a dependency - // change whether or not the lock moved with it. + // A directory, not a file: `uv export` is run with the working + // directory set to the manifest's own, and `pyproject.toml` moving is a + // dependency change whether or not the lock moved with it. // // A directory with no `uv.lock` is left out, as the whole-tree walk // leaves it out by finding no lock there: `uv export` in it would @@ -2234,7 +1567,7 @@ fn guarddog( let locked = lock.is_file(); if !locked { println!( - " {}: no uv.lock beside it, so guarddog has no pinned set to read", + " {}: no uv.lock beside it, so there is no pinned set to read", lock.with_file_name("pyproject.toml").display() ); } @@ -2250,16 +1583,9 @@ fn guarddog( Scope::Changed(..) => "no Python or npm manifest moved in this range", }))); } - if let Some(reason) = ready(&GUARDDOG_FLOOR) { - return Ok(Section::CouldNotLook(reason)); - } - // Handed to every guarddog run as `TMPDIR`; see `guarddog_unpacks_under`. - let unpack = guarddog_unpack_directory(); - let unpacks = || unpack.as_deref().map(|directory| ("TMPDIR", directory)); let mut refused = false; let mut unrun: Option = None; let mut checked = 0_usize; - let mut ledger = Ledger::default(); // Asked only where a git source needs it: `owner_from` is a command, and a // repository with no git dependency has no reason to run it. let mut first_party: Option> = None; @@ -2267,19 +1593,16 @@ fn guarddog( for lock in python { let directory = lock.parent().unwrap_or(root); checked += 1; - if on_path("uv").is_some() { - return Ok(Section::CouldNotLook(String::from( - "uv is not on PATH, so the Python manifests were not exported for guarddog", + if let Some(reason) = on_path("uv") { + return Ok(Section::CouldNotLook(format!( + "{reason}, so the Python manifests were not exported" ))); } // `--frozen`: the pinned set is read as it is and nothing is written. // Without it `uv export` relocks a `uv.lock` that is out of step with // its `pyproject.toml` and rewrites the file in the tree under scan. // Whether a lock is in step with its manifest is uv's question - // (`uv lock --check`), not the scan's: guarddog asks whether the - // pinned packages are malicious, so a lock that records an option its - // manifest lacks, such as an `exclude-newer` passed on the command - // line, is still scanned. + // (`uv lock --check`), not the scan's. let exported = Command::new("uv") .args([ "export", @@ -2294,7 +1617,7 @@ fn guarddog( .map_err(|error| Fatal::new(format!("could not run uv export: {error}")))?; if !exported.status.success() { println!( - " FAILED: guarddog: uv export failed in {}", + " FAILED: git pins: uv export failed in {}", directory.display() ); println!( @@ -2305,15 +1628,6 @@ fn guarddog( continue; } let at = directory.display().to_string(); - // Before guarddog is asked: a manifest whose owner's policy did not - // load has findings nothing here can tell from the accepted ones. - let owners = match nested_waivers(root, directory, nested, &mut found) { - Ok(owners) => owners, - Err(said) => { - unrun = unrun.or(Some(said)); - continue; - } - }; let sorted = references::sort( &String::from_utf8_lossy(&exported.stdout), directory, @@ -2321,7 +1635,7 @@ fn guarddog( std::fs::read_to_string(&lock).ok().as_deref(), ); for said in &sorted.refused { - println!(" FAILED: guarddog pypi: {at}: {said}"); + println!(" FAILED: git pins (pypi): {at}: {said}"); refused = true; } for pin in &sorted.git { @@ -2329,79 +1643,22 @@ fn guarddog( match references::check(pin, deciding, directory, &mut remotes) { references::Checked::Holds(said) => println!(" first party: {said}"), references::Checked::Refused(said) => { - println!(" FAILED: guarddog pypi: {at}: {said}"); + println!(" FAILED: git pins (pypi): {at}: {said}"); refused = true; } references::Checked::Unread(said) => unrun = unrun.or(Some(said)), } } - // Nothing an index resolves is nothing to look up, and guarddog handed - // an empty list answers `[]`, which reads as a network failure. - if sorted.indexed == 0 { - println!( - " {at}: no dependency here resolves from an index, so guarddog was not asked" - ); - continue; - } - let requirements = tempfile_guard::TempFile::containing(&sorted.kept)?; - let status = Command::new("guarddog") - .args(["pypi", "verify", "--output-format", "json"]) - .arg(&requirements.path) - .args(GUARDDOG_RULES) - .envs(unpacks()) - .current_dir(directory) - .output() - .map_err(|error| Fatal::new(format!("could not run guarddog: {error}")))?; - match guarddog_read( - &at, - "pypi", - &sorted.expected, - &Waivers { - root: waivers, - nested: &owners, - }, - &mut ledger, - status.status.code(), - &String::from_utf8_lossy(&status.stdout), - &String::from_utf8_lossy(&status.stderr), - &mut pypi::fetch, - ) { - Read::Clean => { - ledger.read.insert("pypi"); - } - Read::Risks(said, unread) => { - if unread.is_none() { - ledger.read.insert("pypi"); - } - unrun = unrun.or(unread); - println!(" FAILED: guarddog pypi: {said}"); - refused = true; - } - Read::Unread(said) => unrun = unrun.or(Some(said)), - } } - // guarddog reads `devDependencies` as well only under this setting, and - // compares it as Python's `.lower() == "true"`. - let include_dev = std::env::var("GUARDDOG_NPM_INCLUDE_DEV_DEPENDENCIES") - .is_ok_and(|value| value.eq_ignore_ascii_case("true")); for manifest in npm { let directory = manifest.parent().unwrap_or(root); checked += 1; let at = directory.display().to_string(); - // Before guarddog is asked: a manifest whose owner's policy did not - // load has findings nothing here can tell from the accepted ones. - let owners = match nested_waivers(root, directory, nested, &mut found) { - Ok(owners) => owners, - Err(said) => { - unrun = unrun.or(Some(said)); - continue; - } - }; let sorted = std::fs::read_to_string(&manifest) - .map(|text| references::sort_npm(&text, &npm_locks(directory, root), include_dev)) + .map(|text| references::sort_npm(&text, &npm_locks(directory, root))) .unwrap_or_default(); for said in &sorted.refused { - println!(" FAILED: guarddog npm: {at}: {said}"); + println!(" FAILED: git pins (npm): {at}: {said}"); refused = true; } for pin in &sorted.git { @@ -2409,74 +1666,18 @@ fn guarddog( match references::check(pin, deciding, directory, &mut remotes) { references::Checked::Holds(said) => println!(" first party: {said}"), references::Checked::Refused(said) => { - println!(" FAILED: guarddog npm: {at}: {said}"); + println!(" FAILED: git pins (npm): {at}: {said}"); refused = true; } references::Checked::Unread(said) => unrun = unrun.or(Some(said)), } } - // Nothing npm resolves among what guarddog reads -- no `dependencies`, - // only `devDependencies` where it does not read them, or only git - // dependencies -- is nothing to look up, and guarddog would - // answer `[]`, which reads as a network failure. A manifest that did not - // parse still goes to guarddog, for it to say so. - if sorted.read && sorted.indexed == 0 { - println!(" {at}: no dependency here resolves from npm, so guarddog was not asked"); - continue; - } - // The manifest itself where nothing came out of it; otherwise what is - // left. - let rewritten = sorted - .kept - .as_deref() - .map(tempfile_guard::TempFile::containing) - .transpose()?; - let status = Command::new("guarddog") - .args(["npm", "verify", "--output-format", "json"]) - .arg( - rewritten - .as_ref() - .map_or_else(|| Path::new("package.json"), |file| file.path.as_path()), - ) - .args(GUARDDOG_RULES) - .envs(unpacks()) - .current_dir(directory) - .output() - .map_err(|error| Fatal::new(format!("could not run guarddog: {error}")))?; - match guarddog_read( - &at, - "npm", - &sorted.expected, - &Waivers { - root: waivers, - nested: &owners, - }, - &mut ledger, - status.status.code(), - &String::from_utf8_lossy(&status.stdout), - &String::from_utf8_lossy(&status.stderr), - &mut pypi::fetch, - ) { - Read::Clean => { - ledger.read.insert("npm"); - } - Read::Risks(said, unread) => { - if unread.is_none() { - ledger.read.insert("npm"); - } - unrun = unrun.or(unread); - println!(" FAILED: guarddog npm: {said}"); - refused = true; - } - Read::Unread(said) => unrun = unrun.or(Some(said)), - } } println!(" {checked} Python/npm manifest(s) checked"); - report_unmatched(waivers, &ledger, scope); - // A finding outranks an unrun rule, which is this crate's own ranking: a - // refusal is something somebody looked at and objected to. The unrun rules - // are printed either way, so the red that does appear says what was - // still not asked. + // A finding outranks an unrun check, which is this crate's own ranking: a + // refusal is something somebody looked at and objected to. The unrun check + // is printed either way, so the red that does appear says what was still + // not asked. if let Some(ref reason) = unrun { println!(" {reason}"); } @@ -2489,89 +1690,11 @@ fn guarddog( Ok(Section::Clean) } -/// What a row that only repeats an inner repository's is told. -const REPEAT_ADVICE: &str = "a repository nearer the manifest waives it in its own policy, so \ - this copy can be removed"; - -/// A waiver that matched nothing, where the run could have matched it, is -/// one whose version moved or whose finding went away. "Could have" is the -/// package appearing in a report, or, over the whole tree, its ecosystem -/// being read at all -- a range scan reads only the manifests that moved, -/// and a waiver about one that did not is not stale for it. Reported and -/// not refused: the push it would have excused is not less safe for it, -/// and the line that names it is the prompt to delete it. -/// -/// The scan root's waivers only. A nested repository's row that matched -/// nothing here is reported by that repository's own scan, which is the -/// one that can act on it -- the row is in its policy, and a superproject -/// that printed it too would be a second reporter of one fact, naming a -/// file the superproject does not write. -/// -/// A row -- the root's or an outer nested repository's -- that covered risks -/// only where an inner repository's row had already covered them is a copy of -/// that row, and over the whole tree is named as one. A range scan reads only -/// the manifests that moved, so it cannot tell a copy from a row that also -/// covers a manifest it did not read, and says nothing about either: the row -/// did cover a risk, so it is not stale either. -fn report_unmatched(waivers: &[Waiver], ledger: &Ledger, scope: &Scope) { - let whole = matches!(scope, Scope::Whole); - for (index, waiver) in waivers.iter().enumerate() { - let Some((ecosystem, name, _)) = waiver.parts() else { - continue; - }; - let row = (None, index); - if !ledger.matched.contains(&row) - && let Some(repeat) = ledger.duplicated.get(&row) - { - if whole { - println!( - " waiver repeats {}: {} on {} -- {REPEAT_ADVICE}", - repeat.of, waiver.check, waiver.package - ); - } - continue; - } - let could = ledger - .seen - .contains(&(ecosystem.to_owned(), name.to_owned())) - || (matches!(scope, Scope::Whole) && ledger.read.contains(ecosystem)); - if could && !ledger.matched.contains(&row) { - // Except where whether the finding appears is the host's: a pypi - // `metadata_mismatch` reaches the release's manifest on some - // filesystems and not others (`guarddog_unpacks_under`), so its - // absence here is not its absence everywhere. - let advice = if ecosystem == "pypi" && waiver.check == "metadata_mismatch" { - "guarddog reads a pypi release's manifest for this check on some \ - filesystems and not others, so another host may still need it" - } else { - "remove it if the version moved or the finding is gone" - }; - println!( - " waiver matched nothing in this run: {} on {} -- {advice}", - waiver.check, waiver.package - ); - } - } - if !whole { - return; - } - for (row, repeat) in &ledger.duplicated { - if let (Some(source), _) = row - && !ledger.matched.contains(row) - { - println!( - " waiver in {source} repeats {}: {} on {} -- {REPEAT_ADVICE}", - repeat.of, repeat.check, repeat.package - ); - } - } -} - /// The gitleaks release this binary is tested against. /// -/// Pinned here, unlike the other five scanners, because gitleaks' verdict is +/// Pinned here, unlike the other scanners, because gitleaks' verdict is /// the rule list compiled into it: two machines on two versions are two gates -/// over the same commit. The other five answer from databases that move under +/// over the same commit. The others answer from databases that move under /// any version, so a pin would buy them nothing. The README's install line /// and docs/REFERENCE.md name this version too, and a unit test below holds /// the three together. @@ -2825,613 +1948,19 @@ mod tempfile_guard { #[cfg(test)] mod tests { use super::{ - BTreeSet, Ledger, Nested, NestedFound, NestedPolicy, Read, Waivers, guarddog_read, - nested_waivers, tolerated, - }; - use super::{ - DENY_FLOOR, Floor, GITLEAKS_DEFAULT, GITLEAKS_FOUND, GITLEAKS_VERSION, GUARDDOG_FLOOR, - OSV_FLOOR, PRUNE, Scope, VET_FLOOR, Version, ZIZMOR_FLOOR, against_floor, find_named, - find_workflow_dirs, gitleaks_could_not_look, guarddog_unpacks_under, interesting, - parse_version, unscanned_ci, + DENY_FLOOR, Floor, GITLEAKS_DEFAULT, GITLEAKS_FOUND, GITLEAKS_VERSION, OSV_FLOOR, PRUNE, + Scope, VET_FLOOR, Version, ZIZMOR_FLOOR, against_floor, find_named, find_workflow_dirs, + gitleaks_could_not_look, interesting, parse_version, unscanned_ci, }; - use crate::config::{Policy, Waiver}; - - /// guarddog 3.2.0's reports, recorded against the public registries with - /// only the unpack path rewritten: react-dom 19.2.0, whose registry - /// manifest carries npm's default `start`, swagger-ui-express 5.0.1, whose - /// `test` lost its leading `./node_modules/.bin/` at publish, and pandas - /// 3.0.3 on a host where the rule reached its `pyproject.toml`. - const REACT_DOM: &str = include_str!(concat!( - env!("CARGO_MANIFEST_DIR"), - "/tests/fixtures/guarddog/npm-react-dom-19.2.0.json" - )); - const SWAGGER_UI_EXPRESS: &str = include_str!(concat!( - env!("CARGO_MANIFEST_DIR"), - "/tests/fixtures/guarddog/npm-swagger-ui-express-5.0.1.json" - )); - const PANDAS: &str = include_str!(concat!( - env!("CARGO_MANIFEST_DIR"), - "/tests/fixtures/guarddog/pypi-pandas-3.0.3.json" - )); - - /// `report` with its one risk's description replaced, as guarddog would - /// print it for a release that differed in that way. - fn describing(report: &str, description: &str) -> String { - let mut value: serde_json::Value = serde_json::from_str(report).unwrap(); - value[0]["result"]["risks"][0]["threat_description"] = serde_json::Value::from(description); - value.to_string() - } - - /// Whether `report`, read as one run with no waivers, holds a finding, or - /// what was said where it did not read at all. - fn objected(ecosystem: &str, report: &str) -> Result { - let read = guarddog_read( - "web", - ecosystem, - &BTreeSet::new(), - &Waivers::default(), - &mut Ledger::default(), - Some(0), - report, - "", - &mut |_, _| Err(String::from("no registry evidence in this fixture")), - ); - match read { - Read::Risks(..) => Ok(true), - Read::Clean => Ok(false), - Read::Unread(said) => Err(said), - } - } - - const HEADER: &str = "Difference between manifest and package.json found: \n"; - const START: &str = " start: Manifest(\"node server.js\"), package.json(\"None\") \n"; - - /// Each publish-time normalization on its own is tolerated, in the report - /// guarddog printed for a real release, and named. - #[test] - fn a_mismatch_that_is_only_npms_publish_rewrite_is_tolerated_and_named() { - assert_eq!(objected("npm", REACT_DOM), Ok(false)); - assert_eq!(objected("npm", SWAGGER_UI_EXPRESS), Ok(false)); - assert_eq!( - tolerated(&format!("{HEADER}scripts: \n{START}")), - Some(vec![String::from( - "npm default start script (scripts.start = \"node server.js\")" - )]) - ); - assert_eq!( - tolerated(&format!( - "{HEADER}scripts: \n test: Manifest(\"mocha t\"), \ - package.json(\"node_modules/.bin/mocha t\") \n" - )), - Some(vec![String::from( - "npm strips a leading node_modules/.bin/ (scripts.test = \"mocha t\")" - )]) - ); - } - - /// Guarddog repeats the mismatch in `results` and on its risk. A risk's - /// description alone cannot excuse a different underlying result. - #[test] - fn npm_tolerance_requires_the_result_and_risk_to_agree() { - let mut report: serde_json::Value = serde_json::from_str(REACT_DOM).unwrap(); - report[0]["result"]["results"]["metadata_mismatch"] = serde_json::Value::from(format!( - "{HEADER}scripts: \n postinstall: Manifest(\"node evil.js\"), \ - package.json(\"None\") \n" - )); - assert_eq!(objected("npm", &report.to_string()), Ok(true)); - - report[0]["result"]["results"] - .as_object_mut() - .unwrap() - .remove("metadata_mismatch"); - assert_eq!(objected("npm", &report.to_string()), Ok(true)); - } - - /// A normalization alongside any other difference is still a finding: the - /// tolerance is for a risk that is nothing but npm's rewrites. - #[test] - fn a_normalization_beside_any_other_difference_is_still_a_finding() { - for description in [ - // A second script that differs. - format!( - "{HEADER}scripts: \n{START} postinstall: Manifest(\"node x.js\"), \ - package.json(\"None\") \n" - ), - // A dependency that differs, which describe_diff prints first. - format!( - "{HEADER}dependencies: \n left-pad: Manifest(\"1.0.0\"), \ - package.json(\"None\") \nscripts: \n{START}" - ), - // A `main` that differs, printed after `scripts`. - format!( - "{HEADER}scripts: \n{START}main: \n Manifest:\n index.js\n \ - package.json:\n evil.js\n" - ), - // A `start` npm did not write. - format!( - "{HEADER}scripts: \n start: Manifest(\"node evil.js\"), \ - package.json(\"None\") \n" - ), - // The default `start` where the tarball has one of its own. - format!( - "{HEADER}scripts: \n start: Manifest(\"node server.js\"), \ - package.json(\"node other.js\") \n" - ), - // The default under another key. - format!( - "{HEADER}scripts: \n postinstall: Manifest(\"node server.js\"), \ - package.json(\"None\") \n" - ), - ] { - assert_eq!(tolerated(&description), None, "{description}"); - assert_eq!( - objected("npm", &describing(REACT_DOM, &description)), - Ok(true), - "{description}" - ); - } - } - - /// `./node_modules/.bin/` is stripped by npm only at the start of a - /// script, once; the same text anywhere else, or a script that differs in - /// more than that prefix, is a finding. - #[test] - fn a_bin_prefix_removed_anywhere_but_the_start_is_still_a_finding() { - for (registry, tarball) in [ - ("nyc mocha t", "nyc ./node_modules/.bin/mocha t"), - ("mocha t", "./node_modules/.bin/mocha t && curl x | sh"), - ("mocha t", "../node_modules/.bin/mocha t"), - ("mocha t", "./node_modules/.bin/mocha t"), - ("./node_modules/.bin/mocha t", "mocha t"), - ] { - let description = format!( - "{HEADER}scripts: \n test: Manifest(\"{registry}\"), \ - package.json(\"{tarball}\") \n" - ); - assert_eq!(tolerated(&description), None, "{description}"); - assert_eq!( - objected("npm", &describing(SWAGGER_UI_EXPRESS, &description)), - Ok(true), - "{description}" - ); - } - } - - /// A description this cannot read line by line as single differences is a - /// finding, because its values are not escaped and a line that splits two - /// ways could hide a difference behind a tolerated one. - #[test] - fn a_description_line_that_does_not_read_as_one_difference_is_still_a_finding() { - for description in [ - String::new(), - String::from("start script differs"), - format!("{HEADER}scripts: \n"), - format!("{HEADER}scripts: \n{START}something guarddog never prints\n"), - // A value that spans lines. - format!( - "{HEADER}scripts: \n test: Manifest(\"mocha\nt\"), \ - package.json(\"./node_modules/.bin/mocha\nt\") \n" - ), - // A marker inside a value, so the line splits more than one way. - format!( - "{HEADER}scripts: \n start: Manifest(\"node server.js\"), \ - package.json(\"None\"), package.json(\"x\") \n" - ), - format!( - "{HEADER}scripts: \n start: Manifest(\"a: Manifest(\"node server.js\"), \ - package.json(\"None\") \n" - ), - // A key holding the marker: the script `start: Manifest("Z` is - // `curl x | sh` in the registry, and split at the first marker it - // would read as `start` losing a `.bin` prefix. - format!( - "{HEADER}scripts: \n start: Manifest(\"Z: Manifest(\"curl x | sh\"), \ - package.json(\"./node_modules/.bin/Z: Manifest(\"curl x | sh\") \n" - ), - ] { - assert_eq!(tolerated(&description), None, "{description:?}"); - assert_eq!( - objected("npm", &describing(REACT_DOM, &description)), - Ok(true), - "{description:?}" - ); - } - // A risk with no description at all. - let mut bare: serde_json::Value = serde_json::from_str(REACT_DOM).unwrap(); - bare[0]["result"]["risks"][0] - .as_object_mut() - .unwrap() - .remove("threat_description"); - assert_eq!(objected("npm", &bare.to_string()), Ok(true)); - } - - /// Without release evidence, the dropped `extra ==` markers cannot be - /// recovered from the report. An npm-shaped `PyPI` report also stays a finding. - #[test] - fn pypi_metadata_mismatch_is_not_tolerated() { - assert_eq!(objected("pypi", PANDAS), Ok(true)); - let npm_shaped = describing(PANDAS, &format!("{HEADER}scripts: \n{START}")); - assert_eq!(objected("pypi", &npm_shaped), Ok(true)); - } - - #[test] - fn pypi_extras_need_release_evidence_and_agreeing_reports() { - let mut report: serde_json::Value = serde_json::from_str(PANDAS).unwrap(); - let description = "Dependency mismatch between PyPI metadata and pyproject.toml:\n In PyPI metadata but NOT in pyproject.toml: pyarrow\n"; - report[0]["result"]["results"]["metadata_mismatch"] = description.into(); - report[0]["result"]["risks"][0]["threat_description"] = description.into(); - for (requirements, clean) in [ - (vec!["pyarrow>=13; extra == 'parquet'"], true), - (vec!["pyarrow>=13"], false), - (vec!["pyarrow; extra == 'parquet'", "pyarrow"], false), - ] { - let read = guarddog_read( - "project", - "pypi", - &BTreeSet::from([String::from("pandas")]), - &Waivers::default(), - &mut Ledger::default(), - Some(0), - &report.to_string(), - "", - &mut |name, version| { - assert_eq!((name, version), ("pandas", "3.0.3")); - Ok(serde_json::json!({"info": { - "name": name, "version": version, "requires_dist": requirements, - }})) - }, - ); - assert_eq!(matches!(read, Read::Clean), clean); - } - report[0]["result"]["results"]["metadata_mismatch"] = "different evidence".into(); - let mut fetched = false; - let read = guarddog_read( - "project", - "pypi", - &BTreeSet::new(), - &Waivers::default(), - &mut Ledger::default(), - Some(0), - &report.to_string(), - "", - &mut |_, _| { - fetched = true; - Err(String::from("unexpected lookup")) - }, - ); - assert!(matches!(read, Read::Risks(..))); - assert!(!fetched); - } - - #[test] - fn verified_pypi_extras_leave_unrelated_risks_counted() { - let mut report: serde_json::Value = serde_json::from_str(PANDAS).unwrap(); - for other_risk in [false, true] { - if other_risk { - report[0]["result"]["risks"] - .as_array_mut() - .unwrap() - .push(serde_json::json!({"threat_rule": "typosquatting"})); - } - let read = guarddog_read( - "project", - "pypi", - &BTreeSet::from([String::from("pandas")]), - &Waivers::default(), - &mut Ledger::default(), - Some(0), - &report.to_string(), - "", - &mut |_, _| { - Ok(serde_json::from_str(include_str!( - "../tests/fixtures/guarddog/pypi-pandas-3.0.3-metadata.json" - )) - .unwrap()) - }, - ); - assert_eq!(matches!(read, Read::Risks(..)), other_risk); - assert_eq!(matches!(read, Read::Clean), !other_risk); - } - } - - // ── a nested repository's own waivers ── - - /// A waiver as a policy file would carry it. - fn waiver(package: &str, check: &str, reason: &str) -> Waiver { - Waiver { - scanner: String::from("guarddog"), - package: String::from(package), - check: String::from(check), - reason: String::from(reason), - } - } - - /// A scan root that exists only as a path: the walk is lexical, and every - /// question it asks of the disk goes through the loader. - const SUPER: &str = "/srv/example/superproject"; - - /// A loader standing in for `uphold`'s discovery: a policy carrying one - /// `typosquatting` waiver on `npm:left-pad@1.3.0` at each directory in - /// `owners`, a load that fails at each in `broken`, and nothing elsewhere. - /// Every directory it is asked about is recorded, so a test can assert - /// where the walk went as well as what it found. - fn loader<'a>( - owners: &'a [&'a str], - broken: &'a [&'a str], - asked: &'a std::cell::RefCell>, - ) -> impl Fn(&std::path::Path) -> crate::error::Result> + 'a - { - move |directory| { - asked.borrow_mut().push(directory.to_path_buf()); - let relative = directory.strip_prefix(SUPER).unwrap_or(directory); - let path = directory.join("policy/principles.toml"); - if broken.iter().any(|at| std::path::Path::new(at) == relative) { - return Err(crate::error::Fatal::at(&path, "is not valid TOML")); - } - if !owners.iter().any(|at| std::path::Path::new(at) == relative) { - return Ok(None); - } - let mut policy = Policy::default(); - policy.supply_chain.waive = vec![waiver( - "npm:left-pad@1.3.0", - "typosquatting", - &format!("judged by {}", relative.display()), - )]; - Ok(Some((path, policy))) - } - } - - /// The policy files whose waivers `nested_waivers` gave a manifest in - /// `directory`, innermost first; empty where it gave only the root's. - fn owners_of(directory: &str, nested: NestedPolicy<'_>) -> Result, String> { - let root = std::path::Path::new(SUPER); - nested_waivers(root, &root.join(directory), nested, &mut NestedFound::new()) - .map(|found| found.iter().map(|owner| owner.source.clone()).collect()) - } - - /// The boundary, in both directions. A nested repository's waivers reach - /// the manifests in its own tree and none outside it: not the scan root's - /// own, not a sibling's, and not a directory whose name merely begins with - /// the repository's -- `Path::starts_with` is by component, and this is - /// the case that would break if it were ever done on text. - #[test] - fn a_nested_repositorys_waivers_reach_its_own_tree_and_nothing_outside_it() { - let asked = std::cell::RefCell::new(Vec::new()); - let nested = loader(&["leaf"], &[], &asked); - let leaf = || vec![String::from("leaf/policy/principles.toml")]; - for (directory, expected) in [ - ("leaf", leaf()), - ("leaf/web", leaf()), - ("leaf/packages/ui", leaf()), - ("web", vec![]), - ("", vec![]), - ("sibling", vec![]), - ("sibling/web", vec![]), - ("leaf-two/web", vec![]), - ] { - assert_eq!(owners_of(directory, &nested), Ok(expected), "{directory:?}"); - } - // The root is never asked: its policy is the one the run was started - // under, and answering it again as a "nested" one would print the - // root's own rows under a second name. - assert!( - asked - .borrow() - .iter() - .all(|at| at != std::path::Path::new(SUPER)), - "{:?}", - asked.borrow() - ); - } - - /// Every enclosing repository with a policy contributes, innermost first, - /// and each contributes only to its own tree: the outer repository reaches - /// the inner one's manifests, never the other way round. A repository - /// without a policy is passed over rather than stopped at. - #[test] - fn every_enclosing_repository_with_a_policy_contributes_innermost_first() { - let asked = std::cell::RefCell::new(Vec::new()); - let nested = loader(&["outer", "outer/inner"], &[], &asked); - let outer = "outer/policy/principles.toml"; - let inner = "outer/inner/policy/principles.toml"; - for (directory, expected) in [ - ("outer/inner/web", vec![inner, outer]), - ("outer/web", vec![outer]), - // `outer/bare` stands for a repository root with no policy: the - // loader answers `None` there as `uphold`'s discovery does. - ("outer/bare/web", vec![outer]), - ("outer/inner/bare/web", vec![inner, outer]), - ] { - assert_eq!( - owners_of(directory, &nested), - Ok(expected.into_iter().map(String::from).collect()), - "{directory:?}" - ); - } - } - - /// A nested policy that will not load is an error naming the manifest and - /// the repository, never "no waivers": the findings it would have judged - /// cannot be told apart from the ones it accepted. That holds at every - /// level -- an outer policy that will not load fails the manifest even - /// where the inner one loaded -- and the error names each manifest, not - /// the first one that met the broken repository. - #[test] - fn a_nested_policy_that_will_not_load_is_an_error_and_never_no_waivers() { - let asked = std::cell::RefCell::new(Vec::new()); - let inner_broken = loader(&["leaf"], &["leaf/inner"], &asked); - let said = owners_of("leaf/inner/web", &inner_broken).unwrap_err(); - assert!( - said.contains("/srv/example/superproject/leaf/inner/web"), - "{said}" - ); - assert!(said.contains("did not load"), "{said}"); - assert!(said.contains("is not valid TOML"), "{said}"); - // A sibling of the broken repository is judged as before. - assert_eq!( - owners_of("leaf/web", &inner_broken), - Ok(vec![String::from("leaf/policy/principles.toml")]) - ); - - let outer_broken = loader(&["leaf/inner"], &["leaf"], &asked); - let root = std::path::Path::new(SUPER); - let mut found = NestedFound::new(); - for directory in ["leaf/inner/web", "leaf/inner/api"] { - let refused = nested_waivers(root, &root.join(directory), &outer_broken, &mut found) - .map(|_| ()) - .unwrap_err(); - assert!( - refused.contains(&root.join(directory).display().to_string()), - "{refused}" - ); - assert!( - refused.contains(&format!("{} is a repository", root.join("leaf").display())), - "{refused}" - ); - } - } - - /// A report naming one `typosquatting` risk on `left-pad` at `version`. - fn typosquat(version: &str) -> String { - serde_json::json!([{ - "dependency": "left-pad", - "result": { - "errors": {}, "issues": 1, "results": {}, "package_version": version, - "risks": [{"name": "typosquatting", "threat_rule": "typosquatting"}], - }, - }]) - .to_string() - } - - /// `guarddog_read` over [`typosquat`] at `version`, against the root's - /// `root` and the enclosing repositories' `nested`, innermost first, and - /// what it recorded. - fn judged(version: &str, root: &[Waiver], nested: &[std::rc::Rc]) -> (Read, Ledger) { - let mut ledger = Ledger::default(); - let read = guarddog_read( - "outer/inner/web", - "npm", - &BTreeSet::from([String::from("left-pad")]), - &Waivers { root, nested }, - &mut ledger, - Some(0), - &typosquat(version), - "", - &mut |_, _| Err(String::from("no registry evidence in this fixture")), - ); - (read, ledger) - } - - /// A nested repository at `source` waiving `left-pad` 1.3.0 for `reason`. - fn owning(source: &str, reason: &str) -> std::rc::Rc { - std::rc::Rc::new(Nested { - source: String::from(source), - waive: vec![waiver("npm:left-pad@1.3.0", "typosquatting", reason)], - }) - } - - /// The innermost row covers the risk, and every outer row that also - /// covers it -- a nested repository's or the root's -- is recorded as its - /// repeat rather than as a match, so the stale report can say the copy may - /// go. - #[test] - fn the_innermost_waiver_covers_first_and_every_outer_row_for_the_same_risk_repeats_it() { - let inner = owning("outer/inner/policy/principles.toml", "the inner's"); - let outer = owning("outer/policy/principles.toml", "the outer's"); - let root = [waiver("npm:left-pad@1.3.0", "typosquatting", "the copy")]; - let (all, copied) = judged("1.3.0", &root, &[inner.clone(), outer.clone()]); - assert!(matches!(all, Read::Clean)); - assert_eq!( - copied.matched, - BTreeSet::from([(Some(inner.source.clone()), 0)]) - ); - let repeats: Vec<(Option<&str>, &str)> = copied - .duplicated - .iter() - .map(|((source, _), repeat)| (source.as_deref(), repeat.of.as_str())) - .collect(); - assert_eq!( - repeats, - vec![ - (None, "outer/inner/policy/principles.toml"), - ( - Some("outer/policy/principles.toml"), - "outer/inner/policy/principles.toml" - ), - ] - ); - - // An outer repository's row covers what the inner one has no row for. - let bare = std::rc::Rc::new(Nested { - source: String::from("outer/inner/policy/principles.toml"), - waive: vec![], - }); - let (through, outer_only) = judged("1.3.0", &[], &[bare, outer.clone()]); - assert!(matches!(through, Read::Clean)); - assert_eq!( - outer_only.matched, - BTreeSet::from([(Some(outer.source.clone()), 0)]) - ); - - // Keyed on the version as a root row is: the next release is read - // fresh, whoever wrote the row. - let (next, fresh) = judged("1.3.1", &[], &[inner, outer]); - assert!(matches!(next, Read::Risks(..))); - assert!(fresh.matched.is_empty() && fresh.duplicated.is_empty()); - - // And with no nested repository, the root's row is the match it was. - let (alone, rooted) = judged("1.3.0", &root, &[]); - assert!(matches!(alone, Read::Clean)); - assert_eq!(rooted.matched, BTreeSet::from([(None, 0)])); - assert!(rooted.duplicated.is_empty()); - } - - /// guarddog unpacks under the cache directory, whatever `TMPDIR` says: the - /// order its `metadata_mismatch` rule lists an unpacked wheel in is that - /// filesystem's, so `TMPDIR` would otherwise be the verdict's input. - #[test] - fn guarddog_unpacks_under_the_cache_directory_and_never_under_tmpdir() { - let under = |xdg: Option<&str>, home: Option<&str>| { - guarddog_unpacks_under(|name| match name { - "XDG_CACHE_HOME" => xdg.map(std::path::PathBuf::from), - "HOME" => home.map(std::path::PathBuf::from), - "TMPDIR" => Some(std::path::PathBuf::from("/tmp/elsewhere")), - _ => None, - }) - }; - assert_eq!( - under(Some("/x/cache"), Some("/srv/example/user")).as_deref(), - Some(std::path::Path::new("/x/cache/uphold/guarddog")) - ); - // Empty and relative are both "not set": a relative one would unpack - // downloads into the manifest's directory, which guarddog runs in. - for xdg in [None, Some(""), Some("cache")] { - assert_eq!( - under(xdg, Some("/srv/example/user")).as_deref(), - Some(std::path::Path::new( - "/srv/example/user/.cache/uphold/guarddog" - )), - "{xdg:?}" - ); - } - assert_eq!(under(None, None), None); - assert_eq!(under(Some("cache"), Some("home")), None); - } /// Every floor, for the tests that hold them all to one rule. - const FLOORS: [&Floor; 5] = [ - &OSV_FLOOR, - &ZIZMOR_FLOOR, - &DENY_FLOOR, - &VET_FLOOR, - &GUARDDOG_FLOOR, - ]; + const FLOORS: [&Floor; 4] = [&OSV_FLOOR, &ZIZMOR_FLOOR, &DENY_FLOOR, &VET_FLOOR]; /// What each scanner printed for its version command on the host the /// readers were measured on, byte for byte, paired with the floor it /// answers. osv-scanner's is the multi-line one: its own version first, /// then the osv-scalibr library's, which is not the tool's. - const REAL_ANSWERS: [(&Floor, &str, Version); 5] = [ + const REAL_ANSWERS: [(&Floor, &str, Version); 4] = [ ( &OSV_FLOOR, "osv-scanner version: 2.5.1\nosv-scalibr version: 0.5.2\ncommit: n/a\nbuilt at: n/a\n", @@ -3440,7 +1969,6 @@ mod tests { (&ZIZMOR_FLOOR, "zizmor 1.30.0\n", Version(1, 30, 0)), (&DENY_FLOOR, "cargo-deny 0.20.2\n", Version(0, 20, 2)), (&VET_FLOOR, "cargo-vet 0.10.2\n", Version(0, 10, 2)), - (&GUARDDOG_FLOOR, "3.2.0\n", Version(3, 2, 0)), ]; #[test] @@ -3468,8 +1996,8 @@ mod tests { assert_eq!(parse_version("v2.5.1"), Some(Version(2, 5, 1))); assert_eq!(parse_version("zizmor 1.30.0-rc1"), Some(Version(1, 30, 0))); assert_eq!( - parse_version("guarddog 3.2.0+local"), - Some(Version(3, 2, 0)) + parse_version("zizmor 1.30.0+local"), + Some(Version(1, 30, 0)) ); // Newer on every axis passes. for said in ["zizmor 1.30.1", "zizmor 1.31.0", "zizmor 2.0.0"] { @@ -3490,7 +2018,6 @@ mod tests { ), (&DENY_FLOOR, "cargo-deny 0.19.0"), (&VET_FLOOR, "cargo-vet 0.10.1"), - (&GUARDDOG_FLOOR, "2.9.0"), ] { let reason = against_floor(floor, said).unwrap_or_default(); let found = parse_version(said).unwrap().to_string(); @@ -3507,7 +2034,7 @@ mod tests { fn a_version_answer_this_cannot_read_is_refused_rather_than_trusted() { for said in [ "", - "guarddog", + "zizmor", "3.2", "zizmor 1.x.0", "zizmor 1.30.0.1", diff --git a/src/supply/pypi.rs b/src/supply/pypi.rs deleted file mode 100644 index e76668b..0000000 --- a/src/supply/pypi.rs +++ /dev/null @@ -1,293 +0,0 @@ -//! Evidence for guarddog's comparison of optional extras with required dependencies. -//! -//! The report drops markers. Only the exact release's `requires_dist` can -//! establish that every occurrence of a reported name requires an extra. - -use std::collections::{BTreeMap, BTreeSet}; -use std::time::Duration; - -use pyreq::Requirement; -use serde_json::Value; - -/// The release endpoint, rather than the project endpoint, binds the evidence -/// to what guarddog scanned. `PyPI` documents it at . -fn release_url(name: &str, version: &str) -> Option { - if name.is_empty() { - return None; - } - let name = package_name(name)?; - // Versions are a single URL segment, never a caller-selected destination. - if !version.bytes().any(|byte| byte.is_ascii_digit()) - || !version - .bytes() - .all(|byte| byte.is_ascii_alphanumeric() || b".!+_-".contains(&byte)) - { - return None; - } - Some(format!("https://pypi.org/pypi/{name}/{version}/json")) -} - -/// Fetch bounded, exact-release registry evidence. Failure leaves the scanner's -/// risk available to the ordinary finding and waiver handling. -pub(super) fn fetch(name: &str, version: &str) -> Result { - let url = release_url(name, version).ok_or("invalid release identity")?; - let agent: ureq::Agent = ureq::Agent::config_builder() - .https_only(true) - .max_redirects(0) - .timeout_global(Some(Duration::from_secs(15))) - .build() - .into(); - read_response(agent.get(&url).call().map_err(|error| error.to_string())?) -} - -/// Redirect bodies and partial responses cannot supply release evidence even -/// when their JSON has the expected fields. -fn read_response(mut response: ureq::http::Response) -> Result { - if response.status() != 200 { - return Err(format!("unexpected PyPI HTTP status {}", response.status())); - } - let body = response - .body_mut() - .with_config() - .limit(2 * 1024 * 1024) - .read_to_string() - .map_err(|error| error.to_string())?; - serde_json::from_str(&body).map_err(|error| error.to_string()) -} - -/// Only the one-sided, names-only report has a meaning this evidence proves. -/// Unknown blocks, malformed names and duplicate normalized names stay findings. -pub(super) fn differences(description: &str) -> Option> { - let mut lines = description.lines(); - if lines.next()? != "Dependency mismatch between PyPI metadata and pyproject.toml:" { - return None; - } - let names = lines - .next()? - .strip_prefix(" In PyPI metadata but NOT in pyproject.toml: ")?; - if lines.next().is_some() { - return None; - } - let mut parsed = BTreeSet::new(); - for name in names.split(", ") { - if name.is_empty() || !parsed.insert(package_name(name)?) { - return None; - } - } - (!parsed.is_empty()).then_some(parsed) -} - -/// Names are parsed by the requirement reader, then constrained to the bare -/// name form so URL segments and report entries cannot carry extra syntax. -fn package_name(name: &str) -> Option { - let requirement = Requirement::parse(name).ok()?; - (requirement.name == name).then(|| super::references::normalise(name)) -} - -/// Confirm every reported name occurs only under a recognized optional-extra -/// marker in the named release. Any other occurrence prevents tolerance. -pub(super) fn confirms( - metadata: &Value, - name: &str, - version: &str, - differences: &BTreeSet, -) -> Option { - let info = metadata.get("info")?; - if package_name(info.get("name")?.as_str()?)? != package_name(name)? - || info.get("version")?.as_str()? != version - { - return None; - } - // The closed normalization recognizes a simple positive equality. More - // expressive markers stay findings; no host environment is guessed. - let extra_marker = regex::Regex::new( - r#"^(?:extra == "[A-Za-z0-9][A-Za-z0-9._-]*"|"[A-Za-z0-9][A-Za-z0-9._-]*" == extra)$"#, - ) - .ok()?; - let mut optional = BTreeMap::new(); - for raw in info.get("requires_dist")?.as_array()? { - let requirement = Requirement::parse(raw.as_str()?).ok()?; - let extra_only = requirement - .marker - .as_ref() - .is_some_and(|marker| extra_marker.is_match(&marker.to_string())); - optional - .entry(package_name(&requirement.name)?) - .and_modify(|all| *all &= extra_only) - .or_insert(extra_only); - } - if !differences - .iter() - .all(|dependency| optional.get(dependency) == Some(&true)) - { - return None; - } - Some(format!( - "{} ({})", - super::Normalization::OptionalExtras.name(), - differences - .iter() - .map(ToString::to_string) - .collect::>() - .join(", ") - )) -} - -#[cfg(test)] -mod tests { - use super::{confirms, differences, read_response, release_url}; - use serde_json::json; - use std::io::Write as _; - - const DESCRIPTION: &str = "Dependency mismatch between PyPI metadata and pyproject.toml:\n In PyPI metadata but NOT in pyproject.toml: pyarrow\n"; - - /// Exercise the production response reader over a local HTTP connection; - /// production URL selection always uses HTTPS at the fixed registry host. - fn response(status: u16, body: &str) -> Result { - let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); - let address = listener.local_addr().unwrap(); - let body = body.to_owned(); - let server = std::thread::spawn(move || { - let (mut stream, _) = listener.accept().unwrap(); - write!(stream, "HTTP/1.1 {status} Fixture\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", body.len()).ok(); - }); - let agent: ureq::Agent = ureq::Agent::config_builder() - .max_redirects(0) - .http_status_as_error(false) - .timeout_global(Some(std::time::Duration::from_secs(5))) - .build() - .into(); - let result = read_response(agent.get(format!("http://{address}/")).call().unwrap()); - server.join().unwrap(); - result - } - - #[test] - fn only_a_complete_bounded_json_response_can_be_evidence() { - assert_eq!(response(200, "{}"), Ok(json!({}))); - for status in [206, 301, 302, 307, 404, 500] { - assert!(response(status, "{}").is_err(), "{status}"); - } - assert!(response(200, "not json").is_err()); - assert!(response(200, &" ".repeat(2 * 1024 * 1024 + 1)).is_err()); - } - - #[test] - fn the_recorded_pandas_report_is_explained_by_its_release_metadata() { - let report: serde_json::Value = serde_json::from_str(include_str!( - "../../tests/fixtures/guarddog/pypi-pandas-3.0.3.json" - )) - .unwrap(); - let metadata: serde_json::Value = serde_json::from_str(include_str!( - "../../tests/fixtures/guarddog/pypi-pandas-3.0.3-metadata.json" - )) - .unwrap(); - let names = differences( - report[0]["result"]["results"]["metadata_mismatch"] - .as_str() - .unwrap(), - ) - .unwrap(); - assert!(confirms(&metadata, "pandas", "3.0.3", &names).is_some()); - } - - #[test] - fn every_occurrence_must_require_a_positive_extra() { - let names = differences(DESCRIPTION).unwrap(); - for (requirements, accepted) in [ - (vec!["pyarrow>=13; extra == 'parquet'"], true), - ( - vec!["pyarrow; python_version >= '3.10' and extra == 'parquet'"], - false, - ), - (vec!["pyarrow; 'parquet' == extra"], true), - ( - vec!["pyarrow; extra == 'parquet'", "pyarrow; extra == 'test'"], - true, - ), - ( - vec![ - "pyarrow; extra == 'parquet'", - "pyarrow; sys_platform == 'win32'", - ], - false, - ), - ( - vec!["pyarrow; extra == 'parquet' or python_version < '3.10'"], - false, - ), - (vec!["pyarrow; extra != 'parquet'"], false), - (vec!["pyarrow"], false), - (vec!["another; extra == 'parquet'"], false), - (vec!["pyarrow; extra =="], false), - (vec![], false), - ] { - let metadata = json!({"info": {"name": "example", "version": "1.0", "requires_dist": requirements}}); - assert_eq!( - confirms(&metadata, "example", "1.0", &names).is_some(), - accepted, - "{metadata}" - ); - } - } - - #[test] - fn evidence_must_name_the_scanned_release_and_every_difference() { - let names = differences(DESCRIPTION).unwrap(); - for metadata in [ - json!({"info": {"name": "different", "version": "1.0", "requires_dist": ["pyarrow; extra == 'x'"]}}), - json!({"info": {"name": "example", "version": "2.0", "requires_dist": ["pyarrow; extra == 'x'"]}}), - json!({"info": {"name": "example", "version": "1.0", "requires_dist": null}}), - json!({"info": {"name": "example", "version": "1.0", "requires_dist": [42]}}), - json!({}), - ] { - assert!(confirms(&metadata, "example", "1.0", &names).is_none()); - } - let two = differences(&DESCRIPTION.replace("pyarrow\n", "pyarrow, injected\n")).unwrap(); - let metadata = json!({"info": {"name": "Example_Kit", "version": "1.0", "requires_dist": ["PyArrow; extra == 'x'"]}}); - assert!(confirms(&metadata, "example-kit", "1.0", &names).is_some()); - assert!(confirms(&metadata, "example-kit", "1.0", &two).is_none()); - } - - #[test] - fn unknown_or_ambiguous_report_shapes_are_not_candidates() { - for description in [ - DESCRIPTION.replace("pyarrow", ""), - DESCRIPTION.replace("pyarrow", "pyarrow, PyArrow"), - DESCRIPTION.replace("pyarrow", "pyarrow>=13"), - DESCRIPTION.replace( - "pyarrow", - "pyarrow, injected\n In pyproject.toml but NOT in PyPI metadata: removed", - ), - format!("{DESCRIPTION}unrecognized difference\n"), - format!("{DESCRIPTION}\n"), - ] { - assert!(differences(&description).is_none(), "{description}"); - } - } - - #[test] - fn release_identity_cannot_change_the_destination() { - assert_eq!( - release_url("Example_Kit", "1.0+local"), - Some(String::from( - "https://pypi.org/pypi/example-kit/1.0+local/json" - )) - ); - for name in ["../evil", "https://elsewhere", "x?y", "x#y", ""] { - assert!(release_url(name, "1.0").is_none()); - } - for version in [ - "../evil", - "1.0/anything", - "1.0?x", - "1.0#x", - "%2f", - ".", - "..", - "", - ] { - assert!(release_url("example", version).is_none()); - } - } -} diff --git a/src/supply/references.rs b/src/supply/references.rs index 99faa16..910e554 100644 --- a/src/supply/references.rs +++ b/src/supply/references.rs @@ -1,29 +1,23 @@ -//! What in a `uv export` guarddog can look up, and what it cannot. +//! The git sources a lock or a manifest names, and what is done with them. //! -//! `guarddog pypi verify` looks every requirement up on `PyPI` by name. A -//! requirement that is a DIRECT REFERENCE -- PEP 508 `name @ `, which is +//! A requirement that is a DIRECT REFERENCE -- PEP 508 `name @ `, which is //! how `uv export` spells a uv git source, a direct URL and a `file://` path -- -//! names something `PyPI` never held, so guarddog answers a 404, and the section -//! read that as could-not-look. Every repository depending on its own -//! first-party package through a git source was exit 2 on every push, for a -//! question addressed to the wrong registry. -//! -//! So the export is sorted before guarddog sees it, and nothing is dropped -//! silently. What resolves from an index goes to guarddog unchanged. A path -//! inside the tree -- a workspace member, the project itself -- goes to it -//! unchanged too, as it always did. A git source on the repository's own forge, -//! under the owner the policy DECLARES, is asked of its remote instead: the -//! locked commit must be what some ref there points at, and a tag the lock -//! names must point at that commit. Everything else is refused by name: a git -//! source under anybody else, a direct URL, a path outside the tree. Those are -//! dependencies no scanner here can vouch for, and skipping them would be a -//! pass nobody earned. +//! names something no index holds, so no registry and no scanner here can vouch +//! for it. So the export is sorted, and nothing is dropped silently. What +//! resolves from an index is not this section's. A path inside the tree -- a +//! workspace member, the project itself -- is the repository's own. A git +//! source on the repository's own forge, under the owner the policy DECLARES, is +//! asked of its remote: the locked commit must be what some ref there points +//! at, and a tag the lock names must point at that commit. Everything else is +//! refused by name: a git source under anybody else, a direct URL, a path +//! outside the tree. Those are dependencies nothing here can vouch for, and +//! skipping them would be a pass nobody earned. //! //! The owner is the policy's declared one and never `origin`'s, for the reason //! [`crate::config::PolicyFile::owner`] gives: a first-party set read off the //! remote is repointed by the same command that repoints the remote. -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::BTreeMap; use std::path::Path; use std::process::Stdio; @@ -50,14 +44,6 @@ pub(super) struct GitPin { /// One `uv export`, sorted. #[derive(Debug, Default)] pub(super) struct Sorted { - /// The export with every direct reference outside the tree taken out: what - /// guarddog is handed. - pub kept: String, - /// How many of the kept lines are requirements an index resolves, which is - /// what guarddog can look up. Zero is a run with nothing to ask it. - pub indexed: usize, - /// Canonical index package names that guarddog must report on. - pub expected: BTreeSet, /// Git sources, for the owner check and the remote. pub git: Vec, /// Direct references refused as they stand, each a line naming it. @@ -74,36 +60,24 @@ pub(super) fn sort(export: &str, directory: &Path, root: &Path, lock: Option<&st let mut sorted = Sorted::default(); for line in export.lines() { let trimmed = line.trim(); - let keep = if trimmed.is_empty() || trimmed.starts_with('#') || line.starts_with(' ') { - true - } else if let Some(path) = trimmed.strip_prefix("-e ").map(str::trim) { - path_inside(path, directory, root, &mut sorted) + if trimmed.is_empty() || trimmed.starts_with('#') || line.starts_with(' ') { + continue; + } + if let Some(path) = trimmed.strip_prefix("-e ").map(str::trim) { + path_inside(path, directory, root, &mut sorted); } else if trimmed.starts_with('-') { - true + // An option line of the export, not a requirement. } else if is_path(trimmed) { - path_inside(trimmed, directory, root, &mut sorted) + path_inside(trimmed, directory, root, &mut sorted); } else if let Some((name, reference)) = direct_reference(trimmed) { - by_reference(name, reference, directory, root, &tags, &mut sorted) - } else { - sorted.indexed += 1; - let name = trimmed - .split(|character: char| { - !character.is_ascii_alphanumeric() && !matches!(character, '-' | '_' | '.') - }) - .next() - .unwrap_or(trimmed); - sorted.expected.insert(normalise(name)); - true - }; - if keep { - sorted.kept.push_str(line); - sorted.kept.push('\n'); + by_reference(name, reference, directory, root, &tags, &mut sorted); } + // Anything else is an index requirement, which is not this check's. } sorted } -/// A direct reference, sorted: whether its line stays in what guarddog reads. +/// A direct reference, sorted. fn by_reference( name: &str, reference: &str, @@ -111,7 +85,7 @@ fn by_reference( root: &Path, tags: &BTreeMap<(String, String), String>, sorted: &mut Sorted, -) -> bool { +) { if let Some(git) = reference.strip_prefix("git+") { match split_git(git) { Some((remote, commit)) => { @@ -129,35 +103,34 @@ fn by_reference( no pin to check" )), } - return false; + return; } if let Some(path) = reference.strip_prefix("file://") { - return path_inside(path, directory, root, sorted); + path_inside(path, directory, root, sorted); + return; } sorted.refused.push(format!( - "{name} is a direct URL ({reference}): no index holds it, so guarddog cannot look it \ - up and nothing here vouches for it" + "{name} is a direct URL ({reference}): no index holds it and nothing here vouches \ + for it" )); - false } -/// Whether a path requirement stays: inside the tree it does, as it always -/// did; outside, it is refused by name. +/// A path requirement: inside the tree it is the repository's own; outside, it +/// is refused by name. /// /// Both sides are resolved through the filesystem, so a link inside the tree /// that points out of it is outside. A path that does not resolve is refused /// too: nothing can say where it lands. -fn path_inside(path: &str, directory: &Path, root: &Path, sorted: &mut Sorted) -> bool { +fn path_inside(path: &str, directory: &Path, root: &Path, sorted: &mut Sorted) { let joined = directory.join(path); match (joined.canonicalize(), root.canonicalize()) { - (Ok(target), Ok(tree)) if target.starts_with(&tree) => true, + (Ok(target), Ok(tree)) if target.starts_with(&tree) => {} (Ok(target), Ok(_)) => { sorted.refused.push(format!( "{path} is a path dependency outside this repository ({}): no index holds it \ and no scanner here reads it", target.display() )); - false } (Err(error), _) | (_, Err(error)) => { sorted.refused.push(format!( @@ -165,7 +138,6 @@ fn path_inside(path: &str, directory: &Path, root: &Path, sorted: &mut Sorted) - whether it is inside this repository cannot be said", directory.display() )); - false } } } @@ -255,122 +227,62 @@ fn tags_in_lock(lock: &str) -> BTreeMap<(String, String), String> { /// One `package.json`, sorted the same way. /// -/// `guarddog npm verify` reads a manifest's `dependencies` and asks npm for -/// each by name, so a git dependency -- `git+https://…#v0.1.0`, -/// `github:owner/repo`, `owner/repo` -- is a 404 there just as a uv git source -/// is on `PyPI`. The git dependencies come out for the owner check and the -/// remote; the rest of the manifest is handed to guarddog unchanged. +/// A git dependency -- `git+https://...#v0.1.0`, `github:owner/repo`, +/// `owner/repo` -- is held by no registry, just as a uv git source is held by no +/// index. The git dependencies come out for the owner check and the remote. #[derive(Debug, Default)] pub(super) struct NpmSorted { - /// The manifest without its git dependencies, where it had any: what - /// guarddog is handed instead of the file. - pub kept: Option, - /// How many dependencies are left for npm to resolve. - pub indexed: usize, - /// Real npm package names, with aliases merged as guarddog merges them. - pub expected: BTreeSet, - /// Whether the manifest parsed as a JSON object, so that `indexed` counts - /// its `dependencies` -- none at all, or only `devDependencies`, being - /// zero -- rather than a file nobody could read. - pub read: bool, /// Git dependencies, for the owner check and the remote. pub git: Vec, /// Git dependencies refused as they stand, each a line naming it. pub refused: Vec, } -/// Sort a `package.json`'s `dependencies`, and its `devDependencies` too where -/// `include_dev` says guarddog reads them -/// (`GUARDDOG_NPM_INCLUDE_DEV_DEPENDENCIES=true`). `locks` are the texts of the +/// Sort a `package.json`'s `dependencies`. `locks` are the texts of the /// `bun.lock` and `package-lock.json` files that may record what each git -/// dependency resolved to, nearest first. A manifest that does not parse is -/// left whole, for guarddog to say so. -pub(super) fn sort_npm(manifest: &str, locks: &[String], include_dev: bool) -> NpmSorted { +/// dependency resolved to, nearest first. A manifest that does not parse has +/// no git dependency to find. +pub(super) fn sort_npm(manifest: &str, locks: &[String]) -> NpmSorted { let mut sorted = NpmSorted::default(); - let Ok(serde_json::Value::Object(mut parsed)) = serde_json::from_str(manifest) else { + let Ok(serde_json::Value::Object(parsed)) = serde_json::from_str(manifest) else { return sorted; }; - sorted.read = true; - let groups: &[&str] = if include_dev { - &["dependencies", "devDependencies"] - } else { - &["dependencies"] + let Some(serde_json::Value::Object(dependencies)) = parsed.get("dependencies") else { + return sorted; }; - let mut taken = false; - for group in groups { - let Some(serde_json::Value::Object(dependencies)) = parsed.get_mut(*group) else { + for (name, spec) in dependencies { + let Some((remote, fragment)) = spec.as_str().and_then(npm_git) else { continue; }; - dependencies.retain(|name, spec| { - let Some((remote, fragment)) = spec.as_str().and_then(npm_git) else { - return true; - }; - taken = true; - let fragment = fragment.filter(|fragment| !fragment.starts_with("semver:")); - let commit = locks - .iter() - .find_map(|lock| locked_commit(lock, name)) - .or_else(|| { - fragment - .filter(|fragment| is_commit(fragment)) - .map(str::to_owned) - }); - match commit { - Some(commit) => sorted.git.push(GitPin { - name: name.clone(), - remote, - tag: fragment - .filter(|fragment| !commit.starts_with(*fragment)) - .map(str::to_owned), - commit, - committish: true, - }), - None => sorted.refused.push(format!( - "{name} is a git dependency ({remote}), and no bun.lock or \ - package-lock.json records the commit it resolved to, so there is no \ - pin to check" - )), - } - false - }); - sorted.indexed += dependencies.len(); - for (name, spec) in dependencies.iter() { - sorted - .expected - .insert(npm_identity(name, spec.as_str()).to_owned()); + let fragment = fragment.filter(|fragment| !fragment.starts_with("semver:")); + let commit = locks + .iter() + .find_map(|lock| locked_commit(lock, name)) + .or_else(|| { + fragment + .filter(|fragment| is_commit(fragment)) + .map(str::to_owned) + }); + match commit { + Some(commit) => sorted.git.push(GitPin { + name: name.clone(), + remote, + tag: fragment + .filter(|fragment| !commit.starts_with(*fragment)) + .map(str::to_owned), + commit, + committish: true, + }), + None => sorted.refused.push(format!( + "{name} is a git dependency ({remote}), and no bun.lock or \ + package-lock.json records the commit it resolved to, so there is no \ + pin to check" + )), } } - if taken { - sorted.kept = serde_json::to_string(&parsed).ok(); - } sorted } -/// The package guarddog 3.2.0's `resolve_npm_alias` asks the registry for. -/// Invalid alias spellings stay under their manifest name, as they do there. -fn npm_identity<'a>(name: &'a str, spec: Option<&'a str>) -> &'a str { - let Some(alias) = spec.and_then(|spec| spec.strip_prefix("npm:")) else { - return name; - }; - let after_scope = alias.strip_prefix('@').unwrap_or(alias); - let package_end = after_scope.find('@').map_or(alias.len(), |index| { - index + usize::from(alias.starts_with('@')) - }); - let Some(package) = alias.get(..package_end) else { - return name; - }; - let valid = !package.is_empty() - && !package.chars().any(char::is_whitespace) - && (!alias.starts_with('@') - || package.strip_prefix('@').is_some_and(|scoped| { - scoped.split_once('/').is_some_and(|(scope, scoped_name)| { - !scope.is_empty() && !scoped_name.is_empty() && !scope.contains('/') - }) - })) - && alias.get(package_end..) != Some("@"); - if valid { package } else { name } -} - /// The remote and the `#` fragment of an npm git dependency, or `None` for /// anything npm resolves from its registry, a path or a tarball URL. fn npm_git(spec: &str) -> Option<(String, Option<&str>)> { @@ -514,8 +426,8 @@ pub(super) fn check( if !ours { return Checked::Refused(format!( "{name} is a git source under {}/{}, not under {}/{owner}, which is this \ - repository's declared owner: no registry holds it, so guarddog cannot look it \ - up, and nothing here vouches for it", + repository's declared owner: no registry holds it, and nothing here \ + vouches for it", host.as_deref().unwrap_or("no host"), under.unwrap_or("no owner"), first_party.host @@ -637,7 +549,7 @@ mod tests { const OTHER: &str = "0000000000000000000000000000000000000001"; #[test] - fn an_export_is_sorted_into_index_tree_git_and_refused() { + fn an_export_is_sorted_into_git_and_refused() { let base = crate::fixture::scratch("supply-references"); let root = base.join("repo"); let service = root.join("service"); @@ -659,14 +571,6 @@ mod tests { service.join("vendor-copy").display() ); let sorted = sort(&export, &service, &root, None); - assert_eq!(sorted.indexed, 2); - assert!(sorted.kept.contains("-e ./packages/member")); - assert!(sorted.kept.contains("./libs/inner")); - assert!(sorted.kept.contains("local-dep @ file://")); - assert!(!sorted.kept.contains("example-kit")); - assert!(!sorted.kept.contains("wheel-dep")); - assert!(!sorted.kept.contains("elsewhere")); - assert!(!sorted.kept.contains("not-there")); assert_eq!( sorted.git, vec![GitPin { @@ -702,7 +606,6 @@ mod tests { std::fs::create_dir_all(base.join("outside")).unwrap(); std::os::unix::fs::symlink(base.join("outside"), root.join("looks-inside")).unwrap(); let sorted = sort("-e ./looks-inside\n", &root, &root, None); - assert!(sorted.kept.is_empty(), "{}", sorted.kept); assert_eq!(sorted.refused.len(), 1, "{:?}", sorted.refused); } @@ -818,7 +721,7 @@ mod tests { } #[test] - fn a_manifest_loses_its_git_dependencies_to_the_lock_and_keeps_the_rest() { + fn a_manifest_git_dependency_takes_its_commit_from_the_lock() { let manifest = "{\"name\":\"app\",\"dependencies\":{\ \"@example-org/kit\":\"git+https://github.com/example-org/kit.git#v0.1.0\",\ \"loose\":\"github:example-org/loose\",\ @@ -827,8 +730,7 @@ mod tests { let bun = format!( "{{\n \"packages\": {{\n \"@example-org/kit\": [\"@example-org/kit@git+https://github.com/example-org/kit.git#{COMMIT}\", {{}}, \"{COMMIT}\"],\n }}\n}}\n" ); - let sorted = sort_npm(manifest, &[bun], false); - assert_eq!(sorted.indexed, 1); + let sorted = sort_npm(manifest, &[bun]); assert_eq!( sorted.git, vec![GitPin { @@ -841,60 +743,30 @@ mod tests { ); assert_eq!(sorted.refused.len(), 1, "{:?}", sorted.refused); assert!(sorted.refused[0].contains("loose is a git dependency")); - let kept = sorted.kept.unwrap(); - assert!(kept.contains("\"cookie\""), "{kept}"); - assert!(kept.contains("\"jose\""), "{kept}"); - assert!(!kept.contains("example-org"), "{kept}"); } #[test] - fn a_package_lock_supplies_the_commit_and_a_registry_only_manifest_is_left_whole() { + fn a_package_lock_supplies_the_commit_and_a_registry_only_manifest_has_no_pin() { let manifest = "{\"dependencies\":{\"kit\":\"example-org/kit#main\"}}"; let lock = format!( "{{\"packages\":{{\"node_modules/kit\":{{\"resolved\":\"git+ssh://git@github.com/example-org/kit.git#{COMMIT}\"}}}}}}" ); - let sorted = sort_npm(manifest, &[lock], false); - assert_eq!(sorted.indexed, 0); + let sorted = sort_npm(manifest, &[lock]); assert_eq!(sorted.git.len(), 1); assert_eq!(sorted.git[0].tag.as_deref(), Some("main")); - let whole = sort_npm("{\"dependencies\":{\"cookie\":\"^1.1.1\"}}", &[], false); - assert!(whole.kept.is_none() && whole.git.is_empty() && whole.refused.is_empty()); - assert!(whole.read && whole.indexed == 1); - let unread = sort_npm("not json", &[], false); - assert!(unread.kept.is_none() && !unread.read); + let whole = sort_npm("{\"dependencies\":{\"cookie\":\"^1.1.1\"}}", &[]); + assert!(whole.git.is_empty() && whole.refused.is_empty()); + let unread = sort_npm("not json", &[]); + assert!(unread.git.is_empty() && unread.refused.is_empty()); } #[test] - fn a_manifest_with_only_dev_dependencies_leaves_nothing_for_npm() { - // guarddog reads `dependencies` alone and answers `[]` for a manifest - // without them, which the reader takes for a network failure. - let dev = sort_npm("{\"devDependencies\":{\"jose\":\"^6\"}}", &[], false); + fn dev_dependencies_are_not_read() { + let manifest = "{\"devDependencies\":{\"loose\":\"github:example-org/loose\"}}"; + let sorted = sort_npm(manifest, &[]); assert!( - dev.read && dev.indexed == 0 && dev.kept.is_none(), - "{dev:?}" - ); - let empty = sort_npm( - "{\"dependencies\":{},\"devDependencies\":{\"jose\":\"^6\"}}", - &[], - false, - ); - assert!( - empty.read && empty.indexed == 0 && empty.kept.is_none(), - "{empty:?}" - ); - } - - #[test] - fn dev_dependencies_count_and_lose_their_git_sources_where_guarddog_reads_them() { - let manifest = "{\"devDependencies\":{\"jose\":\"^6\",\ - \"loose\":\"github:example-org/loose\"}}"; - let dev = sort_npm(manifest, &[], true); - assert_eq!(dev.indexed, 1, "{dev:?}"); - assert_eq!(dev.refused.len(), 1, "{dev:?}"); - let kept = dev.kept.unwrap(); - assert!( - kept.contains("\"jose\"") && !kept.contains("loose"), - "{kept}" + sorted.git.is_empty() && sorted.refused.is_empty(), + "{sorted:?}" ); } @@ -930,11 +802,7 @@ mod tests { "git+--upload-pack=touch {};git-upload-pack://github.com/example-org/kit#{COMMIT}", probe.display() ); - let sorted = sort_npm( - &format!("{{\"dependencies\":{{\"kit\":\"{spec}\"}}}}"), - &[], - false, - ); + let sorted = sort_npm(&format!("{{\"dependencies\":{{\"kit\":\"{spec}\"}}}}"), &[]); assert_eq!(sorted.git.len(), 1, "{:?}", sorted.refused); let first_party = FirstParty { owner: Ok(Some(String::from("example-org"))), diff --git a/tests/fixtures/guarddog/npm-react-dom-19.2.0.json b/tests/fixtures/guarddog/npm-react-dom-19.2.0.json deleted file mode 100644 index 793bbaf..0000000 --- a/tests/fixtures/guarddog/npm-react-dom-19.2.0.json +++ /dev/null @@ -1 +0,0 @@ -[{"dependency": "react-dom", "version": "19.2.0", "result": {"issues": 1, "errors": {}, "results": {"metadata_mismatch": "Difference between manifest and package.json found: \nscripts: \n start: Manifest(\"node server.js\"), package.json(\"None\") \n"}, "path": "/srv/example/guarddog/tmpdir/react-dom", "risk_score": {"score": 6.3, "label": "suspicious", "findings_count": 1, "score_breakdown": {"severity_component": 0.2, "chain_component": 0.08, "specificity_component": 0.21, "sophistication_component": 0.14, "num_stages": 1, "has_source_code_risks": false, "max_severity": 2, "dominant_specificity": "medium", "dominant_sophistication": "medium"}}, "risks": [{"name": "risk.metadata.manifest-mismatch", "category": "metadata", "severity": "medium", "mitre_tactics": ["execution"], "threat_identifies": "threat.metadata.manifest-mismatch", "threat_rule": "metadata_mismatch", "threat_description": "Difference between manifest and package.json found: \nscripts: \n start: Manifest(\"node server.js\"), package.json(\"None\") \n", "threat_location": "", "threat_code": "", "threat_match": "", "capability_identifies": null, "capability_rule": null, "file_path": ""}], "package_version": "19.2.0"}}] diff --git a/tests/fixtures/guarddog/npm-swagger-ui-express-5.0.1.json b/tests/fixtures/guarddog/npm-swagger-ui-express-5.0.1.json deleted file mode 100644 index 1f573d1..0000000 --- a/tests/fixtures/guarddog/npm-swagger-ui-express-5.0.1.json +++ /dev/null @@ -1 +0,0 @@ -[{"dependency": "swagger-ui-express", "version": "5.0.1", "result": {"issues": 1, "errors": {}, "results": {"metadata_mismatch": "Difference between manifest and package.json found: \nscripts: \n test: Manifest(\"mocha test/*.spec.js\"), package.json(\"./node_modules/.bin/mocha test/*.spec.js\") \n"}, "path": "/srv/example/guarddog/tmpdir/swagger-ui-express", "risk_score": {"score": 6.3, "label": "suspicious", "findings_count": 1, "score_breakdown": {"severity_component": 0.2, "chain_component": 0.08, "specificity_component": 0.21, "sophistication_component": 0.14, "num_stages": 1, "has_source_code_risks": false, "max_severity": 2, "dominant_specificity": "medium", "dominant_sophistication": "medium"}}, "risks": [{"name": "risk.metadata.manifest-mismatch", "category": "metadata", "severity": "medium", "mitre_tactics": ["execution"], "threat_identifies": "threat.metadata.manifest-mismatch", "threat_rule": "metadata_mismatch", "threat_description": "Difference between manifest and package.json found: \nscripts: \n test: Manifest(\"mocha test/*.spec.js\"), package.json(\"./node_modules/.bin/mocha test/*.spec.js\") \n", "threat_location": "", "threat_code": "", "threat_match": "", "capability_identifies": null, "capability_rule": null, "file_path": ""}], "package_version": "5.0.1"}}] diff --git a/tests/fixtures/guarddog/pypi-pandas-3.0.3-metadata.json b/tests/fixtures/guarddog/pypi-pandas-3.0.3-metadata.json deleted file mode 100644 index 5eb1a96..0000000 --- a/tests/fixtures/guarddog/pypi-pandas-3.0.3-metadata.json +++ /dev/null @@ -1,92 +0,0 @@ -{ - "info": { - "name": "pandas", - "version": "3.0.3", - "requires_dist": [ - "numpy>=1.26.0; python_version < \"3.14\"", - "numpy>=2.3.3; python_version >= \"3.14\"", - "python-dateutil>=2.8.2", - "tzdata; sys_platform == \"win32\"", - "tzdata; sys_platform == \"emscripten\"", - "hypothesis>=6.116.0; extra == \"test\"", - "pytest>=8.3.4; extra == \"test\"", - "pytest-xdist>=3.6.1; extra == \"test\"", - "pyarrow>=13.0.0; extra == \"pyarrow\"", - "bottleneck>=1.4.2; extra == \"performance\"", - "numba>=0.60.0; extra == \"performance\"", - "numexpr>=2.10.2; extra == \"performance\"", - "scipy>=1.14.1; extra == \"computation\"", - "xarray>=2024.10.0; extra == \"computation\"", - "fsspec>=2024.10.0; extra == \"fss\"", - "s3fs>=2024.10.0; extra == \"aws\"", - "gcsfs>=2024.10.0; extra == \"gcp\"", - "odfpy>=1.4.1; extra == \"excel\"", - "openpyxl>=3.1.5; extra == \"excel\"", - "python-calamine>=0.3.0; extra == \"excel\"", - "pyxlsb>=1.0.10; extra == \"excel\"", - "xlrd>=2.0.1; extra == \"excel\"", - "xlsxwriter>=3.2.0; extra == \"excel\"", - "pyarrow>=13.0.0; extra == \"parquet\"", - "pyarrow>=13.0.0; extra == \"feather\"", - "pyiceberg>=0.8.1; extra == \"iceberg\"", - "tables>=3.10.1; extra == \"hdf5\"", - "pyreadstat>=1.2.8; extra == \"spss\"", - "SQLAlchemy>=2.0.36; extra == \"postgresql\"", - "psycopg2>=2.9.10; extra == \"postgresql\"", - "adbc-driver-postgresql>=1.2.0; extra == \"postgresql\"", - "SQLAlchemy>=2.0.36; extra == \"mysql\"", - "pymysql>=1.1.1; extra == \"mysql\"", - "SQLAlchemy>=2.0.36; extra == \"sql-other\"", - "adbc-driver-postgresql>=1.2.0; extra == \"sql-other\"", - "adbc-driver-sqlite>=1.2.0; extra == \"sql-other\"", - "beautifulsoup4>=4.12.3; extra == \"html\"", - "html5lib>=1.1; extra == \"html\"", - "lxml>=5.3.0; extra == \"html\"", - "lxml>=5.3.0; extra == \"xml\"", - "matplotlib>=3.9.3; extra == \"plot\"", - "jinja2>=3.1.5; extra == \"output-formatting\"", - "tabulate>=0.9.0; extra == \"output-formatting\"", - "PyQt5>=5.15.9; extra == \"clipboard\"", - "qtpy>=2.4.2; extra == \"clipboard\"", - "zstandard>=0.23.0; extra == \"compression\"", - "pytz>=2020.1; extra == \"timezone\"", - "adbc-driver-postgresql>=1.2.0; extra == \"all\"", - "adbc-driver-sqlite>=1.2.0; extra == \"all\"", - "beautifulsoup4>=4.12.3; extra == \"all\"", - "bottleneck>=1.4.2; extra == \"all\"", - "fastparquet>=2024.11.0; extra == \"all\"", - "fsspec>=2024.10.0; extra == \"all\"", - "gcsfs>=2024.10.0; extra == \"all\"", - "html5lib>=1.1; extra == \"all\"", - "hypothesis>=6.116.0; extra == \"all\"", - "jinja2>=3.1.5; extra == \"all\"", - "lxml>=5.3.0; extra == \"all\"", - "matplotlib>=3.9.3; extra == \"all\"", - "numba>=0.60.0; extra == \"all\"", - "numexpr>=2.10.2; extra == \"all\"", - "odfpy>=1.4.1; extra == \"all\"", - "openpyxl>=3.1.5; extra == \"all\"", - "psycopg2>=2.9.10; extra == \"all\"", - "pyarrow>=13.0.0; extra == \"all\"", - "pyiceberg>=0.8.1; extra == \"all\"", - "pymysql>=1.1.1; extra == \"all\"", - "PyQt5>=5.15.9; extra == \"all\"", - "pyreadstat>=1.2.8; extra == \"all\"", - "pytest>=8.3.4; extra == \"all\"", - "pytest-xdist>=3.6.1; extra == \"all\"", - "python-calamine>=0.3.0; extra == \"all\"", - "pytz>=2020.1; extra == \"all\"", - "pyxlsb>=1.0.10; extra == \"all\"", - "qtpy>=2.4.2; extra == \"all\"", - "scipy>=1.14.1; extra == \"all\"", - "s3fs>=2024.10.0; extra == \"all\"", - "SQLAlchemy>=2.0.36; extra == \"all\"", - "tables>=3.10.1; extra == \"all\"", - "tabulate>=0.9.0; extra == \"all\"", - "xarray>=2024.10.0; extra == \"all\"", - "xlrd>=2.0.1; extra == \"all\"", - "xlsxwriter>=3.2.0; extra == \"all\"", - "zstandard>=0.23.0; extra == \"all\"" - ] - } -} diff --git a/tests/fixtures/guarddog/pypi-pandas-3.0.3.json b/tests/fixtures/guarddog/pypi-pandas-3.0.3.json deleted file mode 100644 index 70ec2c8..0000000 --- a/tests/fixtures/guarddog/pypi-pandas-3.0.3.json +++ /dev/null @@ -1 +0,0 @@ -[{"dependency": "pandas", "version": "3.0.3", "result": {"issues": 1, "errors": {}, "results": {"metadata_mismatch": "Dependency mismatch between PyPI metadata and pyproject.toml:\n In PyPI metadata but NOT in pyproject.toml: adbc-driver-postgresql, adbc-driver-sqlite, beautifulsoup4, bottleneck, fastparquet, fsspec, gcsfs, html5lib, hypothesis, jinja2, lxml, matplotlib, numba, numexpr, odfpy, openpyxl, psycopg2, pyarrow, pyiceberg, pymysql, pyqt5, pyreadstat, pytest, pytest-xdist, python-calamine, pytz, pyxlsb, qtpy, s3fs, scipy, sqlalchemy, tables, tabulate, xarray, xlrd, xlsxwriter, zstandard\n"}, "path": "/srv/example/guarddog/tmpdir/pandas", "risk_score": {"score": 6.3, "label": "suspicious", "findings_count": 1, "score_breakdown": {"severity_component": 0.2, "chain_component": 0.08, "specificity_component": 0.21, "sophistication_component": 0.14, "num_stages": 1, "has_source_code_risks": false, "max_severity": 2, "dominant_specificity": "medium", "dominant_sophistication": "medium"}}, "risks": [{"name": "risk.metadata.manifest-mismatch", "category": "metadata", "severity": "medium", "mitre_tactics": ["execution"], "threat_identifies": "threat.metadata.manifest-mismatch", "threat_rule": "metadata_mismatch", "threat_description": "Dependency mismatch between PyPI metadata and pyproject.toml:\n In PyPI metadata but NOT in pyproject.toml: adbc-driver-postgresql, adbc-driver-sqlite, beautifulsoup4, bottleneck, fastparquet, fsspec, gcsfs, html5lib, hypothesis, jinja2, lxml, matplotlib, numba, numexpr, odfpy, openpyxl, psycopg2, pyarrow, pyiceberg, pymysql, pyqt5, pyreadstat, pytest, pytest-xdist, python-calamine, pytz, pyxlsb, qtpy, s3fs, scipy, sqlalchemy, tables, tabulate, xarray, xlrd, xlsxwriter, zstandard\n", "threat_location": "", "threat_code": "", "threat_match": "", "capability_identifies": null, "capability_rule": null, "file_path": ""}], "package_version": "3.0.3", "pypi_inspector_url": "https://inspector.pypi.io/project/pandas/3.0.3/", "pypi_dist_path": "/packages/42/16/b5c76b838fd9bf6ce84d3a53346b8874ec05c5f0040d75ef2c320100cd2a/pandas-3.0.3-cp311-cp311-macosx_10_9_x86_64.whl"}}] diff --git a/tests/supply_chain_cli.rs b/tests/supply_chain_cli.rs index f432abf..314da4c 100644 --- a/tests/supply_chain_cli.rs +++ b/tests/supply_chain_cli.rs @@ -44,7 +44,6 @@ fn version_answer(name: &str) -> &'static str { version: 0.5.2\\n'; exit 0; }\n" } "zizmor" => "[ \"$1\" = --version ] && { echo 'zizmor 1.30.0'; exit 0; }\n", - "guarddog" => "[ \"$1\" = --version ] && { echo '3.2.0'; exit 0; }\n", "cargo" => { "[ \"$1 $2\" = 'deny --version' ] && { echo 'cargo-deny 0.20.2'; exit 0; }\n\ [ \"$1 $2\" = 'vet --version' ] && { echo 'cargo-vet 0.10.2'; exit 0; }\n" @@ -130,9 +129,6 @@ fn invoke(root: &Path, path: &std::ffi::OsStr, args: &[&str], push: &[(&str, &st .env("PATH", path) // Where each stub records that it ran, and with what. .env("STUB_LOG", root.join("stub.log")) - // Where guarddog unpacks: beside the fixture, so a suite run does not - // write under the developer's own cache directory. - .env("XDG_CACHE_HOME", cache_of(root)) .current_dir(root) .stdin(Stdio::null()); // The binary asks git about the fixture's ranges, and a hook's GIT_DIR @@ -145,9 +141,6 @@ fn invoke(root: &Path, path: &std::ffi::OsStr, args: &[&str], push: &[(&str, &st "PRE_COMMIT_ORIGIN", "PRE_COMMIT_LOCAL_BRANCH", "PRE_COMMIT_REMOTE_BRANCH", - // Decides which npm dependencies guarddog reads, and so whether it is - // asked at all. - "GUARDDOG_NPM_INCLUDE_DEV_DEPENDENCIES", ] { command.env_remove(name); } @@ -157,11 +150,6 @@ fn invoke(root: &Path, path: &std::ffi::OsStr, args: &[&str], push: &[(&str, &st command.output().unwrap() } -/// The cache directory a run against `root` is handed, beside the fixture. -fn cache_of(root: &Path) -> PathBuf { - root.with_extension("cache") -} - /// The scoped form, driven the way prek drives it: the range in the two /// variables the pre-push guard already reads. fn pushed(root: &Path, tools: &Path, from: &str, to: &str) -> Output { @@ -182,37 +170,9 @@ fn journal(root: &Path) -> String { /// A stub that records its name, its working directory and its arguments. /// -/// The working directory is half the assertion in the scoped tests: guarddog is +/// The working directory is half the assertion in the scoped tests: `uv export` is /// run from the manifest's own directory, and a scan of the right file from the /// wrong place reads the wrong `package.json`. -/// A clean `guarddog verify --output-format json` report. -/// -/// guarddog is run with `--output-format json` because its exit code cannot -/// say it found something: `verify` answers 0 either way. So a stub that only -/// answers `exit 0` no longer models the tool -- a clean run PRINTS a report -/// whose `risks` list is empty, and printing nothing is a run that did not -/// report, which is could-not-look. -const GUARDDOG_CLEAN: &str = "echo '[{\"dependency\":\"cookie\",\"result\":\ - {\"errors\":{},\"issues\":0,\"results\":{},\"risks\":[]}}]'\nexit 0"; - -/// The same report with one risk in it, which is a finding at exit 0. -const GUARDDOG_RISK: &str = "echo '[{\"dependency\":\"reqests\",\"result\":\ - {\"errors\":{},\"issues\":1,\"results\":{},\"risks\":\ - [{\"name\":\"typosquatting\",\"severity\":\"high\"}]}}]'\nexit 0"; - -fn clean_dependencies(names: &[&str]) -> String { - let entries: Vec = names - .iter() - .map(|name| { - serde_json::json!({ - "dependency": name, - "result": {"errors": {}, "results": {}, "risks": []} - }) - }) - .collect(); - format!("echo '{}'\nexit 0", serde_json::json!(entries)) -} - fn recording(answer: &str) -> String { format!("echo \"$(basename \"$0\") [$PWD] $*\" >> \"$STUB_LOG\"\n{answer}") } @@ -293,7 +253,7 @@ fn a_missing_scanner_is_could_not_look_and_exit_2_not_a_pass() { #[test] fn clean_scanners_over_a_tree_with_nothing_else_is_a_pass_that_says_what_ran() { let root = repository(); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", "exit 0")]); + let tools = stubs(&[("osv-scanner", "exit 0")]); let output = supply(&root, Some(&tools)); assert_eq!(code(&output), 0, "{}", text(&output)); let said = text(&output); @@ -517,51 +477,15 @@ fn a_repository_with_a_vet_store_runs_vet_and_reports_what_it_said() { assert!(!said.contains("no supply-chain/ store here"), "{said}"); } -/// A Python lock is exported by uv and handed to guarddog. -/// -/// guarddog reads a requirements file, uv.lock is not one, and the export is -/// therefore not plumbing but the whole section: a run that skipped it would -/// scan nothing and say "checked". The refusal is asserted rather than a clean -/// run so that the count and the verdict are both pinned. -#[test] -fn a_python_lock_is_exported_for_guarddog_and_its_refusal_is_the_runs() { - let root = repository(); - std::fs::create_dir_all(root.join("service")).unwrap(); - std::fs::write(root.join("service/uv.lock"), "version = 1\n").unwrap(); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ("uv", "echo 'reqests==2.0.0'"), - ( - "guarddog", - // `$5`, not `$3`: the export path sits after `--output-format json`, - // which guarddog needs because its exit code cannot report a find. - &format!( - "grep -q 'reqests==2.0.0' \"$5\" || {{ echo 'guarddog was not handed the \ - export'; exit 2; }}\n{GUARDDOG_RISK}" - ), - ), - ]); - let output = supply(&root, Some(&tools)); - assert_eq!(code(&output), 1, "{}", text(&output)); - let said = text(&output); - assert!(said.contains("FAILED: guarddog pypi"), "{said}"); - assert!(said.contains("1 Python/npm manifest(s) checked"), "{said}"); - assert!( - !said.contains("guarddog was not handed the export"), - "{said}" - ); -} - /// A `uv` that is not on PATH is could-not-look, not a clean Python section. /// -/// guarddog IS installed here, so the section's own missing-tool check passes -/// and the run reaches the export. Nothing was read; a pass would say the +/// The run reaches the export. Nothing was read; a pass would say the /// Python manifests were scanned when uv never ran. #[test] fn a_python_lock_with_no_uv_on_path_is_could_not_look_not_a_pass() { let root = repository(); std::fs::write(root.join("uv.lock"), "version = 1\n").unwrap(); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", "exit 0")]); + let tools = stubs(&[("osv-scanner", "exit 0")]); let output = supply(&root, Some(&tools)); assert_eq!(code(&output), 2, "{}", text(&output)); let said = text(&output); @@ -572,7 +496,7 @@ fn a_python_lock_with_no_uv_on_path_is_could_not_look_not_a_pass() { /// An export that fails is a failure of the section, not a skipped manifest. /// /// `uv export` refusing -- a lock out of date with its `pyproject.toml` is the -/// ordinary cause -- leaves guarddog nothing to read. Continuing to the next +/// ordinary cause -- leaves nothing to read. Continuing to the next /// manifest without recording it would let a run over one broken lock and one /// clean one print a pass. #[test] @@ -585,29 +509,27 @@ fn a_uv_export_that_fails_is_a_failure_rather_than_a_manifest_quietly_skipped() "uv", "echo 'error: the lock file is not up to date' >&2; exit 2", ), - ("guarddog", "echo 'guarddog ran on nothing'; exit 0"), ]); let output = supply(&root, Some(&tools)); assert_eq!(code(&output), 1, "{}", text(&output)); let said = text(&output); assert!( - said.contains("FAILED: guarddog: uv export failed"), + said.contains("FAILED: git pins: uv export failed"), "{said}" ); - assert!(!said.contains("guarddog ran on nothing"), "{said}"); } -/// A lock out of step with its manifest is exported and scanned as it is, and -/// the tree is not written. +/// A lock out of step with its manifest is exported as it is, and the tree is +/// not written. /// /// The lock records an `exclude-newer` its `pyproject.toml` lacks, which is how /// a lock made with the option on the command line reads to `uv lock --check`. -/// Whether the pair agrees is uv's question; the scan asks about the pinned -/// packages. The `uv` modelled here refuses `--locked` on such a pair, relocks +/// Whether the pair agrees is uv's question; the scan reads the pinned +/// set. The `uv` modelled here refuses `--locked` on such a pair, relocks /// and writes without either flag, and exports the lock as it stands under /// `--frozen`. #[test] -fn a_lock_out_of_step_with_its_manifest_is_exported_and_scanned_without_a_write() { +fn a_lock_out_of_step_with_its_manifest_is_exported_without_a_write() { let root = repository(); write( &root, @@ -628,28 +550,13 @@ fn a_lock_out_of_step_with_its_manifest_is_exported_and_scanned_without_a_write( [ -z \"$frozen\" ] && echo '# relocked' > uv.lock\n\ echo 'requests==2.0.0'", ); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ("uv", &uv), - ( - "guarddog", - &format!( - "grep -q 'requests==2.0.0' \"$5\" || {{ echo 'guarddog was not handed the \ - export'; exit 2; }}\n{}", - clean_dependencies(&["requests"]) - ), - ), - ]); + let tools = stubs(&[("osv-scanner", "exit 0"), ("uv", &uv)]); let before = tree(&root); let output = supply(&root, Some(&tools)); assert_eq!(tree(&root), before, "the scan wrote into the tree"); assert_eq!(code(&output), 0, "{}", text(&output)); let said = text(&output); assert!(said.contains("1 Python/npm manifest(s) checked"), "{said}"); - assert!( - !said.contains("guarddog was not handed the export"), - "{said}" - ); assert!( journal(&root).contains("export --frozen"), "{}", @@ -657,15 +564,13 @@ fn a_lock_out_of_step_with_its_manifest_is_exported_and_scanned_without_a_write( ); } -/// An npm manifest is scanned in its own directory, and a clean one passes. +/// An npm manifest with registry dependencies alone has no git pin to check, +/// and passes. /// -/// `package.json` is handed to guarddog by bare name with the working directory -/// set to the manifest's own, so a nested package scanned from the repository -/// root would read the wrong file or none. The clean case is asserted here -/// because it is the only place the section's success path and its count are -/// both visible. +/// The clean case is asserted here because it is the only place the git pins +/// section's success path and its count are both visible. #[test] -fn an_npm_manifest_is_scanned_where_it_lives_and_a_clean_one_is_a_pass() { +fn an_npm_manifest_with_no_git_dependency_is_counted_and_a_pass() { let root = repository(); std::fs::create_dir_all(root.join("web")).unwrap(); std::fs::write( @@ -673,189 +578,18 @@ fn an_npm_manifest_is_scanned_where_it_lives_and_a_clean_one_is_a_pass() { "{\"name\": \"web\", \"dependencies\": {\"cookie\": \"^1.1.1\"}}\n", ) .unwrap(); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ( - "guarddog", - &format!( - "grep -q '\"web\"' package.json || {{ echo 'wrong directory'; exit 1; }}\n{GUARDDOG_CLEAN}" - ), - ), - ]); + let tools = stubs(&[("osv-scanner", "exit 0")]); let output = supply(&root, Some(&tools)); assert_eq!(code(&output), 0, "{}", text(&output)); let said = text(&output); assert!(said.contains("1 Python/npm manifest(s) checked"), "{said}"); assert!(said.contains("all checks passed"), "{said}"); - assert!(!said.contains("wrong directory"), "{said}"); -} - -/// guarddog unpacks under the cache directory whatever `TMPDIR` the run was -/// handed. -/// -/// guarddog's `metadata_mismatch` rule picks a wheel's root by `os.listdir` -/// order, which is the filesystem's, so a guarddog that inherited `TMPDIR` -/// passed pandas 3.0.3 with it on btrfs and failed it with it on tmpfs. Two -/// runs with two `TMPDIR`s must hand guarddog the same directory, and it must -/// exist for guarddog's `tempfile` to use. -#[test] -fn guarddog_unpacks_in_the_same_directory_whatever_tmpdir_says() { - let root = repository(); - std::fs::create_dir_all(root.join("web")).unwrap(); - std::fs::write( - root.join("web/package.json"), - "{\"name\": \"web\", \"dependencies\": {\"cookie\": \"^1.1.1\"}}\n", - ) - .unwrap(); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ( - "guarddog", - &format!( - "[ -d \"$TMPDIR\" ] || {{ echo \"no directory at $TMPDIR\"; exit 1; }}\n\ - echo \"TMPDIR=$TMPDIR\" >> \"$STUB_LOG\"\n{GUARDDOG_CLEAN}" - ), - ), - ]); - let mut path = tools.as_os_str().to_owned(); - path.push(":/usr/bin:/bin"); - let expected = format!( - "TMPDIR={}", - cache_of(&root).join("uphold/guarddog").display() - ); - for kind in ["supply-chain-tmpfs", "supply-chain-btrfs"] { - let tmpdir = support::scratch(kind); - std::fs::create_dir_all(&tmpdir).unwrap(); - let _ = std::fs::remove_file(root.join("stub.log")); - let output = invoke( - &root, - &path, - &["--all"], - &[("TMPDIR", tmpdir.to_str().unwrap())], - ); - assert_eq!(code(&output), 0, "{}", text(&output)); - assert_eq!(journal(&root).trim(), expected, "{}", text(&output)); - } -} - -/// A refusing npm scan is a failure, named by the directory it came from. -/// -/// A fleet run prints many sections; "guarddog npm" without the directory is a -/// finding nobody can locate, which was the divergence between the seven copies -/// of the shell task this replaces -- one dumped the failure output, the rest -/// printed a bare FAILED. -#[test] -fn a_refusing_npm_scan_names_the_directory_it_refused_in() { - let root = repository(); - std::fs::create_dir_all(root.join("web")).unwrap(); - std::fs::write( - root.join("web/package.json"), - "{\"name\": \"web\", \"dependencies\": {\"cookie\": \"^1.1.1\"}}\n", - ) - .unwrap(); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", GUARDDOG_RISK)]); - let output = supply(&root, Some(&tools)); - assert_eq!(code(&output), 1, "{}", text(&output)); - let said = text(&output); - assert!(said.contains("FAILED: guarddog npm"), "{said}"); - assert!(said.contains("web"), "{said}"); -} - -/// A manifest with `devDependencies` only leaves guarddog nothing to read. -/// -/// `guarddog npm verify` reads `dependencies` alone and answers `[]` for a -/// manifest without them, which the reader takes for a network failure; every -/// push of a test-only package was "could not look". Such a manifest is not -/// handed to guarddog at all, and the section passes. -#[test] -fn a_manifest_with_only_dev_dependencies_is_not_sent_to_guarddog_and_passes() { - let root = repository(); - std::fs::write( - root.join("package.json"), - "{\"name\": \"tests\", \"devDependencies\": {\"jose\": \"^6.2.12\"}}\n", - ) - .unwrap(); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ("guarddog", &recording("echo '[]'; exit 0")), - ]); - let output = supply(&root, Some(&tools)); - let said = text(&output); - assert_eq!(code(&output), 0, "{said}"); - assert!( - said.contains("no dependency here resolves from npm"), - "{said}" - ); - assert!(!said.contains("reported on no dependency"), "{said}"); - assert!(said.contains("all checks passed"), "{said}"); - assert!(!journal(&root).contains("guarddog"), "{}", journal(&root)); -} - -/// Where guarddog is told to read `devDependencies`, they are something to -/// look up, and the same manifest is handed to it. -#[test] -fn dev_dependencies_are_sent_to_guarddog_where_it_is_told_to_read_them() { - let root = repository(); - std::fs::write( - root.join("package.json"), - "{\"name\": \"tests\", \"devDependencies\": {\"jose\": \"^6.2.12\"}}\n", - ) - .unwrap(); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ("guarddog", &recording(&clean_dependencies(&["jose"]))), - ]); - let output = supply_with( - &root, - &tools, - &[( - "GUARDDOG_NPM_INCLUDE_DEV_DEPENDENCIES", - String::from("TRUE"), - )], - ); - let said = text(&output); - assert_eq!(code(&output), 0, "{said}"); - assert!(!said.contains("guarddog was not asked"), "{said}"); - assert!( - journal(&root).contains("] npm verify"), - "{}", - journal(&root) - ); -} - -/// A manifest that does not parse is still handed to guarddog, and its failure -/// to report is could-not-look, never the pass a manifest with nothing to -/// scan earns. -#[test] -fn an_unparseable_manifest_is_still_sent_to_guarddog_and_is_not_checked() { - let root = repository(); - std::fs::write(root.join("package.json"), "{\"name\": \"broken\",\n").unwrap(); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ( - "guarddog", - &recording("echo 'json.decoder.JSONDecodeError: Expecting value' >&2; exit 1"), - ), - ]); - let output = supply(&root, Some(&tools)); - let said = text(&output); - assert_eq!(code(&output), 2, "{said}"); - assert!( - said.contains("NOT CHECKED: guarddog gave no report"), - "{said}" - ); - assert!(!said.contains("guarddog was not asked"), "{said}"); - assert!( - journal(&root).contains("] npm verify"), - "{}", - journal(&root) - ); } /// A section with work to do and no tool to do it names the tool. /// -/// Both of these sections gate on the tree first -- `deny.toml` here, a -/// `package.json` there -- and the gate's other answer is a pass. Reaching the +/// The section gates on the tree first -- `deny.toml` here -- and the gate's +/// other answer is a pass. Reaching the /// missing-tool check only after the gate is what keeps "nothing to check" and /// "nobody checked" apart, and the reader is owed the name of what to install /// rather than a bare exit 2. @@ -864,20 +598,13 @@ fn a_section_with_work_and_no_tool_installed_names_the_tool_and_is_not_a_pass() let root = repository(); std::fs::write(root.join("deny.toml"), "[bans]\n").unwrap(); std::fs::write(root.join("Cargo.toml"), "[package]\nname = 'fixture'\n").unwrap(); - std::fs::write( - root.join("package.json"), - "{\"name\": \"fixture\", \"dependencies\": {\"cookie\": \"^1.1.1\"}}\n", - ) - .unwrap(); let tools = stubs(&[("osv-scanner", "exit 0")]); let output = supply_without_the_system_path(&root, &tools); assert_eq!(code(&output), 2, "{}", text(&output)); let said = text(&output); assert!(said.contains("cargo is not on PATH"), "{said}"); - assert!(said.contains("guarddog is not on PATH"), "{said}"); assert!(!said.contains("all checks passed"), "{said}"); assert!(!said.contains("no deny.toml"), "{said}"); - assert!(!said.contains("no Python or npm manifests here"), "{said}"); } /// A cargo-deny that finds nothing is clean, and says how many it looked at. @@ -1281,7 +1008,6 @@ fn a_range_touching_no_manifest_runs_no_scanner_and_says_so_in_one_line() { 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); @@ -1311,23 +1037,21 @@ fn all_scans_every_manifest_even_where_the_range_holds_none_of_them() { ("osv-scanner", &recording("exit 0")), ("zizmor", &recording("exit 0")), ("uv", &recording("echo 'requests==2.0.0'")), - ("guarddog", &recording(&clean_dependencies(&["requests"]))), ]); let output = supply(&root, Some(&tools)); assert_eq!(code(&output), 0, "{}", text(&output)); let ran = journal(&root); assert!(ran.contains("osv-scanner"), "{ran}"); assert!(ran.contains("zizmor"), "{ran}"); - assert!(ran.contains("guarddog"), "{ran}"); + assert!(ran.contains("export --frozen"), "{ran}"); } /// A changed Python lock is scanned where it lives, and nowhere else. /// -/// The second manifest is the assertion. guarddog is a minute of network per -/// handful of packages, and a scoped run that still walked to every `uv.lock` -/// in the tree would have narrowed nothing at the only cost that mattered. +/// The second manifest is the assertion. A scoped run that still walked to +/// every `uv.lock` in the tree would have narrowed nothing. #[test] -fn a_changed_python_lock_runs_guarddog_in_that_directory_and_not_the_others() { +fn a_changed_python_lock_is_exported_in_that_directory_and_not_the_others() { let root = tracked(); write(&root, "other/uv.lock", "version = 1\n"); let _ = commit(&root, "a lock this push does not touch"); @@ -1337,12 +1061,11 @@ fn a_changed_python_lock_runs_guarddog_in_that_directory_and_not_the_others() { let tools = stubs(&[ ("osv-scanner", &recording("exit 0")), ("uv", &recording("echo 'requests==2.0.0'")), - ("guarddog", &recording(&clean_dependencies(&["requests"]))), ]); let output = pushed(&root, &tools, &before, &after); assert_eq!(code(&output), 0, "{}", text(&output)); let ran = journal(&root); - assert!(ran.contains("guarddog"), "{ran}"); + assert!(ran.contains("export --frozen"), "{ran}"); assert!(ran.contains("harness"), "{ran}"); assert!(!ran.contains("other"), "{ran}"); // The lock is handed to osv-scanner by path, not scanned for by a walk. @@ -1379,7 +1102,6 @@ fn a_moved_pyproject_with_no_uv_lock_is_not_exported_and_no_lock_is_written() { "uv", &recording("echo '# resolved afresh' > uv.lock\necho 'requests==2.0.0'"), ), - ("guarddog", &recording(&clean_dependencies(&["requests"]))), ]); let output = pushed(&root, &tools, &before, &after); assert_eq!(code(&output), 0, "{}", text(&output)); @@ -1440,7 +1162,6 @@ fn a_range_holding_only_a_ci_config_says_the_file_is_unscanned_not_that_there_wa 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); @@ -1514,12 +1235,11 @@ fn a_bumped_submodule_pointer_expands_into_the_members_own_manifests() { let tools = stubs(&[ ("osv-scanner", &recording("exit 0")), ("uv", &recording("echo 'requests==2.0.0'")), - ("guarddog", &recording(&clean_dependencies(&["requests"]))), ]); let output = pushed(&root, &tools, &before, &after); assert_eq!(code(&output), 0, "{}", text(&output)); let ran = journal(&root); - assert!(ran.contains("guarddog"), "{ran}"); + assert!(ran.contains("export --frozen"), "{ran}"); assert!(ran.contains("sub"), "{ran}"); } @@ -1548,13 +1268,16 @@ fn a_submodule_commit_the_store_does_not_have_widens_to_every_manifest_under_it( let tools = stubs(&[ ("osv-scanner", &recording("exit 0")), ("uv", &recording("echo 'requests==2.0.0'")), - ("guarddog", &recording(&clean_dependencies(&["requests"]))), ]); let output = pushed(&root, &tools, &before, &after); assert_eq!(code(&output), 0, "{}", text(&output)); let said = text(&output); assert!(said.contains("does not have"), "{said}"); - assert!(journal(&root).contains("guarddog"), "{}", journal(&root)); + assert!( + journal(&root).contains("export --frozen"), + "{}", + journal(&root) + ); } /// A push from a linked worktree reads the member's range in the member. @@ -1617,7 +1340,6 @@ fn a_push_from_a_linked_worktree_reads_the_members_range_in_the_member() { let tools = stubs(&[ ("osv-scanner", &recording("exit 0")), ("uv", &recording("echo 'requests==2.0.0'")), - ("guarddog", &recording(&clean_dependencies(&["requests"]))), ]); let mut path = tools.as_os_str().to_owned(); path.push(":/usr/bin:/bin"); @@ -1665,39 +1387,6 @@ fn a_submodule_that_is_not_checked_out_is_refused_rather_than_widened() { assert!(said.contains("sub"), "{said}"); } -/// guarddog rules that did not run are could-not-look, not a pass. -/// -/// guarddog prints "Some rules failed to run while scanning " and -/// exits 0, and the two email-domain rules time out routinely. An orchestrator -/// reading only the exit code files that under clean: the publisher-identity -/// question was asked and nobody answered it, which is the third verdict this -/// command exists for, and the reason names the package and how many rules. -#[test] -fn guarddog_rules_that_did_not_run_are_could_not_look_rather_than_a_clean_scan() { - let root = repository(); - std::fs::write( - root.join("package.json"), - "{\"name\": \"fixture\", \"dependencies\": {\"cookie\": \"^1.1.1\"}}\n", - ) - .unwrap(); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ( - "guarddog", - "echo 'Some rules failed to run while scanning left-pad:'\n\ - echo '* potentially_compromised_email_domain: failed to run rule \ - potentially_compromised_email_domain: timed out'\n\ - exit 0", - ), - ]); - let output = supply(&root, Some(&tools)); - assert_eq!(code(&output), 2, "{}", text(&output)); - let said = text(&output); - assert!(said.contains("left-pad"), "{said}"); - assert!(said.contains("1 rule(s) unrun"), "{said}"); - assert!(!said.contains("all checks passed"), "{said}"); -} - /// cargo-vet's finding and cargo-vet's refusal to start share exit 255. /// /// A dependency carrying no audit and a store that does not parse are the same @@ -1872,313 +1561,68 @@ fn cargo_deny_that_never_reached_a_check_is_could_not_look_not_an_advisory() { assert!(!said.contains("all checks passed"), "{said}"); } -/// guarddog reports a finding at exit 0, and the finding must survive that. -/// -/// `guarddog verify` answers 0 whether it found three high-severity risks or -/// none, so a section reading the exit code called every finding clean. This -/// is the false negative that reading `risks` exists to close, on the one -/// scanner here whose job is malware and typosquats. -#[test] -fn guarddog_that_found_risks_and_exited_zero_is_a_finding_not_a_clean_scan() { - let root = repository(); - std::fs::create_dir_all(root.join("web")).unwrap(); - std::fs::write( - root.join("web/package.json"), - "{\"name\": \"web\", \"dependencies\": {\"cookie\": \"^1.1.1\"}}\n", - ) - .unwrap(); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", GUARDDOG_RISK)]); - let output = supply(&root, Some(&tools)); - assert_eq!(code(&output), 1, "{}", text(&output)); - let said = text(&output); - assert!(said.contains("objected to reqests"), "{said}"); - assert!(!said.contains("all checks passed"), "{said}"); -} +// ── version floors ─────────────────────────────────────────────────────── -/// `issues` is not the finding count, so a clean package stays clean. +/// A scanner older than the release its reader was measured against is exit 2 +/// and is never run. /// -/// `six` reports `issues: 2` with `risks: []` and guarddog's own label -/// `no_risks_detected`; the two issues are capability matches on an `exec()`. -/// This is why `--exit-non-zero-on-finding`, which counts issues, is not the -/// remedy for the case above: it would fail a package guarddog calls clean. +/// The readers match the text a scanner prints when it did not look; an older +/// release printing something else would fall through to whichever verdict the +/// unmatched text reaches. The reason names the tool, the version found and the +/// floor, and the scan itself is not in the journal. #[test] -fn guarddog_issues_without_risks_are_not_a_finding() { +fn a_scanner_below_its_floor_is_could_not_look_and_is_never_run() { let root = repository(); - std::fs::create_dir_all(root.join("web")).unwrap(); - std::fs::write( - root.join("web/package.json"), - "{\"name\": \"web\", \"dependencies\": {\"six\": \"^1.1.1\"}}\n", - ) - .unwrap(); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), + std::fs::create_dir_all(root.join(".github/workflows")).unwrap(); + std::fs::write(root.join(".github/workflows/ci.yml"), "on: push\n").unwrap(); + let tools = raw_stubs(&[ + ( + "osv-scanner", + &format!("{}{}", version_answer("osv-scanner"), recording("exit 0")), + ), ( - "guarddog", - "echo '[{\"dependency\":\"six\",\"result\":{\"errors\":{},\"issues\":2,\ - \"results\":{},\"risks\":[]}}]'\nexit 0", + "zizmor", + &format!( + "[ \"$1\" = --version ] && {{ echo 'zizmor 1.29.0'; exit 0; }}\n{}", + recording("exit 0") + ), ), ]); let output = supply(&root, Some(&tools)); - assert_eq!(code(&output), 0, "{}", text(&output)); + let said = text(&output); + assert_eq!(code(&output), 2, "{said}"); assert!( - text(&output).contains("all checks passed"), - "{}", - text(&output) + said.contains("zizmor 1.29.0 is older than 1.30.0"), + "{said}" ); + assert!(said.contains("NOT CHECKED"), "{said}"); + assert!(!said.contains(" FAILED"), "{said}"); + // osv-scanner, at its floor, ran; zizmor, below it, did not. + let journal = journal(&root); + assert!(journal.contains("osv-scanner"), "{journal}"); + assert!(!journal.contains("zizmor"), "{journal}"); } -/// npm skips a dependency whose selector has no matching release. Neither a -/// duplicate entry nor an unrelated package can make up for the missing name. -#[test] -fn guarddog_must_report_every_submitted_npm_dependency() { - let root = repository(); - write( - &root, - "package.json", - r#"{"dependencies":{"cookie":"1.0.0","missing-package":"1.0.0"}}"#, - ); - for names in [ - vec![], - vec!["cookie"], - vec!["cookie", "cookie"], - vec!["cookie", "unrelated"], - ] { - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ("guarddog", &clean_dependencies(&names)), - ]); - let output = supply(&root, Some(&tools)); - let said = text(&output); - assert_eq!(code(&output), 2, "{names:?}: {said}"); - assert!(said.contains("guarddog omitted"), "{said}"); - assert!(said.contains("missing-package"), "{said}"); - assert!(said.contains(&root.display().to_string()), "{said}"); - assert!(!said.contains("all checks passed"), "{said}"); - } -} - -/// `PyPI`'s parser can return the packages before a failed registry request, -/// dropping both the failed dependency and everything after it. +/// A version answer nobody can read, or a version command that fails, is not a +/// pass either. #[test] -fn guarddog_partial_pypi_reports_name_every_omitted_dependency() { +fn a_scanner_whose_version_cannot_be_read_is_could_not_look() { let root = repository(); - write(&root, "uv.lock", "version = 1\n"); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ( - "uv", - "printf '%s\\n' 'requests==2.0.0' 'registry-failed==1.0' 'after-failure==1.0'", - ), - ("guarddog", &clean_dependencies(&["requests"])), - ]); + let tools = raw_stubs(&[("osv-scanner", "echo 'osv-scanner (devel)'\nexit 0")]); let output = supply(&root, Some(&tools)); let said = text(&output); assert_eq!(code(&output), 2, "{said}"); assert!( - said.contains("guarddog omitted after-failure, registry-failed"), + said.contains("`osv-scanner --version` printed no version this could read"), "{said}" ); - assert!(said.contains("submitted pypi dependency"), "{said}"); assert!(!said.contains("all checks passed"), "{said}"); } -/// Alias keys are not the names guarddog reports, and several selectors can -/// merge into one real npm package, including across dependency groups. +/// cargo on PATH and no cargo-deny: `cargo deny --version` is cargo's "no such +/// command", and that is could-not-look before any manifest is handed over. #[test] -fn guarddog_coverage_uses_real_npm_alias_names() { - let root = repository(); - write( - &root, - "package.json", - r#"{"dependencies":{"cookie":"1.0.0","renamed":"npm:cookie@1.0.0","scoped":"npm:@scope/package@^2","latest":"npm:@scope/package"},"devDependencies":{"development":"npm:cookie@^1"}}"#, - ); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ( - "guarddog", - &clean_dependencies(&["cookie", "@scope/package"]), - ), - ]); - let output = supply_with( - &root, - &tools, - &[( - "GUARDDOG_NPM_INCLUDE_DEV_DEPENDENCIES", - String::from("true"), - )], - ); - assert_eq!(code(&output), 0, "{}", text(&output)); - assert!( - text(&output).contains("all checks passed"), - "{}", - text(&output) - ); -} - -/// Extras, markers and repeated punctuation do not change a `PyPI` identity. -/// Local project lines and comments remain outside the expected registry set. -#[test] -fn guarddog_coverage_normalises_pypi_names_and_ignores_local_requirements() { - let root = repository(); - write(&root, "uv.lock", "version = 1\n"); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ( - "uv", - "printf '%s\\n' '# exported' '-e .' 'Example__Package.Name[extra]==1.0 ; python_version >= \"3.9\"' 'example-package-name==1.0'", - ), - ("guarddog", &clean_dependencies(&["EXAMPLE-package_NAME"])), - ]); - let output = supply(&root, Some(&tools)); - assert_eq!(code(&output), 0, "{}", text(&output)); - assert!( - text(&output).contains("all checks passed"), - "{}", - text(&output) - ); -} - -/// A reported risk stays a finding even when another dependency was omitted; -/// the output must also disclose the gap. -#[test] -fn guarddog_risks_survive_a_partial_report() { - let root = repository(); - write( - &root, - "package.json", - r#"{"dependencies":{"reqests":"2.0.0","missing-package":"1.0.0"}}"#, - ); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", GUARDDOG_RISK)]); - let output = supply(&root, Some(&tools)); - let said = text(&output); - assert_eq!(code(&output), 1, "{said}"); - assert!(said.contains("objected to reqests"), "{said}"); - assert!(said.contains("guarddog omitted missing-package"), "{said}"); - assert!(!said.contains("all checks passed"), "{said}"); -} - -/// A report must carry both the checks guarddog ran and its risk list. A -/// missing or mistyped field is an incomplete answer, not an empty finding. -#[test] -fn guarddog_with_an_incomplete_result_is_could_not_look() { - let root = repository(); - std::fs::create_dir_all(root.join("web")).unwrap(); - std::fs::write( - root.join("web/package.json"), - "{\"name\": \"web\", \"dependencies\": {\"cookie\": \"^1.1.1\"}}\n", - ) - .unwrap(); - for result in [ - r#"{"errors":{},"results":{}}"#, - r#"{"errors":{},"results":{},"risks":null}"#, - r#"{"errors":{},"results":null,"risks":[]}"#, - ] { - let report = format!(r#"[{{"dependency":"cookie","result":{result}}}]"#); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ("guarddog", &format!("echo '{report}'\nexit 0")), - ]); - let output = supply(&root, Some(&tools)); - assert_eq!(code(&output), 2, "{result}: {}", text(&output)); - let said = text(&output); - assert!( - said.contains("incomplete report for cookie"), - "{result}: {said}" - ); - assert!(!said.contains("all checks passed"), "{result}: {said}"); - } -} - -/// A dependency guarddog could not download is could-not-look, not clean. -/// -/// The 404 and network paths populate `errors` and drop `results`, and still -/// exit 0. Reading the code alone calls a package nobody scanned clean. -#[test] -fn guarddog_that_could_not_scan_a_dependency_is_could_not_look() { - let root = repository(); - std::fs::create_dir_all(root.join("web")).unwrap(); - std::fs::write( - root.join("web/package.json"), - "{\"name\": \"web\", \"dependencies\": {\"cookie\": \"^1.1.1\"}}\n", - ) - .unwrap(); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ( - "guarddog", - "echo '[{\"dependency\":\"left-pad\",\"result\":{\"errors\":\ - {\"download-package\":\"Received status code: 404 from PyPI\"},\"issues\":0}}]'\nexit 0", - ), - ]); - let output = supply(&root, Some(&tools)); - assert_eq!(code(&output), 2, "{}", text(&output)); - let said = text(&output); - assert!(said.contains("could not scan left-pad"), "{said}"); - assert!(!said.contains("all checks passed"), "{said}"); -} - -// ── version floors ─────────────────────────────────────────────────────── - -/// A scanner older than the release its reader was measured against is exit 2 -/// and is never run. -/// -/// The readers match the text a scanner prints when it did not look; an older -/// release printing something else would fall through to whichever verdict the -/// unmatched text reaches. The reason names the tool, the version found and the -/// floor, and the scan itself is not in the journal. -#[test] -fn a_scanner_below_its_floor_is_could_not_look_and_is_never_run() { - let root = repository(); - std::fs::create_dir_all(root.join(".github/workflows")).unwrap(); - std::fs::write(root.join(".github/workflows/ci.yml"), "on: push\n").unwrap(); - let tools = raw_stubs(&[ - ( - "osv-scanner", - &format!("{}{}", version_answer("osv-scanner"), recording("exit 0")), - ), - ( - "zizmor", - &format!( - "[ \"$1\" = --version ] && {{ echo 'zizmor 1.29.0'; exit 0; }}\n{}", - recording("exit 0") - ), - ), - ]); - let output = supply(&root, Some(&tools)); - let said = text(&output); - assert_eq!(code(&output), 2, "{said}"); - assert!( - said.contains("zizmor 1.29.0 is older than 1.30.0"), - "{said}" - ); - assert!(said.contains("NOT CHECKED"), "{said}"); - assert!(!said.contains(" FAILED"), "{said}"); - // osv-scanner, at its floor, ran; zizmor, below it, did not. - let journal = journal(&root); - assert!(journal.contains("osv-scanner"), "{journal}"); - assert!(!journal.contains("zizmor"), "{journal}"); -} - -/// A version answer nobody can read, or a version command that fails, is not a -/// pass either. -#[test] -fn a_scanner_whose_version_cannot_be_read_is_could_not_look() { - let root = repository(); - let tools = raw_stubs(&[("osv-scanner", "echo 'osv-scanner (devel)'\nexit 0")]); - let output = supply(&root, Some(&tools)); - let said = text(&output); - assert_eq!(code(&output), 2, "{said}"); - assert!( - said.contains("`osv-scanner --version` printed no version this could read"), - "{said}" - ); - assert!(!said.contains("all checks passed"), "{said}"); -} - -/// cargo on PATH and no cargo-deny: `cargo deny --version` is cargo's "no such -/// command", and that is could-not-look before any manifest is handed over. -#[test] -fn a_version_command_that_fails_is_could_not_look_and_no_manifest_is_handed_over() { +fn a_version_command_that_fails_is_could_not_look_and_no_manifest_is_handed_over() { let root = repository(); std::fs::write(root.join("deny.toml"), "[bans]\n").unwrap(); std::fs::write(root.join("Cargo.toml"), "[package]\nname = 'fixture'\n").unwrap(); @@ -2245,17 +1689,13 @@ fn inheriting_credentials() -> PathBuf { /// The two scanners that answer over an empty tree, clean, beside gitleaks. fn quiet_neighbours(gitleaks: &str) -> PathBuf { - stubs(&[ - ("osv-scanner", "exit 0"), - ("guarddog", "exit 0"), - ("gitleaks", gitleaks), - ]) + stubs(&[("osv-scanner", "exit 0"), ("gitleaks", gitleaks)]) } #[test] fn a_policy_inheriting_credentials_with_no_gitleaks_installed_is_exit_2() { let root = inheriting_credentials(); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", "exit 0")]); + let tools = stubs(&[("osv-scanner", "exit 0")]); let output = supply_without_the_system_path(&root, &tools); let said = text(&output); assert_eq!(code(&output), 2, "{said}"); @@ -2371,7 +1811,6 @@ fn a_range_touching_no_manifest_still_runs_gitleaks_over_that_range() { let after = commit(&root, "source only"); let tools = stubs(&[ ("osv-scanner", &recording("exit 0")), - ("guarddog", &recording(GUARDDOG_CLEAN)), ("gitleaks", &gitleaks_stub(GITLEAKS_PINNED, "exit 0")), ]); let output = pushed(&root, &tools, &before, &after); @@ -2387,7 +1826,6 @@ fn a_range_touching_no_manifest_still_runs_gitleaks_over_that_range() { "{ran}" ); assert!(!ran.contains("osv-scanner"), "{ran}"); - assert!(!ran.contains("guarddog"), "{ran}"); } /// A push of git notes: the commits a tool's notes ref carries, here a gate @@ -2439,7 +1877,6 @@ fn a_push_of_git_notes_is_a_declared_gap_and_widens_to_nothing() { let _ = std::fs::remove_file(root.join("stub.log")); let tools = stubs(&[ ("osv-scanner", &recording("exit 0")), - ("guarddog", &recording(GUARDDOG_CLEAN)), ("gitleaks", &gitleaks_stub(GITLEAKS_PINNED, "exit 3")), ]); let output = notes_pushed(&root, &tools, from, ¬es); @@ -2471,7 +1908,6 @@ fn every_scanner(gitleaks: &str) -> PathBuf { ("osv-scanner", &recording("exit 0")), ("zizmor", &recording("exit 0")), ("cargo", &recording("exit 0")), - ("guarddog", &recording(GUARDDOG_CLEAN)), ("gitleaks", gitleaks), ]) } @@ -2580,634 +2016,61 @@ fn staged_with_a_range_flag_is_a_usage_error() { } } -/// A guarddog 3.2.0 report on `swagger-ui-express` 5.0.1, trimmed to the keys -/// read here, with a `metadata_mismatch` risk whose description is not one -/// this reads as npm's publish-time rewrites, so the finding stands until a -/// waiver excuses it. The real report, which is that rewrite and nothing else, -/// is tolerated; see the test after these. -const GUARDDOG_MISMATCH: &str = "echo '[{\"dependency\":\"swagger-ui-express\",\ - \"version\":\"5.0.1\",\"result\":{\"errors\":{},\"issues\":1,\ - \"results\":{\"metadata_mismatch\":\"test script differs\"},\ - \"package_version\":\"5.0.1\",\"risks\":[{\"name\":\"risk.metadata.manifest-mismatch\",\ - \"threat_rule\":\"metadata_mismatch\",\"severity\":\"medium\"}]}}]'\nexit 0"; - -/// The policy of [`repository`], with one waiver appended. -fn waiving(root: &Path, package: &str) { - let path = root.join("policy/principles.toml"); - let policy = std::fs::read_to_string(&path).unwrap(); - std::fs::write( - path, - format!( - "{policy}\n[[supply_chain.waive]]\nscanner = \"guarddog\"\npackage = \"{package}\"\n\ - check = \"metadata_mismatch\"\nreason = \"npm rewrites the test script at publish\"\n" - ), - ) - .unwrap(); -} - -/// A confirmed false positive, waived for the release it was judged on, is -/// printed as waived with its reason and not held against the run. -#[test] -fn a_waived_guarddog_finding_is_printed_and_not_held_against_the_run() { - let root = repository(); - std::fs::create_dir_all(root.join("web")).unwrap(); - std::fs::write( - root.join("web/package.json"), - "{\"name\": \"web\", \"dependencies\": {\"swagger-ui-express\": \"5.0.1\"}}\n", - ) - .unwrap(); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", GUARDDOG_MISMATCH)]); - - let refused = supply(&root, Some(&tools)); - assert_eq!(code(&refused), 1, "{}", text(&refused)); - assert!( - text(&refused).contains("objected to swagger-ui-express@5.0.1"), - "{}", - text(&refused) - ); - - waiving(&root, "npm:swagger-ui-express@5.0.1"); - let waived = supply(&root, Some(&tools)); - assert_eq!(code(&waived), 0, "{}", text(&waived)); - let said = text(&waived); - let line = said - .lines() - .find(|line| line.contains("waived: metadata_mismatch on npm:swagger-ui-express@5.0.1")) - .unwrap_or_default(); - assert!( - line.ends_with("-- npm rewrites the test script at publish"), - "{line}" - ); - assert!(said.contains("all checks passed"), "{said}"); -} - -/// A waiver keyed on a version stops applying when the version moves: the -/// next release is read fresh, and the waiver is named as matching nothing. -#[test] -fn a_waiver_for_another_version_does_not_apply_and_is_reported_stale() { - let root = repository(); - std::fs::create_dir_all(root.join("web")).unwrap(); - std::fs::write( - root.join("web/package.json"), - "{\"name\": \"web\", \"dependencies\": {\"swagger-ui-express\": \"5.0.1\"}}\n", - ) - .unwrap(); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", GUARDDOG_MISMATCH)]); - waiving(&root, "npm:swagger-ui-express@5.0.0"); - let output = supply(&root, Some(&tools)); - assert_eq!(code(&output), 1, "{}", text(&output)); - let said = text(&output); - assert!( - said.contains("objected to swagger-ui-express@5.0.1"), - "{said}" - ); - assert!( - said.contains( - "waiver matched nothing in this run: metadata_mismatch on \ - npm:swagger-ui-express@5.0.0" - ), - "{said}" - ); -} - -/// A pypi `metadata_mismatch` waiver that matched nothing is not called -/// removable, because whether guarddog reaches the release's manifest at all is -/// the unpacking filesystem's: pandas 3.0.3 is reported on one host and not on -/// another, and the advice to delete the row on the second breaks the first. -#[test] -fn an_unmatched_pypi_metadata_mismatch_waiver_is_not_called_removable() { - let root = repository(); - std::fs::create_dir_all(root.join("service")).unwrap(); - std::fs::write(root.join("service/uv.lock"), "version = 1\n").unwrap(); - let clean = "echo '[{\"dependency\":\"pandas\",\"result\":{\"errors\":{},\"issues\":0,\ - \"results\":{\"metadata_mismatch\":null},\"package_version\":\"3.0.3\",\ - \"risks\":[]}}]'\nexit 0"; - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ("uv", "echo 'pandas==3.0.3'"), - ("guarddog", clean), - ]); - waiving(&root, "pypi:pandas@3.0.3"); - let output = supply(&root, Some(&tools)); - assert_eq!(code(&output), 0, "{}", text(&output)); - let said = text(&output); - let line = said - .lines() - .find(|line| line.contains("waiver matched nothing in this run")) - .unwrap_or_default(); - assert!( - line.contains("metadata_mismatch on pypi:pandas@3.0.3"), - "{said}" - ); - assert!(line.contains("another host may still need it"), "{line}"); - assert!(!line.contains("remove it"), "{line}"); -} - -/// guarddog's report for a release whose only mismatch is npm's own default -/// `start` script passes without a waiver, and the run names what it let -/// through. A waiver written for it before is now one that matched nothing. -#[test] -fn a_mismatch_that_is_only_npms_publish_rewrite_is_tolerated_and_named() { - let root = repository(); - std::fs::create_dir_all(root.join("web")).unwrap(); - std::fs::write( - root.join("web/package.json"), - "{\"name\": \"web\", \"dependencies\": {\"react-dom\": \"19.2.0\"}}\n", - ) - .unwrap(); - let report = Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests/fixtures/guarddog/npm-react-dom-19.2.0.json"); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ("guarddog", &format!("cat '{}'\nexit 0", report.display())), - ]); - waiving(&root, "npm:react-dom@19.2.0"); - let output = supply(&root, Some(&tools)); - assert_eq!(code(&output), 0, "{}", text(&output)); - let said = text(&output); - let line = said - .lines() - .find(|line| line.contains("tolerated: metadata_mismatch on npm:react-dom@19.2.0 at ")) - .unwrap_or_default(); - assert!( - line.ends_with("-- npm default start script (scripts.start = \"node server.js\")"), - "{said}" - ); - assert!(!said.contains("waived:"), "{said}"); - assert!( - said.contains( - "waiver matched nothing in this run: metadata_mismatch on npm:react-dom@19.2.0" - ), - "{said}" - ); -} - -/// A waiver with no reason, or no version, is refused at load: exit 2 before -/// any scanner runs. -#[test] -fn a_waiver_without_a_reason_or_a_version_is_refused_at_load() { - for (package, reason, expected) in [ - ("npm:swagger-ui-express@5.0.1", "", "`reason` is empty"), - ( - "npm:swagger-ui-express", - "why", - "`:@`", - ), - ("cargo:serde@1.0.0", "why", "`:@`"), - ] { - let root = repository(); - let path = root.join("policy/principles.toml"); - let policy = std::fs::read_to_string(&path).unwrap(); - std::fs::write( - &path, - format!( - "{policy}\n[[supply_chain.waive]]\nscanner = \"guarddog\"\npackage = \"{package}\"\n\ - check = \"metadata_mismatch\"\nreason = \"{reason}\"\n" - ), - ) - .unwrap(); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", GUARDDOG_CLEAN)]); - let output = supply(&root, Some(&tools)); - assert_eq!(code(&output), 2, "{package}: {}", text(&output)); - assert!( - text(&output).contains(expected), - "{package}: {}", - text(&output) - ); - } -} - -// ── a nested repository's own waivers ── - -/// guarddog's report on `left-pad` 1.3.0 with one `typosquatting` risk, for -/// whichever manifest it is run in: every manifest in these fixtures depends -/// on it, so which of them pass is decided by the waivers alone. -const GUARDDOG_TYPOSQUAT: &str = "echo '[{\"dependency\":\"left-pad\",\"result\":\ - {\"errors\":{},\"issues\":1,\"results\":{},\"package_version\":\"1.3.0\",\ - \"risks\":[{\"name\":\"typosquatting\",\"severity\":\"high\"}]}}]'\nexit 0"; - -/// A manifest depending on `left-pad` 1.3.0 at `directory`. -fn depending_on_left_pad(root: &Path, directory: &str) { - write( - root, - &format!("{directory}/package.json"), - "{\"name\": \"web\", \"dependencies\": {\"left-pad\": \"1.3.0\"}}\n", - ); -} - -/// A `[[supply_chain.waive]]` row for `typosquatting` on `package`. -fn typosquat_waiver(package: &str, reason: &str) -> String { - format!( - "\n[[supply_chain.waive]]\nscanner = \"guarddog\"\npackage = \"{package}\"\n\ - check = \"typosquatting\"\nreason = \"{reason}\"\n" - ) -} +const RETIRED_WAIVER: &str = "\n[[supply_chain.waive]]\nscanner = \"guarddog\"\n\ + package = \"npm:swagger-ui-express@5.0.1\"\ncheck = \"metadata_mismatch\"\n\ + reason = \"a false positive\"\n"; -/// A repository nested in `root` at `directory`, in a submodule's shape: a -/// `.git` FILE pointing into the superproject, which is what `git submodule` -/// leaves where a clone has a directory. `policy` is its policy file's text, -/// at `policy/principles.toml`, or no policy at all. -fn nested_repository(root: &Path, directory: &str, policy: Option<&str>) { - write( - root, - &format!("{directory}/.git"), - &format!("gitdir: ../.git/modules/{directory}\n"), - ); - if let Some(policy) = policy { - write(root, &format!("{directory}/policy/principles.toml"), policy); - } -} - -/// The `guarddog npm` refusal for a manifest at `directory` under `root`. -fn refused_at(root: &Path, directory: &str) -> String { - format!( - "FAILED: guarddog npm: {}: guarddog objected to left-pad@1.3.0", - root.join(directory).display() - ) -} - -/// The live case: a submodule whose own policy waives a confirmed false -/// positive in its own manifest. Scanned from the superproject, that finding -/// is judged by the submodule's row, and the line that lets it through names -/// the submodule's policy file, so whose decision it was is on the line. -/// -/// A nested repository with no policy changes nothing, which is the first run. -#[test] -fn a_nested_repositorys_own_waiver_covers_its_manifests_and_names_its_policy() { - let root = repository(); - depending_on_left_pad(&root, "leaf/web"); - nested_repository(&root, "leaf", None); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", GUARDDOG_TYPOSQUAT)]); - - let refused = supply(&root, Some(&tools)); - assert_eq!(code(&refused), 1, "{}", text(&refused)); - assert!( - text(&refused).contains(&refused_at(&root, "leaf/web")), - "{}", - text(&refused) - ); - - nested_repository( - &root, - "leaf", - Some(&typosquat_waiver( - "npm:left-pad@1.3.0", - "the leaf judged it", - )), - ); - let waived = supply(&root, Some(&tools)); - let said = text(&waived); - assert_eq!(code(&waived), 0, "{said}"); - assert!( - said.contains(&format!( - "waived (leaf/policy/principles.toml): typosquatting on npm:left-pad@1.3.0 at {} \ - -- the leaf judged it", - root.join("leaf/web").display() - )), - "{said}" - ); - assert!(!said.contains("waiver matched nothing"), "{said}"); - assert!(said.contains("all checks passed"), "{said}"); -} - -/// The other half of the boundary. The same row that excuses the leaf's own -/// manifest excuses neither the superproject's nor a sibling submodule's, so -/// one repository's policy cannot pass a finding in a tree it does not own. -/// The sibling has a policy of its own, without the row, so what is tested is -/// whose row it is and not whether a policy is there at all. +/// A `[[supply_chain.waive]]` row naming guarddog, which uphold no longer runs, +/// still loads: the run names the row as ignored on stderr and is not failed +/// by it, so a policy written before the section went keeps working until its +/// owner deletes the row. #[test] -fn a_nested_repositorys_waiver_never_covers_the_superprojects_or_a_siblings_manifests() { +fn a_waiver_naming_guarddog_is_ignored_with_a_warning_and_is_not_a_failure() { let root = repository(); - for directory in ["web", "leaf/web", "sibling/web"] { - depending_on_left_pad(&root, directory); - } - nested_repository( - &root, - "leaf", - Some(&typosquat_waiver( - "npm:left-pad@1.3.0", - "the leaf judged it", - )), - ); - nested_repository(&root, "sibling", Some("allowed_scripts = [\"Latin\"]\n")); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", GUARDDOG_TYPOSQUAT)]); - let output = supply(&root, Some(&tools)); - let said = text(&output); - assert_eq!(code(&output), 1, "{said}"); - assert!(said.contains(&refused_at(&root, "web")), "{said}"); - assert!(said.contains(&refused_at(&root, "sibling/web")), "{said}"); - assert!(!said.contains(&refused_at(&root, "leaf/web")), "{said}"); - assert_eq!( - said.matches("waived (leaf/policy/principles.toml)").count(), - 1, - "{said}" - ); -} - -/// `policy/rg-policy.toml` is a policy `uphold` loads, so it is one here too: -/// the nested repository's file is found by the command's own discovery, not -/// by a second list of names. -#[test] -fn a_nested_repositorys_policy_is_found_under_every_name_uphold_loads() { - let root = repository(); - depending_on_left_pad(&root, "leaf"); - nested_repository(&root, "leaf", None); - write( - &root, - "leaf/policy/rg-policy.toml", - &typosquat_waiver("npm:left-pad@1.3.0", "the leaf judged it"), - ); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", GUARDDOG_TYPOSQUAT)]); - let output = supply(&root, Some(&tools)); - let said = text(&output); - assert_eq!(code(&output), 0, "{said}"); - assert!( - said.contains("waived (leaf/policy/rg-policy.toml)"), - "{said}" - ); -} - -/// A root row that only ever repeats the nested repository's own is a copy of -/// it, and is named as one: the run says which file decides and that the copy -/// can go, which is what removing the copies fleets made before this needs. -/// The root's line format for a row that does the work is unchanged. -#[test] -fn a_root_row_that_repeats_a_nested_repositorys_own_is_named_as_a_removable_copy() { - let root = repository(); - depending_on_left_pad(&root, "leaf/web"); - nested_repository( - &root, - "leaf", - Some(&typosquat_waiver( - "npm:left-pad@1.3.0", - "the leaf judged it", - )), - ); let path = root.join("policy/principles.toml"); let policy = std::fs::read_to_string(&path).unwrap(); - std::fs::write( - &path, - format!( - "{policy}{}", - typosquat_waiver("npm:left-pad@1.3.0", "copied up") - ), - ) - .unwrap(); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", GUARDDOG_TYPOSQUAT)]); + std::fs::write(&path, format!("{policy}{RETIRED_WAIVER}")).unwrap(); + let tools = stubs(&[("osv-scanner", "exit 0")]); let output = supply(&root, Some(&tools)); let said = text(&output); assert_eq!(code(&output), 0, "{said}"); - assert!( - said.contains("waived (leaf/policy/principles.toml)"), - "{said}" - ); - assert!(!said.contains("copied up"), "{said}"); assert!( said.contains( - "waiver repeats leaf/policy/principles.toml: typosquatting on npm:left-pad@1.3.0 \ - -- a repository nearer the manifest waives it in its own policy, so this copy can \ - be removed" + "ignoring `[[supply_chain.waive]]` for npm:swagger-ui-express@5.0.1 \ + (metadata_mismatch)" ), "{said}" ); - assert!(!said.contains("waiver matched nothing"), "{said}"); - - // Where the same row also does work of its own -- a manifest of the - // superproject's -- it is no copy, and nothing is said about it. - depending_on_left_pad(&root, "web"); - let working = supply(&root, Some(&tools)); - let also = text(&working); - assert_eq!(code(&working), 0, "{also}"); - assert!( - also.contains(&format!( - "waived: typosquatting on npm:left-pad@1.3.0 at {} -- copied up", - root.join("web").display() - )), - "{also}" - ); - assert!(!also.contains("waiver repeats"), "{also}"); -} - -/// Three levels: the scan root, `outer` inside it, `inner` inside that. A row -/// in `outer`'s policy about a manifest in `inner`'s tree applies from the -/// root, because `outer` scanned on its own judges that manifest against that -/// row, and a superproject that read only the innermost policy would refuse -/// what the repository between them accepts. `inner` has a policy without the -/// row, so what is tested is that an inner policy does not end the walk. -#[test] -fn an_outer_repositorys_waiver_covers_a_manifest_in_a_repository_inside_it() { - let root = repository(); - depending_on_left_pad(&root, "outer/inner/web"); - nested_repository( - &root, - "outer", - Some(&typosquat_waiver("npm:left-pad@1.3.0", "outer judged it")), - ); - nested_repository( - &root, - "outer/inner", - Some("allowed_scripts = [\"Latin\"]\n"), - ); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", GUARDDOG_TYPOSQUAT)]); - let output = supply(&root, Some(&tools)); - let said = text(&output); - assert_eq!(code(&output), 0, "{said}"); - assert!( - said.contains(&format!( - "waived (outer/policy/principles.toml): typosquatting on npm:left-pad@1.3.0 at {} \ - -- outer judged it", - root.join("outer/inner/web").display() - )), - "{said}" - ); -} - -/// The same three levels the other way round: `inner`'s row covers its own -/// manifest and never `outer`'s, which lies outside `inner`'s tree. -#[test] -fn an_inner_repositorys_waiver_never_covers_the_repository_around_it() { - let root = repository(); - depending_on_left_pad(&root, "outer/web"); - depending_on_left_pad(&root, "outer/inner/web"); - nested_repository(&root, "outer", Some("allowed_scripts = [\"Latin\"]\n")); - nested_repository( - &root, - "outer/inner", - Some(&typosquat_waiver("npm:left-pad@1.3.0", "inner judged it")), - ); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", GUARDDOG_TYPOSQUAT)]); - let output = supply(&root, Some(&tools)); - let said = text(&output); - assert_eq!(code(&output), 1, "{said}"); - assert!(said.contains(&refused_at(&root, "outer/web")), "{said}"); - assert!( - !said.contains(&refused_at(&root, "outer/inner/web")), - "{said}" - ); - assert!( - said.contains(&format!( - "waived (outer/inner/policy/principles.toml): typosquatting on npm:left-pad@1.3.0 \ - at {}", - root.join("outer/inner/web").display() - )), - "{said}" - ); + assert!(said.contains("all checks passed"), "{said}"); } -/// An outer nested repository's row that only repeats the inner one's is -/// named as a copy too, under its own file, and the innermost row is the one -/// on the line that lets the risk through. +/// A waiver naming any other scanner is still refused at load: exit 2 before +/// any scanner runs. Only the one scanner that was removed is read and ignored. #[test] -fn an_outer_repositorys_row_that_repeats_an_inner_one_is_named_as_a_removable_copy() { +fn a_waiver_naming_any_other_scanner_is_refused_at_load() { let root = repository(); - depending_on_left_pad(&root, "outer/inner/web"); - nested_repository( - &root, - "outer", - Some(&typosquat_waiver("npm:left-pad@1.3.0", "copied up")), - ); - nested_repository( - &root, - "outer/inner", - Some(&typosquat_waiver("npm:left-pad@1.3.0", "inner judged it")), - ); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", GUARDDOG_TYPOSQUAT)]); - let output = supply(&root, Some(&tools)); - let said = text(&output); - assert_eq!(code(&output), 0, "{said}"); - assert!( - said.contains("waived (outer/inner/policy/principles.toml)"), - "{said}" - ); - assert!(!said.contains("copied up"), "{said}"); - assert!( - said.contains( - "waiver in outer/policy/principles.toml repeats outer/inner/policy/principles.toml: \ - typosquatting on npm:left-pad@1.3.0 -- a repository nearer the manifest waives it \ - in its own policy, so this copy can be removed" - ), - "{said}" - ); -} - -/// The pushed-range form reaches a nested repository's waiver too: a bumped -/// submodule pointer expands into the member's own diff, and the manifest that -/// moved there is judged against the member's policy. A root row repeating it -/// is not called a copy here, because a range scan reads only what moved and -/// cannot tell a copy from a row that also covers a manifest it did not read; -/// it is not called stale either, because it did cover the risk. -#[test] -fn a_pushed_range_judges_a_moved_submodule_manifest_against_the_submodules_own_waiver() { - let root = tracked(); let path = root.join("policy/principles.toml"); let policy = std::fs::read_to_string(&path).unwrap(); std::fs::write( &path, format!( "{policy}{}", - typosquat_waiver("npm:left-pad@1.3.0", "copied up") + RETIRED_WAIVER.replace("guarddog", "osv-scanner") ), ) .unwrap(); - support::submodule( - &root, - "leaf", - &[ - ( - "policy/principles.toml", - &typosquat_waiver("npm:left-pad@1.3.0", "the leaf judged it"), - ), - ("web/package.json", "{\"name\": \"web\"}\n"), - ], - ); - let before = commit(&root, "track the member"); - let member = root.join("leaf"); - depending_on_left_pad(&member, "web"); - commit(&member, "the member's manifest moves"); - let after = commit(&root, "bump the pointer"); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ("guarddog", &recording(GUARDDOG_TYPOSQUAT)), - ]); - let output = pushed(&root, &tools, &before, &after); - let said = text(&output); - assert_eq!(code(&output), 0, "{said}"); - assert!( - said.contains(&format!( - "waived (leaf/policy/principles.toml): typosquatting on npm:left-pad@1.3.0 at {} \ - -- the leaf judged it", - member.join("web").display() - )), - "{said}" - ); - assert!(!said.contains("waiver repeats"), "{said}"); - assert!(!said.contains("waiver matched nothing"), "{said}"); - assert!(journal(&root).contains("guarddog"), "{}", journal(&root)); -} - -/// A nested repository's row that matched nothing is left to that -/// repository's own scan, which reports it and is the one that can act on it. -/// The superproject does not write that file, and a second reporter of the -/// same stale row is a second voice for one fact. -#[test] -fn a_nested_repositorys_unmatched_waiver_is_reported_by_its_own_scan_alone() { - let root = repository(); - depending_on_left_pad(&root, "leaf/web"); - nested_repository( - &root, - "leaf", - Some(&typosquat_waiver("npm:left-pad@1.2.0", "an older release")), - ); - let tools = stubs(&[("osv-scanner", "exit 0"), ("guarddog", GUARDDOG_TYPOSQUAT)]); - - let from_above = supply(&root, Some(&tools)); - let above = text(&from_above); - assert_eq!(code(&from_above), 1, "{above}"); - assert!(above.contains(&refused_at(&root, "leaf/web")), "{above}"); - assert!(!above.contains("waiver matched nothing"), "{above}"); - - let leaf = root.join("leaf"); - let own = supply(&leaf, Some(&tools)); - let inside = text(&own); - assert_eq!(code(&own), 1, "{inside}"); - assert!( - inside.contains("waiver matched nothing in this run: typosquatting on npm:left-pad@1.2.0"), - "{inside}" - ); -} - -/// A nested policy that will not load leaves its manifests unjudged, as -/// could-not-look naming the repository and the loader's own refusal. It is -/// never read as "no waivers", which would refuse on a guess about findings -/// its owner may have accepted. guarddog is not run there, and a manifest -/// outside that repository is still judged. -#[test] -fn a_nested_policy_that_will_not_load_is_not_checked_rather_than_no_waivers() { - let root = repository(); - depending_on_left_pad(&root, "leaf/web"); - nested_repository(&root, "leaf", Some("this is not = = toml\n")); - let tools = stubs(&[ - ("osv-scanner", "exit 0"), - ("guarddog", &recording(GUARDDOG_CLEAN)), - ]); + let tools = stubs(&[("osv-scanner", "exit 0")]); let output = supply(&root, Some(&tools)); - let said = text(&output); - assert_eq!(code(&output), 2, "{said}"); - assert!(said.contains("NOT CHECKED"), "{said}"); + assert_eq!(code(&output), 2, "{}", text(&output)); assert!( - said.contains(&format!( - "the guarddog findings at {} were not judged: {} is a repository with a policy of \ - its own", - root.join("leaf/web").display(), - root.join("leaf").display() - )), - "{said}" + text(&output).contains("names the scanner \"osv-scanner\""), + "{}", + text(&output) ); - assert!(said.contains("leaf/policy/principles.toml"), "{said}"); - assert!(!journal(&root).contains("guarddog"), "{}", journal(&root)); + assert!(journal(&root).is_empty(), "{}", journal(&root)); } -// ── direct references: git sources, URLs and paths guarddog cannot look up ── +// ── direct references: git sources, URLs and paths no index holds ── /// A repository whose policy declares `owner = "example-org"`. fn owned_by_example_org() -> PathBuf { @@ -3259,9 +2122,7 @@ fn lock_naming(root: &Path, tag: &str, commit: &str) { .unwrap(); } -/// Stubs whose `uv export` prints `lines`, and whose guarddog answers an -/// export still holding a direct reference the way the real tool does -- a -/// 404 from `PyPI` -- and records every run. A `uv.lock` is written where the +/// Stubs whose `uv export` prints `lines`. A `uv.lock` is written where the /// test wrote none, because the section reads nothing without one. fn exporting(root: &Path, lines: &[&str]) -> PathBuf { if !root.join("uv.lock").exists() { @@ -3270,15 +2131,9 @@ fn exporting(root: &Path, lines: &[&str]) -> PathBuf { let quoted: Vec = lines.iter().map(|line| format!("'{line}'")).collect(); stubs(&[ ("osv-scanner", "exit 0"), - ("uv", &format!("printf '%s\\n' {}", quoted.join(" "))), ( - "guarddog", - &recording(&format!( - "grep -q ' @ ' \"$5\" && {{ echo '[{{\"dependency\":\"example-kit\",\ - \"result\":{{\"errors\":{{\"download-package\":\"Received status code: 404 \ - from PyPI\"}},\"issues\":0}}}}]'; exit 0; }}\n{}", - clean_dependencies(&["requests"]) - )), + "uv", + &recording(&format!("printf '%s\\n' {}", quoted.join(" "))), ), ]) } @@ -3301,13 +2156,11 @@ fn supply_with(root: &Path, tools: &Path, environment: &[(&str, String)]) -> Out invoke(root, &path, &["--all"], &borrowed) } -/// The defect this section had: a first-party package depended on through a -/// uv git source was handed to guarddog, which looked it up on `PyPI`, got a -/// 404, and made every push exit 2. Now the git source is asked of its own -/// remote -- the tag the lock names is there and points at the locked commit -/// -- and guarddog is handed the `PyPI` requirements alone, and still runs. +/// A first-party package depended on through a uv git source is asked of its +/// own remote: the tag the lock names is there and points at the locked +/// commit. #[test] -fn a_first_party_git_source_whose_tag_resolves_is_not_sent_to_guarddog_and_passes() { +fn a_first_party_git_source_whose_tag_resolves_passes() { let root = owned_by_example_org(); let (config, tagged, _) = example_kit_remote(); lock_naming(&root, "v0.1.0", &tagged); @@ -3329,7 +2182,11 @@ fn a_first_party_git_source_whose_tag_resolves_is_not_sent_to_guarddog_and_passe )), "{said}" ); - assert!(journal(&root).contains("guarddog"), "{}", journal(&root)); + assert!( + journal(&root).contains("export --frozen"), + "{}", + journal(&root) + ); assert!(said.contains("all checks passed"), "{said}"); } @@ -3435,7 +2292,7 @@ fn a_first_party_remote_that_cannot_be_asked_is_could_not_look() { } /// A git source under an owner other than the declared one is refused by -/// name, and never reaches guarddog or the remote. +/// name, and never reaches the remote. #[test] fn a_git_source_under_another_owner_is_refused_by_name() { let root = owned_by_example_org(); @@ -3501,39 +2358,18 @@ fn a_direct_url_and_a_path_outside_the_tree_are_refused_by_name() { said.contains("../outside-the-tree is a path dependency outside this repository"), "{said}" ); - // The `PyPI` half was still scanned. - assert!(journal(&root).contains("guarddog"), "{}", journal(&root)); -} - -/// A repository whose only dependencies are its own members and a first-party -/// git source has nothing on `PyPI` to look up, and says so rather than handing -/// guarddog an empty list it would answer with `[]`. -#[test] -fn an_export_with_nothing_from_an_index_does_not_ask_guarddog() { - let root = owned_by_example_org(); - std::fs::create_dir_all(root.join("packages/member")).unwrap(); - let (config, _, tip) = example_kit_remote(); - let tools = exporting( - &root, - &[ - "-e .", - "-e ./packages/member", - &format!("example-kit @ git+https://github.com/example-org/example-kit@{tip}"), - ], + // The export was still read. + assert!( + journal(&root).contains("export --frozen"), + "{}", + journal(&root) ); - let output = supply_with(&root, &tools, &forge_environment(&config)); - let said = text(&output); - assert_eq!(code(&output), 0, "{said}"); - assert!(said.contains("guarddog was not asked"), "{said}"); - assert!(!journal(&root).contains("guarddog"), "{}", journal(&root)); } // ── npm git dependencies ── /// A `package.json` whose `dependencies` hold `spec` for example-kit beside a -/// registry range, a `bun.lock` recording `commit` for it, and stubs whose -/// guarddog answers a manifest still naming example-kit the way the real tool -/// does -- a 404 from npm -- and records every run. +/// registry range, a `bun.lock` recording `commit` for it, and stub scanners. fn npm_depending_on(root: &Path, spec: &str, commit: &str) -> PathBuf { std::fs::write( root.join("package.json"), @@ -3551,25 +2387,13 @@ fn npm_depending_on(root: &Path, spec: &str, commit: &str) -> PathBuf { ), ) .unwrap(); - stubs(&[ - ("osv-scanner", "exit 0"), - ( - "guarddog", - &recording(&format!( - "grep -q example-kit \"$5\" && {{ echo '[{{\"dependency\":\"example-kit\",\ - \"result\":{{\"errors\":{{\"download-package\":\"Received status code: 404 \ - from npm\"}},\"issues\":0}}}}]'; exit 0; }}\n{GUARDDOG_CLEAN}" - )), - ), - ]) + stubs(&[("osv-scanner", "exit 0")]) } -/// The npm half of the defect: a first-party package depended on by git was -/// handed to guarddog, which asked npm for it, got a 404 and made every run -/// exit 2. The tag the manifest names is now asked of the remote, against the -/// commit `bun.lock` holds, and guarddog reads the registry dependencies alone. +/// The npm half: a first-party package depended on by git has the tag the +/// manifest names asked of the remote, against the commit `bun.lock` holds. #[test] -fn a_first_party_npm_git_dependency_whose_tag_resolves_is_not_sent_to_guarddog_and_passes() { +fn a_first_party_npm_git_dependency_whose_tag_resolves_passes() { let root = owned_by_example_org(); let (config, tagged, _) = example_kit_remote(); let tools = npm_depending_on( @@ -3587,7 +2411,6 @@ fn a_first_party_npm_git_dependency_whose_tag_resolves_is_not_sent_to_guarddog_a )), "{said}" ); - assert!(journal(&root).contains("guarddog"), "{}", journal(&root)); assert!(said.contains("all checks passed"), "{said}"); } @@ -3603,7 +2426,7 @@ fn a_first_party_npm_tag_that_points_elsewhere_is_refused() { } /// A git dependency under another owner is refused by name, and is never -/// handed to guarddog to 404 on. +/// asked of the remote. #[test] fn an_npm_git_dependency_under_another_owner_is_refused_by_name() { let root = owned_by_example_org();