feat: enforce all Paths to be valid Unicode (#326)

* feat: enforce all Paths to be valid Unicode

This commit introduces the `camino` dependency across the codebase,
ensuring that all Paths are valid UTF-8, making them much easier
to work with, and be confident that invalid UTF-8 will not muck
up our plans.

* fixup windows

Cargo.toml

@@ -19,6 +19,7 @@ timber = { path = "./crates/timber" }
 anyhow = "1.0.38"
 atty = "0.2.14"
 ansi_term = "0.12.1"
+camino = "1.0.2"
 billboard = { git = "https://github.com/EverlastingBugstopper/billboard.git", branch = "main" }
 chrono = "0.4"
 console = "0.14.0"
@@ -52,4 +53,5 @@ members = [".", "crates/*", "installers/binstall"]
 
 [build-dependencies]
 anyhow = "1"
+camino = "1.0"
 which = "4.0.2"

build.rs

@@ -1,11 +1,13 @@
-use anyhow::{anyhow, Context, Result};
+use anyhow::{anyhow, Context, Error, Result};
 use std::{
     env, fs,
-    path::Path,
+    path::PathBuf,
     process::{Command, Output},
     str,
 };
 
+use camino::{Utf8Path, Utf8PathBuf};
+
 /// files to copy from the repo's root directory into the npm tarball
 const FILES_TO_COPY: &[&str; 2] = &["LICENSE", "README.md"];
 
@@ -39,20 +41,26 @@ fn cargo_warn(message: &str) {
 /// these steps are only _required_ when running in release mode
 fn prep_npm(is_release_build: bool) -> Result<()> {
     let npm_install_path = match which::which("npm") {
-        Ok(install_path) => Some(install_path),
+        Ok(install_path) => {
+            Some(Utf8PathBuf::from_path_buf(install_path).map_err(|pb| invalid_path_buf(&pb))?)
+        }
         Err(_) => None,
     };
 
     // we have to work with absolute paths like this because of windows :P
-    let current_dir = env::current_dir().context("Could not find the current directory.")?;
+    let current_dir = Utf8PathBuf::from_path_buf(
+        env::current_dir().context("Could not find the current directory.")?,
+    )
+    .map_err(|pb| invalid_path_buf(&pb))?;
+
     let npm_dir = current_dir.join("installers").join("npm");
 
     let is_npm_installed = npm_install_path.is_some();
 
     if !npm_dir.exists() {
         return Err(anyhow!(
             "The npm package does not seem to be located here:\n{}",
-            npm_dir.display()
+            &npm_dir
         ));
     }
 
@@ -86,6 +94,10 @@ fn prep_npm(is_release_build: bool) -> Result<()> {
     Ok(())
 }
 
+fn invalid_path_buf(pb: &PathBuf) -> Error {
+    anyhow!("Current directory \"{}\" is not valid UTF-8", pb.display())
+}
+
 fn process_command_output(output: &Output) -> Result<()> {
     if !output.status.success() {
         let stdout =
@@ -98,7 +110,7 @@ fn process_command_output(output: &Output) -> Result<()> {
     Ok(())
 }
 
-fn update_dependency_tree(npm_install_path: &Path, npm_dir: &Path) -> Result<()> {
+fn update_dependency_tree(npm_install_path: &Utf8Path, npm_dir: &Utf8Path) -> Result<()> {
     let command_output = Command::new(npm_install_path)
         .current_dir(npm_dir)
         .arg("update")
@@ -110,7 +122,7 @@ fn update_dependency_tree(npm_install_path: &Path, npm_dir: &Path) -> Result<()>
     Ok(())
 }
 
-fn install_dependencies(npm_install_path: &Path, npm_dir: &Path) -> Result<()> {
+fn install_dependencies(npm_install_path: &Utf8Path, npm_dir: &Utf8Path) -> Result<()> {
     let command_output = Command::new(npm_install_path)
         .current_dir(npm_dir)
         .arg("install")
@@ -120,7 +132,7 @@ fn install_dependencies(npm_install_path: &Path, npm_dir: &Path) -> Result<()> {
     process_command_output(&command_output).context("Could not print output of 'npm install'.")
 }
 
-fn update_version(npm_install_path: &Path, npm_dir: &Path) -> Result<()> {
+fn update_version(npm_install_path: &Utf8Path, npm_dir: &Utf8Path) -> Result<()> {
     let command_output = Command::new(npm_install_path)
         .current_dir(npm_dir)
         .arg("version")
@@ -138,7 +150,7 @@ fn update_version(npm_install_path: &Path, npm_dir: &Path) -> Result<()> {
         .with_context(|| format!("Could not print output of 'npm version {}'.", PKG_VERSION))
 }
 
-fn dry_run_publish(npm_install_path: &Path, npm_dir: &Path) -> Result<()> {
+fn dry_run_publish(npm_install_path: &Utf8Path, npm_dir: &Utf8Path) -> Result<()> {
     let command_output = Command::new(npm_install_path)
         .current_dir(npm_dir)
         .arg("publish")
@@ -150,7 +162,11 @@ fn dry_run_publish(npm_install_path: &Path, npm_dir: &Path) -> Result<()> {
         .context("Could not print output of 'npm publish --dry-run'.")
 }
 
-fn copy_files_to_npm_package(files: &[&str], current_dir: &Path, npm_dir: &Path) -> Result<()> {
+fn copy_files_to_npm_package(
+    files: &[&str],
+    current_dir: &Utf8Path,
+    npm_dir: &Utf8Path,
+) -> Result<()> {
     for file in files {
         let context = format!("Could not copy {} to npm package.", &file);
         fs::copy(current_dir.join(file), npm_dir.join(file)).context(context)?;

crates/houston/Cargo.toml

@@ -5,6 +5,7 @@ authors = ["Apollo Developers <[email protected]>"]
 edition = "2018"
 
 [dependencies]
+camino = "1.0"
 directories-next = "2.0"
 serde = { version = "1.0", features = ["derive"] }
 thiserror = "1.0"

crates/houston/src/config.rs

@@ -2,8 +2,8 @@ use directories_next::ProjectDirs;
 
 use crate::HoustonProblem;
 
+use camino::{Utf8Path, Utf8PathBuf};
 use std::fs;
-use std::path::{Path, PathBuf};
 
 /// Config allows end users to override default settings
 /// usually determined by Houston. They are intended to
@@ -12,7 +12,7 @@ use std::path::{Path, PathBuf};
 #[derive(Debug, Clone)]
 pub struct Config {
     /// home is the path to the user's global config directory
-    pub home: PathBuf,
+    pub home: Utf8PathBuf,
 
     /// override_api_key is used for overriding the API key returned
     /// when loading a profile
@@ -22,15 +22,15 @@ pub struct Config {
 impl Config {
     /// Creates a new instance of `Config`
     pub fn new(
-        override_home: Option<&impl AsRef<Path>>,
+        override_home: Option<&impl AsRef<Utf8Path>>,
         override_api_key: Option<String>,
     ) -> Result<Config, HoustonProblem> {
         let home = match override_home {
             Some(home) => {
-                let home_path = PathBuf::from(home.as_ref());
+                let home_path = Utf8PathBuf::from(home.as_ref());
                 if home_path.exists() && !home_path.is_dir() {
                     Err(HoustonProblem::InvalidOverrideConfigDir(
-                        home_path.display().to_string(),
+                        home_path.to_string(),
                     ))
                 } else {
                     Ok(home_path)
@@ -40,17 +40,22 @@ impl Config {
                 // Lin: /home/alice/.config/rover
                 // Win: C:\Users\Alice\AppData\Roaming\Apollo\Rover\config
                 // Mac: /Users/Alice/Library/Application Support/com.Apollo.Rover
-                Ok(ProjectDirs::from("com", "Apollo", "Rover")
+                let project_dirs = ProjectDirs::from("com", "Apollo", "Rover")
                     .ok_or(HoustonProblem::DefaultConfigDirNotFound)?
                     .config_dir()
-                    .to_path_buf())
+                    .to_path_buf();
+
+                Ok(Utf8PathBuf::from_path_buf(project_dirs).map_err(|pb| {
+                    HoustonProblem::PathNotUnicode {
+                        path_display: pb.display().to_string(),
+                    }
+                })?)
             }
         }?;
 
         if !home.exists() {
-            fs::create_dir_all(&home).map_err(|_| {
-                HoustonProblem::CouldNotCreateConfigHome(home.display().to_string())
-            })?;
+            fs::create_dir_all(&home)
+                .map_err(|_| HoustonProblem::CouldNotCreateConfigHome(home.to_string()))?;
         }
 
         Ok(Config {
@@ -63,18 +68,20 @@ impl Config {
     pub fn clear(&self) -> Result<(), HoustonProblem> {
         tracing::debug!(home_dir = ?self.home);
         fs::remove_dir_all(&self.home)
-            .map_err(|_| HoustonProblem::NoConfigFound(self.home.display().to_string()))
+            .map_err(|_| HoustonProblem::NoConfigFound(self.home.to_string()))
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::Config;
     use assert_fs::TempDir;
+    use camino::Utf8PathBuf;
     #[test]
     fn it_can_clear_global_config() {
         let tmp_home = TempDir::new().unwrap();
-        let config = Config::new(Some(&tmp_home.path()), None).unwrap();
+        let tmp_path = Utf8PathBuf::from_path_buf(tmp_home.path().to_path_buf()).unwrap();
+        let config = Config::new(Some(&tmp_path), None).unwrap();
         assert!(config.home.exists());
         config.clear().unwrap();
         assert_eq!(config.home.exists(), false);

crates/houston/src/error.rs

@@ -21,6 +21,13 @@ pub enum HoustonProblem {
     #[error("Could not find a configuration directory at \"{0}\".")]
     NoConfigFound(String),
 
+    /// PathNotUnicode occurs when Houston encounteres a file path that is not valid UTF-8
+    #[error("File path \"{path_display}\" is not valid Unicode")]
+    PathNotUnicode {
+        /// The display name of the invalid path
+        path_display: String,
+    },
+
     /// ProfileNotFound occurs when a profile with a specified name can't be found.
     #[error("There is no profile named \"{0}\".")]
     ProfileNotFound(String),

crates/houston/src/profile/mod.rs

@@ -4,7 +4,7 @@ use crate::{Config, HoustonProblem};
 use sensitive::Sensitive;
 use serde::{Deserialize, Serialize};
 
-use std::path::PathBuf;
+use camino::Utf8PathBuf as PathBuf;
 use std::{fmt, fs, io};
 
 /// Collects configuration related to a profile.

crates/houston/src/profile/sensitive.rs

@@ -1,7 +1,7 @@
 use crate::{profile::Profile, Config, HoustonProblem};
 use serde::{Deserialize, Serialize};
 
-use std::path::PathBuf;
+use camino::Utf8PathBuf as PathBuf;
 use std::{fmt, fs};
 
 /// Holds sensitive information regarding authentication.

crates/houston/tests/auth.rs

@@ -2,6 +2,7 @@ use config::Config;
 use houston as config;
 
 use assert_fs::TempDir;
+use camino::Utf8Path;
 
 #[test]
 fn it_can_set_and_get_an_api_key_via_creds_file() {
@@ -51,5 +52,6 @@ fn it_can_get_an_api_key_via_env_var() {
 
 fn get_config(override_api_key: Option<String>) -> Config {
     let tmp_home = TempDir::new().unwrap();
-    Config::new(Some(&tmp_home.path()), override_api_key).unwrap()
+    let tmp_home_path = Utf8Path::from_path(tmp_home.path()).unwrap().to_owned();
+    Config::new(Some(&tmp_home_path), override_api_key).unwrap()
 }

crates/houston/tests/profile.rs

@@ -1,4 +1,5 @@
 use assert_fs::TempDir;
+use camino::Utf8Path;
 use config::Config;
 use houston as config;
 
@@ -28,5 +29,6 @@ fn it_lists_many_profiles() {
 
 fn get_config(override_api_key: Option<String>) -> Config {
     let tmp_home = TempDir::new().unwrap();
-    Config::new(Some(&tmp_home.path()), override_api_key).unwrap()
+    let tmp_home_path = Utf8Path::from_path(tmp_home.path()).unwrap().to_owned();
+    Config::new(Some(&tmp_home_path), override_api_key).unwrap()
 }

crates/robot-panic/Cargo.toml

@@ -5,6 +5,7 @@ edition = "2018"
 
 [dependencies]
 backtrace = "0.3"
+camino = "1.0"
 os_type = "2.2"
 serde = "1.0"
 termcolor = "1.1"

crates/robot-panic/src/lib.rs

@@ -180,8 +180,7 @@ pub fn print_msg(crash_report: &Report, meta: &Metadata) -> IoResult<()> {
                     "We have generated a report file at \"{}\". Submit an \
                          issue with the subject of \"{} Crash Report\" and include the \
                          report as an attachment.",
-                    path.display(),
-                    name
+                    path, name
                 )?;
             }
             Err(_) => {

crates/robot-panic/src/report.rs

@@ -7,9 +7,10 @@ use std::borrow::Cow;
 use std::error::Error;
 use std::fmt::Write as FmtWrite;
 use std::mem;
-use std::{env, fs::File, io::Write, path::Path, path::PathBuf};
+use std::{env, fs::File, io::Write};
 
 use backtrace::Backtrace;
+use camino::Utf8PathBuf;
 use serde::Serialize;
 use url::Url;
 use uuid::Uuid;
@@ -131,11 +132,16 @@ impl Report {
     }
 
     /// Write a file to disk.
-    pub fn persist(&self) -> Result<PathBuf, Box<dyn Error + 'static>> {
+    pub fn persist(&self) -> Result<Utf8PathBuf, Box<dyn Error + 'static>> {
         let uuid = Uuid::new_v4().to_hyphenated().to_string();
         let tmp_dir = env::temp_dir();
         let file_name = format!("report-{}.toml", &uuid);
-        let file_path = Path::new(&tmp_dir).join(file_name);
+        let base_file_path = Utf8PathBuf::from_path_buf(tmp_dir).map_err(|pb| {
+            let err: Box<dyn Error + 'static> =
+                format!("file \"{}\" is not valid UTF-8", pb.display()).into();
+            err
+        })?;
+        let file_path = base_file_path.join(file_name);
         let mut file = File::create(&file_path)?;
         let toml = self.serialize().unwrap();
         file.write_all(toml.as_bytes())?;