diff --git a/tests/config-env-adapter.test.mjs b/tests/config-env-adapter.test.mjs index e931dca..726e8ee 100644 --- a/tests/config-env-adapter.test.mjs +++ b/tests/config-env-adapter.test.mjs @@ -17,20 +17,41 @@ * - "all settings flow through the adapter" → the committed * `apps/server/src/index.ts` imports `loadConfigFromEnv` from * `@personal-blog/config` and loads its startup configuration through it - * (binding `config.port`, taking `config.databaseUrl`, never reading - * `process.env` itself); the adapter maps `HOST`/`PORT`/`DATABASE_URL`/ - * `EPPP_SESSION_SECRET` onto the validated config shape and validates it - * with `assertValidConfig` (E00-S04-T02) before returning. Locked in - * statically and behaviorally: the deterministic boundary probe exercises - * the compiled adapter exactly as the server consumes it (full env, - * defaults, bad-`PORT` fallback, missing required secret → - * `MissingRequiredSettingError` naming `sessionSecret`, empty - * `DATABASE_URL` → `ConfigStartupError`, and the no-argument call reading - * the real `process.env`), and the server-boot probes execute the issue's - * test plan against the committed server (a `PORT`/`HOST` override shows - * up in the resolved-configuration log — the settings really flow through - * the adapter — and a missing required secret still fails startup naming - * the missing field). + * (binding `config.port`/`config.host`, taking `config.databaseUrl`, never + * reading `process.env` itself); the adapter maps `HOST`/`PORT`/ + * `DATABASE_URL`/`EPPP_SESSION_SECRET` onto the validated config shape + * and validates it with `assertValidConfig` (E00-S04-T02) before + * returning. Locked in statically and behaviorally: the deterministic + * boundary probe exercises the compiled adapter exactly as the server + * consumes it (full env, defaults, bad-`PORT` fallback, valid + * `HOST` hostname/IP forms, invalid `HOST` → `ConfigStartupError` naming + * `host`, missing required secret → `MissingRequiredSettingError` naming + * `sessionSecret`, empty `DATABASE_URL` → `ConfigStartupError`, and the + * no-argument call reading the real `process.env`), and the server-boot + * probes execute the issue's test plan against the committed server (a + * `PORT`/`HOST` override shows up in the resolved-configuration log — the + * settings really flow through the adapter; a missing required secret + * still fails startup naming the missing field; `HOST=127.0.0.1` binds + * loopback only, not all interfaces, and the startup log reflects the + * actual bind; an invalid `HOST` fails startup naming the field without + * ever echoing the raw value). + * - "the HOST setting controls the actual bind interface" → the committed + * server passes `config.host` to `server.listen(config.port, + * config.host, ...)`, so a configured `HOST` binds exactly that interface + * and the startup log never claims a bind the process does not enforce. + * Locked in statically (the server source must pass `config.host` to + * `server.listen`) and by the boot probes (with `HOST=127.0.0.1` the + * server answers on loopback and does not answer on a non-loopback + * interface; the startup log shows `http://127.0.0.1:`). + * - "HOST is validated at the adapter boundary as a hostname or IP address + * before it is used for binding or logged" → the adapter resolves `HOST` + * with `resolveHost`, which accepts hostnames (RFC 1123) and IPv4/IPv6 + * addresses and throws a field-specific `ConfigStartupError` naming + * `host` for anything else — so arbitrary env content is never echoed + * verbatim into logs. Locked in by the deterministic boundary probe + * (valid forms pass, invalid forms throw naming `host`) and by a boot + * probe (an invalid `HOST` exits non-zero naming the field, and the raw + * value never appears in the process output). * * Run: `node --test tests/config-env-adapter.test.mjs` * (node:test — built into Node >= 18; no dependencies, lockfile untouched. @@ -45,7 +66,7 @@ import { readFileSync, readdirSync, writeFileSync, cpSync, mkdtempSync, rmSync, import { readdir } from 'node:fs/promises'; import { spawn, spawnSync } from 'node:child_process'; import { once } from 'node:events'; -import { createServer as createNetServer } from 'node:net'; +import { createServer as createNetServer, connect as netConnect } from 'node:net'; import os from 'node:os'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -324,7 +345,7 @@ function injectEnvRead(rootDir, relPath, line) { function assertAdapterSource(src) { assert.match( src, - /import \{ assertValidConfig \} from '\.\/startup\.js'/, + /import \{ assertValidConfig(?:, ConfigStartupError)? \} from '\.\/startup\.js'/, 'the adapter must validate the mapped environment with assertValidConfig (the E00-S04-T02 startup validation)', ); assert.match( @@ -337,11 +358,26 @@ function assertAdapterSource(src) { /export function loadConfigFromEnv\(env: NodeJS\.ProcessEnv = process\.env\): Config/, 'the adapter must export loadConfigFromEnv reading process.env by default (the workspace\'s single owner of process.env reads)', ); + assert.match( + src, + /import \{ isIP \} from 'node:net'/, + 'the adapter must validate HOST as an IP address via node:net isIP', + ); assert.match( src, /env\.HOST/, 'the adapter must map HOST onto the validated config (host)', ); + assert.match( + src, + /resolveHost\(env\.HOST\)/, + 'the adapter must map HOST onto the validated config (host, resolved by resolveHost)', + ); + assert.match( + src, + /function resolveHost\(raw: string \| undefined\): string/, + 'the adapter must own the HOST resolution (resolveHost, validating hostname/IP at the adapter boundary)', + ); assert.match( src, /resolvePort\(env\.PORT\)/, @@ -389,8 +425,8 @@ function assertServerSource(src) { ); assert.match( src, - /server\.listen\(config\.port/, - 'the server must bind the port from the validated configuration (config.port)', + /server\.listen\(config\.port, config\.host/, + 'the server must pass the validated bind interface to server.listen (config.host), so a configured HOST binds exactly that interface', ); assert.match( src, @@ -568,6 +604,20 @@ test('the adapter not validating the mapped environment fails the validation ass assert.throws(() => assertAdapterSource(noValidation), /must validate the mapped environment/); }); +test('the adapter mapping HOST without resolveHost fails the HOST-validation assertion (mutation probe)', () => { + const src = read(ADAPTER_REL); + const noResolve = src.replace('host: resolveHost(env.HOST),', "host: env.HOST ?? '0.0.0.0',"); + assert.notEqual(noResolve, src, 'the mutation must actually bypass the resolveHost validation'); + assert.throws(() => assertAdapterSource(noResolve), /resolveHost/); +}); + +test('the server binding without config.host fails the bind-interface assertion (mutation probe)', () => { + const src = read(SERVER_SRC); + const noHost = src.replace('server.listen(config.port, config.host,', 'server.listen(config.port,'); + assert.notEqual(noHost, src, 'the mutation must actually drop config.host from server.listen'); + assert.throws(() => assertServerSource(noHost), /must pass the validated bind interface/); +}); + test('dropping the adapter export from the package boundary fails the boundary assertion (mutation probe)', () => { const src = read(INDEX_SRC); const dropped = src.replace("export { loadConfigFromEnv } from './env.js';", ''); @@ -625,8 +675,10 @@ function existsSyncProbe(relPath) { * environment adapter (`loadConfigFromEnv`) exactly as the server consumes * it — a full environment maps onto the validated config, missing settings * fall back to the committed defaults (host 0.0.0.0, port 3000, no - * databaseUrl), a bad `PORT` falls back to 3000, a missing required secret - * throws `MissingRequiredSettingError` naming the field, an empty + * databaseUrl), a bad `PORT` falls back to 3000, valid `HOST` forms + * (hostnames and IPv4/IPv6) pass while an invalid `HOST` throws a + * field-specific `ConfigStartupError` naming `host`, a missing required + * secret throws `MissingRequiredSettingError` naming the field, an empty * `DATABASE_URL` throws `ConfigStartupError`, and the no-argument call reads * the real `process.env`. Written to a temp file inside `packages/config/` so * `ajv`/`@sinclair/typebox` resolve through the package's own dependency @@ -671,6 +723,21 @@ const highPort = loadConfigFromEnv({ PORT: '65536', EPPP_SESSION_SECRET: SECRET const maxPort = loadConfigFromEnv({ PORT: '65535', EPPP_SESSION_SECRET: SECRET }).port; const minPort = loadConfigFromEnv({ PORT: '1', EPPP_SESSION_SECRET: SECRET }).port; +// Valid HOST forms pass the adapter-boundary validation: IPv4, IPv6, +// single-label and dotted hostnames (RFC 1123). +const hostForms = [ + '127.0.0.1', '0.0.0.0', '10.0.0.7', + '::1', '::', 'fe80::1', + 'localhost', 'db', 'api.internal.example', +].map((h) => [h, loadConfigFromEnv({ HOST: h, EPPP_SESSION_SECRET: SECRET }).host]); + +// An invalid HOST fails at the adapter boundary: a field-specific startup +// error naming host, so arbitrary env content is never used for binding or +// echoed verbatim into logs. +const invalidHost = capture(() => loadConfigFromEnv({ HOST: 'not a host!', EPPP_SESSION_SECRET: SECRET })); +const invalidHostPort = capture(() => loadConfigFromEnv({ HOST: '127.0.0.1:3000', EPPP_SESSION_SECRET: SECRET })); +const invalidHostDash = capture(() => loadConfigFromEnv({ HOST: '-bad', EPPP_SESSION_SECRET: SECRET })); + // A missing required secret is a field-specific startup error naming it. const missingSecret = capture(() => loadConfigFromEnv({})); @@ -687,6 +754,8 @@ const result = { full: { host: full.host, port: full.port, databaseUrl: full.databaseUrl, sessionSecret: full.sessionSecret }, defaults: { host: defaults.host, port: defaults.port, hasDatabaseUrl: defaults.databaseUrl !== undefined }, badPort, zeroPort, highPort, maxPort, minPort, + hostForms, + invalidHost, invalidHostPort, invalidHostDash, missingSecret, emptyDb, fromRealEnv: { port: fromRealEnv.port, sessionSecret: fromRealEnv.sessionSecret }, @@ -734,6 +803,36 @@ test('the compiled environment adapter maps the environment onto the validated c assert.equal(result.maxPort, 65535, 'PORT 65535 is the upper edge of the valid range'); assert.equal(result.minPort, 1, 'PORT 1 is the lower edge of the valid range'); + // Valid HOST forms pass the adapter-boundary validation unchanged. + assert.deepEqual( + result.hostForms, + [ + ['127.0.0.1', '127.0.0.1'], + ['0.0.0.0', '0.0.0.0'], + ['10.0.0.7', '10.0.0.7'], + ['::1', '::1'], + ['::', '::'], + ['fe80::1', 'fe80::1'], + ['localhost', 'localhost'], + ['db', 'db'], + ['api.internal.example', 'api.internal.example'], + ], + `valid HOST values (IPv4/IPv6/hostname) must pass through the adapter (got: ${JSON.stringify(result.hostForms)})`, + ); + + // An invalid HOST fails at the adapter boundary with a field-specific + // startup error naming host (so arbitrary env content is never used for + // binding or echoed verbatim into logs). + for (const invalid of [result.invalidHost, result.invalidHostPort, result.invalidHostDash]) { + assert.equal(invalid.threw, true, `an invalid HOST must throw (got: ${JSON.stringify(invalid)})`); + assert.equal(invalid.isConfigStartup, true, `an invalid HOST must throw ConfigStartupError (got: ${JSON.stringify(invalid)})`); + assert.match( + invalid.message, + /host/, + `the startup error must name the violating field host (got: ${JSON.stringify(invalid)})`, + ); + } + // A missing required secret is a field-specific startup error naming it. assert.equal(result.missingSecret.threw, true, 'a config missing the required secret must throw'); assert.equal(result.missingSecret.isMissingRequired, true, 'a missing required setting must throw MissingRequiredSettingError'); @@ -870,6 +969,49 @@ async function stopChild(child) { if (child.exitCode === null && child.signalCode === null) child.kill('SIGKILL'); } +/** Waits until the captured log output matches `pattern` (or the deadline passes). */ +async function waitForLog(readOutput, pattern, deadlineMs = 5_000) { + const deadline = Date.now() + deadlineMs; + while (Date.now() < deadline) { + if (pattern.test(readOutput())) return true; + await delay(100); + } + return false; +} + +/** A non-internal IPv4 address of this host, or undefined when none exists. */ +function nonLoopbackIpv4() { + for (const addrs of Object.values(os.networkInterfaces())) { + for (const addr of addrs ?? []) { + if (addr.family === 'IPv4' && !addr.internal) return addr.address; + } + } + return undefined; +} + +/** + * Attempts a TCP connection; resolves true when it succeeds. A refused + * connection or a timeout (a dropped SYN) both resolve false — the only way + * this resolves true is a listening socket on that interface, which is + * exactly what the loopback-only probe must rule out. + */ +function canConnect(host, port, timeoutMs = 1_500) { + return new Promise((resolve) => { + const socket = netConnect({ host, port }); + let settled = false; + const finish = (ok) => { + if (settled) return; + settled = true; + socket.destroy(); + resolve(ok); + }; + socket.setTimeout(timeoutMs); + socket.once('connect', () => finish(true)); + socket.once('timeout', () => finish(false)); + socket.once('error', () => finish(false)); + }); +} + test('a PORT/HOST override flows through the adapter into the resolved configuration (server boot probe)', { skip: !TS_STRIPPING || !CONFIG_DIST || !DATABASE_POSTGRES_DIST ? BUILD_HINT : false }, async () => { // "All settings flow through the adapter": booting the committed server // with PORT/HOST overrides must surface those values in the resolved @@ -928,3 +1070,73 @@ test('starting without the required secret exits non-zero naming the missing fie `the startup error must name the missing field (sessionSecret); got: ${stdout().trim()} ${stderr().trim()}`, ); }); + +test('a configured HOST binds exactly that interface: HOST=127.0.0.1 answers on loopback only, not all interfaces (server boot probe)', { skip: !TS_STRIPPING || !CONFIG_DIST || !DATABASE_POSTGRES_DIST ? BUILD_HINT : false }, async (t) => { + // The issue's test plan: "boot with HOST=127.0.0.1 and confirm the server + // binds loopback only, not all interfaces", and "startup log reflects the + // actual bind interface". The server passes config.host to server.listen, + // so with HOST=127.0.0.1 the server answers on loopback and does NOT listen + // on a non-loopback interface, and the startup log shows the loopback bind. + const port = await reservePort(); + const { child, stdout, stderr } = bootServer(port, { + EPPP_SESSION_SECRET: SECRET, + HOST: '127.0.0.1', + }); + try { + const response = await waitForAnswer(port, child, stderr); + assert.equal( + response.status, + 200, + `GET /health on loopback must answer 200 with HOST=127.0.0.1 (got ${response.status}); server output: ${stdout().trim()} ${stderr().trim()}`, + ); + // The startup log reflects the actual bind interface. + assert.ok( + await waitForLog(stdout, new RegExp(`http://127\\.0\\.0\\.1:${port}`)), + `the startup log must show the loopback bind (http://127.0.0.1:${port}); got: ${stdout().trim()}`, + ); + // Loopback-only: no non-loopback interface may accept the connection. + const external = nonLoopbackIpv4(); + if (external === undefined) { + t.skip('this host has no non-loopback IPv4 interface; loopback-only binding is trivially satisfied'); + return; + } + assert.equal( + await canConnect(external, port), + false, + `with HOST=127.0.0.1 the server must not listen on the non-loopback interface ${external}:${port} (it would bind all interfaces)`, + ); + assert.equal(await canConnect('127.0.0.1', port), true, 'the loopback interface must still accept connections'); + } finally { + await stopChild(child); + } +}); + +test('an invalid HOST fails startup naming the field and never echoes the raw value (server boot probe)', { skip: !TS_STRIPPING || !CONFIG_DIST || !DATABASE_POSTGRES_DIST ? BUILD_HINT : false }, async () => { + // "HOST is validated at the adapter boundary as a hostname or IP address + // before it is used for binding or logged, so arbitrary env content is + // never echoed verbatim into logs": booting with an invalid HOST must exit + // non-zero with a field-specific startup error naming host — and the raw + // invalid value must never appear in the process output. + const port = await reservePort(); + const invalidHost = 'bogus host!'; + const { child, stdout, stderr } = bootServer(port, { + EPPP_SESSION_SECRET: SECRET, + HOST: invalidHost, + }); + const { code, signal } = await waitForExit(child); + const output = stdout() + stderr(); + assert.notEqual( + code, + 0, + `the server must exit non-zero when HOST is invalid (code ${code}, signal ${signal}); output: ${output.trim()}`, + ); + assert.match( + output, + /host/, + `the startup error must name the invalid field (host); got: ${output.trim()}`, + ); + assert.ok( + !output.includes(invalidHost), + `the raw invalid HOST value must never be echoed verbatim into the process output; got: ${output.trim()}`, + ); +}); diff --git a/tests/config-startup-error.test.mjs b/tests/config-startup-error.test.mjs index 03d6d15..65a395b 100644 --- a/tests/config-startup-error.test.mjs +++ b/tests/config-startup-error.test.mjs @@ -291,8 +291,8 @@ test('moving the adapter call after the server binds fails the order assertion ( const moved = src .replace('const config = loadConfigFromEnv();\n', '') .replace( - 'server.listen(config.port, () => {', - 'server.listen(config.port, () => {\n const config = loadConfigFromEnv();', + 'server.listen(config.port, config.host, () => {', + 'server.listen(config.port, config.host, () => {\n const config = loadConfigFromEnv();', ); assert.notEqual(moved, src, 'the mutation must actually move the adapter call after the bind'); assert.throws(() => assertServerStartupValidation(moved), /before the server binds/);