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]>
This commit is contained in:
ClawHDF5 Planner
co-authored by
Claude Sonnet 4.6
Claude Sonnet 4.6
parent
5ca0b8092e
commit
4aee2fa610
@@ -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,8 +133,63 @@ 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) };
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user