Compare commits
6
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
 DashCaddy Polish Loop
|
4e75b13e90 | ||
|
DashCaddy Polish Loop
|
597bbf67c8 | ||
|
Hermes
|
a2e2a12eb8 | ||
|
DashCaddy Polish Loop
|
c01a011d47 | ||
|
Hermes
|
74fe35d969 | ||
|
Hermes
|
9779feae70 |
@@ -0,0 +1,241 @@
|
||||
/**
|
||||
* DC-103 / DC-064: discover-adopt regression suite
|
||||
*
|
||||
* DC-064 fixes the Caddy admin URL hardcode (`http://localhost:2019` → resolved
|
||||
* from the injected caddy context's `adminUrl`) and stops the route from
|
||||
* reaching raw `fetch` — it must use the injected `fetchT` (which carries
|
||||
* Origin + httpAgent plumbing via src/utils/http.js) so non-loopback Caddy
|
||||
* admin binds (enforce_origin=true) don't 403 the request.
|
||||
*
|
||||
* This suite pins all four invariants:
|
||||
* 1. Route module signature accepts `fetchT` (won't throw if ctx doesn't pass it).
|
||||
* 2. The mounted route uses `fetchT` when provided (proves by mock counts).
|
||||
* 3. The Caddy admin URL is resolved from `caddy.adminUrl`, NOT hardcoded.
|
||||
* 4. Validation: 400 on missing inputs, 400 on bad subdomain, 409 on duplicate id.
|
||||
*/
|
||||
const express = require('express');
|
||||
const request = require('supertest');
|
||||
|
||||
function createApp({ servicesStateManager, caddy, fetchT, adminUrl } = {}) {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
|
||||
const asyncHandler = (fn) => (req, res, next) => Promise.resolve(fn(req, res, next)).catch(next);
|
||||
const discoverAdoptRoutes = require('../../routes/discover-adopt');
|
||||
|
||||
app.use('/api/v1', discoverAdoptRoutes({
|
||||
docker: null,
|
||||
servicesStateManager: servicesStateManager || null,
|
||||
caddy: caddy === undefined
|
||||
? { adminUrl: adminUrl || 'http://localhost:2019' }
|
||||
: caddy,
|
||||
dns: null,
|
||||
siteConfig: { tld: '.sami' },
|
||||
fetchT,
|
||||
asyncHandler,
|
||||
}));
|
||||
return app;
|
||||
}
|
||||
|
||||
// Helper state manager so the route always has somewhere to write
|
||||
function makeStateManager(initial = []) {
|
||||
let services = Array.isArray(initial) ? [...initial] : [];
|
||||
return {
|
||||
_services: services,
|
||||
// eslint-disable-next-line require-await
|
||||
read: jest.fn().mockImplementation(async () => services),
|
||||
// eslint-disable-next-line require-await
|
||||
update: jest.fn().mockImplementation(async (mutator) => {
|
||||
const next = mutator(services);
|
||||
services = next;
|
||||
return services;
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
describe('DC-064: discover-adopt Caddy admin API safety', () => {
|
||||
describe('DI: fetchT is forwarded to the Caddy admin call', () => {
|
||||
it('uses injected fetchT (not raw fetch) when generating Caddy route', async () => {
|
||||
const fetchTMock = jest.fn().mockResolvedValue({ ok: true, status: 200 });
|
||||
const rawFetchSpy = jest.spyOn(global, 'fetch').mockResolvedValue({ ok: true, status: 200 });
|
||||
try {
|
||||
const sm = makeStateManager();
|
||||
const app = createApp({
|
||||
servicesStateManager: sm,
|
||||
caddy: { adminUrl: 'http://caddy-admin.local:2019' },
|
||||
fetchT: fetchTMock,
|
||||
});
|
||||
|
||||
const res = await request(app).post('/api/v1/discover/adopt').send({
|
||||
containerId: 'abc123def456',
|
||||
serviceId: 'myapp',
|
||||
name: 'My App',
|
||||
port: 8080,
|
||||
protocol: 'http',
|
||||
generateDns: false,
|
||||
generateRoute: true,
|
||||
});
|
||||
|
||||
expect(res.status).toBe(201);
|
||||
expect(fetchTMock).toHaveBeenCalledTimes(1);
|
||||
expect(fetchTMock.mock.calls[0][0]).toBe('http://caddy-admin.local:2019/config/apps/http/servers/srv0/routes');
|
||||
expect(fetchTMock.mock.calls[0][1]).toMatchObject({
|
||||
method: 'POST',
|
||||
headers: expect.objectContaining({ 'Content-Type': 'application/json' }),
|
||||
});
|
||||
// Raw fetch must NOT have been called
|
||||
expect(rawFetchSpy).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
rawFetchSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it('does NOT hardcode http://localhost:2019 when caddy.adminUrl is provided', async () => {
|
||||
const fetchTMock = jest.fn().mockResolvedValue({ ok: true, status: 200 });
|
||||
const sm = makeStateManager();
|
||||
const app = createApp({
|
||||
servicesStateManager: sm,
|
||||
caddy: { adminUrl: 'http://caddy-admin.production:2019' },
|
||||
fetchT: fetchTMock,
|
||||
});
|
||||
const res = await request(app).post('/api/v1/discover/adopt').send({
|
||||
containerId: 'abc123def456', serviceId: 'myapp', name: 'My App', port: 8080,
|
||||
});
|
||||
expect(res.status).toBe(201);
|
||||
const calledUrl = fetchTMock.mock.calls[0][0];
|
||||
expect(calledUrl.startsWith('http://caddy-admin.production:2019')).toBe(true);
|
||||
expect(calledUrl.includes('localhost:2019')).toBe(false);
|
||||
});
|
||||
|
||||
it('falls back to raw fetch when fetchT is omitted (test-only path)', async () => {
|
||||
const rawFetchSpy = jest.spyOn(global, 'fetch').mockResolvedValue({ ok: true, status: 200 });
|
||||
try {
|
||||
const sm = makeStateManager();
|
||||
const app = createApp({
|
||||
servicesStateManager: sm,
|
||||
caddy: { adminUrl: 'http://localhost:2019' },
|
||||
fetchT: null, // explicitly omitted
|
||||
});
|
||||
const res = await request(app).post('/api/v1/discover/adopt').send({
|
||||
containerId: 'abc123def456', serviceId: 'myapp', name: 'My App', port: 8080,
|
||||
});
|
||||
expect(res.status).toBe(201);
|
||||
// Raw fetch used because fetchT is null
|
||||
expect(rawFetchSpy).toHaveBeenCalledTimes(1);
|
||||
} finally {
|
||||
rawFetchSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('source convention: static scan', () => {
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
it('does not contain the hardcoded Caddy admin URL string', () => {
|
||||
const src = fs.readFileSync(
|
||||
path.join(__dirname, '../../routes/discover-adopt.js'),
|
||||
'utf8'
|
||||
);
|
||||
// The exact hardcode from before must be gone
|
||||
const hardcodeMatches = (src.match(/const\s+caddyAdminUrl\s*=\s*['"]http:\/\/localhost:2019['"]/g) || []).length;
|
||||
expect(hardcodeMatches).toBe(0);
|
||||
});
|
||||
|
||||
it('does not call raw fetch() — must use httpClient (fetchT or passed fetch)', () => {
|
||||
const src = fs.readFileSync(
|
||||
path.join(__dirname, '../../routes/discover-adopt.js'),
|
||||
'utf8'
|
||||
);
|
||||
// Raw `fetch(` for the Caddy admin call would be a regression
|
||||
const rawFetchMatches = (src.match(/await\s+fetch\(/g) || []).length;
|
||||
expect(rawFetchMatches).toBe(0);
|
||||
});
|
||||
|
||||
it('declares fetchT in the destructure', () => {
|
||||
const src = fs.readFileSync(
|
||||
path.join(__dirname, '../../routes/discover-adopt.js'),
|
||||
'utf8'
|
||||
);
|
||||
expect(src).toMatch(/function\s*\(\s*\{[^}]*fetchT[^}]*\}\s*\)/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('validation unchanged', () => {
|
||||
it('returns 400 when containerId/serviceId/name are missing', async () => {
|
||||
const app = createApp({ servicesStateManager: makeStateManager() });
|
||||
const res = await request(app).post('/api/v1/discover/adopt').send({
|
||||
containerId: 'abc', serviceId: '', name: '',
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it('returns 400 on invalid port', async () => {
|
||||
const app = createApp({ servicesStateManager: makeStateManager() });
|
||||
const res = await request(app).post('/api/v1/discover/adopt').send({
|
||||
containerId: 'abc', serviceId: 'myapp', name: 'My App', port: 99999,
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it('returns 400 on invalid subdomain (must be lowercase, alphanumeric, hyphens)', async () => {
|
||||
const app = createApp({ servicesStateManager: makeStateManager() });
|
||||
const res = await request(app).post('/api/v1/discover/adopt').send({
|
||||
containerId: 'abc', serviceId: 'Bad_SubDomain!', name: 'My App', port: 80,
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it('returns 409 on duplicate service id', async () => {
|
||||
const sm = makeStateManager([{ id: 'myapp', name: 'Existing' }]);
|
||||
const app = createApp({
|
||||
servicesStateManager: sm,
|
||||
caddy: { adminUrl: 'http://localhost:2019' },
|
||||
fetchT: () => Promise.resolve({ ok: true, status: 200 }),
|
||||
});
|
||||
const res = await request(app).post('/api/v1/discover/adopt').send({
|
||||
containerId: 'abc', serviceId: 'myapp', name: 'Dup', port: 80,
|
||||
});
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Caddy route failure does not corrupt the service entry', () => {
|
||||
it('still returns 200/201 result for service when generateRoute=false', async () => {
|
||||
const sm = makeStateManager();
|
||||
const app = createApp({
|
||||
servicesStateManager: sm,
|
||||
caddy: { adminUrl: 'http://localhost:2019' },
|
||||
fetchT: jest.fn().mockResolvedValue({ ok: true, status: 200 }),
|
||||
});
|
||||
const res = await request(app).post('/api/v1/discover/adopt').send({
|
||||
containerId: 'abc', serviceId: 'myapp', name: 'My App', port: 80,
|
||||
generateRoute: false,
|
||||
generateDns: false,
|
||||
});
|
||||
expect(res.status).toBe(201);
|
||||
expect(res.body.service).toBeTruthy();
|
||||
expect(res.body.service.id).toBe('myapp');
|
||||
expect(sm.update).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('captures Caddy API failure in caddyRoute field without rolling back the service', async () => {
|
||||
const sm = makeStateManager();
|
||||
const app = createApp({
|
||||
servicesStateManager: sm,
|
||||
caddy: { adminUrl: 'http://localhost:2019' },
|
||||
fetchT: jest.fn().mockResolvedValue({ ok: false, status: 403 }),
|
||||
});
|
||||
const res = await request(app).post('/api/v1/discover/adopt').send({
|
||||
containerId: 'abc', serviceId: 'myapp', name: 'My App', port: 80,
|
||||
generateDns: false,
|
||||
generateRoute: true,
|
||||
});
|
||||
// Service was still written even though route generation failed
|
||||
expect(res.status).toBe(201);
|
||||
expect(res.body.service).toBeTruthy();
|
||||
expect(res.body.caddyRoute.status).toBe('failed');
|
||||
expect(res.body.caddyRoute.error).toMatch(/403/);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,284 @@
|
||||
/**
|
||||
* DC-063: errorResponse arg-order invariant regression suite.
|
||||
*
|
||||
* Three layers of correctness pinned by this test:
|
||||
*
|
||||
* (1) The validator at responses.js:76-98 catches wrong-order callers
|
||||
* with a clear TypeError naming statusCode. Defense-in-depth: any
|
||||
* future swap is caught at the smallest possible blast radius
|
||||
* (one TypeError on the request thread) instead of an HTTP 500 HTML
|
||||
* panic for the operator and client.
|
||||
*
|
||||
* (2) The static trees under dashcaddy-api/routes/ and
|
||||
* dashcaddy-api/src/utilities/ follow ONE of two equivalent
|
||||
* conventions consistently:
|
||||
*
|
||||
* Convention A — canonical import `errorResponse` from responses.js.
|
||||
* Callsite shape: errorResponse(res, statusCode, message, extras?)
|
||||
* statusCode must be an integer 100..599; message must be a string.
|
||||
*
|
||||
* Convention B — alias import `error: errorResponse` from responses.js,
|
||||
* which binds the local `errorResponse` to the message-first
|
||||
* helper `error(res, message, statusCode = 500)`.
|
||||
* Callsite shape: errorResponse(res, message, statusCode)
|
||||
*
|
||||
* Mixing the alias-import with the canonical-shape callsite is the
|
||||
* DC-063 bug class: at runtime, the alias function fires
|
||||
* `res.status('event not found')` → TypeError → HTTP 500 HTML panic,
|
||||
* silently masking the intended 4xx JSON response for the client.
|
||||
* The validator at (1) does NOT help because the alias path skips it.
|
||||
*
|
||||
* (3) End-to-end smoke for one of each fixed-file: live HTTP hits the
|
||||
* endpoint with the malformed input that triggers the fix-callsite
|
||||
* branch, and asserts the wire response is the expected 4xx JSON
|
||||
* (status + content-type + body) — never a 500 HTML panic.
|
||||
*
|
||||
* Origin (DC-062): shipped 2026-08-18 by Hermes loop. Found 4 callsites in
|
||||
* routes/caddy-upstreams.js and added the validator.
|
||||
*
|
||||
* DC-063 (this file): extended the search across the routes tree with
|
||||
* alias-import awareness. Found 18 instances of the alias-imported +
|
||||
* canonical-shape callsite bug class in 2 files (security.js + 3 calls
|
||||
* in services.js). Fixed by switching those imports to canonical and
|
||||
* rewriting the remaining 4 alias-shape callsites in services.js to
|
||||
* canonical-shape. Adding this regression test to prevent the same
|
||||
* swap from being reintroduced in future route file edits.
|
||||
*/
|
||||
|
||||
const path = require('path');
|
||||
const express = require('express');
|
||||
const http = require('http');
|
||||
const fs = require('fs');
|
||||
const glob = require('glob');
|
||||
|
||||
const repoRoot = path.join(__dirname, '..', '..'); // dashcaddy-api/
|
||||
const { errorResponse, error: aliasError } = require(
|
||||
path.join(repoRoot, 'src/utils/responses')
|
||||
);
|
||||
|
||||
// ─── (1) Type validator (defense-in-depth) ────────────────────────────────
|
||||
describe('DC-063: errorResponse type validator (defense-in-depth)', () => {
|
||||
function makeRes() {
|
||||
return { status: () => makeRes(), json: () => makeRes() };
|
||||
}
|
||||
|
||||
test('canonical (res, statusCode, message) does not throw and JSON is well-formed', () => {
|
||||
expect(() => errorResponse(makeRes(), 400, 'Invalid level')).not.toThrow();
|
||||
expect(() => errorResponse(makeRes(), 503, 'downstream unavailable', { code: 'DC-503' }))
|
||||
.not.toThrow();
|
||||
});
|
||||
|
||||
test('swapped canonical-shape throws TypeError naming statusCode', () => {
|
||||
expect(() => errorResponse(makeRes(), 'msg-not-status', 400))
|
||||
.toThrow(TypeError);
|
||||
expect(() => errorResponse(makeRes(), 'msg-not-status', 400))
|
||||
.toThrow(/statusCode must be an integer HTTP status \(100\.\.599\)/);
|
||||
});
|
||||
|
||||
test.each([
|
||||
[0, 'below range'],
|
||||
[99, 'below range'],
|
||||
[600, 'above range'],
|
||||
[3.14, 'non-integer'],
|
||||
[NaN, 'NaN'],
|
||||
[Infinity, 'Infinity'],
|
||||
])('rejects numeric out-of-band statusCode %p (%s)', (bad) => {
|
||||
expect(() => errorResponse(makeRes(), bad, 'msg')).toThrow(TypeError);
|
||||
});
|
||||
|
||||
test('rejects non-string message', () => {
|
||||
expect(() => errorResponse(makeRes(), 400, 42)).toThrow(TypeError);
|
||||
expect(() => errorResponse(makeRes(), 400, null)).toThrow(TypeError);
|
||||
expect(() => errorResponse(makeRes(), 400, { err: 'oops' })).toThrow(TypeError);
|
||||
});
|
||||
|
||||
test('extras object merges into body and surfaces top-level code (DC-086)', () => {
|
||||
const captured = {};
|
||||
const res = {
|
||||
status(c) { captured.status = c; return res; },
|
||||
json(b) { captured.body = b; return res; },
|
||||
};
|
||||
errorResponse(res, 400, 'Invalid input', { code: 'DC-400', field: 'level' });
|
||||
expect(captured.status).toBe(400);
|
||||
expect(captured.body).toEqual({
|
||||
success: false,
|
||||
error: 'Invalid input',
|
||||
field: 'level',
|
||||
code: 'DC-400',
|
||||
});
|
||||
});
|
||||
|
||||
test('alias error(res, message, statusCode) still works for backward-compat', () => {
|
||||
expect(() => aliasError(makeRes(), 'msg', 400)).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
// ─── (2) Static tree: every callsite follows its file's imported convention ─
|
||||
describe('DC-063: routes/ + utilities/ arg-order matches each file\'s import', () => {
|
||||
function isNumericLiteral(s) {
|
||||
return /^\d+$/.test(s);
|
||||
}
|
||||
function isExpressionReturningNumber(s) {
|
||||
return /^(err|error|response)\.status(Code)?\s*\|\|.*\d+/.test(s) ||
|
||||
/^response\.status$/.test(s);
|
||||
}
|
||||
function isStringy(s) {
|
||||
s = s.trim();
|
||||
if (s.startsWith('"') || s.startsWith("'") || s.startsWith('`')) return true;
|
||||
if (/^[a-zA-Z_][a-zA-Z_0-9]*\([^)]*\)$/.test(s)) return true; // safeErrorMessage(err)
|
||||
if (/^[a-zA-Z_][a-zA-Z_0-9]*\.[a-zA-Z_][a-zA-Z_0-9.]*$/.test(s)) return true; // err.message
|
||||
return false;
|
||||
}
|
||||
function isNumeric(s) {
|
||||
return isNumericLiteral(s.trim()) || isExpressionReturningNumber(s.trim());
|
||||
}
|
||||
// Match `errorResponse(res, ARG1, ARG2)` (allow extras after).
|
||||
const pat = /errorResponse\(\s*res\s*,\s*([^,]+?)\s*,\s*([^,)\s]+)(?:\s*,|\s*\))/g;
|
||||
|
||||
const ROUTES = glob.sync('routes/*.js', { cwd: repoRoot });
|
||||
const UTILS = glob.sync('src/utilities/*.js', { cwd: repoRoot });
|
||||
const ALL = [...ROUTES, ...UTILS];
|
||||
|
||||
function classifyFile(src) {
|
||||
// Filter comments before classification (the comment can mention the alias).
|
||||
const codeOnly = src.split('\n')
|
||||
.filter((l) => !l.trim().startsWith('//') && !l.trim().startsWith('*') && !l.trim().startsWith('/*'))
|
||||
.join('\n');
|
||||
const is_alias = /\berror:\s*errorResponse\b/.test(codeOnly);
|
||||
return { is_alias };
|
||||
}
|
||||
|
||||
test.each(ALL.map((rel) => [rel]))('%s has consistent callsite shape', (rel) => {
|
||||
const abs = path.join(repoRoot, rel);
|
||||
const src = fs.readFileSync(abs, 'utf8');
|
||||
const { is_alias } = classifyFile(src);
|
||||
const bad = [];
|
||||
for (const m of src.matchAll(pat)) {
|
||||
const a1 = m[1].trim();
|
||||
const a2 = m[2].trim();
|
||||
const lineNo = src.slice(0, m.index).split('\n').length;
|
||||
|
||||
if (is_alias) {
|
||||
// Convention B: arg1 = message (string), arg2 = status (number)
|
||||
if (isNumeric(a1) && isStringy(a2)) {
|
||||
bad.push({ lineNo, a1, a2, reason: 'alias-import + canonical-shape (BUG: alias path skips validator)' });
|
||||
}
|
||||
} else {
|
||||
// Convention A: arg1 = status (number), arg2 = message (string)
|
||||
if (isStringy(a1) && isNumeric(a2)) {
|
||||
bad.push({ lineNo, a1, a2, reason: 'canonical-import + alias-shape (BUG: validator fires TypeError -> 500 HTML)' });
|
||||
}
|
||||
}
|
||||
}
|
||||
if (bad.length) {
|
||||
throw new Error(
|
||||
`${rel}: ${bad.length} inconsistent callsite(s):\n` +
|
||||
bad.map((b) => ` L${b.lineNo}: (${b.a1}, ${b.a2}) — ${b.reason}`).join('\n')
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── (3) End-to-end HTTP smoke — invalid input returns the expected JSON ─
|
||||
describe('DC-063: live HTTP smoke — security.js GET /events/:id returns 404 JSON (not 500)', () => {
|
||||
let server, baseUrl;
|
||||
|
||||
beforeAll(() => {
|
||||
process.env.NODE_ENV = 'test';
|
||||
process.env.DASHCADDY_API_TOKEN = process.env.DASHCADDY_API_TOKEN || 'test-token';
|
||||
process.env.DASHCADDY_ENCRYPTION_KEY = process.env.DASHCADDY_ENCRYPTION_KEY || 'k'.repeat(64);
|
||||
process.env.JWT_SECRET = process.env.JWT_SECRET || 'jwt-test-secret-32-chars-minimum-len';
|
||||
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
|
||||
// Auth shim — bypass host authentication middleware.
|
||||
app.use((_req, _res, next) => next());
|
||||
|
||||
// Shim the security event store with a fake.
|
||||
const fakeStore = {
|
||||
get: () => null,
|
||||
append: () => ({ id: 'fake', accepted: true }),
|
||||
list: () => ({ events: [], total: 0 }),
|
||||
query: () => ({ events: [], total: 0 }),
|
||||
};
|
||||
const fakeRegistry = {
|
||||
list: () => [],
|
||||
register: () => ({ host: {}, api_key: 'x' }),
|
||||
get: () => null,
|
||||
update: () => null,
|
||||
remove: () => true,
|
||||
setEnabled: () => true,
|
||||
authHostByApiKey: () => null,
|
||||
authHostByBearer: () => null,
|
||||
};
|
||||
|
||||
// Inject store + registry via a require-cache swap so security.js's
|
||||
// getStore()/getRegistry() return our fakes.
|
||||
require.cache[path.join(repoRoot, 'src/security/event-store')] = {
|
||||
exports: { getStore: () => fakeStore },
|
||||
id: 'fake-event-store', filename: 'fake', loaded: true,
|
||||
};
|
||||
require.cache[path.join(repoRoot, 'src/security/host-registry')] = {
|
||||
exports: { getRegistry: () => fakeRegistry },
|
||||
id: 'fake-host-registry', filename: 'fake', loaded: true,
|
||||
};
|
||||
// platform-paths is required by security.js — provide a minimal shim.
|
||||
require.cache[path.join(repoRoot, 'platform-paths')] = {
|
||||
exports: { configFile: () => '/tmp/x', dataFile: () => '/tmp/y' },
|
||||
id: 'fake-platform-paths', filename: 'fake', loaded: true,
|
||||
};
|
||||
|
||||
const securityRoutes = require(path.join(repoRoot, 'routes/security'));
|
||||
app.use((req, res, next) => {
|
||||
res.success = (data) => res.json({ success: true, ...data });
|
||||
res.errorResponse = (status, msg, extras) => errorResponse(res, status, msg, extras);
|
||||
res.ok = (data) => res.json({ success: true, ...data });
|
||||
next();
|
||||
});
|
||||
app.use('/api/security', securityRoutes({
|
||||
store: fakeStore,
|
||||
registry: fakeRegistry,
|
||||
log: { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} },
|
||||
}));
|
||||
|
||||
server = http.createServer(app).listen(0);
|
||||
// .listen(0) synchronously assigns a port; no need to wait.
|
||||
baseUrl = `http://127.0.0.1:${server.address().port}`;
|
||||
});
|
||||
|
||||
afterAll((done) => {
|
||||
if (server && server.listening) server.close(done);
|
||||
else done();
|
||||
});
|
||||
|
||||
function get(p) {
|
||||
return new Promise((resolve, reject) => {
|
||||
http.get(`${baseUrl}${p}`, (resp) => {
|
||||
let buf = '';
|
||||
resp.on('data', (c) => { buf += c; });
|
||||
resp.on('end', () => resolve({
|
||||
status: resp.statusCode,
|
||||
body: buf,
|
||||
contentType: resp.headers['content-type'] || '',
|
||||
}));
|
||||
}).on('error', reject);
|
||||
});
|
||||
}
|
||||
|
||||
test('GET /api/security/events/nonexistent — pre-fix crashed with 500 HTML, post-fix returns 404 JSON', async () => {
|
||||
const r = await get('/api/security/events/nonexistent');
|
||||
expect(r.status).toBe(404);
|
||||
expect(r.contentType).toMatch(/application\/json/);
|
||||
expect(r.body).toMatch(/event not found/i);
|
||||
expect(r.body).not.toMatch(/<html|<!DOCTYPE|stack/i); // NOT an HTML panic
|
||||
});
|
||||
|
||||
test('GET /api/security/hosts/nonexistent — pre-fix crashed with 500 HTML, post-fix returns 404 JSON', async () => {
|
||||
const r = await get('/api/security/hosts/nonexistent');
|
||||
expect(r.status).toBe(404);
|
||||
expect(r.contentType).toMatch(/application\/json/);
|
||||
expect(r.body).toMatch(/host not found/i);
|
||||
expect(r.body).not.toMatch(/<html|<!DOCTYPE|stack/i);
|
||||
});
|
||||
});
|
||||
@@ -34,14 +34,23 @@ jest.mock('../../src/utilities/pagination', () => ({
|
||||
parsePaginationParams: jest.fn(() => null),
|
||||
}));
|
||||
|
||||
jest.mock('../../src/utils/responses', () => ({
|
||||
success: jest.fn((res, data, statusCode = 200) => {
|
||||
return res.status(statusCode).json({ success: true, ...data });
|
||||
}),
|
||||
error: jest.fn((res, message, statusCode = 500, extra) => {
|
||||
return res.status(statusCode).json({ success: false, error: message, ...extra });
|
||||
}),
|
||||
}));
|
||||
jest.mock('../../src/utils/responses', () => {
|
||||
// DC-063: services.js now imports canonical `errorResponse` (statusCode-first),
|
||||
// so this mock must expose both that AND the legacy `error` alias to keep the
|
||||
// existing fixture working. The canonical validator is bypassed (tests use it
|
||||
// as a structured passthrough); the alias preserves call-shape for any
|
||||
// remaining legacy import.
|
||||
const errorResponse = jest.fn((res, statusCode, message, extra) =>
|
||||
res.status(statusCode).json({ success: false, error: message, ...extra })
|
||||
);
|
||||
return {
|
||||
success: jest.fn((res, data, statusCode = 200) =>
|
||||
res.status(statusCode).json({ success: true, ...data })
|
||||
),
|
||||
errorResponse,
|
||||
error: errorResponse, // alias used by files that import `error: errorResponse`
|
||||
};
|
||||
});
|
||||
|
||||
// errors module NOT mocked — used for real ValidationError/NotFoundError/ConflictError
|
||||
|
||||
|
||||
@@ -0,0 +1,331 @@
|
||||
/**
|
||||
* DC-062: errorResponse arg-order regression test + caddy-upstreams JSON
|
||||
* response guarantees.
|
||||
*
|
||||
* Background: errorResponse(res, statusCode, message, extras) is the canonical
|
||||
* shape from src/utils/responses.js. Routes that import the bare
|
||||
* `errorResponse` (not the `error: errorResponse` alias) MUST call it
|
||||
* statusCode-first. The classic bug is `errorResponse(res, 'message', 503)`
|
||||
* — Express rejects the string with RangeError [ERR_HTTP_INVALID_STATUS_CODE]
|
||||
* and writes a 500 with an HTML stack trace instead of the intended 503 JSON.
|
||||
*
|
||||
* DC-049 (caddy-upstream-watcher, shipped 2026-08-18) had 4 instances of this
|
||||
* exact pattern in its route file, in the `!caddyUpstreamWatcher` defensive
|
||||
* branch. The branch is currently unreachable in prod (the watcher is always
|
||||
* wired in app.js:818-822) but the latent bug is a 1) crash-handler failure
|
||||
* mode if the watcher module ever errored at load time, 2) wrong response
|
||||
* shape (HTML instead of JSON), and 3) HTTP 500 instead of the intended 503.
|
||||
*
|
||||
* Two layers of fix:
|
||||
* 1. routes/caddy-upstreams.js — swap the 4 callsites to (res, 503, msg).
|
||||
* 2. src/utils/responses.js — add a defensive arg validator on
|
||||
* errorResponse() so any future (res, <not-a-valid-status>, ...)
|
||||
* call FAILS FAST with a clear TypeError instead of writing a 500 HTML
|
||||
* panic to the client. The older `error()` helper (message-first,
|
||||
* imported as `error: errorResponse`) intentionally preserves its
|
||||
* existing API and is untouched.
|
||||
*
|
||||
* This test exercises both fixes.
|
||||
*/
|
||||
|
||||
const express = require('express');
|
||||
const http = require('http');
|
||||
const path = require('path');
|
||||
|
||||
// Use the repo's deps so the test fails under exactly the same module
|
||||
// resolution as production code (otherwise symlink/path differences can
|
||||
// mask validator-install gaps).
|
||||
// __dirname = /opt/dashcaddy/dashcaddy-api/__tests__
|
||||
// __dirname/../src/utils/responses = the file under test
|
||||
const repoRoot = path.join(__dirname, '..');
|
||||
|
||||
const { errorResponse, error: legacyError } = require(path.join(repoRoot, 'src/utils/responses'));
|
||||
|
||||
function get(port, urlPath) {
|
||||
return new Promise((resolve, reject) => {
|
||||
const req = http.get(`http://localhost:${port}${urlPath}`, (resp) => {
|
||||
let body = '';
|
||||
resp.on('data', (c) => { body += c; });
|
||||
resp.on('end', () => resolve({ status: resp.statusCode, headers: resp.headers, body }));
|
||||
});
|
||||
req.on('error', reject);
|
||||
});
|
||||
}
|
||||
|
||||
describe('errorResponse canonical arg-order + type guard (DC-062)', () => {
|
||||
test('correct order — (res, 503, msg) returns 503 JSON', () => {
|
||||
const mockRes = {
|
||||
status(code) { mockRes._code = code; return this; },
|
||||
json(body) { mockRes._body = body; return this; },
|
||||
};
|
||||
errorResponse(mockRes, 503, 'Caddy upstream watcher not initialized');
|
||||
expect(mockRes._code).toBe(503);
|
||||
expect(mockRes._body).toEqual({ success: false, error: 'Caddy upstream watcher not initialized' });
|
||||
});
|
||||
|
||||
test('swapped order — (res, msg, statusCode) throws TypeError instead of writing a 500 HTML panic', () => {
|
||||
// Before DC-062: errorResponse would call res.status('string-msg'),
|
||||
// Express throws RangeError, error middleware catches it, writes 500 HTML.
|
||||
// After DC-062: errorResponse itself rejects the call with a clear
|
||||
// TypeError, naming the wrong arg.
|
||||
const mockRes = {
|
||||
status: () => mockRes,
|
||||
json: () => mockRes,
|
||||
};
|
||||
expect(() => errorResponse(mockRes, 'Caddy upstream watcher not initialized', 503))
|
||||
.toThrow(TypeError);
|
||||
expect(() => errorResponse(mockRes, 'Caddy upstream watcher not initialized', 503))
|
||||
.toThrow(/statusCode must be an integer HTTP status/);
|
||||
});
|
||||
|
||||
test.each([
|
||||
['NaN', NaN],
|
||||
['Infinity', Infinity],
|
||||
['string "503"', '503'],
|
||||
['null', null],
|
||||
['undefined', undefined],
|
||||
['underflow 99', 99],
|
||||
['overflow 600', 600],
|
||||
['float 503.5', 503.5],
|
||||
['object', { code: 503 }],
|
||||
['array', [503]],
|
||||
])('rejects invalid statusCode %s', (_name, badStatus) => {
|
||||
const mockRes = {
|
||||
status: () => mockRes,
|
||||
json: () => mockRes,
|
||||
};
|
||||
expect(() => errorResponse(mockRes, badStatus, 'msg')).toThrow(TypeError);
|
||||
});
|
||||
|
||||
test('rejects non-string message', () => {
|
||||
const mockRes = {
|
||||
status: () => mockRes,
|
||||
json: () => mockRes,
|
||||
};
|
||||
expect(() => errorResponse(mockRes, 503, 123)).toThrow(TypeError);
|
||||
expect(() => errorResponse(mockRes, 503, null)).toThrow(TypeError);
|
||||
expect(() => errorResponse(mockRes, 503, undefined)).toThrow(TypeError);
|
||||
expect(() => errorResponse(mockRes, 503, { msg: 'x' })).toThrow(TypeError);
|
||||
});
|
||||
|
||||
test('preserves correct callers (DC-086 extras.code propagation still works)', () => {
|
||||
const mockRes = {
|
||||
status: () => mockRes,
|
||||
json: (b) => { mockRes._lastBody = b; return mockRes; },
|
||||
};
|
||||
errorResponse(mockRes, 409, 'Conflict', { code: 'DC-CONF-1', extra: 'detail' });
|
||||
expect(mockRes._lastBody).toEqual({
|
||||
success: false,
|
||||
error: 'Conflict',
|
||||
code: 'DC-CONF-1',
|
||||
extra: 'detail',
|
||||
});
|
||||
});
|
||||
|
||||
test('legacy `error()` helper (message, status) is UNCHANGED — still works', () => {
|
||||
// Regression guard for alias-style importers (dns.js, services.js,
|
||||
// ssl-monitor.js, license.js, dependencies.js, errorlogs.js, etc.).
|
||||
// The legacy helper takes (res, message, statusCode) order. Make sure
|
||||
// the validator we added to `errorResponse` doesn't bleed into
|
||||
// `error()`.
|
||||
const mockRes = {
|
||||
status(code) { mockRes._code = code; return this; },
|
||||
json(body) { mockRes._body = body; return this; },
|
||||
};
|
||||
legacyError(mockRes, 'service unavailable', 503);
|
||||
expect(mockRes._code).toBe(503);
|
||||
expect(mockRes._body).toEqual({ success: false, error: 'service unavailable' });
|
||||
});
|
||||
|
||||
test('regression: an Express response with res.status(string) emits HTML 500 — proves the bug pre-fix', async () => {
|
||||
// This is the failure mode DC-062 prevents. We still need this to
|
||||
// be true to prove the guard's value: if a call site ever slipped past
|
||||
// the validator (e.g. by sending a non-number disguised as code 0),
|
||||
// the server still doesn't return the intended status as JSON.
|
||||
const server = await new Promise((resolve) => {
|
||||
const app = express();
|
||||
app.get('/probe', (req, res) => {
|
||||
try {
|
||||
res.status('not a status').json({ ok: false });
|
||||
} catch (_) {
|
||||
res.end();
|
||||
}
|
||||
});
|
||||
const s = app.listen(0, () => resolve({
|
||||
port: s.address().port,
|
||||
close: () => new Promise((r) => s.close(r)),
|
||||
}));
|
||||
});
|
||||
try {
|
||||
const resp = await get(server.port, '/probe');
|
||||
expect(resp.status).toBe(500);
|
||||
// Express renders an HTML error page (not JSON) — this is the bug
|
||||
// class DC-062 prevents at the helper layer.
|
||||
expect(resp.headers['content-type'] || '').toMatch(/text\/html/);
|
||||
} finally {
|
||||
await server.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// Mount the real route module and inject a null watcher — proves the
|
||||
// the four `!caddyUpstreamWatcher` paths now respond with the intended
|
||||
// 503 JSON shape, not a 500 HTML panic.
|
||||
describe('caddy-upstreams JSON response shape (route file literal fix)', () => {
|
||||
// The real route module exports a factory `function({ asyncHandler, caddyUpstreamWatcher, healthChecker })`.
|
||||
// We need to provide an asyncHandler shim since the route file uses it.
|
||||
function asyncHandlerShim(fn) { return fn; }
|
||||
// The factory also depends on the asyncHandler resolving rejected
|
||||
// promises to errors. Define a simple one that just calls next(err).
|
||||
function asyncHandler(fn) {
|
||||
return (req, res, next) => {
|
||||
Promise.resolve(fn(req, res, next)).catch(next);
|
||||
};
|
||||
}
|
||||
|
||||
function mountRouter(router) {
|
||||
return new Promise((resolve) => {
|
||||
const app = express();
|
||||
app.use('/api/v1', router);
|
||||
const server = app.listen(0, () => resolve({
|
||||
port: server.address().port,
|
||||
close: () => new Promise((r) => server.close(r)),
|
||||
}));
|
||||
});
|
||||
}
|
||||
|
||||
function loadRoute(deps) {
|
||||
return require(path.join(repoRoot, 'routes/caddy-upstreams'))(deps);
|
||||
}
|
||||
|
||||
test('GET /caddy/upstreams with null watcher — 503 JSON (regression for swap bug)', async () => {
|
||||
const router = loadRoute({
|
||||
asyncHandler,
|
||||
caddyUpstreamWatcher: null,
|
||||
healthChecker: null,
|
||||
});
|
||||
const server = await mountRouter(router);
|
||||
try {
|
||||
const resp = await get(server.port, '/api/v1/caddy/upstreams');
|
||||
expect(resp.status).toBe(503);
|
||||
expect(resp.body).toContain('"success":false');
|
||||
expect(resp.body).toContain('Caddy upstream watcher not initialized');
|
||||
expect(resp.headers['content-type'] || '').toMatch(/application\/json/);
|
||||
} finally {
|
||||
await server.close();
|
||||
}
|
||||
});
|
||||
|
||||
test('POST /caddy/upstreams/:host/mute with null watcher — 503 JSON', async () => {
|
||||
const router = loadRoute({
|
||||
asyncHandler,
|
||||
caddyUpstreamWatcher: null,
|
||||
healthChecker: null,
|
||||
});
|
||||
const server = await mountRouter(router);
|
||||
try {
|
||||
const req = http.request({
|
||||
hostname: 'localhost',
|
||||
port: server.port,
|
||||
method: 'POST',
|
||||
path: '/api/v1/caddy/upstreams/100.74.102.61:8080/mute',
|
||||
}, (res) => {
|
||||
let body = '';
|
||||
res.on('data', (c) => { body += c; });
|
||||
res.on('end', () => {
|
||||
expect(res.statusCode).toBe(503);
|
||||
expect(body).toContain('"success":false');
|
||||
expect(body).toContain('Caddy upstream watcher not initialized');
|
||||
expect(res.headers['content-type'] || '').toMatch(/application\/json/);
|
||||
server.close();
|
||||
});
|
||||
});
|
||||
req.on('error', (e) => { throw e; });
|
||||
req.end();
|
||||
} finally {
|
||||
// server.close() will run via res.on('end') — defensively guard too.
|
||||
// (Don't double-close if test already returned.)
|
||||
}
|
||||
});
|
||||
|
||||
test('POST /caddy/upstreams/mute (bare) with null watcher — 503 JSON', async () => {
|
||||
const router = loadRoute({
|
||||
asyncHandler,
|
||||
caddyUpstreamWatcher: null,
|
||||
healthChecker: null,
|
||||
});
|
||||
const server = await mountRouter(router);
|
||||
try {
|
||||
const resp = await new Promise((resolve, reject) => {
|
||||
const req = http.request({
|
||||
hostname: 'localhost',
|
||||
port: server.port,
|
||||
method: 'POST',
|
||||
path: '/api/v1/caddy/upstreams/mute',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
}, (res) => {
|
||||
let body = '';
|
||||
res.on('data', (c) => { body += c; });
|
||||
res.on('end', () => resolve({ status: res.statusCode, headers: res.headers, body }));
|
||||
});
|
||||
req.on('error', reject);
|
||||
req.end('{"host":"x","muted":true}');
|
||||
});
|
||||
expect(resp.status).toBe(503);
|
||||
expect(resp.body).toContain('Caddy upstream watcher not initialized');
|
||||
expect(resp.headers['content-type'] || '').toMatch(/application\/json/);
|
||||
} finally {
|
||||
await server.close();
|
||||
}
|
||||
});
|
||||
|
||||
test('POST /caddy/upstreams/:host/unmute with null watcher — 503 JSON', async () => {
|
||||
const router = loadRoute({
|
||||
asyncHandler,
|
||||
caddyUpstreamWatcher: null,
|
||||
healthChecker: null,
|
||||
});
|
||||
const server = await mountRouter(router);
|
||||
try {
|
||||
const resp = await new Promise((resolve, reject) => {
|
||||
const req = http.request({
|
||||
hostname: 'localhost',
|
||||
port: server.port,
|
||||
method: 'POST',
|
||||
path: '/api/v1/caddy/upstreams/100.74.102.61:8080/unmute',
|
||||
}, (res) => {
|
||||
let body = '';
|
||||
res.on('data', (c) => { body += c; });
|
||||
res.on('end', () => resolve({ status: res.statusCode, headers: res.headers, body }));
|
||||
});
|
||||
req.on('error', reject);
|
||||
req.end();
|
||||
});
|
||||
expect(resp.status).toBe(503);
|
||||
expect(resp.body).toContain('Caddy upstream watcher not initialized');
|
||||
expect(resp.headers['content-type'] || '').toMatch(/application\/json/);
|
||||
} finally {
|
||||
await server.close();
|
||||
}
|
||||
});
|
||||
|
||||
test('route file source: no swapped-order patterns remain', () => {
|
||||
// Static scan of the post-fix route file: confirms the 4 swapped calls
|
||||
// are gone. If a future refactor re-introduces the pattern, this scan
|
||||
// catches it at test-time (before it ever lands in prod).
|
||||
const fs = require('fs');
|
||||
const src = fs.readFileSync(
|
||||
path.join(repoRoot, 'routes/caddy-upstreams.js'),
|
||||
'utf8'
|
||||
);
|
||||
// Match `errorResponse(res, <quote-or-backtick>, <int>)` — the
|
||||
// swapped-order shape (string literal in the 2nd arg position).
|
||||
const swappedRe = /errorResponse\(res,\s*['"`]/;
|
||||
expect(src).not.toMatch(swappedRe);
|
||||
// And confirm the corrected shape appears at least four times
|
||||
// (the four `!caddyUpstreamWatcher` guards).
|
||||
const canonicalRe = /errorResponse\(res,\s*503,\s*['"]Caddy upstream watcher not initialized['"]/g;
|
||||
const matches = src.match(canonicalRe) || [];
|
||||
expect(matches.length).toBe(4);
|
||||
});
|
||||
});
|
||||
@@ -21,7 +21,14 @@ module.exports = function({ asyncHandler, caddyUpstreamWatcher, healthChecker })
|
||||
|
||||
router.get('/caddy/upstreams', asyncHandler(async (req, res) => {
|
||||
if (!caddyUpstreamWatcher) {
|
||||
return errorResponse(res, 'Caddy upstream watcher not initialized', 503);
|
||||
// DC-062: errorResponse(res, statusCode, message) — statusCode-first per
|
||||
// src/utils/responses.js:66. The prior (res, message, statusCode) call
|
||||
// order passed a STRING as the status code, which made
|
||||
// res.status('Caddy upstream watcher not initialized') throw
|
||||
// RangeError [ERR_HTTP_INVALID_STATUS_CODE] (Express turning it into a
|
||||
// 500 with an HTML stack trace). All four `!caddyUpstreamWatcher`
|
||||
// guards had the same latent bug — fixed to canonical order.
|
||||
return errorResponse(res, 503, 'Caddy upstream watcher not initialized');
|
||||
}
|
||||
success(res, caddyUpstreamWatcher.snapshot());
|
||||
}, 'caddy-upstreams-list'));
|
||||
@@ -54,7 +61,7 @@ module.exports = function({ asyncHandler, caddyUpstreamWatcher, healthChecker })
|
||||
// ergonomic depending on caller.
|
||||
const handleMute = asyncHandler(async (req, res) => {
|
||||
if (!caddyUpstreamWatcher) {
|
||||
return errorResponse(res, 'Caddy upstream watcher not initialized', 503);
|
||||
return errorResponse(res, 503, 'Caddy upstream watcher not initialized');
|
||||
}
|
||||
const host = req.params.host || req.body?.host;
|
||||
if (!host || typeof host !== 'string' || !/^[a-z0-9._:-]+$/i.test(host)) {
|
||||
@@ -76,7 +83,7 @@ module.exports = function({ asyncHandler, caddyUpstreamWatcher, healthChecker })
|
||||
// absent or unparseable; require muted === false explicitly to unmute.
|
||||
router.post('/caddy/upstreams/mute', asyncHandler(async (req, res) => {
|
||||
if (!caddyUpstreamWatcher) {
|
||||
return errorResponse(res, 'Caddy upstream watcher not initialized', 503);
|
||||
return errorResponse(res, 503, 'Caddy upstream watcher not initialized');
|
||||
}
|
||||
const { host, muted } = req.body || {};
|
||||
if (!host || typeof host !== 'string' || !/^[a-z0-9._:-]+$/i.test(host)) {
|
||||
@@ -95,7 +102,7 @@ module.exports = function({ asyncHandler, caddyUpstreamWatcher, healthChecker })
|
||||
router.post('/caddy/upstreams/:host/mute', handleMute);
|
||||
router.post('/caddy/upstreams/:host/unmute', asyncHandler(async (req, res) => {
|
||||
if (!caddyUpstreamWatcher) {
|
||||
return errorResponse(res, 'Caddy upstream watcher not initialized', 503);
|
||||
return errorResponse(res, 503, 'Caddy upstream watcher not initialized');
|
||||
}
|
||||
const host = req.params.host;
|
||||
if (!host || !/^[a-z0-9._:-]+$/i.test(host)) {
|
||||
|
||||
@@ -8,12 +8,17 @@
|
||||
* 3. A DashCaddy service entry
|
||||
*
|
||||
* Used by the "one-click add" flow in the discovery UI.
|
||||
*
|
||||
* DC-064: Caddy admin API safety — uses `fetchT` (with Origin + CSRF cookie
|
||||
* plumbing via http.js) instead of raw `fetch`, and resolves the admin URL
|
||||
* from the injected `caddy` context's `adminUrl` (which itself falls back to
|
||||
* `process.env.CADDY_ADMIN_URL`) instead of a hardcoded `localhost:2019`.
|
||||
*/
|
||||
const express = require('express');
|
||||
const { ok, errorResponse } = require('../src/utils/responses');
|
||||
const { ErrorCodes } = require('../src/utilities/error-codes');
|
||||
|
||||
module.exports = function({ docker, servicesStateManager, caddy, dns, siteConfig, asyncHandler }) {
|
||||
module.exports = function({ docker, servicesStateManager, caddy, dns, siteConfig, asyncHandler, fetchT }) {
|
||||
const router = express.Router();
|
||||
|
||||
/**
|
||||
@@ -65,7 +70,15 @@ module.exports = function({ docker, servicesStateManager, caddy, dns, siteConfig
|
||||
const tld = siteConfig?.tld || '.sami';
|
||||
const domain = `${serviceId}${tld}`;
|
||||
const upstreamHost = protocol === 'https' ? 'https' : 'http';
|
||||
const caddyAdminUrl = 'http://localhost:2019';
|
||||
// DC-064: resolve the Caddy admin URL from the caddy context (which
|
||||
// itself defaults to process.env.CADDY_ADMIN_URL via context/caddy.js).
|
||||
// Never hardcode localhost:2019 — non-loopback Caddy binds disable
|
||||
// enforce_origin and the raw fetch below would 403. Using fetchT (when
|
||||
// provided) includes the Origin header that satisfies enforce_origin;
|
||||
// when fetchT is null we fall back to raw fetch but ONLY for tests that
|
||||
// explicitly mock the admin URL.
|
||||
const caddyAdminUrl = caddy?.adminUrl || process.env.CADDY_ADMIN_URL || 'http://localhost:2019';
|
||||
const httpClient = typeof fetchT === 'function' ? fetchT : fetch;
|
||||
|
||||
const result = {
|
||||
service: null,
|
||||
@@ -119,8 +132,8 @@ module.exports = function({ docker, servicesStateManager, caddy, dns, siteConfig
|
||||
terminal: true,
|
||||
};
|
||||
|
||||
// Add via Caddy admin API
|
||||
const response = await fetch(`${caddyAdminUrl}/config/apps/http/servers/srv0/routes`, {
|
||||
// Add via Caddy admin API (via fetchT so Origin header is present)
|
||||
const response = await httpClient(`${caddyAdminUrl}/config/apps/http/servers/srv0/routes`, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(routeConfig),
|
||||
|
||||
@@ -30,7 +30,11 @@
|
||||
*/
|
||||
|
||||
const express = require('express');
|
||||
const { ok, error: errorResponse } = require('../src/utils/responses');
|
||||
// DC-063: use the canonical `errorResponse(res, statusCode, message, extras)`
|
||||
// shape — alias `error: errorResponse` used here previously was message-first
|
||||
// which silently mis-called every callsite (15 endpoints surfaced as 500 HTML
|
||||
// panics instead of the intended 4xx JSON).
|
||||
const { ok, errorResponse } = require('../src/utils/responses');
|
||||
const { getStore } = require('../src/security/event-store');
|
||||
const { getRegistry } = require('../src/security/host-registry');
|
||||
const platformPaths = require('../platform-paths');
|
||||
|
||||
@@ -10,7 +10,11 @@ const { exists } = require('../src/utilities/fs-helpers');
|
||||
const { paginate, parsePaginationParams } = require('../src/utilities/pagination');
|
||||
const { ValidationError, NotFoundError, ConflictError } = require('../src/utilities/errors');
|
||||
const { resolveServiceUrl } = require('../src/utilities/url-resolver');
|
||||
const { success, error: errorResponse } = require('../src/utils/responses');
|
||||
// DC-063: use the canonical `errorResponse(res, statusCode, message, extras)`
|
||||
// shape — alias `error: errorResponse` used here previously was message-first
|
||||
// which silently mis-called 3 credential-store callsites (returned 500 HTML
|
||||
// panics for invalid serviceId instead of the intended 400 JSON).
|
||||
const { success, errorResponse } = require('../src/utils/responses');
|
||||
const platformPaths = require('../platform-paths');
|
||||
|
||||
/**
|
||||
@@ -398,7 +402,8 @@ module.exports = function({
|
||||
try {
|
||||
validateServiceConfig({ id, name });
|
||||
} catch (validationErr) {
|
||||
return errorResponse(res, validationErr.message, 400, { errors: validationErr.errors });
|
||||
// DC-063: canonical shape (res, statusCode, message, extras) per responses.js:76.
|
||||
return errorResponse(res, 400, validationErr.message, { errors: validationErr.errors });
|
||||
}
|
||||
|
||||
await servicesStateManager.update(services => {
|
||||
@@ -423,7 +428,8 @@ module.exports = function({
|
||||
} catch (error) {
|
||||
log.error('deploy', error, null, { note: 'Error adding service' });
|
||||
if (error.message.includes('already exists')) {
|
||||
errorResponse(res, safeErrorMessage(error), 409);
|
||||
// DC-063: canonical shape per responses.js:76.
|
||||
errorResponse(res, 409, safeErrorMessage(error));
|
||||
} else {
|
||||
// Error handled by middleware
|
||||
}
|
||||
@@ -445,7 +451,8 @@ module.exports = function({
|
||||
try {
|
||||
validateServiceConfig(service);
|
||||
} catch (validationErr) {
|
||||
return errorResponse(res, `Invalid service "${service.id}": ${validationErr.message}`, 400, { errors: validationErr.errors });
|
||||
// DC-063: canonical shape per responses.js:76.
|
||||
return errorResponse(res, 400, `Invalid service "${service.id}": ${validationErr.message}`, { errors: validationErr.errors });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -475,7 +482,8 @@ module.exports = function({
|
||||
});
|
||||
|
||||
if (!found) {
|
||||
return errorResponse(res, `Service "${id}" not found`, 404);
|
||||
// DC-063: canonical shape per responses.js:76.
|
||||
return errorResponse(res, 404, `Service "${id}" not found`);
|
||||
}
|
||||
|
||||
resyncHealthChecker?.().catch(() => {});
|
||||
|
||||
@@ -634,6 +634,7 @@ async function createApp() {
|
||||
caddy: ctx.caddy,
|
||||
dns: ctx.dns,
|
||||
siteConfig: ctx.config,
|
||||
fetchT: ctx.fetchT,
|
||||
asyncHandler: ctx.asyncHandler,
|
||||
}));
|
||||
|
||||
|
||||
@@ -62,8 +62,34 @@ function noContent(res) {
|
||||
*
|
||||
* DC-086: If extras.code is set, it's treated as a machine-readable error code
|
||||
* (e.g. 'DC-CONT-002'). If message looks like a DC code, it's auto-extracted.
|
||||
*
|
||||
* DC-062: Validate that `statusCode` is a valid HTTP status (integer in
|
||||
* 100..599) BEFORE calling res.status(). Without this guard, a caller who
|
||||
* passes (res, message, statusCode) instead of (res, statusCode, message)
|
||||
* ends up with res.status(<string>), which throws
|
||||
* RangeError [ERR_HTTP_INVALID_STATUS_CODE] — Express catches that and
|
||||
* writes a 500 with an HTML stack trace to the client, which is the worst
|
||||
* possible failure mode (looks like a server crash, breaks CSRF and
|
||||
* content-type expectations, leaks the stack). Failing fast with a clear
|
||||
* TypeError names the call site early in the request lifecycle.
|
||||
*/
|
||||
function errorResponse(res, statusCode, message, extras = {}) {
|
||||
if (
|
||||
typeof statusCode !== 'number'
|
||||
|| !Number.isFinite(statusCode)
|
||||
|| !Number.isInteger(statusCode)
|
||||
|| statusCode < 100
|
||||
|| statusCode > 599
|
||||
) {
|
||||
throw new TypeError(
|
||||
`errorResponse(res, statusCode, message, extras): statusCode must be an integer HTTP status (100..599); received ${JSON.stringify(statusCode)} (message=${JSON.stringify(message)})`
|
||||
);
|
||||
}
|
||||
if (typeof message !== 'string') {
|
||||
throw new TypeError(
|
||||
`errorResponse(res, statusCode, message, extras): message must be a string; received ${typeof message} ${JSON.stringify(message)}`
|
||||
);
|
||||
}
|
||||
const body = { success: false, error: message, ...extras };
|
||||
// DC-086: surface machine-readable code at top level for client handling
|
||||
if (extras.code) {
|
||||
|
||||
Reference in New Issue
Block a user