Fix cargo lockfile generation #1233

cd-work · 2023-09-12T17:34:48Z

When generating a lockfile for a Cargo manifest, it is possible that the lockfile will be placed in a parent directory rather than the manifest's directory. Since this wasn't accounted for, lockfile generation would fail and the lockfile would not get cleaned up.

To allow tracking lockfiles generated in directories above the manifest, the find_workspace_root function of cargo is used now to locate the place where the lockfile will be put.

One thing to note is that this probably also applies to other ecosystems, but I wanted to focus on Cargo for now since I'm most confident this will actually work.

When generating a lockfile for a Cargo manifest, it is possible that the lockfile will be placed in a parent directory rather than the manifest's directory. Since this wasn't accounted for, lockfile generation would fail and the lockfile would not get cleaned up. To allow tracking lockfiles generated in directories above the manifest, the `find_workspace_root` function of cargo is used now to locate the place where the lockfile will be put.

phylum-io · 2023-09-12T17:34:58Z

Phylum OSS Supply Chain Risk Analysis - INCOMPLETE

The analysis contains 9 package(s) Phylum has not yet processed,
preventing a complete risk analysis. Phylum is processing these
packages currently and should complete soon.
Please wait for up to 30 minutes, then re-run the analysis.

View this project in the Phylum UI

maxrake

The code was reviewed and appears to fit the bill. However, it was unable to be confirmed with local testing due to a build error:

error: failed to run custom build command for `deno_kv v0.26.0`

Caused by:
  process didn't exit successfully: `/Users/maxrake/dev/phylum/localdev/cli/target/debug/build/deno_kv-0eae3622e5439456/build-script-build` (exit status: 101)
  --- stdout
  cargo:rerun-if-changed=./proto

  --- stderr
  thread 'main' panicked at 'Could not find `protoc` installation and this build crate cannot proceed without
      this knowledge. If `protoc` is installed and this crate had trouble finding
      it, you can set the `PROTOC` environment variable with the specific path to your
      installed `protoc` binary.You could try running `brew install protobuf` or downloading it from https://github.com/protocolbuffers/protobuf/releases

  For more information: https://docs.rs/prost-build/#sourcing-protoc
  ', /Users/maxrake/.cargo/registry/src/index.crates.io-6f17d22bba15001f/prost-build-0.11.9/src/lib.rs:1457:10
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
warning: build failed, waiting for other jobs to finish...

Is updating the deno dependencies required for this PR? If so, and they introduce new build pre-requisites, then the documentation (CONTRIBUTING.md, at least) should be updated to specify those build assumptions.

A CHANGELOG.md entry is needed.

There are some failing tests in CI.

lockfile_generator/src/cargo.rs

cd-work · 2023-09-12T20:50:02Z

Is updating the deno dependencies required for this PR? If so, and they introduce new build pre-requisites, then the documentation (CONTRIBUTING.md, at least) should be updated to specify those build assumptions.

Yes, protoc is required. It's a mess.

phylum-io · 2023-09-12T20:58:41Z

Phylum OSS Supply Chain Risk Analysis - FAILED

This repository analyzes the risk of new dependencies. An
administrator of this repository has set requirements via Phylum policy.

If you see this comment, one or more dependencies have failed Phylum's risk analysis.

Package: `rustfix@0.6.1` failed.

rustfix@0.6.1 may be a typosquatted package

Risk Domain: Malicious Code
Risk Level: high

Reason: risk level cannot exceed medium

View this project in the Phylum UI

phylum-io · 2023-09-12T21:09:07Z

Phylum OSS Supply Chain Risk Analysis - SUCCESS

The Phylum risk analysis is complete and has passed the active policy.

View this project in the Phylum UI

maxrake · 2023-09-12T21:10:57Z

I marked the rustfix package finding as legitimate and not a typosquat. Then, the Phylum GitHub App was run again to produce the latest SUCCESS message and passing check.

maxrake

A CHANGELOG.md entry is needed.

The changes were tested locally and appear to work, with one small exception. A temporarily generated lockfile was not cleaned up after analysis. Here is an example, with a repo that makes use of a Cargo virtual workspace. Before running the code from this PR, there are no lockfiles, only manifests. Running the analysis works, but leaves behind a single Cargo.lock lockfile at the workspace root.

Details (click to expand)

../deep_causality  9  19 on  main [!] via 🦀 v1.72.0
❯ find . -name "Cargo.*"
./deep_causality_macros/Cargo.toml
./Cargo.toml
./ultragraph/Cargo.toml
./deep_causality/Cargo.toml
./deep_causality/examples/smoking/Cargo.toml
./deep_causality/examples/csm/Cargo.toml
./deep_causality/examples/ctx/Cargo.toml
./dcl_data_structures/Cargo.toml

../deep_causality  9  19 on  main [!] via 🦀 v1.72.0
❯ /Users/maxrake/dev/phylum/localdev/cli/target/debug/phylum --version
phylum v5.7.1-4-g0908588-modified

../deep_causality  9  19 on  main [!] via 🦀 v1.72.0
❯ cat .phylum_project
id: 7ab55930-fe6a-4a1e-af6a-9779b5db85fa
name: deep_causality
created_at: 2023-09-12T09:17:32.261275-05:00
group_name: null
lockfiles:
- path: ./deep_causality_macros/Cargo.toml
  type: cargo
- path: ./Cargo.toml
  type: cargo
