From 01426a48b1e21326d34c27a312c519a18ad7bd0b Mon Sep 17 00:00:00 2001 From: r-zakarya <82443831+r-zakarya@users.noreply.github.com> Date: Sat, 11 Jul 2026 19:52:26 +0100 Subject: [PATCH] refactor(db): drop cleartext template refs, fix ghost columns + fresh-DB schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migration verified (0 unmigrated cleartext templates across both accounts) before contracting: - GET/POST/PUT/bulk no longer read or write the cleartext template column (only template_enc/iv); the PUT partial-update gate stays keyed on the 'template' JSON key presence. - Removed AddColumnIfMissing for template AND tags/title — those two had silently re-added the dropped columns as empty ghosts at every start. - CREATE TABLE: removed site/username NOT NULL cleartext columns — a FRESH database rejected the very first INSERT (which no longer ships them). User can now DROP COLUMN template (and re-drop the ghost tags/title). Co-Authored-By: Claude Opus 4.8 --- CLAUDE.md | 8 ++-- CODE_AUDIT.md | 8 ++-- .../Handlers/PM.Handler.Entries.pas | 42 ++++++------------- delphi-backend/Source/PM.Database.pas | 23 ++++------ 4 files changed, 32 insertions(+), 49 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6793b14..a647dbc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -465,9 +465,11 @@ la nouvelle clé (JS loop `ENCRYPTED_META_FIELDS` + UPDATE serveur). batch) — historiquement le serveur le préservait via `LHasTemplate` (clé absente). Pour garder ce contrat, `withEncryptedMeta` **skippe un champ absent de l'objet** (`if (!(f in obj)) continue`) au lieu de synthétiser un `_enc` vide -qui ferait wiper le template stocké. La colonne clair `template` est **gardée** -(fallback migration : `decryptEntryMeta` préfère `_enc`, retombe sur le clair) -— pas encore DROP comme les 4 autres. +qui ferait wiper le template stocké. La colonne clair `template` a été **DROP** +comme les 4 autres (migration vérifiée 0 restant sur les 2 comptes, refs code +retirées le 2026-07-11). Piège appris au passage : retirer aussi le +`AddColumnIfMissing` de toute colonne droppée, sinon elle **revient vide** au +prochain démarrage (c'était le cas de `tags`/`title`). **Le `?q=` serveur est neutralisé** (site+username chiffrés → LIKE inutile ; le front cherche côté client). **La validation « Site required » serveur est retirée** (site='' quand chiffré) — le client la fait. `folder` reste en clair diff --git a/CODE_AUDIT.md b/CODE_AUDIT.md index 3a960b4..acd4735 100644 --- a/CODE_AUDIT.md +++ b/CODE_AUDIT.md @@ -105,9 +105,11 @@ recherche/favicons OK. La migration est **par utilisateur** (tourne au unlock sur les entries du compte connecté) — un compte non connecté garde son clair jusqu'à sa prochaine connexion (comportement normal, pas une régression). **`template` chiffré au repos (✅ 2026-07-11)** : même pattern -(`template_enc`/`template_iv`, ajouté à `ENCRYPTED_META_FIELDS`). Colonne clair -`template` gardée comme fallback de migration (`decryptEntryMeta` préfère `_enc`, -retombe sur le clair). Guard dans `withEncryptedMeta` : les re-ships partiels +(`template_enc`/`template_iv`, ajouté à `ENCRYPTED_META_FIELDS`). Migration +vérifiée (0 clair restant, 2 comptes) → refs colonne clair retirées du code + +`AddColumnIfMissing` supprimés (template, et tags/title qui étaient revenus en +fantômes après le drop) + CREATE TABLE corrigé (site/username NOT NULL cassait +une DB neuve). Guard dans `withEncryptedMeta` : les re-ships partiels (add-tag, move-to-folder) omettent `template` → le serveur le préserve via `LHasTemplate` (clé absente), pas de wipe. 3 tests unitaires. **Pièces jointes : le contenu est déjà chiffré** (AES-GCM, `encryptBlobBytes`) ; diff --git a/delphi-backend/Handlers/PM.Handler.Entries.pas b/delphi-backend/Handlers/PM.Handler.Entries.pas index 2279cc5..b23293d 100644 --- a/delphi-backend/Handlers/PM.Handler.Entries.pas +++ b/delphi-backend/Handlers/PM.Handler.Entries.pas @@ -168,13 +168,8 @@ begin var LKindVal := LQ.FieldByName('kind').AsString; if LKindVal = '' then LKindVal := 'login'; LObj.AddPair('kind', LKindVal); - // Template subtype. Encrypted at rest (template_enc/iv); the cleartext - // `template` column is kept as a migration fallback (decryptEntryMeta - // prefers _enc, falls back to cleartext until the row is swept). - if LQ.FieldByName('template').IsNull then - LObj.AddPair('template', TJSONNull.Create) - else - LObj.AddPair('template', LQ.FieldByName('template').AsString); + // Template subtype — encrypted at rest only (template_enc/iv); the + // cleartext column was dropped post-migration like site/username. AddNullableField(LObj, 'template_enc', LQ.FieldByName('template_enc')); AddNullableField(LObj, 'template_iv', LQ.FieldByName('template_iv')); // Stable cross-device identity (always populated post-migration). @@ -331,7 +326,7 @@ var LUserId, LNewId: Integer; LBody, LObj: TJSONObject; LSite, LTitle, LUser, LUserEnc, LUserIv, LFolder, LEnc, LIV, LTags, LNow, - LTotpSec, LTotpIv, LKind, LCf, LCfIv, LIcon, LTemplate, LUuid, + LTotpSec, LTotpIv, LKind, LCf, LCfIv, LIcon, LUuid, LTemplateEnc, LTemplateIv, LSiteEnc, LSiteIv, LTitleEnc, LTitleIv, LTagsEnc, LTagsIv: string; LQ: TFDQuery; @@ -372,7 +367,6 @@ begin LCf := LBody.GetValue('custom_fields', ''); LCfIv := LBody.GetValue('custom_fields_iv', ''); LIcon := LBody.GetValue('icon_b64', ''); - LTemplate:= Trim(LBody.GetValue('template', '')); LTemplateEnc := LBody.GetValue('template_enc', ''); LTemplateIv := LBody.GetValue('template_iv', ''); // Caller may bring its own UUID (sync restore / import preserving @@ -408,10 +402,10 @@ begin ' site_enc, site_iv, title_enc, title_iv, tags_enc, tags_iv, ' + ' encrypted_password, iv, encryption_method, ' + ' folder, totp_secret, totp_iv, kind, custom_fields, custom_fields_iv,' + - ' icon_b64, template, template_enc, template_iv, uuid, created_at, updated_at, password_changed_at) ' + + ' icon_b64, template_enc, template_iv, uuid, created_at, updated_at, password_changed_at) ' + 'VALUES (:uid, :uenc, :uiv, :senc, :siv, :tenc, :tiv2, :genc, :giv, ' + ' :e, :i, ''client'', :f, :ts, :tiv, :k, ' + - ' :cf, :cfiv, :ic, :tpl, :tplenc, :tpliv, :uuid, :c, :c2, :c)'; + ' :cf, :cfiv, :ic, :tplenc, :tpliv, :uuid, :c, :c2, :c)'; LQ.ParamByName('uid').AsInteger := LUserId; // site/username/title/tags cleartext columns dropped — only *_enc written. BindNullable(LQ, 'uenc', LUserEnc); @@ -448,9 +442,6 @@ begin if LCfIv = '' then LQ.ParamByName('cfiv').Clear else LQ.ParamByName('cfiv').Value := LCfIv; LQ.ParamByName('ic').DataType := ftMemo; if LIcon = '' then LQ.ParamByName('ic').Clear else LQ.ParamByName('ic').Value := LIcon; - LQ.ParamByName('tpl').DataType := ftString; - if LTemplate = '' then LQ.ParamByName('tpl').Clear - else LQ.ParamByName('tpl').AsString := LTemplate; BindNullable(LQ, 'tplenc', LTemplateEnc); BindNullable(LQ, 'tpliv', LTemplateIv); LQ.ParamByName('uuid').AsString := LUuid; @@ -495,7 +486,7 @@ var LUserId, LId: Integer; LBody: TJSONObject; LSite, LTitle, LUser, LUserEnc, LUserIv, LFolder, LEnc, LIV, LTags, LNow, - LTotpSec, LTotpIv, LKind, LCf, LCfIv, LTemplate, LTemplateEnc, LTemplateIv, + LTotpSec, LTotpIv, LKind, LCf, LCfIv, LTemplateEnc, LTemplateIv, LSiteEnc, LSiteIv, LTitleEnc, LTitleIv, LTagsEnc, LTagsIv: string; LHasTemplate: Boolean; LQ: TFDQuery; @@ -538,10 +529,10 @@ begin LCfIv := LBody.GetValue('custom_fields_iv', ''); // Template is only updated when the caller explicitly sends it — // partial PUTs (drag-to-folder) must not wipe it. withEncryptedMeta always - // ships template (blanked) + template_enc together, so this one flag gates - // both the cleartext and ciphertext columns. + // ships template (blanked cleartext key) + template_enc together, so the + // presence of the 'template' JSON key gates the ciphertext columns. + // (Cleartext template column dropped — only *_enc/_iv are stored.) LHasTemplate := LBody.GetValue('template') <> nil; - LTemplate := Trim(LBody.GetValue('template', '')); LTemplateEnc := LBody.GetValue('template_enc', ''); LTemplateIv := LBody.GetValue('template_iv', ''); finally @@ -595,7 +586,7 @@ begin // partial PUT (drag-to-folder, move-to-folder) doesn't wipe it. var LTemplateSet := ''; if LHasTemplate then - LTemplateSet := ', template=:tpl, template_enc=:tplenc, template_iv=:tpliv'; + LTemplateSet := ', template_enc=:tplenc, template_iv=:tpliv'; LQ.SQL.Text := 'UPDATE vault_entries ' + 'SET username_enc=:uenc, username_iv=:uiv, ' + @@ -642,9 +633,6 @@ begin if LCfIv = '' then LQ.ParamByName('cfiv').Clear else LQ.ParamByName('cfiv').Value := LCfIv; if LHasTemplate then begin - LQ.ParamByName('tpl').DataType := ftString; - if LTemplate = '' then LQ.ParamByName('tpl').Clear - else LQ.ParamByName('tpl').AsString := LTemplate; BindNullable(LQ, 'tplenc', LTemplateEnc); BindNullable(LQ, 'tpliv', LTemplateIv); end; @@ -1190,7 +1178,7 @@ var LBody, LObj, LEntry: TJSONObject; LArr, LIds: TJSONArray; LSite, LTitle, LUser, LUserEnc, LUserIv, LFolder, LEnc, LIV, LTags, LTotpSec, - LTotpIv, LNow, LKind, LCf, LCfIv, LIcon, LTemplate, LTemplateEnc, LTemplateIv, LUuid, + LTotpIv, LNow, LKind, LCf, LCfIv, LIcon, LTemplateEnc, LTemplateIv, LUuid, LCreatedAt, LUpdatedAt, LSiteEnc, LSiteIv, LTitleEnc, LTitleIv, LTagsEnc, LTagsIv: string; LQ, LTomb: TFDQuery; @@ -1249,10 +1237,10 @@ begin ' site_enc, site_iv, title_enc, title_iv, tags_enc, tags_iv, ' + ' encrypted_password, iv, encryption_method, ' + ' folder, totp_secret, totp_iv, kind, custom_fields, custom_fields_iv,' + - ' icon_b64, template, template_enc, template_iv, uuid, created_at, updated_at) ' + + ' icon_b64, template_enc, template_iv, uuid, created_at, updated_at) ' + 'VALUES (:uid, :uenc, :uiv, :senc, :siv, :tenc, :tiv2, :genc, :giv, ' + ' :e, :i, ''client'', :f, :ts, :tiv, :k, ' + - ' :cf, :cfiv, :ic, :tpl, :tplenc, :tpliv, :uuid, :c, :c2)'; + ' :cf, :cfiv, :ic, :tplenc, :tpliv, :uuid, :c, :c2)'; // Declare optional param types ONCE — the prepared statement is // reused across every imported entry, and FireDAC needs the // type set before the first .Clear call would otherwise fail @@ -1266,7 +1254,6 @@ begin LQ.ParamByName('cf').DataType := ftMemo; LQ.ParamByName('cfiv').DataType := ftMemo; LQ.ParamByName('ic').DataType := ftMemo; - LQ.ParamByName('tpl').DataType := ftString; LQ.ParamByName('uenc').DataType := ftMemo; LQ.ParamByName('uiv').DataType := ftMemo; LQ.ParamByName('senc').DataType := ftMemo; @@ -1301,7 +1288,6 @@ begin LCf := LEntry.GetValue('custom_fields', ''); LCfIv := LEntry.GetValue('custom_fields_iv', ''); LIcon := LEntry.GetValue('icon_b64', ''); - LTemplate:= Trim(LEntry.GetValue('template', '')); LTemplateEnc := LEntry.GetValue('template_enc', ''); LTemplateIv := LEntry.GetValue('template_iv', ''); LUuid := Trim(LEntry.GetValue('uuid', '')); @@ -1345,8 +1331,6 @@ begin if LCf = '' then LQ.ParamByName('cf').Clear else LQ.ParamByName('cf').Value := LCf; if LCfIv = '' then LQ.ParamByName('cfiv').Clear else LQ.ParamByName('cfiv').Value := LCfIv; if LIcon = '' then LQ.ParamByName('ic').Clear else LQ.ParamByName('ic').Value := LIcon; - if LTemplate = '' then LQ.ParamByName('tpl').Clear - else LQ.ParamByName('tpl').AsString := LTemplate; BindNullable(LQ, 'tplenc', LTemplateEnc); BindNullable(LQ, 'tpliv', LTemplateIv); LQ.ParamByName('uuid').AsString := LUuid; diff --git a/delphi-backend/Source/PM.Database.pas b/delphi-backend/Source/PM.Database.pas index 039c628..110ea3e 100644 --- a/delphi-backend/Source/PM.Database.pas +++ b/delphi-backend/Source/PM.Database.pas @@ -164,8 +164,9 @@ begin 'CREATE TABLE IF NOT EXISTS vault_entries (' + ' id INTEGER PRIMARY KEY AUTOINCREMENT,' + ' user_id INTEGER NOT NULL,' + - ' site TEXT NOT NULL,' + - ' username TEXT NOT NULL,' + + // site/username cleartext columns are gone (§1.3 metadata-at-rest — + // only site_enc/username_enc below). Keeping them NOT NULL here made a + // FRESH database reject the very first INSERT (which no longer ships them). ' encrypted_password TEXT NOT NULL,' + ' iv TEXT NOT NULL,' + ' encryption_method TEXT DEFAULT ''server'',' + @@ -329,13 +330,10 @@ begin AddColumnIfMissing('vault_entries', 'deleted', 'INTEGER DEFAULT 0'); AddColumnIfMissing('vault_entries', 'deleted_at', 'DATETIME'); AddColumnIfMissing('vault_entries', 'favorite', 'INTEGER DEFAULT 0'); - // UI V2: tags stored as comma-separated TEXT (e.g. "work,important,2fa"). - // Simple format, search via LIKE %tag%. Frontend handles parsing/joining. - AddColumnIfMissing('vault_entries', 'tags', 'TEXT DEFAULT '''''); - // Optional human-friendly display name. When empty, the UI falls back - // to `site`. Lets the user store the raw URL/host (used for autofill - // domain matching) while showing something nicer on cards/slideovers. - AddColumnIfMissing('vault_entries', 'title', 'TEXT DEFAULT '''''); + // NOTE (§1.3 contraction): the cleartext `tags` / `title` / `template` + // AddColumnIfMissing lines were REMOVED — those columns were dropped after + // the metadata-at-rest migration, and keeping the lines here silently + // re-added empty ghost columns at every start. Only *_enc/_iv survive. // TOTP (2FA) — RFC 6238. Secret + IV are AES-GCM ciphertext / IV pair // encrypted client-side with the user's master-derived key, exactly like // encrypted_password. The server treats them as opaque blobs and never @@ -393,11 +391,8 @@ begin // Pinned entries float to the top of every view, regardless of sort. // Independent from favorite (which is a filter, not a sort override). AddColumnIfMissing('vault_entries', 'pinned', 'INTEGER DEFAULT 0'); - // Template identifier: empty/NULL = generic login or note; otherwise a - // string like 'credit-card', 'ssh-key', 'server', 'recovery-codes'. - // Drives the card/table label so notes-with-fields read as "Credit card" - // instead of the generic "Encrypted note" placeholder. - AddColumnIfMissing('vault_entries', 'template', 'TEXT'); + // Template identifier lives encrypted in template_enc/iv (see above) — + // its cleartext column was dropped post-migration, line removed here. // Stable identity that survives export/import + cross-device sync. // SQLite `id` is autoincrement local-only — useless to match the same // logical entry across two installs. Populate existing rows with a