# Revisione Docker 2026 — Bookish (fork Docker)

Revisione effettuata il 29/08/2026 su `main` (HEAD `e62c50c`).
Scopo: verificare che il progetto sia **completo, sicuro e manutenibile come
Docker app**, e definire un piano di miglioramento priorizzabile.

---

## 1. Sintesi esecutiva

Il progetto è già una Docker app **funzionante** e ben impacchettata:

- Dockerfile multistage + utente non-root + healthcheck + compose con limiti
- Conversione asincrona con SSE, storage persistente su volume
- Configurazione 12-factor via variabili d'ambiente

La revisione ha però evidenziato **1 problema funzionale critico**, **3 falle di
sicurezza** e una serie di **carenze strutturali** (test, lock, deploy) che
vanno chiuse prima di considerare il progetto "pronto" per un uso reale.

Esito revisione: **❌ non pronta per produzione — servono correzioni**.

> Aggiornamento (sessione corrente): la P0 è chiusa (**R1, R2, R3, R6**) e
> parte della P1 (**R4** lock + **CI** smoke test + **self-host JS offline**).
> Restano P1 R4b e P2 (R5, R7).

---

## 2. Punti di forza (cosa tenere)

1. **Dockerfile a 2 stadi** — builder con venv separato, runtime minimale
   (pandoc, pango, weasyprint deps), `--no-install-recommends` e cancellazione
   apt-lists. 

2. **Compose completo** — `no-new-privileges`, `tmpfs /tmp`, `mem_limit`,
   `cpus`, `restart: unless-stopped`, `stop_grace_period`, `PORT` parametrizzata.

3. **Healthcheck** `/healthz` che verifica davvero la scrivibilità del volume
   dati. 

4. **Non-root + uid deterministico 1000** — volume riutilizzabile senza fix
   di permessi.

5. **Conversione asincrona** via thread + SSE con lock thread-safe e cleanup
   automatico anche senza cliente connesso (`generate_sse_events` con finally).

6. **Fallback localStorage** lato reader: l'app resta utilizzabile se il
   server API non risponde.

7. **SQLite WAL + parametrized query**: robustness accettabile per uso
   single-user.

8. **Documentazione** (README, docs/) completa e in italiano.

---

## 3. Debolezze — Analisi mirata

### 3.1 🔴 [CRITICO] IL Reader fallisce per una quota di file convertiti

Nel modalità Docker i file vengono salvati con prefisso `uuid_`:

```
UPLOAD_FOLDER    → /data/uploads/<uuid>_<safe_name>    (con prefisso uuid)
CONVERTED_FOLDER → /data/converted/<uuid>.<ext>        (nome = solo uuid)
```

Il problema: `bookish/routes/reader.py` cerca i file **per nome**
(`UPLOAD_FOLDER/<filename>`, poi `CONVERTED_FOLDER/<filename>`),
mentre il flusso reale usa:

- upload: `<uuid>_<filename>` — corretto, perché `stored_filename` viene
  passato dalla UI (funziona già).
- convertito: `<uuid>.ext` — il reader NON cerca mai `<uuid>.ext` con il
  nome completo generato;
  cerca il `<filename>` originale, che il file convertito non ha.

Quindi:

- il link "Leggi nel reader" dalla UI dei **convertiti funziona** (il JS
  passa `stored_filename`);
