Skip to content

Commit 03a0888

Browse files
doublegateclaude
andcommitted
docs(todo): add Phase 6 refactoring findings to Phase 7 planning
From comprehensive code review analysis (2025-11-30), add 21 SP of refactoring tasks discovered during Phase 6 evaluation: Sprint 7.1.4: Security Findings (8 SP) - Private key serialization without encryption (HIGH, 3 SP) - TransferSession lacks ZeroizeOnDrop (HIGH, 2 SP) - Configuration validation incomplete (MEDIUM, 1 SP) - File path injection potential (MEDIUM, 1 SP) - Timing information in error paths (MEDIUM, 1 SP) Sprint 7.3.5: Performance Findings (9 SP) - Missing chunks O(n) calculation in chunker (HIGH, 2 SP) - TransferSession missing chunks O(n) (HIGH, 2 SP) - IncrementalTreeHasher hot path allocation (HIGH, 3 SP) - Merkle root per-level allocation (MEDIUM, 1 SP) - Progress calculation overhead (MEDIUM, 1 SP) Sprint 7.4.3: Documentation Findings (4 SP) - Missing API docs for TransferSession (HIGH, 2 SP) - Configuration options undocumented (MEDIUM, 1 SP) - Integration test purpose unclear (MEDIUM, 1 SP) Total Phase 7 story points: 158 → 179 SP (+21 SP) Each finding includes: - Exact file location - Issue description and risk - Proposed fix with code examples - Acceptance criteria checklist 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent b552a1e commit 03a0888

File tree

1 file changed

+183
-1
lines changed

1 file changed

+183
-1
lines changed

to-dos/protocol/phase-7-hardening.md

Lines changed: 183 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Phase 7: Hardening & Optimization Sprint Planning
22

33
**Duration:** Weeks 37-44 (6-8 weeks)
4-
**Total Story Points:** 158
4+
**Total Story Points:** 179 (158 base + 21 from Phase 6 refactoring findings)
55
**Risk Level:** Medium (security critical, time-consuming)
66

77
---
@@ -355,6 +355,65 @@ echo "Dependency audit complete."
355355

356356
---
357357

358+
**7.1.4: Phase 6 Refactoring - Security Findings** (8 SP)
359+
360+
*From comprehensive code review analysis (2025-11-30)*
361+
362+
**HIGH PRIORITY:**
363+
364+
1. **Private Key Serialization Without Encryption** (3 SP)
365+
- **Location:** `crates/wraith-cli/src/main.rs:314`
366+
- **Issue:** `generate_keypair()` writes raw private key bytes to file without encryption
367+
- **Risk:** Key material stored in plaintext on filesystem
368+
- **Fix:** Encrypt private key with passphrase-derived key (Argon2id) before storage
369+
- [ ] Add `argon2` and `rpassword` dependencies
370+
- [ ] Implement `encrypt_private_key()` function
371+
- [ ] Implement `decrypt_private_key()` function
372+
- [ ] Update `generate_keypair` to prompt for passphrase
373+
- [ ] Add tests for encrypted key round-trip
374+
375+
2. **TransferSession Lacks ZeroizeOnDrop** (2 SP)
376+
- **Location:** `crates/wraith-core/src/transfer/session.rs`
377+
- **Issue:** Session state may persist in memory after transfer completion
378+
- **Risk:** Sensitive transfer metadata not cleared from memory
379+
- **Fix:** Add `#[derive(ZeroizeOnDrop)]` and appropriate field annotations
380+
- [ ] Add `zeroize` dependency to wraith-core
381+
- [ ] Implement `ZeroizeOnDrop` for `TransferSession`
382+
- [ ] Mark sensitive fields for zeroization
383+
- [ ] Add test verifying zeroization occurs
384+
385+
**MEDIUM PRIORITY:**
386+
387+
3. **Configuration Validation Incomplete** (1 SP)
388+
- **Location:** `crates/wraith-cli/src/config.rs:260-285`
389+
- **Issue:** `validate()` misses `log_level` and `bootstrap_nodes` URL validation
390+
- [ ] Add `log_level` validation (trace, debug, info, warn, error)
391+
- [ ] Add URL format validation for bootstrap nodes
392+
- [ ] Add tests for invalid configurations
393+
394+
4. **File Path Injection Potential** (1 SP)
395+
- **Location:** `crates/wraith-cli/src/main.rs:144`
396+
- **Issue:** File paths from CLI not sanitized for directory traversal
397+
- [ ] Canonicalize paths before use
398+
- [ ] Validate paths against allowed directories
399+
- [ ] Add tests for path traversal attempts (`../`, symlinks)
400+
401+
5. **Timing Information in Error Paths** (1 SP)
402+
- **Location:** Various crypto error handlers
403+
- **Issue:** Different error branches may have different execution times
404+
- [ ] Audit error paths in crypto operations
405+
- [ ] Ensure constant-time error handling where applicable
406+
- [ ] Add timing tests for error paths
407+
408+
**Acceptance Criteria:**
409+
- [ ] All high-priority items (1-2) completed
410+
- [ ] Private keys encrypted at rest with strong KDF
411+
- [ ] TransferSession properly zeroizes on drop
412+
- [ ] Medium-priority items tracked for completion
413+
- [ ] Security tests added for each fix
414+
415+
---
416+
358417
### Sprint 7.2: Fuzzing & Property Testing (Weeks 39-40)
359418

