diff --git a/.github/subscribe-to-label.json b/.github/subscribe-to-label.json index 08b1ee0d3c95..a39d316323eb 100644 --- a/.github/subscribe-to-label.json +++ b/.github/subscribe-to-label.json @@ -2,6 +2,5 @@ "cfallin": ["isle"], "fitzgen": ["fuzzing", "isle", "wasmtime:ref-types"], "peterhuene": ["wasmtime:api", "wasmtime:c-api"], - "kubkon": ["wasi"], "saulecabrera": ["winch"] } diff --git a/crates/test-programs/tests/wasi-cap-std-sync.rs b/crates/test-programs/tests/wasi-cap-std-sync.rs index 931e3a1b9640..3e1ba3d15cce 100644 --- a/crates/test-programs/tests/wasi-cap-std-sync.rs +++ b/crates/test-programs/tests/wasi-cap-std-sync.rs @@ -188,6 +188,10 @@ fn path_open_create_existing() { run("path_open_create_existing", true).unwrap() } #[test_log::test] +fn path_open_read_write() { + run("path_open_read_write", true).unwrap() +} +#[test_log::test] fn path_open_dirfd_not_dir() { run("path_open_dirfd_not_dir", true).unwrap() } @@ -265,3 +269,7 @@ fn symlink_loop() { fn unlink_file_trailing_slashes() { run("unlink_file_trailing_slashes", true).unwrap() } +#[test_log::test] +fn path_open_preopen() { + run("path_open_preopen", true).unwrap() +} diff --git a/crates/test-programs/tests/wasi-preview1-host-in-preview2.rs b/crates/test-programs/tests/wasi-preview1-host-in-preview2.rs index ec95c3a53b5d..0776cf1017a5 100644 --- a/crates/test-programs/tests/wasi-preview1-host-in-preview2.rs +++ b/crates/test-programs/tests/wasi-preview1-host-in-preview2.rs @@ -229,6 +229,10 @@ async fn path_open_create_existing() { run("path_open_create_existing", true).await.unwrap() } #[test_log::test(tokio::test(flavor = "multi_thread"))] +async fn path_open_read_write() { + run("path_open_read_write", true).await.unwrap() +} +#[test_log::test(tokio::test(flavor = "multi_thread"))] async fn path_open_dirfd_not_dir() { run("path_open_dirfd_not_dir", true).await.unwrap() } @@ -313,3 +317,7 @@ async fn symlink_loop() { async fn unlink_file_trailing_slashes() { run("unlink_file_trailing_slashes", true).await.unwrap() } +#[test_log::test(tokio::test(flavor = "multi_thread"))] +async fn path_open_preopen() { + run("path_open_preopen", true).await.unwrap() +} diff --git a/crates/test-programs/tests/wasi-preview2-components.rs b/crates/test-programs/tests/wasi-preview2-components.rs index 1983d7489cda..2b65ffaf718b 100644 --- a/crates/test-programs/tests/wasi-preview2-components.rs +++ b/crates/test-programs/tests/wasi-preview2-components.rs @@ -216,6 +216,10 @@ async fn path_open_create_existing() { run("path_open_create_existing", true).await.unwrap() } #[test_log::test(tokio::test(flavor = "multi_thread"))] +async fn path_open_read_write() { + run("path_open_read_write", true).await.unwrap() +} +#[test_log::test(tokio::test(flavor = "multi_thread"))] async fn path_open_dirfd_not_dir() { run("path_open_dirfd_not_dir", true).await.unwrap() } @@ -300,3 +304,7 @@ async fn symlink_loop() { async fn unlink_file_trailing_slashes() { run("unlink_file_trailing_slashes", true).await.unwrap() } +#[test_log::test(tokio::test(flavor = "multi_thread"))] +async fn path_open_preopen() { + run("path_open_preopen", true).await.unwrap() +} diff --git a/crates/test-programs/tests/wasi-tokio.rs b/crates/test-programs/tests/wasi-tokio.rs index 7875e90998c7..f14f273d0f2c 100644 --- a/crates/test-programs/tests/wasi-tokio.rs +++ b/crates/test-programs/tests/wasi-tokio.rs @@ -190,6 +190,10 @@ async fn path_open_create_existing() { run("path_open_create_existing", true).await.unwrap() } #[test_log::test(tokio::test(flavor = "multi_thread"))] +async fn path_open_read_write() { + run("path_open_read_write", true).await.unwrap() +} +#[test_log::test(tokio::test(flavor = "multi_thread"))] async fn path_open_dirfd_not_dir() { run("path_open_dirfd_not_dir", true).await.unwrap() } @@ -271,3 +275,7 @@ async fn symlink_loop() { async fn unlink_file_trailing_slashes() { run("unlink_file_trailing_slashes", true).await.unwrap() } +#[test_log::test(tokio::test(flavor = "multi_thread"))] +async fn path_open_preopen() { + run("path_open_preopen", true).await.unwrap() +} diff --git a/crates/test-programs/wasi-tests/src/bin/path_open_preopen.rs b/crates/test-programs/wasi-tests/src/bin/path_open_preopen.rs new file mode 100644 index 000000000000..f1e7a3f7cfba --- /dev/null +++ b/crates/test-programs/wasi-tests/src/bin/path_open_preopen.rs @@ -0,0 +1,92 @@ +const FIRST_PREOPEN: u32 = 3; + +unsafe fn path_open_preopen() { + let prestat = wasi::fd_prestat_get(FIRST_PREOPEN).expect("fd 3 is a preopen"); + assert_eq!( + prestat.tag, + wasi::PREOPENTYPE_DIR.raw(), + "prestat is a directory" + ); + let mut dst = Vec::with_capacity(prestat.u.dir.pr_name_len); + wasi::fd_prestat_dir_name(FIRST_PREOPEN, dst.as_mut_ptr(), dst.capacity()) + .expect("get preopen dir name"); + dst.set_len(prestat.u.dir.pr_name_len); + + let fdstat = wasi::fd_fdstat_get(FIRST_PREOPEN).expect("get fdstat"); + + println!( + "preopen dir: {:?} base {:?} inheriting {:?}", + String::from_utf8_lossy(&dst), + fdstat.fs_rights_base, + fdstat.fs_rights_inheriting + ); + + // Open with same rights it has now: + let _ = wasi::path_open( + FIRST_PREOPEN, + 0, + ".", + 0, + fdstat.fs_rights_base, + fdstat.fs_rights_inheriting, + 0, + ) + .expect("open with same rights"); + + // Open with an empty set of rights: + let _ = wasi::path_open(FIRST_PREOPEN, 0, ".", 0, 0, 0, 0).expect("open with empty rights"); + + // Open OFLAGS_DIRECTORY with an empty set of rights: + let _ = wasi::path_open(FIRST_PREOPEN, 0, ".", wasi::OFLAGS_DIRECTORY, 0, 0, 0) + .expect("open with O_DIRECTORY empty rights"); + + // Open OFLAGS_DIRECTORY with just the read right: + let _ = wasi::path_open( + FIRST_PREOPEN, + 0, + ".", + wasi::OFLAGS_DIRECTORY, + wasi::RIGHTS_FD_READ, + 0, + 0, + ) + .expect("open with O_DIRECTORY and read right"); + + if !wasi_tests::TESTCONFIG.errno_expect_windows() { + // Open OFLAGS_DIRECTORY and read/write rights should fail with isdir: + let err = wasi::path_open( + FIRST_PREOPEN, + 0, + ".", + wasi::OFLAGS_DIRECTORY, + wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE, + 0, + 0, + ) + .err() + .expect("open with O_DIRECTORY and read/write should fail"); + assert_eq!( + err, + wasi::ERRNO_ISDIR, + "opening directory read/write should fail with ISDIR" + ); + } else { + // Open OFLAGS_DIRECTORY and read/write rights will succeed, only on windows: + let _ = wasi::path_open( + FIRST_PREOPEN, + 0, + ".", + wasi::OFLAGS_DIRECTORY, + wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE, + 0, + 0, + ) + .expect("open with O_DIRECTORY and read/write should succeed on windows"); + } +} + +fn main() { + unsafe { + path_open_preopen(); + } +} diff --git a/crates/test-programs/wasi-tests/src/bin/path_open_read_write.rs b/crates/test-programs/wasi-tests/src/bin/path_open_read_write.rs new file mode 100644 index 000000000000..a432f50a92b2 --- /dev/null +++ b/crates/test-programs/wasi-tests/src/bin/path_open_read_write.rs @@ -0,0 +1,140 @@ +use std::{env, process}; +use wasi_tests::{assert_errno, create_file, open_scratch_directory}; + +unsafe fn test_path_open_read_write(dir_fd: wasi::Fd) { + create_file(dir_fd, "file"); + + let f_readonly = wasi::path_open(dir_fd, 0, "file", 0, wasi::RIGHTS_FD_READ, 0, 0) + .expect("open file readonly"); + + let stat = wasi::fd_fdstat_get(f_readonly).expect("get fdstat readonly"); + assert!( + stat.fs_rights_base & wasi::RIGHTS_FD_READ == wasi::RIGHTS_FD_READ, + "readonly has read right" + ); + assert!( + stat.fs_rights_base & wasi::RIGHTS_FD_WRITE == 0, + "readonly does not have write right" + ); + + let buffer = &mut [0u8; 100]; + let iovec = wasi::Iovec { + buf: buffer.as_mut_ptr(), + buf_len: buffer.len(), + }; + let nread = wasi::fd_read(f_readonly, &[iovec]).expect("reading readonly file"); + assert_eq!(nread, 0, "readonly file is empty"); + + let write_buffer = &[1u8; 50]; + let ciovec = wasi::Ciovec { + buf: write_buffer.as_ptr(), + buf_len: write_buffer.len(), + }; + assert_errno!( + wasi::fd_write(f_readonly, &[ciovec]) + .err() + .expect("read of writeonly fails"), + wasi::ERRNO_BADF + ); + + wasi::fd_close(f_readonly).expect("close readonly"); + drop(f_readonly); + + // =============== WRITE ONLY ================== + let f_writeonly = wasi::path_open(dir_fd, 0, "file", 0, wasi::RIGHTS_FD_WRITE, 0, 0) + .expect("open file writeonly"); + + let stat = wasi::fd_fdstat_get(f_writeonly).expect("get fdstat writeonly"); + assert!( + stat.fs_rights_base & wasi::RIGHTS_FD_READ == 0, + "writeonly does not have read right" + ); + assert!( + stat.fs_rights_base & wasi::RIGHTS_FD_WRITE == wasi::RIGHTS_FD_WRITE, + "writeonly has write right" + ); + + assert_errno!( + wasi::fd_read(f_writeonly, &[iovec]) + .err() + .expect("read of writeonly fails"), + wasi::ERRNO_BADF + ); + let bytes_written = wasi::fd_write(f_writeonly, &[ciovec]).expect("write to writeonly"); + assert_eq!(bytes_written, write_buffer.len()); + + wasi::fd_close(f_writeonly).expect("close writeonly"); + drop(f_writeonly); + + // ============== READ WRITE ======================= + + let f_readwrite = wasi::path_open( + dir_fd, + 0, + "file", + 0, + wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE, + 0, + 0, + ) + .expect("open file readwrite"); + let stat = wasi::fd_fdstat_get(f_readwrite).expect("get fdstat readwrite"); + assert!( + stat.fs_rights_base & wasi::RIGHTS_FD_READ == wasi::RIGHTS_FD_READ, + "readwrite has read right" + ); + assert!( + stat.fs_rights_base & wasi::RIGHTS_FD_WRITE == wasi::RIGHTS_FD_WRITE, + "readwrite has write right" + ); + + let nread = wasi::fd_read(f_readwrite, &[iovec]).expect("reading readwrite file"); + assert_eq!( + nread, + write_buffer.len(), + "readwrite file contains contents from writeonly open" + ); + + let write_buffer_2 = &[2u8; 25]; + let ciovec = wasi::Ciovec { + buf: write_buffer_2.as_ptr(), + buf_len: write_buffer_2.len(), + }; + let bytes_written = wasi::fd_write(f_readwrite, &[ciovec]).expect("write to readwrite"); + assert_eq!(bytes_written, write_buffer_2.len()); + + let filestat = wasi::fd_filestat_get(f_readwrite).expect("get filestat readwrite"); + assert_eq!( + filestat.size as usize, + write_buffer.len() + write_buffer_2.len(), + "total written is both write buffers" + ); + + wasi::fd_close(f_readwrite).expect("close readwrite"); + drop(f_readwrite); + + wasi::path_unlink_file(dir_fd, "file").expect("removing a file"); +} + +fn main() { + let mut args = env::args(); + let prog = args.next().unwrap(); + let arg = if let Some(arg) = args.next() { + arg + } else { + eprintln!("usage: {} ", prog); + process::exit(1); + }; + + // Open scratch directory + let dir_fd = match open_scratch_directory(&arg) { + Ok(dir_fd) => dir_fd, + Err(err) => { + eprintln!("{}", err); + process::exit(1) + } + }; + + // Run the tests. + unsafe { test_path_open_read_write(dir_fd) } +} diff --git a/crates/wasi-common/cap-std-sync/src/lib.rs b/crates/wasi-common/cap-std-sync/src/lib.rs index d26d25efb279..15a5bec35aa7 100644 --- a/crates/wasi-common/cap-std-sync/src/lib.rs +++ b/crates/wasi-common/cap-std-sync/src/lib.rs @@ -49,7 +49,7 @@ pub use sched::sched_ctx; use crate::net::Socket; use cap_rand::{Rng, RngCore, SeedableRng}; use std::path::Path; -use wasi_common::{table::Table, Error, WasiCtx, WasiFile}; +use wasi_common::{file::FileAccessMode, table::Table, Error, WasiCtx, WasiFile}; pub struct WasiCtxBuilder(WasiCtx); @@ -126,7 +126,8 @@ impl WasiCtxBuilder { pub fn preopened_socket(self, fd: u32, socket: impl Into) -> Result { let socket: Socket = socket.into(); let file: Box = socket.into(); - self.0.insert_file(fd, file); + self.0 + .insert_file(fd, file, FileAccessMode::READ | FileAccessMode::WRITE); Ok(self) } pub fn build(self) -> WasiCtx { diff --git a/crates/wasi-common/src/ctx.rs b/crates/wasi-common/src/ctx.rs index 4b6bc8f6392f..0d8d495dcfda 100644 --- a/crates/wasi-common/src/ctx.rs +++ b/crates/wasi-common/src/ctx.rs @@ -1,6 +1,6 @@ use crate::clocks::WasiClocks; use crate::dir::{DirEntry, WasiDir}; -use crate::file::{FileEntry, WasiFile}; +use crate::file::{FileAccessMode, FileEntry, WasiFile}; use crate::sched::WasiSched; use crate::string_array::StringArray; use crate::table::Table; @@ -51,12 +51,18 @@ impl WasiCtx { s } - pub fn insert_file(&self, fd: u32, file: Box) { - self.table().insert_at(fd, Arc::new(FileEntry::new(file))); + pub fn insert_file(&self, fd: u32, file: Box, access_mode: FileAccessMode) { + self.table() + .insert_at(fd, Arc::new(FileEntry::new(file, access_mode))); } - pub fn push_file(&self, file: Box) -> Result { - self.table().push(Arc::new(FileEntry::new(file))) + pub fn push_file( + &self, + file: Box, + access_mode: FileAccessMode, + ) -> Result { + self.table() + .push(Arc::new(FileEntry::new(file, access_mode))) } pub fn insert_dir(&self, fd: u32, dir: Box, path: PathBuf) { @@ -92,15 +98,15 @@ impl WasiCtx { } pub fn set_stdin(&self, f: Box) { - self.insert_file(0, f); + self.insert_file(0, f, FileAccessMode::READ); } pub fn set_stdout(&self, f: Box) { - self.insert_file(1, f); + self.insert_file(1, f, FileAccessMode::WRITE); } pub fn set_stderr(&self, f: Box) { - self.insert_file(2, f); + self.insert_file(2, f, FileAccessMode::WRITE); } pub fn push_preopened_dir( diff --git a/crates/wasi-common/src/file.rs b/crates/wasi-common/src/file.rs index 8086a4ea6928..6aaea63aeb5a 100644 --- a/crates/wasi-common/src/file.rs +++ b/crates/wasi-common/src/file.rs @@ -220,17 +220,26 @@ impl TableFileExt for crate::table::Table { pub(crate) struct FileEntry { pub file: Box, + pub access_mode: FileAccessMode, +} + +bitflags! { + pub struct FileAccessMode : u32 { + const READ = 0b1; + const WRITE= 0b10; + } } impl FileEntry { - pub fn new(file: Box) -> Self { - FileEntry { file } + pub fn new(file: Box, access_mode: FileAccessMode) -> Self { + FileEntry { file, access_mode } } pub async fn get_fdstat(&self) -> Result { Ok(FdStat { filetype: self.file.get_filetype().await?, flags: self.file.get_fdflags().await?, + access_mode: self.access_mode, }) } } @@ -239,6 +248,7 @@ impl FileEntry { pub struct FdStat { pub filetype: FileType, pub flags: FdFlags, + pub access_mode: FileAccessMode, } #[derive(Debug, Clone)] diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index 599d27830631..465af436da45 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -1,8 +1,8 @@ use crate::{ dir::{DirEntry, OpenResult, ReaddirCursor, ReaddirEntity, TableDirExt}, file::{ - Advice, FdFlags, FdStat, FileEntry, FileType, Filestat, OFlags, RiFlags, RoFlags, SdFlags, - SiFlags, TableFileExt, WasiFile, + Advice, FdFlags, FdStat, FileAccessMode, FileEntry, FileType, Filestat, OFlags, RiFlags, + RoFlags, SdFlags, SiFlags, TableFileExt, WasiFile, }, sched::{ subscription::{RwEventFlags, SubscriptionResult}, @@ -177,8 +177,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { let _dir_entry: Arc = table.get(fd)?; let dir_fdstat = types::Fdstat { fs_filetype: types::Filetype::Directory, - fs_rights_base: types::Rights::empty(), - fs_rights_inheriting: types::Rights::empty(), + fs_rights_base: directory_base_rights(), + fs_rights_inheriting: directory_inheriting_rights(), fs_flags: types::Fdflags::empty(), }; Ok(dir_fdstat) @@ -293,6 +293,10 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { iovs: &types::IovecArray<'a>, ) -> Result { let f = self.table().get_file(u32::from(fd))?; + // Access mode check normalizes error returned (windows would prefer ACCES here) + if !f.access_mode.contains(FileAccessMode::READ) { + Err(types::Errno::Badf)? + } let f = &f.file; let iovs: Vec> = iovs @@ -364,6 +368,10 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { offset: types::Filesize, ) -> Result { let f = self.table().get_file(u32::from(fd))?; + // Access mode check normalizes error returned (windows would prefer ACCES here) + if !f.access_mode.contains(FileAccessMode::READ) { + Err(types::Errno::Badf)? + } let f = &f.file; let iovs: Vec> = iovs @@ -436,6 +444,10 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { ciovs: &types::CiovecArray<'a>, ) -> Result { let f = self.table().get_file(u32::from(fd))?; + // Access mode check normalizes error returned (windows would prefer ACCES here) + if !f.access_mode.contains(FileAccessMode::WRITE) { + Err(types::Errno::Badf)? + } let f = &f.file; let guest_slices: Vec> = ciovs @@ -463,6 +475,10 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { offset: types::Filesize, ) -> Result { let f = self.table().get_file(u32::from(fd))?; + // Access mode check normalizes error returned (windows would prefer ACCES here) + if !f.access_mode.contains(FileAccessMode::WRITE) { + Err(types::Errno::Badf)? + } let f = &f.file; let guest_slices: Vec> = ciovs @@ -728,6 +744,16 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { let read = fs_rights_base.contains(types::Rights::FD_READ); let write = fs_rights_base.contains(types::Rights::FD_WRITE); + let access_mode = if read { + FileAccessMode::READ + } else { + FileAccessMode::empty() + } | if write { + FileAccessMode::WRITE + } else { + FileAccessMode::empty() + }; + let file = dir_entry .dir .open_file(symlink_follow, path.deref(), oflags, read, write, fdflags) @@ -735,7 +761,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { drop(dir_entry); let fd = match file { - OpenResult::File(file) => table.push(Arc::new(FileEntry::new(file)))?, + OpenResult::File(file) => table.push(Arc::new(FileEntry::new(file, access_mode)))?, OpenResult::Dir(child_dir) => table.push(Arc::new(DirEntry::new(None, child_dir)))?, }; Ok(types::Fd::from(fd)) @@ -1087,7 +1113,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx { let table = self.table(); let f = table.get_file(u32::from(fd))?; let file = f.file.sock_accept(FdFlags::from(flags)).await?; - let fd = table.push(Arc::new(FileEntry::new(file)))?; + let fd = table.push(Arc::new(FileEntry::new(file, FileAccessMode::all())))?; Ok(types::Fd::from(fd)) } @@ -1214,9 +1240,16 @@ impl From for Advice { impl From<&FdStat> for types::Fdstat { fn from(fdstat: &FdStat) -> types::Fdstat { + let mut fs_rights_base = types::Rights::empty(); + if fdstat.access_mode.contains(FileAccessMode::READ) { + fs_rights_base |= types::Rights::FD_READ; + } + if fdstat.access_mode.contains(FileAccessMode::WRITE) { + fs_rights_base |= types::Rights::FD_WRITE; + } types::Fdstat { fs_filetype: types::Filetype::from(&fdstat.filetype), - fs_rights_base: types::Rights::empty(), + fs_rights_base, fs_rights_inheriting: types::Rights::empty(), fs_flags: types::Fdflags::from(fdstat.flags), } @@ -1413,3 +1446,45 @@ fn systimespec( Ok(None) } } + +// This is the default subset of base Rights reported for directories prior to +// https://github.com/bytecodealliance/wasmtime/pull/6265. Some +// implementations still expect this set of rights to be reported. +pub(crate) fn directory_base_rights() -> types::Rights { + types::Rights::PATH_CREATE_DIRECTORY + | types::Rights::PATH_CREATE_FILE + | types::Rights::PATH_LINK_SOURCE + | types::Rights::PATH_LINK_TARGET + | types::Rights::PATH_OPEN + | types::Rights::FD_READDIR + | types::Rights::PATH_READLINK + | types::Rights::PATH_RENAME_SOURCE + | types::Rights::PATH_RENAME_TARGET + | types::Rights::PATH_SYMLINK + | types::Rights::PATH_REMOVE_DIRECTORY + | types::Rights::PATH_UNLINK_FILE + | types::Rights::PATH_FILESTAT_GET + | types::Rights::PATH_FILESTAT_SET_TIMES + | types::Rights::FD_FILESTAT_GET + | types::Rights::FD_FILESTAT_SET_TIMES +} + +// This is the default subset of inheriting Rights reported for directories +// prior to https://github.com/bytecodealliance/wasmtime/pull/6265. Some +// implementations still expect this set of rights to be reported. +pub(crate) fn directory_inheriting_rights() -> types::Rights { + types::Rights::FD_DATASYNC + | types::Rights::FD_READ + | types::Rights::FD_SEEK + | types::Rights::FD_FDSTAT_SET_FLAGS + | types::Rights::FD_SYNC + | types::Rights::FD_TELL + | types::Rights::FD_WRITE + | types::Rights::FD_ADVISE + | types::Rights::FD_ALLOCATE + | types::Rights::FD_FILESTAT_GET + | types::Rights::FD_FILESTAT_SET_SIZE + | types::Rights::FD_FILESTAT_SET_TIMES + | types::Rights::POLL_FD_READWRITE + | directory_base_rights() +} diff --git a/crates/wasi-common/tokio/src/lib.rs b/crates/wasi-common/tokio/src/lib.rs index 5a17b17816bb..250aaabb4f7f 100644 --- a/crates/wasi-common/tokio/src/lib.rs +++ b/crates/wasi-common/tokio/src/lib.rs @@ -9,7 +9,7 @@ pub mod stdio; use std::future::Future; use std::path::Path; pub use wasi_cap_std_sync::{clocks_ctx, random_ctx}; -use wasi_common::{Error, Table, WasiCtx, WasiFile}; +use wasi_common::{file::FileAccessMode, Error, Table, WasiCtx, WasiFile}; use crate::sched::sched_ctx; pub use dir::Dir; @@ -96,7 +96,8 @@ impl WasiCtxBuilder { pub fn preopened_socket(self, fd: u32, socket: impl Into) -> Result { let socket: Socket = socket.into(); let file: Box = socket.into(); - self.0.insert_file(fd, file); + self.0 + .insert_file(fd, file, FileAccessMode::READ | FileAccessMode::WRITE); Ok(self) } diff --git a/crates/wasi-preview1-component-adapter/src/descriptors.rs b/crates/wasi-preview1-component-adapter/src/descriptors.rs index cdeff86082b5..de8b3a13cf7f 100644 --- a/crates/wasi-preview1-component-adapter/src/descriptors.rs +++ b/crates/wasi-preview1-component-adapter/src/descriptors.rs @@ -69,7 +69,7 @@ impl Streams { // For files, we may have adjusted the position for seeking, so // create a new stream. StreamType::File(file) => { - let input = filesystem::read_via_stream(file.fd, file.position.get()); + let input = filesystem::read_via_stream(file.fd, file.position.get())?; self.input.set(Some(input)); Ok(input) } @@ -93,9 +93,9 @@ impl Streams { // create a new stream. StreamType::File(file) => { let output = if file.append { - filesystem::append_via_stream(file.fd) + filesystem::append_via_stream(file.fd)? } else { - filesystem::write_via_stream(file.fd, file.position.get()) + filesystem::write_via_stream(file.fd, file.position.get())? }; self.output.set(Some(output)); Ok(output) diff --git a/crates/wasi/src/preview2/preview1/mod.rs b/crates/wasi/src/preview2/preview1/mod.rs index 9040425acf33..af46bf27f496 100644 --- a/crates/wasi/src/preview2/preview1/mod.rs +++ b/crates/wasi/src/preview2/preview1/mod.rs @@ -1081,11 +1081,11 @@ impl< }; let pos = position.load(Ordering::Relaxed); - let stream = self - .read_via_stream(fd, pos) - .await - .context("failed to call `read-via-stream`") - .map_err(types::Error::trap)?; + let stream = self.read_via_stream(fd, pos).await.map_err(|e| { + e.try_into() + .context("failed to call `read-via-stream`") + .unwrap_or_else(types::Error::trap) + })?; let max = buf.len().try_into().unwrap_or(u64::MAX); let (read, end) = if blocking { self.blocking_read(stream, max) @@ -1141,11 +1141,11 @@ impl< return Ok(0) }; - let stream = self - .read_via_stream(fd, offset) - .await - .context("failed to call `read-via-stream`") - .map_err(types::Error::trap)?; + let stream = self.read_via_stream(fd, offset).await.map_err(|e| { + e.try_into() + .context("failed to call `read-via-stream`") + .unwrap_or_else(types::Error::trap) + })?; let max = buf.len().try_into().unwrap_or(u64::MAX); let (read, end) = if blocking { self.blocking_read(stream, max) @@ -1195,19 +1195,19 @@ impl< return Ok(0) }; let (stream, pos) = if append { - let stream = self - .append_via_stream(fd) - .await - .context("failed to call `append-via-stream`") - .map_err(types::Error::trap)?; + let stream = self.append_via_stream(fd).await.map_err(|e| { + e.try_into() + .context("failed to call `append-via-stream`") + .unwrap_or_else(types::Error::trap) + })?; (stream, 0) } else { let position = position.load(Ordering::Relaxed); - let stream = self - .write_via_stream(fd, position) - .await - .context("failed to call `write-via-stream`") - .map_err(types::Error::trap)?; + let stream = self.write_via_stream(fd, position).await.map_err(|e| { + e.try_into() + .context("failed to call `write-via-stream`") + .unwrap_or_else(types::Error::trap) + })?; (stream, position) }; let n = if blocking { @@ -1253,11 +1253,11 @@ impl< let Some(buf) = first_non_empty_ciovec(ciovs)? else { return Ok(0) }; - let stream = self - .write_via_stream(fd, offset) - .await - .context("failed to call `write-via-stream`") - .map_err(types::Error::trap)?; + let stream = self.write_via_stream(fd, offset).await.map_err(|e| { + e.try_into() + .context("failed to call `write-via-stream`") + .unwrap_or_else(types::Error::trap) + })?; if blocking { self.blocking_write(stream, buf) } else { diff --git a/crates/wasi/src/preview2/preview2/filesystem.rs b/crates/wasi/src/preview2/preview2/filesystem.rs index 73c572314b7c..a2e3135bed25 100644 --- a/crates/wasi/src/preview2/preview2/filesystem.rs +++ b/crates/wasi/src/preview2/preview2/filesystem.rs @@ -352,16 +352,12 @@ impl filesystem::Host for T { let table = self.table(); if table.is_file(fd) { let f = table.get_file(fd)?; - if !f.perms.contains(FilePerms::READ) { - return Err(ErrorCode::NotPermitted.into()); - } + // No permissions check on stat: if opened, allowed to stat it let meta = f.file.metadata()?; Ok(descriptorstat_from(meta)) } else if table.is_dir(fd) { let d = table.get_dir(fd)?; - if !d.perms.contains(DirPerms::READ) { - return Err(ErrorCode::NotPermitted.into()); - } + // No permissions check on stat: if opened, allowed to stat it let meta = d.dir.dir_metadata()?; Ok(descriptorstat_from(meta)) } else { @@ -541,7 +537,7 @@ impl filesystem::Host for T { let set_fd_flags = opened.new_set_fd_flags(FdFlags::NONBLOCK)?; opened.set_fd_flags(set_fd_flags)?; - Ok(table.push_file(File::new(opened, d.file_perms))?) + Ok(table.push_file(File::new(opened, mask_file_perms(d.file_perms, flags)))?) } } @@ -701,13 +697,13 @@ impl filesystem::Host for T { &mut self, fd: filesystem::Descriptor, offset: filesystem::Filesize, - ) -> anyhow::Result { - // FIXME: this skips the perm check. We can't return a NotPermitted - // error code here. Do we need to change the interface? - + ) -> Result { // Trap if fd lookup fails: let f = self.table().get_file(fd)?; + if !f.perms.contains(FilePerms::READ) { + Err(filesystem::ErrorCode::BadDescriptor)?; + } // Duplicate the file descriptor so that we get an indepenent lifetime. let clone = std::sync::Arc::clone(&f.file); @@ -724,10 +720,14 @@ impl filesystem::Host for T { &mut self, fd: filesystem::Descriptor, offset: filesystem::Filesize, - ) -> anyhow::Result { + ) -> Result { // Trap if fd lookup fails: let f = self.table().get_file(fd)?; + if !f.perms.contains(FilePerms::WRITE) { + Err(filesystem::ErrorCode::BadDescriptor)?; + } + // Duplicate the file descriptor so that we get an indepenent lifetime. let clone = std::sync::Arc::clone(&f.file); @@ -743,10 +743,13 @@ impl filesystem::Host for T { async fn append_via_stream( &mut self, fd: filesystem::Descriptor, - ) -> anyhow::Result { + ) -> Result { // Trap if fd lookup fails: let f = self.table().get_file(fd)?; + if !f.perms.contains(FilePerms::WRITE) { + Err(filesystem::ErrorCode::BadDescriptor)?; + } // Duplicate the file descriptor so that we get an indepenent lifetime. let clone = std::sync::Arc::clone(&f.file); @@ -988,6 +991,19 @@ impl TableReaddirExt for Table { self.get(fd) } } + +fn mask_file_perms(p: FilePerms, flags: filesystem::DescriptorFlags) -> FilePerms { + use filesystem::DescriptorFlags; + let mut out = FilePerms::empty(); + if p.contains(FilePerms::READ) && flags.contains(DescriptorFlags::READ) { + out |= FilePerms::READ; + } + if p.contains(FilePerms::WRITE) && flags.contains(DescriptorFlags::WRITE) { + out |= FilePerms::WRITE; + } + out +} + #[cfg(test)] mod test { use super::*; diff --git a/crates/wasi/wit/deps/filesystem/filesystem.wit b/crates/wasi/wit/deps/filesystem/filesystem.wit index 2477264a7367..930f37567044 100644 --- a/crates/wasi/wit/deps/filesystem/filesystem.wit +++ b/crates/wasi/wit/deps/filesystem/filesystem.wit @@ -311,30 +311,30 @@ interface filesystem { /// Multiple read, write, and append streams may be active on the same open /// file and they do not interfere with each other. /// - /// Note: This allows using `read-stream`, which is similar to `read` in POSIX. + /// Note: This allows using `wasi:io/streams.read`, which is similar to `read` in POSIX. read-via-stream: func( this: descriptor, /// The offset within the file at which to start reading. offset: filesize, - ) -> input-stream + ) -> result /// Return a stream for writing to a file. /// - /// Note: This allows using `write-stream`, which is similar to `write` in + /// Note: This allows using `wasi:io/streams.write`, which is similar to `write` in /// POSIX. write-via-stream: func( this: descriptor, /// The offset within the file at which to start writing. offset: filesize, - ) -> output-stream + ) -> result /// Return a stream for appending to a file. /// - /// Note: This allows using `write-stream`, which is similar to `write` with + /// Note: This allows using `wasi:io/streams.write`, which is similar to `write` with /// `O_APPEND` in in POSIX. append-via-stream: func( this: descriptor, - ) -> output-stream + ) -> result /// Provide file advisory information on a descriptor. ///