veza/docs/archive/root-md/AUDIT_STABILITY.md
senke 0fb75f759b chore(audit 2.2, 2.3): nettoyer .md et .json à la racine
- Archiver 131 .md dans docs/archive/root-md/
- Archiver 22 .json dans docs/archive/root-json/
- Conserver 7 .md utiles (README, CONTRIBUTING, CHANGELOG, etc.)
- Conserver package.json, package-lock.json, turbo.json
- Ajouter README d'index dans chaque archive
2026-02-15 14:35:08 +01:00

757 lines
22 KiB
Markdown

# 🔍 AUDIT DE STABILITÉ — PROJET VEZA
**Date** : 2025-01-27
**Objectif** : Identifier toutes les faiblesses potentielles dans la robustesse, cohérence, performances et résilience du système
**Phase** : Zero-Bug / Launch-Ready
---
## 📋 TABLE DES MATIÈRES
1. [Backend Go](#1-backend-go)
2. [Chat Server (Rust)](#2-chat-server-rust)
3. [Stream Server (Rust)](#3-stream-server-rust)
4. [Global Project](#4-global-project)
5. [Résumé des Risques](#5-résumé-des-risques)
---
## 1. BACKEND GO
### 1.1 Handlers HTTP
#### ✅ **P0 - Erreurs JSON non traitées silencieusement** — **RÉSOLU**
**Localisation** : `internal/handlers/common.go:280-287`
**Status** : ✅ **RÉSOLU** — Phase 4 JSON Hardening complétée
**Solution implémentée** :
- Création de `BindAndValidateJSON` dans `CommonHandler` avec :
- Vérification de la taille du body (10MB max)
- Gestion robuste des erreurs JSON (syntaxe, type, body vide, etc.)
- Validation automatique avec le validator centralisé
- Retour d'`AppError` au lieu d'erreurs génériques
- Tous les handlers dans `internal/handlers/` refactorisés pour utiliser `BindAndValidateJSON` + `RespondWithAppError`
- Handlers critiques refactorisés : auth, social, marketplace, playlists, profile, comment, role, analytics, bitrate, settings, room, webhook, config_reload, password_reset
**Impact** : Plus aucune erreur JSON ne passe silencieusement. Toutes les erreurs de parsing/validation sont renvoyées avec un format unifié et des codes HTTP appropriés.
**Note** : Il reste ~26 occurrences dans `internal/api/` (handlers dans des packages différents utilisant des patterns différents). À refactoriser dans une phase ultérieure si nécessaire.
---
#### ⚠️ **P1 - Erreurs silencieuses dans les handlers**
**Localisation** : `internal/handlers/auth.go`, `internal/handlers/social.go`
**Problème** : Certains handlers retournent des erreurs génériques sans contexte suffisant. Exemple :
```go
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "internal server error"})
return
}
```
**Impact** : Difficile de diagnostiquer les problèmes en production.
**Recommandation** : Utiliser systématiquement `RespondWithAppError` avec contexte enrichi.
---
#### ⚠️ **P1 - Validation d'input incomplète**
**Localisation** : Tous les handlers
**Problème** : Certains handlers n'utilisent pas `ValidateRequest` avant de traiter les données.
**Impact** : Risque d'injection SQL, XSS, ou corruption de données.
**Recommandation** : Middleware de validation automatique pour toutes les routes POST/PUT.
---
### 1.2 Base de données
#### ❌ **P0 - Absence de transactions dans certaines opérations critiques**
**Localisation** : `internal/core/marketplace/service.go:134-136`
**Problème** : `CreateOrder` utilise une transaction, mais d'autres opérations multi-étapes non :
```go
// Exemple problématique (si non transactionnel)
func (s *Service) UpdateUserProfile(ctx context.Context, userID uuid.UUID, profile *Profile) error {
// Étape 1: Mise à jour user
s.db.Update(&user)
// Étape 2: Mise à jour profile
s.db.Update(&profile)
// Si étape 2 échoue, étape 1 reste appliquée → INCOHÉRENCE
}
```
**Impact** : Incohérence DB en cas d'erreur partielle.
**Recommandation** : Audit complet des opérations multi-étapes, wrapper dans transactions.
---
#### ⚠️ **P1 - Erreurs DB non wrap**
**Localisation** : Plusieurs services
**Problème** : Certaines erreurs DB sont retournées directement sans contexte :
```go
if err := s.db.First(&user, "id = ?", id).Error; err != nil {
return nil, err // Pas de contexte
}
```
**Impact** : Debugging difficile, pas de traçabilité.
**Recommandation** : Toujours wrapper avec `fmt.Errorf("failed to find user %s: %w", id, err)`.
---
#### ⚠️ **P1 - Pas de retry automatique pour les erreurs transitoires**
**Localisation** : Tous les appels DB
**Problème** : Pas de retry automatique pour `database/sql` errors (timeouts, connection pool exhausted).
**Impact** : Échecs temporaires non récupérés automatiquement.
**Recommandation** : Wrapper DB avec retry logic (exponential backoff) pour erreurs transitoires.
---
### 1.3 Workers
#### ⚠️ **P1 - Race condition potentielle lors des retries**
**Localisation** : `internal/workers/job_worker.go:127-135`
```go
if job.Retries < w.maxRetries {
job.Retries++
delay := time.Duration(job.Retries) * 5 * time.Second
time.Sleep(delay) // ⚠️ Bloque le worker
w.Enqueue(job) // ⚠️ Pas de lock sur job
}
```
**Problème** : Si plusieurs workers tentent de retry le même job simultanément, `Retries` peut être incrémenté plusieurs fois.
**Impact** : Jobs retry plus que `maxRetries`, ou jobs dupliqués dans la queue.
**Recommandation** : Utiliser un mutex ou atomic operations pour `job.Retries`, ou marquer le job comme "retrying" en DB avant ré-enqueue.
---
#### ⚠️ **P1 - Pas de timeout explicite pour les jobs**
**Localisation** : `internal/workers/job_worker.go:116`
```go
jobCtx, cancel := context.WithTimeout(ctx, 5*time.Minute)
defer cancel()
```
**Problème** : Timeout hardcodé, pas configurable. Si un job prend plus de 5 minutes, il est annulé brutalement.
**Impact** : Jobs longs (ex: transcodage) peuvent être interrompus.
**Recommandation** : Timeout configurable par type de job.
---
#### ⚠️ **P2 - Queue in-memory sans persistance**
**Localisation** : `internal/workers/job_worker.go`
**Problème** : La queue est en mémoire (`chan Job`). Si le serveur crash, les jobs en attente sont perdus.
**Impact** : Perte de jobs non traités lors d'un crash.
**Recommandation** : Utiliser une queue persistante (Redis, RabbitMQ) pour les jobs critiques.
---
### 1.4 Password Reset
#### ✅ **Bien protégé contre l'énumération**
**Localisation** : `internal/core/auth/service.go:372-379`
```go
if err == gorm.ErrRecordNotFound {
return nil // Toujours retourner succès
}
```
**Status** : ✅ Implémentation correcte — toujours retourner succès même si email n'existe pas.
---
#### ⚠️ **P1 - Timing attack potentiel**
**Localisation** : `internal/services/password_reset_service.go:70-125`
**Problème** : Le temps de traitement peut différer entre :
- Email existe → Génération token + Hash + DB write
- Email n'existe pas → Simple DB query
**Impact** : Attaquant peut détecter si un email existe via timing.
**Recommandation** : Ajouter un délai artificiel pour égaliser les temps de réponse.
---
### 1.5 Health Check
#### ✅ **Robuste si DB en panne**
**Localisation** : `internal/handlers/health.go:70-77`, `internal/handlers/status_handler.go`
**Status** : ✅ `/health` est stateless (toujours OK). `/status` gère correctement les erreurs DB et retourne `degraded`.
---
#### ⚠️ **P2 - Pas de circuit breaker**
**Localisation** : Health checks
**Problème** : Si DB est down, chaque health check tente une connexion (timeout 5s). Pas de circuit breaker pour éviter de surcharger DB.
**Impact** : Si DB est down, health checks continuent à tenter des connexions.
**Recommandation** : Implémenter un circuit breaker pour les dépendances externes.
---
## 2. CHAT SERVER (RUST)
### 2.1 Race Conditions
#### ❌ **P0 - Race condition dans TypingIndicatorManager**
**Localisation** : `src/typing_indicator.rs:34-48`
```rust
pub async fn user_started_typing(&self, user_id: Uuid, conversation_id: Uuid) {
let mut typing = self.typing_users.write().await;
let conversation_typing = typing
.entry(conversation_id)
.or_insert_with(HashMap::new);
conversation_typing.insert(user_id, Utc::now());
}
```
**Problème** : Le `RwLock` protège la HashMap, mais si deux utilisateurs tapent simultanément dans la même conversation, l'ordre d'insertion peut varier.
**Impact** : Timestamps peuvent être inversés, causant des broadcasts dans le mauvais ordre.
**Recommandation** : Utiliser un `Mutex` au lieu de `RwLock` pour garantir l'ordre, ou utiliser un canal sérialisé.
---
#### ⚠️ **P1 - Race condition dans DeliveredStatusManager**
**Localisation** : `src/delivered_status.rs`
**Problème** : Si plusieurs messages sont marqués comme "delivered" simultanément, les updates DB peuvent se chevaucher.
**Impact** : Statuts de livraison incohérents.
**Recommandation** : Utiliser une queue sérialisée pour les updates de statut.
---
#### ⚠️ **P1 - Race condition dans ReadReceiptManager**
**Localisation** : `src/read_receipts.rs`
**Problème** : Même problème que DeliveredStatusManager.
**Recommandation** : Queue sérialisée ou transaction DB.
---
### 2.2 Panics Potentiels
#### ❌ **P0 - Panics dans WebSocket handler**
**Localisation** : `src/websocket/handler.rs:175-176`
```rust
let incoming: IncomingMessage = serde_json::from_str(text)
.map_err(|e| ChatError::serialization_error("IncomingMessage", text, e))?;
```
**Status** : ✅ Bien géré — erreur retournée, pas de panic.
---
#### ⚠️ **P1 - `.unwrap()` dans plusieurs fichiers**
**Localisation** : 31 fichiers identifiés avec `unwrap()` ou `expect()`
**Exemples** :
- `src/config.rs` : `unwrap()` sur variables d'environnement
- `src/database/pool.rs` : `unwrap()` sur connexions DB
- `src/jwt_manager.rs` : `expect()` sur parsing JWT
**Impact** : Panics possibles si données inattendues.
**Recommandation** : Remplacer tous les `unwrap()` par `?` ou gestion d'erreur explicite.
---
#### ⚠️ **P1 - Pas de panic boundary dans handle_socket**
**Localisation** : `src/websocket/handler.rs:77-163`
**Problème** : Si une panic survient dans `handle_incoming_message`, elle peut faire crasher toute la task Tokio.
**Impact** : Un client malveillant peut faire crasher le serveur.
**Recommandation** : Wrapper `handle_incoming_message` dans `std::panic::catch_unwind` ou utiliser `tokio::spawn` avec supervision.
---
### 2.3 Gestion des Tasks
#### ⚠️ **P1 - Tasks orphelins possibles**
**Localisation** : `src/typing_indicator.rs` (task de monitoring)
**Problème** : La task de monitoring des timeouts est spawnée au démarrage mais n'a pas de mécanisme de shutdown propre.
**Impact** : Task continue à tourner même après arrêt du serveur.
**Recommandation** : Utiliser un `CancellationToken` pour arrêter proprement les tasks.
---
#### ⚠️ **P1 - Pas de timeout explicite pour les opérations DB**
**Localisation** : Tous les appels DB
**Problème** : Pas de timeout sur les queries SQLx. Si DB est lente, les requêtes peuvent bloquer indéfiniment.
**Impact** : Deadlock ou timeout très long.
**Recommandation** : Ajouter des timeouts sur tous les appels DB (via `sqlx::query().fetch_timeout()`).
---
### 2.4 Robustesse WebSocket
#### ✅ **Bien géré — déconnexions propres**
**Localisation** : `src/websocket/handler.rs:134-137`
```rust
Ok(Message::Close(_)) => {
info!("👋 Connexion WebSocket fermée par le client");
break;
}
```
**Status** : ✅ Déconnexions gérées proprement.
---
#### ⚠️ **P1 - Pas de heartbeat timeout**
**Localisation** : `src/websocket/handler.rs`
**Problème** : Pas de mécanisme pour détecter les connexions "zombies" (client déconnecté mais serveur ne le sait pas).
**Impact** : Connexions mortes occupent des ressources.
**Recommandation** : Implémenter un heartbeat (ping/pong) avec timeout.
---
### 2.5 Permissions
#### ✅ **Bien implémenté — PermissionService**
**Localisation** : `src/security/permission.rs`
**Status** : ✅ Vérifications de permissions présentes avant chaque action.
---
#### ⚠️ **P1 - Risque de bypass si PermissionService échoue**
**Localisation** : `src/websocket/handler.rs:194-200`
```rust
state
.permission_service
.can_send_message(sender_uuid, conversation_id)
.await
.map_err(|e| {
warn!(...);
// ⚠️ Que se passe-t-il si l'erreur est ignorée ?
})?;
```
**Problème** : Si `can_send_message` retourne une erreur, elle est loggée mais le handler peut continuer selon l'implémentation.
**Impact** : Bypass de permissions si erreur DB.
**Recommandation** : Toujours refuser l'action si permission check échoue (fail-secure).
---
## 3. STREAM SERVER (RUST)
### 3.1 StreamProcessor
#### ❌ **P0 - Tasks non cancellées proprement en cas d'erreur**
**Localisation** : `src/core/processing/processor.rs:168-169`
```rust
monitor_handle.abort();
event_handle.abort();
```
**Problème** : `abort()` tue brutalement les tasks. Si elles étaient en train d'écrire en DB, la transaction peut rester ouverte.
**Impact** : Handles orphelins, transactions DB non commitées.
**Recommandation** : Utiliser `CancellationToken` pour arrêter proprement, attendre la fin des tasks avant `abort()`.
---
#### ⚠️ **P1 - Erreurs FFmpeg non propagées correctement**
**Localisation** : `src/core/processing/processor.rs:154-156`
```rust
FFmpegEvent::Error(msg) => {
tracing::warn!("⚠️ Erreur FFmpeg détectée: {}", msg);
}
```
**Problème** : Les erreurs FFmpeg sont loggées mais ne causent pas l'arrêt du traitement. Le job continue même si FFmpeg a une erreur fatale.
**Impact** : Jobs peuvent se terminer en "succès" alors que FFmpeg a échoué.
**Recommandation** : Détecter les erreurs fatales FFmpeg et arrêter le traitement immédiatement.
---
#### ⚠️ **P1 - DB pas toujours sync en cas de crash**
**Localisation** : `src/core/processing/processor.rs:238-243`
```rust
async fn finalize(&self, tracker: Arc<SegmentTracker>) -> Result<(), AppError> {
tracker.persist_all().await?;
// ...
}
```
**Problème** : Si le serveur crash avant `finalize()`, les segments détectés mais non persistés sont perdus.
**Impact** : Incohérence entre fichiers segments et DB.
**Recommandation** : Persister immédiatement chaque segment (déjà fait dans `SegmentTracker::register`), mais vérifier que c'est bien transactionnel.
---
### 3.2 SegmentTracker
#### ⚠️ **P1 - Corruption d'état concurrent possible**
**Localisation** : `src/core/processing/segment_tracker.rs:59-78`
```rust
pub async fn register(&self, segment: SegmentInfo) -> Result<(), AppError> {
{
let mut segments = self.segments.write().await;
segments.push(segment.clone());
}
self.persist_segment(&segment).await?;
}
```
**Problème** : Si deux segments sont enregistrés simultanément, l'ordre d'insertion dans le vecteur peut varier, mais la persistance DB se fait séquentiellement.
**Impact** : Segments peuvent être persistés dans le mauvais ordre.
**Recommandation** : Utiliser un canal sérialisé pour les registrations, ou un mutex global.
---
### 3.3 FFmpegMonitor
#### ⚠️ **P1 - Regex non robustes**
**Localisation** : `src/core/processing/ffmpeg_monitor.rs:22-24`
```rust
static ref OPENING_SEGMENT_REGEX: Regex = Regex::new(
r"Opening '([^']+)' for writing"
).unwrap();
```
**Problème** : Si FFmpeg change son format de log, la regex ne matchera plus. Pas de fallback.
**Impact** : Segments non détectés, job échoue silencieusement.
**Recommandation** : Ajouter un fallback : détecter les segments depuis le répertoire de sortie si regex échoue.
---
#### ⚠️ **P1 - Gestion des IO errors incomplète**
**Localisation** : `src/core/processing/ffmpeg_monitor.rs:90-94`
```rust
while let Ok(Some(line)) = lines.next_line().await {
self.process_line(&line).await?;
}
```
**Problème** : Si `next_line()` retourne une erreur (ex: stderr fermé), la boucle s'arrête silencieusement.
**Impact** : Monitoring s'arrête sans notification, job continue mais plus de tracking.
**Recommandation** : Logger l'erreur et propager pour arrêter le job.
---
### 3.4 API HLS
#### ✅ **Path traversal protégé**
**Localisation** : `src/routes/encoding.rs:128-133`, `internal/services/hls_service.go:137-151`
**Status** : ✅ Vérification du chemin absolu avec `HasPrefix` pour éviter path traversal.
---
#### ⚠️ **P1 - Erreurs HTTP silencieuses**
**Localisation** : `src/routes/encoding.rs:144-148`
```rust
if !segment_path.exists() {
return Err(AppError::NotFound { ... });
}
```
**Problème** : Si le fichier existe mais n'est pas lisible (permissions), l'erreur sera générique.
**Impact** : Debugging difficile.
**Recommandation** : Différencier "not found" vs "permission denied" vs "IO error".
---
## 4. GLOBAL PROJECT
### 4.1 Cohérence Inter-Services
#### ❌ **P0 - Pas de transaction distribuée**
**Localisation** : Tous les services
**Problème** : Si un message est créé dans le chat server mais que le backend Go échoue à créer une notification, les deux DB sont incohérentes.
**Impact** : Données incohérentes entre services.
**Recommandation** : Implémenter un pattern Saga ou Event Sourcing pour garantir la cohérence.
---
#### ⚠️ **P1 - Pas de validation croisée des IDs**
**Localisation** : Communication inter-services
**Problème** : Le chat server accepte des `conversation_id` sans vérifier qu'ils existent dans le backend Go.
**Impact** : Messages peuvent être créés pour des conversations inexistantes.
**Recommandation** : Validation croisée via API ou cache partagé.
---
### 4.2 Tests
#### ❌ **P0 - Manque de tests unitaires critiques**
**Localisation** : Tous les services
**Problème** : Beaucoup de tests sont `#[ignore]` car nécessitent une DB de test.
**Impact** : Pas de validation automatique des corrections.
**Recommandation** : Utiliser des mocks (ex: `sqlx::test`) ou des containers Docker pour les tests.
---
#### ⚠️ **P1 - Pas de tests de charge**
**Localisation** : Aucun
**Problème** : Pas de validation que le système supporte 100+ clients simultanés.
**Impact** : Problèmes de performance non détectés.
**Recommandation** : Tests de charge avec k6 ou locust.
---
### 4.3 Fuites Goroutine / Tokio Task
#### ⚠️ **P1 - Goroutines sans mécanisme de shutdown**
**Localisation** : `internal/jobs/cleanup_sessions.go:33-45`
```go
go func() {
for range ticker.C {
// ...
}
}()
```
**Problème** : Pas de moyen d'arrêter cette goroutine proprement.
**Impact** : Goroutine continue après arrêt du serveur.
**Recommandation** : Utiliser `context.Context` avec cancellation.
---
#### ⚠️ **P1 - Tokio tasks spawnées sans supervision**
**Localisation** : `veza-chat-server/src/optimized_persistence.rs:264-285`
```rust
tokio::spawn(async move {
engine_clone.batch_processing_loop().await;
});
```
**Problème** : Si la task panic, elle n'est pas relancée.
**Impact** : Service peut s'arrêter silencieusement.
**Recommandation** : Utiliser un supervisor task qui relance les tasks en cas de panic.
---
### 4.4 Logging Contextuel
#### ⚠️ **P1 - Pas de correlation-id systématique**
**Localisation** : Tous les services
**Problème** : Pas de `correlation-id` ou `trace-id` pour suivre une requête à travers les services.
**Impact** : Debugging difficile en production.
**Recommandation** : Implémenter OpenTelemetry ou un système de tracing distribué.
---
#### ⚠️ **P2 - Logs non structurés dans certains endroits**
**Localisation** : Quelques handlers
**Problème** : Certains logs utilisent `fmt.Printf` au lieu de `tracing` ou `zap`.
**Impact** : Logs non queryables.
**Recommandation** : Standardiser sur `tracing` (Rust) et `zap` (Go).
---
### 4.5 Risques d'Incohérence DB
#### ❌ **P0 - Jobs, messages, segments peuvent être incohérents**
**Localisation** : Tous les services
**Problème** : Si un job de transcodage échoue après avoir créé des segments en DB, les segments restent orphelins.
**Impact** : DB contient des données incohérentes.
**Recommandation** : Jobs de cleanup périodiques pour supprimer les données orphelines.
---
#### ⚠️ **P1 - Pas de vérification d'intégrité**
**Localisation** : Aucun
**Problème** : Pas de job qui vérifie que les fichiers segments correspondent aux enregistrements DB.
**Impact** : Incohérences non détectées.
**Recommandation** : Job de vérification d'intégrité quotidien.
---
## 5. RÉSUMÉ DES RISQUES
### 🔴 P0 — Must-Fix avant déploiement
1. **Backend Go** : Erreurs JSON non traitées silencieusement
2. **Backend Go** : Absence de transactions dans opérations critiques
3. **Chat Server** : Race condition dans TypingIndicatorManager
4. **Chat Server** : Panics possibles (31 fichiers avec `unwrap()`)
5. **Stream Server** : Tasks non cancellées proprement
6. **Global** : Pas de transaction distribuée
7. **Global** : Manque de tests unitaires critiques
8. **Global** : Jobs/messages/segments peuvent être incohérents
### 🟠 P1 — Production-grade minimal
1. **Backend Go** : Erreurs silencieuses, validation input incomplète
2. **Backend Go** : Race condition dans workers retries
3. **Backend Go** : Timing attack password reset
4. **Chat Server** : Race conditions dans DeliveredStatusManager/ReadReceiptManager
5. **Chat Server** : Pas de panic boundary dans WebSocket handler
6. **Chat Server** : Tasks orphelins, pas de heartbeat timeout
7. **Stream Server** : Erreurs FFmpeg non propagées, DB pas toujours sync
8. **Stream Server** : Corruption d'état concurrent dans SegmentTracker
9. **Stream Server** : Regex non robustes, IO errors incomplètes
10. **Global** : Pas de validation croisée IDs, pas de tests de charge
11. **Global** : Fuites goroutine/task, pas de correlation-id
### 🟡 P2 — Qualité continue
1. **Backend Go** : Pas de circuit breaker health check
2. **Backend Go** : Queue in-memory sans persistance
3. **Global** : Logs non structurés, pas de vérification d'intégrité
---
## 📊 STATISTIQUES
- **P0 (Critique)** : 8 problèmes
- **P1 (Important)** : 11 problèmes
- **P2 (Amélioration)** : 3 problèmes
- **Total** : 22 problèmes identifiés
---
## 🔗 LIENS AVEC TRIAGE ACTUEL
Voir `TRIAGE.md` pour l'état fonctionnel des features. Cet audit se concentre sur la **robustesse** et la **stabilité**, pas sur les features manquantes.
---
**Prochaines étapes** : Générer `HARDENING_PLAN.md` avec plan de correction priorisé.