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
This commit is contained in:
Kevin L'Hereec committed 2026-07-23 10:53:13 +02:00
1 parent ec6734f491
commit 3cc189c534
7 files changed
+417 -24

No files matched your search

+1
View File
@@ -9,6 +9,7 @@ dist
# Secrets
.env
.env.local
.env.production
.env.production.local
.env.development.local
.env.test
+320
View File
@@ -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 `<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).
+8 -6
View File
@@ -18,9 +18,8 @@ export default class ComponentsController {
}
async store({ request, response, auth }: HttpContext) {
const data = request.all()
const user = await auth.authenticate()
const { quantity, ...componentData } = await storeComponentValidator.validate(data)
const { quantity, ...componentData } = await request.validateUsing(storeComponentValidator)
await ComponentType.findOrFail(componentData.typeId)
for (let i = 0; i < quantity; i++) {
await Component.create({
@@ -59,9 +58,8 @@ export default class ComponentsController {
}
async update({ params, auth, request, response }: HttpContext) {
const data = request.all()
const componentData = await updateComponentValidator.validate(data)
const user = await auth.authenticate()
const componentData = await request.validateUsing(updateComponentValidator)
const component = await Component.query()
.where('userId', user.id)
.andWhere('id', params.id)
@@ -74,9 +72,13 @@ export default class ComponentsController {
return response.redirect('/components')
}
async destroy({ params, response }: HttpContext) {
async destroy({ params, auth, response }: HttpContext) {
const user = await auth.authenticate()
const componentId = params.id
const component = await Component.findOrFail(componentId)
const component = await Component.query()
.where('userId', user.id)
.andWhere('id', componentId)
.firstOrFail()
await component.delete()
return response.redirect('/components')
}
+27 -12
View File
@@ -1,4 +1,5 @@
import type { HttpContext } from '@adonisjs/core/http'
import { existsSync, rmSync } from 'node:fs'
import Computer from '#models/computer'
import ComputerState from '#models/computer_state'
import Component from '#models/component'
@@ -33,17 +34,17 @@ export default class ComputersController {
}
async store({ auth, request, response }: HttpContext) {
const data = request.all()
const user = await auth.authenticate()
const { pictures, componentsId, ...computerData } = await storeComputerValidator.validate(data)
const { pictures, componentsId, ...computerData } =
await request.validateUsing(storeComputerValidator)
await ComputerState.findOrFail(computerData.stateId)
const computer = await Computer.create({
...computerData,
userId: user.id,
})
await handleComponents(componentsId as number[], computer)
await handleComponents(componentsId as number[], computer, user.id)
await handlePictures(pictures as Image[], request, computer)
return response.redirect('computers/')
return response.redirect('/computers')
}
async show({ auth, inertia, params }: HttpContext) {
@@ -89,22 +90,36 @@ export default class ComputersController {
return inertia.render('computers/edit', { computer: computer.serialize(), availableComponents: availableComponents.map((c) => c.serialize()), states: states.map((s) => s.serialize()) })
}
async update({ params, request, response }: HttpContext) {
async update({ auth, params, request, response }: HttpContext) {
const user = await auth.authenticate()
const computerId = params.id
const { pictures, componentsId, ...computerData } = await updateComputerValidator.validate(
request.all()
const { pictures, componentsId, ...computerData } = await request.validateUsing(
updateComputerValidator
)
const computer = await Computer.findOrFail(computerId)
const computer = await Computer.query()
.where('user_id', user.id)
.andWhere('id', computerId)
.firstOrFail()
computer.merge(computerData)
await computer.save()
await handleComponents(componentsId as number[], computer)
await handleComponents(componentsId as number[], computer, user.id)
await handlePictures(pictures as Image[], request, computer)
return response.redirect(`${computer.id}`)
return response.redirect(`/computers/${computer.id}`)
}
async destroy({ params, response }: HttpContext) {
async destroy({ auth, params, response }: HttpContext) {
const user = await auth.authenticate()
const computerId = params.id
const computer = await Computer.findOrFail(computerId)
const computer = await Computer.query()
.where('user_id', user.id)
.andWhere('id', computerId)
.firstOrFail()
const uploadsDir = `uploads/computer_${computer.id}`
await computer.related('components').query().update({ computerId: null })
if (existsSync(uploadsDir)) {
rmSync(uploadsDir, { recursive: true, force: true })
}
await computer.delete()
return response.redirect('/computers')
}
@@ -0,0 +1,17 @@
import { Exception } from '@adonisjs/core/exceptions'
import type { HttpContext } from '@adonisjs/core/http'
/**
* Thrown when a component selection is invalid, either because:
* - the referenced component does not belong to the current user, or
* - more than one component of the same "unique" type is being attached
* to the same computer at once.
*/
export default class InvalidComponentSelectionException extends Exception {
static status = 422
static code = 'E_INVALID_COMPONENT_SELECTION'
handle(error: this, { response }: HttpContext) {
return response.status(error.status).send({ errors: [{ message: error.message }] })
}
}
+36 -4
View File
@@ -1,21 +1,53 @@
import Component from '#models/component'
import Computer from '#models/computer'
import InvalidComponentSelectionException from '#exceptions/invalid_component_selection_exception'
async function handleComponents(componentsId: number[], computer: Computer) {
async function handleComponents(componentsId: number[], computer: Computer, userId: number) {
const currentComponents = await computer.related('components').query()
const currentComponentsIds = currentComponents.map((component) => component.id)
const componentsToAdd = componentsId.filter((id) => !currentComponentsIds.includes(id))
const componentsToRemove = currentComponentsIds.filter((id) => !componentsId.includes(id))
const componentsToAddRecords = await Promise.all(
componentsToAdd.map((id: number) =>
Component.query().where('userId', userId).andWhere('id', id).preload('type').firstOrFail()
)
)
// Ensure at most one component of each "unique" type ends up attached to
// the computer once the additions/removals are applied.
const remainingComponentIds = currentComponentsIds.filter(
(id) => !componentsToRemove.includes(id)
)
const remainingComponents = remainingComponentIds.length
? await Component.query().whereIn('id', remainingComponentIds).preload('type')
: []
const finalComponents = [...remainingComponents, ...componentsToAddRecords]
const uniqueTypeCounts = new Map<number, number>()
for (const component of finalComponents) {
if (!component.type.unique) continue
const count = (uniqueTypeCounts.get(component.type.id) ?? 0) + 1
uniqueTypeCounts.set(component.type.id, count)
if (count > 1) {
throw new InvalidComponentSelectionException(
`Only one component of type "${component.type.name}" can be attached to a computer at a time.`
)
}
}
await Promise.all(
componentsToAdd.map(async (id: number) => {
const component = await Component.findOrFail(id)
componentsToAddRecords.map(async (component) => {
component.computerId = computer.id
await component.save()
})
)
await Promise.all(
componentsToRemove.map(async (id: number) => {
const component = await Component.findOrFail(id)
const component = await Component.query()
.where('userId', userId)
.andWhere('id', id)
.firstOrFail()
component.computerId = null
await component.save()
})
+8 -2
View File
@@ -19,7 +19,10 @@ async function handlePictures(
await Promise.all(
pictures.map(async (picture: Image, index: number) => {
if ('id' in picture) {
const existingPicture = await ComputerPicture.findOrFail(picture.id)
const existingPicture = await ComputerPicture.query()
.where('computerId', computer.id)
.andWhere('id', picture.id)
.firstOrFail()
existingPicture.order = picture.order
await existingPicture.save()
} else {
@@ -44,7 +47,10 @@ async function handlePictures(
await Promise.all(
imagesToRemove.map(async (id: number) => {
const picture = await ComputerPicture.findOrFail(id)
const picture = await ComputerPicture.query()
.where('computerId', computer.id)
.andWhere('id', id)
.firstOrFail()
const filePath = `uploads/computer_${computer.id}/${picture.link.split('/').pop()}`
if (existsSync(filePath)) {
unlinkSync(filePath)