Phase 2.1.1 : durcissement avant exposition publique

Fail-fast sur la configuration. Une limite de debit illisible (10/hours,
10/banana, -1/hour) arrete desormais le demarrage avec un message nommant la
variable, au lieu de desactiver silencieusement la protection : seules les
valeurs 0, off, none, disabled et vide desactivent volontairement. Meme
traitement pour MAX_SHARED_MAPS, MAX_SHARE_SIZE_MB, DEFAULT_SHARE_EXPIRY_DAYS
et chaque entree de TRUSTED_PROXY_CIDRS.

X-Forwarded-For n'est plus cru sur la seule foi de TRUST_PROXY_HEADERS : la
connexion doit aussi provenir d'un reseau declare dans TRUSTED_PROXY_CIDRS
(ipaddress, IPv4/IPv6/CIDR). Activer la confiance sans declarer de reseau est
refuse au demarrage, faute de quoi la configuration semblerait active sans
l'etre. L'adresse retenue est normalisee : les differentes ecritures d'une meme
IPv6 ne donnent plus des compteurs distincts.

MAX_SHARED_MAPS : comptage et insertion serialises par un verrou de processus,
pour que deux creations simultanees ne franchissent pas ensemble la derniere
place. Documente comme valable pour un service applicatif unique.

Ajoute APP_BIND pour ne pas publier 8080 sur toutes les interfaces en
production, une option backup --keep pour la retention, et documente Nginx,
Traefik en override, cron et systemd.

Corrige un bug de rendu du build local : mon edition avait reecrit
docker-entrypoint.sh en CRLF, rendant le shebang introuvable dans le conteneur.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-09-10 09:53:29 +02:00
parent 49870dcd56
commit 9ede2c5e68
17 changed files with 915 additions and 79 deletions

View File

@@ -192,3 +192,39 @@ def test_restore_puts_back_a_backup(client: TestClient, tmp_path: Path, capsys)
with SessionLocal() as session:
titles = session.execute(select(SharedMap.title)).scalars().all()
assert titles == ["Avant sauvegarde"]
def test_backup_retention_keeps_the_most_recent(client: TestClient, tmp_path: Path, capsys) -> None:
"""--keep N ne conserve que les N sauvegardes les plus recentes du dossier."""
import os
import time
_create(client, "Infrastructure")
folder = tmp_path / "backups"
for index in range(5):
destination = folder / f"openmind-{index}.db"
assert cli.main(["backup", str(destination), "--yes"]) == 0
# Horodatage distinct : le tri de retention doit etre deterministe.
os.utime(destination, (time.time() + index, time.time() + index))
capsys.readouterr()
latest = folder / "openmind-latest.db"
assert cli.main(["backup", str(latest), "--keep", "3"]) == 0
out = capsys.readouterr().out
assert "Ancienne sauvegarde supprimee" in out
remaining = sorted(path.name for path in folder.glob("*.db"))
assert len(remaining) == 3
# La sauvegarde qui vient d'etre ecrite est toujours conservee, et compte
# dans les trois : on ne finit jamais avec une de plus que demande.
assert "openmind-latest.db" in remaining
def test_backup_without_keep_deletes_nothing(client: TestClient, tmp_path: Path) -> None:
_create(client)
folder = tmp_path / "backups"
for index in range(3):
assert cli.main(["backup", str(folder / f"openmind-{index}.db")]) == 0
assert cli.main(["backup", str(folder / "openmind-3.db")]) == 0
assert len(list(folder.glob("*.db"))) == 4

View File

