test: add frontend unit suite + fix mixed local/UTC timestamps

Two CODE_AUDIT items in one session.

§3.2 — Frontend regression net (js/tests/, 35 tests, node:test, zero deps):
- harness.js loads app.js (monofile, no exports) into a node:vm with browser
  globals stubbed, surfacing internals via an export epilogue.
- crypto: deriveKeyAndVerifier (AES key == raw PBKDF2, cross-checked vs Node
  pbkdf2Sync), legacy-vs-v2 verifier decoupling, encrypt/decrypt round-trip,
  IV uniqueness, AEAD tamper/wrong-key.
- csv: parseCSV tokenizer, findColumn heuristics, Bitwarden/KeePass mapping.
- merge: applyRemoteSnapshot add/update/skip (LWW), tombstone delete,
  resurrection arbitration (both NaN branches), local-tombstone veto,
  additive folder merge. Only api() is stubbed; loadEntries/encryptImportEntry
  run for real.
- Wired as a build gate in BuildAssets.ps1 (after node --check, bypass
  PM_SKIP_TESTS=1).

§2.2 — Unify timestamps on UTC:
- Entry created_at/updated_at were written via Delphi FormatDateTime(Now)
  = LOCAL, while deleted_at/tombstones use SQLite CURRENT_TIMESTAMP = UTC.
  The tombstone-resurrection arbitration compared the two zones, skewing by
  the machine's UTC offset even single-device.
- Add NowUTC/NowUTCStr to PM.Database, swap in at every entry/attachment
  write site (Entries create/update/bulk, Attachments POST echo).