- path: ./ultragraph/Cargo.toml
  type: cargo
- path: ./deep_causality/Cargo.toml
  type: cargo
- path: ./deep_causality/examples/smoking/Cargo.toml
  type: cargo
- path: ./deep_causality/examples/csm/Cargo.toml
  type: cargo
- path: ./deep_causality/examples/ctx/Cargo.toml
  type: cargo
- path: ./dcl_data_structures/Cargo.toml
  type: cargo

../deep_causality  9  19 on  main [!] via 🦀 v1.72.0
❯ /Users/maxrake/dev/phylum/localdev/cli/target/debug/phylum analyze
Generating lockfile for manifest "deep_causality_macros/Cargo.toml" using Cargo…
✅ Successfully parsed lockfile "deep_causality_macros/Cargo.toml" as type: cargo
Generating lockfile for manifest "Cargo.toml" using Cargo…
✅ Successfully parsed lockfile "Cargo.toml" as type: cargo
Generating lockfile for manifest "ultragraph/Cargo.toml" using Cargo…
✅ Successfully parsed lockfile "ultragraph/Cargo.toml" as type: cargo
Generating lockfile for manifest "deep_causality/Cargo.toml" using Cargo…
✅ Successfully parsed lockfile "deep_causality/Cargo.toml" as type: cargo
Generating lockfile for manifest "deep_causality/examples/smoking/Cargo.toml" using Cargo…
✅ Successfully parsed lockfile "deep_causality/examples/smoking/Cargo.toml" as type: cargo
Generating lockfile for manifest "deep_causality/examples/csm/Cargo.toml" using Cargo…
✅ Successfully parsed lockfile "deep_causality/examples/csm/Cargo.toml" as type: cargo
Generating lockfile for manifest "deep_causality/examples/ctx/Cargo.toml" using Cargo…
✅ Successfully parsed lockfile "deep_causality/examples/ctx/Cargo.toml" as type: cargo
Generating lockfile for manifest "dcl_data_structures/Cargo.toml" using Cargo…
✅ Successfully parsed lockfile "dcl_data_structures/Cargo.toml" as type: cargo
✅ Job ID: 485800c4-63cd-4185-8c0d-c68e2b972109

Phylum Supply Chain Risk Analysis — SUCCESS

You can find the interactive report here:
  https://app.phylum.io/projects/7ab55930-fe6a-4a1e-af6a-9779b5db85fa?label=uncategorized

../deep_causality  9  20 on  main [!] via 🦀 v1.72.0 took 11s
❯ find . -name "Cargo.*"
./deep_causality_macros/Cargo.toml
./Cargo.toml
./ultragraph/Cargo.toml
./Cargo.lock
./deep_causality/Cargo.toml
./deep_causality/examples/smoking/Cargo.toml
./deep_causality/examples/csm/Cargo.toml
./deep_causality/examples/ctx/Cargo.toml
./dcl_data_structures/Cargo.toml

It appears the cargo metadata command will create a Cargo.lock lockfile when there isn't already one. Maybe that is where the "extra" lockfile came from.

maxrake

The changes were tested locally.

LGTM

cd-work requested a review from a team as a code owner September 12, 2023 17:34

cd-work requested a review from kylewillmon September 12, 2023 17:34

maxrake assigned cd-work Sep 12, 2023

maxrake requested changes Sep 12, 2023

View reviewed changes

lockfile_generator/src/cargo.rs Outdated Show resolved Hide resolved

Fix root detection without workspace

8acf975

Switch to cargo command

0908588

cd-work requested a review from maxrake September 14, 2023 21:21

maxrake requested changes Sep 15, 2023

View reviewed changes

cd-work added 2 commits September 15, 2023 16:56

Add changelog entry

758422d

Fix lockfile deletion

743fc8e

cd-work requested a review from maxrake September 15, 2023 15:44

maxrake approved these changes Sep 15, 2023

View reviewed changes

cd-work merged commit 42c9dfa into main Sep 15, 2023

cd-work deleted the fix_cargo_manifest_analysis branch September 15, 2023 20:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix cargo lockfile generation #1233

Fix cargo lockfile generation #1233

cd-work commented Sep 12, 2023

phylum-io bot commented Sep 12, 2023

maxrake left a comment

cd-work commented Sep 12, 2023

phylum-io bot commented Sep 12, 2023

phylum-io bot commented Sep 12, 2023

maxrake commented Sep 12, 2023

maxrake left a comment •

edited

Loading

maxrake left a comment

Fix cargo lockfile generation #1233

Fix cargo lockfile generation #1233

Conversation

cd-work commented Sep 12, 2023

phylum-io bot commented Sep 12, 2023

Phylum OSS Supply Chain Risk Analysis - INCOMPLETE

maxrake left a comment

Choose a reason for hiding this comment

cd-work commented Sep 12, 2023

phylum-io bot commented Sep 12, 2023

Phylum OSS Supply Chain Risk Analysis - FAILED

Package: rustfix@0.6.1 failed.

rustfix@0.6.1 may be a typosquatted package

phylum-io bot commented Sep 12, 2023

Phylum OSS Supply Chain Risk Analysis - SUCCESS

maxrake commented Sep 12, 2023

maxrake left a comment • edited Loading

Choose a reason for hiding this comment

maxrake left a comment

Choose a reason for hiding this comment

Package: `rustfix@0.6.1` failed.

maxrake left a comment •

edited

Loading