Merge #3240

3240: Fix filesystem rights on WASI, add integration test for file permissions r=Michael-F-Bryan a=fschutt Fixes #3125 Co-authored-by: Felix Schütt <[email protected]> Co-authored-by: Felix Schütt <[email protected]>
wasmerio · Oct 25, 2022 · 8357176 · 8357176
2 parents b1f3114 + f06d0e9
commit 8357176
Show file tree

Hide file tree

Showing 59 changed files with 381 additions and 41 deletions.
diff --git a/build.rs b/build.rs
@@ -20,6 +20,8 @@ fn main() -> anyhow::Result<()> {
         .expect("Can't get directory");
     build_deps::rerun_if_changed_paths("tests/wasi-wast/wasi/snapshot1/*")
         .expect("Can't get directory");
+    build_deps::rerun_if_changed_paths("tests/wasi-wast/wasi/nightly-2022-10-18/*")
+        .expect("Can't get directory");
 
     let out_dir = PathBuf::from(
         env::var_os("OUT_DIR").expect("The OUT_DIR environment variable must be set"),
@@ -66,7 +68,7 @@ fn main() -> anyhow::Result<()> {
         };
 
         with_test_module(&mut wasitests, "wasitests", |wasitests| {
-            for wasi_version in &["unstable", "snapshot1"] {
+            for wasi_version in &["unstable", "snapshot1", "nightly_2022_10_18"] {
                 with_test_module(wasitests, wasi_version, |wasitests| {
                     for (wasi_filesystem_test_name, wasi_filesystem_kind) in &[
                         ("host_fs", "WasiFileSystemKind::Host"),

diff --git a/lib/vfs/src/lib.rs b/lib/vfs/src/lib.rs
@@ -72,15 +72,27 @@ pub trait FileOpener {
 
 #[derive(Debug, Clone)]
 pub struct OpenOptionsConfig {
-    read: bool,
-    write: bool,
-    create_new: bool,
-    create: bool,
-    append: bool,
-    truncate: bool,
+    pub read: bool,
+    pub write: bool,
+    pub create_new: bool,
+    pub create: bool,
+    pub append: bool,
+    pub truncate: bool,
 }
 
 impl OpenOptionsConfig {
+    /// Returns the minimum allowed rights, given the rights of the parent directory
+    pub fn minimum_rights(&self, parent_rights: &Self) -> Self {
+        Self {
+            read: parent_rights.read && self.read,
+            write: parent_rights.write && self.write,
+            create_new: parent_rights.create_new && self.create_new,
+            create: parent_rights.create && self.create,
+            append: parent_rights.append && self.append,
+            truncate: parent_rights.truncate && self.truncate,
+        }
+    }
+
     pub const fn read(&self) -> bool {
         self.read
     }
@@ -106,7 +118,11 @@ impl OpenOptionsConfig {
     }
 }
 
-// TODO: manually implement debug
+impl fmt::Debug for OpenOptions {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        self.conf.fmt(f)
+    }
+}
 
 pub struct OpenOptions {
     opener: Box<dyn FileOpener>,
@@ -127,6 +143,11 @@ impl OpenOptions {
             },
         }
     }
+
+    pub fn get_config(&self) -> OpenOptionsConfig {
+        self.conf.clone()
+    }
+
     pub fn options(&mut self, options: OpenOptionsConfig) -> &mut Self {
         self.conf = options;
         self

diff --git a/lib/vfs/src/mem_fs/filesystem.rs b/lib/vfs/src/mem_fs/filesystem.rs
@@ -327,7 +327,7 @@ impl FileSystemInner {
         let mut components = path.components();
 
         match components.next() {
-            Some(Component::RootDir) => {}
+            Some(Component::RootDir) | None => {}
             _ => return Err(FsError::BaseNotDirectory),
         }
 

diff --git a/lib/wasi/src/syscalls/mod.rs b/lib/wasi/src/syscalls/mod.rs
@@ -2277,9 +2277,10 @@ pub fn path_open<M: MemorySize>(
     if !working_dir.rights.contains(Rights::PATH_OPEN) {
         return Errno::Access;
     }
+
     let path_string = unsafe { get_input_str!(&memory, path, path_len) };
 
-    debug!("=> fd: {}, path: {}", dirfd, &path_string);
+    debug!("=> path_open(): fd: {}, path: {}", dirfd, &path_string);
 
     let path_arg = std::path::PathBuf::from(&path_string);
     let maybe_inode = state.fs.get_inode_at_path(
@@ -2293,8 +2294,61 @@ pub fn path_open<M: MemorySize>(
     // TODO: traverse rights of dirs properly
     // COMMENTED OUT: WASI isn't giving appropriate rights here when opening
     //              TODO: look into this; file a bug report if this is a bug
+    //
+    // Maximum rights: should be the working dir rights
+    // Minimum rights: whatever rights are provided
     let adjusted_rights = /*fs_rights_base &*/ working_dir_rights_inheriting;
     let mut open_options = state.fs_new_open_options();
+
+    let target_rights = match maybe_inode {
+        Ok(_) => {
+            let write_permission = adjusted_rights.contains(Rights::FD_WRITE);
+
+            // append, truncate, and create all require the permission to write
+            let (append_permission, truncate_permission, create_permission) = if write_permission {
+                (
+                    fs_flags.contains(Fdflags::APPEND),
+                    o_flags.contains(Oflags::TRUNC),
+                    o_flags.contains(Oflags::CREATE),
+                )
+            } else {
+                (false, false, false)
+            };
+
+            wasmer_vfs::OpenOptionsConfig {
+                read: fs_rights_base.contains(Rights::FD_READ),
+                write: write_permission,
+                create_new: create_permission && o_flags.contains(Oflags::EXCL),
+                create: create_permission,
+                append: append_permission,
+                truncate: truncate_permission,
+            }
+        }
+        Err(_) => wasmer_vfs::OpenOptionsConfig {
+            append: fs_flags.contains(Fdflags::APPEND),
+            write: fs_rights_base.contains(Rights::FD_WRITE),
+            read: fs_rights_base.contains(Rights::FD_READ),
+            create_new: o_flags.contains(Oflags::CREATE) && o_flags.contains(Oflags::EXCL),
+            create: o_flags.contains(Oflags::CREATE),
+            truncate: o_flags.contains(Oflags::TRUNC),
+        },
+    };
+
+    let parent_rights = wasmer_vfs::OpenOptionsConfig {
+        read: working_dir.rights.contains(Rights::FD_READ),
+        write: working_dir.rights.contains(Rights::FD_WRITE),
+        // The parent is a directory, which is why these options
+        // aren't inherited from the parent (append / truncate doesn't work on directories)
+        create_new: true,
+        create: true,
+        append: true,
+        truncate: true,
+    };
+
+    let minimum_rights = target_rights.minimum_rights(&parent_rights);
+
+    open_options.options(minimum_rights.clone());
+
     let inode = if let Ok(inode) = maybe_inode {
         // Happy path, we found the file we're trying to open
         let mut guard = inodes.arena[inode].write();
@@ -2318,35 +2372,25 @@ pub fn path_open<M: MemorySize>(
                     return Errno::Exist;
                 }
 
-                let write_permission = adjusted_rights.contains(Rights::FD_WRITE);
-                // append, truncate, and create all require the permission to write
-                let (append_permission, truncate_permission, create_permission) =
-                    if write_permission {
-                        (
-                            fs_flags.contains(Fdflags::APPEND),
-                            o_flags.contains(Oflags::TRUNC),
-                            o_flags.contains(Oflags::CREATE),
-                        )
-                    } else {
-                        (false, false, false)
-                    };
                 let open_options = open_options
-                    .read(true)
-                    // TODO: ensure these rights are actually valid given parent, etc.
-                    .write(write_permission)
-                    .create(create_permission)
-                    .append(append_permission)
-                    .truncate(truncate_permission);
-                open_flags |= Fd::READ;
-                if adjusted_rights.contains(Rights::FD_WRITE) {
+                    .write(minimum_rights.write)
+                    .create(minimum_rights.create)
+                    .append(minimum_rights.append)
+                    .truncate(minimum_rights.truncate);
+
+                if minimum_rights.read {
+                    open_flags |= Fd::READ;
+                }
+                if minimum_rights.write {
                     open_flags |= Fd::WRITE;
                 }
-                if o_flags.contains(Oflags::CREATE) {
+                if minimum_rights.create {
                     open_flags |= Fd::CREATE;
                 }
-                if o_flags.contains(Oflags::TRUNC) {
+                if minimum_rights.truncate {
                     open_flags |= Fd::TRUNCATE;
                 }
+
                 *handle = Some(wasi_try!(open_options
                     .open(&path)
                     .map_err(fs_error_into_wasi_err)));
@@ -2393,21 +2437,35 @@ pub fn path_open<M: MemorySize>(
                         new_path.push(&new_entity_name);
                         new_path
                     }
-                    Kind::Root { .. } => return Errno::Access,
+                    Kind::Root { .. } => {
+                        let mut new_path = std::path::PathBuf::new();
+                        new_path.push(&new_entity_name);
+                        new_path
+                    }
                     _ => return Errno::Inval,
                 }
             };
             // once we got the data we need from the parent, we lookup the host file
             // todo: extra check that opening with write access is okay
             let handle = {
                 let open_options = open_options
-                    .read(true)
-                    .append(fs_flags.contains(Fdflags::APPEND))
-                    // TODO: ensure these rights are actually valid given parent, etc.
-                    // write access is required for creating a file
-                    .write(true)
-                    .create_new(true);
-                open_flags |= Fd::READ | Fd::WRITE | Fd::CREATE | Fd::TRUNCATE;
+                    .read(minimum_rights.read)
+                    .append(minimum_rights.append)
+                    .write(minimum_rights.write)
+                    .create_new(minimum_rights.create_new);
+
+                if minimum_rights.read {
+                    open_flags |= Fd::READ;
+                }
+                if minimum_rights.write {
+                    open_flags |= Fd::WRITE;
+                }
+                if minimum_rights.create_new {
+                    open_flags |= Fd::CREATE;
+                }
+                if minimum_rights.truncate {
+                    open_flags |= Fd::TRUNCATE;
+                }
 
                 Some(wasi_try!(open_options.open(&new_file_host_path).map_err(
                     |e| {

diff --git a/tests/ignores.txt b/tests/ignores.txt
@@ -96,3 +96,36 @@ wasitests::unstable::host_fs::unix_open_special_files
 wasitests::snapshot1::host_fs::unix_open_special_files
 wasitests::unstable::mem_fs::unix_open_special_files
 wasitests::snapshot1::mem_fs::unix_open_special_files
+
+# These tests are all tests that are disabled for unstable + snapshot1,
+# so they are also disabled for nightly
+wasitests::nightly_2022_10_18::host_fs::close_preopen_fd
+wasitests::nightly_2022_10_18::host_fs::create_dir
+wasitests::nightly_2022_10_18::host_fs::envvar
+wasitests::nightly_2022_10_18::host_fs::fd_allocate
+wasitests::nightly_2022_10_18::host_fs::fd_close
+wasitests::nightly_2022_10_18::host_fs::fd_pread
+wasitests::nightly_2022_10_18::host_fs::fd_read
+wasitests::nightly_2022_10_18::host_fs::fd_rename_path
+wasitests::nightly_2022_10_18::host_fs::poll_oneoff
+wasitests::nightly_2022_10_18::host_fs::readlink
+wasitests::nightly_2022_10_18::host_fs::unix_open_special_files
+wasitests::nightly_2022_10_18::host_fs::writing
+wasitests::nightly_2022_10_18::mem_fs::close_preopen_fd
+wasitests::nightly_2022_10_18::mem_fs::create_dir
+wasitests::nightly_2022_10_18::mem_fs::envvar
+wasitests::nightly_2022_10_18::mem_fs::fd_allocate
+wasitests::nightly_2022_10_18::mem_fs::fd_close
+wasitests::nightly_2022_10_18::mem_fs::fd_pread
+wasitests::nightly_2022_10_18::mem_fs::fd_read
+wasitests::nightly_2022_10_18::mem_fs::poll_oneoff
+wasitests::nightly_2022_10_18::mem_fs::readlink
+wasitests::nightly_2022_10_18::mem_fs::unix_open_special_files
+wasitests::nightly_2022_10_18::mem_fs::writing
+
+# These tests are failing in CI for some reason, but didn't fail on older compiler versions
+wasitests::nightly_2022_10_18::host_fs::path_symlink
+wasitests::nightly_2022_10_18::mem_fs::path_symlink
+
+wasitests::nightly_2022_10_18::host_fs::fd_append
+wasitests::nightly_2022_10_18::mem_fs::fd_append
diff --git a/tests/wasi-wast/src/main.rs b/tests/wasi-wast/src/main.rs
@@ -7,7 +7,9 @@ mod wasi_version;
 mod wasitests;
 
 pub use crate::set_up_toolchain::install_toolchains;
-pub use crate::wasi_version::{WasiVersion, ALL_WASI_VERSIONS, LATEST_WASI_VERSION};
+pub use crate::wasi_version::{
+    WasiVersion, ALL_WASI_VERSIONS, LATEST_WASI_VERSION, NIGHTLY_VERSION,
+};
 pub use crate::wasitests::{build, WasiOptions, WasiTest};
 
 use gumdrop::Options;
@@ -17,6 +19,8 @@ pub struct TestGenOptions {
     /// if you want to specify specific tests to generate
     #[options(free)]
     free: Vec<String>,
+    /// Whether to use the current nightly instead of the latest snapshot0 compiler
+    nightly: bool,
     /// Whether or not to do operations for all versions of WASI or just the latest.
     all_versions: bool,
     /// Whether or not the Wasm will be generated.
@@ -38,8 +42,11 @@ fn main() {
     let generate_all = opts.all_versions;
     let set_up_toolchain = opts.set_up_toolchain;
     let generate_wasm = opts.generate_wasm;
+    let nightly = opts.nightly;
     let wasi_versions = if generate_all {
         ALL_WASI_VERSIONS
+    } else if nightly {
+        NIGHTLY_VERSION
     } else {
         LATEST_WASI_VERSION
     };

diff --git a/tests/wasi-wast/src/wasi_version.rs b/tests/wasi-wast/src/wasi_version.rs
@@ -1,29 +1,40 @@
 pub static ALL_WASI_VERSIONS: &[WasiVersion] = &[WasiVersion::Unstable, WasiVersion::Snapshot1];
 pub static LATEST_WASI_VERSION: &[WasiVersion] = &[WasiVersion::get_latest()];
+pub static NIGHTLY_VERSION: &[WasiVersion] = &[WasiVersion::current_nightly()];
 
 #[derive(Debug, Clone, Copy)]
 pub enum WasiVersion {
     /// A.K.A. Snapshot0
     Unstable,
     Snapshot1,
+    /// This is for making tests pass on Apple M1 while
+    /// still keeping the old test for compatibility reasons
+    #[allow(non_camel_case_types)]
+    Nightly_2022_10_18,
 }
 
 impl WasiVersion {
     pub const fn get_latest() -> Self {
         Self::Snapshot1
     }
 
+    pub const fn current_nightly() -> Self {
+        Self::Nightly_2022_10_18
+    }
+
     pub fn get_compiler_toolchain(&self) -> &'static str {
         match self {
             WasiVersion::Unstable => "nightly-2019-09-13",
             WasiVersion::Snapshot1 => "1.53.0",
+            WasiVersion::Nightly_2022_10_18 => "nightly-2022-10-18",
         }
     }
 
     pub fn get_directory_name(&self) -> &'static str {
         match self {
             WasiVersion::Unstable => "unstable",
             WasiVersion::Snapshot1 => "snapshot1",
+            WasiVersion::Nightly_2022_10_18 => "nightly_2022_10_18",
         }
     }
 }
diff --git a/tests/wasi-wast/wasi/nightly_2022_10_18/close_preopen_fd.wasm b/tests/wasi-wast/wasi/nightly_2022_10_18/close_preopen_fd.wasm
diff --git a/tests/wasi-wast/wasi/nightly_2022_10_18/close_preopen_fd.wast b/tests/wasi-wast/wasi/nightly_2022_10_18/close_preopen_fd.wast
@@ -0,0 +1,7 @@
+;; This file was generated by https://github.com/wasmerio/wasi-tests
+
+(wasi_test "close_preopen_fd.wasm"
+  (map_dirs "hamlet:test_fs/hamlet")
+  (assert_return (i64.const 0))
+  (assert_stdout "accessing preopen fd was a success\nClosing preopen fd was a success\naccessing closed preopen fd was an EBADF error: true\n")
+)
diff --git a/tests/wasi-wast/wasi/nightly_2022_10_18/create_dir.wasm b/tests/wasi-wast/wasi/nightly_2022_10_18/create_dir.wasm
diff --git a/tests/wasi-wast/wasi/nightly_2022_10_18/create_dir.wast b/tests/wasi-wast/wasi/nightly_2022_10_18/create_dir.wast
@@ -0,0 +1,7 @@
+;; This file was generated by https://github.com/wasmerio/wasi-tests
+
+(wasi_test "create_dir.wasm"
+  (preopens "test_fs")
+  (assert_return (i64.const 0))
+  (assert_stdout "Test file exists: false\nDir exists: false\nDir exists: false\nDir exists: false\nSuccess\n")
+)
diff --git a/tests/wasi-wast/wasi/nightly_2022_10_18/envvar.wasm b/tests/wasi-wast/wasi/nightly_2022_10_18/envvar.wasm
diff --git a/tests/wasi-wast/wasi/nightly_2022_10_18/envvar.wast b/tests/wasi-wast/wasi/nightly_2022_10_18/envvar.wast
@@ -0,0 +1,7 @@
+;; This file was generated by https://github.com/wasmerio/wasi-tests
+
+(wasi_test "envvar.wasm"
+  (envs "DOG=1" "CAT=2")
+  (assert_return (i64.const 0))
+  (assert_stdout "Env vars:\nCAT=2\nDOG=1\nDOG Ok(\"1\")\nDOG_TYPE Err(NotPresent)\nSET VAR Ok(\"HELLO\")\n")
+)
diff --git a/tests/wasi-wast/wasi/nightly_2022_10_18/fd_allocate.wast b/tests/wasi-wast/wasi/nightly_2022_10_18/fd_allocate.wast
@@ -0,0 +1,7 @@
+;; This file was generated by https://github.com/wasmerio/wasi-tests
+
+(wasi_test "fd_allocate.wasm"
+  (temp_dirs ".")
+  (assert_return (i64.const 0))
+  (assert_stdout "171\n1405\n")
+)
diff --git a/tests/wasi-wast/wasi/nightly_2022_10_18/fd_append.wasm b/tests/wasi-wast/wasi/nightly_2022_10_18/fd_append.wasm