@@ -1,11 +1,20 @@
"""Identification du client : X-Forwarded-For n'est cru que sur demande."""
"""Identification du client : X-Forwarded-For n'est cru qu'a deux conditions.
`TRUST_PROXY_HEADERS` doit etre actif, ET la connexion doit venir d'un reseau
declare dans `TRUSTED_PROXY_CIDRS`. Ces tests reproduisent les deux chemins :
derriere un proxy approuve, et en connexion directe forgeant le meme en-tete.
"""
from __future__ import annotations
import pytest
from starlette.datastructures import Headers
from starlette.requests import Request
from app.services.client_ip import resolve_client_ip
from app.config import parse_cidrs
from app.services.client_ip import is_trusted_peer, normalize_ip, resolve_client_ip
PROXY_NETWORKS = parse_cidrs("172.16.0.0/12,10.0.0.0/8,2001:db8::/32")
def _request(headers: dict[str, str], peer: str | None = "203.0.113.9") -> Request:
@@ -19,34 +28,108 @@ def _request(headers: dict[str, str], peer: str | None = "203.0.113.9") -> Reque
return Request(scope)
def test_proxy_headers_are_ignored_by_default() -> None:
request = _request({"x-forwarded-for": "1.2.3.4"})
assert resolve_client_ip(request, trust_proxy_headers=False) == "203.0.113.9"
class TestWithoutTrust:
def test_proxy_headers_are_ignored_by_default(self) -> None:
request = _request({"x-forwarded-for": "1.2.3.4"})
assert resolve_client_ip(request, trust_proxy_headers=False) == "203.0.113.9"
def test_forged_header_cannot_change_identity(self) -> None:
"""Un client direct ne peut pas se demultiplier pour diluer sa limite."""
seen = {
resolve_client_ip(
_request({"x-forwarded-for": f"10.0.0.{i}"}), trust_proxy_headers=False
)
for i in range(20)
}
assert seen == {"203.0.113.9"}
def test_falls_back_to_peer_without_headers(self) -> None:
assert resolve_client_ip(_request({}), trust_proxy_headers=True) == "203.0.113.9"
def test_unknown_client_is_handled(self) -> None:
assert resolve_client_ip(_request({}, peer=None), trust_proxy_headers=False) == "unknown"
def test_forged_header_cannot_change_identity() -> None:
"""Sans confiance explicite, un client direct ne peut pas se demultiplier."""
seen = {
resolve_client_ip(_request({"x-forwarded-for": f"10.0.0.{i}"}), trust_proxy_headers=False)
for i in range(20)
}
assert seen == {"203.0.113.9"}
class TestTrustedProxy:
def test_approved_proxy_is_believed(self) -> None:
# Le proxy (172.18.0.1) a ajoute l'adresse reellement vue.
request = _request({"x-forwarded-for": "9.9.9.9, 198.51.100.7"}, peer="172.18.0.1")
resolved = resolve_client_ip(
request, trust_proxy_headers=True, trusted_networks=PROXY_NETWORKS
)
assert resolved == "198.51.100.7"
def test_direct_client_forging_the_same_header_is_not_believed(self) -> None:
"""Meme en-tete, connexion directe : c'est l'adresse socket qui compte."""
request = _request({"x-forwarded-for": "9.9.9.9, 198.51.100.7"}, peer="203.0.113.9")
resolved = resolve_client_ip(
request, trust_proxy_headers=True, trusted_networks=PROXY_NETWORKS
)
assert resolved == "203.0.113.9"
def test_approved_proxy_falls_back_to_real_ip(self) -> None:
request = _request({"x-real-ip": "198.51.100.8"}, peer="10.1.2.3")
resolved = resolve_client_ip(
request, trust_proxy_headers=True, trusted_networks=PROXY_NETWORKS
)
assert resolved == "198.51.100.8"
def test_no_declared_network_means_no_trust(self) -> None:
request = _request({"x-forwarded-for": "198.51.100.7"}, peer="172.18.0.1")
resolved = resolve_client_ip(request, trust_proxy_headers=True, trusted_networks=[])
assert resolved == "172.18.0.1"
def test_ipv6_proxy_is_recognised(self) -> None:
request = _request({"x-forwarded-for": "198.51.100.7"}, peer="2001:db8::5")
resolved = resolve_client_ip(
request, trust_proxy_headers=True, trusted_networks=PROXY_NETWORKS
)
assert resolved == "198.51.100.7"
def test_peer_membership(self) -> None:
assert is_trusted_peer("172.18.0.1", PROXY_NETWORKS) is True
assert is_trusted_peer("10.255.255.254", PROXY_NETWORKS) is True
assert is_trusted_peer("203.0.113.9", PROXY_NETWORKS) is False
assert is_trusted_peer("2001:db8::1", PROXY_NETWORKS) is True
assert is_trusted_peer("2001:dead::1", PROXY_NETWORKS) is False
assert is_trusted_peer("pas-une-ip", PROXY_NETWORKS) is False
assert is_trusted_peer("10.0.0.1", []) is False
def test_trusted_proxy_uses_the_last_forwarded_value() -> None:
# Le client a forge "9.9.9.9" ; le proxy a ajoute l'adresse reellement vue.
request = _request({"x-forwarded-for": "9.9.9.9, 198.51.100.7"})
assert resolve_client_ip(request, trust_proxy_headers=True) == "198.51.100.7"
class TestNormalisation:
@pytest.mark.parametrize(
("raw", "expected"),
[
("2001:db8::1", "2001:db8::1"),
("2001:0db8:0000:0000:0000:0000:0000:0001", "2001:db8::1"),
("2001:DB8::1", "2001:db8::1"),
("2001:0db8::0:1", "2001:db8::1"),
("[2001:db8::1]:443", "2001:db8::1"),
(" 192.0.2.7 ", "192.0.2.7"),
("192.0.2.7:8080", "192.0.2.7"),
],
)
def test_same_address_gives_the_same_key(self, raw: str, expected: str) -> None:
assert normalize_ip(raw) == expected
def test_ipv6_rewriting_cannot_reset_the_counter(self) -> None:
"""Plusieurs ecritures de la meme IPv6 doivent produire une seule cle."""
forms = [
"2001:db8::1",
"2001:0DB8:0000:0000:0000:0000:0000:0001",
"2001:db8:0:0:0:0:0:1",
"[2001:db8::1]:51000",
]
keys = {
resolve_client_ip(
_request({"x-forwarded-for": form}, peer="172.18.0.1"),
trust_proxy_headers=True,
trusted_networks=PROXY_NETWORKS,
)
for form in forms
}
assert keys == {"2001:db8::1"}
def test_trusted_proxy_falls_back_to_real_ip() -> None:
request = _request({"x-real-ip": "198.51.100.8"})
assert resolve_client_ip(request, trust_proxy_headers=True) == "198.51.100.8"
def test_falls_back_to_peer_without_headers() -> None:
assert resolve_client_ip(_request({}), trust_proxy_headers=True) == "203.0.113.9"
def test_unknown_client_is_handled() -> None:
assert resolve_client_ip(_request({}, peer=None), trust_proxy_headers=False) == "unknown"
def test_unreadable_value_is_kept_bounded(self) -> None:
assert normalize_ip("x" * 500) == "x" * 64
assert normalize_ip(" ") == "unknown"

