Dette passe 1, lot L2 : contrats partagés dans aquashared et client MQTT du boîtier #2

Open
Thomas wants to merge 4 commits from pr/dette-l2 into main
Member

Objet

Lot L2 de la passe 1 de dette : les contrats MQTT échangés entre le boîtier et le serveur sont définis une seule fois dans aquashared, et le client MQTT du boîtier est reconstruit sur ces contrats. Le format de référence de firstco est l'enveloppe {"boitier": {...}, "client": {...}} du fichier config_boitier.json de la passerelle historique.

Dettes traitées et preuves

Dette Correction Preuve (à la tête du lot)
N3, format firstco le boîtier publie le payload de référence lu dans son fichier d'identité ; mark_firstco_done ne modifie que boitier.first_connect ; écriture atomique (fichier temporaire puis renommage) aquaboitier-embedded/src/firstco/mod.rs, src/persist.rs ; avant correction, le test de contrat échouait avec missing field boitier
N6, réessai firstco à chaque ConnAck : abonnement à firstco_ACK-{id} et maj_serv-{id}, puis publication ; réessai jusqu'à un ACK valide, 10 s doublés jusqu'à 300 s aquaboitier-embedded/src/session.rs
N2, accusés et configuration ACK firstco accepté seulement si status = ok et id_boitier égal à l'id du boîtier ; maj_serv : commandes 0 ou 1, version_config décimale canonique, version plus ancienne refusée (y compris après redémarrage) ; refus acquitté par un ACK error src/session.rs, src/config_sync/mod.rs, aquashared/src/config.rs
N5, reconnexion réabonnement à chaque ConnAck, reconnexion avec Backoff de 1 s à 60 s aquaboitier-embedded/src/main.rs
Q5, recommandation 3 client id MQTT_CLIENT_ID s'il est fourni, sinon aquaboitier-{id} ; entrypoint.sh ne force plus le nom d'hôte src/settings.rs, entrypoint.sh
N4, conteneur du boîtier droits de app sur /data et /app/conf, chemins BOITIER_CONFIG_PATH et BOITIER_STATE_PATH uniques ; identité absente : arrêt (code 1), sauf BOITIER_DEV_MODE=1 aquaboitier-embedded/Dockerfile, Dockerfile.prod, src/settings.rs, src/main.rs

Contenu d'aquashared : id (BoitierId, analyse canonique), topics (constantes, enum Topic), provisioning (types firstco déplacés du serveur, FirstConnectAck), config (BoitierConfig, validation, ConfigAck), frame (build_custom_frame déplacé du boîtier), mqtt (limites de taille), backoff. Le serveur et le boîtier réexportent ces types : plus aucune duplication.

Côté serveur, la logique de provisioning n'est pas modifiée : désérialisation par les types partagés, ACK firstco produit par le type partagé (format identique à l'octet près, {"status":"ok","id_boitier":N}).

Tests et résultats

  • TDD : chaque groupe de tests écrit avant le code et vu rouge (18 échecs sur 65 dans aquashared, 22 sur 63 pour la session du boîtier).
  • Ajouts : 64 tests unitaires et 4 d'intégration dans aquashared ; 63 tests unitaires dans le boîtier ; aquaserveur/tests/contract_boitier_serveur.rs, 9 tests de contrat entre crates (le payload produit par la session du boîtier est routé et analysé par le serveur, et inversement pour les ACK et maj_serv).
  • cargo fmt --check et cargo clippy --all-targets --all-features --workspace -D warnings : OK.
  • cargo test --workspace : 233 passés, 0 échec, 39 ignorés (159 passés avant le lot).
  • Tests ignorés sur MariaDB 10.11 jetable (schéma du dump, fixtures 4000 à 4002) : 39 passés, 0 échec.
  • Vérification de bout en bout sur infrastructure jetable (Mosquitto 2.0, serveur, image du boîtier) : provisioning au format de référence, ACK, application et accusé de maj_serv, reconnexion après redémarrage du broker, refus des ACK forgés.

Migrations et actions de déploiement

  • Aucune migration de base.
  • Changement de contrat : mode_boost, generator_state_a et generator_state_b deviennent obligatoires dans maj_serv. Le serveur et le boîtier de ce lot doivent être déployés ensemble.
  • Boîtier de dev : reconstruire l'image (docker compose -f docker-compose.dev.yml up -d --build boitier-dev) pour bénéficier de BOITIER_DEV_MODE=1, puis relancer e2e/boitier/run-all-boitier.sh.

Choix faits

  • Le boîtier publie encore sur firstco (global) : firstco-{id} est défini ici mais routé par le serveur seulement à partir du lot L3.
  • first_connect vaut "ok" après ACK (sémantique « non vide », comme la passerelle).
  • Une configuration refusée est acquittée error : le serveur crée l'alerte 15 (plafond d'insertions prévu en L4).
  • Version égale réappliquée et réacquittée ; seule une version strictement plus ancienne est refusée.
  • Réessai firstco sans limite de tentatives, délai plafonné à 300 s.
  • Mode développement sans identité : attente sans session MQTT, aucune identité fictive créée.

Hors périmètre

Validation canonique de l'id côté serveur et routage par Topic::parse (L3) ; limites de taille côté serveur et broker (L5) ; documentation générale (L9). Inchangés et préexistants : 6 tests de la feature aquaboitier-phase-green (échouent à l'identique sur 674dc89), avertissement [profile.release] du boîtier.

Retour arrière

Revenir les 4 commits du lot (git revert) et redéployer ensemble serveur et boîtier : aucune donnée ni schéma n'est modifié par ce lot.


Pile de PR

Base de cette PR : main. Fusion dans l'ordre de la pile ; apres chaque fusion, vigie.py retarget rebase la PR suivante sur main.

  1. #2 pr/dette-l2 : Dette passe 1, lot L2 : contrats partagés dans aquashared et client MQTT du boîtier (cette PR)
  2. #3 pr/dette-l3 : Dette passe 1, lot L3 : provisioning non destructif et identifiant validé à l'entrée MQTT
  3. #4 pr/dette-l4 : Dette passe 1, lot L4 : mesures, alertes et déduplication
  4. #5 pr/dette-l5 : Dette passe 1, lot L5 : boucle MQTT du serveur, livraison de la configuration et journaux
  5. #6 pr/dette-l4bis : Dette passe 1, lot L4-bis : alertes même si l'écriture échoue, délai unique de 600 s, typage des capteurs, ordre des accusés MQTT
  6. #7 pr/dette-l7 : Dette passe 1, lot L7 : builds reproductibles, dépendances, durcissement des conteneurs
  7. #8 pr/dette-l6 : Dette passe 1, lot L6 : authentification Mosquitto, ACL par compte, healthchecks réels, pile e2e isolée
  8. #9 pr/dette-l8 : Dette passe 1, lot L8 : tests non destructifs, fixtures synthétiques, script de tests sur base jetable

Commits du lot

  • 88b4119 feat: define boitier and server contracts in aquashared
  • 0fe084f refactor: deserialize server messages with the aquashared contracts
  • a55c728 fix: rebuild the boitier MQTT client on the shared contracts
  • 55fc805 fix: give the boitier container a writable state and a single config path
