cargo-auditable icon indicating copy to clipboard operation
cargo-auditable copied to clipboard

Use cargo SBOM precursor files, if available

Open tofay opened this issue 6 months ago • 5 comments

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.

tofay avatar Jun 19 '25 17:06 tofay

Thanks a lot! I'll take a closer look in the next few days.

Slightly interestingly, cargo didn't include build dependencies for the test fixtures unless I added a build.rs files.

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?

Shnatsel avatar Jun 19 '25 17:06 Shnatsel

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.

tofay avatar Jun 19 '25 17:06 tofay

Ah, you are probably right! I am a little rusty on the finer points of Cargo dependencies.

Shnatsel avatar Jun 19 '25 18:06 Shnatsel

Okay, I think I found an actual bug in Cargo: if I run cargo +nightly build -Z sbom --release and then CARGO_BUILD_SBOM=true cargo +nightly build -Z sbom --release, the SBOM file will not be actually generated. It seems that whether the SBOM should be built or not is not part of the check for whether a rebuild is necessary or not.

Shnatsel avatar Jun 19 '25 19:06 Shnatsel

Okay, I think I found an actual bug in Cargo: if I run cargo +nightly build -Z sbom --release and then CARGO_BUILD_SBOM=true cargo +nightly build -Z sbom --release, the SBOM file will not be actually generated. It seems that whether the SBOM should be built or not is not part of the check for whether a rebuild is necessary or not.

Raised at https://github.com/rust-lang/cargo/issues/15695

tofay avatar Jun 22 '25 22:06 tofay

~~I've cross-referenced the output of this PR against the output of #210, and the generated audit data matches exactly, with bit-identical outputs. This gives me a lot more confidence in the correctness of both.~~

UPDATE: I messed up and tested the cargo-tree version instead of this one

I've tried it on these 26 binary crates that produce 35 binaries:

cavif gitoxide cargo-auditable cargo-audit cargo-show-asm cargo-cyclonedx ripgrep fd-find duct cargo-sort cargo-hack cargo-deny lsd procs httm t-rec tealdeer feluda lychee petname sarif-fmt rona zizmor aipack oxker cargo-outdated

20 of them have a different output between the current stable release and either of the precise dependency list branches, so it's great to see them making a difference.

Shnatsel avatar Jun 24 '25 17:06 Shnatsel

I've realized that I've messed up the real-world testing earlier and accidentally compared the cargo-tree based branch against itself instead of this one.

I see surface-level divergences in all produced SBOMs when actually compare against this branch. It'll take me a bit to write a proper structural comparator and check for any divergences in actual content. I'd like to do that before I merge this.

Shnatsel avatar Jun 24 '25 21:06 Shnatsel

I looked into the differences and they all appear to be fixes.

First, when running cargo install, the resulting binary is treated as coming from crates.io rather than the local filesystem. That's an improvement.

Second, on cargo-auditable itself e.g. the crate unicode-ident is now correctly treated as a build-dependency, whereas the audit data collection based on cargo metadata incorrectly includes it as a runtime dependency. Not sure if that's a limitation of cargo metadata or a bug in dependency kind propagation in cargo auditable; could be either.

Shnatsel avatar Jun 25 '25 14:06 Shnatsel

Yeah, cargo metadata reports proc-macro dependencies as runtime dependencies instead of build-only ones. Not much we can do about that. Maybe some really cursed cargo tree parsing could help, but I'm not sure I'm willing to ship something so potentially brittle.

Shnatsel avatar Jun 25 '25 15:06 Shnatsel

Merged. Thank you!

We still need to get https://github.com/rust-lang/cargo/issues/15695 fixed before this becomes actually usable.

And ideally also get https://github.com/rust-lang/cargo/commit/bde57ceb46de4851fc4f0ef64f413465435bdc35 merged so that we could be confident it doesn't break in the future.

In the meantime I'll try to complete https://github.com/rust-secure-code/cargo-auditable/pull/210 and add a revision field to the JSON we embed to distinguish dependency lists created this way from the older algorithm, so that whatever consumes this data could tell how reliable it is.

Shnatsel avatar Jun 25 '25 15:06 Shnatsel