Files
clienthub/.planning/security/SECURITY-SCAN.md
T
simone f7eb7eec23 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>
2026-08-08 22:38:36 +02:00

8.5 KiB

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:

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:

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
<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:

`=== 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.