diff --git a/CLAUDE.md b/CLAUDE.md index 60560ee..96783dd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,6 +28,17 @@ lieu d'embarquer un bundle mort qui ne se révèle qu'après un rebuild Delphi complet). Node optionnel — absent = warning, on continue. Toujours `node --check js/app.js` après un gros edit JS. +Ensuite (même gate, node requis) il lance la **suite de tests frontend** +(`node --test js/tests/**/*.test.js`, ~1.7 s, zéro dépendance) : un +invariant crypto/merge cassé avorte le build comme une erreur de syntaxe. +`PM_SKIP_TESTS=1` pour bypasser en itération rapide. Lancer manuellement +via `npm test`. Voir [js/tests/README.md](js/tests/README.md) — le harness +charge `app.js` (monofichier sans exports) dans un `node:vm` avec les +globals navigateur stubbés, puis expose les internals via un épilogue +d'export. Couvre : round-trip crypto + dérivation verifier (legacy vs -v2), +parsing CSV d'import, et l'arbitrage merge/tombstone de sync +(`applyRemoteSnapshot`, seule `api()` est stubbée). + ## Carte des fichiers | Rôle | Path | @@ -800,6 +811,12 @@ le vide ET vide `FPendingURL` → écran noir permanent sur cold-start lent. nécessite `AttachThreadInput` pour cross-process. - **`updated_at`** est bumpé à chaque PUT entry → sort par défaut = `name` asc pour que modifier une entry ne change pas sa position. +- **Timestamps = UTC partout** : tout `created_at`/`updated_at`/`deleted_at` + écrit côté Delphi passe par `NowUTC`/`NowUTCStr` (`PM.Database`) — JAMAIS + `FormatDateTime(..., Now)` (heure locale). SQLite `CURRENT_TIMESTAMP` / + `datetime('now')` sont déjà UTC. Mélanger les deux décalait l'arbitrage + sync (résurrection tombstone) de l'offset UTC de la machine, même en solo. + Côté JS, générer les timestamps via `toISOString()` (UTC) uniquement. - **TMS render-time vs DOM-attachment** : pattern courant — fonctions qui rendent un widget (`soTagsField`, `soTotpField`) appellent un helper (`renderSoChips`, `startTotpTick`) qui fait `$('#id')` sur un diff --git a/CODE_AUDIT.md b/CODE_AUDIT.md index fcbedb9..c41301d 100644 --- a/CODE_AUDIT.md +++ b/CODE_AUDIT.md @@ -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. diff --git a/delphi-backend/Handlers/PM.Handler.Attachments.pas b/delphi-backend/Handlers/PM.Handler.Attachments.pas index 4daea21..6c26b2e 100644 --- a/delphi-backend/Handlers/PM.Handler.Attachments.pas +++ b/delphi-backend/Handlers/PM.Handler.Attachments.pas @@ -210,7 +210,9 @@ begin LObj.AddPair('filename', LFilename); LObj.AddPair('mime', LMime); LObj.AddPair('size_bytes', TJSONNumber.Create(LSize)); - LObj.AddPair('created_at', FormatDateTime('yyyy-mm-dd"T"hh:nn:ss', Now)); + // UTC to match the stored value (schema default CURRENT_TIMESTAMP is UTC) + // and the GET-list formatter — the POST response echoed local time before. + LObj.AddPair('created_at', FormatDateTime('yyyy-mm-dd"T"hh:nn:ss', NowUTC)); TJSONHelper.SendJSON(AResponse, LObj); end; diff --git a/delphi-backend/Handlers/PM.Handler.Entries.pas b/delphi-backend/Handlers/PM.Handler.Entries.pas index fe2e7b8..04828da 100644 --- a/delphi-backend/Handlers/PM.Handler.Entries.pas +++ b/delphi-backend/Handlers/PM.Handler.Entries.pas @@ -350,7 +350,7 @@ begin Exit; end; - LNow := FormatDateTime('yyyy-mm-dd hh:nn:ss', Now); + LNow := NowUTCStr; // UTC — matches SQLite CURRENT_TIMESTAMP (see CODE_AUDIT §2.2) DB.Lock; try @@ -493,7 +493,7 @@ begin Exit; end; - LNow := FormatDateTime('yyyy-mm-dd hh:nn:ss', Now); + LNow := NowUTCStr; // UTC — matches SQLite CURRENT_TIMESTAMP (see CODE_AUDIT §2.2) DB.Lock; try LQ := TFDQuery.Create(nil); @@ -1138,7 +1138,7 @@ begin Exit; end; - LNow := FormatDateTime('yyyy-mm-dd hh:nn:ss', Now); + LNow := NowUTCStr; // UTC — matches SQLite CURRENT_TIMESTAMP (see CODE_AUDIT §2.2) LImported := 0; // Track newly-inserted IDs in input order so the client can upload // attachments to the right entry afterwards. Skipped rows emit -1 diff --git a/delphi-backend/Source/PM.Database.pas b/delphi-backend/Source/PM.Database.pas index 6492015..bb63897 100644 --- a/delphi-backend/Source/PM.Database.pas +++ b/delphi-backend/Source/PM.Database.pas @@ -48,8 +48,29 @@ var procedure InitDatabase(const ADBPath: string); procedure DoneDatabase; +// Current time as UTC. SQLite's CURRENT_TIMESTAMP / datetime('now') already +// emit UTC, so every Delphi-written timestamp (created_at, updated_at, +// deleted_at, …) MUST use these — mixing Delphi's local-time Now with the +// SQLite-UTC values silently skews the sync last-write-wins / tombstone +// resurrection arbitration by the machine's UTC offset (see CODE_AUDIT §2.2). +function NowUTC: TDateTime; +function NowUTCStr: string; // 'yyyy-mm-dd hh:nn:ss' in UTC + implementation +uses + System.DateUtils; // TTimeZone for local→UTC conversion + +function NowUTC: TDateTime; +begin + Result := TTimeZone.Local.ToUniversalTime(Now); +end; + +function NowUTCStr: string; +begin + Result := FormatDateTime('yyyy-mm-dd hh:nn:ss', NowUTC); +end; + constructor TPMDatabase.Create(const ADBPath: string); begin inherited Create; diff --git a/delphi-backend/assets/BuildAssets.ps1 b/delphi-backend/assets/BuildAssets.ps1 index 056b302..5c2f67e 100644 --- a/delphi-backend/assets/BuildAssets.ps1 +++ b/delphi-backend/assets/BuildAssets.ps1 @@ -102,8 +102,34 @@ if ($jsFiles) { } } Log "JS syntax OK." + + # --- Unit test gate --------------------------------------------------- + # Run the frontend regression suite (crypto round-trip, CSV import, + # sync-merge arbitration) before embedding. A broken crypto/merge + # invariant now blocks the build the same way a syntax error does, + # instead of surfacing only after a full Delphi rebuild + manual test. + # ~1.7 s, zero deps (node:test). Skipped automatically if the suite + # isn't present. Set PM_SKIP_TESTS=1 to bypass during rapid iteration. + if ($env:PM_SKIP_TESTS -eq '1') { + Log "PM_SKIP_TESTS=1 - skipping unit tests." + } elseif (Test-Path (Join-Path $WebRoot 'js\tests')) { + Log "Running frontend unit tests..." + Push-Location $WebRoot + try { + $testOut = $null | & $node.Source --test 'js/tests/**/*.test.js' 2>&1 + $testExit = $LASTEXITCODE + } finally { + Pop-Location + } + if ($testExit -ne 0) { + Log "UNIT TESTS FAILED:" + Log ($testOut | Out-String) + throw "Frontend unit tests failed - aborting asset build. (Set PM_SKIP_TESTS=1 to bypass.)" + } + Log "Unit tests OK." + } } else { - Log "WARNING: node not found - skipping JS syntax check. Install Node to enable it." + Log "WARNING: node not found - skipping JS syntax check + unit tests. Install Node to enable them." } } diff --git a/js/tests/README.md b/js/tests/README.md new file mode 100644 index 0000000..eb6d8fd --- /dev/null +++ b/js/tests/README.md @@ -0,0 +1,58 @@ +# Frontend unit tests + +Regression net for the highest-risk pure/near-pure logic in `js/app.js`: +crypto round-trip, KDF/verifier derivation, CSV import parsing, and the +sync-merge / tombstone-resurrection arbitration. Addresses `CODE_AUDIT.md` +§3.2 (aucun test automatisé). + +## Running + +``` +npm test +# or directly: +node --test "js/tests/**/*.test.js" +``` + +Zero dependencies — uses the Node built-in test runner (`node:test`) and +`webcrypto`. Requires Node ≥ 18 (developed on v24). Runs in ~1.7 s. + +## How it works — `harness.js` + +`app.js` is a ~12k-line browser monofile with **no module exports** and one +top-level side effect (a `DOMContentLoaded` listener). The harness loads the +file's source into a `node:vm` context with browser globals stubbed +(`crypto`, `localStorage`, `document`, `location`, …) so `init()` never +fires, then appends an export epilogue that surfaces the internals on +`globalThis.__test`. + +Two gotchas the harness works around, both documented inline: + +- **`const`/`let` don't attach to the vm global.** Top-level `function`/`var` + declarations become properties of the context global, but `const state`, + `const HASH_ALGO_V2`, etc. do not — hence the explicit export epilogue. +- **Cross-realm prototypes.** Values returned from the sandbox carry the + sandbox realm's prototypes, so `assert.deepStrictEqual` trips on the + prototype check. Structural comparisons normalize through JSON first + (see `eqDeep` in `csv.test.js`). + +The merge tests stub only the `api()` seam (a `function` declaration → +overridable global property) with an in-memory fake server; `loadEntries`, +`loadFolders`, and `encryptImportEntry` all run for real against it — so the +tests exercise the actual pull→merge path, not a re-implementation. + +## Suites + +| File | Covers | +|---|---| +| `crypto.test.js` | `deriveKeyAndVerifier` (AES key == raw PBKDF2, cross-checked vs Node's `pbkdf2Sync`), legacy vs `-v2` verifier decoupling, `encryptPwd`/`decryptPwd` round-trip, IV uniqueness, AEAD tamper/wrong-key → `[ERROR]` | +| `csv.test.js` | `parseCSV` tokenizer (quotes, escaped `""`, CRLF, trailing field), `findColumn` header heuristics, `parseEntriesFromCSV` for Bitwarden/KeePass shapes, note-vs-login classification | +| `merge.test.js` | `applyRemoteSnapshot`: add/update/skip (last-write-wins), tombstone delete, resurrection arbitration (both `NaN` branches), local-tombstone veto, additive folder merge | + +## Notes on encoded behavior + +`merge.test.js` locks in one deliberately-asymmetric behavior: an unparseable +**local** `updated_at` favours KEEP (resurrection wins), but an unparseable +**remote** `deleted_at` still applies the delete. In production `deleted_at` +is always server-formatted (parseable), so this only matters for a corrupted +remote snapshot. If that arbitration is ever changed, the two +`unparseable …` tests are where to update the expectation. diff --git a/js/tests/crypto.test.js b/js/tests/crypto.test.js new file mode 100644 index 0000000..327b0f7 --- /dev/null +++ b/js/tests/crypto.test.js @@ -0,0 +1,129 @@ +// Crypto round-trip + verifier derivation tests. +// +// These lock down the invariants the CODE_AUDIT flagged as highest-risk for +// silent regression: the AES key must ALWAYS be raw PBKDF2 bytes (so entries +// stay decryptable across auth-scheme changes), and the transmitted verifier +// must be decoupled from that key under the v2 scheme. + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const { pbkdf2Sync, createHash } = require('node:crypto'); +const { loadApp } = require('./harness.js'); + +const ctx = loadApp(); +const T = ctx.__test; + +// Reference PBKDF2 computed independently via Node (NOT via app.js) so the +// vectors actually cross-check rather than being self-referential. app.js +// feeds the salt STRING's UTF-8 bytes to PBKDF2 (salt = enc.encode(saltHex)), +// so the Node reference must do the same. +function refKeyHex(pwd, saltHex, iters) { + return pbkdf2Sync(pwd, Buffer.from(saltHex, 'utf8'), iters, 32, 'sha256').toString('hex'); +} +function refSha256Hex(str) { + return createHash('sha256').update(str, 'utf8').digest('hex'); +} + +test('bytesToHex: lowercase, zero-padded, round-trips known bytes', () => { + assert.equal(T.bytesToHex(new Uint8Array([0, 1, 15, 16, 255])), '00010f10ff'); + assert.equal(T.bytesToHex(new Uint8Array([])), ''); +}); + +test('deriveKeyAndVerifier: AES key is raw PBKDF2 output (matches Node reference)', async () => { + const pwd = 'correct horse battery staple'; + const saltHex = 'a1b2c3d4e5f6'; + const iters = 600000; + const expectKeyHex = refKeyHex(pwd, saltHex, iters); + + const { cryptoKey } = await T.deriveKeyAndVerifier(pwd, saltHex, iters, T.HASH_ALGO_V2); + const raw = await ctx.crypto.subtle.exportKey('raw', cryptoKey); + assert.equal(T.bytesToHex(new Uint8Array(raw)), expectKeyHex, + 'AES key must equal hex(PBKDF2) regardless of hash_algo'); +}); + +test('deriveKeyAndVerifier: legacy algo verifier IS the key hex', async () => { + const pwd = 'hunter2'; + const saltHex = 'deadbeef'; + const iters = 100000; + const expectKeyHex = refKeyHex(pwd, saltHex, iters); + + // Any non-v2 label → verifier verbatim = key hex (pre-decoupling accounts). + const { verifier } = await T.deriveKeyAndVerifier(pwd, saltHex, iters, 'pbkdf2-sha256'); + assert.equal(verifier, expectKeyHex); + + // Unknown/empty algo must also fall through to key hex (safe rollout path). + const empty = await T.deriveKeyAndVerifier(pwd, saltHex, iters, ''); + assert.equal(empty.verifier, expectKeyHex); +}); + +test('deriveKeyAndVerifier: v2 verifier is decoupled SHA-256(keyHex + domain)', async () => { + const pwd = 'hunter2'; + const saltHex = 'deadbeef'; + const iters = 100000; + const keyHex = refKeyHex(pwd, saltHex, iters); + const expectVerifier = refSha256Hex(keyHex + T.AUTH_VERIFIER_DOMAIN); + + const { verifier } = await T.deriveKeyAndVerifier(pwd, saltHex, iters, T.HASH_ALGO_V2); + assert.equal(verifier, expectVerifier); + // The whole point of v2: the transmitted verifier must NOT be the key. + assert.notEqual(verifier, keyHex, 'v2 verifier must not leak the AES key'); +}); + +test('verifierFromKeyHex: pure mapping matches deriveKeyAndVerifier', async () => { + const keyHex = 'ab'.repeat(32); + assert.equal(await T.verifierFromKeyHex(keyHex, 'anything-legacy'), keyHex); + assert.equal(await T.verifierFromKeyHex(keyHex, T.HASH_ALGO_V2), + refSha256Hex(keyHex + T.AUTH_VERIFIER_DOMAIN)); +}); + +test('deriveKeyAndVerifier: default iterations = 100000 when falsy', async () => { + const pwd = 'x'; + const saltHex = 'salt'; + const withDefault = await T.deriveKeyAndVerifier(pwd, saltHex, 0, 'pbkdf2-sha256'); + assert.equal(withDefault.verifier, refKeyHex(pwd, saltHex, 100000)); +}); + +test('encryptPwd/decryptPwd: round-trips arbitrary strings under the vault key', async () => { + // encryptPwd/decryptPwd read state.cryptoKey — set it to a derived key. + const { cryptoKey } = await T.deriveKeyAndVerifier('master', 'saltsalt', 100000, T.HASH_ALGO_V2); + T.state.cryptoKey = cryptoKey; + + for (const plain of ['', 'a', 'password123!', 'emoji 🔐 unicode ✓', 'x'.repeat(5000)]) { + const { encrypted, iv } = await T.encryptPwd(plain); + assert.equal(await T.decryptPwd(encrypted, iv), plain, `round-trip failed for len ${plain.length}`); + } +}); + +test('encryptPwd: fresh random IV per call (no IV reuse)', async () => { + const { cryptoKey } = await T.deriveKeyAndVerifier('m', 's', 100000, T.HASH_ALGO_V2); + T.state.cryptoKey = cryptoKey; + const a = await T.encryptPwd('same-plaintext'); + const b = await T.encryptPwd('same-plaintext'); + assert.notEqual(a.iv, b.iv, 'IVs must differ'); + assert.notEqual(a.encrypted, b.encrypted, 'ciphertext must differ for reused plaintext'); +}); + +test('decryptPwd: tampered ciphertext returns "[ERROR]" (AEAD integrity)', async () => { + const { cryptoKey } = await T.deriveKeyAndVerifier('m', 's', 100000, T.HASH_ALGO_V2); + T.state.cryptoKey = cryptoKey; + const { encrypted, iv } = await T.encryptPwd('secret'); + + // Flip a byte in the ciphertext. + const bytes = Uint8Array.from(atob(encrypted), c => c.charCodeAt(0)); + bytes[0] ^= 0xff; + const tampered = btoa(String.fromCharCode(...bytes)); + assert.equal(await T.decryptPwd(tampered, iv), '[ERROR]'); + + // Wrong IV also fails closed. + assert.equal(await T.decryptPwd(encrypted, btoa('bad-iv-1234')), '[ERROR]'); +}); + +test('decryptPwd: wrong key returns "[ERROR]" (not garbage plaintext)', async () => { + const k1 = await T.deriveKeyAndVerifier('pw-one', 'salt', 100000, T.HASH_ALGO_V2); + T.state.cryptoKey = k1.cryptoKey; + const { encrypted, iv } = await T.encryptPwd('top secret'); + + const k2 = await T.deriveKeyAndVerifier('pw-two', 'salt', 100000, T.HASH_ALGO_V2); + T.state.cryptoKey = k2.cryptoKey; + assert.equal(await T.decryptPwd(encrypted, iv), '[ERROR]'); +}); diff --git a/js/tests/csv.test.js b/js/tests/csv.test.js new file mode 100644 index 0000000..baa4968 --- /dev/null +++ b/js/tests/csv.test.js @@ -0,0 +1,114 @@ +// CSV import parsing tests — parseCSV (RFC-ish tokenizer), findColumn +// (header heuristics), and parseEntriesFromCSV (multi-format mapping). +// These are pure functions with no state/DOM coupling. + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const { loadApp } = require('./harness.js'); + +const T = loadApp().__test; + +// Values returned from the vm sandbox carry the sandbox realm's prototypes, +// so deepStrictEqual's prototype check trips. Normalize through JSON to +// compare by structure (fine for plain data). +const eqDeep = (actual, expected, msg) => + assert.deepEqual(JSON.parse(JSON.stringify(actual)), expected, msg); + +test('parseCSV: simple rows', () => { + eqDeep(T.parseCSV('a,b,c\n1,2,3'), [['a', 'b', 'c'], ['1', '2', '3']]); +}); + +test('parseCSV: quoted fields with commas and newlines', () => { + const rows = T.parseCSV('name,note\n"Smith, John","line1\nline2"'); + eqDeep(rows, [['name', 'note'], ['Smith, John', 'line1\nline2']]); +}); + +test('parseCSV: escaped double-quotes ("")', () => { + const rows = T.parseCSV('v\n"say ""hi"""'); + eqDeep(rows, [['v'], ['say "hi"']]); +}); + +test('parseCSV: CRLF line endings', () => { + eqDeep(T.parseCSV('a,b\r\n1,2\r\n'), [['a', 'b'], ['1', '2']]); +}); + +test('parseCSV: trailing field with no final newline', () => { + eqDeep(T.parseCSV('a,b\n1,2'), [['a', 'b'], ['1', '2']]); +}); + +test('parseCSV: blank lines are dropped', () => { + eqDeep(T.parseCSV('a,b\n\n1,2\n'), [['a', 'b'], ['1', '2']]); +}); + +test('findColumn: case/underscore/dash-insensitive matching', () => { + const headers = ['Login_URI', 'User Name', 'PASSWORD']; + assert.equal(T.findColumn(headers, ['login_uri']), 0); + assert.equal(T.findColumn(headers, ['username', 'user_name']), 1); + assert.equal(T.findColumn(headers, ['password']), 2); + assert.equal(T.findColumn(headers, ['nope']), null); +}); + +test('parseEntriesFromCSV: Bitwarden-style header maps site/user/pwd/totp', () => { + const csv = [ + 'folder,name,login_uri,login_username,login_password,login_totp,notes', + 'Work,GitHub,https://github.com,octocat,s3cret,JBSWY3DPEHPK3PXP,hello', + ].join('\n'); + const { entries, skipped } = T.parseEntriesFromCSV(csv); + assert.equal(skipped, 0); + assert.equal(entries.length, 1); + const e = entries[0]; + assert.equal(e.title, 'GitHub'); + assert.equal(e.site, 'https://github.com'); + assert.equal(e.username, 'octocat'); + assert.equal(e.password, 's3cret'); + assert.equal(e.totp_secret, 'JBSWY3DPEHPK3PXP'); + assert.equal(e.folder, 'Work'); +}); + +test('parseEntriesFromCSV: KeePass-style header (Title/URL/Username/Password/Group)', () => { + const csv = [ + 'Title,URL,Username,Password,Group,Notes', + 'Bank,https://bank.example,alice,pw123,Finance,note', + ].join('\n'); + const { entries } = T.parseEntriesFromCSV(csv); + assert.equal(entries[0].title, 'Bank'); + assert.equal(entries[0].site, 'https://bank.example'); + assert.equal(entries[0].username, 'alice'); + assert.equal(entries[0].folder, 'Finance'); +}); + +test('parseEntriesFromCSV: note heuristic — empty site+pwd but notes present → kind=note', () => { + const csv = [ + 'name,url,username,password,notes', + 'My Note,,,,"just some text"', + ].join('\n'); + const { entries } = T.parseEntriesFromCSV(csv); + assert.equal(entries.length, 1); + assert.equal(entries[0].kind, 'note'); +}); + +test('parseEntriesFromCSV: explicit kind=note wins even with site+password present', () => { + // A row with site + password normally classifies as a login; an explicit + // kind=note must override that (precedence: explicit column > heuristic). + const csv = [ + 'name,url,username,password,kind', + 'Recovery Codes,https://example.com,user,BACKUP-CODES,note', + ].join('\n'); + const { entries } = T.parseEntriesFromCSV(csv); + assert.equal(entries.length, 1); + assert.equal(entries[0].kind, 'note'); + assert.equal(entries[0].site, ''); // notes never carry a site + assert.equal(entries[0].password, 'BACKUP-CODES'); // body preserved +}); + +test('parseEntriesFromCSV: throws on missing password column', () => { + assert.throws(() => T.parseEntriesFromCSV('name,url\nfoo,bar'), /password column/i); +}); + +test('parseEntriesFromCSV: throws when no header + data rows', () => { + assert.throws(() => T.parseEntriesFromCSV('name,password'), /header row and at least one data row/i); +}); + +test('parseEntriesFromCSV: throws when no title/url/username column present', () => { + assert.throws(() => T.parseEntriesFromCSV('password,foo\npw,x'), /title\/url or username/i); +}); diff --git a/js/tests/harness.js b/js/tests/harness.js new file mode 100644 index 0000000..6b93914 --- /dev/null +++ b/js/tests/harness.js @@ -0,0 +1,142 @@ +// ============================================================ +// Test harness — load js/app.js into a sandboxed VM context +// ============================================================ +// +// app.js is a ~12k-line browser monofile with no module exports. It runs +// only one top-level statement (a DOMContentLoaded listener); everything +// else is function/const declarations. We load it in a node:vm context with +// browser globals stubbed out so init() never fires, then reach the internals +// we want to test through an appended export epilogue. +// +// Why the epilogue: in vm.runInContext, top-level `function`/`var` declarations +// attach to the context's global object, but top-level `const`/`let` (like +// `state`, `API`, `HASH_ALGO_V2`) do NOT. So we append a line that copies the +// symbols we care about onto globalThis.__test, giving tests a stable handle. +// +// Reassignable seams for the merge tests: `api`, `loadEntries`, `loadFolders`, +// `encryptImportEntry`, etc. are `function` declarations → global properties, +// so a test can override `ctx.api = fake` and the free-variable lookup inside +// applyRemoteSnapshot will pick up the fake. `state` is a `const` (lexical), +// so it can't be replaced — but it CAN be mutated, and applyRemoteSnapshot +// closes over that same object, so mutating ctx.__test.state is visible to it. + +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); +const { webcrypto } = require('node:crypto'); + +const APP_JS = path.join(__dirname, '..', 'app.js'); + +// In-memory Storage stub (Web Storage API surface used by app.js). +function makeStorage() { + const m = new Map(); + return { + getItem: (k) => (m.has(k) ? m.get(k) : null), + setItem: (k, v) => { m.set(k, String(v)); }, + removeItem: (k) => { m.delete(k); }, + clear: () => m.clear(), + key: (i) => Array.from(m.keys())[i] ?? null, + get length() { return m.size; }, + }; +} + +// Minimal no-throw DOM/window stubs. app.js only *executes* one DOM call at +// load (document.addEventListener for DOMContentLoaded) — everything else is +// inside functions we don't call. So these just have to exist and not throw. +function makeDomStubs() { + const noop = () => {}; + const elStub = new Proxy({}, { + get: (_t, prop) => { + if (prop === 'style') return {}; + if (prop === 'classList') return { add: noop, remove: noop, toggle: noop, contains: () => false }; + if (prop === 'addEventListener' || prop === 'removeEventListener') return noop; + if (prop === 'appendChild' || prop === 'append' || prop === 'remove') return noop; + if (prop === 'setAttribute' || prop === 'removeAttribute') return noop; + if (prop === 'querySelector') return () => null; + if (prop === 'querySelectorAll') return () => []; + return undefined; + }, + set: () => true, + }); + const document = { + addEventListener: noop, + removeEventListener: noop, + getElementById: () => null, + querySelector: () => null, + querySelectorAll: () => [], + createElement: () => elStub, + body: elStub, + documentElement: elStub, + }; + return { document, elStub, noop }; +} + +// Build a fresh sandbox + load app.js into it. Returns the contextified +// sandbox; test internals live on ctx.__test. +function loadApp(overrides = {}) { + const { document, noop } = makeDomStubs(); + + const sandbox = { + crypto: webcrypto, + TextEncoder, + TextDecoder, + btoa, + atob, + console, + setTimeout, + clearTimeout, + setInterval, + clearInterval, + Date, + JSON, + Math, + Promise, + URL, + URLSearchParams, + // Browser-ish globals used at load time + location: { pathname: '/index.html', href: 'http://127.0.0.1/index.html', search: '', hash: '' }, + history: { replaceState: noop, pushState: noop }, + navigator: { clipboard: { writeText: async () => {}, readText: async () => '' }, userAgent: 'node-test' }, + localStorage: makeStorage(), + sessionStorage: makeStorage(), + document, + fetch: async () => { throw new Error('fetch not stubbed'); }, + // Some code paths reference matchMedia / requestAnimationFrame + matchMedia: () => ({ matches: false, addEventListener: noop, addListener: noop }), + requestAnimationFrame: (cb) => setTimeout(cb, 0), + ...overrides, + }; + // window / self / globalThis self-reference (app.js reads window.location etc.) + sandbox.window = sandbox; + sandbox.self = sandbox; + sandbox.globalThis = sandbox; + + vm.createContext(sandbox); + + let src = fs.readFileSync(APP_JS, 'utf8'); + + // Export epilogue — surface the lexical (const) symbols we test, plus a + // couple of function-decl seams for convenience. Kept in one place so the + // list of "what tests can touch" is explicit. + src += ` + ;globalThis.__test = { + state, + // crypto + bytesToHex, hexToBytes: (typeof hexToBytes !== 'undefined' ? hexToBytes : undefined), + verifierFromKeyHex, deriveKeyAndVerifier, computeVerifier, + encryptPwd, decryptPwd, sha256Hex, + HASH_ALGO_V2, AUTH_VERIFIER_DOMAIN, + // csv + parseCSV, findColumn, parseEntriesFromCSV, + // strength + computeStrength: (typeof computeStrength !== 'undefined' ? computeStrength : undefined), + // merge (async, coupled — tests stub the io seams below) + applyRemoteSnapshot, buildSyncSnapshot, + }; + `; + + vm.runInContext(src, sandbox, { filename: 'app.js' }); + return sandbox; +} + +module.exports = { loadApp, makeStorage }; diff --git a/js/tests/merge.test.js b/js/tests/merge.test.js new file mode 100644 index 0000000..b4340ba --- /dev/null +++ b/js/tests/merge.test.js @@ -0,0 +1,239 @@ +// Sync merge / resurrection-arbitration tests — the highest-risk logic in +// applyRemoteSnapshot (CODE_AUDIT §2.2). We run the REAL merge function +// against an in-memory fake WebDAV/DB by stubbing only the `api()` seam; +// loadEntries/loadFolders/encryptImportEntry all run for real (with a real +// AES key), so this exercises the full pull→merge path, not a re-implementation. + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const { loadApp } = require('./harness.js'); + +// Build a fresh app context wired to an in-memory fake server. Returns +// { ctx, T, db, install } — call install() to point ctx.api at the fake. +async function freshMerge() { + const ctx = loadApp(); + const T = ctx.__test; + + // Real vault key so encryptImportEntry (called for POST/PUT) works. + const { cryptoKey } = await T.deriveKeyAndVerifier('master-pw', 'saltsalt', 100000, T.HASH_ALGO_V2); + T.state.cryptoKey = cryptoKey; + T.state.token = 'tok'; + T.state.username = 'alice'; + + // Fake persistent store. Entries keyed by uuid; ids auto-increment. + const db = { + entries: [], // { id, uuid, updated_at, site, ... } + tombstones: [], // { uuid, deleted_at } + folders: [], // { name, color, icon } + _nextId: 1, + seedEntry(e) { + const row = Object.assign({ id: this._nextId++, updated_at: '', kind: 'login' }, e); + this.entries.push(row); + return row; + }, + }; + + // Minimal router mirroring the endpoints applyRemoteSnapshot touches. + async function fakeApi(path, opts) { + opts = opts || {}; + const method = (opts.method || 'GET').toUpperCase(); + const body = opts.body ? JSON.parse(opts.body) : null; + + if (path === '/entries' && method === 'GET') { + return db.entries.map(e => Object.assign({}, e)); + } + if (path === '/folders' && method === 'GET') { + return db.folders.map(f => Object.assign({}, f)); + } + if (path === '/folders' && method === 'POST') { + db.folders.push({ name: body.name, color: body.color || '', icon: body.icon || '' }); + return { ok: true }; + } + if (path === '/entries/tombstones' && method === 'GET') { + return db.tombstones.map(t => Object.assign({}, t)); + } + if (path === '/entries/tombstones' && method === 'POST') { + for (const uuid of (body.uuids || [])) { + // Hard-delete matching live rows + record the tombstone. + db.entries = db.entries.filter(e => e.uuid !== uuid); + if (!db.tombstones.some(t => t.uuid === uuid)) + db.tombstones.push({ uuid, deleted_at: new Date().toISOString() }); + } + return { ok: true }; + } + if (path === '/entries' && method === 'POST') { + // Purge any tombstone for this uuid (CLAUDE.md: purge-on-insert). + db.tombstones = db.tombstones.filter(t => t.uuid !== body.uuid); + const row = Object.assign({ id: db._nextId++ }, body); + // Carry updated_at from the remote snapshot if the merge preserved + // it; encryptImportEntry drops it, so default to "now". + if (!row.updated_at) row.updated_at = new Date().toISOString(); + db.entries.push(row); + return { id: row.id }; + } + const mPut = path.match(/^\/entries\/(\d+)$/); + if (mPut && method === 'PUT') { + const id = parseInt(mPut[1], 10); + const row = db.entries.find(e => e.id === id); + if (row) Object.assign(row, body); + return { ok: true }; + } + const mAtt = path.match(/^\/entries\/(\d+)\/attachments$/); + if (mAtt && method === 'GET') return []; + if (mAtt && method === 'POST') return { ok: true }; + + throw new Error('fakeApi: unhandled ' + method + ' ' + path); + } + + ctx.api = fakeApi; + return { ctx, T, db }; +} + +const remoteEntry = (o) => Object.assign({ + uuid: '', site: 'https://x', title: '', username: 'u', password: 'p', + folder: 'All', tags: [], favorite: false, totp_secret: '', kind: 'login', + template: '', custom_fields: [], attachments: [], icon_b64: '', + created_at: '2026-01-01T00:00:00Z', updated_at: '2026-01-01T00:00:00Z', +}, o); + +test('merge: remote-only entry is added locally, keeping its uuid', async () => { + const { T, db } = await freshMerge(); + const res = await T.applyRemoteSnapshot({ + entries: [remoteEntry({ uuid: 'uuid-new', site: 'https://new.example' })], + tombstones: [], folders: [], + }); + assert.equal(res.added, 1); + assert.equal(res.updated, 0); + assert.equal(db.entries.length, 1); + assert.equal(db.entries[0].uuid, 'uuid-new'); + assert.equal(db.entries[0].site, 'https://new.example'); +}); + +test('merge: remote entry newer than local → PUT updates it', async () => { + const { T, db } = await freshMerge(); + db.seedEntry({ uuid: 'u1', site: 'https://old', updated_at: '2026-01-01T00:00:00Z' }); + const res = await T.applyRemoteSnapshot({ + entries: [remoteEntry({ uuid: 'u1', site: 'https://newer', updated_at: '2026-06-01T00:00:00Z' })], + tombstones: [], folders: [], + }); + assert.equal(res.updated, 1); + assert.equal(res.added, 0); + assert.equal(db.entries[0].site, 'https://newer'); +}); + +test('merge: remote entry OLDER than local → skipped (last-write-wins keeps local)', async () => { + const { T, db } = await freshMerge(); + db.seedEntry({ uuid: 'u1', site: 'https://local-wins', updated_at: '2026-06-01T00:00:00Z' }); + const res = await T.applyRemoteSnapshot({ + entries: [remoteEntry({ uuid: 'u1', site: 'https://stale', updated_at: '2026-01-01T00:00:00Z' })], + tombstones: [], folders: [], + }); + assert.equal(res.updated, 0); + assert.equal(db.entries[0].site, 'https://local-wins'); +}); + +test('merge: remote tombstone deletes an untouched local entry', async () => { + const { T, db } = await freshMerge(); + db.seedEntry({ uuid: 'u1', updated_at: '2026-01-01T00:00:00Z' }); + const res = await T.applyRemoteSnapshot({ + entries: [], + tombstones: [{ uuid: 'u1', deleted_at: '2026-06-01T00:00:00Z' }], + folders: [], + }); + assert.equal(res.deleted, 1); + assert.equal(db.entries.length, 0); + assert.ok(db.tombstones.some(t => t.uuid === 'u1')); +}); + +test('merge: RESURRECTION — local edit newer than tombstone survives the delete', async () => { + const { T, db } = await freshMerge(); + // Local entry restored/edited AFTER the remote deletion timestamp. + db.seedEntry({ uuid: 'u1', site: 'https://restored', updated_at: '2026-06-02T00:00:00Z' }); + const res = await T.applyRemoteSnapshot({ + entries: [], + tombstones: [{ uuid: 'u1', deleted_at: '2026-06-01T00:00:00Z' }], + folders: [], + }); + assert.equal(res.deleted, 0, 'a newer local edit must beat the tombstone'); + assert.equal(db.entries.length, 1, 'resurrected entry stays'); + assert.equal(db.entries[0].site, 'https://restored'); +}); + +test('merge: unparseable LOCAL updated_at favours KEEP (resurrection wins)', async () => { + // If we can't parse the local timestamp we can't prove the entry is older + // than the deletion → keep it (data-loss is worse than a stale row). + const { T, db } = await freshMerge(); + db.seedEntry({ uuid: 'u1', updated_at: 'garbage-timestamp' }); + const res = await T.applyRemoteSnapshot({ + entries: [], + tombstones: [{ uuid: 'u1', deleted_at: '2026-06-01T00:00:00Z' }], + folders: [], + }); + assert.equal(res.deleted, 0, 'unparseable local updated_at → keep'); + assert.equal(db.entries.length, 1); +}); + +test('merge: unparseable remote deleted_at → delete IS applied (delete-intent honored)', async () => { + // Documents the deliberate asymmetry: a tombstone with an unknown time + // still expresses delete intent, and the local side has a parseable + // updated_at that it can't prove is newer, so the delete goes through. + // (In production deleted_at is always server-formatted → parseable; this + // is the corrupted-snapshot edge.) + const { T, db } = await freshMerge(); + db.seedEntry({ uuid: 'u1', updated_at: '2026-01-01T00:00:00Z' }); + const res = await T.applyRemoteSnapshot({ + entries: [], + tombstones: [{ uuid: 'u1', deleted_at: 'not-a-date' }], + folders: [], + }); + assert.equal(res.deleted, 1, 'unparseable deleted_at + parseable local → apply delete'); + assert.equal(db.entries.length, 0); +}); + +test('merge: local tombstone vetoes a remote entry with the same uuid', async () => { + const { T, db } = await freshMerge(); + // This device already hard-deleted u1 (tombstone present, no live row). + db.tombstones.push({ uuid: 'u1', deleted_at: '2026-06-01T00:00:00Z' }); + const res = await T.applyRemoteSnapshot({ + entries: [remoteEntry({ uuid: 'u1', site: 'https://should-not-resurrect' })], + tombstones: [], folders: [], + }); + assert.equal(res.added, 0, 'local tombstone must veto the remote entry'); + assert.equal(db.entries.length, 0); +}); + +test('merge: missing remote folders are added additively', async () => { + const { T, db } = await freshMerge(); + db.folders.push({ name: 'Existing', color: '#111', icon: '' }); + await T.applyRemoteSnapshot({ + entries: [], + tombstones: [], + folders: [ + { name: 'Existing', color: '#999', icon: '' }, // must NOT overwrite + { name: 'FromRemote', color: '#0f0', icon: 'star' }, + ], + }); + const existing = db.folders.find(f => f.name === 'Existing'); + const added = db.folders.find(f => f.name === 'FromRemote'); + assert.equal(existing.color, '#111', 'existing folder customisation is preserved'); + assert.ok(added, 'new remote folder is created'); + assert.equal(added.color, '#0f0'); +}); + +test('merge: empty/invalid remote snapshot is a no-op', async () => { + const { T, db } = await freshMerge(); + db.seedEntry({ uuid: 'u1' }); + const res = await T.applyRemoteSnapshot(null); + assert.deepEqual({ ...res }, { added: 0, updated: 0, deleted: 0, failed: 0 }); + assert.equal(db.entries.length, 1); +}); + +test('merge: entries without a uuid are ignored (no ghost rows)', async () => { + const { T, db } = await freshMerge(); + const res = await T.applyRemoteSnapshot({ + entries: [remoteEntry({ uuid: '' })], + tombstones: [], folders: [], + }); + assert.equal(res.added, 0); + assert.equal(db.entries.length, 0); +}); diff --git a/package.json b/package.json new file mode 100644 index 0000000..b7e8868 --- /dev/null +++ b/package.json @@ -0,0 +1,12 @@ +{ + "name": "pmserver-frontend-tests", + "version": "1.0.0", + "private": true, + "description": "Unit tests for the PMServer frontend (js/app.js) — crypto round-trip, CSV import, and sync-merge arbitration. Node built-in test runner, zero dependencies.", + "scripts": { + "test": "node --test \"js/tests/**/*.test.js\"" + }, + "engines": { + "node": ">=18" + } +}