clawhdf5/research/IMPLEMENTATION_BRIEF.md

# 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 1–4, 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 460–470), 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 192–195)
**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 149–151)
**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` ~1061–1068, 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 219–272)
**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 260–266) 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 143–149)
**Problem:**
```rust
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 118–141)
**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 304–330)
**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 339–378, `spreading_activation` lines 435–495, `get_relations_from`/`get_relations_to` lines 247–254)
**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 212–217)
**Problem:**
```rust
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 284–291 and 345–351)
**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 30–39), `crates/clawhdf5-agent/src/accelerate_search.rs` (`accelerate_cosine_batch_vecs`, lines 164–173)
**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 89–142) 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 302–313)
**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