Isolate internal bot-config route from the public listener (#33)

The GET /internal/bot-config route returns the DECRYPTED Discord bot
token and was mounted on the same Express app / port 3000 that Pangolin
proxies publicly. Its only guard was the BOT_INTERNAL_KEY shared secret,
and .env.example shipped a placeholder default — so a forwarded path or a
weak/unrotated key would expose the plaintext token to the internet.

Move server<->bot internal traffic onto its own listener and fail fast on
a weak key:

- Add server/src/internalApp.js: a standalone Express app mounting
  requireInternalKey + /internal (and a no-secret /health), mirroring the
  bot's unpublished port-4100 pattern.
- server.js starts a second listener on INTERNAL_PORT (default 3001),
  closed on graceful shutdown.
- Remove the /internal mount from the public v1.router; the public app now
  404s /api/v1/internal/bot-config even with a valid key.
- Fail fast: new utils/botInternalKey.js rejects an empty, placeholder, or
  <16-char BOT_INTERNAL_KEY — fatal in production (exit 1), warning in dev.
- docker-compose: bot SITE_INTERNAL_URL -> app:3001/internal/bot-config;
  document that INTERNAL_PORT stays unpublished.
- .env.example (root/server/bot): document INTERNAL_PORT, the fail-fast
  behavior, and a defense-in-depth Pangolin deny rule for /api/v1/internal.

Tests: add requireInternalKey.test.js and botInternalKey.test.js
(node --test: 93 pass).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019rao86n5cXpwAyjdBFEshV

server/.env.example

@@ -4,6 +4,10 @@
 
 NODE_ENV=development
 PORT=3000
+# Separate, unpublished port for server<->bot internal traffic (the decrypted
+# bot-token route). Must match the port in bot/.env's SITE_INTERNAL_URL and must
+# never be exposed through a public reverse proxy. See issue #33.
+INTERNAL_PORT=3001
 # Logging — written to BOTH the console and a log file (default <server>/logs/app.log).
 LOG_LEVEL=debug         # console verbosity: error | warn | info | debug
 FILE_LOG_LEVEL=debug    # file verbosity
@@ -77,8 +81,11 @@ CLIENT_ORIGIN=http://localhost:5173
 # Discord bot — internal API (server <-> bot/). BOT_INTERNAL_KEY MUST be
 # byte-for-byte identical to the same variable in bot/.env.example — it is the
 # only auth on both sides' /internal/* routes, so a mismatch silently breaks
-# every server<->bot call with 401s. The Discord bot TOKEN itself is not an env
-# var — it's entered in the admin panel (Discord Bot page) and stored
+# every server<->bot call with 401s. It also guards the server's
+# /internal/bot-config route, which returns the DECRYPTED Discord token: with
+# NODE_ENV=production the app REFUSES TO START if this is blank, a documented
+# placeholder, or shorter than 16 chars (a warning only in dev). The Discord bot
+# TOKEN itself is not an env var — it's entered in the admin panel and stored
 # encrypted in the DB (see the bot_config table / SECRET_ENC_KEY above).
 BOT_INTERNAL_URL=http://localhost:4100
 BOT_INTERNAL_KEY=dev-only-change-me-bot-key

server/src/internalApp.js

@@ -0,0 +1,26 @@
+// Standalone Express app for server<->bot internal traffic. It is mounted on its
+// OWN http listener (INTERNAL_PORT, default 3001) in server.js — an unpublished,
+// compose-network-only port, mirroring how the bot exposes its internal API on
+// 4100. Crucially it is NOT part of the public API app (app.js), so /internal/*
+// (which returns the DECRYPTED Discord bot token) can never ride the same
+// listener Pangolin proxies to the world. Shared-secret gated by
+// requireInternalKey inside internal.routes. See issue #33.
+const express = require('express')
+
+const internalRouter = require('./router/v1/internal/internal.routes')
+
+const internalApp = express()
+
+internalApp.use(express.json())
+
+// Liveness probe for this listener (no secret required); mirrors the bot's
+// /health on 4100. Useful for compose healthchecks without exposing anything.
+internalApp.get('/health', (req, res) => res.json({ status: 'ok' }))
+
+// requireInternalKey is applied inside internal.routes.
+internalApp.use('/internal', internalRouter)
+
+// Anything else on this listener is not a real internal route.
+internalApp.use((req, res) => res.status(404).json({ message: 'Not found' }))
+
+module.exports = internalApp

server/src/router/v1/internal/internal.routes.js

@@ -6,7 +6,8 @@ const ctrl = require('./internal.controller')
 const router = express.Router()
 
 // Shared-secret gated, not session-gated — the caller is the bot process, not
-// a logged-in browser. Mounted before any auth/session middleware in v1.router.
+// a logged-in browser. This router is mounted on the standalone internalApp
+// (its own unpublished port), never on the public /api app. See internalApp.js.
 router.use(requireInternalKey)
 
 router.get(

server/src/router/v1/v1.router.js

@@ -5,13 +5,13 @@ const v1Router = express.Router()
 const authRouter = require('./auth/auth.routes')
 const publicRouter = require('./public/public.routes')
 const adminRouter = require('./admin/admin.routes')
-const internalRouter = require('./internal/internal.routes')
 
 v1Router.use('/auth', authRouter)
 v1Router.use('/public', publicRouter)
 v1Router.use('/admin', adminRouter)
-// Shared-secret gated (not session-gated) — server<->bot only, never exposed
-// through the public reverse proxy. See server/src/middleware/requireInternalKey.js.
-v1Router.use('/internal', internalRouter)
+// NOTE: /internal is intentionally NOT mounted here. Those routes return the
+// decrypted Discord bot token and must never share the public listener that
+// Pangolin proxies. They live on a separate, unpublished port via
+// server/src/internalApp.js (started in server.js). See issue #33.
 
 module.exports = v1Router

server/src/server.js

@@ -2,16 +2,23 @@ require('dotenv').config()
 const http = require('http')
 
 const app = require('./app')
+const internalApp = require('./internalApp')
 const botScore = require('./middleware/botScore')
 const { ensureSchema, close } = require('./utils/db')
 const { seedDefaults, createInitialAdminFromEnv } = require('../db/seed')
 const settings = require('./model/settings/settings.model')
 const mailer = require('./utils/mailer')
 const createLogger = require('./utils/logger')
+const { evaluateBotInternalKey } = require('./utils/botInternalKey')
 const pkg = require('../package.json')
 
 const log = createLogger('server')
 const PORT = Number(process.env.PORT) || 3000
+// Separate, UNPUBLISHED listener for server<->bot /internal/* traffic. Kept off
+// the public PORT so the decrypted-token route can't ride the listener Pangolin
+// proxies to the world (issue #33). Must match the port in the bot's
+// SITE_INTERNAL_URL (docker-compose.yml).
+const INTERNAL_PORT = Number(process.env.INTERNAL_PORT) || 3001
 const HOST = '0.0.0.0' // bind all interfaces so Pangolin / the LAN can reach it
 
 async function start() {
@@ -25,6 +32,20 @@ async function start() {
     smtp: mailer.isConfigured() ? 'configured' : 'not configured (mailto fallback)',
   })
 
+  // Fail fast if the server<->bot shared secret is weak/placeholder. Fatal in
+  // production (the /internal/bot-config route hands back the decrypted Discord
+  // token and this key is its only guard); a warning otherwise.
+  const keyCheck = evaluateBotInternalKey({
+    key: process.env.BOT_INTERNAL_KEY,
+    nodeEnv: process.env.NODE_ENV,
+  })
+  if (keyCheck.fatal) {
+    log.error(keyCheck.message)
+    process.exit(1)
+  } else if (!keyCheck.ok) {
+    log.warn(keyCheck.message)
+  }
+
   log.info('ensuring database schema...')
   await ensureSchema()
   log.info('seeding defaults...')
@@ -39,10 +60,18 @@ async function start() {
     log.info(`listening on http://${HOST}:${PORT}  (API at /api/v1, health at /api/health)`)
   })
 
-  setupShutdown(server)
+  // Internal server<->bot API on a separate, unpublished port. NEVER expose this
+  // through Pangolin/the public reverse proxy — it serves the decrypted Discord
+  // bot token to the bot process over the private compose network only (#33).
+  const internalServer = http.createServer(internalApp)
+  internalServer.listen(INTERNAL_PORT, HOST, () => {
+    log.info(`internal API listening on http://${HOST}:${INTERNAL_PORT}  (server<->bot only — do NOT proxy)`)
+  })
+
+  setupShutdown(server, internalServer)
 }
 
