feat: Implement PTY/shell session support for server (#129) (#153)

* feat: Implement PTY/shell session support for server (#129)

Add Unix PTY handling for interactive shell sessions in bssh-server:
- Create pty.rs module for PTY master/slave pair management with async I/O
- Create shell.rs module for shell process spawning and I/O forwarding
- Implement shell_request handler to start interactive shell sessions
- Implement window_change_request handler for terminal resizing
- Update data handler to forward SSH data to active shell sessions
- Add shell session state tracking to ChannelState
- Enable nix crate 'term' and 'fs' features for PTY operations

* fix(security): Fix PTY fd handling and add shell validation

Priority: CRITICAL/MEDIUM
Issues addressed:
- Fixed file descriptor leak via mem::forget by using dup() for each stdio fd
- Added shell path validation before spawning process
- Added bounds checking for window size dimensions (clamp to u16::MAX)
- Improved error context for TIOCSWINSZ ioctl failures
- Fixed clippy warning for const assertions in tests

Review-Iteration: 1

* test(server): Add comprehensive tests for PTY/shell modules

- Add unit tests for PTY configuration, winsize, and boundary values
- Add unit tests for PTY master operations (resize, config, path)
- Add tests for shell session structures and validation
- Update ARCHITECTURE.md with PTY/shell module documentation
- Update server-configuration.md with shell session architecture

Author: inureyes

ARCHITECTURE.md

@@ -289,6 +289,21 @@ SSH server implementation using the russh library for accepting incoming connect
   - PTY, exec, shell, and subsystem request handling
   - Command execution with stdout/stderr streaming
 
+- **PTY Module** (`src/server/pty.rs`): Pseudo-terminal management for interactive sessions
+  - PTY master/slave pair creation using POSIX APIs via nix crate
+  - Window size management with TIOCSWINSZ ioctl
+  - Async I/O for PTY master file descriptor using tokio's AsyncFd
+  - Configuration management (terminal type, dimensions, pixel sizes)
+  - Implements `AsyncRead` and `AsyncWrite` for PTY I/O
+
+- **Shell Session Module** (`src/server/shell.rs`): Interactive shell session handler
+  - Shell process spawning with login shell configuration (-l flag)
+  - Terminal environment setup (TERM, HOME, USER, SHELL, PATH)
+  - Bidirectional I/O forwarding between SSH channel and PTY master
+  - Window resize event handling forwarded to PTY
+  - Proper session cleanup on disconnect (SIGHUP to shell, process termination)
+  - Controlling terminal setup via TIOCSCTTY ioctl
+
 - **CommandExecutor**: Executes commands requested by SSH clients
   - Shell-based command execution with `-c` flag
   - Environment variable configuration (HOME, USER, SHELL, PATH)

Cargo.toml

@@ -43,7 +43,7 @@ regex = "1.12.2"
 lazy_static = "1.5"
 ctrlc = "3.5.1"
 signal-hook = "0.4.1"
-nix = { version = "0.30", features = ["poll", "process", "signal"] }
+nix = { version = "0.30", features = ["fs", "poll", "process", "signal", "term"] }
 atty = "0.2.14"
 arrayvec = "0.7.6"
 smallvec = "1.15.1"

docs/architecture/server-configuration.md

@@ -403,6 +403,89 @@ bssh-server -c /etc/bssh/server.yaml -p 2222 -b 0.0.0.0
 bssh-server -c /etc/bssh/server.yaml -D -vvv
 ```
 
+## Shell Session Architecture
+
+The bssh-server supports interactive shell sessions through a PTY (pseudo-terminal) subsystem. This enables users to connect and run interactive programs like vim, top, or bash.
+
+### PTY Management
+
+The PTY module (`src/server/pty.rs`) handles pseudo-terminal operations:
+
+**Key Components:**
+- **PtyMaster**: Manages the master side of a PTY pair
+  - Opens PTY pair using `openpty()` from the nix crate
+  - Provides async I/O via tokio's `AsyncFd`
+  - Handles window resize events with `TIOCSWINSZ` ioctl
+  - Configurable terminal type and dimensions
+
+**Configuration:**
+```rust
+use bssh::server::pty::{PtyConfig, PtyMaster};
+
+// Create PTY with custom configuration
+let config = PtyConfig::new(
+    "xterm-256color".to_string(),  // Terminal type
+    80,   // Columns
+    24,   // Rows
+    0,    // Pixel width (optional)
+    0,    // Pixel height (optional)
+);
+
+let pty = PtyMaster::open(config)?;
+```
+
+### Shell Session Handler
+
+The shell module (`src/server/shell.rs`) manages interactive SSH shell sessions:
+
+**Features:**
+- Spawns user's login shell with `-l` flag
+- Sets up proper terminal environment (TERM, HOME, USER, SHELL, PATH)
+- Creates new session and sets controlling terminal (setsid, TIOCSCTTY)
+- Bidirectional I/O forwarding between SSH channel and PTY
+- Window resize event forwarding
+- Graceful shutdown with process cleanup
+
+**Session Lifecycle:**
+1. SSH client sends `pty-request` with terminal configuration
+2. SSH client sends `shell` request
+3. Server creates PTY pair and spawns shell process
+4. I/O forwarding tasks handle data flow:
+   - PTY master -> SSH channel (stdout/stderr)
+   - SSH channel -> PTY master (stdin)
+5. Window resize events update PTY dimensions
+6. On disconnect, shell process receives SIGHUP
+
+**Platform Support:**
+- Unix/Linux: Full support using POSIX PTY APIs
+- Windows: Not yet supported (would require ConPTY)
+
+### SSH Handler Integration
+
+The `SshHandler` orchestrates shell sessions through several handler methods:
+
+```
+SSH Client Request Flow:
+┌───────────────┐     ┌─────────────────┐     ┌──────────────────┐
+│  pty_request  │ --> │ Store PtyConfig │ --> │ channel_success  │
+└───────────────┘     └─────────────────┘     └──────────────────┘
+        │
+        v
+┌───────────────┐     ┌─────────────────┐     ┌──────────────────┐
+│ shell_request │ --> │ Create Session  │ --> │ Start I/O Tasks  │
+└───────────────┘     └─────────────────┘     └──────────────────┘
+        │
+        v
+┌───────────────┐     ┌─────────────────┐     ┌──────────────────┐
+│     data      │ --> │ Forward to PTY  │ --> │  User Typing     │
+└───────────────┘     └─────────────────┘     └──────────────────┘
+        │
+        v
+┌───────────────────┐     ┌─────────────────┐     ┌──────────────┐
+│ window_change_req │ --> │ Resize PTY      │ --> │ TIOCSWINSZ   │
+└───────────────────┘     └─────────────────┘     └──────────────┘
+```
+
 ---
 
 **Related Documentation:**

src/server/handler.rs

@@ -31,8 +31,10 @@ use zeroize::Zeroizing;
 use super::auth::AuthProvider;
 use super::config::ServerConfig;
 use super::exec::CommandExecutor;
+use super::pty::PtyConfig as PtyMasterConfig;
 use super::session::{ChannelState, PtyConfig, SessionId, SessionInfo, SessionManager};
 use super::sftp::SftpHandler;
+use super::shell::ShellSession;
 use crate::shared::rate_limit::RateLimiter;
 
 /// SSH handler for a single client connection.
@@ -716,22 +718,124 @@ impl russh::server::Handler for SshHandler {
 
     /// Handle shell request.
     ///
-    /// Placeholder implementation - will be implemented in a future issue.
+    /// Starts an interactive shell session for the authenticated user.
     fn shell_request(
         &mut self,
         channel_id: ChannelId,
         session: &mut Session,
     ) -> impl std::future::Future<Output = Result<(), Self::Error>> + Send {
-        tracing::debug!("Shell request");
+        tracing::debug!(channel = ?channel_id, "Shell request");
 
-        if let Some(channel_state) = self.channels.get_mut(&channel_id) {
-            channel_state.set_shell();
-        }
+        // Get authenticated user info
+        let username = match self.session_info.as_ref().and_then(|s| s.user.clone()) {
+            Some(user) => user,
+            None => {
+                tracing::warn!(
+                    channel = ?channel_id,
+                    "Shell request without authenticated user"
+                );
+                let _ = session.channel_failure(channel_id);
+                return async { Ok(()) }.boxed();
+            }
+        };
 
-        // Placeholder - reject for now
-        // Will be implemented in #129
-        let _ = session.channel_failure(channel_id);
-        async { Ok(()) }
+        // Get PTY configuration (if set during pty_request)
+        let pty_config = self
+            .channels
+            .get(&channel_id)
+            .and_then(|state| state.pty.as_ref())
+            .map(|pty| {
+                PtyMasterConfig::new(
+                    pty.term.clone(),
+                    pty.col_width,
+                    pty.row_height,
+                    pty.pix_width,
+                    pty.pix_height,
+                )
+            })
+            .unwrap_or_default();
+
+        // Clone what we need for the async block
+        let auth_provider = Arc::clone(&self.auth_provider);
+        let handle = session.handle();
+        let peer_addr = self.peer_addr;
+
+        // Get mutable reference to channel state
+        let channels = &mut self.channels;
+
+        // Signal success before starting shell
+        let _ = session.channel_success(channel_id);
+
+        async move {
+            // Get user info from auth provider
+            let user_info = match auth_provider.get_user_info(&username).await {
+                Ok(Some(info)) => info,
+                Ok(None) => {
+                    tracing::error!(
+                        user = %username,
+                        "User not found after authentication for shell"
+                    );
+                    let _ = handle.close(channel_id).await;
+                    return Ok(());
+                }
+                Err(e) => {
+                    tracing::error!(
+                        user = %username,
+                        error = %e,
+                        "Failed to get user info for shell"
+                    );
+                    let _ = handle.close(channel_id).await;
+                    return Ok(());
+                }
+            };
+
+            tracing::info!(
+                user = %username,
+                peer = ?peer_addr,
+                term = %pty_config.term,
+                size = %format!("{}x{}", pty_config.col_width, pty_config.row_height),
+                "Starting shell session"
+            );
+
+            // Create shell session
+            let mut shell_session = match ShellSession::new(channel_id, pty_config) {
+                Ok(session) => session,
+                Err(e) => {
+                    tracing::error!(
+                        user = %username,
+                        error = %e,
+                        "Failed to create shell session"
+                    );
+                    let _ = handle.close(channel_id).await;
+                    return Ok(());
+                }
+            };
+
+            // Start shell session
+            if let Err(e) = shell_session.start(&user_info, handle.clone()).await {
+                tracing::error!(
+                    user = %username,
+                    error = %e,
+                    "Failed to start shell session"
+                );
+                let _ = handle.close(channel_id).await;
+                return Ok(());
+            }
+
+            // Store shell session in channel state
+            if let Some(channel_state) = channels.get_mut(&channel_id) {
+                channel_state.set_shell_session(shell_session);
+            }
+
+            tracing::info!(
+                user = %username,
+                peer = ?peer_addr,
+                "Shell session started"
+            );
+
+            Ok(())
+        }
+        .boxed()
     }
 
     /// Handle subsystem request.
@@ -849,19 +953,98 @@ impl russh::server::Handler for SshHandler {
     }
 
     /// Handle incoming data from the client.
+    ///
+    /// Forwards data to the active shell session if one exists.
     fn data(
         &mut self,
-        _channel_id: ChannelId,
+        channel_id: ChannelId,
         data: &[u8],
         _session: &mut Session,
     ) -> impl std::future::Future<Output = Result<(), Self::Error>> + Send {
         tracing::trace!(
+            channel = ?channel_id,
             bytes = %data.len(),
             "Received data"
         );
 
-        // Placeholder - data handling will be implemented with exec/shell/sftp
-        async { Ok(()) }
+        // Get the data sender if there's an active shell session
+        let data_sender = self
+            .channels
+            .get(&channel_id)
+            .and_then(|state| state.shell_session.as_ref())
+            .and_then(|shell| shell.data_sender());
+
+        if let Some(tx) = data_sender {
+            let data = data.to_vec();
+            return async move {
+                if let Err(e) = tx.send(data).await {
+                    tracing::debug!(
+                        channel = ?channel_id,
+                        error = %e,
+                        "Error forwarding data to shell"
+                    );
+                }
+                Ok(())
+            }
+            .boxed();
+        }
+
+        async { Ok(()) }.boxed()
+    }
+
+    /// Handle window size change request.
+    ///
+    /// Updates the PTY window size for active shell sessions.
+    #[allow(clippy::too_many_arguments)]
+    fn window_change_request(
+        &mut self,
+        channel_id: ChannelId,
+        col_width: u32,
+        row_height: u32,
+        pix_width: u32,
+        pix_height: u32,
+        _session: &mut Session,
+    ) -> impl std::future::Future<Output = Result<(), Self::Error>> + Send {
+        tracing::debug!(
+            channel = ?channel_id,
+            cols = col_width,
+            rows = row_height,
+            "Window change request"
+        );
+
+        // Update stored PTY config
+        if let Some(state) = self.channels.get_mut(&channel_id) {
+            if let Some(ref mut pty) = state.pty {
+                pty.col_width = col_width;
+                pty.row_height = row_height;
+                pty.pix_width = pix_width;
+                pty.pix_height = pix_height;
+            }
+        }
+
+        // Get the PTY mutex if there's an active shell session
+        let pty_mutex = self
+            .channels
+            .get(&channel_id)
+            .and_then(|state| state.shell_session.as_ref())
+            .map(|shell| Arc::clone(shell.pty()));
+
+        if let Some(pty) = pty_mutex {
+            return async move {
+                let mut pty_guard = pty.lock().await;
+                if let Err(e) = pty_guard.resize(col_width, row_height) {
+                    tracing::debug!(
+                        channel = ?channel_id,
+                        error = %e,
+                        "Error resizing shell PTY"
+                    );
+                }
+                Ok(())
+            }
+            .boxed();
+        }
+
+        async { Ok(()) }.boxed()
     }
 
     /// Handle channel EOF from the client.

src/server/mod.rs

@@ -48,8 +48,10 @@ pub mod auth;
 pub mod config;
 pub mod exec;
 pub mod handler;
+pub mod pty;
 pub mod session;
 pub mod sftp;
+pub mod shell;
 
 use std::net::SocketAddr;
 use std::path::Path;
@@ -66,9 +68,11 @@ use crate::shared::rate_limit::RateLimiter;
 pub use self::config::{ServerConfig, ServerConfigBuilder};
 pub use self::exec::{CommandExecutor, ExecConfig};
 pub use self::handler::SshHandler;
+pub use self::pty::{PtyConfig as PtyMasterConfig, PtyMaster};
 pub use self::session::{
     ChannelMode, ChannelState, PtyConfig, SessionId, SessionInfo, SessionManager,
 };
+pub use self::shell::ShellSession;
 
 /// The main SSH server struct.
 ///