## Objet Lot L2 de la passe 1 de dette : les contrats MQTT échangés entre le boîtier et le serveur sont définis une seule fois dans `aquashared`, et le client MQTT du boîtier est reconstruit sur ces contrats. Le format de référence de `firstco` est l'enveloppe `{"boitier": {...}, "client": {...}}` du fichier `config_boitier.json` de la passerelle historique. ## Dettes traitées et preuves | Dette | Correction | Preuve (à la tête du lot) | |---|---|---| | N3, format `firstco` | le boîtier publie le payload de référence lu dans son fichier d'identité ; `mark_firstco_done` ne modifie que `boitier.first_connect` ; écriture atomique (fichier temporaire puis renommage) | `aquaboitier-embedded/src/firstco/mod.rs`, `src/persist.rs` ; avant correction, le test de contrat échouait avec `missing field boitier` | | N6, réessai `firstco` | à chaque `ConnAck` : abonnement à `firstco_ACK-{id}` et `maj_serv-{id}`, puis publication ; réessai jusqu'à un ACK valide, 10 s doublés jusqu'à 300 s | `aquaboitier-embedded/src/session.rs` | | N2, accusés et configuration | ACK `firstco` accepté seulement si `status = ok` et `id_boitier` égal à l'id du boîtier ; `maj_serv` : commandes 0 ou 1, `version_config` décimale canonique, version plus ancienne refusée (y compris après redémarrage) ; refus acquitté par un ACK `error` | `src/session.rs`, `src/config_sync/mod.rs`, `aquashared/src/config.rs` | | N5, reconnexion | réabonnement à chaque `ConnAck`, reconnexion avec `Backoff` de 1 s à 60 s | `aquaboitier-embedded/src/main.rs` | | Q5, recommandation 3 | client id `MQTT_CLIENT_ID` s'il est fourni, sinon `aquaboitier-{id}` ; `entrypoint.sh` ne force plus le nom d'hôte | `src/settings.rs`, `entrypoint.sh` | | N4, conteneur du boîtier | droits de `app` sur `/data` et `/app/conf`, chemins `BOITIER_CONFIG_PATH` et `BOITIER_STATE_PATH` uniques ; identité absente : arrêt (code 1), sauf `BOITIER_DEV_MODE=1` | `aquaboitier-embedded/Dockerfile`, `Dockerfile.prod`, `src/settings.rs`, `src/main.rs` | Contenu d'`aquashared` : `id` (`BoitierId`, analyse canonique), `topics` (constantes, enum `Topic`), `provisioning` (types `firstco` déplacés du serveur, `FirstConnectAck`), `config` (`BoitierConfig`, validation, `ConfigAck`), `frame` (`build_custom_frame` déplacé du boîtier), `mqtt` (limites de taille), `backoff`. Le serveur et le boîtier réexportent ces types : plus aucune duplication. Côté serveur, la logique de provisioning n'est pas modifiée : désérialisation par les types partagés, ACK `firstco` produit par le type partagé (format identique à l'octet près, `{"status":"ok","id_boitier":N}`). ## Tests et résultats - TDD : chaque groupe de tests écrit avant le code et vu rouge (18 échecs sur 65 dans `aquashared`, 22 sur 63 pour la session du boîtier). - Ajouts : 64 tests unitaires et 4 d'intégration dans `aquashared` ; 63 tests unitaires dans le boîtier ; `aquaserveur/tests/contract_boitier_serveur.rs`, 9 tests de contrat entre crates (le payload produit par la session du boîtier est routé et analysé par le serveur, et inversement pour les ACK et `maj_serv`). - `cargo fmt --check` et `cargo clippy --all-targets --all-features --workspace -D warnings` : OK. - `cargo test --workspace` : 233 passés, 0 échec, 39 ignorés (159 passés avant le lot). - Tests ignorés sur MariaDB 10.11 jetable (schéma du dump, fixtures 4000 à 4002) : 39 passés, 0 échec. - Vérification de bout en bout sur infrastructure jetable (Mosquitto 2.0, serveur, image du boîtier) : provisioning au format de référence, ACK, application et accusé de `maj_serv`, reconnexion après redémarrage du broker, refus des ACK forgés. ## Migrations et actions de déploiement - Aucune migration de base. - Changement de contrat : `mode_boost`, `generator_state_a` et `generator_state_b` deviennent obligatoires dans `maj_serv`. Le serveur et le boîtier de ce lot doivent être déployés ensemble. - Boîtier de dev : reconstruire l'image (`docker compose -f docker-compose.dev.yml up -d --build boitier-dev`) pour bénéficier de `BOITIER_DEV_MODE=1`, puis relancer `e2e/boitier/run-all-boitier.sh`. ## Choix faits - Le boîtier publie encore sur `firstco` (global) : `firstco-{id}` est défini ici mais routé par le serveur seulement à partir du lot L3. - `first_connect` vaut `"ok"` après ACK (sémantique « non vide », comme la passerelle). - Une configuration refusée est acquittée `error` : le serveur crée l'alerte 15 (plafond d'insertions prévu en L4). - Version égale réappliquée et réacquittée ; seule une version strictement plus ancienne est refusée. - Réessai `firstco` sans limite de tentatives, délai plafonné à 300 s. - Mode développement sans identité : attente sans session MQTT, aucune identité fictive créée. ## Hors périmètre Validation canonique de l'id côté serveur et routage par `Topic::parse` (L3) ; limites de taille côté serveur et broker (L5) ; documentation générale (L9). Inchangés et préexistants : 6 tests de la feature `aquaboitier-phase-green` (échouent à l'identique sur `674dc89`), avertissement `[profile.release]` du boîtier. ## Retour arrière Revenir les 4 commits du lot (`git revert`) et redéployer ensemble serveur et boîtier : aucune donnée ni schéma n'est modifié par ce lot. --- ### Pile de PR Base de cette PR : `main`. Fusion dans l'ordre de la pile ; apres chaque fusion, `vigie.py retarget` rebase la PR suivante sur `main`. 1. #2 `pr/dette-l2` : Dette passe 1, lot L2 : contrats partagés dans aquashared et client MQTT du boîtier (cette PR) 2. #3 `pr/dette-l3` : Dette passe 1, lot L3 : provisioning non destructif et identifiant validé à l'entrée MQTT 3. #4 `pr/dette-l4` : Dette passe 1, lot L4 : mesures, alertes et déduplication 4. #5 `pr/dette-l5` : Dette passe 1, lot L5 : boucle MQTT du serveur, livraison de la configuration et journaux 5. #6 `pr/dette-l4bis` : Dette passe 1, lot L4-bis : alertes même si l'écriture échoue, délai unique de 600 s, typage des capteurs, ordre des accusés MQTT 6. #7 `pr/dette-l7` : Dette passe 1, lot L7 : builds reproductibles, dépendances, durcissement des conteneurs 7. #8 `pr/dette-l6` : Dette passe 1, lot L6 : authentification Mosquitto, ACL par compte, healthchecks réels, pile e2e isolée 8. #9 `pr/dette-l8` : Dette passe 1, lot L8 : tests non destructifs, fixtures synthétiques, script de tests sur base jetable ### Commits du lot - `88b4119` feat: define boitier and server contracts in aquashared - `0fe084f` refactor: deserialize server messages with the aquashared contracts - `a55c728` fix: rebuild the boitier MQTT client on the shared contracts - `55fc805` fix: give the boitier container a writable state and a single config path <!-- vigie:stack -->
Add the shared contracts both crates exchange: canonical boitier id
(digits only, no sign, no leading zero, non zero), MQTT topic names with
building and parsing (data, firstco, firstco-{id}, firstco_ACK,
maj_serv, maj_boitier_ACK), firstco payload and its ACK, maj_serv
configuration with range and version checks, maj_boitier_ACK, MPT v2
frame building next to its parser, public epoch validation, MQTT packet
size limits and a bounded backoff.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UH3JcaVWsmFLC6kyEsVL18
The firstco payload types, the maj_serv configuration, the
maj_boitier_ACK payload and its status now come from aquashared instead
of server-local copies. The firstco_ACK payload is produced by the
shared FirstConnectAck type and topic helpers use the shared prefixes.
Provisioning logic is unchanged.

Contract change: mode_boost and generator_state_a/b are now required in
maj_serv, so a missing command is rejected instead of silently meaning
"off" on the boitier.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UH3JcaVWsmFLC6kyEsVL18
N3: the boitier now publishes the reference firstco payload, the
config_boitier.json envelope of the legacy gateway ({"boitier":{...,
"noSerie"},"client":{...}}), which the server parses. The local identity
file uses the same format; marking firstco done only sets
boitier.first_connect and keeps the other fields (atomic write).

