From 5a5a32e2179eddc3e233176aa66f75d1c26e3efc Mon Sep 17 00:00:00 2001 From: danbugs Date: Fri, 15 May 2026 05:35:43 +0000 Subject: [PATCH 1/5] refactor: remove FsSandbox::register duplication, use FsRouter in tests Signed-off-by: danbugs --- host/src/lib.rs | 221 ++---------------------------------------------- 1 file changed, 7 insertions(+), 214 deletions(-) diff --git a/host/src/lib.rs b/host/src/lib.rs index 5dd822d..51a1e6c 100644 --- a/host/src/lib.rs +++ b/host/src/lib.rs @@ -831,215 +831,6 @@ impl FsSandbox { Ok(out) } - /// Register all FS tool handlers on `registry`: - /// - /// - `fs_read` / `fs_write` — UTF-8 text read/write (whole-file). - /// - `fs_read_bytes` / `fs_write_bytes` — binary read/write with - /// optional offset/length/append, base64-encoded payloads. - /// - `fs_list` — directory enumeration as `{name, is_dir, is_file, is_symlink}`. - /// - `fs_stat` — size + file/dir metadata. - /// - `fs_mkdir` / `fs_unlink` — create/remove directory or file. - /// - `fs_truncate` — set file length. - /// - /// Every handler resolves its `path` argument under [`root`](Self::root) - /// via `FsSandbox::resolve`, which rejects `..` escapes, absolute - /// paths that climb outside the root, and symlinks pointing outside. - /// - /// The handlers call through `std::fs`, which behaves differently on - /// Linux and Windows — `normalize_fs_error` smooths out the error - /// wording before responses go back to the guest, so the Unikraft - /// guest's substring-matching classifier works on both hosts. - pub fn register(self, registry: &mut ToolRegistry) { - use serde_json::json; - - let s = self.clone(); - registry.register("fs_read", move |args| { - let path = args["path"] - .as_str() - .ok_or_else(|| anyhow!("fs_read: missing 'path'"))?; - let target = s.resolve(path)?; - let text = std::fs::read_to_string(&target) - .map_err(|e| anyhow!("fs_read {:?}: {}", path, e))?; - Ok(json!({ "text": text })) - }); - - let s = self.clone(); - registry.register("fs_write", move |args| { - let path = args["path"] - .as_str() - .ok_or_else(|| anyhow!("fs_write: missing 'path'"))?; - let text = args["text"] - .as_str() - .ok_or_else(|| anyhow!("fs_write: missing 'text'"))?; - let append = args["append"].as_bool().unwrap_or(false); - let target = s.resolve(path)?; - // Create parent dirs? No — guest must fs_mkdir explicitly. - use std::io::Write; - let mut f = std::fs::OpenOptions::new() - .write(true) - .create(true) - .truncate(!append) - .append(append) - .open(&target) - .map_err(|e| anyhow!("fs_write {:?}: {}", path, e))?; - f.write_all(text.as_bytes()) - .map_err(|e| anyhow!("fs_write {:?}: {}", path, e))?; - Ok(json!({ "bytes_written": text.len() })) - }); - - let s = self.clone(); - registry.register("fs_list", move |args| { - let path = args["path"].as_str().unwrap_or(""); - let target = s.resolve(path)?; - let mut entries = Vec::new(); - for entry in - std::fs::read_dir(&target).map_err(|e| anyhow!("fs_list {:?}: {}", path, e))? - { - let entry = entry?; - let name = entry.file_name().to_string_lossy().into_owned(); - let ft = entry.file_type()?; - entries.push(json!({ - "name": name, - "is_dir": ft.is_dir(), - "is_file": ft.is_file(), - "is_symlink": ft.is_symlink(), - })); - } - Ok(json!({ "entries": entries })) - }); - - let s = self.clone(); - registry.register("fs_stat", move |args| { - let path = args["path"] - .as_str() - .ok_or_else(|| anyhow!("fs_stat: missing 'path'"))?; - let target = s.resolve(path)?; - let md = - std::fs::metadata(&target).map_err(|e| anyhow!("fs_stat {:?}: {}", path, e))?; - Ok(json!({ - "size": md.len(), - "is_dir": md.is_dir(), - "is_file": md.is_file(), - })) - }); - - let s = self.clone(); - registry.register("fs_mkdir", move |args| { - let path = args["path"] - .as_str() - .ok_or_else(|| anyhow!("fs_mkdir: missing 'path'"))?; - let parents = args["parents"].as_bool().unwrap_or(false); - let target = s.resolve(path)?; - if parents { - std::fs::create_dir_all(&target) - } else { - std::fs::create_dir(&target) - } - .map_err(|e| anyhow!("fs_mkdir {:?}: {}", path, e))?; - Ok(json!({})) - }); - - // fs_read_bytes / fs_write_bytes — binary variants for the Phase B - // transparent POSIX shim. Bytes are base64-encoded in the JSON - // payload so arbitrary binary content round-trips intact. - // - // fs_read_bytes args: { path, offset?, len? } → { data: "", eof: bool } - // fs_write_bytes args: { path, data: "", offset?, append? } → { bytes_written } - let s = self.clone(); - registry.register("fs_read_bytes", move |args| { - use base64::Engine; - use std::io::{Read, Seek, SeekFrom}; - let path = args["path"] - .as_str() - .ok_or_else(|| anyhow!("fs_read_bytes: missing 'path'"))?; - let offset = args["offset"].as_u64().unwrap_or(0); - let want = args["len"].as_u64().unwrap_or(65536).min(MAX_FS_READ); - let target = s.resolve(path)?; - let mut f = std::fs::File::open(&target) - .map_err(|e| anyhow!("fs_read_bytes {:?}: {}", path, e))?; - if offset > 0 { - f.seek(SeekFrom::Start(offset)) - .map_err(|e| anyhow!("fs_read_bytes seek {:?}: {}", path, e))?; - } - let mut buf = vec![0u8; want as usize]; - let n = f - .read(&mut buf) - .map_err(|e| anyhow!("fs_read_bytes {:?}: {}", path, e))?; - buf.truncate(n); - let eof = n < want as usize; - let encoded = base64::engine::general_purpose::STANDARD.encode(&buf); - Ok(json!({ "data": encoded, "eof": eof, "bytes_read": n })) - }); - - let s = self.clone(); - registry.register("fs_write_bytes", move |args| { - use base64::Engine; - use std::io::{Seek, SeekFrom, Write}; - let path = args["path"] - .as_str() - .ok_or_else(|| anyhow!("fs_write_bytes: missing 'path'"))?; - let data_b64 = args["data"] - .as_str() - .ok_or_else(|| anyhow!("fs_write_bytes: missing 'data'"))?; - let data = base64::engine::general_purpose::STANDARD - .decode(data_b64) - .map_err(|e| anyhow!("fs_write_bytes: bad base64: {}", e))?; - let offset = args["offset"].as_u64(); - let append = args["append"].as_bool().unwrap_or(false); - let target = s.resolve(path)?; - let mut f = std::fs::OpenOptions::new() - .write(true) - .create(true) - .truncate(offset.is_none() && !append) - .append(append) - .open(&target) - .map_err(|e| anyhow!("fs_write_bytes {:?}: {}", path, e))?; - if let Some(off) = offset { - if !append { - f.seek(SeekFrom::Start(off)) - .map_err(|e| anyhow!("fs_write_bytes seek {:?}: {}", path, e))?; - } - } - f.write_all(&data) - .map_err(|e| anyhow!("fs_write_bytes {:?}: {}", path, e))?; - Ok(json!({ "bytes_written": data.len() })) - }); - - let s = self.clone(); - registry.register("fs_truncate", move |args| { - let path = args["path"] - .as_str() - .ok_or_else(|| anyhow!("fs_truncate: missing 'path'"))?; - let length = args["length"] - .as_u64() - .ok_or_else(|| anyhow!("fs_truncate: missing 'length'"))?; - let target = s.resolve(path)?; - let f = std::fs::OpenOptions::new() - .write(true) - .open(&target) - .map_err(|e| anyhow!("fs_truncate {:?}: {}", path, e))?; - f.set_len(length) - .map_err(|e| anyhow!("fs_truncate {:?}: {}", path, e))?; - Ok(json!({})) - }); - - let s = self.clone(); - registry.register("fs_unlink", move |args| { - let path = args["path"] - .as_str() - .ok_or_else(|| anyhow!("fs_unlink: missing 'path'"))?; - let target = s.resolve(path)?; - let md = - std::fs::metadata(&target).map_err(|e| anyhow!("fs_unlink {:?}: {}", path, e))?; - if md.is_dir() { - std::fs::remove_dir(&target) - } else { - std::fs::remove_file(&target) - } - .map_err(|e| anyhow!("fs_unlink {:?}: {}", path, e))?; - Ok(json!({})) - }); - } } /// Internal helper: assemble the final tool registry from caller-supplied @@ -2657,10 +2448,11 @@ mod tests { // End-to-end through the tool registry: the error surface the // guest actually sees. let root = tmpdir("dispatch"); + let preopens = vec![Preopen::new(&root, "/host").unwrap()]; let mut reg = ToolRegistry::new(); - FsSandbox::new(&root).unwrap().register(&mut reg); + FsRouter::new(&preopens).unwrap().register(&mut reg); - let req = br#"{"name":"fs_read","args":{"path":"../outside.txt"}}"#; + let req = br#"{"name":"fs_read","args":{"path":"/host/../outside.txt"}}"#; let resp = reg.dispatch(req); let s = std::str::from_utf8(&resp).unwrap(); assert!(s.contains("\"error\""), "{s}"); @@ -2740,15 +2532,16 @@ mod tests { #[test] fn fs_write_then_read_roundtrip() { let root = tmpdir("roundtrip"); + let preopens = vec![Preopen::new(&root, "/host").unwrap()]; let mut reg = ToolRegistry::new(); - FsSandbox::new(&root).unwrap().register(&mut reg); + FsRouter::new(&preopens).unwrap().register(&mut reg); - let w = br#"{"name":"fs_write","args":{"path":"hello.txt","text":"hi"}}"#; + let w = br#"{"name":"fs_write","args":{"path":"/host/hello.txt","text":"hi"}}"#; let resp = reg.dispatch(w); let s = std::str::from_utf8(&resp).unwrap(); assert!(s.contains("\"bytes_written\":2"), "{s}"); - let r = br#"{"name":"fs_read","args":{"path":"hello.txt"}}"#; + let r = br#"{"name":"fs_read","args":{"path":"/host/hello.txt"}}"#; let resp = reg.dispatch(r); let s = std::str::from_utf8(&resp).unwrap(); assert!(s.contains("\"text\":\"hi\""), "{s}"); From 04efd41580af943d88ffa205642fcfe292e75510 Mon Sep 17 00:00:00 2001 From: danbugs Date: Fri, 15 May 2026 05:36:07 +0000 Subject: [PATCH 2/5] fix: return error for unsupported socket options instead of silent no-op Signed-off-by: danbugs --- host/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/host/src/lib.rs b/host/src/lib.rs index 51a1e6c..c4dda5a 100644 --- a/host/src/lib.rs +++ b/host/src/lib.rs @@ -1180,14 +1180,14 @@ fn register_net_tools( // SOL_SOCKET=1, SO_REUSEADDR=2 if level == 1 && optname == 2 { sock.set_reuse_address(value != 0)?; - } // SOL_SOCKET=1, SO_KEEPALIVE=9 - if level == 1 && optname == 9 { + } else if level == 1 && optname == 9 { sock.set_keepalive(value != 0)?; - } // IPPROTO_TCP=6, TCP_NODELAY=1 - if level == 6 && optname == 1 { + } else if level == 6 && optname == 1 { sock.set_nodelay(value != 0)?; + } else { + return Err(anyhow!("unsupported socket option: level={}, optname={}", level, optname)); } Ok(json!({})) }); @@ -1208,7 +1208,7 @@ fn register_net_tools( } else if level == 6 && optname == 1 { sock.nodelay()? as i32 } else { - 0 + return Err(anyhow!("unsupported socket option: level={}, optname={}", level, optname)); }; Ok(json!({ "value": val })) }); From 94b454e0b06a839d26ea46bea46a8dcb9235b437 Mon Sep 17 00:00:00 2001 From: danbugs Date: Fri, 15 May 2026 05:37:08 +0000 Subject: [PATCH 3/5] fix: return TempDir from inject_script_into_rootfs to prevent leak Signed-off-by: danbugs --- demos/pptx-gen/src/main.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/demos/pptx-gen/src/main.rs b/demos/pptx-gen/src/main.rs index 5482475..c9a69b2 100644 --- a/demos/pptx-gen/src/main.rs +++ b/demos/pptx-gen/src/main.rs @@ -197,7 +197,7 @@ fn execute_in_sandbox( debug!("script: {:?}", script_path); let cpio_start = std::time::Instant::now(); - let modified_rootfs = inject_script_into_rootfs(rootfs, &script_path)?; + let (modified_rootfs, _rootfs_tmpdir) = inject_script_into_rootfs(rootfs, &script_path)?; if timing { info!(" cpio inject: {:?}", cpio_start.elapsed()); } @@ -228,7 +228,7 @@ fn execute_in_sandbox( Ok(vm_output.output) } -fn inject_script_into_rootfs(original_rootfs: &Path, script_path: &Path) -> Result { +fn inject_script_into_rootfs(original_rootfs: &Path, script_path: &Path) -> Result<(PathBuf, tempfile::TempDir)> { let temp_dir = tempfile::tempdir()?; let extract_dir = temp_dir.path().join("rootfs"); let new_cpio = temp_dir.path().join("rootfs_with_script.cpio"); @@ -270,11 +270,7 @@ fn inject_script_into_rootfs(original_rootfs: &Path, script_path: &Path) -> Resu anyhow::bail!("cpio create failed"); } - // Leak tempdir so file persists - let path = new_cpio.clone(); - std::mem::forget(temp_dir); - - Ok(path) + Ok((new_cpio, temp_dir)) } fn extract_pptx_from_output(output: &str) -> Result> { From e57c37f2c5b37fc771110a20e07ad0f0e18f34ee Mon Sep 17 00:00:00 2001 From: danbugs Date: Fri, 15 May 2026 05:59:02 +0000 Subject: [PATCH 4/5] fmt: fix cargo fmt violations Signed-off-by: danbugs --- host/src/lib.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/host/src/lib.rs b/host/src/lib.rs index c4dda5a..304a1f8 100644 --- a/host/src/lib.rs +++ b/host/src/lib.rs @@ -830,7 +830,6 @@ impl FsSandbox { } Ok(out) } - } /// Internal helper: assemble the final tool registry from caller-supplied @@ -1187,7 +1186,11 @@ fn register_net_tools( } else if level == 6 && optname == 1 { sock.set_nodelay(value != 0)?; } else { - return Err(anyhow!("unsupported socket option: level={}, optname={}", level, optname)); + return Err(anyhow!( + "unsupported socket option: level={}, optname={}", + level, + optname + )); } Ok(json!({})) }); @@ -1208,7 +1211,11 @@ fn register_net_tools( } else if level == 6 && optname == 1 { sock.nodelay()? as i32 } else { - return Err(anyhow!("unsupported socket option: level={}, optname={}", level, optname)); + return Err(anyhow!( + "unsupported socket option: level={}, optname={}", + level, + optname + )); }; Ok(json!({ "value": val })) }); From 6c578d8b37439a4fb8c949b6bf06e96111f6decb Mon Sep 17 00:00:00 2001 From: danbugs Date: Fri, 15 May 2026 06:15:25 +0000 Subject: [PATCH 5/5] fix: update test_fs_read_bytes_capped to use FsRouter Signed-off-by: danbugs --- host/src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/host/src/lib.rs b/host/src/lib.rs index 304a1f8..6c1edec 100644 --- a/host/src/lib.rs +++ b/host/src/lib.rs @@ -2806,10 +2806,12 @@ mod tests { fn test_fs_read_bytes_capped() { let root = tmpdir("readcap"); fs::write(root.join("small.bin"), b"hello").unwrap(); + let preopens = vec![Preopen::new(&root, "/host").unwrap()]; let mut reg = ToolRegistry::new(); - FsSandbox::new(&root).unwrap().register(&mut reg); + FsRouter::new(&preopens).unwrap().register(&mut reg); - let req = br#"{"name":"fs_read_bytes","args":{"path":"small.bin","len":1099511627776}}"#; + let req = + br#"{"name":"fs_read_bytes","args":{"path":"/host/small.bin","len":1099511627776}}"#; let resp = reg.dispatch(req); let s = std::str::from_utf8(&resp).unwrap(); assert!(!s.contains("\"error\""), "should succeed: {s}");