INT-06, INT-08, INT-10: WAL fuzz target, JNI Mutex wrapping, media sandboxing

INT-06 — Add WAL replay fuzz target (crates/clawhdf5-agent/fuzz/). Writes arbitrary bytes to a temp file and runs them through WalFile::read_entries, exercising the magic-byte check, version dispatch, CRC32 guard, length-prefix bounds, and EOF handling. No byte sequence should cause a panic or OOM. INT-08 — Wrap Android JNI HDF5Memory handles in Mutex. Handle type changed from *mut HDF5Memory to *mut Mutex<HDF5Memory>. Every JNI entry point acquires the lock before calling into HDF5Memory, making concurrent calls from multiple Java/Kotlin threads safe without requiring the caller to synchronize externally. Added concurrent_count_active_is_safe test to exercise the path. INT-10 — Add media reference sandboxing to MediaRef::validate(). Path references are canonicalized and checked to stay within an optional sandbox directory (preventing ../ traversal). URL references must use a scheme from ALLOWED_URL_SCHEMES (https, http); file://, data:, and schemeless strings are rejected. Inline references are always accepted. Added 9 unit tests covering the acceptance and rejection paths. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
2026-08-12 11:46:59 +00:00
parent 5ca0b8092e
commit 4aee2fa610
4 changed files with 258 additions and 23 deletions
@@ -0,0 +1,23 @@
+[package]
+name = "clawhdf5-agent-fuzz"
+version = "0.0.0"
+publish = false
+edition = "2024"
+
+[package.metadata]
+cargo-fuzz = true
+
+[dependencies]
+libfuzzer-sys = "0.4"
+tempfile = "3"
+
+[dependencies.clawhdf5-agent]
+path = ".."
+
+[workspace]
+members = ["."]
+
+[[bin]]
+name = "fuzz_wal_replay"
+path = "fuzz_targets/fuzz_wal_replay.rs"
+doc = false
@@ -0,0 +1,21 @@
+#![no_main]
+use libfuzzer_sys::fuzz_target;
+
+use std::io::Write as _;
+
+fuzz_target!(|data: &[u8]| {
+    // Write the fuzz input to a temporary file, then run it through the WAL
+    // replay path.  The goal: verify that no arbitrary byte sequence causes a
+    // panic, OOM, or other safety violation.  CRC32 mismatches, truncated
+    // entries, bad magic bytes, and oversized length fields are all expected to
+    // return an error (not crash).
+    let Ok(mut tmp) = tempfile::NamedTempFile::new() else {
+        return;
+    };
+    if tmp.write_all(data).is_err() {
+        return;
+    }
+    // Flush so the reader sees the data.
+    let _ = tmp.flush();
+    let _ = clawhdf5_agent::wal::WalFile::read_entries(tmp.path());
+});
@@ -133,7 +133,62 @@ impl MediaRef {
            checksum: Some(cs),
        }
    }
