1. Image upload size limit — _MAX_IMAGE_BYTES = 10 MB in both serve/server.py and edit/server.py

2. Image MIME type whitelist — _ALLOWED_IMAGE_TYPES blocks SVG XSS in both servers
  3. Filename collision safety — _unique_image_name() helper in both servers
  4. OAuth CSRF — state token generated in edit/server.py auth-url, stored in _oauth_states, validated and discarded in callback; strava_api.auth_url() accepts optional state param
  5. Error message leak — upload processing errors now return generic "Processing failed" instead of exception type/message
  6. Handle injection in subprocess — _trigger_rebuild now asserts handle matches _VALID_HANDLE before passing to subprocess
This commit is contained in:
Davide Scaini
2026-04-10 13:54:50 +02:00
parent 8b7cdd9ed1
commit 6d3673b2f7
3 changed files with 133 additions and 18 deletions
+34 -9
View File
@@ -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()
+6 -4
View File
@@ -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:
+93 -5
View File
@@ -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)