Malio-ops/CODE_REVIEW.md

Code Review - Scripts Serveur (MALIO)

Date : 2026-03-09 Reviewer : Claude (Opus 4.6) Scope : Revue complète de tout le code du dépôt

---

Note : Le fichier CheckStorage/.env contient un webhook Discord en local mais n'est pas commité dans le dépôt (correctement ignoré par le .gitignore). Pas de fuite de secret.

---

1. BackupVaultWarden/backup-vaultwarden.sh

Qualite globale : Bonne

Le script est bien structuré, utilise set -euo pipefail, et gère correctement les erreurs.

Problemes

Severite	Ligne	Description
CRITIQUE	8	Chemin .env en dur (/home/matt/vaultwarden/scripts/...) au lieu d'utiliser $SCRIPT_DIR. Ce script ne fonctionnera que sur la machine de matt.
HAUTE	80-83	Injection de code dans le message Discord. La variable $msg est injectée directement dans du code Python via un heredoc. Si le contenu du backup contient des guillemets triples """ ou du code Python, il sera exécuté.
MOYENNE	7	SCRIPT_DIR est calculé mais jamais utilisé (variable morte).
BASSE	36	WEBHOOK_URL utilise := (valeur par défaut vide) au lieu de :-. Ce n'est pas un bug mais c'est incohérent avec les autres variables qui utilisent :?.

Suggestions

• Ligne 8 : Remplacer par ENV_FILE="${SCRIPT_DIR}/.env" comme dans les autres scripts.
• Ligne 80-83 : Utiliser jq ou python3 -c avec passage par argument au lieu d'interpoler dans un heredoc Python :

  printf '%s' "$msg" | python3 -c 'import sys,json; print(json.dumps({"content": sys.stdin.read()}))' | curl ...
  

• Ajouter une rotation des backups distants (pas de purge des anciens backups actuellement).

---

2. CheckStorage/check-storage.sh

Qualite globale : Faible

Ce script est le moins mature du dépôt. Pas de set -euo pipefail, pas de gestion d'erreurs, et des pratiques fragiles.

Problemes

Severite	Ligne	Description
HAUTE	1	Pas de set -euo pipefail. Le script continue silencieusement en cas d'erreur.
HAUTE	10	Lecture du .env avec `grep
HAUTE	39-45	Injection JSON : le message Discord est construit par interpolation directe dans du JSON. Si une variable contient " ou \, le JSON sera invalide ou pire.
MOYENNE	10	Chemin .env relatif sans cd vers le répertoire du script. Le script cassera s'il est exécuté depuis un autre répertoire (ex: via cron).
MOYENNE	37	Pas de notification quand tout va bien (aucun message si usage < limite). Impossible de savoir si le cron fonctionne.
BASSE	-	Pas de shebang #!/usr/bin/env bash, utilise #!/bin/bash directement (moins portable).
BASSE	-	Pas de logging dans un fichier, seulement echo sur stdout.

Suggestions

• Aligner sur le pattern des scripts RecetteScripts : set -euo pipefail, source .env, SCRIPT_DIR, gestion d'erreurs.
• Utiliser jq ou Python pour construire le JSON Discord de maniere sure.
• Ajouter un chemin absolu vers le .env base sur $SCRIPT_DIR.
• Ajouter un mode --verbose ou un log file.

---

3. RecetteScripts/backup-bdd-recette.sh

Qualite globale : Tres bonne

C'est le script le plus mature du dépôt. Bien structure, bonne gestion d'erreurs, verrou d'execution, messages Discord granulaires.

Problemes

Severite	Ligne	Description
HAUTE	92	export PGPASSWORD : le mot de passe PostgreSQL est expose dans l'environnement. Tout process fils peut le lire (visible dans /proc/*/environ). Preferer un fichier .pgpass avec permissions 600.
HAUTE	106-107	Injection JSON dans discord_send() : la variable $msg est interpolee directement dans le JSON curl. Meme probleme que les autres scripts.
MOYENNE	223	Les noms de dossiers distants (ferme, sirh, inventory, user) sont en dur au lieu d'etre derives de $DBS. Si on ajoute une base dans .env, le dossier distant ne sera pas cree.
MOYENNE	237-239	L'export des "roles" fait en realite un simple SELECT rolname : ca ne sauvegarde que les noms des roles, pas leurs privileges, mots de passe, ou attributs. Utiliser pg_dumpall --roles-only pour un vrai backup des roles.
BASSE	336	exit 2 en fin de script en cas d'erreur partielle : c'est bien, mais pas documente dans le README.
BASSE	85	Le TMP_DIR est sous /tmp : sur un systeme multi-utilisateur, un autre user pourrait pre-creer le dossier (race condition / symlink attack). Utiliser mktemp -d.

Suggestions

• Remplacer export PGPASSWORD par un fichier .pgpass.
• Deriver les dossiers distants de $DBS_ARRAY au lieu de les hardcoder.
• Utiliser pg_dumpall --roles-only pour un vrai export des roles.
• Utiliser mktemp -d pour le repertoire temporaire.

---

4. RecetteScripts/check-statut-recette.sh

Qualite globale : Bonne

Script bien ecrit, bonne separation des responsabilites, gestion propre des erreurs curl.

Problemes

Severite	Ligne	Description
HAUTE	92-94	Injection JSON dans Discord (meme pattern que partout).
MOYENNE	1	set -u sans set -e. Les erreurs non gerees ne stopperont pas le script. C'est voulu (le script doit continuer pour checker tous les sites), mais set -o pipefail manque.
MOYENNE	52	Schema HTTP en dur (http). Pas de support HTTPS. Si les apps passent en HTTPS, le script retournera des redirections 301/302 au lieu de verifier le vrai endpoint.
BASSE	134	Le mktemp pour stderr n'est pas nettoye en cas d'interruption (pas de trap). Fuite mineure de fichiers temp.

Suggestions

• Ajouter le support HTTPS (configurable par URL dans le .env, ou suivre les redirections avec -L).
• Ajouter set -o pipefail.
• Nettoyer le fichier stderr temporaire dans un trap.

---

5. Problemes transversaux

5.1 Injection JSON dans les notifications Discord

Tous les scripts construisent le payload JSON Discord par interpolation de chaines. C'est le probleme le plus repandu.

Pattern actuel (dangereux) :

curl -d "{\"content\":\"$msg\"}" "$WEBHOOK_URL"

Pattern corrige :

jq -n --arg msg "$msg" '{content: $msg}' | curl -d @- ...

Ou si jq n'est pas disponible :

python3 -c "import sys,json; print(json.dumps({'content': sys.argv[1]}))" "$msg" | curl -d @- ...

5.2 Pas de rotation des sauvegardes

Aucun script ne gere la purge des anciens backups sur le serveur distant. L'espace disque distant finira par se remplir.

Suggestion : ajouter une commande SSH pour supprimer les backups de plus de N jours :

ssh "$REMOTE" "find '$REMOTE_DIR' -name '*.dump' -mtime +30 -delete"

5.3 Incoherence de style entre les scripts

Aspect	check-storage	backup-vaultwarden	backup-bdd-recette	check-statut-recette
set -euo pipefail	Non	Oui	Oui	Partiel (set -u)
Chargement .env	grep|cut	source (chemin dur)	source (SCRIPT_DIR)	source (SCRIPT_DIR)
Logging	echo	tee + fichier	tee + fichier	tee + fichier
Gestion erreurs	Aucune	fail()	Granulaire	log_line()
Shebang	#!/bin/bash	#!/usr/bin/env bash	#!/usr/bin/env bash	#!/usr/bin/env bash

check-storage.sh est clairement en retard sur les conventions adoptees dans les autres scripts.

5.4 README du BackupVaultWarden desynchronise

Le README de BackupVaultWarden decrit l'ancien code (lecture .env avec grep | cut) alors que le script actuel utilise source. La documentation ne correspond plus au code.

---

6. .gitignore

Problemes

Le .gitignore est trop complexe et redondant. Les regles se contredisent :

.env
.env.*
!.env.example
!.env.exemple
RecetteScripts/.env    # redondant avec .env
CheckStorage/.env      # redondant avec .env

Les regles fonctionnent correctement (les fichiers .env ne sont pas commites), mais la redondance rend le fichier difficile a maintenir.

Suggestion : simplifier le .gitignore :

# Secrets
.env
!.env.exemple
!.env.example

---

7. Resume et priorites

Actions immediates (a faire maintenant)

1. Corriger le chemin .env en dur dans backup-vaultwarden.sh (ligne 8)
2. Corriger l'injection JSON dans tous les scripts (utiliser jq)

Actions a court terme

3. Remonter check-storage.sh au niveau des autres scripts (set -euo, source .env, SCRIPT_DIR, logging)
4. Remplacer export PGPASSWORD par .pgpass dans backup-bdd-recette.sh
5. Utiliser pg_dumpall --roles-only au lieu du simple SELECT rolname

Actions a moyen terme

8. Ajouter la rotation des backups distants
9. Ajouter le support HTTPS dans check-statut-recette.sh
10. Mettre a jour le README de BackupVaultWarden
11. Simplifier le .gitignore

---

Revue generee par Claude (Opus 4.6) - Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com