Merge develop into main

CHANGELOG.md

@@ -22,6 +22,66 @@ Features are ordered by implementation priority.
 
 ---
 
+## [0.16.1] — 2026-07-02
+
+### Fixed — Code Review Hardening
+
+#### Detection pipeline
+- **Attachment binary analysis now runs in production** — the parser now retains attachment
+  bytes (`data` key) and `analyze_attachments()` passes them to the binary scanners.
+  VBA macro detection (OLE2/OOXML), PDF JavaScript and suspicious PDF stream checks were
+  previously dead code in the live pipeline (only exercised by tests with synthetic bytes)
+- **Authentication-Results header selection** — SPF/DKIM/DMARC results are now read from
+  the FIRST (topmost) `Authentication-Results` header, the one prepended by the final
+  receiving server. Previously the LAST header was used, which an attacker could inject
+  into the original message to spoof `spf=pass; dkim=pass; dmarc=pass`
+- **Inline attachments analyzed** — MIME parts with a filename but `Content-Disposition:
+  inline` (or none) are now extracted and scanned like regular attachments
+- **Body pattern double-counting removed** — urgency/CTA/credential counters now take the
+  MAX across text sources (plain text, HTML-extracted text, hidden content) instead of
+  summing them. multipart/alternative emails (same content in text+HTML) no longer get
+  doubled counts, duplicated findings and inflated body/NLP scores
+
+#### Correctness
+- **6 missing i18n keys added** (`header.brand_spoofing`, `header.dkim_domain_mismatch`,
+  `url.malicious_cdn`, `body.language_mismatch`, `body.known_campaign`, `analysis.db_error`)
+  — v0.15 findings displayed the raw key instead of a description
+- **WHOIS timeout now effective** — per-call `ThreadPoolExecutor` context managers blocked
+  on `shutdown(wait=True)` until the query completed, making the 8s wall-clock timeout
+  illusory; replaced with a shared executor. URL batch analysis executors now shut down
+  with `wait=False, cancel_futures=True` so `URL_BATCH_TIMEOUT` is actually enforced
+- **Risk label language** — `RISK_LABELS` translations resolved at call time instead of
+  import time; label text now follows runtime language switches via `/api/settings/language`
+- **`_logger` NameError** in `campaign_detector.py` (triggered with >10k emails) — logger
+  now defined
+- **Hostname prefix stripping** — `lstrip("www.")` (strips characters, mangling hosts like
+  `web.example.com` → `eb.example.com`) replaced with `removeprefix("www.")` in reputation
+  indicator extraction (4 occurrences)
+- **Trusted CDN IP over-matching** — prefix match now requires an octet boundary
+  (`"54.1"` no longer matches `54.100.x.x`)
+- **List-Unsubscribe domain check bypass** — external-domain comparison now uses dot-boundary
+  subdomain matching (`evilpaypal.com` no longer passes as internal to `paypal.com`)
+- **Brand spoofing false positives** — brand aliases matched with word boundaries
+  (e.g. "visa" no longer matches inside "advisor")
+- **IPv4 validation** — direct-IP URL detection validates octets 0-255 via `ipaddress`
+  (pattern `999.999.999.999` no longer flagged as IP)
+- **`mail_to` consistency** — stored as JSON in both upload and manual pipelines; GET
+  `/api/analysis/{job_id}` now returns it as a list (same shape as POST)
+- **Manual analysis parity** — `/api/manual/` now passes `header_result` to `analyze_body()`
+  so the NLP model receives real SPF/DKIM/DMARC flags (was always False)
+- **SPA fallback** — unknown `/api/*` paths now return JSON 404 instead of the SPA HTML page
+- **Unbounded upload memory read** — `POST /api/upload/` read the entire request body into
+  memory via `file.read()` before checking it against `MAX_UPLOAD_SIZE_MB`, so an oversized
+  upload was still fully buffered before being rejected (potential memory-exhaustion DoS on
+  internet-facing deployments without a reverse-proxy body-size limit). Now reads in 1MB
+  chunks and rejects as soon as the configured limit is exceeded, without buffering the rest
+
+### Notes
+- All 123 tests passing (1 skipped), zero regressions
+- No API schema changes; PATCH release per SemVer
+
+---
+
 ## [0.16.0] — 2026-06-29
 
 ### Changed — .msg Backend Abstraction & GPL License Resolution

