Skip to content

AUDIT_FASE0.md

Latest commit

 

History

History
396 lines (306 loc) · 21.3 KB

AUDIT_FASE0.md

File metadata and controls

396 lines (306 loc) · 21.3 KB
Raw

AUDIT FASE 0 — verifica delle assunzioni sul codice ereditato

Oggetto: Rizzo-AI-Academy/rizzo-pii @ 96e863c, importato in questo fork come base. Metodo: lettura del sorgente. Ogni verdetto cita file:riga ed è accompagnato dall'estratto rilevante. Nessuna affermazione qui deriva dalla documentazione del progetto. Deliverable di: §3 del piano di lavoro.

Il piano è stato redatto leggendo README.md e CLAUDE.md, non il codice. Questo documento esiste per correggere la rotta prima di costruirci sopra: tre assunzioni su dieci sono risultate sbagliate o parziali, e due di esse cambiano il contenuto dei work package a valle (WP1.2 e WP9.1). Le correzioni sono in §3.

1. Esito sintetico

ID Assunzione Esito Impatto sul piano
A1 Endpoint Flask senza autenticazione CONFERMATA WP1.1 invariato
A2 Nessuna validazione Origin/Host → DNS rebinding CONFERMATA (aggravata) WP1.1 invariato, +POST /config
A3 Dev server Werkzeug anche in produzione CONFERMATA WP1.1 invariato
A4 Dizionario reversibile in chiaro su disco CONFERMATA (in forma diversa da quella ipotizzata — vedi §2.A4 e la rettifica) WP1.2 da riscrivere
A5 Nessuna suite di test né CI SMENTITA WP9.1 da riscrivere
A6 Restore con matching fuzzy CONFERMATA (aggravata: è client-side) WP5.1 invariato
A7 Nessun escaping dei placeholder presenti in input CONFERMATA WP5.2 invariato
A8 Chunking con overlap, dedup su offset globali senza test sui bordi PARZIALE WP5.4 ridimensionato
A9 Soglia O-vs-entità non esposta (argmax secco) CONFERMATA WP3.1 invariato
A10 Rete regex+checksum duplicata app.py ↔ validate_checksums.py PARZIALE WP4.1 ridimensionato

2. Verifica per assunzione

A1 — L'endpoint Flask non ha autenticazione → CONFERMATA

Nessun before_request, nessun decoratore di autorizzazione, nessuna lettura di header di autenticazione in tutto src/app/:

$ grep -rn "Authorization\|before_request\|after_request\|X-.*Token\|compare_digest" src/app/
(nessun risultato)

Tutti i 14 endpoint sono raggiungibili da chiunque possa aprire una connessione TCP verso la porta. src/app/app.py:519:

@app.route("/analyze", methods=["POST"])
def analyze_route():

Il corpo della risposta di /analyze contiene il dizionario placeholder → valore reale (src/app/app.py:421), cioè le PII in chiaro. GET /doc/<doc_id>/file.pdf (src/app/app.py:714) restituisce il PDF caricato: il doc_id è un secrets.token_urlsafe(12) (src/app/app.py:163), quindi non indovinabile, ma è comunque un capability token trasmesso in chiaro e senza scadenza fino alla morte del processo.

A2 — Nessuna validazione di Origin/Host → CONFERMATA, e più grave del previsto

