clienthub/.planning/SECURITY-SCAN.md

# 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.