- No JS change needed: arbitration now compares same-zone values.
- Existing rows self-heal on next edit (no destructive migration).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
r-zakarya
2026-07-04 19:17:33 +01:00
parent 73e4e37f19
commit d9397881dc
12 changed files with 818 additions and 26 deletions
+53 -21
View File
@@ -114,20 +114,38 @@ WebDAV (Nextcloud, Apache mod_dav) supportent les ETags.
### 2.2 🟠 Arbitrage tombstone sensible à l'horloge
### 2.2 🟠 Arbitrage tombstone sensible à l'horloge — **corrigé (2026-07-04)**
`applyRemoteSnapshot()` compare `entry.updated_at > tombstone.deleted_at`
pour décider résurrection vs suppression. Ces timestamps viennent de
`FormatDateTime('yyyy-mm-dd hh:nn:ss', Now)` — **heure locale du device
qui a écrit**. Entre deux machines avec des horloges décalées (ou fuseaux
différents), l'arbitrage last-write-wins peut se tromper :
pour décider résurrection vs suppression.
- Device A (horloge en avance) supprime → deleted_at "futur"
- Device B édite (horloge correcte) → updated_at "passé" vs deleted_at A
- B croit que la suppression est plus récente → tue l'édition de B
**Bug réel trouvé** (pire que décrit) : deux sources d'horloge coexistaient
**sur un même device**. `created_at`/`updated_at` des entries étaient écrits
via Delphi `FormatDateTime(..., Now)` = **heure locale**, tandis que
`deleted_at` (soft-delete + tombstones) venait de SQLite
`CURRENT_TIMESTAMP` / `datetime('now')` = **UTC**. L'arbitrage comparait donc
un `updated_at` local à un `deleted_at` UTC → décalage = l'offset UTC de la
machine, **même en solo mono-device** (pas seulement entre devices décalés).
**Recommandation** : stocker les timestamps en **UTC ISO 8601** partout
(serveur ET snapshot), et idéalement un compteur logique (Lamport) en
complément pour les cas d'égalité. À minima, documenter que les horloges
des devices doivent être synchronisées (NTP).
**Fix** : helper `NowUTC` / `NowUTCStr` dans `PM.Database`, substitué à
`FormatDateTime(..., Now)` à tous les sites d'écriture d'entries/attachments
([PM.Handler.Entries.pas](delphi-backend/Handlers/PM.Handler.Entries.pas)
create/update/bulk, [PM.Handler.Attachments.pas](delphi-backend/Handlers/PM.Handler.Attachments.pas)).
Tout est désormais UTC (les paths SQLite l'étaient déjà). Côté JS aucun
changement nécessaire : `buildSyncSnapshot` utilise `toISOString()` (UTC) et
l'arbitrage compare deux valeurs du **même** fuseau → `Date.parse` applique
le même offset local aux deux, ordre relatif préservé.
**Rows existantes** : décision « laisser se soigner » — les anciennes lignes
gardent leur `updated_at` local jusqu'à leur prochain edit (qui le réécrit en
UTC). Exposition étroite pour un vault solo ; pas de migration destructive
(un shift par l'offset courant serait irréversible et approximatif : DST /
changement de fuseau historique).
**Reste (optionnel)** : compteur logique (Lamport) pour les cas d'égalité
stricte ; incohérence de **format** pré-existante (serveur `'yyyy-mm-dd
hh:nn:ss'` vs export JS `toISOString()` avec `T…Z`) sur les rares rows sans
`created_at`.
### 2.3 🟡 `catch (_) {}` silencieux en cascade
@@ -194,17 +212,31 @@ seulement au runtime).
→ aurait attrapé le syntax error avant le rebuild. Gain immédiat, coût
quasi nul.
### 3.2 🟡 Aucun test automatisé
### 3.2 🟡 Aucun test automatisé — **partiellement adressé (2026-07-04)**
Toute la validation est manuelle (TEST_PLAN.md, TEST_REGRESSION.md). Les
Toute la validation était manuelle (TEST_PLAN.md, TEST_REGRESSION.md). Les
zones à haut risque de régression (crypto round-trip, merge de sync,
arbitrage tombstone, dirty-check) sont exactement celles qui bénéficieraient
de tests unitaires.
arbitrage tombstone) sont exactement celles qui bénéficient d'un filet
unitaire.
**Recommandations** :
- Tests unitaires JS (Vitest/Jest) sur : `encryptPwd`/`decryptPwd` round-trip,
`deriveKeyAndVerifier` (vecteurs connus), `applyRemoteSnapshot` (merge +
résurrection), `isSoDirty`, `parseEntriesFromCSV/JSON`.
**Fait** : suite `js/tests/` (35 tests, Node `node:test`, zéro dépendance,
~1.7 s) — cf. [js/tests/README.md](js/tests/README.md). Harness `node:vm`
qui charge `app.js` (monofichier) avec globals navigateur stubbés :
- `crypto.test.js``deriveKeyAndVerifier` (clé AES == PBKDF2 brut,
cross-checké vs `pbkdf2Sync` Node), découplage verifier legacy vs `-v2`,
`encryptPwd`/`decryptPwd` round-trip, unicité IV, tamper/mauvaise clé → `[ERROR]`.
- `csv.test.js``parseCSV`, `findColumn`, `parseEntriesFromCSV` (Bitwarden/
KeePass, classification note vs login).
- `merge.test.js``applyRemoteSnapshot` : add/update/skip (LWW), tombstone
delete, arbitrage résurrection (2 branches `NaN`), veto tombstone local,
merge additif de folders. Seule `api()` est stubbée (fake server mémoire) ;
`loadEntries`/`encryptImportEntry` tournent en vrai.
Intégré comme **gate de build** dans `BuildAssets.ps1` (après `node --check`,
bypass `PM_SKIP_TESTS=1`).
**Reste à faire** :
- `isSoDirty` (couplé DOM/`soState` — nécessite plus de stubbing).
- Tests Delphi (DUnitX) sur les handlers critiques (bulk-import + tombstone
purge, rotation master pw).
@@ -255,8 +287,8 @@ cf. la checklist "Entry payload" de CLAUDE.md).
1. **`node --check` dans `BuildAssets.cmd`** — 5 min, évite les JS cassés en prod.
2. **Découpler verifier ↔ clé** (§1.1) — fix sécu prioritaire, effort faible.
3. **ETag/If-Match sur sync** (§2.1) — évite la perte de données multi-device.
4. **Timestamps UTC partout** (§2.2) — fiabilise l'arbitrage tombstone.
5. **Tests unitaires crypto + merge** (§3.2) — filet avant d'ajouter des features.
4. ~~**Timestamps UTC partout** (§2.2)~~ — ✅ fait (`NowUTCStr`, going-forward ; rows existantes self-heal).
5. ~~**Tests unitaires crypto + merge** (§3.2)~~ — ✅ fait (35 tests, `js/tests/`, gate de build).
6. **Argon2id** (§1.2) — durcissement KDF, migration progressive.
7. Découpage `app.js` en modules (§3.1) — maintenabilité long terme.