fix: Address CRITICAL and HIGH security audit findings

- CRIT-01: Scrub bot token from teloxide error messages to prevent credential leakage
- CRIT-02: Validate session_id length (128 max) and characters (alphanumeric/hyphens)
- HIGH-03: Verify approval session ownership matches authorized chat_id (prevents IDOR)
- HIGH-04: Add 1MiB per-line limit on NDJSON reader to prevent memory exhaustion
- HIGH-05: Limit concurrent socket connections to 64 via semaphore (DoS prevention)
- HIGH-06: Use i64 for polling offset to prevent overflow at i32::MAX
- MED-05: Set umask(0o177) before socket bind to prevent brief world-accessible window

Co-Authored-By: claude-flow <ruv@ruv.net>

Author: Robert E. Lee

src/bot.rs

@@ -8,6 +8,18 @@ use teloxide::types::{
     ChatId, InlineKeyboardButton, InlineKeyboardMarkup, MessageId, ParseMode, ThreadId,
 };
 
+/// Scrub bot token from error messages to prevent credential leakage.
+/// Teloxide wraps reqwest errors that include the full API URL with token.
+fn scrub_telegram_error(err: &teloxide::RequestError) -> String {
+    let raw = err.to_string();
+    // Telegram API URLs contain the token: /bot<TOKEN>/method
+    // Scrub anything that looks like a bot token (numeric:alphanumeric)
+    let re = regex::Regex::new(r"bot\d+:[A-Za-z0-9_-]+/").unwrap_or_else(|_| {
+        regex::Regex::new(r"$^").unwrap() // fallback: match nothing
+    });
+    re.replace_all(&raw, "bot<REDACTED>/").to_string()
+}
+
 /// Telegram Bot wrapper with rate limiting and forum topic support
 pub struct TelegramBot {
     bot: Bot,
@@ -71,7 +83,8 @@ impl TelegramBot {
             req = req.message_thread_id(ThreadId(MessageId(tid)));
         }
 
-        req.await.map_err(|e| AppError::Telegram(e.to_string()))
+        req.await
+            .map_err(|e| AppError::Telegram(scrub_telegram_error(&e)))
     }
 
     /// Send a message with inline keyboard buttons
@@ -115,7 +128,8 @@ impl TelegramBot {
             req = req.message_thread_id(ThreadId(MessageId(tid)));
         }
 
-        req.await.map_err(|e| AppError::Telegram(e.to_string()))
+        req.await
+            .map_err(|e| AppError::Telegram(scrub_telegram_error(&e)))
     }
 
     /// Create a forum topic
@@ -210,7 +224,8 @@ impl TelegramBot {
             req = req.parse_mode(pm);
         }
 
-        req.await.map_err(|e| AppError::Telegram(e.to_string()))?;
+        req.await
+            .map_err(|e| AppError::Telegram(scrub_telegram_error(&e)))?;
         Ok(())
     }
 
@@ -223,19 +238,20 @@ impl TelegramBot {
             req = req.text(t);
         }
 
-        req.await.map_err(|e| AppError::Telegram(e.to_string()))?;
+        req.await
+            .map_err(|e| AppError::Telegram(scrub_telegram_error(&e)))?;
         Ok(())
     }
 
     /// Poll for updates (long polling)
