From 18ffd2e519318d32c44d5eb407e375eb267c63af Mon Sep 17 00:00:00 2001 From: DashCaddy Polish Date: Tue, 18 Aug 2026 17:07:13 -0700 Subject: [PATCH] fix(nesting-guard): export dataDir from src/config/paths; harden fallback to platform-paths (DC-077) [glm-grade=B] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pre-fix, every dashcaddy-api container startup logged: [nesting-guard] Skipped: The "path" argument must be of type string. Received undefined because src/utilities/nesting-guard.js does require('../config/paths') and calls paths.dataDir — but src/config/paths.js imported platformPaths and only re-exported its specific files (SERVICES_FILE, CONFIG_FILE, etc); dataDir was never re-exported, so paths.dataDir was undefined. Result: path.join(undefined, 'data') threw TypeError, the outer try/catch swallowed it, and the entire nesting-guard became a silent no-op. The cleanup that prevents recursive data/data/data/... directory duplicates never ran on any startup. Bug class is 'silent functional no-op' (same family as DC-056 AggregateError visibility). (1) src/config/paths.js (+11): re-export dataDir as SERVICES_DIR-derived (with platformPaths.dataDir fallback). dataDir is the dirname of SERVICES_FILE in container (env override wins), which equals /app/data — same value platform-paths.dataDir computes for the default config. Either path is fine; SERVICES_DIR is preferred because it respects env-override. (2) src/utilities/nesting-guard.js (+13/-2): defensive fallback to require('../../platform-paths').dataDir if paths.dataDir is missing (any future export-shape drift or older caller). Explicit skip-warn instead of silent catch when both paths fail. (3) __tests__/nesting-guard.test.js (NEW, 112 lines, 4/4 passing): isolates module cache per test, exercises (a) cleanup when nested data/data exists, (b) no-op when clean, (c) dataDir export contract, (d) dataDir === dirname(SERVICES_FILE) under env override. No jest.doMock leaks across tests (verified via 4-call probe sequence). Verified: 4/4 tests passing. Full repo suite: 100/104 suites / 2335/2335 tests passing (4 pre-existing failures in __tests__/billing/* are unrelated module-resolution issues in src/billing/invoice.js, confirmed unaffected by this change via stash+rerun). GLM-5.3 round 1: B (ship, one polish nit — trailing newline on test file, folded in same commit per multi-round-fix-first protocol). Deploy plan: container rebuild + atomic swap via /opt/dashcaddy/start.sh on DNS2; live-verify status.sami=200, dashcaddy-api=Up+healthy, and absence of [nesting-guard] Skipped log line in container logs. --- dashcaddy-api/__tests__/nesting-guard.test.js | 112 ++++++++++++++++++ dashcaddy-api/src/config/paths.js | 11 ++ dashcaddy-api/src/utilities/nesting-guard.js | 15 ++- 3 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 dashcaddy-api/__tests__/nesting-guard.test.js diff --git a/dashcaddy-api/__tests__/nesting-guard.test.js b/dashcaddy-api/__tests__/nesting-guard.test.js new file mode 100644 index 0000000..89ca6fc --- /dev/null +++ b/dashcaddy-api/__tests__/nesting-guard.test.js @@ -0,0 +1,112 @@ +/** + * Nesting-guard tests — DC-077 (data/data recursive duplicate cleanup) + * + * The guard runs at app startup. Pre-fix, `src/config/paths.js` did NOT + * re-export `dataDir`, so `paths.dataDir` resolved to `undefined`. The + * outer try/catch swallowed the resulting `TypeError [ERR_INVALID_ARG_TYPE]` + * and the entire guard became a silent no-op — every startup logged + * `[nesting-guard] Skipped: The "path" argument must be of type string. + * Received undefined`. Post-fix, paths.js exports `dataDir` and the guard + * falls back to platform-paths directly if `paths.dataDir` is missing. + * + * Tests use jest.isolateModules() for clean module-cache isolation. + * jest.doMock is intentionally avoided — it persists across tests in a + * describe and is the root cause of subtle flakes. + */ + +const fs = require('fs'); +const path = require('path'); +const os = require('os'); + +describe('nesting-guard (DC-077)', () => { + const originalEnv = { ...process.env }; + + beforeEach(() => { + jest.restoreAllMocks(); + }); + + afterEach(() => { + process.env = { ...originalEnv }; + jest.restoreAllMocks(); + }); + + function makeTmpTree() { + return fs.mkdtempSync(path.join(os.tmpdir(), 'nest-guard-')); + } + + function writeJson(p, obj) { + fs.mkdirSync(path.dirname(p), { recursive: true }); + fs.writeFileSync(p, JSON.stringify(obj)); + } + + it('removes a recursive data/data duplicate when present', () => { + const tmp = makeTmpTree(); + writeJson(path.join(tmp, 'config.json'), { x: 1 }); + writeJson(path.join(tmp, 'data', 'config.json'), { x: 1 }); + writeJson(path.join(tmp, 'data', 'services.json'), []); + + process.env.SERVICES_FILE = path.join(tmp, 'services.json'); + process.env.CONFIG_FILE = path.join(tmp, 'config.json'); + + let cleanupLog = ''; + let warnLog = ''; + jest.isolateModules(() => { + const guard = require('../src/utilities/nesting-guard'); + jest.spyOn(console, 'log').mockImplementation((m) => { cleanupLog += String(m) + '\n'; }); + jest.spyOn(console, 'warn').mockImplementation((m) => { warnLog += String(m) + '\n'; }); + guard(); + }); + + expect(fs.existsSync(path.join(tmp, 'data'))).toBe(false); + expect(fs.existsSync(path.join(tmp, 'config.json'))).toBe(true); + expect(cleanupLog).toMatch(/Removing recursive data nesting|Recursive nesting removed/); + expect(warnLog).not.toMatch(/Skipped/); + }); + + it('does nothing when no nested data/data directory exists', () => { + const tmp = makeTmpTree(); + writeJson(path.join(tmp, 'config.json'), { x: 1 }); + + process.env.SERVICES_FILE = path.join(tmp, 'services.json'); + process.env.CONFIG_FILE = path.join(tmp, 'config.json'); + + let cleanupLog = ''; + let warnLog = ''; + jest.isolateModules(() => { + const guard = require('../src/utilities/nesting-guard'); + jest.spyOn(console, 'log').mockImplementation((m) => { cleanupLog += String(m) + '\n'; }); + jest.spyOn(console, 'warn').mockImplementation((m) => { warnLog += String(m) + '\n'; }); + guard(); + }); + + expect(fs.existsSync(path.join(tmp, 'config.json'))).toBe(true); + expect(warnLog).not.toMatch(/Skipped/); + expect(cleanupLog).not.toMatch(/Removing recursive data nesting/); + }); + + it('src/config/paths exports dataDir as a non-empty string', () => { + let dataDir; + jest.isolateModules(() => { + const paths = require('../src/config/paths'); + dataDir = paths.dataDir; + }); + expect(typeof dataDir).toBe('string'); + expect(dataDir.length).toBeGreaterThan(0); + }); + + it('src/config/paths.dataDir equals dirname(SERVICES_FILE) when SERVICES_FILE env is set', () => { + const tmp = makeTmpTree(); + process.env.SERVICES_FILE = path.join(tmp, 'services.json'); + process.env.CONFIG_FILE = path.join(tmp, 'config.json'); + + let servicesFile, dataDir; + jest.isolateModules(() => { + const paths = require('../src/config/paths'); + servicesFile = paths.SERVICES_FILE; + dataDir = paths.dataDir; + }); + + expect(dataDir).toBe(path.dirname(servicesFile)); + expect(dataDir).toBe(tmp); + }); +}); diff --git a/dashcaddy-api/src/config/paths.js b/dashcaddy-api/src/config/paths.js index 39284c8..e2af392 100644 --- a/dashcaddy-api/src/config/paths.js +++ b/dashcaddy-api/src/config/paths.js @@ -31,6 +31,17 @@ module.exports = { CADDY_ADMIN_URL, SERVICES_FILE, SERVICES_DIR, + // Re-export the resolved data directory so other modules (notably + // src/utilities/nesting-guard.js) can locate `/app/data` without having to + // also require('../../platform-paths') — keeps a single source of truth for + // the data dir on the src/config/paths surface. Without this, `dataDir` + // resolves to `undefined`, and `path.join(undefined, 'data')` throws + // `TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type + // string. Received undefined` at startup (DC-077 fingerprint). Fall back to + // platformPaths.dataDir if SERVICES_DIR is somehow not a string (defensive — + // SERVICES_DIR is computed from a path.dirname() of a string so it always + // is, but the cost of guarding is one branch). + dataDir: typeof SERVICES_DIR === 'string' && SERVICES_DIR ? SERVICES_DIR : platformPaths.dataDir, CONFIG_FILE, DNS_CREDENTIALS_FILE, TAILSCALE_CONFIG_FILE, diff --git a/dashcaddy-api/src/utilities/nesting-guard.js b/dashcaddy-api/src/utilities/nesting-guard.js index a18f1bd..b1b314c 100644 --- a/dashcaddy-api/src/utilities/nesting-guard.js +++ b/dashcaddy-api/src/utilities/nesting-guard.js @@ -14,8 +14,19 @@ const path = require('path'); module.exports = function nestingGuard() { try { const paths = require('../config/paths'); - const dataDir = paths.dataDir; - const dataDataPath = path.join(dataDir, 'data'); + const dataDir = paths && paths.dataDir; + // Defensive: if paths.dataDir is undefined (older callers or a future + // export-shape drift), fall back to platformPaths.dataDir directly so the + // guard can still execute. Pre-fix this branch was swallowed silently by + // the outer try/catch, leaving the entire nesting-guard a no-op (DC-077). + const effectiveDataDir = typeof dataDir === 'string' && dataDir + ? dataDir + : require('../../platform-paths').dataDir; + if (typeof effectiveDataDir !== 'string' || !effectiveDataDir) { + console.warn('[nesting-guard] Skipped: dataDir unavailable from src/config/paths and platform-paths'); + return; + } + const dataDataPath = path.join(effectiveDataDir, 'data'); // If data/data exists, it's a recursive duplicate — remove it if (fs.existsSync(dataDataPath)) {