From 333cbc0aa8c2312def68100001aa12cc1242632a Mon Sep 17 00:00:00 2001 From: CallMeTechie <34693633+CallMeTechie@users.noreply.github.com> Date: Thu, 25 Jun 2026 21:39:54 +0200 Subject: [PATCH] fix: refuse Caddy /load over a config owned by another instance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The container runs network_mode: host, so the Caddy admin API on 127.0.0.1:2019 is shared with every process on the host — including dev/test runs in .claude worktrees. On 2026-06-25 a host-side run without NODE_ENV=test pushed test-seed routes (x*.0.example.com) via POST /load, replacing the 18 real routes for ~90s and breaking TLS for domaincaster.com (ERR_SSL_PROTOCOL_ERROR). The NODE_ENV guard only protects processes that remember to set it. Add a config-driven ownership guard: - caddyOwner.js: a persistent per-instance id (stored under the Caddy data dir, stable across restarts; ephemeral fallback when the dir is not writable — which, being different from prod's id, makes a foreign process refuse to push). - buildCaddyConfig stamps an owner marker route (impossible host match, gc_owner_ @id) as the last route of srv0. Route-level because Caddy only echoes route @ids back in GET /config/; the foreign owner must be readable to be compared. - _syncToCaddyInner reads the live config before /load and uses ownershipDecision(): fail CLOSED on a genuine read error (cannot verify ownership), refuse on a foreign owner, proceed when null/fresh or our own. (A null "Caddy not running" is claimable; only a thrown read error fails closed, so prod recovery after a Caddy restart is preserved.) - caddyReconciler counts only gc_route_ ids, so the gc_owner_ marker never triggers a divergence/auto-repair loop. The guard is advisory (prevents accidental clobbering), not a security boundary — see caddyOwner.js header. Verified: the full prod config with the marker passes `caddy validate` against GateControl's custom Caddy build; 51 unit tests + 159 buildCaddyConfig tests pass. --- src/services/caddyConfig.js | 33 ++++++- src/services/caddyOwner.js | 168 ++++++++++++++++++++++++++++++++ src/services/caddyReconciler.js | 8 +- tests/caddyOwner.test.js | 156 +++++++++++++++++++++++++++++ tests/caddyReconciler.test.js | 21 ++++ 5 files changed, 383 insertions(+), 3 deletions(-) create mode 100644 src/services/caddyOwner.js create mode 100644 tests/caddyOwner.test.js diff --git a/src/services/caddyConfig.js b/src/services/caddyConfig.js index 7c7d7fd6..e3cf9aff 100644 --- a/src/services/caddyConfig.js +++ b/src/services/caddyConfig.js @@ -46,6 +46,7 @@ const { buildRouteAuthProxy, buildAuthHandlerChain } = require('./caddyAuthSubro const { getAclPeers, setAclPeers } = require('./caddyAcl'); const { renderMaintenancePage } = require('./caddyMaintenance'); const { renderAccessWindowPage } = require('./caddyAccessWindow'); +const { getOwnerId, ownerMarkerRoute, extractOwner, ownershipDecision } = require('./caddyOwner'); const { caddyApi, _caddyApi, @@ -834,6 +835,11 @@ function buildCaddyConfig(injectedRoutes, options = {}) { } if (serverRoutes.length > 0) { + // Ownership marker — LAST route, impossible host match (never served). + // Tags this config with our instance id so a foreign process reading the + // live config refuses to overwrite us. gc_owner_ prefix is ignored by the + // caddyReconciler (which counts only gc_route_ ids), so it never drifts. + serverRoutes.push(ownerMarkerRoute(getOwnerId())); caddyConfig.apps.http.servers.srv0 = { listen: [':443', ':80'], routes: serverRoutes, @@ -928,9 +934,34 @@ async function _syncToCaddyInner() { if (process.env.NODE_ENV === 'test') return; let previousConfig = null; + let readError = null; try { + // caddyApi returns null (not throws) when Caddy is simply not running. previousConfig = await caddyApi('/config/'); - } catch {} + } catch (err) { + readError = err; + } + + // Ownership guard: never overwrite a Caddy that belongs to a DIFFERENT + // GateControl instance. The container runs network_mode: host, so + // 127.0.0.1:2019 is shared with every host process (incl. dev/test runs in + // .claude worktrees). Without this, a foreign process that forgot + // NODE_ENV=test clobbers the live prod config (the 2026-06-25 + // ERR_SSL_PROTOCOL_ERROR incident). See caddyOwner.ownershipDecision: + // read-error → fail closed (cannot verify owner; don't risk a clobber) + // foreign → refuse; proceed → null/fresh (claimable) or our own. + const decision = ownershipDecision(previousConfig, readError, getOwnerId()); + if (decision === 'read-error') { + logger.error({ err: readError && readError.message }, 'Could not read live Caddy config to verify ownership — skipping sync'); + return false; + } + if (decision === 'foreign') { + logger.error( + { liveOwner: extractOwner(previousConfig), myOwner: getOwnerId() }, + 'Live Caddy is owned by another GateControl instance — refusing /load to avoid clobbering it', + ); + return false; + } const caddyConfig = buildCaddyConfig(); diff --git a/src/services/caddyOwner.js b/src/services/caddyOwner.js new file mode 100644 index 00000000..cc8721fd --- /dev/null +++ b/src/services/caddyOwner.js @@ -0,0 +1,168 @@ +'use strict'; + +// ─── Caddy ownership guard ────────────────────────────────────────────── +// +// The deployed container runs with `network_mode: host`, so the Caddy admin +// API on 127.0.0.1:2019 is NOT isolated — every process on the host can POST +// /load and overwrite the live production config. On 2026-06-25 a dev run in +// a `.claude` worktree (no NODE_ENV=test) did exactly that, replacing the 18 +// real routes with test-seed routes (x*.0.example.com) for ~90s and breaking +// TLS for domaincaster.com (ERR_SSL_PROTOCOL_ERROR). +// +// The NODE_ENV==='test' guard in caddyAdminClient/caddyConfig only protects +// processes that remember to set it. This module adds a stronger, config- +// driven defence: every config the production instance pushes carries a +// marker route tagged with a persistent per-instance id. Before a full +// /load, the pusher reads the live config's owner; if it belongs to a +// DIFFERENT instance, the push is refused. A foreign process (different data +// dir → different id) therefore cannot clobber production even without +// NODE_ENV=test. +// +// The marker is a ROUTE (not a server-level @id) because Caddy only echoes +// route-level @ids back in GET /config/ — a server-level @id is addressable +// via /id/ but absent from the body, so the foreign owner could not be read. +// Its @id uses the `gc_owner_` prefix; caddyReconciler counts only +// `gc_route_` ids, so the marker never triggers a divergence/repair loop. +// +// Scope: this is an ADVISORY guard against ACCIDENTAL clobbering (a dev/test +// run that forgot NODE_ENV=test), NOT a security boundary. The owner id is +// visible in plaintext over the shared GET /config/, and any host process can +// POST /load directly; a malicious local actor is not in scope (the real fix +// for that is not sharing the admin API via network_mode: host). There is also +// a benign TOCTOU window — two instances that BOTH boot against a fresh +// (unowned) Caddy each read null and claim it, last-write-wins — which is +// acceptable because production runs a single instance and the loser is just a +// transient dev process the reconciler/next sync corrects. + +const crypto = require('node:crypto'); +const fs = require('node:fs'); +const path = require('node:path'); +const config = require('../../config/default'); +const logger = require('../utils/logger'); + +const OWNER_ID_PREFIX = 'gc_owner_'; +const OWNER_FILE = '.caddy-owner'; +// Host that can never appear on the wire (RFC 6761 reserved TLD). The marker +// route matches only this host, so it never serves or shadows a real route. +const MARKER_HOST = 'gc-owner.invalid'; + +let _cachedOwnerId = null; + +// Resolved at call time (not module load) so the persisted owner file follows +// GC_CADDY_DATA_DIR and tests can point it at a temp dir. +function _ownerDataDir() { + return process.env.GC_CADDY_DATA_DIR + || (config.caddy && config.caddy.dataDir) + || '/data/caddy'; +} + +// This instance's owner id. Persisted under the Caddy data dir so it is +// STABLE across container/process restarts (the prod container always uses +// the same /data/caddy volume). If the file is missing it is created; if it +// cannot be persisted (e.g. a foreign process without write access to the +// dir) an ephemeral id is used — which, being different from prod's persisted +// id, makes that process refuse to clobber prod. Memoised per process. +function getOwnerId() { + if (_cachedOwnerId) return _cachedOwnerId; + + const file = path.join(_ownerDataDir(), OWNER_FILE); + try { + const existing = fs.readFileSync(file, 'utf8').trim(); + if (existing) { + _cachedOwnerId = existing; + return _cachedOwnerId; + } + } catch { /* missing / unreadable → create below */ } + + const id = crypto.randomBytes(8).toString('hex'); + try { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, id, { mode: 0o600 }); + } catch (err) { + logger.warn({ err: err.message, file }, 'Could not persist Caddy owner id — using ephemeral id'); + } + _cachedOwnerId = id; + return _cachedOwnerId; +} + +// The marker route that stamps ownership into a Caddy config. Appended LAST in +// srv0.routes; the impossible host match means it is never reached. +function ownerMarkerRoute(ownerId) { + return { + '@id': OWNER_ID_PREFIX + ownerId, + match: [{ host: [MARKER_HOST] }], + handle: [{ handler: 'static_response', status_code: 421 }], + terminal: true, + }; +} + +// Walk a routes array (recursing into subroute handlers, mirroring +// caddyReconciler) looking for the owner marker. Returns the bare owner id +// (without prefix) or null. +function _findOwnerInRoutes(routes) { + if (!Array.isArray(routes)) return null; + for (const r of routes) { + const id = r && r['@id']; + if (typeof id === 'string' && id.startsWith(OWNER_ID_PREFIX)) { + return id.slice(OWNER_ID_PREFIX.length); + } + const handlers = r && Array.isArray(r.handle) ? r.handle : []; + for (const h of handlers) { + if (h && Array.isArray(h.routes)) { + const found = _findOwnerInRoutes(h.routes); + if (found) return found; + } + } + } + return null; +} + +// Read the owning instance id from a live Caddy /config/ response, or null if +// the config carries no owner marker (fresh Caddy, or pre-guard version). +function extractOwner(caddyConfig) { + const servers = caddyConfig && caddyConfig.apps + && caddyConfig.apps.http && caddyConfig.apps.http.servers; + if (!servers) return null; + for (const name of Object.keys(servers)) { + const found = _findOwnerInRoutes(servers[name] && servers[name].routes); + if (found) return found; + } + return null; +} + +// Decision for the sync guard: true ⇒ the live Caddy is owned by a DIFFERENT +// instance and must not be overwritten. An unowned/fresh Caddy (null) is +// claimable, so it is NOT foreign. +function isForeignOwner(liveConfig, myOwnerId) { + const liveOwner = extractOwner(liveConfig); + return liveOwner !== null && liveOwner !== myOwnerId; +} + +// Full pre-/load ownership decision, given the live config read result. +// 'read-error' — the live config could NOT be read (a thrown error, not the +// null "Caddy not running" signal). Ownership is unverifiable, +// so fail CLOSED: skip the sync rather than risk clobbering a +// foreign-owned Caddy during a transient read glitch. +// 'foreign' — live config is owned by a different instance → refuse. +// 'proceed' — claimable (null/fresh) or our own → go ahead. +// Pure (no I/O) so the guard is unit-testable despite _syncToCaddyInner's +// NODE_ENV=test early return. +function ownershipDecision(liveConfig, readError, myOwnerId) { + if (readError) return 'read-error'; + if (isForeignOwner(liveConfig, myOwnerId)) return 'foreign'; + return 'proceed'; +} + +function _resetOwnerCache() { + _cachedOwnerId = null; +} + +module.exports = { + OWNER_ID_PREFIX, + getOwnerId, + ownerMarkerRoute, + extractOwner, + isForeignOwner, + ownershipDecision, + _resetOwnerCache, +}; diff --git a/src/services/caddyReconciler.js b/src/services/caddyReconciler.js index 5133a2f3..b4f1f493 100644 --- a/src/services/caddyReconciler.js +++ b/src/services/caddyReconciler.js @@ -73,7 +73,7 @@ function extractCaddyRouteIds(caddyConfig) { if (!servers) return ids; for (const name of Object.keys(servers)) { const srv = servers[name]; - collectRouteIds(srv.routes, ids); + collectRouteIds(srv && srv.routes, ids); } return ids; } @@ -81,7 +81,11 @@ function extractCaddyRouteIds(caddyConfig) { function collectRouteIds(routes, ids) { if (!Array.isArray(routes)) return; for (const r of routes) { - if (r && r['@id'] && typeof r['@id'] === 'string') ids.add(r['@id']); + // Only DB-route markers (gc_route_) count toward parity. The + // gc_owner_ ownership marker (see caddyOwner.js) is intentionally NOT a + // route id — counting it would make every cycle report drift (it is never + // in listDbRouteIds) and trigger an endless auto-repair loop. + if (r && typeof r['@id'] === 'string' && r['@id'].startsWith('gc_route_')) ids.add(r['@id']); const handlers = r && Array.isArray(r.handle) ? r.handle : []; for (const h of handlers) { if (h && Array.isArray(h.routes)) collectRouteIds(h.routes, ids); diff --git a/tests/caddyOwner.test.js b/tests/caddyOwner.test.js new file mode 100644 index 00000000..774a2b8d --- /dev/null +++ b/tests/caddyOwner.test.js @@ -0,0 +1,156 @@ +'use strict'; + +// Unit tests for the Caddy ownership guard (caddyOwner.js). +// +// Context: the deployed container runs network_mode: host, so the Caddy +// admin API on 127.0.0.1:2019 is shared with EVERY process on the host — +// including dev/test runs in .claude worktrees. A foreign process that +// pushes a full config via POST /load overwrites the live production +// config (the 2026-06-25 ERR_SSL_PROTOCOL_ERROR incident). The owner +// guard tags every prod config with an instance id and refuses to /load +// over a Caddy already owned by a DIFFERENT instance. + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const crypto = require('node:crypto'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +process.env.GC_SECRET = process.env.GC_SECRET || crypto.randomBytes(32).toString('hex'); +process.env.GC_ENCRYPTION_KEY = process.env.GC_ENCRYPTION_KEY || crypto.randomBytes(32).toString('hex'); + +const owner = require('../src/services/caddyOwner'); +const { OWNER_ID_PREFIX, ownerMarkerRoute, extractOwner, isForeignOwner, ownershipDecision, getOwnerId, _resetOwnerCache } = owner; + +function configWithRoutes(routes) { + return { apps: { http: { servers: { srv0: { listen: [':443'], routes } } } } }; +} + +test('OWNER_ID_PREFIX is the gc_owner_ namespace (distinct from gc_route_)', () => { + assert.equal(OWNER_ID_PREFIX, 'gc_owner_'); +}); + +test('ownerMarkerRoute carries the prefixed @id and an unreachable host match', () => { + const r = ownerMarkerRoute('deadbeef'); + assert.equal(r['@id'], 'gc_owner_deadbeef'); + // Must match an impossible host so it never serves or shadows a real route. + const hosts = r.match[0].host; + assert.ok(Array.isArray(hosts) && hosts.length === 1); + assert.match(hosts[0], /\.invalid$/); +}); + +test('extractOwner returns null for empty / markerless configs', () => { + assert.equal(extractOwner(null), null); + assert.equal(extractOwner({}), null); + assert.equal(extractOwner(configWithRoutes([])), null); + assert.equal(extractOwner(configWithRoutes([{ '@id': 'gc_route_5', handle: [] }])), null); +}); + +test('extractOwner reads the owner id from a top-level marker route', () => { + const cfg = configWithRoutes([ + { '@id': 'gc_route_1', handle: [] }, + ownerMarkerRoute('abc123'), + ]); + assert.equal(extractOwner(cfg), 'abc123'); +}); + +test('extractOwner recurses into subroute handlers', () => { + const cfg = configWithRoutes([ + { handle: [{ handler: 'subroute', routes: [ownerMarkerRoute('nested99')] }] }, + ]); + assert.equal(extractOwner(cfg), 'nested99'); +}); + +test('isForeignOwner: unowned (fresh) Caddy is claimable', () => { + assert.equal(isForeignOwner(configWithRoutes([]), 'me'), false); +}); + +test('isForeignOwner: my own Caddy is not foreign', () => { + const cfg = configWithRoutes([ownerMarkerRoute('me')]); + assert.equal(isForeignOwner(cfg, 'me'), false); +}); + +test('isForeignOwner: a different instance IS foreign (refuse)', () => { + const cfg = configWithRoutes([ownerMarkerRoute('someone-else')]); + assert.equal(isForeignOwner(cfg, 'me'), true); +}); + +test('getOwnerId persists to GC_CADDY_DATA_DIR and is stable across calls', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gc-owner-')); + const prev = process.env.GC_CADDY_DATA_DIR; + process.env.GC_CADDY_DATA_DIR = dir; + try { + _resetOwnerCache(); + const id1 = getOwnerId(); + assert.ok(id1 && typeof id1 === 'string' && id1.length >= 8); + // Persisted to disk. + const onDisk = fs.readFileSync(path.join(dir, '.caddy-owner'), 'utf8').trim(); + assert.equal(onDisk, id1); + // Memoised — same value without re-reading. + assert.equal(getOwnerId(), id1); + // A fresh process (cache reset) reuses the persisted id. + _resetOwnerCache(); + assert.equal(getOwnerId(), id1); + } finally { + process.env.GC_CADDY_DATA_DIR = prev; + _resetOwnerCache(); + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +test('getOwnerId falls back to an ephemeral id when the dir is unwritable', () => { + const prev = process.env.GC_CADDY_DATA_DIR; + // A path under a file (not a dir) cannot be created → mkdir/write fail. + const file = fs.mkdtempSync(path.join(os.tmpdir(), 'gc-owner-')); + const notADir = path.join(file, 'regular-file'); + fs.writeFileSync(notADir, 'x'); + process.env.GC_CADDY_DATA_DIR = path.join(notADir, 'subdir'); + try { + _resetOwnerCache(); + const id = getOwnerId(); + assert.ok(id && typeof id === 'string' && id.length >= 8, 'ephemeral id still returned'); + } finally { + process.env.GC_CADDY_DATA_DIR = prev; + _resetOwnerCache(); + fs.rmSync(file, { recursive: true, force: true }); + } +}); + +test('ownershipDecision: a read error fails CLOSED (skip), not open', () => { + // The whole point of the guard: if we cannot read the live config, we must + // NOT proceed to /load — otherwise a transient read glitch lets a foreign + // process clobber prod. A read error wins even over a same-owner config. + assert.equal(ownershipDecision(null, new Error('timeout'), 'me'), 'read-error'); + assert.equal(ownershipDecision(configWithRoutes([ownerMarkerRoute('me')]), new Error('boom'), 'me'), 'read-error'); +}); + +test('ownershipDecision: foreign owner → refuse', () => { + assert.equal(ownershipDecision(configWithRoutes([ownerMarkerRoute('other')]), null, 'me'), 'foreign'); +}); + +test('ownershipDecision: claimable (null/fresh) or own → proceed', () => { + assert.equal(ownershipDecision(null, null, 'me'), 'proceed'); // Caddy down + assert.equal(ownershipDecision(configWithRoutes([]), null, 'me'), 'proceed'); // fresh + assert.equal(ownershipDecision(configWithRoutes([ownerMarkerRoute('me')]), null, 'me'), 'proceed'); +}); + +test('two distinct instances (fresh dirs) get different ids — the crux of the guard', () => { + const prev = process.env.GC_CADDY_DATA_DIR; + const dirA = fs.mkdtempSync(path.join(os.tmpdir(), 'gc-ownerA-')); + const dirB = fs.mkdtempSync(path.join(os.tmpdir(), 'gc-ownerB-')); + try { + process.env.GC_CADDY_DATA_DIR = dirA; _resetOwnerCache(); + const idA = getOwnerId(); + process.env.GC_CADDY_DATA_DIR = dirB; _resetOwnerCache(); + const idB = getOwnerId(); + assert.notEqual(idA, idB); + // Instance B, seeing A's marker in the live config, refuses. + assert.equal(isForeignOwner(configWithRoutes([ownerMarkerRoute(idA)]), idB), true); + } finally { + process.env.GC_CADDY_DATA_DIR = prev; + _resetOwnerCache(); + fs.rmSync(dirA, { recursive: true, force: true }); + fs.rmSync(dirB, { recursive: true, force: true }); + } +}); diff --git a/tests/caddyReconciler.test.js b/tests/caddyReconciler.test.js index d7139cc2..1fc72026 100644 --- a/tests/caddyReconciler.test.js +++ b/tests/caddyReconciler.test.js @@ -131,6 +131,27 @@ describe('extractCaddyRouteIds', () => { }; assert.deepEqual([...extractCaddyRouteIds(cfg)], ['gc_route_deep']); }); + + test('ignores the gc_owner_ ownership marker (no drift / repair loop)', () => { + // caddyOwner.js appends a gc_owner_ marker route to srv0. It is NOT a + // DB route and never appears in listDbRouteIds, so if the reconciler + // counted it every cycle would report drift and auto-repair would loop. + const cfg = { + apps: { + http: { + servers: { + srv0: { + routes: [ + { '@id': 'gc_route_1', match: [{ host: ['a'] }] }, + { '@id': 'gc_owner_abc123', match: [{ host: ['gc-owner.invalid'] }] }, + ], + }, + }, + }, + }, + }; + assert.deepEqual([...extractCaddyRouteIds(cfg)], ['gc_route_1']); + }); }); describe('runReconciliationCycle', () => {