fix(discover-adopt): use fetchT + caddy.adminUrl (no hardcoded localhost:2019) (DC-064) [glm-grade=A]

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/);
+    });
+  });
+});

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
+        // Add via Caddy admin API (via fetchT so Origin header is present)
-        const response = await fetch(`${caddyAdminUrl}/config/apps/http/servers/srv0/routes`, {
+        const response = await httpClient(`${caddyAdminUrl}/config/apps/http/servers/srv0/routes`, {
           method: 'POST',
           headers: { 'Content-Type': 'application/json' },
           body: JSON.stringify(routeConfig),

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,
   }));