prefer local wildcard audits to remote regular audits

[bholley]

Over on mozilla-central, @jrmuizel and @glandium identified an interesting issue where Jeff was trying to replace a regular audit with a wildcard audit. Rather than reifying the local wildcard audit, cargo-vet instead re-fetches the locally-deleted regular audit from the aggregated mozilla-wide set and re-inserts it as an import.

I'm assuming this is because the precedence order is currently:

1. local regular audits
2. remote regular audits
3. local wildcard audits
4. remote wildcard audits

Ideally we'd swap 2 and 3.

@afranchuk Is my hypothesis correct?

[afranchuk]

I believe so after reading the code, and will verify with a potential fix shortly. As it currently exists, local and remote wildcard audits are put together in a single enum variant when ordering, so that may explain the resulting behavior.

[mystor]

Another option here might be for us to include a mechanism by which you can identify your repository's "public url" (for m-c that'd be "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml"), which would prevent an audit with that aggregated-from URI from being imported, avoiding the cyclic import issue when an entry is removed (though generally removing entries is unsupported).

[bholley]

This issue only comes up when you're removing an audit but are still using it (causing it to be re-imported). I think if we just make sure to always prioritize local audits over remote ones, we should never hit the case where we re-import an audit that could otherwise be dropped.

[mystor]

The reason why it came up in this situation is that a fresh wildcard audit is always somewhat remote (as it requires importing a publisher entry), and there is no differentiation in the resolver right now between a wildcard audit which is partially remote vs. fully remote (which I am guessing is what @afranchuk is proposing adding).

I just figured I'd mention the aggregated-from option because otherwise it would be possible for someone to remove an audit (which they shouldn't be doing, but i'm sure someone will do from time to time), and then have their audits start failing later when aggregation is complete.

[bholley]

I see. That's a fair point, but I'd still opt not to add a user-facing configuration field for such an edge case.

[afranchuk]

I have a script which easily reproduces this problem, and have a set of changes which fixes it. Or half-fixes it: after a cargo vet certify --wildcard it'll still have the old import (which is not "fresh" at that point, having been imported when the mozilla import was added), but subsequent operations prompt the user to prune (and it does remove the unneeded import). I'm wondering whether this is acceptable and desirable behavior? If so, I can convert the state of my test script into a unit test and push up a PR.

[bholley]

That sound fine to me, I think that's consistent with other behavior around e.g. exemptions (certify doesn't implicitly prune).