rust-secure-code / cargo-auditable

Make production Rust binaries auditable
Apache License 2.0
646 stars 28 forks source link

Include commit hashes for git dependencies #122

Open Shnatsel opened 1 year ago

Shnatsel commented 1 year ago

Add a new commit field to track what the exact commit on which the dependency was built. It should only be included when SourceKind is git.

The final entry would look like this:

commit: "311f9932128667b8b18113becdea276b3d98aace"

Shnatsel commented 1 year ago

@figsoda would this cause any issues for Nix, e.g. blow up the binary size? I understand Nix uses Git a lot in its workflows. When the package is built, does it refer to other packages locally (by path), to a git repo (i.e. git = https://example.com in Cargo.lock, or just pull them from crates.io as usual?

@jcgruenhage same question for Void Linux

bjorn3 commented 1 year ago

Another option would be to include the tree hash instead of commit hash. This is invariant to commit message rewords and squash merges when the original commit was already based on the latest commit of the target branch. In addition it would be possible to record the tree hash for just the subdirectory of the respective crate, thus increasing the likelihood of being able to go back to the source of this specific crate when eg force pushing a change to a different crate in a mono repo.

Shnatsel commented 1 year ago

That's a good point, but on the other hand I don't really want to diverge from the hash recorded in Cargo.lock and reported by cargo metadata - that seems like it would be a source of confusion.

figsoda commented 1 year ago

@figsoda would this cause any issues for Nix, e.g. blow up the binary size? I understand Nix uses Git a lot in its workflows. When the package is built, does it refer to other packages locally (by path), to a git repo (i.e. git = https://example.com in Cargo.lock, or just pull them from crates.io as usual?

It shouldn't cause any issue afaict, the sources are vendored from crates.io in a separate derivation (package) and replaces crates.io in the actual build with this cargo config. The Cargo.lock is unmodified

[source.crates-io]
replace-with = "vendored-sources"

[source.vendored-sources]
directory = "..."
jcgruenhage commented 1 year ago

Shouldn't cause any trouble for Void, no. We're sourcing software from tarballs as published either by the upstream on their dev platform (such as github/gitlab/sourcehut/etc), or if that's not available from crates.io. Outside of patching dependencies in some exceptions, we're also not doing any source replacements or changing the dependencies described in the Cargo.toml, so should be perfectly fine for us :)

eminence commented 1 year ago

Hi all :wave:

Any rough ETA when this feature might be released?

Shnatsel commented 1 year ago

@eminence life happened, and I am currently occupied with other things. If you have use cases for this feature I could bump up the priority of cutting a release.

I'd appreciate if you could tell me what your use case is. I need to understand the needs of the users so that I don't accidentally break the use case in the future.

eminence commented 1 year ago

At work, I have binaries that depend on several internal git dependencies (we don't have an internal crate repo to use). The version number stored in the embedded manifest is not sufficient to exactly identify the version of the git dependency. So I'm interested in having the hash of the git dependency.

I thought that the implementation in #123 would solve this. However, I just did a test and I'm not actually seeing the git hashes (when extracted via the rust-audit-info tool). Here's what I'm seeing:

        {
            "name": "terminal_size",
            "version": "0.2.6",
            "source": "git",
            "dependencies": [
                140
            ]
        },

For completeness, here is how I tested:

Shnatsel commented 1 year ago

@Adhalianna I'd appreciate if you could take a look if you're available. If not, that's fine too.