diff --git a/.env.example b/.env.example index 12deace..7b1f286 100644 --- a/.env.example +++ b/.env.example @@ -16,8 +16,17 @@ OBSIGATE_ADMIN_PASSWORD=chab30 # Rate limiting # OBSIGATE_LOGIN_MAX_ATTEMPTS=10 +# OBSIGATE_ACCOUNT_MAX_ATTEMPTS=10 # OBSIGATE_LOGIN_WINDOW_SECONDS=900 +# IP client derrière un reverse proxy (fait confiance à X-Forwarded-For) +# OBSIGATE_TRUST_PROXY=false + +# Webhooks : sécurité SSRF +# OBSIGATE_WEBHOOK_ALLOW_HTTP=false # autoriser http:// (défaut : HTTPS requis) +# OBSIGATE_WEBHOOK_ALLOW_PRIVATE=false # autoriser les IP privées/boucle +# Secret d'un webhook : OBSIGATE_WEBHOOK_SECRET_ + # Watcher # OBSIGATE_WATCHER_ENABLED=true # OBSIGATE_WATCHER_USE_POLLING=false diff --git a/CHANGELOG.md b/CHANGELOG.md index a755228..ecdbc18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,46 @@ et [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Sécurité + +- **#84 Consolidation & sécurité — phase 1 (BUG-021 → BUG-034)** — traitement des + vulnérabilités de la revue statique du 2026-09-13 : + - **BUG-021/022 — XSS stocké** : nouveau sanitizer serveur en liste blanche + (`backend/services/sanitizer.py`, stdlib) appliqué au rendu markdown ; échappement + systématique du `title`, du frontmatter et du JSON de la page publique `/s/{token}` + (`` neutralisé). Tests : `tests/test_security_hardening.py` (9). + - **BUG-023 — brute-force MFA** : rate-limit IP + compte et verrouillage de compte sur + `mfa/totp/verify`, `mfa/recovery` et `mfa/webauthn/verify` (`_enforce_mfa_rate_limit`). + - **BUG-024 — traversal inter-vaults** : `resolve_safe_path` compare désormais les chemins + par **segment** (`Path.relative_to` + repli casse-insensible), plus par préfixe de chaîne : + `vault` ne peut plus lire `vault-evil`. + - **BUG-025 — ReDoS** : validation des regex utilisateur (longueur max, rejet des + quantificateurs imbriqués/backrefs), contenu tronqué et nombre de matchs plafonné + (`backend/services/regex_safety.py`), appliqué à la recherche avancée et au find/replace. + - **BUG-026 — SSRF webhooks** : validation d'URL (HTTPS par défaut, IP privées/boucle + interdites, résolution DNS vérifiée au dispatch, redirections non suivies) et + **externalisation du secret** dans `data/webhook_secrets.json` (0600) ou variable + `OBSIGATE_WEBHOOK_SECRET_` (plus de secret en clair dans `webhooks.json`). + - **BUG-027 — sessions** : rotation du refresh token à chaque usage, révocation du JTI de + l'access token au logout, et vérification de la révocation dans le middleware. + - **BUG-028 — politique de mot de passe** : validation centralisée (8–128 caractères) à la + création, à la modification admin et au changement ; `password_changed_at` invalide tous + les jetons émis avant un changement de mot de passe. + - **BUG-029 — race `users.json`** : verrou `threading.RLock` autour des cycles + lecture-modification-écriture. + - **BUG-030 — audits** : l'adresse IP réelle du client (`X-Forwarded-For` si + `OBSIGATE_TRUST_PROXY=true`) est injectée dans `current_user` et consignée dans les audits. + - **BUG-031 — rate-limiter** : budget **par compte** en plus du budget par IP (rotation d'IP + neutralisée) ; limite mono-process documentée. + - **BUG-032 — indexation** : `_scan_vault` utilise `os.walk(followlinks=False)` et refuse + tout symlink sortant de la racine du vault. + - **BUG-033 — recherche O(N)** : la recherche simple et l'outil IA `search_fulltext` + utilisent l'inverted index (repli sur le scan pendant la construction). + - **BUG-034 — CSP & jetons** : le token d'accès n'est plus persisté dans `sessionStorage` + (mémoire + cookie `HttpOnly`) ; directives CSP durcies (`object-src`, `base-uri`, + `form-action`, `frame-ancestors`). *Reste : migration CSP par nonce (exige la conversion + des gestionnaires d'événements inline).* + ### Ajouté - **#82 Assistant IA — menu `@` instantané** — nouvel endpoint `GET /api/vault/{vault}/paths` diff --git a/README.fr.md b/README.fr.md index ae56f4c..963f7e5 100644 --- a/README.fr.md +++ b/README.fr.md @@ -275,7 +275,11 @@ Un compte **admin** connecté voit une icône 🛡️ dans le header : liste, cr | `OBSIGATE_ACCESS_TOKEN_TTL` | Durée de vie token JWT (secondes) | `3600` | | `OBSIGATE_REFRESH_TOKEN_TTL` | Durée de vie refresh token (secondes) | `2592000` | | `OBSIGATE_LOGIN_MAX_ATTEMPTS` | Tentatives de login max par IP | `10` | +| `OBSIGATE_ACCOUNT_MAX_ATTEMPTS` | Tentatives de login max par compte | `10` | | `OBSIGATE_LOGIN_WINDOW_SECONDS` | Fenêtre de rate limiting (secondes) | `900` | +| `OBSIGATE_TRUST_PROXY` | Faire confiance à `X-Forwarded-For` pour l'IP client (reverse proxy) | `false` | +| `OBSIGATE_WEBHOOK_ALLOW_HTTP` | Autoriser les webhooks non HTTPS | `false` | +| `OBSIGATE_WEBHOOK_ALLOW_PRIVATE` | Autoriser les webhooks vers des adresses privées/boucle | `false` | | `OBSIGATE_PDF_MAX_SIZE_MB` | Taille max des PDF extraits (text indexation) | `50` | | `OBSIGATE_PDF_EXTRACT_TIMEOUT` | Timeout extraction PDF (secondes) | `30` | diff --git a/README.md b/README.md index d774c2c..130837c 100644 --- a/README.md +++ b/README.md @@ -313,7 +313,11 @@ When an **admin** account is logged in, a 🛡️ icon appears in the header. Cl | `OBSIGATE_ACCESS_TOKEN_TTL` | JWT token lifetime (seconds) | `3600` | | `OBSIGATE_REFRESH_TOKEN_TTL` | Refresh token lifetime (seconds) | `2592000` | | `OBSIGATE_LOGIN_MAX_ATTEMPTS` | Max login attempts per IP | `10` | +| `OBSIGATE_ACCOUNT_MAX_ATTEMPTS` | Max login attempts per account | `10` | | `OBSIGATE_LOGIN_WINDOW_SECONDS` | Rate limiting window (seconds) | `900` | +| `OBSIGATE_TRUST_PROXY` | Trust `X-Forwarded-For` for the client IP (reverse proxy) | `false` | +| `OBSIGATE_WEBHOOK_ALLOW_HTTP` | Allow non-HTTPS webhook targets | `false` | +| `OBSIGATE_WEBHOOK_ALLOW_PRIVATE` | Allow webhooks to private/loopback addresses | `false` | | `OBSIGATE_PDF_MAX_SIZE_MB` | Max PDF size for text extraction | `50` | | `OBSIGATE_PDF_EXTRACT_TIMEOUT` | PDF extraction timeout (seconds) | `30` | diff --git a/backend/auth/jwt_handler.py b/backend/auth/jwt_handler.py index 65c152f..6058214 100644 --- a/backend/auth/jwt_handler.py +++ b/backend/auth/jwt_handler.py @@ -62,16 +62,21 @@ def create_access_token(user: dict) -> str: return jwt.encode(payload, get_secret_key(), algorithm=ALGORITHM) -def create_refresh_token(username: str) -> tuple: - """Create a JWT refresh token. Returns (token_string, jti).""" +def create_refresh_token(username: str, remember: bool = False) -> tuple: + """Create a JWT refresh token. Returns (token_string, jti). + + ``remember`` is carried as a claim so token rotation can preserve the + 30-day vs 7-day lifetime chosen at login. + """ now = int(time.time()) jti = str(uuid.uuid4()) payload = { "sub": username, "jti": jti, "iat": now, - "exp": now + REFRESH_TOKEN_EXPIRE_SECONDS, + "exp": now + (2592000 if remember else REFRESH_TOKEN_EXPIRE_SECONDS), "type": "refresh", + "remember": remember, } return jwt.encode(payload, get_secret_key(), algorithm=ALGORITHM), jti diff --git a/backend/auth/middleware.py b/backend/auth/middleware.py index ab6fb4a..57c27f7 100644 --- a/backend/auth/middleware.py +++ b/backend/auth/middleware.py @@ -8,7 +8,9 @@ import os from fastapi import Depends, HTTPException, Request from fastapi.security import HTTPAuthorizationCredentials, HTTPBearer -from .jwt_handler import decode_token +from backend.services.net import get_client_ip + +from .jwt_handler import decode_token, is_token_revoked from .user_store import get_user logger = logging.getLogger("obsigate.auth.middleware") @@ -42,6 +44,7 @@ def get_current_user( "vaults": ["*"], "active": True, "_token_vaults": ["*"], + "_request_ip": get_client_ip(request), } token = None @@ -57,14 +60,31 @@ def get_current_user( if not payload or payload.get("type") != "access": return None + # BUG-027: access tokens revoked at logout must be rejected immediately. + jti = payload.get("jti") + if jti and is_token_revoked(jti): + return None + user = get_user(payload["sub"]) if not user or not user.get("active"): return None + # BUG-028: a password change invalidates every token issued before it. + pca = user.get("password_changed_at") + iat = payload.get("iat") + if pca is not None and iat is not None: + try: + if int(iat) < int(float(pca)): + return None + except (TypeError, ValueError): + return None + # Attach vault permissions from the token (snapshot at login time) user["_token_vaults"] = payload.get("vaults", []) # Attach the token id for per-token rate limiting (AI tool layer). user["_token_jti"] = payload.get("jti") + # BUG-030: expose the real client IP to the audit log. + user["_request_ip"] = get_client_ip(request) return user diff --git a/backend/auth/password.py b/backend/auth/password.py index 76e303b..a2b6b7f 100644 --- a/backend/auth/password.py +++ b/backend/auth/password.py @@ -13,6 +13,32 @@ ph = PasswordHasher( salt_len=16, ) +# Password policy (BUG-028). Applied by the API validators at account creation +# and password change so the rules stay consistent across both paths. +MIN_PASSWORD_LENGTH = 8 +MAX_PASSWORD_LENGTH = 128 + + +def validate_password_strength(password: str) -> str: + """Validate a plaintext password against the project policy. + + Args: + password: Candidate password. + + Returns: + The password unchanged when valid. + + Raises: + ValueError: When the password is too short, too long or blank. + """ + if password is None or len(password) < MIN_PASSWORD_LENGTH: + raise ValueError(f"Minimum {MIN_PASSWORD_LENGTH} caractères") + if len(password) > MAX_PASSWORD_LENGTH: + raise ValueError(f"Maximum {MAX_PASSWORD_LENGTH} caractères") + if not password.strip(): + raise ValueError("Le mot de passe ne peut pas être vide") + return password + def hash_password(password: str) -> str: """Hash a password with Argon2id.""" diff --git a/backend/auth/router.py b/backend/auth/router.py index b6c30ee..8f807c9 100644 --- a/backend/auth/router.py +++ b/backend/auth/router.py @@ -8,9 +8,12 @@ import re from fastapi import APIRouter, Body, Depends, HTTPException, Request, Response from pydantic import BaseModel, validator -from backend.ratelimit import is_rate_limited +from backend.ratelimit import is_account_rate_limited, is_rate_limited +from backend.ratelimit import record_account_failure as rl_record_account_failure +from backend.ratelimit import record_account_success as rl_record_account_success from backend.ratelimit import record_failure as rl_record_failure from backend.ratelimit import record_success as rl_record_success +from backend.services.net import get_client_ip from .jwt_handler import ( ACCESS_TOKEN_EXPIRE_SECONDS, @@ -29,7 +32,7 @@ from .mfa import ( verify_totp, ) from .middleware import is_auth_enabled, require_admin, require_auth -from .password import hash_password, verify_password +from .password import hash_password, validate_password_strength, verify_password from .user_store import ( create_user, delete_user, @@ -61,9 +64,7 @@ class ChangePasswordRequest(BaseModel): @validator("new_password") def password_strength(cls, v): - if len(v) < 8: - raise ValueError("Minimum 8 caractères") - return v + return validate_password_strength(v) class CreateUserRequest(BaseModel): @@ -73,6 +74,10 @@ class CreateUserRequest(BaseModel): role: str = "user" vaults: list[str] = [] + @validator("password") + def password_valid(cls, v): + return validate_password_strength(v) + @validator("username") def username_valid(cls, v): if not re.match(r"^[a-zA-Z0-9_-]{2,32}$", v): @@ -93,6 +98,12 @@ class UpdateUserRequest(BaseModel): password: str | None = None role: str | None = None + @validator("password") + def password_valid(cls, v): + if v is None: + return v + return validate_password_strength(v) + # ── Public endpoints ────────────────────────────────────────────────── @@ -128,16 +139,21 @@ async def login(body: LoginRequest, response: Response, request: Request): raise HTTPException(403, "Compte désactivé") # IP-based rate limiting (10 failures / 15 min per IP) - client_ip = request.client.host if request.client else "unknown" + client_ip = get_client_ip(request) if is_rate_limited(client_ip): raise HTTPException(429, "Trop de tentatives depuis cette adresse IP (15min)") + # BUG-031: per-account budget still applies when the attacker rotates IPs. + if is_account_rate_limited(body.username): + raise HTTPException(429, "Trop de tentatives sur ce compte (15min)") + if is_locked(body.username): raise HTTPException(429, "Compte temporairement verrouillé (15min)") if not verify_password(body.password, user["password_hash"]): attempts = record_login_failure(body.username) rl_attempts, rl_remaining = rl_record_failure(client_ip) + rl_record_account_failure(body.username) remaining = max(0, 5 - attempts) detail = "Identifiants invalides" if 0 < remaining <= 2: @@ -164,9 +180,10 @@ async def login(body: LoginRequest, response: Response, request: Request): def _issue_tokens(user: dict, username: str, remember_me: bool, response: Response) -> dict: """Issue JWT tokens after successful authentication (password or MFA verified).""" record_login_success(username) + rl_record_account_success(username) access_token = create_access_token(user) - refresh_token, refresh_jti = create_refresh_token(username) + refresh_token, refresh_jti = create_refresh_token(username, remember=remember_me) import os max_age = 2592000 if remember_me else 604800 # 30d or 7d @@ -208,6 +225,8 @@ async def refresh_token_endpoint(request: Request, response: Response): """Renew access token via refresh token cookie. Called automatically by the frontend when the access token expires. + The refresh token is rotated on every use (BUG-027) and rejected if it + predates the user's last password change (BUG-028). """ refresh_tok = request.cookies.get("refresh_token") if not refresh_tok: @@ -224,11 +243,38 @@ async def refresh_token_endpoint(request: Request, response: Response): if not user or not user.get("active"): raise HTTPException(401, "Utilisateur introuvable ou inactif") + # BUG-028: reject refresh tokens issued before the last password change. + pca = user.get("password_changed_at") + iat = payload.get("iat") + if pca is not None and iat is not None: + try: + stale = int(iat) < int(float(pca)) + except (TypeError, ValueError): + stale = True + if stale: + raise HTTPException(401, "Session expirée, veuillez vous reconnecter") + + import os + + secure = os.environ.get("OBSIGATE_SECURE_COOKIES", "false").lower() == "true" + remember_me = bool(payload.get("remember", False)) + + # BUG-027: rotate the refresh token — the old one is now single-use. + revoke_token(payload["jti"]) + new_refresh_token, _new_jti = create_refresh_token(user["username"], remember=remember_me) + max_age = 2592000 if remember_me else 604800 + response.set_cookie( + key="refresh_token", + value=new_refresh_token, + max_age=max_age, + httponly=True, + samesite="strict", + secure=secure, + path="/api/auth/refresh", + ) + new_access_token = create_access_token(user) - # Update cookies - import os - secure = os.environ.get("OBSIGATE_SECURE_COOKIES", "false").lower() == "true" response.set_cookie( key="access_token", value=new_access_token, @@ -251,7 +297,7 @@ async def logout( request: Request, response: Response, ): - """Logout: revoke refresh token and delete cookies.""" + """Logout: revoke refresh and access tokens, then delete cookies.""" refresh_tok = request.cookies.get("refresh_token") if refresh_tok: payload = decode_token(refresh_tok) @@ -261,6 +307,21 @@ async def logout( except Exception: pass # token already revoked + # BUG-027: revoke the access token too, otherwise it stays valid until expiry. + access_tok = None + auth_header = request.headers.get("authorization", "") + if auth_header.lower().startswith("bearer "): + access_tok = auth_header[7:].strip() + if not access_tok: + access_tok = request.cookies.get("access_token") + if access_tok: + access_payload = decode_token(access_tok) + if access_payload and access_payload.get("type") == "access": + try: + revoke_token(access_payload["jti"]) + except Exception: + pass + response.delete_cookie("refresh_token", path="/api/auth/refresh") response.delete_cookie("access_token", path="/") response.delete_cookie("access_token", path="/api") # just in case @@ -313,19 +374,51 @@ async def patch_me(req: UpdateMeRequest, current_user=Depends(require_auth)): @router.post("/change-password") async def change_password( req: ChangePasswordRequest, + response: Response, current_user=Depends(require_auth), ): - """Change own password.""" + """Change own password. + + BUG-028: changing the password invalidates all previously issued tokens; + a fresh pair is issued to keep the current session alive. + """ user = get_user(current_user["username"]) assert user is not None, f"User {current_user['username']} not found" if not verify_password(req.current_password, user["password_hash"]): raise HTTPException(400, "Mot de passe actuel incorrect") update_user(current_user["username"], {"password": req.new_password}) - return {"message": "Mot de passe mis à jour"} + updated = get_user(current_user["username"]) + result: dict = {"message": "Mot de passe mis à jour"} + if updated is not None: + result.update(_issue_tokens(updated, updated["username"], False, response)) + return result # ── MFA endpoints ──────────────────────────────────────────────────── +def _enforce_mfa_rate_limit(request: Request, username: str) -> str: + """Reject MFA attempts from a rate-limited IP or on a locked account. + + BUG-023: the second-factor endpoints were previously unprotected, making + the 6-digit TOTP brute-forceable. Returns the resolved client IP. + """ + client_ip = get_client_ip(request) + if is_rate_limited(client_ip): + raise HTTPException(429, "Trop de tentatives depuis cette adresse IP (15min)") + if is_account_rate_limited(username): + raise HTTPException(429, "Trop de tentatives sur ce compte (15min)") + if is_locked(username): + raise HTTPException(429, "Compte temporairement verrouillé (15min)") + return client_ip + + +def _record_mfa_failure(client_ip: str, username: str) -> None: + """Record a failed MFA attempt for the IP, the account and the lockout.""" + record_login_failure(username) + rl_record_failure(client_ip) + rl_record_account_failure(username) + + class MfaVerifyRequest(BaseModel): username: str code: str @@ -601,6 +694,8 @@ async def mfa_webauthn_verify( from .user_store import get_user, update_user from .webauthn_mfa import complete_authentication + client_ip = _enforce_mfa_rate_limit(request, body.username) + user = get_user(body.username) if not user: hash_password("dummy_timing_protection") @@ -616,8 +711,10 @@ async def mfa_webauthn_verify( raise ValueError("Credential non enregistré") new_count = complete_authentication(body.username, body.credential, stored) except ValueError as e: + _record_mfa_failure(client_ip, body.username) raise HTTPException(401, str(e)) except Exception as e: + _record_mfa_failure(client_ip, body.username) logger.warning(f"WebAuthn verification failed for {body.username}: {e}") raise HTTPException(401, "Vérification WebAuthn échouée") @@ -627,7 +724,6 @@ async def mfa_webauthn_verify( c["sign_count"] = new_count update_user(body.username, {"webauthn_credentials": updated}) - client_ip = request.client.host if request.client else "unknown" rl_record_success(client_ip) logger.info(f"User '{body.username}' logged in via WebAuthn") return _issue_tokens(user, body.username, body.remember_me, response) @@ -655,6 +751,8 @@ async def mfa_totp_verify(body: MfaVerifyRequest, response: Response, request: R """ from .user_store import get_user + client_ip = _enforce_mfa_rate_limit(request, body.username) + user = get_user(body.username) if not user: # Timing-safe: simulate work @@ -665,10 +763,10 @@ async def mfa_totp_verify(body: MfaVerifyRequest, response: Response, request: R raise HTTPException(400, "MFA non activé pour cet utilisateur") if not verify_totp(user["mfa_secret"], body.code): + _record_mfa_failure(client_ip, body.username) raise HTTPException(401, "Code TOTP invalide") # Clear IP rate limit on success - client_ip = request.client.host if request.client else "unknown" rl_record_success(client_ip) return _issue_tokens(user, body.username, body.remember_me, response) @@ -682,6 +780,8 @@ async def mfa_recovery_login(body: MfaRecoveryRequest, response: Response, reque """ from .user_store import get_user, update_user + client_ip = _enforce_mfa_rate_limit(request, body.username) + user = get_user(body.username) if not user: hash_password("dummy_timing_protection") @@ -696,6 +796,7 @@ async def mfa_recovery_login(body: MfaRecoveryRequest, response: Response, reque idx = verify_recovery_code(body.recovery_code, hashed_codes) if idx is None: + _record_mfa_failure(client_ip, body.username) raise HTTPException(401, "Code de récupération invalide") # Remove used recovery code (single-use) @@ -703,7 +804,6 @@ async def mfa_recovery_login(body: MfaRecoveryRequest, response: Response, reque update_user(body.username, {"mfa_recovery_codes": hashed_codes}) # Clear IP rate limit - client_ip = request.client.host if request.client else "unknown" rl_record_success(client_ip) logger.info(f"User '{body.username}' logged in via recovery code") diff --git a/backend/auth/user_store.py b/backend/auth/user_store.py index 12c0b94..e8da6ce 100644 --- a/backend/auth/user_store.py +++ b/backend/auth/user_store.py @@ -6,6 +6,7 @@ import json import logging import shutil +import threading import uuid from datetime import datetime, timedelta, timezone from pathlib import Path @@ -16,6 +17,12 @@ logger = logging.getLogger("obsigate.auth.users") USERS_FILE = Path("data/users.json") +# Serialises read-modify-write cycles on users.json. ``RLock`` because a few +# helpers (e.g. ``record_login_failure``) call other mutators while holding it. +# BUG-029: without this, concurrent MFA enable + password change could lose one +# of the two updates (last writer wins). +_users_lock = threading.RLock() + def _read() -> dict: """Read users.json. Returns empty structure if file doesn't exist.""" @@ -75,26 +82,28 @@ def create_user( display_name: str | None = None, ) -> dict: """Create a new user. Raises ValueError if username already taken.""" - data = _read() - if username in data["users"]: - raise ValueError(f"User '{username}' already exists") + with _users_lock: + data = _read() + if username in data["users"]: + raise ValueError(f"User '{username}' already exists") - user = { - "id": str(uuid.uuid4()), - "username": username, - "display_name": display_name or username, - "password_hash": hash_password(password), - "role": role, - "vaults": vaults or [], - "active": True, - "language": "fr", # default UI language - "created_at": datetime.now(timezone.utc).isoformat(), - "last_login": None, - "failed_attempts": 0, - "locked_until": None, - } - data["users"][username] = user - _write(data) + user = { + "id": str(uuid.uuid4()), + "username": username, + "display_name": display_name or username, + "password_hash": hash_password(password), + "role": role, + "vaults": vaults or [], + "active": True, + "language": "fr", # default UI language + "created_at": datetime.now(timezone.utc).isoformat(), + "password_changed_at": datetime.now(timezone.utc).timestamp(), + "last_login": None, + "failed_attempts": 0, + "locked_until": None, + } + data["users"][username] = user + _write(data) logger.info(f"Created user '{username}' (role={role})") return {k: v for k, v in user.items() if k != "password_hash"} @@ -105,28 +114,33 @@ def update_user(username: str, updates: dict) -> dict: Forbidden fields (id, username, created_at) are silently ignored. If 'password' is in updates, it's hashed and stored as password_hash. """ - data = _read() - if username not in data["users"]: - raise ValueError(f"User '{username}' not found") + with _users_lock: + data = _read() + if username not in data["users"]: + raise ValueError(f"User '{username}' not found") - forbidden = {"id", "username", "created_at"} - safe_updates = {k: v for k, v in updates.items() if k not in forbidden} + forbidden = {"id", "username", "created_at"} + safe_updates = {k: v for k, v in updates.items() if k not in forbidden} - if "password" in safe_updates: - safe_updates["password_hash"] = hash_password(safe_updates.pop("password")) + if "password" in safe_updates: + safe_updates["password_hash"] = hash_password(safe_updates.pop("password")) + # BUG-028: invalidate every token issued before this change. + safe_updates["password_changed_at"] = datetime.now(timezone.utc).timestamp() - data["users"][username].update(safe_updates) - _write(data) - return {k: v for k, v in data["users"][username].items() if k != "password_hash"} + data["users"][username].update(safe_updates) + _write(data) + result = {k: v for k, v in data["users"][username].items() if k != "password_hash"} + return result def delete_user(username: str): """Delete a user. Raises ValueError if not found.""" - data = _read() - if username not in data["users"]: - raise ValueError(f"User '{username}' not found") - del data["users"][username] - _write(data) + with _users_lock: + data = _read() + if username not in data["users"]: + raise ValueError(f"User '{username}' not found") + del data["users"][username] + _write(data) logger.info(f"Deleted user '{username}'") @@ -144,24 +158,25 @@ def record_login_failure(username: str) -> int: After 5 failures, locks the account for 15 minutes. """ - data = _read() - user = data["users"].get(username) - if not user: - return 0 + with _users_lock: + data = _read() + user = data["users"].get(username) + if not user: + return 0 - attempts = user.get("failed_attempts", 0) + 1 - updates = {"failed_attempts": attempts} + attempts = user.get("failed_attempts", 0) + 1 + updates = {"failed_attempts": attempts} - # Lock after 5 failed attempts (15 minutes) - if attempts >= 5: - locked_until = ( - datetime.now(timezone.utc) + timedelta(minutes=15) - ).isoformat() - updates["locked_until"] = locked_until - logger.warning(f"Account '{username}' locked after {attempts} failed attempts") + # Lock after 5 failed attempts (15 minutes) + if attempts >= 5: + locked_until = ( + datetime.now(timezone.utc) + timedelta(minutes=15) + ).isoformat() + updates["locked_until"] = locked_until + logger.warning(f"Account '{username}' locked after {attempts} failed attempts") - update_user(username, updates) - return attempts + update_user(username, updates) + return attempts def is_locked(username: str) -> bool: diff --git a/backend/indexer.py b/backend/indexer.py index 4180367..5149369 100644 --- a/backend/indexer.py +++ b/backend/indexer.py @@ -423,94 +423,113 @@ def _scan_vault(vault_name: str, vault_path: str, vault_cfg: dict[str, Any] | No logger.warning(f"Vault path does not exist: {vault_path}") return {"files": [], "tags": {}, "path": vault_path, "paths": []} - for fpath in vault_root.rglob("*"): - # Skip ignored directories - if any(part in IGNORED_DIRS for part in fpath.relative_to(vault_root).parts): - continue + root_resolved = vault_root.resolve(strict=False) - rel_path_str = str(fpath.relative_to(vault_root)).replace("\\", "/") - - # Add all paths (files and directories) to path index - if fpath.is_dir(): + # BUG-032: walk without following symlinks and refuse any symlink that + # escapes the vault root, so external data can never be indexed/exposed. + for dirpath, dirnames, filenames in os.walk(vault_root, followlinks=False): + current_dir = Path(dirpath) + + # Prune ignored and symlinked directories in place (no recursion). + dirnames[:] = [ + d for d in dirnames + if d not in IGNORED_DIRS and not (current_dir / d).is_symlink() + ] + + for d in dirnames: + dpath = current_dir / d + rel_path_str = str(dpath.relative_to(vault_root)).replace("\\", "/") paths.append({ "path": rel_path_str, - "name": fpath.name, + "name": d, "type": "directory" }) - continue - - # Files only from here - if not fpath.is_file(): - continue - ext = fpath.suffix.lower() - # Also match extensionless files named like Dockerfile, Makefile - basename_lower = fpath.name.lower() - if ext not in SUPPORTED_EXTENSIONS and basename_lower not in ("dockerfile", "makefile", "cmakelists.txt"): - continue - - # Add file to path index - paths.append({ - "path": rel_path_str, - "name": fpath.name, - "type": "file" - }) - - try: - relative = fpath.relative_to(vault_root) - stat = fpath.stat() - modified = datetime.fromtimestamp(stat.st_mtime, tz=timezone.utc).isoformat() - # PDF handling — special path (binary, uses pdf_reader) - tags: list[str] = [] - if ext == ".pdf": - from backend.pdf_reader import extract_pdf_metadata, extract_pdf_text - raw = extract_pdf_text(fpath, max_chars=100000) - pdf_meta = extract_pdf_metadata(fpath) - title = pdf_meta.get("title") or fpath.stem.replace("-", " ").replace("_", " ") - content_preview = raw[:200].strip() - elif ext == ".excalidraw" or fpath.name.lower().endswith(".excalidraw.md"): - raw = fpath.read_text(encoding="utf-8", errors="replace") - raw = extract_excalidraw_indexable(raw) - title = fpath.stem.replace(".excalidraw", "").replace("-", " ").replace("_", " ") - content_preview = raw[:200].strip() - else: - raw = fpath.read_text(encoding="utf-8", errors="replace") - title = fpath.stem.replace("-", " ").replace("_", " ") - content_preview = raw[:200].strip() + for fname in filenames: + fpath = current_dir / fname - if ext == ".md": - post = parse_markdown_file(raw) - tags = _extract_tags(post) - inline_tags = _extract_inline_tags(post.content) - tags = list(set(tags) | set(inline_tags)) - title = _extract_title(post, fpath) - content_preview = post.content[:200].strip() + if fpath.is_symlink(): + try: + target = fpath.resolve(strict=True) + except OSError: + continue + try: + target.relative_to(root_resolved) + except ValueError: + logger.warning(f"Skipping symlink outside vault: {fpath}") + continue - _extract_wikilinks_for_backlinks( - vault_name, str(relative).replace("\\", "/"), - title, post.content - ) + rel_path_str = str(fpath.relative_to(vault_root)).replace("\\", "/") - files.append({ - "path": str(relative).replace("\\", "/"), - "title": title, - "tags": tags, - "content_preview": content_preview, - "content": raw[:SEARCH_CONTENT_LIMIT], - "size": stat.st_size, - "modified": modified, - "extension": ext, + ext = fpath.suffix.lower() + # Also match extensionless files named like Dockerfile, Makefile + basename_lower = fpath.name.lower() + if ext not in SUPPORTED_EXTENSIONS and basename_lower not in ("dockerfile", "makefile", "cmakelists.txt"): + continue + + # Add file to path index + paths.append({ + "path": rel_path_str, + "name": fname, + "type": "file" }) - for tag in tags: - tag_counts[tag] = tag_counts.get(tag, 0) + 1 + try: + relative = fpath.relative_to(vault_root) + stat = fpath.stat() + modified = datetime.fromtimestamp(stat.st_mtime, tz=timezone.utc).isoformat() - except PermissionError: - logger.debug(f"Permission denied, skipping {fpath}") - continue - except Exception as e: - logger.error(f"Error indexing {fpath}: {e}") - continue + # PDF handling — special path (binary, uses pdf_reader) + tags: list[str] = [] + if ext == ".pdf": + from backend.pdf_reader import extract_pdf_metadata, extract_pdf_text + raw = extract_pdf_text(fpath, max_chars=100000) + pdf_meta = extract_pdf_metadata(fpath) + title = pdf_meta.get("title") or fpath.stem.replace("-", " ").replace("_", " ") + content_preview = raw[:200].strip() + elif ext == ".excalidraw" or fpath.name.lower().endswith(".excalidraw.md"): + raw = fpath.read_text(encoding="utf-8", errors="replace") + raw = extract_excalidraw_indexable(raw) + title = fpath.stem.replace(".excalidraw", "").replace("-", " ").replace("_", " ") + content_preview = raw[:200].strip() + else: + raw = fpath.read_text(encoding="utf-8", errors="replace") + title = fpath.stem.replace("-", " ").replace("_", " ") + content_preview = raw[:200].strip() + + if ext == ".md": + post = parse_markdown_file(raw) + tags = _extract_tags(post) + inline_tags = _extract_inline_tags(post.content) + tags = list(set(tags) | set(inline_tags)) + title = _extract_title(post, fpath) + content_preview = post.content[:200].strip() + + _extract_wikilinks_for_backlinks( + vault_name, str(relative).replace("\\", "/"), + title, post.content + ) + + files.append({ + "path": str(relative).replace("\\", "/"), + "title": title, + "tags": tags, + "content_preview": content_preview, + "content": raw[:SEARCH_CONTENT_LIMIT], + "size": stat.st_size, + "modified": modified, + "extension": ext, + }) + + for tag in tags: + tag_counts[tag] = tag_counts.get(tag, 0) + 1 + + except PermissionError: + logger.debug(f"Permission denied, skipping {fpath}") + continue + except Exception as e: + logger.error(f"Error indexing {fpath}: {e}") + continue logger.info(f"Vault '{vault_name}': indexed {len(files)} files, {len(paths)} paths, {len(tag_counts)} unique tags") return {"files": files, "tags": tag_counts, "path": vault_path, "paths": paths, "config": {}} diff --git a/backend/main.py b/backend/main.py index dff4258..bf6570a 100644 --- a/backend/main.py +++ b/backend/main.py @@ -136,6 +136,7 @@ from backend.services.mutations import ( restore_backup as service_restore_backup, ) from backend.services.recent import humanize_mtime, list_recent +from backend.services.sanitizer import sanitize_html from backend.services.search import advanced_search_vaults, list_paths, search_paths, search_vaults from backend.services.search import list_tags as service_list_tags from backend.services.vaults import ( @@ -678,7 +679,11 @@ class SecurityHeadersMiddleware(BaseHTTPMiddleware): "connect-src 'self' blob: https://esm.sh https://unpkg.com https://cdnjs.cloudflare.com https://fonts.googleapis.com https://fonts.gstatic.com https://cdn.jsdelivr.net; " "font-src 'self' data: https://fonts.gstatic.com https://esm.sh; " "worker-src 'self' blob:; " - "frame-src 'self' blob:;" + "frame-src 'self' blob:; " + "object-src 'none'; " + "base-uri 'self'; " + "form-action 'self'; " + "frame-ancestors 'self';" ) # Static assets are NOT content-hashed, so they must revalidate: # ``immutable``/long max-age made Cloudflare and mobile browsers serve @@ -1178,7 +1183,10 @@ def _render_markdown(raw_md: str, vault_name: str, current_file_path: Path | Non # Add heading IDs for TOC navigation rendered = _add_heading_ids(rendered) - + + # Sanitize: raw HTML in vault content must never reach the DOM (BUG-021). + rendered = sanitize_html(rendered) + return rendered @@ -2657,14 +2665,15 @@ async def api_advanced_search( Results include ````-highlighted snippets and faceted tag/vault counts. """ loop = asyncio.get_event_loop() - return await loop.run_in_executor( - _search_executor, - partial(advanced_search_vaults, q, vault=vault, tag=tag, + search_fn = partial(advanced_search_vaults, q, vault=vault, tag=tag, limit=limit, offset=offset, sort=sort, case_sensitive=case_sensitive, whole_word=whole_word, regex=regex, include_paths=include_paths, exclude_paths=exclude_paths, - created=created, modified=modified, size=size, semantic=semantic), - ) + created=created, modified=modified, size=size, semantic=semantic) + try: + return await loop.run_in_executor(_search_executor, search_fn) + except ValueError as e: + raise HTTPException(400, str(e)) from e @app.post("/api/search/replace", response_model=ReplaceResponse) @@ -4012,9 +4021,23 @@ async def public_share_view(token: str): title = post.metadata.get("title", file_path.stem) - # JSON-escape raw content for embedding in HTML - import json as _json - raw_json = _json.dumps(raw) + # Escape everything user-controlled before embedding in HTML/JS (BUG-022). + title_esc = html_mod.escape(str(title)) + # Neutralise ```` in the JS string literal too. + title_download_js = ( + _json.dumps(f"{title}.md") + .replace("<", "\\u003c") + .replace(">", "\\u003e") + .replace("&", "\\u0026") + ) + + # JSON-escape raw content for embedding in HTML, and neutralise ````. + raw_json = ( + _json.dumps(raw) + .replace("<", "\\u003c") + .replace(">", "\\u003e") + .replace("&", "\\u0026") + ) fm_html = "" if post.metadata: fm_items = [] @@ -4028,12 +4051,15 @@ async def public_share_view(token: str): v = "✓" if v else "✗" elif v is None: v = "—" - fm_items.append(f'
{k}{v}
') + fm_items.append( + f'
{html_mod.escape(str(k))}' + f'{html_mod.escape(str(v))}
' + ) if fm_items: fm_html = f'
Frontmatter
{"".join(fm_items)}
' return HTMLResponse(f""" -{title} — ObsiGate Share +{title_esc} — ObsiGate Share