From e1c37b520b810894f5df454ab0c53ff6712711bc Mon Sep 17 00:00:00 2001 From: oabrivard Date: Sun, 22 Mar 2026 16:17:36 +0100 Subject: [PATCH] Add architect remediation implementation plan 5 tasks: fix syntheses list contract (frontend aligns to backend preview fields), fix admin rate-limits path (provider_name not id), SSRF redirect per-hop validation with custom reqwest policy, shared typed test fixtures. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../plans/2026-03-22-architect-remediation.md | 432 ++++++++++++++++++ 1 file changed, 432 insertions(+) create mode 100644 docs/superpowers/plans/2026-03-22-architect-remediation.md diff --git a/docs/superpowers/plans/2026-03-22-architect-remediation.md b/docs/superpowers/plans/2026-03-22-architect-remediation.md new file mode 100644 index 0000000..76f2a6f --- /dev/null +++ b/docs/superpowers/plans/2026-03-22-architect-remediation.md @@ -0,0 +1,432 @@ +# Architect Remediation 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 2 API contract breaks, 1 SSRF redirect vulnerability, and 1 test mock drift issue identified in the architect assessment. + +**Architecture:** Frontend types and components are updated to match backend response shapes. Backend scraper gets a custom reqwest redirect policy with per-hop SSRF validation. A shared test fixtures file prevents future mock drift. + +**Tech Stack:** Rust (reqwest redirect policy, std::net::ToSocketAddrs), SolidJS/TypeScript + +**Spec:** `docs/superpowers/specs/2026-03-22-architect-remediation-design.md` + +--- + +## File Map + +### Modified Files +- `frontend/src/types.ts:122-128` — fix `SynthesisListItem` to match backend +- `frontend/src/pages/Home.tsx:179-192` — use preview fields instead of `sections` +- `frontend/src/api/admin.ts:38-39` — pass `providerName` not `id` +- `frontend/src/pages/admin/RateLimits.tsx:70` — pass `provider_name` to API +- `backend/src/services/scraper.rs:53-65` — custom redirect policy with SSRF checks +- `frontend/src/__tests__/pages/home.test.tsx:24-47` — use shared fixtures + +### New Files +- `frontend/src/__tests__/fixtures.ts` — shared typed test mock data + +--- + +## Task 1: Fix `/syntheses` list contract + +**Files:** +- Modify: `frontend/src/types.ts:122-128` +- Modify: `frontend/src/pages/Home.tsx:179-192` +- Modify: `frontend/src/i18n/fr.ts` (may need new key for preview format) + +- [ ] **Step 1: Update SynthesisListItem type** + +In `frontend/src/types.ts`, replace lines 122-128: + +```typescript +// Before: +export interface SynthesisListItem { + id: string; + week: string; + sections: NewsSection[]; + status: string; + created_at: string; +} + +// After: +export interface SynthesisListItem { + id: string; + week: string; + status: string; + created_at: string; + first_section_title: string | null; + first_section_item_count: number; +} +``` + +- [ ] **Step 2: Run TypeScript to see what breaks** + +Run: `cd frontend && npx tsc --noEmit 2>&1 | head -30` +Expected: errors in `Home.tsx` and test files referencing `synth.sections` + +- [ ] **Step 3: Update Home.tsx card preview** + +Replace lines 179-192 in `frontend/src/pages/Home.tsx`: + +```tsx +{/* Before: accesses synth.sections[0].items */} +{/* After: uses preview metadata from backend */} +
+ {t('home.noPreview')}

+ } + > +

{synth.first_section_title}

+

+ {t('home.previewCount', { count: String(synth.first_section_item_count) })} +

