Files
pc_builder/.kilo/plans/remediation-plan.md
T
Kevin L'Hereec 3cc189c534 fix(security): close IDOR on computers/components update & destroy
- Scope update/destroy queries by userId (Computers & Components), matching
  the pattern already used in index/show/edit
- Add missing auth.authenticate() calls on destroy actions
- handleComponents now requires userId and scopes component lookups to the
  current user, preventing cross-user component takeover
- handleComponents rejects attaching more than one component of the same
  'unique' type to a computer (new InvalidComponentSelectionException, 422)
- handlePictures scopes picture lookups by computerId
- destroy(Computer) now detaches its components (computerId = null) instead
  of losing them to cascade delete, and removes the computer's uploads dir
- Switch store/update actions to request.validateUsing() for consistency
- Fix relative redirects to absolute paths (/computers, /computers/:id)
- Untrack .env.production and add it to .gitignore (secrets must not be
  committed); local file kept on disk
2026-07-23 10:53:13 +02:00

15 KiB

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 <Link method="post"> externe au <form> (pas de @submit.prevent), contrairement à components/create.vue qui utilise useForm + @submit.prevent correctement.

Remédiation : refactorer computers/create.vue pour utiliser useForm() d'Inertia avec @submit.prevent="form.post('/computers')", afficher form.processing sur le bouton (état de chargement), harmoniser avec le pattern de components/create.vue.

2.5 origin fantôme dans components/edit.vue

Problème : la page lit props.origin et le renvoie dans le payload, mais components_controller.ts#edit ne le partage jamais dans inertia.render(...).

Remédiation : soit supprimer complètement origin du formulaire et du type de props si la fonctionnalité est abandonnée, soit l'implémenter proprement si un besoin réel existe (ex: retour à la page d'origine après modification). À trancher avec l'utilisateur — proposition : suppression (simplification), sauf avis contraire.

2.6 Suppression d'un Computer : cascade destructive sur les composants + fichiers orphelins

Problème : destroy d'un Computer supprime en cascade DB ses Component liés au lieu de les détacher (computerId = null), ce qui fait perdre définitivement les composants de l'utilisateur. De plus, les fichiers images physiques (uploads/computer_X/...) ne sont jamais nettoyés lors d'un destroy de computer (seulement lors d'un retrait explicite via update).

Remédiation :

  • Dans computers_controller.ts#destroy, avant computer.delete() : détacher les composants (UPDATE components SET computer_id = NULL WHERE computer_id = ?) et supprimer physiquement le dossier uploads/computer_${id} (+ les lignes computer_pictures, qui elles peuvent rester en cascade DB puisque les fichiers sont supprimés en même temps).
  • Vérifier/adapter la contrainte onDelete('CASCADE') sur components.computer_id dans la migration si on souhaite un SET NULL réellement porté par la DB plutôt qu'un traitement applicatif (nécessiterait une migration corrective).

Fichiers impactés :

  • app/controllers/computers_controller.ts
  • Éventuellement une nouvelle migration si onDelete doit changer.

2.7 Duplication ComponentsViewer.vue / ComputersViewer.vue

Remédiation : extraire un composant générique FilterDropdown.vue (props : liste d'options {id, label}, v-model sur l'état de filtre, slot ou prop pour le libellé du bouton) réutilisé dans les deux vues, pour éliminer la duplication du markup dropdown + logique toggleDropdown.

2.8 Composants "widgets" orphelins et fonctionnalité galerie incomplète

Problème : ComponentEditor.vue, ComputerComponentsTable.vue, SimpleGallery.vue, AuthErrorAlert.vue ne sont référencés par aucune page active. La gestion de galerie photo (SimpleGallery.vue + picture_service.ts) n'est appelée par aucun formulaire actuel (computers/create.vue/edit.vue n'ont pas de champ pictures).

