sausage-vfs: review fixes (fallible growth, symlink flags, stream cancel, sized reads, timestamps, error kinds, linker shadowing)

API: Layer::stat(path, follow), Layer::readlink, Open::follow,
Stat::symlink and Entry::symlink. Process::idle documents the shutdown
order: abort, await the task, await idle, release locks.

vfs/tests/guests/files/src/main.rs

@@ -9,6 +9,11 @@ use std::{
 fn main() {
     match std::env::var("VFS_TOPIC").unwrap().as_str() {
         "basic" => basic(),
+        "review_streams" => review_streams(),
+        "review_times" => review_times(),
+        "review_paths" => review_paths(),
+        "review_links" => review_links(),
+        "review_preopen" => review_preopen(),
         "trees" => trees(),
         "options" => options(),
         "rename" => rename(),
@@ -255,3 +260,121 @@ fn temporary() {
     assert!(!original.exists());
     assert_eq!(fs::read("/persisted").unwrap(), b"persisted");
 }
+
+/// Dropped output streams finish writes and small reads do not cache later bytes.
+fn review_streams() {
+    use wasip2::filesystem::{
+        preopens,
+        types::{DescriptorFlags, OpenFlags, PathFlags},
+    };
+    let root = preopens::get_directories().remove(0).0;
+    let file = root
+        .open_at(
+            PathFlags::empty(),
+            "stream-review",
+            OpenFlags::CREATE,
+            DescriptorFlags::READ | DescriptorFlags::WRITE,
+        )
+        .unwrap();
+    let output = file.write_via_stream(0).unwrap();
+    assert!(output.check_write().unwrap() > 0);
+    output.write(b"abcd").unwrap();
+    drop(output);
+    assert_eq!(fs::read("/stream-review").unwrap(), b"abcd");
+    let input = file.read_via_stream(0).unwrap();
+    assert_eq!(input.blocking_read(1).unwrap(), b"a");
+    file.write_via_stream(1)
+        .unwrap()
+        .blocking_write_and_flush(b"Z")
+        .unwrap();
+    assert_eq!(input.blocking_read(1).unwrap(), b"Z");
+}
+/// Timestamp operations accept directories and return overflow rather than panicking.
+fn review_times() {
+    use wasip2::clocks::wall_clock::Datetime;
+    use wasip2::filesystem::{
+        preopens,
+        types::{ErrorCode, NewTimestamp, PathFlags},
+    };
+    let root = preopens::get_directories().remove(0).0;
+    fs::create_dir("/times").unwrap();
+    root.set_times_at(
+        PathFlags::SYMLINK_FOLLOW,
+        "times",
+        NewTimestamp::NoChange,
+        NewTimestamp::Timestamp(Datetime {
+            seconds: 123,
+            nanoseconds: 0,
+        }),
+    )
+    .unwrap();
+    assert_eq!(
+        fs::metadata("/times").unwrap().modified().unwrap(),
+        std::time::UNIX_EPOCH + std::time::Duration::from_secs(123)
+    );
+    assert_eq!(
+        root.set_times(
+            NewTimestamp::Timestamp(Datetime {
+                seconds: u64::MAX,
+                nanoseconds: u32::MAX
+            }),
+            NewTimestamp::NoChange
+        ),
+        Err(ErrorCode::Overflow)
+    );
+}
+/// Trailing slashes require folders and readlink rejects ordinary files with invalid.
+fn review_paths() {
+    use wasip2::filesystem::{preopens, types::ErrorCode};
+    fs::write("/file", b"x").unwrap();
+    assert_eq!(
+        fs::read("/file/").unwrap_err().kind(),
+        ErrorKind::NotADirectory
+    );
+    let root = preopens::get_directories().remove(0).0;
+    assert_eq!(root.readlink_at("file"), Err(ErrorCode::Invalid));
+    assert_eq!(root.remove_directory_at("run"), Err(ErrorCode::Busy));
+}
+/// Removing an internal link leaves its target intact and external links cannot escape.
+fn review_links() {
+    use wasip2::filesystem::{
+        preopens,
+        types::{DescriptorFlags, DescriptorType, OpenFlags, PathFlags},
+    };
+    let root = preopens::get_directories().remove(0).0;
+    assert_eq!(
+        root.stat_at(PathFlags::empty(), "link").unwrap().type_,
+        DescriptorType::SymbolicLink
+    );
+    assert_eq!(root.readlink_at("link").unwrap(), "target");
+    assert!(
+        root.open_at(
+            PathFlags::empty(),
+            "link",
+            OpenFlags::DIRECTORY,
+            DescriptorFlags::READ
+        )
+        .is_err()
+    );
+    assert_eq!(
+        root.open_at(
+            PathFlags::SYMLINK_FOLLOW,
+            "link",
+            OpenFlags::DIRECTORY,
+            DescriptorFlags::READ
+        )
+        .unwrap()
+        .get_type()
+        .unwrap(),
+        DescriptorType::Directory
+    );
+    fs::remove_dir_all("/link").unwrap();
+    assert_eq!(fs::read("/target/keep").unwrap(), b"keep");
+    assert!(fs::read("/outside-link/secret").is_err());
+}
+/// The component sees exactly one virtual preopen rather than native WASI preopens.
+fn review_preopen() {
+    let dirs = wasip2::filesystem::preopens::get_directories();
+    assert_eq!(dirs.len(), 1);
+    assert_eq!(dirs[0].1, "/");
+}

vfs/tests/layers.proptest-regressions

@@ -0,0 +1,7 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc 86f33ba33c9e3b009f9a580dbcd9a669c783a070e032d497bd480de556b48220 # shrinks to ops = [Operation { kind: 0, from: 3, to: 0, bytes: [] }, Operation { kind: 4, from: 3, to: 1, bytes: [] }]

vfs/tests/layers.rs

@@ -283,7 +283,7 @@ async fn apply_layer(layer: &dyn Layer, op: &Operation) -> std::io::Result<()> {
         2 => layer.mkdir(path).await,
         3 => layer.remove_dir(path).await,
         4 => layer.rename(path, PATHS[op.to]).await,
-        8 => layer.stat(path).await.map(|_| ()),
+        8 => layer.stat(path, true).await.map(|_| ()),
         _ => {
             let file = layer
                 .open(
@@ -330,7 +330,7 @@ fn apply_std(root: &std::path::Path, op: &Operation) -> std::io::Result<()> {
     }
 }
 async fn compare_tree(
-    host: &Host,
+    host: &dyn Layer,
     root: &std::path::Path,
 ) -> std::result::Result<(), TestCaseError> {
     let mut pending = vec!["/".to_string()];
@@ -343,6 +343,7 @@ async fn compare_tree(
                 Entry {
                     name: entry.file_name().to_str().unwrap().into(),
                     directory: entry.file_type().unwrap().is_dir(),
+                    symlink: false,
                 }
             })
             .collect::<Vec<_>>();
@@ -350,7 +351,7 @@ async fn compare_tree(
         prop_assert_eq!(host.list(&path).await.unwrap(), entries.clone());
         for entry in entries {
             let child = resolve(&path, &entry.name);
-            let stat = host.stat(&child).await.unwrap();
+            let stat = host.stat(&child, true).await.unwrap();
             prop_assert_eq!(stat.directory, entry.directory);
             if entry.directory {
                 pending.push(child);
@@ -371,25 +372,53 @@ async fn compare_tree(
     }
     Ok(())
 }
-async fn check_host_differential(ops: Vec<Operation>) -> std::result::Result<(), TestCaseError> {
+async fn check_differential(
+    ops: Vec<Operation>,
+    memory: bool,
+) -> std::result::Result<(), TestCaseError> {
     let actual = tempfile::tempdir().unwrap();
     let expected = tempfile::tempdir().unwrap();
-    let host = Host::new(actual.path(), Process::default()).unwrap();
-    for root in [actual.path(), expected.path()] {
+    let host: Arc<dyn Layer> = if memory {
+        Arc::new(Memory::default())
+    } else {
+        Arc::new(Host::new(actual.path(), Process::default()).unwrap())
+    };
+    {
+        let root = expected.path();
         std::fs::create_dir_all(root.join("a/nested")).unwrap();
         std::fs::create_dir_all(root.join("b/nested")).unwrap();
         std::fs::write(root.join("a/child"), b"source").unwrap();
         std::fs::write(root.join("target"), b"destination").unwrap();
     }
+    host.mkdir("/a").await.unwrap();
+    host.mkdir("/a/nested").await.unwrap();
+    host.mkdir("/b").await.unwrap();
+    host.mkdir("/b/nested").await.unwrap();
+    host.open("/a/child", rw())
+        .await
+        .unwrap()
+        .write(0, b"source".to_vec())
+        .await
+        .unwrap();
+    host.open("/target", rw())
+        .await
+        .unwrap()
+        .write(0, b"destination".to_vec())
+        .await
+        .unwrap();
     for op in ops {
-        let actual = apply_layer(&host, &op).await.map_err(|e| e.kind());
+        let actual = apply_layer(host.as_ref(), &op).await.map_err(|e| e.kind());
         let expected_result = apply_std(expected.path(), &op).map_err(|e| e.kind());
         prop_assert_eq!(actual, expected_result, "operation: {:?}", op);
-        compare_tree(&host, expected.path()).await?;
+        compare_tree(host.as_ref(), expected.path()).await?;
     }
     Ok(())
 }
 
+async fn check_host_differential(ops: Vec<Operation>) -> std::result::Result<(), TestCaseError> {
+    check_differential(ops, false).await
+}
+
 /// Nested renames replace existing files and empty folders just like std::fs.
 #[tokio::test]
 async fn nested_rename_replacements() {
@@ -416,6 +445,12 @@ proptest! {
         tokio::runtime::Runtime::new().unwrap().block_on(check_memory_model(ops))?;
     }
 
+    /// Random memory namespace operations match std::fs and the complete tree.
+    #[test]
+    fn memory_differential(ops in operations()) {
+        tokio::runtime::Runtime::new().unwrap().block_on(check_differential(ops, true))?;
+    }
+
     /// Random nested operations match std::fs results and the complete tree.
     #[test]
     fn host_differential(ops in operations()) {

vfs/tests/regressions.rs

@@ -0,0 +1,92 @@
+//! Regression tests for review findings about memory files and process shutdown.
+use sausage_vfs::*;
+use std::{
+    io::ErrorKind,
+    time::{Duration, SystemTime},
+};
+fn rw() -> Open {
+    Open {
+        write: true,
+        create: true,
+        ..Open::default()
+    }
+}
+/// A capacity overflow returns an error without poisoning the shared memory tree.
+#[tokio::test]
+async fn memory_capacity_failure() {
+    let memory = Memory::default();
+    let file = memory.open("/file", rw()).await.unwrap();
+    let copy = file.clone();
+    let result =
+        tokio::spawn(async move { copy.write(i64::MAX as u64 - 10, vec![0; 32]).await }).await;
+    assert_eq!(result.unwrap().unwrap_err().kind(), ErrorKind::FileTooLarge);
+    assert_eq!(
+        file.write(1u64 << 62, vec![1]).await.unwrap_err().kind(),
+        ErrorKind::FileTooLarge
+    );
+    assert_eq!(
+        file.set_size(1u64 << 62).await.unwrap_err().kind(),
+        ErrorKind::FileTooLarge
+    );
+    file.write(0, b"usable".to_vec()).await.unwrap();
+    assert_eq!(memory.stat("/file", true).await.unwrap().size, 6);
+    memory.mkdir("/still-usable").await.unwrap();
+}
+/// Memory metadata distinguishes a missing parent from a regular-file parent.
+#[tokio::test]
+async fn memory_parent_errors() {
+    let memory = Memory::default();
+    memory.open("/file", rw()).await.unwrap();
+    assert_eq!(
+        memory.stat("/file/child", true).await.unwrap_err().kind(),
+        ErrorKind::NotADirectory
+    );
+    assert_eq!(
+        memory
+            .open("/file/child", Open::default())
+            .await
+            .err()
+            .unwrap()
+            .kind(),
+        ErrorKind::NotADirectory
+    );
+}
+/// Memory writes update modification time and read handles can set timestamps.
+#[tokio::test]
+async fn memory_timestamps() {
+    let memory = Memory::default();
+    let file = memory.open("/file", rw()).await.unwrap();
+    file.write(0, vec![1]).await.unwrap();
+    assert!(file.stat().await.unwrap().modified.is_some());
+    let read = memory.open("/file", Open::default()).await.unwrap();
+    let time = SystemTime::UNIX_EPOCH + Duration::from_secs(123);
+    read.set_times(None, Some(time)).await.unwrap();
+    assert_eq!(read.stat().await.unwrap().modified, Some(time));
+}
+/// Awaiting an aborted writer before idle prevents writes after lock release.
+#[tokio::test(flavor = "multi_thread")]
+async fn abort_join_then_idle() {
+    let temp = tempfile::tempdir().unwrap();
+    let process = Process::default();
+    let host = Host::new(temp.path(), process.clone()).unwrap();
+    let file = host.open("/file", rw()).await.unwrap();
+    let (started_tx, started_rx) = tokio::sync::oneshot::channel();
+    let copy = file.clone();
+    let task = tokio::spawn(async move {
+        copy.append(vec![1]).await.unwrap();
+        started_tx.send(()).unwrap();
+        loop {
+            copy.append(vec![1]).await.unwrap();
+        }
+    });
+    started_rx.await.unwrap();
+    task.abort();
+    assert!(task.await.unwrap_err().is_cancelled());
+    process.idle().await;
+    let before = file.stat().await.unwrap().size;
+    assert!(before >= 1);
+    for _ in 0..100 {
+        tokio::task::yield_now().await;
+        assert_eq!(file.stat().await.unwrap().size, before);
+    }
+}

vfs/tests/wasi.rs

@@ -55,6 +55,14 @@ async fn run(topic: &str, shared: Arc<Memory>) {
     let (engine, component) = guest();
     let world = tempfile::tempdir().unwrap();
     std::fs::create_dir(world.path().join("tmp")).unwrap();
+    let outside = tempfile::tempdir().unwrap();
+    if topic == "review_links" {
+        std::fs::create_dir(world.path().join("target")).unwrap();
+        std::fs::write(world.path().join("target/keep"), b"keep").unwrap();
+        std::fs::write(outside.path().join("secret"), b"secret").unwrap();
+        std::os::unix::fs::symlink("target", world.path().join("link")).unwrap();
+        std::os::unix::fs::symlink(outside.path(), world.path().join("outside-link")).unwrap();
+    }
     let mut mounts = Mounts::default();
     mounts.mount(
         "/",
@@ -73,13 +81,20 @@ async fn run(topic: &str, shared: Arc<Memory>) {
         table: &mut s.table,
     })
     .unwrap();
+    let mut wasi = WasiCtxBuilder::new();
+    wasi.inherit_stdio().env("VFS_TOPIC", topic);
+    if topic == "review_preopen" {
+        wasi.preopened_dir(
+            world.path(),
+            "/native",
+            wasmtime_wasi::filesystem::FsPerms::ReadWrite,
+        )
+        .unwrap();
+    }
     let mut store = Store::new(
         engine,
         State {
-            wasi: WasiCtxBuilder::new()
-                .inherit_stdio()
-                .env("VFS_TOPIC", topic)
-                .build(),
+            wasi: wasi.build(),
             table: ResourceTable::new(),
             fs: Adapter::new(mounts),
         },
@@ -150,3 +165,47 @@ async fn sqlite_transactions() {
 async fn named_temporary_files() {
     isolated("tempfile").await;
 }
+
+/// Stream drop preserves writes and small reads observe later mutations.
+#[tokio::test(flavor = "multi_thread")]
+async fn review_streams() {
+    isolated("review_streams").await;
+}
+
+/// Timestamp conversion reports overflow and accepts directories.
+#[tokio::test(flavor = "multi_thread")]
+async fn review_times() {
+    isolated("review_times").await;
+}
+
+/// Path errors distinguish trailing slashes, busy roots, and ordinary files.
+#[tokio::test(flavor = "multi_thread")]
+async fn review_paths() {
+    isolated("review_paths").await;
+}
+
+/// Symlink traversal does not delete targets or escape the host root.
+#[tokio::test(flavor = "multi_thread")]
+async fn review_links() {
+    isolated("review_links").await;
+}
+
+/// The guest sees only the virtual root preopen.
+#[tokio::test(flavor = "multi_thread")]
+async fn review_preopen() {
+    isolated("review_preopen").await;
+}
+
+/// Adding the adapter restores duplicate-import errors on the caller's linker.
+#[test]
+fn linker_shadowing_is_reset() {
+    let engine = Engine::default();
+    let mut linker = Linker::<State>::new(&engine);
+    wasmtime_wasi::p2::add_to_linker_async(&mut linker).unwrap();
+    adapter::add_to_linker(&mut linker, |s| View {
+        adapter: &mut s.fs,
+        table: &mut s.table,
+    })
+    .unwrap();
+    assert!(wasmtime_wasi::p2::add_to_linker_async(&mut linker).is_err());
+}