README.md

@@ -97,6 +97,8 @@ Email (.eml / .msg / plain text)
 
 ## 🔧 Version
 
+**v0.16.1** — 🔧 Hardening release: Enabled attachment binary analysis in the live pipeline (VBA macros, PDF JavaScript, suspicious streams), fixed Authentication-Results header selection (anti-spoofing), inline attachment extraction, removed body pattern double-counting on multipart emails, added 6 missing i18n keys, made WHOIS/URL batch timeouts effective, plus 10 correctness fixes. All 123 tests passing ✅, zero regressions.
+
 **v0.16.0** — 🏗️ Architecture release: Migrated .msg parsing to python-oxmsg (MIT license), eliminated GPL violation, introduced MsgBackend abstraction for pluggable implementations, unblocked beautifulsoup4 to 4.14.0, added transport headers support (SPF/DKIM/DMARC for .msg files), RTF-only email support with optional RTFDE fallback. All 122 tests passing ✅, production-ready.
 
 📖 **See full version history** → [CHANGELOG.md](./CHANGELOG.md)

backend/api/routes/analysis.py

@@ -8,7 +8,7 @@
 from pathlib import Path
 from dataclasses import asdict
 
-from fastapi import APIRouter, HTTPException, Depends
+from fastapi import APIRouter, HTTPException, Depends, Request
 from fastapi.concurrency import run_in_threadpool
 from pydantic import BaseModel
 from sqlalchemy.ext.asyncio import AsyncSession
@@ -19,6 +19,7 @@ class NotesUpdate(BaseModel):
     notes: str = ""
 
 from sqlalchemy import text
+from core.rate_limiting import limiter
 from models.database import get_session, EmailAnalysis, engine
 from utils.config import settings
 from utils.i18n import t
@@ -105,7 +106,9 @@ class BulkDeleteRequest(BaseModel):
 
 
 @router.post("/bulk-delete")
+@limiter.limit("5/minute")
 async def bulk_delete_analyses(
+    request: Request,
     body: BulkDeleteRequest,
     db: AsyncSession = Depends(get_session),
 ):
