From 5f6c25d2e387cd9d0c13a3f631bac8f38e60d4fa Mon Sep 17 00:00:00 2001 From: Hermes Date: Sat, 27 Jun 2026 07:03:41 -0700 Subject: [PATCH] DC-018/DC-019: mark done, bump v1.13.5, CHANGELOG --- BACKLOG.md | 7 ++++--- CHANGELOG.md | 2 ++ VERSION | 2 +- dashcaddy-api/package.json | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index bb4fe2e..3cba936 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -124,15 +124,16 @@ - **result:** Added 3 files, fixed 1 test helper, no production code changed. New: `__tests__/depth2-routes-smoke.test.js` discovers every .js in routes/{apps,arr,auth,config,recipes}/ and asserts (a) the module loads without MODULE_NOT_FOUND, (b) it exports a factory function, (c) the factory runs without throwing when given universal deps; plus 3 source-of-truth scans that fail if any depth-2 route re-introduces class A (`../../../src/...`), class B (`../src/...`), or class C (`utilities/responses` instead of `utils/responses`) require paths. New: `__tests__/public-routes-drift.test.js` walks every aggregator + direct-mount router via Express stack introspection and asserts (a) every PUBLIC_ROUTES entry matches an actually-mounted route, (b) every CSRF excludedPath is publicly accessible, (c) all 5 probe paths are CSRF-exempt, (d) all 5 probe paths are excluded from request logging, (e) all 5 probe paths bypass Tailscale auth. New: `__tests__/test-helpers/universal-deps.js` — a Proxy + seed-object shared by both suites that returns sensible stubs (logger-shaped object, asyncHandler pass-through, path-string stubs for `path.dirname()` calls) for any property access; supports Object.assign/spread via ownKeys+getOwnPropertyDescriptor traps so aggregator factories that copy ctx into subCtx don't lose proxy magic. Fix to the test helper: (a) `log` is now a logger-shaped object (`{error, warn, info, debug, audit}` as noops) not a bare noopFn — fixes `(ctx.log || console).error(...)` in routes/apps/index.js; (b) `asyncHandler` seeded as own enumerable property — survives Object.assign({}, ctx, { helpers }); (c) added `SERVICES_FILE`, `CONFIG_FILE`, `TOTP_CONFIG_FILE`, `TAILSCALE_CONFIG_FILE`, `NOTIFICATIONS_FILE`, `loadSiteConfig`, `loadNotificationConfig`, `configStateManager`, `readConfig`, `saveConfig`, `helpers`, `safeErrorMessage` as own-enumerable seeds so aggregator sub-mounts destructure cleanly. Fix to public-routes-drift: aggregator walks use prefix `/api/v1` (matches src/app.js's bare-mount on apiRouter at /api/v1), direct-mount walks use `/api/v1` + explicit prefixMap entry. Added `routes/themes.js` and `routes/license.js` to directMounts (themes bare-mounted, license on `/license`). Result: **35 suites, 1036 tests, all passing** (was 1030 passing + 6 failing before this commit). The 6 failures were depth-2 factory errors + 22 PUBLIC_ROUTES stale entries that the test infrastructure was silently swallowing. ### DC-019: backup-manager test flakes ~1/64 — tamper uses fixed-char replacement that can be a no-op -- **status:** in-progress +- **status:** done - **owner:** hermes - **details:** `__tests__/backup-manager.test.js:184` "rejects tampered data (auth tag mismatch)" tampers the encrypted blob by replacing its first base64 character with `'X'`: `Buffer.from('X' + str.substring(1))`. The first char is the first base64 char of the random 16-byte IV. When the IV's first base64 char is already `'X'` (~1/64 ≈ 1.6% probability per run), the replacement is a no-op — the "tampered" buffer is byte-identical to the original, AES-256-GCM decryption succeeds, and `expect(...).rejects.toThrow()` fails. Observed: 1 failure in ~15 full-suite runs. The production `encryptBackup`/`decryptBackup` code (AES-256-GCM, correct) is NOT at fault — the bug is in the test's tampering technique. Fix: corrupt the authTag bytes directly (XOR a byte so the value is guaranteed to change), reassemble the `iv:authTag:ciphertext` format. This guarantees a GCM integrity failure every time. -- **result:** (in progress) +- **result:** Fixed. The test now parses the `iv:authTag:ciphertext` format, XORs the first authTag byte with `0xFF` (guaranteed value change — can never be a no-op regardless of the random IV/authTag content), reassembles the blob, then asserts decryption rejects. Verified: **30/30 isolated runs + 8/8 full-suite runs (1036/1036), zero failures.** Production crypto code unchanged (it was correct all along — the bug was purely in the test's tampering technique). Confirmed root cause independently with a Node REPL script: corrupting authTag byte0 always throws `Unsupported state or unable to authenticate data`. ### DC-018: Logger.error() swallows writeErrorLog promise — error.log writes are fire-and-forget (flaky test + lost logs in prod) -- **status:** in-progress +- **status:** done - **owner:** hermes - **details:** `Logger.error()` in `src/utils/logging.js:256` calls `this._log('error', ...)` but does NOT return the result. `_log('error', ...)` returns the promise from `writeErrorLog(...)` (the async disk write to error.log). Because `error()` drops the return value, every `await logError(...)` / `await log.error(...)` caller is actually awaiting `undefined` — the file write becomes fire-and-forget. Symptoms: (1) `__tests__/logging.test.js` "captures request context when req is passed" fails intermittently in the full suite (passes in isolation) — the test reads error.log before the un-awaited appendFile completes. (2) In production, 6 route handlers (`routes/apps/deploy.js`, `routes/apps/removal.js`, `routes/health.js`, `routes/arr/config.js`, `routes/updates.js`) plus the global `boundAsyncHandler` error catcher all `await logError(...)` expecting the write to flush; error entries can be lost if the process exits/restarts immediately after. Latent since the original "unify logger" commit f71e5c5. Fix: add `return` to `Logger.error()` so the `writeErrorLog` promise propagates to callers. No behavior change for `debug/info/warn` (they never returned a promise and don't write to disk). +- **result:** Fixed — one-line change (`return this._log(...)`). The logging flake is eliminated: **10/10 full-suite runs passed** (was ~1-in-6 failure rate before the fix). Production impact: every `await logError(...)` in route handlers and the global Express error catcher now actually waits for the error.log write to flush to disk, so error entries survive fast process exit/restart. No behavior change for debug/info/warn (they never wrote to disk). ESLint clean. ## Coordination Rules diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ffc79b..254a955 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **CSP hash mismatch** between Windows and Linux builds — now computed on LF-normalized `index.html` so hashes are identical across platforms. - **SW cache tag** now derived from bundle content hash, so the service worker invalidates correctly when bundle content changes. - **Updater false-positive loop** when commit hash was unknown — fixed. +- **Logger.error() swallowed the writeErrorLog promise (DC-018).** `Logger.error()` called `this._log('error', ...)` but dropped the return value, so the async error.log disk write was fire-and-forget. Every `await logError(...)` / `await log.error(...)` caller (6 route handlers + the global Express error catcher) was awaiting `undefined`. This caused a flaky `logging.test.js` in the full suite and could lose error-log entries on fast process exit/restart. One-line fix: `return this._log(...)`. +- **Flaky backup-manager tamper test (DC-019).** The "rejects tampered data (auth tag mismatch)" test corrupted the encrypted blob by replacing its first base64 char with `'X'`; when the random IV's first base64 char was already `'X'` (~1/64 chance), the replacement was a no-op and decryption succeeded. Now corrupts the authTag byte directly (XOR `0xFF`) so the tamper is guaranteed to differ. ### Removed - **Dead `/api/v1/health`, `/api/v1/health/live`, `/api/v1/health/ready` routes** (DC-012) — these were registered in `PUBLIC_ROUTES` and CSRF exclusion lists but never actually mounted on the apiRouter. Consolidated to root-level `/health`, `/health/live`, `/health/ready` plus new `/healthz` and `/readyz` aliases. Anyone probing `/api/v1/health` will now get a clean 404 instead of an unexpected behaviour. diff --git a/VERSION b/VERSION index 80138e7..43ded90 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.13.4 +1.13.5 diff --git a/dashcaddy-api/package.json b/dashcaddy-api/package.json index 59a4a98..94e5c89 100644 --- a/dashcaddy-api/package.json +++ b/dashcaddy-api/package.json @@ -1,6 +1,6 @@ { "name": "dashcaddy-api", - "version": "1.13.4", + "version": "1.13.5", "description": "DashCaddy API server - Dashboard backend for Docker, Caddy & DNS management", "main": "server.js", "scripts": {