diff --git a/apps/server/src/services/test-runner-service.ts b/apps/server/src/services/test-runner-service.ts index 71762d8d..d55d7be6 100644 --- a/apps/server/src/services/test-runner-service.ts +++ b/apps/server/src/services/test-runner-service.ts @@ -209,6 +209,16 @@ class TestRunnerService { return `test-${Date.now()}-${Math.random().toString(36).slice(2, 11)}`; } + /** + * Sanitize a test file path to prevent command injection + * Allows only safe characters for file paths + */ + private sanitizeTestFile(testFile: string): string { + // Remove any shell metacharacters and normalize path + // Allow only alphanumeric, dots, slashes, hyphens, underscores, colons (for Windows paths) + return testFile.replace(/[^a-zA-Z0-9.\\/_\-:]/g, ''); + } + /** * Start tests in a worktree using the provided command * @@ -252,9 +262,11 @@ class TestRunnerService { // Build the final command (append test file if specified) let finalCommand = command; if (testFile) { + // Sanitize test file path to prevent command injection + const sanitizedFile = this.sanitizeTestFile(testFile); // Append the test file to the command // Most test runners support: command -- file or command file - finalCommand = `${command} -- ${testFile}`; + finalCommand = `${command} -- ${sanitizedFile}`; } // Parse command into cmd and args (shell execution) @@ -650,15 +662,19 @@ export function getTestRunnerService(): TestRunnerService { } // Cleanup on process exit -process.on('SIGTERM', async () => { +process.on('SIGTERM', () => { if (testRunnerServiceInstance) { - await testRunnerServiceInstance.cleanup(); + testRunnerServiceInstance.cleanup().catch((err) => { + logger.error('Cleanup failed on SIGTERM:', err); + }); } }); -process.on('SIGINT', async () => { +process.on('SIGINT', () => { if (testRunnerServiceInstance) { - await testRunnerServiceInstance.cleanup(); + testRunnerServiceInstance.cleanup().catch((err) => { + logger.error('Cleanup failed on SIGINT:', err); + }); } }); diff --git a/apps/ui/src/components/ui/test-logs-panel.tsx b/apps/ui/src/components/ui/test-logs-panel.tsx index 74d9e948..119557c2 100644 --- a/apps/ui/src/components/ui/test-logs-panel.tsx +++ b/apps/ui/src/components/ui/test-logs-panel.tsx @@ -61,6 +61,12 @@ function getStatusIndicator(status: TestRunStatus | null): { className: 'bg-blue-500/10 text-blue-500', icon: , }; + case 'pending': + return { + text: 'Pending', + className: 'bg-amber-500/10 text-amber-500', + icon: , + }; case 'passed': return { text: 'Passed', diff --git a/apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx b/apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx index 0f42af63..40f10e85 100644 --- a/apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx +++ b/apps/ui/src/components/views/board-view/worktree-panel/worktree-panel.tsx @@ -280,33 +280,36 @@ export function WorktreePanel({ ); // Handler to start tests for a worktree - const handleStartTests = useCallback(async (worktree: WorktreeInfo) => { - setIsStartingTests(true); - try { - const api = getElectronAPI(); - if (!api?.worktree?.startTests) { - toast.error('Test runner API not available'); - return; - } + const handleStartTests = useCallback( + async (worktree: WorktreeInfo) => { + setIsStartingTests(true); + try { + const api = getElectronAPI(); + if (!api?.worktree?.startTests) { + toast.error('Test runner API not available'); + return; + } - const result = await api.worktree.startTests(worktree.path, { projectPath }); - if (result.success) { - toast.success('Tests started', { - description: `Running tests in ${worktree.branch}`, - }); - } else { + const result = await api.worktree.startTests(worktree.path, { projectPath }); + if (result.success) { + toast.success('Tests started', { + description: `Running tests in ${worktree.branch}`, + }); + } else { + toast.error('Failed to start tests', { + description: result.error || 'Unknown error', + }); + } + } catch (error) { toast.error('Failed to start tests', { - description: result.error || 'Unknown error', + description: error instanceof Error ? error.message : 'Unknown error', }); + } finally { + setIsStartingTests(false); } - } catch (error) { - toast.error('Failed to start tests', { - description: error instanceof Error ? error.message : 'Unknown error', - }); - } finally { - setIsStartingTests(false); - } - }, []); + }, + [projectPath] + ); // Handler to stop tests for a worktree const handleStopTests = useCallback( diff --git a/apps/ui/src/components/views/project-settings-view/testing-section.tsx b/apps/ui/src/components/views/project-settings-view/testing-section.tsx index cb00d454..c457145f 100644 --- a/apps/ui/src/components/views/project-settings-view/testing-section.tsx +++ b/apps/ui/src/components/views/project-settings-view/testing-section.tsx @@ -64,12 +64,14 @@ export function TestingSection({ project }: TestingSectionProps) { setIsSaving(true); try { const httpClient = getHttpApiClient(); + const normalizedCommand = testCommand.trim(); const response = await httpClient.settings.updateProject(project.path, { - testCommand: testCommand.trim() || undefined, + testCommand: normalizedCommand || undefined, }); if (response.success) { - setOriginalTestCommand(testCommand); + setTestCommand(normalizedCommand); + setOriginalTestCommand(normalizedCommand); toast.success('Test command saved'); } else { toast.error('Failed to save test command', { diff --git a/apps/ui/src/hooks/use-test-logs.ts b/apps/ui/src/hooks/use-test-logs.ts index e14d2f8c..596d7895 100644 --- a/apps/ui/src/hooks/use-test-logs.ts +++ b/apps/ui/src/hooks/use-test-logs.ts @@ -126,6 +126,9 @@ export function useTestLogs({ // Track the current session ID for filtering events const currentSessionId = useRef(targetSessionId ?? null); + // Guard against stale fetch results when switching worktrees/sessions + const fetchSeq = useRef(0); + /** * Derived state: whether tests are currently running */ @@ -137,11 +140,16 @@ export function useTestLogs({ const fetchLogs = useCallback(async () => { if (!worktreePath && !targetSessionId) return; + // Increment sequence to guard against stale responses + const seq = ++fetchSeq.current; + setState((prev) => ({ ...prev, isLoading: true, error: null })); try { const api = getElectronAPI(); if (!api?.worktree?.getTestLogs) { + // Check if this request is still current + if (seq !== fetchSeq.current) return; setState((prev) => ({ ...prev, isLoading: false, @@ -152,6 +160,9 @@ export function useTestLogs({ const result = await api.worktree.getTestLogs(worktreePath ?? undefined, targetSessionId); + // Check if this request is still current (prevent stale updates) + if (seq !== fetchSeq.current) return; + if (result.success && result.result) { const { sessionId, command, status, testFile, logs, startedAt, finishedAt, exitCode } = result.result; @@ -183,6 +194,8 @@ export function useTestLogs({ })); } } catch (error) { + // Check if this request is still current + if (seq !== fetchSeq.current) return; logger.error('Failed to fetch test logs:', error); setState((prev) => ({ ...prev,