@@ -145,7 +148,9 @@ async def bulk_delete_analyses(
 
 
 @router.post("/{job_id}")
+@limiter.limit("10/minute")
 async def run_analysis(
+    request: Request,
     job_id: str,
     do_whois: bool = True,
     db: AsyncSession = Depends(get_session),
@@ -159,6 +164,18 @@ async def run_analysis(
 
     # Recupera il file
     file_path = _find_upload_file(job_id)
+
+    # Verifica dimensione file prima di leggere in RAM (non affidarsi solo al chunking upload)
+    MAX_SIZE = settings.MAX_UPLOAD_SIZE_MB * 1024 * 1024
+    file_size = file_path.stat().st_size
+    if file_size > MAX_SIZE:
+        _logger.warning("[%s] [FILE SIZE EXCEEDED] File size %d bytes > limit %d bytes",
+                       job_id, file_size, MAX_SIZE)
+        raise HTTPException(
+            status_code=413,
+            detail=t("upload.too_large", max_mb=settings.MAX_UPLOAD_SIZE_MB)
+        )
+
     raw = file_path.read_bytes()
     original_filename = file_path.name  # es. <uuid>.eml
 
@@ -253,20 +270,16 @@ def _pipeline():
         risk_explanation={"explanation": risk.explanation, "contributions": _dataclass_to_dict(risk)},
     )
 
-    # Upsert: se già esiste (riesecuzione analisi), aggiorna
-    # IMPORTANT: atomic transaction — delete + add in single commit to avoid race conditions
-    existing = await db.get(EmailAnalysis, job_id)
-    if existing:
-        _logger.info("[%s] [DB DELETE] Existing analysis found, deleting for upsert", job_id)
-        await db.delete(existing)
-        _logger.info("[%s] [DB DELETE] Existing record marked for deletion", job_id)
-
-    _logger.info("[%s] [DB ADD] Adding new EmailAnalysis record to session", job_id)
+    # Upsert: idempotent merge per evitare race conditions tra get() e add()
+    # SQLAlchemy merge() è atomico: se il record esiste, lo aggiorna; se no, lo crea.
+    # Questo previene la finestra di tempo dove un'altra richiesta potrebbe modificare il record.
+    _logger.info("[%s] [DB UPSERT] Merging EmailAnalysis record (atomic upsert)", job_id)
     try:
-        db.add(record)
-        _logger.info("[%s] [DB COMMIT] Committing transaction to database (includes delete if applicable)", job_id)
+        # merge() è atomico in SQLAlchemy 2.0+: no race condition tra get() e add()
+        merged_record = await db.merge(record)
+        _logger.info("[%s] [DB COMMIT] Committing transaction to database", job_id)
         await db.commit()
-        _logger.info("[%s] [DB SUCCESS] Analysis persisted successfully, record_id=%s", job_id, record.id)
+        _logger.info("[%s] [DB SUCCESS] Analysis persisted successfully, record_id=%s", job_id, merged_record.id)
     except Exception as e:
         _logger.error("[%s] [DB ERROR] Failed to commit to database: %s", job_id, str(e))
         await db.rollback()
@@ -375,6 +388,18 @@ def _build_response_from_record(record) -> dict:
     ai = record.attachment_indicators or {}
     ri = record.risk_explanation or {}
 
+    # mail_to è salvato come JSON (lista serializzata): decodifica per
+    # restituire la stessa struttura del POST. Fallback al valore grezzo
+    # per record storici salvati in altri formati.
+    mail_to = record.mail_to
+    if isinstance(mail_to, str):
+        try:
+            decoded = json.loads(mail_to)
+            if isinstance(decoded, list):
+                mail_to = decoded
+        except (ValueError, TypeError):
+            pass
+
     return {
         "job_id": record.id,
         "status": "completed",
@@ -383,7 +408,7 @@ def _build_response_from_record(record) -> dict:
             "filename":        record.filename,
             "subject":         record.mail_subject,
             "from":            record.mail_from,
-            "to":              record.mail_to,
+            "to":              mail_to,
             "date":            record.mail_date,
             "message_id":      record.message_id,
             "file_hash_sha256": record.file_hash_sha256,
@@ -580,6 +605,8 @@ def _build_response(job_id, parsed, header_result, body_result, url_result, atta
     ]
 )
 
+# Allowlist di tag HTML sicuri per email preview.
+# Tag esclusi per protezione: script, style, iframe, form, input, meta, link (no remote resources/code execution)
 _BLEACH_ALLOWED_TAGS = [
     'a', 'abbr', 'acronym', 'b', 'blockquote', 'br', 'caption',
     'cite', 'code', 'dd', 'del', 'dfn', 'div', 'dl', 'dt', 'em',
@@ -733,8 +760,26 @@ async def delete_analysis(
     if not record:
         raise HTTPException(status_code=404, detail=t("analysis.not_found"))
 
-    await db.delete(record)
-    await db.commit()
-    files_removed = _cleanup_files(job_id)
-    await _vacuum_db()
+    # Cleanup file first, then delete from DB (atomic semantic)
+    # If file cleanup fails, DB delete won't execute
+    try:
+        files_removed = _cleanup_files(job_id)
+    except Exception as e:
+        _logger.error("[%s] [FILE CLEANUP ERROR] Failed to delete files: %s", job_id, e, exc_info=True)
+        raise HTTPException(status_code=500, detail=t("analysis.delete_error"))
+
+    # File cleanup succeeded, safe to delete from DB
+    try:
+        await db.delete(record)
+        await db.commit()
+    except Exception as e:
+        _logger.error("[%s] [DB DELETE ERROR] Failed to delete from database: %s", job_id, e, exc_info=True)
+        raise HTTPException(status_code=500, detail=t("analysis.delete_error"))
+
+    # VACUUM is non-critical, don't fail if it errors
+    try:
+        await _vacuum_db()
+    except Exception as e:
+        _logger.warning("[%s] [VACUUM WARNING] Database compaction failed (non-critical): %s", job_id, e)
+
     return {"status": "deleted", "job_id": job_id, "files_removed": files_removed}

backend/api/routes/manual.py

@@ -11,11 +11,12 @@
 import hashlib
 from pathlib import Path
 
-from fastapi import APIRouter, HTTPException
+from fastapi import APIRouter, HTTPException, Request
 from fastapi.concurrency import run_in_threadpool
 from fastapi.responses import JSONResponse
 from pydantic import BaseModel
 
+from core.rate_limiting import limiter
 from utils.config import settings
 from utils.i18n import t
 from core.analysis.email_parser import parse_email_file, raw_looks_like_eml
@@ -35,7 +36,8 @@ class ManualInput(BaseModel):
 
 
 @router.post("/")
-async def analyze_manual(payload: ManualInput):
+@limiter.limit("10/minute")
+async def analyze_manual(request: Request, payload: ManualInput):
     """
     Analizza un sorgente email incollato manualmente.
     Non richiede upload file: accetta il testo come JSON body.
@@ -87,7 +89,9 @@ def _pipeline():
         """
         _parsed            = parse_email_file(raw, payload.filename)
         _header_result     = analyze_headers(_parsed)
-        _body_result       = analyze_body(_parsed)
+        # header_result passa i flag SPF/DKIM/DMARC al classificatore NLP
+        # (stessa pipeline dell'upload file)
+        _body_result       = analyze_body(_parsed, _header_result)
         _url_result        = analyze_urls(_body_result.extracted_urls, do_whois=_do_whois)
         _attachment_result = analyze_attachments(_parsed.attachments)
         _risk              = compute_risk_score(_header_result, _body_result, _url_result, _attachment_result)
@@ -103,7 +107,7 @@ def _pipeline():
         filename=payload.filename,
         file_hash_sha256=parsed.file_hash_sha256,
         mail_from=parsed.mail_from,
-        mail_to=str(parsed.mail_to),
+        mail_to=json.dumps(parsed.mail_to),
         mail_subject=parsed.mail_subject,
         mail_date=parsed.mail_date,
         message_id=parsed.message_id,

backend/api/routes/reputation.py

@@ -116,7 +116,9 @@ def _is_trusted_cdn_ip(ip: str) -> bool:
     if not ip:
         return False
     for trusted_prefix in _TRUSTED_CDN_IPS:
-        if ip.startswith(trusted_prefix):
+        # Il punto finale evita over-matching: "54.1" non deve matchare
+        # "54.100.x.x" o "54.19.x.x" ma solo "54.1.x.x".
+        if ip.startswith(trusted_prefix + "."):
             return True
     return False
 
@@ -178,7 +180,7 @@ def add_url(raw: str, is_suspicious: bool = False) -> None:
         # Estrai hostname dall'URL
         try:
             from urllib.parse import urlparse
-            hostname = urlparse(url).netloc.split(":")[0].lstrip("www.")
+            hostname = urlparse(url).netloc.split(":")[0].removeprefix("www.")
         except Exception:
             hostname = ""
 
@@ -210,7 +212,7 @@ def add_domain(raw: str) -> None:
             return
         try:
             from urllib.parse import urlparse
-            hostname = urlparse(raw).netloc.split(":")[0].lstrip("www.")
+            hostname = urlparse(raw).netloc.split(":")[0].removeprefix("www.")
             # Scarta IP diretti
             if hostname and not (hostname.count(".") < 1 or hostname.replace(".", "").isdigit()):
                 if hostname not in seen_domains and not _is_trusted_cdn(hostname):
@@ -369,7 +371,7 @@ def add_url_if_worth_checking(u: dict) -> None:
         # Estrai hostname e controlla se trusted CDN (escludi sempre)
         try:
             from urllib.parse import urlparse
-            hostname = urlparse(url_str).netloc.split(":")[0].lstrip("www.")
+            hostname = urlparse(url_str).netloc.split(":")[0].removeprefix("www.")
         except Exception:
             hostname = ""
 
@@ -450,7 +452,7 @@ def add_url_if_worth_checking(u: dict) -> None:
         if url_str:
             try:
                 from urllib.parse import urlparse
-                hostname = urlparse(url_str).netloc.split(":")[0].lstrip("www.")
+                hostname = urlparse(url_str).netloc.split(":")[0].removeprefix("www.")
                 is_suspicious = (
                     u.get("is_ip_address") or u.get("is_ip") or
                     u.get("is_shortener") or u.get("is_new_domain") or

backend/api/routes/upload.py

@@ -12,9 +12,10 @@
 import hashlib
 from pathlib import Path
 
-from fastapi import APIRouter, UploadFile, File, HTTPException, status
+from fastapi import APIRouter, UploadFile, File, HTTPException, status, Request
 from fastapi.responses import JSONResponse
 
+from core.rate_limiting import limiter
 from utils.config import settings
 from utils.i18n import t
 
@@ -24,7 +25,8 @@
 
 
 @router.post("/")
-async def upload_email(file: UploadFile = File(...)):
+@limiter.limit("10/minute")
+async def upload_email(request: Request, file: UploadFile = File(...)):
     # 1. Validazione nome file e estensione
     if not file.filename:
         raise HTTPException(status_code=400, detail=t("upload.no_filename"))
@@ -36,15 +38,29 @@ async def upload_email(file: UploadFile = File(...)):
             detail=t("upload.unsupported_format", ext=ext, allowed=settings.ALLOWED_EXTENSIONS),
         )
 
-    # 2. Lettura e validazione dimensione
-    raw = await file.read()
+    # 2. Lettura a chunk con controllo progressivo della dimensione.
+    # Legge al massimo MAX_SIZE+1 byte prima di rifiutare: un client che
+    # dichiara (o invia) un body enorme non riesce a far bufferizzare al
+    # server più dati del limite configurato, indipendentemente dalla
+    # dimensione reale della richiesta.
+    _CHUNK_SIZE = 1024 * 1024  # 1 MB
+    chunks: list[bytes] = []
+    total_size = 0
+    while True:
+        chunk = await file.read(_CHUNK_SIZE)
+        if not chunk:
+            break
+        total_size += len(chunk)
+        if total_size > MAX_SIZE:
+            raise HTTPException(
+                status_code=413,
+                detail=t("upload.too_large", max_mb=settings.MAX_UPLOAD_SIZE_MB),
+            )
+        chunks.append(chunk)
+    raw = b"".join(chunks)
+
     if len(raw) == 0:
         raise HTTPException(status_code=400, detail=t("upload.empty_file"))
-    if len(raw) > MAX_SIZE:
-        raise HTTPException(
-            status_code=413,
-            detail=t("upload.too_large", max_mb=settings.MAX_UPLOAD_SIZE_MB),
-        )
 
     # 3. Calcola hash SHA256 del file caricato
     sha256 = hashlib.sha256(raw).hexdigest()

backend/core/analysis/attachment_analyzer.py

@@ -360,7 +360,9 @@ def analyze_attachments(attachments: list[dict]) -> AttachmentAnalysisResult:
     _logger.info("[ATTACH START] Analyzing %d attachments", result.total_attachments)
 
     for att in attachments:
-        analysis = analyze_attachment(att, raw_data=None)
+        # I bytes grezzi (chiave "data" dal parser) abilitano l'analisi binaria:
+        # macro VBA in OLE2/OOXML, JavaScript e stream sospetti nei PDF.
+        analysis = analyze_attachment(att, raw_data=att.get("data"))
         result.attachments.append(analysis)
         if any(f.severity == "critical" for f in analysis.findings):
             result.critical_count += 1