Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 Matthieu
|
bfd77a1ca1 |
@@ -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).
|
||||||
+343
@@ -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": "<PROFILE_ID>",
|
||||||
|
"X-Profile-Password": "<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 <url-du-repo>` ; 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 `<mot-de-passe-prod>` (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<string | null>` 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 <CHANGE_ME>
|
||||||
|
# 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`).
|
||||||
+1
-1
@@ -1,2 +1,2 @@
|
|||||||
parameters:
|
parameters:
|
||||||
app.version: '1.9.48'
|
app.version: '1.9.47'
|
||||||
|
|||||||
Reference in New Issue
Block a user