-    pub async fn get_updates(&self, offset: i32) -> Result<Vec<Update>> {
+    pub async fn get_updates(&self, offset: i64) -> Result<Vec<Update>> {
         let updates = self
             .bot
             .get_updates()
-            .offset(offset)
+            .offset(offset as i32)
             .timeout(30)
             .await
-            .map_err(|e| AppError::Telegram(e.to_string()))?;
+            .map_err(|e| AppError::Telegram(scrub_telegram_error(&e)))?;
 
         Ok(updates)
     }

src/bridge.rs

@@ -174,6 +174,21 @@ impl BridgeShared {
         msg: BridgeMessage,
         _broadcast_tx: &broadcast::Sender<BridgeMessage>,
     ) -> Result<()> {
+        // CRIT-02: Validate session_id to prevent unbounded memory growth
+        const MAX_SESSION_ID_LEN: usize = 128;
+        if msg.session_id.len() > MAX_SESSION_ID_LEN
+            || !msg
+                .session_id
+                .chars()
+                .all(|c| c.is_alphanumeric() || c == '-' || c == '_')
+        {
+            tracing::warn!(
+                len = msg.session_id.len(),
+                "Rejecting message with invalid session_id"
+            );
+            return Ok(());
+        }
+
         tracing::debug!(msg_type = ?msg.msg_type, session_id = %msg.session_id, "Socket message");
 
         // Update session activity
@@ -577,13 +592,14 @@ impl BridgeShared {
     // ============ Telegram Update Handling (Telegram -> CLI) ============
 
     async fn poll_telegram_updates(&self) {
-        let mut offset = 0i32;
+        let mut offset = 0i64;
 
         loop {
             match self.bot.get_updates(offset).await {
                 Ok(updates) => {
                     for update in &updates {
-                        offset = update.id.0 as i32 + 1;
+                        // HIGH-06: Use i64 to prevent u32->i32 overflow at i32::MAX
+                        offset = (update.id.0 as i64) + 1;
 
                         // Security fix #5: Chat ID filter on ALL updates
                         if !bot::is_authorized_chat(update, self.config.chat_id) {
@@ -826,6 +842,21 @@ impl BridgeShared {
                 let approval = sessions.get_approval(id);
 
                 if let Some(approval) = approval {
+                    // HIGH-03: Verify approval belongs to this chat (prevent IDOR)
+                    if let Some(session) = sessions.get_session(&approval.session_id) {
+                        if session.chat_id != self.config.chat_id {
+                            tracing::warn!(
+                                approval_id = %id,
+                                "Approval belongs to different chat, rejecting"
+                            );
+                            let _ = self
+                                .bot
+                                .answer_callback_query(&query.id, Some("Unauthorized"))
+                                .await;
+                            return;
+                        }
+                    }
+
                     let status = match action {
                         "approve" => "approved",
                         "reject" | "abort" => "rejected",

src/socket.rs

@@ -1,9 +1,11 @@
 use crate::error::{AppError, Result};
 use crate::types::BridgeMessage;
 use nix::fcntl::{Flock, FlockArg};
+use nix::sys::stat::{umask, Mode};
 use std::fs;
 use std::os::unix::fs::PermissionsExt;
 use std::path::PathBuf;
+use std::sync::Arc;
 use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader};
 use tokio::net::{UnixListener, UnixStream};
 use tokio::sync::{broadcast, mpsc};
@@ -131,13 +133,16 @@ impl SocketServer {
             fs::set_permissions(parent, fs::Permissions::from_mode(0o700))?;
         }
 
-        // Step 4: Bind the listener
+        // Step 4: Bind the listener with restrictive umask (MED-05: prevent brief 0755 window)
+        let old_mask = umask(Mode::from_bits_truncate(0o177));
         let listener = UnixListener::bind(&self.socket_path).map_err(|e| {
+            umask(old_mask);
             self.release_pid_lock();
             AppError::Io(e)
         })?;
+        umask(old_mask);
 
-        // Security fix #3: Set socket file permissions to 0o600
+        // Ensure socket file permissions are 0o600 (belt-and-suspenders with umask)
         fs::set_permissions(&self.socket_path, fs::Permissions::from_mode(0o600))?;
 
         tracing::info!(
@@ -151,14 +156,26 @@ impl SocketServer {
         let (broadcast_tx, _) = broadcast::channel::<BridgeMessage>(256);
         let broadcast_tx_clone = broadcast_tx.clone();
 
+        // HIGH-05: Limit concurrent connections to prevent DoS
+        let conn_semaphore = Arc::new(tokio::sync::Semaphore::new(64));
+
         // Accept connections in background
         tokio::spawn(async move {
             loop {
                 match listener.accept().await {
                     Ok((stream, _addr)) => {
                         let tx = msg_tx.clone();
                         let btx = broadcast_tx_clone.clone();
+                        let permit = match conn_semaphore.clone().try_acquire_owned() {
+                            Ok(p) => p,
+                            Err(_) => {
+                                tracing::warn!("Max connections reached, rejecting");
+                                drop(stream);
+                                continue;
+                            }
+                        };
                         tokio::spawn(async move {
+                            let _permit = permit; // released when task ends
                             if let Err(e) = handle_client_connection(stream, tx, btx).await {
                                 tracing::debug!("Client connection ended: {}", e);
                             }
@@ -213,11 +230,16 @@ async fn handle_client_connection(
     });
 
     // Read NDJSON lines from client
+    const MAX_LINE_BYTES: usize = 1_048_576; // HIGH-04: 1 MiB per-line limit
     while let Ok(Some(line)) = lines.next_line().await {
         let line = line.trim().to_string();
         if line.is_empty() {
             continue;
         }
+        if line.len() > MAX_LINE_BYTES {
+            tracing::warn!(len = line.len(), "Oversized NDJSON line, dropping");
+            continue;
+        }
 
         // Security fix #10: serde_json::from_str returns Result, no unwrap/panic
         match serde_json::from_str::<BridgeMessage>(&line) {