From 039319dcbe2d4b6adf34ad764f1219792b9c9c19 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Fri, 4 Apr 2025 19:07:28 -0400 Subject: [PATCH] Fix `cargo deny` commands so they scan the whole workspace When `cargo deny` commands are run from the top-level directory of a workspace, and no `--manifest-path` option is pased, they implicitly use the top-level `Cargo.toml` file as their manifest. In workspaces where this is a virtual manifest, i.e. those where the top-level `Cargo.toml` defines only the workspace and not also a specific crate, running `cargo deny` without specifying any roots implicitly acts as though `--workspace` has been passed. (See https://embarkstudios.github.io/cargo-deny/cli/common.html for details.) In that situation, `--workspace` can be omitted and all crates in the workspace are still treated as roots for the scan. But the gitoxide repository's top-level workspace's `Cargo.toml` file is not a virtual manifest. It defines a workspace, but it also defines the `gitoxide` crate. As a result, `cargo deny` commands, as run on CI in the `cargo-deny` and `cargo-deny-advisories` jobs, and locally via `just audit`, were not guaranteed to check the entire workspace. They used only the `gitoxide` crate as a root for scanning, rather than using all crates defined in the workspace as roots as when `--workspace` is used or implied. It looks like this was not detected for three reasons: 1. Most of the workspace was usually covered, especially on CI where, since no `arguments` were passed, `cargo-deny-action` used the defualt of `--all-features`. *Most* transitive dependencies inside and outside of the gitoxide project seem to be used by the `gitoxide` crate by in some feature configuration. 2. The distinction between virtual and non-virtual top-level manifests of workspaces is subtle and not highlighted in the `cargo deny` documentation. 3. In `EmbarkStudios/cargo-deny-action`, the default value of `arguments` contains `--all-features` but not `--workspace` (nor any other options). Intuitively it feels like the default value would scan all crates in the repository that the action is being run on. That does happen in the perhaps more common case that the top-level `Cargo.toml` defines no crate, but not here. This was discovered in connection with #1924. It is why the `cargo-deny-advisories` job didn't catch RUSTSEC-2025-0022 (GHSA-4fcv-w3qc-ppgg) even as Dependabot did. To fix it, this adds `--workspace` explicitly in both `cargo deny` jobs run on CI, as well as in the `justfile`. This also adds `--all-features`: - On CI, adding `--all-features` is itself no change from before, since it is the implicit value of `arguments`. It has to be passed explicitly now that `arguments` has an explicit value. - In the `justfile`, adding `--all-features` does (at least in principle) make a difference. --- .github/workflows/ci.yml | 2 ++ justfile | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 26d4755da19..a5c58d4e01a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -344,6 +344,7 @@ jobs: - uses: actions/checkout@v4 - uses: EmbarkStudios/cargo-deny-action@v2 with: + arguments: --workspace --all-features command: check advisories cargo-deny: @@ -353,6 +354,7 @@ jobs: - uses: actions/checkout@v4 - uses: EmbarkStudios/cargo-deny-action@v2 with: + arguments: --workspace --all-features command: check bans licenses sources wasm: diff --git a/justfile b/justfile index f143bc06abc..bf059b8e410 100755 --- a/justfile +++ b/justfile @@ -240,7 +240,7 @@ nix-shell-macos: # Run various auditing tools to help us stay legal and safe audit: - cargo deny check advisories bans licenses sources + cargo deny --workspace --all-features check advisories bans licenses sources # Run tests with `cargo nextest` (all unit-tests, no doc-tests, faster) nextest *FLAGS='--workspace':