diff --git a/CHANGELOG.md b/CHANGELOG.md index d414d7a9..605d5b75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # codeman +## 0.2.7 + +### Patch Changes + +- Fix race condition in StateStore where dirty flag was overwritten after async write, silently discarding mutations +- Fix PlanOrchestrator session leak by adding session.stop() in finally blocks and centralizing cleanup +- Fix symlink path traversal in file-content and file-raw endpoints by adding realpathSync validation +- Fix PTY exit handler to clean up sessionListenerRefs, transcriptWatchers, runSummaryTrackers, and terminal batching state +- Fix sendInput() fire-and-forget by propagating runPrompt errors to task queue via taskError event +- Fix Ralph Loop tick() race condition by running checkTimeouts/assignTasks sequentially with per-iteration error handling +- Fix shell injection in hook scripts by piping HOOK_DATA via printf to curl stdin instead of inline embedding +- Narrow tail-file allowlist to remove ~/.cache and ~/.local/share paths that exposed credentials +- Fix stored XSS in quick-start dropdown by escaping case names with escapeHtml() + ## 0.2.6 ### Patch Changes diff --git a/CLAUDE.md b/CLAUDE.md index a7a7a393..848f60b2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,7 +17,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co **You may be running inside a Codeman-managed tmux session.** Before killing ANY tmux or Claude process: -1. Check: `echo $CODEMAN_TMUX` - if `1`, you're in a managed session +1. Check: `echo $CODEMAN_MUX` - if `1`, you're in a managed session 2. **NEVER** run `tmux kill-session`, `pkill tmux`, or `pkill claude` without confirming 3. Use the web UI or `./scripts/tmux-manager.sh` instead of direct kill commands @@ -52,7 +52,7 @@ When user says "COM": 4. **Sync CLAUDE.md version**: Update the `**Version**` line below to match the new version from `package.json` 5. **Commit and deploy**: `git add -A && git commit -m "chore: version packages" && git push && npm run build && systemctl --user restart codeman-web` -**Version**: 0.2.6 (must match `package.json`) +**Version**: 0.2.7 (must match `package.json`) ## Project Overview @@ -90,15 +90,18 @@ npx vitest run -t "pattern" # Tests matching name npm run test:coverage # With coverage report # Production -npm run build +npm run build # esbuild via scripts/build.mjs (not tsc) +npm run start # node dist/index.js (production) systemctl --user restart codeman-web journalctl --user -u codeman-web -f ``` +**CI**: `.github/workflows/ci.yml` runs `typecheck`, `lint`, and `format:check` on push to master. Tests are intentionally excluded from CI (they spawn tmux). + ## Common Gotchas - **Single-line prompts only** — `writeViaMux()` sends text and Enter separately; multi-line breaks Ink -- **Don't kill tmux sessions blindly** — Check `$CODEMAN_TMUX` first; you might be inside one +- **Don't kill tmux sessions blindly** — Check `$CODEMAN_MUX` first; you might be inside one - **Global regex `lastIndex` sharing** — `ANSI_ESCAPE_PATTERN_FULL/SIMPLE` have `g` flag; use `createAnsiPatternFull/Simple()` factory functions for fresh instances in loops - **DEC 2026 sync blocks** — Never discard incomplete sync blocks (START without END); buffer up to 50ms then flush. See `app.js:extractSyncSegments()` - **Terminal writes during buffer load** — Live SSE writes are queued while `_isLoadingBuffer` is true to prevent interleaving with historical data @@ -116,6 +119,7 @@ journalctl --user -u codeman-web -f | File | Purpose | |------|---------| +| `src/index.ts` | CLI entry point: global error recovery, uncaught exception guard, `MAX_CONSECUTIVE_ERRORS` auto-restart | | `src/session.ts` | PTY wrapper: `runPrompt()`, `startInteractive()`, `startShell()` | | `src/mux-interface.ts` | `TerminalMultiplexer` interface + `MuxSession` type | | `src/mux-factory.ts` | Create tmux multiplexer instance | @@ -168,6 +172,24 @@ journalctl --user -u codeman-web -f | `buffer-limits.ts` | Terminal/text buffer size limits | | `map-limits.ts` | Global limits for Maps, sessions, watchers | +### Utilities (`src/utils/`) + +Re-exported via `src/utils/index.ts`. Key exports: + +| File | Exports | +|------|---------| +| `cleanup-manager.ts` | `CleanupManager` — centralized disposal for timers, intervals, watchers, listeners, streams | +| `lru-map.ts` | `LRUMap` — bounded cache with eviction | +| `stale-expiration-map.ts` | `StaleExpirationMap` — TTL-based map with automatic cleanup | +| `regex-patterns.ts` | `ANSI_ESCAPE_PATTERN_FULL/SIMPLE`, `createAnsiPatternFull/Simple()`, `stripAnsi`, `TOKEN_PATTERN`, `SPINNER_PATTERN` | +| `buffer-accumulator.ts` | `BufferAccumulator` — batches rapid writes into single flushes | +| `claude-cli-resolver.ts` | `findClaudeDir`, `getAugmentedPath` — resolves Claude CLI paths | +| `opencode-cli-resolver.ts` | `resolveOpenCodeDir`, `isOpenCodeAvailable`, `getOpenCodeAugmentedPath` — OpenCode CLI support | +| `string-similarity.ts` | `stringSimilarity`, `fuzzyPhraseMatch`, `todoContentHash` | +| `token-validation.ts` | `validateTokenCounts`, `validateTokensAndCost` | +| `nice-wrapper.ts` | `wrapWithNice` — wraps commands with `nice`/`ionice` for lower priority | +| `type-safety.ts` | `assertNever` — exhaustive switch/case guard | + ### Data Flow 1. Session spawns `claude --dangerously-skip-permissions` via node-pty @@ -426,8 +448,20 @@ Use `LRUMap` for bounded caches with eviction, `StaleExpirationMap` for TTL-base | **Error codes** | `createErrorResponse()` in `src/types.ts` | | **Test utilities** | `test/respawn-test-utils.ts` | | **Mobile test suite** | `mobile-test/README.md` | +| **OpenCode integration** | `docs/opencode-integration.md` | +| **Local echo overlay** | `docs/local-echo-overlay-plan.md` | +| **Performance investigation** | `docs/performance-investigation-report.md` | +| **First-load optimization** | `docs/first-load-optimization-plan.md`, `docs/perf-audit-first-load.md` | +| **Dead code audit** | `docs/cleanup-findings.md` | +| **TypeScript improvements** | `docs/typescript-improvement-suggestions.md` | +| **Browser testing** | `docs/browser-testing-guide.md` | +| **Mobile testing report** | `docs/mobile-testing-report.md` | +| **Voice input** | `docs/voice-input-plan.md` | +| **Improvement roadmaps** | `docs/respawn-improvement-plan.md`, `docs/ralph-improvement-plan.md`, `docs/plan-improvement-roadmap.md` | +| **Background keystroke forwarding** | `docs/background-keystroke-forwarding-merged-plan.md` | +| **Run summary** | `docs/run-summary-plan.md` | -Additional design docs, plans, and investigation reports are in the `docs/` directory. +Additional design docs and investigation reports are in the `docs/` directory. ## Scripts @@ -437,6 +471,9 @@ Additional design docs, plans, and investigation reports are in the `docs/` dire | `scripts/monitor-respawn.sh` | Monitor respawn state machine in real-time | | `scripts/watch-subagents.ts` | Real-time subagent transcript watcher (list, follow by session/agent ID) | | `scripts/codeman-web.service` | systemd service file for production deployment | +| `scripts/codeman-tunnel.service` | systemd service file for persistent Cloudflare tunnel | +| `scripts/tunnel.sh` | Start/stop/check Cloudflare quick tunnel (`./scripts/tunnel.sh start\|stop\|url`) | +| `scripts/build.mjs` | esbuild-based production build (called by `npm run build`) | | `scripts/postinstall.js` | npm postinstall hook for setup | Additional scripts in `scripts/` for screenshots, demos, Ralph wizards, and browser testing. diff --git a/package.json b/package.json index 887423e5..8d717f05 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "codeman", - "version": "0.2.6", + "version": "0.2.7", "description": "The missing control plane for AI coding agents - run 20 autonomous agents with real-time monitoring and session persistence", "type": "module", "main": "dist/index.js", diff --git a/src/file-stream-manager.ts b/src/file-stream-manager.ts index fa187894..4d92563d 100644 --- a/src/file-stream-manager.ts +++ b/src/file-stream-manager.ts @@ -398,13 +398,7 @@ export class FileStreamManager extends EventEmitter { // Check if the resolved path is within the working directory // or common log directories (/tmp intentionally excluded — world-writable) - const allowedPaths = [ - normalizedWorkingDir, - '/var/log', - resolve(homedir(), '.local/share'), - resolve(homedir(), '.cache'), - resolve(homedir(), 'logs'), - ]; + const allowedPaths = [normalizedWorkingDir, '/var/log', resolve(homedir(), 'logs')]; const isAllowed = allowedPaths.some((allowed) => { const rel = relative(allowed, absolutePath); diff --git a/src/hooks-config.ts b/src/hooks-config.ts index f9843c71..79604487 100644 --- a/src/hooks-config.ts +++ b/src/hooks-config.ts @@ -27,9 +27,10 @@ export function generateHooksConfig(): { hooks: Record } { // Falls back to empty object if stdin is unavailable or malformed. const curlCmd = (event: HookEventType) => `HOOK_DATA=$(cat 2>/dev/null || echo '{}'); ` + + `printf '{"event":"${event}","sessionId":"%s","data":%s}' "$CODEMAN_SESSION_ID" "$HOOK_DATA" | ` + `curl -s -X POST "$CODEMAN_API_URL/api/hook-event" ` + `-H 'Content-Type: application/json' ` + - `-d "{\\"event\\":\\"${event}\\",\\"sessionId\\":\\"$CODEMAN_SESSION_ID\\",\\"data\\":$HOOK_DATA}" ` + + `--data @- ` + `2>/dev/null || true`; return { diff --git a/src/plan-orchestrator.ts b/src/plan-orchestrator.ts index 8d61482d..5d96919e 100644 --- a/src/plan-orchestrator.ts +++ b/src/plan-orchestrator.ts @@ -444,8 +444,6 @@ export class PlanOrchestrator { try { const { result: response } = await session.runPrompt(prompt, { model: this.researchModel }); - this.runningSessions.delete(session); - const durationMs = Date.now() - startTime; // Extract JSON from response @@ -528,7 +526,6 @@ export class PlanOrchestrator { return result; } catch (err) { - this.runningSessions.delete(session); const durationMs = Date.now() - startTime; const error = err instanceof Error ? err.message : String(err); onSubagent?.({ @@ -554,7 +551,10 @@ export class PlanOrchestrator { durationMs, }; } finally { - // Always clear the progress interval to prevent memory leaks + // Always clean up session and progress interval — centralizing here + // prevents the race where cancel() and catch both try to manage the set + await session.stop().catch(() => {}); + this.runningSessions.delete(session); clearInterval(progressInterval); } } @@ -617,8 +617,6 @@ export class PlanOrchestrator { try { const { result: response } = await session.runPrompt(prompt, { model: this.plannerModel }); - this.runningSessions.delete(session); - const durationMs = Date.now() - startTime; // Extract JSON from response @@ -670,7 +668,6 @@ export class PlanOrchestrator { return { success: true, items, gaps, warnings }; } catch (err) { - this.runningSessions.delete(session); const durationMs = Date.now() - startTime; const error = err instanceof Error ? err.message : String(err); onSubagent?.({ @@ -684,7 +681,10 @@ export class PlanOrchestrator { }); return { success: false, error }; } finally { - // Always clear the progress interval to prevent memory leaks + // Always clean up session and progress interval — centralizing here + // prevents the race where cancel() and catch both try to manage the set + await session.stop().catch(() => {}); + this.runningSessions.delete(session); clearInterval(progressInterval); } } diff --git a/src/ralph-loop.ts b/src/ralph-loop.ts index f561a756..1c7c8c7b 100644 --- a/src/ralph-loop.ts +++ b/src/ralph-loop.ts @@ -77,6 +77,7 @@ export class RalphLoop extends EventEmitter { completion: (sessionId: string, phrase: string) => void; error: (sessionId: string, error: string) => void; stopped: (sessionId: string) => void; + taskError: (sessionId: string, taskId: string, error: string) => void; } | null = null; constructor(options: RalphLoopOptions = {}) { @@ -118,11 +119,15 @@ export class RalphLoop extends EventEmitter { stopped: (sessionId: string) => { this.handleSessionStopped(sessionId); }, + taskError: (sessionId: string, taskId: string, error: string) => { + this.handleSessionTaskError(sessionId, taskId, error); + }, }; this.sessionManager.on('sessionCompletion', this.sessionEventHandlers.completion); this.sessionManager.on('sessionError', this.sessionEventHandlers.error); this.sessionManager.on('sessionStopped', this.sessionEventHandlers.stopped); + this.sessionManager.on('sessionTaskError', this.sessionEventHandlers.taskError); } /** Remove event listeners to prevent memory leaks */ @@ -131,6 +136,7 @@ export class RalphLoop extends EventEmitter { this.sessionManager.off('sessionCompletion', this.sessionEventHandlers.completion); this.sessionManager.off('sessionError', this.sessionEventHandlers.error); this.sessionManager.off('sessionStopped', this.sessionEventHandlers.stopped); + this.sessionManager.off('sessionTaskError', this.sessionEventHandlers.taskError); this.sessionEventHandlers = null; } } @@ -281,8 +287,11 @@ export class RalphLoop extends EventEmitter { private async tick(): Promise { this.store.setRalphLoopState({ lastCheckAt: Date.now() }); - // Run independent checks in parallel for better performance - await Promise.all([this.checkTimeouts(), this.assignTasks()]); + // Run sequentially: timeouts first so timed-out tasks are cleaned up + // before assignTasks() picks new work (prevents race where both + // mutate the same task concurrently) + await this.checkTimeouts(); + await this.assignTasks(); // Check if we should auto-generate tasks (depends on assignment results) if (this.autoGenerateTasks && this.shouldGenerateTasks()) { @@ -304,7 +313,11 @@ export class RalphLoop extends EventEmitter { break; } - await this.assignTaskToSession(task, session); + try { + await this.assignTaskToSession(task, session); + } catch (err) { + console.error(`[RalphLoop] Failed to assign task ${task.id} to session ${session.id}:`, err); + } } } @@ -400,6 +413,17 @@ export class RalphLoop extends EventEmitter { } } + private handleSessionTaskError(_sessionId: string, taskId: string, error: string): void { + const task = this.taskQueue.getTask(taskId); + if (!task) { + return; + } + + task.fail(error); + this.taskQueue.updateTask(task); + this.emit('taskFailed', task.id, error); + } + private shouldGenerateTasks(): boolean { // Generate tasks if: // 1. No pending tasks diff --git a/src/session-manager.ts b/src/session-manager.ts index 387c9cbe..47a3122b 100644 --- a/src/session-manager.ts +++ b/src/session-manager.ts @@ -54,6 +54,7 @@ interface SessionHandlers { error: (data: string) => void; completion: (phrase: string) => void; exit: () => void; + taskError: (taskId: string, error: string) => void; } export class SessionManager extends EventEmitter { @@ -134,12 +135,16 @@ export class SessionManager extends EventEmitter { this.emit('sessionStopped', session.id); this.updateSessionState(session); }, + taskError: (taskId: string, error: string) => { + this.emit('sessionTaskError', session.id, taskId, error); + }, }; session.on('output', handlers.output); session.on('error', handlers.error); session.on('completion', handlers.completion); session.on('exit', handlers.exit); + session.on('taskError', handlers.taskError); // Store handlers for later cleanup this.sessionHandlers.set(session.id, handlers); @@ -183,6 +188,7 @@ export class SessionManager extends EventEmitter { session.off('error', handlers.error); session.off('completion', handlers.completion); session.off('exit', handlers.exit); + session.off('taskError', handlers.taskError); this.sessionHandlers.delete(id); } diff --git a/src/session.ts b/src/session.ts index 4150aebe..e91024a9 100644 --- a/src/session.ts +++ b/src/session.ts @@ -2203,6 +2203,16 @@ export class Session extends EventEmitter { this._lastActivityAt = Date.now(); this.runPrompt(input).catch((err) => { const errorMsg = err instanceof Error ? err.message : String(err); + // Clean up task state so the task queue doesn't get stuck + if (this._currentTaskId) { + const taskId = this._currentTaskId; + this._currentTaskId = null; + this._status = 'idle'; + this._lastActivityAt = Date.now(); + this.emit('taskError', taskId, errorMsg); + } else { + this._status = 'idle'; + } this.emit('error', errorMsg); }); } diff --git a/src/state-store.ts b/src/state-store.ts index 106b196e..6aac6820 100644 --- a/src/state-store.ts +++ b/src/state-store.ts @@ -210,6 +210,10 @@ export class StateStore { return; } + // Clear dirty flag BEFORE async I/O so mutations during write re-set it. + // The state snapshot is already captured in `json` above. + this.dirty = false; + // Step 2: Create backup via file copy (async, no read+parse+write) try { await access(this.filePath); @@ -223,8 +227,6 @@ export class StateStore { await writeFile(tempPath, json, 'utf-8'); await rename(tempPath, this.filePath); - // Success! Clear dirty flag AFTER write completes - this.dirty = false; this.consecutiveSaveFailures = 0; if (this.circuitBreakerOpen) { console.log('[StateStore] Circuit breaker CLOSED - save succeeded'); @@ -232,6 +234,8 @@ export class StateStore { } } catch (err) { console.error('[StateStore] Failed to write state file:', err); + // Re-mark dirty so the data is retried on the next save cycle + this.dirty = true; this.consecutiveSaveFailures++; // Try to clean up temp file on error diff --git a/src/web/public/app.js b/src/web/public/app.js index bb4f468a..0bbac7e5 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -6391,7 +6391,7 @@ class CodemanApp { const displayName = c.name.length > maxNameLength ? c.name.substring(0, maxNameLength) + '…' : c.name; - options += ``; + options += ``; }); // Add testcase option if it doesn't exist (will be created on first run) diff --git a/src/web/server.ts b/src/web/server.ts index e6a48679..839a73ee 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -16,7 +16,17 @@ import fastifyCookie from '@fastify/cookie'; import fastifyStatic from '@fastify/static'; import { join, dirname, resolve, relative, isAbsolute } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { existsSync, statSync, mkdirSync, writeFileSync, readdirSync, readFileSync, rmSync, chmodSync } from 'node:fs'; +import { + existsSync, + statSync, + mkdirSync, + writeFileSync, + readdirSync, + readFileSync, + rmSync, + chmodSync, + realpathSync, +} from 'node:fs'; import fs from 'node:fs/promises'; import { execSync } from 'node:child_process'; import { randomBytes, timingSafeEqual } from 'node:crypto'; @@ -1391,15 +1401,21 @@ export class WebServer extends EventEmitter { return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Missing path parameter'); } - // Validate path is within working directory (security: proper path traversal check) + // Validate path is within working directory (security: resolve symlinks to prevent traversal) const fullPath = resolve(session.workingDir, filePath); - const relativePath = relative(session.workingDir, fullPath); + let resolvedPath: string; + try { + resolvedPath = realpathSync(fullPath); + } catch { + return createErrorResponse(ApiErrorCode.NOT_FOUND, 'File not found'); + } + const relativePath = relative(session.workingDir, resolvedPath); if (relativePath.startsWith('..') || isAbsolute(relativePath)) { return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Path must be within working directory'); } try { - const stat = await fs.stat(fullPath); + const stat = await fs.stat(resolvedPath); // Check if it's a binary/media file const ext = filePath.split('.').pop()?.toLowerCase() || ''; @@ -1460,7 +1476,7 @@ export class WebServer extends EventEmitter { // Read text file with line limit (bounded to prevent DoS) const MAX_LINES_LIMIT = 10000; const maxLines = Math.min(parseInt(lines || '500', 10) || 500, MAX_LINES_LIMIT); - const content = await fs.readFile(fullPath, 'utf-8'); + const content = await fs.readFile(resolvedPath, 'utf-8'); const allLines = content.split('\n'); const truncatedContent = allLines.length > maxLines; const displayContent = truncatedContent ? allLines.slice(0, maxLines).join('\n') : content; @@ -1497,9 +1513,16 @@ export class WebServer extends EventEmitter { return; } - // Validate path is within working directory (security: proper path traversal check) + // Validate path is within working directory (security: resolve symlinks to prevent traversal) const fullPath = resolve(session.workingDir, filePath); - const relativePath = relative(session.workingDir, fullPath); + let resolvedPath: string; + try { + resolvedPath = realpathSync(fullPath); + } catch { + reply.code(404).send(createErrorResponse(ApiErrorCode.NOT_FOUND, 'File not found')); + return; + } + const relativePath = relative(session.workingDir, resolvedPath); if (relativePath.startsWith('..') || isAbsolute(relativePath)) { reply.code(400).send(createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Path must be within working directory')); return; @@ -1508,7 +1531,7 @@ export class WebServer extends EventEmitter { try { // Validate file size before reading (DoS protection - prevent memory exhaustion) const MAX_RAW_FILE_SIZE = 50 * 1024 * 1024; // 50MB for raw files - const stat = await fs.stat(fullPath); + const stat = await fs.stat(resolvedPath); if (stat.size > MAX_RAW_FILE_SIZE) { reply .code(400) @@ -1541,7 +1564,7 @@ export class WebServer extends EventEmitter { json: 'application/json', }; - const content = await fs.readFile(fullPath); + const content = await fs.readFile(resolvedPath); reply.header('Content-Type', mimeTypes[ext] || 'application/octet-stream'); reply.send(content); } catch (err) { @@ -5027,6 +5050,79 @@ NOW: Generate the implementation plan for the task above. Think step by step.`; } catch (err) { console.error(`[Server] Error cleaning up respawn controller for ${session.id}:`, err); } + + // Clean up per-session resources that are stale after PTY exit. + // These are only cleaned by cleanupSession() on explicit delete, + // so without this they leak when a session exits without deletion. + try { + // Transcript watcher is tied to the specific PTY run + this.stopTranscriptWatcher(session.id); + + // Finalize run summary tracker + const summaryTracker = this.runSummaryTrackers.get(session.id); + if (summaryTracker) { + summaryTracker.recordSessionStopped(); + summaryTracker.stop(); + this.runSummaryTrackers.delete(session.id); + } + + // Flush/clear terminal batching state (no more output coming) + this.terminalBatches.delete(session.id); + this.terminalBatchSizes.delete(session.id); + const batchTimer = this.terminalBatchTimers.get(session.id); + if (batchTimer) { + clearTimeout(batchTimer); + this.terminalBatchTimers.delete(session.id); + } + this.taskUpdateBatches.delete(session.id); + this.stateUpdatePending.delete(session.id); + this.lastTerminalEventTime.delete(session.id); + + // Clear pending persist-debounce timer + const pendingPersist = this.persistDebounceTimers.get(session.id); + if (pendingPersist) { + clearTimeout(pendingPersist); + this.persistDebounceTimers.delete(session.id); + } + + // Close any active file streams + fileStreamManager.closeSessionStreams(session.id); + + // Remove stored listener refs to break closure references (prevents memory leak). + // Without this, the closures capture the Session object (including up to 2MB terminal buffer) + // and keep it alive even after the PTY exits. + const listenerRefs = this.sessionListenerRefs.get(session.id); + if (listenerRefs) { + session.off('terminal', listenerRefs.terminal); + session.off('clearTerminal', listenerRefs.clearTerminal); + session.off('needsRefresh', listenerRefs.needsRefresh); + session.off('message', listenerRefs.message); + session.off('error', listenerRefs.error); + session.off('completion', listenerRefs.completion); + session.off('exit', listenerRefs.exit); + session.off('working', listenerRefs.working); + session.off('idle', listenerRefs.idle); + session.off('taskCreated', listenerRefs.taskCreated); + session.off('taskUpdated', listenerRefs.taskUpdated); + session.off('taskCompleted', listenerRefs.taskCompleted); + session.off('taskFailed', listenerRefs.taskFailed); + session.off('autoClear', listenerRefs.autoClear); + session.off('autoCompact', listenerRefs.autoCompact); + session.off('cliInfoUpdated', listenerRefs.cliInfoUpdated); + session.off('ralphLoopUpdate', listenerRefs.ralphLoopUpdate); + session.off('ralphTodoUpdate', listenerRefs.ralphTodoUpdate); + session.off('ralphCompletionDetected', listenerRefs.ralphCompletionDetected); + session.off('ralphStatusBlockDetected', listenerRefs.ralphStatusBlockDetected); + session.off('ralphCircuitBreakerUpdate', listenerRefs.ralphCircuitBreakerUpdate); + session.off('ralphExitGateMet', listenerRefs.ralphExitGateMet); + session.off('bashToolStart', listenerRefs.bashToolStart); + session.off('bashToolEnd', listenerRefs.bashToolEnd); + session.off('bashToolsUpdate', listenerRefs.bashToolsUpdate); + this.sessionListenerRefs.delete(session.id); + } + } catch (err) { + console.error(`[Server] Error cleaning up session resources on exit for ${session.id}:`, err); + } }, working: () => { diff --git a/test/hooks-config.test.ts b/test/hooks-config.test.ts index 3c4fa688..f10b6dcf 100644 --- a/test/hooks-config.test.ts +++ b/test/hooks-config.test.ts @@ -667,8 +667,8 @@ describe('Hook Config Generation - Extended', () => { for (const hook of notifHooks) { const cmd = hook.hooks[0].command; - // The command contains escaped quotes for the JSON payload: \"event\":\"idle_prompt\" - expect(cmd).toContain(`\\"event\\":\\"${hook.matcher}\\"`); + // The printf format string contains the event name baked in + expect(cmd).toContain(`"event":"${hook.matcher}"`); } }); @@ -684,8 +684,9 @@ describe('Hook Config Generation - Extended', () => { const notifHooks = config.hooks.Notification as Array<{ hooks: Array<{ command: string }> }>; const cmd = notifHooks[0].hooks[0].command; expect(cmd).toContain('HOOK_DATA=$(cat'); - // The data field in the JSON uses escaped quotes: \"data\":$HOOK_DATA - expect(cmd).toContain('\\"data\\":$HOOK_DATA'); + // Data is piped to curl via stdin (--data @-) to prevent shell injection + expect(cmd).toContain('$HOOK_DATA'); + expect(cmd).toContain('--data @-'); }); it('should have consistent structure across all notification hooks', () => { @@ -701,6 +702,18 @@ describe('Hook Config Generation - Extended', () => { } }); + it('should pipe data to curl via stdin to prevent shell injection', () => { + const config = generateHooksConfig(); + const notifHooks = config.hooks.Notification as Array<{ hooks: Array<{ command: string }> }>; + const cmd = notifHooks[0].hooks[0].command; + // HOOK_DATA must NOT be embedded unquoted in a -d "..." argument (shell injection vector) + expect(cmd).not.toMatch(/-d\s+"[^"]*\$HOOK_DATA/); + // Instead, data should be piped to curl via stdin + expect(cmd).toContain('printf'); + expect(cmd).toContain('| curl'); + expect(cmd).toContain('--data @-'); + }); + it('should have stop hook without matcher (catches all)', () => { const config = generateHooksConfig(); const stopHooks = config.hooks.Stop as Array<{ matcher?: string; hooks: Array<{ command: string }> }>;