Stessa grep di A1: nessun controllo di Host, nessun controllo di Origin, nessuna configurazione CORS (Flask di default non emette Access-Control-Allow-Origin, quindi non c'è un wildcard da rimuovere — la §WP1.1.3 del piano non ha nulla da correggere qui).

Due scenari, da distinguere perché hanno gravità diversa:

  1. CSRF semplice (già possibile oggi). L'assenza di header CORS impedisce a una pagina ostile di leggere la risposta, ma non le impedisce di inviare la richiesta. E /analyze accetta anche multipart/form-data (src/app/app.py:530, request.form.get("include_mapping")), che è una "simple request": nessun preflight, la richiesta parte. Peggio, POST /config (src/app/app.py:776) è scrivibile senza autenticazione e persiste l'host di bind su disco:

    @app.route("/config", methods=["POST"])
def config_post():
    data = request.get_json(silent=True) or {}
    host = str(data.get("host", server_config.DEFAULT_HOST)).strip()
    ...
    server_config.save_config(host, port)

    host non è validato in alcun modo. Una pagina web qualunque può scrivere {"host": "0.0.0.0", "port": 5005} in config.json: al riavvio successivo l'app espone il proprio endpoint — dizionario compreso — a tutta la rete locale. È una escalation persistente ottenuta con una singola richiesta cross-origin, e la vittima non vede nulla. (Il Content-Type: application/json richiederebbe un preflight; ma get_json(silent=True) fallisce restituendo {} e il codice prosegue coi default — quindi la strada praticabile è il rebinding del punto 2, che rende il preflight irrilevante.)

  2. DNS rebinding (lettura completa). L'attaccante fa risolvere evil.com a 127.0.0.1: da quel momento il browser considera http://evil.com:5005 same-origin rispetto alla pagina ostile, la SOP non protegge più nulla, e la pagina legge le risposte di /analyze — cioè le PII in chiaro — e scarica i PDF da /doc/<id>/file.pdf. L'unica difesa applicabile è il controllo dell'header Host, che oggi non esiste.

A3 — Dev server Werkzeug anche in produzione → CONFERMATA

Tutti e tre gli entry point chiamano app.run(), che è il server di sviluppo di Werkzeug:

  • src/app/serve.py:56 — sidecar Tauri, cioè il percorso di produzione: 
    app.run(host=HOST, port=PORT, threaded=True)
  • src/app/desktop_app.py:41 — app desktop standalone
  • src/app/app.py:1936 — avvio da riga di comando

requirements.txt elenca flask e nessun server WSGI di produzione: niente waitress, niente gunicorn. Non esiste alcun flag --dev che distingua i due casi.

A4 — Il dizionario reversibile è scritto in chiaro su disco → CONFERMATA

Rettifica. Questa voce era stata chiusa come SMENTITA sulla base del solo codice Python. Era sbagliata: la persistenza c'era, la faceva il JavaScript della pagina, e leggendo app.py dall'alto — dove sta il server — non si vedeva. È emersa cablando il vault. Il verdetto corretto è CONFERMATA; l'analisi del lato server qui sotto resta valida ed è la parte che spiega dove il rischio si sposta. La lezione è quella della regola 1 del piano, applicata anche a chi scrive l'audit: la prova sta nel codice, e il codice di questa app è per due terzi in una stringa di 1100 righe dentro app.py.

Il lato server non persiste mai il dizionario, il lato browser sì.

src/app/app.py:1800 (nel corpo della funzione run() della pagina):

if(d.mapping_enabled!==false){MAP=d.mapping;localStorage.setItem('pii_map',JSON.stringify(MAP));}

localStorage è una scrittura su disco, nel profilo del browser o della WebView, in chiaro, senza scadenza e senza che l'utente lo sappia. Il commento del codice originale a src/app/app.py:1984 lo dice a chiare lettere — «senza queste tre righe il dizionario resta in MAP e su disco» — quindi la cosa era nota a chi l'ha scritta; quello che manca è il trattamento che le compete.

Il resto dell'analisi del lato server. Non c'è alcuna scrittura su disco della mappa placeholder → valore fatta da Python: analyze() la costruisce in memoria (src/app/app.py:381-392), la restituisce nella risposta HTTP (src/app/app.py:421) e la abbandona. Non esiste un endpoint /restore: il ripristino è interamente client-side, in JavaScript, dentro la pagina (src/app/app.py:1720-1735). Anche i PDF caricati stanno in una LRU in memoria e mai su disco, per scelta esplicita e documentata (src/app/app.py:141-143).

Oltre alla persistenza in localStorage, il dizionario è quindi esposto in altri tre punti che il piano non nominava:

  1. Il dizionario attraversa HTTP in chiaro a ogni /analyze. Su loopback è un problema contenuto, ma è esattamente ciò che il DNS rebinding di A2 rende leggibile da remoto.
  2. Il dizionario vive nel DOM del browser per tutta la sessione (variabile MAP, src/app/app.py:1312 e seguenti). Ogni estensione con permesso di lettura sulla pagina lo vede. Nel sidecar Tauri è una WebView, non un browser con estensioni — ma il percorso python app.py + browser dell'utente è documentato e supportato (src/app/app.py:39).
  3. Il pulsante "Scarica dizionario" (src/app/app.py:1185) produce un .json in chiaro nella cartella Download, senza cifratura e senza avviso. È l'unico modo per ripristinare dopo aver chiuso l'app (src/app/app.py:1224), quindi non è un caso limite: è il flusso d'uso normale per un documento lavorato in più sessioni.

Conseguenza per WP1.2: cifrare "il dizionario a riposo" resta necessario, ma il file da cifrare non esisteva: andava prima creato lato server, spostando lì la persistenza che il browser faceva in chiaro. Il work package va riscritto — vedi §3.1.

A5 — Non esiste suite di test né CI → SMENTITA

Esistono entrambe.

  • tests/ contiene 8 file; python -m unittest discover tests esegue 35 test, tutti verdi (2 skip), in 0,07 s.
  • .github/workflows/tests.yml gira su push:main e su ogni PR, matrice Python 3.11 / 3.13 / 3.14, con concurrency e cancel-in-progress.
  • La scelta di non installare dipendenze in CI è deliberata e motivata nel workflow stesso (.github/workflows/tests.yml:36-42): la suite gira sulla sola libreria standard perché detectors.py e i generatori non importano torch.

Quello che manca è diverso da quello che il piano assumeva:

Presente Assente
unittest discover su 3 versioni di Python soglia di copertura
test su CF/omocodie, IBAN stampato, targhe, confini di parola, DOCID/date lint (ruff) e type check (mypy)
fixture economico-giuridiche versionata job che blocchi PII o segreti nei file versionati
— qualunque test su chunk_text, sui bordi di chunk e sul round-trip
— qualunque test property-based (hypothesis non è tra le dipendenze)

Conseguenza per WP9.1: non si costruisce una CI da zero, si estende quella esistente. Vedi §3.2.

A6 — Il restore usa matching fuzzy → CONFERMATA, e client-side

src/app/app.py:1729-1731:

// tollerante: parentesi opzionali / spazi, eventuale grassetto markdown
const rx=new RegExp('\\**\\[?\\s*'+inner.replace(/[.*+?^${}()|[\]\\]/g,'\\$&')+'\\s*\\]?\\**','g');
out=out.replace(rx,MAP[ph].replace(/\$/g,'$$$$'));

Le parentesi quadre sono opzionali (\[? … \]?). Il rischio non è teorico: con il dizionario che contiene [FULLNAME_1], la stringa FULLNAME_1 scritta senza parentesi in mezzo a un discorso viene comunque sostituita col valore reale. C'è una mitigazione parziale — le chiavi sono ordinate per lunghezza decrescente (src/app/app.py:1725) proprio per evitare che FULLNAME_1 catturi dentro FULLNAME_10 — ma la sostituzione resta senza alcuna verifica di integrità: non c'è modo di accorgersi che un placeholder è stato alterato dall'LLM. Conferma la necessità del check HMAC di WP5.1 e del principio fail-closed.

A7 — Nessun escaping dei placeholder presenti nell'input sorgente → CONFERMATA

src/app/app.py:389 genera il placeholder senza verificare che la stringa non compaia già nel documento sorgente:

ph = f"[{e['label']}_{counters[e['label']]}]"

Se il documento contiene letteralmente [FULLNAME_1] — caso tutt'altro che esotico: un documento già passato per questo strumento, o un template — quell'occorrenza sopravvive intatta nel testo anonimizzato e, al restore, viene sostituita col valore di un'altra entità. Corruzione silenziosa, esattamente lo scenario di WP5.2.

A8 — Chunking con overlap, dedup su offset globali senza test sui bordi → PARZIALE

Il meccanismo è più solido di quanto il piano assumesse:

  • chunk_text (src/app/app.py:253-267) taglia su confini di parola e restituisce (sottostringa, offset_globale); lo slice è esatto (src/app/app.py:263), quindi gli offset del modello si riportano al testo globale con una somma, senza riallineamenti fragili.
  • _merge (src/app/app.py:299-357) elimina le sovrapposizioni con una ricerca binaria su kept (già ordinata), e questa parte è stata ottimizzata su un caso reale da 40 000 entità (commento a src/app/app.py:315-318).
  • Esiste un allineamento ai confini di parola con estensione della span (src/app/app.py:337-343), il cui principio dichiarato — "nel dubbio si maschera un carattere in più" — è già quello che WP5.4 chiede.

Confermato invece il buco di test: grep -rn "chunk_text\|MAX_WORDS\|OVERLAP" tests/ non trova nulla. Nessun test copre il chunking, e MAX_WORDS=120 con OVERLAP=20 (src/app/app.py:111-112) significa che un'entità può cadere sul bordo.

Una differenza da registrare, perché contraddice il piano: nella zona di overlap la politica di conflitto non è "estensione massima vince", è greedy per priorità (checksum-valido > regex non-soft > score > lunghezza, src/app/app.py:305-312). Il primo candidato accettato esclude i sovrapposti. WP5.4 deve quindi cambiare una politica esistente, non introdurne una.

A9 — La soglia di decisione O-vs-entità non è esposta → CONFERMATA

src/app/app.py:119-125:

nlp = pipeline(
    "token-classification",
    model=MODEL_DIR,
    tokenizer=MODEL_DIR,
    aggregation_strategy="simple",
    device=device,
)

La pipeline di transformers decide per argmax. Lo score esce (src/app/app.py:284) ed è usato solo come criterio di ordinamento in _merge, mai come soglia. Non esiste alcun parametro configurabile che sposti il punto di lavoro. Identico in src/training/test_pii.py:66-72. Oggi il recall non è regolabile: è quello che è.

A10 — Rete regex+checksum duplicata → PARZIALE: l'estrazione è già fatta a metà

Il lavoro principale di WP4.1 è già stato fatto upstream. La rete vive in src/app/detectors.py (294 righe, import re come unica dipendenza), e app.py la importa ri-esportandone i nomi per compatibilità (src/app/app.py:62-65):

# Rete REGEX + CHECKSUM: modulo a parte, senza dipendenze dal modello. I nomi
# restano importabili da qui (`app.detect_regex`) per non rompere chi li usa.
from detectors import (DETECTORS, SOFT_REGEX_LABELS, cf_ok,  # noqa: F401
                       detect_iban, detect_regex, iban_ok, luhn_ok, piva_ok)

La duplicazione residua è solo verso src/inspect/validate_checksums.py:6-40, che mantiene copie proprie e più povere di iban_ok, piva_ok, cf_ok: nessuna normalizzazione dei separatori, nessun try/except, quindi un IBAN scritto a gruppi di quattro — la forma in cui è stampato su una fattura — viene valutato non valido da quel modulo e valido da detectors.py.

Due vincoli strutturali da risolvere comunque, che il piano non aveva visto:

  • src/ non è un package: nessun __init__.py da nessuna parte, e gli import sono piatti (import server_config, from detectors import ...). Funziona solo perché src/app/ finisce su sys.path quando si lancia da lì o dentro PyInstaller. Il python -m src.eval.leak_rate richiesto da WP2.1 non può funzionare finché questo non cambia.
  • detectors.py sta sotto src/app/, cioè dentro il modulo applicativo, mentre è concettualmente il livello L0 condiviso.

Conseguenza per WP4.1: non è un'estrazione, è uno spostamento src/app/detectors.py → src/core/deterministic.py con package reali, più l'eliminazione della copia in src/inspect/, più i test property-based (quelli sì, interamente da scrivere).

3. Ricalibrazione dei work package

3.1 WP1.2 — riscritto: "Protezione del dizionario reversibile"

L'obiettivo "cifra il file del dizionario" decade: quel file non esiste. Il nuovo obiettivo è ridurre la superficie in cui il dizionario è leggibile in chiaro, nell'ordine:

  1. Il dizionario non deve più uscire dal backend per default. Il ripristino diventa un endpoint server-side autenticato, e la UI smette di tenere MAP nel DOM.
  2. Se il dizionario deve persistere tra sessioni (è il caso d'uso reale del pulsante "Scarica dizionario"), la persistenza va fatta lato server, in un vault cifrato — ed è lì che si applicano AES-256-GCM, keychain di sistema, AAD, retention e purge come da piano originale.
  3. L'export in chiaro va sostituito da un export cifrato con passphrase, con l'avviso esplicito che si tratta di un archivio di dati personali.

Il criterio di accettazione "nessun file su disco contiene PII in chiaro" resta valido e va esteso: nessuna risposta HTTP contiene il dizionario salvo sull'endpoint di restore autenticato.

3.2 WP9.1 — riscritto: "Estendere la CI esistente"

Non creare ci.yml da zero: tests.yml esiste, è ben fatto e la sua scelta di girare senza dipendenze è un pregio da preservare (5 minuti di timeout, zero gigabyte di torch). Da aggiungere: ruff, mypy sui moduli nuovi, copertura con soglia (100% su src/core/deterministic.py), il job che blocca PII e segreti nei file versionati, e hypothesis come dipendenza di test — quest'ultima è l'unica che rompe l'attuale "solo libreria standard", quindi va isolata in un job separato per non rallentare quello veloce.

3.3 WP4.1 — ridimensionato

Da "estrai la rete duplicata" a "sposta detectors.py in src/core/, rendi src/ un package vero, elimina la copia in src/inspect/validate_checksums.py, aggiungi i test property-based". Il grosso del valore del WP è ora nei test, non nel refactoring.

3.4 WP1.1 — un'attività in meno, una in più

  • In meno: non c'è CORS permissivo da rimuovere (§WP1.1.3). Flask non emette header CORS.
  • In più: POST /config accetta un host arbitrario e lo persiste. Va validato (allowlist di indirizzi di loopback, più conferma esplicita per qualunque altro valore) e autenticato, altrimenti il token di sessione protegge /analyze mentre /config resta una porta aperta sul riavvio successivo.

3.5 WP5.4 — cambio di politica, non aggiunta

La politica di conflitto nella zona di overlap esiste già ed è greedy per priorità. Sostituirla con "estensione massima vince" nel profilo documentale è un cambiamento di comportamento su codice ottimizzato per grandi documenti: va misurato, non solo scritto.

4. Mappa dei moduli e dipendenze reali

                      ┌───────────────────────────────────────────┐
   torch/transformers │                                           │
   fitz (PyMuPDF)     ▼                                           │
   flask         src/app/app.py  (1939 righe: server + UI + logica)│
                      │  ├── import pdf_export      (solo re, fitz)│
                      │  ├── import server_config   (stdlib)       │
                      │  └── from detectors import  (solo re)  ◄───┘ nessuna dipendenza pesante
                      │
   entry point ───────┼──► src/app/serve.py        (sidecar Tauri, app.run)
                      ├──► src/app/desktop_app.py  (standalone, app.run + browser)
                      └──► app.py __main__          (CLI, app.run)

   tauri/src-tauri/src/lib.rs ──spawn──► serve.py   (env PII_HOST / PII_PORT)
                              ──legge──► config.json (stessa dir di prefs.json)

   src/training/{train_pii,evaluate_pii,test_pii}.py   ─ torch, transformers
   src/data_pipeline/*.py                              ─ stdlib (+ requests per Gemini)
   src/inspect/validate_checksums.py                   ─ stdlib, COPIA dei validatori
   tests/*.py                                          ─ solo stdlib (2 richiedono il backend attivo)

Osservazioni strutturali rilevanti per le fasi successive:

  • app.py accorpa server HTTP, UI (oltre 1100 righe di HTML/CSS/JS in una stringa), chunking, fusione delle predizioni e generazione dei placeholder. Ogni WP delle fasi 1, 3, 5 e 7 tocca questo file: conviene estrarne il motore (analyze, _merge, chunk_text) prima di moltiplicare le modifiche, ed è ciò che WP4.1 e WP3.1 rendono comunque necessario.
  • Il caricamento del modello avviene a import-time (src/app/app.py:119): importare app per un test significa caricare 0,3 miliardi di parametri. È la ragione per cui i test esistenti che toccano l'HTTP richiedono un backend già avviato invece di usare app.test_client(). Va sciolto prima della Fase 2, altrimenti l'harness di misura eredita lo stesso vincolo.
  • I tre entry point duplicano la stessa sequenza (risolvi config → pre-check porta → carica modello → app.run). Il token di sessione di WP1.1 va introdotto in un punto solo, o si divergerà su tre.

5. Criterio di accettazione della Fase 0

Tutte le 10 assunzioni hanno un esito documentato con riferimento a codice.

Soddisfatto. 10 su 10 con file:riga ed estratto: 7 confermate, 1 smentita, 2 parziali (A4 è stata rettificata da SMENTITA a CONFERMATA durante il WP1.2 — vedi il riquadro in §2.A4). Le quattro ricalibrazioni conseguenti sono in §3 e vanno lette prima di aprire il WP relativo.

Una nota di metodo che vale per chi legge questo documento come garanzia: un audit statico ha trovato 9 verdetti su 10 corretti, e quello sbagliato è emerso solo scrivendo il codice che toccava la stessa area. Vale per il resto del piano: le fasi successive possono smentire questa fase, e quando succede si aggiorna il documento invece di difenderlo.