You cannot select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
106 lines
4.9 KiB
Markdown
106 lines
4.9 KiB
Markdown
# Design: Audit Bug Fixes (P0 + P1)
|
|
|
|
**Date**: 2026-03-26
|
|
**Scope**: Fix 9 bugs identified by the 5-agent code audit
|
|
**Source**: `docs/audit/00-consolidated-summary.md`
|
|
|
|
---
|
|
|
|
## P0 — Fix Immediately
|
|
|
|
### 1. UTF-8 panic in error sanitization
|
|
|
|
**Problem**: `&msg[..200]` in `synthesis.rs:1313` panics when slicing multi-byte UTF-8 characters.
|
|
|
|
**Fix**: Replace with safe char-boundary slicing:
|
|
```rust
|
|
let truncated: String = msg.chars().take(200).collect();
|
|
```
|
|
|
|
**Files**: `backend/src/services/synthesis.rs`
|
|
|
|
### 2. Race condition in job creation
|
|
|
|
**Problem**: Inside `create_job()`, the DashMap iteration (checking for existing active jobs by user_id) and the subsequent `insert` (with a new job_id key) are not atomic. Between the iteration completing and the insert, another thread can pass the same check and also insert. The DashMap is keyed by `job_id` (UUID), not `user_id`, so `entry()` cannot make this atomic directly.
|
|
|
|
**Fix**: Add a secondary `DashSet<Uuid>` keyed by `user_id` that acts as a lock. Before creating a job, attempt `generating_users.insert(user_id)` — if it returns `false` (already present), reject the request. Remove the user_id from the set when the job completes. This makes the check-and-lock atomic via DashSet's insert semantics.
|
|
|
|
**Files**: `backend/src/services/synthesis.rs` (job management section)
|
|
|
|
### 3. XSS via `innerHTML`
|
|
|
|
**Problem**: `GenerateSynthesis.tsx` renders the user's `theme` setting via `innerHTML`, allowing self-XSS.
|
|
|
|
**Fix**: Replace `innerHTML` with text interpolation. Use template literal or JSX text content instead.
|
|
|
|
**Files**: `frontend/src/pages/GenerateSynthesis.tsx`
|
|
|
|
---
|
|
|
|
## P1 — Fix Within 30 Days
|
|
|
|
### 4. Expired session cleanup
|
|
|
|
**Problem**: `sessions::delete_expired()` exists in the DB module but is never called. Sessions accumulate forever.
|
|
|
|
**Fix**: Spawn a background `tokio::spawn` task at startup that calls `sessions::delete_expired()` periodically (every hour).
|
|
|
|
**Files**: `backend/src/main.rs` (or startup logic)
|
|
|
|
### 5. No pipeline timeout
|
|
|
|
**Problem**: A hung LLM API call blocks the generation slot indefinitely. Users cannot trigger a new generation until the stuck job times out via TTL.
|
|
|
|
**Fix**: Wrap the `run_generation_inner` call in `tokio::time::timeout()` with a 15-minute limit. On timeout, emit an error progress event and clean up the job.
|
|
|
|
**Files**: `backend/src/services/synthesis.rs` (the spawn that calls `run_generation_inner`)
|
|
|
|
### 6. SSRF bypass via IPv4-mapped IPv6
|
|
|
|
**Problem**: `::ffff:127.0.0.1` (IPv4-mapped IPv6 address) bypasses all current SSRF checks because the IPv6 branch doesn't check for mapped addresses.
|
|
|
|
**Fix**: In the SSRF validator, check for IPv4-mapped IPv6 addresses using `to_ipv4_mapped()` (or `to_ipv4()`) and apply IPv4 checks to the mapped address. Add tests for `::ffff:127.0.0.1`, `::ffff:10.0.0.1`, `::ffff:192.168.1.1`.
|
|
|
|
**Files**: `backend/src/services/scraper.rs` (SSRF check function)
|
|
|
|
### 7. Missing SSRF check on source page fetch
|
|
|
|
**Problem**: `source_scraper.rs` fetches source page URLs directly without SSRF validation. A user could add a source pointing to `http://169.254.169.254/` (AWS metadata).
|
|
|
|
**Fix**: Call the existing SSRF check function before fetching source pages in both `extract_article_links` and `extract_article_links_with_llm`. If the URL fails SSRF validation, log a warning and return empty results.
|
|
|
|
**Files**: `backend/src/services/source_scraper.rs`
|
|
|
|
### 8. Spawned generation tasks swallow panics
|
|
|
|
**Problem**: If the spawned generation task panics, the `JoinHandle` is never checked. The SSE client hangs waiting for a progress event that never comes.
|
|
|
|
**Fix**: After `.await`-ing the `JoinHandle`, check for `Err(JoinError)` (which indicates a panic). If the task panicked, emit an error progress event so the SSE client gets notified, and clean up the job entry.
|
|
|
|
**Files**: `backend/src/handlers/generation.rs` (the `tokio::spawn` at line ~73)
|
|
|
|
### 9. Missing `onCleanup` for `setTimeout`
|
|
|
|
**Problem**: `setTimeout` calls in `Home.tsx`, `ArticleHistory.tsx`, and `SynthesisDetail.tsx` are not cleaned up on component unmount. The timer callback fires after the component is gone, potentially causing stale state updates.
|
|
|
|
**Fix**: Store the timer ID returned by `setTimeout`, call `clearTimeout(id)` inside `onCleanup(() => ...)`.
|
|
|
|
**Files**: `frontend/src/pages/Home.tsx`, `frontend/src/pages/ArticleHistory.tsx`, `frontend/src/pages/SynthesisDetail.tsx`, `frontend/src/pages/GenerateSynthesis.tsx` (also has uncleaned `setTimeout`, already touched for fix #3)
|
|
|
|
---
|
|
|
|
## Files Summary
|
|
|
|
**Backend**:
|
|
- `backend/src/services/synthesis.rs` — fixes #1, #2, #5
|
|
- `backend/src/handlers/generation.rs` — fix #8
|
|
- `backend/src/services/scraper.rs` — fix #6
|
|
- `backend/src/services/source_scraper.rs` — fix #7
|
|
- `backend/src/main.rs` — fix #4
|
|
|
|
**Frontend**:
|
|
- `frontend/src/pages/GenerateSynthesis.tsx` — fixes #3, #9
|
|
- `frontend/src/pages/Home.tsx` — fix #9
|
|
- `frontend/src/pages/ArticleHistory.tsx` — fix #9
|
|
- `frontend/src/pages/SynthesisDetail.tsx` — fix #9
|