Merge pull request #130 from rust-secure-code/cyclic-dep-fix

Fix `cargo auditable` sometimes encoding a cyclic dependency graph

auditable-serde/Cargo.toml

@@ -29,3 +29,7 @@ schemars = {version = "0.8.10", optional = true }
 [[example]]
 name = "json-to-toml"
 required-features = ["toml"]
+
+[[example]]
+name = "from-metadata"
+required-features = ["from_metadata"]

auditable-serde/src/lib.rs

@@ -388,10 +388,15 @@ impl TryFrom<&cargo_metadata::Metadata> for VersionInfo {
                 // dev-dependencies are not included
                 let package: &mut Package = &mut packages[id_to_index[package_id]];
                 // Dependencies
-                for dep in node.dependencies.iter() {
-                    // omit package if it is a development-only dependency
-                    let dep_id = dep.repr.as_str();
-                    if id_to_dep_kind[dep_id] != PrivateDepKind::Development {
+                for dep in node.deps.iter() {
+                    // Omit the graph edge if this is a development dependency
+                    // to fix https://github.com/rustsec/rustsec/issues/1043
+                    // It is possible that something that we depend on normally
+                    // is also a dev-dependency for something,
+                    // and dev-dependencies are allowed to have cycles,
+                    // so we may end up encoding cyclic graph if we don't handle that.
+                    let dep_id = dep.pkg.repr.as_str();
+                    if strongest_dep_kind(&dep.dep_kinds) != PrivateDepKind::Development {
                         package.dependencies.push(id_to_index[dep_id]);
                     }
                 }
@@ -490,23 +495,36 @@ mod tests {
     #![allow(unused_imports)] // otherwise conditional compilation emits warnings
     use super::*;
     use std::fs;
-    use std::{convert::TryInto, path::PathBuf};
+    use std::{
+        convert::TryInto,
+        path::{Path, PathBuf},
+    };
 
-    #[cfg(feature = "toml")]
     #[cfg(feature = "from_metadata")]
-    fn load_own_metadata() -> cargo_metadata::Metadata {
+    fn load_metadata(cargo_toml_path: &Path) -> cargo_metadata::Metadata {
         let mut cmd = cargo_metadata::MetadataCommand::new();
-        let cargo_toml_path =
-            PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap()).join("Cargo.toml");
         cmd.manifest_path(cargo_toml_path);
         cmd.exec().unwrap()
     }
 
+    #[test]
+    #[cfg(feature = "from_metadata")]
+    fn dependency_cycle() {
+        let cargo_toml_path = PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap())
+            .join("tests/fixtures/cargo-audit-dep-cycle/Cargo.toml");
+        let metadata = load_metadata(&cargo_toml_path);
+        let version_info_struct: VersionInfo = (&metadata).try_into().unwrap();
+        let json = serde_json::to_string(&version_info_struct).unwrap();
+        VersionInfo::from_str(&json).unwrap(); // <- the part we care about succeeding
+    }
+
     #[test]
     #[cfg(feature = "toml")]
     #[cfg(feature = "from_metadata")]
     fn to_toml() {
-        let metadata = load_own_metadata();
+        let cargo_toml_path =
+            PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap()).join("Cargo.toml");
+        let metadata = load_metadata(&cargo_toml_path);
         let version_info_struct: VersionInfo = (&metadata).try_into().unwrap();
         let _lockfile_struct: cargo_lock::Lockfile = (&version_info_struct).try_into().unwrap();
     }

auditable-serde/tests/fixtures/cargo-audit-dep-cycle/.gitignore

@@ -0,0 +1 @@
+/target

auditable-serde/tests/fixtures/cargo-audit-dep-cycle/Cargo.toml

@@ -0,0 +1,12 @@
+[workspace]
+members = ["a", "b"]
+
+[package]
+name = "cargo-audit-dep-cycle"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+a = { path = "a" }

auditable-serde/tests/fixtures/cargo-audit-dep-cycle/README.md

@@ -0,0 +1,11 @@
+# Minimal example for cyclic dependency graph in audit data
+
+When building this project with `cargo auditable build`, and then running `cargo audit` on it, this error is printed:
+
+```
+error: parse error: Failed to deserialize audit data from JSON: The input JSON specifies a cyclic dependency graph
+```
+
+This repository serves as a minimal example for reproducing the issue.
+
+The issue was reported [here](https://github.com/rustsec/rustsec/issues/1043).

auditable-serde/tests/fixtures/cargo-audit-dep-cycle/a/Cargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "a"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+b = { path = "../b" }

auditable-serde/tests/fixtures/cargo-audit-dep-cycle/a/src/lib.rs

@@ -0,0 +1,14 @@
+pub fn add(left: usize, right: usize) -> usize {
+    left + right
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn it_works() {
+        let result = add(2, 2);
+        assert_eq!(result, 4);
+    }
+}

auditable-serde/tests/fixtures/cargo-audit-dep-cycle/b/Cargo.toml

@@ -0,0 +1,11 @@
+[package]
+name = "b"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+
+[dev-dependencies]
+a = { path = "../a" }

auditable-serde/tests/fixtures/cargo-audit-dep-cycle/b/src/lib.rs

@@ -0,0 +1,14 @@
+pub fn add(left: usize, right: usize) -> usize {
+    left + right
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn it_works() {
+        let result = add(2, 2);
+        assert_eq!(result, 4);
+    }
+}

auditable-serde/tests/fixtures/cargo-audit-dep-cycle/src/main.rs

@@ -0,0 +1,3 @@
+fn main() {
+    println!("Hello, world!");
+}