From 1c5c74da834f5dfdf5fe01c478b785e4c021f6e2 Mon Sep 17 00:00:00 2001 From: Christoph Herzog Date: Tue, 5 Sep 2023 17:46:21 +0200 Subject: [PATCH 1/2] fix: Prevent invalid file system merging errors This commit changes the EmptyFileSystem read_dir() implementation to always return an empty iterator when trying to read the root path "/". This is done because some code paths expect the root to always be readable. This also makes sense theoretically, because an empty file system should still be readable, just not contain any files. --- lib/virtual-fs/src/empty_fs.rs | 24 ++++++++++++++++++++++-- lib/virtual-fs/src/lib.rs | 7 +++++++ lib/wasix/src/fs/mod.rs | 8 +++++++- lib/wasix/src/os/console/mod.rs | 9 ++++----- lib/wasix/src/state/env.rs | 1 + 5 files changed, 41 insertions(+), 8 deletions(-) diff --git a/lib/virtual-fs/src/empty_fs.rs b/lib/virtual-fs/src/empty_fs.rs index 04807ef4bad..ce5be69637c 100644 --- a/lib/virtual-fs/src/empty_fs.rs +++ b/lib/virtual-fs/src/empty_fs.rs @@ -13,7 +13,14 @@ pub struct EmptyFileSystem {} #[allow(unused_variables)] impl FileSystem for EmptyFileSystem { fn read_dir(&self, path: &Path) -> Result { - Err(FsError::EntryNotFound) + // Special-case the root path by returning an empty iterator. + // An empty file system should still be readable, just not contain + // any entries. + if path == Path::new("/") { + Ok(ReadDir::new(Vec::new())) + } else { + Err(FsError::EntryNotFound) + } } fn create_dir(&self, path: &Path) -> Result<()> { @@ -29,7 +36,20 @@ impl FileSystem for EmptyFileSystem { } fn metadata(&self, path: &Path) -> Result { - Err(FsError::EntryNotFound) + // Special-case the root path by returning an stub value. + // An empty file system should still be readable, just not contain + // any entries. + if path == Path::new("/") { + Ok(Metadata { + ft: FileType::new_dir(), + accessed: 0, + created: 0, + modified: 0, + len: 0, + }) + } else { + Err(FsError::EntryNotFound) + } } fn symlink_metadata(&self, path: &Path) -> Result { diff --git a/lib/virtual-fs/src/lib.rs b/lib/virtual-fs/src/lib.rs index 0775d5f124f..591f086c44c 100644 --- a/lib/virtual-fs/src/lib.rs +++ b/lib/virtual-fs/src/lib.rs @@ -665,6 +665,13 @@ pub struct FileType { } impl FileType { + pub fn new_dir() -> Self { + Self { + dir: true, + ..Default::default() + } + } + pub fn is_dir(&self) -> bool { self.dir } diff --git a/lib/wasix/src/fs/mod.rs b/lib/wasix/src/fs/mod.rs index 51e49cc5e7e..1ae2dc0cad0 100644 --- a/lib/wasix/src/fs/mod.rs +++ b/lib/wasix/src/fs/mod.rs @@ -363,7 +363,13 @@ async fn merge_filesystems( to_check.push_back(PathBuf::from("/")); while let Some(path) = to_check.pop_front() { - let metadata = source.metadata(&path)?; + let metadata = match source.metadata(&path) { + Ok(m) => m, + Err(err) => { + tracing::debug!(path=%path.display(), source_fs=?source, ?err, "failed to get metadata for path while merging file systems"); + return Err(err); + } + }; if metadata.is_dir() { create_dir_all(destination, &path)?; diff --git a/lib/wasix/src/os/console/mod.rs b/lib/wasix/src/os/console/mod.rs index 886b44ab2e3..e8be39e2eec 100644 --- a/lib/wasix/src/os/console/mod.rs +++ b/lib/wasix/src/os/console/mod.rs @@ -222,15 +222,14 @@ impl Console { .with_envs(self.env.clone().into_iter()) .with_args(args) .with_capabilities(self.capabilities.clone()) + .with_stdin(Box::new(self.stdin.clone())) + .with_stdout(Box::new(self.stdout.clone())) + .with_stderr(Box::new(self.stderr.clone())) .prepare_webc_env(prog, &wasi_opts, &pkg, self.runtime.clone(), Some(root_fs)) // TODO: better error conversion .map_err(|err| SpawnError::Other(err.into()))?; - let env = builder - .stdin(Box::new(self.stdin.clone())) - .stdout(Box::new(self.stdout.clone())) - .stderr(Box::new(self.stderr.clone())) - .build()?; + let env = builder.build()?; // Display the welcome message if !self.whitelabel && !self.no_welcome { diff --git a/lib/wasix/src/state/env.rs b/lib/wasix/src/state/env.rs index 4b0a8f819f9..bf56e90bb79 100644 --- a/lib/wasix/src/state/env.rs +++ b/lib/wasix/src/state/env.rs @@ -869,6 +869,7 @@ impl WasiEnv { /// [cmd-atom]: crate::bin_factory::BinaryPackageCommand::atom() /// [pkg-fs]: crate::bin_factory::BinaryPackage::webc_fs pub fn use_package(&self, pkg: &BinaryPackage) -> Result<(), WasiStateCreationError> { + tracing::trace!(packagae=%pkg.package_name, "merging package dependency into wasi environment"); let root_fs = &self.state.fs.root_fs; // We first need to copy any files in the package over to the From 6f0b022151c88a0a2bf1062c4dcadc8f9ba2fc83 Mon Sep 17 00:00:00 2001 From: Christoph Herzog Date: Tue, 5 Sep 2023 18:19:16 +0200 Subject: [PATCH 2/2] tests: Add regression test for file system merging error Tests merging as done in the `Console`. --- lib/wasix/src/os/console/mod.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/lib/wasix/src/os/console/mod.rs b/lib/wasix/src/os/console/mod.rs index e8be39e2eec..e8129f50cda 100644 --- a/lib/wasix/src/os/console/mod.rs +++ b/lib/wasix/src/os/console/mod.rs @@ -364,4 +364,32 @@ mod tests { assert_eq!(out, "hello VAL1\n"); } + + /// Regression test to ensure merging of multiple packages works correctly. + #[test] + fn test_console_python_merge() { + std::env::set_var("RUST_LOG", "wasmer_wasix=trace"); + tracing_subscriber::fmt::init(); + let tokio_rt = tokio::runtime::Runtime::new().unwrap(); + let rt_handle = tokio_rt.handle().clone(); + let _guard = rt_handle.enter(); + + let tm = TokioTaskManager::new(tokio_rt); + let mut rt = PluggableRuntime::new(Arc::new(tm)); + rt.set_engine(Some(wasmer::Engine::default())) + .set_package_loader(BuiltinPackageLoader::from_env().unwrap()); + + let cmd = "wasmer-tests/python-env-dump --help"; + + let (mut handle, _proc) = Console::new(cmd, Arc::new(rt)).run().unwrap(); + + let code = rt_handle + .block_on(async move { + let res = handle.wait_finished().await?; + Ok::<_, anyhow::Error>(res) + }) + .unwrap(); + + assert_eq!(code.raw(), 0); + } }