From 035e8fdfca2508e06865c18699d896fafe778241 Mon Sep 17 00:00:00 2001 From: nioasoft Date: Thu, 5 Feb 2026 21:08:46 +0200 Subject: [PATCH] fix: accept WebSocket before validation to prevent opaque 403 errors All 5 WebSocket endpoints (expand, spec, assistant, terminal, project) were closing the connection before calling accept() when validation failed. Starlette converts pre-accept close into an HTTP 403, giving clients no meaningful error information. Server changes: - Move websocket.accept() before all validation checks in every WS handler - Send JSON error message before closing so clients get actionable errors - Fix validate_project_name usage (raises HTTPException, not returns bool) - ConnectionManager.connect() no longer calls accept() (caller's job) Client changes: - All 3 WS hooks (useWebSocket, useExpandChat, useSpecChat) skip reconnection on 4xxx close codes (application errors won't self-resolve) - Gate expand button, keyboard shortcut, and modal on hasSpec - Add hasSpec to useEffect dependency array to prevent stale closure - Update keyboard shortcuts help text for E key context Co-Authored-By: Claude Opus 4.6 --- bin/autoforge.js | 0 server/routers/assistant_chat.py | 12 +++++++++--- server/routers/expand_project.py | 10 ++++++++-- server/routers/spec_creation.py | 12 +++++++++--- server/routers/terminal.py | 14 +++++++++++--- server/websocket.py | 14 ++++++++++---- ui/src/App.tsx | 8 ++++---- ui/src/components/KanbanBoard.tsx | 2 +- ui/src/components/KeyboardShortcutsHelp.tsx | 2 +- ui/src/hooks/useExpandChat.ts | 6 +++++- ui/src/hooks/useSpecChat.ts | 7 +++++-- ui/src/hooks/useWebSocket.ts | 6 +++++- 12 files changed, 68 insertions(+), 25 deletions(-) mode change 100644 => 100755 bin/autoforge.js diff --git a/bin/autoforge.js b/bin/autoforge.js old mode 100644 new mode 100755 diff --git a/server/routers/assistant_chat.py b/server/routers/assistant_chat.py index ceae8bd..69fe967 100644 --- a/server/routers/assistant_chat.py +++ b/server/routers/assistant_chat.py @@ -217,20 +217,26 @@ async def assistant_chat_websocket(websocket: WebSocket, project_name: str): - {"type": "error", "content": "..."} - Error message - {"type": "pong"} - Keep-alive pong """ - if not validate_project_name(project_name): + # Always accept WebSocket first to avoid opaque 403 errors + await websocket.accept() + + try: + project_name = validate_project_name(project_name) + except HTTPException: + await websocket.send_json({"type": "error", "content": "Invalid project name"}) await websocket.close(code=4000, reason="Invalid project name") return project_dir = _get_project_path(project_name) if not project_dir: + await websocket.send_json({"type": "error", "content": "Project not found in registry"}) await websocket.close(code=4004, reason="Project not found in registry") return if not project_dir.exists(): + await websocket.send_json({"type": "error", "content": "Project directory not found"}) await websocket.close(code=4004, reason="Project directory not found") return - - await websocket.accept() logger.info(f"Assistant WebSocket connected for project: {project_name}") session: Optional[AssistantChatSession] = None diff --git a/server/routers/expand_project.py b/server/routers/expand_project.py index 5b55824..d680b95 100644 --- a/server/routers/expand_project.py +++ b/server/routers/expand_project.py @@ -104,19 +104,26 @@ async def expand_project_websocket(websocket: WebSocket, project_name: str): - {"type": "error", "content": "..."} - Error message - {"type": "pong"} - Keep-alive pong """ + # Always accept the WebSocket first to avoid opaque 403 errors. + # Starlette returns 403 if we close before accepting. + await websocket.accept() + try: project_name = validate_project_name(project_name) except HTTPException: + await websocket.send_json({"type": "error", "content": "Invalid project name"}) await websocket.close(code=4000, reason="Invalid project name") return # Look up project directory from registry project_dir = _get_project_path(project_name) if not project_dir: + await websocket.send_json({"type": "error", "content": "Project not found in registry"}) await websocket.close(code=4004, reason="Project not found in registry") return if not project_dir.exists(): + await websocket.send_json({"type": "error", "content": "Project directory not found"}) await websocket.close(code=4004, reason="Project directory not found") return @@ -124,11 +131,10 @@ async def expand_project_websocket(websocket: WebSocket, project_name: str): from autoforge_paths import get_prompts_dir spec_path = get_prompts_dir(project_dir) / "app_spec.txt" if not spec_path.exists(): + await websocket.send_json({"type": "error", "content": "Project has no spec. Create a spec first before expanding."}) await websocket.close(code=4004, reason="Project has no spec. Create spec first.") return - await websocket.accept() - session: Optional[ExpandChatSession] = None try: diff --git a/server/routers/spec_creation.py b/server/routers/spec_creation.py index cb7263c..ebe5c5b 100644 --- a/server/routers/spec_creation.py +++ b/server/routers/spec_creation.py @@ -166,22 +166,28 @@ async def spec_chat_websocket(websocket: WebSocket, project_name: str): - {"type": "error", "content": "..."} - Error message - {"type": "pong"} - Keep-alive pong """ - if not validate_project_name(project_name): + # Always accept WebSocket first to avoid opaque 403 errors + await websocket.accept() + + try: + project_name = validate_project_name(project_name) + except HTTPException: + await websocket.send_json({"type": "error", "content": "Invalid project name"}) await websocket.close(code=4000, reason="Invalid project name") return # Look up project directory from registry project_dir = _get_project_path(project_name) if not project_dir: + await websocket.send_json({"type": "error", "content": "Project not found in registry"}) await websocket.close(code=4004, reason="Project not found in registry") return if not project_dir.exists(): + await websocket.send_json({"type": "error", "content": "Project directory not found"}) await websocket.close(code=4004, reason="Project directory not found") return - await websocket.accept() - session: Optional[SpecChatSession] = None try: diff --git a/server/routers/terminal.py b/server/routers/terminal.py index a53b9ab..ff6cbfd 100644 --- a/server/routers/terminal.py +++ b/server/routers/terminal.py @@ -221,8 +221,14 @@ async def terminal_websocket(websocket: WebSocket, project_name: str, terminal_i - {"type": "pong"} - Keep-alive response - {"type": "error", "message": "..."} - Error message """ + # Always accept WebSocket first to avoid opaque 403 errors + await websocket.accept() + # Validate project name - if not validate_project_name(project_name): + try: + project_name = validate_project_name(project_name) + except Exception: + await websocket.send_json({"type": "error", "message": "Invalid project name"}) await websocket.close( code=TerminalCloseCode.INVALID_PROJECT_NAME, reason="Invalid project name" ) @@ -230,6 +236,7 @@ async def terminal_websocket(websocket: WebSocket, project_name: str, terminal_i # Validate terminal ID if not validate_terminal_id(terminal_id): + await websocket.send_json({"type": "error", "message": "Invalid terminal ID"}) await websocket.close( code=TerminalCloseCode.INVALID_PROJECT_NAME, reason="Invalid terminal ID" ) @@ -238,6 +245,7 @@ async def terminal_websocket(websocket: WebSocket, project_name: str, terminal_i # Look up project directory from registry project_dir = _get_project_path(project_name) if not project_dir: + await websocket.send_json({"type": "error", "message": "Project not found in registry"}) await websocket.close( code=TerminalCloseCode.PROJECT_NOT_FOUND, reason="Project not found in registry", @@ -245,6 +253,7 @@ async def terminal_websocket(websocket: WebSocket, project_name: str, terminal_i return if not project_dir.exists(): + await websocket.send_json({"type": "error", "message": "Project directory not found"}) await websocket.close( code=TerminalCloseCode.PROJECT_NOT_FOUND, reason="Project directory not found", @@ -254,14 +263,13 @@ async def terminal_websocket(websocket: WebSocket, project_name: str, terminal_i # Verify terminal exists in metadata terminal_info = get_terminal_info(project_name, terminal_id) if not terminal_info: + await websocket.send_json({"type": "error", "message": "Terminal not found"}) await websocket.close( code=TerminalCloseCode.PROJECT_NOT_FOUND, reason="Terminal not found", ) return - await websocket.accept() - # Get or create terminal session for this project/terminal session = get_terminal_session(project_name, project_dir, terminal_id) diff --git a/server/websocket.py b/server/websocket.py index dfb4dee..2fdc22f 100644 --- a/server/websocket.py +++ b/server/websocket.py @@ -640,9 +640,7 @@ class ConnectionManager: self._lock = asyncio.Lock() async def connect(self, websocket: WebSocket, project_name: str): - """Accept a WebSocket connection for a project.""" - await websocket.accept() - + """Register a WebSocket connection for a project (must already be accepted).""" async with self._lock: if project_name not in self.active_connections: self.active_connections[project_name] = set() @@ -727,16 +725,24 @@ async def project_websocket(websocket: WebSocket, project_name: str): - Agent status changes - Agent stdout/stderr lines """ - if not validate_project_name(project_name): + # Always accept WebSocket first to avoid opaque 403 errors + await websocket.accept() + + try: + project_name = validate_project_name(project_name) + except Exception: + await websocket.send_json({"type": "error", "content": "Invalid project name"}) await websocket.close(code=4000, reason="Invalid project name") return project_dir = _get_project_path(project_name) if not project_dir: + await websocket.send_json({"type": "error", "content": "Project not found in registry"}) await websocket.close(code=4004, reason="Project not found in registry") return if not project_dir.exists(): + await websocket.send_json({"type": "error", "content": "Project directory not found"}) await websocket.close(code=4004, reason="Project directory not found") return diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 8a185a2..9e1b6e5 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -178,8 +178,8 @@ function App() { setShowAddFeature(true) } - // E : Expand project with AI (when project selected and has features) - if ((e.key === 'e' || e.key === 'E') && selectedProject && features && + // E : Expand project with AI (when project selected, has spec and has features) + if ((e.key === 'e' || e.key === 'E') && selectedProject && hasSpec && features && (features.pending.length + features.in_progress.length + features.done.length) > 0) { e.preventDefault() setShowExpandProject(true) @@ -239,7 +239,7 @@ function App() { window.addEventListener('keydown', handleKeyDown) return () => window.removeEventListener('keydown', handleKeyDown) - }, [selectedProject, showAddFeature, showExpandProject, selectedFeature, debugOpen, debugActiveTab, assistantOpen, features, showSettings, showKeyboardHelp, isSpecCreating, viewMode, showResetModal, wsState.agentStatus]) + }, [selectedProject, showAddFeature, showExpandProject, selectedFeature, debugOpen, debugActiveTab, assistantOpen, features, showSettings, showKeyboardHelp, isSpecCreating, viewMode, showResetModal, wsState.agentStatus, hasSpec]) // Combine WebSocket progress with feature data const progress = wsState.progress.total > 0 ? wsState.progress : { @@ -490,7 +490,7 @@ function App() { )} {/* Expand Project Modal - AI-powered bulk feature creation */} - {showExpandProject && selectedProject && ( + {showExpandProject && selectedProject && hasSpec && ( diff --git a/ui/src/components/KeyboardShortcutsHelp.tsx b/ui/src/components/KeyboardShortcutsHelp.tsx index 8ead81f..aa82a42 100644 --- a/ui/src/components/KeyboardShortcutsHelp.tsx +++ b/ui/src/components/KeyboardShortcutsHelp.tsx @@ -19,7 +19,7 @@ const shortcuts: Shortcut[] = [ { key: 'D', description: 'Toggle debug panel' }, { key: 'T', description: 'Toggle terminal tab' }, { key: 'N', description: 'Add new feature', context: 'with project' }, - { key: 'E', description: 'Expand project with AI', context: 'with features' }, + { key: 'E', description: 'Expand project with AI', context: 'with spec & features' }, { key: 'A', description: 'Toggle AI assistant', context: 'with project' }, { key: 'G', description: 'Toggle Kanban/Graph view', context: 'with project' }, { key: ',', description: 'Open settings' }, diff --git a/ui/src/hooks/useExpandChat.ts b/ui/src/hooks/useExpandChat.ts index 9150885..be632a5 100644 --- a/ui/src/hooks/useExpandChat.ts +++ b/ui/src/hooks/useExpandChat.ts @@ -107,16 +107,20 @@ export function useExpandChat({ }, 30000) } - ws.onclose = () => { + ws.onclose = (event) => { setConnectionStatus('disconnected') if (pingIntervalRef.current) { clearInterval(pingIntervalRef.current) pingIntervalRef.current = null } + // Don't retry on application-level errors (4xxx codes won't resolve on retry) + const isAppError = event.code >= 4000 && event.code <= 4999 + // Attempt reconnection if not intentionally closed if ( !manuallyDisconnectedRef.current && + !isAppError && reconnectAttempts.current < maxReconnectAttempts && !isCompleteRef.current ) { diff --git a/ui/src/hooks/useSpecChat.ts b/ui/src/hooks/useSpecChat.ts index b2bac62..3bd09bb 100644 --- a/ui/src/hooks/useSpecChat.ts +++ b/ui/src/hooks/useSpecChat.ts @@ -157,15 +157,18 @@ export function useSpecChat({ }, 30000) } - ws.onclose = () => { + ws.onclose = (event) => { setConnectionStatus('disconnected') if (pingIntervalRef.current) { clearInterval(pingIntervalRef.current) pingIntervalRef.current = null } + // Don't retry on application-level errors (4xxx codes won't resolve on retry) + const isAppError = event.code >= 4000 && event.code <= 4999 + // Attempt reconnection if not intentionally closed - if (reconnectAttempts.current < maxReconnectAttempts && !isCompleteRef.current) { + if (!isAppError && reconnectAttempts.current < maxReconnectAttempts && !isCompleteRef.current) { reconnectAttempts.current++ const delay = Math.min(1000 * Math.pow(2, reconnectAttempts.current), 10000) reconnectTimeoutRef.current = window.setTimeout(connect, delay) diff --git a/ui/src/hooks/useWebSocket.ts b/ui/src/hooks/useWebSocket.ts index 1a44435..b9c0a3f 100644 --- a/ui/src/hooks/useWebSocket.ts +++ b/ui/src/hooks/useWebSocket.ts @@ -335,10 +335,14 @@ export function useProjectWebSocket(projectName: string | null) { } } - ws.onclose = () => { + ws.onclose = (event) => { setState(prev => ({ ...prev, isConnected: false })) wsRef.current = null + // Don't retry on application-level errors (4xxx codes won't resolve on retry) + const isAppError = event.code >= 4000 && event.code <= 4999 + if (isAppError) return + // Exponential backoff reconnection const delay = Math.min(1000 * Math.pow(2, reconnectAttempts.current), 30000) reconnectAttempts.current++