- ma i **link diretti `/reader/<nome_originale>`** (es. da localStorage,
  `recenti`, o bookmark a mano) restituiscono **404** per il converted
  (il reader non risolve `<uuid>` né l'originale).

**Fix proposto** (P0, già coperto da test `tests/test_storage_paths.py`):

1. Nel reader, risolvere il file per il suo **stored id** applicando le due
   regole insieme:
   - cercare prima `UPLOAD_FOLDER/<filename>` (caso upload, già ok),
   - poi `CONVERTED_FOLDER/<filename>` (caso convertito — **aggiungere**
     la ricerca diretta per il nome generato `<uuid>.ext`);
2. In più cercare per suffisso per non rompere i bare-name (fallback).
3. Mantenere `os.path.basename()` e la whitelist estensioni.

**Stato (R1)**: ✅ fatto — il reader ora risolve i file convertiti per ID
(compresi i link diretti `recent`/localStorage con bare name).

### 3.2 🟠 [Security] Nessuna autenticazione → disk DoS

**Stato (R2)**: ✅ fatto in `bookish/services/security.py` — auth Basic
opzionale (`BOOKISH_AUTH_USERNAME`/`BOOKISH_AUTH_PASSWORD`) con
`WWW-Authenticate` su tutte le route tranne healthz/static + rate-limit
in-memory su POST /upload (`BOOKISH_RATE_LIMIT_UPLOAD`, default 20/ora,
0=off).

Note:
- Basic su HTTP non cifra le credenziali → abilitare dietro proxy HTTPS.
- Rate limit in-memory (singolo worker) → ok per uso personale; per difese
  robuste aggiungere limite a livello proxy.

### 3.3 🔴 [Sicurezza] XSS nel reader HTML

`bookish/static/js/reader.js:165`:

```js
viewer.innerHTML = text;   // file HTML grezzo iniettato nel DOM
```

Un file HTML generato da Pandoc/WeasyPrint (o un file con HTML sorgente)
può eseguire script nel dominio delle API.  Certo è "self-hosted file non
affidabili", ma un libro malevolo può esfiltrare bookmarks/cookies.

Mitigazioni:

1. **CSP restrittiva**: header `Content-Security-Policy` con
   `default-src 'self'` e allowlist CDN per epub.js/pdf.js/jszip.
   I contenuti iniettati non possono eseguire `script-src` di terze parti.
2. Sanitizzazione HTML (es. `DOMPurify` client-side, o `bleach` server-side)
   del contenuto iniettato.
3. **Sandbox iframe**: rendere il contenutO in `<iframe sandbox>` senza
   script — fa perdere funzionalità ma è il fix più robusto.

**Stato (R3)**: ✅ fatto —
1. CSP applicata a tutte le risposte via middleware (`object-src 'none'`,
   `base-uri 'self'`, `form-action 'self'`, allowlist CDN) + headers
   `X-Content-Type-Options: nosniff`, `X-Frame-Options: SAMEORIGIN`,
   `Referrer-Policy: no-referrer`.
2. Sanitizzazione lato client dell'HTML iniettato nel reader con
   **DOMPurify self-hosted** (`static/vendor/purify.min.js`, v3.1.6).
3. MD/TXT/FB2/DOCX passano già da `escapeHtml` (sicuri).

### 3.4 🟠 [Sicurezza] Secret di default + no lockfile

- `SECRET_KEY` di default `"cambiami-in-produzione"` accettata in DEBUG. In
  Docker `DEBUG=false` quindi il guard funZiona. OK ma `BOOKISH_SECRET`
  shorthands non validate (lunghezza/entropia).
- `requirements.txt` pinned "alla cieca": ness un lockfile riproducibile per
  il layer pip del Docker builder → build non-reproducible; aggiornamenti di
  patch non tracciati.

**Fix**: `constraints.txt` o lockfile (`pip freeze` nelle release) e archiviare
il lock nel repo; oppure `pip-tools`. Verificare sha/checksum opzionale.

### 3.5 🟠 [Stabilità] `tmpfs /tmp` e conversioni pesanti

WeasyPrint/`pandoc` in /tmp: `tmpfs` piccola di default (è `/tmp` su `/`).
Il compose monta `tmpfs: - /tmp` → i file temporanei di conversione
vengono scritti in RAM → rischio `No space left`/OOM con PDF grandi.

**Valutazione**: con `mem_limit: 1g` e PDF fino a 200 pagine è marginale,
ma è un limite implicito nondocumentato.

**Fix proposto**: documentare, oppure usare il volume dati per tmp
(`/data/tmp`), oppure aumentare `--shm-size`.

### 3.6 🟡 [Build] Immagine senza lock e senza test

- Nessun `pytest`/test nel repo → regressioni possibili.
- Il build Docker non esegue smoke test automatici dell'app.

### 3.7 🟡 [Maintenability] `requirements.txt` include solo run e builder

Il builder copia `requirements.txt` e installa tutto — incluso WeasyPrint
che il runtime usa. Va bene, ma la riga
`pypandoc==1.15` polarizza la ricerca del formato… `pypandoc.get_pandoc_formats()`
richiede il binary `pandoc` in PATH — nel container è installato via apt ✔.

### 3.8 🟡 [Ops] Gunicorn

- `threads=8` con `workers=1` + SSE in-memory → OK per single-user, ma se il
  worker muore, i task in corso si perdono (documentato).
- `timeout=300` (5 min): le conversioni WeasyPrint di libri lunghi possono
  superarlo → il worker viene killato senza errore chiaro.
- `proxy_allow_from = "*"` era un'opzione **inesistente** (gunicorn la
  ignora silenziosamente senza errori) → rimossa.
