ref(wash): Change plugins to support arbitrary path access

This allows plugins to mark arguments as paths so that they can be
preopened and allowed in the component. This tries to walk a path between
security and flexibility. If an argument is marked as a path, wash will
allow full access to it if it is a directory and then limited access to
a directory and full access to the file if it is a path. It isn't
perfect due to the limited nature of preopens, but it does mean that the
plugin will not get access to anything outside of its scratch dir
without the user explicitly passing the path.

Once this is merged there will be two follow ups: one is a PR to this
repo updating the example code and the other will be to the docs repo
to update documentation on the security around paths

Signed-off-by: Taylor Thomas <taylor@cosmonic.com>

crates/wash-cli/src/bin/wash.rs

@@ -36,7 +36,7 @@ use wash_lib::cli::stop::StopCommand;
 use wash_lib::cli::update::UpdateCommand;
 use wash_lib::cli::{CommandOutput, OutputKind};
 use wash_lib::drain::Drain as DrainSelection;
-use wash_lib::plugin::subcommand::SubcommandRunner;
+use wash_lib::plugin::subcommand::{DirMapping, SubcommandRunner};
 
 const HELP: &str = r"
 _________________________________________________________________________________
@@ -225,6 +225,7 @@ async fn main() {
     // Load plugins if they are not disabled
     let plugins = if std::env::var("WASH_DISABLE_PLUGINS").is_err() {
         if let Some((plugins, dir)) = load_plugins().await {
+            let mut plugin_paths = HashMap::new();
             for plugin in plugins.all_metadata().into_iter().cloned() {
                 if command.find_subcommand(&plugin.id).is_some() {
                     tracing::error!(
@@ -233,24 +234,52 @@ async fn main() {
                     );
                     continue;
                 }
-                let subcmd = Command::new(plugin.id)
+                let (flag_args, path_ids): (Vec<_>, Vec<_>) = plugin
+                    .flags
+                    .into_iter()
+                    .map(|(flag, arg)| {
+                        let trimmed = flag.trim_start_matches('-').to_owned();
+                        // Return a list of args with an Option containing the ID of the flag if it was a path
+                        (
+                            Arg::new(trimmed.clone())
+                                .long(trimmed.clone())
+                                .help(arg.description)
+                                .required(arg.required),
+                            arg.is_path.then_some(trimmed),
+                        )
+                    })
+                    .unzip();
+                let (positional_args, positional_ids): (Vec<_>, Vec<_>) = plugin
+                    .arguments
+                    .into_iter()
+                    .map(|(name, arg)| {
+                        let trimmed = name.trim().to_owned();
+                        // Return a list of args with an Option containing the ID of the argument if it was a path
+                        (
+                            Arg::new(trimmed.clone())
+                                .help(arg.description)
+                                .required(arg.required),
+                            arg.is_path.then_some(trimmed),
+                        )
+                    })
+                    .unzip();
+                let subcmd = Command::new(plugin.id.clone())
                     .about(plugin.description)
                     .author(plugin.author)
                     .version(plugin.version)
                     .display_name(plugin.name)
-                    .args(plugin.flags.into_iter().map(|(flag, help)| {
-                        let trimmed = flag.trim_start_matches('-').to_owned();
-                        Arg::new(trimmed.clone()).long(trimmed).help(help)
-                    }))
-                    .args(
-                        plugin
-                            .arguments
-                            .into_iter()
-                            .map(|(name, help)| Arg::new(name).help(help)),
-                    );
+                    .args(flag_args.into_iter().chain(positional_args));
                 command = command.subcommand(subcmd);
+                plugin_paths.insert(
+                    plugin.id,
+                    path_ids
+                        .into_iter()
+                        .chain(positional_ids)
+                        .flatten()
+                        .collect::<Vec<_>>(),
+                );
             }
-            Some((plugins, dir))
+            Some((plugins, dir, plugin_paths))
         } else {
             None
         }
@@ -264,14 +293,39 @@ async fn main() {
 
     let cli = match (Cli::from_arg_matches(&matches), plugins) {
         (Ok(cli), _) => cli,
-        (Err(mut e), Some((mut plugins, plugin_dir))) => {
-            let (id, _sub_matches) = match matches.subcommand() {
+        (Err(mut e), Some((mut plugins, plugin_dir, plugin_paths))) => {
+            let (id, sub_matches) = match matches.subcommand() {
                 Some(data) => data,
                 None => {
                     e.exit();
                 }
             };
 
+            let dir = match ensure_plugin_scratch_dir_exists(plugin_dir, id).await {
+                Ok(dir) => dir,
+                Err(e) => {
+                    eprintln!("Error creating plugin scratch directory: {}", e);
+                    std::process::exit(1);
+                }
+            };
+
+            let mut plugin_dirs = Vec::new();
+
+            // Try fetching all path matches from args marked as paths
+            plugin_dirs.extend(
+                plugin_paths
+                    .get(id)
+                    .map(ToOwned::to_owned)
+                    .unwrap_or_default()
+                    .into_iter()
+                    .filter_map(|id| {
+                        sub_matches.get_one::<String>(&id).map(|path| DirMapping {
+                            host_path: path.into(),
+                            component_path: None,
+                        })
+                    }),
+            );
+
             if plugins.metadata(id).is_none() {
                 e.insert(
                     clap::error::ContextKind::InvalidSubcommand,
@@ -286,14 +340,7 @@ async fn main() {
             // revisit this later with something if we need to. I did do some basic testing that
             // even if you wrap wash in a shell script, it still works.
             let args: Vec<String> = std::env::args().skip(1).collect();
-            let dir = match ensure_plugin_scratch_dir_exists(plugin_dir, id).await {
-                Ok(dir) => dir,
-                Err(e) => {
-                    eprintln!("Error creating plugin scratch directory: {}", e);
-                    std::process::exit(1);
-                }
-            };
-            if let Err(e) = plugins.run(id, dir, &args).await {
+            if let Err(e) = plugins.run(id, dir, plugin_dirs, args).await {
                 eprintln!("Error running plugin: {}", e);
                 std::process::exit(1);
             } else {

crates/wash-lib/src/plugin/subcommand.rs

@@ -19,18 +19,32 @@ use wasmtime_wasi_http::WasiHttpCtx;
 
 use super::Data;
 
+const DIRECTORY_ALLOW: DirPerms = DirPerms::all();
+const DIRECTORY_DENY: DirPerms = DirPerms::READ;
+
 struct InstanceData {
     instance: Subcommands,
     metadata: Metadata,
     loaded_path: PathBuf,
     store: wasmtime::Store<Data>,
 }
 
+/// A struct that manages loading and running subcommand plugins
 pub struct SubcommandRunner {
     engine: Engine,
     plugins: HashMap<String, InstanceData>,
 }
 
+/// Host directory mapping to provide to plugins
+pub struct DirMapping {
+    /// The path on the host that should be opened. If this is a file, its parent directory will be
+    /// added with no RW access, but with RW access to the files in that directory. If it is a
+    /// directory, it will be added with RW access to that directory
+    pub host_path: PathBuf,
+    /// The path that will be accessible in the component. Otherwise defaults to the `host_path`
+    pub component_path: Option<String>,
+}
+
 impl SubcommandRunner {
     /// Creates a new subcommand runner with no plugins loaded.
     pub fn new() -> anyhow::Result<Self> {
@@ -149,18 +163,19 @@ impl SubcommandRunner {
     }
 
     /// Run a subcommand with the given name and args. The plugin will inherit all
-    /// stdout/stderr/stdin/env. The given plugin_dir is used to grant the plugin access to the
-    /// filesystem in a specific directory, and should already exist. An error will only be returned
-    /// if there was a problem with the plugin (such as the plugin_dir not existing) or the
-    /// subcommand itself.
+    /// stdout/stderr/stdin/env. The given plugin_dirs will be mapped into the plugin after
+    /// canonicalizing all paths and normalizing them to use `/` instead of `\`. An error will only
+    /// be returned if there was a problem with the plugin (such as the plugin dirs not existing or
+    /// failure to canonicalize) or the subcommand itself.
     ///
     /// All plugins will be passed environment variables starting with
     /// `WASH_PLUGIN_${plugin_id.to_upper()}_` from the current process. Other vars will be ignored
     pub async fn run(
         &mut self,
         plugin_id: &str,
-        plugin_dir: impl AsRef<Path>,
-        args: &[impl AsRef<str>],
+        plugin_dir: PathBuf,
+        dirs: Vec<DirMapping>,
+        mut args: Vec<String>,
     ) -> anyhow::Result<()> {
         let plugin = self
             .plugins
@@ -171,18 +186,81 @@ impl SubcommandRunner {
         let vars: Vec<_> = std::env::vars()
             .filter(|(k, _)| k.starts_with(&env_prefix))
             .collect();
-        plugin.store.data_mut().ctx = WasiCtxBuilder::new()
-            // Disable socket connections for now. We may gradually open this up later
-            .socket_addr_check(|_, _| false)
-            .inherit_stderr()
-            .inherit_stdin()
+        let mut ctx = WasiCtxBuilder::new();
+        for dir in dirs {
+            // To avoid relative dirs and permissions issues, we canonicalize the host path
+            let canonicalized = tokio::fs::canonicalize(&dir.host_path)
+                .await
+                .context("Error when canonicalizing given path")?;
+            // We need this later and will have to return an error anyway if this fails
+            let str_canonical = canonicalized.to_str().ok_or_else(|| anyhow::anyhow!("Canonicalized path cannot be converted to a string for use in a plugin. This is a limitation of the WASI API"))?.to_string();
+            // Check if the path is a file or a dir so we can handle permissions accordingly
+            let is_dir = tokio::fs::metadata(&canonicalized)
+                .await
+                .map(|m| m.is_dir())
+                .context("Error when checking if path is a file or a dir")?;
+            let (host_path, guest_path, dir_perms) = match (is_dir, dir.component_path) {
+                (true, Some(path)) => (canonicalized.clone(), path, DIRECTORY_ALLOW),
+                (false, Some(path)) => (
+                    canonicalized
+                        .parent()
+                        .ok_or_else(|| anyhow::anyhow!("Could not get parent of given file"))?
+                        .to_path_buf(),
+                    path,
+                    DIRECTORY_DENY,
+                ),
+                (true, None) => (
+                    canonicalized.clone(),
+                    str_canonical.clone(),
+                    DIRECTORY_ALLOW,
+                ),
+                (false, None) => {
+                    let parent = canonicalized
+                        .parent()
+                        .ok_or_else(|| anyhow::anyhow!("Could not get parent of given file"))?
+                        .to_path_buf();
+                    (
+                        parent.clone(),
+                        // SAFETY: We already checked that canonicalized was a string above so we
+                        // can just unwrap here
+                        parent.to_str().unwrap().to_string(),
+                        DIRECTORY_DENY,
+                    )
+                }
+            };
+
+            // On Windows, we need to normalize the path separators to "/" since that is what is
+            // expected by things like `PathBuf` when built for WASI.
+            #[cfg(target_family = "windows")]
+            let guest_path = guest_path.replace('\\', "/");
+            #[cfg(target_family = "windows")]
+            let str_canonical = str_canonical.replace('\\', "/");
+            ctx.preopened_dir(host_path, guest_path, dir_perms, FilePerms::all())
+                .context("Error when preopening path argument")?;
+            // Substitute the path in the args with the canonicalized path
+            let matching = args
+                .iter_mut()
+                .find(|arg| {
+                    <&mut std::string::String as std::convert::AsRef<Path>>::as_ref(arg)
+                        == dir.host_path
+                })
+                .ok_or_else(|| {
+                    anyhow::anyhow!(
+                        "Could not find host path {} in args for replacement",
+                        dir.host_path.display()
+                    )
+                })?;
+            *matching = str_canonical;
+        }
+        // Disable socket connections for now. We may gradually open this up later
+        ctx.socket_addr_check(|_, _| false)
             .inherit_stdio()
-            .inherit_stdout()
-            .preopened_dir(plugin_dir, "/", DirPerms::all(), FilePerms::all())
+            .preopened_dir(plugin_dir, "/", DIRECTORY_ALLOW, FilePerms::all())
             .context("Error when preopening plugin dir")?
-            .args(args)
-            .envs(&vars)
-            .build();
+            .args(&args)
+            .envs(&vars);
+
+        plugin.store.data_mut().ctx = ctx.build();
         plugin
             .instance
             .wasi_cli_run()

crates/wash-lib/tests/plugins.rs

@@ -1,6 +1,7 @@
 use std::path::PathBuf;
 
 use tokio::process::Command;
+use wash_lib::plugin::subcommand::DirMapping;
 
 /// Builds the plugin in the given directory. It must exist inside of tests/plugins/ and the
 /// directory name must match the name of the built binary. Returns the path to the built binary.
@@ -51,10 +52,40 @@ async fn test_subcommand() {
     assert_eq!(metadata.id, "hello");
 
     let temp = tempfile::tempdir().unwrap();
+    let extra_dir = tempfile::tempdir().unwrap();
+    tokio::fs::write(extra_dir.path().join("hello.txt"), "hello")
+        .await
+        .unwrap();
+    tokio::fs::write(extra_dir.path().join("world.txt"), "world")
+        .await
+        .unwrap();
+
+    let file_dir = tempfile::tempdir().unwrap();
+    let file = file_dir.path().join("hello.txt");
+    tokio::fs::write(&file, "Hello from a file").await.unwrap();
 
     // TODO: allow configuration of stdout/stderr so we can check for output
     subcommand
-        .run("hello", temp.path(), &["world"])
+        .run(
+            "hello",
+            temp.path().to_path_buf(),
+            vec![
+                DirMapping {
+                    host_path: extra_dir.path().to_path_buf(),
+                    component_path: None,
+                },
+                DirMapping {
+                    host_path: file.clone(),
+                    component_path: None,
+                },
+            ],
+            vec![
+                "hello".to_string(),
+                "--foo".to_string(),
+                extra_dir.path().to_str().unwrap().to_string(),
+                file.to_str().unwrap().to_string(),
+            ],
+        )
         .await
         .expect("Should be able to run plugin");
 

crates/wash-lib/tests/plugins/hello_plugin/Cargo.toml

@@ -9,6 +9,7 @@ version = "0.1.0"
 crate-type = ["cdylib"]
 
 [dependencies]
+clap = { version = "4", features = ["derive"] }
 serde = { version = "1", features = ["derive"] }
 serde_json = "1"
 wit-bindgen = { version = "0.24", features = ["default"] }