Consider sub-crate feature exclusions for audits

[bholley]

Generally speaking cargo-vet always operates at crate granularity, and we've resisted anything finer-grained on the grounds that it's difficult to reason about, difficult to implement, and of questionable utility in practice.

Earlier today, @selenography was describing his in-progress audit of reqwest, and asking whether there was a way to exclude the socks feature, which was a lot of unfamiliar code which we aren't using anyway. This led @mystor to formulate a feature description that I thought was interesting enough to at least document and discuss further.

The key insight is that cargo-vet already assumes that any code not reachable by enabling all features on the top-level crates (i.e., passing --all-features to cargo-metadata) can be excluded from analysis. At the moment this happens by dropping conditional dependencies which can never be built, because that's all cargo-vet knows how to do. However, we could also apply this same conditionality to the inclusion/exclusion of audits.

A rough sketch of this would be an optional exclude-features = [...] field on audit entries. Cargo-vet would only consider the audit if it were able to verify that none of the features in the list could ever be enabled on that node in the crate graph. Getting this information out of cargo-metadata might be anywhere between trivial and impossible, I haven't investigated.

You could then imagine an additional add-features = [...] field, which would allow you to patch in coverage to an existing audit. So if @selenography audited reqwest-x.y.z without socks, and somebody comes along and wants to use socks, they could just audit the socks part, and cargo-vet could combine the two.

[mystor]

In terms of how difficult exclude-features is to implement, I think it should be fairly straightforward. Cargo doesn't support disabling a feature in a dependency when a feature is enabled (features are additive) so when we run cargo metadata with --all-features, we're seeing the maximal crate graph from the metadata command. We already have the list of enabled features for each package node in the graph (https://docs.rs/cargo_metadata/latest/cargo_metadata/struct.Node.html#structfield.features), so we could effectively disable an audit edge with exclude-features = [...] if any of the listed features are present in the given node.

This has a slightly awkward interaction with the evolution of crates over time, as a given feature could e.g. no longer exist in a newer version (as all checks are done against the current version in the dependency graph, as we don't know e.g. "what features would be enabled on a previous version X of this crate"), or the feature could be renamed.

The add-features idea is considerably more complicated, as it would require being able to combine together multiple separate audit paths to prove a single criteria for a crate, which isn't something our architecture is capable of handling. It might be possible to do with some changes to the resolver, but I haven't looked into it.

[djkoloski]

We'd also benefit from this on Fuchsia. There are a number of crates with features that we don't review code for, since we don't ever enable it.

[danakj]

Chromium will also benefit from this as we attempt to adopt vet.

[anforowicz]

I note that audits that ignore a particular crate feature are just one kind of a partial audit. Another example of a partial audit is ignoring target platforms / OS-es that are irrelevant to the auditor - e.g. Chromium may want to ignore libc-0.2.152/src/unix/hurd/… while still auditing code for platforms relevant to Chromium. OTOH other auditors may ignore some of the Chromium platforms.

I guess the observation above doesn't necessarily mean that "sub-crate feature exclusions for audits" wouldn't be useful to have in cargo vet. OTOH, maybe this observation helps to look at the problem from another angle - for example, maybe just knowing if an audit covered the whole crate is sufficient in many cases.

Just my 2 cents...