From 72fcef2ba95602126b4026890eab6bd9ab4cd134 Mon Sep 17 00:00:00 2001 From: Bruno Charest Date: Wed, 30 Sep 2026 23:20:26 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20A16=20=E2=80=94=20ACL=20sur=20l'export?= =?UTF-8?q?=20et=20les=20pi=C3=A8ces=20jointes=20(v7.3.4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `export.py` : `_load_page_or_404(request, page_id)` — 401 sans session, 404 si `PermissionManager.can_view_page` refuse (les 4 formats d'export) - `dashboard.py` : helper `_require_page_view` sur `GET /api/pages/{id}/download` et `GET /api/pages/{id}/file-content` - +1 test de non-régression → suite **1026/1026**, `ruff check app tests` OK --- CHANGELOG.md | 10 ++++++++++ ROADMAP.md | 4 ++-- VERSION | 2 +- WORKLOAD.md | 2 +- app/main.py | 2 +- app/routers/dashboard.py | 18 ++++++++++++++++-- app/routers/export.py | 20 +++++++++++++++----- tests/test_audit_p0_fixes.py | 11 +++++++++++ 8 files changed, 57 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec936e3..e286268 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog - FlowDeck +## v7.3.4 (2026-09-30) — Audit sécurité : A16 (ACL sur lectures) + +### Fixed + +- **A16** — `_load_page_or_404` (4 exports) et les 2 routes pièce jointe + (`/download`, `/file-content`) passent par session + `PermissionManager.can_view_page` + → 401 sans session, 404 hors ACL ; la lecture legacy `board.py` était déjà + couverte par A7 +- Test `test_exports_and_attachments_require_auth` → suite **1026/1026** + ## v7.3.3 (2026-09-30) — Audit sécurité : A12–A24 (SSRF, auth legacy, uploads, perf) ### Fixed diff --git a/ROADMAP.md b/ROADMAP.md index c575015..73b392e 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1137,7 +1137,7 @@ Quality DB views, Agent IA Palette → Realtime + E - [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**.* - [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**.* +- [x] **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**.* - [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**.* @@ -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 · **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.** +→ **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 · **A16** : exports + pièces jointes sous session + `can_view_page` → suite 1026/1026, version 7.3.4.** diff --git a/VERSION b/VERSION index 44e98ad..c968a57 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -7.3.3 +7.3.4 diff --git a/WORKLOAD.md b/WORKLOAD.md index 93d0a9b..4ad2b7d 100644 --- a/WORKLOAD.md +++ b/WORKLOAD.md @@ -1,6 +1,6 @@ # WORKLOAD — FlowDeck Notion Clone -> **Début**: 2026-07-08 | **Version**: v7.3.3 (audit sécurité A1–A24) | **Statut**: EN COURS 🔄 +> **Début**: 2026-07-08 | **Version**: v7.3.4 (audit sécurité A1–A24 + A16) | **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 351e7ba..8753bde 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.3", + version="7.3.4", docs_url="/docs", redoc_url="/redoc", lifespan=lifespan, diff --git a/app/routers/dashboard.py b/app/routers/dashboard.py index b309542..72cc65b 100644 --- a/app/routers/dashboard.py +++ b/app/routers/dashboard.py @@ -1158,9 +1158,21 @@ def _file_page_disk_path(page: dict): return (full, filename, mime, size) +def _require_page_view(request: Request, page_id: int) -> None: + """A16 : lecture d'une pièce jointe = session + `can_view_page` (404 sinon).""" + user = SessionManager.decode_session(request.cookies.get("flowdeck_session", "")) + if not user or not user.get("id"): + raise HTTPException(401, "Authentication required") + from app.services.permission_manager import PermissionManager + + if not PermissionManager(user["id"], bool(user.get("is_admin"))).can_view_page(page_id): + raise HTTPException(404, "Page not found") + + @router.get("/api/pages/{page_id}/download") -async def download_page_file(page_id: int): +async def download_page_file(request: Request, page_id: int): """Download the original uploaded file of a ``file`` page (attachment).""" + _require_page_view(request, page_id) with get_conn() as conn: row = conn.execute( "SELECT id, title, content, content_format FROM pages " @@ -1181,13 +1193,15 @@ async def download_page_file(page_id: int): @router.get("/api/pages/{page_id}/file-content") -async def page_file_content(page_id: int): +async def page_file_content(request: Request, page_id: int): """Return the textual content of a ``file`` page (for copy to clipboard). Binary files (PDF, images…) answer ``{ok: false}`` — the clipboard copy is only meaningful for plain-text / code / markdown files. """ from app.services.export import _file_text + + _require_page_view(request, page_id) with get_conn() as conn: row = conn.execute( "SELECT id, title, content, content_format FROM pages " diff --git a/app/routers/export.py b/app/routers/export.py index dd23ee8..0833639 100644 --- a/app/routers/export.py +++ b/app/routers/export.py @@ -23,7 +23,15 @@ logger = logging.getLogger(__name__) router = APIRouter(tags=["export"], prefix="/api/export") -def _load_page_or_404(page_id: int) -> dict: +def _load_page_or_404(request: Request, page_id: int) -> dict: + """A16 : session obligatoire + `PermissionManager.can_view_page` — l'export ne + doit pas délivrer le contenu d'une page énumérable par id.""" + from app.auth.session import SessionManager + from app.services.permission_manager import PermissionManager + + user = SessionManager.decode_session(request.cookies.get("flowdeck_session", "")) + if not user or not user.get("id"): + raise HTTPException(status_code=401, detail="Authentication required") with get_conn() as conn: row = conn.execute( "SELECT * FROM pages WHERE id=? AND deleted_at IS NULL", @@ -31,6 +39,8 @@ def _load_page_or_404(page_id: int) -> dict: ).fetchone() if not row: raise HTTPException(status_code=404, detail="Page not found") + if not PermissionManager(user["id"], bool(user.get("is_admin"))).can_view_page(page_id): + raise HTTPException(status_code=404, detail="Page not found") return dict(row) @@ -52,7 +62,7 @@ def _safe_filename(page: dict, ext: str) -> str: @router.get("/markdown/{page_id}") async def export_markdown(page_id: int, request: Request): - page = _load_page_or_404(page_id) + page = _load_page_or_404(request, page_id) md = page_to_markdown(page) filename = _safe_filename(page, "md") headers = _download_header(filename, "text/markdown") @@ -61,7 +71,7 @@ async def export_markdown(page_id: int, request: Request): @router.get("/html/{page_id}") async def export_html(page_id: int, request: Request): - page = _load_page_or_404(page_id) + page = _load_page_or_404(request, page_id) html = page_to_standalone_html(page) filename = _safe_filename(page, "html") headers = _download_header(filename, "text/html") @@ -70,7 +80,7 @@ async def export_html(page_id: int, request: Request): @router.get("/pdf/{page_id}") async def export_pdf(page_id: int, request: Request): - page = _load_page_or_404(page_id) + page = _load_page_or_404(request, page_id) try: pdf_bytes = page_to_pdf_bytes(page) except ImportError: @@ -85,7 +95,7 @@ async def export_pdf(page_id: int, request: Request): @router.get("/site/{page_id}") async def export_site(page_id: int, request: Request): - page = _load_page_or_404(page_id) + page = _load_page_or_404(request, page_id) site_bytes = build_static_site_bytes(page) title = _safe_filename(page, "site").replace(".site", "") or "flowdeck-site" filename = f"{title}_site.zip" diff --git a/tests/test_audit_p0_fixes.py b/tests/test_audit_p0_fixes.py index 100d75d..2d0ddef 100644 --- a/tests/test_audit_p0_fixes.py +++ b/tests/test_audit_p0_fixes.py @@ -97,3 +97,14 @@ def test_upload_requires_session_and_validates_files(client): anon(client) r = client.post("/api/local-workspace/upload", files={"files": ("a.txt", b"x", "text/plain")}) assert r.status_code == 401 + + +def test_exports_and_attachments_require_auth(client): + """A16 : export + pièces jointes = session et `can_view_page` (jamais le contenu).""" + pid = client.post("/board/api/pages?title=Secret§ion=Private").json()["id"] + + anon(client) + assert client.get(f"/api/export/markdown/{pid}").status_code == 401 + assert client.get(f"/api/export/html/{pid}").status_code == 401 + assert client.get(f"/api/pages/{pid}/download").status_code == 401 + assert client.get(f"/api/pages/{pid}/file-content").status_code == 401