diff --git a/REVIEW.md b/REVIEW.md new file mode 100644 index 0000000..b189ce5 --- /dev/null +++ b/REVIEW.md @@ -0,0 +1,434 @@ +# 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). diff --git a/TICKETS.md b/TICKETS.md new file mode 100644 index 0000000..bcf7f38 --- /dev/null +++ b/TICKETS.md @@ -0,0 +1,343 @@ +# Tickets correctifs — Projet Inventory + +> Liste de tâches issues de la review du 2026-06-11 (`REVIEW.md`). +> Chaque ticket est autonome : contexte, ce qu'il faut faire, fichiers concernés. +> Commence par les P0, puis P1, etc. Convention de commit : `fix(T-XXX) : description courte`. + +--- + +## P0 — Urgents (sécurité) + +### T-001 — Révoquer et retirer les credentials de `.mcp.json` +**Pourquoi :** le fichier `.mcp.json` est dans git et contient le mot de passe admin MCP de production (`A123`) et un token Lesstime valide. Toute personne avec accès au dépôt (ou à son historique) peut se connecter aux deux systèmes. Supprimer le fichier ne suffit pas : git garde l'historique — il faut **changer les secrets**. +**À faire :** +1. Changer le mot de passe du profil `admin-default-profile` sur `inventory.malio-dev.fr` (et choisir un vrai mot de passe, pas `A123`). +2. Régénérer le bearer token Lesstime côté Lesstime. +3. Sortir le fichier de git sans le supprimer du disque : + ```bash + git rm --cached .mcp.json + echo ".mcp.json" >> .gitignore + ``` +4. Créer `.mcp.json.example` avec des placeholders : + ```json + { + "mcpServers": { + "inventory": { + "type": "http", + "url": "https://inventory.malio-dev.fr/_mcp", + "headers": { + "X-Profile-Id": "", + "X-Profile-Password": "" + } + } + } + } + ``` +5. Remettre les nouveaux secrets dans ton `.mcp.json` local (désormais ignoré). +**Fichiers :** `.mcp.json`, `.gitignore`, `.mcp.json.example` (nouveau) + +### T-002 — Supprimer `create_test_user.php` et vérifier la prod +**Pourquoi :** ce script de debug, commité à la racine, crée un compte `ROLE_ADMIN` avec `admin@admin.com` / `admin123` — un mot de passe devinable en quelques essais. S'il a déjà tourné en production, un compte admin faible existe peut-être en ce moment. +**À faire :** +1. Vérifier en prod qu'aucun profil `admin@admin.com` n'est actif : + ```sql + SELECT id, email, is_active, roles FROM profiles WHERE email = 'admin@admin.com'; + ``` + S'il existe : le désactiver ou changer son mot de passe immédiatement. +2. Supprimer le script : + ```bash + git rm create_test_user.php + ``` +3. (Optionnel) Si le besoin « créer un admin de dev » existe encore, créer une commande Symfony `app:create-admin` qui prend le mot de passe en argument — ne jamais le hardcoder. +**Fichiers :** `create_test_user.php` + +### T-003 — Changer le mot de passe PG prod et archiver les scripts `*-prod-*.php` +**Pourquoi :** 9 scripts dans `scripts/` contiennent le mot de passe de la base de production en clair (`fermerecette`). Ce sont des scripts de réparation one-shot qui ont déjà servi : ils n'ont plus de raison d'être dans le dépôt avec des secrets dedans. +**À faire :** +1. Changer le mot de passe de l'utilisateur PG concerné sur le serveur de prod. +2. Archiver les scripts (le dossier `_archives/` est déjà dans le `.gitignore`) : + ```bash + mkdir -p _archives/scripts-prod + git rm scripts/check-prod-values.php scripts/check-prod-audit-dates.php \ + scripts/check-prod-missing-piece-cfs.php scripts/check-prod-orphaned-detail.php \ + scripts/fix-prod-all.php scripts/fix-prod-recreate-and-migrate.php \ + scripts/migrate-orphaned-custom-fields.php scripts/restore-custom-field-values.php \ + scripts/verify-prod-health.php + # (les copies locales peuvent aller dans _archives/scripts-prod si tu veux les garder) + ``` +3. Si l'un d'eux doit rester utilisable : remplacer les credentials en dur par `getenv('DATABASE_URL')`. +**Fichiers :** `scripts/*-prod-*.php`, `scripts/migrate-orphaned-custom-fields.php`, `scripts/restore-custom-field-values.php` + +--- + +## P1 — Importants + +### T-004 — Ajouter une CI qui lance tests et lint +**Pourquoi :** aujourd'hui, rien ne lance les tests automatiquement : la CI Gitea ne fait que tagger et builder l'image Docker, et le hook pre-commit est contourné avec `--no-verify` (trop lent). On peut donc livrer en prod avec des tests rouges sans aucune alerte. +**À faire :** +1. Créer `.gitea/workflows/ci.yml` : + ```yaml + name: CI + on: + pull_request: + push: + branches: [develop] + jobs: + backend: + runs-on: ubuntu-latest + services: + postgres: + image: postgres:16-alpine + env: + POSTGRES_USER: root + POSTGRES_PASSWORD: root + POSTGRES_DB: inventory_test + ports: ["5432:5432"] + steps: + - uses: actions/checkout@v4 + - uses: shivammathur/setup-php@v2 + with: { php-version: '8.4', extensions: 'pdo_pgsql, intl' } + - run: composer install --no-interaction --prefer-dist + - run: vendor/bin/php-cs-fixer fix --dry-run --diff --allow-risky=yes + - run: php bin/console doctrine:schema:create --env=test + env: { DATABASE_URL: "postgresql://root:root@localhost:5432/inventory_test" } + - run: vendor/bin/phpunit + env: { DATABASE_URL: "postgresql://root:root@localhost:5432/inventory_test" } + frontend: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: { node-version: 22 } + - run: npm ci + working-directory: frontend + - run: npx eslint . + working-directory: frontend + - run: npx nuxi typecheck + working-directory: frontend + - run: npm run build + working-directory: frontend + ``` + > Adapter les détails (version PHP exacte, env de test) au premier run — l'important est que les 4 vérifications (cs-fixer, PHPUnit, ESLint, typecheck) tournent. +2. Dans Gitea, marquer ce workflow comme requis pour merger une PR vers `develop`. +**Fichiers :** `.gitea/workflows/ci.yml` (nouveau) + +### T-005 — Définir une limite de taille d'upload à tous les niveaux +**Pourquoi :** aucune limite n'est choisie nulle part. Conséquence double : en prod, nginx applique son défaut de **1 Mo** (les gros PDF sont sans doute rejetés avec une erreur brute), et côté application rien n'empêcherait de remplir le disque si les limites infra étaient relevées. Décision à prendre : 50 Mo max (à ajuster au métier). +**À faire :** +1. Check applicatif dans `DocumentUploadProcessor::handleMultipartUpload()` (après la validation MIME, ligne ~79) : + ```php + private const MAX_UPLOAD_BYTES = 50 * 1024 * 1024; // 50 Mo + + if ($file->getSize() > self::MAX_UPLOAD_BYTES) { + throw new BadRequestHttpException('Fichier trop volumineux (max 50 Mo).'); + } + ``` +2. Même check dans `CommentController::create()` dans la boucle `foreach ($files as $file)` (ligne ~106) — renvoyer `$this->json(['message' => 'Fichier trop volumineux (max 50 Mo).'], 400)`. +3. `infra/dev/php.ini` — ajouter : + ```ini + upload_max_filesize = 50M + post_max_size = 55M + ``` + Et vérifier que l'image prod (infra/prod/Dockerfile) reçoit la même config. +4. `infra/prod/nginx.conf` — dans le bloc `server` : + ```nginx + client_max_body_size 55m; + ``` + (idem dans `nginx-proxy.conf` si le proxy frontal est aussi versionné ici). +5. Tester : upload d'un PDF de ~5 Mo en local, et vérifier le message d'erreur propre au-delà de 50 Mo. +**Fichiers :** `src/State/DocumentUploadProcessor.php`, `src/Controller/CommentController.php`, `infra/dev/php.ini`, `infra/prod/nginx.conf`, `infra/prod/Dockerfile` + +### T-006 — Corriger le fallback d'URL API du frontend +**Pourquoi :** si la variable `NUXT_PUBLIC_API_BASE_URL` est vide en prod, tous les appels API partent vers `http://localhost:3000` — c'est-à-dire vers la machine de l'utilisateur. L'app casse silencieusement. Un fallback vide = « même origine », ce qui est le comportement correct. +**À faire :** +1. `frontend/app/composables/useApi.ts` ligne 18 : + ```ts + // AVANT + const API_BASE_URL = (publicConfig.apiBaseUrl as string) || 'http://localhost:3000' + // APRÈS (chaîne vide = même origine ; useApi ajoute déjà /api) + const API_BASE_URL = (publicConfig.apiBaseUrl as string) || '' + ``` +2. `frontend/nuxt.config.ts` ligne ~49 : remplacer le fallback `'http://localhost/api'` par `''` (valeur SSR jamais utilisée, SSR off — autant ne pas mentir). +3. Vérifier en dev que tout fonctionne encore (la variable est définie en dev, donc rien ne doit changer), puis `npx nuxi typecheck`. +**Fichiers :** `frontend/app/composables/useApi.ts`, `frontend/nuxt.config.ts` + +### T-007 — Purger la doc « submodule » (le frontend est dans le monorepo) +**Pourquoi :** le frontend a été rapatrié dans le repo principal, mais README, DEPLOY, RELEASE et le README frontend décrivent encore le clonage `--recurse-submodules` et le workflow de commit en deux temps. Un nouveau dev (ou toi en incident de prod) suit des étapes qui n'existent plus. +**À faire :** +1. `README.md` : lignes 49-50 → `git clone ` ; section « workflow » lignes ~291-296 → décrire « un seul commit depuis la racine couvre backend + frontend ». +2. `DEPLOY.md` : supprimer les `git submodule update --init --recursive` (lignes 58, 62, 208). +3. `RELEASE.md` : supprimer les étapes submodule (lignes 49, 117). +4. `frontend/README.md` : remplacer la section « submodule » (lignes 150-154) par « ce dossier fait partie du monorepo Inventory ». +**Fichiers :** `README.md`, `DEPLOY.md`, `RELEASE.md`, `frontend/README.md` + +### T-008 — Corriger l'utilisateur PG dans `DEPLOY.md` +**Pourquoi :** les commandes de DEPLOY.md utilisent `ferme_user` / `fermerecette` — copié-collé du projet Ferme. Le vrai utilisateur est `inventory_user`. En situation d'incident, suivre la doc ferait taper des commandes qui échouent ou visent la mauvaise base. +**À faire :** +1. Remplacer les 6 occurrences de `ferme_user` par `inventory_user`. +2. Remplacer le mot de passe en clair par un placeholder `` (le mot de passe n'a rien à faire dans la doc). +**Fichiers :** `DEPLOY.md` + +### T-009 — Durcir le garde anti path-traversal de `DocumentStorageService` +**Pourquoi :** le contrôle `realpath()` (qui vérifie que le chemin final est bien dans le dossier de stockage) est sauté quand le fichier n'existe pas, car `realpath()` renvoie `false` dans ce cas. Le risque actuel est faible (le chemin vient de la base, pas de l'utilisateur), mais le contrôle se veut une protection — autant qu'il protège vraiment. +**À faire :** dans `getAbsolutePath()` (`src/Service/DocumentStorageService.php:28-42`) : +```php +// AVANT +$absolutePath = $this->storageDir.'/'.$relativePath; +$realPath = realpath($absolutePath); + +if (false !== $realPath && !str_starts_with($realPath, realpath($this->storageDir))) { + throw new RuntimeException(sprintf('Path traversal detected: "%s"', $relativePath)); +} + +// APRÈS — valider sur le dossier parent, qui existe toujours pour un fichier servi +$absolutePath = $this->storageDir.'/'.$relativePath; +$realParent = realpath(dirname($absolutePath)); +$realStorage = realpath($this->storageDir); + +if (false === $realStorage || false === $realParent + || !str_starts_with($realParent.'/', $realStorage.'/')) { + throw new RuntimeException(sprintf('Path traversal detected: "%s"', $relativePath)); +} +``` +Garder le check `str_contains($relativePath, '..')` existant en première ligne. Ajouter un test unitaire avec un chemin contenant `../` et un chemin absolu. +**Fichiers :** `src/Service/DocumentStorageService.php`, `tests/` (nouveau test) + +--- + +## P2 — Consolidation + +### T-010 — Hardening CSRF : header `X-Requested-With` obligatoire sur les écritures +**Pourquoi :** l'auth est par cookie, et la seule protection contre le CSRF (un site malveillant qui fait faire des requêtes à ton navigateur connecté) est l'attribut `SameSite=Lax` du cookie. Une deuxième barrière peu coûteuse : exiger un header custom, qu'un formulaire HTML cross-site ne peut pas envoyer. +**À faire :** +1. Côté front, dans `useApi.ts`, ajouter `'X-Requested-With': 'XMLHttpRequest'` aux headers de toutes les requêtes. +2. Côté back, créer un listener `kernel.request` qui renvoie 403 si la méthode n'est pas GET/HEAD/OPTIONS, que le chemin matche `^/api` (hors `/api/session/profile` pour le login) et que le header est absent. +3. Supprimer la clé morte `csrfToken` de `nuxt.config.ts` (elle laisse croire qu'une protection CSRF existe). +4. Adapter les tests API (`AbstractApiTestCase`) pour envoyer le header. +**Fichiers :** `frontend/app/composables/useApi.ts`, `src/EventListener/` (nouveau), `frontend/nuxt.config.ts`, `tests/AbstractApiTestCase.php` + +### T-011 — Test fonctionnel : l'ID de session change au login +**Pourquoi :** la protection automatique contre la fixation de session est désactivée (`session_fixation_strategy: none`, choix documenté pour la SPA) et compensée par un `$session->migrate(true)` manuel au login. Si quelqu'un supprime ce `migrate` un jour, plus rien ne protège — un test doit le verrouiller. +**À faire :** écrire un test API qui : récupère le cookie de session avant login → se loggue → vérifie que l'ID de session a changé. Ajouter un commentaire dans `SessionProfileController::activateProfile()` pointant vers `security.yaml:5`. +**Fichiers :** `tests/Api/SessionProfileTest.php` (ou équivalent), `src/Controller/SessionProfileController.php` + +### T-012 — MCP : remplacer le mot de passe par un token Bearer + HTTPS +**Pourquoi :** le mot de passe du profil circule en clair dans les headers de chaque requête MCP (et l'URL configurée est en `http://`). Les headers finissent dans les logs des proxys. Un token dédié révocable, transmis en `Authorization: Bearer`, est le pattern déjà utilisé par Lesstime. +**À faire :** ajouter un champ `mcpToken` (hashé) sur Profile ou une table `api_tokens`, générer via une commande console, adapter `McpHeaderAuthenticator` pour valider le Bearer (garder le rate-limiting), mettre à jour `.mcp.json.example`, et servir `/_mcp` en HTTPS uniquement. +**Fichiers :** `src/Mcp/Security/McpHeaderAuthenticator.php`, `src/Entity/Profile.php` ou nouvelle entité, migration, `.mcp.json.example` + +### T-013 — Maintenance : faire respecter le flag côté backend +**Pourquoi :** le toggle admin écrit `var/maintenance`, mais seul le **frontend** (middleware) le vérifie. Quelqu'un qui appelle l'API directement (curl, MCP, script) passe à travers. Et le second mécanisme (`maintenance.on` lu par nginx, posé par `deploy.sh`) n'est documenté nulle part. +**À faire :** +1. Listener `kernel.request` backend : si `var/maintenance` existe et que l'utilisateur n'est pas admin → 503 JSON (sauf `/api/maintenance/check` et `/api/session/*`). +2. Documenter les deux niveaux (applicatif vs nginx/deploy) dans `DEPLOY.md`. +**Fichiers :** `src/EventListener/` (nouveau), `DEPLOY.md` + +### T-014 — Résoudre l'incohérence provider/email nullable +**Pourquoi :** le user provider Symfony charge les profils par `email`, mais l'email est nullable (profils « kiosque »). Ça marche par chance (l'authenticator charge par ID), mais tout futur usage du provider standard cassera sur ces profils. +**À faire :** décision à prendre — option A : email obligatoire + email technique généré pour les kiosques ; option B : provider custom qui charge par id ou email. Documenter le choix dans `docs/BACKEND.md`. +**Fichiers :** `config/packages/security.yaml`, `src/Entity/Profile.php`, éventuellement `src/Security/` + +### T-015 — Supprimer `@nuxtjs/tailwindcss` +**Pourquoi :** dépendance non utilisée (le projet utilise `@tailwindcss/vite`, Tailwind 4) qui installe en plus un Tailwind 3 parallèle — bloat et risque de conflit de résolution. +**À faire :** +```bash +cd frontend && npm uninstall @nuxtjs/tailwindcss && npm run build && npx nuxi typecheck +``` +**Fichiers :** `frontend/package.json`, `frontend/package-lock.json` + +### T-016 — Mettre à jour la doc de versioning (`VERSION` → `config/version.yaml`) +**Pourquoi :** `RELEASE.md` (lignes 17, 50, 80, 82) et l'arbre projet de `CLAUDE.md` référencent un fichier `VERSION` qui n'existe plus — la version vit dans `config/version.yaml`. +**À faire :** remplacer toutes les références ; vérifier au passage que la description du footer frontend (« lit VERSION au build ») correspond au mécanisme réel. +**Fichiers :** `RELEASE.md`, `CLAUDE.md` + +### T-017 — Sortir les données client de la racine + gitignore défensif +**Pourquoi :** `customer.json`, `Company (1).json`, `Ensemble simple rotor.pdf` (données client réelles → enjeu RGPD) traînent à la racine, non protégés par le `.gitignore` : un `git add .` distrait les commiterait. +**À faire :** +1. Déplacer ces fichiers hors du dépôt (ex. `~/imports/inventory/`). Supprimer aussi `inventory_prod (2).sql.gz` et le `node_modules/` orphelin de la racine. +2. Ajouter au `.gitignore` racine : + ``` + /node_modules/ + /*.json + /*.pdf + ``` + (les `.json` légitimes du projet sont dans des sous-dossiers ou explicitement trackés — `composer.json` etc. restent suivis car déjà trackés ; pour les nouveaux, `git add -f` reste possible). +**Fichiers :** `.gitignore`, racine du projet + +### T-018 — Uniformiser la gestion d'erreur frontend (état d'erreur au lieu de `console.error`) +**Pourquoi :** en cas d'échec de chargement, le pattern actuel est `console.error(...)` puis la page s'affiche à moitié vide, sans message. L'utilisateur ne sait pas que quelque chose a raté ni comment réessayer. +**À faire :** règle : `useApi` est le seul à toaster ; les composables exposent un `error: Ref` que la page affiche (bandeau avec bouton Réessayer). Commencer par les 3 pages principales : détail machine (`useMachineDetailData.ts:372,385`), détail composant, détail pièce. Étendre ensuite au reste. +**Fichiers :** `frontend/app/composables/useMachineDetailData.ts`, `useComponentEdit.ts`, `usePieceEdit.ts`, pages correspondantes + +### T-019 — Cacher le résultat de `/maintenance/check` (TTL) +**Pourquoi :** chaque navigation d'un non-admin déclenche un appel API pour vérifier la maintenance — de la latence sur toutes les transitions de page pour un état qui ne change presque jamais. +**À faire :** dans `profile.global.ts`, stocker le résultat dans un `useState` avec timestamp et ne re-fetcher que si > 60 s. +**Fichiers :** `frontend/app/middleware/profile.global.ts` + +### T-020 — Détracker `infra/dev/.env.docker.local` + fournir un `.example` +**Pourquoi :** le fichier est dans le `.gitignore` mais a été commité avant l'ajout de la règle — git continue donc de le suivre. Chaque dev qui le modifie crée du diff, et ses secrets (même de dev) sont versionnés. +**À faire :** +```bash +cp infra/dev/.env.docker.local infra/dev/.env.docker.local.example +# Dans le .example : remplacer les valeurs par des placeholders +# et supprimer les variables JWT_* (inutilisées, cf. T-023) +git rm --cached infra/dev/.env.docker.local +git add infra/dev/.env.docker.local.example +``` +Mettre à jour le README (section installation) pour mentionner la copie du `.example`. +**Fichiers :** `infra/dev/.env.docker.local`, `infra/dev/.env.docker.local.example` (nouveau), `README.md` + +--- + +## P3 — Nice to have + +### T-021 — Décision métier : commentaires (et pièces jointes) en `ROLE_VIEWER` ? +**Pourquoi :** seule écriture accessible aux lecteurs. Si c'est voulu (« tout le monde commente »), le documenter dans `docs/BACKEND.md` ; sinon passer à `ROLE_GESTIONNAIRE` (`CommentController.php:33`). +**Fichiers :** `src/Controller/CommentController.php`, `docs/BACKEND.md` + +### T-022 — Revoir les pagination max (2000/1000/500) +**Pourquoi :** `Constructeur` (2000), `ConstructeurCategorie` (1000), `Document` (500) — vérifier le besoin réel du front et redescendre, ou commenter pourquoi. +**Fichiers :** `src/Entity/Constructeur.php`, `src/Entity/ConstructeurCategorie.php`, `src/Entity/Document.php` + +### T-023 — Nettoyer les variables JWT et les secrets `changeme` +**Pourquoi :** `JWT_SECRET_KEY`/`JWT_PUBLIC_KEY`/`JWT_PASSPHRASE` ne servent à rien (auth session). `APP_SECRET=changeme_…` mérite une vraie valeur aléatoire locale. (Fusionne naturellement avec T-020.) +**Fichiers :** `infra/dev/.env.docker.local` + +### T-024 — Supprimer la config morte de `nuxt.config.ts` +**Pourquoi :** `csrfToken`, `requestTimeout`, `enableDebug`, `enableAnalytics`, `logLevel` ne sont consommés nulle part — de la config qui ment. (`csrfToken` est déjà traité par T-010.) +**Fichiers :** `frontend/nuxt.config.ts:56-59` + +### T-025 — Nettoyer le Dockerfile dev (blocs Oracle/IMAP/MySQL commentés) +**Pourquoi :** restes d'un template générique sans rapport avec un projet PostgreSQL. +**Fichiers :** `infra/dev/Dockerfile:48-53,82-100` + +### T-026 — Renommer les 3 utils `.js` en `.ts` +**Pourquoi :** `documentPreview.js`, `fileIcons.js`, `printTemplates/machineReport.js` sont importés depuis du TypeScript sans types. Tâche mécanique (bon candidat pour Codex). +**Fichiers :** `frontend/app/utils/documentPreview.js`, `frontend/app/utils/fileIcons.js`, `frontend/app/utils/printTemplates/machineReport.js` + +### T-027 — Ajouter `nosniff` + `CSP: sandbox` à `download()` +**Pourquoi :** `serve()` envoie ces deux headers de protection, `download()` non — asymétrie gratuite. +**À faire :** copier les deux `headers->set(...)` de `serve()` dans `download()` (`DocumentServeController.php:110-116`). +**Fichiers :** `src/Controller/DocumentServeController.php` + +### T-028 — Smoke test de l'image Docker avant push +**Pourquoi :** `build-docker.yml` pousse `latest` sans vérifier que l'image démarre. +**À faire :** entre build et push : `docker run --rm gitea.malio.fr/malio-dev/inventory:${{ gitea.ref_name }} php bin/console about`. +**Fichiers :** `.gitea/workflows/build-docker.yml` + +> **Hors tickets :** la dette d'architecture (smartMatch dupliqué, double flush, `pendingStructure`, God controller à 1121 lignes, `any` ×179…) a déjà son plan d'action chiffré dans `docs/REVIEW_ARCHITECTURE.md`. Recommandation forte : exécuter sa **Phase 1** (4 corrections effort S, sans impact d'interface) avant qu'elle ne prenne encore 3 mois — voir `REVIEW.md` §4. + +--- + +## Résumé + +| Priorité | Tickets | Estimation | +|----------|---------|------------| +| **P0** | T-001 à T-003 | ~2h (+ rotations de secrets) | +| **P1** | T-004 à T-009 | ~1,5 j | +| **P2** | T-010 à T-020 | ~2,5 j | +| **P3** | T-021 à T-028 | ~0,5 j | +| **Total** | 28 tickets | ~5 j | + +> Commence par **T-001** — tant que les secrets ne sont pas révoqués, tout le reste est secondaire. +> Pour chaque ticket, fais un commit dédié avec le numéro dans le message (ex. `fix(T-001) : retirer .mcp.json du dépôt`).