Merge PR #127: require hook-event secret unconditionally + stale-config self-heal

Require the hook-event secret unconditionally (drop managed-tunnel gating)

src/hooks-config.ts

@@ -243,6 +243,43 @@ export async function writeHooksConfig(casePath: string): Promise<void> {
   });
 }
 
+/**
+ * Self-heal a case's hooks block so the COD-91 unconditional hook-secret gate keeps
+ * accepting its hook events.
+ *
+ * `writeHooksConfig` only runs when a case is first CREATED. Cases created before the
+ * X-Codeman-Hook-Secret header was added (COD-54, 2026-06-10) keep hook curls in their
+ * settings.local.json that POST to /api/hook-event WITHOUT the secret — which, once the
+ * gate requires it unconditionally (COD-91), silently 401 on a password-protected
+ * install. This refreshes the hooks block so those stale curls regain the header.
+ *
+ * Deliberately surgical: regenerates ONLY when settings.local.json already contains
+ * Codeman's own hook curls (they target `/api/hook-event`) that lack the secret header.
+ * No-op when the file/hooks are absent (we never impose hooks on a user who removed
+ * them), when the hooks aren't ours, or when the secret is already present — so it never
+ * clobbers a user's customizations and is cheap enough to call on every Claude spawn.
+ */
+export async function refreshStaleHookSecret(casePath: string): Promise<void> {
+  const settingsPath = join(casePath, '.claude', 'settings.local.json');
+  if (!existsSync(settingsPath)) return;
+  await withSettingsLock(settingsPath, async () => {
+    let existing: Record<string, unknown>;
+    try {
+      existing = JSON.parse(await readFile(settingsPath, 'utf-8'));
+    } catch {
+      return; // malformed — leave it untouched (case-create owns the happy path)
+    }
+    const hooksJson = JSON.stringify(existing.hooks ?? null);
+    const isOurs = hooksJson.includes('/api/hook-event');
+    // The generated curl carries this header literal (see generateHooksConfig); its
+    // absence on our own hooks means they predate COD-54 and need regenerating.
+    const hasSecret = hooksJson.includes('X-Codeman-Hook-Secret');
+    if (!isOurs || hasSecret) return;
+    const merged = { ...existing, ...generateHooksConfig() };
+    await writeFile(settingsPath, JSON.stringify(merged, null, 2) + '\n');
+  });
+}
+
 /** Unique marker identifying Codeman's own statusLine command (vs a user's). */
 const STATUSLINE_MARKER = '/api/status-telemetry';
 

src/web/middleware/auth.ts

@@ -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) {

src/web/routes/session-routes.ts

@@ -45,7 +45,13 @@ import {
   validatePathWithinBase,
 } from '../route-helpers.js';
 import { AUTH_COOKIE_NAME } from '../middleware/auth.js';
-import { writeHooksConfig, updateCaseModel, stripCaseEnvKeys, applyStatusLineConfig } from '../../hooks-config.js';
+import {
+  writeHooksConfig,
+  updateCaseModel,
+  stripCaseEnvKeys,
+  applyStatusLineConfig,
+  refreshStaleHookSecret,
+} from '../../hooks-config.js';
 import { generateClaudeMd } from '../../templates/claude-md.js';
 import { imageWatcher } from '../../image-watcher.js';
 import { getLifecycleLog } from '../../session-lifecycle-log.js';
@@ -312,6 +318,13 @@ export function registerSessionRoutes(
       await applyStatusLineConfig(workingDir, true);
     }
 
+    // COD-91 self-heal: refresh a pre-secret hooks block in an existing case so the now
+    // unconditional hook-secret gate keeps accepting its hook events. No-op for fresh
+    // cases (writeHooksConfig already wrote the secret) and for non-Codeman/absent hooks.
+    if ((body.mode ?? 'claude') === 'claude') {
+      await refreshStaleHookSecret(workingDir).catch(() => {});
+    }
+
     // Check OpenCode availability if requested
     if (body.mode === 'opencode') {
       const { isOpenCodeAvailable } = await import('../../utils/opencode-cli-resolver.js');
@@ -1279,6 +1292,11 @@ export function registerSessionRoutes(
       } catch (err) {
         return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `Failed to create case: ${getErrorMessage(err)}`);
       }
+    } else if (mode !== 'opencode') {
+      // COD-91 self-heal for an EXISTING case: refresh a pre-secret hooks block so the
+      // now-unconditional hook-secret gate keeps accepting its hook events. No-op when
+      // the hooks aren't ours or already carry the secret.
+      await refreshStaleHookSecret(casePath).catch(() => {});
     }
 
     // Strip stale disk entries for keys this request is actively setting (Claude only —

src/web/server.ts

@@ -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;

test/auth-security.test.ts

