docs(planning): archivia v2.1/v2.2/v2.3 e documenta v2.4
.planning/ documentava in dettaglio cio che era vecchio e per niente cio
che e in produzione: le fasi 11-22 (v2.1 e v2.2, chiuse a giugno) erano
ancora in phases/ mentre v1.0 e v2.0 stavano gia in milestones/, e il
lavoro degli ultimi due mesi - gate OTP e ciclo di vita dei retainer, cioe
quello che gira su hub.iamcavalli.net - non aveva nessuna cartella.
- phases/{11,12,14} -> milestones/v2.1-phases/, phases/{18..22} ->
milestones/v2.2-phases/. Ora phases/ contiene solo la milestone in
corso, che e quello che state.cjs conta per il progresso
- v2.1-ROADMAP.md ricostruito: era l'unica milestone senza archivio,
interrotta dal reset del 19/06 e mai chiusa formalmente
- v2.3-ROADMAP.md + v2.3-REQUIREMENTS.md: v2.3 e stata eseguita fuori dal
ciclo GSD, non esistono PLAN/SUMMARY per fase. L'archivio E la doc
- REQUIREMENTS.md riscritto per v2.4 con il backlog reale
- phases/13 e phases/26: SUMMARY ricostruiti da commit, migration e
STATUS.md. 26 e il primo numero libero
- research/: cancellate 4 varianti dello stesso PITFALLS e FEATURES/
SUMMARY, superati da PROJECT.md. Diverse anti-feature erano ormai
contraddette dai fatti (il Kanban e stato costruito in Phase 19,
l'email in v2.3, il time tracking esiste)
- cancellati UI-RULES.md e DESIGN-SYSTEM.md (CLAUDE.md li dichiara
superseded: impongono l'inverso della regola attuale) e HANDOFF.md,
fermo al 13/06
- SECURITY-*.md -> security/: audit chiuso, ma i report restano la doc di
cosa e stato ruotato e perche
- PROJECT.md/MILESTONES.md/ROADMAP.md allineati: milestone corrente v2.4,
sessione OTP 90gg non 30, migrazioni fino alla 0016
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,176 @@
|
||||
# Security Audit — Lane codice applicativo
|
||||
|
||||
Data: 2026-07-27 · Branch: `main` · Commit: `dd2d148`
|
||||
|
||||
> **Nota di metodo.** Lo scan multi-agente del plugin `claude-security` è stato avviato ma è
|
||||
> terminato in anticipo (limite di sessione API) dopo la sola fase di inventario, senza produrre
|
||||
> findings. Questo report è quindi il risultato di una revisione manuale mirata sullo stesso
|
||||
> perimetro. Copertura: server actions, modello di autorizzazione, IDOR sui link segreti, SQL
|
||||
> injection, XSS, prompt injection. Non è una revisione riga-per-riga di tutti i 185 file.
|
||||
|
||||
Lane complementare (dipendenze, segreti, deploy): `.planning/SECURITY-AUDIT-INFRA.md`.
|
||||
|
||||
---
|
||||
|
||||
## C-1 · CRITICO — L'autorizzazione admin è un unico punto di rottura, e quel punto ha una CVE
|
||||
|
||||
**Evidenza.** Nessuna delle **21** pagine sotto `src/app/admin/**/page.tsx` esegue un proprio
|
||||
controllo di sessione. Verificato con grep su `getServerSession` / `redirect("/admin/login")`:
|
||||
tutte a zero.
|
||||
|
||||
E `src/app/admin/layout.tsx` **non è un guard** — legge la sessione ma, se manca, renderizza
|
||||
comunque i figli:
|
||||
|
||||
```ts
|
||||
const session = await getServerSession(authOptions);
|
||||
if (!session) {
|
||||
return <div className="min-h-screen bg-background">{children}</div>; // ← rende comunque
|
||||
}
|
||||
```
|
||||
|
||||
Senza sessione la pagina admin viene servita lo stesso, solo senza la chrome di `AdminShell`.
|
||||
|
||||
**Conseguenza.** L'unico controllo effettivo su `/admin/*` è il redirect in `src/proxy.ts`.
|
||||
Non esiste difesa in profondità.
|
||||
|
||||
**Perché ora è critico e non solo fragile:** `next@16.2.6` è affetto da
|
||||
**GHSA-6gpp-xcg3-4w24 — Middleware/Proxy bypass in App Router** (fix in 16.2.11). Un bypass del
|
||||
proxy espone *tutte* le pagine admin — anagrafica clienti, pagamenti, preventivi, marginalità —
|
||||
senza alcun secondo controllo che le fermi.
|
||||
|
||||
**Fix.** Due interventi, entrambi necessari:
|
||||
1. Aggiornare Next a ≥ 16.2.11 (chiude la CVE).
|
||||
2. Rendere `admin/layout.tsx` un guard vero: `if (!session) redirect("/admin/login")`.
|
||||
È una riga, e trasforma il singolo punto di rottura in due livelli indipendenti.
|
||||
|
||||
---
|
||||
|
||||
## C-2 · ALTO — Gli slug cliente hanno solo ~4 caratteri di casualità, e nulla li protegge dal brute force
|
||||
|
||||
**Evidenza.** `src/app/admin/clients/new/actions.ts:17-30`:
|
||||
|
||||
```ts
|
||||
function randomAlpha(len: number): string {
|
||||
const chars = "abcdefghijklmnopqrstuvwxyz0123456789"; // 36 simboli
|
||||
...
|
||||
}
|
||||
function toSlug(name: string): string {
|
||||
const base = name.toLowerCase()...; // ← il nome del cliente, indovinabile
|
||||
return `${base}-${randomAlpha(4)}`; // ← solo 4 caratteri casuali
|
||||
}
|
||||
```
|
||||
|
||||
Lo spazio di ricerca è **36⁴ = 1.679.616** combinazioni, con il prefisso noto o deducibile
|
||||
(è il nome del cliente o del brand).
|
||||
|
||||
**Perché conta.** `src/lib/client-view.ts:163-193` risolve **prima lo slug, poi il token**
|
||||
(scelta D-06). Lo slug è quindi una via d'accesso *parallela e piena* alla dashboard cliente,
|
||||
equivalente al token — ma il token è `nanoid(21)` (~122 bit), lo slug ~20,7 bit. La sicurezza
|
||||
del sistema è quella dell'anello debole.
|
||||
|
||||
**Aggravanti — non c'è niente che rallenti il tentativo:**
|
||||
- Il `matcher` di `src/proxy.ts` copre `/client/:path*`, ma il rate limiter viene applicato
|
||||
**solo** al ramo `/quote/[token]`. Il ramo `/client/` non è limitato.
|
||||
- `/api/internal/validate-slug` è pubblico e non limitato (vedi INFRA §2): è un oracolo che
|
||||
risponde `200`/`404` senza nemmeno dover caricare la pagina.
|
||||
|
||||
**Fix.** Portare il suffisso casuale ad almeno 10-12 caratteri per i nuovi clienti, applicare
|
||||
`rateLimit()` anche al ramo `/client/` del proxy, e rigenerare gli slug esistenti.
|
||||
|
||||
---
|
||||
|
||||
## C-3 · ALTO — `src/lib/quote-actions.ts`: due server action pubbliche senza autenticazione
|
||||
|
||||
**Evidenza.** Il file inizia con `"use server"` ed esporta due funzioni. Nessuna delle due
|
||||
chiama `getServerSession`, e il file non importa affatto `authOptions`:
|
||||
|
||||
- `getOfferWithPhases(offerMicroId)` — legge offerta + fasi + prezzi
|
||||
- `createQuote(input)` — **scrive** una riga in `quotes` per un qualunque `client_id`
|
||||
|
||||
Ogni export in un file `"use server"` diventa un endpoint HTTP pubblico. Il resto della codebase
|
||||
è coerente e corretto — `src/app/admin/**/actions.ts` definisce e usa `requireAdmin()` ovunque
|
||||
(es. `clients/new/actions.ts:12-15`) — **questi due sono l'eccezione**.
|
||||
|
||||
**Aggravante.** `createQuote` e `getOfferWithPhases` non hanno **alcun chiamante** nella codebase
|
||||
(verificato con grep sull'intero `src/`). Sono codice morto — ma codice morto *raggiungibile*:
|
||||
Next.js li compila comunque come endpoint. Superficie d'attacco a costo zero e beneficio zero.
|
||||
|
||||
**Fix.** Cancellare il file. Se serve tenerlo, aggiungere `requireAdmin()` in testa a entrambe.
|
||||
|
||||
---
|
||||
|
||||
## C-4 · MEDIO — XSS immagazzinato: contenuto generato dall'AI renderizzato come HTML grezzo
|
||||
|
||||
**Evidenza.** Quattro sink su pagina pubblica:
|
||||
|
||||
| File | Riga |
|
||||
|---|---|
|
||||
| `src/components/public/proposal/sections/StrategistSection.tsx` | 36 |
|
||||
| `src/components/public/proposal/sections/ScopeSection.tsx` | 31 |
|
||||
| `src/components/public/proposal/sections/SolutionNodeSection.tsx` | 28 |
|
||||
| `src/components/public/proposal/sections/DeliverablesSection.tsx` | 21 |
|
||||
|
||||
```tsx
|
||||
<span dangerouslySetInnerHTML={{ __html: obj }} />
|
||||
```
|
||||
|
||||
`obj` viene da `ProposalContent` (`src/lib/proposal/schema.ts`), cioè dall'output di Claude in
|
||||
`src/lib/proposal/agent.ts:131-135`. Lo schema Zod valida la *struttura*, non il *contenuto*:
|
||||
una stringa con `<img src=x onerror=...>` passa la validazione.
|
||||
|
||||
**Catena di attacco.** `agent.ts:37-40` interpola i transcript nel prompt utente senza
|
||||
delimitazione né sanificazione:
|
||||
|
||||
```ts
|
||||
`=== TRANSCRIPT ${i + 1} — ${t.call_date} ... ===\n${t.content}`
|
||||
```
|
||||
|
||||
Un prospect che invia testo che finisce in un transcript può tentare una prompt injection per far
|
||||
emettere all'AI markup attivo, che viene poi salvato e servito come HTML su `/preventivo/[slug]`.
|
||||
|
||||
**Attenuanti (per cui è MEDIO e non ALTO):** `ProposalDeck` è renderizzato **solo** sulla pagina
|
||||
pubblica, mai in `/admin` — la vittima è il destinatario della proposta, non l'admin, quindi non
|
||||
c'è furto di sessione admin. E richiede che la prompt injection vada a segno.
|
||||
|
||||
**Fix.** Sanificare a monte del render, o — più semplice, dato che serve solo grassetto/corsivo —
|
||||
sostituire il rendering HTML con un piccolo formatter che accetta una whitelist di tag.
|
||||
|
||||
---
|
||||
|
||||
## Cosa invece è risultato SOLIDO
|
||||
|
||||
Vale la pena dirlo esplicitamente, perché è la maggior parte del codice.
|
||||
|
||||
- **SQL injection: nessuna.** Tutti i 17 usi di `` sql`...` `` interpolano riferimenti a colonne
|
||||
Drizzle o valori parametrizzati. `sql.raw` e `db.execute` non compaiono mai nella codebase.
|
||||
|
||||
- **Autorizzazione delle server action admin: corretta e sistematica.** 78 azioni su 80 in
|
||||
`src/app/admin/**` passano da `requireAdmin()`. Le uniche due eccezioni sono in C-3.
|
||||
|
||||
- **Le server action inline nei componenti sono sicure.** Le closure `"use server"` in
|
||||
`PhasesTab.tsx` (4), `DocumentsTab.tsx` (1) e `CommentsTab.tsx` (1) sembrano prive di
|
||||
controlli, ma sono wrapper sottili che delegano ad azioni di
|
||||
`src/app/admin/clients/[id]/actions.ts`, dove `requireAdmin()` c'è. Falso positivo.
|
||||
|
||||
- **IDOR sulla dashboard cliente: assente.** `getProjectView()` è invocato solo con id di progetto
|
||||
già ricavati dal cliente risolto (`page.tsx:120,132`), mai da input utente. La route
|
||||
`/client/[token]` non legge `searchParams`.
|
||||
|
||||
- **Il vincolo `quote_items` di CLAUDE.md è rispettato.** `quote_items` compare solo in
|
||||
`admin-queries.ts`, `quote-service.ts` e `quote-actions.ts`. `src/lib/client-view.ts` — l'unico
|
||||
percorso dati verso il cliente — non lo tocca (commento esplicito alle righe 8 e 217).
|
||||
|
||||
- **Immutabilità di `approved_at`: rispettata**, e in due punti indipendenti.
|
||||
`api/client/approve/route.ts` ritorna un no-op se già valorizzato, e
|
||||
`preventivo/[slug]/actions.ts` usa `isNull(proposals.accepted_at)` nella `WHERE` dell'UPDATE —
|
||||
guard atomico lato DB, resistente alle race condition. Buona ingegneria.
|
||||
|
||||
- **Scoping degli endpoint cliente: corretto.** `api/client/comment/route.ts` verifica la
|
||||
proprietà risalendo la catena client → projects → phases → tasks → deliverables prima di
|
||||
ogni insert. `api/client/approve/route.ts` fa lo stesso con una `innerJoin` che vincola
|
||||
`projects.client_id`. Entrambi sono rate-limitati.
|
||||
|
||||
- **I token sono forti.** `clients.token`, `quotes.token` e `proposals.slug` usano tutti
|
||||
`nanoid(21)` (~122 bit). Il problema è solo lo slug cliente (C-2), che è generato diversamente.
|
||||
|
||||
- **`clients.token` è un campo separato e ruotabile**, mai primary key — vincolo LOCKED rispettato.
|
||||
Reference in New Issue
Block a user