IP real detrás del proxy y caché de papers atómica y acotada

ARCHITECTURE_REVIEW §2.6 y §2.5.

PROXY (§2.6). El limitador usaba `request.client.host`, que detrás de un proxy
—HF Spaces tiene uno— es la dirección del PROXY. `limite_login` (5/minute) y
`limite_papers` no eran por IP sino GLOBALES: a la vez un bypass (fuerza bruta
desde muchas IPs contra un único contador) y una auto-denegación de servicio (un
cliente ruidoso agota el login de todos). El throttle por email+IP de
`auth.py` tenía el mismo problema.

`X-Forwarded-For` NO se lee a ciegas: la manda el cliente, y creérsela permite
falsificar la IP en cada petición. El operador declara cuántos proxies hay
delante (`MORPHOS_PROXY_SALTOS_CONFIABLES`, 0 por defecto) y se toma el elemento
-N, que es el último escrito por infraestructura de confianza; lo que hay a la
izquierda lo controla el cliente y se descarta. Aviso al arrancar si en prod
sigue a 0, porque el fallo es silencioso: los límites "funcionan", sólo que
compartidos.

CACHÉ DE PAPERS (§2.5). Tres fallos:
- `write_text` no es atómico. Dos fallos de caché simultáneos sobre la misma
consulta se entrelazaban y un lector hacía `json.loads` de un fichero truncado
→ 500, sin `try` alrededor. Ahora fichero temporal en el mismo directorio y
`os.replace`; el lector ve el contenido viejo o el nuevo, nunca uno a medias.
- El TTL sólo se miraba al LEER, así que una consulta que no se repite dejaba su
fichero para siempre. Se añade poda con tope de entradas.
- El directorio en `gettempdir()` se reutilizaba con `exist_ok=True` aunque lo
hubiera creado otro usuario del host: `mkdir(mode=0o700)` sólo protege si lo
creamos nosotros. Ahora lleva el uid en el nombre.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

backend/.env.example

@@ -85,6 +85,15 @@ MORPHOS_RAG_MAX_POR_LIBRO=2
 # analizador y suele ser correlativo): el cierre real es filtrar por tenant.
 MORPHOS_LAB_PENDIENTES_HABILITADO=false
 
+# --- Proxy inverso ---
+# Saltos de proxy DE CONFIANZA delante de la app. 0 = no confiar en X-Forwarded-For.
+# Detrás de un proxy, `request.client.host` es el PROXY, así que los límites por IP pasan a ser
+# GLOBALES: fuerza bruta desde muchas IPs no se limita, y un cliente ruidoso agota el login de
+# todos. La cabecera no se lee a ciegas porque la manda el cliente: se toma el elemento -N, el
+# último que escribió infraestructura de confianza.
+# En HF Spaces (hay un router delante): 1.
+MORPHOS_PROXY_SALTOS_CONFIABLES=0
+
 # --- Rate limiting ---
 MORPHOS_LIMITE_INTERPRET=10/minute
 MORPHOS_LIMITE_LOGIN=5/minute

backend/app/config.py

@@ -231,6 +231,24 @@ class Configuracion(BaseSettings):
     max_imagenes: int = Field(default=4)
     max_bytes_imagen: int = Field(default=6 * 1024 * 1024)
 
+    # --- Proxy inverso ---
+    # Saltos de proxy DE CONFIANZA delante de la app. 0 = no confiar en `X-Forwarded-For`.
+    #
+    # Por qué existe: el limitador usaba `request.client.host`, que detrás de un proxy (HF
+    # Spaces, cualquier CDN) es la dirección del PROXY, no la del cliente. Con eso,
+    # `limite_login` (5/minute) y `limite_papers` dejaban de ser por IP y pasaban a ser
+    # GLOBALES: a la vez un bypass (fuerza bruta desde muchas IPs no se limitaba por IP) y una
+    # auto-denegación de servicio (un cliente ruidoso agotaba el login de todos).
+    #
+    # Se declara el número de saltos en vez de leer la cabecera a ciegas porque `X-Forwarded-For`
+    # la pone el cliente: confiar en ella sin más permite falsificar la IP y saltarse cualquier
+    # límite poniendo una distinta en cada petición. Cada proxy AÑADE la dirección de su par, así
+    # que con N saltos de confianza el cliente real es el elemento -N de la lista; todo lo que
+    # haya a la izquierda lo escribió alguien no confiable y se descarta.
+    #
+    # En HF Spaces detrás de su router: 1.
+    proxy_saltos_confiables: int = Field(default=0)
+
     # --- Rate limiting ---
     limite_interpret: str = Field(default="10/minute")
     # Techo por USUARIO además del de IP. La cuota de ZeroGPU es por cuenta y compartida entre
