clawmates: phase work

Mission: 01a00bbb-a6a1-7ae3-8024-2c57538ee242
Phase: 01a00bbb-a6a2-7a32-8ab6-5effd8d99218

Committed by the ClawMates delivery pipeline from the agents' working tree. Authored by agents, not by the named committer.
This commit is contained in:
Omar Sobh
2026-08-16 18:27:43 +00:00
parent 339a5bd06a
commit 167671fd79
2 changed files with 361 additions and 0 deletions
+282
View File
@@ -0,0 +1,282 @@
# ClawHDF5 Performance, Security & Provenance Refactor — Implementation Brief
## Overview
ClawHDF5 is a pure-Rust HDF5 implementation with 16 crates covering read/write, compression filters, GPU acceleration, vector search (HNSW), Python/Node.js bindings, Android JNI, and CLI tooling. The codebase builds, tests pass (18+ passing test suites), and performance benchmarks are comprehensive and reproducible.
**Baseline state:**
- 144 total `unsafe` blocks across the workspace
- 120+ `unwrap()` calls in main `clawhdf5` crate
- 63 `panic!()` invocations in the codebase
- ~500 dependencies (locked versions with some drift from latest)
- Test coverage: 78+ tests passing; zero failures
---
## Performance Optimization Opportunities
### INT-01: Zero-Copy Reader Safety & Alignment Audit
**Issue:** Five `unsafe { slice::from_raw_parts() }` calls in `reader.rs` for zero-copy access (f64, f32, i32, i64).
- **Risk:** Unvalidated alignment assumptions could cause undefined behavior if caller provides misaligned pointers
- **Impact:** These are in hot paths for large dataset reads (100K+ element reads shown in benchmarks)
- **Recommendation:** Wrap unsafe blocks in helper functions that validate alignment, byte order (native-endian only), and contiguity before construction
- **Acceptance:** All zero-copy reads validate preconditions; error types distinguish alignment failure from other reasons
- **Effort:** Medium (add invariant checks, no algorithmic changes)
**Related:** `src/reader.rs` lines 150-200 (estimated, zero-copy methods)
---
### INT-02: Panic Surface Reduction
**Issue:** 120 `unwrap()` calls in `clawhdf5` crate alone; 63 `panic!()` across workspace.
- **Risk:** User-provided data or malformed files can trigger panics, crashing the process instead of returning errors
- **Impact:** Production servers reading untrusted HDF5 files from cloud storage, streaming APIs, or user uploads could be DoS'd
- **Recommendation:** Audit the top 30 `unwrap()`s by call frequency (many are in test code). Convert filesystem/parsing operations to `?` or explicit error handling. Leave only truly unreachable panics (e.g., `expect()` on invariant violations after validation)
- **Acceptance:** Zero panics on malformed input; panics only on violated internal invariants (clearly documented)
- **Effort:** LowMedium (grep + mechanical edits, no structural changes)
**Files to audit:**
- `crates/clawhdf5/src/reader.rs` (dataset construction)
- `crates/clawhdf5/src/writer.rs` (file finalization)
- `crates/clawhdf5-format/src/*.rs` (binary parsing — most critical)
---
### INT-03: Dependency Version Alignment & Security Audit
**Issue:** Cargo.lock shows outdated transitive versions: `criterion` 0.5.1 (latest 0.8.2), `lz4_flex` 0.11.6 (latest 0.14.0), `napi` 2.16.17 (latest 3.12.1).
- **Risk:** Known CVEs in old versions; RUSTSEC advisories for compression codecs
- **Impact:** Supply chain compromise vectors, especially in Python/Node.js bindings (PyO3, napi-sys)
- **Recommendation:** Run `cargo audit`, pin critical deps (SHA2, cryptographic codecs) to latest patched versions, test compatibility
- **Acceptance:** Zero RUSTSEC warnings; all deps ≤2 minor versions behind latest (acceptable for stable APIs)
- **Effort:** Low (update Cargo.toml, regression test; CI integration)
**Critical crates to prioritize:**
- `sha2` (v0.10.9 → v0.11.0) — provenance signing
- `flate2`, `zstd`, `lz4_flex` — decompression attack surface
- `pyo3` / `napi-sys` — FFI boundary security
---
### INT-04: Unsafe Code Audit & Quantification
**Issue:** 144 total `unsafe` blocks; 5 in hot zero-copy path, others in FFI (libaec-sys), SIMD acceleration (clawhdf5-accel), and GPU bindings (clawhdf5-gpu).
- **Risk:** Unvalidated invariants in unsafe code can cause segfaults, data corruption, or privilege escalation (especially in JNI/GPU contexts)
- **Impact:** Crashes when reading malformed files; undefined behavior if WGSL shaders or SIMD code mishandle array bounds
- **Recommendation:**
1. Generate unsafe code audit report (file, line, justification)
2. Add `#![forbid(unsafe_code)]` in low-risk crates (`clawhdf5-derive`, `clawhdf5-cli`)
3. Add `#![deny(unsafe_code)]` in higher-risk crates, with documented exceptions
4. Verify libaec-sys (szip) unsafe calls match upstream C lib signatures (use bindgen for correctness)
- **Acceptance:** All unsafe blocks documented with SAFETY comments; audit trail in comments
- **Effort:** Medium (audit + documentation; no code changes unless violations found)
---
### INT-05: CRC32 Fast-Path Checksum Validation
**Issue:** `fast-checksum` feature uses `crc32fast` instead of default SHA2-based checksums.
- **Risk:** CRC32 is not cryptographically secure; may fail to detect bit flips in adversarial scenarios
- **Impact:** Corrupted memory in agent persistence layers could silently read wrong data if checksum is weak
- **Recommendation:** Make checksum strategy configurable; default to SHA2 for provenance/agent use, allow CRC32 opt-in for speed
- **Acceptance:** Checksums use SHA2 by default; README documents CRC32 fast-path trade-offs
- **Effort:** Low (feature flag reorganization, no new code)
---
## Security Hardening
### INT-06: Path Traversal Prevention in Virtual Datasets
**Issue:** Virtual Dataset (VDS) mapping in `clawhdf5-format` allows external dataset source files relative to file path.
- **Risk:** Malicious HDF5 files can reference `../../../etc/passwd` or other system files, causing data leakage or denial of service
- **Impact:** Remote HDF5 processing pipelines (e.g., user-uploaded files in cloud services) could be exploited
- **Recommendation:**
1. Validate all external dataset paths against a whitelist or jail directory
2. Reject paths containing `..` or absolute paths unless explicitly allowed
3. Add integration test with deliberately malicious VDS file
- **Acceptance:** All external paths validated; test suite includes path-traversal attempt (must fail safely)
- **Effort:** LowMedium (validation logic + test)
**File:** `crates/clawhdf5-format/src/data_layout.rs` (VDS mapping)
---
### INT-07: Buffer Overflow Prevention in Chunk Decompression
**Issue:** Decompression filters (gzip, zstd, LZ4, Pcodec) unpack arbitrary chunk sizes; malformed header could claim 2TB chunk in 256MB file.
- **Risk:** Out-of-memory crash or heap corruption if decompression allocates unboundedly
- **Impact:** Denial of service or information disclosure
- **Recommendation:**
1. Add per-chunk size limit (configurable, default 256MB)
2. Validate `uncompressed_size` against dataset shape × element size before decompression
3. Add test case: malformed chunk header with inflated uncompressed_size
- **Acceptance:** Decompression rejects chunks with uncompressed_size > limit
- **Effort:** Low (validation logic + test)
**File:** `crates/clawhdf5-filters/src/lib.rs` (all codec entry points)
---
### INT-08: Input Validation in Writer Path
**Issue:** `FileBuilder` accepts arbitrary shape vectors without overflow checks (e.g., shape=[1e9, 1e9] → total 1e18 elements).
- **Risk:** Integer overflow in `shape.iter().product()` or allocation size calculation
- **Impact:** Silent data corruption or panic on legitimate-looking but oversized shapes
- **Recommendation:**
1. Validate total element count ≤ 2^63 - 1 (i64::MAX)
2. Check `total_elements * element_size_bytes` doesn't overflow usize
3. Reject shapes with zero dimensions
- **Acceptance:** Shape validation rejects oversized arrays; integration tests with max-i64 dimensions
- **Effort:** Low (arithmetic validation)
**File:** `crates/clawhdf5/src/writer.rs` (FileBuilder::with_shape)
---
## Provenance & Supply Chain
### INT-09: Reproducible Build Metadata
**Issue:** Crate versions pinned at 2.1.0; no build reproducibility documentation or SBOM.
- **Risk:** Difficult to audit exact binary origin or verify supply chain integrity
- **Impact:** Can't prove a binary matches a specific commit
- **Recommendation:**
1. Add `SECURITY.md` documenting threat model and release procedures
2. Generate SBOM on release (use `cargo sbom` or `cyclonedx`)
3. Document Rust version requirement (`1.96.0+` per BENCHMARKS.md)
4. Add build script to `Makefile` or CI that produces deterministic binary hash
- **Acceptance:** SBOM checked into `releases/` directory on each tagged release; README links to provenance
- **Effort:** Low (documentation + CI integration)
---
### INT-10: Provenance Feature Audit
**Issue:** `clawhdf5-format` has `provenance` feature (default-enabled, uses SHA2). Used by `clawhdf5-agent` for session history signing.
- **Risk:** If disabled, agent memory loses tamper-detection; if version of SHA2 has CVE, all signed data is at risk
- **Recommendation:**
1. Verify `sha2` v0.10 has no unpatched CVEs (upgrade to 0.11.0 if available)
2. Add documentation explaining provenance guarantees and limitations
3. Make provenance a hard requirement for `clawhdf5-agent` (remove feature gate)
4. Add test: can't load agent session with disabled provenance feature
- **Acceptance:** Agent crate `forbids` disabling provenance; all signatures validated before trust
- **Effort:** Low (feature gate removal + test)
**File:** `crates/clawhdf5-format/Cargo.toml` (features), `crates/clawhdf5-agent/Cargo.toml` (required feature)
---
## Performance & Algorithmic Improvements
### INT-11: Parallel Chunk Write Optimization
**Issue:** Chunked write with deflate-6 achieves 38.4× speedup vs libhdf5 by compressing all chunks before single `write()`. But Rayon parallelism only kicks in for >2 chunks.
- **Risk:** Small files with many tiny chunks get no parallelism
- **Opportunity:** Parallel compression could improve write throughput for embedding archives (typical use case: 10K × 384-dim = thousands of small chunks)
- **Recommendation:**
1. Lower parallelism threshold from 2 chunks to 1 (always parallel if Rayon available)
2. Add microbenchmark: 1K small chunks (32×32 f32) with/without parallelism
3. Measure impact on agent session writes (typical 1001000 embeddings per session)
- **Acceptance:** Benchmark shows measurable speedup on small-chunk workloads (target: 1020%)
- **Effort:** Low (one-line threshold change + benchmark)
**File:** `crates/clawhdf5-io/src/lib.rs` or relevant chunk writing function
---
### INT-12: Lazy Load Consolidation Efficiency
**Issue:** `LazyDataset` interface allows reading subslices without materializing entire dataset, but consolidation benchmarks show 164 µs for 1K records. Consolidation policy is simplistic (decay score based on access count).
- **Risk:** Stale records stay in memory; memory usage grows indefinitely if consolidation threshold never reached
- **Opportunity:** Improve consolidation heuristic to account for record age, size, and embedding distance (semantic clustering could evict "duplicate" memories)
- **Recommendation:**
1. Add configurable consolidation policy (decay + semantic distance)
2. Benchmark consolidation on agent trace with known duplicate detection ground truth
3. Add watermark: consolidate when store reaches 90% of capacity (not just on tick)
- **Acceptance:** Consolidation policy configurable; benchmark shows <5% false-positive eviction rate
- **Effort:** Medium (heuristic design + evaluation)
**File:** `crates/clawhdf5-agent/src/lib.rs` (consolidation logic)
---
### INT-13: Index Stale-ness Detection in Hybrid Search
**Issue:** HNSW index mirrors flat search cache but can drift if concurrent writes occur. "Self-heal on drift" is claimed but not quantified.
- **Risk:** Stale index returns wrong top-k results; hybrid search quality degrades silently
- **Opportunity:** Explicit version counter or CRC checksum to detect drift; optional async re-index
- **Recommendation:**
1. Add generation counter to HNSW index (incremented on build)
2. Check counter before search; if mismatch, either rebuild or log warning
3. Add test: concurrent writes + search; verify index drift detection
- **Acceptance:** Index drift detected and reported; correctness test passes
- **Effort:** LowMedium (version tracking + test)
**File:** `crates/clawhdf5-ann/src/lib.rs` (index struct)
---
## Documentation & Testing
### INT-14: Security Documentation & Threat Model
**Issue:** No `SECURITY.md`; unsafe code not documented with threat model.
- **Recommendation:**
1. Create `SECURITY.md` with supported versions, vulnerability reporting policy
2. Document threat model: trusted file producer vs. untrusted file format
3. List known limitations (e.g., CRC32 not cryptographic, path traversal mitigations)
- **Acceptance:** `SECURITY.md` merged; README links to it
- **Effort:** Low (documentation only)
---
### INT-15: Fuzz Testing Coverage
**Issue:** Fuzz target exists (`crates/clawhdf5-format/fuzz/`) but not integrated into CI.
- **Recommendation:**
1. Add fuzz target to CI (run 10K iterations on each commit)
2. Set up oss-fuzz integration for continuous fuzzing
3. Document how to run fuzz locally
- **Acceptance:** Fuzz job in CI config; README includes fuzz instructions
- **Effort:** Low (CI integration)
---
## Implementation Prioritization
### Critical (Blocking)
- **INT-07**: Buffer overflow in decompression (DoS risk)
- **INT-08**: Integer overflow in shape validation (data corruption risk)
- **INT-06**: Path traversal in VDS (data leakage risk)
### High Priority (Security)
- **INT-01**: Zero-copy alignment validation (UB risk)
- **INT-02**: Panic surface reduction (DoS risk)
- **INT-03**: Dependency security audit (CVE risk)
### Medium Priority (Stability & Performance)
- **INT-04**: Unsafe code audit & forbid (defensive)
- **INT-11**: Parallel chunk write threshold
- **INT-12**: Consolidation heuristics
- **INT-13**: Index drift detection
### Lower Priority (Hygiene & Provenance)
- **INT-05**: Checksum strategy configuration
- **INT-09**: Reproducible build metadata
- **INT-10**: Provenance feature hardening
- **INT-14**: Security documentation
- **INT-15**: Fuzz testing CI
---
## Success Criteria
All items (INT-01 through INT-15):
1. Code changes merged and tested (`cargo test` passes)
2. Benchmarks re-run showing no regressions (5% tolerance on latency)
3. Documented in commit messages and code comments
4. Integration tests added for security-critical changes (INT-06, INT-07, INT-08, INT-01)
**Estimated effort:**
- Critical items: 35 days (focused bug fixes)
- High priority: 58 days (audits + fixes)
- Medium + Lower: 812 days (improvements + docs)
- **Total: 23 weeks for full suite**
---
## Next Steps
1. **Implement INT-07, INT-08, INT-06** first (blocking security issues)
2. **Run `cargo audit`** (INT-03) immediately
3. **Audit unsafe blocks** (INT-04) in parallel
4. **Reduce unwrap()s** (INT-02) incrementally as part of normal development
5. **Remaining items** in order of priority; performance improvements can be batched
+79
View File
@@ -0,0 +1,79 @@
# ClawHDF5 Implementation Status
## Completed & Committed Items
### INT-08: Input Validation in Writer Path (Shape Overflow)
**Status**: ✅ COMMITTED (commit 339a5bd)
- Added shape validation in `file_writer.rs` to prevent integer overflow
- Validates that total element count doesn't exceed i64::MAX or u64::MAX
- Rejects shapes with dimensions that would overflow when multiplied
- Tests: `test_shape_overflow_multiplication`, `test_shape_exceeds_i64_max`, `test_empty_dataset_with_zero_dimensions`, `test_valid_shape`
- Security Review: APPROVED
- Test Status: All passing (542 tests in clawhdf5-format)
### INT-07: Buffer Overflow Prevention in Chunk Decompression
**Status**: ✅ COMMITTED (commit 339a5bd)
- Added chunk_size validation in `filters.rs:decompress_chunk()`
- Rejects chunks claiming sizes larger than MAX_DECOMPRESS_SIZE (256 MiB)
- Prevents decompression bombs and unbounded allocation attacks
- Tests: `decompress_chunk_rejects_oversized_chunk_declaration`, `decompress_chunk_accepts_reasonable_chunk_size`, `decompress_chunk_rejects_hostile_lz4_size_via_public_entrypoint`
- Security Review: APPROVED
- Test Status: All passing (1,400+ tests across workspace)
### INT-06: Path Traversal Prevention in Virtual Datasets
**Status**: ✅ COMMITTED (commit 339a5bd)
- Added path validation in `data_layout.rs:parse_vds_mappings()`
- Validates external file names to reject absolute filesystem paths (/) and directory traversal (..)
- Allows relative paths and same-file references (".")
- Allows absolute HDF5 paths in dataset names (/data is valid)
- Tests: `parse_vds_mappings_rejects_path_traversal`, `parse_vds_mappings_allows_absolute_hdf5_path`, `parse_vds_mappings_rejects_absolute_filesystem_path`, `parse_vds_mappings_allows_relative_path`
- Security Review: APPROVED
- Test Status: All passing (no regressions)
## In Progress / Planned
### INT-02: Panic Surface Reduction (120+ unwrap calls)
- Requires systematic auditing of unwrap() calls
- Priority: High (DoS risk from malformed input)
### INT-03: Dependency Version Alignment & Security Audit
- Run `cargo audit` to identify CVEs
- Current status: 3 warnings about unmaintained crates (not critical)
- Priority: Medium
### INT-01: Zero-Copy Reader Safety & Alignment Audit
- Affects hot paths for large dataset reads
- Requires alignment validation before unsafe { slice::from_raw_parts() }
- Priority: High (UB risk)
### INT-04: Unsafe Code Audit & Quantification
- 144 total unsafe blocks
- Priority: Medium (defensive measure)
### INT-05: CRC32 Fast-Path Checksum Validation
- Make checksum strategy configurable
- Default to SHA2, allow CRC32 opt-in
- Priority: Low
### INT-09 to INT-15
- Remaining items: Documentation, performance optimizations, testing
## Test Suite Status (Post-Commit)
- ✅ All unit tests passing (542 tests in clawhdf5-format)
- ✅ Integration tests passing (78 tests in clawhdf5)
- ✅ Full workspace tests: All passing (1,400+ tests total)
- ✅ No regressions introduced by INT-06, INT-07, INT-08
- ✅ Commit: 339a5bd (SECURITY: Add overflow, decompression bomb, and path traversal validation)
## Committed Summary
**Phase:** IMPLEMENTATION + COMMIT
**Items Merged:** INT-06, INT-07, INT-08 (3 critical security items)
**Test Coverage:** 100% passing, 0 failures
**Regression Status:** Clean — no test failures or new issues detected
**Security Review:** All three items independently verified and approved before commit
## Remaining Work (Next Phase)
1. INT-02 (Panic Surface Reduction) - focus on top 30 unwrap calls
2. INT-01 (Zero-Copy Alignment Validation)
3. INT-03 (Dependency Security Audit)
4. INT-04 through INT-15 (performance optimizations, docs, testing)