mozilla / cargo-vet

supply-chain security for Rust
Apache License 2.0
669 stars 46 forks source link

Changing criteria on one workspace crate affects criteria applied to others #365

Open divergentdave opened 1 year ago

divergentdave commented 1 year ago

I experimented with setting up cargo vet on a couple repositories that have the following setup: some crates which we publish, and some other testing/development-only crates in the same workspace that depend on the published crates. If I set the criteria on testing crates to safe-to-run in the configuration file, then when I run cargo vet suggest, I only get suggestions for safe-to-run audits, and no suggestions for safe-to-deploy audits. If I explicitly set the criteria for all workspace crates in the configuration file, then cargo vet suggest gives me separate nonempty lists of suggested safe-to-run and safe-to-deploy audits, which is the behavior I want. It sounds like, via dependency edges, the policy changes on one workspace crate are affecting another workspace crate, and overriding the expected default behavior of safe-to-deploy for all workspace crates.

Here's a simplified example:

$ tree
.
├── Cargo.lock
├── Cargo.toml
├── crate-a
│   ├── Cargo.toml
│   └── src
│       └── lib.rs
├── crate-b
│   ├── Cargo.toml
│   └── src
│       └── lib.rs
└── supply-chain
    ├── audits.toml
    ├── config.toml
    └── imports.lock

5 directories, 9 files

$ cargo tree
crate-a v0.1.0 (/home/david/scratch/cargo-vet-workspace-crate-policies/crate-a)
└── ring v0.16.20
    ├── libc v0.2.138
    ├── once_cell v1.16.0
    ├── spin v0.5.2
    └── untrusted v0.7.1
    [build-dependencies]
    └── cc v1.0.78

crate-b v0.1.0 (/home/david/scratch/cargo-vet-workspace-crate-policies/crate-b)
├── crate-a v0.1.0 (/home/david/scratch/cargo-vet-workspace-crate-policies/crate-a) (*)
└── serde v1.0.151

No policy configurations, OK:

$ cargo vet suggest
recommended audits for safe-to-deploy:
    cargo vet inspect serde 1.0.151                       (used by crate-b)                                           (16352 lines)
    cargo vet inspect ring 0.16.20                        (used by crate-a)                                           (264901 lines)
    cargo vet inspect wasm-bindgen-shared 0.2.83          (used by wasm-bindgen-backend, wasm-bindgen-macro-support)  (519 lines)
    cargo vet inspect cfg-if 1.0.0                        (used by log, wasm-bindgen)                                 (588 lines)
    cargo vet inspect untrusted 0.7.1                     (used by ring)                                              (720 lines)
    cargo vet inspect winapi-i686-pc-windows-gnu 0.4.0    (used by winapi)                                            (1133 lines)
    cargo vet inspect winapi-x86_64-pc-windows-gnu 0.4.0  (used by winapi)                                            (1168 lines)
    cargo vet inspect wasm-bindgen-macro 0.2.83           (used by wasm-bindgen)                                      (1308 lines)
    cargo vet inspect spin 0.5.2                          (used by ring)                                              (1660 lines)
    cargo vet inspect wasm-bindgen-macro-support 0.2.83   (used by wasm-bindgen-macro)                                (2054 lines)
    cargo vet inspect unicode-ident 1.0.6                 (used by syn, proc-macro2)                                  (2772 lines)
    cargo vet inspect wasm-bindgen-backend 0.2.83         (used by wasm-bindgen-macro-support)                        (3014 lines)
    cargo vet inspect quote 1.0.23                        (used by syn, and 3 others)                                 (3873 lines)
    cargo vet inspect once_cell 1.16.0                    (used by ring, wasm-bindgen-backend)                        (4494 lines)
    cargo vet inspect log 0.4.17                          (used by wasm-bindgen-backend)                              (5695 lines)
    cargo vet inspect proc-macro2 1.0.49                  (used by syn, quote, and 2 others)                          (6078 lines)
    cargo vet inspect cc 1.0.78                           (used by ring)                                              (7019 lines)
    cargo vet inspect bumpalo 3.11.1                      (used by wasm-bindgen-backend)                              (10640 lines)
    cargo vet inspect js-sys 0.3.60                       (used by web-sys)                                           (12966 lines)
    cargo vet inspect wasm-bindgen 0.2.83                 (used by js-sys, web-sys)                                   (21672 lines)
    cargo vet inspect syn 1.0.107                         (used by wasm-bindgen-backend, wasm-bindgen-macro-support)  (57688 lines)
    cargo vet inspect libc 0.2.138                        (used by ring)                                              (99364 lines)
    cargo vet inspect winapi 0.3.9                        (used by ring)                                              (181329 lines)
    cargo vet inspect web-sys 0.3.60                      (used by ring)                                              (202213 lines)

