diff --git a/docs/superpowers/plans/2026-03-26-audit-bugfixes.md b/docs/superpowers/plans/2026-03-26-audit-bugfixes.md new file mode 100644 index 0000000..b31825b --- /dev/null +++ b/docs/superpowers/plans/2026-03-26-audit-bugfixes.md @@ -0,0 +1,521 @@ +# Audit Bug Fixes — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix 9 bugs (3 P0, 6 P1) identified by the 5-agent code audit. + +**Architecture:** Independent, targeted fixes across backend and frontend. No architectural changes. Each task is self-contained and can be committed independently. + +**Tech Stack:** Rust (Axum, tokio), SolidJS, TypeScript + +**Spec:** `docs/superpowers/specs/2026-03-26-audit-bugfixes-design.md` + +--- + +### Task 1: Fix UTF-8 panic in error sanitization + +**Files:** +- Modify: `backend/src/services/synthesis.rs` + +- [ ] **Step 1: Find and fix the unsafe slice** + +In `backend/src/services/synthesis.rs`, find the `sanitize_error_message` function (around line 1305). Replace: + +```rust +if msg.len() > 200 { + format!("{}...", &msg[..200]) +} +``` + +With: + +```rust +if msg.len() > 200 { + let truncated: String = msg.chars().take(200).collect(); + format!("{}...", truncated) +} +``` + +- [ ] **Step 2: Add a test** + +Add a test in the same file's `#[cfg(test)] mod tests` block: + +```rust +#[test] +fn sanitize_error_message_handles_multibyte_utf8() { + // This would panic with &msg[..200] if the 200th byte falls mid-character + let msg = "é".repeat(150); // 300 bytes, 150 chars + let result = sanitize_error_message(&msg); + assert!(result.len() <= 600); // 200 chars * 2 bytes each + "..." + assert!(result.ends_with("...")); +} +``` + +Note: `sanitize_error_message` may be private. If so, test via a helper or make `pub(crate)` for testing. + +- [ ] **Step 3: Run tests** + +Run: `cd backend && cargo test --lib` +Expected: All pass + +- [ ] **Step 4: Commit** + +```bash +git add backend/src/services/synthesis.rs +git commit -m "fix: prevent UTF-8 panic in error message truncation" +``` + +--- + +### Task 2: Fix job creation race condition + pipeline timeout + panic handling + +**Files:** +- Modify: `backend/src/services/synthesis.rs` +- Modify: `backend/src/handlers/generation.rs` + +These three bugs (race condition, no timeout, swallowed panics) all touch the job lifecycle in `synthesis.rs` and the spawn in `generation.rs`. They must be applied together to avoid a window where panics permanently lock out users. + +- [ ] **Step 1: Add `generating_users` DashSet to JobStore in `synthesis.rs`** + +Add `use dashmap::DashSet;` to imports. Update `JobStore`: + +```rust +pub struct JobStore { + inner: Arc>, + generating_users: Arc>, +} +``` + +Update `new()`: +```rust +pub fn new() -> Self { + Self { + inner: Arc::new(DashMap::new()), + generating_users: Arc::new(DashSet::new()), + } +} +``` + +- [ ] **Step 2: Update `create_job`, `has_active_job`, `cleanup_expired`, add `release_user`** + +Replace `create_job`: +```rust +pub fn create_job(&self, user_id: Uuid) -> Option<(Uuid, Arc>)> { + if !self.generating_users.insert(user_id) { + return None; + } + let job_id = Uuid::new_v4(); + let (tx, rx) = watch::channel(ProgressEvent::Progress { + step: "init".into(), message: "Initialisation...".into(), percent: 0, + }); + let tx = Arc::new(tx); + self.inner.insert(job_id, JobEntry { + tx: Arc::clone(&tx), _rx: rx, user_id, created_at: Instant::now(), + }); + Some((job_id, tx)) +} +``` + +Simplify `has_active_job`: +```rust +pub fn has_active_job(&self, user_id: Uuid) -> Option { + if !self.generating_users.contains(&user_id) { return None; } + for entry in self.inner.iter() { + if entry.value().user_id == user_id { return Some(*entry.key()); } + } + None +} +``` + +Add `release_user`: +```rust +pub fn release_user(&self, user_id: Uuid) { + self.generating_users.remove(&user_id); +} +``` + +Update `cleanup_expired`: +```rust +pub fn cleanup_expired(&self) { + let now = Instant::now(); + self.inner.retain(|_, entry| { + let keep = now.duration_since(entry.created_at) < JOB_TTL; + if !keep { self.generating_users.remove(&entry.user_id); } + keep + }); +} +``` + +- [ ] **Step 3: Update the spawn in `generation.rs` — timeout + panic handling + release** + +In `backend/src/handlers/generation.rs`, find the spawn block (around line 70-75). Replace it with: + +```rust +// Clone tx before moving into spawn so we can use it for panic handling +let tx_for_panic = Arc::clone(&tx); +let state_for_panic = state.clone(); + +let join_handle = tokio::spawn(async move { + let timeout_duration = std::time::Duration::from_secs(900); // 15 minutes + match tokio::time::timeout(timeout_duration, synthesis::run_generation(job_id, state_clone.clone(), user_id, tx.clone())).await { + Ok(()) => {} + Err(_) => { + tracing::error!(job_id = %job_id, user_id = %user_id, "Generation timed out after 15 minutes"); + let _ = tx.send(synthesis::ProgressEvent::Error { + message: "La generation a depasse le delai maximum de 15 minutes.".into(), + }); + } + } + state_clone.job_store.release_user(user_id); +}); + +// Monitor for panics — if the task panics, release_user inside won't run +tokio::spawn(async move { + if let Err(e) = join_handle.await { + tracing::error!(job_id = %job_id, error = %e, "Generation task panicked"); + let _ = tx_for_panic.send(synthesis::ProgressEvent::Error { + message: "Erreur interne lors de la generation.".into(), + }); + state_for_panic.job_store.release_user(user_id); + } +}); +``` + +Read the file first to get the exact variable names (`state_clone`, `tx`, `job_id`, `user_id`). + +- [ ] **Step 4: Build and test** + +Run: `cd backend && cargo build && cargo test --lib` +Expected: All pass + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/services/synthesis.rs backend/src/handlers/generation.rs +git commit -m "fix: atomic job creation, 15min timeout, and panic handling" +``` + +--- + +### Task 3: Fix XSS via innerHTML + setTimeout cleanup in GenerateSynthesis.tsx + +**Files:** +- Modify: `frontend/src/pages/GenerateSynthesis.tsx` + +- [ ] **Step 1: Replace innerHTML with safe text** + +Find the `innerHTML` usage (around line 249): + +```tsx +

