From 28f9a92abc0ff4fe6f318fe56b86c366155db27b Mon Sep 17 00:00:00 2001 From: skadel Date: Sat, 18 Jul 2026 22:09:34 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(s=C3=A9cu):=20gardes=20anti-traversal?= =?UTF-8?q?=20+=20durcissement=20subprocess=20bq=20(audit=202026-07)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Suite à l'audit de sécurité du projet : - Traversée de chemin via `model_name`/`full_path` (contrôlés côté HTTP) : nouveau helper `utils/path_guard.py:safe_join` (containment sous racine), appliqué à `_test_path` (raise), `read_model_sql`/`get_model_file_*`, `load_model_context` et le catch-all SPA de `server.py`. Un `../` ne peut plus lire/écrire/supprimer hors de `.mocksql/tests`, `models_path` ou le dossier statique (fuite `back/.env`). - Injection de commande dans `_run_bq_cli` : retrait de `shell=True`, exécutable résolu via `shutil.which`, et validation stricte (`_require_bq_component`) des composants projet/dataset/table/billing_project au build de la commande. Le wrapper `bq` étant un `.cmd` (arguments re-parsés par cmd.exe même sans shell), la validation des composants est le vrai garde-fou. - Garde hex sur `source_sha` avant injection dans un argv git (option-injection). - Outillage : règle ruff flake8-bandit `S` activée ; bruit inhérent au domaine ignoré (asserts, SQL en f-string). Les subprocess/binds restants sont `# noqa` justifiés ; les 2 binds `0.0.0.0` portent un TODO pointant le finding #1 (non traité — décision produit). Tests : back/tests/test_security_guards.py (10, rouges avant le fix). --- back/build_query/schema_fetcher.py | 44 +++++++- back/cli/main.py | 4 +- back/pyproject.toml | 18 +++ back/server.py | 17 ++- back/storage/context_loader.py | 6 + back/storage/test_repository.py | 48 +++++--- back/tests/test_security_guards.py | 170 +++++++++++++++++++++++++++++ back/utils/path_guard.py | 37 +++++++ 8 files changed, 321 insertions(+), 23 deletions(-) create mode 100644 back/tests/test_security_guards.py create mode 100644 back/utils/path_guard.py diff --git a/back/build_query/schema_fetcher.py b/back/build_query/schema_fetcher.py index 296e98a..24a066c 100644 --- a/back/build_query/schema_fetcher.py +++ b/back/build_query/schema_fetcher.py @@ -14,6 +14,20 @@ def validate_bq_ref(ref: str) -> bool: return len(parts) >= 2 and all(_BQ_IDENT_RE.match(p) for p in parts) +def _require_bq_component(value: str, label: str) -> str: + """Refuse tout composant destiné à la ligne de commande `bq` (projet, dataset, + table, billing_project) qui contient autre chose qu'un identifiant BigQuery. + + Le wrapper `bq` est un `.cmd` sous Windows → ses arguments sont re-parsés par + cmd.exe même sans ``shell=True`` (classe « BatBadBut »). La validation stricte + au point de construction de la commande est donc le vrai garde-fou anti-injection, + en complément de ``validate_bq_ref`` côté API (défense en profondeur, audit 2026-07). + """ + if not isinstance(value, str) or not _BQ_IDENT_RE.match(value): + raise ValueError(f"Invalid BigQuery {label} (illegal characters): {value!r}") + return value + + def parse_ref(ref: str, billing_project: str) -> tuple[str, str, str]: parts = ref.split(".") if len(parts) == 2: @@ -179,9 +193,25 @@ def _is_auth_error(exc: Exception) -> bool: async def _run_bq_cli( cmd: list[str], billing_project: str, timeout: int = _CLI_TIMEOUT ) -> str: - """Run a bq CLI command and return stdout, raising on auth errors or non-zero exit.""" + """Run a bq CLI command and return stdout, raising on auth errors or non-zero exit. + + L'exécutable (``cmd[0]``) est résolu explicitement via ``shutil.which`` et lancé + sans ``shell=True`` : plus aucune interprétation de la ligne par cmd.exe/sh, donc + les métacaractères d'un argument ne peuvent plus enchaîner une commande (audit + sécu 2026-07). Les composants (projet/dataset/table) sont par ailleurs validés + par l'appelant (``_require_bq_component``). + """ + import shutil import subprocess + resolved = shutil.which(cmd[0]) + if not resolved: + raise RuntimeError( + f"'{cmd[0]}' introuvable dans le PATH — installer le Google Cloud SDK " + "(bq) ou utiliser l'API BigQuery." + ) + argv = [resolved, *cmd[1:]] + extra: dict = {} if os.name == "nt": extra["creationflags"] = subprocess.CREATE_NO_WINDOW @@ -189,11 +219,11 @@ async def _run_bq_cli( env["CLOUDSDK_CORE_DISABLE_PROMPTS"] = "1" try: result = await asyncio.to_thread( - lambda c=cmd: subprocess.run( + lambda c=argv: subprocess.run( # noqa: S603 exe résolu (shutil.which), composants validés c, capture_output=True, text=True, - shell=True, + shell=False, stdin=subprocess.DEVNULL, timeout=timeout, env=env, @@ -219,6 +249,10 @@ async def _fetch_table_via_cli( ``bq show --schema`` so that timePartitioning info is available. """ proj, dataset, table = parse_ref(ref, billing_project) + _require_bq_component(billing_project, "billing_project") + _require_bq_component(proj, "project") + _require_bq_component(dataset, "dataset") + _require_bq_component(table, "table") bq_ref = f"{proj}:{dataset}.{table}" cmd = [ "bq", @@ -276,6 +310,10 @@ async def _fetch_partition_values_cli( ) -> tuple[list[str], bool]: """Fetch the last *limit* partition IDs via ``bq query`` on INFORMATION_SCHEMA.""" proj, dataset, table = parse_ref(ref, billing_project) + _require_bq_component(billing_project, "billing_project") + _require_bq_component(proj, "project") + _require_bq_component(dataset, "dataset") + _require_bq_component(table, "table") sql = ( f"SELECT partition_id " f"FROM `{proj}.{dataset}.INFORMATION_SCHEMA.PARTITIONS` " diff --git a/back/cli/main.py b/back/cli/main.py index 4af85f5..f48fba9 100644 --- a/back/cli/main.py +++ b/back/cli/main.py @@ -1128,7 +1128,9 @@ def _open_when_ready() -> None: threading.Thread(target=_open_when_ready, daemon=True).start() - uvicorn.run(server_module, host="0.0.0.0", port=port) + # TODO(sécu audit 2026-07, finding #1) : binder 127.0.0.1 par défaut + token si + # exposition réseau voulue. Bind 0.0.0.0 conservé le temps de la décision produit. + uvicorn.run(server_module, host="0.0.0.0", port=port) # noqa: S104 def main() -> None: diff --git a/back/pyproject.toml b/back/pyproject.toml index 18159d2..930a4ec 100644 --- a/back/pyproject.toml +++ b/back/pyproject.toml @@ -80,6 +80,24 @@ pytest-asyncio = "^1.4.0" pytest-mock = "^3.15.1" httpx = "^0.28.1" +[tool.ruff.lint] +# Défauts ruff (E4/E7/E9 + F) + flake8-bandit (S) pour la sécurité. Les règles S +# à fort bruit et inhérentes au domaine (asserts de tests, SQL en f-string au cœur +# d'un outil de test SQL) sont désactivées ; on garde le signal utile : subprocess, +# bind-all-interfaces, etc. Audit sécu 2026-07. +select = ["E4", "E7", "E9", "F", "S"] +ignore = [ + "S101", # assert — omniprésent et légitime (tests + gardes internes) + "S608", # SQL en f-string — construction interne de requêtes DuckDB (par design) + "S110", # try/except/pass — nettoyages best-effort assumés + "S112", # try/except/continue — idem + "S311", # random non-crypto — génération de données de test, pas de la crypto +] + +[tool.ruff.lint.per-file-ignores] +# Les tests utilisent assert + subprocess + données factices → S non pertinent. +"tests/**" = ["S"] + [tool.pytest.ini_options] asyncio_mode = "auto" testpaths = ["tests"] diff --git a/back/server.py b/back/server.py index 8c5c920..3ba9fb5 100644 --- a/back/server.py +++ b/back/server.py @@ -101,16 +101,23 @@ async def root(): async def serve_spa(full_path: str, request: Request): if request.url.path.startswith("/api"): raise HTTPException(status_code=404, detail="Not Found") - # si le fichier existe physiquement, on le renvoie - file_path = get_static_dir() / full_path - if file_path.is_file(): + # si le fichier existe physiquement, on le renvoie — mais uniquement s'il reste + # sous le dossier statique (un `../` ne doit pas servir back/.env, audit sécu + # 2026-07). Tout chemin hors racine retombe sur l'index SPA. + from utils.path_guard import safe_join + + static_dir = get_static_dir() + file_path = safe_join(static_dir, full_path) + if file_path is not None and file_path.is_file(): return FileResponse(str(file_path)) # sinon SPA - return FileResponse(str(get_static_dir() / "index.html")) + return FileResponse(str(static_dir / "index.html")) # ─── 9) Uvicorn – exécution directe ───────────────── if __name__ == "__main__": import uvicorn - uvicorn.run(app, host="0.0.0.0", port=int(os.getenv("PORT", 8080)), reload=False) + # TODO(sécu audit 2026-07, finding #1) : binder 127.0.0.1 par défaut + token si + # exposition réseau voulue. Bind 0.0.0.0 conservé le temps de la décision produit. + uvicorn.run(app, host="0.0.0.0", port=int(os.getenv("PORT", 8080)), reload=False) # noqa: S104 diff --git a/back/storage/context_loader.py b/back/storage/context_loader.py index 03cb4a3..abaef15 100644 --- a/back/storage/context_loader.py +++ b/back/storage/context_loader.py @@ -1,6 +1,7 @@ from pathlib import Path from storage.config import get_models_path +from utils.path_guard import safe_join def load_model_context(model_name: str) -> str: @@ -19,6 +20,11 @@ def load_model_context(model_name: str) -> str: models_path = get_models_path() parts = Path(model_name).parts # e.g. ("finance", "revenue") + # Garde traversal : `model_name` vient de l'appelant HTTP — un `../` ne doit pas + # aspirer un `.md` hors de models_path (audit sécu 2026-07). + if safe_join(models_path, model_name, suffix=".md") is None: + return "" + fragments: list[str] = [] # Walk from models_path root down to the file's parent directory diff --git a/back/storage/test_repository.py b/back/storage/test_repository.py index d25210e..7bb9463 100644 --- a/back/storage/test_repository.py +++ b/back/storage/test_repository.py @@ -15,6 +15,7 @@ load_preprocessor_fn, ) from storage.test_files import cache_path_for, read_test_doc, write_test_doc +from utils.path_guard import safe_join _UUID_RE = re.compile( r"^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$", re.I @@ -23,6 +24,11 @@ # Namespace fixe pour dériver un test_id stable d'un model_name (uuid5). _TEST_ID_NAMESPACE = uuid.UUID("6f9619ff-8b86-d011-b42d-00cf4fc964ff") +# Un SHA de commit git = hex. Valider avant de l'injecter dans un argv git empêche +# qu'un `source_sha` commençant par `-` soit interprété comme une option git (audit +# sécu 2026-07). +_GIT_SHA_RE = re.compile(r"^[0-9a-fA-F]{7,64}$") + # Tests créés mais pas encore persistés (en attente de validation réussie). _pending_tests: Dict[str, Dict[str, Any]] = {} @@ -39,12 +45,24 @@ def _tests_root() -> Path: def _test_path(model_name: str) -> Path: - """Single file per model: .mocksql/tests/{model_id}.json (supports nested paths).""" - p = _tests_root() / f"{model_name}.json" + """Single file per model: .mocksql/tests/{model_id}.json (supports nested paths). + + ``model_name`` provient des endpoints HTTP → passer par ``safe_join`` pour qu'un + `../` ne fasse pas écrire/supprimer hors de la racine tests (audit sécu 2026-07). + """ + p = safe_join(_tests_root(), model_name, suffix=".json") + if p is None: + raise ValueError(f"Invalid model name (path traversal): {model_name!r}") p.parent.mkdir(parents=True, exist_ok=True) return p +def _model_sql_path(model_name: str) -> Optional[Path]: + """Chemin du fichier `.sql` d'un modèle sous ``models_path``, ou ``None`` si + ``model_name`` tente de s'échapper de la racine (traversal).""" + return safe_join(get_models_path(), model_name, suffix=".sql") + + def _derive_model_name_from_path(p: Path) -> Optional[str]: """`.../tests/.json` → `` (posix), ou None si hors de la racine tests.""" try: @@ -159,8 +177,8 @@ def read_model_sql(model_name: str) -> Optional[str]: if dbt_project and dbt_project.is_dbt_model(model_name): return dbt_project.compiled_sql_for_model(model_name) - sql_file = get_models_path() / f"{model_name}.sql" - if not sql_file.exists(): + sql_file = _model_sql_path(model_name) + if sql_file is None or not sql_file.exists(): return None raw_sql = sql_file.read_text(encoding="utf-8") fn_ref = get_preprocessor_fn() @@ -172,12 +190,12 @@ def read_model_sql(model_name: str) -> Optional[str]: def get_model_file_git_sha(model_name: str) -> Optional[str]: """Return the SHA of the last commit that touched the model's .sql file, or None.""" - sql_file = get_models_path() / f"{model_name}.sql" - if not sql_file.exists(): + sql_file = _model_sql_path(model_name) + if sql_file is None or not sql_file.exists(): return None try: - result = subprocess.run( - ["git", "log", "-1", "--format=%H", "--", str(sql_file)], + result = subprocess.run( # noqa: S603 argv fixe, git en PATH, chemin validé + ["git", "log", "-1", "--format=%H", "--", str(sql_file)], # noqa: S607 capture_output=True, text=True, timeout=5, @@ -191,8 +209,8 @@ def get_model_file_git_sha(model_name: str) -> Optional[str]: def get_model_file_hash(model_name: str) -> Optional[str]: """Return a short SHA-256 hash of the model file's current content, or None.""" - sql_file = get_models_path() / f"{model_name}.sql" - if not sql_file.exists(): + sql_file = _model_sql_path(model_name) + if sql_file is None or not sql_file.exists(): return None try: content = sql_file.read_bytes() @@ -203,12 +221,14 @@ def get_model_file_hash(model_name: str) -> Optional[str]: def get_commits_since_sha(model_name: str, source_sha: str) -> int: """Return the number of commits that touched the model file since source_sha.""" - sql_file = get_models_path() / f"{model_name}.sql" - if not sql_file.exists(): + sql_file = _model_sql_path(model_name) + if sql_file is None or not sql_file.exists(): + return 0 + if not _GIT_SHA_RE.match(source_sha or ""): return 0 try: - result = subprocess.run( - ["git", "rev-list", "--count", f"{source_sha}..HEAD", "--", str(sql_file)], + result = subprocess.run( # noqa: S603 argv fixe, git en PATH, sha/chemin validés + ["git", "rev-list", "--count", f"{source_sha}..HEAD", "--", str(sql_file)], # noqa: S607 capture_output=True, text=True, timeout=5, diff --git a/back/tests/test_security_guards.py b/back/tests/test_security_guards.py new file mode 100644 index 0000000..c204359 --- /dev/null +++ b/back/tests/test_security_guards.py @@ -0,0 +1,170 @@ +"""Régression sécurité (audit 2026-07) : traversée de chemin via `model_name` et +injection de commande via le CLI `bq`. + +1. Traversal — `_test_path` / `read_model_sql` / `load_model_context` joignaient + `model_name` (contrôlé par l'appelant HTTP) au filesystem sans vérifier que le + résultat reste sous la racine : `../../` lisait/écrivait/supprimait hors de + `.mocksql/tests` et de `models_path`. Idem pour le catch-all SPA de server.py + (couvert ici au niveau du helper `safe_join`). + +2. Commande bq — `_run_bq_cli` passait `shell=True` : sous Windows, les + métacaractères cmd.exe (`&`, `|`, `%`) dans un argument non validé exécutent + une commande arbitraire. Le fix retire `shell=True` (résolution de `bq` via + `shutil.which`) et valide chaque composant (proj/dataset/table/billing_project) + au point de construction de la commande — défense en profondeur, l'API valide + déjà en amont via `validate_bq_ref`. +""" + +import asyncio +import subprocess +from pathlib import Path +from types import SimpleNamespace + +import pytest + + +@pytest.fixture +def repo(tmp_path, monkeypatch): + monkeypatch.setenv("MOCKSQL_BASE_DIR", str(tmp_path)) + import storage.config as config + + config.load_config.cache_clear() + import storage.test_repository as tr + + return tr + + +# --------------------------------------------------------------------------- +# safe_join (helper partagé — utilisé aussi par le catch-all SPA de server.py) +# --------------------------------------------------------------------------- + + +def test_safe_join_accepts_nested_relative(tmp_path): + from utils.path_guard import safe_join + + p = safe_join(tmp_path, "finance/revenue", suffix=".json") + assert p is not None + assert p == tmp_path / "finance" / "revenue.json" + + +def test_safe_join_rejects_parent_escape(tmp_path): + from utils.path_guard import safe_join + + assert safe_join(tmp_path, "../evil", suffix=".json") is None + assert safe_join(tmp_path, "a/../../evil") is None + + +def test_safe_join_rejects_absolute_path(tmp_path): + from utils.path_guard import safe_join + + outside = tmp_path.parent / "evil.sql" + assert safe_join(tmp_path, str(outside)) is None + + +# --------------------------------------------------------------------------- +# test_repository : _test_path / read_model_sql +# --------------------------------------------------------------------------- + + +def test_test_path_rejects_traversal(repo, tmp_path): + with pytest.raises(ValueError): + repo._test_path("../../evil") + # Un nom imbriqué légitime continue de fonctionner. + p = repo._test_path("finance/revenue") + assert p.name == "revenue.json" + + +def test_read_model_sql_rejects_traversal(repo, tmp_path): + # Un fichier .sql existe HORS de models_path — il ne doit pas être lisible. + secret = tmp_path / "secret.sql" + secret.write_text("SELECT 'secret'", encoding="utf-8") + models_dir = Path(repo.get_models_path()) + models_dir.mkdir(parents=True, exist_ok=True) + rel_escape = "../secret" + assert repo.read_model_sql(rel_escape) is None + + +def test_model_file_helpers_reject_traversal(repo, tmp_path): + secret = tmp_path / "secret.sql" + secret.write_text("SELECT 'secret'", encoding="utf-8") + assert repo.get_model_file_hash("../secret") is None + assert repo.get_model_file_git_sha("../secret") is None + assert repo.get_commits_since_sha("../secret", "deadbeef") == 0 + + +def test_load_model_context_rejects_traversal(repo, tmp_path): + from storage.context_loader import load_model_context + + (tmp_path / "secret.md").write_text("contexte secret", encoding="utf-8") + assert load_model_context("../secret") == "" + + +# --------------------------------------------------------------------------- +# schema_fetcher : _run_bq_cli sans shell, composants validés +# --------------------------------------------------------------------------- + + +def _fake_completed(stdout="{}"): + return SimpleNamespace(stdout=stdout, stderr="", returncode=0) + + +def test_run_bq_cli_does_not_use_shell(monkeypatch): + from build_query import schema_fetcher + + captured = {} + + def fake_run(cmd, **kwargs): + captured["cmd"] = cmd + captured.update(kwargs) + return _fake_completed() + + monkeypatch.setattr(subprocess, "run", fake_run) + monkeypatch.setattr( + "shutil.which", lambda name: r"C:\fake\google-cloud-sdk\bin\bq.cmd" + ) + + out = asyncio.run( + schema_fetcher._run_bq_cli(["bq", "show", "--format=prettyjson"], "proj") + ) + assert out == "{}" + assert not captured.get("shell"), "shell=True réintroduit dans _run_bq_cli" + # L'exécutable est résolu explicitement (shutil.which), pas par cmd.exe. + assert captured["cmd"][0].endswith(("bq.cmd", "bq.exe", "bq")) + + +def test_run_bq_cli_fails_fast_when_bq_missing(monkeypatch): + from build_query import schema_fetcher + + # Filet : si le code (pré-fix) tente quand même un subprocess, ne jamais + # exécuter le vrai `bq` (hang interactif possible) — échouer immédiatement. + monkeypatch.setattr( + subprocess, "run", lambda *a, **k: _fake_completed("should not run") + ) + monkeypatch.setattr("shutil.which", lambda name: None) + with pytest.raises(RuntimeError, match="bq"): + asyncio.run(schema_fetcher._run_bq_cli(["bq", "show"], "proj")) + + +def test_fetch_table_via_cli_rejects_metacharacters(monkeypatch): + """Défense en profondeur : même si un appelant oublie validate_bq_ref, aucun + subprocess ne doit être lancé avec un composant contenant un métacaractère.""" + from build_query import schema_fetcher + + def boom(*args, **kwargs): # pragma: no cover — ne doit jamais être atteint + raise AssertionError("subprocess lancé avec un ref non validé") + + monkeypatch.setattr(subprocess, "run", boom) + + with pytest.raises(ValueError): + asyncio.run( + schema_fetcher._fetch_table_via_cli("proj.data&calc.table", "billing") + ) + with pytest.raises(ValueError): + asyncio.run( + schema_fetcher._fetch_partition_values_cli("proj.dataset.tab|le", "billing") + ) + with pytest.raises(ValueError): + # billing_project entre aussi dans la ligne de commande (--project_id=…). + asyncio.run( + schema_fetcher._fetch_table_via_cli("proj.dataset.table", "bad&billing") + ) diff --git a/back/utils/path_guard.py b/back/utils/path_guard.py new file mode 100644 index 0000000..04f58cd --- /dev/null +++ b/back/utils/path_guard.py @@ -0,0 +1,37 @@ +"""Garde de containment : joindre un segment relatif contrôlé par l'appelant +(model_name issu d'une requête HTTP, full_path du catch-all SPA…) à une racine +sans laisser `../` ou un chemin absolu s'échapper de cette racine. + +Toute jointure `root / user_input` du codebase doit passer par ``safe_join`` : +un `model_name = "../../secret"` reçu par un endpoint lisait/écrivait/supprimait +sinon hors de la racine prévue (audit sécurité 2026-07). +""" + +from pathlib import Path +from typing import Optional + + +def safe_join(root: Path, *parts: str, suffix: str = "") -> Optional[Path]: + """Retourne ``root/parts…(+suffix)`` résolu si le résultat reste sous ``root``, + sinon ``None``. + + ``root`` est résolu ; chaque élément de ``parts`` est traité comme un segment + relatif. Un composant absolu, un `..` qui remonte au-dessus de ``root``, ou tout + résultat hors racine donne ``None`` (jamais d'exception — l'appelant décide du + code d'erreur : 404 pour une lecture, ValueError pour une écriture). + """ + base = root.resolve() + # Un segment absolu écrase le join côté pathlib → refus explicite. + for part in parts: + if part is None or Path(part).is_absolute(): + return None + try: + candidate = base.joinpath(*parts) + if suffix: + candidate = candidate.with_suffix(suffix) + resolved = candidate.resolve() + except (ValueError, OSError): + return None + if resolved != base and base not in resolved.parents: + return None + return resolved From c6ce2078ef85dd059bbb869549a36d0a81c44b84 Mon Sep 17 00:00:00 2001 From: skadel Date: Sat, 25 Jul 2026 18:47:40 +0200 Subject: [PATCH 2/2] fix(storage): preserve dotted model names --- back/tests/test_security_guards.py | 11 +++++++++++ back/utils/path_guard.py | 6 +++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/back/tests/test_security_guards.py b/back/tests/test_security_guards.py index c204359..cf7bcae 100644 --- a/back/tests/test_security_guards.py +++ b/back/tests/test_security_guards.py @@ -47,6 +47,17 @@ def test_safe_join_accepts_nested_relative(tmp_path): assert p == tmp_path / "finance" / "revenue.json" +def test_safe_join_appends_suffix_without_replacing_model_version(tmp_path): + from utils.path_guard import safe_join + + v1 = safe_join(tmp_path, "finance/foo.v1", suffix=".json") + v2 = safe_join(tmp_path, "finance/foo.v2", suffix=".json") + + assert v1 == tmp_path / "finance" / "foo.v1.json" + assert v2 == tmp_path / "finance" / "foo.v2.json" + assert v1 != v2 + + def test_safe_join_rejects_parent_escape(tmp_path): from utils.path_guard import safe_join diff --git a/back/utils/path_guard.py b/back/utils/path_guard.py index 04f58cd..bfede2f 100644 --- a/back/utils/path_guard.py +++ b/back/utils/path_guard.py @@ -28,7 +28,11 @@ def safe_join(root: Path, *parts: str, suffix: str = "") -> Optional[Path]: try: candidate = base.joinpath(*parts) if suffix: - candidate = candidate.with_suffix(suffix) + # ``suffix`` est une extension à ajouter au model_id, pas à remplacer. + # Un model_id peut légitimement contenir un point (``foo.v1``) : + # Path.with_suffix(".json") ferait alors collision avec ``foo.v2`` en + # ramenant les deux chemins à ``foo.json``. + candidate = candidate.parent / f"{candidate.name}{suffix}" resolved = candidate.resolve() except (ValueError, OSError): return None