feat: revue de code, doc CODE_REVIEW, forfaits 2026, traduction LLM, providers avec modèle
Made-with: Cursor
This commit is contained in:
Sepehr Ramezani
187
docs/CODE_REVIEW.md
Normal file
187
docs/CODE_REVIEW.md
Normal file
@@ -0,0 +1,187 @@
|
||||
# Revue de code complète — Office Translator
|
||||
|
||||
**Date** : Mars 2026
|
||||
**Périmètre** : Backend FastAPI (Python) + Frontend Next.js (TypeScript/React)
|
||||
**Méthode** : Exploration du codebase, recherche bonnes pratiques 2025, audits pip-audit / npm audit, analyse ciblée frontend.
|
||||
|
||||
---
|
||||
|
||||
## 1. Synthèse exécutive
|
||||
|
||||
| Priorité | Thème | Impact |
|
||||
|----------|--------|--------|
|
||||
| **Critique** | Path traversal sur `/download/{filename}` et `/cleanup/{filename}` | Lecture/suppression de fichiers hors répertoire |
|
||||
| **Critique** | Vulnérabilités connues (pip + npm) | RCE, DoS, fuites (Next.js, Starlette, Pillow, etc.) |
|
||||
| **Haute** | Pas de normalisation `session_id` / chemins dans legacy extract/reconstruct | Risque d’accès à des fichiers non prévus |
|
||||
| **Haute** | Token JWT en `localStorage` sans refresh | Expiration = déconnexion brute ; risque XSS |
|
||||
| **Haute** | CORS `*` si `CORS_ORIGINS` vide | En prod, exposition à toutes origines |
|
||||
| **Moyenne** | Config dispersée (`os.getenv` partout) | Erreurs de config, pas de validation au démarrage |
|
||||
| **Moyenne** | Jobs de traduction en mémoire | Pas de partage entre workers (scale horizontal) |
|
||||
| **Moyenne** | Duplication API_BASE / fetch+token côté frontend | Maintenance et incohérences |
|
||||
|
||||
---
|
||||
|
||||
## 2. Architecture (rapport agent)
|
||||
|
||||
- **Backend** : `main.py` → CORS, middlewares (rate limit, security, error, cleanup), router unique `/api/v1` (translate, auth, admin, api-keys, legacy, glossary, prompt).
|
||||
- **Auth** : JWT (access 15 min, refresh 7 j), blocklist JTI (Redis/mémoire), admin par token opaque (Redis/mémoire), API Key (X-API-Key, Pro).
|
||||
- **Traduction** : POST `/translate` → validation fichier/URL → job `tr_*` → `asyncio.to_thread` + excel/word/pptx_translator + provider (OpenRouter, etc.) ; clés/modèles depuis admin settings + env.
|
||||
- **Fichiers** : Upload avec nom unique ; download v1 par `job_id` + ownership ; legacy `/download/{filename}` et `/cleanup/{filename}` **sans normalisation du chemin**.
|
||||
|
||||
---
|
||||
|
||||
## 3. Sécurité
|
||||
|
||||
### 3.1 Path traversal (critique)
|
||||
|
||||
**Fichiers** : `routes/legacy_routes.py`
|
||||
|
||||
- **`GET /api/v1/download/{filename}`** (L.299–311) : `file_path = config.OUTPUT_DIR / filename`. Si `filename = "../../../etc/passwd"`, le chemin peut sortir de `OUTPUT_DIR`.
|
||||
- **`DELETE /api/v1/cleanup/{filename}`** (L.314–325) : même construction.
|
||||
|
||||
**Recommandation** :
|
||||
- Normaliser : `safe_name = Path(filename).name` (pas de `..`).
|
||||
- Résoudre : `resolved = (config.OUTPUT_DIR / safe_name).resolve()`.
|
||||
- Vérifier : `resolved.is_relative_to(config.OUTPUT_DIR.resolve())` (Python 3.9+) ou équivalent.
|
||||
- Sinon 400/404.
|
||||
|
||||
### 3.2 Session / extract-texts / reconstruct-document
|
||||
|
||||
- **`session_id`** (UUID généré côté serveur) : utilisé pour `session_{session_id}.json` et chemins. Vérifier que tout `session_id` reçu en form est bien un UUID strict.
|
||||
- **`session_data["input_path"]`** : s’assurer que le chemin résolu reste sous `UPLOAD_DIR` avant toute lecture.
|
||||
|
||||
### 3.3 Authentification
|
||||
|
||||
- **JWT** : si `JWT_SECRET_KEY` absent, clé éphémère au démarrage → tous les tokens invalidés au redémarrage (log CRITICAL). À corriger en prod.
|
||||
- **Admin** : `ADMIN_PASSWORD` en clair à éviter ; utiliser uniquement `ADMIN_PASSWORD_HASH` (bcrypt) en production.
|
||||
- **Frontend** : token et refresh en `localStorage` → exposés au XSS. Bonnes pratiques 2025 : cookies HTTP-only pour le token (ou au minimum documenter le risque et privilégier des pages à faible exposition XSS).
|
||||
|
||||
### 3.4 CORS
|
||||
|
||||
- **`main.py`** : si `CORS_ORIGINS` vide ou `*`, `allowed_origins = ["*"]` avec warning. En production, définir des origines explicites.
|
||||
|
||||
### 3.5 Stripe webhook
|
||||
|
||||
- **`services/payment_service.py`** : `stripe.Webhook.construct_event(payload, sig_header, STRIPE_WEBHOOK_SECRET)` → **signature bien vérifiée**. Rien à changer côté vérification.
|
||||
|
||||
### 3.6 SSRF / Webhook URL
|
||||
|
||||
- **Translate** : `download_from_url` et validation webhook limitent schéma, DNS et IP privées/loopback. Cohérent avec les bonnes pratiques.
|
||||
|
||||
---
|
||||
|
||||
## 4. Vulnérabilités connues (dépendances)
|
||||
|
||||
### 4.1 Backend (pip-audit)
|
||||
|
||||
| Package | Version | Problème | Correction |
|
||||
|---------|---------|----------|------------|
|
||||
| deep-translator | 1.11.4 | PYSEC-2022-252 | Mettre à jour |
|
||||
| fastapi | 0.109.0 | PYSEC-2024-38 | 0.109.1+ |
|
||||
| pillow | 10.2.0 | CVE-2024-28219 | 10.3.0+ |
|
||||
| python-multipart | 0.0.9 | CVE-2024-53981, CVE-2026-24486 | 0.0.18+ / 0.0.22+ |
|
||||
| starlette | 0.35.1 | CVE-2024-47874, CVE-2025-54121 | 0.40.0+ / 0.47.2+ |
|
||||
|
||||
**Action** : Exécuter `pip-audit` puis mettre à jour les paquets (tests de non-régression après mise à jour).
|
||||
|
||||
### 4.2 Frontend (npm audit)
|
||||
|
||||
| Package | Problème | Action |
|
||||
|---------|----------|--------|
|
||||
| next | RCE (flight), DoS, exposition Server Actions, etc. | `npm audit fix` ou mise à jour ciblée (ex. next@16.1.6) |
|
||||
| minimatch | ReDoS | `npm audit fix` |
|
||||
| ajv | ReDoS ($data) | `npm audit fix` |
|
||||
|
||||
**Action** : `npm audit` puis `npm audit fix` (ou `--force` avec prudence et tests).
|
||||
|
||||
---
|
||||
|
||||
## 5. Fiabilité et bonnes pratiques
|
||||
|
||||
### 5.1 Gestion d’erreurs
|
||||
|
||||
- Middleware global + handlers dédiés (TranslationError, ValidationError, etc.) → correct.
|
||||
- OpenRouter : retry 429 + levée d’exception en cas d’échec (plus de retour silencieux du texte original) → bon.
|
||||
|
||||
### 5.2 Configuration
|
||||
|
||||
- **Config** : tout en `os.getenv()` dans `config.py` et ailleurs. Risque : valeurs manquantes ou incohérentes au démarrage.
|
||||
- **Recommandation** : centraliser dans un module type **Pydantic BaseSettings** (validation, typage, valeurs par défaut) et charger une seule fois au démarrage.
|
||||
|
||||
### 5.3 Jobs de traduction
|
||||
|
||||
- **`_translation_jobs`** en mémoire : en multi-workers (plusieurs processus uvicorn), les jobs ne sont pas partagés.
|
||||
- **Recommandation** : pour un scale horizontal, stocker l’état des jobs dans Redis (ou autre store partagé).
|
||||
|
||||
### 5.4 Legacy
|
||||
|
||||
- **`/translate-batch`**, **`/download/{filename}`**, **`/cleanup/{filename}`**, **`/extract-texts`**, **`/reconstruct-document`** : peu ou pas d’auth, exposition de chemins/fichiers.
|
||||
- **Recommandation** : renforcer l’auth et la validation des paramètres, ou déprécier au profit des endpoints v1 (translate + download par `job_id`).
|
||||
|
||||
---
|
||||
|
||||
## 6. Frontend (rapport agent)
|
||||
|
||||
### 6.1 Types et erreurs
|
||||
|
||||
- **`any`** : `app/(app)/ollama-setup/page.tsx` (models, error), `app/(app)/settings/subscription/page.tsx` (PLAN_ICONS), `app/pricing/page.tsx` (PLAN_ICONS). Remplacer par des types précis.
|
||||
- **Error boundary** : aucun au niveau racine (`app/layout.tsx`). Ajouter un ErrorBoundary pour éviter des écrans blancs.
|
||||
|
||||
### 6.2 Validation et timeout
|
||||
|
||||
- **Login / Admin login** : pas de validation explicite (email, mot de passe) avant submit.
|
||||
- **Translate** : pas de validation centralisée de `file` / `config` (taille, type, champs requis) avant l’appel API.
|
||||
- **Fetch** : plusieurs appels sans timeout (pricing, subscription, admin verify, settings, translate, polling, download). Ajouter `AbortController` + timeout (ex. 8–30 s selon l’endpoint).
|
||||
|
||||
### 6.3 Auth et stockage
|
||||
|
||||
- **Token** : `localStorage` ("token", "refresh_token", "user"). Risque XSS ; en production privilégier cookies HTTP-only si possible.
|
||||
- **Refresh** : `refresh_token` stocké mais **jamais utilisé** ; aucun appel `/auth/refresh`. Quand l’access token expire, l’utilisateur est déconnecté. Implémenter un refresh automatique (intercepteur ou hook) pour améliorer l’UX.
|
||||
|
||||
### 6.4 Duplication et centralisation
|
||||
|
||||
- **API_BASE** : répété dans plusieurs fichiers (useTranslationSubmit, useTranslationConfig, TranslationComplete, settings/services, lib/api). Utiliser une seule source : `@/lib/config` ou `apiClient`.
|
||||
- **Pattern fetch + token** : répété partout. Utiliser systématiquement **apiClient** (déjà en place pour auth, glossaries, api-keys) pour tous les appels authentifiés.
|
||||
- **URL en dur** : `app/(app)/ollama-setup/page.tsx` L.85 `"http://localhost:8000/api/auth/settings"` → remplacer par `API_BASE + "/api/v1/..."` (et vérifier le chemin exact de l’API).
|
||||
|
||||
---
|
||||
|
||||
## 7. Plan d’action priorisé
|
||||
|
||||
### Immédiat (sécurité)
|
||||
|
||||
1. **Path traversal** : normaliser `filename` et vérifier `resolved.is_relative_to(OUTPUT_DIR)` dans `legacy_routes.py` pour `/download/{filename}` et `/cleanup/{filename}`.
|
||||
2. **Dépendances** : mettre à jour FastAPI, Starlette, Pillow, python-multipart, deep-translator ; puis Next.js, minimatch, ajv (avec tests après chaque vague).
|
||||
|
||||
### Court terme (sécurité / robustesse)
|
||||
|
||||
3. **Session / reconstruct** : valider `session_id` (UUID) et s’assurer que tout chemin dérivé reste sous `UPLOAD_DIR`.
|
||||
4. **CORS** : en prod, définir `CORS_ORIGINS` explicitement (pas `*`).
|
||||
5. **Admin** : en prod, n’utiliser que `ADMIN_PASSWORD_HASH` (bcrypt).
|
||||
6. **Frontend** : ajouter un ErrorBoundary racine et des timeouts sur les fetch critiques (translate, download, polling, auth verify).
|
||||
|
||||
### Moyen terme (qualité / maintenabilité)
|
||||
|
||||
7. **Config** : migrer vers Pydantic BaseSettings pour la config backend.
|
||||
8. **Frontend** : centraliser API_BASE et utiliser apiClient partout ; implémenter le refresh token.
|
||||
9. **Jobs** : si scale horizontal prévu, stocker l’état des jobs dans Redis (ou équivalent).
|
||||
10. **Legacy** : protéger ou déprécier les routes legacy (auth + validation des paramètres).
|
||||
|
||||
### Long terme (bonnes pratiques)
|
||||
|
||||
11. **Auth frontend** : étudier le passage à des cookies HTTP-only pour le token (avec adaptation du backend).
|
||||
12. **Validation** : validation explicite login/admin et validation centralisée des paramètres de traduction avant appel API.
|
||||
13. **Types** : supprimer les `any` et renforcer les types (discriminated unions, type predicates) côté TypeScript.
|
||||
|
||||
---
|
||||
|
||||
## 8. Références
|
||||
|
||||
- FastAPI & Next.js security (TurboStarter, Next.js Data Security, Mikul Gohil).
|
||||
- Python code review (DeepSource, Kodus, Medium).
|
||||
- TypeScript security & quality (Krython, Kodus).
|
||||
- Exploration et revue frontend par agents (explore + generalPurpose) sur le dépôt.
|
||||
|
||||
---
|
||||
|
||||
*Document généré dans le cadre d’une revue de code complète. Aucune modification n’a été appliquée automatiquement ; les changements sont à planifier et à tester.*
|
||||
Reference in New Issue
Block a user