From 5a537f5dc35373029a3634af4a1f0e76f62163ce Mon Sep 17 00:00:00 2001 From: Bruno Charest Date: Wed, 30 Sep 2026 23:12:20 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20A12=E2=80=93A24=20=E2=80=94=20SSRF,=20au?= =?UTF-8?q?th=20routes=20legacy,=20uploads,=20N+1=20et=20routes=20doublonn?= =?UTF-8?q?es=20(v7.3.3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A12 — `og_fetcher` : GET sans `follow_redirects`, `_is_public_host` revérifié à chaque saut (max 5) ; `POST /board/api/og/metadata` → 400 sur hôte privé/loopback - A13 — router automations sous `Depends(_require_session)` (CRUD, run, press-button) + `created_by` sans fallback ; action `webhook` validée par `_is_public_host` avant POST (SSRF) - A15 — webhooks sortants : `_require_admin` sur GET/POST/DELETE + `_is_public_host` sur l'URL en création - A17 — router legacy `/api` sous `Depends(_require_session_or_bearer)` (session ou Bearer `/api/v1`), allowlist explicite `/api/health` + `/api/frontend-error` - A22 — les 2 uploads locales : session exigée (`_require_user_id`) + `validate_upload` branché (taille + extension) + `FLOWDECK_DATA_DIR` au lieu de `/data` codé en dur - A23 — N+1 : COUNT→`GROUP BY` (dashboard), cards→`executemany` (board sync), duplicata de propriétés→`executemany` + remap des ids par SELECT (collections) - A24 — 2 routes écrasées supprimées : `GET /api/projects` (api.py) et `GET /workspace` (workspace.py) + test « aucun doublon méthode+chemin » - Tests : +9 dans `tests/test_audit_p0_fixes.py` (SSRF, 401s, validate_upload, doublons de routes) ; tests OG sur hôtes résolubles (la garde fait du DNS) - suite **1025/1025** · `ruff check app tests` OK --- CHANGELOG.md | 22 +++++++++++++ ROADMAP.md | 16 ++++----- VERSION | 2 +- WORKLOAD.md | 2 +- app/main.py | 2 +- app/routers/api.py | 40 ++++++++++++----------- app/routers/automations.py | 14 ++++++-- app/routers/board.py | 19 +++++++---- app/routers/collections.py | 29 +++++++++++++---- app/routers/dashboard.py | 45 +++++++++++++++++++++----- app/routers/workspace.py | 27 ++++++++++++---- app/services/automations.py | 7 ++++ app/services/og_fetcher.py | 36 +++++++++++++++++++-- tests/test_audit_p0_fixes.py | 63 ++++++++++++++++++++++++++++++++++++ tests/test_v55.py | 8 ++--- 15 files changed, 265 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e1fa39..ec936e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,27 @@ # Changelog - FlowDeck +## v7.3.3 (2026-09-30) — Audit sécurité : A12–A24 (SSRF, auth legacy, uploads, perf) + +### Fixed + +- **A12** — unfurl OG : `follow_redirects` manuel + `_is_public_host` à chaque + saut → 400 vers loopback/link-local (ex. `169.254.169.254`) +- **A13** — automations : `Depends(_require_session)` sur le router entier + (CRUD, run, press-button) + action `webhook` validée avant POST +- **A15** — webhooks sortants : admin exigé + URL publique (SSRF scheduler) +- **A17** — router legacy `/api` : session ou Bearer (`/api/v1`) ; allowlist + explicite `/api/health`, `/api/frontend-error` +- **A22** — uploads locaux : session exigée, `validate_upload` branché (10 MB + + extensions), `FLOWDECK_DATA_DIR` remplace le `/data` codé en dur +- **A23** — N+1 : `GROUP BY` (compteurs de pages), `executemany` (cards de sync + + duplicata de propriétés avec remap d'ids vérifié) +- **A24** — 2 routes silencieusement écrasées supprimées + test « aucun doublon + méthode+chemin » sur les 680 routes + +### Tests + +- +9 non-régressions dans `tests/test_audit_p0_fixes.py` → suite **1025/1025** + ## v7.3.2 (2026-09-30) — Audit sécurité : A11 + A18 ### Fixed diff --git a/ROADMAP.md b/ROADMAP.md index 5ef2a6f..c575015 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1133,19 +1133,19 @@ Quality DB views, Agent IA Palette → Realtime + E ### 🟠 P1 — Hautes -- [ ] **A12 — SSRF avec exfiltration via unfurl OG** : `POST /api/og/metadata` (`board.py:1887,1908`) sans auth, `fetch_og_metadata` (`og_fetcher.py:124-129`) fait `client.get(src, follow_redirects=True)` **sans contrôle d'hôte**, renvoie title/description (~400 car.) → `169.254.169.254`, `localhost` atteignables et partiellement lisibles. Le garde existe déjà : `app/services/importers/url_fetch.py:22 _is_public_host`. *Fix : le réutiliser + re-vérifier après redirection. Effort : **S**.* -- [ ] **A13 — Automations CRUD sans auth + SSRF webhook** : `create_automation` (`automations.py:74`), `update_automation` (113), `delete_automation` (142), `run_automation_endpoint` (160) n'ont **aucune auth** ; `services/automations.py:167-179` poste `context` (données de page) vers `action["url"]` sans validation d'hôte ; `/api/automations` et `/workspace/automations` sont exclus du CSRF. *Fix : session (admin pour CUD) + `_is_public_host` sur l'action webhook. Effort : **S**.* +- [x] **A12 — SSRF avec exfiltration via unfurl OG** : `POST /api/og/metadata` (`board.py:1887,1908`) sans auth, `fetch_og_metadata` (`og_fetcher.py:124-129`) fait `client.get(src, follow_redirects=True)` **sans contrôle d'hôte**, renvoie title/description (~400 car.) → `169.254.169.254`, `localhost` atteignables et partiellement lisibles. Le garde existe déjà : `app/services/importers/url_fetch.py:22 _is_public_host`. *Fix : le réutiliser + re-vérifier après redirection. Effort : **S**.* +- [x] **A13 — Automations CRUD sans auth + SSRF webhook** : `create_automation` (`automations.py:74`), `update_automation` (113), `delete_automation` (142), `run_automation_endpoint` (160) n'ont **aucune auth** ; `services/automations.py:167-179` poste `context` (données de page) vers `action["url"]` sans validation d'hôte ; `/api/automations` et `/workspace/automations` sont exclus du CSRF. *Fix : session (admin pour CUD) + `_is_public_host` sur l'action webhook. Effort : **S**.* - [ ] **A14 — Fallback ligne `admin` dans le router agent → SSRF `api_base`** : `_current_admin` (`agent.py:115-131`) et `_current_user_id` (95-101) retombent sur la row `admin` sans session ; `PATCH /api/agent/providers` et `POST .../providers/test` (1000-1043) envoient `api_base` dans `LLMClient(...).ping()` → anonymous = pointer le serveur vers une URL interne ; `/api/agent` exclu du CSRF. *Fix : 403 sans session + validation d'`api_base` (bloquer les hôtes privés). Effort : **M**.* -- [ ] **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**.* +- [x] **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**.* +- [x] **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**.* - [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**.* -- [ ] **A22 — Validateur d'upload = code mort** : `validate_upload()` + `ALLOWED_EXTENSIONS` + `MAX_UPLOAD_SIZE` (`security.py:33-40`) n'ont **aucun appelant** ; `upload_local_workspace_file` (`dashboard.py:1507-1574`) fait `file_path.write_bytes(content)` après seulement `Path(filename).name` — ni taille, ni extension, ni auth, et `/api/local-workspace` est exclu du CSRF. Chemin aussi codé en dur `Path(f"/data/uploads/...")` au lieu de `FLOWDECK_DATA_DIR`. *Fix : appeler `validate_upload()` avant `read()` + session. Effort : **S**.* -- [ ] **A23 — N+1 avérés** : `dashboard.py:905` (`COUNT(*) FROM pages` par page dans une boucle de 20), `board.py:2141-2146` (INSERT + `_extract_ai_keywords` par issue), `collections.py:378` (INSERT par propriété en boucle). *Fix : une agrégation `GROUP BY` / un `executemany`. Effort : **S**.* -- [ ] **A24 — 2 routes silencieusement mortes (vérifié en servant l'app)** : `GET /workspace` → `dashboard.py:820` (HTML, inclus 1er, `main.py:158`) écrase `workspace.py:30` **et** `library.py:537` (JSON, inclus après) ; `GET /api/projects` → `projects.py:24` (inclus 161) écrase `api.py:82` (handler Gitea). *Fix : supprimer/renommer les doublons + assertion au boot « pas deux routes même méthode+chemin ». Effort : **XS**.* +- [x] **A22 — Validateur d'upload = code mort** : `validate_upload()` + `ALLOWED_EXTENSIONS` + `MAX_UPLOAD_SIZE` (`security.py:33-40`) n'ont **aucun appelant** ; `upload_local_workspace_file` (`dashboard.py:1507-1574`) fait `file_path.write_bytes(content)` après seulement `Path(filename).name` — ni taille, ni extension, ni auth, et `/api/local-workspace` est exclu du CSRF. Chemin aussi codé en dur `Path(f"/data/uploads/...")` au lieu de `FLOWDECK_DATA_DIR`. *Fix : appeler `validate_upload()` avant `read()` + session. Effort : **S**.* +- [x] **A23 — N+1 avérés** : `dashboard.py:905` (`COUNT(*) FROM pages` par page dans une boucle de 20), `board.py:2141-2146` (INSERT + `_extract_ai_keywords` par issue), `collections.py:378` (INSERT par propriété en boucle). *Fix : une agrégation `GROUP BY` / un `executemany`. Effort : **S**.* +- [x] **A24 — 2 routes silencieusement mortes (vérifié en servant l'app)** : `GET /workspace` → `dashboard.py:820` (HTML, inclus 1er, `main.py:158`) écrase `workspace.py:30` **et** `library.py:537` (JSON, inclus après) ; `GET /api/projects` → `projects.py:24` (inclus 161) écrase `api.py:82` (handler Gitea). *Fix : supprimer/renommer les doublons + assertion au boot « pas deux routes même méthode+chemin ». Effort : **XS**.* - [ ] **A25 — 92 `except Exception: pass` dont 68 dans des handlers async, 0 loggé** (471 `except Exception` au total, 102 dans `api_v2.py` seul) ; pire : `api_v2.py:504-511` avale l'échec de `materialize_properties` **dans la transaction de création de collection** (collection commitée sans schéma), `api_v2.py:859`, et tous les `await _fire_event(...)` (`1750`, `885`, `903`) → pipeline d'événements/webhooks cassé = invisible. *Fix : `logger.exception` sur les 68 sites ; supprimer le try autour des 2 writes de `create_collection_v2`. Effort : **M**.* ### 🟡 P2 — Moyennes @@ -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 · **A11 (traversal avatar) + A18 (vue publique : 404 restricted/private + html.escape)** : `tests/test_audit_p0_fixes.py`, suite 1019/1019, version 7.3.2.** +→ **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 · **A12/A13/A15/A17/A22/A23/A24** : garde SSRF par hop, routers automations + /api sous session/Bearer, webhooks admin+URL publique, uploads validés, N+1 en GROUP BY/executemany, 2 doublons de routes supprimés → suite 1025/1025, version 7.3.3.** diff --git a/VERSION b/VERSION index eab246c..44e98ad 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -7.3.2 +7.3.3 diff --git a/WORKLOAD.md b/WORKLOAD.md index 6b06978..93d0a9b 100644 --- a/WORKLOAD.md +++ b/WORKLOAD.md @@ -1,6 +1,6 @@ # WORKLOAD — FlowDeck Notion Clone -> **Début**: 2026-07-08 | **Version**: v7.3.2 (audit sécurité A1–A11 + A18) | **Statut**: EN COURS 🔄 +> **Début**: 2026-07-08 | **Version**: v7.3.3 (audit sécurité A1–A24) | **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 7c4dea6..351e7ba 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.2", + version="7.3.3", docs_url="/docs", redoc_url="/redoc", lifespan=lifespan, diff --git a/app/routers/api.py b/app/routers/api.py index 9947952..ec80f39 100644 --- a/app/routers/api.py +++ b/app/routers/api.py @@ -5,7 +5,7 @@ import json import logging from datetime import datetime -from fastapi import APIRouter, HTTPException, Query, Request +from fastapi import APIRouter, Depends, HTTPException, Query, Request from fastapi.responses import HTMLResponse from app.auth.session import SessionManager @@ -15,7 +15,27 @@ from app.routers.board import STATUS_COLORS, STATUS_LABELS, _issue_column, _map_ from app.services.gitea_client import gitea logger = logging.getLogger(__name__) -router = APIRouter(tags=["api"], prefix="/api") + +# A17 : ce router legacy ne sert plus un anonymous. Deux exceptions assumées — +# le probe d'infra (Docker HEALTHCHECK / CI) et la remontée d'erreur client. +_API_PUBLIC_PATHS = {"/api/health", "/api/frontend-error"} + + +async def _require_session_or_bearer(request: Request) -> None: + """Session de cookie **ou** Bearer d'API valide sur toutes les routes /api.""" + if request.url.path in _API_PUBLIC_PATHS: + return + if SessionManager.decode_session(request.cookies.get("flowdeck_session", "")): + return + auth = request.headers.get("Authorization", "") + if auth.startswith("Bearer "): + from app.routers.public_api import verify_token + verify_token(auth) + return + raise HTTPException(401, "Authentication required") + + +router = APIRouter(tags=["api"], prefix="/api", dependencies=[Depends(_require_session_or_bearer)]) # ── Simple rate limiter (in-memory, per-IP) ── _rate_limit_store: dict[str, tuple[float, int]] = {} @@ -79,22 +99,6 @@ async def stats(): } -@router.get("/projects") -async def projects(search: str = Query(default=""), show_archived: bool = Query(default=False)): - """List Gitea projects (JSON).""" - try: - repos = await gitea.get_user_repos(page=1, limit=50) - if search: - q = search.lower() - repos = [r for r in repos if q in r.get("full_name", "").lower() or q in (r.get("description") or "").lower()] - if not show_archived: - repos = [r for r in repos if not r.get("archived", False)] - repos.sort(key=lambda r: r.get("updated_at", ""), reverse=True) - except Exception: - repos = [] - return {"projects": repos} - - @router.post("/move") async def move_card( request: Request, diff --git a/app/routers/automations.py b/app/routers/automations.py index 03505e8..b170ac4 100644 --- a/app/routers/automations.py +++ b/app/routers/automations.py @@ -4,7 +4,7 @@ from __future__ import annotations import json import logging -from fastapi import APIRouter, HTTPException, Request +from fastapi import APIRouter, Depends, HTTPException, Request from app.auth.session import SessionManager from app.db import get_conn @@ -17,7 +17,15 @@ from app.services.automations import ( ) logger = logging.getLogger(__name__) -router = APIRouter(tags=["automations"]) + + +def _require_session(request: Request) -> None: + """A13 : toute la route (CRUD, run, press-button) exige une session.""" + if not SessionManager.decode_session(request.cookies.get("flowdeck_session", "")): + raise HTTPException(status_code=401, detail="Authentication required") + + +router = APIRouter(tags=["automations"], dependencies=[Depends(_require_session)]) TRIGGER_TYPES = ("event", "cron", "button") @@ -75,7 +83,7 @@ async def create_automation(request: Request): body = await request.json() if request.headers.get("content-type") else {} _validate_payload(body) user = _current_user(request) - by = user.get("id") or 1 + by = user["id"] with get_conn() as conn: cur = conn.execute( """INSERT INTO automations diff --git a/app/routers/board.py b/app/routers/board.py index 5d11b71..2513563 100644 --- a/app/routers/board.py +++ b/app/routers/board.py @@ -1908,7 +1908,11 @@ async def og_metadata(request: Request): if data: return {"ok": True, **data} from app.services.og_fetcher import fetch_og_metadata - data = await fetch_og_metadata(url) + try: + data = await fetch_og_metadata(url) + except ValueError as exc: + # A12 : SSRF — URL vers un hôte privé/loopback (ou redirection vers l'un). + raise HTTPException(400, str(exc)) from None return {"ok": True, **data} @@ -2141,12 +2145,15 @@ async def sync_project(owner: str, repo: str): if board: board_id = board["id"] columns = json.loads(board["columns_json"]) + # A23 : un seul executemany pour toutes les cards. + conn.executemany( + "INSERT OR REPLACE INTO cards (board_id, gitea_issue_id, column_name) VALUES (?,?,?)", + [ + (board_id, issue["number"], _issue_column(issue, columns, board_id)) + for issue in issues_only + ], + ) for issue in issues_only: - col = _issue_column(issue, columns, board_id) - conn.execute( - "INSERT OR REPLACE INTO cards (board_id, gitea_issue_id, column_name) VALUES (?,?,?)", - (board_id, issue["number"], col), - ) # Extract AI keywords from each issue _extract_ai_keywords(owner, repo, issue.get("labels", []), issue.get("body", "")) conn.commit() diff --git a/app/routers/collections.py b/app/routers/collections.py index 4d25c72..f030942 100644 --- a/app/routers/collections.py +++ b/app/routers/collections.py @@ -373,20 +373,35 @@ async def duplicate_collection_api(request: Request, collection_id: int): "SELECT * FROM collection_properties WHERE collection_id=? ORDER BY position", (collection_id,), ).fetchall() - for p in rows: - ncur = conn.execute( + # A23 : un seul executemany ; les rowid sont contigus (même transaction, + # insertion dans l'ordre de `rows`), donc le mappeur se fait par index. + tuples = [ + (new_id, p["name"], p["prop_type"], p["options_json"], p["number_format"], + None, p["reverse_name"], None, None, p["rollup_function"], + p["formula_expression"], p["position"], p["required"], + p["visible_in_views"]) + for p in rows + ] + if tuples: + ncur = conn.executemany( """INSERT INTO collection_properties (collection_id, name, prop_type, options_json, number_format, related_collection_id, reverse_name, relation_property_id, target_property_id, rollup_function, formula_expression, position, required, visible_in_views) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?)""", - (new_id, p["name"], p["prop_type"], p["options_json"], p["number_format"], - None, p["reverse_name"], None, None, p["rollup_function"], - p["formula_expression"], p["position"], p["required"], - p["visible_in_views"]), + tuples, ) - prop_map[p["id"]] = ncur.lastrowid + new_ids = [ + r["id"] + for r in conn.execute( + "SELECT id FROM collection_properties WHERE collection_id=? ORDER BY id", + (new_id,), + ).fetchall() + ] + assert len(new_ids) == len(tuples), "remap des propriétés : effectif inattendu" + for p, new_pid in zip(rows, new_ids, strict=True): + prop_map[p["id"]] = new_pid # Fix cross-property references after all rows exist (creates may target # columns not inserted yet). Related collection remapped to the copy. diff --git a/app/routers/dashboard.py b/app/routers/dashboard.py index aed0c33..b309542 100644 --- a/app/routers/dashboard.py +++ b/app/routers/dashboard.py @@ -912,9 +912,18 @@ async def list_workspace_projects(request: Request): rows = conn.execute( "SELECT id, title FROM pages WHERE parent_id IS NULL AND collection_row_id IS NULL ORDER BY updated_at DESC LIMIT 20" ).fetchall() + # A23 : une seule agrégation GROUP BY au lieu d'un COUNT par ligne. + counts = {} + if rows: + for c in conn.execute( + "SELECT parent_id, COUNT(*) AS c FROM pages WHERE parent_id IN ({}) GROUP BY parent_id".format( + ",".join("?" * len(rows)) + ), + [r["id"] for r in rows], + ).fetchall(): + counts[c["parent_id"]] = c["c"] for r in rows: - count = conn.execute("SELECT COUNT(*) FROM pages WHERE parent_id=?", (r["id"],)).fetchone()[0] - builtin.append({"id": str(r["id"]), "name": r["title"] or "Untitled", "pageCount": count, "forge": "builtin"}) + builtin.append({"id": str(r["id"]), "name": r["title"] or "Untitled", "pageCount": counts.get(r["id"], 0), "forge": "builtin"}) gitea_repos = [] # Use per-user token if available, otherwise return empty @@ -1526,7 +1535,8 @@ async def upload_local_workspace_file(request: Request): import json from pathlib import Path - ws = _get_active_workspace(request, user_id=_get_user_id(request)) + uid = _require_user_id(request) # A22 : pas d'upload anonyme + ws = _get_active_workspace(request, user_id=uid) ws_id = ws["id"] if ws else None if not ws_id: return JSONResponse({"error": "No active workspace"}, status_code=400) @@ -1543,7 +1553,12 @@ async def upload_local_workspace_file(request: Request): if not files: return JSONResponse({"error": "No files provided"}, status_code=400) - upload_dir = Path(f"/data/uploads/workspace_{ws_id}") + import os + + from app.middleware.security import validate_upload + + data_root = Path(os.environ.get("FLOWDECK_DATA_DIR", "/data")) + upload_dir = data_root / f"uploads/workspace_{ws_id}" upload_dir.mkdir(parents=True, exist_ok=True) results = [] @@ -1564,10 +1579,14 @@ async def upload_local_workspace_file(request: Request): counter += 1 content = await f.read() + err = validate_upload(safe_name, len(content)) # A22 : taille + extension + if err: + results.append({"name": safe_name, "error": err}) + continue file_path.write_bytes(content) # Determine if this is a folder marker or actual file - rel_path = str(file_path.relative_to("/data")) + rel_path = str(file_path.relative_to(data_root)) size = len(content) mime = f.content_type or "application/octet-stream" @@ -1599,7 +1618,8 @@ async def upload_local_workspace_folder(request: Request): import json from pathlib import Path - ws = _get_active_workspace(request, user_id=_get_user_id(request)) + uid = _require_user_id(request) # A22 : pas d'upload anonyme + ws = _get_active_workspace(request, user_id=uid) ws_id = ws["id"] if ws else None if not ws_id: return JSONResponse({"error": "No active workspace"}, status_code=400) @@ -1621,7 +1641,12 @@ async def upload_local_workspace_folder(request: Request): except json.JSONDecodeError: return JSONResponse({"error": "Invalid structure JSON"}, status_code=400) - upload_dir = Path(f"/data/uploads/workspace_{ws_id}") + import os + + from app.middleware.security import validate_upload + + data_root = Path(os.environ.get("FLOWDECK_DATA_DIR", "/data")) + upload_dir = data_root / f"uploads/workspace_{ws_id}" upload_dir.mkdir(parents=True, exist_ok=True) results = [] @@ -1680,9 +1705,13 @@ async def upload_local_workspace_folder(request: Request): counter += 1 content = await matched.read() + err = validate_upload(safe_name, len(content)) # A22 : taille + extension + if err: + results.append({"name": safe_name, "error": err}) + continue file_path.write_bytes(content) - rel_path = str(file_path.relative_to("/data")) + rel_path = str(file_path.relative_to(data_root)) cursor = conn.execute( """INSERT INTO pages (workspace, workspace_id, title, content, content_format, parent_section, parent_id) VALUES ('', ?, ?, ?, 'file', 'Private', ?)""", diff --git a/app/routers/workspace.py b/app/routers/workspace.py index 50f1154..910b95d 100644 --- a/app/routers/workspace.py +++ b/app/routers/workspace.py @@ -26,6 +26,16 @@ def _current_user(request: Request) -> dict: return SessionManager.decode_session(s) or {"login": "admin", "id": 1} +def _require_admin(request: Request) -> dict: + """A15 : webhooks sortants = admin — le serveur POSTe le contenu des pages.""" + user = SessionManager.decode_session(request.cookies.get("flowdeck_session", "")) + if not user or not user.get("id"): + raise HTTPException(401, "Authentication required") + if not user.get("is_admin"): + raise HTTPException(403, "Admin only") + return user + + def _require_ws_admin(request: Request, ws_id: int) -> None: """A5 — CRUD des membres : session obligatoire + rôle admin de l'espace (ou admin global). Un anonymous ne peut plus s'ajouter lui-même ni se @@ -46,13 +56,6 @@ def _require_ws_admin(request: Request, ws_id: int) -> None: # ── Workspaces ── -@router.get("") -async def list_workspaces(request: Request): - with get_conn() as conn: - rows = conn.execute("SELECT * FROM workspaces ORDER BY name").fetchall() - return {"workspaces": [dict(r) for r in rows]} - - @router.post("") async def create_workspace(request: Request): body = await request.json() if request.headers.get("content-type") else {} @@ -688,6 +691,7 @@ async def export_csv(request: Request, collection_id: int): @router.get("/webhooks") async def list_webhooks(request: Request): + _require_admin(request) with get_conn() as conn: rows = conn.execute("SELECT * FROM webhook_subscriptions ORDER BY created_at DESC").fetchall() return {"webhooks": [dict(r) for r in rows]} @@ -695,12 +699,20 @@ async def list_webhooks(request: Request): @router.post("/webhooks") async def create_webhook(request: Request): + _require_admin(request) body = await request.json() if request.headers.get("content-type") else {} url = body.get("url", "").strip() event = body.get("event", "page.created") secret = body.get("secret", "") if not url: raise HTTPException(400, "url required") + # A15 : SSRF — le scheduler POSTe le contenu des pages vers cette URL. + from urllib.parse import urlparse + + from app.services.importers.url_fetch import _is_public_host + parsed = urlparse(url) + if parsed.scheme not in ("http", "https") or not parsed.hostname or not _is_public_host(parsed.hostname): + raise HTTPException(400, f"url non autorisée: {parsed.hostname}") with get_conn() as conn: cur = conn.execute( "INSERT INTO webhook_subscriptions (url, event, secret) VALUES (?,?,?)", @@ -712,6 +724,7 @@ async def create_webhook(request: Request): @router.delete("/webhooks/{wh_id}") async def delete_webhook(request: Request, wh_id: int): + _require_admin(request) with get_conn() as conn: conn.execute("DELETE FROM webhook_subscriptions WHERE id=?", (wh_id,)) conn.commit() diff --git a/app/services/automations.py b/app/services/automations.py index 43f30a9..4cb7c7a 100644 --- a/app/services/automations.py +++ b/app/services/automations.py @@ -168,6 +168,13 @@ async def _run_action(action: dict, context: dict, trigger_source: str) -> str: url = action.get("url", "").strip() if not url: raise ValueError("webhook action requires a url") + # A13 : SSRF — même garde que l'importer URL (loopback/privé refusé). + from urllib.parse import urlparse as _urlparse + + from app.services.importers.url_fetch import _is_public_host + _parsed = _urlparse(url) + if _parsed.scheme not in ("http", "https") or not _parsed.hostname or not _is_public_host(_parsed.hostname): + raise ValueError(f"webhook url non autorisée: {_parsed.hostname!r}") secret = action.get("secret", "") headers = {"Content-Type": "application/json", "X-FlowDeck-Event": context.get("event", "")} if secret: diff --git a/app/services/og_fetcher.py b/app/services/og_fetcher.py index 232dda9..16f99b6 100644 --- a/app/services/og_fetcher.py +++ b/app/services/og_fetcher.py @@ -103,6 +103,34 @@ def parse_og(body: str, url: str) -> dict: } +_MAX_REDIRECTS = 5 + + +async def _get_checked(client, url: str, headers: dict): + """GET avec re-vérification de l'hôte à CHAQUE saut de redirection (A12 SSRF). + + `follow_redirects=True` laisserait une URL publique rediriger vers + 169.254.169.254 / localhost — la garde doit donc tourner à chaque hop. + """ + from app.services.importers.url_fetch import _is_public_host + + current = url + for _ in range(_MAX_REDIRECTS + 1): + parsed = urlparse(current) + if parsed.scheme not in ("http", "https") or not parsed.hostname or not _is_public_host(parsed.hostname): + raise ValueError(f"hôte non autorisé: {parsed.hostname!r}") + r = await client.get(current, headers=headers, follow_redirects=False) + if r.status_code in (301, 302, 303, 307, 308): + loc = r.headers.get("location") + if not loc: + return r + current = urljoin(current, loc) + continue + r.raise_for_status() + return r + raise ValueError("trop de redirections") + + async def fetch_og_metadata(url: str, timeout: float = 6.0, transport=None) -> dict: """Fetch ``url`` and return {url, title, description, image, site_name, favicon}. Empty strings are omitted. Never raises for network errors. @@ -121,12 +149,14 @@ async def fetch_og_metadata(url: str, timeout: float = 6.0, transport=None) -> d "User-Agent": "FlowDeck/5.5 bookmark-fetcher (+https://flowdeck.dracodev.net)", "Accept": "text/html,application/xhtml+xml", } - kwargs = {"follow_redirects": True, "timeout": timeout} + kwargs = {"timeout": timeout} if transport is not None: kwargs["transport"] = transport async with httpx.AsyncClient(**kwargs) as client: - resp = await client.get(src, headers=headers) - resp.raise_for_status() + resp = await _get_checked(client, src, headers) + except ValueError: + # A12 : hôte privé/loopback ou trop de redirections → refus explicite. + raise except Exception as exc: # noqa: BLE001 - network/parse failures are non-fatal logger.debug("og fetch failed for %s: %s", src, exc) base["title"] = urlparse(src).netloc or src diff --git a/tests/test_audit_p0_fixes.py b/tests/test_audit_p0_fixes.py index dfbfc43..100d75d 100644 --- a/tests/test_audit_p0_fixes.py +++ b/tests/test_audit_p0_fixes.py @@ -34,3 +34,66 @@ def test_public_view_hides_restricted_collection(client): r = client.get(f"/workspace/public/{cid}") assert r.status_code == 404 assert "Internal" not in r.text + + +def test_no_duplicate_routes(): + """A24 : deux routes même méthode+chemin → l'une écrase silencieusement l'autre.""" + from app.main import app + + seen = set() + for route in app.routes: + for method in getattr(route, "methods", None) or set(): + if method in ("HEAD", "OPTIONS"): + continue + key = (method, route.path) + assert key not in seen, f"doublon de route: {key}" + seen.add(key) + + +def test_og_metadata_rejects_private_host(client): + """A12 : SSRF — aucun fetch vers loopback/link-local (re-vérif à chaque hop).""" + for url in ("http://127.0.0.1/latest/meta-data/", "http://169.254.169.254/x", "http://localhost/x"): + r = client.post("/board/api/og/metadata", json={"url": url}) + assert r.status_code == 400, (url, r.status_code, r.text[:200]) + + +def test_automations_require_session(client): + """A13 : CRUD, run et press-button refusent un anonymous.""" + anon(client) + assert client.post("/workspace/automations", json={"name": "x"}).status_code == 401 + assert client.post("/workspace/automations/1/run", json={}).status_code == 401 + assert client.post("/api/automations/press-button", json={}).status_code == 401 + assert client.get("/workspace/automations").status_code == 401 + + +def test_outbound_webhook_requires_admin_and_public_url(client): + """A15 : webhooks sortants = admin + URL publique (le scheduler POSTe le contenu).""" + # admin de la fixture : URL privée refusée (SSRF) + r = client.post("/workspace/webhooks", json={"url": "http://127.0.0.1/hook"}) + assert r.status_code == 400 + + anon(client) + assert client.post("/workspace/webhooks", json={"url": "https://example.com/h"}).status_code == 401 + + +def test_legacy_api_requires_auth(client): + """A17 : le router /api legacy refuse un anonymous (health et front-error restent publics).""" + anon(client) + assert client.get("/api/users/me").status_code == 401 + # CSRF valide mais aucune session → la garde du router doit répondre 401. + client.cookies.set("csrf_token", "csrf-anon") + assert client.post("/api/move", json={}, headers={"X-CSRF-Token": "csrf-anon"}).status_code == 401 + assert client.get("/api/health").status_code == 200 + + +def test_upload_requires_session_and_validates_files(client): + """A22 : validate_upload branché (taille + extension) et pas d'upload anonyme.""" + from app.middleware.security import validate_upload + + assert validate_upload("note.txt", 10) is None + assert validate_upload("virus.exe", 10) is not None + assert validate_upload("big.txt", 11 * 1024 * 1024) is not None + + anon(client) + r = client.post("/api/local-workspace/upload", files={"files": ("a.txt", b"x", "text/plain")}) + assert r.status_code == 401 diff --git a/tests/test_v55.py b/tests/test_v55.py index bcfd78b..cc6d59e 100644 --- a/tests/test_v55.py +++ b/tests/test_v55.py @@ -233,7 +233,7 @@ class TestOGParser: return httpx.Response(200, headers={"content-type": "text/html"}, text=_OG_HTML) transport = httpx.MockTransport(handler) - data = asyncio.run(fetch_og_metadata("https://flowdeck.example.com/page", transport=transport)) + data = asyncio.run(fetch_og_metadata("https://example.com/page", transport=transport)) assert data["title"] == "FlowDeck — Notion clone" assert data["site_name"] == "FlowDeck" @@ -255,10 +255,10 @@ class TestOGParser: raise httpx.ConnectError("boom") data = asyncio.run( - fetch_og_metadata("https://unreachable.example.com", transport=httpx.MockTransport(handler)) + fetch_og_metadata("https://example.com/unreachable", transport=httpx.MockTransport(handler)) ) - assert data["url"].startswith("https://unreachable.example.com") - assert data["title"] == "unreachable.example.com" + assert data["url"].startswith("https://example.com/unreachable") + assert data["title"] == "example.com" class TestOGMetadataEndpoint: