cargo vendor --versioned-dirs doesn't notice that local vendored content has diverged from crate registry

[jhfrontz]

Problem

I had been hoping that I could use cargo vendor to quickly identify any unofficial/local changes to a crate.

To test this, I made an innocuous change to a local copy of a crate -- I was expecting when I did cargo vendor that it would either point out the difference (or even just silently overwrite it). However, it doesn't seem to do either.

The workaround seems to be to delete the entire vendor directory and then compare the result to what had been checked in (a la git diff --exit-code) -- after ignoring any semi-ephemeral content (e.g., git add $(find . -type f -name Cargo.lock -o -name .cargo-checksum.json))

Steps

No response

Possible Solution(s)

It's not clear to me whether this is the intended behavior for cargo vendor or not -- but if it is, there should be a major note on the man page about cargo vendor retaining local changes.

Notes

No response

Version

$ cargo version --verbose
cargo 1.65.0
release: 1.65.0
host: x86_64-unknown-linux-gnu
libgit2: 1.5.0 (sys:0.15.0 vendored)
libcurl: 7.29.0 (sys:0.4.55+curl-7.83.1 system ssl:NSS/3.53.1)
os: CentOS 7.9.2009 (Core) [64-bit]

[weihanglo]

Hmm… cargo vendor does check the integrity of vendored packages. It also overwrites files if you modify them manually. Can you provide steps to help the reproduction?

BTW, to make vendoring take effect, you need to put some configs under .cargo/config.toml first, like

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

[source.vendored-sources]
directory = "vendor"

[weihanglo]

This is a reproducer that verifies Cargo is aware of vendored source being modified:

cargo new cargo-issue-11897
pushd cargo-issue-11897
cargo add empty-library
mkdir -p .cargo
cargo vendor > .cargo/config.toml
echo "pub fn f() {}" > vendor/empty-library/src/lib.rs
cargo c

You'll get an error like

error: the listed checksum of `/projects/cargo-issue-11897/vendor/empty-library/src/lib.rs` has changed:
expected: b81956f17c56fffd4b5e81f93eb647dd8ce78a89bccec826b5525368f172baba
actual:   16c2fca9e371936458d01576b4ca311c22d166a45539ccbc9104823d0b10db47

The things you bumped into might be #9455, which registry sources are not readonly. I believe this is not cargo-vendor's fault — cargo-vendor always re-unpack from tarballs when vendoring. See #12509.

I am going to close due to lack of a minimal set of steps to reproduce, and it doesn't look like an issue of cargo-vendor. Here are tips about how to create a minimal, complete, and verifiable example. Let us know if there is any update on your side.

[apoelstra]

Hi @weihanglo. Thanks for the minimal example! Here is a tweaked version which shows the bug we intended to report:

Here is an example:

cargo new cargo-issue-11897
pushd cargo-issue-11897
cargo add empty-library
mkdir -p .cargo
cargo vendor --versioned-dirs > .cargo/config.toml
echo "pub fn f() {}" > vendor/empty-library-1.0.0/src/lib.rs
sed -i 's/b81956f17c56fffd4b5e81f93eb647dd8ce78a89bccec826b5525368f172baba/16c2fca9e371936458d01576b4ca311c22d166a45539ccbc9104823d0b10db47/' vendor/empty-library-1.0.0/.cargo-checksum.json
cargo c # this passes, unsurprisingly

So far so good -- I don't think anybody expects cargo to be able to locally detect tampering when you've updated the checksum file. Within a CI setup that has no Internet access this would be literaly impossible (absent some sort of WoT rooted inside of cargo itself I guess.)

This bug report is about the following behavior though:

# ...execute same commands as above...
cargo vendor --versioned-dirs > .cargo/config.toml
cargo c # still passes...
cat vendor/empty-library-1.0.0/src/lib.rs #...even though the file is still compromised

If you do exactly the same steps but without --versioned-dirs (and without the -1.0.0 in the paths) then we see the expected behavior, which is that the compromised lib.rs file gets overwritten by re-running cargo vendor.

[weihanglo]

Nice catch, and thanks for the repro!

The quickest fix is removing these lines. Cargo assumes that vendor sources never changes, but you never know what users would do.

diff --git a/src/cargo/ops/vendor.rs b/src/cargo/ops/vendor.rs
index 3ee46db32..a9d32dc02 100644
--- a/src/cargo/ops/vendor.rs
+++ b/src/cargo/ops/vendor.rs
@@ -212,10 +212,6 @@ fn sync(
         let dst = canonical_destination.join(&dst_name);
         to_remove.remove(&dst);
         let cksum = dst.join(".cargo-checksum.json");
-        if dir_has_version_suffix && cksum.exists() {
-            // Always re-copy directory without version suffix in case the version changed
-            continue;
-        }
 
         config.shell().status(
             "Vendoring",

Given people shouldn't modify the content in the first place, I am not sure if this is a good way to resolve this. Maybe, when cargo vendors stuff, it should mark all files as read-only. Though it will lead us to #9455 I guess.