-function setupShutdown(server) {
+function setupShutdown(server, internalServer) {
   let closing = false
   const shutdown = async (signal) => {
     if (closing) return
@@ -50,6 +79,7 @@ function setupShutdown(server) {
     log.warn(`${signal} received — shutting down gracefully`)
     botScore.stopSweeper() // stop the bot-store cleanup interval
     server.close(() => log.info('http server closed'))
+    if (internalServer) internalServer.close(() => log.info('internal http server closed'))
     try {
       await close()
       log.info('database pool closed')

server/src/utils/botInternalKey.js

@@ -0,0 +1,43 @@
+// Boot-time validation of BOT_INTERNAL_KEY — the shared secret that is the ONLY
+// auth on the server<->bot /internal/* routes (which return the decrypted Discord
+// bot token). A missing, placeholder, or trivially short key would leave that
+// endpoint effectively unguarded, so in production we refuse to start; in dev we
+// warn but continue so local work isn't blocked. See issue #33.
+
+// The placeholders shipped in the repo's .env.example files. If any of these
+// reaches production it means the operator never generated a real key.
+const PLACEHOLDERS = new Set([
+  'change-me-to-a-long-random-string', // root .env.example
+  'dev-only-change-me-bot-key', // server/.env.example, bot/.env.example
+])
+
+const MIN_LENGTH = 16
+
+// Returns { ok, fatal, message }. `fatal` is only ever true in production —
+// callers should exit non-zero on fatal, and log a warning (but continue) when
+// !ok && !fatal.
+function evaluateBotInternalKey({ key, nodeEnv } = {}) {
+  const value = key || ''
+
+  let reason = null
+  if (value.length === 0) reason = 'BOT_INTERNAL_KEY is not set'
+  else if (PLACEHOLDERS.has(value)) reason = 'BOT_INTERNAL_KEY is still the documented placeholder value'
+  else if (value.length < MIN_LENGTH) reason = `BOT_INTERNAL_KEY is too short (< ${MIN_LENGTH} chars)`
+
+  if (!reason) return { ok: true, fatal: false, message: null }
+
+  const production = nodeEnv === 'production'
+  const detail =
+    `${reason}. It is the only guard on /internal/bot-config, which returns the ` +
+    'decrypted Discord bot token.'
+
+  return {
+    ok: false,
+    fatal: production,
+    message: production
+      ? `${detail} Refusing to start in production — set a long random BOT_INTERNAL_KEY (matching bot/.env).`
+      : `${detail} Continuing because NODE_ENV is not "production" — set a strong value before deploying.`,
+  }
+}
+
+module.exports = { evaluateBotInternalKey, PLACEHOLDERS, MIN_LENGTH }

server/test/botInternalKey.test.js

@@ -0,0 +1,55 @@
+const { test } = require('node:test')
+const assert = require('node:assert/strict')
+
+const { evaluateBotInternalKey, MIN_LENGTH } = require('../src/utils/botInternalKey')
+
+const STRONG = 'x'.repeat(MIN_LENGTH + 8)
+
+test('a strong key is ok in every environment', () => {
+  for (const nodeEnv of ['production', 'development', undefined]) {
+    const r = evaluateBotInternalKey({ key: STRONG, nodeEnv })
+    assert.equal(r.ok, true)
+    assert.equal(r.fatal, false)
+    assert.equal(r.message, null)
+  }
+})
+
+test('empty key is fatal in production, warn otherwise', () => {
+  const prod = evaluateBotInternalKey({ key: '', nodeEnv: 'production' })
+  assert.equal(prod.ok, false)
+  assert.equal(prod.fatal, true)
+
+  const dev = evaluateBotInternalKey({ key: '', nodeEnv: 'development' })
+  assert.equal(dev.ok, false)
+  assert.equal(dev.fatal, false)
+})
+
+test('undefined key behaves like empty', () => {
+  const r = evaluateBotInternalKey({ key: undefined, nodeEnv: 'production' })
+  assert.equal(r.ok, false)
+  assert.equal(r.fatal, true)
+})
+
+test('documented placeholders are rejected (fatal in production)', () => {
+  for (const key of ['change-me-to-a-long-random-string', 'dev-only-change-me-bot-key']) {
+    const r = evaluateBotInternalKey({ key, nodeEnv: 'production' })
+    assert.equal(r.ok, false, `placeholder should be rejected: ${key}`)
+    assert.equal(r.fatal, true)
+  }
+})
+
+test('a too-short key is rejected', () => {
+  const short = 'a'.repeat(MIN_LENGTH - 1)
+  const r = evaluateBotInternalKey({ key: short, nodeEnv: 'production' })
+  assert.equal(r.ok, false)
+  assert.equal(r.fatal, true)
+
+  const dev = evaluateBotInternalKey({ key: short, nodeEnv: 'development' })
+  assert.equal(dev.ok, false)
+  assert.equal(dev.fatal, false)
+})
+
+test('a key exactly MIN_LENGTH long is accepted', () => {
+  const r = evaluateBotInternalKey({ key: 'a'.repeat(MIN_LENGTH), nodeEnv: 'production' })
+  assert.equal(r.ok, true)
+})

server/test/requireInternalKey.test.js

@@ -0,0 +1,66 @@
+const { test } = require('node:test')
+const assert = require('node:assert/strict')
+
+const requireInternalKey = require('../src/middleware/requireInternalKey')
+const { startApp } = require('./_helper')
+
+const KEY = 'test-internal-key-1234567890'
+
+async function withApp(run) {
+  const prev = process.env.BOT_INTERNAL_KEY
+  process.env.BOT_INTERNAL_KEY = KEY
+  const app = await startApp((a) => {
+    a.get('/internal/thing', requireInternalKey, (req, res) => res.json({ ok: true }))
+  })
+  try {
+    await run(app)
+  } finally {
+    await app.close()
+    if (prev === undefined) delete process.env.BOT_INTERNAL_KEY
+    else process.env.BOT_INTERNAL_KEY = prev
+  }
+}
+
+test('requireInternalKey: 401 when no key header is sent', async () => {
+  await withApp(async (app) => {
+    const res = await fetch(`${app.url}/internal/thing`)
+    assert.equal(res.status, 401)
+  })
+})
+
+test('requireInternalKey: 401 on a wrong key', async () => {
+  await withApp(async (app) => {
+    const res = await fetch(`${app.url}/internal/thing`, {
+      headers: { 'X-Internal-Key': 'nope' },
+    })
+    assert.equal(res.status, 401)
+  })
+})
+
+test('requireInternalKey: 200 with the correct key', async () => {
+  await withApp(async (app) => {
+    const res = await fetch(`${app.url}/internal/thing`, {
+      headers: { 'X-Internal-Key': KEY },
+    })
+    assert.equal(res.status, 200)
+    const body = await res.json()
+    assert.deepEqual(body, { ok: true })
+  })
+})
+
+test('requireInternalKey: 401 when the expected key is empty (never a wildcard)', async () => {
+  const prev = process.env.BOT_INTERNAL_KEY
+  process.env.BOT_INTERNAL_KEY = ''
+  const app = await startApp((a) => {
+    a.get('/internal/thing', requireInternalKey, (req, res) => res.json({ ok: true }))
+  })
+  try {
+    // Even sending an empty key must not match an empty expected key.
+    const res = await fetch(`${app.url}/internal/thing`, { headers: { 'X-Internal-Key': '' } })
+    assert.equal(res.status, 401)
+  } finally {
+    await app.close()
+    if (prev === undefined) delete process.env.BOT_INTERNAL_KEY
+    else process.env.BOT_INTERNAL_KEY = prev
+  }
+})