@@ -327,6 +345,15 @@ class Configuracion(BaseSettings):
                 "registrarse. Si la base de usuarios está vacía, nadie podrá entrar."
             )
 
+        if self.entorno == "prod" and self.proxy_saltos_confiables <= 0:
+            # Silencioso y caro: los límites siguen "funcionando", sólo que compartidos por todo
+            # el mundo, así que no se nota hasta que alguien agota el login de los demás.
+            log.warning(
+                "MORPHOS_PROXY_SALTOS_CONFIABLES=0 en producción: si hay un proxy delante "
+                "(HF Spaces lo tiene), los límites por IP son en realidad GLOBALES. Declara "
+                "cuántos saltos de confianza hay."
+            )
+
     def validar_prod(self) -> None:
         """Requisitos que sólo aplican en producción; falla cerrado si faltan."""
         if self.entorno != "prod":

backend/app/routers/auth.py

@@ -24,7 +24,7 @@ from ..db import (
     verificar_password,
 )
 from ..security.authz import usuario_actual
-from ..security.rate_limit import limiter
+from ..security.rate_limit import ip_cliente, limiter
 from ..security.session import (
     COOKIE_CSRF,
     COOKIE_SESION,
@@ -81,7 +81,9 @@ async def login(request: Request, body: LoginBody, response: Response) -> dict:
     # endpoint `async`, así que en el bucle de eventos bloqueaban el proceso entero. scrypt es
     # además caro A PROPÓSITO (n=2**14, decenas de ms): es justo el trabajo que no puede vivir
     # en el bucle, y el login es el endpoint que más veces lo ejecuta.
-    ip = request.client.host if request.client else "?"
+    # `ip_cliente` y no `request.client.host`: detrás del proxy este último es el proxy, así
+    # que el throttle por email+IP degeneraba en un contador global.
+    ip = ip_cliente(request)
     if await asyncio.to_thread(intentos_recientes, body.email, ip, _VENTANA_THROTTLE_S) >= _MAX_INTENTOS:
         raise HTTPException(status.HTTP_429_TOO_MANY_REQUESTS, "Demasiados intentos. Espera unos minutos.")
 

backend/app/routers/papers.py

@@ -8,6 +8,7 @@ from __future__ import annotations
 
 import hashlib
 import json
+import os
 import re
 import tempfile
 import time
@@ -21,22 +22,83 @@ from ..security.rate_limit import limiter
 
 router = APIRouter()
 
-_DIR_CACHE = Path(tempfile.gettempdir()) / "morphos_papers_cache"
+# El uid en el nombre evita el caso en que el directorio ya existe creado por OTRO usuario del
+# host compartido: `mkdir(mode=0o700)` sólo protege si lo creamos nosotros, y con `exist_ok=True`
+# un directorio ajeno (o un enlace simbólico plantado ahí) se habría usado tal cual.
+_DIR_CACHE = Path(tempfile.gettempdir()) / f"morphos_papers_cache_{os.getuid()}"
 _TTL_S = 1800
+_MAX_ENTRADAS = 500
 _CABECERAS = {"User-Agent": "Morphos/1.0 (mailto:ceo@equipamed.net)", "Accept": "application/json"}
 
 
+def _ruta(clave: str) -> Path:
+    return _DIR_CACHE / f"{hashlib.sha256(clave.encode()).hexdigest()}.json"
+
+
 def _leer_cache(clave: str) -> dict | None:
-    archivo = _DIR_CACHE / f"{hashlib.md5(clave.encode()).hexdigest()}.json"
-    if archivo.exists() and (time.time() - archivo.stat().st_mtime) < _TTL_S:
+    """Entrada viva de la caché, o None. Un fichero ilegible es un fallo de caché, no un 500.
+
+    Antes, `json.loads` sobre un fichero a medio escribir tumbaba la petición con 500. Con la
+    escritura atómica de abajo eso ya no debería ocurrir, pero la caché vive en un directorio
+    temporal que puede truncarse por otras razones (disco lleno, limpieza del host), y un fallo
+    de caché siempre es recuperable: se vuelve a pedir a NCBI.
+    """
+    archivo = _ruta(clave)
+    try:
+        if (time.time() - archivo.stat().st_mtime) >= _TTL_S:
+            return None
         return json.loads(archivo.read_text(encoding="utf-8"))
-    return None
+    except FileNotFoundError:
+        return None
+    except (OSError, json.JSONDecodeError, UnicodeDecodeError):
+        archivo.unlink(missing_ok=True)  # corrupta: que no se relea eternamente
+        return None
+
+
+def _podar_cache() -> None:
+    """Borra lo caducado y, si aún sobran entradas, lo más antiguo.
+
+    Sin esto el directorio sólo crecía: el TTL se comprobaba al LEER, así que una consulta que
+    no se repite nunca dejaba su fichero para siempre.
+    """
+    try:
+        entradas = list(_DIR_CACHE.glob("*.json"))
+    except OSError:
+        return
+    ahora = time.time()
+    vivas = []
+    for f in entradas:
+        try:
+            if ahora - f.stat().st_mtime >= _TTL_S:
+                f.unlink(missing_ok=True)
+            else:
+                vivas.append(f)
+        except OSError:
+            continue
+    if len(vivas) > _MAX_ENTRADAS:
+        vivas.sort(key=lambda f: f.stat().st_mtime)
+        for f in vivas[: len(vivas) - _MAX_ENTRADAS]:
+            f.unlink(missing_ok=True)
 
 
 def _escribir_cache(clave: str, datos: dict) -> None:
+    """Escritura ATÓMICA: fichero temporal en el mismo directorio y `os.replace`.
+
+    `write_text` no es atómico. Dos fallos de caché simultáneos sobre la misma consulta se
+    entrelazaban y un lector veía un JSON truncado. `os.replace` dentro del mismo sistema de
+    ficheros es atómico: el lector ve el contenido viejo o el nuevo, nunca uno a medias.
+    """
     _DIR_CACHE.mkdir(mode=0o700, parents=True, exist_ok=True)
-    archivo = _DIR_CACHE / f"{hashlib.md5(clave.encode()).hexdigest()}.json"
-    archivo.write_text(json.dumps(datos, ensure_ascii=False), encoding="utf-8")
+    destino = _ruta(clave)
+    fd, temporal = tempfile.mkstemp(dir=_DIR_CACHE, suffix=".tmp")
+    try:
+        with os.fdopen(fd, "w", encoding="utf-8") as fh:
+            json.dump(datos, fh, ensure_ascii=False)
+        os.replace(temporal, destino)
+    except OSError:
+        Path(temporal).unlink(missing_ok=True)  # la caché es opcional: nunca romper la petición
+        return
+    _podar_cache()
 
 
 # Sin guarda de sesión a propósito: la búsqueda en PubMed no es sensible ni consume la

backend/app/security/rate_limit.py

@@ -11,9 +11,35 @@ from fastapi import Request
 from slowapi import Limiter
 from slowapi.util import get_remote_address
 
+from ..config import obtener_config
 from .session import COOKIE_SESION, leer_sesion
 
-limiter = Limiter(key_func=get_remote_address)
+
+def ip_cliente(request: Request) -> str:
+    """IP real del cliente, teniendo en cuenta los proxies de confianza declarados.
+
+    `get_remote_address` devuelve `request.client.host`, que detrás de un proxy es el proxy: en
+    HF Spaces eso convertía `limite_login` y `limite_papers` en límites GLOBALES compartidos por
+    todo el mundo (bypass de la fuerza bruta y auto-DoS a la vez).
+
+    `X-Forwarded-For` NO se lee a ciegas —la manda el cliente, y creérsela permite falsificar la
+    IP en cada petición y saltarse cualquier límite—. Se usa sólo si el operador declaró cuántos
+    proxies hay delante (`proxy_saltos_confiables`), y se toma el elemento -N: cada proxy añade
+    la dirección de su par, así que ése es el último valor escrito por infraestructura de
+    confianza. Lo de más a la izquierda es lo que dijo el cliente y se descarta.
+    """
+    saltos = obtener_config().proxy_saltos_confiables
+    if saltos > 0:
+        reenviada = request.headers.get("x-forwarded-for", "")
+        partes = [p.strip() for p in reenviada.split(",") if p.strip()]
+        if len(partes) >= saltos:
+            return partes[-saltos]
+        # Menos entradas de las declaradas: la petición no vino por la cadena esperada. Se cae
+        # al peer directo en vez de coger un valor que el cliente controle.
+    return get_remote_address(request)
+
+
+limiter = Limiter(key_func=ip_cliente)
 
 
 def clave_usuario(request: Request) -> str:
@@ -30,4 +56,4 @@ def clave_usuario(request: Request) -> str:
     sesion = leer_sesion(request.cookies.get(COOKIE_SESION))
     if sesion and sesion.get("email"):
         return f"user:{sesion['email']}"
-    return f"ip:{get_remote_address(request)}"
+    return f"ip:{ip_cliente(request)}"

backend/tests/test_cache_papers.py

@@ -0,0 +1,101 @@
+"""Caché en disco de /api/papers: atómica, tolerante a basura y acotada.
+
+Tres fallos que tenía: `write_text` no es atómico (dos fallos de caché simultáneos sobre la
+misma consulta se entrelazaban y un lector veía JSON truncado → 500 sin `try`), el TTL sólo se
+miraba al LEER (una consulta que no se repite dejaba su fichero para siempre) y el directorio
+en `gettempdir()` se reutilizaba aunque lo hubiera creado otro usuario del host.
+"""
+
+from __future__ import annotations
+
+import json
+import os
+import time
+
+from app.routers import papers
+
+
+def _limpiar():
+    if papers._DIR_CACHE.exists():
+        for f in papers._DIR_CACHE.glob("*"):
+            f.unlink(missing_ok=True)
+
+
+def test_ida_y_vuelta():
+    _limpiar()
+    papers._escribir_cache("pm:anemia", {"resultados": [1, 2, 3]})
+    assert papers._leer_cache("pm:anemia") == {"resultados": [1, 2, 3]}
+
+
+def test_fichero_truncado_es_fallo_de_cache_no_un_500():
+    """El caso real: `json.loads` sobre un fichero a medio escribir tumbaba la petición."""
+    _limpiar()
+    papers._escribir_cache("pm:x", {"a": 1})
+    papers._ruta("pm:x").write_text('{"a": ', encoding="utf-8")  # JSON cortado
+
+    assert papers._leer_cache("pm:x") is None
+    # Y se borra: si no, se releería basura hasta que caduque.
+    assert not papers._ruta("pm:x").exists()
+
+
+def test_entrada_caducada_no_se_sirve():
+    _limpiar()
+    papers._escribir_cache("pm:viejo", {"a": 1})
+    viejo = time.time() - papers._TTL_S - 10
+    os.utime(papers._ruta("pm:viejo"), (viejo, viejo))
+    assert papers._leer_cache("pm:viejo") is None
+
+
+def test_la_poda_borra_lo_caducado():
+    """El TTL se comprobaba sólo al leer, así que el directorio crecía sin tope."""
+    _limpiar()
+    for i in range(5):
+        papers._escribir_cache(f"pm:{i}", {"i": i})
+    viejo = time.time() - papers._TTL_S - 10
+    for i in range(3):
+        os.utime(papers._ruta(f"pm:{i}"), (viejo, viejo))
+
+    papers._podar_cache()
+
+    vivas = list(papers._DIR_CACHE.glob("*.json"))
+    assert len(vivas) == 2
+
+
+def test_la_escritura_no_deja_temporales():
+    """`os.replace` mueve el temporal; si quedara, el directorio crecería igual."""
+    _limpiar()
+    papers._escribir_cache("pm:limpio", {"a": 1})
+    assert list(papers._DIR_CACHE.glob("*.tmp")) == []
+    assert len(list(papers._DIR_CACHE.glob("*.json"))) == 1
+
+
+def test_un_lector_nunca_ve_un_json_a_medias(monkeypatch):
+    """Con escritura atómica, mientras se escribe la nueva entrada se sigue leyendo la vieja.
+
+    Se simula la ventana de entrelazado: durante `json.dump` del segundo escritor, un lector
+    consulta la misma clave. Sin `os.replace` vería el fichero destino truncado.
+    """
+    _limpiar()
+    papers._escribir_cache("pm:carrera", {"version": "vieja"})
+    leido_durante_la_escritura = {}
+
+    volcado_real = json.dump
+
+    def _volcado_que_lee_a_la_vez(datos, fh, **kw):
+        volcado_real(datos, fh, **kw)
+        fh.flush()
+        leido_durante_la_escritura["valor"] = papers._leer_cache("pm:carrera")
+
+    monkeypatch.setattr(papers.json, "dump", _volcado_que_lee_a_la_vez)
+    papers._escribir_cache("pm:carrera", {"version": "nueva"})
+
+    assert leido_durante_la_escritura["valor"] == {"version": "vieja"}
+    assert papers._leer_cache("pm:carrera") == {"version": "nueva"}
+
+
+def test_el_directorio_es_privado_y_propio():
+    """0700 y con el uid en el nombre: `exist_ok=True` sobre un directorio ajeno lo habría usado."""
+    _limpiar()
+    papers._escribir_cache("pm:permisos", {"a": 1})
+    assert str(os.getuid()) in papers._DIR_CACHE.name
+    assert (papers._DIR_CACHE.stat().st_mode & 0o777) == 0o700

backend/tests/test_ip_cliente.py

@@ -0,0 +1,75 @@
+"""IP de cliente detrás de un proxy inverso.
+
+`request.client.host` detrás de un proxy es el PROXY. Con eso, `limite_login` (5/minute) y
+`limite_papers` dejaban de ser por IP y pasaban a ser globales: bypass de la fuerza bruta (muchas
+IPs, un solo contador) y auto-DoS (un cliente ruidoso agota el login de todos) a la vez.
+
+La cabecera NO puede creerse a ciegas: la manda el cliente. Estas pruebas fijan las dos mitades
+—se usa cuando el operador declara los saltos, se ignora cuando no— porque equivocarse en
+cualquiera de ellas es un agujero.
+"""
+
+from __future__ import annotations
+
+import pytest
+
+from app.config import obtener_config
+from app.security.rate_limit import ip_cliente
+
+
+class _PeticionFalsa:
+    """Lo mínimo que `ip_cliente` mira: cabeceras y peer directo."""
+
+    def __init__(self, xff: str | None, peer: str = "10.0.0.1"):
+        self.headers = {"x-forwarded-for": xff} if xff is not None else {}
+        self.client = type("C", (), {"host": peer})()
+
+
+@pytest.fixture
+def saltos(monkeypatch):
+    def _fijar(n: int):
+        monkeypatch.setattr(obtener_config(), "proxy_saltos_confiables", n)
+
+    return _fijar
+
+
+def test_sin_proxy_declarado_se_ignora_la_cabecera(saltos):
+    """El defecto: `X-Forwarded-For` la pone cualquiera, así que sin declarar proxy no vale nada."""
+    saltos(0)
+    assert ip_cliente(_PeticionFalsa("1.2.3.4", peer="10.0.0.1")) == "10.0.0.1"
+
+
+def test_un_salto_toma_la_ultima_entrada(saltos):
+    """Con un proxy delante, la entrada que él añadió es la del cliente real."""
+    saltos(1)
+    assert ip_cliente(_PeticionFalsa("203.0.113.9", peer="10.0.0.1")) == "203.0.113.9"
+
+
+def test_no_se_deja_falsificar_anteponiendo_entradas(saltos):
+    """El atacante controla lo que va a la IZQUIERDA; sólo cuenta lo que escribió el proxy.
+
+    Es el fallo clásico de coger `xff[0]`: bastaría mandar `X-Forwarded-For: <lo-que-sea>` para
+    estrenar contador de rate limiting en cada petición.
+    """
+    saltos(1)
+    falsa = _PeticionFalsa("6.6.6.6, 203.0.113.9", peer="10.0.0.1")
+    assert ip_cliente(falsa) == "203.0.113.9"
+
+
+def test_dos_saltos_saltan_el_cdn(saltos):
+    """Cliente → CDN → proxy → app: el cliente es el penúltimo."""
+    saltos(2)
+    peticion = _PeticionFalsa("203.0.113.9, 198.51.100.7", peer="10.0.0.1")
+    assert ip_cliente(peticion) == "203.0.113.9"
+
+
+def test_cadena_mas_corta_de_lo_declarado_cae_al_peer(saltos):
+    """La petición no vino por la cadena esperada: mejor el peer directo que un valor del cliente."""
+    saltos(2)
+    assert ip_cliente(_PeticionFalsa("203.0.113.9", peer="10.0.0.1")) == "10.0.0.1"
+    assert ip_cliente(_PeticionFalsa(None, peer="10.0.0.1")) == "10.0.0.1"
+
+
+def test_tolera_espacios_y_entradas_vacias(saltos):
+    saltos(1)
+    assert ip_cliente(_PeticionFalsa(" 6.6.6.6 ,  , 203.0.113.9 ")) == "203.0.113.9"