-
Notifications
You must be signed in to change notification settings - Fork 33
Use cargo SBOM precursor files, if available #213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
If cargo sbom function is enabled, cargo-auditable will read the SBOM precursor file and use it to generate dependency information rather than trying to use the `cargo metadata` command.
Thanks a lot! I'll take a closer look in the next few days.
Looks like a bug in Cargo's SBOM support. I don't think we can actually ship with a bug like that. @arlosi are you aware of this issue? Should we file a bug upstream against Cargo? |
Isn't that a case of cargo being more accurate than cargo metadata? Without a build.rs, a crate can have no build dependencies, regardless of what is declared in Cargo.toml. |
Ah, you are probably right! I am a little rusty on the finer points of Cargo dependencies. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've only done a cursory look so far - it's a really hot day, sorry 😅
By and large this looks great! I've noted some nits, and I'll take a closer look at the format transformation algorithm in the next few days.
Thanks again!
@@ -21,6 +21,7 @@ cargo_metadata = "0.18" | |||
pico-args = { version = "0.5", features = ["eq-separator", "short-space-opt"] } | |||
serde = "1.0.147" | |||
wasm-gen = "0.1.4" | |||
cargo-util-schemas = "0.8.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pulls in a really sprawling dependency tree. If we're only using two structs from it anyway, I'd prefer to just copy-paste their definitions and avoid this massive dependency bloat.
cargo auditable
is meant to be used by companies too, and I shudder at the thought of dragging all that through a security review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I was going to ask about this, all 400 lines of cargo.lock additions are from this dep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the new dependencies result from cargo-util-schemas
depending on url
, which is necessary to parse the package id spec that is part of the SBOM precursor. So I suggest we just keep the dependency here?
Since cargo-util-schemas is a dependency of cargo, any company that is reviewing all its dependencies will already depend on some versions of the new deps via cargo anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to parse the package ID spec with the full generality of URLs?
Can we use either a stripped-down version of URL crate with all the fancy features disabled, or just rely on the crate names being ASCII without weird characters?
Also, if the package ID spec cannot be treated as mostly opaque and we need the full generality of the URL crate to actually parse it, that's an issue with the format that I'd like to get fixed before it's stabilized.
Okay, I think I found an actual bug in Cargo: if I run |
Raised at rust-lang/cargo#15695 |
If cargo sbom function is enabled, cargo-auditable will read the SBOM precursor file and use it to generate dependency information rather than trying to use the
cargo metadata
command.Closes #192
Slightly interestingly, cargo didn't include build dependencies for the test fixtures unless I added a build.rs files.