From 499d35566b0deee500e1159b7ba6e815c200d5e5 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 16 Aug 2026 19:15:08 +0200 Subject: [PATCH] review fixes: never install workspace hooks for a remote attach or a cwd-fallback create MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A claude-mode attachRemoteSession create overwrites workingDir with the user@host:session pseudo-path, which is a RELATIVE path locally — the old refresh-only call no-op'd on it, but ensureCodemanHooks mkdirs, so it created a junk local directory. And with workingDir omitted the cwd fallback reaches the hooks write unvalidated; under installer-created services cwd is $HOME, so hooks materialized in ~/.claude/settings.local.json. Both guarded at the applyWorkspaceHooks call site; regression tests prove the remote attach leaves no junk dir and the no-workingDir create leaves the server cwd untouched. Co-Authored-By: Claude Fable 5 --- src/web/routes/session-routes.ts | 8 +++-- .../session-routes-workspace-hooks.test.ts | 32 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 5174221f..c4a00c57 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -793,8 +793,12 @@ export function registerSessionRoutes( } // Hooks for the workspace this session runs in (install vs refresh-only is the - // `workspaceHooksEnabled` setting; see applyWorkspaceHooks). - if ((body.mode ?? 'claude') === 'claude') { + // `workspaceHooksEnabled` setting; see applyWorkspaceHooks). Never for a remote + // attach (workingDir is a user@host:session pseudo-path — mkdir would create it + // as a junk local dir), and only when the caller named a workingDir: the + // process-cwd fallback is $HOME under installer-created services, and hooks + // materializing in ~/.claude/settings.local.json was never asked for. + if (!remote && body.workingDir && (body.mode ?? 'claude') === 'claude') { await applyWorkspaceHooks(ctx, workingDir); // Agent skill (docs/agent-control-plan.md §2): ADD-ONLY on create, same shared- // .claude rationale as the statusLine above: a create must never remove the diff --git a/test/routes/session-routes-workspace-hooks.test.ts b/test/routes/session-routes-workspace-hooks.test.ts index e1eb8130..eac6685e 100644 --- a/test/routes/session-routes-workspace-hooks.test.ts +++ b/test/routes/session-routes-workspace-hooks.test.ts @@ -23,6 +23,7 @@ import { createMockRouteContext } from '../mocks/index.js'; import { installRouteErrorHandler } from '../../src/web/route-error-handler.js'; import { registerSessionRoutes } from '../../src/web/routes/session-routes.js'; import { generateHooksConfig } from '../../src/hooks-config.js'; +import { getDataDir } from '../../src/config/instance.js'; interface HooksFile { hooks?: Record }>>; @@ -138,6 +139,37 @@ describe('POST /api/sessions workspace hooks', () => { expect(existsSync(settingsPath())).toBe(false); }); + it('leaves the server cwd alone when workingDir is omitted', async () => { + // workingDir falls back to process.cwd(), which is $HOME under installer-created + // services — hooks must not materialize in ~/.claude/settings.local.json. + const cwdSettings = join(process.cwd(), '.claude', 'settings.local.json'); + const before = existsSync(cwdSettings) ? await readFile(cwdSettings, 'utf-8') : null; + + expect((await createSession({ name: 'hooks-no-dir', mode: 'claude' })).statusCode).toBe(200); + + const after = existsSync(cwdSettings) ? await readFile(cwdSettings, 'utf-8') : null; + expect(after).toBe(before); + }); + + it('never writes hooks for a remote attach (workingDir is a user@host pseudo-path)', async () => { + // A claude-mode attachRemoteSession create overwrites workingDir with + // `user@host:session` — locally a RELATIVE path, so a mkdir would create it + // as a junk directory under the server cwd. + await mkdir(getDataDir(), { recursive: true }); + await writeFile( + join(getDataDir(), 'remote-hosts.json'), + JSON.stringify([{ id: 'h1', label: 'box', host: '10.0.0.5', username: 'dev' }]) + ); + + const res = await createSession({ + name: 'hooks-remote', + mode: 'claude', + attachRemoteSession: { hostId: 'h1', remoteSessionName: 'codeman-ssh-abc123' }, + }); + expect(res.statusCode).toBe(200); + expect(existsSync(join(process.cwd(), 'dev@10.0.0.5:codeman-ssh-abc123'))).toBe(false); + }); + it('leaves a malformed settings file untouched rather than replacing it', async () => { await mkdir(join(workingDir, '.claude'), { recursive: true }); await writeFile(settingsPath(), '{ not json');