View File

@@ -0,0 +1,153 @@
"""Validation de la configuration au demarrage.
Une valeur illisible doit arreter l'application avec un message nommant la
variable fautive, jamais desactiver silencieusement une protection.
"""
from __future__ import annotations
import pytest
from pydantic import ValidationError
from app.config import ConfigurationError, Settings, get_settings, parse_cidrs
from app.services.rate_limit import RateLimitSyntaxError, parse_rate
@pytest.mark.parametrize(
("spec", "limit", "window"),
[
("10/hour", 10, 3600),
("120/minute", 120, 60),
("5/second", 5, 1),
("2/day", 2, 86400),
("10/h", 10, 3600),
(" 30/MINUTE ", 30, 60),
("3", 3, 60),
],
)
def test_valid_rate_specs(spec: str, limit: int, window: int) -> None:
rule = parse_rate(spec)
assert rule is not None
assert rule.limit == limit
assert rule.window_seconds == window
@pytest.mark.parametrize("spec", ["", "0", "off", "none", "disabled", " OFF "])
def test_explicit_deactivation(spec: str) -> None:
"""Desactiver une limite reste possible, mais doit etre explicite."""
assert parse_rate(spec) is None
@pytest.mark.parametrize(
"spec",
["10/hours", "abc", "-1/hour", "10/banana", "10/", "/hour", "dix/hour", "10 par heure"],
)
def test_invalid_rate_specs_are_refused(spec: str) -> None:
with pytest.raises(RateLimitSyntaxError):
parse_rate(spec)
def test_invalid_rate_message_is_actionable() -> None:
with pytest.raises(RateLimitSyntaxError) as error:
parse_rate("10/hours")
message = str(error.value)
assert "10/hours" in message
assert "hours" in message
assert "second" in message and "hour" in message
class TestSettingsValidation:
def test_valid_configuration_loads(self) -> None:
settings = Settings(
share_create_rate_limit="10/hour",
share_read_rate_limit="120/minute",
share_write_rate_limit="off",
)
assert settings.share_write_rate_limit == "off"
@pytest.mark.parametrize(
"field",
["share_create_rate_limit", "share_read_rate_limit", "share_write_rate_limit"],
)
def test_each_rate_variable_is_validated(self, field: str) -> None:
with pytest.raises(Exception) as error:
Settings(**{field: "10/banana"})
assert "banana" in str(error.value)
def test_negative_shared_map_limit_is_refused(self) -> None:
with pytest.raises(ValidationError):
Settings(max_shared_maps=-1)
def test_negative_share_size_is_refused(self) -> None:
with pytest.raises(ValidationError):
Settings(max_share_size_mb=-2)
def test_zero_share_size_means_inherit(self) -> None:
settings = Settings(max_share_size_mb=0, max_map_size_mb=10)
assert settings.max_share_size_bytes == settings.max_map_size_bytes
def test_negative_expiry_is_refused(self) -> None:
with pytest.raises(ValidationError):
Settings(default_share_expiry_days=-1)
def test_zero_expiry_is_allowed(self) -> None:
assert Settings(default_share_expiry_days=0).default_share_expiry_days == 0
def test_zero_map_size_is_refused(self) -> None:
with pytest.raises(ValidationError):
Settings(max_map_size_mb=0)
class TestTrustedProxyConfiguration:
def test_valid_cidrs(self) -> None:
networks = parse_cidrs("172.16.0.0/12, 10.0.0.0/8 ,2001:db8::/32")
assert len(networks) == 3
assert str(networks[0]) == "172.16.0.0/12"
def test_single_address_becomes_a_host_network(self) -> None:
assert str(parse_cidrs("192.0.2.7")[0]) == "192.0.2.7/32"
def test_empty_value_is_allowed(self) -> None:
assert parse_cidrs("") == []
assert parse_cidrs(" , ") == []
@pytest.mark.parametrize("value", ["999.1.1.1/8", "10.0.0.0/64", "pas-un-reseau", "10.0.0.0/"])
def test_invalid_cidr_is_refused(self, value: str) -> None:
with pytest.raises(ValueError, match="reseau valide"):
parse_cidrs(value)
def test_invalid_cidr_stops_settings(self) -> None:
with pytest.raises(Exception) as error:
Settings(trusted_proxy_cidrs="10.0.0.0/8,pas-un-reseau")
assert "pas-un-reseau" in str(error.value)
def test_trusting_proxies_without_declaring_them_is_refused(self) -> None:
"""Faire confiance sans dire a qui n'appliquerait aucune confiance."""
with pytest.raises(Exception) as error:
Settings(trust_proxy_headers=True, trusted_proxy_cidrs="")
assert "TRUSTED_PROXY_CIDRS" in str(error.value)
def test_trusting_proxies_with_networks_is_accepted(self) -> None:
settings = Settings(trust_proxy_headers=True, trusted_proxy_cidrs="172.16.0.0/12")
assert len(settings.trusted_proxy_networks) == 1
class TestStartupFailure:
def test_bad_rate_stops_startup_with_a_readable_message(self, monkeypatch) -> None:
monkeypatch.setenv("SHARE_CREATE_RATE_LIMIT", "10/banana")
get_settings.cache_clear()
with pytest.raises(ConfigurationError) as error:
get_settings()
message = str(error.value)
assert "Configuration invalide" in message
assert "SHARE_CREATE_RATE_LIMIT" in message
assert "banana" in message
get_settings.cache_clear()
def test_bad_cidr_stops_startup(self, monkeypatch) -> None:
monkeypatch.setenv("TRUSTED_PROXY_CIDRS", "10.0.0.0/8,nawak")
get_settings.cache_clear()
with pytest.raises(ConfigurationError) as error:
get_settings()
assert "TRUSTED_PROXY_CIDRS" in str(error.value)
get_settings.cache_clear()