estimated audit backlog: 909220 lines

Use |cargo vet certify| to record the audits.

After setting policy.crate-b.criteria = "safe-to-run", I get this unexpected behavior:

$ cargo vet suggest
recommended audits for safe-to-run:
    cargo vet inspect serde 1.0.151                       (used by crate-b)                                           (16352 lines)
    cargo vet inspect ring 0.16.20                        (used by crate-a)                                           (264901 lines)
    cargo vet inspect wasm-bindgen-shared 0.2.83          (used by wasm-bindgen-backend, wasm-bindgen-macro-support)  (519 lines)
    cargo vet inspect cfg-if 1.0.0                        (used by log, wasm-bindgen)                                 (588 lines)
    cargo vet inspect untrusted 0.7.1                     (used by ring)                                              (720 lines)
    cargo vet inspect winapi-i686-pc-windows-gnu 0.4.0    (used by winapi)                                            (1133 lines)
    cargo vet inspect winapi-x86_64-pc-windows-gnu 0.4.0  (used by winapi)                                            (1168 lines)
    cargo vet inspect wasm-bindgen-macro 0.2.83           (used by wasm-bindgen)                                      (1308 lines)
    cargo vet inspect spin 0.5.2                          (used by ring)                                              (1660 lines)
    cargo vet inspect wasm-bindgen-macro-support 0.2.83   (used by wasm-bindgen-macro)                                (2054 lines)
    cargo vet inspect unicode-ident 1.0.6                 (used by syn, proc-macro2)                                  (2772 lines)
    cargo vet inspect wasm-bindgen-backend 0.2.83         (used by wasm-bindgen-macro-support)                        (3014 lines)
    cargo vet inspect quote 1.0.23                        (used by syn, and 3 others)                                 (3873 lines)
    cargo vet inspect once_cell 1.16.0                    (used by ring, wasm-bindgen-backend)                        (4494 lines)
    cargo vet inspect log 0.4.17                          (used by wasm-bindgen-backend)                              (5695 lines)
    cargo vet inspect proc-macro2 1.0.49                  (used by syn, quote, and 2 others)                          (6078 lines)
    cargo vet inspect cc 1.0.78                           (used by ring)                                              (7019 lines)
    cargo vet inspect bumpalo 3.11.1                      (used by wasm-bindgen-backend)                              (10640 lines)
    cargo vet inspect js-sys 0.3.60                       (used by web-sys)                                           (12966 lines)
    cargo vet inspect wasm-bindgen 0.2.83                 (used by js-sys, web-sys)                                   (21672 lines)
    cargo vet inspect syn 1.0.107                         (used by wasm-bindgen-backend, wasm-bindgen-macro-support)  (57688 lines)
    cargo vet inspect libc 0.2.138                        (used by ring)                                              (99364 lines)
    cargo vet inspect winapi 0.3.9                        (used by ring)                                              (181329 lines)
    cargo vet inspect web-sys 0.3.60                      (used by ring)                                              (202213 lines)

estimated audit backlog: 909220 lines

Use |cargo vet certify| to record the audits.

$ cargo vet dump-graph
graph LR
    subgraph roots
        node4{crate-b:0.1.0}
    end
    subgraph workspace-members
        node3[/crate-a:0.1.0/]
    end
    subgraph first-party
    end
    subgraph third-party
    end
    node4 --> node3

Lastly, I set both policy.crate-a.criteria = "safe-to-deploy" and policy.crate-b.criteria = "safe-to-run", and get the behavior I want:

$ cargo vet suggest
recommended audits for safe-to-deploy:
    cargo vet inspect ring 0.16.20                        (used by crate-a)                                           (264901 lines)
    cargo vet inspect wasm-bindgen-shared 0.2.83          (used by wasm-bindgen-backend, wasm-bindgen-macro-support)  (519 lines)
    cargo vet inspect cfg-if 1.0.0                        (used by log, wasm-bindgen)                                 (588 lines)
    cargo vet inspect untrusted 0.7.1                     (used by ring)                                              (720 lines)
    cargo vet inspect winapi-i686-pc-windows-gnu 0.4.0    (used by winapi)                                            (1133 lines)
    cargo vet inspect winapi-x86_64-pc-windows-gnu 0.4.0  (used by winapi)                                            (1168 lines)
    cargo vet inspect wasm-bindgen-macro 0.2.83           (used by wasm-bindgen)                                      (1308 lines)
    cargo vet inspect spin 0.5.2                          (used by ring)                                              (1660 lines)
    cargo vet inspect wasm-bindgen-macro-support 0.2.83   (used by wasm-bindgen-macro)                                (2054 lines)
    cargo vet inspect unicode-ident 1.0.6                 (used by syn, proc-macro2)                                  (2772 lines)
    cargo vet inspect wasm-bindgen-backend 0.2.83         (used by wasm-bindgen-macro-support)                        (3014 lines)
    cargo vet inspect quote 1.0.23                        (used by syn, and 3 others)                                 (3873 lines)
    cargo vet inspect once_cell 1.16.0                    (used by ring, wasm-bindgen-backend)                        (4494 lines)
    cargo vet inspect log 0.4.17                          (used by wasm-bindgen-backend)                              (5695 lines)
    cargo vet inspect proc-macro2 1.0.49                  (used by syn, quote, and 2 others)                          (6078 lines)
    cargo vet inspect cc 1.0.78                           (used by ring)                                              (7019 lines)
    cargo vet inspect bumpalo 3.11.1                      (used by wasm-bindgen-backend)                              (10640 lines)
    cargo vet inspect js-sys 0.3.60                       (used by web-sys)                                           (12966 lines)
    cargo vet inspect wasm-bindgen 0.2.83                 (used by js-sys, web-sys)                                   (21672 lines)
    cargo vet inspect syn 1.0.107                         (used by wasm-bindgen-backend, wasm-bindgen-macro-support)  (57688 lines)
    cargo vet inspect libc 0.2.138                        (used by ring)                                              (99364 lines)
    cargo vet inspect winapi 0.3.9                        (used by ring)                                              (181329 lines)
    cargo vet inspect web-sys 0.3.60                      (used by ring)                                              (202213 lines)

recommended audits for safe-to-run:
    cargo vet inspect serde 1.0.151  (used by crate-b)  (16352 lines)

estimated audit backlog: 909220 lines

Use |cargo vet certify| to record the audits.

I installed cargo-vet from commit 61d140dabe996938cf10900b73e93b705270cc9f.

bholley commented 1 year ago

This seems like a bug. We'll look into it.

mystor commented 1 year ago

This is caused by the way that cargo-vet thinks about roots and crate graphs. Rather than considering every crate in your workspace as an individual root, crates are considered roots if they have no reverse-dependencies. This prevents the default per-root policy from applying to any crates which could instead have requirements enforced by dependency edges.

This ends up impacting your case, as the test crate depends on your primary crate, so the primary crate is not considered a root. This leads to the primary crate's requirements to be inferred from the test crate's through the dependency edge, which adds a safe-to-run requirement to it.

The decision was made originally not not have the policy apply to each workspace member individually for use-cases where the workspace contains dependency trees used to implement internal tooling. For example, in Gecko we have a geckodriver crate which depends on other internal crates such as marionette and mozversion. If every workspace crate was considered a root, you'd need to specify a policy for each crate in the workspace to reduce the geckodriver subtree to safe-to-run, whereas with this model, it happens by inheritance.

I can't think of a way off the top of my head to detect and handle both this case and the geckodriver case automatically, as they look the same in the dependency graph. The easiest approach right now is to explicitly specify the safe-to-deploy policy for each crate which is depended on by a safe-to-run crate.

We might be able to add an option to toggle the behaviour for what to treat as a root for evaluation, or perhaps add some option to the policy making in-workspace dependencies be treated as roots, if this turns out to be a big issue.