Files
clawhdf5/research/IMPLEMENTATION_BRIEF.md
Omar Sobh 122849b5a9 research: add implementation brief with 17 numbered INT items
Covers performance, security, and provenance findings across
clawhdf5-format, clawhdf5-migrate, and memory/query crates. Each item
lists target file, problem, and proposed change for the coding phase.
2026-08-17 00:22:21 +00:00

20 KiB
Raw Permalink Blame History

Implementation Brief — Performance, Security & Provenance

Phase: Research Date: 2026-08-17 Scope: clawhdf5 Rust workspace (/mission/repo)

Method

Read ROADMAP.md, IMPROVEMENT_LOG.md, CLAUDE.md, CHANGELOG.md, and recent git log before scoping this brief, to avoid re-proposing work already merged. The repo has already been through several hardening passes (Tier 14, see CHANGELOG.md "Unreleased" section and the git log entries tagged security:/perf:): bounds-check audits on chunked_read.rs/data_read.rs/ local_heap.rs/btree_v1.rs, MAX_DECOMPRESS_SIZE output caps, WAL v2 per-entry CRC32, Android JNI length validation, pyo3 bump, O(1) chunk-cache lookup with Arc-shared buffers, and optional rayon parallelism for HNSW prune_connections. None of that is re-proposed here.

Four focused audits were run against the areas those passes did not cover: (1) the HDF5 binary parser files outside the already-audited set, plus clawhdf5-accel/clawhdf5-gpu unsafe code; (2) clawhdf5-agent's query-time hot paths (search/rerank/consolidation/knowledge graph); (3) the provenance/anomaly-detection subsystem end-to-end; (4) error handling in clawhdf5-io, clawhdf5-migrate, clawhdf5-py, and the clawhdf5 facade.

clawhdf5-accel (SIMD dispatch), clawhdf5-gpu (no unsafe code, wgpu-mediated), clawhdf5-io, clawhdf5-py, and the clawhdf5 facade crate were all found already sound for the failure modes investigated — no items proposed for those beyond what's listed below. Say so once here rather than padding the list with manufactured items.


Section A — Parser crash safety (crafted-file DoS)