- `forwarded_allow_ips = "*"` dava fiducia a qualsiasi `X-Forwarded-For`
  da internet → ora in configurabile via `FORWARDED_ALLOW_IPS` (default
  `127.0.0.1`) e **non più wildcard**.

**Stato (R6)**: ✅ risolto in `gunicorn.conf.py` — opzioni inesistenti
rimosse, trust proxy ristretto via env.

### 3.9 🟡 [Observability]

- Unico log: gunicorn `accesslog/errorlog "-"` and `app.logger` non
  configurate (DEBUG/INFO default). Nessun structured logging, nessuna
  metrica (prometheus/flask-exporter assente).

---

## 4. Punti di debolezza perchè lo chiedi: read critica

### 4.1 Testing
- **Zero test** automatici. `bookish/` ha import di prova sboccata da noi
  dopo — significa **nessun gate** per le correzioni Docker.

### 4.2 UI
- Reader self-hosted: epub.js/pdf.js+worker/jszip sono in `static/vendor/`
  (no CDN, funziona offline). ✅ fatto — resta il tema CSP `'unsafe-inline'`
  per lo script inline del template (marginale).

### 4.3 Multi-user
- `workers=1` by design per gli SSE: **no scalabilità orizzontale**, no
  multiutente. OK per lo scope dichiarato — ma la doc lo dice?

### 4.4 Storage
- Upload e converted separati (uploads/ + converted/) → backup e reasoning
  più complicati. Un solo `data/files` sarebbe più semplice.

---

## 5. Matrice dei rischi

| # | Rischio | Gravità | Probabilità | Stato |
|---|---------|---------|-------------|-------|
| R1 | Reader 404 su file convertiti | Alta | Alta | ✅ risolto |
| R2 | Disk/CPU DoS via upload non autenticato | Alta | Media | ✅ mitigato (auth+rate) |
| R3 | XSS da file HTML malevoli | Alta | Bassa | ✅ mitigato (CSP+DOMPurify) |
| R4 | Build non riproducibile | Media | Media | ✅ lockfile |
| R5 | Errori timeout conversioni lunghe | Media | Media | ⏳ P2 |
| R6 | Trust proxy / opzioni fantasma | Bassa | Media | ✅ risolto |
| R7 | /tmp tmpfs vs conversioni grandi | Bassa | Bassa | ⏳ P1 |

---

## 6. Raccomandazioni prioritizzate (Roadmap)

### P0 — Critico (prima di prod)
- [x] **R1** Fix naming/storage nel reader (file convertiti + upload con
      prefisso uuid). Aggiungere test. ✅ fatto
- [x] **R2** Autentica/rate-limit su `/upload` (auth Basic + rate limit). ✅ fatto
- [x] **R3** CSP + sanitizzazione in reader (CSP headers + DOMPurify). ✅ fatto

### P1 — Alta
- [x] **R4** Lockfile/constraints per dipendenze Python.
      ✅ `requirements.lock` (pip-compile, 112 righe) + Dockerfile lo usa.
- [x] Self-hosting dei JS frontend (per offline).
      ✅ epub.js/pdf.js+worker/JSZip vendored in `static/vendor/`, CSP senza CDN.
- [ ] **R4b** `git` revision pin nel Dockerfile build-arg `--build-arg
      requirements_checksum` (advanced).
- [x] **R6** Cleanup gunicorn.conf (opzioni inesistenti; `forwarded_allow_ips`
      solo per proxy fidato noto). ✅ già fatto
- [x] Smoke test Docker nella CI (`docker build` + `docker compose up` +
      healthcheck + upload/conversion test). ✅ `.github/workflows/ci.yml`

### P2 — Manutenibilità
- [ ] **R5** Retry/status per conversioni lunghe (job esterno opzionale —
      out of scope per ora, ma documentare).
- [ ] **R7** Logging strutturato + metrica basilica (`/metrics` semplice o
      prometheus exporter).
- [x] Self-hosting dei JS frontend (per offline). ✅ fatto — vedi §3.2/5.
- [ ] Valutare volume unico `/data/files` (uploads+converted) per semplificare
      backup e cleanup docs.

### P3 — Stretch (solo se serve)
- [ ] Multi-utente/autenticazione reale.
- [ ] Task queue esterna (Celery/Redis/Dramatiq) per portare SSE e conversione
      fuori dal gunicorn worker.
- [ ] Pagination e scansione `recent` server-side nel reader.