N6: firstco_ACK-{id} is subscribed before firstco is published, then
firstco is published again with a doubling delay (10 s up to 300 s)
until a valid ACK arrives.

N2: the firstco ACK must carry status ok and the boitier id; maj_serv
commands must be 0 or 1 and its version must not be older than the
applied one (persisted, survives restarts). Refusals and apply failures
are answered with an error ACK; unreadable payloads get none.

N5: every ConnAck resubscribes to the boitier topics; reconnection uses
a bounded backoff and MQTT requests no longer block the event loop.

Q5: the MQTT client id is derived from the identity (aquaboitier-{id})
unless MQTT_CLIENT_ID is set. A missing or invalid identity stops the
process unless BOITIER_DEV_MODE is set, instead of running as id 0.

Topics, payload types, frame building and the boitier id come from
aquashared. Cross-crate contract tests produce each message with the
boitier code and read it with the server code, and the reverse. The e2e
fixtures follow the reference format and maj_serv versions are numeric.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UH3JcaVWsmFLC6kyEsVL18
fix: give the boitier container a writable state and a single config path
All checks were successful
aquaprocess/revue-statique succes : 9 verifications, 0 bloquant
55fc8050c3
N4: the image gives the app user write access to /data and /app/conf
and sets BOITIER_CONFIG_PATH=/app/conf/config.json and
BOITIER_STATE_PATH=/data/config_state.json, the paths the binary and
the e2e scripts use (the scripts created /conf instead of /app/conf).
The dev compose sets BOITIER_DEV_MODE=1 so the simulator idles without
an identity instead of exiting; outside dev mode it stops.

Q5, recommendation 3: the entrypoint no longer forces
MQTT_CLIENT_ID=aquaboitier-$(hostname); the binary derives the client id
from the identity. The new variables are documented in the README.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UH3JcaVWsmFLC6kyEsVL18
Thomas left a comment

Revue statique Vigie : PR #2

Tete 55fc8050c3, base main (674dc89dc2), 4 commit(s), 39 fichier(s) ajoutes ou modifies.

Statut aquaprocess/revue-statique : success

Constats : 0 bloquant, 4 majeur, 22 mineur, 1 style. Commentaires en ligne : 4 (mode important, plafond 10).

Verifications automatiques

Verification Resultat Detail
cargo fmt --check success aucune difference
cargo clippy -D warnings success aucun avertissement
cargo test --workspace success 233 passes, 0 echec(s), 39 ignores
tests ignores (base jetable) success 39 passes, 0 echec(s) ; pas de broker MQTT (tests sans TEST_MQTT_URL) ; dump charge en 36 s ; 1 fichier(s) de fixtures ; migrations du depot : aucune
cargo audit attention introduite(s) : aucune ; deja presente(s) sur la base : RUSTSEC-2026-0049 (rustls-webpki), RUSTSEC-2026-0098 (rustls-webpki), RUSTSEC-2026-0099 (rustls-webpki), RUSTSEC-2026-0104 (rustls-webpki), RUSTSEC-2026-0258 (h2), RUSTSEC-2026-0285 (rustls), 2 ignoree(s) par audit.toml
semgrep (regles du depot) success 27 fichier(s) ; lignes ajoutees : 77 resultat(s) hors tests, tests : no-unwrap-in-production x4 ; 25 sur lignes inchangees
gitleaks (plage de commits) success 4 commit(s) analyses, aucune fuite
format des commits success 4 commit(s) conformes
emoji dans le code ajoute success 3243 ligne(s) ajoutee(s), aucune

Analyse qualitative

Verdict : aucun bloquant ; 4 constats majeurs à traiter avant la production (anti-retour en arrière sans borne, écriture non durable, republication de firstco qui réécrit les commandes, données client sur un topic global), 9 mineurs et 1 de style. Le lot tient l'essentiel de ce qu'il annonce : contrats centralisés dans aquashared, session du boîtier pure et bien testée, ACK contrôlés.

Sécurité

  • Le broker livré (configs/mosquitto/mosquitto.conf) accepte les clients anonymes sans ACL. Les contrôles N2 protègent donc contre un message mal formé, pas contre un message forgé : une version égale est réappliquée, donc un maj_serv forgé avec la version courante et d'autres commandes passe. C'est cohérent avec le renvoi de l'ACL à L6, mais il faut le dire clairement dans la suite.
  • Nouveau risque introduit par l'anti-retour en arrière : une seule version très grande, acceptée puis persistée, bloque définitivement la réception des configurations légitimes (aquashared/src/config.rs:87). Sans borne haute ni voie de réinitialisation, c'est un déni de service durable.
  • Le boîtier publie maintenant l'objet client (nom, adresse) sur firstco global, lisible par tout abonné à # (aquaboitier-embedded/src/session.rs:178).
  • L'ACK d'erreur recopie la version reçue sans borne, deux fois, et le serveur la stocke en alerte 15 (aquaboitier-embedded/src/session.rs:223).
  • Point positif : l'id canonique (BoitierId::parse) et Topic::parse côté boîtier écartent les alias (maj_serv-04000), et l'ACK firstco n'est accepté que pour le bon boîtier avec status = ok. Aucun secret ni donnée réelle dans les fixtures (valeurs fictives 4990).

Correction

  • write_atomic (aquaboitier-embedded/src/persist.rs:17) n'appelle aucun sync_all : l'atomicité vaut pour un arrêt du processus, pas pour une coupure d'alimentation, alors que le commentaire l'affirme. Un fichier d'identité vide arrête le boîtier.
  • Republication de firstco : l'upsert serveur réécrit modeBoost, generatorStateA/B et updated_at à partir des valeurs figées du fichier d'identité. Le choix de passer opérationnel quand la persistance échoue, avec republication au démarrage suivant (aquaboitier-embedded/src/session.rs:196), peut donc annuler des commandes posées par l'administrateur, que le poller repousse ensuite au boîtier.
  • Boucle principale (aquaboitier-embedded/src/main.rs) : le passage à try_publish/try_subscribe supprime bien le risque d'interblocage, mais un échec n'est que journalisé (main.rs:35), sans nouvel essai avant la reconnexion suivante.
  • Le réessai firstco (10 s doublés jusqu'à 300 s, arrêt hors connexion et à l'ACK) est correct ; il manque une gigue pour éviter la synchronisation de toute une flotte (aquashared/src/backoff.rs:26).
  • État persistant hors volume (aquaboitier-embedded/Dockerfile.prod:73) : la version appliquée et l'identité disparaissent à la recréation du conteneur.
  • Le serveur garde mode_boost: row.mode_boost.unwrap_or(0) (aquaserveur/src/config_poller.rs:53, ligne non modifiée) : le défaut silencieux que le contrat veut supprimer côté boîtier existe toujours, mais il est appliqué côté serveur.
  • Mineurs : parse_broker_url retombe silencieusement sur des défauts (settings.rs:86), processus PID 1 sans gestion de SIGTERM (main.rs:60), version e2e tirée de l'horloge de l'hôte et jamais nettoyée (e2e/boitier/s7-config-sync.sh:37).

Cohérence avec le compte rendu

