From dfb102de51ed5d27a7fca5fd0c2511a84f06efc1 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 WebSocket endpoints now call websocket.accept() before any validation checks. Previously, closing the connection before accepting caused Starlette to return an opaque HTTP 403 instead of a meaningful error message. Changes: - Server: Accept WebSocket first, then send JSON error + close with 4xxx code if validation fails (expand, spec, assistant, terminal, main project WS) - Server: ConnectionManager.connect() no longer calls accept() to avoid double-accept - UI: Gate expand button and keyboard shortcut on hasSpec - UI: Skip WebSocket reconnection on application error codes (4000-4999) - UI: Update keyboard shortcuts help text 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 | 10 ++++++++-- server/websocket.py | 13 ++++++++----- 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, 63 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 ceae8bd2..69fe9672 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 5b55824e..d680b952 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 cb7263ce..ebe5c5bd 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 a53b9abe..6845a27e 100644 --- a/server/routers/terminal.py +++ b/server/routers/terminal.py @@ -221,8 +221,12 @@ 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): + await websocket.send_json({"type": "error", "message": "Invalid project name"}) await websocket.close( code=TerminalCloseCode.INVALID_PROJECT_NAME, reason="Invalid project name" ) @@ -230,6 +234,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 +243,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 +251,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 +261,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 dfb4dee7..e6600643 100644 --- a/server/websocket.py +++ b/server/websocket.py @@ -640,9 +640,7 @@ def __init__(self): 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,22 @@ async def project_websocket(websocket: WebSocket, project_name: str): - Agent status changes - Agent stdout/stderr lines """ + # Always accept WebSocket first to avoid opaque 403 errors + await websocket.accept() + if not validate_project_name(project_name): + 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 @@ -879,8 +883,7 @@ async def on_dev_status_change(status: str): break except json.JSONDecodeError: logger.warning(f"Invalid JSON from WebSocket: {data[:100] if data else 'empty'}") - except Exception as e: - logger.warning(f"WebSocket error: {e}") + except Exception: break finally: diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 8a185a27..9e1b6e52 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 8ead81fa..aa82a425 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 91508852..be632a54 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 b2bac628..3bd09bb2 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 1a444359..b9c0a3fe 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++