360419
**Duration:** 2 weeks
@@ -1151,6 +1210,89 @@ These benchmarks were removed from `benches/transfer.rs` during code cleanup (de
11511210

11521211
---
11531212

1213+
**7.3.5: Phase 6 Refactoring - Performance Findings** (9 SP)
1214+
1215+
*From comprehensive code review analysis (2025-11-30)*
1216+
1217+
**HIGH PRIORITY:**
1218+
1219+
6. **Missing Chunks Calculation is O(n)** (2 SP)
1220+
- **Location:** `crates/wraith-files/src/chunker.rs:226-230`
1221+
- **Current:** Iterates all chunks to find missing ones
1222+
- **Impact:** For 1 GB file (4096 chunks), O(4096) per call
1223+
- **Fix:** Maintain explicit `missing_chunks: HashSet<u64>` updated on each write
1224+
```rust
1225+
// Before: O(n)
1226+
pub fn missing_chunks(&self) -> Vec<u64> {
1227+
(0..self.total_chunks)
1228+
.filter(|i| !self.received_chunks.contains(i))
1229+
.collect()
1230+
}
1231+
1232+
// After: O(m) where m = missing count
1233+
pub fn missing_chunks(&self) -> Vec<u64> {
1234+
self.missing_chunks.iter().copied().collect()
1235+
}
1236+
```
1237+
- [ ] Add `missing_chunks: HashSet<u64>` to `FileReassembler`
1238+
- [ ] Initialize with all chunk indices in constructor
1239+
- [ ] Remove from set on `write_chunk()`
1240+
- [ ] Update `missing_chunks()` to return set contents
1241+
- [ ] Add benchmark comparing old vs new implementation
1242+
1243+
7. **TransferSession Missing Chunks Also O(n)** (2 SP)
1244+
- **Location:** `crates/wraith-core/src/transfer/session.rs:212-218`
1245+
- **Same Issue:** Iterates all chunks to calculate missing
1246+
- **Fix:** Same pattern - maintain explicit missing set
1247+
- [ ] Add `missing_chunks: HashSet<u64>` to `TransferSession`
1248+
- [ ] Update on `mark_chunk_transferred()`
1249+
- [ ] Update `missing_count()` to return set length
1250+
- [ ] Add benchmark for transfer session operations
1251+
1252+
8. **IncrementalTreeHasher Allocates in Hot Path** (3 SP)
1253+
- **Location:** `crates/wraith-files/src/tree_hash.rs:212-215`
1254+
- **Current:** `drain().collect()` allocates new Vec for each chunk
1255+
- **Impact:** Unnecessary allocation per chunk (256 KiB default)
1256+
- **Fix:** Use slice-based approach without allocation
1257+
```rust
1258+
// Before: allocates
1259+
let chunk = self.current_buffer.drain(..self.chunk_size).collect::<Vec<_>>();
1260+
1261+
// After: no allocation
1262+
let chunk = &self.current_buffer[..self.chunk_size];
1263+
let hash = blake3::hash(chunk);
1264+
self.chunk_hashes.push(*hash.as_bytes());
1265+
self.current_buffer = self.current_buffer[self.chunk_size..].to_vec();
1266+
```
1267+
- [ ] Refactor `process_chunk()` to use slices
1268+
- [ ] Benchmark before/after (target: 10% improvement)
1269+
- [ ] Consider ring buffer for truly zero-allocation
1270+
1271+
**MEDIUM PRIORITY:**
1272+
1273+
9. **Merkle Root Computation Allocates Per Level** (1 SP)
1274+
- **Location:** `crates/wraith-files/src/tree_hash.rs:116-149`
1275+
- **Issue:** Creates new `Vec` for each tree level
1276+
- **Impact:** Log(n) allocations for n chunks
1277+
- [ ] Pre-allocate with capacity
1278+
- [ ] Use in-place computation if possible
1279+
1280+
10. **Progress Calculation on Every Update** (1 SP)
1281+
- **Location:** `crates/wraith-core/src/transfer/session.rs:173-179`
1282+
- **Issue:** Division on every `progress()` call
1283+
- [ ] Cache progress value
1284+
- [ ] Invalidate cache on chunk completion
1285+
- [ ] Benchmark impact
1286+
1287+
**Acceptance Criteria:**
1288+
- [ ] All high-priority items (6-8) completed
1289+
- [ ] Missing chunks operations reduced from O(n) to O(m)
1290+
- [ ] Hot path allocations eliminated
1291+
- [ ] Benchmarks show measurable improvement (>10%)
1292+
- [ ] No performance regressions in existing benchmarks
1293+
1294+
---
1295+
11541296
### Sprint 7.4: Documentation (Weeks 42-43)
11551297

11561298
**Duration:** 1.5 weeks
@@ -1647,6 +1789,46 @@ wraith nat-status
16471789

16481790
---
16491791

1792+
**7.4.3: Phase 6 Refactoring - Documentation Findings** (4 SP)
1793+
1794+
*From comprehensive code review analysis (2025-11-30)*
1795+
1796+
**HIGH PRIORITY:**
1797+
1798+
11. **Missing API Documentation for TransferSession** (2 SP)
1799+
- **Location:** `crates/wraith-core/src/transfer/session.rs`
1800+
- **Issue:** Module has struct docs but no crate-level visibility or comprehensive examples
1801+
- [ ] Add `#![doc = "Transfer session management"]` at crate level
1802+
- [ ] Add usage examples in module doc
1803+
- [ ] Document all public methods with `# Examples` sections
1804+
- [ ] Document state machine transitions with diagram
1805+
- [ ] Add `# Safety` sections for any sensitive operations
1806+
1807+
**MEDIUM PRIORITY:**
1808+
1809+
12. **Configuration Options Undocumented** (1 SP)
1810+
- **Location:** `crates/wraith-cli/src/config.rs`
1811+
- **Issue:** Struct fields have serde attributes but minimal doc comments
1812+
- [ ] Document each configuration option
1813+
- [ ] Add default values in documentation
1814+
- [ ] Add valid ranges/constraints
1815+
- [ ] Create example config file with comments
1816+
1817+
13. **Integration Test Purpose Unclear** (1 SP)
1818+
- **Location:** `tests/integration_tests.rs`
1819+
- **Issue:** Phase 7 placeholder tests lack detail on expected behavior
1820+
- [ ] Document expected test scenarios in comments
1821+
- [ ] Add test plan for each placeholder
1822+
- [ ] Define success criteria for end-to-end tests
1823+
1824+
**Acceptance Criteria:**
1825+
- [ ] TransferSession has complete rustdoc with examples
1826+
- [ ] Configuration options fully documented
1827+
- [ ] Integration test expectations documented
1828+
- [ ] `cargo doc --workspace` builds without warnings
1829+
1830+
---
1831+
16501832
### Sprint 7.5: Cross-Platform & Packaging (Weeks 43-44)
16511833

16521834
**Duration:** 1.5 weeks

0 commit comments

Comments
 (0)