Fix /addserver command stability and improve URL escaping

- Fix conversation handler timeout issues that caused /addserver to stop working
- Add 5-minute timeout to prevent stuck conversations
- Enhance input validation for server name, API URL, and SHA256
- Add duplicate server prevention (same API URL)
- Improve error handling and database session management
- Add comprehensive logging for debugging
- Add new /status command for admin monitoring
- Refactor all Telegram messages to use MarkdownV2 with proper escaping
- Escape all access URLs and dynamic content to prevent markdown injection
- Update scheduler notifications to use proper escaping
- Add comprehensive test suite for URL escaping functionality
- Improve conversation state management and cleanup
- Add graceful error recovery with user-friendly messages

Author: Mhr1375

bot/handlers/admin.py

@@ -170,13 +170,23 @@ async def start_add_server(update: Update, context: ContextTypes.DEFAULT_TYPE) -
 
 async def get_server_name(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
     """Get server name from user."""
+    if not update.message or not update.message.text:
+        await update.message.reply_text("❌ Please send a text message with the server name.")
+        return WAITING_SERVER_NAME
+    
     server_name = update.message.text.strip()
 
     if not server_name:
         await update.message.reply_text("❌ Server name cannot be empty. Please enter a valid name:")
         return WAITING_SERVER_NAME
 
+    if len(server_name) > 100:
+        await update.message.reply_text("❌ Server name too long. Please enter a name with less than 100 characters:")
+        return WAITING_SERVER_NAME
+    
     context.user_data['server_name'] = server_name
+    logger.info(f"User {update.effective_user.id} set server name: {server_name}")
+    
     await update.message.reply_text(
         f"✅ Server name set to: {server_name}\n\n"
         "Now please enter the API URL (e.g., https://your-server.com:12345/access-keys):"
@@ -186,6 +196,10 @@ async def get_server_name(update: Update, context: ContextTypes.DEFAULT_TYPE) ->
 
 async def get_api_url(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
     """Get API URL from user."""
+    if not update.message or not update.message.text:
+        await update.message.reply_text("❌ Please send a text message with the API URL.")
+        return WAITING_API_URL
+    
     api_url = update.message.text.strip()
 
     if not api_url.startswith(('http://', 'https://')):
@@ -194,7 +208,13 @@ async def get_api_url(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int
         )
         return WAITING_API_URL
 
+    if len(api_url) > 500:
+        await update.message.reply_text("❌ URL too long. Please enter a shorter URL:")
+        return WAITING_API_URL
+    
     context.user_data['api_url'] = api_url
+    logger.info(f"User {update.effective_user.id} set API URL: {api_url}")
+    
     await update.message.reply_text(
         f"✅ API URL set to: {api_url}\n\n"
         "Finally, please enter the certificate SHA256 fingerprint:"
@@ -204,6 +224,10 @@ async def get_api_url(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int
 
 async def get_cert_sha256(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
     """Get certificate SHA256 and save the server."""
+    if not update.message or not update.message.text:
+        await update.message.reply_text("❌ Please send a text message with the certificate SHA256.")
+        return WAITING_CERT_SHA256
+    
     cert_sha256 = update.message.text.strip()
 
     if len(cert_sha256) != 64:
@@ -212,36 +236,73 @@ async def get_cert_sha256(update: Update, context: ContextTypes.DEFAULT_TYPE) ->
         )
         return WAITING_CERT_SHA256
 
-    # Save the server
-    async with get_db_session() as session:
-        server = Server(
-            name=context.user_data['server_name'],
-            api_url=context.user_data['api_url'],
-            cert_sha256=cert_sha256
+    # Validate that it's a valid hex string
+    try:
+        int(cert_sha256, 16)
+    except ValueError:
+        await update.message.reply_text(
+            "❌ Invalid SHA256 format. Please enter a valid hexadecimal SHA256 hash:"
         )
-        session.add(server)
-        await session.commit()
-        
-        await log_action(
-            update.effective_user.id,
-            "server_added",
-            "server",
-            str(server.id),
-            f"Added server {server.name}"
+        return WAITING_CERT_SHA256
+    
+    # Check that we have all required data
+    if 'server_name' not in context.user_data or 'api_url' not in context.user_data:
+        await update.message.reply_text(
+            "❌ Missing server information. Please start over with /addserver"
         )
-        
-        escaped_name = escape_markdown(server.name, version=2)
-        escaped_api_url = escape_markdown(server.api_url, version=2)
-        escaped_cert = escape_markdown(cert_sha256[:16] + "...", version=2)
-        
+        context.user_data.clear()
+        return ConversationHandler.END
+    
+    # Save the server
+    try:
+        async with get_db_session() as session:
+            # Check if a server with the same API URL already exists
+            existing_server = await session.execute(
+                select(Server).where(Server.api_url == context.user_data['api_url'])
+            )
+            if existing_server.scalar_one_or_none():
+                await update.message.reply_text(
+                    "❌ A server with this API URL already exists. Please use a different URL."
+                )
+                return WAITING_API_URL
+            
+            server = Server(
+                name=context.user_data['server_name'],
+                api_url=context.user_data['api_url'],
+                cert_sha256=cert_sha256
+            )
+            session.add(server)
+            await session.commit()
+            
+            logger.info(f"User {update.effective_user.id} added server: {server.name} (ID: {server.id})")
+            
+            await log_action(
+                update.effective_user.id,
+                "server_added",
+                "server",
+                str(server.id),
+                f"Added server {server.name}"
+            )
+            
+            escaped_name = escape_markdown(server.name, version=2)
+            escaped_api_url = escape_markdown(server.api_url, version=2)
+            escaped_cert = escape_markdown(cert_sha256[:16] + "...", version=2)
+            
+            await update.message.reply_text(
+                f"✅ Server added successfully\\!\n\n"
+                f"**Name:** {escaped_name}\n"
+                f"**ID:** {server.id}\n"
+                f"**API URL:** {escaped_api_url}\n"
+                f"**Cert SHA256:** {escaped_cert}",
+                parse_mode='MarkdownV2'
+            )
+    except Exception as e:
+        logger.error(f"Error adding server for user {update.effective_user.id}: {e}")
         await update.message.reply_text(
-            f"✅ Server added successfully\\!\n\n"
-            f"**Name:** {escaped_name}\n"
-            f"**ID:** {server.id}\n"
-            f"**API URL:** {escaped_api_url}\n"
-            f"**Cert SHA256:** {escaped_cert}",
-            parse_mode='MarkdownV2'
+            "❌ An error occurred while adding the server. Please try again later."
         )
+        context.user_data.clear()
+        return ConversationHandler.END
 
     # Clear conversation data
     context.user_data.clear()
@@ -255,6 +316,72 @@ async def cancel_add_server(update: Update, context: ContextTypes.DEFAULT_TYPE)
     return ConversationHandler.END
 
 
+async def timeout_add_server(update: Update, context: ContextTypes.DEFAULT_TYPE) -> int:
+    """Handle conversation timeout for add server."""
+    if update.message:
+        await update.message.reply_text(
+            "⏰ Server addition timed out due to inactivity. Please run /addserver again to start over."
+        )
+    context.user_data.clear()
+    return ConversationHandler.END
+
+
+@admin_only
+async def bot_status(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None:
+    """Show bot status information for debugging."""
+    try:
+        async with get_db_session() as session:
+            # Count users by role
+            users_result = await session.execute(select(User))
+            users = users_result.scalars().all()
+            
+            user_counts = {
+                "pending": sum(1 for u in users if u.role == UserRole.PENDING),
+                "sales": sum(1 for u in users if u.role == UserRole.SALES),
+                "blocked": sum(1 for u in users if u.role == UserRole.BLOCKED),
+                "admin": sum(1 for u in users if u.role == UserRole.ADMIN),
+            }
+            
+            # Count servers
+            servers_result = await session.execute(select(Server))
+            servers = servers_result.scalars().all()
+            active_servers = sum(1 for s in servers if s.is_active)
+            
+            # Count access keys
+            keys_result = await session.execute(select(AccessKey))
+            keys = keys_result.scalars().all()
+            
+            now = datetime.utcnow()
+            active_keys = sum(1 for k in keys if k.disabled_at is None and k.expires_at > now)
+            expired_keys = sum(1 for k in keys if k.disabled_at is None and k.expires_at <= now)
+            disabled_keys = sum(1 for k in keys if k.disabled_at is not None)
+            
+            status_message = (
+                f"🤖 **Bot Status**\n\n"
+                f"👥 **Users:**\n"
+                f"  • Total: {len(users)}\n"
+                f"  • Pending: {user_counts['pending']}\n"
+                f"  • Sales: {user_counts['sales']}\n"
+                f"  • Blocked: {user_counts['blocked']}\n"
+                f"  • Admin: {user_counts['admin']}\n\n"
+                f"🖥️ **Servers:**\n"
+                f"  • Total: {len(servers)}\n"
+                f"  • Active: {active_servers}\n\n"
+                f"🔑 **Access Keys:**\n"
+                f"  • Total: {len(keys)}\n"
+                f"  • Active: {active_keys}\n"
+                f"  • Expired: {expired_keys}\n"
+                f"  • Disabled: {disabled_keys}\n\n"
+                f"⏰ **Timestamp:** {datetime.utcnow().strftime('%Y-%m-%d %H:%M:%S')} UTC"
+            )
+            
+            await update.message.reply_text(status_message)
+            
+    except Exception as e:
+        logger.error(f"Error getting bot status: {e}")
+        await update.message.reply_text(f"❌ Error retrieving bot status: {e}")
+
+
 @admin_only
 async def remove_server(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None:
     """Remove a VPN server."""
@@ -434,13 +561,4 @@ async def notify_admin_new_user(context: ContextTypes.DEFAULT_TYPE, user: User)
         logger.error(f"Failed to notify admin about new user {user.id}: {e}")
 
 
-# Conversation handler for adding servers
-add_server_conv_handler = ConversationHandler(
-    entry_points=[],  # Will be set in main.py
-    states={
-        WAITING_SERVER_NAME: [],
-        WAITING_API_URL: [],
-        WAITING_CERT_SHA256: [],
-    },
-    fallbacks=[],
-) 
+# Note: Conversation handler is properly configured in main.py 

bot/main.py

@@ -21,11 +21,13 @@
     approve_user,
     block_user,
     list_servers,
+    bot_status,
     start_add_server,
     get_server_name,
     get_api_url,
     get_cert_sha256,
     cancel_add_server,
+    timeout_add_server,
     remove_server,
     handle_approval_callback,
     WAITING_SERVER_NAME,
@@ -106,7 +108,15 @@ def _setup_handlers(self) -> None:
                 WAITING_API_URL: [MessageHandler(filters.TEXT & ~filters.COMMAND, get_api_url)],
                 WAITING_CERT_SHA256: [MessageHandler(filters.TEXT & ~filters.COMMAND, get_cert_sha256)],
             },
-            fallbacks=[CommandHandler("cancel", cancel_add_server)],
+            fallbacks=[
+                CommandHandler("cancel", cancel_add_server),
+                MessageHandler(filters.COMMAND, cancel_add_server)  # Cancel on any other command
+            ],
+            conversation_timeout=300,  # 5 minutes timeout
+            name="add_server_conversation",
+            persistent=False,
+            allow_reentry=True,
+            per_message=False
         )
 
         # User conversation handler for creating new keys
@@ -117,7 +127,15 @@ def _setup_handlers(self) -> None:
                 SELECTING_VALIDITY: [CallbackQueryHandler(handle_validity_selection)],
                 SELECTING_QUOTA: [CallbackQueryHandler(handle_quota_selection)],
             },
-            fallbacks=[CommandHandler("cancel", cancel_new_key)],
+            fallbacks=[
+                CommandHandler("cancel", cancel_new_key),
+                MessageHandler(filters.COMMAND, cancel_new_key)  # Cancel on any other command
+            ],
+            conversation_timeout=300,  # 5 minutes timeout
+            name="new_key_conversation",
+            persistent=False,
+            allow_reentry=True,
+            per_message=False
         )
 
         # Add conversation handlers
@@ -128,6 +146,7 @@ def _setup_handlers(self) -> None:
         self.application.add_handler(CommandHandler("approve", approve_user))
         self.application.add_handler(CommandHandler("block", block_user))
         self.application.add_handler(CommandHandler("servers", list_servers))
+        self.application.add_handler(CommandHandler("status", bot_status))
         self.application.add_handler(CommandHandler("removeserver", remove_server))
 
         # User command handlers