@@ -14,6 +14,7 @@ import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach, vi }
 import { WebServer } from '../src/web/server.js';
 import { TmuxManager } from '../src/tmux-manager.js';
 import { SettingsUpdateSchema } from '../src/web/schemas.js';
+import { getHookSecret, HOOK_SECRET_HEADER } from '../src/config/hook-secret.js';
 
 const AUTH_PORT = 3160;
 const NOAUTH_PORT = 3161;
@@ -250,28 +251,28 @@ describe('Auth Security', () => {
   });
 
   describe('Hook Event Endpoint', () => {
-    it('should allow hook events from localhost without auth', async () => {
+    it('should allow hook events from localhost with the hook secret (no Basic auth)', async () => {
       const res = await fetch(`${baseUrl}/api/hook-event`, {
         method: 'POST',
-        headers: { 'Content-Type': 'application/json' },
+        headers: { 'Content-Type': 'application/json', [HOOK_SECRET_HEADER]: getHookSecret() },
         body: JSON.stringify({
           event: 'stop',
           sessionId: 'nonexistent-session',
           data: {},
         }),
       });
-      // Should pass auth (localhost bypass) but may 404 on session — that's fine
-      // The key assertion is it does NOT return 401
+      // Should pass auth (localhost bypass + hook secret) but may 404 on session — that's fine.
+      // The key assertion is it does NOT return 401 (COD-91: secret required even with no tunnel).
       expect(res.status).not.toBe(401);
     });
 
     it('should reject hook events with invalid schema', async () => {
       const res = await fetch(`${baseUrl}/api/hook-event`, {
         method: 'POST',
-        headers: { 'Content-Type': 'application/json' },
+        headers: { 'Content-Type': 'application/json', [HOOK_SECRET_HEADER]: getHookSecret() },
         body: JSON.stringify({ invalid: 'data' }),
       });
-      // Schema validation should catch this
+      // Past the auth gate (valid secret) → schema validation should catch this (not a 401).
       expect(res.status).not.toBe(401); // Not an auth error
     });
   });

test/cod54-hook-event-auth.test.ts

@@ -4,15 +4,19 @@
  * The `/api/hook-event` localhost bypass let tunnel traffic (cloudflared
  * --url http://127.0.0.1:port) reach the loopback origin with req.ip ===
  * 127.0.0.1 and drive respawn/Ralph signals unauthenticated. The fix gates
- * the bypass behind a shared hook secret WHEN A TUNNEL IS RUNNING, while
- * keeping the plain localhost bypass for the normal loopback-only case so
- * already-deployed (pre-secret) hooks and the loop's own channel keep working.
+ * the bypass behind a shared hook secret. COD-91 makes that requirement
+ * UNCONDITIONAL — the loopback bypass requires the secret whether or not a
+ * managed tunnel is running, because Codeman can't detect a user's own loopback
+ * reverse proxy (own cloudflared / `tailscale serve` / nginx → 127.0.0.1).
+ * Managed-session hooks always present the secret, so the legitimate channel
+ * keeps working.
  *
  * Tests:
  *  - tunnel running + no secret  → 401 (closes the hole)
  *  - tunnel running + bad secret → 401
  *  - tunnel running + good secret → not 401 (allowed)
- *  - tunnel NOT running + no secret → not 401 (back-compat regression guard)
+ *  - tunnel NOT running + no secret → 401 (COD-91: secret required unconditionally)
+ *  - tunnel NOT running + good secret → not 401 (allowed)
  *  - rate limiting: rapid unauthorized hook POSTs eventually 429
  *
  * Port: 3230 (tunnel-running), 3231 (tunnel-down), 3232 (rate-limit)
@@ -83,7 +87,7 @@ describe('COD-54 hook-event auth — tunnel running requires secret', () => {
   });
 });
 
-describe('COD-54 hook-event auth — tunnel down keeps localhost bypass (back-compat)', () => {
+describe('COD-91 hook-event auth — tunnel down ALSO requires the secret', () => {
   let server: WebServer;
   let baseUrl: string;
   let isRunningSpy: ReturnType<typeof vi.spyOn>;
@@ -105,8 +109,13 @@ describe('COD-54 hook-event auth — tunnel down keeps localhost bypass (back-co
     delete process.env.CODEMAN_USERNAME;
   });
 
-  it('still allows a localhost hook POST WITHOUT a secret (existing hooks + loop channel keep working)', async () => {
+  it('rejects a localhost hook POST WITHOUT a secret even with no tunnel (COD-91)', async () => {
     const res = await postHook(baseUrl);
+    expect(res.status).toBe(401);
+  });
+
+  it('allows a localhost hook POST WITH the correct secret when no tunnel is running', async () => {
+    const res = await postHook(baseUrl, { [HOOK_SECRET_HEADER]: getHookSecret() });
     expect(res.status).not.toBe(401);
   });
 });

test/hook-secret-selfheal.test.ts

@@ -0,0 +1,106 @@
+/**
+ * COD-91 — `refreshStaleHookSecret` self-heal.
+ *
+ * Making the hook-event secret unconditionally required (PR #127) would silently 401 the
+ * hook curls baked into cases created before the secret header existed (COD-54). Those
+ * curls live in `.claude/settings.local.json` and `writeHooksConfig` only runs at case
+ * CREATION, so existing cases never refresh. `refreshStaleHookSecret` regenerates the
+ * hooks block on session spawn — but ONLY when the case already holds Codeman's own
+ * pre-secret hook curls, never clobbering a user's customizations.
+ *
+ * Pure filesystem logic against a temp dir — no port / server / tmux.
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync, rmSync } from 'node:fs';
+import { join } from 'node:path';
+import { tmpdir } from 'node:os';
+import { refreshStaleHookSecret } from '../src/hooks-config.js';
+
+const SECRET_HEADER = 'X-Codeman-Hook-Secret';
+
+// A faithful pre-secret Codeman hook curl (what cases created before COD-54 contain):
+// targets /api/hook-event, but with NO X-Codeman-Hook-Secret header.
+function staleCodemanHooks() {
+  return {
+    Stop: [
+      {
+        matcher: '',
+        hooks: [
+          {
+            type: 'command',
+            command:
+              "HOOK_DATA=$(cat 2>/dev/null || echo '{}'); " +
+              'printf \'{"event":"stop","sessionId":"%s","data":%s}\' "$CODEMAN_SESSION_ID" "$HOOK_DATA" | ' +
+              'curl -s -X POST "$CODEMAN_API_URL/api/hook-event" -H \'Content-Type: application/json\' --data @- 2>/dev/null || true',
+            timeout: 5,
+          },
+        ],
+      },
+    ],
+  };
+}
+
+describe('refreshStaleHookSecret', () => {
+  let dir: string;
+  let settingsPath: string;
+
+  beforeEach(() => {
+    dir = mkdtempSync(join(tmpdir(), 'codeman-selfheal-'));
+    mkdirSync(join(dir, '.claude'), { recursive: true });
+    settingsPath = join(dir, '.claude', 'settings.local.json');
+  });
+
+  afterEach(() => {
+    rmSync(dir, { recursive: true, force: true });
+  });
+
+  it('adds the secret header to a stale Codeman hooks block and preserves other keys', async () => {
+    writeFileSync(
+      settingsPath,
+      JSON.stringify({ env: { CLAUDE_CODE_FOO: '1' }, model: 'opus', hooks: staleCodemanHooks() }, null, 2)
+    );
+    await refreshStaleHookSecret(dir);
+
+    const after = JSON.parse(readFileSync(settingsPath, 'utf-8'));
+    expect(JSON.stringify(after.hooks)).toContain(SECRET_HEADER);
+    expect(JSON.stringify(after.hooks)).toContain('CODEMAN_HOOK_SECRET_FILE');
+    // sibling keys untouched
+    expect(after.env).toEqual({ CLAUDE_CODE_FOO: '1' });
+    expect(after.model).toBe('opus');
+  });
+
+  it('leaves a hooks block that already carries the secret unchanged', async () => {
+    // Seed with a current block by healing a stale one first, then re-heal: second pass must no-op.
+    writeFileSync(settingsPath, JSON.stringify({ hooks: staleCodemanHooks() }, null, 2));
+    await refreshStaleHookSecret(dir);
+    const healed = readFileSync(settingsPath, 'utf-8');
+    expect(healed).toContain(SECRET_HEADER);
+
+    await refreshStaleHookSecret(dir);
+    expect(readFileSync(settingsPath, 'utf-8')).toBe(healed); // byte-identical: no rewrite
+  });
+
+  it('does not touch hooks that are not Codeman’s (no /api/hook-event)', async () => {
+    const foreign = JSON.stringify(
+      { hooks: { Stop: [{ matcher: '', hooks: [{ type: 'command', command: 'echo hi', timeout: 5 }] }] } },
+      null,
+      2
+    );
+    writeFileSync(settingsPath, foreign);
+    await refreshStaleHookSecret(dir);
+    expect(readFileSync(settingsPath, 'utf-8')).toBe(foreign);
+  });
+
+  it('is a no-op when settings.local.json is absent (does not create one)', async () => {
+    await refreshStaleHookSecret(dir);
+    expect(existsSync(settingsPath)).toBe(false);
+  });
+
+  it('leaves a malformed settings file untouched', async () => {
+    const garbage = '{ not valid json';
+    writeFileSync(settingsPath, garbage);
+    await refreshStaleHookSecret(dir);
+    expect(readFileSync(settingsPath, 'utf-8')).toBe(garbage);
+  });
+});