Require the hook-event secret unconditionally, not only under a managed tunnel

COD-54 gated the /api/hook-event + /api/status-telemetry localhost bypass
behind the shared X-Codeman-Hook-Secret only WHILE a managed tunnel was
running, keeping a plain localhost bypass otherwise. But Codeman can't detect
a user's OWN loopback reverse proxy (their own `cloudflared --url`,
`tailscale serve`, nginx -> 127.0.0.1), which proxies internet traffic into
the loopback origin with req.ip === 127.0.0.1 — so that setup kept the unsafe
plain bypass.

Require the secret on the loopback bypass unconditionally. Managed-session
hooks already always present it (X-Codeman-Hook-Secret from
$CODEMAN_HOOK_SECRET_FILE, generated for every instance), so the legitimate
hook channel is unaffected; only the previously-unguarded own-proxy path is
now rejected. Drops the now-unused getTunnelRunning param from
registerAuthMiddleware.

Tests: cod54-hook-event-auth (tunnel-down now also requires the secret, plus
a good-secret positive case); auth-security (hook tests present the secret to
reach schema validation).
This commit is contained in:
Aamer Akhter
2026-06-14 12:46:58 -04:00
parent e742d00c98
commit f0f43ddbad
4 changed files with 37 additions and 39 deletions
+14 -26
View File
@@ -36,20 +36,12 @@ interface AuthState {
* Register HTTP Basic Auth middleware with session cookies and rate limiting.
* Only active when CODEMAN_PASSWORD is set.
*
* @param getTunnelRunning - returns true while a managed tunnel is active. Used
* to gate the `/api/hook-event` localhost bypass: when a tunnel is up, tunneled
* internet traffic reaches the loopback origin with `req.ip === 127.0.0.1`, so
* the bypass additionally requires the shared hook secret (COD-54). When no
* tunnel is running (loopback-only, the normal case) the plain localhost bypass
* is kept so already-deployed (pre-secret) hooks + the loop channel keep working.
* Optional; defaults to "no tunnel" (unchanged behavior) when omitted.
* The `/api/hook-event` + `/api/status-telemetry` localhost bypass requires the
* shared hook secret unconditionally (COD-91) — see the onRequest hook below.
*
* @returns AuthState for lifecycle management (dispose on server stop)
*/
export function registerAuthMiddleware(
app: FastifyInstance,
https: boolean,
getTunnelRunning: () => boolean = () => false
): AuthState {
export function registerAuthMiddleware(app: FastifyInstance, https: boolean): AuthState {
const state: AuthState = {
authSessions: null,
authFailures: null,
@@ -114,30 +106,26 @@ export function registerAuthMiddleware(
// COD-54: the bare localhost bypass is unsafe while a tunnel is running, because
// `cloudflared --url http://127.0.0.1:port` proxies internet traffic INTO the
// loopback origin, so a tunneled request arrives with req.ip === 127.0.0.1 and
// would pass. So:
// - tunnel running → bypass requires the shared hook secret (local hooks present
// it via the X-Codeman-Hook-Secret header; internet traffic can't know it),
// - tunnel not running (loopback-only, the normal case) → keep the plain
// localhost bypass so already-deployed (pre-secret) hooks + the loop's own
// credential-less hook channel keep working.
// would pass. COD-91: require the shared hook secret on the loopback bypass
// UNCONDITIONALLY (not just while the managed tunnel is up). Codeman can't detect
// a user's own loopback reverse proxy (their own `cloudflared --url`, `tailscale
// serve`, nginx → 127.0.0.1), so tunnel-gating left that path with the unsafe plain
// bypass. Managed-session hooks always present the secret (X-Codeman-Hook-Secret,
// from $CODEMAN_HOOK_SECRET_FILE — generated for every instance), so requiring it
// always closes the gap without breaking the legitimate hook channel.
if ((req.url === '/api/hook-event' || req.url === '/api/status-telemetry') && req.method === 'POST') {
const ip = req.ip;
const isLoopback = ip === '127.0.0.1' || ip === '::1' || ip === '::ffff:127.0.0.1';
if (isLoopback) {
if (!getTunnelRunning()) {
// Loopback-only: unchanged behavior.
done();
return;
}
// Tunnel up: require the shared secret (constant-time compare).
// Always require the shared secret (constant-time compare).
const presented = Buffer.from(req.headers[HOOK_SECRET_HEADER.toLowerCase()]?.toString() ?? '');
const expected = Buffer.from(getHookSecret());
if (presented.length === expected.length && timingSafeEqual(presented, expected)) {
done();
return;
}
// Wrong/absent secret while tunneled — rate-limit per IP in the DEDICATED
// hook bucket (never authFailures, which would lock out the login path).
// Wrong/absent secret — rate-limit per IP in the DEDICATED hook bucket
// (never authFailures, which would lock out the login path).
const hookIp = req.ip;
const hookFailures = hookSecretFailures.get(hookIp) ?? 0;
if (hookFailures >= AUTH_FAILURE_MAX) {
+1 -1
View File
@@ -630,7 +630,7 @@ export class WebServer extends EventEmitter {
registerHostGuard(this.app, () => this.getHostPolicy());
// Auth middleware (Basic Auth + session cookies + rate limiting)
const authState = registerAuthMiddleware(this.app, this.https, () => this.tunnelManager.isRunning());
const authState = registerAuthMiddleware(this.app, this.https);
if (authState) {
this.authSessions = authState.authSessions;
this.authFailures = authState.authFailures;