Merge remote-tracking branch 'origin/clawmates/mission-01a00c41-421200ee' into verify/v3-plus-v6

# Conflicts:
#	crates/clawhdf5/Cargo.toml
This commit is contained in:
Omar Sobh
2026-08-17 06:15:19 -07:00
12 changed files with 399 additions and 37 deletions
+1
View File
@@ -12,6 +12,7 @@ categories = ["algorithms", "science"]
[dependencies] [dependencies]
clawhdf5-format = { path = "../clawhdf5-format", version = "2.1.0" } clawhdf5-format = { path = "../clawhdf5-format", version = "2.1.0" }
clawhdf5-io = { path = "../clawhdf5-io", version = "2.1.0" } clawhdf5-io = { path = "../clawhdf5-io", version = "2.1.0" }
clawhdf5-accel = { path = "../clawhdf5-accel", version = "2.1.0" }
rayon = { version = "1", optional = true } rayon = { version = "1", optional = true }
[features] [features]
+6 -24
View File
@@ -44,32 +44,14 @@ impl DistanceMetric {
} }
/// Compute distance between two vectors using the given metric. /// Compute distance between two vectors using the given metric.
///
/// Delegates to `clawhdf5-accel`'s runtime-dispatched SIMD kernels (AVX2 on
/// x86_64, NEON on aarch64, portable scalar fallback elsewhere) — this is
/// the hottest loop in both HNSW build and every `hybrid_search` query.
fn compute_distance(a: &[f32], b: &[f32], metric: DistanceMetric) -> f32 { fn compute_distance(a: &[f32], b: &[f32], metric: DistanceMetric) -> f32 {
match metric { match metric {
DistanceMetric::L2 => { DistanceMetric::L2 => clawhdf5_accel::l2_distance(a, b),
let mut sum = 0.0f32; DistanceMetric::Cosine => 1.0 - clawhdf5_accel::cosine_similarity(a, b),
for i in 0..a.len() {
let d = a[i] - b[i];
sum += d * d;
}
sum.sqrt()
}
DistanceMetric::Cosine => {
let mut dot = 0.0f32;
let mut norm_a = 0.0f32;
let mut norm_b = 0.0f32;
for i in 0..a.len() {
dot += a[i] * b[i];
norm_a += a[i] * a[i];
norm_b += b[i] * b[i];
}
let denom = norm_a.sqrt() * norm_b.sqrt();
if denom < f32::EPSILON {
1.0
} else {
1.0 - (dot / denom)
}
}
} }
} }
+24 -8
View File
@@ -59,11 +59,16 @@ pub trait AsyncHDF5Read: Send + Sync {
/// Async file-backed reader using tokio for non-blocking I/O. /// Async file-backed reader using tokio for non-blocking I/O.
/// ///
/// Opens a file and reads it asynchronously. The file is read into memory /// Opens a file and reads it asynchronously. The underlying file handle is
/// on first access, making subsequent operations fast. /// opened once (lazily, on first access) and cached for the lifetime of this
/// reader, so repeated granular `read_at` calls reuse the open descriptor
/// and cached length instead of paying an open+stat syscall pair every time.
/// The handle is guarded by a mutex, which also correctly serializes the
/// seek-then-read pairs of concurrent callers sharing the one file position.
#[derive(Debug)] #[derive(Debug)]
pub struct AsyncFileReader { pub struct AsyncFileReader {
path: std::path::PathBuf, path: std::path::PathBuf,
handle: tokio::sync::Mutex<Option<(tokio::fs::File, u64)>>,
} }
impl AsyncFileReader { impl AsyncFileReader {
@@ -73,6 +78,7 @@ impl AsyncFileReader {
pub fn new<P: AsRef<Path>>(path: P) -> Self { pub fn new<P: AsRef<Path>>(path: P) -> Self {
Self { Self {
path: path.as_ref().to_path_buf(), path: path.as_ref().to_path_buf(),
handle: tokio::sync::Mutex::new(None),
} }
} }
@@ -89,23 +95,33 @@ impl AsyncFileReader {
impl AsyncHDF5Read for AsyncFileReader { impl AsyncHDF5Read for AsyncFileReader {
async fn read_at(&self, offset: u64, len: usize) -> io::Result<Vec<u8>> { async fn read_at(&self, offset: u64, len: usize) -> io::Result<Vec<u8>> {
let mut file = tokio::fs::File::open(&self.path).await?; let mut guard = self.handle.lock().await;
let metadata = file.metadata().await?; if guard.is_none() {
let file_len = metadata.len(); let file = tokio::fs::File::open(&self.path).await?;
let file_len = file.metadata().await?.len();
*guard = Some((file, file_len));
}
let (file, file_len) = guard.as_mut().expect("just populated above");
let file_len = *file_len;
if offset >= file_len { if offset >= file_len {
return Ok(Vec::new()); return Ok(Vec::new());
} }
let available = (file_len - offset) as usize; let available = (file_len - offset) as usize;
let to_read = len.min(available); let to_read = len.min(available);
tokio::io::AsyncSeekExt::seek(&mut file, io::SeekFrom::Start(offset)).await?; tokio::io::AsyncSeekExt::seek(file, io::SeekFrom::Start(offset)).await?;
let mut buf = vec![0u8; to_read]; let mut buf = vec![0u8; to_read];
file.read_exact(&mut buf).await?; file.read_exact(&mut buf).await?;
Ok(buf) Ok(buf)
} }
async fn len(&self) -> io::Result<u64> { async fn len(&self) -> io::Result<u64> {
let metadata = tokio::fs::metadata(&self.path).await?; let mut guard = self.handle.lock().await;
Ok(metadata.len()) if guard.is_none() {
let file = tokio::fs::File::open(&self.path).await?;
let file_len = file.metadata().await?.len();
*guard = Some((file, file_len));
}
Ok(guard.as_ref().expect("just populated above").1)
} }
} }
@@ -49,6 +49,10 @@ pub fn read_hdf5(path: &str) -> Result<SqliteData, BoxErr> {
entities, entities,
relations, relations,
embedding_dim, embedding_dim,
// Not a SQLite read — the caller (incremental migration) carries
// forward the current run's actual `source_path` from the fresh
// SQLite read instead of using this placeholder.
source_path: String::new(),
}) })
} }
+91 -5
View File
@@ -20,6 +20,7 @@ pub fn write_hdf5(
opts: &WriteOptions, opts: &WriteOptions,
) -> Result<(), Box<dyn std::error::Error>> { ) -> Result<(), Box<dyn std::error::Error>> {
let mut builder = FileBuilder::new(); let mut builder = FileBuilder::new();
let timestamp = iso8601_now();
// Root-level metadata attributes // Root-level metadata attributes
builder.set_attr("agent_id", AttrValue::String(opts.agent_id.clone())); builder.set_attr("agent_id", AttrValue::String(opts.agent_id.clone()));
@@ -27,8 +28,18 @@ pub fn write_hdf5(
builder.set_attr("embedding_dim", AttrValue::I64(data.embedding_dim as i64)); builder.set_attr("embedding_dim", AttrValue::I64(data.embedding_dim as i64));
builder.set_attr("source", AttrValue::String("sqlite-migration".into())); builder.set_attr("source", AttrValue::String("sqlite-migration".into()));
builder.set_attr("version", AttrValue::I64(1)); builder.set_attr("version", AttrValue::I64(1));
// Lineage: which SQLite database this output was migrated from and when,
// plus the migrator tool version — so a chain of `--incremental` runs
// still has an audit trail instead of every run overwriting the same
// static attributes (see research/03_provenance.md, INT-03).
builder.set_attr("source_path", AttrValue::String(data.source_path.clone()));
builder.set_attr("migrated_at", AttrValue::String(timestamp.clone()));
builder.set_attr(
"migrator_version",
AttrValue::String(env!("CARGO_PKG_VERSION").to_owned()),
);
write_chunks_group(&mut builder, data, opts); write_chunks_group(&mut builder, data, opts, &timestamp);
write_sessions_group(&mut builder, data); write_sessions_group(&mut builder, data);
write_entities_group(&mut builder, data); write_entities_group(&mut builder, data);
write_relations_group(&mut builder, data); write_relations_group(&mut builder, data);
@@ -37,6 +48,36 @@ pub fn write_hdf5(
Ok(()) Ok(())
} }
/// Current UTC time formatted as an ISO-8601 / RFC-3339 timestamp
/// (`YYYY-MM-DDTHH:MM:SSZ`), with no external date/time dependency.
fn iso8601_now() -> String {
let secs = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap_or_default()
.as_secs();
let days = (secs / 86_400) as i64;
let time_of_day = secs % 86_400;
let (h, m, s) = (time_of_day / 3600, (time_of_day % 3600) / 60, time_of_day % 60);
let (y, mo, d) = civil_from_days(days);
format!("{y:04}-{mo:02}-{d:02}T{h:02}:{m:02}:{s:02}Z")
}
/// Days-since-epoch to (year, month, day), Howard Hinnant's `civil_from_days`
/// algorithm (proleptic Gregorian calendar, valid for the full `i64` range).
fn civil_from_days(z: i64) -> (i64, u32, u32) {
let z = z + 719_468;
let era = if z >= 0 { z } else { z - 146_096 } / 146_097;
let doe = (z - era * 146_097) as u64; // [0, 146096]
let yoe = (doe - doe / 1460 + doe / 36_524 - doe / 146_096) / 365; // [0, 399]
let y = yoe as i64 + era * 400;
let doy = doe - (365 * yoe + yoe / 4 - yoe / 100); // [0, 365]
let mp = (5 * doy + 2) / 153; // [0, 11]
let d = (doy - (153 * mp + 2) / 5 + 1) as u32; // [1, 31]
let m = (if mp < 10 { mp + 3 } else { mp - 9 }) as u32; // [1, 12]
let y = if m <= 2 { y + 1 } else { y };
(y, m, d)
}
/// Build a fixed-length string Datatype from the max byte length of the items. /// Build a fixed-length string Datatype from the max byte length of the items.
fn string_dtype(max_len: usize) -> Datatype { fn string_dtype(max_len: usize) -> Datatype {
Datatype::String { Datatype::String {
@@ -66,7 +107,12 @@ fn apply_compression(ds: &mut clawhdf5_format::type_builders::DatasetBuilder, op
} }
} }
fn write_chunks_group(builder: &mut FileBuilder, data: &SqliteData, opts: &WriteOptions) { fn write_chunks_group(
builder: &mut FileBuilder,
data: &SqliteData,
opts: &WriteOptions,
timestamp: &str,
) {
let mut group = builder.create_group("chunks"); let mut group = builder.create_group("chunks");
let n = data.chunks.len() as u64; let n = data.chunks.len() as u64;
@@ -78,6 +124,16 @@ fn write_chunks_group(builder: &mut FileBuilder, data: &SqliteData, opts: &Write
group.set_attr("count", AttrValue::I64(n as i64)); group.set_attr("count", AttrValue::I64(n as i64));
// Source attribution attached directly to the content-bearing datasets
// (SHA-256 of the raw bytes + creator/timestamp/source), so the chunk
// text and embeddings each carry their own verifiable provenance
// (see clawhdf5_format::provenance / `Dataset::verify_provenance`).
let source_opt = if data.source_path.is_empty() {
None
} else {
Some(data.source_path.as_str())
};
// ids // ids
let ids: Vec<i64> = data.chunks.iter().map(|c| c.id).collect(); let ids: Vec<i64> = data.chunks.iter().map(|c| c.id).collect();
group.create_dataset("id").with_i64_data(&ids); group.create_dataset("id").with_i64_data(&ids);
@@ -87,7 +143,8 @@ fn write_chunks_group(builder: &mut FileBuilder, data: &SqliteData, opts: &Write
let (text_raw, text_len) = pack_strings(&texts); let (text_raw, text_len) = pack_strings(&texts);
group group
.create_dataset("text") .create_dataset("text")
.with_compound_data(string_dtype(text_len), text_raw, n); .with_compound_data(string_dtype(text_len), text_raw, n)
.with_provenance("clawhdf5-migrate", timestamp, source_opt);
// embeddings - flatten to [N, dim] // embeddings - flatten to [N, dim]
let dim = data.embedding_dim; let dim = data.embedding_dim;
@@ -116,7 +173,8 @@ fn write_chunks_group(builder: &mut FileBuilder, data: &SqliteData, opts: &Write
let ds = group let ds = group
.create_dataset("embeddings") .create_dataset("embeddings")
.with_compound_data(f16_dtype, raw, n) .with_compound_data(f16_dtype, raw, n)
.with_shape(&[n, dim as u64]); .with_shape(&[n, dim as u64])
.with_provenance("clawhdf5-migrate", timestamp, source_opt);
apply_compression(ds, opts); apply_compression(ds, opts);
} else { } else {
let flat: Vec<f32> = data let flat: Vec<f32> = data
@@ -127,7 +185,8 @@ fn write_chunks_group(builder: &mut FileBuilder, data: &SqliteData, opts: &Write
let ds = group let ds = group
.create_dataset("embeddings") .create_dataset("embeddings")
.with_f32_data(&flat) .with_f32_data(&flat)
.with_shape(&[n, dim as u64]); .with_shape(&[n, dim as u64])
.with_provenance("clawhdf5-migrate", timestamp, source_opt);
apply_compression(ds, opts); apply_compression(ds, opts);
} }
@@ -274,3 +333,30 @@ fn write_relations_group(builder: &mut FileBuilder, data: &SqliteData) {
builder.add_group(group.finish()); builder.add_group(group.finish());
} }
#[cfg(test)]
mod time_tests {
use super::civil_from_days;
#[test]
fn epoch_day_zero_is_1970_01_01() {
assert_eq!(civil_from_days(0), (1970, 1, 1));
}
#[test]
fn known_dates_roundtrip() {
// 2026-08-16 is 20,681 days after 1970-01-01.
assert_eq!(civil_from_days(20_681), (2026, 8, 16));
// 2000-02-29 (leap day itself) and 2000-03-01 (the day after).
assert_eq!(civil_from_days(11_016), (2000, 2, 29));
assert_eq!(civil_from_days(11_017), (2000, 3, 1));
}
#[test]
fn iso8601_now_has_expected_shape() {
let ts = super::iso8601_now();
assert_eq!(ts.len(), "2026-08-16T00:00:00Z".len());
assert!(ts.starts_with("20")); // sanity: 21st-century year
assert!(ts.ends_with('Z'));
}
}
+9
View File
@@ -154,6 +154,10 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
base.entities = source.entities; base.entities = source.entities;
base.relations = source.relations; base.relations = source.relations;
base.embedding_dim = source.embedding_dim.max(base.embedding_dim); base.embedding_dim = source.embedding_dim.max(base.embedding_dim);
// Carry the current run's real SQLite source forward for
// provenance — `base` (re-read from the prior HDF5 output) has
// no meaningful source_path of its own.
base.source_path = source.source_path;
if cli.verbose { if cli.verbose {
eprintln!("Incremental: appended {added} new chunks (id > {min_chunk_id})"); eprintln!("Incremental: appended {added} new chunks (id > {min_chunk_id})");
} }
@@ -199,6 +203,11 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
summary.embedding_dim, summary.embedding_dim,
summary.rows_checked, summary.rows_checked,
); );
if summary.provenance_verified {
eprintln!("Provenance: chunks/text and chunks/embeddings SHA-256 hashes verified.");
} else if cli.verbose {
eprintln!("Provenance: no provenance hash found to verify (older output format?).");
}
Ok(()) Ok(())
} }
@@ -51,6 +51,11 @@ pub struct SqliteData {
pub entities: Vec<Entity>, pub entities: Vec<Entity>,
pub relations: Vec<Relation>, pub relations: Vec<Relation>,
pub embedding_dim: usize, pub embedding_dim: usize,
/// Filesystem path of the SQLite database this data was read from, for
/// provenance attribution on the HDF5 output. Empty when the data did
/// not come directly from a SQLite read (e.g. re-read of a prior HDF5
/// migration output for an incremental merge).
pub source_path: String,
} }
/// A table name plus the ordered column names the reader maps by position. /// A table name plus the ordered column names the reader maps by position.
@@ -225,6 +230,7 @@ pub fn read_sqlite_filtered(
entities, entities,
relations, relations,
embedding_dim: dim, embedding_dim: dim,
source_path: path.to_owned(),
}) })
} }
+47
View File
@@ -1,3 +1,6 @@
use clawhdf5::reader::File as Hdf5File;
use clawhdf5_format::provenance::VerifyResult;
use crate::hdf5_reader::read_hdf5; use crate::hdf5_reader::read_hdf5;
use crate::sqlite_reader::SqliteData; use crate::sqlite_reader::SqliteData;
@@ -13,6 +16,12 @@ pub struct ValidationSummary {
pub embedding_dim: u64, pub embedding_dim: u64,
/// Number of rows whose full content was compared against the source. /// Number of rows whose full content was compared against the source.
pub rows_checked: u64, pub rows_checked: u64,
/// Whether the `chunks/text` and `chunks/embeddings` SHINES provenance
/// hashes (written via [`crate::hdf5_writer`]) were both present and
/// matched their recomputed SHA-256 on read-back. `false` when either
/// dataset has no provenance metadata (e.g. an older output file) or
/// there are zero chunks to check.
pub provenance_verified: bool,
} }
/// Validate a migrated HDF5 file against the source data. /// Validate a migrated HDF5 file against the source data.
@@ -30,6 +39,7 @@ pub fn validate_hdf5(
float16: bool, float16: bool,
) -> Result<ValidationSummary, BoxErr> { ) -> Result<ValidationSummary, BoxErr> {
let got = read_hdf5(path)?; let got = read_hdf5(path)?;
let provenance_verified = verify_chunk_provenance(path)?;
// ---- Counts ---- // ---- Counts ----
check_count("chunk", got.chunks.len(), source.chunks.len())?; check_count("chunk", got.chunks.len(), source.chunks.len())?;
@@ -126,6 +136,7 @@ pub fn validate_hdf5(
relations: got.relations.len() as u64, relations: got.relations.len() as u64,
embedding_dim: got.embedding_dim as u64, embedding_dim: got.embedding_dim as u64,
rows_checked, rows_checked,
provenance_verified,
}) })
} }
@@ -136,6 +147,42 @@ fn check_count(kind: &str, got: usize, expected: usize) -> Result<(), BoxErr> {
Ok(()) Ok(())
} }
/// Re-verify the SHA-256 provenance hash of `chunks/text` and
/// `chunks/embeddings` against their actual stored bytes, catching
/// post-write corruption that a plain content comparison against the
/// in-memory source wouldn't (the source is compared against what
/// `read_hdf5` decoded, not against the raw bytes on disk).
///
/// Returns `Ok(true)` only if both datasets exist and both hashes match.
/// Returns `Ok(false)` (not an error) if a dataset has no provenance
/// attributes at all (e.g. a file written before this check existed) or
/// there are zero chunks. Returns an error only on an actual hash mismatch —
/// that indicates real corruption.
fn verify_chunk_provenance(path: &str) -> Result<bool, BoxErr> {
let file = Hdf5File::open(path)?;
let Ok(chunks) = file.group("chunks") else {
return Ok(false);
};
let mut all_present = true;
for name in ["text", "embeddings"] {
let Ok(ds) = chunks.dataset(name) else {
all_present = false;
continue;
};
match ds.verify_provenance()? {
VerifyResult::Ok => {}
VerifyResult::NoHash => all_present = false,
VerifyResult::Mismatch { stored, computed } => {
return Err(format!(
"provenance hash mismatch on chunks/{name}: stored {stored}, recomputed {computed} — data may be corrupted"
)
.into());
}
}
}
Ok(all_present)
}
fn field_err<T: std::fmt::Display>(kind: &str, i: usize, field: &str, s: T, g: T) -> BoxErr { fn field_err<T: std::fmt::Display>(kind: &str, i: usize, field: &str, s: T, g: T) -> BoxErr {
format!("{kind}[{i}].{field} mismatch: source {s}, HDF5 {g}").into() format!("{kind}[{i}].{field} mismatch: source {s}, HDF5 {g}").into()
} }
+17
View File
@@ -426,6 +426,23 @@ impl<'f> Dataset<'f> {
Ok(data_read::read_as_strings(&raw, &dt)?) Ok(data_read::read_as_strings(&raw, &dt)?)
} }
/// Verify this dataset's SHINES provenance hash (the `_provenance_sha256`
/// attribute written by [`clawhdf5_format::type_builders::DatasetBuilder::with_provenance`])
/// against its actual stored bytes.
///
/// Returns [`clawhdf5_format::provenance::VerifyResult::NoHash`] if the
/// dataset was never written with provenance metadata. Requires the
/// `provenance` Cargo feature on `clawhdf5-format` (enabled by default).
#[cfg(feature = "provenance")]
pub fn verify_provenance(&self) -> Result<clawhdf5_format::provenance::VerifyResult, Error> {
Ok(clawhdf5_format::provenance::verify_dataset(
self.file.data.as_bytes(),
&self.header,
self.file.offset_size(),
self.file.length_size(),
)?)
}
// ----- Selection-based read methods ----- // ----- Selection-based read methods -----
/// Read selected elements as raw bytes. /// Read selected elements as raw bytes.
+55
View File
@@ -0,0 +1,55 @@
# Research: Performance — clawhdf5
Scope: opportunities not already covered by the Tier 1-4 hardening passes
recorded in `ROADMAP.md`/`CHANGELOG.md`/`IMPROVEMENT_LOG.md` (O(1) chunk
cache, rayon-parallel `prune_connections`, workspace-hoisted deps, etc).
## Finding P1 — HNSW's hot distance loop is scalar despite an existing SIMD crate
**Location:** `crates/clawhdf5-ann/src/hnsw.rs:47-74` (`compute_distance`), called
from `greedy_closest` and `search_layer` — the innermost loop of both index
build and every `hybrid_search` query.
**Problem:** `compute_distance` is a plain per-component `for i in 0..a.len()`
scalar loop for both the `L2` and `Cosine` metrics. The workspace already ships
`clawhdf5-accel` with runtime-dispatched AVX2/NEON/scalar-fallback
`l2_distance`/`cosine_similarity` (`crates/clawhdf5-accel/src/lib.rs:125,148`),
and `clawhdf5-agent` already depends on and uses it for its own linear cosine
scan. `clawhdf5-ann/Cargo.toml` simply never lists `clawhdf5-accel` as a
dependency, so the ANN crate — the one place with the tightest, most-called
distance loop in the whole codebase — is the one place not using it.
**Fix implemented (INT-01):** Added `clawhdf5-accel` as a dependency of
`clawhdf5-ann` and rewired `compute_distance` to call
`clawhdf5_accel::l2_distance` / `clawhdf5_accel::cosine_similarity` (mapping
`1.0 - similarity` for the cosine-distance semantics the rest of the file
expects). The accel crate already carries its own scalar fallback for
platforms without AVX2/NEON, so no separate fallback branch is needed here.
Existing `hnsw.rs` unit tests (build/search/serialize round-trip) validate
behavior is unchanged; no format or public-API change.
## Finding P2 — `AsyncFileReader::read_at` reopens and re-stats the file on every call
**Location:** `crates/clawhdf5-io/src/async_read.rs:90-104`.
**Problem:** Each `read_at` call does `tokio::fs::File::open` +
`.metadata()` + `seek` + `read_exact` — two extra syscalls (open + stat) on
every single granular read, with no persistent handle and no buffering. This
directly defeats the purpose of the "chunked/granular async access" this type
is documented for; callers doing many small reads (e.g. chunked dataset
iteration) pay file-open overhead per chunk.
**Fix implemented (INT-02):** `AsyncFileReader` now lazily opens the file
once and caches the open handle (plus its length) behind a `tokio::sync::Mutex`,
so subsequent `read_at`/`len` calls reuse the already-open descriptor instead
of reopening. First call pays one open+stat; every call after is just a
seek+read (or a length lookup with no syscall at all, since length is cached
at open time). Behavior (including short-read truncation semantics) is
unchanged and covered by the existing `async_file_reader_*` tests.
## Not implemented — flagged for follow-up
- **HNSW build-loop parallelism** (`hnsw.rs` insert loop) — ROADMAP already
notes this needs its own correctness-sensitive design pass (insert order
affects the graph, unlike `prune_connections`'s embarrassingly-parallel
per-node distance computation). Left as-is; out of scope for this pass.
+42
View File
@@ -0,0 +1,42 @@
# Research: Security — clawhdf5
Scope: opportunities not already covered by the shipped hardening (WAL CRC32
trailer / `WAL_VERSION` 2, `MAX_WAL_FIELD_LEN` field caps, Android JNI length
validation, `chunked_read.rs`/`data_read.rs` bounds-check + fuzz pass,
decompression-bomb output bound, etc — see `ROADMAP.md`).
## Finding S1 — WAL v2 still allocates untrusted field buffers before the CRC32 check runs
**Location:** `crates/clawhdf5-agent/src/wal.rs`, entry read path
(`read_len_prefixed_str`/`read_embedding` helpers feeding into the `Save`
entry parser around lines 340-380; CRC verification happens afterward at
~lines 246-255).
**Problem:** Each `Save` entry currently contains three independent
length-prefixed strings plus one length-prefixed embedding buffer. Each field
is capped individually at `MAX_WAL_FIELD_LEN` (64 MiB) — but that cap is
checked and then the buffer is **allocated immediately** as each field's
length prefix is read, before the entry's trailing CRC32 is ever checked. A
single corrupted entry (bit-flipped length prefixes) can therefore force up
to ~4 allocations near 64 MiB each (~256 MB) before the CRC finally rejects
it. This is exactly what `ROADMAP.md`'s "What's Next" section already flags
as open: *"a stronger per-entry format (explicit length prefix, avoiding the
read-then-verify restructuring) could still be revisited."*
**Why not implemented in this pass:** Fixing this properly means a WAL format
version bump (`WAL_VERSION` 3): frame each entry as one outer
`[total_len: u32][entry_bytes][crc32: u32]`, read+CRC-check the whole raw
entry buffer *first*, and only then parse the individual fields out of the
already-verified buffer — mirroring the v1→v2 migration this file already
does on open. That's a real, self-contained, well-testable change (the file
already has a legacy-format migration test harness and corruption-detection
tests to extend), but it touches the on-disk framing and the read/write pair
needs to stay in lock-step, so it deserves its own dedicated
implement-and-test pass rather than being bundled in alongside unrelated
performance/provenance changes. Tracked as **INT-04** below for follow-up.
## Finding S2 — no dataset-level integrity check on the agent memory read path
See `research/03_provenance.md` finding PR2 (`INT-06`) — closely related to
security (corruption detection on read), tracked there since the mechanism
(`ProvenanceStore::verify_integrity`) is a provenance primitive.
+97
View File
@@ -0,0 +1,97 @@
# Research: Provenance — clawhdf5
Scope: data lineage, source attribution, and tamper-evidence for both the
low-level HDF5 format layer and the higher-level agent-memory / migration
tools built on top of it.
## Finding PR1 — SHINES provenance (SHA-256 + creator/timestamp/source) is fully built and tested, but zero production write paths use it
**Location:** `crates/clawhdf5-format/src/provenance.rs` (the whole module —
`Provenance::build_attrs`, `sha256_hex`, `verify_dataset`) and
`crates/clawhdf5-format/src/type_builders.rs:671-686`
(`DatasetBuilder::with_provenance`, feature-gated on `provenance`, which is
**on by default** in `clawhdf5-format`).
**Problem:** This is a complete, working, already-tested feature — it writes
`_provenance_sha256` / `_provenance_creator` / `_provenance_timestamp` /
`_provenance_source` attributes on a dataset and can re-verify the hash later
via `verify_dataset`. `grep -rl with_provenance crates/` shows it is
exercised only by `clawhdf5-format`'s own tests/benches
(`tests/robustness_tests.rs`, `tests/writer_h5py_tests.rs`,
`benches/bench.rs`). Neither `clawhdf5-agent` (the memory backend) nor
`clawhdf5-migrate` (the SQLite→HDF5 migration tool — the one place data
crosses a genuine trust/source boundary) calls it. Concretely,
`crates/clawhdf5-migrate/src/hdf5_writer.rs:24-28` sets only a handful of
static root attributes (`agent_id`, `embedder`, `embedding_dim`, a *constant*
`source="sqlite-migration"`, a *constant* `version=1`) — there is no source
file path, no content hash of the source database, no migration timestamp,
and `--incremental` runs (`main.rs` ~122-133) overwrite these same static
attributes on every append, so a chain of incremental merges leaves no audit
trail: a corrupted incremental append is indistinguishable after the fact
from a clean one.
**Fix implemented (INT-03):** Wired the *existing* SHINES provenance
mechanism into the migration write path instead of inventing a new one:
- `clawhdf5-migrate/src/hdf5_writer.rs`: the `embeddings` and `text` chunk
datasets are now built with `.with_provenance("clawhdf5-migrate", <RFC3339
timestamp>, Some(<source sqlite path>))`, so each migrated dataset carries
a verifiable SHA-256 of its own bytes plus who/when/where it came from.
- `clawhdf5-migrate/src/sqlite_reader.rs`: `SqliteData` gained a
`source_path: String` field (the SQLite path actually read), threaded
through `read_sqlite_filtered`.
- `clawhdf5-migrate/src/main.rs`: the incremental-merge arm now carries the
*current* run's `source_path` forward instead of silently keeping
whatever the previous run recorded.
- `clawhdf5-migrate/src/validate.rs`: `validate_hdf5` now also calls
`clawhdf5_format::provenance::verify_dataset` on the embeddings dataset and
fails validation on a hash mismatch, so migration validation catches
post-write corruption, not just source/dest content drift.
This directly closes the exact gap ROADMAP's "What's Next" implicitly left
open (migration recorded no real lineage) using code that was already
shipped, tested, and sitting unused one crate over — no new format version,
no new dependency, minimal blast radius (2 struct-literal sites for the new
`SqliteData` field, both updated).
## Finding PR2 — agent-level `MemoryProvenance`/`AnomalyDetector` are dead code on the real save path (ROADMAP claims Track 5 "complete")
**Location:** `crates/clawhdf5-agent/src/lib.rs` (`HDF5Memory::save` /
`save_batch`, ~lines 538-572); `crates/clawhdf5-agent/src/provenance.rs`
(`MemoryProvenance`, `ProvenanceStore::verify_integrity`/`mark_verified`);
`crates/clawhdf5-agent/src/anomaly.rs` (`AnomalyDetector::check_rate_anomaly`
/ `check_pattern_anomaly` / `check_source_anomaly`).
**Problem:** `ROADMAP.md` Track 5 ("Memory Security & Provenance") is marked
🟢 Complete, but `save()`/`save_batch()` push straight into the in-memory
cache + WAL without ever constructing a `MemoryProvenance` record, without
ever calling any `AnomalyDetector` check, and without going through
`SourceIsolation`. A `grep` for `provenance::`/`anomaly::` usage across the
crate turns up only each module's own `#[cfg(test)]` block. So today a
forged- or poisoned-source memory write is stored and later retrieved with
zero attribution and zero anomaly screening, contradicting the shipped-status
claim in the docs.
**Why not implemented in this pass:** This is a real fix, but it is
core-save-path surgery — it has to interact correctly with the WAL replay
path (a provenance record written to cache but not WAL, or vice versa, would
silently desync memory from the durable log on crash-recovery) and with
`save_batch`'s different code path from `save`. That needs its own focused
implement-and-test pass with the existing `provenance.rs`/`anomaly.rs` unit
tests as a base, rather than being bundled in under time pressure alongside
unrelated changes. Tracked as **INT-05** below.
## Finding PR3 — nothing on the retrieval path ever calls `verify_integrity`
**Location:** `crates/clawhdf5-agent/src/provenance.rs:128`
(`ProvenanceStore::verify_integrity`), vs. `search.rs`/`hybrid.rs` (no
callers).
**Problem:** Even independent of PR2, nothing in the retrieval pipeline
calls `verify_integrity` before returning a chunk to the caller, so
corruption of stored chunk text is retrievable and usable without any check
ever running.
**Why not implemented in this pass:** Blocked on PR2/INT-05 landing first —
`verify_integrity` needs a `MemoryProvenance` record to check *against*, and
none are currently produced. Tracked as **INT-06**, sequenced after INT-05.