From 8ab65699749268abba052c9915ef32dbe6ca3d04 Mon Sep 17 00:00:00 2001 From: Bruno Charest Date: Wed, 30 Sep 2026 22:40:34 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20A11=20+=20A18=20=E2=80=94=20path=20trave?= =?UTF-8?q?rsal=20avatar=20et=20XSS/flags=20sur=20la=20vue=20publique=20(v?= =?UTF-8?q?7.3.2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A11 : `GET /api/settings/avatar/{filename:path}` → `resolve()` + `relative_to()` (motif de `serve_uploaded_file`), 403 hors de `/data/avatars` - A18 : `GET /workspace/public/{id}` → 404 HTML explicite pour `permission_type` restricted/private, `html.escape` sur le nom, l'icône et les titres de lignes (le f-string HTML ne passe pas par Jinja2) - `tests/test_audit_p0_fixes.py` : 3 tests de non-régression (traversal, échappement, hidden restricted) - ROADMAP A11/A18 cochés · CHANGELOG/WORKLOAD/VERSION → 7.3.2 · suite **1019/1019** · `ruff check app tests` OK --- CHANGELOG.md | 12 ++++++++++++ ROADMAP.md | 6 +++--- VERSION | 2 +- WORKLOAD.md | 2 +- app/main.py | 2 +- app/routers/dashboard.py | 9 ++++++++- app/routers/workspace.py | 26 ++++++++++++++++++++++---- tests/test_audit_p0_fixes.py | 36 ++++++++++++++++++++++++++++++++++++ 8 files changed, 84 insertions(+), 11 deletions(-) create mode 100644 tests/test_audit_p0_fixes.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cbd602..8e1fa39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog - FlowDeck +## v7.3.2 (2026-09-30) — Audit sécurité : A11 + A18 + +### Fixed + +- **A11** — `GET /api/settings/avatar/{filename:path}` : `resolve()` + + `relative_to()` (le motif de `serve_uploaded_file`) → 403 hors `/data/avatars` +- **A18** — `GET /workspace/public/{id}` : 404 explicite pour les bases + `restricted`/`private` (`permission_type`) et `html.escape` sur nom, icône et + titres de lignes — ce f-string HTML ne passe pas par Jinja2, donc l'autoescape + A10 ne le couvrait pas +- Tests : `tests/test_audit_p0_fixes.py` (3 non-régressions) — suite **1019/1019** + ## v7.3.1 (2026-09-30) — Audit sécurité P0 : A1–A10 > Corrections du bloc critique de l'audit du 2026-09-30 (ROADMAP) : plus aucune diff --git a/ROADMAP.md b/ROADMAP.md index d17d1f9..5ef2a6f 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1129,7 +1129,7 @@ Quality DB views, Agent IA Palette → Realtime + E - [x] **A8 — Mot de passe admin codé en dur, re-seedé à chaque boot** : `app/main.py:80` `hash_password("FlowDeck2026!")` puis `INSERT OR IGNORE ... 'admin' ... is_admin=1` (80-85). Literal commité + scannable + réappliqué si le hash est effacé. *Fix : mot de passe aléatoire au premier boot (affiché une fois) ou `FLOWDECK_ADMIN_PASSWORD` ; ne re-hasher qu'au premier démarrage. Effort : **XS**.* - [x] **A9 — DB de prod trackée dans git** : `flowdeck.db` (11 users, e-mails, `password_hash`, 9 sessions actives), `flowdeck_dev.db`, `test-commit.md`, `upload_test.txt` sont dans l'index **et** absents de `.gitignore` **et** de `.dockerignore` → `COPY . .` les embarque dans l'image. *Fix : `git rm --cached` + ajouter `*.db`, `*.db-*`, `test-commit.md`, `upload_test.txt`, `e2e/node_modules/`, `e2e/shots/` à `.gitignore` **et** `.dockerignore` + rotation du `app_secret_key` (les sessions sont révoquées). Effort : **S**.* - [x] **A10 — Jinja2 `autoescape` désactivé partout** : les 29 sites construisent `Environment(loader=FileSystemLoader("app/templates"))` sans `autoescape` (vérifié à l'exécution : `autoescape = False`, jinja2 3.1.6 ; `grep autoescape app/*.py` → 0 hit). Résultat : 326 interpolations `{{ … }}` brutes dans 39 templates, et **tous les `|safe` du codebase sont des no-op**. Pannes concrètes : `notes.html:16,25` (`` + preview), `public_page.html:7,161` (titre non échappé sur les pages publiques `/s/`), `card_detail.html:48,85` (`issue.body|safe`, `comment.body|safe`), `base.html:140` (nom de page injecté en JS inline → exécution), `base.html:1789` (`innerHTML + item.name` depuis l'arbre Gitea), `base.html:1333` (`safeName` n'échappe que les guillemets, pas `<`/`>`). *Fix **à la racine** : un seul `app/templating.py` avec `ENV = Environment(loader=..., autoescape=select_autoescape(["html"]))`, remplacer les 29 instantiations, puis re-trier les `|safe`. Effort : **M**.* -- [ ] **A11 — Path traversal en lecture** : `GET /api/settings/avatar/{filename:path}` (`dashboard.py:1870-1877`) fait `Path("/data/avatars") / filename` puis `FileResponse` **sans `.resolve()` ni `relative_to()`** alors que le bon motif existe 40 lignes plus bas (`dashboard.py:1479-1484`). Le `:path` Starlette accepte les `/`. *Fix : copier la garde de `serve_uploaded_file`. Effort : **XS**.* +- [x] **A11 — Path traversal en lecture** : `GET /api/settings/avatar/{filename:path}` (`dashboard.py:1870-1877`) fait `Path("/data/avatars") / filename` puis `FileResponse` **sans `.resolve()` ni `relative_to()`** alors que le bon motif existe 40 lignes plus bas (`dashboard.py:1479-1484`). Le `:path` Starlette accepte les `/`. *Fix : copier la garde de `serve_uploaded_file`. Effort : **XS**.* ### 🟠 P1 — Hautes @@ -1139,7 +1139,7 @@ Quality DB views, Agent IA Palette → Realtime + E - [ ] **A15 — Webhooks sortants créés sans auth** : `POST /workspace/webhooks` (`workspace.py:672-686`) : aucune auth, aucune validation d'URL, `DELETE` (689) idem → + le retry scheduler, le serveur POSTe chaque événement (titres, contenu) vers l'URL d'un attaquant. *Fix : session admin + `_is_public_host`. Effort : **S**.* - [ ] **A16 — Lectures de pages/export sans aucune ACL** : `export.py:53` (`_load_page_or_404` = simple `SELECT ... WHERE id=?`), `dashboard.py:1141-1186` (`download_page_file`, `page_file_content`), et la lecture legacy `board.py:1420-1424` → contenu de **toute** page énumérable par id, sans session. *Fix : passer par `PermissionManager.can_view_page` + 401 anonymous. Effort : **M**.* - [ ] **A17 — Router legacy `/api` qui mute sans auth** : `move_card` (`api.py:98`), `set_col_mapping` (177), `delete_col_mapping` (207), `create_issue`/`update_issue` (281/322), `delete_checklist[_item]` (522/531), `PUT /users/me` (558) → seul garde = `_check_rate_limit`. *Fix : un `dependencies=[Depends(...)]` au niveau du router (session **ou** Bearer). Effort : **S**.* -- [ ] **A18 — Collection publiée quelconque + stocké XSS** : `GET /workspace/public/{collection_id}` (`workspace.py:699-719`) « no auth required », **ignore les flags `restricted/private`**, et interpole `coll['name']`/`p['title']` dans un `HTMLResponse(f"""…""")` sans `html.escape`. *Fix : respecter les flags de partage + `html.escape`. Effort : **S**.* +- [x] **A18 — Collection publiée quelconque + stocké XSS** : `GET /workspace/public/{collection_id}` (`workspace.py:699-719`) « no auth required », **ignore les flags `restricted/private`**, et interpole `coll['name']`/`p['title']` dans un `HTMLResponse(f"""…""")` sans `html.escape`. *Fix : respecter les flags de partage + `html.escape`. Effort : **S**.* - [ ] **A19 — Liste CSRF trop large (34 préfixes, match `startswith`)** : `csrf.py:21,25` couvre `/api/v2`, `/api/admin`, `/db/`, `/workspace`, `/api/user`, `/api/settings`, `/board/api/pages`, `/api/local-workspace`, `/api/comments`, `/api/agent`, `/api/automations`, `/auth/2fa` — tous **cookie-auth**. Seul `/scim/v2` est justifié par le commentaire de la ligne 19-20. Bonus : `/api/workspace` exempt aussi `/api/workspaces/*`. Filet restant = `SameSite=Lax` par défaut (jamais déclaré explicitement dans `main.py:150`). *Fix : garder un petit ensemble SAFE (webhooks, `/api/v1`, `/api/v2` Bearer, `/scim/v2`, callbacks OAuth/SSO) + ancrer les préfixes ; ajouter le header sur les 49 `fetch()` concernés (helper `csrfFetch` existe déjà : `base.html:892`). Effort : **M**.* - [ ] **A20 — CSP sans filet : `script-src 'unsafe-inline' 'unsafe-eval'`** (`security.py:67`) → aucun nonce/hash ; combiné à A10, chaque sink XSS ci-dessus tourne sans violation CSP. *Fix : externaliser le JS inline (A27), passer à `'nonce-…'`, retirer `'unsafe-eval'` (Alpine/HTMX n'en ont pas besoin par défaut), resserrer `img-src`/`connect-src`. Effort : **L**.* - [ ] **A21 — `sqlite3` synchrone sur l'event loop** : `get_conn()` (`db.py:833-843`) est synchrone et **510 des 689 `async def` de routes** l'appellent (805 occurrences au total ; 0 `run_in_threadpool`, 1 seul `asyncio.to_thread` dans tout le dépôt : `semantic_search.py:262`) ; connexion neuve par requête (`connect` + 2 PRAGMA), **aucun `busy_timeout`**. Chaque requête bloque la boucle. *Fix : wrapper async (`anyio.to_thread.run_sync`) partagé, migrer d'abord `api_v2`/`dashboard`/`collections`/`board` + `PRAGMA busy_timeout=5000`. Effort : **M**.* @@ -1185,4 +1185,4 @@ Quality DB views, Agent IA Palette → Realtime + E → Puis **A3–A8** (le bloc « fallback admin ») d'un seul tenant, puis **A10** (autoescape) qui débloque A18/A20. *Audit produit le 2026-09-30 · 43 items · aucun code modifié ( ROADMAP seul ).* -→ **A1–A9 corrigés le 2026-09-30** : deps réinstallées (`pyotp`/`webauthn`/`cbor2`), rebinding de `settings` supprimé dans `test_v54.py` → **suite 1016/1016 verts**, cycle committé (`1706ad1`) + tag `v7.3.0` poussé, `.db`/fichiers de test désindexés, `APP_SECRET_KEY` roté dans `.env` (sessions révoquées) · **A3–A8 : 401 sans session sur les routes de compte (mdp actuel exigé), tokens `/api/v1` + `/api/user` sans session → 401, CRUD membres d'espace sous session+role admin, `_require_view`/`_require_edit` sans session → 404/401, création/lecture de page sous session, `/board/api/pages` + `/api/user` sortis du CSRF exempt, seed admin sans mdp en dur (aléatoire ou `FLOWDECK_ADMIN_PASSWORD`). Tests : client connecte par defaut (`_TestSessionAuth`), helper `anon()` sur les 40 tests d'anonymat → suite 1016/1016 + ruff OK, commit `d125eb3` · **A10 : `app/templating.py` (ENV partagé + autoescape `select_autoescape(["html"])`) remplace les 29 instantiations, `|safe` retriés (corps d'issue/commentaires echappes, `sidebar_config` en `|tojson`) → suite 1016/1016, version 7.3.1.** +→ **A1–A9 corrigés le 2026-09-30** : deps réinstallées (`pyotp`/`webauthn`/`cbor2`), rebinding de `settings` supprimé dans `test_v54.py` → **suite 1016/1016 verts**, cycle committé (`1706ad1`) + tag `v7.3.0` poussé, `.db`/fichiers de test désindexés, `APP_SECRET_KEY` roté dans `.env` (sessions révoquées) · **A3–A8 : 401 sans session sur les routes de compte (mdp actuel exigé), tokens `/api/v1` + `/api/user` sans session → 401, CRUD membres d'espace sous session+role admin, `_require_view`/`_require_edit` sans session → 404/401, création/lecture de page sous session, `/board/api/pages` + `/api/user` sortis du CSRF exempt, seed admin sans mdp en dur (aléatoire ou `FLOWDECK_ADMIN_PASSWORD`). Tests : client connecte par defaut (`_TestSessionAuth`), helper `anon()` sur les 40 tests d'anonymat → suite 1016/1016 + ruff OK, commit `d125eb3` · **A10 : `app/templating.py` (ENV partagé + autoescape `select_autoescape(["html"])`) remplace les 29 instantiations, `|safe` retriés (corps d'issue/commentaires echappes, `sidebar_config` en `|tojson`) → suite 1016/1016, version 7.3.1 · **A11 (traversal avatar) + A18 (vue publique : 404 restricted/private + html.escape)** : `tests/test_audit_p0_fixes.py`, suite 1019/1019, version 7.3.2.** diff --git a/VERSION b/VERSION index 643916c..eab246c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -7.3.1 +7.3.2 diff --git a/WORKLOAD.md b/WORKLOAD.md index b251549..6b06978 100644 --- a/WORKLOAD.md +++ b/WORKLOAD.md @@ -1,6 +1,6 @@ # WORKLOAD — FlowDeck Notion Clone -> **Début**: 2026-07-08 | **Version**: v7.3.1 (audit sécurité P0 A1–A10) | **Statut**: EN COURS 🔄 +> **Début**: 2026-07-08 | **Version**: v7.3.2 (audit sécurité A1–A11 + A18) | **Statut**: EN COURS 🔄 > **Cible**: parité Notion + intégration forge · **Follow-ups v7.3 livrés**: sidebar teamspaces, notif `page.updated`, charts `number` + dashboards multi-DB, unfurl forge, UI Settings → Audit — voir `ROADMAP.md § v7.3.0` ## Avancement Global diff --git a/app/main.py b/app/main.py index 4b60774..7c4dea6 100644 --- a/app/main.py +++ b/app/main.py @@ -153,7 +153,7 @@ async def lifespan(_app: FastAPI): app = FastAPI( title="FlowDeck", - version="7.3.1", + version="7.3.2", docs_url="/docs", redoc_url="/redoc", lifespan=lifespan, diff --git a/app/routers/dashboard.py b/app/routers/dashboard.py index 164ea53..aed0c33 100644 --- a/app/routers/dashboard.py +++ b/app/routers/dashboard.py @@ -1884,7 +1884,14 @@ async def serve_avatar_file(filename: str): from pathlib import Path from fastapi.responses import FileResponse - filepath = Path("/data/avatars") / filename + # A11 : garde path traversal (motif de serve_uploaded_file) — `:path` Starlette + # accepte les `/`, donc `..%2f` ressortirait du dossier avatars. + base_dir = Path("/data/avatars").resolve() + filepath = (base_dir / filename).resolve() + try: + filepath.relative_to(base_dir) + except ValueError: + return JSONResponse({"error": "Path traversal denied"}, status_code=403) if not filepath.is_file(): return JSONResponse({"error": "Not found"}, status_code=404) return FileResponse(filepath) diff --git a/app/routers/workspace.py b/app/routers/workspace.py index ba8b951..50f1154 100644 --- a/app/routers/workspace.py +++ b/app/routers/workspace.py @@ -2,6 +2,7 @@ from __future__ import annotations import csv +import html import io import json import logging @@ -721,22 +722,39 @@ async def delete_webhook(request: Request, wh_id: int): @router.get("/public/{collection_id}") async def public_view(request: Request, collection_id: int): - """Simple public read-only view — no auth required.""" + """Simple public read-only view — no auth required. + + A18 : les bases ``restricted``/``private`` (``permission_type``) restent + masquées (404) et toute interpolation part dans ``html.escape`` (XSS stocké + sur le titre de la base ou d'une ligne). + """ with get_conn() as conn: coll = conn.execute("SELECT * FROM collections WHERE id=?", (collection_id,)).fetchone() if not coll: raise HTTPException(404, "Collection not found") + ptype = coll["permission_type"] if "permission_type" in coll.keys() else "inherit" + if ptype in ("restricted", "private"): + # 404 explicite : le handler global transformerait un HTTPException(404) + # en redirection 302 → login pour un chemin HTML. + return HTMLResponse( + "404" + "

404 — Not found

", + status_code=404, + ) pages = conn.execute( "SELECT id, title, icon, property_values_json FROM collection_pages WHERE collection_id=? ORDER BY position", (collection_id,), ).fetchall() + esc = html.escape + name = esc(str(coll["name"] or "")) + icon = esc(str(coll["icon"] or "")) items = "".join( - f"
  • {p['icon']} {p['title']}
    • " + f"
  • {esc(str(p['icon'] or ''))} {esc(str(p['title'] or ''))}
    • " for p in pages ) return HTMLResponse(f""" -{coll['name']} — FlowDeck Public +{name} — FlowDeck Public -

    {coll['icon']} {coll['name']}

    {len(pages)} items

    """) +

    {icon} {name}

    {len(pages)} items

    """) diff --git a/tests/test_audit_p0_fixes.py b/tests/test_audit_p0_fixes.py new file mode 100644 index 0000000..dfbfc43 --- /dev/null +++ b/tests/test_audit_p0_fixes.py @@ -0,0 +1,36 @@ +"""Non-régression de l'audit sécurité 2026-09-30 — A11 (traversal) et A18 (XSS public).""" +from conftest import anon + + +def test_avatar_path_traversal_denied(client): + """A11 : `:path` accepte les `/` — la lecture doit rester dans /data/avatars.""" + r = client.get("/api/settings/avatar/..%2f..%2fetc%2fpasswd") + assert r.status_code in (403, 404), r.status_code + + +def test_public_view_escapes_output(client): + """A18 : titre de base et titre de ligne interpolés dans un f-string HTML.""" + cid = client.post("/db/api", json={"name": ""}).json()["id"] + client.post(f"/db/{cid}/pages/api", json={"title": ""}) + + anon(client) + r = client.get(f"/workspace/public/{cid}") + assert r.status_code == 200 + assert "