Thanks for the writeup @untitaker! I’m sorry I haven’t been able to keep up with much of the conversation up to this point, but I figure it’s about time I write down some thoughts 
In general I agree and like the motivation section. I think there’s a real use case for publishing and notifying about security vulnerabilities on crates.io, and doing this through Cargo seems quite natural and what one might expect. Additionally I think it’s reasonable to avoid yank as the alternatives section outlines as well.
It may be worth putting some thought into the motivation section, though, in terms of ergonomics of a separate cargo advisory command. For example is the motivation to weed out vulnerable crates in the ecosystem ASAP, or is it to primarily just notify developers? To me there’s a spectrum of how “hard” Cargo can be about using a vulnerable crate, and we can certainly tweak the slider to see where we come up on that spectrum.
I personally would advocate for something which retains the current ergonomics of Cargo. That is, a cargo advisory feature would:
- Not overly pollute build logs
- Never break an existing build
- Be lightweight and easy to use without lots of process
(etc). Some of my comments below I believe will touch on this as well!
I’d recommend just requiring --vers. I think it’s easier to understand and I’d prefer to not have subtle differences between cargo yank and cargo advisory. I think it’d be fine to just run the command once per version as well, in theory this is something that’s called very rarely.
Cargo currently doesn’t do this, and it’s a tricky thing to do across platforms, so perhaps an alternative strategy could be taken? I’d recommend Cargo having the ability to generate a template (like you pasted) and then that template is passed as an argument to cargo advisory. Cargo can still do validation and print out summary information, but it avoids all the headaches of interactive CLI tools by sidestepping the problem!
My recommendation would be that if these are optional keys they don’t need to be included at all, e.g. omission is allowed.
Note that I very much like that the only required field here is a description, it feels like the right balance for “I can get something out” vs “I did all the rigamarole and I want to message everything”
Another thought of mine, can you add two advisory reports to a version of a crate? I’d argue that this should be allowed. For example a low-risk advisory could be issued but then later a more serious advisory could be added which should be messaged out again.
To me this I think will be the most contentious part of the RFC. I personally would find this behavior a non-starter, for example, as it would drastically change the ergonomics of Cargo in my opinion. In the few times I’ve used npm I’m constantly getting deprecation warnings about packages I’ve never heard of. Not only do I not know how to fix the issue but it’s also not always my problem (some upstream maintainer may need to update to a new major version).
So put another way, I feel like this is making Cargo far too harsh in terms of messaging this out. I think that messaging out is crucial, but we need to find a right balance here. If Cargo is constantly telling you that you’re using vulnerable packages, then we run the risk of developers conditioning themselves to just ignore all the reports, defeating the purpose of the feature in the first place!
I unfortunately don’t have a great concrete recommendation to give here, though. We could explore possibilities like “warn once a day” or “once a week” or “only on a new dependency being formed”. It may help to explicitly outline use cases of where an advisory would be read and fixed, and then given those scenarios we could design a messaging system that is effective but also unobtrusive for other scenarios.
I would personally also find this as a bit too harsh and go against my personal goal of “Cargo never breaks your build”. I feel that with the right messaging strategy we won’t need a restriction like this.
I feel like this is on the right track, and I like the idea that cargo publish has stricter requirements than other commands (as it’s so much more rarely called). I feel like it may be a bit too harsh, though, to reject a publish if any vulnerable version matches. As you mentioned there are legitimate use cases for relying on vulnerable crates, so I’d perhaps recommend the logic of if a fresh resolution has any direct vulnerable dependencies the publish is aborted. This abort-by-default behavior, though, could be papered over with a flag.
I also feel that allow-vulnerablein Cargo.toml is perhaps a little heavyweight for this use case, but maybe a flag doesn’t always suffice?
Sorry if some of that is dragging up discussions that have already happened, but feel free to just point me to existing threads if that’s the case!
