Compare commits

...
2 Commits
Author SHA1 Message Date
Hermes c1ac0baa5e DC-019: claim for Hermes — backup-manager flaky tamper test
CI / Security audit (push) Has been cancelled
CI / Test & Lint (push) Has been cancelled
2026-06-27 06:52:10 -07:00
Hermes 1c8f55edc1 DC-018: return writeErrorLog promise from Logger.error()
Logger.error() called this._log('error',...) but dropped the return value.
_log returns the writeErrorLog(...) promise for error level, so every
await logError(...)/await log.error(...) caller was awaiting undefined —
the error.log disk write was fire-and-forget. This caused:

1. __tests__/logging.test.js 'captures request context' to flake in the
   full suite (test read error.log before the un-awaited appendFile
   completed; passed in isolation).
2. In production, 6 route handlers + the global boundAsyncHandler error
   catcher all await logError(...) expecting the write to flush — error
   entries could be lost on fast process exit/restart.

Fix: add 'return' so the promise propagates. Verified: logging test
passes 10/10 full-suite runs (was ~1/6 failure rate). No behavior change
for debug/info/warn (they never wrote to disk).
2026-06-27 06:51:59 -07:00
2 changed files with 10 additions and 1 deletions

No files matched your search

+6
View File
@@ -123,6 +123,12 @@
- **details:** After DC-005 path-fix (commit c39c80b) shipped 67 broken-require repairs across 21 depth-2 route files, two test gaps remained: (1) no test imported any depth-2 route module, so future refactors could reintroduce class A/B/C broken paths undetected; (2) no test verified that PUBLIC_ROUTES entries (in src/utilities/middleware.js) all correspond to actually-mounted routes — exactly the kind of drift DC-012 added a regression check for (probe paths), but only for the 5 probes. The full ~27-entry PUBLIC_ROUTES list could silently go stale.
- **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
- **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)
### DC-018: Logger.error() swallows writeErrorLog promise — error.log writes are fire-and-forget (flaky test + lost logs in prod)
- **status:** in-progress
- **owner:** hermes
+4 -1
View File
@@ -256,7 +256,10 @@ class Logger extends EventEmitter {
error(ctx, err, req, extra) {
const errObj = err instanceof Error ? err : new Error(String(err));
const payload = extra && Object.keys(extra).length ? extra : undefined;
this._log('error', ctx, errObj.message, errObj, { req, payload });
// Return the promise from _log so callers that `await log.error(...)` /
// `await logError(...)` actually wait for the error.log write to flush.
// _log returns the writeErrorLog(...) promise for level === 'error'.
return this._log('error', ctx, errObj.message, errObj, { req, payload });
}
_log(level, ctx, msg, data, { req, payload } = {}) {