diff --git a/docs/superpowers/specs/2026-03-26-audit-bugfixes-design.md b/docs/superpowers/specs/2026-03-26-audit-bugfixes-design.md new file mode 100644 index 0000000..68c6ed4 --- /dev/null +++ b/docs/superpowers/specs/2026-03-26-audit-bugfixes-design.md @@ -0,0 +1,105 @@ +# 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` 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