These three files use raw offset + N > file_data.len() arithmetic instead of the checked_add-based ensure_len helper that every other parser in clawhdf5-format already uses (established pattern: btree_v2.rs, global_heap.rs, fractal_heap.rs, shared_message.rs, local_heap.rs's own ensure_len, etc.). On a crafted file with an address field close to u64::MAX, the addition overflows — panicking in debug builds, silently wrapping in the release profile (no overflow-checks set anywhere in the workspace Cargo.toml), after which the bounds check passes falsely and the next slice operation panics anyway. Net effect either way: a crafted file crashes the parser instead of returning Err.

INT-01 — crates/clawhdf5-format/src/fixed_array.rs, crates/clawhdf5-format/src/extensible_array.rs

Problem: Six unguarded-addition bounds checks: FixedArrayHeader::parse (fixed_array.rs:69), the data-block header check in read_fixed_array_chunks (fixed_array.rs:129), ExtensibleArrayHeader::parse (extensible_array.rs:101), read_extensible_array_data_block (extensible_array.rs:278), the index-block parse (extensible_array.rs:429), and the super-block parse (extensible_array.rs:630). The offending offsets (data_block_address/index_block_address) come from DataLayout::parse (data_layout.rs, chunk_index_type 3/4 branches, ~lines 460470), which only special-cases the exact all-0xFF sentinel via is_undefined — any other near-max value passes through unchanged. Change: Replace every raw offset + N > file_data.len() in both files with the checked_add-based ensure_len pattern already used elsewhere in the crate (e.g. mirror local_heap.rs's ensure_len).

INT-02 — crates/clawhdf5-format/src/symbol_table.rs

Problem: SymbolTableNode::parse (line 83) uses raw offset + 8 > file_data.len(), unlike read_offset in the same file which already uses checked_add. offset is a SNOD address taken verbatim from a v1 B-tree leaf entry and passed straight through by group_v1.rs:49 with no sentinel/range check — a crafted v1-group B-tree leaf with a near-u64::MAX child pointer overflows the check the same way as INT-01. Change: Use offset.checked_add(8) (ensure_len pattern) at line 83. Note: the entries_start + num_symbols * entry_size addition at line 106 has the same raw-arithmetic style, but num_symbols is u16 so the multiply itself can't overflow — lower priority, but worth fixing for consistency in the same pass.

INT-03 — crates/clawhdf5-format/src/datatype.rs

Problem: Datatype::parse recurses into itself with no depth counter (grep -n "depth" datatype.rs — zero hits) for Compound members (lines 361, 387), Enumeration base type (line 418), VariableLength base type (line 471), and Array base type (lines 497, 518). A message data size is capped at u16::MAX (65535 bytes; see object_header.rs:141 v1, object_header.rs:411 v2), so a crafted Compound-of-Compound-of-Compound... datatype message can nest ~8000 levels deep — enough to blow the stack, and materially worse on the project's documented no_std/embedded targets (thumbv7em-none-eabihf, per CHANGELOG.md) where available stack is a few KB. The changelog records this exact class of bug already fixed for the N-Bit filter's type tree, but that fix was never applied to the general Datatype::parse reader used for every Dataspace/Attribute/Dataset datatype message. Change: Thread a depth: u16 counter through Datatype::parse's recursive call sites (mirror object_header.rs's continuation-depth guards) and return a new FormatError::NestingDepthExceeded past a fixed limit (suggest 64).


Section B — Provenance & anomaly detection

The most significant finding of this brief: the provenance/anomaly subsystem exists and is tested, but is never invoked from the real save/load path. It's a fully-built, unused API surface, not an active control.

INT-04 — crates/clawhdf5-agent/src/provenance.rs, crates/clawhdf5-agent/src/anomaly.rs, crates/clawhdf5-agent/src/lib.rs

Problem: ProvenanceStore, MemoryProvenance::new, verify_integrity, mark_verified, WriteAnomalyDetector, record_write, check_pattern_anomaly, check_rate_anomaly, check_source_anomaly have zero callers outside their own module/tests. lib.rs only declares pub mod provenance; / pub mod anomaly; (lines 22, 33) — neither is referenced from HDF5Memory::save_or_update (~line 495) or the WAL replay path (wal.rs::replay_into_cache, line 311). Concretely: the 15 injection-pattern checks, rate limiting, and content-hash integrity verification described as shipped in ROADMAP.md Track 5 never execute during normal library usage today. Change: Call ProvenanceStore::add and WriteAnomalyDetector::record_write + the check_* methods from HDF5Memory::save_or_update, and call verify_integrity from the open/load path (surfacing a mismatch to the caller, not panicking). If the intent is genuinely opt-in-only, that's a legitimate design choice, but it must be documented prominently at the crate root / in CLAUDE.md — right now it reads as an active control and isn't one.

INT-05 — crates/clawhdf5-agent/src/lib.rs (MemoryEntry.source_channel, ~line 167), crates/clawhdf5-agent/src/consolidation.rs (ConsolidationEngine::add_memory, ~line 205)

Problem: source_channel: String is free text set entirely by the caller of save/save_or_update — nothing validates it against an allowlist, so a write can claim source_channel = "system" or any other privileged-looking label. Separately, add_memory takes source: MemorySource (User/System/Tool/Retrieval/Correction) as a plain parameter; MemorySource::Correction/System get elevated importance weighting in score_correction (~line 133), so any caller can claim a trust level the content doesn't warrant. Change: Derive MemorySource/source_channel at the actual trust boundary (the ingestion layer that knows the true origin), not as a caller-supplied argument to the storage API. At minimum, gate MemorySource::System/Correction construction behind a distinct constructor not exposed to the same call path as untrusted content.

INT-06 — crates/clawhdf5-agent/src/anomaly.rs (check_pattern_anomaly, ~lines 192195)

Problem: Matching is chunk.to_lowercase().contains(pattern.as_str()) — plain literal-substring test after case folding only. Inserting any character inside a pattern (extra whitespace, a zero-width character, . between letters) or substituting a homoglyph for one Latin letter defeats every one of the 15 injection patterns; there's no Unicode confusable-normalization or punctuation/whitespace stripping. Change: Normalize input before matching (strip zero-width characters and punctuation, apply NFKC + confusable-folding) or switch to fuzzy/token-based detection instead of raw contains.

INT-07 — crates/clawhdf5-agent/src/anomaly.rs (check_rate_anomaly, ~lines 149151)

Problem: The per-minute rate check uses a single global sliding window (self.window.len()) across all sessions/sources combined. One noisy session can trip the shared window without the alert naming the offending session (unlike the separate cumulative max_writes_per_session check, which does name it); conversely, many distinct low-volume sessions can jointly flood the shared window without any individual one tripping its own per-session limit. Change: Key the sliding window by session/source (or add a per-source rolling count) so the rate check attributes to, and can throttle, the actual offender.

INT-08 — crates/clawhdf5-format/src/provenance.rs (verify_dataset, ~line 126)

Problem: The SHA-256 content hash is written automatically on save when db.provenance is set (file_writer.rs ~10611068, gated on the provenance feature), but verify_dataset is only ever called from test files — no reader/open path in clawhdf5-io or the clawhdf5 facade calls it. A corrupted dataset is silently readable with no automatic integrity check; the write-side machinery exists but nothing consumes it. (Note: CHANGELOG.md already documents that this hash is unkeyed/tamper-evident not tamper-proof — that's accepted and not re-flagged here; this item is about it never being invoked at all, not about its cryptographic strength.) Change: Optionally call verify_dataset on dataset open (behind the provenance feature) and surface a mismatch as a typed error/warning to the caller instead of leaving verification purely opt-in/manual.

INT-09 — crates/clawhdf5-agent/src/wal.rs (WalFile::read_entries, ~lines 219272)

Problem: Two related gaps. (a) WAL v2's per-entry CRC32 covers only each entry's own bytes — there's no sequence number or entry-chaining, so entries could be reordered, duplicated, or spliced (e.g. a Tombstone moved before/after its target Save) while every individual entry still passes its own CRC check, silently changing replayed cache state. (b) The WAL_VERSION_LEGACY_NO_CRC branch (~lines 260266) does no CRC verification at all, and the version byte itself is a single unauthenticated byte — since read_entries is a public standalone API (not just reached via open()'s one-time migrate-on-read), flipping that byte from 2 to 1 silently downgrades every subsequent entry in the file to the fully-unverified pre-hardening parser. Change: Add a monotonic sequence number or entry-chaining (CRC/hash including the previous entry's CRC) to detect reordering/splicing. Restrict the legacy-no-CRC branch to the open() migration path only, or emit a warning when read_entries falls back to it via any other entry point.


Section C — Correctness bug (panic on valid, untrusted input)

INT-10 — crates/clawhdf5-migrate/src/validate.rs (truncate, lines 143149)

Problem:

fn truncate(s: &str) -> String {
    if s.len() <= 40 {
        s.to_string()
    } else {
        format!("{}…", &s[..40])   // byte-index slice, not char-boundary safe
    }
}

s is source.chunk — arbitrary UTF-8 text read from the source SQLite database, called from the chunk-text mismatch branch of validate_hdf5 (~line 58) whenever migrated text doesn't exactly match the source. This is the default (non---dry-run) validation path, not test-only code — the file has no #[cfg(test)] block. If a multi-byte character (emoji, accented letter, CJK, etc.) straddles byte offset 40, &s[..40] panics with "byte index 40 is not a char boundary" instead of producing the diagnostic the code exists to report. Change: Truncate on a char boundary, e.g. let cut = s.char_indices().nth(40).map(|(i, _)| i).unwrap_or(s.len()); format!("{}…", &s[..cut]).


Section D — Performance (query-time hot paths, clawhdf5-agent)

search.rs, vector_search.rs, hybrid.rs, reranker.rs, confidence.rs, temporal.rs, ivf.rs, pq.rs, and gpu_search.rs were reviewed and found already efficient (temporal index uses partition_point binary search, hybrid merge uses HashMap accumulation not nested loops, no gratuitous clones in the batch vector paths) — no items proposed there.

INT-11 — crates/clawhdf5-agent/src/bm25.rs (BM25Index::search, ~lines 118141)

Problem: The WAND top-k threshold update calls top_k_scores.sort_by(...) over the full k-sized buffer for every matching document that beats the running threshold (twice in the >= k branch), plus another full sort on reaching exactly k results. For m matching documents this is O(m·k log k) where a heap gives O(m log k). Change: Replace top_k_scores: Vec<f32> with a min-heap (BinaryHeap<Reverse<f32>>) of size k; pop/push instead of sort-and-index.

INT-12 — crates/clawhdf5-agent/src/knowledge.rs (KnowledgeCache::resolve_or_create, lines 304330)

Problem: self.entities.iter().map(|e| levenshtein(&lower_name, &e.name.to_lowercase())) allocates a fresh lowercased String for every entity on every resolution call (this runs per extracted mention during entity/relation extraction) and never short-circuits even on an exact dist == 0 match — it scores every remaining entity regardless. Change: Cache a lowercased name on Entity to avoid the per-call allocation, and break out of the scan as soon as a dist == 0 match is found.

INT-13 — crates/clawhdf5-agent/src/knowledge.rs (bfs_neighbors lines 339378, spreading_activation lines 435495, get_relations_from/get_relations_to lines 247254)

Problem: All four functions filter/scan the entire self.relations list per node processed (O(V·E) for BFS instead of O(V+E); O(max_steps · active_nodes · relations) for spreading activation), and bfs_neighbors additionally calls self.get_entity(neighbour_id) per discovered neighbor, itself an O(n) linear .find() over self.entities. Change: Build (or maintain incrementally on add_entity/add_relation) a HashMap<u64, Vec<usize>> adjacency index and a HashMap<u64, usize> id→index map, shared across all four functions, replacing the linear scans with O(1)/O(degree) lookups.

INT-14 — crates/clawhdf5-agent/src/consolidation.rs (ConsolidationEngine::add_memory, lines 212217)

Problem:

let working: Vec<MemoryRecord> = self.records.iter()
    .filter(|r| r.tier == MemoryTier::Working)
    .cloned()
    .collect();

score_surprise (the only consumer) only reads r.embedding by reference — the full clone (chunk text + embedding Vec<f32>) of every working-tier record is discarded immediately after use. Change: Collect Vec<&MemoryRecord> (or iterate the filtered self.records directly, passing an iterator of &[f32]) instead of .cloned().

INT-15 — crates/clawhdf5-agent/src/consolidation.rs (consolidate, lines 284291 and 345351)

Problem: self.records.retain(|r| !evict_ids.contains(&r.id)) where evict_ids: Vec<u64>retain calls .contains() (linear scan) for every record in self.records, giving O(n·m) cost (n = records, m = eviction count) on both the Working-tier eviction (line 289) and Episodic-tier eviction (line 350), on every consolidation tick. Change: Build evict_ids as a HashSet<u64> for O(1) membership checks.

INT-16 — crates/clawhdf5-agent/src/blas_search.rs (blas_cosine_batch, lines 3039), crates/clawhdf5-agent/src/accelerate_search.rs (accelerate_cosine_batch_vecs, lines 164173)

Problem: cache.embeddings is stored as Vec<Vec<f32>>; both functions re-flatten the entire corpus into a fresh Vec<f32> (flat.extend_from_slice(&vectors[i]) per non-tombstoned vector) on every single query before running the actual BLAS/Accelerate matmul — an O(N·dim) copy paid per query when the fast-math feature is enabled. The fix pattern already exists in-file: blas_cosine_batch_flat (same file, lines 89142) has an all_active fast path that skips this copy when reading from a pre-flattened buffer directly — it's just not used for the Vec<Vec<f32>> call sites. Change: Maintain a persistent flat embedding buffer alongside cache.embeddings (updated incrementally on insert/delete) and call blas_cosine_batch_flat instead of blas_cosine_batch from both files' query paths.

INT-17 — crates/clawhdf5-agent/src/entity_extract.rs (dedup_overlapping, lines 302313)

Problem: result.iter().any(|existing| ...) checks every candidate entity against all already-accepted entities — O(n²) in entities-per-extraction-call. This runs at ingestion time (every memory save), not query time, and is bounded by entities-per-chunk (typically small), so it's lower priority than INT-11 through INT-16. Change: If profiling shows this matters in practice (large chunks with many extracted entities), replace with a spatial/interval-based overlap index; otherwise leave as-is — flagging for completeness, not urgency.


Summary table

INT Area File(s) Category
INT-01 Parser crash safety fixed_array.rs, extensible_array.rs Security
INT-02 Parser crash safety symbol_table.rs Security
INT-03 Parser crash safety datatype.rs Security
INT-04 Provenance wiring provenance.rs, anomaly.rs, lib.rs Provenance
INT-05 Source trust boundary lib.rs, consolidation.rs Provenance
INT-06 Anomaly pattern bypass anomaly.rs Provenance
INT-07 Rate-limit attribution anomaly.rs Provenance
INT-08 Integrity verification unwired clawhdf5-format/provenance.rs Provenance
INT-09 WAL ordering/legacy fallback wal.rs Provenance
INT-10 Char-boundary panic clawhdf5-migrate/validate.rs Correctness
INT-11 WAND top-k re-sort bm25.rs Performance
INT-12 Entity resolution scan knowledge.rs Performance
INT-13 Graph traversal scan knowledge.rs Performance
INT-14 Unneeded clone consolidation.rs Performance
INT-15 O(n·m) eviction consolidation.rs Performance
INT-16 Per-query re-flatten blas_search.rs, accelerate_search.rs Performance
INT-17 O(n²) dedup (low priority) entity_extract.rs Performance

Follow-ups for the coding phase

TASK: INT-01 — Fix unchecked-overflow bounds checks in fixed_array.rs/extensible_array.rs TASK: INT-02 — Fix unchecked-overflow bounds check in symbol_table.rs TASK: INT-03 — Add recursion-depth guard to Datatype::parse TASK: INT-04 — Wire provenance.rs/anomaly.rs into save/load path TASK: INT-05 — Enforce source-of-truth for MemorySource/source_channel at trust boundary TASK: INT-06 — Harden anomaly pattern matching against whitespace/homoglyph bypass TASK: INT-07 — Make anomaly rate-limit window per-source TASK: INT-08 — Wire clawhdf5-format provenance verify_dataset into read path TASK: INT-09 — Add WAL entry ordering protection and restrict legacy no-CRC fallback TASK: INT-10 — Fix byte-index slice panic in clawhdf5-migrate validate.rs truncate() TASK: INT-11 — Replace BM25 top-k re-sort with a min-heap TASK: INT-12 — Cache lowercased entity names and early-exit in resolve_or_create TASK: INT-13 — Add adjacency index for knowledge graph traversal functions TASK: INT-14 — Avoid cloning working-tier records in consolidation add_memory TASK: INT-15 — Use HashSet for eviction ID membership checks in consolidation TASK: INT-16 — Use persistent flat embedding buffer in blas_search/accelerate_search TASK: INT-17 — (optional/low-priority) revisit entity_extract dedup_overlapping if profiling shows it matters