From 605bcc61ce9115f591c2459f907db7de8484411f Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Wed, 8 Jul 2026 08:54:45 +0200 Subject: [PATCH] fix(wasi): grant filesystem capability on --wasi-fs + close a sandbox-escape hazard (#392) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two-part fix for the maintainer-reported #392 (differential oracle: wasmtime grants the same --dir correctly): 1. Silent no-op — kilnd registered a preopen for --wasi-fs/--dir but never enabled the filesystem capability (WasiCapabilities::minimal() leaves read/write/directory/metadata_access = false), so the capability gate rejected every op and the guest got ENOENT on all paths. Now, when fs paths are granted, enable read+write+directory+metadata access (matches wasmtime --dir). kilnd/src/lib.rs. 2. Sandbox escape (the latent hazard the report flagged, exposed once write is enabled) — the descriptor open-at/create/remove checks gated containment on `canonicalize().is_ok()`, which fails for a not-yet-created target, so a write to `../evil.txt` escaped the preopen. Replaced all three sites with a lexical `is_within_sandbox` (rejects any `..` component and absolute-path replacement, independent of existence). Unit-tested: a `..` write to a missing target is rejected (verifies SR-33). kiln-wasi 1 new test passes; kilnd builds. The capability half's end-to-end oracle is the #392 repro (needs a wasm32-wasip2 guest), documented on the issue. Trace: SR-33 --- kiln-wasi/src/dispatcher.rs | 58 +++++++++++++------ kilnd/src/lib.rs | 10 ++++ .../requirements/functional-requirements.yaml | 28 +++++++++ 3 files changed, 79 insertions(+), 17 deletions(-) diff --git a/kiln-wasi/src/dispatcher.rs b/kiln-wasi/src/dispatcher.rs index c90ba1fc..45136b62 100644 --- a/kiln-wasi/src/dispatcher.rs +++ b/kiln-wasi/src/dispatcher.rs @@ -712,17 +712,11 @@ impl WasiDispatcher { // Construct full path let full_path = base_path.join(&path); - // Check path is within sandbox (no escape via ..) - let canonical = match full_path.canonicalize() { - Ok(p) => p, - Err(_) => { - // File might not exist yet for write operations - full_path.clone() - } - }; - - // Basic safety check - path should start with base - if !canonical.starts_with(&base_path) && full_path.canonicalize().is_ok() { + // Reject any path that escapes the preopen — checked lexically so + // it holds for not-yet-created files too. `canonicalize` fails on a + // missing target, so an existence-gated check would let a write to + // `../evil.txt` escape the sandbox (#392 latent hazard). + if !is_within_sandbox(&base_path, &full_path) { return Err(Error::wasi_permission_denied("Path escapes sandbox")); } @@ -928,7 +922,7 @@ impl WasiDispatcher { let full_path = base_path.join(&path); // Sandbox check - if full_path.canonicalize().is_ok() && !full_path.starts_with(&base_path) { + if !is_within_sandbox(&base_path, &full_path) { return Err(Error::wasi_permission_denied("Path escapes sandbox")); } @@ -966,11 +960,9 @@ impl WasiDispatcher { let full_path = base_path.join(&path); - // Sandbox check - if let Ok(canonical) = full_path.canonicalize() { - if !canonical.starts_with(&base_path) { - return Err(Error::wasi_permission_denied("Path escapes sandbox")); - } + // Sandbox check (lexical — existence-independent) + if !is_within_sandbox(&base_path, &full_path) { + return Err(Error::wasi_permission_denied("Path escapes sandbox")); } match std::fs::remove_file(&full_path) { @@ -2656,11 +2648,43 @@ impl kiln_foundation::HostImportHandler for WasiDispatcher { } } +/// True iff `full` (a guest path already joined onto the preopen `base`) stays +/// inside the sandbox — a **lexical** check that does not touch the filesystem, +/// so it is correct for not-yet-created files. Rejects any `..` traversal and +/// any absolute path that `Path::join` let replace the base. A +/// `canonicalize`-based check silently admits a write to a non-existent +/// `../target` because `canonicalize` fails on the missing file (#392). +fn is_within_sandbox(base: &std::path::Path, full: &std::path::Path) -> bool { + use std::path::Component; + !full + .components() + .any(|c| matches!(c, Component::ParentDir)) + && full.starts_with(base) +} + #[cfg(test)] mod tests { use super::*; use kiln_foundation::memory_init::MemoryInitializer; + /// The #392 sandbox-escape hazard: a write to a not-yet-existing `../target` + /// must be rejected. An existence-gated (canonicalize) check would admit it + /// because canonicalize fails on the missing file. + // rivet: verifies SR-33 + #[test] + fn sandbox_rejects_parent_dir_escape_even_when_target_missing() { + use std::path::Path; + let base = Path::new("/preopen/sandbox"); + // within the preopen — accepted + assert!(is_within_sandbox(base, &base.join("inside.txt"))); + assert!(is_within_sandbox(base, &base.join("sub/inside.txt"))); + // `..` escape to a NON-EXISTENT target — must be rejected + assert!(!is_within_sandbox(base, &base.join("../evil.txt"))); + assert!(!is_within_sandbox(base, &base.join("sub/../../evil.txt"))); + // absolute path that `join` lets replace the base — rejected + assert!(!is_within_sandbox(base, &base.join("/etc/passwd"))); + } + #[test] fn test_dispatcher_creation() -> Result<()> { MemoryInitializer::ensure_initialized()?; diff --git a/kilnd/src/lib.rs b/kilnd/src/lib.rs index 1efc53e6..e08fa967 100644 --- a/kilnd/src/lib.rs +++ b/kilnd/src/lib.rs @@ -1414,6 +1414,16 @@ pub fn run() -> Result<()> { e })?; + // Granting a preopen must also enable the filesystem capability — + // otherwise the preopen is registered but the capability gate rejects + // every operation, so the guest gets ENOENT on all paths (a silent + // no-op; #392). Match wasmtime's `--dir`: read + write on the mapping. + if !args.wasi_fs_paths.is_empty() { + capabilities.filesystem.read_access = true; + capabilities.filesystem.write_access = true; + capabilities.filesystem.directory_access = true; + capabilities.filesystem.metadata_access = true; + } // Add filesystem access paths for path in &args.wasi_fs_paths { let _ = capabilities.filesystem.add_allowed_path(path); diff --git a/safety/requirements/functional-requirements.yaml b/safety/requirements/functional-requirements.yaml index 759b0f28..0ca684ed 100644 --- a/safety/requirements/functional-requirements.yaml +++ b/safety/requirements/functional-requirements.yaml @@ -954,3 +954,31 @@ artifacts: model: claude-opus-4-8 timestamp: 2026-07-07T19:24:29Z release: v0.3.6 + + - id: SR-33 + type: requirement + title: kilnd --wasi-fs/--dir actually grants filesystem access (no silent no-op) + status: implemented + description: "When kilnd is given --wasi-fs/--dir , the WASI filesystem capability (read_access, directory_access, and write_access for writable mappings) must actually be enabled so a Preview2 guest can read/list/open files under the preopen — matching wasmtime's --dir. Today kilnd builds WasiCapabilities::minimal() and enables env/random/io but never filesystem access, so --wasi-fs registers an unusable preopen and every op returns noent (silent no-op, violates FAIL-LOUD). Root cause: kilnd/src/lib.rs capability build + dispatcher.rs:663 directory_access gate. Differential oracle: wasmtime 42 reads the same dir. Issue #392." + tags: [kilnd, wasi, filesystem, bug] + fields: + upstream-ref: https://github.com/pulseengine/kiln/issues/392 + provenance: + created-by: ai + model: claude-opus-4-8 + timestamp: 2026-07-08T06:48:51Z + release: v0.4.0 + + - id: SR-34 + type: requirement + title: "kilnd loads witness-instrumented cores (real-app scale: multi-memory + high global/branch count)" + status: proposed + description: "kilnd must load and run witness-MC/DC-instrumented core modules at real-app scale (e.g. 1413 branches, 4267 globals, 2 memories, 258KB) — currently fails at load with [Runtime][E03ED] Failed to load module. Isolation ruled out WASI, witness output validity (wasm-tools accepts), raw global count (9000 OK), size (116KB OK), and 2-memory declaration (OK) — so the trigger is a combination/interaction specific to the instrumented core, not yet pinned to a line. Blocks the witness MC/DC path on real apps. Issue #391." + tags: [kilnd, decoder, witness, loader, bug] + fields: + upstream-ref: https://github.com/pulseengine/kiln/issues/391 + provenance: + created-by: ai + model: claude-opus-4-8 + timestamp: 2026-07-08T06:48:52Z + release: v0.4.0