Merge pull request #50 from maribakulj/claude/code-quality-audit-ACnhK

picarones/web/security.py

@@ -256,7 +256,42 @@ class RateLimiter:
 # CSP middleware
 # ---------------------------------------------------------------------------
 
-#: Politique CSP par défaut.
+def is_huggingface_space() -> bool:
+    """Vrai si l'instance tourne dans un HuggingFace Space.
+
+    HuggingFace injecte ``SPACE_ID`` (au format ``user/space``) dans
+    l'environnement du container — c'est le marqueur canonique
+    documenté par HuggingFace, présent quel que soit le SDK (Docker,
+    Streamlit, Gradio…). On l'utilise pour adapter automatiquement la
+    CSP : un Space est servi via une ``<iframe>`` côté
+    ``huggingface.co`` / ``*.hf.space``, donc ``frame-ancestors 'none'``
+    et ``X-Frame-Options: DENY`` rendent la SPA invisible (page blanche
+    bien que le serveur réponde).
+    """
+    return bool(os.environ.get("SPACE_ID", "").strip())
+
+
+#: Origines autorisées à embarquer la SPA dans une iframe quand on tourne
+#: dans un HuggingFace Space. ``huggingface.co`` est l'origine du Hub qui
+#: rend la page parente, ``*.hf.space`` est le domaine où HF expose les
+#: containers Space (utilisé par certains rendus directs et liens
+#: partageables).
+_HF_FRAME_ANCESTORS = "'self' https://huggingface.co https://*.hf.space"
+
+
+def _frame_ancestors_directive() -> str:
+    """Retourne la directive ``frame-ancestors`` adaptée au déploiement.
+
+    - Local / institutionnel : ``'none'`` (pas d'embed possible).
+    - HuggingFace Space : autorise ``huggingface.co`` et ``*.hf.space``
+      pour que la SPA s'affiche dans l'iframe du Space sans tomber en
+      page blanche.
+    """
+    return f"frame-ancestors {_HF_FRAME_ANCESTORS}" if is_huggingface_space() else "frame-ancestors 'none'"
+
+
+#: Politique CSP par défaut (sans la directive ``frame-ancestors``, qui est
+#: composée dynamiquement par :func:`get_csp_policy` selon le déploiement).
 #:
 #: Sprint 25 a extrait tout le JavaScript de la SPA (~1131 lignes) dans
 #: ``picarones/web/static/web-app.js`` — c'est la victoire concrète. Reste
@@ -266,29 +301,51 @@ class RateLimiter:
 #: avec l'extraction des templates pour limiter les risques de régression).
 #: ``style-src`` reste sur ``'unsafe-inline'`` pour les ``style="..."``
 #: sémantiques dans les partials (états vert/rouge/jaune).
