Files
bookish-fork-dokerized/docs/review-docker-2026.md
T
truetype 12d2018ac5 feat(R4b): reproducible builds — pin requirements.lock checksum
- Dockerfile: build-arg REQUIREMENTS_CHECKSUM verifica lo sha256 del
  lockfile (fallisce la build se non coincide; vuoto = skip locale)
- docker-compose.yml: default = hash corrente del lock
- CI: nuovo job checksum.yml che fallisce se il pin non coincide
- Fix YAML preesistente in ci.yml (name: con ':' non quotati)
- Docs: README, .env.example, review aggiornati
2026-08-29 22:36:31 +02:00

11 KiB

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 + R4b checksum lock nel Dockerfile/CI). Restano 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:

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 + checksum pinned
R5 Errori timeout conversioni lunghe Media Media ⏳ P2
R6 Trust proxy / opzioni fantasma Bassa Media ✅ risolto
R7 /tmp tmpfs vs conversioni grandi Bassa Bassa ⏳ P2

6. Raccomandazioni prioritizzate (Roadmap)

P0 — Critico (prima di prod)

  • R1 Fix naming/storage nel reader (file convertiti + upload con prefisso uuid). Aggiungere test. ✅ fatto
  • R2 Autentica/rate-limit su /upload (auth Basic + rate limit). ✅ fatto
  • R3 CSP + sanitizzazione in reader (CSP headers + DOMPurify). ✅ fatto

P1 — Alta

  • R4 Lockfile/constraints per dipendenze Python. ✅ requirements.lock (pip-compile, 112 righe) + Dockerfile lo usa.
  • Self-hosting dei JS frontend (per offline). ✅ epub.js/pdf.js+worker/JSZip vendored in static/vendor/, CSP senza CDN.
  • R4b Hash del lockfile pinnato nel Dockerfile (build-arg REQUIREMENTS_CHECKSUM, default in compose) + job CI di verifica (.github/workflows/checksum.yml). ✅ fatto
  • R6 Cleanup gunicorn.conf (opzioni inesistenti; forwarded_allow_ips solo per proxy fidato noto). ✅ già fatto
  • 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).
  • 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.