[SECURITY AUDIT] Decrypted Discord bot token served from an endpoint on the public API router, guarded only by a shared secret #33

New Issue

wtclaude · 2026-07-04T22:01:30Z

wtclaude commented

2026-07-04 22:01:30 +00:00

Severity: Medium (escalates toward High if the default BOT_INTERNAL_KEY ships or the proxy forwards the path)
Confidence: Medium — depends on reverse-proxy (Pangolin) path filtering that is not visible in this repo
Scope area: Token storage / secret handling (audit areas 4 & 5), auth transition (area 2)

What

GET /internal/bot-config returns the decrypted Discord bot token:

// server/src/router/v1/internal/internal.controller.js:8-17
const config = await botConfig.getWithToken()
return res.json({ enabled: config.enabled, token: config.token, guildId: config.guildId })

This route is mounted inside the same Express app that serves the public API: api.router.js mounts /v1 → v1.router.js:15 mounts /internal → so the full path is /api/v1/internal/bot-config, on the same listener (port 3000) that Pangolin proxies to the world. The only access control is requireInternalKey (server/src/middleware/requireInternalKey.js), a timing-safe compare against BOT_INTERNAL_KEY.

The code comments repeatedly assert this route is "never exposed through the public reverse proxy," but nothing in this repository enforces that — there is no separate port, no bind to the compose-internal interface, and no IP allowlist. The guarantee rests entirely on (a) Pangolin being configured to not forward /api/v1/internal/*, and (b) BOT_INTERNAL_KEY being strong. .env.example ships the placeholder BOT_INTERNAL_KEY=change-me-to-a-long-random-string.

Why it matters

If either assumption fails — the proxy forwards the path, or the key is left at the documented placeholder / a weak value — an unauthenticated remote attacker retrieves the plaintext Discord bot token and can fully control the bot (read channels, post, manage roles/bans depending on its Discord permissions). The whole point of encrypting the token at rest (secretBox, write-only admin API) is undermined by a plaintext read endpoint sharing the public attack surface. By contrast, the bot's own internal API is correctly isolated on an unpublished port (4100); the server's internal route is not.

Where

server/src/router/v1/internal/internal.controller.js:8-17 (returns decrypted token)
server/src/router/v1/v1.router.js:15 (mounted under the public /api/v1)
server/src/middleware/requireInternalKey.js (sole guard)
.env.example / server/.env.example (BOT_INTERNAL_KEY placeholder default)

Suggested fix (not implemented)

Move server↔bot internal traffic off the public listener: bind the /internal routes to a second Express server on a compose-internal-only port (mirroring how the bot exposes 4100 unpublished), or add an allowlist restricting /internal/* to the compose network source range. At minimum, fail startup in production if BOT_INTERNAL_KEY is unset or equal to the placeholder, and document an explicit proxy deny rule for /api/v1/internal. Defense in depth: even a strong shared secret shouldn't be the only thing between the open internet and a plaintext credential.

**Severity:** Medium (escalates toward High if the default `BOT_INTERNAL_KEY` ships or the proxy forwards the path) **Confidence:** Medium — depends on reverse-proxy (Pangolin) path filtering that is not visible in this repo **Scope area:** Token storage / secret handling (audit areas 4 & 5), auth transition (area 2) ### What `GET /internal/bot-config` returns the **decrypted** Discord bot token: ```js // server/src/router/v1/internal/internal.controller.js:8-17 const config = await botConfig.getWithToken() return res.json({ enabled: config.enabled, token: config.token, guildId: config.guildId }) ``` This route is mounted inside the same Express app that serves the public API: `api.router.js` mounts `/v1` → `v1.router.js:15` mounts `/internal` → so the full path is `/api/v1/internal/bot-config`, on the same listener (port 3000) that Pangolin proxies to the world. The **only** access control is `requireInternalKey` (`server/src/middleware/requireInternalKey.js`), a timing-safe compare against `BOT_INTERNAL_KEY`. The code comments repeatedly assert this route is "never exposed through the public reverse proxy," but nothing in this repository enforces that — there is no separate port, no bind to the compose-internal interface, and no IP allowlist. The guarantee rests entirely on (a) Pangolin being configured to not forward `/api/v1/internal/*`, and (b) `BOT_INTERNAL_KEY` being strong. `.env.example` ships the placeholder `BOT_INTERNAL_KEY=change-me-to-a-long-random-string`. ### Why it matters If either assumption fails — the proxy forwards the path, or the key is left at the documented placeholder / a weak value — an unauthenticated remote attacker retrieves the plaintext Discord bot token and can fully control the bot (read channels, post, manage roles/bans depending on its Discord permissions). The whole point of encrypting the token at rest (`secretBox`, write-only admin API) is undermined by a plaintext read endpoint sharing the public attack surface. By contrast, the *bot's* own internal API is correctly isolated on an unpublished port (4100); the server's internal route is not. ### Where - `server/src/router/v1/internal/internal.controller.js:8-17` (returns decrypted token) - `server/src/router/v1/v1.router.js:15` (mounted under the public `/api/v1`) - `server/src/middleware/requireInternalKey.js` (sole guard) - `.env.example` / `server/.env.example` (`BOT_INTERNAL_KEY` placeholder default) ### Suggested fix (not implemented) Move server↔bot internal traffic off the public listener: bind the `/internal` routes to a second Express server on a compose-internal-only port (mirroring how the bot exposes 4100 unpublished), or add an allowlist restricting `/internal/*` to the compose network source range. At minimum, fail startup in production if `BOT_INTERNAL_KEY` is unset or equal to the placeholder, and document an explicit proxy deny rule for `/api/v1/internal`. Defense in depth: even a strong shared secret shouldn't be the only thing between the open internet and a plaintext credential.

wtclaude added the

severity:medium

label 2026-07-04 22:01:30 +00:00

wtclaude referenced this issue from a commit

2026-07-04 22:35:16 +00:00

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

wtclaude referenced a pull request that will close this issue

2026-07-04 22:37:21 +00:00

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

whitlocktech closed this issue

2026-07-04 22:50:08 +00:00

whitlocktech referenced this issue from a commit