|
| 1 | +# Design & Implementation Plan: Security Fixes for Issue #630 |
| 2 | + |
| 3 | +**Status**: Draft |
| 4 | +**Author**: Agent |
| 5 | +**Date**: 2026-04-20 |
| 6 | +**Gitea Issue**: #630 |
| 7 | +**Phase**: 2 (Design) |
| 8 | +**Research Doc**: `.docs/research-issue-630-security-fixes.md` |
| 9 | + |
| 10 | +--- |
| 11 | + |
| 12 | +## 1. Summary of Target Behaviour |
| 13 | + |
| 14 | +After implementation: |
| 15 | +- `cargo audit` reports 0 vulnerabilities on default workspace build (no features) |
| 16 | +- `cargo deny check advisories` passes on default build |
| 17 | +- rustls-webpki updated to 0.103.12, fixing RUSTSEC-2026-0098 and RUSTSEC-2026-0099 on the reqwest chain |
| 18 | +- Direct `rand` dependencies removed from `terraphim_multi_agent` and `terraphim_kg_agents`, replaced with `fastrand` (WASM-compatible, already in tree) |
| 19 | +- `deny.toml` updated with accurate ignore entries for discord-feature-only CVEs |
| 20 | +- Port 11434 remediation documented (infrastructure fix, not code) |
| 21 | + |
| 22 | +--- |
| 23 | + |
| 24 | +## 2. Key Invariants and Acceptance Criteria |
| 25 | + |
| 26 | +| # | Invariant | Acceptance Criteria | |
| 27 | +|---|-----------|---------------------| |
| 28 | +| I1 | Default build has no rustls-webpki CVEs | `cargo audit` returns 0 errors on `cargo build --workspace` | |
| 29 | +| I2 | rand removed from direct deps | `cargo tree -p terraphim_multi_agent` and `-p terraphim_kg_agents` show no `rand` | |
| 30 | +| I3 | fastrand replaces rand for RNG | LoadBalancingStrategy::Random and worker success simulation produce equivalent behaviour | |
| 31 | +| I4 | deny.toml is accurate | `cargo deny check` passes; ignore entries have correct RUSTSEC IDs | |
| 32 | +| I5 | Discord feature CVEs documented | RUSTSEC-2026-0098, 0099, 0049 listed in deny.toml with serenity caveat | |
| 33 | +| I6 | Build passes | `cargo build --workspace` succeeds; `cargo clippy --workspace` succeeds | |
| 34 | +| I7 | Tests pass | All existing tests pass unchanged | |
| 35 | + |
| 36 | +--- |
| 37 | + |
| 38 | +## 3. High-Level Design and Boundaries |
| 39 | + |
| 40 | +### Changes Inside Existing Components |
| 41 | + |
| 42 | +1. **Root Cargo.toml** -- Update git patch tag for rustls-webpki from `v/0.103.10` to `v/0.103.12` |
| 43 | +2. **deny.toml** -- Add 0098/0099 to ignore list; update 0049 comment |
| 44 | +3. **terraphim_multi_agent** -- Replace `rand = "0.9"` dep with `fastrand = "2"`, update 1 call site |
| 45 | +4. **terraphim_kg_agents** -- Replace `rand = "0.9"` dep with `fastrand = "2"`, update 1 call site |
| 46 | + |
| 47 | +### No New Components |
| 48 | + |
| 49 | +All changes are modifications to existing files. |
| 50 | + |
| 51 | +### Complected Areas |
| 52 | + |
| 53 | +None -- each change is isolated to a single crate or config file. |
| 54 | + |
| 55 | +--- |
| 56 | + |
| 57 | +## 4. File/Module-Level Change Plan |
| 58 | + |
| 59 | +| File | Action | Before | After | |
| 60 | +|------|--------|--------|-------| |
| 61 | +| `Cargo.toml` (root) | Modify | `tag = "v/0.103.10"` | `tag = "v/0.103.12"` | |
| 62 | +| `Cargo.toml` (root) | Modify | Comment says "RUSTSEC-2026-0049" | Comment says "RUSTSEC-2026-0049, 0098, 0099" | |
| 63 | +| `deny.toml` | Modify | Missing 0098/0099 ignores | Add 0098/0099 with serenity caveat | |
| 64 | +| `crates/terraphim_multi_agent/Cargo.toml` | Modify | `rand = "0.9"` | `fastrand = "2"` | |
| 65 | +| `crates/terraphim_multi_agent/src/pool.rs` | Modify | `rand::rng().random_range(0..available.len())` | `fastrand::Rng::new().usize(0..available.len())` (or similar) | |
| 66 | +| `crates/terraphim_kg_agents/Cargo.toml` | Modify | `rand = "0.9"` | `fastrand = "2"` | |
| 67 | +| `crates/terraphim_kg_agents/src/worker.rs` | Modify | `rand::random()` | `fastrand::Rng::new().f64()` (0..1 range) | |
| 68 | + |
| 69 | +--- |
| 70 | + |
| 71 | +## 5. Step-by-Step Implementation Sequence |
| 72 | + |
| 73 | +### Step 1: Update rustls-webpki git patch (5 min) |
| 74 | + |
| 75 | +**Purpose**: Fix RUSTSEC-2026-0098 and RUSTSEC-2026-0099 on the default build chain. |
| 76 | + |
| 77 | +**File**: `Cargo.toml` (root) |
| 78 | + |
| 79 | +**Changes**: |
| 80 | +- Change `tag = "v/0.103.10"` to `tag = "v/0.103.12"` in `[patch.crates-io]` |
| 81 | +- Update comment on `rustls-webpki` workspace dep and patch to reference all three CVEs |
| 82 | + |
| 83 | +**Deployable**: Yes -- `cargo build --workspace` must pass after this change. |
| 84 | + |
| 85 | +**Verification**: |
| 86 | +```bash |
| 87 | +cargo build --workspace |
| 88 | +cargo tree --workspace | grep "rustls-webpki" # should show 0.103.12 |
| 89 | +``` |
| 90 | + |
| 91 | +### Step 2: Update deny.toml (5 min) |
| 92 | + |
| 93 | +**Purpose**: Document all known CVEs with accurate ignore entries. |
| 94 | + |
| 95 | +**File**: `deny.toml` |
| 96 | + |
| 97 | +**Changes**: |
| 98 | +- Update existing `RUSTSEC-2026-0049` comment to reference serenity chain |
| 99 | +- Add `RUSTSEC-2026-0098` with comment: "rustls-webpki name constraints bypass (URI) -- transitive via serenity -> rustls 0.22 -> webpki 0.102.x; discord feature only" |
| 100 | +- Add `RUSTSEC-2026-0099` with comment: "rustls-webpki name constraints bypass (wildcards) -- transitive via serenity -> rustls 0.22 -> webpki 0.102.x; discord feature only" |
| 101 | + |
| 102 | +**Deployable**: Yes. |
| 103 | + |
| 104 | +**Verification**: |
| 105 | +```bash |
| 106 | +cargo deny check advisories 2>&1 |
| 107 | +``` |
| 108 | + |
| 109 | +### Step 3: Replace rand in terraphim_multi_agent (15 min) |
| 110 | + |
| 111 | +**Purpose**: Remove direct rand dependency, use WASM-compatible fastrand. |
| 112 | + |
| 113 | +**Files**: |
| 114 | +- `crates/terraphim_multi_agent/Cargo.toml` -- replace `rand = "0.9"` with `fastrand = "2"` |
| 115 | +- `crates/terraphim_multi_agent/src/pool.rs` -- replace lines 349-350 |
| 116 | + |
| 117 | +**Before**: |
| 118 | +```rust |
| 119 | +LoadBalancingStrategy::Random => { |
| 120 | + use rand::Rng; |
| 121 | + rand::rng().random_range(0..available.len()) |
| 122 | +} |
| 123 | +``` |
| 124 | + |
| 125 | +**After**: |
| 126 | +```rust |
| 127 | +LoadBalancingStrategy::Random => { |
| 128 | + fastrand::usize(0..available.len()) |
| 129 | +} |
| 130 | +``` |
| 131 | + |
| 132 | +**Deployable**: Yes. |
| 133 | + |
| 134 | +**Verification**: |
| 135 | +```bash |
| 136 | +cargo build -p terraphim_multi_agent |
| 137 | +cargo test -p terraphim_multi_agent |
| 138 | +cargo tree -p terraphim_multi_agent | grep rand # should show no direct rand |
| 139 | +``` |
| 140 | + |
| 141 | +### Step 4: Replace rand in terraphim_kg_agents (15 min) |
| 142 | + |
| 143 | +**Purpose**: Remove direct rand dependency, use WASM-compatible fastrand. |
| 144 | + |
| 145 | +**Files**: |
| 146 | +- `crates/terraphim_kg_agents/Cargo.toml` -- replace `rand = "0.9"` with `fastrand = "2"` |
| 147 | +- `crates/terraphim_kg_agents/src/worker.rs` -- replace line 394 |
| 148 | + |
| 149 | +**Before**: |
| 150 | +```rust |
| 151 | +let random_value: f64 = rand::random(); |
| 152 | +``` |
| 153 | + |
| 154 | +**After**: |
| 155 | +```rust |
| 156 | +let random_value: f64 = fastrand::f64(); |
| 157 | +``` |
| 158 | + |
| 159 | +**Deployable**: Yes. |
| 160 | + |
| 161 | +**Verification**: |
| 162 | +```bash |
| 163 | +cargo build -p terraphim_kg_agents |
| 164 | +cargo test -p terraphim_kg_agents |
| 165 | +cargo tree -p terraphim_kg_agents | grep rand # should show no direct rand |
| 166 | +``` |
| 167 | + |
| 168 | +### Step 5: Full workspace verification (15 min) |
| 169 | + |
| 170 | +**Purpose**: Confirm all changes work together. |
| 171 | + |
| 172 | +**Verification**: |
| 173 | +```bash |
| 174 | +cargo build --workspace |
| 175 | +cargo clippy --workspace -- -D warnings |
| 176 | +cargo test --workspace |
| 177 | +cargo tree --workspace | grep "rustls-webpki" # only 0.103.12 |
| 178 | +cargo tree -p terraphim_multi_agent | grep -E "^.*rand" # no direct rand |
| 179 | +cargo tree -p terraphim_kg_agents | grep -E "^.*rand" # no direct rand |
| 180 | +cargo deny check advisories 2>&1 |
| 181 | +``` |
| 182 | + |
| 183 | +### Step 6: Commit and update Gitea (5 min) |
| 184 | + |
| 185 | +```bash |
| 186 | +git add -A |
| 187 | +git commit -m "fix(security): resolve RUSTSEC-2026-0098/0099, replace rand with fastrand Refs #630" |
| 188 | +git push |
| 189 | +``` |
| 190 | + |
| 191 | +Update Gitea issue #630 with summary comment. |
| 192 | + |
| 193 | +--- |
| 194 | + |
| 195 | +## 6. Testing and Verification Strategy |
| 196 | + |
| 197 | +| Acceptance Criteria | Test Type | Verification | |
| 198 | +|---------------------|-----------|--------------| |
| 199 | +| I1: No webpki CVEs on default build | Build + tree inspection | `cargo tree --workspace \| grep rustls-webpki` shows only 0.103.12 | |
| 200 | +| I2: rand removed from direct deps | Dependency inspection | `cargo tree -p terraphim_multi_agent -i rand` empty; same for kg_agents | |
| 201 | +| I3: fastrand produces equivalent results | Unit (existing) | Existing tests for pool load balancing and worker simulation pass | |
| 202 | +| I4: deny.toml accurate | `cargo deny check` | Exits 0 on default build | |
| 203 | +| I5: Discord CVEs documented | File inspection | deny.toml has 0049, 0098, 0099 entries | |
| 204 | +| I6: Build passes | CI | `cargo build --workspace && cargo clippy --workspace` | |
| 205 | +| I7: Tests pass | CI | `cargo test --workspace` | |
| 206 | + |
| 207 | +### Note on fastrand behavioural equivalence |
| 208 | + |
| 209 | +- `rand::rng().random_range(0..n)` and `fastrand::usize(0..n)` both produce uniform random indices in `[0, n)`. Behaviour is equivalent. |
| 210 | +- `rand::random::<f64>()` produces `[0, 1)` and `fastrand::f64()` also produces `[0, 1)`. Behaviour is equivalent. |
| 211 | +- Neither call site requires cryptographic randomness -- both are for load balancing and simulation. |
| 212 | + |
| 213 | +--- |
| 214 | + |
| 215 | +## 7. Risk and Complexity Review |
| 216 | + |
| 217 | +| Risk (from Phase 1) | Mitigation | Residual Risk | |
| 218 | +|----------------------|------------|---------------| |
| 219 | +| rustls-webpki 0.103.12 tag doesn't exist | Verified: tag exists at `27131d47` | None | |
| 220 | +| fastrand changes load balancing behaviour | fastrand produces uniform distribution; equivalent to rand | None | |
| 221 | +| getrandom API differs from rand | Using fastrand, not getrandom directly -- API maps 1:1 | None | |
| 222 | +| portpicker still pulls rand 0.8 | Deferred to separate issue -- out of scope | Low (test-only code path) | |
| 223 | +| bincode still present | Already in deny.toml ignore; out of scope | Low (external dep) | |
| 224 | +| Discord feature still has webpki 0.102.8 CVEs | Documented in deny.toml; feature off by default | Low (opt-in only) | |
| 225 | + |
| 226 | +--- |
| 227 | + |
| 228 | +## 8. Open Questions / Decisions for Human Review |
| 229 | + |
| 230 | +1. **fastrand vs getrandom**: This plan uses `fastrand` for the two call sites because it's simpler (one function call vs byte buffer + conversion) and already in the tree. Acceptable? |
| 231 | + |
| 232 | +2. **portpicker deferral**: The `portpicker` crate pulls `rand 0.8` into `terraphim_server`. Defer to a separate issue? It's only used in tests and main.rs for dev-mode port picking. |
| 233 | + |
| 234 | +3. **rand 0.10.0 via axum-test**: This is a dev-dependency only (terraphim_validation). Defer? |
| 235 | + |
| 236 | +4. **Commit message format**: Use `fix(security): ... Refs #630` or prefer different convention? |
| 237 | + |
| 238 | +--- |
| 239 | + |
| 240 | +## Deferred Items (Not In Scope) |
| 241 | + |
| 242 | +| Item | Reason | Tracking | |
| 243 | +|------|--------|----------| |
| 244 | +| Replace portpicker (rand 0.8) | Test-only code, non-blocking | Separate issue | |
| 245 | +| Replace bincode in terraphim_automata | Requires migration to postcard/rkyv | RUSTSEC-2025-0141 in deny.toml | |
| 246 | +| Port 11434 firewall on bigbox | Infrastructure, not code | Document in issue comment | |
| 247 | +| Serenity 0.13 upgrade | Breaking API changes (5 break points) | PR #353 context | |
0 commit comments