+
+    /// Validate this reference against a sandbox directory and a URL scheme allowlist.
+    ///
+    /// * `Path` references are canonicalized and checked to be within `sandbox`
+    ///   (if `sandbox` is `Some`).  A path that escapes the sandbox via `..`
+    ///   or symlinks is rejected with an error.
+    /// * `Url` references must begin with one of the schemes in
+    ///   [`ALLOWED_URL_SCHEMES`].  An empty or scheme-less URL is rejected.
+    /// * `Inline` references are always valid (no external resolution).
+    ///
+    /// Returns `Ok(())` when the reference passes all checks, or an `Err`
+    /// with a human-readable reason otherwise.
+    pub fn validate(&self, sandbox: Option<&std::path::Path>) -> Result<(), String> {
+        match &self.ref_type {
+            MediaRefType::Path(raw) => {
+                let candidate = std::path::Path::new(raw);
+                let canonical = candidate
+                    .canonicalize()
+                    .map_err(|e| format!("path canonicalization failed for {raw:?}: {e}"))?;
+                if let Some(root) = sandbox {
+                    let root_canonical = root
+                        .canonicalize()
+                        .map_err(|e| format!("sandbox canonicalization failed: {e}"))?;
+                    if !canonical.starts_with(&root_canonical) {
+                        return Err(format!(
+                            "path {canonical:?} escapes sandbox {root_canonical:?}"
+                        ));
                    }
+                }
+                Ok(())
+            }
+            MediaRefType::Url(url) => {
+                let scheme_end = url
+                    .find("://")
+                    .ok_or_else(|| format!("URL {url:?} has no scheme"))?;
+                let scheme = &url[..scheme_end];
+                if ALLOWED_URL_SCHEMES.contains(&scheme) {
+                    Ok(())
+                } else {
+                    Err(format!(
+                        "URL scheme {scheme:?} is not in the allowlist {:?}",
+                        ALLOWED_URL_SCHEMES
+                    ))
+                }
+            }
+            MediaRefType::Inline(_) => Ok(()),
+        }
+    }
+}
+
+/// URL schemes that are permitted in `MediaRef::Url` references.
+///
+/// Any scheme not in this list is rejected by [`MediaRef::validate`].  Keeping
+/// the list explicit prevents `file://` or `data:` URIs from being smuggled in
+/// via adversarial memory content.
+pub const ALLOWED_URL_SCHEMES: &[&str] = &["https", "http"];

 // ---------------------------------------------------------------------------
 // FNV-1a helper (no external deps)
@@ -807,4 +862,69 @@ mod tests {
        let r = store.get_record(id).unwrap();
        assert_eq!(r.metadata.get("source").unwrap(), "camera-1");
    }
+
+    // -----------------------------------------------------------------------
+    // MediaRef::validate — sandboxing
+    // -----------------------------------------------------------------------
+
+    #[test]
+    fn inline_always_valid() {
+        let r = MediaRef::inline(vec![1, 2, 3], "application/octet-stream");
+        assert!(r.validate(None).is_ok());
+    }
+
+    #[test]
+    fn url_allowed_scheme_https() {
+        let r = MediaRef::url("https://example.com/img.png", "image/png");
+        assert!(r.validate(None).is_ok());
+    }
+
+    #[test]
+    fn url_allowed_scheme_http() {
+        let r = MediaRef::url("http://example.com/img.png", "image/png");
+        assert!(r.validate(None).is_ok());
+    }
+
+    #[test]
+    fn url_disallowed_scheme_file() {
+        let r = MediaRef::url("file:///etc/passwd", "text/plain");
+        assert!(r.validate(None).is_err());
+    }
+
+    #[test]
+    fn url_disallowed_scheme_data() {
+        let r = MediaRef::url("data:text/html,<script>", "text/html");
+        assert!(r.validate(None).is_err());
+    }
+
+    #[test]
+    fn url_no_scheme_rejected() {
+        let r = MediaRef::url("not-a-url", "text/plain");
+        assert!(r.validate(None).is_err());
+    }
+
+    #[test]
+    fn path_within_sandbox_accepted() {
+        let dir = tempfile::tempdir().unwrap();
+        let file = dir.path().join("audio.mp3");
+        std::fs::write(&file, b"dummy").unwrap();
+        let r = MediaRef::path(file.to_str().unwrap(), "audio/mpeg");
+        assert!(r.validate(Some(dir.path())).is_ok());
+    }
+
+    #[test]
+    fn path_outside_sandbox_rejected() {
+        let sandbox = tempfile::tempdir().unwrap();
+        // /tmp itself exists and is outside the sandbox subdir
+        let r = MediaRef::path("/tmp", "inode/directory");
+        let result = r.validate(Some(sandbox.path()));
+        // May fail at canonicalization or at the starts_with check; either is correct
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn path_nonexistent_rejected_at_canonicalize() {
+        let r = MediaRef::path("/this/path/does/not/exist/abc123", "text/plain");
+        assert!(r.validate(None).is_err());
+    }
 }
