From 597bbf67c8d132c1a7a07a19a5706a819cc5c228 Mon Sep 17 00:00:00 2001 From: DashCaddy Polish Loop Date: Tue, 18 Aug 2026 11:31:47 -0700 Subject: [PATCH] fix(discover-adopt): use fetchT + caddy.adminUrl (no hardcoded localhost:2019) (DC-064) [glm-grade=A] --- .../routes/discover-adopt.routes.test.js | 241 ++++++++++++++++++ dashcaddy-api/routes/discover-adopt.js | 21 +- dashcaddy-api/src/app.js | 1 + 3 files changed, 259 insertions(+), 4 deletions(-) create mode 100644 dashcaddy-api/__tests__/routes/discover-adopt.routes.test.js diff --git a/dashcaddy-api/__tests__/routes/discover-adopt.routes.test.js b/dashcaddy-api/__tests__/routes/discover-adopt.routes.test.js new file mode 100644 index 0000000..a65bc4c --- /dev/null +++ b/dashcaddy-api/__tests__/routes/discover-adopt.routes.test.js @@ -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/); + }); + }); +}); diff --git a/dashcaddy-api/routes/discover-adopt.js b/dashcaddy-api/routes/discover-adopt.js index c6dcd54..ef6bec4 100644 --- a/dashcaddy-api/routes/discover-adopt.js +++ b/dashcaddy-api/routes/discover-adopt.js @@ -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), diff --git a/dashcaddy-api/src/app.js b/dashcaddy-api/src/app.js index b3a8588..95da181 100644 --- a/dashcaddy-api/src/app.js +++ b/dashcaddy-api/src/app.js @@ -634,6 +634,7 @@ async function createApp() { caddy: ctx.caddy, dns: ctx.dns, siteConfig: ctx.config, + fetchT: ctx.fetchT, asyncHandler: ctx.asyncHandler, }));