Matthieu
435 lines
30 KiB
Markdown
435 lines
30 KiB
Markdown
# Review complète — Projet Inventory
|
||
|
||
> Audit éducatif du projet. Chaque point explique le problème, pourquoi c'est un problème, et comment le corriger.
|
||
> Document généré le 2026-06-11 (branche `develop`, v1.9.47).
|
||
> Complément de `docs/REVIEW_ARCHITECTURE.md` (2026-03-23) — les findings d'architecture n'y sont pas dupliqués, seul leur **statut** est mis à jour ici (§4).
|
||
|
||
---
|
||
|
||
## Table des matières
|
||
1. [Sécurité](#1-sécurité)
|
||
2. [Bugs fonctionnels](#2-bugs-fonctionnels)
|
||
3. [Code mort et violations des règles projet](#3-code-mort-et-violations-des-règles-projet)
|
||
4. [Dette d'architecture — suivi REVIEW_ARCHITECTURE.md](#4-dette-darchitecture--suivi-review_architecturemd)
|
||
5. [Documentation et configuration](#5-documentation-et-configuration)
|
||
6. [Frontend et UX](#6-frontend-et-ux)
|
||
7. [CI/CD et dépendances](#7-cicd-et-dépendances)
|
||
8. [Hygiène git](#8-hygiène-git)
|
||
9. [Bonnes pratiques à retenir](#9-bonnes-pratiques-à-retenir)
|
||
|
||
---
|
||
|
||
## 1. Sécurité
|
||
|
||
### 1.1 CRITIQUE — Credentials de production commités dans `.mcp.json`
|
||
|
||
**Fichier :** `.mcp.json` (tracké dans git, **pas** dans `.gitignore`)
|
||
|
||
Le fichier contient le mot de passe du profil admin MCP de production (`X-Profile-Password: A123` pour `inventory.malio-dev.fr`) **et** un bearer token Lesstime valide (`project.malio-dev.fr`). Toute personne ayant accès au dépôt (ou à son historique, même après suppression du fichier) peut s'authentifier sur les deux systèmes de production.
|
||
|
||
**Pourquoi c'est grave :** un secret commité reste dans l'historique git pour toujours. Le token Lesstime donne accès à tout le système de gestion de projet (tâches, temps, absences). Le mot de passe `A123` est en plus trivial.
|
||
|
||
**Correction :**
|
||
1. **Révoquer/changer** les deux secrets (mot de passe du profil MCP + régénérer le token Lesstime) — c'est l'étape la plus importante, supprimer le fichier ne suffit pas.
|
||
2. `git rm --cached .mcp.json` + ajouter `.mcp.json` au `.gitignore`.
|
||
3. Conserver un `.mcp.json.example` avec des placeholders.
|
||
|
||
### 1.2 CRITIQUE — `create_test_user.php` tracké à la racine avec credentials admin en clair
|
||
|
||
**Fichier :** `create_test_user.php:16,59-61` (tracké dans git)
|
||
|
||
Script de debug qui crée un compte `ROLE_ADMIN` avec `admin@admin.com` / `admin123` (hardcodé et affiché en clair sur stdout), connexion PDO brute `root`/`root`. Posé à la racine du projet, il est embarquable dans une image de prod et exécutable partout où `vendor/` existe.
|
||
|
||
**Pourquoi c'est grave :** si ce script tourne (ou a tourné) en production, il existe un compte admin avec un mot de passe devinable en 3 essais. Les credentials sont aussi dans l'historique git.
|
||
|
||
**Correction :** supprimer le fichier du dépôt (`git rm`), vérifier **en prod** qu'aucun profil `admin@admin.com` actif n'existe. Le besoin légitime (seed d'un admin de dev) est déjà couvert par `fixtures/` ou peut devenir une commande Symfony `app:create-admin` qui demande le mot de passe en argument.
|
||
|
||
### 1.3 IMPORTANT — Mot de passe de la base de prod hardcodé dans 9 scripts trackés
|
||
|
||
**Fichiers :** `scripts/check-prod-values.php`, `scripts/fix-prod-all.php`, `scripts/restore-custom-field-values.php`, `scripts/migrate-orphaned-custom-fields.php`, `scripts/check-prod-audit-dates.php`, `scripts/check-prod-missing-piece-cfs.php`, `scripts/check-prod-orphaned-detail.php`, `scripts/fix-prod-recreate-and-migrate.php`, `scripts/verify-prod-health.php`
|
||
|
||
Neuf scripts de réparation one-shot contiennent le couple `ferme_user`/`fermerecette` en dur — des credentials de base de données de production, dans le dépôt.
|
||
|
||
**Pourquoi c'est grave :** même problème que 1.1 — secret en clair dans l'historique. En plus ces scripts contournent Doctrine et l'audit : les exécuter par erreur modifie la prod sans trace.
|
||
|
||
**Correction :** changer le mot de passe PG concerné, puis archiver/supprimer ces scripts (ils ont déjà servi). S'ils doivent rester, lire les credentials depuis l'environnement (`getenv('DATABASE_URL')`) et les déplacer dans `_archives/` (déjà gitignoré).
|
||
|
||
### 1.4 IMPORTANT — Aucune limite de taille d'upload (ni applicative, ni infra)
|
||
|
||
**Fichiers :** `src/State/DocumentUploadProcessor.php:55-116`, `src/Controller/CommentController.php:104-137`, `infra/dev/php.ini`, `infra/prod/nginx.conf`
|
||
|
||
Aucun des deux chemins d'upload ne vérifie `$file->getSize()`. Côté infra : `php.ini` ne définit ni `upload_max_filesize` ni `post_max_size` (défauts PHP : 2 Mo / 8 Mo), et `nginx.conf` prod n'a pas de `client_max_body_size` (défaut nginx : **1 Mo**).
|
||
|
||
**Pourquoi c'est un problème (double) :**
|
||
- *Fonctionnel* : en prod, tout upload > 1 Mo est probablement rejeté par nginx avec une erreur 413 brute (non gérée par le front) — alors que l'app est censée stocker des PDF techniques.
|
||
- *Sécurité* : aucune limite **choisie** n'existe ; le jour où quelqu'un monte les limites infra "pour faire passer un gros PDF", plus rien ne protège le disque (un `ROLE_VIEWER` peut uploader via les commentaires, cf. 1.9).
|
||
|
||
**Correction :** décider d'une limite métier (ex. 50 Mo), puis l'appliquer aux 3 niveaux :
|
||
1. Check applicatif `if ($file->getSize() > self::MAX_UPLOAD_BYTES)` dans les deux chemins (erreur 400 propre).
|
||
2. `upload_max_filesize = 50M` / `post_max_size = 55M` dans `infra/dev/php.ini` **et** l'image prod.
|
||
3. `client_max_body_size 55m;` dans `infra/prod/nginx.conf`.
|
||
|
||
### 1.5 MOYEN — Garde anti path-traversal incomplet dans `DocumentStorageService`
|
||
|
||
**Fichier :** `src/Service/DocumentStorageService.php:28-42`
|
||
|
||
`getAbsolutePath()` vérifie `str_contains($relativePath, '..')` puis compare `realpath()` au répertoire de stockage — mais `realpath()` renvoie `false` pour un fichier inexistant, donc le second contrôle est **sauté** dans ce cas. Un chemin absolu (`/etc/passwd`) passe le premier contrôle (pas de `..`) : `$this->storageDir.'/'.'/etc/passwd'`… ne résout pas vers `/etc/passwd`, mais un chemin via symlink dans le storage le pourrait. L'exploitation exige d'écrire `document.path` en base (pas d'input direct utilisateur), donc le risque actuel est faible — c'est du **hardening**.
|
||
|
||
**Correction :** valider sur le répertoire parent, qui existe toujours :
|
||
|
||
```php
|
||
$absolutePath = $this->storageDir.'/'.$relativePath;
|
||
$realParent = realpath(dirname($absolutePath));
|
||
if (false === $realParent || !str_starts_with($realParent.'/', realpath($this->storageDir).'/')) {
|
||
throw new RuntimeException(sprintf('Path traversal detected: "%s"', $relativePath));
|
||
}
|
||
```
|
||
|
||
### 1.6 MOYEN — Pas de protection CSRF : `SameSite=Lax` est la seule barrière
|
||
|
||
**Fichiers :** `config/packages/framework.yaml:8`, `config/packages/nelmio_cors.yaml`
|
||
|
||
L'auth est par cookie de session, et aucun endpoint d'écriture ne vérifie de token CSRF ni de header custom. La protection repose à 100 % sur `cookie_samesite: lax` (et le CORS pour les lectures cross-origin). C'est la posture courante pour une SPA en 2026, mais c'est une **défense à un seul étage** : un navigateur ancien/exotique ou une config future en sous-domaine partagé la ferait tomber. À noter : `csrfToken` existe dans `nuxt.config.ts:59` mais n'est branché nulle part (config morte, cf. 3.1).
|
||
|
||
**Correction (peu coûteuse) :** exiger un header `X-Requested-With: XMLHttpRequest` sur les méthodes non-GET du firewall `api` (un listener de 15 lignes) — un formulaire HTML cross-site ne peut pas envoyer ce header. L'ajouter dans `useApi.ts` côté front. Supprimer le `csrfToken` mort.
|
||
|
||
### 1.7 MOYEN — `session_fixation_strategy: none` désactive la protection globalement
|
||
|
||
**Fichier :** `config/packages/security.yaml:5`, mitigé par `src/Controller/SessionProfileController.php:96`
|
||
|
||
Le choix est documenté (le `migrate` par défaut casse les requêtes concurrentes de la SPA) et le login appelle bien `$session->migrate(true)` manuellement — le flux actuel est correct. Le risque est **futur** : tout nouveau chemin d'authentification (reset de mot de passe, impersonation…) n'aura pas la régénération d'ID de session, silencieusement.
|
||
|
||
**Correction :** garder le réglage, mais l'encadrer : un test fonctionnel qui vérifie que l'ID de session change au login (échouera si quelqu'un retire le `migrate(true)`), et un commentaire dans `SessionProfileController` pointant vers `security.yaml`.
|
||
|
||
### 1.8 MOYEN — MCP HTTP : mot de passe en clair dans les headers à chaque requête
|
||
|
||
**Fichier :** `src/Mcp/Security/McpHeaderAuthenticator.php:43-44`, `infra/prod/nginx-proxy.conf`
|
||
|
||
Chaque requête MCP porte `X-Profile-Password` en clair. Les headers transitent par le proxy nginx et peuvent finir dans des logs (proxy, APM, outils de debug). Le rate-limiting et le hash côté serveur sont bien faits, mais le secret circule en permanence — et l'URL configurée dans `.mcp.json` est en **`http://`** (pas de TLS).
|
||
|
||
**Correction :** passer à un token d'API dédié (longue chaîne aléatoire, stockée hashée, comparée via le hasher existant), transmis en `Authorization: Bearer` — comme le fait déjà Lesstime. Et servir `/_mcp` uniquement en HTTPS.
|
||
|
||
### 1.9 MINEUR — Création de commentaires + upload de fichiers ouverte à `ROLE_VIEWER`
|
||
|
||
**Fichier :** `src/Controller/CommentController.php:33`
|
||
|
||
Convention du projet : lecture = `ROLE_VIEWER`, écriture = `ROLE_GESTIONNAIRE`. La création de commentaire (avec pièces jointes !) est la seule écriture accessible aux viewers. Si c'est un choix métier (« tout le monde peut commenter »), OK — mais combiné à 1.4, un compte en lecture seule peut remplir le disque.
|
||
|
||
**Correction :** confirmer le choix métier et le documenter dans `docs/BACKEND.md` ; appliquer la limite de taille de 1.4 dans tous les cas.
|
||
|
||
### 1.10 MINEUR — `download()` sans les headers de protection de `serve()`
|
||
|
||
**Fichier :** `src/Controller/DocumentServeController.php:105-116`
|
||
|
||
`serve()` envoie `X-Content-Type-Options: nosniff` + `Content-Security-Policy: sandbox` (très bien) ; `download()` n'envoie ni l'un ni l'autre pour les fichiers disque. La disposition `attachment` protège déjà beaucoup, mais l'asymétrie est gratuite.
|
||
|
||
**Correction :** copier les deux headers dans `download()`.
|
||
|
||
### 1.11 MINEUR — Pagination max très élevée sur certaines ressources
|
||
|
||
**Fichiers :** `src/Entity/Constructeur.php:47` (2000), `src/Entity/ConstructeurCategorie.php:43` (1000), `src/Entity/Document.php:55` (500)
|
||
|
||
Un `?itemsPerPage=2000` charge 2000 entités + sérialisation en une requête. Pour des catalogues internes c'est sans doute volontaire (dropdowns sans pagination), mais c'est aussi un vecteur de charge facile.
|
||
|
||
**Correction :** vérifier ce que le front demande réellement et redescendre au besoin réel (ou documenter pourquoi 2000).
|
||
|
||
### 1.12 MINEUR — `infra/dev/.env.docker.local` tracké malgré le `.gitignore`
|
||
|
||
**Fichier :** `infra/dev/.env.docker.local:27,34`
|
||
|
||
Le fichier est dans `.gitignore` (ligne 23) **mais déjà tracké** (ajouté avant la règle — gitignore n'agit pas sur les fichiers déjà suivis). Secrets de dev faibles (`changeme_…`) : sans gravité en soi, mais le fichier est censé être local et chaque dev qui le modifie crée du diff.
|
||
|
||
**Correction :** `git rm --cached infra/dev/.env.docker.local`, fournir `infra/dev/.env.docker.local.example` (référencé par le README au passage).
|
||
|
||
---
|
||
|
||
## 2. Bugs fonctionnels
|
||
|
||
### 2.1 IMPORTANT — Fallback `http://localhost:3000` dans `useApi.ts`
|
||
|
||
**Fichier :** `frontend/app/composables/useApi.ts:18`
|
||
|
||
```ts
|
||
const API_BASE_URL = (publicConfig.apiBaseUrl as string) || 'http://localhost:3000'
|
||
```
|
||
|
||
Si `NUXT_PUBLIC_API_BASE_URL` est vide au build/runtime de prod, **tous** les appels API partent silencieusement vers `localhost:3000` (le port du dev server, qui ne sert même pas l'API). Échec garanti mais difficile à diagnostiquer.
|
||
|
||
**Correction :** fallback relatif `'/api'`… attention, `useApi` préfixe déjà `/api` lui-même — le bon fallback est donc `''` (origine courante) :
|
||
|
||
```ts
|
||
const API_BASE_URL = (publicConfig.apiBaseUrl as string) || ''
|
||
```
|
||
|
||
Même nettoyage pour `nuxt.config.ts:49` (`http://localhost/api`, valeur SSR jamais utilisée puisque `ssr: false`).
|
||
|
||
### 2.2 IMPORTANT — Uploads de prod probablement plafonnés à 1 Mo par nginx
|
||
|
||
Voir 1.4 — c'est le versant fonctionnel : sans `client_max_body_size`, nginx rejette en 413 tout body > 1 Mo, et le front n'affiche pas d'erreur claire (le toast générique de `useApi` au mieux). À tester en prod avec un PDF de 5 Mo ; si les uploads passent, c'est qu'une config existe ailleurs et il faut l'aligner dans le dépôt.
|
||
|
||
### 2.3 MOYEN — Deux mécanismes de maintenance déconnectés
|
||
|
||
**Fichiers :** `src/Controller/MaintenanceController.php:56` (flag `var/maintenance`), `infra/prod/nginx.conf:6` (flag `maintenance.on`), `infra/prod/deploy.sh:47`
|
||
|
||
Le toggle admin (`PUT /api/admin/maintenance`) écrit `var/maintenance`, lu par le middleware front — maintenance **applicative**. Le déploiement crée `maintenance.on`, lu par nginx — maintenance **infra**. Les deux coexistent volontairement mais rien ne le documente : un admin qui active la maintenance via l'UI ne bloque pas les appels API directs (le flag n'est vérifié que par le middleware front), et inversement.
|
||
|
||
**Correction :** documenter les deux niveaux dans `DEPLOY.md` ; idéalement faire vérifier le flag applicatif côté backend (listener kernel.request qui renvoie 503 pour les non-admins) plutôt que de ne compter que sur le middleware front (contournable).
|
||
|
||
### 2.4 MOYEN — Provider Symfony par `email` alors que `email` est nullable
|
||
|
||
**Fichiers :** `config/packages/security.yaml:18`, `src/Entity/Profile.php:59-61`, `src/Controller/AdminProfileController.php:66`
|
||
|
||
On peut créer un profil sans email (profils « kiosque »), mais le user provider charge par `property: email`. Ça marche aujourd'hui parce que `SessionProfileAuthenticator` charge par ID — le provider n'est jamais utilisé pour ces profils. Incohérence latente : tout futur usage du provider standard (remember-me, impersonation, commande console) cassera sur ces profils.
|
||
|
||
**Correction :** soit rendre l'email obligatoire et générer un email technique pour les kiosques, soit écrire un `UserProviderInterface` custom qui charge par id **ou** email, et le déclarer dans `security.yaml`.
|
||
|
||
---
|
||
|
||
## 3. Code mort et violations des règles projet
|
||
|
||
### 3.1 MINEUR — Config morte dans `nuxt.config.ts`
|
||
|
||
**Fichier :** `frontend/nuxt.config.ts:56-59`
|
||
|
||
`csrfToken`, `requestTimeout`, `enableDebug`, `enableAnalytics`, `logLevel` : définis, jamais consommés (seul `apiTimeout` est lu par `useApi.ts`). Du code mort en config laisse croire que des fonctionnalités existent (un lecteur pense que le CSRF est géré — il ne l'est pas, cf. 1.6).
|
||
|
||
**Correction :** supprimer ces 5 clés.
|
||
|
||
### 3.2 MINEUR — Variables JWT dans un projet 100 % session
|
||
|
||
**Fichier :** `infra/dev/.env.docker.local:28-30`
|
||
|
||
`JWT_SECRET_KEY`, `JWT_PUBLIC_KEY`, `JWT_PASSPHRASE` — copiées d'un autre projet (Lesstime/Starseed utilisent JWT, pas Inventory). Le CLAUDE.md martèle « pas JWT » ; ces variables sèment le doute.
|
||
|
||
**Correction :** supprimer les 3 lignes.
|
||
|
||
### 3.3 MOYEN — `@nuxtjs/tailwindcss` : dépendance inutilisée et conflictuelle
|
||
|
||
**Fichier :** `frontend/package.json:21`
|
||
|
||
Le projet utilise Tailwind 4 via `@tailwindcss/vite` (`nuxt.config.ts:1,64`). `@nuxtjs/tailwindcss` n'est référencé nulle part et installe son propre Tailwind **v3** dans `node_modules` — bloat + risque de résolution ambiguë.
|
||
|
||
**Correction :** `npm uninstall @nuxtjs/tailwindcss` puis vérifier `npm run build`.
|
||
|
||
### 3.4 MINEUR — Dockerfile dev pollué par un template générique
|
||
|
||
**Fichier :** `infra/dev/Dockerfile:48-53,82-100`
|
||
|
||
Blocs commentés Oracle OCI8, IMAP/Kerberos, PDO MySQL/SQLite — aucun rapport avec un projet PostgreSQL. Bruit pur.
|
||
|
||
**Correction :** supprimer les blocs commentés.
|
||
|
||
### 3.5 MINEUR — `node_modules/` orphelin à la racine backend
|
||
|
||
Pas de `package.json` à la racine, mais un `node_modules/` (untracked) y traîne, et `node_modules/` n'est pas dans le `.gitignore` racine — seul le hasard l'empêche d'être commité un jour.
|
||
|
||
**Correction :** `rm -rf node_modules/` à la racine + ajouter `/node_modules/` au `.gitignore`.
|
||
|
||
### 3.6 MINEUR — 3 fichiers utils restés en `.js` non typés
|
||
|
||
**Fichiers :** `frontend/app/utils/documentPreview.js`, `frontend/app/utils/fileIcons.js`, `frontend/app/utils/printTemplates/machineReport.js`
|
||
|
||
Importés depuis du `.ts` sans aucune sécurité de type — incohérent avec la règle « TypeScript, 0 erreur typecheck ».
|
||
|
||
**Correction :** renommer en `.ts` et typer les signatures (mécanique, bon candidat Codex).
|
||
|
||
---
|
||
|
||
## 4. Dette d'architecture — suivi REVIEW_ARCHITECTURE.md
|
||
|
||
État des 10 chantiers identifiés le 2026-03-23, vérifié ce jour : **1 corrigé sur 10**, et le God controller a grossi.
|
||
|
||
| # | Source de complexité | Statut 2026-06-11 |
|
||
|---|---------------------|-------------------|
|
||
| 1 | `smartMatch` dupliqué dans les Sync Strategies | ❌ Toujours dupliqué (`ComposantSyncStrategy.php:380`, `PieceSyncStrategy.php:244`) |
|
||
| 2 | Custom Fields : 4-6 FK nullable (polymorphisme pauvre) | ❌ Inchangé, pas de contrainte CHECK |
|
||
| 3 | Composables géants | ⚠️ Partiel : `useComponentEdit.ts` 539 LOC, `usePieceEdit.ts` 404, `useComponentCreate.ts` 366 |
|
||
| 4 | Triple duplication utils custom fields | ✅ **Corrigé** — fusionné dans `shared/utils/customFields.ts` |
|
||
| 5 | `pendingStructure` canal caché | ❌ Toujours sans `try/finally` (`ModelTypeProcessor.php`) |
|
||
| 6 | `PieceProductSyncSubscriber` legacy | ❌ Inchangé (`recomputeSingleEntityChangeSet` toujours là) |
|
||
| 7 | Double flush dans les processors | ❌ Inchangé (`ComposantProcessor.php:45,132`) |
|
||
| 8 | `MachineStructureController` God controller | ❌ **Aggravé** : 300+ → **1121 lignes** |
|
||
| 9 | Dépendance circulaire `useMachineDetailData` | ❌ Proxy ref toujours en place (`:133`) |
|
||
| 10 | Typage `any` systématique | ❌ **179 occurrences** dans 26 composables |
|
||
|
||
**Le point clé :** la Phase 1 « quick wins » du plan (items 1, 5, 6, 7 — effort S chacun, sans impact d'interface) n'a pas été entamée en presque 3 mois, alors que ce sont les corrections au meilleur ratio risque/bénéfice du projet. Le `MachineStructureController` qui grossit de +150 lignes confirme la trajectoire : sans extraction de services, chaque feature l'alourdit.
|
||
|
||
---
|
||
|
||
## 5. Documentation et configuration
|
||
|
||
### 5.1 IMPORTANT — Toute la doc décrit encore un submodule qui n'existe plus
|
||
|
||
**Fichiers :** `README.md:49-50,291-296`, `DEPLOY.md:58,62,208`, `RELEASE.md:49,117`, `frontend/README.md:150-154`
|
||
|
||
Le frontend a été intégré au monorepo (cf. CLAUDE.md « plus de submodule »), mais le README explique `git clone --recurse-submodules`, un workflow de commit en deux temps, et DEPLOY/RELEASE font des `git submodule update`. Un nouveau dev qui suit le README perd du temps sur des commandes sans effet ; un déploiement scripté depuis DEPLOY.md exécute des étapes mortes.
|
||
|
||
**Correction :** purger toute mention de submodule des 4 fichiers, décrire le workflow monorepo (1 commit racine).
|
||
|
||
### 5.2 IMPORTANT — `DEPLOY.md` utilise l'utilisateur PG d'un autre projet
|
||
|
||
**Fichier :** `DEPLOY.md` (6 occurrences de `ferme_user`)
|
||
|
||
Les commandes psql/backup de DEPLOY.md utilisent `ferme_user`/`fermerecette` — copié-collé du projet **Ferme**. Le vrai user prod est `inventory_user` (cf. `infra/prod/.env.example`). Quelqu'un qui suit la doc en incident de prod tape des commandes qui échouent (ou pire, sur la mauvaise base si les deux co-habitent sur l'instance partagée).
|
||
|
||
**Correction :** remplacer les 6 occurrences par `inventory_user` et des placeholders de mot de passe.
|
||
|
||
### 5.3 MOYEN — `RELEASE.md` et `CLAUDE.md` référencent un fichier `VERSION` inexistant
|
||
|
||
**Fichiers :** `RELEASE.md:17,50,80,82`, `CLAUDE.md` (arbre projet)
|
||
|
||
La version vit dans `config/version.yaml` (1.9.47) — le fichier `VERSION` n'existe plus. `scripts/release.sh` et la CI (`auto-tag-develop.yml`) sont à jour, mais la doc décrit l'ancien système, y compris « `frontend/nuxt.config.ts` lit `VERSION` au build ».
|
||
|
||
**Correction :** mettre à jour RELEASE.md et la ligne d'arbre dans CLAUDE.md vers `config/version.yaml`.
|
||
|
||
### 5.4 MINEUR — Pas de `.env.example` backend ni de `.env.docker.local.example`
|
||
|
||
Un nouveau dev n'a aucun modèle des variables attendues côté backend (le README pointe vers `infra/dev/.env.docker.local`… qui est censé être local/ignoré, cf. 1.12).
|
||
|
||
**Correction :** créer `infra/dev/.env.docker.local.example` avec placeholders.
|
||
|
||
### 5.5 MINEUR — Fichiers de données métier à la racine du projet
|
||
|
||
`Company (1).json`, `customer.json`, `customer.original.json`, `Ensemble simple rotor.pdf`, `inventory_prod (2).sql.gz` traînent à la racine (untracked — le `.sql.gz` est protégé par le gitignore, **pas les `.json` ni le `.pdf`** : un `git add .` distrait les commiterait, et `customer.json` ressemble à des données client réelles → RGPD).
|
||
|
||
**Correction :** déplacer hors du dépôt (ex. `~/imports/`), et ajouter une règle défensive au `.gitignore` (`/*.json` racine, `/*.pdf`).
|
||
|
||
---
|
||
|
||
## 6. Frontend et UX
|
||
|
||
### 6.1 MOYEN — Erreurs avalées : `console.error` sans feedback utilisateur
|
||
|
||
**Fichiers :** 57+ occurrences ; ex. `useMachineDetailData.ts:372,385`, `useProfileSession.ts:27`
|
||
|
||
Le pattern dominant en cas d'échec API est `console.error(...)` puis on continue avec `null`/`[]`. `useApi` toaste déjà les erreurs HTTP, donc une partie est couverte — mais les erreurs réseau/parsing et les branches qui catchent **avant** le toast laissent l'utilisateur devant une page partiellement vide sans explication, et créent du double-reporting ailleurs.
|
||
|
||
**Correction :** définir une règle unique : `useApi` est le seul à toaster ; les composables ne re-loggent pas, mais positionnent un état d'erreur (`error.value = ...`) que la page affiche (bandeau « Impossible de charger X — Réessayer »).
|
||
|
||
### 6.2 MOYEN — Appel `/maintenance/check` à chaque navigation
|
||
|
||
**Fichier :** `frontend/app/middleware/profile.global.ts:34-39`
|
||
|
||
Chaque changement de route d'un non-admin déclenche un aller-retour API. Latence ajoutée à **toutes** les navigations pour un état qui change une fois par an.
|
||
|
||
**Correction :** cacher le résultat dans un `useState` avec TTL (ex. 60 s) ; le 503 éventuel d'un appel API normal peut aussi servir de signal.
|
||
|
||
### 6.3 Rappels (déjà au §4)
|
||
|
||
`any` ×179, composables géants, dépendance circulaire — voir tableau §4 et `docs/REVIEW_ARCHITECTURE.md` pour les solutions détaillées.
|
||
|
||
---
|
||
|
||
## 7. CI/CD et dépendances
|
||
|
||
### 7.1 IMPORTANT — Aucun garde-fou automatisé : la CI ne lance ni tests ni lint
|
||
|
||
**Fichiers :** `.gitea/workflows/auto-tag-develop.yml`, `.gitea/workflows/build-docker.yml`, workflow de commit (CLAUDE.md : « committer avec `--no-verify` »)
|
||
|
||
La chaîne actuelle : push sur `develop` → auto-tag → build Docker → image prod. **Aucune étape ne lance PHPUnit, php-cs-fixer, ESLint ou `nuxi typecheck`.** Et comme le hook pre-commit (qui devait jouer ce rôle) est trop lent, la convention projet est de le contourner avec `--no-verify`. Résultat : il est possible de tagger et construire une image de prod avec des tests rouges sans qu'aucun système ne le signale.
|
||
|
||
**Pourquoi c'est le finding process le plus important de cette review :** la suite de tests est bonne (48 fichiers, DAMA, factories) — mais une suite de tests qui ne tourne pas en CI ne protège rien.
|
||
|
||
**Correction :** ajouter un workflow `ci.yml` déclenché sur PR + push develop :
|
||
1. `composer install` + `php-cs-fixer --dry-run` + PHPUnit (avec un service PG 16).
|
||
2. `npm ci` + `eslint` + `npx nuxi typecheck` + `npm run build` dans `frontend/`.
|
||
3. Faire dépendre `auto-tag-develop` du succès de la CI (ou au minimum bloquer les PR).
|
||
Le hook pre-commit lent peut alors être assumé comme optionnel.
|
||
|
||
### 7.2 MOYEN — Le build Docker de prod n'est pas testé avant le push
|
||
|
||
`build-docker.yml` build + push `latest` dès qu'un tag est posé — sans health-check de l'image (la CI de 7.1 règle l'essentiel ; un `docker run --rm image php bin/console about` est un bon smoke test bon marché).
|
||
|
||
### 7.3 Dépendances
|
||
|
||
- Backend : propre, à jour (Symfony 8.0.*, AP ^4.2, ORM ^3.6). `symfony/twig-bundle` à vérifier : aucun template Twig dans `templates/` — si seul le MCP bundle le requiert, le laisser en dépendance transitive.
|
||
- Frontend : `@nuxtjs/tailwindcss` à supprimer (cf. 3.3). Le reste est à jour et utilisé.
|
||
|
||
---
|
||
|
||
## 8. Hygiène git
|
||
|
||
Synthèse des fichiers trackés à tort (détails en §1/§5) :
|
||
|
||
| Fichier | Problème | Action |
|
||
|---------|----------|--------|
|
||
| `.mcp.json` | Credentials prod | `git rm --cached` + gitignore + **rotation** |
|
||
| `create_test_user.php` | Script debug + credentials | `git rm` |
|
||
| `scripts/{check,fix,restore,migrate,verify}-prod-*.php` (9) | Mot de passe PG prod | rotation + archive/suppression |
|
||
| `infra/dev/.env.docker.local` | Censé être local, déjà tracké | `git rm --cached` + `.example` |
|
||
| `.claude/settings.json` | Inoffensif (config plugins) | OK, peut rester |
|
||
|
||
Gaps `.gitignore` racine : `/node_modules/`, `.mcp.json`, `/*.json` (défensif), `/*.pdf`.
|
||
|
||
> Note : contrairement au premier rapport d'agent, `.claude/settings.local.json` n'est **pas** tracké — pas d'action nécessaire.
|
||
|
||
---
|
||
|
||
## 9. Bonnes pratiques à retenir
|
||
|
||
### Ce qui est bien fait dans le projet
|
||
|
||
- **`declare(strict_types=1)` partout**, attributs PHP 8 modernes, code backend homogène et lisible.
|
||
- **Sécurité API systématique** : chaque opération API Platform porte son `security:`, chaque controller custom ouvre par `denyAccessUnlessGranted()` — aucun endpoint accidentellement public trouvé.
|
||
- **Rate limiting** sur le login **et** sur l'auth MCP (souvent oublié ailleurs).
|
||
- **IDs CUID** : pas d'énumération séquentielle possible.
|
||
- **Upload : validation MIME par contenu** (`finfo`), nom de fichier régénéré (CUID), stockage hors de `public/` — et SVG exclu de l'allowlist (XSS évité).
|
||
- **`serve()` documents** : `nosniff` + `CSP: sandbox` — au-dessus du standard.
|
||
- **Système d'audit** propre (`AbstractAuditSubscriber` en template method, flag `skipAudit` réfléchi).
|
||
- **Tests solides** : 48 fichiers, `AbstractApiTestCase` avec factories, DAMA rollback, coûts de hash réduits en test.
|
||
- **Le refacto `customFields.ts` a été fait** — la dette de REVIEW_ARCHITECTURE n'est pas ignorée, juste lente.
|
||
- **Docker prod multi-stage** propre, page maintenance nginx, volumes nommés pour le storage.
|
||
- **Zéro `console.log`**, zéro TODO/FIXME oublié dans `src/` et `frontend/app/`.
|
||
|
||
### Les règles à graver
|
||
|
||
1. **Un secret commité est un secret grillé** : la suppression du fichier ne suffit jamais, il faut **révoquer** (cf. `.mcp.json`, scripts prod).
|
||
2. **`.gitignore` n'agit pas sur les fichiers déjà trackés** — `git rm --cached` d'abord.
|
||
3. **Une suite de tests qui ne tourne pas en CI ne protège rien** : si le hook est trop lent pour être vécu, le garde-fou doit vivre en CI.
|
||
4. **Toute limite (taille, pagination, timeout) doit être choisie, pas héritée d'un défaut** — sinon c'est le défaut le plus bas de la chaîne qui décide (nginx 1 Mo).
|
||
5. **Un fallback doit échouer bruyamment ou être correct** — `|| 'http://localhost:3000'` en prod est le pire des deux mondes.
|
||
6. **La doc copiée d'un autre projet est pire que pas de doc** (`ferme_user` dans DEPLOY.md).
|
||
7. **Config morte = mensonge** : un `csrfToken` non branché fait croire qu'une protection existe.
|
||
8. **Les scripts one-shot ont une date de péremption** : après usage → `_archives/` ou suppression.
|
||
9. **Quand un God controller existe, chaque feature le fait grossir** : extraire tôt (1121 lignes et ça monte).
|
||
10. **Les quick wins planifiés perdent leur valeur s'ils ne sont jamais faits** : la Phase 1 de REVIEW_ARCHITECTURE (4×effort S) attend depuis 3 mois.
|
||
|
||
---
|
||
|
||
## Résumé par priorité
|
||
|
||
| Priorité | # | Problème | Fichier |
|
||
|----------|---|----------|---------|
|
||
| **P0** | 1.1 | Credentials prod + token Lesstime commités (rotation requise) | `.mcp.json` |
|
||
| **P0** | 1.2 | Script admin avec mot de passe en clair tracké | `create_test_user.php` |
|
||
| **P0** | 1.3 | Mot de passe PG prod dans 9 scripts trackés | `scripts/*-prod-*.php` |
|
||
| **P1** | 7.1 | CI sans tests ni lint + hook contourné | `.gitea/workflows/` |
|
||
| **P1** | 1.4/2.2 | Aucune limite d'upload choisie (et défaut nginx 1 Mo) | `DocumentUploadProcessor.php`, `CommentController.php`, `infra/` |
|
||
| **P1** | 2.1 | Fallback API `localhost:3000` en prod | `frontend/app/composables/useApi.ts:18` |
|
||
| **P1** | 5.1 | Doc submodule obsolète (4 fichiers) | `README.md`, `DEPLOY.md`, `RELEASE.md`, `frontend/README.md` |
|
||
| **P1** | 5.2 | `ferme_user` dans DEPLOY.md | `DEPLOY.md` |
|
||
| **P1** | 1.5 | Garde path-traversal incomplet | `DocumentStorageService.php:28-42` |
|
||
| **P2** | 1.6 | CSRF : une seule barrière (SameSite) | `framework.yaml`, `useApi.ts` |
|
||
| **P2** | 1.7 | Session fixation : protection désactivée globalement | `security.yaml:5` |
|
||
| **P2** | 1.8 | Mot de passe MCP dans headers + HTTP sans TLS | `McpHeaderAuthenticator.php` |
|
||
| **P2** | 2.3 | Double mécanisme maintenance non documenté | `MaintenanceController.php`, `nginx.conf` |
|
||
| **P2** | 2.4 | Provider `email` vs email nullable | `security.yaml:18`, `Profile.php` |
|
||
| **P2** | 3.3 | `@nuxtjs/tailwindcss` inutilisé (TW3 vs TW4) | `frontend/package.json:21` |
|
||
| **P2** | 5.3 | Doc `VERSION` obsolète | `RELEASE.md`, `CLAUDE.md` |
|
||
| **P2** | 5.5 | Données client à la racine (`customer.json`…) | racine projet |
|
||
| **P2** | 6.1 | Erreurs avalées sans feedback UI | composables (57+) |
|
||
| **P2** | 6.2 | Maintenance check à chaque navigation | `profile.global.ts:34-39` |
|
||
| **P2** | 1.12 | `.env.docker.local` tracké malgré gitignore | `infra/dev/.env.docker.local` |
|
||
| **P3** | 1.9 | Commentaires+upload en ROLE_VIEWER (à confirmer métier) | `CommentController.php:33` |
|
||
| **P3** | 1.10 | Pagination max 2000/1000/500 | `Constructeur.php`, `Document.php` |
|
||
| **P3** | 1.11/3.2 | Secrets dev faibles + vars JWT mortes | `infra/dev/.env.docker.local` |
|
||
| **P3** | 3.1 | Config morte nuxt.config (5 clés) | `frontend/nuxt.config.ts:56-59` |
|
||
| **P3** | 3.4 | Dockerfile dev : blocs commentés Oracle/MySQL | `infra/dev/Dockerfile` |
|
||
| **P3** | 3.5 | `node_modules/` orphelin racine + gitignore | racine projet |
|
||
| **P3** | 3.6 | 3 fichiers `.js` non typés | `frontend/app/utils/` |
|
||
| **P3** | 1.10 | `download()` sans nosniff/CSP | `DocumentServeController.php:105-116` |
|
||
| **P3** | 5.4 | Pas de `.env.docker.local.example` | `infra/dev/` |
|
||
| **P3** | 7.2 | Image Docker poussée sans smoke test | `build-docker.yml` |
|
||
|
||
> La dette d'architecture (§4) a son propre plan dans `docs/REVIEW_ARCHITECTURE.md` — recommandation : exécuter enfin sa **Phase 1** (4 corrections effort S, sans impact d'interface).
|