@@ -3,13 +3,14 @@
 //! Exposes `extern "C"` functions for use via JNI from Kotlin.
 //! Each HDF5Memory instance is managed via an opaque handle (pointer).
 //!
-//! Thread safety: the caller (Kotlin side) must synchronize access
-//! to a single handle. Multiple handles are independent.
+//! Thread safety: each handle wraps `HDF5Memory` in a `Mutex`, so concurrent
+//! calls on the same handle are safe.  Multiple handles are fully independent.

 use std::ffi::{CStr, CString};
 use std::os::raw::c_char;
 use std::path::PathBuf;
 use std::ptr;
+use std::sync::Mutex;

 use clawhdf5_agent::{AgentMemory, HDF5Memory, MemoryConfig, MemoryEntry};

@@ -17,8 +18,12 @@ use clawhdf5_agent::{AgentMemory, HDF5Memory, MemoryConfig, MemoryEntry};
 // Handle management
 // ---------------------------------------------------------------------------

-/// Opaque handle to an HDF5Memory instance.
-type Handle = *mut HDF5Memory;
+/// Opaque handle to a mutex-protected HDF5Memory instance.
+///
+/// Stored on the heap so that the raw pointer (an integer from JNI's
+/// perspective) is stable across calls.  The `Mutex` makes concurrent JNI
+/// calls on the same handle safe without requiring the caller to synchronize.
+type Handle = *mut Mutex<HDF5Memory>;

 /// Create a new HDF5 memory file.
 ///
@@ -46,7 +51,7 @@ pub unsafe extern "C" fn edgehdf5_create(

    let config = MemoryConfig::new(PathBuf::from(path), &agent_id, embedding_dim as usize);
    match HDF5Memory::create(config) {
-        Ok(mem) => Box::into_raw(Box::new(mem)),
+        Ok(mem) => Box::into_raw(Box::new(Mutex::new(mem))),
        Err(_) => ptr::null_mut(),
    }
 }
@@ -67,7 +72,7 @@ pub unsafe extern "C" fn edgehdf5_open(path: *const c_char) -> Handle {
    };

    match HDF5Memory::open(std::path::Path::new(&path)) {
-        Ok(mem) => Box::into_raw(Box::new(mem)),
+        Ok(mem) => Box::into_raw(Box::new(Mutex::new(mem))),
        Err(_) => ptr::null_mut(),
    }
 }