+``` + +Replace with: + +```tsx +

+ {t('generate.description', { + days: String(settings().max_age_days), + theme: settings().theme, + })} +

+``` + +- [ ] **Step 2: Add onCleanup for setTimeout** + +Find the `setTimeout` (around line 187): + +```tsx +setTimeout(() => { + navigate(`/synthesis/${synthId}`); +}, 2000); +``` + +Replace with: + +```tsx +const timer = setTimeout(() => { + navigate(`/synthesis/${synthId}`); +}, 2000); +onCleanup(() => clearTimeout(timer)); +``` + +Make sure `onCleanup` is imported from `solid-js` at the top of the file. + +- [ ] **Step 3: TypeScript check** + +Run: `cd frontend && npx tsc --noEmit` +Expected: No errors + +- [ ] **Step 4: Commit** + +```bash +git add frontend/src/pages/GenerateSynthesis.tsx +git commit -m "fix: remove XSS via innerHTML and add setTimeout cleanup" +``` + +--- + +### Task 4: Add expired session cleanup background task + +**Files:** +- Modify: `backend/src/main.rs` + +- [ ] **Step 1: Add periodic session cleanup** + +In `backend/src/main.rs`, find the `Commands::Serve` block. After the rate limiter reload and before `let app = router::build_router(...)`, spawn a background task: + +```rust +// Periodic session cleanup (every hour) +{ + let cleanup_pool = state.pool.clone(); + tokio::spawn(async move { + let mut interval = tokio::time::interval(std::time::Duration::from_secs(3600)); + loop { + interval.tick().await; + match crate::db::sessions::delete_expired(&cleanup_pool).await { + Ok(count) => { + if count > 0 { + tracing::info!(deleted = count, "Cleaned up expired sessions"); + } + } + Err(e) => { + tracing::warn!(error = %e, "Failed to clean up expired sessions"); + } + } + } + }); +} +``` + +- [ ] **Step 2: Build and test** + +Run: `cd backend && cargo build && cargo test --lib` +Expected: All pass + +- [ ] **Step 3: Commit** + +```bash +git add backend/src/main.rs +git commit -m "fix: add periodic expired session cleanup (hourly)" +``` + +--- + +### Task 5: Fix SSRF bypass via IPv4-mapped IPv6 + +**Files:** +- Modify: `backend/src/services/scraper.rs` + +- [ ] **Step 1: Add IPv4-mapped check to `is_private_ip`** + +In `backend/src/services/scraper.rs`, find the `is_private_ip` function (around line 294). Add a check at the beginning of the `IpAddr::V6` branch: + +```rust +IpAddr::V6(v6) => { + // Check for IPv4-mapped IPv6 addresses (::ffff:x.x.x.x) + if let Some(mapped_v4) = v6.to_ipv4_mapped() { + return mapped_v4.is_loopback() + || mapped_v4.is_private() + || mapped_v4.is_link_local() + || mapped_v4.is_unspecified(); + } + + let segments = v6.segments(); + v6.is_loopback() + // ... rest of existing checks +``` + +- [ ] **Step 2: Add tests for IPv4-mapped addresses** + +Find the SSRF tests section and add: + +```rust +#[test] +fn rejects_ipv4_mapped_ipv6_loopback() { + let ip: IpAddr = "::ffff:127.0.0.1".parse().unwrap(); + assert!(is_private_ip(ip)); +} + +#[test] +fn rejects_ipv4_mapped_ipv6_private_10() { + let ip: IpAddr = "::ffff:10.0.0.1".parse().unwrap(); + assert!(is_private_ip(ip)); +} + +#[test] +fn rejects_ipv4_mapped_ipv6_private_192() { + let ip: IpAddr = "::ffff:192.168.1.1".parse().unwrap(); + assert!(is_private_ip(ip)); +} + +#[test] +fn rejects_ipv4_mapped_ipv6_link_local() { + let ip: IpAddr = "::ffff:169.254.1.1".parse().unwrap(); + assert!(is_private_ip(ip)); +} + +#[test] +fn allows_ipv4_mapped_ipv6_public() { + let ip: IpAddr = "::ffff:8.8.8.8".parse().unwrap(); + assert!(!is_private_ip(ip)); +} +``` + +- [ ] **Step 3: Run tests** + +Run: `cd backend && cargo test --lib scraper` +Expected: All pass including the 5 new tests + +- [ ] **Step 4: Commit** + +```bash +git add backend/src/services/scraper.rs +git commit -m "fix: block SSRF via IPv4-mapped IPv6 addresses" +``` + +--- + +### Task 6: Add SSRF check to source page fetching + +**Files:** +- Modify: `backend/src/services/source_scraper.rs` +- Modify: `backend/src/services/scraper.rs` (make `check_ssrf` public) + +- [ ] **Step 1: Make `check_ssrf` public** + +In `backend/src/services/scraper.rs`, find `async fn check_ssrf` (around line 240) and change to: + +```rust +pub async fn check_ssrf(url: &url::Url) -> Result<(), AppError> { +``` + +- [ ] **Step 2: Add SSRF check in `extract_article_links`** + +In `backend/src/services/source_scraper.rs`, find `extract_article_links` (around line 32). Add the SSRF check before the HTTP fetch: + +```rust +pub async fn extract_article_links( + http_client: &reqwest::Client, + source_url: &str, + max_links: usize, +) -> Result, AppError> { + let base_url = Url::parse(source_url) + .map_err(|e| AppError::BadRequest(format!("Invalid source URL: {}", e)))?; + + // SSRF check before fetching + if let Err(e) = crate::services::scraper::check_ssrf(&base_url).await { + tracing::warn!(url = source_url, error = %e, "Source URL failed SSRF check"); + return Ok(Vec::new()); + } + + let base_domain = base_url.host_str().unwrap_or("").to_lowercase(); + // ... rest unchanged +``` + +- [ ] **Step 3: Add SSRF check in `extract_article_links_with_llm`** + +Same pattern in `extract_article_links_with_llm` (around line 185), add after `base_url` parse: + +```rust + // SSRF check before fetching + if let Err(e) = crate::services::scraper::check_ssrf(&base_url).await { + tracing::warn!(url = source_url, error = %e, "Source URL failed SSRF check"); + return Ok(Vec::new()); + } +``` + +- [ ] **Step 4: Build and test** + +Run: `cd backend && cargo build && cargo test --lib` +Expected: All pass + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/services/scraper.rs backend/src/services/source_scraper.rs +git commit -m "fix: add SSRF check before fetching source pages" +``` + +--- + +### Task 7: Add onCleanup for setTimeout in frontend pages + +**Files:** +- Modify: `frontend/src/pages/Home.tsx` +- Modify: `frontend/src/pages/ArticleHistory.tsx` +- Modify: `frontend/src/pages/SynthesisDetail.tsx` + +- [ ] **Step 1: Fix Home.tsx** + +In `frontend/src/pages/Home.tsx`, add `onCleanup` to the `solid-js` import. Add cleanup for the delete confirmation timers at the component level: + +```tsx +onCleanup(() => { + const timers = deleteTimers(); + Object.values(timers).forEach((timer) => clearTimeout(timer)); +}); +``` + +- [ ] **Step 2: Fix ArticleHistory.tsx** + +In `frontend/src/pages/ArticleHistory.tsx`, add `onCleanup` and `createSignal` to the `solid-js` import (if not already present). Use a signal to track the timer: + +```tsx +const [confirmTimer, setConfirmTimer] = createSignal | undefined>(); + +// At component top level: +onCleanup(() => { + const t = confirmTimer(); + if (t) clearTimeout(t); +}); +``` + +Then in the `handleClear` function, replace: +```tsx +setTimeout(() => setConfirming(false), 3000); +``` +with: +```tsx +setConfirmTimer(setTimeout(() => setConfirming(false), 3000)); +``` + +- [ ] **Step 3: Fix SynthesisDetail.tsx** + +In `frontend/src/pages/SynthesisDetail.tsx`, add `onCleanup` and `createSignal` to the `solid-js` import. The `setTimeout` is inside the async `sendEmail` handler, so use a signal to track it (same pattern as ArticleHistory): + +```tsx +const [emailTimer, setEmailTimer] = createSignal | undefined>(); + +// At component top level: +onCleanup(() => { + const t = emailTimer(); + if (t) clearTimeout(t); +}); +``` + +Then replace: +```tsx +setTimeout(() => setEmailSuccess(false), 5000); +``` +with: +```tsx +setEmailTimer(setTimeout(() => setEmailSuccess(false), 5000)); +``` + +Read the file first to find ALL `setTimeout` calls and apply the same pattern to each. + +- [ ] **Step 4: TypeScript check** + +Run: `cd frontend && npx tsc --noEmit` +Expected: No errors + +- [ ] **Step 5: Commit** + +```bash +git add frontend/src/pages/Home.tsx frontend/src/pages/ArticleHistory.tsx frontend/src/pages/SynthesisDetail.tsx +git commit -m "fix: add onCleanup for setTimeout in frontend pages" +```