Merge pull request #431 from cbgbt/canonicalize

twoliter: disallow std::path::Path::canonicalize

Author: cbgbt

Cargo.lock

@@ -54,6 +54,7 @@ buildsys-config = { version = "0.1", path = "tools/buildsys-config" }
 krane-static = { version = "0.1", path = "tools/krane" }
 oci-cli-wrapper = { version = "0.1", path = "tools/oci-cli-wrapper" }
 parse-datetime = { version = "0.1", path = "tools/parse-datetime" }
+path-absolutize = "3.1"
 pipesys = { version = "0.1", path = "tools/pipesys", lib = true, artifact = [ "bin:pipesys" ] }
 pubsys = { version = "0.1", path = "tools/pubsys", artifact = [ "bin:pubsys" ] }
 pubsys-config = { version = "0.1", path = "tools/pubsys-config" }
@@ -147,3 +148,6 @@ which = "6"
 [profile.dist]
 inherits = "release"
 lto = "thin"
+
+[workspace.lints.clippy]
+disallowed-methods = "deny"

Cargo.toml

@@ -0,0 +1,12 @@
+[[disallowed-methods]]
+path = "std::path::Path::canonicalize"
+reason = "Twoliter usually does not want to resolve symlinks. Use path_absolutize instead"
+
+[[disallowed-methods]]
+path = "std::fs::canonicalize"
+reason = "Twoliter usually does not want to resolve symlinks. Use path_absolutize instead"
+
+[[disallowed-methods]]
+path = "tokio::fs::canonicalize"
+reason = "Twoliter usually does not want to resolve symlinks. Use path_absolutize instead"
+

clippy.toml

@@ -5,6 +5,7 @@ use std::path::PathBuf;
 use std::process::Command;
 use tempfile::TempDir;
 
+mod twoliter_build;
 mod twoliter_update;
 
 pub const TWOLITER_PATH: &'static str = env!("CARGO_BIN_FILE_TWOLITER");

tests/integration-tests/src/lib.rs

@@ -0,0 +1,75 @@
+use super::{run_command, test_projects_dir, TWOLITER_PATH};
+use std::path::Path;
+use tempfile::TempDir;
+
+#[test]
+#[ignore]
+fn test_workspace_symlinks_not_followed() {
+    // Ensure a symlinked `Twoliter.toml` does not trick us into putting our build directory into
+    // the symlink target's parent directory.
+
+    let target_kit = test_projects_dir().join("local-kit");
+    let work_dir = copy_project_to_temp_dir(&target_kit);
+
+    let work_dir_path = work_dir.path();
+
+    let working_twoliter_toml = work_dir_path.join("Twoliter.toml");
+
+    // Replace Twoliter.toml with a symlink
+    std::fs::remove_file(&working_twoliter_toml).unwrap();
+    std::os::unix::fs::symlink(
+        target_kit.join("Twoliter.toml"),
+        work_dir_path.join("Twoliter.toml"),
+    )
+    .unwrap();
+
+    assert!(!work_dir_path.join("build").is_dir());
+
+    run_command(
+        TWOLITER_PATH,
+        [
+            "update",
+            "--project-path",
+            working_twoliter_toml.to_str().unwrap(),
+        ],
+        [],
+    );
+    run_command(
+        TWOLITER_PATH,
+        [
+            "fetch",
+            "--project-path",
+            working_twoliter_toml.to_str().unwrap(),
+        ],
+        [],
+    );
+
+    assert!(work_dir_path.join("build").is_dir());
+}
+
+pub(crate) fn copy_project_to_temp_dir(project: impl AsRef<Path>) -> TempDir {
+    let temp_dir = TempDir::new().unwrap();
+    copy_most_dirs_recursively(project, &temp_dir);
+    temp_dir
+}
+
+/// Copy dirs recursively except for some of the larger "ignoreable" dirs that may exist in the
+/// user's checkout.
+fn copy_most_dirs_recursively(src: impl AsRef<Path>, dst: impl AsRef<Path>) {
+    let src = src.as_ref();
+    let dst = dst.as_ref();
+    for entry in std::fs::read_dir(src).unwrap() {
+        std::fs::create_dir_all(&dst).unwrap();
+        let entry = entry.unwrap();
+        let file_type = entry.file_type().unwrap();
+        if file_type.is_dir() {
+            let name = entry.file_name().to_str().unwrap().to_string();
+            if matches!(name.as_ref(), "target" | "build" | ".gomodcache" | ".cargo") {
+                continue;
+            }
+            copy_most_dirs_recursively(&entry.path(), &dst.join(entry.file_name()));
+        } else {
+            std::fs::copy(entry.path(), dst.join(entry.file_name())).unwrap();
+        }
+    }
+}

tests/integration-tests/src/twoliter_build.rs

@@ -27,6 +27,7 @@ lazy_static.workspace = true
 log.workspace = true
 oci-cli-wrapper.workspace = true
 olpc-cjson.workspace = true
+path-absolutize.workspace = true
 semver = { workspace = true, features = ["serde"] }
 serde = { workspace = true, features = ["derive"] }
 serde_json.workspace = true
@@ -61,3 +62,6 @@ test-case.workspace = true
 default = ["integ-tests", "pubsys"]
 integ-tests = []
 pubsys = ["dep:pubsys"]
+
+[lints]
+workspace = true

twoliter/Cargo.toml

@@ -72,18 +72,24 @@ pub(crate) async fn exec(cmd: &mut Command, quiet: bool) -> Result<Option<String
 #[allow(dead_code)]
 pub(crate) mod fs {
     use anyhow::{Context, Result};
+    use path_absolutize::Absolutize;
     use std::fs::Metadata;
     use std::io::ErrorKind;
     use std::path::{Path, PathBuf};
     use tokio::fs;
     use tracing::instrument;
 
+    /// Canonicalizes the input path based on `cwd` without resolving symlinks or accessing the
+    /// filesystem.
     #[instrument(level = "trace", skip(path), fields(path = %path.as_ref().display()))]
     pub(crate) async fn canonicalize(path: impl AsRef<Path>) -> Result<PathBuf> {
-        fs::canonicalize(path.as_ref()).await.context(format!(
-            "Unable to canonicalize '{}'",
-            path.as_ref().display()
-        ))
+        path.as_ref()
+            .absolutize()
+            .context(format!(
+                "Unable to canonicalize '{}'",
+                path.as_ref().display()
+            ))
+            .map(|p| p.to_path_buf())
     }
 
     #[instrument(

twoliter/src/common.rs

@@ -7,6 +7,7 @@ pub(crate) use self::image::{Image, ProjectImage, ValidIdentifier, VendedArtifac
 pub(crate) use self::vendor::ArtifactVendor;
 use lock::LockedImage;
 pub(crate) use lock::VerificationTagger;
+use path_absolutize::Absolutize;
 
 use self::lock::{Lock, LockedSDK, Override};
 use crate::common::fs::{self, read_to_string};
@@ -108,7 +109,7 @@ impl Project<Unlocked> {
             dir.display()
         );
         let dir = dir
-            .canonicalize()
+            .absolutize()
             .context(format!("Unable to canonicalize '{}'", dir.display()))?;
         let filepath = dir.join("Twoliter.toml");
         if filepath.is_file() {