Affirmations vérifiées sur la tête 55fc805 (références fichier:ligne exactes) :

  1. aquashared/src/id.rs:31 et :42 (BoitierId, parse canonique) : conforme.
  2. aquashared/src/topics.rs:20-25, :63, :107-127 : conforme.
  3. aquashared/src/config.rs:23, :54, :68, :84, :100 ; frame.rs:48, :55, :82, :170 ; mqtt.rs:14-22 ; backoff.rs:16-33 : conforme.
  4. session.rs:29-31, :113, :132, :167, :185, :204 et le comportement annoncé (abonnement avant publication, délais 10 à 300 s sans limite, rien hors connexion, ACK forgés refusés, payload illisible sans ACK) : conforme.
  5. main.rs:56-60, :85, :104, :116 ; settings.rs:18, :57, :70 ; entrypoint.sh:11 ; Dockerfile:86, :98-99 ; Dockerfile.prod:73, :85-86 ; docker-compose.dev.yml:103 ; common-boitier.sh:40 : conforme.
  6. Serveur : provisioning/types.rs:7 et config_sync.rs:16 réexportent les types partagés, firstco_ack_payload en config_sync.rs:94, appel en main.rs:218-219, format {"status":"ok","id_boitier":N} testé à l'octet : conforme.
  7. mark_firstco_done conserve les autres champs : conforme (testé), mais l'ordre des clés et les permissions changent (firstco/mod.rs:89).

Écarts :

  • « Écriture atomique » : partiellement vrai (pas de durabilité, voir persist.rs:17).
  • « Publié tel quel » (compte rendu section 1 et firstco/mod.rs:7) : le payload est resérialisé, les champs non modélisés sont perdus.
  • « Valable après redémarrage » : vrai pour un docker restart, faux pour une recréation du conteneur.
  • Décompte des tests : session compte 16 tests (17 annoncés) et config_sync 10 (9 annoncés) ; le total de 63 tests unitaires du boîtier hors alert_detector est cohérent.
  • Les résultats cargo test (233 passés, 39 ignorés) et la vérification de bout en bout n'ont pas pu être rejoués ici (revue en lecture seule, sans cargo ni docker).

Tests

  • Bonne couverture des décisions de session : ordre des actions, délais, ACK forgés (autre id, status error, "1", vide, non UTF-8), bornes, version plus ancienne, redémarrage, échec d'écriture. Pas de sleep, répertoires temporaires suffixés par le pid.
  • La boucle main.rs (sélection entre poll et minuterie, retry_at, échecs try_*) n'a aucun test automatisé ; elle ne repose que sur la vérification manuelle décrite.
  • Les tests de contrat inter crates sont utiles mais en partie circulaires (aquaserveur/tests/contract_boitier_serveur.rs:113) : mêmes types des deux côtés, fixtures réécrites dans ce lot.
  • Aucun test ne couvre une version très grande ou trop longue, ni la durabilité de l'écriture.
  • aquashared/tests/integration_test.rs (test_every_message_has_its_topic_and_type) a des assertions faibles (accessibilité de l'API plus qu'un comportement).

Questions ouvertes

  • EventLoop::poll de rumqttc 0.25 est-il sûr à l'annulation dans le tokio::select! de main.rs:102-125 ? Une annulation pendant la connexion ou une lecture partielle mérite une vérification dans la documentation de la version utilisée.
  • Type et fuseau de boitiers.updated_at : si c'est un DATETIME local, UNIX_TIMESTAMP n'est pas monotone au changement d'heure (25 octobre 2026), et une configuration légitime pourrait être refusée comme plus ancienne.
  • En production, comment le fichier d'identité est-il fourni ? Un montage d'un fichier seul fait échouer le renommage (EBUSY), donc first_connect ne serait jamais persisté.
  • Comportement de rumqttc quand un paquet sortant dépasse max_packet_size : simple erreur ou déconnexion répétée ?
  • Le réessai sans limite de tentatives remplace les 3 essais de EPIC004:308 : choix à faire valider explicitement par le produit.

Autres constats (non publies en ligne)

