Zaki/Password-Manager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(js): extract favicon module + add faviconHost tests (§3.1)

Browse Source 
Sixth slice of the app.js split. Moves the favicon fetch/cache section
(faviconHost, saveEntryIcon, ensureEntryFavicon, backfillFavicons,
clearAllFavicons) to js/app.favicon.js. Pure declarations, no top-level
side effects → loads before app.js.

- Code moved byte-for-byte; no duplicate const; syntax OK on all 7 app parts.
- NEW: js/tests/favicon.test.js — 7 tests for faviconHost, the pure
  URL→validated-hostname function that decides which domain is sent to the
  DuckDuckGo proxy (a bug there leaks the wrong host). Covers scheme/www/
  path/port stripping, non-hostname rejection, malformed dotting, unsafe
  chars, and the 253-char DNS cap.
- Fixed an inaccurate source comment surfaced by the tests: it claimed raw
  IPs "stay valid", but the TLD rule /\.[a-z]{2,}$/ rejects a numeric final
  label, so IPs get no favicon lookup (fine). Test pins the real behaviour.
- Suite: 55 → 62 tests, all green. Assets regenerated (8 ordered JS files).

app.js: 11936 → 9790 lines (6 modules extracted).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
r-zakarya
2026-07-05 16:58:16 +01:00
parent 4fd768d4cf
commit a7ad81c708
11 changed files with 206 additions and 118 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CLAUDE.md
+2
View File
@@ -34,6 +34,7 @@ visibles des suivants exactement comme dans le monofichier. Ordre actuel :
js/argon2.js (IIFE, globalThis.NobleArgon2)
js/app.crypto.js (KDF, verifier, encrypt/decrypt — extrait §3.1)
js/app.totp.js (TOTP RFC 6238 + TOTP/custom-field crypto — extrait §3.1)
js/app.favicon.js (favicon fetch/cache + faviconHost — extrait §3.1)
js/app.import.js (export container + import CSV/JSON — extrait §3.1)
js/app.backup.js (auto-backup planifié — extrait §3.1)
js/app.js (le reste : state, Bridge, api, UI…)
@@ -103,6 +104,7 @@ seule `api()` est stubbée).
| Frontend principal (en cours de découpage §3.1) | `js/app.js` |
| Crypto frontend (KDF, verifier, AES-GCM) — extrait §3.1 | `js/app.crypto.js` |
| TOTP (RFC 6238) + TOTP/custom-field crypto — extrait §3.1 | `js/app.totp.js` |
| Favicons (fetch/cache, `faviconHost`) — extrait §3.1 | `js/app.favicon.js` |
| Import/export frontend (CSV/JSON parse, export container) — extrait §3.1 | `js/app.import.js` |
| Auto-backup frontend (planifié, chiffré) — extrait §3.1 | `js/app.backup.js` |
| Sync frontend (WebDAV, snapshot, merge) — extrait §3.1 | `js/app.sync.js` |
CODE_AUDIT.md
+7 -4
View File
@@ -238,11 +238,14 @@ réécriture des call-sites, risque quasi nul vs conversion en modules ES).
RFC 6238)** ajoutés en même temps → extraction *et* nouvelle couverture.
- ✅ `js/app.backup.js` extrait (auto-backup planifié) — byte-for-byte
identique, chargé AVANT app.js (pures déclarations).
- `app.js` : 11 936 → **9 900 lignes** (crypto + totp + import + backup + sync
sortis — 5 modules).
- Suite de tests : 42 → **55 tests**.
- ✅ `js/app.favicon.js` extrait (fetch/cache favicons) — chargé AVANT app.js.
**+7 tests `faviconHost`** (extraction hostname → décide quel domaine part
vers DDG). A révélé un commentaire faux (IPs « valides » alors qu'elles
sont rejetées par la règle TLD `[a-z]{2,}`) — corrigé.
- `app.js` : 11 936 → **9 790 lignes** (6 modules sortis).
- Suite de tests : 42 → **62 tests**.
- Reste à extraire (grosses sections cohésives) : slideover, settings,
quicksearch, autofill, favicons, vault-health…
quicksearch, autofill, vault-health…
- ✅ `node --check` en pré-étape de `BuildAssets.ps1` : **déjà fait** (cf. §3.2).
### 3.2 🟡 Aucun test automatisé — **partiellement adressé (2026-07-04)**
delphi-backend/assets/BuildAssets.ps1
+1
View File
@@ -54,6 +54,7 @@ $patterns = @(
'js\argon2.js',
'js\app.crypto.js',
'js\app.totp.js',
'js\app.favicon.js',
'js\app.import.js',
'js\app.backup.js',
'js\app.js',
delphi-backend/assets/assets.inc
+2 -1
View File
@@ -1,11 +1,12 @@
// Auto-generated by BuildAssets.ps1 - do not edit by hand.
const
EMBEDDED_ASSET_COUNT = 9;
EMBEDDED_ASSET_COUNT = 10;
EMBEDDED_ASSETS: array[0..EMBEDDED_ASSET_COUNT-1] of TEmbeddedAsset = (
(UrlPath: '/index.html'; ResName: 'INDEX_HTML'),
(UrlPath: '/js/argon2.js'; ResName: 'JS_ARGON2_JS'),
(UrlPath: '/js/app.crypto.js'; ResName: 'JS_APP_CRYPTO_JS'),
(UrlPath: '/js/app.totp.js'; ResName: 'JS_APP_TOTP_JS'),
(UrlPath: '/js/app.favicon.js'; ResName: 'JS_APP_FAVICON_JS'),
(UrlPath: '/js/app.import.js'; ResName: 'JS_APP_IMPORT_JS'),
(UrlPath: '/js/app.backup.js'; ResName: 'JS_APP_BACKUP_JS'),
(UrlPath: '/js/app.js'; ResName: 'JS_APP_JS'),
delphi-backend/assets/assets.rc
+1
View File
@@ -5,6 +5,7 @@ INDEX_HTML RCDATA "Z:\\password-manager\\index.html"
JS_ARGON2_JS RCDATA "Z:\\password-manager\\js\\argon2.js"
JS_APP_CRYPTO_JS RCDATA "Z:\\password-manager\\js\\app.crypto.js"
JS_APP_TOTP_JS RCDATA "Z:\\password-manager\\js\\app.totp.js"
JS_APP_FAVICON_JS RCDATA "Z:\\password-manager\\js\\app.favicon.js"
JS_APP_IMPORT_JS RCDATA "Z:\\password-manager\\js\\app.import.js"
JS_APP_BACKUP_JS RCDATA "Z:\\password-manager\\js\\app.backup.js"
JS_APP_JS RCDATA "Z:\\password-manager\\js\\app.js"
delphi-backend/assets/assets.res
BIN
View File
Binary file not shown.
index.html
+1
View File
@@ -1195,6 +1195,7 @@
<script src="js/argon2.js"></script>
<script src="js/app.crypto.js"></script>
<script src="js/app.totp.js"></script>
<script src="js/app.favicon.js"></script>
<script src="js/app.import.js"></script>
<script src="js/app.backup.js"></script>
<script src="js/app.js"></script>
js/app.favicon.js
+125
View File
@@ -0,0 +1,125 @@
// ============================================================
// app.favicon.js — FAVICONS module (extracted from app.js, §3.1)
// ============================================================
//
// Website favicon fetch/cache (opt-in, via the Delphi DuckDuckGo proxy).
// faviconHost() is a pure URL→validated-hostname function (unit-tested);
// the rest are api/Bridge/render calls. Pure declarations, no top-level
// side effects → loads BEFORE app.js. Uses state, api, Bridge, render via
// shared global scope at call time.
//
// ============================================================
// FAVICONS (opt-in, cached server-side as base64 data URI)
// ============================================================
// Extract a usable host from entry.site (we accept anything user-typed).
// Returns '' for values that don't look like real hostnames — common case
// is users storing a brand label ("Gitea", "Work GitHub") to help the
// autofill matcher. Sending those to DDG would leak meaningless tokens
// without ever producing an icon.
function faviconHost(siteRaw) {
if (!siteRaw) return '';
let s = String(siteRaw).trim().toLowerCase();
s = s.replace(/^https?:\/\//, '').replace(/^www\./, '');
s = s.split('/')[0].split(':')[0];
// Validate: dot-separated labels, only hostname-safe chars, TLD ≥ 2
// LETTERS. Rejects "Gitea", "my work pwd", and raw IPs like "1.2.3.4"
// (numeric final label fails the [a-z]{2,} TLD rule — no favicon for a
// bare IP, which is fine). 253-char overall cap mirrors the DNS spec.
if (!s || s.length > 253) return '';
if (!/^[a-z0-9.-]+$/.test(s)) return '';
if (s.indexOf('.') < 1) return '';
if (!/\.[a-z]{2,}$/.test(s)) return '';
if (s.startsWith('.') || s.endsWith('.')) return '';
if (s.indexOf('..') >= 0) return '';
return s;
}
// Save the icon for one entry via the dedicated endpoint (no full PUT,
// no re-encryption). Fire-and-forget: failures are silent so a flaky
// network doesn't break the user's flow.
async function saveEntryIcon(entryId, dataUri) {
try {
const r = await fetch(API + '/entries/' + entryId + '/icon', {
method: 'POST',
headers: authHeaders({ 'Content-Type': 'application/json' }),
body: JSON.stringify({ icon_b64: dataUri || '' }),
});
if (!r.ok) {
// Surface the server's reason so silent persistence failures
// (size cap, auth) stop being invisible bugs.
let msg = 'HTTP ' + r.status;
try { const b = await r.json(); if (b && b.error) msg = b.error; }
catch (_) {}
toast('Icon NOT saved: ' + msg, 'error');
}
} catch (e) {
toast('Icon save failed: ' + (e && e.message || e), 'error');
}
}
// Fetch + save the favicon for one entry. Updates state.entries in-place
// so the next render() picks it up. No-op if the entry already has one.
// opts: { force: bypass "already has icon" skip, manual: bypass the global
// faviconsEnabled toggle (for explicit user actions like the Refresh button) }
async function ensureEntryFavicon(entry, opts) {
opts = opts || {};
if (!Bridge.active) return;
if (!opts.manual && !state.faviconsEnabled) return;
if (!opts.force && entry.icon_b64) return;
const host = faviconHost(entry.site);
if (!host) return;
const dataUri = await Bridge.fetchFavicon(host);
if (!dataUri) return;
entry.icon_b64 = dataUri;
await saveEntryIcon(entry.id, dataUri);
// Full render() — patching the avatar in place is fragile because
// the avatar also contains the checkbox overlay.
render();
}
// Backfill: walk state.entries, fetch missing icons one at a time so we
// don't hammer the upstream. Used by the "Refresh icons" button.
async function backfillFavicons(force) {
if (!Bridge.active) return;
const all = state.entries;
const eligible = all.filter(e => faviconHost(e.site));
const skipped = all.length - eligible.length;
const targets = eligible.filter(e => force || !e.icon_b64);
if (targets.length === 0) {
if (skipped > 0) {
toast('No icons to fetch — ' + skipped +
' entries have a non-domain site (e.g. "Gitea")', 'warning');
} else {
toast('No icons to fetch');
}
return;
}
toast('Fetching ' + targets.length + ' icon' + (targets.length === 1 ? '' : 's') + '…');
let ok = 0;
for (const e of targets) {
// Explicit user action — bypass the global toggle so the buttons
// work even when "Fetch website icons" is OFF (the toggle only
// gates auto-fetch on save).
await ensureEntryFavicon(e, { force: !!force, manual: true });
if (e.icon_b64) ok++;
}
toast('Fetched ' + ok + ' / ' + targets.length + ' icons');
render();
}
async function clearAllFavicons() {
try {
await fetch(API + '/entries/icons/all', {
method: 'DELETE',
headers: authHeaders(),
});
} catch (e) {
toast('Failed to clear icons', 'error');
return;
}
state.entries.forEach(e => { e.icon_b64 = null; });
render();
toast('Cached icons cleared');
}
js/app.js
+2 -112
View File
@@ -718,120 +718,10 @@ async function api(path, opts) {
// ============================================================
// ============================================================
// FAVICONS (opt-in, cached server-side as base64 data URI)
// FAVICONS — extracted to js/app.favicon.js (§3.1), loaded as a
// separate <script> before this file (pure declarations).
// ============================================================
// Extract a usable host from entry.site (we accept anything user-typed).
// Returns '' for values that don't look like real hostnames — common case
// is users storing a brand label ("Gitea", "Work GitHub") to help the
// autofill matcher. Sending those to DDG would leak meaningless tokens
// without ever producing an icon.
function faviconHost(siteRaw) {
if (!siteRaw) return '';
let s = String(siteRaw).trim().toLowerCase();
s = s.replace(/^https?:\/\//, '').replace(/^www\./, '');
s = s.split('/')[0].split(':')[0];
// Validate: dot-separated labels, only hostname-safe chars, TLD ≥ 2
// letters. Rejects "Gitea", "my work pwd", IP-like "1.2.3.4" stays
// valid (DDG handles IPs gracefully). 253-char overall cap mirrors
// the DNS spec.
if (!s || s.length > 253) return '';
if (!/^[a-z0-9.-]+$/.test(s)) return '';
if (s.indexOf('.') < 1) return '';
if (!/\.[a-z]{2,}$/.test(s)) return '';
if (s.startsWith('.') || s.endsWith('.')) return '';
if (s.indexOf('..') >= 0) return '';
return s;
}
// Save the icon for one entry via the dedicated endpoint (no full PUT,
// no re-encryption). Fire-and-forget: failures are silent so a flaky
// network doesn't break the user's flow.
async function saveEntryIcon(entryId, dataUri) {
try {
const r = await fetch(API + '/entries/' + entryId + '/icon', {
method: 'POST',
headers: authHeaders({ 'Content-Type': 'application/json' }),
body: JSON.stringify({ icon_b64: dataUri || '' }),
});
if (!r.ok) {
// Surface the server's reason so silent persistence failures
// (size cap, auth) stop being invisible bugs.
let msg = 'HTTP ' + r.status;
try { const b = await r.json(); if (b && b.error) msg = b.error; }
catch (_) {}
toast('Icon NOT saved: ' + msg, 'error');
}
} catch (e) {
toast('Icon save failed: ' + (e && e.message || e), 'error');
}
}
// Fetch + save the favicon for one entry. Updates state.entries in-place
// so the next render() picks it up. No-op if the entry already has one.
// opts: { force: bypass "already has icon" skip, manual: bypass the global
// faviconsEnabled toggle (for explicit user actions like the Refresh button) }
async function ensureEntryFavicon(entry, opts) {
opts = opts || {};
if (!Bridge.active) return;
if (!opts.manual && !state.faviconsEnabled) return;
if (!opts.force && entry.icon_b64) return;
const host = faviconHost(entry.site);
if (!host) return;
const dataUri = await Bridge.fetchFavicon(host);
if (!dataUri) return;
entry.icon_b64 = dataUri;
await saveEntryIcon(entry.id, dataUri);
// Full render() — patching the avatar in place is fragile because
// the avatar also contains the checkbox overlay.
render();
}
// Backfill: walk state.entries, fetch missing icons one at a time so we
// don't hammer the upstream. Used by the "Refresh icons" button.
async function backfillFavicons(force) {
if (!Bridge.active) return;
const all = state.entries;
const eligible = all.filter(e => faviconHost(e.site));
const skipped = all.length - eligible.length;
const targets = eligible.filter(e => force || !e.icon_b64);
if (targets.length === 0) {
if (skipped > 0) {
toast('No icons to fetch — ' + skipped +
' entries have a non-domain site (e.g. "Gitea")', 'warning');
} else {
toast('No icons to fetch');
}
return;
}
toast('Fetching ' + targets.length + ' icon' + (targets.length === 1 ? '' : 's') + '…');
let ok = 0;
for (const e of targets) {
// Explicit user action — bypass the global toggle so the buttons
// work even when "Fetch website icons" is OFF (the toggle only
// gates auto-fetch on save).
await ensureEntryFavicon(e, { force: !!force, manual: true });
if (e.icon_b64) ok++;
}
toast('Fetched ' + ok + ' / ' + targets.length + ' icons');
render();
}
async function clearAllFavicons() {
try {
await fetch(API + '/entries/icons/all', {
method: 'DELETE',
headers: authHeaders(),
});
} catch (e) {
toast('Failed to clear icons', 'error');
return;
}
state.entries.forEach(e => { e.icon_b64 = null; });
render();
toast('Cached icons cleared');
}
// ============================================================
// QUICK SEARCH MODAL (tray menu → fast password copy)
// ============================================================
js/tests/favicon.test.js
+62
View File
@@ -0,0 +1,62 @@
// faviconHost tests (js/app.favicon.js) — the pure URL→validated-hostname
// function that decides which domain (if any) is sent to the DuckDuckGo
// favicon proxy. A bug here means a DNS-ish leak of the wrong host, so the
// validation rules are worth pinning down.
const test = require('node:test');
const assert = require('node:assert/strict');
const { loadApp } = require('./harness.js');
const T = loadApp().__test;
const host = (s) => T.faviconHost(s);
test('faviconHost: strips scheme, www, path, port', () => {
assert.equal(host('https://www.github.com/user/repo'), 'github.com');
assert.equal(host('http://example.com'), 'example.com');
assert.equal(host('www.example.com'), 'example.com');
assert.equal(host('example.com:8443/login'), 'example.com');
assert.equal(host('HTTPS://WWW.Example.COM'), 'example.com');
});
test('faviconHost: keeps subdomains (SLD fallback is the caller\'s job)', () => {
assert.equal(host('chat.qwen.ai'), 'chat.qwen.ai');
assert.equal(host('git.example.co.uk'), 'git.example.co.uk');
});
test('faviconHost: rejects non-hostnames → "" (no lookup, no leak)', () => {
assert.equal(host('Gitea'), ''); // brand name, no dot
assert.equal(host('my work pwd'), ''); // spaces
assert.equal(host('localhost'), ''); // no TLD
assert.equal(host(''), '');
assert.equal(host(null), '');
assert.equal(host(undefined), '');
});
test('faviconHost: rejects malformed dotting', () => {
assert.equal(host('.example.com'), ''); // leading dot
assert.equal(host('example.com.'), ''); // trailing dot
assert.equal(host('a..b.com'), ''); // double dot
assert.equal(host('example.x'), ''); // 1-char TLD
});
test('faviconHost: rejects hostname-unsafe characters', () => {
assert.equal(host('exa mple.com'), '');
assert.equal(host('exam_ple.com'), ''); // underscore not hostname-safe
assert.equal(host('ex+ample.com'), '');
});
test('faviconHost: raw IPs are rejected (numeric final label fails the TLD rule)', () => {
// NOTE: the source comment claims IPs "stay valid", but the TLD check
// /\.[a-z]{2,}$/ requires a LETTERS final label, so a numeric last octet
// is rejected → '' (no lookup). Actual behaviour, pinned here; the code
// comment is inaccurate. Harmless: DDG has no favicon for a bare IP anyway.
assert.equal(host('1.2.3.44'), '');
assert.equal(host('192.168.0.1'), '');
});
test('faviconHost: enforces the 253-char DNS cap', () => {
const long = 'a'.repeat(250) + '.com'; // 254 chars
assert.equal(host(long), '');
const ok = 'a'.repeat(60) + '.com'; // well under cap, valid
assert.equal(host(ok), ok);
});
js/tests/harness.js
+3 -1
View File
@@ -30,7 +30,7 @@ const { webcrypto } = require('node:crypto');
// top-level const/let across separate runInContext calls, so we CONCATENATE
// the app.* parts (in <script> load order) into one script. argon2.js is a
// self-contained IIFE and loads separately (see below).
const APP_PARTS = ['app.crypto.js', 'app.totp.js', 'app.import.js', 'app.backup.js', 'app.js', 'app.sync.js'].map(f => path.join(__dirname, '..', f));
const APP_PARTS = ['app.crypto.js', 'app.totp.js', 'app.favicon.js', 'app.import.js', 'app.backup.js', 'app.js', 'app.sync.js'].map(f => path.join(__dirname, '..', f));
// In-memory Storage stub (Web Storage API surface used by app.js).
function makeStorage() {
@@ -144,6 +144,8 @@ function loadApp(overrides = {}) {
NobleArgon2: (typeof NobleArgon2 !== 'undefined' ? NobleArgon2 : undefined),
// totp
base32Decode, generateTOTP, parseOtpAuthUri,
// favicon
faviconHost,
// csv
parseCSV, findColumn, parseEntriesFromCSV,
// strength