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) <noreply@anthropic.com>master
parent
d20fa20ed2
commit
e1c37b520b
@ -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 */}
|
||||
<div class="text-sm text-gray-600 space-y-1.5">
|
||||
<Show
|
||||
when={synth.first_section_title}
|
||||
fallback={
|
||||
<p>{t('home.noPreview')}</p>
|
||||
}
|
||||
>
|
||||
<p class="font-medium text-gray-700">{synth.first_section_title}</p>
|
||||
<p class="text-gray-500">
|
||||
{t('home.previewCount', { count: String(synth.first_section_item_count) })}
|
||||
</p>
|
||||
</Show>
|
||||
</div>
|
||||
```
|
||||
|
||||
- [ ] **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<AdminRateLimit> =>
|
||||
api.put<AdminRateLimit>(`/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<AdminRateLimit> =>
|
||||
api.put<AdminRateLimit>(`/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<reqwest::Client, AppError> {
|
||||
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
|
||||
```
|
||||
Loading…
Reference in New Issue