diff --git a/.gitignore b/.gitignore index ae28bf2..d8a215f 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ dist # Secrets .env .env.local +.env.production .env.production.local .env.development.local .env.test diff --git a/.kilo/plans/remediation-plan.md b/.kilo/plans/remediation-plan.md new file mode 100644 index 0000000..46c6232 --- /dev/null +++ b/.kilo/plans/remediation-plan.md @@ -0,0 +1,320 @@ +# Plan de remédiation — PC Builder + +Basé sur la revue de code du 2026-07-23. Objectif : corriger les failles de +sécurité critiques avant toute mise en production, puis assainir la base du +code (majeur), puis traiter les points mineurs/qualité. + +--- + +## Phase 1 — CRITIQUE (bloquant avant prod) + +### 1.1 IDOR sur `update`/`destroy` (Computers & Components) + +**Problème** : `computers_controller.ts` (`update` L92-103, `destroy` L105-110) +et `components_controller.ts` (`destroy` L77-82) chargent la ressource via +`findOrFail(id)` sans filtrer par `userId`. `destroy` des deux controllers +n'authentifie même pas l'utilisateur (`auth.authenticate()` absent). N'importe +quel utilisateur connecté peut modifier/supprimer les PC ou composants d'un +autre utilisateur. + +**Remédiation** : +- `computers_controller.ts#update` : ajouter `auth.authenticate()`, remplacer + `Computer.findOrFail(computerId)` par + `Computer.query().where('user_id', user.id).andWhere('id', computerId).firstOrFail()` + (même pattern que `show`/`edit`). +- `computers_controller.ts#destroy` : ajouter `auth.authenticate()`, même + pattern de query scopée par `user_id`. +- `components_controller.ts#destroy` : ajouter `auth.authenticate()`, remplacer + `Component.findOrFail(componentId)` par une query scopée + `where('userId', user.id).andWhere('id', componentId).firstOrFail()`. +- Vérifier les routes (`start/routes.ts`) : `middleware.auth()` est déjà + appliqué sur les groupes `/computers` et `/components`, donc l'utilisateur + est authentifiable dans le contexte — juste manquant l'appel explicite + + le filtre. + +**Fichiers impactés** : +- `app/controllers/computers_controller.ts` +- `app/controllers/components_controller.ts` + +### 1.2 IDOR transitif via `handleComponents` / `handlePictures` + +**Problème** : même après la 1.1, `component_service.ts#handleComponents` +(L11, L18) et `picture_service.ts#handlePictures` (L22, L47) font +`findOrFail(id)` sans vérifier que le composant/l'image appartient à +l'utilisateur courant. Un attaquant authentifié peut poster un `componentsId` +contenant l'ID d'un composant appartenant à un autre utilisateur pour +se l'approprier (`component.computerId = computer.id`). + +**Remédiation** : +- `handleComponents(componentsId, computer, userId)` : ajouter le paramètre + `userId`, et scoper les requêtes `Component.query().where('userId', userId).andWhere('id', id).firstOrFail()` + au lieu de `Component.findOrFail(id)`. Si un ID demandé n'appartient pas à + l'utilisateur, lever une erreur explicite (404 ou 403) plutôt que d'échouer + silencieusement. +- `handlePictures` : scoper `ComputerPicture.findOrFail(picture.id)` par + `computerId` du computer courant (`.where('computerId', computer.id)`), pour + empêcher de manipuler des images d'un autre computer/utilisateur. +- Mettre à jour les appels dans `computers_controller.ts#store` et `#update` + pour passer `user.id`. + +**Fichiers impactés** : +- `app/services/component_service.ts` +- `app/services/picture_service.ts` +- `app/controllers/computers_controller.ts` (appels à `handleComponents`) + +### 1.3 Secrets commités : `.env.production` + +**Problème** : `.env.production` est suivi par git et contient `APP_KEY`, +`DB_PASSWORD`, `DB_USER`, `DB_DATABASE` en clair (commit `c822872`). Le CI +(`.gitea/workflows/build.yml`) le lit et le déploie tel quel sur le NAS. + +**Remédiation** (à faire avec le user — actions sensibles) : +1. **Révoquer/régénérer immédiatement** les secrets réels côté NAS : + nouveau `APP_KEY` (`node ace generate:key`), nouveau mot de passe MySQL, + éventuellement recréer l'utilisateur DB. +2. Ajouter `.env.production` au `.gitignore` (garder uniquement + `.env.production.example` comme gabarit, déjà présent et sans valeurs + réelles). +3. Retirer le fichier du suivi git : `git rm --cached .env.production`. +4. Purger l'historique git du secret (`git filter-repo` ou BFG Repo-Cleaner) + — **à valider avec l'utilisateur** car cela réécrit l'historique partagé + (impact sur les autres clones/branches). +5. Migrer la distribution des secrets de prod vers les **Secrets Gitea Actions** + (comme `secrets.NASPASS`) plutôt qu'un fichier committé : injecter les + variables via `${{ secrets.APP_KEY }}`, `${{ secrets.DB_PASSWORD }}`, etc., + et générer `.env` à la volée dans le job de déploiement. + +**Fichiers impactés** : +- `.gitignore` +- `.gitea/workflows/build.yml` +- `.env.production` (suppression du suivi) +- Historique git (opération destructive, confirmation utilisateur requise) + +### 1.4 Secrets en ligne de commande dans le pipeline CI (NASPASS) + +**Problème** : `.gitea/workflows/build.yml` L46-56 utilise `sshpass -p` et +`sudo -S` avec le secret NAS directement en argument de commande shell — +risque d'exposition dans les logs/process list. + +**Remédiation** : +- Remplacer l'authentification par mot de passe par une **clé SSH dédiée** + stockée en secret Gitea (`secrets.NAS_SSH_KEY`), utilisée via + `ssh -i <(echo "$NAS_SSH_KEY")` ou `webfactory/ssh-agent`-like action. +- Configurer un accès `sudo` sans mot de passe restreint aux commandes + nécessaires (`visudo` avec `NOPASSWD` limité aux scripts `setup-db.sh`, + `docker compose`, `docker exec`) plutôt que de faire transiter le mot de + passe utilisateur via `sudo -S`. +- À défaut de refonte complète immédiate, au minimum masquer la commande + (`set +x` explicite, éviter `-x` en debug) et documenter le risque résiduel. + +**Fichiers impactés** : +- `.gitea/workflows/build.yml` +- Configuration NAS (hors dépôt — à faire manuellement) + +### 1.5 Pas de contrainte d'unicité applicative pour les composants "uniques" + +**Problème** : `ComponentType.unique` (ex: un seul CPU par PC) n'est vérifié +nulle part côté serveur. Un POST direct sur `/computers` ou `/computers/:id` +avec plusieurs composants du même type "unique" dans `componentsId` est accepté. + +**Remédiation** : +- Dans `handleComponents`, après résolution des composants à ajouter, grouper + par `typeId` (en préchargeant `type.unique`) et rejeter (erreur 422/exception + métier) si plus d'un composant du même type "unique" seraient attachés au + computer simultanément. +- Ajouter un test automatisé couvrant ce cas (voir Phase 4). + +**Fichiers impactés** : +- `app/services/component_service.ts` + +--- + +## Phase 2 — MAJEUR (avant la prochaine itération fonctionnelle) + +### 2.1 Redirections relatives fragiles + +**Problème** : `computers_controller.ts#store` (L46) fait +`response.redirect('computers/')` et `#update` (L102) fait +`response.redirect(`${computer.id}`)` — chemins relatifs incohérents avec +le reste du code (`/components`, `/auth/login`, etc.). + +**Remédiation** : remplacer par des chemins absolus explicites : +- `store` → `response.redirect('/computers')` +- `update` → `response.redirect(`/computers/${computer.id}`)` + +### 2.2 Incohérence `request.all()` vs `request.validateUsing()` + +**Problème** : `components_controller.ts` et `computers_controller.ts` +utilisent `request.all()` + `xxxValidator.validate(data)` au lieu du helper +standard `request.validateUsing(validator)` utilisé par +`session_controller.ts`/`new_account_controller.ts`. Risque d'incohérence +sur le flash des erreurs de validation Inertia (`inputErrorsBag`). + +**Remédiation** : uniformiser tous les controllers sur +`await request.validateUsing(xxxValidator)`, supprimer les appels manuels à +`.validate()` sur `request.all()`. + +**Fichiers impactés** : +- `app/controllers/computers_controller.ts` (store, update) +- `app/controllers/components_controller.ts` (store, update) + +### 2.3 Validateurs d'inscription dupliqués et incohérents + +**Problème** : `app/validators/Auth/register.ts` (`registerValidator`) n'est +utilisé nulle part (code mort) ; `new_account_controller.ts` utilise +`signupValidator` de `app/validators/user.ts`. Les deux ont des règles +différentes sur `name` (obligatoire minLength 3 vs nullable sans contrainte). + +**Remédiation** : +- Supprimer `app/validators/Auth/register.ts` (code mort), ou le fusionner + si une seconde route d'inscription est prévue. +- Décider de la règle métier définitive sur `name` (obligatoire ou non) dans + `signupValidator` et l'appliquer. + +### 2.4 Formulaire `computers/create.vue` ne se soumet pas via Enter / pas d'état de chargement + +**Problème** : le bouton "Create" est un `` externe au +`