Gravite Emplacement Source Constat
mineur aquaboitier-embedded/Dockerfile.prod:73 revue Le commentaire présente /data et /app/conf comme l'état persistant, mais aucun volume ne les couvre (docker-compose.dev.yml ne monte que /data/pending) : une recréation du conteneur (up --build) perd l'identité, ce qui arrête le boîtier hors mode développement, et la version appliquée, ce qui désactive l'anti-retour en arrière. L'affirmation « valable après redémarrage » ne tient que pour un docker restart. Piste : déclarer VOLUME ou documenter les montages requis pour /app/conf et /data.
mineur aquaboitier-embedded/src/config_ack/mod.rs:35 semgrep semgrep no-unwrap-in-production (WARNING) : .unwrap() ou .expect() peut paniquer en production. 4 occurrence(s) sur lignes ajoutees (35, 49, 60, 60), y compris d'eventuels modules de test internes.
mineur aquaboitier-embedded/src/config_sync/mod.rs:297 semgrep semgrep no-unwrap-in-production (WARNING) : .unwrap() ou .expect() peut paniquer en production. 3 occurrence(s) sur lignes ajoutees (297, 298, 315), y compris d'eventuels modules de test internes.
mineur aquaboitier-embedded/src/firstco/mod.rs:7 revue La documentation (et le compte rendu) indiquent que le boîtier publie le fichier d'identité tel quel, mais firstco_payload resérialise le type FirstConnectPayload : les champs non modélisés (activate_date, Isopen, présents dans la fixture FRESH des tests) sont supprimés du message publié, contrairement à la passerelle historique qui publiait le fichier brut. Piste : corriger la documentation ou publier le contenu brut validé.
mineur aquaboitier-embedded/src/firstco/mod.rs:162 semgrep semgrep no-unwrap-in-production (WARNING) : .unwrap() ou .expect() peut paniquer en production. 18 occurrence(s) sur lignes ajoutees (162, 169, 173, 183, 198, 199, 211, 221, 231, 238, 240, 242 ...), y compris d'eventuels modules de test internes.
mineur aquaboitier-embedded/src/main.rs:18 semgrep semgrep tokio-spawn-fire-and-forget (WARNING) : tokio::spawn() sans stocker le JoinHandle (fire-and-forget). 2 occurrence(s) sur lignes ajoutees (18, 18), y compris d'eventuels modules de test internes.
mineur aquaboitier-embedded/src/main.rs:35 revue Un échec de try_subscribe (canal plein) est seulement journalisé : l'abonnement à maj_serv-{id} ou firstco_ACK-{id} n'est retenté qu'au ConnAck suivant, ce qui peut laisser le boîtier sourd aux configurations pendant toute la durée de la connexion. Même remarque pour un ACK de configuration perdu par try_publish (le serveur ne republie qu'au prochain changement). Piste : renvoyer les abonnements en échec à la session pour les réessayer, ou dimensionner le canal au-delà du nombre d'actions d'un ConnAck.
mineur aquaboitier-embedded/src/main.rs:60 revue En mode développement sans identité, le processus attend indéfiniment sans gestionnaire de signal ; lancé par exec dans entrypoint.sh, il est PID 1 et le noyau ignore alors SIGTERM, donc docker stop attend le délai puis envoie SIGKILL (même situation dans la boucle principale). Piste : sélectionner sur tokio::signal (SIGTERM, ctrl_c) ou lancer le conteneur avec --init.
mineur aquaboitier-embedded/src/persist.rs:29 semgrep semgrep no-unwrap-in-production (WARNING) : .unwrap() ou .expect() peut paniquer en production. 4 occurrence(s) sur lignes ajoutees (29, 31, 33, 37), y compris d'eventuels modules de test internes.
mineur aquaboitier-embedded/src/session.rs:223 revue L'ACK d'erreur recopie la version_config reçue telle quelle, deux fois (champ version_config et error_detail, puisque ConfigRejection::InvalidVersion inclut le texte brut). Un maj_serv forgé avec une version de plusieurs dizaines de Kio (accepté jusqu'à MQTT_MAX_PACKET_SIZE, environ 74 Kio) donnerait un ACK au moins deux fois plus gros, qui dépasserait vraisemblablement la limite sortante de rumqttc, puis serait stocké en alerte 15 par le serveur. Piste : refuser une version de plus de 20 caractères avant tout écho et tronquer error_detail.
mineur aquaboitier-embedded/src/session.rs:268 semgrep semgrep no-unwrap-in-production (WARNING) : .unwrap() ou .expect() peut paniquer en production. 10 occurrence(s) sur lignes ajoutees (268, 273, 292, 293, 340, 357, 361, 525, 586, 591), y compris d'eventuels modules de test internes.
mineur aquaboitier-embedded/src/settings.rs:86 revue parse_broker_url ne retire que le préfixe mqtt:// et remplace silencieusement toute valeur invalide par un défaut : mqtts://h:8883 donne l'hôte "mqtts", mqtt://h:1883/ donne le port 1883 par défaut, un port non numérique aussi. Une erreur de configuration se traduit alors par des reconnexions sans fin vers une mauvaise cible. Piste : refuser au démarrage un schéma ou un port invalide, comme pour l'identité.
mineur aquaboitier-embedded/src/settings.rs:119 semgrep semgrep no-unwrap-in-production (WARNING) : .unwrap() ou .expect() peut paniquer en production. 6 occurrence(s) sur lignes ajoutees (119, 210, 215, 218, 218, 221), y compris d'eventuels modules de test internes.
mineur aquaserveur/src/config_sync.rs:99 semgrep semgrep no-unwrap-in-production (WARNING) : .unwrap() ou .expect() peut paniquer en production. 1 occurrence(s) sur lignes ajoutees (99), y compris d'eventuels modules de test internes.
mineur aquaserveur/tests/contract_boitier_serveur.rs:113 revue Les tests de contrat sont en partie circulaires : les deux côtés utilisent les mêmes types d'aquashared et les fixtures e2e ont été réécrites dans ce lot pour correspondre à ces types. Ils détectent une divergence entre crates, pas une divergence avec le format réel de la passerelle. Piste : ajouter un test qui lit un payload de référence indépendant de ce lot (par exemple celui de e2e/firstco/firstco-A.sh, ou une copie anonymisée de la structure de config_boitier.json).
mineur aquashared/src/backoff.rs:26 revue Le délai est strictement déterministe (doublement sans gigue). Après un redémarrage du broker, tous les boîtiers se reconnectent et republient firstco aux mêmes instants (1, 2, 4 s puis 10, 20, 40 s), ce qui concentre la charge sur le broker et le serveur. Piste : ajouter une gigue (par exemple délai tiré entre la moitié et la totalité de la valeur courante).
mineur aquashared/src/config.rs:152 semgrep semgrep no-unwrap-in-production (WARNING) : .unwrap() ou .expect() peut paniquer en production. 5 occurrence(s) sur lignes ajoutees (152, 162, 248, 258, 262), y compris d'eventuels modules de test internes.
mineur aquashared/src/frame.rs:214 semgrep semgrep no-unwrap-in-production (WARNING) : .unwrap() ou .expect() peut paniquer en production. 10 occurrence(s) sur lignes ajoutees (214, 223, 230, 237, 243, 250, 256, 349, 356, 363), y compris d'eventuels modules de test internes.
mineur aquashared/src/id.rs:164 semgrep semgrep no-unwrap-in-production (WARNING) : .unwrap() ou .expect() peut paniquer en production. 1 occurrence(s) sur lignes ajoutees (164), y compris d'eventuels modules de test internes.
mineur aquashared/src/provisioning.rs:209 semgrep semgrep no-unwrap-in-production (WARNING) : .unwrap() ou .expect() peut paniquer en production. 12 occurrence(s) sur lignes ajoutees (209, 215, 225, 232, 253, 254, 258, 265, 271, 277, 283, 284), y compris d'eventuels modules de test internes.
mineur aquashared/src/topics.rs:136 semgrep semgrep no-unwrap-in-production (WARNING) : .unwrap() ou .expect() peut paniquer en production. 1 occurrence(s) sur lignes ajoutees (136), y compris d'eventuels modules de test internes.
mineur e2e/boitier/s7-config-sync.sh:37 revue La version publiée est l'epoch de l'hôte qui lance le script, et rien ne nettoie ensuite /data/config_state.json : le boîtier 4990 garde une version appliquée postérieure à boitiers.updated_at, donc la republication du serveur à son redémarrage serait refusée (ACK error et alerte 15) jusqu'à la prochaine modification en base. Piste : supprimer le fichier d'état dans cleanup-boitier-4990.sh, ou dériver la version de updated_at lu en base.
style aquaboitier-embedded/src/firstco/mod.rs:89 revue La réécriture via serde_json::Value (sans la fonctionnalité preserve_order) trie les clés et compacte le fichier d'identité, et le fichier temporaire est créé avec les droits par défaut (umask), qui remplacent ceux du fichier d'origine alors qu'il contient des données client. Piste : activer preserve_order si l'ordre compte pour un outil externe, et reprendre les permissions du fichier d'origine dans write_atomic.

Limites

Revue publiee en COMMENT : l'auteur des PR et le relecteur sont le meme compte, Forgejo refuse APPROVE et REQUEST_CHANGES. Le blocage s'exprime par le statut de commit aquaprocess/revue-statique. Analyse statique et tests automatises seulement, sans fusion ni deploiement.

## Revue statique Vigie : PR #2 Tete `55fc8050c3`, base `main` (`674dc89dc2`), 4 commit(s), 39 fichier(s) ajoutes ou modifies. **Statut `aquaprocess/revue-statique` : success** Constats : 0 bloquant, 4 majeur, 22 mineur, 1 style. Commentaires en ligne : 4 (mode `important`, plafond 10). ### Verifications automatiques | Verification | Resultat | Detail | |---|---|---| | cargo fmt --check | success | aucune difference | | cargo clippy -D warnings | success | aucun avertissement | | cargo test --workspace | success | 233 passes, 0 echec(s), 39 ignores | | tests ignores (base jetable) | success | 39 passes, 0 echec(s) ; pas de broker MQTT (tests sans TEST_MQTT_URL) ; dump charge en 36 s ; 1 fichier(s) de fixtures ; migrations du depot : aucune | | cargo audit | attention | introduite(s) : aucune ; deja presente(s) sur la base : RUSTSEC-2026-0049 (rustls-webpki), RUSTSEC-2026-0098 (rustls-webpki), RUSTSEC-2026-0099 (rustls-webpki), RUSTSEC-2026-0104 (rustls-webpki), RUSTSEC-2026-0258 (h2), RUSTSEC-2026-0285 (rustls), 2 ignoree(s) par audit.toml | | semgrep (regles du depot) | success | 27 fichier(s) ; lignes ajoutees : 77 resultat(s) hors tests, tests : no-unwrap-in-production x4 ; 25 sur lignes inchangees | | gitleaks (plage de commits) | success | 4 commit(s) analyses, aucune fuite | | format des commits | success | 4 commit(s) conformes | | emoji dans le code ajoute | success | 3243 ligne(s) ajoutee(s), aucune | ### Analyse qualitative Verdict : aucun bloquant ; 4 constats majeurs à traiter avant la production (anti-retour en arrière sans borne, écriture non durable, republication de firstco qui réécrit les commandes, données client sur un topic global), 9 mineurs et 1 de style. Le lot tient l'essentiel de ce qu'il annonce : contrats centralisés dans aquashared, session du boîtier pure et bien testée, ACK contrôlés. ### Sécurité - Le broker livré (`configs/mosquitto/mosquitto.conf`) accepte les clients anonymes sans ACL. Les contrôles N2 protègent donc contre un message mal formé, pas contre un message forgé : une version égale est réappliquée, donc un `maj_serv` forgé avec la version courante et d'autres commandes passe. C'est cohérent avec le renvoi de l'ACL à L6, mais il faut le dire clairement dans la suite. - Nouveau risque introduit par l'anti-retour en arrière : une seule version très grande, acceptée puis persistée, bloque définitivement la réception des configurations légitimes (`aquashared/src/config.rs:87`). Sans borne haute ni voie de réinitialisation, c'est un déni de service durable. - Le boîtier publie maintenant l'objet `client` (nom, adresse) sur `firstco` global, lisible par tout abonné à `#` (`aquaboitier-embedded/src/session.rs:178`). - L'ACK d'erreur recopie la version reçue sans borne, deux fois, et le serveur la stocke en alerte 15 (`aquaboitier-embedded/src/session.rs:223`). - Point positif : l'id canonique (`BoitierId::parse`) et `Topic::parse` côté boîtier écartent les alias (`maj_serv-04000`), et l'ACK `firstco` n'est accepté que pour le bon boîtier avec `status = ok`. Aucun secret ni donnée réelle dans les fixtures (valeurs fictives 4990). ### Correction - `write_atomic` (`aquaboitier-embedded/src/persist.rs:17`) n'appelle aucun `sync_all` : l'atomicité vaut pour un arrêt du processus, pas pour une coupure d'alimentation, alors que le commentaire l'affirme. Un fichier d'identité vide arrête le boîtier. - Republication de `firstco` : l'upsert serveur réécrit `modeBoost`, `generatorStateA/B` et `updated_at` à partir des valeurs figées du fichier d'identité. Le choix de passer opérationnel quand la persistance échoue, avec republication au démarrage suivant (`aquaboitier-embedded/src/session.rs:196`), peut donc annuler des commandes posées par l'administrateur, que le poller repousse ensuite au boîtier. - Boucle principale (`aquaboitier-embedded/src/main.rs`) : le passage à `try_publish`/`try_subscribe` supprime bien le risque d'interblocage, mais un échec n'est que journalisé (`main.rs:35`), sans nouvel essai avant la reconnexion suivante. - Le réessai `firstco` (10 s doublés jusqu'à 300 s, arrêt hors connexion et à l'ACK) est correct ; il manque une gigue pour éviter la synchronisation de toute une flotte (`aquashared/src/backoff.rs:26`). - État persistant hors volume (`aquaboitier-embedded/Dockerfile.prod:73`) : la version appliquée et l'identité disparaissent à la recréation du conteneur. - Le serveur garde `mode_boost: row.mode_boost.unwrap_or(0)` (`aquaserveur/src/config_poller.rs:53`, ligne non modifiée) : le défaut silencieux que le contrat veut supprimer côté boîtier existe toujours, mais il est appliqué côté serveur. - Mineurs : `parse_broker_url` retombe silencieusement sur des défauts (`settings.rs:86`), processus PID 1 sans gestion de SIGTERM (`main.rs:60`), version e2e tirée de l'horloge de l'hôte et jamais nettoyée (`e2e/boitier/s7-config-sync.sh:37`). ### Cohérence avec le compte rendu Affirmations vérifiées sur la tête 55fc805 (références fichier:ligne exactes) : 1. `aquashared/src/id.rs:31` et `:42` (`BoitierId`, `parse` canonique) : conforme. 2. `aquashared/src/topics.rs:20-25`, `:63`, `:107-127` : conforme. 3. `aquashared/src/config.rs:23`, `:54`, `:68`, `:84`, `:100` ; `frame.rs:48`, `:55`, `:82`, `:170` ; `mqtt.rs:14-22` ; `backoff.rs:16-33` : conforme. 4. `session.rs:29-31`, `:113`, `:132`, `:167`, `:185`, `:204` et le comportement annoncé (abonnement avant publication, délais 10 à 300 s sans limite, rien hors connexion, ACK forgés refusés, payload illisible sans ACK) : conforme. 5. `main.rs:56-60`, `:85`, `:104`, `:116` ; `settings.rs:18`, `:57`, `:70` ; `entrypoint.sh:11` ; `Dockerfile:86`, `:98-99` ; `Dockerfile.prod:73`, `:85-86` ; `docker-compose.dev.yml:103` ; `common-boitier.sh:40` : conforme. 6. Serveur : `provisioning/types.rs:7` et `config_sync.rs:16` réexportent les types partagés, `firstco_ack_payload` en `config_sync.rs:94`, appel en `main.rs:218-219`, format `{"status":"ok","id_boitier":N}` testé à l'octet : conforme. 7. `mark_firstco_done` conserve les autres champs : conforme (testé), mais l'ordre des clés et les permissions changent (`firstco/mod.rs:89`). Écarts : - « Écriture atomique » : partiellement vrai (pas de durabilité, voir `persist.rs:17`). - « Publié tel quel » (compte rendu section 1 et `firstco/mod.rs:7`) : le payload est resérialisé, les champs non modélisés sont perdus. - « Valable après redémarrage » : vrai pour un `docker restart`, faux pour une recréation du conteneur. - Décompte des tests : `session` compte 16 tests (17 annoncés) et `config_sync` 10 (9 annoncés) ; le total de 63 tests unitaires du boîtier hors `alert_detector` est cohérent. - Les résultats `cargo test` (233 passés, 39 ignorés) et la vérification de bout en bout n'ont pas pu être rejoués ici (revue en lecture seule, sans cargo ni docker). ### Tests - Bonne couverture des décisions de session : ordre des actions, délais, ACK forgés (autre id, `status error`, `"1"`, vide, non UTF-8), bornes, version plus ancienne, redémarrage, échec d'écriture. Pas de `sleep`, répertoires temporaires suffixés par le pid. - La boucle `main.rs` (sélection entre `poll` et minuterie, `retry_at`, échecs `try_*`) n'a aucun test automatisé ; elle ne repose que sur la vérification manuelle décrite. - Les tests de contrat inter crates sont utiles mais en partie circulaires (`aquaserveur/tests/contract_boitier_serveur.rs:113`) : mêmes types des deux côtés, fixtures réécrites dans ce lot. - Aucun test ne couvre une version très grande ou trop longue, ni la durabilité de l'écriture. - `aquashared/tests/integration_test.rs` (`test_every_message_has_its_topic_and_type`) a des assertions faibles (accessibilité de l'API plus qu'un comportement). ### Questions ouvertes - `EventLoop::poll` de rumqttc 0.25 est-il sûr à l'annulation dans le `tokio::select!` de `main.rs:102-125` ? Une annulation pendant la connexion ou une lecture partielle mérite une vérification dans la documentation de la version utilisée. - Type et fuseau de `boitiers.updated_at` : si c'est un DATETIME local, `UNIX_TIMESTAMP` n'est pas monotone au changement d'heure (25 octobre 2026), et une configuration légitime pourrait être refusée comme plus ancienne. - En production, comment le fichier d'identité est-il fourni ? Un montage d'un fichier seul fait échouer le renommage (EBUSY), donc `first_connect` ne serait jamais persisté. - Comportement de rumqttc quand un paquet sortant dépasse `max_packet_size` : simple erreur ou déconnexion répétée ? - Le réessai sans limite de tentatives remplace les 3 essais de EPIC004:308 : choix à faire valider explicitement par le produit. ### Autres constats (non publies en ligne) | Gravite | Emplacement | Source | Constat | |---|---|---|---| | mineur | `aquaboitier-embedded/Dockerfile.prod:73` | revue | Le commentaire présente /data et /app/conf comme l'état persistant, mais aucun volume ne les couvre (docker-compose.dev.yml ne monte que /data/pending) : une recréation du conteneur (up --build) perd l'identité, ce qui arrête le boîtier hors mode développement, et la version appliquée, ce qui désactive l'anti-retour en arrière. L'affirmation « valable après redémarrage » ne tient que pour un docker restart. Piste : déclarer VOLUME ou documenter les montages requis pour /app/conf et /data. | | mineur | `aquaboitier-embedded/src/config_ack/mod.rs:35` | semgrep | semgrep `no-unwrap-in-production` (WARNING) : `.unwrap()` ou `.expect()` peut paniquer en production. 4 occurrence(s) sur lignes ajoutees (35, 49, 60, 60), y compris d'eventuels modules de test internes. | | mineur | `aquaboitier-embedded/src/config_sync/mod.rs:297` | semgrep | semgrep `no-unwrap-in-production` (WARNING) : `.unwrap()` ou `.expect()` peut paniquer en production. 3 occurrence(s) sur lignes ajoutees (297, 298, 315), y compris d'eventuels modules de test internes. | | mineur | `aquaboitier-embedded/src/firstco/mod.rs:7` | revue | La documentation (et le compte rendu) indiquent que le boîtier publie le fichier d'identité tel quel, mais firstco_payload resérialise le type FirstConnectPayload : les champs non modélisés (activate_date, Isopen, présents dans la fixture FRESH des tests) sont supprimés du message publié, contrairement à la passerelle historique qui publiait le fichier brut. Piste : corriger la documentation ou publier le contenu brut validé. | | mineur | `aquaboitier-embedded/src/firstco/mod.rs:162` | semgrep | semgrep `no-unwrap-in-production` (WARNING) : `.unwrap()` ou `.expect()` peut paniquer en production. 18 occurrence(s) sur lignes ajoutees (162, 169, 173, 183, 198, 199, 211, 221, 231, 238, 240, 242 ...), y compris d'eventuels modules de test internes. | | mineur | `aquaboitier-embedded/src/main.rs:18` | semgrep | semgrep `tokio-spawn-fire-and-forget` (WARNING) : `tokio::spawn()` sans stocker le JoinHandle (fire-and-forget). 2 occurrence(s) sur lignes ajoutees (18, 18), y compris d'eventuels modules de test internes. | | mineur | `aquaboitier-embedded/src/main.rs:35` | revue | Un échec de try_subscribe (canal plein) est seulement journalisé : l'abonnement à maj_serv-{id} ou firstco_ACK-{id} n'est retenté qu'au ConnAck suivant, ce qui peut laisser le boîtier sourd aux configurations pendant toute la durée de la connexion. Même remarque pour un ACK de configuration perdu par try_publish (le serveur ne republie qu'au prochain changement). Piste : renvoyer les abonnements en échec à la session pour les réessayer, ou dimensionner le canal au-delà du nombre d'actions d'un ConnAck. | | mineur | `aquaboitier-embedded/src/main.rs:60` | revue | En mode développement sans identité, le processus attend indéfiniment sans gestionnaire de signal ; lancé par exec dans entrypoint.sh, il est PID 1 et le noyau ignore alors SIGTERM, donc docker stop attend le délai puis envoie SIGKILL (même situation dans la boucle principale). Piste : sélectionner sur tokio::signal (SIGTERM, ctrl_c) ou lancer le conteneur avec --init. | | mineur | `aquaboitier-embedded/src/persist.rs:29` | semgrep | semgrep `no-unwrap-in-production` (WARNING) : `.unwrap()` ou `.expect()` peut paniquer en production. 4 occurrence(s) sur lignes ajoutees (29, 31, 33, 37), y compris d'eventuels modules de test internes. | | mineur | `aquaboitier-embedded/src/session.rs:223` | revue | L'ACK d'erreur recopie la version_config reçue telle quelle, deux fois (champ version_config et error_detail, puisque ConfigRejection::InvalidVersion inclut le texte brut). Un maj_serv forgé avec une version de plusieurs dizaines de Kio (accepté jusqu'à MQTT_MAX_PACKET_SIZE, environ 74 Kio) donnerait un ACK au moins deux fois plus gros, qui dépasserait vraisemblablement la limite sortante de rumqttc, puis serait stocké en alerte 15 par le serveur. Piste : refuser une version de plus de 20 caractères avant tout écho et tronquer error_detail. | | mineur | `aquaboitier-embedded/src/session.rs:268` | semgrep | semgrep `no-unwrap-in-production` (WARNING) : `.unwrap()` ou `.expect()` peut paniquer en production. 10 occurrence(s) sur lignes ajoutees (268, 273, 292, 293, 340, 357, 361, 525, 586, 591), y compris d'eventuels modules de test internes. | | mineur | `aquaboitier-embedded/src/settings.rs:86` | revue | parse_broker_url ne retire que le préfixe mqtt:// et remplace silencieusement toute valeur invalide par un défaut : mqtts://h:8883 donne l'hôte "mqtts", mqtt://h:1883/ donne le port 1883 par défaut, un port non numérique aussi. Une erreur de configuration se traduit alors par des reconnexions sans fin vers une mauvaise cible. Piste : refuser au démarrage un schéma ou un port invalide, comme pour l'identité. | | mineur | `aquaboitier-embedded/src/settings.rs:119` | semgrep | semgrep `no-unwrap-in-production` (WARNING) : `.unwrap()` ou `.expect()` peut paniquer en production. 6 occurrence(s) sur lignes ajoutees (119, 210, 215, 218, 218, 221), y compris d'eventuels modules de test internes. | | mineur | `aquaserveur/src/config_sync.rs:99` | semgrep | semgrep `no-unwrap-in-production` (WARNING) : `.unwrap()` ou `.expect()` peut paniquer en production. 1 occurrence(s) sur lignes ajoutees (99), y compris d'eventuels modules de test internes. | | mineur | `aquaserveur/tests/contract_boitier_serveur.rs:113` | revue | Les tests de contrat sont en partie circulaires : les deux côtés utilisent les mêmes types d'aquashared et les fixtures e2e ont été réécrites dans ce lot pour correspondre à ces types. Ils détectent une divergence entre crates, pas une divergence avec le format réel de la passerelle. Piste : ajouter un test qui lit un payload de référence indépendant de ce lot (par exemple celui de e2e/firstco/firstco-A.sh, ou une copie anonymisée de la structure de config_boitier.json). | | mineur | `aquashared/src/backoff.rs:26` | revue | Le délai est strictement déterministe (doublement sans gigue). Après un redémarrage du broker, tous les boîtiers se reconnectent et republient firstco aux mêmes instants (1, 2, 4 s puis 10, 20, 40 s), ce qui concentre la charge sur le broker et le serveur. Piste : ajouter une gigue (par exemple délai tiré entre la moitié et la totalité de la valeur courante). | | mineur | `aquashared/src/config.rs:152` | semgrep | semgrep `no-unwrap-in-production` (WARNING) : `.unwrap()` ou `.expect()` peut paniquer en production. 5 occurrence(s) sur lignes ajoutees (152, 162, 248, 258, 262), y compris d'eventuels modules de test internes. | | mineur | `aquashared/src/frame.rs:214` | semgrep | semgrep `no-unwrap-in-production` (WARNING) : `.unwrap()` ou `.expect()` peut paniquer en production. 10 occurrence(s) sur lignes ajoutees (214, 223, 230, 237, 243, 250, 256, 349, 356, 363), y compris d'eventuels modules de test internes. | | mineur | `aquashared/src/id.rs:164` | semgrep | semgrep `no-unwrap-in-production` (WARNING) : `.unwrap()` ou `.expect()` peut paniquer en production. 1 occurrence(s) sur lignes ajoutees (164), y compris d'eventuels modules de test internes. | | mineur | `aquashared/src/provisioning.rs:209` | semgrep | semgrep `no-unwrap-in-production` (WARNING) : `.unwrap()` ou `.expect()` peut paniquer en production. 12 occurrence(s) sur lignes ajoutees (209, 215, 225, 232, 253, 254, 258, 265, 271, 277, 283, 284), y compris d'eventuels modules de test internes. | | mineur | `aquashared/src/topics.rs:136` | semgrep | semgrep `no-unwrap-in-production` (WARNING) : `.unwrap()` ou `.expect()` peut paniquer en production. 1 occurrence(s) sur lignes ajoutees (136), y compris d'eventuels modules de test internes. | | mineur | `e2e/boitier/s7-config-sync.sh:37` | revue | La version publiée est l'epoch de l'hôte qui lance le script, et rien ne nettoie ensuite /data/config_state.json : le boîtier 4990 garde une version appliquée postérieure à boitiers.updated_at, donc la republication du serveur à son redémarrage serait refusée (ACK error et alerte 15) jusqu'à la prochaine modification en base. Piste : supprimer le fichier d'état dans cleanup-boitier-4990.sh, ou dériver la version de updated_at lu en base. | | style | `aquaboitier-embedded/src/firstco/mod.rs:89` | revue | La réécriture via serde_json::Value (sans la fonctionnalité preserve_order) trie les clés et compacte le fichier d'identité, et le fichier temporaire est créé avec les droits par défaut (umask), qui remplacent ceux du fichier d'origine alors qu'il contient des données client. Piste : activer preserve_order si l'ordre compte pour un outil externe, et reprendre les permissions du fichier d'origine dans write_atomic. | ### Limites Revue publiee en `COMMENT` : l'auteur des PR et le relecteur sont le meme compte, Forgejo refuse APPROVE et REQUEST_CHANGES. Le blocage s'exprime par le statut de commit `aquaprocess/revue-statique`. Analyse statique et tests automatises seulement, sans fusion ni deploiement. <!-- vigie:review -->
@ -0,0 +14,4 @@
let mut tmp_name = file_name.to_os_string();
tmp_name.push(".tmp");
let tmp = path.with_file_name(tmp_name);
std::fs::write(&tmp, contents).with_context(|| format!("write {}", tmp.display()))?;
Author
Member

[majeur] write_atomic écrit le fichier temporaire puis le renomme sans sync_all() du fichier ni fsync du répertoire. Le renommage est atomique vis-à-vis d'un arrêt du processus, mais pas d'une coupure d'alimentation : selon le système de fichiers de la carte, le fichier renommé peut se retrouver vide après redémarrage, ce que le commentaire des lignes 7 à 9 affirme pourtant éviter. Pour le fichier d'identité, un contenu vide arrête le boîtier (code 1) au démarrage suivant. Piste : File::create, write_all, sync_all sur le temporaire, rename, puis ouverture et sync_all du répertoire parent.

Source : revue.

**[majeur]** write_atomic écrit le fichier temporaire puis le renomme sans sync_all() du fichier ni fsync du répertoire. Le renommage est atomique vis-à-vis d'un arrêt du processus, mais pas d'une coupure d'alimentation : selon le système de fichiers de la carte, le fichier renommé peut se retrouver vide après redémarrage, ce que le commentaire des lignes 7 à 9 affirme pourtant éviter. Pour le fichier d'identité, un contenu vide arrête le boîtier (code 1) au démarrage suivant. Piste : File::create, write_all, sync_all sur le temporaire, rename, puis ouverture et sync_all du répertoire parent. _Source : revue._
@ -0,0 +175,4 @@
self.firstco.on_publish();
vec![
Outgoing::Publish {
topic: topics::FIRSTCO.to_string(),
Author
Member

[majeur] Le boîtier publie désormais l'enveloppe complète, objet client compris (customerName, address, postcode, city), sur le topic global firstco. Avec la configuration actuelle du broker (anonyme, sans ACL), tout client abonné à firstco ou à # reçoit ces données personnelles pour chaque boîtier ; l'ancien boîtier Rust ne publiait que trois champs. Piste : conditionner la mise en production à l'ACL (L6) et au topic firstco-{id}, ou ne publier que les champs dont le provisioning a besoin.

Source : revue.

**[majeur]** Le boîtier publie désormais l'enveloppe complète, objet client compris (customerName, address, postcode, city), sur le topic global firstco. Avec la configuration actuelle du broker (anonyme, sans ACL), tout client abonné à firstco ou à # reçoit ces données personnelles pour chaque boîtier ; l'ancien boîtier Rust ne publiait que trois champs. Piste : conditionner la mise en production à l'ACL (L6) et au topic firstco-{id}, ou ne publier que les champs dont le provisioning a besoin. _Source : revue._
@ -0,0 +193,4 @@
}
if let Err(e) = mark_firstco_done(&self.paths.config) {
// Le serveur a provisionné le boîtier : rester en attente ferait
// republier firstco indéfiniment. Le prochain démarrage republiera.
Author
Member

[majeur] Quand l'ACK n'est pas persisté, le boîtier passe opérationnel et compte sur le prochain démarrage pour republier firstco ; de même un ACK perdu provoque une republication. Or le payload firstco porte modeBoost et config.generatorStateA/B figés dans le fichier d'identité, et l'upsert serveur (provisioning/handler.rs, ON DUPLICATE KEY UPDATE modeBoost, generatorStateA/B, updated_at) les réécrit en base : toute republication tardive annule les commandes posées par l'administrateur, et le poller repousse ces valeurs anciennes au boîtier. Piste : retenter la persistance au lieu de passer opérationnel, ou ne pas réécrire les commandes lors d'un upsert sur un boîtier déjà provisionné (côté serveur, L3).

Source : revue.

**[majeur]** Quand l'ACK n'est pas persisté, le boîtier passe opérationnel et compte sur le prochain démarrage pour republier firstco ; de même un ACK perdu provoque une republication. Or le payload firstco porte modeBoost et config.generatorStateA/B figés dans le fichier d'identité, et l'upsert serveur (provisioning/handler.rs, ON DUPLICATE KEY UPDATE modeBoost, generatorStateA/B, updated_at) les réécrit en base : toute republication tardive annule les commandes posées par l'administrateur, et le poller repousse ces valeurs anciennes au boîtier. Piste : retenter la persistance au lieu de passer opérationnel, ou ne pas réécrire les commandes lors d'un upsert sur un boîtier déjà provisionné (côté serveur, L3). _Source : revue._
@ -0,0 +84,4 @@
pub fn validate_against(&self, applied: Option<u64>) -> Result<u64, ConfigRejection> {
let received = self.validate()?;
match applied {
Some(applied) if received < applied => {
Author
Member

[majeur] L'anti-retour en arrière n'a ni borne haute ni voie de réinitialisation : une configuration acceptée avec une version très grande (par exemple "18446744073709551615", ou un epoch futur venant d'une horloge décalée) est persistée dans /data/config_state.json, puis toute configuration légitime ultérieure est refusée comme Stale, redémarrage compris, avec une alerte 15 côté serveur à chaque refus. Le broker actuel (allow_anonymous true, sans ACL) permet de le provoquer depuis le réseau, et une restauration de base (updated_at plus anciens) produit le même blocage. Piste : borner la version (longueur et plausibilité, par exemple au plus horloge du boîtier plus une marge), et prévoir un moyen explicite de réinitialiser la version appliquée.

Source : revue.

**[majeur]** L'anti-retour en arrière n'a ni borne haute ni voie de réinitialisation : une configuration acceptée avec une version très grande (par exemple "18446744073709551615", ou un epoch futur venant d'une horloge décalée) est persistée dans /data/config_state.json, puis toute configuration légitime ultérieure est refusée comme Stale, redémarrage compris, avec une alerte 15 côté serveur à chaque refus. Le broker actuel (allow_anonymous true, sans ACL) permet de le provoquer depuis le réseau, et une restauration de base (updated_at plus anciens) produit le même blocage. Piste : borner la version (longueur et plausibilité, par exemple au plus horloge du boîtier plus une marge), et prévoir un moyen explicite de réinitialiser la version appliquée. _Source : revue._
All checks were successful
aquaprocess/revue-statique succes : 9 verifications, 0 bloquant
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin pr/dette-l2:pr/dette-l2
git switch pr/dette-l2

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch main
git merge --no-ff pr/dette-l2
git switch pr/dette-l2
git rebase main
git switch main
git merge --ff-only pr/dette-l2
git switch pr/dette-l2
git rebase main
git switch main
git merge --no-ff pr/dette-l2
git switch main
git merge --squash pr/dette-l2
git switch main
git merge --ff-only pr/dette-l2
git switch main
git merge pr/dette-l2
git push origin main
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
AcadeNice/Acquarefactoring-Thomas!2
No description provided.