+ +
+``` + +- [ ] **Step 4: Add i18n key if needed** + +In `frontend/src/i18n/fr.ts`, add: +```typescript +'home.previewCount': '{count} articles', +``` + +- [ ] **Step 5: Verify compilation and tests** + +Run: `cd frontend && npx tsc --noEmit` +Expected: errors only in test files (home.test.tsx mocks still use old shape — fixed in Task 4) + +Run: `cd frontend && npx vitest run src/__tests__/pages/home.test.tsx 2>&1 | tail -5` +Expected: may fail due to mock shape — that's OK, Task 4 fixes it + +- [ ] **Step 6: Commit** + +```bash +cd /Users/oabrivard/Projects/rust/ai_synth +git add frontend/src/types.ts frontend/src/pages/Home.tsx frontend/src/i18n/fr.ts +git commit -m "fix: align SynthesisListItem with backend response (preview fields, not sections)" +``` + +--- + +## Task 2: Fix admin rate-limits update path + +**Files:** +- Modify: `frontend/src/api/admin.ts:38-39` +- Modify: `frontend/src/pages/admin/RateLimits.tsx:70` + +- [ ] **Step 1: Update API client** + +In `frontend/src/api/admin.ts`, replace lines 38-39: + +```typescript +// Before: +update: (id: string, data: UpdateRateLimitRequest): Promise => + api.put(`/admin/rate-limits/${id}`, data), + +// After: +/** PUT /admin/rate-limits/:provider_name -- update rate limit for a specific provider. */ +update: (providerName: string, data: UpdateRateLimitRequest): Promise => + api.put(`/admin/rate-limits/${providerName}`, data), +``` + +- [ ] **Step 2: Update RateLimits page save handler** + +In `frontend/src/pages/admin/RateLimits.tsx`, change ONLY line 70: + +```typescript +// Before: +await adminRateLimitsApi.update(limit.id, { + +// After: +await adminRateLimitsApi.update(limit.provider_name, { +``` + +Do NOT change any other usage of `limit.id` — it's still used for UI state (`savingId`, `updateLocal`). + +- [ ] **Step 3: Verify compilation** + +Run: `cd frontend && npx tsc --noEmit` +Expected: no errors (or only the home.test.tsx mock shape errors from Task 1) + +- [ ] **Step 4: Commit** + +```bash +git add frontend/src/api/admin.ts frontend/src/pages/admin/RateLimits.tsx +git commit -m "fix: admin rate-limits API passes provider_name instead of id" +``` + +--- + +## Task 3: SSRF redirect validation per hop + +**Files:** +- Modify: `backend/src/services/scraper.rs:53-65` + +- [ ] **Step 1: Read the current build_scraper_client function** + +Read `backend/src/services/scraper.rs` lines 53-65. + +- [ ] **Step 2: Replace with custom redirect policy** + +Replace the `build_scraper_client()` function. The new version uses `reqwest::redirect::Policy::custom()` with per-hop DNS resolution and `is_private_ip()` checks: + +```rust +/// Build a `reqwest::Client` configured for scraping. +/// +/// Uses a custom redirect policy that validates each hop against private/internal +/// IP addresses (SSRF prevention). DNS is resolved synchronously in the redirect +/// handler via `std::net::ToSocketAddrs`. Max 3 redirects, only http/https schemes. +/// +/// **Residual risk**: There is a theoretical TOCTOU gap between the DNS check in +/// the redirect policy and reqwest's actual TCP connection. DNS rebinding could +/// bypass the check. This is accepted as a known limitation. +pub fn build_scraper_client() -> Result { + use std::net::ToSocketAddrs; + + let redirect_policy = reqwest::redirect::Policy::custom(|attempt| { + if attempt.previous().len() >= 3 { + return attempt.error("Too many redirects"); + } + + let url = attempt.url(); + + if url.scheme() != "http" && url.scheme() != "https" { + return attempt.error("Blocked redirect to non-HTTP scheme"); + } + + if let Some(host) = url.host_str() { + let port = url.port().unwrap_or(if url.scheme() == "https" { 443 } else { 80 }); + let addr_str = format!("{}:{}", host, port); + match addr_str.to_socket_addrs() { + Ok(addrs) => { + for addr in addrs { + if is_private_ip(addr.ip()) { + return attempt.error("Blocked redirect to private/internal IP"); + } + } + } + Err(_) => { + return attempt.error("DNS resolution failed for redirect target"); + } + } + } + + attempt.follow() + }); + + reqwest::Client::builder() + .user_agent(USER_AGENT) + .connect_timeout(std::time::Duration::from_secs(5)) + .timeout(std::time::Duration::from_secs(15)) + .redirect(redirect_policy) + .build() + .map_err(|e| AppError::Internal(anyhow::anyhow!("Failed to build scraper client: {}", e))) +} +``` + +- [ ] **Step 3: Verify compilation and tests** + +Run: `cd backend && cargo check && cargo test --lib scraper` +Expected: compiles, all scraper tests pass (existing `is_private_ip` tests still work, redirect policy is exercised at runtime not in unit tests) + +- [ ] **Step 4: Run full backend tests** + +Run: `cd backend && cargo test --lib` +Expected: all pass + +- [ ] **Step 5: Commit** + +```bash +cd /Users/oabrivard/Projects/rust/ai_synth +git add backend/src/services/scraper.rs +git commit -m "security: SSRF redirect validation per hop with custom reqwest policy" +``` + +--- + +## Task 4: Shared test fixtures + fix mock drift + +**Files:** +- Create: `frontend/src/__tests__/fixtures.ts` +- Modify: `frontend/src/__tests__/pages/home.test.tsx` +- Modify: `frontend/src/__tests__/pages/settings.test.tsx` +- Modify: `frontend/src/__tests__/pages/sources.test.tsx` +- Modify: `frontend/src/__tests__/pages/generate.test.tsx` + +- [ ] **Step 1: Create shared fixtures file** + +Create `frontend/src/__tests__/fixtures.ts`: + +```typescript +/** + * Shared test fixtures typed against actual API response interfaces. + * All page tests import from here instead of defining inline mocks. + * If a type changes in types.ts, fixtures fail to compile — catching drift. + */ +import type { + SynthesisListItem, + Synthesis, + NewsSection, + Source, + UserSettings, + ProviderConfig, + AdminProvider, + AdminRateLimit, + GenerateResponse, +} from '~/types'; +import { DEFAULT_SETTINGS } from '~/types'; + +// ---- Syntheses (list view — matches backend SynthesisListItem) ---- + +export const MOCK_SYNTHESIS_LIST_ITEM: SynthesisListItem = { + id: 'test-synth-1', + week: '2026-W12', + status: 'completed', + created_at: '2026-03-21T10:00:00Z', + first_section_title: 'Annonces majeures', + first_section_item_count: 3, +}; + +export const MOCK_SYNTHESIS_LIST: SynthesisListItem[] = [ + MOCK_SYNTHESIS_LIST_ITEM, + { + ...MOCK_SYNTHESIS_LIST_ITEM, + id: 'test-synth-2', + week: '2026-W11', + first_section_title: null, + first_section_item_count: 0, + }, +]; + +export const MOCK_SYNTHESIS_IN_PROGRESS: SynthesisListItem = { + ...MOCK_SYNTHESIS_LIST_ITEM, + id: 'test-synth-progress', + status: 'in_progress', +}; + +// ---- Syntheses (detail view — has full sections) ---- + +export const MOCK_SECTIONS: NewsSection[] = [ + { + title: 'Annonces majeures', + items: [ + { title: 'GPT-5 Released', url: 'https://example.com/1', summary: 'Summary 1' }, + { title: 'New Chip Launch', url: 'https://example.com/2', summary: 'Summary 2' }, + ], + }, +]; + +export const MOCK_SYNTHESIS_DETAIL: Synthesis = { + id: 'test-synth-1', + user_id: 'test-user-1', + week: '2026-W12', + sections: MOCK_SECTIONS, + status: 'completed', + created_at: '2026-03-21T10:00:00Z', +}; + +// ---- Sources ---- + +export const MOCK_SOURCE: Source = { + id: 'src-1', + user_id: 'u1', + title: 'Test Blog', + url: 'https://test.example.com/blog', + created_at: '2026-03-21T10:00:00Z', +}; + +export const MOCK_SOURCES: Source[] = [ + MOCK_SOURCE, + { ...MOCK_SOURCE, id: 'src-2', title: 'News Site', url: 'https://news.example.com' }, +]; + +// ---- Settings ---- + +export const MOCK_SETTINGS: UserSettings = { + ...DEFAULT_SETTINGS, + theme: 'Intelligence Artificielle', + ai_provider: 'gemini', + ai_model: 'gemini-2.5-flash', +}; + +// ---- Providers ---- + +export const MOCK_PROVIDER_CONFIG: ProviderConfig = { + provider_name: 'gemini', + display_name: 'Google Gemini', + models: [ + { model_id: 'gemini-2.5-flash', display_name: 'Gemini 2.5 Flash' }, + { model_id: 'gemini-2.5-pro', display_name: 'Gemini 2.5 Pro' }, + ], +}; + +export const MOCK_PROVIDER_CONFIGS: ProviderConfig[] = [MOCK_PROVIDER_CONFIG]; + +// ---- Generate ---- + +export const MOCK_GENERATE_RESPONSE: GenerateResponse = { + job_id: 'job-test-1', +}; +``` + +- [ ] **Step 2: Update home.test.tsx to use fixtures** + +In `frontend/src/__tests__/pages/home.test.tsx`: +- Replace the inline `mockSyntheses` data with imports from fixtures +- Update assertions to use `first_section_title` and `first_section_item_count` instead of `sections` +- The mock for `synthesesApi.list` should return `MOCK_SYNTHESIS_LIST` +- For the in-progress test, include `MOCK_SYNTHESIS_IN_PROGRESS` in the list + +- [ ] **Step 3: Update settings.test.tsx to use fixtures** + +Import `MOCK_SETTINGS`, `MOCK_PROVIDER_CONFIGS` from fixtures. Replace inline mock data. + +- [ ] **Step 4: Update sources.test.tsx to use fixtures** + +Import `MOCK_SOURCES`, `MOCK_SOURCE` from fixtures. Replace inline mock data. + +- [ ] **Step 5: Update generate.test.tsx to use fixtures** + +Import `MOCK_SETTINGS`, `MOCK_PROVIDER_CONFIGS`, `MOCK_GENERATE_RESPONSE` from fixtures. Replace inline mock data. + +- [ ] **Step 6: Verify everything passes** + +Run: `cd frontend && npx tsc --noEmit && npx vitest run` +Expected: all tests pass with no TypeScript errors + +- [ ] **Step 7: Verify production build** + +Run: `cd frontend && npx vite build` +Expected: builds successfully + +- [ ] **Step 8: Commit** + +```bash +cd /Users/oabrivard/Projects/rust/ai_synth +git add frontend/src/__tests__/fixtures.ts frontend/src/__tests__/pages/ +git commit -m "test: shared typed fixtures to prevent mock drift from backend contracts" +``` + +--- + +## Task 5: Final verification + +- [ ] **Step 1: Full backend check** + +Run: `cd backend && cargo check && cargo test --lib` +Expected: compiles, all tests pass + +- [ ] **Step 2: Full frontend check** + +Run: `cd frontend && npx tsc --noEmit && npx vitest run && npx vite build` +Expected: type-checks, all tests pass, builds + +- [ ] **Step 3: Commit if any remaining changes** + +```bash +git status +# Only commit if there are uncommitted changes +```