Copilot commented on code in PR #3547: URL: https://github.com/apache/hertzbeat/pull/3547#discussion_r2244142452
########## mcp-servers/mcp-bash-server/Dockerfile: ########## @@ -0,0 +1,100 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Multi-stage build for mcp-bash-server +# Stage 1: Build stage +FROM ubuntu:noble AS builder + +# Set environment variables +ENV DEBIAN_FRONTEND=noninteractive +ENV RUST_VERSION=stable +ENV PATH="/root/.cargo/bin:${PATH}" + +# Set proxy environment variables if needed +# Uncomment the following lines if you need to use a proxy +ENV https_proxy=http://172.17.0.1:7897 +ENV http_proxy=http://172.17.0.1:7897 Review Comment: Hardcoded proxy configuration in production Dockerfile poses security risks and may expose internal network topology. Consider removing these proxy settings or making them configurable via build arguments. ```suggestion # Use build arguments to configure proxy settings dynamically ARG https_proxy ARG http_proxy ENV https_proxy=${https_proxy} ENV http_proxy=${http_proxy} ``` ########## mcp-servers/mcp-bash-server/src/common/validator.rs: ########## @@ -0,0 +1,450 @@ +//! Command validation module for security enforcement +//! +//! This module provides command validation functionality to prevent execution +//! of dangerous commands and operations based on configurable blacklists. + +use std::ffi::OsStr; +use tracing::debug; + +use crate::{common::config::Whitelist, config::Blacklist}; +use rmcp::model::ErrorData; +use tracing::error; + +/// Command validator that checks commands against security blacklists +/// Prevents execution of dangerous commands and operations +#[derive(Debug, Clone)] +pub struct Validator { + /// Security blacklist configuration containing forbidden commands and operations + blacklist: Blacklist, + /// Security whitelist configuration containing secure commands + whitelist: Whitelist, +} + +impl Validator { + /// Create a new validator with the specified blacklist configuration + pub fn new(blacklist: Blacklist, whitelist: Whitelist) -> Self { + Validator { + blacklist, + whitelist, + } + } + + /// Validate a command against the security blacklist and whitelist + /// Returns Ok(()) if command is safe, Err(ErrorData) if command is blocked + /// + /// Security Logic: + /// 1. All commands are denied by default + /// 2. Blacklist has higher priority than whitelist + /// 3. If command matches any blacklist pattern (exact or regex), deny + /// 4. If command doesn't match blacklist and matches whitelist (exact or regex), allow + /// 5. Otherwise, deny + pub fn is_unsafe_command(&self, args: Vec<&OsStr>) -> Result<(), ErrorData> { + // Handle empty command - deny by default + if args.is_empty() { + return Err(ErrorData::invalid_request( + "Empty command not allowed".to_string(), + None, + )); + } + + let full_cmd = args + .join(OsStr::new(" ")) + .into_string() + .expect("Convert OsStr to String failed!"); + Review Comment: The error message "Convert OsStr to String failed!" is not helpful for debugging. Consider providing more context about why the conversion failed and what valid input is expected. ```suggestion .map_err(|os_str| { let invalid_input = format!("{:?}", os_str); error!( "Failed to convert OsStr to String. Invalid input: {}", invalid_input ); ErrorData::invalid_request( format!("Failed to convert command to String. Invalid input: {}", invalid_input), None, ) })?; ``` ########## mcp-servers/mcp-bash-server/Dockerfile: ########## @@ -0,0 +1,100 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Multi-stage build for mcp-bash-server +# Stage 1: Build stage +FROM ubuntu:noble AS builder + +# Set environment variables +ENV DEBIAN_FRONTEND=noninteractive +ENV RUST_VERSION=stable +ENV PATH="/root/.cargo/bin:${PATH}" + +# Set proxy environment variables if needed +# Uncomment the following lines if you need to use a proxy +ENV https_proxy=http://172.17.0.1:7897 +ENV http_proxy=http://172.17.0.1:7897 Review Comment: Hardcoded proxy configuration in production Dockerfile poses security risks and may expose internal network topology. Consider removing these proxy settings or making them configurable via build arguments. ```suggestion # Set proxy environment variables via build arguments ARG HTTPS_PROXY ARG HTTP_PROXY ENV https_proxy=${HTTPS_PROXY} ENV http_proxy=${HTTP_PROXY} ``` ########## mcp-servers/mcp-bash-server/src/common/oauth.rs: ########## @@ -0,0 +1,1011 @@ +//! OAuth2 authentication implementation for MCP server +//! +//! This module provides OAuth2 authentication capabilities including: +//! - Client registration and validation +//! - Authorization code flow +//! - Token management and validation +//! - Session management for auth flows +//! - Middleware for request authentication + +use std::{collections::HashMap, sync::Arc}; + +use askama::Template; +use axum::{ + Json, + body::Body, + extract::{Form, Query, State}, + http::{HeaderMap, Request, StatusCode}, + middleware::Next, + response::{Html, IntoResponse, Redirect, Response}, +}; +use chrono; +use oauth2::{AccessToken, EmptyExtraTokenFields, RefreshToken, StandardTokenResponse}; +use rand::{Rng, distributions::Alphanumeric}; +use rmcp::serde_json::{self, Value}; +use rmcp::transport::auth::{ + AuthorizationMetadata, ClientRegistrationRequest, ClientRegistrationResponse, OAuthClientConfig, +}; +use serde::{Deserialize, Serialize}; +use tokio::sync::RwLock; +use tracing::{debug, error, info, warn}; +use uuid::Uuid; + +/// Type alias for OAuth2 standard token response +/// Type alias for OAuth2 standard token response +pub type AuthToken = StandardTokenResponse<EmptyExtraTokenFields, oauth2::basic::BasicTokenType>; + +/// Centralized OAuth store for managing clients, sessions, and tokens +/// Provides thread-safe access to OAuth-related data structures +#[derive(Clone, Debug)] +pub struct McpOAuthStore { + /// Registered OAuth clients with their configurations + pub clients: Arc<RwLock<HashMap<String, OAuthClientConfig>>>, + /// Active authorization sessions indexed by session ID + pub auth_sessions: Arc<RwLock<HashMap<String, AuthSession>>>, + /// Valid access tokens indexed by token string + pub access_tokens: Arc<RwLock<HashMap<String, McpAccessToken>>>, +} + +impl McpOAuthStore { + /// Create a new OAuth store with a default client configuration + pub fn new() -> Self { + let mut clients = HashMap::new(); + clients.insert( + "mcp-client".to_string(), + OAuthClientConfig { + client_id: "mcp-client".to_string(), + client_secret: Some("mcp-client-secret".to_string()), + scopes: vec!["profile".to_string(), "email".to_string()], + redirect_uri: "http://localhost:8080/callback".to_string(), + }, + ); + + Self { + clients: Arc::new(RwLock::new(clients)), + auth_sessions: Arc::new(RwLock::new(HashMap::new())), + access_tokens: Arc::new(RwLock::new(HashMap::new())), + } + } + + /// Validate client credentials and redirect URI + /// Returns Some(client_config) if valid, None otherwise + pub async fn validate_client( + &self, + client_id: &str, + redirect_uri: &str, + ) -> Option<OAuthClientConfig> { + let clients = self.clients.read().await; + if let Some(client) = clients.get(client_id) { + if client.redirect_uri.contains(&redirect_uri.to_string()) { Review Comment: Using .contains() for redirect URI validation is insecure and vulnerable to open redirect attacks. An attacker could use 'http://evil.com/steal-tokens?http://localhost:8080/callback' which would pass this check. Use exact string comparison instead. ```suggestion if client.redirect_uri == redirect_uri { ``` ########## mcp-servers/mcp-bash-server/src/common/bash_server.rs: ########## @@ -0,0 +1,943 @@ +//! Bash Server Implementation for MCP (Model Context Protocol) +//! +//! This module provides a bash command execution server that can run shell commands +//! safely with validation and timeout controls. It supports multiple operating systems +//! and provides various execution methods for different use cases. + +use rmcp::model::Content; +use rmcp::{ + RoleServer, ServerHandler, + handler::server::{ + router::tool::ToolRouter, + tool::{IntoCallToolResult, Parameters}, + }, + model::*, + schemars, + serde_json::Value, + service::RequestContext, + tool, +}; +use rmcp::{serde_json, tool_handler, tool_router}; +use serde::{Deserialize, Serialize}; +use std::ffi::OsStr; +use std::process::Output; +use std::{ + borrow::Cow, + env, + fs::{self, File}, + io::Write, + os::unix::fs::PermissionsExt, + process::Command, +}; +use tracing::error; +use tracing::info; +use uuid::Uuid; + +use crate::common::config::Config; +use crate::common::validator::Validator; + +/// Request structure for executing bash commands +/// Contains all necessary parameters for command execution including +/// working directory, environment variables, and timeout settings +#[derive(Debug, Deserialize, schemars::JsonSchema)] +pub struct DefaultExecuteRequest { + #[schemars(description = "The bash command or script to execute")] + pub command: String, + #[schemars(description = "Working directory for the command (optional)")] + pub working_dir: Option<String>, + #[schemars(description = "Environment variables (optional)")] + pub env_vars: Option<std::collections::HashMap<String, String>>, + #[schemars(description = "Timeout in seconds (default: 30)")] + pub timeout_seconds: Option<u64>, +} + +/// Response structure for command execution results +/// Contains stdout, stderr, exit code, success status and any parsed data +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct DefaultExecuteResponse { + /// Standard output from the executed command + pub stdout: String, + /// Standard error output from the executed command + pub stderr: String, + /// Exit code returned by the command (0 for success, non-zero for failure) + pub exit_code: i32, + /// Whether the command executed successfully (exit code 0) + pub success: bool, + /// Additional parsed data from command output (JSON format) + pub parsed_data: serde_json::Value, +} + +impl IntoCallToolResult for DefaultExecuteResponse { + fn into_call_tool_result(self) -> Result<CallToolResult, ErrorData> { + let content = if self.success { + format!( + "Command executed successfully (exit code: {})\n\nSTDOUT:\n{}\n\nSTDERR:\n{}", + self.exit_code, self.stdout, self.stderr + ) + } else { + format!( + "Command failed (exit code: {})\n\nSTDOUT:\n{}\n\nSTDERR:\n{}", + self.exit_code, self.stdout, self.stderr + ) + }; + + Ok(CallToolResult { + content: vec![Content::text(content)], + is_error: Some(!self.success), + }) + } +} + +/// Main bash server implementation that handles command execution +/// with optional validation and timeout controls +#[derive(Debug, Clone)] +pub struct BashServer { + validator: Option<Validator>, + tool_router: ToolRouter<BashServer>, +} + +/// Trait for command execution utilities +/// Provides common functionality for command formatting and execution +pub trait CommandRunner { + /// Convert a Command instance to a readable string representation + /// Handles proper quoting of arguments containing spaces + fn stringify_command(cmd: &Command) -> String { + let program = cmd.get_program().to_string_lossy(); + let args = cmd + .get_args() + .map(|arg| { + let s = arg.to_string_lossy(); + if s.contains(' ') || s.contains('"') { + format!("{s:?}") + } else { + s.to_string() + } + }) + .collect::<Vec<String>>() + .join(" "); + + format!("{program} {args}") + } + + /// Execute a command with timeout protection + /// Returns command output or timeout error + async fn execute_command_with_timeout( + timeout: std::time::Duration, + mut cmd: Command, + ) -> Result<Output, ErrorData> { + let cmd_str = Self::stringify_command(&cmd); + // Execute command with timeout + let output = tokio::time::timeout(timeout, async { + tokio::task::spawn_blocking(move || cmd.output()).await + }) + .await + .map_err(|_| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::Owned("Command execution timed out".to_string()), + data: None, + })? + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::Owned(format!("Failed to spawn command: {e}")), + data: None, + })? + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::Owned(format!("Command execution failed: {e}")), + data: None, + })?; + + // log execution of command + info!("Execute command: {cmd_str}"); + Ok(output) + } +} + +impl CommandRunner for BashServer {} + +#[tool_router] +impl BashServer { + /// Create a new BashServer instance + /// Attempts to load configuration from config.toml, creates validator if successful + pub fn new() -> Self { + let tool_router = Self::tool_router(); + if let Ok(config) = Config::read_config("config.toml") + .inspect_err(|e| eprintln!("read config.toml fail, error: {e}")) + { + let blacklist = config.blacklist; + let whitelist = config.whitelist; + BashServer { + validator: Some(Validator::new(blacklist, whitelist)), + tool_router, + } + } else { + Self { + validator: None, + tool_router, + } + } + } + + /// Internal method for executing commands via default shell + /// Supports validation toggle and handles cross-platform shell differences + async fn _all_execute_via_default_shell( + &self, + need_validate: bool, + request: DefaultExecuteRequest, + ) -> Result<CallToolResult, ErrorData> { + let timeout_duration = + std::time::Duration::from_secs(request.timeout_seconds.unwrap_or(30)); + + let mut cmd = if cfg!(target_os = "linux") { + let mut cmd = Command::new("bash"); + cmd.arg("-c"); + cmd + } else if cfg!(target_os = "windows") { + let mut cmd = Command::new("powershell"); + cmd.arg("-c"); + cmd + } else { + let mut cmd = Command::new("sh"); + cmd.arg("-c"); + cmd + }; + cmd.arg(&request.command); + + // Set working directory if provided + if let Some(working_dir) = &request.working_dir { + cmd.current_dir(working_dir); + } + + // Set environment variables if provided + if let Some(env_vars) = &request.env_vars { + for (key, value) in env_vars { + cmd.env(key, value); + } + } + + // Validate the commands + if let Some(validator) = &self.validator + && need_validate + { + let program = cmd.get_program(); + let mut full_args: Vec<&OsStr> = vec![program]; + let args: Vec<&OsStr> = cmd.get_args().collect(); + full_args.extend(args); + validator.is_unsafe_command(full_args)?; + } + + let output: Output = Self::execute_command_with_timeout(timeout_duration, cmd).await?; + + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + let exit_code = output.status.code().unwrap_or(-1); + let success = output.status.success(); + + let response = DefaultExecuteResponse { + stdout, + stderr, + exit_code, + success, + parsed_data: Value::Null, + }; + Ok(CallToolResult::success(vec![Content::json(response)?])) + } + + #[tool(description = "Execute commands using default shell in all kinds of os")] + async fn all_execute_via_default_shell( + &self, + Parameters(request): Parameters<DefaultExecuteRequest>, + ) -> Result<CallToolResult, ErrorData> { + self._all_execute_via_default_shell(true, request).await + } + + #[tool(description = "Execute a python script")] + async fn unix_execute_python( + &self, + Parameters(request): Parameters<DefaultExecuteRequest>, + ) -> Result<CallToolResult, ErrorData> { + let timeout_duration = + std::time::Duration::from_secs(request.timeout_seconds.unwrap_or(30)); + + // Check if /usr/bin/env exists before proceeding + let env_exists = std::path::Path::new("/usr/bin/env").exists(); + if !env_exists { + return Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::Owned( + "Python execution failed: /usr/bin/env not found on system".to_string(), + ), + data: None, + }); + } + + let mut cmd = Command::new("/usr/bin/env"); + cmd.arg("python3").arg("-c").arg(&request.command); + + // Set working directory if provided + if let Some(working_dir) = &request.working_dir { + cmd.current_dir(working_dir); + } + + // Set environment variables if provided + if let Some(env_vars) = &request.env_vars { + for (key, value) in env_vars { + cmd.env(key, value); + } + } + + let output: Output = Self::execute_command_with_timeout(timeout_duration, cmd).await?; + + // log the execution of python + info!("Execute python script: {}", &request.command); + + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + let exit_code = output.status.code().unwrap_or(-1); + let success = output.status.success(); + + let response = DefaultExecuteResponse { + stdout, + stderr, + exit_code, + success, + parsed_data: Value::Null, + }; + Ok(CallToolResult::success(vec![Content::json(response)?])) + } + + /// Execute a Unix script by writing it to a temporary file + /// Creates a temporary shell script file with proper permissions and executes it + #[tool(description = "Execute a unix script")] + async fn unix_execute_script( + &self, + Parameters(request): Parameters<DefaultExecuteRequest>, + ) -> Result<CallToolResult, ErrorData> { + let timeout_duration = + std::time::Duration::from_secs(request.timeout_seconds.unwrap_or(30)); + + // Write the string to a temporary file + let tmp_dir = env::temp_dir(); + // Generate a random script name + let tmp_name = Uuid::new_v4().to_string(); + let script_path = tmp_dir.join(format!("{tmp_name}.sh")); + { + let mut file = + File::create(&script_path).expect("Can not create temporary script file"); + file.write_all(request.command.as_bytes()) + .expect("Write to script file error"); + // Set execution mode + let mut perms = file.metadata().unwrap().permissions(); + perms.set_mode(0o755); + fs::set_permissions(&script_path, perms).expect("Set mode 755 fail"); Review Comment: Using .expect() for temporary file creation can cause panics and may leave temporary files in an inconsistent state. Consider using proper error handling with Result types. ```suggestion let mut file = File::create(&script_path) .map_err(|e| ErrorData::new(format!("Failed to create temporary script file: {}", e)))?; file.write_all(request.command.as_bytes()) .map_err(|e| ErrorData::new(format!("Failed to write to script file: {}", e)))?; // Set execution mode let mut perms = file.metadata() .map_err(|e| ErrorData::new(format!("Failed to retrieve file metadata: {}", e)))? .permissions(); perms.set_mode(0o755); fs::set_permissions(&script_path, perms) .map_err(|e| ErrorData::new(format!("Failed to set file permissions: {}", e)))?; ``` ########## mcp-servers/mcp-bash-server/src/common/bash_server.rs: ########## @@ -0,0 +1,943 @@ +//! Bash Server Implementation for MCP (Model Context Protocol) +//! +//! This module provides a bash command execution server that can run shell commands +//! safely with validation and timeout controls. It supports multiple operating systems +//! and provides various execution methods for different use cases. + +use rmcp::model::Content; +use rmcp::{ + RoleServer, ServerHandler, + handler::server::{ + router::tool::ToolRouter, + tool::{IntoCallToolResult, Parameters}, + }, + model::*, + schemars, + serde_json::Value, + service::RequestContext, + tool, +}; +use rmcp::{serde_json, tool_handler, tool_router}; +use serde::{Deserialize, Serialize}; +use std::ffi::OsStr; +use std::process::Output; +use std::{ + borrow::Cow, + env, + fs::{self, File}, + io::Write, + os::unix::fs::PermissionsExt, + process::Command, +}; +use tracing::error; +use tracing::info; +use uuid::Uuid; + +use crate::common::config::Config; +use crate::common::validator::Validator; + +/// Request structure for executing bash commands +/// Contains all necessary parameters for command execution including +/// working directory, environment variables, and timeout settings +#[derive(Debug, Deserialize, schemars::JsonSchema)] +pub struct DefaultExecuteRequest { + #[schemars(description = "The bash command or script to execute")] + pub command: String, + #[schemars(description = "Working directory for the command (optional)")] + pub working_dir: Option<String>, + #[schemars(description = "Environment variables (optional)")] + pub env_vars: Option<std::collections::HashMap<String, String>>, + #[schemars(description = "Timeout in seconds (default: 30)")] + pub timeout_seconds: Option<u64>, +} + +/// Response structure for command execution results +/// Contains stdout, stderr, exit code, success status and any parsed data +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct DefaultExecuteResponse { + /// Standard output from the executed command + pub stdout: String, + /// Standard error output from the executed command + pub stderr: String, + /// Exit code returned by the command (0 for success, non-zero for failure) + pub exit_code: i32, + /// Whether the command executed successfully (exit code 0) + pub success: bool, + /// Additional parsed data from command output (JSON format) + pub parsed_data: serde_json::Value, +} + +impl IntoCallToolResult for DefaultExecuteResponse { + fn into_call_tool_result(self) -> Result<CallToolResult, ErrorData> { + let content = if self.success { + format!( + "Command executed successfully (exit code: {})\n\nSTDOUT:\n{}\n\nSTDERR:\n{}", + self.exit_code, self.stdout, self.stderr + ) + } else { + format!( + "Command failed (exit code: {})\n\nSTDOUT:\n{}\n\nSTDERR:\n{}", + self.exit_code, self.stdout, self.stderr + ) + }; + + Ok(CallToolResult { + content: vec![Content::text(content)], + is_error: Some(!self.success), + }) + } +} + +/// Main bash server implementation that handles command execution +/// with optional validation and timeout controls +#[derive(Debug, Clone)] +pub struct BashServer { + validator: Option<Validator>, + tool_router: ToolRouter<BashServer>, +} + +/// Trait for command execution utilities +/// Provides common functionality for command formatting and execution +pub trait CommandRunner { + /// Convert a Command instance to a readable string representation + /// Handles proper quoting of arguments containing spaces + fn stringify_command(cmd: &Command) -> String { + let program = cmd.get_program().to_string_lossy(); + let args = cmd + .get_args() + .map(|arg| { + let s = arg.to_string_lossy(); + if s.contains(' ') || s.contains('"') { + format!("{s:?}") + } else { + s.to_string() + } + }) + .collect::<Vec<String>>() + .join(" "); + + format!("{program} {args}") + } + + /// Execute a command with timeout protection + /// Returns command output or timeout error + async fn execute_command_with_timeout( + timeout: std::time::Duration, + mut cmd: Command, + ) -> Result<Output, ErrorData> { + let cmd_str = Self::stringify_command(&cmd); + // Execute command with timeout + let output = tokio::time::timeout(timeout, async { + tokio::task::spawn_blocking(move || cmd.output()).await + }) + .await + .map_err(|_| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::Owned("Command execution timed out".to_string()), + data: None, + })? + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::Owned(format!("Failed to spawn command: {e}")), + data: None, + })? + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::Owned(format!("Command execution failed: {e}")), + data: None, + })?; + + // log execution of command + info!("Execute command: {cmd_str}"); + Ok(output) + } +} + +impl CommandRunner for BashServer {} + +#[tool_router] +impl BashServer { + /// Create a new BashServer instance + /// Attempts to load configuration from config.toml, creates validator if successful + pub fn new() -> Self { + let tool_router = Self::tool_router(); + if let Ok(config) = Config::read_config("config.toml") + .inspect_err(|e| eprintln!("read config.toml fail, error: {e}")) + { + let blacklist = config.blacklist; + let whitelist = config.whitelist; + BashServer { + validator: Some(Validator::new(blacklist, whitelist)), + tool_router, + } + } else { + Self { + validator: None, + tool_router, + } + } + } + + /// Internal method for executing commands via default shell + /// Supports validation toggle and handles cross-platform shell differences + async fn _all_execute_via_default_shell( + &self, + need_validate: bool, + request: DefaultExecuteRequest, + ) -> Result<CallToolResult, ErrorData> { + let timeout_duration = + std::time::Duration::from_secs(request.timeout_seconds.unwrap_or(30)); + + let mut cmd = if cfg!(target_os = "linux") { + let mut cmd = Command::new("bash"); + cmd.arg("-c"); + cmd + } else if cfg!(target_os = "windows") { + let mut cmd = Command::new("powershell"); + cmd.arg("-c"); + cmd + } else { + let mut cmd = Command::new("sh"); + cmd.arg("-c"); + cmd + }; + cmd.arg(&request.command); + + // Set working directory if provided + if let Some(working_dir) = &request.working_dir { + cmd.current_dir(working_dir); + } + + // Set environment variables if provided + if let Some(env_vars) = &request.env_vars { + for (key, value) in env_vars { + cmd.env(key, value); + } + } + + // Validate the commands + if let Some(validator) = &self.validator + && need_validate + { + let program = cmd.get_program(); + let mut full_args: Vec<&OsStr> = vec![program]; + let args: Vec<&OsStr> = cmd.get_args().collect(); + full_args.extend(args); + validator.is_unsafe_command(full_args)?; + } + + let output: Output = Self::execute_command_with_timeout(timeout_duration, cmd).await?; + + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + let exit_code = output.status.code().unwrap_or(-1); + let success = output.status.success(); + + let response = DefaultExecuteResponse { + stdout, + stderr, + exit_code, + success, + parsed_data: Value::Null, + }; + Ok(CallToolResult::success(vec![Content::json(response)?])) + } + + #[tool(description = "Execute commands using default shell in all kinds of os")] + async fn all_execute_via_default_shell( + &self, + Parameters(request): Parameters<DefaultExecuteRequest>, + ) -> Result<CallToolResult, ErrorData> { + self._all_execute_via_default_shell(true, request).await + } + + #[tool(description = "Execute a python script")] + async fn unix_execute_python( + &self, + Parameters(request): Parameters<DefaultExecuteRequest>, + ) -> Result<CallToolResult, ErrorData> { + let timeout_duration = + std::time::Duration::from_secs(request.timeout_seconds.unwrap_or(30)); + + // Check if /usr/bin/env exists before proceeding + let env_exists = std::path::Path::new("/usr/bin/env").exists(); + if !env_exists { + return Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::Owned( + "Python execution failed: /usr/bin/env not found on system".to_string(), + ), + data: None, + }); + } + + let mut cmd = Command::new("/usr/bin/env"); + cmd.arg("python3").arg("-c").arg(&request.command); + + // Set working directory if provided + if let Some(working_dir) = &request.working_dir { + cmd.current_dir(working_dir); + } + + // Set environment variables if provided + if let Some(env_vars) = &request.env_vars { + for (key, value) in env_vars { + cmd.env(key, value); + } + } + + let output: Output = Self::execute_command_with_timeout(timeout_duration, cmd).await?; + + // log the execution of python + info!("Execute python script: {}", &request.command); + + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + let exit_code = output.status.code().unwrap_or(-1); + let success = output.status.success(); + + let response = DefaultExecuteResponse { + stdout, + stderr, + exit_code, + success, + parsed_data: Value::Null, + }; + Ok(CallToolResult::success(vec![Content::json(response)?])) + } + + /// Execute a Unix script by writing it to a temporary file + /// Creates a temporary shell script file with proper permissions and executes it + #[tool(description = "Execute a unix script")] + async fn unix_execute_script( + &self, + Parameters(request): Parameters<DefaultExecuteRequest>, + ) -> Result<CallToolResult, ErrorData> { + let timeout_duration = + std::time::Duration::from_secs(request.timeout_seconds.unwrap_or(30)); + + // Write the string to a temporary file + let tmp_dir = env::temp_dir(); + // Generate a random script name + let tmp_name = Uuid::new_v4().to_string(); + let script_path = tmp_dir.join(format!("{tmp_name}.sh")); + { + let mut file = + File::create(&script_path).expect("Can not create temporary script file"); + file.write_all(request.command.as_bytes()) + .expect("Write to script file error"); + // Set execution mode + let mut perms = file.metadata().unwrap().permissions(); + perms.set_mode(0o755); + fs::set_permissions(&script_path, perms).expect("Set mode 755 fail"); Review Comment: Using .expect() on file permission operations can cause panics and may leave temporary files with incorrect permissions. Consider using proper error handling and cleanup to ensure security. ```suggestion if let Err(e) = fs::set_permissions(&script_path, perms) { error!("Failed to set permissions on {}: {}", script_path.display(), e); if let Err(cleanup_err) = fs::remove_file(&script_path) { error!("Failed to clean up temporary file {}: {}", script_path.display(), cleanup_err); } return Err(ErrorData::new("Failed to set script file permissions")); } ``` ########## mcp-servers/mcp-bash-server/src/common/bash_server.rs: ########## @@ -0,0 +1,943 @@ +//! Bash Server Implementation for MCP (Model Context Protocol) +//! +//! This module provides a bash command execution server that can run shell commands +//! safely with validation and timeout controls. It supports multiple operating systems +//! and provides various execution methods for different use cases. + +use rmcp::model::Content; +use rmcp::{ + RoleServer, ServerHandler, + handler::server::{ + router::tool::ToolRouter, + tool::{IntoCallToolResult, Parameters}, + }, + model::*, + schemars, + serde_json::Value, + service::RequestContext, + tool, +}; +use rmcp::{serde_json, tool_handler, tool_router}; +use serde::{Deserialize, Serialize}; +use std::ffi::OsStr; +use std::process::Output; +use std::{ + borrow::Cow, + env, + fs::{self, File}, + io::Write, + os::unix::fs::PermissionsExt, + process::Command, +}; +use tracing::error; +use tracing::info; +use uuid::Uuid; + +use crate::common::config::Config; +use crate::common::validator::Validator; + +/// Request structure for executing bash commands +/// Contains all necessary parameters for command execution including +/// working directory, environment variables, and timeout settings +#[derive(Debug, Deserialize, schemars::JsonSchema)] +pub struct DefaultExecuteRequest { + #[schemars(description = "The bash command or script to execute")] + pub command: String, + #[schemars(description = "Working directory for the command (optional)")] + pub working_dir: Option<String>, + #[schemars(description = "Environment variables (optional)")] + pub env_vars: Option<std::collections::HashMap<String, String>>, + #[schemars(description = "Timeout in seconds (default: 30)")] + pub timeout_seconds: Option<u64>, +} + +/// Response structure for command execution results +/// Contains stdout, stderr, exit code, success status and any parsed data +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct DefaultExecuteResponse { + /// Standard output from the executed command + pub stdout: String, + /// Standard error output from the executed command + pub stderr: String, + /// Exit code returned by the command (0 for success, non-zero for failure) + pub exit_code: i32, + /// Whether the command executed successfully (exit code 0) + pub success: bool, + /// Additional parsed data from command output (JSON format) + pub parsed_data: serde_json::Value, +} + +impl IntoCallToolResult for DefaultExecuteResponse { + fn into_call_tool_result(self) -> Result<CallToolResult, ErrorData> { + let content = if self.success { + format!( + "Command executed successfully (exit code: {})\n\nSTDOUT:\n{}\n\nSTDERR:\n{}", + self.exit_code, self.stdout, self.stderr + ) + } else { + format!( + "Command failed (exit code: {})\n\nSTDOUT:\n{}\n\nSTDERR:\n{}", + self.exit_code, self.stdout, self.stderr + ) + }; + + Ok(CallToolResult { + content: vec![Content::text(content)], + is_error: Some(!self.success), + }) + } +} + +/// Main bash server implementation that handles command execution +/// with optional validation and timeout controls +#[derive(Debug, Clone)] +pub struct BashServer { + validator: Option<Validator>, + tool_router: ToolRouter<BashServer>, +} + +/// Trait for command execution utilities +/// Provides common functionality for command formatting and execution +pub trait CommandRunner { + /// Convert a Command instance to a readable string representation + /// Handles proper quoting of arguments containing spaces + fn stringify_command(cmd: &Command) -> String { + let program = cmd.get_program().to_string_lossy(); + let args = cmd + .get_args() + .map(|arg| { + let s = arg.to_string_lossy(); + if s.contains(' ') || s.contains('"') { + format!("{s:?}") + } else { + s.to_string() + } + }) + .collect::<Vec<String>>() + .join(" "); + + format!("{program} {args}") + } + + /// Execute a command with timeout protection + /// Returns command output or timeout error + async fn execute_command_with_timeout( + timeout: std::time::Duration, + mut cmd: Command, + ) -> Result<Output, ErrorData> { + let cmd_str = Self::stringify_command(&cmd); + // Execute command with timeout + let output = tokio::time::timeout(timeout, async { + tokio::task::spawn_blocking(move || cmd.output()).await + }) + .await + .map_err(|_| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::Owned("Command execution timed out".to_string()), + data: None, + })? + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::Owned(format!("Failed to spawn command: {e}")), + data: None, + })? + .map_err(|e| ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::Owned(format!("Command execution failed: {e}")), + data: None, + })?; + + // log execution of command + info!("Execute command: {cmd_str}"); + Ok(output) + } +} + +impl CommandRunner for BashServer {} + +#[tool_router] +impl BashServer { + /// Create a new BashServer instance + /// Attempts to load configuration from config.toml, creates validator if successful + pub fn new() -> Self { + let tool_router = Self::tool_router(); + if let Ok(config) = Config::read_config("config.toml") + .inspect_err(|e| eprintln!("read config.toml fail, error: {e}")) + { + let blacklist = config.blacklist; + let whitelist = config.whitelist; + BashServer { + validator: Some(Validator::new(blacklist, whitelist)), + tool_router, + } + } else { + Self { + validator: None, + tool_router, + } + } + } + + /// Internal method for executing commands via default shell + /// Supports validation toggle and handles cross-platform shell differences + async fn _all_execute_via_default_shell( + &self, + need_validate: bool, + request: DefaultExecuteRequest, + ) -> Result<CallToolResult, ErrorData> { + let timeout_duration = + std::time::Duration::from_secs(request.timeout_seconds.unwrap_or(30)); + + let mut cmd = if cfg!(target_os = "linux") { + let mut cmd = Command::new("bash"); + cmd.arg("-c"); + cmd + } else if cfg!(target_os = "windows") { + let mut cmd = Command::new("powershell"); + cmd.arg("-c"); + cmd + } else { + let mut cmd = Command::new("sh"); + cmd.arg("-c"); + cmd + }; + cmd.arg(&request.command); + + // Set working directory if provided + if let Some(working_dir) = &request.working_dir { + cmd.current_dir(working_dir); + } + + // Set environment variables if provided + if let Some(env_vars) = &request.env_vars { + for (key, value) in env_vars { + cmd.env(key, value); + } + } + + // Validate the commands + if let Some(validator) = &self.validator + && need_validate + { + let program = cmd.get_program(); + let mut full_args: Vec<&OsStr> = vec![program]; + let args: Vec<&OsStr> = cmd.get_args().collect(); + full_args.extend(args); + validator.is_unsafe_command(full_args)?; + } + + let output: Output = Self::execute_command_with_timeout(timeout_duration, cmd).await?; + + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + let exit_code = output.status.code().unwrap_or(-1); + let success = output.status.success(); + + let response = DefaultExecuteResponse { + stdout, + stderr, + exit_code, + success, + parsed_data: Value::Null, + }; + Ok(CallToolResult::success(vec![Content::json(response)?])) + } + + #[tool(description = "Execute commands using default shell in all kinds of os")] + async fn all_execute_via_default_shell( + &self, + Parameters(request): Parameters<DefaultExecuteRequest>, + ) -> Result<CallToolResult, ErrorData> { + self._all_execute_via_default_shell(true, request).await + } + + #[tool(description = "Execute a python script")] + async fn unix_execute_python( + &self, + Parameters(request): Parameters<DefaultExecuteRequest>, + ) -> Result<CallToolResult, ErrorData> { + let timeout_duration = + std::time::Duration::from_secs(request.timeout_seconds.unwrap_or(30)); + + // Check if /usr/bin/env exists before proceeding + let env_exists = std::path::Path::new("/usr/bin/env").exists(); + if !env_exists { + return Err(ErrorData { + code: ErrorCode::INTERNAL_ERROR, + message: Cow::Owned( + "Python execution failed: /usr/bin/env not found on system".to_string(), + ), + data: None, + }); + } + + let mut cmd = Command::new("/usr/bin/env"); + cmd.arg("python3").arg("-c").arg(&request.command); + + // Set working directory if provided + if let Some(working_dir) = &request.working_dir { + cmd.current_dir(working_dir); + } + + // Set environment variables if provided + if let Some(env_vars) = &request.env_vars { + for (key, value) in env_vars { + cmd.env(key, value); + } + } + + let output: Output = Self::execute_command_with_timeout(timeout_duration, cmd).await?; + + // log the execution of python + info!("Execute python script: {}", &request.command); + + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + let exit_code = output.status.code().unwrap_or(-1); + let success = output.status.success(); + + let response = DefaultExecuteResponse { + stdout, + stderr, + exit_code, + success, + parsed_data: Value::Null, + }; + Ok(CallToolResult::success(vec![Content::json(response)?])) + } + + /// Execute a Unix script by writing it to a temporary file + /// Creates a temporary shell script file with proper permissions and executes it + #[tool(description = "Execute a unix script")] + async fn unix_execute_script( + &self, + Parameters(request): Parameters<DefaultExecuteRequest>, + ) -> Result<CallToolResult, ErrorData> { + let timeout_duration = + std::time::Duration::from_secs(request.timeout_seconds.unwrap_or(30)); + + // Write the string to a temporary file + let tmp_dir = env::temp_dir(); + // Generate a random script name + let tmp_name = Uuid::new_v4().to_string(); + let script_path = tmp_dir.join(format!("{tmp_name}.sh")); + { + let mut file = + File::create(&script_path).expect("Can not create temporary script file"); + file.write_all(request.command.as_bytes()) + .expect("Write to script file error"); + // Set execution mode + let mut perms = file.metadata().unwrap().permissions(); + perms.set_mode(0o755); + fs::set_permissions(&script_path, perms).expect("Set mode 755 fail"); Review Comment: Using .expect() for file write operations can cause panics and may leave temporary files in an inconsistent state. Consider using proper error handling with Result types. ```suggestion let mut file = match File::create(&script_path) { Ok(f) => f, Err(e) => { error!("Failed to create temporary script file: {}", e); return Err(ErrorData::new("FileCreationError", e.to_string())); } }; if let Err(e) = file.write_all(request.command.as_bytes()) { error!("Failed to write to script file: {}", e); return Err(ErrorData::new("FileWriteError", e.to_string())); } // Set execution mode let mut perms = match file.metadata() { Ok(meta) => meta.permissions(), Err(e) => { error!("Failed to retrieve file metadata: {}", e); return Err(ErrorData::new("MetadataError", e.to_string())); } }; perms.set_mode(0o755); if let Err(e) = fs::set_permissions(&script_path, perms) { error!("Failed to set file permissions: {}", e); return Err(ErrorData::new("PermissionError", e.to_string())); } ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