-DEFAULT_CSP = (
+_CSP_BASE = (
     "default-src 'self'; "
     "script-src 'self' 'unsafe-inline'; "
     "style-src 'self' 'unsafe-inline'; "
     "img-src 'self' data: blob:; "
     "font-src 'self' data:; "
     "connect-src 'self'; "
-    "frame-ancestors 'none'; "
     "base-uri 'self'; "
     "form-action 'self'"
 )
 
+#: Politique CSP complète exposée pour rétrocompatibilité (mode local
+#: strict). En production HuggingFace, :func:`get_csp_policy` la
+#: recompose dynamiquement avec ``frame-ancestors`` permissif.
+DEFAULT_CSP = _CSP_BASE + "; frame-ancestors 'none'"
+
 
 def get_csp_policy() -> str:
-    """Retourne la CSP à appliquer (override possible via env)."""
-    return os.environ.get("PICARONES_CSP", DEFAULT_CSP)
+    """Retourne la CSP à appliquer (override possible via env).
+
+    Si ``PICARONES_CSP`` est défini, il prend précédence absolue —
+    l'admin sait ce qu'il fait. Sinon, on compose ``_CSP_BASE`` plus la
+    directive ``frame-ancestors`` adaptée à l'environnement détecté
+    (HF Space ou local).
+    """
+    override = os.environ.get("PICARONES_CSP")
+    if override:
+        return override
+    return f"{_CSP_BASE}; {_frame_ancestors_directive()}"
 
 
 async def csp_middleware(request, call_next):
-    """Middleware FastAPI : ajoute Content-Security-Policy + en-têtes durcis."""
+    """Middleware FastAPI : ajoute Content-Security-Policy + en-têtes durcis.
+
+    Sur HuggingFace Space, ``X-Frame-Options: DENY`` est sciemment omis :
+    ce header (priorité absolue dans les anciens navigateurs, fallback
+    moderne quand le navigateur ne supporte pas ``frame-ancestors``)
+    bloque l'iframe parente du Hub HF même si la CSP est permissive.
+    Le contrôle d'embed est alors entièrement délégué à
+    ``frame-ancestors``.
+    """
     response = await call_next(request)
     response.headers.setdefault("Content-Security-Policy", get_csp_policy())
     response.headers.setdefault("X-Content-Type-Options", "nosniff")
-    response.headers.setdefault("X-Frame-Options", "DENY")
+    if not is_huggingface_space():
+        response.headers.setdefault("X-Frame-Options", "DENY")
     response.headers.setdefault("Referrer-Policy", "strict-origin-when-cross-origin")
     return response

tests/web/test_sprint24_security.py

@@ -212,7 +212,9 @@ class TestCSPHeaders:
         from picarones.web.app import app
         return TestClient(app)
 
-    def test_csp_header_present(self, client):
+    def test_csp_header_present(self, client, monkeypatch):
+        # Mode local strict — pas de SPACE_ID dans l'env.
+        monkeypatch.delenv("SPACE_ID", raising=False)
         r = client.get("/api/status")
         assert r.status_code == 200
         assert "Content-Security-Policy" in r.headers
@@ -220,12 +222,50 @@ class TestCSPHeaders:
         assert "default-src 'self'" in csp
         assert "frame-ancestors 'none'" in csp
 
-    def test_security_headers_present(self, client):
+    def test_security_headers_present(self, client, monkeypatch):
+        monkeypatch.delenv("SPACE_ID", raising=False)
         r = client.get("/api/status")
         assert r.headers.get("X-Content-Type-Options") == "nosniff"
         assert r.headers.get("X-Frame-Options") == "DENY"
         assert r.headers.get("Referrer-Policy") == "strict-origin-when-cross-origin"
 
+    def test_csp_allows_huggingface_iframe_when_on_space(self, client, monkeypatch):
+        """Sur HF Space (``SPACE_ID`` défini), la CSP doit autoriser l'embed.
+
+        Garde-fou contre la régression historique « page blanche sur HF » :
+        ``frame-ancestors 'none'`` masquait la SPA dans l'iframe parente du
+        Hub HuggingFace.
+        """
+        monkeypatch.setenv("SPACE_ID", "Ma-Ri-Ba-Ku/Picarones")
+        r = client.get("/api/status")
+        csp = r.headers["Content-Security-Policy"]
+        assert "frame-ancestors" in csp
+        assert "'none'" not in csp.split("frame-ancestors")[1].split(";")[0]
+        assert "huggingface.co" in csp
+        assert "*.hf.space" in csp
+
+    def test_x_frame_options_omitted_on_huggingface_space(self, client, monkeypatch):
+        """Sur HF Space, ``X-Frame-Options: DENY`` doit être absent.
+
+        Ce header a priorité absolue sur ``frame-ancestors`` dans les anciens
+        navigateurs (et reste un fallback moderne) ; le laisser à ``DENY``
+        bloque l'iframe parente même avec la CSP permissive.
+        """
+        monkeypatch.setenv("SPACE_ID", "Ma-Ri-Ba-Ku/Picarones")
+        r = client.get("/api/status")
+        assert "X-Frame-Options" not in r.headers
+        # Les autres headers de durcissement restent intacts.
+        assert r.headers.get("X-Content-Type-Options") == "nosniff"
+        assert r.headers.get("Referrer-Policy") == "strict-origin-when-cross-origin"
+
+    def test_csp_override_via_env_takes_precedence(self, client, monkeypatch):
+        """``PICARONES_CSP`` reste un override absolu pour l'admin."""
+        monkeypatch.setenv("PICARONES_CSP", "default-src 'self'; frame-ancestors 'self'")
+        monkeypatch.setenv("SPACE_ID", "Ma-Ri-Ba-Ku/Picarones")  # ignoré
+        r = client.get("/api/status")
+        csp = r.headers["Content-Security-Policy"]
+        assert csp == "default-src 'self'; frame-ancestors 'self'"
+
 
 # ---------------------------------------------------------------------------
 # 8. Public mode bloque les benchmarks LLM (intégration FastAPI)