Remédiation — décision à prendre avec l'utilisateur :

  • Option A (simplification) : supprimer ces composants orphelins et le code backend de gestion d'images s'ils ne sont plus dans la roadmap.
  • Option B (finaliser la fonctionnalité) : intégrer SimpleGallery.vue dans computers/create.vue/edit.vue pour rendre l'upload d'images réellement fonctionnel, et réintégrer ComponentEditor.vue (gestion propre des composants uniques via select) à la place des checkboxes brutes actuelles — ce qui réglerait aussi une partie du problème 1.5 côté UX.

Phase 3 — MINEUR (qualité, hygiène)

3.1 Sécurité de configuration

  • config/session.ts L45 : remplacer secure: false par secure: app.inProduction.
  • config/shield.ts L12 : activer une CSP de base (csp.enabled: true) avec des directives minimales (default-src 'self', autoriser les sources Vite en dev).

3.2 Typage frontend

  • Remplacer les any dans computers/*.vue et components/*.vue par les types réels (Computer, Component, ComputerState, ComponentType importés depuis #models/* comme le fait déjà ComponentsViewer.vue).
  • Corriger/aligner inertia/pages/types/UITypes.ts avec le schéma backend réel (typeId au lieu de componentTypeId, retirer componentStateId inexistant) ou supprimer ce fichier si les vrais modèles Lucid suffisent.

3.3 Encodage UTF-8 cassé (mojibake)

  • Corriger l'encodage dans ComponentsViewer.vue, ComponentEditor.vue, ComputerComponentsTable.vue, TableComponent.vue (caractères ├®, Ôé¼ etc. à réécrire en UTF-8 propre : é, à, ô, €).

3.4 Accessibilité

  • Corriger les doublons d'id HTML dans ComputersViewer.vue (L82/94, deux fois id="used").
  • Ajouter aria-expanded/aria-haspopup sur les boutons de dropdown filtres.
  • Remplacer CustomSelect.vue par un <select> natif stylé, ou ajouter les rôles ARIA manquants (role="listbox", navigation clavier).

3.5 Nettoyage divers

  • Remplacer console.warn dans computer.ts#cost par un logger applicatif ou un comportement documenté.
  • Déplacer/supprimer MIGRATION_PAGES_SUMMARY.md, MIGRATION_COMPLETE.md, MIGRATION_CHECKLIST.md (artefacts temporaires) vers docs/ ou les supprimer une fois la migration validée.

Phase 4 — Tests de non-régression

Pour valider les correctifs de sécurité (Phase 1), ajouter des tests fonctionnels Japa :

  • Un utilisateur A ne peut pas PUT/DELETE un computer/component appartenant à l'utilisateur B (attend 404/403).
  • Un utilisateur ne peut pas attacher via componentsId un composant appartenant à un autre utilisateur.
  • Impossible d'attacher deux composants du même type "unique" à un même computer.
  • Après suppression d'un computer, ses composants redeviennent disponibles (computerId = null) au lieu d'être supprimés.

Fichiers : tests/functional/computers.spec.ts, tests/functional/components.spec.ts (à créer si absents).


Ordre d'exécution proposé

  1. Phase 1 entièrement (sécurité bloquante), avec confirmation utilisateur explicite avant toute réécriture d'historique git (1.3) ou changement de config NAS (1.4).
  2. Phase 4 (tests) en parallèle de la Phase 1 pour prouver les correctifs.
  3. Phase 2 (majeur), avec un point de décision utilisateur sur 2.5 et 2.8 (garder/supprimer/finaliser des fonctionnalités).
  4. Phase 3 (mineur) en dernier, en petits commits séparés par thème (config sécurité / typage / encodage / accessibilité / nettoyage fichiers).

Points nécessitant une décision explicite de l'utilisateur avant exécution

  • 1.3 : confirmer la réécriture de l'historique git pour purger le secret, et confirmer que les secrets réels (APP_KEY, DB_PASSWORD) seront bien régénérés côté NAS avant/juste après.
  • 1.4 : confirmer l'accès pour reconfigurer l'authentification SSH/sudo sur le NAS (hors du dépôt, nécessite un accès serveur).
  • 2.5 : garder, implémenter ou supprimer le champ origin.
  • 2.8 : supprimer les composants orphelins (Option A) ou finaliser la fonctionnalité galerie/éditeur de composants uniques (Option B).