feat: include cargo trees in advisory reports#52
Conversation
| let auditReport = vulnerableAuditReport; | ||
| let cargoTreeExitCode = 0; | ||
|
|
||
| async function importMain(): Promise<void> { |
There was a problem hiding this comment.
This is a bit fragile, importing main.ts right now has side effects.
Doing something like this:
if (require.main === module) {
main();
}
...might break the runtime usage (IDK js module loading that well) but we could also just split into an inner entrypoint file.
|
Don't think I'll be able to review this. @tarcieri maybe? |
|
I'd personally like to retire this whole thing and replace it with an extremely simple GitHub Action that just installs and runs the CLI, which would solve this same problem. I thought we had a statement on this repo about how it's in life support mode and not accepting new features, but maybe not. |
This is essentially what cargo-deny does, along with some more stuff (license checks, etc). I can go discuss adding an "open an issue" hook with them instead, if we are trying to sunset this project. That's the main thing they are missing compared to audit-check. |
Closes: #5
When getting a vulnerability finding, you immediately want to see where it comes from.
Often that is based on cargo features (example: aws sdks bring in old rustls stuff with their default feature set, and lots of libraries don't have
--no-default-features).This change dumps an inverted cargo tree for any findings, including features, into the report.
It is on by default, with no opt-out. I can make it opt-out or opt-in if desired. From experience maintaining similar tooling inside my company, you basically always want to show it, saves a hop for any debug.
Example:
On failure, it shows the error.
Testing
I added some e2e snapshot tests with mocked cargo audit and cargo tree outputs, covering happy and sad paths.
I also manually tested the commands myself locally (though I didn't spin up a local github action runner).
I didn't add any unit tests exercising the real
cargo treeinvocation, there wasn't an integration harness yet, but i could be convinced to add it if desired.