@@ -82,7 +87,7 @@ pub unsafe extern "C" fn edgehdf5_open(path: *const c_char) -> Handle {
 pub unsafe extern "C" fn edgehdf5_close(handle: Handle) {
    if !handle.is_null() {
        // SAFETY: handle was created by Box::into_raw in edgehdf5_create; this is the final use.
-        unsafe { drop(Box::from_raw(handle)) };
+        unsafe { drop(Box::<Mutex<HDF5Memory>>::from_raw(handle)) };
    }
 }

@@ -115,11 +120,15 @@ pub unsafe extern "C" fn edgehdf5_save(
    session_id: *const c_char,
    tags: *const c_char,
 ) -> i64 {
-    // SAFETY: handle is a valid non-null Handle from edgehdf5_create; caller ensures exclusive access.
-    let mem = match unsafe { handle.as_mut() } {
+    // SAFETY: handle is a valid non-null Handle from edgehdf5_create.
+    let mtx = match unsafe { handle.as_ref() } {
        Some(m) => m,
        None => return -1,
    };
+    let mut mem = match mtx.lock() {
+        Ok(g) => g,
+        Err(_) => return -1,
+    };

    // SAFETY: JNI caller guarantees the pointer argument is a valid null-terminated C string.
    let chunk = match unsafe { cstr_to_string(chunk) } {
@@ -176,7 +185,7 @@ pub unsafe extern "C" fn edgehdf5_save(
 pub unsafe extern "C" fn edgehdf5_count_active(handle: Handle) -> u64 {
    // SAFETY: handle is a valid non-null Handle from edgehdf5_create.
    match unsafe { handle.as_ref() } {
-        Some(mem) => mem.count_active() as u64,
+        Some(mtx) => mtx.lock().map(|g| g.count_active() as u64).unwrap_or(0),
        None => 0,
    }
 }
@@ -190,7 +199,7 @@ pub unsafe extern "C" fn edgehdf5_count_active(handle: Handle) -> u64 {
 pub unsafe extern "C" fn edgehdf5_count(handle: Handle) -> u64 {
    // SAFETY: handle is a valid non-null Handle from edgehdf5_create.
    match unsafe { handle.as_ref() } {
-        Some(mem) => mem.count() as u64,
+        Some(mtx) => mtx.lock().map(|g| g.count() as u64).unwrap_or(0),
        None => 0,
    }
 }
@@ -202,11 +211,15 @@ pub unsafe extern "C" fn edgehdf5_count(handle: Handle) -> u64 {
 /// `handle` must be a valid, non-null handle.
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn edgehdf5_delete(handle: Handle, index: u64) -> i32 {
-    // SAFETY: handle is a valid non-null Handle from edgehdf5_create; caller ensures exclusive access.
-    let mem = match unsafe { handle.as_mut() } {
+    // SAFETY: handle is a valid non-null Handle from edgehdf5_create.
+    let mtx = match unsafe { handle.as_ref() } {
        Some(m) => m,
        None => return -1,
    };
+    let mut mem = match mtx.lock() {
+        Ok(g) => g,
+        Err(_) => return -1,
+    };

    match mem.delete(index as usize) {
        Ok(()) => 0,
@@ -250,11 +263,15 @@ pub unsafe extern "C" fn edgehdf5_hybrid_search(
    out_scores: *mut f32,
    out_chunks: *mut *mut c_char,
 ) -> u32 {
-    // SAFETY: handle is a valid non-null Handle from edgehdf5_create; caller ensures exclusive access.
-    let mem = match unsafe { handle.as_mut() } {
+    // SAFETY: handle is a valid non-null Handle from edgehdf5_create.
+    let mtx = match unsafe { handle.as_ref() } {
        Some(m) => m,
        None => return 0,
    };
+    let mut mem = match mtx.lock() {
+        Ok(g) => g,
+        Err(_) => return 0,
+    };
    // SAFETY: JNI caller guarantees the pointer argument is a valid null-terminated C string.
    let query_text = match unsafe { cstr_to_string(query_text) } {
        Some(s) => s,
@@ -329,11 +346,15 @@ pub unsafe extern "C" fn edgehdf5_add_session(
    channel: *const c_char,
    summary: *const c_char,
 ) -> i32 {
-    // SAFETY: handle is a valid non-null Handle from edgehdf5_create; caller ensures exclusive access.
-    let mem = match unsafe { handle.as_mut() } {
+    // SAFETY: handle is a valid non-null Handle from edgehdf5_create.
+    let mtx = match unsafe { handle.as_ref() } {
        Some(m) => m,
        None => return -1,
    };
+    let mut mem = match mtx.lock() {
+        Ok(g) => g,
+        Err(_) => return -1,
+    };
    // SAFETY: JNI caller guarantees the pointer argument is a valid null-terminated C string.
    let id = match unsafe { cstr_to_string(id) } {
        Some(s) => s,
@@ -375,10 +396,14 @@ pub unsafe extern "C" fn edgehdf5_get_session_summary(
    session_id: *const c_char,
 ) -> *mut c_char {
    // SAFETY: handle is a valid non-null Handle from edgehdf5_create.
-    let mem = match unsafe { handle.as_ref() } {
+    let mtx = match unsafe { handle.as_ref() } {
        Some(m) => m,
        None => return ptr::null_mut(),
    };
+    let mem = match mtx.lock() {
+        Ok(g) => g,
+        Err(_) => return ptr::null_mut(),
+    };
    // SAFETY: JNI caller guarantees the pointer argument is a valid null-terminated C string.
    let session_id = match unsafe { cstr_to_string(session_id) } {
        Some(s) => s,
@@ -411,11 +436,15 @@ pub unsafe extern "C" fn edgehdf5_add_entity(
    entity_type: *const c_char,
    embedding_idx: i64,
 ) -> i64 {
-    // SAFETY: handle is a valid non-null Handle from edgehdf5_create; caller ensures exclusive access.
-    let mem = match unsafe { handle.as_mut() } {
+    // SAFETY: handle is a valid non-null Handle from edgehdf5_create.
+    let mtx = match unsafe { handle.as_ref() } {
        Some(m) => m,
        None => return -1,
    };
+    let mut mem = match mtx.lock() {
+        Ok(g) => g,
+        Err(_) => return -1,
+    };
    // SAFETY: JNI caller guarantees the pointer argument is a valid null-terminated C string.
    let name = match unsafe { cstr_to_string(name) } {
        Some(s) => s,
@@ -447,11 +476,15 @@ pub unsafe extern "C" fn edgehdf5_add_relation(
    relation: *const c_char,
    weight: f32,
 ) -> i32 {
-    // SAFETY: handle is a valid non-null Handle from edgehdf5_create; caller ensures exclusive access.
-    let mem = match unsafe { handle.as_mut() } {
+    // SAFETY: handle is a valid non-null Handle from edgehdf5_create.
+    let mtx = match unsafe { handle.as_ref() } {
        Some(m) => m,
        None => return -1,
    };
+    let mut mem = match mtx.lock() {
+        Ok(g) => g,
+        Err(_) => return -1,
+    };
    // SAFETY: JNI caller guarantees the pointer argument is a valid null-terminated C string.
    let relation = match unsafe { cstr_to_string(relation) } {
        Some(s) => s,
@@ -492,7 +525,8 @@ mod tests {
    fn open_handle(dir: &tempfile::TempDir) -> Handle {
        let path = CString::new(dir.path().join("mem.h5").to_str().unwrap()).unwrap();
        let agent_id = CString::new("test-agent").unwrap();
-        // SAFETY: both C strings are valid and null-terminated.
+        // SAFETY: both C strings are valid and null-terminated; returned handle
+        // wraps HDF5Memory in a Mutex and is safe to use from multiple threads.
        unsafe { edgehdf5_create(path.as_ptr(), agent_id.as_ptr(), EMBEDDING_DIM) }
    }

@@ -590,4 +624,41 @@ mod tests {

        unsafe { edgehdf5_close(handle) };
    }
+
+    /// Verify that concurrent calls on the same handle do not cause data races.
+    ///
+    /// Each thread calls `edgehdf5_count_active` on the shared handle.  With the
+    /// `Mutex` wrapper in place this must complete without a panic or SIGABRT.
+    /// Without the mutex it would be UB.
+    #[test]
+    fn concurrent_count_active_is_safe() {
+        use std::sync::Arc;
+
+        let dir = tempfile::tempdir().unwrap();
+        let handle = open_handle(&dir);
+        assert!(!handle.is_null());
+
+        // Share the raw pointer across threads via a copy-friendly wrapper.
+        // SAFETY: the Mutex inside the handle makes concurrent access sound.
+        #[derive(Clone, Copy)]
+        struct SendableHandle(Handle);
+        unsafe impl Send for SendableHandle {}
+
+        let shared = Arc::new(SendableHandle(handle));
+        let threads: Vec<_> = (0..8)
+            .map(|_| {
+                let h = Arc::clone(&shared);
+                std::thread::spawn(move || {
+                    // SAFETY: handle is valid (not yet closed); Mutex guards access.
+                    let count = unsafe { edgehdf5_count_active(h.0) };
+                    assert_eq!(count, 0);
+                })
+            })
+            .collect();
+        for t in threads {
+            t.join().expect("thread panicked");
+        }
+
+        unsafe { edgehdf5_close(handle) };
+    }
 }