search

mozilla / cargo-vet

supply-chain security for Rust
Apache License 2.0
649 stars 43 forks source link

Audit requirements may be overly strong when dealing with multiple root crates in a workspace #626

Open mystor opened 2 months ago

mystor commented 2 months ago

This was noticed in https://bugzilla.mozilla.org/show_bug.cgi?id=1907810, specifically because of the changes in https://phabricator.services.mozilla.com/D212959#inline-1194604.

If we have multiple crates in the local workspace which have different audit requirements, it is possible for an overly strong requirement to be placed on an indirect dependency.

Specifically if we have a dependency graph like the following, where W1 and W2 are toplevel workspace crates, D1 and D2 are third-party dependencies, where D1 optionally depends on D2. W1 does not enable this dependency but W2 does. If W1 has stronger audit requirements than W2 does, those stronger requirements can end up applying to D2, as the two D1 nodes are unified when running cargo metadata to have the combined feature set.

W1[req:safe-to-deploy] -> D1[features:]
W2[req:safe-to-run] -> D1[features:D2] -> D2

(D2 should require "safe-to-run", but will instead require "safe-to-deploy")

Unfortunately, I don't think that the output of cargo metadata really provides a way to solve this ambiguity and get what the resolution would be for each workspace crate independently, such that we could treat the two D1 dependencies as separate "nodes", to not propagate the same audit requirements to their dependencies.

It might be possible if we didn't trust the dependency resolution done by cargo and implemented feature resolution ourselves, but that seems like it'd be both a lot of work, and likely to break as cargo updates and changes how feature resolution is handled.

afranchuk commented 2 months ago

I've experimented a bit and it looks like using cargo tree --format "{p} {f}" --package CRATE will show the correct subset of features and thus dependent crates. Unfortunately, though, cargo tree doesn't support an output format more easily ingested by programs. It'd be cool if cargo metadata supported a --package flag (like many other commands do).

mystor commented 2 months ago

Unfortunately it does look like if you run the cargo tree without a specific --package flag that it'll still unify features, as the in the example from that bug, the neqo-udp crate still has a tokio dependency even under the gkrust crate unless you explicitly scope to only the gkrust package.

Does seem like cargo metadata would need a --package flag and we'd need a reliable way to enumerate all packages within the workspace so that we could do separate metadata runs for each one. On top of that we'd also need to tweak the graph building for the resolver and such so that dependency lists and inherited audit requirements could be dependent on which package the dependency graph is coming from.