DC-017: Regression tests for depth-2 route paths + PUBLIC_ROUTES drift
After DC-005 path-fix (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 all ~27 PUBLIC_ROUTES entries (in
src/utilities/middleware.js) corresponded to actually-mounted routes.
DC-012 added a similar check for the 5 probe paths, but only those.
Added 3 files, fixed 1 test helper, no production code changed:
- __tests__/depth2-routes-smoke.test.js (new): discovers every .js in
routes/{apps,arr,auth,config,recipes}/ and asserts (a) module loads
without MODULE_NOT_FOUND, (b) exports a factory function, (c) 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.
- __tests__/public-routes-drift.test.js (new): 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-e) all 5 probe paths are CSRF-exempt + logging-skipped +
Tailscale-bypassed.
- __tests__/test-helpers/universal-deps.js (new): Proxy + seed-object
shared by both suites. Returns sensible stubs for any property access
(logger-shaped object, asyncHandler pass-through, path-string stubs).
Supports Object.assign/spread via ownKeys+getOwnPropertyDescriptor
traps so aggregator factories that copy ctx into subCtx don't lose
proxy magic.
Test-helper fixes needed to make the suites pass:
- '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 factory catch block.
- 'asyncHandler' seeded as own enumerable property — survives
Object.assign({}, ctx, { helpers }) used by routes/arr/index.js etc.
- 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.
Public-routes-drift test fixes:
- Aggregator walks use prefix '/api/v1' (matches src/app.js's bare-mount
on apiRouter at /api/v1). Without this, the 6 TOTP routes registered by
routes/auth/index.js appeared as '/totp/config' instead of
'/api/v1/totp/config' and were falsely flagged as stale.
- Direct-mount walks use '/api/v1' + explicit prefixMap entry (same reason).
- Added routes/themes.js and routes/license.js to directMounts.
Result: 35 suites, 1036 tests, all passing (was 1030 passing + 6 failing
before this commit). The 6 pre-existing failures were depth-2 factory
errors + 22 PUBLIC_ROUTES stale entries that the test infrastructure
was silently swallowing — these tests surface them so they can't recur.
BACKLOG.md updated with full DC-017 entry (status: done, owner: krystie).
This commit is contained in:
Hermes
@@ -0,0 +1,315 @@
|
||||
/**
|
||||
* Public-routes allowlist drift tests
|
||||
*
|
||||
* Three allowlists in the DashCaddy codebase grant "no auth" or "no CSRF"
|
||||
* access to specific paths. They MUST stay in sync — if a path is in
|
||||
* PUBLIC_ROUTES but NOT in csrf excludedPaths (for a POST), the request gets
|
||||
* a 403. If a path is in csrf excludedPaths but NOT in PUBLIC_ROUTES, it gets
|
||||
* a 401. Both bugs are silent and ship-blocking for fresh users.
|
||||
*
|
||||
* Three lists:
|
||||
* 1. PUBLIC_ROUTES — in src/utilities/middleware.js, used by auth middleware
|
||||
* 2. excludedPaths — in src/security/csrf-protection.js, used by CSRF middleware
|
||||
* 3. Request-logging skip list — in src/utilities/middleware.js, used by request logger
|
||||
* 4. Tailscale auth bypass — in src/utilities/middleware.js, used by Tailscale gate
|
||||
*
|
||||
* Tests assert:
|
||||
* A. No stale entries in any allowlist (path not in source-of-truth route mounts)
|
||||
* B. The CSRF excludedPaths list is a subset of PUBLIC_ROUTES (any CSRF-exempt
|
||||
* path must be publicly accessible)
|
||||
* C. Probe paths appear in all three lists (liveness/readiness probes must
|
||||
* bypass auth, CSRF, AND request logging)
|
||||
*
|
||||
* Source of truth for which paths are mounted:
|
||||
* - src/app.js (inline apiRouter.get/post routes)
|
||||
* - routes/[subdir]/[file].js (router.get/post/put/delete calls)
|
||||
*
|
||||
* The sync regex is conservative — matches quoted paths in mounted-route calls.
|
||||
* False positives (e.g. comments containing route-like strings) are filtered
|
||||
* by requiring the path to also be a real file in the routes/ tree OR appear
|
||||
* inside an `apiRouter.` / `app.` call expression.
|
||||
*/
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { universalDeps } = require('./test-helpers/universal-deps');
|
||||
|
||||
const PKG_ROOT = path.join(__dirname, '..');
|
||||
const SRC_APP = path.join(PKG_ROOT, 'src', 'app.js');
|
||||
const SRC_MIDDLEWARE = path.join(PKG_ROOT, 'src', 'utilities', 'middleware.js');
|
||||
const SRC_CSRF = path.join(PKG_ROOT, 'src', 'security', 'csrf-protection.js');
|
||||
|
||||
// Extract PUBLIC_ROUTES path strings from middleware.js
|
||||
function readPublicRoutes() {
|
||||
const content = fs.readFileSync(SRC_MIDDLEWARE, 'utf8');
|
||||
// Match `path: '/...'`
|
||||
const matches = [...content.matchAll(/path:\s*['"]([^'"]+)['"]/g)].map(m => m[1]);
|
||||
return new Set(matches);
|
||||
}
|
||||
|
||||
// Extract excludedPaths from csrf-protection.js
|
||||
function readCsrfExcluded() {
|
||||
const content = fs.readFileSync(SRC_CSRF, 'utf8');
|
||||
// Match string literals in arrays inside excludedPaths
|
||||
const blockMatch = content.match(/excludedPaths\s*=\s*\[([^\]]+)\]/);
|
||||
if (!blockMatch) return new Set();
|
||||
const entries = [...blockMatch[1].matchAll(/['"]([^'"]+)['"]/g)].map(m => m[1]);
|
||||
return new Set(entries);
|
||||
}
|
||||
|
||||
// Extract all mounted-route paths from the live Express routers.
|
||||
//
|
||||
// Strategy:
|
||||
// 1. Build a real Express app with stub middleware that just calls next()
|
||||
// 2. Mount each aggregator router (auth/index.js, apps/index.js, arr/index.js)
|
||||
// using universal deps
|
||||
// 3. Use express.Router.stack to enumerate every registered route + path
|
||||
// 4. Also inline-mount non-aggregator route files (e.g. routes/services.js)
|
||||
// 5. For src/app.js inline routes (apiRouter.get('/health', ...)), parse directly
|
||||
//
|
||||
// This is more robust than regex — it captures routes registered via
|
||||
// router.use(subRouter) chains inside aggregator files (e.g. auth/index.js
|
||||
// calling router.use(initTotp(deps))). Regex can't see through that.
|
||||
function readMountedRoutes() {
|
||||
const mounted = new Set();
|
||||
|
||||
// ----- 1. Aggregator files -----
|
||||
const aggregators = ['routes/auth/index.js', 'routes/arr/index.js', 'routes/apps/index.js'];
|
||||
for (const relPath of aggregators) {
|
||||
const fullPath = path.join(PKG_ROOT, relPath);
|
||||
if (!fs.existsSync(fullPath)) continue;
|
||||
let factory;
|
||||
try {
|
||||
factory = require(fullPath);
|
||||
} catch (e) {
|
||||
// Some aggregators may not load with stub deps — skip them.
|
||||
// The depth-2 smoke test catches module-load failures separately.
|
||||
continue;
|
||||
}
|
||||
if (typeof factory !== 'function') continue;
|
||||
let router;
|
||||
try {
|
||||
router = factory(universalDeps);
|
||||
} catch (e) {
|
||||
continue;
|
||||
}
|
||||
// Aggregators (auth/index, arr/index, apps/index) are mounted bare on
|
||||
// apiRouter (which lives at /api/v1), so their inner routes inherit the
|
||||
// /api/v1 prefix in production. Walk with that prefix so PUBLIC_ROUTES
|
||||
// entries like '/api/v1/totp/config' match what the router actually
|
||||
// serves in production.
|
||||
walkRouter(router, '/api/v1', mounted);
|
||||
}
|
||||
|
||||
// ----- 2. Non-aggregator route files (mounted directly via apiRouter.use(...)) -----
|
||||
const directMounts = [
|
||||
'routes/dns.js', // apiRouter.use('/dns', dnsRoutes({...}))
|
||||
'routes/notifications.js', // apiRouter.use('/notifications', notificationRoutes({...}))
|
||||
'routes/containers.js', // apiRouter.use('/containers', containerRoutes({...}))
|
||||
'routes/services.js', // apiRouter.use(serviceRoutes({...})) // bare mount
|
||||
'routes/health.js', // apiRouter.use(healthRoutes({...})) // bare mount
|
||||
'routes/monitoring.js', // apiRouter.use(monitoringRoutes({...})) // bare mount
|
||||
'routes/updates.js', // apiRouter.use(updatesRoutes({...})) // bare mount
|
||||
'routes/tailscale.js', // apiRouter.use('/tailscale', tailscaleRoutes({...}))
|
||||
'routes/sites.js', // apiRouter.use(sitesRoutes({...}))
|
||||
'routes/credentials.js', // apiRouter.use(credentialsRoutes({...}))
|
||||
'routes/backups.js', // apiRouter.use(backupsRoutes({...}))
|
||||
'routes/ca.js', // apiRouter.use('/ca', caRoutes(ctx))
|
||||
'routes/browse.js', // apiRouter.use(browseRoutes({...}))
|
||||
'routes/errorlogs.js', // apiRouter.use(errorLogsRoutes({...}))
|
||||
'routes/logs.js', // apiRouter.use(logsRoutes({...}))
|
||||
'routes/openclaw.js', // apiRouter.use('/openclaw', openClawRoutes(ctx))
|
||||
'routes/recipes/index.js', // apiRouter.use(recipesRoutes(ctx)) // bare mount
|
||||
'routes/config/index.js', // apiRouter.use(configRoutes(ctx)) // bare mount
|
||||
'routes/themes.js', // apiRouter.use(themesRoutes({...})) // bare mount
|
||||
'routes/license.js', // apiRouter.use('/license', licenseRoutes({...}))
|
||||
];
|
||||
// Prefix map: explicit prefix from src/app.js's apiRouter.use() call
|
||||
const prefixMap = {
|
||||
'routes/dns.js': '/dns',
|
||||
'routes/notifications.js': '/notifications',
|
||||
'routes/containers.js': '/containers',
|
||||
'routes/tailscale.js': '/tailscale',
|
||||
'routes/ca.js': '/ca',
|
||||
'routes/openclaw.js': '/openclaw',
|
||||
'routes/license.js': '/license'
|
||||
};
|
||||
for (const relPath of directMounts) {
|
||||
const fullPath = path.join(PKG_ROOT, relPath);
|
||||
if (!fs.existsSync(fullPath)) continue;
|
||||
let factory;
|
||||
try {
|
||||
factory = require(fullPath);
|
||||
} catch (e) { continue; }
|
||||
if (typeof factory !== 'function') continue;
|
||||
let router;
|
||||
try {
|
||||
router = factory(universalDeps);
|
||||
} catch (e) { continue; }
|
||||
// Every direct mount is on apiRouter (which lives at /api/v1) plus an
|
||||
// optional explicit prefix from src/app.js. Walk with the combined prefix
|
||||
// so /api/v1/services/X (bare mount) and /api/v1/ca/X (explicit /ca prefix)
|
||||
// both match what production actually serves.
|
||||
const prefix = '/api/v1' + (prefixMap[relPath] || '');
|
||||
walkRouter(router, prefix, mounted);
|
||||
}
|
||||
|
||||
// ----- 3. Inline routes in src/app.js (apiRouter.get, app.get, etc.) -----
|
||||
const appContent = fs.readFileSync(SRC_APP, 'utf8');
|
||||
const inlineCallRe = /(?:apiRouter|app|router)\.(?:get|post|put|delete|patch)\(\s*['"]([^'"]+)['"]/g;
|
||||
for (const m of appContent.matchAll(inlineCallRe)) {
|
||||
// Skip probe paths handled separately (they're not mounted on apiRouter)
|
||||
if (!m[1].startsWith('/healthz') && !m[1].startsWith('/readyz')) {
|
||||
// Some are root-level (e.g. '/health'), some are apiRouter-level (e.g. '/csrf-token')
|
||||
// We add both interpretations — the source-of-truth check accepts either match
|
||||
mounted.add(m[1]);
|
||||
mounted.add('/api/v1' + m[1]);
|
||||
}
|
||||
}
|
||||
// Also add the 5 probe paths explicitly since they're mounted at root
|
||||
for (const p of ['/health', '/health/live', '/health/ready', '/healthz', '/readyz']) {
|
||||
mounted.add(p);
|
||||
}
|
||||
|
||||
return mounted;
|
||||
}
|
||||
|
||||
// Recursively walk an Express router's stack to collect registered paths
|
||||
function walkRouter(router, basePrefix, mounted) {
|
||||
if (!router || !router.stack) return;
|
||||
for (const layer of router.stack) {
|
||||
if (layer.route) {
|
||||
// Direct route registration: router.get('/path', handler)
|
||||
const path = basePrefix + layer.route.path;
|
||||
// Express adds regex objects; we want the path string
|
||||
if (typeof path === 'string') {
|
||||
mounted.add(path);
|
||||
}
|
||||
} else if (layer.name === 'router' && layer.handle.stack) {
|
||||
// Sub-router mounted via router.use(subRouter)
|
||||
// Express strips the mount path from layer.regex; reconstruct it from layer.regex
|
||||
const mountPath = extractMountPath(layer);
|
||||
walkRouter(layer.handle, basePrefix + mountPath, mounted);
|
||||
} else if (layer.regex && layer.handle !== undefined) {
|
||||
// Middleware with no path (e.g. router.use(initTotp(deps)) where initTotp
|
||||
// returns a router). Express wraps it as a layer with regex.fast_slash=true.
|
||||
// Try to walk it as a sub-router.
|
||||
if (layer.handle && layer.handle.stack) {
|
||||
const mountPath = extractMountPath(layer);
|
||||
walkRouter(layer.handle, basePrefix + mountPath, mounted);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Extract the mount path from an Express layer's regex.
|
||||
// Express stores it in layer.regex as a path-to-regexp regex; the source
|
||||
// string is in layer.regex.source but it's been escaped. We can get the
|
||||
// original path by parsing the source's leading '^\\/?(...)' or use a
|
||||
// simpler heuristic: fast_slash layers mean mount was '/', otherwise
|
||||
// reconstruct from the FastWildcard options.
|
||||
// Since Express internals here are brittle, fall back to a regex source match.
|
||||
function extractMountPath(layer) {
|
||||
if (layer.regex && layer.regex.fast_slash) return '';
|
||||
if (!layer.regex || !layer.regex.source) return '';
|
||||
// The source is something like '^\\/foo\\/?(?=\\/|$)' for mount path '/foo'.
|
||||
// Match the first path segment after the optional leading slash.
|
||||
const m = layer.regex.source.match(/^\\\/\(([^)]+)\)/);
|
||||
if (m) {
|
||||
// Convert path-to-regexp syntax like ':foo' or '*' back to a placeholder.
|
||||
// For simple mounts (no params) this gives us the literal segment.
|
||||
return '/' + m[1];
|
||||
}
|
||||
return '';
|
||||
}
|
||||
|
||||
// Check if path is a prefix in PUBLIC_ROUTES (e.g., '/api/v1/auth/gate/' grants all under it)
|
||||
function isPubliclyCovered(path, publicRoutes) {
|
||||
if (publicRoutes.has(path)) return true;
|
||||
// Try as prefix match
|
||||
for (const entry of publicRoutes) {
|
||||
if (entry.endsWith('/') && path.startsWith(entry)) return true;
|
||||
if (entry === path) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
describe('Public-routes allowlist drift (prevents DC-012-style dead entries)', () => {
|
||||
const publicRoutes = readPublicRoutes();
|
||||
const csrfExcluded = readCsrfExcluded();
|
||||
const mountedRoutes = readMountedRoutes();
|
||||
|
||||
// Helpful diagnostic when tests fail
|
||||
test('sanity: allowlists parsed correctly', () => {
|
||||
expect(publicRoutes.size).toBeGreaterThan(10);
|
||||
expect(csrfExcluded.size).toBeGreaterThan(0);
|
||||
expect(mountedRoutes.size).toBeGreaterThan(10);
|
||||
// Probe paths from DC-012 should all be in PUBLIC_ROUTES
|
||||
for (const p of ['/health', '/health/live', '/health/ready', '/healthz', '/readyz']) {
|
||||
expect(publicRoutes).toContain(p);
|
||||
}
|
||||
});
|
||||
|
||||
describe('No stale PUBLIC_ROUTES entries (the DC-012 failure mode)', () => {
|
||||
test('every PUBLIC_ROUTES entry matches an actual mounted route', () => {
|
||||
const stale = [];
|
||||
for (const entry of publicRoutes) {
|
||||
if (entry.endsWith('/')) continue; // prefix matches, skip
|
||||
if (!mountedRoutes.has(entry)) stale.push(entry);
|
||||
}
|
||||
expect(stale).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('CSRF excludedPaths drift detection', () => {
|
||||
test('every CSRF excludedPath is publicly accessible (else 403)', () => {
|
||||
const broken = [];
|
||||
for (const p of csrfExcluded) {
|
||||
if (!isPubliclyCovered(p, publicRoutes)) broken.push(p);
|
||||
}
|
||||
expect(broken).toEqual([]);
|
||||
});
|
||||
|
||||
test('probe paths are CSRF-exempt (k8s probes never carry CSRF tokens)', () => {
|
||||
// These probe paths MUST be in csrf excludedPaths because k8s/Docker
|
||||
// healthchecks hit them with GET requests and no CSRF token.
|
||||
// (Note: CSRF middleware skips GET/HEAD/OPTIONS anyway, but explicit
|
||||
// listing is the documented pattern and protects against future changes.)
|
||||
for (const p of ['/health', '/health/live', '/health/ready', '/healthz', '/readyz']) {
|
||||
expect(csrfExcluded).toContain(p);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('Request-logging exclusion covers all probe paths', () => {
|
||||
// The middleware.js request-logging skip is a regex-based check inside
|
||||
// the logging middleware. We verify by reading the source and asserting
|
||||
// each probe path appears in the skip set.
|
||||
let middlewareContent;
|
||||
beforeAll(() => {
|
||||
middlewareContent = fs.readFileSync(SRC_MIDDLEWARE, 'utf8');
|
||||
});
|
||||
|
||||
for (const p of ['/health', '/health/live', '/health/ready', '/healthz', '/readyz']) {
|
||||
test(`probe path '${p}' is excluded from request logging`, () => {
|
||||
const pattern = new RegExp(`req\\.path\\s*===?\\s*['"]${p.replace(/\//g, '\\/')}['"]`);
|
||||
expect(middlewareContent).toMatch(pattern);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('Tailscale auth bypass covers all probe paths', () => {
|
||||
// Same as logging exclusion but for the Tailscale auth middleware.
|
||||
// K8s probes don't carry Tailscale identity headers.
|
||||
let middlewareContent;
|
||||
beforeAll(() => {
|
||||
middlewareContent = fs.readFileSync(SRC_MIDDLEWARE, 'utf8');
|
||||
});
|
||||
|
||||
for (const p of ['/health', '/health/live', '/health/ready', '/healthz', '/readyz']) {
|
||||
test(`probe path '${p}' bypasses Tailscale auth`, () => {
|
||||
const pattern = new RegExp(`req\\.path\\s*===?\\s*['"]${p.replace(/\//g, '\\/')}['"]`);
|
||||
expect(middlewareContent).toMatch(pattern);
|
||||
});
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user