View File

@@ -113,6 +113,48 @@ def test_management_token_never_appears_in_logs(client: TestClient, caplog) -> N
assert "share=" in logged
def test_no_token_leaks_on_any_management_route(client: TestClient, caplog) -> None:
"""PATCH, DELETE et verify manipulent aussi les deux jetons."""
with caplog.at_level(logging.DEBUG):
created = client.post("/api/shares", json={"title": "x", "map": build_map()}).json()
token = created["share_token"]
management = created["management_token"]
headers = {"X-Management-Token": management}
client.patch(f"/api/shares/{token}", json={"expires_in_days": 7}, headers=headers)
client.post(f"/api/shares/{token}/verify", headers=headers)
client.post(f"/api/shares/{token}/verify", headers={"X-Management-Token": "faux"})
client.patch(
f"/api/shares/{token}", json={"permanent": True}, headers={"X-Management-Token": "faux"}
)
client.delete(f"/api/shares/{token}", headers=headers)
logged = "\n".join(
record.getMessage()
for record in caplog.records
if not record.name.startswith(("httpx", "httpcore"))
)
assert management not in logged
assert token not in logged
assert "share=" in logged
def test_access_log_redaction_covers_every_share_route() -> None:
"""Les chemins journalises par uvicorn ne doivent porter aucun jeton."""
token = "sbcY5FwVInYtTSGLNjBCyw"
for method, path in [
("GET", f"/api/shares/{token}"),
("PUT", f"/api/shares/{token}"),
("PATCH", f"/api/shares/{token}"),
("DELETE", f"/api/shares/{token}"),
("POST", f"/api/shares/{token}/verify"),
("GET", f"/s/{token}"),
]:
line = redact_tokens(f"{method} {path} HTTP/1.1")
assert token not in line, line
assert "[REDACTED]" in line
def test_application_logging_survives_migrations(client: TestClient, caplog) -> None:
"""Les migrations ne doivent pas desactiver les loggers de l'application.

View File

@@ -27,7 +27,8 @@ class TestRateRuleParsing:
assert parse_rate("3").window_seconds == 60
def test_disabled_values(self) -> None:
for spec in ("", "0", "off", "none", "-4/hour", "n'importe quoi"):
"""Seules ces valeurs desactivent : voir test_configuration.py pour le reste."""
for spec in ("", "0", "off", "none", "disabled"):
assert parse_rate(spec) is None
@@ -155,3 +156,37 @@ def test_share_specific_size_limit(client: TestClient, configure) -> None:
response = client.post("/api/shares", json={"title": "x", "map": build_map(node_count=50)})
assert response.status_code == 413
assert "limite de partage" in response.json()["detail"]
def test_capacity_is_not_crossed_by_simultaneous_creations(client: TestClient, configure) -> None:
"""La derniere place ne doit pas etre prise deux fois.
Le comptage et l'insertion sont serialises par un verrou de processus :
avec un seul service applicatif devant SQLite, cela suffit a ce que deux
requetes concurrentes ne franchissent pas ensemble la limite.
"""
import threading
configure(max_shared_maps=5)
for _ in range(4):
assert _post(client) == 201
results: list[int] = []
lock = threading.Lock()
def attempt() -> None:
status = client.post("/api/shares", json={"title": "x", "map": build_map()}).status_code
with lock:
results.append(status)
threads = [threading.Thread(target=attempt) for _ in range(8)]
for thread in threads:
thread.start()
for thread in threads:
thread.join()
assert results.count(201) == 1
assert results.count(503) == 7
listing = client.get("/api/config")
assert listing.status_code == 200

View File

@@ -0,0 +1,103 @@
"""Limitation de debit derriere un reverse proxy, de bout en bout.
Deux chemins sont reproduits sur l'application reelle :
* une connexion venant d'un proxy approuve, qui transmet l'adresse du client ;
* une connexion directe qui forge le meme en-tete.
Dans le premier cas la limite doit suivre l'adresse transmise, dans le second
l'adresse reelle de la socket.
"""
from __future__ import annotations
from collections.abc import Iterator
import pytest
from fastapi.testclient import TestClient
from app.main import app
from tests.conftest import build_map
PROXY = "172.18.0.1"
DIRECT = "203.0.113.9"
def _client(peer: str) -> Iterator[TestClient]:
with TestClient(app, client=(peer, 40000)) as test_client:
yield test_client
@pytest.fixture()
def behind_proxy(configure) -> Iterator[TestClient]:
configure(
trust_proxy_headers=True,
trusted_proxy_cidrs="172.16.0.0/12,10.0.0.0/8",
share_create_rate_limit="2/hour",
)
yield from _client(PROXY)
@pytest.fixture()
def direct_client(configure) -> Iterator[TestClient]:
configure(
trust_proxy_headers=True,
trusted_proxy_cidrs="172.16.0.0/12,10.0.0.0/8",
share_create_rate_limit="2/hour",
)
yield from _client(DIRECT)
def _create(client: TestClient, forwarded_for: str | None) -> int:
headers = {"X-Forwarded-For": forwarded_for} if forwarded_for else {}
return client.post(
"/api/shares", json={"title": "x", "map": build_map()}, headers=headers
).status_code
def test_limit_follows_the_address_transmitted_by_an_approved_proxy(
behind_proxy: TestClient,
) -> None:
assert _create(behind_proxy, "198.51.100.10") == 201
assert _create(behind_proxy, "198.51.100.10") == 201
assert _create(behind_proxy, "198.51.100.10") == 429
# Un autre client, derriere le meme proxy, garde son propre compteur.
assert _create(behind_proxy, "198.51.100.11") == 201
def test_approved_proxy_ignores_the_part_forged_by_the_client(
behind_proxy: TestClient,
) -> None:
"""Le client ecrit "9.9.9.9" ; le proxy ajoute l'adresse qu'il a vue."""
assert _create(behind_proxy, "9.9.9.9, 198.51.100.20") == 201
assert _create(behind_proxy, "9.9.9.9, 198.51.100.20") == 201
assert _create(behind_proxy, "9.9.9.9, 198.51.100.20") == 429
# Changer la partie forgee ne remet pas le compteur a zero.
assert _create(behind_proxy, "1.1.1.1, 198.51.100.20") == 429
def test_direct_client_cannot_escape_the_limit_by_forging_the_header(
direct_client: TestClient,
) -> None:
assert _create(direct_client, "10.0.0.1") == 201
assert _create(direct_client, "10.0.0.2") == 201
# Adresse forgee differente a chaque fois : la socket, elle, ne change pas.
assert _create(direct_client, "10.0.0.3") == 429
assert _create(direct_client, "10.0.0.4") == 429
assert _create(direct_client, None) == 429
def test_ipv6_rewriting_does_not_reset_the_counter(behind_proxy: TestClient) -> None:
assert _create(behind_proxy, "2001:db8::1") == 201
assert _create(behind_proxy, "2001:0db8:0000:0000:0000:0000:0000:0001") == 201
assert _create(behind_proxy, "2001:DB8:0:0:0:0:0:1") == 429
def test_headers_are_ignored_when_trust_is_disabled(configure) -> None:
configure(trust_proxy_headers=False, share_create_rate_limit="2/hour")
with TestClient(app, client=(PROXY, 40000)) as client:
assert _create(client, "198.51.100.30") == 201
assert _create(client, "198.51.100.31") == 201
assert _create(client, "198.51.100.32") == 429