Dette passe 1, lot L3 : provisioning non destructif et identifiant validé à l'entrée MQTT #3
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "pr/dette-l3"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Objet
Lot L3 de la passe 1 de dette : un
firstcone réécrit plus jamais un boîtier déjà provisionné, et l'identifiant de boîtier est validé une seule fois, à l'entrée MQTT, paraquashared::Topic::parse.Dettes traitées et preuves
N1, V2, Q6, recommandation 5 (partie
firstco), décision D4Avant :
handle_first_connectfaisait un UPSERT complet deboitierspuis des capteurs ; unfirstcoforgé réécrivait la fiche et faisait pousser par le poller les commandes forgées au vrai boîtier.firstco-{id}égal à celui du payload, numéro de série non videaquaserveur/src/provisioning/handler.rs,error.rsAlreadyProvisioned: aucune écriture (fiche,updated_at, capteurs, seuils)handler.rsSerialMismatch: refus, aucune écriturehandler.rsINSERTsimple au lieu de l'UPSERThandler.rsCôté
main.rs: succès (nouveau ou déjà provisionné) suivi de l'ACK{"status":"ok","id_boitier":N}surfirstco_ACK-{id}(format inchangé) ; boîtier déjà provisionné : republication de la configuration du serveur surmaj_serv-{id}; refus : journal d'avertissement, aucun ACK, sans numéro de série ni donnée client.Décision D3 :
firstco-{id}Routage de
firstco-{id}avec contrôle d'égalité entre l'id du topic et celui du payload avant toute requête ; le topic historiquefirstcoreste accepté ; le simulateur de boîtier publie surfirstco-{id}.Recommandation 5, N9, Q7 : routage et id
split_once, id enString(data-4000-extraaccepté)Topic::parse,BoitierIdvalidé une fois (mqtt/subscriber.rs)data-04000,data-foo,data-0InvalidTopic, aucun handler appeléstore_frame,handle_config_ack&strrevalidé,parse::<u64>, repliunwrap_or(0)BoitierIdboitier_existsavant analyse, stockage et alertes (database/store.rs,data_handler.rs)La branche
datasort demain.rsversaquaserveur/src/data_handler.rspour être testable ; contenu déplacé tel quel, seules la clé (BoitierId) et la vérification d'existence changent.Tests et résultats
firstco_non_destructive.rs6 échecs sur 7 avant implémentation ; routeur, contrat etdata_canonical_id.rsvus rouges ; contre-épreuve : contrôle d'existence neutralisé, le test du boîtier inconnu échoue.aquaserveur/tests/firstco_non_destructive.rs(7 + 4 ignorés),aquaserveur/tests/data_canonical_id.rs(2 + 2 ignorés), tests du routeur, test de contrat surfirstco-4990.cargo fmt --check,cargo clippy ... -D warnings: OK.cargo test --workspace: 242 passés, 0 échec, 45 ignorés (L2 : 233 et 39).datad'un boîtier inconnu sans écriture ni alerte.Migrations et actions de déploiement
firstco-{id}, que l'ancien serveur ne route pas).handle_first_connect) : carte remplacée, mettre à jourboitiers.noSerie; capteurs modifiés, mettre à jour{id}_boitier_capteurset les seuils ; reprise complète, supprimer la fiche et ses dépendances.Choix faits
status: error: le comportement historique (passerelle Node.js) traite tout message reçu comme un succès ; l'absence d'ACK est le seul format qu'aucun récepteur ne peut confondre avec un succès.firstcoidempotent.noSerieNULL refusée.SELECT 1 FROM boitiers WHERE id = ?par trame, sans cache, pour qu'une fiche supprimée cesse d'être alimentée immédiatement.Hors périmètre
Journalisation du payload (L5) ; authentification de l'émetteur et ACL
firstco-%u(L6) ; poller et suivi de version jusqu'à l'ACK (L5) ; documentation générale (L9).Retour arrière
Revenir les 3 commits du lot (
git revert) et redéployer ensemble serveur et simulateur : aucun schéma ni donnée n'est modifié.Pile de PR
Base de cette PR :
pr/dette-l2. Fusion dans l'ordre de la pile ; apres chaque fusion,vigie.py retargetrebase la PR suivante surmain.pr/dette-l2: Dette passe 1, lot L2 : contrats partagés dans aquashared et client MQTT du boîtierpr/dette-l3: Dette passe 1, lot L3 : provisioning non destructif et identifiant validé à l'entrée MQTT (cette PR)pr/dette-l4: Dette passe 1, lot L4 : mesures, alertes et déduplicationpr/dette-l5: Dette passe 1, lot L5 : boucle MQTT du serveur, livraison de la configuration et journauxpr/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 MQTTpr/dette-l7: Dette passe 1, lot L7 : builds reproductibles, dépendances, durcissement des conteneurspr/dette-l6: Dette passe 1, lot L6 : authentification Mosquitto, ACL par compte, healthchecks réels, pile e2e isoléepr/dette-l8: Dette passe 1, lot L8 : tests non destructifs, fixtures synthétiques, script de tests sur base jetableCommits du lot
2d7ae74fix: stop firstco from rewriting a provisioned boitierbec3b01fix: validate the boitier id once at the MQTT entry3c8c48efeat: publish firstco on firstco-{id} from the boitier simulatorA firstco for an id already present in boitiers no longer writes anything: client fields, generator commands, versionSoftware, updated_at, sensors and alert seeds stay as the operator left them. The same serial number is an idempotent success: the server sends the historical ACK and republishes its own configuration on maj_serv. A different or missing serial number is refused with a warning that carries no serial number nor client data, and no ACK. The server also accepts firstco-{id} and refuses a payload whose id differs from the topic id, before any query. The historical firstco topic stays accepted. A new boitier is still provisioned, with a plain INSERT that cannot overwrite a concurrent row. Fixes N1, V2 and Q6 (decisions D3 and D4). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UH3JcaVWsmFLC6kyEsVL18The Rust boitier now publishes its firstco payload on firstco-{id}, so the server can check that the topic id matches the payload id (decision D3) and a future ACL can bind the topic to the account. The global firstco topic stays accepted by the server for the ESP32 firmware kept outside the repository. The contract test checks the whole path: the topic published by the boitier session is routed with its canonical id and accepted by the server checks. The S7 E2E scripts, the S7 plan and the boitier README listen on and document the new topic. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UH3JcaVWsmFLC6kyEsVL18Revue statique Vigie : PR #3
Tete
3c8c48e78b, basepr/dette-l2(55fc8050c3), 3 commit(s), 21 fichier(s) ajoutes ou modifies.Statut
aquaprocess/revue-statique: failure (emoji dans le code ajoute; 1 constat(s) bloquant(s) de la revue)Constats : 1 bloquant, 4 majeur, 11 mineur, 0 style. Commentaires en ligne : 4 (mode
important, plafond 10).Verifications automatiques
Analyse qualitative
Verdict : 1 bloquant, 3 majeurs. Le lot ferme bien la réécriture d'une fiche provisionnée par
firstcoet valide l'id une seule fois à l'entrée MQTT, mais le passage de l'UPSERT à un INSERT suivi d'un « déjà provisionné » sans écriture rend définitif tout provisionnement interrompu.Sécurité
aquashared::Topic::parseetBoitierId::parse(chiffres seuls, ni signe, ni zéro de tête, non nul, borne u32) remplacentsplit_onceetparse::<u64>. Le nom de table{id}_boitier_*ne peut plus recevoir que des chiffres par construction ;fetch_boitier_confign'assemble que des littéraux et lie l'id. Aucune injection relevée.firstcone réécrit plus une fiche existante : aucune requête avantcheck_first_connect(handler.rs:250-268).aquaserveur/src/provisioning/handler.rs:264) : unfirstcoforgé sur un id encore libre crée la fiche avec un numéro de série choisi par l'émetteur, et le vrai boîtier est ensuite refusé définitivement. Avant L3, il écrasait la fiche. Sans authentification (L6), c'est une prise de possession d'id à traiter avant la production.firstco, routé sans id (aquaserveur/src/mqtt/subscriber.rs:66). C'est assumé par le compte rendu, mais la protection ne vaut que pour des clients qui utilisentfirstco-{id}.aquaserveur/src/provisioning/error.rs:7promet des messages sans donnée client, ce queParseError(texte serde) ne garantit pas. Le payload complet reste journalisé àmain.rs:143(renvoyé à L5 par le compte rendu).main.rs:184recharge les règles d'alerte à chaquefirstcoaccepté etregister_ruleles ajoute sans dédoublonner : tout rejeu avec le bon numéro de série fait croître la mémoire sans borne et duplique les alertes (corrigé ensuite parreplace_rulesen L4).Correction
aquaserveur/src/provisioning/handler.rs:335: la ficheboitiersest écrite en autocommit avant les DDL, la transaction des capteurs et les seuils. Un échec ou un redémarrage entre ces étapes laisse une fiche sans tables ; le rejeu du boîtier obtient alorsAlreadyProvisionedet l'ACK de succès, puis chaque trame échoue dansstore_frame(le contrôleboitier_existspasse). L'ancien UPSERT rendait ce rejeu réparateur. Piste : écrire la fiche en dernier, ou compléter les dépendances manquantes (DDL idempotents) sur le chemin « même numéro ».main.rs:201ajoute une seconde publication dans la boucleselect!, alors quemain.rs:93-96explique que publier depuis cette boucle peut la bloquer (canal borné à 100, vidé pareventloop.poll()). Le risque existait pour l'ACK ; il est doublé ici (la boucle est découplée en L5).main.rs:212confond refus et panne de base dans un même avertissement ;main.rs:205ignore sans trace l'absence de configuration à renvoyer.handler.rs:203compare les numéros de série à l'octet près alors que le contrôle de présence rogne les espaces.data_handler.rsreprend fidèlement l'ancienne branche (ordre, TTL de 300 s, repli(5000, 10000)). Les tables d'état sont indexées parBoitierIdet ne grossissent plus que pour des boîtiers existants. Le SELECT d'existence précède l'analyse de la trame (data_handler.rs:109) : une requête par message, même illisible.unwrapniexpectajouté dans le code de production du diff.Cohérence avec le compte rendu
Affirmations vérifiées à la tête
3c8c48e: les référenceshandler.rs,error.rs,main.rs,config_poller.rs:155,subscriber.rs:63-81,store.rs:90et:112,config_ack_handler.rs:74et:96,data_handler.rs:41,:43,:109,session.rs:181pointent sur le code décrit ;MissingNoSerien'était levée nulle part avant le lot ; l'UPSERTboitiersest remplacé par un INSERT simple ;extract_boitier_idest supprimée ; décomptes de tests exacts (7 + 4 ignorés dansfirstco_non_destructive.rs, 2 + 2 dansdata_canonical_id.rs) ; simulateur, README, scripts S7 et plan E2E publiés surfirstco-{id}.Écarts :
main.rs:184).SerialMismatch; le compte rendu reconnaît lui-même que serde peut citer un fragment.Tests
data_canonical_id.rs.firstco_non_destructive.rsvérifie la fiche colonne par colonne, les capteurs et le nombre d'alertes. Il ne couvre ni le provisionnement interrompu, ni la fiche insérée entre la lecture et l'INSERT (firstco_non_destructive.rs:363), ni la branche demain.rs(ACK, republication, rechargement des règles).e2e_t3.rs:74est devenu une assertion unitaire tautologique dans un test qui demande MariaDB.s7-firstco-skip.sh:34n'écoute plus le topic historique.Questions ouvertes
noSerierenseigné et identique à celui de leur firmware ? Sinon, chacun sera refusé sans ACK et republiera toutes les 300 s.firstco(comportement antérieur au lot, corrigé en L4 par un chargement global).firstcooufirstco-{id}? Cela fixe la date de fermeture du topic historique.Autres constats (non publies en ligne)
aquaserveur/src/main.rs:2012d7ae74, decalee dans la vue de la PR)aquaserveur/src/data_handler.rs:109aquaserveur/src/database/store.rs:413no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 6 occurrence(s) sur lignes ajoutees (413, 413, 450, 450, 494, 494), y compris d'eventuels modules de test internes.aquaserveur/src/main.rs:205aquaserveur/src/main.rs:212aquaserveur/src/mqtt/subscriber.rs:66aquaserveur/src/mqtt/subscriber.rs:90no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 1 occurrence(s) sur lignes ajoutees (90), y compris d'eventuels modules de test internes.aquaserveur/src/provisioning/error.rs:7aquaserveur/src/provisioning/handler.rs:203aquaserveur/tests/e2e_t3.rs:74aquaserveur/tests/firstco_non_destructive.rs:363e2e/boitier/s7-firstco-skip.sh:34Limites
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 commitaquaprocess/revue-statique. Analyse statique et tests automatises seulement, sans fusion ni deploiement.@ -197,0 +160,4 @@" [WARN] data-{} : boîtier inconnu, trame ignorée (ni écriture ni alerte)",boitier_id),Err(e) => eprintln!(" ❌ data-{}: {}", boitier_id, e),[majeur] Caractere emoji U+274C dans une ligne ajoutee (regle IA-23).
Source : emoji.
@ -210,2 +182,3 @@// chargées en mémoire pour evaluate_sensor_value.let device_key = id.to_string();match load_alert_rules_from_db(id as i64, &pool, &mut alert_manager, &device_key).await {match load_alert_rules_from_db(i64::from(id.get()), &pool, &mut data_ctx.alert_manager, &device_key).await {[majeur] Chaque firstco accepté, y compris le chemin idempotent AlreadyProvisioned, recharge les règles d'alerte via load_alert_rules_from_db, qui appelle register_rule ; celle-ci ajoute au vecteur sans dédoublonner (alert_manager.rs:103-106). Chaque rejeu duplique donc les règles en mémoire (croissance non bornée, alertes multipliées dans evaluate_sensor_value). Piste : remplacer les règles du boîtier (clear puis chargement) ou ne charger que sur Provisioned.
Source : revue.
@ -185,0 +261,4 @@Some((stored_serial,)) => {check_existing_serial(id, stored_serial.as_deref(), &payload.boitier.no_serie)}None => {[majeur] Premier arrivé, premier servi : tout client MQTT peut publier un firstco forgé pour un id pas encore provisionné (ids matériels prévisibles) et créer la fiche avec son propre numéro de série. Avec D4, le vrai boîtier est ensuite refusé définitivement (SerialMismatch, aucun ACK) jusqu'à une action opérateur, alors qu'avant L3 il écrasait la fiche. Piste : préenregistrement des ids et numéros attendus par l'opérateur (firstco ne crée plus), ou état « en attente de validation », en plus de l'ACL prévue en L6.
Source : revue.
@ -231,3 +335,1 @@// ── 4. UPSERT boitiers ────────────────────────────────────────────────────// INSERT avec id EXPLICITE (pré-programmé hardware, NON AUTO_INCREMENT).// ON DUPLICATE KEY UPDATE préserve la PK et les FK — PAS de REPLACE INTO !// ── INSERT boitiers ───────────────────────────────────────────────────────[bloquant] Provisionnement partiel devenu définitif : l'INSERT de la fiche boitiers est validé seul (autocommit), avant les DDL, la transaction des capteurs et les seuils d'alerte. Si une étape suivante échoue (erreur base transitoire, redémarrage du serveur, DDL refusé), le firstco rejoué par le boîtier trouve la fiche, rend AlreadyProvisioned (handler.rs:260-262) et reçoit l'ACK de succès, sans tables {id}boitier* ni capteurs : toutes les trames suivantes échouent dans store_frame. Avant L3, l'UPSERT rendait le rejeu réparateur. Piste : DDL d'abord, puis fiche, capteurs et seuils dans une seule transaction, ou compléter de façon idempotente les structures manquantes sur le chemin « même numéro » sans toucher à la fiche.
Source : revue.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.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.