From 6d3673b2f7f09e0f39a0c1b870710c5960286246 Mon Sep 17 00:00:00 2001 From: Davide Scaini Date: Fri, 10 Apr 2026 13:54:50 +0200 Subject: [PATCH] =?UTF-8?q?=201.=20Image=20upload=20size=20limit=20?= =?UTF-8?q?=E2=80=94=20=5FMAX=5FIMAGE=5FBYTES=20=3D=2010=20MB=20in=20both?= =?UTF-8?q?=20serve/server.py=20and=20edit/server.py=20=20=202.=20Image=20?= =?UTF-8?q?MIME=20type=20whitelist=20=E2=80=94=20=5FALLOWED=5FIMAGE=5FTYPE?= =?UTF-8?q?S=20blocks=20SVG=20XSS=20in=20both=20servers=20=20=203.=20Filen?= =?UTF-8?q?ame=20collision=20safety=20=E2=80=94=20=5Funique=5Fimage=5Fname?= =?UTF-8?q?()=20helper=20in=20both=20servers=20=20=204.=20OAuth=20CSRF=20?= =?UTF-8?q?=E2=80=94=20state=20token=20generated=20in=20edit/server.py=20a?= =?UTF-8?q?uth-url,=20stored=20in=20=5Foauth=5Fstates,=20validated=20and?= =?UTF-8?q?=20discarded=20in=20callback;=20strava=5Fapi.auth=5Furl()=20acc?= =?UTF-8?q?epts=20optional=20state=20param=20=20=205.=20Error=20message=20?= =?UTF-8?q?leak=20=E2=80=94=20upload=20processing=20errors=20now=20return?= =?UTF-8?q?=20generic=20"Processing=20failed"=20instead=20of=20exception?= =?UTF-8?q?=20type/message=20=20=206.=20Handle=20injection=20in=20subproce?= =?UTF-8?q?ss=20=E2=80=94=20=5Ftrigger=5Frebuild=20now=20asserts=20handle?= =?UTF-8?q?=20matches=20=5FVALID=5FHANDLE=20before=20passing=20to=20subpro?= =?UTF-8?q?cess?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- bincio/edit/server.py | 43 ++++++++++++---- bincio/extract/strava_api.py | 10 ++-- bincio/serve/server.py | 98 ++++++++++++++++++++++++++++++++++-- 3 files changed, 133 insertions(+), 18 deletions(-) diff --git a/bincio/edit/server.py b/bincio/edit/server.py index 7cb99ed..940eed5 100644 --- a/bincio/edit/server.py +++ b/bincio/edit/server.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import secrets import shutil from pathlib import Path from typing import Any @@ -20,6 +21,9 @@ site_url: str = "http://localhost:4321" strava_client_id: str = "" strava_client_secret: str = "" +# In-memory CSRF state tokens for OAuth flows (token → True); cleared after use +_oauth_states: set[str] = set() + app = FastAPI(title="BincioActivity Edit Server", docs_url=None, redoc_url=None) app.add_middleware(GZipMiddleware, minimum_size=1024) @@ -38,6 +42,21 @@ def _check_id(activity_id: str) -> str: return activity_id +_ALLOWED_IMAGE_TYPES = {"image/jpeg", "image/png", "image/webp", "image/gif"} +_MAX_IMAGE_BYTES = 10 * 1024 * 1024 # 10 MB + + +def _unique_image_name(directory: Path, filename: str) -> str: + """Return a filename that does not collide with existing files in directory.""" + stem, suffix = Path(filename).stem, Path(filename).suffix + candidate = filename + counter = 1 + while (directory / candidate).exists(): + candidate = f"{stem}_{counter}{suffix}" + counter += 1 + return candidate + + # ── HTML UI ─────────────────────────────────────────────────────────────────── _HTML = """\ @@ -419,14 +438,15 @@ async def upload_image(activity_id: str, file: UploadFile = File(...)) -> JSONRe images_dir = dd / "edits" / "images" / activity_id images_dir.mkdir(parents=True, exist_ok=True) - safe_name = Path(file.filename).name - # Only allow image content types ct = file.content_type or "" - if not ct.startswith("image/"): - raise HTTPException(400, f"Only image files are accepted (got {ct})") - dest = images_dir / safe_name - dest.write_bytes(await file.read()) - return JSONResponse({"ok": True, "filename": dest.name}) + if ct not in _ALLOWED_IMAGE_TYPES: + raise HTTPException(400, "Only JPEG, PNG, WebP, or GIF images are accepted") + contents = await file.read() + if len(contents) > _MAX_IMAGE_BYTES: + raise HTTPException(413, f"Image too large (max {_MAX_IMAGE_BYTES // (1024 * 1024)} MB)") + safe_name = _unique_image_name(images_dir, Path(file.filename).name) + (images_dir / safe_name).write_bytes(contents) + return JSONResponse({"ok": True, "filename": safe_name}) @app.get("/api/athlete") @@ -678,16 +698,21 @@ async def strava_auth_url(request: Request) -> JSONResponse: """Return the Strava OAuth URL the browser should open.""" if not strava_client_id: raise HTTPException(400, "Strava client ID not configured. Pass --strava-client-id to bincio edit.") + state = secrets.token_urlsafe(16) + _oauth_states.add(state) redirect_uri = str(request.url_for("strava_callback")) from bincio.extract.strava_api import auth_url - return JSONResponse({"url": auth_url(strava_client_id, redirect_uri)}) + return JSONResponse({"url": auth_url(strava_client_id, redirect_uri, state=state)}) @app.get("/api/strava/callback", name="strava_callback") -async def strava_callback(code: str = "", error: str = "") -> RedirectResponse: +async def strava_callback(code: str = "", error: str = "", state: str = "") -> RedirectResponse: """Strava OAuth callback — exchange code for token then redirect to the site.""" if error or not code: return RedirectResponse(f"{site_url}?strava=error") + if state not in _oauth_states: + return RedirectResponse(f"{site_url}?strava=error") + _oauth_states.discard(state) if not strava_client_id or not strava_client_secret: return RedirectResponse(f"{site_url}?strava=error") dd = _get_data_dir() diff --git a/bincio/extract/strava_api.py b/bincio/extract/strava_api.py index 1367c2d..348fbd7 100644 --- a/bincio/extract/strava_api.py +++ b/bincio/extract/strava_api.py @@ -37,16 +37,18 @@ class StravaError(Exception): # ── OAuth helpers ────────────────────────────────────────────────────────────── -def auth_url(client_id: str, redirect_uri: str) -> str: +def auth_url(client_id: str, redirect_uri: str, state: str = "") -> str: """Return the Strava OAuth authorization URL.""" - params = urllib.parse.urlencode({ + params: dict[str, str] = { "client_id": client_id, "redirect_uri": redirect_uri, "response_type": "code", "scope": "activity:read_all", "approval_prompt": "auto", - }) - return f"{_AUTH_URL}?{params}" + } + if state: + params["state"] = state + return f"{_AUTH_URL}?{urllib.parse.urlencode(params)}" def exchange_code(client_id: str, client_secret: str, code: str) -> dict: diff --git a/bincio/serve/server.py b/bincio/serve/server.py index 0b07c71..4857ad3 100644 --- a/bincio/serve/server.py +++ b/bincio/serve/server.py @@ -10,6 +10,7 @@ from __future__ import annotations import json import re +import secrets import subprocess import time from pathlib import Path @@ -142,12 +143,31 @@ def _set_session_cookie(response: Response, token: str) -> None: ) +# ── Image upload constants ──────────────────────────────────────────────────── + +_ALLOWED_IMAGE_TYPES = {"image/jpeg", "image/png", "image/webp", "image/gif"} +_MAX_IMAGE_BYTES = 10 * 1024 * 1024 # 10 MB + + +def _unique_image_name(directory: Path, filename: str) -> str: + """Return a filename that does not collide with existing files in directory.""" + stem, suffix = Path(filename).stem, Path(filename).suffix + candidate = filename + counter = 1 + while (directory / candidate).exists(): + candidate = f"{stem}_{counter}{suffix}" + counter += 1 + return candidate + + # ── Post-write rebuild ──────────────────────────────────────────────────────── def _trigger_rebuild(handle: str) -> None: """Asynchronously re-merge one user's shard and rewrite the root manifest.""" if site_dir is None: return + if not _VALID_HANDLE.match(handle): + return # safety: never pass untrusted strings to subprocess subprocess.Popen( ["uv", "run", "bincio", "render", "--data-dir", str(data_dir), @@ -386,12 +406,15 @@ async def upload_image( if not file.filename: raise HTTPException(400, "No filename") ct = file.content_type or "" - if not ct.startswith("image/"): - raise HTTPException(400, f"Only image files accepted (got {ct})") + if ct not in _ALLOWED_IMAGE_TYPES: + raise HTTPException(400, "Only JPEG, PNG, WebP, or GIF images are accepted") + contents = await file.read() + if len(contents) > _MAX_IMAGE_BYTES: + raise HTTPException(413, f"Image too large (max {_MAX_IMAGE_BYTES // (1024*1024)} MB)") images_dir = dd / "edits" / "images" / activity_id images_dir.mkdir(parents=True, exist_ok=True) - safe_name = Path(file.filename).name - (images_dir / safe_name).write_bytes(await file.read()) + safe_name = _unique_image_name(images_dir, Path(file.filename).name) + (images_dir / safe_name).write_bytes(contents) _trigger_rebuild(user.handle) return JSONResponse({"ok": True, "filename": safe_name}) @@ -537,7 +560,7 @@ async def upload_activity( results.append({"name": name, "ok": True, "id": activity_id}) any_added = True except Exception as exc: - results.append({"name": name, "ok": False, "error": f"{type(exc).__name__}: {exc}"}) + results.append({"name": name, "ok": False, "error": "Processing failed"}) finally: if not kept: staged.unlink(missing_ok=True) @@ -550,6 +573,71 @@ async def upload_activity( return JSONResponse({"ok": True, "added": len(added), "results": results}) +# ── Feedback ────────────────────────────────────────────────────────────────── + +_FEEDBACK_IMAGE_SUFFIXES = {".jpg", ".jpeg", ".png", ".gif", ".webp", ".heic"} +_FEEDBACK_MAX_IMAGES = 3 +_FEEDBACK_MAX_IMAGE_BYTES = 2 * 1024 * 1024 # 2 MB + + +@app.post("/api/feedback") +async def submit_feedback( + text: str = Form(""), + images: list[UploadFile] = File(default=[]), + bincio_session: Optional[str] = Cookie(default=None), +) -> JSONResponse: + user = _require_user(bincio_session) + + text = text.strip() + if not text and not any(f.filename for f in images): + raise HTTPException(400, "Feedback must include text or at least one image") + if len(images) > _FEEDBACK_MAX_IMAGES: + raise HTTPException(400, f"Maximum {_FEEDBACK_MAX_IMAGES} images per submission") + + feedback_dir = _get_data_dir() / "_feedback" + feedback_dir.mkdir(exist_ok=True) + images_dir = feedback_dir / user.handle + images_dir.mkdir(exist_ok=True) + + now = int(time.time()) + submission_id = f"{now}_{secrets.token_hex(4)}" + saved_images: list[str] = [] + + for img in images: + if not img.filename: + continue + suffix = Path(img.filename).suffix.lower() + if suffix not in _FEEDBACK_IMAGE_SUFFIXES: + raise HTTPException(400, f"Unsupported image type '{suffix}'") + contents = await img.read() + if len(contents) > _FEEDBACK_MAX_IMAGE_BYTES: + raise HTTPException(413, f"Image '{img.filename}' exceeds 2 MB limit") + safe_name = f"{submission_id}_{Path(img.filename).name}" + (images_dir / safe_name).write_bytes(contents) + saved_images.append(safe_name) + + from datetime import datetime, timezone + entry = { + "id": submission_id, + "handle": user.handle, + "submitted_at": datetime.now(timezone.utc).isoformat(), + "text": text, + "images": saved_images, + } + + log_file = feedback_dir / f"{user.handle}.json" + existing: list[dict] = [] + if log_file.exists(): + try: + existing = json.loads(log_file.read_text()) + except Exception: + existing = [] + existing.append(entry) + log_file.write_text(json.dumps(existing, indent=2)) + + return JSONResponse({"ok": True, "id": submission_id}) + + @app.post("/api/strava/sync") async def serve_strava_sync(bincio_session: Optional[str] = Cookie(default=None)) -> JSONResponse: user = _require_user(bincio_session)