fix(hooks): self-heal stale pre-secret hook configs so COD-91 doesn't 401 them

Making the hook-event secret unconditionally required closes the own-loopback-proxy gap,
but it would also silently 401 the hook curls baked into cases created BEFORE the secret
header existed (COD-54, 2026-06-10): writeHooksConfig only runs at case CREATION, so an
existing/linked case on a password-protected install keeps secret-less curls that the new
gate rejects (degrading idle/stop/teammate/task signalling with no error surfaced).
No-password installs are unaffected — the gate isn't registered without CODEMAN_PASSWORD.

Add `refreshStaleHookSecret(casePath)` and call it on Claude-mode spawns in
POST /api/sessions and POST /api/quick-start (existing-case branch). It regenerates the
hooks block ONLY when settings.local.json already holds Codeman's own hook curls (they
target /api/hook-event) that lack the X-Codeman-Hook-Secret header — a no-op when the
hooks are absent, not ours, or already current, so it never clobbers user customizations
and is cheap on every spawn. Fresh cases are unaffected (writeHooksConfig already wrote
the secret). withSettingsLock serializes it with the model/statusLine writers.

Verified: new test/hook-secret-selfheal.test.ts 5/5 (heal + key-preservation + no-op on
current/foreign/absent/malformed); the PR's cod54 + auth-security suites still pass
(36); tsc, lint, format:check, and npm run build all clean (symbol present in dist).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude (Codeman maintainer)
2026-06-14 22:35:29 +02:00
parent f0f43ddbad
commit 21fbff4d8a
3 changed files with 162 additions and 1 deletions
+37
View File
@@ -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';
+19 -1
View File
@@ -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 —
+106
View File
@@ -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);
});
});