Skip to content

Commit c864b1b

Browse files
committed
Merge pull request 'Full sync: GitHub main -> Gitea main (all 67 commits with manual conflict resolution)' (#740) from sync/github-to-gitea-full into main
2 parents fb3bd1e + 70b1dd5 commit c864b1b

73 files changed

Lines changed: 6492 additions & 815 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
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 |
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
# Implementation Plan: Fix CI `sort_by` Clippy Failures for PR #818
2+
3+
**Status**: Approved
4+
**Research Doc**: `.docs/research-pr818-clippy-lint-fixes.md`
5+
**Author**: opencode (disciplined-design)
6+
**Date**: 2026-04-17
7+
**Estimated Effort**: 30 minutes
8+
9+
## Overview
10+
11+
### Summary
12+
Replace all `sort_by` calls flagged by Rust 1.95 clippy with `sort_by_key` equivalents. 13 mechanical conversions + 3 special cases.
13+
14+
### Approach
15+
Mechanical search-and-replace. Each `sort_by` converting to `sort_by_key` either:
16+
- **Ascending**: `sort_by(|a, b| a.x.cmp(&b.x))` -> `sort_by_key(|a| a.x)`
17+
- **Descending**: `sort_by(|a, b| b.x.cmp(&a.x))` -> `sort_by_key(|b| std::cmp::Reverse(b.x))`
18+
19+
### Scope
20+
**In Scope:**
21+
- Fix all 13 clippy-flagged `sort_by` calls across 5 files in 3 crates
22+
- Handle 3 special cases (multi-key, string-parse, IndexMap)
23+
24+
**Out of Scope:**
25+
- `tui_desktop_parity_test.rs` type annotation errors
26+
- Upgrading local Rust version
27+
- Any changes to terraphim-cli or terraphim-automata (branch's own code)
28+
29+
## File Changes
30+
31+
### Modified Files
32+
| File | Changes |
33+
|------|---------|
34+
| `crates/terraphim-markdown-parser/src/lib.rs` | 1 sort_by -> sort_by_key |
35+
| `crates/terraphim_router/src/keyword.rs` | 1 sort_by -> sort_by_key |
36+
| `crates/terraphim-session-analyzer/src/analyzer.rs` | 4 sort_by -> sort_by_key + 2 IndexMap sort_by |
37+
| `crates/terraphim-session-analyzer/src/parser.rs` | 1 sort_by -> sort_by_key |
38+
| `crates/terraphim-session-analyzer/src/reporter.rs` | 7 sort_by -> sort_by_key |
39+
40+
### No new or deleted files.
41+
42+
## Implementation Steps
43+
44+
### Step 1: terraphim-markdown-parser (1 change)
45+
**File:** `crates/terraphim-markdown-parser/src/lib.rs:144`
46+
47+
```rust
48+
// Before:
49+
edits.sort_by(|a, b| b.range.start.cmp(&a.range.start));
50+
// After:
51+
edits.sort_by_key(|b| std::cmp::Reverse(b.range.start));
52+
```
53+
54+
### Step 2: terraphim_router (1 change)
55+
**File:** `crates/terraphim_router/src/keyword.rs:61`
56+
57+
```rust
58+
// Before:
59+
matched_keywords.sort_by(|a, b| b.1.cmp(&a.1));
60+
// After:
61+
matched_keywords.sort_by_key(|b| std::cmp::Reverse(b.1));
62+
```
63+
64+
### Step 3: terraphim-session-analyzer/analyzer.rs (6 changes)
65+
**File:** `crates/terraphim-session-analyzer/src/analyzer.rs`
66+
67+
| Line | Before | After |
68+
|------|--------|-------|
69+
| 210 | `agents.sort_by(\|a, b\| a.timestamp.cmp(&b.timestamp))` | `agents.sort_by_key(\|a\| a.timestamp)` |
70+
| 564 | `sorted.sort_by(\|a, b\| b.1.cmp(&a.1))` | `sorted.sort_by_key(\|b\| std::cmp::Reverse(b.1))` |
71+
| 637 | `correlations.sort_by(\|a, b\| b.usage_count.cmp(&a.usage_count))` | `correlations.sort_by_key(\|b\| std::cmp::Reverse(b.usage_count))` |
72+
| 718 | `stats.sort_by(\|_, v1, _, v2\| v2.total_invocations.cmp(&v1.total_invocations))` | `stats.sort_by(\|_, v1, _, v2\| v2.total_invocations.cmp(&v1.total_invocations))` **ALLOW** (IndexMap method, not Vec) |
73+
| 739 | `breakdown.sort_by(\|_, v1, _, v2\| v2.cmp(v1))` | `breakdown.sort_by(\|_, v1, _, v2\| v2.cmp(v1))` **ALLOW** (IndexMap method, not Vec) |
74+
| 880 | `chains.sort_by(\|a, b\| b.frequency.cmp(&a.frequency))` | `chains.sort_by_key(\|b\| std::cmp::Reverse(b.frequency))` |
75+
76+
**Lines 718, 739**: These use `IndexMap::sort_by` (not `Vec::sort_by`). Clippy's `unnecessary_sort_by` lint only fires on `Vec::sort_by`, so these should be safe. Verify after fix by running clippy. If they do fire, add `#[allow(clippy::unnecessary_sort_by)]`.
77+
78+
### Step 4: terraphim-session-analyzer/parser.rs (1 change)
79+
**File:** `crates/terraphim-session-analyzer/src/parser.rs:422`
80+
81+
```rust
82+
// Before:
83+
events.sort_by(|a, b| a.timestamp.cmp(&b.timestamp));
84+
// After:
85+
events.sort_by_key(|a| a.timestamp);
86+
```
87+
88+
### Step 5: terraphim-session-analyzer/reporter.rs (7 changes)
89+
**File:** `crates/terraphim-session-analyzer/src/reporter.rs`
90+
91+
| Line | Before | After |
92+
|------|--------|-------|
93+
| 168 | `events.sort_by(\|a, b\| a.0.cmp(&b.0))` | `events.sort_by_key(\|a\| a.0)` |
94+
| 227 | `sorted_agents.sort_by(\|a, b\| b.1.cmp(&a.1))` | `sorted_agents.sort_by_key(\|b\| std::cmp::Reverse(b.1))` |
95+
| 447 | `tool_stats.sort_by(\|a, b\| b.1.total_invocations.cmp(&a.1.total_invocations))` | `tool_stats.sort_by_key(\|b\| std::cmp::Reverse(b.1.total_invocations))` |
96+
| 548 | Multi-key with string parse | Add `#[allow(clippy::unnecessary_sort_by)]` (see note) |
97+
| 570 | `category_rows.sort_by(\|a, b\| b.1.cmp(&a.1))` | `category_rows.sort_by_key(\|b\| std::cmp::Reverse(b.1))` |
98+
| 763 | `category_rows.sort_by(\|a, b\| b.1.cmp(&a.1))` | `category_rows.sort_by_key(\|b\| std::cmp::Reverse(b.1))` |
99+
| 784 | `tool_list.sort_by(\|a, b\| b.1.total_invocations.cmp(&a.1.total_invocations))` | `tool_list.sort_by_key(\|b\| std::cmp::Reverse(b.1.total_invocations))` |
100+
101+
**Line 548**: Sorts by parsing `count` field from `String` to `u32`. Cannot be expressed as a pure key function because `parse()` is fallible. Add `#[allow(clippy::unnecessary_sort_by)]` attribute above the sort call.
102+
103+
### Step 6: Verify
104+
```bash
105+
rustup update stable # Update to 1.95
106+
cargo clippy --workspace --all-targets -- -D warnings
107+
cargo test --workspace
108+
cargo fmt --check
109+
```
110+
111+
Then push and monitor CI.
112+
113+
## Test Strategy
114+
115+
No new tests needed -- these are semantically equivalent transformations. Existing tests cover the sorting behaviour.
116+
117+
## Rollback Plan
118+
119+
`git revert` the commit if any sorting behaviour regresses.

0 commit comments

Comments
 (0)