search

rust-bitcoin / rust-bip39

A Rust library for working with Bitcoin BIP-39 mnemonics
Creative Commons Zero v1.0 Universal
95 stars 59 forks source link

Missing Cargo.lock #61

Open steinerkelvin opened 11 months ago

steinerkelvin commented 11 months ago

Is there a specific reason for this repo to not have a Cargo.lock?

I need to build this package with Nix and the Cargo.lock is required. It wouldn't make sense to generate this during build, in this case.

tcharding commented 11 months ago

Rust library crates do not typically commit their lock files. I went looking for a citation for you and stumbled across this

https://blog.rust-lang.org/2023/08/29/committing-lockfiles.html

steinerkelvin commented 11 months ago

Hum, I didn't know that. TIL.

I solved my problem another way (it wasn't even related to this package lol), so I don't have a demand for it anymore but I guess it could be interesting to add the Cargo.lock with the corresponding adjustments on CI.

axelpale commented 9 months ago

Wouldn't the missing Cargo.lock make the package susceptible to an attack down in the dependency tree? For example, let's assume zeroize, a dependency of rust-bip39, becomes tampered in a patch version increment. If rust-bip39 does not lock the zeroize version, then new builds that depend on rust-bip39 will install the version that includes the malicious patch.

Of course we cannot know for sure if the patch actually solves a threat instead of introducing a new one, but with a lock file the maintainers are at least let to do their due diligence before upgrading to the patch. Without a lock file things can turn bad without anyone's notice.

To quote the citation above: "there isn't a universal answer to these situations, we felt it was best to leave the choice to developers"

Therefore having a lock file feels more safe. We are talking about a library that deals with bitcoin wallet private keys, after all.

I am new to Rust with extensive JS library background, so my knowledge might fail on how version ranges in Cargo.toml behave. Nevertheless the threat described above is very real in JavaScript domain where lock files are strongly recommended.

tcharding commented 9 months ago

That would imply that the rust-bip39 maintainers reviewed and signed off on every line of code in every dependency. I don't believe they have nor is it reasonable to expect they will (except if non of the optional dependencies are used since that leaves only bitcoin_hashes which is loosely from the same people). It would be awesome if we all did these kind of reviews, and we over in rust-bitcoin have tried to push cargo crev for just that, but it doesn't seem to be getting much traction.